From 69e27c78b576212808bc253132131bd0654fe34b Mon Sep 17 00:00:00 2001 From: winrhcp Date: Thu, 10 Oct 2024 21:58:56 +0700 Subject: [PATCH] refactor:Tidy up scalar module (#251) # Rationale for this change MontScalar and Scalar logic is currently disorganized. We should consolidate this logic. # What changes are included in this PR? Combined logic held within the four files "mont_scalar", "mont_scalar_from", "mont_scalar_test", "mont_scalar_from_test" into the files "mont_scalar" and "mont_scalar_test" --- crates/proof-of-sql/src/base/scalar/mod.rs | 3 - .../src/base/scalar/mont_scalar.rs | 64 +++++- .../src/base/scalar/mont_scalar_from.rs | 61 ------ .../src/base/scalar/mont_scalar_from_test.rs | 183 ----------------- .../src/base/scalar/mont_scalar_test.rs | 186 +++++++++++++++++- 5 files changed, 245 insertions(+), 252 deletions(-) delete mode 100644 crates/proof-of-sql/src/base/scalar/mont_scalar_from.rs delete mode 100644 crates/proof-of-sql/src/base/scalar/mont_scalar_from_test.rs diff --git a/crates/proof-of-sql/src/base/scalar/mod.rs b/crates/proof-of-sql/src/base/scalar/mod.rs index 6ba7e8208..1759554f3 100644 --- a/crates/proof-of-sql/src/base/scalar/mod.rs +++ b/crates/proof-of-sql/src/base/scalar/mod.rs @@ -8,9 +8,6 @@ use alloc::string::String; use core::{cmp::Ordering, ops::Sub}; pub use mont_scalar::Curve25519Scalar; pub(crate) use mont_scalar::MontScalar; -mod mont_scalar_from; -#[cfg(test)] -mod mont_scalar_from_test; /// Module for a test Scalar #[cfg(test)] pub mod test_scalar; diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar.rs index 4705b3607..0a4ae8f6d 100644 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar.rs +++ b/crates/proof-of-sql/src/base/scalar/mont_scalar.rs @@ -1,6 +1,6 @@ use super::{Scalar, ScalarConversionError}; use crate::base::math::decimal::MAX_SUPPORTED_PRECISION; -use alloc::{format, vec::Vec}; +use alloc::{format, string::String, vec::Vec}; use ark_ff::{BigInteger, Field, Fp, Fp256, MontBackend, MontConfig, PrimeField}; use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use bytemuck::TransparentWrapper; @@ -13,7 +13,7 @@ use core::{ ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}, }; use num_bigint::BigInt; -use num_traits::Signed; +use num_traits::{Signed, Zero}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[derive(CanonicalSerialize, CanonicalDeserialize, TransparentWrapper)] @@ -117,6 +117,66 @@ impl> Ord for MontScalar { // end replacement for #[derive(...)] // -------------------------------------------------------------------------------- +/// TODO: add docs +macro_rules! impl_from_for_mont_scalar_for_type_supported_by_from { + ($tt:ty) => { + impl> From<$tt> for MontScalar { + fn from(x: $tt) -> Self { + Self(x.into()) + } + } + }; +} + +/// Implement `From<&[u8]>` for `MontScalar` +impl> From<&[u8]> for MontScalar { + fn from(x: &[u8]) -> Self { + if x.is_empty() { + return Self::zero(); + } + + let hash = blake3::hash(x); + let mut bytes: [u8; 32] = hash.into(); + bytes[31] &= 0b0000_1111_u8; + + Self::from_le_bytes_mod_order(&bytes) + } +} + +/// TODO: add docs +macro_rules! impl_from_for_mont_scalar_for_string { + ($tt:ty) => { + impl> From<$tt> for MontScalar { + fn from(x: $tt) -> Self { + x.as_bytes().into() + } + } + }; +} + +impl_from_for_mont_scalar_for_type_supported_by_from!(bool); +impl_from_for_mont_scalar_for_type_supported_by_from!(u8); +impl_from_for_mont_scalar_for_type_supported_by_from!(u16); +impl_from_for_mont_scalar_for_type_supported_by_from!(u32); +impl_from_for_mont_scalar_for_type_supported_by_from!(u64); +impl_from_for_mont_scalar_for_type_supported_by_from!(u128); +impl_from_for_mont_scalar_for_type_supported_by_from!(i8); +impl_from_for_mont_scalar_for_type_supported_by_from!(i16); +impl_from_for_mont_scalar_for_type_supported_by_from!(i32); +impl_from_for_mont_scalar_for_type_supported_by_from!(i64); +impl_from_for_mont_scalar_for_type_supported_by_from!(i128); +impl_from_for_mont_scalar_for_string!(&str); +impl_from_for_mont_scalar_for_string!(String); + +impl, T> From<&T> for MontScalar +where + T: Into> + Clone, +{ + fn from(x: &T) -> Self { + x.clone().into() + } +} + /// A wrapper type around the field element `ark_curve25519::Fr` and should be used in place of `ark_curve25519::Fr`. /// /// Using the `Scalar` trait rather than this type is encouraged to allow for easier switching of the underlying field. diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar_from.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar_from.rs deleted file mode 100644 index 7951cb871..000000000 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar_from.rs +++ /dev/null @@ -1,61 +0,0 @@ -use crate::base::scalar::MontScalar; -use alloc::string::String; -use ark_ff::MontConfig; -use num_traits::Zero; - -/// TODO: add docs -macro_rules! impl_from_for_mont_scalar_for_type_supported_by_from { - ($tt:ty) => { - impl> From<$tt> for MontScalar { - fn from(x: $tt) -> Self { - Self(x.into()) - } - } - }; -} -impl> From<&[u8]> for MontScalar { - fn from(x: &[u8]) -> Self { - if x.is_empty() { - return Self::zero(); - } - - let hash = blake3::hash(x); - let mut bytes: [u8; 32] = hash.into(); - bytes[31] &= 0b0000_1111_u8; - - Self::from_le_bytes_mod_order(&bytes) - } -} -/// TODO: add docs -macro_rules! impl_from_for_mont_scalar_for_string { - ($tt:ty) => { - impl> From<$tt> for MontScalar { - fn from(x: $tt) -> Self { - x.as_bytes().into() - } - } - }; -} - -impl_from_for_mont_scalar_for_type_supported_by_from!(bool); -impl_from_for_mont_scalar_for_type_supported_by_from!(u8); -impl_from_for_mont_scalar_for_type_supported_by_from!(u16); -impl_from_for_mont_scalar_for_type_supported_by_from!(u32); -impl_from_for_mont_scalar_for_type_supported_by_from!(u64); -impl_from_for_mont_scalar_for_type_supported_by_from!(u128); -impl_from_for_mont_scalar_for_type_supported_by_from!(i8); -impl_from_for_mont_scalar_for_type_supported_by_from!(i16); -impl_from_for_mont_scalar_for_type_supported_by_from!(i32); -impl_from_for_mont_scalar_for_type_supported_by_from!(i64); -impl_from_for_mont_scalar_for_type_supported_by_from!(i128); -impl_from_for_mont_scalar_for_string!(&str); -impl_from_for_mont_scalar_for_string!(String); - -impl, T> From<&T> for MontScalar -where - T: Into> + Clone, -{ - fn from(x: &T) -> Self { - x.clone().into() - } -} diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar_from_test.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar_from_test.rs deleted file mode 100644 index dec43c9f9..000000000 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar_from_test.rs +++ /dev/null @@ -1,183 +0,0 @@ -use crate::base::{ - map::IndexSet, - scalar::{Curve25519Scalar, Scalar}, -}; -use alloc::{format, string::ToString, vec::Vec}; -use byte_slice_cast::AsByteSlice; -use core::cmp::Ordering; -use num_traits::{One, Zero}; -use rand::{ - distributions::{Distribution, Uniform}, - rngs::StdRng, - Rng, -}; -use rand_core::SeedableRng; - -#[test] -fn the_zero_integer_maps_to_the_zero_scalar() { - assert_eq!(Curve25519Scalar::from(0_u32), Curve25519Scalar::zero()); - assert_eq!(Curve25519Scalar::from(0_u64), Curve25519Scalar::zero()); - assert_eq!(Curve25519Scalar::from(0_u128), Curve25519Scalar::zero()); - assert_eq!(Curve25519Scalar::from(0_i32), Curve25519Scalar::zero()); - assert_eq!(Curve25519Scalar::from(0_i64), Curve25519Scalar::zero()); - assert_eq!(Curve25519Scalar::from(0_i128), Curve25519Scalar::zero()); -} - -#[test] -fn bools_map_to_curve25519_scalar_properly() { - assert_eq!(Curve25519Scalar::from(true), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(false), Curve25519Scalar::zero()); -} - -#[test] -fn the_one_integer_maps_to_the_zero_scalar() { - assert_eq!(Curve25519Scalar::from(1_u32), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(1_u64), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(1_u128), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(1_i32), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(1_i64), Curve25519Scalar::one()); - assert_eq!(Curve25519Scalar::from(1_i128), Curve25519Scalar::one()); -} - -#[test] -fn the_zero_scalar_is_the_additive_identity() { - let mut rng = StdRng::seed_from_u64(0u64); - for _ in 0..1000 { - let a = Curve25519Scalar::from(rng.gen::()); - let b = Curve25519Scalar::from(rng.gen::()); - assert_eq!(a + b, b + a); - assert_eq!(a + Curve25519Scalar::zero(), a); - assert_eq!(b + Curve25519Scalar::zero(), b); - assert_eq!( - Curve25519Scalar::zero() + Curve25519Scalar::zero(), - Curve25519Scalar::zero() - ); - } -} - -#[test] -fn the_one_scalar_is_the_multiplicative_identity() { - let mut rng = StdRng::seed_from_u64(0u64); - for _ in 0..1000 { - let a = Curve25519Scalar::from(rng.gen::()); - let b = Curve25519Scalar::from(rng.gen::()); - assert_eq!(a * b, b * a); - assert_eq!(a * Curve25519Scalar::one(), a); - assert_eq!(b * Curve25519Scalar::one(), b); - assert_eq!( - Curve25519Scalar::one() * Curve25519Scalar::one(), - Curve25519Scalar::one() - ); - } -} - -#[test] -fn scalar_comparison_works() { - let zero = Curve25519Scalar::ZERO; - let one = Curve25519Scalar::ONE; - let two = Curve25519Scalar::TWO; - let max = Curve25519Scalar::MAX_SIGNED; - let min = max + one; - assert_eq!(max.signed_cmp(&one), Ordering::Greater); - assert_eq!(one.signed_cmp(&zero), Ordering::Greater); - assert_eq!(min.signed_cmp(&zero), Ordering::Less); - assert_eq!((two * max).signed_cmp(&zero), Ordering::Less); - assert_eq!(two * max + one, zero); -} - -#[test] -fn the_empty_string_will_be_mapped_to_the_zero_scalar() { - assert_eq!(Curve25519Scalar::from(""), Curve25519Scalar::zero()); - assert_eq!( - Curve25519Scalar::from(<&str>::default()), - Curve25519Scalar::zero() - ); -} - -#[test] -fn two_different_strings_map_to_different_scalars() { - let s = "abc12"; - assert_ne!(Curve25519Scalar::from(s), Curve25519Scalar::zero()); - assert_ne!(Curve25519Scalar::from(s), Curve25519Scalar::from("abc123")); -} - -#[test] -fn the_empty_buffer_will_be_mapped_to_the_zero_scalar() { - let buf = Vec::::default(); - assert_eq!(Curve25519Scalar::from(&buf[..]), Curve25519Scalar::zero()); -} - -#[test] -fn byte_arrays_with_the_same_content_but_different_types_map_to_different_scalars() { - let array = [1_u8, 2_u8, 34_u8]; - assert_ne!( - Curve25519Scalar::from(array.as_byte_slice()), - Curve25519Scalar::zero() - ); - assert_ne!( - Curve25519Scalar::from(array.as_byte_slice()), - Curve25519Scalar::from([1_u32, 2_u32, 34_u32].as_byte_slice()) - ); -} - -#[test] -fn strings_of_arbitrary_size_map_to_different_scalars() { - let mut prev_scalars = IndexSet::default(); - let mut rng = StdRng::from_seed([0u8; 32]); - let dist = Uniform::new(1, 100); - - for i in 0..100 { - let s = format!( - "{}_{}_{}", - dist.sample(&mut rng), - i, - "testing string to scalar".repeat(dist.sample(&mut rng)) - ); - assert!(prev_scalars.insert(Curve25519Scalar::from(s.as_str()))); - } -} - -#[test] -fn byte_arrays_of_arbitrary_size_map_to_different_scalars() { - let mut prev_scalars = IndexSet::default(); - let mut rng = StdRng::from_seed([0u8; 32]); - let dist = Uniform::new(1, 100); - - for _ in 0..100 { - let v = (0..dist.sample(&mut rng)) - .map(|_v| (dist.sample(&mut rng) % 255) as u8) - .collect::>(); - assert!(prev_scalars.insert(Curve25519Scalar::from(&v[..]))); - } -} - -#[test] -fn the_string_hash_implementation_uses_the_full_range_of_bits() { - let max_iters = 20; - let mut rng = StdRng::from_seed([0u8; 32]); - let dist = Uniform::new(1, i32::MAX); - - for i in 0..252 { - let mut curr_iters = 0; - let mut bset = IndexSet::default(); - - loop { - let s: Curve25519Scalar = dist.sample(&mut rng).to_string().as_str().into(); - let bytes = s.to_bytes_le(); //Note: this is the only spot that these tests are different from the to_curve25519_scalar tests. - - let is_ith_bit_set = bytes[i / 8] & (1 << (i % 8)) != 0; - - bset.insert(is_ith_bit_set); - - if bset == IndexSet::from_iter([false, true]) { - break; - } - - // this guarantees that, if the above test fails, - // we'll be able to identify it's failing - assert!(curr_iters <= max_iters); - - curr_iters += 1; - } - } -} diff --git a/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs b/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs index 66fd35d61..3a26fd8c0 100644 --- a/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs +++ b/crates/proof-of-sql/src/base/scalar/mont_scalar_test.rs @@ -1,7 +1,18 @@ -use crate::base::scalar::{Curve25519Scalar, Scalar, ScalarConversionError}; -use alloc::format; +use crate::base::{ + map::IndexSet, + scalar::{Curve25519Scalar, Scalar, ScalarConversionError}, +}; +use alloc::{format, string::ToString, vec::Vec}; +use byte_slice_cast::AsByteSlice; +use core::cmp::Ordering; use num_bigint::BigInt; -use num_traits::{Inv, One}; +use num_traits::{Inv, One, Zero}; +use rand::{ + distributions::{Distribution, Uniform}, + rngs::StdRng, + Rng, +}; +use rand_core::SeedableRng; #[test] fn test_dalek_interop_1() { @@ -291,3 +302,172 @@ fn test_curve25519_scalar_from_bigint() { -Curve25519Scalar::ONE ); } + +#[test] +fn the_zero_integer_maps_to_the_zero_scalar() { + assert_eq!(Curve25519Scalar::from(0_u32), Curve25519Scalar::zero()); + assert_eq!(Curve25519Scalar::from(0_u64), Curve25519Scalar::zero()); + assert_eq!(Curve25519Scalar::from(0_u128), Curve25519Scalar::zero()); + assert_eq!(Curve25519Scalar::from(0_i32), Curve25519Scalar::zero()); + assert_eq!(Curve25519Scalar::from(0_i64), Curve25519Scalar::zero()); + assert_eq!(Curve25519Scalar::from(0_i128), Curve25519Scalar::zero()); +} + +#[test] +fn bools_map_to_curve25519_scalar_properly() { + assert_eq!(Curve25519Scalar::from(true), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(false), Curve25519Scalar::zero()); +} + +#[test] +fn the_one_integer_maps_to_the_zero_scalar() { + assert_eq!(Curve25519Scalar::from(1_u32), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(1_u64), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(1_u128), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(1_i32), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(1_i64), Curve25519Scalar::one()); + assert_eq!(Curve25519Scalar::from(1_i128), Curve25519Scalar::one()); +} + +#[test] +fn the_zero_scalar_is_the_additive_identity() { + let mut rng = StdRng::seed_from_u64(0u64); + for _ in 0..1000 { + let a = Curve25519Scalar::from(rng.gen::()); + let b = Curve25519Scalar::from(rng.gen::()); + assert_eq!(a + b, b + a); + assert_eq!(a + Curve25519Scalar::zero(), a); + assert_eq!(b + Curve25519Scalar::zero(), b); + assert_eq!( + Curve25519Scalar::zero() + Curve25519Scalar::zero(), + Curve25519Scalar::zero() + ); + } +} + +#[test] +fn the_one_scalar_is_the_multiplicative_identity() { + let mut rng = StdRng::seed_from_u64(0u64); + for _ in 0..1000 { + let a = Curve25519Scalar::from(rng.gen::()); + let b = Curve25519Scalar::from(rng.gen::()); + assert_eq!(a * b, b * a); + assert_eq!(a * Curve25519Scalar::one(), a); + assert_eq!(b * Curve25519Scalar::one(), b); + assert_eq!( + Curve25519Scalar::one() * Curve25519Scalar::one(), + Curve25519Scalar::one() + ); + } +} + +#[test] +fn scalar_comparison_works() { + let zero = Curve25519Scalar::ZERO; + let one = Curve25519Scalar::ONE; + let two = Curve25519Scalar::TWO; + let max = Curve25519Scalar::MAX_SIGNED; + let min = max + one; + assert_eq!(max.signed_cmp(&one), Ordering::Greater); + assert_eq!(one.signed_cmp(&zero), Ordering::Greater); + assert_eq!(min.signed_cmp(&zero), Ordering::Less); + assert_eq!((two * max).signed_cmp(&zero), Ordering::Less); + assert_eq!(two * max + one, zero); +} + +#[test] +fn the_empty_string_will_be_mapped_to_the_zero_scalar() { + assert_eq!(Curve25519Scalar::from(""), Curve25519Scalar::zero()); + assert_eq!( + Curve25519Scalar::from(<&str>::default()), + Curve25519Scalar::zero() + ); +} + +#[test] +fn two_different_strings_map_to_different_scalars() { + let s = "abc12"; + assert_ne!(Curve25519Scalar::from(s), Curve25519Scalar::zero()); + assert_ne!(Curve25519Scalar::from(s), Curve25519Scalar::from("abc123")); +} + +#[test] +fn the_empty_buffer_will_be_mapped_to_the_zero_scalar() { + let buf = Vec::::default(); + assert_eq!(Curve25519Scalar::from(&buf[..]), Curve25519Scalar::zero()); +} + +#[test] +fn byte_arrays_with_the_same_content_but_different_types_map_to_different_scalars() { + let array = [1_u8, 2_u8, 34_u8]; + assert_ne!( + Curve25519Scalar::from(array.as_byte_slice()), + Curve25519Scalar::zero() + ); + assert_ne!( + Curve25519Scalar::from(array.as_byte_slice()), + Curve25519Scalar::from([1_u32, 2_u32, 34_u32].as_byte_slice()) + ); +} + +#[test] +fn strings_of_arbitrary_size_map_to_different_scalars() { + let mut prev_scalars = IndexSet::default(); + let mut rng = StdRng::from_seed([0u8; 32]); + let dist = Uniform::new(1, 100); + + for i in 0..100 { + let s = format!( + "{}_{}_{}", + dist.sample(&mut rng), + i, + "testing string to scalar".repeat(dist.sample(&mut rng)) + ); + assert!(prev_scalars.insert(Curve25519Scalar::from(s.as_str()))); + } +} + +#[test] +fn byte_arrays_of_arbitrary_size_map_to_different_scalars() { + let mut prev_scalars = IndexSet::default(); + let mut rng = StdRng::from_seed([0u8; 32]); + let dist = Uniform::new(1, 100); + + for _ in 0..100 { + let v = (0..dist.sample(&mut rng)) + .map(|_v| (dist.sample(&mut rng) % 255) as u8) + .collect::>(); + assert!(prev_scalars.insert(Curve25519Scalar::from(&v[..]))); + } +} + +#[test] +fn the_string_hash_implementation_uses_the_full_range_of_bits() { + let max_iters = 20; + let mut rng = StdRng::from_seed([0u8; 32]); + let dist = Uniform::new(1, i32::MAX); + + for i in 0..252 { + let mut curr_iters = 0; + let mut bset = IndexSet::default(); + + loop { + let s: Curve25519Scalar = dist.sample(&mut rng).to_string().as_str().into(); + let bytes = s.to_bytes_le(); //Note: this is the only spot that these tests are different from the to_curve25519_scalar tests. + + let is_ith_bit_set = bytes[i / 8] & (1 << (i % 8)) != 0; + + bset.insert(is_ith_bit_set); + + if bset == IndexSet::from_iter([false, true]) { + break; + } + + // this guarantees that, if the above test fails, + // we'll be able to identify it's failing + assert!(curr_iters <= max_iters); + + curr_iters += 1; + } + } +}