From a0275942d2e391e6a9fc3181de95faca915d8d36 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 15 Mar 2023 13:51:41 +0000 Subject: [PATCH 01/60] pallet_treasury: implement treasury pallet over the pay trait in pallet_salary --- frame/treasury/src/lib.rs | 500 +++++++++++++------------------------- 1 file changed, 165 insertions(+), 335 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 450aee51f2ce8..d0f65f13e9180 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -66,33 +66,70 @@ pub mod weights; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_runtime::{ - traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, - Permill, RuntimeDebug, -}; -use sp_std::{collections::btree_map::BTreeMap, prelude::*}; - +use codec::FullCodec; use frame_support::{ - print, + dispatch::fmt::Debug, traits::{ - Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, - ReservableCurrency, WithdrawReasons, + tokens::{AssetId, Balance, BalanceConversion}, + Get, }, weights::Weight, PalletId, }; +use sp_runtime::{ + traits::{CheckedAdd, Saturating, StaticLookup, Zero}, + Permill, RuntimeDebug, +}; +use sp_std::{collections::btree_map::BTreeMap, prelude::*}; pub use pallet::*; pub use weights::WeightInfo; -pub type BalanceOf = - <>::Currency as Currency<::AccountId>>::Balance; -pub type PositiveImbalanceOf = <>::Currency as Currency< - ::AccountId, ->>::PositiveImbalance; -pub type NegativeImbalanceOf = <>::Currency as Currency< - ::AccountId, ->>::NegativeImbalance; +/// Status for making a payment via the `Pay::pay` trait function. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum PaymentStatus { + /// Payment is in progress. Nothing to report yet. + InProgress, + /// Payment status is unknowable. It will never be reported successful or failed. + Unknown, + /// Payment happened successfully. + Success, + /// Payment failed. It may safely be retried. + Failure, +} + +// TODO: Should be replaced with a version in frame_support +pub trait Pay { + /// The type by which we measure units of the currency in which we make payments. + type Balance: Balance; + /// The type by which we identify the individuals to whom a payment may be made. + type AccountId; + /// An identifier given to an individual payment. + type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; + type AssetId: AssetId; + /// Make a payment and return an identifier for later evaluation of success in some off-chain + /// mechanism (likely an event, but possibly not on this chain). + fn pay( + who: &Self::AccountId, + asset_id: Self::AssetId, + amount: Self::Balance, + ) -> Result; + /// Check how a payment has proceeded. `id` must have been a previously returned by `pay` for + /// the result of this call to be meaningful. + fn check_payment(id: Self::Id) -> PaymentStatus; + /// Ensure that a call to pay with the given parameters will be successful if done immediately + /// after this call. Used in benchmarking code. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); + /// Ensure that a call to `check_payment` with the given parameters will return either `Success` + /// or `Failure`. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id); +} + +pub type BalanceOf = <>::Paymaster as Pay>::Balance; + type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; /// A trait to allow the Treasury Pallet to spend it's funds for other purposes. @@ -108,35 +145,38 @@ type AccountIdLookupOf = <::Lookup as StaticLookup /// mark this value as `true`. This will prevent the treasury from burning the excess funds. #[impl_trait_for_tuples::impl_for_tuples(30)] pub trait SpendFunds, I: 'static = ()> { - fn spend_funds( - budget_remaining: &mut BalanceOf, - imbalance: &mut PositiveImbalanceOf, - total_weight: &mut Weight, - missed_any: &mut bool, - ); + fn spend_funds(total_weight: &mut Weight, total_spent: T::Balance, total_missed: u32); } /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; +/// A count of proposals. Just a `u32`. +pub type ProposalsCount = u32; /// A spending proposal. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] -pub struct Proposal { +pub struct Proposal { /// The account proposing it. proposer: AccountId, - /// The (total) amount that should be paid if the proposal is accepted. - value: Balance, + /// The asset_id of the amount to be paid + asset_id: AssetId, + /// The (total) amount that should be paid. + value: SpendBalance, /// The account to whom the payment should be made if the proposal is accepted. beneficiary: AccountId, - /// The amount held on deposit (reserved) for making this proposal. - bond: Balance, + /// The amount to be paid, but normalized to the native asset class + normalized_value: Balance, + // payment_status: PaymentStatus, + payment_id: Option, } #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{dispatch_context::with_context, pallet_prelude::*}; + use frame_support::{ + dispatch_context::with_context, pallet_prelude::*, traits::tokens::AssetId, + }; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -144,35 +184,29 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config { - /// The staking balance. - type Currency: Currency + ReservableCurrency; + type Balance: Balance; + + /// The identifier for what asset should be spent. + type AssetId: AssetId; - /// Origin from which approvals must come. - type ApproveOrigin: EnsureOrigin; + /// Means by which we can make payments to accounts. This also defines the currency and the + /// balance which we use to denote that currency. + type Paymaster: Pay< + AccountId = ::AccountId, + AssetId = Self::AssetId, + >; - /// Origin from which rejections must come. - type RejectOrigin: EnsureOrigin; + type BalanceConverter: BalanceConversion< + BalanceOf, + Self::AssetId, + Self::Balance, + Error = Error, + >; /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; - /// Handler for the unbalanced decrease when slashing for a rejected proposal or bounty. - type OnSlash: OnUnbalanced>; - - /// Fraction of a proposal's value that should be bonded in order to place the proposal. - /// An accepted proposal gets these back. A rejected proposal does not. - #[pallet::constant] - type ProposalBond: Get; - - /// Minimum amount of funds that should be placed in a deposit for making a proposal. - #[pallet::constant] - type ProposalBondMinimum: Get>; - - /// Maximum amount of funds that should be placed in a deposit for making a proposal. - #[pallet::constant] - type ProposalBondMaximum: Get>>; - /// Period between successive spends. #[pallet::constant] type SpendPeriod: Get; @@ -185,54 +219,29 @@ pub mod pallet { #[pallet::constant] type PalletId: Get; - /// Handler for the unbalanced decrease when treasury funds are burned. - type BurnDestination: OnUnbalanced>; - /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; /// Runtime hooks to external pallet using treasury to compute spend funds. type SpendFunds: SpendFunds; - /// The maximum number of approvals that can wait in the spending queue. - /// - /// NOTE: This parameter is also used within the Bounties Pallet extension if enabled. - #[pallet::constant] - type MaxApprovals: Get; - /// The origin required for approving spends from the treasury outside of the proposal /// process. The `Success` value is the maximum amount that this origin is allowed to /// spend at a time. type SpendOrigin: EnsureOrigin>; } - /// Number of proposals that have been made. - #[pallet::storage] - #[pallet::getter(fn proposal_count)] - pub(crate) type ProposalCount = StorageValue<_, ProposalIndex, ValueQuery>; - /// Proposals that have been made. #[pallet::storage] #[pallet::getter(fn proposals)] - pub type Proposals, I: 'static = ()> = StorageMap< + pub type Proposals, I: 'static = ()> = CountedStorageMap< _, Twox64Concat, ProposalIndex, - Proposal>, + Proposal, ::Id>, OptionQuery, >; - /// The amount which has been reported as inactive to Currency. - #[pallet::storage] - pub type Deactivated, I: 'static = ()> = - StorageValue<_, BalanceOf, ValueQuery>; - - /// Proposal indices that have been approved but not yet awarded. - #[pallet::storage] - #[pallet::getter(fn approvals)] - pub type Approvals, I: 'static = ()> = - StorageValue<_, BoundedVec, ValueQuery>; - #[pallet::genesis_config] pub struct GenesisConfig; @@ -259,14 +268,7 @@ pub mod pallet { #[pallet::genesis_build] impl, I: 'static> GenesisBuild for GenesisConfig { - fn build(&self) { - // Create Treasury account - let account_id = >::account_id(); - let min = T::Currency::minimum_balance(); - if T::Currency::free_balance(&account_id) < min { - let _ = T::Currency::make_free_balance_be(&account_id, min); - } - } + fn build(&self) {} } #[pallet::event] @@ -275,17 +277,10 @@ pub mod pallet { /// New proposal. Proposed { proposal_index: ProposalIndex }, /// We have ended a spend period and will now allocate funds. - Spending { budget_remaining: BalanceOf }, - /// Some funds have been allocated. - Awarded { proposal_index: ProposalIndex, award: BalanceOf, account: T::AccountId }, - /// A proposal was rejected; funds were slashed. - Rejected { proposal_index: ProposalIndex, slashed: BalanceOf }, - /// Some of our funds have been burnt. - Burnt { burnt_funds: BalanceOf }, - /// Spending has finished; this is the amount that rolls over until next spend. - Rollover { rollover_balance: BalanceOf }, - /// Some funds have been deposited. - Deposit { value: BalanceOf }, + Spending { waiting_proposals: ProposalsCount }, + /// Spending has finished; this is the number of proposals rolled over till next + /// T::SpendPeriod. + Rollover { rollover_proposals: ProposalsCount, allocated_proposals: ProposalsCount }, /// A new spend proposal has been approved. SpendApproved { proposal_index: ProposalIndex, @@ -294,22 +289,32 @@ pub mod pallet { }, /// The inactive funds of the pallet have been updated. UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, + /// The proposal was paid successfully + ProposalPaymentSuccess { + proposal_index: ProposalIndex, + asset_id: T::AssetId, + amount: BalanceOf, + }, + // The proposal payment failed. Payment will be retried in next spend period. + ProposalPaymentFailure { + proposal_index: ProposalIndex, + asset_id: T::AssetId, + amount: BalanceOf, + }, } /// Error for the treasury pallet. #[pallet::error] pub enum Error { - /// Proposer's balance is too low. - InsufficientProposersBalance, /// No proposal or bounty at that index. InvalidIndex, - /// Too many approvals in the queue. - TooManyApprovals, + // /// Too many approvals in the queue. + // TooManyApprovals, /// The spend origin is valid but the amount it is allowed to spend is lower than the /// amount to be spent. InsufficientPermission, - /// Proposal has not been approved. - ProposalNotApproved, + /// Unable to convert asset to native balance + BalanceConversionFailed, } #[pallet::hooks] @@ -317,18 +322,6 @@ pub mod pallet { /// ## Complexity /// - `O(A)` where `A` is the number of approvals fn on_initialize(n: T::BlockNumber) -> Weight { - let pot = Self::pot(); - let deactivated = Deactivated::::get(); - if pot != deactivated { - T::Currency::reactivate(deactivated); - T::Currency::deactivate(pot); - Deactivated::::put(&pot); - Self::deposit_event(Event::::UpdatedInactive { - reactivated: deactivated, - deactivated: pot, - }); - } - // Check to see if we should spend some funds! if (n % T::SpendPeriod::get()).is_zero() { Self::spend_funds() @@ -345,82 +338,6 @@ pub mod pallet { #[pallet::call] impl, I: 'static> Pallet { - /// Put forward a suggestion for spending. A deposit proportional to the value - /// is reserved and slashed if the proposal is rejected. It is returned once the - /// proposal is awarded. - /// - /// ## Complexity - /// - O(1) - #[pallet::call_index(0)] - #[pallet::weight(T::WeightInfo::propose_spend())] - pub fn propose_spend( - origin: OriginFor, - #[pallet::compact] value: BalanceOf, - beneficiary: AccountIdLookupOf, - ) -> DispatchResult { - let proposer = ensure_signed(origin)?; - let beneficiary = T::Lookup::lookup(beneficiary)?; - - let bond = Self::calculate_bond(value); - T::Currency::reserve(&proposer, bond) - .map_err(|_| Error::::InsufficientProposersBalance)?; - - let c = Self::proposal_count(); - >::put(c + 1); - >::insert(c, Proposal { proposer, value, beneficiary, bond }); - - Self::deposit_event(Event::Proposed { proposal_index: c }); - Ok(()) - } - - /// Reject a proposed spend. The original deposit will be slashed. - /// - /// May only be called from `T::RejectOrigin`. - /// - /// ## Complexity - /// - O(1) - #[pallet::call_index(1)] - #[pallet::weight((T::WeightInfo::reject_proposal(), DispatchClass::Operational))] - pub fn reject_proposal( - origin: OriginFor, - #[pallet::compact] proposal_id: ProposalIndex, - ) -> DispatchResult { - T::RejectOrigin::ensure_origin(origin)?; - - let proposal = - >::take(&proposal_id).ok_or(Error::::InvalidIndex)?; - let value = proposal.bond; - let imbalance = T::Currency::slash_reserved(&proposal.proposer, value).0; - T::OnSlash::on_unbalanced(imbalance); - - Self::deposit_event(Event::::Rejected { - proposal_index: proposal_id, - slashed: value, - }); - Ok(()) - } - - /// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary - /// and the original deposit will be returned. - /// - /// May only be called from `T::ApproveOrigin`. - /// - /// ## Complexity - /// - O(1). - #[pallet::call_index(2)] - #[pallet::weight((T::WeightInfo::approve_proposal(T::MaxApprovals::get()), DispatchClass::Operational))] - pub fn approve_proposal( - origin: OriginFor, - #[pallet::compact] proposal_id: ProposalIndex, - ) -> DispatchResult { - T::ApproveOrigin::ensure_origin(origin)?; - - ensure!(>::contains_key(proposal_id), Error::::InvalidIndex); - Approvals::::try_append(proposal_id) - .map_err(|_| Error::::TooManyApprovals)?; - Ok(()) - } - /// Propose and approve a spend of treasury funds. /// /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. @@ -433,6 +350,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, + asset_id: T::AssetId, #[pallet::compact] amount: BalanceOf, beneficiary: AccountIdLookupOf, ) -> DispatchResult { @@ -460,174 +378,86 @@ pub mod pallet { .unwrap_or(Ok(()))?; let beneficiary = T::Lookup::lookup(beneficiary)?; - let proposal_index = Self::proposal_count(); - Approvals::::try_append(proposal_index) - .map_err(|_| Error::::TooManyApprovals)?; + let proposal = Proposal { proposer: beneficiary.clone(), + asset_id, value: amount, beneficiary: beneficiary.clone(), - bond: Default::default(), + normalized_value: T::BalanceConverter::to_asset_balance(amount, asset_id)?, + // payment_status: PaymentStatus::Unknown, + payment_id: None, }; + + let proposal_index = Proposals::::count(); Proposals::::insert(proposal_index, proposal); - ProposalCount::::put(proposal_index + 1); Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary }); Ok(()) } - - /// Force a previously approved proposal to be removed from the approval queue. - /// The original deposit will no longer be returned. - /// - /// May only be called from `T::RejectOrigin`. - /// - `proposal_id`: The index of a proposal - /// - /// ## Complexity - /// - O(A) where `A` is the number of approvals - /// - /// Errors: - /// - `ProposalNotApproved`: The `proposal_id` supplied was not found in the approval queue, - /// i.e., the proposal has not been approved. This could also mean the proposal does not - /// exist altogether, thus there is no way it would have been approved in the first place. - #[pallet::call_index(4)] - #[pallet::weight((T::WeightInfo::remove_approval(), DispatchClass::Operational))] - pub fn remove_approval( - origin: OriginFor, - #[pallet::compact] proposal_id: ProposalIndex, - ) -> DispatchResult { - T::RejectOrigin::ensure_origin(origin)?; - - Approvals::::try_mutate(|v| -> DispatchResult { - if let Some(index) = v.iter().position(|x| x == &proposal_id) { - v.remove(index); - Ok(()) - } else { - Err(Error::::ProposalNotApproved.into()) - } - })?; - - Ok(()) - } } } impl, I: 'static> Pallet { - // Add public immutables and private mutables. - - /// The account ID of the treasury pot. - /// - /// This actually does computation. If you need to keep using it, then make sure you cache the - /// value and only call this once. - pub fn account_id() -> T::AccountId { - T::PalletId::get().into_account_truncating() - } - - /// The needed bond for a proposal whose spend is `value`. - fn calculate_bond(value: BalanceOf) -> BalanceOf { - let mut r = T::ProposalBondMinimum::get().max(T::ProposalBond::get() * value); - if let Some(m) = T::ProposalBondMaximum::get() { - r = r.min(m); - } - r - } - /// Spend some money! returns number of approvals before spend. pub fn spend_funds() -> Weight { let mut total_weight = Weight::zero(); - - let mut budget_remaining = Self::pot(); - Self::deposit_event(Event::Spending { budget_remaining }); - let account_id = Self::account_id(); - - let mut missed_any = false; - let mut imbalance = >::zero(); - let proposals_len = Approvals::::mutate(|v| { - let proposals_approvals_len = v.len() as u32; - v.retain(|&index| { - // Should always be true, but shouldn't panic if false or we're screwed. - if let Some(p) = Self::proposals(index) { - if p.value <= budget_remaining { - budget_remaining -= p.value; - >::remove(index); - - // return their deposit. - let err_amount = T::Currency::unreserve(&p.proposer, p.bond); - debug_assert!(err_amount.is_zero()); - - // provide the allocation. - imbalance.subsume(T::Currency::deposit_creating(&p.beneficiary, p.value)); - - Self::deposit_event(Event::Awarded { - proposal_index: index, - award: p.value, - account: p.beneficiary, - }); - false - } else { - missed_any = true; - true - } - } else { - false + let mut total_spent = T::Balance::zero(); + let mut missed_proposals: u32 = 0; + let proposals_len = Proposals::::count(); + + Self::deposit_event(Event::Spending { waiting_proposals: proposals_len }); + + for key in Proposals::::iter_keys() { + if let Some(mut p) = Proposals::::get(key) { + match p.payment_id { + None => + if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_id, p.value) { + total_spent += p.normalized_value; + p.payment_id = Some(id); + Proposals::::set(key, Some(p)); + } else { + missed_proposals = missed_proposals.saturating_add(1); + }, + Some(payment_id) => match T::Paymaster::check_payment(payment_id) { + PaymentStatus::Failure => { + // try again in the next `T::SpendPeriod`. + missed_proposals = missed_proposals.saturating_add(1); + Self::deposit_event(Event::ProposalPaymentFailure { + proposal_index: key, + asset_id: p.asset_id, + amount: p.value, + }); + }, + PaymentStatus::Success => { + Proposals::::remove(key); + Self::deposit_event(Event::ProposalPaymentSuccess { + proposal_index: key, + asset_id: p.asset_id, + amount: p.value, + }); + }, + // PaymentStatus::InProgress and PaymentStatus::Unknown indicate that the + // proposal status is inconclusive, and might still be successful or failed + // in the future. + _ => {}, + }, } - }); - proposals_approvals_len - }); + } else { + } + } total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); // Call Runtime hooks to external pallet using treasury to compute spend funds. - T::SpendFunds::spend_funds( - &mut budget_remaining, - &mut imbalance, - &mut total_weight, - &mut missed_any, - ); - - if !missed_any { - // burn some proportion of the remaining budget if we run a surplus. - let burn = (T::Burn::get() * budget_remaining).min(budget_remaining); - budget_remaining -= burn; - - let (debit, credit) = T::Currency::pair(burn); - imbalance.subsume(debit); - T::BurnDestination::on_unbalanced(credit); - Self::deposit_event(Event::Burnt { burnt_funds: burn }) - } + // We could trigger burning of funds in the spendFunds hook as well. + T::SpendFunds::spend_funds(&mut total_weight, total_spent, missed_proposals); - // Must never be an error, but better to be safe. - // proof: budget_remaining is account free balance minus ED; - // Thus we can't spend more than account free balance minus ED; - // Thus account is kept alive; qed; - if let Err(problem) = - T::Currency::settle(&account_id, imbalance, WithdrawReasons::TRANSFER, KeepAlive) - { - print("Inconsistent state - couldn't settle imbalance for funds spent by treasury"); - // Nothing else to do here. - drop(problem); - } - - Self::deposit_event(Event::Rollover { rollover_balance: budget_remaining }); + Self::deposit_event(Event::Rollover { + rollover_proposals: missed_proposals, + allocated_proposals: proposals_len.saturating_sub(missed_proposals), + }); total_weight } - - /// Return the amount of money in the pot. - // The existential deposit is not part of the pot so treasury account never gets deleted. - pub fn pot() -> BalanceOf { - T::Currency::free_balance(&Self::account_id()) - // Must never be less than 0 but better be safe. - .saturating_sub(T::Currency::minimum_balance()) - } -} - -impl, I: 'static> OnUnbalanced> for Pallet { - fn on_nonzero_unbalanced(amount: NegativeImbalanceOf) { - let numeric_amount = amount.peek(); - - // Must resolve into existing but better to be safe. - let _ = T::Currency::resolve_creating(&Self::account_id(), amount); - - Self::deposit_event(Event::Deposit { value: numeric_amount }); - } } From 29194c3ee3321c6fc4ebcdc4450a926293edab43 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 15 Mar 2023 17:58:48 +0000 Subject: [PATCH 02/60] update the new spend logic to exist parallel to original spend logic --- frame/treasury/src/lib.rs | 492 +++++++++++++++++++++++++++++++++++--- 1 file changed, 457 insertions(+), 35 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index d0f65f13e9180..7d38241f6a5cc 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -63,21 +63,22 @@ mod benchmarking; mod tests; pub mod weights; -use codec::{Decode, Encode, MaxEncodedLen}; -use scale_info::TypeInfo; - -use codec::FullCodec; +use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use frame_support::{ dispatch::fmt::Debug, + print, traits::{ tokens::{AssetId, Balance, BalanceConversion}, - Get, + Currency, + ExistenceRequirement::KeepAlive, + Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, }, weights::Weight, PalletId, }; +use scale_info::TypeInfo; use sp_runtime::{ - traits::{CheckedAdd, Saturating, StaticLookup, Zero}, + traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, Permill, RuntimeDebug, }; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; @@ -128,7 +129,15 @@ pub trait Pay { fn ensure_concluded(id: Self::Id); } -pub type BalanceOf = <>::Paymaster as Pay>::Balance; +pub type PayBalanceOf = <>::Paymaster as Pay>::Balance; +pub type BalanceOf = + <>::Currency as Currency<::AccountId>>::Balance; +pub type PositiveImbalanceOf = <>::Currency as Currency< + ::AccountId, +>>::PositiveImbalance; +pub type NegativeImbalanceOf = <>::Currency as Currency< + ::AccountId, +>>::NegativeImbalance; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; @@ -143,6 +152,16 @@ type AccountIdLookupOf = <::Lookup as StaticLookup /// value. /// * `missed_any`: If there were items that you want to spend on, but there were not enough funds, /// mark this value as `true`. This will prevent the treasury from burning the excess funds. +#[impl_trait_for_tuples::impl_for_tuples(30)] +pub trait SpendFundsLocal, I: 'static = ()> { + fn spend_funds( + budget_remaining: &mut BalanceOf, + imbalance: &mut PositiveImbalanceOf, + total_weight: &mut Weight, + missed_any: &mut bool, + ); +} + #[impl_trait_for_tuples::impl_for_tuples(30)] pub trait SpendFunds, I: 'static = ()> { fn spend_funds(total_weight: &mut Weight, total_spent: T::Balance, total_missed: u32); @@ -156,13 +175,27 @@ pub type ProposalsCount = u32; /// A spending proposal. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] -pub struct Proposal { +pub struct Proposal { + /// The account proposing it. + proposer: AccountId, + /// The (total) amount that should be paid if the proposal is accepted. + value: Balance, + /// The account to whom the payment should be made if the proposal is accepted. + beneficiary: AccountId, + /// The amount held on deposit (reserved) for making this proposal. + bond: Balance, +} + +/// A spending proposal. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +pub struct PendingPayment { /// The account proposing it. proposer: AccountId, /// The asset_id of the amount to be paid - asset_id: AssetId, + asset_id: AssetKind, /// The (total) amount that should be paid. - value: SpendBalance, + value: AssetBalance, /// The account to whom the payment should be made if the proposal is accepted. beneficiary: AccountId, /// The amount to be paid, but normalized to the native asset class @@ -184,6 +217,15 @@ pub mod pallet { #[pallet::config] pub trait Config: frame_system::Config { + /// The staking balance. + type Currency: Currency + ReservableCurrency; + + /// Origin from which approvals must come. + type ApproveOrigin: EnsureOrigin; + + /// Origin from which rejections must come. + type RejectOrigin: EnsureOrigin; + type Balance: Balance; /// The identifier for what asset should be spent. @@ -197,7 +239,7 @@ pub mod pallet { >; type BalanceConverter: BalanceConversion< - BalanceOf, + PayBalanceOf, Self::AssetId, Self::Balance, Error = Error, @@ -207,6 +249,22 @@ pub mod pallet { type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// Handler for the unbalanced decrease when slashing for a rejected proposal or bounty. + type OnSlash: OnUnbalanced>; + + /// Fraction of a proposal's value that should be bonded in order to place the proposal. + /// An accepted proposal gets these back. A rejected proposal does not. + #[pallet::constant] + type ProposalBond: Get; + + /// Minimum amount of funds that should be placed in a deposit for making a proposal. + #[pallet::constant] + type ProposalBondMinimum: Get>; + + /// Maximum amount of funds that should be placed in a deposit for making a proposal. + #[pallet::constant] + type ProposalBondMaximum: Get>>; + /// Period between successive spends. #[pallet::constant] type SpendPeriod: Get; @@ -219,29 +277,79 @@ pub mod pallet { #[pallet::constant] type PalletId: Get; + /// Handler for the unbalanced decrease when treasury funds are burned. + type BurnDestination: OnUnbalanced>; + /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; /// Runtime hooks to external pallet using treasury to compute spend funds. type SpendFunds: SpendFunds; + /// Runtime hooks to external pallet using treasury to compute spend funds. + type SpendFundsLocal: SpendFundsLocal; + + /// The maximum number of approvals that can wait in the spending queue. + /// + /// NOTE: This parameter is also used within the Bounties Pallet extension if enabled. + #[pallet::constant] + type MaxApprovals: Get; + + /// The origin required for approving spends from the treasury outside of the proposal + /// process. The `Success` value is the maximum amount that this origin is allowed to + /// spend at a time. + type SpendOriginLocal: EnsureOrigin>; + /// The origin required for approving spends from the treasury outside of the proposal /// process. The `Success` value is the maximum amount that this origin is allowed to /// spend at a time. - type SpendOrigin: EnsureOrigin>; + type SpendOrigin: EnsureOrigin>; } + /// Number of proposals that have been made. + #[pallet::storage] + #[pallet::getter(fn proposal_count)] + pub(crate) type ProposalCount = StorageValue<_, ProposalIndex, ValueQuery>; + /// Proposals that have been made. #[pallet::storage] #[pallet::getter(fn proposals)] - pub type Proposals, I: 'static = ()> = CountedStorageMap< + pub type Proposals, I: 'static = ()> = StorageMap< + _, + Twox64Concat, + ProposalIndex, + Proposal>, + OptionQuery, + >; + + /// Proposals that have been made. + #[pallet::storage] + #[pallet::getter(fn pending_payments)] + pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, Twox64Concat, ProposalIndex, - Proposal, ::Id>, + PendingPayment< + T::AccountId, + T::Balance, + T::AssetId, + PayBalanceOf, + ::Id, + >, OptionQuery, >; + /// The amount which has been reported as inactive to Currency. + #[pallet::storage] + pub type Deactivated, I: 'static = ()> = + StorageValue<_, BalanceOf, ValueQuery>; + + /// Proposal indices that have been approved but not yet awarded. + #[pallet::storage] + #[pallet::getter(fn approvals)] + pub type Approvals, I: 'static = ()> = + StorageValue<_, BoundedVec, ValueQuery>; + #[pallet::genesis_config] pub struct GenesisConfig; @@ -277,42 +385,63 @@ pub mod pallet { /// New proposal. Proposed { proposal_index: ProposalIndex }, /// We have ended a spend period and will now allocate funds. - Spending { waiting_proposals: ProposalsCount }, + Spending { budget_remaining: BalanceOf }, + /// Some funds have been allocated. + Awarded { proposal_index: ProposalIndex, award: BalanceOf, account: T::AccountId }, + /// A proposal was rejected; funds were slashed. + Rejected { proposal_index: ProposalIndex, slashed: BalanceOf }, + /// Some of our funds have been burnt. + Burnt { burnt_funds: BalanceOf }, + /// Spending has finished; this is the amount that rolls over until next spend. + Rollover { rollover_balance: BalanceOf }, + /// Some funds have been deposited. + Deposit { value: BalanceOf }, + /// We have ended a spend period and will now allocate funds. + ProcessingProposals { waiting_proposals: ProposalIndex }, /// Spending has finished; this is the number of proposals rolled over till next /// T::SpendPeriod. - Rollover { rollover_proposals: ProposalsCount, allocated_proposals: ProposalsCount }, + RolloverPayments { rollover_proposals: ProposalsCount, allocated_proposals: ProposalsCount }, /// A new spend proposal has been approved. SpendApproved { proposal_index: ProposalIndex, amount: BalanceOf, beneficiary: T::AccountId, }, + QueuedPayment { + proposal_index: ProposalIndex, + amount: PayBalanceOf, + beneficiary: T::AccountId, + }, /// The inactive funds of the pallet have been updated. UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, /// The proposal was paid successfully ProposalPaymentSuccess { proposal_index: ProposalIndex, asset_id: T::AssetId, - amount: BalanceOf, + amount: PayBalanceOf, }, // The proposal payment failed. Payment will be retried in next spend period. ProposalPaymentFailure { proposal_index: ProposalIndex, asset_id: T::AssetId, - amount: BalanceOf, + amount: PayBalanceOf, }, } /// Error for the treasury pallet. #[pallet::error] pub enum Error { + /// Proposer's balance is too low. + InsufficientProposersBalance, /// No proposal or bounty at that index. InvalidIndex, - // /// Too many approvals in the queue. - // TooManyApprovals, + /// Too many approvals in the queue. + TooManyApprovals, /// The spend origin is valid but the amount it is allowed to spend is lower than the /// amount to be spent. InsufficientPermission, + /// Proposal has not been approved. + ProposalNotApproved, /// Unable to convert asset to native balance BalanceConversionFailed, } @@ -322,9 +451,20 @@ pub mod pallet { /// ## Complexity /// - `O(A)` where `A` is the number of approvals fn on_initialize(n: T::BlockNumber) -> Weight { + let pot = Self::pot(); + let deactivated = Deactivated::::get(); + if pot != deactivated { + T::Currency::reactivate(deactivated); + T::Currency::deactivate(pot); + Deactivated::::put(&pot); + Self::deposit_event(Event::::UpdatedInactive { + reactivated: deactivated, + deactivated: pot, + }); + } // Check to see if we should spend some funds! if (n % T::SpendPeriod::get()).is_zero() { - Self::spend_funds() + Self::spend_funds().saturating_add(Self::spend_funds_local()) } else { Weight::zero() } @@ -338,6 +478,82 @@ pub mod pallet { #[pallet::call] impl, I: 'static> Pallet { + /// Put forward a suggestion for spending. A deposit proportional to the value + /// is reserved and slashed if the proposal is rejected. It is returned once the + /// proposal is awarded. + /// + /// ## Complexity + /// - O(1) + #[pallet::call_index(0)] + #[pallet::weight(T::WeightInfo::propose_spend())] + pub fn propose_spend( + origin: OriginFor, + #[pallet::compact] value: BalanceOf, + beneficiary: AccountIdLookupOf, + ) -> DispatchResult { + let proposer = ensure_signed(origin)?; + let beneficiary = T::Lookup::lookup(beneficiary)?; + + let bond = Self::calculate_bond(value); + T::Currency::reserve(&proposer, bond) + .map_err(|_| Error::::InsufficientProposersBalance)?; + + let c = Self::proposal_count(); + >::put(c + 1); + >::insert(c, Proposal { proposer, value, beneficiary, bond }); + + Self::deposit_event(Event::Proposed { proposal_index: c }); + Ok(()) + } + + /// Reject a proposed spend. The original deposit will be slashed. + /// + /// May only be called from `T::RejectOrigin`. + /// + /// ## Complexity + /// - O(1) + #[pallet::call_index(1)] + #[pallet::weight((T::WeightInfo::reject_proposal(), DispatchClass::Operational))] + pub fn reject_proposal( + origin: OriginFor, + #[pallet::compact] proposal_id: ProposalIndex, + ) -> DispatchResult { + T::RejectOrigin::ensure_origin(origin)?; + + let proposal = + >::take(&proposal_id).ok_or(Error::::InvalidIndex)?; + let value = proposal.bond; + let imbalance = T::Currency::slash_reserved(&proposal.proposer, value).0; + T::OnSlash::on_unbalanced(imbalance); + + Self::deposit_event(Event::::Rejected { + proposal_index: proposal_id, + slashed: value, + }); + Ok(()) + } + + /// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary + /// and the original deposit will be returned. + /// + /// May only be called from `T::ApproveOrigin`. + /// + /// ## Complexity + /// - O(1). + #[pallet::call_index(2)] + #[pallet::weight((T::WeightInfo::approve_proposal(T::MaxApprovals::get()), DispatchClass::Operational))] + pub fn approve_proposal( + origin: OriginFor, + #[pallet::compact] proposal_id: ProposalIndex, + ) -> DispatchResult { + T::ApproveOrigin::ensure_origin(origin)?; + + ensure!(>::contains_key(proposal_id), Error::::InvalidIndex); + Approvals::::try_append(proposal_id) + .map_err(|_| Error::::TooManyApprovals)?; + Ok(()) + } + /// Propose and approve a spend of treasury funds. /// /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. @@ -348,16 +564,67 @@ pub mod pallet { /// beneficiary. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::spend())] + pub fn spend_local( + origin: OriginFor, + #[pallet::compact] amount: BalanceOf, + beneficiary: AccountIdLookupOf, + ) -> DispatchResult { + let max_amount = T::SpendOriginLocal::ensure_origin(origin)?; + ensure!(amount <= max_amount, Error::::InsufficientPermission); + with_context::>, _>(|v| { + let context = v.or_default(); + // We group based on `max_amount`, to dinstinguish between different kind of + // origins. (assumes that all origins have different `max_amount`) + // + // Worst case is that we reject some "valid" request. + let spend = context.spend_in_context.entry(max_amount).or_default(); + // Ensure that we don't overflow nor use more than `max_amount` + if spend.checked_add(&amount).map(|s| s > max_amount).unwrap_or(true) { + Err(Error::::InsufficientPermission) + } else { + *spend = spend.saturating_add(amount); + Ok(()) + } + }) + .unwrap_or(Ok(()))?; + + let beneficiary = T::Lookup::lookup(beneficiary)?; + let proposal_index = Self::proposal_count(); + Approvals::::try_append(proposal_index) + .map_err(|_| Error::::TooManyApprovals)?; + let proposal = Proposal { + proposer: beneficiary.clone(), + value: amount, + beneficiary: beneficiary.clone(), + bond: Default::default(), + }; + Proposals::::insert(proposal_index, proposal); + ProposalCount::::put(proposal_index + 1); + + Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary }); + Ok(()) + } + + /// Propose and approve a spend of treasury funds. + /// + /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. + /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. + /// - `beneficiary`: The destination account for the transfer. + /// + /// NOTE: For record-keeping purposes, the proposer is deemed to be equivalent to the + /// beneficiary. + #[pallet::call_index(5)] + #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, asset_id: T::AssetId, - #[pallet::compact] amount: BalanceOf, + #[pallet::compact] amount: PayBalanceOf, beneficiary: AccountIdLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; ensure!(amount <= max_amount, Error::::InsufficientPermission); - with_context::>, _>(|v| { + with_context::>, _>(|v| { let context = v.or_default(); // We group based on `max_amount`, to dinstinguish between different kind of @@ -379,43 +646,94 @@ pub mod pallet { let beneficiary = T::Lookup::lookup(beneficiary)?; - let proposal = Proposal { + let proposal = PendingPayment { proposer: beneficiary.clone(), asset_id, value: amount, beneficiary: beneficiary.clone(), normalized_value: T::BalanceConverter::to_asset_balance(amount, asset_id)?, - // payment_status: PaymentStatus::Unknown, payment_id: None, }; - let proposal_index = Proposals::::count(); - Proposals::::insert(proposal_index, proposal); + let proposal_index = PendingPayments::::count(); + PendingPayments::::insert(proposal_index, proposal); + + Self::deposit_event(Event::QueuedPayment { proposal_index, amount, beneficiary }); + Ok(()) + } + + /// Force a previously approved proposal to be removed from the approval queue. + /// The original deposit will no longer be returned. + /// + /// May only be called from `T::RejectOrigin`. + /// - `proposal_id`: The index of a proposal + /// + /// ## Complexity + /// - O(A) where `A` is the number of approvals + /// + /// Errors: + /// - `ProposalNotApproved`: The `proposal_id` supplied was not found in the approval queue, + /// i.e., the proposal has not been approved. This could also mean the proposal does not + /// exist altogether, thus there is no way it would have been approved in the first place. + #[pallet::call_index(4)] + #[pallet::weight((T::WeightInfo::remove_approval(), DispatchClass::Operational))] + pub fn remove_approval( + origin: OriginFor, + #[pallet::compact] proposal_id: ProposalIndex, + ) -> DispatchResult { + T::RejectOrigin::ensure_origin(origin)?; + + Approvals::::try_mutate(|v| -> DispatchResult { + if let Some(index) = v.iter().position(|x| x == &proposal_id) { + v.remove(index); + Ok(()) + } else { + Err(Error::::ProposalNotApproved.into()) + } + })?; - Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary }); Ok(()) } } } impl, I: 'static> Pallet { + // Add public immutables and private mutables. + + /// The account ID of the treasury pot. + /// + /// This actually does computation. If you need to keep using it, then make sure you cache the + /// value and only call this once. + pub fn account_id() -> T::AccountId { + T::PalletId::get().into_account_truncating() + } + + /// The needed bond for a proposal whose spend is `value`. + fn calculate_bond(value: BalanceOf) -> BalanceOf { + let mut r = T::ProposalBondMinimum::get().max(T::ProposalBond::get() * value); + if let Some(m) = T::ProposalBondMaximum::get() { + r = r.min(m); + } + r + } + /// Spend some money! returns number of approvals before spend. pub fn spend_funds() -> Weight { let mut total_weight = Weight::zero(); let mut total_spent = T::Balance::zero(); let mut missed_proposals: u32 = 0; - let proposals_len = Proposals::::count(); + let proposals_len = PendingPayments::::count(); - Self::deposit_event(Event::Spending { waiting_proposals: proposals_len }); + Self::deposit_event(Event::ProcessingProposals { waiting_proposals: proposals_len }); - for key in Proposals::::iter_keys() { - if let Some(mut p) = Proposals::::get(key) { + for key in PendingPayments::::iter_keys() { + if let Some(mut p) = PendingPayments::::get(key) { match p.payment_id { None => if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_id, p.value) { total_spent += p.normalized_value; p.payment_id = Some(id); - Proposals::::set(key, Some(p)); + PendingPayments::::set(key, Some(p)); } else { missed_proposals = missed_proposals.saturating_add(1); }, @@ -428,9 +746,13 @@ impl, I: 'static> Pallet { asset_id: p.asset_id, amount: p.value, }); + // Force the payment to none, so a fresh payment is sent during the next + // T::SpendPeriod. + p.payment_id = None; + PendingPayments::::set(key, Some(p)); }, PaymentStatus::Success => { - Proposals::::remove(key); + PendingPayments::::remove(key); Self::deposit_event(Event::ProposalPaymentSuccess { proposal_index: key, asset_id: p.asset_id, @@ -453,11 +775,111 @@ impl, I: 'static> Pallet { // We could trigger burning of funds in the spendFunds hook as well. T::SpendFunds::spend_funds(&mut total_weight, total_spent, missed_proposals); - Self::deposit_event(Event::Rollover { + Self::deposit_event(Event::RolloverPayments { rollover_proposals: missed_proposals, allocated_proposals: proposals_len.saturating_sub(missed_proposals), }); total_weight } + + /// Spend some money! returns number of approvals before spend. + pub fn spend_funds_local() -> Weight { + let mut total_weight = Weight::zero(); + + let mut budget_remaining = Self::pot(); + Self::deposit_event(Event::Spending { budget_remaining }); + let account_id = Self::account_id(); + + let mut missed_any = false; + let mut imbalance = >::zero(); + let proposals_len = Approvals::::mutate(|v| { + let proposals_approvals_len = v.len() as u32; + v.retain(|&index| { + // Should always be true, but shouldn't panic if false or we're screwed. + if let Some(p) = Self::proposals(index) { + if p.value <= budget_remaining { + budget_remaining -= p.value; + >::remove(index); + + // return their deposit. + let err_amount = T::Currency::unreserve(&p.proposer, p.bond); + debug_assert!(err_amount.is_zero()); + + // provide the allocation. + imbalance.subsume(T::Currency::deposit_creating(&p.beneficiary, p.value)); + + Self::deposit_event(Event::Awarded { + proposal_index: index, + award: p.value, + account: p.beneficiary, + }); + false + } else { + missed_any = true; + true + } + } else { + false + } + }); + proposals_approvals_len + }); + + total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); + + // Call Runtime hooks to external pallet using treasury to compute spend funds. + T::SpendFundsLocal::spend_funds( + &mut budget_remaining, + &mut imbalance, + &mut total_weight, + &mut missed_any, + ); + + if !missed_any { + // burn some proportion of the remaining budget if we run a surplus. + let burn = (T::Burn::get() * budget_remaining).min(budget_remaining); + budget_remaining -= burn; + + let (debit, credit) = T::Currency::pair(burn); + imbalance.subsume(debit); + T::BurnDestination::on_unbalanced(credit); + Self::deposit_event(Event::Burnt { burnt_funds: burn }) + } + + // Must never be an error, but better to be safe. + // proof: budget_remaining is account free balance minus ED; + // Thus we can't spend more than account free balance minus ED; + // Thus account is kept alive; qed; + if let Err(problem) = + T::Currency::settle(&account_id, imbalance, WithdrawReasons::TRANSFER, KeepAlive) + { + print("Inconsistent state - couldn't settle imbalance for funds spent by treasury"); + // Nothing else to do here. + drop(problem); + } + + Self::deposit_event(Event::Rollover { rollover_balance: budget_remaining }); + + total_weight + } + + /// Return the amount of money in the pot. + // The existential deposit is not part of the pot so treasury account never gets deleted. + pub fn pot() -> BalanceOf { + T::Currency::free_balance(&Self::account_id()) + // Must never be less than 0 but better be safe. + .saturating_sub(T::Currency::minimum_balance()) + } +} + +impl, I: 'static> OnUnbalanced> for Pallet { + fn on_nonzero_unbalanced(amount: NegativeImbalanceOf) { + let numeric_amount = amount.peek(); + + // Must resolve into existing but better to be safe. + let _ = T::Currency::resolve_creating(&Self::account_id(), amount); + + Self::deposit_event(Event::Deposit { value: numeric_amount }); + } } From 11cd988fc16b6791e8a2d4af40b4980437e515cf Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 09:28:52 +0000 Subject: [PATCH 03/60] clean up imports diffs --- frame/treasury/src/lib.rs | 84 ++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 49 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7d38241f6a5cc..ca7e4ff847ed5 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -64,6 +64,14 @@ mod tests; pub mod weights; use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; +use scale_info::TypeInfo; + +use sp_runtime::{ + traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, + Permill, RuntimeDebug, +}; +use sp_std::{collections::btree_map::BTreeMap, prelude::*}; + use frame_support::{ dispatch::fmt::Debug, print, @@ -76,59 +84,10 @@ use frame_support::{ weights::Weight, PalletId, }; -use scale_info::TypeInfo; -use sp_runtime::{ - traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, - Permill, RuntimeDebug, -}; -use sp_std::{collections::btree_map::BTreeMap, prelude::*}; pub use pallet::*; pub use weights::WeightInfo; -/// Status for making a payment via the `Pay::pay` trait function. -#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] -#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub enum PaymentStatus { - /// Payment is in progress. Nothing to report yet. - InProgress, - /// Payment status is unknowable. It will never be reported successful or failed. - Unknown, - /// Payment happened successfully. - Success, - /// Payment failed. It may safely be retried. - Failure, -} - -// TODO: Should be replaced with a version in frame_support -pub trait Pay { - /// The type by which we measure units of the currency in which we make payments. - type Balance: Balance; - /// The type by which we identify the individuals to whom a payment may be made. - type AccountId; - /// An identifier given to an individual payment. - type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; - type AssetId: AssetId; - /// Make a payment and return an identifier for later evaluation of success in some off-chain - /// mechanism (likely an event, but possibly not on this chain). - fn pay( - who: &Self::AccountId, - asset_id: Self::AssetId, - amount: Self::Balance, - ) -> Result; - /// Check how a payment has proceeded. `id` must have been a previously returned by `pay` for - /// the result of this call to be meaningful. - fn check_payment(id: Self::Id) -> PaymentStatus; - /// Ensure that a call to pay with the given parameters will be successful if done immediately - /// after this call. Used in benchmarking code. - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); - /// Ensure that a call to `check_payment` with the given parameters will return either `Success` - /// or `Failure`. - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(id: Self::Id); -} - pub type PayBalanceOf = <>::Paymaster as Pay>::Balance; pub type BalanceOf = <>::Currency as Currency<::AccountId>>::Balance; @@ -883,3 +842,30 @@ impl, I: 'static> OnUnbalanced> for Palle Self::deposit_event(Event::Deposit { value: numeric_amount }); } } + +// TODO: PaymentStatus struct and Pay trait be replaced with version in frame_support +// These are only left here temporarily until the Pay trait is merged into master. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum PaymentStatus { + InProgress, + Unknown, + Success, + Failure, +} +pub trait Pay { + type Balance: Balance; + type AccountId; + type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; + type AssetId: AssetId; + fn pay( + who: &Self::AccountId, + asset_id: Self::AssetId, + amount: Self::Balance, + ) -> Result; + fn check_payment(id: Self::Id) -> PaymentStatus; + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id); +} From 57aacbebe8112b88ae8e3299d5e68849010f1fb3 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 11:13:12 +0000 Subject: [PATCH 04/60] switch to using the pay trait in frame support --- frame/treasury/src/lib.rs | 48 ++++++++------------------------------- 1 file changed, 10 insertions(+), 38 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index ca7e4ff847ed5..68f3484e05c6c 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -63,7 +63,7 @@ mod benchmarking; mod tests; pub mod weights; -use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; +use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_runtime::{ @@ -73,10 +73,9 @@ use sp_runtime::{ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ - dispatch::fmt::Debug, print, traits::{ - tokens::{AssetId, Balance, BalanceConversion}, + tokens::{Balance, BalanceConversion, Pay, PaymentStatus}, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -188,18 +187,18 @@ pub mod pallet { type Balance: Balance; /// The identifier for what asset should be spent. - type AssetId: AssetId; + type AssetKind: AssetId; /// Means by which we can make payments to accounts. This also defines the currency and the /// balance which we use to denote that currency. type Paymaster: Pay< - AccountId = ::AccountId, - AssetId = Self::AssetId, + Beneficiary = ::AccountId, + AssetKind = Self::AssetKind, >; type BalanceConverter: BalanceConversion< PayBalanceOf, - Self::AssetId, + Self::AssetKind, Self::Balance, Error = Error, >; @@ -291,7 +290,7 @@ pub mod pallet { PendingPayment< T::AccountId, T::Balance, - T::AssetId, + T::AssetKind, PayBalanceOf, ::Id, >, @@ -376,13 +375,13 @@ pub mod pallet { /// The proposal was paid successfully ProposalPaymentSuccess { proposal_index: ProposalIndex, - asset_id: T::AssetId, + asset_id: T::AssetKind, amount: PayBalanceOf, }, // The proposal payment failed. Payment will be retried in next spend period. ProposalPaymentFailure { proposal_index: ProposalIndex, - asset_id: T::AssetId, + asset_id: T::AssetKind, amount: PayBalanceOf, }, } @@ -576,7 +575,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, - asset_id: T::AssetId, + asset_id: T::AssetKind, #[pallet::compact] amount: PayBalanceOf, beneficiary: AccountIdLookupOf, ) -> DispatchResult { @@ -842,30 +841,3 @@ impl, I: 'static> OnUnbalanced> for Palle Self::deposit_event(Event::Deposit { value: numeric_amount }); } } - -// TODO: PaymentStatus struct and Pay trait be replaced with version in frame_support -// These are only left here temporarily until the Pay trait is merged into master. -#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] -#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub enum PaymentStatus { - InProgress, - Unknown, - Success, - Failure, -} -pub trait Pay { - type Balance: Balance; - type AccountId; - type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; - type AssetId: AssetId; - fn pay( - who: &Self::AccountId, - asset_id: Self::AssetId, - amount: Self::Balance, - ) -> Result; - fn check_payment(id: Self::Id) -> PaymentStatus; - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(id: Self::Id); -} From 67b9f7a8dd52d52dfd29b44c38cb9f4e9e0eb28f Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 11:21:48 +0000 Subject: [PATCH 05/60] remove the changes to the build genesis config method --- frame/treasury/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 68f3484e05c6c..0b12fd157df23 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -334,7 +334,14 @@ pub mod pallet { #[pallet::genesis_build] impl, I: 'static> GenesisBuild for GenesisConfig { - fn build(&self) {} + fn build(&self) { + // Create Treasury account + let account_id = >::account_id(); + let min = T::Currency::minimum_balance(); + if T::Currency::free_balance(&account_id) < min { + let _ = T::Currency::make_free_balance_be(&account_id, min); + } + } } #[pallet::event] From 2b20924fdbeec2b5a7f086ddf1ff0ccba4687795 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 11:42:13 +0000 Subject: [PATCH 06/60] fix some type errors --- frame/treasury/src/lib.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 0b12fd157df23..7b9e4532cf400 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -75,7 +75,7 @@ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ print, traits::{ - tokens::{Balance, BalanceConversion, Pay, PaymentStatus}, + tokens::{AssetId, Balance, BalanceConversion, Pay, PaymentStatus}, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -96,7 +96,6 @@ pub type PositiveImbalanceOf = <>::Currency as Currenc pub type NegativeImbalanceOf = <>::Currency as Currency< ::AccountId, >>::NegativeImbalance; - type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; /// A trait to allow the Treasury Pallet to spend it's funds for other purposes. @@ -127,8 +126,6 @@ pub trait SpendFunds, I: 'static = ()> { /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; -/// A count of proposals. Just a `u32`. -pub type ProposalsCount = u32; /// A spending proposal. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] @@ -144,30 +141,26 @@ pub struct Proposal { bond: Balance, } -/// A spending proposal. +/// A treasury spend payment which has not yet succeeded. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] pub struct PendingPayment { - /// The account proposing it. - proposer: AccountId, + /// The account to whom the payment should be made if the proposal is accepted. + beneficiary: AccountId, /// The asset_id of the amount to be paid asset_id: AssetKind, /// The (total) amount that should be paid. value: AssetBalance, - /// The account to whom the payment should be made if the proposal is accepted. - beneficiary: AccountId, /// The amount to be paid, but normalized to the native asset class normalized_value: Balance, - // payment_status: PaymentStatus, + // the identifier for tracking the status of a payment which is in flight. payment_id: Option, } #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{ - dispatch_context::with_context, pallet_prelude::*, traits::tokens::AssetId, - }; + use frame_support::{dispatch_context::with_context, pallet_prelude::*}; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -365,7 +358,7 @@ pub mod pallet { ProcessingProposals { waiting_proposals: ProposalIndex }, /// Spending has finished; this is the number of proposals rolled over till next /// T::SpendPeriod. - RolloverPayments { rollover_proposals: ProposalsCount, allocated_proposals: ProposalsCount }, + RolloverPayments { rollover_proposals: ProposalIndex, allocated_proposals: ProposalIndex }, /// A new spend proposal has been approved. SpendApproved { proposal_index: ProposalIndex, @@ -612,7 +605,6 @@ pub mod pallet { let beneficiary = T::Lookup::lookup(beneficiary)?; let proposal = PendingPayment { - proposer: beneficiary.clone(), asset_id, value: amount, beneficiary: beneficiary.clone(), From 360284493f2e1cc1c0b332866fe9a391bfac758a Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 16:50:57 +0000 Subject: [PATCH 07/60] add a test for the new spend workflow --- Cargo.lock | 1 + frame/treasury/Cargo.toml | 2 + frame/treasury/src/lib.rs | 7 +- frame/treasury/src/tests.rs | 178 ++++++++++++++++++++++++++++++------ 4 files changed, 159 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d0aea7729bd2d..211cc07e14205 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6841,6 +6841,7 @@ dependencies = [ "frame-support", "frame-system", "impl-trait-for-tuples", + "pallet-assets", "pallet-balances", "pallet-utility", "parity-scale-codec", diff --git a/frame/treasury/Cargo.toml b/frame/treasury/Cargo.toml index 23fcb7944bfb7..bfa004fc5e9d6 100644 --- a/frame/treasury/Cargo.toml +++ b/frame/treasury/Cargo.toml @@ -24,6 +24,7 @@ frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional 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-balances = { version = "4.0.0-dev", default-features = false, path = "../balances" } +pallet-assets = { version = "4.0.0-dev", default-features = false, path = "../assets" } sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" } @@ -40,6 +41,7 @@ std = [ "frame-support/std", "frame-system/std", "pallet-balances/std", + "pallet-assets/std", "scale-info/std", "serde", "sp-runtime/std", diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7b9e4532cf400..1a54f70a22356 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -182,13 +182,14 @@ pub mod pallet { /// The identifier for what asset should be spent. type AssetKind: AssetId; - /// Means by which we can make payments to accounts. This also defines the currency and the - /// balance which we use to denote that currency. + /// The means by which we can make payments to beneficiaries. + /// This can be implmented over fungibles or some other means. type Paymaster: Pay< Beneficiary = ::AccountId, AssetKind = Self::AssetKind, >; + // THe means of knowing what is the equivalent native Balance of a given asset id Balance. type BalanceConverter: BalanceConversion< PayBalanceOf, Self::AssetKind, @@ -378,7 +379,7 @@ pub mod pallet { asset_id: T::AssetKind, amount: PayBalanceOf, }, - // The proposal payment failed. Payment will be retried in next spend period. + /// The proposal payment failed. Payment will be retried in next spend period. ProposalPaymentFailure { proposal_index: ProposalIndex, asset_id: T::AssetKind, diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 24d2d01f92f8a..4ba8977c1215d 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -19,19 +19,23 @@ #![cfg(test)] -use sp_core::H256; -use sp_runtime::{ - testing::Header, - traits::{BadOrigin, BlakeTwo256, Dispatchable, IdentityLookup}, -}; - use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, - pallet_prelude::GenesisBuild, + pallet_prelude::{GenesisBuild, PhantomData}, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{ + fungibles::{self, *}, + AsEnsureOriginWithArg, ConstU32, ConstU64, OnInitialize, + }, PalletId, }; +use frame_system::EnsureRoot; +use pallet_assets; +use sp_core::{TypedGet, H256}; +use sp_runtime::{ + testing::Header, + traits::{BadOrigin, BlakeTwo256, Dispatchable, IdentityLookup}, +}; use super::*; use crate as treasury; @@ -51,6 +55,7 @@ frame_support::construct_runtime!( Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, Treasury: treasury::{Pallet, Call, Storage, Config, Event}, Utility: pallet_utility, + Assets: pallet_assets::{Pallet, Call, Config, Storage, Event}, } ); @@ -80,6 +85,7 @@ impl frame_system::Config for Test { type OnSetCode = (); type MaxConsumers = ConstU32<16>; } + impl pallet_balances::Config for Test { type MaxLocks = (); type MaxReserves = (); @@ -99,10 +105,38 @@ impl pallet_utility::Config for Test { type WeightInfo = (); } +type AssetId = u32; +type BalanceU64 = u64; + +impl pallet_assets::Config for Test { + type RuntimeEvent = RuntimeEvent; + type Balance = BalanceU64; + type RemoveItemsLimit = ConstU32<1000>; + type AssetId = AssetId; + type AssetIdParameter = AssetId; + type Currency = Balances; + type CreateOrigin = AsEnsureOriginWithArg>; + type ForceOrigin = EnsureRoot; + type AssetDeposit = ConstU64<2>; + type AssetAccountDeposit = ConstU64<2>; + type MetadataDepositBase = ConstU64<0>; + type MetadataDepositPerByte = ConstU64<0>; + type ApprovalDeposit = ConstU64<0>; + type StringLimit = ConstU32<20>; + type Freezer = (); + type Extra = (); + type CallbackHandle = (); + type WeightInfo = (); + pallet_assets::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } +} + parameter_types! { pub const ProposalBond: Permill = Permill::from_percent(5); pub const Burn: Permill = Permill::from_percent(50); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); + pub TreasuryAccount: u128 = Treasury::account_id(); } pub struct TestSpendOrigin; impl frame_support::traits::EnsureOrigin for TestSpendOrigin { @@ -125,6 +159,10 @@ impl frame_support::traits::EnsureOrigin for TestSpendOrigin { impl Config for Test { type PalletId = TreasuryPalletId; + type Balance = BalanceU64; + type AssetKind = AssetId; + type Paymaster = AssetsPaymaster; + type BalanceConverter = DummyBalanceConverter; type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; @@ -138,8 +176,47 @@ impl Config for Test { type BurnDestination = (); // Just gets burned. type WeightInfo = (); type SpendFunds = (); + type SpendFundsLocal = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; + type SpendOriginLocal = TestSpendOrigin; +} + +pub struct AssetsPaymaster(sp_std::marker::PhantomData<(F, A)>); +impl + fungibles::Mutate> Pay + for AssetsPaymaster +{ + type Balance = F::Balance; + type Beneficiary = A::Type; + type AssetKind = F::AssetId; + type Id = (); + fn pay( + who: &Self::Beneficiary, + asset_id: Self::AssetKind, + amount: Self::Balance, + ) -> Result { + >::transfer(asset_id, &A::get(), who, amount, false) + .map_err(|_| ())?; + Ok(()) + } + fn check_payment(_: ()) -> PaymentStatus { + PaymentStatus::Success + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, amount: Self::Balance) {} + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} +} + +pub struct DummyBalanceConverter(PhantomData); +impl BalanceConversion for DummyBalanceConverter { + type Error = Error; + fn to_asset_balance( + _balance: BalanceU64, + _asset_id: AssetId, + ) -> Result { + Ok(0) + } } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -162,41 +239,64 @@ fn genesis_config_works() { }); } +#[test] +fn spend_local_origin_permissioning_works() { + new_test_ext().execute_with(|| { + assert_noop!(Treasury::spend_local(RuntimeOrigin::signed(1), 1, 1), BadOrigin); + assert_noop!( + Treasury::spend_local(RuntimeOrigin::signed(10), 6, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend_local(RuntimeOrigin::signed(11), 11, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend_local(RuntimeOrigin::signed(12), 21, 1), + Error::::InsufficientPermission + ); + assert_noop!( + Treasury::spend_local(RuntimeOrigin::signed(13), 51, 1), + Error::::InsufficientPermission + ); + }); +} + #[test] fn spend_origin_permissioning_works() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::spend(RuntimeOrigin::signed(1), 1, 1), BadOrigin); + assert_noop!(Treasury::spend(RuntimeOrigin::signed(1), 0, 1, 1), BadOrigin); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(10), 6, 1), + Treasury::spend(RuntimeOrigin::signed(10), 0, 6, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(11), 11, 1), + Treasury::spend(RuntimeOrigin::signed(11), 0, 11, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(12), 21, 1), + Treasury::spend(RuntimeOrigin::signed(12), 0, 21, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(13), 51, 1), + Treasury::spend(RuntimeOrigin::signed(13), 0, 51, 1), Error::::InsufficientPermission ); }); } #[test] -fn spend_origin_works() { +fn spend_local_origin_works() { new_test_ext().execute_with(|| { // Check that accumulate works when we have Some value in Dummy already. Balances::make_free_balance_be(&Treasury::account_id(), 101); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 10, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 20, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(13), 50, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(11), 10, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(12), 20, 6)); + assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(13), 50, 6)); >::on_initialize(1); assert_eq!(Balances::free_balance(6), 0); @@ -207,6 +307,32 @@ fn spend_origin_works() { }); } +#[test] +fn spend_origin_works() { + new_test_ext().execute_with(|| { + // Check that accumulate works when we have Some value in Dummy already. + assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, Treasury::account_id(), true, 1)); + assert_ok!(Assets::mint_into(0, &Treasury::account_id(), 100)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 0, 10, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 0, 20, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(13), 0, 50, 6)); + + // Treasury account should still have funds until next T::SpendPeriod + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 100); + assert_eq!(Assets::reducible_balance(0, &6, false), 0); + >::on_initialize(1); + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 100); + + >::on_initialize(2); + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 0); + assert_eq!(Assets::reducible_balance(0, &6, false), 100); + }); +} + #[test] fn minting_works() { new_test_ext().execute_with(|| { @@ -450,7 +576,7 @@ fn max_approvals_limited() { Balances::make_free_balance_be(&Treasury::account_id(), u64::MAX); Balances::make_free_balance_be(&0, u64::MAX); - for _ in 0..::MaxApprovals::get() { + for _ in 0..<::MaxApprovals as sp_core::TypedGet>::get() { assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)); assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0)); } @@ -488,8 +614,8 @@ fn spending_in_batch_respects_max_total() { // Respect the `max_total` for the given origin. assert_ok!(RuntimeCall::from(UtilityCall::batch_all { calls: vec![ - RuntimeCall::from(TreasuryCall::spend { amount: 2, beneficiary: 100 }), - RuntimeCall::from(TreasuryCall::spend { amount: 2, beneficiary: 101 }) + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 100 }), + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 101 }) ] }) .dispatch(RuntimeOrigin::signed(10))); @@ -497,8 +623,8 @@ fn spending_in_batch_respects_max_total() { assert_err_ignore_postinfo!( RuntimeCall::from(UtilityCall::batch_all { calls: vec![ - RuntimeCall::from(TreasuryCall::spend { amount: 2, beneficiary: 100 }), - RuntimeCall::from(TreasuryCall::spend { amount: 4, beneficiary: 101 }) + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 100 }), + RuntimeCall::from(TreasuryCall::spend_local { amount: 4, beneficiary: 101 }) ] }) .dispatch(RuntimeOrigin::signed(10)), From dd99d7c5f5fa0170d336a7a3ba4ab08b46b10a01 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 17:24:02 +0000 Subject: [PATCH 08/60] implement initial tests for the new spend workflow --- frame/treasury/src/lib.rs | 30 +++++++++++++----------------- frame/treasury/src/tests.rs | 5 ++--- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 1a54f70a22356..705937e95f20e 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -121,7 +121,7 @@ pub trait SpendFundsLocal, I: 'static = ()> { #[impl_trait_for_tuples::impl_for_tuples(30)] pub trait SpendFunds, I: 'static = ()> { - fn spend_funds(total_weight: &mut Weight, total_spent: T::Balance, total_missed: u32); + fn spend_funds(total_weight: &mut Weight, total_spent: BalanceOf, total_missed: u32); } /// An index of a proposal. Just a `u32`. @@ -193,8 +193,7 @@ pub mod pallet { type BalanceConverter: BalanceConversion< PayBalanceOf, Self::AssetKind, - Self::Balance, - Error = Error, + BalanceOf, >; /// The overarching event type. @@ -250,12 +249,7 @@ pub mod pallet { /// The origin required for approving spends from the treasury outside of the proposal /// process. The `Success` value is the maximum amount that this origin is allowed to /// spend at a time. - type SpendOriginLocal: EnsureOrigin>; - - /// The origin required for approving spends from the treasury outside of the proposal - /// process. The `Success` value is the maximum amount that this origin is allowed to - /// spend at a time. - type SpendOrigin: EnsureOrigin>; + type SpendOrigin: EnsureOrigin>; } /// Number of proposals that have been made. @@ -283,7 +277,7 @@ pub mod pallet { ProposalIndex, PendingPayment< T::AccountId, - T::Balance, + BalanceOf, T::AssetKind, PayBalanceOf, ::Id, @@ -528,7 +522,7 @@ pub mod pallet { #[pallet::compact] amount: BalanceOf, beneficiary: AccountIdLookupOf, ) -> DispatchResult { - let max_amount = T::SpendOriginLocal::ensure_origin(origin)?; + let max_amount = T::SpendOrigin::ensure_origin(origin)?; ensure!(amount <= max_amount, Error::::InsufficientPermission); with_context::>, _>(|v| { let context = v.or_default(); @@ -581,9 +575,11 @@ pub mod pallet { beneficiary: AccountIdLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; - ensure!(amount <= max_amount, Error::::InsufficientPermission); + let normalized_amount = T::BalanceConverter::to_asset_balance(amount, asset_id) + .map_err(|_| Error::::BalanceConversionFailed)?; + ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); - with_context::>, _>(|v| { + with_context::>, _>(|v| { let context = v.or_default(); // We group based on `max_amount`, to dinstinguish between different kind of @@ -593,10 +589,10 @@ pub mod pallet { let spend = context.spend_in_context.entry(max_amount).or_default(); // Ensure that we don't overflow nor use more than `max_amount` - if spend.checked_add(&amount).map(|s| s > max_amount).unwrap_or(true) { + if spend.checked_add(&normalized_amount).map(|s| s > max_amount).unwrap_or(true) { Err(Error::::InsufficientPermission) } else { - *spend = spend.saturating_add(amount); + *spend = spend.saturating_add(normalized_amount); Ok(()) } @@ -609,7 +605,7 @@ pub mod pallet { asset_id, value: amount, beneficiary: beneficiary.clone(), - normalized_value: T::BalanceConverter::to_asset_balance(amount, asset_id)?, + normalized_value: normalized_amount, payment_id: None, }; @@ -678,7 +674,7 @@ impl, I: 'static> Pallet { /// Spend some money! returns number of approvals before spend. pub fn spend_funds() -> Weight { let mut total_weight = Weight::zero(); - let mut total_spent = T::Balance::zero(); + let mut total_spent = BalanceOf::::zero(); let mut missed_proposals: u32 = 0; let proposals_len = PendingPayments::::count(); diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 4ba8977c1215d..1cb920b640e58 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -179,7 +179,6 @@ impl Config for Test { type SpendFundsLocal = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; - type SpendOriginLocal = TestSpendOrigin; } pub struct AssetsPaymaster(sp_std::marker::PhantomData<(F, A)>); @@ -212,10 +211,10 @@ pub struct DummyBalanceConverter(PhantomData); impl BalanceConversion for DummyBalanceConverter { type Error = Error; fn to_asset_balance( - _balance: BalanceU64, + balance: BalanceU64, _asset_id: AssetId, ) -> Result { - Ok(0) + Ok(balance) } } From 5f52015f35f25b9724767af11047528b8b683176 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 16 Mar 2023 18:22:28 +0000 Subject: [PATCH 09/60] remove unneeded Balance type from Confg --- bin/node/runtime/src/lib.rs | 3 +++ frame/treasury/src/lib.rs | 20 +++----------------- frame/treasury/src/tests.rs | 8 ++++---- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 9f4ad2ed26474..10577aa553fdb 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1091,6 +1091,9 @@ parameter_types! { impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; + type Balance = Balances; + type AssetKind = u32; + type Paymaster = PayFromAccount; type Currency = Balances; type ApproveOrigin = EitherOfDiverse< EnsureRoot, diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 705937e95f20e..48dc5fb4eaf07 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -75,7 +75,7 @@ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ print, traits::{ - tokens::{AssetId, Balance, BalanceConversion, Pay, PaymentStatus}, + tokens::{AssetId, BalanceConversion, Pay, PaymentStatus}, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -110,7 +110,7 @@ type AccountIdLookupOf = <::Lookup as StaticLookup /// * `missed_any`: If there were items that you want to spend on, but there were not enough funds, /// mark this value as `true`. This will prevent the treasury from burning the excess funds. #[impl_trait_for_tuples::impl_for_tuples(30)] -pub trait SpendFundsLocal, I: 'static = ()> { +pub trait SpendFunds, I: 'static = ()> { fn spend_funds( budget_remaining: &mut BalanceOf, imbalance: &mut PositiveImbalanceOf, @@ -119,11 +119,6 @@ pub trait SpendFundsLocal, I: 'static = ()> { ); } -#[impl_trait_for_tuples::impl_for_tuples(30)] -pub trait SpendFunds, I: 'static = ()> { - fn spend_funds(total_weight: &mut Weight, total_spent: BalanceOf, total_missed: u32); -} - /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; @@ -177,8 +172,6 @@ pub mod pallet { /// Origin from which rejections must come. type RejectOrigin: EnsureOrigin; - type Balance: Balance; - /// The identifier for what asset should be spent. type AssetKind: AssetId; @@ -237,9 +230,6 @@ pub mod pallet { /// Runtime hooks to external pallet using treasury to compute spend funds. type SpendFunds: SpendFunds; - /// Runtime hooks to external pallet using treasury to compute spend funds. - type SpendFundsLocal: SpendFundsLocal; - /// The maximum number of approvals that can wait in the spending queue. /// /// NOTE: This parameter is also used within the Bounties Pallet extension if enabled. @@ -725,10 +715,6 @@ impl, I: 'static> Pallet { total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); - // Call Runtime hooks to external pallet using treasury to compute spend funds. - // We could trigger burning of funds in the spendFunds hook as well. - T::SpendFunds::spend_funds(&mut total_weight, total_spent, missed_proposals); - Self::deposit_event(Event::RolloverPayments { rollover_proposals: missed_proposals, allocated_proposals: proposals_len.saturating_sub(missed_proposals), @@ -783,7 +769,7 @@ impl, I: 'static> Pallet { total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); // Call Runtime hooks to external pallet using treasury to compute spend funds. - T::SpendFundsLocal::spend_funds( + T::SpendFunds::spend_funds( &mut budget_remaining, &mut imbalance, &mut total_weight, diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 1cb920b640e58..dccae1d96cb46 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -159,7 +159,6 @@ impl frame_support::traits::EnsureOrigin for TestSpendOrigin { impl Config for Test { type PalletId = TreasuryPalletId; - type Balance = BalanceU64; type AssetKind = AssetId; type Paymaster = AssetsPaymaster; type BalanceConverter = DummyBalanceConverter; @@ -176,14 +175,15 @@ impl Config for Test { type BurnDestination = (); // Just gets burned. type WeightInfo = (); type SpendFunds = (); - type SpendFundsLocal = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; } pub struct AssetsPaymaster(sp_std::marker::PhantomData<(F, A)>); -impl + fungibles::Mutate> Pay - for AssetsPaymaster +impl< + A: TypedGet, + F: fungibles::Transfer + fungibles::Mutate + fungibles::Inspect, + > Pay for AssetsPaymaster { type Balance = F::Balance; type Beneficiary = A::Type; From 1925806d7eb830a2fbc06cdabe3841c2e1e4f0b1 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 17:31:34 +0100 Subject: [PATCH 10/60] Update lib.rs --- frame/treasury/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 48dc5fb4eaf07..82438b89d3017 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -137,6 +137,7 @@ pub struct Proposal { } /// A treasury spend payment which has not yet succeeded. +/// When a `PendingPayment` is verified to be successful, it is deleted from storage. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] pub struct PendingPayment { From f94e20a8f17efb4698f4cacf18291c41a336376c Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 17:25:09 +0000 Subject: [PATCH 11/60] introduce a unit implementation for BalanceConversion which behaves as an identity function or transformation --- bin/node/runtime/src/lib.rs | 6 ++--- frame/support/src/traits/tokens.rs | 2 +- frame/support/src/traits/tokens/misc.rs | 9 +++++++ frame/support/src/traits/tokens/pay.rs | 30 +++++++++++++++++++++- frame/treasury/src/tests.rs | 33 +++---------------------- 5 files changed, 45 insertions(+), 35 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 10577aa553fdb..7a9900f69ecd6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -33,7 +33,7 @@ use frame_support::{ parameter_types, traits::{ fungible::ItemOf, - tokens::{nonfungibles_v2::Inspect, GetSalary, PayFromAccount}, + tokens::{nonfungibles_v2::Inspect, GetSalary, PayFromAccount, PayFungibles}, AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, WithdrawReasons, @@ -1091,9 +1091,9 @@ parameter_types! { impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; - type Balance = Balances; type AssetKind = u32; - type Paymaster = PayFromAccount; + type Paymaster = PayFungibles; + type BalanceConverter = (); type Currency = Balances; type ApproveOrigin = EitherOfDiverse< EnsureRoot, diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index 2d2bd63ada551..f90569b21662d 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -32,4 +32,4 @@ pub use misc::{ AssetId, Balance, BalanceConversion, BalanceStatus, ConvertRank, DepositConsequence, ExistenceRequirement, GetSalary, Locker, WithdrawConsequence, WithdrawReasons, }; -pub use pay::{Pay, PayFromAccount, PaymentStatus}; +pub use pay::{Pay, PayFromAccount, PayFungibles, PaymentStatus}; diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 6113642c83460..3aaab4ed4d6cf 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -195,6 +195,15 @@ pub trait BalanceConversion { fn to_asset_balance(balance: InBalance, asset_id: AssetId) -> Result; } +impl> + BalanceConversion for () +{ + type Error = (); + fn to_asset_balance(balance: InBalance, _asset_id: AssetId) -> Result { + Ok(balance.into()) + } +} + /// Trait to handle asset locking mechanism to ensure interactions with the asset can be implemented /// downstream to extend logic of Uniques current functionality. pub trait Locker { diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 1c6a147b5f22e..ad8e20a2602de 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -22,7 +22,7 @@ use scale_info::TypeInfo; use sp_core::{RuntimeDebug, TypedGet}; use sp_std::fmt::Debug; -use super::{fungible, Balance}; +use super::{fungible, fungibles, Balance}; /// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with /// XCM/MultiAsset and made generic over assets. @@ -101,3 +101,31 @@ impl + fungible::Mutate> Pa #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} } + +pub struct PayFungibles(sp_std::marker::PhantomData<(F, A)>); +impl< + A: TypedGet, + F: fungibles::Transfer + fungibles::Mutate + fungibles::Inspect, + > Pay for PayFungibles +{ + type Balance = F::Balance; + type Beneficiary = A::Type; + type AssetKind = F::AssetId; + type Id = (); + fn pay( + who: &Self::Beneficiary, + asset_id: Self::AssetKind, + amount: Self::Balance, + ) -> Result { + >::transfer(asset_id, &A::get(), who, amount, false) + .map_err(|_| ())?; + Ok(()) + } + fn check_payment(_: ()) -> PaymentStatus { + PaymentStatus::Success + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, amount: Self::Balance) {} + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} +} diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index dccae1d96cb46..7d60b0f2219b1 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -25,6 +25,7 @@ use frame_support::{ parameter_types, traits::{ fungibles::{self, *}, + tokens::pay::PayFungibles, AsEnsureOriginWithArg, ConstU32, ConstU64, OnInitialize, }, PalletId, @@ -160,8 +161,8 @@ impl frame_support::traits::EnsureOrigin for TestSpendOrigin { impl Config for Test { type PalletId = TreasuryPalletId; type AssetKind = AssetId; - type Paymaster = AssetsPaymaster; - type BalanceConverter = DummyBalanceConverter; + type Paymaster = PayFungibles; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; @@ -179,34 +180,6 @@ impl Config for Test { type SpendOrigin = TestSpendOrigin; } -pub struct AssetsPaymaster(sp_std::marker::PhantomData<(F, A)>); -impl< - A: TypedGet, - F: fungibles::Transfer + fungibles::Mutate + fungibles::Inspect, - > Pay for AssetsPaymaster -{ - type Balance = F::Balance; - type Beneficiary = A::Type; - type AssetKind = F::AssetId; - type Id = (); - fn pay( - who: &Self::Beneficiary, - asset_id: Self::AssetKind, - amount: Self::Balance, - ) -> Result { - >::transfer(asset_id, &A::get(), who, amount, false) - .map_err(|_| ())?; - Ok(()) - } - fn check_payment(_: ()) -> PaymentStatus { - PaymentStatus::Success - } - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: &Self::Beneficiary, amount: Self::Balance) {} - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(_: Self::Id) {} -} - pub struct DummyBalanceConverter(PhantomData); impl BalanceConversion for DummyBalanceConverter { type Error = Error; From 3aaed66f29fc1c1a6d0f8f7d438239a993e39220 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 17:50:47 +0000 Subject: [PATCH 12/60] fix rebase introduced error --- frame/support/src/traits/tokens/pay.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 4575f22322075..ec147b0d0173d 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -22,7 +22,7 @@ use scale_info::TypeInfo; use sp_core::{RuntimeDebug, TypedGet}; use sp_std::fmt::Debug; -use super::{fungible, Balance, Preservation::Expendable}; +use super::{fungible, fungibles, Balance, Preservation::Expendable}; /// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with /// XCM/MultiAsset and made generic over assets. @@ -101,10 +101,8 @@ impl> Pay for PayFromAccount { } pub struct PayFungibles(sp_std::marker::PhantomData<(F, A)>); -impl< - A: TypedGet, - F: fungibles::Transfer + fungibles::Mutate + fungibles::Inspect, - > Pay for PayFungibles +impl + fungibles::Inspect> Pay + for PayFungibles { type Balance = F::Balance; type Beneficiary = A::Type; @@ -115,7 +113,7 @@ impl< asset_id: Self::AssetKind, amount: Self::Balance, ) -> Result { - >::transfer(asset_id, &A::get(), who, amount, false) + >::transfer(asset_id, &A::get(), who, amount, Expendable) .map_err(|_| ())?; Ok(()) } From a33d4dc558a01941a271694793ed32e61a37773e Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 18:14:51 +0000 Subject: [PATCH 13/60] fix broken tests due to rebase --- frame/treasury/src/tests.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 60fac113c7f08..c7227f6736aff 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -24,15 +24,15 @@ use frame_support::{ pallet_prelude::{GenesisBuild, PhantomData}, parameter_types, traits::{ - fungibles::{self, *}, - tokens::pay::PayFungibles, + fungibles::*, + tokens::{pay::PayFungibles, Fortitude::Polite, Preservation::Expendable}, AsEnsureOriginWithArg, ConstU32, ConstU64, OnInitialize, }, PalletId, }; use frame_system::EnsureRoot; use pallet_assets; -use sp_core::{TypedGet, H256}; +use sp_core::H256; use sp_runtime::{ testing::Header, traits::{BadOrigin, BlakeTwo256, Dispatchable, IdentityLookup}, @@ -298,14 +298,14 @@ fn spend_origin_works() { assert_ok!(Treasury::spend(RuntimeOrigin::signed(13), 0, 50, 6)); // Treasury account should still have funds until next T::SpendPeriod - assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 100); - assert_eq!(Assets::reducible_balance(0, &6, false), 0); + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), Expendable, Polite), 100); + assert_eq!(Assets::reducible_balance(0, &6, Expendable, Polite), 0); >::on_initialize(1); - assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 100); + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), Expendable, Polite), 100); >::on_initialize(2); - assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), false), 0); - assert_eq!(Assets::reducible_balance(0, &6, false), 100); + assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), Expendable, Polite), 0); + assert_eq!(Assets::reducible_balance(0, &6, Expendable, Polite), 100); }); } From 414e8e24ae98a2d356eb2b51b77b4bbc22382004 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 18:55:49 +0000 Subject: [PATCH 14/60] fix tests in tips and bounties --- frame/bounties/src/tests.rs | 10 +++++++++- frame/tips/src/tests.rs | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index ef3da7564874e..9733483888711 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -26,7 +26,7 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{tokens::PayFromAccount, ConstU32, ConstU64, OnInitialize}, PalletId, }; @@ -59,6 +59,8 @@ frame_support::construct_runtime!( parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); + pub TreasuryAccount: u128 = Treasury::account_id(); + pub Treasury1Account: u128 = Treasury1::account_id(); } type Balance = u64; @@ -116,6 +118,9 @@ parameter_types! { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; + type AssetKind = (); + type Paymaster = PayFromAccount; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; @@ -135,6 +140,9 @@ impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId2; + type AssetKind = (); + type Paymaster = PayFromAccount; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index b2d97de18312f..a25a67a45b2ac 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -32,7 +32,7 @@ use frame_support::{ pallet_prelude::GenesisBuild, parameter_types, storage::StoragePrefixedMap, - traits::{ConstU32, ConstU64, SortedMembers, StorageVersion}, + traits::{tokens::PayFromAccount, ConstU32, ConstU64, SortedMembers, StorageVersion}, PalletId, }; @@ -59,6 +59,8 @@ frame_support::construct_runtime!( parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); + pub TreasuryAccount: u128 = Treasury::account_id(); + pub Treasury1Account: u128 = Treasury1::account_id(); } impl frame_system::Config for Test { type BaseCallFilter = frame_support::traits::Everything; @@ -134,6 +136,9 @@ parameter_types! { } impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; + type AssetKind = (); + type Paymaster = PayFromAccount; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; @@ -153,6 +158,9 @@ impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId2; + type AssetKind = (); + type Paymaster = PayFromAccount; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; From 2f60a1b6d5663ae9bbef250b4e4f5629e6862a2c Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 19:02:27 +0000 Subject: [PATCH 15/60] fix lint error for unused fn argument in Pay trait --- frame/support/src/traits/tokens/pay.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index ec147b0d0173d..a21c5575c7958 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -121,7 +121,7 @@ impl + fungibles::Inspect> P PaymentStatus::Success } #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: &Self::Beneficiary, amount: Self::Balance) {} + fn ensure_successful(_: &Self::Beneficiary, _: Self::Balance) {} #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} } From 182c7336619aa282f546d87720c3481dc40b499e Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 19:15:00 +0000 Subject: [PATCH 16/60] fix tests in child-bounties pallet --- frame/child-bounties/src/tests.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index a936312aec868..ed779f51aa5ce 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -26,11 +26,10 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{tokens::PayFromAccount, ConstU32, ConstU64, OnInitialize}, weights::Weight, PalletId, }; - use sp_core::H256; use sp_runtime::{ testing::Header, @@ -113,10 +112,14 @@ parameter_types! { pub const Burn: Permill = Permill::from_percent(50); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const SpendLimit: Balance = u64::MAX; + pub TreasuryAccount: u128 = Treasury::account_id(); } impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; + type AssetKind = (); + type Paymaster = PayFromAccount; + type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; type RejectOrigin = frame_system::EnsureRoot; From 0916aff3f7131d8916563c9338909bb6f4de748b Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 22 Mar 2023 19:20:22 +0000 Subject: [PATCH 17/60] remove unneeded DummyBalanceConverter --- frame/treasury/src/tests.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index c7227f6736aff..1b271f19d8b33 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -184,17 +184,6 @@ impl Config for Test { type SpendOrigin = TestSpendOrigin; } -pub struct DummyBalanceConverter(PhantomData); -impl BalanceConversion for DummyBalanceConverter { - type Error = Error; - fn to_asset_balance( - balance: BalanceU64, - _asset_id: AssetId, - ) -> Result { - Ok(balance) - } -} - pub fn new_test_ext() -> sp_io::TestExternalities { let mut t = frame_system::GenesisConfig::default().build_storage::().unwrap(); pallet_balances::GenesisConfig:: { From fdcd66a1d2685ad0644cc72f25356249d840dd78 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 23 Mar 2023 20:34:53 +0100 Subject: [PATCH 18/60] Update frame/bounties/src/tests.rs Co-authored-by: Muharem Ismailov --- frame/bounties/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 9733483888711..2b6f1646bbad7 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -60,7 +60,7 @@ frame_support::construct_runtime!( parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); pub TreasuryAccount: u128 = Treasury::account_id(); - pub Treasury1Account: u128 = Treasury1::account_id(); + pub TreasuryAccount1: u128 = Treasury1::account_id(); } type Balance = u64; From 5673e79d5ae9423b16de962e1318193c8c976c73 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 23 Mar 2023 20:35:03 +0100 Subject: [PATCH 19/60] Update frame/bounties/src/tests.rs Co-authored-by: Muharem Ismailov --- frame/bounties/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 2b6f1646bbad7..a04cd0f0128bb 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -141,7 +141,7 @@ impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId2; type AssetKind = (); - type Paymaster = PayFromAccount; + type Paymaster = PayFromAccount; type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; From c6f9c5a109e4ad082cf5273ee2e5a655c1964cf4 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 23 Mar 2023 20:35:16 +0100 Subject: [PATCH 20/60] Update frame/treasury/src/lib.rs Co-authored-by: Muharem Ismailov --- frame/treasury/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 82438b89d3017..1895ccdd06a64 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -149,7 +149,7 @@ pub struct PendingPayment, } From bb372a25438434ae993a1de5c7c307da501c7672 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 11:12:09 +0100 Subject: [PATCH 21/60] tiny cleanups --- frame/treasury/src/lib.rs | 2 +- frame/treasury/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 82438b89d3017..00f6c74a01bc3 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -179,7 +179,7 @@ pub mod pallet { /// The means by which we can make payments to beneficiaries. /// This can be implmented over fungibles or some other means. type Paymaster: Pay< - Beneficiary = ::AccountId, + Beneficiary = Self::AccountId, AssetKind = Self::AssetKind, >; diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 1b271f19d8b33..c561809a3cea0 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -21,7 +21,7 @@ use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, - pallet_prelude::{GenesisBuild, PhantomData}, + pallet_prelude::GenesisBuild, parameter_types, traits::{ fungibles::*, From a56897d3b1c5b9dba34d4dfb913e6aec02e981f6 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 11:33:01 +0100 Subject: [PATCH 22/60] rename asset_id to asset_kind and proposal --- frame/treasury/src/lib.rs | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 00f6c74a01bc3..5d3e5721c1258 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -143,8 +143,8 @@ pub struct Proposal { pub struct PendingPayment { /// The account to whom the payment should be made if the proposal is accepted. beneficiary: AccountId, - /// The asset_id of the amount to be paid - asset_id: AssetKind, + /// The asset_kind of the amount to be paid + asset_kind: AssetKind, /// The (total) amount that should be paid. value: AssetBalance, /// The amount to be paid, but normalized to the native asset class @@ -178,10 +178,7 @@ pub mod pallet { /// The means by which we can make payments to beneficiaries. /// This can be implmented over fungibles or some other means. - type Paymaster: Pay< - Beneficiary = Self::AccountId, - AssetKind = Self::AssetKind, - >; + type Paymaster: Pay; // THe means of knowing what is the equivalent native Balance of a given asset id Balance. type BalanceConverter: BalanceConversion< @@ -361,13 +358,13 @@ pub mod pallet { /// The proposal was paid successfully ProposalPaymentSuccess { proposal_index: ProposalIndex, - asset_id: T::AssetKind, + asset_kind: T::AssetKind, amount: PayBalanceOf, }, /// The proposal payment failed. Payment will be retried in next spend period. ProposalPaymentFailure { proposal_index: ProposalIndex, - asset_id: T::AssetKind, + asset_kind: T::AssetKind, amount: PayBalanceOf, }, } @@ -561,12 +558,12 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, - asset_id: T::AssetKind, + asset_kind: T::AssetKind, #[pallet::compact] amount: PayBalanceOf, beneficiary: AccountIdLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; - let normalized_amount = T::BalanceConverter::to_asset_balance(amount, asset_id) + let normalized_amount = T::BalanceConverter::to_asset_balance(amount, asset_kind) .map_err(|_| Error::::BalanceConversionFailed)?; ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); @@ -592,18 +589,22 @@ pub mod pallet { let beneficiary = T::Lookup::lookup(beneficiary)?; - let proposal = PendingPayment { - asset_id, + let pending_payment = PendingPayment { + asset_kind, value: amount, beneficiary: beneficiary.clone(), normalized_value: normalized_amount, payment_id: None, }; - let proposal_index = PendingPayments::::count(); - PendingPayments::::insert(proposal_index, proposal); + let next_index = PendingPayments::::count(); + PendingPayments::::insert(next_index, pending_payment); - Self::deposit_event(Event::QueuedPayment { proposal_index, amount, beneficiary }); + Self::deposit_event(Event::QueuedPayment { + proposal_index: next_index, + amount, + beneficiary, + }); Ok(()) } @@ -675,7 +676,7 @@ impl, I: 'static> Pallet { if let Some(mut p) = PendingPayments::::get(key) { match p.payment_id { None => - if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_id, p.value) { + if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { total_spent += p.normalized_value; p.payment_id = Some(id); PendingPayments::::set(key, Some(p)); @@ -688,7 +689,7 @@ impl, I: 'static> Pallet { missed_proposals = missed_proposals.saturating_add(1); Self::deposit_event(Event::ProposalPaymentFailure { proposal_index: key, - asset_id: p.asset_id, + asset_kind: p.asset_kind, amount: p.value, }); // Force the payment to none, so a fresh payment is sent during the next @@ -700,7 +701,7 @@ impl, I: 'static> Pallet { PendingPayments::::remove(key); Self::deposit_event(Event::ProposalPaymentSuccess { proposal_index: key, - asset_id: p.asset_id, + asset_kind: p.asset_kind, amount: p.value, }); }, From e07ee86dde7250103f6fd3d7695e5883469dcea9 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 13:19:27 +0200 Subject: [PATCH 23/60] Update frame/treasury/src/lib.rs Co-authored-by: Muharem Ismailov --- frame/treasury/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index cdddbd062cf4e..4b46cf1999986 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -548,7 +548,7 @@ pub mod pallet { /// Propose and approve a spend of treasury funds. /// - /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. + /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount`. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The destination account for the transfer. /// From 304f5361949df801b2f3e26df834015c24e2815a Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 12:35:28 +0100 Subject: [PATCH 24/60] implememt PR feedback --- frame/treasury/src/lib.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index cdddbd062cf4e..64b2fad9fbb25 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -256,9 +256,8 @@ pub mod pallet { OptionQuery, >; - /// Proposals that have been made. + /// PendingPayments that have been made. #[pallet::storage] - #[pallet::getter(fn pending_payments)] pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, Twox64Concat, @@ -348,13 +347,21 @@ pub mod pallet { amount: BalanceOf, beneficiary: T::AccountId, }, + /// The inactive funds of the pallet have been updated. + UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, + /// The payment has been queued to be paid out at the next Spend Period QueuedPayment { proposal_index: ProposalIndex, + asset_kind: T::AssetKind, amount: PayBalanceOf, beneficiary: T::AccountId, }, - /// The inactive funds of the pallet have been updated. - UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, + /// The has been processed but awaiting payment status. + TriggerPayment { + proposal_index: ProposalIndex, + asset_kind: T::AssetKind, + amount: PayBalanceOf, + }, /// The proposal was paid successfully ProposalPaymentSuccess { proposal_index: ProposalIndex, @@ -549,6 +556,7 @@ pub mod pallet { /// Propose and approve a spend of treasury funds. /// /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. + /// - `asset_kind`: An indicator of the specific asset class which should be spent /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The destination account for the transfer. /// @@ -602,6 +610,7 @@ pub mod pallet { Self::deposit_event(Event::QueuedPayment { proposal_index: next_index, + asset_kind, amount, beneficiary, }); @@ -679,6 +688,11 @@ impl, I: 'static> Pallet { if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { total_spent += p.normalized_value; p.payment_id = Some(id); + Self::deposit_event(Event::TriggerPayment { + proposal_index: key, + asset_kind: p.asset_kind, + amount: p.value, + }); PendingPayments::::set(key, Some(p)); } else { missed_proposals = missed_proposals.saturating_add(1); From e7ecdbfe04537f5e8481d99c49cc406d36b810e3 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 12:37:11 +0100 Subject: [PATCH 25/60] update doc comments --- frame/treasury/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 64b2fad9fbb25..641ba427217f9 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -502,7 +502,8 @@ pub mod pallet { Ok(()) } - /// Propose and approve a spend of treasury funds. + /// Propose and approve a spend of treasury funds. This is a legacy extrinsic which might be + /// removed in the future. /// /// - `origin`: Must be `SpendOrigin` with the `Success` value being at least `amount`. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. @@ -559,9 +560,6 @@ pub mod pallet { /// - `asset_kind`: An indicator of the specific asset class which should be spent /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The destination account for the transfer. - /// - /// NOTE: For record-keeping purposes, the proposer is deemed to be equivalent to the - /// beneficiary. #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::spend())] pub fn spend( From 3cd5760af652639b2726942d5fe523beafb2f193 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 12:48:14 +0100 Subject: [PATCH 26/60] include correct weight funchtion --- frame/treasury/src/lib.rs | 2 +- frame/treasury/src/weights.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 380dbdde57c60..fbc7f449497b7 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -512,7 +512,7 @@ pub mod pallet { /// NOTE: For record-keeping purposes, the proposer is deemed to be equivalent to the /// beneficiary. #[pallet::call_index(3)] - #[pallet::weight(T::WeightInfo::spend())] + #[pallet::weight(T::WeightInfo::spend_local())] pub fn spend_local( origin: OriginFor, #[pallet::compact] amount: BalanceOf, diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index a7c093a8001ec..50c1298275e60 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -62,6 +62,17 @@ pub trait WeightInfo { /// Weights for pallet_treasury using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { + /// Storage: Treasury PendingPayments (r:0 w:1) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + fn spend_local() -> Weight { + // Proof Size summary in bytes: + // Measured: `42` + // Estimated: `3376` + // Minimum execution time: 17_010_000 picoseconds. + Weight::from_parts(17_247_000, 3376) + .saturating_add(T::DbWeight::get().reads(2_u64)) + .saturating_add(T::DbWeight::get().writes(3_u64)) + } /// Storage: Treasury ProposalCount (r:1 w:1) /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) From d97886d3123736456dbb5299259b0c2ffc89fca3 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 13:03:08 +0100 Subject: [PATCH 27/60] add spend_local to weight.rs --- frame/treasury/src/weights.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index 50c1298275e60..88ed906a317c2 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -51,6 +51,7 @@ use sp_std::marker::PhantomData; /// Weight functions needed for pallet_treasury. pub trait WeightInfo { + fn spend_local() -> Weight; fn spend() -> Weight; fn propose_spend() -> Weight; fn reject_proposal() -> Weight; @@ -170,6 +171,17 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { + /// Storage: Treasury Proposals (r:0 w:1) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + fn spend_local() -> Weight { + // Proof Size summary in bytes: + // Measured: `42` + // Estimated: `3376` + // Minimum execution time: 17_010_000 picoseconds. + Weight::from_parts(17_247_000, 3376) + .saturating_add(RocksDbWeight::get().reads(2_u64)) + .saturating_add(RocksDbWeight::get().writes(3_u64)) + } /// Storage: Treasury ProposalCount (r:1 w:1) /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) From 805d595906045277358f256c393642240bbdae83 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 27 Mar 2023 13:54:29 +0100 Subject: [PATCH 28/60] benchmarking updates --- frame/treasury/src/benchmarking.rs | 17 +++++++++++ frame/treasury/src/lib.rs | 2 +- frame/treasury/src/weights.rs | 46 ++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index a3761083e4faa..4f136fa7eb4ef 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -55,6 +55,16 @@ fn create_approved_proposals, I: 'static>(n: u32) -> Result<(), &'s Ok(()) } +// Create pending payments that are approved for use in `on_initialize`. +fn create_pending_payments, I: 'static>(n: u32) -> Result<(), &'static str> { + for i in 0..n { + let (caller, value, lookup) = setup_proposal::(i); + Treasury::::spend(RawOrigin::Signed(caller).into(), 0, value, lookup)?; + } + ensure!(>::count() == n as usize, "Not all approved"); + Ok(()) +} + fn setup_pot_account, I: 'static>() { let pot_account = Treasury::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); @@ -138,5 +148,12 @@ benchmarks_instance_pallet! { Treasury::::on_initialize(T::BlockNumber::zero()); } + on_initialize_pending_payments { + let p in 0 .. T::MaxApprovals::get(); + create_pending_payments::(p)?; + }: { + Treasury::::on_initialize(T::BlockNumber::zero()); + } + impl_benchmark_test_suite!(Treasury, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index fbc7f449497b7..6b9ae187f8fd7 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -727,7 +727,7 @@ impl, I: 'static> Pallet { } } - total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); + total_weight += T::WeightInfo::on_initialize_pending_payments(proposals_len); Self::deposit_event(Event::RolloverPayments { rollover_proposals: missed_proposals, diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index 88ed906a317c2..aee2d0a8376d6 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -58,6 +58,7 @@ pub trait WeightInfo { fn approve_proposal(p: u32, ) -> Weight; fn remove_approval() -> Weight; fn on_initialize_proposals(p: u32, ) -> Weight; + fn on_initialize_pending_payments(p: u32, ) -> Weight; } /// Weights for pallet_treasury using the Substrate node and recommended hardware. @@ -167,6 +168,31 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().writes((3_u64).saturating_mul(p.into()))) .saturating_add(Weight::from_parts(0, 7789).saturating_mul(p.into())) } + /// Storage: Treasury Deactivated (r:1 w:1) + /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) + /// Storage: Treasury Approvals (r:1 w:1) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Storage: Treasury Proposals (r:100 w:100) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Storage: System Account (r:200 w:200) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + /// Storage: Bounties BountyApprovals (r:1 w:1) + /// Proof: Bounties BountyApprovals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// The range of component `p` is `[0, 100]`. + fn on_initialize_pending_payments(p: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `387 + p * (251 ±0)` + // Estimated: `7255 + p * (7789 ±0)` + // Minimum execution time: 43_781_000 picoseconds. + Weight::from_parts(68_521_487, 7255) + // Standard Error: 58_804 + .saturating_add(Weight::from_parts(33_271_211, 0).saturating_mul(p.into())) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().reads((3_u64).saturating_mul(p.into()))) + .saturating_add(T::DbWeight::get().writes(3_u64)) + .saturating_add(T::DbWeight::get().writes((3_u64).saturating_mul(p.into()))) + .saturating_add(Weight::from_parts(0, 7789).saturating_mul(p.into())) + } } // For backwards compatibility and tests @@ -275,4 +301,24 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().writes((3_u64).saturating_mul(p.into()))) .saturating_add(Weight::from_parts(0, 7789).saturating_mul(p.into())) } + /// Storage: Treasury Deactivated (r:1 w:1) + /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) + /// Storage: Treasury PendingPayments (r:100 w:100) + /// Proof: Treasury PendingPayments (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Storage: System Account (r:200 w:200) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + /// The range of component `p` is `[0, 100]`. + fn on_initialize_pending_payments(p: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `387 + p * (251 ±0)` + // Estimated: `7255 + p * (7789 ±0)` + // Minimum execution time: 43_781_000 picoseconds. + Weight::from_parts(68_521_487, 7255) + // Standard Error: 58_804 + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().reads((3_u64).saturating_mul(p.into()))) + .saturating_add(RocksDbWeight::get().writes(3_u64)) + .saturating_add(RocksDbWeight::get().writes((3_u64).saturating_mul(p.into()))) + .saturating_add(Weight::from_parts(0, 7789).saturating_mul(p.into())) + } } From bbf83fa771887ed9c1c3eff1ab68724542461b06 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 30 Mar 2023 14:15:46 +0100 Subject: [PATCH 29/60] rename Treasury1Account to TreasuryAccount1 --- frame/tips/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index a25a67a45b2ac..394296c5eb2d9 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -60,7 +60,7 @@ frame_support::construct_runtime!( parameter_types! { pub const AvailableBlockRatio: Perbill = Perbill::one(); pub TreasuryAccount: u128 = Treasury::account_id(); - pub Treasury1Account: u128 = Treasury1::account_id(); + pub TreasuryAccount1: u128 = Treasury1::account_id(); } impl frame_system::Config for Test { type BaseCallFilter = frame_support::traits::Everything; @@ -159,7 +159,7 @@ impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId2; type AssetKind = (); - type Paymaster = PayFromAccount; + type Paymaster = PayFromAccount; type BalanceConverter = (); type Currency = pallet_balances::Pallet; type ApproveOrigin = frame_system::EnsureRoot; From 8aa1c3c82faf9834359f89f6652994138e5d7912 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 30 Mar 2023 14:19:09 +0100 Subject: [PATCH 30/60] update PendingPayment storage comment --- frame/treasury/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 6b9ae187f8fd7..067f6724ad8a7 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -136,7 +136,7 @@ pub struct Proposal { bond: Balance, } -/// A treasury spend payment which has not yet succeeded. +/// PendingPayment represents treasury spend payment which has not yet succeeded. /// When a `PendingPayment` is verified to be successful, it is deleted from storage. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] @@ -256,7 +256,7 @@ pub mod pallet { OptionQuery, >; - /// PendingPayments that have been made. + /// PendingPayments that have not yet processed or are not yet successful. #[pallet::storage] pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, From aabfadfd99033e3b76a90350ef640c6b6c9d5c37 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 30 Mar 2023 14:20:33 +0100 Subject: [PATCH 31/60] update PendingPayment storage comment --- frame/treasury/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 067f6724ad8a7..92c62432a4a5b 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -137,7 +137,6 @@ pub struct Proposal { } /// PendingPayment represents treasury spend payment which has not yet succeeded. -/// When a `PendingPayment` is verified to be successful, it is deleted from storage. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] pub struct PendingPayment { @@ -257,6 +256,7 @@ pub mod pallet { >; /// PendingPayments that have not yet processed or are not yet successful. + /// When a `PendingPayment` is verified to be successful, it is deleted from storage. #[pallet::storage] pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, From 592b81b0998ae40422f1a82cbcc43a37e4cf5ad6 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 30 Mar 2023 14:38:46 +0100 Subject: [PATCH 32/60] adjust naming of Events to not mix PendingPayment and Proposal concepts --- frame/treasury/src/lib.rs | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 92c62432a4a5b..a7b9131552fb6 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -122,6 +122,9 @@ pub trait SpendFunds, I: 'static = ()> { /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; +/// An index of a pending payment. Just a `u32`. +pub type PendingPaymentIndex = u32; + /// A spending proposal. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] @@ -261,7 +264,7 @@ pub mod pallet { pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, Twox64Concat, - ProposalIndex, + PendingPaymentIndex, PendingPayment< T::AccountId, BalanceOf, @@ -350,27 +353,27 @@ pub mod pallet { /// The inactive funds of the pallet have been updated. UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, /// The payment has been queued to be paid out at the next Spend Period - QueuedPayment { - proposal_index: ProposalIndex, + PaymentQueued { + payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, beneficiary: T::AccountId, }, /// The has been processed but awaiting payment status. - TriggerPayment { - proposal_index: ProposalIndex, + PaymentTriggered { + payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, }, /// The proposal was paid successfully - ProposalPaymentSuccess { - proposal_index: ProposalIndex, + PaymentSuccess { + payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, }, /// The proposal payment failed. Payment will be retried in next spend period. - ProposalPaymentFailure { - proposal_index: ProposalIndex, + PaymentFailure { + payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, }, @@ -606,8 +609,8 @@ pub mod pallet { let next_index = PendingPayments::::count(); PendingPayments::::insert(next_index, pending_payment); - Self::deposit_event(Event::QueuedPayment { - proposal_index: next_index, + Self::deposit_event(Event::PaymentQueued { + payment_index: next_index, asset_kind, amount, beneficiary, @@ -686,8 +689,8 @@ impl, I: 'static> Pallet { if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { total_spent += p.normalized_value; p.payment_id = Some(id); - Self::deposit_event(Event::TriggerPayment { - proposal_index: key, + Self::deposit_event(Event::PaymentTriggered { + payment_index: key, asset_kind: p.asset_kind, amount: p.value, }); @@ -699,8 +702,8 @@ impl, I: 'static> Pallet { PaymentStatus::Failure => { // try again in the next `T::SpendPeriod`. missed_proposals = missed_proposals.saturating_add(1); - Self::deposit_event(Event::ProposalPaymentFailure { - proposal_index: key, + Self::deposit_event(Event::PaymentFailure { + payment_index: key, asset_kind: p.asset_kind, amount: p.value, }); @@ -711,8 +714,8 @@ impl, I: 'static> Pallet { }, PaymentStatus::Success => { PendingPayments::::remove(key); - Self::deposit_event(Event::ProposalPaymentSuccess { - proposal_index: key, + Self::deposit_event(Event::PaymentSuccess { + payment_index: key, asset_kind: p.asset_kind, amount: p.value, }); From b295ea77994210dc6a7094efc25ff529df5236ff Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 30 Mar 2023 17:24:40 +0100 Subject: [PATCH 33/60] adjust tests to check more cases --- frame/treasury/src/benchmarking.rs | 46 ++++++++++++++++++++++++++---- frame/treasury/src/tests.rs | 43 +++++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 7 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 4f136fa7eb4ef..68377499a60e0 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -43,6 +43,20 @@ fn setup_proposal, I: 'static>( (caller, value, beneficiary_lookup) } +// Create the pre-requisite information needed to create a treasury `spend` call. +fn setup_spend_call, I: 'static>( + u: u32, +) -> (T::AssetKind, T::AccountId, PayBalanceOf, AccountIdLookupOf) { + let caller = account("caller", u, SEED); + // let _ = Assets::force_create(RuntimeOrigin::root(), 0, &caller, true, 100); + // let _ = Assets::mint_into(0, &caller, 100); + + let value: PayBalanceOf = 1000; + let beneficiary = account("beneficiary", u, SEED); + let beneficiary_lookup = T::Lookup::unlookup(beneficiary); + (0, caller, value, beneficiary_lookup) +} + // Create proposals that are approved for use in `on_initialize`. fn create_approved_proposals, I: 'static>(n: u32) -> Result<(), &'static str> { for i in 0..n { @@ -56,12 +70,14 @@ fn create_approved_proposals, I: 'static>(n: u32) -> Result<(), &'s } // Create pending payments that are approved for use in `on_initialize`. -fn create_pending_payments, I: 'static>(n: u32) -> Result<(), &'static str> { +fn create_pending_payments, I: 'static>( + n: u32, +) -> Result<(), &'static str> { for i in 0..n { - let (caller, value, lookup) = setup_proposal::(i); + let (caller, value, lookup) = setup_spend_call::(i); Treasury::::spend(RawOrigin::Signed(caller).into(), 0, value, lookup)?; } - ensure!(>::count() == n as usize, "Not all approved"); + ensure!(>::count() == n, "Enpty pending payments storage"); Ok(()) } @@ -78,11 +94,11 @@ fn assert_last_event, I: 'static>(generic_event: >:: benchmarks_instance_pallet! { // This benchmark is short-circuited if `SpendOrigin` cannot provide // a successful origin, in which case `spend` is un-callable and can use weight=0. - spend { + spend_local { let (_, value, beneficiary_lookup) = setup_proposal::(SEED); let origin = T::SpendOrigin::try_successful_origin(); let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); - let call = Call::::spend { amount: value, beneficiary: beneficiary_lookup }; + let call = Call::::spend_local { amount: value, beneficiary: beneficiary_lookup }; }: { if let Ok(origin) = origin.clone() { call.dispatch_bypass_filter(origin)?; @@ -94,6 +110,24 @@ benchmarks_instance_pallet! { } } + // This benchmark is short-circuited if `SpendOrigin` cannot provide + // a successful origin, in which case `spend` is un-callable and can use weight=0. + spend { + let (asset_kind, _, value, beneficiary_lookup) = setup_spend_call::(SEED); + let origin = T::SpendOrigin::try_successful_origin(); + let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); + let call = Call::::spend { asset_kind: 0, amount: value, beneficiary: beneficiary_lookup }; + }: { + if let Ok(origin) = origin.clone() { + call.dispatch_bypass_filter(origin)?; + } + } + verify { + if origin.is_ok() { + assert_last_event::(Event::PaymentQueued{ payment_index: 0, asset_kind, amount: value, beneficiary }.into()) + } + } + propose_spend { let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); // Whitelist caller account from further DB operations. @@ -150,7 +184,7 @@ benchmarks_instance_pallet! { on_initialize_pending_payments { let p in 0 .. T::MaxApprovals::get(); - create_pending_payments::(p)?; + create_pending_payments::(p)?; }: { Treasury::::on_initialize(T::BlockNumber::zero()); } diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index c561809a3cea0..d3aadebb3be87 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -574,7 +574,7 @@ fn remove_already_removed_approval_fails() { } #[test] -fn spending_in_batch_respects_max_total() { +fn spending_local_in_batch_respects_max_total() { new_test_ext().execute_with(|| { // Respect the `max_total` for the given origin. assert_ok!(RuntimeCall::from(UtilityCall::batch_all { @@ -597,3 +597,44 @@ fn spending_in_batch_respects_max_total() { ); }) } + +#[test] +fn spending_in_batch_respects_max_total() { + new_test_ext().execute_with(|| { + // Respect the `max_total` for the given origin. + assert_ok!(RuntimeCall::from(UtilityCall::batch_all { + calls: vec![ + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 0, + amount: 2, + beneficiary: 100 + }), + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 0, + amount: 2, + beneficiary: 101 + }) + ] + }) + .dispatch(RuntimeOrigin::signed(10))); + + assert_err_ignore_postinfo!( + RuntimeCall::from(UtilityCall::batch_all { + calls: vec![ + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 0, + amount: 2, + beneficiary: 100 + }), + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 0, + amount: 4, + beneficiary: 101 + }) + ] + }) + .dispatch(RuntimeOrigin::signed(10)), + Error::::InsufficientPermission + ); + }) +} From 1e85a67ba3cd65e0bffb9464cd04414c19842319 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Mon, 3 Apr 2023 17:47:57 +0200 Subject: [PATCH 34/60] benchmarking update --- frame/treasury/src/benchmarking.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 68377499a60e0..9b976e7839e8f 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -43,18 +43,22 @@ fn setup_proposal, I: 'static>( (caller, value, beneficiary_lookup) } +const asset_kind: AssetId = 0; + // Create the pre-requisite information needed to create a treasury `spend` call. -fn setup_spend_call, I: 'static>( +fn setup_spend_call, I: 'static>( u: u32, ) -> (T::AssetKind, T::AccountId, PayBalanceOf, AccountIdLookupOf) { let caller = account("caller", u, SEED); // let _ = Assets::force_create(RuntimeOrigin::root(), 0, &caller, true, 100); // let _ = Assets::mint_into(0, &caller, 100); - let value: PayBalanceOf = 1000; + + // TODO: add to seed so value is not empty + let value: PayBalanceOf = SEED.into(); let beneficiary = account("beneficiary", u, SEED); let beneficiary_lookup = T::Lookup::unlookup(beneficiary); - (0, caller, value, beneficiary_lookup) + (asset_kind, caller, value, beneficiary_lookup) } // Create proposals that are approved for use in `on_initialize`. @@ -74,8 +78,8 @@ fn create_pending_payments, I: 'static>( n: u32, ) -> Result<(), &'static str> { for i in 0..n { - let (caller, value, lookup) = setup_spend_call::(i); - Treasury::::spend(RawOrigin::Signed(caller).into(), 0, value, lookup)?; + let (asset_kind, caller, value, lookup) = setup_spend_call::(i); + Treasury::::spend(RawOrigin::Signed(caller).into(), asset_kind, value, lookup)?; } ensure!(>::count() == n, "Enpty pending payments storage"); Ok(()) @@ -116,7 +120,7 @@ benchmarks_instance_pallet! { let (asset_kind, _, value, beneficiary_lookup) = setup_spend_call::(SEED); let origin = T::SpendOrigin::try_successful_origin(); let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); - let call = Call::::spend { asset_kind: 0, amount: value, beneficiary: beneficiary_lookup }; + let call = Call::::spend { asset_kind, amount: value, beneficiary: beneficiary_lookup }; }: { if let Ok(origin) = origin.clone() { call.dispatch_bypass_filter(origin)?; From eb4b2fede5f5883e949a5f251b83879aef93bb3c Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 4 Apr 2023 12:20:06 +0200 Subject: [PATCH 35/60] make benchmarks pass --- frame/treasury/src/benchmarking.rs | 22 ++++++++++------------ frame/treasury/src/lib.rs | 17 ++++++++++++++++- frame/treasury/src/tests.rs | 2 ++ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 9b976e7839e8f..681a1e689d34f 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -31,6 +31,10 @@ use frame_system::RawOrigin; const SEED: u32 = 0; +fn default_asset_kind, I: 'static>() -> T::AssetKind { + T::BenchmarkHelper::create_asset_kind(0) +} + // Create the pre-requisite information needed to create a treasury `propose_spend`. fn setup_proposal, I: 'static>( u: u32, @@ -43,21 +47,17 @@ fn setup_proposal, I: 'static>( (caller, value, beneficiary_lookup) } -const asset_kind: AssetId = 0; - // Create the pre-requisite information needed to create a treasury `spend` call. fn setup_spend_call, I: 'static>( u: u32, ) -> (T::AssetKind, T::AccountId, PayBalanceOf, AccountIdLookupOf) { let caller = account("caller", u, SEED); - // let _ = Assets::force_create(RuntimeOrigin::root(), 0, &caller, true, 100); - // let _ = Assets::mint_into(0, &caller, 100); - // TODO: add to seed so value is not empty let value: PayBalanceOf = SEED.into(); let beneficiary = account("beneficiary", u, SEED); let beneficiary_lookup = T::Lookup::unlookup(beneficiary); + let asset_kind = default_asset_kind::(); (asset_kind, caller, value, beneficiary_lookup) } @@ -74,12 +74,10 @@ fn create_approved_proposals, I: 'static>(n: u32) -> Result<(), &'s } // Create pending payments that are approved for use in `on_initialize`. -fn create_pending_payments, I: 'static>( - n: u32, -) -> Result<(), &'static str> { +fn create_pending_payments, I: 'static>(n: u32) -> Result<(), &'static str> { for i in 0..n { - let (asset_kind, caller, value, lookup) = setup_spend_call::(i); - Treasury::::spend(RawOrigin::Signed(caller).into(), asset_kind, value, lookup)?; + let (asset_kind, _, value, lookup) = setup_spend_call::(i); + Treasury::::spend(RawOrigin::Root.into(), asset_kind, value, lookup)?; } ensure!(>::count() == n, "Enpty pending payments storage"); Ok(()) @@ -117,9 +115,9 @@ benchmarks_instance_pallet! { // This benchmark is short-circuited if `SpendOrigin` cannot provide // a successful origin, in which case `spend` is un-callable and can use weight=0. spend { - let (asset_kind, _, value, beneficiary_lookup) = setup_spend_call::(SEED); + let (asset_kind, _, value, beneficiary_lookup) = setup_spend_call::(SEED); let origin = T::SpendOrigin::try_successful_origin(); - let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); + let beneficiary: T::AccountId = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); let call = Call::::spend { asset_kind, amount: value, beneficiary: beneficiary_lookup }; }: { if let Ok(origin) = origin.clone() { diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index a7b9131552fb6..86273e6bf1832 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -58,7 +58,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -mod benchmarking; +pub mod benchmarking; #[cfg(test)] mod tests; pub mod weights; @@ -164,6 +164,17 @@ pub mod pallet { #[pallet::pallet] pub struct Pallet(PhantomData<(T, I)>); + #[cfg(feature = "runtime-benchmarks")] + pub trait BenchmarkHelper { + fn create_asset_kind(id: u32) -> AssetKind; + } + #[cfg(feature = "runtime-benchmarks")] + impl> BenchmarkHelper for () { + fn create_asset_kind(id: u32) -> AssetKind { + id.into() + } + } + #[pallet::config] pub trait Config: frame_system::Config { /// The staking balance. @@ -240,6 +251,10 @@ pub mod pallet { /// process. The `Success` value is the maximum amount that this origin is allowed to /// spend at a time. type SpendOrigin: EnsureOrigin>; + + /// Helper trait for benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: BenchmarkHelper; } /// Number of proposals that have been made. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index d3aadebb3be87..9f5b77a584592 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -182,6 +182,8 @@ impl Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } pub fn new_test_ext() -> sp_io::TestExternalities { From 310578d488a45b479ac3d3014f9005254bdbd2da Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 4 Apr 2023 13:12:16 +0200 Subject: [PATCH 36/60] update BalanceConverter to use ConversionFromAssetBalance trait --- frame/support/src/traits/tokens/misc.rs | 4 ++-- frame/treasury/src/lib.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 2559eaab071ed..7ac481d5e699e 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -260,10 +260,10 @@ pub trait ConversionFromAssetBalance { } impl> - BalanceConversion for () + ConversionFromAssetBalance for () { type Error = (); - fn to_asset_balance(balance: InBalance, _asset_id: AssetId) -> Result { + fn from_asset_balance(balance: InBalance, _asset_id: AssetId) -> Result { Ok(balance.into()) } } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 86273e6bf1832..efa4bed678845 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -58,7 +58,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -pub mod benchmarking; +mod benchmarking; #[cfg(test)] mod tests; pub mod weights; @@ -75,7 +75,7 @@ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ print, traits::{ - tokens::{AssetId, BalanceConversion, Pay, PaymentStatus}, + tokens::{AssetId, ConversionFromAssetBalance, Pay, PaymentStatus}, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, @@ -194,7 +194,7 @@ pub mod pallet { type Paymaster: Pay; // THe means of knowing what is the equivalent native Balance of a given asset id Balance. - type BalanceConverter: BalanceConversion< + type BalanceConverter: ConversionFromAssetBalance< PayBalanceOf, Self::AssetKind, BalanceOf, @@ -587,7 +587,7 @@ pub mod pallet { beneficiary: AccountIdLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; - let normalized_amount = T::BalanceConverter::to_asset_balance(amount, asset_kind) + let normalized_amount = T::BalanceConverter::from_asset_balance(amount, asset_kind) .map_err(|_| Error::::BalanceConversionFailed)?; ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); From 22e43a14fe77128b0de91c7a31826053ce63b747 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 4 Apr 2023 16:01:05 +0200 Subject: [PATCH 37/60] Add BenchmarkHelper to runtimes in dependent pallets --- bin/node/runtime/src/lib.rs | 2 ++ frame/bounties/src/tests.rs | 4 ++++ frame/child-bounties/src/tests.rs | 2 ++ frame/support/src/traits/tokens/misc.rs | 5 ++++- 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4aefda5b416d2..ef0db61eb4b15 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1132,6 +1132,8 @@ impl pallet_treasury::Config for Runtime { type WeightInfo = pallet_treasury::weights::SubstrateWeight; type MaxApprovals = MaxApprovals; type SpendOrigin = EnsureWithSuccess, AccountId, MaxBalance>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index a04cd0f0128bb..ae386a01cac54 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -136,6 +136,8 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -158,6 +160,8 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties1; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index ed779f51aa5ce..2557b613bb8e2 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -135,6 +135,8 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { // This will be 50% of the bounty fee. diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 7ac481d5e699e..b1b015be4a4c1 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -263,7 +263,10 @@ impl> ConversionFromAssetBalance for () { type Error = (); - fn from_asset_balance(balance: InBalance, _asset_id: AssetId) -> Result { + fn from_asset_balance( + balance: InBalance, + _asset_id: AssetId, + ) -> Result { Ok(balance.into()) } } From d96d5414a3c2431d4685902e64741e24c15d6a64 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 4 Apr 2023 16:04:28 +0200 Subject: [PATCH 38/60] Update frame/treasury/src/lib.rs Co-authored-by: Francisco Aguirre --- frame/treasury/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index efa4bed678845..5cec8900e7267 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -193,7 +193,7 @@ pub mod pallet { /// This can be implmented over fungibles or some other means. type Paymaster: Pay; - // THe means of knowing what is the equivalent native Balance of a given asset id Balance. + // The means of knowing what is the equivalent native Balance of a given asset id Balance. type BalanceConverter: ConversionFromAssetBalance< PayBalanceOf, Self::AssetKind, From dc891cb2ab6bfa490e74b2f8a15758067353ceda Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 4 Apr 2023 16:04:40 +0200 Subject: [PATCH 39/60] Update frame/treasury/src/lib.rs Co-authored-by: Francisco Aguirre --- frame/treasury/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 5cec8900e7267..dced5c16b76c1 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -374,7 +374,7 @@ pub mod pallet { amount: PayBalanceOf, beneficiary: T::AccountId, }, - /// The has been processed but awaiting payment status. + /// The payment has been processed but awaiting payment status. PaymentTriggered { payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, From 3bd3c8e6372bfdd18f490e8c92be6ce3c64890a9 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 5 Apr 2023 13:28:30 +0200 Subject: [PATCH 40/60] add benchmark helper to tips --- frame/tips/src/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 394296c5eb2d9..9e286280e5b83 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -154,6 +154,8 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -176,6 +178,8 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { From c04955d7c5152d87a27df00b3fd9f1db30eb27e9 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 5 Apr 2023 13:51:55 +0200 Subject: [PATCH 41/60] add From for () --- frame/treasury/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index dced5c16b76c1..c0b86d5514d98 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -174,6 +174,12 @@ pub mod pallet { id.into() } } + #[cfg(feature = "runtime-benchmarks")] + impl From for () { + fn from(error: io::Error) -> Self { + () + } + } #[pallet::config] pub trait Config: frame_system::Config { From b5b5d0ec2c7428e3880ad40e6f45cdb585606396 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 5 Apr 2023 17:57:01 +0200 Subject: [PATCH 42/60] introduce NilBenchmarkHelper for unit asset ids --- frame/bounties/src/tests.rs | 12 ++++++++++-- frame/child-bounties/src/tests.rs | 2 +- frame/tips/src/tests.rs | 5 +++-- frame/treasury/src/lib.rs | 24 +++++++++++++++++++----- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index ae386a01cac54..2e2a76a26740d 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -116,6 +116,14 @@ parameter_types! { pub static SpendLimit1: Balance = u64::MAX; } +pub struct NilBenchmarkHelper; +#[cfg(feature = "runtime-benchmarks")] +impl> pallet_treasury::BenchmarkHelper for NilBenchmarkHelper { + fn create_asset_kind(_id: u32) -> AssetKind { + ().into() + } +} + impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; type AssetKind = (); @@ -137,7 +145,7 @@ impl pallet_treasury::Config for Test { type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + type BenchmarkHelper = NilBenchmarkHelper; } impl pallet_treasury::Config for Test { @@ -161,7 +169,7 @@ impl pallet_treasury::Config for Test { type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + type BenchmarkHelper = NilBenchmarkHelper; } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index 2557b613bb8e2..e9a625ef154ae 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -136,7 +136,7 @@ impl pallet_treasury::Config for Test { type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } parameter_types! { // This will be 50% of the bounty fee. diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 9e286280e5b83..df1c2cfc72735 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -134,6 +134,7 @@ parameter_types! { pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2"); } + impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; type AssetKind = (); @@ -155,7 +156,7 @@ impl pallet_treasury::Config for Test { type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } impl pallet_treasury::Config for Test { @@ -179,7 +180,7 @@ impl pallet_treasury::Config for Test { type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } parameter_types! { diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index c0b86d5514d98..7db126762f293 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -170,17 +170,31 @@ pub mod pallet { } #[cfg(feature = "runtime-benchmarks")] impl> BenchmarkHelper for () { - fn create_asset_kind(id: u32) -> AssetKind { - id.into() + fn create_asset_kind(_id: u32) -> AssetKind { + 0u32.into() } } #[cfg(feature = "runtime-benchmarks")] - impl From for () { - fn from(error: io::Error) -> Self { - () + pub struct NilBenchmarkHelper; + #[cfg(feature = "runtime-benchmarks")] + impl> BenchmarkHelper for NilBenchmarkHelper { + fn create_asset_kind(_id: u32) -> AssetKind { + ().into() } } + // impl> BenchmarkHelper for () { + // fn create_asset_kind(id: u32) -> AssetKind { + // id.into() + // } + // } + // #[cfg(feature = "runtime-benchmarks")] + // impl From for () { + // fn from(_: u32) -> Self { + // () + // } + // } + #[pallet::config] pub trait Config: frame_system::Config { /// The staking balance. From 65c71a059e7f0e48b6c63009e06de907d4fe12c9 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Wed, 5 Apr 2023 18:05:21 +0200 Subject: [PATCH 43/60] remove comment --- frame/treasury/src/benchmarking.rs | 1 - frame/treasury/src/lib.rs | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 681a1e689d34f..96d2da69bfe66 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -53,7 +53,6 @@ fn setup_spend_call, I: 'static>( ) -> (T::AssetKind, T::AccountId, PayBalanceOf, AccountIdLookupOf) { let caller = account("caller", u, SEED); - // TODO: add to seed so value is not empty let value: PayBalanceOf = SEED.into(); let beneficiary = account("beneficiary", u, SEED); let beneficiary_lookup = T::Lookup::unlookup(beneficiary); diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7db126762f293..7b93cf1db848e 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -183,18 +183,6 @@ pub mod pallet { } } - // impl> BenchmarkHelper for () { - // fn create_asset_kind(id: u32) -> AssetKind { - // id.into() - // } - // } - // #[cfg(feature = "runtime-benchmarks")] - // impl From for () { - // fn from(_: u32) -> Self { - // () - // } - // } - #[pallet::config] pub trait Config: frame_system::Config { /// The staking balance. From 5750f0b2467ae142eff90c466e449e94362145ec Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 20 Apr 2023 19:28:27 +0200 Subject: [PATCH 44/60] add debug log for pay errors; Pay trait should support an error type --- frame/support/src/traits/tokens/pay.rs | 16 ++++++++++------ frame/treasury/src/lib.rs | 23 +++++++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index a21c5575c7958..73d02e40662ed 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -20,6 +20,7 @@ use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_core::{RuntimeDebug, TypedGet}; +use sp_runtime::DispatchError; use sp_std::fmt::Debug; use super::{fungible, fungibles, Balance, Preservation::Expendable}; @@ -38,13 +39,15 @@ pub trait Pay { type AssetKind; /// An identifier given to an individual payment. type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; + /// An error which could be returned by the Pay type + type Error: Debug; /// Make a payment and return an identifier for later evaluation of success in some off-chain /// mechanism (likely an event, but possibly not on this chain). fn pay( who: &Self::Beneficiary, asset_kind: Self::AssetKind, amount: Self::Balance, - ) -> Result; + ) -> Result; /// Check how a payment has proceeded. `id` must have been previously returned by `pay` for /// the result of this call to be meaningful. Once this returns anything other than /// `InProgress` for some `id` it must return `Unknown` rather than the actual result @@ -81,12 +84,13 @@ impl> Pay for PayFromAccount { type Beneficiary = A::Type; type AssetKind = (); type Id = (); + type Error = DispatchError; fn pay( who: &Self::Beneficiary, _: Self::AssetKind, amount: Self::Balance, - ) -> Result { - >::transfer(&A::get(), who, amount, Expendable).map_err(|_| ())?; + ) -> Result { + >::transfer(&A::get(), who, amount, Expendable)?; Ok(()) } fn check_payment(_: ()) -> PaymentStatus { @@ -108,13 +112,13 @@ impl + fungibles::Inspect> P type Beneficiary = A::Type; type AssetKind = F::AssetId; type Id = (); + type Error = DispatchError; fn pay( who: &Self::Beneficiary, asset_id: Self::AssetKind, amount: Self::Balance, - ) -> Result { - >::transfer(asset_id, &A::get(), who, amount, Expendable) - .map_err(|_| ())?; + ) -> Result { + >::transfer(asset_id, &A::get(), who, amount, Expendable)?; Ok(()) } fn check_payment(_: ()) -> PaymentStatus { diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7b93cf1db848e..fc3b800caff0f 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -73,7 +73,7 @@ use sp_runtime::{ use sp_std::{collections::btree_map::BTreeMap, prelude::*}; use frame_support::{ - print, + log, print, traits::{ tokens::{AssetId, ConversionFromAssetBalance, Pay, PaymentStatus}, Currency, @@ -87,6 +87,8 @@ use frame_support::{ pub use pallet::*; pub use weights::WeightInfo; +const LOG_TARGET: &str = "runtime::treasury"; + pub type PayBalanceOf = <>::Paymaster as Pay>::Balance; pub type BalanceOf = <>::Currency as Currency<::AccountId>>::Balance; @@ -696,7 +698,8 @@ impl, I: 'static> Pallet { r } - /// Spend some money! returns number of approvals before spend. + /// Spend_funds is triggered periodically and uses the `T::Paymaster` to payout all spend + /// requests in the `PendingPayments` storage map. pub fn spend_funds() -> Weight { let mut total_weight = Weight::zero(); let mut total_spent = BalanceOf::::zero(); @@ -708,8 +711,8 @@ impl, I: 'static> Pallet { for key in PendingPayments::::iter_keys() { if let Some(mut p) = PendingPayments::::get(key) { match p.payment_id { - None => - if let Ok(id) = T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { + None => match T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { + Ok(id) => { total_spent += p.normalized_value; p.payment_id = Some(id); Self::deposit_event(Event::PaymentTriggered { @@ -718,9 +721,17 @@ impl, I: 'static> Pallet { amount: p.value, }); PendingPayments::::set(key, Some(p)); - } else { + }, + Err(err) => { + log::debug!(target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?} and error: {:?}", key, err); missed_proposals = missed_proposals.saturating_add(1); + Self::deposit_event(Event::PaymentFailure { + payment_index: key, + asset_kind: p.asset_kind, + amount: p.value, + }); }, + }, Some(payment_id) => match T::Paymaster::check_payment(payment_id) { PaymentStatus::Failure => { // try again in the next `T::SpendPeriod`. @@ -746,7 +757,7 @@ impl, I: 'static> Pallet { // PaymentStatus::InProgress and PaymentStatus::Unknown indicate that the // proposal status is inconclusive, and might still be successful or failed // in the future. - _ => {}, + PaymentStatus::InProgress | PaymentStatus::Unknown => {}, }, } } else { From e26dee69ef66911e9161021b81a1424418e9021c Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Thu, 20 Apr 2023 20:33:45 +0200 Subject: [PATCH 45/60] add error associated type to Salary pallet test pay impl --- frame/salary/src/lib.rs | 2 +- frame/salary/src/tests.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 0b2b4b47d8b70..a81ae5c33ce65 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -437,7 +437,7 @@ pub mod pallet { claimant.last_active = status.cycle_index; let id = T::Paymaster::pay(&beneficiary, (), payout) - .map_err(|()| Error::::PayError)?; + .map_err(|_| Error::::PayError)?; claimant.status = Attempted { registered, id, amount: payout }; diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index 1b7bc6cbb6df5..081a240b079f0 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -103,12 +103,13 @@ impl Pay for TestPay { type Balance = u64; type Id = u64; type AssetKind = (); + type Error = (); fn pay( who: &Self::Beneficiary, _: Self::AssetKind, amount: Self::Balance, - ) -> Result { + ) -> Result { PAID.with(|paid| *paid.borrow_mut().entry(*who).or_default() += amount); Ok(LAST_ID.with(|lid| { let x = *lid.borrow(); From 0739c29e6b7952dfac43a648db1c2264a65f5b3c Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 21 Apr 2023 11:09:01 +0200 Subject: [PATCH 46/60] fix fmt error --- frame/salary/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index a81ae5c33ce65..1279a30717752 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -436,8 +436,8 @@ pub mod pallet { claimant.last_active = status.cycle_index; - let id = T::Paymaster::pay(&beneficiary, (), payout) - .map_err(|_| Error::::PayError)?; + let id = + T::Paymaster::pay(&beneficiary, (), payout).map_err(|_| Error::::PayError)?; claimant.status = Attempted { registered, id, amount: payout }; From 87b905cc5597bbf2e69c08c75c783c47087c2f4e Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 21 Apr 2023 17:37:23 +0200 Subject: [PATCH 47/60] Trigger CI From 5bc0af97313afc1e0ef3fdf5712b78eacd9a9e91 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 21 Apr 2023 23:19:31 +0200 Subject: [PATCH 48/60] Trigger CI From 1fca0e92b156e3937780083b4db7c403e24c6a70 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 28 Apr 2023 12:10:08 +0200 Subject: [PATCH 49/60] add a debug log for when Paymaster::check_payment returns failure --- frame/treasury/src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index fc3b800caff0f..77620791da6c5 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -734,6 +734,11 @@ impl, I: 'static> Pallet { }, Some(payment_id) => match T::Paymaster::check_payment(payment_id) { PaymentStatus::Failure => { + log::debug!( + target: LOG_TARGET, + "Paymaster::pay failed for PendingPayment with index: {:?}", + key + ); // try again in the next `T::SpendPeriod`. missed_proposals = missed_proposals.saturating_add(1); Self::deposit_event(Event::PaymentFailure { From 7d712e2c60238caa7eaeac5679bf78ba23a70484 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 28 Apr 2023 13:12:02 +0200 Subject: [PATCH 50/60] add payment_id to events --- frame/treasury/src/benchmarking.rs | 2 +- frame/treasury/src/lib.rs | 25 ++++++++++++++++--------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 96d2da69bfe66..bc60d94c7fd61 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -125,7 +125,7 @@ benchmarks_instance_pallet! { } verify { if origin.is_ok() { - assert_last_event::(Event::PaymentQueued{ payment_index: 0, asset_kind, amount: value, beneficiary }.into()) + assert_last_event::(Event::PaymentQueued{ pending_payment_index: 0, asset_kind, amount: value, beneficiary }.into()) } } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 77620791da6c5..7ff8de655fec7 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -379,28 +379,31 @@ pub mod pallet { UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, /// The payment has been queued to be paid out at the next Spend Period PaymentQueued { - payment_index: PendingPaymentIndex, + pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, beneficiary: T::AccountId, }, /// The payment has been processed but awaiting payment status. PaymentTriggered { - payment_index: PendingPaymentIndex, + pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, + payment_id: ::Id, }, /// The proposal was paid successfully PaymentSuccess { - payment_index: PendingPaymentIndex, + pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, + payment_id: ::Id, }, /// The proposal payment failed. Payment will be retried in next spend period. PaymentFailure { - payment_index: PendingPaymentIndex, + pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, amount: PayBalanceOf, + payment_id: Option<::Id>, }, } @@ -635,7 +638,7 @@ pub mod pallet { PendingPayments::::insert(next_index, pending_payment); Self::deposit_event(Event::PaymentQueued { - payment_index: next_index, + pending_payment_index: next_index, asset_kind, amount, beneficiary, @@ -716,9 +719,10 @@ impl, I: 'static> Pallet { total_spent += p.normalized_value; p.payment_id = Some(id); Self::deposit_event(Event::PaymentTriggered { - payment_index: key, + pending_payment_index: key, asset_kind: p.asset_kind, amount: p.value, + payment_id: id, }); PendingPayments::::set(key, Some(p)); }, @@ -726,9 +730,10 @@ impl, I: 'static> Pallet { log::debug!(target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?} and error: {:?}", key, err); missed_proposals = missed_proposals.saturating_add(1); Self::deposit_event(Event::PaymentFailure { - payment_index: key, + pending_payment_index: key, asset_kind: p.asset_kind, amount: p.value, + payment_id: None, }); }, }, @@ -742,9 +747,10 @@ impl, I: 'static> Pallet { // try again in the next `T::SpendPeriod`. missed_proposals = missed_proposals.saturating_add(1); Self::deposit_event(Event::PaymentFailure { - payment_index: key, + pending_payment_index: key, asset_kind: p.asset_kind, amount: p.value, + payment_id: Some(payment_id), }); // Force the payment to none, so a fresh payment is sent during the next // T::SpendPeriod. @@ -754,9 +760,10 @@ impl, I: 'static> Pallet { PaymentStatus::Success => { PendingPayments::::remove(key); Self::deposit_event(Event::PaymentSuccess { - payment_index: key, + pending_payment_index: key, asset_kind: p.asset_kind, amount: p.value, + payment_id, }); }, // PaymentStatus::InProgress and PaymentStatus::Unknown indicate that the From 415561da5833c4ec3ed5a78d50601bdf11d4dcfd Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 15:18:18 +0200 Subject: [PATCH 51/60] Trigger companion CI From 449d48487ec6ed51a67336d3478b0c62c6ce679a Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 16:24:35 +0200 Subject: [PATCH 52/60] Trigger companion CI From 47410377f49e265394b26bdc96e8f735ef996967 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 17:01:01 +0200 Subject: [PATCH 53/60] Trigger Companion CI From 9cad10897e4ade96ce4faf72b100b8daed8e4201 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 17:27:45 +0200 Subject: [PATCH 54/60] Trigger Companion CI From b64432963ea9f1db961649f543dfba32df1df8f5 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 17:47:59 +0200 Subject: [PATCH 55/60] Trigger Companion CI From 8a9903b0b229d09eec417c172a93250c619643dc Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 18:13:04 +0200 Subject: [PATCH 56/60] Trigger Companion CI From f07e8b0ecaa2871137d8986e194b4b6ebde11972 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Fri, 5 May 2023 18:38:43 +0200 Subject: [PATCH 57/60] Trigger Companion CI From 34c859b74ea1ff5a11bf2c9edb6fbe2582c1028f Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 30 May 2023 12:35:18 +0200 Subject: [PATCH 58/60] support vector of assets for treasury spend --- frame/support/src/traits/dispatch.rs | 4 +- frame/support/src/traits/tokens/pay.rs | 4 +- frame/treasury/src/lib.rs | 265 +++++++++++++++++-------- frame/treasury/src/tests.rs | 71 +++++-- 4 files changed, 235 insertions(+), 109 deletions(-) diff --git a/frame/support/src/traits/dispatch.rs b/frame/support/src/traits/dispatch.rs index 6961e69ba5750..90f7888b3c919 100644 --- a/frame/support/src/traits/dispatch.rs +++ b/frame/support/src/traits/dispatch.rs @@ -163,7 +163,9 @@ pub trait EnsureOriginWithArg { /// /// ** Should be used for benchmarking only!!! ** #[cfg(feature = "runtime-benchmarks")] - fn try_successful_origin(a: &Argument) -> Result; + fn try_successful_origin(a: &Argument) -> Result{ + Err(()) + } } pub struct AsEnsureOriginWithArg(sp_std::marker::PhantomData); diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 73d02e40662ed..f666dc0229260 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -23,13 +23,13 @@ use sp_core::{RuntimeDebug, TypedGet}; use sp_runtime::DispatchError; use sp_std::fmt::Debug; -use super::{fungible, fungibles, Balance, Preservation::Expendable}; +use super::{fungible, fungibles, Preservation::Expendable}; /// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with /// XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance: Balance; + type Balance; /// The type by which we identify the beneficiaries to whom a payment may be made. type Beneficiary; /// The type for the kinds of asset that are going to be paid. diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7ff8de655fec7..94e24806fea2c 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -121,12 +121,20 @@ pub trait SpendFunds, I: 'static = ()> { ); } +pub trait Asset { + fn asset_kind(&self) -> AssetId; + fn amount(&self) -> Fungibility; +} + /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; /// An index of a pending payment. Just a `u32`. pub type PendingPaymentIndex = u32; +/// An index for the number tor times a payment is retried. Just a u32 +pub type RetryIndex = u32; + /// A spending proposal. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] @@ -144,17 +152,19 @@ pub struct Proposal { /// PendingPayment represents treasury spend payment which has not yet succeeded. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] -pub struct PendingPayment { +pub struct PendingPayment { /// The account to whom the payment should be made if the proposal is accepted. beneficiary: AccountId, /// The asset_kind of the amount to be paid asset_kind: AssetKind, /// The (total) amount that should be paid. - value: AssetBalance, + // value: AssetBalance, /// The amount to be paid, but normalized to the native asset class normalized_value: Balance, - /// the identifier for tracking the status of a payment which is in flight. + /// The identifier for tracking the status of a payment which is in flight. payment_id: Option, + /// The number of times this payment has been attempted + tries: RetryIndex, } #[frame_support::pallet] @@ -197,11 +207,16 @@ pub mod pallet { type RejectOrigin: EnsureOrigin; /// The identifier for what asset should be spent. - type AssetKind: AssetId; + type AssetId: AssetId; + + // TODO: replace with individual types + type AssetKind: Asset> + AssetId; /// The means by which we can make payments to beneficiaries. /// This can be implmented over fungibles or some other means. - type Paymaster: Pay; + type Paymaster: Pay; + + type MaxPaymentRetries: Get; // The means of knowing what is the equivalent native Balance of a given asset id Balance. type BalanceConverter: ConversionFromAssetBalance< @@ -283,20 +298,26 @@ pub mod pallet { OptionQuery, >; - /// PendingPayments that have not yet processed or are not yet successful. - /// When a `PendingPayment` is verified to be successful, it is deleted from storage. + /// PendingPaymentsInbox that have not yet processed or are not yet successful. + /// When a `PendingPayment` is processed or paid out, it is moved to the PendingPayments + /// storage where it is monitored until it is successful. + #[pallet::storage] + pub type PendingPaymentsInbox, I: 'static = ()> = CountedStorageMap< + _, + Twox64Concat, + PendingPaymentIndex, + PendingPayment, T::AssetKind, ::Id>, + OptionQuery, + >; + + /// PendingPayments that have are not yet successful. When a `PendingPayment` is verified to be + /// successful, it is deleted from storage. #[pallet::storage] pub type PendingPayments, I: 'static = ()> = CountedStorageMap< _, Twox64Concat, PendingPaymentIndex, - PendingPayment< - T::AccountId, - BalanceOf, - T::AssetKind, - PayBalanceOf, - ::Id, - >, + PendingPayment, T::AssetKind, ::Id>, OptionQuery, >; @@ -368,7 +389,10 @@ pub mod pallet { ProcessingProposals { waiting_proposals: ProposalIndex }, /// Spending has finished; this is the number of proposals rolled over till next /// T::SpendPeriod. - RolloverPayments { rollover_proposals: ProposalIndex, allocated_proposals: ProposalIndex }, + RolloverPayments { + rollover_payments: ProposalIndex, + allocated_payments: PendingPaymentIndex, + }, /// A new spend proposal has been approved. SpendApproved { proposal_index: ProposalIndex, @@ -381,29 +405,28 @@ pub mod pallet { PaymentQueued { pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, - amount: PayBalanceOf, beneficiary: T::AccountId, }, /// The payment has been processed but awaiting payment status. PaymentTriggered { pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, - amount: PayBalanceOf, payment_id: ::Id, + tries: RetryIndex, }, /// The proposal was paid successfully PaymentSuccess { pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, - amount: PayBalanceOf, payment_id: ::Id, + tries: RetryIndex, }, /// The proposal payment failed. Payment will be retried in next spend period. PaymentFailure { pending_payment_index: PendingPaymentIndex, asset_kind: T::AssetKind, - amount: PayBalanceOf, payment_id: Option<::Id>, + tries: RetryIndex, }, } @@ -423,6 +446,8 @@ pub mod pallet { ProposalNotApproved, /// Unable to convert asset to native balance BalanceConversionFailed, + /// Invalid Spend Request + InvalidSpendRequest, } #[pallet::hooks] @@ -579,7 +604,7 @@ pub mod pallet { bond: Default::default(), }; Proposals::::insert(proposal_index, proposal); - ProposalCount::::put(proposal_index + 1); + ProposalCount::::put(proposal_index.saturating_add(1)); Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary }); Ok(()) @@ -595,54 +620,56 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, - asset_kind: T::AssetKind, - #[pallet::compact] amount: PayBalanceOf, + assets: Vec, beneficiary: AccountIdLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; - let normalized_amount = T::BalanceConverter::from_asset_balance(amount, asset_kind) - .map_err(|_| Error::::BalanceConversionFailed)?; - ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); - - with_context::>, _>(|v| { - let context = v.or_default(); - - // We group based on `max_amount`, to dinstinguish between different kind of - // origins. (assumes that all origins have different `max_amount`) - // - // Worst case is that we reject some "valid" request. - let spend = context.spend_in_context.entry(max_amount).or_default(); - - // Ensure that we don't overflow nor use more than `max_amount` - if spend.checked_add(&normalized_amount).map(|s| s > max_amount).unwrap_or(true) { - Err(Error::::InsufficientPermission) - } else { - *spend = spend.saturating_add(normalized_amount); - - Ok(()) - } - }) - .unwrap_or(Ok(()))?; - let beneficiary = T::Lookup::lookup(beneficiary)?; - let pending_payment = PendingPayment { - asset_kind, - value: amount, - beneficiary: beneficiary.clone(), - normalized_value: normalized_amount, - payment_id: None, - }; - - let next_index = PendingPayments::::count(); - PendingPayments::::insert(next_index, pending_payment); + for asset in assets { + let normalized_amount = + T::BalanceConverter::from_asset_balance(asset.amount(), asset) + .map_err(|_| Error::::BalanceConversionFailed)?; + ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); + + with_context::>, _>(|v| { + let context = v.or_default(); + + // We group based on `max_amount`, to dinstinguish between different kind of + // origins. (assumes that all origins have different `max_amount`) + // + // Worst case is that we reject some "valid" request. + let spend = context.spend_in_context.entry(max_amount).or_default(); + + // Ensure that we don't overflow nor use more than `max_amount` + if spend.checked_add(&normalized_amount).map(|s| s > max_amount).unwrap_or(true) + { + Err(Error::::InsufficientPermission) + } else { + *spend = spend.saturating_add(normalized_amount); - Self::deposit_event(Event::PaymentQueued { - pending_payment_index: next_index, - asset_kind, - amount, - beneficiary, - }); + Ok(()) + } + }) + .unwrap_or(Ok(()))?; + + let pending_payment = PendingPayment { + asset_kind: asset, + beneficiary: beneficiary.clone(), + normalized_value: normalized_amount, + payment_id: None, + tries: 0, + }; + + let next_index = PendingPaymentsInbox::::count(); + PendingPaymentsInbox::::insert(next_index, pending_payment); + + Self::deposit_event(Event::PaymentQueued { + pending_payment_index: next_index, + asset_kind: asset, + beneficiary: beneficiary.clone(), + }); + } Ok(()) } @@ -703,58 +730,65 @@ impl, I: 'static> Pallet { /// Spend_funds is triggered periodically and uses the `T::Paymaster` to payout all spend /// requests in the `PendingPayments` storage map. - pub fn spend_funds() -> Weight { + pub fn check_and_retry_payments() -> Weight { let mut total_weight = Weight::zero(); - let mut total_spent = BalanceOf::::zero(); - let mut missed_proposals: u32 = 0; - let proposals_len = PendingPayments::::count(); + let pending_payments_len = PendingPayments::::count(); - Self::deposit_event(Event::ProcessingProposals { waiting_proposals: proposals_len }); + Self::deposit_event(Event::ProcessingProposals { waiting_proposals: pending_payments_len }); for key in PendingPayments::::iter_keys() { if let Some(mut p) = PendingPayments::::get(key) { match p.payment_id { - None => match T::Paymaster::pay(&p.beneficiary, p.asset_kind, p.value) { + None => match T::Paymaster::pay( + &p.beneficiary, + p.asset_kind.asset_kind(), + p.asset_kind.amount(), + ) { Ok(id) => { - total_spent += p.normalized_value; + total_spent = total_spent.saturating_add(p.normalized_value); + p.tries = p.tries.saturating_add(1); p.payment_id = Some(id); Self::deposit_event(Event::PaymentTriggered { pending_payment_index: key, asset_kind: p.asset_kind, - amount: p.value, payment_id: id, + tries: p.tries, }); PendingPayments::::set(key, Some(p)); }, Err(err) => { log::debug!(target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?} and error: {:?}", key, err); - missed_proposals = missed_proposals.saturating_add(1); + missed_payments = missed_payments.saturating_add(1); Self::deposit_event(Event::PaymentFailure { pending_payment_index: key, asset_kind: p.asset_kind, - amount: p.value, payment_id: None, + tries: p.tries, }); + p.tries = p.tries.saturating_add(1); + PendingPayments::::set(key, Some(p)); }, }, Some(payment_id) => match T::Paymaster::check_payment(payment_id) { - PaymentStatus::Failure => { + PaymentStatus::Failure | PaymentStatus::Unknown => { log::debug!( target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?}", key ); // try again in the next `T::SpendPeriod`. - missed_proposals = missed_proposals.saturating_add(1); + missed_payments = missed_payments.saturating_add(1); + // Force the payment to none, so a fresh payment is sent during the next + // T::SpendPeriod. + p.payment_id = None; + p.tries = p.tries.saturating_add(1); + Self::deposit_event(Event::PaymentFailure { pending_payment_index: key, asset_kind: p.asset_kind, - amount: p.value, payment_id: Some(payment_id), + tries: p.tries, }); - // Force the payment to none, so a fresh payment is sent during the next - // T::SpendPeriod. - p.payment_id = None; PendingPayments::::set(key, Some(p)); }, PaymentStatus::Success => { @@ -762,25 +796,85 @@ impl, I: 'static> Pallet { Self::deposit_event(Event::PaymentSuccess { pending_payment_index: key, asset_kind: p.asset_kind, - amount: p.value, payment_id, + tries: p.tries, }); }, // PaymentStatus::InProgress and PaymentStatus::Unknown indicate that the // proposal status is inconclusive, and might still be successful or failed // in the future. - PaymentStatus::InProgress | PaymentStatus::Unknown => {}, + PaymentStatus::InProgress => {}, }, } - } else { } } - total_weight += T::WeightInfo::on_initialize_pending_payments(proposals_len); + total_weight = total_weight + .saturating_add(T::WeightInfo::on_initialize_pending_payments(pending_payments_len)); + + Self::deposit_event(Event::RolloverPayments { + rollover_payments: missed_payments, + allocated_payments: pending_payments_len.saturating_sub(missed_payments), + }); + + total_weight + } + + /// Spend_funds is triggered periodically and uses the `T::Paymaster` to payout all spend + /// requests in the `PendingPayments` storage map. + pub fn spend_funds() -> Weight { + let mut total_weight = Weight::zero(); + let mut total_spent = BalanceOf::::zero(); + let mut missed_payments: u32 = 0; + let pending_payments_len = PendingPayments::::count(); + + Self::deposit_event(Event::ProcessingProposals { waiting_proposals: pending_payments_len }); + + for key in PendingPaymentsInbox::::iter_keys() { + if let Some(mut p) = PendingPaymentsInbox::::get(key) { + match p.payment_id { + None => match T::Paymaster::pay( + &p.beneficiary, + p.asset_kind.asset_kind(), + p.asset_kind.amount(), + ) { + Ok(id) => { + total_spent = total_spent.saturating_add(p.normalized_value); + Self::deposit_event(Event::PaymentTriggered { + pending_payment_index: key, + asset_kind: p.asset_kind, + payment_id: Some(id), + tries: p.tries.saturating_add(1), + }); + PendingPayments::::insert(key, Some(p)); + PendingPaymentsInbox::::remove(key); + }, + Err(err) => { + log::debug!(target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?} and error: {:?}", key, err); + missed_payments = missed_payments.saturating_add(1); + Self::deposit_event(Event::PaymentFailure { + pending_payment_index: key, + asset_kind: p.asset_kind, + payment_id: None, + tries: p.tries, + }); + p.tries = p.tries.saturating_add(1); + // Insert it into `T::PendingPayments` to be retried. + PendingPayments::::insert(key, Some(p)); + PendingPaymentsInbox::::remove(key); + }, + }, + Some(_payment_id) => unreachable!(), + } + } + } + + total_weight = total_weight + .saturating_add(T::WeightInfo::on_initialize_pending_payments(pending_payments_len)); Self::deposit_event(Event::RolloverPayments { - rollover_proposals: missed_proposals, - allocated_proposals: proposals_len.saturating_sub(missed_proposals), + rollover_payments: missed_payments, + allocated_payments: pending_payments_len.saturating_sub(missed_payments), }); total_weight @@ -829,7 +923,8 @@ impl, I: 'static> Pallet { proposals_approvals_len }); - total_weight += T::WeightInfo::on_initialize_proposals(proposals_len); + total_weight = + total_weight.saturating_add(T::WeightInfo::on_initialize_proposals(proposals_len)); // Call Runtime hooks to external pallet using treasury to compute spend funds. T::SpendFunds::spend_funds( diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 9f5b77a584592..52d90871999be 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -162,9 +162,38 @@ impl frame_support::traits::EnsureOrigin for TestSpendOrigin { } } +// #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +// pub struct NilAssetKind(u32); + +// impl, I: 'static> Asset for NilAssetKind { +// fn asset_kind(&self) -> T::AssetId { +// ().into() +// } +// fn amount(&self) -> PayBalanceOf { +// let NilAssetKind(val) = self; +// (*val).into() +// } +// } + +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +pub struct SimpleAsset(AssetId, Fungibility); + +impl Asset for SimpleAsset { + fn asset_kind(&self) -> A { + let SimpleAsset(asset_kind, _val) = self; + *asset_kind + } + fn amount(&self) -> F { + let SimpleAsset(_asset_kind, val) = self; + (*val).into() + } +} + impl Config for Test { type PalletId = TreasuryPalletId; - type AssetKind = AssetId; + type AssetId = AssetId; + // TODO: can that AssetId be a type argument of SimpleAsset instead? + type AssetKind = SimpleAsset>; type Paymaster = PayFungibles; type BalanceConverter = (); type Currency = pallet_balances::Pallet; @@ -182,6 +211,7 @@ impl Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; + type MaxPaymentRetries = ConstU32<3>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } @@ -232,21 +262,24 @@ fn spend_local_origin_permissioning_works() { #[test] fn spend_origin_permissioning_works() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::spend(RuntimeOrigin::signed(1), 0, 1, 1), BadOrigin); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(10), 0, 6, 1), + Treasury::spend(RuntimeOrigin::signed(1), vec![SimpleAsset(0, 1)], 1), + BadOrigin + ); + assert_noop!( + Treasury::spend(RuntimeOrigin::signed(10), vec![SimpleAsset(0, 6)], 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(11), 0, 11, 1), + Treasury::spend(RuntimeOrigin::signed(11), vec![SimpleAsset(0, 11)], 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(12), 0, 21, 1), + Treasury::spend(RuntimeOrigin::signed(12), vec![SimpleAsset(0, 21)], 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(13), 0, 51, 1), + Treasury::spend(RuntimeOrigin::signed(13), vec![SimpleAsset(0, 51)], 1), Error::::InsufficientPermission ); }); @@ -280,13 +313,13 @@ fn spend_origin_works() { // Check that accumulate works when we have Some value in Dummy already. assert_ok!(Assets::force_create(RuntimeOrigin::root(), 0, Treasury::account_id(), true, 1)); assert_ok!(Assets::mint_into(0, &Treasury::account_id(), 100)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 0, 5, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 0, 10, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 0, 20, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(13), 0, 50, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), vec![SimpleAsset(0, 5)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), vec![SimpleAsset(0, 5)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), vec![SimpleAsset(0, 5)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), vec![SimpleAsset(0, 5)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), vec![SimpleAsset(0, 10)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), vec![SimpleAsset(0, 20)], 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(13), vec![SimpleAsset(0, 50)], 6)); // Treasury account should still have funds until next T::SpendPeriod assert_eq!(Assets::reducible_balance(0, &Treasury::account_id(), Expendable, Polite), 100); @@ -607,13 +640,11 @@ fn spending_in_batch_respects_max_total() { assert_ok!(RuntimeCall::from(UtilityCall::batch_all { calls: vec![ RuntimeCall::from(TreasuryCall::spend { - asset_kind: 0, - amount: 2, + assets: vec![SimpleAsset(0, 2)], beneficiary: 100 }), RuntimeCall::from(TreasuryCall::spend { - asset_kind: 0, - amount: 2, + assets: vec![SimpleAsset(0, 2)], beneficiary: 101 }) ] @@ -624,13 +655,11 @@ fn spending_in_batch_respects_max_total() { RuntimeCall::from(UtilityCall::batch_all { calls: vec![ RuntimeCall::from(TreasuryCall::spend { - asset_kind: 0, - amount: 2, + assets: vec![SimpleAsset(0, 2)], beneficiary: 100 }), RuntimeCall::from(TreasuryCall::spend { - asset_kind: 0, - amount: 4, + assets: vec![SimpleAsset(0, 4)], beneficiary: 101 }) ] From c02912367781cc9c10a891b9162215b6dd519e87 Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 30 May 2023 15:18:14 +0200 Subject: [PATCH 59/60] fix types on child-bounties and tips --- frame/child-bounties/src/tests.rs | 4 ++- frame/tips/src/tests.rs | 8 +++-- frame/treasury/src/lib.rs | 54 ++++++++++++++++++------------- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index 4b3df123b4abb..3a1e965e198f0 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -117,7 +117,8 @@ parameter_types! { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; - type AssetKind = (); + type AssetId = (); + type AssetKind = u64; type Paymaster = PayFromAccount; type BalanceConverter = (); type Currency = pallet_balances::Pallet; @@ -135,6 +136,7 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + type MaxPaymentRetries = ConstU32<3>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index ef54afbb1e537..53ac127c2d1d6 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -137,7 +137,8 @@ parameter_types! { impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; - type AssetKind = (); + type AssetId = (); + type AssetKind = u64; type Paymaster = PayFromAccount; type BalanceConverter = (); type Currency = pallet_balances::Pallet; @@ -155,13 +156,15 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + type MaxPaymentRetries = ConstU32<3>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId2; - type AssetKind = (); + type AssetId = (); + type AssetKind = u64; type Paymaster = PayFromAccount; type BalanceConverter = (); type Currency = pallet_balances::Pallet; @@ -179,6 +182,7 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + type MaxPaymentRetries = ConstU32<3>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = pallet_treasury::NilBenchmarkHelper; } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 968a08228699f..690936fb7af08 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -126,6 +126,16 @@ pub trait Asset { fn amount(&self) -> Fungibility; } +/// An asset implementation for the unit. Useful in cases where the asset_id is not needed +impl Asset<(), Self> for u64 { + fn asset_kind(&self) -> () { + ().into() + } + fn amount(&self) -> Self { + *self + } +} + /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32; @@ -622,7 +632,7 @@ pub mod pallet { for asset in assets { let normalized_amount = - T::BalanceConverter::from_asset_balance(asset.amount(), asset) + T::BalanceConverter::from_asset_balance(asset.amount(), asset.clone()) .map_err(|_| Error::::BalanceConversionFailed)?; ensure!(normalized_amount <= max_amount, Error::::InsufficientPermission); @@ -648,7 +658,7 @@ pub mod pallet { .unwrap_or(Ok(()))?; let pending_payment = PendingPayment { - asset_kind: asset, + asset_kind: asset.clone(), beneficiary: beneficiary.clone(), normalized_value: normalized_amount, payment_id: None, @@ -732,6 +742,11 @@ impl, I: 'static> Pallet { for key in PendingPayments::::iter_keys() { if let Some(mut p) = PendingPayments::::get(key) { + if p.tries > T::MaxPaymentRetries::get() { + // Skip further processing of payments which have hit the maximum number of + // retures + continue + } match p.payment_id { None => match T::Paymaster::pay( &p.beneficiary, @@ -739,12 +754,11 @@ impl, I: 'static> Pallet { p.asset_kind.amount(), ) { Ok(id) => { - total_spent = total_spent.saturating_add(p.normalized_value); p.tries = p.tries.saturating_add(1); p.payment_id = Some(id); Self::deposit_event(Event::PaymentTriggered { pending_payment_index: key, - asset_kind: p.asset_kind, + asset_kind: p.asset_kind.clone(), payment_id: id, tries: p.tries, }); @@ -752,10 +766,9 @@ impl, I: 'static> Pallet { }, Err(err) => { log::debug!(target: LOG_TARGET, "Paymaster::pay failed for PendingPayment with index: {:?} and error: {:?}", key, err); - missed_payments = missed_payments.saturating_add(1); Self::deposit_event(Event::PaymentFailure { pending_payment_index: key, - asset_kind: p.asset_kind, + asset_kind: p.asset_kind.clone(), payment_id: None, tries: p.tries, }); @@ -770,16 +783,17 @@ impl, I: 'static> Pallet { "Paymaster::pay failed for PendingPayment with index: {:?}", key ); - // try again in the next `T::SpendPeriod`. - missed_payments = missed_payments.saturating_add(1); - // Force the payment to none, so a fresh payment is sent during the next - // T::SpendPeriod. - p.payment_id = None; + p.tries = p.tries.saturating_add(1); + if p.tries < T::MaxPaymentRetries::get() { + // Force the payment to none, so a fresh payment is sent during the + // next T::CheckAndRetryPeriod. + p.payment_id = None; + } Self::deposit_event(Event::PaymentFailure { pending_payment_index: key, - asset_kind: p.asset_kind, + asset_kind: p.asset_kind.clone(), payment_id: Some(payment_id), tries: p.tries, }); @@ -805,12 +819,6 @@ impl, I: 'static> Pallet { total_weight = total_weight .saturating_add(T::WeightInfo::on_initialize_pending_payments(pending_payments_len)); - - Self::deposit_event(Event::RolloverPayments { - rollover_payments: missed_payments, - allocated_payments: pending_payments_len.saturating_sub(missed_payments), - }); - total_weight } @@ -836,11 +844,11 @@ impl, I: 'static> Pallet { total_spent = total_spent.saturating_add(p.normalized_value); Self::deposit_event(Event::PaymentTriggered { pending_payment_index: key, - asset_kind: p.asset_kind, - payment_id: Some(id), + asset_kind: p.asset_kind.clone(), + payment_id: id, tries: p.tries.saturating_add(1), }); - PendingPayments::::insert(key, Some(p)); + PendingPayments::::insert(key, p); PendingPaymentsInbox::::remove(key); }, Err(err) => { @@ -848,13 +856,13 @@ impl, I: 'static> Pallet { missed_payments = missed_payments.saturating_add(1); Self::deposit_event(Event::PaymentFailure { pending_payment_index: key, - asset_kind: p.asset_kind, + asset_kind: p.asset_kind.clone(), payment_id: None, tries: p.tries, }); p.tries = p.tries.saturating_add(1); // Insert it into `T::PendingPayments` to be retried. - PendingPayments::::insert(key, Some(p)); + PendingPayments::::insert(key, p); PendingPaymentsInbox::::remove(key); }, }, From 83a2097637515a0c6894a9ddc4bae3ea1e81312a Mon Sep 17 00:00:00 2001 From: Anthony Alaribe Date: Tue, 30 May 2023 15:57:10 +0200 Subject: [PATCH 60/60] switch node template to use SimpleAsset --- bin/node/runtime/src/lib.rs | 4 +++- frame/support/src/traits/tokens/pay.rs | 4 ++-- frame/treasury/src/lib.rs | 15 +++++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 3c0dda8e56071..fea52fee6f587 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1106,7 +1106,8 @@ parameter_types! { impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; - type AssetKind = u32; + type AssetId = u32; + type AssetKind = pallet_treasury::SimpleAsset; type Paymaster = PayFungibles; type BalanceConverter = (); type Currency = Balances; @@ -1130,6 +1131,7 @@ impl pallet_treasury::Config for Runtime { type WeightInfo = pallet_treasury::weights::SubstrateWeight; type MaxApprovals = MaxApprovals; type SpendOrigin = EnsureWithSuccess, AccountId, MaxBalance>; + type MaxPaymentRetries = ConstU32<3>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 003d0889d9732..b2d6b7130b4e4 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -23,13 +23,13 @@ use sp_core::{RuntimeDebug, TypedGet}; use sp_runtime::DispatchError; use sp_std::fmt::Debug; -use super::{fungible, fungibles, Preservation::Expendable}; +use super::{fungible, fungibles, Balance, Preservation::Expendable}; /// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with /// XCM/MultiAsset and made generic over assets. pub trait Pay { /// The type by which we measure units of the currency in which we make payments. - type Balance; + type Balance: Balance; /// The type by which we identify the beneficiaries to whom a payment may be made. type Beneficiary; /// The type for the kinds of asset that are going to be paid. diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 690936fb7af08..a6b575ac2ba1a 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -136,6 +136,21 @@ impl Asset<(), Self> for u64 { } } +/// SimpleAsset is an asset adapter where we can specify the AssetId and fungibility. +#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +pub struct SimpleAsset(AssetId, Fungibility); + +impl Asset for SimpleAsset { + fn asset_kind(&self) -> A { + let SimpleAsset(asset_kind, _val) = self; + *asset_kind + } + fn amount(&self) -> F { + let SimpleAsset(_asset_kind, val) = self; + (*val).into() + } +} + /// An index of a proposal. Just a `u32`. pub type ProposalIndex = u32;