Refactor BlockHashOperation block height check into Operation (#8091)

* Refactor BlockHashOperation block height check into Operation

Refactor the 4 different places block height is checked across Besu into
the BlockHashOperation. Add support to make the lookback range
configurable in the BlockHashLookup interface, but default it to 256.

Signed-off-by: Danno Ferrin <danno@numisight.com>

* spotless

Signed-off-by: Danno Ferrin <danno@numisight.com>

* javadoc

Signed-off-by: Danno Ferrin <danno@numisight.com>

* Update Unit Tests to use the operation.

Signed-off-by: Danno Ferrin <danno@numisight.com>

* Update Unit Tests to use the operation.

Signed-off-by: Danno Ferrin <danno@numisight.com>

---------

Signed-off-by: Danno Ferrin <danno@numisight.com>
This commit is contained in:
Danno Ferrin
2025-01-13 08:51:11 -07:00
committed by GitHub
parent f9776cbf55
commit 0810b73ee3
8 changed files with 72 additions and 45 deletions

View File

@@ -35,16 +35,12 @@ import java.util.Map;
* all transactions within that block.
*/
public class BlockchainBasedBlockHashLookup implements BlockHashLookup {
private static final int MAX_RELATIVE_BLOCK = 256;
private final long currentBlockNumber;
private ProcessableBlockHeader searchStartHeader;
private final Blockchain blockchain;
private final Map<Long, Hash> hashByNumber = new HashMap<>();
public BlockchainBasedBlockHashLookup(
final ProcessableBlockHeader currentBlock, final Blockchain blockchain) {
this.currentBlockNumber = currentBlock.getNumber();
this.searchStartHeader = currentBlock;
this.blockchain = blockchain;
hashByNumber.put(currentBlock.getNumber() - 1, currentBlock.getParentHash());
@@ -52,14 +48,6 @@ public class BlockchainBasedBlockHashLookup implements BlockHashLookup {
@Override
public Hash apply(final MessageFrame frame, final Long blockNumber) {
// If the current block is the genesis block or the sought block is
// not within the last 256 completed blocks, zero is returned.
if (currentBlockNumber == 0
|| blockNumber >= currentBlockNumber
|| blockNumber < (currentBlockNumber - MAX_RELATIVE_BLOCK)) {
return ZERO;
}
final Hash cachedHash = hashByNumber.get(blockNumber);
if (cachedHash != null) {
return cachedHash;

View File

@@ -74,13 +74,6 @@ public class Eip7709BlockHashLookup implements BlockHashLookup {
@Override
public Hash apply(final MessageFrame frame, final Long blockNumber) {
final long currentBlockNumber = frame.getBlockValues().getNumber();
final long minBlockServe = Math.max(0, currentBlockNumber - blockHashServeWindow);
if (blockNumber >= currentBlockNumber || blockNumber < minBlockServe) {
LOG.trace("failed to read hash from system account for block {}", blockNumber);
return ZERO;
}
final Hash cachedHash = hashByNumber.get(blockNumber);
if (cachedHash != null) {
return cachedHash;
@@ -105,4 +98,9 @@ public class Eip7709BlockHashLookup implements BlockHashLookup {
hashByNumber.put(blockNumber, blockHash);
return blockHash;
}
@Override
public long getLookback() {
return blockHashServeWindow;
}
}

View File

@@ -16,6 +16,7 @@ package org.hyperledger.besu.ethereum.vm;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
@@ -27,10 +28,14 @@ import org.hyperledger.besu.ethereum.chain.Blockchain;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
import org.hyperledger.besu.evm.frame.BlockValues;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.CancunGasCalculator;
import org.hyperledger.besu.evm.operation.BlockHashOperation;
import java.util.Optional;
import org.apache.tuweni.bytes.Bytes;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -47,6 +52,7 @@ class BlockchainBasedBlockHashLookupTest {
private BlockHeader[] headers;
private BlockHashLookup lookup;
private final MessageFrame messageFrameMock = mock(MessageFrame.class);
private final BlockValues blockValuesMock = mock(BlockValues.class);
@BeforeEach
void setUp() {
@@ -138,7 +144,18 @@ class BlockchainBasedBlockHashLookupTest {
}
private void assertHashForBlockNumber(final int blockNumber, final Hash hash) {
Assertions.assertThat(lookup.apply(messageFrameMock, (long) blockNumber)).isEqualTo(hash);
clearInvocations(messageFrameMock, blockValuesMock);
BlockHashOperation op = new BlockHashOperation(new CancunGasCalculator());
when(messageFrameMock.getRemainingGas()).thenReturn(10_000_000L);
when(messageFrameMock.popStackItem()).thenReturn(Bytes.ofUnsignedInt(blockNumber));
when(messageFrameMock.getBlockValues()).thenReturn(blockValuesMock);
when(messageFrameMock.getBlockHashLookup()).thenReturn(lookup);
when(blockValuesMock.getNumber()).thenReturn((long) CURRENT_BLOCK_NUMBER);
op.execute(messageFrameMock, null);
verify(messageFrameMock).pushStackItem(hash);
}
private BlockHeader createHeader(final int blockNumber, final BlockHeader parentHeader) {

View File

@@ -15,7 +15,6 @@
package org.hyperledger.besu.ethereum.vm;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
@@ -35,17 +34,19 @@ import org.hyperledger.besu.evm.fluent.SimpleAccount;
import org.hyperledger.besu.evm.fluent.SimpleWorld;
import org.hyperledger.besu.evm.frame.BlockValues;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.CancunGasCalculator;
import org.hyperledger.besu.evm.operation.BlockHashOperation;
import org.hyperledger.besu.evm.worldstate.WorldUpdater;
import java.util.ArrayList;
import java.util.List;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
public class Eip7709BlockHashLookupTest {
class Eip7709BlockHashLookupTest {
private static final long BLOCKHASH_SERVE_WINDOW = 160;
private static final Address STORAGE_ADDRESS = Address.fromHexString("0x0");
private static final long HISTORY_SERVE_WINDOW = 200L;
@@ -58,9 +59,9 @@ public class Eip7709BlockHashLookupTest {
@BeforeEach
void setUp() {
headers = new ArrayList<>();
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, createWorldUpdater(0, CURRENT_BLOCK_NUMBER));
lookup =
new Eip7709BlockHashLookup(STORAGE_ADDRESS, HISTORY_SERVE_WINDOW, BLOCKHASH_SERVE_WINDOW);
frame = createMessageFrame(CURRENT_BLOCK_NUMBER, createWorldUpdater(0, CURRENT_BLOCK_NUMBER));
}
private WorldUpdater createWorldUpdater(final int fromBlockNumber, final int toBlockNumber) {
@@ -84,6 +85,8 @@ public class Eip7709BlockHashLookupTest {
final BlockValues blockValues = mock(BlockValues.class);
when(blockValues.getNumber()).thenReturn(currentBlockNumber);
when(messageFrame.getBlockValues()).thenReturn(blockValues);
when(messageFrame.getBlockHashLookup()).thenReturn(lookup);
when(messageFrame.getRemainingGas()).thenReturn(10_000_000L);
when(messageFrame.getWorldUpdater()).thenReturn(worldUpdater);
return messageFrame;
}
@@ -100,7 +103,7 @@ public class Eip7709BlockHashLookupTest {
@Test
void shouldReturnEmptyHashWhenRequestedBlockHigherThanHead() {
assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
assertHashForBlockNumber(CURRENT_BLOCK_NUMBER + 20, Hash.ZERO);
}
@Test
@@ -138,12 +141,9 @@ public class Eip7709BlockHashLookupTest {
assertHashForBlockNumber(blockNumber3);
assertHashForBlockNumber(blockNumber3);
verify(account, times(1))
.getStorageValue(eq(UInt256.valueOf(blockNumber1 % HISTORY_SERVE_WINDOW)));
verify(account, times(1))
.getStorageValue(eq(UInt256.valueOf(blockNumber2 % HISTORY_SERVE_WINDOW)));
verify(account, times(1))
.getStorageValue(eq(UInt256.valueOf(blockNumber3 % HISTORY_SERVE_WINDOW)));
verify(account, times(1)).getStorageValue(UInt256.valueOf(blockNumber1 % HISTORY_SERVE_WINDOW));
verify(account, times(1)).getStorageValue(UInt256.valueOf(blockNumber2 % HISTORY_SERVE_WINDOW));
verify(account, times(1)).getStorageValue(UInt256.valueOf(blockNumber3 % HISTORY_SERVE_WINDOW));
verifyNoMoreInteractions(account);
}
@@ -177,7 +177,14 @@ public class Eip7709BlockHashLookupTest {
}
private void assertHashForBlockNumber(final int blockNumber, final Hash hash) {
Assertions.assertThat(lookup.apply(frame, (long) blockNumber)).isEqualTo(hash);
clearInvocations(frame);
BlockHashOperation op = new BlockHashOperation(new CancunGasCalculator());
when(frame.popStackItem()).thenReturn(Bytes.ofUnsignedInt(blockNumber));
op.execute(frame, null);
verify(frame).pushStackItem(hash);
}
private BlockHeader createHeader(final long blockNumber, final BlockHeader parentHeader) {

View File

@@ -271,9 +271,7 @@ public class StateTestSubCommand implements Runnable {
transaction,
blockHeader.getCoinbase(),
(__, blockNumber) ->
blockNumber >= blockHeader.getNumber()
? Hash.ZERO
: Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))),
Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))),
false,
TransactionValidationParams.processingBlock(),
tracer,

View File

@@ -395,13 +395,8 @@ public class T8nExecutor {
// from them so need to
// add in a manual BlockHashLookup
blockHashLookup =
(__, blockNumber) -> {
if (referenceTestEnv.getNumber() - blockNumber > 256L
|| blockNumber >= referenceTestEnv.getNumber()) {
return Hash.ZERO;
}
return referenceTestEnv.getBlockhashByNumber(blockNumber).orElse(Hash.ZERO);
};
(__, blockNumber) ->
referenceTestEnv.getBlockhashByNumber(blockNumber).orElse(Hash.ZERO);
}
result =
processor.processTransaction(

View File

@@ -25,4 +25,14 @@ import java.util.function.BiFunction;
* <p>Arg is the current executing message frame. The Result is the Hash, which may be zero based on
* lookup rules.
*/
public interface BlockHashLookup extends BiFunction<MessageFrame, Long, Hash> {}
public interface BlockHashLookup extends BiFunction<MessageFrame, Long, Hash> {
/**
* How far back from the current block are hash queries valid for? Default is 256.
*
* @return The number of blocks before the current that should return a hash value.
*/
default long getLookback() {
return 256L;
}
}

View File

@@ -17,11 +17,13 @@ package org.hyperledger.besu.evm.operation;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.evm.EVM;
import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
import org.hyperledger.besu.evm.frame.BlockValues;
import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
import org.hyperledger.besu.evm.frame.MessageFrame;
import org.hyperledger.besu.evm.gascalculator.GasCalculator;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
/** The Block hash operation. */
public class BlockHashOperation extends AbstractOperation {
@@ -50,9 +52,21 @@ public class BlockHashOperation extends AbstractOperation {
return new OperationResult(cost, null);
}
final long soughtBlock = blockArg.toLong();
final BlockValues blockValues = frame.getBlockValues();
final long currentBlockNumber = blockValues.getNumber();
final BlockHashLookup blockHashLookup = frame.getBlockHashLookup();
final Hash blockHash = blockHashLookup.apply(frame, blockArg.toLong());
frame.pushStackItem(blockHash);
// If the current block is the genesis block or the sought block is
// not within the lookback window, zero is returned.
if (currentBlockNumber == 0
|| soughtBlock >= currentBlockNumber
|| soughtBlock < (currentBlockNumber - blockHashLookup.getLookback())) {
frame.pushStackItem(Bytes32.ZERO);
} else {
final Hash blockHash = blockHashLookup.apply(frame, soughtBlock);
frame.pushStackItem(blockHash);
}
return new OperationResult(cost, null);
}