From 6fde74224ee0a1fd1d0bf19bac09c48d37ebaec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= <4142+huitseeker@users.noreply.github.com> Date: Fri, 2 Feb 2024 09:45:21 -0500 Subject: [PATCH] Refactor MSM instances in the IPA (#290) * refactor: remove unneeded MSM from IPA - Added new trait usage and extended `AffineExt` with more bounds in `src/provider/traits.rs` to enable greater functionality. - new bounds on AffineExt are trivially satisfied and parrot existing ones, they are repeated to work around the lack of associated_type_bounds - Optimized vector processing in the `fold` method of `src/provider/pedersen.rs`. Element-wise vector addition replaced the previous multiscalar multiplication method. * chore: remove unneeded trait bounds * chore: some simplifications * refactor: Optimize commitment key handling and IPA - Transitioned variable ck and ck_c to mutable to facilitate in-place operations. - Optimized the `split_at` function in `Pedersen.rs` using the `split_off` method. - Reconfigured the `combine` function to clone and chain iterables for expedited vector creation. - Reassessed the `scale` function to in-place operations, eliminating the need for new struct instances. - Streamlined code in `scale` and `fold` functions by removing superfluous variable assignments. * refactor: test going back to MSM in fold --- src/provider/hyperkzg.rs | 5 ++- src/provider/ipa_pc.rs | 40 +++++++---------- src/provider/pedersen.rs | 97 +++++++++++++++------------------------- src/provider/traits.rs | 13 +++++- src/traits/evaluation.rs | 5 +-- 5 files changed, 70 insertions(+), 90 deletions(-) diff --git a/src/provider/hyperkzg.rs b/src/provider/hyperkzg.rs index 36367a70..7fbbda9c 100644 --- a/src/provider/hyperkzg.rs +++ b/src/provider/hyperkzg.rs @@ -202,7 +202,10 @@ where let B = kzg_compute_batch_polynomial(f, q); // Now open B at u0, ..., u_{t-1} - let w = u.par_iter().map(|ui| kzg_open(&B, *ui)).collect::>(); + let w = u + .into_par_iter() + .map(|ui| kzg_open(&B, *ui)) + .collect::>(); // The prover computes the challenge to keep the transcript in the same // state as that of the verifier diff --git a/src/provider/ipa_pc.rs b/src/provider/ipa_pc.rs index 7071e4c7..ea412eb7 100644 --- a/src/provider/ipa_pc.rs +++ b/src/provider/ipa_pc.rs @@ -9,7 +9,7 @@ use crate::{ evaluation::EvaluationEngineTrait, Engine, TranscriptEngineTrait, TranscriptReprTrait, }, - Commitment, CommitmentKey, CompressedCommitment, CE, + zip_with, Commitment, CommitmentKey, CompressedCommitment, CE, }; use core::iter; use ff::Field; @@ -19,13 +19,13 @@ use std::marker::PhantomData; use std::sync::Arc; /// Provides an implementation of the prover key -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct ProverKey { ck_s: CommitmentKey, } /// Provides an implementation of the verifier key -#[derive(Clone, Debug, Serialize)] +#[derive(Debug, Serialize)] #[serde(bound = "")] pub struct VerifierKey { ck_v: Arc>, @@ -76,7 +76,7 @@ where let u = InnerProductInstance::new(comm, &EqPolynomial::evals_from_points(point), eval); let w = InnerProductWitness::new(poly); - InnerProductArgument::prove(ck, &pk.ck_s, &u, &w, transcript) + InnerProductArgument::prove(ck.clone(), pk.ck_s.clone(), &u, &w, transcript) } /// A method to verify purported evaluations of a batch of polynomials @@ -90,24 +90,14 @@ where ) -> Result<(), NovaError> { let u = InnerProductInstance::new(comm, &EqPolynomial::evals_from_points(point), eval); - arg.verify( - &vk.ck_v, - &vk.ck_s, - (2_usize).pow(point.len() as u32), - &u, - transcript, - )?; + arg.verify(&vk.ck_v, vk.ck_s.clone(), 1 << point.len(), &u, transcript)?; Ok(()) } } fn inner_product(a: &[T], b: &[T]) -> T { - assert_eq!(a.len(), b.len()); - (0..a.len()) - .into_par_iter() - .map(|i| a[i] * b[i]) - .reduce(|| T::ZERO, |x, y| x + y) + zip_with!(par_iter, (a, b), |x, y| *x * y).sum() } /// An inner product instance consists of a commitment to a vector `a` and another vector `b` @@ -175,8 +165,8 @@ where } fn prove( - ck: &CommitmentKey, - ck_c: &CommitmentKey, + ck: CommitmentKey, + mut ck_c: CommitmentKey, U: &InnerProductInstance, W: &InnerProductWitness, transcript: &mut E::TE, @@ -194,12 +184,12 @@ where // sample a random base for commiting to the inner product let r = transcript.squeeze(b"r")?; - let ck_c = ck_c.scale(&r); + ck_c.scale(&r); // a closure that executes a step of the recursive inner product argument let prove_inner = |a_vec: &[E::Scalar], b_vec: &[E::Scalar], - ck: &CommitmentKey, + ck: CommitmentKey, transcript: &mut E::TE| -> Result< ( @@ -255,7 +245,7 @@ where .map(|(b_L, b_R)| *b_L * r_inverse + r * *b_R) .collect::>(); - let ck_folded = ck.fold(&r_inverse, &r); + let ck_folded = CommitmentKeyExtTrait::fold(&ck_L, &ck_R, &r_inverse, &r); Ok((L, R, a_vec_folded, b_vec_folded, ck_folded)) }; @@ -270,7 +260,7 @@ where let mut ck = ck; for _i in 0..usize::try_from(U.b_vec.len().ilog2()).unwrap() { let (L, R, a_vec_folded, b_vec_folded, ck_folded) = - prove_inner(&a_vec, &b_vec, &ck, transcript)?; + prove_inner(&a_vec, &b_vec, ck, transcript)?; L_vec.push(L); R_vec.push(R); @@ -289,12 +279,12 @@ where fn verify( &self, ck: &CommitmentKey, - ck_c: &CommitmentKey, + mut ck_c: CommitmentKey, n: usize, U: &InnerProductInstance, transcript: &mut E::TE, ) -> Result<(), NovaError> { - let (ck, _) = ck.split_at(U.b_vec.len()); + let (ck, _) = ck.clone().split_at(U.b_vec.len()); transcript.dom_sep(Self::protocol_name()); if U.b_vec.len() != n @@ -310,7 +300,7 @@ where // sample a random base for commiting to the inner product let r = transcript.squeeze(b"r")?; - let ck_c = ck_c.scale(&r); + ck_c.scale(&r); let P = U.comm_a_vec + CE::::commit(&ck_c, &[U.c]); diff --git a/src/provider/pedersen.rs b/src/provider/pedersen.rs index 6ee5d353..c5b73d82 100644 --- a/src/provider/pedersen.rs +++ b/src/provider/pedersen.rs @@ -6,6 +6,7 @@ use crate::{ commitment::{CommitmentEngineTrait, CommitmentTrait, Len}, AbsorbInROTrait, Engine, ROTrait, TranscriptReprTrait, }, + zip_with, }; use abomonation_derive::Abomonation; use core::{ @@ -14,12 +15,15 @@ use core::{ ops::{Add, Mul, MulAssign}, }; use ff::Field; -use group::{prime::PrimeCurve, Curve, Group, GroupEncoding}; +use group::{ + prime::{PrimeCurve, PrimeCurveAffine}, + Curve, Group, GroupEncoding, +}; use rayon::prelude::*; use serde::{Deserialize, Serialize}; /// A type that holds commitment generators -#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Abomonation)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Abomonation)] #[abomonation_omit_bounds] pub struct CommitmentKey where @@ -30,19 +34,6 @@ where ck: Vec<::Affine>, } -/// [`CommitmentKey`]s are often large, and this helps with cloning bottlenecks -impl Clone for CommitmentKey -where - E: Engine, - E::GE: DlogGroup, -{ - fn clone(&self) -> Self { - Self { - ck: self.ck[..].par_iter().cloned().collect(), - } - } -} - impl Len for CommitmentKey where E: Engine, @@ -243,7 +234,7 @@ where E::GE: DlogGroup, { /// Splits the commitment key into two pieces at a specified point - fn split_at(&self, n: usize) -> (Self, Self) + fn split_at(self, n: usize) -> (Self, Self) where Self: Sized; @@ -251,10 +242,10 @@ where fn combine(&self, other: &Self) -> Self; /// Folds the two commitment keys into one using the provided weights - fn fold(&self, w1: &E::Scalar, w2: &E::Scalar) -> Self; + fn fold(L: &Self, R: &Self, w1: &E::Scalar, w2: &E::Scalar) -> Self; /// Scales the commitment key using the provided scalar - fn scale(&self, r: &E::Scalar) -> Self; + fn scale(&mut self, r: &E::Scalar); /// Reinterprets commitments as commitment keys fn reinterpret_commitments_as_ck( @@ -269,64 +260,50 @@ where E: Engine>, E::GE: DlogGroup, { - fn split_at(&self, n: usize) -> (Self, Self) { - ( - Self { - ck: self.ck[0..n].to_vec(), - }, - Self { - ck: self.ck[n..].to_vec(), - }, - ) + fn split_at(mut self, n: usize) -> (Self, Self) { + let right = self.ck.split_off(n); + (self, Self { ck: right }) } fn combine(&self, other: &Self) -> Self { let ck = { - let mut c = self.ck.clone(); - c.extend(other.ck.clone()); - c + self + .ck + .iter() + .cloned() + .chain(other.ck.iter().cloned()) + .collect::>() }; Self { ck } } // combines the left and right halves of `self` using `w1` and `w2` as the weights - fn fold(&self, w1: &E::Scalar, w2: &E::Scalar) -> Self { - let w = vec![*w1, *w2]; - let (L, R) = self.split_at(self.ck.len() / 2); - - let ck = (0..self.ck.len() / 2) - .into_par_iter() - .map(|i| { - let bases = [L.ck[i].clone(), R.ck[i].clone()].to_vec(); - E::GE::vartime_multiscalar_mul(&w, &bases).to_affine() - }) - .collect(); - - Self { ck } + fn fold(L: &Self, R: &Self, w1: &E::Scalar, w2: &E::Scalar) -> Self { + debug_assert!(L.ck.len() == R.ck.len()); + let ck_curve: Vec = zip_with!(par_iter, (L.ck, R.ck), |l, r| { + E::GE::vartime_multiscalar_mul(&[*w1, *w2], &[*l, *r]) + }) + .collect(); + let mut ck_affine = vec![::Affine::identity(); L.ck.len()]; + E::GE::batch_normalize(&ck_curve, &mut ck_affine); + + Self { ck: ck_affine } } /// Scales each element in `self` by `r` - fn scale(&self, r: &E::Scalar) -> Self { - let ck_scaled = self - .ck - .clone() - .into_par_iter() - .map(|g| E::GE::vartime_multiscalar_mul(&[*r], &[g]).to_affine()) - .collect(); - - Self { ck: ck_scaled } + fn scale(&mut self, r: &E::Scalar) { + let ck_scaled: Vec = self.ck.par_iter().map(|g| *g * r).collect(); + E::GE::batch_normalize(&ck_scaled, &mut self.ck); } /// reinterprets a vector of commitments as a set of generators fn reinterpret_commitments_as_ck(c: &[CompressedCommitment]) -> Result { - let d = (0..c.len()) - .into_par_iter() - .map(|i| Commitment::::decompress(&c[i])) - .collect::>, NovaError>>()?; - let ck = (0..d.len()) - .into_par_iter() - .map(|i| d[i].comm.to_affine()) - .collect(); + let d = c + .par_iter() + .map(|c| Commitment::::decompress(c).map(|c| c.comm)) + .collect::, NovaError>>()?; + let mut ck = vec![::Affine::identity(); d.len()]; + E::GE::batch_normalize(&d, &mut ck); Ok(Self { ck }) } } diff --git a/src/provider/traits.rs b/src/provider/traits.rs index 5a1d83f5..5d52ee42 100644 --- a/src/provider/traits.rs +++ b/src/provider/traits.rs @@ -1,7 +1,9 @@ use crate::traits::{Group, TranscriptReprTrait}; +use group::prime::PrimeCurveAffine; use group::{prime::PrimeCurve, GroupEncoding}; use serde::{Deserialize, Serialize}; use std::fmt::Debug; +use std::ops::Mul; /// A trait that defines extensions to the Group trait pub trait DlogGroup: @@ -11,7 +13,16 @@ pub trait DlogGroup: + PrimeCurve::ScalarExt, Affine = ::AffineExt> { type ScalarExt; - type AffineExt: Clone + Debug + Eq + Serialize + for<'de> Deserialize<'de> + Sync + Send; + type AffineExt: Clone + + Debug + + Eq + + Serialize + + for<'de> Deserialize<'de> + + Sync + + Send + // technical bounds, should disappear when associated_type_bounds stabilizes + + Mul + + PrimeCurveAffine; type Compressed: Clone + Debug + Eq diff --git a/src/traits/evaluation.rs b/src/traits/evaluation.rs index b5b85c0e..28a7fdb8 100644 --- a/src/traits/evaluation.rs +++ b/src/traits/evaluation.rs @@ -12,11 +12,10 @@ use serde::{Deserialize, Serialize}; /// A trait that ties different pieces of the commitment evaluation together pub trait EvaluationEngineTrait: Clone + Send + Sync { /// A type that holds the prover key - type ProverKey: Clone + Send + Sync; + type ProverKey: Send + Sync; /// A type that holds the verifier key - type VerifierKey: Clone - + Send + type VerifierKey: Send + Sync // required for easy Digest computation purposes, could be relaxed to // [`crate::digest::Digestible`]