refactor: op queue

barretenberg/cpp/src/barretenberg/benchmark/goblin_bench/eccvm.bench.cpp

@@ -35,7 +35,7 @@ Builder generate_trace(size_t target_num_gates)
         op_queue->mul_accumulate(b, y);
         op_queue->add_accumulate(a);
         op_queue->mul_accumulate(b, x);
-        op_queue->eq();
+        op_queue->eq_and_reset();
     }
 
     Builder builder{ op_queue };

barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.hpp

@@ -116,13 +116,12 @@ class ECCVMCircuitBuilder {
         std::vector<std::pair<size_t, size_t>> msm_mul_index;
         std::vector<size_t> msm_sizes;
 
-        // std::vector<std::vector<size_t>> msm_indices;
-        // std::vector<size_t> active_msm_indices;
-        for (size_t i = 0; i < op_queue->raw_ops.size(); ++i) {
-            const auto& op = op_queue->raw_ops[i];
+        const auto& raw_ops = op_queue->get_raw_ops();
+        size_t op_idx = 0;
+        for (const auto& op : raw_ops) {
             if (op.mul) {
                 if (op.z1 != 0 || op.z2 != 0) {
-                    msm_opqueue_index.push_back(i);
+                    msm_opqueue_index.push_back(op_idx);
                     msm_mul_index.emplace_back(msm_count, active_mul_count);
                 }
                 if (op.z1 != 0) {
@@ -136,9 +135,10 @@ class ECCVMCircuitBuilder {
                 msm_count++;
                 active_mul_count = 0;
             }
+            op_idx++;
         }
         // if last op is a mul we have not correctly computed the total number of msms
-        if (op_queue->raw_ops.back().mul) {
+        if (raw_ops.back().mul) {
             msm_sizes.push_back(active_mul_count);
             msm_count++;
         }
@@ -152,7 +152,7 @@ class ECCVMCircuitBuilder {
             for (size_t i = start; i < end; i++) {
                 //  for (size_t i = 0; i < msm_opqueue_index.size(); ++i) {
                 const size_t opqueue_index = msm_opqueue_index[i];
-                const auto& op = op_queue->raw_ops[opqueue_index];
+                const auto& op = raw_ops[opqueue_index];
                 auto [msm_index, mul_index] = msm_mul_index[i];
                 if (op.z1 != 0) {
                     ASSERT(msms_test.size() > msm_index);

barretenberg/cpp/src/barretenberg/eccvm/eccvm_circuit_builder.test.cpp

@@ -29,11 +29,11 @@ TEST(ECCVMCircuitBuilderTests, BaseCase)
     op_queue->mul_accumulate(b, y);
     op_queue->add_accumulate(a);
     op_queue->mul_accumulate(b, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->add_accumulate(c);
     op_queue->mul_accumulate(a, x);
     op_queue->mul_accumulate(b, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->mul_accumulate(a, x);
     op_queue->mul_accumulate(b, x);
     op_queue->mul_accumulate(c, x);
@@ -86,7 +86,7 @@ TEST(ECCVMCircuitBuilderTests, ShortMul)
     Fr x = small_x;
 
     op_queue->mul_accumulate(a, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
 
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
@@ -95,7 +95,6 @@ TEST(ECCVMCircuitBuilderTests, ShortMul)
 
 TEST(ECCVMCircuitBuilderTests, EqFails)
 {
-    using ECCVMOperation = eccvm::VMOperation<G1>;
     std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
 
     auto generators = G1::derive_generators("test generators", 3);
@@ -104,14 +103,8 @@ TEST(ECCVMCircuitBuilderTests, EqFails)
 
     op_queue->mul_accumulate(a, x);
     // Tamper with the eq op such that the expected value is incorect
-    op_queue->raw_ops.emplace_back(ECCVMOperation{ .add = false,
-                                                   .mul = false,
-                                                   .eq = true,
-                                                   .reset = true,
-                                                   .base_point = a,
-                                                   .z1 = 0,
-                                                   .z2 = 0,
-                                                   .mul_scalar_full = 0 });
+    op_queue->add_erroneous_equality_op_for_testing();
+
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
     EXPECT_EQ(result, false);
@@ -121,7 +114,7 @@ TEST(ECCVMCircuitBuilderTests, EmptyRow)
 {
     std::shared_ptr<ECCOpQueue> op_queue = std::make_shared<ECCOpQueue>();
 
-    op_queue->empty_row();
+    op_queue->empty_row_for_testing();
 
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
@@ -137,8 +130,8 @@ TEST(ECCVMCircuitBuilderTests, EmptyRowBetweenOps)
     Fr x = Fr::random_element(&engine);
 
     op_queue->mul_accumulate(a, x);
-    op_queue->empty_row();
-    op_queue->eq();
+    op_queue->empty_row_for_testing();
+    op_queue->eq_and_reset();
 
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
@@ -154,7 +147,7 @@ TEST(ECCVMCircuitBuilderTests, EndWithEq)
     Fr x = Fr::random_element(&engine);
 
     op_queue->mul_accumulate(a, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
 
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
@@ -170,7 +163,7 @@ TEST(ECCVMCircuitBuilderTests, EndWithAdd)
     Fr x = Fr::random_element(&engine);
 
     op_queue->mul_accumulate(a, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->add_accumulate(a);
 
     ECCVMCircuitBuilder circuit{ op_queue };
@@ -187,7 +180,7 @@ TEST(ECCVMCircuitBuilderTests, EndWithMul)
     Fr x = Fr::random_element(&engine);
 
     op_queue->add_accumulate(a);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->mul_accumulate(a, x);
 
     ECCVMCircuitBuilder circuit{ op_queue };
@@ -204,10 +197,10 @@ TEST(ECCVMCircuitBuilderTests, EndWithNoop)
     Fr x = Fr::random_element(&engine);
 
     op_queue->add_accumulate(a);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->mul_accumulate(a, x);
 
-    op_queue->empty_row();
+    op_queue->empty_row_for_testing();
     ECCVMCircuitBuilder circuit{ op_queue };
     bool result = ECCVMTraceChecker::check(circuit);
     EXPECT_EQ(result, true);
@@ -228,7 +221,7 @@ TEST(ECCVMCircuitBuilderTests, MSM)
             expected += (points[i] * scalars[i]);
             op_queue->mul_accumulate(points[i], scalars[i]);
         }
-        op_queue->eq();
+        op_queue->eq_and_reset();
     };
 
     // single msms

barretenberg/cpp/src/barretenberg/eccvm/eccvm_composer.test.cpp

@@ -43,11 +43,11 @@ ECCVMCircuitBuilder generate_circuit(numeric::RNG* engine = nullptr)
     op_queue->mul_accumulate(b, y);
     op_queue->add_accumulate(a);
     op_queue->mul_accumulate(b, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->add_accumulate(c);
     op_queue->mul_accumulate(a, x);
     op_queue->mul_accumulate(b, x);
-    op_queue->eq();
+    op_queue->eq_and_reset();
     op_queue->mul_accumulate(a, x);
     op_queue->mul_accumulate(b, x);
     op_queue->mul_accumulate(c, x);
@@ -68,17 +68,10 @@ TEST_F(ECCVMComposerTests, BaseCase)
 
 TEST_F(ECCVMComposerTests, EqFails)
 {
-    using ECCVMOperation = eccvm::VMOperation<G1>;
     auto builder = generate_circuit(&engine);
     // Tamper with the eq op such that the expected value is incorect
-    builder.op_queue->raw_ops.emplace_back(ECCVMOperation{ .add = false,
-                                                           .mul = false,
-                                                           .eq = true,
-                                                           .reset = true,
-                                                           .base_point = G1::affine_one,
-                                                           .z1 = 0,
-                                                           .z2 = 0,
-                                                           .mul_scalar_full = 0 });
+    builder.op_queue->add_erroneous_equality_op_for_testing();
+
     builder.op_queue->num_transcript_rows++;
     ECCVMProver prover(builder);
 

barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp

@@ -520,7 +520,7 @@ class ECCVMFlavor {
 
             std::array<std::vector<size_t>, 2> point_table_read_counts;
             const auto transcript_state = ECCVMTranscriptBuilder::compute_transcript_state(
-                builder.op_queue->raw_ops, builder.get_number_of_muls());
+                builder.op_queue->get_raw_ops(), builder.get_number_of_muls());
             const auto precompute_table_state = ECCVMPrecomputedTablesBuilder::compute_precompute_state(flattened_muls);
             const auto msm_state = ECCVMMSMMBuilder::compute_msm_state(
                 msms, point_table_read_counts, builder.get_number_of_muls(), builder.op_queue->get_num_msm_rows());

barretenberg/cpp/src/barretenberg/eccvm/eccvm_transcript.test.cpp

@@ -218,11 +218,11 @@ class ECCVMTranscriptTests : public ::testing::Test {
         op_queue->mul_accumulate(b, y);
         op_queue->add_accumulate(a);
         op_queue->mul_accumulate(b, x);
-        op_queue->eq();
+        op_queue->eq_and_reset();
         op_queue->add_accumulate(c);
         op_queue->mul_accumulate(a, x);
         op_queue->mul_accumulate(b, x);
-        op_queue->eq();
+        op_queue->eq_and_reset();
         op_queue->mul_accumulate(a, x);
         op_queue->mul_accumulate(b, x);
         op_queue->mul_accumulate(c, x);

barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/goblin_ultra_circuit_builder.cpp

@@ -95,25 +95,23 @@ template <typename FF> void GoblinUltraCircuitBuilder_<FF>::add_gates_to_ensure_
 }
 
 /**
- * @brief Add gates for simple point addition (no mul) and add the raw operation data to the op queue
+ * @brief Add simple point addition operation to the op queue and add corresponding gates
  *
  * @param point Point to be added into the accumulator
  */
 template <typename FF>
 ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::queue_ecc_add_accum(const bb::g1::affine_element& point)
 {
-    // Add raw op to queue
-    op_queue->add_accumulate(point);
-
-    // Decompose operation inputs into width-four form and add ecc op gates
-    auto op_tuple = decompose_ecc_operands(add_accum_op_idx, point);
-    populate_ecc_op_wires(op_tuple);
+    // Add the operation to the op queue
+    auto ultra_op = op_queue->add_accumulate(point);
 
+    // Add corresponding gates for the operation
+    ecc_op_tuple op_tuple = populate_ecc_op_wires(ultra_op);
     return op_tuple;
 }
 
 /**
- * @brief Add gates for point mul-then-accumulate and add the raw operation data to the op queue
+ * @brief Add point mul-then-accumulate operation to the op queue and add corresponding gates
  *
  * @tparam FF
  * @param point
@@ -123,97 +121,58 @@ ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::queue_ecc_add_accum(const bb::g1::a
 template <typename FF>
 ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::queue_ecc_mul_accum(const bb::g1::affine_element& point, const FF& scalar)
 {
-    // Add raw op to op queue
-    op_queue->mul_accumulate(point, scalar);
-
-    // Decompose operation inputs into width-four form and add ecc op gates
-    auto op_tuple = decompose_ecc_operands(mul_accum_op_idx, point, scalar);
-    populate_ecc_op_wires(op_tuple);
+    // Add the operation to the op queue
+    auto ultra_op = op_queue->mul_accumulate(point, scalar);
 
+    // Add corresponding gates for the operation
+    ecc_op_tuple op_tuple = populate_ecc_op_wires(ultra_op);
     return op_tuple;
 }
 
 /**
- * @brief Add point equality gates based on the current value of the accumulator internal to the op queue and add the
- * raw operation data to the op queue
+ * @brief Add point equality operation to the op queue based on the value of the internal accumulator and add
+ * corresponding gates
  *
  * @return ecc_op_tuple encoding the point to which equality has been asserted
  */
 template <typename FF> ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::queue_ecc_eq()
 {
-    // Add raw op to op queue
-    auto point = op_queue->eq();
-
-    // Decompose operation inputs into width-four form and add ecc op gates
-    auto op_tuple = decompose_ecc_operands(equality_op_idx, point);
-    populate_ecc_op_wires(op_tuple);
+    // Add the operation to the op queue
+    auto ultra_op = op_queue->eq_and_reset();
 
+    // Add corresponding gates for the operation
+    ecc_op_tuple op_tuple = populate_ecc_op_wires(ultra_op);
     return op_tuple;
 }
 
 /**
- * @brief Decompose ecc operands into components, add corresponding variables, return ecc op tuple
+ * @brief Add goblin ecc op gates for a single operation
  *
- * @param op_idx Index of op code in variables array
- * @param point
- * @param scalar
- * @return ecc_op_tuple Tuple of indices into variables array used to construct pair of ecc op gates
+ * @param ultra_op Operation data expressed in the ultra format
+ * @note All selectors are set to 0 since the ecc op selector is derived later based on the block size/location.
  */
-template <typename FF>
-ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::decompose_ecc_operands(uint32_t op_idx,
-                                                                    const g1::affine_element& point,
-                                                                    const FF& scalar)
+template <typename FF> ecc_op_tuple GoblinUltraCircuitBuilder_<FF>::populate_ecc_op_wires(const UltraOp& ultra_op)
 {
-    // Decompose point coordinates (Fq) into hi-lo chunks (Fr)
-    const size_t CHUNK_SIZE = 2 * DEFAULT_NON_NATIVE_FIELD_LIMB_BITS;
-    auto x_256 = uint256_t(point.x);
-    auto y_256 = uint256_t(point.y);
-    auto x_lo = FF(x_256.slice(0, CHUNK_SIZE));
-    auto x_hi = FF(x_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2));
-    auto y_lo = FF(y_256.slice(0, CHUNK_SIZE));
-    auto y_hi = FF(y_256.slice(CHUNK_SIZE, CHUNK_SIZE * 2));
-
-    // Split scalar into 128 bit endomorphism scalars
-    FF z_1 = 0;
-    FF z_2 = 0;
-    auto converted = scalar.from_montgomery_form();
-    FF::split_into_endomorphism_scalars(converted, z_1, z_2);
-    z_1 = z_1.to_montgomery_form();
-    z_2 = z_2.to_montgomery_form();
-
-    // Populate ultra ops in OpQueue with the decomposed operands
-    op_queue->populate_ultra_ops({ this->variables[op_idx], x_lo, x_hi, y_lo, y_hi, z_1, z_2 });
-
-    // Add variables for decomposition and get indices needed for op wires
-    auto x_lo_idx = this->add_variable(x_lo);
-    auto x_hi_idx = this->add_variable(x_hi);
-    auto y_lo_idx = this->add_variable(y_lo);
-    auto y_hi_idx = this->add_variable(y_hi);
-    auto z_1_idx = this->add_variable(z_1);
-    auto z_2_idx = this->add_variable(z_2);
-
-    return { op_idx, x_lo_idx, x_hi_idx, y_lo_idx, y_hi_idx, z_1_idx, z_2_idx };
-}
-
-/**
- * @brief Add ecc operation to queue
- *
- * @param in Variables array indices corresponding to operation inputs
- * @note We dont explicitly set values for the selectors here since their values are fully determined by
- * the number of ecc op gates. E.g. in the composer we can reconstruct q_ecc_op as the indicator over the range of ecc
- * op gates. All other selectors are simply 0 on this domain.
- */
-template <typename FF> void GoblinUltraCircuitBuilder_<FF>::populate_ecc_op_wires(const ecc_op_tuple& in)
-{
-    this->blocks.ecc_op.populate_wires(in.op, in.x_lo, in.x_hi, in.y_lo);
+    ecc_op_tuple op_tuple;
+    op_tuple.op = get_ecc_op_idx(ultra_op.op_code);
+    op_tuple.x_lo = this->add_variable(ultra_op.x_lo);
+    op_tuple.x_hi = this->add_variable(ultra_op.x_hi);
+    op_tuple.y_lo = this->add_variable(ultra_op.y_lo);
+    op_tuple.y_hi = this->add_variable(ultra_op.y_hi);
+    op_tuple.z_1 = this->add_variable(ultra_op.z_1);
+    op_tuple.z_2 = this->add_variable(ultra_op.z_2);
+
+    this->blocks.ecc_op.populate_wires(op_tuple.op, op_tuple.x_lo, op_tuple.x_hi, op_tuple.y_lo);
     for (auto& selector : this->blocks.ecc_op.selectors) {
         selector.emplace_back(0);
     }
 
-    this->blocks.ecc_op.populate_wires(this->zero_idx, in.y_hi, in.z_1, in.z_2);
+    this->blocks.ecc_op.populate_wires(this->zero_idx, op_tuple.y_hi, op_tuple.z_1, op_tuple.z_2);
     for (auto& selector : this->blocks.ecc_op.selectors) {
         selector.emplace_back(0);
     }
+
+    return op_tuple;
 };
 
 template <typename FF> void GoblinUltraCircuitBuilder_<FF>::set_goblin_ecc_op_code_constant_variables()