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 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. |
|
good to hear the |
|
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
|
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> { |
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 { |
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>; |
There was a problem hiding this comment.
this file looks very boilerplatey given there are two instances of the same API with minor differences...
|
some questions. other than those concerns it looks good. |
|
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
|
@gavofyork resolved the merge conflict. |
gavofyork
approved these changes
Nov 13, 2018
:100644 100644 898aadc7 49217199 M Cargo.lock :100644 100644 61570436 465ed664 M core/client/src/backend.rs :100644 100644 5d0c886b 64d710fd M core/client/src/block_builder.rs :100644 100644 c447855e 5ecbe474 M core/client/src/client.rs :100644 100644 139cef13 f90dbf3d M core/client/src/error.rs :100644 100644 2800c503 3298e66a M core/client/src/runtime_api.rs :100644 100644 affa1c5c 809b08bc M core/primitives/src/lib.rs :100644 100644 2877dfa9 d5547413 M core/sr-api/Cargo.toml :100644 100644 9a49784d 6a625a03 M core/sr-api/src/lib.rs :100644 100644 7c28e1c7 a1a444a9 M core/sr-primitives/src/traits.rs :100644 100644 2e113ab6 dcc01a6d M srml/metadata/Cargo.toml :100644 100644 ea722a70 0809531a 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