traitify client

[gnunicorn]

the Client from sc-client is a massive pull-it-all-together within the outer substrate infrastructure. While many features already depend on specific traits rather than the client directly, the client still has a few functions exclusive to it, that some other crates depend on, but some also depend on client without actually needing these functions. The goal is to make nothing in sc-* depend on client directly anymore and move client into the internals of sc-service.

The way forward is probably (in this order):

• remove Client<..> dependencies where other traits are defining the use API well enough already
• create proper traits for remaining functionality in Client in sc-api
• replace remaining Client-dependencies with dependencies on sc-api
• move Client into sc-service, make it internal–this probably needs some tweaking for tests (see test-client)
• drop sc-client

This is an evolved refactor that probably has to touch a lot of (internal) code and where-clauses. Rust experience is required. Expect this to take a while. I am happy to mentor.

[seunlanlege]

@gnunicorn Interested in picking this up.

[arkpar]

drop sc-client

I'd suggest leaving it in a separate crate still. Client currently is basically an abstraction over the blockchain database that can handle block import. While service is supposed to be a full-blown substrate node with networking, RPC, etc. which utilises Client. I can see Client being potentially useful for bridgers, or tools that only need database access. So it is important to keep the API boundary here. I believe some of the CLI commands don't actually create the Service instance, just the Client, because all they require is database access.

[gnunicorn]

I believe some of the CLI commands don't actually create the Service instance, just the Client, because all they require is database access.

Nope. client-cli only depends on sc-client-api and sc-service. node-cli does depend on it and node-template, but only for the LongestChain-Selector and node-cli only in interaction with service, no direct interaction.

I see your point, though – arguably – that boundary is less clear cut as you make it out to be, client itself is already a full-blown thing, with telemetry and link to consensus, which I wouldn't expect to be living there if the boundary was as you described it to be. If we wanted to make that inner object accessible, which is currently practically impossible to set up without a service, we should define that boundary more clearly and trim down what that object should contain – and whether it shouldn't instead be a set of traits that we have alternative implementations for different use cases for.

Either way, this isn't a short-term goal and is mainly to prevent external parties to directly bind to client::Client rather than use the traits they should be bound to. I am happy to revisit this when it becomes more realistic to do and we have moved further in the actual code for bridges and other potential applications.

[arkpar]

I meant API usage, and not crate dependencies. Commands like export-blocks don't create a service instance, but user Client API directly. And sc-service simply re-exports a bunch of stuff from sc-client

I see your point, though – arguably – that boundary is less clear cut as you make it out to be, client itself is already a full-blown thing, with telemetry and link to consensus,

Yes, and that's unfortunate. I'd rather make telemetry just another subscriber to client notifications, rather than bake it in. Consensus APIs, such as sp_consensus::BlockImport should also rather be client APIs, defined in sc-client-api and not in sp-consensus

Either way, this isn't a short-term goal and is mainly to prevent external parties to directly bind to client::Client rather than use the traits they should be bound to. I am happy to revisit this when it becomes more realistic to do and we have moved further in the actual code for bridges and other potential applications.

👍