propose a fix to uni_random

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Add `from_var_bytes` to scalar
 
+### Changed
+
+- Change `Scalar::uni_random` to sample u64 without manipulating bits
+
 ## [0.12.3] - 2023-11-01
 
 ### Added

Cargo.toml

@@ -32,6 +32,7 @@ sha2 = "0.9"
 sha3 = "0.9"
 # Dev-dependencies added by Dusk
 bincode = "1"
+rand = "0.8"
 rkyv = {version = "0.7", default-features = false, features = ["size_32"]}
 quickcheck = "1"
 

src/scalar/dusk.rs

@@ -281,20 +281,23 @@ impl Scalar {
     where
         R: RngCore + CryptoRng,
     {
-        let mut buf = [0; 32];
-        let mut scalar: Option<Self> = None;
-
-        // We loop as long as it takes to generate a valid scalar.
-        // As long as the random number generator is implemented properly, this
-        // loop will terminate.
-        while scalar == None {
-            rng.fill_bytes(&mut buf);
-            // Since modulus has at most 255 bits, we can zero the MSB and like
-            // this improve our chances of hitting a valid scalar to above 50%
-            buf[32 - 1] &= 0b0111_1111;
-            scalar = Self::from_bytes(&buf).into();
+        loop {
+            let s0 = rng.next_u64();
+            let s1 = rng.next_u64();
+            let s2 = rng.next_u64();
+            let s3 = rng.next_u64();
+
+            let bx = s3 <= MODULUS.0[3];
+            let b1 = bx && MODULUS.0[0] > s0;
+            let b2 = bx && (MODULUS.0[1] + b1 as u64) > s1;
+            let b3 = bx && (MODULUS.0[2] + b2 as u64) > s2;
+            let b4 = bx && (MODULUS.0[3] + b3 as u64) > s3;
+
+            // there is a borrow; hence, in range
+            if b4 {
+                return Self::from_raw([s0, s1, s2, s3]);
+            }
         }
-        scalar.unwrap()
     }
 
     /// Creates a `Scalar` from arbitrary bytes by hashing the input with BLAKE2b into a 256-bits
@@ -481,18 +484,37 @@ fn test_scalar_eq_and_hash() {
 mod fuzz {
     use alloc::vec::Vec;
 
+    use rand::rngs::StdRng;
+    use rand::SeedableRng;
+
     use crate::scalar::{Scalar, MODULUS};
     use crate::util::sbb;
 
+    fn is_scalar_in_range(scalar: &Scalar) -> bool {
+        // subtraction against modulus must underflow
+        let borrow = scalar
+            .0
+            .iter()
+            .zip(MODULUS.0.iter())
+            .fold(0, |borrow, (&s, &m)| sbb(s, m, borrow).1);
+
+        borrow == u64::MAX
+    }
+
     quickcheck::quickcheck! {
         fn prop_scalar_from_raw_bytes(bytes: Vec<u8>) -> bool {
-            let Scalar(s) = Scalar::from_var_bytes(&bytes);
-            let Scalar(m) = MODULUS;
+            let scalar = Scalar::from_var_bytes(&bytes);
 
-            // subtraction against modulus must underflow
-            let borrow = s.iter().zip(m.iter()).fold(0, |borrow, (&s, &m)| sbb(s, m, borrow).1);
+            is_scalar_in_range(&scalar)
+        }
+    }
+
+    quickcheck::quickcheck! {
+        fn prop_scalar_uni_random(seed: u64) -> bool {
+            let rng = &mut StdRng::seed_from_u64(seed);
+            let scalar = Scalar::uni_random(rng);
 
-            (borrow >> 63) == 1
+            is_scalar_in_range(&scalar)
         }
     }
 }