Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
560373c
swapping in folding appears to work
ledwards2225 Nov 14, 2024
981074c
WiP; dyadic size bump tst failing without real clues
ledwards2225 Nov 15, 2024
090e720
Merge branch 'master' into lde/swap_polys
ledwards2225 Nov 18, 2024
2eded0b
Merge branch 'master' into lde/swap_polys
ledwards2225 Nov 18, 2024
d06c1e8
changes from other PR; test passes if overflow is set in advance
ledwards2225 Nov 18, 2024
b4cb9de
WiP debug with check circuit shows decider sumcheck failure
ledwards2225 Nov 18, 2024
a7231ef
add test that proves out dynamic virtual size increase
ledwards2225 Nov 18, 2024
0cf3757
move relation checker to pk inspector
ledwards2225 Nov 18, 2024
f7e8fc7
add proper relation checker class
ledwards2225 Nov 19, 2024
35c15a0
debug
ledwards2225 Nov 19, 2024
515fe8b
WiP virtual size increase pg test
ledwards2225 Nov 19, 2024
e07eb4d
basic pg test passes after replacing cir size with vir size
ledwards2225 Nov 19, 2024
7d4ba63
correctly set circuit size in pg verifier; pg decide test passes
ledwards2225 Nov 19, 2024
ba7bfd5
Merge branch 'master' into lde/swap_polys
ledwards2225 Nov 20, 2024
559bb2e
turn off debugging check circuit calls
ledwards2225 Nov 20, 2024
916fc98
comments and fix build error
ledwards2225 Nov 20, 2024
b23cd91
corrctly set circuit size in pg rec verifier
ledwards2225 Nov 20, 2024
864ee3a
comments and cleanup
ledwards2225 Nov 20, 2024
bfb2a70
add some debug utility in ivc tests
ledwards2225 Nov 20, 2024
2494af1
debug stuff
ledwards2225 Nov 20, 2024
531c0d1
comments
ledwards2225 Nov 20, 2024
0b582eb
loads of cleanup and execution trace tracker needs fixing
Nov 26, 2024
d781974
fixed
Nov 26, 2024
0b714d2
cleanup and enable parallelisation
Nov 26, 2024
c9e6af0
create todos
Nov 26, 2024
90277ab
make a folding test utils
Nov 26, 2024
0891578
Merge remote-tracking branch 'origin/master' into lde/swap_polys
Nov 26, 2024
9f05f0b
undo comment
Nov 26, 2024
c13923b
revert submodule changes
Nov 26, 2024
48052d0
some constifying
Nov 26, 2024
681c1e6
unused variable error when building everything
Nov 26, 2024
1f79b46
now both builds work
Nov 27, 2024
e7d8499
wopsie, I removed the commitment key from DeciderProvingKey construct…
Nov 27, 2024
1f4df0f
fix commitment ket caveat
Nov 27, 2024
cd92b66
update PR based on review
Nov 27, 2024
067cbb0
remove extra boolean in compute_row_evaluations
Nov 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<Verific
circuit.add_pairing_point_accumulator(stdlib::recursion::init_default_agg_obj_indices<ClientCircuit>(circuit));

// Construct the proving key for circuit
std::shared_ptr<DeciderProvingKey> proving_key;
if (!initialized) {
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);
}
std::shared_ptr<DeciderProvingKey> proving_key = std::make_shared<DeciderProvingKey>(circuit, trace_settings);

// The commitment key is initialised with the number of points determined by the trace_settings' dyadic size. If a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is the caveat with overflowing circuits and having a single CIVC commitment key, maybe this could be refined in a follow up PR but seemed the easiest solution as it is. In the ideal case when we never overflow we still can benefit from having a single commitment key

// circuit overflows past the dyadic size the commitment key will not have enough points so we need to increase it
if (proving_key->proving_key.circuit_size > trace_settings.dyadic_size()) {
bn254_commitment_key = std::make_shared<CommitmentKey<curve::BN254>>(proving_key->proving_key.circuit_size);
goblin.commitment_key = bn254_commitment_key;
}
proving_key->proving_key.commitment_key = bn254_commitment_key;

vinfo("getting honk vk... precomputed?: ", precomputed_vk);
Expand All @@ -192,7 +192,8 @@ void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<Verific
}
vinfo("set honk vk metadata");

