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

Introduce TransactionSelectionContext to pass data between selectors and plugin #30

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

fab-10
Copy link
Collaborator

@fab-10 fab-10 commented Jan 8, 2024

Some tx selector plugins needs more context about the current tx being evaluated, for example the current minGasPrice, or the effective price of the tx and possibly more data in the future, for this we introduce the TransactionSelectionContext to hold that data and pass it across all the selectors and the plugin. Another advantage of this context is to avoid to compute multiple time the same data, in case it is needed in more that one selector.

This PR will be ported to main Besu repo as well.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@Gabriel-Trintinalia
Copy link

lgtm, does it make sense to have onTransactionSelected and onTransactionNotSelected also passing the TransactionEvaluationContext instead of PendingTransaction so that plugins can handle the events with the same information that performed the evaluation?

@fab-10
Copy link
Collaborator Author

fab-10 commented Jan 9, 2024

@Gabriel-Trintinalia good suggestion, will pass the context to those method too

…ctor

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@Filter94
Copy link

Filter94 commented Jan 9, 2024

Will we be able to get RLP encoded transaction size in the plugin, without it being a part of the new context?

@fab-10
Copy link
Collaborator Author

fab-10 commented Jan 9, 2024

Will we be able to get RLP encoded transaction size in the plugin, without it being a part of the new context?

that information is already provided by Transaction::getSize

@fab-10
Copy link
Collaborator Author

fab-10 commented Jan 10, 2024

I am merging this since all the comments have been resolved and it is needed to unblock the plugin side.
I will open the same PR on main Besu repo so if you still have feedback you can add there.

@fab-10 fab-10 merged commit c029a9f into zkbesu Jan 10, 2024
4 checks passed
@fab-10 fab-10 deleted the transaction-evaluation-context branch March 19, 2024 13:32
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