feat(avm): Sorting of memory entries (#16336)

Author: jeanmon

barretenberg/cpp/src/barretenberg/vm2/simulation/events/memory_event.cpp

@@ -0,0 +1,14 @@
+#include "barretenberg/vm2/simulation/events/memory_event.hpp"
+
+namespace bb::avm2::simulation {
+
+static_assert(MemoryMode::READ < MemoryMode::WRITE, "MemoryMode::READ must be less than MemoryMode::WRITE");
+
+// Sorting order precedence is: space_id, addr, execution_clk, mode.
+bool MemoryEvent::operator<(MemoryEvent const& other) const
+{
+    return std::make_tuple(space_id, addr, execution_clk, mode) <
+           std::make_tuple(other.space_id, other.addr, other.execution_clk, other.mode);
+}
+
+} // namespace bb::avm2::simulation

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

@@ -17,6 +17,12 @@ struct MemoryEvent {
     MemoryAddress addr;
     MemoryValue value;
     uint32_t space_id;
+
+    /**
+     * @brief A comparator to be used by sorting algorithm (std::sort()). We sort first by
+     *        ascending space_id, followed by address, then by clk and finally read/write.
+     */
+    bool operator<(MemoryEvent const& other) const;
 };
 
 } // namespace bb::avm2::simulation

barretenberg/cpp/src/barretenberg/vm2/tracegen/keccakf1600_trace.cpp

@@ -797,6 +797,6 @@ const InteractionDefinition KeccakF1600TraceBuilder::interactions =
         .add<lookup_keccakf1600_src_abs_diff_positive_settings, InteractionType::LookupGeneric>()
         .add<lookup_keccakf1600_dst_abs_diff_positive_settings, InteractionType::LookupGeneric>()
         // Keccak slice memory to memory sub-trace
-        .add<lookup_keccak_memory_slice_to_mem_settings, InteractionType::LookupSequential>();
+        .add<lookup_keccak_memory_slice_to_mem_settings, InteractionType::LookupGeneric>();
 
 } // namespace bb::avm2::tracegen

barretenberg/cpp/src/barretenberg/vm2/tracegen/memory_trace.cpp

@@ -1,6 +1,7 @@
 #include "barretenberg/vm2/tracegen/memory_trace.hpp"
 
 #include <memory>
+#include <vector>
 
 #include "barretenberg/vm2/common/field.hpp"
 #include "barretenberg/vm2/tracegen/lib/interaction_def.hpp"
@@ -10,19 +11,29 @@ namespace bb::avm2::tracegen {
 void MemoryTraceBuilder::process(const simulation::EventEmitterInterface<simulation::MemoryEvent>::Container& events,
                                  TraceContainer& trace)
 {
+    // Create a vector of pointers to avoid copying the events
+    std::vector<const simulation::MemoryEvent*> event_ptrs;
+    event_ptrs.reserve(events.size());
+
+    for (const auto& event : events) {
+        event_ptrs.push_back(&event);
+    }
+
+    std::ranges::sort(event_ptrs, [](const auto* lhs, const auto* rhs) { return lhs->operator<(*rhs); });
+
     using C = Column;
 
     uint32_t row = 0;
-    for (const auto& event : events) {
+    for (const auto* event_ptr : event_ptrs) {
         trace.set(row,
                   { {
                       { C::memory_sel, 1 },
-                      { C::memory_clk, event.execution_clk },
-                      { C::memory_address, event.addr },
-                      { C::memory_value, event.value },
-                      { C::memory_tag, static_cast<uint8_t>(event.value.get_tag()) },
-                      { C::memory_rw, event.mode == simulation::MemoryMode::WRITE ? 1 : 0 },
-                      { C::memory_space_id, event.space_id },
+                      { C::memory_clk, event_ptr->execution_clk },
+                      { C::memory_address, event_ptr->addr },
+                      { C::memory_value, event_ptr->value },
+                      { C::memory_tag, static_cast<uint8_t>(event_ptr->value.get_tag()) },
+                      { C::memory_rw, event_ptr->mode == simulation::MemoryMode::WRITE ? 1 : 0 },
+                      { C::memory_space_id, event_ptr->space_id },
                   } });
         row++;
     }

barretenberg/cpp/src/barretenberg/vm2/tracegen/memory_trace.test.cpp

@@ -0,0 +1,80 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "barretenberg/vm2/simulation/events/memory_event.hpp"
+#include "barretenberg/vm2/testing/macros.hpp"
+#include "barretenberg/vm2/tracegen/memory_trace.hpp"
+#include "barretenberg/vm2/tracegen/test_trace_container.hpp"
+
+namespace bb::avm2::tracegen {
+namespace {
+
+using R = TestTraceContainer::Row;
+using C = Column;
+using simulation::MemoryEvent;
+
+TEST(MemoryTraceGenTest, Sorting)
+{
+    TestTraceContainer trace;
+
+    // We use .value field to check that the events are sorted correctly. The values
+    // are set to be in increasing order once the events are sorted.
+    std::vector<MemoryEvent> events = {
+        {
+            .execution_clk = UINT32_MAX,
+            .mode = simulation::MemoryMode::WRITE,
+            .addr = 20,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 5),
+            .space_id = 1001,
+        },
+        {
+            .execution_clk = 20,
+            .mode = simulation::MemoryMode::WRITE,
+            .addr = 5,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 2),
+            .space_id = 1000,
+        },
+        {
+            .execution_clk = UINT32_MAX,
+            .mode = simulation::MemoryMode::READ,
+            .addr = 20,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 4),
+            .space_id = 1001,
+        },
+        {
+            .execution_clk = 10,
+            .mode = simulation::MemoryMode::WRITE,
+            .addr = 6,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 3),
+            .space_id = 1000,
+        },
+        {
+            .execution_clk = 20,
+            .mode = simulation::MemoryMode::READ,
+            .addr = 5,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 1),
+            .space_id = 1000,
+        },
+        {
+            .execution_clk = 15,
+            .mode = simulation::MemoryMode::WRITE,
+            .addr = 5,
+            .value = MemoryValue::from_tag(MemoryTag::U8, 0),
+            .space_id = 1000,
+        },
+    };
+
+    MemoryTraceBuilder memory_trace_builder;
+    memory_trace_builder.process(events, trace);
+
+    const auto& rows = trace.as_rows();
+
+    ASSERT_EQ(rows.size(), 6);
+
+    for (uint32_t i = 0; i < trace.as_rows().size(); i++) {
+        EXPECT_THAT(rows.at(i), ROW_FIELD_EQ(memory_value, i));
+    }
+}
+
+} // namespace
+} // namespace bb::avm2::tracegen