feat: Refactor R1CS shape to split the commitment key generation

benches/compressed-snark.rs

@@ -216,7 +216,7 @@ impl<F: PrimeField> StepCircuit<F> for NonTrivialCircuit<F> {
     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])
   }

benches/compute-digest.rs

@@ -68,7 +68,7 @@ impl<F: PrimeField> StepCircuit<F> for NonTrivialCircuit<F> {
     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])
   }

benches/ppsnark.rs

@@ -124,7 +124,7 @@ impl<F: PrimeField> StepCircuit<F> for NonTrivialCircuit<F> {
     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])
   }

benches/recursive-snark.rs

@@ -165,7 +165,7 @@ impl<F: PrimeField> StepCircuit<F> for NonTrivialCircuit<F> {
     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])
   }

src/bellpepper/mod.rs

@@ -45,7 +45,7 @@ mod tests {
     // First create the shape
     let mut cs: ShapeCS<E> = 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::<E>::new();

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<E: Engine> {
   /// 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<E>) -> (R1CSShape<E>, CommitmentKey<E>);
+  fn r1cs_shape_and_ck(&self, ck_hint: &CommitmentKeyHint<E>) -> (R1CSShape<E>, CommitmentKey<E>) {
+    let S = self.r1cs_shape();
+    let ck = commitment_key(&S, ck_hint);
+
+    (S, ck)
+  }
+  /// Return an appropriate `R1CSShape`.
+  fn r1cs_shape(&self) -> R1CSShape<E>;
 }
 
 impl<E: Engine> NovaWitness<E> for SatisfyingAssignment<E> {
@@ -53,7 +60,7 @@ macro_rules! impl_nova_shape {
     where
       E::Scalar: PrimeField,
     {
-      fn r1cs_shape(&self, ck_hint: &CommitmentKeyHint<E>) -> (R1CSShape<E>, CommitmentKey<E>) {
+      fn r1cs_shape(&self) -> R1CSShape<E> {
         let mut A = SparseMatrix::<E::Scalar>::empty();
         let mut B = SparseMatrix::<E::Scalar>::empty();
         let mut C = SparseMatrix::<E::Scalar>::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::<E>::commitment_key(&S, ck_hint);
-
-        (S, ck)
+        let res = R1CSShape::new(num_constraints, num_vars, num_inputs - 1, A, B, C);
+        res.unwrap()
       }
     }
   };

src/circuit.rs

@@ -396,7 +396,7 @@ mod tests {
       NovaAugmentedCircuit::new(primary_params, None, &tc1, ro_consts1.clone());
     let mut cs: TestShapeCS<E1> = 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<E2> = 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

src/gadgets/ecc.rs

@@ -1033,7 +1033,7 @@ mod tests {
     let mut cs: TestShapeCS<E2> = TestShapeCS::new();
     let _ = synthesize_smul::<E1, _>(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::<E2>::new();
@@ -1089,7 +1089,7 @@ mod tests {
     let mut cs: TestShapeCS<E2> = TestShapeCS::new();
     let _ = synthesize_add_equal::<E1, _>(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::<E2>::new();
@@ -1149,7 +1149,7 @@ mod tests {
     let mut cs: TestShapeCS<E2> = TestShapeCS::new();
     let _ = synthesize_add_negation::<E1, _>(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::<E2>::new();

src/gadgets/nonnative/mod.rs

@@ -6,16 +6,12 @@ use ff::PrimeField;
 
 trait OptionExt<T> {
   fn grab(&self) -> Result<&T, SynthesisError>;
-  fn grab_mut(&mut self) -> Result<&mut T, SynthesisError>;
 }
 
 impl<T> OptionExt<T> for Option<T> {
   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 {

src/gadgets/nonnative/util.rs

@@ -14,8 +14,6 @@ use std::io::{self, Write};
 pub struct Bit<Scalar: PrimeField> {
   /// The linear combination which constrain the value of the bit
   pub bit: LinearCombination<Scalar>,
-  /// The value of the bit (filled at witness-time)
-  pub value: Option<bool>,
 }
 
 #[derive(Clone)]
@@ -58,7 +56,6 @@ impl<Scalar: PrimeField> Bit<Scalar> {
 
     Ok(Self {
       bit: LinearCombination::zero() + var,
-      value,
     })
   }
 }

src/lib.rs

@@ -168,7 +168,7 @@ where
     );
     let mut cs: ShapeCS<E1> = 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<E2> = 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);

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<E> = 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 =
       <<E as Engine>::RO as ROTrait<<E as Engine>::Base, <E as Engine>::Scalar>>::Constants::default();
 
@@ -327,7 +327,7 @@ mod tests {
     };
 
     // generate generators and ro constants
-    let ck = R1CS::<E>::commitment_key(&S, &*default_ck_hint());
+    let ck = commitment_key(&S, &*default_ck_hint());
     let ro_consts =
       <<E as Engine>::RO as ROTrait<<E as Engine>::Base, <E as Engine>::Scalar>>::Constants::default();
 

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()

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<E: Engine> {
-  _p: PhantomData<E>,
-}
-
 /// A type that holds the shape of the R1CS matrices
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub struct R1CSShape<E: Engine> {
@@ -77,24 +70,31 @@ pub struct RelaxedR1CSInstance<E: Engine> {
 
 pub type CommitmentKeyHint<E> = dyn Fn(&R1CSShape<E>) -> usize;
 
-impl<E: Engine> R1CS<E> {
-  /// 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<E>, ck_floor: &CommitmentKeyHint<E>) -> CommitmentKey<E> {
-    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<E: Engine>(
+  S: &R1CSShape<E>,
+  ck_floor: &CommitmentKeyHint<E>,
+) -> CommitmentKey<E> {
+  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<E: Engine>(S: &R1CSShape<E>, ck_floor: &CommitmentKeyHint<E>) -> 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<E: Engine> R1CSShape<E> {
@@ -157,6 +157,7 @@ impl<E: Engine> R1CSShape<E> {
     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<E: Engine> RelaxedR1CSInstance<E> {
     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
   }
 

src/spartan/direct.rs

@@ -109,7 +109,7 @@ impl<E: Engine, S: RelaxedR1CSSNARKTrait<E>, C: StepCircuit<E::Scalar>> DirectSN
     let mut cs: ShapeCS<E> = 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

src/spartan/math.rs

@@ -1,23 +1,8 @@
 pub trait Math {
-  fn pow2(self) -> usize;
-  fn get_bits(self, num_bits: usize) -> Vec<bool>;
   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<bool> {
-    (0..num_bits)
-      .map(|shift_amount| ((self & (1 << (num_bits - shift_amount - 1))) > 0))
-      .collect::<Vec<bool>>()
-  }
-
   fn log_2(self) -> usize {
     assert_ne!(self, 0);