From b0c1dd456b3d149fc8da897f6005ebeb3b35e7ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= <4142+huitseeker@users.noreply.github.com> Date: Tue, 7 Nov 2023 11:16:51 -0500 Subject: [PATCH] refactor: Refactor trait requirements and update CurveCycleEquipped (#847) **Context**: Developing [zeromorph](https://github.com/lurk-lab/arecibo/tree/zeromorph) forced me to abstract several Nova APIs over the used `EvaluationEngineTrait` (which was the point of the whole exercise). And it forced me to realize that Nova APIs that genericise over the group implementations, in order to represent the concept of a given curve cycle, become a lot simpler when they jointly abstract over the associated `EvaluationEngine`. This insight led to https://github.com/microsoft/Nova/pull/234 **This PR**: We prepare the code base from where it is now (hard-coding that the Evaluation Engine used in nova is the IPA, and requiring the corresponding idiosyncratic trait bound `CommitmentKeyExtTrait`) to something that's Zeromorph-ready (recognizing that any curve cycle may have its unspecified choice of `EvaluationEngineTrait` implementations). We reap the associated simplicity benefits. **In Detail**: - Removed Sync and Send trait requirements for multiple associated types across various modules, simplifying the codebase. - Significant updates made to the trait `CurveCycleEquipped` in `nova.rs`. Removed four previous type and introduced two new ones, `EE1` and `EE2`, shifting the focus towards an improved Evaluation Engine. - This change in trait `CurveCycleEquipped` is also reflected in the pallas::Scalar and bn256::Scalar trait implementations. - The necessity for detailed explanations about the removed type aliases in `CurveCycleEquipped` was eliminated. Added brief explanations for the new type aliases. - Removed `Sync + Send` trait bounds from `F::CK1` and `F::CK2` in the `public_params`, `supernova_circuit_params`, and `supernova_aux_params` functions, resulting in simpler functions. --- src/cli/lurk_proof.rs | 4 --- src/cli/repl/meta_cmd.rs | 3 -- src/proof/nova.rs | 58 +++++++++++------------------- src/public_parameters/mem_cache.rs | 2 -- src/public_parameters/mod.rs | 8 ----- 5 files changed, 21 insertions(+), 54 deletions(-) diff --git a/src/cli/lurk_proof.rs b/src/cli/lurk_proof.rs index f1e085b6da..b9e5923a52 100644 --- a/src/cli/lurk_proof.rs +++ b/src/cli/lurk_proof.rs @@ -100,8 +100,6 @@ impl<'a, F: CurveCycleEquipped + Serialize, M: MultiFrameTrait<'a, F, Coproc> where < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, - ::CK1: Sync + Send, - ::CK2: Sync + Send, { #[inline] pub(crate) fn persist(self, proof_key: &str) -> Result<()> { @@ -119,8 +117,6 @@ impl< where < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, - ::CK1: Sync + Send, - ::CK2: Sync + Send, { pub(crate) fn verify_proof(proof_key: &str) -> Result<()> { let lurk_proof: LurkProof<'_, F, Coproc, M> = load(&proof_path(proof_key))?; diff --git a/src/cli/repl/meta_cmd.rs b/src/cli/repl/meta_cmd.rs index 920e4e97ad..cc3aa9e90b 100644 --- a/src/cli/repl/meta_cmd.rs +++ b/src/cli/repl/meta_cmd.rs @@ -367,11 +367,8 @@ impl MetaCmd { impl MetaCmd where - // TODO(huitseeker): this is a bit pedantic, revisit later. < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, - ::CK1: Sync + Send, - ::CK2: Sync + Send, { const VERIFY: MetaCmd = MetaCmd { name: diff --git a/src/proof/nova.rs b/src/proof/nova.rs index f197794d6c..593f825ba3 100644 --- a/src/proof/nova.rs +++ b/src/proof/nova.rs @@ -7,10 +7,9 @@ use ff::Field; use nova::{ errors::NovaError, provider::bn256_grumpkin::{bn256, grumpkin}, - provider::pedersen::CommitmentKeyExtTrait, traits::{ circuit::{StepCircuit, TrivialCircuit}, - commitment::CommitmentEngineTrait, + evaluation::EvaluationEngineTrait, snark::RelaxedR1CSSNARKTrait, Group, }, @@ -45,42 +44,29 @@ use crate::{ /// (currently Pallas/Vesta and BN254/Grumpkin). It being pegged on the `LurkField` trait encodes that we do /// not expect more than one such cycle to be supported at a time for a given field. pub trait CurveCycleEquipped: LurkField { - /// ## Why the next 4 types? - /// - /// The next 4 types are purely technical, and aim at laying out type bounds in a way that rust can find them. - /// They should eventually be replaceable by a bound on projections, once bounds on associated types progress. - /// They are technically equivalent to bounds of - /// >::CommitmentKey: CommitmentKeyExtTrait::CE>, - /// >::CommitmentKey: CommitmentKeyExtTrait::CE>, - /// but where clauses can't be *found* by the compiler at the point where Self::G1, Self::G2 are used - - /// ## OK, but why do we need bounds at all in the first place? + /// ## Why the next 2 types? + + /// In theory it would be sufficient to abstract over the two group types of the curve cycle, but in practice Nova is a + /// bit idiosyncratic in the [`nova::traits::evaluation::EvaluationEngineTrait`], (PCS) it uses on these (its multilinear IPA : [`nova::provider::ipa_pc::EvaluationEngine`]) + /// *and* that implementation requires an additional trait bound `CommitmentKeyExtTrait` for this type. /// - /// As to *why* those see https://github.com/microsoft/Nova/pull/200 - /// and the bound `CommitmentKey: CommitmentKeyExtTrait` on [`nova::provider::ipa_pc::EvaluationEngine`] - /// Essentially, Nova relies on a commitment scheme that is additively homomorphic, but encodes the practicalities of this - /// (properties are unwieldy to encode) in the form of this CommitmentKeyExtTrait. - - /// The type of the commitment key used for points of the first curve in the cycle. - type CK1: CommitmentKeyExtTrait; - /// The type of the commitment key used for points of the second curve in the cycle. - type CK2: CommitmentKeyExtTrait; - /// The commitment engine type for the first curve in the cycle. - type CE1: CommitmentEngineTrait; - /// The commitment engine type for the second curve in the cycle. - type CE2: CommitmentEngineTrait; + /// The following abstracts over curve cycle groups for which there exists an implementation of [`nova::traits::evaluation::EvaluationEngineTrait`], + /// encapsulating these idiosyncracies within Nova. + + /// a concrete implementation of an [`nova::traits::evaluation::EvaluationEngineTrait`] for G1, + type EE1: EvaluationEngineTrait; + /// a concrete implementation of an [`nova::traits::evaluation::EvaluationEngineTrait`] for G2, + type EE2: EvaluationEngineTrait; /// The group type for the first curve in the cycle. - type G1: Group::Scalar, Scalar = Self, CE = Self::CE1>; + type G1: Group::Scalar, Scalar = Self>; /// The group type for the second curve in the cycle. - type G2: Group::Scalar, CE = Self::CE2>; + type G2: Group::Scalar>; } impl CurveCycleEquipped for pallas::Scalar { - type CK1 = nova::provider::pedersen::CommitmentKey; - type CK2 = nova::provider::pedersen::CommitmentKey; - type CE1 = nova::provider::pedersen::CommitmentEngine; - type CE2 = nova::provider::pedersen::CommitmentEngine; + type EE1 = nova::provider::ipa_pc::EvaluationEngine; + type EE2 = nova::provider::ipa_pc::EvaluationEngine; type G1 = pallas::Point; type G2 = vesta::Point; @@ -88,10 +74,8 @@ impl CurveCycleEquipped for pallas::Scalar { // The impl CurveCycleEquipped for vesta::Scalar is academically possible, but voluntarily omitted to avoid confusion. impl CurveCycleEquipped for bn256::Scalar { - type CK1 = nova::provider::pedersen::CommitmentKey; - type CK2 = nova::provider::pedersen::CommitmentKey; - type CE1 = nova::provider::pedersen::CommitmentEngine; - type CE2 = nova::provider::pedersen::CommitmentEngine; + type EE1 = nova::provider::ipa_pc::EvaluationEngine; + type EE2 = nova::provider::ipa_pc::EvaluationEngine; type G1 = bn256::Point; type G2 = grumpkin::Point; @@ -104,9 +88,9 @@ pub type G1 = ::G1; pub type G2 = ::G2; /// Type alias for the Evaluation Engine using G1 group elements. -pub type EE1 = nova::provider::ipa_pc::EvaluationEngine>; +pub type EE1 = ::EE1; /// Type alias for the Evaluation Engine using G2 group elements. -pub type EE2 = nova::provider::ipa_pc::EvaluationEngine>; +pub type EE2 = ::EE2; /// Type alias for the Relaxed R1CS Spartan SNARK using G1 group elements, EE1. // NOTE: this is not a SNARK that uses computational commitments, diff --git a/src/public_parameters/mem_cache.rs b/src/public_parameters/mem_cache.rs index cfa9b32e54..8d4c850ec9 100644 --- a/src/public_parameters/mem_cache.rs +++ b/src/public_parameters/mem_cache.rs @@ -111,8 +111,6 @@ impl PublicParamMemCache { default: Fn, ) -> Result>, Error> where - F::CK1: Sync + Send, - F::CK2: Sync + Send, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, { diff --git a/src/public_parameters/mod.rs b/src/public_parameters/mod.rs index 9e7cd3d5ed..91350594ee 100644 --- a/src/public_parameters/mod.rs +++ b/src/public_parameters/mod.rs @@ -27,8 +27,6 @@ pub fn public_params< instance: &Instance<'static, F, C, M>, ) -> Result>, Error> where - F::CK1: Sync + Send, - F::CK2: Sync + Send, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, { @@ -90,8 +88,6 @@ pub fn supernova_circuit_params< instance: &Instance<'a, F, C, M>, ) -> Result, Error> where - F::CK1: Sync + Send, - F::CK2: Sync + Send, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, { @@ -118,8 +114,6 @@ pub fn supernova_aux_params< instance: &Instance<'a, F, C, M>, ) -> Result, Error> where - F::CK1: Sync + Send, - F::CK2: Sync + Send, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, { @@ -151,8 +145,6 @@ pub fn supernova_public_params< instance_primary: &Instance<'a, F, C, M>, ) -> Result, Error> where - F::CK1: Sync + Send, - F::CK2: Sync + Send, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, < as Group>::Scalar as ff::PrimeField>::Repr: Abomonation, {