From 0be9f253238cc1453d07385ece565f946d4212a3 Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:37:47 -0700 Subject: [PATCH] feat: handle consecutive kernels in IVC (#8924) Prior to this PR, the IVC scheme made the assumption that every other circuit was a kernel, i.e. app, kernel, app, kernel, ... etc. In practice, however, it will be common to have two or more consecutive kernels without a corresponding app, e.g. an inner kernel followed immediately by a reset kernel. This PR updates the IVC so that whether or not a circuit is treated as a kernel is determined by the the already existing tag `circuit.databus_propagation_data.is_kernel`. When constructing circuits from noir programs, the above flag is set to true if and only if the circuit has calldata (which apps never do). This allows us to reinstate the full set of circuits in the ivc integration test suite which contains 3 consecutive kernels (inner, reset, tail). In accordance with this change I had to add explicit setting of `is_kernel` to various test suites and flows which previously utilized the default assumption that every other circuit was a kernel. (Many of these cases will soon go away once we are ready to do away with the `auto_verify_mode` version of IVC which exists for testing convenience and does not have any practical use case). Closes https://github.com/AztecProtocol/barretenberg/issues/1111 --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 17 ++++++ .../barretenberg/client_ivc/client_ivc.cpp | 4 +- .../barretenberg/client_ivc/client_ivc.hpp | 1 - .../client_ivc_auto_verify.test.cpp | 61 +++++++++++++------ .../client_ivc_integration.test.cpp | 29 +++++++++ .../client_ivc/mock_circuit_producer.hpp | 18 ++++-- .../dsl/acir_format/block_constraint.cpp | 4 ++ .../barretenberg/dsl/acir_proofs/c_bind.cpp | 4 ++ .../client_ivc_recursive_verifier.test.cpp | 1 + .../src/client_ivc_integration.test.ts | 55 ++++++++--------- 10 files changed, 137 insertions(+), 57 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index ad3b4e111df..0d902ade0f1 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -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(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); } @@ -458,6 +467,7 @@ 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(); @@ -465,9 +475,13 @@ bool foldAndVerifyProgram(const std::string& bytecodePath, const std::string& wi auto builder = acir_format::create_circuit( 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(); } @@ -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( 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); diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp index 2309dfe689f..66926c1a07d 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp @@ -148,9 +148,7 @@ void ClientIVC::complete_kernel_circuit_logic(ClientCircuit& circuit) */ void ClientIVC::accumulate(ClientCircuit& circuit, const std::shared_ptr& precomputed_vk, bool mock_vk) { - is_kernel = !is_kernel; // toggle on each call (every even circuit is a kernel) - - if (auto_verify_mode && is_kernel) { + if (auto_verify_mode && circuit.databus_propagation_data.is_kernel) { complete_kernel_circuit_logic(circuit); } diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp index 8ccc371efad..8f0f3400f00 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp @@ -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 diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_auto_verify.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_auto_verify.test.cpp index 5e413362f7b..dc76f673d21 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_auto_verify.test.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_auto_verify.test.cpp @@ -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); @@ -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; } }; @@ -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); + 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()); }; @@ -90,7 +112,8 @@ TEST_F(ClientIVCAutoVerifyTests, BasicLarge) size_t NUM_CIRCUITS = 6; std::vector 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 @@ -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); @@ -139,7 +162,8 @@ TEST_F(ClientIVCAutoVerifyTests, PrecomputedVerificationKeys) size_t NUM_CIRCUITS = 4; std::vector 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 @@ -167,7 +191,8 @@ TEST_F(ClientIVCAutoVerifyTests, StructuredPrecomputedVKs) size_t NUM_CIRCUITS = 4; std::vector 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 diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_integration.test.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_integration.test.cpp index aae13a136a2..fe62b038db2 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc_integration.test.cpp @@ -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 diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/mock_circuit_producer.hpp b/barretenberg/cpp/src/barretenberg/client_ivc/mock_circuit_producer.hpp index 5946a222bff..7e2332f5309 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/mock_circuit_producer.hpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/mock_circuit_producer.hpp @@ -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)); } @@ -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) { diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 4a81d284b5f..0e2bf5d74de 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -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); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp index 170140dc1e3..1f4853ade0c 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_proofs/c_bind.cpp @@ -96,6 +96,7 @@ 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(); @@ -103,6 +104,9 @@ WASM_EXPORT void acir_fold_and_verify_program_stack(uint8_t const* acir_vec, uin auto builder = acir_format::create_circuit( 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(); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/client_ivc_verifier/client_ivc_recursive_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/client_ivc_verifier/client_ivc_recursive_verifier.test.cpp index 0bb5b61075d..4771632be21 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/client_ivc_verifier/client_ivc_recursive_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/client_ivc_verifier/client_ivc_recursive_verifier.test.cpp @@ -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); } diff --git a/yarn-project/ivc-integration/src/client_ivc_integration.test.ts b/yarn-project/ivc-integration/src/client_ivc_integration.test.ts index 7ec2e0d03ff..1a8409f3588 100644 --- a/yarn-project/ivc-integration/src/client_ivc_integration.test.ts +++ b/yarn-project/ivc-integration/src/client_ivc_integration.test.ts @@ -10,13 +10,18 @@ import path from 'path'; import { fileURLToPath } from 'url'; import { + MOCK_MAX_COMMITMENTS_PER_TX, MockAppCreatorCircuit, MockAppReaderCircuit, MockPrivateKernelInitCircuit, MockPrivateKernelInnerCircuit, + MockPrivateKernelResetCircuit, + MockPrivateKernelTailCircuit, witnessGenCreatorAppMockCircuit, witnessGenMockPrivateKernelInitCircuit, witnessGenMockPrivateKernelInnerCircuit, + witnessGenMockPrivateKernelResetCircuit, + witnessGenMockPrivateKernelTailCircuit, witnessGenReaderAppMockCircuit, } from './index.js'; @@ -74,22 +79,16 @@ describe('Client IVC Integration', () => { tx, }); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/1101): While using auto_veriy_mode, we can only process an even - // number of circuits into the IVC. This restriction can be removed once we remove use of auto_verify_mode. - // const tailWitnessGenResult = await witnessGenMockPrivateKernelTailCircuit({ - // prev_kernel_public_inputs: initWitnessGenResult.publicInputs, - // }); + const tailWitnessGenResult = await witnessGenMockPrivateKernelTailCircuit({ + prev_kernel_public_inputs: initWitnessGenResult.publicInputs, + }); // Create client IVC proof const bytecodes = [ MockAppCreatorCircuit.bytecode, MockPrivateKernelInitCircuit.bytecode, - // MockPrivateKernelTailCircuit.bytecode, - ]; - const witnessStack = [ - appWitnessGenResult.witness, - initWitnessGenResult.witness, - // tailWitnessGenResult.witness + MockPrivateKernelTailCircuit.bytecode, ]; + const witnessStack = [appWitnessGenResult.witness, initWitnessGenResult.witness, tailWitnessGenResult.witness]; const proof = await createClientIvcProof(witnessStack, bytecodes); await proof.writeToOutputDirectory(bbWorkingDirectory); @@ -122,21 +121,19 @@ describe('Client IVC Integration', () => { app_inputs: readerAppWitnessGenResult.publicInputs, }); - // TODO: https://github.com/AztecProtocol/barretenberg/issues/1111 - Add back reset and tail when this is fixed. - - // const resetWitnessGenResult = await witnessGenMockPrivateKernelResetCircuit({ - // prev_kernel_public_inputs: innerWitnessGenResult.publicInputs, - // commitment_read_hints: [ - // '0x1', // Reader reads commitment 0x2, which is at index 1 of the created commitments - // MOCK_MAX_COMMITMENTS_PER_TX.toString(), // Pad with no-ops - // MOCK_MAX_COMMITMENTS_PER_TX.toString(), - // MOCK_MAX_COMMITMENTS_PER_TX.toString(), - // ], - // }); + const resetWitnessGenResult = await witnessGenMockPrivateKernelResetCircuit({ + prev_kernel_public_inputs: innerWitnessGenResult.publicInputs, + commitment_read_hints: [ + '0x1', // Reader reads commitment 0x2, which is at index 1 of the created commitments + MOCK_MAX_COMMITMENTS_PER_TX.toString(), // Pad with no-ops + MOCK_MAX_COMMITMENTS_PER_TX.toString(), + MOCK_MAX_COMMITMENTS_PER_TX.toString(), + ], + }); - // const tailWitnessGenResult = await witnessGenMockPrivateKernelTailCircuit({ - // prev_kernel_public_inputs: resetWitnessGenResult.publicInputs, - // }); + const tailWitnessGenResult = await witnessGenMockPrivateKernelTailCircuit({ + prev_kernel_public_inputs: resetWitnessGenResult.publicInputs, + }); // Create client IVC proof const bytecodes = [ @@ -144,16 +141,16 @@ describe('Client IVC Integration', () => { MockPrivateKernelInitCircuit.bytecode, MockAppReaderCircuit.bytecode, MockPrivateKernelInnerCircuit.bytecode, - // MockPrivateKernelResetCircuit.bytecode, - // MockPrivateKernelTailCircuit.bytecode, + MockPrivateKernelResetCircuit.bytecode, + MockPrivateKernelTailCircuit.bytecode, ]; const witnessStack = [ creatorAppWitnessGenResult.witness, initWitnessGenResult.witness, readerAppWitnessGenResult.witness, innerWitnessGenResult.witness, - // resetWitnessGenResult.witness, - // tailWitnessGenResult.witness, + resetWitnessGenResult.witness, + tailWitnessGenResult.witness, ]; const proof = await createClientIvcProof(witnessStack, bytecodes);