Skip to content

Enforce valid modulus for Residue and associated macros#243

Merged
tarcieri merged 4 commits intoRustCrypto:masterfrom
AaronFeickert:compile-modulus-check
May 26, 2023
Merged

Enforce valid modulus for Residue and associated macros#243
tarcieri merged 4 commits intoRustCrypto:masterfrom
AaronFeickert:compile-modulus-check

Conversation

@AaronFeickert
Copy link
Contributor

@AaronFeickert AaronFeickert commented May 25, 2023

This PR follows in the footsteps of #240 and enforces a valid (odd) modulus for compile-time constructions.

It updates impl_modulus! to panic on an invalid modulus.

For Residue, it adds a panic to new and deprecates it, and adds a new_checked constructor that returns a CtOption based on the modulus validity. This is added since it's possible to define a modulus without impl_modulus! and use Residue directly.

It also updates const_residue! to use Residue::new_checked and unwrap, so it will also panic on an invalid modulus.

Closes #232.

@AaronFeickert AaronFeickert force-pushed the compile-modulus-check branch from 8c7f233 to 8e7e982 Compare May 25, 2023 16:22
@AaronFeickert
Copy link
Contributor Author

@fjarri: Would appreciate a review if you're available.

@AaronFeickert
Copy link
Contributor Author

@fjarri: I'll wait to address your review comments until we decide on a common deprecation strategy, as suggested by @tarcieri in #240.

@AaronFeickert AaronFeickert force-pushed the compile-modulus-check branch from e6913c7 to c0b42fa Compare May 26, 2023 03:43
Co-authored-by: Tony Arcieri <bascule@gmail.com>
@tarcieri
Copy link
Member

Ok, I guess I'll go ahead and merge this, although I'd really, really like to find a way to get rid of new_checked

@tarcieri tarcieri merged commit b158ced into RustCrypto:master May 26, 2023
@AaronFeickert
Copy link
Contributor Author

Ok, I guess I'll go ahead and merge this, although I'd really, really like to find a way to get rid of new_checked

Agreed. Being able to rely solely on compile-time checks would be much more elegant and in line with the intent of the design, for sure.

@AaronFeickert AaronFeickert deleted the compile-modulus-check branch May 26, 2023 16:33
@tarcieri tarcieri mentioned this pull request Sep 4, 2023
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.

Residue has improper behavior with even moduli

4 participants

Comments