From eaf56820f5aff227dab4621a5afd7c0bdecfc159 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 20 Nov 2023 12:49:17 +0100 Subject: [PATCH] New option tx-pool-min-gas-price to set a lower bound when accepting txs to the pool (#6098) Signed-off-by: Fabio Di Fabio Co-authored-by: Sally MacFarlane --- CHANGELOG.md | 1 + .../org/hyperledger/besu/cli/BesuCommand.java | 17 ++ .../cli/options/TransactionPoolOptions.java | 22 ++ .../controller/BesuControllerBuilder.java | 1 - .../hyperledger/besu/cli/BesuCommandTest.java | 25 ++ .../besu/services/BesuEventsImplTest.java | 11 +- .../src/test/resources/everything_config.toml | 1 + .../blockcreation/CliqueBlockCreatorTest.java | 1 - .../CliqueMinerExecutorTest.java | 1 - .../ibft/support/TestContextBuilder.java | 1 - .../blockcreation/BftBlockCreatorTest.java | 1 - .../blockcreation/MergeCoordinatorTest.java | 1 - .../qbft/support/TestContextBuilder.java | 1 - .../EthGetFilterChangesIntegrationTest.java | 2 - .../EthGetFilterChangesIntegrationTest.java | 2 - .../AbstractBlockCreatorTest.java | 1 - .../AbstractBlockTransactionSelectorTest.java | 4 +- ...FeeMarketBlockTransactionSelectorTest.java | 6 +- ...FeeMarketBlockTransactionSelectorTest.java | 4 +- .../blockcreation/PoWBlockCreatorTest.java | 1 - .../blockcreation/PoWMinerExecutorTest.java | 7 +- .../bonsai/AbstractIsolationTests.java | 1 - .../eth/transactions/TransactionPool.java | 16 +- .../TransactionPoolConfiguration.java | 6 + .../transactions/TransactionPoolFactory.java | 5 - .../eth/manager/EthProtocolManagerTest.java | 2 - .../ethtaskutils/AbstractMessageTaskTest.java | 2 - .../AbstractTransactionPoolTest.java | 260 +++++++++--------- .../ethereum/eth/transactions/TestNode.java | 2 - .../TransactionPoolFactoryTest.java | 3 - .../ethereum/retesteth/RetestethContext.java | 1 - 31 files changed, 214 insertions(+), 195 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48652cadafe..8a6b80cdeda 100644 --- 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 index 40d6eb989dd..9cfc44c57b5 100644 --- 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 index a8a50025f58..3b34c4d2a61 100644 --- 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 prioritySenders = TransactionPoolConfiguration.DEFAULT_PRIORITY_SENDERS; + @CommandLine.Option( + names = {TX_POOL_MIN_GAS_PRICE}, + paramLabel = "", + 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 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 index 2c975bccb92..643aed546cb 100644 --- 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 index cfd90bc42e1..204245f92c7 100644 --- 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 index 6eaa113893e..6245a8a6ba8 100644 --- 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 index 2bbf25a97d4..085a8ad444f 100644 --- 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/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java index 631cad835f4..645a1ca9482 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java +++ b/consensus/clique/src/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/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 ab2545f8b3a..090d2b52c5a 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 @@ -222,7 +222,6 @@ private TransactionPool createTransactionPool() { cliqueProtocolContext, mock(TransactionBroadcaster.class), cliqueEthContext, - mock(MiningParameters.class), new TransactionPoolMetrics(metricsSystem), conf, null); diff --git a/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java b/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java index 6223ebdf026..ce06496d547 100644 --- a/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java +++ b/consensus/ibft/src/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/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java index 9ef7c4c3a91..3e884139f50 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java +++ b/consensus/ibft/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/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index cad6e05d60c..16a147c70e5 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/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/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java index b2ee8421146..b5b0a26bd62 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java +++ b/consensus/qbft/src/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/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java index 116c2d75181..cff37a0392b 100644 --- a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/api/src/integration-test/java/org/hyperledger/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/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java index 70025eeef07..0026c56ad84 100644 --- a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/api/src/integration-test/java/org/hyperledger/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java index a317c12f669..953fdeb7f42 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java +++ b/ethereum/blockcreation/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java index 9858893f017..35bfd3754b6 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java index ba719700852..fdda1b7f4d2 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LegacyFeeMarketBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java index 3ff84afa56a..63e0d54ecd7 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java index 2aef4a15a7f..3e50328dd43 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java +++ b/ethereum/blockcreation/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/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutorTest.java index d6b54606e5f..84c46ea6afb 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutorTest.java +++ b/ethereum/blockcreation/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 index 8cd26eb17b8..4f40afdb437 100644 --- 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); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java index 779523ae27b..bb0cdf70eb6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java @@ -28,7 +28,6 @@ import org.hyperledger.besu.ethereum.chain.BlockAddedObserver; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeer; @@ -100,7 +99,6 @@ public class TransactionPool implements BlockAddedObserver { private final ProtocolContext protocolContext; private final EthContext ethContext; private final TransactionBroadcaster transactionBroadcaster; - private final MiningParameters miningParameters; private final TransactionPoolMetrics metrics; private final TransactionPoolConfiguration configuration; private final AtomicBoolean isPoolEnabled = new AtomicBoolean(false); @@ -118,7 +116,6 @@ public TransactionPool( final ProtocolContext protocolContext, final TransactionBroadcaster transactionBroadcaster, final EthContext ethContext, - final MiningParameters miningParameters, final TransactionPoolMetrics metrics, final TransactionPoolConfiguration configuration, final PluginTransactionValidatorFactory pluginTransactionValidatorFactory) { @@ -127,7 +124,6 @@ public TransactionPool( this.protocolContext = protocolContext; this.ethContext = ethContext; this.transactionBroadcaster = transactionBroadcaster; - this.miningParameters = miningParameters; this.metrics = metrics; this.configuration = configuration; this.pluginTransactionValidator = @@ -289,7 +285,7 @@ private Optional getMaxGasPrice(final Transaction transaction) { private boolean isMaxGasPriceBelowConfiguredMinGasPrice(final Transaction transaction) { return getMaxGasPrice(transaction) - .map(g -> g.lessThan(miningParameters.getMinTransactionGasPrice())) + .map(g -> g.lessThan(configuration.getMinGasPrice())) .orElse(true); } @@ -510,11 +506,9 @@ && getMaxGasPrice(transaction).get().greaterThan(configuration.getTxFeeCap())) { } } if (hasPriority) { - // allow priority transactions to be below minGas as long as we are mining - // or at least gas price is above the configured floor - if ((!miningParameters.isMiningEnabled() - && isMaxGasPriceBelowConfiguredMinGasPrice(transaction)) - || !feeMarket.satisfiesFloorTxFee(transaction)) { + // allow priority transactions to be below minGas as long as the gas price is above the + // configured floor + if (!feeMarket.satisfiesFloorTxFee(transaction)) { return TransactionInvalidReason.GAS_PRICE_TOO_LOW; } } else { @@ -522,7 +516,7 @@ && isMaxGasPriceBelowConfiguredMinGasPrice(transaction)) LOG.atTrace() .setMessage("Discard transaction {} below min gas price {}") .addArgument(transaction::toTraceLog) - .addArgument(miningParameters::getMinTransactionGasPrice) + .addArgument(configuration::getMinGasPrice) .log(); return TransactionInvalidReason.GAS_PRICE_TOO_LOW; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java index 874446b7671..e30d24c158f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java @@ -69,6 +69,7 @@ enum Implementation { int DEFAULT_MAX_FUTURE_BY_SENDER = 200; Implementation DEFAULT_TX_POOL_IMPLEMENTATION = Implementation.LAYERED; Set
DEFAULT_PRIORITY_SENDERS = Set.of(); + Wei DEFAULT_TX_POOL_MIN_GAS_PRICE = Wei.of(1000); TransactionPoolConfiguration DEFAULT = ImmutableTransactionPoolConfiguration.builder().build(); @@ -147,6 +148,11 @@ default Set
getPrioritySenders() { return DEFAULT_PRIORITY_SENDERS; } + @Value.Default + default Wei getMinGasPrice() { + return DEFAULT_TX_POOL_MIN_GAS_PRICE; + } + @Value.Default default Unstable getUnstable() { return Unstable.DEFAULT; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java index 0f620653f1e..97c2a1538fa 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java @@ -17,7 +17,6 @@ import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED; import org.hyperledger.besu.ethereum.ProtocolContext; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.messages.EthPV62; import org.hyperledger.besu.ethereum.eth.messages.EthPV65; @@ -54,7 +53,6 @@ public static TransactionPool createTransactionPool( final Clock clock, final MetricsSystem metricsSystem, final SyncState syncState, - final MiningParameters miningParameters, final TransactionPoolConfiguration transactionPoolConfiguration, final PluginTransactionValidatorFactory pluginTransactionValidatorFactory, final BlobCache blobCache) { @@ -75,7 +73,6 @@ public static TransactionPool createTransactionPool( clock, metrics, syncState, - miningParameters, transactionPoolConfiguration, transactionTracker, transactionsMessageSender, @@ -91,7 +88,6 @@ static TransactionPool createTransactionPool( final Clock clock, final TransactionPoolMetrics metrics, final SyncState syncState, - final MiningParameters miningParameters, final TransactionPoolConfiguration transactionPoolConfiguration, final PeerTransactionTracker transactionTracker, final TransactionsMessageSender transactionsMessageSender, @@ -117,7 +113,6 @@ static TransactionPool createTransactionPool( transactionsMessageSender, newPooledTransactionHashesMessageSender), ethContext, - miningParameters, metrics, transactionPoolConfiguration, pluginTransactionValidatorFactory); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java index 66d34e4561b..3b7178c365b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java @@ -36,7 +36,6 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; import org.hyperledger.besu.ethereum.core.Difficulty; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.ProtocolScheduleFixture; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; @@ -1117,7 +1116,6 @@ public void transactionMessagesGoToTheCorrectExecutor() { TestClock.system(ZoneId.systemDefault()), metricsSystem, new SyncState(blockchain, ethManager.ethContext().getEthPeers()), - MiningParameters.newDefault(), TransactionPoolConfiguration.DEFAULT, null, new BlobCache()) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java index 5ff8d22dc31..c5ca2f3d8a7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java @@ -25,7 +25,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.eth.EthProtocol; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthContext; @@ -136,7 +135,6 @@ public void setupTest() { TestClock.system(ZoneId.systemDefault()), metricsSystem, syncState, - MiningParameters.newDefault(), TransactionPoolConfiguration.DEFAULT, null, new BlobCache()); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java index a1597df1932..a87ca36bfb8 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java @@ -60,7 +60,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.PrivacyParameters; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; @@ -102,6 +101,7 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -110,6 +110,8 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Answers; import org.mockito.ArgumentCaptor; @@ -133,9 +135,6 @@ public abstract class AbstractTransactionPoolTest { @Mock protected PendingTransactionAddedListener listener; - @Mock(answer = Answers.RETURNS_DEEP_STUBS) - protected MiningParameters miningParameters; - @Mock protected TransactionsMessageSender transactionsMessageSender; @Mock protected NewPooledTransactionHashesMessageSender newPooledTransactionHashesMessageSender; @Mock protected ProtocolSpec protocolSpec; @@ -253,11 +252,10 @@ public void setUp() { transactionPool = createTransactionPool(); blockchain.observeBlockAdded(transactionPool); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.of(2)); } protected TransactionPool createTransactionPool() { - return createTransactionPool(b -> {}); + return createTransactionPool(b -> b.minGasPrice(Wei.of(2))); } protected TransactionPool createTransactionPool( @@ -290,7 +288,6 @@ private TransactionPool createTransactionPool( protocolContext, transactionBroadcaster, ethContext, - miningParameters, new TransactionPoolMetrics(metricsSystem), poolConfig, pluginTransactionValidatorFactory); @@ -625,7 +622,8 @@ public void shouldNotNotifyBatchListenerWhenRemoteTransactionDoesNotReplaceExist @ValueSource(booleans = {true, false}) public void shouldNotNotifyBatchListenerWhenLocalTransactionDoesNotReplaceExisting( final boolean noLocalPriority) { - transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); + transactionPool = + createTransactionPool(b -> b.minGasPrice(Wei.of(2)).noLocalPriority(noLocalPriority)); final Transaction transaction0a = createTransaction(0, Wei.of(10)); final Transaction transaction0b = createTransaction(0, Wei.of(9)); @@ -792,58 +790,6 @@ public void shouldAcceptRemoteTransactionEvenIfFeeCapExceeded(final boolean hasP addAndAssertRemoteTransactionsValid(hasPriority, remoteTransaction); } - @ParameterizedTest - @ValueSource(booleans = {true, false}) - public void shouldRejectZeroGasPriceLocalTransactionWhenNotMining(final boolean noLocalPriority) { - transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); - when(miningParameters.isMiningEnabled()).thenReturn(false); - - final Transaction transaction = createTransaction(0, Wei.ZERO); - - givenTransactionIsValid(transaction); - - addAndAssertTransactionViaApiInvalid(transaction, GAS_PRICE_TOO_LOW); - } - - @Test - @DisabledIf("isBaseFeeMarket") - public void shouldAcceptZeroGasPriceFrontierLocalPriorityTransactionsWhenMining() { - transactionPool = createTransactionPool(b -> b.noLocalPriority(false)); - when(miningParameters.isMiningEnabled()).thenReturn(true); - - final Transaction transaction = createTransaction(0, Wei.ZERO); - - givenTransactionIsValid(transaction); - - addAndAssertTransactionViaApiValid(transaction, false); - } - - @ParameterizedTest - @ValueSource(booleans = {true, false}) - public void shouldRejectZeroGasPriceRemoteTransactionWhenNotMining(final boolean hasPriority) { - final Transaction transaction = createTransaction(0, Wei.ZERO); - final Set
prioritySenders = hasPriority ? Set.of(transaction.getSender()) : Set.of(); - transactionPool = createTransactionPool(b -> b.prioritySenders(prioritySenders)); - when(miningParameters.isMiningEnabled()).thenReturn(false); - - givenTransactionIsValid(transaction); - - addAndAssertRemoteTransactionInvalid(transaction); - } - - @Test - @DisabledIf("isBaseFeeMarket") - public void shouldAcceptZeroGasPriceFrontierRemotePriorityTransactionsWhenMining() { - final Transaction transaction = createTransaction(0, Wei.ZERO); - transactionPool = - createTransactionPool(b -> b.prioritySenders(Set.of(transaction.getSender()))); - when(miningParameters.isMiningEnabled()).thenReturn(true); - - givenTransactionIsValid(transaction); - - addAndAssertRemoteTransactionsValid(true, transaction); - } - @ParameterizedTest @ValueSource(booleans = {true, false}) public void transactionNotRejectedByPluginShouldBeAdded(final boolean noLocalPriority) { @@ -998,54 +944,104 @@ public void shouldIgnoreEIP1559TransactionWhenNotAllowed() { @ParameterizedTest @ValueSource(booleans = {true, false}) @DisabledIf("isBaseFeeMarket") - public void shouldAcceptZeroGasPriceTransactionWhenMinGasPriceIsZero( - final boolean noLocalPriority) { - transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + public void shouldAcceptZeroGasPriceFrontierPriorityTransactions(final boolean isLocal) { + final Transaction transaction = createFrontierTransaction(0, Wei.ZERO); + transactionPool = + createTransactionPool(b -> b.prioritySenders(List.of(transaction.getSender()))); + + givenTransactionIsValid(transaction); + + if (isLocal) { + addAndAssertTransactionViaApiValid(transaction, false); + } else { + addAndAssertRemoteTransactionsValid(true, transaction); + } + } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldRejectZeroGasPriceNoPriorityTransaction(final boolean isLocal) { final Transaction transaction = createTransaction(0, Wei.ZERO); + transactionPool = createTransactionPool(b -> b.noLocalPriority(true)); givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, noLocalPriority); + if (isLocal) { + addAndAssertTransactionViaApiInvalid(transaction, GAS_PRICE_TOO_LOW); + } else { + addAndAssertRemoteTransactionInvalid(transaction); + } } @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldAcceptZeroGasPriceFrontierTxsWhenMinGasPriceIsZeroAndLondonWithZeroBaseFee( - final boolean noLocalPriority) { - transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); - when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(Wei.ZERO))); - whenBlockBaseFeeIs(Wei.ZERO); + @DisabledIf("isBaseFeeMarket") + public void shouldAcceptZeroGasPriceNoPriorityTransactionWhenMinGasPriceIsZero( + final boolean isLocal) { + transactionPool = createTransactionPool(b -> b.minGasPrice(Wei.ZERO).noLocalPriority(true)); - final Transaction frontierTransaction = createFrontierTransaction(0, Wei.ZERO); + final Transaction transaction = createTransaction(0, Wei.ZERO); - givenTransactionIsValid(frontierTransaction); - addAndAssertTransactionViaApiValid(frontierTransaction, noLocalPriority); + givenTransactionIsValid(transaction); + + if (isLocal) { + addAndAssertTransactionViaApiValid(transaction, true); + } else { + addAndAssertRemoteTransactionsValid(false, transaction); + } } @ParameterizedTest - @ValueSource(booleans = {true, false}) + @MethodSource("provideHasPriorityAndIsLocal") + public void shouldAcceptZeroGasPriceFrontierTxsWhenMinGasPriceIsZeroAndLondonWithZeroBaseFee( + final boolean hasPriority, final boolean isLocal) { + final Transaction frontierTransaction = createFrontierTransaction(0, Wei.ZERO); + internalAcceptZeroGasPriceTxsWhenMinGasPriceIsZeroAndZeroBaseFee( + hasPriority, isLocal, frontierTransaction); + } + + @ParameterizedTest + @MethodSource("provideHasPriorityAndIsLocal") public void shouldAcceptZeroGasPrice1559TxsWhenMinGasPriceIsZeroAndLondonWithZeroBaseFee( - final boolean noLocalPriority) { - transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + final boolean hasPriority, final boolean isLocal) { + final Transaction transaction = createTransactionBaseFeeMarket(0, Wei.ZERO); + internalAcceptZeroGasPriceTxsWhenMinGasPriceIsZeroAndZeroBaseFee( + hasPriority, isLocal, transaction); + } + + private void internalAcceptZeroGasPriceTxsWhenMinGasPriceIsZeroAndZeroBaseFee( + final boolean hasPriority, final boolean isLocal, final Transaction transaction) { + transactionPool = + createTransactionPool( + b -> { + b.minGasPrice(Wei.ZERO); + if (hasPriority) { + b.prioritySenders(List.of(transaction.getSender())); + } else { + b.noLocalPriority(true); + } + }); + when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(Wei.ZERO))); whenBlockBaseFeeIs(Wei.ZERO); - final Transaction transaction = createTransaction(0, Wei.ZERO); - givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, noLocalPriority); + if (isLocal) { + addAndAssertTransactionViaApiValid(transaction, !hasPriority); + } else { + addAndAssertRemoteTransactionsValid(hasPriority, transaction); + } } @ParameterizedTest @ValueSource(booleans = {true, false}) public void samePriceTxReplacementWhenPriceBumpIsZeroFrontier(final boolean noLocalPriority) { transactionPool = - createTransactionPool(b -> b.priceBump(Percentage.ZERO).noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + createTransactionPool( + b -> + b.priceBump(Percentage.ZERO) + .noLocalPriority(noLocalPriority) + .minGasPrice(Wei.ZERO)); final Transaction transaction1a = createBaseTransactionGasPriceMarket(0) @@ -1081,8 +1077,11 @@ public void samePriceTxReplacementWhenPriceBumpIsZeroFrontier(final boolean noLo @EnabledIf("isBaseFeeMarket") public void replaceSamePriceTxWhenPriceBumpIsZeroLondon(final boolean noLocalPriority) { transactionPool = - createTransactionPool(b -> b.priceBump(Percentage.ZERO).noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + createTransactionPool( + b -> + b.priceBump(Percentage.ZERO) + .noLocalPriority(noLocalPriority) + .minGasPrice(Wei.ZERO)); final Transaction transaction1a = createBaseTransactionBaseFeeMarket(0) @@ -1120,8 +1119,11 @@ public void replaceSamePriceTxWhenPriceBumpIsZeroLondon(final boolean noLocalPri @EnabledIf("isBaseFeeMarket") public void replaceSamePriceTxWhenPriceBumpIsZeroLondonToFrontier(final boolean noLocalPriority) { transactionPool = - createTransactionPool(b -> b.priceBump(Percentage.ZERO).noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + createTransactionPool( + b -> + b.priceBump(Percentage.ZERO) + .noLocalPriority(noLocalPriority) + .minGasPrice(Wei.ZERO)); final Transaction transaction1a = createBaseTransactionBaseFeeMarket(0) @@ -1158,8 +1160,11 @@ public void replaceSamePriceTxWhenPriceBumpIsZeroLondonToFrontier(final boolean @EnabledIf("isBaseFeeMarket") public void replaceSamePriceTxWhenPriceBumpIsZeroFrontierToLondon(final boolean noLocalPriority) { transactionPool = - createTransactionPool(b -> b.priceBump(Percentage.ZERO).noLocalPriority(noLocalPriority)); - when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); + createTransactionPool( + b -> + b.priceBump(Percentage.ZERO) + .noLocalPriority(noLocalPriority) + .minGasPrice(Wei.ZERO)); final Transaction transaction1a = createBaseTransactionGasPriceMarket(0) @@ -1191,79 +1196,56 @@ public void replaceSamePriceTxWhenPriceBumpIsZeroFrontierToLondon(final boolean .containsOnly(transaction1b); } - @Test - public void shouldAcceptBaseFeeFloorGasPriceFrontierLocalPriorityTransactionsWhenMining() { - transactionPool = createTransactionPool(b -> b.noLocalPriority(false)); - final Transaction frontierTransaction = createFrontierTransaction(0, BASE_FEE_FLOOR); - - givenTransactionIsValid(frontierTransaction); - - addAndAssertTransactionViaApiValid(frontierTransaction, false); + private static Stream provideHasPriorityAndIsLocal() { + return Stream.of( + Arguments.of(true, true), + Arguments.of(true, false), + Arguments.of(false, true), + Arguments.of(false, false)); } - @Test - public void shouldAcceptBaseFeeFloorGasPriceFrontierRemotePriorityTransactionsWhenMining() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldAcceptBaseFeeFloorGasPriceFrontierPriorityTransactions(final boolean isLocal) { final Transaction frontierTransaction = createFrontierTransaction(0, BASE_FEE_FLOOR); transactionPool = - createTransactionPool(b -> b.prioritySenders(Set.of(frontierTransaction.getSender()))); + createTransactionPool(b -> b.prioritySenders(List.of(frontierTransaction.getSender()))); givenTransactionIsValid(frontierTransaction); - addAndAssertRemoteTransactionsValid(frontierTransaction); + if (isLocal) { + addAndAssertTransactionViaApiValid(frontierTransaction, false); + } else { + addAndAssertRemoteTransactionsValid(true, frontierTransaction); + } } @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldRejectRemote1559TxsWhenMaxFeePerGasBelowMinGasPrice(final boolean hasPriority) { + public void shouldRejectNoPriorityTxsWhenMaxFeePerGasBelowMinGasPrice(final boolean isLocal) { final Wei genesisBaseFee = Wei.of(100L); final Wei minGasPrice = Wei.of(200L); final Wei lastBlockBaseFee = minGasPrice.add(50L); final Wei txMaxFeePerGas = minGasPrice.subtract(1L); assertThat( - add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false, hasPriority)) + addTxAndGetPendingTxsCount( + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, isLocal, false)) .isZero(); } @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldAcceptRemote1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinGasPrice( - final boolean hasPriority) { + public void shouldAcceptNoPriorityTxsWhenMaxFeePerGasIsAtLeastEqualToMinGasPrice( + final boolean isLocal) { final Wei genesisBaseFee = Wei.of(100L); final Wei minGasPrice = Wei.of(200L); final Wei lastBlockBaseFee = minGasPrice.add(50L); final Wei txMaxFeePerGas = minGasPrice; assertThat( - add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false, hasPriority)) - .isEqualTo(1); - } - - @Test - public void shouldRejectLocal1559TxsWhenMaxFeePerGasBelowMinGasPrice() { - final Wei genesisBaseFee = Wei.of(100L); - final Wei minGasPrice = Wei.of(200L); - final Wei lastBlockBaseFee = minGasPrice.add(50L); - final Wei txMaxFeePerGas = minGasPrice.subtract(1L); - - assertThat( - add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true, true)) - .isZero(); - } - - @Test - public void shouldAcceptLocal1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinMinGasPrice() { - final Wei genesisBaseFee = Wei.of(100L); - final Wei minGasPrice = Wei.of(200L); - final Wei lastBlockBaseFee = minGasPrice.add(50L); - final Wei txMaxFeePerGas = minGasPrice; - - assertThat( - add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true, true)) + addTxAndGetPendingTxsCount( + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, isLocal, false)) .isEqualTo(1); } @@ -1470,22 +1452,26 @@ protected Transaction createBlobTransaction(final int nonce) { .createTransaction(KEY_PAIR1); } - protected int add1559TxAndGetPendingTxsCount( + protected int addTxAndGetPendingTxsCount( final Wei genesisBaseFee, final Wei minGasPrice, final Wei lastBlockBaseFee, final Wei txMaxFeePerGas, final boolean isLocal, final boolean hasPriority) { - when(miningParameters.getMinTransactionGasPrice()).thenReturn(minGasPrice); when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(genesisBaseFee))); whenBlockBaseFeeIs(lastBlockBaseFee); final Transaction transaction = createTransaction(0, txMaxFeePerGas); if (hasPriority) { transactionPool = - createTransactionPool(b -> b.prioritySenders(Set.of(transaction.getSender()))); + createTransactionPool( + b -> b.minGasPrice(minGasPrice).prioritySenders(Set.of(transaction.getSender()))); + } else { + transactionPool = + createTransactionPool(b -> b.minGasPrice(minGasPrice).noLocalPriority(true)); } + givenTransactionIsValid(transaction); if (isLocal) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java index 02fd3613c0d..d593c38b5ea 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java @@ -31,7 +31,6 @@ import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions; import org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.difficulty.fixed.FixedDifficultyProtocolSchedule; import org.hyperledger.besu.ethereum.eth.EthProtocol; @@ -162,7 +161,6 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod TestClock.system(ZoneId.systemDefault()), metricsSystem, syncState, - MiningParameters.newDefault(), TransactionPoolConfiguration.DEFAULT, null, new BlobCache()); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java index b3d2de98be8..70d24e8c9e9 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java @@ -31,7 +31,6 @@ import org.hyperledger.besu.ethereum.chain.BlockAddedObserver; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; -import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; import org.hyperledger.besu.ethereum.eth.manager.EthContext; @@ -240,7 +239,6 @@ private void setupInitialSyncPhase(final boolean hasInitialSyncPhase) { TestClock.fixed(), new TransactionPoolMetrics(new NoOpMetricsSystem()), syncState, - MiningParameters.newDefault(), ImmutableTransactionPoolConfiguration.builder() .txPoolMaxSize(1) .pendingTxRetentionPeriod(1) @@ -350,7 +348,6 @@ private TransactionPool createTransactionPool( TestClock.fixed(), new NoOpMetricsSystem(), syncState, - MiningParameters.newDefault(), ImmutableTransactionPoolConfiguration.builder() .txPoolImplementation(implementation) .txPoolMaxSize(1) diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java index 3f88fd63ec6..2286fe67cfe 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java @@ -251,7 +251,6 @@ private boolean buildContext( retestethClock, metricsSystem, syncState, - miningParameters, transactionPoolConfiguration, null, new BlobCache());