-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add staging runtime api #5048
Add staging runtime api #5048
Conversation
9dd0cb4
to
05d8d59
Compare
Where would this staging runtime api being used? What is the idea behind using If you will try to implement the staging + the normal runtime api in the same runtime, the macro will start to complain that this doesn't work. |
I think we should only enable one of them (either v2 or vstaging) conditionally. Quoting some previous comments from @rphmeier
|
Okay thank you. You will need some sort of node code anyway to handle staging vs v2/vx/etc. You could also just add the methods to the normal runtime api. As your node code will need to be prepared for the staging version anyway, you can use that "to version". Aka you just add new methods but they are not being used by the normal node, just by staging stuff. If you then want to stabilize a method, you bump the trait version and change your node code to make this runtime api function usable when version X is present. You could also do something like name the method But these are just some random ideas by me. |
Thanks, that's a good suggestion. @tdimitrov - what I suggest, then, is that we move the declaration of |
05d8d59
to
3771cf7
Compare
3771cf7
to
3eda74f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. One thing struck me though:
ParachainsHost
is versioned .... so should not staging be implemented in the following way:
- Have staging correspond to the next version number that is not yet used - we merely just name it staging to make it clear that we don't want that version on production networks yet.
- Only support the latest stable version of
ParachainHost
on production network, but support the staging version number (3 currently) on test networks. - Staging gets stabilized, by letting staging point to the next version number (4) and by implementing/supporting the old staging ParachainHost (3) on production networks.
I think this is a problem, because two runtime API versions can't easily coexist in the code. For this reason the |
3eda74f
to
2be2d06
Compare
`ParachainHost` is no longer versioned and is in `runtime_api` module. This is a companion for paritytech/polkadot#5048
`ParachainHost` is no longer versioned and is in `runtime_api` module. This is a companion for paritytech/polkadot#5048
44e00a6
to
660680b
Compare
How do we prevent an |
There would be no problem of having a |
But how the client is supposed to quiery this API? We use versioning here polkadot/node/core/runtime-api/src/lib.rs Lines 427 to 430 in 6ec8ede
When introducing an unstable API, what version should be put there? And how's the client will be using it? Like any other runtime API? Or are we going to feature gate the unstable functionality on the client side? Then the worry is accidental feature leaking. |
4467629
to
425104e
Compare
`ParachainHost` is no longer versioned and is in `runtime_api` module. This is a companion for paritytech/polkadot#5048
`trait ParachainHost` is no longer part of a specific primitives version. Instead there is a single trait for stable and staging api versions. The trait contains stable AND staging methods. The latter are explicitly marked as unstable.
`polkadot_primitives::v2` becomes `polkadot_primitives::runtime_api`
Introduces the concept for 'staging functions' in runtime API. These functions are still in testing and they are meant to be used only within test networks (Westend). They coexist with the stable calls for technical reasons - maintaining different runtime APIs for different networks is hard to implement. Check the doc comments in source files for more details how the staging API should be used.
Add `staging_get_session_disputes` to `ParachainHost` as the first method of the staging API.
219a57f
to
2f84976
Compare
bot merge |
Error: pr-custom-review is not passing for paritytech/cumulus#1108 |
bot merge |
* Handle relocation of `ParachainHost` in Polkadot `ParachainHost` is no longer versioned and is in `runtime_api` module. This is a companion for paritytech/polkadot#5048 * Update dependencies
* master: (62 commits) Bump tracing from 0.1.32 to 0.1.33 (#5299) Add staging runtime api (#5048) CI: rename ambiguous jobs (#5313) Prepare for rust 1.60 (#5282) Bump proc-macro2 from 1.0.36 to 1.0.37 (#5265) Fixes the dead lock when any of the channels get at capacity. (#5297) Bump syn from 1.0.90 to 1.0.91 (#5273) create .github/workflows/pr-custom-review.yml (#5280) [ci] fix pallet benchmark script (#5247) (#5292) bump spec_version to 9190 (#5291) bump version to 0.9.19 (#5290) Session is actually ancient not unknown. (#5286) [ci] point benchmark script to sub-commands (#5247) (#5289) Remove Travis CI (#5287) Improve error handling in approval voting block import (#5283) fix .github/pr-custom-review.yml (#5279) Co #11164: Sub-commands for `benchmark` (#5247) Bump clap from 3.1.6 to 3.1.8 (#5240) Custom Slots Migration for Fund Index Change (#5227) create pr-custom-review.yml (#5253) ...
* Handle relocation of `ParachainHost` in Polkadot `ParachainHost` is no longer versioned and is in `runtime_api` module. This is a companion for paritytech/polkadot#5048 * Update dependencies
Quoting @rphmeier (with small edits):
The broader idea I have around runtime API upgrades is that we'd
maintain a vstaging which is based on the latest API version and has a
special STAGING_VERSION (u32::MAX). We'd be able to conditionally enable
this on runtimes for testing. And then we'd eventually set the next
runtime API version to be equal to vstaging and start a fresh vstaging
cumulus companion: paritytech/cumulus#1108