// If this is the first circuit in the IVC, use oink to complete the decider proving key and generate an oink proof
// If this is the first circuit in the IVC, use oink to complete the decider proving key and generate an oink
// proof
if (!initialized) {
OinkProver<Flavor> oink_prover{ proving_key };
vinfo("computing oink proof...");
Expand All @@ -210,8 +211,8 @@ void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<Verific

initialized = true;
} else { // Otherwise, fold the new key into the accumulator
vinfo("computing folding proof");
FoldingProver folding_prover({ fold_output.accumulator, proving_key }, trace_usage_tracker);
vinfo("constructed folding prover");
fold_output = folding_prover.prove();
vinfo("constructed folding proof");

Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ class ClientIVC {
bool initialized = false; // Is the IVC accumulator initialized

ClientIVC(TraceSettings trace_settings = {}, bool auto_verify_mode = false)
: trace_settings(trace_settings)
: trace_usage_tracker(trace_settings)
, trace_settings(trace_settings)
, auto_verify_mode(auto_verify_mode)
, bn254_commitment_key(trace_settings.structure.has_value()
? std::make_shared<CommitmentKey<curve::BN254>>(trace_settings.dyadic_size())
Expand Down
59 changes: 58 additions & 1 deletion barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
#include "barretenberg/client_ivc/test_bench_shared.hpp"
#include "barretenberg/goblin/goblin.hpp"
#include "barretenberg/goblin/mock_circuits.hpp"
#include "barretenberg/protogalaxy/folding_test_utils.hpp"
#include "barretenberg/stdlib_circuit_builders/mega_circuit_builder.hpp"
#include "barretenberg/stdlib_circuit_builders/ultra_circuit_builder.hpp"

#include <gtest/gtest.h>

using namespace bb;
Expand Down Expand Up @@ -403,5 +403,62 @@ TEST_F(ClientIVCTests, StructuredTraceOverflow)
log2_num_gates += 1;
}

EXPECT_TRUE(ivc.prove_and_verify());
};

/**
* @brief Test dynamic structured trace overflow block mechanism
* @details Tests the case where the required overflow capacity is not known until runtime. Accumulates two circuits,
* the second of which overflows the trace but not enough to change the dyadic circuit size and thus there is no need
* for a virtual size increase of the first key.
*
*/
TEST_F(ClientIVCTests, DynamicOverflow)
{
// Define trace settings with zero overflow capacity
ClientIVC ivc{ { SMALL_TEST_STRUCTURE_FOR_OVERFLOWS, /*overflow_capacity=*/0 } };

MockCircuitProducer circuit_producer;

const size_t NUM_CIRCUITS = 2;

// define parameters for two circuits; the first fits within the structured trace, the second overflows
const std::vector<size_t> log2_num_arith_gates = { 14, 16 };
// Accumulate
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
auto circuit = circuit_producer.create_next_circuit(ivc, log2_num_arith_gates[idx]);
ivc.accumulate(circuit);
}

EXPECT_EQ(check_accumulator_target_sum_manual(ivc.fold_output.accumulator), true);
EXPECT_TRUE(ivc.prove_and_verify());
};

/**
* @brief Test dynamic trace overflow where the dyadic circuit size also increases
* @details Accumulates two circuits, the second of which overflows the trace structure and leads to an increased dyadic
* circuit size. This requires the virtual size of the polynomials in the first key to be increased accordingly which
* should be handled automatically in PG/ClientIvc.
*
*/
TEST_F(ClientIVCTests, DynamicOverflowCircuitSizeChange)
{
uint32_t overflow_capacity = 0;
// uint32_t overflow_capacity = 1 << 1;
ClientIVC ivc{ { SMALL_TEST_STRUCTURE_FOR_OVERFLOWS, overflow_capacity } };

MockCircuitProducer circuit_producer;

const size_t NUM_CIRCUITS = 2;

// define parameters for two circuits; the first fits within the structured trace, the second overflows
const std::vector<size_t> log2_num_arith_gates = { 14, 18 };
// Accumulate
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
auto circuit = circuit_producer.create_next_circuit(ivc, log2_num_arith_gates[idx]);
ivc.accumulate(circuit);
}

EXPECT_EQ(check_accumulator_target_sum_manual(ivc.fold_output.accumulator), true);
EXPECT_TRUE(ivc.prove_and_verify());
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "../claim.hpp"
#include "barretenberg/commitment_schemes/claim.hpp"
#include "barretenberg/commitment_schemes/commitment_key.hpp"
#include "barretenberg/commitment_schemes/utils/batch_mul_native.hpp"
#include "barretenberg/commitment_schemes/verification_key.hpp"
Expand Down
16 changes: 16 additions & 0 deletions barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@

