Skip to content

Use PendingTransaction in BlockTransactionSelector#5966

Merged
fab-10 merged 1 commit intobesu-eth:mainfrom
fab-10:tx-selector-pending-tx
Oct 2, 2023
Merged

Use PendingTransaction in BlockTransactionSelector#5966
fab-10 merged 1 commit intobesu-eth:mainfrom
fab-10:tx-selector-pending-tx

Conversation

@fab-10
Copy link
Copy Markdown
Contributor

@fab-10 fab-10 commented Sep 28, 2023

PR description

This is just a refactoring, no functionality change, to have the PendingTransaction object during the selection of transactions, since in the PendingTransaction there are additional metadata about the candidate transaction that can be used by the selection process, and more will be added in a next PR to better support priority senders.

@github-actions
Copy link
Copy Markdown

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • 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

* and deciding which transactions to drop when the transaction pool reaches its size limit.
*/
public abstract class PendingTransaction {
public abstract class PendingTransaction

Check notice

Code scanning / CodeQL

Class has same name as super class

PendingTransaction has the same name as its supertype [org.hyperledger.besu.datatypes.PendingTransaction](1).
@fab-10 fab-10 force-pushed the tx-selector-pending-tx branch from 7d1bcf1 to 68cbc2c Compare September 28, 2023 09:47
@fab-10 fab-10 marked this pull request as ready for review September 28, 2023 10:01
@fab-10 fab-10 mentioned this pull request Sep 28, 2023
Copy link
Copy Markdown
Contributor

@Gabriel-Trintinalia Gabriel-Trintinalia left a comment

Choose a reason for hiding this comment

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

LGTM, I am wondering if it makes sense to have it named PooledTransaction instead of PendingTransaction, since PendingTransaction are transactions that are in the transaction pool and not confirmed yet. That would also match the methods GetPooledTransactions and NewPooledTransactionHashes as well as the EncodingContext in the transaction encoder

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 force-pushed the tx-selector-pending-tx branch from 68cbc2c to 5e62c01 Compare October 2, 2023 08:10
@fab-10 fab-10 enabled auto-merge (squash) October 2, 2023 08:10
@fab-10
Copy link
Copy Markdown
Contributor Author

fab-10 commented Oct 2, 2023

@Gabriel-Trintinalia it seems that pending and pooled are both used as synonyms, here for example, but I have no objections if you want to evaluate a renaming

@fab-10 fab-10 merged commit 987d33c into besu-eth:main Oct 2, 2023
@fab-10 fab-10 deleted the tx-selector-pending-tx branch October 2, 2023 09:32
jflo pushed a commit to jflo/besu that referenced this pull request Nov 10, 2023
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
NickSneo pushed a commit to NickSneo/besu that referenced this pull request Nov 12, 2023
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
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.

3 participants