From 2d529b828a4a4cda198fee1f6844327d5daf7a1c Mon Sep 17 00:00:00 2001 From: cfelde Date: Wed, 18 Sep 2019 16:05:06 +0100 Subject: [PATCH 1/4] [PAN-3023] Add command line option for target gas limit Signed-off-by: cfelde --- .../dsl/node/ThreadBesuNodeRunner.java | 2 + .../org/hyperledger/besu/cli/BesuCommand.java | 9 +++- .../controller/BesuControllerBuilder.java | 7 +++ .../CliqueBesuControllerBuilder.java | 3 +- .../besu/controller/GasLimitCalculator.java | 46 +++++++++++++++++++ .../controller/IbftBesuControllerBuilder.java | 2 +- .../MainnetBesuControllerBuilder.java | 3 +- .../org/hyperledger/besu/PrivacyTest.java | 2 + .../java/org/hyperledger/besu/RunnerTest.java | 4 ++ .../chainexport/RlpBlockExporterTest.java | 2 + .../chainimport/JsonBlockImporterTest.java | 2 + .../chainimport/RlpBlockImporterTest.java | 3 ++ .../hyperledger/besu/cli/BesuCommandTest.java | 32 +++++++++++++ .../besu/cli/CommandTestAbstract.java | 1 + .../controller/GasLimitCalculatorTest.java | 44 ++++++++++++++++++ .../src/test/resources/everything_config.toml | 3 ++ .../blockcreation/CliqueMinerExecutor.java | 8 ++-- .../CliqueMinerExecutorTest.java | 10 ++-- .../blockcreation/AbstractMinerExecutor.java | 6 ++- .../blockcreation/EthHashMinerExecutor.java | 8 ++-- .../EthHashMinerExecutorTest.java | 7 ++- 21 files changed, 188 insertions(+), 16 deletions(-) create mode 100644 besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java create mode 100644 besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java index f3a4265ab8b..666f616708b 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java @@ -19,6 +19,7 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.controller.BesuController; import org.hyperledger.besu.controller.BesuControllerBuilder; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.controller.KeyPairUtil; import org.hyperledger.besu.ethereum.api.graphql.GraphQLConfiguration; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; @@ -146,6 +147,7 @@ public void startNode(final BesuNode node) { .clock(Clock.systemUTC()) .isRevertReasonEnabled(node.isRevertReasonEnabled()) .storageProvider(storageProvider) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); } catch (final IOException e) { throw new RuntimeException("Error building BesuController", e); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 57c027cc18e..ece4f19838a 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -652,6 +652,12 @@ void setBannedNodeIds(final List values) { "The name of a file containing the private key used to sign privacy marker transactions. If unset, each will be signed with a random key.") private final Path privacyMarkerTransactionSigningKeyPath = null; + @Option( + names = {"--target-gas-limit"}, + description = + "Sets target gas limit per block. If set each blocks gas limit will approach this setting over time if the current gas limit is different.") + private final Long targetGasLimit = null; + @Option( names = {"--tx-pool-max-size"}, paramLabel = MANDATORY_INTEGER_FORMAT_HELP, @@ -1047,7 +1053,8 @@ public BesuControllerBuilder getControllerBuilder() { .storageProvider(keyStorageProvider(keyValueStorageName)) .isPruningEnabled(isPruningEnabled) .pruningConfiguration(buildPruningConfiguration()) - .genesisConfigOverrides(genesisConfigOverrides); + .genesisConfigOverrides(genesisConfigOverrides) + .targetGasLimit(targetGasLimit); } catch (final IOException e) { throw new ExecutionException(this.commandLine, "Invalid path", e); } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index a200d4b5a4b..1c6fa275c97 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -83,6 +83,7 @@ public abstract class BesuControllerBuilder { protected Clock clock; protected KeyPair nodeKeys; protected boolean isRevertReasonEnabled; + protected GasLimitCalculator gasLimitCalculator; private StorageProvider storageProvider; private final List shutdownActions = new ArrayList<>(); private boolean isPruningEnabled; @@ -180,6 +181,11 @@ public BesuControllerBuilder genesisConfigOverrides( return this; } + public BesuControllerBuilder targetGasLimit(final Long targetGasLimit) { + this.gasLimitCalculator = new GasLimitCalculator(targetGasLimit); + return this; + } + public BesuController build() { checkNotNull(genesisConfig, "Missing genesis config"); checkNotNull(syncConfig, "Missing sync config"); @@ -193,6 +199,7 @@ public BesuController build() { checkNotNull(transactionPoolConfiguration, "Missing transaction pool configuration"); checkNotNull(nodeKeys, "Missing node keys"); checkNotNull(storageProvider, "Must supply a storage provider"); + checkNotNull(gasLimitCalculator, "Missing gas limit calculator"); prepForBuild(); diff --git a/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java index b3087c50a50..e8ed8640b7a 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/CliqueBesuControllerBuilder.java @@ -94,7 +94,8 @@ protected MiningCoordinator createMiningCoordinator( protocolContext.getConsensusState().getVoteTallyCache(), localAddress, secondsBetweenBlocks), - epochManager); + epochManager, + gasLimitCalculator); final CliqueMiningCoordinator miningCoordinator = new CliqueMiningCoordinator( protocolContext.getBlockchain(), diff --git a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java new file mode 100644 index 00000000000..204bfae2804 --- /dev/null +++ b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java @@ -0,0 +1,46 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package org.hyperledger.besu.controller; + +import java.util.function.Function; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +public class GasLimitCalculator implements Function { + private static final Logger LOG = LogManager.getLogger(); + public static final long ADJUSTMENT_FACTOR = 1024L; + public static final Long DEFAULT = null; + private final long targetGasLimit; + + public GasLimitCalculator(final Long targetGasLimit) { + this.targetGasLimit = targetGasLimit == null ? 0L : targetGasLimit; + } + + @Override + public Long apply(final Long gasLimit) { + long newGasLimit; + + if (targetGasLimit > gasLimit) { + newGasLimit = Math.min(targetGasLimit, gasLimit + ADJUSTMENT_FACTOR); + } else if (targetGasLimit < gasLimit) { + newGasLimit = Math.max(targetGasLimit, gasLimit - ADJUSTMENT_FACTOR); + } else { + return gasLimit; + } + + LOG.debug("Adjusting block gas limit from {} to {}", gasLimit, newGasLimit); + + return newGasLimit; + } +} diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java index aa9de1f4b80..6323614f1e1 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java @@ -111,7 +111,7 @@ protected MiningCoordinator createMiningCoordinator( final IbftBlockCreatorFactory blockCreatorFactory = new IbftBlockCreatorFactory( - (gasLimit) -> gasLimit, + gasLimitCalculator, transactionPool.getPendingTransactions(), protocolContext, protocolSchedule, diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java index c8faae7b73d..a5850ba2f50 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java @@ -56,7 +56,8 @@ protected MiningCoordinator createMiningCoordinator( new DefaultBlockScheduler( MainnetBlockHeaderValidator.MINIMUM_SECONDS_SINCE_PARENT, MainnetBlockHeaderValidator.TIMESTAMP_TOLERANCE_S, - clock)); + clock), + gasLimitCalculator); final EthHashMiningCoordinator miningCoordinator = new EthHashMiningCoordinator(protocolContext.getBlockchain(), executor, syncState); diff --git a/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java b/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java index 676183c5423..e63db2b91f9 100644 --- a/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java +++ b/besu/src/test/java/org/hyperledger/besu/PrivacyTest.java @@ -16,6 +16,7 @@ import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; import org.hyperledger.besu.ethereum.core.Account; import org.hyperledger.besu.ethereum.core.Address; @@ -77,6 +78,7 @@ public void privacyPrecompiled() throws IOException { .clock(TestClock.fixed()) .privacyParameters(privacyParameters) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); final Address privacyContractAddress = Address.privacyPrecompiled(ADDRESS); diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java index 69fd957105d..57b6a02340c 100644 --- a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java +++ b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.cli.config.EthNetworkConfig; import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.controller.KeyPairUtil; import org.hyperledger.besu.controller.MainnetBesuControllerBuilder; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; @@ -143,6 +144,7 @@ private void syncFromGenesis(final SyncMode mode) throws Exception { .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) .storageProvider(createKeyValueStorageProvider(dbAhead)) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build()) { setupState(blockCount, controller.getProtocolSchedule(), controller.getProtocolContext()); } @@ -162,6 +164,7 @@ private void syncFromGenesis(final SyncMode mode) throws Exception { .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) .storageProvider(createKeyValueStorageProvider(dbAhead)) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); final String listenHost = InetAddress.getLoopbackAddress().getHostAddress(); final JsonRpcConfiguration aheadJsonRpcConfiguration = jsonRpcConfiguration(); @@ -220,6 +223,7 @@ private void syncFromGenesis(final SyncMode mode) throws Exception { .privacyParameters(PrivacyParameters.DEFAULT) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); final EnodeURL enode = runnerAhead.getLocalEnode().get(); final EthNetworkConfig behindEthNetworkConfiguration = diff --git a/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java b/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java index a4b0817b19a..8e164fe656f 100644 --- a/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/chainexport/RlpBlockExporterTest.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.chainimport.RlpBlockImporter; import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Block; @@ -88,6 +89,7 @@ private static BesuController createController() throws IOException { .dataDirectory(dataDir) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); } diff --git a/besu/src/test/java/org/hyperledger/besu/chainimport/JsonBlockImporterTest.java b/besu/src/test/java/org/hyperledger/besu/chainimport/JsonBlockImporterTest.java index 1108634ba57..a63e554e071 100644 --- a/besu/src/test/java/org/hyperledger/besu/chainimport/JsonBlockImporterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/chainimport/JsonBlockImporterTest.java @@ -19,6 +19,7 @@ import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.config.JsonUtil; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Address; @@ -425,6 +426,7 @@ protected BesuController createController(final GenesisConfigFile genesisConf .dataDirectory(dataDir) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); } } diff --git a/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java b/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java index 8d1faa5cd24..991d66a8c08 100644 --- a/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java @@ -17,6 +17,7 @@ import org.hyperledger.besu.config.GenesisConfigFile; import org.hyperledger.besu.controller.BesuController; +import org.hyperledger.besu.controller.GasLimitCalculator; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; import org.hyperledger.besu.ethereum.core.InMemoryStorageProvider; import org.hyperledger.besu.ethereum.core.MiningParametersTestBuilder; @@ -66,6 +67,7 @@ public void blockImport() throws IOException { .dataDirectory(dataDir) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); final RlpBlockImporter.ImportResult result = rlpBlockImporter.importBlockchain(source, targetController); @@ -105,6 +107,7 @@ public void ibftImport() throws IOException { .dataDirectory(dataDir) .clock(TestClock.fixed()) .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build()) + .targetGasLimit(GasLimitCalculator.DEFAULT) .build(); final RlpBlockImporter.ImportResult result = rlpBlockImporter.importBlockchain(source, controller); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index f996a1dddb6..cad51267e56 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -30,6 +30,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNotNull; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -174,6 +175,7 @@ public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() throws E verify(mockControllerBuilder).miningParameters(miningArg.capture()); verify(mockControllerBuilder).nodePrivateKeyFile(isNotNull()); verify(mockControllerBuilder).storageProvider(storageProviderArgumentCaptor.capture()); + verify(mockControllerBuilder).targetGasLimit(isNull()); verify(mockControllerBuilder).build(); assertThat(storageProviderArgumentCaptor.getValue()).isNotNull(); @@ -2751,4 +2753,34 @@ public void tomlThatHasInvalidOptions() throws IOException { assertThat(commandErrorOutput.toString()) .contains("Unknown options in TOML configuration file: invalid_option, invalid_option2"); } + + @Test + public void targetGasLimitIsEnabledWhenSpecified() throws Exception { + parseCommand("--target-gas-limit=10000000"); + + final ArgumentCaptor targetGasLimitArg = ArgumentCaptor.forClass(Long.class); + + verify(mockControllerBuilder).targetGasLimit(targetGasLimitArg.capture()); + verify(mockControllerBuilder).build(); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + + assertThat(targetGasLimitArg.getValue()).isEqualTo(10_000_000L); + } + + @Test + public void targetGasLimitIsDisabledWhenNotSpecified() throws Exception { + parseCommand(); + + final ArgumentCaptor targetGasLimitArg = ArgumentCaptor.forClass(Long.class); + + verify(mockControllerBuilder).targetGasLimit(targetGasLimitArg.capture()); + verify(mockControllerBuilder).build(); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + + assertThat(targetGasLimitArg.getValue()).isEqualTo(null); + } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 8cd613425d7..305ffb4fc45 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -167,6 +167,7 @@ public void initMocks() throws Exception { when(mockControllerBuilder.isPruningEnabled(anyBoolean())).thenReturn(mockControllerBuilder); when(mockControllerBuilder.pruningConfiguration(any())).thenReturn(mockControllerBuilder); when(mockControllerBuilder.genesisConfigOverrides(any())).thenReturn(mockControllerBuilder); + when(mockControllerBuilder.targetGasLimit(any())).thenReturn(mockControllerBuilder); // doReturn used because of generic BesuController doReturn(mockController).when(mockControllerBuilder).build(); diff --git a/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java new file mode 100644 index 00000000000..bebafd8cc5a --- /dev/null +++ b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package org.hyperledger.besu.controller; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.function.Function; + +import org.junit.Test; + +public class GasLimitCalculatorTest { + @Test + public void verifyGasLimitIsIncreasedWithinLimits() { + Function gasLimitCalculator = new GasLimitCalculator(10_000_000L); + assertThat(gasLimitCalculator.apply(8_000_000L)) + .isEqualTo(8_000_000L + GasLimitCalculator.ADJUSTMENT_FACTOR); + } + + @Test + public void verifyGasLimitIsDecreasedWithinLimits() { + Function gasLimitCalculator = new GasLimitCalculator(10_000_000L); + assertThat(gasLimitCalculator.apply(12_000_000L)) + .isEqualTo(12_000_000L - GasLimitCalculator.ADJUSTMENT_FACTOR); + } + + @Test + public void verifyGasLimitReachesTarget() { + final long target = 10_000_000L; + final long offset = GasLimitCalculator.ADJUSTMENT_FACTOR / 2; + Function gasLimitCalculator = new GasLimitCalculator(target); + assertThat(gasLimitCalculator.apply(target - offset)).isEqualTo(target); + assertThat(gasLimitCalculator.apply(target + offset)).isEqualTo(target); + } +} diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index c5ffd7dd9fb..df223091c88 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -109,3 +109,6 @@ revert-reason-enabled=false # Storage plugin to use key-value-storage="rocksdb" + +# Gas limit +target-gas-limit=8000000 \ No newline at end of file diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java index 2fe35765eb2..0dcbfdf383d 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java @@ -53,14 +53,16 @@ public CliqueMinerExecutor( final KeyPair nodeKeys, final MiningParameters miningParams, final AbstractBlockScheduler blockScheduler, - final EpochManager epochManager) { + final EpochManager epochManager, + final Function gasLimitCalculator) { super( protocolContext, executorService, protocolSchedule, pendingTransactions, miningParams, - blockScheduler); + blockScheduler, + gasLimitCalculator); this.nodeKeys = nodeKeys; this.localAddress = Util.publicKeyToAddress(nodeKeys.getPublicKey()); this.epochManager = epochManager; @@ -89,7 +91,7 @@ private CliqueBlockMiner createMiner( pendingTransactions, protocolContext, protocolSchedule, - (gasLimit) -> gasLimit, + gasLimitCalculator, nodeKeys, minTransactionGasPrice, header, diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java index 5c2418bf542..831ad12bdc8 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java @@ -46,6 +46,7 @@ import java.util.List; import java.util.Random; import java.util.concurrent.Executors; +import java.util.function.Function; import com.google.common.collect.Lists; import org.junit.Before; @@ -98,7 +99,8 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() { proposerKeyPair, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, vanityData, false), mock(CliqueBlockScheduler.class), - new EpochManager(EPOCH_LENGTH)); + new EpochManager(EPOCH_LENGTH), + Function.identity()); // NOTE: Passing in the *parent* block, so must be 1 less than EPOCH final BlockHeader header = blockHeaderBuilder.number(EPOCH_LENGTH - 1).buildHeader(); @@ -135,7 +137,8 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() { proposerKeyPair, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, vanityData, false), mock(CliqueBlockScheduler.class), - new EpochManager(EPOCH_LENGTH)); + new EpochManager(EPOCH_LENGTH), + Function.identity()); // Parent block was epoch, so the next block should contain no validators. final BlockHeader header = blockHeaderBuilder.number(EPOCH_LENGTH).buildHeader(); @@ -172,7 +175,8 @@ public void shouldUseLatestVanityData() { proposerKeyPair, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, initialVanityData, false), mock(CliqueBlockScheduler.class), - new EpochManager(EPOCH_LENGTH)); + new EpochManager(EPOCH_LENGTH), + Function.identity()); executor.setExtraData(modifiedVanityData); final BytesValue extraDataBytes = executor.calculateExtraData(blockHeaderBuilder.buildHeader()); diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractMinerExecutor.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractMinerExecutor.java index d3b49f044ee..0408b147e86 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractMinerExecutor.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractMinerExecutor.java @@ -25,6 +25,7 @@ import java.util.Optional; import java.util.concurrent.ExecutorService; +import java.util.function.Function; public abstract class AbstractMinerExecutor< C, M extends BlockMiner>> { @@ -34,6 +35,7 @@ public abstract class AbstractMinerExecutor< protected final ProtocolSchedule protocolSchedule; protected final PendingTransactions pendingTransactions; protected final AbstractBlockScheduler blockScheduler; + protected final Function gasLimitCalculator; protected volatile BytesValue extraData; protected volatile Wei minTransactionGasPrice; @@ -44,7 +46,8 @@ public AbstractMinerExecutor( final ProtocolSchedule protocolSchedule, final PendingTransactions pendingTransactions, final MiningParameters miningParams, - final AbstractBlockScheduler blockScheduler) { + final AbstractBlockScheduler blockScheduler, + final Function gasLimitCalculator) { this.protocolContext = protocolContext; this.executorService = executorService; this.protocolSchedule = protocolSchedule; @@ -52,6 +55,7 @@ public AbstractMinerExecutor( this.extraData = miningParams.getExtraData(); this.minTransactionGasPrice = miningParams.getMinTransactionGasPrice(); this.blockScheduler = blockScheduler; + this.gasLimitCalculator = gasLimitCalculator; } public abstract M startAsyncMining( diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutor.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutor.java index 44d22a21f26..332a7278e99 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutor.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutor.java @@ -37,14 +37,16 @@ public EthHashMinerExecutor( final ProtocolSchedule protocolSchedule, final PendingTransactions pendingTransactions, final MiningParameters miningParams, - final AbstractBlockScheduler blockScheduler) { + final AbstractBlockScheduler blockScheduler, + final Function gasLimitCalculator) { super( protocolContext, executorService, protocolSchedule, pendingTransactions, miningParams, - blockScheduler); + blockScheduler, + gasLimitCalculator); this.coinbase = miningParams.getCoinbase(); } @@ -77,7 +79,7 @@ private EthHashBlockMiner createMiner( pendingTransactions, protocolContext, protocolSchedule, - (gasLimit) -> gasLimit, + gasLimitCalculator, solver, minTransactionGasPrice, parentHeader); diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java index 5fc373c1ab4..a4d4ebbb65e 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.util.Subscribers; import java.util.concurrent.Executors; +import java.util.function.Function; import org.junit.Test; @@ -49,7 +50,8 @@ public void startingMiningWithoutCoinbaseThrowsException() { null, pendingTransactions, miningParameters, - new DefaultBlockScheduler(1, 10, TestClock.fixed())); + new DefaultBlockScheduler(1, 10, TestClock.fixed()), + Function.identity()); assertThatExceptionOfType(CoinbaseNotSetException.class) .isThrownBy(() -> executor.startAsyncMining(Subscribers.create(), null)) @@ -74,7 +76,8 @@ public void settingCoinbaseToNullThrowsException() { null, pendingTransactions, miningParameters, - new DefaultBlockScheduler(1, 10, TestClock.fixed())); + new DefaultBlockScheduler(1, 10, TestClock.fixed()), + Function.identity()); assertThatExceptionOfType(IllegalArgumentException.class) .isThrownBy(() -> executor.setCoinbase(null)) From fd742388fb7b88aa07d37c3319064314c4a0499a Mon Sep 17 00:00:00 2001 From: Christian Felde Date: Thu, 19 Sep 2019 15:53:53 +0100 Subject: [PATCH 2/4] [PAN-3023] Add command line option for target gas limit Signed-off-by: Christian Felde --- .../org/hyperledger/besu/cli/BesuCommand.java | 2 +- .../controller/BesuControllerBuilder.java | 2 +- .../besu/controller/GasLimitCalculator.java | 53 ++++++++++++++----- .../hyperledger/besu/cli/BesuCommandTest.java | 15 +++--- .../controller/GasLimitCalculatorTest.java | 23 ++++++-- 5 files changed, 71 insertions(+), 24 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index ece4f19838a..4feead58494 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1054,7 +1054,7 @@ public BesuControllerBuilder getControllerBuilder() { .isPruningEnabled(isPruningEnabled) .pruningConfiguration(buildPruningConfiguration()) .genesisConfigOverrides(genesisConfigOverrides) - .targetGasLimit(targetGasLimit); + .targetGasLimit(targetGasLimit == null ? Optional.empty() : Optional.of(targetGasLimit)); } catch (final IOException e) { throw new ExecutionException(this.commandLine, "Invalid path", e); } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index 11ebe0adf05..584d00618e3 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -180,7 +180,7 @@ public BesuControllerBuilder genesisConfigOverrides( return this; } - public BesuControllerBuilder targetGasLimit(final Long targetGasLimit) { + public BesuControllerBuilder targetGasLimit(final Optional targetGasLimit) { this.gasLimitCalculator = new GasLimitCalculator(targetGasLimit); return this; } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java index 204bfae2804..e755ac9f918 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java @@ -12,6 +12,7 @@ */ package org.hyperledger.besu.controller; +import java.util.Optional; import java.util.function.Function; import org.apache.logging.log4j.LogManager; @@ -20,27 +21,53 @@ public class GasLimitCalculator implements Function { private static final Logger LOG = LogManager.getLogger(); public static final long ADJUSTMENT_FACTOR = 1024L; - public static final Long DEFAULT = null; - private final long targetGasLimit; + public static final Optional DEFAULT = Optional.empty(); + private final Optional targetGasLimit; - public GasLimitCalculator(final Long targetGasLimit) { - this.targetGasLimit = targetGasLimit == null ? 0L : targetGasLimit; + public GasLimitCalculator(final Optional targetGasLimit) { + if (targetGasLimit.orElse(0L) < 0L) { + throw new IllegalArgumentException("Invalid target gas limit"); + } + + this.targetGasLimit = targetGasLimit; } @Override public Long apply(final Long gasLimit) { - long newGasLimit; + long newGasLimit = + targetGasLimit + .map( + target -> { + if (target > gasLimit) { + return Math.min(target, safeAdd(gasLimit)); + } else if (target < gasLimit) { + return Math.max(target, safeSub(gasLimit)); + } else { + return gasLimit; + } + }) + .orElse(gasLimit); - if (targetGasLimit > gasLimit) { - newGasLimit = Math.min(targetGasLimit, gasLimit + ADJUSTMENT_FACTOR); - } else if (targetGasLimit < gasLimit) { - newGasLimit = Math.max(targetGasLimit, gasLimit - ADJUSTMENT_FACTOR); - } else { - return gasLimit; + if (newGasLimit != gasLimit) { + LOG.debug("Adjusting block gas limit from {} to {}", gasLimit, newGasLimit); } - LOG.debug("Adjusting block gas limit from {} to {}", gasLimit, newGasLimit); - return newGasLimit; } + + private long safeAdd(final long gasLimit) { + if (gasLimit + ADJUSTMENT_FACTOR > gasLimit) { + return gasLimit + ADJUSTMENT_FACTOR; + } else { + return Long.MAX_VALUE; + } + } + + private long safeSub(final long gasLimit) { + if (gasLimit - ADJUSTMENT_FACTOR < gasLimit) { + return Math.max(gasLimit - ADJUSTMENT_FACTOR, 0); + } else { + return 0; + } + } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index cad51267e56..3102d9224a3 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -30,7 +30,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNotNull; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -175,7 +174,7 @@ public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() throws E verify(mockControllerBuilder).miningParameters(miningArg.capture()); verify(mockControllerBuilder).nodePrivateKeyFile(isNotNull()); verify(mockControllerBuilder).storageProvider(storageProviderArgumentCaptor.capture()); - verify(mockControllerBuilder).targetGasLimit(isNull()); + verify(mockControllerBuilder).targetGasLimit(eq(Optional.empty())); verify(mockControllerBuilder).build(); assertThat(storageProviderArgumentCaptor.getValue()).isNotNull(); @@ -2758,7 +2757,9 @@ public void tomlThatHasInvalidOptions() throws IOException { public void targetGasLimitIsEnabledWhenSpecified() throws Exception { parseCommand("--target-gas-limit=10000000"); - final ArgumentCaptor targetGasLimitArg = ArgumentCaptor.forClass(Long.class); + @SuppressWarnings("unchecked") + final ArgumentCaptor> targetGasLimitArg = + ArgumentCaptor.forClass(Optional.class); verify(mockControllerBuilder).targetGasLimit(targetGasLimitArg.capture()); verify(mockControllerBuilder).build(); @@ -2766,14 +2767,16 @@ public void targetGasLimitIsEnabledWhenSpecified() throws Exception { assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); - assertThat(targetGasLimitArg.getValue()).isEqualTo(10_000_000L); + assertThat(targetGasLimitArg.getValue()).isEqualTo(Optional.of(10_000_000L)); } @Test public void targetGasLimitIsDisabledWhenNotSpecified() throws Exception { parseCommand(); - final ArgumentCaptor targetGasLimitArg = ArgumentCaptor.forClass(Long.class); + @SuppressWarnings("unchecked") + final ArgumentCaptor> targetGasLimitArg = + ArgumentCaptor.forClass(Optional.class); verify(mockControllerBuilder).targetGasLimit(targetGasLimitArg.capture()); verify(mockControllerBuilder).build(); @@ -2781,6 +2784,6 @@ public void targetGasLimitIsDisabledWhenNotSpecified() throws Exception { assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); - assertThat(targetGasLimitArg.getValue()).isEqualTo(null); + assertThat(targetGasLimitArg.getValue()).isEqualTo(Optional.empty()); } } diff --git a/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java index bebafd8cc5a..9b82d648e85 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java @@ -14,6 +14,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.Optional; import java.util.function.Function; import org.junit.Test; @@ -21,14 +22,14 @@ public class GasLimitCalculatorTest { @Test public void verifyGasLimitIsIncreasedWithinLimits() { - Function gasLimitCalculator = new GasLimitCalculator(10_000_000L); + Function gasLimitCalculator = new GasLimitCalculator(Optional.of(10_000_000L)); assertThat(gasLimitCalculator.apply(8_000_000L)) .isEqualTo(8_000_000L + GasLimitCalculator.ADJUSTMENT_FACTOR); } @Test public void verifyGasLimitIsDecreasedWithinLimits() { - Function gasLimitCalculator = new GasLimitCalculator(10_000_000L); + Function gasLimitCalculator = new GasLimitCalculator(Optional.of(10_000_000L)); assertThat(gasLimitCalculator.apply(12_000_000L)) .isEqualTo(12_000_000L - GasLimitCalculator.ADJUSTMENT_FACTOR); } @@ -37,8 +38,24 @@ public void verifyGasLimitIsDecreasedWithinLimits() { public void verifyGasLimitReachesTarget() { final long target = 10_000_000L; final long offset = GasLimitCalculator.ADJUSTMENT_FACTOR / 2; - Function gasLimitCalculator = new GasLimitCalculator(target); + Function gasLimitCalculator = new GasLimitCalculator(Optional.of(target)); assertThat(gasLimitCalculator.apply(target - offset)).isEqualTo(target); assertThat(gasLimitCalculator.apply(target + offset)).isEqualTo(target); } + + @Test + public void verifyNoNegative() { + final long target = 0L; + final long offset = GasLimitCalculator.ADJUSTMENT_FACTOR / 2; + Function gasLimitCalculator = new GasLimitCalculator(Optional.of(target)); + assertThat(gasLimitCalculator.apply(target + offset)).isEqualTo(target); + } + + @Test + public void verifyNoOverflow() { + final long target = Long.MAX_VALUE; + final long offset = GasLimitCalculator.ADJUSTMENT_FACTOR / 2; + Function gasLimitCalculator = new GasLimitCalculator(Optional.of(target)); + assertThat(gasLimitCalculator.apply(target - offset)).isEqualTo(target); + } } From e0012e74d3f4e05ef09a62ba8d96a1ef347d661f Mon Sep 17 00:00:00 2001 From: Christian Felde Date: Tue, 24 Sep 2019 17:35:55 +0100 Subject: [PATCH 3/4] [PAN-3023] Add command line option for target gas limit Signed-off-by: Christian Felde --- .../org/hyperledger/besu/controller/GasLimitCalculator.java | 2 ++ .../org/hyperledger/besu/controller/GasLimitCalculatorTest.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java index e755ac9f918..a92c66852fb 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java @@ -9,6 +9,8 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 */ package org.hyperledger.besu.controller; diff --git a/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java index 9b82d648e85..a7903a981e2 100644 --- a/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java +++ b/besu/src/test/java/org/hyperledger/besu/controller/GasLimitCalculatorTest.java @@ -9,6 +9,8 @@ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 */ package org.hyperledger.besu.controller; From a18c3eee5b2f43710057adc54407631cd91045ba Mon Sep 17 00:00:00 2001 From: Christian Felde Date: Mon, 30 Sep 2019 09:50:05 +0100 Subject: [PATCH 4/4] [PAN-3023] Add command line option for target gas limit Signed-off-by: Christian Felde --- .../besu/controller/GasLimitCalculator.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java index a92c66852fb..fbcf6bcb7bb 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/GasLimitCalculator.java @@ -58,17 +58,17 @@ public Long apply(final Long gasLimit) { } private long safeAdd(final long gasLimit) { - if (gasLimit + ADJUSTMENT_FACTOR > gasLimit) { - return gasLimit + ADJUSTMENT_FACTOR; - } else { + try { + return Math.addExact(gasLimit, ADJUSTMENT_FACTOR); + } catch (final ArithmeticException ex) { return Long.MAX_VALUE; } } private long safeSub(final long gasLimit) { - if (gasLimit - ADJUSTMENT_FACTOR < gasLimit) { - return Math.max(gasLimit - ADJUSTMENT_FACTOR, 0); - } else { + try { + return Math.max(Math.subtractExact(gasLimit, ADJUSTMENT_FACTOR), 0); + } catch (final ArithmeticException ex) { return 0; } }