Trigger contextEnter/Exit for all frames, including root (hyperledger…

…#6074) * Trigger contextEnter/Exit for all frames, including root * Differentiate between context entry and re-entry in `OperationTracer` * Update evm/src/test/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessorTest.java Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: delehef <github@odena.eu> --------- Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu> Signed-off-by: delehef <github@odena.eu> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Gabriel-Trintinalia · Oct 26, 2023 · 0345b24 · 0345b24
1 parent f58f6cf
commit 0345b24
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 36 deletions.
diff --git a/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java b/evm/src/main/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessor.java
@@ -184,8 +184,12 @@ private void codeExecute(final MessageFrame frame, final OperationTracer operati
    * @param operationTracer the operation tracer
    */
   public void process(final MessageFrame frame, final OperationTracer operationTracer) {
-    if (operationTracer != null && frame.getMessageStackSize() > 1) {
-      operationTracer.traceContextEnter(frame);
+    if (operationTracer != null) {
+      if (frame.getState() == MessageFrame.State.NOT_STARTED) {
+        operationTracer.traceContextEnter(frame);
+      } else {
+        operationTracer.traceContextReEnter(frame);
+      }
     }
 
     if (frame.getState() == MessageFrame.State.NOT_STARTED) {
@@ -213,13 +217,13 @@ public void process(final MessageFrame frame, final OperationTracer operationTra
     }
 
     if (frame.getState() == MessageFrame.State.COMPLETED_SUCCESS) {
-      if (operationTracer != null && frame.getMessageStackSize() > 1) {
+      if (operationTracer != null) {
         operationTracer.traceContextExit(frame);
       }
       completedSuccess(frame);
     }
     if (frame.getState() == MessageFrame.State.COMPLETED_FAILED) {
-      if (operationTracer != null && frame.getMessageStackSize() > 1) {
+      if (operationTracer != null) {
         operationTracer.traceContextExit(frame);
       }
       completedFailed(frame);

diff --git a/evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java b/evm/src/main/java/org/hyperledger/besu/evm/tracing/OperationTracer.java
@@ -102,6 +102,13 @@ default void traceEndTransaction(
    */
   default void traceContextEnter(final MessageFrame frame) {}
 
+  /**
+   * Trace the re-entry in a context from a child context
+   *
+   * @param frame the frame
+   */
+  default void traceContextReEnter(final MessageFrame frame) {}
+
   /**
    * Trace the exiting of a context
    *

diff --git a/evm/src/test/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessorTest.java b/evm/src/test/java/org/hyperledger/besu/evm/processor/AbstractMessageProcessorTest.java
@@ -17,9 +17,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hyperledger.besu.evm.processor.AbstractMessageProcessorTest.ContextTracer.TRACE_TYPE.CONTEXT_ENTER;
 import static org.hyperledger.besu.evm.processor.AbstractMessageProcessorTest.ContextTracer.TRACE_TYPE.CONTEXT_EXIT;
+import static org.hyperledger.besu.evm.processor.AbstractMessageProcessorTest.ContextTracer.TRACE_TYPE.CONTEXT_RE_ENTER;
 import static org.hyperledger.besu.evm.processor.AbstractMessageProcessorTest.ContextTracer.TRACE_TYPE.POST_EXECUTION;
 import static org.hyperledger.besu.evm.processor.AbstractMessageProcessorTest.ContextTracer.TRACE_TYPE.PRE_EXECUTION;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -30,8 +30,8 @@
 import org.hyperledger.besu.evm.fluent.EVMExecutor;
 import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.operation.Operation;
+import org.hyperledger.besu.evm.toy.ToyWorld;
 import org.hyperledger.besu.evm.tracing.OperationTracer;
-import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -41,59 +41,37 @@
 import org.apache.tuweni.bytes.Bytes;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
 abstract class AbstractMessageProcessorTest<T extends AbstractMessageProcessor> {
-
   @Mock MessageFrame messageFrame;
   @Mock OperationTracer operationTracer;
   @Mock Deque<MessageFrame> messageFrameStack;
-  @Mock WorldUpdater worldUpdater;
 
   protected abstract T getAbstractMessageProcessor();
 
-  @ParameterizedTest
-  @ValueSource(ints = {0, 1})
-  void shouldNotTraceContextIfStackSizeIsZero(final int stackSize) {
-    when(messageFrame.getMessageStackSize()).thenReturn(stackSize);
-    when(messageFrame.getState())
-        .thenReturn(MessageFrame.State.COMPLETED_SUCCESS, MessageFrame.State.COMPLETED_FAILED);
-    when(messageFrame.getMessageFrameStack()).thenReturn(messageFrameStack);
-
-    getAbstractMessageProcessor().process(messageFrame, operationTracer);
-
-    verify(operationTracer, never()).traceContextEnter(messageFrame);
-    verify(operationTracer, never()).traceContextExit(messageFrame);
-  }
-
-  @ParameterizedTest
-  @ValueSource(ints = {2, 3, 5, 15, Integer.MAX_VALUE})
-  void shouldTraceContextIfStackSizeIsGreaterZeroAndSuccess(final int stackSize) {
-    when(messageFrame.getMessageStackSize()).thenReturn(stackSize);
+  @Test
+  void shouldTraceExitForContext() {
+    when(messageFrame.getWorldUpdater()).thenReturn(new ToyWorld());
     when(messageFrame.getState()).thenReturn(MessageFrame.State.COMPLETED_SUCCESS);
     when(messageFrame.getMessageFrameStack()).thenReturn(messageFrameStack);
-    when(messageFrame.getWorldUpdater()).thenReturn(worldUpdater);
 
     getAbstractMessageProcessor().process(messageFrame, operationTracer);
 
-    verify(operationTracer, times(1)).traceContextEnter(messageFrame);
+    // As the only MessageFrame state will be COMPLETED_SUCCESS, only a contextExit is expected
     verify(operationTracer, times(1)).traceContextExit(messageFrame);
   }
 
-  @ParameterizedTest
-  @ValueSource(ints = {2, 3, 5, 15, Integer.MAX_VALUE})
-  void shouldTraceContextIfStackSizeIsGreaterZeroAndFailure(final int stackSize) {
-    when(messageFrame.getMessageStackSize()).thenReturn(stackSize);
+  @Test
+  void shouldTraceExitEvenIfContextFailed() {
     when(messageFrame.getState()).thenReturn(MessageFrame.State.COMPLETED_FAILED);
     when(messageFrame.getMessageFrameStack()).thenReturn(messageFrameStack);
 
     getAbstractMessageProcessor().process(messageFrame, operationTracer);
 
-    verify(operationTracer, times(1)).traceContextEnter(messageFrame);
+    // As the only MessageFrame state will be COMPLETED_FAILED, only a contextExit is expected
     verify(operationTracer, times(1)).traceContextExit(messageFrame);
   }
 
@@ -133,6 +111,7 @@ void shouldTraceContextEnterExitForEip3155Test() {
 
     final List<ContextTracer.TRACE_TYPE> expectedTraces =
         Arrays.asList(
+            CONTEXT_ENTER, // Entry in root context
             PRE_EXECUTION, // PUSH1
             POST_EXECUTION, // PUSH1
             PRE_EXECUTION, // DUP1
@@ -161,10 +140,12 @@ void shouldTraceContextEnterExitForEip3155Test() {
             POST_EXECUTION, // STATICCALL
             CONTEXT_ENTER, // STATICCALL
             CONTEXT_EXIT, // STATICCALL
+            CONTEXT_RE_ENTER, // Re-entry in root context post-STATICALL
             PRE_EXECUTION, // PUSH1
             POST_EXECUTION, // PUSH1
             PRE_EXECUTION, // RETURN
-            POST_EXECUTION // RETURN
+            POST_EXECUTION, // RETURN
+            CONTEXT_EXIT // Exiting root context
             );
 
     assertThat(contextTracer.traceHistory()).isEqualTo(expectedTraces);
@@ -175,6 +156,7 @@ enum TRACE_TYPE {
       PRE_EXECUTION,
       POST_EXECUTION,
       CONTEXT_ENTER,
+      CONTEXT_RE_ENTER,
       CONTEXT_EXIT
     }
 
@@ -196,6 +178,11 @@ public void traceContextEnter(final MessageFrame frame) {
       traceHistory.add(TRACE_TYPE.CONTEXT_ENTER);
     }
 
+    @Override
+    public void traceContextReEnter(final MessageFrame frame) {
+      traceHistory.add(CONTEXT_RE_ENTER);
+    }
+
     @Override
     public void traceContextExit(final MessageFrame frame) {
       traceHistory.add(TRACE_TYPE.CONTEXT_EXIT);