Align gas cap for transaction simulation to Geth approach (#7703)

Signed-off-by: Fabio Di Fabio <[email protected]>
hyperledger · Oct 1, 2024 · 7af03b7 · 7af03b7
1 parent e522b63
commit 7af03b7
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@
 - LUKSO Cancun Hardfork [#7686](https://github.com/hyperledger/besu/pull/7686)
 - Add configuration of Consolidation Request Contract Address via genesis configuration [#7647](https://github.com/hyperledger/besu/pull/7647)
 - Interrupt pending transaction processing on block creation timeout [#7673](https://github.com/hyperledger/besu/pull/7673)
+- Align gas cap calculation for transaction simulation to Geth approach [#7703](https://github.com/hyperledger/besu/pull/7703)
 
 ### Bug fixes
 - Fix mounted data path directory permissions for besu user [#7575](https://github.com/hyperledger/besu/pull/7575)

diff --git a/...um/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java b/...um/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java
@@ -230,16 +230,9 @@ public Optional<TransactionSimulatorResult> processWithWorldUpdater(
     final Account sender = updater.get(senderAddress);
     final long nonce = sender != null ? sender.getNonce() : 0L;
 
-    long gasLimit =
-        callParams.getGasLimit() >= 0
-            ? callParams.getGasLimit()
-            : blockHeaderToProcess.getGasLimit();
-    if (rpcGasCap > 0) {
-      gasLimit = rpcGasCap;
-      LOG.trace(
-          "Gas limit capped at {} for transaction simulation due to provided RPC gas cap.",
-          rpcGasCap);
-    }
+    final long simulationGasCap =
+        calculateSimulationGasCap(callParams.getGasLimit(), blockHeaderToProcess.getGasLimit());
+
     final Wei value = callParams.getValue() != null ? callParams.getValue() : Wei.ZERO;
     final Bytes payload = callParams.getPayload() != null ? callParams.getPayload() : Bytes.EMPTY;
 
@@ -265,7 +258,7 @@ public Optional<TransactionSimulatorResult> processWithWorldUpdater(
             header,
             senderAddress,
             nonce,
-            gasLimit,
+            simulationGasCap,
             value,
             payload,
             blobGasPrice);
@@ -291,6 +284,38 @@ public Optional<TransactionSimulatorResult> processWithWorldUpdater(
     return Optional.of(new TransactionSimulatorResult(transaction, result));
   }
 
+  private long calculateSimulationGasCap(
+      final long userProvidedGasLimit, final long blockGasLimit) {
+    final long simulationGasCap;
+
+    // when not set gas limit is -1
+    if (userProvidedGasLimit >= 0) {
+      if (rpcGasCap > 0 && userProvidedGasLimit > rpcGasCap) {
+        LOG.trace(
+            "User provided gas limit {} is bigger than the value of rpc-gas-cap {}, setting simulation gas cap to the latter",
+            userProvidedGasLimit,
+            rpcGasCap);
+        simulationGasCap = rpcGasCap;
+      } else {
+        LOG.trace("Using provided gas limit {} set as simulation gas cap", userProvidedGasLimit);
+        simulationGasCap = userProvidedGasLimit;
+      }
+    } else {
+      if (rpcGasCap > 0) {
+        LOG.trace(
+            "No user provided gas limit, setting simulation gas cap to the value of rpc-gas-cap {}",
+            rpcGasCap);
+        simulationGasCap = rpcGasCap;
+      } else {
+        simulationGasCap = blockGasLimit;
+        LOG.trace(
+            "No user provided gas limit and rpc-gas-cap options is not set, setting simulation gas cap to block gas limit {}",
+            blockGasLimit);
+      }
+    }
+    return simulationGasCap;
+  }
+
   private Optional<Transaction> buildTransaction(
       final CallParameter callParams,
       final TransactionValidationParams transactionValidationParams,

diff --git a/...ore/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java b/...ore/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java
@@ -79,7 +79,8 @@ public class TransactionSimulatorTest {
 
   private static final Address DEFAULT_FROM =
       Address.fromHexString("0x0000000000000000000000000000000000000000");
-  private static final long GASCAP = 500L;
+  private static final long GAS_CAP = 500000L;
+  private static final long TRANSFER_GAS_LIMIT = 21000L;
   private TransactionSimulator transactionSimulator;
   private TransactionSimulator cappedTransactionSimulator;
 
@@ -96,7 +97,7 @@ public void setUp() {
     this.transactionSimulator =
         new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, 0);
     this.cappedTransactionSimulator =
-        new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, GASCAP);
+        new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, GAS_CAP);
   }
 
   @Test
@@ -124,7 +125,7 @@ public void shouldReturnSuccessfulResultWhenProcessingIsSuccessful() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(callParameter.getGasPrice())
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -155,7 +156,7 @@ public void shouldSetGasPriceToZeroWhenExceedingBalanceAllowed() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(Wei.ZERO)
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -175,7 +176,8 @@ public void shouldSetGasPriceToZeroWhenExceedingBalanceAllowed() {
 
   @Test
   public void shouldSetFeePerGasToZeroWhenExceedingBalanceAllowed() {
-    final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE);
+    final CallParameter callParameter =
+        eip1559TransactionCallParameter(Wei.ONE, Wei.ONE, TRANSFER_GAS_LIMIT);
 
     final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE);
 
@@ -187,7 +189,7 @@ public void shouldSetFeePerGasToZeroWhenExceedingBalanceAllowed() {
             .type(TransactionType.EIP1559)
             .chainId(BigInteger.ONE)
             .nonce(1L)
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(TRANSFER_GAS_LIMIT)
             .maxFeePerGas(Wei.ZERO)
             .maxPriorityFeePerGas(Wei.ZERO)
             .to(callParameter.getTo())
@@ -223,7 +225,7 @@ public void shouldNotSetGasPriceToZeroWhenExceedingBalanceIsNotAllowed() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(callParameter.getGasPrice())
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -244,7 +246,8 @@ public void shouldNotSetGasPriceToZeroWhenExceedingBalanceIsNotAllowed() {
 
   @Test
   public void shouldNotSetFeePerGasToZeroWhenExceedingBalanceIsNotAllowed() {
-    final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE);
+    final CallParameter callParameter =
+        eip1559TransactionCallParameter(Wei.ONE, Wei.ONE, TRANSFER_GAS_LIMIT);
 
     final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE);
 
@@ -256,7 +259,7 @@ public void shouldNotSetFeePerGasToZeroWhenExceedingBalanceIsNotAllowed() {
             .type(TransactionType.EIP1559)
             .chainId(BigInteger.ONE)
             .nonce(1L)
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(TRANSFER_GAS_LIMIT)
             .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
             .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
             .to(callParameter.getTo())
@@ -349,7 +352,7 @@ public void shouldReturnFailureResultWhenProcessingFails() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(callParameter.getGasPrice())
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -390,7 +393,7 @@ public void shouldReturnSuccessfulResultWhenProcessingIsSuccessfulByHash() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(callParameter.getGasPrice())
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -479,7 +482,7 @@ public void shouldReturnFailureResultWhenProcessingFailsByHash() {
             .type(TransactionType.FRONTIER)
             .nonce(1L)
             .gasPrice(callParameter.getGasPrice())
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .to(callParameter.getTo())
             .sender(callParameter.getFrom())
             .value(callParameter.getValue())
@@ -509,7 +512,7 @@ public void shouldReturnSuccessfulResultWhenEip1559TransactionProcessingIsSucces
             .type(TransactionType.EIP1559)
             .chainId(BigInteger.ONE)
             .nonce(1L)
-            .gasLimit(callParameter.getGasLimit())
+            .gasLimit(blockHeader.getGasLimit())
             .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
             .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
             .to(callParameter.getTo())
@@ -530,7 +533,7 @@ public void shouldReturnSuccessfulResultWhenEip1559TransactionProcessingIsSucces
   @Test
   public void shouldCapGasLimitWhenOriginalTransactionExceedsGasCap() {
     final CallParameter callParameter =
-        eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GASCAP + 1);
+        eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GAS_CAP + 1);
 
     final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE);
 
@@ -542,7 +545,7 @@ public void shouldCapGasLimitWhenOriginalTransactionExceedsGasCap() {
             .type(TransactionType.EIP1559)
             .chainId(BigInteger.ONE)
             .nonce(1L)
-            .gasLimit(GASCAP)
+            .gasLimit(GAS_CAP)
             .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
             .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
             .to(callParameter.getTo())
@@ -566,11 +569,48 @@ public void shouldCapGasLimitWhenOriginalTransactionExceedsGasCap() {
   }
 
   @Test
-  public void shouldUseRpcGasCapWhenCapIsHigherThanGasLimit() {
-    // generate a transaction with a gas limit that is lower than the gas cap,
-    // expect the gas cap to override parameter gas limit
+  public void shouldUseProvidedGasLimitWhenBelowRpcCapGas() {
     final CallParameter callParameter =
-        eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GASCAP - 1);
+        eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, GAS_CAP / 2);
+
+    final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE);
+
+    mockBlockchainForBlockHeader(blockHeader);
+    mockWorldStateForAccount(blockHeader, callParameter.getFrom(), 1L);
+
+    final Transaction expectedTransaction =
+        Transaction.builder()
+            .type(TransactionType.EIP1559)
+            .chainId(BigInteger.ONE)
+            .nonce(1L)
+            .gasLimit(GAS_CAP / 2)
+            .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
+            .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
+            .to(callParameter.getTo())
+            .sender(callParameter.getFrom())
+            .value(callParameter.getValue())
+            .payload(callParameter.getPayload())
+            .signature(FAKE_SIGNATURE)
+            .build();
+
+    mockProtocolSpecForProcessWithWorldUpdater();
+
+    // call process with original transaction
+    cappedTransactionSimulator.process(
+        callParameter,
+        TransactionValidationParams.transactionSimulator(),
+        OperationTracer.NO_TRACING,
+        1L);
+
+    // expect overwritten transaction to be processed
+    verifyTransactionWasProcessed(expectedTransaction);
+  }
+
+  @Test
+  public void shouldUseRpcGasCapWhenGasLimitNoPresent() {
+    // generate call parameters that do not specify a gas limit,
+    // expect the rpc gas cap to be used for simulation
+    final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO, -1);
 
     final BlockHeader blockHeader = mockBlockHeader(Hash.ZERO, 1L, Wei.ONE);
 
@@ -591,7 +631,7 @@ public void shouldUseRpcGasCapWhenCapIsHigherThanGasLimit() {
             .value(callParameter.getValue())
             .payload(callParameter.getPayload())
             .signature(FAKE_SIGNATURE)
-            .gasLimit(GASCAP)
+            .gasLimit(GAS_CAP)
             .build();
 
     // call process with original transaction
@@ -781,7 +821,7 @@ private CallParameter legacyTransactionCallParameter(final Wei gasPrice) {
     return new CallParameter(
         Address.fromHexString("0x0"),
         Address.fromHexString("0x0"),
-        0,
+        -1,
         gasPrice,
         Wei.of(0),
         Bytes.EMPTY);
@@ -793,7 +833,7 @@ private CallParameter eip1559TransactionCallParameter() {
 
   private CallParameter eip1559TransactionCallParameter(
       final Wei maxFeePerGas, final Wei maxPriorityFeePerGas) {
-    return eip1559TransactionCallParameter(maxFeePerGas, maxPriorityFeePerGas, 0L);
+    return eip1559TransactionCallParameter(maxFeePerGas, maxPriorityFeePerGas, -1);
   }
 
   private CallParameter eip1559TransactionCallParameter(