From 54a641583838eb242826739427624bc74218337e Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 5 Feb 2025 14:32:06 +1000 Subject: [PATCH] update rpc gas cap default to 50M (#8251) * update rpc gas cap default to 50M Signed-off-by: Sally MacFarlane --------- Signed-off-by: Sally MacFarlane Signed-off-by: Jason Frame --- CHANGELOG.md | 2 + .../cli/options/ApiConfigurationOptions.java | 2 +- .../options/ApiConfigurationOptionsTest.java | 37 ++++++- .../besu/ethereum/api/ApiConfiguration.java | 7 +- .../transaction/TransactionSimulatorTest.java | 96 ++++++++++++++----- 5 files changed, 114 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb941f0ac9e..f0c28640f91 100644 --- 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 index c5954f50639..e8f41805baa 100644 --- 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 index 6e6c17f4dda..83e737b81ee 100644 --- 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 index e317845f841..4180f113ada 100644 --- 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/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java index 3515a5b6719..a07cc7e982c 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java +++ b/ethereum/core/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 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 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 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 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 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 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 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 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 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 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 result = - transactionSimulator.process(callParameter, 1L); + uncappedTransactionSimulator.process(callParameter, 1L); verifyTransactionWasProcessed(expectedTransaction); assertThat(result.get().isSuccessful()).isTrue();