From 18e341ff18becda2f6fe942bce664ccf1264d37b Mon Sep 17 00:00:00 2001
From: wborgeaud <williamborgeaud@gmail.com>
Date: Thu, 3 Mar 2022 08:06:21 +0100
Subject: [PATCH] Comments

---
 plonky2/src/gadgets/curve_fixed_base.rs | 16 ++++++++++------
 plonky2/src/gadgets/curve_msm.rs        |  6 +++++-
 plonky2/src/gadgets/glv.rs              |  1 -
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/plonky2/src/gadgets/curve_fixed_base.rs b/plonky2/src/gadgets/curve_fixed_base.rs
index 0cbd7a9e..3c470044 100644
--- a/plonky2/src/gadgets/curve_fixed_base.rs
+++ b/plonky2/src/gadgets/curve_fixed_base.rs
@@ -12,13 +12,13 @@ use crate::plonk::config::{GenericHashOut, Hasher};
 
 impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
     /// Do windowed fixed-base scalar multiplication, using a 4-bit window.
-    // TODO: Benchmark other window sizes.
     pub fn fixed_base_curve_mul<C: Curve>(
         &mut self,
         base: AffinePoint<C>,
         scalar: &NonNativeTarget<C::ScalarField>,
     ) -> AffinePointTarget<C> {
-        let doubled_base = (0..scalar.value.limbs.len() * 8).scan(base, |acc, _| {
+        // Holds `(16^i) * base` for `i=0..scalar.value.limbs.len() * 8`.
+        let scaled_base = (0..scalar.value.limbs.len() * 8).scan(base, |acc, _| {
             let tmp = *acc;
             for _ in 0..4 {
                 *acc = acc.double();
@@ -26,17 +26,20 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
             Some(tmp)
         });
 
-        let bits = self.split_nonnative_to_4_bit_limbs(scalar);
+        let limbs = self.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(
             &GenericHashOut::<F>::to_bytes(&hash_0),
         ));
         let rando = (CurveScalar(hash_0_scalar) * C::GENERATOR_PROJECTIVE).to_affine();
+
         let zero = self.zero();
         let mut result = self.constant_affine_point(rando);
-        for (limb, point) in bits.into_iter().zip(doubled_base) {
-            let mul_point = (0..16)
+        // `s * P = sum s_i * P_i` with `P_i = (16^i) * P` and `s = sum s_i * (16^i)`.
+        for (limb, point) in limbs.into_iter().zip(scaled_base) {
+            // Holds `t * P_i` for `p=0..16`.
+            let muls_point = (0..16)
                 .scan(AffinePoint::ZERO, |acc, _| {
                     let tmp = *acc;
                     *acc = (point + *acc).to_affine();
@@ -46,7 +49,8 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
                 .collect::<Vec<_>>();
             let is_zero = self.is_equal(limb, zero);
             let should_add = self.not(is_zero);
-            let r = self.random_access_curve_points(limb, mul_point);
+            // `r = s_i * P_i`
+            let r = self.random_access_curve_points(limb, muls_point);
             result = self.curve_conditional_add(&result, &r, should_add);
         }
 
diff --git a/plonky2/src/gadgets/curve_msm.rs b/plonky2/src/gadgets/curve_msm.rs
index df13e8f3..5d505c4d 100644
--- a/plonky2/src/gadgets/curve_msm.rs
+++ b/plonky2/src/gadgets/curve_msm.rs
@@ -11,7 +11,10 @@ use crate::plonk::circuit_builder::CircuitBuilder;
 use crate::plonk::config::{GenericHashOut, Hasher};
 
 impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
-    /// Computes `n*p + m*q`.
+    /// Computes `n*p + m*q` using windowed MSM, with a 2-bit window.
+    /// See Algorithm 9.23 in Handbook of Elliptic and Hyperelliptic Curve Cryptography for a
+    /// description.
+    /// Note: Doesn't work if `p == q`.
     pub fn curve_msm<C: Curve>(
         &mut self,
         p: &AffinePointTarget<C>,
@@ -32,6 +35,7 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         let rando_t = self.constant_affine_point(rando);
         let neg_rando = self.constant_affine_point(-rando);
 
+        // Precomputes `precomputation[i + 4*j] = i*p + j*q` for `i,j=0..4`.
         let mut precomputation = vec![p.clone(); 16];
         let mut cur_p = rando_t.clone();
         let mut cur_q = rando_t.clone();
diff --git a/plonky2/src/gadgets/glv.rs b/plonky2/src/gadgets/glv.rs
index f0c4704b..4bc3efd6 100644
--- a/plonky2/src/gadgets/glv.rs
+++ b/plonky2/src/gadgets/glv.rs
@@ -19,7 +19,6 @@ impl<F: RichField + Extendable<D>, const D: usize> CircuitBuilder<F, D> {
         self.constant_nonnative(GLV_BETA)
     }
 
-    // TODO: Add decomposition check.
     pub fn decompose_secp256k1_scalar(
         &mut self,
         k: &NonNativeTarget<Secp256K1Scalar>,