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: handle consecutive kernels in IVC #8924

Merged
merged 15 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 17 additions & 0 deletions barretenberg/cpp/src/barretenberg/bb/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,18 @@ void client_ivc_prove_output_all_msgpack(const std::string& bytecodePath,
ivc.trace_structure = TraceStructure::E2E_FULL_TEST;

// Accumulate the entire program stack into the IVC
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1116): remove manual setting of is_kernel once databus
// has been integrated into noir kernel programs
bool is_kernel = false;
for (Program& program : folding_stack) {
// Construct a bberg circuit from the acir representation then accumulate it into the IVC
auto circuit = create_circuit<Builder>(program.constraints, 0, program.witness, false, ivc.goblin.op_queue);

// Set the internal is_kernel flag based on the local mechanism only if it has not already been set to true
if (!circuit.databus_propagation_data.is_kernel) {
circuit.databus_propagation_data.is_kernel = is_kernel;
}
is_kernel = !is_kernel;
ivc.accumulate(circuit);
}

Expand Down Expand Up @@ -458,16 +467,21 @@ bool foldAndVerifyProgram(const std::string& bytecodePath, const std::string& wi
// assumes that folding is never done with ultrahonk.

// Accumulate the entire program stack into the IVC
bool is_kernel = false;
while (!program_stack.empty()) {
auto stack_item = program_stack.back();

// Construct a bberg circuit from the acir representation
auto builder = acir_format::create_circuit<Builder>(
stack_item.constraints, 0, stack_item.witness, /*honk_recursion=*/false, ivc.goblin.op_queue);

// Set the internal is_kernel flag to trigger automatic appending of kernel logic if true
builder.databus_propagation_data.is_kernel = is_kernel;

ivc.accumulate(builder);

program_stack.pop_back();
is_kernel = !is_kernel; // toggle the kernel indicator flag on/off
}
return ivc.prove_and_verify();
}
Expand Down Expand Up @@ -503,12 +517,15 @@ void client_ivc_prove_output_all(const std::string& bytecodePath,
// assumes that folding is never done with ultrahonk.

// Accumulate the entire program stack into the IVC
bool is_kernel = false;
while (!program_stack.empty()) {
auto stack_item = program_stack.back();

// Construct a bberg circuit from the acir representation
auto circuit = acir_format::create_circuit<Builder>(
stack_item.constraints, 0, stack_item.witness, false, ivc.goblin.op_queue);
circuit.databus_propagation_data.is_kernel = is_kernel;
is_kernel = !is_kernel; // toggle on/off so every second circuit is intepreted as a kernel

ivc.accumulate(circuit);

Expand Down
4 changes: 1 addition & 3 deletions barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,7 @@ void ClientIVC::complete_kernel_circuit_logic(ClientCircuit& circuit)
*/
void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr<VerificationKey>& precomputed_vk, bool mock_vk)
{
is_kernel = !is_kernel; // toggle on each call (every even circuit is a kernel)
Copy link
Contributor Author

@ledwards2225 ledwards2225 Oct 2, 2024

Choose a reason for hiding this comment

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

This is the main point of this PR. IVC will non longer internally assume that every other circuit is a kernel. Instead it will check circuit.databus_propagation_data.is_kernel. This tag will be set to true automatically in ACIR land when the corresponding noir program uses calldata (since only kernels use calldata). Everywhere else (e.g. random test suites), the tag must be set manually for now.


if (auto_verify_mode && is_kernel) {
if (auto_verify_mode && circuit.databus_propagation_data.is_kernel) {
complete_kernel_circuit_logic(circuit);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ class ClientIVC {
// TODO(https://github.com/AztecProtocol/barretenberg/issues/1101): eventually do away with this.
// Setting auto_verify_mode = true will cause kernel completion logic to be added to kernels automatically
bool auto_verify_mode = false;
bool is_kernel = true;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ClientIVCAutoVerifyTests : public ::testing::Test {
* polynomials will bump size to next power of 2)
*
*/
static Builder create_mock_circuit(ClientIVC& ivc, size_t log2_num_gates = 16)
static Builder create_mock_circuit(ClientIVC& ivc, bool is_kernel, size_t log2_num_gates = 16)
{
Builder circuit{ ivc.goblin.op_queue };
MockCircuits::construct_arithmetic_circuit(circuit, log2_num_gates);
Expand All @@ -49,6 +49,8 @@ class ClientIVCAutoVerifyTests : public ::testing::Test {
// circuits (where we don't explicitly need to add goblin ops), in ClientIVC merge proving happens prior to
// folding where the absense of goblin ecc ops will result in zero commitments.
MockCircuits::construct_goblin_ecc_op_circuit(circuit);

circuit.databus_propagation_data.is_kernel = is_kernel;
return circuit;
}
};
Expand All @@ -62,17 +64,37 @@ TEST_F(ClientIVCAutoVerifyTests, Basic)
ClientIVC ivc;
ivc.auto_verify_mode = true;

{
// Initialize the IVC with an arbitrary circuit
Builder circuit_0 = create_mock_circuit(ivc);
ivc.accumulate(circuit_0);
}
// Initialize the IVC with an arbitrary circuit
Builder circuit_0 = create_mock_circuit(ivc, /*is_kernel=*/false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you have is_kernel = false by default for mock circuits so that you only need a second parameter to this function when the circuit is acctually a kernel and you need to set it to 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.

I could but the method already has a 3rd argument with a default value and I try to avoid having methods with multiple default arguments when possible. This whole test suite will be deleted soon enough anyway

ivc.accumulate(circuit_0);

{
// Create another circuit and accumulate
Builder circuit_1 = create_mock_circuit(ivc);
ivc.accumulate(circuit_1);
}
// Create another circuit and accumulate
Builder circuit_1 = create_mock_circuit(ivc, /*is_kernel=*/true);
ivc.accumulate(circuit_1);

EXPECT_TRUE(ivc.prove_and_verify());
};

/**
* @brief The number of circuits processed can be odd as long as the last one is a kernel
*
*/
TEST_F(ClientIVCAutoVerifyTests, BasicOdd)
{
ClientIVC ivc;
ivc.auto_verify_mode = true;

// Initialize the IVC with an arbitrary circuit
Builder circuit_0 = create_mock_circuit(ivc, /*is_kernel=*/false);
ivc.accumulate(circuit_0);

// Create another circuit and accumulate
Builder circuit_1 = create_mock_circuit(ivc, /*is_kernel=*/true);
ivc.accumulate(circuit_1);

// Create another circuit and accumulate
Builder circuit_2 = create_mock_circuit(ivc, /*is_kernel=*/true);
ivc.accumulate(circuit_2);

EXPECT_TRUE(ivc.prove_and_verify());
};
Expand All @@ -90,7 +112,8 @@ TEST_F(ClientIVCAutoVerifyTests, BasicLarge)
size_t NUM_CIRCUITS = 6;
std::vector<Builder> circuits;
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
circuits.emplace_back(create_mock_circuit(ivc));
bool is_kernel = (idx % 2 == 1); // every second circuit is a kernel
circuits.emplace_back(create_mock_circuit(ivc, is_kernel));
}

// Accumulate each circuit
Expand All @@ -112,10 +135,10 @@ TEST_F(ClientIVCAutoVerifyTests, BasicStructured)
ivc.trace_structure = TraceStructure::SMALL_TEST;

// Construct some circuits of varying size
Builder circuit_0 = create_mock_circuit(ivc, /*log2_num_gates=*/5);
Builder circuit_1 = create_mock_circuit(ivc, /*log2_num_gates=*/8);
Builder circuit_2 = create_mock_circuit(ivc, /*log2_num_gates=*/11);
Builder circuit_3 = create_mock_circuit(ivc, /*log2_num_gates=*/11);
Builder circuit_0 = create_mock_circuit(ivc, /*is_kernel=*/false, /*log2_num_gates=*/5);
Builder circuit_1 = create_mock_circuit(ivc, /*is_kernel=*/true, /*log2_num_gates=*/8);
Builder circuit_2 = create_mock_circuit(ivc, /*is_kernel=*/false, /*log2_num_gates=*/11);
Builder circuit_3 = create_mock_circuit(ivc, /*is_kernel=*/true, /*log2_num_gates=*/11);

// The circuits can be accumulated as normal due to the structured trace
ivc.accumulate(circuit_0);
Expand All @@ -139,7 +162,8 @@ TEST_F(ClientIVCAutoVerifyTests, PrecomputedVerificationKeys)
size_t NUM_CIRCUITS = 4;
std::vector<Builder> circuits;
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
circuits.emplace_back(create_mock_circuit(ivc));
bool is_kernel = (idx % 2 == 1); // every second circuit is a kernel
circuits.emplace_back(create_mock_circuit(ivc, is_kernel));
}

// Precompute the verification keys that will be needed for the IVC
Expand Down Expand Up @@ -167,7 +191,8 @@ TEST_F(ClientIVCAutoVerifyTests, StructuredPrecomputedVKs)
size_t NUM_CIRCUITS = 4;
std::vector<Builder> circuits;
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
circuits.emplace_back(create_mock_circuit(ivc, /*log2_num_gates=*/5));
bool is_kernel = (idx % 2 == 1); // every second circuit is a kernel
circuits.emplace_back(create_mock_circuit(ivc, is_kernel, /*log2_num_gates=*/5));
}

// Precompute the (structured) verification keys that will be needed for the IVC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,35 @@ TEST_F(ClientIVCIntegrationTests, BenchmarkCaseSimple)
EXPECT_TRUE(ivc.prove_and_verify());
};

/**
* @brief Accumulate a set of circuits that includes consecutive kernels
* @details In practice its common to have multiple consecutive kernels without intermittent apps e.g. an inner followed
* immediately by a reset, or an inner-reset-tail sequence. This test ensures that such cases are handled correctly.
*
*/
TEST_F(ClientIVCIntegrationTests, ConsecutiveKernels)
{
ClientIVC ivc;
ivc.trace_structure = TraceStructure::CLIENT_IVC_BENCH;

MockCircuitProducer circuit_producer;

// Accumulate a series of mocked circuits (app, kernel, app, kernel)
size_t NUM_CIRCUITS = 4;
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
Builder circuit = circuit_producer.create_next_circuit(ivc);
ivc.accumulate(circuit);
}

// Cap the IVC with two more kernels (say, a 'reset' and a 'tail') without intermittent apps
Builder reset_kernel = circuit_producer.create_next_circuit(ivc, /*force_is_kernel=*/true);
ivc.accumulate(reset_kernel);
Builder tail_kernel = circuit_producer.create_next_circuit(ivc, /*force_is_kernel=*/true);
ivc.accumulate(tail_kernel);

EXPECT_TRUE(ivc.prove_and_verify());
};

/**
* @brief Prove and verify accumulation of a set of mocked private function execution circuits with precomputed
* verification keys
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,18 @@ class MockDatabusProducer {
*/
void populate_kernel_databus(ClientCircuit& circuit)
{
for (auto& val : kernel_return_data) { // populate calldata from previous kernel return data
// Populate calldata from previous kernel return data (if it exists)
for (auto& val : kernel_return_data) {
circuit.add_public_calldata(circuit.add_variable(val));
}
for (auto& val : app_return_data) { // populate secondary_calldata from app return data
// Populate secondary_calldata from app return data (if it exists), then clear the app return data
for (auto& val : app_return_data) {
circuit.add_public_secondary_calldata(circuit.add_variable(val));
}
kernel_return_data = generate_random_bus_array(); // update the return data for the present kernel circuit
app_return_data.clear();

// Mock the return data for the present kernel circuit
kernel_return_data = generate_random_bus_array();
for (auto& val : kernel_return_data) {
circuit.add_public_return_data(circuit.add_variable(val));
}
Expand Down Expand Up @@ -93,13 +98,14 @@ class PrivateFunctionExecutionMockCircuitProducer {

public:
/**
* @brief Create a the next circuit (app/kernel) in a mocked private function execution stack
* @brief Create the next circuit (app/kernel) in a mocked private function execution stack
*/
ClientCircuit create_next_circuit(ClientIVC& ivc)
ClientCircuit create_next_circuit(ClientIVC& ivc, bool force_is_kernel = false)
{
circuit_counter++;

bool is_kernel = (circuit_counter % 2 == 0); // Every other circuit is a kernel, starting from the second
// Assume only every second circuit is a kernel, unless force_is_kernel == true
bool is_kernel = (circuit_counter % 2 == 0) || force_is_kernel;

ClientCircuit circuit{ ivc.goblin.op_queue };
if (is_kernel) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ void create_block_constraints(MegaCircuitBuilder& builder,
} break;
case BlockType::CallData: {
process_call_data_operations(builder, constraint, has_valid_witness_assignments, init);
// The presence of calldata is used to indicate that the present circuit is a kernel. This is needed in the
// databus consistency checks to indicate that the corresponding return data belongs to a kernel (else an app).
info("ACIR: Setting is_kernel to TRUE.");
builder.databus_propagation_data.is_kernel = true;
} break;
case BlockType::ReturnData: {
process_return_data_operations(constraint, init);
Expand Down
4 changes: 4 additions & 0 deletions barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,17 @@ WASM_EXPORT void acir_fold_and_verify_program_stack(uint8_t const* acir_vec, uin
ivc.auto_verify_mode = true;
ivc.trace_structure = TraceStructure::SMALL_TEST;

bool is_kernel = false;
while (!program_stack.empty()) {
auto stack_item = program_stack.back();

// Construct a bberg circuit from the acir representation
auto builder = acir_format::create_circuit<Builder>(
stack_item.constraints, 0, stack_item.witness, /*honk_recursion=*/false, ivc.goblin.op_queue);

builder.databus_propagation_data.is_kernel = is_kernel;
is_kernel = !is_kernel; // toggle on/off so every second circuit is intepreted as a kernel

ivc.accumulate(builder);

program_stack.pop_back();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ClientIVCRecursionTests : public testing::Test {
for (size_t idx = 0; idx < NUM_CIRCUITS; ++idx) {
Builder circuit{ ivc.goblin.op_queue };
GoblinMockCircuits::construct_mock_function_circuit(circuit);
circuit.databus_propagation_data.is_kernel = (idx % 2 == 1); // every second circuit is a kernel
ivc.accumulate(circuit);
}

Expand Down
Loading
Loading