Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Calculate and refund weight for identity pallet #5680

Merged
merged 39 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
bd03eb7
add old_registrar_count as param to estimate weight
apopiak Apr 17, 2020
eeaf09f
cast count to Weight
apopiak Apr 17, 2020
abf2bf2
add weight calculation for set_identity
apopiak Apr 17, 2020
b4cb9de
remove superfluous weight comment
apopiak Apr 17, 2020
77e4568
add detailed weight estimation for set_subs
apopiak Apr 20, 2020
3c6bb77
adjust benchmarking code to the new API
apopiak Apr 20, 2020
6a64c7e
add second parameter to set_subs benchmark
apopiak Apr 20, 2020
31ba72f
rename o to p
apopiak Apr 20, 2020
d4feebe
calculate weight based on benchmarks
apopiak Apr 20, 2020
f34aad1
Merge remote-tracking branch 'origin' into apopiak-weight-params
apopiak Apr 20, 2020
dd023c1
Merge remote-tracking branch 'origin/master' into apopiak-weight-params
apopiak Apr 20, 2020
6d08bc1
use try_mutate for registrars
apopiak Apr 20, 2020
fa04aab
fix weight number typo
apopiak Apr 20, 2020
3fb9789
update weights for set_subs + add weights for clear_identity and requ…
apopiak Apr 21, 2020
9d027ba
improve naming and docs
apopiak Apr 21, 2020
6c0dfa9
add weight calculation for cancel_request
apopiak Apr 21, 2020
d662f5a
fix benchmark
apopiak Apr 21, 2020
1316345
fix tests
apopiak Apr 21, 2020
c699699
fix arithmetic overflow in balances triggered by tests
apopiak Apr 21, 2020
269f50b
add weight calcluations for more dispatchables
apopiak Apr 21, 2020
8dec341
add weight calculation for provide_judgement
apopiak Apr 21, 2020
155f193
mark param as unused
apopiak Apr 21, 2020
d6f68ff
add MaxRegistrars associated type used for weight estimation
apopiak Apr 21, 2020
ba839d6
check that MaxRegistrars is not exceeded
apopiak Apr 21, 2020
4719cc5
add remaining weight calculations
apopiak Apr 21, 2020
05343cb
Merge branch 'master' into apopiak-weight-params
shawntabrizi Apr 21, 2020
6dce9f1
use weight refunds to use more constants in weight estimation
apopiak Apr 22, 2020
34825db
Merge branch 'apopiak-weight-params' of github.com:paritytech/substra…
apopiak Apr 22, 2020
21ee413
adjust usage of clear_identity
apopiak Apr 23, 2020
9316e8e
refund request_judgement weights and remove param
apopiak Apr 23, 2020
94624ba
refund weights for cancel_request and remove param
apopiak Apr 23, 2020
8574053
add remaining refunds and remove params
apopiak Apr 23, 2020
cb04804
refund weight for set_subs and remove param
apopiak Apr 23, 2020
e0bbfcc
make comment more specific
apopiak Apr 23, 2020
f5ffbb5
add range note to benchmarking docs
apopiak Apr 23, 2020
1072ff8
fix inconsistencies before review
apopiak Apr 23, 2020
e4724da
Merge branch 'master' of github.com:paritytech/substrate into apopiak…
apopiak Apr 23, 2020
98718d9
fix actual weight calculation for add_registrar
apopiak Apr 23, 2020
02f8cab
remove duplicate balance ops weights + refund on all dispatchables
apopiak Apr 24, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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>;
Expand Down
2 changes: 1 addition & 1 deletion frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1099,7 +1099,7 @@ impl<T: Trait<I>, I: Instance> Currency<T::AccountId> for Module<T, I> 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))
Expand Down
4 changes: 4 additions & 0 deletions frame/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up, we can just change the macro syntax to MIN ..= MAX, which I also think solve this syntax mismatch right?

/// 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
Expand All @@ -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);
/// }
Expand Down
49 changes: 27 additions & 22 deletions frame/identity/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: Trait>(name: &'static str, index: u32) -> T::AccountId {
let entropy = (name, index).using_encoded(blake2_256);
Expand All @@ -53,9 +50,9 @@ fn add_registrars<T: Trait>(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<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'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<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
let mut subs = Vec::new();
let who_origin = RawOrigin::Signed(who.clone());
let data = Data::Raw(vec![0; 32]);
Expand All @@ -70,9 +67,18 @@ fn add_sub_accounts<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::Acco
let info = create_identity_info::<T>(1);
Identity::<T>::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<T: Trait>(who: &T::AccountId, s: u32) -> Result<Vec<(T::AccountId, Data)>, &'static str> {
let who_origin = RawOrigin::Signed(who.clone());
let subs = create_sub_accounts::<T>(who, s)?;

Identity::<T>::set_subs(who_origin.into(), subs.clone())?;

return Ok(subs)
Ok(subs)
}

// This creates an `IdentityInfo` object with `num_fields` extra fields.
Expand All @@ -98,7 +104,9 @@ fn create_identity_info<T: Trait>(num_fields: u32) -> IdentityInfo {
benchmarks! {
// These are the common parameters along with their instancing.
_ {
let r in 1 .. MAX_REGISTRARS => add_registrars::<T>(r)?;
let r in 1 .. T::MaxRegistrars::get() => add_registrars::<T>(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::<T>("caller", 0);
Expand All @@ -114,7 +122,7 @@ benchmarks! {
}

add_registrar {
let r in ...;
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
}: _(RawOrigin::Root, account::<T>("registrar", r + 1))

set_identity {
Expand Down Expand Up @@ -153,16 +161,13 @@ benchmarks! {
set_subs {
let caller = account::<T>("caller", 0);

// Give them s many sub accounts.
let s in 1 .. T::MaxSubAccounts::get() - 1 => {
let _ = add_sub_accounts::<T>(&caller, s)?;
// Give them p many previous sub accounts.
let p in 1 .. T::MaxSubAccounts::get() => {
let _ = add_sub_accounts::<T>(&caller, p)?;
};

let mut subs = Module::<T>::subs(&caller);

// Create an s + 1 sub account.
let data = Data::Raw(vec![0; 32]);
subs.push((account::<T>("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::<T>(&caller, s)?;

}: _(RawOrigin::Signed(caller), subs)

Expand Down Expand Up @@ -210,7 +215,7 @@ benchmarks! {
set_fee {
let caller = account::<T>("caller", 0);

let r in ...;
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
}: _(RawOrigin::Signed(caller), r, 10.into())
Expand All @@ -219,7 +224,7 @@ benchmarks! {
let caller = account::<T>("caller", 0);
let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());

let r in ...;
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
}: _(RawOrigin::Signed(caller), r, account::<T>("new", 0))
Expand All @@ -228,7 +233,7 @@ benchmarks! {
let caller = account::<T>("caller", 0);
let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());

let r in ...;
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;

Identity::<T>::add_registrar(RawOrigin::Root.into(), caller.clone())?;
let fields = IdentityFields(
Expand All @@ -247,7 +252,7 @@ benchmarks! {
let caller = account::<T>("caller", 0);
let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());

let r in ...;
let r in 1 .. T::MaxRegistrars::get() - 1 => add_registrars::<T>(r)?;
// For this x, it's the user identity that gts the fields, not the caller.
let x in _ .. _ => {
let info = create_identity_info::<T>(x);
Expand Down