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 TransactionEvaluationContext to pass data between selectors and plugin #6381

Merged
merged 3 commits into from
Jan 12, 2024

Conversation

fab-10
Copy link
Contributor

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

PR description

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 TransactionEvaluationContext 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.

Fixed Issue(s)

…and plugin (hyperledger#30)

* Introduce TransactionSelectionContext to pass data between selectors

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

* Use TransactionSelectionContext in all the methods of the plugin selector

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

---------

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

github-actions bot commented Jan 10, 2024

  • 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

Signed-off-by: Fabio Di Fabio <[email protected]>
* @param transactionSelectionResult The transaction selection result
*/
default void onTransactionNotSelected(
final PendingTransaction pendingTransaction,
final TransactionEvaluationContext<? extends PendingTransaction> evaluationContext,

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'evaluationContext' is never used.
* @param processingResult The result of processing the selected transaction.
*/
default void onTransactionSelected(
final PendingTransaction pendingTransaction,
final TransactionEvaluationContext<? extends PendingTransaction> evaluationContext,

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'evaluationContext' is never used.
* @param processingResult the transaction processing result
* @return TransactionSelectionResult that indicates whether to include the transaction
*/
TransactionSelectionResult evaluateTransactionPostProcessing(
PendingTransaction pendingTransaction, TransactionProcessingResult processingResult);
TransactionEvaluationContext<? extends PendingTransaction> evaluationContext,

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'evaluationContext' is never used.
* @param processingResult the transaction processing result
* @return TransactionSelectionResult that indicates whether to include the transaction
*/
TransactionSelectionResult evaluateTransactionPostProcessing(
PendingTransaction pendingTransaction, TransactionProcessingResult processingResult);
TransactionEvaluationContext<? extends PendingTransaction> evaluationContext,
TransactionProcessingResult processingResult);

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'processingResult' is never used.
* @return TransactionSelectionResult that indicates whether to include the transaction
*/
TransactionSelectionResult evaluateTransactionPreProcessing(
PendingTransaction pendingTransaction);
TransactionEvaluationContext<? extends PendingTransaction> evaluationContext);

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'evaluationContext' is never used.
@fab-10 fab-10 marked this pull request as ready for review January 10, 2024 13:53
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) January 12, 2024 08:34
@fab-10 fab-10 merged commit 540f5a4 into hyperledger:main Jan 12, 2024
17 checks passed
@fab-10 fab-10 deleted the transaction-selection-context branch January 12, 2024 09:34
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