migrations: VersionedRuntimeUpgrade #14311

liamaharon · 2023-06-06T21:22:27Z

Description

Make it easier to write versioned runtime upgrades.

VersionedRuntimeUpgrade allows developers to write migrations without worrying about checking and setting storage versions. Instead, the developer wraps their migration in this struct which takes care of version handling using best practices.

It takes 5 type parameters:

From: The version being upgraded from
To: The version being upgraded to
Inner: An implementation of OnRuntimeUpgrade
Pallet: The Pallet being upgraded
Weight: The runtime's RuntimeDbWeight implementation

When a VersionedRuntimeUpgrades on_runtime_upgrade method is called, the on-chain version of the pallet is compared to From. If they match, Inner::on_runtime_upgrade is called and the pallets on-chain version is set to To. Otherwise, a warning is logged notifying the developer that the upgrade was a noop and should probably be removed.

Example

Example using this for a migration in the Polkadot repo: https://github.com/paritytech/polkadot/compare/liam-versioned-runtime-upgrade-example

Decision not to remove `#[pallet::storage_version]`

The possibility of removing #[pallet::storage_version] leaving only an on-chain version has been discussed. Although this would be nice, I think it is necessary to keep so that we may correctly set the initial on-chain version of a pallet when it is first initialized.

See paritytech/polkadot-sdk#109 for more which I'm planning to work on soon, which appears to require #[pallet::storage_version].

Decision to use `From` and `To` rather than just `Version` (`From`)

See #14311 (comment)

TODOs

Add some tests making sure the Inner hooks are called when the version matches, and not called when the version doesn't match
Test that bytes are passed correctly from pre_upgrade to post_upgrade
Update CI to run experimental tests [FRAME/Meta/PM] "experimental" FRAME features #14345 (comment)

ggwpez

Thanks for starting this!

frame/support/src/migrations.rs

gui1117 · 2023-06-11T04:13:26Z

Looks good,

I saw the example in https://github.com/paritytech/polkadot/compare/liam-versioned-runtime-upgrade-example
I wonder about usage: instead of using the type in the kusama runtime it should be used to declare the migration with version checked inside the pallet directly, no?
the pallet configuration would expose both VersionedCheckedMigrateToV6 and VersionedUncheckedMigrateToV6:

type VersionCheckedMigrateToV6 = frame_support::migrations::VersionedRuntimeUpgrade<
			V5ToV6,
			parachains_configuration::migration::v6::VersionedUncheckedMigrateToV6<Runtime>,
			Configuration,
			RocksDbWeight,
		>,

liamaharon · 2023-06-11T17:58:39Z

Thanks @ggwpez and @thiolliere for your comments. I've addressed them:

Replaced Version with To and From for clearer behavior, and allowing skipping versions
Updated my example on https://github.com/paritytech/polkadot/compare/liam-versioned-runtime-upgrade-example to define VersionCheckedMigrateV5ToV6 in the migrations file for use in the runtimes

and also written some tests.

Ready to review again when you have some time.

liamaharon · 2023-06-11T21:04:19Z

Question: after merging, should we apply this to applicable Unreleased migrations?

kianenigma · 2023-06-12T08:40:54Z

The main reason I suggested removing #[pallet::storage_version] is that I envision a single system-like migration pallet (possibly #14275) to handle all the things that @thiolliere and @ggwpez discussed, like jumping migrations and so on.

Moreover, we have always considered a new hook for when a pallet is added to the runtime. All in all

I think it is necessary to keep so that we may correctly set the initial on-chain version of a pallet when it is first initialized.

can be solved in various other ways.

--

From a DevEx perspective, I still want to suggest finding a way to not do this, as having both the onchain version and the fixed pallet version is not optimal.

All in all, I do agree that this is a clear improvement over the existing mechanism. My comment is that the existing mechanism is itself somewhat clunky/broken and can be reconsidered.

kianenigma · 2023-06-12T08:43:28Z

frame/support/src/migrations.rs

+/// that the upgrade was a noop and should probably be removed.
+///
+/// Example:
+/// ```ignore


possibly not to difficult to make this not be ignore and actually compile.

are you thinking I'd use the dummy pallet and upgrades from my test file? I'd rather not use a migration from a real pallet since it creates a dependency that could break. I'm also curious what the benefit/s are of making the comment compile

ggwpez · 2023-06-23T17:23:58Z

I think we still use the C* labels for release priority. So anything that is not a hot/security fix is "low".

frame/support/src/migrations.rs

frame/support/tests/versioned_runtime_upgrade.rs

frame/support/src/migrations.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

…rsioned-runtime-upgrade

liamaharon · 2023-07-02T08:36:06Z

bot merge

* VersionedRuntimeUpgrade * only require one version and add a pre-upgrade check * add docs * improve warning log * improve comments * fix log * use associated constants * allow passing from and to versions * test versioned_runtime_upgrade * fix typo * improve docs * docs * docs * remove event from dummy pallet * remove pre_upgrade current storage version check * derive_impl * skip pre/post checks if the actual migration will not run * improve variable naming * docs * fix post_upgrade 'should run' logic * fix comments * pre and post hook tests * feature gate try-runtime stuff * remove deprecated macro * Update frame/support/src/migrations.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * decode_all * make experimental * use rust generics * add info log when running * simplify tests * improve log * improve log * cleaner pre_upgrade encoding * Update frame/support/src/migrations.rs Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * Update frame/support/src/migrations.rs Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * Update frame/support/src/migrations.rs Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * Update frame/support/src/migrations.rs Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * Update frame/support/src/migrations.rs Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com> * VersionedPostUpgradeData enum * move versioned runtime upgrade tests to test/tests * fix rust doc * clarify comment --------- Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

