Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make layered txpool aware of minGasPrice and minPriorityFeePerGas dynamic options #6611

Merged
merged 5 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
### Additions and Improvements
- Extend `Blockchain` service [#6592](https://github.com/hyperledger/besu/pull/6592)
- RocksDB database metadata refactoring [#6555](https://github.com/hyperledger/besu/pull/6555)
- Make layered txpool aware of minGasPrice and minPriorityFeePerGas dynamic options [#6611](https://github.com/hyperledger/besu/pull/6611)

### Bug fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,8 @@ public BesuController build() {
syncState,
transactionPoolConfiguration,
pluginTransactionValidatorFactory,
besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()));
besuComponent.map(BesuComponent::getBlobCache).orElse(new BlobCache()),
miningParameters);

final List<PeerValidator> peerValidators = createPeerValidators(protocolSchedule);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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.MiningParameters;
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 @@ -167,7 +168,8 @@ public void setUp() {
syncState,
txPoolConfig,
null,
new BlobCache());
new BlobCache(),
MiningParameters.newDefault());

serviceImpl = new BesuEventsImpl(blockchain, blockBroadcaster, transactionPool, syncState);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ public abstract class AbstractIsolationTests {
new EndLayer(txPoolMetrics),
txPoolMetrics,
transactionReplacementTester,
new BlobCache()));
new BlobCache(),
MiningParameters.newDefault()));

protected final List<GenesisAllocation> accounts =
GenesisConfigFile.development()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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;
Expand Down Expand Up @@ -55,7 +56,8 @@ public static TransactionPool createTransactionPool(
final SyncState syncState,
final TransactionPoolConfiguration transactionPoolConfiguration,
final PluginTransactionValidatorFactory pluginTransactionValidatorFactory,
final BlobCache blobCache) {
final BlobCache blobCache,
final MiningParameters miningParameters) {

final TransactionPoolMetrics metrics = new TransactionPoolMetrics(metricsSystem);

Expand All @@ -78,7 +80,8 @@ public static TransactionPool createTransactionPool(
transactionsMessageSender,
newPooledTransactionHashesMessageSender,
pluginTransactionValidatorFactory,
blobCache);
blobCache,
miningParameters);
}

