refactor: removed ROConstantsTrait

- trait refactored to Default in associated type,
- obviates the need for a separate trait,
- refactored call sites and only implementation

Author: huitseeker

src/circuit.rs

@@ -378,7 +378,7 @@ mod tests {
     bellpepper::r1cs::{NovaShape, NovaWitness},
     gadgets::utils::scalar_as_base,
     provider::poseidon::PoseidonConstantsCircuit,
-    traits::{circuit::TrivialTestCircuit, ROConstantsTrait},
+    traits::circuit::TrivialTestCircuit,
   };
 
   // In the following we use 1 to refer to the primary, and 2 to refer to the secondary circuit
@@ -454,8 +454,8 @@ mod tests {
   fn test_recursive_circuit_pasta() {
     let params1 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, true);
     let params2 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, false);
-    let ro_consts1: ROConstantsCircuit<PastaG2> = PoseidonConstantsCircuit::new();
-    let ro_consts2: ROConstantsCircuit<PastaG1> = PoseidonConstantsCircuit::new();
+    let ro_consts1: ROConstantsCircuit<PastaG2> = PoseidonConstantsCircuit::default();
+    let ro_consts2: ROConstantsCircuit<PastaG1> = PoseidonConstantsCircuit::default();
 
     test_recursive_circuit_with::<PastaG1, PastaG2>(
       &params1, &params2, ro_consts1, ro_consts2, 9815, 10347,
@@ -467,9 +467,9 @@ mod tests {
     let params1 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, true);
     let params2 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, false);
     let ro_consts1: ROConstantsCircuit<provider::bn256_grumpkin::grumpkin::Point> =
-      PoseidonConstantsCircuit::new();
+      PoseidonConstantsCircuit::default();
     let ro_consts2: ROConstantsCircuit<provider::bn256_grumpkin::bn256::Point> =
-      PoseidonConstantsCircuit::new();
+      PoseidonConstantsCircuit::default();
 
     test_recursive_circuit_with::<
       provider::bn256_grumpkin::bn256::Point,
@@ -482,9 +482,9 @@ mod tests {
     let params1 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, true);
     let params2 = NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, false);
     let ro_consts1: ROConstantsCircuit<provider::secp_secq::secq256k1::Point> =
-      PoseidonConstantsCircuit::new();
+      PoseidonConstantsCircuit::default();
     let ro_consts2: ROConstantsCircuit<provider::secp_secq::secp256k1::Point> =
-      PoseidonConstantsCircuit::new();
+      PoseidonConstantsCircuit::default();
 
     test_recursive_circuit_with::<
       provider::secp_secq::secp256k1::Point,

src/lib.rs

@@ -45,7 +45,7 @@ use traits::{
   circuit::StepCircuit,
   commitment::{CommitmentEngineTrait, CommitmentTrait},
   snark::RelaxedR1CSSNARKTrait,
-  AbsorbInROTrait, Group, ROConstants, ROConstantsCircuit, ROConstantsTrait, ROTrait,
+  AbsorbInROTrait, Group, ROConstants, ROConstantsCircuit, ROTrait,
 };
 
 /// A type that holds public parameters of Nova
@@ -89,15 +89,15 @@ where
     let augmented_circuit_params_secondary =
       NovaAugmentedCircuitParams::new(BN_LIMB_WIDTH, BN_N_LIMBS, false);
 
-    let ro_consts_primary: ROConstants<G1> = ROConstants::<G1>::new();
-    let ro_consts_secondary: ROConstants<G2> = ROConstants::<G2>::new();
+    let ro_consts_primary: ROConstants<G1> = ROConstants::<G1>::default();
+    let ro_consts_secondary: ROConstants<G2> = ROConstants::<G2>::default();
 
     let F_arity_primary = c_primary.arity();
     let F_arity_secondary = c_secondary.arity();
 
     // ro_consts_circuit_primary are parameterized by G2 because the type alias uses G2::Base = G1::Scalar
-    let ro_consts_circuit_primary: ROConstantsCircuit<G2> = ROConstantsCircuit::<G2>::new();
-    let ro_consts_circuit_secondary: ROConstantsCircuit<G1> = ROConstantsCircuit::<G1>::new();
+    let ro_consts_circuit_primary: ROConstantsCircuit<G2> = ROConstantsCircuit::<G2>::default();
+    let ro_consts_circuit_secondary: ROConstantsCircuit<G1> = ROConstantsCircuit::<G1>::default();
 
     // Initialize ck for the primary
     let circuit_primary: NovaAugmentedCircuit<'_, G2, C1> = NovaAugmentedCircuit::new(

src/nifs.rs

@@ -118,10 +118,7 @@ impl<G: Group> NIFS<G> {
 #[cfg(test)]
 mod tests {
   use super::*;
-  use crate::{
-    r1cs::R1CS,
-    traits::{Group, ROConstantsTrait},
-  };
+  use crate::{r1cs::R1CS, traits::Group};
   use ::bellpepper_core::{num::AllocatedNum, ConstraintSystem, SynthesisError};
   use ff::{Field, PrimeField};
   use rand::rngs::OsRng;
@@ -176,7 +173,7 @@ mod tests {
     let _ = synthesize_tiny_r1cs_bellpepper(&mut cs, None);
     let (shape, ck) = cs.r1cs_shape();
     let ro_consts =
-      <<G as Group>::RO as ROTrait<<G as Group>::Base, <G as Group>::Scalar>>::Constants::new();
+      <<G as Group>::RO as ROTrait<<G as Group>::Base, <G as Group>::Scalar>>::Constants::default();
 
     // Now get the instance and assignment for one instance
     let mut cs: SatisfyingAssignment<G> = SatisfyingAssignment::new();
@@ -328,7 +325,7 @@ mod tests {
     // generate generators and ro constants
     let ck = R1CS::<G>::commitment_key(&S);
     let ro_consts =
-      <<G as Group>::RO as ROTrait<<G as Group>::Base, <G as Group>::Scalar>>::Constants::new();
+      <<G as Group>::RO as ROTrait<<G as Group>::Base, <G as Group>::Scalar>>::Constants::default();
 
     let rand_inst_witness_generator =
       |ck: &CommitmentKey<G>, I: &G::Scalar| -> (G::Scalar, R1CSInstance<G>, R1CSWitness<G>) {

src/provider/poseidon.rs

@@ -1,5 +1,5 @@
 //! Poseidon Constants and Poseidon-based RO used in Nova
-use crate::traits::{ROCircuitTrait, ROConstantsTrait, ROTrait};
+use crate::traits::{ROCircuitTrait, ROTrait};
 use bellpepper_core::{
   boolean::{AllocatedBit, Boolean},
   num::AllocatedNum,
@@ -24,13 +24,9 @@ use serde::{Deserialize, Serialize};
 #[derive(Clone, Serialize, Deserialize)]
 pub struct PoseidonConstantsCircuit<Scalar: PrimeField>(PoseidonConstants<Scalar, U24>);
 
-impl<Scalar> ROConstantsTrait<Scalar> for PoseidonConstantsCircuit<Scalar>
-where
-  Scalar: PrimeField + PrimeFieldBits,
-{
+impl<Scalar: PrimeField> Default for PoseidonConstantsCircuit<Scalar> {
   /// Generate Poseidon constants
-  #[allow(clippy::new_without_default)]
-  fn new() -> Self {
+  fn default() -> Self {
     Self(Sponge::<Scalar, U24>::api_constants(Strength::Standard))
   }
 }
@@ -39,8 +35,8 @@ where
 #[derive(Serialize, Deserialize)]
 pub struct PoseidonRO<Base, Scalar>
 where
-  Base: PrimeField + PrimeFieldBits,
-  Scalar: PrimeField + PrimeFieldBits,
+  Base: PrimeField,
+  Scalar: PrimeField,
 {
   // Internal State
   state: Vec<Base>,
@@ -53,8 +49,9 @@ where
 impl<Base, Scalar> ROTrait<Base, Scalar> for PoseidonRO<Base, Scalar>
 where
   Base: PrimeField + PrimeFieldBits + Serialize + for<'de> Deserialize<'de>,
-  Scalar: PrimeField + PrimeFieldBits,
+  Scalar: PrimeField,
 {
+  type CircuitRO = PoseidonROCircuit<Base>;
   type Constants = PoseidonConstantsCircuit<Base>;
 
   fn new(constants: PoseidonConstantsCircuit<Base>, num_absorbs: usize) -> Self {
@@ -123,6 +120,7 @@ impl<Scalar> ROCircuitTrait<Scalar> for PoseidonROCircuit<Scalar>
 where
   Scalar: PrimeField + PrimeFieldBits + Serialize + for<'de> Deserialize<'de>,
 {
+  type VanillaRO<T: PrimeField> = PoseidonRO<Scalar, T>;
   type Constants = PoseidonConstantsCircuit<Scalar>;
 
   /// Initialize the internal state and set the poseidon constants
@@ -217,7 +215,7 @@ mod tests {
   {
     // Check that the number computed inside the circuit is equal to the number computed outside the circuit
     let mut csprng: OsRng = OsRng;
-    let constants = PoseidonConstantsCircuit::<G::Scalar>::new();
+    let constants = PoseidonConstantsCircuit::<G::Scalar>::default();
     let num_absorbs = 32;
     let mut ro: PoseidonRO<G::Scalar, G::Base> = PoseidonRO::new(constants.clone(), num_absorbs);
     let mut ro_gadget: PoseidonROCircuit<G::Scalar> =

src/traits/mod.rs

@@ -117,14 +117,12 @@ pub trait AbsorbInROTrait<G: Group> {
 }
 
 /// A helper trait that defines the behavior of a hash function that we use as an RO
-pub trait ROTrait<Base, Scalar> {
+pub trait ROTrait<Base: PrimeField, Scalar> {
+  /// The circuit alter ego of this trait impl - this constrains it to use the same constants
+  type CircuitRO: ROCircuitTrait<Base, Constants = Self::Constants>;
+
   /// A type representing constants/parameters associated with the hash function
-  type Constants: ROConstantsTrait<Base>
-    + Clone
-    + Send
-    + Sync
-    + Serialize
-    + for<'de> Deserialize<'de>;
+  type Constants: Default + Clone + Send + Sync + Serialize + for<'de> Deserialize<'de>;
 
   /// Initializes the hash function
   fn new(constants: Self::Constants, num_absorbs: usize) -> Self;
@@ -138,13 +136,11 @@ pub trait ROTrait<Base, Scalar> {
 
 /// A helper trait that defines the behavior of a hash function that we use as an RO in the circuit model
 pub trait ROCircuitTrait<Base: PrimeField> {
-  /// A type representing constants/parameters associated with the hash function
-  type Constants: ROConstantsTrait<Base>
-    + Clone
-    + Send
-    + Sync
-    + Serialize
-    + for<'de> Deserialize<'de>;
+  /// the vanilla alter ego of this trait - this constrains it to use the same constants
+  type VanillaRO<T: PrimeField>: ROTrait<Base, T, Constants = Self::Constants>;
+
+  /// A type representing constants/parameters associated with the hash function on this Base field
+  type Constants: Default + Clone + Send + Sync + Serialize + for<'de> Deserialize<'de>;
 
   /// Initializes the hash function
   fn new(constants: Self::Constants, num_absorbs: usize) -> Self;
@@ -158,12 +154,6 @@ pub trait ROCircuitTrait<Base: PrimeField> {
     CS: ConstraintSystem<Base>;
 }
 
-/// A helper trait that defines the constants associated with a hash function
-pub trait ROConstantsTrait<Base> {
-  /// produces constants/parameters associated with the hash function
-  fn new() -> Self;
-}
-
 /// An alias for constants associated with G::RO
 pub type ROConstants<G> =
   <<G as Group>::RO as ROTrait<<G as Group>::Base, <G as Group>::Scalar>>::Constants;