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.

Sign up for GitHub

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

Add test to check that TX selection doesn't prioritize TXs from the same sender #6022

Merged
merged 14 commits into from
Nov 13, 2023
Merged

Add test to check that TX selection doesn't prioritize TXs from the same sender #6022

Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c474dea
Add bool TX pool flag to allow TX selection to skip finding all TXs f…
matthew1001 Oct 11, 2023
ee7a66f
Implement one approach to eliminating sender TX grouping. Note - this…
matthew1001 Oct 18, 2023
4199ec6
Merge main
matthew1001 Nov 8, 2023
3ae0747
Make the no-sdr-grouping flag experimental, remove code no longer nee…
matthew1001 Nov 8, 2023
20ad749
Update tests, tidy up empty lines
matthew1001 Nov 8, 2023
cc214ce
Fix tests to be inline with recent sender-priority changes
matthew1001 Nov 8, 2023
6600b49
Merge main
matthew1001 Nov 8, 2023
439a0ec
Address PR comments
matthew1001 Nov 9, 2023
4d0d375
Merge with main
matthew1001 Nov 10, 2023
540ef39
More unit test updates
matthew1001 Nov 10, 2023
3aefab8
Remove --Xtx-pool-disable-sender-grouping - the new legacy TX pool or…
matthew1001 Nov 10, 2023
f7cba5d
Fix test merge error
matthew1001 Nov 10, 2023
42f9fcb
Merge branch 'main' into no-sdr-grouping
matthew1001 Nov 10, 2023
2d5b16f
Merge branch 'main' into no-sdr-grouping
fab-10 Nov 13, 2023
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
94 changes: 94 additions & 0 deletions ...yperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
public abstract class AbstractPendingTransactionsTestBase {

protected static final int MAX_TRANSACTIONS = 5;
protected static final int MAX_TRANSACTIONS_LARGE_POOL = 15;
private static final float LIMITED_TRANSACTIONS_BY_SENDER_PERCENTAGE = 0.8f;
protected static final Supplier<SignatureAlgorithm> SIGNATURE_ALGORITHM =
Suppliers.memoize(SignatureAlgorithmFactory::getInstance);
Expand Down Expand Up @@ -93,9 +94,24 @@ public abstract class AbstractPendingTransactionsTestBase {
.build();
protected PendingTransactions senderLimitedTransactions =
getPendingTransactions(senderLimitedConfig, Optional.empty());
protected AbstractPendingTransactionsSorter transactionsLarge =
getPendingTransactions(
ImmutableTransactionPoolConfiguration.builder()
.txPoolMaxSize(MAX_TRANSACTIONS_LARGE_POOL)
.txPoolLimitByAccountPercentage(Fraction.fromFloat(1.0f))
.build(),
Optional.empty());

protected final Transaction transaction1 = createTransaction(2);
protected final Transaction transaction2 = createTransaction(1);
protected final Transaction transaction3 = createTransaction(3);

protected final Transaction zeroGasPriceTX1Sdr1 = createZeroGasPriceTransactionSender1(1);
protected final Transaction zeroGasPriceTX2Sdr1 = createZeroGasPriceTransactionSender1(2);
protected final Transaction zeroGasPriceTX3Sdr1 = createZeroGasPriceTransactionSender1(3);
protected final Transaction zeroGasPriceTX1Sdr2 = createZeroGasPriceTransactionSender2(1);
protected final Transaction zeroGasPriceTX2Sdr2 = createZeroGasPriceTransactionSender2(2);
protected final Transaction zeroGasPriceTX3Sdr2 = createZeroGasPriceTransactionSender2(3);

protected final PendingTransactionAddedListener listener =
mock(PendingTransactionAddedListener.class);
Expand All @@ -109,6 +125,7 @@ abstract AbstractPendingTransactionsSorter getPendingTransactions(

@Test
public void shouldReturnExclusivelyLocalTransactionsWhenAppropriate() {

final Transaction localTransaction0 = createTransaction(0);
transactions.addTransaction(createLocalPendingTransaction(localTransaction0), Optional.empty());
assertThat(transactions.size()).isEqualTo(1);
Expand Down Expand Up @@ -320,6 +337,62 @@ public void shouldNotNotifyDroppedListenerWhenTransactionAddedToBlock() {
verifyNoInteractions(droppedListener);
}

@Test
public void selectTransactionsInCorrectOrder() {
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX2Sdr1), Optional.empty()))
.isEqualTo(ADDED);
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX3Sdr1), Optional.empty()))
.isEqualTo(ADDED);
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX1Sdr2), Optional.empty()))
.isEqualTo(ADDED);
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX1Sdr1), Optional.empty()))
.isEqualTo(ADDED);
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX2Sdr2), Optional.empty()))
.isEqualTo(ADDED);
assertThat(
transactionsLarge.addTransaction(
createRemotePendingTransaction(zeroGasPriceTX3Sdr2), Optional.empty()))
.isEqualTo(ADDED);

final List<Transaction> parsedTransactions = Lists.newArrayList();
transactionsLarge.selectTransactions(
pendingTx -> {
parsedTransactions.add(pendingTx.getTransaction());

if (parsedTransactions.size() == 6) {
return TransactionSelectionResult.BLOCK_OCCUPANCY_ABOVE_THRESHOLD;
}
return SELECTED;
});

// All 6 transactions should have been selected
assertThat(parsedTransactions.size()).isEqualTo(6);

// The order should be:
// sender 2, nonce 1
// sender 1, nonce 1
// sender 1, nonce 2
// sender 1, nonce 3
// sender 2, nonce 2
// sender 2, nonce 3
assertThat(parsedTransactions.get(0)).isEqualTo(zeroGasPriceTX1Sdr2);
assertThat(parsedTransactions.get(1)).isEqualTo(zeroGasPriceTX1Sdr1);
assertThat(parsedTransactions.get(2)).isEqualTo(zeroGasPriceTX2Sdr1);
assertThat(parsedTransactions.get(3)).isEqualTo(zeroGasPriceTX3Sdr1);
assertThat(parsedTransactions.get(4)).isEqualTo(zeroGasPriceTX2Sdr2);
assertThat(parsedTransactions.get(5)).isEqualTo(zeroGasPriceTX3Sdr2);
}

@Test
public void selectTransactionsUntilSelectorRequestsNoMore() {
transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty());
Expand Down Expand Up @@ -659,9 +732,30 @@ protected Transaction createTransaction(final long transactionNumber) {
return new TransactionTestFixture()
.value(Wei.of(transactionNumber))
.nonce(transactionNumber)
.gasPrice(Wei.of(0))
.createTransaction(KEYS1);
}

protected Transaction createZeroGasPriceTransactionSender1(final long transactionNumber) {
return new TransactionTestFixture()
.value(Wei.of(transactionNumber))
.nonce(transactionNumber)
.gasPrice(Wei.of(0))
.maxFeePerGas(Optional.of(Wei.of(0)))
.maxPriorityFeePerGas(Optional.of(Wei.of(0)))
.createTransaction(KEYS1);
}

protected Transaction createZeroGasPriceTransactionSender2(final long transactionNumber) {
return new TransactionTestFixture()
.value(Wei.of(transactionNumber))
.nonce(transactionNumber)
.gasPrice(Wei.of(0))
.maxFeePerGas(Optional.of(Wei.of(0)))
.maxPriorityFeePerGas(Optional.of(Wei.of(0)))
.createTransaction(KEYS2);
}

private PendingTransaction createRemotePendingTransaction(
final Transaction transaction, final long addedAt) {
return PendingTransaction.newPendingTransaction(transaction, false, false, addedAt);
Expand Down
Loading