namespace bb {

/**
* @brief An arbitrary but small-ish structuring that can be used for testing with non-trivial circuits in cases when
* they overflow
*/
static constexpr TraceStructure SMALL_TEST_STRUCTURE_FOR_OVERFLOWS{ .ecc_op = 1 << 14,
.pub_inputs = 1 << 14,
.busread = 1 << 14,
.arithmetic = 1 << 15,
.delta_range = 1 << 14,
.elliptic = 1 << 14,
.aux = 1 << 14,
.poseidon2_external = 1 << 14,
.poseidon2_internal = 1 << 15,
.lookup = 1 << 14,
.overflow = 0 };

class GoblinMockCircuits {
public:
using Curve = curve::BN254;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ struct ExecutionTraceUsageTracker {
size_t max_databus_size = 0;
size_t max_tables_size = 0;

// For printing only. Must match the order of the members in the arithmetization
static constexpr std::array<std::string_view, 13> block_labels{ "ecc_op",
"pub_inputs",
"busread",
"arithmetic",
"delta_range",
"elliptic",
"aux",
"poseidon2_external",
"poseidon2_internal",
"lookup",
"overflow",
"databus_table_data",
"lookup_table_data" };

TraceSettings trace_settings;

ExecutionTraceUsageTracker(const TraceSettings& trace_settings = TraceSettings{})
Expand Down Expand Up @@ -72,8 +87,12 @@ struct ExecutionTraceUsageTracker {
}

// The active ranges must also include the rows where the actual databus and lookup table data are stored.
// (Note: lookup tables are constructed at the end of the trace; databus data is constructed at the start).
size_t dyadic_circuit_size = fixed_sizes.get_structured_dyadic_size();
// (Note: lookup tables are constructed at the end of the trace; databus data is constructed at the start) so we

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment will break if the ordering in the trace structure ever changes--there's little chance that the person changing the structure remembers this comment exists, if they ever knew.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

a way to make this more robust is to update the active ranges as we are constructing the DeciderProvingKey so if the position of the rows changes it is obvious that the tracker's active ranges also need to be updated, will add an issue.

// need to determine the dyadic size for this. We call the size function on the current circuit which will have
// the same fixed block sizes but might also have an overflow block potentially influencing the dyadic circuit
// size.
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1160)
const size_t dyadic_circuit_size = circuit.blocks.get_structured_dyadic_size();

// TODO(https://github.com/AztecProtocol/barretenberg/issues/1152): should be able to use simply Range{ 0,
// max_databus_size } but this breaks for certain choices of num_threads.
Expand All @@ -98,24 +117,13 @@ struct ExecutionTraceUsageTracker {
});
}

// For printing only. Must match the order of the members in the arithmetization
std::vector<std::string> block_labels{ "ecc_op",
"pub_inputs",
"busread",
"arithmetic",
"delta_range",
"elliptic",
"aux",
"poseidon2_external",
"poseidon2_internal",
"lookup",
"overflow" };

void print()
{
info("Minimum required block sizes for structured trace: ");
for (auto [label, max_size] : zip_view(block_labels, max_sizes.get())) {
std::cout << std::left << std::setw(20) << (label + ":") << max_size << std::endl;
size_t idx = 0;
for (auto max_size : max_sizes.get()) {
std::cout << std::left << std::setw(20) << block_labels[idx] << ": " << max_size << std::endl;
idx++;
}
info("");
}
Expand All @@ -124,8 +132,18 @@ struct ExecutionTraceUsageTracker {
{
info("Active regions of accumulator: ");
for (auto [label, range] : zip_view(block_labels, active_ranges)) {
std::cout << std::left << std::setw(20) << (label + ":") << "(" << range.first << ", " << range.second
<< ")" << std::endl;
std::cout << std::left << std::setw(20) << label << ": (" << range.first << ", " << range.second << ")"
<< std::endl;
}
info("");
}

void print_previous_active_ranges()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

when i was printing the active ranges it wasn't reflecting everything due to the lookup and databus caveat so I think it made sense to refine the labels

{
info("Active regions of previous accumulator: ");
for (auto [label, range] : zip_view(block_labels, previous_active_ranges)) {
std::cout << std::left << std::setw(20) << label << ": (" << range.first << ", " << range.second << ")"
<< std::endl;
}
info("");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,17 @@ template <typename T> struct MegaTraceBlockData {

std::vector<std::string_view> get_labels() const
{
return { "ecc_op", "pub_inputs", "busread",
"arithmetic", "delta_range", "elliptic",
"aux", "poseidon2_external", "poseidon2_internal",
"lookup" };
return { "ecc_op",
"pub_inputs",
"busread",
"arithmetic",
"delta_range",
"elliptic",
"aux",
"poseidon2_external",
"poseidon2_internal",
"lookup",
"overflow" };
}

auto get()
Expand Down Expand Up @@ -220,10 +227,10 @@ class MegaExecutionTraceBlocks : public MegaTraceBlockData<MegaTraceBlock> {
info("");
}

size_t get_structured_dyadic_size()
size_t get_structured_dyadic_size() const
{
size_t total_size = 1; // start at 1 because the 0th row is unused for selectors for Honk
for (auto block : this->get()) {
for (const auto& block : this->get()) {
total_size += block.get_fixed_size();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ typename Flavor::FF compute_public_input_delta(std::span<const typename Flavor::
// Note: The public inputs may be offset from the 0th index of the wires, for example due to the inclusion of an
// initial zero row or Goblin-stlye ECC op gates. Accordingly, the indices i in the above formulas are given by i =
// [0, m-1] + offset, i.e. i = offset, 1 + offset, …, m - 1 + offset.

// TODO(https://github.com/AztecProtocol/barretenberg/issues/1158): Ensure correct construction of public input
// delta in the face of increases to virtual size caused by execution trace overflow
Field numerator_acc = gamma + (beta * Field(domain_size + offset));
Field denominator_acc = gamma - beta * Field(1 + offset);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "barretenberg/common/assert.hpp"
#include "barretenberg/common/log.hpp"

namespace bb::proving_key_inspector {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#include "relation_checker.hpp"

// Hack to make the module compile.
Loading