Add Runtime Api support #703
Conversation
haerdib
force-pushed
the
bh/add-runtime-api
branch
2 times, most recently
from
4c32fc0
to
c359ac4
Compare
|
I'm wondering if there is a way to check, if we are still up-to-date. Do you know if there is a file with the whole runtime api, from which we could generate tests? And if the generated test-file doesn't compile, we know something is wrong (?) |
|
We are actually doing that with our system tests in the testing crate. CI runs the newest substrate kitchensink node (https://github.com/scs/substrate-api-client/blob/master/.github/workflows/ci.yml#L176-L178) and runs all our function against the node (see https://github.com/scs/substrate-api-client/pull/703/files#diff-0cf9c44c6a61eeed79634c6ff344d8d096de895c5f48add5ce11a2ae343f2f71). So long as we run our CI from time to time, we will know if we're up-to-date or not. There's probably also a way to do it statically with the metadata - retrieve the metadata somehow from an up-to-date kitchensink node (not sure if Parity provide something like that or if we'd need to compile it ourselves) and use it to compare our functions. But I'm not sure what we'd gain from that compared to the already existing system tests. |
|
The current tests will catch if an existing call changes its interface. But it will not notice, when there are new calls we should add. Generally it would be cleaner to have all of this automated. If the code for the runtime api was generated. Subxt is doing that (as far as I understand). On the gains: automating these things is less error prone and reduces work for us in the long run. |
|
Ah, I see. Yes, subxt generates the api code itself from the metadata. I suppose we could do the same for a completeness check. But that would be a separate issue, I believe. Because if we decide to implement it, I'd not only do it for the runtime api but also for the rpc. |
There was a problem hiding this comment.
I haven't checked the complete code. But I think the 2 remarks I wrote should be fixed everywhere, in case there are more places. Generally: have one set of files with the 1:1 official runtime api interfaces, with the exact same names. And then (if it makes sense) add additional functions in separate files in a separate folder.
The good thing is: we can still add an automation at a later time then.
| &self, | ||
| extrinsic: Vec<u8>, | ||
| at_block: Option<Self::Hash>, | ||
| ) -> Result<Self::ApplyExtrinsicResult>; |
There was a problem hiding this comment.
You added a function here, that is not in the official runtime api.
I would keep the files, so that they are 1:1 the runtime api. Also with the exact name from the runtime api. Else it's confusing. If you want to add convenient functions, put them in a separate file in a separate folder. So, we know, which once were written additionally and which ones are plain forwards to the api.
There was a problem hiding this comment.
We already have the same pattern in the rpc-api, offering strongly typed and weak typed (opaque/encoded) functions where necessary (mostly regarding UncheckedExtrinsic). The reasoning behind that is, while with the opaque method the user has the full freedom of which (encoded) extrinsic to apply, but users that do not need this freedom should use the normal, strongly typed method to get errors during compilation.
And I disagree that the functions should match 1:1 to the interface of the Substrate one, because the api-client should provide convenience functions for the users. As long as it stays clear which Api is addressed in the end, why not offer more functionality?
There was a problem hiding this comment.
I'm not against extra functionality. But it'd be nice to separate the code for the two and have a naming scheme that doesn't give new names to standard substrate functions.
I agree. Also see my comment above. I think with these adjustments I propose there, we'd be well prepared for automating things at a later time. |
haerdib
force-pushed
the
bh/add-runtime-api
branch
from
94dbf48
to
c2005e5
Compare
fix clippy add call_raw and automatic decoding add missing await use vec instead of option readd Option once again add some runtime api calls and first test add authority test fix build fix no-std build fix build fix result return values remove unnecessary result return add finalize block add core runtime api fix build add RutimeApiClient for clear distinguishion add transaction, staking , mmr and session_keys api fix build fix build fix clippy fix naming of session keys function add mmr tests add session key tests fix no-std error by defining types by self for now add sakintapi test and fix fix build fix tets update tests add runtime api example update README add example of self creation of call add metadata decoding add list functions add some nice printing fix build remove mmr fix async build update jsonrspee to v21 (#707)
haerdib
force-pushed
the
bh/add-runtime-api
branch
from
c2005e5
to
4dd039f
Compare
Adds general runtime api support via rpc call
state_call:closes #520