fix(avm): fix lookup builder and FF hashing

barretenberg/cpp/src/barretenberg/common/utils.hpp

@@ -27,20 +27,9 @@ template <typename... Ts> size_t hash_as_tuple(const Ts&... ts)
     return seed;
 }
 
-// Like std::tuple, but you can hash it and therefore use in maps/sets.
-template <typename... Ts> struct HashableTuple : public std::tuple<Ts...> {
-    using std::tuple<Ts...>::tuple;
-    std::size_t hash() const noexcept { return std::apply(utils::hash_as_tuple<Ts...>, *this); }
-};
-
 } // namespace bb::utils
 
-// Needed for HashableTuple to work as a tuple.
-template <typename... Ts> struct std::tuple_size<bb::utils::HashableTuple<Ts...>> {
-    static constexpr size_t value = sizeof...(Ts);
-};
-
-// Define std::hash for any type that has a hash() method. This includes HashableTuple.
+// Define std::hash for any type that has a hash() method.
 template <typename T>
 concept Hashable = requires(const T& t) {
     {

barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp

@@ -729,6 +729,8 @@ template <typename B, typename Params> void write(B& buf, field<Params> const& v
 template <typename Params> struct std::hash<bb::field<Params>> {
     std::size_t operator()(const bb::field<Params>& ff) const noexcept
     {
-        return bb::utils::hash_as_tuple(ff.data[0], ff.data[1], ff.data[2], ff.data[3]);
+        // Just like in equality, we need to reduce the field element before hashing.
+        auto reduced = ff.reduce_once();
+        return bb::utils::hash_as_tuple(reduced.data[0], reduced.data[1], reduced.data[2], reduced.data[3]);
     }
 };

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp

@@ -25,7 +25,7 @@ namespace {
 using ::testing::NiceMock;
 using ::testing::TestWithParam;
 
-using tracegen::LookupIntoDynamicTableSequential;
+using tracegen::LookupIntoDynamicTableGeneric;
 using tracegen::TestTraceContainer;
 
 using simulation::EventEmitter;
@@ -119,8 +119,8 @@ TEST_P(FieldGreaterThanInteractionsTests, InteractionsWithRangeCheck)
     builder.process(event_emitter.dump_events(), trace);
     range_check_builder.process(range_check_event_emitter.dump_events(), trace);
 
-    LookupIntoDynamicTableSequential<lookup_a_hi_range::Settings>().process(trace);
-    LookupIntoDynamicTableSequential<lookup_a_lo_range::Settings>().process(trace);
+    LookupIntoDynamicTableGeneric<lookup_a_hi_range::Settings>().process(trace);
+    LookupIntoDynamicTableGeneric<lookup_a_lo_range::Settings>().process(trace);
 
     check_relation<ff_gt>(trace);
 }

barretenberg/cpp/src/barretenberg/vm2/simulation/events/range_check_event.hpp

@@ -12,7 +12,7 @@ struct RangeCheckEvent {
     uint128_t value;
     uint8_t num_bits;
 
-    bool operator==(const RangeCheckEvent& other) const { return value == other.value && num_bits == other.num_bits; }
+    bool operator==(const RangeCheckEvent& other) const = default;
 
     // To be used with deduplicating event emitters.
     using Key = std::tuple<uint128_t, uint8_t>;

barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_dbs.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
-#include "barretenberg/common/utils.hpp"
+#include <tuple>
+
 #include "barretenberg/crypto/merkle_tree/hash_path.hpp"
 #include "barretenberg/crypto/merkle_tree/indexed_tree/indexed_leaf.hpp"
 #include "barretenberg/crypto/merkle_tree/response.hpp"
@@ -61,27 +62,26 @@ class HintedRawMerkleDB final : public LowLevelMerkleDBInterface {
 
     // Query hints.
     using GetSiblingPathKey =
-        utils::HashableTuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::index_t>;
+        std::tuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::index_t>;
     unordered_flat_map<GetSiblingPathKey, crypto::merkle_tree::fr_sibling_path> get_sibling_path_hints;
-    using GetPreviousValueIndexKey = utils::HashableTuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, FF>;
+    using GetPreviousValueIndexKey = std::tuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, FF>;
     unordered_flat_map<GetPreviousValueIndexKey, crypto::merkle_tree::GetLowIndexedLeafResponse>
         get_previous_value_index_hints;
-    using GetLeafPreimageKey = utils::HashableTuple<AppendOnlyTreeSnapshot, crypto::merkle_tree::index_t>;
+    using GetLeafPreimageKey = std::tuple<AppendOnlyTreeSnapshot, crypto::merkle_tree::index_t>;
     unordered_flat_map<GetLeafPreimageKey, crypto::merkle_tree::IndexedLeaf<crypto::merkle_tree::PublicDataLeafValue>>
         get_leaf_preimage_hints_public_data_tree;
     unordered_flat_map<GetLeafPreimageKey, crypto::merkle_tree::IndexedLeaf<crypto::merkle_tree::NullifierLeafValue>>
         get_leaf_preimage_hints_nullifier_tree;
-    using GetLeafValueKey =
-        utils::HashableTuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::index_t>;
+    using GetLeafValueKey = std::tuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::index_t>;
     unordered_flat_map<GetLeafValueKey, FF> get_leaf_value_hints;
     // State modification hints.
-    using SequentialInsertHintPublicDataTreeKey = utils::
-        HashableTuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::PublicDataLeafValue>;
+    using SequentialInsertHintPublicDataTreeKey =
+        std::tuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::PublicDataLeafValue>;
     unordered_flat_map<SequentialInsertHintPublicDataTreeKey,
                        SequentialInsertHint<crypto::merkle_tree::PublicDataLeafValue>>
         sequential_insert_hints_public_data_tree;
-    using SequentialInsertHintNullifierTreeKey = utils::
-        HashableTuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::NullifierLeafValue>;
+    using SequentialInsertHintNullifierTreeKey =
+        std::tuple<AppendOnlyTreeSnapshot, world_state::MerkleTreeId, crypto::merkle_tree::NullifierLeafValue>;
     unordered_flat_map<SequentialInsertHintNullifierTreeKey,
                        SequentialInsertHint<crypto::merkle_tree::NullifierLeafValue>>
         sequential_insert_hints_nullifier_tree;

barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp

@@ -106,27 +106,38 @@ template <typename LookupSettings> class LookupIntoDynamicTableSequential : publ
 
         SetDummyInverses<LookupSettings>(trace);
 
+        // For the sequential builder, it is critical that we visit the source rows in order.
+        // Since the trace does not guarantee visiting rows in order, we need to collect the rows.
+        std::vector<uint32_t> src_rows_in_order;
+        src_rows_in_order.reserve(trace.get_column_rows(LookupSettings::SRC_SELECTOR));
         trace.visit_column(LookupSettings::SRC_SELECTOR, [&](uint32_t row, const FF& src_sel_value) {
             assert(src_sel_value == 1);
             (void)src_sel_value; // Avoid GCC complaining of unused parameter when asserts are disabled.
+            src_rows_in_order.push_back(row);
+        });
+        std::sort(src_rows_in_order.begin(), src_rows_in_order.end());
 
+        for (uint32_t row : src_rows_in_order) {
             auto src_values = trace.get_multiple(LookupSettings::SRC_COLUMNS, row);
 
             // We find the first row in the destination columns where the values match.
-            while (dst_row < max_dst_row) {
+            bool found = false;
+            while (!found && dst_row < max_dst_row) {
                 // TODO: As an optimization, we could try to only walk the rows where the selector is active.
                 // We can't just do a visit because we cannot skip rows with that.
                 auto dst_selector = trace.get(LookupSettings::DST_SELECTOR, dst_row);
                 if (dst_selector == 1 && src_values == trace.get_multiple(LookupSettings::DST_COLUMNS, dst_row)) {
                     trace.set(LookupSettings::COUNTS, dst_row, trace.get(LookupSettings::COUNTS, dst_row) + 1);
-                    return; // Done with this source row.
+                    found = true;
                 }
                 ++dst_row;
             }
 
-            throw std::runtime_error("Failed computing counts for " + std::string(LookupSettings::NAME) +
-                                     ". Could not find tuple in destination.");
-        });
+            if (!found) {
+                throw std::runtime_error("Failed computing counts for " + std::string(LookupSettings::NAME) +
+                                         ". Could not find tuple in destination.");
+            }
+        }
     }
 };
 

barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp

@@ -359,8 +359,8 @@ TraceContainer AvmTraceGenHelper::generate_trace(EventsContainer&& events)
                 LookupIntoDynamicTableSequential<lookup_address_derivation_preaddress_scalar_mul_settings>>(),
             std::make_unique<LookupIntoDynamicTableSequential<lookup_address_derivation_address_ecadd_settings>>(),
             // Field GT
-            std::make_unique<LookupIntoDynamicTableSequential<lookup_ff_gt_a_lo_range_settings>>(),
-            std::make_unique<LookupIntoDynamicTableSequential<lookup_ff_gt_a_hi_range_settings>>(),
+            std::make_unique<LookupIntoDynamicTableGeneric<lookup_ff_gt_a_lo_range_settings>>(),
+            std::make_unique<LookupIntoDynamicTableGeneric<lookup_ff_gt_a_hi_range_settings>>(),
             // Merkle checks
             std::make_unique<LookupIntoDynamicTableSequential<lookup_merkle_check_merkle_poseidon2_read_settings>>(),
             std::make_unique<LookupIntoDynamicTableSequential<lookup_merkle_check_merkle_poseidon2_write_settings>>(),