From 94e527bd01d8c14d59869d7889ae4a582cbd1246 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Thu, 7 Mar 2024 17:10:32 -0500 Subject: [PATCH] feat: Refactor R1CS shape to split the commitment key generation TL;DR: splits off one major source of diff lines from PR #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. --- src/bellpepper/mod.rs | 2 +- src/bellpepper/r1cs.rs | 19 +++++++++------ src/circuit.rs | 4 ++-- src/gadgets/ecc.rs | 6 ++--- src/lib.rs | 4 ++-- src/nifs.rs | 6 ++--- src/r1cs/mod.rs | 53 +++++++++++++++++++++--------------------- src/spartan/direct.rs | 2 +- 8 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/bellpepper/mod.rs b/src/bellpepper/mod.rs index 7b0aaf07..a8a657e0 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_key(&*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..8d2401fe 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_key(&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..129fed58 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_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 = 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 diff --git a/src/gadgets/ecc.rs b/src/gadgets/ecc.rs index ebb8f2de..f9d8de16 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_key(&*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_key(&*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_key(&*default_ck_hint()); // Then the satisfying assignment let mut cs = SatisfyingAssignment::::new(); diff --git a/src/lib.rs b/src/lib.rs index a999f127..3e31479d 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_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 = 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); diff --git a/src/nifs.rs b/src/nifs.rs index 0329d38b..914f49cf 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_key(&*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/r1cs/mod.rs b/src/r1cs/mod.rs index 56ef1394..25509eaf 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], diff --git a/src/spartan/direct.rs b/src/spartan/direct.rs index 80986a2a..bee96b82 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_key(&*S::ck_floor()); let (pk, vk) = S::setup(&ck, &shape)?;