Plugin API: pass pending block header when creating selectors#10034
Plugin API: pass pending block header when creating selectors#10034fab-10 wants to merge 1 commit intobesu-eth:mainfrom
Conversation
a992e32 to
ee23877
Compare
There was a problem hiding this comment.
Pull request overview
Updates the transaction selection plugin API to provide the pending/processable block header when creating plugin transaction selectors, enabling selector logic to depend on the block being built.
Changes:
- Extend
PluginTransactionSelectorFactory#create(...)to receive aProcessableBlockHeader. - Extend
TransactionSelectionService#createPluginTransactionSelector(...)to pass through the block header to factories. - Update call sites, tests, and the plugin-api API hash/changelog entry accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin-api/src/main/java/org/hyperledger/besu/plugin/services/txselection/PluginTransactionSelectorFactory.java | Changes factory creation method signature to include pending block header. |
| plugin-api/src/main/java/org/hyperledger/besu/plugin/services/TransactionSelectionService.java | Extends service method signature to accept and forward the block header. |
| plugin-api/build.gradle | Updates plugin-api ABI hash for the API change gate. |
| ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningConfiguration.java | Updates in-core default TransactionSelectionService implementation signature. |
| ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java | Updates mocks/usages for the new selector factory signature and passes block header in tests. |
| ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java | Passes processableBlockHeader into selector creation. |
| app/src/main/java/org/hyperledger/besu/services/TransactionSelectionServiceImpl.java | Forwards block header to all registered selector factories. |
| CHANGELOG.md | Adds changelog entry for the plugin API change. |
You can also share your feedback on Copilot code review. Take the survey.
| default PluginTransactionSelector create( | ||
| final ProcessableBlockHeader pendingBlockHeader, | ||
| final SelectorsStateManager selectorsStateManager) { | ||
| return PluginTransactionSelector.ACCEPT_ALL; | ||
| } |
There was a problem hiding this comment.
This is a source-breaking change for external plugin implementers because the previous create(SelectorsStateManager) signature was removed. To preserve backwards compatibility for plugins, consider reintroducing the old method as a deprecated default overload and add the new overload as well (with one delegating to the other), so existing plugins continue to compile while new plugins can use the block header.
| * @param processableBlockHeader the processable block header | ||
| * @param selectorsStateManager the selectors state manager | ||
| * @return the transaction selector plugin | ||
| */ | ||
| PluginTransactionSelector createPluginTransactionSelector( | ||
| final ProcessableBlockHeader processableBlockHeader, |
There was a problem hiding this comment.
Parameter naming is inconsistent across the API: PluginTransactionSelectorFactory uses pendingBlockHeader while TransactionSelectionService uses processableBlockHeader for the same type/purpose. Consider aligning the parameter name across both interfaces (and their Javadocs) to reduce ambiguity for plugin authors.
| * @param processableBlockHeader the processable block header | |
| * @param selectorsStateManager the selectors state manager | |
| * @return the transaction selector plugin | |
| */ | |
| PluginTransactionSelector createPluginTransactionSelector( | |
| final ProcessableBlockHeader processableBlockHeader, | |
| * @param pendingBlockHeader the pending block header | |
| * @param selectorsStateManager the selectors state manager | |
| * @return the transaction selector plugin | |
| */ | |
| PluginTransactionSelector createPluginTransactionSelector( | |
| final ProcessableBlockHeader pendingBlockHeader, |
f97a04a to
af70194
Compare
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
af70194 to
886b147
Compare
There was a problem hiding this comment.
Would this be an issue for any Linea or other downstream plugins -
Silent breaking change for existing plugins that implement create(SelectorsStateManager).
Before this PR, the interface had:
default PluginTransactionSelector create(SelectorsStateManager mgr)
After this PR, only the new two-parameter signature exists. Any deployed plugin that overrides the old single-parameter signature now has an orphaned method — it no longer overrides anything. Java won't warn about this at compile time (unless the plugin used @Override), and at runtime their custom selector will never be called — ACCEPT_ALL will be returned instead. This is a silent behavior regression for operators running such plugins.
Mitigation options:
- Keep the old signature as a default that delegates to the new one with a null or empty header
- Add a
@Deprecatedbridge in the interface - At minimum, document the migration path in the PR/changelog
PR description
This branch adds
ProcessableBlockHeaderas a parameter to the plugin transaction selector factory’s create method and toTransactionSelectionService.createPluginTransactionSelector. This allows plugins to access the pending block header at the time aselector is created — useful for block-aware filtering logic.
Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-requiredlabel to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew spotlessApply./gradlew build./gradlew acceptanceTest./gradlew integrationTest./gradlew ethereum:referenceTests:referenceTests