From e93c7a8ef3fedf7fe8ed2a4ec383bdda45c48f01 Mon Sep 17 00:00:00 2001 From: Youssef El Housni Date: Tue, 28 Jan 2025 20:53:34 -0500 Subject: [PATCH 1/5] feat: implement Joye07 scalar mul --- src/groups/curves/short_weierstrass/mod.rs | 132 ++++++++++-------- .../short_weierstrass/non_zero_affine.rs | 119 ++++++++++++++++ src/groups/mod.rs | 83 +++++++++++ 3 files changed, 276 insertions(+), 58 deletions(-) diff --git a/src/groups/curves/short_weierstrass/mod.rs b/src/groups/curves/short_weierstrass/mod.rs index 6adc381..b605cb9 100644 --- a/src/groups/curves/short_weierstrass/mod.rs +++ b/src/groups/curves/short_weierstrass/mod.rs @@ -498,6 +498,80 @@ where Ok(Self::new(self.x.clone(), self.y.negate()?, self.z.clone())) } + /// Computes `bits * self`, where `bits` is a little-endian + /// `Boolean` representation of a scalar. + /// + /// [Joye07](), Alg.1. + #[tracing::instrument(target = "r1cs", skip(bits))] + fn scalar_mul_joye_le<'a>( + &self, + bits: impl Iterator::BasePrimeField>>, + ) -> Result { + if self.is_constant() { + if self.value().unwrap().is_zero() { + return Ok(self.clone()); + } + } + let self_affine = self.to_affine()?; + let (x, y, infinity) = (self_affine.x, self_affine.y, self_affine.infinity); + // We first handle the non-zero case, and then later will conditionally select + // zero if `self` was zero. However, we also want to make sure that generated + // constraints are satisfiable in both cases. + // + // In particular, using non-sensible values for `x` and `y` in zero-case may cause + // `unchecked` operations to generate constraints that can never be satisfied, depending + // on the curve equation coefficients. + // + // The safest approach is to use coordinates of some point from the curve, thus not + // violating assumptions of `NonZeroAffine`. For instance, generator point. + let x = infinity.select(&F::constant(P::GENERATOR.x), &x)?; + let y = infinity.select(&F::constant(P::GENERATOR.y), &y)?; + let non_zero_self = NonZeroAffineVar::new(x, y); + + let mut bits = bits.collect::>(); + if bits.len() == 0 { + return Ok(Self::zero()); + } + // Remove unnecessary constant zeros in the most-significant positions. + bits = bits + .into_iter() + // We iterate from the MSB down. + .rev() + // Skip leading zeros, if they are constants. + .skip_while(|b| b.is_constant() && (b.value().unwrap() == false)) + .collect(); + // After collecting we are in big-endian form; we have to reverse to get back to + // little-endian. + bits.reverse(); + + // second bit + let mut acc = non_zero_self.triple()?; + let mut acc0 = bits[1].select(&acc, &non_zero_self)?; + let mut acc1 = bits[1].select(&non_zero_self, &acc)?; + + for bit in bits.iter().skip(2).rev().skip(1).rev() { + acc = acc0.double_and_select_add_unchecked(bit, &acc1)?; + acc0 = bit.select(&acc, &acc0)?; + acc1 = bit.select(&acc1, &acc)?; + } + + // last bit + let n = bits.len() - 1; + acc = acc0.double_and_select_add_unchecked(bits[n], &acc1)?; + acc0 = bits[n].select(&acc, &acc0)?; + + // first bit + let temp = NonZeroAffineVar::new(non_zero_self.x, non_zero_self.y.negate()?); + acc1 = acc0.add_unchecked(&temp)?; + acc0 = bits[0].select( + &acc0, + &acc1, + )?; + + let mul_result = acc0.into_projective(); + infinity.select(&Self::zero(), &mul_result) + } + /// Computes `bits * self`, where `bits` is a little-endian /// `Boolean` representation of a scalar. #[tracing::instrument(target = "r1cs", skip(bits))] @@ -978,61 +1052,3 @@ where Ok(bytes) } } - -#[cfg(test)] -mod test_sw_curve { - use crate::{ - alloc::AllocVar, - eq::EqGadget, - fields::{fp::FpVar, nonnative::NonNativeFieldVar}, - groups::{curves::short_weierstrass::ProjectiveVar, CurveVar}, - ToBitsGadget, - }; - use ark_ec::{ - short_weierstrass::{Projective, SWCurveConfig}, - CurveGroup, - }; - use ark_ff::PrimeField; - use ark_relations::r1cs::{ConstraintSystem, Result}; - use ark_std::UniformRand; - use num_traits::Zero; - - fn zero_point_scalar_mul_satisfied() -> Result - where - G: CurveGroup, - G::BaseField: PrimeField, - G::Config: SWCurveConfig, - { - let mut rng = ark_std::test_rng(); - - let cs = ConstraintSystem::new_ref(); - let point_in = Projective::::zero(); - let point_out = Projective::::zero(); - let scalar = G::ScalarField::rand(&mut rng); - - let point_in = - ProjectiveVar::>::new_witness(cs.clone(), || { - Ok(point_in) - })?; - let point_out = - ProjectiveVar::>::new_input(cs.clone(), || { - Ok(point_out) - })?; - let scalar = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar))?; - - let mul = point_in.scalar_mul_le(scalar.to_bits_le().unwrap().iter())?; - - point_out.enforce_equal(&mul)?; - - cs.is_satisfied() - } - - #[test] - fn test_zero_point_scalar_mul() { - assert!(zero_point_scalar_mul_satisfied::().unwrap()); - assert!(zero_point_scalar_mul_satisfied::().unwrap()); - assert!(zero_point_scalar_mul_satisfied::().unwrap()); - assert!(zero_point_scalar_mul_satisfied::().unwrap()); - assert!(zero_point_scalar_mul_satisfied::().unwrap()); - } -} diff --git a/src/groups/curves/short_weierstrass/non_zero_affine.rs b/src/groups/curves/short_weierstrass/non_zero_affine.rs index 281b562..8aa40b1 100644 --- a/src/groups/curves/short_weierstrass/non_zero_affine.rs +++ b/src/groups/curves/short_weierstrass/non_zero_affine.rs @@ -130,6 +130,79 @@ where } } + /// Conditionally computes `(self + other) + self` or `(self + other) + other` + /// depending on the value of `cond`. + /// + /// This follows the formulae from [\[ELM03\]](https://arxiv.org/abs/math/0208038). + #[tracing::instrument(target = "r1cs", skip(self))] + pub fn double_and_select_add_unchecked( + &self, + cond: &Boolean<::BasePrimeField>, + other: &Self, + ) -> Result { + if [self].is_constant() || other.is_constant() { + // /!\ TODO: correct constant case /!\ + self.double()?.add_unchecked(other) + } else { + // It's okay to use `unchecked` the precondition is that `self != ±other` (i.e. + // same logic as in `add_unchecked`) + let (x1, y1) = (&self.x, &self.y); + let (x2, y2) = (&other.x, &other.y); + + // Calculate self + other: + // slope lambda := (y2 - y1)/(x2 - x1); + // x3 = lambda^2 - x1 - x2; + // y3 = lambda * (x1 - x3) - y1 + let numerator = y2 - y1; + let denominator = x2 - x1; + let lambda_1 = numerator.mul_by_inverse_unchecked(&denominator)?; + + let x3 = lambda_1.square()? - x1 - x2; + + let x = &F::conditionally_select(&cond, &x1, &x2)?; + let y = &F::conditionally_select(&cond, &y1, &y2)?; + + // Calculate final addition slope: + let lambda_2 = + (lambda_1 + y.double()?.mul_by_inverse_unchecked(&(&x3 - x))?).negate()?; + + // Calculate (self + other) + (self or other): + let x4 = lambda_2.square()? - x - x3; + let y4 = lambda_2 * &(x - &x4) - y; + Ok(Self::new(x4, y4)) + } + } + + /// Triples `self`. + /// + /// This follows the formulae from [\[ELM03\]](https://arxiv.org/abs/math/0208038). + #[tracing::instrument(target = "r1cs", skip(self))] + pub fn triple(&self) -> Result { + if [self].is_constant() { + self.double()?.add_unchecked(self) + } else { + let (x1, y1) = (&self.x, &self.y); + let x1_sqr = x1.square()?; + // tangent lambda_1 := (3 * x1^2 + a) / (2 * y1); + // x3 = lambda_1^2 - 2x1 + // y3 = lambda_1 * (x1 - x3) - y1 + let numerator = x1_sqr.double()? + &x1_sqr + P::COEFF_A; + let denominator = y1.double()?; + // It's okay to use `unchecked` here, because the precondition of `double` is + // that self != zero. + let lambda_1 = numerator.mul_by_inverse_unchecked(&denominator)?; + let x3 = lambda_1.square()? - x1.double()?; + + // Calculate final addition slope: + let lambda_2 = + (lambda_1 + y1.double()?.mul_by_inverse_unchecked(&(&x3 - x1))?).negate()?; + + let x4 = lambda_2.square()? - x1 - x3; + let y4 = lambda_2 * &(x1 - &x4) - y1; + Ok(Self::new(x4, y4)) + } + } + /// Doubles `self` in place. #[tracing::instrument(target = "r1cs", skip(self))] pub fn double_in_place(&mut self) -> Result<(), SynthesisError> { @@ -390,4 +463,50 @@ mod test_non_zero_affine { assert!(cs.is_satisfied().unwrap()); } + + #[test] + fn correctness_double_and_add_select() { + let cs = ConstraintSystem::::new_ref(); + + let x = FpVar::Var( + AllocatedFp::::new_witness(cs.clone(), || Ok(G1Config::GENERATOR.x)).unwrap(), + ); + let y = FpVar::Var( + AllocatedFp::::new_witness(cs.clone(), || Ok(G1Config::GENERATOR.y)).unwrap(), + ); + + // The following code tests `double_and_add`. + let sum_a = { + let a = ProjectiveVar::>::new( + x.clone(), + y.clone(), + FpVar::Constant(Fq::one()), + ); + + let mut cur = a.clone(); + cur.double_in_place().unwrap(); + for _ in 1..10 { + cur.double_in_place().unwrap(); + cur = cur + &a; + } + + let sum = cur.value().unwrap().into_affine(); + (sum.x, sum.y) + }; + + let sum_b = { + let a = NonZeroAffineVar::>::new(x, y); + + let mut cur = a.double().unwrap(); + for _ in 1..10 { + cur = cur.double_and_add_unchecked(&a).unwrap(); + } + + (cur.x.value().unwrap(), cur.y.value().unwrap()) + }; + + assert!(cs.is_satisfied().unwrap()); + assert_eq!(sum_a.0, sum_b.0); + assert_eq!(sum_a.1, sum_b.1); + } } diff --git a/src/groups/mod.rs b/src/groups/mod.rs index fb8c746..7596808 100644 --- a/src/groups/mod.rs +++ b/src/groups/mod.rs @@ -107,6 +107,30 @@ pub trait CurveVar: Ok(res) } + /// Computes `bits * self`, where `bits` is a little-endian + /// `Boolean` representation of a scalar. + /// + /// [Joye07](), Alg.1. + #[tracing::instrument(target = "r1cs", skip(bits))] + fn scalar_mul_joye_le<'a>( + &self, + bits: impl Iterator>, + ) -> Result { + // TODO: in the constant case we should call precomputed_scalar_mul_le, + // but rn there's a bug when doing this with TE curves. + + // Computes the standard little-endian double-and-add algorithm + // (Algorithm 3.26, Guide to Elliptic Curve Cryptography) + let mut res = Self::zero(); + let mut multiple = self.clone(); + for bit in bits { + let tmp = res.clone() + &multiple; + res = bit.select(&tmp, &res)?; + multiple.double_in_place()?; + } + Ok(res) + } + /// Computes a `I * self` in place, where `I` is a `Boolean` *little-endian* /// representation of the scalar. /// @@ -161,3 +185,62 @@ pub trait CurveVar: Ok(result) } } + +#[cfg(test)] +mod test_sw_arithmetic { + use crate::{ + alloc::AllocVar, + eq::EqGadget, + fields::{fp::FpVar, nonnative::NonNativeFieldVar}, + groups::{curves::short_weierstrass::ProjectiveVar, CurveVar}, + ToBitsGadget, + }; + use ark_ec::{ + short_weierstrass::{Projective, SWCurveConfig}, + CurveGroup, + }; + use ark_ff::PrimeField; + use ark_relations::r1cs::{ConstraintSystem, Result}; + use ark_std::UniformRand; + + fn point_scalar_mul_joye_satisfied() -> Result + where + G: CurveGroup, + G::BaseField: PrimeField, + G::Config: SWCurveConfig, + { + let mut rng = ark_std::test_rng(); + + let cs = ConstraintSystem::new_ref(); + let point_in = Projective::::rand(&mut rng); + let scalar = G::ScalarField::rand(&mut rng); + let point_out = point_in * scalar; + + let point_in = + ProjectiveVar::>::new_witness(cs.clone(), || { + Ok(point_in) + })?; + let point_out = + ProjectiveVar::>::new_input(cs.clone(), || { + Ok(point_out) + })?; + let scalar = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar))?; + + let mul = point_in.scalar_mul_joye_le(scalar.to_bits_le().unwrap().iter())?; + + point_out.enforce_equal(&mul)?; + + println!( + "#r1cs for scalar_mul_joye_le: {}", + cs.num_constraints() + ); + + + cs.is_satisfied() + } + + #[test] + fn test_point_scalar_mul_joye() { + assert!(point_scalar_mul_joye_satisfied::().unwrap()); + } +} From 0ae123fbb1b948dabd4c5424937f201bbf5e5df6 Mon Sep 17 00:00:00 2001 From: Youssef El Housni Date: Tue, 28 Jan 2025 20:59:40 -0500 Subject: [PATCH 2/5] feat: implement joint scalar mul --- src/groups/curves/short_weierstrass/mod.rs | 66 ++++++++++++++++++++++ src/groups/mod.rs | 64 ++++++++++++++++++++- 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/src/groups/curves/short_weierstrass/mod.rs b/src/groups/curves/short_weierstrass/mod.rs index b605cb9..8251bdb 100644 --- a/src/groups/curves/short_weierstrass/mod.rs +++ b/src/groups/curves/short_weierstrass/mod.rs @@ -632,6 +632,72 @@ where infinity.select(&Self::zero(), &mul_result) } + /// Computes `bits1 * self + bits2 * p`, where `bits1` and `bits2` are big-endian + /// `Boolean` representation of the scalars. + /// + /// `self` and `p` are non-zero and `self` ≠ `-p`. + #[tracing::instrument(target = "r1cs", skip(bits1, bits2))] + fn joint_scalar_mul_be<'a>( + &self, + p: &Self, + bits1: impl Iterator::BasePrimeField>>, + bits2: impl Iterator::BasePrimeField>>, + ) -> Result { + // prepare bits decomposition + let mut bits1 = bits1.collect::>(); + if bits1.len() == 0 { + return Ok(Self::zero()); + } + // Remove unnecessary constant zeros in the most-significant positions. + bits1 = bits1 + .into_iter() + // We iterate from the MSB down. + .rev() + // Skip leading zeros, if they are constants. + .skip_while(|b| b.is_constant() && (b.value().unwrap() == false)) + .collect(); + + let mut bits2 = bits2.collect::>(); + if bits2.len() == 0 { + return Ok(Self::zero()); + } + // Remove unnecessary constant zeros in the most-significant positions. + bits2 = bits2 + .into_iter() + // We iterate from the MSB down. + .rev() + // Skip leading zeros, if they are constants. + .skip_while(|b| b.is_constant() && (b.value().unwrap() == false)) + .collect(); + + // precompute points + let aff1 = self.to_affine()?; + let nz_aff1 = NonZeroAffineVar::new(aff1.x, aff1.y); + + let aff2 = p.to_affine()?; + let nz_aff2 = NonZeroAffineVar::new(aff2.x, aff2.y); + + let mut aff1_neg = NonZeroAffineVar::new(nz_aff1.x.clone(), nz_aff1.y.negate()?); + let mut aff2_neg = NonZeroAffineVar::new(nz_aff2.x.clone(), nz_aff2.y.negate()?); + let mut acc = nz_aff1.add_unchecked(&nz_aff2.clone())?; + + // double-and-add loop + for (bit1, bit2) in (bits1.iter().rev().skip(1).rev()).zip(bits2.iter().rev().skip(1).rev()) { + let mut b = bit1.select(&nz_aff1, &aff1_neg)?; + acc = acc.double_and_add_unchecked(&b)?; + b = bit2.select(&nz_aff2, &aff2_neg)?; + acc = acc.add_unchecked(&b)?; + } + + // last bit + aff1_neg = aff1_neg.add_unchecked(&acc)?; + acc = bits1[bits1.len()-1].select(&acc, &aff1_neg)?; + aff2_neg = aff2_neg.add_unchecked(&acc)?; + acc = bits2[bits1.len()-1].select(&acc, &aff2_neg)?; + + Ok(acc.into_projective()) + } + #[tracing::instrument(target = "r1cs", skip(scalar_bits_with_bases))] fn precomputed_base_scalar_mul_le<'a, I, B>( &mut self, diff --git a/src/groups/mod.rs b/src/groups/mod.rs index 7596808..d9fc650 100644 --- a/src/groups/mod.rs +++ b/src/groups/mod.rs @@ -131,6 +131,20 @@ pub trait CurveVar: Ok(res) } + /// Computes a `I1 * self + I2 * p` in place, where `I1` and `I2` are `Boolean` *big-endian* + /// representation of the scalars. + #[tracing::instrument(target = "r1cs", skip(bits1, bits2))] + fn joint_scalar_mul_be<'a>( + &self, + p: &Self, + bits1: impl Iterator>, + bits2: impl Iterator>, + ) -> Result { + let res1 = self.scalar_mul_le(bits1)?; + let res2 = p.scalar_mul_le(bits2)?; + Ok(res1+res2) + } + /// Computes a `I * self` in place, where `I` is a `Boolean` *little-endian* /// representation of the scalar. /// @@ -239,8 +253,56 @@ mod test_sw_arithmetic { cs.is_satisfied() } + fn point_joint_scalar_mul_satisfied() -> Result + where + G: CurveGroup, + G::BaseField: PrimeField, + G::Config: SWCurveConfig, + { + let mut rng = ark_std::test_rng(); + + let cs = ConstraintSystem::new_ref(); + let point_in1 = Projective::::rand(&mut rng); + let point_in2 = Projective::::rand(&mut rng); + let scalar1 = G::ScalarField::rand(&mut rng); + let scalar2 = G::ScalarField::rand(&mut rng); + let point_out = point_in1 * scalar1 + point_in2 * scalar2; + + let point_in1 = + ProjectiveVar::>::new_witness(cs.clone(), || { + Ok(point_in1) + })?; + let point_in2 = + ProjectiveVar::>::new_witness(cs.clone(), || { + Ok(point_in2) + })?; + let point_out = + ProjectiveVar::>::new_input(cs.clone(), || { + Ok(point_out) + })?; + let scalar1 = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar1))?; + let scalar2 = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar2))?; + + let res = point_in1.joint_scalar_mul_be(&point_in2, scalar1.to_bits_le().unwrap().iter(), scalar2.to_bits_le().unwrap().iter())?; + + point_out.enforce_equal(&res)?; + + println!( + "#r1cs for joint_scalar_mul: {}", + cs.num_constraints() + ); + + + cs.is_satisfied() + } + #[test] - fn test_point_scalar_mul_joye() { + fn test_point_scalar_mul() { assert!(point_scalar_mul_joye_satisfied::().unwrap()); } + #[test] + fn test_point_joint_scalar_mul() { + assert!(point_joint_scalar_mul_satisfied::().unwrap()); + } + } From e67ed82eb5c3d93e59e29c6e829e11351114885f Mon Sep 17 00:00:00 2001 From: Youssef El Housni Date: Thu, 30 Jan 2025 13:56:16 -0500 Subject: [PATCH 3/5] perf: save 1 dbl in scalar_mul_le + fmt --- src/groups/curves/short_weierstrass/mod.rs | 55 +++++++++++------ .../short_weierstrass/non_zero_affine.rs | 4 +- src/groups/mod.rs | 61 ++++++++++++++----- 3 files changed, 83 insertions(+), 37 deletions(-) diff --git a/src/groups/curves/short_weierstrass/mod.rs b/src/groups/curves/short_weierstrass/mod.rs index 8251bdb..b9d09bf 100644 --- a/src/groups/curves/short_weierstrass/mod.rs +++ b/src/groups/curves/short_weierstrass/mod.rs @@ -356,7 +356,7 @@ where *mul_result += result - subtrahend; // Now, let's finish off the rest of the bits using our complete formulae - for bit in proj_bits { + for bit in proj_bits.iter().rev().skip(1).rev() { if bit.is_constant() { if *bit == &Boolean::TRUE { *mul_result += &multiple_of_power_of_two.into_projective(); @@ -367,6 +367,21 @@ where } multiple_of_power_of_two.double_in_place()?; } + + // last bit + // we don't need the last doubling of multiple_of_power_of_two + let n = proj_bits.len(); + if n >= 1 { + if proj_bits[n - 1].is_constant() { + if proj_bits[n - 1] == &Boolean::TRUE { + *mul_result += &multiple_of_power_of_two.into_projective(); + } + } else { + let temp = &*mul_result + &multiple_of_power_of_two.into_projective(); + *mul_result = proj_bits[n - 1].select(&temp, &mul_result)?; + } + } + Ok(()) } } @@ -518,12 +533,13 @@ where // zero if `self` was zero. However, we also want to make sure that generated // constraints are satisfiable in both cases. // - // In particular, using non-sensible values for `x` and `y` in zero-case may cause - // `unchecked` operations to generate constraints that can never be satisfied, depending - // on the curve equation coefficients. + // In particular, using non-sensible values for `x` and `y` in zero-case may + // cause `unchecked` operations to generate constraints that can never + // be satisfied, depending on the curve equation coefficients. // - // The safest approach is to use coordinates of some point from the curve, thus not - // violating assumptions of `NonZeroAffine`. For instance, generator point. + // The safest approach is to use coordinates of some point from the curve, thus + // not violating assumptions of `NonZeroAffine`. For instance, generator + // point. let x = infinity.select(&F::constant(P::GENERATOR.x), &x)?; let y = infinity.select(&F::constant(P::GENERATOR.y), &y)?; let non_zero_self = NonZeroAffineVar::new(x, y); @@ -563,10 +579,7 @@ where // first bit let temp = NonZeroAffineVar::new(non_zero_self.x, non_zero_self.y.negate()?); acc1 = acc0.add_unchecked(&temp)?; - acc0 = bits[0].select( - &acc0, - &acc1, - )?; + acc0 = bits[0].select(&acc0, &acc1)?; let mul_result = acc0.into_projective(); infinity.select(&Self::zero(), &mul_result) @@ -590,12 +603,13 @@ where // zero if `self` was zero. However, we also want to make sure that generated // constraints are satisfiable in both cases. // - // In particular, using non-sensible values for `x` and `y` in zero-case may cause - // `unchecked` operations to generate constraints that can never be satisfied, depending - // on the curve equation coefficients. + // In particular, using non-sensible values for `x` and `y` in zero-case may + // cause `unchecked` operations to generate constraints that can never + // be satisfied, depending on the curve equation coefficients. // - // The safest approach is to use coordinates of some point from the curve, thus not - // violating assumptions of `NonZeroAffine`. For instance, generator point. + // The safest approach is to use coordinates of some point from the curve, thus + // not violating assumptions of `NonZeroAffine`. For instance, generator + // point. let x = infinity.select(&F::constant(P::GENERATOR.x), &x)?; let y = infinity.select(&F::constant(P::GENERATOR.y), &y)?; let non_zero_self = NonZeroAffineVar::new(x, y); @@ -632,8 +646,8 @@ where infinity.select(&Self::zero(), &mul_result) } - /// Computes `bits1 * self + bits2 * p`, where `bits1` and `bits2` are big-endian - /// `Boolean` representation of the scalars. + /// Computes `bits1 * self + bits2 * p`, where `bits1` and `bits2` are + /// big-endian `Boolean` representation of the scalars. /// /// `self` and `p` are non-zero and `self` ≠ `-p`. #[tracing::instrument(target = "r1cs", skip(bits1, bits2))] @@ -682,7 +696,8 @@ where let mut acc = nz_aff1.add_unchecked(&nz_aff2.clone())?; // double-and-add loop - for (bit1, bit2) in (bits1.iter().rev().skip(1).rev()).zip(bits2.iter().rev().skip(1).rev()) { + for (bit1, bit2) in (bits1.iter().rev().skip(1).rev()).zip(bits2.iter().rev().skip(1).rev()) + { let mut b = bit1.select(&nz_aff1, &aff1_neg)?; acc = acc.double_and_add_unchecked(&b)?; b = bit2.select(&nz_aff2, &aff2_neg)?; @@ -691,9 +706,9 @@ where // last bit aff1_neg = aff1_neg.add_unchecked(&acc)?; - acc = bits1[bits1.len()-1].select(&acc, &aff1_neg)?; + acc = bits1[bits1.len() - 1].select(&acc, &aff1_neg)?; aff2_neg = aff2_neg.add_unchecked(&acc)?; - acc = bits2[bits1.len()-1].select(&acc, &aff2_neg)?; + acc = bits2[bits1.len() - 1].select(&acc, &aff2_neg)?; Ok(acc.into_projective()) } diff --git a/src/groups/curves/short_weierstrass/non_zero_affine.rs b/src/groups/curves/short_weierstrass/non_zero_affine.rs index 8aa40b1..e1268af 100644 --- a/src/groups/curves/short_weierstrass/non_zero_affine.rs +++ b/src/groups/curves/short_weierstrass/non_zero_affine.rs @@ -130,8 +130,8 @@ where } } - /// Conditionally computes `(self + other) + self` or `(self + other) + other` - /// depending on the value of `cond`. + /// Conditionally computes `(self + other) + self` or `(self + other) + + /// other` depending on the value of `cond`. /// /// This follows the formulae from [\[ELM03\]](https://arxiv.org/abs/math/0208038). #[tracing::instrument(target = "r1cs", skip(self))] diff --git a/src/groups/mod.rs b/src/groups/mod.rs index d9fc650..bb26e0d 100644 --- a/src/groups/mod.rs +++ b/src/groups/mod.rs @@ -131,8 +131,8 @@ pub trait CurveVar: Ok(res) } - /// Computes a `I1 * self + I2 * p` in place, where `I1` and `I2` are `Boolean` *big-endian* - /// representation of the scalars. + /// Computes a `I1 * self + I2 * p` in place, where `I1` and `I2` are + /// `Boolean` *big-endian* representation of the scalars. #[tracing::instrument(target = "r1cs", skip(bits1, bits2))] fn joint_scalar_mul_be<'a>( &self, @@ -142,7 +142,7 @@ pub trait CurveVar: ) -> Result { let res1 = self.scalar_mul_le(bits1)?; let res2 = p.scalar_mul_le(bits2)?; - Ok(res1+res2) + Ok(res1 + res2) } /// Computes a `I * self` in place, where `I` is a `Boolean` *little-endian* @@ -217,6 +217,38 @@ mod test_sw_arithmetic { use ark_relations::r1cs::{ConstraintSystem, Result}; use ark_std::UniformRand; + fn point_scalar_mul_satisfied() -> Result + where + G: CurveGroup, + G::BaseField: PrimeField, + G::Config: SWCurveConfig, + { + let mut rng = ark_std::test_rng(); + + let cs = ConstraintSystem::new_ref(); + let point_in = Projective::::rand(&mut rng); + let scalar = G::ScalarField::rand(&mut rng); + let point_out = point_in * scalar; + + let point_in = + ProjectiveVar::>::new_witness(cs.clone(), || { + Ok(point_in) + })?; + let point_out = + ProjectiveVar::>::new_input(cs.clone(), || { + Ok(point_out) + })?; + let scalar = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar))?; + + let mul = point_in.scalar_mul_le(scalar.to_bits_le().unwrap().iter())?; + + point_out.enforce_equal(&mul)?; + + println!("#r1cs for scalar_mul_le: {}", cs.num_constraints()); + + cs.is_satisfied() + } + fn point_scalar_mul_joye_satisfied() -> Result where G: CurveGroup, @@ -244,11 +276,7 @@ mod test_sw_arithmetic { point_out.enforce_equal(&mul)?; - println!( - "#r1cs for scalar_mul_joye_le: {}", - cs.num_constraints() - ); - + println!("#r1cs for scalar_mul_joye_le: {}", cs.num_constraints()); cs.is_satisfied() } @@ -283,26 +311,29 @@ mod test_sw_arithmetic { let scalar1 = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar1))?; let scalar2 = NonNativeFieldVar::new_input(cs.clone(), || Ok(scalar2))?; - let res = point_in1.joint_scalar_mul_be(&point_in2, scalar1.to_bits_le().unwrap().iter(), scalar2.to_bits_le().unwrap().iter())?; + let res = point_in1.joint_scalar_mul_be( + &point_in2, + scalar1.to_bits_le().unwrap().iter(), + scalar2.to_bits_le().unwrap().iter(), + )?; point_out.enforce_equal(&res)?; - println!( - "#r1cs for joint_scalar_mul: {}", - cs.num_constraints() - ); - + println!("#r1cs for joint_scalar_mul: {}", cs.num_constraints()); cs.is_satisfied() } #[test] fn test_point_scalar_mul() { + assert!(point_scalar_mul_satisfied::().unwrap()); + } + #[test] + fn test_point_scalar_mul_joye() { assert!(point_scalar_mul_joye_satisfied::().unwrap()); } #[test] fn test_point_joint_scalar_mul() { assert!(point_joint_scalar_mul_satisfied::().unwrap()); } - } From e6bcd582aaa0cd3178de007cfaf872f47bdaf1ff Mon Sep 17 00:00:00 2001 From: Youssef El Housni Date: Fri, 31 Jan 2025 13:26:08 -0500 Subject: [PATCH 4/5] perf(scalar_mul_le): use add_mixed for tail bits --- src/groups/curves/short_weierstrass/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/groups/curves/short_weierstrass/mod.rs b/src/groups/curves/short_weierstrass/mod.rs index b9d09bf..ebc64cf 100644 --- a/src/groups/curves/short_weierstrass/mod.rs +++ b/src/groups/curves/short_weierstrass/mod.rs @@ -359,10 +359,10 @@ where for bit in proj_bits.iter().rev().skip(1).rev() { if bit.is_constant() { if *bit == &Boolean::TRUE { - *mul_result += &multiple_of_power_of_two.into_projective(); + *mul_result = mul_result.add_mixed(&multiple_of_power_of_two)?; } } else { - let temp = &*mul_result + &multiple_of_power_of_two.into_projective(); + let temp = mul_result.add_mixed(&multiple_of_power_of_two)?; *mul_result = bit.select(&temp, &mul_result)?; } multiple_of_power_of_two.double_in_place()?; @@ -374,10 +374,10 @@ where if n >= 1 { if proj_bits[n - 1].is_constant() { if proj_bits[n - 1] == &Boolean::TRUE { - *mul_result += &multiple_of_power_of_two.into_projective(); + *mul_result = mul_result.add_mixed(&multiple_of_power_of_two)?; } } else { - let temp = &*mul_result + &multiple_of_power_of_two.into_projective(); + let temp = mul_result.add_mixed(&multiple_of_power_of_two)?; *mul_result = proj_bits[n - 1].select(&temp, &mul_result)?; } } From b120f9e111143b94747155d676e0e8e2e7afd5a6 Mon Sep 17 00:00:00 2001 From: Youssef El Housni Date: Fri, 31 Jan 2025 14:49:02 -0500 Subject: [PATCH 5/5] perf(scalar_mul_le): use add_mixed for conditional subtraction --- src/groups/curves/short_weierstrass/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/groups/curves/short_weierstrass/mod.rs b/src/groups/curves/short_weierstrass/mod.rs index ebc64cf..94f74c6 100644 --- a/src/groups/curves/short_weierstrass/mod.rs +++ b/src/groups/curves/short_weierstrass/mod.rs @@ -350,10 +350,10 @@ where // We can convert to projective safely because the result is guaranteed to be // non-zero by the condition on `affine_bits.len()`, and by the fact // that `accumulator` is non-zero - let result = accumulator.into_projective(); + *mul_result += accumulator.into_projective(); // If bits[0] is 0, then we have to subtract `self`; else, we subtract zero. - let subtrahend = bits[0].select(&Self::zero(), &initial_acc_value)?; - *mul_result += result - subtrahend; + let neg = NonZeroAffineVar::new(initial_acc_value.x, initial_acc_value.y.negate()?); + *mul_result = bits[0].select(mul_result, &mul_result.add_mixed(&neg)?)?; // Now, let's finish off the rest of the bits using our complete formulae for bit in proj_bits.iter().rev().skip(1).rev() {