static TransactionPool createTransactionPool(
Expand All @@ -93,7 +96,8 @@ static TransactionPool createTransactionPool(
final TransactionsMessageSender transactionsMessageSender,
final NewPooledTransactionHashesMessageSender newPooledTransactionHashesMessageSender,
final PluginTransactionValidatorFactory pluginTransactionValidatorFactory,
final BlobCache blobCache) {
final BlobCache blobCache,
final MiningParameters miningParameters) {

final TransactionPool transactionPool =
new TransactionPool(
Expand All @@ -104,7 +108,8 @@ static TransactionPool createTransactionPool(
clock,
metrics,
transactionPoolConfiguration,
blobCache),
blobCache,
miningParameters),
protocolSchedule,
protocolContext,
new TransactionBroadcaster(
Expand Down Expand Up @@ -233,7 +238,8 @@ private static PendingTransactions createPendingTransactions(
final Clock clock,
final TransactionPoolMetrics metrics,
final TransactionPoolConfiguration transactionPoolConfiguration,
final BlobCache blobCache) {
final BlobCache blobCache,
final MiningParameters miningParameters) {

boolean isFeeMarketImplementBaseFee =
protocolSchedule.anyMatch(
Expand All @@ -246,7 +252,8 @@ private static PendingTransactions createPendingTransactions(
metrics,
transactionPoolConfiguration,
isFeeMarketImplementBaseFee,
blobCache);
blobCache,
miningParameters);
} else {
return createPendingTransactionSorter(
protocolContext,
Expand Down Expand Up @@ -284,7 +291,8 @@ private static PendingTransactions createLayeredPendingTransactions(
final TransactionPoolMetrics metrics,
final TransactionPoolConfiguration transactionPoolConfiguration,
final boolean isFeeMarketImplementBaseFee,
final BlobCache blobCache) {
final BlobCache blobCache,
final MiningParameters miningParameters) {

final TransactionPoolReplacementHandler transactionReplacementHandler =
new TransactionPoolReplacementHandler(transactionPoolConfiguration.getPriceBump());
Expand Down Expand Up @@ -327,15 +335,17 @@ private static PendingTransactions createLayeredPendingTransactions(
metrics,
transactionReplacementTester,
feeMarket,
blobCache);
blobCache,
miningParameters);
} else {
pendingTransactionsSorter =
new GasPricePrioritizedTransactions(
transactionPoolConfiguration,
readyTransactions,
metrics,
transactionReplacementTester,
blobCache);
blobCache,
miningParameters);
}

return new LayeredPendingTransactions(transactionPoolConfiguration, pendingTransactionsSorter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.eth.transactions.layered;

import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.ethereum.core.MiningParameters;
import org.hyperledger.besu.ethereum.eth.transactions.BlobCache;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
import org.hyperledger.besu.ethereum.eth.transactions.TransactionAddedResult;
Expand All @@ -36,16 +37,19 @@
*/
public abstract class AbstractPrioritizedTransactions extends AbstractSequentialTransactionsLayer {
protected final TreeSet<PendingTransaction> orderByFee;
protected final MiningParameters miningParameters;

public AbstractPrioritizedTransactions(
final TransactionPoolConfiguration poolConfig,
final TransactionsLayer prioritizedTransactions,
final TransactionPoolMetrics metrics,
final BiFunction<PendingTransaction, PendingTransaction, Boolean>
transactionReplacementTester,
final BlobCache blobCache) {
final BlobCache blobCache,
final MiningParameters miningParameters) {
super(poolConfig, prioritizedTransactions, transactionReplacementTester, metrics, blobCache);
this.orderByFee = new TreeSet<>(this::compareByFee);
this.miningParameters = miningParameters;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.hyperledger.besu.datatypes.Wei;
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.transactions.BlobCache;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
Expand Down Expand Up @@ -47,8 +48,10 @@ public BaseFeePrioritizedTransactions(
final BiFunction<PendingTransaction, PendingTransaction, Boolean>
transactionReplacementTester,
final FeeMarket feeMarket,
final BlobCache blobCache) {
super(poolConfig, nextLayer, metrics, transactionReplacementTester, blobCache);
final BlobCache blobCache,
final MiningParameters miningParameters) {
super(
poolConfig, nextLayer, metrics, transactionReplacementTester, blobCache, miningParameters);
this.nextBlockBaseFee =
Optional.of(calculateNextBlockBaseFee(feeMarket, chainHeadHeaderSupplier.get()));
}
Expand Down Expand Up @@ -146,11 +149,34 @@ private Wei calculateNextBlockBaseFee(final FeeMarket feeMarket, final BlockHead

@Override
protected boolean promotionFilter(final PendingTransaction pendingTransaction) {
return nextBlockBaseFee
.map(
baseFee ->
pendingTransaction.getTransaction().getMaxGasPrice().greaterOrEqualThan(baseFee))
.orElse(false);
// check if the tx is willing to pay at least the base fee
if (nextBlockBaseFee
.map(pendingTransaction.getTransaction().getMaxGasPrice()::lessThan)
.orElse(true)) {
return false;
}

// priority txs are promoted even if they pay less
if (!pendingTransaction.hasPriority()) {
// check if max fee per gas is higher than the min gas price
if (pendingTransaction
.getTransaction()
.getMaxGasPrice()
.lessThan(miningParameters.getMinTransactionGasPrice())) {
return false;
}

// check if enough priority fee is paid
if (!miningParameters.getMinPriorityFeePerGas().equals(Wei.ZERO)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why might we promote the tx when the min priority fee is 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just an optimization, since by default minPriorityFeePerGas is 0, then if the default is not changed we can just skip the check

final Wei priorityFeePerGas =
pendingTransaction.getTransaction().getEffectivePriorityFeePerGas(nextBlockBaseFee);
if (priorityFeePerGas.lessThan(miningParameters.getMinPriorityFeePerGas())) {
return false;
}
}
}

return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static java.util.Comparator.comparing;

import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.MiningParameters;
import org.hyperledger.besu.ethereum.eth.transactions.BlobCache;
import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction;
import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
Expand All @@ -39,8 +40,10 @@ public GasPricePrioritizedTransactions(
final TransactionPoolMetrics metrics,
final BiFunction<PendingTransaction, PendingTransaction, Boolean>
transactionReplacementTester,
final BlobCache blobCache) {
super(poolConfig, nextLayer, metrics, transactionReplacementTester, blobCache);
final BlobCache blobCache,
final MiningParameters miningParameters) {
super(
poolConfig, nextLayer, metrics, transactionReplacementTester, blobCache, miningParameters);
}

@Override
Expand All @@ -58,7 +61,12 @@ protected void internalBlockAdded(final BlockHeader blockHeader, final FeeMarket

@Override
protected boolean promotionFilter(final PendingTransaction pendingTransaction) {
return true;
return pendingTransaction.hasPriority()
|| pendingTransaction
.getTransaction()
.getGasPrice()
.map(miningParameters.getMinTransactionGasPrice()::lessThan)
.orElse(false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
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;
Expand Down Expand Up @@ -1118,7 +1119,8 @@ public void transactionMessagesGoToTheCorrectExecutor() {
new SyncState(blockchain, ethManager.ethContext().getEthPeers()),
TransactionPoolConfiguration.DEFAULT,
null,
new BlobCache())
new BlobCache(),
MiningParameters.newDefault())
.setEnabled();

// Send just a transaction message.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
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;
Expand Down Expand Up @@ -136,7 +137,8 @@ public void setupTest() {
syncState,
TransactionPoolConfiguration.DEFAULT,
null,
new BlobCache());
new BlobCache(),
MiningParameters.newDefault());
transactionPool.setEnabled();

ethProtocolManager =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ public abstract class AbstractTransactionPoolTest {
private static final KeyPair KEY_PAIR2 =
SignatureAlgorithmFactory.getInstance().generateKeyPair();
protected static final Wei BASE_FEE_FLOOR = Wei.of(7L);
protected static final Wei DEFAULT_MIN_GAS_PRICE = Wei.of(50L);

@Mock(answer = Answers.RETURNS_DEEP_STUBS)
protected TransactionValidatorFactory transactionValidatorFactory;
Expand Down Expand Up @@ -455,6 +456,7 @@ public void shouldNotReAddTransactionsThatAreInBothForksWhenReorgHappens() {
}

@Test
@EnabledIf("isBaseFeeMarket")
public void shouldReAddBlobTxsWhenReorgHappens() {
givenTransactionIsValid(transaction0);
givenTransactionIsValid(transaction1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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;
Expand Down Expand Up @@ -166,7 +167,8 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod
syncState,
TransactionPoolConfiguration.DEFAULT,
null,
new BlobCache());
new BlobCache(),
MiningParameters.newDefault());

final EthProtocolManager ethProtocolManager =
new EthProtocolManager(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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;
Expand Down Expand Up @@ -251,7 +252,8 @@ private void setupInitialSyncPhase(final boolean hasInitialSyncPhase) {
transactionsMessageSender,
newPooledTransactionHashesMessageSender,
null,
new BlobCache());
new BlobCache(),
MiningParameters.newDefault());

ethProtocolManager =
new EthProtocolManager(
Expand Down Expand Up @@ -359,7 +361,8 @@ private TransactionPool createTransactionPool(
.build())
.build(),
null,
new BlobCache());
new BlobCache(),
MiningParameters.newDefault());

txPool.setEnabled();
return txPool;
Expand Down
Loading
Loading