fixup! eip-7709 implement BLOCKHASH opcode from system contract state

reimplement blockhashlookup with MessageFrame instead of WorldUpdater Signed-off-by: Luis Pinto <[email protected]>
hyperledger · Dec 4, 2024 · 04678ff · 04678ff
1 parent 366d6bd
commit 04678ff
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 23 deletions.
diff --git a/...m/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java b/...m/core/src/main/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookup.java
@@ -20,8 +20,8 @@
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
+import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.operation.BlockHashOperation;
-import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -51,7 +51,7 @@ public BlockchainBasedBlockHashLookup(
   }
 
   @Override
-  public Hash apply(final WorldUpdater worldUpdater, final Long blockNumber) {
+  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

diff --git a/...eum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java b/...eum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java
@@ -21,6 +21,7 @@
 import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.evm.account.Account;
 import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
+import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import java.util.HashMap;
@@ -81,7 +82,7 @@ public ContractBasedBlockHashLookup(
   }
 
   @Override
-  public Hash apply(final WorldUpdater worldUpdater, final Long blockNumber) {
+  public Hash apply(final MessageFrame frame, final Long blockNumber) {
     final long currentBlockNumber = blockHeader.getNumber();
     final long minBlockServe = Math.max(0, currentBlockNumber - blockHashServeWindow);
     if (blockNumber >= currentBlockNumber || blockNumber < minBlockServe) {
@@ -94,6 +95,7 @@ public Hash apply(final WorldUpdater worldUpdater, final Long blockNumber) {
       return cachedHash;
     }
 
+    final WorldUpdater worldUpdater = frame.getWorldUpdater();
     Account account = worldUpdater.get(contractAddress);
     if (account == null) {
       LOG.error("cannot query system contract {}", contractAddress);

diff --git a/...re/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java b/...re/src/test/java/org/hyperledger/besu/ethereum/vm/BlockchainBasedBlockHashLookupTest.java
@@ -27,7 +27,7 @@
 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.worldstate.WorldUpdater;
+import org.hyperledger.besu.evm.frame.MessageFrame;
 
 import java.util.Optional;
 
@@ -46,7 +46,7 @@ class BlockchainBasedBlockHashLookupTest {
   private final Blockchain blockchain = mock(Blockchain.class);
   private BlockHeader[] headers;
   private BlockHashLookup lookup;
-  private final WorldUpdater worldUpdaterMock = mock(WorldUpdater.class);
+  private final MessageFrame messageFrameMock = mock(MessageFrame.class);
 
   @BeforeEach
   void setUp() {
@@ -86,7 +86,7 @@ void shouldGetHashOfMaxBlocksBehind() {
 
   @Test
   void shouldReturnEmptyHashWhenRequestedBlockNotOnchain() {
-    assertThat(lookup.apply(worldUpdaterMock, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
+    assertThat(lookup.apply(messageFrameMock, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
   }
 
   @Test
@@ -96,7 +96,7 @@ void shouldReturnEmptyHashWhenParentBlockNotOnchain() {
             new BlockHeaderTestFixture().number(CURRENT_BLOCK_NUMBER + 20).buildHeader(),
             blockchain);
     Assertions.assertThat(
-            lookupWithUnavailableParent.apply(worldUpdaterMock, (long) CURRENT_BLOCK_NUMBER))
+            lookupWithUnavailableParent.apply(messageFrameMock, (long) CURRENT_BLOCK_NUMBER))
         .isEqualTo(Hash.ZERO);
   }
 
@@ -138,7 +138,7 @@ private void assertHashForBlockNumber(final int blockNumber) {
   }
 
   private void assertHashForBlockNumber(final int blockNumber, final Hash hash) {
-    Assertions.assertThat(lookup.apply(worldUpdaterMock, (long) blockNumber)).isEqualTo(hash);
+    Assertions.assertThat(lookup.apply(messageFrameMock, (long) blockNumber)).isEqualTo(hash);
   }
 
   private BlockHeader createHeader(final int blockNumber, final BlockHeader parentHeader) {

diff --git a/...core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java b/...core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java
@@ -33,6 +33,7 @@
 import org.hyperledger.besu.evm.blockhash.BlockHashLookup;
 import org.hyperledger.besu.evm.fluent.SimpleAccount;
 import org.hyperledger.besu.evm.fluent.SimpleWorld;
+import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import java.util.ArrayList;
@@ -51,12 +52,14 @@ public class ContractBasedBlockHashLookupTest {
       Math.toIntExact(HISTORY_SERVE_WINDOW + BLOCKHASH_SERVE_WINDOW / 2);
   private List<BlockHeader> headers;
   private BlockHashLookup lookup;
-  private WorldUpdater worldUpdater;
+  private MessageFrame frame;
 
   @BeforeEach
   void setUp() {
     headers = new ArrayList<>();
-    worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
+    frame = mock(MessageFrame.class);
+    final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
+    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
     lookup =
         new ContractBasedBlockHashLookup(
             createHeader(CURRENT_BLOCK_NUMBER, headers.getLast()),
@@ -92,18 +95,21 @@ void shouldGetHashOfMaxBlocksBehind() {
 
   @Test
   void shouldReturnEmptyHashWhenRequestedBlockHigherThanHead() {
-    assertThat(lookup.apply(worldUpdater, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
+    assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
   }
 
   @Test
   void shouldReturnEmptyHashWhenSystemContractNotExists() {
-    worldUpdater = new SimpleWorld();
-    assertThat(lookup.apply(worldUpdater, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.ZERO);
+    final WorldUpdater worldUpdater = new SimpleWorld();
+    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+    assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.ZERO);
   }
 
   @Test
   void shouldReturnEmptyHashWhenParentBlockNotInContract() {
-    worldUpdater = createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
+    final WorldUpdater worldUpdater =
+        createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
+    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
     lookup =
         new ContractBasedBlockHashLookup(
             new BlockHeaderTestFixture().number(CURRENT_BLOCK_NUMBER).buildHeader(),
@@ -115,7 +121,9 @@ void shouldReturnEmptyHashWhenParentBlockNotInContract() {
 
   @Test
   void shouldCacheBlockHashes() {
-    Account account = worldUpdater.get(STORAGE_ADDRESS);
+    final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
+    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+    final Account account = worldUpdater.get(STORAGE_ADDRESS);
     clearInvocations(account);
 
     int blockNumber1 = CURRENT_BLOCK_NUMBER - 1;
@@ -139,7 +147,8 @@ void shouldCacheBlockHashes() {
 
   @Test
   void shouldGetHashWhenParentIsGenesis() {
-    worldUpdater = createWorldUpdater(0, 1);
+    final WorldUpdater worldUpdater = createWorldUpdater(0, 1);
+    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
     lookup =
         new ContractBasedBlockHashLookup(
             createHeader(1, headers.getFirst()),
@@ -171,7 +180,7 @@ private void assertHashForBlockNumber(final int blockNumber) {
   }
 
   private void assertHashForBlockNumber(final int blockNumber, final Hash hash) {
-    Assertions.assertThat(lookup.apply(worldUpdater, (long) blockNumber)).isEqualTo(hash);
+    Assertions.assertThat(lookup.apply(frame, (long) blockNumber)).isEqualTo(hash);
   }
 
   private BlockHeader createHeader(final long blockNumber, final BlockHeader parentHeader) {

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
@@ -15,7 +15,7 @@
 package org.hyperledger.besu.evm.blockhash;
 
 import org.hyperledger.besu.datatypes.Hash;
-import org.hyperledger.besu.evm.worldstate.WorldUpdater;
+import org.hyperledger.besu.evm.frame.MessageFrame;
 
 import java.util.function.BiFunction;
 
@@ -25,4 +25,4 @@
  * <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<WorldUpdater, Long, Hash> {}
+public interface BlockHashLookup extends BiFunction<MessageFrame, Long, Hash> {}
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
@@ -20,7 +20,6 @@
 import org.hyperledger.besu.evm.frame.ExceptionalHaltReason;
 import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.gascalculator.GasCalculator;
-import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import org.apache.tuweni.bytes.Bytes;
 
@@ -51,10 +50,8 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
       return new OperationResult(cost, null);
     }
 
-    final WorldUpdater worldUpdater = frame.getWorldUpdater();
-
     final BlockHashLookup blockHashLookup = frame.getBlockHashLookup();
-    final Hash blockHash = blockHashLookup.apply(worldUpdater, blockArg.toLong());
+    final Hash blockHash = blockHashLookup.apply(frame, blockArg.toLong());
     frame.pushStackItem(blockHash);
 
     return new OperationResult(cost, null);