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

Use #[pallet::storage_version] for pallet staking #12728

Merged
merged 15 commits into from
Jan 3, 2023

Conversation

Ank4n
Copy link
Contributor

@Ank4n Ank4n commented Nov 17, 2022

Changes how staking pallet maintains versioning to the new standard #[pallet::storage_version].
Fixes #12041.
polkadot companion: paritytech/polkadot#6365.

@Ank4n Ank4n linked an issue Nov 17, 2022 that may be closed by this pull request
@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 17, 2022
@Ank4n Ank4n force-pushed the ankan/12041-staking-migration-versioning branch from b592459 to b3b1090 Compare November 19, 2022 15:57
@Ank4n
Copy link
Contributor Author

Ank4n commented Nov 29, 2022

bot rebase

@paritytech-processbot
Copy link

Rebased

@Ank4n Ank4n added 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 Nov 29, 2022
@Ank4n Ank4n marked this pull request as ready for review November 29, 2022 11:39
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Nov 29, 2022

/// Alias to the old storage item for keeping versions. Outdated since v12.
#[storage_alias]
type StorageVersion<T: Config> = StorageValue<Pallet<T>, ObsoleteReleases, ValueQuery>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be scoped in v13?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its used in all migrations prior to v13. May be we should get rid of old migrations (unless you see a reason to keep them) and keep it scoped only in v13?

@@ -60,8 +60,12 @@ pub mod pallet {

use super::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(13);
Copy link
Contributor

Choose a reason for hiding this comment

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

technically we could have started from 0 as well, but whatever :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically yes :). But may be still nice to convey this is the 13th migration. 😉

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Cheers. has my blessing if try-runtime has been tested once.

In the companion, now there should be a test that runs in CI and checks try-runtime actually https://github.com/paritytech/polkadot/blob/5b844f8aeb47df6275dc5afe7be78cabf18c659a/runtime/polkadot/src/lib.rs#L2418

Please be wary about the migration label, it is important ;) For example, this job is skipped because of the label: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2098421

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

frame/staking/src/migrations.rs Show resolved Hide resolved
@Ank4n
Copy link
Contributor Author

Ank4n commented Dec 28, 2022

bot rebase

@paritytech-processbot
Copy link

Rebased

@Ank4n Ank4n merged commit c32968a into master Jan 3, 2023
@Ank4n Ank4n deleted the ankan/12041-staking-migration-versioning branch January 3, 2023 19:46
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* delete releases

* use standard pallet storage version

* migrate to standard storage version for staking

* not compiling

* keep old releases enum around for decoding

* fix releases

* rename old releases

* retriggering ci

* fix migration comments

* doc update

Co-authored-by: parity-processbot <>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* delete releases

* use standard pallet storage version

* migrate to standard storage version for staking

* not compiling

* keep old releases enum around for decoding

* fix releases

* rename old releases

* retriggering ci

* fix migration comments

* doc update

Co-authored-by: parity-processbot <>
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. 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
Status: Done
Development

Successfully merging this pull request may close these issues.

Move pallet-staking to use the standard #[pallet::storage_version]
5 participants