Support http endpoints

[chevdor]

I would be nice being able to connect via http as well :)

[trevor-crypto]

@clangenb continuing from the comments in #121...
I'd like to implement that Client trait, but it seems a little coupled with the Api because of the subscriber model, which wouldn't exist in an HTTP RPC.

fn's such as subscribe_events, subscribe_finalized_heads, etc should be moved out of Api and to WsRpc (new websocket rpc) ? Perhaps I can also try conditional compilation to include those functions, which will be necessary anyway for excluding the ws crate

[clangenb]

Ah yes, for http it would make sense to separate this.

I would go for a different approach, which I had planned for anyhow. I would like to separate the Api's methods as I have done it here in some extension traits. integritee-network/worker#268

Hence, we can extract the subscriber methods to a Subscriptions trait (maybe improve naming). If we go that path, I think it might make sense to feature gate http-client and ws-client, as most users will not need both anyhow. Subscriptions would be only available in with the ws-client feature.

[trevor-crypto]

@clangenb ok great. I will put up a PR soon and work through any feedback that you have.

I think it might make sense to feature gate http-client and ws-client

For my use case it's important that the http client is generic (so I can implement them in my own project with various backends), so I will try to feature-gate ws-client only if that's ok

[trevor-crypto]

Another thing I am trying to figure out is how to add the Client to the Api:

pub struct Api<P, C>
where
    P: Pair,
    MultiSignature: From<P::Signature>,
    C: RpcClient,
{
    pub url: String,
    pub signer: Option<P>,
    pub genesis_hash: Hash,
    pub metadata: Metadata,
    pub runtime_version: RuntimeVersion,
    client: C,
}

Doing it like this will change the signature of Api::new() which is sort of undesirable, since everyone using the library will need to update their code.
Please let me know if you have any ideas

[clangenb]

I think this will be unpreventable. Usually, you could work around this with the builder pattern, i.e.,

Api::new().with_client(client); but as the client is mandatory for the Api to be usable, this does not make sense for me. But for me, it is okay if we change the signature, we will just bump the version.

Just as a sidenode, something that I would do differently today: I would remove the constraints on P while initializing. If you only fetch storage you don't need a signer and for all these methods you could just have: Api<(), Client>. (But you don't need to change that if you don't want.)

[trevor-crypto]

@clangenb Thanks for the tips.

I would remove the constraints on P while initializing.

This is actually much better for my use case, so I have done that as well. I will try to get a draft pull request up, but need to finish some work first.

Side question: do you know specifics around why substrate-api-client won't work for Polkadot/Westend? I saw it on the README but I couldn't understand why.

[clangenb]

No hurry.

Side question: do you know specifics around why substrate-api-client won't work for Polkadot/Westend? I saw it on the README but I couldn't understand why.

Hmm, I did not know this at all. I actually believe this is an outdated statement.

@brenzi would probably know this.

[brenzi]

By now it should work if nailing cargo to the right commit on substrate. I think the remark is outdated

[trevor-crypto]

Just confirming, yes, it does work with the most recent Polkadot. Funnily enough, the official crate (substrate-subxt) isn't working with the latest version due to metadata being old!

[clangenb]

If you supply a custom client, we do support http requests already:

substrate-api-client/src/std/mod.rs

Line 57 in 66d9734

/// impl MyClient {

[lucgerrits]

So in the end, does the library support HTTP endpoints ?

[clangenb]

We do not provide an http client in this library yet, but you can provide your own client that implements the RPCClient, this may very well be an http-client.