From 8c151eab1492dda901a1d6b691c9ca68960fd9e6 Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Thu, 11 Apr 2024 06:41:40 -0700 Subject: [PATCH] fix: Simplify ECCVM prover constructor and add a TODO (#5681) Simplify ECCVM prover constructor and clarify the confusion around what we thought was the use of uninitialized polynomials via an issue and a TODO. Note: I think there may have been a merge mistake or something in Cody's recent PR (which I approved) that left the ECCVM prover constructors in a very weird state. (Also there was a WORKTODO left in code). There is still some weirdness (captured in the TODO) but hopefully this clarifies the situation a bit. `ClientIVCBench/Full/6 22891 ms 17769 ms 1` --- .../src/barretenberg/eccvm/eccvm_prover.cpp | 47 ++++++------------- .../src/barretenberg/eccvm/eccvm_prover.hpp | 4 -- .../barretenberg/polynomials/polynomial.hpp | 18 +++++++ 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp index 31832e22574..0f5066d8e2b 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp @@ -12,44 +12,26 @@ namespace bb { -/** - * Create ECCVMProver from proving key, witness and manifest. - * - * @param input_key Proving key. - * @param input_manifest Input manifest - * - * @tparam settings Settings class. - * */ -ECCVMProver::ECCVMProver(const std::shared_ptr& input_key, - const std::shared_ptr& commitment_key, - const std::shared_ptr& transcript) - : transcript(transcript) - , key(input_key) - , commitment_key(commitment_key) -{ // this will be initialized properly later - key->z_perm = Polynomial(key->circuit_size); - for (auto [prover_poly, key_poly] : zip_view(prover_polynomials.get_unshifted(), key->get_all())) { - ASSERT(flavor_get_label(prover_polynomials, prover_poly) == flavor_get_label(*key, key_poly)); - prover_poly = key_poly.share(); - } - for (auto [prover_poly, key_poly] : zip_view(prover_polynomials.get_shifted(), key->get_to_be_shifted())) { - ASSERT(flavor_get_label(prover_polynomials, prover_poly) == (flavor_get_label(*key, key_poly) + "_shift")); - prover_poly = key_poly.shifted(); - } -} - ECCVMProver::ECCVMProver(CircuitBuilder& builder, const std::shared_ptr& transcript) + : transcript(transcript) + , prover_polynomials(builder) { BB_OP_COUNT_TIME_NAME("ECCVMProver(CircuitBuilder&)"); - // compute wire polynomials and copy into proving key - auto local_key = std::make_shared(builder); - ProverPolynomials polynomials(builder); // WORKTODO: inefficient - for (auto [prover_poly, key_poly] : zip_view(polynomials.get_wires(), local_key->get_wires())) { + + // TODO(https://github.com/AztecProtocol/barretenberg/issues/939): Remove redundancy between + // ProvingKey/ProverPolynomials and update the model to reflect what's done in all other proving systems. + + // Construct the proving key; populates all polynomials except for witness polys + key = std::make_shared(builder); + + // Share all unshifted polys from the prover polynomials to the proving key. Note: this means that updating a + // polynomial in one container automatically updates it in the other via the shared memory. + for (auto [prover_poly, key_poly] : zip_view(prover_polynomials.get_unshifted(), key->get_all())) { ASSERT(flavor_get_label(prover_polynomials, prover_poly) == flavor_get_label(*key, key_poly)); - std::copy(prover_poly.begin(), prover_poly.end(), key_poly.begin()); + key_poly = prover_poly.share(); } - *this = ECCVMProver(std::move(local_key), std::make_shared(local_key->circuit_size), transcript); + commitment_key = std::make_shared(key->circuit_size); } /** @@ -98,7 +80,6 @@ void ECCVMProver::execute_log_derivative_commitments_round() compute_logderivative_inverse( prover_polynomials, relation_parameters, key->circuit_size); transcript->send_to_verifier(commitment_labels.lookup_inverses, commitment_key->commit(key->lookup_inverses)); - prover_polynomials.lookup_inverses = key->lookup_inverses.share(); } /** diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp index 3c5967ff258..b91b6d851e2 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp @@ -26,10 +26,6 @@ class ECCVMProver { using CircuitBuilder = typename Flavor::CircuitBuilder; public: - explicit ECCVMProver(const std::shared_ptr& input_key, - const std::shared_ptr& commitment_key, - const std::shared_ptr& transcript = std::make_shared()); - explicit ECCVMProver(CircuitBuilder& builder, const std::shared_ptr& transcript = std::make_shared()); diff --git a/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp b/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp index 8ca718a4796..2b3a02a7736 100644 --- a/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp +++ b/barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp @@ -68,6 +68,24 @@ template class Polynomial { size_ = 0; } + /** + * @brief Check whether or not a polynomial is identically zero + * + */ + bool is_zero() + { + if (is_empty()) { + ASSERT(false); + info("Checking is_zero on an empty Polynomial!"); + } + for (size_t i = 0; i < size(); i++) { + if (coefficients_[i] != 0) { + return false; + } + } + return true; + } + bool operator==(Polynomial const& rhs) const; // Const and non const versions of coefficient accessors