Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions frame/elections-phragmen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ use frame_support::{
traits::{
Currency, Get, LockableCurrency, LockIdentifier, ReservableCurrency, WithdrawReasons,
ChangeMembers, OnUnbalanced, WithdrawReason, Contains, BalanceStatus, InitializeMembers,
ContainsCountUpperBound,
}
};
use sp_phragmen::{build_support_map, ExtendedBalance, VoteWeight, PhragmenResult};
Expand Down Expand Up @@ -878,6 +879,13 @@ impl<T: Trait> Contains<T::AccountId> for Module<T> {
}
}

/// Implementation uses a parameter type so calling is cost-free.
impl<T: Trait> ContainsCountUpperBound for Module<T> {
fn count_upper_bound() -> usize {
Self::desired_members() as usize
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
11 changes: 8 additions & 3 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ impl<T: Default> Get<T> 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<T: Ord> {
/// Return `true` if this "contains" the given value `t`.
fn contains(t: &T) -> bool { Self::sorted_members().binary_search(t).is_ok() }
Expand All @@ -211,6 +209,13 @@ pub trait Contains<T: Ord> {
fn add(t: &T) { unimplemented!() }
}

/// A trait for querying an upper bound on the number of element in a type that implements
/// `Contains`.
pub trait ContainsCountUpperBound {
/// An upper bound for the number of element contained.
fn count_upper_bound() -> usize;
}

/// Determiner to say whether a given account is unused.
pub trait IsDeadAccount<AccountId> {
/// Is the given account dead?
Expand Down
7 changes: 4 additions & 3 deletions frame/treasury/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ fn create_approved_proposals<T: Trait>(n: u32) -> Result<(), &'static str> {
}

const MAX_BYTES: u32 = 16384;
const MAX_TIPPERS: u32 = 100;

benchmarks! {
_ { }
Expand Down Expand Up @@ -157,13 +158,13 @@ benchmarks! {

tip_new {
let r in 0 .. MAX_BYTES;
let t in 1 .. MAX_TIPPERS_COUNT;
let t in 1 .. MAX_TIPPERS;

let (caller, reason, beneficiary, value) = setup_tip::<T>(r, t)?;
}: _(RawOrigin::Signed(caller), reason, beneficiary, value)

tip {
let t in 1 .. MAX_TIPPERS_COUNT;
let t in 1 .. MAX_TIPPERS;
let (member, reason, beneficiary, value) = setup_tip::<T>(0, t)?;
let value = T::Currency::minimum_balance().saturating_mul(100.into());
Treasury::<T>::tip_new(
Expand All @@ -180,7 +181,7 @@ benchmarks! {
}: _(RawOrigin::Signed(caller), hash, value)

close_tip {
let t in 1 .. MAX_TIPPERS_COUNT;
let t in 1 .. MAX_TIPPERS;

// Make sure pot is funded
let pot_account = Treasury::<T>::account_id();
Expand Down
47 changes: 19 additions & 28 deletions frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
use serde::{Serialize, Deserialize};
use sp_std::prelude::*;
use frame_support::{decl_module, decl_storage, decl_event, ensure, print, decl_error, Parameter};
use frame_support::dispatch::DispatchResultWithPostInfo;
use frame_support::traits::{
Currency, Get, Imbalance, OnUnbalanced, ExistenceRequirement::KeepAlive,
ReservableCurrency, WithdrawReason
Expand All @@ -100,7 +99,7 @@ use sp_runtime::{Permill, ModuleId, Percent, RuntimeDebug, traits::{
Zero, StaticLookup, AccountIdConversion, Saturating, Hash, BadOrigin
}};
use frame_support::weights::Weight;
use frame_support::traits::{Contains, EnsureOrigin};
use frame_support::traits::{Contains, ContainsCountUpperBound, EnsureOrigin};
use codec::{Encode, Decode};
use frame_system::{self as system, ensure_signed, ensure_root};

Expand All @@ -114,9 +113,6 @@ type NegativeImbalanceOf<T> = <<T as Trait>::Currency as Currency<<T as frame_sy
/// The treasury's module id, used for deriving its sovereign account ID.
const MODULE_ID: ModuleId = ModuleId(*b"py/trsry");

/// The maximum size of tippers, this is used to compute weight, the exceeding will be refund.
pub const MAX_TIPPERS_COUNT: u64 = 1000;

pub trait Trait: frame_system::Trait {
/// The staking balance.
type Currency: Currency<Self::AccountId> + ReservableCurrency<Self::AccountId>;
Expand All @@ -128,9 +124,10 @@ pub trait Trait: frame_system::Trait {
type RejectOrigin: EnsureOrigin<Self::Origin>;

/// Origin from which tippers must come.
/// The lenght must be less than [`MAX_TIPPERS_COUNT`](./constant.MAX_TIPPERS_COUNT.html).
/// Its cost it expected to be same as if it was stored sorted in a storage value.
type Tippers: Contains<Self::AccountId>;
///
/// `ContainsCountUpperBound::count_upper_bound` must be cost free.
/// (i.e. no storage read or heavy operation)
type Tippers: Contains<Self::AccountId> + ContainsCountUpperBound;

/// The period for which a tip remains open after is has achieved threshold tippers.
type TipCountdown: Get<Self::BlockNumber>;
Expand Down Expand Up @@ -482,18 +479,17 @@ decl_module! {
/// # <weight>
/// - 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 less than `MAX_TIPPERS_COUNT`, it is charged as maximum and then refund.
/// `T` is charged as upper bound given by `ContainsCountUpperBound`.
/// 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>
#[weight = 107_700_000 + 4_000 * reason.len() as u64 + 481_000 * MAX_TIPPERS_COUNT
#[weight = 110_000_000
+ 4_000 * reason.len() as u64
+ 480_000 * T::Tippers::count_upper_bound() as u64
+ T::DbWeight::get().reads_writes(2, 2)]
fn tip_new(origin, reason: Vec<u8>, who: T::AccountId, tip_value: BalanceOf<T>)
-> DispatchResultWithPostInfo
{
// NOTE: we don't refund in case of error because checking cost too much.
fn tip_new(origin, reason: Vec<u8>, who: T::AccountId, tip_value: BalanceOf<T>) {
let tipper = ensure_signed(origin)?;
ensure!(T::Tippers::contains(&tipper), BadOrigin);
let reason_hash = T::Hashing::hash(&reason[..]);
Expand All @@ -505,8 +501,6 @@ decl_module! {
let tips = vec![(tipper, tip_value)];
let tip = OpenTip { reason: reason_hash, who, finder: None, closes: None, tips };
Tips::<T>::insert(&hash, tip);

Ok(Some(MAX_TIPPERS_COUNT.saturating_sub(T::Tippers::count() as u64) * 481_000).into())
}

/// Declare a tip value for an already-open tip.
Expand All @@ -526,17 +520,17 @@ decl_module! {
/// # <weight>
/// - Complexity: `O(T)` where `T` is the number of tippers.
/// decoding `Tipper` vec of length `T`, insert tip and check closing,
/// `T` is less than `MAX_TIPPERS_COUNT`, it is charged as maximum and then refund.
/// `T` is charged as upper bound given by `ContainsCountUpperBound`.
/// 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>
#[weight = 67_730_000 + 1_912_000 * MAX_TIPPERS_COUNT + T::DbWeight::get().reads_writes(2, 1)]
fn tip(origin, hash: T::Hash, tip_value: BalanceOf<T>) -> DispatchResultWithPostInfo {
// NOTE: we don't refund in case of error because checking cost too much.
#[weight = 68_000_000 + 1_900_000 * T::Tippers::count_upper_bound() as u64
+ T::DbWeight::get().reads_writes(2, 1)]
fn tip(origin, hash: T::Hash, tip_value: BalanceOf<T>) {
let tipper = ensure_signed(origin)?;
ensure!(T::Tippers::contains(&tipper), BadOrigin);

Expand All @@ -545,8 +539,6 @@ decl_module! {
Self::deposit_event(RawEvent::TipClosing(hash.clone()));
}
Tips::<T>::insert(&hash, tip);

Ok(Some(MAX_TIPPERS_COUNT.saturating_sub(T::Tippers::count() as u64) * 1_912_000).into())
}

/// Close and payout a tip.
Expand All @@ -561,13 +553,14 @@ decl_module! {
/// # <weight>
/// - Complexity: `O(T)` where `T` is the number of tippers.
/// decoding `Tipper` vec of length `T`.
/// `T` is less than `MAX_TIPPERS_COUNT`, it is charged as maximum and then refund.
/// `T` is charged as upper bound given by `ContainsCountUpperBound`.
/// The actual cost depends on the implementation of `T::Tippers`.
/// - DbReads: `Tips`, `Tippers`, `tip finder`
/// - DbWrites: `Reasons`, `Tips`, `Tippers`, `tip finder`
/// # </weight>
#[weight = 211_000_000 + 1_094_000 * MAX_TIPPERS_COUNT + T::DbWeight::get().reads_writes(3, 3)]
fn close_tip(origin, hash: T::Hash) -> DispatchResultWithPostInfo {
#[weight = 210_000_000 + 1_100_000 * T::Tippers::count_upper_bound() as u64
+ T::DbWeight::get().reads_writes(3, 3)]
fn close_tip(origin, hash: T::Hash) {
ensure_signed(origin)?;

let tip = Tips::<T>::get(hash).ok_or(Error::<T>::UnknownTip)?;
Expand All @@ -577,8 +570,6 @@ decl_module! {
Reasons::<T>::remove(&tip.reason);
Tips::<T>::remove(hash);
Self::payout_tip(hash, tip);

Ok(Some(MAX_TIPPERS_COUNT.saturating_sub(T::Tippers::count() as u64) * 1_094_000).into())
}

/// # <weight>
Expand All @@ -593,7 +584,7 @@ decl_module! {
if (n % T::SpendPeriod::get()).is_zero() {
let approvals_len = Self::spend_funds();

263_500_000 * approvals_len
260_000_000 * approvals_len
+ T::DbWeight::get().reads_writes(2 + approvals_len * 3, 2 + approvals_len * 3)
} else {
0
Expand Down
5 changes: 5 additions & 0 deletions frame/treasury/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ impl Contains<u64> for TenToFourteen {
})
}
}
impl ContainsCountUpperBound for TenToFourteen {
fn count_upper_bound() -> usize {
TEN_TO_FOURTEEN.with(|v| v.borrow().len())
}
}
parameter_types! {
pub const ProposalBond: Permill = Permill::from_percent(5);
pub const ProposalBondMinimum: u64 = 1;
Expand Down