Make random_odd_integer fallible and add tests.

CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Renamed `Sieve` to `SmallPrimesSieve`. ([#64])
 - Bumped `crypto-bigint` to 0.6.0-rc.7 and MSRV to 1.83. ([#67])
 - Bumped `crypto-bigint` to 0.6. ([#68])
+- `random_odd_integer()` takes an additional `SetBits` argument. ([#69])
+- `random_odd_integer()` now returns a `Result` instead of panicking. ([#69])
 
 
 ### Added
@@ -23,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#64]: https://github.com/entropyxyz/crypto-primes/pull/64
 [#67]: https://github.com/entropyxyz/crypto-primes/pull/67
 [#68]: https://github.com/entropyxyz/crypto-primes/pull/68
+[#69]: https://github.com/entropyxyz/crypto-primes/pull/69
 
 
 ## [0.6.0-pre.2] - 2024-10-18

benches/bench.rs

@@ -37,7 +37,7 @@ fn make_random_rng() -> ChaCha8Rng {
 }
 
 fn random_odd_uint<T: RandomBits + Integer>(rng: &mut impl CryptoRngCore, bit_length: u32) -> Odd<T> {
-    random_odd_integer::<T>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb)
+    random_odd_integer::<T>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb).unwrap()
 }
 
 fn make_sieve<const L: usize>(rng: &mut impl CryptoRngCore) -> SmallPrimesSieve<Uint<L>> {
@@ -449,7 +449,9 @@ fn bench_glass_pumpkin(c: &mut Criterion) {
     // Mimics the sequence of checks `glass-pumpkin` does to find a prime.
     fn prime_like_gp(bit_length: u32, rng: &mut impl CryptoRngCore) -> BoxedUint {
         loop {
-            let start = random_odd_integer::<BoxedUint>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb).get();
+            let start = random_odd_integer::<BoxedUint>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb)
+                .unwrap()
+                .get();
             let sieve = SmallPrimesSieve::new(start, NonZero::new(bit_length).unwrap(), false);
             for num in sieve {
                 let odd_num = Odd::new(num.clone()).unwrap();
@@ -473,7 +475,9 @@ fn bench_glass_pumpkin(c: &mut Criterion) {
     // Mimics the sequence of checks `glass-pumpkin` does to find a safe prime.
     fn safe_prime_like_gp(bit_length: u32, rng: &mut impl CryptoRngCore) -> BoxedUint {
         loop {
-            let start = random_odd_integer::<BoxedUint>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb).get();
+            let start = random_odd_integer::<BoxedUint>(rng, NonZero::new(bit_length).unwrap(), SetBits::Msb)
+                .unwrap()
+                .get();
             let sieve = SmallPrimesSieve::new(start, NonZero::new(bit_length).unwrap(), true);
             for num in sieve {
                 let odd_num = Odd::new(num.clone()).unwrap();

src/hazmat/miller_rabin.rs

@@ -196,7 +196,7 @@ mod tests {
     #[test]
     fn trivial() {
         let mut rng = ChaCha8Rng::from_seed(*b"01234567890123456789012345678901");
-        let start = random_odd_integer::<U1024>(&mut rng, NonZero::new(1024).unwrap(), SetBits::Msb);
+        let start = random_odd_integer::<U1024>(&mut rng, NonZero::new(1024).unwrap(), SetBits::Msb).unwrap();
         for num in SmallPrimesSieve::new(start.get(), NonZero::new(1024).unwrap(), false).take(10) {
             let mr = MillerRabin::new(Odd::new(num).unwrap());
 

src/hazmat/sieve.rs

@@ -5,54 +5,64 @@ use alloc::{vec, vec::Vec};
 use core::marker::PhantomData;
 use core::num::{NonZero, NonZeroU32};
 
-use crypto_bigint::{Integer, Odd, RandomBits};
+use crypto_bigint::{RandomBitsError, Integer, Odd, RandomBits};
 use rand_core::CryptoRngCore;
 
 use crate::hazmat::precomputed::{SmallPrime, LAST_SMALL_PRIME, RECIPROCALS, SMALL_PRIMES};
 use crate::traits::SieveFactory;
 
-/// Set bits in the returned number.
+/// Decide how prime candidates are manipulated by setting certain bits before primality testing,
+/// influencing the range of the prime.
 #[derive(Debug, Clone, Copy)]
 pub enum SetBits {
-    /// No additional bits set.
-    None,
-    /// Set the most significant bit.
+    /// Set the most significant bit, thus limiting the range to `[MAX/2 + 1, MAX]`.
+    ///
+    /// In other words, all candidates will have the same bit size.
     Msb,
-    /// Set two most significant bits.
+    /// Set two most significant bits, limiting the range to `[MAX - MAX/4 + 1, MAX]`.
     ///
     /// This is useful in the RSA case because a product of two such numbers will have a guaranteed bit size.
     TwoMsb,
+    /// No additional bits set; uses the full range `[1, MAX]`.
+    None,
 }
 
-/// Returns a random odd integer up to the given bit length
-/// (that is, with both `0` and `bit_length-1` bits set).
+/// Returns a random odd integer up to the given bit length.
 ///
-/// Additionally, some bits can be set depending on the `set_bits` value.
+/// The `set_bits` parameter decides which extra bits are set, which decides the range of the number.
 ///
-/// *Panics*: if the `bit_length` is bigger than the bits available in the `Integer`, e.g. 37 for a
-/// `U32`.
+/// Returns an error variant if `bit_length` is greater than the maximum allowed for `T`
+/// (applies to fixed-length types).
 pub fn random_odd_integer<T: Integer + RandomBits>(
     rng: &mut impl CryptoRngCore,
     bit_length: NonZeroU32,
     set_bits: SetBits,
-) -> Odd<T> {
+) -> Result<Odd<T>, RandomBitsError> {
     let bit_length = bit_length.get();
 
-    let mut random = T::random_bits(rng, bit_length);
+    let mut random = T::try_random_bits(rng, bit_length)?;
+
     // Make it odd
+    // `bit_length` is non-zero, so the 0-th bit exists.
     random.set_bit_vartime(0, true);
 
-    // Will not overflow since `bit_length` is ensured to be within the size of the integer.
+    // Will not overflow since `bit_length` is ensured to be within the size of the integer
+    // (checked within the `T::try_random_bits()` call).
+    // `bit_length - 1`-th bit exists since `bit_length` is non-zero.
     match set_bits {
         SetBits::None => {}
         SetBits::Msb => random.set_bit_vartime(bit_length - 1, true),
         SetBits::TwoMsb => {
             random.set_bit_vartime(bit_length - 1, true);
-            random.set_bit_vartime(bit_length - 2, true);
+            // We could panic here, but since the primary purpose of `TwoMsb` is to ensure the bit length
+            // of the product of two numbers, ignoring this for `bit_length = 1` leads to the desired result.
+            if bit_length > 1 {
+                random.set_bit_vartime(bit_length - 2, true);
+            }
         }
     }
 
-    Odd::new(random).expect("the number is odd by construction")
+    Ok(Odd::new(random).expect("the number is odd by construction"))
 }
 
 // The type we use to calculate incremental residues.
@@ -318,7 +328,7 @@ impl<T: Integer + RandomBits> SieveFactory for SmallPrimesSieveFactory<T> {
         rng: &mut impl CryptoRngCore,
         _previous_sieve: Option<&Self::Sieve>,
     ) -> Option<Self::Sieve> {
-        let start = random_odd_integer::<T>(rng, self.max_bit_length, self.set_bits);
+        let start = random_odd_integer::<T>(rng, self.max_bit_length, self.set_bits).expect("random_odd_integer() failed");
         Some(SmallPrimesSieve::new(
             start.get(),
             self.max_bit_length,
@@ -347,7 +357,9 @@ mod tests {
         let max_prime = SMALL_PRIMES[SMALL_PRIMES.len() - 1];
 
         let mut rng = ChaCha8Rng::from_seed(*b"01234567890123456789012345678901");
-        let start = random_odd_integer::<U64>(&mut rng, NonZero::new(32).unwrap(), SetBits::Msb).get();
+        let start = random_odd_integer::<U64>(&mut rng, NonZero::new(32).unwrap(), SetBits::Msb)
+            .unwrap()
+            .get();
         for num in SmallPrimesSieve::new(start, NonZero::new(32).unwrap(), false).take(100) {
             let num_u64 = u64::from(num);
             assert!(num_u64.leading_zeros() == 32);
@@ -363,8 +375,9 @@ mod tests {
         let max_prime = SMALL_PRIMES[SMALL_PRIMES.len() - 1];
 
         let mut rng = ChaCha8Rng::from_seed(*b"01234567890123456789012345678901");
-        let start =
-            random_odd_integer::<crypto_bigint::BoxedUint>(&mut rng, NonZero::new(32).unwrap(), SetBits::Msb).get();
+        let start = random_odd_integer::<crypto_bigint::BoxedUint>(&mut rng, NonZero::new(32).unwrap(), SetBits::Msb)
+            .unwrap()
+            .get();
 
         for num in SmallPrimesSieve::new(start, NonZero::new(32).unwrap(), false).take(100) {
             // For 32-bit targets
@@ -442,15 +455,16 @@ mod tests {
     #[test]
     fn random_below_max_length() {
         for _ in 0..10 {
-            let r = random_odd_integer::<U64>(&mut OsRng, NonZero::new(50).unwrap(), SetBits::Msb).get();
+            let r = random_odd_integer::<U64>(&mut OsRng, NonZero::new(50).unwrap(), SetBits::Msb)
+                .unwrap()
+                .get();
             assert_eq!(r.bits(), 50);
         }
     }
 
     #[test]
-    #[should_panic(expected = "try_random_bits() failed: BitLengthTooLarge { bit_length: 65, bits_precision: 64 }")]
     fn random_odd_uint_too_many_bits() {
-        let _p = random_odd_integer::<U64>(&mut OsRng, NonZero::new(65).unwrap(), SetBits::Msb);
+        assert!(random_odd_integer::<U64>(&mut OsRng, NonZero::new(65).unwrap(), SetBits::Msb).is_err());
     }
 
     #[test]
@@ -486,4 +500,31 @@ mod tests {
     fn too_few_bits_safe_primes() {
         let _fac = SmallPrimesSieveFactory::<U64>::new_safe_primes(2, SetBits::Msb);
     }
+
+    #[test]
+    fn set_bits() {
+        for _ in 0..10 {
+            let x = random_odd_integer::<U64>(&mut OsRng, NonZero::new(64).unwrap(), SetBits::Msb).unwrap();
+            assert!(bool::from(x.bit(63)));
+        }
+
+        for _ in 0..10 {
+            let x = random_odd_integer::<U64>(&mut OsRng, NonZero::new(64).unwrap(), SetBits::TwoMsb).unwrap();
+            assert!(bool::from(x.bit(63)));
+            assert!(bool::from(x.bit(62)));
+        }
+
+        // 1 in 2^30 chance of spurious failure... good enough?
+        assert!((0..30).map(|_| {
+            random_odd_integer::<U64>(&mut OsRng, NonZero::new(64).unwrap(), SetBits::None).unwrap()
+        }).any(|x| !bool::from(x.bit(63))));
+    }
+
+    #[test]
+    fn set_two_msb_small_bit_length() {
+        // Check that when technically there isn't a second most significant bit,
+        // `random_odd_integer()` still returns a number.
+        let x = random_odd_integer::<U64>(&mut OsRng, NonZero::new(1).unwrap(), SetBits::TwoMsb).unwrap().get();
+        assert_eq!(x, U64::ONE);
+    }
 }

src/presets.rs

@@ -389,13 +389,13 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "try_random_bits() failed: BitLengthTooLarge { bit_length: 65, bits_precision: 64 }")]
+    #[should_panic(expected = "random_odd_integer() failed: BitLengthTooLarge { bit_length: 65, bits_precision: 64 }")]
     fn generate_prime_too_many_bits() {
         let _p: U64 = generate_prime_with_rng(&mut OsRng, 65);
     }
 
     #[test]
-    #[should_panic(expected = "try_random_bits() failed: BitLengthTooLarge { bit_length: 65, bits_precision: 64 }")]
+    #[should_panic(expected = "random_odd_integer() failed: BitLengthTooLarge { bit_length: 65, bits_precision: 64 }")]
     fn generate_safe_prime_too_many_bits() {
         let _p: U64 = generate_safe_prime_with_rng(&mut OsRng, 65);
     }
@@ -517,7 +517,7 @@ mod tests_openssl {
 
         // Generate random numbers, check if our test agrees with OpenSSL
         for _ in 0..100 {
-            let p = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb);
+            let p = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb).unwrap();
             let actual = is_prime(p.as_ref());
             let p_bn = to_openssl(&p);
             let expected = openssl_is_prime(&p_bn, &mut ctx);
@@ -567,7 +567,7 @@ mod tests_gmp {
 
         // Generate primes with GMP, check them
         for _ in 0..100 {
-            let start = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb);
+            let start = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb).unwrap();
             let start_bn = to_gmp(&start);
             let p_bn = start_bn.next_prime();
             let p = from_gmp(&p_bn);
@@ -576,7 +576,7 @@ mod tests_gmp {
 
         // Generate random numbers, check if our test agrees with GMP
         for _ in 0..100 {
-            let p = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb);
+            let p = random_odd_integer::<U128>(&mut OsRng, NonZero::new(128).unwrap(), SetBits::Msb).unwrap();
             let actual = is_prime(p.as_ref());
             let p_bn = to_gmp(&p);
             let expected = gmp_is_prime(&p_bn);