Better identifier and logging for runtime upgrades #8123

kianenigma · 2021-02-15T11:28:38Z

This builds on top of #8038 and assigns a unique ID to each OnRuntimeUpgrade, which is used for two purposes:

Generate a storage key that can be used in pre-upgrade/post-upgrade. I don't have an example of this yet in substrate, see an example here: paritytech/polkadot@146f1e2#diff-db5840c3c03c740de25aed56b9257559c224a04619c079c94cddbb23f9a855a4R1007

The use case is that we want to write some data in pre-migration and read it back in post migration. This storage key facilitates that.

Log some useful information about the migration. Running a migration with the bot will now output:

2021-02-18 17:25:08.848   INFO main remote-ext: scraping keypairs from cache "./Kusama,0xdd772d86d5cfd2be9a11dd504866c7878ee071aefe02ff7db3d749f98bf890b8,.bin"
2021-02-18 17:25:09.356   INFO main remote-ext: extending externalities with 1 manually injected keys
2021-02-18 17:25:09.357   INFO main remote-ext: injecting a total of 327269 keys
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Lottery' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Mmr' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Tips' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Bounties' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Multisig' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Proxy' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Scheduler' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Vesting' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Recovery' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Society' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Identity' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'RandomnessCollectiveFlip' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Historical' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Offences' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'AuthorityDiscovery' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'ImOnline' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Sudo' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Contracts' and setting new storage version to PalletVersion { major: 2, minor: 0, patch: 1 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Treasury' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Grandpa' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'TechnicalMembership' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Elections' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'TechnicalCommittee' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Council' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Democracy' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:17.850   INFO main runtime::frame-support: ✅ no migration for 'Session' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ⚠️ running migration for Staking and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ✅ no migration for 'TransactionPayment' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ✅ no migration for 'Indices' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ✅ no migration for 'Authorship' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ✅ no migration for 'Babe' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main runtime::frame-support: ✅ no migration for 'Utility' and setting new storage version to PalletVersion { major: 3, minor: 0, patch: 0 }
2021-02-18 17:25:18.497   INFO main try_runtime_cli: try-runtime executed without errors. Consumed weight = 1024921600000000, total weight = 512000000000000 (2.0018)

I've made the assumption again that each crate has one pallet only.

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

…' of github.com:paritytech/substrate into kiz-finally-finally-finally-finally-migration-testing-2

frame/support/procedural/src/pallet_version.rs

frame/support/src/traits.rs

gui1117 · 2021-02-18T09:20:21Z

I don't think you need an additional identifier here.
The name given by PalletInfo is already enough in this context I think, it is unique for the pallet and identify the pallet.
Then if you want to have multiple unique keys inside the pallet you can concatenate unique keys like:

<T as frame_system::PalletInfo as frame_support::traits::PalletInfo>::name::<Pallet<T>>() ++ some unique key for migration purpose

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

kianenigma · 2021-02-18T10:16:47Z

Will remove the Identifier, I also noticed that it is optional at least as far as this PR goes.

…ate into kiz-migration-testin-2

kianenigma · 2021-02-23T06:59:09Z

Now the per-pallet OnRutnimeUpgrades can log their name and what they are doing, namely by the virtue of macros and that we add the log for them. For the custom ones, we can't do the same as far as I can see. Ideally, we should be able to achieve the same for the custom OnRuntimeUpgrades passed to executive as well. Not sure how. Even the const ID that I added before to OnRuntimeUpgrade won't help.

kianenigma · 2021-02-23T07:06:12Z

I've made the assumption again that each crate has one pallet only.

What you mean by this?

This is no longer relevant. In the past I made a new proc macro the get the create name from the carg file and use that for logging, now I use PalletInfo.

bkchr

Some last questions :D

frame/support/src/traits.rs

…ration-testin-2

kianenigma · 2021-02-23T18:32:39Z

@bkchr I hope you are happy now :D d6e7b8b

frame/support/src/traits.rs

gui1117 · 2021-02-24T12:04:20Z

bin/node/runtime/Cargo.toml

