Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add a way to read memory without altering the word capacity #6073

Merged
merged 8 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
delehef marked this conversation as resolved.
Show resolved Hide resolved

### 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() {
delehef marked this conversation as resolved.
Show resolved Hide resolved
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 {
delehef marked this conversation as resolved.
Show resolved Hide resolved
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
@@ -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);
}
}
Loading