New option tx-pool-min-gas-price to set a lower bound when accepting …

…txs to the pool (hyperledger#6098) Signed-off-by: Fabio Di Fabio <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Signed-off-by: Justin Florentine <[email protected]>
jflo · Nov 20, 2023 · 2aae592 · 2aae592
1 parent 70fa9f7
commit 2aae592
Show file tree

Hide file tree

Showing 31 changed files with 214 additions and 195 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Additions and Improvements
 - Implement debug_traceCall [#5885](https://github.com/hyperledger/besu/pull/5885)
 - 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)
 
 ## 23.10.2
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -2830,6 +2830,23 @@ private TransactionPoolConfiguration buildTransactionPoolConfiguration() {
       txPoolConfBuilder.priceBump(Percentage.ZERO);
     }
 
+    if (getMiningParameters().getMinTransactionGasPrice().lessThan(txPoolConf.getMinGasPrice())) {
+      if (transactionPoolOptions.isMinGasPriceSet(commandLine)) {
+        throw new ParameterException(
+            commandLine, "tx-pool-min-gas-price cannot be greater than the value of min-gas-price");
+
+      } else {
+        // for backward compatibility, if tx-pool-min-gas-price is not set, we adjust its value
+        // to be the same as min-gas-price, so the behavior is as before this change, and we notify
+        // the user of the change
+        logger.warn(
+            "Forcing tx-pool-min-gas-price="
+                + getMiningParameters().getMinTransactionGasPrice().toDecimalString()
+                + ", since it cannot be greater than the value of min-gas-price");
+        txPoolConfBuilder.minGasPrice(getMiningParameters().getMinTransactionGasPrice());
+      }
+    }
+
     return txPoolConfBuilder.build();
   }
 

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/TransactionPoolOptions.java
@@ -54,6 +54,7 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
   private static final String STRICT_TX_REPLAY_PROTECTION_ENABLED_FLAG =
       "--strict-tx-replay-protection-enabled";
   private static final String TX_POOL_PRIORITY_SENDERS = "--tx-pool-priority-senders";
+  private static final String TX_POOL_MIN_GAS_PRICE = "--tx-pool-min-gas-price";
 
   @CommandLine.Option(
       names = {TX_POOL_IMPLEMENTATION},
@@ -122,6 +123,15 @@ public class TransactionPoolOptions implements CLIOptions<TransactionPoolConfigu
       arity = "1..*")
   private Set<Address> prioritySenders = TransactionPoolConfiguration.DEFAULT_PRIORITY_SENDERS;
 
+  @CommandLine.Option(
+      names = {TX_POOL_MIN_GAS_PRICE},
+      paramLabel = "<Wei>",
+      description =
+          "Transactions with gas price (in Wei) lower than this minimum will not be accepted into the txpool"
+              + "(not to be confused with min-gas-price, that is applied on block creation) (default: ${DEFAULT-VALUE})",
+      arity = "1")
+  private Wei minGasPrice = TransactionPoolConfiguration.DEFAULT_TX_POOL_MIN_GAS_PRICE;
+
   @CommandLine.ArgGroup(
       validate = false,
       heading = "@|bold Tx Pool Layered Implementation Options|@%n")
@@ -257,6 +267,7 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
     options.saveFile = config.getSaveFile();
     options.strictTxReplayProtectionEnabled = config.getStrictTransactionReplayProtectionEnabled();
     options.prioritySenders = config.getPrioritySenders();
+    options.minGasPrice = config.getMinGasPrice();
     options.layeredOptions.txPoolLayerMaxCapacity =
         config.getPendingTransactionsLayerMaxCapacityBytes();
     options.layeredOptions.txPoolMaxPrioritized = config.getMaxPrioritizedTransactions();
