From 9d67a9afd519f6e8f32dd99ade0737746bd626a2 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 11:34:53 +0000 Subject: [PATCH 1/7] rework code so goblin is not used in mock_circuits --- .../barretenberg/benchmark/goblin_bench/goblin.bench.cpp | 5 ++++- .../cpp/src/barretenberg/goblin/goblin_recursion.test.cpp | 4 +++- barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp | 6 +++++- .../src/barretenberg/goblin/mock_circuits_pinning.test.cpp | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp index 92f36e885f0d..237be4c7868b 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp @@ -48,7 +48,10 @@ class GoblinBench : public benchmark::Fixture { // Construct and accumulate the mock kernel circuit // Note: in first round, kernel_accum is empty since there is no previous kernel to recursively verify GoblinUltraCircuitBuilder circuit_builder{ goblin.op_queue }; - GoblinMockCircuits::construct_mock_recursion_kernel_circuit(circuit_builder, function_accum, kernel_accum); + GoblinMockCircuits::construct_mock_recursion_kernel_circuit( + circuit_builder, + { function_accum.proof, function_accum.verification_key }, + { kernel_accum.proof, kernel_accum.verification_key }); kernel_accum = goblin.accumulate(circuit_builder); } } diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp index 96463863ebbc..5ecc0fcf82a3 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp @@ -58,7 +58,9 @@ TEST_F(GoblinRecursionTests, Vanilla) // Construct and accumulate the mock kernel circuit (no kernel accum in first round) GoblinUltraCircuitBuilder kernel_circuit{ goblin.op_queue }; - GoblinMockCircuits::construct_mock_kernel_small(kernel_circuit, function_accum, kernel_accum); + GoblinMockCircuits::construct_mock_kernel_small(kernel_circuit, + { function_accum.proof, function_accum.verification_key }, + { kernel_accum.proof, kernel_accum.verification_key }); info("kernel accum"); goblin.merge(kernel_circuit); kernel_accum = construct_accumulator(kernel_circuit); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index 884e5c27ab87..7cb78107421a 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -30,13 +30,17 @@ class GoblinMockCircuits { using Flavor = bb::GoblinUltraFlavor; using RecursiveFlavor = bb::GoblinUltraRecursiveFlavor_; using RecursiveVerifier = bb::stdlib::recursion::honk::UltraRecursiveVerifier_; - using KernelInput = Goblin::AccumulationOutput; using VerifierInstance = bb::VerifierInstance_; using RecursiveVerifierInstance = ::bb::stdlib::recursion::honk::RecursiveVerifierInstance_; using RecursiveVerifierAccumulator = std::shared_ptr; using VerificationKey = Flavor::VerificationKey; static constexpr size_t NUM_OP_QUEUE_COLUMNS = Flavor::NUM_WIRES; + struct KernelInput { + HonkProof proof; + std::shared_ptr verification_key; + }; + /** * @brief Information required by the verifier to verify a folding round besides the previous accumulator. */ diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp index 0b9e06e0ff82..44c056ae06cb 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp @@ -45,7 +45,10 @@ TEST_F(MockCircuits, PinRecursionKernelSizes) GoblinMockCircuits::construct_mock_function_circuit(app_circuit, large); auto function_accum = goblin.accumulate(app_circuit); GoblinUltraCircuitBuilder kernel_circuit{ goblin.op_queue }; - GoblinMockCircuits::construct_mock_recursion_kernel_circuit(kernel_circuit, function_accum, kernel_accum); + GoblinMockCircuits::construct_mock_recursion_kernel_circuit( + kernel_circuit, + { function_accum.proof, function_accum.verification_key }, + { kernel_accum.proof, kernel_accum.verification_key }); auto instance = std::make_shared(kernel_circuit); if (large) { From 82b64283abb13feb165d00344d8d4e5750d3f7f6 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 12:10:40 +0000 Subject: [PATCH 2/7] ensure first op queue mocking is internal to the Goblin and not done in benchmarks; make it unique function --- .../benchmark/goblin_bench/goblin.bench.cpp | 13 ----- .../barretenberg/client_ivc/client_ivc.cpp | 12 +---- .../barretenberg/client_ivc/client_ivc.hpp | 2 - .../cpp/src/barretenberg/goblin/goblin.hpp | 2 + .../goblin/goblin_recursion.test.cpp | 3 -- .../src/barretenberg/goblin/mock_circuits.hpp | 6 +-- .../goblin/mock_circuits_pinning.test.cpp | 1 - .../proof_system/op_queue/ecc_op_queue.hpp | 24 --------- .../honk/verifier/goblin_verifier.test.cpp | 6 ++- .../honk/verifier/merge_verifier.test.cpp | 8 ++- .../ultra_honk/databus_composer.test.cpp | 4 +- .../ultra_honk/goblin_ultra_composer.test.cpp | 53 ++++--------------- 12 files changed, 30 insertions(+), 104 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp index 237be4c7868b..0c9760933b9b 100644 --- a/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp +++ b/barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/goblin.bench.cpp @@ -65,10 +65,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinFull)(benchmark::State& state) { Goblin goblin; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723): Simply populate the OpQueue with some data - // and corresponding commitments so the merge protocol has "prev" data into which it can accumulate - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); - for (auto _ : state) { BB_REPORT_OP_COUNT_IN_BENCH(state); // Perform a specified number of iterations of function/kernel accumulation @@ -87,9 +83,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinAccumulate)(benchmark::State& state) { Goblin goblin; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723) - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); - // Perform a specified number of iterations of function/kernel accumulation for (auto _ : state) { perform_goblin_accumulation_rounds(state, goblin); @@ -104,9 +97,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinECCVMProve)(benchmark::State& state) { Goblin goblin; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723) - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); - // Perform a specified number of iterations of function/kernel accumulation perform_goblin_accumulation_rounds(state, goblin); @@ -124,9 +114,6 @@ BENCHMARK_DEFINE_F(GoblinBench, GoblinTranslatorProve)(benchmark::State& state) { Goblin goblin; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723) - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); - // Perform a specified number of iterations of function/kernel accumulation perform_goblin_accumulation_rounds(state, goblin); diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp index 69408f8aee92..db20524dd935 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp @@ -2,12 +2,6 @@ namespace bb { -ClientIVC::ClientIVC() -{ - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723): - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); -} - /** * @brief Initialize the IVC with a first circuit * @details Initializes the accumulator and performs the initial goblin merge @@ -125,10 +119,8 @@ void ClientIVC::precompute_folding_verification_keys() vks.kernel_vk = std::make_shared(prover_instance->proving_key); - // Clean the ivc state - goblin.op_queue = std::make_shared(); - goblin.merge_proof_exists = false; - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); + // Clean the Goblin state (reinitialise op_queue with mocking and clear merge proofs) + goblin = Goblin(); } } // namespace bb \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp index 1d26519f323d..f2e0ee309c3b 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.hpp @@ -63,8 +63,6 @@ class ClientIVC { // be needed in the real IVC as they are provided as inputs std::shared_ptr prover_instance; - ClientIVC(); - void initialize(ClientCircuit& circuit); FoldProof accumulate(ClientCircuit& circuit); diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 339d7f1f03ad..6ac3114372ba 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -2,6 +2,7 @@ #include "barretenberg/eccvm/eccvm_composer.hpp" #include "barretenberg/flavor/goblin_ultra.hpp" +#include "barretenberg/goblin/mock_circuits.hpp" #include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp" @@ -88,6 +89,7 @@ class Goblin { AccumulationOutput accumulator; // Used only for ACIR methods for now public: + Goblin() { GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); } /** * @brief Construct a GUH proof and a merge proof for the present circuit. * @details If there is a previous merge proof, recursively verify it. diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp index 5ecc0fcf82a3..2f10cd038457 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin_recursion.test.cpp @@ -42,9 +42,6 @@ TEST_F(GoblinRecursionTests, Vanilla) Goblin::AccumulationOutput kernel_accum; - // TODO(https://github.com/AztecProtocol/barretenberg/issues/723): - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); - size_t NUM_CIRCUITS = 2; for (size_t circuit_idx = 0; circuit_idx < NUM_CIRCUITS; ++circuit_idx) { diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index 7cb78107421a..c1dcb4eb2c03 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -7,7 +7,6 @@ #include "barretenberg/crypto/merkle_tree/memory_store.hpp" #include "barretenberg/crypto/merkle_tree/merkle_tree.hpp" #include "barretenberg/flavor/goblin_ultra.hpp" -#include "barretenberg/goblin/goblin.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp" #include "barretenberg/srs/global_crs.hpp" #include "barretenberg/stdlib/encryption/ecdsa/ecdsa.hpp" @@ -167,11 +166,8 @@ class GoblinMockCircuits { * * @param builder */ - static void construct_simple_initial_circuit(GoblinUltraBuilder& builder) + static void construct_simple_circuit(GoblinUltraBuilder& builder) { - // TODO(https://github.com/AztecProtocol/barretenberg/issues/800) Testing cleanup - perform_op_queue_interactions_for_mock_first_circuit(builder.op_queue); - // Add some arbitrary ecc op gates for (size_t i = 0; i < 3; ++i) { auto point = Point::random_element(); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp index 44c056ae06cb..3646a5be696e 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits_pinning.test.cpp @@ -39,7 +39,6 @@ TEST_F(MockCircuits, PinRecursionKernelSizes) const auto run_test = [](bool large) { { Goblin goblin; - GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(goblin.op_queue); Goblin::AccumulationOutput kernel_accum; GoblinUltraCircuitBuilder app_circuit{ goblin.op_queue }; GoblinMockCircuits::construct_mock_function_circuit(app_circuit, large); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp index 3443fbdd2e01..7afee28f38fe 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/op_queue/ecc_op_queue.hpp @@ -166,30 +166,6 @@ class ECCOpQueue { return result; } - /** - * @brief TESTING PURPOSES ONLY: Populate ECC op queue with mock data as stand in for "previous circuit" in tests - * @details TODO(#723): We currently cannot support Goblin proofs (specifically, transcript aggregation) if there - * is not existing data in the ECC op queue (since this leads to zero-commitment issues). This method populates the - * op queue with mock data so that the prover of an arbitrary 'first' circuit can behave as if it were not the - * prover over the first circuit in the stack. This method should be removed entirely once this is resolved. - * - * @param op_queue - */ - void populate_with_mock_initital_data() - { - // Add a single row of data to the op queue and commit to each column as [1] * FF(data) - std::array mock_op_queue_commitments; - for (size_t idx = 0; idx < 4; idx++) { - auto mock_data = Fr::random_element(); - this->ultra_ops[idx].emplace_back(mock_data); - mock_op_queue_commitments[idx] = Point::one() * mock_data; - } - // Set some internal data based on the size of the op queue data - this->set_size_data(); - // Add the commitments to the op queue data for use by the next circuit - this->set_commitment_data(mock_op_queue_commitments); - } - /** * @brief Write point addition op to queue and natively perform addition * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp index 242977c212ba..362b90af041f 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/goblin_verifier.test.cpp @@ -58,9 +58,13 @@ template class GoblinRecursiveVerifierTest : public testi // Instantiate ECC op queue and add mock data to simulate interaction with a previous circuit auto op_queue = std::make_shared(); - op_queue->populate_with_mock_initital_data(); InnerBuilder builder(op_queue); + // Add a mul accum op and an equality op + auto p = point::one() * fr::random_element(); + auto scalar = fr::random_element(); + builder.queue_ecc_mul_accum(p, scalar); + builder.queue_ecc_eq(); // Create 2^log_n many add gates based on input log num gates const size_t num_gates = 1 << log_num_gates; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/merge_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/merge_verifier.test.cpp index 011f26e7c9d3..f20c79a6c193 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/merge_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/merge_verifier.test.cpp @@ -1,7 +1,11 @@ +#include "barretenberg/ultra_honk/merge_verifier.hpp" #include "barretenberg/common/test.hpp" #include "barretenberg/goblin/mock_circuits.hpp" #include "barretenberg/stdlib/primitives/curves/bn254.hpp" #include "barretenberg/stdlib/recursion/honk/verifier/merge_recursive_verifier.hpp" +#include "barretenberg/ultra_honk/merge_prover.hpp" +#include "barretenberg/ultra_honk/ultra_prover.hpp" +#include "barretenberg/ultra_honk/ultra_verifier.hpp" namespace bb::stdlib::recursion::goblin { @@ -42,9 +46,11 @@ class RecursiveMergeVerifierTest : public testing::Test { static void test_recursive_merge_verification() { auto op_queue = std::make_shared(); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/800) Testing cleanup + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); InnerBuilder sample_circuit{ op_queue }; - GoblinMockCircuits::construct_simple_initial_circuit(sample_circuit); + GoblinMockCircuits::construct_simple_circuit(sample_circuit); // Generate a proof over the inner circuit MergeProver merge_prover{ op_queue }; diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index 089a6e14572d..eb6c858f3274 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -9,6 +9,8 @@ #include "barretenberg/proof_system/instance_inspector.hpp" #include "barretenberg/ultra_honk/ultra_prover.hpp" +#include "barretenberg/ultra_honk/ultra_verifier.hpp" + using namespace bb; namespace { @@ -49,7 +51,7 @@ TEST_F(DataBusComposerTests, CallDataRead) auto op_queue = std::make_shared(); // Add mock data to op queue to simulate interaction with a previous circuit - op_queue->populate_with_mock_initital_data(); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); auto builder = GoblinUltraCircuitBuilder{ op_queue }; diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp index bed11a6fb4b9..7766c045fa73 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp @@ -3,6 +3,8 @@ #include #include "barretenberg/common/log.hpp" +#include "barretenberg/goblin/mock_circuits.hpp" +#include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp" #include "barretenberg/ultra_honk/merge_prover.hpp" @@ -26,36 +28,6 @@ class GoblinUltraHonkComposerTests : public ::testing::Test { using MergeProver = MergeProver_; using MergeVerifier = MergeVerifier_; - /** - * @brief Generate a simple test circuit with some ECC op gates and conventional arithmetic gates - * - * @param builder - */ - void generate_test_circuit(auto& builder) - { - // Add some ecc op gates - for (size_t i = 0; i < 3; ++i) { - auto point = Point::one() * FF::random_element(); - auto scalar = FF::random_element(); - builder.queue_ecc_mul_accum(point, scalar); - } - builder.queue_ecc_eq(); - - // Add some conventional gates that utilize public inputs - for (size_t i = 0; i < 10; ++i) { - FF a = FF::random_element(); - FF b = FF::random_element(); - FF c = FF::random_element(); - FF d = a + b + c; - uint32_t a_idx = builder.add_public_variable(a); - uint32_t b_idx = builder.add_variable(b); - uint32_t c_idx = builder.add_variable(c); - uint32_t d_idx = builder.add_variable(d); - - builder.create_big_add_gate({ a_idx, b_idx, c_idx, d_idx, FF(1), FF(1), FF(1), FF(-1), FF(0) }); - } - } - /** * @brief Construct and a verify a Honk proof * @@ -99,12 +71,10 @@ TEST_F(GoblinUltraHonkComposerTests, SingleCircuit) { auto op_queue = std::make_shared(); - // Add mock data to op queue to simulate interaction with a previous circuit - op_queue->populate_with_mock_initital_data(); - + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); auto builder = GoblinUltraCircuitBuilder{ op_queue }; - generate_test_circuit(builder); + GoblinMockCircuits::construct_simple_circuit(builder); // Construct and verify Honk proof bool honk_verified = construct_and_verify_honk_proof(builder); @@ -125,15 +95,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsMergeOnly) // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - // Add mock data to op queue to simulate interaction with a previous circuit - op_queue->populate_with_mock_initital_data(); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { auto builder = GoblinUltraCircuitBuilder{ op_queue }; - generate_test_circuit(builder); + GoblinMockCircuits::construct_simple_circuit(builder); // Construct and verify Goblin ECC op queue Merge proof auto merge_verified = construct_and_verify_merge_proof(op_queue); @@ -151,15 +120,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsHonkOnly) // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - // Add mock data to op queue to simulate interaction with a previous circuit - op_queue->populate_with_mock_initital_data(); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { auto builder = GoblinUltraCircuitBuilder{ op_queue }; - generate_test_circuit(builder); + GoblinMockCircuits::construct_simple_circuit(builder); // Construct and verify Honk proof bool honk_verified = construct_and_verify_honk_proof(builder); @@ -177,15 +145,14 @@ TEST_F(GoblinUltraHonkComposerTests, MultipleCircuitsHonkAndMerge) // Instantiate EccOpQueue. This will be shared across all circuits in the series auto op_queue = std::make_shared(); - // Add mock data to op queue to simulate interaction with a previous circuit - op_queue->populate_with_mock_initital_data(); + GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); // Construct multiple test circuits that share an ECC op queue. Generate and verify a proof for each. size_t NUM_CIRCUITS = 3; for (size_t i = 0; i < NUM_CIRCUITS; ++i) { auto builder = GoblinUltraCircuitBuilder{ op_queue }; - generate_test_circuit(builder); + GoblinMockCircuits::construct_simple_circuit(builder); // Construct and verify Honk proof bool honk_verified = construct_and_verify_honk_proof(builder); From 5371b605b8b65c3c627a1046b4a2250756a991b8 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 12:19:18 +0000 Subject: [PATCH 3/7] remove unnecessary include --- .../src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp index 7766c045fa73..f998b8f492ab 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/goblin_ultra_composer.test.cpp @@ -4,7 +4,6 @@ #include "barretenberg/common/log.hpp" #include "barretenberg/goblin/mock_circuits.hpp" -#include "barretenberg/proof_system/circuit_builder/eccvm/eccvm_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp" #include "barretenberg/ultra_honk/merge_prover.hpp" From 8191d2d93c1fcdc5a50c61c78b7945098456c092 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 13:39:18 +0000 Subject: [PATCH 4/7] fix acir tests? --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index a08bf91a12b7..975df4786eda 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -144,10 +144,6 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); - // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation - acir_proofs::GoblinAcirComposer acir_composer; - acir_composer.create_circuit(constraint_system, witness); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set // to max circuit size present in acir tests suite. size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; @@ -155,6 +151,10 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); + // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation + acir_proofs::GoblinAcirComposer acir_composer; + acir_composer.create_circuit(constraint_system, witness); + // Call accumulate to generate a GoblinUltraHonk proof auto proof = acir_composer.accumulate(); @@ -183,10 +183,6 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& wi auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); - // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation - acir_proofs::GoblinAcirComposer acir_composer; - acir_composer.create_circuit(constraint_system, witness); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set // to max circuit size present in acir tests suite. size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; @@ -194,6 +190,10 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& wi size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); + // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation + acir_proofs::GoblinAcirComposer acir_composer; + acir_composer.create_circuit(constraint_system, witness); + // Generate a GoblinUltraHonk proof and a full Goblin proof auto proof = acir_composer.accumulate_and_prove(); @@ -222,9 +222,9 @@ void prove(const std::string& bytecodePath, const std::string& witnessPath, cons acir_proofs::AcirComposer acir_composer{ 0, verbose }; acir_composer.create_circuit(constraint_system, witness); - init_bn254_crs(acir_composer.get_dyadic_circuit_size()); acir_composer.init_proving_key(); auto proof = acir_composer.create_proof(); + init_bn254_crs(acir_composer.get_dyadic_circuit_size()); if (outputPath == "-") { writeRawBytesToStdout(proof); From 1e3e4472bcbb59bbdfc1de60f289e86e0f98cb98 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 13:54:07 +0000 Subject: [PATCH 5/7] hm --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 18 +++++++++--------- .../src/barretenberg/goblin/mock_circuits.hpp | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 975df4786eda..a08bf91a12b7 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -144,6 +144,10 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); + // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation + acir_proofs::GoblinAcirComposer acir_composer; + acir_composer.create_circuit(constraint_system, witness); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set // to max circuit size present in acir tests suite. size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; @@ -151,10 +155,6 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); - // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation - acir_proofs::GoblinAcirComposer acir_composer; - acir_composer.create_circuit(constraint_system, witness); - // Call accumulate to generate a GoblinUltraHonk proof auto proof = acir_composer.accumulate(); @@ -183,6 +183,10 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& wi auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); + // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation + acir_proofs::GoblinAcirComposer acir_composer; + acir_composer.create_circuit(constraint_system, witness); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set // to max circuit size present in acir tests suite. size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; @@ -190,10 +194,6 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& wi size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); - // Instantiate a Goblin acir composer and construct a bberg circuit from the acir representation - acir_proofs::GoblinAcirComposer acir_composer; - acir_composer.create_circuit(constraint_system, witness); - // Generate a GoblinUltraHonk proof and a full Goblin proof auto proof = acir_composer.accumulate_and_prove(); @@ -222,9 +222,9 @@ void prove(const std::string& bytecodePath, const std::string& witnessPath, cons acir_proofs::AcirComposer acir_composer{ 0, verbose }; acir_composer.create_circuit(constraint_system, witness); + init_bn254_crs(acir_composer.get_dyadic_circuit_size()); acir_composer.init_proving_key(); auto proof = acir_composer.create_proof(); - init_bn254_crs(acir_composer.get_dyadic_circuit_size()); if (outputPath == "-") { writeRawBytesToStdout(proof); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index c1dcb4eb2c03..97876b0e624d 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -151,6 +151,7 @@ class GoblinMockCircuits { op_queue->set_size_data(); // 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()); std::array op_queue_commitments; size_t idx = 0; From 2adaa14370882f6f6608c0f2ce1481724fe53da5 Mon Sep 17 00:00:00 2001 From: maramihali Date: Mon, 11 Mar 2024 14:11:53 +0000 Subject: [PATCH 6/7] initalise crs at the correct time? --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index a08bf91a12b7..6a0421da4b04 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -140,6 +140,13 @@ bool proveAndVerify(const std::string& bytecodePath, const std::string& witnessP */ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::string& witnessPath) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set + // to max circuit size present in acir tests suite. + size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; + init_bn254_crs(hardcoded_bn254_dyadic_size_hack); + size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only + init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); + // Populate the acir constraint system and witness from gzipped data auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); @@ -148,13 +155,6 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin acir_proofs::GoblinAcirComposer acir_composer; acir_composer.create_circuit(constraint_system, witness); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set - // to max circuit size present in acir tests suite. - size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; - init_bn254_crs(hardcoded_bn254_dyadic_size_hack); - size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only - init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); - // Call accumulate to generate a GoblinUltraHonk proof auto proof = acir_composer.accumulate(); @@ -179,6 +179,13 @@ bool accumulateAndVerifyGoblin(const std::string& bytecodePath, const std::strin */ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& witnessPath) { + // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set + // to max circuit size present in acir tests suite. + size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; + init_bn254_crs(hardcoded_bn254_dyadic_size_hack); + size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only + init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); + // Populate the acir constraint system and witness from gzipped data auto constraint_system = get_constraint_system(bytecodePath); auto witness = get_witness(witnessPath); @@ -187,13 +194,6 @@ bool proveAndVerifyGoblin(const std::string& bytecodePath, const std::string& wi acir_proofs::GoblinAcirComposer acir_composer; acir_composer.create_circuit(constraint_system, witness); - // TODO(https://github.com/AztecProtocol/barretenberg/issues/811): Don't hardcode dyadic circuit size. Currently set - // to max circuit size present in acir tests suite. - size_t hardcoded_bn254_dyadic_size_hack = 1 << 19; - init_bn254_crs(hardcoded_bn254_dyadic_size_hack); - size_t hardcoded_grumpkin_dyadic_size_hack = 1 << 10; // For eccvm only - init_grumpkin_crs(hardcoded_grumpkin_dyadic_size_hack); - // Generate a GoblinUltraHonk proof and a full Goblin proof auto proof = acir_composer.accumulate_and_prove(); From d96ef572c7d2203686ae8a0a79c0dd9c58e75b76 Mon Sep 17 00:00:00 2001 From: maramihali Date: Tue, 12 Mar 2024 19:39:48 +0000 Subject: [PATCH 7/7] add todo comment with issue --- .../cpp/src/barretenberg/client_ivc/client_ivc.cpp | 3 +++ barretenberg/cpp/src/barretenberg/goblin/goblin.hpp | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp index db20524dd935..eef0001e0c2a 100644 --- a/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp +++ b/barretenberg/cpp/src/barretenberg/client_ivc/client_ivc.cpp @@ -77,6 +77,9 @@ HonkProof ClientIVC::decider_prove() const * recursive merge verifier), initial kernel verification key (with recursive merge verifier appended, no previous * kernel to fold), "full" kernel verification key( two recursive folding verifiers and merge verifier). * + * TODO(https://github.com/AztecProtocol/barretenberg/issues/904): This function should ultimately be moved outside of + * this class since it's used only for testing and benchmarking purposes and it requires us to clear state afterwards. + * (e.g. in the Goblin object) */ void ClientIVC::precompute_folding_verification_keys() { diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 6ac3114372ba..f273170edd2b 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -89,7 +89,12 @@ class Goblin { AccumulationOutput accumulator; // Used only for ACIR methods for now public: - Goblin() { GoblinMockCircuits::perform_op_queue_interactions_for_mock_first_circuit(op_queue); } + Goblin() + { // 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); + } /** * @brief Construct a GUH proof and a merge proof for the present circuit. * @details If there is a previous merge proof, recursively verify it.