Simplifications to trait structures (microsoft#219)

* refactor: Remove redundant/uneeded traits from various modules

- Removed redundant trait bounds (e.g. PrimeFieldBits implies PrimeField, Group implies Serialize + for<'de> Deserialize),
- Removed some uneeded trait bounds (e.g. no one serializes instances of the `CommitmentEngineTrait`),
- this passes Nova tests of course, but Lurk tests built on this pass as well (relevant since many of these were introduced in microsoft#123)

* chore: Refactor traits and integrate 'group' module

- use traits from 'group' where they are a direct replacement

* refactor: Refactor bn256_grumpkin module and update dependencies

- Modified the `bn256_grumpkin.rs` imports, shifting the use of the `group` module out of `pasta_curves` scope.
- annotated future simplifications for the provider traits

* chore: Update `halo2curves` dependency

- Updated the major dependency `halo2curves` to the latest version

* refactor: Remove needless Phantom in CommitmentKey

- Eliminated `_p: PhantomData<G>` from `CommitmentKey` structure within the `pedersen.rs` source file.
- Purged `_p: Default::default(),` from every method under the `CommitmentEngine` trait implementation for `CommitmentKey<G>`.

* refactor: Refactor CommitmentKeyExtTrait and adjust impls

- The trait pedersen::CommitmentKeyExtTrait no longer requires an associated CommitmentEngine,
- The struct pedersen::CommitmentKey only implements pedersen::CommitmentKeyExtTrait when the group's CommitmentEngineTrait implementation is pedersen::CommitmentEngine

This amounts to shifting the following bounds in the code base:
"we require a `Group` which associated type `CE` has an associated `CommitmentKey` which implements `pedersen::CommitmentKeyExtTrait` with the same choice of associated type `CE` as the group itself"
and "we provide a `pedersen::CommitmentKeyExtTrait<G>` implementation for any `pedersen::CommitmentKey<G>`"
to:
"we require a `Group` which associated type `CE` has an associated `CommitmentKey` which implements `pedersen::CommitmentKeyExtTrait`"
and "we provide a `pedersen::CommitmentKeyExtTrait<G>` implementation for those groups which associated type `CE` is `pedersen::CommitmentKeyExtTrait`.

The claim is that the later is simpler.

This is a breaking change only for those code bases that program generically over nova using the public `CommitmentKeyExtTrait` (since 0.23). At least for lurk, the adaptation is trivial.

* refactor: Refactor InnerProductArgument struct in ipa_pc.rs

- Removed unnecessary usage of `_p: PhantomData<G>` from the `InnerProductArgument` struct.
- Correspondingly, eliminated `_p: Default::default()` from the struct's instantiation process.

* refactor: Replace default values of `Default::default()` for phantom types with `PhantomData` throughout codebase

* refactor: Refactor EvaluationEngineTrait

Removes the uneeded CE associated type

* Revert "chore: Refactor traits and integrate 'group' module"

This reverts commit ae1440b.

Author: huitseeker

Cargo.toml

@@ -32,7 +32,8 @@ bincode = "1.3"
 bitvec = "1.0"
 byteorder = "1.4.3"
 thiserror = "1.0"
-halo2curves = { version = "0.1.0", features = ["derive_serde"] }
+halo2curves = { version = "0.4.0", features = ["derive_serde"] }
+group = "0.13.0"
 
 [target.'cfg(any(target_arch = "x86_64", target_arch = "aarch64"))'.dependencies]
 pasta-msm = { version = "0.1.4" }

benches/compressed-snark.rs

@@ -233,7 +233,7 @@ where
   pub fn new(num_cons: usize) -> Self {
     Self {
       num_cons,
-      _p: Default::default(),
+      _p: PhantomData,
     }
   }
 }

benches/compute-digest.rs

@@ -45,7 +45,7 @@ where
   pub fn new(num_cons: usize) -> Self {
     Self {
       num_cons,
-      _p: Default::default(),
+      _p: PhantomData,
     }
   }
 }

benches/recursive-snark.rs

@@ -136,7 +136,7 @@ where
   pub fn new(num_cons: usize) -> Self {
     Self {
       num_cons,
-      _p: Default::default(),
+      _p: PhantomData,
     }
   }
 }

benches/sha256.rs

@@ -34,7 +34,7 @@ impl<Scalar: PrimeField + PrimeFieldBits> Sha256Circuit<Scalar> {
   pub fn new(preimage: Vec<u8>) -> Self {
     Self {
       preimage,
-      _p: Default::default(),
+      _p: PhantomData,
     }
   }
 }

src/lib.rs

