Skip to content

Enable pallet contracts storage migration #774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shunsukew merged 6 commits into from
Nov 2, 2022
Merged

Enable pallet contracts storage migration #774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b5d8fc
Enable pallet contracts runtime upgrade migration
shunsukew Oct 26, 2022
8197adc
cargo fmt
shunsukew Oct 26, 2022
fbe40f0
pallet contracts storage version migration
shunsukew Oct 30, 2022
9c0fae1
cargo fmt
shunsukew Oct 30, 2022
93e1d81
remove local chain storage migration
shunsukew Nov 2, 2022
e3ce231
up
shunsukew Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions runtime/astar/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ runtime-benchmarks = [
"pallet-xc-asset-config/runtime-benchmarks",
]
try-runtime = [
"log",
Copy link
Contributor

@Dinonard Dinonard Oct 27, 2022

Choose a reason for hiding this comment

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

Isn't log 3rd party product? It shouldn't have any deps with try-runtime.

Copy link
Contributor Author

@shunsukew shunsukew Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@Dinonard Dinonard Nov 1, 2022

Choose a reason for hiding this comment

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

It will, this is just related to features. You'd have to make it optinal or a dev dependency for it to be excluded.

Copy link
Contributor Author

@shunsukew shunsukew Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

log dependency is already optional

Astar/runtime/shiden/Cargo.toml

Line 11 in 9c0fae1

log = { version = "0.4.17", optional = true }

Copy link
Contributor

@Dinonard Dinonard Nov 2, 2022

Choose a reason for hiding this comment

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

Must have missed it. 👍

"frame-try-runtime",
"frame-executive/try-runtime",
"frame-support/try-runtime",
Expand Down
1 change: 1 addition & 0 deletions runtime/shibuya/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ runtime-benchmarks = [
"xcm-builder/runtime-benchmarks",
]
try-runtime = [
"log",
"frame-try-runtime",
"frame-executive/try-runtime",
"frame-support/try-runtime",
Expand Down
6 changes: 6 additions & 0 deletions runtime/shibuya/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub use sp_consensus_aura::sr25519::AuthorityId as AuraId;
pub use sp_runtime::BuildStorage;

mod chain_extensions;
mod migration;
Copy link
Contributor

@Dinonard Dinonard Nov 1, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@shunsukew shunsukew Nov 1, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@Dinonard Dinonard Nov 1, 2022

Choose a reason for hiding this comment

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

It's fine if it's your personal preference, no need to change.

mod precompiles;
mod weights;
mod xcm_config;
Expand All @@ -67,6 +68,7 @@ pub use precompiles::{ShibuyaNetworkPrecompiles, ASSET_PRECOMPILE_ADDRESS_PREFIX
pub type Precompiles = ShibuyaNetworkPrecompiles<Runtime, ShibuyaAssetLocationIdConverter>;

use chain_extensions::*;
use migration::*;

/// Constant values used within the runtime.
pub const MILLISDN: Balance = 1_000_000_000_000_000;
Expand Down Expand Up @@ -1121,6 +1123,10 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
(
ContractsStorageVersionMigration<Runtime>,
pallet_contracts::Migration<Runtime>,
),
>;

impl fp_self_contained::SelfContainedCall for RuntimeCall {
Expand Down
35 changes: 35 additions & 0 deletions runtime/shibuya/src/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use frame_support::{traits::StorageVersion, weights::Weight};
use sp_core::Get;
use sp_std::{marker::PhantomData, prelude::Vec, vec};

pub struct ContractsStorageVersionMigration<T: pallet_contracts::Config>(PhantomData<T>);

impl<T: pallet_contracts::Config> frame_support::traits::OnRuntimeUpgrade
for ContractsStorageVersionMigration<T>
{
fn on_runtime_upgrade() -> Weight {
let version = StorageVersion::get::<pallet_contracts::Pallet<T>>();
let mut weight = Weight::zero();

if version < 7 {
StorageVersion::new(7).put::<pallet_contracts::Pallet<T>>();
weight = weight.saturating_add(T::DbWeight::get().writes(1));
}

weight
}

#[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);
Ok(vec![])
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
let version = StorageVersion::get::<pallet_contracts::Pallet<T>>();
log::info!("Post upgrade StorageVersion: {:?}", version);
Ok(())
}
}
1 change: 1 addition & 0 deletions runtime/shiden/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ runtime-benchmarks = [
"pallet-xc-asset-config/runtime-benchmarks",
]
try-runtime = [
"log",
"frame-try-runtime",
"frame-executive/try-runtime",
"frame-support/try-runtime",
Expand Down
7 changes: 7 additions & 0 deletions runtime/shiden/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ pub use sp_consensus_aura::sr25519::AuthorityId as AuraId;
#[cfg(any(feature = "std", test))]
pub use sp_runtime::BuildStorage;

mod migration;
mod precompiles;
mod weights;
mod xcm_config;

use migration::*;

pub type ShidenAssetLocationIdConverter = AssetLocationIdConverter<AssetId, XcAssetConfig>;

pub use precompiles::{ShidenNetworkPrecompiles, ASSET_PRECOMPILE_ADDRESS_PREFIX};
Expand Down Expand Up @@ -871,6 +874,10 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
(
ContractsStorageVersionMigration<Runtime>,
pallet_contracts::Migration<Runtime>,
),
>;

impl fp_self_contained::SelfContainedCall for RuntimeCall {
Expand Down
35 changes: 35 additions & 0 deletions runtime/shiden/src/migration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
use frame_support::{traits::StorageVersion, weights::Weight};
use sp_core::Get;
use sp_std::{marker::PhantomData, prelude::Vec, vec};

pub struct ContractsStorageVersionMigration<T: pallet_contracts::Config>(PhantomData<T>);

impl<T: pallet_contracts::Config> frame_support::traits::OnRuntimeUpgrade
for ContractsStorageVersionMigration<T>
{
fn on_runtime_upgrade() -> Weight {
let version = StorageVersion::get::<pallet_contracts::Pallet<T>>();
let mut weight = Weight::zero();

if version < 7 {
StorageVersion::new(7).put::<pallet_contracts::Pallet<T>>();
weight = weight.saturating_add(T::DbWeight::get().writes(1));
}

weight
}

#[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);
Copy link
Contributor

@Dinonard Dinonard Nov 2, 2022

Choose a reason for hiding this comment

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

In general, I'd advise using asserts in pre/post upgrade hooks. That way you'll get a clear fail when running it.

Ok(vec![])
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
let version = StorageVersion::get::<pallet_contracts::Pallet<T>>();
log::info!("Post upgrade StorageVersion: {:?}", version);
Ok(())
}
}