Remove `sp-api` requirement on compile time bounds

[bkchr]

sp-api or better known as runtime api currently requires on the node compile time bounds. This means when you want to use a certain runtime api, you need to ensure that you pass some type around that implements this runtime api. The idea behind this was that people are made aware at compile time which runtime apis will be required. The problem is that with the removal of the native runtime we can not implement the compile time check any more. This compile time check also had quite some disadvantages like introducing high compile times by making everything statically typed and requiring the compiler to resolve all types to ensure that the required api is implemented. As the check was at compile time, it also complicated shipping a node that supports a runtime api while not having the runtime api enabled yet in the runtime.

So, we should remove this requirement on compile time bound checking. While doing this we should also improve the runtime api usage. It was a big mistake to put the at as first parameter automatically to every runtime api function or generating these with_context functions. The proper solution for this is to create some builder pattern like approach for setting up a runtime api instance. This runtime api instance can then be used to call into the runtime for every possible runtime api.

Part of: https://github.com/paritytech/polkadot-sdk/issues/62

[rphmeier]

I actually think the compile-time generics are quite cool and allow clean expression of which runtime APIs are required in a function and helps name resolution in the compiler. The only issue is that the implementations are generated in impl_runtime_apis, not decl_runtime_apis, so the native runtime becomes a factor.

One implementation approach that I think would be pretty light-touch is this:

1. We still ask functions to use where T: ProvideRuntimeApi, T::Api: SomeApi syntax.
2. We alter decl_runtime_apis to produce a blanket implementation, something like

impl<T: GenericRuntimeApi> ThisApi for T {
   // The `GenericRuntimeApi` trait is just a thin wrapper around wasm, that deals with raw function names and `Vec`s
}

3. We add a blanket implementation to Client in client/service:

impl ProvideRuntimeApi for Client {
    type Api = SomeTypeImplementingGenericRuntimeApi;

    // ...
}

4. We remove the trait implementations from impl_runtime_apis and just have it generate wasm exports.

This approach would effectively implement all APIs for every client, and the detection would be done at runtime. It would also be fully backwards compatible with the way we've used runtime APIs in higher-level code so far.

[bkchr]

Thanks for the input, but what you are proposing only brings the advantage of seeing in the docs automatically what kind of runtime api is required. The downside of this is that it adds extra complexity which also slows down compilation. Expressing the required runtime api in docs can also be done manually by the dev. As it would be a blanket implementation, it would any way never appear at compile time that the runtime api is missing.

I have already some working branch where these compile time checks are removed. I also want to add very easy checking for a runtime api at runtime so that at initialization this can be used to bail out early and the inform people on what to do.

[rphmeier]

I think that is fair - it's also to have slightly less "magic" for the users and better readability of the code. I would like to have the API be something fairly explicit, where the person writing the code has to write

MyApi::some_function(client, at, params);

(or anything including the name of the API) rather than

client.some_function(at, params)

Though this is mostly stylistic, I believe a good API here should require some explicitness for the purpose of making code easier to read.

[koushiro]

@bkchr I see that you have introduced an new error UsingSameInstanceForDifferentBlocks in paritytech/substrate#14387 . According to the description of this issue, the runtime api design should be like the following, right?

let api = client.runtime_api(at);
api.some_function(params);

Is anyone currently working on this？

[bkchr]

Is anyone currently working on this？

Yeah, I'm working on this.

[atenjin]

yeah @koushiro and I are waiting for this, there is a bug in frontier eth rpc related with this issue. Though we can skip this part and fix that thing directly, I think if this change is finished and pick to polkadot-1.0.0 branch is more better for us and all ecology parters.

[rphmeier]

@bkchr - are you doing this in a way which breaks downstream typechecks? As far as I can tell, we do need some additional code in decl_runtime_apis which does a blanket implementation to avoid that.

And I still believe that

let api = client.runtime_api(at);
api.some_function(params);

is a bad API, and it should be more like

let api = client.runtime_api(at);
MyApi::some_function(api, params);

[bkchr]

let api = client.runtime_api(at);
MyApi::some_function(api, params);

Yeah that should be possible. I can see the advantage of being more verbose on declaring the trait.

yeah @koushiro and I are waiting for this, there is a bug in frontier eth rpc related with this issue. Though we can skip this part and fix that thing directly, I think if this change is finished and pick to polkadot-1.0.0 branch is more better for us and all ecology parters.

In the future it will not be possible to re-use a runtime api instance for calls to different blocks at compile time. Currently I added this runtime check that will throw the error you are mentioning, but you can just create a new instance of the api to work around this.