Skip to content

Commit

Permalink
New option tx-pool-min-gas-price to set a lower bound when accepting …
Browse files Browse the repository at this point in the history
…txs to the pool (hyperledger#6098)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
  • Loading branch information
fab-10 and macfarla authored Nov 20, 2023
1 parent f01e4a2 commit eaf5682
Show file tree
Hide file tree
Showing 31 changed files with 214 additions and 195 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -312,6 +323,7 @@ public TransactionPoolConfiguration toDomainObject() {
.saveFile(saveFile)
.strictTransactionReplayProtectionEnabled(strictTxReplayProtectionEnabled)
.prioritySenders(prioritySenders)
.minGasPrice(minGasPrice)
.pendingTransactionsLayerMaxCapacityBytes(layeredOptions.txPoolLayerMaxCapacity)
.maxPrioritizedTransactions(layeredOptions.txPoolMaxPrioritized)
.maxFutureBySender(layeredOptions.txPoolMaxFutureBySender)
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,6 @@ public BesuController build() {
clock,
metricsSystem,
syncState,
miningParameters,
transactionPoolConfiguration,
pluginTransactionValidatorFactory,
besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()));
Expand Down
25 changes: 25 additions & 0 deletions besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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());
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ private TransactionPool createTransactionPool() {
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
mock(MiningParameters.class),
new TransactionPoolMetrics(metricsSystem),
conf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ private TransactionPool createTransactionPool() {
cliqueProtocolContext,
mock(TransactionBroadcaster.class),
cliqueEthContext,
mock(MiningParameters.class),
new TransactionPoolMetrics(metricsSystem),
conf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@ private static ControllerAndState createControllerAndFinalState(
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
miningParams,
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public BlockHeaderValidator.Builder createBlockHeaderRuleset(
protContext,
mock(TransactionBroadcaster.class),
ethContext,
mock(MiningParameters.class),
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ public void setUp() {
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
miningParameters,
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,6 @@ private static ControllerAndState createControllerAndFinalState(
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
miningParams,
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,7 +118,6 @@ public void setUp() {
protocolContext,
batchAddedListener,
ethContext,
MiningParameters.newDefault(),
new TransactionPoolMetrics(metricsSystem),
TransactionPoolConfiguration.DEFAULT,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,7 +118,6 @@ public void setUp() {
protocolContext,
batchAddedListener,
ethContext,
MiningParameters.newDefault(),
new TransactionPoolMetrics(metricsSystem),
TransactionPoolConfiguration.DEFAULT,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ private AbstractBlockCreator createBlockCreator(
executionContextTestFixture.getProtocolContext(),
mock(TransactionBroadcaster.class),
ethContext,
mock(MiningParameters.class),
new TransactionPoolMetrics(new NoOpMetricsSystem()),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1026,7 +1026,7 @@ protected BlockTransactionSelector createBlockSelectorAndSetupTxPool(
final Wei blobGasPrice,
final PluginTransactionSelectorFactory transactionSelectorFactory) {

transactionPool = createTransactionPool(miningParameters);
transactionPool = createTransactionPool();

return createBlockSelector(
miningParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -86,7 +87,6 @@ protected TransactionPool createTransactionPool(final MiningParameters miningPar
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
miningParameters,
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -94,7 +95,6 @@ protected TransactionPool createTransactionPool(final MiningParameters miningPar
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
miningParameters,
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ private TransactionPool createTransactionPool(
executionContextTestFixture.getProtocolContext(),
mock(TransactionBroadcaster.class),
ethContext,
mock(MiningParameters.class),
new TransactionPoolMetrics(metricsSystem),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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 =
Expand All @@ -116,7 +116,6 @@ private TransactionPool createTransactionPool(final MiningParameters miningParam
mock(ProtocolContext.class),
mock(TransactionBroadcaster.class),
ethContext,
miningParameters,
new TransactionPoolMetrics(new NoOpMetricsSystem()),
poolConf,
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ public void createStorage() {
protocolContext,
mock(TransactionBroadcaster.class),
ethContext,
mock(MiningParameters.class),
txPoolMetrics,
poolConfiguration,
null);
Expand Down
Loading

0 comments on commit eaf5682

Please sign in to comment.