Layered txpool by default and txpool options hoverhaul (hyperledger#5772

) Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
garyschulte · Sep 20, 2023 · a6fe86d · a6fe86d
1 parent 58634af
commit a6fe86d
Show file tree

Hide file tree

Showing 46 changed files with 1,184 additions and 691 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,11 +5,15 @@
 
 ### Breaking Changes
 - Removed support for Kotti network (ETC) [#5816](https://github.com/hyperledger/besu/pull/5816)
+- Layered transaction pool implementation is now stable and enabled by default, so the following changes to experimental options have been done [#5772](https://github.com/hyperledger/besu):
+    - `--Xlayered-tx-pool` is gone, to select the implementation use the new `--tx-pool` option with values `layered` (default) or `legacy`
+    - `--Xlayered-tx-pool-layer-max-capacity`, `--Xlayered-tx-pool-max-prioritized` and `--Xlayered-tx-pool-max-future-by-sender` just drop the `X` and keep the same behavior
 
 ### Additions and Improvements
+- Layered transaction pool implementation is now stable and enabled by default. If you want still to use the legacy implementation, use `--tx-pool=legacy` [#5772](https://github.com/hyperledger/besu)
 
 ### Bug Fixes
-- do not create ignorable storage on revert storage-variables subcommand [#5830](https://github.com/hyperledger/besu/pull/5830) 
+- do not create ignorable storage on revert storage-variables subcommand [#5830](https://github.com/hyperledger/besu/pull/5830)
 
 ### Download Links
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -46,7 +46,6 @@
 import org.hyperledger.besu.chainimport.RlpBlockImporter;
 import org.hyperledger.besu.cli.config.EthNetworkConfig;
 import org.hyperledger.besu.cli.config.NetworkName;
-import org.hyperledger.besu.cli.converter.FractionConverter;
 import org.hyperledger.besu.cli.converter.MetricCategoryConverter;
 import org.hyperledger.besu.cli.converter.PercentageConverter;
 import org.hyperledger.besu.cli.custom.CorsAllowedOriginsProperty;
@@ -59,6 +58,7 @@
 import org.hyperledger.besu.cli.options.stable.LoggingLevelOption;
 import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption;
 import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions;
+import org.hyperledger.besu.cli.options.stable.TransactionPoolOptions;
 import org.hyperledger.besu.cli.options.unstable.ChainPruningOptions;
 import org.hyperledger.besu.cli.options.unstable.DnsOptions;
 import org.hyperledger.besu.cli.options.unstable.EthProtocolOptions;
@@ -73,7 +73,6 @@
 import org.hyperledger.besu.cli.options.unstable.PrivacyPluginOptions;
 import org.hyperledger.besu.cli.options.unstable.RPCOptions;
 import org.hyperledger.besu.cli.options.unstable.SynchronizerOptions;
-import org.hyperledger.besu.cli.options.unstable.TransactionPoolOptions;
 import org.hyperledger.besu.cli.presynctasks.PreSynchronizationTaskRunner;
 import org.hyperledger.besu.cli.presynctasks.PrivateDatabaseMigrationPreSyncTask;
 import org.hyperledger.besu.cli.subcommands.PasswordSubCommand;
@@ -127,6 +126,7 @@
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
 import org.hyperledger.besu.ethereum.eth.sync.SyncMode;
 import org.hyperledger.besu.ethereum.eth.sync.SynchronizerConfiguration;
+import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.mainnet.FrontierTargetingGasLimitCalculator;
 import org.hyperledger.besu.ethereum.p2p.config.DiscoveryConfiguration;
@@ -280,7 +280,9 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   final SynchronizerOptions unstableSynchronizerOptions = SynchronizerOptions.create();
   final EthProtocolOptions unstableEthProtocolOptions = EthProtocolOptions.create();
   final MetricsCLIOptions unstableMetricsCLIOptions = MetricsCLIOptions.create();
-  final TransactionPoolOptions unstableTransactionPoolOptions = TransactionPoolOptions.create();
+  final org.hyperledger.besu.cli.options.unstable.TransactionPoolOptions
+      unstableTransactionPoolOptions =
+          org.hyperledger.besu.cli.options.unstable.TransactionPoolOptions.create();
   private final DnsOptions unstableDnsOptions = DnsOptions.create();
   private final MiningOptions unstableMiningOptions = MiningOptions.create();
   private final NatOptions unstableNatOptions = NatOptions.create();
@@ -298,6 +300,10 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
       NodePrivateKeyFileOption.create();
   private final LoggingLevelOption loggingLevelOption = LoggingLevelOption.create();
 
+  @CommandLine.ArgGroup(validate = false, heading = "@|bold Tx Pool Common Options|@%n")
+  final org.hyperledger.besu.cli.options.stable.TransactionPoolOptions
+      stableTransactionPoolOptions = TransactionPoolOptions.create();
+
   private final RunnerBuilder runnerBuilder;
   private final BesuController.Builder controllerBuilderFactory;
   private final BesuPluginContextImpl besuPluginContext;
@@ -454,10 +460,8 @@ static class P2PDiscoveryOptionGroup {
             "The maximum percentage of P2P connections that can be initiated remotely. Must be between 0 and 100 inclusive. (default: ${DEFAULT-VALUE})",
         arity = "1",
         converter = PercentageConverter.class)
-    private final Integer maxRemoteConnectionsPercentage =
-        Fraction.fromFloat(DEFAULT_FRACTION_REMOTE_WIRE_CONNECTIONS_ALLOWED)
-            .toPercentage()
-            .getValue();
+    private final Percentage maxRemoteConnectionsPercentage =
+        Fraction.fromFloat(DEFAULT_FRACTION_REMOTE_WIRE_CONNECTIONS_ALLOWED).toPercentage();
 
     @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
     @CommandLine.Option(
@@ -1111,13 +1115,6 @@ static class MinerOptionGroup {
       arity = "1")
   private final Wei minTransactionGasPrice = DEFAULT_MIN_TRANSACTION_GAS_PRICE;
 
-  @Option(
-      names = {"--rpc-tx-feecap"},
-      description =
-          "Maximum transaction fees (in Wei) accepted for transaction submitted through RPC (default: ${DEFAULT-VALUE})",
-      arity = "1")
-  private final Wei txFeeCap = DEFAULT_RPC_TX_FEE_CAP;
-
   @Option(
       names = {"--min-block-occupancy-ratio"},
       description = "Minimum occupancy ratio for a mined block (default: ${DEFAULT-VALUE})",
@@ -1209,75 +1206,6 @@ static class PermissionsOptionGroup {
           "Sets target gas limit per block. If set, each block's gas limit will approach this setting over time if the current gas limit is different.")
   private final Long targetGasLimit = null;
 
-  // Tx Pool Option Group
-  @CommandLine.ArgGroup(validate = false, heading = "@|bold Tx Pool Options|@%n")
-  TxPoolOptionGroup txPoolOptionGroup = new TxPoolOptionGroup();
-
-  static class TxPoolOptionGroup {
-    @CommandLine.Option(
-        names = {"--tx-pool-disable-locals"},
-        paramLabel = "<Boolean>",
-        description =
-            "Set to true if transactions sent via RPC should have the same checks and not be prioritized over remote ones (default: ${DEFAULT-VALUE})",
-        fallbackValue = "true",
-        arity = "0..1")
-    private Boolean disableLocalTxs = TransactionPoolConfiguration.DEFAULT_DISABLE_LOCAL_TXS;
-
-    @CommandLine.Option(
-        names = {"--tx-pool-enable-save-restore"},
-        paramLabel = "<Boolean>",
-        description =
-            "Set to true to enable saving the txpool content to file on shutdown and reloading it on startup (default: ${DEFAULT-VALUE})",
-        fallbackValue = "true",
-        arity = "0..1")
-    private Boolean saveRestoreEnabled = TransactionPoolConfiguration.DEFAULT_ENABLE_SAVE_RESTORE;
-
-    @CommandLine.Option(
-        names = {"--tx-pool-limit-by-account-percentage"},
-        paramLabel = "<DOUBLE>",
-        converter = FractionConverter.class,
-        description =
-            "Maximum portion of the transaction pool which a single account may occupy with future transactions (default: ${DEFAULT-VALUE})",
-        arity = "1")
-    private Float txPoolLimitByAccountPercentage =
-        TransactionPoolConfiguration.DEFAULT_LIMIT_TX_POOL_BY_ACCOUNT_PERCENTAGE;
-
-    @CommandLine.Option(
-        names = {"--tx-pool-save-file"},
-        paramLabel = "<STRING>",
-        description =
-            "If saving the txpool content is enabled, define a custom path for the save file (default: ${DEFAULT-VALUE} in the data-dir)",
-        arity = "1")
-    private File saveFile = TransactionPoolConfiguration.DEFAULT_SAVE_FILE;
-
-    @Option(
-        names = {"--tx-pool-max-size"},
-        paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
-        description =
-            "Maximum number of pending transactions that will be kept in the transaction pool (default: ${DEFAULT-VALUE})",
-        arity = "1")
-    private final Integer txPoolMaxSize =
-        TransactionPoolConfiguration.DEFAULT_MAX_PENDING_TRANSACTIONS;
-
-    @Option(
-        names = {"--tx-pool-retention-hours"},
-        paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
-        description =
-            "Maximum retention period of pending transactions in hours (default: ${DEFAULT-VALUE})",
-        arity = "1")
-    private final Integer pendingTxRetentionPeriod =
-        TransactionPoolConfiguration.DEFAULT_TX_RETENTION_HOURS;
-
-    @Option(
-        names = {"--tx-pool-price-bump"},
-        paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
-        converter = PercentageConverter.class,
-        description =
-            "Price bump percentage to replace an already existing transaction  (default: ${DEFAULT-VALUE})",
-        arity = "1")
-    private final Integer priceBump = TransactionPoolConfiguration.DEFAULT_PRICE_BUMP.getValue();
-  }
-
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
   @Option(
       names = {"--key-value-storage"},
@@ -1906,10 +1834,15 @@ private void validateOptions() {
     validateRpcOptionsParams();
     validateChainDataPruningParams();
     validatePostMergeCheckpointBlockRequirements();
+    validateTransactionPoolOptions();
     p2pTLSConfigOptions.checkP2PTLSOptionsDependencies(logger, commandLine);
     pkiBlockCreationOptions.checkPkiBlockCreationOptionsDependencies(logger, commandLine);
   }
 
+  private void validateTransactionPoolOptions() {
+    stableTransactionPoolOptions.validate(commandLine);
+  }
+
   private void validateRequiredOptions() {
     commandLine
         .getCommandSpec()
@@ -2987,28 +2920,14 @@ private SynchronizerConfiguration buildSyncConfig() {
   }
 
   private TransactionPoolConfiguration buildTransactionPoolConfiguration() {
-    return unstableTransactionPoolOptions
-        .toDomainObject()
-        .enableSaveRestore(txPoolOptionGroup.saveRestoreEnabled)
-        .disableLocalTransactions(txPoolOptionGroup.disableLocalTxs)
-        .txPoolLimitByAccountPercentage(txPoolOptionGroup.txPoolLimitByAccountPercentage)
-        .txPoolMaxSize(txPoolOptionGroup.txPoolMaxSize)
-        .pendingTxRetentionPeriod(txPoolOptionGroup.pendingTxRetentionPeriod)
-        .priceBump(Percentage.fromInt(txPoolOptionGroup.priceBump))
-        .txFeeCap(txFeeCap)
-        .saveFile(dataPath.resolve(txPoolOptionGroup.saveFile.getPath()).toFile())
+    final var stableTxPoolOption = stableTransactionPoolOptions.toDomainObject();
+    return ImmutableTransactionPoolConfiguration.builder()
+        .from(stableTxPoolOption)
+        .unstable(unstableTransactionPoolOptions.toDomainObject())
+        .saveFile((dataPath.resolve(stableTxPoolOption.getSaveFile().getPath()).toFile()))
         .build();
   }
 
-  /**
-   * Return the file where to save txpool content if the relative option is enabled.
-   *
-   * @return the save file
-   */
-  public File getSaveFile() {
-    return txPoolOptionGroup.saveFile;
-  }
-
   private boolean isPruningEnabled() {
     return pruningEnabled;
   }
@@ -3613,9 +3532,7 @@ private String generateConfigurationOverview() {
       builder.setHighSpecEnabled();
     }
 
-    if (buildTransactionPoolConfiguration().getLayeredTxPoolEnabled()) {
-      builder.setLayeredTxPoolEnabled();
-    }
+    builder.setTxPoolImplementation(buildTransactionPoolConfiguration().getTxPoolImplementation());
 
     return builder.build();
   }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilder.java b/besu/src/main/java/org/hyperledger/besu/cli/ConfigurationOverviewBuilder.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.cli;
 
 import org.hyperledger.besu.BesuInfo;
+import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.util.log.FramedLogMessage;
 import org.hyperledger.besu.util.platform.PlatformDetector;
 
@@ -47,7 +48,7 @@ public class ConfigurationOverviewBuilder {
   private Collection<String> engineApis;
   private String engineJwtFilePath;
   private boolean isHighSpec = false;
-  private boolean isLayeredTxPool = false;
+  private TransactionPoolConfiguration.Implementation txPoolImplementation;
   private Map<String, String> environment;
 
   /**
@@ -167,12 +168,14 @@ public ConfigurationOverviewBuilder setHighSpecEnabled() {
   }
 
   /**
-   * Sets experimental layered txpool enabled.
+   * Sets the txpool implementation in use.
    *
+   * @param implementation the txpool implementation
    * @return the builder
    */
-  public ConfigurationOverviewBuilder setLayeredTxPoolEnabled() {
-    isLayeredTxPool = true;
+  public ConfigurationOverviewBuilder setTxPoolImplementation(
+      final TransactionPoolConfiguration.Implementation implementation) {
+    txPoolImplementation = implementation;
     return this;
   }
 
@@ -251,9 +254,7 @@ public String build() {
       lines.add("Experimental high spec configuration enabled");
     }
 
-    if (isLayeredTxPool) {
-      lines.add("Experimental layered transaction pool configuration enabled");
-    }
+    lines.add("Using " + txPoolImplementation + " transaction pool implementation");
 
     lines.add("");
     lines.add("Host:");

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java b/besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java
@@ -16,7 +16,6 @@
 
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.authentication.JwtAlgorithm;
-import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.p2p.config.RlpxConfiguration;
 import org.hyperledger.besu.nat.NatMethod;
 
@@ -61,9 +60,6 @@ public interface DefaultCommandValues {
   String MANDATORY_NODE_ID_FORMAT_HELP = "<NODEID>";
   /** The constant DEFAULT_MIN_TRANSACTION_GAS_PRICE. */
   Wei DEFAULT_MIN_TRANSACTION_GAS_PRICE = Wei.of(1000);
-  /** The constant DEFAULT_RPC_TX_FEE_CAP. */
-  Wei DEFAULT_RPC_TX_FEE_CAP = TransactionPoolConfiguration.DEFAULT_RPC_TX_FEE_CAP;
-
   /** The constant DEFAULT_MIN_BLOCK_OCCUPANCY_RATIO. */
   Double DEFAULT_MIN_BLOCK_OCCUPANCY_RATIO = 0.8;
   /** The constant DEFAULT_EXTRA_DATA. */

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/DurationMillisConverter.java
@@ -0,0 +1,44 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli.converter;
+
+import org.hyperledger.besu.cli.converter.exception.DurationConversionException;
+
+import java.time.Duration;
+
+import picocli.CommandLine;
+
+/** The Duration (milliseconds) Cli type converter. */
+public class DurationMillisConverter
+    implements CommandLine.ITypeConverter<Duration>, TypeFormatter<Duration> {
+
+  @Override
+  public Duration convert(final String value) throws DurationConversionException {
+    try {
+      final long millis = Long.parseLong(value);
+      if (millis < 0) {
+        throw new DurationConversionException(millis);
+      }
+      return Duration.ofMillis(Long.parseLong(value));
+    } catch (NullPointerException | IllegalArgumentException e) {
+      throw new DurationConversionException(value);
+    }
+  }
+
+  @Override
+  public String format(final Duration value) {
+    return Long.toString(value.toMillis());
+  }
+}
diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/FractionConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/FractionConverter.java
@@ -20,12 +20,12 @@
 import picocli.CommandLine;
 
 /** The Fraction converter to convert floats in CLI. */
-public class FractionConverter implements CommandLine.ITypeConverter<Float> {
+public class FractionConverter implements CommandLine.ITypeConverter<Fraction> {
 
   @Override
-  public Float convert(final String value) throws FractionConversionException {
+  public Fraction convert(final String value) throws FractionConversionException {
     try {
-      return Fraction.fromString(value).getValue();
+      return Fraction.fromString(value);
     } catch (final NullPointerException | IllegalArgumentException e) {
       throw new FractionConversionException(value);
     }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/converter/PercentageConverter.java b/besu/src/main/java/org/hyperledger/besu/cli/converter/PercentageConverter.java
@@ -20,12 +20,12 @@
 import picocli.CommandLine;
 
 /** The Percentage Cli type converter. */
-public class PercentageConverter implements CommandLine.ITypeConverter<Integer> {
+public class PercentageConverter implements CommandLine.ITypeConverter<Percentage> {
 
   @Override
-  public Integer convert(final String value) throws PercentageConversionException {
+  public Percentage convert(final String value) throws PercentageConversionException {
     try {
-      return Percentage.fromString(value).getValue();
+      return Percentage.fromString(value);
     } catch (NullPointerException | IllegalArgumentException e) {
       throw new PercentageConversionException(value);
     }