diff --git a/src/errors.rs b/src/errors.rs index 42fafba59..121704b9e 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -4,6 +4,7 @@ use thiserror::Error; /// Errors returned by Nova #[derive(Clone, Debug, Eq, PartialEq, Error)] +#[non_exhaustive] pub enum NovaError { /// returned if the supplied row or col in (row,col,val) tuple is out of range #[error("InvalidIndex")] @@ -29,6 +30,9 @@ pub enum NovaError { /// returned if proof verification fails #[error("ProofVerifyError")] ProofVerifyError, + /// returned if the provided commitment key is not of sufficient length + #[error("InvalidCommitmentKeyLength")] + InvalidCommitmentKeyLength, /// returned if the provided number of steps is zero #[error("InvalidNumSteps")] InvalidNumSteps, diff --git a/src/lib.rs b/src/lib.rs index 34b835f7c..ebd8bbbf9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -172,11 +172,12 @@ where /// /// let circuit1 = TrivialCircuit::<::Scalar>::default(); /// let circuit2 = TrivialCircuit::<::Scalar>::default(); - /// // Only relevant for a SNARK using computational commitments, pass &(|_| 0) otherwise. - /// let pp_hint1 = &*SPrime::::commitment_key_floor(); - /// let pp_hint2 = &*SPrime::::commitment_key_floor(); + /// // Only relevant for a SNARK using computational commitments, pass &(|_| 0) + /// // or &*nova_snark::traits::snark::default_commitment_key_hint() otherwise. + /// let ck_hint1 = &*SPrime::::commitment_key_floor(); + /// let ck_hint2 = &*SPrime::::commitment_key_floor(); /// - /// let pp = PublicParams::new(&circuit1, &circuit2, pp_hint1, pp_hint2); + /// let pp = PublicParams::new(&circuit1, &circuit2, ck_hint1, ck_hint2); /// ``` pub fn new( c_primary: &C1, @@ -542,24 +543,23 @@ where z0_secondary: &[G2::Scalar], ) -> Result<(Vec, Vec), NovaError> { // number of steps cannot be zero - if num_steps == 0 { - return Err(NovaError::ProofVerifyError); - } + let is_num_steps_zero = num_steps == 0; // check if the provided proof has executed num_steps - if self.i != num_steps { - return Err(NovaError::ProofVerifyError); - } + let is_num_steps_not_match = self.i != num_steps; // check if the initial inputs match - if self.z0_primary != z0_primary || self.z0_secondary != z0_secondary { - return Err(NovaError::ProofVerifyError); - } + let is_inputs_not_match = self.z0_primary != z0_primary || self.z0_secondary != z0_secondary; // check if the (relaxed) R1CS instances have two public outputs - if self.l_u_secondary.X.len() != 2 + let is_instance_has_two_outpus = self.l_u_secondary.X.len() != 2 || self.r_U_primary.X.len() != 2 - || self.r_U_secondary.X.len() != 2 + || self.r_U_secondary.X.len() != 2; + + if is_num_steps_zero + || is_num_steps_not_match + || is_inputs_not_match + || is_instance_has_two_outpus { return Err(NovaError::ProofVerifyError); } @@ -1033,9 +1033,9 @@ mod tests { ::Repr: Abomonation, { // this tests public parameters with a size specifically intended for a spark-compressed SNARK - let pp_hint1 = &*SPrime::::commitment_key_floor(); - let pp_hint2 = &*SPrime::::commitment_key_floor(); - let pp = PublicParams::::new(circuit1, circuit2, pp_hint1, pp_hint2); + let ck_hint1 = &*SPrime::::commitment_key_floor(); + let ck_hint2 = &*SPrime::::commitment_key_floor(); + let pp = PublicParams::::new(circuit1, circuit2, ck_hint1, ck_hint2); let digest_str = pp .digest() diff --git a/src/provider/ipa_pc.rs b/src/provider/ipa_pc.rs index b2dad42be..d4c731e7b 100644 --- a/src/provider/ipa_pc.rs +++ b/src/provider/ipa_pc.rs @@ -320,18 +320,17 @@ where acc *= v[i]; } - // we can compute an inversion only if acc is non-zero - if acc == G::Scalar::ZERO { - return Err(NovaError::InternalError); - } + // return error if acc is zero + acc = match Option::from(acc.invert()) { + Some(inv) => inv, + None => return Err(NovaError::InternalError), + }; // compute the inverse once for all entries - acc = acc.invert().unwrap(); - let mut inv = vec![G::Scalar::ZERO; v.len()]; - for i in 0..v.len() { - let tmp = acc * v[v.len() - 1 - i]; - inv[v.len() - 1 - i] = products[v.len() - 1 - i] * acc; + for i in (0..v.len()).rev() { + let tmp = acc * v[i]; + inv[i] = products[i] * acc; acc = tmp; } diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 17a892570..3387fe3a8 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -19,8 +19,6 @@ use rayon::{current_num_threads, prelude::*}; /// Native implementation of fast multiexp /// Adapted from zcash/halo2 fn cpu_multiexp_serial(coeffs: &[C::Scalar], bases: &[C]) -> C::Curve { - let coeffs: Vec<_> = coeffs.iter().map(|a| a.to_repr()).collect(); - let c = if bases.len() < 4 { 1 } else if bases.len() < 32 { @@ -50,64 +48,57 @@ fn cpu_multiexp_serial(coeffs: &[C::Scalar], bases: &[C]) -> C:: } let segments = (256 / c) + 1; - let mut acc = C::Curve::identity(); - for current_segment in (0..segments).rev() { - for _ in 0..c { - acc = acc.double(); - } + (0..segments) + .rev() + .fold(C::Curve::identity(), |mut acc, segment| { + (0..c).for_each(|_| acc = acc.double()); - #[derive(Clone, Copy)] - enum Bucket { - None, - Affine(C), - Projective(C::Curve), - } + #[derive(Clone, Copy)] + enum Bucket { + None, + Affine(C), + Projective(C::Curve), + } - impl Bucket { - fn add_assign(&mut self, other: &C) { - *self = match *self { - Bucket::None => Bucket::Affine(*other), - Bucket::Affine(a) => Bucket::Projective(a + *other), - Bucket::Projective(mut a) => { - a += *other; - Bucket::Projective(a) + impl Bucket { + fn add_assign(&mut self, other: &C) { + *self = match *self { + Bucket::None => Bucket::Affine(*other), + Bucket::Affine(a) => Bucket::Projective(a + *other), + Bucket::Projective(a) => Bucket::Projective(a + other), } } - } - fn add(self, mut other: C::Curve) -> C::Curve { - match self { - Bucket::None => other, - Bucket::Affine(a) => { - other += a; - other + fn add(self, other: C::Curve) -> C::Curve { + match self { + Bucket::None => other, + Bucket::Affine(a) => other + a, + Bucket::Projective(a) => other + a, } - Bucket::Projective(a) => other + a, } } - } - let mut buckets: Vec> = vec![Bucket::None; (1 << c) - 1]; + let mut buckets = vec![Bucket::None; (1 << c) - 1]; - for (coeff, base) in coeffs.iter().zip(bases.iter()) { - let coeff = get_at::(current_segment, c, coeff); - if coeff != 0 { - buckets[coeff - 1].add_assign(base); + for (coeff, base) in coeffs.iter().zip(bases.iter()) { + let coeff = get_at::(segment, c, &coeff.to_repr()); + if coeff != 0 { + buckets[coeff - 1].add_assign(base); + } } - } - // Summation by parts - // e.g. 3a + 2b + 1c = a + - // (a) + b + - // ((a) + b) + c - let mut running_sum = C::Curve::identity(); - for exp in buckets.into_iter().rev() { - running_sum = exp.add(running_sum); - acc += &running_sum; - } - } - acc + // Summation by parts + // e.g. 3a + 2b + 1c = a + + // (a) + b + + // ((a) + b) + c + let mut running_sum = C::Curve::identity(); + for exp in buckets.into_iter().rev() { + running_sum = exp.add(running_sum); + acc += &running_sum; + } + acc + }) } /// Performs a multi-exponentiation operation without GPU acceleration. @@ -268,3 +259,44 @@ macro_rules! impl_traits { } }; } + +#[cfg(test)] +mod tests { + use super::cpu_best_multiexp; + + use crate::provider::{ + bn256_grumpkin::{bn256, grumpkin}, + secp_secq::{secp256k1, secq256k1}, + }; + use group::{ff::Field, Group}; + use halo2curves::CurveAffine; + use pasta_curves::{pallas, vesta}; + use rand_core::OsRng; + + fn test_msm_with>() { + let n = 8; + let coeffs = (0..n).map(|_| F::random(OsRng)).collect::>(); + let bases = (0..n) + .map(|_| A::from(A::generator() * F::random(OsRng))) + .collect::>(); + let naive = coeffs + .iter() + .zip(bases.iter()) + .fold(A::CurveExt::identity(), |acc, (coeff, base)| { + acc + *base * coeff + }); + let msm = cpu_best_multiexp(&coeffs, &bases); + + assert_eq!(naive, msm) + } + + #[test] + fn test_msm() { + test_msm_with::(); + test_msm_with::(); + test_msm_with::(); + test_msm_with::(); + test_msm_with::(); + test_msm_with::(); + } +} diff --git a/src/r1cs/mod.rs b/src/r1cs/mod.rs index f712b489a..0e5502ca5 100644 --- a/src/r1cs/mod.rs +++ b/src/r1cs/mod.rs @@ -104,8 +104,8 @@ pub fn commitment_key_size( ) -> usize { let num_cons = S.num_cons; let num_vars = S.num_vars; - let generators_hint = commitment_key_floor(S); - max(max(num_cons, num_vars), generators_hint) + let ck_hint = commitment_key_floor(S); + max(max(num_cons, num_vars), ck_hint) } impl R1CSShape { diff --git a/src/spartan/ppsnark.rs b/src/spartan/ppsnark.rs index 977c6b075..38a33c949 100644 --- a/src/spartan/ppsnark.rs +++ b/src/spartan/ppsnark.rs @@ -983,8 +983,9 @@ where S: &R1CSShape, ) -> Result<(Self::ProverKey, Self::VerifierKey), NovaError> { // check the provided commitment key meets minimal requirements - assert!(ck.length() >= Self::commitment_key_floor()(S)); - + if ck.length() < Self::commitment_key_floor()(S) { + return Err(NovaError::InvalidCommitmentKeyLength); + } let (pk_ee, vk_ee) = EE::setup(ck); // pad the R1CS matrices