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

Ensure all StorageVersions on Polkadot/Kusama are correct #7199

Merged
merged 5 commits into from
May 17, 2023
Merged

Ensure all StorageVersions on Polkadot/Kusama are correct #7199

merged 5 commits into from
May 17, 2023

Conversation

@bkchr
Copy link
Member

@bkchr bkchr commented May 8, 2023

With the recent change in Substrate we now detect issues with StorageVersion when running try-runtime. This pr fixes all the issues to make try-runtime working for Polkadot/Kusama again.

@bkchr bkchr 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 May 8, 2023
@paritytech-ci paritytech-ci requested review from a team May 8, 2023 15:49
@ggwpez ggwpez added the E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. label May 8, 2023
ggwpez
ggwpez approved these changes May 8, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Noice. Lets see what the CI says.

Now we just need paritytech/substrate#13013 and then we are set.

xcm/pallet-xcm/src/migration.rs

log::info!("v1 applied successfully");
STORAGE_VERSION.put::<Pallet<T>>();
StorageVersion::new(1).put::<Pallet<T>>();
Copy link
Member

Choose a reason for hiding this comment

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

I think we should pay attention in general to never use STORAGE_VERSION.put.
There is one more case in runtime/parachains/src/configuration/migration.rs.

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 we should pay attention in general to never use STORAGE_VERSION.put.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because when you change STORAGE_VERSION, you will change the version the migration is writing to storage.

@kianenigma
Copy link
Contributor

Doesn't this address parts of #7187?

@kianenigma kianenigma self-requested a review May 9, 2023 08:02
@bkchr
Copy link
Member Author

bkchr commented May 9, 2023

Doesn't this address parts of #7187?

Yes

@ggwpez ggwpez mentioned this pull request May 10, 2023
Closed
@ggwpez ggwpez mentioned this pull request May 14, 2023
Closed
15 tasks
@liamaharon
Copy link
Contributor

liamaharon commented May 17, 2023 •
edited
Loading

Tested on_runtime_upgrade on Polkadot/Kusama with a runtime built from this branch, looks good.

try-runtime-cli on-runtime-upgrade is currently broken when used with runtimes built from 1203b25 and above, this fixes it so going to merge it.

@liamaharon
Copy link
Contributor

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit f13e389 into master May 17, 2023
@paritytech-processbot paritytech-processbot bot deleted the bkchr-storage-version-fall-out branch May 17, 2023 14:53
ordian pushed a commit that referenced this pull request May 23, 2023
Merge branch 'master' into ao-past-session-slashing-runtime
f45594d 
* master: (60 commits)
  Ensure all `StorageVersion`s on Rococo/Westend are correct and migration hooks pass (#7251)
  Try-runtime proper return types (#7146)
  Have OCW mined election once a week on Westend (#7248)
  Bump enumn from 0.1.5 to 0.1.8 (#7226)
  Companion to #14183: FRAME: Allow message ID to be mutated in `ProcessMessage` (#7262)
  Remove TODO comment (#7260)
  Fix build (#7261)
  Update syn (#7258)
  Use Message Queue pallet for UMP dispatch (#6271)
  Freeze chain if there are byzantine threshold + 1 invalid votes against a local candidate (#7225)
  Revert chain if at least f+1 validators voted against a candidate (#7151)
  Ensure all `StorageVersion`s on Polkadot/Kusama are correct (#7199)
  Forgotten pub reexport for `GlobalConsensusParachainConvertsFor` (#7238)
  PVF: Vote invalid on panics in execution thread (after a retry) (#7155)
  PVF: Remove `rayon` and some uses of `tokio` (#7153)
  [xcm] Foreign global consensus parachain LocationToAccountId converter (#7016)
  Update docs (#7230)
  Bump parity-db to 0.4.8 (#7231)
  Merge branch 'master' of https://github.com/paritytech/polkadot (#7224)
  Relax the watermark rule in the runtime (#7188)
  ...
ordian pushed a commit that referenced this pull request May 23, 2023
Merge branch 'ao-past-session-slashing-runtime' into ao-past-session-…
4bd06fb 
…slashing-client

* ao-past-session-slashing-runtime: (61 commits)
  Ensure all `StorageVersion`s on Rococo/Westend are correct and migration hooks pass (#7251)
  Try-runtime proper return types (#7146)
  Have OCW mined election once a week on Westend (#7248)
  Bump enumn from 0.1.5 to 0.1.8 (#7226)
  Companion to #14183: FRAME: Allow message ID to be mutated in `ProcessMessage` (#7262)
  Remove TODO comment (#7260)
  Fix build (#7261)
  Update syn (#7258)
  Use Message Queue pallet for UMP dispatch (#6271)
  Freeze chain if there are byzantine threshold + 1 invalid votes against a local candidate (#7225)
  Revert chain if at least f+1 validators voted against a candidate (#7151)
  Ensure all `StorageVersion`s on Polkadot/Kusama are correct (#7199)
  Forgotten pub reexport for `GlobalConsensusParachainConvertsFor` (#7238)
  PVF: Vote invalid on panics in execution thread (after a retry) (#7155)
  PVF: Remove `rayon` and some uses of `tokio` (#7153)
  [xcm] Foreign global consensus parachain LocationToAccountId converter (#7016)
  Update docs (#7230)
  Bump parity-db to 0.4.8 (#7231)
  Merge branch 'master' of https://github.com/paritytech/polkadot (#7224)
  Relax the watermark rule in the runtime (#7188)
  ...
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Closed
@kiltbot kiltbot mentioned this pull request Jun 21, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@kianenigma kianenigma Awaiting requested review from kianenigma

6 more reviewers

@gavofyork gavofyork gavofyork approved these changes

@sam0x17 sam0x17 sam0x17 approved these changes

@ordian ordian ordian approved these changes

@ggwpez ggwpez ggwpez approved these changes

@liamaharon liamaharon liamaharon approved these changes

@lexnv lexnv lexnv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@bkchr @kianenigma @liamaharon @gavofyork @sam0x17 @ordian @ggwpez @lexnv