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

Add DivRemLimb and RemLimb traits #496

Merged
merged 5 commits into from
Dec 23, 2023

Conversation

fjarri
Copy link
Contributor

@fjarri fjarri commented Dec 23, 2023

This is the next step for entropyxyz/crypto-primes#36

  • add more bounds on Monty::Params (goes in line with the bounds on Monty and Integer);
  • add From<Limb> bound for Integer;
  • use original num-bigint instead of num-bigint-dig for compatibility with num-modular (used in tests);
  • fix some vartime usage in constant time division methods
  • add DivRemLimb and RemLimb traits (and separate rem_limb() methods - there's a significant speedup, and these are the ones I actually use in crypto-primes)

@fjarri fjarri marked this pull request as draft December 23, 2023 00:20
@fjarri
Copy link
Contributor Author

fjarri commented Dec 23, 2023

Making a draft temporarily, need to add comments about the unwraps. Also probably a good idea for it to be a draft until we figure out what the API should be.

@tarcieri
Copy link
Member

tarcieri commented Dec 23, 2023

I don't need gcd_vartime() for crypto-primes, it was just added for completeness sake.

Since we have Bernstein-Yang now defined on both Uint and BoxedUint (just landed in #497), perhaps it's unnecessary?

I can add a trait for computing GCD and bound Integer on it.

If you do want to include it for completeness, it'd be good to check if it's actually faster than the constant-time implementation (Bernstein-Yang is surprisingly fast), and then I'd prefer it be exposed as a provided Integer::gcd_vartime method (though the implementation could look like it does now, just pub(crate))

@fjarri
Copy link
Contributor Author

fjarri commented Dec 23, 2023

Since we have Bernstein-Yang now defined on both Uint and BoxedUint (just landed in #497), perhaps it's unnecessary?

But that's constant-time, correct? I'm using it in a vartime context, so running the constant time thing would lead to performance degradatation.

Let me run some benchmarks.

Cargo.toml Outdated Show resolved Hide resolved
@fjarri
Copy link
Contributor Author

fjarri commented Dec 23, 2023

Actually let me roll it back. There are a few assorted considerations:

  • The GCD and Jacobi symbol functions I need are somewhat specialized, and if Integer provides the required functionality, I can just keep them in crypto-primes for now. Plus I'm not exactly sure how to implement Jacobi symbol in terms of Bernstein-Yang (although I remember reading that it's possible).
  • The vartime division algorithm we have is super inefficient, I want to make a separate PR for it.
  • Vartime division-by-limb can be added, but that's also a separate PR (although the constant-time one is already fast enough since in my application I only need to do it once).

@fjarri fjarri changed the title Vartime GCD and Jacobi Add DivRemLimb and RemLimb traits Dec 23, 2023
@fjarri fjarri marked this pull request as ready for review December 23, 2023 06:30
@tarcieri tarcieri merged commit 7c65c66 into RustCrypto:master Dec 23, 2023
17 checks passed
@fjarri fjarri deleted the uint-adjustments branch December 23, 2023 20:14
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