Merge pull request #59 from zcash/optimise-polycommit

Documentation updates + clippy fixes

src/poly/commitment.rs

@@ -283,22 +283,18 @@ fn test_opening_proof() {
     let mut transcript = Transcript::init_with_hashers(&hasher, &scalar_hasher);
 
     loop {
-        let transcript_dup = transcript.clone();
+        let mut transcript_dup = transcript.clone();
 
         let opening_proof = Proof::create(&params, &mut transcript, &px, blind, x);
-        if opening_proof.is_err() {
+        if let Ok(opening_proof) = opening_proof {
-            transcript = transcript_dup;
-            transcript.absorb_base(Field::one());
-        } else {
-            let opening_proof = opening_proof.unwrap();
             // Verify the opening proof
             let mut commitment_msm = params.empty_msm();
-            commitment_msm.add_term(Field::one(), p);
+            commitment_msm.append_term(Field::one(), p);
             let guard = opening_proof
                 .verify(
                     &params,
                     params.empty_msm(),
-                    &mut transcript_dup.clone(),
+                    &mut transcript_dup,
                     x,
                     commitment_msm,
                     v,
@@ -315,33 +311,12 @@ fn test_opening_proof() {
                 let g = guard.compute_g();
                 let (msm_g, _accumulator) = guard.clone().use_g(g);
                 assert!(msm_g.eval());
+
+                break;
             }
-
+        } else {
-            // Check another proof to populate `msm.g_scalars`
+            transcript = transcript_dup;
-            let msm = guard.use_challenges();
+            transcript.absorb_base(Field::one());
-            let mut commitment_msm = params.empty_msm();
-            commitment_msm.add_term(Field::one(), p);
-            let guard = opening_proof
-                .verify(
-                    &params,
-                    msm,
-                    &mut transcript_dup.clone(),
-                    x,
-                    commitment_msm,
-                    v,
-                )
-                .unwrap();
-
-            // Test use_challenges()
-            let msm_challenges = guard.clone().use_challenges();
-            assert!(msm_challenges.eval());
-
-            // Test use_g()
-            let g = guard.compute_g();
-            let (msm_g, _accumulator) = guard.clone().use_g(g);
-            assert!(msm_g.eval());
-
-            break;
         }
     }
 }

src/poly/commitment/msm.rs

@@ -1,5 +1,5 @@
 use super::Params;
-use crate::arithmetic::{best_multiexp, Curve, CurveAffine};
+use crate::arithmetic::{best_multiexp, parallelize, Curve, CurveAffine};
 
 /// A multiscalar multiplication in the polynomial commitment scheme
 #[derive(Debug, Clone)]
@@ -34,52 +34,58 @@ impl<'a, C: CurveAffine> MSM<'a, C> {
         self.other_bases.extend(other.other_bases.iter());
 
         if let Some(g_scalars) = &other.g_scalars {
-            self.add_to_g(&g_scalars);
+            self.add_to_g_scalars(&g_scalars);
         }
 
         if let Some(h_scalar) = &other.h_scalar {
-            self.add_to_h(*h_scalar);
+            self.add_to_h_scalar(*h_scalar);
         }
     }
 
     /// Add arbitrary term (the scalar and the point)
-    pub fn add_term(&mut self, scalar: C::Scalar, point: C) {
+    pub fn append_term(&mut self, scalar: C::Scalar, point: C) {
         self.other_scalars.push(scalar);
         self.other_bases.push(point);
     }
 
     /// Add a vector of scalars to `g_scalars`. This function will panic if the
     /// caller provides a slice of scalars that is not of length `params.n`.
-    // TODO: parallelize
+    pub fn add_to_g_scalars(&mut self, scalars: &[C::Scalar]) {
-    pub fn add_to_g(&mut self, scalars: &[C::Scalar]) {
         assert_eq!(scalars.len(), self.params.n as usize);
         if let Some(g_scalars) = &mut self.g_scalars {
-            for (g_scalar, scalar) in g_scalars.iter_mut().zip(scalars.iter()) {
+            parallelize(g_scalars, |g_scalars, start| {
-                *g_scalar += scalar;
+                for (g_scalar, scalar) in g_scalars.iter_mut().zip(scalars[start..].iter()) {
-            }
+                    *g_scalar += scalar;
+                }
+            })
         } else {
             self.g_scalars = Some(scalars.to_vec());
         }
     }
 
-    /// Add term to h
+    /// Add to `h_scalar`
-    pub fn add_to_h(&mut self, scalar: C::Scalar) {
+    pub fn add_to_h_scalar(&mut self, scalar: C::Scalar) {
         self.h_scalar = self.h_scalar.map_or(Some(scalar), |a| Some(a + &scalar));
     }
 
     /// Scale all scalars in the MSM by some scaling factor
-    // TODO: parallelize
     pub fn scale(&mut self, factor: C::Scalar) {
         if let Some(g_scalars) = &mut self.g_scalars {
-            for g_scalar in g_scalars.iter_mut() {
+            parallelize(g_scalars, |g_scalars, _| {
-                *g_scalar *= &factor;
+                for g_scalar in g_scalars {
-            }
+                    *g_scalar *= &factor;
+                }
+            })
         }
 
-        // TODO: parallelize
+        if !self.other_scalars.is_empty() {
-        for other_scalar in self.other_scalars.iter_mut() {
+            parallelize(&mut self.other_scalars, |other_scalars, _| {
-            *other_scalar *= &factor;
+                for other_scalar in other_scalars {
+                    *other_scalar *= &factor;
+                }
+            })
         }
+
         self.h_scalar = self.h_scalar.map(|a| a * &factor);
     }
 

src/poly/commitment/verifier.rs

@@ -32,16 +32,16 @@ impl<'a, C: CurveAffine> Guard<'a, C> {
     /// scalars and points.
     pub fn use_challenges(mut self) -> MSM<'a, C> {
         let s = compute_s(&self.challenges_sq, self.allinv * &self.neg_z1);
-        self.msm.add_to_g(&s);
+        self.msm.add_to_g_scalars(&s);
-        self.msm.add_to_h(self.neg_z1);
+        self.msm.add_to_h_scalar(self.neg_z1);
 
         self.msm
     }
 
-    /// Lets caller supply the purported G point and simply appends it to
+    /// Lets caller supply the purported G point and simply appends
-    /// return an updated MSM.
+    /// [-z1] G to return an updated MSM.
     pub fn use_g(mut self, g: C) -> (MSM<'a, C>, Accumulator<C>) {
-        self.msm.add_term(self.neg_z1, g);
+        self.msm.append_term(self.neg_z1, g);
 
         let accumulator = Accumulator {
             g,
@@ -51,7 +51,7 @@ impl<'a, C: CurveAffine> Guard<'a, C> {
         (self.msm, accumulator)
     }
 
-    /// Computes the g value when given a potential scalar as input.
+    /// Computes G + H, where G = ⟨s, params.g⟩ and H is used for blinding
     pub fn compute_g(&self) -> C {
         let s = compute_s(&self.challenges_sq, self.allinv);
 
@@ -159,9 +159,11 @@ impl<C: CurveAffine> Proof<C> {
         let c_packed = transcript.squeeze().get_lower_128();
         let c: C::Scalar = get_challenge_scalar(Challenge(c_packed));
 
-        // Check
+        // Construct
-        // [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 - [z1 - z2] H
+        // [c] P + [c * v] U + [c] sum(L_i * u_i^2) + [c] sum(R_i * u_i^-2) + delta - [z1 * b] U + [z1 - z2] H
-        // = 0
+        // = [z1] (G + H)
+        // The computation of [z1] (G + H) happens in either Guard::use_challenges()
+        // or Guard::use_g().
 
         let b = compute_b(x, &challenges, &challenges_inv);
 
@@ -171,22 +173,23 @@ impl<C: CurveAffine> Proof<C> {
         commitment_msm.scale(c);
         msm.add_msm(&commitment_msm);
 
+        // [c] sum(L_i * u_i^2) + [c] sum(R_i * u_i^-2)
         for scalar in &mut extra_scalars {
             *scalar *= &c;
         }
 
         for (scalar, base) in extra_scalars.iter().zip(extra_bases.iter()) {
-            msm.add_term(*scalar, *base);
+            msm.append_term(*scalar, *base);
         }
 
         // [c * v] U - [z1 * b] U
-        msm.add_term((c * &v) + &(neg_z1 * &b), u);
+        msm.append_term((c * &v) + &(neg_z1 * &b), u);
 
         // delta
-        msm.add_term(Field::one(), self.delta);
+        msm.append_term(Field::one(), self.delta);
 
-        // - [z1 - z2] H
+        // + [z1 - z2] H
-        msm.add_to_h(self.z1 - &self.z2);
+        msm.add_to_h_scalar(self.z1 - &self.z2);
 
         let guard = Guard {
             msm,

src/poly/multiopen/verifier.rs

@@ -54,7 +54,7 @@ impl<C: CurveAffine> Proof<C> {
         {
             let mut accumulate = |set_idx: usize, new_commitment, evals: Vec<C::Scalar>| {
                 q_commitments[set_idx].scale(x_4);
-                q_commitments[set_idx].add_term(C::Scalar::one(), new_commitment);
+                q_commitments[set_idx].append_term(C::Scalar::one(), new_commitment);
                 for (eval, set_eval) in evals.iter().zip(q_eval_sets[set_idx].iter_mut()) {
                     *set_eval *= &x_4;
                     *set_eval += eval;
@@ -109,7 +109,7 @@ impl<C: CurveAffine> Proof<C> {
 
         // Compute the final commitment that has to be opened
         let mut commitment_msm = params.empty_msm();
-        commitment_msm.add_term(C::Scalar::one(), self.f_commitment);
+        commitment_msm.append_term(C::Scalar::one(), self.f_commitment);
         let (commitment_msm, msm_eval) = q_commitments.into_iter().zip(self.q_evals.iter()).fold(
             (commitment_msm, msm_eval),
             |(mut commitment_msm, msm_eval), (q_commitment, q_eval)| {

src/tweedle/fields/fp.rs

@@ -658,11 +658,11 @@ fn test_zeta() {
     );
 
     let a = Fp::ZETA;
-    assert!(bool::from(a != Fp::one()));
+    assert!(a != Fp::one());
     let b = a * a;
-    assert!(bool::from(b != Fp::one()));
+    assert!(b != Fp::one());
     let c = b * a;
-    assert!(bool::from(c == Fp::one()));
+    assert!(c == Fp::one());
 }
 
 #[test]

src/tweedle/fields/fq.rs

@@ -672,11 +672,11 @@ fn test_zeta() {
         "0x36c66d3a1e049a5887ad8b5ff9731ffe69cf8de720e52ec14394c2bd148fa4fd"
     );
     let a = Fq::ZETA;
-    assert!(bool::from(a != Fq::one()));
+    assert!(a != Fq::one());
     let b = a * a;
-    assert!(bool::from(b != Fq::one()));
+    assert!(b != Fq::one());
     let c = b * a;
-    assert!(bool::from(c == Fq::one()));
+    assert!(c == Fq::one());
 }
 
 #[test]