update rpc gas cap default to 50M (hyperledger#8251)

* update rpc gas cap default to 50M Signed-off-by: Sally MacFarlane <[email protected]> --------- Signed-off-by: Sally MacFarlane <[email protected]> Signed-off-by: Jason Frame <[email protected]>
jframe · Feb 5, 2025 · 54a6415 · 54a6415
1 parent 4e73152
commit 54a6415
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 ### Breaking Changes
+- `rpc-gas-cap` default value has changed from 0 (unlimited) to 50M. If you require `rpc-gas-cap` greater than 50M, you'll need to set that explicitly. [#8251](https://github.com/hyperledger/besu/issues/8251)
 ### Upcoming Breaking Changes
 - `MetricSystem::createLabelledGauge` is deprecated and will be removed in a future release, replace it with `MetricSystem::createLabelledSuppliedGauge`
 - k8s (KUBERNETES) Nat method is now deprecated and will be removed in a future release. Use docker or none instead.
@@ -16,6 +17,7 @@
     - Fast Sync
 ### Additions and Improvements
 - Add a tx selector to skip txs from the same sender after the first not selected [#8216](https://github.com/hyperledger/besu/pull/8216)
+- `rpc-gas-cap` default value has changed from 0 (unlimited) to 50M [#8251](https://github.com/hyperledger/besu/issues/8251)
 
 #### Prague
 - Add timestamps to enable Prague hardfork on Sepolia and Holesky test networks [#8163](https://github.com/hyperledger/besu/pull/8163)

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/ApiConfigurationOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/ApiConfigurationOptions.java
@@ -81,7 +81,7 @@ public ApiConfigurationOptions() {}
       names = {"--rpc-gas-cap"},
       description =
           "Specifies the gasLimit cap for transaction simulation RPC methods. Must be >=0. 0 specifies no limit  (default: ${DEFAULT-VALUE})")
-  private final Long rpcGasCap = 0L;
+  private final Long rpcGasCap = ApiConfiguration.DEFAULT_GAS_CAP;
 
   @CommandLine.Option(
       names = {"--rpc-max-trace-filter-range"},

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/ApiConfigurationOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/ApiConfigurationOptionsTest.java
@@ -19,6 +19,7 @@
 import static org.mockito.Mockito.verify;
 
 import org.hyperledger.besu.cli.CommandTestAbstract;
+import org.hyperledger.besu.ethereum.api.ApiConfiguration;
 import org.hyperledger.besu.ethereum.api.ImmutableApiConfiguration;
 
 import org.junit.jupiter.api.Test;
@@ -111,7 +112,7 @@ public void rpcMaxLogsRangeOptionMustBeUsed() {
     verify(mockRunnerBuilder).build();
 
     assertThat(apiConfigurationCaptor.getValue())
-        .isEqualTo(ImmutableApiConfiguration.builder().maxLogsRange((rpcMaxLogsRange)).build());
+        .isEqualTo(ImmutableApiConfiguration.builder().maxLogsRange(rpcMaxLogsRange).build());
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@@ -126,7 +127,37 @@ public void rpcGasCapOptionMustBeUsed() {
     verify(mockRunnerBuilder).build();
 
     assertThat(apiConfigurationCaptor.getValue())
-        .isEqualTo(ImmutableApiConfiguration.builder().gasCap((rpcGasCap)).build());
+        .isEqualTo(ImmutableApiConfiguration.builder().gasCap(rpcGasCap).build());
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void rpcGasCapDefault() {
+    parseCommand();
+
+    verify(mockRunnerBuilder).apiConfiguration(apiConfigurationCaptor.capture());
+    verify(mockRunnerBuilder).build();
+
+    assertThat(apiConfigurationCaptor.getValue())
+        .isEqualTo(
+            ImmutableApiConfiguration.builder().gasCap(ApiConfiguration.DEFAULT_GAS_CAP).build());
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void rpcGasCapAcceptsZero() {
+    final long rpcGasCap = 0L;
+    parseCommand("--rpc-gas-cap", Long.toString(rpcGasCap));
+
+    verify(mockRunnerBuilder).apiConfiguration(apiConfigurationCaptor.capture());
+    verify(mockRunnerBuilder).build();
+
+    assertThat(apiConfigurationCaptor.getValue())
+        .isEqualTo(ImmutableApiConfiguration.builder().gasCap(rpcGasCap).build());
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@@ -143,7 +174,7 @@ public void rpcMaxTraceFilterOptionMustBeUsed() {
     assertThat(apiConfigurationCaptor.getValue())
         .isEqualTo(
             ImmutableApiConfiguration.builder()
-                .maxTraceFilterRange((rpcMaxTraceFilterOption))
+                .maxTraceFilterRange(rpcMaxTraceFilterOption)
                 .build());
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();

diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/ApiConfiguration.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/ApiConfiguration.java
@@ -38,6 +38,9 @@ public abstract class ApiConfiguration {
    */
   public static final long DEFAULT_UPPER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT = Long.MAX_VALUE;
 
+  /** The default gas cap used for transaction simulation */
+  public static final long DEFAULT_GAS_CAP = 50_000_000L;
+
   /** Constructs a new ApiConfiguration with default values. */
   protected ApiConfiguration() {}
 
@@ -93,13 +96,13 @@ public Long getMaxLogsRange() {
   }
 
   /**
-   * Returns the gas cap. Default value is 0.
+   * Returns the gas cap. Default value is 50M.
    *
    * @return the gas cap
    */
   @Value.Default
   public Long getGasCap() {
-    return 0L;
+    return DEFAULT_GAS_CAP;
   }
 
   /**

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
@@ -37,6 +37,7 @@
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.datatypes.parameters.UnsignedLongParameter;
 import org.hyperledger.besu.ethereum.GasLimitCalculator;
+import org.hyperledger.besu.ethereum.api.ApiConfiguration;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter.JsonCallParameterBuilder;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.BlobTestFixture;
@@ -93,8 +94,9 @@ public class TransactionSimulatorTest {
       Address.fromHexString("0x0000000000000000000000000000000000000000");
   private static final long GAS_CAP = 500000L;
   private static final long TRANSFER_GAS_LIMIT = 21000L;
-  private TransactionSimulator transactionSimulator;
+  private TransactionSimulator uncappedTransactionSimulator;
   private TransactionSimulator cappedTransactionSimulator;
+  private TransactionSimulator defaultCappedTransactionSimulator;
 
   @Mock private Blockchain blockchain;
   @Mock private WorldStateArchive worldStateArchive;
@@ -107,12 +109,22 @@ public class TransactionSimulatorTest {
   @BeforeEach
   public void setUp() {
     final var miningConfiguration = MiningConfiguration.newDefault().setCoinbase(Address.ZERO);
-    this.transactionSimulator =
+    // rpc gas cap 0 means unlimited
+    this.uncappedTransactionSimulator =
         new TransactionSimulator(
             blockchain, worldStateArchive, protocolSchedule, miningConfiguration, 0);
+    // capped at a lower limit
     this.cappedTransactionSimulator =
         new TransactionSimulator(
             blockchain, worldStateArchive, protocolSchedule, miningConfiguration, GAS_CAP);
+    // capped at default limit
+    this.defaultCappedTransactionSimulator =
+        new TransactionSimulator(
+            blockchain,
+            worldStateArchive,
+            protocolSchedule,
+            miningConfiguration,
+            ApiConfiguration.DEFAULT_GAS_CAP);
   }
 
   @Test
@@ -174,7 +186,7 @@ public void shouldReturnEmptyWhenBlockDoesNotExist() {
     when(blockchain.getBlockHeader(eq(1L))).thenReturn(Optional.empty());
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(legacyTransactionCallParameterBuilder().build(), 1L);
+        uncappedTransactionSimulator.process(legacyTransactionCallParameterBuilder().build(), 1L);
 
     assertThat(result.isPresent()).isFalse();
   }
@@ -203,7 +215,7 @@ public void shouldReturnSuccessfulResultWhenProcessingIsSuccessful() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     assertThat(result.get().isSuccessful()).isTrue();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -234,12 +246,12 @@ public void simulateOnPendingBlockWorks() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.processOnPending(
+        uncappedTransactionSimulator.processOnPending(
             callParameter,
             Optional.empty(),
             TransactionValidationParams.transactionSimulator(),
             NO_TRACING,
-            transactionSimulator.simulatePendingBlockHeader());
+            uncappedTransactionSimulator.simulatePendingBlockHeader());
 
     assertThat(result.get().isSuccessful()).isTrue();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -269,7 +281,7 @@ public void shouldSetGasPriceToZeroWhenExceedingBalanceAllowed() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(
+    uncappedTransactionSimulator.process(
         callParameter,
         ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(true).build(),
         NO_TRACING,
@@ -306,7 +318,7 @@ public void shouldSetFeePerGasToZeroWhenExceedingBalanceAllowed() {
 
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(
+    uncappedTransactionSimulator.process(
         callParameter,
         ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(true).build(),
         NO_TRACING,
@@ -340,7 +352,7 @@ public void shouldNotSetGasPriceToZeroWhenExceedingBalanceIsNotAllowed() {
 
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(
+    uncappedTransactionSimulator.process(
         callParameter,
         ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(false).build(),
         NO_TRACING,
@@ -376,7 +388,7 @@ public void shouldNotSetFeePerGasToZeroWhenExceedingBalanceIsNotAllowed() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(
+    uncappedTransactionSimulator.process(
         callParameter,
         ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(false).build(),
         NO_TRACING,
@@ -409,7 +421,7 @@ public void shouldUseDefaultValuesWhenMissingOptionalFields() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(callParameter, 1L);
+    uncappedTransactionSimulator.process(callParameter, 1L);
 
     verifyTransactionWasProcessed(expectedTransaction);
   }
@@ -438,7 +450,7 @@ public void shouldUseZeroNonceWhenAccountDoesNotExist() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(callParameter, 1L);
+    uncappedTransactionSimulator.process(callParameter, 1L);
 
     verifyTransactionWasProcessed(expectedTransaction);
   }
@@ -472,7 +484,7 @@ public void shouldUseSpecifiedNonceWhenProvided() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(callParameter, 1L);
+    uncappedTransactionSimulator.process(callParameter, 1L);
     verifyTransactionWasProcessed(expectedTransaction);
   }
 
@@ -501,7 +513,7 @@ public void shouldReturnFailureResultWhenProcessingFails() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.FAILED);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     assertThat(result.get().isSuccessful()).isFalse();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -512,7 +524,8 @@ public void shouldReturnEmptyWhenBlockDoesNotExistByHash() {
     when(blockchain.getBlockHeader(eq(Hash.ZERO))).thenReturn(Optional.empty());
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(legacyTransactionCallParameterBuilder().build(), Hash.ZERO);
+        uncappedTransactionSimulator.process(
+            legacyTransactionCallParameterBuilder().build(), Hash.ZERO);
 
     assertThat(result.isPresent()).isFalse();
   }
@@ -542,7 +555,7 @@ public void shouldReturnSuccessfulResultWhenProcessingIsSuccessfulByHash() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, blockHeader.getBlockHash());
+        uncappedTransactionSimulator.process(callParameter, blockHeader.getBlockHash());
 
     assertThat(result.get().isSuccessful()).isTrue();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -572,7 +585,7 @@ public void shouldUseDefaultValuesWhenMissingOptionalFieldsByHash() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(callParameter, blockHeader.getBlockHash());
+    uncappedTransactionSimulator.process(callParameter, blockHeader.getBlockHash());
 
     verifyTransactionWasProcessed(expectedTransaction);
   }
@@ -601,7 +614,7 @@ public void shouldUseZeroNonceWhenAccountDoesNotExistByHash() {
             .build();
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
-    transactionSimulator.process(callParameter, blockHeader.getBlockHash());
+    uncappedTransactionSimulator.process(callParameter, blockHeader.getBlockHash());
 
     verifyTransactionWasProcessed(expectedTransaction);
   }
@@ -631,7 +644,7 @@ public void shouldReturnFailureResultWhenProcessingFailsByHash() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.FAILED);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, blockHeader.getBlockHash());
+        uncappedTransactionSimulator.process(callParameter, blockHeader.getBlockHash());
 
     assertThat(result.get().isSuccessful()).isFalse();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -662,7 +675,7 @@ public void shouldReturnSuccessfulResultWhenEip1559TransactionProcessingIsSucces
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     assertThat(result.get().isSuccessful()).isTrue();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -735,7 +748,7 @@ public void shouldUseProvidedGasLimitWhenBelowRpcCapGas() {
   }
 
   @Test
-  public void shouldUseRpcGasCapWhenGasLimitNoPresent() {
+  public void shouldUseRpcGasCapWhenGasLimitNotPresent() {
     // generate call parameters that do not specify a gas limit,
     // expect the rpc gas cap to be used for simulation
 
@@ -769,6 +782,41 @@ public void shouldUseRpcGasCapWhenGasLimitNoPresent() {
     verifyTransactionWasProcessed(expectedTransaction);
   }
 
+  @Test
+  public void shouldUseDefaultRpcGasCapWhenGasLimitNotPresent() {
+    // generate call parameters that do not specify a gas limit,
+    // expect the default rpc gas cap to be used for simulation
+
+    final CallParameter callParameter =
+        eip1559TransactionCallParameterBuilder().withGas(-1L).build();
+
+    mockBlockchainAndWorldState(callParameter);
+    mockProtocolSpecForProcessWithWorldUpdater();
+
+    final Transaction expectedTransaction =
+        Transaction.builder()
+            .type(TransactionType.EIP1559)
+            .chainId(BigInteger.ONE)
+            .nonce(1L)
+            .gasLimit(callParameter.getGasLimit())
+            .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
+            .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
+            .to(callParameter.getTo())
+            .sender(callParameter.getFrom())
+            .value(callParameter.getValue())
+            .payload(callParameter.getPayload())
+            .signature(FAKE_SIGNATURE)
+            .gasLimit(ApiConfiguration.DEFAULT_GAS_CAP)
+            .build();
+
+    // call process with original transaction
+    defaultCappedTransactionSimulator.process(
+        callParameter, TransactionValidationParams.transactionSimulator(), NO_TRACING, 1L);
+
+    // expect transaction with the original gas limit to be processed
+    verifyTransactionWasProcessed(expectedTransaction);
+  }
+
   @Test
   public void shouldReturnSuccessfulResultWhenBlobTransactionProcessingIsSuccessful() {
     final CallParameter callParameter = blobTransactionCallParameter();
@@ -780,7 +828,7 @@ public void shouldReturnSuccessfulResultWhenBlobTransactionProcessingIsSuccessfu
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     assertThat(result.get().isSuccessful()).isTrue();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -797,7 +845,7 @@ public void shouldReturnFailureResultWhenBlobTransactionProcessingFails() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.FAILED);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     assertThat(result.get().isSuccessful()).isFalse();
     verifyTransactionWasProcessed(expectedTransaction);
@@ -980,7 +1028,7 @@ public void shouldSimulateLegacyTransactionWhenBaseFeeNotZero() {
     mockProcessorStatusForTransaction(expectedTransaction, Status.SUCCESSFUL);
 
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(callParameter, 1L);
+        uncappedTransactionSimulator.process(callParameter, 1L);
 
     verifyTransactionWasProcessed(expectedTransaction);
     assertThat(result.get().isSuccessful()).isTrue();