From 32ceb3aa767c51a3e688d2e5229347e235c1255d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 4 Jan 2024 19:18:41 +0000 Subject: [PATCH 01/11] call data read gate fctn --- .../goblin_ultra_circuit_builder.cpp | 61 +++++++++++-------- .../goblin_ultra_circuit_builder.hpp | 3 + 2 files changed, 40 insertions(+), 24 deletions(-) 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..9e3fdf70b57 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 @@ -43,30 +43,8 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ } // 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; + uint32_t read_idx = 1; // index into calldata array at which we want to read + create_calldata_read_gate(read_idx); // mock gates that use poseidon selectors, with all zeros as input this->w_l().emplace_back(this->zero_idx); @@ -254,6 +232,41 @@ 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 read_index Raw index into calldata array + */ +template void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const uint32_t& read_index) +{ + // Construct gate corresponding to a single calldata read + // read_index is raw index into calldata array at which we want to read + this->w_l().emplace_back(public_calldata[read_index]); // populate with value of calldata at read index + this->w_r().emplace_back(this->add_variable(read_index)); // populate with read index as witness + calldata_read_counts[read_index]++; // 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; +} + 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..f72c0f87adb 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 @@ -171,6 +171,9 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui } public_calldata.emplace_back(witness_index); } + + void create_calldata_read_gate(const uint32_t& read_index); + void create_poseidon2_external_gate(const poseidon2_external_gate_& in); void create_poseidon2_internal_gate(const poseidon2_internal_gate_& in); From 342c5be2aac2f48059ed4c97c849f59e5ea18b91 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 4 Jan 2024 20:55:18 +0000 Subject: [PATCH 02/11] standalone calldata test --- .../goblin_ultra_circuit_builder.cpp | 16 +---- .../goblin_ultra_circuit_builder.hpp | 20 +++--- .../proof_system/instance_inspector.hpp | 18 +++--- .../ultra_honk/databus_composer.test.cpp | 63 +++++++------------ 4 files changed, 44 insertions(+), 73 deletions(-) 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 9e3fdf70b57..7fd12d20803 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,19 +31,9 @@ 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 - uint32_t read_idx = 1; // index into calldata array at which we want to read + // Add a single arbitrary value to the calldata and create a read gate + add_public_calldata(FF(25)); + uint32_t read_idx = 0; // index into calldata array at which we want to read create_calldata_read_gate(read_idx); // mock gates that use poseidon selectors, with all zeros as input 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 f72c0f87adb..73fb3e87fdb 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,21 +155,17 @@ 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); + // WORKTODO: this would be inefficent to do every time but need a safe place to do this + calldata_read_counts.resize(public_calldata.size()); + return index; } void create_calldata_read_gate(const uint32_t& read_index); 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/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index 4ee34bab917..299ee203cc6 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,37 @@ 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 calldata lookup 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); + // Add some ecc op gates and arithmetic gates + barretenberg::GoblinTestingUtils::construct_goblin_ecc_op_circuit(builder); + barretenberg::GoblinTestingUtils::construct_arithmetic_circuit(builder); + + // Add some values to calldata and then create some calldata lookup gates + std::array calldata_inputs = { 7, 10, 3, 12, 1 }; + for (auto& val : calldata_inputs) { + builder.add_public_calldata(val); } - 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; + std::array read_indices = { 1, 4 }; + for (auto& idx : read_indices) { + builder.create_calldata_read_gate(idx); + } } }; /** - * @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(); @@ -92,8 +72,13 @@ TEST_F(DataBusComposerTests, SingleCircuit) 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 From 949f87e12a8771c76477932b07cdd50ec72626f0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 4 Jan 2024 20:57:58 +0000 Subject: [PATCH 03/11] comment --- .../circuit_builder/goblin_ultra_circuit_builder.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 73fb3e87fdb..cd87fb26d7a 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 @@ -163,7 +163,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui { const uint32_t index = this->add_variable(in); public_calldata.emplace_back(index); - // WORKTODO: this would be inefficent to do every time but need a safe place to do this + // 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; } From dbd0b4933a0a9ea4e4ceb3fba7dc22eb2682967e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 4 Jan 2024 23:09:36 +0000 Subject: [PATCH 04/11] make read return a value --- .../goblin_ultra_circuit_builder.cpp | 9 ++++-- .../goblin_ultra_circuit_builder.hpp | 2 +- .../ultra_honk/databus_composer.test.cpp | 32 ++++++++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) 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 7fd12d20803..ce78cdc4fa6 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 @@ -34,7 +34,7 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ // Add a single arbitrary value to the calldata and create a read gate add_public_calldata(FF(25)); uint32_t read_idx = 0; // index into calldata array at which we want to read - create_calldata_read_gate(read_idx); + read_calldata_at_index(read_idx); // mock gates that use poseidon selectors, with all zeros as input this->w_l().emplace_back(this->zero_idx); @@ -228,11 +228,12 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co * @tparam FF * @param read_index Raw index into calldata array */ -template void GoblinUltraCircuitBuilder_::create_calldata_read_gate(const uint32_t& read_index) +template uint32_t GoblinUltraCircuitBuilder_::read_calldata_at_index(const uint32_t& read_index) { // Construct gate corresponding to a single calldata read // read_index is raw index into calldata array at which we want to read - this->w_l().emplace_back(public_calldata[read_index]); // populate with value of calldata at read index + uint32_t calldata_value_idx = public_calldata[read_index]; + this->w_l().emplace_back(calldata_value_idx); // populate with value of calldata at read index this->w_r().emplace_back(this->add_variable(read_index)); // populate with read index as witness calldata_read_counts[read_index]++; // increment read count at read index q_busread().emplace_back(1); // read selector on @@ -255,6 +256,8 @@ template void GoblinUltraCircuitBuilder_::create_calldata_read this->q_poseidon2_internal().emplace_back(0); ++this->num_gates; + + return calldata_value_idx; } template 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 cd87fb26d7a..e336600430b 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 @@ -168,7 +168,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui return index; } - void create_calldata_read_gate(const uint32_t& read_index); + uint32_t read_calldata_at_index(const uint32_t& read_index); 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/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index 299ee203cc6..f7db533eb41 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -37,17 +37,6 @@ class DataBusComposerTests : public ::testing::Test { // Add some ecc op gates and arithmetic gates barretenberg::GoblinTestingUtils::construct_goblin_ecc_op_circuit(builder); barretenberg::GoblinTestingUtils::construct_arithmetic_circuit(builder); - - // Add some values to calldata and then create some calldata lookup gates - std::array calldata_inputs = { 7, 10, 3, 12, 1 }; - for (auto& val : calldata_inputs) { - builder.add_public_calldata(val); - } - - std::array read_indices = { 1, 4 }; - for (auto& idx : read_indices) { - builder.create_calldata_read_gate(idx); - } } }; @@ -67,8 +56,29 @@ TEST_F(DataBusComposerTests, CallDataRead) auto builder = proof_system::GoblinUltraCircuitBuilder{ op_queue }; + // Add some ecc op gates and arithmetic gates to the circuit generate_test_circuit(builder); + // Add some values to calldata + std::array calldata_inputs = { 7, 10, 3, 12, 1 }; + for (auto& val : calldata_inputs) { + builder.add_public_calldata(val); + } + + // Create some calldata lookup gates + std::array read_indices = { 1, 4 }; + std::vector calldata_value_indices; + for (auto& idx : read_indices) { + calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); + } + + // Sanity check: the values we read should match the calldata values set originally + for (size_t i = 0; i < read_indices.size(); ++i) { + FF expected_value = calldata_inputs[read_indices[i]]; + FF read_value = builder.get_variable(calldata_value_indices[i]); + EXPECT_EQ(expected_value, read_value); + } + auto composer = GoblinUltraComposer(); // Construct and verify Honk proof From 2cee342b08c9966a14f16c3c97e83aa4d424f1d3 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 00:56:37 +0000 Subject: [PATCH 05/11] test --- .../ultra_honk/databus_composer.test.cpp | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) 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 f7db533eb41..578c1b2882c 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -37,6 +37,26 @@ class DataBusComposerTests : public ::testing::Test { // Add some ecc op gates and arithmetic gates barretenberg::GoblinTestingUtils::construct_goblin_ecc_op_circuit(builder); barretenberg::GoblinTestingUtils::construct_arithmetic_circuit(builder); + + // Add some values to calldata + std::array calldata_inputs = { 7, 10, 3, 12, 1 }; + for (auto& val : calldata_inputs) { + builder.add_public_calldata(val); + } + + // Create some calldata lookup gates + std::array read_indices = { 1, 4 }; + std::vector calldata_value_indices; + for (auto& idx : read_indices) { + calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); + } + + // Sanity check: the values we read should match the calldata values set originally + for (size_t i = 0; i < read_indices.size(); ++i) { + FF expected_value = calldata_inputs[read_indices[i]]; + FF read_value = builder.get_variable(calldata_value_indices[i]); + EXPECT_EQ(expected_value, read_value); + } } }; @@ -56,29 +76,9 @@ TEST_F(DataBusComposerTests, CallDataRead) auto builder = proof_system::GoblinUltraCircuitBuilder{ op_queue }; - // Add some ecc op gates and arithmetic gates to the circuit + // Create a circuit including calldata lookups generate_test_circuit(builder); - // Add some values to calldata - std::array calldata_inputs = { 7, 10, 3, 12, 1 }; - for (auto& val : calldata_inputs) { - builder.add_public_calldata(val); - } - - // Create some calldata lookup gates - std::array read_indices = { 1, 4 }; - std::vector calldata_value_indices; - for (auto& idx : read_indices) { - calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); - } - - // Sanity check: the values we read should match the calldata values set originally - for (size_t i = 0; i < read_indices.size(); ++i) { - FF expected_value = calldata_inputs[read_indices[i]]; - FF read_value = builder.get_variable(calldata_value_indices[i]); - EXPECT_EQ(expected_value, read_value); - } - auto composer = GoblinUltraComposer(); // Construct and verify Honk proof From 791c5aa8b7ec01b9fc25bef3ecc6fdb54b8b949f Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 01:10:00 +0000 Subject: [PATCH 06/11] naming fix --- .../ultra_honk/databus_composer.test.cpp | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) 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 578c1b2882c..c9e75bf34a9 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -35,28 +35,8 @@ class DataBusComposerTests : public ::testing::Test { void generate_test_circuit(auto& builder) { // Add some ecc op gates and arithmetic gates - barretenberg::GoblinTestingUtils::construct_goblin_ecc_op_circuit(builder); - barretenberg::GoblinTestingUtils::construct_arithmetic_circuit(builder); - - // Add some values to calldata - std::array calldata_inputs = { 7, 10, 3, 12, 1 }; - for (auto& val : calldata_inputs) { - builder.add_public_calldata(val); - } - - // Create some calldata lookup gates - std::array read_indices = { 1, 4 }; - std::vector calldata_value_indices; - for (auto& idx : read_indices) { - calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); - } - - // Sanity check: the values we read should match the calldata values set originally - for (size_t i = 0; i < read_indices.size(); ++i) { - FF expected_value = calldata_inputs[read_indices[i]]; - FF read_value = builder.get_variable(calldata_value_indices[i]); - EXPECT_EQ(expected_value, read_value); - } + GoblinMockCircuits::construct_goblin_ecc_op_circuit(builder); + GoblinMockCircuits::construct_arithmetic_circuit(builder); } }; @@ -76,9 +56,29 @@ TEST_F(DataBusComposerTests, CallDataRead) auto builder = proof_system::GoblinUltraCircuitBuilder{ op_queue }; - // Create a circuit including calldata lookups + // Create a general test circuit generate_test_circuit(builder); + // Add some values to calldata + std::array calldata_inputs = { 7, 10, 3, 12, 1 }; + for (auto& val : calldata_inputs) { + builder.add_public_calldata(val); + } + + // Create some calldata lookup gates + std::array read_indices = { 1, 4 }; + std::vector calldata_value_indices; + for (auto& idx : read_indices) { + calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); + } + + // Sanity check: the values we read should match the calldata values set originally + for (size_t i = 0; i < read_indices.size(); ++i) { + FF expected_value = calldata_inputs[read_indices[i]]; + FF read_value = builder.get_variable(calldata_value_indices[i]); + EXPECT_EQ(expected_value, read_value); + } + auto composer = GoblinUltraComposer(); // Construct and verify Honk proof From 0c2599c70f4cec58dd9d3454bb6262bde3cbc15d Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 17:24:53 +0000 Subject: [PATCH 07/11] revert back to create gate style function --- .../arithmetization/gate_data.hpp | 5 ++++ .../goblin_ultra_circuit_builder.cpp | 27 +++++++++---------- .../goblin_ultra_circuit_builder.hpp | 2 +- .../ultra_honk/databus_composer.test.cpp | 27 ++++++++++--------- 4 files changed, 33 insertions(+), 28 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 ce78cdc4fa6..f0e02cdd732 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,10 +31,12 @@ 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 - // Add a single arbitrary value to the calldata and create a read gate - add_public_calldata(FF(25)); - uint32_t read_idx = 0; // index into calldata array at which we want to read - read_calldata_at_index(read_idx); + // 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); + auto value_idx = public_calldata[raw_read_idx]; + create_calldata_lookup_gate({ read_idx, value_idx }); // mock gates that use poseidon selectors, with all zeros as input this->w_l().emplace_back(this->zero_idx); @@ -226,17 +228,16 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co * @brief Create a calldata lookup/read gate * * @tparam FF - * @param read_index Raw index into calldata array + * @param databus_lookup_gate_ witness indices corresponding to: calldata index, calldata value */ -template uint32_t GoblinUltraCircuitBuilder_::read_calldata_at_index(const uint32_t& read_index) +template +void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_lookup_gate_& in) { // Construct gate corresponding to a single calldata read - // read_index is raw index into calldata array at which we want to read - uint32_t calldata_value_idx = public_calldata[read_index]; - this->w_l().emplace_back(calldata_value_idx); // populate with value of calldata at read index - this->w_r().emplace_back(this->add_variable(read_index)); // populate with read index as witness - calldata_read_counts[read_index]++; // increment read count at read index - q_busread().emplace_back(1); // read selector on + this->w_l().emplace_back(in.value); // populate with value of calldata at read index + this->w_r().emplace_back(in.index); // populate with read index as witness + calldata_read_counts[static_cast(this->get_variable(in.index))]++; // 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); @@ -256,8 +257,6 @@ template uint32_t GoblinUltraCircuitBuilder_::read_calldata_at this->q_poseidon2_internal().emplace_back(0); ++this->num_gates; - - return calldata_value_idx; } template 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 e336600430b..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 @@ -168,7 +168,7 @@ template class GoblinUltraCircuitBuilder_ : public UltraCircuitBui return index; } - uint32_t read_calldata_at_index(const uint32_t& read_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/ultra_honk/databus_composer.test.cpp b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp index c9e75bf34a9..96b16081822 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -59,24 +59,25 @@ TEST_F(DataBusComposerTests, CallDataRead) // Create a general test circuit generate_test_circuit(builder); - // Add some values to calldata - std::array calldata_inputs = { 7, 10, 3, 12, 1 }; - for (auto& val : calldata_inputs) { - builder.add_public_calldata(val); + // Add some values to calldata and store the corresponding witness index + std::array calldata_values = { 7, 10, 3, 12, 1 }; + std::vector calldata_value_indices; + for (auto& val : calldata_values) { + calldata_value_indices.emplace_back(builder.add_public_calldata(val)); } - // Create some calldata lookup gates - std::array read_indices = { 1, 4 }; - std::vector calldata_value_indices; - for (auto& idx : read_indices) { - calldata_value_indices.emplace_back(builder.read_calldata_at_index(idx)); + // Add some read index values and store the corresponding witness index + std::array read_index_values = { 1, 4 }; + std::vector read_indices; + for (auto& val : read_index_values) { + read_indices.emplace_back(builder.add_variable(val)); } - // Sanity check: the values we read should match the calldata values set originally + // Create some calldata lookup gates for (size_t i = 0; i < read_indices.size(); ++i) { - FF expected_value = calldata_inputs[read_indices[i]]; - FF read_value = builder.get_variable(calldata_value_indices[i]); - EXPECT_EQ(expected_value, read_value); + uint32_t read_idx = read_indices[i]; + uint32_t value_idx = calldata_value_indices[read_index_values[i]]; + builder.create_calldata_lookup_gate({ read_idx, value_idx }); } auto composer = GoblinUltraComposer(); From 14af71ea088f919f63793a9d7cd2eab112fd2a24 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 17:27:45 +0000 Subject: [PATCH 08/11] fix comment --- .../circuit_builder/goblin_ultra_circuit_builder.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 f0e02cdd732..a08a18adff6 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 @@ -233,11 +233,10 @@ template void GoblinUltraCircuitBuilder_::set_goblin_ecc_op_co template void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_lookup_gate_& in) { - // Construct gate corresponding to a single calldata read - this->w_l().emplace_back(in.value); // populate with value of calldata at read index - this->w_r().emplace_back(in.index); // populate with read index as witness - calldata_read_counts[static_cast(this->get_variable(in.index))]++; // increment read count at read index - q_busread().emplace_back(1); // read selector on + this->w_l().emplace_back(in.value); + this->w_r().emplace_back(in.index); + calldata_read_counts[static_cast(this->get_variable(in.index))]++; // increment read count at raw read index + q_busread().emplace_back(1); // populate all other components with zero this->w_o().emplace_back(this->zero_idx); From 81166090762402a6a7d30a738df850fa10150194 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 17:29:46 +0000 Subject: [PATCH 09/11] another comment fix --- .../cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 96b16081822..933c7b42464 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -28,7 +28,7 @@ class DataBusComposerTests : public ::testing::Test { using CommitmentKey = pcs::CommitmentKey; /** - * @brief Generate a simple test circuit that includes calldata lookup gates + * @brief Generate a simple test circuit that includes arithmetic and goblin ecc op gates * * @param builder */ From 2b5dae9fab340ce11f780eb11ece958c9e1b570c Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 5 Jan 2024 18:26:15 +0000 Subject: [PATCH 10/11] possible read counts workaround --- .../circuit_builder/goblin_ultra_circuit_builder.cpp | 4 +++- .../cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) 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 a08a18adff6..97fc4bd8a5a 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 @@ -37,6 +37,7 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ auto read_idx = this->add_variable(raw_read_idx); auto value_idx = public_calldata[raw_read_idx]; create_calldata_lookup_gate({ read_idx, value_idx }); + 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); @@ -235,7 +236,8 @@ void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_l { this->w_l().emplace_back(in.value); this->w_r().emplace_back(in.index); - calldata_read_counts[static_cast(this->get_variable(in.index))]++; // increment read count at raw read index + // WORKTODO: is there a way to do this here? Should we? + // calldata_read_counts[static_cast(this->get_variable(in.index))]++; q_busread().emplace_back(1); // populate all other components with zero 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 933c7b42464..d00f4b6677e 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -78,6 +78,7 @@ TEST_F(DataBusComposerTests, CallDataRead) uint32_t read_idx = read_indices[i]; uint32_t value_idx = calldata_value_indices[read_index_values[i]]; builder.create_calldata_lookup_gate({ read_idx, value_idx }); + builder.calldata_read_counts[read_index_values[i]]++; } auto composer = GoblinUltraComposer(); From a95f5736b3b01be593e297b479a22e7c15ebe5f0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Mon, 8 Jan 2024 22:47:55 +0000 Subject: [PATCH 11/11] new variable for lookup result plus TODO comments --- .../goblin_ultra_circuit_builder.cpp | 6 ++--- .../sumcheck/instance/prover_instance.cpp | 1 + .../ultra_honk/databus_composer.test.cpp | 27 ++++++++++--------- 3 files changed, 19 insertions(+), 15 deletions(-) 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 97fc4bd8a5a..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 @@ -35,8 +35,10 @@ template void GoblinUltraCircuitBuilder_::add_gates_to_ensure_ 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); - auto value_idx = public_calldata[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 @@ -236,8 +238,6 @@ void GoblinUltraCircuitBuilder_::create_calldata_lookup_gate(const databus_l { this->w_l().emplace_back(in.value); this->w_r().emplace_back(in.index); - // WORKTODO: is there a way to do this here? Should we? - // calldata_read_counts[static_cast(this->get_variable(in.index))]++; q_busread().emplace_back(1); // populate all other components with zero 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 d00f4b6677e..e9adb253f0c 100644 --- a/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp +++ b/barretenberg/cpp/src/barretenberg/ultra_honk/databus_composer.test.cpp @@ -61,24 +61,27 @@ TEST_F(DataBusComposerTests, CallDataRead) // Add some values to calldata and store the corresponding witness index std::array calldata_values = { 7, 10, 3, 12, 1 }; - std::vector calldata_value_indices; for (auto& val : calldata_values) { - calldata_value_indices.emplace_back(builder.add_public_calldata(val)); + builder.add_public_calldata(val); } - // Add some read index values and store the corresponding witness index + // Define some indices at which to read calldata std::array read_index_values = { 1, 4 }; - std::vector read_indices; - for (auto& val : read_index_values) { - read_indices.emplace_back(builder.add_variable(val)); - } // Create some calldata lookup gates - for (size_t i = 0; i < read_indices.size(); ++i) { - uint32_t read_idx = read_indices[i]; - uint32_t value_idx = calldata_value_indices[read_index_values[i]]; - builder.create_calldata_lookup_gate({ read_idx, value_idx }); - builder.calldata_read_counts[read_index_values[i]]++; + 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();