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

[Plugin API] - TransactionSelector - Notify plugins when transaction is selected/rejected #6005

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
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,7 @@ public TransactionSelectionResults buildTransactionListForBlock() {
pendingTransaction -> {
final var res = evaluateTransaction(pendingTransaction);
if (!res.selected()) {
transactionSelectionResults.updateNotSelected(
pendingTransaction.getTransaction(), res);
updateTransactionRejected(pendingTransaction, res);
}
return res;
});
Expand All @@ -169,9 +168,10 @@ public TransactionSelectionResults buildTransactionListForBlock() {
public TransactionSelectionResults evaluateTransactions(final List<Transaction> transactions) {
transactions.forEach(
transaction -> {
final var res = evaluateTransaction(new PendingTransaction.Local(transaction));
var pendingTransaction = new PendingTransaction.Local(transaction);
final var res = evaluateTransaction(pendingTransaction);
if (!res.selected()) {
transactionSelectionResults.updateNotSelected(transaction, res);
updateTransactionRejected(pendingTransaction, res);
}
});
return transactionSelectionResults;
Expand Down Expand Up @@ -234,8 +234,7 @@ private TransactionSelectionResult evaluateTransaction(
final long blobGasUsed =
blockSelectionContext.gasCalculator().blobGasCost(transaction.getBlobCount());

transactionSelectionResults.updateSelected(
transaction, receipt, gasUsedByTransaction, blobGasUsed);
updateTransactionSelected(pendingTransaction, receipt, gasUsedByTransaction, blobGasUsed);

LOG.atTrace()
.setMessage("Selected {} for block creation")
Expand All @@ -245,6 +244,30 @@ private TransactionSelectionResult evaluateTransaction(
return TransactionSelectionResult.SELECTED;
}

private void updateTransactionSelected(
final PendingTransaction pendingTransaction,
final TransactionReceipt receipt,
final long gasUsedByTransaction,
final long blobGasUsed) {

transactionSelectionResults.updateSelected(
pendingTransaction.getTransaction(), receipt, gasUsedByTransaction, blobGasUsed);

// notify external selector if any
externalTransactionSelector.onTransactionSelected(pendingTransaction);
}

private void updateTransactionRejected(
final PendingTransaction pendingTransaction,
final TransactionSelectionResult processingResult) {

transactionSelectionResults.updateNotSelected(
pendingTransaction.getTransaction(), processingResult);

// notify external selector if any
externalTransactionSelector.onTransactionRejected(pendingTransaction);
}

/**
* This method evaluates a transaction by pre-processing it through a series of selectors. It
* first processes the transaction through internal selectors, and if the transaction is selected,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.config.GenesisConfigFile;
Expand All @@ -33,6 +36,7 @@
import org.hyperledger.besu.ethereum.ProtocolContext;
import org.hyperledger.besu.ethereum.blockcreation.txselection.BlockTransactionSelector;
import org.hyperledger.besu.ethereum.blockcreation.txselection.TransactionSelectionResults;
import org.hyperledger.besu.ethereum.blockcreation.txselection.selectors.AllAcceptingTransactionSelector;
import org.hyperledger.besu.ethereum.chain.DefaultBlockchain;
import org.hyperledger.besu.ethereum.chain.GenesisState;
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
Expand Down Expand Up @@ -84,6 +88,7 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
Expand Down Expand Up @@ -659,6 +664,43 @@ public TransactionSelectionResult evaluateTransactionPostProcessing(
.containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid")));
}

@Test
public void transactionSelectionPluginShouldBeNotifiedWhenTransactionSelectionCompletes() {
final TransactionSelectorFactory transactionSelectorFactory =
mock(TransactionSelectorFactory.class);
TransactionSelector transactionSelector = spy(AllAcceptingTransactionSelector.INSTANCE);
when(transactionSelectorFactory.create()).thenReturn(transactionSelector);

final Transaction transaction = createTransaction(0, Wei.of(10), 21_000);
ensureTransactionIsValid(transaction, 21_000, 0);

final Transaction invalidTransaction = createTransaction(1, Wei.of(10), 21_000);
ensureTransactionIsInvalid(
invalidTransaction, TransactionInvalidReason.PLUGIN_TX_VALIDATOR_INVALIDATED);
transactionPool.addRemoteTransactions(List.of(transaction, invalidTransaction));

createBlockSelectorWithTxSelPlugin(
transactionProcessor,
createBlock(300_000),
Wei.ZERO,
AddressHelpers.ofValue(1),
Wei.ZERO,
MIN_OCCUPANCY_80_PERCENT,
transactionSelectorFactory)
.buildTransactionListForBlock();

ArgumentCaptor<PendingTransaction> argumentCaptor =
ArgumentCaptor.forClass(PendingTransaction.class);

verify(transactionSelector).onTransactionSelected(argumentCaptor.capture());
PendingTransaction selected = argumentCaptor.getValue();
assertThat(selected.getTransaction()).isEqualTo(transaction);

verify(transactionSelector).onTransactionRejected(argumentCaptor.capture());
PendingTransaction rejectedTransaction = argumentCaptor.getValue();
assertThat(rejectedTransaction.getTransaction()).isEqualTo(invalidTransaction);
}

@Test
public void transactionWithIncorrectNonceRemainsInPoolAndNotSelected() {
final ProcessableBlockHeader blockHeader = createBlock(5_000_000);
Expand Down
2 changes: 1 addition & 1 deletion plugin-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Calculated : ${currentHash}
tasks.register('checkAPIChanges', FileStateChecker) {
description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
files = sourceSets.main.allJava.files
knownHash = '8NVdDoCnMQiibEZUsZuemZU+wm3W1LPklcQkiKsriKs='
knownHash = '+7wo9cABKEFyYvjtpDFAOXqVKBAkffdnb433hT0VQ7I='
}
check.dependsOn('checkAPIChanges')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,17 @@
*/
TransactionSelectionResult evaluateTransactionPostProcessing(
PendingTransaction pendingTransaction, TransactionProcessingResult processingResult);

/**
* Method called when a transaction is selected to be added to a block.
*
* @param pendingTransaction The transaction that has been selected.
*/
default void onTransactionSelected(final PendingTransaction pendingTransaction) {}

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'pendingTransaction' is never used.
/**
* Method called when a transaction is rejected to be added to a block.
*
* @param pendingTransaction The transaction that has been rejected.
*/
default void onTransactionRejected(final PendingTransaction pendingTransaction) {}

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'pendingTransaction' is never used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there are many reasons a tx is not selected for inclusion, it make sense here to also pass the TransactionSelectionResult otherwise the plugin could make the wrong assumption.

Then not sure about the word rejected, since a tx could not be selected, because it is invalid, so it is also removed from the pool, or just skipped because there is no more space or similarly does not fit with other limits in the current block, but could go in a future block

}
Loading