Skip to content
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

Further coverage of UintLike #37

Closed
wants to merge 39 commits into from

Conversation

xuganyu96
Copy link
Contributor

@xuganyu96 xuganyu96 commented Dec 14, 2023

This PR is a continuation of #36 (and thus is a halfway point to #34), though it is still WIP.

#36 in its current state will not compile. There are many unimplemented!(). Some of the API's from crypto-bigint has also changed.

This PR improves on #36 by:

  • Completing the implementation of jacobi_small and gcd_small, which includes transitioning hazmat::jacobi and hazmat::gcd to be implemented using <T: UintLike> instead of Uint<L>
  • Implement random_bits correctly
  • Try implement UintLike for BoxedUint
  • Write test cases and bench marks for prime generation using BoxedUint

As of ba9d3c5633020e7b50e5569e6ddb8e831fbd3d2c the crate will compile, but not all tests will pass:

@xuganyu96
Copy link
Contributor Author

Now all tests pass.

@xuganyu96 xuganyu96 marked this pull request as draft December 14, 2023 06:06
let d = Option::from((n >> s).checked_add(&Uint::<L>::ONE)).expect("Integer overflow");
// TODO: shr(s-1).shr(1) is a hack around the fact that a full right shift will panic
// see https://github.com/RustCrypto/crypto-bigint/commit/55312b6aa71#r134960147
let d = Option::from((n.clone().shr(s - 1).shr(1)).checked_add(&T::one()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole thing can be vartime, so you can just use shr_vartime(s).unwrap_or(T::zero())

src/presets.rs Outdated

/// Returns a random prime of size `bit_length` using [`OsRng`] as the RNG.
/// If `bit_length` is `None`, the full size of `Uint<L>` is used.
/// TODO: bits_precision?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think bit_length is more appropriate in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the confusion. I put to TODO here to remind myself to add documentation for the parameter bits_length. Previously the precision is implied with the generics <const L: usize>, but now since we need to accommodate both Uint and BoxedUint, the size of the big integer will have to be passed in at runtime as an extra parameter.

@tarcieri
Copy link

Related: RustCrypto/crypto-bigint#425

@xuganyu96
Copy link
Contributor Author

xuganyu96 commented Dec 14, 2023

I made an attempt to implement UintLike for BoxedUint and UintModLike for BoxedResidue. There are a few missing implementations that will need to be merged into RustCrypto/crypto-bigint

  • BoxedUint::bit_vartime (trivial implementation)
  • BoxedUint::trailing_ones (trivial implementation)
  • BoxedUint::sqrt_vartime, which is non-trivial
    • Needs cmp_vartime?
    • Needs wrapping_div_vartime?
    • Needs is_nonzero?
  • BoxedUint::shr_vartime, which is already implemented, but behaves differently from Uint::shr_vartime
  • BoxedUint::shl_vartime, same as BoxedUint::shr_vartime
  • BoxedUint::div_rem_limb_with_reciprocal and BoxedUint::div_rem_limb
    • both makes call to div_rem_limb_with_reciprocal
    • div_rem_limb_with_reciprocal makes call to shl_limb
    • div2by1 can be reused from src/uint/div_limb.rs
    • BoxedUint needs to be instantiated from an array of Limb
  • BoxedResidue::div_by_2

@xuganyu96
Copy link
Contributor Author

There seems to be a bug with BoxedResidue::square (see RustCrypto/crypto-bigint#441). This can be temporarily side-stepped by calling retrieving and re-instantiating the BoxedResidue, although it is certainly not the ideal solution.

At least for now, it is possible to run generate_prime and generate_safe_prime with BoxedUint. Two simple tests have been added to presets.rs to demonstrate the progress.

@xuganyu96 xuganyu96 marked this pull request as ready for review December 17, 2023 01:50
@xuganyu96
Copy link
Contributor Author

xuganyu96 commented Dec 17, 2023

@fjarri @tarcieri
Missing arithmetic operations for BoxedUint have been implemented: RustCrypto/crypto-bigint#436. Prime generation for BoxedUint is tested in presets.rs.

There are a two awkward API's. T::one() and T:zero() both need to be replaced with T::one_with_precision() and T::zero_with_precision, which is a bit weird for stack-allocated Uint; also, many T::from(u32) must be immediately followed by a widen(bits_precision), which is also weird for stack-allocated Uint.

Also, the main public API generate_prime and generate_safe_prime need to accommodate the new argument bits_precision. I think with some clear explanation in the documentation, the user should be able to understand, but maybe we can do better. I am open to ideas (maybe we can make generate_prime into a macro?)

Thank you!

tarcieri pushed a commit to RustCrypto/crypto-bigint that referenced this pull request Dec 21, 2023
@xuganyu96
Copy link
Contributor Author

Closing this PR to consolidate work on #36

@xuganyu96 xuganyu96 closed this Dec 24, 2023
@xuganyu96 xuganyu96 deleted the uint-trait branch December 25, 2023 12:28
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.

3 participants