Refactor supernova tests

benches/recursive-snark-supernova.rs

@@ -6,8 +6,9 @@ use criterion::*;
 use ff::PrimeField;
 use nova_snark::{
   compute_digest,
+  r1cs::R1CS,
   supernova::RecursiveSNARK,
-  supernova::{gen_commitment_key_by_r1cs, PublicParams, RunningClaim},
+  supernova::{PublicParams, RunningClaim},
   traits::{
     circuit_supernova::{StepCircuit, TrivialTestCircuit},
     Group,
@@ -66,8 +67,8 @@ fn bench_one_augmented_circuit_recursive_snark(c: &mut Criterion) {
     >::new(0, c_primary, c_secondary.clone(), 1);
 
     let (r1cs_shape_primary, r1cs_shape_secondary) = running_claim1.get_r1cs_shape();
-    let ck_primary = gen_commitment_key_by_r1cs(r1cs_shape_primary, None);
-    let ck_secondary = gen_commitment_key_by_r1cs(r1cs_shape_secondary, None);
+    let ck_primary = R1CS::commitment_key(r1cs_shape_primary, None);
+    let ck_secondary = R1CS::commitment_key(r1cs_shape_secondary, None);
 
     // set unified ck_primary, ck_secondary and update digest
     running_claim1.set_commitment_key(ck_primary.clone(), ck_secondary.clone());
@@ -182,8 +183,8 @@ fn bench_two_augmented_circuit_recursive_snark(c: &mut Criterion) {
     >::new(1, c_primary, c_secondary.clone(), 2);
 
     let (r1cs_shape_primary, r1cs_shape_secondary) = running_claim1.get_r1cs_shape();
-    let ck_primary = gen_commitment_key_by_r1cs(r1cs_shape_primary, None);
-    let ck_secondary = gen_commitment_key_by_r1cs(r1cs_shape_secondary, None);
+    let ck_primary = R1CS::commitment_key(r1cs_shape_primary, None);
+    let ck_secondary = R1CS::commitment_key(r1cs_shape_secondary, None);
 
     // set unified ck_primary, ck_secondary and update digest
     running_claim1.set_commitment_key(ck_primary.clone(), ck_secondary.clone());

src/lib.rs

@@ -16,12 +16,12 @@ mod bellpepper;
 mod circuit;
 mod constants;
 mod nifs;
-mod r1cs;
 
 // public modules
 pub mod errors;
 pub mod gadgets;
 pub mod provider;
+pub mod r1cs;
 pub mod spartan;
 pub mod traits;
 
@@ -42,6 +42,7 @@ use errors::NovaError;
 use ff::{Field, PrimeField};
 use gadgets::utils::scalar_as_base;
 use nifs::NIFS;
+pub use r1cs::R1CS;
 use r1cs::{
   CommitmentKeyHint, R1CSInstance, R1CSShape, R1CSWitness, RelaxedR1CSInstance, RelaxedR1CSWitness,
 };

src/r1cs.rs

@@ -73,6 +73,7 @@ pub struct RelaxedR1CSInstance<G: Group> {
   pub(crate) u: G::Scalar,
 }
 
+/// A type for functions that hints commitment key sizing by returning the floor of the number of required generators.
 pub type CommitmentKeyHint<G> = Box<dyn Fn(&R1CSShape<G>) -> usize>;
 
 impl<G: Group> R1CS<G> {
@@ -86,16 +87,25 @@ impl<G: Group> R1CS<G> {
   /// * `S`: The shape of the R1CS matrices.
   /// * `commitment_key_hint`: An optional function that provides a floor for the number of
   ///   generators. A good function to provide is the commitment_key_floor field in the trait `RelaxedR1CSSNARKTrait`.
-  ///   If no floot function is provided, the default number of generators will be max(S.num_cons, S.num_vars).
+  ///   If no floor function is provided, the default number of generators will be max(S.num_cons, S.num_vars).
   ///
   pub fn commitment_key(
     S: &R1CSShape<G>,
     commitment_key_floor: Option<CommitmentKeyHint<G>>,
   ) -> CommitmentKey<G> {
+    let size = Self::commitment_key_size(S, commitment_key_floor);
+    G::CE::setup(b"ck", size)
+  }
+
+  /// Computes the number of generators required for the commitment key corresponding to shape `S`.
+  pub fn commitment_key_size(
+    S: &R1CSShape<G>,
+    commitment_key_floor: Option<CommitmentKeyHint<G>>,
+  ) -> usize {
     let num_cons = S.num_cons;
     let num_vars = S.num_vars;
     let generators_hint = commitment_key_floor.map(|f| f(S)).unwrap_or(0);
-    G::CE::setup(b"ck", max(max(num_cons, num_vars), generators_hint))
+    max(max(num_cons, num_vars), generators_hint)
   }
 }
 
@@ -165,7 +175,7 @@ impl<G: Group> R1CSShape<G> {
     assert!(self.num_io < self.num_vars);
   }
 
-  pub fn multiply_vec(
+  pub(crate) fn multiply_vec(
     &self,
     z: &[G::Scalar],
   ) -> Result<(Vec<G::Scalar>, Vec<G::Scalar>, Vec<G::Scalar>), NovaError> {

src/supernova/mod.rs

@@ -6,10 +6,7 @@ use crate::{
   bellpepper::shape_cs::ShapeCS,
   constants::{BN_LIMB_WIDTH, BN_N_LIMBS, NUM_HASH_BITS},
   errors::NovaError,
-  r1cs::{
-    CommitmentKeyHint, R1CSInstance, R1CSShape, R1CSWitness, RelaxedR1CSInstance,
-    RelaxedR1CSWitness, R1CS,
-  },
+  r1cs::{R1CSInstance, R1CSShape, R1CSWitness, RelaxedR1CSInstance, RelaxedR1CSWitness, R1CS},
   scalar_as_base,
   traits::{
     circuit_supernova::StepCircuit, commitment::CommitmentTrait, AbsorbInROTrait, Group,
@@ -778,29 +775,35 @@ where
   }
 }
 
-/// Generates public parameters (a `CommitmentKey`) for a Rank-1 Constraint System (R1CS) circuit,
-/// appropriate for use with a particular SNARK protocol.
-///
-/// To generate a commitment key that is tailored to a specific R1CS circuit and SNARK protocol,
-/// this function considers the 'shape' of the R1CS circuit (provided via the `R1CSShape` parameter `shape`)
-/// and optionally specific requirements of the SNARK protocol’s methodology.
-///
-/// # Arguments
-///
-/// * `shape`: The shape of the R1CS matrices, which is essential to understanding the structure of the
-///   R1CS circuit for which the `CommitmentKey` is being generated.
-///
-/// * `optfn`: An optional parameter that encapsulates specific requirements of the
-///   SNARK protocol's methodology. For "classical" SNARKs with no special needs, this can be `None`.
-///   For specific SNARKs, like Spartan with computational commitments, a particular value should be
-///   passed here. This value is typically provided in the SNARK's trait or implementation details.
-///
-/// # Returns
-///
-/// A `CommitmentKey` tailored to the given R1CS circuit shape and SNARK protocol specifics.
-pub fn gen_commitment_key_by_r1cs<G: Group>(
-  shape: &R1CSShape<G>,
-  optfn: Option<CommitmentKeyHint<G>>,
-) -> CommitmentKey<G> {
-  R1CS::<G>::commitment_key(shape, optfn)
+/// Compute primary and secondary commitment keys sized to handle the largest of the circuits in the provided
+/// `PublicParams`.
+pub fn compute_commitment_keys<G1: Group, G2: Group>(
+  circuit_public_params: &[&PublicParams<G1, G2>],
+) -> (CommitmentKey<G1>, CommitmentKey<G2>)
+where
+  G1: Group<Base = <G2 as Group>::Scalar>,
+  G2: Group<Base = <G1 as Group>::Scalar>,
+{
+  macro_rules! max_shape {
+    ($shape_getter:ident) => {
+      circuit_public_params
+        .iter()
+        .map(|params| {
+          let shape = &params.$shape_getter;
+          let size = R1CS::commitment_key_size(&shape, None);
+          (shape, size)
+        })
+        .max_by(|a, b| a.1.cmp(&b.1))
+        .unwrap()
+        .0
+    };
+  }
+
+  let shape_primary = max_shape!(r1cs_shape_primary);
+  let shape_secondary = max_shape!(r1cs_shape_secondary);
+
+  let ck_primary = R1CS::commitment_key(shape_primary, None);
+  let ck_secondary = R1CS::commitment_key(shape_secondary, None);
+
+  (ck_primary, ck_secondary)
 }

src/supernova/test.rs

@@ -18,9 +18,9 @@ use tap::TapOptional;
 
 use super::*;
 
-fn constraint_augmented_circuit_index<F: PrimeField, CS: ConstraintSystem<F>>(
+fn constrain_augmented_circuit_index<F: PrimeField, CS: ConstraintSystem<F>>(
   mut cs: CS,
-  pc_counter: &AllocatedNum<F>,
+  program_counter: &AllocatedNum<F>,
   rom: &[AllocatedNum<F>],
   circuit_index: &AllocatedNum<F>,
 ) -> Result<(), SynthesisError> {
@@ -37,7 +37,7 @@ fn constraint_augmented_circuit_index<F: PrimeField, CS: ConstraintSystem<F>>(
       let equal_bit = Boolean::from(alloc_num_equals(
         cs.namespace(|| format!("rom_values {} equal bit", i)),
         &index_alloc,
-        pc_counter,
+        program_counter,
       )?);
       conditionally_select(
         cs.namespace(|| format!("rom_values {} conditionally_select ", i)),
@@ -95,17 +95,17 @@ where
   fn synthesize<CS: ConstraintSystem<F>>(
     &self,
     cs: &mut CS,
-    pc_counter: &AllocatedNum<F>,
+    pc: &AllocatedNum<F>,
     z: &[AllocatedNum<F>],
   ) -> Result<(AllocatedNum<F>, Vec<AllocatedNum<F>>), SynthesisError> {
     // constrain rom[pc] equal to `self.circuit_index`
     let circuit_index = alloc_const(
       cs.namespace(|| "circuit_index"),
       F::from(self.circuit_index as u64),
     )?;
-    constraint_augmented_circuit_index(
+    constrain_augmented_circuit_index(
       cs.namespace(|| "CubicCircuit agumented circuit constraint"),
-      pc_counter,
+      pc,
       &z[1..],
       &circuit_index,
     )?;
@@ -114,7 +114,7 @@ where
     let pc_next = add_allocated_num(
       // pc = pc + 1
       cs.namespace(|| "pc = pc + 1".to_string()),
-      pc_counter,
+      pc,
       &one,
     )?;
 
@@ -178,25 +178,25 @@ where
   fn synthesize<CS: ConstraintSystem<F>>(
     &self,
     cs: &mut CS,
-    pc_counter: &AllocatedNum<F>,
+    pc: &AllocatedNum<F>,
     z: &[AllocatedNum<F>],
   ) -> Result<(AllocatedNum<F>, Vec<AllocatedNum<F>>), SynthesisError> {
     // constrain rom[pc] equal to `self.circuit_index`
     let circuit_index = alloc_const(
       cs.namespace(|| "circuit_index"),
       F::from(self.circuit_index as u64),
     )?;
-    constraint_augmented_circuit_index(
+    constrain_augmented_circuit_index(
       cs.namespace(|| "SquareCircuit agumented circuit constraint"),
-      pc_counter,
+      pc,
       &z[1..],
       &circuit_index,
     )?;
     let one = alloc_one(cs.namespace(|| "alloc one"))?;
     let pc_next = add_allocated_num(
       // pc = pc + 1
       cs.namespace(|| "pc = pc + 1"),
-      pc_counter,
+      pc,
       &one,
     )?;
 
@@ -331,29 +331,12 @@ where
   );
 
   // generate the commitkey based on max num of constraints and reused it for all other augmented circuit
-  let circuit_public_params = vec![&running_claim1.params, &running_claim2.params];
-  let (max_index_circuit, _) = circuit_public_params
-    .iter()
-    .enumerate()
-    .map(|(i, params)| -> (usize, usize) { (i, params.r1cs_shape_primary.num_cons) })
-    .max_by(|(_, circuit_size1), (_, circuit_size2)| circuit_size1.cmp(circuit_size2))
-    .unwrap();
-
-  let ck_primary = gen_commitment_key_by_r1cs(
-    &circuit_public_params[max_index_circuit].r1cs_shape_primary,
-    None,
-  );
-  let ck_secondary = gen_commitment_key_by_r1cs(
-    &circuit_public_params[max_index_circuit].r1cs_shape_secondary,
-    None,
-  );
+  let (ck_primary, ck_secondary) =
+    compute_commitment_keys(&[&running_claim1.params, &running_claim2.params]);
 
   // set unified ck_primary, ck_secondary and update digest
-  running_claim1.params.ck_primary = Some(ck_primary.clone());
-  running_claim1.params.ck_secondary = Some(ck_secondary.clone());
-
-  running_claim2.params.ck_primary = Some(ck_primary);
-  running_claim2.params.ck_secondary = Some(ck_secondary);
+  running_claim1.set_commitment_key(ck_primary.clone(), ck_secondary.clone());
+  running_claim2.set_commitment_key(ck_primary, ck_secondary);
 
   let digest = compute_digest::<G1, PublicParams<G1, G2>>(&[
     running_claim1.get_public_params(),