Unify pcs interfaces, fix IPA and make Honk commitment agnostic (#445)

* wip * wip * ipa changes * ipa tweak * srs fix 3 * more wip * stuff * link to new issue * resolve pr comments --------- Co-authored-by: maramihali <[email protected]> Co-authored-by: zac-williamson <[email protected]>
AztecProtocol · Jun 1, 2023 · 832b7fd · 832b7fd
1 parent fbbb342
commit 832b7fd
Show file tree

Hide file tree

Showing 30 changed files with 526 additions and 556 deletions.
diff --git a/cpp/src/barretenberg/ecc/curves/bn254/scalar_multiplication/scalar_multiplication.cpp b/cpp/src/barretenberg/ecc/curves/bn254/scalar_multiplication/scalar_multiplication.cpp
@@ -101,6 +101,10 @@
 namespace barretenberg {
 namespace scalar_multiplication {
 
+/**
+ * The pippppenger point table computes for each point P = (x,y), a point P' = (\beta * x, -y) which enables us
+ * to use the curve endomorphism for faster scalar multiplication. See below for more details.
+ */
 void generate_pippenger_point_table(g1::affine_element* points, g1::affine_element* table, size_t num_points)
 {
     // iterate backwards, so that `points` and `table` can point to the same memory location
@@ -916,14 +920,14 @@ g1::element pippenger(fr* scalars,
  * We use affine-addition formula in this method, which paradoxically is ~45% faster than the mixed addition formulae.
  * See `scalar_multiplication.cpp` for a more detailed description.
  *
- * It's...unsafe, because we assume that the incomplete addition formula exceptions are not triggered.
+ * It's...unsafe, because we assume that the incomplete addition formula exceptions are not triggered i.e. that all the
+ * points provided as arguments to the msm are distinct.
  * We don't bother to check for this to avoid conditional branches in a critical section of our code.
- * This is fine for situations where your bases are linearly independent (i.e. KZG10 polynomial commitments),
- * because triggering the incomplete addition exceptions is about as hard as solving the disrete log problem.
- *
- * This is ok for the prover, but GIANT RED CLAXON WARNINGS FOR THE VERIFIER
- * Don't use this in a verification algorithm! That would be a really bad idea.
- * Unless you're a malicious adversary, then it would be a great idea!
+ * This is fine for situations where your bases are linearly independent (i.e. KZG10 polynomial commitments where
+ * there should be no equal points in the SRS), because triggering the incomplete addition exceptions is about as hard
+ *as solving the disrete log problem. This is ok for the prover, but GIANT RED CLAXON WARNINGS FOR THE VERIFIER Don't
+ *use this in a verification algorithm! That would be a really bad idea. Unless you're a malicious adversary, then it
+ *would be a great idea!
  *
  **/
 g1::element pippenger_unsafe(fr* scalars,

diff --git a/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.cpp b/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.cpp
@@ -43,13 +43,11 @@ std::shared_ptr<StandardHonkComposerHelper::ProvingKey> StandardHonkComposerHelp
  */
 
 std::shared_ptr<StandardHonkComposerHelper::VerificationKey> StandardHonkComposerHelper::compute_verification_key_base(
-    std::shared_ptr<StandardHonkComposerHelper::ProvingKey> const& proving_key,
-    std::shared_ptr<VerifierReferenceString> const& vrs)
+    std::shared_ptr<StandardHonkComposerHelper::ProvingKey> const& proving_key)
 {
     auto key = std::make_shared<VerificationKey>(
-        proving_key->circuit_size, proving_key->num_public_inputs, vrs, proving_key->composer_type);
-    // TODO(kesha): Dirty hack for now. Need to actually make commitment-agnositc
-    auto commitment_key = pcs::kzg::CommitmentKey(proving_key->circuit_size, "../srs_db/ignition");
+        proving_key->circuit_size, proving_key->num_public_inputs, proving_key->composer_type);
+    auto commitment_key = PCSParams::CommitmentKey(proving_key->circuit_size, "../srs_db/ignition");
 
     // Compute and store commitments to all precomputed polynomials
     key->q_m = commitment_key.commit(proving_key->q_m);
@@ -135,8 +133,7 @@ std::shared_ptr<StandardHonkComposerHelper::VerificationKey> StandardHonkCompose
         compute_proving_key(circuit_constructor);
     }
 
-    verification_key =
-        StandardHonkComposerHelper::compute_verification_key_base(proving_key, crs_factory_->get_verifier_crs());
+    verification_key = StandardHonkComposerHelper::compute_verification_key_base(proving_key);
     verification_key->composer_type = proving_key->composer_type;
 
     return verification_key;
@@ -147,10 +144,10 @@ StandardVerifier StandardHonkComposerHelper::create_verifier(const CircuitConstr
     compute_verification_key(circuit_constructor);
     StandardVerifier output_state(verification_key);
 
-    // TODO(Cody): This should be more generic
-    auto kate_verification_key = std::make_unique<pcs::kzg::VerificationKey>("../srs_db/ignition");
+    auto pcs_verification_key =
+        std::make_unique<PCSParams::VerificationKey>(verification_key->circuit_size, "../srs_db/ignition");
 
-    output_state.kate_verification_key = std::move(kate_verification_key);
+    output_state.pcs_verification_key = std::move(pcs_verification_key);
 
     return output_state;
 }
@@ -159,9 +156,13 @@ StandardProver StandardHonkComposerHelper::create_prover(const CircuitConstructo
 {
     compute_proving_key(circuit_constructor);
     compute_witness(circuit_constructor);
-
     StandardProver output_state(proving_key);
 
+    auto pcs_commitment_key =
+        std::make_unique<PCSParams::CommitmentKey>(proving_key->circuit_size, "../srs_db/ignition");
+
+    output_state.pcs_commitment_key = std::move(pcs_commitment_key);
+
     return output_state;
 }
 } // namespace proof_system::honk
diff --git a/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.hpp b/cpp/src/barretenberg/honk/composer/composer_helper/standard_honk_composer_helper.hpp
@@ -15,6 +15,7 @@ namespace proof_system::honk {
 class StandardHonkComposerHelper {
   public:
     using Flavor = flavor::Standard;
+    using PCSParams = Flavor::PCSParams;
     using CircuitConstructor = Flavor::CircuitConstructor;
     using ProvingKey = Flavor::ProvingKey;
     using VerificationKey = Flavor::VerificationKey;
@@ -63,7 +64,7 @@ class StandardHonkComposerHelper {
     // This needs to be static as it may be used only to compute the selector commitments.
 
     static std::shared_ptr<VerificationKey> compute_verification_key_base(
-        std::shared_ptr<ProvingKey> const& proving_key, std::shared_ptr<VerifierReferenceString> const& vrs);
+        std::shared_ptr<ProvingKey> const& proving_key);
 
     void compute_witness(const CircuitConstructor& circuit_constructor, const size_t minimum_circuit_size = 0);
 };

diff --git a/cpp/src/barretenberg/honk/composer/composer_helper/ultra_honk_composer_helper.cpp b/cpp/src/barretenberg/honk/composer/composer_helper/ultra_honk_composer_helper.cpp
@@ -143,6 +143,11 @@ UltraProver UltraHonkComposerHelper::create_prover(CircuitConstructor& circuit_c
 
     UltraProver output_state(proving_key);
 
+    auto pcs_commitment_key =
+        std::make_unique<PCSParams::CommitmentKey>(proving_key->circuit_size, "../srs_db/ignition");
+
+    output_state.pcs_commitment_key = std::move(pcs_commitment_key);
+
     return output_state;
 }
 
@@ -158,10 +163,10 @@ UltraVerifier UltraHonkComposerHelper::create_verifier(const CircuitConstructor&
 
     UltraVerifier output_state(verification_key);
 
-    // TODO(Cody): This should be more generic
-    auto kate_verification_key = std::make_unique<pcs::kzg::VerificationKey>("../srs_db/ignition");
+    auto pcs_verification_key =
+        std::make_unique<PCSVerificationKey>(verification_key->circuit_size, "../srs_db/ignition");
 
-    output_state.kate_verification_key = std::move(kate_verification_key);
+    output_state.pcs_verification_key = std::move(pcs_verification_key);
 
     return output_state;
 }
@@ -310,13 +315,10 @@ std::shared_ptr<UltraHonkComposerHelper::VerificationKey> UltraHonkComposerHelpe
         compute_proving_key(circuit_constructor);
     }
 
-    verification_key = std::make_shared<UltraHonkComposerHelper::VerificationKey>(proving_key->circuit_size,
-                                                                                  proving_key->num_public_inputs,
-                                                                                  crs_factory_->get_verifier_crs(),
-                                                                                  proving_key->composer_type);
+    verification_key = std::make_shared<UltraHonkComposerHelper::VerificationKey>(
+        proving_key->circuit_size, proving_key->num_public_inputs, proving_key->composer_type);
 
-    // TODO(kesha): Dirty hack for now. Need to actually make commitment-agnositc
-    auto commitment_key = pcs::kzg::CommitmentKey(proving_key->circuit_size, "../srs_db/ignition");
+    auto commitment_key = PCSCommitmentKey(proving_key->circuit_size, "../srs_db/ignition");
 
     // Compute and store commitments to all precomputed polynomials
     verification_key->q_m = commitment_key.commit(proving_key->q_m);

diff --git a/cpp/src/barretenberg/honk/composer/composer_helper/ultra_honk_composer_helper.hpp b/cpp/src/barretenberg/honk/composer/composer_helper/ultra_honk_composer_helper.hpp
@@ -19,6 +19,10 @@ class UltraHonkComposerHelper {
     using CircuitConstructor = Flavor::CircuitConstructor;
     using ProvingKey = Flavor::ProvingKey;
     using VerificationKey = Flavor::VerificationKey;
+    using PCSParams = Flavor::PCSParams;
+    using PCS = Flavor::PCS;
+    using PCSCommitmentKey = PCSParams::CommitmentKey;
+    using PCSVerificationKey = PCSParams::VerificationKey;
 
     static constexpr size_t NUM_RESERVED_GATES = 4; // equal to the number of multilinear evaluations leaked
     static constexpr size_t NUM_WIRES = CircuitConstructor::NUM_WIRES;

diff --git a/cpp/src/barretenberg/honk/flavor/standard.hpp b/cpp/src/barretenberg/honk/flavor/standard.hpp
@@ -7,6 +7,7 @@
 #include <vector>
 #include "barretenberg/honk/pcs/commitment_key.hpp"
 #include "barretenberg/honk/sumcheck/polynomials/barycentric_data.hpp"
+#include "barretenberg/honk/pcs/kzg/kzg.hpp"
 #include "barretenberg/honk/sumcheck/polynomials/univariate.hpp"
 #include "barretenberg/ecc/curves/bn254/g1.hpp"
 #include "barretenberg/honk/sumcheck/relations/arithmetic_relation.hpp"
@@ -40,6 +41,7 @@ class Standard {
     using Commitment = G1::affine_element;
     using CommitmentHandle = G1::affine_element;
     using PCSParams = pcs::kzg::Params;
+    using PCS = pcs::kzg::KZG<PCSParams>;
 
     static constexpr size_t NUM_WIRES = CircuitConstructor::NUM_WIRES;
     // The number of multivariate polynomials on which a sumcheck prover sumcheck operates (including shifts). We often

diff --git a/cpp/src/barretenberg/honk/flavor/ultra.hpp b/cpp/src/barretenberg/honk/flavor/ultra.hpp
@@ -7,6 +7,7 @@
 #include <vector>
 #include "barretenberg/honk/pcs/commitment_key.hpp"
 #include "barretenberg/honk/sumcheck/polynomials/barycentric_data.hpp"
+#include "barretenberg/honk/pcs/ipa/ipa.hpp"
 #include "barretenberg/honk/sumcheck/polynomials/univariate.hpp"
 #include "barretenberg/ecc/curves/bn254/g1.hpp"
 #include "barretenberg/honk/transcript/transcript.hpp"
@@ -35,7 +36,11 @@ class Ultra {
     using GroupElement = G1::element;
     using Commitment = G1::affine_element;
     using CommitmentHandle = G1::affine_element;
-    using PCSParams = pcs::kzg::Params;
+    // UltraHonk will be run with KZG by default but temporarily we set the commitment to IPA to
+    // be able to do e2e tests with this pcs as well
+    // TODO: instantiate this with but IPA and KZG when the templating work is finished
+    using PCSParams = pcs::ipa::Params;
+    using PCS = pcs::ipa::IPA<PCSParams>;
 
     static constexpr size_t NUM_WIRES = CircuitConstructor::NUM_WIRES;
     // The number of multivariate polynomials on which a sumcheck prover sumcheck operates (including shifts). We often

diff --git a/cpp/src/barretenberg/honk/pcs/claim.hpp b/cpp/src/barretenberg/honk/pcs/claim.hpp
@@ -26,15 +26,15 @@ template <typename Params> class OpeningPair {
  * @tparam Params for the given commitment scheme
  */
 template <typename Params> class OpeningClaim {
-    using CK = typename Params::CK;
-    using CommitmentAffine = typename Params::C;
+    using CK = typename Params::CommitmentKey;
+    using Commitment = typename Params::Commitment;
     using Fr = typename Params::Fr;
 
   public:
     // (challenge r, evaluation v = p(r))
     OpeningPair<Params> opening_pair;
     // commitment to univariate polynomial p(X)
-    CommitmentAffine commitment;
+    Commitment commitment;
 
     /**
      * @brief inefficiently check that the claim is correct by recomputing the commitment
@@ -44,7 +44,7 @@ template <typename Params> class OpeningClaim {
      * @param polynomial the claimed witness polynomial p(X)
      * @return C = Commit(p(X)) && p(r) = v
      */
-    bool verify(CK* ck, const barretenberg::Polynomial<Fr>& polynomial) const
+    bool verify(std::shared_ptr<CK> ck, const barretenberg::Polynomial<Fr>& polynomial) const
     {
         Fr real_eval = polynomial.evaluate(opening_pair.challenge);
         if (real_eval != opening_pair.evaluation) {
@@ -78,14 +78,14 @@ template <typename Params> class OpeningClaim {
  * @tparam CommitmentKey
  */
 template <typename Params> class MLEOpeningClaim {
-    using CommitmentAffine = typename Params::C;
+    using Commitment = typename Params::Commitment;
     using Fr = typename Params::Fr;
 
   public:
     // commitment to a univariate polynomial
     // whose coefficients are the multi-linear evaluations
     // of C = [f]
-    CommitmentAffine commitment;
+    Commitment commitment;
     // v  = f(u) = ∑ᵢ aᵢ⋅Lᵢ(u)
     // v↺ = g(u) = a₁⋅L₀(u) + … + aₙ₋₁⋅Lₙ₋₂(u)
     Fr evaluation;