Skip to content

Commit

Permalink
feat: add a way to read memory without altering the word capacity (hy…
Browse files Browse the repository at this point in the history
…perledger#6073)

* feat: add a way to read memory without altering the word capacity
* add tests
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

* Fix read-past-end
* Do not abuse method overload
* Update CHANGELOG.md

Signed-off-by: delehef <github@odena.eu>
Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

* add tests for MessageFrame.shadowReadMemory

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

* Straddled reads tests

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>

---------

Signed-off-by: Franklin Delehelle <franklin.delehelle@odena.eu>
Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>
Signed-off-by: delehef <github@odena.eu>
Signed-off-by: delehef <franklin.delehelle@odena.eu>
Co-authored-by: Daniel Lehrner <daniel.lehrner@consensys.net>
(cherry picked from commit edf23cb)
  • Loading branch information
delehef authored and jflo committed Nov 10, 2023
1 parent 36c87e9 commit f97d4b4
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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
@@ -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);
}
}

0 comments on commit f97d4b4

Please sign in to comment.