Skip to content

Removed bigint dependency and changed RSA algorithm to use u128 #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

mgostIH
Copy link

@mgostIH mgostIH commented Jul 2, 2020

This removes a big dependency by using Rust's available u128 for the RSA step. I checked that tests pass and the change is quite simple. Should reduce compile times quite a lot!

@mlafeldt
Copy link
Owner

mlafeldt commented Jul 3, 2020

Hey @mgostIH, thanks for the PR!

I actually don't mind longer compile times if the code is more readable and maintainable by using dedicated crates. That said, I'm not sure if I should merge this change or not.

In any case, it's good to see that it can be done with u128. 👍

Mind telling me where exactly you copied that mod_pow from?

@mgostIH
Copy link
Author

mgostIH commented Jul 3, 2020

From Rust's pow and just added a couple of % operations: https://doc.rust-lang.org/src/core/num/mod.rs.html#3908-3928
Also another reason for using this code is that it should allow for no_std, but I am too sure on what kind of operations you do elsewhere that would prevent it

@mlafeldt
Copy link
Owner

mlafeldt commented Jul 3, 2020

Ah! The crate already supports no_std, see #1

@mgostIH
Copy link
Author

mgostIH commented Jul 3, 2020

Indeed, but it would still require the alloc crate, while from what I am seeing this whole crate could work without it, it's only used inside tests for vec! but it could be easily be removed. The advantage of removing alloc is that someone using your crate externally wouldn't require to set up a fixed allocator or something basic, and expecially if it's for small devices that work directly on the PS2 it would be an additional benefit.
An additional way to make this code simpler would instead of mutating input arguments to return a tuple (u32, u32), possibly a Result too in order to detect the case where there was a failure in decoding the values due to the check of code < modulus

@mlafeldt
Copy link
Owner

mlafeldt commented Jul 3, 2020

I justed merged #4, which removes the last remaining vector. So now, String and Vec are only needed for tests. :)

@mgostIH
Copy link
Author

mgostIH commented Jul 3, 2020

So are you going to merge this and remove the usage of the heap entirely?

@mlafeldt mlafeldt closed this Nov 21, 2020
@mlafeldt mlafeldt deleted the branch mlafeldt:master November 21, 2020 16:12
@mlafeldt
Copy link
Owner

mlafeldt commented Aug 8, 2021

@mgostIH I noticed that the PR was automatically closed when I renamed master to main. That wasn't my intention, sorry.

That being said, I'm still going to stick with modpow function though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants