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

Stop evaluating txs for sender after the first skipped one #5498

Merged
merged 36 commits into from
May 31, 2023

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented May 25, 2023

PR description

This PR is built on top of #5492 and #5491 so check them first.

This PR introduce a simple optimization to avoid to waste time validating transactions that will not be selected during the current block creation process.
Basically if a sender have more than one prioritized candidate transactions, and for some reason any of them is not selected during the block creation process, it make no sense to keep evaluating any more transaction after the skipped one, since it will not be selected due to the nonce gap.

Enabling the selection stats implemented in #5492 is it possible to see the difference before and after this PR, the wasted validation work that we are removing is represented by the INVALID_TRANSIENT(NONCE_TOO_HIGH) label.

Before

Selection stats: Totals[Evaluated=2000, Selected=109, Skipped=1891, Dropped=0]; Detailed[INVALID_TRANSIENT(GAS_PRICE_BELOW_CURRENT_BASE_FEE)=1712, INVALID_TRANSIENT(NONCE_TOO_HIGH)=158, INVALID_TRANSIENT(TX_TOO_LARGE_FOR_REMAINING_GAS)=21, SELECTED=109]

After

Selection stats: Totals[Evaluated=1743, Selected=126, Skipped=1617, Dropped=0]; Detailed[INVALID_TRANSIENT(GAS_PRICE_BELOW_CURRENT_BASE_FEE)=1600, INVALID_TRANSIENT(TX_TOO_LARGE_FOR_REMAINING_GAS)=17, SELECTED=126]

fab-10 added 28 commits May 12, 2023 11:41
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
# Conflicts:
#	consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/PostMergeContext.java
#	ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineGetPayload.java
#	ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java
# Conflicts:
#	ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java
Signed-off-by: Fabio Di Fabio <[email protected]>
…x-selection-metrics

# Conflicts:
#	ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java
…txpool-tx-selection-metrics"

This reverts commit 1a899b4, reversing
changes made to fba82c8.

Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>

# Conflicts:
#	plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java
Signed-off-by: Fabio Di Fabio <[email protected]>
@github-actions
Copy link

github-actions bot commented May 25, 2023

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

@fab-10 fab-10 changed the title Stop evaluating sender txs after there is a skipped one for the sender Stop evaluating txs for sender after the first skipped one May 25, 2023
Signed-off-by: Fabio Di Fabio <[email protected]>
@fab-10 fab-10 marked this pull request as ready for review May 25, 2023 10:30
Signed-off-by: Fabio Di Fabio <[email protected]>
@fab-10 fab-10 force-pushed the layered-txpool-skip-sender-opt branch from f04312c to d2e8bb2 Compare May 25, 2023 10:42
# Conflicts:
#	ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java
#	ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java
#	plugin-api/build.gradle
#	plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java
#	plugin-api/src/main/java/org/hyperledger/besu/plugin/services/txselection/TransactionSelector.java
@fab-10 fab-10 force-pushed the layered-txpool-skip-sender-opt branch from 97a0a56 to 630ff82 Compare May 26, 2023 14:22
…l-skip-sender-opt

# Conflicts:
#	ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java
#	plugin-api/build.gradle
#	plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java
@fab-10 fab-10 force-pushed the layered-txpool-skip-sender-opt branch from 630ff82 to 0c31ae1 Compare May 26, 2023 15:05
fab-10 and others added 3 commits May 30, 2023 10:19
…ransactions/layered/LayeredPendingTransactions.java

Co-authored-by: Sally MacFarlane <[email protected]>
Signed-off-by: Fabio Di Fabio <[email protected]>
# Conflicts:
#	ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java
#	ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java
#	plugin-api/build.gradle
#	plugin-api/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionSelectionResult.java
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM

@fab-10 fab-10 enabled auto-merge (squash) May 31, 2023 06:58
@fab-10 fab-10 merged commit 5fe53cf into hyperledger:main May 31, 2023
@fab-10 fab-10 deleted the layered-txpool-skip-sender-opt branch May 31, 2023 07:21
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants