diff --git a/CHANGELOG.md b/CHANGELOG.md index e264da3056f..cbc9246d66c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/MemoryTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/MemoryTest.java index ab254a3b0c7..d37a1fccf54 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/MemoryTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/MemoryTest.java @@ -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); diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/Memory.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/Memory.java index ac5aa396b88..fe2bba89cf5 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/Memory.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/Memory.java @@ -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; @@ -179,7 +180,8 @@ int getActiveBytes() { * * @return The current number of active words stored in memory. */ - int getActiveWords() { + @VisibleForTesting + public int getActiveWords() { return activeWords; } @@ -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. * diff --git a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java index 9eca8d88429..f8e9d3d6121 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/frame/MessageFrame.java @@ -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 . * diff --git a/evm/src/test/java/org/hyperledger/besu/evm/frame/MessageFrameTest.java b/evm/src/test/java/org/hyperledger/besu/evm/frame/MessageFrameTest.java new file mode 100644 index 00000000000..9d175ff6f9a --- /dev/null +++ b/evm/src/test/java/org/hyperledger/besu/evm/frame/MessageFrameTest.java @@ -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); + } +}