diff --git a/frame/elections-phragmen/src/lib.rs b/frame/elections-phragmen/src/lib.rs index 50c26dba0fc21..d77bd750e80c6 100644 --- a/frame/elections-phragmen/src/lib.rs +++ b/frame/elections-phragmen/src/lib.rs @@ -93,6 +93,7 @@ use frame_support::{ traits::{ Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons, ChangeMembers, OnUnbalanced, WithdrawReason, Contains, BalanceStatus, InitializeMembers, + ContainsLengthBound, } }; use sp_phragmen::{build_support_map, ExtendedBalance, VoteWeight, PhragmenResult}; @@ -880,6 +881,15 @@ impl Contains for Module { } } +impl ContainsLengthBound for Module { + fn min_len() -> usize { 0 } + + /// Implementation uses a parameter type so calling is cost-free. + fn max_len() -> usize { + Self::desired_members() as usize + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index dfc2ddf4de129..3d5a3403a9724 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -190,9 +190,7 @@ impl Get for () { fn get() -> T { T::default() } } -/// A trait for querying whether a type can be said to statically "contain" a value. Similar -/// in nature to `Get`, except it is designed to be lazy rather than active (you can't ask it to -/// enumerate all values that it contains) and work for multiple values rather than just one. +/// A trait for querying whether a type can be said to "contain" a value. pub trait Contains { /// Return `true` if this "contains" the given value `t`. fn contains(t: &T) -> bool { Self::sorted_members().binary_search(t).is_ok() } @@ -211,6 +209,14 @@ pub trait Contains { fn add(_t: &T) { unimplemented!() } } +/// A trait for querying bound for the length of an implementation of `Contains` +pub trait ContainsLengthBound { + /// Minimum number of elements contained + fn min_len() -> usize; + /// Maximum number of elements contained + fn max_len() -> usize; +} + /// Determiner to say whether a given account is unused. pub trait IsDeadAccount { /// Is the given account dead? diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index b1646e4a20d5e..270a710a2c29e 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -99,7 +99,7 @@ use sp_runtime::{Permill, ModuleId, Percent, RuntimeDebug, traits::{ Zero, StaticLookup, AccountIdConversion, Saturating, Hash, BadOrigin }}; use frame_support::weights::{Weight, DispatchClass}; -use frame_support::traits::{Contains, EnsureOrigin}; +use frame_support::traits::{Contains, ContainsLengthBound, EnsureOrigin}; use codec::{Encode, Decode}; use frame_system::{self as system, ensure_signed, ensure_root}; @@ -124,7 +124,9 @@ pub trait Trait: frame_system::Trait { type RejectOrigin: EnsureOrigin; /// Origin from which tippers must come. - type Tippers: Contains; + /// + /// `ContainsLengthBound::max_len` must be cost free (i.e. no storage read or heavy operation). + type Tippers: Contains + ContainsLengthBound; /// The period for which a tip remains open after is has achieved threshold tippers. type TipCountdown: Get; @@ -326,11 +328,11 @@ decl_module! { /// proposal is awarded. /// /// # - /// - O(1). - /// - Limited storage reads. - /// - One DB change, one extra DB entry. + /// - Complexity: O(1) + /// - DbReads: `ProposalCount`, `origin account` + /// - DbWrites: `ProposalCount`, `Proposals`, `origin account` /// # - #[weight = 500_000_000] + #[weight = 120_000_000 + T::DbWeight::get().reads_writes(1, 2)] fn propose_spend( origin, #[compact] value: BalanceOf, @@ -353,11 +355,11 @@ decl_module! { /// Reject a proposed spend. The original deposit will be slashed. /// /// # - /// - O(1). - /// - Limited storage reads. - /// - One DB clear. + /// - Complexity: O(1) + /// - DbReads: `Proposals`, `rejected proposer account` + /// - DbWrites: `Proposals`, `rejected proposer account` /// # - #[weight = (100_000_000, DispatchClass::Operational)] + #[weight = (130_000_000 + T::DbWeight::get().reads_writes(2, 2), DispatchClass::Operational)] fn reject_proposal(origin, #[compact] proposal_id: ProposalIndex) { T::RejectOrigin::try_origin(origin) .map(|_| ()) @@ -375,11 +377,11 @@ decl_module! { /// and the original deposit will be returned. /// /// # - /// - O(1). - /// - Limited storage reads. - /// - One DB change. + /// - Complexity: O(1). + /// - DbReads: `Proposals`, `Approvals` + /// - DbWrite: `Approvals` /// # - #[weight = (100_000_000, DispatchClass::Operational)] + #[weight = (34_000_000 + T::DbWeight::get().reads_writes(2, 1), DispatchClass::Operational)] fn approve_proposal(origin, #[compact] proposal_id: ProposalIndex) { T::ApproveOrigin::try_origin(origin) .map(|_| ()) @@ -403,12 +405,12 @@ decl_module! { /// Emits `NewTip` if successful. /// /// # - /// - `O(R)` where `R` length of `reason`. - /// - One balance operation. - /// - One storage mutation (codec `O(R)`). - /// - One event. + /// - Complexity: `O(R)` where `R` length of `reason`. + /// - encoding and hashing of 'reason' + /// - DbReads: `Reasons`, `Tips`, `who account data` + /// - DbWrites: `Tips`, `who account data` /// # - #[weight = 100_000_000] + #[weight = 140_000_000 + 4_000 * reason.len() as Weight + T::DbWeight::get().reads_writes(3, 2)] fn report_awesome(origin, reason: Vec, who: T::AccountId) { let finder = ensure_signed(origin)?; @@ -445,12 +447,12 @@ decl_module! { /// Emits `TipRetracted` if successful. /// /// # - /// - `O(T)` - /// - One balance operation. - /// - Two storage removals (one read, codec `O(T)`). - /// - One event. + /// - Complexity: `O(1)` + /// - Depends on the length of `T::Hash` which is fixed. + /// - DbReads: `Tips`, `origin account` + /// - DbWrites: `Reasons`, `Tips`, `origin account` /// # - #[weight = 50_000_000] + #[weight = 120_000_000 + T::DbWeight::get().reads_writes(1, 2)] fn retract_tip(origin, hash: T::Hash) { let who = ensure_signed(origin)?; let tip = Tips::::get(&hash).ok_or(Error::::UnknownTip)?; @@ -477,12 +479,18 @@ decl_module! { /// Emits `NewTip` if successful. /// /// # - /// - `O(R + T)` where `R` length of `reason`, `T` is the number of tippers. `T` is - /// naturally capped as a membership set, `R` is limited through transaction-size. - /// - Two storage insertions (codecs `O(R)`, `O(T)`), one read `O(1)`. - /// - One event. + /// - Complexity: `O(R + T)` where `R` length of `reason`, `T` is the number of tippers. + /// - `O(T)`: decoding `Tipper` vec of length `T` + /// `T` is charged as upper bound given by `ContainsLengthBound`. + /// The actual cost depends on the implementation of `T::Tippers`. + /// - `O(R)`: hashing and encoding of reason of length `R` + /// - DbReads: `Tippers`, `Reasons` + /// - DbWrites: `Reasons`, `Tips` /// # - #[weight = 150_000_000] + #[weight = 110_000_000 + + 4_000 * reason.len() as Weight + + 480_000 * T::Tippers::max_len() as Weight + + T::DbWeight::get().reads_writes(2, 2)] fn tip_new(origin, reason: Vec, who: T::AccountId, tip_value: BalanceOf) { let tipper = ensure_signed(origin)?; ensure!(T::Tippers::contains(&tipper), BadOrigin); @@ -512,11 +520,18 @@ decl_module! { /// has started. /// /// # - /// - `O(T)` - /// - One storage mutation (codec `O(T)`), one storage read `O(1)`. - /// - Up to one event. + /// - Complexity: `O(T)` where `T` is the number of tippers. + /// decoding `Tipper` vec of length `T`, insert tip and check closing, + /// `T` is charged as upper bound given by `ContainsLengthBound`. + /// The actual cost depends on the implementation of `T::Tippers`. + /// + /// Actually weight could be lower as it depends on how many tips are in `OpenTip` but it + /// is weighted as if almost full i.e of length `T-1`. + /// - DbReads: `Tippers`, `Tips` + /// - DbWrites: `Tips` /// # - #[weight = 50_000_000] + #[weight = 68_000_000 + 2_000_000 * T::Tippers::max_len() as Weight + + T::DbWeight::get().reads_writes(2, 1)] fn tip(origin, hash: T::Hash, tip_value: BalanceOf) { let tipper = ensure_signed(origin)?; ensure!(T::Tippers::contains(&tipper), BadOrigin); @@ -538,11 +553,15 @@ decl_module! { /// as the hash of the tuple of the original tip `reason` and the beneficiary account ID. /// /// # - /// - `O(T)` - /// - One storage retrieval (codec `O(T)`) and two removals. - /// - Up to three balance operations. + /// - Complexity: `O(T)` where `T` is the number of tippers. + /// decoding `Tipper` vec of length `T`. + /// `T` is charged as upper bound given by `ContainsLengthBound`. + /// The actual cost depends on the implementation of `T::Tippers`. + /// - DbReads: `Tips`, `Tippers`, `tip finder` + /// - DbWrites: `Reasons`, `Tips`, `Tippers`, `tip finder` /// # - #[weight = 50_000_000] + #[weight = 220_000_000 + 1_100_000 * T::Tippers::max_len() as Weight + + T::DbWeight::get().reads_writes(3, 3)] fn close_tip(origin, hash: T::Hash) { ensure_signed(origin)?; @@ -555,13 +574,23 @@ decl_module! { Self::payout_tip(hash, tip); } + /// # + /// - Complexity: `O(A)` where `A` is the number of approvals + /// - Db reads and writes: `Approvals`, `pot account data` + /// - Db reads and writes per approval: + /// `Proposals`, `proposer account data`, `beneficiary account data` + /// - The weight is overestimated if some approvals got missed. + /// # fn on_initialize(n: T::BlockNumber) -> Weight { // Check to see if we should spend some funds! if (n % T::SpendPeriod::get()).is_zero() { - Self::spend_funds(); - } + let approvals_len = Self::spend_funds(); - 0 + 270_000_000 * approvals_len + + T::DbWeight::get().reads_writes(2 + approvals_len * 3, 2 + approvals_len * 3) + } else { + 0 + } } } } @@ -653,14 +682,15 @@ impl Module { Self::deposit_event(RawEvent::TipClosed(hash, tip.who, payout)); } - // Spend some money! - fn spend_funds() { + /// Spend some money! returns number of approvals before spend. + fn spend_funds() -> u64 { let mut budget_remaining = Self::pot(); Self::deposit_event(RawEvent::Spending(budget_remaining)); let mut missed_any = false; let mut imbalance = >::zero(); - Approvals::mutate(|v| { + let prior_approvals_len = Approvals::mutate(|v| { + let prior_approvals_len = v.len() as u64; v.retain(|&index| { // Should always be true, but shouldn't panic if false or we're screwed. if let Some(p) = Self::proposals(index) { @@ -684,6 +714,7 @@ impl Module { false } }); + prior_approvals_len }); if !missed_any { @@ -710,6 +741,8 @@ impl Module { } Self::deposit_event(RawEvent::Rollover(budget_remaining)); + + prior_approvals_len } /// Return the amount of money in the pot. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 2ce07547988ea..18b1d1c50eebd 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -111,6 +111,12 @@ impl Contains for TenToFourteen { }) } } +impl ContainsLengthBound for TenToFourteen { + fn max_len() -> usize { + TEN_TO_FOURTEEN.with(|v| v.borrow().len()) + } + fn min_len() -> usize { 0 } +} parameter_types! { pub const ProposalBond: Permill = Permill::from_percent(5); pub const ProposalBondMinimum: u64 = 1;