Time limited block creation (#6044)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
hyperledger · Nov 8, 2023 · 8319fae · 8319fae
1 parent 636ad8a
commit 8319fae
Show file tree

Hide file tree

Showing 57 changed files with 948 additions and 324 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 - Upgrade EVM Reference Tests to v13 (Cancun) [#6114](https://github.com/hyperledger/besu/pull/6114)
 - Add `yParity` to GraphQL and JSON-RPC for relevant querise. [6119](https://github.com/hyperledger/besu/pull/6119)
 - Force tx replacement price bump to zero when zero base fee market is configured or `--min-gas-price` is set to 0. This allows for easier tx replacement in networks where there is not gas price. [#6079](https://github.com/hyperledger/besu/pull/6079)
+- Introduce the possibility to limit the time spent selecting pending transactions during block creation, using the new experimental option `Xblock-txs-selection-max-time` on PoS and PoW networks (by default set to 5000ms) or `Xpoa-block-txs-selection-max-time` on PoA networks (by default 75% of the min block time) [#6044](https://github.com/hyperledger/besu/pull/6044)
 
 ### Bug fixes
 - Upgrade netty to address CVE-2023-44487, CVE-2023-34462 [#6100](https://github.com/hyperledger/besu/pull/6100)
@@ -33,8 +34,8 @@
 ### Additions and Improvements
 - New option `--tx-pool-priority-senders` to specify a list of senders, that has the effect to prioritize any transactions sent by these senders from any source [#5959](https://github.com/hyperledger/besu/pull/5959)
 - Cache last n blocks by using a new Besu flag `--cache-last-blocks=n` [#6009](https://github.com/hyperledger/besu/pull/6009)
-- Optimize performances of RPC method `eth_feeHistory` [#6011](https://github.com/hyperledger/besu/pull/6011) [#6035](https://github.com/hyperledger/besu/pull/6035) 
-- Logging summary of plugins at Info as part of the config overview [#5964](https://github.com/hyperledger/besu/pull/5964) [#6049](https://github.com/hyperledger/besu/pull/6049) 
+- Optimize performances of RPC method `eth_feeHistory` [#6011](https://github.com/hyperledger/besu/pull/6011) [#6035](https://github.com/hyperledger/besu/pull/6035)
+- Logging summary of plugins at Info as part of the config overview [#5964](https://github.com/hyperledger/besu/pull/5964) [#6049](https://github.com/hyperledger/besu/pull/6049)
 - Layered tx pool memory improvements [#5985](https://github.com/hyperledger/besu/pull/5985) [#5974](https://github.com/hyperledger/besu/pull/5974)
 - Update Bouncy Castle to 1.76, and force the use of the `jdk18on` variant [#5748](https://github.com/hyperledger/besu/pull/5748)
 - Add GraphQL support for new fields in Cancun [#5923](https://github.com/hyperledger/besu/pull/5923) [#5975](https://github.com/hyperledger/besu/pull/5975)

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -122,6 +122,7 @@
 import org.hyperledger.besu.ethereum.api.tls.TlsClientAuthConfiguration;
 import org.hyperledger.besu.ethereum.api.tls.TlsConfiguration;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
+import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
 import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
@@ -1806,8 +1807,7 @@ private void validateRequiredOptions() {
   }
 
   private void validateMiningParams() {
-    miningOptions.validate(
-        commandLine, logger, isMergeEnabled(), getActualGenesisConfigOptions().isEthHash());
+    miningOptions.validate(commandLine, getActualGenesisConfigOptions(), isMergeEnabled(), logger);
   }
 
   /**
@@ -2830,11 +2830,36 @@ private TransactionPoolConfiguration buildTransactionPoolConfiguration() {
 
   private MiningParameters getMiningParameters() {
     if (miningParameters == null) {
-      miningParameters = miningOptions.toDomainObject();
+      final var miningParametersBuilder =
+          ImmutableMiningParameters.builder().from(miningOptions.toDomainObject());
+      final var actualGenesisOptions = getActualGenesisConfigOptions();
+      if (actualGenesisOptions.isPoa()) {
+        miningParametersBuilder.unstable(
+            ImmutableMiningParameters.Unstable.builder()
+                .minBlockTime(getMinBlockTime(actualGenesisOptions))
+                .build());
+      }
+      miningParameters = miningParametersBuilder.build();
     }
     return miningParameters;
   }
 
+  private int getMinBlockTime(final GenesisConfigOptions genesisConfigOptions) {
+    if (genesisConfigOptions.isClique()) {
+      return genesisConfigOptions.getCliqueConfigOptions().getBlockPeriodSeconds();
+    }
+
+    if (genesisConfigOptions.isIbft2()) {
+      return genesisConfigOptions.getBftConfigOptions().getBlockPeriodSeconds();
+    }
+
+    if (genesisConfigOptions.isQbft()) {
+      return genesisConfigOptions.getQbftConfigOptions().getBlockPeriodSeconds();
+    }
+
+    throw new IllegalArgumentException("Should only be called for a PoA network");
+  }
+
   private boolean isPruningEnabled() {
     return pruningEnabled;
   }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java
@@ -15,23 +15,29 @@
 package org.hyperledger.besu.cli.options;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.MutableInitValues.DEFAULT_EXTRA_DATA;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.MutableInitValues.DEFAULT_MIN_BLOCK_OCCUPANCY_RATIO;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.MutableInitValues.DEFAULT_MIN_PRIORITY_FEE_PER_GAS;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.MutableInitValues.DEFAULT_MIN_TRANSACTION_GAS_PRICE;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_MAX_OMMERS_DEPTH;
+import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME;
+import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_POA_BLOCK_TXS_SELECTION_MAX_TIME;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_POS_BLOCK_CREATION_MAX_TIME;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_POS_BLOCK_CREATION_REPETITION_MIN_DURATION;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_POW_JOB_TTL;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_REMOTE_SEALERS_LIMIT;
 import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_REMOTE_SEALERS_TTL;
 
+import org.hyperledger.besu.cli.converter.PercentageConverter;
 import org.hyperledger.besu.cli.util.CommandLineUtils;
+import org.hyperledger.besu.config.GenesisConfigOptions;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
 import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.MutableInitValues;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
+import org.hyperledger.besu.util.number.Percentage;
 
 import java.util.List;
 
@@ -162,6 +168,25 @@ static class Unstable {
                 + " then it waits before next repetition. Must be positive and ≤ 2000 (default: ${DEFAULT-VALUE} milliseconds)")
     private Long posBlockCreationRepetitionMinDuration =
         DEFAULT_POS_BLOCK_CREATION_REPETITION_MIN_DURATION;
+
+    @CommandLine.Option(
+        hidden = true,
+        names = {"--Xblock-txs-selection-max-time"},
+        description =
+            "Specifies the maximum time, in milliseconds, that could be spent selecting transactions to be included in the block."
+                + " Not compatible with PoA networks, see Xpoa-block-txs-selection-max-time."
+                + " Must be positive and ≤ (default: ${DEFAULT-VALUE})")
+    private Long nonPoaBlockTxsSelectionMaxTime = DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME;
+
+    @CommandLine.Option(
+        hidden = true,
+        names = {"--Xpoa-block-txs-selection-max-time"},
+        converter = PercentageConverter.class,
+        description =
+            "Specifies the maximum time that could be spent selecting transactions to be included in the block, as a percentage of the fixed block time of the PoA network."
+                + " To be only used on PoA networks, for other networks see Xblock-txs-selection-max-time."
+                + " (default: ${DEFAULT-VALUE})")
+    private Percentage poaBlockTxsSelectionMaxTime = DEFAULT_POA_BLOCK_TXS_SELECTION_MAX_TIME;
   }
 
   private MiningOptions() {}
@@ -180,15 +205,15 @@ public static MiningOptions create() {
    * options are valid for the selected implementation.
    *
    * @param commandLine the full commandLine to check all the options specified by the user
-   * @param logger the logger
+   * @param genesisConfigOptions is EthHash?
    * @param isMergeEnabled is the Merge enabled?
-   * @param isEthHash is EthHash?
+   * @param logger the logger
    */
   public void validate(
       final CommandLine commandLine,
-      final Logger logger,
+      final GenesisConfigOptions genesisConfigOptions,
       final boolean isMergeEnabled,
-      final boolean isEthHash) {
+      final Logger logger) {
     if (Boolean.TRUE.equals(isMiningEnabled) && coinbase == null) {
       throw new ParameterException(
           commandLine,
@@ -203,7 +228,7 @@ public void validate(
     }
 
     // Check that block producer options work
-    if (!isMergeEnabled && isEthHash) {
+    if (!isMergeEnabled && genesisConfigOptions.isEthHash()) {
       CommandLineUtils.checkOptionDependencies(
           logger,
           commandLine,
@@ -231,14 +256,41 @@ public void validate(
     if (unstableOptions.posBlockCreationMaxTime <= 0
         || unstableOptions.posBlockCreationMaxTime > DEFAULT_POS_BLOCK_CREATION_MAX_TIME) {
       throw new ParameterException(
-          commandLine, "--Xpos-block-creation-max-time must be positive and ≤ 12000");
+          commandLine,
+          "--Xpos-block-creation-max-time must be positive and ≤ "
+              + DEFAULT_POS_BLOCK_CREATION_MAX_TIME);
     }
 
     if (unstableOptions.posBlockCreationRepetitionMinDuration <= 0
         || unstableOptions.posBlockCreationRepetitionMinDuration > 2000) {
       throw new ParameterException(
           commandLine, "--Xpos-block-creation-repetition-min-duration must be positive and ≤ 2000");
     }
+
+    if (genesisConfigOptions.isPoa()) {
+      CommandLineUtils.failIfOptionDoesntMeetRequirement(
+          commandLine,
+          "--Xblock-txs-selection-max-time can't be used with PoA networks,"
+              + " see Xpoa-block-txs-selection-max-time instead",
+          false,
+          singletonList("--Xblock-txs-selection-max-time"));
+    } else {
+      CommandLineUtils.failIfOptionDoesntMeetRequirement(
+          commandLine,
+          "--Xpoa-block-txs-selection-max-time can be only used with PoA networks,"
+              + " see --Xblock-txs-selection-max-time instead",
+          false,
+          singletonList("--Xpoa-block-txs-selection-max-time"));
+
+      if (unstableOptions.nonPoaBlockTxsSelectionMaxTime <= 0
+          || unstableOptions.nonPoaBlockTxsSelectionMaxTime
+              > DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME) {
+        throw new ParameterException(
+            commandLine,
+            "--Xblock-txs-selection-max-time must be positive and ≤ "
+                + DEFAULT_NON_POA_BLOCK_TXS_SELECTION_MAX_TIME);
+      }
+    }
   }
 
   static MiningOptions fromConfig(final MiningParameters miningParameters) {
@@ -265,6 +317,10 @@ static MiningOptions fromConfig(final MiningParameters miningParameters) {
         miningParameters.getUnstable().getPosBlockCreationMaxTime();
     miningOptions.unstableOptions.posBlockCreationRepetitionMinDuration =
         miningParameters.getUnstable().getPosBlockCreationRepetitionMinDuration();
+    miningOptions.unstableOptions.nonPoaBlockTxsSelectionMaxTime =
+        miningParameters.getUnstable().getBlockTxsSelectionMaxTime();
+    miningOptions.unstableOptions.poaBlockTxsSelectionMaxTime =
+        miningParameters.getUnstable().getPoaBlockTxsSelectionMaxTime();
 
     miningParameters.getCoinbase().ifPresent(coinbase -> miningOptions.coinbase = coinbase);
     miningParameters.getTargetGasLimit().ifPresent(tgl -> miningOptions.targetGasLimit = tgl);
@@ -304,6 +360,8 @@ public MiningParameters toDomainObject() {
                     .posBlockCreationMaxTime(unstableOptions.posBlockCreationMaxTime)
                     .posBlockCreationRepetitionMinDuration(
                         unstableOptions.posBlockCreationRepetitionMinDuration)
+                    .nonPoaBlockTxsSelectionMaxTime(unstableOptions.nonPoaBlockTxsSelectionMaxTime)
+                    .poaBlockTxsSelectionMaxTime(unstableOptions.poaBlockTxsSelectionMaxTime)
                     .build());
 
     return miningParametersBuilder.build();

diff --git 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(
                 localAddress,
                 secondsBetweenBlocks),
             epochManager,
-            createEmptyBlocks);
+            createEmptyBlocks,
+            ethProtocolManager.ethContext().getScheduler());
     final CliqueMiningCoordinator miningCoordinator =
         new CliqueMiningCoordinator(
             protocolContext.getBlockchain(),

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java
@@ -153,7 +153,8 @@ protected MiningCoordinator createMiningCoordinator(
             forksSchedule,
             miningParameters,
             localAddress,
-            bftExtraDataCodec().get());
+            bftExtraDataCodec().get(),
+            ethProtocolManager.ethContext().getScheduler());
 
     final ValidatorProvider validatorProvider =
         protocolContext.getConsensusContext(BftContext.class).getValidatorProvider();

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MainnetBesuControllerBuilder.java
@@ -55,7 +55,8 @@ protected MiningCoordinator createMiningCoordinator(
                 MainnetBlockHeaderValidator.MINIMUM_SECONDS_SINCE_PARENT,
                 MainnetBlockHeaderValidator.TIMESTAMP_TOLERANCE_S,
                 clock),
-            epochCalculator);
+            epochCalculator,
+            ethProtocolManager.ethContext().getScheduler());
 
     final PoWMiningCoordinator miningCoordinator =
         new PoWMiningCoordinator(

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java
@@ -35,7 +35,6 @@
 import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManager;
 import org.hyperledger.besu.ethereum.eth.manager.EthScheduler;
 import org.hyperledger.besu.ethereum.eth.manager.MergePeerFilter;
-import org.hyperledger.besu.ethereum.eth.manager.MonitoredExecutors;
 import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator;
 import org.hyperledger.besu.ethereum.eth.peervalidation.RequiredBlocksPeerValidator;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
@@ -46,13 +45,10 @@
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ScheduleBasedBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
-import org.hyperledger.besu.plugin.services.MetricsSystem;
 
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalLong;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutorService;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.slf4j.Logger;
@@ -85,7 +81,7 @@ protected MiningCoordinator createMiningCoordinator(
             syncState,
             BackwardChain.from(
                 storageProvider, ScheduleBasedBlockHeaderFunctions.create(protocolSchedule))),
-        metricsSystem);
+        ethProtocolManager.ethContext().getScheduler());
   }
 
   @Override
@@ -146,7 +142,7 @@ protected EthProtocolManager createEthProtocolManager(
    * @param miningParameters the mining parameters
    * @param syncState the sync state
    * @param backwardSyncContext the backward sync context
-   * @param metricsSystem the metrics system
+   * @param ethScheduler the scheduler
    * @return the mining coordinator
    */
   protected MiningCoordinator createTransitionMiningCoordinator(
@@ -156,24 +152,18 @@ protected MiningCoordinator createTransitionMiningCoordinator(
       final MiningParameters miningParameters,
       final SyncState syncState,
       final BackwardSyncContext backwardSyncContext,
-      final MetricsSystem metricsSystem) {
+      final EthScheduler ethScheduler) {
 
     this.syncState.set(syncState);
 
-    final ExecutorService blockBuilderExecutor =
-        MonitoredExecutors.newSingleThreadExecutor("PoS-Block-Builder", metricsSystem);
-
     final GenesisConfigOptions genesisConfigOptions = configOptionsSupplier.get();
     final Optional<Address> depositContractAddress =
         genesisConfigOptions.getDepositContractAddress();
 
     return new MergeCoordinator(
         protocolContext,
         protocolSchedule,
-        task -> {
-          LOG.debug("Block builder executor status {}", blockBuilderExecutor);
-          return CompletableFuture.runAsync(task, blockBuilderExecutor);
-        },
+        ethScheduler,
         transactionPool,
         miningParameters,
         backwardSyncContext,

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -191,7 +191,8 @@ protected MiningCoordinator createMiningCoordinator(
             qbftForksSchedule,
             miningParameters,
             localAddress,
-            bftExtraDataCodec().get());
+            bftExtraDataCodec().get(),
+            ethProtocolManager.ethContext().getScheduler());
 
     final ValidatorProvider validatorProvider =
         protocolContext.getConsensusContext(BftContext.class).getValidatorProvider();

diff --git a/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/TransitionBesuControllerBuilder.java
@@ -144,7 +144,7 @@ protected MiningCoordinator createMiningCoordinator(
                 transitionMiningParameters,
                 syncState,
                 transitionBackwardsSyncContext,
-                metricsSystem));
+                ethProtocolManager.ethContext().getScheduler()));
     initTransitionWatcher(protocolContext, composedCoordinator);
     return composedCoordinator;
   }