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

pallet-session: Migrate the historical part to the new pallet macro #9878

Merged
merged 16 commits into from
Nov 17, 2021
Merged

pallet-session: Migrate the historical part to the new pallet macro #9878

merged 16 commits into from
Nov 17, 2021

Conversation

koushiro
Copy link
Contributor

@koushiro koushiro commented Sep 28, 2021

Part of #7882

Migrate historical part of pallet-session to the new pallet attribute macro.

⚠️ 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 and rococo use Historical as pallet name (but the current module_prefix is Session), thus need to migrate the storages.


polkadot companion: paritytech/polkadot#3949

Polkadot address: 15XuanNimo5951s3RjFTPX1AvYVnCwfr3SDhb3AV4fQF3LpK

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro koushiro marked this pull request as ready for review September 28, 2021 07:51
@koushiro
Copy link
Contributor Author

/cc @thiolliere @KiChjang PTAL

@gui1117 gui1117 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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Sep 28, 2021
@koushiro
Copy link
Contributor Author

@thiolliere @KiChjang @shawntabrizi Could you review this PR?

@koushiro
Copy link
Contributor Author

Does anyone have time to review this PR? 😕
This PR has been opened for a long time.

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.

looks good to me,

The migration needs to be tested on kusama/polkadot with try-runtime

frame/session/src/migrations/mod.rs Outdated Show resolved Hide resolved
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@@ -96,6 +96,7 @@ frame_support::construct_runtime!(
Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
Staking: pallet_staking::{Pallet, Call, Config<T>, Storage, Event<T>},
Session: pallet_session::{Pallet, Call, Storage, Event, Config<T>},
Historical: pallet_session::historical::{Pallet, Storage},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it now needs to have its name in PalletInfo, for the storages, so it needs to be in construct_runtime

@shawntabrizi
Copy link
Member

/tip medium

@substrate-tip-bot
Copy link

Please fix the following problems before calling the tip bot again:

  • Contributor did not properly post their Polkadot or Kusama address. Make sure the pull request has: "{network} address: {address}".

@emostov
Copy link
Contributor

emostov commented Nov 10, 2021

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/polkadot#3949 is not mergeable

@koushiro koushiro requested a review from andresilva as a code owner November 16, 2021 05:32
@gui1117
Copy link
Contributor

gui1117 commented Nov 17, 2021

bot merge

@paritytech-processbot
Copy link

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/reference/pulls#get-a-pull-request","message":"Not Found"}

@gui1117
Copy link
Contributor

gui1117 commented Nov 17, 2021

bot merge

@paritytech-processbot
Copy link

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/reference/pulls#get-a-pull-request","message":"Not Found"}

@gui1117
Copy link
Contributor

gui1117 commented Nov 17, 2021

if the bot give up on us, we will need another approval on the companion in order to merge manually the companion on polkadot

@emostov
Copy link
Contributor

emostov commented Nov 17, 2021

bot merge

@paritytech-processbot
Copy link

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/reference/pulls#get-a-pull-request","message":"Not Found"}

@gui1117
Copy link
Contributor

gui1117 commented Nov 17, 2021

bot merge force

@paritytech-processbot
Copy link

Error: Response error (status 404 Not Found):

{"documentation_url":"https://docs.github.com/rest/reference/pulls#get-a-pull-request","message":"Not Found"}

@gui1117 gui1117 merged commit cc4bf91 into paritytech:master Nov 17, 2021
@koushiro koushiro deleted the migrate-session-historical branch November 17, 2021 07:31
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
…aritytech#9878)

* Migrate session-historical to the new pallet macro

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* pallet-session: Migrate the historical part to the new pallet macro

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix staking test runtime

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Update frame/session/src/historical/mod.rs

* Update frame/session/src/historical/mod.rs

* update migration doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* use hardcoded prefix for migration v1

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo +nightly-2021-11-08 fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
AurevoirXavier added a commit to darwinia-network/darwinia-common that referenced this pull request Aug 4, 2022
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
…aritytech#9878)

* Migrate session-historical to the new pallet macro

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* pallet-session: Migrate the historical part to the new pallet macro

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix staking test runtime

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Update frame/session/src/historical/mod.rs

* Update frame/session/src/historical/mod.rs

* update migration doc

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* use hardcoded prefix for migration v1

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* cargo +nightly-2021-11-08 fmt

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants