From b08c0b37eb858b96c0a357a5a6c9188e71becb7d Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 8 Jan 2025 20:59:27 -0700 Subject: [PATCH 1/5] 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 --- .../vm/BlockchainBasedBlockHashLookup.java | 12 ------------ .../ethereum/vm/Eip7709BlockHashLookup.java | 12 +++++------- .../besu/evmtool/StateTestSubCommand.java | 5 +---- .../hyperledger/besu/evmtool/T8nExecutor.java | 9 ++------- .../besu/evm/blockhash/BlockHashLookup.java | 7 ++++++- .../besu/evm/operation/BlockHashOperation.java | 18 ++++++++++++++++-- 6 files changed, 30 insertions(+), 33 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java index 9c09956aba9..248a4216fd9 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java @@ -35,16 +35,12 @@ * 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 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 BlockchainBasedBlockHashLookup( @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; diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookup.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookup.java index 0627a416109..1b9fca3bf76 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookup.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookup.java @@ -74,13 +74,6 @@ public Eip7709BlockHashLookup(final Address contractAddress, final long historyS @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 Hash apply(final MessageFrame frame, final Long blockNumber) { hashByNumber.put(blockNumber, blockHash); return blockHash; } + + @Override + public long getLookback() { + return blockHashServeWindow; + } } diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java index b15aec6b9d0..6e4f05eaa61 100644 --- a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java +++ b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java @@ -270,10 +270,7 @@ private void traceTestSpecs(final String test, final List - blockNumber >= blockHeader.getNumber() - ? Hash.ZERO - : Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))), + (__, blockNumber) -> Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))), false, TransactionValidationParams.processingBlock(), tracer, diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java index 14f1cce978c..7a5650792fd 100644 --- a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java +++ b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/T8nExecutor.java @@ -395,13 +395,8 @@ static T8nResult runTest( // 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( diff --git a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java index 47f9414a4a9..54c02653afd 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java @@ -25,4 +25,9 @@ *

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 {} +public interface BlockHashLookup extends BiFunction { + + default long getLookback() { + return 256L; + } +} diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/BlockHashOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/BlockHashOperation.java index c88dc137093..60a16229e35 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/operation/BlockHashOperation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/BlockHashOperation.java @@ -17,11 +17,13 @@ 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 OperationResult execute(final MessageFrame frame, final EVM evm) { 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); } From 25c938420e55462be4e847076218bdb22175ccd3 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 8 Jan 2025 22:37:51 -0700 Subject: [PATCH 2/5] spotless Signed-off-by: Danno Ferrin --- .../org/hyperledger/besu/evmtool/StateTestSubCommand.java | 3 ++- .../org/hyperledger/besu/evm/blockhash/BlockHashLookup.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java index 6e4f05eaa61..07dbf872280 100644 --- a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java +++ b/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/StateTestSubCommand.java @@ -270,7 +270,8 @@ private void traceTestSpecs(final String test, final List Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))), + (__, blockNumber) -> + Hash.hash(Bytes.wrap(Long.toString(blockNumber).getBytes(UTF_8))), false, TransactionValidationParams.processingBlock(), tracer, diff --git a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java index 54c02653afd..eb1f8c43b0c 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java @@ -27,7 +27,7 @@ */ public interface BlockHashLookup extends BiFunction { - default long getLookback() { - return 256L; - } + default long getLookback() { + return 256L; + } } From 104ca31a9fe99e337e480565d9db7ac343037202 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Wed, 8 Jan 2025 22:54:17 -0700 Subject: [PATCH 3/5] javadoc Signed-off-by: Danno Ferrin --- .../org/hyperledger/besu/evm/blockhash/BlockHashLookup.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java index eb1f8c43b0c..cd27843de9b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/blockhash/BlockHashLookup.java @@ -27,6 +27,11 @@ */ public interface BlockHashLookup extends BiFunction { + /** + * 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; } From 5c19bbaaddc043d60efb609f6b6822897eaf5fdb Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Thu, 9 Jan 2025 11:17:12 -0700 Subject: [PATCH 4/5] Update Unit Tests to use the operation. Signed-off-by: Danno Ferrin --- .../BlockchainBasedBlockHashLookupTest.java | 19 ++++++++++++- .../vm/Eip7709BlockHashLookupTest.java | 28 +++++++++++++------ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java index 8504d613967..b9e70ee99c2 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java @@ -16,6 +16,7 @@ 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.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 @@ private void assertHashForBlockNumber(final int blockNumber) { } 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) { diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java index d9cf91d78b8..e2b95d3df49 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java @@ -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.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 @@ private MessageFrame createMessageFrame( 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 @@ void shouldGetHashOfMaxBlocksBehind() { @Test void shouldReturnEmptyHashWhenRequestedBlockHigherThanHead() { - assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO); + assertHashForBlockNumber(CURRENT_BLOCK_NUMBER + 20, Hash.ZERO); } @Test @@ -139,11 +142,11 @@ void shouldCacheBlockHashes() { assertHashForBlockNumber(blockNumber3); verify(account, times(1)) - .getStorageValue(eq(UInt256.valueOf(blockNumber1 % HISTORY_SERVE_WINDOW))); + .getStorageValue(UInt256.valueOf(blockNumber1 % HISTORY_SERVE_WINDOW)); verify(account, times(1)) - .getStorageValue(eq(UInt256.valueOf(blockNumber2 % HISTORY_SERVE_WINDOW))); + .getStorageValue(UInt256.valueOf(blockNumber2 % HISTORY_SERVE_WINDOW)); verify(account, times(1)) - .getStorageValue(eq(UInt256.valueOf(blockNumber3 % HISTORY_SERVE_WINDOW))); + .getStorageValue(UInt256.valueOf(blockNumber3 % HISTORY_SERVE_WINDOW)); verifyNoMoreInteractions(account); } @@ -177,7 +180,14 @@ private void assertHashForBlockNumber(final int blockNumber) { } 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) { From 65d119287ce6ac4cd71bd63cccdba8a47201eee6 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Thu, 9 Jan 2025 11:23:08 -0700 Subject: [PATCH 5/5] Update Unit Tests to use the operation. Signed-off-by: Danno Ferrin --- .../besu/ethereum/vm/Eip7709BlockHashLookupTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java index e2b95d3df49..163b76e0f7b 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/Eip7709BlockHashLookupTest.java @@ -141,12 +141,9 @@ void shouldCacheBlockHashes() { assertHashForBlockNumber(blockNumber3); assertHashForBlockNumber(blockNumber3); - 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)); + 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); }