Fix incorrect decomposition in GLV (arkworks-rs#803)

* update the implementation * update the changelog * fmt
winderica · Mar 25, 2024 · bb663bc · bb663bc
1 parent de81cf0
commit bb663bc
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@
 ### Bugfixes
 
 - [\#747](https://github.com/arkworks-rs/algebra/pull/747) (`ark-ff-macros`) Fix fetching attributes in `MontConfig` macro
+- [\#803](https://github.com/arkworks-rs/algebra/pull/803) (`ark-ec`, `ark-test-template`) Fix incorrect decomposition in GLV
 
 ## v0.4.2
 

diff --git a/ec/Cargo.toml b/ec/Cargo.toml
@@ -23,6 +23,7 @@ ark-poly.workspace = true
 derivative = { workspace = true, features = ["use_core"] }
 num-bigint.workspace = true
 num-traits.workspace = true
+num-integer.workspace = true
 rayon = { workspace = true, optional = true }
 zeroize = { workspace = true, features = ["zeroize_derive"] }
 hashbrown.workspace = true

diff --git a/ec/src/scalar_mul/glv.rs b/ec/src/scalar_mul/glv.rs
@@ -3,8 +3,10 @@ use crate::{
  AdditiveGroup, CurveGroup,
 };
 use ark_ff::{PrimeField, Zero};
+use ark_std::ops::{AddAssign, Neg};
 use num_bigint::{BigInt, BigUint, Sign};
-use num_traits::Signed;
+use num_integer::Integer;
+use num_traits::{One, Signed};
 
 /// The GLV parameters for computing the endomorphism and scalar decomposition.
 pub trait GLVConfig: Send + Sync + 'static + SWCurveConfig {
@@ -39,8 +41,20 @@ pub trait GLVConfig: Send + Sync + 'static + SWCurveConfig {
  // The inverse of N is 1/r * Matrix([[n22, -n12], [-n21, n11]]).
  // so β = (k*n22, -k*n12)/r
 
- let beta_1 = &scalar * &n22 / &r;
- let beta_2 = &scalar * &n12 / &r;
+ let beta_1 = {
+ let (mut div, rem) = (&scalar * &n22).div_rem(&r);
+ if (&rem + &rem) > r {
+ div.add_assign(BigInt::one());
+ }
+ div
+ };
+ let beta_2 = {
+ let (mut div, rem) = (&scalar * &n12.clone().neg()).div_rem(&r);
+ if (&rem + &rem) > r {
+ div.add_assign(BigInt::one());
+ }
+ div
+ };
 
  // b = vector([int(beta[0]), int(beta[1])]) * self.curve.N
  // b = (β1N11 + β2N21, β1N12 + β2N22) with the signs!

diff --git a/test-templates/src/glv.rs b/test-templates/src/glv.rs
@@ -1,12 +1,10 @@
-use std::ops::Mul;
-
 use ark_ec::{
  scalar_mul::{glv::GLVConfig, sw_double_and_add_affine, sw_double_and_add_projective},
  short_weierstrass::{Affine, Projective},
  AffineRepr, CurveGroup, PrimeGroup,
 };
-use ark_ff::PrimeField;
-use ark_std::UniformRand;
+use ark_ff::{BigInteger, PrimeField};
+use ark_std::{ops::Mul, UniformRand};
 
 pub fn glv_scalar_decomposition<P: GLVConfig>() {
  let mut rng = ark_std::test_rng();
@@ -28,7 +26,19 @@ pub fn glv_scalar_decomposition<P: GLVConfig>() {
  if !is_k1_positive && !is_k2_positive {
  assert_eq!(-k1 - k2 * P::LAMBDA, k);
  }
- // could be nice to check if k1 and k2 are indeed small.
+
+ // check if k1 and k2 are indeed small.
+ let expected_max_bits = (P::ScalarField::MODULUS_BIT_SIZE + 1) / 2;
+ assert!(
+ k1.into_bigint().num_bits() <= expected_max_bits,
+ "k1 has {} bits",
+ k1.into_bigint().num_bits()
+ );
+ assert!(
+ k2.into_bigint().num_bits() <= expected_max_bits,
+ "k2 has {} bits",
+ k2.into_bigint().num_bits()
+ );
  }
 }