Feature/service manager in protocol context #8533
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
238fb8e to
b5244a8
Compare
bcd1ac8 to
c3285d6
Compare
siladu
approved these changes
Apr 9, 2025
| @@ -52,4 +54,22 @@ public interface ServiceManager { | |||
| * is unavailable | |||
| */ | |||
| <T extends BesuService> Optional<T> getService(Class<T> serviceType); | |||
|
|
|||
| /** A toy implementation of ServiceManager, suitable for test and mocks. */ | |||
There was a problem hiding this comment.
Could this live in the test packages instead?
There was a problem hiding this comment.
Protocol context builder is using it for the default/unset service manager. I might need to revisit that comment to reflect that
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
…y their concerns Signed-off-by: garyschulte <garyschulte@gmail.com>
… base class Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
c3285d6 to
2ca8869
Compare
jflo
approved these changes
Apr 9, 2025
| @@ -52,4 +54,22 @@ public interface ServiceManager { | |||
| * is unavailable | |||
| */ | |||
| <T extends BesuService> Optional<T> getService(Class<T> serviceType); | |||
|
|
|||
| /** A basic implementation of ServiceManager, suitable for tests. */ | |||
| class SimpleServiceManager implements ServiceManager { | |||
There was a problem hiding this comment.
personal pref: break this out to its own class, possibly in a test path.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR description
Adds plugin service manager to protocol context. The specific motivation is to have plugin services available to AbstractBlockProcessor (in a subsequent PR), but generally this is a logical and convenient way to surface plugin services in the besu codebase.
The scope of the code change is minimal, but there are a lot of test fixture changes. Also, the ProtocolContext constructor is now protected so we do not have to plumb a bunch of unnecessary items into it for unit tests which directly construct a test protocol context.
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