From 12dc5eff077057841241de345357b4774224b867 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 25 Feb 2020 11:14:52 +0100 Subject: [PATCH 01/24] Initial idea of `on_runtime_upgrade` --- frame/balances/src/lib.rs | 84 +------------------------------ frame/balances/src/migration.rs | 85 ++++++++++++++++++++++++++++++++ frame/executive/src/lib.rs | 5 +- primitives/runtime/src/traits.rs | 8 +++ 4 files changed, 98 insertions(+), 84 deletions(-) create mode 100644 frame/balances/src/migration.rs diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 4dbaf4b8a886d..0b34193f5b411 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -156,6 +156,7 @@ mod tests_composite; #[macro_use] mod tests; mod benchmarking; +mod migration; use sp_std::prelude::*; use sp_std::{cmp, result, mem, fmt::Debug, ops::BitOr, convert::Infallible}; @@ -516,13 +517,6 @@ decl_module! { let dest = T::Lookup::lookup(dest)?; >::transfer(&transactor, &dest, value, KeepAlive)?; } - - fn on_initialize() { - if !IsUpgraded::::get() { - IsUpgraded::::put(true); - Self::do_upgrade(); - } - } } } @@ -547,82 +541,6 @@ impl OldBalanceLock { impl, I: Instance> Module { // PRIVATE MUTABLES - // Upgrade from the pre-#4649 balances/vesting into the new balances. - pub fn do_upgrade() { - sp_runtime::print("Upgrading account balances..."); - // First, migrate from old FreeBalance to new Account. - // We also move all locks across since only accounts with FreeBalance values have locks. - // FreeBalance: map T::AccountId => T::Balance - for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { - let mut account = AccountData { free, ..Default::default() }; - // Locks: map T::AccountId => Vec - let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); - if let Some(locks) = old_locks { - let locks = locks.into_iter() - .map(|i| { - let (result, expiry) = i.upgraded(); - if expiry != T::BlockNumber::max_value() { - // Any `until`s that are not T::BlockNumber::max_value come from - // democracy and need to be migrated over there. - // Democracy: Locks get(locks): map T::AccountId => Option; - put_storage_value(b"Democracy", b"Locks", &hash, expiry); - } - result - }) - .collect::>(); - for l in locks.iter() { - if l.reasons == Reasons::All || l.reasons == Reasons::Misc { - account.misc_frozen = account.misc_frozen.max(l.amount); - } - if l.reasons == Reasons::All || l.reasons == Reasons::Fee { - account.fee_frozen = account.fee_frozen.max(l.amount); - } - } - put_storage_value(b"Balances", b"Locks", &hash, locks); - } - put_storage_value(b"Balances", b"Account", &hash, account); - } - // Second, migrate old ReservedBalance into new Account. - // ReservedBalance: map T::AccountId => T::Balance - for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - account.reserved = reserved; - put_storage_value(b"Balances", b"Account", &hash, account); - } - - // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock - // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the - // lock to something sensible. - // pub Vesting: map T::AccountId => Option; - for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); - locks.push(BalanceLock { - id: *b"vesting ", - amount: vesting.0.clone(), - reasons: Reasons::Misc, - }); - account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); - put_storage_value(b"Vesting", b"Vesting", &hash, vesting); - put_storage_value(b"Balances", b"Locks", &hash, locks); - put_storage_value(b"Balances", b"Account", &hash, account); - } - - for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { - let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); - let mut refs: system::RefCount = 0; - // The items in Kusama that would result in a ref count being incremented. - if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } - // We skip Recovered since it's being replaced anyway. - let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); - prefixed_hash.extend(&b":session:keys"[..]); - prefixed_hash.extend(&hash[..]); - if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } - if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } - put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); - } - } - /// Get the free balance of an account. pub fn free_balance(who: impl sp_std::borrow::Borrow) -> T::Balance { Self::account(who.borrow()).free diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs new file mode 100644 index 0000000000000..33549cc3b6fe7 --- /dev/null +++ b/frame/balances/src/migration.rs @@ -0,0 +1,85 @@ +use super::*; +use sp_runtime::traits::OnRuntimeUpgrade; + +impl, I: Instance> OnRuntimeUpgrade for Module { + + // Upgrade from the pre-#4649 balances/vesting into the new balances. + fn on_runtime_upgrade(version: T::Version) { + if version.spec_version < 225 { return }; + + IsUpgraded::::put(true); + + sp_runtime::print("Upgrading account balances..."); + // First, migrate from old FreeBalance to new Account. + // We also move all locks across since only accounts with FreeBalance values have locks. + // FreeBalance: map T::AccountId => T::Balance + for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { + let mut account = AccountData { free, ..Default::default() }; + // Locks: map T::AccountId => Vec + let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); + if let Some(locks) = old_locks { + let locks = locks.into_iter() + .map(|i| { + let (result, expiry) = i.upgraded(); + if expiry != T::BlockNumber::max_value() { + // Any `until`s that are not T::BlockNumber::max_value come from + // democracy and need to be migrated over there. + // Democracy: Locks get(locks): map T::AccountId => Option; + put_storage_value(b"Democracy", b"Locks", &hash, expiry); + } + result + }) + .collect::>(); + for l in locks.iter() { + if l.reasons == Reasons::All || l.reasons == Reasons::Misc { + account.misc_frozen = account.misc_frozen.max(l.amount); + } + if l.reasons == Reasons::All || l.reasons == Reasons::Fee { + account.fee_frozen = account.fee_frozen.max(l.amount); + } + } + put_storage_value(b"Balances", b"Locks", &hash, locks); + } + put_storage_value(b"Balances", b"Account", &hash, account); + } + // Second, migrate old ReservedBalance into new Account. + // ReservedBalance: map T::AccountId => T::Balance + for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + account.reserved = reserved; + put_storage_value(b"Balances", b"Account", &hash, account); + } + + // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock + // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the + // lock to something sensible. + // pub Vesting: map T::AccountId => Option; + for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); + locks.push(BalanceLock { + id: *b"vesting ", + amount: vesting.0.clone(), + reasons: Reasons::Misc, + }); + account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); + put_storage_value(b"Vesting", b"Vesting", &hash, vesting); + put_storage_value(b"Balances", b"Locks", &hash, locks); + put_storage_value(b"Balances", b"Account", &hash, account); + } + + for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { + let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); + let mut refs: system::RefCount = 0; + // The items in Kusama that would result in a ref count being incremented. + if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } + // We skip Recovered since it's being replaced anyway. + let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); + prefixed_hash.extend(&b":session:keys"[..]); + prefixed_hash.extend(&hash[..]); + if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } + if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } + put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); + } + } +} \ No newline at end of file diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index d7fecf45909c8..be729ef56d0f2 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -81,7 +81,7 @@ use sp_runtime::{ ApplyExtrinsicResult, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, - NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, + NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, OnRuntimeUpgrade, }, transaction_validity::TransactionValidity, }; @@ -110,6 +110,7 @@ impl< Context: Default, UnsignedValidator, AllModules: + OnRuntimeUpgrade + OnInitialize + OnFinalize + OffchainWorker + @@ -135,6 +136,7 @@ impl< Context: Default, UnsignedValidator, AllModules: + OnRuntimeUpgrade + OnInitialize + OnFinalize + OffchainWorker + @@ -176,6 +178,7 @@ where extrinsics_root: &System::Hash, digest: &Digest, ) { + >::on_runtime_upgrade(); >::initialize( block_number, parent_hash, diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index fef20c826af12..84e77a4ea96c1 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -345,6 +345,14 @@ pub trait OnInitialize { fn on_initialize(_n: BlockNumber) {} } +/// The runtime upgrade trait. Implementing this lets you express what should happen +/// when the runtime upgrades, and changes may need to occur to your module. +#[impl_for_tuples(30)] +pub trait OnRuntimeUpgrade { + /// The block is being initialized. Implement to have something happen. + fn on_runtime_upgrade(_v: RuntimeVersion) {} +} + /// Off-chain computation trait. /// /// Implementing this trait on a module allows you to perform long-running tasks From c0f524a44b6897d4322c034c1af7980c096e2efe Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 25 Feb 2020 15:01:07 +0100 Subject: [PATCH 02/24] Runtime storage for module version --- frame/balances/src/lib.rs | 6 +++--- frame/balances/src/migration.rs | 21 +++++++++++++-------- frame/executive/src/lib.rs | 6 +++--- primitives/runtime/src/traits.rs | 6 +++--- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 0b34193f5b411..9828e9afb61d8 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -367,10 +367,10 @@ decl_storage! { /// NOTE: Should only be accessed when setting, changing and freeing a lock. pub Locks get(fn locks): map hasher(blake2_256) T::AccountId => Vec>; - /// True if network has been upgraded to this version. + /// Module version number. /// - /// True for new networks. - IsUpgraded build(|_: &GenesisConfig| true): bool; + /// Default to version 1 for new networks. + ModuleVersion build(|_: &GenesisConfig| 1): u8; } add_extra_genesis { config(balances): Vec<(T::AccountId, T::Balance)>; diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index 33549cc3b6fe7..7d3a21b7e02f8 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -1,14 +1,15 @@ use super::*; use sp_runtime::traits::OnRuntimeUpgrade; -impl, I: Instance> OnRuntimeUpgrade for Module { - - // Upgrade from the pre-#4649 balances/vesting into the new balances. - fn on_runtime_upgrade(version: T::Version) { - if version.spec_version < 225 { return }; - - IsUpgraded::::put(true); +impl, I: Instance> OnRuntimeUpgrade for Module { + // Upgrade from the pre-#4649 balances/vesting into the new balances. + fn on_runtime_upgrade() { + let current_version = ModuleVersion::::get(); + if current_version == 1 { + return + } else if current_version == 0 { + ModuleVersion::::put(1); sp_runtime::print("Upgrading account balances..."); // First, migrate from old FreeBalance to new Account. // We also move all locks across since only accounts with FreeBalance values have locks. @@ -81,5 +82,9 @@ impl, I: Instance> OnRuntimeUpgrade for Module { if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); } + + // Recursively call the upgrade function to apply all upgrades. + Self::on_runtime_upgrade() } -} \ No newline at end of file + } +} diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index be729ef56d0f2..3ea1606b6789d 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -110,7 +110,7 @@ impl< Context: Default, UnsignedValidator, AllModules: - OnRuntimeUpgrade + + OnRuntimeUpgrade + OnInitialize + OnFinalize + OffchainWorker + @@ -136,7 +136,7 @@ impl< Context: Default, UnsignedValidator, AllModules: - OnRuntimeUpgrade + + OnRuntimeUpgrade + OnInitialize + OnFinalize + OffchainWorker + @@ -178,7 +178,7 @@ where extrinsics_root: &System::Hash, digest: &Digest, ) { - >::on_runtime_upgrade(); + ::on_runtime_upgrade(); >::initialize( block_number, parent_hash, diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 84e77a4ea96c1..0932f4e239d00 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -348,9 +348,9 @@ pub trait OnInitialize { /// The runtime upgrade trait. Implementing this lets you express what should happen /// when the runtime upgrades, and changes may need to occur to your module. #[impl_for_tuples(30)] -pub trait OnRuntimeUpgrade { - /// The block is being initialized. Implement to have something happen. - fn on_runtime_upgrade(_v: RuntimeVersion) {} +pub trait OnRuntimeUpgrade { + /// Perform a module upgrade. + fn on_runtime_upgrade() {} } /// Off-chain computation trait. From 8e8d58859b0e8f44af395530e1d56f0f8e1bc76a Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:59:29 +0200 Subject: [PATCH 03/24] Gui shawntabrizi runtime upgrade (#5118) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * adding unleash to ci (#5020) * adding unleash to ci * fixing formatting * with a dot please * alpha.3 now * do not publish testing helpers * remove old test-helpers cruft * fix cargo.lock * with alpha 4 * do not publish runtime-interface-test either * disable more test crates from publishing * switch to alpha.5 * replace tempdir with tempfile * update lru * switch to bytes 0.5 * release script fixes * switch on and to latest alpha * BUT THE SPACES * Fix: CI failing for some CLI tests (#5043) * Initial commit Forked at: 41bb2193a267805e2093a081bc3e2aaccc64283a Parent branch: origin/master * Increase killing grace period of CLI tests and display more info * Use --dev everywhere possible * Put pruning mode to its own params struct * Add pruning params to export-blocks command * Added missing file * Removed not-dev mode in tests * Add pruning mode to the revert command * Decrease killing grace period again * Move back unsafe_pruning to import_params * Applied proposed changes * aura: remove unused tx pool (#5046) * aura: remove unused transaction pool parameter * node-template: remove transaction pool from aura * aura: fix tests * Extend rust nightly detection in `wasm-builder` (#5021) Instead of just testing `cargo` and `rustup run nightly`, we now test the `CARGO` env variable and also scan non default nightlies. The user is also now able to select the toolchain with `WASM_BUILD_TOOLCHAIN`. * Add steps setting to benchmarking CLI (#5033) * Add steps setting to CLI, use max value to hit worst case. * Bump impl_version. * Apply review suggestion. * Remove indices from node-template (#5025) * Remove indices from node-template * Use identity lookup instead * Bump impl * clean cargo.toml * Fix documentation for "BlockBuilder::push_trusted" (#5051) * fix doc * rephrase * do not check unleash on every PR, only master and tags (#5054) * do not check unleash on every PR, only master and tags * move scripts folder * add signed-tag check to CI * remove publish-to-crates-io dependencies Co-authored-by: s3krit * prepare version to alpha.1 (#5055) bump version to -alpha.1 * Sync: validate block responses for required data (#5052) * Less verbose state-db logging * Validate block responses for block bodies * Update client/network/src/protocol.rs Co-Authored-By: Bastian Köcher * Added validation test * Disconnect on missing header as well * Typo Co-Authored-By: André Silva Co-authored-by: Bastian Köcher Co-authored-by: André Silva * Make these chainspecs fields private (#5031) * Fix dockerfile (#5059) * Adds documentation for `wipe` and `commit` (#5053) * Adds documentation for `wipe` and `commit` This adds documentation to `wipe` and `commit` of `Externalities`. Besides that it removes the default implementation that would just panic and requires that all implementers of the trait implement the functions. * Update primitives/externalities/src/lib.rs Co-Authored-By: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Fix the issue with `trybuild`'s `ui` tests (#4992) * repro ui bug * fix the tests * test with the new image * test without CARGO_HOME * test without fixes * test again * fix trybuild old versions * bump CArgo.lock * fix trybuild newest versions * bump Cargo.lock * trying on the latest image * bump Cargo.lock * run with the old image * ci will be green on the image from 2020-02-19 [skip ci] * bump Cargo.lock * Activate publishing of draft releases... (#5062) * Activate publishing of draft releases... ... And fix the message sending (missing parameter). * publish_draft_release.sh now checks latest... ... release on github rather than just a tag * Fix/div by zero (#5041) * Handle gas_price being zero separately * Bump spec_version * Add a unit & integration tests for gas price = 0 * set missing metadata fields, prepping alpha.2 (#5067) * setting first batch of descriptions * fix what I just broke * next batch * and pallets, too * last batch * set cargo.lock * keep'em dev-deps * bump version to alpha.2 * Fix revalidation not revalidating multiple times (#5065) * removes use of sc_client::Client from sc_finality_grandpa (#5030) * removes use of sc_client::Client from sc_finality_grandpa * code formatting * code formatting * removes use of sc_client::Client from sc_finality_grandpa * Remove deprecated host functions (#5038) Sadly we need to keep one function `ext_blake2_256`. This function is manually defined in `sp-core`. * removes use of sc_client::Client from sc_basic_authorship (#5050) * removes use of sc-client from sc-basic-authorship * refactor use of ProposerFactory * correct dep path * pallet-transaction-payment clean up (#5070) * Formatting clean up * Introduce separate setters for the fees. * *: Rename prometheus-exporter crate to substrate-prometheus-end… (#5076) This patch renames the crate for the following two reasons: 1. The prometheus-exporter crate introduces native in-process Prometheus style instrumentation to the Substrate project. Within the Prometheus ecosystem the term "exporter" is used for external processes exposing metrics for e.g. the Linux Kernel. In-process exposition would be described via the term "endpoint". 2. "prometheus-exporter" is generic and ignores the fact that it is only usable within the context of Substrate. In addition the name "prometheus-exporter" is already taken on crates.io. * rename `browser-utils` to `substrate-browser-utils` (#5079) * prepping for Alpha.3 (#5080) * Bump to alpha.3 * update gitlab-ci * Propagate DispatchError for benchmarks. (#5075) * Propagate DispatchError for benchmarks. * Apply review suggestions. * Use RuntimeString. * fix expect Co-Authored-By: Bastian Köcher Co-authored-by: Bastian Köcher * Add options to overwrite range bounds in benchmark command. (#5072) * Add --mins --maxs to benchmark command. * Apply review suggestions. * Update yamux to version 0.4.4. (#5086) * Remove more instances of futures01 (#4633) * Start removing last few instances of futures01 * Use to_poll on wasm * Revert "Use to_poll on wasm" This reverts commit 1c61728f10d520df5f9b28c415a0db68e478b9c7. * Fix fg test * Upgrade network test futures * Update offchain hyper version * Update service test * bump tokio to 0.2.10 * Removed some unneeded tokios * fixes * fix run_until_all_full * Make service test debuggable * Update client/offchain/src/api/http.rs Co-Authored-By: Demi Obenour <48690212+DemiMarie-parity@users.noreply.github.com> * Add service_test to test-int output * nitpicking * Finally fix test * Give up and revert client/serviec/test * Revert gitlab ci too Co-authored-by: Demi Obenour * Make export blocks default to json on stdout (#5090) * Make export blocks default to json on stdout * Multiline instead of single line to stay under 100 cols * Change --json flag to --binary, defaulting to json * Offence reporting returns a result (#5082) * Offence reporting returns a result * Bump spec_version * Use unwrap instead of assertions * Fix more review grumbles * Update to libp2p 0.16.2 (#5088) * Remove request ID from the new protocol (#5049) * Make sure we remove a peer on disconnect in gossip (#5104) * Make sure we remove peers on disconnect in gossip state machine * Clear up the code * Add a comment * Expose `state-db` memory info (#5110) This exposes memory statistics from the state-db. * Change extrinsic_count to extrinsic_index in pallet-utility (#5044) Co-authored-by: Benjamin Kampmann * Add more metrics to prometheus (#5034) * Add a few things * Add finality_grandpa_round * fix fg tests * Nitpicks * Nitpicks * Fix name of prometheus crate * Update to SCALE 1.2.0 (#5113) This updates `parity-scale-codec` to `1.2.0`, which includes multiple performance improvements and a fix that bounds the capacity of a vector at decoding. * Lazy payouts (#4474) * TODOs * Remove superfluous: * partial implementation * full implementation * fix preferences * update comments * upgrade test WIP * fix more tests * fix cutoff * fix saturation * comment * upgrade mock * upgrade test * WIP migration * WIP migration * remove slot stake stuff * fix merge * migration of ledger * remove equalize from test * add test * fix * update doc * fix compilation * improve test readibility * improve doc * fix most todo * fix migration and test * remove println * WIP * add test and spec * weight * update doc * safer end_era * fix exposure of conversion * Revert "safer end_era" This reverts commit 72ff737d27be67d87308514b13e2574bc5f09fce. * fix useless put * exposure clipped * doc * fix payout with clipped * fix node runtime * add doc * pluggable and generalized staking module * remove print * update doc * refactor * improve documentation and implementation * fix test * Fix test * fix test * fix test * fix remove lowest stake from exposure, not biggest. * nomination index arguments in nominator_payout * add test * try to fix offence * apply slashed and bond eras until active era * doc * update spec version * add test upgrade from previous test environment * Apply suggestions from code review Co-Authored-By: Shawn Tabrizi * nominators upgrade has been cleaned * dynamic history depth implementation * make current_era - history_depth included * Change equality check to start era to less than or equal * Use era specific validator prefs * Add print statement and comment about start era if < * fix next_reward overflow * make more check for bad era claim for zero cost * small refactor * code refactor + fix use of deprecated storage * fix wasm build * add comment * Fix tests * remove outdated comment * Apply suggestions from code review Co-Authored-By: Shawn Tabrizi * gather active era information into one storage Co-authored-by: thiolliere Co-authored-by: Shawn Tabrizi * impl on_runtime_upgrade Co-authored-by: Benjamin Kampmann Co-authored-by: Cecile Tonglet Co-authored-by: André Silva Co-authored-by: Bastian Köcher Co-authored-by: Marcio Diaz Co-authored-by: Nikolay Volf Co-authored-by: s3krit Co-authored-by: Arkadiy Paronyan Co-authored-by: Pierre Krieger Co-authored-by: Chevdor Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Denis Pisarev Co-authored-by: Eric Co-authored-by: Seun Lanlege Co-authored-by: Sergei Pepyakin Co-authored-by: Max Inden Co-authored-by: Ashley Co-authored-by: Toralf Wittner Co-authored-by: Demi Obenour Co-authored-by: pscott <30843220+pscott@users.noreply.github.com> Co-authored-by: Fedor Sakharov Co-authored-by: Gavin Wood Co-authored-by: thiolliere --- frame/executive/src/lib.rs | 7 ++ frame/staking/src/lib.rs | 5 +- frame/support/src/dispatch.rs | 168 ++++++++++++++++++++++++++++++++++ frame/support/src/weights.rs | 10 ++ frame/system/src/lib.rs | 5 + 5 files changed, 194 insertions(+), 1 deletion(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 3ea1606b6789d..c36ce36b83069 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -186,6 +186,13 @@ where digest, frame_system::InitKind::Full, ); + let last_runtime_upgrade = >::last_runtime_upgrade(); + if last_runtime_upgrade.map(|n| n == *block_number).unwrap_or(false) { + ::on_runtime_upgrade(); + >::register_extra_weight_unchecked( + >::on_runtime_upgrade() + ); + } >::on_initialize(*block_number); >::register_extra_weight_unchecked( >::on_initialize(*block_number) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 3609191f4cb2c..e6afdd421190a 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -927,10 +927,13 @@ decl_module! { fn deposit_event() = default; - fn on_initialize() { + fn on_runtime_upgrade() { Self::ensure_storage_upgraded(); } + fn on_initialize() { + } + fn on_finalize() { // Set the start of the first era. if let Some(mut active_era) = Self::active_era() { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 084ea285af464..3c13de11be4a3 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -196,6 +196,10 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// /// The following reserved functions also take the block number (with type `T::BlockNumber`) as an optional input: /// +/// * `on_runtime_upgrade`: Executes at the beginning of a block prior to on_initialize when there +/// is a runtime ugprade. This allow each module to upgrade their storage, calling other module +/// must be avoided. Using this function will implement the +/// [`OnInitialize`](../sp_runtime/traits/trait.OnInitialize.html) trait. /// * `on_initialize`: Executes at the beginning of a block. Using this function will /// implement the [`OnInitialize`](../sp_runtime/traits/trait.OnInitialize.html) trait. /// * `on_finalize`: Executes at the end of a block. Using this function will @@ -229,6 +233,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -261,6 +266,7 @@ macro_rules! decl_module { {} {} {} + {} [] $($t)* ); @@ -274,6 +280,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } {} { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -290,6 +297,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $vis fn deposit_event() = default; } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -305,6 +313,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } {} { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -326,6 +335,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } {} { $( $offchain:tt )* } { $( $constants:tt )* } @@ -342,6 +352,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { #[weight = $crate::dispatch::SimpleDispatchInfo::zero()] fn on_finalize( $( $param_name : $param ),* ) { $( $impl )* } @@ -361,6 +372,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } {} { $( $offchain:tt )* } { $( $constants:tt )* } @@ -378,6 +390,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { #[weight = $weight] fn on_finalize( $( $param_name : $param ),* ) { $( $impl )* } @@ -389,6 +402,85 @@ macro_rules! decl_module { $($rest)* ); }; + // Add on_runtime_upgrade, without a given weight. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + {} + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type<$trait_instance: $trait_name$(, I: $instantiable $(= $module_default_instance)?)?> + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { + #[weight = $crate::dispatch::SimpleDispatchInfo::zero()] + fn on_runtime_upgrade( $( $param_name : $param ),* ) { $( $impl )* } + } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { $( $error_type )* } + [ $( $dispatchables )* ] + $($rest)* + ); + }; + // Add on_runtime_upgrade, given weight. + (@normalize + $(#[$attr:meta])* + pub struct $mod_type:ident< + $trait_instance:ident: $trait_name:ident$(, I: $instantiable:path $(= $module_default_instance:path)?)? + > + for enum $call_type:ident where origin: $origin_type:ty, system = $system:ident + { $( $other_where_bounds:tt )* } + { $( $deposit_event:tt )* } + { $( $on_initialize:tt )* } + {} + { $( $on_finalize:tt )* } + { $( $offchain:tt )* } + { $( $constants:tt )* } + { $( $error_type:tt )* } + [ $( $dispatchables:tt )* ] + $(#[doc = $doc_attr:tt])* + #[weight = $weight:expr] + fn on_runtime_upgrade( $( $param_name:ident : $param:ty ),* $(,)? ) { $( $impl:tt )* } + $($rest:tt)* + ) => { + $crate::decl_module!(@normalize + $(#[$attr])* + pub struct $mod_type<$trait_instance: $trait_name$(, I: $instantiable $(= $module_default_instance)?)?> + for enum $call_type where origin: $origin_type, system = $system + { $( $other_where_bounds )* } + { $( $deposit_event )* } + { $( $on_initialize )* } + { + #[weight = $weight] + fn on_runtime_upgrade( $( $param_name : $param ),* ) { $( $impl )* } + } + { $( $on_finalize )* } + { $( $offchain )* } + { $( $constants )* } + { $( $error_type )* } + [ $( $dispatchables )* ] + $($rest)* + ); + }; // Add on_initialize, without a given weight. (@normalize $(#[$attr:meta])* @@ -399,6 +491,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } {} + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -418,6 +511,7 @@ macro_rules! decl_module { #[weight = $crate::dispatch::SimpleDispatchInfo::zero()] fn on_initialize( $( $param_name : $param ),* ) { $( $impl )* } } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -436,6 +530,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } {} + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -456,6 +551,7 @@ macro_rules! decl_module { #[weight = $weight] fn on_initialize( $( $param_name : $param ),* ) { $( $impl )* } } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -474,6 +570,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { } { $( $constants:tt )* } @@ -492,6 +589,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { fn offchain_worker( $( $param_name : $param ),* ) { $( $impl )* } } { $( $constants )* } @@ -512,6 +610,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -531,6 +630,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { @@ -555,6 +655,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -573,6 +674,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -592,6 +694,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -608,6 +711,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -628,6 +732,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -649,6 +754,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -676,6 +782,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -696,6 +803,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -717,6 +825,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -742,6 +851,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -767,6 +877,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -793,6 +904,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -808,6 +920,7 @@ macro_rules! decl_module { { $( $other_where_bounds )* } { $( $deposit_event )* } { $( $on_initialize )* } + { $( $on_runtime_upgrade )* } { $( $on_finalize )* } { $( $offchain )* } { $( $constants )* } @@ -903,6 +1016,39 @@ macro_rules! decl_module { {} }; + (@impl_on_runtime_upgrade + $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + #[weight = $weight:expr] + fn on_runtime_upgrade() { $( $impl:tt )* } + ) => { + impl<$trait_instance: $trait_name$(, $instance: $instantiable)?> + $crate::sp_runtime::traits::OnRuntimeUpgrade + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + { + fn on_runtime_upgrade() { + use $crate::sp_std::if_std; + if_std! { + use $crate::tracing; + let span = tracing::span!(tracing::Level::DEBUG, "on_runtime_upgrade"); + let _enter = span.enter(); + } + { $( $impl )* } + } + } + }; + + (@impl_on_runtime_upgrade + $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; + { $( $other_where_bounds:tt )* } + ) => { + impl<$trait_instance: $trait_name$(, $instance: $instantiable)?> + $crate::sp_runtime::traits::OnRuntimeUpgrade + for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* + {} + }; + + (@impl_on_finalize $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; { $( $other_where_bounds:tt )* } @@ -961,6 +1107,10 @@ macro_rules! decl_module { (@impl_block_hooks_weight $module:ident<$trait_instance:ident: $trait_name:ident$(, $instance:ident: $instantiable:path)?>; { $( $other_where_bounds:tt )* } + @runtime_upgrade $( + #[weight = $weight_runtime_update:expr] + fn on_runtime_upgrade($( $param_runtime_upgrade:ident : $param_ty_runtime_upgrade:ty )*) { $( $impl_runtime_upgrade:tt )* } + )? @init $( #[weight = $weight_initialize:expr] fn on_initialize($( $param_initialize:ident : $param_ty_initialize:ty )*) { $( $impl_initialize:tt )* } @@ -974,6 +1124,11 @@ macro_rules! decl_module { $crate::dispatch::WeighBlock<$trait_instance::BlockNumber> for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )* { + $( + fn on_runtime_upgrade() -> $crate::dispatch::Weight { + >::weigh_data(&$weight_initialize, ()) + } + )? $( fn on_initialize(n: $trait_instance::BlockNumber) -> $crate::dispatch::Weight { >::weigh_data(&$weight_initialize, n) @@ -1208,6 +1363,7 @@ macro_rules! decl_module { { $( $other_where_bounds:tt )* } { $( $deposit_event:tt )* } { $( $on_initialize:tt )* } + { $( $on_runtime_upgrade:tt )* } { $( $on_finalize:tt )* } { $( $offchain:tt )* } { $( $constants:tt )* } @@ -1231,6 +1387,14 @@ macro_rules! decl_module { $( $on_initialize )* } + $crate::decl_module! { + @impl_on_runtime_upgrade + $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; + { $( $other_where_bounds )* } + $( $on_runtime_upgrade )* + } + + $crate::decl_module! { @impl_on_finalize $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; @@ -1242,6 +1406,7 @@ macro_rules! decl_module { @impl_block_hooks_weight $mod_type<$trait_instance: $trait_name $(, $instance: $instantiable)?>; { $( $other_where_bounds )* } + @runtime_upgrade $( $on_runtime_upgrade )* @init $( $on_initialize )* @fin $( $on_finalize )* } @@ -1869,6 +2034,9 @@ macro_rules! __check_reserved_fn_name { (on_initialize $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error on_initialize); }; + (on_runtime_upgrade $( $rest:ident )*) => { + $crate::__check_reserved_fn_name!(@compile_error on_runtime_upgrade); + }; (on_initialise $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error_renamed on_initialise on_initialize); }; diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index c46cca683ba93..883aed693ddda 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -70,6 +70,8 @@ pub trait ClassifyDispatch { /// Means of determining the weight of a block's life cycle hooks: `on_initialize`, `on_finalize` and /// such. pub trait WeighBlock { + /// Return the weight of the block's on_runtime_upgrade hook. + fn on_runtime_upgrade() -> Weight { Zero::zero() } /// Return the weight of the block's on_initialize hook. fn on_initialize(_: BlockNumber) -> Weight { Zero::zero() } /// Return the weight of the block's on_finalize hook. @@ -87,6 +89,14 @@ pub trait PaysFee { /// Maybe I can do something to remove the duplicate code here. #[impl_for_tuples(30)] impl WeighBlock for SingleModule { + fn on_runtime_upgrade() -> Weight { + let mut accumulated_weight: Weight = Zero::zero(); + for_tuples!( + #( accumulated_weight = accumulated_weight.saturating_add(SingleModule::on_runtime_upgrade()); )* + ); + accumulated_weight + } + fn on_initialize(n: BlockNumber) -> Weight { let mut accumulated_weight: Weight = Zero::zero(); for_tuples!( diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index f89c9efbd2962..7968e11082131 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -367,6 +367,9 @@ decl_storage! { /// the `EventIndex` then in case if the topic has the same contents on the next block /// no notification will be triggered thus the event might be lost. EventTopics get(fn event_topics): map hasher(blake2_256) T::Hash => Vec<(T::BlockNumber, EventIndex)>; + + /// Store the block number of last runtime upgrade. + LastRuntimeUpgrade get(fn last_runtime_upgrade): Option; } add_extra_genesis { config(changes_trie_config): Option; @@ -486,6 +489,7 @@ decl_module! { } storage::unhashed::put_raw(well_known_keys::CODE, &code); + >::put(Self::block_number()); Self::deposit_event(RawEvent::CodeUpdated); } @@ -494,6 +498,7 @@ decl_module! { pub fn set_code_without_checks(origin, code: Vec) { ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::CODE, &code); + >::put(Self::block_number()); Self::deposit_event(RawEvent::CodeUpdated); } From 731ea6cdd83c00bc99fc6349a4926d5665c1f2a5 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:10:29 +0100 Subject: [PATCH 04/24] make compile --- frame/balances/src/lib.rs | 4 + frame/balances/src/migration.rs | 153 ++++++++++++++++---------------- 2 files changed, 79 insertions(+), 78 deletions(-) diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 9828e9afb61d8..3033d4b6bb176 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -517,6 +517,10 @@ decl_module! { let dest = T::Lookup::lookup(dest)?; >::transfer(&transactor, &dest, value, KeepAlive)?; } + + fn on_runtime_upgrade() { + migration::on_runtime_upgrade::(); + } } } diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index 7d3a21b7e02f8..ff25d130bb948 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -1,90 +1,87 @@ use super::*; -use sp_runtime::traits::OnRuntimeUpgrade; -impl, I: Instance> OnRuntimeUpgrade for Module { - // Upgrade from the pre-#4649 balances/vesting into the new balances. - fn on_runtime_upgrade() { - let current_version = ModuleVersion::::get(); +// Upgrade from the pre-#4649 balances/vesting into the new balances. +pub fn on_runtime_upgrade, I: Instance>() { + let current_version = ModuleVersion::::get(); - if current_version == 1 { - return - } else if current_version == 0 { - ModuleVersion::::put(1); - sp_runtime::print("Upgrading account balances..."); - // First, migrate from old FreeBalance to new Account. - // We also move all locks across since only accounts with FreeBalance values have locks. - // FreeBalance: map T::AccountId => T::Balance - for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { - let mut account = AccountData { free, ..Default::default() }; - // Locks: map T::AccountId => Vec - let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); - if let Some(locks) = old_locks { - let locks = locks.into_iter() - .map(|i| { - let (result, expiry) = i.upgraded(); - if expiry != T::BlockNumber::max_value() { - // Any `until`s that are not T::BlockNumber::max_value come from - // democracy and need to be migrated over there. - // Democracy: Locks get(locks): map T::AccountId => Option; - put_storage_value(b"Democracy", b"Locks", &hash, expiry); - } - result - }) - .collect::>(); - for l in locks.iter() { - if l.reasons == Reasons::All || l.reasons == Reasons::Misc { - account.misc_frozen = account.misc_frozen.max(l.amount); - } - if l.reasons == Reasons::All || l.reasons == Reasons::Fee { - account.fee_frozen = account.fee_frozen.max(l.amount); + if current_version == 1 { + return + } else if current_version == 0 { + ModuleVersion::::put(1); + sp_runtime::print("Upgrading account balances..."); + // First, migrate from old FreeBalance to new Account. + // We also move all locks across since only accounts with FreeBalance values have locks. + // FreeBalance: map T::AccountId => T::Balance + for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { + let mut account = AccountData { free, ..Default::default() }; + // Locks: map T::AccountId => Vec + let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); + if let Some(locks) = old_locks { + let locks = locks.into_iter() + .map(|i| { + let (result, expiry) = i.upgraded(); + if expiry != T::BlockNumber::max_value() { + // Any `until`s that are not T::BlockNumber::max_value come from + // democracy and need to be migrated over there. + // Democracy: Locks get(locks): map T::AccountId => Option; + put_storage_value(b"Democracy", b"Locks", &hash, expiry); } + result + }) + .collect::>(); + for l in locks.iter() { + if l.reasons == Reasons::All || l.reasons == Reasons::Misc { + account.misc_frozen = account.misc_frozen.max(l.amount); + } + if l.reasons == Reasons::All || l.reasons == Reasons::Fee { + account.fee_frozen = account.fee_frozen.max(l.amount); } - put_storage_value(b"Balances", b"Locks", &hash, locks); } - put_storage_value(b"Balances", b"Account", &hash, account); - } - // Second, migrate old ReservedBalance into new Account. - // ReservedBalance: map T::AccountId => T::Balance - for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - account.reserved = reserved; - put_storage_value(b"Balances", b"Account", &hash, account); - } - - // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock - // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the - // lock to something sensible. - // pub Vesting: map T::AccountId => Option; - for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); - locks.push(BalanceLock { - id: *b"vesting ", - amount: vesting.0.clone(), - reasons: Reasons::Misc, - }); - account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); - put_storage_value(b"Vesting", b"Vesting", &hash, vesting); put_storage_value(b"Balances", b"Locks", &hash, locks); - put_storage_value(b"Balances", b"Account", &hash, account); - } - - for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { - let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); - let mut refs: system::RefCount = 0; - // The items in Kusama that would result in a ref count being incremented. - if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } - // We skip Recovered since it's being replaced anyway. - let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); - prefixed_hash.extend(&b":session:keys"[..]); - prefixed_hash.extend(&hash[..]); - if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } - if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } - put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); } + put_storage_value(b"Balances", b"Account", &hash, account); + } + // Second, migrate old ReservedBalance into new Account. + // ReservedBalance: map T::AccountId => T::Balance + for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + account.reserved = reserved; + put_storage_value(b"Balances", b"Account", &hash, account); + } - // Recursively call the upgrade function to apply all upgrades. - Self::on_runtime_upgrade() + // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock + // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the + // lock to something sensible. + // pub Vesting: map T::AccountId => Option; + for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); + locks.push(BalanceLock { + id: *b"vesting ", + amount: vesting.0.clone(), + reasons: Reasons::Misc, + }); + account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); + put_storage_value(b"Vesting", b"Vesting", &hash, vesting); + put_storage_value(b"Balances", b"Locks", &hash, locks); + put_storage_value(b"Balances", b"Account", &hash, account); } + + for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { + let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); + let mut refs: system::RefCount = 0; + // The items in Kusama that would result in a ref count being incremented. + if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } + // We skip Recovered since it's being replaced anyway. + let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); + prefixed_hash.extend(&b":session:keys"[..]); + prefixed_hash.extend(&hash[..]); + if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } + if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } + put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); + } + + // Recursively call the upgrade function to apply all upgrades. + on_runtime_upgrade::() } } From a36e3abd882df78ee8a1d4d818c08677284bac22 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:30:08 +0100 Subject: [PATCH 05/24] Add some tests --- frame/support/src/dispatch.rs | 15 +++++-- .../invalid_module_details.stderr | 2 +- .../missing_where_block.stderr | 2 +- .../missing_where_param.stderr | 2 +- .../tests/reserved_keyword/on_initialize.rs | 2 +- .../reserved_keyword/on_initialize.stderr | 44 +++++++++++-------- 6 files changed, 41 insertions(+), 26 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 3c13de11be4a3..12d385b81072d 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2082,7 +2082,7 @@ macro_rules! __check_reserved_fn_name { #[allow(dead_code)] mod tests { use super::*; - use crate::sp_runtime::traits::{OnInitialize, OnFinalize}; + use crate::sp_runtime::traits::{OnInitialize, OnFinalize, OnRuntimeUpgrade}; use crate::weights::{DispatchInfo, DispatchClass}; use crate::traits::{CallMetadata, GetCallMetadata, GetCallName}; @@ -2104,8 +2104,8 @@ mod tests { } } - struct BLockWeight; - impl> WeighData for BLockWeight { + struct BlockWeight; + impl> WeighData for BlockWeight { fn weigh_data(&self, target: BlockNumber) -> Weight { let target: u32 = target.into(); if target % 2 == 0 { 10 } else { 0 } @@ -2125,8 +2125,10 @@ mod tests { #[weight = SimpleDispatchInfo::FixedNormal(7)] fn on_initialize(n: T::BlockNumber,) { if n.into() == 42 { panic!("on_initialize") } } - #[weight = BLockWeight] + #[weight = BlockWeight] fn on_finalize(n: T::BlockNumber) { if n.into() == 42 { panic!("on_finalize") } } + #[weight = SimpleDispatchInfo::FixedOperational(69)] + fn on_runtime_upgrade() { } fn offchain_worker() {} #[weight = SimpleDispatchInfo::FixedOperational(5)] @@ -2268,6 +2270,11 @@ mod tests { as OnFinalize>::on_finalize(42); } + #[test] + fn on_runtime_upgrade_should_work() { + as OnRuntimeUpgrade>::on_runtime_upgrade(); + } + #[test] fn weight_should_attach_to_call_enum() { // operational. diff --git a/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr b/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr index 559a4637d67ff..7f8f5d83cc26e 100644 --- a/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr +++ b/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr @@ -2,4 +2,4 @@ error: expected curly braces --> $DIR/invalid_module_details.rs:9:19 | 9 | system: System::(), - | ^^ + | ^ diff --git a/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr b/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr index 4af672a2610b6..103816dd1a5b3 100644 --- a/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr +++ b/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr @@ -2,4 +2,4 @@ error: expected `where` --> $DIR/missing_where_block.rs:4:19 | 4 | pub enum Runtime {} - | ^^ + | ^ diff --git a/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr b/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr index ac7313523c0c4..b3d877f730d53 100644 --- a/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr +++ b/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr @@ -2,4 +2,4 @@ error: Missing associated type for `UncheckedExtrinsic`. Add `UncheckedExtrinsic --> $DIR/missing_where_param.rs:7:2 | 7 | {} - | ^^ + | ^ diff --git a/frame/support/test/tests/reserved_keyword/on_initialize.rs b/frame/support/test/tests/reserved_keyword/on_initialize.rs index e389529bca57e..979ce934cd163 100644 --- a/frame/support/test/tests/reserved_keyword/on_initialize.rs +++ b/frame/support/test/tests/reserved_keyword/on_initialize.rs @@ -27,6 +27,6 @@ macro_rules! reserved { } } -reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); +reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); fn main() {} diff --git a/frame/support/test/tests/reserved_keyword/on_initialize.stderr b/frame/support/test/tests/reserved_keyword/on_initialize.stderr index 13c2ef8d2c869..4133f308ce5c0 100644 --- a/frame/support/test/tests/reserved_keyword/on_initialize.stderr +++ b/frame/support/test/tests/reserved_keyword/on_initialize.stderr @@ -1,47 +1,55 @@ error: Invalid call fn name: `on_finalize`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: Invalid call fn name: `on_initialize`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `on_finalise` was renamed to `on_finalize`. Please rename your function accordingly. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `on_initialise` was renamed to `on_initialize`. Please rename your function accordingly. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: Invalid call fn name: `on_runtime_upgrade`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. + --> $DIR/on_initialize.rs:30:1 + | +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: Invalid call fn name: `offchain_worker`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: `deposit_event` function is reserved and must follow the syntax: `$vis:vis fn deposit_event() = default;` --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) From 9eb8a2191073f1104ff739e6026472e528f090f7 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:34:52 +0100 Subject: [PATCH 06/24] docs --- frame/support/src/dispatch.rs | 6 +++--- frame/support/src/storage/mod.rs | 12 ++++++------ frame/support/src/weights.rs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 12d385b81072d..b03a3756f22a6 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -197,9 +197,9 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// The following reserved functions also take the block number (with type `T::BlockNumber`) as an optional input: /// /// * `on_runtime_upgrade`: Executes at the beginning of a block prior to on_initialize when there -/// is a runtime ugprade. This allow each module to upgrade their storage, calling other module -/// must be avoided. Using this function will implement the -/// [`OnInitialize`](../sp_runtime/traits/trait.OnInitialize.html) trait. +/// is a runtime upgrade. This allow each module to upgrade their storage before the storage items are used. +/// As such, **calling other module must be avoided**!! Using this function will implement the +/// [`OnRuntimeUpgrade`](../sp_runtime/traits/trait.OnRuntimeUpgrade.html) trait. /// * `on_initialize`: Executes at the beginning of a block. Using this function will /// implement the [`OnInitialize`](../sp_runtime/traits/trait.OnInitialize.html) trait. /// * `on_finalize`: Executes at the end of a block. Using this function will diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 4bca5ea402398..c28626ad2c318 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -65,8 +65,8 @@ pub trait StorageValue { /// /// # Usage /// - /// This would typically be called inside the module implementation of on_initialize, while - /// ensuring **no usage of this storage are made before the call to `on_initialize`**. (More + /// This would typically be called inside the module implementation of on_runtime_upgrade, while + /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More /// precisely prior initialized modules doesn't make use of this storage). fn translate) -> Option>(f: F) -> Result, ()>; @@ -265,8 +265,8 @@ pub trait StorageLinkedMap { /// /// # Usage /// - /// This would typically be called inside the module implementation of on_initialize, while - /// ensuring **no usage of this storage are made before the call to `on_initialize`**. (More + /// This would typically be called inside the module implementation of on_runtime_upgrade, while + /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More /// precisely prior initialized modules doesn't make use of this storage). fn translate(translate_key: TK, translate_val: TV) -> Result<(), Option> where K2: FullCodec + Clone, V2: Decode, TK: Fn(K2) -> K, TV: Fn(V2) -> V; @@ -460,8 +460,8 @@ pub trait StoragePrefixedMap { /// /// # Usage /// - /// This would typically be called inside the module implementation of on_initialize, while - /// ensuring **no usage of this storage are made before the call to `on_initialize`**. (More + /// This would typically be called inside the module implementation of on_runtime_upgrade, while + /// ensuring **no usage of this storage are made before the call to `on_runtime_upgrade`**. (More /// precisely prior initialized modules doesn't make use of this storage). fn translate_values(translate_val: TV) -> Result<(), u32> where OldValue: Decode, TV: Fn(OldValue) -> Value diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index 883aed693ddda..8926ed949302b 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -67,8 +67,8 @@ pub trait ClassifyDispatch { fn classify_dispatch(&self, target: T) -> DispatchClass; } -/// Means of determining the weight of a block's life cycle hooks: `on_initialize`, `on_finalize` and -/// such. +/// Means of determining the weight of a block's life cycle hooks: `on_initialize`, `on_finalize`, +/// `on_runtime_upgrade`, and such. pub trait WeighBlock { /// Return the weight of the block's on_runtime_upgrade hook. fn on_runtime_upgrade() -> Weight { Zero::zero() } From dd6c6c3e1376fa9e28880d5aeb7a3d3573d58964 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:44:22 +0100 Subject: [PATCH 07/24] Remove "useless" code --- frame/staking/src/lib.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index e6afdd421190a..f4fd3d855e887 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -931,9 +931,6 @@ decl_module! { Self::ensure_storage_upgraded(); } - fn on_initialize() { - } - fn on_finalize() { // Set the start of the first era. if let Some(mut active_era) = Self::active_era() { @@ -1135,8 +1132,6 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(750_000)] fn validate(origin, prefs: ValidatorPrefs) { - Self::ensure_storage_upgraded(); - let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash = &ledger.stash; @@ -1157,8 +1152,6 @@ decl_module! { /// # #[weight = SimpleDispatchInfo::FixedNormal(750_000)] fn nominate(origin, targets: Vec<::Source>) { - Self::ensure_storage_upgraded(); - let controller = ensure_signed(origin)?; let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; let stash = &ledger.stash; @@ -1436,7 +1429,6 @@ decl_module! { /// /// - `stash`: The stash account to reap. Its balance must be zero. fn reap_stash(_origin, stash: T::AccountId) { - Self::ensure_storage_upgraded(); ensure!(T::Currency::total_balance(&stash).is_zero(), Error::::FundedTarget); Self::kill_stash(&stash)?; T::Currency::remove_lock(STAKING_ID, &stash); @@ -2111,7 +2103,6 @@ impl Module { /// some session can lag in between the newest session planned and the latest session started. impl pallet_session::SessionManager for Module { fn new_session(new_index: SessionIndex) -> Option> { - Self::ensure_storage_upgraded(); Self::new_session(new_index) } fn start_session(start_index: SessionIndex) { From 5edb8e80a4b3cc521d9eb6131f0f5891bedc01b9 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 17:49:36 +0100 Subject: [PATCH 08/24] Fix merge and use n + 1 block number --- frame/executive/src/lib.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index c36ce36b83069..2d57542f416ac 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -178,7 +178,13 @@ where extrinsics_root: &System::Hash, digest: &Digest, ) { - ::on_runtime_upgrade(); + let last_runtime_upgrade = >::last_runtime_upgrade(); + if last_runtime_upgrade.map(|n| n + 1 == *block_number).unwrap_or(false) { + ::on_runtime_upgrade(); + >::register_extra_weight_unchecked( + >::on_runtime_upgrade() + ); + } >::initialize( block_number, parent_hash, @@ -186,13 +192,6 @@ where digest, frame_system::InitKind::Full, ); - let last_runtime_upgrade = >::last_runtime_upgrade(); - if last_runtime_upgrade.map(|n| n == *block_number).unwrap_or(false) { - ::on_runtime_upgrade(); - >::register_extra_weight_unchecked( - >::on_runtime_upgrade() - ); - } >::on_initialize(*block_number); >::register_extra_weight_unchecked( >::on_initialize(*block_number) From 464229acd94c265081014ede067807536c2fa8b9 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 18:00:32 +0100 Subject: [PATCH 09/24] Fix tests --- frame/executive/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 2d57542f416ac..ffafe88541701 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -179,7 +179,7 @@ where digest: &Digest, ) { let last_runtime_upgrade = >::last_runtime_upgrade(); - if last_runtime_upgrade.map(|n| n + 1 == *block_number).unwrap_or(false) { + if last_runtime_upgrade.map(|n| n + One::one() == *block_number).unwrap_or(false) { ::on_runtime_upgrade(); >::register_extra_weight_unchecked( >::on_runtime_upgrade() @@ -582,7 +582,7 @@ mod tests { header: Header { parent_hash: [69u8; 32].into(), number: 1, - state_root: hex!("17caebd966d10cc6dc9659edf7fa3196511593f6c39f80f9b97cdbc3b0855cf3").into(), + state_root: hex!("c96987b52003684f590a6b9b55af9ded88b2e6f8d84441c456f46e59f5e73070").into(), extrinsics_root: hex!("03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314").into(), digest: Digest { logs: vec![], }, }, From 9806b4debab146a5bae3a4d33b682a78c971b40f Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 18:40:43 +0100 Subject: [PATCH 10/24] unfix ui tests --- .../tests/construct_runtime_ui/invalid_module_details.stderr | 2 +- .../test/tests/construct_runtime_ui/missing_where_block.stderr | 2 +- .../test/tests/construct_runtime_ui/missing_where_param.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr b/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr index 7f8f5d83cc26e..559a4637d67ff 100644 --- a/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr +++ b/frame/support/test/tests/construct_runtime_ui/invalid_module_details.stderr @@ -2,4 +2,4 @@ error: expected curly braces --> $DIR/invalid_module_details.rs:9:19 | 9 | system: System::(), - | ^ + | ^^ diff --git a/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr b/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr index 103816dd1a5b3..4af672a2610b6 100644 --- a/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr +++ b/frame/support/test/tests/construct_runtime_ui/missing_where_block.stderr @@ -2,4 +2,4 @@ error: expected `where` --> $DIR/missing_where_block.rs:4:19 | 4 | pub enum Runtime {} - | ^ + | ^^ diff --git a/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr b/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr index b3d877f730d53..ac7313523c0c4 100644 --- a/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr +++ b/frame/support/test/tests/construct_runtime_ui/missing_where_param.stderr @@ -2,4 +2,4 @@ error: Missing associated type for `UncheckedExtrinsic`. Add `UncheckedExtrinsic --> $DIR/missing_where_param.rs:7:2 | 7 | {} - | ^ + | ^^ From 2f515955d4ce87b4d92819a5e69d04b07ada37a2 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 19:04:58 +0100 Subject: [PATCH 11/24] Update on_initialize.stderr --- .../reserved_keyword/on_initialize.stderr | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/frame/support/test/tests/reserved_keyword/on_initialize.stderr b/frame/support/test/tests/reserved_keyword/on_initialize.stderr index 4133f308ce5c0..466791c6f2439 100644 --- a/frame/support/test/tests/reserved_keyword/on_initialize.stderr +++ b/frame/support/test/tests/reserved_keyword/on_initialize.stderr @@ -2,54 +2,54 @@ error: Invalid call fn name: `on_finalize`, name is reserved and doesn't match e --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `on_initialize`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: `on_finalise` was renamed to `on_finalize`. Please rename your function accordingly. --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: `on_initialise` was renamed to `on_initialize`. Please rename your function accordingly. --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `on_runtime_upgrade`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `offchain_worker`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: `deposit_event` function is reserved and must follow the syntax: `$vis:vis fn deposit_event() = default;` --> $DIR/on_initialize.rs:30:1 | 30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) From d039747ae6f50b61be77aa40efbf606731f822ac Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 19:26:32 +0100 Subject: [PATCH 12/24] fix test --- frame/democracy/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/democracy/src/lib.rs b/frame/democracy/src/lib.rs index 6755fabd2039f..e3d376f732f66 100644 --- a/frame/democracy/src/lib.rs +++ b/frame/democracy/src/lib.rs @@ -1249,7 +1249,7 @@ mod tests { }; use sp_core::H256; use sp_runtime::{ - traits::{BlakeTwo256, IdentityLookup, Bounded, BadOrigin, OnInitialize}, + traits::{BlakeTwo256, IdentityLookup, Bounded, BadOrigin, OnRuntimeUpgrade}, testing::Header, Perbill, }; use pallet_balances::{BalanceLock, Error as BalancesError}; @@ -1407,7 +1407,7 @@ mod tests { ]; s.top = data.into_iter().collect(); sp_io::TestExternalities::new(s).execute_with(|| { - Balances::on_initialize(1); + Balances::on_runtime_upgrade(); assert_eq!(Balances::free_balance(1), 5); assert_eq!(Balances::reserved_balance(1), 5); assert_eq!(Balances::usable_balance(&1), 2); From 7a23da97b64a46d38b5c3d0db491b3a5704965d3 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 19:56:17 +0100 Subject: [PATCH 13/24] Fix test --- frame/vesting/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/vesting/src/lib.rs b/frame/vesting/src/lib.rs index fdc480ddd0417..5e485333eedf6 100644 --- a/frame/vesting/src/lib.rs +++ b/frame/vesting/src/lib.rs @@ -301,7 +301,7 @@ mod tests { use sp_runtime::{ Perbill, testing::Header, - traits::{BlakeTwo256, IdentityLookup, Identity, OnInitialize}, + traits::{BlakeTwo256, IdentityLookup, Identity, OnRuntimeUpgrade}, }; use sp_storage::Storage; @@ -436,7 +436,7 @@ mod tests { ]; s.top = data.into_iter().collect(); sp_io::TestExternalities::new(s).execute_with(|| { - Balances::on_initialize(1); + Balances::on_runtime_upgrade(); assert_eq!(Balances::free_balance(6), 60); assert_eq!(Balances::usable_balance(&6), 30); System::set_block_number(2); From 3b5506062e50e38b11cab8fe8fcf303ec24d7ed7 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 19:57:26 +0100 Subject: [PATCH 14/24] Bump spec --- bin/node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index be5ca25630d4a..424111ffa0b88 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 227, + spec_version: 228, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; From f31ff58f0572daee0dcab8b193cf898ecb981c75 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 21:20:37 +0100 Subject: [PATCH 15/24] Remove `on_finalise` and `on_initialise` --- frame/support/src/dispatch.rs | 6 ---- .../tests/reserved_keyword/on_initialize.rs | 2 +- .../reserved_keyword/on_initialize.stderr | 36 ++++++------------- .../consensus/common/src/select_chain.rs | 4 +-- primitives/runtime/src/offchain/http.rs | 2 +- 5 files changed, 14 insertions(+), 36 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index b03a3756f22a6..11b501c7a38af 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2037,15 +2037,9 @@ macro_rules! __check_reserved_fn_name { (on_runtime_upgrade $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error on_runtime_upgrade); }; - (on_initialise $( $rest:ident )*) => { - $crate::__check_reserved_fn_name!(@compile_error_renamed on_initialise on_initialize); - }; (on_finalize $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error on_finalize); }; - (on_finalise $( $rest:ident )*) => { - $crate::__check_reserved_fn_name!(@compile_error_renamed on_finalise on_finalize); - }; (offchain_worker $( $rest:ident )*) => { $crate::__check_reserved_fn_name!(@compile_error offchain_worker); }; diff --git a/frame/support/test/tests/reserved_keyword/on_initialize.rs b/frame/support/test/tests/reserved_keyword/on_initialize.rs index 979ce934cd163..84feb2d93f36c 100644 --- a/frame/support/test/tests/reserved_keyword/on_initialize.rs +++ b/frame/support/test/tests/reserved_keyword/on_initialize.rs @@ -27,6 +27,6 @@ macro_rules! reserved { } } -reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); +reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); fn main() {} diff --git a/frame/support/test/tests/reserved_keyword/on_initialize.stderr b/frame/support/test/tests/reserved_keyword/on_initialize.stderr index 466791c6f2439..d20a6e11451e7 100644 --- a/frame/support/test/tests/reserved_keyword/on_initialize.stderr +++ b/frame/support/test/tests/reserved_keyword/on_initialize.stderr @@ -1,55 +1,39 @@ error: Invalid call fn name: `on_finalize`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `on_initialize`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -error: `on_finalise` was renamed to `on_finalize`. Please rename your function accordingly. - --> $DIR/on_initialize.rs:30:1 - | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - -error: `on_initialise` was renamed to `on_initialize`. Please rename your function accordingly. - --> $DIR/on_initialize.rs:30:1 - | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `on_runtime_upgrade`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: Invalid call fn name: `offchain_worker`, name is reserved and doesn't match expected signature, please refer to `decl_module!` documentation to see the appropriate usage, or rename it to an unreserved keyword. --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) error: `deposit_event` function is reserved and must follow the syntax: `$vis:vis fn deposit_event() = default;` --> $DIR/on_initialize.rs:30:1 | -30 | reserved!(on_finalize on_initialize on_finalise on_initialise on_runtime_upgrade offchain_worker deposit_event); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation +30 | reserved!(on_finalize on_initialize on_runtime_upgrade offchain_worker deposit_event); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this macro invocation | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) diff --git a/primitives/consensus/common/src/select_chain.rs b/primitives/consensus/common/src/select_chain.rs index d94511c11027a..fe0d3972043b9 100644 --- a/primitives/consensus/common/src/select_chain.rs +++ b/primitives/consensus/common/src/select_chain.rs @@ -23,11 +23,11 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; /// specific chain build. /// /// The Strategy can be customized for the two use cases of authoring new blocks -/// upon the best chain or which fork to finalise. Unless implemented differently +/// upon the best chain or which fork to finalize. Unless implemented differently /// by default finalization methods fall back to use authoring, so as a minimum /// `_authoring`-functions must be implemented. /// -/// Any particular user must make explicit, however, whether they intend to finalise +/// Any particular user must make explicit, however, whether they intend to finalize /// or author through the using the right function call, as these might differ in /// some implementations. /// diff --git a/primitives/runtime/src/offchain/http.rs b/primitives/runtime/src/offchain/http.rs index 88f4323ad7e9e..bbc929526b759 100644 --- a/primitives/runtime/src/offchain/http.rs +++ b/primitives/runtime/src/offchain/http.rs @@ -241,7 +241,7 @@ impl<'a, I: AsRef<[u8]>, T: IntoIterator> Request<'a, T> { sp_io::offchain::http_request_write_body(id, chunk.as_ref(), self.deadline)?; } - // finalise the request + // finalize the request sp_io::offchain::http_request_write_body(id, &[], self.deadline)?; Ok(PendingRequest { From 8aecd6ebb1e01cb5125249ec5c2e39c000f38d7d Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Tue, 3 Mar 2020 22:38:07 +0100 Subject: [PATCH 16/24] Use bool for tracking runtime upgraded --- frame/executive/src/lib.rs | 8 +++++--- frame/system/src/lib.rs | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index ffafe88541701..754ce52298cd2 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -75,7 +75,10 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_std::{prelude::*, marker::PhantomData}; -use frame_support::weights::{GetDispatchInfo, WeighBlock, DispatchInfo}; +use frame_support::{ + storage::StorageValue, + weights::{GetDispatchInfo, WeighBlock, DispatchInfo} +}; use sp_runtime::{ generic::Digest, ApplyExtrinsicResult, @@ -178,8 +181,7 @@ where extrinsics_root: &System::Hash, digest: &Digest, ) { - let last_runtime_upgrade = >::last_runtime_upgrade(); - if last_runtime_upgrade.map(|n| n + One::one() == *block_number).unwrap_or(false) { + if frame_system::RuntimeUpgraded::take() { ::on_runtime_upgrade(); >::register_extra_weight_unchecked( >::on_runtime_upgrade() diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 7968e11082131..79bbd9a397f8f 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -368,8 +368,8 @@ decl_storage! { /// no notification will be triggered thus the event might be lost. EventTopics get(fn event_topics): map hasher(blake2_256) T::Hash => Vec<(T::BlockNumber, EventIndex)>; - /// Store the block number of last runtime upgrade. - LastRuntimeUpgrade get(fn last_runtime_upgrade): Option; + /// A bool to track if the runtime was upgraded last block. + pub RuntimeUpgraded: bool; } add_extra_genesis { config(changes_trie_config): Option; @@ -489,7 +489,7 @@ decl_module! { } storage::unhashed::put_raw(well_known_keys::CODE, &code); - >::put(Self::block_number()); + RuntimeUpgraded::put(true); Self::deposit_event(RawEvent::CodeUpdated); } @@ -498,7 +498,7 @@ decl_module! { pub fn set_code_without_checks(origin, code: Vec) { ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::CODE, &code); - >::put(Self::block_number()); + RuntimeUpgraded::put(true); Self::deposit_event(RawEvent::CodeUpdated); } From 107436e49d846da4075ccd438d8a0a9df99a716b Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 19:11:55 +0100 Subject: [PATCH 17/24] typo --- frame/support/src/dispatch.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 11b501c7a38af..a9c48097ad64a 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -197,8 +197,8 @@ impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} /// The following reserved functions also take the block number (with type `T::BlockNumber`) as an optional input: /// /// * `on_runtime_upgrade`: Executes at the beginning of a block prior to on_initialize when there -/// is a runtime upgrade. This allow each module to upgrade their storage before the storage items are used. -/// As such, **calling other module must be avoided**!! Using this function will implement the +/// is a runtime upgrade. This allows each module to upgrade its storage before the storage items are used. +/// As such, **calling other modules must be avoided**!! Using this function will implement the /// [`OnRuntimeUpgrade`](../sp_runtime/traits/trait.OnRuntimeUpgrade.html) trait. /// * `on_initialize`: Executes at the beginning of a block. Using this function will /// implement the [`OnInitialize`](../sp_runtime/traits/trait.OnInitialize.html) trait. From ef19542fd1182f77120fc8029ab553d9892f8ce0 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 20:12:42 +0100 Subject: [PATCH 18/24] Support runtime upgrade with `set_storage` --- frame/support/src/storage/unhashed.rs | 4 ++++ frame/system/src/lib.rs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/frame/support/src/storage/unhashed.rs b/frame/support/src/storage/unhashed.rs index 54910e99a4d86..1ecf46ef18647 100644 --- a/frame/support/src/storage/unhashed.rs +++ b/frame/support/src/storage/unhashed.rs @@ -101,6 +101,10 @@ pub fn get_raw(key: &[u8]) -> Option> { } /// Put a raw byte slice into storage. +/// +/// **WARNING**: If you set the storage of the Substrate Wasm (`well_known_keys::CODE`), +/// you should also call `frame_system::RuntimeUpgraded::put(true)` to trigger the +/// `on_runtime_upgrade` logic. pub fn put_raw(key: &[u8], value: &[u8]) { sp_io::storage::set(key, value) } diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 79bbd9a397f8f..d7d731783f472 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -526,6 +526,9 @@ decl_module! { ensure_root(origin)?; for i in &items { storage::unhashed::put_raw(&i.0, &i.1); + if i.0 == well_known_keys::CODE { + RuntimeUpgraded::put(true); + } } } @@ -1976,6 +1979,26 @@ mod tests { System::events(), vec![EventRecord { phase: Phase::ApplyExtrinsic(0), event: 102u16, topics: vec![] }], ); + + assert_eq!(RuntimeUpgraded::get(), true); + }); + } + + #[test] + fn runtime_upgraded_with_set_storage() { + let executor = substrate_test_runtime_client::new_native_executor(); + let mut ext = new_test_ext(); + ext.register_extension(sp_core::traits::CallInWasmExt::new(executor)); + ext.execute_with(|| { + System::set_storage( + RawOrigin::Root.into(), + vec![( + well_known_keys::CODE.to_vec(), + substrate_test_runtime_client::runtime::WASM_BINARY.to_vec() + )], + ).unwrap(); + + assert_eq!(RuntimeUpgraded::get(), true); }); } } From f0ddeb8c8f607672abea8e98d828e44dfce26adf Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 21:39:50 +0100 Subject: [PATCH 19/24] Refactor migration code location --- frame/balances/src/lib.rs | 21 +- frame/balances/src/migration.rs | 140 ++++++----- frame/staking/src/lib.rs | 177 ++------------ frame/staking/src/migration/deprecated.rs | 70 ++++++ frame/staking/src/migration/mod.rs | 112 +++++++++ .../test_upgrade_from_master_dataset.rs | 0 frame/staking/src/migration/tests.rs | 220 ++++++++++++++++++ frame/staking/src/tests.rs | 218 ----------------- 8 files changed, 507 insertions(+), 451 deletions(-) create mode 100644 frame/staking/src/migration/deprecated.rs create mode 100644 frame/staking/src/migration/mod.rs rename frame/staking/src/{tests => migration}/test_upgrade_from_master_dataset.rs (100%) create mode 100644 frame/staking/src/migration/tests.rs diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 3033d4b6bb176..52be559e6bac4 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -348,6 +348,21 @@ impl AccountData { } } +// A value placed in storage that represents the current version of the Balances storage. +// This value is used by the `on_runtime_upgrade` logic to determine whether we run +// storage migration logic. This should match directly with the semantic versions of the Rust crate. +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] +enum Releases { + V1_0_0, + V2_0_0, +} + +impl Default for Releases { + fn default() -> Self { + Releases::V1_0_0 + } +} + decl_storage! { trait Store for Module, I: Instance=DefaultInstance> as Balances { /// The total units issued in the system. @@ -367,10 +382,10 @@ decl_storage! { /// NOTE: Should only be accessed when setting, changing and freeing a lock. pub Locks get(fn locks): map hasher(blake2_256) T::AccountId => Vec>; - /// Module version number. + /// Storage version of the pallet. /// - /// Default to version 1 for new networks. - ModuleVersion build(|_: &GenesisConfig| 1): u8; + /// This is set to v2.0.0 for new networks. + StorageVersion build(|_: &GenesisConfig| Releases::V2_0_0): Releases; } add_extra_genesis { config(balances): Vec<(T::AccountId, T::Balance)>; diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index ff25d130bb948..98e753c556b3a 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -2,86 +2,80 @@ use super::*; // Upgrade from the pre-#4649 balances/vesting into the new balances. pub fn on_runtime_upgrade, I: Instance>() { - let current_version = ModuleVersion::::get(); + if StorageVersion::::get() == Releases::V2_0_0 { return }; - if current_version == 1 { - return - } else if current_version == 0 { - ModuleVersion::::put(1); - sp_runtime::print("Upgrading account balances..."); - // First, migrate from old FreeBalance to new Account. - // We also move all locks across since only accounts with FreeBalance values have locks. - // FreeBalance: map T::AccountId => T::Balance - for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { - let mut account = AccountData { free, ..Default::default() }; - // Locks: map T::AccountId => Vec - let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); - if let Some(locks) = old_locks { - let locks = locks.into_iter() - .map(|i| { - let (result, expiry) = i.upgraded(); - if expiry != T::BlockNumber::max_value() { - // Any `until`s that are not T::BlockNumber::max_value come from - // democracy and need to be migrated over there. - // Democracy: Locks get(locks): map T::AccountId => Option; - put_storage_value(b"Democracy", b"Locks", &hash, expiry); - } - result - }) - .collect::>(); - for l in locks.iter() { - if l.reasons == Reasons::All || l.reasons == Reasons::Misc { - account.misc_frozen = account.misc_frozen.max(l.amount); - } - if l.reasons == Reasons::All || l.reasons == Reasons::Fee { - account.fee_frozen = account.fee_frozen.max(l.amount); + sp_runtime::print("Upgrading account balances..."); + // First, migrate from old FreeBalance to new Account. + // We also move all locks across since only accounts with FreeBalance values have locks. + // FreeBalance: map T::AccountId => T::Balance + for (hash, free) in StorageIterator::::new(b"Balances", b"FreeBalance").drain() { + let mut account = AccountData { free, ..Default::default() }; + // Locks: map T::AccountId => Vec + let old_locks = get_storage_value::>>(b"Balances", b"Locks", &hash); + if let Some(locks) = old_locks { + let locks = locks.into_iter() + .map(|i| { + let (result, expiry) = i.upgraded(); + if expiry != T::BlockNumber::max_value() { + // Any `until`s that are not T::BlockNumber::max_value come from + // democracy and need to be migrated over there. + // Democracy: Locks get(locks): map T::AccountId => Option; + put_storage_value(b"Democracy", b"Locks", &hash, expiry); } + result + }) + .collect::>(); + for l in locks.iter() { + if l.reasons == Reasons::All || l.reasons == Reasons::Misc { + account.misc_frozen = account.misc_frozen.max(l.amount); + } + if l.reasons == Reasons::All || l.reasons == Reasons::Fee { + account.fee_frozen = account.fee_frozen.max(l.amount); } - put_storage_value(b"Balances", b"Locks", &hash, locks); } - put_storage_value(b"Balances", b"Account", &hash, account); - } - // Second, migrate old ReservedBalance into new Account. - // ReservedBalance: map T::AccountId => T::Balance - for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - account.reserved = reserved; - put_storage_value(b"Balances", b"Account", &hash, account); - } - - // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock - // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the - // lock to something sensible. - // pub Vesting: map T::AccountId => Option; - for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { - let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); - let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); - locks.push(BalanceLock { - id: *b"vesting ", - amount: vesting.0.clone(), - reasons: Reasons::Misc, - }); - account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); - put_storage_value(b"Vesting", b"Vesting", &hash, vesting); put_storage_value(b"Balances", b"Locks", &hash, locks); - put_storage_value(b"Balances", b"Account", &hash, account); } + put_storage_value(b"Balances", b"Account", &hash, account); + } + // Second, migrate old ReservedBalance into new Account. + // ReservedBalance: map T::AccountId => T::Balance + for (hash, reserved) in StorageIterator::::new(b"Balances", b"ReservedBalance").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + account.reserved = reserved; + put_storage_value(b"Balances", b"Account", &hash, account); + } - for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { - let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); - let mut refs: system::RefCount = 0; - // The items in Kusama that would result in a ref count being incremented. - if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } - // We skip Recovered since it's being replaced anyway. - let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); - prefixed_hash.extend(&b":session:keys"[..]); - prefixed_hash.extend(&hash[..]); - if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } - if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } - put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); - } + // Finally, migrate vesting and ensure locks are in place. We will be lazy and just lock + // for the maximum amount (i.e. at genesis). Users will need to call "vest" to reduce the + // lock to something sensible. + // pub Vesting: map T::AccountId => Option; + for (hash, vesting) in StorageIterator::<(T::Balance, T::Balance, T::BlockNumber)>::new(b"Balances", b"Vesting").drain() { + let mut account = get_storage_value::>(b"Balances", b"Account", &hash).unwrap_or_default(); + let mut locks = get_storage_value::>>(b"Balances", b"Locks", &hash).unwrap_or_default(); + locks.push(BalanceLock { + id: *b"vesting ", + amount: vesting.0.clone(), + reasons: Reasons::Misc, + }); + account.misc_frozen = account.misc_frozen.max(vesting.0.clone()); + put_storage_value(b"Vesting", b"Vesting", &hash, vesting); + put_storage_value(b"Balances", b"Locks", &hash, locks); + put_storage_value(b"Balances", b"Account", &hash, account); + } - // Recursively call the upgrade function to apply all upgrades. - on_runtime_upgrade::() + for (hash, balances) in StorageIterator::>::new(b"Balances", b"Account").drain() { + let nonce = take_storage_value::(b"System", b"AccountNonce", &hash).unwrap_or_default(); + let mut refs: system::RefCount = 0; + // The items in Kusama that would result in a ref count being incremented. + if have_storage_value(b"Democracy", b"Proxy", &hash) { refs += 1 } + // We skip Recovered since it's being replaced anyway. + let mut prefixed_hash = twox_64(&b":session:keys"[..]).to_vec(); + prefixed_hash.extend(&b":session:keys"[..]); + prefixed_hash.extend(&hash[..]); + if have_storage_value(b"Session", b"NextKeys", &prefixed_hash) { refs += 1 } + if have_storage_value(b"Staking", b"Bonded", &hash) { refs += 1 } + put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); } + + StorageVersion::::put(Releases::V2_0_0); } diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index f4fd3d855e887..89ddca0f7af8c 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -254,6 +254,7 @@ mod mock; #[cfg(test)] mod tests; mod slashing; +mod migration; pub mod inflation; @@ -679,6 +680,21 @@ impl Default for Forcing { fn default() -> Self { Forcing::NotForcing } } +// A value placed in storage that represents the current version of the Balances storage. +// This value is used by the `on_runtime_upgrade` logic to determine whether we run +// storage migration logic. This should match directly with the semantic versions of the Rust crate. +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] +enum Releases { + V1_0_0, + V2_0_0, +} + +impl Default for Releases { + fn default() -> Self { + Releases::V1_0_0 + } +} + decl_storage! { trait Store for Module as Staking { /// Number of era to keep in history. @@ -832,10 +848,10 @@ decl_storage! { /// The earliest era for which we have a pending, unapplied slash. EarliestUnappliedSlash: Option; - /// True if network has been upgraded to this version. + /// Storage version of the pallet. /// - /// True for new networks. - IsUpgraded build(|_| true): bool; + /// This is set to v2.0.0 for new networks. + StorageVersion build(|_: &GenesisConfig| Releases::V2_0_0): Releases; } add_extra_genesis { config(stakers): @@ -928,7 +944,7 @@ decl_module! { fn deposit_event() = default; fn on_runtime_upgrade() { - Self::ensure_storage_upgraded(); + migration::on_runtime_upgrade::(); } fn on_finalize() { @@ -1572,14 +1588,6 @@ impl Module { >::remove(stash); } - /// Ensures storage is upgraded to most recent necessary state. - fn ensure_storage_upgraded() { - if !IsUpgraded::get() { - IsUpgraded::put(true); - Self::do_upgrade(); - } - } - /// Actually make a payment to a staker. This uses the currency's reward function /// to pay the right payee for the given staker account. fn make_payout(stash: &T::AccountId, amount: BalanceOf) -> Option> { @@ -1953,147 +1961,6 @@ impl Module { _ => ForceEra::put(Forcing::ForceNew), } } - - /// Update storages to current version - /// - /// In old version the staking module has several issue about handling session delay, the - /// current era was always considered the active one. - /// - /// After the migration the current era will still be considered the active one for the era of - /// the upgrade. And the delay issue will be fixed when planning the next era. - // * create: - // * ActiveEraStart - // * ErasRewardPoints - // * ActiveEra - // * ErasStakers - // * ErasStakersClipped - // * ErasValidatorPrefs - // * ErasTotalStake - // * ErasStartSessionIndex - // * translate StakingLedger - // * removal of: - // * Stakers - // * SlotStake - // * CurrentElected - // * CurrentEraStart - // * CurrentEraStartSessionIndex - // * CurrentEraPointsEarned - fn do_upgrade() { - /// Deprecated storages used for migration only. - mod deprecated { - use crate::{Trait, BalanceOf, MomentOf, SessionIndex, Exposure}; - use codec::{Encode, Decode}; - use frame_support::{decl_module, decl_storage}; - use sp_std::prelude::*; - - /// Reward points of an era. Used to split era total payout between validators. - #[derive(Encode, Decode, Default)] - pub struct EraPoints { - /// Total number of points. Equals the sum of reward points for each validator. - pub total: u32, - /// The reward points earned by a given validator. The index of this vec corresponds to the - /// index into the current validator set. - pub individual: Vec, - } - - decl_module! { - pub struct Module for enum Call where origin: T::Origin { } - } - - decl_storage! { - pub trait Store for Module as Staking { - pub SlotStake: BalanceOf; - - /// The currently elected validator set keyed by stash account ID. - pub CurrentElected: Vec; - - /// The start of the current era. - pub CurrentEraStart: MomentOf; - - /// The session index at which the current era started. - pub CurrentEraStartSessionIndex: SessionIndex; - - /// Rewards for the current era. Using indices of current elected set. - pub CurrentEraPointsEarned: EraPoints; - - /// Nominators for a particular account that is in action right now. You can't iterate - /// through validators here, but you can find them in the Session module. - /// - /// This is keyed by the stash account. - pub Stakers: map hasher(blake2_256) T::AccountId => Exposure>; - } - } - } - - #[derive(Encode, Decode)] - struct OldStakingLedger { - stash: AccountId, - #[codec(compact)] - total: Balance, - #[codec(compact)] - active: Balance, - unlocking: Vec>, - } - - let current_era_start_index = deprecated::CurrentEraStartSessionIndex::get(); - let current_era = as Store>::CurrentEra::get().unwrap_or(0); - let current_era_start = deprecated::CurrentEraStart::::get(); - as Store>::ErasStartSessionIndex::insert(current_era, current_era_start_index); - as Store>::ActiveEra::put(ActiveEraInfo { - index: current_era, - start: Some(current_era_start), - }); - - let current_elected = deprecated::CurrentElected::::get(); - let mut current_total_stake = >::zero(); - for validator in ¤t_elected { - let exposure = deprecated::Stakers::::get(validator); - current_total_stake += exposure.total; - as Store>::ErasStakers::insert(current_era, validator, &exposure); - - let mut exposure_clipped = exposure; - let clipped_max_len = T::MaxNominatorRewardedPerValidator::get() as usize; - if exposure_clipped.others.len() > clipped_max_len { - exposure_clipped.others.sort_unstable_by(|a, b| a.value.cmp(&b.value).reverse()); - exposure_clipped.others.truncate(clipped_max_len); - } - as Store>::ErasStakersClipped::insert(current_era, validator, exposure_clipped); - - let pref = as Store>::Validators::get(validator); - as Store>::ErasValidatorPrefs::insert(current_era, validator, pref); - } - as Store>::ErasTotalStake::insert(current_era, current_total_stake); - - let points = deprecated::CurrentEraPointsEarned::get(); - as Store>::ErasRewardPoints::insert(current_era, EraRewardPoints { - total: points.total, - individual: current_elected.iter().cloned().zip(points.individual.iter().cloned()).collect(), - }); - - let res = as Store>::Ledger::translate_values( - |old: OldStakingLedger>| StakingLedger { - stash: old.stash, - total: old.total, - active: old.active, - unlocking: old.unlocking, - last_reward: None, - } - ); - if let Err(e) = res { - frame_support::print("Encountered error in migration of Staking::Ledger map."); - frame_support::print("The number of removed key/value is:"); - frame_support::print(e); - } - - - // Kill old storages - deprecated::Stakers::::remove_all(); - deprecated::SlotStake::::kill(); - deprecated::CurrentElected::::kill(); - deprecated::CurrentEraStart::::kill(); - deprecated::CurrentEraStartSessionIndex::kill(); - deprecated::CurrentEraPointsEarned::kill(); - } } /// In this implementation `new_session(session)` must be called before `end_session(session-1)` @@ -2202,8 +2069,6 @@ impl OnOffenceHandler>::ensure_storage_upgraded(); - let reward_proportion = SlashRewardFraction::get(); let active_era = { @@ -2291,8 +2156,6 @@ impl ReportOffence O: Offence, { fn report_offence(reporters: Vec, offence: O) -> Result<(), OffenceError> { - >::ensure_storage_upgraded(); - // disallow any slashing from before the current bonding period. let offence_session = offence.session_index(); let bonded_eras = BondedEras::get(); diff --git a/frame/staking/src/migration/deprecated.rs b/frame/staking/src/migration/deprecated.rs new file mode 100644 index 0000000000000..bb37fb284b6aa --- /dev/null +++ b/frame/staking/src/migration/deprecated.rs @@ -0,0 +1,70 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +/// Deprecated storages used for migration from v1.0.0 to v2.0.0 only. + +use crate::{Trait, BalanceOf, MomentOf, SessionIndex, Exposure, UnlockChunk}; +use codec::{Encode, Decode, HasCompact}; +use frame_support::{decl_module, decl_storage}; +use sp_std::prelude::*; + +/// Reward points of an era. Used to split era total payout between validators. +#[derive(Encode, Decode, Default)] +pub struct EraPoints { + /// Total number of points. Equals the sum of reward points for each validator. + pub total: u32, + /// The reward points earned by a given validator. The index of this vec corresponds to the + /// index into the current validator set. + pub individual: Vec, +} + +#[derive(Encode, Decode)] +pub struct OldStakingLedger { + pub stash: AccountId, + #[codec(compact)] + pub total: Balance, + #[codec(compact)] + pub active: Balance, + pub unlocking: Vec>, +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin { } +} + +decl_storage! { + pub trait Store for Module as Staking { + pub SlotStake: BalanceOf; + + /// The currently elected validator set keyed by stash account ID. + pub CurrentElected: Vec; + + /// The start of the current era. + pub CurrentEraStart: MomentOf; + + /// The session index at which the current era started. + pub CurrentEraStartSessionIndex: SessionIndex; + + /// Rewards for the current era. Using indices of current elected set. + pub CurrentEraPointsEarned: EraPoints; + + /// Nominators for a particular account that is in action right now. You can't iterate + /// through validators here, but you can find them in the Session module. + /// + /// This is keyed by the stash account. + pub Stakers: map hasher(blake2_256) T::AccountId => Exposure>; + } +} \ No newline at end of file diff --git a/frame/staking/src/migration/mod.rs b/frame/staking/src/migration/mod.rs new file mode 100644 index 0000000000000..fbcefd864da52 --- /dev/null +++ b/frame/staking/src/migration/mod.rs @@ -0,0 +1,112 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +/// Update storage from v1.0.0 to v2.0.0 +/// +/// In old version the staking module has several issue about handling session delay, the +/// current era was always considered the active one. +/// +/// After the migration the current era will still be considered the active one for the era of +/// the upgrade. And the delay issue will be fixed when planning the next era. +// * create: +// * ActiveEraStart +// * ErasRewardPoints +// * ActiveEra +// * ErasStakers +// * ErasStakersClipped +// * ErasValidatorPrefs +// * ErasTotalStake +// * ErasStartSessionIndex +// * translate StakingLedger +// * removal of: +// * Stakers +// * SlotStake +// * CurrentElected +// * CurrentEraStart +// * CurrentEraStartSessionIndex +// * CurrentEraPointsEarned + +use super::*; +mod deprecated; +#[cfg(test)] +mod tests; +#[cfg(test)] +mod test_upgrade_from_master_dataset; + +pub fn on_runtime_upgrade() { + if StorageVersion::get() == Releases::V2_0_0 { return }; + + let current_era_start_index = deprecated::CurrentEraStartSessionIndex::get(); + let current_era = as Store>::CurrentEra::get().unwrap_or(0); + let current_era_start = deprecated::CurrentEraStart::::get(); + as Store>::ErasStartSessionIndex::insert(current_era, current_era_start_index); + as Store>::ActiveEra::put(ActiveEraInfo { + index: current_era, + start: Some(current_era_start), + }); + + let current_elected = deprecated::CurrentElected::::get(); + let mut current_total_stake = >::zero(); + for validator in ¤t_elected { + let exposure = deprecated::Stakers::::get(validator); + current_total_stake += exposure.total; + as Store>::ErasStakers::insert(current_era, validator, &exposure); + + let mut exposure_clipped = exposure; + let clipped_max_len = T::MaxNominatorRewardedPerValidator::get() as usize; + if exposure_clipped.others.len() > clipped_max_len { + exposure_clipped.others.sort_unstable_by(|a, b| a.value.cmp(&b.value).reverse()); + exposure_clipped.others.truncate(clipped_max_len); + } + as Store>::ErasStakersClipped::insert(current_era, validator, exposure_clipped); + + let pref = as Store>::Validators::get(validator); + as Store>::ErasValidatorPrefs::insert(current_era, validator, pref); + } + as Store>::ErasTotalStake::insert(current_era, current_total_stake); + + let points = deprecated::CurrentEraPointsEarned::get(); + as Store>::ErasRewardPoints::insert(current_era, EraRewardPoints { + total: points.total, + individual: current_elected.iter().cloned().zip(points.individual.iter().cloned()).collect(), + }); + + let res = as Store>::Ledger::translate_values( + |old: deprecated::OldStakingLedger>| StakingLedger { + stash: old.stash, + total: old.total, + active: old.active, + unlocking: old.unlocking, + last_reward: None, + } + ); + if let Err(e) = res { + frame_support::print("Encountered error in migration of Staking::Ledger map."); + frame_support::print("The number of removed key/value is:"); + frame_support::print(e); + } + + + // Kill old storages + deprecated::Stakers::::remove_all(); + deprecated::SlotStake::::kill(); + deprecated::CurrentElected::::kill(); + deprecated::CurrentEraStart::::kill(); + deprecated::CurrentEraStartSessionIndex::kill(); + deprecated::CurrentEraPointsEarned::kill(); + + StorageVersion::put(Releases::V2_0_0); +} diff --git a/frame/staking/src/tests/test_upgrade_from_master_dataset.rs b/frame/staking/src/migration/test_upgrade_from_master_dataset.rs similarity index 100% rename from frame/staking/src/tests/test_upgrade_from_master_dataset.rs rename to frame/staking/src/migration/test_upgrade_from_master_dataset.rs diff --git a/frame/staking/src/migration/tests.rs b/frame/staking/src/migration/tests.rs new file mode 100644 index 0000000000000..90e61edef772f --- /dev/null +++ b/frame/staking/src/migration/tests.rs @@ -0,0 +1,220 @@ +use crate::*; +use crate::mock::*; +use frame_support::storage::migration::*; +use sp_core::hashing::blake2_256; +use super::test_upgrade_from_master_dataset; +use sp_runtime::traits::OnRuntimeUpgrade; + +#[test] +fn upgrade_works() { + ExtBuilder::default().build().execute_with(|| { + start_era(3); + + assert_eq!(Session::validators(), vec![21, 11]); + + // Insert fake data to check the migration + put_storage_value::>(b"Staking", b"CurrentElected", b"", vec![21, 31]); + put_storage_value::(b"Staking", b"CurrentEraStartSessionIndex", b"", 5); + put_storage_value::>(b"Staking", b"CurrentEraStart", b"", 777); + put_storage_value( + b"Staking", b"Stakers", &blake2_256(&11u64.encode()), + Exposure:: { + total: 10, + own: 10, + others: vec![], + } + ); + put_storage_value( + b"Staking", b"Stakers", &blake2_256(&21u64.encode()), + Exposure:: { + total: 20, + own: 20, + others: vec![], + } + ); + put_storage_value( + b"Staking", b"Stakers", &blake2_256(&31u64.encode()), + Exposure:: { + total: 30, + own: 30, + others: vec![], + } + ); + put_storage_value::<(u32, Vec)>(b"Staking", b"CurrentEraPointsEarned", b"", (12, vec![2, 10])); + ::ErasStakers::remove_all(); + ::ErasStakersClipped::remove_all(); + + ::StorageVersion::put(Releases::V1_0_0); + + // Perform upgrade + Staking::on_runtime_upgrade(); + + assert_eq!(::StorageVersion::get(), Releases::V2_0_0); + + // Check migration + assert_eq!(::ErasStartSessionIndex::get(3).unwrap(), 5); + assert_eq!(::ErasRewardPoints::get(3), EraRewardPoints { + total: 12, + individual: vec![(21, 2), (31, 10)].into_iter().collect(), + }); + assert_eq!(::ActiveEra::get().unwrap().index, 3); + assert_eq!(::ActiveEra::get().unwrap().start, Some(777)); + assert_eq!(::CurrentEra::get().unwrap(), 3); + assert_eq!(::ErasStakers::get(3, 11), Exposure { + total: 0, + own: 0, + others: vec![], + }); + assert_eq!(::ErasStakers::get(3, 21), Exposure { + total: 20, + own: 20, + others: vec![], + }); + assert_eq!(::ErasStakers::get(3, 31), Exposure { + total: 30, + own: 30, + others: vec![], + }); + assert_eq!(::ErasStakersClipped::get(3, 11), Exposure { + total: 0, + own: 0, + others: vec![], + }); + assert_eq!(::ErasStakersClipped::get(3, 21), Exposure { + total: 20, + own: 20, + others: vec![], + }); + assert_eq!(::ErasStakersClipped::get(3, 31), Exposure { + total: 30, + own: 30, + others: vec![], + }); + assert_eq!(::ErasValidatorPrefs::get(3, 21), Staking::validators(21)); + assert_eq!(::ErasValidatorPrefs::get(3, 31), Staking::validators(31)); + assert_eq!(::ErasTotalStake::get(3), 50); + }) +} + +// Test that an upgrade from previous test environment works. +#[test] +fn test_upgrade_from_master_works() { + let data_sets = &[ + test_upgrade_from_master_dataset::_0, + test_upgrade_from_master_dataset::_1, + test_upgrade_from_master_dataset::_2, + test_upgrade_from_master_dataset::_3, + test_upgrade_from_master_dataset::_4, + test_upgrade_from_master_dataset::_5, + test_upgrade_from_master_dataset::_6, + test_upgrade_from_master_dataset::_7, + test_upgrade_from_master_dataset::_8, + ]; + for data_set in data_sets.iter() { + let mut storage = sp_runtime::Storage::default(); + for (key, value) in data_set.iter() { + storage.top.insert(key.to_vec(), value.to_vec()); + } + let mut ext = sp_io::TestExternalities::from(storage); + ext.execute_with(|| { + let old_stakers = + get_storage_value::>(b"Staking", b"CurrentElected", b"").unwrap(); + let old_staker_0 = old_stakers[0]; + let old_staker_1 = old_stakers[1]; + let old_current_era = + get_storage_value::(b"Staking", b"CurrentEra", b"").unwrap(); + let old_staker_0_exposure = get_storage_value::>( + b"Staking", b"Stakers", &blake2_256(&old_staker_0.encode()) + ).unwrap(); + let old_staker_1_exposure = get_storage_value::>( + b"Staking", b"Stakers", &blake2_256(&old_staker_1.encode()) + ).unwrap(); + let ( + old_era_points_earned_total, + old_era_points_earned_individual + ) = get_storage_value::<(u32, Vec)>(b"Staking", b"CurrentEraPointsEarned", b"") + .unwrap_or((0, vec![])); + + Staking::on_runtime_upgrade(); + assert!(::StorageVersion::get() == Releases::V2_0_0); + + // Check ActiveEra and CurrentEra + let active_era = Staking::active_era().unwrap().index; + let current_era = Staking::current_era().unwrap(); + assert!(current_era == active_era); + assert!(current_era == old_current_era); + + // Check ErasStartSessionIndex + let active_era_start = Staking::eras_start_session_index(active_era).unwrap(); + let current_era_start = Staking::eras_start_session_index(current_era).unwrap(); + let current_session_index = Session::current_index(); + assert!(current_era_start == active_era_start); + assert!(active_era_start <= current_session_index); + assert_eq!(::ErasStartSessionIndex::iter().count(), 1); + + // Check ErasStakers + assert_eq!(::ErasStakers::iter().count(), 2); + assert_eq!( + ::ErasStakers::get(current_era, old_staker_0), + old_staker_0_exposure + ); + assert_eq!( + ::ErasStakers::get(current_era, old_staker_1), + old_staker_1_exposure + ); + + // Check ErasStakersClipped + assert_eq!(::ErasStakersClipped::iter().count(), 2); + assert!(::ErasStakersClipped::iter().all(|exposure_clipped| { + let max = ::MaxNominatorRewardedPerValidator::get() as usize; + exposure_clipped.others.len() <= max + })); + assert_eq!( + ::ErasStakersClipped::get(current_era, old_staker_0), + old_staker_0_exposure + ); + assert_eq!( + ::ErasStakersClipped::get(current_era, old_staker_1), + old_staker_1_exposure + ); + + // Check ErasValidatorPrefs + assert_eq!(::ErasValidatorPrefs::iter().count(), 2); + assert_eq!( + ::ErasValidatorPrefs::get(current_era, old_staker_0), + Staking::validators(old_staker_0) + ); + assert_eq!( + ::ErasValidatorPrefs::get(current_era, old_staker_1), + Staking::validators(old_staker_1) + ); + + // Check ErasTotalStake + assert_eq!(::ErasTotalStake::iter().count(), 1); + assert_eq!( + ::ErasTotalStake::get(current_era), + old_staker_0_exposure.total + old_staker_1_exposure.total + ); + + // Check ErasRewardPoints + assert_eq!(::ErasRewardPoints::iter().count(), 1); + let mut individual = BTreeMap::new(); + if let Some(p) = old_era_points_earned_individual.get(0) { + individual.insert(old_staker_0, p.clone()); + } + if let Some(p) = old_era_points_earned_individual.get(1) { + individual.insert(old_staker_1, p.clone()); + } + assert_eq!( + ::ErasRewardPoints::get(current_era), + EraRewardPoints { + total: old_era_points_earned_total, + individual, + } + ); + + // Check ErasValidatorReward + assert_eq!(::ErasValidatorReward::iter().count(), 0); + }); + } +} \ No newline at end of file diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 98536a042aa82..507b5591d5f87 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -16,8 +16,6 @@ //! Tests for the module. -mod test_upgrade_from_master_dataset; - use super::*; use mock::*; use sp_runtime::{assert_eq_error_rate, traits::{OnInitialize, BadOrigin}}; @@ -26,10 +24,8 @@ use frame_support::{ assert_ok, assert_noop, traits::{Currency, ReservableCurrency}, StorageMap, - storage::migration::{put_storage_value, get_storage_value}, }; use pallet_balances::Error as BalancesError; -use sp_io::hashing::blake2_256; use substrate_test_utils::assert_eq_uvec; use crate::Store; @@ -2843,97 +2839,6 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { }); } -#[test] -fn upgrade_works() { - ExtBuilder::default().build().execute_with(|| { - start_era(3); - - assert_eq!(Session::validators(), vec![21, 11]); - - // Insert fake data to check the migration - put_storage_value::>(b"Staking", b"CurrentElected", b"", vec![21, 31]); - put_storage_value::(b"Staking", b"CurrentEraStartSessionIndex", b"", 5); - put_storage_value::>(b"Staking", b"CurrentEraStart", b"", 777); - put_storage_value( - b"Staking", b"Stakers", &blake2_256(&11u64.encode()), - Exposure:: { - total: 10, - own: 10, - others: vec![], - } - ); - put_storage_value( - b"Staking", b"Stakers", &blake2_256(&21u64.encode()), - Exposure:: { - total: 20, - own: 20, - others: vec![], - } - ); - put_storage_value( - b"Staking", b"Stakers", &blake2_256(&31u64.encode()), - Exposure:: { - total: 30, - own: 30, - others: vec![], - } - ); - put_storage_value::<(u32, Vec)>(b"Staking", b"CurrentEraPointsEarned", b"", (12, vec![2, 10])); - ::ErasStakers::remove_all(); - ::ErasStakersClipped::remove_all(); - - ::IsUpgraded::put(false); - - // Perform upgrade - Staking::ensure_storage_upgraded(); - - assert_eq!(::IsUpgraded::get(), true); - - // Check migration - assert_eq!(::ErasStartSessionIndex::get(3).unwrap(), 5); - assert_eq!(::ErasRewardPoints::get(3), EraRewardPoints { - total: 12, - individual: vec![(21, 2), (31, 10)].into_iter().collect(), - }); - assert_eq!(::ActiveEra::get().unwrap().index, 3); - assert_eq!(::ActiveEra::get().unwrap().start, Some(777)); - assert_eq!(::CurrentEra::get().unwrap(), 3); - assert_eq!(::ErasStakers::get(3, 11), Exposure { - total: 0, - own: 0, - others: vec![], - }); - assert_eq!(::ErasStakers::get(3, 21), Exposure { - total: 20, - own: 20, - others: vec![], - }); - assert_eq!(::ErasStakers::get(3, 31), Exposure { - total: 30, - own: 30, - others: vec![], - }); - assert_eq!(::ErasStakersClipped::get(3, 11), Exposure { - total: 0, - own: 0, - others: vec![], - }); - assert_eq!(::ErasStakersClipped::get(3, 21), Exposure { - total: 20, - own: 20, - others: vec![], - }); - assert_eq!(::ErasStakersClipped::get(3, 31), Exposure { - total: 30, - own: 30, - others: vec![], - }); - assert_eq!(::ErasValidatorPrefs::get(3, 21), Staking::validators(21)); - assert_eq!(::ErasValidatorPrefs::get(3, 31), Staking::validators(31)); - assert_eq!(::ErasTotalStake::get(3), 50); - }) -} - #[test] fn zero_slash_keeps_nominators() { ExtBuilder::default().build().execute_with(|| { @@ -3071,129 +2976,6 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( }); } -// Test that an upgrade from previous test environment works. -#[test] -fn test_upgrade_from_master_works() { - let data_sets = &[ - test_upgrade_from_master_dataset::_0, - test_upgrade_from_master_dataset::_1, - test_upgrade_from_master_dataset::_2, - test_upgrade_from_master_dataset::_3, - test_upgrade_from_master_dataset::_4, - test_upgrade_from_master_dataset::_5, - test_upgrade_from_master_dataset::_6, - test_upgrade_from_master_dataset::_7, - test_upgrade_from_master_dataset::_8, - ]; - for data_set in data_sets.iter() { - let mut storage = sp_runtime::Storage::default(); - for (key, value) in data_set.iter() { - storage.top.insert(key.to_vec(), value.to_vec()); - } - let mut ext = sp_io::TestExternalities::from(storage); - ext.execute_with(|| { - let old_stakers = - get_storage_value::>(b"Staking", b"CurrentElected", b"").unwrap(); - let old_staker_0 = old_stakers[0]; - let old_staker_1 = old_stakers[1]; - let old_current_era = - get_storage_value::(b"Staking", b"CurrentEra", b"").unwrap(); - let old_staker_0_exposure = get_storage_value::>( - b"Staking", b"Stakers", &blake2_256(&old_staker_0.encode()) - ).unwrap(); - let old_staker_1_exposure = get_storage_value::>( - b"Staking", b"Stakers", &blake2_256(&old_staker_1.encode()) - ).unwrap(); - let ( - old_era_points_earned_total, - old_era_points_earned_individual - ) = get_storage_value::<(u32, Vec)>(b"Staking", b"CurrentEraPointsEarned", b"") - .unwrap_or((0, vec![])); - - Staking::ensure_storage_upgraded(); - assert!(::IsUpgraded::get()); - - // Check ActiveEra and CurrentEra - let active_era = Staking::active_era().unwrap().index; - let current_era = Staking::current_era().unwrap(); - assert!(current_era == active_era); - assert!(current_era == old_current_era); - - // Check ErasStartSessionIndex - let active_era_start = Staking::eras_start_session_index(active_era).unwrap(); - let current_era_start = Staking::eras_start_session_index(current_era).unwrap(); - let current_session_index = Session::current_index(); - assert!(current_era_start == active_era_start); - assert!(active_era_start <= current_session_index); - assert_eq!(::ErasStartSessionIndex::iter().count(), 1); - - // Check ErasStakers - assert_eq!(::ErasStakers::iter().count(), 2); - assert_eq!( - ::ErasStakers::get(current_era, old_staker_0), - old_staker_0_exposure - ); - assert_eq!( - ::ErasStakers::get(current_era, old_staker_1), - old_staker_1_exposure - ); - - // Check ErasStakersClipped - assert_eq!(::ErasStakersClipped::iter().count(), 2); - assert!(::ErasStakersClipped::iter().all(|exposure_clipped| { - let max = ::MaxNominatorRewardedPerValidator::get() as usize; - exposure_clipped.others.len() <= max - })); - assert_eq!( - ::ErasStakersClipped::get(current_era, old_staker_0), - old_staker_0_exposure - ); - assert_eq!( - ::ErasStakersClipped::get(current_era, old_staker_1), - old_staker_1_exposure - ); - - // Check ErasValidatorPrefs - assert_eq!(::ErasValidatorPrefs::iter().count(), 2); - assert_eq!( - ::ErasValidatorPrefs::get(current_era, old_staker_0), - Staking::validators(old_staker_0) - ); - assert_eq!( - ::ErasValidatorPrefs::get(current_era, old_staker_1), - Staking::validators(old_staker_1) - ); - - // Check ErasTotalStake - assert_eq!(::ErasTotalStake::iter().count(), 1); - assert_eq!( - ::ErasTotalStake::get(current_era), - old_staker_0_exposure.total + old_staker_1_exposure.total - ); - - // Check ErasRewardPoints - assert_eq!(::ErasRewardPoints::iter().count(), 1); - let mut individual = BTreeMap::new(); - if let Some(p) = old_era_points_earned_individual.get(0) { - individual.insert(old_staker_0, p.clone()); - } - if let Some(p) = old_era_points_earned_individual.get(1) { - individual.insert(old_staker_1, p.clone()); - } - assert_eq!( - ::ErasRewardPoints::get(current_era), - EraRewardPoints { - total: old_era_points_earned_total, - individual, - } - ); - - // Check ErasValidatorReward - assert_eq!(::ErasValidatorReward::iter().count(), 0); - }); - } -} - #[test] fn set_history_depth_works() { ExtBuilder::default().build().execute_with(|| { From cf9a1898ccbf8d76c985420c3ab3b76237db9c83 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 21:43:12 +0100 Subject: [PATCH 20/24] add trailing newlines --- frame/staking/src/migration/deprecated.rs | 2 +- frame/staking/src/migration/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/migration/deprecated.rs b/frame/staking/src/migration/deprecated.rs index bb37fb284b6aa..42ddefaea770b 100644 --- a/frame/staking/src/migration/deprecated.rs +++ b/frame/staking/src/migration/deprecated.rs @@ -67,4 +67,4 @@ decl_storage! { /// This is keyed by the stash account. pub Stakers: map hasher(blake2_256) T::AccountId => Exposure>; } -} \ No newline at end of file +} diff --git a/frame/staking/src/migration/tests.rs b/frame/staking/src/migration/tests.rs index 90e61edef772f..d1ba35cadcead 100644 --- a/frame/staking/src/migration/tests.rs +++ b/frame/staking/src/migration/tests.rs @@ -217,4 +217,4 @@ fn test_upgrade_from_master_works() { assert_eq!(::ErasValidatorReward::iter().count(), 0); }); } -} \ No newline at end of file +} From fbc40bb42d425f5c969a167f612e4e1834323e81 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 22:15:31 +0100 Subject: [PATCH 21/24] Remove old `IsUpgraded` flag --- frame/balances/src/migration.rs | 2 ++ frame/staking/src/migration/deprecated.rs | 3 +++ frame/staking/src/migration/mod.rs | 2 ++ 3 files changed, 7 insertions(+) diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index 98e753c556b3a..340df48fb075a 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -77,5 +77,7 @@ pub fn on_runtime_upgrade, I: Instance>() { put_storage_value(b"System", b"Account", &hash, (nonce, refs, &balances)); } + take_storage_value::(b"Balances", b"IsUpgraded", &[]); + StorageVersion::::put(Releases::V2_0_0); } diff --git a/frame/staking/src/migration/deprecated.rs b/frame/staking/src/migration/deprecated.rs index 42ddefaea770b..41cf6652291e1 100644 --- a/frame/staking/src/migration/deprecated.rs +++ b/frame/staking/src/migration/deprecated.rs @@ -66,5 +66,8 @@ decl_storage! { /// /// This is keyed by the stash account. pub Stakers: map hasher(blake2_256) T::AccountId => Exposure>; + + /// Old upgrade flag. + pub IsUpgraded: bool; } } diff --git a/frame/staking/src/migration/mod.rs b/frame/staking/src/migration/mod.rs index fbcefd864da52..45e83778aa2f8 100644 --- a/frame/staking/src/migration/mod.rs +++ b/frame/staking/src/migration/mod.rs @@ -49,6 +49,8 @@ mod test_upgrade_from_master_dataset; pub fn on_runtime_upgrade() { if StorageVersion::get() == Releases::V2_0_0 { return }; + deprecated::IsUpgraded::kill(); + let current_era_start_index = deprecated::CurrentEraStartSessionIndex::get(); let current_era = as Store>::CurrentEra::get().unwrap_or(0); let current_era_start = deprecated::CurrentEraStart::::get(); From 0de3f970cf8f2beaaacf5468f7f2f0fe6256a5a2 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Wed, 4 Mar 2020 22:22:30 +0100 Subject: [PATCH 22/24] Update state root --- frame/executive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 754ce52298cd2..4880e2b72d32f 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -584,7 +584,7 @@ mod tests { header: Header { parent_hash: [69u8; 32].into(), number: 1, - state_root: hex!("c96987b52003684f590a6b9b55af9ded88b2e6f8d84441c456f46e59f5e73070").into(), + state_root: hex!("8a22606e925c39bb0c8e8f6f5871c0aceab88a2fcff6b2d92660af8f6daff0b1").into(), extrinsics_root: hex!("03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314").into(), digest: Digest { logs: vec![], }, }, From 446f77ef024b197e95777392f86b1e082b30e80a Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 5 Mar 2020 10:13:31 +0100 Subject: [PATCH 23/24] Exhaustive match statement --- frame/balances/src/migration.rs | 7 ++++++- frame/staking/src/migration/mod.rs | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index 340df48fb075a..faeca4f3cd611 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -2,8 +2,13 @@ use super::*; // Upgrade from the pre-#4649 balances/vesting into the new balances. pub fn on_runtime_upgrade, I: Instance>() { - if StorageVersion::::get() == Releases::V2_0_0 { return }; + match StorageVersion::::get() { + Releases::V2_0_0 => return, + Releases::V1_0_0 => upgrade_v1_to_v2::(), + } +} +fn upgrade_v1_to_v2, I: Instance>() { sp_runtime::print("Upgrading account balances..."); // First, migrate from old FreeBalance to new Account. // We also move all locks across since only accounts with FreeBalance values have locks. diff --git a/frame/staking/src/migration/mod.rs b/frame/staking/src/migration/mod.rs index 45e83778aa2f8..1a946bc25a808 100644 --- a/frame/staking/src/migration/mod.rs +++ b/frame/staking/src/migration/mod.rs @@ -47,8 +47,13 @@ mod tests; mod test_upgrade_from_master_dataset; pub fn on_runtime_upgrade() { - if StorageVersion::get() == Releases::V2_0_0 { return }; + match StorageVersion::get() { + Releases::V2_0_0 => return, + Releases::V1_0_0 => upgrade_v1_to_v2::(), + } +} +fn upgrade_v1_to_v2() { deprecated::IsUpgraded::kill(); let current_era_start_index = deprecated::CurrentEraStartSessionIndex::get(); From d8e76070a28e6c10226008e068ea84f8b58ed2b1 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 5 Mar 2020 13:09:55 +0200 Subject: [PATCH 24/24] Apply suggestions from code review Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/balances/src/migration.rs | 2 +- frame/staking/src/lib.rs | 2 +- frame/staking/src/migration/mod.rs | 14 +++++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/balances/src/migration.rs b/frame/balances/src/migration.rs index faeca4f3cd611..16c764d59f1aa 100644 --- a/frame/balances/src/migration.rs +++ b/frame/balances/src/migration.rs @@ -1,6 +1,5 @@ use super::*; -// Upgrade from the pre-#4649 balances/vesting into the new balances. pub fn on_runtime_upgrade, I: Instance>() { match StorageVersion::::get() { Releases::V2_0_0 => return, @@ -8,6 +7,7 @@ pub fn on_runtime_upgrade, I: Instance>() { } } +// Upgrade from the pre-#4649 balances/vesting into the new balances. fn upgrade_v1_to_v2, I: Instance>() { sp_runtime::print("Upgrading account balances..."); // First, migrate from old FreeBalance to new Account. diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 89ddca0f7af8c..612efecea16b9 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -680,7 +680,7 @@ impl Default for Forcing { fn default() -> Self { Forcing::NotForcing } } -// A value placed in storage that represents the current version of the Balances storage. +// A value placed in storage that represents the current version of the Staking storage. // This value is used by the `on_runtime_upgrade` logic to determine whether we run // storage migration logic. This should match directly with the semantic versions of the Rust crate. #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug)] diff --git a/frame/staking/src/migration/mod.rs b/frame/staking/src/migration/mod.rs index 1a946bc25a808..971e409189188 100644 --- a/frame/staking/src/migration/mod.rs +++ b/frame/staking/src/migration/mod.rs @@ -14,13 +14,13 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -/// Update storage from v1.0.0 to v2.0.0 -/// -/// In old version the staking module has several issue about handling session delay, the -/// current era was always considered the active one. -/// -/// After the migration the current era will still be considered the active one for the era of -/// the upgrade. And the delay issue will be fixed when planning the next era. +//! Update storage from v1.0.0 to v2.0.0 +//! +//! In old version the staking module has several issue about handling session delay, the +//! current era was always considered the active one. +//! +//! After the migration the current era will still be considered the active one for the era of +//! the upgrade. And the delay issue will be fixed when planning the next era. // * create: // * ActiveEraStart // * ErasRewardPoints