Enable pallet contracts storage migration #774
Conversation
shunsukew
changed the title
| @@ -195,6 +195,7 @@ runtime-benchmarks = [ | |||
| "pallet-xc-asset-config/runtime-benchmarks", | |||
| ] | |||
| try-runtime = [ | |||
| "log", | |||
There was a problem hiding this comment.
Isn't log 3rd party product? It shouldn't have any deps with try-runtime.
There was a problem hiding this comment.
I just would like to show storage version in pre and post runtime upgrade.
log::info!("Pre upgrade StorageVersion: {:?}", version);
Is there any reason? even though it won't be included in node binary and wasm.
There was a problem hiding this comment.
It will, this is just related to features. You'd have to make it optinal or a dev dependency for it to be excluded.
There was a problem hiding this comment.
log dependency is already optional
Astar/runtime/shiden/Cargo.toml
Line 11 in 9c0fae1
| @@ -57,6 +57,7 @@ pub use sp_consensus_aura::sr25519::AuthorityId as AuraId; | |||
| pub use sp_runtime::BuildStorage; | |||
|
|
|||
| mod chain_extensions; | |||
| mod migration; | |||
There was a problem hiding this comment.
Imo, no need for this. It's few lines of code. Just putting this where we usually place custom OnRuntimeUpgrade hooks is fine. We'll have to delete this anyhow later on.
There was a problem hiding this comment.
It's preference matter. I thought deleting a file can remove unnecessary stuffs later on, and it's clear to understand. Anywau, I'll stop using separated file.
There was a problem hiding this comment.
It's fine if it's your personal preference, no need to change.
| #[cfg(feature = "try-runtime")] | ||
| fn pre_upgrade() -> Result<Vec<u8>, &'static str> { | ||
| let version = StorageVersion::get::<pallet_contracts::Pallet<T>>(); | ||
| log::info!("Pre upgrade StorageVersion: {:?}", version); |
There was a problem hiding this comment.
In general, I'd advise using asserts in pre/post upgrade hooks. That way you'll get a clear fail when running it.
Pull Request Summary
In Shibuya and Shiden networks, pallet-contracts storage migration was missing. paritytech/substrate#12083
As a result of the commit history & current storage value investigation, we found out only Migration v8 needs to get executed with polkadot-v0.9.30 uplifts. https://github.com/paritytech/substrate/blob/a3ed0119c45cdd0d571ad34e5b3ee7518c8cef8d/frame/contracts/src/migration.rs#L58
This PR make sure pallet contracts
StorageVersionupdated to7(now it is0) before pallet contracts Migration, and only pallet contracts Migration v8 get executed with runtime upgrade.Check list