From 2362c0066ac9e9f452ca10598a131b6c41bbee2e Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 21 Jun 2023 20:00:24 +0200 Subject: [PATCH 01/36] api draft --- frame/treasury/src/lib.rs | 203 +++++++++++++++++++++++++++++++++++++- 1 file changed, 199 insertions(+), 4 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 509ab933430bb..34a181166d775 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -133,10 +133,49 @@ pub struct Proposal { bond: Balance, } -#[frame_support::pallet] +/// The state of the payment claim. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +pub enum PaymentState { + /// Pending claim. + Pending, + /// Payment attempted with a payment identifier. + Attempted { id: Id }, + /// Payment failed. + Failed, +} + +/// Info regarding an approved treasury spend. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] +pub struct SpendStatus { + /// The kind of asset that are going to be spend. + asset_kind: AssetKind, + /// The beneficiary of the spend. + beneficiary: Beneficiary, + /// The block number by which the spend has to be claimed. + expire_at: BlockNumber, + /// The status of the payout/claim. + status: PaymentState, +} + +/// Index of an approved treasury spend. +pub type SpendIndex = u32; + +/// Trait to determine the fungibility of an `AssetKind` and retrieve its balance. +pub trait MatchesFungible { + fn matches_fungible(a: &AssetKind) -> Option; +} + +// TODO remove dev_mode +#[frame_support::pallet(dev_mode)] 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::{ConversionFromAssetBalance, Pay}, + }; use frame_system::pallet_prelude::*; #[pallet::pallet] @@ -204,6 +243,32 @@ pub mod pallet { /// process. The `Success` value is the maximum amount that this origin is allowed to /// spend at a time. type SpendOrigin: EnsureOrigin>; + + /// Type parameter representing the asset kinds to be spent from the treasury. + /// + /// This type can describe both fungible and non-fungible assets and is inspected by + /// [`Self::AssetMatcher`]. + type AssetKind: Parameter + MaxEncodedLen; + + /// Type parameter used to identify the beneficiaries eligible to receive treasury spend. + type Beneficiary: Parameter + MaxEncodedLen; + + /// Type for processing spends of [Self::AssetKind] in favor of [Self::Beneficiary]. + type Paymaster: Pay; + + /// Type to determine the fungibility of an `AssetKind` and retrieving its balance. + type AssetMatcher: MatchesFungible::Balance>; + + /// Type to convert the balance of an [`Self::AssetKind`] to balance of the native asset. + type BalanceConverter: ConversionFromAssetBalance< + ::Balance, + Self::AssetKind, + BalanceOf, + >; + + /// The period during which an approved treasury spend has to be claimed. + #[pallet::constant] + type PayoutPeriod: Get; } /// Number of proposals that have been made. @@ -233,6 +298,20 @@ pub mod pallet { pub type Approvals, I: 'static = ()> = StorageValue<_, BoundedVec, ValueQuery>; + /// The count of spends that have been made. + #[pallet::storage] + pub(crate) type SpendCount = StorageValue<_, SpendIndex, ValueQuery>; + + /// Spends that have been approved and being processed. + #[pallet::storage] + pub type Spends, I: 'static = ()> = StorageMap< + _, + Twox64Concat, + SpendIndex, + SpendStatus::Id>, + OptionQuery, + >; + #[pallet::genesis_config] #[derive(Default)] pub struct GenesisConfig; @@ -294,7 +373,7 @@ pub mod pallet { pub enum Error { /// Proposer's balance is too low. InsufficientProposersBalance, - /// No proposal or bounty at that index. + /// No proposal, bounty or spend at that index. InvalidIndex, /// Too many approvals in the queue. TooManyApprovals, @@ -303,6 +382,14 @@ pub mod pallet { InsufficientPermission, /// Proposal has not been approved. ProposalNotApproved, + /// The asset kind is not fungible. + NotFungible, + /// The balance of the asset kind is not convertible to the balance of the native asset. + FailedToConvertBalance, + /// The spend has expired and cannot be claimed. + SpendExpired, + /// The payment has already been attempted. + AlreadyAttempted, } #[pallet::hooks] @@ -328,6 +415,8 @@ pub mod pallet { } else { Weight::zero() } + + // TODO: once in a while remove expired `Spends` } } @@ -424,7 +513,7 @@ pub mod pallet { /// beneficiary. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::spend())] - pub fn spend( + pub fn spend_local( origin: OriginFor, #[pallet::compact] amount: BalanceOf, beneficiary: AccountIdLookupOf, @@ -501,6 +590,112 @@ pub mod pallet { Ok(()) } + + /// Propose and approve a spend of treasury funds. + /// + /// An approved spend must be claimed via the `payout` dispatchable within the + /// [`T::PayoutPeriod`]. + /// + /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount`. + /// - `asset_kind`: An indicator of the specific asset class to be spent. + /// - `beneficiary`: The beneficiary of the spend. + // TODO weight + #[pallet::call_index(5)] + pub fn spend( + origin: OriginFor, + asset_kind: T::AssetKind, + beneficiary: T::Beneficiary, + ) -> DispatchResult { + let max_amount = T::SpendOrigin::ensure_origin(origin)?; + + let asset_amount = + T::AssetMatcher::matches_fungible(&asset_kind).ok_or(Error::::NotFungible)?; + let native_amount = + T::BalanceConverter::from_asset_balance(asset_amount, asset_kind.clone()) + .map_err(|_| Error::::FailedToConvertBalance)?; + + ensure!(native_amount <= max_amount, Error::::InsufficientPermission); + + with_context::>, _>(|v| { + let context = v.or_default(); + // We group based on `max_amount`, to distinguish 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(&native_amount).map(|s| s > max_amount).unwrap_or(true) { + Err(Error::::InsufficientPermission) + } else { + *spend = spend.saturating_add(native_amount); + Ok(()) + } + }) + .unwrap_or(Ok(()))?; + + let index = SpendCount::::get(); + Spends::::insert( + index, + SpendStatus { + asset_kind, + beneficiary, + expire_at: frame_system::Pallet::::block_number() + .saturating_add(T::PayoutPeriod::get()), + status: PaymentState::Pending, + }, + ); + SpendCount::::put(index + 1); + + // TODO deposit event + + Ok(()) + } + + /// Claim a spend. + /// + /// To claim a spend, it must be done within the [`T::PayoutPeriod`] after approval. + /// In case of a payout failure, the spend status should be updated with the `check_status` + /// before retrying with the current function. + /// + /// - `origin`: Must be `Signed`. + /// - `index`: The spend index. + // TODO weight + #[pallet::call_index(6)] + pub fn payout(origin: OriginFor, index: SpendIndex) -> DispatchResult { + ensure_signed(origin)?; + + let spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; + ensure!( + spend.expire_at > frame_system::Pallet::::block_number(), + Error::::SpendExpired + ); + ensure!( + matches!(spend.status, PaymentState::Pending | PaymentState::Failed), + Error::::AlreadyAttempted + ); + + // TODO payout + // TODO update status + + Ok(()) + } + + /// Check the status of the spend. + /// + /// The status check is a prerequisite for retrying a failed payout. + /// + /// - `origin`: Must be `Signed`. + /// - `index`: The spend index. + // TODO weight + #[pallet::call_index(7)] + pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResult { + ensure_signed(origin)?; + + // TODO check and update status + + Ok(()) + } } } From 36a74c1c98a9e9f46501d3ce0816c37a16766c6e Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 23 Jun 2023 14:29:35 +0200 Subject: [PATCH 02/36] separate asset kind and amount --- bin/node/runtime/src/impls.rs | 14 +++++++++++ bin/node/runtime/src/lib.rs | 10 ++++++-- frame/support/src/traits/tokens/pay.rs | 34 +++++++++++++++++++++++++- frame/treasury/src/lib.rs | 25 ++++++------------- 4 files changed, 63 insertions(+), 20 deletions(-) diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index 8c4a1ed4bbda2..af9b9ecbbc702 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -21,6 +21,7 @@ use frame_support::{ pallet_prelude::*, traits::{ fungibles::{Balanced, Credit}, + tokens::ConversionFromAssetBalance, Currency, OnUnbalanced, }, }; @@ -110,6 +111,19 @@ impl ProposalProvider for AllianceProposalProvider } } +/// A structure that performs identity conversion from asset balance value into balance. +pub struct IdentityBalanceConversion; +impl + ConversionFromAssetBalance for IdentityBalanceConversion +where + AssetBalance: Into, +{ + type Error = (); + fn from_asset_balance(balance: AssetBalance, _: AssetId) -> Result { + Ok(balance.into()) + } +} + #[cfg(test)] mod multiplier_tests { use frame_support::{ diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7d16a1afa1f2d..45feccb55a8a7 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -35,7 +35,7 @@ use frame_support::{ parameter_types, traits::{ fungible::ItemOf, - tokens::{nonfungibles_v2::Inspect, GetSalary, PayFromAccount}, + tokens::{nonfungibles_v2::Inspect, pay::PayAssetFromAccount, GetSalary, PayFromAccount}, AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, WithdrawReasons, @@ -101,7 +101,7 @@ pub use sp_runtime::BuildStorage; pub mod impls; #[cfg(not(feature = "runtime-benchmarks"))] use impls::AllianceIdentityVerifier; -use impls::{AllianceProposalProvider, Author, CreditToBlockAuthor}; +use impls::{AllianceProposalProvider, Author, CreditToBlockAuthor, IdentityBalanceConversion}; /// Constant values used within the runtime. pub mod constants; @@ -1105,6 +1105,7 @@ parameter_types! { pub const MaximumReasonLength: u32 = 300; pub const MaxApprovals: u32 = 100; pub const MaxBalance: Balance = Balance::max_value(); + pub const SpendPayoutPeriod: BlockNumber = 30 * DAYS; } impl pallet_treasury::Config for Runtime { @@ -1130,6 +1131,11 @@ impl pallet_treasury::Config for Runtime { type WeightInfo = pallet_treasury::weights::SubstrateWeight; type MaxApprovals = MaxApprovals; type SpendOrigin = EnsureWithSuccess, AccountId, MaxBalance>; + type AssetKind = u32; + type Beneficiary = AccountId; + type Paymaster = PayAssetFromAccount; + type BalanceConverter = IdentityBalanceConversion; + type PayoutPeriod = SpendPayoutPeriod; } impl pallet_asset_rate::Config for Runtime { diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 78f8e7b873480..7cc7dc318954b 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -23,7 +23,7 @@ use sp_core::{RuntimeDebug, TypedGet}; use sp_runtime::DispatchError; 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. @@ -107,3 +107,35 @@ impl> Pay for PayFromAccount { #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} } + +/// Simple implementation of `Pay` for assets which makes a payment from a "pot" - i.e. a single +/// account. +pub struct PayAssetFromAccount(sp_std::marker::PhantomData<(F, A)>); +impl frame_support::traits::tokens::Pay for PayAssetFromAccount +where + A: TypedGet, + F: fungibles::Mutate, +{ + type Balance = F::Balance; + type Beneficiary = A::Type; + type AssetKind = F::AssetId; + type Id = (); + type Error = DispatchError; + fn pay( + who: &Self::Beneficiary, + asset: Self::AssetKind, + amount: Self::Balance, + ) -> Result { + >::transfer(asset, &A::get(), who, amount, Expendable)?; + Ok(()) + } + fn check_payment(_: ()) -> PaymentStatus { + PaymentStatus::Success + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, asset: Self::AssetKind, amount: Self::Balance) { + >::mint_into(asset.id, &A::get(), amount).unwrap(); + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} +} diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 34a181166d775..26cd4d5a0084e 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::{ - Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, + tokens::Pay, Currency, ExistenceRequirement::KeepAlive, Get, Imbalance, OnUnbalanced, ReservableCurrency, WithdrawReasons, }, weights::Weight, @@ -87,6 +87,7 @@ pub use weights::WeightInfo; pub type BalanceOf = <>::Currency as Currency<::AccountId>>::Balance; +pub type AssetBalanceOf = <>::Paymaster as Pay>::Balance; pub type PositiveImbalanceOf = <>::Currency as Currency< ::AccountId, >>::PositiveImbalance; @@ -162,19 +163,13 @@ pub struct SpendStatus { /// Index of an approved treasury spend. pub type SpendIndex = u32; -/// Trait to determine the fungibility of an `AssetKind` and retrieve its balance. -pub trait MatchesFungible { - fn matches_fungible(a: &AssetKind) -> Option; -} - // TODO remove dev_mode #[frame_support::pallet(dev_mode)] pub mod pallet { use super::*; use frame_support::{ - dispatch_context::with_context, - pallet_prelude::*, - traits::tokens::{ConversionFromAssetBalance, Pay}, + dispatch_context::with_context, pallet_prelude::*, + traits::tokens::ConversionFromAssetBalance, }; use frame_system::pallet_prelude::*; @@ -256,9 +251,6 @@ pub mod pallet { /// Type for processing spends of [Self::AssetKind] in favor of [Self::Beneficiary]. type Paymaster: Pay; - /// Type to determine the fungibility of an `AssetKind` and retrieving its balance. - type AssetMatcher: MatchesFungible::Balance>; - /// Type to convert the balance of an [`Self::AssetKind`] to balance of the native asset. type BalanceConverter: ConversionFromAssetBalance< ::Balance, @@ -598,21 +590,20 @@ pub mod pallet { /// /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount`. /// - `asset_kind`: An indicator of the specific asset class to be spent. + /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. // TODO weight #[pallet::call_index(5)] pub fn spend( origin: OriginFor, asset_kind: T::AssetKind, + #[pallet::compact] amount: AssetBalanceOf, beneficiary: T::Beneficiary, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; - let asset_amount = - T::AssetMatcher::matches_fungible(&asset_kind).ok_or(Error::::NotFungible)?; - let native_amount = - T::BalanceConverter::from_asset_balance(asset_amount, asset_kind.clone()) - .map_err(|_| Error::::FailedToConvertBalance)?; + let native_amount = T::BalanceConverter::from_asset_balance(amount, asset_kind.clone()) + .map_err(|_| Error::::FailedToConvertBalance)?; ensure!(native_amount <= max_amount, Error::::InsufficientPermission); From 58437115767237bbb8f0efc14c98a495550b4f2d Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 23 Jun 2023 14:40:30 +0200 Subject: [PATCH 03/36] add amount to the spend --- frame/treasury/src/lib.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 26cd4d5a0084e..daf50c02e1c0c 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -149,9 +149,11 @@ pub enum PaymentState { /// Info regarding an approved treasury spend. #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] -pub struct SpendStatus { +pub struct SpendStatus { /// The kind of asset that are going to be spend. asset_kind: AssetKind, + /// The asset amount of the spend. + amount: AssetBalance, /// The beneficiary of the spend. beneficiary: Beneficiary, /// The block number by which the spend has to be claimed. @@ -300,7 +302,13 @@ pub mod pallet { _, Twox64Concat, SpendIndex, - SpendStatus::Id>, + SpendStatus< + T::AssetKind, + AssetBalanceOf, + T::Beneficiary, + T::BlockNumber, + ::Id, + >, OptionQuery, >; @@ -630,6 +638,7 @@ pub mod pallet { index, SpendStatus { asset_kind, + amount, beneficiary, expire_at: frame_system::Pallet::::block_number() .saturating_add(T::PayoutPeriod::get()), From 7afa3c37285960b1fc2c3a28198089b140ab7ed7 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 23 Jun 2023 15:54:27 +0200 Subject: [PATCH 04/36] beneficiary lookup --- bin/node/runtime/src/lib.rs | 1 + frame/treasury/src/lib.rs | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 45feccb55a8a7..85da83bfa4a74 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1133,6 +1133,7 @@ impl pallet_treasury::Config for Runtime { type SpendOrigin = EnsureWithSuccess, AccountId, MaxBalance>; type AssetKind = u32; type Beneficiary = AccountId; + type BeneficiaryLookup = Indices; type Paymaster = PayAssetFromAccount; type BalanceConverter = IdentityBalanceConversion; type PayoutPeriod = SpendPayoutPeriod; diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index daf50c02e1c0c..13e8b000eaeb8 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -95,6 +95,7 @@ pub type NegativeImbalanceOf = <>::Currency as Currenc ::AccountId, >>::NegativeImbalance; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; +type BeneficiaryLookupOf = <>::BeneficiaryLookup as StaticLookup>::Source; /// A trait to allow the Treasury Pallet to spend it's funds for other purposes. /// There is an expectation that the implementer of this trait will correctly manage @@ -250,6 +251,9 @@ pub mod pallet { /// Type parameter used to identify the beneficiaries eligible to receive treasury spend. type Beneficiary: Parameter + MaxEncodedLen; + /// Converting trait to take a source type and convert to `Beneficiary`. + type BeneficiaryLookup: StaticLookup; + /// Type for processing spends of [Self::AssetKind] in favor of [Self::Beneficiary]. type Paymaster: Pay; @@ -606,9 +610,10 @@ pub mod pallet { origin: OriginFor, asset_kind: T::AssetKind, #[pallet::compact] amount: AssetBalanceOf, - beneficiary: T::Beneficiary, + beneficiary: BeneficiaryLookupOf, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; + let beneficiary = T::BeneficiaryLookup::lookup(beneficiary)?; let native_amount = T::BalanceConverter::from_asset_balance(amount, asset_kind.clone()) .map_err(|_| Error::::FailedToConvertBalance)?; From bfccac0f6dd45e275db07d1254db19f3e71d2d93 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 25 Jun 2023 18:46:02 +0200 Subject: [PATCH 05/36] asset-rate rename asset_id to asset_kind --- bin/node/runtime/src/lib.rs | 2 +- frame/asset-rate/src/benchmarking.rs | 22 +++++----- frame/asset-rate/src/lib.rs | 60 ++++++++++++++-------------- frame/asset-rate/src/mock.rs | 2 +- frame/asset-rate/src/tests.rs | 8 ++-- 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 85da83bfa4a74..35e1a1a76c53e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1145,7 +1145,7 @@ impl pallet_asset_rate::Config for Runtime { type UpdateOrigin = EnsureRoot; type Balance = Balance; type Currency = Balances; - type AssetId = u32; + type AssetKind = u32; type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_asset_rate::weights::SubstrateWeight; } diff --git a/frame/asset-rate/src/benchmarking.rs b/frame/asset-rate/src/benchmarking.rs index 1209f8db6693b..a8703e58e8e29 100644 --- a/frame/asset-rate/src/benchmarking.rs +++ b/frame/asset-rate/src/benchmarking.rs @@ -30,18 +30,18 @@ fn default_conversion_rate() -> FixedU128 { FixedU128::from_u32(1u32) } -#[benchmarks(where ::AssetId: From)] +#[benchmarks(where ::AssetKind: From)] mod benchmarks { use super::*; #[benchmark] fn create() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_kind: T::AssetKind = ASSET_ID.into(); #[extrinsic_call] - _(RawOrigin::Root, asset_id.clone(), default_conversion_rate()); + _(RawOrigin::Root, asset_kind.clone(), default_conversion_rate()); assert_eq!( - pallet_asset_rate::ConversionRateToNative::::get(asset_id), + pallet_asset_rate::ConversionRateToNative::::get(asset_kind), Some(default_conversion_rate()) ); Ok(()) @@ -49,18 +49,18 @@ mod benchmarks { #[benchmark] fn update() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_kind: T::AssetKind = ASSET_ID.into(); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), - asset_id.clone(), + asset_kind.clone(), default_conversion_rate() )); #[extrinsic_call] - _(RawOrigin::Root, asset_id.clone(), FixedU128::from_u32(2)); + _(RawOrigin::Root, asset_kind.clone(), FixedU128::from_u32(2)); assert_eq!( - pallet_asset_rate::ConversionRateToNative::::get(asset_id), + pallet_asset_rate::ConversionRateToNative::::get(asset_kind), Some(FixedU128::from_u32(2)) ); Ok(()) @@ -68,7 +68,7 @@ mod benchmarks { #[benchmark] fn remove() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_kind: T::AssetKind = ASSET_ID.into(); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), ASSET_ID.into(), @@ -76,9 +76,9 @@ mod benchmarks { )); #[extrinsic_call] - _(RawOrigin::Root, asset_id.clone()); + _(RawOrigin::Root, asset_kind.clone()); - assert!(pallet_asset_rate::ConversionRateToNative::::get(asset_id).is_none()); + assert!(pallet_asset_rate::ConversionRateToNative::::get(asset_kind).is_none()); Ok(()) } diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index ecc793184094b..67fa03863b68a 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -78,8 +78,8 @@ pub mod weights; // Type alias for `frame_system`'s account id. type AccountIdOf = ::AccountId; -// This pallet's asset id and balance type. -type AssetIdOf = ::AssetId; +// This pallet's asset kind and balance type. +type AssetKindOf = ::AssetKind; // Generic fungible balance type. type BalanceOf = <::Currency as Inspect>>::Balance; @@ -115,32 +115,32 @@ pub mod pallet { /// The currency mechanism for this pallet. type Currency: Inspect; - /// The identifier for the class of asset. - type AssetId: frame_support::traits::tokens::AssetId; + /// The type for asset kinds for which the conversion rate to native balance is set. + type AssetKind: Parameter + MaxEncodedLen; } /// Maps an asset to its fixed point representation in the native balance. /// - /// E.g. `native_amount = asset_amount * ConversionRateToNative::::get(asset_id)` + /// E.g. `native_amount = asset_amount * ConversionRateToNative::::get(asset_kind)` #[pallet::storage] pub type ConversionRateToNative = - StorageMap<_, Blake2_128Concat, T::AssetId, FixedU128, OptionQuery>; + StorageMap<_, Blake2_128Concat, T::AssetKind, FixedU128, OptionQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - // Some `asset_id` conversion rate was created. - AssetRateCreated { asset_id: T::AssetId, rate: FixedU128 }, - // Some `asset_id` conversion rate was removed. - AssetRateRemoved { asset_id: T::AssetId }, - // Some existing `asset_id` conversion rate was updated from `old` to `new`. - AssetRateUpdated { asset_id: T::AssetId, old: FixedU128, new: FixedU128 }, + // Some `asset_kind` conversion rate was created. + AssetRateCreated { asset_kind: T::AssetKind, rate: FixedU128 }, + // Some `asset_kind` conversion rate was removed. + AssetRateRemoved { asset_kind: T::AssetKind }, + // Some existing `asset_kind` conversion rate was updated from `old` to `new`. + AssetRateUpdated { asset_kind: T::AssetKind, old: FixedU128, new: FixedU128 }, } #[pallet::error] pub enum Error { /// The given asset ID is unknown. - UnknownAssetId, + UnknownAssetKind, /// The given asset ID already has an assigned conversion rate and cannot be re-created. AlreadyExists, } @@ -155,18 +155,18 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::create())] pub fn create( origin: OriginFor, - asset_id: T::AssetId, + asset_kind: T::AssetKind, rate: FixedU128, ) -> DispatchResult { T::CreateOrigin::ensure_origin(origin)?; ensure!( - !ConversionRateToNative::::contains_key(asset_id.clone()), + !ConversionRateToNative::::contains_key(asset_kind.clone()), Error::::AlreadyExists ); - ConversionRateToNative::::set(asset_id.clone(), Some(rate)); + ConversionRateToNative::::set(asset_kind.clone(), Some(rate)); - Self::deposit_event(Event::AssetRateCreated { asset_id, rate }); + Self::deposit_event(Event::AssetRateCreated { asset_kind, rate }); Ok(()) } @@ -178,24 +178,24 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::update())] pub fn update( origin: OriginFor, - asset_id: T::AssetId, + asset_kind: T::AssetKind, rate: FixedU128, ) -> DispatchResult { T::UpdateOrigin::ensure_origin(origin)?; let mut old = FixedU128::zero(); - ConversionRateToNative::::mutate(asset_id.clone(), |maybe_rate| { + ConversionRateToNative::::mutate(asset_kind.clone(), |maybe_rate| { if let Some(r) = maybe_rate { old = *r; *r = rate; Ok(()) } else { - Err(Error::::UnknownAssetId) + Err(Error::::UnknownAssetKind) } })?; - Self::deposit_event(Event::AssetRateUpdated { asset_id, old, new: rate }); + Self::deposit_event(Event::AssetRateUpdated { asset_kind, old, new: rate }); Ok(()) } @@ -205,23 +205,23 @@ pub mod pallet { /// - O(1) #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::remove())] - pub fn remove(origin: OriginFor, asset_id: T::AssetId) -> DispatchResult { + pub fn remove(origin: OriginFor, asset_kind: T::AssetKind) -> DispatchResult { T::RemoveOrigin::ensure_origin(origin)?; ensure!( - ConversionRateToNative::::contains_key(asset_id.clone()), - Error::::UnknownAssetId + ConversionRateToNative::::contains_key(asset_kind.clone()), + Error::::UnknownAssetKind ); - ConversionRateToNative::::remove(asset_id.clone()); + ConversionRateToNative::::remove(asset_kind.clone()); - Self::deposit_event(Event::AssetRateRemoved { asset_id }); + Self::deposit_event(Event::AssetRateRemoved { asset_kind }); Ok(()) } } } /// Exposes conversion of an arbitrary balance of an asset to native balance. -impl ConversionFromAssetBalance, AssetIdOf, BalanceOf> for Pallet +impl ConversionFromAssetBalance, AssetKindOf, BalanceOf> for Pallet where T: Config, BalanceOf: FixedPointOperand + Zero, @@ -230,10 +230,10 @@ where fn from_asset_balance( balance: BalanceOf, - asset_id: AssetIdOf, + asset_kind: AssetKindOf, ) -> Result, pallet::Error> { - let rate = pallet::ConversionRateToNative::::get(asset_id) - .ok_or(pallet::Error::::UnknownAssetId.into())?; + let rate = pallet::ConversionRateToNative::::get(asset_kind) + .ok_or(pallet::Error::::UnknownAssetKind.into())?; Ok(rate.saturating_mul_int(balance)) } } diff --git a/frame/asset-rate/src/mock.rs b/frame/asset-rate/src/mock.rs index 2d90fcfecd79e..4e6fa87542a63 100644 --- a/frame/asset-rate/src/mock.rs +++ b/frame/asset-rate/src/mock.rs @@ -91,7 +91,7 @@ impl pallet_asset_rate::Config for Test { type UpdateOrigin = frame_system::EnsureRoot; type Balance = u64; type Currency = Balances; - type AssetId = u32; + type AssetKind = u32; } // Build genesis storage according to the mock runtime. diff --git a/frame/asset-rate/src/tests.rs b/frame/asset-rate/src/tests.rs index 4e5a3167bef21..8990ba9fc28d6 100644 --- a/frame/asset-rate/src/tests.rs +++ b/frame/asset-rate/src/tests.rs @@ -66,7 +66,7 @@ fn remove_unknown_throws() { new_test_ext().execute_with(|| { assert_noop!( AssetRate::remove(RuntimeOrigin::root(), ASSET_ID,), - Error::::UnknownAssetId + Error::::UnknownAssetKind ); }); } @@ -89,7 +89,7 @@ fn update_unknown_throws() { new_test_ext().execute_with(|| { assert_noop!( AssetRate::update(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.5)), - Error::::UnknownAssetId + Error::::UnknownAssetKind ); }); } @@ -101,7 +101,7 @@ fn convert_works() { let conversion = , - ::AssetId, + ::AssetKind, BalanceOf, >>::from_asset_balance(10, ASSET_ID); assert_eq!(conversion.expect("Conversion rate exists for asset"), 25); @@ -113,7 +113,7 @@ fn convert_unknown_throws() { new_test_ext().execute_with(|| { let conversion = , - ::AssetId, + ::AssetKind, BalanceOf, >>::from_asset_balance(10, ASSET_ID); assert!(conversion.is_err()); From 5fb73faced51205262b2fab55fb13d1049e46437 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 25 Jun 2023 19:06:21 +0200 Subject: [PATCH 06/36] fex bench --- 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 7cc7dc318954b..29780a083dbe9 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -134,7 +134,7 @@ where } #[cfg(feature = "runtime-benchmarks")] fn ensure_successful(_: &Self::Beneficiary, asset: Self::AssetKind, amount: Self::Balance) { - >::mint_into(asset.id, &A::get(), amount).unwrap(); + >::mint_into(asset, &A::get(), amount).unwrap(); } #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} From 004534ebaabf9ba9ff0096272fc054d236867465 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 25 Jun 2023 19:07:42 +0200 Subject: [PATCH 07/36] fix docs --- frame/treasury/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 13e8b000eaeb8..2c437ff715d76 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -151,7 +151,7 @@ pub enum PaymentState { #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Eq, MaxEncodedLen, RuntimeDebug, TypeInfo)] pub struct SpendStatus { - /// The kind of asset that are going to be spend. + // The kind of asset to be spent. asset_kind: AssetKind, /// The asset amount of the spend. amount: AssetBalance, @@ -251,10 +251,10 @@ pub mod pallet { /// Type parameter used to identify the beneficiaries eligible to receive treasury spend. type Beneficiary: Parameter + MaxEncodedLen; - /// Converting trait to take a source type and convert to `Beneficiary`. + /// Converting trait to take a source type and convert to [`Self::Beneficiary`]. type BeneficiaryLookup: StaticLookup; - /// Type for processing spends of [Self::AssetKind] in favor of [Self::Beneficiary]. + /// Type for processing spends of [Self::AssetKind] in favor of [`Self::Beneficiary`]. type Paymaster: Pay; /// Type to convert the balance of an [`Self::AssetKind`] to balance of the native asset. From 6ff5c5dcc521645900b1a3d7ceb4c306a30bf74e Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 26 Jun 2023 14:13:20 +0200 Subject: [PATCH 08/36] Revert "asset-rate rename asset_id to asset_kind" This reverts commit bfccac0f6dd45e275db07d1254db19f3e71d2d93. --- bin/node/runtime/src/lib.rs | 2 +- frame/asset-rate/src/benchmarking.rs | 22 +++++----- frame/asset-rate/src/lib.rs | 60 ++++++++++++++-------------- frame/asset-rate/src/mock.rs | 2 +- frame/asset-rate/src/tests.rs | 8 ++-- 5 files changed, 47 insertions(+), 47 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 35e1a1a76c53e..85da83bfa4a74 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1145,7 +1145,7 @@ impl pallet_asset_rate::Config for Runtime { type UpdateOrigin = EnsureRoot; type Balance = Balance; type Currency = Balances; - type AssetKind = u32; + type AssetId = u32; type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_asset_rate::weights::SubstrateWeight; } diff --git a/frame/asset-rate/src/benchmarking.rs b/frame/asset-rate/src/benchmarking.rs index a8703e58e8e29..1209f8db6693b 100644 --- a/frame/asset-rate/src/benchmarking.rs +++ b/frame/asset-rate/src/benchmarking.rs @@ -30,18 +30,18 @@ fn default_conversion_rate() -> FixedU128 { FixedU128::from_u32(1u32) } -#[benchmarks(where ::AssetKind: From)] +#[benchmarks(where ::AssetId: From)] mod benchmarks { use super::*; #[benchmark] fn create() -> Result<(), BenchmarkError> { - let asset_kind: T::AssetKind = ASSET_ID.into(); + let asset_id: T::AssetId = ASSET_ID.into(); #[extrinsic_call] - _(RawOrigin::Root, asset_kind.clone(), default_conversion_rate()); + _(RawOrigin::Root, asset_id.clone(), default_conversion_rate()); assert_eq!( - pallet_asset_rate::ConversionRateToNative::::get(asset_kind), + pallet_asset_rate::ConversionRateToNative::::get(asset_id), Some(default_conversion_rate()) ); Ok(()) @@ -49,18 +49,18 @@ mod benchmarks { #[benchmark] fn update() -> Result<(), BenchmarkError> { - let asset_kind: T::AssetKind = ASSET_ID.into(); + let asset_id: T::AssetId = ASSET_ID.into(); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), - asset_kind.clone(), + asset_id.clone(), default_conversion_rate() )); #[extrinsic_call] - _(RawOrigin::Root, asset_kind.clone(), FixedU128::from_u32(2)); + _(RawOrigin::Root, asset_id.clone(), FixedU128::from_u32(2)); assert_eq!( - pallet_asset_rate::ConversionRateToNative::::get(asset_kind), + pallet_asset_rate::ConversionRateToNative::::get(asset_id), Some(FixedU128::from_u32(2)) ); Ok(()) @@ -68,7 +68,7 @@ mod benchmarks { #[benchmark] fn remove() -> Result<(), BenchmarkError> { - let asset_kind: T::AssetKind = ASSET_ID.into(); + let asset_id: T::AssetId = ASSET_ID.into(); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), ASSET_ID.into(), @@ -76,9 +76,9 @@ mod benchmarks { )); #[extrinsic_call] - _(RawOrigin::Root, asset_kind.clone()); + _(RawOrigin::Root, asset_id.clone()); - assert!(pallet_asset_rate::ConversionRateToNative::::get(asset_kind).is_none()); + assert!(pallet_asset_rate::ConversionRateToNative::::get(asset_id).is_none()); Ok(()) } diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index 67fa03863b68a..ecc793184094b 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -78,8 +78,8 @@ pub mod weights; // Type alias for `frame_system`'s account id. type AccountIdOf = ::AccountId; -// This pallet's asset kind and balance type. -type AssetKindOf = ::AssetKind; +// This pallet's asset id and balance type. +type AssetIdOf = ::AssetId; // Generic fungible balance type. type BalanceOf = <::Currency as Inspect>>::Balance; @@ -115,32 +115,32 @@ pub mod pallet { /// The currency mechanism for this pallet. type Currency: Inspect; - /// The type for asset kinds for which the conversion rate to native balance is set. - type AssetKind: Parameter + MaxEncodedLen; + /// The identifier for the class of asset. + type AssetId: frame_support::traits::tokens::AssetId; } /// Maps an asset to its fixed point representation in the native balance. /// - /// E.g. `native_amount = asset_amount * ConversionRateToNative::::get(asset_kind)` + /// E.g. `native_amount = asset_amount * ConversionRateToNative::::get(asset_id)` #[pallet::storage] pub type ConversionRateToNative = - StorageMap<_, Blake2_128Concat, T::AssetKind, FixedU128, OptionQuery>; + StorageMap<_, Blake2_128Concat, T::AssetId, FixedU128, OptionQuery>; #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] pub enum Event { - // Some `asset_kind` conversion rate was created. - AssetRateCreated { asset_kind: T::AssetKind, rate: FixedU128 }, - // Some `asset_kind` conversion rate was removed. - AssetRateRemoved { asset_kind: T::AssetKind }, - // Some existing `asset_kind` conversion rate was updated from `old` to `new`. - AssetRateUpdated { asset_kind: T::AssetKind, old: FixedU128, new: FixedU128 }, + // Some `asset_id` conversion rate was created. + AssetRateCreated { asset_id: T::AssetId, rate: FixedU128 }, + // Some `asset_id` conversion rate was removed. + AssetRateRemoved { asset_id: T::AssetId }, + // Some existing `asset_id` conversion rate was updated from `old` to `new`. + AssetRateUpdated { asset_id: T::AssetId, old: FixedU128, new: FixedU128 }, } #[pallet::error] pub enum Error { /// The given asset ID is unknown. - UnknownAssetKind, + UnknownAssetId, /// The given asset ID already has an assigned conversion rate and cannot be re-created. AlreadyExists, } @@ -155,18 +155,18 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::create())] pub fn create( origin: OriginFor, - asset_kind: T::AssetKind, + asset_id: T::AssetId, rate: FixedU128, ) -> DispatchResult { T::CreateOrigin::ensure_origin(origin)?; ensure!( - !ConversionRateToNative::::contains_key(asset_kind.clone()), + !ConversionRateToNative::::contains_key(asset_id.clone()), Error::::AlreadyExists ); - ConversionRateToNative::::set(asset_kind.clone(), Some(rate)); + ConversionRateToNative::::set(asset_id.clone(), Some(rate)); - Self::deposit_event(Event::AssetRateCreated { asset_kind, rate }); + Self::deposit_event(Event::AssetRateCreated { asset_id, rate }); Ok(()) } @@ -178,24 +178,24 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::update())] pub fn update( origin: OriginFor, - asset_kind: T::AssetKind, + asset_id: T::AssetId, rate: FixedU128, ) -> DispatchResult { T::UpdateOrigin::ensure_origin(origin)?; let mut old = FixedU128::zero(); - ConversionRateToNative::::mutate(asset_kind.clone(), |maybe_rate| { + ConversionRateToNative::::mutate(asset_id.clone(), |maybe_rate| { if let Some(r) = maybe_rate { old = *r; *r = rate; Ok(()) } else { - Err(Error::::UnknownAssetKind) + Err(Error::::UnknownAssetId) } })?; - Self::deposit_event(Event::AssetRateUpdated { asset_kind, old, new: rate }); + Self::deposit_event(Event::AssetRateUpdated { asset_id, old, new: rate }); Ok(()) } @@ -205,23 +205,23 @@ pub mod pallet { /// - O(1) #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::remove())] - pub fn remove(origin: OriginFor, asset_kind: T::AssetKind) -> DispatchResult { + pub fn remove(origin: OriginFor, asset_id: T::AssetId) -> DispatchResult { T::RemoveOrigin::ensure_origin(origin)?; ensure!( - ConversionRateToNative::::contains_key(asset_kind.clone()), - Error::::UnknownAssetKind + ConversionRateToNative::::contains_key(asset_id.clone()), + Error::::UnknownAssetId ); - ConversionRateToNative::::remove(asset_kind.clone()); + ConversionRateToNative::::remove(asset_id.clone()); - Self::deposit_event(Event::AssetRateRemoved { asset_kind }); + Self::deposit_event(Event::AssetRateRemoved { asset_id }); Ok(()) } } } /// Exposes conversion of an arbitrary balance of an asset to native balance. -impl ConversionFromAssetBalance, AssetKindOf, BalanceOf> for Pallet +impl ConversionFromAssetBalance, AssetIdOf, BalanceOf> for Pallet where T: Config, BalanceOf: FixedPointOperand + Zero, @@ -230,10 +230,10 @@ where fn from_asset_balance( balance: BalanceOf, - asset_kind: AssetKindOf, + asset_id: AssetIdOf, ) -> Result, pallet::Error> { - let rate = pallet::ConversionRateToNative::::get(asset_kind) - .ok_or(pallet::Error::::UnknownAssetKind.into())?; + let rate = pallet::ConversionRateToNative::::get(asset_id) + .ok_or(pallet::Error::::UnknownAssetId.into())?; Ok(rate.saturating_mul_int(balance)) } } diff --git a/frame/asset-rate/src/mock.rs b/frame/asset-rate/src/mock.rs index 4e6fa87542a63..2d90fcfecd79e 100644 --- a/frame/asset-rate/src/mock.rs +++ b/frame/asset-rate/src/mock.rs @@ -91,7 +91,7 @@ impl pallet_asset_rate::Config for Test { type UpdateOrigin = frame_system::EnsureRoot; type Balance = u64; type Currency = Balances; - type AssetKind = u32; + type AssetId = u32; } // Build genesis storage according to the mock runtime. diff --git a/frame/asset-rate/src/tests.rs b/frame/asset-rate/src/tests.rs index 8990ba9fc28d6..4e5a3167bef21 100644 --- a/frame/asset-rate/src/tests.rs +++ b/frame/asset-rate/src/tests.rs @@ -66,7 +66,7 @@ fn remove_unknown_throws() { new_test_ext().execute_with(|| { assert_noop!( AssetRate::remove(RuntimeOrigin::root(), ASSET_ID,), - Error::::UnknownAssetKind + Error::::UnknownAssetId ); }); } @@ -89,7 +89,7 @@ fn update_unknown_throws() { new_test_ext().execute_with(|| { assert_noop!( AssetRate::update(RuntimeOrigin::root(), ASSET_ID, FixedU128::from_float(0.5)), - Error::::UnknownAssetKind + Error::::UnknownAssetId ); }); } @@ -101,7 +101,7 @@ fn convert_works() { let conversion = , - ::AssetKind, + ::AssetId, BalanceOf, >>::from_asset_balance(10, ASSET_ID); assert_eq!(conversion.expect("Conversion rate exists for asset"), 25); @@ -113,7 +113,7 @@ fn convert_unknown_throws() { new_test_ext().execute_with(|| { let conversion = , - ::AssetKind, + ::AssetId, BalanceOf, >>::from_asset_balance(10, ASSET_ID); assert!(conversion.is_err()); From edec8004fe42b19535fa297f6ac43b02444cb36d Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 26 Jun 2023 15:38:07 +0200 Subject: [PATCH 09/36] update docs --- frame/treasury/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 2c437ff715d76..673c3312ea6ab 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -243,9 +243,6 @@ pub mod pallet { type SpendOrigin: EnsureOrigin>; /// Type parameter representing the asset kinds to be spent from the treasury. - /// - /// This type can describe both fungible and non-fungible assets and is inspected by - /// [`Self::AssetMatcher`]. type AssetKind: Parameter + MaxEncodedLen; /// Type parameter used to identify the beneficiaries eligible to receive treasury spend. From ee7be40db498cf36fe4a545aed006c69b856889a Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 26 Jun 2023 15:42:56 +0200 Subject: [PATCH 10/36] clone identity --- primitives/runtime/src/traits.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 9c09bad21d9bc..a9bf46668f123 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -731,6 +731,15 @@ impl Convert for Identity { a } } + +/// A structure that performs identity conversion of the reference by cloning it. +pub struct CloneIdentity; +impl Convert<&T, T> for CloneIdentity { + fn convert(a: &T) -> T { + a.clone() + } +} + impl ConvertBack for Identity { fn convert_back(a: T) -> T { a From 7a623f57316144de9cc5e9256b6e672d16a12046 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 28 Jun 2023 13:34:09 +0200 Subject: [PATCH 11/36] payout, check_status dispatchables, tests --- bin/node/runtime/src/lib.rs | 1 + frame/treasury/src/lib.rs | 102 +++++++++++--- frame/treasury/src/tests.rs | 269 +++++++++++++++++++++++++++++++++--- 3 files changed, 337 insertions(+), 35 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 85da83bfa4a74..033e56fbf1b38 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1137,6 +1137,7 @@ impl pallet_treasury::Config for Runtime { type Paymaster = PayAssetFromAccount; type BalanceConverter = IdentityBalanceConversion; type PayoutPeriod = SpendPayoutPeriod; + type BenchmarkHelper = (); } impl pallet_asset_rate::Config for Runtime { diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 673c3312ea6ab..78bd15ba9be45 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -166,13 +166,36 @@ pub struct SpendStatus { + /// Factory function for an asset kind + fn create_asset_kind(seed: u32) -> AssetKind; + /// Factory function for a beneficiary + fn create_beneficiary(seed: [u8; 32]) -> Beneficiary; +} +#[cfg(feature = "runtime-benchmarks")] +impl BenchmarkHelper for () +where + AssetKind: From, + Beneficiary: From<[u8; 32]>, +{ + fn create_asset_kind(seed: u32) -> AssetKind { + seed.into() + } + fn create_beneficiary(seed: [u8; 32]) -> Beneficiary { + seed.into() + } +} + // TODO remove dev_mode #[frame_support::pallet(dev_mode)] pub mod pallet { use super::*; use frame_support::{ - dispatch_context::with_context, pallet_prelude::*, - traits::tokens::ConversionFromAssetBalance, + dispatch_context::with_context, + pallet_prelude::*, + traits::tokens::{ConversionFromAssetBalance, PaymentStatus}, }; use frame_system::pallet_prelude::*; @@ -264,6 +287,10 @@ pub mod pallet { /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] type PayoutPeriod: Get; + + /// Helper type for benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: BenchmarkHelper; } /// Number of proposals that have been made. @@ -367,6 +394,20 @@ pub mod pallet { }, /// The inactive funds of the pallet have been updated. UpdatedInactive { reactivated: BalanceOf, deactivated: BalanceOf }, + /// A new asset spend proposal has been approved. + AssetSpendApproved { + index: SpendIndex, + asset_kind: T::AssetKind, + amount: AssetBalanceOf, + beneficiary: T::Beneficiary, + expire_at: T::BlockNumber, + }, + /// A payment happened. + Paid { index: SpendIndex, payment_id: ::Id }, + /// A payment succeed, the spend removed from the storage. + PaymentSucceed { index: SpendIndex, payment_id: ::Id }, + /// A payment failed. + PaymentFailed { index: SpendIndex, payment_id: ::Id }, } /// Error for the treasury pallet. @@ -383,14 +424,18 @@ pub mod pallet { InsufficientPermission, /// Proposal has not been approved. ProposalNotApproved, - /// The asset kind is not fungible. - NotFungible, /// The balance of the asset kind is not convertible to the balance of the native asset. FailedToConvertBalance, /// The spend has expired and cannot be claimed. SpendExpired, /// The payment has already been attempted. AlreadyAttempted, + /// There was some issue with the mechanism of payment. + PayoutError, + /// The payout was not yet attempted/claimed. + NotAttempted, + /// The payment has neither failed nor succeeded yet. + Inconclusive, } #[pallet::hooks] @@ -513,7 +558,6 @@ 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())] pub fn spend_local( origin: OriginFor, #[pallet::compact] amount: BalanceOf, @@ -636,21 +680,27 @@ pub mod pallet { .unwrap_or(Ok(()))?; let index = SpendCount::::get(); + let expire_at = + frame_system::Pallet::::block_number().saturating_add(T::PayoutPeriod::get()); Spends::::insert( index, SpendStatus { - asset_kind, + asset_kind: asset_kind.clone(), amount, - beneficiary, - expire_at: frame_system::Pallet::::block_number() - .saturating_add(T::PayoutPeriod::get()), + beneficiary: beneficiary.clone(), + expire_at, status: PaymentState::Pending, }, ); SpendCount::::put(index + 1); - // TODO deposit event - + Self::deposit_event(Event::AssetSpendApproved { + index, + asset_kind, + amount, + beneficiary, + expire_at, + }); Ok(()) } @@ -666,8 +716,7 @@ pub mod pallet { #[pallet::call_index(6)] pub fn payout(origin: OriginFor, index: SpendIndex) -> DispatchResult { ensure_signed(origin)?; - - let spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; + let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; ensure!( spend.expire_at > frame_system::Pallet::::block_number(), Error::::SpendExpired @@ -677,8 +726,13 @@ pub mod pallet { Error::::AlreadyAttempted ); - // TODO payout - // TODO update status + let id = T::Paymaster::pay(&spend.beneficiary, spend.asset_kind.clone(), spend.amount) + .map_err(|_| Error::::PayoutError)?; + + spend.status = PaymentState::Attempted { id }; + Spends::::insert(index, spend); + + Self::deposit_event(Event::::Paid { index, payment_id: id }); Ok(()) } @@ -693,9 +747,25 @@ pub mod pallet { #[pallet::call_index(7)] pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResult { ensure_signed(origin)?; + let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; - // TODO check and update status + let payment_id = match spend.status { + PaymentState::Attempted { id } => id, + _ => return Err(Error::::NotAttempted.into()), + }; + match T::Paymaster::check_payment(payment_id) { + PaymentStatus::Failure => { + spend.status = PaymentState::Failed; + Spends::::insert(index, spend); + Self::deposit_event(Event::::PaymentFailed { index, payment_id }); + }, + PaymentStatus::Success => { + Spends::::remove(index); + Self::deposit_event(Event::::PaymentSucceed { index, payment_id }); + }, + _ => return Err(Error::::Inconclusive.into()), + } Ok(()) } } diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 0659c2f5941b1..5c8db7202a4b6 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -19,6 +19,7 @@ #![cfg(test)] +use core::{cell::RefCell, marker::PhantomData}; use sp_core::H256; use sp_runtime::{ testing::Header, @@ -29,7 +30,10 @@ use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{ + tokens::{ConversionFromAssetBalance, PaymentStatus}, + ConstU32, ConstU64, OnInitialize, + }, PalletId, }; @@ -103,10 +107,64 @@ impl pallet_utility::Config for Test { type WeightInfo = (); } +thread_local! { + pub static PAID: RefCell> = RefCell::new(BTreeMap::new()); + pub static STATUS: RefCell> = RefCell::new(BTreeMap::new()); + pub static LAST_ID: RefCell = RefCell::new(0u64); +} + +/// paid balance for a given account and asset ids +fn paid(who: u128, asset_id: u32) -> u64 { + PAID.with(|p| p.borrow().get(&(who, asset_id)).cloned().unwrap_or(0)) +} + +/// reduce paid balance for a given account and asset ids +fn unpay(who: u128, asset_id: u32, amount: u64) { + PAID.with(|p| p.borrow_mut().entry((who, asset_id)).or_default().saturating_reduce(amount)) +} + +/// set status for a given payment id +fn set_status(id: u64, s: PaymentStatus) { + STATUS.with(|m| m.borrow_mut().insert(id, s)); +} + +pub struct TestPay; +impl Pay for TestPay { + type Beneficiary = u128; + type Balance = u64; + type Id = u64; + type AssetKind = u32; + type Error = (); + + fn pay( + who: &Self::Beneficiary, + asset_kind: Self::AssetKind, + amount: Self::Balance, + ) -> Result { + PAID.with(|paid| *paid.borrow_mut().entry((*who, asset_kind)).or_default() += amount); + Ok(LAST_ID.with(|lid| { + let x = *lid.borrow(); + lid.replace(x + 1); + x + })) + } + fn check_payment(id: Self::Id) -> PaymentStatus { + STATUS.with(|s| s.borrow().get(&id).cloned().unwrap_or(PaymentStatus::Unknown)) + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, _: Self::AssetKind, _: Self::Balance) {} + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id) { + set_status(id, PaymentStatus::Failure) + } +} + 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 = TreasuryPalletId::get().into_account_truncating(); + pub const SpendPayoutPeriod: u64 = 5; } pub struct TestSpendOrigin; impl frame_support::traits::EnsureOrigin for TestSpendOrigin { @@ -127,6 +185,14 @@ impl frame_support::traits::EnsureOrigin for TestSpendOrigin { } } +pub struct MulBy(PhantomData); +impl> ConversionFromAssetBalance for MulBy { + type Error = (); + fn from_asset_balance(balance: u64, _asset_id: u32) -> Result { + return balance.checked_mul(N::get()).ok_or(()) + } +} + impl Config for Test { type PalletId = TreasuryPalletId; type Currency = pallet_balances::Pallet; @@ -144,6 +210,12 @@ impl Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = TestSpendOrigin; + type AssetKind = u32; + type Beneficiary = u128; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = TestPay; + type BalanceConverter = MulBy>; + type PayoutPeriod = SpendPayoutPeriod; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -167,40 +239,40 @@ fn genesis_config_works() { } #[test] -fn spend_origin_permissioning_works() { +fn spend_local_origin_permissioning_works() { new_test_ext().execute_with(|| { - assert_noop!(Treasury::spend(RuntimeOrigin::signed(1), 1, 1), BadOrigin); + assert_noop!(Treasury::spend_local(RuntimeOrigin::signed(1), 1, 1), BadOrigin); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(10), 6, 1), + Treasury::spend_local(RuntimeOrigin::signed(10), 6, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(11), 11, 1), + Treasury::spend_local(RuntimeOrigin::signed(11), 11, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(12), 21, 1), + Treasury::spend_local(RuntimeOrigin::signed(12), 21, 1), Error::::InsufficientPermission ); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(13), 51, 1), + Treasury::spend_local(RuntimeOrigin::signed(13), 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); @@ -486,14 +558,47 @@ fn remove_already_removed_approval_fails() { }); } +#[test] +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 { + calls: vec![ + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 100 }), + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 101 }) + ] + }) + .dispatch(RuntimeOrigin::signed(10))); + + assert_err_ignore_postinfo!( + RuntimeCall::from(UtilityCall::batch_all { + calls: vec![ + RuntimeCall::from(TreasuryCall::spend_local { amount: 2, beneficiary: 100 }), + RuntimeCall::from(TreasuryCall::spend_local { amount: 4, beneficiary: 101 }) + ] + }) + .dispatch(RuntimeOrigin::signed(10)), + Error::::InsufficientPermission + ); + }) +} + #[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 { amount: 2, beneficiary: 100 }), - RuntimeCall::from(TreasuryCall::spend { amount: 2, beneficiary: 101 }) + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 1, + amount: 1, + beneficiary: 100 + }), + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 1, + amount: 1, + beneficiary: 101 + }) ] }) .dispatch(RuntimeOrigin::signed(10))); @@ -501,8 +606,16 @@ 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 { + asset_kind: 1, + amount: 2, + beneficiary: 100 + }), + RuntimeCall::from(TreasuryCall::spend { + asset_kind: 1, + amount: 2, + beneficiary: 101 + }) ] }) .dispatch(RuntimeOrigin::signed(10)), @@ -510,3 +623,121 @@ fn spending_in_batch_respects_max_total() { ); }) } + +#[test] +fn spend_origin_works() { + new_test_ext().execute_with(|| { + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 1, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_noop!( + Treasury::spend(RuntimeOrigin::signed(10), 1, 3, 6), + Error::::InsufficientPermission + ); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 1, 5, 6)); + assert_noop!( + Treasury::spend(RuntimeOrigin::signed(11), 1, 6, 6), + Error::::InsufficientPermission + ); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 1, 10, 6)); + assert_noop!( + Treasury::spend(RuntimeOrigin::signed(12), 1, 11, 6), + Error::::InsufficientPermission + ); + + assert_eq!(SpendCount::::get(), 4); + assert_eq!(Spends::::iter().count(), 4); + }); +} + +#[test] +fn spend_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + + assert_eq!(SpendCount::::get(), 1); + assert_eq!( + Spends::::get(0).unwrap(), + SpendStatus { + asset_kind: 1, + amount: 2, + beneficiary: 6, + expire_at: 6, + status: PaymentState::Pending, + } + ); + System::assert_last_event( + Event::::AssetSpendApproved { + index: 0, + asset_kind: 1, + amount: 2, + beneficiary: 6, + expire_at: 6, + } + .into(), + ); + }); +} + +#[test] +fn spend_expires() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + System::set_block_number(6); + assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::SpendExpired); + }); +} + +#[test] +fn payout_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + assert_eq!(paid(6, 1), 2); + assert_eq!(SpendCount::::get(), 1); + let spend = Spends::::get(0).unwrap(); + let maybe_payment_id = match spend.status { + PaymentState::Attempted { id } => Some(id), + _ => None, + }; + let payment_id = maybe_payment_id.expect("no payment attempt"); + System::assert_last_event(Event::::Paid { index: 0, payment_id }.into()); + set_status(payment_id, PaymentStatus::Success); + assert_ok!(Treasury::check_status(RuntimeOrigin::signed(1), 0)); + System::assert_last_event(Event::::PaymentSucceed { index: 0, payment_id }.into()); + // cannot payout again + assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::InvalidIndex); + }); +} + +#[test] +fn payout_retry_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + assert_eq!(paid(6, 1), 2); + let spend = Spends::::get(0).unwrap(); + let maybe_payment_id = match spend.status { + PaymentState::Attempted { id } => Some(id), + _ => None, + }; + let payment_id = maybe_payment_id.expect("no payment attempt"); + // spend payment is failed + set_status(payment_id, PaymentStatus::Failure); + unpay(6, 1, 2); + // cannot payout a spend in the attempted state + assert_noop!( + Treasury::payout(RuntimeOrigin::signed(1), 0), + Error::::AlreadyAttempted + ); + // check status and update it to retry the payout again + assert_ok!(Treasury::check_status(RuntimeOrigin::signed(1), 0)); + System::assert_last_event(Event::::PaymentFailed { index: 0, payment_id }.into()); + // the payout can be retried now + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + assert_eq!(paid(6, 1), 2); + }); +} From 7658928b0f87c3d76de4df35f599f617423889c4 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 29 Jun 2023 19:31:52 +0200 Subject: [PATCH 12/36] spend bench --- frame/treasury/src/benchmarking.rs | 29 +++++++++++++++++++++++++++-- frame/treasury/src/lib.rs | 14 +++++++------- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index a3761083e4faa..4b42d2da64ff5 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -65,14 +65,23 @@ fn assert_last_event, I: 'static>(generic_event: >:: frame_system::Pallet::::assert_last_event(generic_event.into()); } +// Create the pre-requisite information needed to create a treasury `spend`. +fn setup_spend, I: 'static>( +) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) { + let asset_kind = T::BenchmarkHelper::create_asset_kind(SEED); + let beneficiary = T::BenchmarkHelper::create_beneficiary([SEED.try_into().unwrap(); 32]); + let beneficiary_lookup = T::BeneficiaryLookup::unlookup(beneficiary.clone()); + (asset_kind, 100u32.into(), beneficiary, beneficiary_lookup) +} + 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)?; @@ -138,5 +147,21 @@ benchmarks_instance_pallet! { Treasury::::on_initialize(T::BlockNumber::zero()); } + spend { + let origin = + T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let (asset_kind, amount, beneficiary, beneficiary_lookup) = setup_spend::(); + }: _(origin, asset_kind.clone(), amount, beneficiary_lookup) + verify { + let expire_at = + frame_system::Pallet::::block_number().saturating_add(T::PayoutPeriod::get()); + assert_last_event::(Event::AssetSpendApproved { + index: 0, + asset_kind, + amount, + beneficiary, + expire_at}.into()); + } + 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 78bd15ba9be45..21df78ce926ee 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -168,23 +168,23 @@ pub type SpendIndex = u32; /// Trait describing factory functions for dispatchables' parameters #[cfg(feature = "runtime-benchmarks")] -pub trait BenchmarkHelper { +pub trait ParametersFactory { /// Factory function for an asset kind fn create_asset_kind(seed: u32) -> AssetKind; /// Factory function for a beneficiary fn create_beneficiary(seed: [u8; 32]) -> Beneficiary; } #[cfg(feature = "runtime-benchmarks")] -impl BenchmarkHelper for () +impl ParametersFactory for () where AssetKind: From, Beneficiary: From<[u8; 32]>, { - fn create_asset_kind(seed: u32) -> AssetKind { - seed.into() + fn create_asset_kind(id: u32) -> AssetKind { + id.into() } - fn create_beneficiary(seed: [u8; 32]) -> Beneficiary { - seed.into() + fn create_beneficiary(id: [u8; 32]) -> Beneficiary { + id.into() } } @@ -290,7 +290,7 @@ pub mod pallet { /// Helper type for benchmarks. #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper: BenchmarkHelper; + type BenchmarkHelper: ParametersFactory; } /// Number of proposals that have been made. From 38b029c0ddfbd53e0f4ddaacc287fd970299c2e2 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 30 Jun 2023 18:08:15 +0200 Subject: [PATCH 13/36] benches --- frame/support/src/traits/tokens/pay.rs | 3 +- frame/treasury/src/benchmarking.rs | 71 +++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 29780a083dbe9..18af7e5e54838 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -114,7 +114,7 @@ pub struct PayAssetFromAccount(sp_std::marker::PhantomData<(F, A)>); impl frame_support::traits::tokens::Pay for PayAssetFromAccount where A: TypedGet, - F: fungibles::Mutate, + F: fungibles::Mutate + fungibles::Create, { type Balance = F::Balance; type Beneficiary = A::Type; @@ -134,6 +134,7 @@ where } #[cfg(feature = "runtime-benchmarks")] fn ensure_successful(_: &Self::Beneficiary, asset: Self::AssetKind, amount: Self::Balance) { + >::create(asset.clone(), A::get(), true, amount).unwrap(); >::mint_into(asset, &A::get(), amount).unwrap(); } #[cfg(feature = "runtime-benchmarks")] diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 4b42d2da64ff5..4275a2d1e2ac0 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -25,7 +25,7 @@ use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError use frame_support::{ dispatch::UnfilteredDispatchable, ensure, - traits::{EnsureOrigin, OnInitialize}, + traits::{tokens::PaymentStatus, EnsureOrigin, OnInitialize}, }; use frame_system::RawOrigin; @@ -65,11 +65,12 @@ fn assert_last_event, I: 'static>(generic_event: >:: frame_system::Pallet::::assert_last_event(generic_event.into()); } -// Create the pre-requisite information needed to create a treasury `spend`. -fn setup_spend, I: 'static>( +// Create the arguments for the `spend` dispatchable. +fn create_spend_arguments, I: 'static>( + seed: u32, ) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) { - let asset_kind = T::BenchmarkHelper::create_asset_kind(SEED); - let beneficiary = T::BenchmarkHelper::create_beneficiary([SEED.try_into().unwrap(); 32]); + let asset_kind = T::BenchmarkHelper::create_asset_kind(seed); + let beneficiary = T::BenchmarkHelper::create_beneficiary([seed.try_into().unwrap(); 32]); let beneficiary_lookup = T::BeneficiaryLookup::unlookup(beneficiary.clone()); (asset_kind, 100u32.into(), beneficiary, beneficiary_lookup) } @@ -150,7 +151,7 @@ benchmarks_instance_pallet! { spend { let origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - let (asset_kind, amount, beneficiary, beneficiary_lookup) = setup_spend::(); + let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); }: _(origin, asset_kind.clone(), amount, beneficiary_lookup) verify { let expire_at = @@ -163,5 +164,63 @@ benchmarks_instance_pallet! { expire_at}.into()); } + payout { + let origin = + T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); + Treasury::::spend( + origin, + asset_kind.clone(), + amount, + beneficiary_lookup, + )?; + T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); + let caller: T::AccountId = account("caller", 0, SEED); + }: _(RawOrigin::Signed(caller.clone()), 0u32) + verify { + let id = match Spends::::get(0).unwrap().status { + PaymentState::Attempted { id, .. } => { + assert_ne!(T::Paymaster::check_payment(id), PaymentStatus::Failure); + id + } + _ => panic!("No payout attempt made"), + }; + assert_last_event::(Event::Paid { + index: 0, + payment_id: id,}.into()); + assert!(Treasury::::payout(RawOrigin::Signed(caller).into(), 0u32).is_err()); + } + + check_status { + let origin = + T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); + Treasury::::spend( + origin, + asset_kind.clone(), + amount, + beneficiary_lookup, + )?; + T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); + let caller: T::AccountId = account("caller", 0, SEED); + Treasury::::payout( + RawOrigin::Signed(caller.clone()).into(), + 0u32, + )?; + let id = match Spends::::get(0).unwrap().status { + PaymentState::Attempted { id, .. } => { + T::Paymaster::ensure_concluded(id); + assert_eq!(T::Paymaster::check_payment(id), PaymentStatus::Success); + id + } + _ => panic!("No payout attempt made"), + }; + }: _(RawOrigin::Signed(caller.clone()), 0u32) + verify { + assert_last_event::(Event::PaymentSucceed { + index: 0, + payment_id: id,}.into()); + } + impl_benchmark_test_suite!(Treasury, crate::tests::new_test_ext(), crate::tests::Test); } From ab15fe66ee9e7c29fa01dcaa2bb4756008c9e69f Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 30 Jun 2023 18:34:00 +0200 Subject: [PATCH 14/36] remove dev mode --- bin/node/runtime/src/lib.rs | 1 + frame/treasury/src/lib.rs | 10 +- frame/treasury/src/weights.rs | 244 ++++++++++++++++++++++------------ 3 files changed, 167 insertions(+), 88 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 033e56fbf1b38..524930229d7ef 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1137,6 +1137,7 @@ impl pallet_treasury::Config for Runtime { type Paymaster = PayAssetFromAccount; type BalanceConverter = IdentityBalanceConversion; type PayoutPeriod = SpendPayoutPeriod; + #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 21df78ce926ee..35a9c3f06cb3c 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -188,8 +188,7 @@ where } } -// TODO remove dev_mode -#[frame_support::pallet(dev_mode)] +#[frame_support::pallet] pub mod pallet { use super::*; use frame_support::{ @@ -558,6 +557,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_local())] pub fn spend_local( origin: OriginFor, #[pallet::compact] amount: BalanceOf, @@ -645,8 +645,8 @@ pub mod pallet { /// - `asset_kind`: An indicator of the specific asset class to be spent. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. - // TODO weight #[pallet::call_index(5)] + #[pallet::weight(T::WeightInfo::spend())] pub fn spend( origin: OriginFor, asset_kind: T::AssetKind, @@ -712,8 +712,8 @@ pub mod pallet { /// /// - `origin`: Must be `Signed`. /// - `index`: The spend index. - // TODO weight #[pallet::call_index(6)] + #[pallet::weight(T::WeightInfo::payout())] pub fn payout(origin: OriginFor, index: SpendIndex) -> DispatchResult { ensure_signed(origin)?; let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; @@ -743,8 +743,8 @@ pub mod pallet { /// /// - `origin`: Must be `Signed`. /// - `index`: The spend index. - // TODO weight #[pallet::call_index(7)] + #[pallet::weight(T::WeightInfo::check_status())] pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResult { ensure_signed(origin)?; let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index 8f1418f76d969..4b11a5f99570a 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -18,28 +18,23 @@ //! Autogenerated weights for pallet_treasury //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-06-16, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-06-30, STEPS: `50`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `runner-e8ezs4ez-project-145-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! HOSTNAME: `cob`, CPU: `` +//! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 // Executed Command: -// ./target/production/substrate +// ./target/debug/substrate // benchmark // pallet // --chain=dev // --steps=50 -// --repeat=20 -// --pallet=pallet_treasury -// --no-storage-info -// --no-median-slopes -// --no-min-squares +// --repeat=2 +// --pallet=pallet-treasury // --extrinsic=* -// --execution=wasm // --wasm-execution=compiled // --heap-pages=4096 -// --output=./frame/treasury/src/weights.rs -// --header=./HEADER-APACHE2 +// --output=./frame/treasury/src/._weights.rs // --template=./.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -52,91 +47,93 @@ use core::marker::PhantomData; /// Weight functions needed for pallet_treasury. pub trait WeightInfo { - fn spend() -> Weight; + fn spend_local() -> Weight; fn propose_spend() -> Weight; fn reject_proposal() -> Weight; fn approve_proposal(p: u32, ) -> Weight; fn remove_approval() -> Weight; fn on_initialize_proposals(p: u32, ) -> Weight; + fn spend() -> Weight; + fn payout() -> Weight; + fn check_status() -> Weight; } /// Weights for pallet_treasury using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) + /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) - fn spend() -> Weight { + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + fn spend_local() -> Weight { // Proof Size summary in bytes: // Measured: `76` - // Estimated: `1887` - // Minimum execution time: 15_057_000 picoseconds. - Weight::from_parts(15_803_000, 1887) + // Estimated: `1561` + // Minimum execution time: 70_000_000 picoseconds. + Weight::from_parts(78_000_000, 1561) .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) + /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) fn propose_spend() -> Weight { // Proof Size summary in bytes: // Measured: `177` - // Estimated: `1489` - // Minimum execution time: 28_923_000 picoseconds. - Weight::from_parts(29_495_000, 1489) + // Estimated: `1662` + // Minimum execution time: 121_000_000 picoseconds. + Weight::from_parts(166_000_000, 1662) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn reject_proposal() -> Weight { // Proof Size summary in bytes: // Measured: `335` - // Estimated: `3593` - // Minimum execution time: 30_539_000 picoseconds. - Weight::from_parts(30_986_000, 3593) + // Estimated: `3800` + // Minimum execution time: 159_000_000 picoseconds. + Weight::from_parts(164_000_000, 3800) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:0) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// The range of component `p` is `[0, 99]`. fn approve_proposal(p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `504 + p * (8 ±0)` - // Estimated: `3573` - // Minimum execution time: 9_320_000 picoseconds. - Weight::from_parts(12_606_599, 3573) - // Standard Error: 1_302 - .saturating_add(Weight::from_parts(71_054, 0).saturating_mul(p.into())) + // Estimated: `3922 + p * (9 ±0)` + // Minimum execution time: 50_000_000 picoseconds. + Weight::from_parts(94_256_431, 3922) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) + .saturating_add(Weight::from_parts(0, 9).saturating_mul(p.into())) } /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) fn remove_approval() -> Weight { // Proof Size summary in bytes: // Measured: `161` - // Estimated: `1887` - // Minimum execution time: 7_231_000 picoseconds. - Weight::from_parts(7_459_000, 1887) + // Estimated: `1646` + // Minimum execution time: 40_000_000 picoseconds. + Weight::from_parts(43_000_000, 1646) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: Treasury Deactivated (r:1 w:1) - /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Deactivated (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:100 w:100) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// 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) @@ -145,95 +142,135 @@ impl WeightInfo for SubstrateWeight { fn on_initialize_proposals(p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `421 + p * (251 ±0)` - // Estimated: `1887 + p * (5206 ±0)` - // Minimum execution time: 44_769_000 picoseconds. - Weight::from_parts(57_915_572, 1887) - // Standard Error: 59_484 - .saturating_add(Weight::from_parts(42_343_732, 0).saturating_mul(p.into())) + // Estimated: `1894 + p * (5206 ±0)` + // Minimum execution time: 167_000_000 picoseconds. + Weight::from_parts(451_684_629, 1894) + // Standard Error: 1_698_594 + .saturating_add(Weight::from_parts(164_282_216, 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, 5206).saturating_mul(p.into())) } + /// Storage: Treasury SpendCount (r:1 w:1) + /// Proof Skipped: Treasury SpendCount (max_values: Some(1), max_size: None, mode: Measured) + /// Storage: Treasury Spends (r:0 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + fn spend() -> Weight { + // Proof Size summary in bytes: + // Measured: `76` + // Estimated: `1561` + // Minimum execution time: 56_000_000 picoseconds. + Weight::from_parts(57_000_000, 1561) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Storage: Assets Asset (r:1 w:1) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: Assets Account (r:2 w:2) + /// Proof: Assets Account (max_values: None, max_size: Some(134), added: 2609, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + fn payout() -> Weight { + // Proof Size summary in bytes: + // Measured: `705` + // Estimated: `6208` + // Minimum execution time: 268_000_000 picoseconds. + Weight::from_parts(360_000_000, 6208) + .saturating_add(T::DbWeight::get().reads(5_u64)) + .saturating_add(T::DbWeight::get().writes(5_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + fn check_status() -> Weight { + // Proof Size summary in bytes: + // Measured: `194` + // Estimated: `3659` + // Minimum execution time: 61_000_000 picoseconds. + Weight::from_parts(65_000_000, 3659) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } } // For backwards compatibility and tests impl WeightInfo for () { /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) + /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) - fn spend() -> Weight { + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + fn spend_local() -> Weight { // Proof Size summary in bytes: // Measured: `76` - // Estimated: `1887` - // Minimum execution time: 15_057_000 picoseconds. - Weight::from_parts(15_803_000, 1887) + // Estimated: `1561` + // Minimum execution time: 70_000_000 picoseconds. + Weight::from_parts(78_000_000, 1561) .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) + /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) fn propose_spend() -> Weight { // Proof Size summary in bytes: // Measured: `177` - // Estimated: `1489` - // Minimum execution time: 28_923_000 picoseconds. - Weight::from_parts(29_495_000, 1489) + // Estimated: `1662` + // Minimum execution time: 121_000_000 picoseconds. + Weight::from_parts(166_000_000, 1662) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:1) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn reject_proposal() -> Weight { // Proof Size summary in bytes: // Measured: `335` - // Estimated: `3593` - // Minimum execution time: 30_539_000 picoseconds. - Weight::from_parts(30_986_000, 3593) + // Estimated: `3800` + // Minimum execution time: 159_000_000 picoseconds. + Weight::from_parts(164_000_000, 3800) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:0) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// The range of component `p` is `[0, 99]`. fn approve_proposal(p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `504 + p * (8 ±0)` - // Estimated: `3573` - // Minimum execution time: 9_320_000 picoseconds. - Weight::from_parts(12_606_599, 3573) - // Standard Error: 1_302 - .saturating_add(Weight::from_parts(71_054, 0).saturating_mul(p.into())) + // Estimated: `3922 + p * (9 ±0)` + // Minimum execution time: 50_000_000 picoseconds. + Weight::from_parts(94_256_431, 3922) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) + .saturating_add(Weight::from_parts(0, 9).saturating_mul(p.into())) } /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) fn remove_approval() -> Weight { // Proof Size summary in bytes: // Measured: `161` - // Estimated: `1887` - // Minimum execution time: 7_231_000 picoseconds. - Weight::from_parts(7_459_000, 1887) + // Estimated: `1646` + // Minimum execution time: 40_000_000 picoseconds. + Weight::from_parts(43_000_000, 1646) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: Treasury Deactivated (r:1 w:1) - /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Deactivated (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) /// Storage: Treasury Proposals (r:100 w:100) - /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) /// 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) @@ -242,15 +279,56 @@ impl WeightInfo for () { fn on_initialize_proposals(p: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `421 + p * (251 ±0)` - // Estimated: `1887 + p * (5206 ±0)` - // Minimum execution time: 44_769_000 picoseconds. - Weight::from_parts(57_915_572, 1887) - // Standard Error: 59_484 - .saturating_add(Weight::from_parts(42_343_732, 0).saturating_mul(p.into())) + // Estimated: `1894 + p * (5206 ±0)` + // Minimum execution time: 167_000_000 picoseconds. + Weight::from_parts(451_684_629, 1894) + // Standard Error: 1_698_594 + .saturating_add(Weight::from_parts(164_282_216, 0).saturating_mul(p.into())) .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, 5206).saturating_mul(p.into())) } + /// Storage: Treasury SpendCount (r:1 w:1) + /// Proof Skipped: Treasury SpendCount (max_values: Some(1), max_size: None, mode: Measured) + /// Storage: Treasury Spends (r:0 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + fn spend() -> Weight { + // Proof Size summary in bytes: + // Measured: `76` + // Estimated: `1561` + // Minimum execution time: 56_000_000 picoseconds. + Weight::from_parts(57_000_000, 1561) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Storage: Assets Asset (r:1 w:1) + /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) + /// Storage: Assets Account (r:2 w:2) + /// Proof: Assets Account (max_values: None, max_size: Some(134), added: 2609, mode: MaxEncodedLen) + /// Storage: System Account (r:1 w:1) + /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) + fn payout() -> Weight { + // Proof Size summary in bytes: + // Measured: `705` + // Estimated: `6208` + // Minimum execution time: 268_000_000 picoseconds. + Weight::from_parts(360_000_000, 6208) + .saturating_add(RocksDbWeight::get().reads(5_u64)) + .saturating_add(RocksDbWeight::get().writes(5_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + fn check_status() -> Weight { + // Proof Size summary in bytes: + // Measured: `194` + // Estimated: `3659` + // Minimum execution time: 61_000_000 picoseconds. + Weight::from_parts(65_000_000, 3659) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } } From d06e9f9e454257bcee8ac179363a7528c9b905bd Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 1 Jul 2023 15:08:09 +0200 Subject: [PATCH 15/36] tests fix --- bin/node/runtime/src/impls.rs | 14 -------------- bin/node/runtime/src/lib.rs | 4 ++-- frame/bounties/src/tests.rs | 18 ++++++++++++++++-- frame/child-bounties/src/tests.rs | 9 ++++++++- frame/support/src/traits/tokens/misc.rs | 13 +++++++++++++ frame/support/src/traits/tokens/pay.rs | 22 ++++++++++++++++++++++ frame/tips/src/tests.rs | 17 ++++++++++++++++- 7 files changed, 77 insertions(+), 20 deletions(-) diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index af9b9ecbbc702..8c4a1ed4bbda2 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -21,7 +21,6 @@ use frame_support::{ pallet_prelude::*, traits::{ fungibles::{Balanced, Credit}, - tokens::ConversionFromAssetBalance, Currency, OnUnbalanced, }, }; @@ -111,19 +110,6 @@ impl ProposalProvider for AllianceProposalProvider } } -/// A structure that performs identity conversion from asset balance value into balance. -pub struct IdentityBalanceConversion; -impl - ConversionFromAssetBalance for IdentityBalanceConversion -where - AssetBalance: Into, -{ - type Error = (); - fn from_asset_balance(balance: AssetBalance, _: AssetId) -> Result { - Ok(balance.into()) - } -} - #[cfg(test)] mod multiplier_tests { use frame_support::{ diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 524930229d7ef..4128631ecf88b 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -101,7 +101,7 @@ pub use sp_runtime::BuildStorage; pub mod impls; #[cfg(not(feature = "runtime-benchmarks"))] use impls::AllianceIdentityVerifier; -use impls::{AllianceProposalProvider, Author, CreditToBlockAuthor, IdentityBalanceConversion}; +use impls::{AllianceProposalProvider, Author, CreditToBlockAuthor}; /// Constant values used within the runtime. pub mod constants; @@ -1135,7 +1135,7 @@ impl pallet_treasury::Config for Runtime { type Beneficiary = AccountId; type BeneficiaryLookup = Indices; type Paymaster = PayAssetFromAccount; - type BalanceConverter = IdentityBalanceConversion; + type BalanceConverter = (); type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 9fe34b16029cb..b7cbf5dd62042 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -26,14 +26,14 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{ConstU32, ConstU64, OnInitialize}, + traits::{tokens::PayFromAccount, ConstU32, ConstU64, OnInitialize}, PalletId, }; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BadOrigin, BlakeTwo256, IdentityLookup}, + traits::{AccountIdConversion, BadOrigin, BlakeTwo256, IdentityLookup}, BuildStorage, Perbill, Storage, }; @@ -112,6 +112,8 @@ parameter_types! { pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2"); pub static SpendLimit: Balance = u64::MAX; pub static SpendLimit1: Balance = u64::MAX; + pub TreasuryAccount: u128 = TreasuryPalletId::get().into_account_truncating(); + pub TreasuryAccount2: u128 = TreasuryPalletId2::get().into_account_truncating(); } impl pallet_treasury::Config for Test { @@ -131,6 +133,12 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + type AssetKind = (); + type Beneficiary = Self::AccountId; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = PayFromAccount; + type BalanceConverter = (); + type PayoutPeriod = ConstU64<10>; } impl pallet_treasury::Config for Test { @@ -150,6 +158,12 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties1; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + type AssetKind = (); + type Beneficiary = Self::AccountId; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = PayFromAccount; + type BalanceConverter = (); + type PayoutPeriod = ConstU64<10>; } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index a8f0e16ea2136..e72938c475f75 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-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}, weights::Weight, PalletId, }; @@ -112,6 +112,7 @@ 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 = TreasuryPalletId::get().into_account_truncating(); pub const SpendLimit: Balance = u64::MAX; } @@ -132,6 +133,12 @@ impl pallet_treasury::Config for Test { type SpendFunds = Bounties; type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_system::EnsureRootWithSuccess; + type AssetKind = (); + type Beneficiary = Self::AccountId; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = PayFromAccount; + type BalanceConverter = (); + type PayoutPeriod = ConstU64<10>; } 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 baf3fd5f35464..e7631ee9d70aa 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -262,6 +262,19 @@ pub trait ConversionFromAssetBalance { ) -> Result; } +/// Implements the [ConversionFromAssetBalance] trait for a tuple type, enabling a 1:1 conversion of +/// the asset balance value to the balance. +impl + ConversionFromAssetBalance for () +where + AssetBalance: Into, +{ + type Error = (); + fn from_asset_balance(balance: AssetBalance, _: AssetId) -> Result { + Ok(balance.into()) + } +} + /// Trait to handle NFT locking mechanism to ensure interactions with the asset can be implemented /// downstream to extend logic of Uniques/Nfts current functionality. pub trait Locker { diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index 18af7e5e54838..d94dbb0b76aaa 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -140,3 +140,25 @@ where #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} } + +impl Pay for () { + type Balance = u32; + type Beneficiary = (); + type AssetKind = (); + type Id = (); + type Error = (); + fn pay( + _: &Self::Beneficiary, + _: Self::AssetKind, + _: Self::Balance, + ) -> Result { + Err(()) + } + fn check_payment(_: Self::Id) -> PaymentStatus { + PaymentStatus::Failure + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, _: Self::AssetKind, _: Self::Balance) {} + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} +} diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 25043b413db07..83ca517cabebc 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, }; @@ -131,7 +131,10 @@ parameter_types! { pub const Burn: Permill = Permill::from_percent(50); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2"); + pub TreasuryAccount: u128 = TreasuryPalletId::get().into_account_truncating(); + pub TreasuryAccount2: u128 = TreasuryPalletId2::get().into_account_truncating(); } + impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; type Currency = pallet_balances::Pallet; @@ -149,6 +152,12 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + type AssetKind = (); + type Beneficiary = Self::AccountId; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = PayFromAccount; + type BalanceConverter = (); + type PayoutPeriod = ConstU64<10>; } impl pallet_treasury::Config for Test { @@ -168,6 +177,12 @@ impl pallet_treasury::Config for Test { type SpendFunds = (); type MaxApprovals = ConstU32<100>; type SpendOrigin = frame_support::traits::NeverEnsureOrigin; + type AssetKind = (); + type Beneficiary = Self::AccountId; + type BeneficiaryLookup = IdentityLookup; + type Paymaster = PayFromAccount; + type BalanceConverter = (); + type PayoutPeriod = ConstU64<10>; } parameter_types! { From de453799b9148e0887b03516e9a81b4cf6cd974a Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 1 Jul 2023 19:00:32 +0200 Subject: [PATCH 16/36] migrate to v2 benchmarks --- frame/treasury/src/benchmarking.rs | 179 +++++++++++++++++------------ 1 file changed, 106 insertions(+), 73 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 4275a2d1e2ac0..d7f9a6c2a5d2c 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -21,9 +21,11 @@ use super::{Pallet as Treasury, *}; -use frame_benchmarking::v1::{account, benchmarks_instance_pallet, BenchmarkError}; +use frame_benchmarking::{ + v1::{account, BenchmarkError}, + v2::*, +}; use frame_support::{ - dispatch::UnfilteredDispatchable, ensure, traits::{tokens::PaymentStatus, EnsureOrigin, OnInitialize}, }; @@ -75,151 +77,182 @@ fn create_spend_arguments, I: 'static>( (asset_kind, 100u32.into(), beneficiary, beneficiary_lookup) } -benchmarks_instance_pallet! { +#[instance_benchmarks] +mod benchmarks { + use super::*; + // 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_local { + #[benchmark] + fn spend_local() -> Result<(), BenchmarkError> { let (_, value, beneficiary_lookup) = setup_proposal::(SEED); - let origin = T::SpendOrigin::try_successful_origin(); + let origin = + T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let beneficiary = T::Lookup::lookup(beneficiary_lookup.clone()).unwrap(); - let call = Call::::spend_local { 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::SpendApproved { proposal_index: 0, amount: value, beneficiary }.into()) - } + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, value, beneficiary_lookup); + + assert_last_event::( + Event::SpendApproved { proposal_index: 0, amount: value, beneficiary }.into(), + ); + Ok(()) } - propose_spend { + #[benchmark] + fn propose_spend() -> Result<(), BenchmarkError> { let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); // Whitelist caller account from further DB operations. let caller_key = frame_system::Account::::hashed_key_for(&caller); frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); - }: _(RawOrigin::Signed(caller), value, beneficiary_lookup) - reject_proposal { + #[extrinsic_call] + _(RawOrigin::Signed(caller), value, beneficiary_lookup); + + Ok(()) + } + + #[benchmark] + fn reject_proposal() -> Result<(), BenchmarkError> { let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); Treasury::::propose_spend( RawOrigin::Signed(caller).into(), value, - beneficiary_lookup + beneficiary_lookup, )?; let proposal_id = Treasury::::proposal_count() - 1; let reject_origin = T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(reject_origin, proposal_id) - approve_proposal { - let p in 0 .. T::MaxApprovals::get() - 1; + #[extrinsic_call] + _(reject_origin as T::RuntimeOrigin, proposal_id); + + Ok(()) + } + + #[benchmark] + fn approve_proposal( + p: Linear<0, { T::MaxApprovals::get() - 1 }>, + ) -> Result<(), BenchmarkError> { create_approved_proposals::(p)?; let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); Treasury::::propose_spend( RawOrigin::Signed(caller).into(), value, - beneficiary_lookup + beneficiary_lookup, )?; let proposal_id = Treasury::::proposal_count() - 1; let approve_origin = T::ApproveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(approve_origin, proposal_id) - remove_approval { + #[extrinsic_call] + _(approve_origin as T::RuntimeOrigin, proposal_id); + + Ok(()) + } + + #[benchmark] + fn remove_approval() -> Result<(), BenchmarkError> { let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); Treasury::::propose_spend( RawOrigin::Signed(caller).into(), value, - beneficiary_lookup + beneficiary_lookup, )?; let proposal_id = Treasury::::proposal_count() - 1; Treasury::::approve_proposal(RawOrigin::Root.into(), proposal_id)?; let reject_origin = T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(reject_origin, proposal_id) - on_initialize_proposals { - let p in 0 .. T::MaxApprovals::get(); + #[extrinsic_call] + _(reject_origin as T::RuntimeOrigin, proposal_id); + + Ok(()) + } + + #[benchmark] + fn on_initialize_proposals( + p: Linear<0, { T::MaxApprovals::get() - 1 }>, + ) -> Result<(), BenchmarkError> { setup_pot_account::(); create_approved_proposals::(p)?; - }: { - Treasury::::on_initialize(T::BlockNumber::zero()); + + #[block] + { + Treasury::::on_initialize(0u32.into()); + } + + Ok(()) } - spend { + #[benchmark] + fn spend() -> Result<(), BenchmarkError> { let origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); - }: _(origin, asset_kind.clone(), amount, beneficiary_lookup) - verify { + let (asset_kind, amount, beneficiary, beneficiary_lookup) = + create_spend_arguments::(SEED); + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, asset_kind.clone(), amount, beneficiary_lookup); + let expire_at = frame_system::Pallet::::block_number().saturating_add(T::PayoutPeriod::get()); - assert_last_event::(Event::AssetSpendApproved { - index: 0, - asset_kind, - amount, - beneficiary, - expire_at}.into()); + assert_last_event::( + Event::AssetSpendApproved { index: 0, asset_kind, amount, beneficiary, expire_at } + .into(), + ); + Ok(()) } - payout { + #[benchmark] + fn payout() -> Result<(), BenchmarkError> { let origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); - Treasury::::spend( - origin, - asset_kind.clone(), - amount, - beneficiary_lookup, - )?; + let (asset_kind, amount, beneficiary, beneficiary_lookup) = + create_spend_arguments::(SEED); + Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); - }: _(RawOrigin::Signed(caller.clone()), 0u32) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), 0u32); + let id = match Spends::::get(0).unwrap().status { PaymentState::Attempted { id, .. } => { assert_ne!(T::Paymaster::check_payment(id), PaymentStatus::Failure); id - } + }, _ => panic!("No payout attempt made"), }; - assert_last_event::(Event::Paid { - index: 0, - payment_id: id,}.into()); + assert_last_event::(Event::Paid { index: 0, payment_id: id }.into()); assert!(Treasury::::payout(RawOrigin::Signed(caller).into(), 0u32).is_err()); + Ok(()) } - check_status { + #[benchmark] + fn check_status() -> Result<(), BenchmarkError> { let origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); - Treasury::::spend( - origin, - asset_kind.clone(), - amount, - beneficiary_lookup, - )?; + let (asset_kind, amount, beneficiary, beneficiary_lookup) = + create_spend_arguments::(SEED); + Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); - Treasury::::payout( - RawOrigin::Signed(caller.clone()).into(), - 0u32, - )?; + Treasury::::payout(RawOrigin::Signed(caller.clone()).into(), 0u32)?; let id = match Spends::::get(0).unwrap().status { PaymentState::Attempted { id, .. } => { T::Paymaster::ensure_concluded(id); assert_eq!(T::Paymaster::check_payment(id), PaymentStatus::Success); id - } + }, _ => panic!("No payout attempt made"), }; - }: _(RawOrigin::Signed(caller.clone()), 0u32) - verify { - assert_last_event::(Event::PaymentSucceed { - index: 0, - payment_id: id,}.into()); + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), 0u32); + + assert_last_event::(Event::PaymentSucceed { index: 0, payment_id: id }.into()); + Ok(()) } impl_benchmark_test_suite!(Treasury, crate::tests::new_test_ext(), crate::tests::Test); From 22e1c4fbe5913cb0a6f4c7b3730ae55614d97849 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 2 Jul 2023 09:02:09 +0200 Subject: [PATCH 17/36] benchmark helper for tests setup --- frame/bounties/src/tests.rs | 4 ++++ frame/child-bounties/src/tests.rs | 2 ++ frame/tips/src/tests.rs | 4 ++++ frame/treasury/src/tests.rs | 2 ++ 4 files changed, 12 insertions(+) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index b7cbf5dd62042..a444af92f3b9b 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -139,6 +139,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -164,6 +166,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[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 e72938c475f75..e321e8d2fd2ee 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -139,6 +139,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } 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 83ca517cabebc..20feb1d46d161 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -158,6 +158,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -183,6 +185,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 5c8db7202a4b6..f385819b2682d 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -216,6 +216,8 @@ impl Config for Test { type Paymaster = TestPay; type BalanceConverter = MulBy>; type PayoutPeriod = SpendPayoutPeriod; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } pub fn new_test_ext() -> sp_io::TestExternalities { From 76d76afa0d441ebc94f79eca2e6881e53d700b45 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 2 Jul 2023 10:14:36 +0200 Subject: [PATCH 18/36] benchmarks --- frame/bounties/src/tests.rs | 10 ++++++---- frame/child-bounties/src/tests.rs | 5 +++-- frame/tips/src/tests.rs | 10 ++++++---- frame/treasury/src/tests.rs | 5 +++-- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index a444af92f3b9b..0b6855667589b 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -139,8 +139,9 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } impl pallet_treasury::Config for Test { @@ -166,8 +167,9 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index e321e8d2fd2ee..a119782525af7 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -139,8 +139,9 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } 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 20feb1d46d161..fbd5acbafa143 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -158,8 +158,9 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } impl pallet_treasury::Config for Test { @@ -185,8 +186,9 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } parameter_types! { diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index f385819b2682d..a122078299e07 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -216,8 +216,9 @@ impl Config for Test { type Paymaster = TestPay; type BalanceConverter = MulBy>; type PayoutPeriod = SpendPayoutPeriod; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); + pallet_treasury::runtime_benchmarks_enabled! { + type BenchmarkHelper = (); + } } pub fn new_test_ext() -> sp_io::TestExternalities { From 3f314cb7736de2d9c5606bb567a7cec71cfefebc Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 2 Jul 2023 14:46:11 +0200 Subject: [PATCH 19/36] benchmarks bounds for parameters --- bin/node/runtime/src/lib.rs | 2 -- frame/bounties/src/tests.rs | 6 ------ frame/child-bounties/src/tests.rs | 3 --- frame/tips/src/tests.rs | 6 ------ frame/treasury/Cargo.toml | 2 +- frame/treasury/src/benchmarking.rs | 20 ++++++++++++++++---- frame/treasury/src/lib.rs | 26 -------------------------- frame/treasury/src/tests.rs | 3 --- primitives/core/src/crypto.rs | 12 ++++++++++++ 9 files changed, 29 insertions(+), 51 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4128631ecf88b..630eef6c81c52 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1137,8 +1137,6 @@ impl pallet_treasury::Config for Runtime { type Paymaster = PayAssetFromAccount; type BalanceConverter = (); type PayoutPeriod = SpendPayoutPeriod; - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper = (); } impl pallet_asset_rate::Config for Runtime { diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 0b6855667589b..b7cbf5dd62042 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -139,9 +139,6 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } impl pallet_treasury::Config for Test { @@ -167,9 +164,6 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } parameter_types! { diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index a119782525af7..e72938c475f75 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -139,9 +139,6 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } 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 fbd5acbafa143..83ca517cabebc 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -158,9 +158,6 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } impl pallet_treasury::Config for Test { @@ -186,9 +183,6 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } parameter_types! { diff --git a/frame/treasury/Cargo.toml b/frame/treasury/Cargo.toml index e4d06bba64594..33aa5338ba147 100644 --- a/frame/treasury/Cargo.toml +++ b/frame/treasury/Cargo.toml @@ -26,9 +26,9 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys pallet-balances = { version = "4.0.0-dev", default-features = false, path = "../balances" } sp-runtime = { version = "24.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } +sp-core = { version = "21.0.0", default-features = false, path = "../../primitives/core" } [dev-dependencies] -sp-core = { version = "21.0.0", path = "../../primitives/core" } sp-io = { version = "23.0.0", path = "../../primitives/io" } pallet-utility = { version = "4.0.0-dev", path = "../utility" } diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index d7f9a6c2a5d2c..241850a5ab41e 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -30,6 +30,8 @@ use frame_support::{ traits::{tokens::PaymentStatus, EnsureOrigin, OnInitialize}, }; use frame_system::RawOrigin; +use sp_core::crypto::FromEntropy; +use sp_runtime::traits::TrailingZeroInput; const SEED: u32 = 0; @@ -70,14 +72,24 @@ fn assert_last_event, I: 'static>(generic_event: >:: // Create the arguments for the `spend` dispatchable. fn create_spend_arguments, I: 'static>( seed: u32, -) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) { - let asset_kind = T::BenchmarkHelper::create_asset_kind(seed); - let beneficiary = T::BenchmarkHelper::create_beneficiary([seed.try_into().unwrap(); 32]); +) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) +where + >::AssetKind: FromEntropy, + >::Beneficiary: FromEntropy, +{ + let asset_kind = T::AssetKind::from_entropy(&mut seed.encode().as_slice()).unwrap(); + let beneficiary = + T::Beneficiary::from_entropy(&mut TrailingZeroInput::new(seed.encode().as_slice())) + .unwrap(); let beneficiary_lookup = T::BeneficiaryLookup::unlookup(beneficiary.clone()); (asset_kind, 100u32.into(), beneficiary, beneficiary_lookup) } -#[instance_benchmarks] +#[instance_benchmarks( + where + >::AssetKind: FromEntropy, + >::Beneficiary: FromEntropy + )] mod benchmarks { use super::*; diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 35a9c3f06cb3c..cc9c0b049a070 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -166,28 +166,6 @@ pub struct SpendStatus { - /// Factory function for an asset kind - fn create_asset_kind(seed: u32) -> AssetKind; - /// Factory function for a beneficiary - fn create_beneficiary(seed: [u8; 32]) -> Beneficiary; -} -#[cfg(feature = "runtime-benchmarks")] -impl ParametersFactory for () -where - AssetKind: From, - Beneficiary: From<[u8; 32]>, -{ - fn create_asset_kind(id: u32) -> AssetKind { - id.into() - } - fn create_beneficiary(id: [u8; 32]) -> Beneficiary { - id.into() - } -} - #[frame_support::pallet] pub mod pallet { use super::*; @@ -286,10 +264,6 @@ pub mod pallet { /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] type PayoutPeriod: Get; - - /// Helper type for benchmarks. - #[cfg(feature = "runtime-benchmarks")] - type BenchmarkHelper: ParametersFactory; } /// Number of proposals that have been made. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index a122078299e07..5c8db7202a4b6 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -216,9 +216,6 @@ impl Config for Test { type Paymaster = TestPay; type BalanceConverter = MulBy>; type PayoutPeriod = SpendPayoutPeriod; - pallet_treasury::runtime_benchmarks_enabled! { - type BenchmarkHelper = (); - } } pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 800942065f558..fd39efa75e065 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -632,6 +632,12 @@ impl sp_std::str::FromStr for AccountId32 { } } +impl FromEntropy for AccountId32 { + fn from_entropy(input: &mut impl codec::Input) -> Result { + Ok(AccountId32::new(FromEntropy::from_entropy(input)?)) + } +} + #[cfg(feature = "std")] pub use self::dummy::*; @@ -1171,6 +1177,12 @@ impl FromEntropy for bool { } } +impl FromEntropy for () { + fn from_entropy(_: &mut impl codec::Input) -> Result { + Ok(()) + } +} + macro_rules! impl_from_entropy { ($type:ty , $( $others:tt )*) => { impl_from_entropy!($type); From 9eb50e2a193054161fbefa14e4e157300b72c3e3 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 2 Jul 2023 15:46:01 +0200 Subject: [PATCH 20/36] benchmarks fix --- frame/support/src/traits/tokens/pay.rs | 22 ---------------------- frame/treasury/src/benchmarking.rs | 8 ++++---- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs index d94dbb0b76aaa..18af7e5e54838 100644 --- a/frame/support/src/traits/tokens/pay.rs +++ b/frame/support/src/traits/tokens/pay.rs @@ -140,25 +140,3 @@ where #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(_: Self::Id) {} } - -impl Pay for () { - type Balance = u32; - type Beneficiary = (); - type AssetKind = (); - type Id = (); - type Error = (); - fn pay( - _: &Self::Beneficiary, - _: Self::AssetKind, - _: Self::Balance, - ) -> Result { - Err(()) - } - fn check_payment(_: Self::Id) -> PaymentStatus { - PaymentStatus::Failure - } - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: &Self::Beneficiary, _: Self::AssetKind, _: Self::Balance) {} - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(_: Self::Id) {} -} diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 241850a5ab41e..3c1c9dc198ca9 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -251,11 +251,9 @@ mod benchmarks { T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); Treasury::::payout(RawOrigin::Signed(caller.clone()).into(), 0u32)?; - let id = match Spends::::get(0).unwrap().status { + match Spends::::get(0).unwrap().status { PaymentState::Attempted { id, .. } => { T::Paymaster::ensure_concluded(id); - assert_eq!(T::Paymaster::check_payment(id), PaymentStatus::Success); - id }, _ => panic!("No payout attempt made"), }; @@ -263,7 +261,9 @@ mod benchmarks { #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), 0u32); - assert_last_event::(Event::PaymentSucceed { index: 0, payment_id: id }.into()); + if let Some(s) = Spends::::get(0) { + assert!(!matches!(s.status, PaymentState::Attempted { .. })); + } Ok(()) } From 0f490e1d6f59a823370580bf1d870e9b96a78c24 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 3 Jul 2023 12:22:56 +0200 Subject: [PATCH 21/36] benchmarks fix --- bin/node/runtime/src/lib.rs | 2 ++ frame/bounties/src/tests.rs | 4 +++ frame/child-bounties/src/tests.rs | 2 ++ frame/tips/src/tests.rs | 4 +++ frame/treasury/src/benchmarking.rs | 39 ++++++++++++++++++------------ frame/treasury/src/lib.rs | 6 +++++ frame/treasury/src/tests.rs | 2 ++ 7 files changed, 44 insertions(+), 15 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 630eef6c81c52..4128631ecf88b 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1137,6 +1137,8 @@ impl pallet_treasury::Config for Runtime { type Paymaster = PayAssetFromAccount; type BalanceConverter = (); type PayoutPeriod = SpendPayoutPeriod; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_asset_rate::Config for Runtime { diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index b7cbf5dd62042..a444af92f3b9b 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -139,6 +139,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -164,6 +166,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[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 e72938c475f75..e321e8d2fd2ee 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -139,6 +139,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } 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 83ca517cabebc..20feb1d46d161 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -158,6 +158,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } impl pallet_treasury::Config for Test { @@ -183,6 +185,8 @@ impl pallet_treasury::Config for Test { type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 3c1c9dc198ca9..86cbd4b0f7285 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -31,7 +31,26 @@ use frame_support::{ }; use frame_system::RawOrigin; use sp_core::crypto::FromEntropy; -use sp_runtime::traits::TrailingZeroInput; + +/// Trait describing factory functions for dispatchables' parameters +pub trait ArgumentsFactory { + /// Factory function for an asset kind + fn create_asset_kind(seed: u32) -> AssetKind; + /// Factory function for a beneficiary + fn create_beneficiary(seed: [u8; 32]) -> Beneficiary; +} +impl ArgumentsFactory for () +where + AssetKind: FromEntropy, + Beneficiary: FromEntropy, +{ + fn create_asset_kind(seed: u32) -> AssetKind { + AssetKind::from_entropy(&mut seed.encode().as_slice()).unwrap() + } + fn create_beneficiary(seed: [u8; 32]) -> Beneficiary { + Beneficiary::from_entropy(&mut seed.as_slice()).unwrap() + } +} const SEED: u32 = 0; @@ -72,24 +91,14 @@ fn assert_last_event, I: 'static>(generic_event: >:: // Create the arguments for the `spend` dispatchable. fn create_spend_arguments, I: 'static>( seed: u32, -) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) -where - >::AssetKind: FromEntropy, - >::Beneficiary: FromEntropy, -{ - let asset_kind = T::AssetKind::from_entropy(&mut seed.encode().as_slice()).unwrap(); - let beneficiary = - T::Beneficiary::from_entropy(&mut TrailingZeroInput::new(seed.encode().as_slice())) - .unwrap(); +) -> (T::AssetKind, AssetBalanceOf, T::Beneficiary, BeneficiaryLookupOf) { + let asset_kind = T::BenchmarkHelper::create_asset_kind(seed); + let beneficiary = T::BenchmarkHelper::create_beneficiary([seed.try_into().unwrap(); 32]); let beneficiary_lookup = T::BeneficiaryLookup::unlookup(beneficiary.clone()); (asset_kind, 100u32.into(), beneficiary, beneficiary_lookup) } -#[instance_benchmarks( - where - >::AssetKind: FromEntropy, - >::Beneficiary: FromEntropy - )] +#[instance_benchmarks] mod benchmarks { use super::*; diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index cc9c0b049a070..c364ab473e1cc 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -62,6 +62,8 @@ mod benchmarking; #[cfg(test)] mod tests; pub mod weights; +#[cfg(feature = "runtime-benchmarks")] +pub use benchmarking::ArgumentsFactory; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; @@ -264,6 +266,10 @@ pub mod pallet { /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] type PayoutPeriod: Get; + + /// Helper type for benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: ArgumentsFactory; } /// Number of proposals that have been made. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 5c8db7202a4b6..f385819b2682d 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -216,6 +216,8 @@ impl Config for Test { type Paymaster = TestPay; type BalanceConverter = MulBy>; type PayoutPeriod = SpendPayoutPeriod; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } pub fn new_test_ext() -> sp_io::TestExternalities { From 6f78a7f8d5aea826da9c12d3f93d209161bce2ef Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 3 Jul 2023 14:14:24 +0200 Subject: [PATCH 22/36] fix docs --- 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 c364ab473e1cc..099479aec67bc 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -619,7 +619,7 @@ pub mod pallet { /// Propose and approve a spend of treasury funds. /// /// An approved spend must be claimed via the `payout` dispatchable within the - /// [`T::PayoutPeriod`]. + /// [`Config::PayoutPeriod`]. /// /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount`. /// - `asset_kind`: An indicator of the specific asset class to be spent. @@ -686,7 +686,7 @@ pub mod pallet { /// Claim a spend. /// - /// To claim a spend, it must be done within the [`T::PayoutPeriod`] after approval. + /// To claim a spend, it must be done within the [`Config::PayoutPeriod`] after approval. /// In case of a payout failure, the spend status should be updated with the `check_status` /// before retrying with the current function. /// From 2ef382c339525b1271ce7617e38319b80945c1e8 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jul 2023 16:36:06 +0200 Subject: [PATCH 23/36] asset-rate pallet benchmarks asset kind factory --- bin/node/runtime/src/lib.rs | 2 ++ frame/asset-rate/Cargo.toml | 2 +- frame/asset-rate/src/benchmarking.rs | 25 ++++++++++++++++++++----- frame/asset-rate/src/lib.rs | 8 +++++++- frame/asset-rate/src/mock.rs | 2 ++ 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 4128631ecf88b..fe38efc0f88d9 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1150,6 +1150,8 @@ impl pallet_asset_rate::Config for Runtime { type AssetId = u32; type RuntimeEvent = RuntimeEvent; type WeightInfo = pallet_asset_rate::weights::SubstrateWeight; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } parameter_types! { diff --git a/frame/asset-rate/Cargo.toml b/frame/asset-rate/Cargo.toml index 565f658024e6a..215bd25934d52 100644 --- a/frame/asset-rate/Cargo.toml +++ b/frame/asset-rate/Cargo.toml @@ -22,10 +22,10 @@ frame-support = { version = "4.0.0-dev", default-features = false, path = "../su frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } sp-runtime = { version = "24.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } +sp-core = { version = "21.0.0", default-features = false, path = "../../primitives/core" } [dev-dependencies] pallet-balances = { version = "4.0.0-dev", path = "../balances" } -sp-core = { version = "21.0.0", path = "../../primitives/core" } sp-io = { version = "23.0.0", path = "../../primitives/io" } [features] diff --git a/frame/asset-rate/src/benchmarking.rs b/frame/asset-rate/src/benchmarking.rs index 1209f8db6693b..bb474833eab64 100644 --- a/frame/asset-rate/src/benchmarking.rs +++ b/frame/asset-rate/src/benchmarking.rs @@ -20,9 +20,24 @@ use super::*; use crate::{pallet as pallet_asset_rate, Pallet as AssetRate}; +use codec::Encode; use frame_benchmarking::v2::*; use frame_support::assert_ok; use frame_system::RawOrigin; +use sp_core::crypto::FromEntropy; + +/// Trait describing the factory function for AssetKind parameter +pub trait AssetKindFactory { + fn create_asset_kind(seed: u32) -> AssetKind; +} +impl AssetKindFactory for () +where + AssetKind: FromEntropy, +{ + fn create_asset_kind(seed: u32) -> AssetKind { + AssetKind::from_entropy(&mut seed.encode().as_slice()).unwrap() + } +} const ASSET_ID: u32 = 1; @@ -30,13 +45,13 @@ fn default_conversion_rate() -> FixedU128 { FixedU128::from_u32(1u32) } -#[benchmarks(where ::AssetId: From)] +#[benchmarks] mod benchmarks { use super::*; #[benchmark] fn create() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_id: T::AssetId = T::BenchmarkHelper::create_asset_kind(ASSET_ID); #[extrinsic_call] _(RawOrigin::Root, asset_id.clone(), default_conversion_rate()); @@ -49,7 +64,7 @@ mod benchmarks { #[benchmark] fn update() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_id: T::AssetId = T::BenchmarkHelper::create_asset_kind(ASSET_ID); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), asset_id.clone(), @@ -68,10 +83,10 @@ mod benchmarks { #[benchmark] fn remove() -> Result<(), BenchmarkError> { - let asset_id: T::AssetId = ASSET_ID.into(); + let asset_id: T::AssetId = T::BenchmarkHelper::create_asset_kind(ASSET_ID); assert_ok!(AssetRate::::create( RawOrigin::Root.into(), - ASSET_ID.into(), + asset_id.clone(), default_conversion_rate() )); diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index ecc793184094b..e999dcb26fd38 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -69,12 +69,14 @@ pub use pallet::*; pub use weights::WeightInfo; #[cfg(feature = "runtime-benchmarks")] -pub mod benchmarking; +mod benchmarking; #[cfg(test)] mod mock; #[cfg(test)] mod tests; pub mod weights; +#[cfg(feature = "runtime-benchmarks")] +pub use benchmarking::AssetKindFactory; // Type alias for `frame_system`'s account id. type AccountIdOf = ::AccountId; @@ -117,6 +119,10 @@ pub mod pallet { /// The identifier for the class of asset. type AssetId: frame_support::traits::tokens::AssetId; + + /// Helper type for benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: crate::AssetKindFactory; } /// Maps an asset to its fixed point representation in the native balance. diff --git a/frame/asset-rate/src/mock.rs b/frame/asset-rate/src/mock.rs index 2d90fcfecd79e..669a49dfb9190 100644 --- a/frame/asset-rate/src/mock.rs +++ b/frame/asset-rate/src/mock.rs @@ -92,6 +92,8 @@ impl pallet_asset_rate::Config for Test { type Balance = u64; type Currency = Balances; type AssetId = u32; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = (); } // Build genesis storage according to the mock runtime. From 9b21932222119b0b6b2ee3f07a4f49a505a8e49c Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jul 2023 17:01:45 +0200 Subject: [PATCH 24/36] test nits --- frame/bounties/src/tests.rs | 8 ++++---- frame/child-bounties/src/tests.rs | 2 +- frame/tips/src/tests.rs | 6 +++--- frame/treasury/src/tests.rs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index a444af92f3b9b..6bc80379ac0ea 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -33,7 +33,7 @@ use frame_support::{ use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{AccountIdConversion, BadOrigin, BlakeTwo256, IdentityLookup}, + traits::{BadOrigin, BlakeTwo256, IdentityLookup}, BuildStorage, Perbill, Storage, }; @@ -112,8 +112,8 @@ parameter_types! { pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2"); pub static SpendLimit: Balance = u64::MAX; pub static SpendLimit1: Balance = u64::MAX; - pub TreasuryAccount: u128 = TreasuryPalletId::get().into_account_truncating(); - pub TreasuryAccount2: u128 = TreasuryPalletId2::get().into_account_truncating(); + pub TreasuryAccount: u128 = Treasury::account_id(); + pub TreasuryAccount1: u128 = Treasury1::account_id(); } impl pallet_treasury::Config for Test { @@ -163,7 +163,7 @@ impl pallet_treasury::Config for Test { type AssetKind = (); type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; - type Paymaster = PayFromAccount; + type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index e321e8d2fd2ee..5b63f580689a5 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -112,7 +112,7 @@ 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 = TreasuryPalletId::get().into_account_truncating(); + pub TreasuryAccount: u128 = Treasury::account_id(); pub const SpendLimit: Balance = u64::MAX; } diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 20feb1d46d161..8edfe299a4a3a 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -131,8 +131,8 @@ parameter_types! { pub const Burn: Permill = Permill::from_percent(50); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2"); - pub TreasuryAccount: u128 = TreasuryPalletId::get().into_account_truncating(); - pub TreasuryAccount2: u128 = TreasuryPalletId2::get().into_account_truncating(); + pub TreasuryAccount: u128 = Treasury::account_id(); + pub TreasuryAccount1: u128 = Treasury1::account_id(); } impl pallet_treasury::Config for Test { @@ -182,7 +182,7 @@ impl pallet_treasury::Config for Test { type AssetKind = (); type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; - type Paymaster = PayFromAccount; + type Paymaster = PayFromAccount; type BalanceConverter = (); type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index f385819b2682d..6c6f3e9dd75ab 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -163,7 +163,7 @@ 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 = TreasuryPalletId::get().into_account_truncating(); + pub TreasuryAccount: u128 = Treasury::account_id(); pub const SpendPayoutPeriod: u64 = 5; } pub struct TestSpendOrigin; From aa83c6c1ca04ab1f1c3296af06faf48b886b0b23 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jul 2023 18:23:35 +0200 Subject: [PATCH 25/36] asset balance conversion changes --- bin/node/runtime/src/lib.rs | 2 +- frame/asset-rate/src/lib.rs | 4 ++++ frame/bounties/src/tests.rs | 9 ++++++--- frame/child-bounties/src/tests.rs | 7 +++++-- frame/support/src/traits/tokens.rs | 3 ++- frame/support/src/traits/tokens/misc.rs | 13 ++++++++++--- frame/tips/src/tests.rs | 9 ++++++--- frame/treasury/src/benchmarking.rs | 8 +++++++- frame/treasury/src/tests.rs | 2 ++ 9 files changed, 43 insertions(+), 14 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index fe38efc0f88d9..45fcb37ef222e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1135,7 +1135,7 @@ impl pallet_treasury::Config for Runtime { type Beneficiary = AccountId; type BeneficiaryLookup = Indices; type Paymaster = PayAssetFromAccount; - type BalanceConverter = (); + type BalanceConverter = AssetRate; type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index e999dcb26fd38..a2561ca645b95 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -242,4 +242,8 @@ where .ok_or(pallet::Error::::UnknownAssetId.into())?; Ok(rate.saturating_mul_int(balance)) } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(asset_id: AssetIdOf) { + pallet::ConversionRateToNative::::set(asset_id.clone(), Some(1.into())); + } } diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 6bc80379ac0ea..53c4bb6d98194 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -26,7 +26,10 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{tokens::PayFromAccount, ConstU32, ConstU64, OnInitialize}, + traits::{ + tokens::{PayFromAccount, UnityAssetBalanceConversion}, + ConstU32, ConstU64, OnInitialize, + }, PalletId, }; @@ -137,7 +140,7 @@ impl pallet_treasury::Config for Test { type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; - type BalanceConverter = (); + type BalanceConverter = UnityAssetBalanceConversion; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -164,7 +167,7 @@ impl pallet_treasury::Config for Test { type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; - type BalanceConverter = (); + type BalanceConverter = UnityAssetBalanceConversion; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index 5b63f580689a5..7ce68826de443 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -26,7 +26,10 @@ use frame_support::{ assert_noop, assert_ok, pallet_prelude::GenesisBuild, parameter_types, - traits::{tokens::PayFromAccount, ConstU32, ConstU64, OnInitialize}, + traits::{ + tokens::{PayFromAccount, UnityAssetBalanceConversion}, + ConstU32, ConstU64, OnInitialize, + }, weights::Weight, PalletId, }; @@ -137,7 +140,7 @@ impl pallet_treasury::Config for Test { type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; - type BalanceConverter = (); + type BalanceConverter = UnityAssetBalanceConversion; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index 253b49c6671f8..3635311e64357 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -31,6 +31,7 @@ pub mod pay; pub use misc::{ AssetId, Balance, BalanceStatus, ConversionFromAssetBalance, ConversionToAssetBalance, ConvertRank, DepositConsequence, ExistenceRequirement, Fortitude, GetSalary, Locker, Precision, - Preservation, Provenance, Restriction, WithdrawConsequence, WithdrawReasons, + Preservation, Provenance, Restriction, UnityAssetBalanceConversion, WithdrawConsequence, + WithdrawReasons, }; pub use pay::{Pay, PayFromAccount, PaymentStatus}; diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index e7631ee9d70aa..6d648f00254a5 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -260,12 +260,17 @@ pub trait ConversionFromAssetBalance { balance: AssetBalance, asset_id: AssetId, ) -> Result; + /// Ensures that a conversion for the `asset_id` will be successful if done immediately after + /// this call. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(asset_id: AssetId); } -/// Implements the [ConversionFromAssetBalance] trait for a tuple type, enabling a 1:1 conversion of -/// the asset balance value to the balance. +/// Implements the [`ConversionFromAssetBalance`] enabling a 1:1 conversion of the asset balance +/// value to the balance. +pub struct UnityAssetBalanceConversion; impl - ConversionFromAssetBalance for () + ConversionFromAssetBalance for UnityAssetBalanceConversion where AssetBalance: Into, { @@ -273,6 +278,8 @@ where fn from_asset_balance(balance: AssetBalance, _: AssetId) -> Result { Ok(balance.into()) } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: AssetId) {} } /// Trait to handle NFT locking mechanism to ensure interactions with the asset can be implemented diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 8edfe299a4a3a..961ece1ab4829 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -32,7 +32,10 @@ use frame_support::{ pallet_prelude::GenesisBuild, parameter_types, storage::StoragePrefixedMap, - traits::{tokens::PayFromAccount, ConstU32, ConstU64, SortedMembers, StorageVersion}, + traits::{ + tokens::{PayFromAccount, UnityAssetBalanceConversion}, + ConstU32, ConstU64, SortedMembers, StorageVersion, + }, PalletId, }; @@ -156,7 +159,7 @@ impl pallet_treasury::Config for Test { type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; - type BalanceConverter = (); + type BalanceConverter = UnityAssetBalanceConversion; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -183,7 +186,7 @@ impl pallet_treasury::Config for Test { type Beneficiary = Self::AccountId; type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; - type BalanceConverter = (); + type BalanceConverter = UnityAssetBalanceConversion; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 86cbd4b0f7285..7f44097e8200f 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -27,7 +27,10 @@ use frame_benchmarking::{ }; use frame_support::{ ensure, - traits::{tokens::PaymentStatus, EnsureOrigin, OnInitialize}, + traits::{ + tokens::{ConversionFromAssetBalance, PaymentStatus}, + EnsureOrigin, OnInitialize, + }, }; use frame_system::RawOrigin; use sp_core::crypto::FromEntropy; @@ -212,6 +215,7 @@ mod benchmarks { T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); + T::BalanceConverter::ensure_successful(asset_kind.clone()); #[extrinsic_call] _(origin as T::RuntimeOrigin, asset_kind.clone(), amount, beneficiary_lookup); @@ -231,6 +235,7 @@ mod benchmarks { T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); + T::BalanceConverter::ensure_successful(asset_kind.clone()); Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); @@ -256,6 +261,7 @@ mod benchmarks { T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); + T::BalanceConverter::ensure_successful(asset_kind.clone()); Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 6c6f3e9dd75ab..1e4977afebd9f 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -191,6 +191,8 @@ impl> ConversionFromAssetBalance for MulBy { fn from_asset_balance(balance: u64, _asset_id: u32) -> Result { return balance.checked_mul(N::get()).ok_or(()) } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: AssetId) {} } impl Config for Test { From 727e309601463a5b0a92785c180b2b2a483e39de Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 4 Jul 2023 18:33:38 +0200 Subject: [PATCH 26/36] fix tests --- frame/treasury/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 1e4977afebd9f..9c1cef8b4d11f 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -192,7 +192,7 @@ impl> ConversionFromAssetBalance for MulBy { return balance.checked_mul(N::get()).ok_or(()) } #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: AssetId) {} + fn ensure_successful(_: u32) {} } impl Config for Test { From 7c1d3f942998c67b91a5882eed5e2ac3a82232dd Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 7 Jul 2023 13:21:08 +0200 Subject: [PATCH 27/36] docs fixes --- frame/asset-rate/src/lib.rs | 1 + frame/treasury/src/benchmarking.rs | 8 +++++--- frame/treasury/src/lib.rs | 11 +++++++---- primitives/core/src/crypto.rs | 2 ++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index a2561ca645b95..97ce80c896e55 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -242,6 +242,7 @@ where .ok_or(pallet::Error::::UnknownAssetId.into())?; Ok(rate.saturating_mul_int(balance)) } + /// Set a conversion rate to `1` for the `asset_id`. #[cfg(feature = "runtime-benchmarks")] fn ensure_successful(asset_id: AssetIdOf) { pallet::ConversionRateToNative::::set(asset_id.clone(), Some(1.into())); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 7f44097e8200f..760547c7d690d 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -35,13 +35,15 @@ use frame_support::{ use frame_system::RawOrigin; use sp_core::crypto::FromEntropy; -/// Trait describing factory functions for dispatchables' parameters +/// Trait describing factory functions for dispatchables' parameters. pub trait ArgumentsFactory { - /// Factory function for an asset kind + /// Factory function for an asset kind. fn create_asset_kind(seed: u32) -> AssetKind; - /// Factory function for a beneficiary + /// Factory function for a beneficiary. fn create_beneficiary(seed: [u8; 32]) -> Beneficiary; } + +/// Implementation that expects the parameters implement the [`FromEntropy`] trait. impl ArgumentsFactory for () where AssetKind: FromEntropy, diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 099479aec67bc..ee830f0de6696 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -240,8 +240,8 @@ pub mod pallet { 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. + /// process. The `Success` value is the maximum amount in a native asset that this origin + /// is allowed to spend at a time. type SpendOrigin: EnsureOrigin>; /// Type parameter representing the asset kinds to be spent from the treasury. @@ -256,7 +256,9 @@ pub mod pallet { /// Type for processing spends of [Self::AssetKind] in favor of [`Self::Beneficiary`]. type Paymaster: Pay; - /// Type to convert the balance of an [`Self::AssetKind`] to balance of the native asset. + /// Type for converting the balance of an [Self::AssetKind] to the balance of the native + /// asset, solely for the purpose of asserting the result against the maximum allowed spend + /// amount of the [`Self::SpendOrigin`]. type BalanceConverter: ConversionFromAssetBalance< ::Balance, Self::AssetKind, @@ -621,7 +623,8 @@ pub mod pallet { /// An approved spend must be claimed via the `payout` dispatchable within the /// [`Config::PayoutPeriod`]. /// - /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount`. + /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount` of + /// `asset_kind` in the native asset. /// - `asset_kind`: An indicator of the specific asset class to be spent. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index fd39efa75e065..f932fb5563faf 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -632,6 +632,7 @@ impl sp_std::str::FromStr for AccountId32 { } } +/// Creates an [`AccountId32`] from the input, which should contain at least 32 bytes. impl FromEntropy for AccountId32 { fn from_entropy(input: &mut impl codec::Input) -> Result { Ok(AccountId32::new(FromEntropy::from_entropy(input)?)) @@ -1177,6 +1178,7 @@ impl FromEntropy for bool { } } +/// Create the unit type for any given input. impl FromEntropy for () { fn from_entropy(_: &mut impl codec::Input) -> Result { Ok(()) From 6324f428459ddf118bf13b389de3095fa28792fe Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 8 Jul 2023 11:02:45 +0200 Subject: [PATCH 28/36] spend valid from, void spend --- bin/node/runtime/src/lib.rs | 1 + frame/bounties/src/tests.rs | 2 + frame/child-bounties/src/tests.rs | 1 + frame/tips/src/tests.rs | 2 + frame/treasury/src/benchmarking.rs | 34 ++-- frame/treasury/src/lib.rs | 46 +++++- frame/treasury/src/tests.rs | 77 +++++++-- frame/treasury/src/weights.rs | 241 ++++++++++++++++------------- 8 files changed, 263 insertions(+), 141 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 239789d519ef6..95c80d8cea625 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1143,6 +1143,7 @@ impl pallet_treasury::Config for Runtime { type BeneficiaryLookup = Indices; type Paymaster = PayAssetFromAccount; type BalanceConverter = AssetRate; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 53c4bb6d98194..62c13cc25be0a 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -141,6 +141,7 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -168,6 +169,7 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index 7ce68826de443..c72f223a6d2b0 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -141,6 +141,7 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index 961ece1ab4829..a3b2823227823 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -160,6 +160,7 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -187,6 +188,7 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 760547c7d690d..edce6f15399ee 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -220,12 +220,11 @@ mod benchmarks { T::BalanceConverter::ensure_successful(asset_kind.clone()); #[extrinsic_call] - _(origin as T::RuntimeOrigin, asset_kind.clone(), amount, beneficiary_lookup); + _(origin as T::RuntimeOrigin, asset_kind.clone(), amount, beneficiary_lookup, None); - let expire_at = - frame_system::Pallet::::block_number().saturating_add(T::PayoutPeriod::get()); + let valid_from = frame_system::Pallet::::block_number(); assert_last_event::( - Event::AssetSpendApproved { index: 0, asset_kind, amount, beneficiary, expire_at } + Event::AssetSpendApproved { index: 0, asset_kind, amount, beneficiary, valid_from } .into(), ); Ok(()) @@ -233,12 +232,11 @@ mod benchmarks { #[benchmark] fn payout() -> Result<(), BenchmarkError> { - let origin = - T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::SpendOrigin::try_successful_origin().map_err(|_| "No origin")?; let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); T::BalanceConverter::ensure_successful(asset_kind.clone()); - Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; + Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup, None)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); @@ -259,12 +257,11 @@ mod benchmarks { #[benchmark] fn check_status() -> Result<(), BenchmarkError> { - let origin = - T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::SpendOrigin::try_successful_origin().map_err(|_| "No origin")?; let (asset_kind, amount, beneficiary, beneficiary_lookup) = create_spend_arguments::(SEED); T::BalanceConverter::ensure_successful(asset_kind.clone()); - Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup)?; + Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup, None)?; T::Paymaster::ensure_successful(&beneficiary, asset_kind, amount); let caller: T::AccountId = account("caller", 0, SEED); Treasury::::payout(RawOrigin::Signed(caller.clone()).into(), 0u32)?; @@ -284,5 +281,22 @@ mod benchmarks { Ok(()) } + #[benchmark] + fn void_spend() -> Result<(), BenchmarkError> { + let origin = T::SpendOrigin::try_successful_origin().map_err(|_| "No origin")?; + let (asset_kind, amount, _, beneficiary_lookup) = create_spend_arguments::(SEED); + T::BalanceConverter::ensure_successful(asset_kind.clone()); + Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup, None)?; + assert!(Spends::::get(0).is_some()); + let origin = + T::VoidOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, 0u32); + + assert!(Spends::::get(0).is_none()); + Ok(()) + } + 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 ee830f0de6696..73615e77dabbc 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -159,8 +159,8 @@ pub struct SpendStatus, } @@ -265,6 +265,9 @@ pub mod pallet { BalanceOf, >; + /// Origin from which an approved spend can be voided. + type VoidOrigin: EnsureOrigin; + /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] type PayoutPeriod: Get; @@ -381,8 +384,10 @@ pub mod pallet { asset_kind: T::AssetKind, amount: AssetBalanceOf, beneficiary: T::Beneficiary, - expire_at: T::BlockNumber, + valid_from: T::BlockNumber, }, + /// An approved spend was voided. + AssetSpendVoided { index: SpendIndex }, /// A payment happened. Paid { index: SpendIndex, payment_id: ::Id }, /// A payment succeed, the spend removed from the storage. @@ -409,6 +414,8 @@ pub mod pallet { FailedToConvertBalance, /// The spend has expired and cannot be claimed. SpendExpired, + /// The spend is not yet eligible for payout. + EarlyPayout, /// The payment has already been attempted. AlreadyAttempted, /// There was some issue with the mechanism of payment. @@ -628,6 +635,8 @@ pub mod pallet { /// - `asset_kind`: An indicator of the specific asset class to be spent. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. + /// - `valid_from`: The block number from which the spend can be claimed. If `None`, the + /// spend can be claimed immediately after approval. #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::spend())] pub fn spend( @@ -635,6 +644,7 @@ pub mod pallet { asset_kind: T::AssetKind, #[pallet::compact] amount: AssetBalanceOf, beneficiary: BeneficiaryLookupOf, + valid_from: Option, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; let beneficiary = T::BeneficiaryLookup::lookup(beneficiary)?; @@ -663,15 +673,14 @@ pub mod pallet { .unwrap_or(Ok(()))?; let index = SpendCount::::get(); - let expire_at = - frame_system::Pallet::::block_number().saturating_add(T::PayoutPeriod::get()); + let valid_from = valid_from.unwrap_or(frame_system::Pallet::::block_number()); Spends::::insert( index, SpendStatus { asset_kind: asset_kind.clone(), amount, beneficiary: beneficiary.clone(), - expire_at, + valid_from, status: PaymentState::Pending, }, ); @@ -682,7 +691,7 @@ pub mod pallet { asset_kind, amount, beneficiary, - expire_at, + valid_from, }); Ok(()) } @@ -700,8 +709,10 @@ pub mod pallet { pub fn payout(origin: OriginFor, index: SpendIndex) -> DispatchResult { ensure_signed(origin)?; let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; + let now = frame_system::Pallet::::block_number(); + ensure!(now >= spend.valid_from, Error::::EarlyPayout); ensure!( - spend.expire_at > frame_system::Pallet::::block_number(), + spend.valid_from.saturating_add(T::PayoutPeriod::get()) > now, Error::::SpendExpired ); ensure!( @@ -751,6 +762,25 @@ pub mod pallet { } Ok(()) } + + /// Void previously approved spend. + /// + /// - `origin`: Must be `T::VoidOrigin`. + /// - `index`: The spend index. + #[pallet::call_index(8)] + #[pallet::weight(T::WeightInfo::void_spend())] + pub fn void_spend(origin: OriginFor, index: SpendIndex) -> DispatchResult { + T::VoidOrigin::ensure_origin(origin)?; + let spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; + ensure!( + matches!(spend.status, PaymentState::Pending | PaymentState::Failed), + Error::::AlreadyAttempted + ); + + Spends::::remove(index); + Self::deposit_event(Event::::AssetSpendVoided { index }); + Ok(()) + } } } diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 9c1cef8b4d11f..a51c9424aa120 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -217,6 +217,7 @@ impl Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = TestPay; type BalanceConverter = MulBy>; + type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -596,12 +597,14 @@ fn spending_in_batch_respects_max_total() { RuntimeCall::from(TreasuryCall::spend { asset_kind: 1, amount: 1, - beneficiary: 100 + beneficiary: 100, + valid_from: None, }), RuntimeCall::from(TreasuryCall::spend { asset_kind: 1, amount: 1, - beneficiary: 101 + beneficiary: 101, + valid_from: None, }) ] }) @@ -613,12 +616,14 @@ fn spending_in_batch_respects_max_total() { RuntimeCall::from(TreasuryCall::spend { asset_kind: 1, amount: 2, - beneficiary: 100 + beneficiary: 100, + valid_from: None, }), RuntimeCall::from(TreasuryCall::spend { asset_kind: 1, amount: 2, - beneficiary: 101 + beneficiary: 101, + valid_from: None, }) ] }) @@ -631,20 +636,20 @@ fn spending_in_batch_respects_max_total() { #[test] fn spend_origin_works() { new_test_ext().execute_with(|| { - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 1, 6)); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 1, 6, None)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(10), 1, 3, 6), + Treasury::spend(RuntimeOrigin::signed(10), 1, 3, 6, None), Error::::InsufficientPermission ); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 1, 5, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(11), 1, 5, 6, None)); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(11), 1, 6, 6), + Treasury::spend(RuntimeOrigin::signed(11), 1, 6, 6, None), Error::::InsufficientPermission ); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 1, 10, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(12), 1, 10, 6, None)); assert_noop!( - Treasury::spend(RuntimeOrigin::signed(12), 1, 11, 6), + Treasury::spend(RuntimeOrigin::signed(12), 1, 11, 6, None), Error::::InsufficientPermission ); @@ -657,7 +662,7 @@ fn spend_origin_works() { fn spend_works() { new_test_ext().execute_with(|| { System::set_block_number(1); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); assert_eq!(SpendCount::::get(), 1); assert_eq!( @@ -666,7 +671,7 @@ fn spend_works() { asset_kind: 1, amount: 2, beneficiary: 6, - expire_at: 6, + valid_from: 1, status: PaymentState::Pending, } ); @@ -676,7 +681,7 @@ fn spend_works() { asset_kind: 1, amount: 2, beneficiary: 6, - expire_at: 6, + valid_from: 1, } .into(), ); @@ -687,7 +692,7 @@ fn spend_works() { fn spend_expires() { new_test_ext().execute_with(|| { System::set_block_number(1); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); System::set_block_number(6); assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::SpendExpired); }); @@ -697,7 +702,7 @@ fn spend_expires() { fn payout_works() { new_test_ext().execute_with(|| { System::set_block_number(1); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); assert_eq!(paid(6, 1), 2); assert_eq!(SpendCount::::get(), 1); @@ -720,7 +725,7 @@ fn payout_works() { fn payout_retry_works() { new_test_ext().execute_with(|| { System::set_block_number(1); - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6)); + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); assert_eq!(paid(6, 1), 2); let spend = Spends::::get(0).unwrap(); @@ -745,3 +750,41 @@ fn payout_retry_works() { assert_eq!(paid(6, 1), 2); }); } + +#[test] +fn spend_valid_from_works() { + new_test_ext().execute_with(|| { + assert_eq!(::PayoutPeriod::get(), 5); + System::set_block_number(1); + // spend valid from block `2`. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(2))); + assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::EarlyPayout); + System::set_block_number(2); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + + System::set_block_number(5); + // spend valid from block `0`. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(0))); + // spend expired. + assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 1), Error::::SpendExpired); + }); +} + +#[test] +fn void_spend_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + // spend cannot be voided if already attempted. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(1))); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + assert_noop!( + Treasury::void_spend(RuntimeOrigin::root(), 0), + Error::::AlreadyAttempted + ); + + // void spend. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(10))); + assert_ok!(Treasury::void_spend(RuntimeOrigin::root(), 1)); + assert_eq!(Spends::::get(1), None); + }); +} diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index 4b11a5f99570a..030e18980eb54 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_treasury //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-06-30, STEPS: `50`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-07-07, STEPS: `20`, REPEAT: `2`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `cob`, CPU: `` //! EXECUTION: None, WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 @@ -28,7 +28,7 @@ // benchmark // pallet // --chain=dev -// --steps=50 +// --steps=20 // --repeat=2 // --pallet=pallet-treasury // --extrinsic=* @@ -56,118 +56,122 @@ pub trait WeightInfo { fn spend() -> Weight; fn payout() -> Weight; fn check_status() -> Weight; + fn void_spend() -> Weight; } /// Weights for pallet_treasury using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) fn spend_local() -> Weight { // Proof Size summary in bytes: // Measured: `76` - // Estimated: `1561` - // Minimum execution time: 70_000_000 picoseconds. - Weight::from_parts(78_000_000, 1561) + // Estimated: `1887` + // Minimum execution time: 179_000_000 picoseconds. + Weight::from_parts(190_000_000, 1887) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) fn propose_spend() -> Weight { // Proof Size summary in bytes: // Measured: `177` - // Estimated: `1662` - // Minimum execution time: 121_000_000 picoseconds. - Weight::from_parts(166_000_000, 1662) + // Estimated: `1489` + // Minimum execution time: 349_000_000 picoseconds. + Weight::from_parts(398_000_000, 1489) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn reject_proposal() -> Weight { // Proof Size summary in bytes: // Measured: `335` - // Estimated: `3800` - // Minimum execution time: 159_000_000 picoseconds. - Weight::from_parts(164_000_000, 3800) + // Estimated: `3593` + // Minimum execution time: 367_000_000 picoseconds. + Weight::from_parts(388_000_000, 3593) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:0) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) /// The range of component `p` is `[0, 99]`. fn approve_proposal(p: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `504 + p * (8 ±0)` - // Estimated: `3922 + p * (9 ±0)` - // Minimum execution time: 50_000_000 picoseconds. - Weight::from_parts(94_256_431, 3922) + // Measured: `483 + p * (9 ±0)` + // Estimated: `3573` + // Minimum execution time: 111_000_000 picoseconds. + Weight::from_parts(108_813_243, 3573) + // Standard Error: 147_887 + .saturating_add(Weight::from_parts(683_216, 0).saturating_mul(p.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 9).saturating_mul(p.into())) } /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) fn remove_approval() -> Weight { // Proof Size summary in bytes: // Measured: `161` - // Estimated: `1646` - // Minimum execution time: 40_000_000 picoseconds. - Weight::from_parts(43_000_000, 1646) + // Estimated: `1887` + // Minimum execution time: 71_000_000 picoseconds. + Weight::from_parts(78_000_000, 1887) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: Treasury Deactivated (r:1 w:1) - /// Proof Skipped: Treasury Deactivated (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) - /// Storage: Treasury Proposals (r:100 w:100) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) - /// Storage: System Account (r:200 w:200) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Storage: Treasury Proposals (r:99 w:99) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Storage: System Account (r:198 w:198) /// 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]`. + /// The range of component `p` is `[0, 99]`. fn on_initialize_proposals(p: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `421 + p * (251 ±0)` - // Estimated: `1894 + p * (5206 ±0)` - // Minimum execution time: 167_000_000 picoseconds. - Weight::from_parts(451_684_629, 1894) - // Standard Error: 1_698_594 - .saturating_add(Weight::from_parts(164_282_216, 0).saturating_mul(p.into())) + // Measured: `427 + p * (251 ±0)` + // Estimated: `1887 + p * (5206 ±0)` + // Minimum execution time: 614_000_000 picoseconds. + Weight::from_parts(498_501_558, 1887) + // Standard Error: 1_070_260 + .saturating_add(Weight::from_parts(599_011_690, 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, 5206).saturating_mul(p.into())) } + /// Storage: AssetRate ConversionRateToNative (r:1 w:0) + /// Proof: AssetRate ConversionRateToNative (max_values: None, max_size: Some(36), added: 2511, mode: MaxEncodedLen) /// Storage: Treasury SpendCount (r:1 w:1) - /// Proof Skipped: Treasury SpendCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury SpendCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Spends (r:0 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) fn spend() -> Weight { // Proof Size summary in bytes: - // Measured: `76` - // Estimated: `1561` - // Minimum execution time: 56_000_000 picoseconds. - Weight::from_parts(57_000_000, 1561) - .saturating_add(T::DbWeight::get().reads(1_u64)) + // Measured: `140` + // Estimated: `3501` + // Minimum execution time: 214_000_000 picoseconds. + Weight::from_parts(216_000_000, 3501) + .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } /// Storage: Treasury Spends (r:1 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) /// Storage: Assets Asset (r:1 w:1) /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) /// Storage: Assets Account (r:2 w:2) @@ -178,19 +182,30 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `705` // Estimated: `6208` - // Minimum execution time: 268_000_000 picoseconds. - Weight::from_parts(360_000_000, 6208) + // Minimum execution time: 760_000_000 picoseconds. + Weight::from_parts(822_000_000, 6208) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().writes(5_u64)) } /// Storage: Treasury Spends (r:1 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) fn check_status() -> Weight { // Proof Size summary in bytes: // Measured: `194` - // Estimated: `3659` - // Minimum execution time: 61_000_000 picoseconds. - Weight::from_parts(65_000_000, 3659) + // Estimated: `3534` + // Minimum execution time: 153_000_000 picoseconds. + Weight::from_parts(160_000_000, 3534) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) + fn void_spend() -> Weight { + // Proof Size summary in bytes: + // Measured: `194` + // Estimated: `3534` + // Minimum execution time: 147_000_000 picoseconds. + Weight::from_parts(181_000_000, 3534) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -199,112 +214,115 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) fn spend_local() -> Weight { // Proof Size summary in bytes: // Measured: `76` - // Estimated: `1561` - // Minimum execution time: 70_000_000 picoseconds. - Weight::from_parts(78_000_000, 1561) + // Estimated: `1887` + // Minimum execution time: 179_000_000 picoseconds. + Weight::from_parts(190_000_000, 1887) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } /// Storage: Treasury ProposalCount (r:1 w:1) - /// Proof Skipped: Treasury ProposalCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury ProposalCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Proposals (r:0 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) fn propose_spend() -> Weight { // Proof Size summary in bytes: // Measured: `177` - // Estimated: `1662` - // Minimum execution time: 121_000_000 picoseconds. - Weight::from_parts(166_000_000, 1662) + // Estimated: `1489` + // Minimum execution time: 349_000_000 picoseconds. + Weight::from_parts(398_000_000, 1489) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:1) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) /// Storage: System Account (r:1 w:1) /// Proof: System Account (max_values: None, max_size: Some(128), added: 2603, mode: MaxEncodedLen) fn reject_proposal() -> Weight { // Proof Size summary in bytes: // Measured: `335` - // Estimated: `3800` - // Minimum execution time: 159_000_000 picoseconds. - Weight::from_parts(164_000_000, 3800) + // Estimated: `3593` + // Minimum execution time: 367_000_000 picoseconds. + Weight::from_parts(388_000_000, 3593) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: Treasury Proposals (r:1 w:0) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) /// The range of component `p` is `[0, 99]`. fn approve_proposal(p: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `504 + p * (8 ±0)` - // Estimated: `3922 + p * (9 ±0)` - // Minimum execution time: 50_000_000 picoseconds. - Weight::from_parts(94_256_431, 3922) + // Measured: `483 + p * (9 ±0)` + // Estimated: `3573` + // Minimum execution time: 111_000_000 picoseconds. + Weight::from_parts(108_813_243, 3573) + // Standard Error: 147_887 + .saturating_add(Weight::from_parts(683_216, 0).saturating_mul(p.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) - .saturating_add(Weight::from_parts(0, 9).saturating_mul(p.into())) } /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) fn remove_approval() -> Weight { // Proof Size summary in bytes: // Measured: `161` - // Estimated: `1646` - // Minimum execution time: 40_000_000 picoseconds. - Weight::from_parts(43_000_000, 1646) + // Estimated: `1887` + // Minimum execution time: 71_000_000 picoseconds. + Weight::from_parts(78_000_000, 1887) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: Treasury Deactivated (r:1 w:1) - /// Proof Skipped: Treasury Deactivated (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury Deactivated (max_values: Some(1), max_size: Some(16), added: 511, mode: MaxEncodedLen) /// Storage: Treasury Approvals (r:1 w:1) - /// Proof Skipped: Treasury Approvals (max_values: Some(1), max_size: None, mode: Measured) - /// Storage: Treasury Proposals (r:100 w:100) - /// Proof Skipped: Treasury Proposals (max_values: None, max_size: None, mode: Measured) - /// Storage: System Account (r:200 w:200) + /// Proof: Treasury Approvals (max_values: Some(1), max_size: Some(402), added: 897, mode: MaxEncodedLen) + /// Storage: Treasury Proposals (r:99 w:99) + /// Proof: Treasury Proposals (max_values: None, max_size: Some(108), added: 2583, mode: MaxEncodedLen) + /// Storage: System Account (r:198 w:198) /// 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]`. + /// The range of component `p` is `[0, 99]`. fn on_initialize_proposals(p: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `421 + p * (251 ±0)` - // Estimated: `1894 + p * (5206 ±0)` - // Minimum execution time: 167_000_000 picoseconds. - Weight::from_parts(451_684_629, 1894) - // Standard Error: 1_698_594 - .saturating_add(Weight::from_parts(164_282_216, 0).saturating_mul(p.into())) + // Measured: `427 + p * (251 ±0)` + // Estimated: `1887 + p * (5206 ±0)` + // Minimum execution time: 614_000_000 picoseconds. + Weight::from_parts(498_501_558, 1887) + // Standard Error: 1_070_260 + .saturating_add(Weight::from_parts(599_011_690, 0).saturating_mul(p.into())) .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, 5206).saturating_mul(p.into())) } + /// Storage: AssetRate ConversionRateToNative (r:1 w:0) + /// Proof: AssetRate ConversionRateToNative (max_values: None, max_size: Some(36), added: 2511, mode: MaxEncodedLen) /// Storage: Treasury SpendCount (r:1 w:1) - /// Proof Skipped: Treasury SpendCount (max_values: Some(1), max_size: None, mode: Measured) + /// Proof: Treasury SpendCount (max_values: Some(1), max_size: Some(4), added: 499, mode: MaxEncodedLen) /// Storage: Treasury Spends (r:0 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) fn spend() -> Weight { // Proof Size summary in bytes: - // Measured: `76` - // Estimated: `1561` - // Minimum execution time: 56_000_000 picoseconds. - Weight::from_parts(57_000_000, 1561) - .saturating_add(RocksDbWeight::get().reads(1_u64)) + // Measured: `140` + // Estimated: `3501` + // Minimum execution time: 214_000_000 picoseconds. + Weight::from_parts(216_000_000, 3501) + .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } /// Storage: Treasury Spends (r:1 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) /// Storage: Assets Asset (r:1 w:1) /// Proof: Assets Asset (max_values: None, max_size: Some(210), added: 2685, mode: MaxEncodedLen) /// Storage: Assets Account (r:2 w:2) @@ -315,19 +333,30 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `705` // Estimated: `6208` - // Minimum execution time: 268_000_000 picoseconds. - Weight::from_parts(360_000_000, 6208) + // Minimum execution time: 760_000_000 picoseconds. + Weight::from_parts(822_000_000, 6208) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().writes(5_u64)) } /// Storage: Treasury Spends (r:1 w:1) - /// Proof Skipped: Treasury Spends (max_values: None, max_size: None, mode: Measured) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) fn check_status() -> Weight { // Proof Size summary in bytes: // Measured: `194` - // Estimated: `3659` - // Minimum execution time: 61_000_000 picoseconds. - Weight::from_parts(65_000_000, 3659) + // Estimated: `3534` + // Minimum execution time: 153_000_000 picoseconds. + Weight::from_parts(160_000_000, 3534) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) + } + /// Storage: Treasury Spends (r:1 w:1) + /// Proof: Treasury Spends (max_values: None, max_size: Some(69), added: 2544, mode: MaxEncodedLen) + fn void_spend() -> Weight { + // Proof Size summary in bytes: + // Measured: `194` + // Estimated: `3534` + // Minimum execution time: 147_000_000 picoseconds. + Weight::from_parts(181_000_000, 3534) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } From 51492eaab392af6688c52bbeda38f4b86be6623a Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 8 Jul 2023 12:01:28 +0200 Subject: [PATCH 29/36] use reject origin --- bin/node/runtime/src/lib.rs | 1 - frame/bounties/src/tests.rs | 2 -- frame/child-bounties/src/tests.rs | 1 - frame/tips/src/tests.rs | 2 -- frame/treasury/src/benchmarking.rs | 2 +- frame/treasury/src/lib.rs | 7 ++----- frame/treasury/src/tests.rs | 1 - 7 files changed, 3 insertions(+), 13 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 95c80d8cea625..239789d519ef6 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1143,7 +1143,6 @@ impl pallet_treasury::Config for Runtime { type BeneficiaryLookup = Indices; type Paymaster = PayAssetFromAccount; type BalanceConverter = AssetRate; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index 62c13cc25be0a..53c4bb6d98194 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -141,7 +141,6 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -169,7 +168,6 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/child-bounties/src/tests.rs b/frame/child-bounties/src/tests.rs index c72f223a6d2b0..7ce68826de443 100644 --- a/frame/child-bounties/src/tests.rs +++ b/frame/child-bounties/src/tests.rs @@ -141,7 +141,6 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index a3b2823227823..961ece1ab4829 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -160,7 +160,6 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); @@ -188,7 +187,6 @@ impl pallet_treasury::Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = PayFromAccount; type BalanceConverter = UnityAssetBalanceConversion; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = ConstU64<10>; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index edce6f15399ee..61510e9a3024b 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -289,7 +289,7 @@ mod benchmarks { Treasury::::spend(origin, asset_kind.clone(), amount, beneficiary_lookup, None)?; assert!(Spends::::get(0).is_some()); let origin = - T::VoidOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] _(origin as T::RuntimeOrigin, 0u32); diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 73615e77dabbc..9cce8d49b2a87 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -265,9 +265,6 @@ pub mod pallet { BalanceOf, >; - /// Origin from which an approved spend can be voided. - type VoidOrigin: EnsureOrigin; - /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] type PayoutPeriod: Get; @@ -765,12 +762,12 @@ pub mod pallet { /// Void previously approved spend. /// - /// - `origin`: Must be `T::VoidOrigin`. + /// - `origin`: Must be `T::RejectOrigin`. /// - `index`: The spend index. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::void_spend())] pub fn void_spend(origin: OriginFor, index: SpendIndex) -> DispatchResult { - T::VoidOrigin::ensure_origin(origin)?; + T::RejectOrigin::ensure_origin(origin)?; let spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; ensure!( matches!(spend.status, PaymentState::Pending | PaymentState::Failed), diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index a51c9424aa120..076ffac1bd0c4 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -217,7 +217,6 @@ impl Config for Test { type BeneficiaryLookup = IdentityLookup; type Paymaster = TestPay; type BalanceConverter = MulBy>; - type VoidOrigin = frame_system::EnsureRoot; type PayoutPeriod = SpendPayoutPeriod; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = (); From 84d04515d95df697147f4db9f6060b49e58c4f43 Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 8 Jul 2023 12:32:01 +0200 Subject: [PATCH 30/36] store expire_at --- frame/treasury/src/benchmarking.rs | 12 ++++++++++-- frame/treasury/src/lib.rs | 11 +++++++---- frame/treasury/src/tests.rs | 2 ++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 61510e9a3024b..8ed62a0bbc0a9 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -223,9 +223,17 @@ mod benchmarks { _(origin as T::RuntimeOrigin, asset_kind.clone(), amount, beneficiary_lookup, None); let valid_from = frame_system::Pallet::::block_number(); + let expire_at = valid_from.saturating_add(T::PayoutPeriod::get()); assert_last_event::( - Event::AssetSpendApproved { index: 0, asset_kind, amount, beneficiary, valid_from } - .into(), + Event::AssetSpendApproved { + index: 0, + asset_kind, + amount, + beneficiary, + valid_from, + expire_at, + } + .into(), ); Ok(()) } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 9cce8d49b2a87..7f8b730bbe9b1 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -161,6 +161,8 @@ pub struct SpendStatus, } @@ -382,6 +384,7 @@ pub mod pallet { amount: AssetBalanceOf, beneficiary: T::Beneficiary, valid_from: T::BlockNumber, + expire_at: T::BlockNumber, }, /// An approved spend was voided. AssetSpendVoided { index: SpendIndex }, @@ -671,6 +674,7 @@ pub mod pallet { let index = SpendCount::::get(); let valid_from = valid_from.unwrap_or(frame_system::Pallet::::block_number()); + let expire_at = valid_from.saturating_add(T::PayoutPeriod::get()); Spends::::insert( index, SpendStatus { @@ -678,6 +682,7 @@ pub mod pallet { amount, beneficiary: beneficiary.clone(), valid_from, + expire_at, status: PaymentState::Pending, }, ); @@ -689,6 +694,7 @@ pub mod pallet { amount, beneficiary, valid_from, + expire_at, }); Ok(()) } @@ -708,10 +714,7 @@ pub mod pallet { let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; let now = frame_system::Pallet::::block_number(); ensure!(now >= spend.valid_from, Error::::EarlyPayout); - ensure!( - spend.valid_from.saturating_add(T::PayoutPeriod::get()) > now, - Error::::SpendExpired - ); + ensure!(spend.expire_at > now, Error::::SpendExpired); ensure!( matches!(spend.status, PaymentState::Pending | PaymentState::Failed), Error::::AlreadyAttempted diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 076ffac1bd0c4..cd538c4301e7f 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -671,6 +671,7 @@ fn spend_works() { amount: 2, beneficiary: 6, valid_from: 1, + expire_at: 6, status: PaymentState::Pending, } ); @@ -681,6 +682,7 @@ fn spend_works() { amount: 2, beneficiary: 6, valid_from: 1, + expire_at: 6, } .into(), ); From dfe7502b64859e20387d4a8a5d0238a3b143816f Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 10 Jul 2023 18:09:00 +0200 Subject: [PATCH 31/36] docs according to guidelines --- Cargo.lock | 1 + frame/treasury/Cargo.toml | 2 + frame/treasury/src/lib.rs | 204 +++++++++++++++++++++++++++--------- frame/treasury/src/tests.rs | 16 ++- 4 files changed, 171 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 19cc8cd781a71..2582927b6a2aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7585,6 +7585,7 @@ dependencies = [ name = "pallet-treasury" version = "4.0.0-dev" dependencies = [ + "docify", "frame-benchmarking", "frame-support", "frame-system", diff --git a/frame/treasury/Cargo.toml b/frame/treasury/Cargo.toml index 33aa5338ba147..88c858368708d 100644 --- a/frame/treasury/Cargo.toml +++ b/frame/treasury/Cargo.toml @@ -28,6 +28,8 @@ sp-runtime = { version = "24.0.0", default-features = false, path = "../../primi sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } sp-core = { version = "21.0.0", default-features = false, path = "../../primitives/core" } +docify = "0.1.13" + [dev-dependencies] sp-io = { version = "23.0.0", path = "../../primitives/io" } pallet-utility = { version = "4.0.0-dev", path = "../utility" } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7f8b730bbe9b1..3e01a15c9b819 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -15,46 +15,60 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! > Made with *Substrate*, for *Polkadot*. +//! +//! [![github]](https://github.com/paritytech/substrate/frame/fast-unstake) - +//! [![polkadot]](https://polkadot.network) +//! +//! [polkadot]: https://img.shields.io/badge/polkadot-E6007A?style=for-the-badge&logo=polkadot&logoColor=white +//! [github]: https://img.shields.io/badge/github-8da0cb?style=for-the-badge&labelColor=555555&logo=github +//! //! # Treasury Pallet //! //! The Treasury pallet provides a "pot" of funds that can be managed by stakeholders in the system //! and a structure for making spending proposals from this pot. //! -//! - [`Config`] -//! - [`Call`] -//! //! ## Overview //! //! The Treasury Pallet itself provides the pot to store funds, and a means for stakeholders to -//! propose, approve, and deny expenditures. The chain will need to provide a method (e.g. -//! inflation, fees) for collecting funds. +//! propose and claim expenditures (aka spends). The chain will need to provide a method to approve +//! spends (e.g. public referendum) and a method for collecting funds (e.g. inflation, fees). //! -//! By way of example, the Council could vote to fund the Treasury with a portion of the block +//! By way of example, stakeholders could vote to fund the Treasury with a portion of the block //! reward and use the funds to pay developers. //! -//! //! ### Terminology //! //! - **Proposal:** A suggestion to allocate funds from the pot to a beneficiary. //! - **Beneficiary:** An account who will receive the funds from a proposal iff the proposal is //! approved. -//! - **Deposit:** Funds that a proposer must lock when making a proposal. The deposit will be -//! returned or slashed if the proposal is approved or rejected respectively. //! - **Pot:** Unspent funds accumulated by the treasury pallet. +//! - **Spend** An approved proposal for transferring a specific amount of funds to a designated +//! beneficiary. +//! +//! ### Example //! -//! ## Interface +//! 1. Multiple local spends approved by spend origins and received by a beneficiary. +#![doc = docify::embed!("frame/treasury/src/tests.rs", spend_local_origin_works)] //! -//! ### Dispatchable Functions +//! 2. Approve a spend of some asset kind and claim it. +#![doc = docify::embed!("frame/treasury/src/tests.rs", spend_payout_works)] //! -//! General spending/proposal protocol: -//! - `propose_spend` - Make a spending proposal and stake the required deposit. -//! - `reject_proposal` - Reject a proposal, slashing the deposit. -//! - `approve_proposal` - Accept the proposal, returning the deposit. -//! - `remove_approval` - Remove an approval, the deposit will no longer be returned. +//! ## Pallet API //! -//! ## GenesisConfig +//! See the [`pallet`] module for more information about the interfaces this pallet exposes, +//! including its configuration trait, dispatchables, storage items, events and errors. //! -//! The Treasury pallet depends on the [`GenesisConfig`]. +//! ## Low Level / Implementation Details +//! +//! Spends can be initiated using either the `spend_local` or `spend` dispatchable. The +//! `spend_local` dispatchable enables the creation of spends using the native currency of the +//! chain, utilizing the funds stored in the pot. These spends are automatically paid out every +//! [`pallet::Config::SpendPeriod`]. On the other hand, the `spend` dispatchable allows spending of +//! any asset kind managed by the treasury, with payment facilitated by a designated +//! [`pallet::Config::Paymaster`]. To claim these spends, the `payout` dispatchable should be called +//! within some temporal bounds, starting from the moment they become valid and within one +//! [`pallet::Config::PayoutPeriod`]. #![cfg_attr(not(feature = "std"), no_std)] @@ -308,6 +322,7 @@ pub mod pallet { pub(crate) type SpendCount = StorageValue<_, SpendIndex, ValueQuery>; /// Spends that have been approved and being processed. + // Hasher: Twox safe since `SpendIndex` is an internal count based index. #[pallet::storage] pub type Spends, I: 'static = ()> = StorageMap< _, @@ -461,12 +476,22 @@ 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. + /// Put forward a suggestion for spending. /// - /// ## Complexity + /// ## Dispatch Origin + /// + /// Must be signed. + /// + /// ## Details + /// 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) + /// + /// ## Events + /// + /// Emits [`Event::Proposed`] if successful. #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::propose_spend())] pub fn propose_spend( @@ -489,12 +514,21 @@ pub mod pallet { Ok(()) } - /// Reject a proposed spend. The original deposit will be slashed. + /// Reject a proposed spend. /// - /// May only be called from `T::RejectOrigin`. + /// ## Dispatch Origin /// - /// ## Complexity + /// Must be [`Config::RejectOrigin`]. + /// + /// ## Details + /// The original deposit will be slashed. + /// + /// ### Complexity /// - O(1) + /// + /// ## Events + /// + /// Emits [`Event::Rejected`] if successful. #[pallet::call_index(1)] #[pallet::weight((T::WeightInfo::reject_proposal(), DispatchClass::Operational))] pub fn reject_proposal( @@ -516,13 +550,23 @@ pub mod pallet { Ok(()) } - /// Approve a proposal. At a later time, the proposal will be allocated to the beneficiary - /// and the original deposit will be returned. + /// Approve a proposal. /// - /// May only be called from `T::ApproveOrigin`. + /// ## Dispatch Origin /// - /// ## Complexity + /// Must be [`Config::ApproveOrigin`]. + /// + /// ## Details + /// + /// At a later time, the proposal will be allocated to the beneficiary and the original + /// deposit will be returned. + /// + /// ### Complexity /// - O(1). + /// + /// ## Events + /// + /// No events are emitted from this dispatch. #[pallet::call_index(2)] #[pallet::weight((T::WeightInfo::approve_proposal(T::MaxApprovals::get()), DispatchClass::Operational))] pub fn approve_proposal( @@ -539,12 +583,21 @@ pub mod pallet { /// 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. + /// ## Dispatch Origin /// + /// Must be [`Config::SpendOrigin`] with the `Success` value being at least `amount`. + /// + /// ### Details /// NOTE: For record-keeping purposes, the proposer is deemed to be equivalent to the /// beneficiary. + /// + /// ### Parameters + /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. + /// - `beneficiary`: The destination account for the transfer. + /// + /// ## Events + /// + /// Emits [`Event::SpendApproved`] if successful. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::spend_local())] pub fn spend_local( @@ -593,18 +646,26 @@ pub mod pallet { } /// Force a previously approved proposal to be removed from the approval queue. + /// + /// ## Dispatch Origin + /// + /// Must be [`Config::RejectOrigin`]. + /// + /// ## Details + /// /// The original deposit will no longer be returned. /// - /// May only be called from `T::RejectOrigin`. + /// ### Parameters /// - `proposal_id`: The index of a proposal /// - /// ## Complexity + /// ### 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. + /// ### Errors + /// - [`Error::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( @@ -627,16 +688,28 @@ pub mod pallet { /// Propose and approve a spend of treasury funds. /// - /// An approved spend must be claimed via the `payout` dispatchable within the - /// [`Config::PayoutPeriod`]. + /// ## Dispatch Origin /// - /// - `origin`: Must be `T::SpendOrigin` with the `Success` value being at least `amount` of - /// `asset_kind` in the native asset. + /// Must be [`Config::SpendOrigin`] with the `Success` value being at least + /// `amount` of `asset_kind` in the native asset. The amount of `asset_kind` is converted + /// for assertion using the [`Config::BalanceConverter`]. + /// + /// ## Details + /// + /// Create an approved spend for transferring a specific `amount` of `asset_kind` to a + /// designated beneficiary. The spend must be claimed using the `payout` dispatchable within + /// the [`Config::PayoutPeriod`]. + /// + /// ### Parameters /// - `asset_kind`: An indicator of the specific asset class to be spent. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. /// - `valid_from`: The block number from which the spend can be claimed. If `None`, the /// spend can be claimed immediately after approval. + /// + /// ## Events + /// + /// Emits [`Event::AssetSpendApproved`] if successful. #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::spend())] pub fn spend( @@ -701,12 +774,23 @@ pub mod pallet { /// Claim a spend. /// - /// To claim a spend, it must be done within the [`Config::PayoutPeriod`] after approval. - /// In case of a payout failure, the spend status should be updated with the `check_status` - /// before retrying with the current function. + /// ## Dispatch Origin + /// + /// Must be signed. + /// + /// ## Details /// - /// - `origin`: Must be `Signed`. + /// Spends must be claimed within some temporal bounds. A spend may be claimed within one + /// [`Config::PayoutPeriod`] from the `valid_from` block. + /// In case of a payout failure, the spend status must be updated with the check_status + /// dispatchable before retrying with the current function. + /// + /// ### Parameters /// - `index`: The spend index. + /// + /// ## Events + /// + /// Emits [`Event::Paid`] if successful. #[pallet::call_index(6)] #[pallet::weight(T::WeightInfo::payout())] pub fn payout(origin: OriginFor, index: SpendIndex) -> DispatchResult { @@ -733,10 +817,22 @@ pub mod pallet { /// Check the status of the spend. /// + /// ## Dispatch Origin + /// + /// Must be signed. + /// + /// ## Details + /// /// The status check is a prerequisite for retrying a failed payout. + /// If the payout has succeed the spend is removed from the storage. /// - /// - `origin`: Must be `Signed`. + /// ### Parameters /// - `index`: The spend index. + /// + /// ## Events + /// + /// Emits [`Event::PaymentFailed`] if the spend payout has failed. + /// Emits [`Event::PaymentSucceed`] if the spend payout has succeed. #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::check_status())] pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResult { @@ -765,8 +861,20 @@ pub mod pallet { /// Void previously approved spend. /// - /// - `origin`: Must be `T::RejectOrigin`. + /// ## Dispatch Origin + /// + /// Must be [`Config::RejectOrigin`]. + /// + /// ## Details + /// + /// A spend void is only possible if the payout has not been attempted yet. + /// + /// ### Parameters /// - `index`: The spend index. + /// + /// ## Events + /// + /// Emits [`Event::AssetSpendVoided`] if successful. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::void_spend())] pub fn void_spend(origin: OriginFor, index: SpendIndex) -> DispatchResult { diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index cd538c4301e7f..765bba2417578 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -265,11 +265,13 @@ fn spend_local_origin_permissioning_works() { }); } +#[docify::export] #[test] 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); + // approve spend of some amount to beneficiary `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)); @@ -277,12 +279,13 @@ fn spend_local_origin_works() { 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)); - + // free balance of `6` is zero, spend period has not passed. >::on_initialize(1); assert_eq!(Balances::free_balance(6), 0); - + // free balance of `6` is `100`, spend period has passed. >::on_initialize(2); assert_eq!(Balances::free_balance(6), 100); + // `100` spent, `1` burned. assert_eq!(Treasury::pot(), 0); }); } @@ -699,12 +702,16 @@ fn spend_expires() { }); } +#[docify::export] #[test] -fn payout_works() { +fn spend_payout_works() { new_test_ext().execute_with(|| { System::set_block_number(1); + // approve a `2` coins spend of asset `1` to beneficiary `6`, the spend valid from now. assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + // payout the spend. assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); + // beneficiary received `2` coins of asset `1`. assert_eq!(paid(6, 1), 2); assert_eq!(SpendCount::::get(), 1); let spend = Spends::::get(0).unwrap(); @@ -715,9 +722,10 @@ fn payout_works() { let payment_id = maybe_payment_id.expect("no payment attempt"); System::assert_last_event(Event::::Paid { index: 0, payment_id }.into()); set_status(payment_id, PaymentStatus::Success); + // the payment succeed. assert_ok!(Treasury::check_status(RuntimeOrigin::signed(1), 0)); System::assert_last_event(Event::::PaymentSucceed { index: 0, payment_id }.into()); - // cannot payout again + // cannot payout the same spend twice. assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::InvalidIndex); }); } From 2ea9379b2ec8496254f4aa94bcea3ac94a57cb00 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 12 Jul 2023 12:11:46 +0200 Subject: [PATCH 32/36] valid_from and expiration constraints --- frame/treasury/src/lib.rs | 13 +++++++++---- frame/treasury/src/tests.rs | 18 ++++++++++++++---- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 3e01a15c9b819..54cf22dcd62e1 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -704,8 +704,10 @@ pub mod pallet { /// - `asset_kind`: An indicator of the specific asset class to be spent. /// - `amount`: The amount to be transferred from the treasury to the `beneficiary`. /// - `beneficiary`: The beneficiary of the spend. - /// - `valid_from`: The block number from which the spend can be claimed. If `None`, the - /// spend can be claimed immediately after approval. + /// - `valid_from`: The block number from which the spend can be claimed. It can refer to + /// the past if the resulting spend has not yet expired according to the + /// [`Config::PayoutPeriod`]. If `None`, the spend can be claimed immediately after + /// approval. /// /// ## Events /// @@ -722,6 +724,11 @@ pub mod pallet { let max_amount = T::SpendOrigin::ensure_origin(origin)?; let beneficiary = T::BeneficiaryLookup::lookup(beneficiary)?; + let now = frame_system::Pallet::::block_number(); + let valid_from = valid_from.unwrap_or(now); + let expire_at = valid_from.saturating_add(T::PayoutPeriod::get()); + ensure!(expire_at > now, Error::::SpendExpired); + let native_amount = T::BalanceConverter::from_asset_balance(amount, asset_kind.clone()) .map_err(|_| Error::::FailedToConvertBalance)?; @@ -746,8 +753,6 @@ pub mod pallet { .unwrap_or(Ok(()))?; let index = SpendCount::::get(); - let valid_from = valid_from.unwrap_or(frame_system::Pallet::::block_number()); - let expire_at = valid_from.saturating_add(T::PayoutPeriod::get()); Spends::::insert( index, SpendStatus { diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 765bba2417578..bf154d50e4b31 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -695,10 +695,19 @@ fn spend_works() { #[test] fn spend_expires() { new_test_ext().execute_with(|| { + assert_eq!(::PayoutPeriod::get(), 5); + + // spend `0` expires in 5 blocks after the creating. System::set_block_number(1); assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); System::set_block_number(6); assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::SpendExpired); + + // spend cannot be approved since its already expired. + assert_noop!( + Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(0)), + Error::::SpendExpired + ); }); } @@ -765,6 +774,7 @@ fn spend_valid_from_works() { new_test_ext().execute_with(|| { assert_eq!(::PayoutPeriod::get(), 5); System::set_block_number(1); + // spend valid from block `2`. assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(2))); assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::EarlyPayout); @@ -772,10 +782,10 @@ fn spend_valid_from_works() { assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); System::set_block_number(5); - // spend valid from block `0`. - assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(0))); - // spend expired. - assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 1), Error::::SpendExpired); + // spend approved even if `valid_from` in the past since the payout period has not passed. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, Some(4))); + // spend paid. + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 1)); }); } From 7c7790e1d247062bc5eefa8740b8ed16ccc394f0 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 12 Jul 2023 16:18:39 +0200 Subject: [PATCH 33/36] check status removes expired spends --- frame/treasury/src/lib.rs | 44 ++++++++++++------- frame/treasury/src/tests.rs | 87 +++++++++++++++++++++++++++++++------ 2 files changed, 101 insertions(+), 30 deletions(-) diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 54cf22dcd62e1..e7eb30c70dd4f 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -405,10 +405,11 @@ pub mod pallet { AssetSpendVoided { index: SpendIndex }, /// A payment happened. Paid { index: SpendIndex, payment_id: ::Id }, - /// A payment succeed, the spend removed from the storage. - PaymentSucceed { index: SpendIndex, payment_id: ::Id }, - /// A payment failed. + /// A payment failed and can be retried. PaymentFailed { index: SpendIndex, payment_id: ::Id }, + /// A spend processed and removed from the storage. It might have been successfully paid or + /// it may have expired. + SpendProcessed { index: SpendIndex }, } /// Error for the treasury pallet. @@ -464,8 +465,6 @@ pub mod pallet { } else { Weight::zero() } - - // TODO: once in a while remove expired `Spends` } } @@ -787,7 +786,7 @@ pub mod pallet { /// /// Spends must be claimed within some temporal bounds. A spend may be claimed within one /// [`Config::PayoutPeriod`] from the `valid_from` block. - /// In case of a payout failure, the spend status must be updated with the check_status + /// In case of a payout failure, the spend status must be updated with the `check_status` /// dispatchable before retrying with the current function. /// /// ### Parameters @@ -820,7 +819,7 @@ pub mod pallet { Ok(()) } - /// Check the status of the spend. + /// Check the status of the spend and remove it from the storage if processed. /// /// ## Dispatch Origin /// @@ -829,7 +828,8 @@ pub mod pallet { /// ## Details /// /// The status check is a prerequisite for retrying a failed payout. - /// If the payout has succeed the spend is removed from the storage. + /// If a spend has either succeeded or expired, it is removed from the storage by this + /// function. In such instances, transaction fees are refunded. /// /// ### Parameters /// - `index`: The spend index. @@ -837,31 +837,43 @@ pub mod pallet { /// ## Events /// /// Emits [`Event::PaymentFailed`] if the spend payout has failed. - /// Emits [`Event::PaymentSucceed`] if the spend payout has succeed. + /// Emits [`Event::SpendProcessed`] if the spend payout has succeed. #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::check_status())] - pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResult { + pub fn check_status(origin: OriginFor, index: SpendIndex) -> DispatchResultWithPostInfo { + use PaymentState as State; + use PaymentStatus as Status; + ensure_signed(origin)?; let mut spend = Spends::::get(index).ok_or(Error::::InvalidIndex)?; + let now = frame_system::Pallet::::block_number(); + + if now > spend.expire_at && !matches!(spend.status, State::Attempted { .. }) { + // spend has expired and no further status update is expected. + Spends::::remove(index); + Self::deposit_event(Event::::SpendProcessed { index }); + return Ok(Pays::No.into()) + } let payment_id = match spend.status { - PaymentState::Attempted { id } => id, + State::Attempted { id } => id, _ => return Err(Error::::NotAttempted.into()), }; match T::Paymaster::check_payment(payment_id) { - PaymentStatus::Failure => { + Status::Failure => { spend.status = PaymentState::Failed; Spends::::insert(index, spend); Self::deposit_event(Event::::PaymentFailed { index, payment_id }); }, - PaymentStatus::Success => { + Status::Success | Status::Unknown => { Spends::::remove(index); - Self::deposit_event(Event::::PaymentSucceed { index, payment_id }); + Self::deposit_event(Event::::SpendProcessed { index }); + return Ok(Pays::No.into()) }, - _ => return Err(Error::::Inconclusive.into()), + Status::InProgress => return Err(Error::::Inconclusive.into()), } - Ok(()) + return Ok(Pays::Yes.into()) } /// Void previously approved spend. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index bf154d50e4b31..f55c510b48c1d 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -28,7 +28,7 @@ use sp_runtime::{ use frame_support::{ assert_err_ignore_postinfo, assert_noop, assert_ok, - pallet_prelude::GenesisBuild, + pallet_prelude::{GenesisBuild, Pays}, parameter_types, traits::{ tokens::{ConversionFromAssetBalance, PaymentStatus}, @@ -234,6 +234,14 @@ pub fn new_test_ext() -> sp_io::TestExternalities { t.into() } +fn get_payment_id(i: SpendIndex) -> Option { + let spend = Spends::::get(i).expect("no spend"); + match spend.status { + PaymentState::Attempted { id } => Some(id), + _ => None, + } +} + #[test] fn genesis_config_works() { new_test_ext().execute_with(|| { @@ -723,17 +731,12 @@ fn spend_payout_works() { // beneficiary received `2` coins of asset `1`. assert_eq!(paid(6, 1), 2); assert_eq!(SpendCount::::get(), 1); - let spend = Spends::::get(0).unwrap(); - let maybe_payment_id = match spend.status { - PaymentState::Attempted { id } => Some(id), - _ => None, - }; - let payment_id = maybe_payment_id.expect("no payment attempt"); + let payment_id = get_payment_id(0).expect("no payment attempt"); System::assert_last_event(Event::::Paid { index: 0, payment_id }.into()); set_status(payment_id, PaymentStatus::Success); // the payment succeed. assert_ok!(Treasury::check_status(RuntimeOrigin::signed(1), 0)); - System::assert_last_event(Event::::PaymentSucceed { index: 0, payment_id }.into()); + System::assert_last_event(Event::::SpendProcessed { index: 0 }.into()); // cannot payout the same spend twice. assert_noop!(Treasury::payout(RuntimeOrigin::signed(1), 0), Error::::InvalidIndex); }); @@ -746,12 +749,7 @@ fn payout_retry_works() { assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 0)); assert_eq!(paid(6, 1), 2); - let spend = Spends::::get(0).unwrap(); - let maybe_payment_id = match spend.status { - PaymentState::Attempted { id } => Some(id), - _ => None, - }; - let payment_id = maybe_payment_id.expect("no payment attempt"); + let payment_id = get_payment_id(0).expect("no payment attempt"); // spend payment is failed set_status(payment_id, PaymentStatus::Failure); unpay(6, 1, 2); @@ -807,3 +805,64 @@ fn void_spend_works() { assert_eq!(Spends::::get(1), None); }); } + +#[test] +fn check_status_works() { + new_test_ext().execute_with(|| { + assert_eq!(::PayoutPeriod::get(), 5); + System::set_block_number(1); + + // spend `0` expired and can be removed. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + System::set_block_number(7); + let info = Treasury::check_status(RuntimeOrigin::signed(1), 0).unwrap(); + assert_eq!(info.pays_fee, Pays::No); + System::assert_last_event(Event::::SpendProcessed { index: 0 }.into()); + + // spend `1` payment failed and expired hence can be removed. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + assert_noop!( + Treasury::check_status(RuntimeOrigin::signed(1), 1), + Error::::NotAttempted + ); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 1)); + let payment_id = get_payment_id(1).expect("no payment attempt"); + set_status(payment_id, PaymentStatus::Failure); + // spend expired. + System::set_block_number(13); + let info = Treasury::check_status(RuntimeOrigin::signed(1), 1).unwrap(); + assert_eq!(info.pays_fee, Pays::Yes); + System::assert_last_event(Event::::PaymentFailed { index: 1, payment_id }.into()); + let info = Treasury::check_status(RuntimeOrigin::signed(1), 1).unwrap(); + assert_eq!(info.pays_fee, Pays::No); + System::assert_last_event(Event::::SpendProcessed { index: 1 }.into()); + + // spend `2` payment succeed. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 2)); + let payment_id = get_payment_id(2).expect("no payment attempt"); + set_status(payment_id, PaymentStatus::Success); + let info = Treasury::check_status(RuntimeOrigin::signed(1), 2).unwrap(); + assert_eq!(info.pays_fee, Pays::No); + System::assert_last_event(Event::::SpendProcessed { index: 2 }.into()); + + // spend `3` payment in process. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 3)); + let payment_id = get_payment_id(3).expect("no payment attempt"); + set_status(payment_id, PaymentStatus::InProgress); + assert_noop!( + Treasury::check_status(RuntimeOrigin::signed(1), 3), + Error::::Inconclusive + ); + + // spend `4` removed since the payment status is unknown. + assert_ok!(Treasury::spend(RuntimeOrigin::signed(10), 1, 2, 6, None)); + assert_ok!(Treasury::payout(RuntimeOrigin::signed(1), 4)); + let payment_id = get_payment_id(4).expect("no payment attempt"); + set_status(payment_id, PaymentStatus::Unknown); + let info = Treasury::check_status(RuntimeOrigin::signed(1), 4).unwrap(); + assert_eq!(info.pays_fee, Pays::No); + System::assert_last_event(Event::::SpendProcessed { index: 4 }.into()); + }); +} From 0d8ebcb372792eb265eeae14adee9fccc1e4bd4e Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 27 Jul 2023 18:19:52 +0200 Subject: [PATCH 34/36] fixes after master merge --- Cargo.lock | 2 +- frame/asset-rate/src/lib.rs | 2 +- frame/treasury/src/lib.rs | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c745f2ab0ee3c..b3ba82a26aaae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7251,7 +7251,7 @@ dependencies = [ name = "pallet-treasury" version = "4.0.0-dev" dependencies = [ - "docify", + "docify 0.1.16", "frame-benchmarking", "frame-support", "frame-system", diff --git a/frame/asset-rate/src/lib.rs b/frame/asset-rate/src/lib.rs index 56d9c103e5a29..08ae5eee59cf1 100644 --- a/frame/asset-rate/src/lib.rs +++ b/frame/asset-rate/src/lib.rs @@ -244,7 +244,7 @@ where } /// Set a conversion rate to `1` for the `asset_id`. #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(asset_id: AssetIdOf) { + fn ensure_successful(asset_id: AssetKindOf) { pallet::ConversionRateToNative::::set(asset_id.clone(), Some(1.into())); } } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7b01cc604dcf6..16bcdc444cd4c 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -283,7 +283,7 @@ pub mod pallet { /// The period during which an approved treasury spend has to be claimed. #[pallet::constant] - type PayoutPeriod: Get; + type PayoutPeriod: Get>; /// Helper type for benchmarks. #[cfg(feature = "runtime-benchmarks")] @@ -332,7 +332,7 @@ pub mod pallet { T::AssetKind, AssetBalanceOf, T::Beneficiary, - T::BlockNumber, + BlockNumberFor, ::Id, >, OptionQuery, @@ -388,8 +388,8 @@ pub mod pallet { asset_kind: T::AssetKind, amount: AssetBalanceOf, beneficiary: T::Beneficiary, - valid_from: T::BlockNumber, - expire_at: T::BlockNumber, + valid_from: BlockNumberFor, + expire_at: BlockNumberFor, }, /// An approved spend was voided. AssetSpendVoided { index: SpendIndex }, @@ -708,7 +708,7 @@ pub mod pallet { asset_kind: T::AssetKind, #[pallet::compact] amount: AssetBalanceOf, beneficiary: BeneficiaryLookupOf, - valid_from: Option, + valid_from: Option>, ) -> DispatchResult { let max_amount = T::SpendOrigin::ensure_origin(origin)?; let beneficiary = T::BeneficiaryLookup::lookup(beneficiary)?; From 147e9187d7deccf03dc9a8c6930637777b037a7e Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 28 Jul 2023 17:08:38 +0200 Subject: [PATCH 35/36] update docify version --- Cargo.lock | 2 +- frame/treasury/Cargo.toml | 2 +- frame/treasury/src/lib.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b3ba82a26aaae..8b85167515419 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7251,7 +7251,7 @@ dependencies = [ name = "pallet-treasury" version = "4.0.0-dev" dependencies = [ - "docify 0.1.16", + "docify 0.2.0", "frame-benchmarking", "frame-support", "frame-system", diff --git a/frame/treasury/Cargo.toml b/frame/treasury/Cargo.toml index 88c858368708d..d686bd35643a2 100644 --- a/frame/treasury/Cargo.toml +++ b/frame/treasury/Cargo.toml @@ -28,7 +28,7 @@ sp-runtime = { version = "24.0.0", default-features = false, path = "../../primi sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" } sp-core = { version = "21.0.0", default-features = false, path = "../../primitives/core" } -docify = "0.1.13" +docify = "0.2.0" [dev-dependencies] sp-io = { version = "23.0.0", path = "../../primitives/io" } diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 16bcdc444cd4c..06e469917f21a 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -49,10 +49,10 @@ //! ### Example //! //! 1. Multiple local spends approved by spend origins and received by a beneficiary. -#![doc = docify::embed!("frame/treasury/src/tests.rs", spend_local_origin_works)] +#![doc = docify::embed!("src/tests.rs", spend_local_origin_works)] //! //! 2. Approve a spend of some asset kind and claim it. -#![doc = docify::embed!("frame/treasury/src/tests.rs", spend_payout_works)] +#![doc = docify::embed!("src/tests.rs", spend_payout_works)] //! //! ## Pallet API //! From 5c78c383957f2eff42664166fd9d3690138d4c1a Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 2 Aug 2023 10:41:55 +0200 Subject: [PATCH 36/36] correct docs --- frame/asset-rate/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/asset-rate/src/benchmarking.rs b/frame/asset-rate/src/benchmarking.rs index 72b4f3264a806..0e13697806043 100644 --- a/frame/asset-rate/src/benchmarking.rs +++ b/frame/asset-rate/src/benchmarking.rs @@ -26,7 +26,7 @@ use frame_support::assert_ok; use frame_system::RawOrigin; use sp_core::crypto::FromEntropy; -/// Trait describing the factory function for AssetKind parameter +/// Trait describing the factory function for the `AssetKind` parameter. pub trait AssetKindFactory { fn create_asset_kind(seed: u32) -> AssetKind; }