diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d225d3e478033..2839944b9f65e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -599,6 +599,7 @@ parameter_types! { pub const SubAccountDeposit: Balance = 2 * DOLLARS; // 53 bytes on-chain pub const MaxSubAccounts: u32 = 100; pub const MaxAdditionalFields: u32 = 100; + pub const MaxRegistrars: u32 = 20; } impl pallet_identity::Trait for Runtime { @@ -609,6 +610,7 @@ impl pallet_identity::Trait for Runtime { type SubAccountDeposit = SubAccountDeposit; type MaxSubAccounts = MaxSubAccounts; type MaxAdditionalFields = MaxAdditionalFields; + type MaxRegistrars = MaxRegistrars; type Slashed = Treasury; type ForceOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>; type RegistrarOrigin = pallet_collective::EnsureProportionMoreThan<_1, _2, AccountId, CouncilCollective>; diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 98d6a93738147..5f86bbcec0350 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -1099,7 +1099,7 @@ impl, I: Instance> Currency for Module where // equal and opposite cause (returned as an Imbalance), then in the // instance that there's no other accounts on the system at all, we might // underflow the issuance and our arithmetic will be off. - ensure!(value + account.reserved >= ed || !account.total().is_zero(), ()); + ensure!(value.saturating_add(account.reserved) >= ed || !account.total().is_zero(), ()); let imbalance = if account.free <= value { SignedImbalance::Positive(PositiveImbalance::new(value - account.free)) diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 6bb10f3d97216..c7f5e005035f3 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -69,6 +69,9 @@ pub use paste; /// (or not) by each arm. Syntax is available to allow for only the range to be drawn upon if /// desired, allowing an alternative instancing expression to be given. /// +/// Note that the ranges are *inclusive* on both sides. This is in contrast to ranges in Rust which +/// are left-inclusive right-exclusive. +/// /// Each arm may also have a block of code which is run prior to any instancing and a block of code /// which is run afterwards. All code blocks may draw upon the specific value of each parameter /// at any time. Local variables are shared between the two pre- and post- code blocks, but do not @@ -81,6 +84,7 @@ pub use paste; /// ```ignore /// benchmarks! { /// // common parameter; just one for this example. +/// // will be `1`, `MAX_LENGTH` or any value inbetween /// _ { /// let l in 1 .. MAX_LENGTH => initialize_l(l); /// } diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index fe99cd990720c..81a9f3e1340cf 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -27,9 +27,6 @@ use sp_runtime::traits::Bounded; use crate::Module as Identity; -// The maximum number of identity registrars we will test. -const MAX_REGISTRARS: u32 = 50; - // Support Functions fn account(name: &'static str, index: u32) -> T::AccountId { let entropy = (name, index).using_encoded(blake2_256); @@ -53,9 +50,9 @@ fn add_registrars(r: u32) -> Result<(), &'static str> { Ok(()) } -// Adds `s` sub-accounts to the identity of `who`. Each wil have 32 bytes of raw data added to it. -// This additionally returns the vector of sub-accounts to it can be modified if needed. -fn add_sub_accounts(who: &T::AccountId, s: u32) -> Result, &'static str> { +// Create `s` sub-accounts for the identity of `who` and return them. +// Each will have 32 bytes of raw data added to it. +fn create_sub_accounts(who: &T::AccountId, s: u32) -> Result, &'static str> { let mut subs = Vec::new(); let who_origin = RawOrigin::Signed(who.clone()); let data = Data::Raw(vec![0; 32]); @@ -70,9 +67,18 @@ fn add_sub_accounts(who: &T::AccountId, s: u32) -> Result(1); Identity::::set_identity(who_origin.clone().into(), info)?; + Ok(subs) +} + +// Adds `s` sub-accounts to the identity of `who`. Each will have 32 bytes of raw data added to it. +// This additionally returns the vector of sub-accounts so it can be modified if needed. +fn add_sub_accounts(who: &T::AccountId, s: u32) -> Result, &'static str> { + let who_origin = RawOrigin::Signed(who.clone()); + let subs = create_sub_accounts::(who, s)?; + Identity::::set_subs(who_origin.into(), subs.clone())?; - return Ok(subs) + Ok(subs) } // This creates an `IdentityInfo` object with `num_fields` extra fields. @@ -98,7 +104,9 @@ fn create_identity_info(num_fields: u32) -> IdentityInfo { benchmarks! { // These are the common parameters along with their instancing. _ { - let r in 1 .. MAX_REGISTRARS => add_registrars::(r)?; + let r in 1 .. T::MaxRegistrars::get() => add_registrars::(r)?; + // extra parameter for the set_subs bench for previous sub accounts + let p in 1 .. T::MaxSubAccounts::get() => (); let s in 1 .. T::MaxSubAccounts::get() => { // Give them s many sub accounts let caller = account::("caller", 0); @@ -114,7 +122,7 @@ benchmarks! { } add_registrar { - let r in ...; + let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; }: _(RawOrigin::Root, account::("registrar", r + 1)) set_identity { @@ -153,16 +161,13 @@ benchmarks! { set_subs { let caller = account::("caller", 0); - // Give them s many sub accounts. - let s in 1 .. T::MaxSubAccounts::get() - 1 => { - let _ = add_sub_accounts::(&caller, s)?; + // Give them p many previous sub accounts. + let p in 1 .. T::MaxSubAccounts::get() => { + let _ = add_sub_accounts::(&caller, p)?; }; - - let mut subs = Module::::subs(&caller); - - // Create an s + 1 sub account. - let data = Data::Raw(vec![0; 32]); - subs.push((account::("sub", s + 1), data)); + // Create a new subs vec with s sub accounts + let s in 1 .. T::MaxSubAccounts::get() => (); + let subs = create_sub_accounts::(&caller, s)?; }: _(RawOrigin::Signed(caller), subs) @@ -210,7 +215,7 @@ benchmarks! { set_fee { let caller = account::("caller", 0); - let r in ...; + let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; }: _(RawOrigin::Signed(caller), r, 10.into()) @@ -219,7 +224,7 @@ benchmarks! { let caller = account::("caller", 0); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in ...; + let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; }: _(RawOrigin::Signed(caller), r, account::("new", 0)) @@ -228,7 +233,7 @@ benchmarks! { let caller = account::("caller", 0); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in ...; + let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; let fields = IdentityFields( @@ -247,7 +252,7 @@ benchmarks! { let caller = account::("caller", 0); let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let r in ...; + let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::(r)?; // For this x, it's the user identity that gts the fields, not the caller. let x in _ .. _ => { let info = create_identity_info::(x); diff --git a/frame/identity/src/lib.rs b/frame/identity/src/lib.rs index 31eb93f04b21a..273cf5f71a544 100644 --- a/frame/identity/src/lib.rs +++ b/frame/identity/src/lib.rs @@ -69,12 +69,13 @@ use sp_std::prelude::*; use sp_std::{fmt::Debug, ops::Add, iter::once}; use enumflags2::BitFlags; use codec::{Encode, Decode}; -use sp_runtime::{DispatchResult, RuntimeDebug}; +use sp_runtime::{DispatchError, RuntimeDebug}; use sp_runtime::traits::{StaticLookup, Zero, AppendZerosInput}; use frame_support::{ decl_module, decl_event, decl_storage, ensure, decl_error, + dispatch::DispatchResultWithPostInfo, traits::{Currency, ReservableCurrency, OnUnbalanced, Get, BalanceStatus, EnsureOrigin}, - weights::MINIMUM_WEIGHT, + weights::Weight, }; use frame_system::{self as system, ensure_signed, ensure_root}; @@ -108,6 +109,10 @@ pub trait Trait: frame_system::Trait { /// required to access an identity, but can be pretty high. type MaxAdditionalFields: Get; + /// Maxmimum number of registrars allowed in the system. Needed to bound the complexity + /// of, e.g., updating judgements. + type MaxRegistrars: Get; + /// What to do with slashed funds. type Slashed: OnUnbalanced>; @@ -451,9 +456,107 @@ decl_error! { InvalidTarget, /// Too many additional fields. TooManyFields, + /// Maximum amount of registrars reached. Cannot add any more. + TooManyRegistrars, } } +/// Functions for calcuating the weight of dispatchables. +mod weight_for { + use frame_support::weights::{RuntimeDbWeight, Weight}; + + /// Weight calculation for `set_identity`. + pub(crate) fn set_identity( + db: RuntimeDbWeight, + judgements: impl Into, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(1, 1) + + 150_000_000 // constant + + 700_000 * judgements.into() // R + + 3_000_000 * extra_fields.into() // X + } + + /// Weight calculation for `set_subs`. + pub(crate) fn set_subs( + db: RuntimeDbWeight, + old_subs: impl Into + Copy, + subs: impl Into + Copy + ) -> Weight { + db.reads(1) // storage-exists (`IdentityOf::contains_key`) + + db.reads_writes(1, old_subs.into()) // `SubsOf::get` read + P old DB deletions + + db.writes(subs.into() + 1) // S + 1 new DB writes + + 130_000_000 // constant + + 5_200_000 * old_subs.into() // P + + 7_300_000 * subs.into() // S + } + + /// Weight calculation for `clear_identity`. + pub(crate) fn clear_identity( + db: RuntimeDbWeight, + judgements: impl Into, + subs: impl Into + Copy, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(2, subs.into() + 2) // S + 2 deletions + + 160_000_000 // constant + + 500_000 * judgements.into() // R + + 5_400_000 * subs.into() // S + + 2_000_000 * extra_fields.into() // X + } + + /// Weight calculation for `request_judgement`. + pub(crate) fn request_judgement( + db: RuntimeDbWeight, + judgements: impl Into, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(2, 1) + + 180_000_000 // constant + + 950_000 * judgements.into() // R + + 3_400_000 * extra_fields.into() // X + } + + /// Weight calculation for `cancel_request`. + pub(crate) fn cancel_request( + db: RuntimeDbWeight, + judgements: impl Into, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(1, 1) + + 150_000_000 // constant + + 600_000 * judgements.into() // R + + 3_600_000 * extra_fields.into() // X + } + + /// Weight calculation for `provide_judgement`. + pub(crate) fn provide_judgement( + db: RuntimeDbWeight, + judgements: impl Into, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(2, 1) + + 120_000_000 // constant + + 1_100_000 * judgements.into() // R + + 3_500_000 * extra_fields.into()// X + } + + /// Weight calculation for `kill_identity`. + pub(crate) fn kill_identity( + db: RuntimeDbWeight, + judgements: impl Into, + subs: impl Into + Copy, + extra_fields: impl Into + ) -> Weight { + db.reads_writes(3, subs.into() + 3) // 2 `take`s + S deletions + + db.reads_writes(1, 1) // balance ops + + 170_000_000 // constant + + 1_200_000 * judgements.into() // R + + 5_400_000 * subs.into() // S + + 2_300_000 * extra_fields.into() // X + } +} + decl_module! { // Simple declaration of the `Module` type. Lets the macro know what it's working on. pub struct Module for enum Call where origin: T::Origin { @@ -470,22 +573,32 @@ decl_module! { /// Emits `RegistrarAdded` if successful. /// /// # - /// - `O(R)` where `R` registrar-count (governance-bounded). + /// - `O(R)` where `R` registrar-count (governance-bounded and code-bounded). /// - One storage mutation (codec `O(R)`). /// - One event. + /// - Benchmarks: + /// - 78.71 + R * 0.965 µs (min squares analysis) + /// - 94.28 + R * 0.991 µs (min squares analysis) /// # - #[weight = MINIMUM_WEIGHT] - fn add_registrar(origin, account: T::AccountId) { + #[weight = T::DbWeight::get().reads_writes(1, 1) + + 95_000_000 // constant + + 1_000_000 * T::MaxRegistrars::get() as Weight // R + ] + fn add_registrar(origin, account: T::AccountId) -> DispatchResultWithPostInfo { T::RegistrarOrigin::try_origin(origin) .map(|_| ()) .or_else(ensure_root)?; - let i = >::mutate(|r| { - r.push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() })); - (r.len() - 1) as RegistrarIndex - }); + let (i, registrar_count) = >::try_mutate(|registrars| -> Result<(RegistrarIndex, usize), DispatchError> { + ensure!((registrars.len() as u32) < T::MaxRegistrars::get(), Error::::TooManyRegistrars); + registrars.push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() })); + Ok(((registrars.len() - 1) as RegistrarIndex, registrars.len())) + })?; Self::deposit_event(RawEvent::RegistrarAdded(i)); + + Ok(Some(T::DbWeight::get().reads_writes(1, 1) + + 95_000_000 + 1_000_000 * registrar_count as Weight).into()) } /// Set an account's identity information and reserve the appropriate deposit. @@ -493,21 +606,29 @@ decl_module! { /// If the account already has identity information, the deposit is taken as part payment /// for the new deposit. /// - /// The dispatch origin for this call must be _Signed_ and the sender must have a registered - /// identity. + /// The dispatch origin for this call must be _Signed_. /// /// - `info`: The identity information. /// /// Emits `IdentitySet` if successful. /// /// # - /// - `O(X + X' + R)` where `X` additional-field-count (deposit-bounded and code-bounded). - /// - At most two balance operations. + /// - `O(X + X' + R)` + /// - where `X` additional-field-count (deposit-bounded and code-bounded) + /// - where `R` judgements-count (registrar-count-bounded) + /// - One balance reserve operation. /// - One storage mutation (codec-read `O(X' + R)`, codec-write `O(X + R)`). /// - One event. + /// - Benchmarks: + /// - 136.6 + R * 0.62 + X * 2.62 µs (min squares analysis) + /// - 146.2 + R * 0.372 + X * 2.98 µs (min squares analysis) /// # - #[weight = 50_000_000] - fn set_identity(origin, info: IdentityInfo) { + #[weight = weight_for::set_identity( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxAdditionalFields::get() // X + )] + fn set_identity(origin, info: IdentityInfo) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let extra_fields = info.additional.len() as u32; ensure!(extra_fields <= T::MaxAdditionalFields::get(), Error::::TooManyFields); @@ -532,8 +653,15 @@ decl_module! { let _ = T::Currency::unreserve(&sender, old_deposit - id.deposit); } + let judgements = id.judgements.len() as Weight; >::insert(&sender, id); Self::deposit_event(RawEvent::IdentitySet(sender)); + + Ok(Some(weight_for::set_identity( + T::DbWeight::get(), + judgements, // R + extra_fields as Weight // X + )).into()) } /// Set the sub-accounts of the sender. @@ -544,16 +672,28 @@ decl_module! { /// The dispatch origin for this call must be _Signed_ and the sender must have a registered /// identity. /// - /// - `subs`: The identity's sub-accounts. + /// - `subs`: The identity's (new) sub-accounts. /// /// # - /// - `O(S)` where `S` subs-count (hard- and deposit-bounded). - /// - At most two balance operations. - /// - At most O(2 * S + 1) storage mutations; codec complexity `O(1 * S + S * 1)`); - /// one storage-exists. + /// - `O(P + S)` + /// - where `P` old-subs-count (hard- and deposit-bounded). + /// - where `S` subs-count (hard- and deposit-bounded). + /// - At most one balance operations. + /// - DB: + /// - `P + S` storage mutations (codec complexity `O(1)`) + /// - One storage read (codec complexity `O(P)`). + /// - One storage write (codec complexity `O(S)`). + /// - One storage-exists (`IdentityOf::contains_key`). + /// - Benchmarks: + /// - 115.2 + P * 5.11 + S * 6.67 µs (min squares analysis) + /// - 121 + P * 4.852 + S * 7.111 µs (min squares analysis) /// # - #[weight = 50_000_000] - fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) { + #[weight = weight_for::set_subs( + T::DbWeight::get(), + T::MaxSubAccounts::get(), // P + subs.len() as Weight // S + )] + fn set_subs(origin, subs: Vec<(T::AccountId, Data)>) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; ensure!(>::contains_key(&sender), Error::::NotFound); ensure!(subs.len() <= T::MaxSubAccounts::get() as usize, Error::::TooManySubAccounts); @@ -576,15 +716,22 @@ decl_module! { >::insert(&id, (sender.clone(), name)); id }).collect::>(); + let new_subs = ids.len() as Weight; if ids.is_empty() { >::remove(&sender); } else { >::insert(&sender, (new_deposit, ids)); } + + Ok(Some(weight_for::set_subs( + T::DbWeight::get(), + old_ids.len() as Weight, // P + new_subs // S + )).into()) } - /// Clear an account's identity info and all sub-account and return all deposits. + /// Clear an account's identity info and all sub-accounts and return all deposits. /// /// Payment: All reserved balances on the account are returned. /// @@ -594,17 +741,29 @@ decl_module! { /// Emits `IdentityCleared` if successful. /// /// # - /// - `O(R + S + X)`. - /// - One balance-reserve operation. - /// - `S + 2` storage deletions. + /// - `O(R + S + X)` + /// - where `R` registrar-count (governance-bounded). + /// - where `S` subs-count (hard- and deposit-bounded). + /// - where `X` additional-field-count (deposit-bounded and code-bounded). + /// - One balance-unreserve operation. + /// - `2` storage reads and `S + 2` storage deletions. /// - One event. + /// - Benchmarks: + /// - 152.3 + R * 0.306 + S * 4.967 + X * 1.697 µs (min squares analysis) + /// - 139.5 + R * 0.466 + S * 5.304 + X * 1.895 µs (min squares analysis) /// # - #[weight = 50_000_000] - fn clear_identity(origin) { + #[weight = weight_for::clear_identity( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxSubAccounts::get(), // S + T::MaxAdditionalFields::get() // X + )] + fn clear_identity(origin) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let (subs_deposit, sub_ids) = >::take(&sender); - let deposit = >::take(&sender).ok_or(Error::::NotNamed)?.total_deposit() + let id = >::take(&sender).ok_or(Error::::NotNamed)?; + let deposit = id.total_deposit() + subs_deposit; for sub in sub_ids.iter() { >::remove(sub); @@ -613,6 +772,13 @@ decl_module! { let _ = T::Currency::unreserve(&sender, deposit.clone()); Self::deposit_event(RawEvent::IdentityCleared(sender, deposit)); + + Ok(Some(weight_for::clear_identity( + T::DbWeight::get(), + id.judgements.len() as Weight, // R + sub_ids.len() as Weight, // S + id.info.additional.len() as Weight // X + )).into()) } /// Request a judgement from a registrar. @@ -627,7 +793,7 @@ decl_module! { /// - `max_fee`: The maximum fee that may be paid. This should just be auto-populated as: /// /// ```nocompile - /// Self::registrars(reg_index).unwrap().fee + /// Self::registrars().get(reg_index).unwrap().fee /// ``` /// /// Emits `JudgementRequested` if successful. @@ -637,12 +803,19 @@ decl_module! { /// - One balance-reserve operation. /// - Storage: 1 read `O(R)`, 1 mutate `O(X + R)`. /// - One event. + /// - Benchmarks: + /// - 154 + R * 0.932 + X * 3.302 µs (min squares analysis) + /// - 172.9 + R * 0.69 + X * 3.304 µs (min squares analysis) /// # - #[weight = 50_000_000] + #[weight = weight_for::request_judgement( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxAdditionalFields::get() // X + )] fn request_judgement(origin, #[compact] reg_index: RegistrarIndex, #[compact] max_fee: BalanceOf, - ) { + ) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let registrars = >::get(); let registrar = registrars.get(reg_index as usize).and_then(Option::as_ref) @@ -662,9 +835,13 @@ decl_module! { T::Currency::reserve(&sender, registrar.fee)?; + let judgements = id.judgements.len() as Weight; + let extra_fields = id.info.additional.len() as Weight; >::insert(&sender, id); Self::deposit_event(RawEvent::JudgementRequested(sender, reg_index)); + + Ok(Some(weight_for::request_judgement(T::DbWeight::get(), judgements, extra_fields)).into()) } /// Cancel a previous request. @@ -683,9 +860,16 @@ decl_module! { /// - One balance-reserve operation. /// - One storage mutation `O(R + X)`. /// - One event. + /// - Benchmarks: + /// - 135.3 + R * 0.574 + X * 3.394 µs (min squares analysis) + /// - 144.3 + R * 0.316 + X * 3.53 µs (min squares analysis) /// # - #[weight = 50_000_000] - fn cancel_request(origin, reg_index: RegistrarIndex) { + #[weight = weight_for::cancel_request( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxAdditionalFields::get() // X + )] + fn cancel_request(origin, reg_index: RegistrarIndex) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let mut id = >::get(&sender).ok_or(Error::::NoIdentity)?; @@ -698,9 +882,13 @@ decl_module! { }; let _ = T::Currency::unreserve(&sender, fee); + let judgements = id.judgements.len() as Weight; + let extra_fields = id.info.additional.len() as Weight; >::insert(&sender, id); Self::deposit_event(RawEvent::JudgementUnrequested(sender, reg_index)); + + Ok(Some(weight_for::request_judgement(T::DbWeight::get(), judgements, extra_fields)).into()) } /// Set the fee required for a judgement to be requested from a registrar. @@ -714,20 +902,29 @@ decl_module! { /// # /// - `O(R)`. /// - One storage mutation `O(R)`. + /// - Benchmarks: + /// - 23.81 + R * 0.774 µs (min squares analysis) /// # - #[weight = 50_000_000] + #[weight = T::DbWeight::get().reads_writes(1, 1) + + 24_000_000 // constant + + 780_000 * T::MaxRegistrars::get() as Weight // R + ] fn set_fee(origin, #[compact] index: RegistrarIndex, #[compact] fee: BalanceOf, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - >::mutate(|rs| + let registrars = >::mutate(|rs| -> Result { rs.get_mut(index as usize) .and_then(|x| x.as_mut()) .and_then(|r| if r.account == who { r.fee = fee; Some(()) } else { None }) - .ok_or_else(|| Error::::InvalidIndex.into()) - ) + .ok_or_else(|| DispatchError::from(Error::::InvalidIndex))?; + Ok(rs.len()) + })?; + Ok(Some(T::DbWeight::get().reads_writes(1, 1) + + 24_000_000 + 780_000 * registrars as Weight // R + ).into()) } /// Change the account associated with a registrar. @@ -741,20 +938,28 @@ decl_module! { /// # /// - `O(R)`. /// - One storage mutation `O(R)`. + /// - Benchmark: 24.59 + R * 0.832 µs (min squares analysis) /// # - #[weight = 50_000_000] + #[weight = T::DbWeight::get().reads_writes(1, 1) + + 25_000_000 // constant + + 850_000 * T::MaxRegistrars::get() as Weight // R + ] fn set_account_id(origin, #[compact] index: RegistrarIndex, new: T::AccountId, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - >::mutate(|rs| + let registrars = >::mutate(|rs| -> Result { rs.get_mut(index as usize) .and_then(|x| x.as_mut()) .and_then(|r| if r.account == who { r.account = new; Some(()) } else { None }) - .ok_or_else(|| Error::::InvalidIndex.into()) - ) + .ok_or_else(|| DispatchError::from(Error::::InvalidIndex))?; + Ok(rs.len()) + })?; + Ok(Some(T::DbWeight::get().reads_writes(1, 1) + + 25_000_000 + 850_000 * registrars as Weight // R + ).into()) } /// Set the field information for a registrar. @@ -768,20 +973,28 @@ decl_module! { /// # /// - `O(R)`. /// - One storage mutation `O(R)`. + /// - Benchmark: 22.85 + R * 0.853 µs (min squares analysis) /// # - #[weight = 50_000_000] + #[weight = T::DbWeight::get().reads_writes(1, 1) + + 23_000_000 // constant + + 860_000 * T::MaxRegistrars::get() as Weight // R + ] fn set_fields(origin, #[compact] index: RegistrarIndex, fields: IdentityFields, - ) -> DispatchResult { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - >::mutate(|rs| + let registrars = >::mutate(|rs| -> Result { rs.get_mut(index as usize) .and_then(|x| x.as_mut()) .and_then(|r| if r.account == who { r.fields = fields; Some(()) } else { None }) - .ok_or_else(|| Error::::InvalidIndex.into()) - ) + .ok_or_else(|| DispatchError::from(Error::::InvalidIndex))?; + Ok(rs.len()) + })?; + Ok(Some(T::DbWeight::get().reads_writes(1, 1) + + 23_000_000 + 860_000 * registrars as Weight // R + ).into()) } /// Provide a judgement for an account's identity. @@ -802,13 +1015,18 @@ decl_module! { /// - Up to one account-lookup operation. /// - Storage: 1 read `O(R)`, 1 mutate `O(R + X)`. /// - One event. + /// - Benchmark: 110.7 + R * 1.066 + X * 3.402 µs (min squares analysis) /// # - #[weight = 50_000_000] + #[weight = weight_for::provide_judgement( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxAdditionalFields::get() // X + )] fn provide_judgement(origin, #[compact] reg_index: RegistrarIndex, target: ::Source, judgement: Judgement>, - ) { + ) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let target = T::Lookup::lookup(target)?; ensure!(!judgement.has_deposit(), Error::::InvalidJudgement); @@ -829,8 +1047,13 @@ decl_module! { } Err(position) => id.judgements.insert(position, item), } + + let judgements = id.judgements.len() as Weight; + let extra_fields = id.info.additional.len() as Weight; >::insert(&target, id); Self::deposit_event(RawEvent::JudgementGiven(target, reg_index)); + + Ok(Some(weight_for::provide_judgement(T::DbWeight::get(), judgements, extra_fields)).into()) } /// Remove an account's identity and sub-account information and slash the deposits. @@ -851,9 +1074,15 @@ decl_module! { /// - One balance-reserve operation. /// - `S + 2` storage mutations. /// - One event. + /// - Benchmark: 167.4 + R * 1.107 + S * 5.343 + X * 2.294 µs (min squares analysis) /// # - #[weight = 100_000_000] - fn kill_identity(origin, target: ::Source) { + #[weight = weight_for::kill_identity( + T::DbWeight::get(), + T::MaxRegistrars::get(), // R + T::MaxSubAccounts::get(), // S + T::MaxAdditionalFields::get() // X + )] + fn kill_identity(origin, target: ::Source) -> DispatchResultWithPostInfo { T::ForceOrigin::try_origin(origin) .map(|_| ()) .or_else(ensure_root)?; @@ -862,8 +1091,8 @@ decl_module! { let target = T::Lookup::lookup(target)?; // Grab their deposit (and check that they have one). let (subs_deposit, sub_ids) = >::take(&target); - let deposit = >::take(&target).ok_or(Error::::NotNamed)?.total_deposit() - + subs_deposit; + let id = >::take(&target).ok_or(Error::::NotNamed)?; + let deposit = id.total_deposit() + subs_deposit; for sub in sub_ids.iter() { >::remove(sub); } @@ -871,6 +1100,13 @@ decl_module! { T::Slashed::on_unbalanced(T::Currency::slash_reserved(&target, deposit).0); Self::deposit_event(RawEvent::IdentityKilled(target, deposit)); + + Ok(Some(weight_for::kill_identity( + T::DbWeight::get(), + id.judgements.len() as Weight, // R + sub_ids.len() as Weight, // S + id.info.additional.len() as Weight // X + )).into()) } } } @@ -955,6 +1191,7 @@ mod tests { pub const SubAccountDeposit: u64 = 10; pub const MaxSubAccounts: u32 = 2; pub const MaxAdditionalFields: u32 = 2; + pub const MaxRegistrars: u32 = 20; } ord_parameter_types! { pub const One: u64 = 1; @@ -969,6 +1206,7 @@ mod tests { type SubAccountDeposit = SubAccountDeposit; type MaxSubAccounts = MaxSubAccounts; type MaxAdditionalFields = MaxAdditionalFields; + type MaxRegistrars = MaxRegistrars; type RegistrarOrigin = EnsureSignedBy; type ForceOrigin = EnsureSignedBy; } @@ -1025,6 +1263,20 @@ mod tests { }); } + #[test] + fn amount_of_registrars_is_limited() { + new_test_ext().execute_with(|| { + for i in 1..MaxRegistrars::get() + 1 { + assert_ok!(Identity::add_registrar(Origin::signed(1), i as u64)); + } + let last_registrar = MaxRegistrars::get() as u64 + 1; + assert_noop!( + Identity::add_registrar(Origin::signed(1), last_registrar), + Error::::TooManyRegistrars + ); + }); + } + #[test] fn registration_should_work() { new_test_ext().execute_with(|| { diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 35e1231698a46..dfc2ddf4de129 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -208,7 +208,7 @@ pub trait Contains { /// /// **Should be used for benchmarking only!!!** #[cfg(feature = "runtime-benchmarks")] - fn add(t: &T) { unimplemented!() } + fn add(_t: &T) { unimplemented!() } } /// Determiner to say whether a given account is unused.