Make clients clonable

[slumber]

It's a widespread pattern to implement Clone for clients, jsonrpc, reqwest, actix_web, etc. Makes it easier to pass the client around.

There's another strong argument towards this: if you wrap your client into Arc, you lose all the generated traits implementations from jsonrpsee proc macros.

[niklasad1]

It's a widespread pattern to implement Clone for clients, jsonrpc, reqwest, actix_web, etc. Makes it easier to pass the client around.

We have been reasoned about it as follows: #243 (comment), to make it explicit for users one client (WS) == one background task then users can share it by a Mutex, Arc, or similar and have to deal with this logic because we didn't want to have complicated logic such as restarting. If this is super annoying I guess we could change it but not a restart API.

There's another strong argument towards this: if you wrap your client into Arc, you lose all the generated traits implementations from jsonrpsee proc macros.

You could use Arc::Deref and it should work AFAIK such as:

RpcClient::subscribe_storage(&*client, None).await.unwrap();?

[slumber]

We have been reasoned about it as follows: #243 (comment), to make it explicit for users one client (WS) == one background task then users can share it by a Mutex, Arc, or similar and have to deal with this logic because we didn't want to have complicated logic such as restarting. If this is super annoying I guess we could change it but not a restart API.

Totally makes sense for websocket client, but it's different in nature from http, there's nothing wrong with cloning http client since we can think of it as being stateless. Also reqwest actually has a pool of connections which makes it even better.

You could use Arc::Deref and it should work AFAIK such as:

RpcClient::subscribe_storage(&*client, None).await.unwrap();?

I mean suppose you have a struct

struct MyClientWrapper<C: MyRpcClient + Clone>  {
     client: C,
}

You can't pass Arc<HttpClient> because it doesn't automatically derive the trait.
And the client itself doesn't implement Clone (also suppose I don't care about WS client because it's simply not my usecase).
The only option is to require Sync and pass the reference around, but this feels weird and restricts you with lifetimes.

I see your point, feel free to close the issue, but I would consider the opportunity of deviation from "unified interface" path for the above reasons.

[dvdplm]

I see your point, feel free to close the issue, but I would consider the opportunity of deviation from "unified interface" path for the above reasons.

FWIW we have deviated from the "unified interface" dogma at several junctions in the past. It inevitably leads to some head scratching at times when interfaces are different between the transports, but otoh there are things that simply make sense for one transport and not at all for another. We will discuss this.

[niklasad1]

Yupp, I agree that WS and HTTP are fundamentally different so I think we could diverge from that in this case.

Furthermore, the implementation/behavior is already different between the two thus I see no problem by having Clone for just the HTTP client

[slumber]

Closed via #583