feat(sdk, node-builder): add BuilderInternals trait #14686
Conversation
fgimenez
changed the title
fgimenez
changed the title
There was a problem hiding this comment.
the internals trait makes sense if we can erase a bunch of additional generics,
but this currently cascades, because we no longer use any (internal) adapter types to deconstruct the builder state.
I think one issue here is that NodeBuilderWithComponents directly wraps T and not some (internal) helper struct that is generic over T.
I need to experiment with this a bit
| @@ -165,7 +165,9 @@ where | |||
| let NodeHandle { node, node_exit_future: _ } = NodeBuilder::new(node_config.clone()) | |||
| .testing_node(exec.clone()) | |||
| .with_types_and_provider::<N, BlockchainProvider<_>>() | |||
| .with_components(node.components_builder()) | |||
| .with_components::<BuilderComponentsAdapter<FullNodeTypesAdapter<_, _, _>, _>>( | |||
There was a problem hiding this comment.
this we don't want because this leaks into user code, forcing the user to provide this adapter mapping
| for EngineNodeLauncher | ||
| where |
There was a problem hiding this comment.
hmm, I see because we removed the adapter type here this requires more bloat now?
| pub components_builder: CB, | ||
| /// Additional node extensions. | ||
| pub add_ons: AddOns<NodeAdapter<T, CB::Components>, AO>, | ||
| pub adapter: T, |
There was a problem hiding this comment.
I think what we should try here is introducing an actual Adapter struct on this level, which I should would allow us to hide most of this, this is like nodeadapter but over internals.
then this maybe doesn't cascade
| @@ -65,36 +67,42 @@ impl EngineNodeLauncher { | |||
| } | |||
| } | |||
|
|
|||
| impl<Types, DB, T, CB, AO> LaunchNode<NodeBuilderWithComponents<T, CB, AO>> for EngineNodeLauncher | |||
| impl<T> LaunchNode<NodeBuilderWithComponents<T>> | |||
There was a problem hiding this comment.
this looks a loot better I think
fgimenez
force-pushed
the
fgimenez/builder-internals
branch
from
d8e9603 to
d0c05fa
Compare
fgimenez
force-pushed
the
fgimenez/builder-internals
branch
from
d0c05fa to
fd54886
Compare
Closes #14592