@@ -1087,10 +1087,8 @@ mod tests {
     G1: Group<Base = <G2 as Group>::Scalar>,
     G2: Group<Base = <G1 as Group>::Scalar>,
     // this is due to the reliance on CommitmentKeyExtTrait as a bound in ipa_pc
-    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey:
-      CommitmentKeyExtTrait<G1, CE = <G1 as Group>::CE>,
-    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey:
-      CommitmentKeyExtTrait<G2, CE = <G2 as Group>::CE>,
+    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey: CommitmentKeyExtTrait<G1>,
+    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey: CommitmentKeyExtTrait<G2>,
   {
     let circuit_primary = TrivialTestCircuit::default();
     let circuit_secondary = CubicCircuit::default();
@@ -1182,10 +1180,8 @@ mod tests {
     G1: Group<Base = <G2 as Group>::Scalar>,
     G2: Group<Base = <G1 as Group>::Scalar>,
     // this is due to the reliance on CommitmentKeyExtTrait as a bound in ipa_pc
-    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey:
-      CommitmentKeyExtTrait<G1, CE = <G1 as Group>::CE>,
-    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey:
-      CommitmentKeyExtTrait<G2, CE = <G2 as Group>::CE>,
+    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey: CommitmentKeyExtTrait<G1>,
+    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey: CommitmentKeyExtTrait<G2>,
   {
     let circuit_primary = TrivialTestCircuit::default();
     let circuit_secondary = CubicCircuit::default();
@@ -1280,10 +1276,8 @@ mod tests {
     G1: Group<Base = <G2 as Group>::Scalar>,
     G2: Group<Base = <G1 as Group>::Scalar>,
     // this is due to the reliance on CommitmentKeyExtTrait as a bound in ipa_pc
-    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey:
-      CommitmentKeyExtTrait<G1, CE = <G1 as Group>::CE>,
-    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey:
-      CommitmentKeyExtTrait<G2, CE = <G2 as Group>::CE>,
+    <G1::CE as CommitmentEngineTrait<G1>>::CommitmentKey: CommitmentKeyExtTrait<G1>,
+    <G2::CE as CommitmentEngineTrait<G2>>::CommitmentKey: CommitmentKeyExtTrait<G2>,
   {
     // y is a non-deterministic advice representing the fifth root of the input at a step.
     #[derive(Clone, Debug)]

src/nifs.rs

@@ -72,7 +72,7 @@ impl<G: Group> NIFS<G> {
     Ok((
       Self {
         comm_T: comm_T.compress(),
-        _p: Default::default(),
+        _p: PhantomData,
       },
       (U, W),
     ))

src/provider/bn256_grumpkin.rs

@@ -10,13 +10,11 @@ use crate::{
 };
 use digest::{ExtendableOutput, Update};
 use ff::{FromUniformBytes, PrimeField};
+use group::{cofactor::CofactorCurveAffine, Curve, Group as AnotherGroup, GroupEncoding};
 use num_bigint::BigInt;
 use num_traits::Num;
-use pasta_curves::{
-  self,
-  arithmetic::{CurveAffine, CurveExt},
-  group::{cofactor::CofactorCurveAffine, Curve, Group as AnotherGroup, GroupEncoding},
-};
+// Remove this when https://github.com/zcash/pasta_curves/issues/41 resolves
+use pasta_curves::arithmetic::{CurveAffine, CurveExt};
 use rayon::prelude::*;
 use sha3::Shake256;
 use std::io::Read;

src/provider/ipa_pc.rs

@@ -48,15 +48,14 @@ pub struct EvaluationEngine<G: Group> {
 impl<G> EvaluationEngineTrait<G> for EvaluationEngine<G>
 where
   G: Group,
-  CommitmentKey<G>: CommitmentKeyExtTrait<G, CE = G::CE>,
+  CommitmentKey<G>: CommitmentKeyExtTrait<G>,
 {
-  type CE = G::CE;
   type ProverKey = ProverKey<G>;
   type VerifierKey = VerifierKey<G>;
   type EvaluationArgument = EvaluationArgument<G>;
 
   fn setup(
-    ck: &<Self::CE as CommitmentEngineTrait<G>>::CommitmentKey,
+    ck: &<<G as Group>::CE as CommitmentEngineTrait<G>>::CommitmentKey,
   ) -> (Self::ProverKey, Self::VerifierKey) {
     let pk = ProverKey {
       ck_s: G::CE::setup(b"ipa", 1),
@@ -169,13 +168,12 @@ struct InnerProductArgument<G: Group> {
   L_vec: Vec<CompressedCommitment<G>>,
   R_vec: Vec<CompressedCommitment<G>>,
   a_hat: G::Scalar,
-  _p: PhantomData<G>,
 }
 
 impl<G> InnerProductArgument<G>
 where
   G: Group,
-  CommitmentKey<G>: CommitmentKeyExtTrait<G, CE = G::CE>,
+  CommitmentKey<G>: CommitmentKeyExtTrait<G>,
 {
   const fn protocol_name() -> &'static [u8] {
     b"IPA"
@@ -290,7 +288,6 @@ where
       L_vec,
       R_vec,
       a_hat: a_vec[0],
-      _p: Default::default(),
     })
   }
 

src/provider/keccak.rs

@@ -55,7 +55,7 @@ impl<G: Group> TranscriptEngineTrait<G> for Keccak256Transcript<G> {
       round: 0u16,
       state: output,
       transcript: keccak_instance,
-      _p: Default::default(),
+      _p: PhantomData,
     }
   }