From 66bfa47ad76514fcf94b61fc0e9f5e75f89a278c Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:34:31 +0000 Subject: [PATCH 1/9] start refactoring multitableid --- .vscode/settings.json | 3 +- .../circuit_builder/ultra_circuit_builder.cpp | 4 +- .../circuit_builder/ultra_circuit_builder.hpp | 2 +- .../plookup_tables/plookup_tables.cpp | 10 ++- .../plookup_tables/plookup_tables.hpp | 1 + .../proof_system/plookup_tables/types.hpp | 86 +++++-------------- .../primitives/plookup/plookup.test.cpp | 41 +++++++++ 7 files changed, 76 insertions(+), 71 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index b7aa953af51..a3fa432542e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -155,5 +155,6 @@ "C_Cpp.vcpkg.enabled": false, "C_Cpp.default.includePath": [ "barretenberg/cpp/src" - ] + ], + "cmake.sourceDirectory": "/mnt/user-data/adam/aztec-packages/barretenberg/cpp" } diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index 21aa754edfe..e65527a4867 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -633,12 +633,12 @@ plookup::BasicTable& UltraCircuitBuilder_::get_table(const ploo template plookup::ReadData UltraCircuitBuilder_::create_gates_from_plookup_accumulators( - const plookup::MultiTableId& id, + const plookup::MultiTableIdOrPtr& id, const plookup::ReadData& read_values, const uint32_t key_a_index, std::optional key_b_index) { - const auto& multi_table = plookup::create_table(id); + const auto& multi_table = plookup::get_table(id); const size_t num_lookups = read_values[plookup::ColumnIdx::C1].size(); plookup::ReadData read_data; for (size_t i = 0; i < num_lookups; ++i) { diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index 78242840d9c..1abd54ef5de 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -1009,7 +1009,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase create_gates_from_plookup_accumulators( - const plookup::MultiTableId& id, + const plookup::MultiTableIdOrPtr& id, const plookup::ReadData& read_values, const uint32_t key_a_index, std::optional key_b_index = std::nullopt); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp index 6a2257bf102..132670d2a4a 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp @@ -108,13 +108,21 @@ const MultiTable& create_table(const MultiTableId id) return MULTI_TABLES[id]; } +const MultiTable& get_table(const MultiTableIdOrPtr& id) +{ + if (id.ptr == nullptr) { + return create_table(id.id); + } + return *id.ptr; +} + ReadData get_lookup_accumulators(const MultiTableId id, const fr& key_a, const fr& key_b, const bool is_2_to_1_lookup) { // return multi-table, populating global array of all multi-tables if need be - const auto& multi_table = create_table(id); + const auto& multi_table = get_table(id); const size_t num_lookups = multi_table.lookup_ids.size(); ReadData lookup; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp index 492793150d3..3e8552f8b1b 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp @@ -19,6 +19,7 @@ namespace bb::plookup { const MultiTable& create_table(MultiTableId id); +const MultiTable& get_table(const MultiTableIdOrPtr& id); ReadData get_lookup_accumulators(MultiTableId id, const bb::fr& key_a, diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp index 489d9e60071..f0ffde3f8e7 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp @@ -121,6 +121,21 @@ enum MultiTableId { NUM_MULTI_TABLES = KECCAK_NORMALIZE_AND_ROTATE + 25, }; +struct MultiTable; +struct MultiTableIdOrPtr { + // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. + MultiTableId id; + MultiTable* ptr; + MultiTableIdOrPtr(MultiTable* ptr) + : id(NUM_MULTI_TABLES) + , ptr(ptr) + {} + MultiTableIdOrPtr(MultiTableId id) + : id(id) + , ptr(nullptr) + {} +}; + struct MultiTable { // Coefficients are accumulated products of corresponding step sizes until that point std::vector column_1_coefficients; @@ -132,17 +147,17 @@ struct MultiTable { std::vector column_1_step_sizes; std::vector column_2_step_sizes; std::vector column_3_step_sizes; - typedef std::array table_out; - typedef std::array table_in; + using table_out = std::array; + using table_in = std::array; std::vector get_table_values; private: void init_step_sizes() { const size_t num_lookups = column_1_coefficients.size(); - column_1_step_sizes.emplace_back(bb::fr(1)); - column_2_step_sizes.emplace_back(bb::fr(1)); - column_3_step_sizes.emplace_back(bb::fr(1)); + column_1_step_sizes.emplace_back(1); + column_2_step_sizes.emplace_back(1); + column_3_step_sizes.emplace_back(1); std::vector coefficient_inverses(column_1_coefficients.begin(), column_1_coefficients.end()); std::copy(column_2_coefficients.begin(), column_2_coefficients.end(), std::back_inserter(coefficient_inverses)); @@ -192,67 +207,6 @@ struct MultiTable { MultiTable& operator=(MultiTable&& other) = default; }; -// struct PlookupLargeKeyTable { -// struct KeyEntry { -// uint256_t key; -// std::array value{ bb::fr(0), bb::fr(0) }; -// bool operator<(const KeyEntry& other) const { return key < other.key; } - -// std::array to_sorted_list_components(const bool use_two_keys) const -// { -// return { -// key[0], -// value[0], -// value[1], -// }; -// } -// }; - -// BasicTableId id; -// size_t table_index; -// size_t size; -// bool use_twin_keys; - -// bb::fr column_1_step_size = bb::fr(0); -// bb::fr column_2_step_size = bb::fr(0); -// bb::fr column_3_step_size = bb::fr(0); -// std::vector column_1; -// std::vector column_3; -// std::vector column_2; -// std::vector lookup_gates; - -// std::array (*get_values_from_key)(const std::array); -// }; - -// struct PlookupFatKeyTable { -// struct KeyEntry { -// bb::fr key; -// std::array values{ 0, 0 }; -// bool operator<(const KeyEntry& other) const -// { -// return (key.from_montgomery_form() < other.key.from_montgomery_form()); -// } - -// std::array to_sorted_list_components() const { return { key, values[0], values[0] }; } -// } - -// BasicTableId id; -// size_t table_index; -// size_t size; -// bool use_twin_keys; - -// bb::fr column_1_step_size = bb::fr(0); -// bb::fr column_2_step_size = bb::fr(0); -// bb::fr column_3_step_size = bb::fr(0); -// std::vector column_1; -// std::vector column_3; -// std::vector column_2; -// std::vector lookup_gates; - -// std::array (*get_values_from_key)(const std::array); - -// } - /** * @brief The structure contains the most basic table serving one function (for, example an xor table) * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp index 00d2b6846ee..1a4d80a2ca8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp @@ -468,6 +468,47 @@ TEST(stdlib_plookup, blake2s_xor) EXPECT_EQ(result, true); } +TEST(stdlib_plookup, dynamic_uint32_and) +{ + Builder builder = Builder(); + + const size_t num_lookups = (32 + 5) / 6; + + uint256_t left_value = (engine.get_random_uint256() & 0xffffffffULL); + uint256_t right_value = (engine.get_random_uint256() & 0xffffffffULL); + + field_ct left = witness_ct(&builder, bb::fr(left_value)); + field_ct right = witness_ct(&builder, bb::fr(right_value)); + + const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true); + const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); + const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); + std::vector out_expected(num_lookups); + std::vector left_expected(num_lookups); + std::vector right_expected(num_lookups); + + for (size_t i = 0; i < left_slices.size(); ++i) { + out_expected[i] = left_slices[i] & right_slices[i]; + left_expected[i] = left_slices[i]; + right_expected[i] = right_slices[i]; + } + + for (size_t i = num_lookups - 2; i < num_lookups; --i) { + out_expected[i] += out_expected[i + 1] * (1 << 6); + left_expected[i] += left_expected[i + 1] * (1 << 6); + right_expected[i] += right_expected[i + 1] * (1 << 6); + } + + for (size_t i = 0; i < num_lookups; ++i) { + EXPECT_EQ(lookup[ColumnIdx::C1][i].get_value(), bb::fr(left_expected[i])); + EXPECT_EQ(lookup[ColumnIdx::C2][i].get_value(), bb::fr(right_expected[i])); + EXPECT_EQ(lookup[ColumnIdx::C3][i].get_value(), bb::fr(out_expected[i])); + } + + bool result = builder.check_circuit(); + + EXPECT_EQ(result, true); +} TEST(stdlib_plookup, uint32_and) { Builder builder = Builder(); From e21b9d1bea3544f7b8bb6fbe0aa2b569309872e9 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:45:12 +0000 Subject: [PATCH 2/9] test dynamic interface --- .../proof_system/plookup_tables/plookup_tables.cpp | 2 +- .../proof_system/plookup_tables/plookup_tables.hpp | 2 +- .../stdlib/primitives/plookup/plookup.cpp | 11 ++++++----- .../stdlib/primitives/plookup/plookup.hpp | 9 +++++---- .../stdlib/primitives/plookup/plookup.test.cpp | 5 ++++- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp index 132670d2a4a..ee05af86e28 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp @@ -116,7 +116,7 @@ const MultiTable& get_table(const MultiTableIdOrPtr& id) return *id.ptr; } -ReadData get_lookup_accumulators(const MultiTableId id, +ReadData get_lookup_accumulators(const MultiTableIdOrPtr& id, const fr& key_a, const fr& key_b, const bool is_2_to_1_lookup) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp index 3e8552f8b1b..be85303fa8d 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp @@ -21,7 +21,7 @@ namespace bb::plookup { const MultiTable& create_table(MultiTableId id); const MultiTable& get_table(const MultiTableIdOrPtr& id); -ReadData get_lookup_accumulators(MultiTableId id, +ReadData get_lookup_accumulators(const MultiTableIdOrPtr& id, const bb::fr& key_a, const bb::fr& key_b = 0, bool is_2_to_1_lookup = false); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp index 103784ba2de..becddf86d14 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp @@ -14,7 +14,7 @@ using plookup::MultiTableId; using namespace bb; template -plookup::ReadData> plookup_read::get_lookup_accumulators(const MultiTableId id, +plookup::ReadData> plookup_read::get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id, const field_t& key_a_in, const field_t& key_b_in, const bool is_2_to_1_lookup) @@ -64,8 +64,8 @@ plookup::ReadData> plookup_read::get_lookup_accumulato } template -std::pair, field_t> plookup_read::read_pair_from_table(const MultiTableId id, - const field_t& key) +std::pair, field_t> plookup_read::read_pair_from_table( + const plookup::MultiTableIdOrPtr& id, const field_t& key) { const auto lookup = get_lookup_accumulators(id, key); @@ -73,7 +73,7 @@ std::pair, field_t> plookup_read::read_pair_f } template -field_t plookup_read::read_from_2_to_1_table(const MultiTableId id, +field_t plookup_read::read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id, const field_t& key_a, const field_t& key_b) { @@ -83,7 +83,8 @@ field_t plookup_read::read_from_2_to_1_table(const MultiTableI } template -field_t plookup_read::read_from_1_to_2_table(const MultiTableId id, const field_t& key_a) +field_t plookup_read::read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id, + const field_t& key_a) { const auto lookup = get_lookup_accumulators(id, key_a); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp index eb85f164242..36193072132 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp @@ -12,14 +12,15 @@ template class plookup_read { typedef field_t field_pt; public: - static std::pair read_pair_from_table(const plookup::MultiTableId id, const field_pt& key); + static std::pair read_pair_from_table(const plookup::MultiTableIdOrPtr& id, + const field_pt& key); - static field_pt read_from_2_to_1_table(const plookup::MultiTableId id, + static field_pt read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a, const field_pt& key_b); - static field_pt read_from_1_to_2_table(const plookup::MultiTableId id, const field_pt& key_a); + static field_pt read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a); - static plookup::ReadData get_lookup_accumulators(const plookup::MultiTableId id, + static plookup::ReadData get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a, const field_pt& key_b = 0, const bool is_2_to_1_lookup = false); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp index 1a4d80a2ca8..dfeba63895a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp @@ -468,6 +468,7 @@ TEST(stdlib_plookup, blake2s_xor) EXPECT_EQ(result, true); } +// Tests the dynamic multitable interface used by ACIR (the Noir interface to bb) TEST(stdlib_plookup, dynamic_uint32_and) { Builder builder = Builder(); @@ -480,7 +481,9 @@ TEST(stdlib_plookup, dynamic_uint32_and) field_ct left = witness_ct(&builder, bb::fr(left_value)); field_ct right = witness_ct(&builder, bb::fr(right_value)); - const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true); + MultiTable and_table = bb::plookup::uint_tables::get_uint32_and_table(); + + const auto lookup = plookup_read::get_lookup_accumulators(&and_table, left, right, true); const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); std::vector out_expected(num_lookups); From 85e680d554e5ae6c7fab6898071ce9cf89414d84 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:48:15 +0000 Subject: [PATCH 3/9] comment --- .../proof_system/plookup_tables/types.hpp | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp index f0ffde3f8e7..f4dd1083a6e 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp @@ -121,21 +121,6 @@ enum MultiTableId { NUM_MULTI_TABLES = KECCAK_NORMALIZE_AND_ROTATE + 25, }; -struct MultiTable; -struct MultiTableIdOrPtr { - // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. - MultiTableId id; - MultiTable* ptr; - MultiTableIdOrPtr(MultiTable* ptr) - : id(NUM_MULTI_TABLES) - , ptr(ptr) - {} - MultiTableIdOrPtr(MultiTableId id) - : id(id) - , ptr(nullptr) - {} -}; - struct MultiTable { // Coefficients are accumulated products of corresponding step sizes until that point std::vector column_1_coefficients; @@ -199,7 +184,7 @@ struct MultiTable { init_step_sizes(); } - MultiTable(){}; + MultiTable() = default; MultiTable(const MultiTable& other) = default; MultiTable(MultiTable&& other) = default; @@ -207,6 +192,23 @@ struct MultiTable { MultiTable& operator=(MultiTable&& other) = default; }; +// Represents either a predefined table from our enum list of supported lookup tables, or a dynamic lookup table defined +// by ACIR +struct MultiTableIdOrPtr { + // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. + MultiTableId id; + // Used if we are using a lookup table from a lookup table defined by e.g. ACIR, otherwise set to nullptr. + MultiTable* ptr; + MultiTableIdOrPtr(MultiTable* ptr) + : id(NUM_MULTI_TABLES) + , ptr(ptr) + {} + MultiTableIdOrPtr(MultiTableId id) + : id(id) + , ptr(nullptr) + {} +}; + /** * @brief The structure contains the most basic table serving one function (for, example an xor table) * From 4f3f5fde8d9affe0503e886fb16e4f09b0c46764 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:49:04 +0000 Subject: [PATCH 4/9] Update settings.json --- .vscode/settings.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index a3fa432542e..b7aa953af51 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -155,6 +155,5 @@ "C_Cpp.vcpkg.enabled": false, "C_Cpp.default.includePath": [ "barretenberg/cpp/src" - ], - "cmake.sourceDirectory": "/mnt/user-data/adam/aztec-packages/barretenberg/cpp" + ] } From 8d9cb1557e5d46aa1e097870b747d35d141e1297 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:34:31 +0000 Subject: [PATCH 5/9] start refactoring multitableid --- .vscode/settings.json | 3 +- .../circuit_builder/ultra_circuit_builder.cpp | 4 +- .../circuit_builder/ultra_circuit_builder.hpp | 2 +- .../plookup_tables/plookup_tables.cpp | 10 ++- .../plookup_tables/plookup_tables.hpp | 1 + .../proof_system/plookup_tables/types.hpp | 86 +++++-------------- .../primitives/plookup/plookup.test.cpp | 41 +++++++++ 7 files changed, 76 insertions(+), 71 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index b7aa953af51..a3fa432542e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -155,5 +155,6 @@ "C_Cpp.vcpkg.enabled": false, "C_Cpp.default.includePath": [ "barretenberg/cpp/src" - ] + ], + "cmake.sourceDirectory": "/mnt/user-data/adam/aztec-packages/barretenberg/cpp" } diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp index 21aa754edfe..e65527a4867 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp @@ -633,12 +633,12 @@ plookup::BasicTable& UltraCircuitBuilder_::get_table(const ploo template plookup::ReadData UltraCircuitBuilder_::create_gates_from_plookup_accumulators( - const plookup::MultiTableId& id, + const plookup::MultiTableIdOrPtr& id, const plookup::ReadData& read_values, const uint32_t key_a_index, std::optional key_b_index) { - const auto& multi_table = plookup::create_table(id); + const auto& multi_table = plookup::get_table(id); const size_t num_lookups = read_values[plookup::ColumnIdx::C1].size(); plookup::ReadData read_data; for (size_t i = 0; i < num_lookups; ++i) { diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp index 78242840d9c..1abd54ef5de 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp @@ -1009,7 +1009,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase create_gates_from_plookup_accumulators( - const plookup::MultiTableId& id, + const plookup::MultiTableIdOrPtr& id, const plookup::ReadData& read_values, const uint32_t key_a_index, std::optional key_b_index = std::nullopt); diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp index 6a2257bf102..132670d2a4a 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp @@ -108,13 +108,21 @@ const MultiTable& create_table(const MultiTableId id) return MULTI_TABLES[id]; } +const MultiTable& get_table(const MultiTableIdOrPtr& id) +{ + if (id.ptr == nullptr) { + return create_table(id.id); + } + return *id.ptr; +} + ReadData get_lookup_accumulators(const MultiTableId id, const fr& key_a, const fr& key_b, const bool is_2_to_1_lookup) { // return multi-table, populating global array of all multi-tables if need be - const auto& multi_table = create_table(id); + const auto& multi_table = get_table(id); const size_t num_lookups = multi_table.lookup_ids.size(); ReadData lookup; diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp index 492793150d3..3e8552f8b1b 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp @@ -19,6 +19,7 @@ namespace bb::plookup { const MultiTable& create_table(MultiTableId id); +const MultiTable& get_table(const MultiTableIdOrPtr& id); ReadData get_lookup_accumulators(MultiTableId id, const bb::fr& key_a, diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp index 489d9e60071..f0ffde3f8e7 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp @@ -121,6 +121,21 @@ enum MultiTableId { NUM_MULTI_TABLES = KECCAK_NORMALIZE_AND_ROTATE + 25, }; +struct MultiTable; +struct MultiTableIdOrPtr { + // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. + MultiTableId id; + MultiTable* ptr; + MultiTableIdOrPtr(MultiTable* ptr) + : id(NUM_MULTI_TABLES) + , ptr(ptr) + {} + MultiTableIdOrPtr(MultiTableId id) + : id(id) + , ptr(nullptr) + {} +}; + struct MultiTable { // Coefficients are accumulated products of corresponding step sizes until that point std::vector column_1_coefficients; @@ -132,17 +147,17 @@ struct MultiTable { std::vector column_1_step_sizes; std::vector column_2_step_sizes; std::vector column_3_step_sizes; - typedef std::array table_out; - typedef std::array table_in; + using table_out = std::array; + using table_in = std::array; std::vector get_table_values; private: void init_step_sizes() { const size_t num_lookups = column_1_coefficients.size(); - column_1_step_sizes.emplace_back(bb::fr(1)); - column_2_step_sizes.emplace_back(bb::fr(1)); - column_3_step_sizes.emplace_back(bb::fr(1)); + column_1_step_sizes.emplace_back(1); + column_2_step_sizes.emplace_back(1); + column_3_step_sizes.emplace_back(1); std::vector coefficient_inverses(column_1_coefficients.begin(), column_1_coefficients.end()); std::copy(column_2_coefficients.begin(), column_2_coefficients.end(), std::back_inserter(coefficient_inverses)); @@ -192,67 +207,6 @@ struct MultiTable { MultiTable& operator=(MultiTable&& other) = default; }; -// struct PlookupLargeKeyTable { -// struct KeyEntry { -// uint256_t key; -// std::array value{ bb::fr(0), bb::fr(0) }; -// bool operator<(const KeyEntry& other) const { return key < other.key; } - -// std::array to_sorted_list_components(const bool use_two_keys) const -// { -// return { -// key[0], -// value[0], -// value[1], -// }; -// } -// }; - -// BasicTableId id; -// size_t table_index; -// size_t size; -// bool use_twin_keys; - -// bb::fr column_1_step_size = bb::fr(0); -// bb::fr column_2_step_size = bb::fr(0); -// bb::fr column_3_step_size = bb::fr(0); -// std::vector column_1; -// std::vector column_3; -// std::vector column_2; -// std::vector lookup_gates; - -// std::array (*get_values_from_key)(const std::array); -// }; - -// struct PlookupFatKeyTable { -// struct KeyEntry { -// bb::fr key; -// std::array values{ 0, 0 }; -// bool operator<(const KeyEntry& other) const -// { -// return (key.from_montgomery_form() < other.key.from_montgomery_form()); -// } - -// std::array to_sorted_list_components() const { return { key, values[0], values[0] }; } -// } - -// BasicTableId id; -// size_t table_index; -// size_t size; -// bool use_twin_keys; - -// bb::fr column_1_step_size = bb::fr(0); -// bb::fr column_2_step_size = bb::fr(0); -// bb::fr column_3_step_size = bb::fr(0); -// std::vector column_1; -// std::vector column_3; -// std::vector column_2; -// std::vector lookup_gates; - -// std::array (*get_values_from_key)(const std::array); - -// } - /** * @brief The structure contains the most basic table serving one function (for, example an xor table) * diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp index 00d2b6846ee..1a4d80a2ca8 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp @@ -468,6 +468,47 @@ TEST(stdlib_plookup, blake2s_xor) EXPECT_EQ(result, true); } +TEST(stdlib_plookup, dynamic_uint32_and) +{ + Builder builder = Builder(); + + const size_t num_lookups = (32 + 5) / 6; + + uint256_t left_value = (engine.get_random_uint256() & 0xffffffffULL); + uint256_t right_value = (engine.get_random_uint256() & 0xffffffffULL); + + field_ct left = witness_ct(&builder, bb::fr(left_value)); + field_ct right = witness_ct(&builder, bb::fr(right_value)); + + const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true); + const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); + const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); + std::vector out_expected(num_lookups); + std::vector left_expected(num_lookups); + std::vector right_expected(num_lookups); + + for (size_t i = 0; i < left_slices.size(); ++i) { + out_expected[i] = left_slices[i] & right_slices[i]; + left_expected[i] = left_slices[i]; + right_expected[i] = right_slices[i]; + } + + for (size_t i = num_lookups - 2; i < num_lookups; --i) { + out_expected[i] += out_expected[i + 1] * (1 << 6); + left_expected[i] += left_expected[i + 1] * (1 << 6); + right_expected[i] += right_expected[i + 1] * (1 << 6); + } + + for (size_t i = 0; i < num_lookups; ++i) { + EXPECT_EQ(lookup[ColumnIdx::C1][i].get_value(), bb::fr(left_expected[i])); + EXPECT_EQ(lookup[ColumnIdx::C2][i].get_value(), bb::fr(right_expected[i])); + EXPECT_EQ(lookup[ColumnIdx::C3][i].get_value(), bb::fr(out_expected[i])); + } + + bool result = builder.check_circuit(); + + EXPECT_EQ(result, true); +} TEST(stdlib_plookup, uint32_and) { Builder builder = Builder(); From 0d230519bcf0ec3ad8e674a6809e31bb803d5fc3 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:45:12 +0000 Subject: [PATCH 6/9] test dynamic interface --- .../proof_system/plookup_tables/plookup_tables.cpp | 2 +- .../proof_system/plookup_tables/plookup_tables.hpp | 2 +- .../stdlib/primitives/plookup/plookup.cpp | 11 ++++++----- .../stdlib/primitives/plookup/plookup.hpp | 9 +++++---- .../stdlib/primitives/plookup/plookup.test.cpp | 5 ++++- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp index 132670d2a4a..ee05af86e28 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.cpp @@ -116,7 +116,7 @@ const MultiTable& get_table(const MultiTableIdOrPtr& id) return *id.ptr; } -ReadData get_lookup_accumulators(const MultiTableId id, +ReadData get_lookup_accumulators(const MultiTableIdOrPtr& id, const fr& key_a, const fr& key_b, const bool is_2_to_1_lookup) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp index 3e8552f8b1b..be85303fa8d 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/plookup_tables.hpp @@ -21,7 +21,7 @@ namespace bb::plookup { const MultiTable& create_table(MultiTableId id); const MultiTable& get_table(const MultiTableIdOrPtr& id); -ReadData get_lookup_accumulators(MultiTableId id, +ReadData get_lookup_accumulators(const MultiTableIdOrPtr& id, const bb::fr& key_a, const bb::fr& key_b = 0, bool is_2_to_1_lookup = false); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp index 103784ba2de..becddf86d14 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.cpp @@ -14,7 +14,7 @@ using plookup::MultiTableId; using namespace bb; template -plookup::ReadData> plookup_read::get_lookup_accumulators(const MultiTableId id, +plookup::ReadData> plookup_read::get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id, const field_t& key_a_in, const field_t& key_b_in, const bool is_2_to_1_lookup) @@ -64,8 +64,8 @@ plookup::ReadData> plookup_read::get_lookup_accumulato } template -std::pair, field_t> plookup_read::read_pair_from_table(const MultiTableId id, - const field_t& key) +std::pair, field_t> plookup_read::read_pair_from_table( + const plookup::MultiTableIdOrPtr& id, const field_t& key) { const auto lookup = get_lookup_accumulators(id, key); @@ -73,7 +73,7 @@ std::pair, field_t> plookup_read::read_pair_f } template -field_t plookup_read::read_from_2_to_1_table(const MultiTableId id, +field_t plookup_read::read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id, const field_t& key_a, const field_t& key_b) { @@ -83,7 +83,8 @@ field_t plookup_read::read_from_2_to_1_table(const MultiTableI } template -field_t plookup_read::read_from_1_to_2_table(const MultiTableId id, const field_t& key_a) +field_t plookup_read::read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id, + const field_t& key_a) { const auto lookup = get_lookup_accumulators(id, key_a); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp index eb85f164242..36193072132 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.hpp @@ -12,14 +12,15 @@ template class plookup_read { typedef field_t field_pt; public: - static std::pair read_pair_from_table(const plookup::MultiTableId id, const field_pt& key); + static std::pair read_pair_from_table(const plookup::MultiTableIdOrPtr& id, + const field_pt& key); - static field_pt read_from_2_to_1_table(const plookup::MultiTableId id, + static field_pt read_from_2_to_1_table(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a, const field_pt& key_b); - static field_pt read_from_1_to_2_table(const plookup::MultiTableId id, const field_pt& key_a); + static field_pt read_from_1_to_2_table(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a); - static plookup::ReadData get_lookup_accumulators(const plookup::MultiTableId id, + static plookup::ReadData get_lookup_accumulators(const plookup::MultiTableIdOrPtr& id, const field_pt& key_a, const field_pt& key_b = 0, const bool is_2_to_1_lookup = false); diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp index 1a4d80a2ca8..dfeba63895a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp @@ -468,6 +468,7 @@ TEST(stdlib_plookup, blake2s_xor) EXPECT_EQ(result, true); } +// Tests the dynamic multitable interface used by ACIR (the Noir interface to bb) TEST(stdlib_plookup, dynamic_uint32_and) { Builder builder = Builder(); @@ -480,7 +481,9 @@ TEST(stdlib_plookup, dynamic_uint32_and) field_ct left = witness_ct(&builder, bb::fr(left_value)); field_ct right = witness_ct(&builder, bb::fr(right_value)); - const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true); + MultiTable and_table = bb::plookup::uint_tables::get_uint32_and_table(); + + const auto lookup = plookup_read::get_lookup_accumulators(&and_table, left, right, true); const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); std::vector out_expected(num_lookups); From 3def753f55b7c00fbcb87f4473ab2551bb97b82f Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:48:15 +0000 Subject: [PATCH 7/9] comment --- .../proof_system/plookup_tables/types.hpp | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp index f0ffde3f8e7..f4dd1083a6e 100644 --- a/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp +++ b/barretenberg/cpp/src/barretenberg/proof_system/plookup_tables/types.hpp @@ -121,21 +121,6 @@ enum MultiTableId { NUM_MULTI_TABLES = KECCAK_NORMALIZE_AND_ROTATE + 25, }; -struct MultiTable; -struct MultiTableIdOrPtr { - // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. - MultiTableId id; - MultiTable* ptr; - MultiTableIdOrPtr(MultiTable* ptr) - : id(NUM_MULTI_TABLES) - , ptr(ptr) - {} - MultiTableIdOrPtr(MultiTableId id) - : id(id) - , ptr(nullptr) - {} -}; - struct MultiTable { // Coefficients are accumulated products of corresponding step sizes until that point std::vector column_1_coefficients; @@ -199,7 +184,7 @@ struct MultiTable { init_step_sizes(); } - MultiTable(){}; + MultiTable() = default; MultiTable(const MultiTable& other) = default; MultiTable(MultiTable&& other) = default; @@ -207,6 +192,23 @@ struct MultiTable { MultiTable& operator=(MultiTable&& other) = default; }; +// Represents either a predefined table from our enum list of supported lookup tables, or a dynamic lookup table defined +// by ACIR +struct MultiTableIdOrPtr { + // Used if we are using a lookup table from our predefined list, otherwise set to NUM_MULTI_TABLES and unused. + MultiTableId id; + // Used if we are using a lookup table from a lookup table defined by e.g. ACIR, otherwise set to nullptr. + MultiTable* ptr; + MultiTableIdOrPtr(MultiTable* ptr) + : id(NUM_MULTI_TABLES) + , ptr(ptr) + {} + MultiTableIdOrPtr(MultiTableId id) + : id(id) + , ptr(nullptr) + {} +}; + /** * @brief The structure contains the most basic table serving one function (for, example an xor table) * From 35219811786f107d8ac3d9308082e98e1492c659 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:52:47 +0000 Subject: [PATCH 8/9] dedupe test --- .../primitives/plookup/plookup.test.cpp | 54 ++++--------------- 1 file changed, 11 insertions(+), 43 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp index dfeba63895a..e147fdbae89 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/plookup/plookup.test.cpp @@ -468,8 +468,7 @@ TEST(stdlib_plookup, blake2s_xor) EXPECT_EQ(result, true); } -// Tests the dynamic multitable interface used by ACIR (the Noir interface to bb) -TEST(stdlib_plookup, dynamic_uint32_and) +static void test_uint32_and(const MultiTableIdOrPtr& id) { Builder builder = Builder(); @@ -481,9 +480,7 @@ TEST(stdlib_plookup, dynamic_uint32_and) field_ct left = witness_ct(&builder, bb::fr(left_value)); field_ct right = witness_ct(&builder, bb::fr(right_value)); - MultiTable and_table = bb::plookup::uint_tables::get_uint32_and_table(); - - const auto lookup = plookup_read::get_lookup_accumulators(&and_table, left, right, true); + const auto lookup = plookup_read::get_lookup_accumulators(id, left, right, true); const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); std::vector out_expected(num_lookups); @@ -512,46 +509,17 @@ TEST(stdlib_plookup, dynamic_uint32_and) EXPECT_EQ(result, true); } -TEST(stdlib_plookup, uint32_and) -{ - Builder builder = Builder(); - - const size_t num_lookups = (32 + 5) / 6; - - uint256_t left_value = (engine.get_random_uint256() & 0xffffffffULL); - uint256_t right_value = (engine.get_random_uint256() & 0xffffffffULL); - - field_ct left = witness_ct(&builder, bb::fr(left_value)); - field_ct right = witness_ct(&builder, bb::fr(right_value)); - - const auto lookup = plookup_read::get_lookup_accumulators(MultiTableId::UINT32_AND, left, right, true); - const auto left_slices = numeric::slice_input(left_value, 1 << 6, num_lookups); - const auto right_slices = numeric::slice_input(right_value, 1 << 6, num_lookups); - std::vector out_expected(num_lookups); - std::vector left_expected(num_lookups); - std::vector right_expected(num_lookups); - - for (size_t i = 0; i < left_slices.size(); ++i) { - out_expected[i] = left_slices[i] & right_slices[i]; - left_expected[i] = left_slices[i]; - right_expected[i] = right_slices[i]; - } - - for (size_t i = num_lookups - 2; i < num_lookups; --i) { - out_expected[i] += out_expected[i + 1] * (1 << 6); - left_expected[i] += left_expected[i + 1] * (1 << 6); - right_expected[i] += right_expected[i + 1] * (1 << 6); - } - - for (size_t i = 0; i < num_lookups; ++i) { - EXPECT_EQ(lookup[ColumnIdx::C1][i].get_value(), bb::fr(left_expected[i])); - EXPECT_EQ(lookup[ColumnIdx::C2][i].get_value(), bb::fr(right_expected[i])); - EXPECT_EQ(lookup[ColumnIdx::C3][i].get_value(), bb::fr(out_expected[i])); - } - bool result = builder.check_circuit(); +// Tests the dynamic multitable interface used by ACIR (the Noir interface to bb) +TEST(stdlib_plookup, dynamic_uint32_and) +{ + MultiTable and_table = bb::plookup::uint_tables::get_uint32_and_table(); + test_uint32_and(&and_table); +} - EXPECT_EQ(result, true); +TEST(stdlib_plookup, uint32_and) +{ + test_uint32_and(MultiTableId::UINT32_AND); } TEST(stdlib_plookup, secp256k1_generator) From a53bd243109d98a4f99f2c9ee5c6cef39a1cdcd9 Mon Sep 17 00:00:00 2001 From: ludamad Date: Mon, 19 Feb 2024 13:54:25 +0000 Subject: [PATCH 9/9] Update settings.json --- .vscode/settings.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index a3fa432542e..b7aa953af51 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -155,6 +155,5 @@ "C_Cpp.vcpkg.enabled": false, "C_Cpp.default.includePath": [ "barretenberg/cpp/src" - ], - "cmake.sourceDirectory": "/mnt/user-data/adam/aztec-packages/barretenberg/cpp" + ] }