From 2ba0cd352cc9ab473a9bd26a9603b053f799c338 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 13 Feb 2025 20:25:26 +0000 Subject: [PATCH 01/16] new op queue with basic functionality and a test --- .../op_queue/ecc_op_queue.test.cpp | 110 +++++++ .../op_queue/new_ecc_op_queue.hpp | 299 ++++++++++++++++++ 2 files changed, 409 insertions(+) create mode 100644 barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index a0d2596da286..9111694c65a1 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -1,8 +1,60 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include "barretenberg/common/zip_view.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp" #include +#include + using namespace bb; +class ECCOpQueueTest : public ::testing::Test { + using scalar = fr; + + public: + struct MockUltraOpsTable { + std::array, 4> table; + void append(const UltraOp& op) + { + table[0].push_back(op.op); + table[1].push_back(op.x_lo); + table[2].push_back(op.x_hi); + table[3].push_back(op.y_lo); + + table[0].push_back(0); + table[1].push_back(op.y_hi); + table[2].push_back(op.z_1); + table[3].push_back(op.z_2); + } + + // Construct the mock ultra ops table such that the chunks appear in reverse order, as if prepended + MockUltraOpsTable(const auto& ops_chunks) + { + for (auto& ops_chunk : std::ranges::reverse_view(ops_chunks)) { + for (const auto& op : ops_chunk) { + append(op); + } + } + } + + size_t size() const { return table[0].size(); } + }; + + static UltraOp random_ultra_op() + { + UltraOp op; + op.op_code = NULL_OP; + op.op = scalar::random_element(); + op.x_lo = scalar::random_element(); + op.x_hi = scalar::random_element(); + op.y_lo = scalar::random_element(); + op.y_hi = scalar::random_element(); + op.z_1 = scalar::random_element(); + op.z_2 = scalar::random_element(); + op.return_is_infinity = false; + return op; + } +}; + TEST(ECCOpQueueTest, Basic) { ECCOpQueue op_queue; @@ -36,3 +88,61 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) op_queue.eq_and_reset(); EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } + +TEST(ECCOpQueueTest, New) +{ + using point = g1::affine_element; + using scalar = fr; + + // Compute a simple point accumulation natively + auto P1 = point::random_element(); + auto P2 = point::random_element(); + auto z = scalar::random_element(); + auto P_expected = P1 + P2 * z; + + // Add the same operations to the ECC op queue; the native computation is performed under the hood. + NewECCOpQueue op_queue; + op_queue.add_accumulate(P1); + op_queue.mul_accumulate(P2, z); + + // The correct result should now be stored in the accumulator within the op queue + EXPECT_EQ(op_queue.get_accumulator(), P_expected); + + // Adding an equality op should reset the accumulator to zero (the point at infinity) + op_queue.eq_and_reset(); + EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); +} + +TEST(ECCOpQueueTest, NewConstruction) +{ + // Construct chunks of ultra ops, each representing the set of ops added by a single circuit + const size_t NUM_CHUNKS = 3; + std::array, NUM_CHUNKS> ultra_ops_chunks; + std::array chunk_sizes = { 4, 2, 7 }; + for (auto [chunk, size] : zip_view(ultra_ops_chunks, chunk_sizes)) { + for (size_t i = 0; i < size; ++i) { + chunk.push_back(ECCOpQueueTest::random_ultra_op()); + } + } + + // Construct the mock ultra ops table which contains the chunks ordered in reverse (as if prepended) + ECCOpQueueTest::MockUltraOpsTable mock_table(ultra_ops_chunks); + + // Construct the concatenated table internal to the op queue + NewECCOpQueue op_queue; + for (const auto& chunk : ultra_ops_chunks) { + for (const auto& op : chunk) { + op_queue.append_ultra_op(op); + } + op_queue.construct_concatenated_table(); + } + + // Compute the expected total table size as the sum of the chink sizes times 2 + size_t expected_table_size = 0; + for (const auto& size : chunk_sizes) { + expected_table_size += size * 2; + } + + EXPECT_EQ(op_queue.ultra_ops_size(), expected_table_size); + EXPECT_EQ(mock_table.size(), expected_table_size); +} diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp new file mode 100644 index 000000000000..90819a1fa349 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -0,0 +1,299 @@ +#pragma once + +#include "barretenberg/ecc/curves/bn254/bn254.hpp" +#include "barretenberg/eccvm/eccvm_builder_types.hpp" +#include "barretenberg/stdlib/primitives/bigfield/constants.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/eccvm_row_tracker.hpp" +namespace bb { + +/** + * @brief Used to construct execution trace representations of elliptic curve operations. + * + * @details Currently the targets in execution traces are: four advice wires in UltraCircuitBuilder and 5 wires in the + * ECCVM. In each case, the variable values are stored in this class, since the same values will need to be used later + * by the TranslationVMCircuitBuilder. The circuit builders will store witness indices which are indices in the + * ultra (resp. eccvm) ops members of this class (rather than in the builder's variables array). + */ +class NewECCOpQueue { + using Curve = curve::BN254; + using Point = Curve::AffineElement; + using Fr = Curve::ScalarField; + Point point_at_infinity = Curve::Group::affine_point_at_infinity; + + // The operations written to the queue are also performed natively; the result is stored in accumulator + Point accumulator = point_at_infinity; + + static constexpr size_t DEFAULT_NON_NATIVE_FIELD_LIMB_BITS = stdlib::NUM_LIMB_BITS_IN_FIELD_SIMULATION; + + std::vector> raw_ops; + std::array, 4> ultra_ops; // ops encoded in the width-4 Ultra format + + // Tracks numer of muls and size of eccvm in real time as the op queue is updated + EccvmRowTracker eccvm_row_tracker; + + struct UltraTableChunk { + std::array, 4> columns; + std::unique_ptr prev; + + UltraTableChunk(const size_t size_hint = 0) + { + // reserve space for the columns + for (auto& column : columns) { + column.reserve(size_hint); + } + } + + size_t size() const { return columns[0].size(); } + + /** + * @brief Populate two rows of the table, representing a complete ECC operation + * @note Only the first 'op' field is utilized so the second is explicitly set to 0 + * + */ + void append_operation(const UltraOp& tuple) + { + columns[0].emplace_back(tuple.op); + columns[1].emplace_back(tuple.x_lo); + columns[2].emplace_back(tuple.x_hi); + columns[3].emplace_back(tuple.y_lo); + + columns[0].emplace_back(0); + columns[1].emplace_back(tuple.y_hi); + columns[2].emplace_back(tuple.z_1); + columns[3].emplace_back(tuple.z_2); + } + }; + + std::unique_ptr head = std::make_unique(); + + public: + using ECCVMOperation = bb::eccvm::VMOperation; + + void append_ultra_op(const UltraOp& op) { head->append_operation(op); } + + size_t ultra_ops_size() const + { + size_t total_size = 0; + for (auto* chunk = head.get(); chunk != nullptr; chunk = chunk->prev.get()) { + total_size += chunk->size(); + } + return total_size; + } + + void construct_concatenated_table(const size_t size_hint = 0) + { + auto new_head = std::make_unique(size_hint); // new empty table chunk + new_head->prev = std::move(head); // link new chunk to current head + head = std::move(new_head); // update head + } + + const std::vector& get_raw_ops() { return raw_ops; } + + /** + * @brief Get the number of rows in the 'msm' column section, for all msms in the circuit + */ + size_t get_num_msm_rows() const { return eccvm_row_tracker.get_num_msm_rows(); } + + /** + * @brief Get the number of rows for the current ECCVM circuit + */ + size_t get_num_rows() const { return eccvm_row_tracker.get_num_rows(); } + + /** + * @brief get number of muls for the current ECCVM circuit + */ + uint32_t get_number_of_muls() const { return eccvm_row_tracker.get_number_of_muls(); } + + // TODO(https://github.com/AztecProtocol/barretenberg/issues/905): Can remove this with better handling of scalar + // mul against 0 + void append_nonzero_ops() + { + // Add an element and scalar the accumulation of which leaves no Point-at-Infinity commitments + const auto x = uint256_t(0xd3c208c16d87cfd3, 0xd97816a916871ca8, 0x9b85045b68181585, 0x30644e72e131a02); + const auto y = uint256_t(0x3ce1cc9c7e645a83, 0x2edac647851e3ac5, 0xd0cbe61fced2bc53, 0x1a76dae6d3272396); + auto padding_element = Point(x, y); + auto padding_scalar = -Fr::one(); + mul_accumulate(padding_element, padding_scalar); + eq_and_reset(); + } + + /** + * @brief A fuzzing only method for setting raw ops directly + * + */ + void set_raw_ops_for_fuzzing(std::vector& raw_ops_in) { raw_ops = raw_ops_in; } + + /** + * @brief A testing only method that adds an erroneous equality op to the raw ops + * @brief May be used to ensure that ECCVM responds as expected when encountering a bad op + * + */ + void add_erroneous_equality_op_for_testing() + { + raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = Point::random_element() }); + } + + /** + * @brief Write empty row to queue + * @warning This is for testing purposes only. Currently no valid use case. + * + */ + void empty_row_for_testing() + { + raw_ops.emplace_back(ECCVMOperation{ .base_point = point_at_infinity }); + + eccvm_row_tracker.update_cached_msms(raw_ops.back()); + } + + Point get_accumulator() { return accumulator; } + + /** + * @brief Write point addition op to queue and natively perform addition + * + * @param to_add + */ + UltraOp add_accumulate(const Point& to_add) + { + // Update the accumulator natively + accumulator = accumulator + to_add; + + // Store the raw operation + raw_ops.emplace_back(ECCVMOperation{ .add = true, .base_point = to_add }); + eccvm_row_tracker.update_cached_msms(raw_ops.back()); + + // Construct and store the operation in the ultra op format + return construct_and_populate_ultra_ops(ADD_ACCUM, to_add); + } + + /** + * @brief Write multiply and add op to queue and natively perform operation + * + * @param to_add + */ + UltraOp mul_accumulate(const Point& to_mul, const Fr& scalar) + { + // Update the accumulator natively + accumulator = accumulator + to_mul * scalar; + + // Construct and store the operation in the ultra op format + UltraOp ultra_op = construct_and_populate_ultra_ops(MUL_ACCUM, to_mul, scalar); + + // Store the raw operation + raw_ops.emplace_back(ECCVMOperation{ + .mul = true, + .base_point = to_mul, + .z1 = ultra_op.z_1, + .z2 = ultra_op.z_2, + .mul_scalar_full = scalar, + }); + eccvm_row_tracker.update_cached_msms(raw_ops.back()); + + return ultra_op; + } + + /** + * @brief Write no op (i.e. empty row) + * + */ + UltraOp no_op() + { + // Store raw operation + raw_ops.emplace_back(ECCVMOperation{}); + eccvm_row_tracker.update_cached_msms(raw_ops.back()); + + // Construct and store the operation in the ultra op format + return construct_and_populate_ultra_ops(NULL_OP, accumulator); + } + + /** + * @brief Write equality op using internal accumulator point + * + * @return current internal accumulator point (prior to reset to 0) + */ + UltraOp eq_and_reset() + { + auto expected = accumulator; + accumulator.self_set_infinity(); + + // Store raw operation + raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = expected }); + eccvm_row_tracker.update_cached_msms(raw_ops.back()); + + // Construct and store the operation in the ultra op format + return construct_and_populate_ultra_ops(EQUALITY, expected); + } + + private: + /** + * @brief Given an ecc operation and its inputs, decompose into ultra format and populate ultra_ops + * + * @param op_code + * @param point + * @param scalar + * @return UltraOp + */ + UltraOp construct_and_populate_ultra_ops(EccOpCode op_code, const Point& point, const Fr& scalar = Fr::zero()) + { + UltraOp ultra_op; + ultra_op.op_code = op_code; + ultra_op.op = Fr(static_cast(op_code)); + + // Decompose point coordinates (Fq) into hi-lo chunks (Fr) + const size_t CHUNK_SIZE = 2 * DEFAULT_NON_NATIVE_FIELD_LIMB_BITS; + uint256_t x_256(point.x); + uint256_t y_256(point.y); + ultra_op.return_is_infinity = point.is_point_at_infinity(); + // if we have a point at infinity, set x/y to zero + // in the biggroup_goblin class we use `assert_equal` statements to validate + // the original in-circuit coordinate values are also zero + if (point.is_point_at_infinity()) { + x_256 = 0; + y_256 = 0; + } + ultra_op.x_lo = Fr(x_256.slice(0, CHUNK_SIZE)); + ultra_op.x_hi = Fr(x_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2)); + ultra_op.y_lo = Fr(y_256.slice(0, CHUNK_SIZE)); + ultra_op.y_hi = Fr(y_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2)); + + // Split scalar into 128 bit endomorphism scalars + Fr z_1 = 0; + Fr z_2 = 0; + auto converted = scalar.from_montgomery_form(); + uint256_t converted_u256(scalar); + if (converted_u256.get_msb() <= 128) { + ultra_op.z_1 = scalar; + ultra_op.z_2 = 0; + } else { + Fr::split_into_endomorphism_scalars(converted, z_1, z_2); + ultra_op.z_1 = z_1.to_montgomery_form(); + ultra_op.z_2 = z_2.to_montgomery_form(); + } + + append_to_ultra_ops(ultra_op); + head->append_operation(ultra_op); + + return ultra_op; + } + + /** + * @brief Populate two rows of the ultra ops,representing a complete ECC operation + * @note Only the first 'op' field is utilized so the second is explicitly set to 0 + * + * @param tuple + */ + void append_to_ultra_ops(UltraOp tuple) + { + ultra_ops[0].emplace_back(tuple.op); + ultra_ops[1].emplace_back(tuple.x_lo); + ultra_ops[2].emplace_back(tuple.x_hi); + ultra_ops[3].emplace_back(tuple.y_lo); + + ultra_ops[0].emplace_back(0); + ultra_ops[1].emplace_back(tuple.y_hi); + ultra_ops[2].emplace_back(tuple.z_1); + ultra_ops[3].emplace_back(tuple.z_2); + } +}; + +} // namespace bb From 453a72b0c11860a6c11c559b6deed2085b03f94f Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 13 Feb 2025 21:15:56 +0000 Subject: [PATCH 02/16] basic support for populating ultra ops table --- .../op_queue/ecc_op_queue.test.cpp | 22 ++++++++++++++++++- .../op_queue/new_ecc_op_queue.hpp | 19 +++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 9111694c65a1..f369f9e980e4 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -134,7 +134,7 @@ TEST(ECCOpQueueTest, NewConstruction) for (const auto& op : chunk) { op_queue.append_ultra_op(op); } - op_queue.construct_concatenated_table(); + op_queue.concatenate_subtable(); } // Compute the expected total table size as the sum of the chink sizes times 2 @@ -145,4 +145,24 @@ TEST(ECCOpQueueTest, NewConstruction) EXPECT_EQ(op_queue.ultra_ops_size(), expected_table_size); EXPECT_EQ(mock_table.size(), expected_table_size); + + std::array, 4> ultra_ops_table; + for (auto& column : ultra_ops_table) { + column.resize(expected_table_size); + } + + std::array, 4> ultra_ops_table_spans; + for (auto [column_span, column] : zip_view(ultra_ops_table_spans, ultra_ops_table)) { + column_span = column; + } + + op_queue.populate_ultra_ops_table(ultra_ops_table_spans); + + // ultra_ops_table[2][3] += 1; + + for (auto [expected_column, column] : zip_view(mock_table.table, ultra_ops_table)) { + for (auto [expected_value, value] : zip_view(expected_column, column)) { + EXPECT_EQ(expected_value, value); + } + } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index 90819a1fa349..60cb69348bdd 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -21,6 +21,8 @@ class NewECCOpQueue { using Fr = Curve::ScalarField; Point point_at_infinity = Curve::Group::affine_point_at_infinity; + static constexpr size_t ULTRA_OPS_TABLE_WIDTH = 4; + // The operations written to the queue are also performed natively; the result is stored in accumulator Point accumulator = point_at_infinity; @@ -32,6 +34,7 @@ class NewECCOpQueue { // Tracks numer of muls and size of eccvm in real time as the op queue is updated EccvmRowTracker eccvm_row_tracker; + // WORKTODO: instead of chunk maybe segment? subtable? struct UltraTableChunk { std::array, 4> columns; std::unique_ptr prev; @@ -81,13 +84,27 @@ class NewECCOpQueue { return total_size; } - void construct_concatenated_table(const size_t size_hint = 0) + void concatenate_subtable(const size_t size_hint = 0) { auto new_head = std::make_unique(size_hint); // new empty table chunk new_head->prev = std::move(head); // link new chunk to current head head = std::move(new_head); // update head } + void populate_ultra_ops_table(std::array, ULTRA_OPS_TABLE_WIDTH>& ultra_ops_table) + { + size_t i = 0; + for (auto* chunk = head.get(); chunk != nullptr; chunk = chunk->prev.get()) { + for (size_t j = 0; j < chunk->size(); ++j) { + ultra_ops_table[0][i] = chunk->columns[0][j]; + ultra_ops_table[1][i] = chunk->columns[1][j]; + ultra_ops_table[2][i] = chunk->columns[2][j]; + ultra_ops_table[3][i] = chunk->columns[3][j]; + ++i; + } + } + } + const std::vector& get_raw_ops() { return raw_ops; } /** From 95cbd1fc467c00970acad6168d9ed908ddd06226 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 13 Feb 2025 21:50:57 +0000 Subject: [PATCH 03/16] improve naming --- .../op_queue/ecc_op_queue.test.cpp | 54 +++++++++---------- .../op_queue/new_ecc_op_queue.hpp | 39 +++++++------- 2 files changed, 45 insertions(+), 48 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index f369f9e980e4..5a7fd0c0c3a7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -26,11 +26,11 @@ class ECCOpQueueTest : public ::testing::Test { table[3].push_back(op.z_2); } - // Construct the mock ultra ops table such that the chunks appear in reverse order, as if prepended - MockUltraOpsTable(const auto& ops_chunks) + // Construct the= ultra ops table such that the subtables appear in reverse order, as if prepended + MockUltraOpsTable(const auto& subtable_ops) { - for (auto& ops_chunk : std::ranges::reverse_view(ops_chunks)) { - for (const auto& op : ops_chunk) { + for (auto& ops : std::ranges::reverse_view(subtable_ops)) { + for (const auto& op : ops) { append(op); } } @@ -113,54 +113,52 @@ TEST(ECCOpQueueTest, New) EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } -TEST(ECCOpQueueTest, NewConstruction) +TEST(ECCOpQueueTest, ConcatenatedTableConstruction) { - // Construct chunks of ultra ops, each representing the set of ops added by a single circuit - const size_t NUM_CHUNKS = 3; - std::array, NUM_CHUNKS> ultra_ops_chunks; - std::array chunk_sizes = { 4, 2, 7 }; - for (auto [chunk, size] : zip_view(ultra_ops_chunks, chunk_sizes)) { - for (size_t i = 0; i < size; ++i) { - chunk.push_back(ECCOpQueueTest::random_ultra_op()); + // Construct sets of ultra ops, each representing those added by a single circuit + const size_t NUM_SUBTABLES = 3; + std::array, NUM_SUBTABLES> subtable_ultra_ops; + std::array subtable_op_counts = { 4, 2, 7 }; + for (auto [subtable_ops, op_count] : zip_view(subtable_ultra_ops, subtable_op_counts)) { + for (size_t i = 0; i < op_count; ++i) { + subtable_ops.push_back(ECCOpQueueTest::random_ultra_op()); } } - // Construct the mock ultra ops table which contains the chunks ordered in reverse (as if prepended) - ECCOpQueueTest::MockUltraOpsTable mock_table(ultra_ops_chunks); + // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) + ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); // Construct the concatenated table internal to the op queue NewECCOpQueue op_queue; - for (const auto& chunk : ultra_ops_chunks) { - for (const auto& op : chunk) { + for (const auto& subtable_ops : subtable_ultra_ops) { + for (const auto& op : subtable_ops) { op_queue.append_ultra_op(op); } - op_queue.concatenate_subtable(); + op_queue.concatenate_ultra_subtable(); } - // Compute the expected total table size as the sum of the chink sizes times 2 + // Compute the expected total table size as the sum of the subtable op counts times 2 size_t expected_table_size = 0; - for (const auto& size : chunk_sizes) { - expected_table_size += size * 2; + for (const auto& count : subtable_op_counts) { + expected_table_size += count * 2; } - EXPECT_EQ(op_queue.ultra_ops_size(), expected_table_size); - EXPECT_EQ(mock_table.size(), expected_table_size); + // Check that the ultra ops table internal to the op queue has the correct size + EXPECT_EQ(op_queue.ultra_ops_table_size(), expected_table_size); + // Define a storage for the genuine ultra ops table and populate using the data stored in the op queue std::array, 4> ultra_ops_table; - for (auto& column : ultra_ops_table) { - column.resize(expected_table_size); - } - std::array, 4> ultra_ops_table_spans; for (auto [column_span, column] : zip_view(ultra_ops_table_spans, ultra_ops_table)) { + column.resize(expected_table_size); column_span = column; } - op_queue.populate_ultra_ops_table(ultra_ops_table_spans); // ultra_ops_table[2][3] += 1; - for (auto [expected_column, column] : zip_view(mock_table.table, ultra_ops_table)) { + // Check that the ultra ops table constructed by the op queue matches the expected table + for (auto [expected_column, column] : zip_view(expected_ultra_ops_table.table, ultra_ops_table)) { for (auto [expected_value, value] : zip_view(expected_column, column)) { EXPECT_EQ(expected_value, value); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index 60cb69348bdd..2c959c1cb5b0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -34,12 +34,11 @@ class NewECCOpQueue { // Tracks numer of muls and size of eccvm in real time as the op queue is updated EccvmRowTracker eccvm_row_tracker; - // WORKTODO: instead of chunk maybe segment? subtable? - struct UltraTableChunk { + struct UltraSubtable { std::array, 4> columns; - std::unique_ptr prev; + std::unique_ptr prev; - UltraTableChunk(const size_t size_hint = 0) + UltraSubtable(const size_t size_hint = 0) { // reserve space for the columns for (auto& column : columns) { @@ -68,38 +67,38 @@ class NewECCOpQueue { } }; - std::unique_ptr head = std::make_unique(); + std::unique_ptr current_ultra_subtable = std::make_unique(); public: using ECCVMOperation = bb::eccvm::VMOperation; - void append_ultra_op(const UltraOp& op) { head->append_operation(op); } + void append_ultra_op(const UltraOp& op) { current_ultra_subtable->append_operation(op); } - size_t ultra_ops_size() const + size_t ultra_ops_table_size() const { size_t total_size = 0; - for (auto* chunk = head.get(); chunk != nullptr; chunk = chunk->prev.get()) { - total_size += chunk->size(); + for (auto* subtable = current_ultra_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { + total_size += subtable->size(); } return total_size; } - void concatenate_subtable(const size_t size_hint = 0) + void concatenate_ultra_subtable(const size_t size_hint = 0) { - auto new_head = std::make_unique(size_hint); // new empty table chunk - new_head->prev = std::move(head); // link new chunk to current head - head = std::move(new_head); // update head + auto new_ultra_subtable = std::make_unique(size_hint); // new empty subtable + new_ultra_subtable->prev = std::move(current_ultra_subtable); // link new subtable to current subtable + current_ultra_subtable = std::move(new_ultra_subtable); // update current subtable } void populate_ultra_ops_table(std::array, ULTRA_OPS_TABLE_WIDTH>& ultra_ops_table) { size_t i = 0; - for (auto* chunk = head.get(); chunk != nullptr; chunk = chunk->prev.get()) { - for (size_t j = 0; j < chunk->size(); ++j) { - ultra_ops_table[0][i] = chunk->columns[0][j]; - ultra_ops_table[1][i] = chunk->columns[1][j]; - ultra_ops_table[2][i] = chunk->columns[2][j]; - ultra_ops_table[3][i] = chunk->columns[3][j]; + for (auto* subtable = current_ultra_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { + for (size_t j = 0; j < subtable->size(); ++j) { + ultra_ops_table[0][i] = subtable->columns[0][j]; + ultra_ops_table[1][i] = subtable->columns[1][j]; + ultra_ops_table[2][i] = subtable->columns[2][j]; + ultra_ops_table[3][i] = subtable->columns[3][j]; ++i; } } @@ -288,7 +287,7 @@ class NewECCOpQueue { } append_to_ultra_ops(ultra_op); - head->append_operation(ultra_op); + current_ultra_subtable->append_operation(ultra_op); return ultra_op; } From bce7d70b686aafe02b18a9661c086e7c1b7f0331 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Thu, 13 Feb 2025 23:31:47 +0000 Subject: [PATCH 04/16] class is reduced to UltraOpsTable --- .../op_queue/ecc_op_queue.test.cpp | 66 ++-- .../op_queue/new_ecc_op_queue.hpp | 296 +++--------------- 2 files changed, 59 insertions(+), 303 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 5a7fd0c0c3a7..c6905dba01f5 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -12,18 +12,18 @@ class ECCOpQueueTest : public ::testing::Test { public: struct MockUltraOpsTable { - std::array, 4> table; + std::array, 4> columns; void append(const UltraOp& op) { - table[0].push_back(op.op); - table[1].push_back(op.x_lo); - table[2].push_back(op.x_hi); - table[3].push_back(op.y_lo); - - table[0].push_back(0); - table[1].push_back(op.y_hi); - table[2].push_back(op.z_1); - table[3].push_back(op.z_2); + columns[0].push_back(op.op); + columns[1].push_back(op.x_lo); + columns[2].push_back(op.x_hi); + columns[3].push_back(op.y_lo); + + columns[0].push_back(0); + columns[1].push_back(op.y_hi); + columns[2].push_back(op.z_1); + columns[3].push_back(op.z_2); } // Construct the= ultra ops table such that the subtables appear in reverse order, as if prepended @@ -36,7 +36,7 @@ class ECCOpQueueTest : public ::testing::Test { } } - size_t size() const { return table[0].size(); } + size_t size() const { return columns[0].size(); } }; static UltraOp random_ultra_op() @@ -89,31 +89,7 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } -TEST(ECCOpQueueTest, New) -{ - using point = g1::affine_element; - using scalar = fr; - - // Compute a simple point accumulation natively - auto P1 = point::random_element(); - auto P2 = point::random_element(); - auto z = scalar::random_element(); - auto P_expected = P1 + P2 * z; - - // Add the same operations to the ECC op queue; the native computation is performed under the hood. - NewECCOpQueue op_queue; - op_queue.add_accumulate(P1); - op_queue.mul_accumulate(P2, z); - - // The correct result should now be stored in the accumulator within the op queue - EXPECT_EQ(op_queue.get_accumulator(), P_expected); - - // Adding an equality op should reset the accumulator to zero (the point at infinity) - op_queue.eq_and_reset(); - EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); -} - -TEST(ECCOpQueueTest, ConcatenatedTableConstruction) +TEST(ECCOpQueueTest, UltraOpsTableConstruction) { // Construct sets of ultra ops, each representing those added by a single circuit const size_t NUM_SUBTABLES = 3; @@ -129,12 +105,12 @@ TEST(ECCOpQueueTest, ConcatenatedTableConstruction) ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); // Construct the concatenated table internal to the op queue - NewECCOpQueue op_queue; + UltraOpsTable ultra_ops_table; for (const auto& subtable_ops : subtable_ultra_ops) { for (const auto& op : subtable_ops) { - op_queue.append_ultra_op(op); + ultra_ops_table.append_operation(op); } - op_queue.concatenate_ultra_subtable(); + ultra_ops_table.concatenate_subtable(); } // Compute the expected total table size as the sum of the subtable op counts times 2 @@ -144,21 +120,21 @@ TEST(ECCOpQueueTest, ConcatenatedTableConstruction) } // Check that the ultra ops table internal to the op queue has the correct size - EXPECT_EQ(op_queue.ultra_ops_table_size(), expected_table_size); + EXPECT_EQ(ultra_ops_table.size(), expected_table_size); // Define a storage for the genuine ultra ops table and populate using the data stored in the op queue - std::array, 4> ultra_ops_table; - std::array, 4> ultra_ops_table_spans; - for (auto [column_span, column] : zip_view(ultra_ops_table_spans, ultra_ops_table)) { + std::array, 4> ultra_ops_columns; + std::array, 4> ultra_ops_column_spans; + for (auto [column_span, column] : zip_view(ultra_ops_column_spans, ultra_ops_columns)) { column.resize(expected_table_size); column_span = column; } - op_queue.populate_ultra_ops_table(ultra_ops_table_spans); + ultra_ops_table.populate_columns(ultra_ops_column_spans); // ultra_ops_table[2][3] += 1; // Check that the ultra ops table constructed by the op queue matches the expected table - for (auto [expected_column, column] : zip_view(expected_ultra_ops_table.table, ultra_ops_table)) { + for (auto [expected_column, column] : zip_view(expected_ultra_ops_table.columns, ultra_ops_columns)) { for (auto [expected_value, value] : zip_view(expected_column, column)) { EXPECT_EQ(expected_value, value); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index 2c959c1cb5b0..1da215a39864 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -8,40 +8,30 @@ namespace bb { /** - * @brief Used to construct execution trace representations of elliptic curve operations. + * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format + * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each + * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated + * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the + * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point + * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: * - * @details Currently the targets in execution traces are: four advice wires in UltraCircuitBuilder and 5 wires in the - * ECCVM. In each case, the variable values are stored in this class, since the same values will need to be used later - * by the TranslationVMCircuitBuilder. The circuit builders will store witness indices which are indices in the - * ultra (resp. eccvm) ops members of this class (rather than in the builder's variables array). + * OP | X_lo | X_hi | Y_lo + * 0 | Y_hi | z1 | z2 */ -class NewECCOpQueue { +class UltraOpsTable { using Curve = curve::BN254; using Point = Curve::AffineElement; using Fr = Curve::ScalarField; - Point point_at_infinity = Curve::Group::affine_point_at_infinity; static constexpr size_t ULTRA_OPS_TABLE_WIDTH = 4; - // The operations written to the queue are also performed natively; the result is stored in accumulator - Point accumulator = point_at_infinity; + struct Subtable { + std::array, ULTRA_OPS_TABLE_WIDTH> columns; + std::unique_ptr prev; - static constexpr size_t DEFAULT_NON_NATIVE_FIELD_LIMB_BITS = stdlib::NUM_LIMB_BITS_IN_FIELD_SIMULATION; - - std::vector> raw_ops; - std::array, 4> ultra_ops; // ops encoded in the width-4 Ultra format - - // Tracks numer of muls and size of eccvm in real time as the op queue is updated - EccvmRowTracker eccvm_row_tracker; - - struct UltraSubtable { - std::array, 4> columns; - std::unique_ptr prev; - - UltraSubtable(const size_t size_hint = 0) + Subtable(const size_t size_hint = 0) { - // reserve space for the columns - for (auto& column : columns) { + for (auto& column : columns) { // reserve space in each column column.reserve(size_hint); } } @@ -50,266 +40,56 @@ class NewECCOpQueue { /** * @brief Populate two rows of the table, representing a complete ECC operation - * @note Only the first 'op' field is utilized so the second is explicitly set to 0 * */ - void append_operation(const UltraOp& tuple) + void append_operation(const UltraOp& op) { - columns[0].emplace_back(tuple.op); - columns[1].emplace_back(tuple.x_lo); - columns[2].emplace_back(tuple.x_hi); - columns[3].emplace_back(tuple.y_lo); - - columns[0].emplace_back(0); - columns[1].emplace_back(tuple.y_hi); - columns[2].emplace_back(tuple.z_1); - columns[3].emplace_back(tuple.z_2); + columns[0].emplace_back(op.op); + columns[1].emplace_back(op.x_lo); + columns[2].emplace_back(op.x_hi); + columns[3].emplace_back(op.y_lo); + + columns[0].emplace_back(0); // only the first 'op' field is utilized + columns[1].emplace_back(op.y_hi); + columns[2].emplace_back(op.z_1); + columns[3].emplace_back(op.z_2); } }; - std::unique_ptr current_ultra_subtable = std::make_unique(); + std::unique_ptr current_subtable = std::make_unique(); public: - using ECCVMOperation = bb::eccvm::VMOperation; - - void append_ultra_op(const UltraOp& op) { current_ultra_subtable->append_operation(op); } + void append_operation(const UltraOp& op) { current_subtable->append_operation(op); } - size_t ultra_ops_table_size() const + size_t size() const { size_t total_size = 0; - for (auto* subtable = current_ultra_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { + for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { total_size += subtable->size(); } return total_size; } - void concatenate_ultra_subtable(const size_t size_hint = 0) + void concatenate_subtable(const size_t size_hint = 0) { - auto new_ultra_subtable = std::make_unique(size_hint); // new empty subtable - new_ultra_subtable->prev = std::move(current_ultra_subtable); // link new subtable to current subtable - current_ultra_subtable = std::move(new_ultra_subtable); // update current subtable + auto new_subtable = std::make_unique(size_hint); // new empty subtable + new_subtable->prev = std::move(current_subtable); // link new subtable to current subtable + current_subtable = std::move(new_subtable); // update current subtable } - void populate_ultra_ops_table(std::array, ULTRA_OPS_TABLE_WIDTH>& ultra_ops_table) + void populate_columns(std::array, ULTRA_OPS_TABLE_WIDTH>& target_columns) { size_t i = 0; - for (auto* subtable = current_ultra_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { + for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { for (size_t j = 0; j < subtable->size(); ++j) { - ultra_ops_table[0][i] = subtable->columns[0][j]; - ultra_ops_table[1][i] = subtable->columns[1][j]; - ultra_ops_table[2][i] = subtable->columns[2][j]; - ultra_ops_table[3][i] = subtable->columns[3][j]; + target_columns[0][i] = subtable->columns[0][j]; + target_columns[1][i] = subtable->columns[1][j]; + target_columns[2][i] = subtable->columns[2][j]; + target_columns[3][i] = subtable->columns[3][j]; ++i; } } } - - const std::vector& get_raw_ops() { return raw_ops; } - - /** - * @brief Get the number of rows in the 'msm' column section, for all msms in the circuit - */ - size_t get_num_msm_rows() const { return eccvm_row_tracker.get_num_msm_rows(); } - - /** - * @brief Get the number of rows for the current ECCVM circuit - */ - size_t get_num_rows() const { return eccvm_row_tracker.get_num_rows(); } - - /** - * @brief get number of muls for the current ECCVM circuit - */ - uint32_t get_number_of_muls() const { return eccvm_row_tracker.get_number_of_muls(); } - - // TODO(https://github.com/AztecProtocol/barretenberg/issues/905): Can remove this with better handling of scalar - // mul against 0 - void append_nonzero_ops() - { - // Add an element and scalar the accumulation of which leaves no Point-at-Infinity commitments - const auto x = uint256_t(0xd3c208c16d87cfd3, 0xd97816a916871ca8, 0x9b85045b68181585, 0x30644e72e131a02); - const auto y = uint256_t(0x3ce1cc9c7e645a83, 0x2edac647851e3ac5, 0xd0cbe61fced2bc53, 0x1a76dae6d3272396); - auto padding_element = Point(x, y); - auto padding_scalar = -Fr::one(); - mul_accumulate(padding_element, padding_scalar); - eq_and_reset(); - } - - /** - * @brief A fuzzing only method for setting raw ops directly - * - */ - void set_raw_ops_for_fuzzing(std::vector& raw_ops_in) { raw_ops = raw_ops_in; } - - /** - * @brief A testing only method that adds an erroneous equality op to the raw ops - * @brief May be used to ensure that ECCVM responds as expected when encountering a bad op - * - */ - void add_erroneous_equality_op_for_testing() - { - raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = Point::random_element() }); - } - - /** - * @brief Write empty row to queue - * @warning This is for testing purposes only. Currently no valid use case. - * - */ - void empty_row_for_testing() - { - raw_ops.emplace_back(ECCVMOperation{ .base_point = point_at_infinity }); - - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - } - - Point get_accumulator() { return accumulator; } - - /** - * @brief Write point addition op to queue and natively perform addition - * - * @param to_add - */ - UltraOp add_accumulate(const Point& to_add) - { - // Update the accumulator natively - accumulator = accumulator + to_add; - - // Store the raw operation - raw_ops.emplace_back(ECCVMOperation{ .add = true, .base_point = to_add }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - - // Construct and store the operation in the ultra op format - return construct_and_populate_ultra_ops(ADD_ACCUM, to_add); - } - - /** - * @brief Write multiply and add op to queue and natively perform operation - * - * @param to_add - */ - UltraOp mul_accumulate(const Point& to_mul, const Fr& scalar) - { - // Update the accumulator natively - accumulator = accumulator + to_mul * scalar; - - // Construct and store the operation in the ultra op format - UltraOp ultra_op = construct_and_populate_ultra_ops(MUL_ACCUM, to_mul, scalar); - - // Store the raw operation - raw_ops.emplace_back(ECCVMOperation{ - .mul = true, - .base_point = to_mul, - .z1 = ultra_op.z_1, - .z2 = ultra_op.z_2, - .mul_scalar_full = scalar, - }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - - return ultra_op; - } - - /** - * @brief Write no op (i.e. empty row) - * - */ - UltraOp no_op() - { - // Store raw operation - raw_ops.emplace_back(ECCVMOperation{}); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - - // Construct and store the operation in the ultra op format - return construct_and_populate_ultra_ops(NULL_OP, accumulator); - } - - /** - * @brief Write equality op using internal accumulator point - * - * @return current internal accumulator point (prior to reset to 0) - */ - UltraOp eq_and_reset() - { - auto expected = accumulator; - accumulator.self_set_infinity(); - - // Store raw operation - raw_ops.emplace_back(ECCVMOperation{ .eq = true, .reset = true, .base_point = expected }); - eccvm_row_tracker.update_cached_msms(raw_ops.back()); - - // Construct and store the operation in the ultra op format - return construct_and_populate_ultra_ops(EQUALITY, expected); - } - - private: - /** - * @brief Given an ecc operation and its inputs, decompose into ultra format and populate ultra_ops - * - * @param op_code - * @param point - * @param scalar - * @return UltraOp - */ - UltraOp construct_and_populate_ultra_ops(EccOpCode op_code, const Point& point, const Fr& scalar = Fr::zero()) - { - UltraOp ultra_op; - ultra_op.op_code = op_code; - ultra_op.op = Fr(static_cast(op_code)); - - // Decompose point coordinates (Fq) into hi-lo chunks (Fr) - const size_t CHUNK_SIZE = 2 * DEFAULT_NON_NATIVE_FIELD_LIMB_BITS; - uint256_t x_256(point.x); - uint256_t y_256(point.y); - ultra_op.return_is_infinity = point.is_point_at_infinity(); - // if we have a point at infinity, set x/y to zero - // in the biggroup_goblin class we use `assert_equal` statements to validate - // the original in-circuit coordinate values are also zero - if (point.is_point_at_infinity()) { - x_256 = 0; - y_256 = 0; - } - ultra_op.x_lo = Fr(x_256.slice(0, CHUNK_SIZE)); - ultra_op.x_hi = Fr(x_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2)); - ultra_op.y_lo = Fr(y_256.slice(0, CHUNK_SIZE)); - ultra_op.y_hi = Fr(y_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2)); - - // Split scalar into 128 bit endomorphism scalars - Fr z_1 = 0; - Fr z_2 = 0; - auto converted = scalar.from_montgomery_form(); - uint256_t converted_u256(scalar); - if (converted_u256.get_msb() <= 128) { - ultra_op.z_1 = scalar; - ultra_op.z_2 = 0; - } else { - Fr::split_into_endomorphism_scalars(converted, z_1, z_2); - ultra_op.z_1 = z_1.to_montgomery_form(); - ultra_op.z_2 = z_2.to_montgomery_form(); - } - - append_to_ultra_ops(ultra_op); - current_ultra_subtable->append_operation(ultra_op); - - return ultra_op; - } - - /** - * @brief Populate two rows of the ultra ops,representing a complete ECC operation - * @note Only the first 'op' field is utilized so the second is explicitly set to 0 - * - * @param tuple - */ - void append_to_ultra_ops(UltraOp tuple) - { - ultra_ops[0].emplace_back(tuple.op); - ultra_ops[1].emplace_back(tuple.x_lo); - ultra_ops[2].emplace_back(tuple.x_hi); - ultra_ops[3].emplace_back(tuple.y_lo); - - ultra_ops[0].emplace_back(0); - ultra_ops[1].emplace_back(tuple.y_hi); - ultra_ops[2].emplace_back(tuple.z_1); - ultra_ops[3].emplace_back(tuple.z_2); - } }; } // namespace bb From af7432184006f9b82ddcf5d546c9098dcde24c6a Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 17:23:38 +0000 Subject: [PATCH 05/16] add iterator; range based loop works --- .../op_queue/ecc_op_queue.test.cpp | 12 ++++ .../op_queue/new_ecc_op_queue.hpp | 57 ++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index c6905dba01f5..ce2d0bb0d162 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -91,6 +91,8 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) TEST(ECCOpQueueTest, UltraOpsTableConstruction) { + using Fr = fr; + // Construct sets of ultra ops, each representing those added by a single circuit const size_t NUM_SUBTABLES = 3; std::array, NUM_SUBTABLES> subtable_ultra_ops; @@ -139,4 +141,14 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) EXPECT_EQ(expected_value, value); } } + + // Check that the ultra ops table constructed by the op queue matches the expected table using row iterator + std::array, 4> columns; + for (const auto& row : ultra_ops_table) { + for (size_t i = 0; i < 4; ++i) { + columns[i].push_back(row[i]); + } + } + + EXPECT_EQ(expected_ultra_ops_table.columns, columns); } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index 1da215a39864..a00ef353c170 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -1,5 +1,7 @@ #pragma once +#include "barretenberg/common/ref_array.hpp" +#include "barretenberg/common/zip_view.hpp" #include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/eccvm/eccvm_builder_types.hpp" #include "barretenberg/stdlib/primitives/bigfield/constants.hpp" @@ -23,10 +25,10 @@ class UltraOpsTable { using Point = Curve::AffineElement; using Fr = Curve::ScalarField; - static constexpr size_t ULTRA_OPS_TABLE_WIDTH = 4; + static constexpr size_t TABLE_WIDTH = 4; struct Subtable { - std::array, ULTRA_OPS_TABLE_WIDTH> columns; + std::array, TABLE_WIDTH> columns; std::unique_ptr prev; Subtable(const size_t size_hint = 0) @@ -77,7 +79,7 @@ class UltraOpsTable { current_subtable = std::move(new_subtable); // update current subtable } - void populate_columns(std::array, ULTRA_OPS_TABLE_WIDTH>& target_columns) + void populate_columns(std::array, TABLE_WIDTH>& target_columns) { size_t i = 0; for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { @@ -90,6 +92,55 @@ class UltraOpsTable { } } } + + // Enable range based iteration over the rows of the full table + class Iterator { + Subtable* subtable = nullptr; + size_t index = 0; + size_t subtable_size = 0; + + using Row = RefArray; + + public: + Iterator() = default; + Iterator(Subtable* sub, size_t idx) + : subtable(sub) + , index(idx) + , subtable_size(sub != nullptr ? sub->size() : 0) + {} + + Row operator*() + { + std::array row; + for (size_t i = 0; i < TABLE_WIDTH; ++i) { + row[i] = &subtable->columns[i][index]; + } + return row; + } + + Iterator& operator++() + { + if (++index < subtable_size) { // return row within the current subtable + return *this; + } + if (subtable->prev) { // update to next subtable + subtable = subtable->prev.get(); + subtable_size = subtable->size(); + index = 0; + } else { + *this = Iterator(); // end of iteration + } + return *this; + } + + bool operator!=(const Iterator& other) const { return subtable != other.subtable || index != other.index; } + }; + + // begin() and end() to enable range based iteration over the full table + // WORKTODO: need to skip over the first empty subtable created from the last call to concatenate_subtable(). Might + // be nicer to have the user instead call create_subtable(). + Iterator begin() { return { current_subtable->prev.get(), 0 }; } + static Iterator end() { return {}; } }; } // namespace bb From 272ad024b44ecfc04f15cdfc6d8db83a9a98a6d0 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 18:55:23 +0000 Subject: [PATCH 06/16] implement the ultra ops table via inheritance --- .../op_queue/ecc_op_queue.test.cpp | 4 +- .../op_queue/new_ecc_op_queue.hpp | 132 ++++++++++-------- 2 files changed, 73 insertions(+), 63 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index ce2d0bb0d162..f670b5ce29f7 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -112,7 +112,7 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) for (const auto& op : subtable_ops) { ultra_ops_table.append_operation(op); } - ultra_ops_table.concatenate_subtable(); + ultra_ops_table.create_new_subtable(); } // Compute the expected total table size as the sum of the subtable op counts times 2 @@ -133,7 +133,7 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) } ultra_ops_table.populate_columns(ultra_ops_column_spans); - // ultra_ops_table[2][3] += 1; + // expected_ultra_ops_table.columns[2][3] += 1; // Check that the ultra ops table constructed by the op queue matches the expected table for (auto [expected_column, column] : zip_view(expected_ultra_ops_table.columns, ultra_ops_columns)) { diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index a00ef353c170..baa14b5de6ff 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -10,58 +10,35 @@ namespace bb { /** - * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format - * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each - * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated - * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the - * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point - * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: + * @brief * - * OP | X_lo | X_hi | Y_lo - * 0 | Y_hi | z1 | z2 + * @tparam Derived CRTP (curiously recurring template pattern) implementation class + * @tparam TABLE_WIDTH + * @tparam OpFormat */ -class UltraOpsTable { +template class OpsTable { using Curve = curve::BN254; - using Point = Curve::AffineElement; using Fr = Curve::ScalarField; - static constexpr size_t TABLE_WIDTH = 4; - + protected: struct Subtable { - std::array, TABLE_WIDTH> columns; + std::array, TABLE_WIDTH> columns; std::unique_ptr prev; - Subtable(const size_t size_hint = 0) + Subtable(size_t size_hint = 0) { - for (auto& column : columns) { // reserve space in each column + for (auto& column : columns) { column.reserve(size_hint); } } size_t size() const { return columns[0].size(); } - - /** - * @brief Populate two rows of the table, representing a complete ECC operation - * - */ - void append_operation(const UltraOp& op) - { - columns[0].emplace_back(op.op); - columns[1].emplace_back(op.x_lo); - columns[2].emplace_back(op.x_hi); - columns[3].emplace_back(op.y_lo); - - columns[0].emplace_back(0); // only the first 'op' field is utilized - columns[1].emplace_back(op.y_hi); - columns[2].emplace_back(op.z_1); - columns[3].emplace_back(op.z_2); - } }; std::unique_ptr current_subtable = std::make_unique(); public: - void append_operation(const UltraOp& op) { current_subtable->append_operation(op); } + using Row = RefArray; size_t size() const { @@ -72,41 +49,17 @@ class UltraOpsTable { return total_size; } - void concatenate_subtable(const size_t size_hint = 0) - { - auto new_subtable = std::make_unique(size_hint); // new empty subtable - new_subtable->prev = std::move(current_subtable); // link new subtable to current subtable - current_subtable = std::move(new_subtable); // update current subtable - } - - void populate_columns(std::array, TABLE_WIDTH>& target_columns) - { - size_t i = 0; - for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { - for (size_t j = 0; j < subtable->size(); ++j) { - target_columns[0][i] = subtable->columns[0][j]; - target_columns[1][i] = subtable->columns[1][j]; - target_columns[2][i] = subtable->columns[2][j]; - target_columns[3][i] = subtable->columns[3][j]; - ++i; - } - } - } - - // Enable range based iteration over the rows of the full table class Iterator { Subtable* subtable = nullptr; size_t index = 0; size_t subtable_size = 0; - using Row = RefArray; - public: Iterator() = default; Iterator(Subtable* sub, size_t idx) : subtable(sub) , index(idx) - , subtable_size(sub != nullptr ? sub->size() : 0) + , subtable_size(sub ? sub->size() : 0) {} Row operator*() @@ -138,9 +91,66 @@ class UltraOpsTable { // begin() and end() to enable range based iteration over the full table // WORKTODO: need to skip over the first empty subtable created from the last call to concatenate_subtable(). Might - // be nicer to have the user instead call create_subtable(). - Iterator begin() { return { current_subtable->prev.get(), 0 }; } - static Iterator end() { return {}; } + // be nicer to have the user instead call create_subtable() prior to construction rather than at the end to mark + // completion. + Iterator begin() { return Iterator(current_subtable->prev.get(), 0); } + Iterator end() { return {}; } + + void create_new_subtable(size_t size_hint = 0) + { + auto new_subtable = std::make_unique(size_hint); + new_subtable->prev = std::move(current_subtable); + current_subtable = std::move(new_subtable); + } + + void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } +}; + +/** + * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format + * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each + * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated + * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the + * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point + * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: + * + * OP | X_lo | X_hi | Y_lo + * 0 | Y_hi | z1 | z2 + */ +class UltraOpsTable : public OpsTable { + using Curve = curve::BN254; + using Point = Curve::AffineElement; + using Fr = Curve::ScalarField; + + static constexpr size_t TABLE_WIDTH = 4; + + public: + void append_operation_impl(const UltraOp& op) + { + current_subtable->columns[0].emplace_back(op.op); + current_subtable->columns[1].emplace_back(op.x_lo); + current_subtable->columns[2].emplace_back(op.x_hi); + current_subtable->columns[3].emplace_back(op.y_lo); + + current_subtable->columns[0].emplace_back(0); // only the first 'op' field is utilized + current_subtable->columns[1].emplace_back(op.y_hi); + current_subtable->columns[2].emplace_back(op.z_1); + current_subtable->columns[3].emplace_back(op.z_2); + } + + void populate_columns(std::array, TABLE_WIDTH>& target_columns) + { + size_t i = 0; + for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { + for (size_t j = 0; j < subtable->size(); ++j) { + target_columns[0][i] = subtable->columns[0][j]; + target_columns[1][i] = subtable->columns[1][j]; + target_columns[2][i] = subtable->columns[2][j]; + target_columns[3][i] = subtable->columns[3][j]; + ++i; + } + } + } }; } // namespace bb From 28be4723d6d7b012541ca7ace31bc940c7868eec Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 19:44:42 +0000 Subject: [PATCH 07/16] test updates --- .../op_queue/ecc_op_queue.test.cpp | 37 ++++++++++--------- .../op_queue/new_ecc_op_queue.hpp | 1 - 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index f670b5ce29f7..53f4e3f08f1b 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -1,5 +1,6 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" #include "barretenberg/common/zip_view.hpp" +#include "barretenberg/polynomials/polynomial.hpp" #include "barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp" #include @@ -93,6 +94,8 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) { using Fr = fr; + constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table + // Construct sets of ultra ops, each representing those added by a single circuit const size_t NUM_SUBTABLES = 3; std::array, NUM_SUBTABLES> subtable_ultra_ops; @@ -118,30 +121,14 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) // Compute the expected total table size as the sum of the subtable op counts times 2 size_t expected_table_size = 0; for (const auto& count : subtable_op_counts) { - expected_table_size += count * 2; + expected_table_size += count * NUM_ROWS_PER_OP; } // Check that the ultra ops table internal to the op queue has the correct size EXPECT_EQ(ultra_ops_table.size(), expected_table_size); - // Define a storage for the genuine ultra ops table and populate using the data stored in the op queue - std::array, 4> ultra_ops_columns; - std::array, 4> ultra_ops_column_spans; - for (auto [column_span, column] : zip_view(ultra_ops_column_spans, ultra_ops_columns)) { - column.resize(expected_table_size); - column_span = column; - } - ultra_ops_table.populate_columns(ultra_ops_column_spans); - // expected_ultra_ops_table.columns[2][3] += 1; - // Check that the ultra ops table constructed by the op queue matches the expected table - for (auto [expected_column, column] : zip_view(expected_ultra_ops_table.columns, ultra_ops_columns)) { - for (auto [expected_value, value] : zip_view(expected_column, column)) { - EXPECT_EQ(expected_value, value); - } - } - // Check that the ultra ops table constructed by the op queue matches the expected table using row iterator std::array, 4> columns; for (const auto& row : ultra_ops_table) { @@ -151,4 +138,20 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) } EXPECT_EQ(expected_ultra_ops_table.columns, columns); + + // Construct polynomials corresponding to the columns of the ultra ops table + std::array, 4> ultra_ops_table_polynomials; + std::array, 4> column_spans; + for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { + column = Polynomial(expected_table_size); + column_span = column.coeffs(); + } + ultra_ops_table.populate_columns(column_spans); + + // Check that the ultra ops table constructed by the op queue matches the expected table + for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { + for (auto [expected_value, value] : zip_view(expected_column, poly.coeffs())) { + EXPECT_EQ(expected_value, value); + } + } } diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index baa14b5de6ff..d7d4530c2e0d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -119,7 +119,6 @@ template class OpsTabl */ class UltraOpsTable : public OpsTable { using Curve = curve::BN254; - using Point = Curve::AffineElement; using Fr = Curve::ScalarField; static constexpr size_t TABLE_WIDTH = 4; From e671a429f3e630a04f120cb73b8783b4bdd192d8 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 19:48:52 +0000 Subject: [PATCH 08/16] change model so subtable must be created rather than completed --- .../op_queue/ecc_op_queue.test.cpp | 2 +- .../op_queue/new_ecc_op_queue.hpp | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 53f4e3f08f1b..79518cf3fe1c 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -112,10 +112,10 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) // Construct the concatenated table internal to the op queue UltraOpsTable ultra_ops_table; for (const auto& subtable_ops : subtable_ultra_ops) { + ultra_ops_table.create_new_subtable(); for (const auto& op : subtable_ops) { ultra_ops_table.append_operation(op); } - ultra_ops_table.create_new_subtable(); } // Compute the expected total table size as the sum of the subtable op counts times 2 diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp index d7d4530c2e0d..0ff560fa9d8a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp @@ -12,7 +12,7 @@ namespace bb { /** * @brief * - * @tparam Derived CRTP (curiously recurring template pattern) implementation class + * @tparam Derived child class type for CRTP (curiously recurring template pattern) implementation class * @tparam TABLE_WIDTH * @tparam OpFormat */ @@ -35,7 +35,7 @@ template class OpsTabl size_t size() const { return columns[0].size(); } }; - std::unique_ptr current_subtable = std::make_unique(); + std::unique_ptr current_subtable = nullptr; public: using Row = RefArray; @@ -90,10 +90,7 @@ template class OpsTabl }; // begin() and end() to enable range based iteration over the full table - // WORKTODO: need to skip over the first empty subtable created from the last call to concatenate_subtable(). Might - // be nicer to have the user instead call create_subtable() prior to construction rather than at the end to mark - // completion. - Iterator begin() { return Iterator(current_subtable->prev.get(), 0); } + Iterator begin() { return Iterator(current_subtable.get(), 0); } Iterator end() { return {}; } void create_new_subtable(size_t size_hint = 0) From 672e831bc57287807fbce250a48d76e5d1cd4299 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 20:05:56 +0000 Subject: [PATCH 09/16] name changefile --- .../op_queue/ecc_op_queue.test.cpp | 2 +- ...ew_ecc_op_queue.hpp => ultra_ops_table.hpp} | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) rename barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/{new_ecc_op_queue.hpp => ultra_ops_table.hpp} (87%) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 79518cf3fe1c..f88c84870750 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -1,7 +1,7 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" #include "barretenberg/common/zip_view.hpp" #include "barretenberg/polynomials/polynomial.hpp" -#include "barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp" #include #include diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp similarity index 87% rename from barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp rename to barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp index 0ff560fa9d8a..036520ee3e1d 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/new_ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp @@ -149,4 +149,22 @@ class UltraOpsTable : public OpsTable { } }; +// class RawOpsTable : public OpsTable> { +// using Curve = curve::BN254; +// using Fr = Curve::ScalarField; +// using ECCVMOperation = eccvm::VMOperation; + +// static constexpr size_t TABLE_WIDTH = 5; + +// public: +// void append_operation_impl(const ECCVMOperation& op) +// { +// current_subtable->columns[0].emplace_back(op.get_opcode_value()); +// current_subtable->columns[1].emplace_back(op.z1); +// current_subtable->columns[2].emplace_back(op.z2); +// current_subtable->columns[3].emplace_back(op.mul_scalar_full); +// current_subtable->columns[4].emplace_back(op.base_point.x); +// } +// } + } // namespace bb From 7cc7295865f0d6090ae04df4e98c6b433ba10d3b Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 20:37:09 +0000 Subject: [PATCH 10/16] simpler table concept --- .../op_queue/ecc_op_queue.test.cpp | 98 +++++-- .../op_queue/ultra_ops_table.hpp | 242 ++++++++++++------ 2 files changed, 238 insertions(+), 102 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index f88c84870750..339e2cc92a7a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -90,7 +90,73 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } -TEST(ECCOpQueueTest, UltraOpsTableConstruction) +// TEST(ECCOpQueueTest, UltraOpsTableConstruction) +// { +// using Fr = fr; + +// constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table + +// // Construct sets of ultra ops, each representing those added by a single circuit +// const size_t NUM_SUBTABLES = 3; +// std::array, NUM_SUBTABLES> subtable_ultra_ops; +// std::array subtable_op_counts = { 4, 2, 7 }; +// for (auto [subtable_ops, op_count] : zip_view(subtable_ultra_ops, subtable_op_counts)) { +// for (size_t i = 0; i < op_count; ++i) { +// subtable_ops.push_back(ECCOpQueueTest::random_ultra_op()); +// } +// } + +// // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) +// ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); + +// // Construct the concatenated table internal to the op queue +// UltraOpsTable ultra_ops_table; +// for (const auto& subtable_ops : subtable_ultra_ops) { +// ultra_ops_table.create_new_subtable(); +// for (const auto& op : subtable_ops) { +// ultra_ops_table.append_operation(op); +// } +// } + +// // Compute the expected total table size as the sum of the subtable op counts times 2 +// size_t expected_table_size = 0; +// for (const auto& count : subtable_op_counts) { +// expected_table_size += count * NUM_ROWS_PER_OP; +// } + +// // Check that the ultra ops table internal to the op queue has the correct size +// EXPECT_EQ(ultra_ops_table.size(), expected_table_size); + +// // expected_ultra_ops_table.columns[2][3] += 1; + +// // Check that the ultra ops table constructed by the op queue matches the expected table using row iterator +// std::array, 4> columns; +// for (const auto& row : ultra_ops_table) { +// for (size_t i = 0; i < 4; ++i) { +// columns[i].push_back(row[i]); +// } +// } + +// EXPECT_EQ(expected_ultra_ops_table.columns, columns); + +// // Construct polynomials corresponding to the columns of the ultra ops table +// std::array, 4> ultra_ops_table_polynomials; +// std::array, 4> column_spans; +// for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { +// column = Polynomial(expected_table_size); +// column_span = column.coeffs(); +// } +// ultra_ops_table.populate_columns(column_spans); + +// // Check that the ultra ops table constructed by the op queue matches the expected table +// for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { +// for (auto [expected_value, value] : zip_view(expected_column, poly.coeffs())) { +// EXPECT_EQ(expected_value, value); +// } +// } +// } + +TEST(ECCOpQueueTest, NewUltraOpsTableConstruction) { using Fr = fr; @@ -110,43 +176,29 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); // Construct the concatenated table internal to the op queue - UltraOpsTable ultra_ops_table; + UltraEccOpsTable ultra_ops_table; for (const auto& subtable_ops : subtable_ultra_ops) { ultra_ops_table.create_new_subtable(); for (const auto& op : subtable_ops) { - ultra_ops_table.append_operation(op); + ultra_ops_table.push(op); } } - // Compute the expected total table size as the sum of the subtable op counts times 2 - size_t expected_table_size = 0; - for (const auto& count : subtable_op_counts) { - expected_table_size += count * NUM_ROWS_PER_OP; - } - // Check that the ultra ops table internal to the op queue has the correct size - EXPECT_EQ(ultra_ops_table.size(), expected_table_size); + auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); + EXPECT_EQ(ultra_ops_table.size(), expected_num_ops); - // expected_ultra_ops_table.columns[2][3] += 1; - - // Check that the ultra ops table constructed by the op queue matches the expected table using row iterator - std::array, 4> columns; - for (const auto& row : ultra_ops_table) { - for (size_t i = 0; i < 4; ++i) { - columns[i].push_back(row[i]); - } - } - - EXPECT_EQ(expected_ultra_ops_table.columns, columns); + // // expected_ultra_ops_table.columns[2][3] += 1; // Construct polynomials corresponding to the columns of the ultra ops table + const size_t expected_table_size = expected_num_ops * NUM_ROWS_PER_OP; std::array, 4> ultra_ops_table_polynomials; std::array, 4> column_spans; for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { column = Polynomial(expected_table_size); column_span = column.coeffs(); } - ultra_ops_table.populate_columns(column_spans); + ultra_ops_table.populate_column_data(column_spans); // Check that the ultra ops table constructed by the op queue matches the expected table for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { @@ -154,4 +206,4 @@ TEST(ECCOpQueueTest, UltraOpsTableConstruction) EXPECT_EQ(expected_value, value); } } -} +} \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp index 036520ee3e1d..03fa646de860 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp @@ -9,37 +9,164 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/eccvm_row_tracker.hpp" namespace bb { -/** - * @brief - * - * @tparam Derived child class type for CRTP (curiously recurring template pattern) implementation class - * @tparam TABLE_WIDTH - * @tparam OpFormat - */ -template class OpsTable { +// /** +// * @brief +// * +// * @tparam Derived child class type for CRTP (curiously recurring template pattern) implementation class +// * @tparam TABLE_WIDTH +// * @tparam OpFormat +// */ +// template class OpsTable { +// using Curve = curve::BN254; +// using Fr = Curve::ScalarField; + +// protected: +// struct Subtable { +// std::array, TABLE_WIDTH> columns; +// std::unique_ptr prev; + +// Subtable(size_t size_hint = 0) +// { +// for (auto& column : columns) { +// column.reserve(size_hint); +// } +// } + +// size_t size() const { return columns[0].size(); } +// }; + +// std::unique_ptr current_subtable = nullptr; + +// public: +// using Row = RefArray; + +// size_t size() const +// { +// size_t total_size = 0; +// for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { +// total_size += subtable->size(); +// } +// return total_size; +// } + +// class Iterator { +// Subtable* subtable = nullptr; +// size_t index = 0; +// size_t subtable_size = 0; + +// public: +// Iterator() = default; +// Iterator(Subtable* sub, size_t idx) +// : subtable(sub) +// , index(idx) +// , subtable_size(sub ? sub->size() : 0) +// {} + +// Row operator*() +// { +// std::array row; +// for (size_t i = 0; i < TABLE_WIDTH; ++i) { +// row[i] = &subtable->columns[i][index]; +// } +// return row; +// } + +// Iterator& operator++() +// { +// if (++index < subtable_size) { // return row within the current subtable +// return *this; +// } +// if (subtable->prev) { // update to next subtable +// subtable = subtable->prev.get(); +// subtable_size = subtable->size(); +// index = 0; +// } else { +// *this = Iterator(); // end of iteration +// } +// return *this; +// } + +// bool operator!=(const Iterator& other) const { return subtable != other.subtable || index != other.index; } +// }; + +// // begin() and end() to enable range based iteration over the full table +// Iterator begin() { return Iterator(current_subtable.get(), 0); } +// Iterator end() { return {}; } + +// void create_new_subtable(size_t size_hint = 0) +// { +// auto new_subtable = std::make_unique(size_hint); +// new_subtable->prev = std::move(current_subtable); +// current_subtable = std::move(new_subtable); +// } + +// void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } +// }; + +// /** +// * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format +// * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each +// * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations +// associated +// * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the +// * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point +// * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: +// * +// * OP | X_lo | X_hi | Y_lo +// * 0 | Y_hi | z1 | z2 +// */ +// class UltraOpsTable : public OpsTable { +// using Curve = curve::BN254; +// using Fr = Curve::ScalarField; + +// static constexpr size_t TABLE_WIDTH = 4; + +// public: +// void append_operation_impl(const UltraOp& op) +// { +// current_subtable->columns[0].emplace_back(op.op); +// current_subtable->columns[1].emplace_back(op.x_lo); +// current_subtable->columns[2].emplace_back(op.x_hi); +// current_subtable->columns[3].emplace_back(op.y_lo); + +// current_subtable->columns[0].emplace_back(0); // only the first 'op' field is utilized +// current_subtable->columns[1].emplace_back(op.y_hi); +// current_subtable->columns[2].emplace_back(op.z_1); +// current_subtable->columns[3].emplace_back(op.z_2); +// } + +// void populate_columns(std::array, TABLE_WIDTH>& target_columns) +// { +// size_t i = 0; +// for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { +// for (size_t j = 0; j < subtable->size(); ++j) { +// target_columns[0][i] = subtable->columns[0][j]; +// target_columns[1][i] = subtable->columns[1][j]; +// target_columns[2][i] = subtable->columns[2][j]; +// target_columns[3][i] = subtable->columns[3][j]; +// ++i; +// } +// } +// } +// }; + +template class EccOpsTable { using Curve = curve::BN254; using Fr = Curve::ScalarField; protected: struct Subtable { - std::array, TABLE_WIDTH> columns; + std::vector data; std::unique_ptr prev; - Subtable(size_t size_hint = 0) - { - for (auto& column : columns) { - column.reserve(size_hint); - } - } + Subtable(size_t size_hint = 0) { data.reserve(size_hint); } - size_t size() const { return columns[0].size(); } + size_t size() const { return data.size(); } }; std::unique_ptr current_subtable = nullptr; public: - using Row = RefArray; - size_t size() const { size_t total_size = 0; @@ -62,14 +189,7 @@ template class OpsTabl , subtable_size(sub ? sub->size() : 0) {} - Row operator*() - { - std::array row; - for (size_t i = 0; i < TABLE_WIDTH; ++i) { - row[i] = &subtable->columns[i][index]; - } - return row; - } + OpFormat operator*() { return subtable->data[index]; } Iterator& operator++() { @@ -100,71 +220,35 @@ template class OpsTabl current_subtable = std::move(new_subtable); } - void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } + // void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } + void push(const OpFormat& op) { current_subtable->data.push_back(op); } }; -/** - * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format - * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each - * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated - * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the - * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point - * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: - * - * OP | X_lo | X_hi | Y_lo - * 0 | Y_hi | z1 | z2 - */ -class UltraOpsTable : public OpsTable { +class UltraEccOpsTable : public EccOpsTable { + static constexpr size_t TABLE_WIDTH = 4; using Curve = curve::BN254; using Fr = Curve::ScalarField; - static constexpr size_t TABLE_WIDTH = 4; - public: - void append_operation_impl(const UltraOp& op) - { - current_subtable->columns[0].emplace_back(op.op); - current_subtable->columns[1].emplace_back(op.x_lo); - current_subtable->columns[2].emplace_back(op.x_hi); - current_subtable->columns[3].emplace_back(op.y_lo); - - current_subtable->columns[0].emplace_back(0); // only the first 'op' field is utilized - current_subtable->columns[1].emplace_back(op.y_hi); - current_subtable->columns[2].emplace_back(op.z_1); - current_subtable->columns[3].emplace_back(op.z_2); - } - - void populate_columns(std::array, TABLE_WIDTH>& target_columns) + // WORKTODO: multithreaded version of this function + void populate_column_data(std::array, TABLE_WIDTH>& target_columns) { size_t i = 0; for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { - for (size_t j = 0; j < subtable->size(); ++j) { - target_columns[0][i] = subtable->columns[0][j]; - target_columns[1][i] = subtable->columns[1][j]; - target_columns[2][i] = subtable->columns[2][j]; - target_columns[3][i] = subtable->columns[3][j]; - ++i; + for (const auto& op : subtable->data) { + target_columns[0][i] = op.op; + target_columns[1][i] = op.x_lo; + target_columns[2][i] = op.x_hi; + target_columns[3][i] = op.y_lo; + i++; + target_columns[0][i] = 0; // only the first 'op' field is utilized + target_columns[1][i] = op.y_hi; + target_columns[2][i] = op.z_1; + target_columns[3][i] = op.z_2; + i++; } } } }; -// class RawOpsTable : public OpsTable> { -// using Curve = curve::BN254; -// using Fr = Curve::ScalarField; -// using ECCVMOperation = eccvm::VMOperation; - -// static constexpr size_t TABLE_WIDTH = 5; - -// public: -// void append_operation_impl(const ECCVMOperation& op) -// { -// current_subtable->columns[0].emplace_back(op.get_opcode_value()); -// current_subtable->columns[1].emplace_back(op.z1); -// current_subtable->columns[2].emplace_back(op.z2); -// current_subtable->columns[3].emplace_back(op.mul_scalar_full); -// current_subtable->columns[4].emplace_back(op.base_point.x); -// } -// } - } // namespace bb From 9d818c42001010e1589866b806e345c642259361 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 21:13:52 +0000 Subject: [PATCH 11/16] a simpler inheritence structure for both ultra and raw tables --- .../op_queue/ecc_op_queue.test.cpp | 151 ++++++++-------- .../op_queue/ultra_ops_table.hpp | 167 ++---------------- 2 files changed, 93 insertions(+), 225 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 339e2cc92a7a..fd189f61c9a0 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -9,11 +9,14 @@ using namespace bb; class ECCOpQueueTest : public ::testing::Test { - using scalar = fr; + using Curve = curve::BN254; + using Point = Curve::AffineElement; + using Scalar = fr; + using ECCVMOperation = bb::eccvm::VMOperation; public: struct MockUltraOpsTable { - std::array, 4> columns; + std::array, 4> columns; void append(const UltraOp& op) { columns[0].push_back(op.op); @@ -44,16 +47,39 @@ class ECCOpQueueTest : public ::testing::Test { { UltraOp op; op.op_code = NULL_OP; - op.op = scalar::random_element(); - op.x_lo = scalar::random_element(); - op.x_hi = scalar::random_element(); - op.y_lo = scalar::random_element(); - op.y_hi = scalar::random_element(); - op.z_1 = scalar::random_element(); - op.z_2 = scalar::random_element(); + op.op = Scalar::random_element(); + op.x_lo = Scalar::random_element(); + op.x_hi = Scalar::random_element(); + op.y_lo = Scalar::random_element(); + op.y_hi = Scalar::random_element(); + op.z_1 = Scalar::random_element(); + op.z_2 = Scalar::random_element(); op.return_is_infinity = false; return op; } + + struct MockRawOpsTable { + std::vector raw_ops; + void append(const ECCVMOperation& op) { raw_ops.push_back(op); } + + MockRawOpsTable(const auto& subtable_ops) + { + for (auto& ops : std::ranges::reverse_view(subtable_ops)) { + for (const auto& op : ops) { + append(op); + } + } + } + }; + + static ECCVMOperation random_raw_op() + { + return ECCVMOperation{ .mul = true, + .base_point = curve::BN254::Group::affine_element::random_element(), + .z1 = uint256_t(Scalar::random_element()), + .z2 = uint256_t(Scalar::random_element()), + .mul_scalar_full = Scalar::random_element() }; + } }; TEST(ECCOpQueueTest, Basic) @@ -90,73 +116,7 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } -// TEST(ECCOpQueueTest, UltraOpsTableConstruction) -// { -// using Fr = fr; - -// constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table - -// // Construct sets of ultra ops, each representing those added by a single circuit -// const size_t NUM_SUBTABLES = 3; -// std::array, NUM_SUBTABLES> subtable_ultra_ops; -// std::array subtable_op_counts = { 4, 2, 7 }; -// for (auto [subtable_ops, op_count] : zip_view(subtable_ultra_ops, subtable_op_counts)) { -// for (size_t i = 0; i < op_count; ++i) { -// subtable_ops.push_back(ECCOpQueueTest::random_ultra_op()); -// } -// } - -// // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) -// ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); - -// // Construct the concatenated table internal to the op queue -// UltraOpsTable ultra_ops_table; -// for (const auto& subtable_ops : subtable_ultra_ops) { -// ultra_ops_table.create_new_subtable(); -// for (const auto& op : subtable_ops) { -// ultra_ops_table.append_operation(op); -// } -// } - -// // Compute the expected total table size as the sum of the subtable op counts times 2 -// size_t expected_table_size = 0; -// for (const auto& count : subtable_op_counts) { -// expected_table_size += count * NUM_ROWS_PER_OP; -// } - -// // Check that the ultra ops table internal to the op queue has the correct size -// EXPECT_EQ(ultra_ops_table.size(), expected_table_size); - -// // expected_ultra_ops_table.columns[2][3] += 1; - -// // Check that the ultra ops table constructed by the op queue matches the expected table using row iterator -// std::array, 4> columns; -// for (const auto& row : ultra_ops_table) { -// for (size_t i = 0; i < 4; ++i) { -// columns[i].push_back(row[i]); -// } -// } - -// EXPECT_EQ(expected_ultra_ops_table.columns, columns); - -// // Construct polynomials corresponding to the columns of the ultra ops table -// std::array, 4> ultra_ops_table_polynomials; -// std::array, 4> column_spans; -// for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { -// column = Polynomial(expected_table_size); -// column_span = column.coeffs(); -// } -// ultra_ops_table.populate_columns(column_spans); - -// // Check that the ultra ops table constructed by the op queue matches the expected table -// for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { -// for (auto [expected_value, value] : zip_view(expected_column, poly.coeffs())) { -// EXPECT_EQ(expected_value, value); -// } -// } -// } - -TEST(ECCOpQueueTest, NewUltraOpsTableConstruction) +TEST(ECCOpQueueTest, UltraOpsTableConstruction) { using Fr = fr; @@ -206,4 +166,41 @@ TEST(ECCOpQueueTest, NewUltraOpsTableConstruction) EXPECT_EQ(expected_value, value); } } -} \ No newline at end of file +} + +TEST(ECCOpQueueTest, RawOpsTableConstruction) +{ + using ECCVMOperation = bb::eccvm::VMOperation; + + // Construct sets of ultra ops, each representing those added by a single circuit + const size_t NUM_SUBTABLES = 3; + std::array, NUM_SUBTABLES> subtable_raw_ops; + std::array subtable_op_counts = { 4, 2, 7 }; + for (auto [subtable_ops, op_count] : zip_view(subtable_raw_ops, subtable_op_counts)) { + for (size_t i = 0; i < op_count; ++i) { + subtable_ops.push_back(ECCOpQueueTest::random_raw_op()); + } + } + + // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) + ECCOpQueueTest::MockRawOpsTable expected_raw_ops_table(subtable_raw_ops); + + // Construct the concatenated raw ops table + RawEccOpsTable raw_ops_table; + for (const auto& subtable_ops : subtable_raw_ops) { + raw_ops_table.create_new_subtable(); + for (const auto& op : subtable_ops) { + raw_ops_table.push(op); + } + } + + // Check that the table has the correct size + auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); + EXPECT_EQ(raw_ops_table.size(), expected_num_ops); + + // Check that the manually constructed mock table matches the genuine table + size_t idx = 0; + for (const auto& op : raw_ops_table) { + EXPECT_EQ(expected_raw_ops_table.raw_ops[idx++], op); + } +} diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp index 03fa646de860..1d5a948d0bde 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp @@ -1,156 +1,11 @@ #pragma once -#include "barretenberg/common/ref_array.hpp" -#include "barretenberg/common/zip_view.hpp" #include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/eccvm/eccvm_builder_types.hpp" -#include "barretenberg/stdlib/primitives/bigfield/constants.hpp" #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" -#include "barretenberg/stdlib_circuit_builders/op_queue/eccvm_row_tracker.hpp" namespace bb { -// /** -// * @brief -// * -// * @tparam Derived child class type for CRTP (curiously recurring template pattern) implementation class -// * @tparam TABLE_WIDTH -// * @tparam OpFormat -// */ -// template class OpsTable { -// using Curve = curve::BN254; -// using Fr = Curve::ScalarField; - -// protected: -// struct Subtable { -// std::array, TABLE_WIDTH> columns; -// std::unique_ptr prev; - -// Subtable(size_t size_hint = 0) -// { -// for (auto& column : columns) { -// column.reserve(size_hint); -// } -// } - -// size_t size() const { return columns[0].size(); } -// }; - -// std::unique_ptr current_subtable = nullptr; - -// public: -// using Row = RefArray; - -// size_t size() const -// { -// size_t total_size = 0; -// for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { -// total_size += subtable->size(); -// } -// return total_size; -// } - -// class Iterator { -// Subtable* subtable = nullptr; -// size_t index = 0; -// size_t subtable_size = 0; - -// public: -// Iterator() = default; -// Iterator(Subtable* sub, size_t idx) -// : subtable(sub) -// , index(idx) -// , subtable_size(sub ? sub->size() : 0) -// {} - -// Row operator*() -// { -// std::array row; -// for (size_t i = 0; i < TABLE_WIDTH; ++i) { -// row[i] = &subtable->columns[i][index]; -// } -// return row; -// } - -// Iterator& operator++() -// { -// if (++index < subtable_size) { // return row within the current subtable -// return *this; -// } -// if (subtable->prev) { // update to next subtable -// subtable = subtable->prev.get(); -// subtable_size = subtable->size(); -// index = 0; -// } else { -// *this = Iterator(); // end of iteration -// } -// return *this; -// } - -// bool operator!=(const Iterator& other) const { return subtable != other.subtable || index != other.index; } -// }; - -// // begin() and end() to enable range based iteration over the full table -// Iterator begin() { return Iterator(current_subtable.get(), 0); } -// Iterator end() { return {}; } - -// void create_new_subtable(size_t size_hint = 0) -// { -// auto new_subtable = std::make_unique(size_hint); -// new_subtable->prev = std::move(current_subtable); -// current_subtable = std::move(new_subtable); -// } - -// void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } -// }; - -// /** -// * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format -// * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each -// * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations -// associated -// * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the -// * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point -// * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: -// * -// * OP | X_lo | X_hi | Y_lo -// * 0 | Y_hi | z1 | z2 -// */ -// class UltraOpsTable : public OpsTable { -// using Curve = curve::BN254; -// using Fr = Curve::ScalarField; - -// static constexpr size_t TABLE_WIDTH = 4; - -// public: -// void append_operation_impl(const UltraOp& op) -// { -// current_subtable->columns[0].emplace_back(op.op); -// current_subtable->columns[1].emplace_back(op.x_lo); -// current_subtable->columns[2].emplace_back(op.x_hi); -// current_subtable->columns[3].emplace_back(op.y_lo); - -// current_subtable->columns[0].emplace_back(0); // only the first 'op' field is utilized -// current_subtable->columns[1].emplace_back(op.y_hi); -// current_subtable->columns[2].emplace_back(op.z_1); -// current_subtable->columns[3].emplace_back(op.z_2); -// } - -// void populate_columns(std::array, TABLE_WIDTH>& target_columns) -// { -// size_t i = 0; -// for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { -// for (size_t j = 0; j < subtable->size(); ++j) { -// target_columns[0][i] = subtable->columns[0][j]; -// target_columns[1][i] = subtable->columns[1][j]; -// target_columns[2][i] = subtable->columns[2][j]; -// target_columns[3][i] = subtable->columns[3][j]; -// ++i; -// } -// } -// } -// }; - -template class EccOpsTable { +template class EccOpsTable { using Curve = curve::BN254; using Fr = Curve::ScalarField; @@ -220,11 +75,21 @@ template class EccOpsTable { current_subtable = std::move(new_subtable); } - // void append_operation(const OpFormat& op) { static_cast(this)->append_operation_impl(op); } void push(const OpFormat& op) { current_subtable->data.push_back(op); } }; -class UltraEccOpsTable : public EccOpsTable { +/** + * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format + * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each + * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated + * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the + * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point + * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: + * + * OP | X_lo | X_hi | Y_lo + * 0 | Y_hi | z1 | z2 + */ +class UltraEccOpsTable : public EccOpsTable { static constexpr size_t TABLE_WIDTH = 4; using Curve = curve::BN254; using Fr = Curve::ScalarField; @@ -251,4 +116,10 @@ class UltraEccOpsTable : public EccOpsTable { } }; +/** + * @brief The table of ECC ops used by the ECCVM circuit builder + * @note just an alias for a specialized EccOpsTable + */ +using RawEccOpsTable = EccOpsTable>; + } // namespace bb From 0d5664ae4f924b9541c70f98f0585481e0dfa6c5 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 22:10:08 +0000 Subject: [PATCH 12/16] comments update --- .../op_queue/ultra_ops_table.hpp | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp index 1d5a948d0bde..a1131e2aa8fc 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp @@ -5,21 +5,31 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" namespace bb { +/** + * @brief A table of ECC operations + * @details The table is constructed via concatenation of subtables of ECC operations. The table concatentation protocol + * (Merge protocol) requires that the concatenation be achieved via PRE-pending successive tables. To avoid the need for + * expensive memory reallocations associated with physically prepending, the subtables are stored in a linked list that + * can be traversed to reconstruct the columns of the aggregate tables as needed (e.g. in corresponding polynomials). + * + * @tparam OpFormat Format of the ECC operations stored in the table + */ template class EccOpsTable { using Curve = curve::BN254; using Fr = Curve::ScalarField; protected: + // A segment of the table. (In practice, each subtable stores the operations of a single circuit) struct Subtable { std::vector data; - std::unique_ptr prev; + std::unique_ptr prev; // the previously constructed subtable in the linked list Subtable(size_t size_hint = 0) { data.reserve(size_hint); } size_t size() const { return data.size(); } }; - std::unique_ptr current_subtable = nullptr; + std::unique_ptr current_subtable = nullptr; // the subtable to which new ops are appended public: size_t size() const @@ -31,6 +41,7 @@ template class EccOpsTable { return total_size; } + // An iterator to allow range based iteration over the full table class Iterator { Subtable* subtable = nullptr; size_t index = 0; @@ -64,30 +75,32 @@ template class EccOpsTable { bool operator!=(const Iterator& other) const { return subtable != other.subtable || index != other.index; } }; - // begin() and end() to enable range based iteration over the full table Iterator begin() { return Iterator(current_subtable.get(), 0); } - Iterator end() { return {}; } + Iterator end() { return {}; } // end marked by default iterator void create_new_subtable(size_t size_hint = 0) { - auto new_subtable = std::make_unique(size_hint); - new_subtable->prev = std::move(current_subtable); - current_subtable = std::move(new_subtable); + auto new_subtable = std::make_unique(size_hint); // new subtable + new_subtable->prev = std::move(current_subtable); // old one is the prev of the new one + current_subtable = std::move(new_subtable); // set the new one to current } + // push a new operation to the current subtable void push(const OpFormat& op) { current_subtable->data.push_back(op); } }; /** - * @brief Stores a width-4 aggregate table of elliptic curve operations represented in the Ultra format - * @details The aggregate Ultra ops table is constructed by succesively PREpending subtables of ultra ops, where each - * subtable represents the operations performed in a single circuit. To avoid expensive memory reallocations associated - * with physically prepending, the subtables are stored in a linked list that can be traversed to reconstruct the - * columns of the aggregate tables as needed (e.g. in corresponding polynomials). An EC operation OP involving point - * P(X, Y) and scalar z is encoded in the Ultra format as two rows in a width-4 table as follows: + * @brief Stores a table of elliptic curve operations represented in the Ultra format + * @details An ECC operation OP involing point P(X,Y) and scalar z is represented in the Ultra format as a tuple of the + * form {OP, X_lo, X_hi, Y_lo, Y_hi, z1, z2}, where the coordinates are split into hi and lo limbs and z1, z2 are the + * endomorphism scalars associated with z. Because the Ultra/Mega arithmetization utilizes 4 wires, each op occupies two + * rows in a width-4 execution trace, arranged as follows: * * OP | X_lo | X_hi | Y_lo * 0 | Y_hi | z1 | z2 + * + * The table data is stored in the UltraOp tuple format but is converted to four columns of Fr scalars for use in the + * polynomials in the proving system. */ class UltraEccOpsTable : public EccOpsTable { static constexpr size_t TABLE_WIDTH = 4; @@ -95,7 +108,11 @@ class UltraEccOpsTable : public EccOpsTable { using Fr = Curve::ScalarField; public: - // WORKTODO: multithreaded version of this function + /** + * @brief Populate the provided array of columns with the width-4 representation of the table data + * @todo multithreaded this functionality + * @param target_columns + */ void populate_column_data(std::array, TABLE_WIDTH>& target_columns) { size_t i = 0; From 11b1751fb19840e230d40ab55436ccc194b2b08e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 22:11:47 +0000 Subject: [PATCH 13/16] file name change --- .../stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp | 2 +- .../op_queue/{ultra_ops_table.hpp => ecc_ops_table.hpp} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/{ultra_ops_table.hpp => ecc_ops_table.hpp} (100%) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index fd189f61c9a0..553f8a4e0adb 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -1,7 +1,7 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" #include "barretenberg/common/zip_view.hpp" #include "barretenberg/polynomials/polynomial.hpp" -#include "barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp" #include #include diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp similarity index 100% rename from barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ultra_ops_table.hpp rename to barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp From e31e190cf6d2b317e5629ffdc1f189f9fcf07ce9 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Fri, 14 Feb 2025 23:36:48 +0000 Subject: [PATCH 14/16] separate test file --- .../op_queue/ecc_op_queue.test.cpp | 168 ----------------- .../op_queue/ecc_ops_table.test.cpp | 173 ++++++++++++++++++ 2 files changed, 173 insertions(+), 168 deletions(-) create mode 100644 barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp index 553f8a4e0adb..a0d2596da286 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.test.cpp @@ -1,87 +1,8 @@ #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" -#include "barretenberg/common/zip_view.hpp" -#include "barretenberg/polynomials/polynomial.hpp" -#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp" #include -#include - using namespace bb; -class ECCOpQueueTest : public ::testing::Test { - using Curve = curve::BN254; - using Point = Curve::AffineElement; - using Scalar = fr; - using ECCVMOperation = bb::eccvm::VMOperation; - - public: - struct MockUltraOpsTable { - std::array, 4> columns; - void append(const UltraOp& op) - { - columns[0].push_back(op.op); - columns[1].push_back(op.x_lo); - columns[2].push_back(op.x_hi); - columns[3].push_back(op.y_lo); - - columns[0].push_back(0); - columns[1].push_back(op.y_hi); - columns[2].push_back(op.z_1); - columns[3].push_back(op.z_2); - } - - // Construct the= ultra ops table such that the subtables appear in reverse order, as if prepended - MockUltraOpsTable(const auto& subtable_ops) - { - for (auto& ops : std::ranges::reverse_view(subtable_ops)) { - for (const auto& op : ops) { - append(op); - } - } - } - - size_t size() const { return columns[0].size(); } - }; - - static UltraOp random_ultra_op() - { - UltraOp op; - op.op_code = NULL_OP; - op.op = Scalar::random_element(); - op.x_lo = Scalar::random_element(); - op.x_hi = Scalar::random_element(); - op.y_lo = Scalar::random_element(); - op.y_hi = Scalar::random_element(); - op.z_1 = Scalar::random_element(); - op.z_2 = Scalar::random_element(); - op.return_is_infinity = false; - return op; - } - - struct MockRawOpsTable { - std::vector raw_ops; - void append(const ECCVMOperation& op) { raw_ops.push_back(op); } - - MockRawOpsTable(const auto& subtable_ops) - { - for (auto& ops : std::ranges::reverse_view(subtable_ops)) { - for (const auto& op : ops) { - append(op); - } - } - } - }; - - static ECCVMOperation random_raw_op() - { - return ECCVMOperation{ .mul = true, - .base_point = curve::BN254::Group::affine_element::random_element(), - .z1 = uint256_t(Scalar::random_element()), - .z2 = uint256_t(Scalar::random_element()), - .mul_scalar_full = Scalar::random_element() }; - } -}; - TEST(ECCOpQueueTest, Basic) { ECCOpQueue op_queue; @@ -115,92 +36,3 @@ TEST(ECCOpQueueTest, InternalAccumulatorCorrectness) op_queue.eq_and_reset(); EXPECT_TRUE(op_queue.get_accumulator().is_point_at_infinity()); } - -TEST(ECCOpQueueTest, UltraOpsTableConstruction) -{ - using Fr = fr; - - constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table - - // Construct sets of ultra ops, each representing those added by a single circuit - const size_t NUM_SUBTABLES = 3; - std::array, NUM_SUBTABLES> subtable_ultra_ops; - std::array subtable_op_counts = { 4, 2, 7 }; - for (auto [subtable_ops, op_count] : zip_view(subtable_ultra_ops, subtable_op_counts)) { - for (size_t i = 0; i < op_count; ++i) { - subtable_ops.push_back(ECCOpQueueTest::random_ultra_op()); - } - } - - // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) - ECCOpQueueTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); - - // Construct the concatenated table internal to the op queue - UltraEccOpsTable ultra_ops_table; - for (const auto& subtable_ops : subtable_ultra_ops) { - ultra_ops_table.create_new_subtable(); - for (const auto& op : subtable_ops) { - ultra_ops_table.push(op); - } - } - - // Check that the ultra ops table internal to the op queue has the correct size - auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); - EXPECT_EQ(ultra_ops_table.size(), expected_num_ops); - - // // expected_ultra_ops_table.columns[2][3] += 1; - - // Construct polynomials corresponding to the columns of the ultra ops table - const size_t expected_table_size = expected_num_ops * NUM_ROWS_PER_OP; - std::array, 4> ultra_ops_table_polynomials; - std::array, 4> column_spans; - for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { - column = Polynomial(expected_table_size); - column_span = column.coeffs(); - } - ultra_ops_table.populate_column_data(column_spans); - - // Check that the ultra ops table constructed by the op queue matches the expected table - for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { - for (auto [expected_value, value] : zip_view(expected_column, poly.coeffs())) { - EXPECT_EQ(expected_value, value); - } - } -} - -TEST(ECCOpQueueTest, RawOpsTableConstruction) -{ - using ECCVMOperation = bb::eccvm::VMOperation; - - // Construct sets of ultra ops, each representing those added by a single circuit - const size_t NUM_SUBTABLES = 3; - std::array, NUM_SUBTABLES> subtable_raw_ops; - std::array subtable_op_counts = { 4, 2, 7 }; - for (auto [subtable_ops, op_count] : zip_view(subtable_raw_ops, subtable_op_counts)) { - for (size_t i = 0; i < op_count; ++i) { - subtable_ops.push_back(ECCOpQueueTest::random_raw_op()); - } - } - - // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) - ECCOpQueueTest::MockRawOpsTable expected_raw_ops_table(subtable_raw_ops); - - // Construct the concatenated raw ops table - RawEccOpsTable raw_ops_table; - for (const auto& subtable_ops : subtable_raw_ops) { - raw_ops_table.create_new_subtable(); - for (const auto& op : subtable_ops) { - raw_ops_table.push(op); - } - } - - // Check that the table has the correct size - auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); - EXPECT_EQ(raw_ops_table.size(), expected_num_ops); - - // Check that the manually constructed mock table matches the genuine table - size_t idx = 0; - for (const auto& op : raw_ops_table) { - EXPECT_EQ(expected_raw_ops_table.raw_ops[idx++], op); - } -} diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp new file mode 100644 index 000000000000..58ab0397f10e --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp @@ -0,0 +1,173 @@ +#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp" +#include "barretenberg/common/zip_view.hpp" +#include "barretenberg/polynomials/polynomial.hpp" +#include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include + +#include + +using namespace bb; + +class EccOpsTableTest : public ::testing::Test { + using Curve = curve::BN254; + using Scalar = fr; + using ECCVMOperation = eccvm::VMOperation; + + public: + // Mock ultra ops table that constructs a concatenated table from successively prepended subtables + struct MockUltraOpsTable { + std::array, 4> columns; + void append(const UltraOp& op) + { + columns[0].push_back(op.op); + columns[1].push_back(op.x_lo); + columns[2].push_back(op.x_hi); + columns[3].push_back(op.y_lo); + + columns[0].push_back(0); + columns[1].push_back(op.y_hi); + columns[2].push_back(op.z_1); + columns[3].push_back(op.z_2); + } + + // Construct the= ultra ops table such that the subtables appear in reverse order, as if prepended + MockUltraOpsTable(const auto& subtable_ops) + { + for (auto& ops : std::ranges::reverse_view(subtable_ops)) { + for (const auto& op : ops) { + append(op); + } + } + } + + size_t size() const { return columns[0].size(); } + }; + + // Mock raw ops table that constructs a concatenated table from successively prepended subtables + struct MockRawOpsTable { + std::vector raw_ops; + void append(const ECCVMOperation& op) { raw_ops.push_back(op); } + + MockRawOpsTable(const auto& subtable_ops) + { + for (auto& ops : std::ranges::reverse_view(subtable_ops)) { + for (const auto& op : ops) { + append(op); + } + } + } + }; + + static UltraOp random_ultra_op() + { + UltraOp op; + op.op_code = NULL_OP; + op.op = Scalar::random_element(); + op.x_lo = Scalar::random_element(); + op.x_hi = Scalar::random_element(); + op.y_lo = Scalar::random_element(); + op.y_hi = Scalar::random_element(); + op.z_1 = Scalar::random_element(); + op.z_2 = Scalar::random_element(); + op.return_is_infinity = false; + return op; + } + + static ECCVMOperation random_raw_op() + { + return ECCVMOperation{ .mul = true, + .base_point = curve::BN254::Group::affine_element::random_element(), + .z1 = uint256_t(Scalar::random_element()), + .z2 = uint256_t(Scalar::random_element()), + .mul_scalar_full = Scalar::random_element() }; + } +}; + +// Ensure UltraOpsTable correctly constructs a concatenated table from successively prepended subtables +TEST(EccOpsTableTest, UltraOpsTable) +{ + using Fr = fr; + + constexpr size_t NUM_ROWS_PER_OP = 2; // Each ECC op is represented by two rows in the ultra ops table + + // Construct sets of ultra ops, each representing those added by a single circuit + const size_t NUM_SUBTABLES = 3; + std::array, NUM_SUBTABLES> subtable_ultra_ops; + std::array subtable_op_counts = { 4, 2, 7 }; + for (auto [subtable_ops, op_count] : zip_view(subtable_ultra_ops, subtable_op_counts)) { + for (size_t i = 0; i < op_count; ++i) { + subtable_ops.push_back(EccOpsTableTest::random_ultra_op()); + } + } + + // Construct the mock ultra ops table which contains the subtables ordered in reverse (as if prepended) + EccOpsTableTest::MockUltraOpsTable expected_ultra_ops_table(subtable_ultra_ops); + + // Construct the concatenated table internal to the op queue + UltraEccOpsTable ultra_ops_table; + for (const auto& subtable_ops : subtable_ultra_ops) { + ultra_ops_table.create_new_subtable(); + for (const auto& op : subtable_ops) { + ultra_ops_table.push(op); + } + } + + // Check that the ultra ops table internal to the op queue has the correct size + auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); + EXPECT_EQ(ultra_ops_table.size(), expected_num_ops); + + // Construct polynomials corresponding to the columns of the ultra ops table + const size_t expected_table_size = expected_num_ops * NUM_ROWS_PER_OP; + std::array, 4> ultra_ops_table_polynomials; + std::array, 4> column_spans; + for (auto [column_span, column] : zip_view(column_spans, ultra_ops_table_polynomials)) { + column = Polynomial(expected_table_size); + column_span = column.coeffs(); + } + ultra_ops_table.populate_column_data(column_spans); + + // Check that the ultra ops table constructed by the op queue matches the expected table + for (auto [expected_column, poly] : zip_view(expected_ultra_ops_table.columns, ultra_ops_table_polynomials)) { + for (auto [expected_value, value] : zip_view(expected_column, poly.coeffs())) { + EXPECT_EQ(expected_value, value); + } + } +} + +// Ensure RawOpsTable correctly constructs a concatenated table from successively prepended subtables +TEST(EccOpsTableTest, RawOpsTable) +{ + using ECCVMOperation = bb::eccvm::VMOperation; + + // Construct sets of raw ops, each representing those added by a single circuit + const size_t NUM_SUBTABLES = 3; + std::array, NUM_SUBTABLES> subtable_raw_ops; + std::array subtable_op_counts = { 4, 2, 7 }; + for (auto [subtable_ops, op_count] : zip_view(subtable_raw_ops, subtable_op_counts)) { + for (size_t i = 0; i < op_count; ++i) { + subtable_ops.push_back(EccOpsTableTest::random_raw_op()); + } + } + + // Construct the mock raw ops table which contains the subtables ordered in reverse (as if prepended) + EccOpsTableTest::MockRawOpsTable expected_raw_ops_table(subtable_raw_ops); + + // Construct the concatenated raw ops table + RawEccOpsTable raw_ops_table; + for (const auto& subtable_ops : subtable_raw_ops) { + raw_ops_table.create_new_subtable(); + for (const auto& op : subtable_ops) { + raw_ops_table.push(op); + } + } + + // Check that the table has the correct size + auto expected_num_ops = std::accumulate(subtable_op_counts.begin(), subtable_op_counts.end(), size_t(0)); + EXPECT_EQ(raw_ops_table.size(), expected_num_ops); + + // Check that the table matches the manually constructed mock table + size_t idx = 0; + for (const auto& op : raw_ops_table) { + EXPECT_EQ(expected_raw_ops_table.raw_ops[idx++], op); + } +} From b04384c7d91365257783e1d925c72102cd62fc66 Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sat, 15 Feb 2025 16:20:44 +0000 Subject: [PATCH 15/16] simplify by using an std container and composition over inheritance --- .../op_queue/ecc_ops_table.hpp | 109 +++++++----------- .../op_queue/ecc_ops_table.test.cpp | 5 +- 2 files changed, 41 insertions(+), 73 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp index a1131e2aa8fc..04893dbdb0de 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp @@ -3,92 +3,60 @@ #include "barretenberg/ecc/curves/bn254/bn254.hpp" #include "barretenberg/eccvm/eccvm_builder_types.hpp" #include "barretenberg/stdlib_circuit_builders/op_queue/ecc_op_queue.hpp" +#include namespace bb { /** * @brief A table of ECC operations * @details The table is constructed via concatenation of subtables of ECC operations. The table concatentation protocol * (Merge protocol) requires that the concatenation be achieved via PRE-pending successive tables. To avoid the need for - * expensive memory reallocations associated with physically prepending, the subtables are stored in a linked list that + * expensive memory reallocations associated with physically prepending, the subtables are stored as a std::deque that * can be traversed to reconstruct the columns of the aggregate tables as needed (e.g. in corresponding polynomials). * * @tparam OpFormat Format of the ECC operations stored in the table */ template class EccOpsTable { - using Curve = curve::BN254; - using Fr = Curve::ScalarField; - - protected: - // A segment of the table. (In practice, each subtable stores the operations of a single circuit) - struct Subtable { - std::vector data; - std::unique_ptr prev; // the previously constructed subtable in the linked list - - Subtable(size_t size_hint = 0) { data.reserve(size_hint); } - - size_t size() const { return data.size(); } - }; - - std::unique_ptr current_subtable = nullptr; // the subtable to which new ops are appended + using Subtable = std::vector; + std::deque table; public: size_t size() const { - size_t total_size = 0; - for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { - total_size += subtable->size(); + size_t total = 0; + for (const auto& subtable : table) { + total += subtable.size(); } - return total_size; + return total; } - // An iterator to allow range based iteration over the full table - class Iterator { - Subtable* subtable = nullptr; - size_t index = 0; - size_t subtable_size = 0; - - public: - Iterator() = default; - Iterator(Subtable* sub, size_t idx) - : subtable(sub) - , index(idx) - , subtable_size(sub ? sub->size() : 0) - {} + auto& get() const { return table; } - OpFormat operator*() { return subtable->data[index]; } - - Iterator& operator++() - { - if (++index < subtable_size) { // return row within the current subtable - return *this; - } - if (subtable->prev) { // update to next subtable - subtable = subtable->prev.get(); - subtable_size = subtable->size(); - index = 0; - } else { - *this = Iterator(); // end of iteration - } - return *this; - } - - bool operator!=(const Iterator& other) const { return subtable != other.subtable || index != other.index; } - }; - - Iterator begin() { return Iterator(current_subtable.get(), 0); } - Iterator end() { return {}; } // end marked by default iterator + void push(const OpFormat& op) { table.front().push_back(op); } void create_new_subtable(size_t size_hint = 0) { - auto new_subtable = std::make_unique(size_hint); // new subtable - new_subtable->prev = std::move(current_subtable); // old one is the prev of the new one - current_subtable = std::move(new_subtable); // set the new one to current + std::vector new_subtable; + new_subtable.reserve(size_hint); + table.push_front(std::move(new_subtable)); } - // push a new operation to the current subtable - void push(const OpFormat& op) { current_subtable->data.push_back(op); } + // const version of operator[] + const OpFormat& operator[](size_t index) const + { + ASSERT(index < size()); + // simple linear search to find the correct subtable + for (const auto& subtable : table) { + if (index < subtable.size()) { + return subtable[index]; // found the correct subtable + } + index -= subtable.size(); // move to the next subtable + } + return table.front().front(); // should never reach here + } }; +using RawEccOpsTable = EccOpsTable>; + /** * @brief Stores a table of elliptic curve operations represented in the Ultra format * @details An ECC operation OP involing point P(X,Y) and scalar z is represented in the Ultra format as a tuple of the @@ -102,12 +70,19 @@ template class EccOpsTable { * The table data is stored in the UltraOp tuple format but is converted to four columns of Fr scalars for use in the * polynomials in the proving system. */ -class UltraEccOpsTable : public EccOpsTable { - static constexpr size_t TABLE_WIDTH = 4; +class UltraEccOpsTable { using Curve = curve::BN254; using Fr = Curve::ScalarField; + using UltraOpsTable = EccOpsTable; + + UltraOpsTable table; + static constexpr size_t TABLE_WIDTH = 4; public: + size_t size() const { return table.size(); } + void create_new_subtable(size_t size_hint = 0) { table.create_new_subtable(size_hint); } + void push(const UltraOp& op) { table.push(op); } + /** * @brief Populate the provided array of columns with the width-4 representation of the table data * @todo multithreaded this functionality @@ -116,8 +91,8 @@ class UltraEccOpsTable : public EccOpsTable { void populate_column_data(std::array, TABLE_WIDTH>& target_columns) { size_t i = 0; - for (auto* subtable = current_subtable.get(); subtable != nullptr; subtable = subtable->prev.get()) { - for (const auto& op : subtable->data) { + for (const auto& subtable : table.get()) { + for (const auto& op : subtable) { target_columns[0][i] = op.op; target_columns[1][i] = op.x_lo; target_columns[2][i] = op.x_hi; @@ -133,10 +108,4 @@ class UltraEccOpsTable : public EccOpsTable { } }; -/** - * @brief The table of ECC ops used by the ECCVM circuit builder - * @note just an alias for a specialized EccOpsTable - */ -using RawEccOpsTable = EccOpsTable>; - } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp index 58ab0397f10e..b8ccfe4c77bf 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.test.cpp @@ -166,8 +166,7 @@ TEST(EccOpsTableTest, RawOpsTable) EXPECT_EQ(raw_ops_table.size(), expected_num_ops); // Check that the table matches the manually constructed mock table - size_t idx = 0; - for (const auto& op : raw_ops_table) { - EXPECT_EQ(expected_raw_ops_table.raw_ops[idx++], op); + for (size_t i = 0; i < expected_num_ops; ++i) { + EXPECT_EQ(expected_raw_ops_table.raw_ops[i], raw_ops_table[i]); } } From 5a37316275785580244a700b6729859d0ce7ba9e Mon Sep 17 00:00:00 2001 From: ledwards2225 Date: Sat, 15 Feb 2025 16:41:09 +0000 Subject: [PATCH 16/16] ditch the deque --- .../stdlib_circuit_builders/op_queue/ecc_ops_table.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp index 04893dbdb0de..890e117a478a 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/op_queue/ecc_ops_table.hpp @@ -17,7 +17,7 @@ namespace bb { */ template class EccOpsTable { using Subtable = std::vector; - std::deque table; + std::vector table; public: size_t size() const @@ -37,7 +37,7 @@ template class EccOpsTable { { std::vector new_subtable; new_subtable.reserve(size_hint); - table.push_front(std::move(new_subtable)); + table.insert(table.begin(), std::move(new_subtable)); } // const version of operator[]