From 7a23b2f83e1519c271690f17d83944b7e133810d Mon Sep 17 00:00:00 2001 From: Filip Bielejec Date: Wed, 8 Dec 2021 16:47:07 +0100 Subject: [PATCH] runtime storage migration in aleph pallet (#197) * changes to pallet storage + storage migration --- Cargo.lock | 3 +- bin/runtime/src/lib.rs | 2 +- common/Cargo.lock | 32 ++++++++++++++++- pallet/Cargo.toml | 6 ++-- pallet/src/lib.rs | 49 ++++++++++++------------- pallet/src/migrations/mod.rs | 1 + pallet/src/migrations/v0_to_v1.rs | 60 +++++++++++++++++++++++++++++++ pallet/src/mock.rs | 10 ++++-- pallet/src/tests.rs | 60 ++++++++++++++++++++++++++++++- 9 files changed, 186 insertions(+), 37 deletions(-) create mode 100644 pallet/src/migrations/mod.rs create mode 100644 pallet/src/migrations/v0_to_v1.rs diff --git a/Cargo.lock b/Cargo.lock index aec6fcebd1..baa8dd5898 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4518,7 +4518,7 @@ dependencies = [ [[package]] name = "pallet-aleph" -version = "0.1.0" +version = "0.2.0" dependencies = [ "frame-support", "frame-system", @@ -4529,6 +4529,7 @@ dependencies = [ "primitives", "serde", "sp-core", + "sp-io", "sp-runtime", "sp-std", ] diff --git a/bin/runtime/src/lib.rs b/bin/runtime/src/lib.rs index 52f54817d8..8b376a0bee 100644 --- a/bin/runtime/src/lib.rs +++ b/bin/runtime/src/lib.rs @@ -109,7 +109,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("aleph-node"), impl_name: create_runtime_str!("aleph-node"), authoring_version: 1, - spec_version: 5, + spec_version: 6, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 3, diff --git a/common/Cargo.lock b/common/Cargo.lock index 5f6c5ee782..c9f714dcfc 100644 --- a/common/Cargo.lock +++ b/common/Cargo.lock @@ -12,6 +12,34 @@ dependencies = [ "regex", ] +[[package]] +name = "ac-node-api" +version = "0.1.0" +source = "git+https://github.com/scs/substrate-api-client?rev=aa4b84f8775f972e5967aca3d96dd163619f65d5#aa4b84f8775f972e5967aca3d96dd163619f65d5" +dependencies = [ + "ac-primitives", + "frame-metadata 14.0.0 (git+https://github.com/paritytech/frame-metadata.git?branch=main)", + "frame-support", + "frame-system", + "hex", + "log", + "parity-scale-codec", + "serde", + "serde_json", + "sp-core", + "sp-runtime", + "thiserror", +] + +[[package]] +name = "ac-primitives" +version = "0.1.0" +source = "git+https://github.com/scs/substrate-api-client?rev=aa4b84f8775f972e5967aca3d96dd163619f65d5#aa4b84f8775f972e5967aca3d96dd163619f65d5" +dependencies = [ + "parity-scale-codec", + "sp-core", +] + [[package]] name = "addr2line" version = "0.17.0" @@ -2380,8 +2408,10 @@ dependencies = [ [[package]] name = "substrate-api-client" version = "0.6.0" -source = "git+https://github.com/scs/substrate-api-client?rev=f9d3a773bb5c76a0c0689b012fc08599beb90ab7#f9d3a773bb5c76a0c0689b012fc08599beb90ab7" +source = "git+https://github.com/scs/substrate-api-client?rev=aa4b84f8775f972e5967aca3d96dd163619f65d5#aa4b84f8775f972e5967aca3d96dd163619f65d5" dependencies = [ + "ac-node-api", + "ac-primitives", "frame-metadata 14.0.0 (git+https://github.com/paritytech/frame-metadata.git?branch=main)", "frame-support", "frame-system", diff --git a/pallet/Cargo.toml b/pallet/Cargo.toml index 9410a95f4a..80544c9de5 100644 --- a/pallet/Cargo.toml +++ b/pallet/Cargo.toml @@ -1,11 +1,9 @@ [package] name = "pallet-aleph" -version = "0.1.0" +version = "0.2.0" authors = ["Cardinal Cryptography"] edition = "2018" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } frame-support = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } @@ -14,9 +12,9 @@ sp-std = { default-features = false, git = "https://github.com/paritytech/substr serde = "1.0" primitives = { path = "../primitives", default-features = false} pallet-session = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } +sp-io = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } [dev-dependencies] - pallet-balances = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } pallet-timestamp = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } sp-runtime = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.9" } diff --git a/pallet/src/lib.rs b/pallet/src/lib.rs index 0a86731f21..3b9630444c 100644 --- a/pallet/src/lib.rs +++ b/pallet/src/lib.rs @@ -13,12 +13,20 @@ mod mock; #[cfg(test)] mod tests; +mod migrations; + use frame_support::Parameter; use sp_std::prelude::*; -use frame_support::{sp_runtime::BoundToRuntimeAppPublic, traits::OneSessionHandler}; +use frame_support::{ + sp_runtime::BoundToRuntimeAppPublic, + traits::{OneSessionHandler, StorageVersion}, +}; pub use pallet::*; +/// The current storage version. +const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[frame_support::pallet] pub mod pallet { use super::*; @@ -33,25 +41,13 @@ pub mod pallet { ApiError as AlephApiError, DEFAULT_MILLISECS_PER_BLOCK, DEFAULT_SESSION_PERIOD, }; - #[pallet::type_value] - pub fn DefaultValidators() -> Option> { - None - } - #[pallet::storage] #[pallet::getter(fn validators)] - pub type Validators = - StorageValue<_, Option>, ValueQuery, DefaultValidators>; - - #[pallet::type_value] - pub fn DefaultSessionForValidatorsChange() -> Option { - None - } + pub type Validators = StorageValue<_, Vec, OptionQuery>; #[pallet::storage] #[pallet::getter(fn session_for_validators_change)] - pub type SessionForValidatorsChange = - StorageValue<_, Option, ValueQuery, DefaultSessionForValidatorsChange>; + pub type SessionForValidatorsChange = StorageValue<_, u32, OptionQuery>; #[pallet::config] pub trait Config: frame_system::Config + pallet_session::Config { @@ -73,10 +69,15 @@ pub mod pallet { pub struct AlephSessionManager(sp_std::marker::PhantomData); #[pallet::pallet] - pub struct Pallet(sp_std::marker::PhantomData); + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(_); #[pallet::hooks] - impl Hooks> for Pallet {} + impl Hooks> for Pallet { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + migrations::v0_to_v1::migrate::() + } + } #[pallet::call] impl Pallet { @@ -87,8 +88,8 @@ pub mod pallet { session_for_validators_change: u32, ) -> DispatchResult { ensure_root(origin)?; - Validators::::put(Some(validators.clone())); - SessionForValidatorsChange::::put(Some(session_for_validators_change)); + Validators::::put(validators.clone()); + SessionForValidatorsChange::::put(session_for_validators_change); Self::deposit_event(Event::ChangeValidators( validators, session_for_validators_change, @@ -146,8 +147,6 @@ pub mod pallet { fn build(&self) { >::put(&self.session_period); >::put(&self.millisecs_per_block); - >::put(Some(&self.validators)); - >::put(Some(0)); } } @@ -180,11 +179,9 @@ pub mod pallet { Pallet::::session_for_validators_change() { if session_for_validators_change <= session { - let validators = Pallet::::validators().expect( - "Validators also should be Some(), when session_for_validators_change is", - ); - Validators::::put(None::>); - SessionForValidatorsChange::::put(None::); + let validators = Validators::::take() + .expect("When SessionForValidatorsChange is Some so should be Validators"); + let _ = SessionForValidatorsChange::::take().unwrap(); return Some(validators); } } diff --git a/pallet/src/migrations/mod.rs b/pallet/src/migrations/mod.rs new file mode 100644 index 0000000000..e32ac57514 --- /dev/null +++ b/pallet/src/migrations/mod.rs @@ -0,0 +1 @@ +pub mod v0_to_v1; diff --git a/pallet/src/migrations/v0_to_v1.rs b/pallet/src/migrations/v0_to_v1.rs new file mode 100644 index 0000000000..3868f3129a --- /dev/null +++ b/pallet/src/migrations/v0_to_v1.rs @@ -0,0 +1,60 @@ +use crate::Config; +use frame_support::log; +use frame_support::{ + traits::{Get, GetStorageVersion, PalletInfoAccess, StorageVersion}, + weights::Weight, +}; +use sp_std::prelude::*; + +frame_support::generate_storage_alias!( + Aleph, SessionForValidatorsChange => Value> +); + +frame_support::generate_storage_alias!( + Aleph, Validators => Value>> +); + +pub fn migrate() -> Weight { + let on_chain_storage_version =

::on_chain_storage_version(); + let current_storage_version =

::current_storage_version(); + + if on_chain_storage_version == StorageVersion::default() && current_storage_version == 1 { + log::info!(target: "pallet_aleph", "Running migration from STORAGE_VERSION 0 to 1"); + + let mut writes = 0; + match SessionForValidatorsChange::translate(sp_std::convert::identity) { + Ok(_) => { + writes += 1; + log::info!(target: "pallet_aleph", "Succesfully migrated storage for SessionForValidatorsChange"); + } + Err(why) => { + log::error!(target: "pallet_aleph", "Something went wrong during the migration of SessionForValidatorsChange {:?}", why); + } + }; + + match Validators::::translate(sp_std::convert::identity) { + Ok(_) => { + writes += 1; + log::info!(target: "pallet_aleph", "Succesfully migrated storage for Validators"); + } + Err(why) => { + log::error!(target: "pallet_aleph", "Something went wrong during the migration of Validators storage {:?}", why); + } + }; + + // store new version + StorageVersion::new(1).put::

(); + writes += 1; + + T::DbWeight::get().reads(3) + T::DbWeight::get().writes(writes) + } else { + log::warn!( + target: "pallet_aleph", + "Not applying any storage migration because on-chain storage version is {:?} and the version declared in the aleph pallet is {:?}", + on_chain_storage_version, + current_storage_version + ); + // I have only read the version + T::DbWeight::get().reads(1) + } +} diff --git a/pallet/src/mock.rs b/pallet/src/mock.rs index aa009ef4a9..5b1aa26662 100644 --- a/pallet/src/mock.rs +++ b/pallet/src/mock.rs @@ -3,13 +3,13 @@ use super::*; use crate as pallet_aleph; -use sp_core::H256; - use frame_support::{ construct_runtime, parameter_types, sp_io, traits::{OnFinalize, OnInitialize}, + weights::RuntimeDbWeight, }; use primitives::AuthorityId; +use sp_core::H256; use sp_runtime::{ impl_opaque_keys, testing::{Header, TestXt, UintAuthorityId}, @@ -48,6 +48,10 @@ parameter_types! { pub const BlockHashCount: u64 = 250; pub BlockWeights: frame_system::limits::BlockWeights = frame_system::limits::BlockWeights::simple_max(1024); + pub const TestDbWeight: RuntimeDbWeight = RuntimeDbWeight { + read: 25, + write: 100 + }; } impl frame_system::Config for Test { @@ -65,7 +69,7 @@ impl frame_system::Config for Test { type Header = Header; type Event = Event; type BlockHashCount = BlockHashCount; - type DbWeight = (); + type DbWeight = TestDbWeight; type Version = (); type PalletInfo = PalletInfo; type AccountData = pallet_balances::AccountData; diff --git a/pallet/src/tests.rs b/pallet/src/tests.rs index b8354d64f2..f66ff329e2 100644 --- a/pallet/src/tests.rs +++ b/pallet/src/tests.rs @@ -1,7 +1,64 @@ #![cfg(test)] -use crate::mock::*; +use crate::{migrations, mock::*, pallet}; use frame_support::assert_ok; +use frame_support::traits::{GetStorageVersion, StorageVersion}; + +#[test] +fn migration_from_v0_to_v1_works() { + new_test_ext(&[(1u64, 1u64), (2u64, 2u64)]).execute_with(|| { + frame_support::migration::put_storage_value( + b"Aleph", + b"SessionForValidatorsChange", + &[], + 1u32, + ); + + frame_support::migration::put_storage_value( + b"Aleph", + b"Validators", + &[], + vec![AccountId::default()], + ); + + let v0 = as GetStorageVersion>::on_chain_storage_version(); + + assert_eq!( + v0, + StorageVersion::default(), + "Storage version before applying migration should be default" + ); + + let _weight = migrations::v0_to_v1::migrate::(); + + let v1 = as GetStorageVersion>::on_chain_storage_version(); + + assert_ne!( + v1, + StorageVersion::default(), + "Storage version after applying migration should be incremented" + ); + + assert_eq!( + Aleph::session_for_validators_change(), + Some(1u32), + "Migration should preserve ongoing session change with respect to the session number" + ); + + assert_eq!( + Aleph::validators(), + Some(vec![AccountId::default()]), + "Migration should preserve ongoing session change with respect to the validators set" + ); + + let noop_weight = migrations::v0_to_v1::migrate::(); + assert_eq!( + noop_weight, + TestDbWeight::get().reads(1), + "Migration cannot be run twice" + ); + }) +} #[test] fn test_update_authorities() { @@ -38,6 +95,7 @@ fn test_change_validators() { 0 )); + assert_eq!(Aleph::session_for_validators_change(), Some(0)); assert_eq!(Aleph::validators(), Some(vec![AccountId::default()])); }); }