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

Calculate and refund weight for identity pallet #5680

Merged
gnunicorn merged 39 commits into from
Apr 24, 2020
Merged

Calculate and refund weight for identity pallet #5680

Changes from 1 commit
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions frame/identity/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ use sp_runtime::traits::{StaticLookup, Zero, AppendZerosInput};
use frame_support::{
decl_module, decl_event, decl_storage, ensure, decl_error,
traits::{Currency, ReservableCurrency, OnUnbalanced, Get, BalanceStatus, EnsureOrigin},
weights::{SimpleDispatchInfo, MINIMUM_WEIGHT},
weights::{SimpleDispatchInfo, FunctionOf, DispatchClass},
};
use frame_system::{self as system, ensure_signed, ensure_root};

Expand Down Expand Up @@ -474,18 +474,28 @@ decl_module! {
/// - One storage mutation (codec `O(R)`).
/// - One event.
/// # </weight>
#[weight = SimpleDispatchInfo::FixedNormal(MINIMUM_WEIGHT)]
fn add_registrar(origin, account: T::AccountId) {
#[weight = FunctionOf(
|(_, &old_count): (&T::AccountId, &u32)| {
T::DbWeight::get().reads_writes(1, 1)
+ 500_000 * (old_count as u64)
Comment thread
apopiak marked this conversation as resolved.
Outdated
},
DispatchClass::Normal,
true
)]
fn add_registrar(origin, account: T::AccountId, old_registrar_count: u32) -> DispatchResult {
Comment thread
apopiak marked this conversation as resolved.
Outdated
T::RegistrarOrigin::try_origin(origin)
.map(|_| ())
.or_else(ensure_root)?;
let mut registrars = Self::registrars();
ensure!(registrars.len() as u32 <= old_registrar_count, "invalid count");

let i = <Registrars<T>>::mutate(|r| {
r.push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() }));
(r.len() - 1) as RegistrarIndex
});
registrars.push(Some(RegistrarInfo { account, fee: Zero::zero(), fields: Default::default() }));
let i = (registrars.len() - 1) as RegistrarIndex;
<Registrars<T>>::put(registrars);
Comment thread
apopiak marked this conversation as resolved.
Outdated

Self::deposit_event(RawEvent::RegistrarAdded(i));

Ok(())
}

/// Set an account's identity information and reserve the appropriate deposit.
Expand Down Expand Up @@ -1015,7 +1025,7 @@ mod tests {
#[test]
fn adding_registrar_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
let fields = IdentityFields(IdentityField::Display | IdentityField::Legal);
assert_ok!(Identity::set_fields(Origin::signed(3), 0, fields));
Expand All @@ -1028,7 +1038,7 @@ mod tests {
#[test]
fn registration_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
let mut three_fields = ten();
three_fields.additional.push(Default::default());
Expand All @@ -1055,7 +1065,7 @@ mod tests {
Error::<Test>::InvalidIndex
);

assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_noop!(
Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable),
Error::<Test>::InvalidTarget
Expand All @@ -1079,7 +1089,7 @@ mod tests {
#[test]
fn clearing_judgement_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
assert_ok!(Identity::provide_judgement(Origin::signed(3), 0, 10, Judgement::Reasonable));
assert_ok!(Identity::clear_identity(Origin::signed(10)));
Expand Down Expand Up @@ -1165,7 +1175,7 @@ mod tests {
#[test]
fn cancelling_requested_judgement_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
assert_noop!(Identity::cancel_request(Origin::signed(10), 0), Error::<Test>::NoIdentity);
assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
Expand All @@ -1182,7 +1192,7 @@ mod tests {
#[test]
fn requesting_judgement_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
assert_ok!(Identity::set_identity(Origin::signed(10), ten()));
assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 9), Error::<Test>::FeeChanged);
Expand All @@ -1200,7 +1210,7 @@ mod tests {
assert_noop!(Identity::request_judgement(Origin::signed(10), 0, 10), Error::<Test>::StickyJudgement);

// Requesting from a second registrar still works.
assert_ok!(Identity::add_registrar(Origin::signed(1), 4));
assert_ok!(Identity::add_registrar(Origin::signed(1), 4, 1));
assert_ok!(Identity::request_judgement(Origin::signed(10), 1, 10));

// Re-requesting after the judgement has been reduced works.
Expand All @@ -1212,7 +1222,7 @@ mod tests {
#[test]
fn field_deposit_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
assert_ok!(Identity::set_fee(Origin::signed(3), 0, 10));
assert_ok!(Identity::set_identity(Origin::signed(10), IdentityInfo {
additional: vec![
Expand All @@ -1227,7 +1237,7 @@ mod tests {
#[test]
fn setting_account_id_should_work() {
new_test_ext().execute_with(|| {
assert_ok!(Identity::add_registrar(Origin::signed(1), 3));
assert_ok!(Identity::add_registrar(Origin::signed(1), 3, 0));
// account 4 cannot change the first registrar's identity since it's owned by 3.
assert_noop!(Identity::set_account_id(Origin::signed(4), 0, 3), Error::<Test>::InvalidIndex);
// account 3 can, because that's the registrar's current account.
Expand Down