Nova forward ports (argumentcomputer#126)

* Simplify Group trait (argumentcomputer#259)

* simplify Group trait

* fix clippy

* further simplifications

* fix Copy

* Cut duplicate code (argumentcomputer#261)

* cut duplicate code

* bound --> bind for clarity

* expose asm feature

* test: check test_pp_digest with asm feature in CI

* expose size of the base field (argumentcomputer#262)

* fix: add mention of halo2curves issue explaining the problem

---------

Co-authored-by: Srinath Setty <srinath@microsoft.com>

.github/workflows/rust.yml

@@ -33,6 +33,9 @@ jobs:
     - name: Linux Tests in parallel, with nextest profile ci and cargo profile dev-ci
       run: |
         cargo nextest run --profile ci --workspace --cargo-profile dev-ci
+    - name: Run test_pp_digest with the asm feature
+      run: |
+        cargo nextest run --profile ci --workspace --cargo-profile dev-ci --features "asm" -E 'test(test_pp_digest)'
 
   check-lurk-compiles:
     if: github.event_name == 'pull_request'

Cargo.toml

@@ -82,6 +82,7 @@ harness = false
 
 [features]
 default = []
+asm = ["halo2curves/asm"]
 # Compiles in portable mode, w/o ISA extensions => binary can be executed on all systems.
 portable = ["pasta-msm/portable"]
 cuda = ["neptune/cuda", "neptune/pasta", "neptune/arity24"]

src/lib.rs

@@ -942,10 +942,14 @@ type CE<G> = <G as Group>::CE;
 
 #[cfg(test)]
 mod tests {
-  use crate::provider::bn256_grumpkin::{bn256, grumpkin};
-  use crate::provider::secp_secq::{secp256k1, secq256k1};
-  use crate::traits::evaluation::EvaluationEngineTrait;
-  use crate::traits::snark::default_ck_hint;
+  use crate::{
+    provider::{
+      bn256_grumpkin::{bn256, grumpkin},
+      secp_secq::{secp256k1, secq256k1},
+      GroupExt,
+    },
+    traits::{evaluation::EvaluationEngineTrait, snark::default_ck_hint},
+  };
   use core::fmt::Write;
 
   use super::*;
@@ -1014,8 +1018,8 @@ mod tests {
 
   fn test_pp_digest_with<G1, G2, T1, T2, E1, E2>(circuit1: &T1, circuit2: &T2, _expected: &str)
   where
-    G1: Group<Base = <G2 as Group>::Scalar>,
-    G2: Group<Base = <G1 as Group>::Scalar>,
+    G1: Group<Base = <G2 as Group>::Scalar> + GroupExt,
+    G2: Group<Base = <G1 as Group>::Scalar> + GroupExt,
     T1: StepCircuit<G1::Scalar>,
     T2: StepCircuit<G2::Scalar>,
     E1: EvaluationEngineTrait<G1>,
@@ -1065,11 +1069,28 @@ mod tests {
     let trivial_circuit2_grumpkin = TrivialCircuit::<<grumpkin::Point as Group>::Scalar>::default();
     let cubic_circuit1_grumpkin = CubicCircuit::<<bn256::Point as Group>::Scalar>::default();
 
+    // These tests should not need be different on the "asm" feature for bn256.
+    // See https://github.com/privacy-scaling-explorations/halo2curves/issues/100 for why they are - closing the issue there
+    // should eliminate the discrepancy here.
+    #[cfg(feature = "asm")]
+    test_pp_digest_with::<bn256::Point, grumpkin::Point, _, _, EE<_>, EE<_>>(
+      &trivial_circuit1_grumpkin,
+      &trivial_circuit2_grumpkin,
+      "c4ecd363a6c1473de7e0d24fc1dbb660f563556e2e13fb4614acdff04cab7701",
+    );
+    #[cfg(feature = "asm")]
+    test_pp_digest_with::<bn256::Point, grumpkin::Point, _, _, EE<_>, EE<_>>(
+      &cubic_circuit1_grumpkin,
+      &trivial_circuit2_grumpkin,
+      "4853a6463b6309f6ae76442934d0a423f51f1e10abaddd0d39bf5644ed589100",
+    );
+    #[cfg(not(feature = "asm"))]
     test_pp_digest_with::<bn256::Point, grumpkin::Point, _, _, EE<_>, EE<_>>(
       &trivial_circuit1_grumpkin,
       &trivial_circuit2_grumpkin,
       "c26cc841d42c19bf98bc2482e66cd30903922f2a923927b85d66f375a821f101",
     );
+    #[cfg(not(feature = "asm"))]
     test_pp_digest_with::<bn256::Point, grumpkin::Point, _, _, EE<_>, EE<_>>(
       &cubic_circuit1_grumpkin,
       &trivial_circuit2_grumpkin,

src/provider/bn256_grumpkin.rs

@@ -6,8 +6,9 @@ use crate::{
     keccak::Keccak256Transcript,
     pedersen::CommitmentEngine,
     poseidon::{PoseidonRO, PoseidonROCircuit},
+    CompressedGroup, GroupExt,
   },
-  traits::{CompressedGroup, Group, PrimeFieldExt, TranscriptReprTrait},
+  traits::{Group, PrimeFieldExt, TranscriptReprTrait},
 };
 use digest::{ExtendableOutput, Update};
 use ff::{FromUniformBytes, PrimeField};
@@ -41,32 +42,22 @@ pub mod grumpkin {
   };
 }
 
-impl<G: Group> TranscriptReprTrait<G> for grumpkin::Base {
-  fn to_transcript_bytes(&self) -> Vec<u8> {
-    self.to_repr().to_vec()
-  }
-}
-
-impl<G: Group> TranscriptReprTrait<G> for grumpkin::Scalar {
-  fn to_transcript_bytes(&self) -> Vec<u8> {
-    self.to_repr().to_vec()
-  }
-}
-
 impl_traits!(
   bn256,
   Bn256Compressed,
   Bn256Point,
   Bn256Affine,
-  "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001"
+  "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001",
+  "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47"
 );
 
 impl_traits!(
   grumpkin,
   GrumpkinCompressed,
   GrumpkinPoint,
   GrumpkinAffine,
-  "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47"
+  "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47",
+  "30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001"
 );
 
 #[cfg(test)]
@@ -94,7 +85,7 @@ mod tests {
     for n in [
       1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 1021,
     ] {
-      let ck_par = <G as Group>::from_label(label, n);
+      let ck_par = <G as GroupExt>::from_label(label, n);
       let ck_ser = from_label_serial(label, n);
       assert_eq!(ck_par.len(), n);
       assert_eq!(ck_ser.len(), n);

src/provider/ipa_pc.rs

@@ -1,7 +1,7 @@
 //! This module implements `EvaluationEngine` using an IPA-based polynomial commitment scheme
 use crate::{
   errors::NovaError,
-  provider::pedersen::CommitmentKeyExtTrait,
+  provider::{pedersen::CommitmentKeyExtTrait, GroupExt},
   spartan::polys::eq::EqPolynomial,
   traits::{
     commitment::{CommitmentEngineTrait, CommitmentTrait},
@@ -42,7 +42,7 @@ pub struct EvaluationEngine<G: Group> {
 
 impl<G> EvaluationEngineTrait<G> for EvaluationEngine<G>
 where
-  G: Group,
+  G: GroupExt,
   CommitmentKey<G>: CommitmentKeyExtTrait<G>,
 {
   type ProverKey = ProverKey<G>;
@@ -164,7 +164,7 @@ pub struct InnerProductArgument<G: Group> {
 
 impl<G> InnerProductArgument<G>
 where
-  G: Group,
+  G: GroupExt,
   CommitmentKey<G>: CommitmentKeyExtTrait<G>,
 {
   const fn protocol_name() -> &'static [u8] {

src/provider/keccak.rs

@@ -155,7 +155,7 @@ mod tests {
   fn test_keccak_example() {
     let mut hasher = Keccak256::new();
     hasher.update(0xffffffff_u32.to_le_bytes());
-    let output: [u8; 32] = hasher.finalize().try_into().unwrap();
+    let output: [u8; 32] = hasher.finalize().into();
     assert_eq!(
       hex::encode(output),
       "29045a592007d0c246ef02c2223570da9522d0cf0f73282c79a1bc8f0bb2c238"

src/provider/mod.rs

@@ -3,6 +3,96 @@
 //! `Group` with pasta curves and BN256/Grumpkin
 //! `RO` traits with Poseidon
 //! `EvaluationEngine` with an IPA-based polynomial evaluation argument
+use crate::traits::{commitment::ScalarMul, Group, TranscriptReprTrait};
+use core::{
+  fmt::Debug,
+  ops::{Add, AddAssign, Sub, SubAssign},
+};
+use serde::{Deserialize, Serialize};
+
+/// Represents a compressed version of a group element
+pub trait CompressedGroup:
+  Clone
+  + Copy
+  + Debug
+  + Eq
+  + Send
+  + Sync
+  + TranscriptReprTrait<Self::GroupElement>
+  + Serialize
+  + for<'de> Deserialize<'de>
+  + 'static
+{
+  /// A type that holds the decompressed version of the compressed group element
+  type GroupElement: Group;
+
+  /// Decompresses the compressed group element
+  fn decompress(&self) -> Option<Self::GroupElement>;
+}
+
+/// A helper trait for types with a group operation.
+pub trait GroupOps<Rhs = Self, Output = Self>:
+  Add<Rhs, Output = Output> + Sub<Rhs, Output = Output> + AddAssign<Rhs> + SubAssign<Rhs>
+{
+}
+
+impl<T, Rhs, Output> GroupOps<Rhs, Output> for T where
+  T: Add<Rhs, Output = Output> + Sub<Rhs, Output = Output> + AddAssign<Rhs> + SubAssign<Rhs>
+{
+}
+
+/// A helper trait for references with a group operation.
+pub trait GroupOpsOwned<Rhs = Self, Output = Self>: for<'r> GroupOps<&'r Rhs, Output> {}
+impl<T, Rhs, Output> GroupOpsOwned<Rhs, Output> for T where T: for<'r> GroupOps<&'r Rhs, Output> {}
+
+/// A helper trait for references implementing group scalar multiplication.
+pub trait ScalarMulOwned<Rhs, Output = Self>: for<'r> ScalarMul<&'r Rhs, Output> {}
+impl<T, Rhs, Output> ScalarMulOwned<Rhs, Output> for T where T: for<'r> ScalarMul<&'r Rhs, Output> {}
+
+/// A trait that defines extensions to the Group trait
+pub trait GroupExt:
+  Group
+  + Serialize
+  + for<'de> Deserialize<'de>
+  + GroupOps
+  + GroupOpsOwned
+  + ScalarMul<<Self as Group>::Scalar>
+  + ScalarMulOwned<<Self as Group>::Scalar>
+{
+  /// A type representing the compressed version of the group element
+  type CompressedGroupElement: CompressedGroup<GroupElement = Self>;
+
+  /// A type representing preprocessed group element
+  type PreprocessedGroupElement: Clone
+    + Debug
+    + PartialEq
+    + Eq
+    + Send
+    + Sync
+    + Serialize
+    + for<'de> Deserialize<'de>;
+
+  /// A method to compute a multiexponentation
+  fn vartime_multiscalar_mul(
+    scalars: &[Self::Scalar],
+    bases: &[Self::PreprocessedGroupElement],
+  ) -> Self;
+
+  /// Produce a vector of group elements using a static label
+  fn from_label(label: &'static [u8], n: usize) -> Vec<Self::PreprocessedGroupElement>;
+
+  /// Compresses the group element
+  fn compress(&self) -> Self::CompressedGroupElement;
+
+  /// Produces a preprocessed element
+  fn preprocessed(&self) -> Self::PreprocessedGroupElement;
+
+  /// Returns an element that is the additive identity of the group
+  fn zero() -> Self;
+
+  /// Returns the affine coordinates (x, y, infinty) for the point
+  fn to_coordinates(&self) -> (Self::Base, Self::Base, bool);
+}
 
 pub mod bn256_grumpkin;
 pub mod ipa_pc;
@@ -135,18 +225,31 @@ macro_rules! impl_traits {
     $name_compressed:ident,
     $name_curve:ident,
     $name_curve_affine:ident,
-    $order_str:literal
+    $order_str:literal,
+    $base_str:literal
   ) => {
     impl Group for $name::Point {
       type Base = $name::Base;
       type Scalar = $name::Scalar;
-      type CompressedGroupElement = $name_compressed;
-      type PreprocessedGroupElement = $name::Affine;
       type RO = PoseidonRO<Self::Base, Self::Scalar>;
       type ROCircuit = PoseidonROCircuit<Self::Base>;
       type TE = Keccak256Transcript<Self>;
       type CE = CommitmentEngine<Self>;
 
+      fn get_curve_params() -> (Self::Base, Self::Base, BigInt, BigInt) {
+        let A = $name::Point::a();
+        let B = $name::Point::b();
+        let order = BigInt::from_str_radix($order_str, 16).unwrap();
+        let base = BigInt::from_str_radix($base_str, 16).unwrap();
+
+        (A, B, order, base)
+      }
+    }
+
+    impl GroupExt for $name::Point {
+      type CompressedGroupElement = $name_compressed;
+      type PreprocessedGroupElement = $name::Affine;
+
       fn vartime_multiscalar_mul(
         scalars: &[Self::Scalar],
         bases: &[Self::PreprocessedGroupElement],
@@ -208,33 +311,21 @@ macro_rules! impl_traits {
         }
       }
 
+      fn zero() -> Self {
+        $name::Point::identity()
+      }
+
       fn to_coordinates(&self) -> (Self::Base, Self::Base, bool) {
         // see: grumpkin implementation at src/provider/bn256_grumpkin.rs
         let coordinates = self.to_affine().coordinates();
         if coordinates.is_some().unwrap_u8() == 1
-          && (Self::PreprocessedGroupElement::identity() != self.to_affine())
+          && ($name_curve_affine::identity() != self.to_affine())
         {
           (*coordinates.unwrap().x(), *coordinates.unwrap().y(), false)
         } else {
           (Self::Base::zero(), Self::Base::zero(), true)
         }
       }
-
-      fn get_curve_params() -> (Self::Base, Self::Base, BigInt) {
-        let A = $name::Point::a();
-        let B = $name::Point::b();
-        let order = BigInt::from_str_radix($order_str, 16).unwrap();
-
-        (A, B, order)
-      }
-
-      fn zero() -> Self {
-        $name::Point::identity()
-      }
-
-      fn get_generator() -> Self {
-        $name::Point::generator()
-      }
     }
 
     impl PrimeFieldExt for $name::Scalar {
@@ -257,6 +348,12 @@ macro_rules! impl_traits {
         Some($name_curve::from_bytes(&self).unwrap())
       }
     }
+
+    impl<G: Group> TranscriptReprTrait<G> for $name::Scalar {
+      fn to_transcript_bytes(&self) -> Vec<u8> {
+        self.to_repr().to_vec()
+      }
+    }
   };
 }