Split up solana-client

[CriesofCarrots]

Problem

The solana-client crate has become a kitchen sink of a lot of only loosely related apis. The result is that a user that wants to use a particular client has to pull in all the dependencies for all client types and utilities in the crate. This can be hard to manage in a large, complex monorepo.

Summary of Changes

Split up solana-client into various new client crates with slimmed down dependencies. This is a noop, reorg only, with re-exports in solana-client for backward compatibility.
While commits are intended to be sensibly distinct, they don't compile until Add client-common crate to avoid circular dependencies

Open to bike shedding on crate names, and opinions on organization.

Potential follow-up work before any backports:

• Replace all monorepo imports of solana-client with specific client crate
• Rework ClientErrorKind so that the common crate doesn't depend on reqwest
• Rework modules in solana-tpu-client so that quic- and udp-clients can be split to their own modules
• Move with_spinner methods behind a feature, making indicatif dependency optional
• Consider moving mock_sender to its own crate

[CriesofCarrots]

@t-nelson , can I please get your opinions on nonce-client? Do the name and contents make sense?

@pgarg66 , can you take a quick peek at the collection in tpu-client? I'd still like to maybe split out udp and quic client, but if I don't get there, what do you think? Does the crate name make sense?

[mvines]

Is the idea that a particular client would now be able to only use solana-client-common?

It sucks to see spinner.rs in solana-client-common though. If that could be moved elsewhere then solana-client-common is looking pretty close to something like a solana-rpc-client-api

[CriesofCarrots]

Is the idea that a particular client would now be able to only use solana-client-common?

Yes, that's the idea. rpc-client/Cargo.toml is probably the best example of the outcome of these changes.

It sucks to see spinner.rs in solana-client-common though. If that could be moved elsewhere then solana-client-common is looking pretty close to something like a solana-rpc-client-api

Good point! Looks like I can split that file out to other places, and rename solana-client-common

[CriesofCarrots]

I can split that file out to other places, and rename solana-client-common

@mvines , done. Wdyt?

[mvines]

I can split that file out to other places, and rename solana-client-common

@mvines , done. Wdyt?

Looking better. I took the opportunity to nit some more!

[t-nelson]

looks great! i nitted it up. some can likely be done in followups, just need to reach consensus on nonce client rename (or not!)

[t-nelson]

thanks!

Pull request has been modified.

[brson]

It's a good refactoring that does make it easier to understand the division of responsibilities, and does reduce some of the dependencies needed for those that don't need tpu-client in particular.

Comparing the API surface of solana-client as it is on docs.rs and this I see these modules missing from the refactored crate:

• mock_sender_for_cli
• rpc_cache
• spinner

These will be breaking changes. I don't know if that's a concern.

With the way the module structure is outlined within the solana_client crate - the module hierarchy is reproduced instead of reexporting every top-level module - all of those module docs are lost when viewing solana_client. This affects

• nonce_utils
• quic_client
• rpc_client
• rpc_custom_error
• rpc_sender
• thin_client
• upd_client
• most of the modules within nonblocking

This pattern of outlining the top-level modules within solona_client will require the module docs to be duplicated. I don't know if there are advantages that outweigh that disadvantage. I am happy to go in and touch up these docs myself later though if they are going to be blanked out by this PR.

ClientErrorKind lost the FaucetError variant, a breaking change.

[CriesofCarrots]

Thank you, @brson !

Comparing the API surface of solana-client as it is on docs.rs and this I see these modules missing from the refactored crate:

* mock_sender_for_cli
* rpc_cache
* spinner

I think I'm comfortable with the rpc_cache breakage, since solana-rpc is the only realistic caller. And the spinner apis were only pub for the solana-client crate previously. I'll restore mock_sender_for_cli, though. (edit) ✅

This pattern of outlining the top-level modules within solona_client will require the module docs to be duplicated. I don't know if there are advantages that outweigh that disadvantage. I am happy to go in and touch up these docs myself later though if they are going to be blanked out by this PR.

Ah, yeah, I did have the module docs duplicated, before I stripped out all those module files in c2c8635. I can restore those to the lib file. (edit) done in 5dc85fa

ClientErrorKind lost the FaucetError variant, a breaking change.

Correct, but I'm comfortable with this breakage as well, since there is no longer any path in our repo that utilizes this variant.

[kevinheavey]

Are these new crates going to be released on crates.io?

[CriesofCarrots]

Are these new crates going to be released on crates.io?

Yes. These changes are only in master so far, but once v1.15 moves to beta (testnet), the crates will be published. Or if we decide to backport these changes to the current beta branch in the meantime, they would also be published.