@@ -312,6 +323,7 @@ public TransactionPoolConfiguration toDomainObject() {
         .saveFile(saveFile)
         .strictTransactionReplayProtectionEnabled(strictTxReplayProtectionEnabled)
         .prioritySenders(prioritySenders)
+        .minGasPrice(minGasPrice)
         .pendingTransactionsLayerMaxCapacityBytes(layeredOptions.txPoolLayerMaxCapacity)
         .maxPrioritizedTransactions(layeredOptions.txPoolMaxPrioritized)
         .maxFutureBySender(layeredOptions.txPoolMaxFutureBySender)
@@ -340,4 +352,14 @@ public List<String> getCLIOptions() {
   public boolean isPriceBumpSet(final CommandLine commandLine) {
     return CommandLineUtils.isOptionSet(commandLine, TransactionPoolOptions.TX_POOL_PRICE_BUMP);
   }
+
+  /**
+   * Is min gas price option set?
+   *
+   * @param commandLine the command line
+   * @return true if tx-pool-min-gas-price is set
+   */
+  public boolean isMinGasPriceSet(final CommandLine commandLine) {
+    return CommandLineUtils.isOptionSet(commandLine, TransactionPoolOptions.TX_POOL_MIN_GAS_PRICE);
+  }
 }
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java
@@ -722,7 +722,6 @@ public BesuController build() {
             clock,
             metricsSystem,
             syncState,
-            miningParameters,
             transactionPoolConfiguration,
             pluginTransactionValidatorFactory,
             besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()));

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -5248,6 +5248,31 @@ public void txpoolPriceBumpKeepItsValueIfSetEvenWhenMinGasPriceZero() {
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
+  @Test
+  public void txpoolWhenNotSetForceTxPoolMinGasPriceToZeroWhenMinGasPriceZero() {
+    parseCommand("--min-gas-price", "0");
+    verify(mockControllerBuilder)
+        .transactionPoolConfiguration(transactionPoolConfigCaptor.capture());
+
+    final Wei txPoolMinGasPrice = transactionPoolConfigCaptor.getValue().getMinGasPrice();
+    assertThat(txPoolMinGasPrice).isEqualTo(Wei.ZERO);
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+    verify(mockLogger, atLeast(1))
+        .warn(
+            contains(
+                "Forcing tx-pool-min-gas-price=0, since it cannot be greater than the value of min-gas-price"));
+  }
+
+  @Test
+  public void txpoolTxPoolMinGasPriceMustNotBeGreaterThanMinGasPriceZero() {
+    parseCommand("--min-gas-price", "100", "--tx-pool-min-gas-price", "101");
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains("tx-pool-min-gas-price cannot be greater than the value of min-gas-price");
+  }
+
   @Test
   public void snapsyncHealingOptionShouldBeDisabledByDefault() {
     final TestBesuCommand besuCommand = parseCommand();

diff --git a/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java b/besu/src/test/java/org/hyperledger/besu/services/BesuEventsImplTest.java
@@ -33,8 +33,6 @@
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.Difficulty;
-import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters;
-import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.MutableInitValues;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.core.TransactionReceipt;
 import org.hyperledger.besu.ethereum.core.TransactionTestFixture;
@@ -147,7 +145,10 @@ public void setUp() {
     blockBroadcaster = new BlockBroadcaster(mockEthContext);
     syncState = new SyncState(blockchain, mockEthPeers);
     TransactionPoolConfiguration txPoolConfig =
-        ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build();
+        ImmutableTransactionPoolConfiguration.builder()
+            .txPoolMaxSize(1)
+            .minGasPrice(Wei.ZERO)
+            .build();
 
     transactionPool =
         TransactionPoolFactory.createTransactionPool(
@@ -157,10 +158,6 @@ public void setUp() {
             TestClock.system(ZoneId.systemDefault()),
             new NoOpMetricsSystem(),
             syncState,
-            ImmutableMiningParameters.builder()
-                .mutableInitValues(
-                    MutableInitValues.builder().minTransactionGasPrice(Wei.ZERO).build())
-                .build(),
             txPoolConfig,
             null,
             new BlobCache());

diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml
@@ -188,6 +188,7 @@ tx-pool-max-future-by-sender=321
 tx-pool-retention-hours=999
 tx-pool-max-size=1234
 tx-pool-limit-by-account-percentage=0.017
+tx-pool-min-gas-price=1000
 
 # Revert Reason
 revert-reason-enabled=false

diff --git a/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/...test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java
@@ -238,7 +238,6 @@ private TransactionPool createTransactionPool() {
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            mock(MiningParameters.class),
             new TransactionPoolMetrics(metricsSystem),
             conf,
             null);

diff --git a/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/...est/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java
@@ -222,7 +222,6 @@ private TransactionPool createTransactionPool() {
             cliqueProtocolContext,
             mock(TransactionBroadcaster.class),
             cliqueEthContext,
-            mock(MiningParameters.class),
             new TransactionPoolMetrics(metricsSystem),
             conf,
             null);

diff --git a/...integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java b/...integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java
@@ -362,7 +362,6 @@ private static ControllerAndState createControllerAndFinalState(
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParams,
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/.../src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java b/.../src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java
@@ -146,7 +146,6 @@ public BlockHeaderValidator.Builder createBlockHeaderRuleset(
             protContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            mock(MiningParameters.class),
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/...rc/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/...rc/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java
@@ -227,7 +227,6 @@ public void setUp() {
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParameters,
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java b/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java
@@ -450,7 +450,6 @@ private static ControllerAndState createControllerAndFinalState(
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParams,
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/...r/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java b/...r/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java
@@ -44,7 +44,6 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.ExecutionContextTestFixture;
-import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.core.TransactionReceipt;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
@@ -119,7 +118,6 @@ public void setUp() {
             protocolContext,
             batchAddedListener,
             ethContext,
-            MiningParameters.newDefault(),
             new TransactionPoolMetrics(metricsSystem),
             TransactionPoolConfiguration.DEFAULT,
             null);

diff --git a/...ger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java b/...ger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java
@@ -44,7 +44,6 @@
 import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
 import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.ExecutionContextTestFixture;
-import org.hyperledger.besu.ethereum.core.MiningParameters;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.core.TransactionReceipt;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
@@ -119,7 +118,6 @@ public void setUp() {
             protocolContext,
             batchAddedListener,
             ethContext,
-            MiningParameters.newDefault(),
             new TransactionPoolMetrics(metricsSystem),
             TransactionPoolConfiguration.DEFAULT,
             null);

diff --git a/...n/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java b/...n/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java
@@ -383,7 +383,6 @@ private AbstractBlockCreator createBlockCreator(
             executionContextTestFixture.getProtocolContext(),
             mock(TransactionBroadcaster.class),
             ethContext,
-            mock(MiningParameters.class),
             new TransactionPoolMetrics(new NoOpMetricsSystem()),
             poolConf,
             null);

diff --git a/...ava/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java b/...ava/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java
@@ -181,7 +181,7 @@ public void setup() {
 
   protected abstract ProtocolSchedule createProtocolSchedule();
 
-  protected abstract TransactionPool createTransactionPool(final MiningParameters miningParameters);
+  protected abstract TransactionPool createTransactionPool();
 
   private Boolean isCancelled() {
     return false;
@@ -1026,7 +1026,7 @@ protected BlockTransactionSelector createBlockSelectorAndSetupTxPool(
       final Wei blobGasPrice,
       final PluginTransactionSelectorFactory transactionSelectorFactory) {
 
-    transactionPool = createTransactionPool(miningParameters);
+    transactionPool = createTransactionPool();
 
     return createBlockSelector(
         miningParameters,

diff --git a/.../hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java b/.../hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java
@@ -20,7 +20,7 @@
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.config.GenesisConfigFile;
-import org.hyperledger.besu.ethereum.core.MiningParameters;
+import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.core.PrivacyParameters;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
@@ -62,11 +62,12 @@ protected ProtocolSchedule createProtocolSchedule() {
   }
 
   @Override
-  protected TransactionPool createTransactionPool(final MiningParameters miningParameters) {
+  protected TransactionPool createTransactionPool() {
     final TransactionPoolConfiguration poolConf =
         ImmutableTransactionPoolConfiguration.builder()
             .txPoolMaxSize(5)
             .txPoolLimitByAccountPercentage(Fraction.fromFloat(1f))
+            .minGasPrice(Wei.ONE)
             .build();
 
     final PendingTransactions pendingTransactions =
@@ -86,7 +87,6 @@ protected TransactionPool createTransactionPool(final MiningParameters miningPar
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParameters,
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/.../hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java b/.../hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java
@@ -74,11 +74,12 @@ protected ProtocolSchedule createProtocolSchedule() {
   }
 
   @Override
-  protected TransactionPool createTransactionPool(final MiningParameters miningParameters) {
+  protected TransactionPool createTransactionPool() {
     final TransactionPoolConfiguration poolConf =
         ImmutableTransactionPoolConfiguration.builder()
             .txPoolMaxSize(5)
             .txPoolLimitByAccountPercentage(Fraction.fromFloat(1f))
+            .minGasPrice(Wei.ONE)
             .build();
     final PendingTransactions pendingTransactions =
         new BaseFeePendingTransactionsSorter(
@@ -94,7 +95,6 @@ protected TransactionPool createTransactionPool(final MiningParameters miningPar
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParameters,
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/...eation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java b/...eation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java
@@ -332,7 +332,6 @@ private TransactionPool createTransactionPool(
             executionContextTestFixture.getProtocolContext(),
             mock(TransactionBroadcaster.class),
             ethContext,
-            mock(MiningParameters.class),
             new TransactionPoolMetrics(metricsSystem),
             poolConf,
             null);

diff --git a/...ation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutorTest.java b/...ation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutorTest.java
@@ -52,7 +52,7 @@ public class PoWMinerExecutorTest {
   public void startingMiningWithoutCoinbaseThrowsException() {
     final MiningParameters miningParameters = MiningParameters.newDefault();
 
-    final TransactionPool transactionPool = createTransactionPool(miningParameters);
+    final TransactionPool transactionPool = createTransactionPool();
 
     final PoWMinerExecutor executor =
         new PoWMinerExecutor(
@@ -73,7 +73,7 @@ public void startingMiningWithoutCoinbaseThrowsException() {
   public void settingCoinbaseToNullThrowsException() {
     final MiningParameters miningParameters = MiningParameters.newDefault();
 
-    final TransactionPool transactionPool = createTransactionPool(miningParameters);
+    final TransactionPool transactionPool = createTransactionPool();
 
     final PoWMinerExecutor executor =
         new PoWMinerExecutor(
@@ -96,7 +96,7 @@ private static BlockHeader mockBlockHeader() {
     return blockHeader;
   }
 
-  private TransactionPool createTransactionPool(final MiningParameters miningParameters) {
+  private TransactionPool createTransactionPool() {
     final TransactionPoolConfiguration poolConf =
         ImmutableTransactionPoolConfiguration.builder().txPoolMaxSize(1).build();
     final GasPricePendingTransactionsSorter pendingTransactions =
@@ -116,7 +116,6 @@ private TransactionPool createTransactionPool(final MiningParameters miningParam
             mock(ProtocolContext.class),
             mock(TransactionBroadcaster.class),
             ethContext,
-            miningParameters,
             new TransactionPoolMetrics(new NoOpMetricsSystem()),
             poolConf,
             null);

diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java
@@ -170,7 +170,6 @@ public void createStorage() {
             protocolContext,
             mock(TransactionBroadcaster.class),
             ethContext,
-            mock(MiningParameters.class),
             txPoolMetrics,
             poolConfiguration,
             null);