From 7353a358aa3f364d1d31fd00c73a9e1a4b6dff4e Mon Sep 17 00:00:00 2001 From: ledwards2225 <98505400+ledwards2225@users.noreply.github.com> Date: Tue, 9 Jan 2024 10:53:05 -0700 Subject: [PATCH] feat: Standalone calldata test (#3842) Adds a basic gate construction method for a calldata lookup and a standalone test to leave the DataBus PoC work in a slightly better state while I'm distracted with other things. --- .../arithmetization/gate_data.hpp | 5 ++ .../goblin_ultra_circuit_builder.cpp | 78 ++++++++++-------- .../goblin_ultra_circuit_builder.hpp | 23 +++--- .../proof_system/instance_inspector.hpp | 18 ++-- .../sumcheck/instance/prover_instance.cpp | 1 + .../ultra_honk/databus_composer.test.cpp | 82 +++++++++---------- 6 files changed, 109 insertions(+), 98 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp b/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp index 91540d90d72..55afb319b0d 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp @@ -133,6 +133,11 @@ template struct ecc_dbl_gate_ { uint32_t y3; }; +template struct databus_lookup_gate_ { + uint32_t index; + uint32_t value; +}; + template struct poseidon2_external_gate_ { uint32_t a; uint32_t b; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp index 85e4a36cbb2..de95ff324cb 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp @@ -31,42 +31,15 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ // All that remains is to handle databus related and poseidon2 related polynomials. In what follows we populate the // calldata with some mock data then constuct a single calldata read gate - // Populate the calldata with some data - public_calldata.emplace_back(this->add_variable(FF(5))); - public_calldata.emplace_back(this->add_variable(FF(7))); - public_calldata.emplace_back(this->add_variable(FF(9))); - - // Construct read counts with length of calldata - calldata_read_counts.resize(public_calldata.size()); - for (auto& val : calldata_read_counts) { - val = 0; - } - - // Construct gate corresponding to a single calldata read - size_t read_idx = 1; // index into calldata array at which we want to read - this->w_l().emplace_back(public_calldata[read_idx]); // populate with value of calldata at read index - this->w_r().emplace_back(this->add_variable(FF(read_idx))); // populate with read index as witness - calldata_read_counts[read_idx]++; // increment read count at read index - q_busread().emplace_back(1); // read selector on - - // populate all other components with zero - this->w_o().emplace_back(this->zero_idx); - this->w_4().emplace_back(this->zero_idx); - this->q_m().emplace_back(0); - this->q_1().emplace_back(0); - this->q_2().emplace_back(0); - this->q_3().emplace_back(0); - this->q_c().emplace_back(0); - this->q_sort().emplace_back(0); - this->q_arith().emplace_back(0); - this->q_4().emplace_back(0); - this->q_lookup_type().emplace_back(0); - this->q_elliptic().emplace_back(0); - this->q_aux().emplace_back(0); - this->q_poseidon2_external().emplace_back(0); - this->q_poseidon2_internal().emplace_back(0); - - ++this->num_gates; + // Create an arbitrary calldata read gate + add_public_calldata(FF(25)); // ensure there is at least one entry in calldata + uint32_t raw_read_idx = 0; // read first entry in calldata + auto read_idx = this->add_variable(raw_read_idx); + FF calldata_value = this->get_variable(public_calldata[raw_read_idx]); + auto value_idx = this->add_variable(calldata_value); + create_calldata_lookup_gate({ read_idx, value_idx }); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts + calldata_read_counts[raw_read_idx]++; // mock gates that use poseidon selectors, with all zeros as input this->w_l().emplace_back(this->zero_idx); @@ -254,6 +227,39 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co equality_op_idx = this->put_constant_variable(FF(EccOpCode::EQUALITY)); } +/** + * @brief Create a calldata lookup/read gate + * + * @tparam FF + * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value + */ +template +void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_lookup_gate_& in) +{ + this->w_l().emplace_back(in.value); + this->w_r().emplace_back(in.index); + q_busread().emplace_back(1); + + // populate all other components with zero + this->w_o().emplace_back(this->zero_idx); + this->w_4().emplace_back(this->zero_idx); + this->q_m().emplace_back(0); + this->q_1().emplace_back(0); + this->q_2().emplace_back(0); + this->q_3().emplace_back(0); + this->q_c().emplace_back(0); + this->q_sort().emplace_back(0); + this->q_arith().emplace_back(0); + this->q_4().emplace_back(0); + this->q_lookup_type().emplace_back(0); + this->q_elliptic().emplace_back(0); + this->q_aux().emplace_back(0); + this->q_poseidon2_external().emplace_back(0); + this->q_poseidon2_internal().emplace_back(0); + + ++this->num_gates; +} + template void GoblinUltraCircuitBuilder_::create_poseidon2_external_gate(const poseidon2_external_gate_& in) { diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp index f077228effc..7d85c15b9dd 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp @@ -155,22 +155,21 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui } /** - * Make a witness variable a member of the public calldata. + * @brief Add a witness variable to the public calldata. * - * @param witness_index The index of the witness. + * @param in Value to be added to calldata. * */ - void set_public_calldata(const uint32_t witness_index) + uint32_t add_public_calldata(const FF& in) { - for (const uint32_t calldata : public_calldata) { - if (calldata == witness_index) { - if (!this->failed()) { - this->failure("Attempted to redundantly set a public calldata!"); - } - return; - } - } - public_calldata.emplace_back(witness_index); + const uint32_t index = this->add_variable(in); + public_calldata.emplace_back(index); + // Note: this is a bit inefficent to do every time but for safety these need to be coupled + calldata_read_counts.resize(public_calldata.size()); + return index; } + + void create_calldata_lookup_gate(const databus_lookup_gate_& in); + void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp b/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp index 63c2d61fbfe..dea90fe4ea1 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/instance_inspector.hpp @@ -48,19 +48,19 @@ void inspect_instance(auto& prover_instance) void print_databus_info(auto& prover_instance) { info("\nInstance Inspector: Printing databus gate info."); - auto& prover_polys = prover_instance->prover_polynomials; + auto& key = prover_instance->proving_key; for (size_t idx = 0; idx < prover_instance->proving_key->circuit_size; ++idx) { - if (prover_polys.q_busread[idx] == 1) { + if (key->q_busread[idx] == 1) { info("idx = ", idx); - info("q_busread = ", prover_polys.q_busread[idx]); - info("w_l = ", prover_polys.w_l[idx]); - info("w_r = ", prover_polys.w_r[idx]); + info("q_busread = ", key->q_busread[idx]); + info("w_l = ", key->w_l[idx]); + info("w_r = ", key->w_r[idx]); } - if (prover_polys.calldata_read_counts[idx] > 0) { + if (key->calldata_read_counts[idx] > 0) { info("idx = ", idx); - info("read_counts = ", prover_polys.calldata_read_counts[idx]); - info("calldata = ", prover_polys.calldata[idx]); - info("databus_id = ", prover_polys.databus_id[idx]); + info("read_counts = ", key->calldata_read_counts[idx]); + info("calldata = ", key->calldata[idx]); + info("databus_id = ", key->databus_id[idx]); } } info(); diff --git a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp index 340425429f8..e9c430b77c1 100644 --- a/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp +++ b/barretenberg/cpp/src/barretenberg/sumcheck/instance/prover_instance.cpp @@ -195,6 +195,7 @@ void ProverInstance_::construct_databus_polynomials(Circuit& circuit) // Note: We do not utilize a zero row for databus columns for (size_t idx = 0; idx < circuit.public_calldata.size(); ++idx) { public_calldata[idx] = circuit.get_variable(circuit.public_calldata[idx]); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts calldata_read_counts[idx] = circuit.calldata_read_counts[idx]; } 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 4ee34bab917..e9adb253f0c 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -3,8 +3,10 @@ #include #include "barretenberg/common/log.hpp" +#include "barretenberg/goblin/mock_circuits.hpp" #include "barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp" #include "barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp" +#include "barretenberg/proof_system/instance_inspector.hpp" #include "barretenberg/ultra_honk/ultra_composer.hpp" #include "barretenberg/ultra_honk/ultra_prover.hpp" @@ -26,59 +28,26 @@ class DataBusComposerTests : public ::testing::Test { using CommitmentKey = pcs::CommitmentKey; /** - * @brief Generate a simple test circuit with some ECC op gates and conventional arithmetic gates + * @brief Generate a simple test circuit that includes arithmetic and goblin ecc op 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 - * - */ - bool construct_and_verify_honk_proof(auto& composer, auto& builder) - { - auto instance = composer.create_instance(builder); - auto prover = composer.create_prover(instance); - auto verifier = composer.create_verifier(instance); - auto proof = prover.construct_proof(); - bool verified = verifier.verify_proof(proof); - - return verified; + // Add some ecc op gates and arithmetic gates + GoblinMockCircuits::construct_goblin_ecc_op_circuit(builder); + GoblinMockCircuits::construct_arithmetic_circuit(builder); } }; /** - * @brief Test proof construction/verification for a circuit with ECC op gates, public inputs, and basic arithmetic + * @brief Test proof construction/verification for a circuit with calldata lookup gates * gates * @note We simulate op queue interactions with a previous circuit so the actual circuit under test utilizes an op queue * with non-empty 'previous' data. This avoid complications with zero-commitments etc. * */ -TEST_F(DataBusComposerTests, SingleCircuit) +TEST_F(DataBusComposerTests, CallDataRead) { auto op_queue = std::make_shared(); @@ -87,13 +56,44 @@ TEST_F(DataBusComposerTests, SingleCircuit) auto builder = proof_system::GoblinUltraCircuitBuilder{ op_queue }; + // Create a general test circuit generate_test_circuit(builder); + // Add some values to calldata and store the corresponding witness index + std::array calldata_values = { 7, 10, 3, 12, 1 }; + for (auto& val : calldata_values) { + builder.add_public_calldata(val); + } + + // Define some indices at which to read calldata + std::array read_index_values = { 1, 4 }; + + // Create some calldata lookup gates + for (uint32_t& read_index : read_index_values) { + // Create a variable corresponding to the index at which we want to read into calldata + uint32_t read_idx_witness_idx = builder.add_variable(read_index); + + // Create a variable corresponding to the result of the read. Note that we do not in general connect reads from + // calldata via copy constraints (i.e. we create a unique variable for the result of each read) + ASSERT_LT(read_index, builder.public_calldata.size()); + FF calldata_value = builder.get_variable(builder.public_calldata[read_index]); + uint32_t value_witness_idx = builder.add_variable(calldata_value); + + builder.create_calldata_lookup_gate({ read_idx_witness_idx, value_witness_idx }); + // TODO(https://github.com/AztecProtocol/barretenberg/issues/821): automate updating of read counts + builder.calldata_read_counts[read_index]++; + } + auto composer = GoblinUltraComposer(); // Construct and verify Honk proof - auto honk_verified = construct_and_verify_honk_proof(composer, builder); - EXPECT_TRUE(honk_verified); + auto instance = composer.create_instance(builder); + // For debugging, use "instance_inspector::print_databus_info(instance)" + auto prover = composer.create_prover(instance); + auto verifier = composer.create_verifier(instance); + auto proof = prover.construct_proof(); + bool verified = verifier.verify_proof(proof); + EXPECT_TRUE(verified); } } // namespace test_ultra_honk_composer