diff --git a/benches/compressed-snark.rs b/benches/compressed-snark.rs index 7da57365..8c713dab 100644 --- a/benches/compressed-snark.rs +++ b/benches/compressed-snark.rs @@ -216,7 +216,7 @@ impl StepCircuit for NonTrivialCircuit { let mut y = x.clone(); for i in 0..self.num_cons { y = x.square(cs.namespace(|| format!("x_sq_{i}")))?; - x = y.clone(); + x.clone_from(&y); } Ok(vec![y]) } diff --git a/benches/compute-digest.rs b/benches/compute-digest.rs index 64cc4851..f8bdacdc 100644 --- a/benches/compute-digest.rs +++ b/benches/compute-digest.rs @@ -68,7 +68,7 @@ impl StepCircuit for NonTrivialCircuit { let mut y = x.clone(); for i in 0..self.num_cons { y = x.square(cs.namespace(|| format!("x_sq_{i}")))?; - x = y.clone(); + x.clone_from(&y); } Ok(vec![y]) } diff --git a/benches/ppsnark.rs b/benches/ppsnark.rs index 56a24ac0..332f64b2 100644 --- a/benches/ppsnark.rs +++ b/benches/ppsnark.rs @@ -124,7 +124,7 @@ impl StepCircuit for NonTrivialCircuit { let mut y = z[0].clone(); for i in 0..self.num_cons { y = x.square(cs.namespace(|| format!("x_sq_{i}")))?; - x = y.clone(); + x.clone_from(&y); } Ok(vec![y]) } diff --git a/benches/recursive-snark.rs b/benches/recursive-snark.rs index 72beaa16..34bdc491 100644 --- a/benches/recursive-snark.rs +++ b/benches/recursive-snark.rs @@ -165,7 +165,7 @@ impl StepCircuit for NonTrivialCircuit { let mut y = x.clone(); for i in 0..self.num_cons { y = x.square(cs.namespace(|| format!("x_sq_{i}")))?; - x = y.clone(); + x.clone_from(&y); } Ok(vec![y]) } diff --git a/src/bellpepper/mod.rs b/src/bellpepper/mod.rs index 7b0aaf07..2ffcb12b 100644 --- a/src/bellpepper/mod.rs +++ b/src/bellpepper/mod.rs @@ -45,7 +45,7 @@ mod tests { // First create the shape let mut cs: ShapeCS = ShapeCS::new(); synthesize_alloc_bit(&mut cs); - let (shape, ck) = cs.r1cs_shape(&*default_ck_hint()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*default_ck_hint()); // Now get the assignment let mut cs = SatisfyingAssignment::::new(); diff --git a/src/bellpepper/r1cs.rs b/src/bellpepper/r1cs.rs index 3a7fd535..ed8301b9 100644 --- a/src/bellpepper/r1cs.rs +++ b/src/bellpepper/r1cs.rs @@ -5,7 +5,7 @@ use super::{shape_cs::ShapeCS, solver::SatisfyingAssignment, test_shape_cs::TestShapeCS}; use crate::{ errors::NovaError, - r1cs::{CommitmentKeyHint, R1CSInstance, R1CSShape, R1CSWitness, SparseMatrix, R1CS}, + r1cs::{commitment_key, CommitmentKeyHint, R1CSInstance, R1CSShape, R1CSWitness, SparseMatrix}, traits::Engine, CommitmentKey, }; @@ -27,7 +27,14 @@ pub trait NovaShape { /// Return an appropriate `R1CSShape` and `CommitmentKey` structs. /// A `CommitmentKeyHint` should be provided to help guide the construction of the `CommitmentKey`. /// This parameter is documented in `r1cs::R1CS::commitment_key`. - fn r1cs_shape(&self, ck_hint: &CommitmentKeyHint) -> (R1CSShape, CommitmentKey); + fn r1cs_shape_and_ck(&self, ck_hint: &CommitmentKeyHint) -> (R1CSShape, CommitmentKey) { + let S = self.r1cs_shape(); + let ck = commitment_key(&S, ck_hint); + + (S, ck) + } + /// Return an appropriate `R1CSShape`. + fn r1cs_shape(&self) -> R1CSShape; } impl NovaWitness for SatisfyingAssignment { @@ -53,7 +60,7 @@ macro_rules! impl_nova_shape { where E::Scalar: PrimeField, { - fn r1cs_shape(&self, ck_hint: &CommitmentKeyHint) -> (R1CSShape, CommitmentKey) { + fn r1cs_shape(&self) -> R1CSShape { let mut A = SparseMatrix::::empty(); let mut B = SparseMatrix::::empty(); let mut C = SparseMatrix::::empty(); @@ -80,10 +87,8 @@ macro_rules! impl_nova_shape { C.cols = num_vars + num_inputs; // Don't count One as an input for shape's purposes. - let S = R1CSShape::new(num_constraints, num_vars, num_inputs - 1, A, B, C).unwrap(); - let ck = R1CS::::commitment_key(&S, ck_hint); - - (S, ck) + let res = R1CSShape::new(num_constraints, num_vars, num_inputs - 1, A, B, C); + res.unwrap() } } }; diff --git a/src/circuit.rs b/src/circuit.rs index 1402e934..1b7c508b 100644 --- a/src/circuit.rs +++ b/src/circuit.rs @@ -396,7 +396,7 @@ mod tests { NovaAugmentedCircuit::new(primary_params, None, &tc1, ro_consts1.clone()); let mut cs: TestShapeCS = TestShapeCS::new(); let _ = circuit1.synthesize(&mut cs); - let (shape1, ck1) = cs.r1cs_shape(&*default_ck_hint()); + let (shape1, ck1) = cs.r1cs_shape_and_ck(&*default_ck_hint()); assert_eq!(cs.num_constraints(), num_constraints_primary); let tc2 = TrivialCircuit::default(); @@ -405,7 +405,7 @@ mod tests { NovaAugmentedCircuit::new(secondary_params, None, &tc2, ro_consts2.clone()); let mut cs: TestShapeCS = TestShapeCS::new(); let _ = circuit2.synthesize(&mut cs); - let (shape2, ck2) = cs.r1cs_shape(&*default_ck_hint()); + let (shape2, ck2) = cs.r1cs_shape_and_ck(&*default_ck_hint()); assert_eq!(cs.num_constraints(), num_constraints_secondary); // Execute the base case for the primary diff --git a/src/gadgets/ecc.rs b/src/gadgets/ecc.rs index ebb8f2de..25a1334b 100644 --- a/src/gadgets/ecc.rs +++ b/src/gadgets/ecc.rs @@ -1033,7 +1033,7 @@ mod tests { let mut cs: TestShapeCS = TestShapeCS::new(); let _ = synthesize_smul::(cs.namespace(|| "synthesize")); println!("Number of constraints: {}", cs.num_constraints()); - let (shape, ck) = cs.r1cs_shape(&*default_ck_hint()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*default_ck_hint()); // Then the satisfying assignment let mut cs = SatisfyingAssignment::::new(); @@ -1089,7 +1089,7 @@ mod tests { let mut cs: TestShapeCS = TestShapeCS::new(); let _ = synthesize_add_equal::(cs.namespace(|| "synthesize add equal")); println!("Number of constraints: {}", cs.num_constraints()); - let (shape, ck) = cs.r1cs_shape(&*default_ck_hint()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*default_ck_hint()); // Then the satisfying assignment let mut cs = SatisfyingAssignment::::new(); @@ -1149,7 +1149,7 @@ mod tests { let mut cs: TestShapeCS = TestShapeCS::new(); let _ = synthesize_add_negation::(cs.namespace(|| "synthesize add equal")); println!("Number of constraints: {}", cs.num_constraints()); - let (shape, ck) = cs.r1cs_shape(&*default_ck_hint()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*default_ck_hint()); // Then the satisfying assignment let mut cs = SatisfyingAssignment::::new(); diff --git a/src/gadgets/nonnative/mod.rs b/src/gadgets/nonnative/mod.rs index 8167e5a7..4d611cbb 100644 --- a/src/gadgets/nonnative/mod.rs +++ b/src/gadgets/nonnative/mod.rs @@ -6,16 +6,12 @@ use ff::PrimeField; trait OptionExt { fn grab(&self) -> Result<&T, SynthesisError>; - fn grab_mut(&mut self) -> Result<&mut T, SynthesisError>; } impl OptionExt for Option { fn grab(&self) -> Result<&T, SynthesisError> { self.as_ref().ok_or(SynthesisError::AssignmentMissing) } - fn grab_mut(&mut self) -> Result<&mut T, SynthesisError> { - self.as_mut().ok_or(SynthesisError::AssignmentMissing) - } } trait BitAccess { diff --git a/src/gadgets/nonnative/util.rs b/src/gadgets/nonnative/util.rs index f26c09f6..b4997368 100644 --- a/src/gadgets/nonnative/util.rs +++ b/src/gadgets/nonnative/util.rs @@ -14,8 +14,6 @@ use std::io::{self, Write}; pub struct Bit { /// The linear combination which constrain the value of the bit pub bit: LinearCombination, - /// The value of the bit (filled at witness-time) - pub value: Option, } #[derive(Clone)] @@ -58,7 +56,6 @@ impl Bit { Ok(Self { bit: LinearCombination::zero() + var, - value, }) } } diff --git a/src/lib.rs b/src/lib.rs index a999f127..f838fa90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -168,7 +168,7 @@ where ); let mut cs: ShapeCS = ShapeCS::new(); let _ = circuit_primary.synthesize(&mut cs); - let (r1cs_shape_primary, ck_primary) = cs.r1cs_shape(ck_hint1); + let (r1cs_shape_primary, ck_primary) = cs.r1cs_shape_and_ck(ck_hint1); // Initialize ck for the secondary let circuit_secondary: NovaAugmentedCircuit<'_, E1, C2> = NovaAugmentedCircuit::new( @@ -179,7 +179,7 @@ where ); let mut cs: ShapeCS = ShapeCS::new(); let _ = circuit_secondary.synthesize(&mut cs); - let (r1cs_shape_secondary, ck_secondary) = cs.r1cs_shape(ck_hint2); + let (r1cs_shape_secondary, ck_secondary) = cs.r1cs_shape_and_ck(ck_hint2); if r1cs_shape_primary.num_io != 2 || r1cs_shape_secondary.num_io != 2 { return Err(NovaError::InvalidStepCircuitIO); diff --git a/src/nifs.rs b/src/nifs.rs index 0329d38b..9e75d57f 100644 --- a/src/nifs.rs +++ b/src/nifs.rs @@ -124,7 +124,7 @@ mod tests { test_shape_cs::TestShapeCS, }, provider::{Bn256EngineKZG, PallasEngine, Secp256k1Engine}, - r1cs::{SparseMatrix, R1CS}, + r1cs::{commitment_key, SparseMatrix}, traits::{snark::default_ck_hint, Engine}, }; use ::bellpepper_core::{num::AllocatedNum, ConstraintSystem, SynthesisError}; @@ -168,7 +168,7 @@ mod tests { // First create the shape let mut cs: TestShapeCS = TestShapeCS::new(); let _ = synthesize_tiny_r1cs_bellpepper(&mut cs, None); - let (shape, ck) = cs.r1cs_shape(&*default_ck_hint()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*default_ck_hint()); let ro_consts = <::RO as ROTrait<::Base, ::Scalar>>::Constants::default(); @@ -327,7 +327,7 @@ mod tests { }; // generate generators and ro constants - let ck = R1CS::::commitment_key(&S, &*default_ck_hint()); + let ck = commitment_key(&S, &*default_ck_hint()); let ro_consts = <::RO as ROTrait<::Base, ::Scalar>>::Constants::default(); diff --git a/src/provider/hyperkzg.rs b/src/provider/hyperkzg.rs index 6afc9a53..65ff7efa 100644 --- a/src/provider/hyperkzg.rs +++ b/src/provider/hyperkzg.rs @@ -721,6 +721,7 @@ mod tests { } #[test] + #[allow(clippy::assigning_clones)] fn test_hyperkzg_small() { let n = 4; @@ -765,7 +766,9 @@ mod tests { // Change the proof and expect verification to fail let mut bad_proof = proof.clone(); + bad_proof.v[0] = bad_proof.v[1].clone(); + let mut verifier_transcript2 = Keccak256Transcript::new(b"TestEval"); assert!(EvaluationEngine::verify( &vk, @@ -779,6 +782,7 @@ mod tests { } #[test] + #[allow(clippy::assigning_clones)] fn test_hyperkzg_large() { // test the hyperkzg prover and verifier with random instances (derived from a seed) for ell in [4, 5, 6] { @@ -808,7 +812,9 @@ mod tests { // Change the proof and expect verification to fail let mut bad_proof = proof.clone(); + bad_proof.v[0] = bad_proof.v[1].clone(); + let mut verifier_tr2 = Keccak256Transcript::new(b"TestEval"); assert!( EvaluationEngine::verify(&vk, &mut verifier_tr2, &C, &point, &eval, &bad_proof).is_err() diff --git a/src/r1cs/mod.rs b/src/r1cs/mod.rs index 56ef1394..ddba6dcd 100644 --- a/src/r1cs/mod.rs +++ b/src/r1cs/mod.rs @@ -12,7 +12,7 @@ use crate::{ }, Commitment, CommitmentKey, CE, }; -use core::{cmp::max, marker::PhantomData}; +use core::cmp::max; use ff::Field; use once_cell::sync::OnceCell; @@ -22,13 +22,6 @@ use serde::{Deserialize, Serialize}; mod sparse; pub(crate) use sparse::SparseMatrix; -/// Public parameters for a given R1CS -#[derive(Clone, Serialize, Deserialize)] -#[serde(bound = "")] -pub struct R1CS { - _p: PhantomData, -} - /// A type that holds the shape of the R1CS matrices #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct R1CSShape { @@ -77,24 +70,31 @@ pub struct RelaxedR1CSInstance { pub type CommitmentKeyHint = dyn Fn(&R1CSShape) -> usize; -impl R1CS { - /// Generates public parameters for a Rank-1 Constraint System (R1CS). - /// - /// This function takes into consideration the shape of the R1CS matrices and a hint function - /// for the number of generators. It returns a `CommitmentKey`. - /// - /// # Arguments - /// - /// * `S`: The shape of the R1CS matrices. - /// * `ck_floor`: A function that provides a floor for the number of generators. A good function - /// to provide is the ck_floor field defined in the trait `RelaxedR1CSSNARKTrait`. - /// - pub fn commitment_key(S: &R1CSShape, ck_floor: &CommitmentKeyHint) -> CommitmentKey { - let num_cons = S.num_cons; - let num_vars = S.num_vars; - let ck_hint = ck_floor(S); - E::CE::setup(b"ck", max(max(num_cons, num_vars), ck_hint)) - } +/// Generates public parameters for a Rank-1 Constraint System (R1CS). +/// +/// This function takes into consideration the shape of the R1CS matrices and a hint function +/// for the number of generators. It returns a `CommitmentKey`. +/// +/// # Arguments +/// +/// * `S`: The shape of the R1CS matrices. +/// * `ck_floor`: A function that provides a floor for the number of generators. A good function +/// to provide is the ck_floor field defined in the trait `RelaxedR1CSSNARKTrait`. +/// +pub fn commitment_key( + S: &R1CSShape, + ck_floor: &CommitmentKeyHint, +) -> CommitmentKey { + let size = commitment_key_size(S, ck_floor); + E::CE::setup(b"ck", size) +} + +/// Computes the number of generators required for the commitment key corresponding to shape `S`. +fn commitment_key_size(S: &R1CSShape, ck_floor: &CommitmentKeyHint) -> usize { + let num_cons = S.num_cons; + let num_vars = S.num_vars; + let ck_hint = ck_floor(S); + max(max(num_cons, num_vars), ck_hint) } impl R1CSShape { @@ -157,6 +157,7 @@ impl R1CSShape { cons_valid && vars_valid && io_lt_vars } + /// multiplies a vector with the matrix pub fn multiply_vec( &self, z: &[E::Scalar], @@ -480,7 +481,7 @@ impl RelaxedR1CSInstance { let mut r_instance = RelaxedR1CSInstance::default(ck, S); r_instance.comm_W = instance.comm_W; r_instance.u = E::Scalar::ONE; - r_instance.X = instance.X.clone(); + r_instance.X.clone_from(&instance.X); r_instance } diff --git a/src/spartan/direct.rs b/src/spartan/direct.rs index 80986a2a..af9c86fc 100644 --- a/src/spartan/direct.rs +++ b/src/spartan/direct.rs @@ -109,7 +109,7 @@ impl, C: StepCircuit> DirectSN let mut cs: ShapeCS = ShapeCS::new(); let _ = circuit.synthesize(&mut cs); - let (shape, ck) = cs.r1cs_shape(&*S::ck_floor()); + let (shape, ck) = cs.r1cs_shape_and_ck(&*S::ck_floor()); let (pk, vk) = S::setup(&ck, &shape)?; @@ -277,7 +277,7 @@ mod tests { assert!(res.is_ok()); // set input to the next step - z_i = z_i_plus_one.clone(); + z_i.clone_from(&z_i_plus_one); } // sanity: check the claimed output with a direct computation of the same diff --git a/src/spartan/math.rs b/src/spartan/math.rs index 691fec5d..22dbce17 100644 --- a/src/spartan/math.rs +++ b/src/spartan/math.rs @@ -1,23 +1,8 @@ pub trait Math { - fn pow2(self) -> usize; - fn get_bits(self, num_bits: usize) -> Vec; fn log_2(self) -> usize; } impl Math for usize { - #[inline] - fn pow2(self) -> usize { - let base: usize = 2; - base.pow(self as u32) - } - - /// Returns the `num_bits` from n in a canonical order - fn get_bits(self, num_bits: usize) -> Vec { - (0..num_bits) - .map(|shift_amount| ((self & (1 << (num_bits - shift_amount - 1))) > 0)) - .collect::>() - } - fn log_2(self) -> usize { assert_ne!(self, 0);