feat: Avoid requiring arith gates in sequence (#4869)

Another installation in the series of PRs to allow for trace sorting. A
common pattern in the builder is that the final gate in some sequence
(usually aux or sort gates) is an arithmetic gate that serves two
purposes: (1) provides wires values to the preceding gate via shifts,
and (2) performs some check expressed as an arithmetic constraint. This
causes problems if we sort the gates by type, which brings the
arithmetic gate out of sequence. The solution is simply to add a dummy
gate (for purpose 1) prior to the arithmetic gate (purpose 2).

Note 1: I've added a `create_dummy_constraint` method and used it in
some places to replace equivalent logic.
Note 2: The additional dummy gate in the process ROM array logic changes
the vkey for circuits that use it, hence the updated vk hash in the js
test that checks vk hash consistency.

No change:
```
--------------------------------------------------------------------------------
Benchmark                      Time             CPU   Iterations UserCounters...
--------------------------------------------------------------------------------
ClientIVCBench/Full/6      32585 ms        26673 ms           
```

barretenberg/cpp/src/barretenberg/join_split_example/proofs/join_split/join_split.test.cpp

@@ -707,7 +707,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change)
     // The below part detects any changes in the join-split circuit
     constexpr uint32_t CIRCUIT_GATE_COUNT = 49492;
     constexpr uint32_t GATES_NEXT_POWER_OF_TWO = 65535;
-    const uint256_t VK_HASH("893b71911c19c3a06a2658f0ef5f66f6e23455af7c8771a67c80addb060a479c");
+    const uint256_t VK_HASH("29f333ac68164d4e079b3d4243c95425432f317aa26ad67fd668ca883b28e236");
 
     auto number_of_gates_js = result.number_of_gates;
     std::cout << get_verification_key()->sha256_hash() << std::endl;

barretenberg/cpp/src/barretenberg/proof_system/arithmetization/gate_data.hpp

@@ -155,12 +155,4 @@ template <typename FF> struct poseidon2_internal_gate_ {
     uint32_t d;
     size_t round_idx;
 };
-
-/* Last gate for poseidon2, needed because poseidon2 gates compare against the shifted wires. */
-template <typename FF> struct poseidon2_end_gate_ {
-    uint32_t a;
-    uint32_t b;
-    uint32_t c;
-    uint32_t d;
-};
 } // namespace bb

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.cpp

@@ -303,34 +303,6 @@ void GoblinUltraCircuitBuilder_<FF>::create_poseidon2_internal_gate(const poseid
     ++this->num_gates;
 }
 
