From 2571862f00eb997c4bacc2118b731ce18a05e996 Mon Sep 17 00:00:00 2001
From: wborgeaud <williamborgeaud@gmail.com>
Date: Wed, 2 Mar 2022 13:31:16 +0100
Subject: [PATCH] Working GLV decomposition check

---
 plonky2/src/curve/glv.rs         | 12 ++++++------
 plonky2/src/gadgets/curve.rs     | 12 ++++--------
 plonky2/src/gadgets/glv.rs       | 12 +++++++++---
 plonky2/src/gadgets/nonnative.rs | 13 +++++++++++++
 4 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/plonky2/src/curve/glv.rs b/plonky2/src/curve/glv.rs
index 11172be0..aeeb463e 100644
--- a/plonky2/src/curve/glv.rs
+++ b/plonky2/src/curve/glv.rs
@@ -8,14 +8,14 @@ use crate::curve::curve_msm::msm_parallel;
 use crate::curve::curve_types::{AffinePoint, ProjectivePoint};
 use crate::curve::secp256k1::Secp256K1;
 
-pub const BETA: Secp256K1Base = Secp256K1Base([
+pub const GLV_BETA: Secp256K1Base = Secp256K1Base([
     13923278643952681454,
     11308619431505398165,
     7954561588662645993,
     8856726876819556112,
 ]);
 
-const S: Secp256K1Scalar = Secp256K1Scalar([
+pub const GLV_S: Secp256K1Scalar = Secp256K1Scalar([
     16069571880186789234,
     1310022930574435960,
     11900229862571533402,
@@ -52,7 +52,7 @@ pub fn decompose_secp256k1_scalar(
 
     let k1_raw = k - c1 * A1 - c2 * A2;
     let k2_raw = c1 * MINUS_B1 - c2 * B2;
-    debug_assert!(k1_raw + S * k2_raw == k);
+    debug_assert!(k1_raw + GLV_S * k2_raw == k);
 
     let two = BigUint::from_slice(&[2]);
     let k1_neg = k1_raw.to_canonical_biguint() > p.clone() / two.clone();
@@ -80,7 +80,7 @@ pub fn glv_mul(p: ProjectivePoint<Secp256K1>, k: Secp256K1Scalar) -> ProjectiveP
 
     let p_affine = p.to_affine();
     let sp = AffinePoint::<Secp256K1> {
-        x: p_affine.x * BETA,
+        x: p_affine.x * GLV_BETA,
         y: p_affine.y,
         zero: p_affine.zero,
     };
@@ -102,7 +102,7 @@ mod tests {
     use plonky2_field::secp256k1_scalar::Secp256K1Scalar;
 
     use crate::curve::curve_types::{Curve, CurveScalar};
-    use crate::curve::glv::{decompose_secp256k1_scalar, glv_mul, S};
+    use crate::curve::glv::{decompose_secp256k1_scalar, glv_mul, GLV_S};
     use crate::curve::secp256k1::Secp256K1;
 
     #[test]
@@ -113,7 +113,7 @@ mod tests {
         let m1 = if k1_neg { -one } else { one };
         let m2 = if k2_neg { -one } else { one };
 
-        assert!(k1 * m1 + S * k2 * m2 == k);
+        assert!(k1 * m1 + GLV_S * k2 * m2 == k);
 
         Ok(())
     }
diff --git a/plonky2/src/gadgets/curve.rs b/plonky2/src/gadgets/curve.rs
index a1fc3a8b..e4e66a4e 100644
--- a/plonky2/src/gadgets/curve.rs
+++ b/plonky2/src/gadgets/curve.rs
@@ -76,14 +76,10 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         p: &AffinePointTarget<C>,
         b: BoolTarget,
     ) -> AffinePointTarget<C> {
-        let not_b = self.not(b);
-        let neg = self.curve_neg(p);
-        let y_if_true = self.mul_nonnative_by_bool(&neg.y, b);
-        let y_if_false = self.mul_nonnative_by_bool(&p.y, not_b);
-
-        let y = self.add_nonnative(&y_if_true, &y_if_false);
-
-        AffinePointTarget { x: p.x.clone(), y }
+        AffinePointTarget {
+            x: p.x.clone(),
+            y: self.nonnative_conditional_neg(&p.y, b),
+        }
     }
 
     pub fn curve_double<C: Curve>(&mut self, p: &AffinePointTarget<C>) -> AffinePointTarget<C> {
diff --git a/plonky2/src/gadgets/glv.rs b/plonky2/src/gadgets/glv.rs
index e0a4cfaa..8447137d 100644
--- a/plonky2/src/gadgets/glv.rs
+++ b/plonky2/src/gadgets/glv.rs
@@ -4,7 +4,7 @@ use plonky2_field::extension_field::Extendable;
 use plonky2_field::secp256k1_base::Secp256K1Base;
 use plonky2_field::secp256k1_scalar::Secp256K1Scalar;
 
-use crate::curve::glv::{decompose_secp256k1_scalar, BETA};
+use crate::curve::glv::{decompose_secp256k1_scalar, GLV_BETA, GLV_S};
 use crate::curve::secp256k1::Secp256K1;
 use crate::gadgets::curve::AffinePointTarget;
 use crate::gadgets::nonnative::NonNativeTarget;
@@ -16,7 +16,7 @@ use crate::plonk::circuit_builder::CircuitBuilder;
 
 impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     pub fn secp256k1_glv_beta(&mut self) -> NonNativeTarget<Secp256K1Base> {
-        self.constant_nonnative(BETA)
+        self.constant_nonnative(GLV_BETA)
     }
 
     // TODO: Add decomposition check.
@@ -43,7 +43,13 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
             _phantom: PhantomData,
         });
 
-        // debug_assert!(k1_raw + S * k2_raw == k);
+        // Check that `k1_raw + GLV_S * k2_raw == k`.
+        let k1_raw = self.nonnative_conditional_neg(&k1, k1_neg);
+        let k2_raw = self.nonnative_conditional_neg(&k2, k2_neg);
+        let s = self.constant_nonnative(GLV_S);
+        let mut should_be_k = self.mul_nonnative(&s, &k2_raw);
+        should_be_k = self.add_nonnative(&should_be_k, &k1_raw);
+        self.connect_nonnative(&should_be_k, &k);
 
         (k1, k2, k1_neg, k2_neg)
     }
diff --git a/plonky2/src/gadgets/nonnative.rs b/plonky2/src/gadgets/nonnative.rs
index 73bc0ad3..6c483a86 100644
--- a/plonky2/src/gadgets/nonnative.rs
+++ b/plonky2/src/gadgets/nonnative.rs
@@ -338,6 +338,19 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
 
         result
     }
+
+    pub fn nonnative_conditional_neg<FF: PrimeField>(
+        &mut self,
+        x: &NonNativeTarget<FF>,
+        b: BoolTarget,
+    ) -> NonNativeTarget<FF> {
+        let not_b = self.not(b);
+        let neg = self.neg_nonnative(x);
+        let x_if_true = self.mul_nonnative_by_bool(&neg, b);
+        let x_if_false = self.mul_nonnative_by_bool(x, not_b);
+
+        self.add_nonnative(&x_if_true, &x_if_false)
+    }
 }
 
 #[derive(Debug)]