Skip to content

Commit

Permalink
Force tx replacement price bump to zero when zero base fee market is …
Browse files Browse the repository at this point in the history
…configured (#6079)


Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
  • Loading branch information
fab-10 authored Nov 8, 2023
1 parent 0203092 commit 636ad8a
Show file tree
Hide file tree
Showing 19 changed files with 635 additions and 299 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
- Clique config option `createemptyblocks` to not create empty blocks [#6082](https://github.com/hyperledger/besu/pull/6082)
- Upgrade EVM Reference Tests to v13 (Cancun) [#6114](https://github.com/hyperledger/besu/pull/6114)
- Add `yParity` to GraphQL and JSON-RPC for relevant querise. [6119](https://github.com/hyperledger/besu/pull/6119)
- Force tx replacement price bump to zero when zero base fee market is configured or `--min-gas-price` is set to 0. This allows for easier tx replacement in networks where there is not gas price. [#6079](https://github.com/hyperledger/besu/pull/6079)

### Bug fixes

- Upgrade netty to address CVE-2023-44487, CVE-2023-34462 [#6100](https://github.com/hyperledger/besu/pull/6100)
- Upgrade grpc to address CVE-2023-32731, CVE-2023-33953, CVE-2023-44487, CVE-2023-4785 [#6100](https://github.com/hyperledger/besu/pull/6100)
- Fix blob gas calculation in reference tests [#6107](https://github.com/hyperledger/besu/pull/6107)
Expand Down
37 changes: 23 additions & 14 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@
import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler;
import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler;
import org.hyperledger.besu.cli.options.MiningOptions;
import org.hyperledger.besu.cli.options.TransactionPoolOptions;
import org.hyperledger.besu.cli.options.stable.DataStorageOptions;
import org.hyperledger.besu.cli.options.stable.EthstatsOptions;
import org.hyperledger.besu.cli.options.stable.LoggingLevelOption;
import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption;
import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions;
import org.hyperledger.besu.cli.options.stable.TransactionPoolOptions;
import org.hyperledger.besu.cli.options.unstable.ChainPruningOptions;
import org.hyperledger.besu.cli.options.unstable.DnsOptions;
import org.hyperledger.besu.cli.options.unstable.EthProtocolOptions;
Expand Down Expand Up @@ -283,9 +283,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
final SynchronizerOptions unstableSynchronizerOptions = SynchronizerOptions.create();
final EthProtocolOptions unstableEthProtocolOptions = EthProtocolOptions.create();
final MetricsCLIOptions unstableMetricsCLIOptions = MetricsCLIOptions.create();
final org.hyperledger.besu.cli.options.unstable.TransactionPoolOptions
unstableTransactionPoolOptions =
org.hyperledger.besu.cli.options.unstable.TransactionPoolOptions.create();
private final DnsOptions unstableDnsOptions = DnsOptions.create();
private final NatOptions unstableNatOptions = NatOptions.create();
private final NativeLibraryOptions unstableNativeLibraryOptions = NativeLibraryOptions.create();
Expand All @@ -303,8 +300,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
private final LoggingLevelOption loggingLevelOption = LoggingLevelOption.create();

@CommandLine.ArgGroup(validate = false, heading = "@|bold Tx Pool Common Options|@%n")
final org.hyperledger.besu.cli.options.stable.TransactionPoolOptions
stableTransactionPoolOptions = TransactionPoolOptions.create();
final TransactionPoolOptions transactionPoolOptions = TransactionPoolOptions.create();

@CommandLine.ArgGroup(validate = false, heading = "@|bold Block Builder Options|@%n")
final MiningOptions miningOptions = MiningOptions.create();
Expand Down Expand Up @@ -1525,7 +1521,6 @@ private void handleUnstableOptions() {
.put("NAT Configuration", unstableNatOptions)
.put("Privacy Plugin Configuration", unstablePrivacyPluginOptions)
.put("Synchronizer", unstableSynchronizerOptions)
.put("TransactionPool", unstableTransactionPoolOptions)
.put("Native Library", unstableNativeLibraryOptions)
.put("EVM Options", unstableEvmOptions)
.put("IPC Options", unstableIpcOptions)
Expand Down Expand Up @@ -1794,7 +1789,7 @@ private void validateOptions() {
}

private void validateTransactionPoolOptions() {
stableTransactionPoolOptions.validate(commandLine);
transactionPoolOptions.validate(commandLine, getActualGenesisConfigOptions());
}

private void validateRequiredOptions() {
Expand Down Expand Up @@ -2811,12 +2806,26 @@ private SynchronizerConfiguration buildSyncConfig() {
}

private TransactionPoolConfiguration buildTransactionPoolConfiguration() {
final var stableTxPoolOption = stableTransactionPoolOptions.toDomainObject();
return ImmutableTransactionPoolConfiguration.builder()
.from(stableTxPoolOption)
.unstable(unstableTransactionPoolOptions.toDomainObject())
.saveFile((dataPath.resolve(stableTxPoolOption.getSaveFile().getPath()).toFile()))
.build();
final var txPoolConf = transactionPoolOptions.toDomainObject();
final var txPoolConfBuilder =
ImmutableTransactionPoolConfiguration.builder()
.from(txPoolConf)
.saveFile((dataPath.resolve(txPoolConf.getSaveFile().getPath()).toFile()));

if (getActualGenesisConfigOptions().isZeroBaseFee()) {
logger.info(
"Forcing price bump for transaction replacement to 0, since we are on a zero basefee network");
txPoolConfBuilder.priceBump(Percentage.ZERO);
}

if (getMiningParameters().getMinTransactionGasPrice().equals(Wei.ZERO)
&& !transactionPoolOptions.isPriceBumpSet(commandLine)) {
logger.info(
"Forcing price bump for transaction replacement to 0, since min-gas-price is set to 0");
txPoolConfBuilder.priceBump(Percentage.ZERO);
}

return txPoolConfBuilder.build();
}

private MiningParameters getMiningParameters() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,19 @@
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.cli.options.stable;
package org.hyperledger.besu.cli.options;

import static org.hyperledger.besu.cli.DefaultCommandValues.MANDATORY_DOUBLE_FORMAT_HELP;
import static org.hyperledger.besu.cli.DefaultCommandValues.MANDATORY_INTEGER_FORMAT_HELP;
import static org.hyperledger.besu.cli.DefaultCommandValues.MANDATORY_LONG_FORMAT_HELP;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LAYERED;
import static org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration.Implementation.LEGACY;

import org.hyperledger.besu.cli.converter.DurationMillisConverter;
import org.hyperledger.besu.cli.converter.FractionConverter;
import org.hyperledger.besu.cli.converter.PercentageConverter;
import org.hyperledger.besu.cli.options.CLIOptions;
import org.hyperledger.besu.cli.util.CommandLineUtils;
import org.hyperledger.besu.config.GenesisConfigOptions;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration;
Expand All @@ -32,6 +33,7 @@
import org.hyperledger.besu.util.number.Percentage;

import java.io.File;
import java.time.Duration;
import java.util.List;
import java.util.Set;

Expand Down Expand Up @@ -195,6 +197,39 @@ static class Legacy {
Integer txPoolMaxSize = TransactionPoolConfiguration.DEFAULT_MAX_PENDING_TRANSACTIONS;
}

@CommandLine.ArgGroup(validate = false)
private final TransactionPoolOptions.Unstable unstableOptions =
new TransactionPoolOptions.Unstable();

static class Unstable {
private static final String TX_MESSAGE_KEEP_ALIVE_SEC_FLAG =
"--Xincoming-tx-messages-keep-alive-seconds";

private static final String ETH65_TX_ANNOUNCED_BUFFERING_PERIOD_FLAG =
"--Xeth65-tx-announced-buffering-period-milliseconds";

@CommandLine.Option(
names = {TX_MESSAGE_KEEP_ALIVE_SEC_FLAG},
paramLabel = "<INTEGER>",
hidden = true,
description =
"Keep alive of incoming transaction messages in seconds (default: ${DEFAULT-VALUE})",
arity = "1")
private Integer txMessageKeepAliveSeconds =
TransactionPoolConfiguration.Unstable.DEFAULT_TX_MSG_KEEP_ALIVE;

@CommandLine.Option(
names = {ETH65_TX_ANNOUNCED_BUFFERING_PERIOD_FLAG},
paramLabel = "<LONG>",
converter = DurationMillisConverter.class,
hidden = true,
description =
"The period for which the announced transactions remain in the buffer before being requested from the peers in milliseconds (default: ${DEFAULT-VALUE})",
arity = "1")
private Duration eth65TrxAnnouncedBufferingPeriod =
TransactionPoolConfiguration.Unstable.ETH65_TRX_ANNOUNCED_BUFFERING_PERIOD;
}

private TransactionPoolOptions() {}

/**
Expand Down Expand Up @@ -230,6 +265,10 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
config.getTxPoolLimitByAccountPercentage();
options.legacyOptions.txPoolMaxSize = config.getTxPoolMaxSize();
options.legacyOptions.pendingTxRetentionPeriod = config.getPendingTxRetentionPeriod();
options.unstableOptions.txMessageKeepAliveSeconds =
config.getUnstable().getTxMessageKeepAliveSeconds();
options.unstableOptions.eth65TrxAnnouncedBufferingPeriod =
config.getUnstable().getEth65TrxAnnouncedBufferingPeriod();

return options;
}
Expand All @@ -239,8 +278,10 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati
* options are valid for the selected implementation.
*
* @param commandLine the full commandLine to check all the options specified by the user
* @param genesisConfigOptions the genesis config options
*/
public void validate(final CommandLine commandLine) {
public void validate(
final CommandLine commandLine, final GenesisConfigOptions genesisConfigOptions) {
CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"Could not use legacy transaction pool options with layered implementation",
Expand All @@ -252,6 +293,12 @@ public void validate(final CommandLine commandLine) {
"Could not use layered transaction pool options with legacy implementation",
!txPoolImplementation.equals(LEGACY),
CommandLineUtils.getCLIOptionNames(Layered.class));

CommandLineUtils.failIfOptionDoesntMeetRequirement(
commandLine,
"Price bump option is not compatible with zero base fee market",
!genesisConfigOptions.isZeroBaseFee(),
List.of(TX_POOL_PRICE_BUMP));
}

@Override
Expand All @@ -271,11 +318,26 @@ public TransactionPoolConfiguration toDomainObject() {
.txPoolLimitByAccountPercentage(legacyOptions.txPoolLimitByAccountPercentage)
.txPoolMaxSize(legacyOptions.txPoolMaxSize)
.pendingTxRetentionPeriod(legacyOptions.pendingTxRetentionPeriod)
.unstable(
ImmutableTransactionPoolConfiguration.Unstable.builder()
.txMessageKeepAliveSeconds(unstableOptions.txMessageKeepAliveSeconds)
.eth65TrxAnnouncedBufferingPeriod(unstableOptions.eth65TrxAnnouncedBufferingPeriod)
.build())
.build();
}

@Override
public List<String> getCLIOptions() {
return CommandLineUtils.getCLIOptions(this, new TransactionPoolOptions());
}

/**
* Is price bump option set?
*
* @param commandLine the command line
* @return true is tx-pool-price-bump is set
*/
public boolean isPriceBumpSet(final CommandLine commandLine) {
return CommandLineUtils.isOptionSet(commandLine, TransactionPoolOptions.TX_POOL_PRICE_BUMP);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,17 @@ private static boolean isOptionSet(final CommandLine.Model.OptionSpec option) {
return false;
}
}

/**
* Is the option with that name set on the command line?
*
* @param commandLine the command line
* @param optionName the option name to check
* @return true if set
*/
public static boolean isOptionSet(final CommandLine commandLine, final String optionName) {
return commandLine.getCommandSpec().options().stream()
.filter(optionSpec -> Arrays.stream(optionSpec.names()).anyMatch(optionName::equals))
.anyMatch(CommandLineUtils::isOptionSet);
}
}
Loading

0 comments on commit 636ad8a

Please sign in to comment.