-/**
- * @brief Poseidon2 end round gate, needed because poseidon2 rounds compare with shifted wires
- * @details The Poseidon2 permutation is 64 rounds, but needs to be a block of 65 rows, since the result of applying a
- * round of Poseidon2 is stored in the next row (the shifted row). As a result, we need this end row to compare with the
- * result from the 64th round of Poseidon2. Note that it does not activate any selectors since it only serves as a
- * comparison through the shifted wires.
- */
-template <typename FF> void GoblinUltraCircuitBuilder_<FF>::create_poseidon2_end_gate(const poseidon2_end_gate_<FF>& in)
-{
-    this->blocks.main.populate_wires(in.a, in.b, in.c, in.d);
-    this->blocks.main.q_m().emplace_back(0);
-    this->blocks.main.q_1().emplace_back(0);
-    this->blocks.main.q_2().emplace_back(0);
-    this->blocks.main.q_3().emplace_back(0);
-    this->blocks.main.q_c().emplace_back(0);
-    this->blocks.main.q_arith().emplace_back(0);
-    this->blocks.main.q_4().emplace_back(0);
-    this->blocks.main.q_sort().emplace_back(0);
-    this->blocks.main.q_lookup_type().emplace_back(0);
-    this->blocks.main.q_elliptic().emplace_back(0);
-    this->blocks.main.q_aux().emplace_back(0);
-    this->blocks.main.q_busread().emplace_back(0);
-    this->blocks.main.q_poseidon2_external().emplace_back(0);
-    this->blocks.main.q_poseidon2_internal().emplace_back(0);
-    this->check_selector_length_consistency();
-    ++this->num_gates;
-}
-
 template <typename FF>
 inline FF GoblinUltraCircuitBuilder_<FF>::compute_poseidon2_external_identity(FF q_poseidon2_external_value,
                                                                               FF q_1_value,

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_ultra_circuit_builder.hpp

@@ -140,7 +140,6 @@ template <typename FF> class GoblinUltraCircuitBuilder_ : public UltraCircuitBui
 
     void create_poseidon2_external_gate(const poseidon2_external_gate_<FF>& in);
     void create_poseidon2_internal_gate(const poseidon2_internal_gate_<FF>& in);
-    void create_poseidon2_end_gate(const poseidon2_end_gate_<FF>& in);
 
     FF compute_poseidon2_external_identity(FF q_poseidon2_external_value,
                                            FF q_1_value,

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.cpp

@@ -947,24 +947,39 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint(const std::ve
         check_selector_length_consistency();
     }
     // dummy gate needed because of sort widget's check of next row
-    blocks.main.populate_wires(
-        variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, this->zero_idx);
-    ++this->num_gates;
-    blocks.main.q_m().emplace_back(0);
-    blocks.main.q_1().emplace_back(0);
-    blocks.main.q_2().emplace_back(0);
-    blocks.main.q_3().emplace_back(0);
-    blocks.main.q_c().emplace_back(0);
-    blocks.main.q_arith().emplace_back(0);
-    blocks.main.q_4().emplace_back(0);
-    blocks.main.q_sort().emplace_back(0);
-    blocks.main.q_elliptic().emplace_back(0);
-    blocks.main.q_lookup_type().emplace_back(0);
-    blocks.main.q_aux().emplace_back(0);
+    create_dummy_gate(
+        blocks.main, variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, this->zero_idx);
+}
+
+/**
+ * @brief Create a gate with no constraints but with possibly non-trivial wire values
+ * @details A dummy gate can be used to provide wire values to be accessed via shifts by the gate that proceeds it. The
+ * dummy gate itself does not have to satisfy any constraints (all selectors are zero).
+ *
+ * @tparam Arithmetization
+ * @param variable_index
+ */
+template <typename Arithmetization>
+void UltraCircuitBuilder_<Arithmetization>::create_dummy_gate(
+    auto& block, const uint32_t& idx_1, const uint32_t& idx_2, const uint32_t& idx_3, const uint32_t& idx_4)
+{
+    block.populate_wires(idx_1, idx_2, idx_3, idx_4);
+    block.q_m().emplace_back(0);
+    block.q_1().emplace_back(0);
+    block.q_2().emplace_back(0);
+    block.q_3().emplace_back(0);
+    block.q_c().emplace_back(0);
+    block.q_arith().emplace_back(0);
+    block.q_4().emplace_back(0);
+    block.q_sort().emplace_back(0);
+    block.q_elliptic().emplace_back(0);
+    block.q_lookup_type().emplace_back(0);
+    block.q_aux().emplace_back(0);
     if constexpr (HasAdditionalSelectors<Arithmetization>) {
-        blocks.main.pad_additional();
+        block.pad_additional();
     }
     check_selector_length_consistency();
+    ++this->num_gates;
 }
 
 // useful to put variables in the witness that aren't already used - e.g. the dummy variables of the range constraint in
@@ -982,23 +997,7 @@ void UltraCircuitBuilder_<Arithmetization>::create_dummy_constraints(const std::
     this->assert_valid_variables(padded_list);
 
     for (size_t i = 0; i < padded_list.size(); i += gate_width) {
-        blocks.main.populate_wires(padded_list[i], padded_list[i + 1], padded_list[i + 2], padded_list[i + 3]);
-        ++this->num_gates;
-        blocks.main.q_m().emplace_back(0);
-        blocks.main.q_1().emplace_back(0);
-        blocks.main.q_2().emplace_back(0);
-        blocks.main.q_3().emplace_back(0);
-        blocks.main.q_c().emplace_back(0);
-        blocks.main.q_arith().emplace_back(0);
-        blocks.main.q_4().emplace_back(0);
-        blocks.main.q_sort().emplace_back(0);
-        blocks.main.q_elliptic().emplace_back(0);
-        blocks.main.q_lookup_type().emplace_back(0);
-        blocks.main.q_aux().emplace_back(0);
-        if constexpr (HasAdditionalSelectors<Arithmetization>) {
-            blocks.main.pad_additional();
-        }
-        check_selector_length_consistency();
+        create_dummy_gate(blocks.main, padded_list[i], padded_list[i + 1], padded_list[i + 2], padded_list[i + 3]);
     }
 }
 
@@ -1079,24 +1078,20 @@ void UltraCircuitBuilder_<Arithmetization>::create_sort_constraint_with_edges(
 
     // dummy gate needed because of sort widget's check of next row
     // use this gate to check end condition
-    blocks.main.populate_wires(
-        variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, this->zero_idx);
-    ++this->num_gates;
-    blocks.main.q_m().emplace_back(0);
-    blocks.main.q_1().emplace_back(1);
-    blocks.main.q_2().emplace_back(0);
-    blocks.main.q_3().emplace_back(0);
-    blocks.main.q_c().emplace_back(-end);
-    blocks.main.q_arith().emplace_back(1);
-    blocks.main.q_4().emplace_back(0);
-    blocks.main.q_sort().emplace_back(0);
-    blocks.main.q_elliptic().emplace_back(0);
-    blocks.main.q_lookup_type().emplace_back(0);
-    blocks.main.q_aux().emplace_back(0);
-    if constexpr (HasAdditionalSelectors<Arithmetization>) {
-        blocks.main.pad_additional();
-    }
-    check_selector_length_consistency();
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/879): This was formerly a single arithmetic gate. A
+    // dummy gate has been added to allow the previous gate to access the required wire data via shifts, allowing the
+    // arithmetic gate to occur out of sequence.
+    create_dummy_gate(
+        blocks.main, variable_index[variable_index.size() - 1], this->zero_idx, this->zero_idx, this->zero_idx);
+    create_big_add_gate({ variable_index[variable_index.size() - 1],
+                          this->zero_idx,
+                          this->zero_idx,
+                          this->zero_idx,
+                          1,
+                          0,
+                          0,
+                          0,
+                          -end });
 }
 
 // range constraint a value by decomposing it into limbs whose size should be the default range constraint size
