feat: Refactor R1CS shape to split the commitment key generation

TL;DR: splits off one major source of diff lines from PR microsoft#283. Inherently, there are many R1CS shapes to consider when tailoring public parameter creation to non-uniform step-circuits.
However, the commitment key generation should only be done once for all circuits (once a suitable size has been determined by looking at all R1CS shapes). This splits the relevant Nova functions
into `r1cs_shape_and_key`, `r1cs_shape` and `commitment_key` to enable the flexibility deamnded by the above model.

In detail:
- Renamed the `r1cs_shape` method across various files to `r1cs_shape_and_key`, indicating its functionality is to return both `R1CSShape` and `CommitmentKey`.
- Altered function calls from `r1cs_shape` to `r1cs_shape_and_key` in files such as `direct.rs`, `nifs.rs`, `lib.rs` and `circuit.rs`,
- Split the creation of `R1CSShape` and `CommitmentKey` into separate functions in the `NovaShape` object in `r1cs.rs`
- Removed the `R1CS` struct in `mod.rs` as it only contained a phantom data, with related operations performed elsewhere.
- Implemented changes to enhance code readability, including the addition of a new `commitment_key_size` function, and overall code reformatting for clarity.

Author: huitseeker

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_key(&*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_key(&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_key(&*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_key(&*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_key(&*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_key(&*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_key(&*default_ck_hint());
 
     // Then the satisfying assignment
     let mut cs = SatisfyingAssignment::<E2>::new();

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_key(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_key(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_key(&*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/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],

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_key(&*S::ck_floor());
 
     let (pk, vk) = S::setup(&ck, &shape)?;