Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Single commitment key allocation in CIVC #9974

Merged
merged 13 commits into from
Nov 21, 2024
Merged
20 changes: 15 additions & 5 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<Verific
proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);
trace_usage_tracker = ExecutionTraceUsageTracker(trace_settings);
} else {
proving_key = std::make_shared<DeciderProvingKey>(
circuit, trace_settings, fold_output.accumulator->proving_key.commitment_key);
proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);
}

proving_key->proving_key.commitment_key = bn254_commitment_key;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the PR is just threading the CIVC class-level ck through.


vinfo("getting honk vk... precomputed?: ", precomputed_vk);
// Update the accumulator trace usage based on the present circuit
trace_usage_tracker.update(circuit);
Expand Down Expand Up @@ -278,7 +279,7 @@ HonkProof ClientIVC::construct_and_prove_hiding_circuit()
MergeProof merge_proof = goblin.prove_merge(builder);
merge_verification_queue.emplace_back(merge_proof);

auto decider_pk = std::make_shared<DeciderProvingKey>(builder);
auto decider_pk = std::make_shared<DeciderProvingKey>(builder, TraceSettings(), bn254_commitment_key);
honk_vk = std::make_shared<VerificationKey>(decider_pk->proving_key);
MegaProver prover(decider_pk);

Expand Down Expand Up @@ -338,6 +339,7 @@ bool ClientIVC::verify(const Proof& proof)
HonkProof ClientIVC::decider_prove() const
{
vinfo("prove decider...");
fold_output.accumulator->proving_key.commitment_key = bn254_commitment_key;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to do this but IIRC a bigger refactor would be needed to construct the initial accumulator with this key (it starts out as a nullptr).

MegaDeciderProver decider_prover(fold_output.accumulator);
return decider_prover.construct_proof();
vinfo("finished decider proving.");
Expand All @@ -352,11 +354,19 @@ HonkProof ClientIVC::decider_prove() const
bool ClientIVC::prove_and_verify()
{
auto start = std::chrono::steady_clock::now();
auto proof = prove();
const auto proof = prove();
auto end = std::chrono::steady_clock::now();
auto diff = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);
vinfo("time to call ClientIVC::prove: ", diff.count(), " ms.");
return verify(proof);

start = end;
const bool verified = verify(proof);
end = std::chrono::steady_clock::now();

diff = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);
vinfo("time to verify ClientIVC proof: ", diff.count(), " ms.");

return verified;
}

/**
Expand Down
10 changes: 8 additions & 2 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ class ClientIVC {
using ProverFoldOutput = FoldingResult<Flavor>;

public:
GoblinProver goblin;

ProverFoldOutput fold_output; // prover accumulator and fold proof

std::shared_ptr<DeciderVerificationKey> verifier_accumulator; // verifier accumulator
Expand All @@ -122,11 +120,19 @@ class ClientIVC {
// Setting auto_verify_mode = true will cause kernel completion logic to be added to kernels automatically
bool auto_verify_mode;

std::shared_ptr<typename MegaFlavor::CommitmentKey> bn254_commitment_key;

GoblinProver goblin;

bool initialized = false; // Is the IVC accumulator initialized

ClientIVC(TraceSettings trace_settings = {}, bool auto_verify_mode = false)
: trace_settings(trace_settings)
, auto_verify_mode(auto_verify_mode)
, bn254_commitment_key(trace_settings.structure.has_value()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when would the structure not have value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As things are set up we don't always use the structuring. This used to be encoded with an enum but now it's encoded with an optional.

? std::make_shared<CommitmentKey<curve::BN254>>(trace_settings.dyadic_size())
: nullptr)
, goblin(bn254_commitment_key)
{}

void instantiate_stdlib_verification_queue(
Expand Down
6 changes: 5 additions & 1 deletion barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ WASM_EXPORT void acir_prove_and_verify_aztec_client(uint8_t const* acir_stack,
}
// TODO(#7371) dedupe this with the rest of the similar code
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1101): remove use of auto_verify_mode
ClientIVC ivc{ { E2E_FULL_TEST_STRUCTURE }, /*auto_verify_mode=*/true };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bigger structure not necessary for any tests of this code.

ClientIVC ivc{ { CLIENT_IVC_BENCH_STRUCTURE }, /*auto_verify_mode=*/true };

// Accumulate the entire program stack into the IVC
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1116): remove manual setting of is_kernel once databus
Expand Down Expand Up @@ -267,6 +267,10 @@ WASM_EXPORT void acir_prove_and_verify_aztec_client(uint8_t const* acir_stack,
bool result = ivc.prove_and_verify();
info("verified?: ", result);

end = std::chrono::steady_clock::now();
diff = std::chrono::duration_cast<std::chrono::milliseconds>(end - start);
vinfo("time to construct, accumulate, prove and verify all circuits: ", diff.count());

*verified = result;
}

Expand Down
3 changes: 1 addition & 2 deletions barretenberg/cpp/src/barretenberg/eccvm/eccvm_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ void ECCVMProver::execute_relation_check_rounds()
gate_challenges[idx] = transcript->template get_challenge<FF>("Sumcheck:gate_challenge_" + std::to_string(idx));
}

auto commitment_key = std::make_shared<CommitmentKey>(Flavor::BATCHED_RELATION_PARTIAL_LENGTH);
zk_sumcheck_data = ZKSumcheckData<Flavor>(key->log_circuit_size, transcript, commitment_key);
zk_sumcheck_data = ZKSumcheckData<Flavor>(key->log_circuit_size, transcript, key->commitment_key);

sumcheck_output = sumcheck.prove(key->polynomials, relation_parameters, alpha, gate_challenges, zk_sumcheck_data);
}
Expand Down
10 changes: 6 additions & 4 deletions barretenberg/cpp/src/barretenberg/goblin/goblin.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class GoblinProver {
*/

std::shared_ptr<OpQueue> op_queue = std::make_shared<OpQueue>();
std::shared_ptr<CommitmentKey<curve::BN254>> commitment_key;

MergeProof merge_proof;
GoblinProof goblin_proof;
Expand All @@ -70,11 +71,12 @@ class GoblinProver {
GoblinAccumulationOutput accumulator; // Used only for ACIR methods for now

public:
GoblinProver()
GoblinProver(const std::shared_ptr<CommitmentKey<curve::BN254>>& bn254_commitment_key = nullptr)
{ // Mocks the interaction of a first circuit with the op queue due to the inability to currently handle zero
// commitments (https://github.com/AztecProtocol/barretenberg/issues/871) which would otherwise appear in the
// first round of the merge protocol. To be removed once the issue has been resolved.
GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue);
commitment_key = bn254_commitment_key ? bn254_commitment_key : nullptr;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, pointer/nullptr are truthy with values true/false, and I think this is clean and widely used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not just commitment_key = bn254_commitment_key?

GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue, commitment_key);
}
/**
* @brief Construct a MegaHonk proof and a merge proof for the present circuit.
Expand Down Expand Up @@ -160,7 +162,7 @@ class GoblinProver {
merge_proof_exists = true;
}

MergeProver merge_prover{ circuit_builder.op_queue };
MergeProver merge_prover{ circuit_builder.op_queue, commitment_key };
return merge_prover.construct_proof();
};

Expand Down Expand Up @@ -209,7 +211,7 @@ class GoblinProver {

auto translator_builder =
std::make_unique<TranslatorBuilder>(translation_batching_challenge_v, evaluation_challenge_x, op_queue);
translator_prover = std::make_unique<TranslatorProver>(*translator_builder, transcript);
translator_prover = std::make_unique<TranslatorProver>(*translator_builder, transcript, commitment_key);
}

{
Expand Down
8 changes: 5 additions & 3 deletions barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ class GoblinMockCircuits {
*
* @param op_queue
*/
static void perform_op_queue_interactions_for_mock_first_circuit(std::shared_ptr<bb::ECCOpQueue>& op_queue)
static void perform_op_queue_interactions_for_mock_first_circuit(
std::shared_ptr<bb::ECCOpQueue>& op_queue, std::shared_ptr<CommitmentKey> commitment_key = nullptr)
{
PROFILE_THIS();

Expand All @@ -134,11 +135,12 @@ class GoblinMockCircuits {

// Manually compute the op queue transcript commitments (which would normally be done by the merge prover)
bb::srs::init_crs_factory("../srs_db/ignition");
auto commitment_key = CommitmentKey(op_queue->get_current_size());
auto bn254_commitment_key =
commitment_key ? commitment_key : std::make_shared<CommitmentKey>(op_queue->get_current_size());
std::array<Point, Flavor::NUM_WIRES> op_queue_commitments;
size_t idx = 0;
for (auto& entry : op_queue->get_aggregate_transcript()) {
op_queue_commitments[idx++] = commitment_key.commit({ 0, entry });
op_queue_commitments[idx++] = bn254_commitment_key->commit({ 0, entry });
}
// Store the commitment data for use by the prover of the next circuit
op_queue->set_commitment_data(op_queue_commitments);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,15 @@ template <typename T> struct MegaTraceBlockData {
};
}

static uint32_t size() { return 0; }
static uint32_t dyadic_size() { return 0; }
size_t size() const
requires std::same_as<T, uint32_t>
{
size_t result{ 0 };
for (const auto& block_size : get()) {
result += block_size;
}
return static_cast<size_t>(result);
}

bool operator==(const MegaTraceBlockData& other) const = default;
};
Expand All @@ -72,6 +79,15 @@ struct TraceSettings {
// The size of the overflow block. Specified separately because it is allowed to be determined at runtime in the
// context of VK computation
uint32_t overflow_capacity = 0;

size_t size() const { return structure->size() + static_cast<size_t>(overflow_capacity); }

size_t dyadic_size() const
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also want a dyadic size on the structure but it wasn't needed.

{
const size_t total_size = size();
const size_t lower_dyadic = 1 << numeric::get_msb(total_size);
return total_size > lower_dyadic ? lower_dyadic << 1 : lower_dyadic;
}
};

class MegaTraceBlock : public ExecutionTraceBlock<fr, /*NUM_WIRES_ */ 4, /*NUM_SELECTORS_*/ 14> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,12 @@ class MegaFlavor {
VerificationKey(ProvingKey& proving_key)
{
set_metadata(proving_key);
if (proving_key.commitment_key == nullptr) {
proving_key.commitment_key = std::make_shared<CommitmentKey>(proving_key.circuit_size);
auto& ck = proving_key.commitment_key;
if (!ck || ck->srs->get_monomial_size() < proving_key.circuit_size) {
ck = std::make_shared<CommitmentKey>(proving_key.circuit_size);
}
for (auto [polynomial, commitment] : zip_view(proving_key.polynomials.get_precomputed(), this->get_all())) {
commitment = proving_key.commitment_key->commit(polynomial);
commitment = ck->commit(polynomial);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,18 @@

namespace bb {

TranslatorProver::TranslatorProver(CircuitBuilder& circuit_builder, const std::shared_ptr<Transcript>& transcript)
TranslatorProver::TranslatorProver(CircuitBuilder& circuit_builder,
const std::shared_ptr<Transcript>& transcript,
std::shared_ptr<CommitmentKey> commitment_key)
: dyadic_circuit_size(Flavor::compute_dyadic_circuit_size(circuit_builder))
, mini_circuit_dyadic_size(Flavor::compute_mini_circuit_dyadic_size(circuit_builder))
, transcript(transcript)
, key(std::make_shared<ProvingKey>(circuit_builder))
{
PROFILE_THIS();

// Compute total number of gates, dyadic circuit size, etc.
key = std::make_shared<ProvingKey>(circuit_builder);
key->commitment_key = commitment_key ? commitment_key : std::make_shared<CommitmentKey>(key->circuit_size);
compute_witness(circuit_builder);
compute_commitment_key(key->circuit_size);
}

/**
Expand Down Expand Up @@ -159,9 +160,8 @@ void TranslatorProver::execute_relation_check_rounds()
gate_challenges[idx] = transcript->template get_challenge<FF>("Sumcheck:gate_challenge_" + std::to_string(idx));
}

// create masking polynomials for sumcheck round univariates and auxiliary data
auto commitment_key = std::make_shared<CommitmentKey>(Flavor::BATCHED_RELATION_PARTIAL_LENGTH);
zk_sumcheck_data = ZKSumcheckData<Flavor>(key->log_circuit_size, transcript, commitment_key);
// // create masking polynomials for sumcheck round univariates and auxiliary data
zk_sumcheck_data = ZKSumcheckData<Flavor>(key->log_circuit_size, transcript, key->commitment_key);

sumcheck_output = sumcheck.prove(key->polynomials, relation_parameters, alpha, gate_challenges, zk_sumcheck_data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ class TranslatorProver {
size_t dyadic_circuit_size = 0; // final power-of-2 circuit size
size_t mini_circuit_dyadic_size = 0; // The size of the small circuit that contains non-range constraint relations

explicit TranslatorProver(CircuitBuilder& circuit_builder, const std::shared_ptr<Transcript>& transcript);
explicit TranslatorProver(CircuitBuilder& circuit_builder,
const std::shared_ptr<Transcript>& transcript,
std::shared_ptr<CommitmentKey> commitment_key = nullptr);

void compute_witness(CircuitBuilder& circuit_builder);
void compute_commitment_key(size_t circuit_size);
Expand Down
14 changes: 6 additions & 8 deletions barretenberg/cpp/src/barretenberg/ultra_honk/decider_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,33 +57,31 @@ template <IsUltraFlavor Flavor> void DeciderProver_<Flavor>::execute_relation_ch
*/
template <IsUltraFlavor Flavor> void DeciderProver_<Flavor>::execute_pcs_rounds()
{
if (proving_key->proving_key.commitment_key == nullptr) {
proving_key->proving_key.commitment_key =
std::make_shared<CommitmentKey>(proving_key->proving_key.circuit_size);
}
vinfo("made commitment key");
using OpeningClaim = ProverOpeningClaim<Curve>;

auto& ck = proving_key->proving_key.commitment_key;
ck = ck ? ck : std::make_shared<CommitmentKey>(proving_key->proving_key.circuit_size);

OpeningClaim prover_opening_claim;
if constexpr (!Flavor::HasZK) {
prover_opening_claim = ShpleminiProver_<Curve>::prove(proving_key->proving_key.circuit_size,
proving_key->proving_key.polynomials.get_unshifted(),
proving_key->proving_key.polynomials.get_to_be_shifted(),
sumcheck_output.challenge,
proving_key->proving_key.commitment_key,
ck,
transcript);
} else {
prover_opening_claim = ShpleminiProver_<Curve>::prove(proving_key->proving_key.circuit_size,
proving_key->proving_key.polynomials.get_unshifted(),
proving_key->proving_key.polynomials.get_to_be_shifted(),
sumcheck_output.challenge,
proving_key->proving_key.commitment_key,
ck,
transcript,
zk_sumcheck_data.libra_univariates_monomial,
sumcheck_output.claimed_libra_evaluations);
}
vinfo("executed multivariate-to-univarite reduction");
PCS::compute_opening_proof(proving_key->proving_key.commitment_key, prover_opening_claim, transcript);
PCS::compute_opening_proof(ck, prover_opening_claim, transcript);
vinfo("computed opening proof");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
std::vector<FF> gate_challenges;
// The target sum, which is typically nonzero for a ProtogalaxyProver's accmumulator
FF target_sum;

size_t final_active_wire_idx{ 0 }; // idx of last non-trivial wire value in the trace
size_t dyadic_circuit_size{ 0 }; // final power-of-2 circuit size

DeciderProvingKey_(Circuit& circuit,
TraceSettings trace_settings = {},
std::shared_ptr<typename Flavor::CommitmentKey> commitment_key = nullptr)
std::shared_ptr<CommitmentKey> commitment_key = nullptr)
: is_structured(trace_settings.structure.has_value())
{
PROFILE_THIS_NAME("DeciderProvingKey(Circuit&)");
Expand Down Expand Up @@ -105,6 +105,7 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
if ((IsMegaFlavor<Flavor> && !is_structured) || (is_structured && circuit.blocks.has_overflow)) {
// Allocate full size polynomials
proving_key.polynomials = typename Flavor::ProverPolynomials(dyadic_circuit_size);
vinfo("allocated polynomials object in proving key");
} else { // Allocate only a correct amount of memory for each polynomial
// Allocate the wires and selectors polynomials
{
Expand Down Expand Up @@ -262,6 +263,7 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
/* size=*/dyadic_circuit_size, /*virtual size=*/dyadic_circuit_size, /*start_idx=*/0);
}
}
vinfo("allocated polynomials object in proving key");
// We can finally set the shifted polynomials now that all of the to_be_shifted polynomials are
// defined.
proving_key.polynomials.set_shifted(); // Ensure shifted wires are set correctly
Expand Down Expand Up @@ -334,7 +336,6 @@ template <IsUltraFlavor Flavor> class DeciderProvingKey_ {
private:
static constexpr size_t num_zero_rows = Flavor::has_zero_row ? 1 : 0;
static constexpr size_t NUM_WIRES = Circuit::NUM_WIRES;
size_t dyadic_circuit_size = 0; // final power-of-2 circuit size

size_t compute_dyadic_size(Circuit&);

Expand Down
7 changes: 4 additions & 3 deletions barretenberg/cpp/src/barretenberg/ultra_honk/merge_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ namespace bb {
*
*/
template <class Flavor>
MergeProver_<Flavor>::MergeProver_(const std::shared_ptr<ECCOpQueue>& op_queue)
MergeProver_<Flavor>::MergeProver_(const std::shared_ptr<ECCOpQueue>& op_queue,
std::shared_ptr<CommitmentKey> commitment_key)
: op_queue(op_queue)
{
// Update internal size data in the op queue that allows for extraction of e.g. previous aggregate transcript
op_queue->set_size_data();
// Get the appropriate commitment based on the updated ultra ops size
pcs_commitment_key = std::make_shared<CommitmentKey>(op_queue->get_current_size());
pcs_commitment_key =
commitment_key ? commitment_key : std::make_shared<CommitmentKey>(op_queue->get_current_size());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ template <typename Flavor> class MergeProver_ {
public:
std::shared_ptr<Transcript> transcript;

explicit MergeProver_(const std::shared_ptr<ECCOpQueue>&);
explicit MergeProver_(const std::shared_ptr<ECCOpQueue>& op_queue,
std::shared_ptr<CommitmentKey> commitment_key = nullptr);

BB_PROFILE HonkProof construct_proof();

Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/ultra_honk/oink_prover.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ template <IsUltraFlavor Flavor> void OinkProver<Flavor>::prove()
// Generate relation separators alphas for sumcheck/combiner computation
proving_key->alphas = generate_alphas_round();

#ifndef __wasm__
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of this? Seems like it won't be freed anyway if since ClientIVC is storing a shared_ptr to this.

I guess for UH in wasm, it will no longer be freed then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You raise a good point--I wasn't thinking of UH WASM performance here. However, this optimization of freeing and reallocating the ck was only tested to work in x86, so you can think of this macro as almost being documentation of that fact. In the future we should have a PR that specifically tunes WASM memory performance of UH and yeah, perhaps an optimization at that point will require something different here.

// Free the commitment key
proving_key->proving_key.commitment_key = nullptr;
#endif
}

/**
Expand Down
Loading
Loading