@@ -2146,24 +2141,8 @@ void UltraCircuitBuilder_<Arithmetization>::create_final_sorted_RAM_gate(RamReco
     // of sequence with the RAM gates.
 
     // Create a final gate with all selectors zero; wire values are accessed by the previous RAM gate via shifted wires
-    blocks.main.populate_wires(
-        record.index_witness, record.timestamp_witness, record.value_witness, record.record_witness);
-    blocks.main.q_m().emplace_back(0);
-    blocks.main.q_1().emplace_back(0);
-    blocks.main.q_2().emplace_back(0);
-    blocks.main.q_3().emplace_back(0);
-    blocks.main.q_c().emplace_back(0);
-    blocks.main.q_arith().emplace_back(0);
-    blocks.main.q_4().emplace_back(0);
-    blocks.main.q_sort().emplace_back(0);
-    blocks.main.q_elliptic().emplace_back(0);
-    blocks.main.q_lookup_type().emplace_back(0);
-    blocks.main.q_aux().emplace_back(0);
-    if constexpr (HasAdditionalSelectors<Arithmetization>) {
-        blocks.main.pad_additional();
-    }
-    check_selector_length_consistency();
-    ++this->num_gates;
+    create_dummy_gate(
+        blocks.main, record.index_witness, record.timestamp_witness, record.value_witness, record.record_witness);
 
     // Create an add gate ensuring the final index is consistent with the size of the RAM array
     create_big_add_gate({
@@ -2510,6 +2489,10 @@ template <typename Arithmetization> void UltraCircuitBuilder_<Arithmetization>::
     // we have validated that all ROM reads are correctly constrained
     FF max_index_value((uint64_t)rom_array.state.size());
     uint32_t max_index = this->add_variable(max_index_value);
+    // TODO(https://github.com/AztecProtocol/barretenberg/issues/879): This was formerly a single arithmetic gate. A
+    // dummy gate has been added to allow the previous gate to access the required wire data via shifts, allowing the
+    // arithmetic gate to occur out of sequence.
+    create_dummy_gate(blocks.main, max_index, this->zero_idx, this->zero_idx, this->zero_idx);
     create_big_add_gate(
         {
             max_index,
@@ -2654,17 +2637,8 @@ template <typename Arithmetization> void UltraCircuitBuilder_<Arithmetization>::
     // add the index/timestamp values of the last sorted record in an empty add gate.
     // (the previous gate will access the wires on this gate and requires them to be those of the last record)
     const auto& last = sorted_ram_records[ram_array.records.size() - 1];
-    create_big_add_gate({
-        last.index_witness,
-        last.timestamp_witness,
-        this->zero_idx,
-        this->zero_idx,
-        0,
-        0,
-        0,
-        0,
-        0,
-    });
+    create_dummy_gate(blocks.main, last.index_witness, last.timestamp_witness, this->zero_idx, this->zero_idx);
+
     // Step 3: validate difference in timestamps is monotonically increasing. i.e. is <= maximum timestamp
     const size_t max_timestamp = ram_array.access_count - 1;
     for (auto& w : timestamp_deltas) {

barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/ultra_circuit_builder.hpp

@@ -702,6 +702,7 @@ class UltraCircuitBuilder_ : public CircuitBuilderBase<typename Arithmetization_
         const uint32_t variable_index,
         const size_t num_bits,
         std::string const& msg = "decompose_into_default_range_better_for_oddlimbnum");
+    void create_dummy_gate(auto& block, const uint32_t&, const uint32_t&, const uint32_t&, const uint32_t&);
     void create_dummy_constraints(const std::vector<uint32_t>& variable_index);
     void create_sort_constraint(const std::vector<uint32_t>& variable_index);
     void create_sort_constraint_with_edges(const std::vector<uint32_t>& variable_index, const FF&, const FF&);

barretenberg/cpp/src/barretenberg/stdlib/hash/poseidon2/poseidon2_permutation.cpp

@@ -81,15 +81,15 @@ typename Poseidon2Permutation<Params, Builder>::State Poseidon2Permutation<Param
             current_state[j] = witness_t<Builder>(builder, current_native_state[j]);
         }
     }
-    // need to add an extra row here to ensure that things check out, more details found in poseidon2_end_gate_
-    // definition
-    poseidon2_end_gate_<FF> in{
-        current_state[0].witness_index,
-        current_state[1].witness_index,
-        current_state[2].witness_index,
-        current_state[3].witness_index,
-    };
-    builder->create_poseidon2_end_gate(in);
+    // The Poseidon2 permutation is 64 rounds, but needs to be a block of 65 rows, since the result of
+    // applying a round of Poseidon2 is stored in the next row (the shifted row). As a result, we need this end row to
+    // compare with the result from the 64th round of Poseidon2. Note that it does not activate any selectors since it
+    // only serves as a comparison through the shifted wires.
+    builder->create_dummy_gate(builder->blocks.main,
+                               current_state[0].witness_index,
+                               current_state[1].witness_index,
+                               current_state[2].witness_index,
+                               current_state[3].witness_index);
     return current_state;
 }