From e2cb2115aa71a65ec23224e9513a834d7c2b9ea2 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 17:18:40 -0400 Subject: [PATCH 01/21] migrate bounties pallet --- frame/bounties/src/lib.rs | 315 +++++++++++++++++++----------------- frame/bounties/src/tests.rs | 10 +- 2 files changed, 168 insertions(+), 157 deletions(-) diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index a5be4a00cb94c..e0b5e82d32869 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -80,8 +80,6 @@ pub mod weights; use sp_std::prelude::*; -use frame_support::{decl_error, decl_event, decl_module, decl_storage, ensure}; - use frame_support::traits::{ Currency, ExistenceRequirement::AllowDeath, Get, Imbalance, OnUnbalanced, ReservableCurrency, }; @@ -93,44 +91,17 @@ use sp_runtime::{ use frame_support::{dispatch::DispatchResultWithPostInfo, traits::EnsureOrigin}; -use frame_support::weights::Weight; - -use codec::{Decode, Encode}; -use frame_system::{self as system, ensure_signed}; +use frame_system::pallet_prelude::*; +use frame_support::pallet_prelude::*; pub use weights::WeightInfo; +pub use pallet::*; + type BalanceOf = pallet_treasury::BalanceOf; type PositiveImbalanceOf = pallet_treasury::PositiveImbalanceOf; -pub trait Config: frame_system::Config + pallet_treasury::Config { - /// The amount held on deposit for placing a bounty proposal. - type BountyDepositBase: Get>; - - /// The delay period for which a bounty beneficiary need to wait before claim the payout. - type BountyDepositPayoutDelay: Get; - - /// Bounty duration in blocks. - type BountyUpdatePeriod: Get; - - /// Percentage of the curator fee that will be reserved upfront as deposit for bounty curator. - type BountyCuratorDeposit: Get; - - /// Minimum value for a bounty. - type BountyValueMinimum: Get>; - - /// The amount held on deposit per byte within the tip report reason or bounty description. - type DataDepositPerByte: Get>; - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// Maximum acceptable reason length. - type MaximumReasonLength: Get; - - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} /// An index of a bounty. Just a `u32`. pub type BountyIndex = u32; @@ -185,55 +156,53 @@ pub enum BountyStatus { }, } -// Note :: For backward compatibility reasons, -// pallet-bounties uses Treasury for storage. -// This is temporary solution, soon will get replaced with -// Own storage identifier. -decl_storage! { - trait Store for Module as Treasury { - - /// Number of bounty proposals that have been made. - pub BountyCount get(fn bounty_count): BountyIndex; +#[frame_support::pallet] +pub mod pallet{ + use super::*; - /// Bounties that have been made. - pub Bounties get(fn bounties): - map hasher(twox_64_concat) BountyIndex - => Option, T::BlockNumber>>; - - /// The description of each bounty. - pub BountyDescriptions get(fn bounty_descriptions): map hasher(twox_64_concat) BountyIndex => Option>; - - /// Bounty indices that have been approved but not yet funded. - pub BountyApprovals get(fn bounty_approvals): Vec; - } -} + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); -decl_event!( - pub enum Event - where - Balance = BalanceOf, - ::AccountId, - { - /// New bounty proposal. \[index\] - BountyProposed(BountyIndex), - /// A bounty proposal was rejected; funds were slashed. \[index, bond\] - BountyRejected(BountyIndex, Balance), - /// A bounty proposal is funded and became active. \[index\] - BountyBecameActive(BountyIndex), - /// A bounty is awarded to a beneficiary. \[index, beneficiary\] - BountyAwarded(BountyIndex, AccountId), - /// A bounty is claimed by beneficiary. \[index, payout, beneficiary\] - BountyClaimed(BountyIndex, Balance, AccountId), - /// A bounty is cancelled. \[index\] - BountyCanceled(BountyIndex), - /// A bounty expiry is extended. \[index\] - BountyExtended(BountyIndex), + #[pallet::config] + pub trait Config: frame_system::Config + pallet_treasury::Config { + /// The amount held on deposit for placing a bounty proposal. + #[pallet::constant] + type BountyDepositBase: Get>; + + /// The delay period for which a bounty beneficiary need to wait before claim the payout. + #[pallet::constant] + type BountyDepositPayoutDelay: Get; + + /// Bounty duration in blocks. + #[pallet::constant] + type BountyUpdatePeriod: Get; + + /// Percentage of the curator fee that will be reserved upfront as deposit for bounty curator. + #[pallet::constant] + type BountyCuratorDeposit: Get; + + /// Minimum value for a bounty. + #[pallet::constant] + type BountyValueMinimum: Get>; + + /// The amount held on deposit per byte within the tip report reason or bounty description. + #[pallet::constant] + type DataDepositPerByte: Get>; + + /// The overarching event type. + type Event: From> + IsType<::Event>; + + /// Maximum acceptable reason length. + #[pallet::constant] + type MaximumReasonLength: Get; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; } -); -decl_error! { - /// Error for the treasury module. - pub enum Error for Module { + #[pallet::error] + pub enum Error { /// Proposer's balance is too low. InsufficientProposersBalance, /// No proposal or bounty at that index. @@ -254,38 +223,64 @@ decl_error! { /// The bounties 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 amount held on deposit per byte within bounty description. - const DataDepositPerByte: BalanceOf = T::DataDepositPerByte::get(); - - /// The amount held on deposit for placing a bounty proposal. - const BountyDepositBase: BalanceOf = T::BountyDepositBase::get(); - - /// The delay period for which a bounty beneficiary need to wait before claim the payout. - const BountyDepositPayoutDelay: T::BlockNumber = T::BountyDepositPayoutDelay::get(); - - /// Bounty duration in blocks. - const BountyUpdatePeriod: T::BlockNumber = T::BountyUpdatePeriod::get(); - - /// Percentage of the curator fee that will be reserved upfront as deposit for bounty curator. - const BountyCuratorDeposit: Permill = T::BountyCuratorDeposit::get(); - - /// Minimum value for a bounty. - const BountyValueMinimum: BalanceOf = T::BountyValueMinimum::get(); - /// Maximum acceptable reason length. - const MaximumReasonLength: u32 = T::MaximumReasonLength::get(); - - type Error = Error; - - fn deposit_event() = default; + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AccountId = "AccountId", BalanceOf = "Balance")] + pub enum Event { + /// New bounty proposal. \[index\] + BountyProposed(BountyIndex), + /// A bounty proposal was rejected; funds were slashed. \[index, bond\] + BountyRejected(BountyIndex, BalanceOf), + /// A bounty proposal is funded and became active. \[index\] + BountyBecameActive(BountyIndex), + /// A bounty is awarded to a beneficiary. \[index, beneficiary\] + BountyAwarded(BountyIndex, T::AccountId), + /// A bounty is claimed by beneficiary. \[index, payout, beneficiary\] + BountyClaimed(BountyIndex, BalanceOf, T::AccountId), + /// A bounty is cancelled. \[index\] + BountyCanceled(BountyIndex), + /// A bounty expiry is extended. \[index\] + BountyExtended(BountyIndex), + } + // Note :: For backward compatibility reasons, + // pallet-bounties uses Treasury for storage. + // This is temporary solution, soon will get replaced with + // Own storage identifier. + + /// Number of bounty proposals that have been made. + #[pallet::storage] + #[pallet::getter(fn bounty_count)] + pub type BountyCount = StorageValue<_, BountyIndex, ValueQuery>; + + /// Bounties that have been made. + #[pallet::storage] + #[pallet::getter(fn bounties)] + pub type Bounties = StorageMap< + _, + Twox64Concat, + BountyIndex, + Bounty, T::BlockNumber> + >; + + /// The description of each bounty. + #[pallet::storage] + #[pallet::getter(fn bounty_descriptions)] + pub type BountyDescriptions = StorageMap< + _, + Twox64Concat, + BountyIndex, + Vec + >; + + /// Bounty indices that have been approved but not yet funded. + #[pallet::storage] + #[pallet::getter(fn bounty_approvals)] + pub type BountyApprovals = StorageValue<_, Vec, ValueQuery>; + + #[pallet::call] + impl Pallet { /// Propose a new bounty. /// /// The dispatch origin for this call must be _Signed_. @@ -298,14 +293,15 @@ decl_module! { /// - `fee`: The curator fee. /// - `value`: The total payment amount of this bounty, curator fee included. /// - `description`: The description of this bounty. - #[weight = ::WeightInfo::propose_bounty(description.len() as u32)] - fn propose_bounty( - origin, - #[compact] value: BalanceOf, + #[pallet::weight(::WeightInfo::propose_bounty(description.len() as u32))] + pub fn propose_bounty( + origin: OriginFor, + #[pallet::compact] value: BalanceOf, description: Vec, - ) { + ) -> DispatchResult { let proposer = ensure_signed(origin)?; Self::create_bounty(proposer, description, value)?; + Ok(()) } /// Approve a bounty proposal. At a later time, the bounty will be funded and become active @@ -316,8 +312,8 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::approve_bounty()] - fn approve_bounty(origin, #[compact] bounty_id: BountyIndex) { + #[pallet::weight(::WeightInfo::approve_bounty())] + pub fn approve_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { T::ApproveOrigin::ensure_origin(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { @@ -326,10 +322,11 @@ decl_module! { bounty.status = BountyStatus::Approved; - BountyApprovals::append(bounty_id); + BountyApprovals::::append(bounty_id); Ok(()) })?; + Ok(()) } /// Assign a curator to a funded bounty. @@ -339,13 +336,13 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::propose_curator()] - fn propose_curator( - origin, - #[compact] bounty_id: BountyIndex, + #[pallet::weight(::WeightInfo::propose_curator())] + pub fn propose_curator( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, curator: ::Source, - #[compact] fee: BalanceOf, - ) { + #[pallet::compact] fee: BalanceOf, + ) -> DispatchResult { T::ApproveOrigin::ensure_origin(origin)?; let curator = T::Lookup::lookup(curator)?; @@ -364,6 +361,7 @@ decl_module! { Ok(()) })?; + Ok(()) } /// Unassign curator from a bounty. @@ -384,11 +382,11 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::unassign_curator()] - fn unassign_curator( - origin, - #[compact] bounty_id: BountyIndex, - ) { + #[pallet::weight(::WeightInfo::unassign_curator())] + pub fn unassign_curator( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + ) -> DispatchResult { let maybe_sender = ensure_signed(origin.clone()) .map(Some) .or_else(|_| T::RejectOrigin::ensure_origin(origin).map(|_| None))?; @@ -424,7 +422,7 @@ decl_module! { // If the sender is not the curator, and the curator is inactive, // slash the curator. if sender != *curator { - let block_number = system::Pallet::::block_number(); + let block_number = frame_system::Pallet::::block_number(); if *update_due < block_number { slash_curator(curator, &mut bounty.curator_deposit); // Continue to change bounty status below... @@ -455,6 +453,7 @@ decl_module! { bounty.status = BountyStatus::Funded; Ok(()) })?; + Ok(()) } /// Accept the curator role for a bounty. @@ -465,8 +464,8 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::accept_curator()] - fn accept_curator(origin, #[compact] bounty_id: BountyIndex) { + #[pallet::weight(::WeightInfo::accept_curator())] + pub fn accept_curator(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { let signer = ensure_signed(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { @@ -480,7 +479,7 @@ decl_module! { T::Currency::reserve(curator, deposit)?; bounty.curator_deposit = deposit; - let update_due = system::Pallet::::block_number() + T::BountyUpdatePeriod::get(); + let update_due = frame_system::Pallet::::block_number() + T::BountyUpdatePeriod::get(); bounty.status = BountyStatus::Active { curator: curator.clone(), update_due }; Ok(()) @@ -488,6 +487,7 @@ decl_module! { _ => Err(Error::::UnexpectedStatus.into()), } })?; + Ok(()) } /// Award bounty to a beneficiary account. The beneficiary will be able to claim the funds after a delay. @@ -500,8 +500,12 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::award_bounty()] - fn award_bounty(origin, #[compact] bounty_id: BountyIndex, beneficiary: ::Source) { + #[pallet::weight(::WeightInfo::award_bounty())] + pub fn award_bounty( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + beneficiary: ::Source + ) -> DispatchResult { let signer = ensure_signed(origin)?; let beneficiary = T::Lookup::lookup(beneficiary)?; @@ -519,13 +523,14 @@ decl_module! { bounty.status = BountyStatus::PendingPayout { curator: signer, beneficiary: beneficiary.clone(), - unlock_at: system::Pallet::::block_number() + T::BountyDepositPayoutDelay::get(), + unlock_at: frame_system::Pallet::::block_number() + T::BountyDepositPayoutDelay::get(), }; Ok(()) })?; Self::deposit_event(Event::::BountyAwarded(bounty_id, beneficiary)); + Ok(()) } /// Claim the payout from an awarded bounty after payout delay. @@ -537,14 +542,14 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::claim_bounty()] - fn claim_bounty(origin, #[compact] bounty_id: BountyIndex) { + #[pallet::weight(::WeightInfo::claim_bounty())] + pub fn claim_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { let _ = ensure_signed(origin)?; // anyone can trigger claim Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { let bounty = maybe_bounty.take().ok_or(Error::::InvalidIndex)?; if let BountyStatus::PendingPayout { curator, beneficiary, unlock_at } = bounty.status { - ensure!(system::Pallet::::block_number() >= unlock_at, Error::::Premature); + ensure!(frame_system::Pallet::::block_number() >= unlock_at, Error::::Premature); let bounty_account = Self::bounty_account_id(bounty_id); let balance = T::Currency::free_balance(&bounty_account); let fee = bounty.fee.min(balance); // just to be safe @@ -558,7 +563,7 @@ decl_module! { *maybe_bounty = None; - BountyDescriptions::remove(bounty_id); + BountyDescriptions::::remove(bounty_id); Self::deposit_event(Event::::BountyClaimed(bounty_id, payout, beneficiary)); Ok(()) @@ -566,6 +571,7 @@ decl_module! { Err(Error::::UnexpectedStatus.into()) } })?; + Ok(()) } /// Cancel a proposed or active bounty. All the funds will be sent to treasury and @@ -578,8 +584,8 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::close_bounty_proposed().max(::WeightInfo::close_bounty_active())] - fn close_bounty(origin, #[compact] bounty_id: BountyIndex) -> DispatchResultWithPostInfo { + #[pallet::weight(::WeightInfo::close_bounty_proposed().max(::WeightInfo::close_bounty_active()))] + pub fn close_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResultWithPostInfo { T::RejectOrigin::ensure_origin(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResultWithPostInfo { @@ -588,7 +594,7 @@ decl_module! { match &bounty.status { BountyStatus::Proposed => { // The reject origin would like to cancel a proposed bounty. - BountyDescriptions::remove(bounty_id); + BountyDescriptions::::remove(bounty_id); let value = bounty.bond; let imbalance = T::Currency::slash_reserved(&bounty.proposer, value).0; T::OnSlash::on_unbalanced(imbalance); @@ -624,7 +630,7 @@ decl_module! { let bounty_account = Self::bounty_account_id(bounty_id); - BountyDescriptions::remove(bounty_id); + BountyDescriptions::::remove(bounty_id); let balance = T::Currency::free_balance(&bounty_account); let res = T::Currency::transfer(&bounty_account, &Self::account_id(), balance, AllowDeath); // should not fail @@ -646,8 +652,12 @@ decl_module! { /// # /// - O(1). /// # - #[weight = ::WeightInfo::extend_bounty_expiry()] - fn extend_bounty_expiry(origin, #[compact] bounty_id: BountyIndex, _remark: Vec) { + #[pallet::weight(::WeightInfo::extend_bounty_expiry())] + pub fn extend_bounty_expiry( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + _remark: Vec + ) -> DispatchResult { let signer = ensure_signed(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { @@ -656,7 +666,7 @@ decl_module! { match bounty.status { BountyStatus::Active { ref curator, ref mut update_due } => { ensure!(*curator == signer, Error::::RequireCurator); - *update_due = (system::Pallet::::block_number() + T::BountyUpdatePeriod::get()).max(*update_due); + *update_due = (frame_system::Pallet::::block_number() + T::BountyUpdatePeriod::get()).max(*update_due); }, _ => return Err(Error::::UnexpectedStatus.into()), } @@ -665,11 +675,12 @@ decl_module! { })?; Self::deposit_event(Event::::BountyExtended(bounty_id)); + Ok(()) } } } -impl Module { +impl Pallet { // Add public immutables and private mutables. /// The account ID of the treasury pot. @@ -706,7 +717,7 @@ impl Module { T::Currency::reserve(&proposer, bond) .map_err(|_| Error::::InsufficientProposersBalance)?; - BountyCount::put(index + 1); + BountyCount::::put(index + 1); let bounty = Bounty { proposer, @@ -718,22 +729,22 @@ impl Module { }; Bounties::::insert(index, &bounty); - BountyDescriptions::insert(index, description); + BountyDescriptions::::insert(index, description); - Self::deposit_event(RawEvent::BountyProposed(index)); + Self::deposit_event(Event::::BountyProposed(index)); Ok(()) } } -impl pallet_treasury::SpendFunds for Module { +impl pallet_treasury::SpendFunds for Pallet { fn spend_funds( budget_remaining: &mut BalanceOf, imbalance: &mut PositiveImbalanceOf, total_weight: &mut Weight, missed_any: &mut bool, ) { - let bounties_len = BountyApprovals::mutate(|v| { + let bounties_len = BountyApprovals::::mutate(|v| { let bounties_approval_len = v.len() as u32; v.retain(|&index| { Bounties::::mutate(index, |bounty| { @@ -754,7 +765,7 @@ impl pallet_treasury::SpendFunds for Module { bounty.value, )); - Self::deposit_event(RawEvent::BountyBecameActive(index)); + Self::deposit_event(Event::::BountyBecameActive(index)); false } else { *missed_any = true; diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index ff058a3601e07..5e6d1d19cab2f 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -396,7 +396,7 @@ fn propose_bounty_works() { assert_ok!(Bounties::propose_bounty(Origin::signed(0), 10, b"1234567890".to_vec())); - assert_eq!(last_event(), RawEvent::BountyProposed(0)); + assert_eq!(last_event(), Event::::BountyProposed(0)); let deposit: u64 = 85 + 5; assert_eq!(Balances::reserved_balance(0), deposit); @@ -458,7 +458,7 @@ fn close_bounty_works() { let deposit: u64 = 80 + 5; - assert_eq!(last_event(), RawEvent::BountyRejected(0, deposit)); + assert_eq!(last_event(), Event::::BountyRejected(0, deposit)); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 100 - deposit); @@ -690,7 +690,7 @@ fn award_and_claim_bounty_works() { assert_ok!(Bounties::claim_bounty(Origin::signed(1), 0)); - assert_eq!(last_event(), RawEvent::BountyClaimed(0, 56, 3)); + assert_eq!(last_event(), Event::::BountyClaimed(0, 56, 3)); assert_eq!(Balances::free_balance(4), 14); // initial 10 + fee 4 @@ -729,7 +729,7 @@ fn claim_handles_high_fee() { assert_ok!(Bounties::claim_bounty(Origin::signed(1), 0)); - assert_eq!(last_event(), RawEvent::BountyClaimed(0, 0, 3)); + assert_eq!(last_event(), Event::::BountyClaimed(0, 0, 3)); assert_eq!(Balances::free_balance(4), 70); // 30 + 50 - 10 assert_eq!(Balances::free_balance(3), 0); @@ -806,7 +806,7 @@ fn award_and_cancel() { assert_ok!(Bounties::unassign_curator(Origin::root(), 0)); assert_ok!(Bounties::close_bounty(Origin::root(), 0)); - assert_eq!(last_event(), RawEvent::BountyCanceled(0)); + assert_eq!(last_event(), Event::::BountyCanceled(0)); assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0); From c1f5a7f525f1678719d30b2b174e1a5ec734e331 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 17:50:35 -0400 Subject: [PATCH 02/21] events in tests --- frame/bounties/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 5e6d1d19cab2f..1039ab6b9e82c 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -160,11 +160,11 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } -fn last_event() -> RawEvent { +fn last_event() -> pallet::Event { System::events() .into_iter() .map(|r| r.event) - .filter_map(|e| if let Event::Bounties(inner) = e { Some(inner) } else { None }) + .filter_map(|e| if let Event::::Bounties(inner) = e { Some(inner) } else { None }) .last() .unwrap() } From b87bac1b186b88766441012dd00563bbd721f886 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 18:18:24 -0400 Subject: [PATCH 03/21] test import event --- frame/bounties/src/tests.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 1039ab6b9e82c..91c30d8bc4cbc 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -35,6 +35,8 @@ use sp_runtime::{ Perbill, }; +use super::Event as BountiesEvent; + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -160,11 +162,11 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } -fn last_event() -> pallet::Event { +fn last_event() -> BountiesEvent { System::events() .into_iter() .map(|r| r.event) - .filter_map(|e| if let Event::::Bounties(inner) = e { Some(inner) } else { None }) + .filter_map(|e| if let Event::Bounties(inner) = e { Some(inner) } else { None }) .last() .unwrap() } @@ -396,7 +398,7 @@ fn propose_bounty_works() { assert_ok!(Bounties::propose_bounty(Origin::signed(0), 10, b"1234567890".to_vec())); - assert_eq!(last_event(), Event::::BountyProposed(0)); + assert_eq!(last_event(), BountiesEvent::BountyProposed(0)); let deposit: u64 = 85 + 5; assert_eq!(Balances::reserved_balance(0), deposit); @@ -458,7 +460,7 @@ fn close_bounty_works() { let deposit: u64 = 80 + 5; - assert_eq!(last_event(), Event::::BountyRejected(0, deposit)); + assert_eq!(last_event(), BountiesEvent::BountyRejected(0, deposit)); assert_eq!(Balances::reserved_balance(0), 0); assert_eq!(Balances::free_balance(0), 100 - deposit); @@ -690,7 +692,7 @@ fn award_and_claim_bounty_works() { assert_ok!(Bounties::claim_bounty(Origin::signed(1), 0)); - assert_eq!(last_event(), Event::::BountyClaimed(0, 56, 3)); + assert_eq!(last_event(), BountiesEvent::BountyClaimed(0, 56, 3)); assert_eq!(Balances::free_balance(4), 14); // initial 10 + fee 4 @@ -729,7 +731,7 @@ fn claim_handles_high_fee() { assert_ok!(Bounties::claim_bounty(Origin::signed(1), 0)); - assert_eq!(last_event(), Event::::BountyClaimed(0, 0, 3)); + assert_eq!(last_event(), BountiesEvent::BountyClaimed(0, 0, 3)); assert_eq!(Balances::free_balance(4), 70); // 30 + 50 - 10 assert_eq!(Balances::free_balance(3), 0); @@ -806,7 +808,7 @@ fn award_and_cancel() { assert_ok!(Bounties::unassign_curator(Origin::root(), 0)); assert_ok!(Bounties::close_bounty(Origin::root(), 0)); - assert_eq!(last_event(), Event::::BountyCanceled(0)); + assert_eq!(last_event(), BountiesEvent::BountyCanceled(0)); assert_eq!(Balances::free_balance(Bounties::bounty_account_id(0)), 0); From bd2571438bc439e1fe4a6228124541283f452fe8 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 18:25:57 -0400 Subject: [PATCH 04/21] Update frame/bounties/src/lib.rs Co-authored-by: Keith Yeung --- frame/bounties/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index e0b5e82d32869..d11b88b6ca984 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -157,7 +157,7 @@ pub enum BountyStatus { } #[frame_support::pallet] -pub mod pallet{ +pub mod pallet { use super::*; #[pallet::pallet] From 91073a3c4d8044e7ebcec1f6b5755311a8439984 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 18:28:14 -0400 Subject: [PATCH 05/21] cargo fmt --- frame/bounties/src/lib.rs | 217 +++++++++++++++++++++----------------- 1 file changed, 121 insertions(+), 96 deletions(-) diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index e0b5e82d32869..dee05a5ad2257 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -91,8 +91,8 @@ use sp_runtime::{ use frame_support::{dispatch::DispatchResultWithPostInfo, traits::EnsureOrigin}; -use frame_system::pallet_prelude::*; use frame_support::pallet_prelude::*; +use frame_system::pallet_prelude::*; pub use weights::WeightInfo; pub use pallet::*; @@ -101,8 +101,6 @@ type BalanceOf = pallet_treasury::BalanceOf; type PositiveImbalanceOf = pallet_treasury::PositiveImbalanceOf; - - /// An index of a bounty. Just a `u32`. pub type BountyIndex = u32; @@ -157,7 +155,7 @@ pub enum BountyStatus { } #[frame_support::pallet] -pub mod pallet{ +pub mod pallet { use super::*; #[pallet::pallet] @@ -169,34 +167,35 @@ pub mod pallet{ /// The amount held on deposit for placing a bounty proposal. #[pallet::constant] type BountyDepositBase: Get>; - + /// The delay period for which a bounty beneficiary need to wait before claim the payout. #[pallet::constant] type BountyDepositPayoutDelay: Get; - + /// Bounty duration in blocks. #[pallet::constant] type BountyUpdatePeriod: Get; - - /// Percentage of the curator fee that will be reserved upfront as deposit for bounty curator. + + /// Percentage of the curator fee that will be reserved upfront as deposit for bounty + /// curator. #[pallet::constant] type BountyCuratorDeposit: Get; - + /// Minimum value for a bounty. #[pallet::constant] type BountyValueMinimum: Get>; - + /// The amount held on deposit per byte within the tip report reason or bounty description. #[pallet::constant] type DataDepositPerByte: Get>; - + /// The overarching event type. type Event: From> + IsType<::Event>; - + /// Maximum acceptable reason length. #[pallet::constant] type MaximumReasonLength: Get; - + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; } @@ -261,18 +260,13 @@ pub mod pallet{ _, Twox64Concat, BountyIndex, - Bounty, T::BlockNumber> + Bounty, T::BlockNumber>, >; /// The description of each bounty. #[pallet::storage] #[pallet::getter(fn bounty_descriptions)] - pub type BountyDescriptions = StorageMap< - _, - Twox64Concat, - BountyIndex, - Vec - >; + pub type BountyDescriptions = StorageMap<_, Twox64Concat, BountyIndex, Vec>; /// Bounty indices that have been approved but not yet funded. #[pallet::storage] @@ -313,7 +307,10 @@ pub mod pallet{ /// - O(1). /// # #[pallet::weight(::WeightInfo::approve_bounty())] - pub fn approve_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { + pub fn approve_bounty( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + ) -> DispatchResult { T::ApproveOrigin::ensure_origin(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { @@ -347,7 +344,6 @@ pub mod pallet{ let curator = T::Lookup::lookup(curator)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { - let mut bounty = maybe_bounty.as_mut().ok_or(Error::::InvalidIndex)?; match bounty.status { BountyStatus::Proposed | BountyStatus::Approved | BountyStatus::Funded => {}, @@ -368,8 +364,8 @@ pub mod pallet{ /// /// This function can only be called by the `RejectOrigin` a signed origin. /// - /// If this function is called by the `RejectOrigin`, we assume that the curator is malicious - /// or inactive. As a result, we will slash the curator when possible. + /// If this function is called by the `RejectOrigin`, we assume that the curator is + /// malicious or inactive. As a result, we will slash the curator when possible. /// /// If the origin is the curator, we take this as a sign they are unable to do their job and /// they willingly give up. We could slash them, but for now we allow them to recover their @@ -404,7 +400,7 @@ pub mod pallet{ BountyStatus::Proposed | BountyStatus::Approved | BountyStatus::Funded => { // No curator to unassign at this point. return Err(Error::::UnexpectedStatus.into()) - } + }, BountyStatus::CuratorProposed { ref curator } => { // A curator has been proposed, but not accepted yet. // Either `RejectOrigin` or the proposed curator can unassign the curator. @@ -425,7 +421,7 @@ pub mod pallet{ let block_number = frame_system::Pallet::::block_number(); if *update_due < block_number { slash_curator(curator, &mut bounty.curator_deposit); - // Continue to change bounty status below... + // Continue to change bounty status below... } else { // Curator has more time to give an update. return Err(Error::::Premature.into()) @@ -433,7 +429,8 @@ pub mod pallet{ } else { // Else this is the curator, willingly giving up their role. // Give back their deposit. - let err_amount = T::Currency::unreserve(&curator, bounty.curator_deposit); + let err_amount = + T::Currency::unreserve(&curator, bounty.curator_deposit); debug_assert!(err_amount.is_zero()); // Continue to change bounty status below... } @@ -447,7 +444,7 @@ pub mod pallet{ ensure!(maybe_sender.is_none(), BadOrigin); slash_curator(curator, &mut bounty.curator_deposit); // Continue to change bounty status below... - } + }, }; bounty.status = BountyStatus::Funded; @@ -465,7 +462,10 @@ pub mod pallet{ /// - O(1). /// # #[pallet::weight(::WeightInfo::accept_curator())] - pub fn accept_curator(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { + pub fn accept_curator( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + ) -> DispatchResult { let signer = ensure_signed(origin)?; Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { @@ -479,8 +479,10 @@ pub mod pallet{ T::Currency::reserve(curator, deposit)?; bounty.curator_deposit = deposit; - let update_due = frame_system::Pallet::::block_number() + T::BountyUpdatePeriod::get(); - bounty.status = BountyStatus::Active { curator: curator.clone(), update_due }; + let update_due = frame_system::Pallet::::block_number() + + T::BountyUpdatePeriod::get(); + bounty.status = + BountyStatus::Active { curator: curator.clone(), update_due }; Ok(()) }, @@ -490,7 +492,8 @@ pub mod pallet{ Ok(()) } - /// Award bounty to a beneficiary account. The beneficiary will be able to claim the funds after a delay. + /// Award bounty to a beneficiary account. The beneficiary will be able to claim the funds + /// after a delay. /// /// The dispatch origin for this call must be the curator of this bounty. /// @@ -502,9 +505,9 @@ pub mod pallet{ /// # #[pallet::weight(::WeightInfo::award_bounty())] pub fn award_bounty( - origin: OriginFor, - #[pallet::compact] bounty_id: BountyIndex, - beneficiary: ::Source + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + beneficiary: ::Source, ) -> DispatchResult { let signer = ensure_signed(origin)?; let beneficiary = T::Lookup::lookup(beneficiary)?; @@ -512,10 +515,7 @@ pub mod pallet{ Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { let mut bounty = maybe_bounty.as_mut().ok_or(Error::::InvalidIndex)?; match &bounty.status { - BountyStatus::Active { - curator, - .. - } => { + BountyStatus::Active { curator, .. } => { ensure!(signer == *curator, Error::::RequireCurator); }, _ => return Err(Error::::UnexpectedStatus.into()), @@ -523,7 +523,8 @@ pub mod pallet{ bounty.status = BountyStatus::PendingPayout { curator: signer, beneficiary: beneficiary.clone(), - unlock_at: frame_system::Pallet::::block_number() + T::BountyDepositPayoutDelay::get(), + unlock_at: frame_system::Pallet::::block_number() + + T::BountyDepositPayoutDelay::get(), }; Ok(()) @@ -543,13 +544,21 @@ pub mod pallet{ /// - O(1). /// # #[pallet::weight(::WeightInfo::claim_bounty())] - pub fn claim_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResult { + pub fn claim_bounty( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + ) -> DispatchResult { let _ = ensure_signed(origin)?; // anyone can trigger claim Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult { let bounty = maybe_bounty.take().ok_or(Error::::InvalidIndex)?; - if let BountyStatus::PendingPayout { curator, beneficiary, unlock_at } = bounty.status { - ensure!(frame_system::Pallet::::block_number() >= unlock_at, Error::::Premature); + if let BountyStatus::PendingPayout { curator, beneficiary, unlock_at } = + bounty.status + { + ensure!( + frame_system::Pallet::::block_number() >= unlock_at, + Error::::Premature + ); let bounty_account = Self::bounty_account_id(bounty_id); let balance = T::Currency::free_balance(&bounty_account); let fee = bounty.fee.min(balance); // just to be safe @@ -558,7 +567,8 @@ pub mod pallet{ debug_assert!(err_amount.is_zero()); let res = T::Currency::transfer(&bounty_account, &curator, fee, AllowDeath); // should not fail debug_assert!(res.is_ok()); - let res = T::Currency::transfer(&bounty_account, &beneficiary, payout, AllowDeath); // should not fail + let res = + T::Currency::transfer(&bounty_account, &beneficiary, payout, AllowDeath); // should not fail debug_assert!(res.is_ok()); *maybe_bounty = None; @@ -585,61 +595,74 @@ pub mod pallet{ /// - O(1). /// # #[pallet::weight(::WeightInfo::close_bounty_proposed().max(::WeightInfo::close_bounty_active()))] - pub fn close_bounty(origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex) -> DispatchResultWithPostInfo { + pub fn close_bounty( + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + ) -> DispatchResultWithPostInfo { T::RejectOrigin::ensure_origin(origin)?; - Bounties::::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResultWithPostInfo { - let bounty = maybe_bounty.as_ref().ok_or(Error::::InvalidIndex)?; - - match &bounty.status { - BountyStatus::Proposed => { - // The reject origin would like to cancel a proposed bounty. - BountyDescriptions::::remove(bounty_id); - let value = bounty.bond; - let imbalance = T::Currency::slash_reserved(&bounty.proposer, value).0; - T::OnSlash::on_unbalanced(imbalance); - *maybe_bounty = None; - - Self::deposit_event(Event::::BountyRejected(bounty_id, value)); - // Return early, nothing else to do. - return Ok(Some(::WeightInfo::close_bounty_proposed()).into()) - }, - BountyStatus::Approved => { - // For weight reasons, we don't allow a council to cancel in this phase. - // We ask for them to wait until it is funded before they can cancel. - return Err(Error::::UnexpectedStatus.into()) - }, - BountyStatus::Funded | - BountyStatus::CuratorProposed { .. } => { - // Nothing extra to do besides the removal of the bounty below. - }, - BountyStatus::Active { curator, .. } => { - // Cancelled by council, refund deposit of the working curator. - let err_amount = T::Currency::unreserve(&curator, bounty.curator_deposit); - debug_assert!(err_amount.is_zero()); - // Then execute removal of the bounty below. - }, - BountyStatus::PendingPayout { .. } => { - // Bounty is already pending payout. If council wants to cancel - // this bounty, it should mean the curator was acting maliciously. - // So the council should first unassign the curator, slashing their - // deposit. - return Err(Error::::PendingPayout.into()) + Bounties::::try_mutate_exists( + bounty_id, + |maybe_bounty| -> DispatchResultWithPostInfo { + let bounty = maybe_bounty.as_ref().ok_or(Error::::InvalidIndex)?; + + match &bounty.status { + BountyStatus::Proposed => { + // The reject origin would like to cancel a proposed bounty. + BountyDescriptions::::remove(bounty_id); + let value = bounty.bond; + let imbalance = T::Currency::slash_reserved(&bounty.proposer, value).0; + T::OnSlash::on_unbalanced(imbalance); + *maybe_bounty = None; + + Self::deposit_event(Event::::BountyRejected(bounty_id, value)); + // Return early, nothing else to do. + return Ok( + Some(::WeightInfo::close_bounty_proposed()).into() + ) + }, + BountyStatus::Approved => { + // For weight reasons, we don't allow a council to cancel in this phase. + // We ask for them to wait until it is funded before they can cancel. + return Err(Error::::UnexpectedStatus.into()) + }, + BountyStatus::Funded | BountyStatus::CuratorProposed { .. } => { + // Nothing extra to do besides the removal of the bounty below. + }, + BountyStatus::Active { curator, .. } => { + // Cancelled by council, refund deposit of the working curator. + let err_amount = + T::Currency::unreserve(&curator, bounty.curator_deposit); + debug_assert!(err_amount.is_zero()); + // Then execute removal of the bounty below. + }, + BountyStatus::PendingPayout { .. } => { + // Bounty is already pending payout. If council wants to cancel + // this bounty, it should mean the curator was acting maliciously. + // So the council should first unassign the curator, slashing their + // deposit. + return Err(Error::::PendingPayout.into()) + }, } - } - let bounty_account = Self::bounty_account_id(bounty_id); + let bounty_account = Self::bounty_account_id(bounty_id); - BountyDescriptions::::remove(bounty_id); + BountyDescriptions::::remove(bounty_id); - let balance = T::Currency::free_balance(&bounty_account); - let res = T::Currency::transfer(&bounty_account, &Self::account_id(), balance, AllowDeath); // should not fail - debug_assert!(res.is_ok()); - *maybe_bounty = None; + let balance = T::Currency::free_balance(&bounty_account); + let res = T::Currency::transfer( + &bounty_account, + &Self::account_id(), + balance, + AllowDeath, + ); // should not fail + debug_assert!(res.is_ok()); + *maybe_bounty = None; - Self::deposit_event(Event::::BountyCanceled(bounty_id)); - Ok(Some(::WeightInfo::close_bounty_active()).into()) - }) + Self::deposit_event(Event::::BountyCanceled(bounty_id)); + Ok(Some(::WeightInfo::close_bounty_active()).into()) + }, + ) } /// Extend the expiry time of an active bounty. @@ -654,9 +677,9 @@ pub mod pallet{ /// # #[pallet::weight(::WeightInfo::extend_bounty_expiry())] pub fn extend_bounty_expiry( - origin: OriginFor, - #[pallet::compact] bounty_id: BountyIndex, - _remark: Vec + origin: OriginFor, + #[pallet::compact] bounty_id: BountyIndex, + _remark: Vec, ) -> DispatchResult { let signer = ensure_signed(origin)?; @@ -666,7 +689,9 @@ pub mod pallet{ match bounty.status { BountyStatus::Active { ref curator, ref mut update_due } => { ensure!(*curator == signer, Error::::RequireCurator); - *update_due = (frame_system::Pallet::::block_number() + T::BountyUpdatePeriod::get()).max(*update_due); + *update_due = (frame_system::Pallet::::block_number() + + T::BountyUpdatePeriod::get()) + .max(*update_due); }, _ => return Err(Error::::UnexpectedStatus.into()), } From cb168bd14bbae5d65b5d0ea00c1a83d2c946604f Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 18:34:24 -0400 Subject: [PATCH 06/21] line width --- frame/bounties/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index dee05a5ad2257..a79ce5fecbf7e 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -594,7 +594,8 @@ pub mod pallet { /// # /// - O(1). /// # - #[pallet::weight(::WeightInfo::close_bounty_proposed().max(::WeightInfo::close_bounty_active()))] + #[pallet::weight(::WeightInfo::close_bounty_proposed() + .max(::WeightInfo::close_bounty_active()))] pub fn close_bounty( origin: OriginFor, #[pallet::compact] bounty_id: BountyIndex, From 8c989117debf6b2c82d24965268e74797c506380 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 14 Aug 2021 18:52:00 -0400 Subject: [PATCH 07/21] benchmarks compile --- frame/bounties/src/benchmarking.rs | 33 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs index 832c053f024db..ba94de2e3e24d 100644 --- a/frame/bounties/src/benchmarking.rs +++ b/frame/bounties/src/benchmarking.rs @@ -22,11 +22,10 @@ use super::*; use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller}; -use frame_support::traits::OnInitialize; use frame_system::RawOrigin; use sp_runtime::traits::Bounded; -use crate::Module as Bounties; +use crate::Pallet as Bounties; use pallet_treasury::Pallet as Treasury; const SEED: u32 = 0; @@ -36,10 +35,10 @@ fn create_approved_bounties(n: u32) -> Result<(), &'static str> { for i in 0..n { let (caller, _curator, _fee, value, reason) = setup_bounty::(i, MAX_BYTES); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; Bounties::::approve_bounty(RawOrigin::Root.into(), bounty_id)?; } - ensure!(BountyApprovals::get().len() == n as usize, "Not all bounty approved"); + ensure!(BountyApprovals::::get().len() == n as usize, "Not all bounty approved"); Ok(()) } @@ -64,7 +63,7 @@ fn create_bounty( let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; Bounties::::approve_bounty(RawOrigin::Root.into(), bounty_id)?; Treasury::::on_initialize(T::BlockNumber::zero()); Bounties::::propose_curator(RawOrigin::Root.into(), bounty_id, curator_lookup.clone(), fee)?; @@ -94,7 +93,7 @@ benchmarks! { approve_bounty { let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; }: _(RawOrigin::Root, bounty_id) propose_curator { @@ -102,7 +101,7 @@ benchmarks! { let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; Bounties::::approve_bounty(RawOrigin::Root.into(), bounty_id)?; Bounties::::on_initialize(T::BlockNumber::zero()); }: _(RawOrigin::Root, bounty_id, curator_lookup, fee) @@ -112,7 +111,7 @@ benchmarks! { setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; frame_system::Pallet::::set_block_number(T::BountyUpdatePeriod::get() + 1u32.into()); let caller = whitelisted_caller(); }: _(RawOrigin::Signed(caller), bounty_id) @@ -122,7 +121,7 @@ benchmarks! { let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; Bounties::::approve_bounty(RawOrigin::Root.into(), bounty_id)?; Bounties::::on_initialize(T::BlockNumber::zero()); Bounties::::propose_curator(RawOrigin::Root.into(), bounty_id, curator_lookup, fee)?; @@ -133,7 +132,7 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup)?; let beneficiary = T::Lookup::unlookup(account("beneficiary", 0, SEED)); }: _(RawOrigin::Signed(curator), bounty_id, beneficiary) @@ -143,7 +142,7 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup)?; let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED); @@ -162,17 +161,17 @@ benchmarks! { setup_pot_account::(); let (caller, curator, fee, value, reason) = setup_bounty::(0, 0); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; }: close_bounty(RawOrigin::Root, bounty_id) close_bounty_active { setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; }: close_bounty(RawOrigin::Root, bounty_id) verify { - assert_last_event::(RawEvent::BountyCanceled(bounty_id).into()) + assert_last_event::(Event::BountyCanceled(bounty_id).into()) } extend_bounty_expiry { @@ -180,11 +179,11 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup)?; }: _(RawOrigin::Signed(curator), bounty_id, Vec::new()) verify { - assert_last_event::(RawEvent::BountyExtended(bounty_id).into()) + assert_last_event::(Event::BountyExtended(bounty_id).into()) } spend_funds { @@ -207,7 +206,7 @@ benchmarks! { verify { ensure!(budget_remaining < BalanceOf::::max_value(), "Budget not used"); ensure!(missed_any == false, "Missed some"); - assert_last_event::(RawEvent::BountyBecameActive(b - 1).into()) + assert_last_event::(Event::BountyBecameActive(b - 1).into()) } } From 4436610b50dd9daaab324b0fe95569221ffd9c5b Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Fri, 20 Aug 2021 16:37:42 -0400 Subject: [PATCH 08/21] add migrations --- frame/bounties/src/lib.rs | 5 - frame/bounties/src/migrations/mod.rs | 19 ++++ frame/bounties/src/migrations/v4.rs | 134 +++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 frame/bounties/src/migrations/mod.rs create mode 100644 frame/bounties/src/migrations/v4.rs diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index a79ce5fecbf7e..a8473ef086da4 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -243,11 +243,6 @@ pub mod pallet { BountyExtended(BountyIndex), } - // Note :: For backward compatibility reasons, - // pallet-bounties uses Treasury for storage. - // This is temporary solution, soon will get replaced with - // Own storage identifier. - /// Number of bounty proposals that have been made. #[pallet::storage] #[pallet::getter(fn bounty_count)] diff --git a/frame/bounties/src/migrations/mod.rs b/frame/bounties/src/migrations/mod.rs new file mode 100644 index 0000000000000..e5553744dfcd1 --- /dev/null +++ b/frame/bounties/src/migrations/mod.rs @@ -0,0 +1,19 @@ +// 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. +pub mod v4; \ No newline at end of file diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs new file mode 100644 index 0000000000000..3e0088678bdee --- /dev/null +++ b/frame/bounties/src/migrations/v4.rs @@ -0,0 +1,134 @@ +// 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 frame_support::{ + traits::{Get, GetStorageVersion, STORAGE_VERSION_STORAGE_KEY_POSTFIX}, + weights::Weight, +}; +use sp_core::hexdisplay::HexDisplay; +use sp_io::hashing::twox_128; + +/// 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::bounties", + "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::bounties", + "Running migration to v4 for bounties with storage version {:?}", + on_chain_storage_version, + ); + + if on_chain_storage_version < 4 { + log::info!(target: "runtime::bounties", "new prefix: {}", new_pallet_name); + frame_support::storage::migration::move_pallet( + old_pallet_name.as_bytes(), + new_pallet_name.as_bytes(), + ); + ::BlockWeights::get().max_block + } else { + log::warn!( + target: "runtime::bounties", + "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_migration>( + 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(); + log::info!( + "pre-migration bounties, old prefix = {}, new prefix = {}", + old_pallet_name, + new_pallet_name, + ); + + // the next key must exist, and start with the hash of old prefix. + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let next_key = sp_io::storage::next_key(&old_pallet_prefix).unwrap(); + assert!(next_key.starts_with(&old_pallet_prefix)); + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; + let pallet_version_key = + [&new_pallet_prefix, &twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX)[..]].concat(); + let storage_version_key = + [&new_pallet_prefix, &twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX)[..]].concat(); + + // ensure nothing is stored in the new prefix. + assert!( + sp_io::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 == pallet_version_key || + next_key == storage_version_key + }, + ), + "unexpected next_key({}) = {:?}", + new_pallet_name, + HexDisplay::from(&sp_io::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_migration>(old_pallet_name: N) { + log::info!("post-migration bounties"); + + let old_pallet_name = old_pallet_name.as_ref().as_bytes(); + let old_pallet_prefix = twox_128(old_pallet_name); + // Assert that nothing remains at the old prefix + assert!(sp_io::storage::next_key(&old_pallet_prefix) + .map_or(true, |next_key| !next_key.starts_with(&old_pallet_prefix))); + assert_eq!(

::on_chain_storage_version(), 4); +} From 7d328eb4faab89ec7f754fd6a7bdabbd23c92353 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Mon, 23 Aug 2021 23:39:00 -0400 Subject: [PATCH 09/21] fmt --- frame/bounties/src/migrations/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/bounties/src/migrations/mod.rs b/frame/bounties/src/migrations/mod.rs index e5553744dfcd1..1743cf68852b6 100644 --- a/frame/bounties/src/migrations/mod.rs +++ b/frame/bounties/src/migrations/mod.rs @@ -16,4 +16,5 @@ // limitations under the License. /// Version 4. -pub mod v4; \ No newline at end of file +pub mod v4; + From dc2932d0fef0983dd027bb6316de38ccb3fefbb4 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Tue, 24 Aug 2021 00:36:37 -0400 Subject: [PATCH 10/21] comments --- frame/bounties/src/migrations/v4.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index 3e0088678bdee..f4c4afd9633e4 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -26,7 +26,7 @@ use sp_io::hashing::twox_128; /// /// This new prefix must be the same as the one set in construct_runtime. For safety, use /// `PalletInfo` to get it, as: -/// `::PalletInfo::name::`. +/// `::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 From 31c82d1dfd49625e5ae05afe2998cf3e329ffaca Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Tue, 24 Aug 2021 02:01:09 -0400 Subject: [PATCH 11/21] mod migrations --- frame/bounties/Cargo.toml | 9 ++++++--- frame/bounties/src/lib.rs | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/frame/bounties/Cargo.toml b/frame/bounties/Cargo.toml index 84147b96f9104..cff361369d5dc 100644 --- a/frame/bounties/Cargo.toml +++ b/frame/bounties/Cargo.toml @@ -21,23 +21,26 @@ sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../pr 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" } - +sp-io = { version = "4.0.0-dev", path = "../../primitives/io" } +sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true } +log = { version = "0.4.14", default-features = false } [dev-dependencies] -sp-io = { version = "4.0.0-dev", path = "../../primitives/io" } -sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } pallet-balances = { version = "4.0.0-dev", path = "../balances" } [features] default = ["std"] std = [ "codec/std", + "sp-core/std", + "sp-io/std", "sp-std/std", "sp-runtime/std", "frame-support/std", "frame-system/std", "pallet-treasury/std", + "log/std", ] runtime-benchmarks = [ "frame-benchmarking", diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index a8473ef086da4..49a31ed7f98a7 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -77,6 +77,7 @@ mod benchmarking; mod tests; pub mod weights; +pub mod migrations; use sp_std::prelude::*; From 5ca705844df684e96e9beb1dc123676646cf6cb4 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Tue, 24 Aug 2021 02:18:45 -0400 Subject: [PATCH 12/21] fix Cargo.toml --- Cargo.lock | 1 + frame/bounties/Cargo.toml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f02cafd76f0db..ebea2c92e25ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5052,6 +5052,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log 0.4.14", "pallet-balances", "pallet-treasury", "parity-scale-codec", diff --git a/frame/bounties/Cargo.toml b/frame/bounties/Cargo.toml index cff361369d5dc..a1310fc23d689 100644 --- a/frame/bounties/Cargo.toml +++ b/frame/bounties/Cargo.toml @@ -21,8 +21,8 @@ sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../pr 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" } -sp-io = { version = "4.0.0-dev", path = "../../primitives/io" } -sp-core = { version = "4.0.0-dev", path = "../../primitives/core" } +sp-io = { version = "4.0.0-dev", path = "../../primitives/io", default-features = false } +sp-core = { version = "4.0.0-dev", path = "../../primitives/core", default-features = false } frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = "../benchmarking", optional = true } log = { version = "0.4.14", default-features = false } From 95f7a85dd34592dccdee1e3e45a9439f56a422e2 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Tue, 24 Aug 2021 02:31:45 -0400 Subject: [PATCH 13/21] never remember cargo fmt --- frame/bounties/src/lib.rs | 2 +- frame/bounties/src/migrations/mod.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/bounties/src/lib.rs b/frame/bounties/src/lib.rs index 49a31ed7f98a7..dc37ff65ba755 100644 --- a/frame/bounties/src/lib.rs +++ b/frame/bounties/src/lib.rs @@ -75,9 +75,9 @@ #![cfg_attr(not(feature = "std"), no_std)] mod benchmarking; +pub mod migrations; mod tests; pub mod weights; -pub mod migrations; use sp_std::prelude::*; diff --git a/frame/bounties/src/migrations/mod.rs b/frame/bounties/src/migrations/mod.rs index 1743cf68852b6..26d07a0cd5ac8 100644 --- a/frame/bounties/src/migrations/mod.rs +++ b/frame/bounties/src/migrations/mod.rs @@ -17,4 +17,3 @@ /// Version 4. pub mod v4; - From 4b5f866e12b5383b885289ea37b11dacbab2d290 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Tue, 7 Sep 2021 18:28:39 -0400 Subject: [PATCH 14/21] fix migration --- frame/bounties/src/migrations/v4.rs | 121 ++++++++++++++++++++-------- frame/bounties/src/tests.rs | 37 +++++++++ 2 files changed, 126 insertions(+), 32 deletions(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index f4c4afd9633e4..97eb4d22c1d47 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -15,12 +15,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{ - traits::{Get, GetStorageVersion, STORAGE_VERSION_STORAGE_KEY_POSTFIX}, - weights::Weight, -}; -use sp_core::hexdisplay::HexDisplay; -use sp_io::hashing::twox_128; +use frame_support::{storage::StoragePrefixedMap, traits::{Get, GetStorageVersion, PalletInfoAccess, STORAGE_VERSION_STORAGE_KEY_POSTFIX, StorageVersion}, weights::Weight}; +use sp_std::str; +use sp_core::{hexdisplay::HexDisplay, twox_128}; +use sp_io::storage; + +use crate as pallet_bounties; /// Migrate the entire storage of this pallet to a new prefix. /// @@ -31,7 +31,7 @@ use sp_io::hashing::twox_128; /// 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>( +pub fn migrate>( old_pallet_name: N, new_pallet_name: N, ) -> Weight { @@ -54,11 +54,39 @@ pub fn migrate>( ); if on_chain_storage_version < 4 { - log::info!(target: "runtime::bounties", "new prefix: {}", new_pallet_name); - frame_support::storage::migration::move_pallet( + let storage_prefix = pallet_bounties::BountyCount::::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_bounties::Bounties::::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_bounties::BountyDescriptions::::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_bounties::BountyApprovals::::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!( @@ -74,41 +102,42 @@ pub fn migrate>( /// [`frame_support::traits::OnRuntimeUpgrade::pre_upgrade`] for further testing. /// /// Panics if anything goes wrong. -pub fn pre_migration>( +pub fn pre_migration>( 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(); - log::info!( - "pre-migration bounties, old prefix = {}, new prefix = {}", - old_pallet_name, - new_pallet_name, - ); + let storage_prefix_bounties_count = pallet_bounties::BountyCount::::storage_prefix(); + let storage_prefix_bounties = pallet_bounties::Bounties::::storage_prefix(); + let storage_prefix_bounties_description = pallet_bounties::BountyDescriptions::::storage_prefix(); + let storage_prefix_bounties_approvals = pallet_bounties::BountyApprovals::::storage_prefix(); + log_migration("pre-migration", storage_prefix_bounties_count, old_pallet_name, new_pallet_name); + log_migration("pre-migration", storage_prefix_bounties, old_pallet_name, new_pallet_name); + log_migration("pre-migration", storage_prefix_bounties_description, old_pallet_name, new_pallet_name); + log_migration("pre-migration", storage_prefix_bounties_approvals, old_pallet_name, new_pallet_name); // the next key must exist, and start with the hash of old prefix. let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); - let next_key = sp_io::storage::next_key(&old_pallet_prefix).unwrap(); - assert!(next_key.starts_with(&old_pallet_prefix)); + let old_bounties_count_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + let old_bounties_key= [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); + let old_bounties_description_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let old_bounties_approvals_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + assert!(storage::next_key(&old_bounties_count_key).map_or(true, |next_key| next_key.starts_with(&old_bounties_key))); let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); - const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; - let pallet_version_key = - [&new_pallet_prefix, &twox_128(PALLET_VERSION_STORAGE_KEY_POSTFIX)[..]].concat(); let storage_version_key = [&new_pallet_prefix, &twox_128(STORAGE_VERSION_STORAGE_KEY_POSTFIX)[..]].concat(); // ensure nothing is stored in the new prefix. assert!( - sp_io::storage::next_key(&new_pallet_prefix).map_or( + 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 == pallet_version_key || - next_key == storage_version_key + !next_key.starts_with(&new_pallet_prefix) || next_key == storage_version_key }, ), "unexpected next_key({}) = {:?}", @@ -122,13 +151,41 @@ pub fn pre_migration>(old_pallet_name: N) { - log::info!("post-migration bounties"); - - let old_pallet_name = old_pallet_name.as_ref().as_bytes(); - let old_pallet_prefix = twox_128(old_pallet_name); - // Assert that nothing remains at the old prefix - assert!(sp_io::storage::next_key(&old_pallet_prefix) - .map_or(true, |next_key| !next_key.starts_with(&old_pallet_prefix))); +pub fn post_migration>(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_bounties_count = pallet_bounties::BountyCount::::storage_prefix(); + let storage_prefix_bounties = pallet_bounties::Bounties::::storage_prefix(); + let storage_prefix_bounties_description = pallet_bounties::BountyDescriptions::::storage_prefix(); + let storage_prefix_bounties_approvals = pallet_bounties::BountyApprovals::::storage_prefix(); + log_migration("post-migration", storage_prefix_bounties_count, old_pallet_name, new_pallet_name); + log_migration("post-migration", storage_prefix_bounties, old_pallet_name, new_pallet_name); + log_migration("post-migration", storage_prefix_bounties_description, old_pallet_name, new_pallet_name); + log_migration("post-migration", storage_prefix_bounties_approvals, old_pallet_name, new_pallet_name); + + let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); + let old_bounties_count_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + let old_bounties_key= [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); + let old_bounties_description_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let old_bounties_approvals_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + + let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); + let new_bounties_count_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + let new_bounties_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); + let new_bounties_description_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let new_bounties_approvals_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + assert!(storage::next_key(&new_bounties_count_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::bounties", + "{} prefix of storage '{}': '{}' ==> '{}'", + stage, + str::from_utf8(storage_prefix).unwrap_or(""), + old_pallet_name, + new_pallet_name, + ); +} diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 91c30d8bc4cbc..0b2ba97841d67 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -936,6 +936,43 @@ fn extend_expiry() { }); } +/* +#[test] +fn test_migration_v4() { + let mut s = Storage::default(); + + //let reason1 = BlakeTwo256::hash(b""); + //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 41a917f8c40d4778a643c2aec27bdd38fd29d4d7 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Wed, 8 Sep 2021 02:37:05 -0400 Subject: [PATCH 15/21] migrations and test --- frame/bounties/src/migrations/v4.rs | 138 ++++++++++++++++++++++------ frame/bounties/src/tests.rs | 48 +++++----- 2 files changed, 135 insertions(+), 51 deletions(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index 97eb4d22c1d47..0e53446fe0a5d 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -15,14 +15,21 @@ // See the License for the specific language governing permissions and // limitations under the License. -use frame_support::{storage::StoragePrefixedMap, traits::{Get, GetStorageVersion, PalletInfoAccess, STORAGE_VERSION_STORAGE_KEY_POSTFIX, StorageVersion}, weights::Weight}; +use frame_support::{ + storage::{generator::StorageValue, StoragePrefixedMap}, + traits::{ + Get, GetStorageVersion, PalletInfoAccess, StorageVersion, + STORAGE_VERSION_STORAGE_KEY_POSTFIX, + }, + weights::Weight, +}; +use sp_core::hexdisplay::HexDisplay; +use sp_io::{hashing::twox_128, storage}; use sp_std::str; -use sp_core::{hexdisplay::HexDisplay, twox_128}; -use sp_io::storage; use crate as pallet_bounties; -/// Migrate the entire storage of this pallet to a new prefix. +/// Migrate the storage of the bounties pallet to a new prefix, leaving all other storage untouched /// /// This new prefix must be the same as the one set in construct_runtime. For safety, use /// `PalletInfo` to get it, as: @@ -31,7 +38,11 @@ use crate as pallet_bounties; /// 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>( +pub fn migrate< + T: pallet_bounties::Config, + P: GetStorageVersion + PalletInfoAccess, + N: AsRef, +>( old_pallet_name: N, new_pallet_name: N, ) -> Weight { @@ -110,20 +121,51 @@ pub fn pre_migration::storage_prefix(); let storage_prefix_bounties = pallet_bounties::Bounties::::storage_prefix(); - let storage_prefix_bounties_description = pallet_bounties::BountyDescriptions::::storage_prefix(); + let storage_prefix_bounties_description = + pallet_bounties::BountyDescriptions::::storage_prefix(); let storage_prefix_bounties_approvals = pallet_bounties::BountyApprovals::::storage_prefix(); log_migration("pre-migration", storage_prefix_bounties_count, old_pallet_name, new_pallet_name); log_migration("pre-migration", storage_prefix_bounties, old_pallet_name, new_pallet_name); - log_migration("pre-migration", storage_prefix_bounties_description, old_pallet_name, new_pallet_name); - log_migration("pre-migration", storage_prefix_bounties_approvals, old_pallet_name, new_pallet_name); + log_migration( + "pre-migration", + storage_prefix_bounties_description, + old_pallet_name, + new_pallet_name, + ); + log_migration( + "pre-migration", + storage_prefix_bounties_approvals, + old_pallet_name, + new_pallet_name, + ); - // the next key must exist, and start with the hash of old prefix. let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); - let old_bounties_count_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); - let old_bounties_key= [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); - let old_bounties_description_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); - let old_bounties_approvals_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); - assert!(storage::next_key(&old_bounties_count_key).map_or(true, |next_key| next_key.starts_with(&old_bounties_key))); + let old_bounties_count_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + + let old_bounties_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); + let old_bounties_description_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let old_bounties_approvals_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + /* + assert!(storage::next_key(&old_bounties_count_key) + .map_or(true, |next_key| next_key.starts_with(&old_bounties_key) || + next_key.starts_with(&old_bounties_description_key) || + next_key.starts_with(&old_bounties_approvals_key))); + assert!(storage::next_key(&old_bounties_key) + .map_or(true, |next_key| next_key.starts_with(&old_bounties_count_key) || + next_key.starts_with(&old_bounties_description_key) || + next_key.starts_with(&old_bounties_approvals_key))); + assert!(storage::next_key(&old_bounties_description_key) + .map_or(true, |next_key| next_key.starts_with(&old_bounties_count_key) || + next_key.starts_with(&old_bounties_key) || + next_key.starts_with(&old_bounties_approvals_key))); + assert!(storage::next_key(&old_bounties_approvals_key) + .map_or(true, |next_key| next_key.starts_with(&old_bounties_key) || + next_key.starts_with(&old_bounties_description_key) || + next_key.starts_with(&old_bounties_count_key))); + */ let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); let storage_version_key = @@ -151,30 +193,72 @@ pub fn pre_migration>(old_pallet_name: N, new_pallet_name: N) { +pub fn post_migration>( + 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_bounties_count = pallet_bounties::BountyCount::::storage_prefix(); let storage_prefix_bounties = pallet_bounties::Bounties::::storage_prefix(); - let storage_prefix_bounties_description = pallet_bounties::BountyDescriptions::::storage_prefix(); + let storage_prefix_bounties_description = + pallet_bounties::BountyDescriptions::::storage_prefix(); let storage_prefix_bounties_approvals = pallet_bounties::BountyApprovals::::storage_prefix(); - log_migration("post-migration", storage_prefix_bounties_count, old_pallet_name, new_pallet_name); + log_migration( + "post-migration", + storage_prefix_bounties_count, + old_pallet_name, + new_pallet_name, + ); log_migration("post-migration", storage_prefix_bounties, old_pallet_name, new_pallet_name); - log_migration("post-migration", storage_prefix_bounties_description, old_pallet_name, new_pallet_name); - log_migration("post-migration", storage_prefix_bounties_approvals, old_pallet_name, new_pallet_name); + log_migration( + "post-migration", + storage_prefix_bounties_description, + old_pallet_name, + new_pallet_name, + ); + log_migration( + "post-migration", + storage_prefix_bounties_approvals, + old_pallet_name, + new_pallet_name, + ); let old_pallet_prefix = twox_128(old_pallet_name.as_bytes()); - let old_bounties_count_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); - let old_bounties_key= [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); - let old_bounties_description_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); - let old_bounties_approvals_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + let old_bounties_count_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + let old_bounties_key = [&old_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); + let old_bounties_description_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let old_bounties_approvals_key = + [&old_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + assert!(storage::next_key(&old_bounties_count_key) + .map_or(true, |next_key| !next_key.starts_with(&old_bounties_count_key))); + assert!(storage::next_key(&old_bounties_key) + .map_or(true, |next_key| !next_key.starts_with(&old_bounties_key))); + assert!(storage::next_key(&old_bounties_description_key) + .map_or(true, |next_key| !next_key.starts_with(&old_bounties_description_key))); + assert!(storage::next_key(&old_bounties_approvals_key) + .map_or(true, |next_key| !next_key.starts_with(&old_bounties_approvals_key))); let new_pallet_prefix = twox_128(new_pallet_name.as_bytes()); - let new_bounties_count_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); + let new_bounties_count_key = + [&new_pallet_prefix, &twox_128(storage_prefix_bounties_count)[..]].concat(); let new_bounties_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties)[..]].concat(); - let new_bounties_description_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); - let new_bounties_approvals_key = [&new_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); - assert!(storage::next_key(&new_bounties_count_key)); + let new_bounties_description_key = + [&new_pallet_prefix, &twox_128(storage_prefix_bounties_description)[..]].concat(); + let new_bounties_approvals_key = + [&new_pallet_prefix, &twox_128(storage_prefix_bounties_approvals)[..]].concat(); + /* + assert!(storage::next_key(&new_bounties_count_key) + .map_or(true, |next_key| next_key.starts_with(&new_bounties_count_key))); + assert!(storage::next_key(&new_bounties_key) + .map_or(true, |next_key| next_key.starts_with(&new_bounties_key))); + assert!(storage::next_key(&new_bounties_description_key) + .map_or(true, |next_key| next_key.starts_with(&new_bounties_description_key))); + assert!(storage::next_key(&new_bounties_approvals_key) + .map_or(true, |next_key| next_key.starts_with(&new_bounties_approvals_key))); + */ assert_eq!(

::on_chain_storage_version(), 4); } diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 0b2ba97841d67..ea50f97c655b6 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -31,8 +31,8 @@ use frame_support::{ use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BadOrigin, BlakeTwo256, IdentityLookup}, - Perbill, + traits::{BadOrigin, BlakeTwo256, Hash, IdentityLookup}, + Perbill, Storage, }; use super::Event as BountiesEvent; @@ -936,42 +936,42 @@ fn extend_expiry() { }); } -/* #[test] fn test_migration_v4() { let mut s = Storage::default(); - //let reason1 = BlakeTwo256::hash(b""); - //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 index: u32 = 0; + + let bounty = Bounty:: { + proposer: 0, + value: 20, + fee: 20, + curator_deposit: 20, + bond: 50, + status: BountyStatus::::Proposed, }; 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()), + (pallet_bounties::Bounties::::hashed_key_for(index), bounty.encode().to_vec()), + (pallet_bounties::BountyDescriptions::::hashed_key_for(index), vec![0, 0]), ]; 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); + let old_pallet_name = ::PalletInfo::name::() + .expect("Bounties is part of runtime, so it has a name; qed"); + let new_pallet_name = "NewBounties"; + + crate::migrations::v4::pre_migration::(old_pallet_name, new_pallet_name); + crate::migrations::v4::migrate::(old_pallet_name, new_pallet_name); + crate::migrations::v4::post_migration::( + old_pallet_name, + new_pallet_name, + ); }); -}*/ +} #[test] fn genesis_funding_works() { From 166e70a9040a3f9dace5f26a0a26329a2c5c205f Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Thu, 9 Sep 2021 00:00:17 -0400 Subject: [PATCH 16/21] change checks in migration --- frame/bounties/src/migrations/v4.rs | 46 ++++------------------------- frame/bounties/src/tests.rs | 6 ++-- 2 files changed, 10 insertions(+), 42 deletions(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index 0e53446fe0a5d..37d275bb6a502 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -139,34 +139,6 @@ pub fn pre_migration::on_chain_storage_version(), 4); } diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index ea50f97c655b6..849347f99e4fd 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -31,7 +31,7 @@ use frame_support::{ use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BadOrigin, BlakeTwo256, Hash, IdentityLookup}, + traits::{BadOrigin, BlakeTwo256, IdentityLookup}, Perbill, Storage, }; @@ -940,7 +940,7 @@ fn extend_expiry() { fn test_migration_v4() { let mut s = Storage::default(); - let index: u32 = 0; + let index: u32 = 10; let bounty = Bounty:: { proposer: 0, @@ -952,8 +952,10 @@ fn test_migration_v4() { }; let data = vec![ + (pallet_bounties::BountyCount::::hashed_key().to_vec(), 10.encode().to_vec()), (pallet_bounties::Bounties::::hashed_key_for(index), bounty.encode().to_vec()), (pallet_bounties::BountyDescriptions::::hashed_key_for(index), vec![0, 0]), + (pallet_bounties::BountyApprovals::::hashed_key().to_vec(), vec![10 as u32].encode().to_vec()) ]; s.top = data.into_iter().collect(); From f5b10e16e626d55ba3c457b286c9cba71c1ff6e1 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Thu, 9 Sep 2021 03:56:23 -0400 Subject: [PATCH 17/21] remove unused values --- frame/bounties/src/migrations/v4.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index 37d275bb6a502..0d4dddadaa3b0 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -217,15 +217,6 @@ pub fn post_migration::on_chain_storage_version(), 4); } From 2065e692b90aa040c4867c7862f031bce7fd3e87 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Thu, 9 Sep 2021 12:00:21 +0200 Subject: [PATCH 18/21] Update frame/bounties/src/migrations/v4.rs --- frame/bounties/src/migrations/v4.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index 0d4dddadaa3b0..a252060d2b74d 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -149,7 +149,7 @@ pub fn pre_migration Date: Fri, 10 Sep 2021 01:09:56 -0400 Subject: [PATCH 19/21] cargo fmt --- frame/bounties/src/migrations/v4.rs | 8 +++----- frame/bounties/src/tests.rs | 5 ++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frame/bounties/src/migrations/v4.rs b/frame/bounties/src/migrations/v4.rs index a252060d2b74d..a1ca0e47680b0 100644 --- a/frame/bounties/src/migrations/v4.rs +++ b/frame/bounties/src/migrations/v4.rs @@ -151,11 +151,9 @@ pub fn pre_migration::hashed_key().to_vec(), 10.encode().to_vec()), (pallet_bounties::Bounties::::hashed_key_for(index), bounty.encode().to_vec()), (pallet_bounties::BountyDescriptions::::hashed_key_for(index), vec![0, 0]), - (pallet_bounties::BountyApprovals::::hashed_key().to_vec(), vec![10 as u32].encode().to_vec()) + ( + pallet_bounties::BountyApprovals::::hashed_key().to_vec(), + vec![10 as u32].encode().to_vec(), + ), ]; s.top = data.into_iter().collect(); From 1c3e5d96a9a1d3544799c5cabb6f26fd77585a51 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Mon, 13 Sep 2021 01:59:44 -0400 Subject: [PATCH 20/21] fix benchmarking --- frame/bounties/src/benchmarking.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/bounties/src/benchmarking.rs b/frame/bounties/src/benchmarking.rs index 26921031b51f1..1aa1eabdb5177 100644 --- a/frame/bounties/src/benchmarking.rs +++ b/frame/bounties/src/benchmarking.rs @@ -132,7 +132,7 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?; let beneficiary = T::Lookup::unlookup(account("beneficiary", 0, SEED)); @@ -143,7 +143,7 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?; let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED); @@ -180,7 +180,7 @@ benchmarks! { let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); - let bounty_id = BountyCount::get() - 1; + let bounty_id = BountyCount::::get() - 1; let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?; }: _(RawOrigin::Signed(curator), bounty_id, Vec::new()) verify { From 1b955e3163e3de1920b9c57b6007eed6693ad2a7 Mon Sep 17 00:00:00 2001 From: ferrell-code Date: Sat, 18 Sep 2021 14:47:42 -0400 Subject: [PATCH 21/21] trigger ci