@@ -154,6 +154,7 @@ std = [
 	"pallet-society/std",
 	"pallet-recovery/std",
 	"pallet-vesting/std",
+	"frame-try-runtime/std",


yes this is needed because otherwise we can't compile node-runtime with try-runtime features and std :-/
the unused dependency on std should harm to much, and this is what we do for frame-benchmarking.

bkchr

I'm still not happy with bincode, but fine ;D

…ration-testin-2

kianenigma · 2021-02-25T15:13:04Z

@bkchr I hope you are happy now :D d6e7b8b

likewise: 4e154f0

kianenigma · 2021-02-25T15:13:17Z

bot merge

ghost · 2021-02-25T15:13:21Z

Waiting for commit status.

ghost · 2021-02-25T15:43:27Z

Checks failed; merge aborted.

kianenigma · 2021-02-26T15:41:18Z

bot merge

ghost · 2021-02-26T15:41:22Z

Trying merge.

* A clean new attempt * Checkpoint to move remote. * A lot of dependency wiring to make it feature gated. * bad macro, bad macro. * Undo the DB mess. * Update frame/support/src/traits.rs Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> * Apply suggestions from code review Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> * unbreak the build * Better logging and ids for migrations * Fix doc. * Test * Update frame/try-runtime/src/lib.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update utils/frame/try-runtime/cli/Cargo.toml Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Update frame/try-runtime/Cargo.toml Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Address most review grumbles. * Fix build * Add some comments * Remove allowing one pallet at a time. * Rework the PR * nit * Slightly better error handling. * Remove files * Update utils/frame/remote-externalities/src/lib.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * Update frame/support/src/dispatch.rs * Update frame/support/src/dispatch.rs * Fix test * Make extension trait. * Bring back try-runtime/std * remove bincode * Remove warning * Change test features Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

kianenigma and others added 11 commits February 3, 2021 16:25

A clean new attempt

4c0abc5

Checkpoint to move remote.

7f9b5b8

A lot of dependency wiring to make it feature gated.

4883812

bad macro, bad macro.

be549b4

Master.into()

8e97733

Undo the DB mess.

d84dad4

Update frame/support/src/traits.rs

aeb7a0e

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

Apply suggestions from code review

d968f58

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

unbreak the build

ce4128b

Merge branch 'kiz-finally-finally-finally-finally-migration-testing-2…

ee8ae08

…' of github.com:paritytech/substrate into kiz-finally-finally-finally-finally-migration-testing-2

Better logging and ids for migrations

87baef8

kianenigma 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. labels Feb 15, 2021

kianenigma requested review from bkchr and gui1117 February 15, 2021 11:28

kianenigma commented Feb 15, 2021

View reviewed changes

frame/support/procedural/src/pallet_version.rs Outdated Show resolved Hide resolved

kianenigma commented Feb 15, 2021

View reviewed changes

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

Fix doc.

01298eb

github-actions bot added the A7-needspolkadotpr label Feb 15, 2021

kianenigma added 2 commits February 15, 2021 15:25

Test

14bf87f

Master.into()

62be119

Update frame/try-runtime/src/lib.rs

712c240

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

kianenigma mentioned this pull request Feb 18, 2021

Migration testing runtime API/Bot #8038

Merged

kianenigma and others added 2 commits February 18, 2021 10:11

Update utils/frame/try-runtime/cli/Cargo.toml

9a23940

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

Update frame/try-runtime/Cargo.toml

93f299a

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

kianenigma removed A0-please_review Pull request needs code review. A7-needspolkadotpr labels Feb 18, 2021

Update frame/support/src/dispatch.rs

415bca9

paritytech-ci added the D2-breaksapi label Feb 23, 2021

kianenigma added 2 commits February 23, 2021 06:56

Fix test

bdad8a5

Merge branch 'kiz-migration-testin-2' of github.com:paritytech/substr…

2d6df71

…ate into kiz-migration-testin-2

kianenigma requested a review from bkchr February 23, 2021 07:25

bkchr reviewed Feb 23, 2021

View reviewed changes

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

kianenigma added 2 commits February 23, 2021 18:18

Merge branch 'master' of github.com:paritytech/substrate into kiz-mig…

05a5daf

…ration-testin-2

Make extension trait.

d6e7b8b

gui1117 reviewed Feb 24, 2021

View reviewed changes

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

Bring back try-runtime/std

6714bfc

gui1117 reviewed Feb 24, 2021

View reviewed changes

gui1117 approved these changes Feb 24, 2021

View reviewed changes

bkchr approved these changes Feb 25, 2021

View reviewed changes

kianenigma added 2 commits February 25, 2021 15:04

Merge branch 'master' of github.com:paritytech/substrate into kiz-mig…

d06d069

…ration-testin-2

remove bincode

4e154f0

kianenigma added 2 commits February 25, 2021 15:46

Remove warning

280666f

Change test features

6ff2845

ghost merged commit 28107d4 into master Feb 26, 2021

ghost deleted the kiz-migration-testin-2 branch February 26, 2021 15:41

hackfisher mentioned this pull request Mar 15, 2021

Storage Name Checking Script darwinia-network/darwinia-common#314

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better identifier and logging for runtime upgrades #8123

Better identifier and logging for runtime upgrades #8123

kianenigma commented Feb 15, 2021 •

edited

Loading

gui1117 commented Feb 18, 2021

kianenigma commented Feb 18, 2021

kianenigma commented Feb 23, 2021

kianenigma commented Feb 23, 2021

bkchr left a comment

kianenigma commented Feb 23, 2021

gui1117 Feb 24, 2021

bkchr left a comment

kianenigma commented Feb 25, 2021

kianenigma commented Feb 25, 2021

ghost commented Feb 25, 2021

ghost commented Feb 25, 2021

kianenigma commented Feb 26, 2021

ghost commented Feb 26, 2021

Better identifier and logging for runtime upgrades #8123

Better identifier and logging for runtime upgrades #8123

Conversation

kianenigma commented Feb 15, 2021 • edited Loading

gui1117 commented Feb 18, 2021

kianenigma commented Feb 18, 2021

kianenigma commented Feb 23, 2021

kianenigma commented Feb 23, 2021

bkchr left a comment

Choose a reason for hiding this comment

kianenigma commented Feb 23, 2021

gui1117 Feb 24, 2021

Choose a reason for hiding this comment

bkchr left a comment

Choose a reason for hiding this comment

kianenigma commented Feb 25, 2021

kianenigma commented Feb 25, 2021

ghost commented Feb 25, 2021

ghost commented Feb 25, 2021

kianenigma commented Feb 26, 2021

ghost commented Feb 26, 2021

kianenigma commented Feb 15, 2021 •

edited

Loading