From a063e06fb6a78668ddcaaa4dae02d84d1103aac0 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 22 May 2023 13:10:52 -0500 Subject: [PATCH 1/8] Enforce odd modulus for `DynResidueParams` --- benches/bench.rs | 2 + src/uint/modular/runtime_mod.rs | 78 ++++++++++++++++++++- src/uint/modular/runtime_mod/runtime_add.rs | 1 + src/uint/modular/runtime_mod/runtime_sub.rs | 1 + tests/proptests.rs | 3 + 5 files changed, 83 insertions(+), 2 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index 8be5f5928..58df3b94c 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -73,6 +73,7 @@ fn bench_division(group: &mut BenchmarkGroup<'_, M>) { }); } +#[allow(deprecated)] fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); group.bench_function("multiplication, U256*U256", |b| { @@ -103,6 +104,7 @@ fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { }); } +#[allow(deprecated)] fn bench_montgomery_conversion(group: &mut BenchmarkGroup<'_, M>) { group.bench_function("DynResidueParams creation", |b| { b.iter_batched( diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index 32b078599..d1c13a5e7 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -20,7 +20,7 @@ mod runtime_pow; /// Subtractions between residues with a modulus set at runtime mod runtime_sub; -/// The parameters to efficiently go to and from the Montgomery form for a modulus provided at runtime. +/// The parameters to efficiently go to and from the Montgomery form for an odd modulus provided at runtime. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct DynResidueParams { // The constant modulus @@ -37,8 +37,18 @@ pub struct DynResidueParams { } impl DynResidueParams { - /// Instantiates a new set of `ResidueParams` representing the given `modulus`. + #[deprecated( + since = "0.5.3", + note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." + )] + /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. + /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. pub const fn new(modulus: &Uint) -> Self { + // The modulus must be odd + if modulus.ct_is_odd().to_u8() != 1 { + panic!("Modulus must be odd."); + } + let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; @@ -59,6 +69,38 @@ impl DynResidueParams { } } + /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. + /// Returns an `Option` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. + pub const fn new_checked(modulus: &Uint) -> Option { + match modulus.ct_is_odd().to_u8() { + // The modulus is odd, which is valid + 1 => { + let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); + let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; + + // Since we are calculating the inverse modulo (Word::MAX+1), + // we can take the modulo right away and calculate the inverse of the first limb only. + let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); + let mod_neg_inv = Limb( + Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0), + ); + + let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); + + Some(Self { + modulus: *modulus, + r, + r2, + r3, + mod_neg_inv, + }) + } + + // The modulus is even, which is invalid + _ => None, + } + } + /// Returns the modulus which was used to initialize these parameters. pub const fn modulus(&self) -> &Uint { &self.modulus @@ -194,3 +236,35 @@ impl zeroize::Zeroize for DynResidue { self.montgomery_form.zeroize() } } + +#[cfg(test)] +mod test { + use super::*; + use crate::nlimbs; + + const LIMBS: usize = nlimbs!(64); + + #[test] + #[allow(deprecated)] + // Test that a valid modulus yields `DynResidueParams` + fn test_valid_modulus() { + let valid_modulus = Uint::::from(3u8); + + assert!(DynResidueParams::::new_checked(&valid_modulus).is_some()); + DynResidueParams::::new(&valid_modulus); + } + + #[test] + // Test that an invalid checked modulus does not yield `DynResidueParams` + fn test_invalid_checked_modulus() { + assert!(DynResidueParams::::new_checked(&Uint::from(2u8)).is_none()); + } + + #[test] + #[allow(deprecated)] + #[should_panic] + // Tets that an invalid modulus panics + fn test_invalid_modulus() { + DynResidueParams::::new(&Uint::from(2u8)); + } +} diff --git a/src/uint/modular/runtime_mod/runtime_add.rs b/src/uint/modular/runtime_mod/runtime_add.rs index eb4708603..e09fe0ff4 100644 --- a/src/uint/modular/runtime_mod/runtime_add.rs +++ b/src/uint/modular/runtime_mod/runtime_add.rs @@ -69,6 +69,7 @@ mod tests { }; #[test] + #[allow(deprecated)] fn add_overflow() { let params = DynResidueParams::new(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", diff --git a/src/uint/modular/runtime_mod/runtime_sub.rs b/src/uint/modular/runtime_mod/runtime_sub.rs index dd6fd84c8..801a7bb75 100644 --- a/src/uint/modular/runtime_mod/runtime_sub.rs +++ b/src/uint/modular/runtime_mod/runtime_sub.rs @@ -69,6 +69,7 @@ mod tests { }; #[test] + #[allow(deprecated)] fn sub_overflow() { let params = DynResidueParams::new(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", diff --git a/tests/proptests.rs b/tests/proptests.rs index 572d990d1..71dfc1159 100644 --- a/tests/proptests.rs +++ b/tests/proptests.rs @@ -251,6 +251,7 @@ proptest! { } #[test] + #[allow(deprecated)] fn residue_pow(a in uint_mod_p(P), b in uint()) { let a_bi = to_biguint(&a); let b_bi = to_biguint(&b); @@ -266,6 +267,7 @@ proptest! { } #[test] + #[allow(deprecated)] fn residue_pow_bounded_exp(a in uint_mod_p(P), b in uint(), exponent_bits in any::()) { let b_masked = b & (U256::ONE << exponent_bits.into()).wrapping_sub(&U256::ONE); @@ -284,6 +286,7 @@ proptest! { } #[test] + #[allow(deprecated)] fn residue_div_by_2(a in uint_mod_p(P)) { let a_bi = to_biguint(&a); let p_bi = to_biguint(&P); From 71f6d028511a89eaf79b128c012fd739d2dccd63 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 22 May 2023 13:28:44 -0500 Subject: [PATCH 2/8] Refactor --- src/uint/modular/runtime_mod.rs | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index d1c13a5e7..ecb63dea4 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -37,35 +37,16 @@ pub struct DynResidueParams { } impl DynResidueParams { + /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. + /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. #[deprecated( since = "0.5.3", note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." )] - /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. - /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. pub const fn new(modulus: &Uint) -> Self { - // The modulus must be odd - if modulus.ct_is_odd().to_u8() != 1 { - panic!("Modulus must be odd."); - } - - let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); - let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; - - // Since we are calculating the inverse modulo (Word::MAX+1), - // we can take the modulo right away and calculate the inverse of the first limb only. - let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); - let mod_neg_inv = - Limb(Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0)); - - let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); - - Self { - modulus: *modulus, - r, - r2, - r3, - mod_neg_inv, + match Self::new_checked(modulus) { + Some(params) => params, + None => panic!("modulus must be odd"), } } From e3aa66edc6b963b49679cebb38cc756a0be2788b Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 22 May 2023 13:58:24 -0500 Subject: [PATCH 3/8] Review comments --- benches/bench.rs | 12 +++--- src/uint/modular/runtime_mod.rs | 48 ++++++++++----------- src/uint/modular/runtime_mod/runtime_add.rs | 6 +-- src/uint/modular/runtime_mod/runtime_sub.rs | 6 +-- tests/proptests.rs | 9 ++-- 5 files changed, 36 insertions(+), 45 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index 58df3b94c..bf8a7e1a5 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -73,9 +73,8 @@ fn bench_division(group: &mut BenchmarkGroup<'_, M>) { }); } -#[allow(deprecated)] fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { - let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); + let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); group.bench_function("multiplication, U256*U256", |b| { b.iter_batched( || { @@ -89,7 +88,7 @@ fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { }); let m = U256::random(&mut OsRng) | U256::ONE; - let params = DynResidueParams::new(&m); + let params = DynResidueParams::new_checked(&m).unwrap(); group.bench_function("modpow, U256^U256", |b| { b.iter_batched( || { @@ -104,17 +103,16 @@ fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { }); } -#[allow(deprecated)] fn bench_montgomery_conversion(group: &mut BenchmarkGroup<'_, M>) { group.bench_function("DynResidueParams creation", |b| { b.iter_batched( || U256::random(&mut OsRng) | U256::ONE, - |modulus| DynResidueParams::new(&modulus), + |modulus| DynResidueParams::new_checked(&modulus).unwrap(), BatchSize::SmallInput, ) }); - let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); + let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); group.bench_function("DynResidue creation", |b| { b.iter_batched( || U256::random(&mut OsRng), @@ -123,7 +121,7 @@ fn bench_montgomery_conversion(group: &mut BenchmarkGroup<'_, M> ) }); - let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); + let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); group.bench_function("DynResidue retrieve", |b| { b.iter_batched( || DynResidue::new(&U256::random(&mut OsRng), params), diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index ecb63dea4..6ce3afbef 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -53,33 +53,29 @@ impl DynResidueParams { /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. /// Returns an `Option` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. pub const fn new_checked(modulus: &Uint) -> Option { - match modulus.ct_is_odd().to_u8() { - // The modulus is odd, which is valid - 1 => { - let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); - let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; - - // Since we are calculating the inverse modulo (Word::MAX+1), - // we can take the modulo right away and calculate the inverse of the first limb only. - let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); - let mod_neg_inv = Limb( - Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0), - ); - - let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); - - Some(Self { - modulus: *modulus, - r, - r2, - r3, - mod_neg_inv, - }) - } - - // The modulus is even, which is invalid - _ => None, + // A modulus must be odd to be valid + if modulus.ct_is_odd().to_u8() == 0 { + return None; } + + let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); + let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; + + // Since we are calculating the inverse modulo (Word::MAX+1), + // we can take the modulo right away and calculate the inverse of the first limb only. + let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); + let mod_neg_inv = + Limb(Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0)); + + let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); + + Some(Self { + modulus: *modulus, + r, + r2, + r3, + mod_neg_inv, + }) } /// Returns the modulus which was used to initialize these parameters. diff --git a/src/uint/modular/runtime_mod/runtime_add.rs b/src/uint/modular/runtime_mod/runtime_add.rs index e09fe0ff4..c497ee0d1 100644 --- a/src/uint/modular/runtime_mod/runtime_add.rs +++ b/src/uint/modular/runtime_mod/runtime_add.rs @@ -69,11 +69,11 @@ mod tests { }; #[test] - #[allow(deprecated)] fn add_overflow() { - let params = DynResidueParams::new(&U256::from_be_hex( + let params = DynResidueParams::new_checked(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", - )); + )) + .unwrap(); let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); diff --git a/src/uint/modular/runtime_mod/runtime_sub.rs b/src/uint/modular/runtime_mod/runtime_sub.rs index 801a7bb75..46d0dcfe2 100644 --- a/src/uint/modular/runtime_mod/runtime_sub.rs +++ b/src/uint/modular/runtime_mod/runtime_sub.rs @@ -69,11 +69,11 @@ mod tests { }; #[test] - #[allow(deprecated)] fn sub_overflow() { - let params = DynResidueParams::new(&U256::from_be_hex( + let params = DynResidueParams::new_checked(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", - )); + )) + .unwrap(); let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); diff --git a/tests/proptests.rs b/tests/proptests.rs index 71dfc1159..26996db95 100644 --- a/tests/proptests.rs +++ b/tests/proptests.rs @@ -251,7 +251,6 @@ proptest! { } #[test] - #[allow(deprecated)] fn residue_pow(a in uint_mod_p(P), b in uint()) { let a_bi = to_biguint(&a); let b_bi = to_biguint(&b); @@ -259,7 +258,7 @@ proptest! { let expected = to_uint(a_bi.modpow(&b_bi, &p_bi)); - let params = DynResidueParams::new(&P); + let params = DynResidueParams::new_checked(&P).unwrap(); let a_m = DynResidue::new(&a, params); let actual = a_m.pow(&b).retrieve(); @@ -267,7 +266,6 @@ proptest! { } #[test] - #[allow(deprecated)] fn residue_pow_bounded_exp(a in uint_mod_p(P), b in uint(), exponent_bits in any::()) { let b_masked = b & (U256::ONE << exponent_bits.into()).wrapping_sub(&U256::ONE); @@ -278,7 +276,7 @@ proptest! { let expected = to_uint(a_bi.modpow(&b_bi, &p_bi)); - let params = DynResidueParams::new(&P); + let params = DynResidueParams::new_checked(&P).unwrap(); let a_m = DynResidue::new(&a, params); let actual = a_m.pow_bounded_exp(&b, exponent_bits.into()).retrieve(); @@ -286,7 +284,6 @@ proptest! { } #[test] - #[allow(deprecated)] fn residue_div_by_2(a in uint_mod_p(P)) { let a_bi = to_biguint(&a); let p_bi = to_biguint(&P); @@ -300,7 +297,7 @@ proptest! { }; let expected = to_uint(expected); - let params = DynResidueParams::new(&P); + let params = DynResidueParams::new_checked(&P).unwrap(); let a_m = DynResidue::new(&a, params); let actual = a_m.div_by_2().retrieve(); From b32a0a4fbe0130d807e6017771b90d51d02a6f4e Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 22 May 2023 14:31:57 -0500 Subject: [PATCH 4/8] Use a `CtOption` --- src/uint/modular/runtime_mod.rs | 51 +++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index 6ce3afbef..4df1b430f 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -7,6 +7,8 @@ use super::{ Retrieve, }; +use subtle::CtOption; + /// Additions between residues with a modulus set at runtime mod runtime_add; /// Multiplicative inverses of residues with a modulus set at runtime @@ -44,18 +46,9 @@ impl DynResidueParams { note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." )] pub const fn new(modulus: &Uint) -> Self { - match Self::new_checked(modulus) { - Some(params) => params, - None => panic!("modulus must be odd"), - } - } - - /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. - /// Returns an `Option` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. - pub const fn new_checked(modulus: &Uint) -> Option { - // A modulus must be odd to be valid + // A valid modulus must be odd if modulus.ct_is_odd().to_u8() == 0 { - return None; + panic!("modulus must be odd"); } let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); @@ -69,13 +62,40 @@ impl DynResidueParams { let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); - Some(Self { + Self { modulus: *modulus, r, r2, r3, mod_neg_inv, - }) + } + } + + /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. + /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. + pub fn new_checked(modulus: &Uint) -> CtOption { + let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); + let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; + + // Since we are calculating the inverse modulo (Word::MAX+1), + // we can take the modulo right away and calculate the inverse of the first limb only. + let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); + let mod_neg_inv = + Limb(Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0)); + + let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); + + // A valid modulus must be odd, which we check in constant time + CtOption::new( + Self { + modulus: *modulus, + r, + r2, + r3, + mod_neg_inv, + }, + modulus.ct_is_odd().into() + ) } /// Returns the modulus which was used to initialize these parameters. @@ -227,14 +247,15 @@ mod test { fn test_valid_modulus() { let valid_modulus = Uint::::from(3u8); - assert!(DynResidueParams::::new_checked(&valid_modulus).is_some()); + DynResidueParams::::new_checked(&valid_modulus).unwrap(); DynResidueParams::::new(&valid_modulus); } #[test] + #[should_panic] // Test that an invalid checked modulus does not yield `DynResidueParams` fn test_invalid_checked_modulus() { - assert!(DynResidueParams::::new_checked(&Uint::from(2u8)).is_none()); + DynResidueParams::::new_checked(&Uint::from(2u8)).unwrap(); } #[test] From cbd2416171f52aeb9290f221af44bfc52ac56999 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Mon, 22 May 2023 14:46:48 -0500 Subject: [PATCH 5/8] Formatting --- src/uint/modular/runtime_mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index 4df1b430f..e2be0f492 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -94,7 +94,7 @@ impl DynResidueParams { r3, mod_neg_inv, }, - modulus.ct_is_odd().into() + modulus.ct_is_odd().into(), ) } From 883fe14ace11722bc6ef09a953c8f5d0d11380c9 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 25 May 2023 16:20:49 -0500 Subject: [PATCH 6/8] Refactor constructors --- src/uint/modular/runtime_mod.rs | 51 ++++++++++++--------------------- 1 file changed, 18 insertions(+), 33 deletions(-) diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index e2be0f492..649d1b8b1 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -39,18 +39,8 @@ pub struct DynResidueParams { } impl DynResidueParams { - /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. - /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. - #[deprecated( - since = "0.5.3", - note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." - )] - pub const fn new(modulus: &Uint) -> Self { - // A valid modulus must be odd - if modulus.ct_is_odd().to_u8() == 0 { - panic!("modulus must be odd"); - } - + // Internal helper function to generate parameters; this lets us wrap the constructors more cleanly + const fn generate_params(modulus: &Uint) -> Self { let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; @@ -71,31 +61,26 @@ impl DynResidueParams { } } + /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. + /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. + #[deprecated( + since = "0.5.3", + note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." + )] + pub const fn new(modulus: &Uint) -> Self { + // A valid modulus must be odd + if modulus.ct_is_odd().to_u8() == 0 { + panic!("modulus must be odd"); + } + + Self::generate_params(modulus) + } + /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. pub fn new_checked(modulus: &Uint) -> CtOption { - let r = Uint::MAX.const_rem(modulus).0.wrapping_add(&Uint::ONE); - let r2 = Uint::const_rem_wide(r.square_wide(), modulus).0; - - // Since we are calculating the inverse modulo (Word::MAX+1), - // we can take the modulo right away and calculate the inverse of the first limb only. - let modulus_lo = Uint::<1>::from_words([modulus.limbs[0].0]); - let mod_neg_inv = - Limb(Word::MIN.wrapping_sub(modulus_lo.inv_mod2k(Word::BITS as usize).limbs[0].0)); - - let r3 = montgomery_reduction(&r2.square_wide(), modulus, mod_neg_inv); - // A valid modulus must be odd, which we check in constant time - CtOption::new( - Self { - modulus: *modulus, - r, - r2, - r3, - mod_neg_inv, - }, - modulus.ct_is_odd().into(), - ) + CtOption::new(Self::generate_params(modulus), modulus.ct_is_odd().into()) } /// Returns the modulus which was used to initialize these parameters. From 30fe070f7928c7850b4c4e8d53c5cc5a4ed66117 Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 25 May 2023 16:26:41 -0500 Subject: [PATCH 7/8] Test cleanup --- src/uint/modular/runtime_mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index 649d1b8b1..f50c5cb7e 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -237,10 +237,11 @@ mod test { } #[test] - #[should_panic] // Test that an invalid checked modulus does not yield `DynResidueParams` fn test_invalid_checked_modulus() { - DynResidueParams::::new_checked(&Uint::from(2u8)).unwrap(); + assert!(bool::from( + DynResidueParams::::new_checked(&Uint::from(2u8)).is_none() + )) } #[test] From 2348e143b76f0f6565044d70f27c6913394b095a Mon Sep 17 00:00:00 2001 From: Aaron Feickert <66188213+AaronFeickert@users.noreply.github.com> Date: Thu, 25 May 2023 18:24:14 -0500 Subject: [PATCH 8/8] Update deprecation strategy --- benches/bench.rs | 10 +++++----- src/uint/modular/runtime_mod.rs | 10 +++++----- src/uint/modular/runtime_mod/runtime_add.rs | 5 ++--- src/uint/modular/runtime_mod/runtime_sub.rs | 5 ++--- tests/proptests.rs | 6 +++--- 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/benches/bench.rs b/benches/bench.rs index bf8a7e1a5..8be5f5928 100644 --- a/benches/bench.rs +++ b/benches/bench.rs @@ -74,7 +74,7 @@ fn bench_division(group: &mut BenchmarkGroup<'_, M>) { } fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { - let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); + let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); group.bench_function("multiplication, U256*U256", |b| { b.iter_batched( || { @@ -88,7 +88,7 @@ fn bench_montgomery_ops(group: &mut BenchmarkGroup<'_, M>) { }); let m = U256::random(&mut OsRng) | U256::ONE; - let params = DynResidueParams::new_checked(&m).unwrap(); + let params = DynResidueParams::new(&m); group.bench_function("modpow, U256^U256", |b| { b.iter_batched( || { @@ -107,12 +107,12 @@ fn bench_montgomery_conversion(group: &mut BenchmarkGroup<'_, M> group.bench_function("DynResidueParams creation", |b| { b.iter_batched( || U256::random(&mut OsRng) | U256::ONE, - |modulus| DynResidueParams::new_checked(&modulus).unwrap(), + |modulus| DynResidueParams::new(&modulus), BatchSize::SmallInput, ) }); - let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); + let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); group.bench_function("DynResidue creation", |b| { b.iter_batched( || U256::random(&mut OsRng), @@ -121,7 +121,7 @@ fn bench_montgomery_conversion(group: &mut BenchmarkGroup<'_, M> ) }); - let params = DynResidueParams::new_checked(&(U256::random(&mut OsRng) | U256::ONE)).unwrap(); + let params = DynResidueParams::new(&(U256::random(&mut OsRng) | U256::ONE)); group.bench_function("DynResidue retrieve", |b| { b.iter_batched( || DynResidue::new(&U256::random(&mut OsRng), params), diff --git a/src/uint/modular/runtime_mod.rs b/src/uint/modular/runtime_mod.rs index f50c5cb7e..4468d71ac 100644 --- a/src/uint/modular/runtime_mod.rs +++ b/src/uint/modular/runtime_mod.rs @@ -63,10 +63,6 @@ impl DynResidueParams { /// Instantiates a new set of `ResidueParams` representing the given `modulus`, which _must_ be odd. /// If `modulus` is not odd, this function will panic; use [`new_checked`][`DynResidueParams::new_checked`] if you want to be able to detect an invalid modulus. - #[deprecated( - since = "0.5.3", - note = "This will return an `Option` in a future version to account for an invalid modulus, but for now will panic if this happens. Consider using `new_checked` until then." - )] pub const fn new(modulus: &Uint) -> Self { // A valid modulus must be odd if modulus.ct_is_odd().to_u8() == 0 { @@ -78,6 +74,10 @@ impl DynResidueParams { /// Instantiates a new set of `ResidueParams` representing the given `modulus` if it is odd. /// Returns a `CtOption` that is `None` if the provided modulus is not odd; this is a safer version of [`new`][`DynResidueParams::new`], which can panic. + #[deprecated( + since = "0.5.3", + note = "This functionality will be moved to `new` in a future release." + )] pub fn new_checked(modulus: &Uint) -> CtOption { // A valid modulus must be odd, which we check in constant time CtOption::new(Self::generate_params(modulus), modulus.ct_is_odd().into()) @@ -237,6 +237,7 @@ mod test { } #[test] + #[allow(deprecated)] // Test that an invalid checked modulus does not yield `DynResidueParams` fn test_invalid_checked_modulus() { assert!(bool::from( @@ -245,7 +246,6 @@ mod test { } #[test] - #[allow(deprecated)] #[should_panic] // Tets that an invalid modulus panics fn test_invalid_modulus() { diff --git a/src/uint/modular/runtime_mod/runtime_add.rs b/src/uint/modular/runtime_mod/runtime_add.rs index c497ee0d1..eb4708603 100644 --- a/src/uint/modular/runtime_mod/runtime_add.rs +++ b/src/uint/modular/runtime_mod/runtime_add.rs @@ -70,10 +70,9 @@ mod tests { #[test] fn add_overflow() { - let params = DynResidueParams::new_checked(&U256::from_be_hex( + let params = DynResidueParams::new(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", - )) - .unwrap(); + )); let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); diff --git a/src/uint/modular/runtime_mod/runtime_sub.rs b/src/uint/modular/runtime_mod/runtime_sub.rs index 46d0dcfe2..dd6fd84c8 100644 --- a/src/uint/modular/runtime_mod/runtime_sub.rs +++ b/src/uint/modular/runtime_mod/runtime_sub.rs @@ -70,10 +70,9 @@ mod tests { #[test] fn sub_overflow() { - let params = DynResidueParams::new_checked(&U256::from_be_hex( + let params = DynResidueParams::new(&U256::from_be_hex( "ffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551", - )) - .unwrap(); + )); let x = U256::from_be_hex("44acf6b7e36c1342c2c5897204fe09504e1e2efb1a900377dbc4e7a6a133ec56"); diff --git a/tests/proptests.rs b/tests/proptests.rs index 26996db95..572d990d1 100644 --- a/tests/proptests.rs +++ b/tests/proptests.rs @@ -258,7 +258,7 @@ proptest! { let expected = to_uint(a_bi.modpow(&b_bi, &p_bi)); - let params = DynResidueParams::new_checked(&P).unwrap(); + let params = DynResidueParams::new(&P); let a_m = DynResidue::new(&a, params); let actual = a_m.pow(&b).retrieve(); @@ -276,7 +276,7 @@ proptest! { let expected = to_uint(a_bi.modpow(&b_bi, &p_bi)); - let params = DynResidueParams::new_checked(&P).unwrap(); + let params = DynResidueParams::new(&P); let a_m = DynResidue::new(&a, params); let actual = a_m.pow_bounded_exp(&b, exponent_bits.into()).retrieve(); @@ -297,7 +297,7 @@ proptest! { }; let expected = to_uint(expected); - let params = DynResidueParams::new_checked(&P).unwrap(); + let params = DynResidueParams::new(&P); let a_m = DynResidue::new(&a, params); let actual = a_m.div_by_2().retrieve();