Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rpc-gas-cap via transaction simulator #6156

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
542af22
Add `rpc-gas-cap` and pipe it to TransactionSimulator
gfukushima Nov 10, 2023
eb5b205
Add gasCap to TransactionSimulator and cap gasLimit when cap is defined
gfukushima Nov 10, 2023
c2dd666
Fix tests
gfukushima Nov 10, 2023
c65a6c9
Add Optional.empty() for qbft and testcontextbuilder
gfukushima Nov 10, 2023
f98b5a7
Add gasCap tests to TransactionSimulator
gfukushima Nov 10, 2023
7a6e7bd
spotless
gfukushima Nov 10, 2023
97ef83c
Merge branch 'main' into rpcGasCap_via_transactionSimulator
gfukushima Nov 10, 2023
9b82b99
Merge branch 'main' into rpcGasCap_via_transactionSimulator
gfukushima Nov 12, 2023
320bb74
Merge branch 'main' into rpcGasCap_via_transactionSimulator
gfukushima Nov 13, 2023
2ebe361
Add gasCap to DebugJsonRpcMethods for DebugTraceCall
gfukushima Nov 13, 2023
0d43e45
Add log to inform users that gasCap has been applied
gfukushima Nov 13, 2023
e1653d7
Fix javadoc copy and paste description
gfukushima Nov 14, 2023
555365f
Remove unused ArgumentCaptor
gfukushima Nov 14, 2023
3767cf3
Rename test and replaced random values for better values to add conte…
gfukushima Nov 14, 2023
ca951fe
Merge branch 'main' into rpcGasCap_via_transactionSimulator
gfukushima Nov 16, 2023
a86ca70
Remove mocked result from tests that don't need it.
gfukushima Nov 16, 2023
7423749
Fix test touched accidentally
gfukushima Nov 16, 2023
5482443
Merge branch 'main' into rpcGasCap_via_transactionSimulator
gfukushima Nov 21, 2023
4302214
Add changelog
gfukushima Nov 21, 2023
23eb8e8
Merge remote-tracking branch 'origin/rpcGasCap_via_transactionSimulat…
gfukushima Nov 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
- Transactions that takes too long to evaluate, during block creation, are dropped from the txpool [#6163](https://github.com/hyperledger/besu/pull/6163)
- New option `tx-pool-min-gas-price` to set a lower bound when accepting txs to the pool [#6098](https://github.com/hyperledger/besu/pull/6098)
- Allow a transaction selection plugin to specify custom selection results [#6190](https://github.com/hyperledger/besu/pull/6190)
- Add `rpc-gas-cap` to allow users to set gas limit to the RPC methods used to simulate transactions[#6156](https://github.com/hyperledger/besu/pull/6156)


## 23.10.2

Expand Down
17 changes: 15 additions & 2 deletions besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ public class RunnerBuilder {
private JsonRpcIpcConfiguration jsonRpcIpcConfiguration;
private boolean legacyForkIdEnabled;
private Optional<Long> rpcMaxLogsRange;
private Optional<Long> rpcGasCap;
private Optional<EnodeDnsConfiguration> enodeDnsConfiguration;

/**
Expand Down Expand Up @@ -585,6 +586,16 @@ public RunnerBuilder rpcMaxLogsRange(final Long rpcMaxLogsRange) {
this.rpcMaxLogsRange = rpcMaxLogsRange > 0 ? Optional.of(rpcMaxLogsRange) : Optional.empty();
return this;
}
/**
* Add Rpc gasLimit cap .
*
* @param rpcGasCap the rpc gas limit cap for transaction simulation methods
* @return the runner builder
*/
public RunnerBuilder rpcGasCap(final Long rpcGasCap) {
this.rpcGasCap = rpcGasCap > 0 ? Optional.of(rpcGasCap) : Optional.empty();
return this;
}

/**
* Add enode DNS configuration
Expand Down Expand Up @@ -661,7 +672,7 @@ public Runner build() {

final TransactionSimulator transactionSimulator =
new TransactionSimulator(
context.getBlockchain(), context.getWorldStateArchive(), protocolSchedule);
context.getBlockchain(), context.getWorldStateArchive(), protocolSchedule, rpcGasCap);

final Bytes localNodeId = nodeKey.getPublicKey().getEncodedBytes();
final Optional<NodePermissioningController> nodePermissioningController =
Expand Down Expand Up @@ -910,6 +921,7 @@ public Runner build() {
graphQlContextMap.putIfAbsent(GraphQLContextType.SYNCHRONIZER, synchronizer);
graphQlContextMap.putIfAbsent(
GraphQLContextType.CHAIN_ID, protocolSchedule.getChainId().map(UInt256::valueOf));
graphQlContextMap.putIfAbsent(GraphQLContextType.GAS_CAP, rpcGasCap);
final GraphQL graphQL;
try {
graphQL = GraphQLProvider.buildGraphQL(fetchers);
Expand Down Expand Up @@ -1240,7 +1252,8 @@ private Map<String, JsonRpcMethod> jsonRpcMethods(
besuController.getProtocolManager().ethContext().getEthPeers(),
consensusEngineServer,
rpcMaxLogsRange,
enodeDnsConfiguration);
enodeDnsConfiguration,
rpcGasCap);
methods.putAll(besuController.getAdditionalJsonRpcMethods(jsonRpcApis));

final var pluginMethods =
Expand Down
7 changes: 7 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,12 @@ static class PermissionsOptionGroup {
"Specifies the maximum number of blocks to retrieve logs from via RPC. Must be >=0. 0 specifies no limit (default: ${DEFAULT-VALUE})")
private final Long rpcMaxLogsRange = 5000L;

@CommandLine.Option(
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;

@CommandLine.Option(
names = {"--cache-last-blocks"},
description = "Specifies the number of last blocks to cache (default: ${DEFAULT-VALUE})")
Expand Down Expand Up @@ -2948,6 +2954,7 @@ private Runner synchronize(
.storageProvider(keyValueStorageProvider(keyValueStorageName))
.rpcEndpointService(rpcEndpointServiceImpl)
.rpcMaxLogsRange(rpcMaxLogsRange)
.rpcGasCap(rpcGasCap)
.enodeDnsConfiguration(getEnodeDnsConfiguration())
.build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ protected BftContext createConsensusContext(
blockchain, epochManager, bftBlockInterface().get(), validatorOverrides);

final TransactionSimulator transactionSimulator =
new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule);
new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule, Optional.empty());
transactionValidatorProvider =
new TransactionValidatorProvider(
blockchain, new ValidatorContractController(transactionSimulator), qbftForksSchedule);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ public void enodeUrlShouldHaveAdvertisedHostWhenDiscoveryDisabled() {
.dataDir(dataDir.getRoot())
.storageProvider(mock(KeyValueStorageProvider.class, RETURNS_DEEP_STUBS))
.rpcEndpointService(new RpcEndpointServiceImpl())
.rpcGasCap(50_000_000L)
.build();
runner.startEthereumMainLoop();

Expand Down Expand Up @@ -211,6 +212,7 @@ public void movingAcrossProtocolSpecsUpdatesNodeRecord() {
.dataDir(dataDir.getRoot())
.storageProvider(storageProvider)
.rpcEndpointService(new RpcEndpointServiceImpl())
.rpcGasCap(50_000_000L)
.build();
runner.startEthereumMainLoop();

Expand Down Expand Up @@ -270,6 +272,7 @@ public void whenEngineApiAddedListensOnDefaultPort() {
.storageProvider(mock(KeyValueStorageProvider.class, RETURNS_DEEP_STUBS))
.rpcEndpointService(new RpcEndpointServiceImpl())
.besuPluginContext(mock(BesuPluginContextImpl.class))
.rpcGasCap(50_000_000L)
.build();

assertThat(runner.getJsonRpcPort()).isPresent();
Expand Down Expand Up @@ -312,6 +315,7 @@ public void whenEngineApiAddedWebSocketReadyOnSamePort() {
.storageProvider(mock(KeyValueStorageProvider.class, RETURNS_DEEP_STUBS))
.rpcEndpointService(new RpcEndpointServiceImpl())
.besuPluginContext(mock(BesuPluginContextImpl.class))
.rpcGasCap(50_000_000L)
.build();

assertThat(runner.getEngineJsonRpcPort()).isPresent();
Expand Down Expand Up @@ -353,6 +357,7 @@ public void whenEngineApiAddedEthSubscribeAvailable() {
.storageProvider(mock(KeyValueStorageProvider.class, RETURNS_DEEP_STUBS))
.rpcEndpointService(new RpcEndpointServiceImpl())
.besuPluginContext(mock(BesuPluginContextImpl.class))
.rpcGasCap(50_000_000L)
.build();

assertThat(runner.getEngineJsonRpcPort()).isPresent();
Expand Down Expand Up @@ -396,6 +401,7 @@ public void noEngineApiNoServiceForMethods() {
.rpcEndpointService(new RpcEndpointServiceImpl())
.besuPluginContext(mock(BesuPluginContextImpl.class))
.networkingConfiguration(NetworkingConfiguration.create())
.rpcGasCap(50_000_000L)
.build();

assertThat(runner.getJsonRpcPort()).isPresent();
Expand Down
3 changes: 2 additions & 1 deletion besu/src/test/java/org/hyperledger/besu/RunnerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ private void syncFromGenesis(final SyncMode mode, final GenesisConfigFile genesi
.permissioningService(new PermissioningServiceImpl())
.staticNodes(emptySet())
.storageProvider(new InMemoryKeyValueStorageProvider())
.rpcEndpointService(new RpcEndpointServiceImpl());
.rpcEndpointService(new RpcEndpointServiceImpl())
.rpcGasCap(50_000_000L);

Runner runnerBehind = null;
final Runner runnerAhead =
Expand Down
14 changes: 14 additions & 0 deletions besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,20 @@ public void rpcMaxLogsRangeOptionMustBeUsed() {
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void rpcGasCapOptionMustBeUsed() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit confused what this test name means...it's optional right? In what sense must it be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added following the pattern that a lot of other CLI options follow. Agree that the name could be better but I take that it means that the option must be used when specified and throw no error.

final long rpcGasCap = 150L;
parseCommand("--rpc-gas-cap", Long.toString(rpcGasCap));

verify(mockRunnerBuilder).rpcGasCap(longArgumentCaptor.capture());
verify(mockRunnerBuilder).build();

assertThat(longArgumentCaptor.getValue()).isEqualTo(rpcGasCap);

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void p2pPeerUpperBound_without_p2pPeerLowerBound_shouldSetLowerBoundEqualToUpperBound() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ public abstract class CommandTestAbstract {
@Captor protected ArgumentCaptor<String> stringArgumentCaptor;
@Captor protected ArgumentCaptor<Integer> intArgumentCaptor;
@Captor protected ArgumentCaptor<Long> longArgumentCaptor;
@Captor protected ArgumentCaptor<Float> floatCaptor;
@Captor protected ArgumentCaptor<EthNetworkConfig> ethNetworkConfigArgumentCaptor;
@Captor protected ArgumentCaptor<SynchronizerConfiguration> syncConfigurationCaptor;
@Captor protected ArgumentCaptor<JsonRpcConfiguration> jsonRpcConfigArgumentCaptor;
Expand Down Expand Up @@ -315,6 +314,7 @@ public void initMocks() throws Exception {
when(mockRunnerBuilder.legacyForkId(anyBoolean())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.rpcMaxLogsRange(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.enodeDnsConfiguration(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.rpcGasCap(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.build()).thenReturn(mockRunner);

final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ rpc-http-max-request-content-length = 5242880
rpc-max-logs-range=100
json-pretty-print-enabled=false
cache-last-blocks=512
rpc-gas-cap = 50000000

# PRIVACY TLS
privacy-tls-enabled=false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ private static ControllerAndState createControllerAndFinalState(

final BftValidatorOverrides validatorOverrides = convertBftForks(qbftForks);
final TransactionSimulator transactionSimulator =
new TransactionSimulator(blockChain, worldStateArchive, protocolSchedule);
new TransactionSimulator(blockChain, worldStateArchive, protocolSchedule, Optional.empty());

final BlockValidatorProvider blockValidatorProvider =
BlockValidatorProvider.forkingValidatorProvider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ public Map<String, JsonRpcMethod> methods() {
ethPeers,
Vertx.vertx(new VertxOptions().setWorkerPoolSize(1)),
Optional.empty(),
Optional.empty(),
Optional.empty());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ public enum GraphQLContextType {
MINING_COORDINATOR,
SYNCHRONIZER,
IS_ALIVE_HANDLER,
CHAIN_ID
CHAIN_ID,
GAS_CAP
}
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,10 @@ private Optional<CallResult> executeCall(final DataFetchingEnvironment environme
final ProtocolSchedule protocolSchedule =
environment.getGraphQlContext().get(GraphQLContextType.PROTOCOL_SCHEDULE);
final long bn = header.getNumber();

final Optional<Long> gasCap = environment.getGraphQlContext().get(GraphQLContextType.GAS_CAP);
final TransactionSimulator transactionSimulator =
new TransactionSimulator(
query.getBlockchain(), query.getWorldStateArchive(), protocolSchedule);
query.getBlockchain(), query.getWorldStateArchive(), protocolSchedule, gasCap);

long gasParam = -1;
Wei gasPriceParam = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ public Optional<CallResult> getCall(final DataFetchingEnvironment environment) {
final BlockchainQueries query = getBlockchainQueries(environment);
final ProtocolSchedule protocolSchedule =
environment.getGraphQlContext().get(GraphQLContextType.PROTOCOL_SCHEDULE);

final Optional<Long> gasCap = environment.getGraphQlContext().get(GraphQLContextType.GAS_CAP);
final TransactionSimulator transactionSimulator =
new TransactionSimulator(
query.getBlockchain(), query.getWorldStateArchive(), protocolSchedule);
query.getBlockchain(), query.getWorldStateArchive(), protocolSchedule, gasCap);

long gasParam = -1;
Wei gasPriceParam = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

import java.nio.file.Path;
import java.util.Map;
import java.util.Optional;

public class DebugJsonRpcMethods extends ApiGroupJsonRpcMethods {

Expand All @@ -64,21 +65,25 @@ public class DebugJsonRpcMethods extends ApiGroupJsonRpcMethods {
private final Synchronizer synchronizer;
private final Path dataDir;

private final Optional<Long> gasCap;

DebugJsonRpcMethods(
final BlockchainQueries blockchainQueries,
final ProtocolContext protocolContext,
final ProtocolSchedule protocolSchedule,
final ObservableMetricsSystem metricsSystem,
final TransactionPool transactionPool,
final Synchronizer synchronizer,
final Path dataDir) {
final Path dataDir,
final Optional<Long> gasCap) {
this.blockchainQueries = blockchainQueries;
this.protocolContext = protocolContext;
this.protocolSchedule = protocolSchedule;
this.metricsSystem = metricsSystem;
this.transactionPool = transactionPool;
this.synchronizer = synchronizer;
this.dataDir = dataDir;
this.gasCap = gasCap;
}

@Override
Expand Down Expand Up @@ -122,6 +127,7 @@ protected Map<String, JsonRpcMethod> create() {
new TransactionSimulator(
blockchainQueries.getBlockchain(),
blockchainQueries.getWorldStateArchive(),
protocolSchedule)));
protocolSchedule,
gasCap)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public class EthJsonRpcMethods extends ApiGroupJsonRpcMethods {
private final MiningCoordinator miningCoordinator;
private final Set<Capability> supportedCapabilities;
private final Optional<Long> maxLogRange;
private final Optional<Long> gasCap;

public EthJsonRpcMethods(
final BlockchainQueries blockchainQueries,
Expand All @@ -96,7 +97,8 @@ public EthJsonRpcMethods(
final TransactionPool transactionPool,
final MiningCoordinator miningCoordinator,
final Set<Capability> supportedCapabilities,
final Optional<Long> maxLogRange) {
final Optional<Long> maxLogRange,
final Optional<Long> gasCap) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wondering if it might make sense to use Long.MAX_VALUE instead of a optional field...but also like that this same pattern as maxLogRange.

this.blockchainQueries = blockchainQueries;
this.synchronizer = synchronizer;
this.protocolSchedule = protocolSchedule;
Expand All @@ -105,6 +107,7 @@ public EthJsonRpcMethods(
this.miningCoordinator = miningCoordinator;
this.supportedCapabilities = supportedCapabilities;
this.maxLogRange = maxLogRange;
this.gasCap = gasCap;
}

@Override
Expand All @@ -128,7 +131,8 @@ protected Map<String, JsonRpcMethod> create() {
new TransactionSimulator(
blockchainQueries.getBlockchain(),
blockchainQueries.getWorldStateArchive(),
protocolSchedule)),
protocolSchedule,
gasCap)),
new EthFeeHistory(protocolSchedule, blockchainQueries.getBlockchain()),
new EthGetCode(blockchainQueries),
new EthGetLogs(blockchainQueries, maxLogRange),
Expand Down Expand Up @@ -157,13 +161,15 @@ protected Map<String, JsonRpcMethod> create() {
new TransactionSimulator(
blockchainQueries.getBlockchain(),
blockchainQueries.getWorldStateArchive(),
protocolSchedule)),
protocolSchedule,
gasCap)),
new EthCreateAccessList(
blockchainQueries,
new TransactionSimulator(
blockchainQueries.getBlockchain(),
blockchainQueries.getWorldStateArchive(),
protocolSchedule)),
protocolSchedule,
gasCap)),
new EthMining(miningCoordinator),
new EthCoinbase(miningCoordinator),
new EthProtocolVersion(supportedCapabilities),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public Map<String, JsonRpcMethod> methods(
final EthPeers ethPeers,
final Vertx consensusEngineServer,
final Optional<Long> maxLogRange,
final Optional<EnodeDnsConfiguration> enodeDnsConfiguration) {
final Optional<EnodeDnsConfiguration> enodeDnsConfiguration,
final Optional<Long> gasCap) {
final Map<String, JsonRpcMethod> enabled = new HashMap<>();
if (!rpcApis.isEmpty()) {
final JsonRpcMethod modules = new RpcModules(rpcApis);
Expand All @@ -104,7 +105,8 @@ public Map<String, JsonRpcMethod> methods(
metricsSystem,
transactionPool,
synchronizer,
dataDir),
dataDir,
gasCap),
new EeaJsonRpcMethods(
blockchainQueries, protocolSchedule, transactionPool, privacyParameters),
new ExecutionEngineJsonRpcMethods(
Expand All @@ -121,7 +123,8 @@ public Map<String, JsonRpcMethod> methods(
transactionPool,
miningCoordinator,
supportedCapabilities,
maxLogRange),
maxLogRange,
gasCap),
new NetJsonRpcMethods(
p2pNetwork,
networkId,
Expand All @@ -139,7 +142,7 @@ public Map<String, JsonRpcMethod> methods(
new PrivxJsonRpcMethods(
blockchainQueries, protocolSchedule, transactionPool, privacyParameters),
new Web3JsonRpcMethods(clientVersion),
new TraceJsonRpcMethods(blockchainQueries, protocolSchedule),
new TraceJsonRpcMethods(blockchainQueries, protocolSchedule, gasCap),
new TxPoolJsonRpcMethods(transactionPool),
new PluginsJsonRpcMethods(namedPlugins));

Expand Down
Loading