From eb439e4a87068208f31b8d9a417924651da85978 Mon Sep 17 00:00:00 2001 From: Zeke Mostov <32168567+emostov@users.noreply.github.com> Date: Thu, 25 Mar 2021 09:23:22 -0700 Subject: [PATCH 1/9] [pallet-staking] Refund unused weight for `payout_stakers` fixes #8428 --- frame/staking/src/lib.rs | 49 +++++++++---- frame/staking/src/mock.rs | 2 + frame/staking/src/tests.rs | 146 ++++++++++++++++++++++++++++++++++--- 3 files changed, 174 insertions(+), 23 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index fe1738ca33316..802f33b5444e7 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -291,7 +291,7 @@ use codec::{HasCompact, Encode, Decode}; use frame_support::{ decl_module, decl_event, decl_storage, ensure, decl_error, weights::{ - Weight, + Weight, WithPostDispatchInfo, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}, }, storage::IterableStorageMap, @@ -1831,8 +1831,8 @@ decl_module! { /// NOTE: weights are assuming that payouts are made to alive stash account (Staked). /// Paying even a dead controller is cheaper weight-wise. We don't do any refunds here. /// # - #[weight = T::WeightInfo::payout_stakers_alive_staked(T::MaxNominatorRewardedPerValidator::get())] - fn payout_stakers(origin, validator_stash: T::AccountId, era: EraIndex) -> DispatchResult { + #[weight = T::WeightInfo::payout_stakers_alive_staked(T::MaxNominatorRewardedPerValidator::get() + 1)] + fn payout_stakers(origin, validator_stash: T::AccountId, era: EraIndex) -> DispatchResultWithPostInfo { ensure_signed(origin)?; Self::do_payout_stakers(validator_stash, era) } @@ -1996,24 +1996,39 @@ impl Module { }) } - fn do_payout_stakers(validator_stash: T::AccountId, era: EraIndex) -> DispatchResult { + fn do_payout_stakers(validator_stash: T::AccountId, era: EraIndex) -> DispatchResultWithPostInfo { // Validate input data - let current_era = CurrentEra::get().ok_or(Error::::InvalidEraToReward)?; - ensure!(era <= current_era, Error::::InvalidEraToReward); + let current_era = CurrentEra::get().ok_or( + Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + )?; + ensure!( + era <= current_era, + Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + ); let history_depth = Self::history_depth(); - ensure!(era >= current_era.saturating_sub(history_depth), Error::::InvalidEraToReward); + ensure!( + era >= current_era.saturating_sub(history_depth), + Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + ); // Note: if era has no reward to be claimed, era may be future. better not to update // `ledger.claimed_rewards` in this case. let era_payout = >::get(&era) - .ok_or_else(|| Error::::InvalidEraToReward)?; - - let controller = Self::bonded(&validator_stash).ok_or(Error::::NotStash)?; + .ok_or_else(|| + Error::::InvalidEraToReward + .with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + )?; + + let controller = Self::bonded(&validator_stash).ok_or( + Error::::NotStash.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + )?; let mut ledger = >::get(&controller).ok_or_else(|| Error::::NotController)?; ledger.claimed_rewards.retain(|&x| x >= current_era.saturating_sub(history_depth)); match ledger.claimed_rewards.binary_search(&era) { - Ok(_) => Err(Error::::AlreadyClaimed)?, + Ok(_) => Err( + Error::::AlreadyClaimed.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + )?, Err(pos) => ledger.claimed_rewards.insert(pos, era), } @@ -2037,7 +2052,9 @@ impl Module { .unwrap_or_else(|| Zero::zero()); // Nothing to do if they have no reward points. - if validator_reward_points.is_zero() { return Ok(())} + if validator_reward_points.is_zero() { + return Ok(Some(T::WeightInfo::payout_stakers_alive_staked(0)).into()) + } // This is the fraction of the total reward that the validator and the // nominators will get. @@ -2062,11 +2079,15 @@ impl Module { ); let validator_staking_payout = validator_exposure_part * validator_leftover_payout; + // Track the number of payouts in order to track the actual weight + let mut payout_count: u32 = 0; + // We can now make total validator payout: if let Some(imbalance) = Self::make_payout( &ledger.stash, validator_staking_payout + validator_commission_payout ) { + payout_count += 1; Self::deposit_event(RawEvent::Reward(ledger.stash, imbalance.peek())); } @@ -2081,11 +2102,13 @@ impl Module { let nominator_reward: BalanceOf = nominator_exposure_part * validator_leftover_payout; // We can now make nominator payout: if let Some(imbalance) = Self::make_payout(&nominator.who, nominator_reward) { + // Note: this logic does not count payouts for `RewardDestination::None`. + payout_count += 1; Self::deposit_event(RawEvent::Reward(nominator.who.clone(), imbalance.peek())); } } - Ok(()) + Ok(Some(T::WeightInfo::payout_stakers_alive_staked(payout_count)).into()) } /// Update the ledger for a controller. diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index b4c84059a2106..1dc0f20ab7d04 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -268,6 +268,8 @@ where } pub type Extrinsic = TestXt; +pub(crate) type StakingCall = crate::Call; +pub(crate) type TestRuntimeCall = ::Call; pub struct ExtBuilder { validator_pool: bool, diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index df3456bf29926..b7b5b2d370b6a 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -20,12 +20,14 @@ use super::*; use mock::*; use sp_runtime::{ - assert_eq_error_rate, traits::BadOrigin, + assert_eq_error_rate, + traits::{BadOrigin, Dispatchable}, }; use sp_staking::offence::OffenceDetails; use frame_support::{ assert_ok, assert_noop, StorageMap, traits::{Currency, ReservableCurrency, OnInitialize}, + weights::{extract_actual_weight, GetDispatchInfo}, }; use pallet_balances::Error as BalancesError; use substrate_test_utils::assert_eq_uvec; @@ -2976,6 +2978,9 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { // * an invalid era to claim doesn't update last_reward // * double claim of one era fails ExtBuilder::default().nominate(true).build_and_execute(|| { + // Consumed weight for all payout_stakers dispatches that fail + let err_weight = weights::SubstrateWeight::::payout_stakers_alive_staked(0); + let init_balance_10 = Balances::total_balance(&10); let init_balance_100 = Balances::total_balance(&100); @@ -3021,19 +3026,19 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { assert_noop!( Staking::payout_stakers(Origin::signed(1337), 11, 0), // Fail: Era out of history - Error::::InvalidEraToReward + Error::::InvalidEraToReward.with_weight(err_weight) ); assert_ok!(Staking::payout_stakers(Origin::signed(1337), 11, 1)); assert_ok!(Staking::payout_stakers(Origin::signed(1337), 11, 2)); assert_noop!( Staking::payout_stakers(Origin::signed(1337), 11, 2), // Fail: Double claim - Error::::AlreadyClaimed + Error::::AlreadyClaimed.with_weight(err_weight) ); assert_noop!( Staking::payout_stakers(Origin::signed(1337), 11, active_era), // Fail: Era not finished yet - Error::::InvalidEraToReward + Error::::InvalidEraToReward.with_weight(err_weight) ); // Era 0 can't be rewarded anymore and current era can't be rewarded yet @@ -3287,6 +3292,9 @@ fn test_payout_stakers() { fn payout_stakers_handles_basic_errors() { // Here we will test payouts handle all errors. ExtBuilder::default().has_stakers(false).build_and_execute(|| { + // Consumed weight for all payout_stakers dispatches that fail + let err_weight = weights::SubstrateWeight::::payout_stakers_alive_staked(0); + // Same setup as the test above let balance = 1000; bond_validator(11, 10, balance); // Default(64) @@ -3305,9 +3313,15 @@ fn payout_stakers_handles_basic_errors() { mock::start_active_era(2); // Wrong Era, too big - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 11, 2), Error::::InvalidEraToReward); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 11, 2), + Error::::InvalidEraToReward.with_weight(err_weight) + ); // Wrong Staker - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 10, 1), Error::::NotStash); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 10, 1), + Error::::NotStash.with_weight(err_weight) + ); for i in 3..100 { Staking::reward_by_ids(vec![(11, 1)]); @@ -3317,14 +3331,126 @@ fn payout_stakers_handles_basic_errors() { } // We are at era 99, with history depth of 84 // We should be able to payout era 15 through 98 (84 total eras), but not 14 or 99. - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 11, 14), Error::::InvalidEraToReward); - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 11, 99), Error::::InvalidEraToReward); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 11, 14), + Error::::InvalidEraToReward.with_weight(err_weight) + ); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 11, 99), + Error::::InvalidEraToReward.with_weight(err_weight) + ); assert_ok!(Staking::payout_stakers(Origin::signed(1337), 11, 15)); assert_ok!(Staking::payout_stakers(Origin::signed(1337), 11, 98)); // Can't claim again - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 11, 15), Error::::AlreadyClaimed); - assert_noop!(Staking::payout_stakers(Origin::signed(1337), 11, 98), Error::::AlreadyClaimed); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 11, 15), + Error::::AlreadyClaimed.with_weight(err_weight) + ); + assert_noop!( + Staking::payout_stakers(Origin::signed(1337), 11, 98), + Error::::AlreadyClaimed.with_weight(err_weight) + ); + }); +} + +#[test] +fn payout_stakers_handles_weight_refund() { + // N.B. this test relies on the assumption that `payout_stakers_alive_staked` is solely used to + // calculate weight. + ExtBuilder::default().has_stakers(false).build_and_execute(|| { + let max_nom_rewarded = ::MaxNominatorRewardedPerValidator::get(); + let half_max_nom_rewarded = max_nom_rewarded.checked_div(2).unwrap(); + // We add 1 to account for the payout ops to the validator + let max_nom_rewarded_weight + = ::WeightInfo::payout_stakers_alive_staked(max_nom_rewarded +1); + let half_max_nom_rewarded_weight + = ::WeightInfo::payout_stakers_alive_staked(half_max_nom_rewarded + 1); + let zero_payouts_weight = ::WeightInfo::payout_stakers_alive_staked(0); + + let balance = 1000; + bond_validator(11, 10, balance); + + /* Era 1*/ + start_active_era(1); + + // Reward just the validator. + Staking::reward_by_ids(vec![(11, 1)]); + + // Add some `half_max_nom_rewarded` nominators who will start backing the validator in the + // next era. + for i in 0..half_max_nom_rewarded { + bond_nominator((1000 + i).into(), (100 + i).into(), balance + i as Balance, vec![11]); + } + + /* Era 2 */ + start_active_era(2); + + // Collect payouts when there are no nominators + let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 1)); + let info = call.get_dispatch_info(); + let result = call.dispatch(Origin::signed(20)); + assert_ok!(result); + assert_eq!( + extract_actual_weight(&result, &info), + ::WeightInfo::payout_stakers_alive_staked(1) + ); + + // The validator is not rewarded in this era; so there will be zero payouts to claim for this era. + + /* Era 3 */ + start_active_era(3); + + // Collect payouts for an era where the validator did not receive any points. + let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 2)); + let info = call.get_dispatch_info(); + let result = call.dispatch(Origin::signed(20)); + assert_ok!(result); + assert_eq!(extract_actual_weight(&result, &info), zero_payouts_weight); + + // Reward the validator and its nominators. + Staking::reward_by_ids(vec![(11, 1)]); + + /* Era 4 */ + start_active_era(4); + + // Collect payouts when the validator has `half_max_nom_rewarded` nominators. + let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 3)); + let info = call.get_dispatch_info(); + let result = call.dispatch(Origin::signed(20)); + assert_ok!(result); + assert_eq!(extract_actual_weight(&result, &info), half_max_nom_rewarded_weight); + + // Add enough nominators so that we are at the limit. They will be active nominators + // in the next era. + for i in half_max_nom_rewarded..max_nom_rewarded { + bond_nominator((1000 + i).into(), (100 + i).into(), balance + i as Balance, vec![11]); + } + + /* Era 5 */ + start_active_era(5); + // We now have `max_nom_rewarded` nominators actively nominating our validator. + + // Reward the validator so we can collect for everyone in the next era + Staking::reward_by_ids(vec![(11, 1)]); + + /* Era 6 */ + start_active_era(6); + + // Collect payouts when the validator had `half_max_nom_rewarded` nominators + let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 5)); + let info = call.get_dispatch_info(); + let result = call.dispatch(Origin::signed(20)); + assert_ok!(result); + assert_eq!(extract_actual_weight(&result, &info), max_nom_rewarded_weight); + + // Try and collect payouts for an era that has already been collected. + let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 5)); + let info = call.get_dispatch_info(); + let result = call.dispatch(Origin::signed(20)); + assert!(result.is_err()); + // When there is an error the consumed weight == weight when there are 0 payouts. + assert_eq!(extract_actual_weight(&result, &info), zero_payouts_weight); }); } From 583dd3b4e1ae80e5ce11355b011cb8c169da8360 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Thu, 25 Mar 2021 09:42:46 -0700 Subject: [PATCH 2/9] Use periods in comments --- frame/staking/src/lib.rs | 2 +- frame/staking/src/tests.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 802f33b5444e7..b508e8d0fbcf0 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -2079,7 +2079,7 @@ impl Module { ); let validator_staking_payout = validator_exposure_part * validator_leftover_payout; - // Track the number of payouts in order to track the actual weight + // Track the number of payouts in order to track the actual weight. let mut payout_count: u32 = 0; // We can now make total validator payout: diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index b7b5b2d370b6a..c9c467b9a5982 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3431,13 +3431,13 @@ fn payout_stakers_handles_weight_refund() { start_active_era(5); // We now have `max_nom_rewarded` nominators actively nominating our validator. - // Reward the validator so we can collect for everyone in the next era + // Reward the validator so we can collect for everyone in the next era. Staking::reward_by_ids(vec![(11, 1)]); /* Era 6 */ start_active_era(6); - // Collect payouts when the validator had `half_max_nom_rewarded` nominators + // Collect payouts when the validator had `half_max_nom_rewarded` nominators. let call = TestRuntimeCall::Staking(StakingCall::payout_stakers(11, 5)); let info = call.get_dispatch_info(); let result = call.dispatch(Origin::signed(20)); From 0c33cae19d02c88e3b3fc54c53b7f312b312def9 Mon Sep 17 00:00:00 2001 From: Parity Benchmarking Bot Date: Thu, 25 Mar 2021 23:31:37 +0000 Subject: [PATCH 3/9] cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_staking --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/staking/src/weights.rs --template=./.maintain/frame-weight-template.hbs --- frame/staking/src/weights.rs | 206 +++++++++++++++++------------------ 1 file changed, 103 insertions(+), 103 deletions(-) diff --git a/frame/staking/src/weights.rs b/frame/staking/src/weights.rs index 520bef8c539b2..d3274cad8050e 100644 --- a/frame/staking/src/weights.rs +++ b/frame/staking/src/weights.rs @@ -18,7 +18,7 @@ //! Autogenerated weights for pallet_staking //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 -//! DATE: 2021-03-19, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2021-03-25, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: @@ -76,155 +76,155 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn bond() -> Weight { - (82_121_000 as Weight) + (79_895_000 as Weight) .saturating_add(T::DbWeight::get().reads(5 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) } fn bond_extra() -> Weight { - (61_899_000 as Weight) + (60_561_000 as Weight) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn unbond() -> Weight { - (56_392_000 as Weight) + (54_996_000 as Weight) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn withdraw_unbonded_update(s: u32, ) -> Weight { - (57_382_000 as Weight) + (56_056_000 as Weight) // Standard Error: 0 - .saturating_add((70_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((67_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn withdraw_unbonded_kill(s: u32, ) -> Weight { - (92_185_000 as Weight) + (90_267_000 as Weight) // Standard Error: 1_000 - .saturating_add((2_844_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((2_787_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(6 as Weight)) .saturating_add(T::DbWeight::get().writes(8 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn validate() -> Weight { - (16_892_000 as Weight) + (16_345_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn kick(k: u32, ) -> Weight { - (27_411_000 as Weight) + (27_080_000 as Weight) // Standard Error: 14_000 - .saturating_add((19_272_000 as Weight).saturating_mul(k as Weight)) + .saturating_add((18_739_000 as Weight).saturating_mul(k as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(k as Weight))) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(k as Weight))) } fn nominate(n: u32, ) -> Weight { - (30_188_000 as Weight) - // Standard Error: 24_000 - .saturating_add((5_666_000 as Weight).saturating_mul(n as Weight)) + (29_101_000 as Weight) + // Standard Error: 23_000 + .saturating_add((5_670_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(n as Weight))) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn chill() -> Weight { - (15_870_000 as Weight) + (15_771_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn set_payee() -> Weight { - (13_853_000 as Weight) + (13_329_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_controller() -> Weight { - (30_291_000 as Weight) + (29_807_000 as Weight) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn set_validator_count() -> Weight { - (2_397_000 as Weight) + (2_323_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn force_no_eras() -> Weight { - (2_627_000 as Weight) + (2_528_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn force_new_era() -> Weight { - (2_679_000 as Weight) + (2_529_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn force_new_era_always() -> Weight { - (2_643_000 as Weight) + (2_527_000 as Weight) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn set_invulnerables(v: u32, ) -> Weight { - (2_871_000 as Weight) + (2_661_000 as Weight) // Standard Error: 0 .saturating_add((35_000 as Weight).saturating_mul(v as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn force_unstake(s: u32, ) -> Weight { - (65_876_000 as Weight) + (64_650_000 as Weight) // Standard Error: 1_000 - .saturating_add((2_832_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((2_755_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(8 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn cancel_deferred_slash(s: u32, ) -> Weight { - (5_896_640_000 as Weight) - // Standard Error: 391_000 - .saturating_add((34_808_000 as Weight).saturating_mul(s as Weight)) + (5_904_642_000 as Weight) + // Standard Error: 393_000 + .saturating_add((34_810_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn payout_stakers_dead_controller(n: u32, ) -> Weight { - (137_975_000 as Weight) - // Standard Error: 20_000 - .saturating_add((54_061_000 as Weight).saturating_mul(n as Weight)) + (131_368_000 as Weight) + // Standard Error: 17_000 + .saturating_add((52_611_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(10 as Weight)) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) .saturating_add(T::DbWeight::get().writes(2 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } fn payout_stakers_alive_staked(n: u32, ) -> Weight { - (163_885_000 as Weight) - // Standard Error: 20_000 - .saturating_add((68_096_000 as Weight).saturating_mul(n as Weight)) + (165_079_000 as Weight) + // Standard Error: 27_000 + .saturating_add((66_740_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(11 as Weight)) .saturating_add(T::DbWeight::get().reads((5 as Weight).saturating_mul(n as Weight))) .saturating_add(T::DbWeight::get().writes(3 as Weight)) .saturating_add(T::DbWeight::get().writes((3 as Weight).saturating_mul(n as Weight))) } fn rebond(l: u32, ) -> Weight { - (37_847_000 as Weight) - // Standard Error: 1_000 - .saturating_add((89_000 as Weight).saturating_mul(l as Weight)) + (37_039_000 as Weight) + // Standard Error: 2_000 + .saturating_add((93_000 as Weight).saturating_mul(l as Weight)) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } fn set_history_depth(e: u32, ) -> Weight { (0 as Weight) - // Standard Error: 69_000 - .saturating_add((34_413_000 as Weight).saturating_mul(e as Weight)) + // Standard Error: 71_000 + .saturating_add((34_403_000 as Weight).saturating_mul(e as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(4 as Weight)) .saturating_add(T::DbWeight::get().writes((7 as Weight).saturating_mul(e as Weight))) } fn reap_stash(s: u32, ) -> Weight { - (69_257_000 as Weight) - // Standard Error: 1_000 - .saturating_add((2_819_000 as Weight).saturating_mul(s as Weight)) + (67_561_000 as Weight) + // Standard Error: 0 + .saturating_add((2_766_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(4 as Weight)) .saturating_add(T::DbWeight::get().writes(8 as Weight)) .saturating_add(T::DbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn new_era(v: u32, n: u32, ) -> Weight { (0 as Weight) - // Standard Error: 1_013_000 - .saturating_add((382_529_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 50_000 - .saturating_add((63_170_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 1_016_000 + .saturating_add((389_979_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 51_000 + .saturating_add((63_208_000 as Weight).saturating_mul(n as Weight)) .saturating_add(T::DbWeight::get().reads(10 as Weight)) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) @@ -233,12 +233,12 @@ impl WeightInfo for SubstrateWeight { } fn get_npos_voters(v: u32, n: u32, s: u32, ) -> Weight { (0 as Weight) - // Standard Error: 90_000 - .saturating_add((27_108_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 90_000 - .saturating_add((29_962_000 as Weight).saturating_mul(n as Weight)) - // Standard Error: 1_228_000 - .saturating_add((26_080_000 as Weight).saturating_mul(s as Weight)) + // Standard Error: 95_000 + .saturating_add((26_419_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 95_000 + .saturating_add((29_033_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 1_305_000 + .saturating_add((23_680_000 as Weight).saturating_mul(s as Weight)) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) @@ -247,7 +247,7 @@ impl WeightInfo for SubstrateWeight { fn get_npos_targets(v: u32, ) -> Weight { (0 as Weight) // Standard Error: 32_000 - .saturating_add((11_220_000 as Weight).saturating_mul(v as Weight)) + .saturating_add((11_317_000 as Weight).saturating_mul(v as Weight)) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().reads((1 as Weight).saturating_mul(v as Weight))) } @@ -256,155 +256,155 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { fn bond() -> Weight { - (82_121_000 as Weight) + (79_895_000 as Weight) .saturating_add(RocksDbWeight::get().reads(5 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) } fn bond_extra() -> Weight { - (61_899_000 as Weight) + (60_561_000 as Weight) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn unbond() -> Weight { - (56_392_000 as Weight) + (54_996_000 as Weight) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn withdraw_unbonded_update(s: u32, ) -> Weight { - (57_382_000 as Weight) + (56_056_000 as Weight) // Standard Error: 0 - .saturating_add((70_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((67_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn withdraw_unbonded_kill(s: u32, ) -> Weight { - (92_185_000 as Weight) + (90_267_000 as Weight) // Standard Error: 1_000 - .saturating_add((2_844_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((2_787_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(6 as Weight)) .saturating_add(RocksDbWeight::get().writes(8 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn validate() -> Weight { - (16_892_000 as Weight) + (16_345_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn kick(k: u32, ) -> Weight { - (27_411_000 as Weight) + (27_080_000 as Weight) // Standard Error: 14_000 - .saturating_add((19_272_000 as Weight).saturating_mul(k as Weight)) + .saturating_add((18_739_000 as Weight).saturating_mul(k as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(k as Weight))) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(k as Weight))) } fn nominate(n: u32, ) -> Weight { - (30_188_000 as Weight) - // Standard Error: 24_000 - .saturating_add((5_666_000 as Weight).saturating_mul(n as Weight)) + (29_101_000 as Weight) + // Standard Error: 23_000 + .saturating_add((5_670_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(n as Weight))) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn chill() -> Weight { - (15_870_000 as Weight) + (15_771_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn set_payee() -> Weight { - (13_853_000 as Weight) + (13_329_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn set_controller() -> Weight { - (30_291_000 as Weight) + (29_807_000 as Weight) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn set_validator_count() -> Weight { - (2_397_000 as Weight) + (2_323_000 as Weight) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn force_no_eras() -> Weight { - (2_627_000 as Weight) + (2_528_000 as Weight) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn force_new_era() -> Weight { - (2_679_000 as Weight) + (2_529_000 as Weight) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn force_new_era_always() -> Weight { - (2_643_000 as Weight) + (2_527_000 as Weight) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn set_invulnerables(v: u32, ) -> Weight { - (2_871_000 as Weight) + (2_661_000 as Weight) // Standard Error: 0 .saturating_add((35_000 as Weight).saturating_mul(v as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn force_unstake(s: u32, ) -> Weight { - (65_876_000 as Weight) + (64_650_000 as Weight) // Standard Error: 1_000 - .saturating_add((2_832_000 as Weight).saturating_mul(s as Weight)) + .saturating_add((2_755_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(8 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn cancel_deferred_slash(s: u32, ) -> Weight { - (5_896_640_000 as Weight) - // Standard Error: 391_000 - .saturating_add((34_808_000 as Weight).saturating_mul(s as Weight)) + (5_904_642_000 as Weight) + // Standard Error: 393_000 + .saturating_add((34_810_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn payout_stakers_dead_controller(n: u32, ) -> Weight { - (137_975_000 as Weight) - // Standard Error: 20_000 - .saturating_add((54_061_000 as Weight).saturating_mul(n as Weight)) + (131_368_000 as Weight) + // Standard Error: 17_000 + .saturating_add((52_611_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(10 as Weight)) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(n as Weight))) } fn payout_stakers_alive_staked(n: u32, ) -> Weight { - (163_885_000 as Weight) - // Standard Error: 20_000 - .saturating_add((68_096_000 as Weight).saturating_mul(n as Weight)) + (165_079_000 as Weight) + // Standard Error: 27_000 + .saturating_add((66_740_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(11 as Weight)) .saturating_add(RocksDbWeight::get().reads((5 as Weight).saturating_mul(n as Weight))) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) .saturating_add(RocksDbWeight::get().writes((3 as Weight).saturating_mul(n as Weight))) } fn rebond(l: u32, ) -> Weight { - (37_847_000 as Weight) - // Standard Error: 1_000 - .saturating_add((89_000 as Weight).saturating_mul(l as Weight)) + (37_039_000 as Weight) + // Standard Error: 2_000 + .saturating_add((93_000 as Weight).saturating_mul(l as Weight)) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } fn set_history_depth(e: u32, ) -> Weight { (0 as Weight) - // Standard Error: 69_000 - .saturating_add((34_413_000 as Weight).saturating_mul(e as Weight)) + // Standard Error: 71_000 + .saturating_add((34_403_000 as Weight).saturating_mul(e as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(4 as Weight)) .saturating_add(RocksDbWeight::get().writes((7 as Weight).saturating_mul(e as Weight))) } fn reap_stash(s: u32, ) -> Weight { - (69_257_000 as Weight) - // Standard Error: 1_000 - .saturating_add((2_819_000 as Weight).saturating_mul(s as Weight)) + (67_561_000 as Weight) + // Standard Error: 0 + .saturating_add((2_766_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(4 as Weight)) .saturating_add(RocksDbWeight::get().writes(8 as Weight)) .saturating_add(RocksDbWeight::get().writes((1 as Weight).saturating_mul(s as Weight))) } fn new_era(v: u32, n: u32, ) -> Weight { (0 as Weight) - // Standard Error: 1_013_000 - .saturating_add((382_529_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 50_000 - .saturating_add((63_170_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 1_016_000 + .saturating_add((389_979_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 51_000 + .saturating_add((63_208_000 as Weight).saturating_mul(n as Weight)) .saturating_add(RocksDbWeight::get().reads(10 as Weight)) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) @@ -413,12 +413,12 @@ impl WeightInfo for () { } fn get_npos_voters(v: u32, n: u32, s: u32, ) -> Weight { (0 as Weight) - // Standard Error: 90_000 - .saturating_add((27_108_000 as Weight).saturating_mul(v as Weight)) - // Standard Error: 90_000 - .saturating_add((29_962_000 as Weight).saturating_mul(n as Weight)) - // Standard Error: 1_228_000 - .saturating_add((26_080_000 as Weight).saturating_mul(s as Weight)) + // Standard Error: 95_000 + .saturating_add((26_419_000 as Weight).saturating_mul(v as Weight)) + // Standard Error: 95_000 + .saturating_add((29_033_000 as Weight).saturating_mul(n as Weight)) + // Standard Error: 1_305_000 + .saturating_add((23_680_000 as Weight).saturating_mul(s as Weight)) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(v as Weight))) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(n as Weight))) @@ -427,7 +427,7 @@ impl WeightInfo for () { fn get_npos_targets(v: u32, ) -> Weight { (0 as Weight) // Standard Error: 32_000 - .saturating_add((11_220_000 as Weight).saturating_mul(v as Weight)) + .saturating_add((11_317_000 as Weight).saturating_mul(v as Weight)) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().reads((1 as Weight).saturating_mul(v as Weight))) } From e3bb4e75cacaea587e7068a8e4211b764f4eb90d Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Thu, 25 Mar 2021 16:39:22 -0700 Subject: [PATCH 4/9] Address Shawn's Feedback --- frame/staking/src/lib.rs | 6 +----- frame/staking/src/tests.rs | 10 ++++++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index b508e8d0fbcf0..a608b0c150694 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -2001,13 +2001,9 @@ impl Module { let current_era = CurrentEra::get().ok_or( Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) )?; - ensure!( - era <= current_era, - Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) - ); let history_depth = Self::history_depth(); ensure!( - era >= current_era.saturating_sub(history_depth), + era <= current_era && era >= current_era.saturating_sub(history_depth), Error::::InvalidEraToReward.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) ); diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c9c467b9a5982..9d4a2aad068cd 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3360,7 +3360,13 @@ fn payout_stakers_handles_weight_refund() { // calculate weight. ExtBuilder::default().has_stakers(false).build_and_execute(|| { let max_nom_rewarded = ::MaxNominatorRewardedPerValidator::get(); - let half_max_nom_rewarded = max_nom_rewarded.checked_div(2).unwrap(); + // Make sure the configured value is meaningful for our use. + assert!(max_nom_rewarded >= 4); + let half_max_nom_rewarded = max_nom_rewarded / 2; + // Sanity check our max and half max nominator quantities. + assert!(half_max_nom_rewarded > 0); + assert!(max_nom_rewarded > half_max_nom_rewarded); + // We add 1 to account for the payout ops to the validator let max_nom_rewarded_weight = ::WeightInfo::payout_stakers_alive_staked(max_nom_rewarded +1); @@ -3371,7 +3377,7 @@ fn payout_stakers_handles_weight_refund() { let balance = 1000; bond_validator(11, 10, balance); - /* Era 1*/ + /* Era 1 */ start_active_era(1); // Reward just the validator. From 0910c61e30668a0c7ce39245bc6f57828711a0dd Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Thu, 25 Mar 2021 16:53:04 -0700 Subject: [PATCH 5/9] Assert monotomic weights && improve test note --- frame/staking/src/tests.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 9d4a2aad068cd..153b01fdfd1a1 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3356,8 +3356,8 @@ fn payout_stakers_handles_basic_errors() { #[test] fn payout_stakers_handles_weight_refund() { - // N.B. this test relies on the assumption that `payout_stakers_alive_staked` is solely used to - // calculate weight. + // Note: this test relies on the assumption that `payout_stakers_alive_staked` is solely used by + // `payout_stakers` to calculate the weight of each payout op. ExtBuilder::default().has_stakers(false).build_and_execute(|| { let max_nom_rewarded = ::MaxNominatorRewardedPerValidator::get(); // Make sure the configured value is meaningful for our use. @@ -3373,6 +3373,10 @@ fn payout_stakers_handles_weight_refund() { let half_max_nom_rewarded_weight = ::WeightInfo::payout_stakers_alive_staked(half_max_nom_rewarded + 1); let zero_payouts_weight = ::WeightInfo::payout_stakers_alive_staked(0); + assert!(zero_payouts_weight > 0); + assert!(half_max_nom_rewarded_weight > zero_payouts_weight); + assert!(max_nom_rewarded_weight > half_max_nom_rewarded_weight); + let balance = 1000; bond_validator(11, 10, balance); From c86750a53f6154ab7d337841a2e52867d7027b5c Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Thu, 25 Mar 2021 16:57:20 -0700 Subject: [PATCH 6/9] Remove stray new line --- frame/staking/src/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 153b01fdfd1a1..56bc980105c99 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3377,7 +3377,6 @@ fn payout_stakers_handles_weight_refund() { assert!(half_max_nom_rewarded_weight > zero_payouts_weight); assert!(max_nom_rewarded_weight > half_max_nom_rewarded_weight); - let balance = 1000; bond_validator(11, 10, balance); From 33bcc94f8b4c87c0fab2d94b854681986f695811 Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Fri, 26 Mar 2021 10:11:58 -0700 Subject: [PATCH 7/9] debug_assert payout_count <= max --- frame/staking/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 315a2a55d95e6..e080cbcb77b74 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -2075,6 +2075,7 @@ impl Module { } } + debug_assert!(payout_count <= T::MaxNominatorRewardedPerValidator::get() +1); Ok(Some(T::WeightInfo::payout_stakers_alive_staked(payout_count)).into()) } From 035d8e4e4267ef8546b9d68a1b69c0227b641b6e Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sat, 27 Mar 2021 14:17:13 -0700 Subject: [PATCH 8/9] Only track payouts to nominators; not validators --- frame/staking/src/lib.rs | 16 ++++++++-------- frame/staking/src/tests.rs | 17 ++++++++--------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index e080cbcb77b74..d9894eabc355b 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1802,7 +1802,7 @@ decl_module! { /// NOTE: weights are assuming that payouts are made to alive stash account (Staked). /// Paying even a dead controller is cheaper weight-wise. We don't do any refunds here. /// # - #[weight = T::WeightInfo::payout_stakers_alive_staked(T::MaxNominatorRewardedPerValidator::get() + 1)] + #[weight = T::WeightInfo::payout_stakers_alive_staked(T::MaxNominatorRewardedPerValidator::get())] fn payout_stakers(origin, validator_stash: T::AccountId, era: EraIndex) -> DispatchResultWithPostInfo { ensure_signed(origin)?; Self::do_payout_stakers(validator_stash, era) @@ -2046,18 +2046,18 @@ impl Module { ); let validator_staking_payout = validator_exposure_part * validator_leftover_payout; - // Track the number of payouts in order to track the actual weight. - let mut payout_count: u32 = 0; - // We can now make total validator payout: if let Some(imbalance) = Self::make_payout( &ledger.stash, validator_staking_payout + validator_commission_payout ) { - payout_count += 1; Self::deposit_event(RawEvent::Reward(ledger.stash, imbalance.peek())); } + // Track the number of payout ops to nominators. Note: `WeightInfo::payout_stakers_alive_staked` + // always assumes at least a validator is paid out, so we do not need to count their payout op. + let mut nominator_payout_count: u32 = 0; + // Lets now calculate how this is split to the nominators. // Reward only the clipped exposures. Note this is not necessarily sorted. for nominator in exposure.others.iter() { @@ -2070,13 +2070,13 @@ impl Module { // We can now make nominator payout: if let Some(imbalance) = Self::make_payout(&nominator.who, nominator_reward) { // Note: this logic does not count payouts for `RewardDestination::None`. - payout_count += 1; + nominator_payout_count += 1; Self::deposit_event(RawEvent::Reward(nominator.who.clone(), imbalance.peek())); } } - debug_assert!(payout_count <= T::MaxNominatorRewardedPerValidator::get() +1); - Ok(Some(T::WeightInfo::payout_stakers_alive_staked(payout_count)).into()) + debug_assert!(nominator_payout_count <= T::MaxNominatorRewardedPerValidator::get()); + Ok(Some(T::WeightInfo::payout_stakers_alive_staked(nominator_payout_count)).into()) } /// Update the ledger for a controller. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 56bc980105c99..26ab4747de654 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3367,14 +3367,13 @@ fn payout_stakers_handles_weight_refund() { assert!(half_max_nom_rewarded > 0); assert!(max_nom_rewarded > half_max_nom_rewarded); - // We add 1 to account for the payout ops to the validator let max_nom_rewarded_weight - = ::WeightInfo::payout_stakers_alive_staked(max_nom_rewarded +1); + = ::WeightInfo::payout_stakers_alive_staked(max_nom_rewarded); let half_max_nom_rewarded_weight - = ::WeightInfo::payout_stakers_alive_staked(half_max_nom_rewarded + 1); - let zero_payouts_weight = ::WeightInfo::payout_stakers_alive_staked(0); - assert!(zero_payouts_weight > 0); - assert!(half_max_nom_rewarded_weight > zero_payouts_weight); + = ::WeightInfo::payout_stakers_alive_staked(half_max_nom_rewarded); + let zero_nom_payouts_weight = ::WeightInfo::payout_stakers_alive_staked(0); + assert!(zero_nom_payouts_weight > 0); + assert!(half_max_nom_rewarded_weight > zero_nom_payouts_weight); assert!(max_nom_rewarded_weight > half_max_nom_rewarded_weight); let balance = 1000; @@ -3402,7 +3401,7 @@ fn payout_stakers_handles_weight_refund() { assert_ok!(result); assert_eq!( extract_actual_weight(&result, &info), - ::WeightInfo::payout_stakers_alive_staked(1) + zero_nom_payouts_weight ); // The validator is not rewarded in this era; so there will be zero payouts to claim for this era. @@ -3415,7 +3414,7 @@ fn payout_stakers_handles_weight_refund() { let info = call.get_dispatch_info(); let result = call.dispatch(Origin::signed(20)); assert_ok!(result); - assert_eq!(extract_actual_weight(&result, &info), zero_payouts_weight); + assert_eq!(extract_actual_weight(&result, &info), zero_nom_payouts_weight); // Reward the validator and its nominators. Staking::reward_by_ids(vec![(11, 1)]); @@ -3459,7 +3458,7 @@ fn payout_stakers_handles_weight_refund() { let result = call.dispatch(Origin::signed(20)); assert!(result.is_err()); // When there is an error the consumed weight == weight when there are 0 payouts. - assert_eq!(extract_actual_weight(&result, &info), zero_payouts_weight); + assert_eq!(extract_actual_weight(&result, &info), zero_nom_payouts_weight); }); } From 2dc1977c63fa9cfae874dbbf3ff03f99a692e47e Mon Sep 17 00:00:00 2001 From: emostov <32168567+emostov@users.noreply.github.com> Date: Sat, 27 Mar 2021 14:20:20 -0700 Subject: [PATCH 9/9] Trivial comment update --- frame/staking/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 26ab4747de654..0fc53d9d8f0d4 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3457,7 +3457,7 @@ fn payout_stakers_handles_weight_refund() { let info = call.get_dispatch_info(); let result = call.dispatch(Origin::signed(20)); assert!(result.is_err()); - // When there is an error the consumed weight == weight when there are 0 payouts. + // When there is an error the consumed weight == weight when there are 0 nominator payouts. assert_eq!(extract_actual_weight(&result, &info), zero_nom_payouts_weight); }); }