feat: NodeTypesWithDB

[klkvr]

This PR extracts DB AT from FullNodeTypes into separate trait NodeTypesWithDB. This trait is moved to new crate reth-node-types along with NodePrimitives, NodeTypes and NodeTypesWithEngine This is needed to allow reth-provider depend on NodeTypesWithDB

FullProvider is now generic over NodeTypesWithDB and FullNodeTypes is changed to the following form:

reth/crates/node/api/src/node.rs

Lines 95 to 100 in 416fc6b

	pub trait FullNodeTypes: Send + Sync + Unpin + 'static {
	/// Node's types with the database.
	type Types: NodeTypesWithDB;
	/// The provider type used to interact with the node.
	type Provider: FullProvider<Self::Types>;
	}

As it no more depends on NodeTypes I had to change the way we access some of the ATs (Node::Engine -> <Node::Types as NodeTypesWithEngine>::Engine)

This should allow usage of NodeTypesWithDB in provider traits

[emhane]

why not use syntax?

trait FullNodeTypes: NodeTypes<Engine: EngineTypes, DB: FullDatabase> { .. }

[klkvr]

why not use syntax?

trait FullNodeTypes: NodeTypes<Engine: EngineTypes, DB: FullDatabase> { .. }

this would leave us with something like

trait FullNodeTypes: NodeTypes<Engine: EngineTypes, DB: FullDatabase> { 
    type Provider = FullProvider<Self>;
}

and such structure would make it harder to deal with generics on providers, eg expressing this bound becomes not trivial if BlockchainProvider has NodeTypes generic:

reth/crates/node/builder/src/launch/mod.rs

Line 106 in 25997b4

T: FullNodeTypes<Provider = BlockchainProvider<Types::DB>, Types = Types>,

[emhane]

why not use syntax?

trait FullNodeTypes: NodeTypes<Engine: EngineTypes, DB: FullDatabase> { .. }

this would leave us with something like

trait FullNodeTypes: NodeTypes<Engine: EngineTypes, DB: FullDatabase> { 
    type Provider = FullProvider<Self>;
}

and such structure would make it harder to deal with generics on providers, eg expressing this bound becomes not trivial if BlockchainProvider has NodeTypes generic:

reth/crates/node/builder/src/launch/mod.rs

Line 106 in 25997b4

T: FullNodeTypes<Provider = BlockchainProvider<Types::DB>, Types = Types>,

ok I see, then why have these traits got associated types? and not just super traits? instead of NodeTypesWithDB just

trait FullDB: DatabaseMetrics + Database + NodeTypes {}

[klkvr]

ok I see, then why have these traits got associated types? and not just super traits? instead of NodeTypesWithDB just

trait FullDB: DatabaseMetrics + Database + NodeTypes {}

hmm, what's the benefit of such trait? doesn't feel right that we'd impl NodeTypes on DB instead of layering it on top of other types as another AT

[emhane]

ok I see, then why have these traits got associated types? and not just super traits? instead of NodeTypesWithDB just

trait FullDB: DatabaseMetrics + Database + NodeTypes {}

hmm, what's the benefit of such trait? doesn't feel right that we'd impl NodeTypes on DB instead of layering it on top of other types as another AT

doesn't feel right to have a trait with one associated type. what are the reasons for this?

it's a shame that we loose the nice aggregation of stateless components that we used to have in NodeTypes. think we may be getting an abstraction layer too much if we unite them again in a new aggregation trait. would have been simpler with removing the primitives associated type.

trait NodeTypes: PrimitiveTypes {
    type Engine: EngineTypes;
    type ChainSpec: ChainsSpec;
}

nonetheless, we can see where this ends up, and if we need to deal with debt later. it is beneficial that we are agiley moving forward.

[klkvr]

doesn't feel right to have a trait with one associated type. what are the reasons for this?

we've added NodeTypesWithEngine to close #9555, and the motivation for NodeTypesWithDB is that NodeTypes are coming from the user and need to be configured separately (this is primary motivation for the FullNodeTypesAdapter):

reth/crates/node/builder/src/builder/mod.rs

Lines 209 to 212 in 2c28438

	pub fn with_types<T>(self) -> NodeBuilderWithTypes<RethFullAdapter<DB, T>>
	where
	T: NodeTypesWithEngine<ChainSpec = ChainSpec>,
	{

it's a shame that we loose the nice aggregation of stateless components that we used to have in NodeTypes. think we may be getting an abstraction layer too much if we unite them again in a new aggregation trait. would have been simpler with removing the primitives associated type.

agreed that we might end up with too many layers. helper traits aggregating types should help, we could also consider restructuring NodeBuilder a bit to not depend on its interface, especially given it already uses adapters

[mattsse]

we could also consider restructuring NodeBuilder a bit to not depend on its interface, especially given it already uses adapters

fully supportive of this

[mattsse]

cool,

this will be slightly messy for a bit and some parts will change often but we need incremental progress here

[mattsse]

for nodebuilder I think we should perhaps try to stack trait types, like T: NodeTypesWithDB + EngineTypes and consider getting rid of

enginetypes: NodeTypes

but for now this seems appropriate