chore(rpc): build rpc as OnComponentsInitializedHook
#9243
Conversation
…to matt/scaffold-ethapi
Co-authored-by: Matthias Seitz <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
emhane
requested review from
mattsse,
Rjected,
onbjerg and
gakonst
as code owners
emhane
changed the title
OnComponentsInitializedHook
| // fn beacon_engine_handle(&self) -> &Self::Node::Engine::Handle; | ||
| fn beacon_engine_handle(&self) -> &BeaconConsensusEngineHandle<Self::Node::Engine>; |
There was a problem hiding this comment.
will just move this getter into RpcBuilder trait, and same for other dependencies getters, and delete these Install traits
crates/node/api/src/node.rs
Outdated
| pub trait FullNodeComponentsExt: FullNodeComponents { | ||
| type Rpc: Rpc<Self>; | ||
|
|
||
| /// Returns the RPC component, if installed. | ||
| fn rpc(&self) -> Option<&Self::Rpc>; | ||
| } | ||
|
|
||
| pub trait Rpc<N: FullNodeComponents>: Send + Sync { | ||
| type ServerHandles; | ||
| type Registry; | ||
|
|
||
| fn handles(&self) -> &Self::ServerHandles; | ||
|
|
||
| fn registry(&self) -> &Self::Registry; | ||
| } |
There was a problem hiding this comment.
I think RPC is downstream of components and part of the Node / Launch process.
so I think we want:
- keep the rpc extension API on nodebuilder
- make rpc part of the Node helper trait, to embed it into
DefaultLauncher
but somehow we'd need to a way to clarify the EthApi type for the nodehooks I assume?
maybe a solution that could work is is replacing the RpcHooks type with the RpcBuilder
There was a problem hiding this comment.
keep the rpc extension API on nodebuilder
make new trait NodeComponentsBuilderExt, this is where the rpc builder will be stored, yes?
make rpc part of the Node helper trait, to embed it into DefaultLauncher
node helper trait? I can't find it, need the proper name
maybe a solution that could work is is replacing the RpcHooks type with the RpcBuilder
configuring rpc hooks as part of rpc builder makes sense. still need to have them in NodeAddOnsExt type also, to pass as param in OnComponentsInitializedHook. the hooks param to OnComponentsInitializedHook should be general. alt, have a super trait for all hooks on extension components.
in future we may want to stick to the pattern and start exex as OnComponentsInitializedHook as well. we may also want to separate engine rpc and regular rpc as two components, thereby also separate RpcHooks into RpcEngineHooks and RpcHooks. keeping this in mind, making the NodeAddOnsExt type.
There was a problem hiding this comment.
we may also want to separate engine rpc and regular rpc as two components
definitely, this behaviour should be entirely node specific, hence I'm thinking this should be a separate thing and perhaps replace
reth/crates/node/builder/src/rpc.rs
Line 35 in 38f2d00
and also be configured
reth/crates/node/builder/src/node.rs
Line 23 in 38f2d00
emhane
added
the
S-needs-design
|
closing, needs design |
Build RPC component as
OnComponentsInitializedHook. Part of task to exposeEthApitop level, for user to configure, like for different networks.Exposes extension component builders as
Option<OnComponentsInitializedHook<N>>s. The hook takes a mutable reference to the extension component container as param. This is how extension component builders stage their output for installation in the node, as well as how they read state from previously initialised components that they may need to build themselves. This has two main benefits:Engine<Node>is the only way to get toRpc<Engine<Node>>, unlike for core components.DefaultNodeLauncher::launch_node.