Skip to content

Commit

Permalink
fix: Simplify ECCVM prover constructor and add a TODO (#5681)
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
ledwards2225 authored Apr 11, 2024
1 parent 0c5dc0a commit 8c151ea
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 37 deletions.
47 changes: 14 additions & 33 deletions barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProvingKey>& input_key,
const std::shared_ptr<CommitmentKey>& commitment_key,
const std::shared_ptr<Transcript>& 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(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<ProvingKey>(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<ProvingKey>(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<CommitmentKey>(local_key->circuit_size), transcript);
commitment_key = std::make_shared<CommitmentKey>(key->circuit_size);
}

/**
Expand Down Expand Up @@ -98,7 +80,6 @@ void ECCVMProver::execute_log_derivative_commitments_round()
compute_logderivative_inverse<Flavor, typename Flavor::LookupRelation>(
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();
}

/**
Expand Down
4 changes: 0 additions & 4 deletions barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ class ECCVMProver {
using CircuitBuilder = typename Flavor::CircuitBuilder;

public:
explicit ECCVMProver(const std::shared_ptr<ProvingKey>& input_key,
const std::shared_ptr<CommitmentKey>& commitment_key,
const std::shared_ptr<Transcript>& transcript = std::make_shared<Transcript>());

explicit ECCVMProver(CircuitBuilder& builder,
const std::shared_ptr<Transcript>& transcript = std::make_shared<Transcript>());

Expand Down
18 changes: 18 additions & 0 deletions barretenberg/cpp/src/barretenberg/polynomials/polynomial.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ template <typename Fr> 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
Expand Down

0 comments on commit 8c151ea

Please sign in to comment.