Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add new RPC method to get the chain type #5576

Merged
merged 5 commits into from
Apr 8, 2020
Merged

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Apr 8, 2020

This adds a new RPC method to get the chain type of the running chain.
The chain type needs to be specified in the chain spec. This should make
it easier for tools/UI to display extra information without needing to
rely on parsing the chain name.

Let's keep @jacogr sane :)

This adds a new RPC method to get the chain type of the running chain.
The chain type needs to be specified in the chain spec. This should make
it easier for tools/UI to display extra information without needing to
rely on parsing the chain name.
@bkchr bkchr added A0-please_review Pull request needs code review. B1-clientnoteworthy labels Apr 8, 2020
@bkchr bkchr requested a review from tomusdrw as a code owner April 8, 2020 10:17
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to get rid sc dependency in rpc-api. Actually I think we should add a rule to the rule enforcement script to prevent bringing non-primitive dependencies there.

client/rpc-api/src/system/mod.rs Outdated Show resolved Hide resolved
fn system_type_works() {
assert_eq!(
api(None).system_type().unwrap(),
Default::default(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer something more explicit, but can live with that :)

@@ -21,6 +21,7 @@ parking_lot = "0.10.0"
sp-core = { version = "2.0.0-alpha.5", path = "../../primitives/core" }
sp-version = { version = "2.0.0-alpha.5", path = "../../primitives/version" }
sp-runtime = { path = "../../primitives/runtime" , version = "2.0.0-alpha.5"}
sc-chain-spec = { path = "../chain-spec" , version = "2.0.0-alpha.5"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpc-api should not depend on client stuff. The whole point of this crate is to avoid brining in 90% of substrate when just needing the APIs. Can we move the type to some primitives?

bkchr and others added 2 commits April 8, 2020 12:25
Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
@bkchr bkchr requested a review from tomusdrw April 8, 2020 10:52
Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, small grumbles.

@@ -30,6 +30,7 @@ sp-state-machine = { version = "0.8.0-alpha.5", path = "../../primitives/state-m
sc-executor = { version = "0.8.0-alpha.5", path = "../executor" }
sc-block-builder = { version = "0.8.0-alpha.5", path = "../../client/block-builder" }
sc-keystore = { version = "2.0.0-alpha.5", path = "../keystore" }
sc-chain-spec = { version = "2.0.0-alpha.5", path = "../chain-spec" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be sp-chain-spec now it seems.

pub use self::gen_client::Client as SystemClient;
use sp_chain_spec::Properties;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth to either consistently import both Properties and ChainType or use full path in both places.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants