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.

Sign up for GitHub

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

Jump to bottom

Add plugin API to select Transactions #5396

Merged
merged 19 commits into from
May 23, 2023
Merged

Add plugin API to select Transactions #5396

merged 19 commits into from
May 23, 2023

Conversation

pinges
Copy link
Contributor

@pinges pinges commented Apr 27, 2023

PR description

This PR adds an API that can be used to select Transactions while they are added to a block

pinges added 3 commits April 20, 2023 18:47
@pinges
first go at the plugin
cee6d43 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges
Merge branch 'main' of github.com:hyperledger/besu into AddTransactio…
3988260 
…nEvaluator
@pinges
working
9ce176c 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@github-actions
Copy link

github-actions bot commented Apr 27, 2023 •
edited by pinges
Loading

  • 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

@pinges
merge main
f2ef300 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 27, 2023
View reviewed changes
...-api/src/main/java/org/hyperledger/besu/plugin/services/txselection/TransactionSelector.java
public interface TransactionSelector {

default TransactionSelectionResult selectTransaction(
final Transaction transaction, final TransactionReceipt receipt) {

Check notice

Code scanning / CodeQL

Useless parameter

The parameter 'transaction' is never used.
...-api/src/main/java/org/hyperledger/besu/plugin/services/txselection/TransactionSelector.java
public interface TransactionSelector {

default TransactionSelectionResult selectTransaction(
final Transaction transaction, final TransactionReceipt receipt) {

Check notice

Code scanning / CodeQL

Useless parameter

The parameter 'receipt' is never used.
@pinges
spotless
247951c 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
Gabriel-Trintinalia
Gabriel-Trintinalia reviewed Apr 28, 2023
View reviewed changes
besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -360,6 +364,8 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
@CommandLine.ArgGroup(validate = false, heading = "@|bold P2P Discovery Options|@%n")
P2PDiscoveryOptionGroup p2PDiscoveryOptionGroup = new P2PDiscoveryOptionGroup();

private final TransactionSelectionServiceImpl transactionSelectionServiceImpl;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a good idea to annotate it with @Unstable as we have for the other new services

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java
import java.util.Optional;

/** The Storage service implementation. */
public class TransactionSelectionServiceImpl implements TransactionSelectionService {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a good idea to annotate it with @Unstable as we have for the other new services

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

plugin-api/src/main/java/org/hyperledger/besu/plugin/services/TransactionSelectionService.java Outdated Show resolved Hide resolved
besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java Outdated Show resolved Hide resolved
@Gabriel-Trintinalia Gabriel-Trintinalia added plugins TeamRevenant GH issues worked on by Revenant Team labels Apr 28, 2023
pinges added 2 commits May 17, 2023 16:38
@pinges
changes to make it work smoothly
08d70d9 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges
mark interface as unstable
a4c603b 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
macfarla
macfarla reviewed May 17, 2023
View reviewed changes
besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java Outdated Show resolved Hide resolved
besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java Outdated Show resolved Hide resolved
...tion/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java Outdated
@@ -331,28 +341,50 @@ private TransactionSelectionResult evaluateTransaction(
processableBlockHeader,
transaction,
miningBeneficiary,
// externalTransactionSelector.getTracer(),
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a related TODO here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

...tion/src/main/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelector.java Outdated
transactionReceiptFactory.create(
transaction.getType(), effectiveResult, worldState, cumulativeGasUsed);

TransactionSelectionResult transactionSelectionResult =
Copy link
Contributor

Choose a reason for hiding this comment

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

final

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java Outdated Show resolved Hide resolved
plugin-api/src/main/java/org/hyperledger/besu/plugin/services/TransactionSelectionService.java Outdated Show resolved Hide resolved
besu/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java Outdated Show resolved Hide resolved
pinges and others added 10 commits May 18, 2023 13:11
@pinges
merge main
0a24c50 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges @macfarla
Update besu/src/main/java/org/hyperledger/besu/services/TransactionSe…
b856a76 
…lectionServiceImpl.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
@pinges @macfarla
Update besu/src/main/java/org/hyperledger/besu/services/TransactionSe…
bd7495d 
…lectionServiceImpl.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
@pinges @macfarla
Update plugin-api/src/main/java/org/hyperledger/besu/plugin/data/Tran…
4040bca 
…sactionSelectionResult.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
@pinges @macfarla
Update plugin-api/src/main/java/org/hyperledger/besu/plugin/services/…
57f128a 
…TransactionSelectionService.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
@pinges @macfarla
Update besu/src/main/java/org/hyperledger/besu/services/TransactionSe…
372c3cb 
…lectionServiceImpl.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
@pinges
Merge branch 'main' into AddTransactionEvaluator
47f1295
@pinges
some comments were missing and fix a test
4e41055 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges
fix unit test
befc74a 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges
spotless
41949e3 
Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges pinges marked this pull request as ready for review May 22, 2023 23:35
macfarla
macfarla approved these changes May 23, 2023
View reviewed changes
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. do we want a changelog?

...s/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java
final BesuPluginContextImpl besuPluginContext) {
final Optional<TransactionSelectionService> txSelectionService =
besuPluginContext.getService(TransactionSelectionService.class);
return txSelectionService.isPresent() ? txSelectionService.get().get() : Optional.empty();
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to suggest txSelectionService.orElse() but you have an extra get() in there

@pinges pinges enabled auto-merge (squash) May 23, 2023 03:09
@pinges pinges merged commit 65bcc55 into hyperledger:main May 23, 2023
@pinges pinges deleted the AddTransactionEvaluator branch May 23, 2023 03:37
siladu added a commit to siladu/besu that referenced this pull request May 25, 2023
@siladu
Revert "Add plugin API to select Transactions (hyperledger#5396)"
c2005df 
This reverts commit 65bcc55.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
siladu added a commit that referenced this pull request May 26, 2023
@siladu
Revert "Add plugin API to select Transactions (#5396)" (#5499)
a0c6052 
* Revert "Add plugin API to select Transactions (#5396)"

This reverts commit 65bcc55.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
pinges added a commit to pinges/besu that referenced this pull request May 29, 2023
@pinges
Revert "Revert "Add plugin API to select Transactions (hyperledger#5396…
85ebace 
…)" (hyperledger#5499)"

This reverts commit a0c6052.
@pinges pinges mentioned this pull request May 29, 2023
Merged
pinges added a commit to pinges/besu that referenced this pull request May 29, 2023
@pinges
Revert "Revert "Add plugin API to select Transactions (hyperledger#5396…
9a4af0a 
…)" (hyperledger#5499)"

This reverts commit a0c6052.

Signed-off-by: Stefan <stefan.pingel@consensys.net>
pinges added a commit that referenced this pull request Jun 2, 2023
@pinges @siladu @usmansaleem
Revert the revert of the tx selection commit (#5507)
b97c935 
* Revert "Revert "Add plugin API to select Transactions (#5396)" (#5499)"

This reverts commit a0c6052.

* fix receipt root bug

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Usman Saleem <usman@usmans.info>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@pinges @macfarla @elenduuche
Add plugin API to select Transactions (hyperledger#5396)
ddbb9f0 
This API fulfils the basic requirements, but will probably be extended in the near future. 

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@siladu @elenduuche
Revert "Add plugin API to select Transactions (hyperledger#5396)" (hy…
29e9211 
…perledger#5499)

* Revert "Add plugin API to select Transactions (hyperledger#5396)"

This reverts commit 65bcc55.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
@pinges @siladu
@usmansaleem @elenduuche
Revert the revert of the tx selection commit (hyperledger#5507)
4cb4720 
* Revert "Revert "Add plugin API to select Transactions (hyperledger#5396)" (hyperledger#5499)"

This reverts commit a0c6052.

* fix receipt root bug

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Usman Saleem <usman@usmans.info>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
@pinges @macfarla @eum602
Add plugin API to select Transactions (hyperledger#5396)
7bf8c48 
This API fulfils the basic requirements, but will probably be extended in the near future. 

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
@siladu @eum602
Revert "Add plugin API to select Transactions (hyperledger#5396)" (hy…
6ce201e 
…perledger#5499)

* Revert "Add plugin API to select Transactions (hyperledger#5396)"

This reverts commit 65bcc55.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
@pinges @siladu
@usmansaleem @eum602
Revert the revert of the tx selection commit (hyperledger#5507)
df54912 
* Revert "Revert "Add plugin API to select Transactions (hyperledger#5396)" (hyperledger#5499)"

This reverts commit a0c6052.

* fix receipt root bug

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Usman Saleem <usman@usmans.info>
Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Co-authored-by: Simon Dudley <simon.dudley@consensys.net>
Co-authored-by: Usman Saleem <usman@usmans.info>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gabriel-Trintinalia Gabriel-Trintinalia Gabriel-Trintinalia left review comments

@macfarla macfarla macfarla approved these changes

Assignees
No one assigned
Labels
plugins TeamRevenant GH issues worked on by Revenant Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pinges @macfarla @Gabriel-Trintinalia