From 598eb8bb9bc846df0bf216aad1d4e81678d40f22 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 May 2024 16:13:40 +0000 Subject: [PATCH 01/15] reads the return data --- .../dsl/acir_format/block_constraint.cpp | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index af567e63b968..7dd139b09f21 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -103,19 +103,12 @@ void create_block_constraints(Builder& builder, const BlockConstraint constraint databus_ct databus; // Populate the returndata in the databus databus.return_data.set_values(init); - for (const auto& op : constraint.trace) { - ASSERT(op.access_type == 0); - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); - } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(databus.return_data[w]); - w.assert_equal(index); + int c = 0; + for (const auto& value : init) { + value.assert_equal(databus.return_data[c]); + c++; } + ASSERT(constraint.trace.size() == 0); } } break; default: From b23522589554fa676fd255f73819d76002a01db2 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 24 May 2024 20:00:29 +0000 Subject: [PATCH 02/15] add a simple test for the databus program --- .../dsl/acir_format/acir_integration.test.cpp | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index 81c02043a28a..d838f7a5f8f9 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -66,6 +66,9 @@ class AcirIntegrationTest : public ::testing::Test { return verifier.verify_proof(proof); } + + protected: + static void SetUpTestSuite() { srs::init_crs_factory("../srs_db/ignition"); } }; class AcirIntegrationSingleTest : public AcirIntegrationTest, public testing::WithParamInterface { @@ -332,4 +335,27 @@ INSTANTIATE_TEST_SUITE_P(AcirTests, "fold_distinct_return", "fold_fibonacci", "fold_numeric_generic_poseidon")); -#endif \ No newline at end of file +#endif + +/** + * @brief A basic test of a circuit generated in noir that makes use of the databus + * + */ +TEST_F(AcirIntegrationTest, Databus) +{ + using Flavor = GoblinUltraFlavor; + using Builder = Flavor::CircuitBuilder; + + std::string test_name = "databus"; + info("Test: ", test_name); + acir_format::AcirProgram acir_program = get_program_data_from_test_file(test_name); + + // Construct a bberg circuit from the acir representation + Builder builder = acir_format::create_circuit(acir_program.constraints, 0, acir_program.witness); + + // This prints a summary of the types of gates in the circuit + builder.blocks.summarize(); + + // Construct and verify Honk proof + EXPECT_TRUE(prove_and_verify_honk(builder)); +} \ No newline at end of file From b5231751d04ef2604cf2f37151bddd162a98e533 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 27 May 2024 08:06:17 +0000 Subject: [PATCH 03/15] do not optimise return-data --- .../src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp index 6756d17f1323..76ba21fda9d8 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp @@ -500,7 +500,7 @@ AcirFormat circuit_serde_to_acir_format(Program::Circuit const& circuit) gate.value); } for (const auto& [block_id, block] : block_id_to_block_constraint) { - if (!block.trace.empty()) { + if (!block.trace.empty() || block.type == BlockType::ReturnData) { af.block_constraints.push_back(block); } } From a427fa8e60cedc353fce5bb13eb24179c5591118 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 May 2024 14:29:24 +0000 Subject: [PATCH 04/15] better flavor --- .../src/barretenberg/dsl/acir_format/acir_integration.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index c9aeb3c64964..b5c74406a6e6 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -371,7 +371,7 @@ INSTANTIATE_TEST_SUITE_P(AcirTests, */ TEST_F(AcirIntegrationTest, Databus) { - using Flavor = GoblinUltraFlavor; + using Flavor = MegaFlavor; using Builder = Flavor::CircuitBuilder; std::string test_name = "databus"; From 169909d5e9c7a40f5a8f6299f997b948a527e91d Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 29 May 2024 17:14:37 +0000 Subject: [PATCH 05/15] fix merge issue --- .../src/barretenberg/dsl/acir_format/acir_integration.test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index 1415e290e69e..f9ab03dfe4e2 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -416,7 +416,6 @@ TEST_P(AcirIntegrationFoldingTest, DISABLED_FoldAndVerifyProgramStack) INSTANTIATE_TEST_SUITE_P(AcirTests, AcirIntegrationFoldingTest, testing::Values("fold_basic", "fold_basic_nested_call")); -#endif /** *@brief A basic test of a circuit generated in noir that makes use of the databus From 3cd6c410f93ae689bbcbc605fa9f5c704b00ccad Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 30 May 2024 08:52:12 +0000 Subject: [PATCH 06/15] update path to acir artifacts --- .../src/barretenberg/dsl/acir_format/acir_integration.test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index f9ab03dfe4e2..961ca991f8a0 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -34,7 +34,8 @@ class AcirIntegrationTest : public ::testing::Test { acir_format::AcirProgramStack get_program_stack_data_from_test_file(const std::string& test_program_name) { - std::string base_path = "../../acir_tests/acir_tests/" + test_program_name + "/target"; + std::string base_path = + "../../../noir/noir-repo/test_programs/execution_success/" + test_program_name + "/target"; std::string bytecode_path = base_path + "/program.json"; std::string witness_path = base_path + "/witness.gz"; From b8f22c50e62cf6ac3073e6437a8095043111657b Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 30 May 2024 13:56:26 +0000 Subject: [PATCH 07/15] disable acir databus test from running in the CI (artifact is missing) --- .../src/barretenberg/dsl/acir_format/acir_integration.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index 961ca991f8a0..f68417b8ec22 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -422,7 +422,7 @@ INSTANTIATE_TEST_SUITE_P(AcirTests, *@brief A basic test of a circuit generated in noir that makes use of the databus * */ -TEST_F(AcirIntegrationTest, Databus) +TEST_F(AcirIntegrationTest, DISABLED_Databus) { using Flavor = MegaFlavor; using Builder = Flavor::CircuitBuilder; From e79886afcc5a50a282e39d9645040821166adc3e Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 31 May 2024 14:14:06 +0000 Subject: [PATCH 08/15] use mega builder for the databus, when flavor is mega-honk --- .../acir_format/acir_to_constraint_buf.cpp | 10 +- .../dsl/acir_format/block_constraint.cpp | 113 ++++++++++++++---- .../dsl/acir_format/block_constraint.hpp | 9 +- 3 files changed, 99 insertions(+), 33 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp index 1603d87ea037..aef7bf5eb9e2 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp @@ -441,12 +441,10 @@ BlockConstraint handle_memory_init(Program::Opcode::MemoryInit const& mem_init) } // Databus is only supported for Goblin, non Goblin builders will treat call_data and return_data as normal array. - if (IsMegaBuilder) { - if (std::holds_alternative(mem_init.block_type.value)) { - block.type = BlockType::CallData; - } else if (std::holds_alternative(mem_init.block_type.value)) { - block.type = BlockType::ReturnData; - } + if (std::holds_alternative(mem_init.block_type.value)) { + block.type = BlockType::CallData; + } else if (std::holds_alternative(mem_init.block_type.value)) { + block.type = BlockType::ReturnData; } return block; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 8b6122a31ee5..904cef46bc4d 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -24,6 +24,71 @@ template bb::stdlib::field_t poly_to_field_ct(const template void create_block_constraints(Builder& builder, const BlockConstraint constraint, bool has_valid_witness_assignments) + requires IsNotMegaBuilder +{ + using field_ct = bb::stdlib::field_t; + using rom_table_ct = bb::stdlib::rom_table; + using ram_table_ct = bb::stdlib::ram_table; + + std::vector init; + for (auto i : constraint.init) { + field_ct value = poly_to_field_ct(i, builder); + init.push_back(value); + } + + switch (constraint.type) { + case BlockType::CallData: + case BlockType::ReturnData: + case BlockType::ROM: { + rom_table_ct table(init); + for (auto& op : constraint.trace) { + ASSERT(op.access_type == 0); + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + // For a ROM table, constant read should be optimized out: + // The rom_table won't work with a constant read because the table may not be initialized + ASSERT(op.index.q_l != 0); + // We create a new witness w to avoid issues with non-valid witness assignements: + // if witness are not assigned, then w will be zero and table[w] will work + fr w_value = 0; + if (has_valid_witness_assignments) { + // If witness are assigned, we use the correct value for w + w_value = index.get_value(); + } + field_ct w = field_ct::from_witness(&builder, w_value); + value.assert_equal(table[w]); + w.assert_equal(index); + } + } break; + case BlockType::RAM: { + ram_table_ct table(init); + for (auto& op : constraint.trace) { + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + + // We create a new witness w to avoid issues with non-valid witness assignements. + // If witness are not assigned, then index will be zero and table[index] won't hit bounds check. + fr index_value = has_valid_witness_assignments ? index.get_value() : 0; + // Create new witness and ensure equal to index. + field_ct::from_witness(&builder, index_value).assert_equal(index); + + if (op.access_type == 0) { + value.assert_equal(table.read(index)); + } else { + ASSERT(op.access_type == 1); + table.write(index, value); + } + } + } break; + default: + ASSERT(false); + break; + } +} + +template +void create_block_constraints(Builder& builder, const BlockConstraint constraint, bool has_valid_witness_assignments) + requires IsMegaBuilder { using field_ct = bb::stdlib::field_t; using rom_table_ct = bb::stdlib::rom_table; @@ -79,37 +144,33 @@ void create_block_constraints(Builder& builder, const BlockConstraint constraint } } break; case BlockType::CallData: { - if constexpr (IsMegaBuilder) { - databus_ct databus; - // Populate the calldata in the databus - databus.calldata.set_values(init); - for (const auto& op : constraint.trace) { - ASSERT(op.access_type == 0); - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); - } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(databus.calldata[w]); - w.assert_equal(index); + databus_ct databus; + // Populate the calldata in the databus + databus.calldata.set_values(init); + for (const auto& op : constraint.trace) { + ASSERT(op.access_type == 0); + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + fr w_value = 0; + if (has_valid_witness_assignments) { + // If witness are assigned, we use the correct value for w + w_value = index.get_value(); } + field_ct w = field_ct::from_witness(&builder, w_value); + value.assert_equal(databus.calldata[w]); + w.assert_equal(index); } } break; case BlockType::ReturnData: { - if constexpr (IsMegaBuilder) { - databus_ct databus; - // Populate the returndata in the databus - databus.return_data.set_values(init); - int c = 0; - for (const auto& value : init) { - value.assert_equal(databus.return_data[c]); - c++; - } - ASSERT(constraint.trace.size() == 0); + databus_ct databus; + // Populate the returndata in the databus + databus.return_data.set_values(init); + int c = 0; + for (const auto& value : init) { + value.assert_equal(databus.return_data[c]); + c++; } + ASSERT(constraint.trace.size() == 0); } break; default: ASSERT(false); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp index 0a5f2c2b40ac..120f0f24f93b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp @@ -28,7 +28,14 @@ struct BlockConstraint { template void create_block_constraints(Builder& builder, const BlockConstraint constraint, - bool has_valid_witness_assignments = true); + bool has_valid_witness_assignments = true) + requires IsMegaBuilder; + +template +void create_block_constraints(Builder& builder, + const BlockConstraint constraint, + bool has_valid_witness_assignments = true) + requires IsNotMegaBuilder; template inline void read(B& buf, MemOp& mem_op) { From 51042c742fd40998ae3a6d5b3327d8939568c3d9 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 31 May 2024 19:16:22 +0000 Subject: [PATCH 09/15] fix build issue --- .../dsl/acir_format/acir_integration.test.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index 23e09583af1f..202f92aceaf9 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -33,17 +33,19 @@ class AcirIntegrationTest : public ::testing::Test { } acir_format::AcirProgramStack get_program_stack_data_from_test_file(const std::string& test_program_name, - bool honk_recursion) + bool honk_recursion = false) { - std::string base_path = - "../../../noir/noir-repo/test_programs/execution_success/" + test_program_name + "/target"; + // std::string base_path = + // "../../../noir/noir-repo/test_programs/execution_success/" + test_program_name + "/target"; + std::string base_path = "../../acir_tests/acir_tests/" + test_program_name + "/target"; std::string bytecode_path = base_path + "/program.json"; std::string witness_path = base_path + "/witness.gz"; return acir_format::get_acir_program_stack(bytecode_path, witness_path, honk_recursion); } - acir_format::AcirProgram get_program_data_from_test_file(const std::string& test_program_name, bool honk_recursion) + acir_format::AcirProgram get_program_data_from_test_file(const std::string& test_program_name, + bool honk_recursion = false) { auto program_stack = get_program_stack_data_from_test_file(test_program_name, honk_recursion); ASSERT(program_stack.size() == 1); // Otherwise this method will not return full stack data From 10b31f5d6cc31b9fc6e4aa827b1bbd9851194639 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 5 Jun 2024 15:36:59 +0000 Subject: [PATCH 10/15] merge issues --- .../cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp | 4 ++-- .../cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 400b3ed50f69..4e35d57b512f 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -25,7 +25,7 @@ template stdlib::field_t poly_to_field_ct(const poly } template -void create_block_constraints(Builder& builder, const BlockConstraint constraint, bool has_valid_witness_assignments) +void create_block_constraints(Builder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments) requires IsNotMegaBuilder { using field_ct = bb::stdlib::field_t; @@ -89,7 +89,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint constraint } template -void create_block_constraints(Builder& builder, const BlockConstraint constraint, bool has_valid_witness_assignments) +void create_block_constraints(Builder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments) requires IsMegaBuilder { using field_ct = stdlib::field_t; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp index 5fee6dc6161b..5b5ffaee262b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp @@ -26,13 +26,13 @@ struct BlockConstraint { template void create_block_constraints(Builder& builder, - const BlockConstraint constraint, + const BlockConstraint& constraint, bool has_valid_witness_assignments = true) requires IsMegaBuilder; template void create_block_constraints(Builder& builder, - const BlockConstraint constraint, + const BlockConstraint& constraint, bool has_valid_witness_assignments = true) requires IsNotMegaBuilder; From 37fcdf77c7d51655d0bf58afec445477d3f198b3 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 5 Jun 2024 16:44:47 +0000 Subject: [PATCH 11/15] test using process ROM method --- .../dsl/acir_format/block_constraint.cpp | 74 +++++++++---------- .../dsl/acir_format/block_constraint.hpp | 9 +++ 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 4e35d57b512f..b819983b28c8 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -29,7 +29,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain requires IsNotMegaBuilder { using field_ct = bb::stdlib::field_t; - using rom_table_ct = bb::stdlib::rom_table; + // using rom_table_ct = bb::stdlib::rom_table; using ram_table_ct = bb::stdlib::ram_table; std::vector init; @@ -42,25 +42,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain case BlockType::CallData: case BlockType::ReturnData: case BlockType::ROM: { - rom_table_ct table(init); - for (auto& op : constraint.trace) { - ASSERT(op.access_type == 0); - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - // For a ROM table, constant read should be optimized out: - // The rom_table won't work with a constant read because the table may not be initialized - ASSERT(op.index.q_l != 0); - // We create a new witness w to avoid issues with non-valid witness assignements: - // if witness are not assigned, then w will be zero and table[w] will work - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); - } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(table[w]); - w.assert_equal(index); - } + process_ROM_operations(builder, constraint, init, has_valid_witness_assignments); } break; case BlockType::RAM: { ram_table_ct table(init); @@ -93,7 +75,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain requires IsMegaBuilder { using field_ct = stdlib::field_t; - using rom_table_ct = stdlib::rom_table; + // using rom_table_ct = stdlib::rom_table; using ram_table_ct = stdlib::ram_table; using databus_ct = stdlib::databus; @@ -105,25 +87,7 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain switch (constraint.type) { case BlockType::ROM: { - rom_table_ct table(init); - for (auto& op : constraint.trace) { - ASSERT(op.access_type == 0); - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - // For a ROM table, constant read should be optimized out: - // The rom_table won't work with a constant read because the table may not be initialized - ASSERT(op.index.q_l != 0); - // We create a new witness w to avoid issues with non-valid witness assignements: - // if witness are not assigned, then w will be zero and table[w] will work - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); - } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(table[w]); - w.assert_equal(index); - } + process_ROM_operations(builder, constraint, init, has_valid_witness_assignments); } break; case BlockType::RAM: { ram_table_ct table(init); @@ -180,6 +144,36 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain } } +template +void process_ROM_operations(Builder& builder, + const BlockConstraint& constraint, + std::vector>& init, + bool has_valid_witness_assignments) +{ + using field_ct = stdlib::field_t; + using rom_table_ct = stdlib::rom_table; + + rom_table_ct table(init); + for (auto& op : constraint.trace) { + ASSERT(op.access_type == 0); + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + // For a ROM table, constant read should be optimized out: + // The rom_table won't work with a constant read because the table may not be initialized + ASSERT(op.index.q_l != 0); + // We create a new witness w to avoid issues with non-valid witness assignements: + // if witness are not assigned, then w will be zero and table[w] will work + fr w_value = 0; + if (has_valid_witness_assignments) { + // If witness are assigned, we use the correct value for w + w_value = index.get_value(); + } + field_ct w = field_ct::from_witness(&builder, w_value); + value.assert_equal(table[w]); + w.assert_equal(index); + } +} + template void create_block_constraints(UltraCircuitBuilder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp index 5b5ffaee262b..525b7adccbf4 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp @@ -36,6 +36,15 @@ void create_block_constraints(Builder& builder, bool has_valid_witness_assignments = true) requires IsNotMegaBuilder; +template +void process_ROM_operations(Builder& builder, + const BlockConstraint& constraint, + std::vector>& init, + bool has_valid_witness_assignments); +template void process_RAM_operations(std::vector>& init); +template void process_call_data_operations(std::vector>& init); +template void process_return_data_operations(std::vector>& init); + template inline void read(B& buf, MemOp& mem_op) { using serialize::read; From 5604c18a88b9de79175e0933c883b6490a07cd62 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 5 Jun 2024 17:34:59 +0000 Subject: [PATCH 12/15] use constraint specific methods --- .../dsl/acir_format/block_constraint.cpp | 151 ++++++++++-------- .../dsl/acir_format/block_constraint.hpp | 19 ++- 2 files changed, 95 insertions(+), 75 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index b819983b28c8..7bbf6350d6d6 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -29,8 +29,6 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain requires IsNotMegaBuilder { using field_ct = bb::stdlib::field_t; - // using rom_table_ct = bb::stdlib::rom_table; - using ram_table_ct = bb::stdlib::ram_table; std::vector init; for (auto i : constraint.init) { @@ -39,30 +37,14 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain } switch (constraint.type) { + // Note: CallData/ReturnData not supported by Ultra; interpreted as ROM ops instead case BlockType::CallData: case BlockType::ReturnData: case BlockType::ROM: { - process_ROM_operations(builder, constraint, init, has_valid_witness_assignments); + process_ROM_operations(builder, constraint, has_valid_witness_assignments, init); } break; case BlockType::RAM: { - ram_table_ct table(init); - for (auto& op : constraint.trace) { - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - - // We create a new witness w to avoid issues with non-valid witness assignements. - // If witness are not assigned, then index will be zero and table[index] won't hit bounds check. - fr index_value = has_valid_witness_assignments ? index.get_value() : 0; - // Create new witness and ensure equal to index. - field_ct::from_witness(&builder, index_value).assert_equal(index); - - if (op.access_type == 0) { - value.assert_equal(table.read(index)); - } else { - ASSERT(op.access_type == 1); - table.write(index, value); - } - } + process_RAM_operations(builder, constraint, has_valid_witness_assignments, init); } break; default: ASSERT(false); @@ -75,9 +57,6 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain requires IsMegaBuilder { using field_ct = stdlib::field_t; - // using rom_table_ct = stdlib::rom_table; - using ram_table_ct = stdlib::ram_table; - using databus_ct = stdlib::databus; std::vector init; for (auto i : constraint.init) { @@ -87,56 +66,16 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain switch (constraint.type) { case BlockType::ROM: { - process_ROM_operations(builder, constraint, init, has_valid_witness_assignments); + process_ROM_operations(builder, constraint, has_valid_witness_assignments, init); } break; case BlockType::RAM: { - ram_table_ct table(init); - for (auto& op : constraint.trace) { - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - - // We create a new witness w to avoid issues with non-valid witness assignements. - // If witness are not assigned, then index will be zero and table[index] won't hit bounds check. - fr index_value = has_valid_witness_assignments ? index.get_value() : 0; - // Create new witness and ensure equal to index. - field_ct::from_witness(&builder, index_value).assert_equal(index); - - if (op.access_type == 0) { - value.assert_equal(table.read(index)); - } else { - ASSERT(op.access_type == 1); - table.write(index, value); - } - } + process_RAM_operations(builder, constraint, has_valid_witness_assignments, init); } break; case BlockType::CallData: { - databus_ct databus; - // Populate the calldata in the databus - databus.calldata.set_values(init); - for (const auto& op : constraint.trace) { - ASSERT(op.access_type == 0); - field_ct value = poly_to_field_ct(op.value, builder); - field_ct index = poly_to_field_ct(op.index, builder); - fr w_value = 0; - if (has_valid_witness_assignments) { - // If witness are assigned, we use the correct value for w - w_value = index.get_value(); - } - field_ct w = field_ct::from_witness(&builder, w_value); - value.assert_equal(databus.calldata[w]); - w.assert_equal(index); - } + process_call_data_operations(builder, constraint, has_valid_witness_assignments, init); } break; case BlockType::ReturnData: { - databus_ct databus; - // Populate the returndata in the databus - databus.return_data.set_values(init); - int c = 0; - for (const auto& value : init) { - value.assert_equal(databus.return_data[c]); - c++; - } - ASSERT(constraint.trace.size() == 0); + process_return_data_operations(constraint, init); } break; default: ASSERT(false); @@ -147,8 +86,8 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain template void process_ROM_operations(Builder& builder, const BlockConstraint& constraint, - std::vector>& init, - bool has_valid_witness_assignments) + bool has_valid_witness_assignments, + std::vector>& init) { using field_ct = stdlib::field_t; using rom_table_ct = stdlib::rom_table; @@ -174,6 +113,78 @@ void process_ROM_operations(Builder& builder, } } +template +void process_RAM_operations(Builder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments, + std::vector>& init) +{ + using field_ct = stdlib::field_t; + using ram_table_ct = stdlib::ram_table; + + ram_table_ct table(init); + for (auto& op : constraint.trace) { + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + + // We create a new witness w to avoid issues with non-valid witness assignements. + // If witness are not assigned, then index will be zero and table[index] won't hit bounds check. + fr index_value = has_valid_witness_assignments ? index.get_value() : 0; + // Create new witness and ensure equal to index. + field_ct::from_witness(&builder, index_value).assert_equal(index); + + if (op.access_type == 0) { + value.assert_equal(table.read(index)); + } else { + ASSERT(op.access_type == 1); + table.write(index, value); + } + } +} + +template +void process_call_data_operations(Builder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments, + std::vector>& init) +{ + using field_ct = stdlib::field_t; + using databus_ct = stdlib::databus; + + databus_ct databus; + // Populate the calldata in the databus + databus.calldata.set_values(init); + for (const auto& op : constraint.trace) { + ASSERT(op.access_type == 0); + field_ct value = poly_to_field_ct(op.value, builder); + field_ct index = poly_to_field_ct(op.index, builder); + fr w_value = 0; + if (has_valid_witness_assignments) { + // If witness are assigned, we use the correct value for w + w_value = index.get_value(); + } + field_ct w = field_ct::from_witness(&builder, w_value); + value.assert_equal(databus.calldata[w]); + w.assert_equal(index); + } +} + +template +void process_return_data_operations(const BlockConstraint& constraint, std::vector>& init) +{ + using databus_ct = stdlib::databus; + + databus_ct databus; + // Populate the returndata in the databus + databus.return_data.set_values(init); + int c = 0; + for (const auto& value : init) { + value.assert_equal(databus.return_data[c]); + c++; + } + ASSERT(constraint.trace.size() == 0); +} + template void create_block_constraints(UltraCircuitBuilder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp index 525b7adccbf4..1f243161e948 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp @@ -39,11 +39,20 @@ void create_block_constraints(Builder& builder, template void process_ROM_operations(Builder& builder, const BlockConstraint& constraint, - std::vector>& init, - bool has_valid_witness_assignments); -template void process_RAM_operations(std::vector>& init); -template void process_call_data_operations(std::vector>& init); -template void process_return_data_operations(std::vector>& init); + bool has_valid_witness_assignments, + std::vector>& init); +template +void process_RAM_operations(Builder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments, + std::vector>& init); +template +void process_call_data_operations(Builder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments, + std::vector>& init); +template +void process_return_data_operations(const BlockConstraint& constraint, std::vector>& init); template inline void read(B& buf, MemOp& mem_op) { From ff1364aa4484e8c02f068672413dc1243152e4bc Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 5 Jun 2024 17:41:32 +0000 Subject: [PATCH 13/15] use template specialization instead of concepts --- .../dsl/acir_format/block_constraint.cpp | 34 +++++++++++-------- .../dsl/acir_format/block_constraint.hpp | 9 +---- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index 7bbf6350d6d6..a27174cfe217 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -24,11 +24,17 @@ template stdlib::field_t poly_to_field_ct(const poly return x; } -template -void create_block_constraints(Builder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments) - requires IsNotMegaBuilder +/** + * @brief Create block constraints; Specialization for Ultra arithmetization + * @details Ultra does not support DataBus operations so calldata/returndata are treated as ROM ops + * + */ +template <> +void create_block_constraints(UltraCircuitBuilder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments) { - using field_ct = bb::stdlib::field_t; + using field_ct = bb::stdlib::field_t; std::vector init; for (auto i : constraint.init) { @@ -52,11 +58,16 @@ void create_block_constraints(Builder& builder, const BlockConstraint& constrain } } -template -void create_block_constraints(Builder& builder, const BlockConstraint& constraint, bool has_valid_witness_assignments) - requires IsMegaBuilder +/** + * @brief Create block constraints; Specialization for Mega arithmetization + * + */ +template <> +void create_block_constraints(MegaCircuitBuilder& builder, + const BlockConstraint& constraint, + bool has_valid_witness_assignments) { - using field_ct = stdlib::field_t; + using field_ct = stdlib::field_t; std::vector init; for (auto i : constraint.init) { @@ -185,11 +196,4 @@ void process_return_data_operations(const BlockConstraint& constraint, std::vect ASSERT(constraint.trace.size() == 0); } -template void create_block_constraints(UltraCircuitBuilder& builder, - const BlockConstraint& constraint, - bool has_valid_witness_assignments); -template void create_block_constraints(MegaCircuitBuilder& builder, - const BlockConstraint& constraint, - bool has_valid_witness_assignments); - } // namespace acir_format \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp index 1f243161e948..8a0da27058ba 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.hpp @@ -27,14 +27,7 @@ struct BlockConstraint { template void create_block_constraints(Builder& builder, const BlockConstraint& constraint, - bool has_valid_witness_assignments = true) - requires IsMegaBuilder; - -template -void create_block_constraints(Builder& builder, - const BlockConstraint& constraint, - bool has_valid_witness_assignments = true) - requires IsNotMegaBuilder; + bool has_valid_witness_assignments = true); template void process_ROM_operations(Builder& builder, From e8dad408a1482e1a4bbddf0728fdac61415d8886 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 5 Jun 2024 17:47:26 +0000 Subject: [PATCH 14/15] update comment on return data --- .../src/barretenberg/dsl/acir_format/block_constraint.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp index a27174cfe217..1ba27ff530dc 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.cpp @@ -188,7 +188,10 @@ void process_return_data_operations(const BlockConstraint& constraint, std::vect databus_ct databus; // Populate the returndata in the databus databus.return_data.set_values(init); - int c = 0; + // For each entry of the return data, explicitly assert equality with the initialization value. This implicitly + // creates the return data read gates that are required to connect witness values in the main wires to witness + // values in the databus return data column. + size_t c = 0; for (const auto& value : init) { value.assert_equal(databus.return_data[c]); c++; From 9efd5a52f020a31e76a84c38ab73ab1a41d60ac3 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Wed, 5 Jun 2024 17:50:53 +0000 Subject: [PATCH 15/15] comments --- .../src/barretenberg/dsl/acir_format/acir_integration.test.cpp | 2 -- .../src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp index 464f29841069..2842056f8482 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_integration.test.cpp @@ -38,8 +38,6 @@ class AcirIntegrationTest : public ::testing::Test { acir_format::AcirProgramStack get_program_stack_data_from_test_file(const std::string& test_program_name, bool honk_recursion = false) { - // std::string base_path = - // "../../../noir/noir-repo/test_programs/execution_success/" + test_program_name + "/target"; std::string base_path = "../../acir_tests/acir_tests/" + test_program_name + "/target"; std::string bytecode_path = base_path + "/program.json"; std::string witness_path = base_path + "/witness.gz"; diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp index 40634fa617e6..f045846f8229 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp @@ -512,6 +512,7 @@ AcirFormat circuit_serde_to_acir_format(Program::Circuit const& circuit, bool ho gate.value); } for (const auto& [block_id, block] : block_id_to_block_constraint) { + // Note: the trace will always be empty for ReturnData since it cannot be explicitly read from in noir if (!block.trace.empty() || block.type == BlockType::ReturnData) { af.block_constraints.push_back(block); }