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

Parachains shared.rs to Frame V2 #3425

Merged
11 commits merged into from
Jul 22, 2021
Merged

Conversation

ferrell-code
Copy link
Contributor

relates: #2882

Following the upgrade guidelines here: https://crates.parity.io/frame_support/attr.pallet.html#upgrade-guidelines.

⚠️ Breaking Change ⚠️

From https://crates.parity.io/frame_support/attr.pallet.html#checking-upgrade-guidelines

storages now use PalletInfo for module_prefix instead of the one given to decl_storage: Thus any use of this pallet in construct_runtime! should be careful to update name in order not to break storage or to upgrade storage (moreover for instantiable pallet). If pallet is published, make sure to warn about this breaking change.

So users of the ParasShared pallet must be careful about the name they used in construct_runtime!. Hence the runtime-migration label, which might not be needed depending on the configuration of the ParasShared pallet.

Kusama and westend both already use ParasShared in construct_runtime! no migration needed. Polkadot does not use the ParasShared pallet.

Rococo uses Shared in construct_runtime!, therefore I altered it to ParasShared to keep the pallet name the same as seen in decl_storage! of ParasShared

Comment on lines 532 to 538
ParasInherent: parachains_paras_inherent::{Pallet, Call, Storage, Inherent},
Initializer: parachains_initializer::{Pallet, Call, Storage},
Paras: parachains_paras::{Pallet, Call, Storage, Origin, Event},
ParasShared: parachains_shared::{Pallet, Call, Storage},
Scheduler: parachains_scheduler::{Pallet, Call, Storage},
ParasSudoWrapper: paras_sudo_wrapper::{Pallet, Call},
SessionInfo: parachains_session_info::{Pallet, Call, Storage},
Copy link
Contributor Author

@ferrell-code ferrell-code Jul 7, 2021

Choose a reason for hiding this comment

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

pallet must be put in construct_runtime for tests... if you make a call into ParasShared storage without doing so frame_support will panic as it needs a pallet name (the one that was previously declared in decl_storage!).

I probably should check if any of cumulus tests do this (don't think CI checks)

Edit: I believe it is all good, but if any tests randomly start to panic that's why

@emostov emostov self-requested a review July 21, 2021 22:29
@gui1117
Copy link
Contributor

gui1117 commented Jul 22, 2021

looking at the number of line removed, I guess some master merge got wrong, isn't it ? @ferrell-code

@ferrell-code
Copy link
Contributor Author

ferrell-code commented Jul 22, 2021

Tabrizi swooping in with that 5 am merge lol, I force pushed back to the original point, re-merged and resolved conflicts (I know you guys don't like force pushes but I think reverting a merge to re-merge is reasonable)

@KiChjang KiChjang added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jul 22, 2021
@KiChjang
Copy link
Contributor

bot merge

@ghost
Copy link

ghost commented Jul 22, 2021

Waiting for commit status.

@emostov emostov added E5-breaksapi C1-low PR touches the given topic and has a low impact on builders. and removed A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jul 22, 2021
@emostov emostov added B0-silent Changes should not be mentioned in any release notes D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. A0-please_review Pull request needs code review. labels Jul 22, 2021
@ghost ghost merged commit 71b2a4e into paritytech:master Jul 22, 2021
@emostov emostov removed E5-breaksapi A0-please_review Pull request needs code review. labels Jul 22, 2021
@ferrell-code ferrell-code deleted the fer-shared-framev2 branch July 22, 2021 22:34
@shawntabrizi
Copy link
Member

@ferrell-code yeah sorry about that.

Was gonna fix it for you, but then got too tired, and gave up lol

@ferrell-code
Copy link
Contributor Author

I got a kick out of it, always appreciate your help 🐪

ordian added a commit that referenced this pull request Jul 23, 2021
* master:
  Reduce staking miner reward (companion `substrate/pull/9395`) (#3465)
  Parachains shared.rs to Frame V2 (#3425)
  Parachains hrmp.rs to Frame V2 (#3475)
  Migrate slots pallet to pallet attribute macro. (#3218)
  Improve test in bridge (#3507)
  parachain dmp.rs to Frame V2 (#3426)
  Parachains inclusion.rs to Frame V2 (#3440)
  Dispute coordinator - Recover disputes on startup (#3481)
  Use correct syntax for owning all files in a folder (#3510)
  Add wococo-local chain spec (#3509)
  Dispute vote filtering for block authors (#3498)
  Bump indexmap from 1.6.1 to 1.7.0 (#3497)
  Companion for substrate #9315 (#3477)
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants