diff --git a/ecdsa/src/curve/curve_msm.rs b/ecdsa/src/curve/curve_msm.rs index 6d07c097..f681deb2 100644 --- a/ecdsa/src/curve/curve_msm.rs +++ b/ecdsa/src/curve/curve_msm.rs @@ -207,7 +207,7 @@ mod tests { 0b00001111111111111111111111111111, 0b11111111111111111111111111111111, ]; - let x = Secp256K1Scalar::from_biguint(BigUint::from_slice(&x_canonical)); + let x = Secp256K1Scalar::from_noncanonical_biguint(BigUint::from_slice(&x_canonical)); assert_eq!(x.to_canonical_biguint().to_u32_digits(), x_canonical); assert_eq!( to_digits::(&x, 17), @@ -240,13 +240,13 @@ mod tests { let generator_2 = generator_1 + generator_1; let generator_3 = generator_1 + generator_2; - let scalar_1 = Secp256K1Scalar::from_biguint(BigUint::from_slice(&[ + let scalar_1 = Secp256K1Scalar::from_noncanonical_biguint(BigUint::from_slice(&[ 11111111, 22222222, 33333333, 44444444, ])); - let scalar_2 = Secp256K1Scalar::from_biguint(BigUint::from_slice(&[ + let scalar_2 = Secp256K1Scalar::from_noncanonical_biguint(BigUint::from_slice(&[ 22222222, 22222222, 33333333, 44444444, ])); - let scalar_3 = Secp256K1Scalar::from_biguint(BigUint::from_slice(&[ + let scalar_3 = Secp256K1Scalar::from_noncanonical_biguint(BigUint::from_slice(&[ 33333333, 22222222, 33333333, 44444444, ])); diff --git a/ecdsa/src/curve/curve_types.rs b/ecdsa/src/curve/curve_types.rs index bbf66d65..96821672 100644 --- a/ecdsa/src/curve/curve_types.rs +++ b/ecdsa/src/curve/curve_types.rs @@ -277,9 +277,9 @@ impl Neg for ProjectivePoint { } pub fn base_to_scalar(x: C::BaseField) -> C::ScalarField { - C::ScalarField::from_biguint(x.to_canonical_biguint()) + C::ScalarField::from_noncanonical_biguint(x.to_canonical_biguint()) } pub fn scalar_to_base(x: C::ScalarField) -> C::BaseField { - C::BaseField::from_biguint(x.to_canonical_biguint()) + C::BaseField::from_noncanonical_biguint(x.to_canonical_biguint()) } diff --git a/ecdsa/src/curve/glv.rs b/ecdsa/src/curve/glv.rs index 05ecea44..c58032ec 100644 --- a/ecdsa/src/curve/glv.rs +++ b/ecdsa/src/curve/glv.rs @@ -45,14 +45,14 @@ pub fn decompose_secp256k1_scalar( ) .round() .to_integer(); - let c1 = Secp256K1Scalar::from_biguint(c1_biguint); + let c1 = Secp256K1Scalar::from_noncanonical_biguint(c1_biguint); let c2_biguint = Ratio::new( MINUS_B1.to_canonical_biguint() * k.to_canonical_biguint(), p.clone(), ) .round() .to_integer(); - let c2 = Secp256K1Scalar::from_biguint(c2_biguint); + let c2 = Secp256K1Scalar::from_noncanonical_biguint(c2_biguint); let k1_raw = k - c1 * A1 - c2 * A2; let k2_raw = c1 * MINUS_B1 - c2 * B2; @@ -61,13 +61,13 @@ pub fn decompose_secp256k1_scalar( let two = BigUint::from_slice(&[2]); let k1_neg = k1_raw.to_canonical_biguint() > p.clone() / two.clone(); let k1 = if k1_neg { - Secp256K1Scalar::from_biguint(p.clone() - k1_raw.to_canonical_biguint()) + Secp256K1Scalar::from_noncanonical_biguint(p.clone() - k1_raw.to_canonical_biguint()) } else { k1_raw }; let k2_neg = k2_raw.to_canonical_biguint() > p.clone() / two; let k2 = if k2_neg { - Secp256K1Scalar::from_biguint(p - k2_raw.to_canonical_biguint()) + Secp256K1Scalar::from_noncanonical_biguint(p - k2_raw.to_canonical_biguint()) } else { k2_raw }; diff --git a/ecdsa/src/curve/secp256k1.rs b/ecdsa/src/curve/secp256k1.rs index e46fbb3d..8f7bccf3 100644 --- a/ecdsa/src/curve/secp256k1.rs +++ b/ecdsa/src/curve/secp256k1.rs @@ -71,7 +71,7 @@ mod tests { #[test] fn test_g1_multiplication() { - let lhs = Secp256K1Scalar::from_biguint(BigUint::from_slice(&[ + let lhs = Secp256K1Scalar::from_noncanonical_biguint(BigUint::from_slice(&[ 1111, 2222, 3333, 4444, 5555, 6666, 7777, 8888, ])); assert_eq!( diff --git a/ecdsa/src/gadgets/curve_fixed_base.rs b/ecdsa/src/gadgets/curve_fixed_base.rs index d99d5760..44dc9488 100644 --- a/ecdsa/src/gadgets/curve_fixed_base.rs +++ b/ecdsa/src/gadgets/curve_fixed_base.rs @@ -30,7 +30,7 @@ pub fn fixed_base_curve_mul_circuit, cons let limbs = builder.split_nonnative_to_4_bit_limbs(scalar); let hash_0 = KeccakHash::<32>::hash_no_pad(&[F::ZERO]); - let hash_0_scalar = C::ScalarField::from_biguint(BigUint::from_bytes_le( + let hash_0_scalar = C::ScalarField::from_noncanonical_biguint(BigUint::from_bytes_le( &GenericHashOut::::to_bytes(&hash_0), )); let rando = (CurveScalar(hash_0_scalar) * C::GENERATOR_PROJECTIVE).to_affine(); diff --git a/ecdsa/src/gadgets/curve_msm.rs b/ecdsa/src/gadgets/curve_msm.rs index 1265d399..e059638c 100644 --- a/ecdsa/src/gadgets/curve_msm.rs +++ b/ecdsa/src/gadgets/curve_msm.rs @@ -29,7 +29,7 @@ pub fn curve_msm_circuit, const D: usize> let num_limbs = limbs_n.len(); let hash_0 = KeccakHash::<32>::hash_no_pad(&[F::ZERO]); - let hash_0_scalar = C::ScalarField::from_biguint(BigUint::from_bytes_le( + let hash_0_scalar = C::ScalarField::from_noncanonical_biguint(BigUint::from_bytes_le( &GenericHashOut::::to_bytes(&hash_0), )); let rando = (CurveScalar(hash_0_scalar) * C::GENERATOR_PROJECTIVE).to_affine(); diff --git a/ecdsa/src/gadgets/curve_windowed_mul.rs b/ecdsa/src/gadgets/curve_windowed_mul.rs index d9dcc734..bc4e1caf 100644 --- a/ecdsa/src/gadgets/curve_windowed_mul.rs +++ b/ecdsa/src/gadgets/curve_windowed_mul.rs @@ -131,7 +131,7 @@ impl, const D: usize> CircuitBuilderWindowedMul, ) -> AffinePointTarget { let hash_0 = KeccakHash::<25>::hash_no_pad(&[F::ZERO]); - let hash_0_scalar = C::ScalarField::from_biguint(BigUint::from_bytes_le( + let hash_0_scalar = C::ScalarField::from_noncanonical_biguint(BigUint::from_bytes_le( &GenericHashOut::::to_bytes(&hash_0), )); let starting_point = CurveScalar(hash_0_scalar) * C::GENERATOR_PROJECTIVE; diff --git a/ecdsa/src/gadgets/glv.rs b/ecdsa/src/gadgets/glv.rs index 746d661f..8e62e906 100644 --- a/ecdsa/src/gadgets/glv.rs +++ b/ecdsa/src/gadgets/glv.rs @@ -116,7 +116,7 @@ impl, const D: usize> SimpleGenerator } fn run_once(&self, witness: &PartitionWitness, out_buffer: &mut GeneratedValues) { - let k = Secp256K1Scalar::from_biguint(witness_get_biguint_target( + let k = Secp256K1Scalar::from_noncanonical_biguint(witness_get_biguint_target( witness, self.k.value.clone(), )); diff --git a/ecdsa/src/gadgets/nonnative.rs b/ecdsa/src/gadgets/nonnative.rs index 3c2e2ed6..393aac75 100644 --- a/ecdsa/src/gadgets/nonnative.rs +++ b/ecdsa/src/gadgets/nonnative.rs @@ -467,8 +467,14 @@ impl, const D: usize, FF: PrimeField> SimpleGenerat } fn run_once(&self, witness: &PartitionWitness, out_buffer: &mut GeneratedValues) { - let a = FF::from_biguint(witness_get_biguint_target(witness, self.a.value.clone())); - let b = FF::from_biguint(witness_get_biguint_target(witness, self.b.value.clone())); + let a = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.a.value.clone(), + )); + let b = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.b.value.clone(), + )); let a_biguint = a.to_canonical_biguint(); let b_biguint = b.to_canonical_biguint(); let sum_biguint = a_biguint + b_biguint; @@ -508,7 +514,10 @@ impl, const D: usize, FF: PrimeField> SimpleGenerat .summands .iter() .map(|summand| { - FF::from_biguint(witness_get_biguint_target(witness, summand.value.clone())) + FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + summand.value.clone(), + )) }) .collect(); let summand_biguints: Vec<_> = summands @@ -553,8 +562,14 @@ impl, const D: usize, FF: PrimeField> SimpleGenerat } fn run_once(&self, witness: &PartitionWitness, out_buffer: &mut GeneratedValues) { - let a = FF::from_biguint(witness_get_biguint_target(witness, self.a.value.clone())); - let b = FF::from_biguint(witness_get_biguint_target(witness, self.b.value.clone())); + let a = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.a.value.clone(), + )); + let b = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.b.value.clone(), + )); let a_biguint = a.to_canonical_biguint(); let b_biguint = b.to_canonical_biguint(); @@ -594,8 +609,14 @@ impl, const D: usize, FF: PrimeField> SimpleGenerat } fn run_once(&self, witness: &PartitionWitness, out_buffer: &mut GeneratedValues) { - let a = FF::from_biguint(witness_get_biguint_target(witness, self.a.value.clone())); - let b = FF::from_biguint(witness_get_biguint_target(witness, self.b.value.clone())); + let a = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.a.value.clone(), + )); + let b = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.b.value.clone(), + )); let a_biguint = a.to_canonical_biguint(); let b_biguint = b.to_canonical_biguint(); @@ -625,7 +646,10 @@ impl, const D: usize, FF: PrimeField> SimpleGenerat } fn run_once(&self, witness: &PartitionWitness, out_buffer: &mut GeneratedValues) { - let x = FF::from_biguint(witness_get_biguint_target(witness, self.x.value.clone())); + let x = FF::from_noncanonical_biguint(witness_get_biguint_target( + witness, + self.x.value.clone(), + )); let inv = x.inverse(); let x_biguint = x.to_canonical_biguint(); diff --git a/field/src/extension/quadratic.rs b/field/src/extension/quadratic.rs index d68df42e..278abba9 100644 --- a/field/src/extension/quadratic.rs +++ b/field/src/extension/quadratic.rs @@ -3,7 +3,6 @@ use std::iter::{Product, Sum}; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Sub, SubAssign}; use num::bigint::BigUint; -use num::Integer; use serde::{Deserialize, Serialize}; use crate::extension::{Extendable, FieldExtension, Frobenius, OEF}; @@ -89,9 +88,8 @@ impl> Field for QuadraticExtension { )) } - fn from_biguint(n: BigUint) -> Self { - let (high, low) = n.div_rem(&F::order()); - Self([F::from_biguint(low), F::from_biguint(high)]) + fn from_noncanonical_biguint(n: BigUint) -> Self { + F::from_noncanonical_biguint(n).into() } fn from_canonical_u64(n: u64) -> Self { diff --git a/field/src/extension/quartic.rs b/field/src/extension/quartic.rs index fc0cbcf8..6df39903 100644 --- a/field/src/extension/quartic.rs +++ b/field/src/extension/quartic.rs @@ -4,7 +4,6 @@ use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Sub, SubAssi use num::bigint::BigUint; use num::traits::Pow; -use num::Integer; use serde::{Deserialize, Serialize}; use crate::extension::{Extendable, FieldExtension, Frobenius, OEF}; @@ -94,16 +93,8 @@ impl> Field for QuarticExtension { )) } - fn from_biguint(n: BigUint) -> Self { - let (rest, first) = n.div_rem(&F::order()); - let (rest, second) = rest.div_rem(&F::order()); - let (rest, third) = rest.div_rem(&F::order()); - Self([ - F::from_biguint(first), - F::from_biguint(second), - F::from_biguint(third), - F::from_biguint(rest), - ]) + fn from_noncanonical_biguint(n: BigUint) -> Self { + F::from_noncanonical_biguint(n).into() } fn from_canonical_u64(n: u64) -> Self { diff --git a/field/src/extension/quintic.rs b/field/src/extension/quintic.rs index 564674c3..6680ebc7 100644 --- a/field/src/extension/quintic.rs +++ b/field/src/extension/quintic.rs @@ -99,8 +99,8 @@ impl> Field for QuinticExtension { Some(FieldExtension::<5>::scalar_mul(&f, g.inverse())) } - fn from_biguint(n: BigUint) -> Self { - Self([F::from_biguint(n), F::ZERO, F::ZERO, F::ZERO, F::ZERO]) + fn from_noncanonical_biguint(n: BigUint) -> Self { + F::from_noncanonical_biguint(n).into() } fn from_canonical_u64(n: u64) -> Self { diff --git a/field/src/goldilocks_field.rs b/field/src/goldilocks_field.rs index c5075b5d..c1bb60b0 100644 --- a/field/src/goldilocks_field.rs +++ b/field/src/goldilocks_field.rs @@ -90,7 +90,7 @@ impl Field for GoldilocksField { try_inverse_u64(self) } - fn from_biguint(n: BigUint) -> Self { + fn from_noncanonical_biguint(n: BigUint) -> Self { Self(n.mod_floor(&Self::order()).to_u64_digits()[0]) } diff --git a/field/src/secp256k1_base.rs b/field/src/secp256k1_base.rs index 9e39b982..504d63d7 100644 --- a/field/src/secp256k1_base.rs +++ b/field/src/secp256k1_base.rs @@ -106,7 +106,7 @@ impl Field for Secp256K1Base { Some(self.exp_biguint(&(Self::order() - BigUint::one() - BigUint::one()))) } - fn from_biguint(val: BigUint) -> Self { + fn from_noncanonical_biguint(val: BigUint) -> Self { Self( val.to_u64_digits() .into_iter() @@ -135,7 +135,7 @@ impl Field for Secp256K1Base { #[cfg(feature = "rand")] fn rand_from_rng(rng: &mut R) -> Self { use num::bigint::RandBigInt; - Self::from_biguint(rng.gen_biguint_below(&Self::order())) + Self::from_noncanonical_biguint(rng.gen_biguint_below(&Self::order())) } } @@ -157,7 +157,7 @@ impl Neg for Secp256K1Base { if self.is_zero() { Self::ZERO } else { - Self::from_biguint(Self::order() - self.to_canonical_biguint()) + Self::from_noncanonical_biguint(Self::order() - self.to_canonical_biguint()) } } } @@ -171,7 +171,7 @@ impl Add for Secp256K1Base { if result >= Self::order() { result -= Self::order(); } - Self::from_biguint(result) + Self::from_noncanonical_biguint(result) } } @@ -210,7 +210,7 @@ impl Mul for Secp256K1Base { #[inline] fn mul(self, rhs: Self) -> Self { - Self::from_biguint( + Self::from_noncanonical_biguint( (self.to_canonical_biguint() * rhs.to_canonical_biguint()).mod_floor(&Self::order()), ) } diff --git a/field/src/secp256k1_scalar.rs b/field/src/secp256k1_scalar.rs index eea67fab..e70b154d 100644 --- a/field/src/secp256k1_scalar.rs +++ b/field/src/secp256k1_scalar.rs @@ -115,7 +115,7 @@ impl Field for Secp256K1Scalar { Some(self.exp_biguint(&(Self::order() - BigUint::one() - BigUint::one()))) } - fn from_biguint(val: BigUint) -> Self { + fn from_noncanonical_biguint(val: BigUint) -> Self { Self( val.to_u64_digits() .into_iter() @@ -144,7 +144,7 @@ impl Field for Secp256K1Scalar { #[cfg(feature = "rand")] fn rand_from_rng(rng: &mut R) -> Self { use num::bigint::RandBigInt; - Self::from_biguint(rng.gen_biguint_below(&Self::order())) + Self::from_noncanonical_biguint(rng.gen_biguint_below(&Self::order())) } } @@ -166,7 +166,7 @@ impl Neg for Secp256K1Scalar { if self.is_zero() { Self::ZERO } else { - Self::from_biguint(Self::order() - self.to_canonical_biguint()) + Self::from_noncanonical_biguint(Self::order() - self.to_canonical_biguint()) } } } @@ -180,7 +180,7 @@ impl Add for Secp256K1Scalar { if result >= Self::order() { result -= Self::order(); } - Self::from_biguint(result) + Self::from_noncanonical_biguint(result) } } @@ -219,7 +219,7 @@ impl Mul for Secp256K1Scalar { #[inline] fn mul(self, rhs: Self) -> Self { - Self::from_biguint( + Self::from_noncanonical_biguint( (self.to_canonical_biguint() * rhs.to_canonical_biguint()).mod_floor(&Self::order()), ) } diff --git a/field/src/types.rs b/field/src/types.rs index 87fd8dd4..ac94bcfa 100644 --- a/field/src/types.rs +++ b/field/src/types.rs @@ -270,9 +270,8 @@ pub trait Field: subgroup.into_iter().map(|x| x * shift).collect() } - // TODO: The current behavior for composite fields doesn't seem natural or useful. - // Rename to `from_noncanonical_biguint` and have it return `n % Self::characteristic()`. - fn from_biguint(n: BigUint) -> Self; + /// Returns `n % Self::characteristic()`. + fn from_noncanonical_biguint(n: BigUint) -> Self; /// Returns `n`. Assumes that `n` is already in canonical form, i.e. `n < Self::order()`. // TODO: Should probably be unsafe.