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

rpc: Implement chainSpec RPC API #12261

Merged
merged 10 commits into from
Sep 20, 2022
Merged

rpc: Implement chainSpec RPC API #12261

merged 10 commits into from
Sep 20, 2022

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Sep 13, 2022

This PR lays the ground for the implementation of the RPC spec
and adds the chainSpec methods.

The RPC methods are placed in the rpc-spec crate subject to
change in the future.

The chainSpec methods include:

  • chainSpec_unstable_chainName - Get the chain name, as present in the chain specification.
  • chainSpec_unstable_genesisHash - Get the chain's genesis hash.
  • chainSpec_unstable_properties - Get the properties of the chain, as present in the chain specification.

Testing Done

  • Unit tests are added for each methods
  • CURL requests submitted to the local node
$ curl -H "Content-Type: application/json" -d '{"id":1, "jsonrpc":"2.0", "method": "chainSpec_unstable_properties"}' http://localhost:9933/
  {"jsonrpc":"2.0","result":{},"id":1}

curl -H "Content-Type: application/json" -d '{"id":1, "jsonrpc":"2.0", "method": "chainSpec_unstable_chainName"}' http://localhost:9933/
  {"jsonrpc":"2.0","result":"Development","id":1}

 curl -H "Content-Type: application/json" -d '{"id":1, "jsonrpc":"2.0", "method": "chainSpec_unstable_genesisHash"}' http://localhost:9933/
{"jsonrpc":"2.0","result":"0x7d341d884f3bea9b5668d29be58ac5e307d57122a7df21021a2099ffdd609928","id":1}

First part of #12071 .

@lexnv lexnv added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. labels Sep 13, 2022
@lexnv lexnv self-assigned this Sep 13, 2022
@lexnv lexnv force-pushed the lexnv/rpc_chain_spec branch from be4c5bc to fb6d8c8 Compare September 13, 2022 20:58
@lexnv lexnv added B0-silent Changes should not be mentioned in any release notes B3-apinoteworthy and removed B0-silent Changes should not be mentioned in any release notes labels Sep 13, 2022
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv force-pushed the lexnv/rpc_chain_spec branch from fb6d8c8 to cca0c81 Compare September 13, 2022 21:03
@lexnv lexnv requested a review from tomaka September 13, 2022 21:05
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@tomaka
Copy link
Contributor

tomaka commented Sep 14, 2022

The logic looks good to me. Can't comment on the code organization.

@niklasad1
Copy link
Member

niklasad1 commented Sep 14, 2022

I think it's better to call the crate rpc-spec something like rpc-api-v2/rpcv2/rpc-spec2/rpc2-unstable instead if we are planning to merge piece by piece.

For now it's probably alright to have the rpc api def and the implementation in the same crate as you did otherwise looks good.

lexnv and others added 3 commits September 14, 2022 18:58
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv
Copy link
Contributor Author

lexnv commented Sep 15, 2022

We have also decided to remove the version: 1 from the result of the rpc_methods.
This is to simplify the rpc_methods response, as the new API should group and version methods.

///
/// # Unstable
///
/// This method is unstable and subject to change in the future.
Copy link
Contributor

@kianenigma kianenigma Sep 16, 2022

Choose a reason for hiding this comment

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

why are these all unstable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although they will be part of the final RPC, these methods are expected to go through a testing phase before standardizing and replacing the old API. During this time (1-2months TBD), we can still add breaking changes. At least, that's how I interpret the unstable from the spec

Copy link
Contributor

Choose a reason for hiding this comment

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

That is my understanding of Pierre's suggestion with this spec; we get them in marked as unstable and then have the opporunity to stabilise them when we're happy.

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

LGTM except all the wrong license years.

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
///
/// This method is unstable and subject to change in the future.
#[method(name = "chainSpec_unstable_genesisHash")]
fn chain_spec_unstable_genesis_hash(&self) -> RpcResult<String>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny thing; I think of chainspec as one word personally; chainspec_unstable_genesis_hash for instance.

Not bothered either way though :)

@lexnv lexnv merged commit de5a7f6 into master Sep 20, 2022
@lexnv lexnv deleted the lexnv/rpc_chain_spec branch September 20, 2022 15:18
altaua pushed a commit that referenced this pull request Sep 20, 2022
* rpc/chain_spec: Add traits for `chainSpec` API

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Implement `chainSpec` RPC methods

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Add tests

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* bin/node: Enable `chainSpec` API

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Assume `genesis_hash` as non-empty

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* Update client/rpc-spec/Cargo.toml

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* Update client/rpc-spec/src/lib.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* client/rpc_spec: Rename crate to `rpc_spec_v2`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc-servers: Remove the `version` field from `rpc_methods`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Fix copyright years

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
mergify bot pushed a commit to paritytech/smoldot that referenced this pull request Sep 23, 2022
Mimics the change done here:
paritytech/substrate#12261

Let's merge this PR only if
paritytech/substrate#12261 is approved and
merged.
ordian added a commit that referenced this pull request Sep 23, 2022
* master:
  [Fix] parameter_types! dead code errors (#12340)
  [Feature] Sequential migration execution for try-runtime (#12319)
  bench: Use `_` instead  of `::` in auto-generated file names (#12332)
  Fast Unstake Pallet (#12129)
  Rename anonymous to pure proxy (#12283)
  Migrate remaining old decl_* macros to the new pallet attribute macros (#12271)
  pallet-utility: Disallow none origin (#12321)
  Make automatic storage deposits resistant against changing deposit prices (#12083)
  Format templates and fix `--steps` default value (#12286)
  Bump `wasmtime` to 1.0.0 (#12317)
  Introduce 'intermediate_insert' method to hide implementation details (#12215)
  Bound staking storage items (#12230)
  Use `array-bytes` for All Array/Bytes/Hex Operations (#12190)
  BREAKING: Rename Origin (#12258)
  Use temporary db for benchmarking (#12254)
  rpc: Implement `chainSpec` RPC API (#12261)
  Import target block body during warp sync (#12300)
  Proper naming wrt expectations (#12311)
  [ci] Revert cancel-pipeline job (#12309)
@lexnv lexnv mentioned this pull request Dec 19, 2022
4 tasks
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* rpc/chain_spec: Add traits for `chainSpec` API

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Implement `chainSpec` RPC methods

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Add tests

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* bin/node: Enable `chainSpec` API

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Assume `genesis_hash` as non-empty

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* Update client/rpc-spec/Cargo.toml

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* Update client/rpc-spec/src/lib.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* client/rpc_spec: Rename crate to `rpc_spec_v2`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc-servers: Remove the `version` field from `rpc_methods`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* rpc/chain_spec: Fix copyright years

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
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. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants