Skip to content

Commit

Permalink
Merge branch 'main' into t8nSupportParentBeaconRoot
Browse files Browse the repository at this point in the history
Signed-off-by: Justin Florentine <justin+github@florentine.us>
  • Loading branch information
jflo authored Oct 27, 2023
2 parents 7ce34c3 + edf23cb commit e3c3e95
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

### Additions and Improvements
- Ethereum Classic Spiral network upgrade [#6078](https://github.com/hyperledger/besu/pull/6078)
- Add a method to read from a `Memory` instance without altering its inner state [#6073](https://github.com/hyperledger/besu/pull/6073)

### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,22 @@ public void shouldSetMemoryWhenLengthGreaterThanSourceLength() {
assertThat(memory.getWord(64)).isEqualTo(Bytes32.ZERO);
}

@Test
public void shouldNotIncreaseActiveWordsIfGetBytesWithoutGrowth() {
final Bytes value = Bytes.concatenate(WORD1, WORD2);
memory.setBytes(0, value.size(), value);
final int initialActiveWords = memory.getActiveWords();

assertThat(memory.getBytesWithoutGrowth(64, Bytes32.SIZE)).isEqualTo((Bytes32.ZERO));
assertThat(memory.getActiveWords()).isEqualTo(initialActiveWords);

assertThat(memory.getBytes(32, Bytes32.SIZE)).isEqualTo((WORD2));
assertThat(memory.getActiveWords()).isEqualTo(initialActiveWords);

assertThat(memory.getBytes(64, Bytes32.SIZE)).isEqualTo((Bytes32.ZERO));
assertThat(memory.getActiveWords()).isEqualTo(initialActiveWords + 1);
}

@Test
public void shouldClearMemoryAfterSourceDataWhenLengthGreaterThanSourceLength() {
memory.setWord(64, WORD3);
Expand Down
35 changes: 33 additions & 2 deletions evm/src/main/java/org/hyperledger/besu/evm/frame/Memory.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.util.Arrays;

import com.google.common.annotations.VisibleForTesting;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.bytes.MutableBytes;
Expand Down Expand Up @@ -179,7 +180,8 @@ int getActiveBytes() {
*
* @return The current number of active words stored in memory.
*/
int getActiveWords() {
@VisibleForTesting
public int getActiveWords() {
return activeWords;
}

Expand All @@ -202,11 +204,40 @@ public Bytes getBytes(final long location, final long numBytes) {
}

final int start = asByteIndex(location);

ensureCapacityForBytes(start, length);
return Bytes.wrap(Arrays.copyOfRange(memBytes, start, start + length));
}

/**
* Returns a copy of bytes by peeking into memory without expanding the active words.
*
* @param location The location in memory to start with.
* @param numBytes The number of bytes to get.
* @return A fresh copy of the bytes from memory starting at {@code location} and extending {@code
* numBytes}.
*/
public Bytes getBytesWithoutGrowth(final long location, final long numBytes) {
// Note: if length == 0, we don't require any memory expansion, whatever location is. So
// we must call asByteIndex(location) after this check so as it doesn't throw if the location
// is too big but the length is 0 (which is somewhat nonsensical, but is exercise by some
// tests).
final int length = asByteLength(numBytes);
if (length == 0) {
return Bytes.EMPTY;
}

final int start = asByteIndex(location);

// Arrays.copyOfRange would throw if start > memBytes.length, so just return the expected
// number of zeros without expanding the memory.
// Otherwise, just follow the happy path.
if (start > memBytes.length) {
return Bytes.wrap(new byte[(int) numBytes]);
} else {
return Bytes.wrap(Arrays.copyOfRange(memBytes, start, start + length));
}
}

/**
* Returns a copy of bytes from memory.
*
Expand Down
11 changes: 11 additions & 0 deletions evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,17 @@ public MutableBytes readMutableMemory(final long offset, final long length) {
return readMutableMemory(offset, length, false);
}

/**
* Read bytes in memory without expanding the word capacity.
*
* @param offset The offset in memory
* @param length The length of the bytes to read
* @return The bytes in the specified range
*/
public Bytes shadowReadMemory(final long offset, final long length) {
return memory.getBytesWithoutGrowth(offset, length);
}

/**
* Read bytes in memory .
*
Expand Down
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
@@ -0,0 +1,91 @@
/*
* Copyright Hyperledger Besu Contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.evm.frame;

import static org.assertj.core.api.Assertions.assertThat;

import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.evm.code.CodeV0;
import org.hyperledger.besu.evm.toy.ToyBlockValues;
import org.hyperledger.besu.evm.toy.ToyWorld;

import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.junit.jupiter.MockitoExtension;

@ExtendWith(MockitoExtension.class)
class MessageFrameTest {

private static final Bytes32 WORD1 = Bytes32.fromHexString(Long.toString(1).repeat(64));
private static final Bytes32 WORD2 = Bytes32.fromHexString(Long.toString(2).repeat(64));

private MessageFrame.Builder messageFrameBuilder;

@BeforeEach
void setUp() {
messageFrameBuilder =
MessageFrame.builder()
.worldUpdater(new ToyWorld())
.originator(Address.ZERO)
.gasPrice(Wei.ONE)
.blobGasPrice(Wei.ONE)
.blockValues(new ToyBlockValues())
.miningBeneficiary(Address.ZERO)
.blockHashLookup((l) -> Hash.ZERO)
.type(MessageFrame.Type.MESSAGE_CALL)
.initialGas(1)
.address(Address.ZERO)
.contract(Address.ZERO)
.inputData(Bytes32.ZERO)
.sender(Address.ZERO)
.value(Wei.ZERO)
.apparentValue(Wei.ZERO)
.code(CodeV0.EMPTY_CODE)
.completer(messageFrame -> {});
}

@Test
void shouldNotExpandMemory() {
final MessageFrame messageFrame = messageFrameBuilder.build();

final Bytes value = Bytes.concatenate(WORD1, WORD2);
messageFrame.writeMemory(0, value.size(), value);
int initialActiveWords = messageFrame.memoryWordSize();

// Fully in bounds read
assertThat(messageFrame.shadowReadMemory(64, Bytes32.SIZE)).isEqualTo(Bytes32.ZERO);
assertThat(messageFrame.memoryWordSize()).isEqualTo(initialActiveWords);

// Straddling read
final Bytes straddlingRead = messageFrame.shadowReadMemory(50, Bytes32.SIZE);
assertThat(messageFrame.memoryWordSize()).isEqualTo(initialActiveWords);
assertThat(straddlingRead.get(0)).isEqualTo((byte) 0x22); // Still in WORD2
assertThat(straddlingRead.get(13)).isEqualTo((byte) 0x22); // Just before uninitialized memory
assertThat(straddlingRead.get(14)).isEqualTo((byte) 0); // Just in uninitialized memory
assertThat(straddlingRead.get(20)).isEqualTo((byte) 0); // In uninitialized memory

// Fully out of bounds read
assertThat(messageFrame.shadowReadMemory(64, Bytes32.SIZE)).isEqualTo(Bytes32.ZERO);
assertThat(messageFrame.memoryWordSize()).isEqualTo(initialActiveWords);

assertThat(messageFrame.shadowReadMemory(32, Bytes32.SIZE)).isEqualTo(WORD2);
assertThat(messageFrame.memoryWordSize()).isEqualTo(initialActiveWords);
}
}
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 e3c3e95

Please sign in to comment.