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

Remove v0 parachains runtime #1501

Merged
merged 28 commits into from
Aug 13, 2020
Merged

Remove v0 parachains runtime #1501

merged 28 commits into from
Aug 13, 2020

Conversation

rphmeier
Copy link
Contributor

@rphmeier rphmeier commented Jul 30, 2020

All of the functionality except for the Registrar module has been superseded by the v1 runtime modules. I removed the Registrar regardless, leaving the runtime without parachains

The Registrar Module

The registrar module previously had these functions:

  1. Parachain and Parathread registrar
  2. Parachain and parathread scheduling
  3. Parathread auctions

Function 1 is now mostly provided by the Paras module. This Registrar should provide a small wrapper around the Paras for handling deposits.
Function 2 is now handled by the Scheduler module.
Function 3 should be handled by a separate a parathread_auction module.

So although the Registrar is currently being removed, those relevant pieces of logic can be recovered from the commit history when the new version is created in a follow-on PR.

We need to add para swapping (one feature of the old registrar) to the Paras module and make some small tweaks: swapping should only happen at session boundaries.

Compatibility with previous runtime

There are two main compatibility concerns: Call indices and SessionKey layout. For the former, I replaced each of the existing parachains modules with an instanced dummy module. For the latter, I will ensure that the session key layout remains the same by keeping the Parachains session key around, but where updates are simply discarded for the time being.

I also created a dummy implementation of the v0::ParachainHost runtime API. This can be removed once the BlockBuilder in polkadot-validation no longer relies on it.

@rphmeier rphmeier added A3-in_progress Pull request is in progress. No review needed at this stage. B2-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Jul 30, 2020
@rphmeier
Copy link
Contributor Author

@bkchr I'm getting a runtime error:

conflicting implementations of trait `sp_api_hidden_includes_construct_runtime::hidden_include::IsSubType<polkadot_runtime_common::dummy::Call<Runtime>>` for type `Call`:

probably because there is no typed differentiator of the dummy instances.

I'm also getting "duplicate lang item in crate std" which is puzzling, since I didn't add or change any dependency of the polkadot-runtime crate or the polkadot-runtime-common crate in this PR.

@rphmeier rphmeier marked this pull request as ready for review July 31, 2020 00:20
@rphmeier rphmeier added A1-onice and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jul 31, 2020
@rphmeier
Copy link
Contributor Author

Icing until we can test the runtime upgrades.

@gui1117
Copy link
Contributor

gui1117 commented Aug 4, 2020

Registrar, Parachains, and Slots have some keys on chain (in polkadot), I guess they should be removed with some migration code
(maybe kusama also have some attestation keys)

@rphmeier
Copy link
Contributor Author

rphmeier commented Aug 5, 2020

I guess they should be removed with some migration code

I think we can also defer to governance on this, right? But we could also do remove_prefix

Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

I did start a kusama and a polkadot testnet with storage datas from kusama and polkadot (except babe, grandpa, session and staking). with the runtime from this PR (except with small session length and era length) and the client from v0.8.19, and it worked fine finalizing and new eras.

still I think we need to adress the origin changing issue otherwise looks good to me

// Parachains stuff; slots are disabled (no auctions initially). The rest are safe as they
// have no public dispatchables. Disabled `Call` on all of them, but this should be
// uncommented once we're ready to start parachains.
Parachains: parachains::{Module, Call, Storage, Config, Inherent, Origin},
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the outer origin enum, this requires an upgrade of scheduler::Agenda storage (or a dummy variant in origin, maybe using Dummy)

(scheduler agenda holds the origin since paritytech/substrate#6387 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point. We'll add the parachains origin to this PR

Copy link
Contributor Author

@rphmeier rphmeier Aug 12, 2020

Choose a reason for hiding this comment

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

We did some digging, and found that the migration is not really necessary because:

  1. The council on Polkadot and Kusama has not scheduled anything with a Parachains origin and will not, either
  2. The Origin enum had the Parachains origin at the end, so no indices of other origins were altered

@rphmeier rphmeier removed the A1-onice label Aug 12, 2020
@rphmeier rphmeier merged commit ae5990c into master Aug 13, 2020
@rphmeier rphmeier deleted the rh-remove-v0-runtime branch August 13, 2020 13:55
ordian added a commit that referenced this pull request Aug 14, 2020
* master:
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
  Update .editorconfig to what we have in practice (#1545)
  Companion PR for substrate #6672 (#1560)
  pre-redenomination tockenSymbol change (#1561)
ordian added a commit that referenced this pull request Aug 17, 2020
…n-race-condition

* master:
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
ordian added a commit that referenced this pull request Aug 17, 2020
* master:
  overseer: fix build (#1596)
  Companion PR for #6862 (#1564)
  implement collation generation subsystem (#1557)
  Add spawn_blocking to SubsystemContext (#1570)
  Companion PR for #6846 (#1568)
  overseer: add a test to ensure all subsystem receive msgs (#1590)
  Implementer's Guide: Flesh out more details for upward messages (#1556)
  Make parachain validation wasm executor functional (#1574)
  Use async test helper to simplify node testing (#1578)
  guide: validation data refactoring (#1576)
  Remove v0 parachains runtime (#1501)
  [CI] Add github token to generate-release-text (#1581)
  Allow using any polkadot client instead of enum Client (#1575)
  service/src/lib: Update authority discovery construction (#1563)
  Update .editorconfig to what we have in practice (#1545)
  Companion PR for substrate #6672 (#1560)
  pre-redenomination tockenSymbol change (#1561)
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.

4 participants