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

Migrate pallet-mmr to the new pallet attribute macro #9181

Merged
6 commits merged into from
Jul 14, 2021
Merged

Migrate pallet-mmr to the new pallet attribute macro #9181

6 commits merged into from
Jul 14, 2021

Conversation

koushiro
Copy link
Contributor

@koushiro koushiro commented Jun 23, 2021

Part of #7882

Migrate the pallet-mmr to the new pallet attribute macro

diff mmr-before.json mmr-after.json (metadata of substrate/bin/node):

4c4
<     "prefix": "MerkleMountainRange",
---
>     "prefix": "Mmr",

⚠️ 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 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 pallet.

polkadot / kusama / westend don't use pallet-mmr for runtime, so no need for migration there.
(However, rococo used pallet-mmr, I'm not sure if need to migrate storage for rococo)

/cc @shawntabrizi @thiolliere

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
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.

Nice! This is only deployed on Rocooco/Wococo now so we don't necessarily need to take care of the migration, but we should restart Rococo when that lands.

frame/merkle-mountain-range/src/lib.rs Outdated Show resolved Hide resolved
@tomusdrw tomusdrw added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Jul 8, 2021
@tomusdrw
Copy link
Contributor

tomusdrw commented Jul 8, 2021

@koushiro would you be so kind to prepare a polkadot companion PR (migrating the existing instance)?

@tomusdrw tomusdrw requested a review from HCastano July 8, 2021 11:33
@gui1117
Copy link
Contributor

gui1117 commented Jul 14, 2021

@tomusdrw is it ok to merge once green ? or should I ping somebody about the rococo restart ?

@tomusdrw
Copy link
Contributor

We were planning to do Rococo runtime upgrade/restart anyway, so IMHO good to merge.

@gui1117
Copy link
Contributor

gui1117 commented Jul 14, 2021

bot merge

@ghost
Copy link

ghost commented Jul 14, 2021

Trying merge.

@ghost ghost merged commit 846cd08 into paritytech:master Jul 14, 2021
@koushiro koushiro deleted the migrate-pallet-mmr branch July 15, 2021 02:11
This pull request was closed.
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. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants