From c2766db7c559aa49ae4607fcb1000c5c618e1b04 Mon Sep 17 00:00:00 2001 From: koushiro Date: Mon, 6 Sep 2021 19:59:24 +0800 Subject: [PATCH 1/2] Migrate pallet-tips to the new pallet attribute macro Signed-off-by: koushiro --- Cargo.lock | 1 + frame/tips/Cargo.toml | 19 ++- frame/tips/README.md | 4 +- frame/tips/src/benchmarking.rs | 6 +- frame/tips/src/lib.rs | 285 ++++++++++++++++--------------- frame/tips/src/migrations/mod.rs | 23 +++ frame/tips/src/migrations/v4.rs | 186 ++++++++++++++++++++ frame/tips/src/tests.rs | 207 ++++++++++++---------- 8 files changed, 493 insertions(+), 238 deletions(-) create mode 100644 frame/tips/src/migrations/mod.rs create mode 100644 frame/tips/src/migrations/v4.rs diff --git a/Cargo.lock b/Cargo.lock index 529fbfc17315f..e98a13ea027b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5714,6 +5714,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log 0.4.14", "pallet-balances", "pallet-treasury", "parity-scale-codec", diff --git a/frame/tips/Cargo.toml b/frame/tips/Cargo.toml index a0b554166c049..d706552393e6e 100644 --- a/frame/tips/Cargo.toml +++ b/frame/tips/Cargo.toml @@ -13,10 +13,15 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -serde = { version = "1.0.126", optional = true, features = ["derive"] } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } -sp-std = { version = "4.0.0-dev", default-features = false, path = "../../primitives/std" } +log = { version = "0.4.0", default-features = false } +serde = { version = "1.0.126", features = ["derive"], optional = true } + +sp-core = { version = "4.0.0-dev", default-features = false, path = "../../primitives/core" } +sp-io = { version = "4.0.0-dev", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../primitives/runtime" } +sp-std = { version = "4.0.0-dev", default-features = false, path = "../../primitives/std" } + frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } pallet-treasury = { version = "4.0.0-dev", default-features = false, path = "../treasury" } @@ -24,18 +29,20 @@ pallet-treasury = { version = "4.0.0-dev", default-features = false, path = "../ frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true } [dev-dependencies] -sp-io ={ version = "4.0.0-dev", path = "../../primitives/io" } -sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } sp-storage = { version = "4.0.0-dev", path = "../../primitives/storage" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } [features] default = ["std"] std = [ - "serde", "codec/std", - "sp-std/std", + "log/std", + "serde", + + "sp-core/std", + "sp-io/std", "sp-runtime/std", + "sp-std/std", "frame-support/std", "frame-system/std", "pallet-treasury/std", diff --git a/frame/tips/README.md b/frame/tips/README.md index 36148e276edc2..d885ce770f795 100644 --- a/frame/tips/README.md +++ b/frame/tips/README.md @@ -1,11 +1,11 @@ -# Tipping Module ( pallet-tips ) +# Tipping Pallet ( pallet-tips ) **Note :: This pallet is tightly coupled to pallet-treasury** A subsystem to allow for an agile "tipping" process, whereby a reward may be given without first having a pre-determined stakeholder group come to consensus on how much should be paid. -A group of `Tippers` is determined through the config `Trait`. After half of these have declared +A group of `Tippers` is determined through the config `Config`. After half of these have declared some amount that they believe a particular reported reason deserves, then a countdown period is entered where any remaining members can declare their tip amounts also. After the close of the countdown period, the median of all declared tips is paid to the reported beneficiary, along with diff --git a/frame/tips/src/benchmarking.rs b/frame/tips/src/benchmarking.rs index 2c51f6394a524..5e08121855210 100644 --- a/frame/tips/src/benchmarking.rs +++ b/frame/tips/src/benchmarking.rs @@ -19,13 +19,13 @@ #![cfg(feature = "runtime-benchmarks")] -use super::*; - use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller}; +use frame_support::ensure; use frame_system::RawOrigin; use sp_runtime::traits::Saturating; -use crate::Module as TipsMod; +use super::*; +use crate::Pallet as TipsMod; const SEED: u32 = 0; diff --git a/frame/tips/src/lib.rs b/frame/tips/src/lib.rs index ca327f6c8710e..50abe4684cded 100644 --- a/frame/tips/src/lib.rs +++ b/frame/tips/src/lib.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # Tipping Module ( pallet-tips ) +//! # Tipping Pallet ( pallet-tips ) //! //! > NOTE: This pallet is tightly coupled with pallet-treasury. //! @@ -56,55 +56,31 @@ mod benchmarking; mod tests; + +pub mod migrations; pub mod weights; -use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, ensure, - traits::{Currency, ExistenceRequirement::KeepAlive, Get, ReservableCurrency}, - Parameter, +use sp_runtime::{ + traits::{AccountIdConversion, BadOrigin, Hash, Zero}, + Percent, RuntimeDebug, }; use sp_std::prelude::*; use codec::{Decode, Encode}; -use frame_support::traits::{ContainsLengthBound, EnsureOrigin, OnUnbalanced, SortedMembers}; -use frame_system::{self as system, ensure_signed}; -use sp_runtime::{ - traits::{AccountIdConversion, BadOrigin, Hash, Zero}, - Percent, RuntimeDebug, +use frame_support::{ + traits::{ + ContainsLengthBound, Currency, EnsureOrigin, ExistenceRequirement::KeepAlive, Get, + OnUnbalanced, ReservableCurrency, SortedMembers, StorageVersion, + }, + Parameter, }; + +pub use pallet::*; pub use weights::WeightInfo; pub type BalanceOf = pallet_treasury::BalanceOf; pub type NegativeImbalanceOf = pallet_treasury::NegativeImbalanceOf; -pub trait Config: frame_system::Config + pallet_treasury::Config { - /// Maximum acceptable reason length. - type MaximumReasonLength: Get; - - /// The amount held on deposit per byte within the tip report reason or bounty description. - type DataDepositPerByte: Get>; - - /// Origin from which tippers must come. - /// - /// `ContainsLengthBound::max_len` must be cost free (i.e. no storage read or heavy operation). - type Tippers: SortedMembers + ContainsLengthBound; - - /// The period for which a tip remains open after is has achieved threshold tippers. - type TipCountdown: Get; - - /// The percent of the final tip which goes to the original reporter of the tip. - type TipFindersFee: Get; - - /// The amount held on deposit for placing a tip report. - type TipReportDepositBase: Get>; - - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} - /// An open tipping "motion". Retains all details of a tip including information on the finder /// and the members who have voted. #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] @@ -132,50 +108,96 @@ pub struct OpenTip< finders_fee: bool, } -// Note :: For backward compatability reasons, -// pallet-tips uses Treasury for storage. -// This is temporary solution, soon will get replaced with -// Own storage identifier. -decl_storage! { - trait Store for Module as Treasury { +#[frame_support::pallet] +pub mod pallet { + use super::*; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + /// The current storage version. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + #[pallet::storage_version(STORAGE_VERSION)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: frame_system::Config + pallet_treasury::Config { + /// The overarching event type. + type Event: From> + IsType<::Event>; - /// TipsMap that are not yet completed. Keyed by the hash of `(reason, who)` from the value. - /// This has the insecure enumerable hash function since the key itself is already - /// guaranteed to be a secure hash. - pub Tips get(fn tips): - map hasher(twox_64_concat) T::Hash - => Option, T::BlockNumber, T::Hash>>; + /// Maximum acceptable reason length. + #[pallet::constant] + type MaximumReasonLength: Get; + + /// The amount held on deposit per byte within the tip report reason or bounty description. + #[pallet::constant] + type DataDepositPerByte: Get>; + + /// The period for which a tip remains open after is has achieved threshold tippers. + #[pallet::constant] + type TipCountdown: Get; - /// Simple preimage lookup from the reason's hash to the original data. Again, has an - /// insecure enumerable hash since the key is guaranteed to be the result of a secure hash. - pub Reasons get(fn reasons): map hasher(identity) T::Hash => Option>; + /// The percent of the final tip which goes to the original reporter of the tip. + #[pallet::constant] + type TipFindersFee: Get; + /// The amount held on deposit for placing a tip report. + #[pallet::constant] + type TipReportDepositBase: Get>; + + /// Origin from which tippers must come. + /// + /// `ContainsLengthBound::max_len` must be cost free (i.e. no storage read or heavy + /// operation). + type Tippers: SortedMembers + ContainsLengthBound; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } -} -decl_event!( - pub enum Event - where - Balance = BalanceOf, - ::AccountId, - ::Hash, - { + /// TipsMap that are not yet completed. Keyed by the hash of `(reason, who)` from the value. + /// This has the insecure enumerable hash function since the key itself is already + /// guaranteed to be a secure hash. + #[pallet::storage] + #[pallet::getter(fn tips)] + pub type Tips = StorageMap< + _, + Twox64Concat, + T::Hash, + OpenTip, T::BlockNumber, T::Hash>, + OptionQuery, + >; + + /// Simple preimage lookup from the reason's hash to the original data. Again, has an + /// insecure enumerable hash since the key is guaranteed to be the result of a secure hash. + #[pallet::storage] + #[pallet::getter(fn reasons)] + pub type Reasons = StorageMap<_, Identity, T::Hash, Vec, OptionQuery>; + + #[pallet::event] + #[pallet::metadata(T::Hash = "Hash", T::AccountId = "AccountId", BalanceOf = "Balance")] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + pub enum Event { /// A new tip suggestion has been opened. \[tip_hash\] - NewTip(Hash), + NewTip(T::Hash), /// A tip suggestion has reached threshold and is closing. \[tip_hash\] - TipClosing(Hash), + TipClosing(T::Hash), /// A tip suggestion has been closed. \[tip_hash, who, payout\] - TipClosed(Hash, AccountId, Balance), + TipClosed(T::Hash, T::AccountId, BalanceOf), /// A tip suggestion has been retracted. \[tip_hash\] - TipRetracted(Hash), + TipRetracted(T::Hash), /// A tip suggestion has been slashed. \[tip_hash, finder, deposit\] - TipSlashed(Hash, AccountId, Balance), + TipSlashed(T::Hash, T::AccountId, BalanceOf), } -); -decl_error! { - /// Error for the tips module. - pub enum Error for Module { + /// Old name generated by `decl_event`. + #[deprecated(note = "use `Event` instead")] + pub type RawEvent = Event; + + #[pallet::error] + pub enum Error { /// The reason given is just too big. ReasonTooBig, /// The tip was already found/started. @@ -189,32 +211,9 @@ decl_error! { /// The tip cannot be claimed/closed because it's still in the countdown period. Premature, } -} - -decl_module! { - pub struct Module - for enum Call - where origin: T::Origin - { - /// The period for which a tip remains open after is has achieved threshold tippers. - const TipCountdown: T::BlockNumber = T::TipCountdown::get(); - - /// The amount of the final tip which goes to the original reporter of the tip. - const TipFindersFee: Percent = T::TipFindersFee::get(); - - /// The amount held on deposit for placing a tip report. - const TipReportDepositBase: BalanceOf = T::TipReportDepositBase::get(); - - /// The amount held on deposit per byte within the tip report reason. - const DataDepositPerByte: BalanceOf = T::DataDepositPerByte::get(); - - /// Maximum acceptable reason length. - const MaximumReasonLength: u32 = T::MaximumReasonLength::get(); - - type Error = Error; - - fn deposit_event() = default; + #[pallet::call] + impl Pallet { /// Report something `reason` that deserves a tip and claim any eventual the finder's fee. /// /// The dispatch origin for this call must be _Signed_. @@ -234,19 +233,26 @@ decl_module! { /// - DbReads: `Reasons`, `Tips` /// - DbWrites: `Reasons`, `Tips` /// # - #[weight = ::WeightInfo::report_awesome(reason.len() as u32)] - fn report_awesome(origin, reason: Vec, who: T::AccountId) { + #[pallet::weight(::WeightInfo::report_awesome(reason.len() as u32))] + pub fn report_awesome( + origin: OriginFor, + reason: Vec, + who: T::AccountId, + ) -> DispatchResult { let finder = ensure_signed(origin)?; - ensure!(reason.len() <= T::MaximumReasonLength::get() as usize, Error::::ReasonTooBig); + ensure!( + reason.len() <= T::MaximumReasonLength::get() as usize, + Error::::ReasonTooBig + ); let reason_hash = T::Hashing::hash(&reason[..]); ensure!(!Reasons::::contains_key(&reason_hash), Error::::AlreadyKnown); let hash = T::Hashing::hash_of(&(&reason_hash, &who)); ensure!(!Tips::::contains_key(&hash), Error::::AlreadyKnown); - let deposit = T::TipReportDepositBase::get() - + T::DataDepositPerByte::get() * (reason.len() as u32).into(); + let deposit = T::TipReportDepositBase::get() + + T::DataDepositPerByte::get() * (reason.len() as u32).into(); T::Currency::reserve(&finder, deposit)?; Reasons::::insert(&reason_hash, &reason); @@ -257,10 +263,11 @@ decl_module! { deposit, closes: None, tips: vec![], - finders_fee: true + finders_fee: true, }; Tips::::insert(&hash, tip); - Self::deposit_event(RawEvent::NewTip(hash)); + Self::deposit_event(Event::NewTip(hash)); + Ok(()) } /// Retract a prior tip-report from `report_awesome`, and cancel the process of tipping. @@ -282,8 +289,8 @@ decl_module! { /// - DbReads: `Tips`, `origin account` /// - DbWrites: `Reasons`, `Tips`, `origin account` /// # - #[weight = ::WeightInfo::retract_tip()] - fn retract_tip(origin, hash: T::Hash) { + #[pallet::weight(::WeightInfo::retract_tip())] + pub fn retract_tip(origin: OriginFor, hash: T::Hash) -> DispatchResult { let who = ensure_signed(origin)?; let tip = Tips::::get(&hash).ok_or(Error::::UnknownTip)?; ensure!(tip.finder == who, Error::::NotFinder); @@ -294,7 +301,8 @@ decl_module! { let err_amount = T::Currency::unreserve(&who, tip.deposit); debug_assert!(err_amount.is_zero()); } - Self::deposit_event(RawEvent::TipRetracted(hash)); + Self::deposit_event(Event::TipRetracted(hash)); + Ok(()) } /// Give a tip for something new; no finder's fee will be taken. @@ -312,15 +320,20 @@ decl_module! { /// /// # /// - Complexity: `O(R + T)` where `R` length of `reason`, `T` is the number of tippers. - /// - `O(T)`: decoding `Tipper` vec of length `T` - /// `T` is charged as upper bound given by `ContainsLengthBound`. - /// The actual cost depends on the implementation of `T::Tippers`. + /// - `O(T)`: decoding `Tipper` vec of length `T`. `T` is charged as upper bound given by + /// `ContainsLengthBound`. The actual cost depends on the implementation of + /// `T::Tippers`. /// - `O(R)`: hashing and encoding of reason of length `R` /// - DbReads: `Tippers`, `Reasons` /// - DbWrites: `Reasons`, `Tips` /// # - #[weight = ::WeightInfo::tip_new(reason.len() as u32, T::Tippers::max_len() as u32)] - fn tip_new(origin, reason: Vec, who: T::AccountId, #[compact] tip_value: BalanceOf) { + #[pallet::weight(::WeightInfo::tip_new(reason.len() as u32, T::Tippers::max_len() as u32))] + pub fn tip_new( + origin: OriginFor, + reason: Vec, + who: T::AccountId, + #[pallet::compact] tip_value: BalanceOf, + ) -> DispatchResult { let tipper = ensure_signed(origin)?; ensure!(T::Tippers::contains(&tipper), BadOrigin); let reason_hash = T::Hashing::hash(&reason[..]); @@ -328,7 +341,7 @@ decl_module! { let hash = T::Hashing::hash_of(&(&reason_hash, &who)); Reasons::::insert(&reason_hash, &reason); - Self::deposit_event(RawEvent::NewTip(hash.clone())); + Self::deposit_event(Event::NewTip(hash.clone())); let tips = vec![(tipper.clone(), tip_value)]; let tip = OpenTip { reason: reason_hash, @@ -340,6 +353,7 @@ decl_module! { finders_fee: false, }; Tips::::insert(&hash, tip); + Ok(()) } /// Declare a tip value for an already-open tip. @@ -357,26 +371,30 @@ decl_module! { /// has started. /// /// # - /// - Complexity: `O(T)` where `T` is the number of tippers. - /// decoding `Tipper` vec of length `T`, insert tip and check closing, - /// `T` is charged as upper bound given by `ContainsLengthBound`. - /// The actual cost depends on the implementation of `T::Tippers`. + /// - Complexity: `O(T)` where `T` is the number of tippers. decoding `Tipper` vec of length + /// `T`, insert tip and check closing, `T` is charged as upper bound given by + /// `ContainsLengthBound`. The actual cost depends on the implementation of `T::Tippers`. /// /// Actually weight could be lower as it depends on how many tips are in `OpenTip` but it /// is weighted as if almost full i.e of length `T-1`. /// - DbReads: `Tippers`, `Tips` /// - DbWrites: `Tips` /// # - #[weight = ::WeightInfo::tip(T::Tippers::max_len() as u32)] - fn tip(origin, hash: T::Hash, #[compact] tip_value: BalanceOf) { + #[pallet::weight(::WeightInfo::tip(T::Tippers::max_len() as u32))] + pub fn tip( + origin: OriginFor, + hash: T::Hash, + #[pallet::compact] tip_value: BalanceOf, + ) -> DispatchResult { let tipper = ensure_signed(origin)?; ensure!(T::Tippers::contains(&tipper), BadOrigin); let mut tip = Tips::::get(hash).ok_or(Error::::UnknownTip)?; if Self::insert_tip_and_check_closing(&mut tip, tipper, tip_value) { - Self::deposit_event(RawEvent::TipClosing(hash.clone())); + Self::deposit_event(Event::TipClosing(hash.clone())); } Tips::::insert(&hash, tip); + Ok(()) } /// Close and payout a tip. @@ -389,24 +407,24 @@ decl_module! { /// as the hash of the tuple of the original tip `reason` and the beneficiary account ID. /// /// # - /// - Complexity: `O(T)` where `T` is the number of tippers. - /// decoding `Tipper` vec of length `T`. - /// `T` is charged as upper bound given by `ContainsLengthBound`. - /// The actual cost depends on the implementation of `T::Tippers`. + /// - Complexity: `O(T)` where `T` is the number of tippers. decoding `Tipper` vec of length + /// `T`. `T` is charged as upper bound given by `ContainsLengthBound`. The actual cost + /// depends on the implementation of `T::Tippers`. /// - DbReads: `Tips`, `Tippers`, `tip finder` /// - DbWrites: `Reasons`, `Tips`, `Tippers`, `tip finder` /// # - #[weight = ::WeightInfo::close_tip(T::Tippers::max_len() as u32)] - fn close_tip(origin, hash: T::Hash) { + #[pallet::weight(::WeightInfo::close_tip(T::Tippers::max_len() as u32))] + pub fn close_tip(origin: OriginFor, hash: T::Hash) -> DispatchResult { ensure_signed(origin)?; let tip = Tips::::get(hash).ok_or(Error::::UnknownTip)?; let n = tip.closes.as_ref().ok_or(Error::::StillOpen)?; - ensure!(system::Pallet::::block_number() >= *n, Error::::Premature); + ensure!(frame_system::Pallet::::block_number() >= *n, Error::::Premature); // closed. Reasons::::remove(&tip.reason); Tips::::remove(hash); Self::payout_tip(hash, tip); + Ok(()) } /// Remove and slash an already-open tip. @@ -421,8 +439,8 @@ decl_module! { /// `T` is charged as upper bound given by `ContainsLengthBound`. /// The actual cost depends on the implementation of `T::Tippers`. /// # - #[weight = ::WeightInfo::slash_tip(T::Tippers::max_len() as u32)] - fn slash_tip(origin, hash: T::Hash) { + #[pallet::weight(::WeightInfo::slash_tip(T::Tippers::max_len() as u32))] + pub fn slash_tip(origin: OriginFor, hash: T::Hash) -> DispatchResult { T::RejectOrigin::ensure_origin(origin)?; let tip = Tips::::take(hash).ok_or(Error::::UnknownTip)?; @@ -432,12 +450,13 @@ decl_module! { T::OnSlash::on_unbalanced(imbalance); } Reasons::::remove(&tip.reason); - Self::deposit_event(RawEvent::TipSlashed(hash, tip.finder, tip.deposit)); + Self::deposit_event(Event::TipSlashed(hash, tip.finder, tip.deposit)); + Ok(()) } } } -impl Module { +impl Pallet { // Add public immutables and private mutables. /// The account ID of the treasury pot. @@ -464,7 +483,7 @@ impl Module { Self::retain_active_tips(&mut tip.tips); let threshold = (T::Tippers::count() + 1) / 2; if tip.tips.len() >= threshold && tip.closes.is_none() { - tip.closes = Some(system::Pallet::::block_number() + T::TipCountdown::get()); + tip.closes = Some(frame_system::Pallet::::block_number() + T::TipCountdown::get()); true } else { false @@ -526,10 +545,10 @@ impl Module { // same as above: best-effort only. let res = T::Currency::transfer(&treasury, &tip.who, payout, KeepAlive); debug_assert!(res.is_ok()); - Self::deposit_event(RawEvent::TipClosed(hash, tip.who, payout)); + Self::deposit_event(Event::TipClosed(hash, tip.who, payout)); } - pub fn migrate_retract_tip_for_tip_new() { + pub fn migrate_retract_tip_for_tip_new(module: &[u8], item: &[u8]) { /// An open tipping "motion". Retains all details of a tip including information on the /// finder and the members who have voted. #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] @@ -559,7 +578,7 @@ impl Module { T::Hash, OldOpenTip, T::BlockNumber, T::Hash>, Twox64Concat, - >(b"Treasury", b"Tips") + >(module, item) .drain() { let (finder, deposit, finders_fee) = match old_tip.finder { diff --git a/frame/tips/src/migrations/mod.rs b/frame/tips/src/migrations/mod.rs new file mode 100644 index 0000000000000..81139120da1c8 --- /dev/null +++ b/frame/tips/src/migrations/mod.rs @@ -0,0 +1,23 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// Version 4. +/// +/// For backward compatability reasons, pallet-tips uses `Treasury` for storage module prefix +/// before calling this migration. After calling this migration, it will get replaced with +/// own storage identifier. +pub mod v4; diff --git a/frame/tips/src/migrations/v4.rs b/frame/tips/src/migrations/v4.rs new file mode 100644 index 0000000000000..e00e45beaa66f --- /dev/null +++ b/frame/tips/src/migrations/v4.rs @@ -0,0 +1,186 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use sp_core::hexdisplay::HexDisplay; +use sp_io::{hashing::twox_128, storage}; +use sp_std::str; + +use frame_support::{ + storage::StoragePrefixedMap, + traits::{ + Get, GetStorageVersion, PalletInfoAccess, StorageVersion, + STORAGE_VERSION_STORAGE_KEY_POSTFIX, + }, + weights::Weight, +}; + +use crate as pallet_tips; + +/// Migrate the entire storage of this pallet to a new prefix. +/// +/// This new prefix must be the same as the one set in construct_runtime. +/// For safety, use `PalletInfo` to get it, as: +/// `::PalletInfo::name::`. +/// +/// The migration will look into the storage version in order not to trigger a migration on an up +/// to date storage. Thus the on chain storage version must be less than 4 in order to trigger the +/// migration. +pub fn migrate>( + old_pallet_name: N, + new_pallet_name: N, +) -> Weight { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name = new_pallet_name.as_ref(); + + if new_pallet_name == old_pallet_name { + log::info!( + target: "runtime::tips", + "New pallet name is equal to the old prefix. No migration needs to be done.", + ); + return 0 + } + + let on_chain_storage_version =

::on_chain_storage_version(); + log::info!( + target: "runtime::tips", + "Running migration to v4 for tips with storage version {:?}", + on_chain_storage_version, + ); + + if on_chain_storage_version < 4 { + let storage_prefix = pallet_tips::Tips::::storage_prefix(); + frame_support::storage::migration::move_storage_from_pallet( + storage_prefix, + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + + let storage_prefix = pallet_tips::Reasons::::storage_prefix(); + frame_support::storage::migration::move_storage_from_pallet( + storage_prefix, + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + log_migration("migration", storage_prefix, old_pallet_name, new_pallet_name); + + StorageVersion::new(4).put::

(); + ::BlockWeights::get().max_block + } else { + log::warn!( + target: "runtime::tips", + "Attempted to apply migration to v4 but failed because storage version is {:?}", + on_chain_storage_version, + ); + 0 + } +} + +/// Some checks prior to migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn pre_migrate>( + old_pallet_name: N, + new_pallet_name: N, +) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name = new_pallet_name.as_ref(); + + let storage_prefix_tips = pallet_tips::Tips::::storage_prefix(); + let storage_prefix_reasons = pallet_tips::Reasons::::storage_prefix(); + + log_migration("pre-migration", storage_prefix_tips, old_pallet_name, new_pallet_name); + log_migration("pre-migration", storage_prefix_reasons, old_pallet_name, new_pallet_name); + + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_tips_key = [&old_pallet_prefix, &twox_128(storage_prefix_tips)[..]].concat(); + let old_reasons_key = [&old_pallet_prefix, &twox_128(storage_prefix_reasons)[..]].concat(); + assert!(storage::next_key(&old_tips_key) + .map_or(true, |next_key| next_key.starts_with(&old_tips_key))); + assert!(storage::next_key(&old_reasons_key) + .map_or(true, |next_key| next_key.starts_with(&old_reasons_key))); + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let storage_version_key = + [&new_pallet_prefix, &twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX)[..]].concat(); + // ensure nothing is stored in the new prefix. + assert!( + storage::next_key(&new_pallet_prefix).map_or( + // either nothing is there + true, + // or we ensure that it has no common prefix with twox_128(new), + // or isn't the pallet version that is already stored using the pallet name + |next_key| { + !next_key.starts_with(&new_pallet_prefix) || next_key == storage_version_key + }, + ), + "unexpected next_key({}) = {:?}", + new_pallet_name, + HexDisplay::from(&storage::next_key(&new_pallet_prefix).unwrap()), + ); + assert!(

::on_chain_storage_version() < 4); +} + +/// Some checks for after migration. This can be linked to +/// [`frame_support::traits::OnRuntimeUpgrade::post_upgrade`] for further testing. +/// +/// Panics if anything goes wrong. +pub fn post_migrate>( + old_pallet_name: N, + new_pallet_name: N, +) { + let old_pallet_name = old_pallet_name.as_ref(); + let new_pallet_name = new_pallet_name.as_ref(); + + let storage_prefix_tips = pallet_tips::Tips::::storage_prefix(); + let storage_prefix_reasons = pallet_tips::Reasons::::storage_prefix(); + + log_migration("post-migration", storage_prefix_tips, old_pallet_name, new_pallet_name); + log_migration("post-migration", storage_prefix_reasons, old_pallet_name, new_pallet_name); + + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_tips_key = [&old_pallet_prefix, &twox_128(storage_prefix_tips)[..]].concat(); + let old_reasons_key = [&old_pallet_prefix, &twox_128(storage_prefix_reasons)[..]].concat(); + // Assert that nothing remains at the old prefix + assert!(storage::next_key(&old_tips_key) + .map_or(true, |next_key| !next_key.starts_with(&old_tips_key))); + assert!(storage::next_key(&old_reasons_key) + .map_or(true, |next_key| !next_key.starts_with(&old_reasons_key))); + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let new_tips_key = [&new_pallet_prefix, &twox_128(storage_prefix_tips)[..]].concat(); + let new_reasons_key = [&new_pallet_prefix, &twox_128(storage_prefix_reasons)[..]].concat(); + // Assert that the `Tips` and `Reasons` storages have been moved to the new prefix + assert!(storage::next_key(&new_tips_key) + .map_or(true, |next_key| next_key.starts_with(&new_tips_key))); + assert!(storage::next_key(&new_reasons_key) + .map_or(true, |next_key| next_key.starts_with(&new_reasons_key))); + + assert_eq!(

::on_chain_storage_version(), 4); +} + +fn log_migration(stage: &str, storage_prefix: &[u8], old_pallet_name: &str, new_pallet_name: &str) { + log::info!( + target: "runtime::tips", + "{} prefix of storage '{}': '{}' ==> '{}'", + stage, + str::from_utf8(storage_prefix).unwrap_or(""), + old_pallet_name, + new_pallet_name, + ); +} diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 8611320563c7d..23f137924d547 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -19,19 +19,23 @@ #![cfg(test)] -use super::*; -use crate as tips; -use frame_support::{ - assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, traits::SortedMembers, - weights::Weight, PalletId, -}; +use std::cell::RefCell; + use sp_core::H256; use sp_runtime::{ testing::Header, traits::{BadOrigin, BlakeTwo256, IdentityLookup}, Perbill, Permill, }; -use std::cell::RefCell; +use sp_storage::Storage; + +use frame_support::{ + assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, + storage::StoragePrefixedMap, traits::SortedMembers, weights::Weight, PalletId, +}; + +use super::*; +use crate::{self as pallet_tips, Event as TipEvent}; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -45,7 +49,7 @@ frame_support::construct_runtime!( System: frame_system::{Pallet, Call, Config, Storage, Event}, Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Treasury: pallet_treasury::{Pallet, Call, Storage, Config, Event}, - TipsModTestInst: tips::{Pallet, Call, Storage, Event}, + Tips: pallet_tips::{Pallet, Call, Storage, Event}, } ); @@ -173,11 +177,11 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } -fn last_event() -> RawEvent { +fn last_event() -> TipEvent { System::events() .into_iter() .map(|r| r.event) - .filter_map(|e| if let Event::TipsModTestInst(inner) = e { Some(inner) } else { None }) + .filter_map(|e| if let Event::Tips(inner) = e { Some(inner) } else { None }) .last() .unwrap() } @@ -198,9 +202,9 @@ fn tip_hash() -> H256 { fn tip_new_cannot_be_used_twice() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); + assert_ok!(Tips::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); assert_noop!( - TipsModTestInst::tip_new(Origin::signed(11), b"awesome.dot".to_vec(), 3, 10), + Tips::tip_new(Origin::signed(11), b"awesome.dot".to_vec(), 3, 10), Error::::AlreadyKnown ); }); @@ -210,23 +214,23 @@ fn tip_new_cannot_be_used_twice() { fn report_awesome_and_tip_works() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); + assert_ok!(Tips::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); assert_eq!(Balances::reserved_balance(0), 12); assert_eq!(Balances::free_balance(0), 88); // other reports don't count. assert_noop!( - TipsModTestInst::report_awesome(Origin::signed(1), b"awesome.dot".to_vec(), 3), + Tips::report_awesome(Origin::signed(1), b"awesome.dot".to_vec(), 3), Error::::AlreadyKnown ); let h = tip_hash(); - assert_ok!(TipsModTestInst::tip(Origin::signed(10), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10)); - assert_noop!(TipsModTestInst::tip(Origin::signed(9), h.clone(), 10), BadOrigin); + assert_ok!(Tips::tip(Origin::signed(10), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10)); + assert_noop!(Tips::tip(Origin::signed(9), h.clone(), 10), BadOrigin); System::set_block_number(2); - assert_ok!(TipsModTestInst::close_tip(Origin::signed(100), h.into())); + assert_ok!(Tips::close_tip(Origin::signed(100), h.into())); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 102); assert_eq!(Balances::free_balance(3), 8); @@ -237,15 +241,15 @@ fn report_awesome_and_tip_works() { fn report_awesome_from_beneficiary_and_tip_works() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 0)); + assert_ok!(Tips::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 0)); assert_eq!(Balances::reserved_balance(0), 12); assert_eq!(Balances::free_balance(0), 88); let h = BlakeTwo256::hash_of(&(BlakeTwo256::hash(b"awesome.dot"), 0u128)); - assert_ok!(TipsModTestInst::tip(Origin::signed(10), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(10), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10)); System::set_block_number(2); - assert_ok!(TipsModTestInst::close_tip(Origin::signed(100), h.into())); + assert_ok!(Tips::close_tip(Origin::signed(100), h.into())); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 110); }); @@ -259,39 +263,30 @@ fn close_tip_works() { Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_eq!(Treasury::pot(), 100); - assert_ok!(TipsModTestInst::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); + assert_ok!(Tips::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); let h = tip_hash(); - assert_eq!(last_event(), RawEvent::NewTip(h)); + assert_eq!(last_event(), TipEvent::NewTip(h)); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); - assert_noop!( - TipsModTestInst::close_tip(Origin::signed(0), h.into()), - Error::::StillOpen - ); + assert_noop!(Tips::close_tip(Origin::signed(0), h.into()), Error::::StillOpen); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10)); - assert_eq!(last_event(), RawEvent::TipClosing(h)); + assert_eq!(last_event(), TipEvent::TipClosing(h)); - assert_noop!( - TipsModTestInst::close_tip(Origin::signed(0), h.into()), - Error::::Premature - ); + assert_noop!(Tips::close_tip(Origin::signed(0), h.into()), Error::::Premature); System::set_block_number(2); - assert_noop!(TipsModTestInst::close_tip(Origin::none(), h.into()), BadOrigin); - assert_ok!(TipsModTestInst::close_tip(Origin::signed(0), h.into())); + assert_noop!(Tips::close_tip(Origin::none(), h.into()), BadOrigin); + assert_ok!(Tips::close_tip(Origin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); - assert_eq!(last_event(), RawEvent::TipClosed(h, 3, 10)); + assert_eq!(last_event(), TipEvent::TipClosed(h, 3, 10)); - assert_noop!( - TipsModTestInst::close_tip(Origin::signed(100), h.into()), - Error::::UnknownTip - ); + assert_noop!(Tips::close_tip(Origin::signed(100), h.into()), Error::::UnknownTip); }); } @@ -305,20 +300,20 @@ fn slash_tip_works() { assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 100); - assert_ok!(TipsModTestInst::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); + assert_ok!(Tips::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); assert_eq!(Balances::reserved_balance(0), 12); assert_eq!(Balances::free_balance(0), 88); let h = tip_hash(); - assert_eq!(last_event(), RawEvent::NewTip(h)); + assert_eq!(last_event(), TipEvent::NewTip(h)); // can't remove from any origin - assert_noop!(TipsModTestInst::slash_tip(Origin::signed(0), h.clone()), BadOrigin); + assert_noop!(Tips::slash_tip(Origin::signed(0), h.clone()), BadOrigin); // can remove from root. - assert_ok!(TipsModTestInst::slash_tip(Origin::root(), h.clone())); - assert_eq!(last_event(), RawEvent::TipSlashed(h, 0, 12)); + assert_ok!(Tips::slash_tip(Origin::root(), h.clone())); + assert_eq!(last_event(), TipEvent::TipSlashed(h, 0, 12)); // tipper slashed assert_eq!(Balances::reserved_balance(0), 0); @@ -331,38 +326,26 @@ fn retract_tip_works() { new_test_ext().execute_with(|| { // with report awesome Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); + assert_ok!(Tips::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); let h = tip_hash(); - assert_ok!(TipsModTestInst::tip(Origin::signed(10), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10)); - assert_noop!( - TipsModTestInst::retract_tip(Origin::signed(10), h.clone()), - Error::::NotFinder - ); - assert_ok!(TipsModTestInst::retract_tip(Origin::signed(0), h.clone())); + assert_ok!(Tips::tip(Origin::signed(10), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10)); + assert_noop!(Tips::retract_tip(Origin::signed(10), h.clone()), Error::::NotFinder); + assert_ok!(Tips::retract_tip(Origin::signed(0), h.clone())); System::set_block_number(2); - assert_noop!( - TipsModTestInst::close_tip(Origin::signed(0), h.into()), - Error::::UnknownTip - ); + assert_noop!(Tips::close_tip(Origin::signed(0), h.into()), Error::::UnknownTip); // with tip new Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); + assert_ok!(Tips::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); let h = tip_hash(); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10)); - assert_noop!( - TipsModTestInst::retract_tip(Origin::signed(0), h.clone()), - Error::::NotFinder - ); - assert_ok!(TipsModTestInst::retract_tip(Origin::signed(10), h.clone())); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10)); + assert_noop!(Tips::retract_tip(Origin::signed(0), h.clone()), Error::::NotFinder); + assert_ok!(Tips::retract_tip(Origin::signed(10), h.clone())); System::set_block_number(2); - assert_noop!( - TipsModTestInst::close_tip(Origin::signed(10), h.into()), - Error::::UnknownTip - ); + assert_noop!(Tips::close_tip(Origin::signed(10), h.into()), Error::::UnknownTip); }); } @@ -370,12 +353,12 @@ fn retract_tip_works() { fn tip_median_calculation_works() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 0)); + assert_ok!(Tips::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 0)); let h = tip_hash(); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 1000000)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 1000000)); System::set_block_number(2); - assert_ok!(TipsModTestInst::close_tip(Origin::signed(0), h.into())); + assert_ok!(Tips::close_tip(Origin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); }); } @@ -384,25 +367,23 @@ fn tip_median_calculation_works() { fn tip_changing_works() { new_test_ext().execute_with(|| { Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(TipsModTestInst::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10000)); + assert_ok!(Tips::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10000)); let h = tip_hash(); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 10000)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 10000)); - assert_ok!(TipsModTestInst::tip(Origin::signed(13), h.clone(), 0)); - assert_ok!(TipsModTestInst::tip(Origin::signed(14), h.clone(), 0)); - assert_ok!(TipsModTestInst::tip(Origin::signed(12), h.clone(), 1000)); - assert_ok!(TipsModTestInst::tip(Origin::signed(11), h.clone(), 100)); - assert_ok!(TipsModTestInst::tip(Origin::signed(10), h.clone(), 10)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 10000)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 10000)); + assert_ok!(Tips::tip(Origin::signed(13), h.clone(), 0)); + assert_ok!(Tips::tip(Origin::signed(14), h.clone(), 0)); + assert_ok!(Tips::tip(Origin::signed(12), h.clone(), 1000)); + assert_ok!(Tips::tip(Origin::signed(11), h.clone(), 100)); + assert_ok!(Tips::tip(Origin::signed(10), h.clone(), 10)); System::set_block_number(2); - assert_ok!(TipsModTestInst::close_tip(Origin::signed(0), h.into())); + assert_ok!(Tips::close_tip(Origin::signed(0), h.into())); assert_eq!(Balances::free_balance(3), 10); }); } #[test] fn test_last_reward_migration() { - use sp_storage::Storage; - let mut s = Storage::default(); #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] @@ -449,18 +430,20 @@ fn test_last_reward_migration() { }; let data = vec![ - (Tips::::hashed_key_for(hash1), old_tip_finder.encode().to_vec()), - (Tips::::hashed_key_for(hash2), old_tip_no_finder.encode().to_vec()), + (pallet_tips::Tips::::hashed_key_for(hash1), old_tip_finder.encode().to_vec()), + (pallet_tips::Tips::::hashed_key_for(hash2), old_tip_no_finder.encode().to_vec()), ]; s.top = data.into_iter().collect(); sp_io::TestExternalities::new(s).execute_with(|| { - TipsModTestInst::migrate_retract_tip_for_tip_new(); + let module = pallet_tips::Tips::::module_prefix(); + let item = pallet_tips::Tips::::storage_prefix(); + Tips::migrate_retract_tip_for_tip_new(module, item); // Test w/ finder assert_eq!( - Tips::::get(hash1), + pallet_tips::Tips::::get(hash1), Some(OpenTip { reason: reason1, who: 10, @@ -474,7 +457,7 @@ fn test_last_reward_migration() { // Test w/o finder assert_eq!( - Tips::::get(hash2), + pallet_tips::Tips::::get(hash2), Some(OpenTip { reason: reason2, who: 20, @@ -488,6 +471,42 @@ fn test_last_reward_migration() { }); } +#[test] +fn test_migration_v4() { + let mut s = Storage::default(); + + let reason1 = BlakeTwo256::hash(b"reason1"); + let hash1 = BlakeTwo256::hash_of(&(reason1, 10u64)); + + let tip = OpenTip:: { + reason: reason1, + who: 10, + finder: 20, + deposit: 30, + closes: Some(13), + tips: vec![(40, 50), (60, 70)], + finders_fee: true, + }; + + let data = vec![ + (pallet_tips::Reasons::::hashed_key_for(hash1), reason1.encode().to_vec()), + (pallet_tips::Tips::::hashed_key_for(hash1), tip.encode().to_vec()), + ]; + + s.top = data.into_iter().collect(); + + sp_io::TestExternalities::new(s).execute_with(|| { + use frame_support::traits::PalletInfo; + let old_pallet_name = ::PalletInfo::name::() + .expect("Tips is part of runtime, so it has a name; qed"); + let new_pallet_name = "NewTips"; + + crate::migrations::v4::pre_migrate::(old_pallet_name, new_pallet_name); + crate::migrations::v4::migrate::(old_pallet_name, new_pallet_name); + crate::migrations::v4::post_migrate::(old_pallet_name, new_pallet_name); + }); +} + #[test] fn genesis_funding_works() { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); From de5c8c316a67edd9079ba10bc0c1c18fca80f758 Mon Sep 17 00:00:00 2001 From: koushiro Date: Mon, 13 Sep 2021 15:14:22 +0800 Subject: [PATCH 2/2] Fix migration Signed-off-by: koushiro --- frame/tips/src/migrations/v4.rs | 99 ++++++++++++++++++--------------- frame/tips/src/tests.rs | 40 +++++++++---- 2 files changed, 84 insertions(+), 55 deletions(-) diff --git a/frame/tips/src/migrations/v4.rs b/frame/tips/src/migrations/v4.rs index e00e45beaa66f..69df1d08d2c8a 100644 --- a/frame/tips/src/migrations/v4.rs +++ b/frame/tips/src/migrations/v4.rs @@ -15,8 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use sp_core::hexdisplay::HexDisplay; -use sp_io::{hashing::twox_128, storage}; +use sp_io::hashing::twox_128; use sp_std::str; use frame_support::{ @@ -41,10 +40,9 @@ use crate as pallet_tips; /// migration. pub fn migrate>( old_pallet_name: N, - new_pallet_name: N, ) -> Weight { let old_pallet_name = old_pallet_name.as_ref(); - let new_pallet_name = new_pallet_name.as_ref(); + let new_pallet_name =

::name(); if new_pallet_name == old_pallet_name { log::info!( @@ -94,12 +92,15 @@ pub fn migrate>( +pub fn pre_migrate< + T: pallet_tips::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( old_pallet_name: N, - new_pallet_name: N, ) { let old_pallet_name = old_pallet_name.as_ref(); - let new_pallet_name = new_pallet_name.as_ref(); + let new_pallet_name =

::name(); let storage_prefix_tips = pallet_tips::Tips::::storage_prefix(); let storage_prefix_reasons = pallet_tips::Reasons::::storage_prefix(); @@ -107,32 +108,22 @@ pub fn pre_migrate::on_chain_storage_version() < 4); } @@ -140,12 +131,15 @@ pub fn pre_migrate>( +pub fn post_migrate< + T: pallet_tips::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( old_pallet_name: N, - new_pallet_name: N, ) { let old_pallet_name = old_pallet_name.as_ref(); - let new_pallet_name = new_pallet_name.as_ref(); + let new_pallet_name =

::name(); let storage_prefix_tips = pallet_tips::Tips::::storage_prefix(); let storage_prefix_reasons = pallet_tips::Reasons::::storage_prefix(); @@ -153,23 +147,38 @@ pub fn post_migrate> log_migration("post-migration", storage_prefix_tips, old_pallet_name, new_pallet_name); log_migration("post-migration", storage_prefix_reasons, old_pallet_name, new_pallet_name); + if new_pallet_name == old_pallet_name { + return + } + + // Assert that no `Tips` and `Reasons` storages remains at the old prefix. let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); let old_tips_key = [&old_pallet_prefix, &twox_128(storage_prefix_tips)[..]].concat(); + let old_tips_key_iter = frame_support::storage::KeyPrefixIterator::new( + old_tips_key.to_vec(), + old_tips_key.to_vec(), + |_| Ok(()), + ); + assert_eq!(old_tips_key_iter.count(), 0); + let old_reasons_key = [&old_pallet_prefix, &twox_128(storage_prefix_reasons)[..]].concat(); - // Assert that nothing remains at the old prefix - assert!(storage::next_key(&old_tips_key) - .map_or(true, |next_key| !next_key.starts_with(&old_tips_key))); - assert!(storage::next_key(&old_reasons_key) - .map_or(true, |next_key| !next_key.starts_with(&old_reasons_key))); + let old_reasons_key_iter = frame_support::storage::KeyPrefixIterator::new( + old_reasons_key.to_vec(), + old_reasons_key.to_vec(), + |_| Ok(()), + ); + assert_eq!(old_reasons_key_iter.count(), 0); + // Assert that the `Tips` and `Reasons` storages (if they exist) have been moved to the new + // prefix. + // NOTE: storage_version_key is already in the new prefix. let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); - let new_tips_key = [&new_pallet_prefix, &twox_128(storage_prefix_tips)[..]].concat(); - let new_reasons_key = [&new_pallet_prefix, &twox_128(storage_prefix_reasons)[..]].concat(); - // Assert that the `Tips` and `Reasons` storages have been moved to the new prefix - assert!(storage::next_key(&new_tips_key) - .map_or(true, |next_key| next_key.starts_with(&new_tips_key))); - assert!(storage::next_key(&new_reasons_key) - .map_or(true, |next_key| next_key.starts_with(&new_reasons_key))); + let new_pallet_prefix_iter = frame_support::storage::KeyPrefixIterator::new( + new_pallet_prefix.to_vec(), + new_pallet_prefix.to_vec(), + |_| Ok(()), + ); + assert!(new_pallet_prefix_iter.count() >= 1); assert_eq!(

::on_chain_storage_version(), 4); } diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 23f137924d547..7ea80d78c5532 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -473,8 +473,6 @@ fn test_last_reward_migration() { #[test] fn test_migration_v4() { - let mut s = Storage::default(); - let reason1 = BlakeTwo256::hash(b"reason1"); let hash1 = BlakeTwo256::hash_of(&(reason1, 10u64)); @@ -493,17 +491,39 @@ fn test_migration_v4() { (pallet_tips::Tips::::hashed_key_for(hash1), tip.encode().to_vec()), ]; + let mut s = Storage::default(); s.top = data.into_iter().collect(); sp_io::TestExternalities::new(s).execute_with(|| { - use frame_support::traits::PalletInfo; - let old_pallet_name = ::PalletInfo::name::() - .expect("Tips is part of runtime, so it has a name; qed"); - let new_pallet_name = "NewTips"; - - crate::migrations::v4::pre_migrate::(old_pallet_name, new_pallet_name); - crate::migrations::v4::migrate::(old_pallet_name, new_pallet_name); - crate::migrations::v4::post_migrate::(old_pallet_name, new_pallet_name); + use frame_support::traits::PalletInfoAccess; + + let old_pallet = "Treasury"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet); + crate::migrations::v4::migrate::(old_pallet); + crate::migrations::v4::post_migrate::(old_pallet); + }); + + sp_io::TestExternalities::new(Storage::default()).execute_with(|| { + use frame_support::traits::PalletInfoAccess; + + let old_pallet = "Treasury"; + let new_pallet = ::name(); + frame_support::storage::migration::move_pallet( + new_pallet.as_bytes(), + old_pallet.as_bytes(), + ); + StorageVersion::new(0).put::(); + + crate::migrations::v4::pre_migrate::(old_pallet); + crate::migrations::v4::migrate::(old_pallet); + crate::migrations::v4::post_migrate::(old_pallet); }); }