This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Implement runtime api client side directly in the runtime #1094
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gavofyork
reviewed
Nov 9, 2018
core/client/src/block_builder/mod.rs
Outdated
| //! Utility struct to build a block. | ||
| #[cfg(feature = "std")] | ||
| mod block_builder; |
gavofyork
reviewed
Nov 9, 2018
| pub use rstd::slice; | ||
| #[doc(hidden)] | ||
| pub use codec::{Encode, Decode}; | ||
| //! Macros for declaring and implementing the runtime API's. |
Member
|
good to hear the |
Member
|
would be nice to have an example of a new API being added and used from within |
bkchr
force-pushed
the
bkchr-refactor-runtime-api-v2
branch
from
5186859 to
4c02cf0
Compare
Member
Author
|
Fixed the grumbles and the merge conflict. :) |
gavofyork
reviewed
Nov 12, 2018
core/client/src/runtime_api/core.rs
Outdated
| pub mod runtime { | ||
| use super::*; | ||
|
|
||
| /// The `Core` api trait that is mandantory for each runtime. |
gavofyork
reviewed
Nov 12, 2018
|
|
||
| #[cfg(feature = "std")] | ||
| impl client::runtime_api::Core<GBlock> for ClientWithApi { | ||
| fn version(&self, at: &GBlockId) -> Result<RuntimeVersion, client::error::Error> { |
Member
There was a problem hiding this comment.
is this code due to be auto-generated?
gavofyork
reviewed
Nov 12, 2018
| } | ||
|
|
||
| #[cfg(feature = "std")] | ||
| impl client::runtime_api::Core<Block> for ClientWithApi { |
Member
There was a problem hiding this comment.
this code is very boilerplatey - is it due to be autogenerated?
gavofyork
reviewed
Nov 12, 2018
| #[cfg(feature = "std")] | ||
| pub trait Core<Block: BlockT>: 'static + Send + Sync + ConstructRuntimeApi<Block> + ApiExt { | ||
| /// Returns the version of the runtime. | ||
| fn version(&self, at: &BlockId<Block>) -> Result<RuntimeVersion>; |
Member
There was a problem hiding this comment.
this file looks very boilerplatey given there are two instances of the same API with minor differences...
Member
|
some questions. other than those concerns it looks good. |
Member
Author
|
Everything where |
gavofyork
added
A7-looksgoodcantmerge
and removed
A0-please_review
bkchr
force-pushed
the
bkchr-refactor-runtime-api-v2
branch
from
43d5781 to
696e416
Compare
Member
Author
|
@gavofyork resolved the merge conflict. |
gavofyork
approved these changes
Nov 13, 2018
:100644 100644 898aadc 49217199 M Cargo.lock :100644 100644 6157043 465ed664 M core/client/src/backend.rs :100644 100644 5d0c886 64d710fd M core/client/src/block_builder.rs :100644 100644 c447855e 5ecbe474 M core/client/src/client.rs :100644 100644 139cef1 f90dbf3d M core/client/src/error.rs :100644 100644 2800c50 3298e66 M core/client/src/runtime_api.rs :100644 100644 affa1c5 809b08b M core/primitives/src/lib.rs :100644 100644 2877dfa d554741 M core/sr-api/Cargo.toml :100644 100644 9a49784 6a625a0 M core/sr-api/src/lib.rs :100644 100644 7c28e1c a1a444a9 M core/sr-primitives/src/traits.rs :100644 100644 2e113ab dcc01a6 M srml/metadata/Cargo.toml :100644 100644 ea722a7 0809531 M srml/metadata/src/lib.rs
On `wasm` it basically just exports the runtime api stuff.
bkchr
force-pushed
the
bkchr-refactor-runtime-api-v2
branch
from
696e416 to
f3cca84
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull requests moves the implementation of the runtime api client side into the runtime as well. This makes the api safer, as the compiler now enforces the user to implement a certain api when he wants to use it. The compile time checks obviously only work for the native runtime interface.
The
Coreapi is now also enforced by the compiler by making each other api derive from it.This pull requests also moves the
sr-apicrate intosubstrate-clientand thus, the client is required to compile onwasmas well. Onwasmthe client only exposes the api stuff, the rest is disabled. I made this design decision to minimize the size of a where clause when using a runtime api to made the api more ergonomic. Otherwise, the user would needed to specify the error each time and that blowed up easily. I'm still not quite happy with this solution, maybe while reviewing this pull request, we can come to a better solution.In a follow up pull request, I will provide some sort of macro to auto implement much of the stuff that is now hand written code in the
test-runtimeandnode/runtime. So, all theClientWithApistuff should be gone with this follow up pr.Progress on: #1031