refactor: avoid superfluous clones in ProverKey and VerifierKey f…

…or Nova & SuperNova (#285)

* refactor: Refactor codebase to use `Arc` for commitment keys

- Major changes in the handling of the Commitment Key object across several files, moving from direct references to using the Arc (Atomic Reference Counting) mechanism for improved thread-safety and efficient sharing of data.
- Removed the `Abomonation` trait from several structures, such as `ProverKey` and `VerifierKey`.
- Introduced the `SimpleDigestible` trait to VerifierKey in several files, allowing them to be converted into a digestible form for hashing or storage.
- Numerous updates in test cases and instances to reflect the shift from direct Commitment Key references to Arc.
- Refactoring of the `setup` and `prove` methods in multiple files to handle the Arc type for Commitment Key.
- Removal of several `abomonate_with` and `abomonation_skip` attribute annotations.
- Enhancement of error details in multiple `assert` statements.
- Added "rc" feature to the serde dependency in the Cargo.toml file.
- Made comments on minimizing unnecessary data copies and keeping `setup` methods inexpensive.

* refactor: Serialization with Abomonation

- Extended functionality in `lib.rs` with new `PublicParams` structure, efficient abomonationwith `FlatPublicParams`,
- Updated `mod.rs` for conditional feature `abomonate`, added `AuxParams` struct, and implemented conversions between `AuxParams` and `FlatAuxParams`.
- demoed the new use of Abomonation in the minroot example

* refactor: clean up superfluous Abomonation bounds

- Removed Abomonation dependency from several files.

* fix: remove uneeded Abomonation bounds

Cargo.toml

@@ -28,7 +28,7 @@ generic-array = "1.0.0"
 num-bigint = { version = "0.4", features = ["serde", "rand"] }
 num-traits = "0.2"
 num-integer = "0.1"
-serde = { version = "1.0", features = ["derive"] }
+serde = { version = "1.0", features = ["derive", "rc"] }
 bincode = "1.3"
 bitvec = "1.0"
 byteorder = "1.4.3"

benches/pcs.rs

@@ -14,6 +14,7 @@ use halo2curves::bn256::Bn256;
 use rand::rngs::StdRng;
 use rand_core::{CryptoRng, RngCore, SeedableRng};
 use std::any::type_name;
+use std::sync::Arc;
 use std::time::Duration;
 
 // To run these benchmarks, first download `criterion` with `cargo install cargo-criterion`.
@@ -44,7 +45,7 @@ struct BenchAssests<E: Engine, EE: EvaluationEngineTrait<E>> {
   poly: MultilinearPolynomial<<E as Engine>::Scalar>,
   point: Vec<<E as Engine>::Scalar>,
   eval: <E as Engine>::Scalar,
-  ck: <<E as Engine>::CE as CommitmentEngineTrait<E>>::CommitmentKey,
+  ck: Arc<<<E as Engine>::CE as CommitmentEngineTrait<E>>::CommitmentKey>,
   commitment: <<E as Engine>::CE as CommitmentEngineTrait<E>>::Commitment,
   prover_key: <EE as EvaluationEngineTrait<E>>::ProverKey,
   verifier_key: <EE as EvaluationEngineTrait<E>>::VerifierKey,
@@ -78,10 +79,11 @@ impl<E: Engine, EE: EvaluationEngineTrait<E>> BenchAssests<E, EE> {
 
     // Mock commitment key.
     let ck = E::CE::setup(b"test", 1 << num_vars);
+    let ck = Arc::new(ck);
     // Commits to the provided vector using the provided generators.
     let commitment = E::CE::commit(&ck, poly.evaluations());
 
-    let (prover_key, verifier_key) = EE::setup(&ck);
+    let (prover_key, verifier_key) = EE::setup(ck.clone());
 
     // Generate proof so that we can bench verification.
     let proof = EE::prove(

examples/minroot.rs

@@ -1,6 +1,8 @@
 //! Demonstrates how to use Nova to produce a recursive proof of the correct execution of
 //! iterations of the `MinRoot` function, thereby realizing a Nova-based verifiable delay function (VDF).
 //! We execute a configurable number of iterations of the `MinRoot` function per step of Nova's recursion.
+#[cfg(feature = "abomonate")]
+use arecibo::FlatPublicParams;
 use arecibo::{
   provider::{Bn256EngineKZG, GrumpkinEngine},
   traits::{
@@ -232,14 +234,16 @@ fn main() {
     );
     println!("PublicParams::setup, took {:?} ", start.elapsed());
     #[cfg(feature = "abomonate")]
-    {
+    let pp = {
       use abomonation::encode;
       let mut file = std::fs::File::create(utils::FILEPATH).unwrap();
+      let flat_params = FlatPublicParams::try_from(pp).expect("error encoding pps!");
       unsafe {
-        encode(&pp, &mut file).unwrap();
+        encode(&flat_params, &mut file).unwrap();
       }
       println!("Encoded!");
-    }
+      PublicParams::from(flat_params)
+    };
 
     println!(
       "Number of constraints per step (primary circuit): {}",
@@ -270,16 +274,18 @@ fn main() {
       reader.read_to_end(&mut bytes).unwrap();
       if let Some((result, remaining)) = unsafe {
         decode::<
-          PublicParams<
+          FlatPublicParams<
             E1,
             E2,
             MinRootCircuit<<E1 as Engine>::GE>,
             TrivialCircuit<<E2 as Engine>::Scalar>,
           >,
         >(&mut bytes)
       } {
-        assert!(*result == pp, "decoded parameters not equal to original!");
+        let result_pp = PublicParams::from(result.clone());
+        assert!(result_pp == pp, "decoded parameters not equal to original!");
         assert!(remaining.is_empty());
+        println!("Decoded!");
       } else {
         println!("Decoding failure!");
       }
@@ -352,8 +358,8 @@ fn main() {
     type E2 = GrumpkinEngine;
     type EE1 = arecibo::provider::hyperkzg::EvaluationEngine<Bn256, E1>;
     type EE2 = arecibo::provider::ipa_pc::EvaluationEngine<E2>;
-    type S1 = arecibo::spartan::snark::RelaxedR1CSSNARK<E1, EE1>; // non-preprocessing SNARK
-    type S2 = arecibo::spartan::snark::RelaxedR1CSSNARK<E2, EE2>; // non-preprocessing SNARK
+    type S1 = arecibo::spartan::ppsnark::RelaxedR1CSSNARK<E1, EE1>; // non-preprocessing SNARK
+    type S2 = arecibo::spartan::ppsnark::RelaxedR1CSSNARK<E2, EE2>; // non-preprocessing SNARK
 
     let res = CompressedSNARK::<_, _, _, _, S1, S2>::prove(&pp, &pk, &recursive_snark);
     println!(

src/lib.rs

@@ -52,6 +52,7 @@ use r1cs::{
   CommitmentKeyHint, R1CSInstance, R1CSShape, R1CSWitness, RelaxedR1CSInstance, RelaxedR1CSWitness,
 };
 use serde::{Deserialize, Serialize};
+use std::sync::Arc;
 use traits::{
   circuit::StepCircuit,
   commitment::{CommitmentEngineTrait, CommitmentTrait},
@@ -87,6 +88,37 @@ impl<E: Engine> R1CSWithArity<E> {
 }
 
 /// A type that holds public parameters of Nova
+#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(bound = "")]
+pub struct PublicParams<E1, E2, C1, C2>
+where
+  E1: Engine<Base = <E2 as Engine>::Scalar>,
+  E2: Engine<Base = <E1 as Engine>::Scalar>,
+  C1: StepCircuit<E1::Scalar>,
+  C2: StepCircuit<E2::Scalar>,
+{
+  F_arity_primary: usize,
+  F_arity_secondary: usize,
+  ro_consts_primary: ROConstants<E1>,
+  ro_consts_circuit_primary: ROConstantsCircuit<E2>,
+  ck_primary: Arc<CommitmentKey<E1>>,
+  circuit_shape_primary: R1CSWithArity<E1>,
+  ro_consts_secondary: ROConstants<E2>,
+  ro_consts_circuit_secondary: ROConstantsCircuit<E1>,
+  ck_secondary: Arc<CommitmentKey<E2>>,
+  circuit_shape_secondary: R1CSWithArity<E2>,
+  augmented_circuit_params_primary: NovaAugmentedCircuitParams,
+  augmented_circuit_params_secondary: NovaAugmentedCircuitParams,
+  #[serde(skip, default = "OnceCell::new")]
+  digest: OnceCell<E1::Scalar>,
+  _p: PhantomData<(C1, C2)>,
+}
+
+// Ensure to include necessary crates and features in your Cargo.toml
+// e.g., abomonation, serde, etc., with the appropriate feature flags.
+
+/// A version of [`crate::PublicParams`] that is amenable to fast ser/de using Abomonation
+#[cfg(feature = "abomonate")]
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, Abomonation)]
 #[serde(bound = "")]
 #[abomonation_bounds(
@@ -98,7 +130,7 @@ where
   <E1::Scalar as PrimeField>::Repr: Abomonation,
   <E2::Scalar as PrimeField>::Repr: Abomonation,
 )]
-pub struct PublicParams<E1, E2, C1, C2>
+pub struct FlatPublicParams<E1, E2, C1, C2>
 where
   E1: Engine<Base = <E2 as Engine>::Scalar>,
   E2: Engine<Base = <E1 as Engine>::Scalar>,
@@ -117,12 +149,70 @@ where
   circuit_shape_secondary: R1CSWithArity<E2>,
   augmented_circuit_params_primary: NovaAugmentedCircuitParams,
   augmented_circuit_params_secondary: NovaAugmentedCircuitParams,
-  #[abomonation_skip]
-  #[serde(skip, default = "OnceCell::new")]
-  digest: OnceCell<E1::Scalar>,
   _p: PhantomData<(C1, C2)>,
 }
 
+#[cfg(feature = "abomonate")]
+impl<E1, E2, C1, C2> TryFrom<PublicParams<E1, E2, C1, C2>> for FlatPublicParams<E1, E2, C1, C2>
+where
+  E1: Engine<Base = <E2 as Engine>::Scalar>,
+  E2: Engine<Base = <E1 as Engine>::Scalar>,
+  C1: StepCircuit<E1::Scalar>,
+  C2: StepCircuit<E2::Scalar>,
+{
+  type Error = &'static str;
+
+  fn try_from(value: PublicParams<E1, E2, C1, C2>) -> Result<Self, Self::Error> {
+    let ck_primary =
+      Arc::try_unwrap(value.ck_primary).map_err(|_| "Failed to unwrap Arc for ck_primary")?;
+    let ck_secondary =
+      Arc::try_unwrap(value.ck_secondary).map_err(|_| "Failed to unwrap Arc for ck_secondary")?;
+    Ok(Self {
+      F_arity_primary: value.F_arity_primary,
+      F_arity_secondary: value.F_arity_secondary,
+      ro_consts_primary: value.ro_consts_primary,
+      ro_consts_circuit_primary: value.ro_consts_circuit_primary,
+      ck_primary,
+      circuit_shape_primary: value.circuit_shape_primary,
+      ro_consts_secondary: value.ro_consts_secondary,
+      ro_consts_circuit_secondary: value.ro_consts_circuit_secondary,
+      ck_secondary,
+      circuit_shape_secondary: value.circuit_shape_secondary,
+      augmented_circuit_params_primary: value.augmented_circuit_params_primary,
+      augmented_circuit_params_secondary: value.augmented_circuit_params_secondary,
+      _p: PhantomData,
+    })
+  }
+}
+
+#[cfg(feature = "abomonate")]
+impl<E1, E2, C1, C2> From<FlatPublicParams<E1, E2, C1, C2>> for PublicParams<E1, E2, C1, C2>
+where
+  E1: Engine<Base = <E2 as Engine>::Scalar>,
+  E2: Engine<Base = <E1 as Engine>::Scalar>,
+  C1: StepCircuit<E1::Scalar>,
+  C2: StepCircuit<E2::Scalar>,
+{
+  fn from(value: FlatPublicParams<E1, E2, C1, C2>) -> Self {
+    Self {
+      F_arity_primary: value.F_arity_primary,
+      F_arity_secondary: value.F_arity_secondary,
+      ro_consts_primary: value.ro_consts_primary,
+      ro_consts_circuit_primary: value.ro_consts_circuit_primary,
+      ck_primary: Arc::new(value.ck_primary),
+      circuit_shape_primary: value.circuit_shape_primary,
+      ro_consts_secondary: value.ro_consts_secondary,
+      ro_consts_circuit_secondary: value.ro_consts_circuit_secondary,
+      ck_secondary: Arc::new(value.ck_secondary),
+      circuit_shape_secondary: value.circuit_shape_secondary,
+      augmented_circuit_params_primary: value.augmented_circuit_params_primary,
+      augmented_circuit_params_secondary: value.augmented_circuit_params_secondary,
+      digest: OnceCell::new(),
+      _p: PhantomData,
+    }
+  }
+}
+
 impl<E1, E2, C1, C2> SimpleDigestible for PublicParams<E1, E2, C1, C2>
 where
   E1: Engine<Base = <E2 as Engine>::Scalar>,
@@ -213,6 +303,7 @@ where
     let mut cs: ShapeCS<E1> = ShapeCS::new();
     let _ = circuit_primary.synthesize(&mut cs);
     let (r1cs_shape_primary, ck_primary) = cs.r1cs_shape_and_key(ck_hint1);
+    let ck_primary = Arc::new(ck_primary);
     let circuit_shape_primary = R1CSWithArity::new(r1cs_shape_primary, F_arity_primary);
 
     // Initialize ck for the secondary
@@ -225,6 +316,7 @@ where
     let mut cs: ShapeCS<E2> = ShapeCS::new();
     let _ = circuit_secondary.synthesize(&mut cs);
     let (r1cs_shape_secondary, ck_secondary) = cs.r1cs_shape_and_key(ck_hint2);
+    let ck_secondary = Arc::new(ck_secondary);
     let circuit_shape_secondary = R1CSWithArity::new(r1cs_shape_secondary, F_arity_secondary);
 
     Self {
@@ -386,7 +478,7 @@ where
     let l_u_primary = u_primary;
     let r_W_primary = RelaxedR1CSWitness::from_r1cs_witness(r1cs_primary, l_w_primary);
     let r_U_primary = RelaxedR1CSInstance::from_r1cs_instance(
-      &pp.ck_primary,
+      &*pp.ck_primary,
       &pp.circuit_shape_primary.r1cs_shape,
       l_u_primary,
     );
@@ -469,7 +561,7 @@ where
 
     // fold the secondary circuit's instance
     let nifs_secondary = NIFS::prove_mut(
-      &pp.ck_secondary,
+      &*pp.ck_secondary,
       &pp.ro_consts_secondary,
       &scalar_as_base::<E1>(pp.digest()),
       &pp.circuit_shape_secondary.r1cs_shape,
@@ -510,7 +602,7 @@ where
 
     // fold the primary circuit's instance
     let nifs_primary = NIFS::prove_mut(
-      &pp.ck_primary,
+      &*pp.ck_primary,
       &pp.ro_consts_primary,
       &pp.digest(),
       &pp.circuit_shape_primary.r1cs_shape,
@@ -688,9 +780,7 @@ where
 }
 
 /// A type that holds the prover key for `CompressedSNARK`
-#[derive(Clone, Debug, Serialize, Deserialize, Abomonation)]
-#[serde(bound = "")]
-#[abomonation_omit_bounds]
+#[derive(Clone, Debug)]
 pub struct ProverKey<E1, E2, C1, C2, S1, S2>
 where
   E1: Engine<Base = <E2 as Engine>::Scalar>,
@@ -706,18 +796,8 @@ where
 }
 
 /// A type that holds the verifier key for `CompressedSNARK`
-#[derive(Debug, Clone, Serialize, Deserialize, Abomonation)]
+#[derive(Debug, Clone, Serialize)]
 #[serde(bound = "")]
-#[abomonation_bounds(
-  where
-    E1: Engine<Base = <E2 as Engine>::Scalar>,
-    E2: Engine<Base = <E1 as Engine>::Scalar>,
-    C1: StepCircuit<E1::Scalar>,
-    C2: StepCircuit<E2::Scalar>,
-    S1: RelaxedR1CSSNARKTrait<E1>,
-    S2: RelaxedR1CSSNARKTrait<E2>,
-    <E1::Scalar as PrimeField>::Repr: Abomonation,
-  )]
 pub struct VerifierKey<E1, E2, C1, C2, S1, S2>
 where
   E1: Engine<Base = <E2 as Engine>::Scalar>,
@@ -731,7 +811,6 @@ where
   F_arity_secondary: usize,
   ro_consts_primary: ROConstants<E1>,
   ro_consts_secondary: ROConstants<E2>,
-  #[abomonate_with(<E1::Scalar as PrimeField>::Repr)]
   pp_digest: E1::Scalar,
   vk_primary: S1::VerifierKey,
   vk_secondary: S2::VerifierKey,
@@ -783,9 +862,12 @@ where
     ),
     NovaError,
   > {
-    let (pk_primary, vk_primary) = S1::setup(&pp.ck_primary, &pp.circuit_shape_primary.r1cs_shape)?;
-    let (pk_secondary, vk_secondary) =
-      S2::setup(&pp.ck_secondary, &pp.circuit_shape_secondary.r1cs_shape)?;
+    let (pk_primary, vk_primary) =
+      S1::setup(pp.ck_primary.clone(), &pp.circuit_shape_primary.r1cs_shape)?;
+    let (pk_secondary, vk_secondary) = S2::setup(
+      pp.ck_secondary.clone(),
+      &pp.circuit_shape_secondary.r1cs_shape,
+    )?;
 
     let pk = ProverKey {
       pk_primary,
@@ -815,7 +897,7 @@ where
   ) -> Result<Self, NovaError> {
     // fold the secondary circuit's instance with its running instance
     let (nifs_secondary, (f_U_secondary, f_W_secondary)) = NIFS::prove(
-      &pp.ck_secondary,
+      &*pp.ck_secondary,
       &pp.ro_consts_secondary,
       &scalar_as_base::<E1>(pp.digest()),
       &pp.circuit_shape_secondary.r1cs_shape,

src/provider/hyperkzg.rs

@@ -10,7 +10,7 @@ use crate::{
   errors::NovaError,
   provider::{
     kzg_commitment::KZGCommitmentEngine,
-    non_hiding_kzg::{KZGProverKey, KZGVerifierKey, UniversalKZGParam},
+    non_hiding_kzg::{trim, KZGProverKey, KZGVerifierKey, UniversalKZGParam},
     pedersen::Commitment,
     traits::DlogGroup,
     util::iterators::DoubleEndedIteratorExt as _,
@@ -31,6 +31,7 @@ use pairing::{Engine, MillerLoopResult, MultiMillerLoop};
 use rayon::prelude::*;
 use ref_cast::RefCast as _;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
+use std::sync::Arc;
 
 /// Provides an implementation of a polynomial evaluation argument
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -121,8 +122,9 @@ where
   type ProverKey = KZGProverKey<E>;
   type VerifierKey = KZGVerifierKey<E>;
 
-  fn setup(ck: &UniversalKZGParam<E>) -> (Self::ProverKey, Self::VerifierKey) {
-    ck.trim(ck.length() - 1)
+  fn setup(ck: Arc<UniversalKZGParam<E>>) -> (Self::ProverKey, Self::VerifierKey) {
+    let len = ck.length() - 1;
+    trim(ck, len)
   }
 
   fn prove(
@@ -428,7 +430,9 @@ mod tests {
     let n = 4;
     let ck: CommitmentKey<NE> =
       <KZGCommitmentEngine<E> as CommitmentEngineTrait<NE>>::setup(b"test", n);
-    let (pk, _vk): (KZGProverKey<E>, KZGVerifierKey<E>) = EvaluationEngine::<E, NE>::setup(&ck);
+    let ck = Arc::new(ck);
+    let (pk, _vk): (KZGProverKey<E>, KZGVerifierKey<E>) =
+      EvaluationEngine::<E, NE>::setup(ck.clone());
 
     // poly is in eval. representation; evaluated at [(0,0), (0,1), (1,0), (1,1)]
     let poly = vec![Fr::from(1), Fr::from(2), Fr::from(2), Fr::from(4)];
@@ -464,7 +468,9 @@ mod tests {
     fn test_inner(n: usize, poly: &[Fr], point: &[Fr], eval: Fr) -> Result<(), NovaError> {
       let ck: CommitmentKey<NE> =
         <KZGCommitmentEngine<E> as CommitmentEngineTrait<NE>>::setup(b"test", n);
-      let (pk, vk): (KZGProverKey<E>, KZGVerifierKey<E>) = EvaluationEngine::<E, NE>::setup(&ck);
+      let ck = Arc::new(ck);
+      let (pk, vk): (KZGProverKey<E>, KZGVerifierKey<E>) =
+        EvaluationEngine::<E, NE>::setup(ck.clone());
 
       // make a commitment
       let C = KZGCommitmentEngine::commit(&ck, poly);
@@ -522,7 +528,9 @@ mod tests {
 
     let ck: CommitmentKey<NE> =
       <KZGCommitmentEngine<E> as CommitmentEngineTrait<NE>>::setup(b"test", n);
-    let (pk, vk): (KZGProverKey<E>, KZGVerifierKey<E>) = EvaluationEngine::<E, NE>::setup(&ck);
+    let ck = Arc::new(ck);
+    let (pk, vk): (KZGProverKey<E>, KZGVerifierKey<E>) =
+      EvaluationEngine::<E, NE>::setup(ck.clone());
 
     // make a commitment
     let C = KZGCommitmentEngine::commit(&ck, &poly);