feat: declarative builder v2 #6447
Merged
Conversation
This file contains 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
mattsse
force-pushed
the
matt/declarative-node-builderv2
branch
from
30ac64e to
9511150
Compare
onbjerg
reviewed
Feb 13, 2024
Comment on lines
+29
to
+30
| pub fn components<Node>( | ||
| ) -> ComponentsBuilder<Node, EthereumPoolBuilder, EthereumPayloadBuilder, EthereumNetwork> |
Co-authored-by: Oliver Nordbjerg <[email protected]>
Co-authored-by: Oliver Nordbjerg <[email protected]>
Co-authored-by: Oliver Nordbjerg <[email protected]>
Co-authored-by: Oliver Nordbjerg <[email protected]>
Co-authored-by: Oliver Nordbjerg <[email protected]>
onbjerg
reviewed
Feb 13, 2024
|
general q: i can see that network, engine, payload builder, pool is all configured as components, but rpc seems separate, is this deliberate or will that config be unified later? |
RPC is based on the configured modules this can be extended with a hook. I think we can/want to move away from this as well but for now it's significantly easier to rely on the |
|
gotcha |
Rjected
approved these changes
Feb 13, 2024
crates/node-builder/src/rpc.rs
Outdated
Comment on lines
203
to
204
| // TODO simplify registry trait bounds | ||
| pub registry: &'a mut RpcRegistry<Node>, |
There was a problem hiding this comment.
trait bound is just Node: FullNodeComponents, so I'm guessing this todo is complete?
Comment on lines
+58
to
+60
| pub struct EthereumPoolBuilder { | ||
| // TODO add options for txpool args | ||
| } |
There was a problem hiding this comment.
wdyt would go in here eventually?
There was a problem hiding this comment.
additional values to override cli args
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.
supersedes #5932
Introduces a declarative node builder that configures the node step by step using types and components.
Future work
currently, type traits are just placeholders that can be configured once everything is generic over types. Ideally we start with the Engine types (ethereum/optimism) but this needs more work on the existing components first (make them generic over EngineConfig).
Update
This currently only adds the node-builder, I want to do the replacement in a followup. because the PR is already gigantic
TODO
Breaking changes
This phases out the NodeCommandConfig trait in it's existing form, instead all of its functionality will be moved into the NodeBuilder directly (hooks + helper functions for customizing payload builder for example)