diff --git a/barretenberg/cpp/src/barretenberg/common/utils.hpp b/barretenberg/cpp/src/barretenberg/common/utils.hpp index 5bf5f3c0f9a4..627dd4dd9344 100644 --- a/barretenberg/cpp/src/barretenberg/common/utils.hpp +++ b/barretenberg/cpp/src/barretenberg/common/utils.hpp @@ -27,20 +27,9 @@ template 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 struct HashableTuple : public std::tuple { - using std::tuple::tuple; - std::size_t hash() const noexcept { return std::apply(utils::hash_as_tuple, *this); } -}; - } // namespace bb::utils -// Needed for HashableTuple to work as a tuple. -template struct std::tuple_size> { - 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 concept Hashable = requires(const T& t) { { diff --git a/barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp b/barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp index 3b0f77f37778..21bc5b23471a 100644 --- a/barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp +++ b/barretenberg/cpp/src/barretenberg/ecc/fields/field_declarations.hpp @@ -729,6 +729,8 @@ template void write(B& buf, field const& v template struct std::hash> { std::size_t operator()(const bb::field& 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]); } }; diff --git a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp b/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp index 74ea911952ac..7f90783f125e 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp +++ b/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().process(trace); - LookupIntoDynamicTableSequential().process(trace); + LookupIntoDynamicTableGeneric().process(trace); + LookupIntoDynamicTableGeneric().process(trace); check_relation(trace); } diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/events/range_check_event.hpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/events/range_check_event.hpp index c78771f28f05..5f1274637c21 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/events/range_check_event.hpp +++ b/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; diff --git a/barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_dbs.hpp b/barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_dbs.hpp index bb6f786115d8..74d3e02e8fb9 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_dbs.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_dbs.hpp @@ -1,6 +1,7 @@ #pragma once -#include "barretenberg/common/utils.hpp" +#include + #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; + std::tuple; unordered_flat_map get_sibling_path_hints; - using GetPreviousValueIndexKey = utils::HashableTuple; + using GetPreviousValueIndexKey = std::tuple; unordered_flat_map get_previous_value_index_hints; - using GetLeafPreimageKey = utils::HashableTuple; + using GetLeafPreimageKey = std::tuple; unordered_flat_map> get_leaf_preimage_hints_public_data_tree; unordered_flat_map> get_leaf_preimage_hints_nullifier_tree; - using GetLeafValueKey = - utils::HashableTuple; + using GetLeafValueKey = std::tuple; unordered_flat_map get_leaf_value_hints; // State modification hints. - using SequentialInsertHintPublicDataTreeKey = utils:: - HashableTuple; + using SequentialInsertHintPublicDataTreeKey = + std::tuple; unordered_flat_map> sequential_insert_hints_public_data_tree; - using SequentialInsertHintNullifierTreeKey = utils:: - HashableTuple; + using SequentialInsertHintNullifierTreeKey = + std::tuple; unordered_flat_map> sequential_insert_hints_nullifier_tree; diff --git a/barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp b/barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp index ab093588866e..03d7a4d95c62 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp @@ -106,27 +106,38 @@ template class LookupIntoDynamicTableSequential : publ SetDummyInverses(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 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."); + } + } } }; diff --git a/barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp b/barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp index e7bb017fa709..ba3349605236 100644 --- a/barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp +++ b/barretenberg/cpp/src/barretenberg/vm2/tracegen_helper.cpp @@ -359,8 +359,8 @@ TraceContainer AvmTraceGenHelper::generate_trace(EventsContainer&& events) LookupIntoDynamicTableSequential>(), std::make_unique>(), // Field GT - std::make_unique>(), - std::make_unique>(), + std::make_unique>(), + std::make_unique>(), // Merkle checks std::make_unique>(), std::make_unique>(),