From 702cb818f0dc977d14b7756c711d16e6063b01e5 Mon Sep 17 00:00:00 2001
From: Luis Pinto <luis.pinto@consensys.net>
Date: Sat, 30 Nov 2024 22:35:05 +0000
Subject: [PATCH] EIP-7709 Implement gas costs for BLOCKHASH implementation
 with system contract

Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
---
 .../mainnet/MainnetProtocolSpecs.java         |  3 +-
 .../vm/ContractBasedBlockHashLookup.java      | 23 ++++-
 .../core/MessageFrameTestFixture.java         |  8 ++
 .../vm/ContractBasedBlockHashLookupTest.java  | 83 +++++++++++++++----
 .../evm/operation/BlockHashOperation.java     |  9 +-
 .../evm/operation/BlockHashOperationTest.java | 43 +++++++++-
 6 files changed, 147 insertions(+), 22 deletions(-)

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java
index 181a76b7296..73327228a16 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java
@@ -35,6 +35,7 @@
 import org.hyperledger.besu.ethereum.mainnet.ClearEmptyAccountStrategy.NotClearEmptyAccount;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpecBuilder.BlockValidatorBuilder;
 import org.hyperledger.besu.ethereum.mainnet.blockhash.CancunBlockHashProcessor;
+import org.hyperledger.besu.ethereum.mainnet.blockhash.Eip7709BlockHashProcessor;
 import org.hyperledger.besu.ethereum.mainnet.blockhash.FrontierBlockHashProcessor;
 import org.hyperledger.besu.ethereum.mainnet.blockhash.PragueBlockHashProcessor;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
@@ -905,7 +906,7 @@ static ProtocolSpecBuilder verkleDefinition(
                     CoinbaseFeePriceCalculator.eip1559()))
         .withdrawalsProcessor(new WithdrawalsProcessor(clearEmptyAccountStrategy))
         .executionWitnessValidator(new ExecutionWitnessValidator.AllowedExecutionWitness())
