Reduce memory usage of debug_trace* calls #9584

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracer.java

@@ -286,6 +286,11 @@ private Optional<Map<UInt256, UInt256>> captureStorage(final MessageFrame frame)
   private Optional<Bytes[]> captureMemory(final MessageFrame frame) {
     if (!options.traceMemory() || frame.memoryWordSize() == 0) {
       return Optional.empty();
+    } else if (frame.getMaybeUpdatedMemory().isEmpty()
+        && lastFrame != null
+        && lastFrame.getDepth() == frame.getDepth()
+        && lastFrame.getMemory().get().length == frame.memoryWordSize()) {
+      return lastFrame.getMemory();
     }
     final Bytes[] memoryContents = new Bytes[frame.memoryWordSize()];
     for (int i = 0; i < memoryContents.length; i++) {

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/DebugOperationTracerTest.java

@@ -38,10 +38,12 @@
 import org.hyperledger.besu.evm.tracing.TraceFrame;
 import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
+import java.util.List;
 import java.util.Map;
 import java.util.OptionalLong;
 import java.util.TreeMap;
 
+import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 import org.apache.tuweni.units.bigints.UInt256;
 import org.assertj.core.api.Assertions;
@@ -55,6 +57,8 @@ class DebugOperationTracerTest {
 
   private static final int DEPTH = 4;
   private static final long INITIAL_GAS = 1000L;
+  private static final Bytes32 WORD_1 = Bytes32.fromHexString("0x" + "aa".repeat(32));
+  private static final Bytes32 WORD_2 = Bytes32.fromHexString("0x" + "bb".repeat(32));
 
   @Mock private WorldUpdater worldUpdater;
 
@@ -67,6 +71,15 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
           return new OperationResult(20L, null);
         }
       };
+  private final Operation MSTORE_OPERATION =
+      new AbstractOperation(0x52, "MSTORE", 2, 0, null) {
+        @Override
+        public OperationResult execute(final MessageFrame frame, final EVM evm) {
+          // explicitMemoryUpdate=true simulates what MSTORE does in the real EVM
+          frame.writeMemory(0L, 32, WORD_2, true);
+          return new OperationResult(3L, null);
+        }
+      };
 
   private final CallOperation callOperation = new CallOperation(new CancunGasCalculator());
 
@@ -284,6 +297,126 @@ void shouldCaptureFrameWhenExceptionalHaltOccurs() {
     assertThat(traceFrame.getStorage()).contains(updatedStorage);
   }
 
+  @Test
+  void shouldUseLastSnapshotInNonMemoryWriteOperation() {
+    final MessageFrame frame = validMessageFrameWithInitiatedMemory(WORD_1);
+    final DebugOperationTracer tracer = createDebugOperationTracerWithMemory();
+
+    // Frame 0: non-memory-writing op — captures initial snapshot
+    traceFrame(frame, tracer, anOperation);
+    // Frame 1: another non-memory-writing op — should reuse last snapshot, not re-capture
+    traceFrame(frame, tracer, anOperation);
+
+    final List<TraceFrame> frames = tracer.getTraceFrames();
+    assertThat(frames).hasSize(2);
+    assertThat(frames.get(0).getMemory()).isPresent();
+    assertThat(frames.get(1).getMemory()).isPresent();
+    final Bytes[] frame0Memory = frames.get(0).getMemory().get();
+    final Bytes[] frame1Memory = frames.get(1).getMemory().get();
+
+    assertThat(frame1Memory)
+        .as("For a non-memory-writing opcode, the last snapshot must be reused")
+        .isSameAs(frame0Memory);
+  }
+
+  @Test
+  void shouldTakeNewMemorySnapshotWhenMemorySizeGrowsWithoutExplicitWrite() {
+    final MessageFrame frame = validMessageFrameWithInitiatedMemory(WORD_1);
+    final DebugOperationTracer tracer = createDebugOperationTracerWithMemory();
+
+    traceFrame(frame, tracer, anOperation);
+
+    frame.writeMemory(32L, 32, WORD_2);
+    traceFrame(frame, tracer, anOperation);
+
+    final List<TraceFrame> frames = tracer.getTraceFrames();
+    assertThat(frames).hasSize(2);
+
+    assertThat(frames.get(0).getMemory()).isPresent();
+    assertThat(frames.get(1).getMemory()).isPresent();
+    final Bytes[] snapshot0 = frames.get(0).getMemory().get();
+    final Bytes[] snapshot1 = frames.get(1).getMemory().get();
+
+    assertThat(snapshot1)
+        .as("When memory size grows, a fresh snapshot must be taken even without an explicit write")
+        .isNotSameAs(snapshot0);
+    assertThat(snapshot0).hasSize(1);
+    assertThat(snapshot1).hasSize(2);
+    assertThat(snapshot1[1]).isEqualTo(WORD_2);
+  }
+
+  @Test
+  void shouldTakeNewMemorySnapshotAfterExplicitMemoryWrite() {
+    final MessageFrame frame = validMessageFrameWithInitiatedMemory(WORD_1);
+    final DebugOperationTracer tracer = createDebugOperationTracerWithMemory();
+
+    traceFrame(frame, tracer, anOperation);
+    traceFrame(frame, tracer, MSTORE_OPERATION);
+
+    final List<TraceFrame> frames = tracer.getTraceFrames();
+    assertThat(frames).hasSize(2);
+    assertThat(frames.get(0).getMemory()).isPresent();
+    assertThat(frames.get(1).getMemory()).isPresent();
+
+    final Bytes[] before = frames.get(0).getMemory().get();
+    final Bytes[] after = frames.get(1).getMemory().get();
+
+    assertThat(after)
+        .as("After a memory-writing opcode, a new memory snapshot must be taken")
+        .isNotSameAs(before);
+    assertThat(before[0]).isEqualTo(WORD_1);
+    assertThat(after[0]).isEqualTo(WORD_2);
+  }
+
+  @Test
+  void shouldCaptureMemoryDirectlyWhenLastFrameIsNull() {
+    final MessageFrame frame = validMessageFrameWithInitiatedMemory(WORD_1);
+    final DebugOperationTracer tracer = createDebugOperationTracerWithMemory();
+    traceFrame(frame, tracer, anOperation);
+
+    final List<TraceFrame> frames = tracer.getTraceFrames();
+    assertThat(frames).hasSize(1);
+    assertThat(frames.getFirst().getMemory()).isPresent();
+    assertThat(frames.getFirst().getMemory().get()).containsExactly(WORD_1);
+  }
+
+  @Test
+  void shouldHandleCallScenarioAndDepthChange() {
+    final DebugOperationTracer tracer = createDebugOperationTracerWithMemory();
+    final MessageFrame parentFrame = validMessageFrameWithInitiatedMemory(WORD_1);
+
+    traceFrame(parentFrame, tracer, callOperation);
+
+    // Child frame enters at depth 1
+    final MessageFrame childFrame = validMessageFrameWithInitiatedMemory(WORD_2);
+    childFrame.getMessageFrameStack().add(childFrame);
+
+    traceFrame(childFrame, tracer, anOperation);
+    traceFrame(parentFrame, tracer, anOperation);
+
+    final List<TraceFrame> frames = tracer.getTraceFrames();
+    assertThat(frames).hasSize(3);
+    assertThat(frames.get(0).getMemory()).isPresent();
+    assertThat(frames.get(1).getMemory()).isPresent();
+    assertThat(frames.get(2).getMemory()).isPresent();
+
+    final Bytes[] parentSnapshot = frames.get(0).getMemory().get();
+    final Bytes[] childSnapshot = frames.get(1).getMemory().get();
+    final Bytes[] parentAfterReturn = frames.get(2).getMemory().get();
+
+    assertThat(childSnapshot)
+        .as("On CALL entry, child must get a fresh snapshot — not reuse the parent's")
+        .isNotSameAs(parentSnapshot);
+    assertThat(childSnapshot[0]).isEqualTo(WORD_2);
+
+    assertThat(parentAfterReturn)
+        .as("After RETURN, parent memory must be freshly captured — not the child's snapshot")
+        .isNotSameAs(childSnapshot);
+    assertThat(parentAfterReturn[0])
+        .as("After RETURN, parent memory must reflect the parent frame's actual content")
+        .isEqualTo(WORD_1);
+  }
+
   private TraceFrame traceFrame(final MessageFrame frame) {
     return traceFrame(
         frame,
@@ -306,6 +439,14 @@ private TraceFrame traceFrame(
     return getOnlyTraceFrame(tracer);
   }
 
+  private void traceFrame(
+      final MessageFrame frame, final DebugOperationTracer tracer, final Operation operation) {
+    frame.setCurrentOperation(operation);
+    tracer.tracePreExecution(frame);
+    OperationResult operationResult = operation.execute(frame, null);
+    tracer.tracePostExecution(frame, operationResult);
+  }
+
   private MessageFrame validMessageFrame() {
     final MessageFrame frame = validMessageFrameBuilder().build();
     frame.setCurrentOperation(anOperation);
@@ -337,6 +478,13 @@ private MessageFrameTestFixture validMessageFrameBuilder() {
         .blockchain(blockchain);
   }
 
+  private MessageFrame validMessageFrameWithInitiatedMemory(final Bytes32 initiatedMemory) {
+    final MessageFrame frame = validMessageFrameBuilder().build();
+    frame.writeMemory(0L, 32, initiatedMemory);
+
+    return frame;
+  }
+
   private Map<UInt256, UInt256> setupStorageForCapture(final MessageFrame frame) {
     final MutableAccount account = mock(MutableAccount.class);
     when(worldUpdater.getAccount(frame.getRecipientAddress())).thenReturn(account);
@@ -353,4 +501,14 @@ private Map<UInt256, UInt256> setupStorageForCapture(final MessageFrame frame) {
     frame.writeMemory(64, 32, word3);
     return updatedStorage;
   }
+
+  private static DebugOperationTracer createDebugOperationTracerWithMemory() {
+    final OpCodeTracerConfig config =
+        OpCodeTracerConfigBuilder.createFrom(OpCodeTracerConfig.DEFAULT)
+            .traceMemory(true)
+            .traceStack(false)
+            .traceStorage(false)
+            .build();
+    return new DebugOperationTracer(config, false);
+  }
 }