liamaharon added 6 commits June 6, 2023 20:39

VersionedRuntimeUpgrade

0885a4f

only require one version and add a pre-upgrade check

9c0072f

add docs

2f30d00

improve warning log

451de26

improve comments

d8bc4d7

fix log

53f6170

liamaharon requested review from a team June 6, 2023 21:22

ggwpez reviewed Jun 7, 2023

View reviewed changes

frame/support/src/migrations.rs Outdated Show resolved Hide resolved

frame/support/src/migrations.rs Show resolved Hide resolved

frame/support/src/migrations.rs Outdated Show resolved Hide resolved

paritytech-ci requested a review from a team June 7, 2023 11:15

ggwpez mentioned this pull request Jun 7, 2023

Fix migrations paritytech/polkadot#7340

Merged

use associated constants

617c864

liamaharon added 2 commits June 11, 2023 19:41

allow passing from and to versions

1b9c68d

test versioned_runtime_upgrade

acc9b0c

liamaharon requested review from gui1117 and ggwpez June 11, 2023 17:56

liamaharon added 2 commits June 11, 2023 20:00

fix typo

c3638c5

improve docs

f5cab56

liamaharon added 2 commits June 11, 2023 23:16

docs

bd42fbe

docs

0aae7e8

kianenigma reviewed Jun 12, 2023

View reviewed changes

liamaharon added 3 commits June 23, 2023 12:07

improve log

52b920e

improve log

a16eba7

cleaner pre_upgrade encoding

c8e0538

sam0x17 approved these changes Jun 23, 2023

View reviewed changes

ggwpez added C1-low PR touches the given topic and has a low impact on builders. and removed C3-medium PR touches the given topic and has a medium impact on builders. labels Jun 23, 2023

liamaharon mentioned this pull request Aug 24, 2023

[meta] strict on-runtime-upgrade CI checks to prevent accidental runtime upgrade breakages paritytech/polkadot-sdk#481

Closed

9 tasks

gpestana reviewed Jun 29, 2023

View reviewed changes

liamaharon and others added 11 commits June 30, 2023 00:20

Update frame/support/src/migrations.rs

2417754

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

Update frame/support/src/migrations.rs

a72c9b7

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

Update frame/support/src/migrations.rs

63542cf

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

Update frame/support/src/migrations.rs

547feec

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

Update frame/support/src/migrations.rs

8c2050c

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

Merge branch 'master' of github.com:paritytech/substrate into liam-ve…

ae484c7

…rsioned-runtime-upgrade

VersionedPostUpgradeData enum

ab880cb

move versioned runtime upgrade tests to test/tests

605f63d

fix rust doc

8a9e31c

Merge branch 'master' of github.com:paritytech/substrate into liam-ve…

e5d4207

…rsioned-runtime-upgrade

clarify comment

888f5eb

paritytech-processbot bot merged commit 9e8b2f6 into master Jul 2, 2023

paritytech-processbot bot deleted the liam-versioned-runtime-upgrade branch July 2, 2023 08:36

nazar-pc mentioned this pull request Sep 13, 2023

Upgrade Substrate and Frontier autonomys/subspace#1954

Merged

1 task

crystalin mentioned this pull request Oct 20, 2023

Update substrate/polkadot/cumulus from v0.9.43 to v1.1.0 moonbeam-foundation/moonbeam#2535

Closed

JesseAbram mentioned this pull request Oct 26, 2023

Update substrate and subxt entropyxyz/entropy-core#435

Merged

lemunozm mentioned this pull request Apr 8, 2024

Polkadot v1.1.0 centrifuge/centrifuge-chain#1756

Merged

58 tasks

lemunozm mentioned this pull request Apr 19, 2024

fix: outdated storage versions on Demo centrifuge/centrifuge-chain#1820

Merged

4 tasks

surangap mentioned this pull request May 29, 2024

Upgrade Substrate to v1.0.0 futureversecom/trn-seed#821

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrations: VersionedRuntimeUpgrade #14311

migrations: VersionedRuntimeUpgrade #14311

liamaharon commented Jun 6, 2023 •

edited

Loading

ggwpez left a comment

gui1117 commented Jun 11, 2023

liamaharon commented Jun 11, 2023 •

edited

Loading

liamaharon commented Jun 11, 2023

kianenigma commented Jun 12, 2023 •

edited

Loading

kianenigma Jun 12, 2023

liamaharon Jun 13, 2023

ggwpez commented Jun 23, 2023

liamaharon commented Jul 2, 2023

migrations: VersionedRuntimeUpgrade #14311

migrations: VersionedRuntimeUpgrade #14311

Conversation

liamaharon commented Jun 6, 2023 • edited Loading

Description

Example

Decision not to remove #[pallet::storage_version]

Decision to use From and To rather than just Version (From)

TODOs

ggwpez left a comment

Choose a reason for hiding this comment

gui1117 commented Jun 11, 2023

liamaharon commented Jun 11, 2023 • edited Loading

liamaharon commented Jun 11, 2023

kianenigma commented Jun 12, 2023 • edited Loading

kianenigma Jun 12, 2023

Choose a reason for hiding this comment

liamaharon Jun 13, 2023

Choose a reason for hiding this comment

ggwpez commented Jun 23, 2023

liamaharon commented Jul 2, 2023

liamaharon commented Jun 6, 2023 •

edited

Loading

Decision not to remove `#[pallet::storage_version]`

Decision to use `From` and `To` rather than just `Version` (`From`)

liamaharon commented Jun 11, 2023 •

edited

Loading

kianenigma commented Jun 12, 2023 •

edited

Loading