Skip to content

Commit

Permalink
Trigger contextEnter/Exit for all frames, including root (hyperledger…
Browse files Browse the repository at this point in the history
…#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>
  • Loading branch information
delehef and macfarla authored Oct 26, 2023
1 parent f58f6cf commit 0345b24
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -175,6 +156,7 @@ enum TRACE_TYPE {
PRE_EXECUTION,
POST_EXECUTION,
CONTEXT_ENTER,
CONTEXT_RE_ENTER,
CONTEXT_EXIT
}

Expand All @@ -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);
Expand Down

0 comments on commit 0345b24

Please sign in to comment.