Address review comments

src/plonk/verifier.rs

@@ -264,7 +264,7 @@ impl<C: CurveAffine> Proof<C> {
         }
 
         // Verify the opening proof
-        let default_msm = MSM::default(&params);
+        let default_msm = params.msm();
         let guard = self
             .opening
             .verify(
@@ -277,8 +277,8 @@ impl<C: CurveAffine> Proof<C> {
             )
             .unwrap();
 
-        let msm: &MSM<C> = &guard.use_challenges(params).unwrap();
+        let msm: &MSM<C> = &guard.use_challenges();
 
-        msm.is_zero(params)
+        msm.is_zero()
     }
 }

src/poly/commitment.rs

@@ -3,7 +3,7 @@
 //!
 //! [halo]: https://eprint.iacr.org/2019/1021
 
-use super::{Coeff, Error, LagrangeCoeff, Polynomial};
+use super::{Coeff, LagrangeCoeff, Polynomial};
 use crate::arithmetic::{
     best_fft, best_multiexp, parallelize, Challenge, Curve, CurveAffine, Field,
 };
@@ -25,36 +25,15 @@ pub struct OpeningProof<C: CurveAffine> {
 
 /// A multiscalar multiplication in the polynomial commitment scheme
 #[derive(Debug)]
-pub struct MSM<C: CurveAffine> {
+pub struct MSM<'a, C: CurveAffine> {
-    /// TODO: documentation
+    params: &'a Params<C>,
-    pub g_scalars: Option<Vec<C::Scalar>>,
+    g_scalars: Option<Vec<C::Scalar>>,
-
+    h_scalar: Option<C::Scalar>,
-    /// TODO: documentation
+    other_scalars: Vec<C::Scalar>,
-    pub h_scalar: Option<C::Scalar>,
+    other_bases: Vec<C>,
-
-    /// TODO: documentation
-    pub other_scalars: Vec<C::Scalar>,
-
-    /// TODO: documentation
-    pub other_bases: Vec<C>,
 }
 
-impl<'a, C: CurveAffine> MSM<C> {
+impl<'a, C: CurveAffine> MSM<'a, C> {
-    /// Empty MSM
-    pub fn default(params: &Params<C>) -> Self {
-        let g_scalars = Some(vec![C::Scalar::one(); params.n as usize]);
-        let h_scalar = Some(C::Scalar::one());
-        let other_scalars: Vec<C::Scalar> = Vec::with_capacity(params.k as usize * 2 + 3);
-        let other_bases: Vec<C> = Vec::with_capacity(params.k as usize * 2 + 3);
-
-        MSM {
-            g_scalars,
-            h_scalar,
-            other_scalars,
-            other_bases,
-        }
-    }
-
     /// Add arbitrary term (the scalar and the point)
     pub fn add_term(&mut self, scalar: C::Scalar, point: C) {
         &self.other_scalars.push(scalar);
@@ -62,50 +41,54 @@ impl<'a, C: CurveAffine> MSM<C> {
     }
 
     /// Add a vector of scalars to `g_scalars`
-    pub fn add_to_g(&mut self, scalars: Vec<C::Scalar>) {
+    pub fn add_to_g(&mut self, scalars: &[C::Scalar]) {
-        for (g_scalar, scalar) in self
+        if let Some(g_scalars) = &mut self.g_scalars {
-            .g_scalars
+            for (g_scalar, scalar) in g_scalars.iter_mut().zip(scalars.iter()) {
-            .as_mut()
+                *g_scalar += &scalar;
-            .unwrap()
+            }
-            .iter_mut()
+        } else {
-            .zip(scalars.iter())
+            self.g_scalars = Some(scalars.to_vec());
-        {
-            *g_scalar += &scalar;
         }
     }
 
     /// Add term to h
     pub fn add_to_h(&mut self, scalar: C::Scalar) {
-        self.h_scalar = Some(self.h_scalar.unwrap() + &scalar);
+        self.h_scalar = self.h_scalar.map_or(Some(scalar), |a| Some(a + &scalar));
     }
 
     /// Scale all scalars in the MSM by a random blinding factor
     pub fn scale(&mut self, factor: C::Scalar) {
-        for g_scalar in self.g_scalars.as_mut().unwrap().iter_mut() {
+        if let Some(g_scalars) = &mut self.g_scalars {
-            *g_scalar *= &factor;
+            for g_scalar in g_scalars.iter_mut() {
+                *g_scalar *= &factor;
+            }
         }
+
         for other_scalar in self.other_scalars.iter_mut() {
             *other_scalar *= &factor;
         }
-        self.h_scalar = Some(self.h_scalar.unwrap() * &factor);
+        self.h_scalar = self.h_scalar.map(|a| a * &factor);
     }
 
     /// Perform multiexp and check that it results in zero
-    pub fn is_zero(&self, params: &'a Params<C>) -> bool {
+    pub fn is_zero(&self) -> bool {
-        let mut scalars: Vec<C::Scalar> = vec![];
+        let len = self.g_scalars.as_ref().map(|v| v.len()).unwrap_or(0)
-        let mut bases: Vec<C> = vec![];
+            + self.h_scalar.map(|_| 1).unwrap_or(0)
+            + self.other_scalars.len();
+        let mut scalars: Vec<C::Scalar> = Vec::with_capacity(len);
+        let mut bases: Vec<C> = Vec::with_capacity(len);
 
         scalars.extend(&self.other_scalars);
         bases.extend(&self.other_bases);
 
         if let Some(h_scalar) = self.h_scalar {
             scalars.push(h_scalar);
-            bases.push(params.h);
+            bases.push(self.params.h);
         }
 
         if let Some(g_scalars) = &self.g_scalars {
             scalars.extend(g_scalars);
-            bases.extend(params.g.iter());
+            bases.extend(self.params.g.iter());
         }
 
         bool::from(best_multiexp(&scalars, &bases).is_zero())
@@ -243,51 +226,52 @@ impl<C: CurveAffine> Params<C> {
 
         best_multiexp::<C>(&tmp_scalars, &tmp_bases)
     }
+
+    /// Generates an empty multiscalar multiplication struct using the
+    /// appropriate params.
+    pub fn msm(&self) -> MSM<C> {
+        let g_scalars = None;
+        let h_scalar = None;
+        let other_scalars = vec![];
+        let other_bases = vec![];
+
+        MSM {
+            params: &self,
+            g_scalars,
+            h_scalar,
+            other_scalars,
+            other_bases,
+        }
+    }
 }
 
 /// A guard returned by the verifier
 #[derive(Debug)]
-pub struct Guard<C: CurveAffine> {
+pub struct Guard<'a, C: CurveAffine> {
-    msm: MSM<C>,
+    msm: MSM<'a, C>,
     neg_z1: C::Scalar,
     allinv: C::Scalar,
     challenges_sq: Vec<C::Scalar>,
     challenges_sq_packed: Vec<Challenge>,
 }
 
-impl<C: CurveAffine> Guard<C> {
+impl<'a, C: CurveAffine> Guard<'a, C> {
     /// Lets caller supply the challenges and obtain an MSM with updated
     /// scalars and points.
-    pub fn use_challenges(mut self, params: &Params<C>) -> Result<MSM<C>, Error> {
+    pub fn use_challenges(mut self) -> MSM<'a, C> {
-        let mut scalars: Vec<C::Scalar> = vec![];
-        let mut bases: Vec<C> = vec![];
-
-        scalars.extend(&self.msm.other_scalars);
-        bases.extend(&self.msm.other_bases);
-
-        // - [z2] H
-        if let Some(h_scalar) = self.msm.h_scalar {
-            scalars.push(h_scalar);
-            bases.push(params.h);
-        }
-
-        // - [z1] G
         let s = compute_s(&self.challenges_sq, self.allinv * &self.neg_z1);
-        scalars.extend(&s);
+        self.msm.add_to_g(&s);
-        bases.extend(&params.g);
 
-        self.msm.g_scalars = Some(s);
+        self.msm
-
-        Ok(self.msm)
     }
 
     /// Lets caller supply the purported G point and simply appends it to
     /// return an updated MSM.
-    pub fn use_g(mut self, g: C) -> Result<MSM<C>, Error> {
+    pub fn use_g(mut self, g: C) -> MSM<'a, C> {
         &self.msm.other_scalars.push(self.neg_z1);
         &self.msm.other_bases.push(g);
 
-        Ok(self.msm)
+        self.msm
     }
 }
 
@@ -411,14 +395,14 @@ fn test_opening_proof() {
         } else {
             let opening_proof = opening_proof.unwrap();
             // Verify the opening proof
-            let msm = MSM::default(&params);
+            let msm = params.msm();
             let guard = opening_proof
                 .verify(&params, msm, &mut transcript_dup, x, &p, v)
                 .unwrap();
 
-            let msm = guard.use_challenges(&params).unwrap();
+            let msm = guard.use_challenges();
 
-            assert!(msm.is_zero(&params));
+            assert!(msm.is_zero());
             break;
         }
     }

src/poly/commitment/verifier.rs

@@ -11,12 +11,12 @@ impl<C: CurveAffine> OpeningProof<C> {
     pub fn verify<'a, H: Hasher<C::Base>>(
         &self,
         params: &'a Params<C>,
-        mut msm: MSM<C>,
+        mut msm: MSM<'a, C>,
         transcript: &mut H,
         x: C::Scalar,
         p: &C,
         v: C::Scalar,
-    ) -> Result<Guard<C>, Error> {
+    ) -> Result<Guard<'a, C>, Error> {
         // Check for well-formedness
         if self.rounds.len() != params.k as usize {
             return Err(Error::OpeningError);
@@ -38,6 +38,9 @@ impl<C: CurveAffine> OpeningProof<C> {
             C::from_xy(u_x, u_y).unwrap()
         };
 
+        let mut extra_scalars = Vec::with_capacity(self.rounds.len() * 2 + 4 + params.n as usize);
+        let mut extra_bases = Vec::with_capacity(self.rounds.len() * 2 + 4 + params.n as usize);
+
         // Data about the challenges from each of the rounds.
         let mut challenges = Vec::with_capacity(self.rounds.len());
         let mut challenges_inv = Vec::with_capacity(self.rounds.len());
@@ -79,10 +82,10 @@ impl<C: CurveAffine> OpeningProof<C> {
 
             let challenge_sq_inv = challenge_inv.square();
 
-            msm.other_scalars.push(challenge_sq);
+            extra_scalars.push(challenge_sq);
-            msm.other_bases.push(round.0);
+            extra_bases.push(round.0);
-            msm.other_scalars.push(challenge_sq_inv);
+            extra_scalars.push(challenge_sq_inv);
-            msm.other_bases.push(round.1);
+            extra_bases.push(round.1);
 
             challenges.push(challenge);
             challenges_inv.push(challenge_inv);
@@ -108,28 +111,35 @@ impl<C: CurveAffine> OpeningProof<C> {
         // [c] P + [c * v] U + [c] sum(L_i * u_i^2) + [c] sum(R_i * u_i^-2) + delta - [z1] G - [z1 * b] U - [z2] H
         // = 0
 
-        for scalar in &mut msm.other_scalars {
+        // Scale the MSM by a random factor to ensure that if the existing MSM
+        // has is_zero() == false then this argument won't be able to interfere
+        // with it to make it true. It's a way of keeping the MSM's linearly
+        // independent.
+        msm.scale(C::Scalar::random());
+
+        for scalar in &mut extra_scalars {
             *scalar *= &c;
         }
 
+        for (scalar, base) in extra_scalars.iter().zip(extra_bases.iter()) {
+            msm.add_term(*scalar, *base);
+        }
+
         let b = compute_b(x, &challenges, &challenges_inv);
 
         let neg_z1 = -self.z1;
 
         // [c] P
-        msm.other_bases.push(*p);
+        msm.add_term(c, *p);
-        msm.other_scalars.push(c);
 
         // [c * v] U - [z1 * b] U
-        msm.other_bases.push(u);
+        msm.add_term((c * &v) + &(neg_z1 * &b), u);
-        msm.other_scalars.push((c * &v) + &(neg_z1 * &b));
 
         // delta
-        msm.other_bases.push(self.delta);
+        msm.add_term(Field::one(), self.delta);
-        msm.other_scalars.push(Field::one());
 
         // z2
-        msm.h_scalar = Some(-self.z2);
+        msm.add_to_h(-self.z2);
 
         let guard = Guard {
             msm,