-        .blockHashProcessor(new PragueBlockHashProcessor())
+        .blockHashProcessor(new Eip7709BlockHashProcessor())
         .blockHeaderFunctions(new VerkleDevnetBlockHeaderFunctions())
         .name("Verkle");
   }
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java
index 8aab22d9d64..53ed8d2c1a3 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookup.java
@@ -38,6 +38,7 @@
 public class ContractBasedBlockHashLookup implements BlockHashLookup {
   private static final Logger LOG = LoggerFactory.getLogger(ContractBasedBlockHashLookup.class);
   private static final long BLOCKHASH_SERVE_WINDOW = 256L;
+  private static final long WARM_STORAGE_READ_COST = 100L;
 
   private final ProcessableBlockHeader blockHeader;
   private final Address contractAddress;
@@ -90,6 +91,13 @@ public Hash apply(final MessageFrame frame, final Long blockNumber) {
       return ZERO;
     }
 
+    final UInt256 slot = UInt256.valueOf(blockNumber % historyServeWindow);
+    final long cost = cost(frame, slot);
+    if (frame.getRemainingGas() < cost) {
+      return Hash.EMPTY;
+    }
+    frame.decrementRemainingGas(cost);
+
     final Hash cachedHash = hashByNumber.get(blockNumber);
     if (cachedHash != null) {
       return cachedHash;
@@ -102,7 +110,6 @@ public Hash apply(final MessageFrame frame, final Long blockNumber) {
       return ZERO;
     }
 
-    UInt256 slot = UInt256.valueOf(blockNumber % historyServeWindow);
     final UInt256 value = account.getStorageValue(slot);
     LOG.atTrace()
         .log(
@@ -114,4 +121,18 @@ public Hash apply(final MessageFrame frame, final Long blockNumber) {
     hashByNumber.put(blockNumber, blockHash);
     return blockHash;
   }
+
+  protected long cost(final MessageFrame frame, final UInt256 slot) {
+    long gas = frame.getAccessWitness().touchAndChargeStorageLoad(contractAddress, slot);
+
+    if (gas == 0) {
+      return getWarmStorageReadCost();
+    }
+
+    return gas;
+  }
+
+  protected long getWarmStorageReadCost() {
+    return WARM_STORAGE_READ_COST;
+  }
 }
diff --git a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java
index c4f492e3c80..31fd2f59a9d 100644
--- a/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java
+++ b/ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/MessageFrameTestFixture.java
@@ -16,6 +16,7 @@
 
 import static org.hyperledger.besu.evm.frame.MessageFrame.DEFAULT_MAX_STACK_SIZE;
 
+import org.hyperledger.besu.datatypes.AccessWitness;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
@@ -56,6 +57,7 @@ public class MessageFrameTestFixture {
   private Optional<BlockHeader> blockHeader = Optional.empty();
   private Optional<BlockHashLookup> blockHashLookup = Optional.empty();
   private ExecutionContextTestFixture executionContextTestFixture;
+  private AccessWitness accessWitness;
 
   public MessageFrameTestFixture parentFrame(final MessageFrame parentFrame) {
     this.parentFrame = parentFrame;
@@ -153,6 +155,11 @@ public MessageFrameTestFixture blockHashLookup(final BlockHashLookup blockHashLo
     return this;
   }
 
+  public MessageFrameTestFixture accessWitness(final AccessWitness accessWitness) {
+    this.accessWitness = accessWitness;
+    return this;
+  }
+
   public MessageFrame build() {
     final Blockchain localBlockchain = this.blockchain.orElseGet(this::createDefaultBlockchain);
     final BlockHeader localBlockHeader =
@@ -185,6 +192,7 @@ public MessageFrame build() {
                             .getBlockHashProcessor()
                             .createBlockHashLookup(localBlockchain, localBlockHeader)))
             .maxStackSize(maxStackSize)
+            .accessWitness(accessWitness)
             .build();
     stackItems.forEach(frame::pushStackItem);
     return frame;
diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java
index 0b8201bbe70..65ddbcdeddf 100644
--- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java
+++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/vm/ContractBasedBlockHashLookupTest.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.vm;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.mock;
@@ -24,16 +25,19 @@
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.datatypes.AccessWitness;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
+import org.hyperledger.besu.ethereum.core.MessageFrameTestFixture;
 import org.hyperledger.besu.evm.account.Account;
 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.gascalculator.stateless.NoopAccessWitness;
 import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 
 import java.util.ArrayList;
@@ -53,13 +57,18 @@ public class ContractBasedBlockHashLookupTest {
   private List<BlockHeader> headers;
   private BlockHashLookup lookup;
   private MessageFrame frame;
+  private WorldUpdater worldUpdater;
 
   @BeforeEach
   void setUp() {
     headers = new ArrayList<>();
-    frame = mock(MessageFrame.class);
-    final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
-    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+    worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .initialGas(Long.MAX_VALUE)
+            .accessWitness(NoopAccessWitness.get())
+            .build();
     lookup =
         new ContractBasedBlockHashLookup(
             createHeader(CURRENT_BLOCK_NUMBER, headers.getLast()),
@@ -94,22 +103,58 @@ void shouldGetHashOfMaxBlocksBehind() {
   }
 
   @Test
-  void shouldReturnEmptyHashWhenRequestedBlockHigherThanHead() {
+  void shouldReturnZeroHashWhenRequestedBlockHigherThanHead() {
     assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER + 20L)).isEqualTo(Hash.ZERO);
   }
 
   @Test
-  void shouldReturnEmptyHashWhenSystemContractNotExists() {
-    final WorldUpdater worldUpdater = new SimpleWorld();
-    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+  void shouldReturnZeroHashWhenSystemContractNotExists() {
+    worldUpdater = new SimpleWorld();
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(NoopAccessWitness.get())
+            .build();
     assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.ZERO);
   }
 
   @Test
-  void shouldReturnEmptyHashWhenParentBlockNotInContract() {
-    final WorldUpdater worldUpdater =
-        createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
-    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+  @SuppressWarnings("ReturnValueIgnored")
+  void shouldDecrementRemainingGasFromFrame() {
+    AccessWitness accessWitness = mock(AccessWitness.class);
+    when(accessWitness.touchAndChargeStorageLoad(any(), any())).thenReturn(100L);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(accessWitness)
+            .initialGas(200L)
+            .build();
+    lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L);
+    assertThat(frame.getRemainingGas()).isEqualTo(100L);
+  }
+
+  @Test
+  void insufficientGasReturnsEmptyHash() {
+    worldUpdater = new SimpleWorld();
+    AccessWitness accessWitness = mock(AccessWitness.class);
+    when(accessWitness.touchAndChargeStorageLoad(any(), any())).thenReturn(100L);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(accessWitness)
+            .initialGas(1)
+            .build();
+    assertThat(lookup.apply(frame, CURRENT_BLOCK_NUMBER - 1L)).isEqualTo(Hash.EMPTY);
+  }
+
+  @Test
+  void shouldReturnZeroHashWhenParentBlockNotInContract() {
+    worldUpdater = createWorldUpdater(CURRENT_BLOCK_NUMBER - 10, CURRENT_BLOCK_NUMBER);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(NoopAccessWitness.get())
+            .build();
     lookup =
         new ContractBasedBlockHashLookup(
             new BlockHeaderTestFixture().number(CURRENT_BLOCK_NUMBER).buildHeader(),
@@ -121,8 +166,12 @@ void shouldReturnEmptyHashWhenParentBlockNotInContract() {
 
   @Test
   void shouldCacheBlockHashes() {
-    final WorldUpdater worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
-    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+    worldUpdater = createWorldUpdater(0, CURRENT_BLOCK_NUMBER);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(NoopAccessWitness.get())
+            .build();
     final Account account = worldUpdater.get(STORAGE_ADDRESS);
     clearInvocations(account);
 
@@ -147,8 +196,12 @@ void shouldCacheBlockHashes() {
 
   @Test
   void shouldGetHashWhenParentIsGenesis() {
-    final WorldUpdater worldUpdater = createWorldUpdater(0, 1);
-    when(frame.getWorldUpdater()).thenReturn(worldUpdater);
+    worldUpdater = createWorldUpdater(0, 1);
+    frame =
+        new MessageFrameTestFixture()
+            .worldUpdater(worldUpdater)
+            .accessWitness(NoopAccessWitness.get())
+            .build();
     lookup =
         new ContractBasedBlockHashLookup(
             createHeader(1, headers.getFirst()),
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..67edc590421 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
@@ -50,11 +50,18 @@ public OperationResult execute(final MessageFrame frame, final EVM evm) {
       return new OperationResult(cost, null);
     }
 
+    final long remainingGas = frame.getRemainingGas();
     final BlockHashLookup blockHashLookup = frame.getBlockHashLookup();
     final Hash blockHash = blockHashLookup.apply(frame, blockArg.toLong());
+    final long lookupCost = remainingGas - frame.getRemainingGas();
+    if (Hash.EMPTY.equals(blockHash)) {
+      return new OperationResult(cost + lookupCost, ExceptionalHaltReason.INSUFFICIENT_GAS);
+    }
+    // give lookupCost back as it will be taken after
+    frame.incrementRemainingGas(lookupCost);
     frame.pushStackItem(blockHash);
 
-    return new OperationResult(cost, null);
+    return new OperationResult(cost + lookupCost, null);
   }
 
   /**
diff --git a/evm/src/test/java/org/hyperledger/besu/evm/operation/BlockHashOperationTest.java b/evm/src/test/java/org/hyperledger/besu/evm/operation/BlockHashOperationTest.java
index 9fd5c3243f8..fcba0c713f1 100644
--- a/evm/src/test/java/org/hyperledger/besu/evm/operation/BlockHashOperationTest.java
+++ b/evm/src/test/java/org/hyperledger/besu/evm/operation/BlockHashOperationTest.java
@@ -46,7 +46,7 @@ void shouldReturnZeroWhenArgIsBiggerThanALong() {
   }
 
   @Test
-  void shouldReturnBlockHashUsingLookupFromFrameWhenItIsWithinTheAllowedRange() {
+  void returnsBlockHashWhenWithinAllowedRange() {
     final Hash blockHash = Hash.hash(Bytes.fromHexString("0x1293487297"));
     assertBlockHash(
         100,
@@ -56,6 +56,23 @@ void shouldReturnBlockHashUsingLookupFromFrameWhenItIsWithinTheAllowedRange() {
         ENOUGH_GAS);
   }
 
+  @Test
+  void remainingGasDoesNotChange() {
+    final Hash blockHash = Hash.hash(Bytes.fromHexString("0x1293487297"));
+    assertBlockHash(
+        100,
+        blockHash,
+        200,
+        (frame, block) -> {
+          frame.decrementRemainingGas(100L);
+          if (block == 100) {
+            return blockHash;
+          }
+          return Hash.ZERO;
+        },
+        ENOUGH_GAS);
+  }
+
   @Test
   void shouldFailWithInsufficientGas() {
     assertFailure(
@@ -63,7 +80,22 @@ void shouldFailWithInsufficientGas() {
         ExceptionalHaltReason.INSUFFICIENT_GAS,
         200,
         (__, ___) -> Hash.hash(Bytes.fromHexString("0x1293487297")),
-        1);
+        1L,
+        20L);
+  }
+
+  @Test
+  void shouldFailWithInsufficientGasWithinLookup() {
+    assertFailure(
+        Bytes32.fromHexString("0x64"),
+        ExceptionalHaltReason.INSUFFICIENT_GAS,
+        200,
+        (frame, ___) -> {
+          frame.decrementRemainingGas(100);
+          return Hash.EMPTY;
+        },
+        100L,
+        120L);
   }
 
   private void assertBlockHash(
@@ -93,10 +125,12 @@ private void assertBlockHash(
             .pushStackItem(UInt256.fromBytes(input))
             .initialGas(initialGas)
             .build();
+    long remainingGasBefore = frame.getRemainingGas();
     blockHashOperation.execute(frame, null);
     final Bytes result = frame.popStackItem();
     assertThat(result).isEqualTo(expectedOutput);
     assertThat(frame.stackSize()).isZero();
+    assertThat(frame.getRemainingGas()).isEqualTo(remainingGasBefore);
   }
 
   private void assertFailure(
@@ -104,7 +138,8 @@ private void assertFailure(
       final ExceptionalHaltReason haltReason,
       final long currentBlockNumber,
       final BlockHashLookup blockHashLookup,
-      final long initialGas) {
+      final long initialGas,
+      final long gasCost) {
     final MessageFrame frame =
         new TestMessageFrameBuilder()
             .blockHashLookup(blockHashLookup)
@@ -114,6 +149,6 @@ private void assertFailure(
             .build();
     Operation.OperationResult operationResult = blockHashOperation.execute(frame, null);
     assertThat(operationResult.getHaltReason()).isEqualTo(haltReason);
-    assertThat(frame.stackSize()).isOne();
+    assertThat(operationResult.gasCost).isEqualTo(gasCost);
   }
 }