From fe9b7a329271d5a2534350df422fcf631880847e Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 31 Oct 2023 09:33:56 +0700 Subject: [PATCH 1/5] insert primitives --- substrate/primitives/staking/src/lib.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index dfc18987d1525..de594a4b45c79 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -24,7 +24,7 @@ use crate::currency_to_vote::CurrencyToVote; use codec::{FullCodec, MaxEncodedLen}; use scale_info::TypeInfo; use sp_core::RuntimeDebug; -use sp_runtime::{DispatchError, DispatchResult, Saturating}; +use sp_runtime::{DispatchError, DispatchResult, Perbill, Saturating}; use sp_std::{collections::btree_map::BTreeMap, ops::Sub, vec::Vec}; pub mod offence; @@ -54,6 +54,27 @@ impl From for StakingAccount { } } +// A enum contaning payout destination aliases. Used for configuring a payout destination without +// knowing the stash and controller accounts. +#[derive(PartialEq, Copy, Clone)] +pub enum PayoutDestinationAlias { + /// Alias for the controller account. + Controller, + // Alias for a split payout destination either for the stash or controller account. + Split((Perbill, PayoutSplitOpt)), +} + +// Options for aliased payouts. Used to alias stash and controller accounts, which are assumed not +// to be known at the time at usage. +// +// NOTE: This enum can be discontinued after the `PayoutDestination` lazy migration, as the only +// valid split alias would be to the stash account. +#[derive(PartialEq, Copy, Clone)] +pub enum PayoutSplitOpt { + Stash, + Controller, +} + /// Representation of the status of a staker. #[derive(RuntimeDebug, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize, PartialEq, Eq, Clone))] From ffe16727dcd3bf3b79e8690d41a40f7d4e96bcfc Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 31 Oct 2023 11:55:41 +0700 Subject: [PATCH 2/5] mv code across to StakingLedger --- substrate/frame/fast-unstake/src/mock.rs | 6 +- .../nomination-pools/test-staking/src/lib.rs | 11 +- substrate/frame/staking/src/benchmarking.rs | 76 ++++-- substrate/frame/staking/src/ledger.rs | 45 ++- substrate/frame/staking/src/lib.rs | 110 +++++++- substrate/frame/staking/src/pallet/impls.rs | 88 ++++-- substrate/frame/staking/src/pallet/mod.rs | 142 +++++----- substrate/frame/staking/src/tests.rs | 258 +++++++++++++----- substrate/frame/staking/src/weights.rs | 35 +++ 9 files changed, 560 insertions(+), 211 deletions(-) diff --git a/substrate/frame/fast-unstake/src/mock.rs b/substrate/frame/fast-unstake/src/mock.rs index 6b866224ab996..60d02c1e6325d 100644 --- a/substrate/frame/fast-unstake/src/mock.rs +++ b/substrate/frame/fast-unstake/src/mock.rs @@ -25,7 +25,7 @@ use frame_support::{ }; use sp_runtime::{ traits::{Convert, IdentityLookup}, - BuildStorage, + BuildStorage, Perbill, }; use pallet_staking::{Exposure, IndividualExposure, StakerStatus}; @@ -338,7 +338,7 @@ pub(crate) fn next_block(on_idle: bool) { pub fn assert_unstaked(stash: &AccountId) { assert!(!pallet_staking::Bonded::::contains_key(stash)); - assert!(!pallet_staking::Payee::::contains_key(stash)); + assert!(!pallet_staking::Payees::::contains_key(stash)); assert!(!pallet_staking::Validators::::contains_key(stash)); assert!(!pallet_staking::Nominators::::contains_key(stash)); } @@ -352,7 +352,7 @@ pub fn create_exposed_nominator(exposed: AccountId, era: u32) { assert_ok!(Staking::bond( RuntimeOrigin::signed(exposed), 10, - pallet_staking::RewardDestination::Staked + pallet_staking::PayoutDestination::Split((Perbill::from_percent(50), exposed)) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(exposed), vec![exposed])); // register the exposed one. diff --git a/substrate/frame/nomination-pools/test-staking/src/lib.rs b/substrate/frame/nomination-pools/test-staking/src/lib.rs index 9108da510a3a5..c50bec8f404a2 100644 --- a/substrate/frame/nomination-pools/test-staking/src/lib.rs +++ b/substrate/frame/nomination-pools/test-staking/src/lib.rs @@ -25,7 +25,9 @@ use pallet_nomination_pools::{ BondedPools, Error as PoolsError, Event as PoolsEvent, LastPoolId, PoolMember, PoolMembers, PoolState, }; -use pallet_staking::{CurrentEra, Event as StakingEvent, Payee, RewardDestination}; +use pallet_staking::{ + CheckedPayoutDestination, CurrentEra, Event as StakingEvent, Payees, PayoutDestination, +}; use sp_runtime::{bounded_btree_map, traits::Zero}; #[test] @@ -214,8 +216,11 @@ fn pool_slash_e2e() { ] ); - assert_eq!(Payee::::get(POOL1_BONDED), RewardDestination::Account(POOL1_REWARD)); - + assert_eq!( + Payees::::get(POOL1_BONDED), + CheckedPayoutDestination(PayoutDestination::Deposit(POOL1_REWARD)) + ); + // have two members join assert_ok!(Pools::join(RuntimeOrigin::signed(20), 20, 1)); assert_ok!(Pools::join(RuntimeOrigin::signed(21), 20, 1)); diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index f94d9bf4b328a..1651963f65959 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -72,7 +72,7 @@ pub fn create_validator_with_nominators( upper_bound: u32, dead_controller: bool, unique_controller: bool, - destination: RewardDestination, + destination: PayoutRoute, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { // Clean up any existing state. clear_validators_and_nominators::(); @@ -175,7 +175,7 @@ impl ListScenario { let (origin_stash1, origin_controller1) = create_stash_controller_with_balance::( USER_SEED + 2, origin_weight, - Default::default(), + PayoutDestination::Stake, )?; Staking::::nominate( RawOrigin::Signed(origin_controller1.clone()).into(), @@ -186,7 +186,7 @@ impl ListScenario { let (_origin_stash2, origin_controller2) = create_stash_controller_with_balance::( USER_SEED + 3, origin_weight, - Default::default(), + PayoutDestination::Stake, )?; Staking::::nominate( RawOrigin::Signed(origin_controller2).into(), @@ -206,7 +206,7 @@ impl ListScenario { let (_dest_stash1, dest_controller1) = create_stash_controller_with_balance::( USER_SEED + 1, dest_weight, - Default::default(), + PayoutDestination::Stake, )?; Staking::::nominate( RawOrigin::Signed(dest_controller1).into(), @@ -222,10 +222,10 @@ const USER_SEED: u32 = 999666; benchmarks! { bond { let stash = create_funded_user::("stash", USER_SEED, 100); - let reward_destination = RewardDestination::Staked; + let payout_destination = PayoutDestination::Split((Perbill::from_percent(50), stash.clone())); let amount = T::Currency::minimum_balance() * 10u32.into(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), amount, reward_destination) + }: _(RawOrigin::Signed(stash.clone()), amount, payout_destination) verify { assert!(Bonded::::contains_key(stash.clone())); assert!(Ledger::::contains_key(stash)); @@ -290,7 +290,9 @@ benchmarks! { withdraw_unbonded_update { // Slashing Spans let s in 0 .. MAX_SPANS; - let (stash, controller) = create_stash_controller::(0, 100, Default::default())?; + let (stash, controller) = create_stash_controller::( + 0, 100, PayoutRoute::Direct(PayoutDestination::Stake) + )?; add_slashing_spans::(&stash, s); let amount = T::Currency::minimum_balance() * 5u32.into(); // Half of total Staking::::unbond(RawOrigin::Signed(controller.clone()).into(), amount)?; @@ -339,7 +341,7 @@ benchmarks! { let (stash, controller) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, - Default::default(), + PayoutRoute::Direct(PayoutDestination::Stake), )?; // because it is chilled. assert!(!T::VoterList::contains(&stash)); @@ -367,7 +369,7 @@ benchmarks! { let (stash, controller) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, - Default::default(), + PayoutRoute::Direct(PayoutDestination::Stake), )?; let stash_lookup = T::Lookup::unlookup(stash.clone()); @@ -382,7 +384,7 @@ benchmarks! { let (n_stash, n_controller) = create_stash_controller::( MaxNominationsOf::::get() + i, 100, - Default::default(), + PayoutRoute::Direct(PayoutDestination::Stake), )?; // bake the nominations; we first clone them from the rest of the validators. @@ -430,7 +432,7 @@ benchmarks! { let (stash, controller) = create_stash_controller_with_balance::( SEED + MaxNominationsOf::::get() + 1, // make sure the account does not conflict with others origin_weight, - Default::default(), + PayoutDestination::Stake, ).unwrap(); assert!(!Nominators::::contains_key(&stash)); @@ -464,16 +466,38 @@ benchmarks! { } set_payee { - let (stash, controller) = create_stash_controller::(USER_SEED, 100, Default::default())?; - assert_eq!(Payee::::get(&stash), RewardDestination::Staked); + let (stash, controller) = create_stash_controller::( + USER_SEED, 100, PayoutRoute::Direct(PayoutDestination::Stake) + )?; + assert_eq!(Payees::::get(&stash), CheckedPayoutDestination(PayoutDestination::Stake)); + + // Payee should exist to be migrated on `update_payee`. + DeprecatedPayee::::insert(&stash,RewardDestination::Staked); whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), RewardDestination::Controller) + }: _(RawOrigin::Signed(controller.clone()), PayoutDestination::Split((Perbill::from_percent(50), controller.clone()))) verify { - assert_eq!(Payee::::get(&stash), RewardDestination::Controller); + assert!(!DeprecatedPayee::::contains_key(&stash)); + assert_eq!(Payees::::get(&stash), CheckedPayoutDestination(PayoutDestination::Split((Perbill::from_percent(50), controller.clone())))); + } + + update_payee { + let (stash, controller) = create_stash_controller::( + USER_SEED, 100, PayoutRoute::Direct(PayoutDestination::Stake) + )?; + // DeprecatedPayee should exist to be migrated on `update_payee`. + DeprecatedPayee::::insert(&stash,RewardDestination::Staked); + Payees::::remove(&stash); + assert!(!Payees::::contains_key(&stash)); + whitelist_account!(controller); + }: _(RawOrigin::Signed(stash.clone()), controller) + verify { + assert_eq!(Payees::::get(&stash), CheckedPayoutDestination(PayoutDestination::Stake)); } set_controller { - let (stash, ctlr) = create_unique_stash_controller::(9000, 100, Default::default(), false)?; + let (stash, ctlr) = create_unique_stash_controller::( + 9000, 100, PayoutRoute::Direct(PayoutDestination::Stake), false + )?; // ensure `ctlr` is the currently stored controller. assert!(!Ledger::::contains_key(&stash)); assert!(Ledger::::contains_key(&ctlr)); @@ -558,15 +582,17 @@ benchmarks! { T::MaxNominatorRewardedPerValidator::get() as u32, true, true, - RewardDestination::Controller, + PayoutRoute::Alias( + PayoutDestinationAlias::Split((Perbill::from_percent(50), PayoutSplitOpt::Controller)) + ), )?; + let validator_controller = >::get(&validator).unwrap(); let current_era = CurrentEra::::get().unwrap(); // set the commission for this particular era as well. >::insert(current_era, validator.clone(), >::validators(&validator)); let caller = whitelisted_caller(); - let validator_controller = >::get(&validator).unwrap(); let balance_before = T::Currency::free_balance(&validator_controller); for (_, controller) in &nominators { let balance = T::Currency::free_balance(controller); @@ -592,7 +618,9 @@ benchmarks! { T::MaxNominatorRewardedPerValidator::get() as u32, false, true, - RewardDestination::Staked, + PayoutRoute::Alias( + PayoutDestinationAlias::Split((Perbill::from_percent(50), PayoutSplitOpt::Stash)) + ), )?; let current_era = CurrentEra::::get().unwrap(); @@ -773,7 +801,9 @@ benchmarks! { #[extra] do_slash { let l in 1 .. T::MaxUnlockingChunks::get() as u32; - let (stash, controller) = create_stash_controller::(0, 100, Default::default())?; + let (stash, controller) = create_stash_controller::( + 0, 100, PayoutRoute::Direct(PayoutDestination::Stake) + )?; let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); let unlock_chunk = UnlockChunk::> { value: 1u32.into(), @@ -907,7 +937,7 @@ benchmarks! { // Create a validator with a commission of 50% let (stash, controller) = - create_stash_controller::(1, 1, RewardDestination::Staked)?; + create_stash_controller::(1, 1, PayoutRoute::Direct(PayoutDestination::Stake))?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(controller).into(), validator_prefs)?; @@ -987,7 +1017,7 @@ mod tests { <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, false, - RewardDestination::Staked, + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); @@ -1017,7 +1047,7 @@ mod tests { <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, false, - RewardDestination::Staked, + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); diff --git a/substrate/frame/staking/src/ledger.rs b/substrate/frame/staking/src/ledger.rs index cf9b4635bf55a..ab48d3cfe53b6 100644 --- a/substrate/frame/staking/src/ledger.rs +++ b/substrate/frame/staking/src/ledger.rs @@ -40,7 +40,8 @@ use sp_staking::{EraIndex, StakingAccount}; use sp_std::prelude::*; use crate::{ - BalanceOf, Bonded, Config, Error, Ledger, Payee, RewardDestination, StakingLedger, STAKING_ID, + BalanceOf, Bonded, CheckedPayoutDestination, Config, DeprecatedPayee, Error, Ledger, Payees, + StakingLedger, STAKING_ID, }; #[cfg(any(feature = "runtime-benchmarks", test))] @@ -125,13 +126,13 @@ impl StakingLedger { .ok_or(Error::::NotController) } - /// Returns the reward destination of a staking ledger, stored in [`Payee`]. + /// Returns the payout destination of a staking ledger, stored in [`Payees`]. /// - /// Note: if the stash is not bonded and/or does not have an entry in [`Payee`], it returns the - /// default reward destination. + /// Note: if the stash is not bonded and/or does not have an entry in [`Payees`], it returns the + /// default payout destination. pub(crate) fn reward_destination( account: StakingAccount, - ) -> RewardDestination { + ) -> CheckedPayoutDestination { let stash = match account { StakingAccount::Stash(stash) => Some(stash), StakingAccount::Controller(controller) => @@ -139,10 +140,10 @@ impl StakingLedger { }; if let Some(stash) = stash { - >::get(stash) + >::get(stash) } else { - defensive!("fetched reward destination from unbonded stash {}", stash); - RewardDestination::default() + defensive!("fetched payout destination from unbonded stash {}", stash); + CheckedPayoutDestination::default() } } @@ -186,22 +187,33 @@ impl StakingLedger { /// Bonds a ledger. /// /// It sets the reward preferences for the bonded stash. - pub(crate) fn bond(self, payee: RewardDestination) -> Result<(), Error> { + pub(crate) fn bond( + self, + payee: CheckedPayoutDestination, + ) -> Result<(), Error> { if >::contains_key(&self.stash) { Err(Error::::AlreadyBonded) } else { - >::insert(&self.stash, payee); + >::insert(&self.stash, payee); >::insert(&self.stash, &self.stash); self.update() } } /// Sets the ledger Payee. - pub(crate) fn set_payee(self, payee: RewardDestination) -> Result<(), Error> { + pub(crate) fn set_payee( + self, + payee: CheckedPayoutDestination, + ) -> Result<(), Error> { if !>::contains_key(&self.stash) { Err(Error::::NotStash) } else { - >::insert(&self.stash, payee); + >::insert(&self.stash, payee); + // In-progress lazy migration to `Payees` storage item. + // NOTE: To be removed in next runtime upgrade once migration is completed. + if DeprecatedPayee::::contains_key(&self.stash) { + DeprecatedPayee::::remove(self.stash); + } Ok(()) } } @@ -214,9 +226,14 @@ impl StakingLedger { >::get(&controller).ok_or(Error::::NotController).map(|ledger| { T::Currency::remove_lock(STAKING_ID, &ledger.stash); Ledger::::remove(controller); - >::remove(&stash); - >::remove(&stash); + // NOTE: Checks both `Payees` and `Payee` records during migration period. + // Tracking issue: + if >::contains_key(&stash) { + >::remove(stash); + } else { + >::remove(stash); + } Ok(()) })? diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 227326763a9bd..df7e2da94822e 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -316,7 +316,7 @@ use sp_runtime::{ pub use sp_staking::StakerStatus; use sp_staking::{ offence::{Offence, OffenceError, ReportOffence}, - EraIndex, OnStakingUpdate, SessionIndex, StakingAccount, + EraIndex, OnStakingUpdate, PayoutDestinationAlias, SessionIndex, StakingAccount, }; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; pub use weights::WeightInfo; @@ -389,7 +389,113 @@ impl Default for EraRewardPoints { } } +/// The payout destination for an account. +/// +/// NOTE: Being lazily migrated to. Logic pertaining to this enum has been introduced to `set_payee` +/// and payout logic, replacing `RewardDestination`. +#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] +pub enum PayoutDestination { + /// Payout goes into the stash account and is added to bond. + Stake, + /// Deposit the specified percentage of payout to the specified account as free balance, and + /// pay the rest into the stash account and add to bond. 0% and 100% should be disallowed and + /// handled as `Stake` and `Deposit` respectively. + Split((Perbill, AccountId)), + /// Deposit payout as free balance to an account. + Deposit(AccountId), + /// Receive no payout. + Forgo, +} + +/// A payout destination that has been checked via `PayoutDestination::to_checked`. Ensures that 0% +/// and 100% splits do not make it into storage. +#[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] +pub struct CheckedPayoutDestination(pub PayoutDestination); + +impl Default for CheckedPayoutDestination { + fn default() -> Self { + Self(PayoutDestination::Stake) + } +} + +// Used for testing and benchmarking where stash and controller accounts are sometimes generated +// after providing the desired `PayoutDestination`. In such scenarios the provided `Alias` variant +// is used. If payout destination accounts are already known, `Direct` can be used. +#[derive(PartialEq, Copy, Clone)] +pub enum PayoutRoute { + Direct(PayoutDestination), + Alias(PayoutDestinationAlias), +} + +impl PayoutDestination { + /// NOTE: This function can be removed once lazy migration to `PayoutDestination` is completed. + pub fn from_reward_destination( + v: RewardDestination, + stash: AccountId, + controller: AccountId, + ) -> CheckedPayoutDestination { + let dest = match v { + RewardDestination::Staked => Self::Stake, + RewardDestination::Stash => Self::Deposit(stash), + RewardDestination::Controller => Self::Deposit(controller), + RewardDestination::Account(a) => Self::Deposit(a), + RewardDestination::None => Self::Forgo, + }; + CheckedPayoutDestination(dest) + } + + /// Formats a `CheckedPayoutDestination` from a `PayoutDestination` provided in a call, which + /// could include a 0% and 100% split variant. + /// + /// Falls back to `Stake` or `Deposit` variants if a 0% or 100% perbill is provided in a `Split` + /// variant for an account respectively. + pub fn to_checked(v: PayoutDestination) -> CheckedPayoutDestination { + let dest = match v { + PayoutDestination::Split((share, deposit_to)) => { + if share == Perbill::from_percent(100) { + PayoutDestination::Deposit(deposit_to) + } else if share == Perbill::zero() { + PayoutDestination::Stake + } else { + PayoutDestination::Split((share, deposit_to)) + } + }, + PayoutDestination::Stake | PayoutDestination::Deposit(_) | PayoutDestination::Forgo => + v, + }; + CheckedPayoutDestination(dest) + } +} + +#[cfg(any(test, feature = "runtime-benchmarks"))] +impl PayoutDestination { + // NOTE: the `ctlr` parameter can be discontinued after the lazy migration to + // `PayoutDestination` is completed. + pub fn from_route(v: PayoutRoute, stash: &AccountId, ctlr: &AccountId) -> Self { + match v { + PayoutRoute::Direct(destination) => destination, + PayoutRoute::Alias(alias) => match alias { + PayoutDestinationAlias::Controller => PayoutDestination::Deposit(ctlr.clone()), + PayoutDestinationAlias::Split((percent, dest)) => + if dest == PayoutSplitOpt::Stash { + PayoutDestination::Split((percent, stash.clone())) + } else { + PayoutDestination::Split((percent, ctlr.clone())) + }, + }, + } + } +} + +impl Default for PayoutDestination { + fn default() -> Self { + PayoutDestination::Stake + } +} + /// A destination account for payment. +/// NOTE: Being lazily migrated and deprecated in favour of `PayoutDestination`. +/// Tracking at #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum RewardDestination { /// Pay into the stash account, increasing the amount at stake accordingly. @@ -404,7 +510,7 @@ pub enum RewardDestination { None, } -impl Default for RewardDestination { +impl Default for RewardDestination { fn default() -> Self { RewardDestination::Staked } diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index ad2de1d59315a..2cb6bd87dd4dd 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -49,9 +49,9 @@ use sp_std::prelude::*; use crate::{ election_size_tracker::StaticTracker, log, slashing, weights::WeightInfo, ActiveEraInfo, - BalanceOf, EraPayout, Exposure, ExposureOf, Forcing, IndividualExposure, MaxNominationsOf, - MaxWinnersOf, Nominations, NominationsQuota, PositiveImbalanceOf, RewardDestination, - SessionInterface, StakingLedger, ValidatorPrefs, + BalanceOf, CheckedPayoutDestination, EraPayout, Exposure, ExposureOf, Forcing, + IndividualExposure, MaxNominationsOf, MaxWinnersOf, Nominations, NominationsQuota, + PayoutDestination, PositiveImbalanceOf, SessionInterface, StakingLedger, ValidatorPrefs, }; use super::pallet::*; @@ -75,7 +75,7 @@ impl Pallet { StakingLedger::::get(account) } - pub fn payee(account: StakingAccount) -> RewardDestination { + pub fn payee(account: StakingAccount) -> CheckedPayoutDestination { StakingLedger::::reward_destination(account) } @@ -260,9 +260,12 @@ impl Pallet { total_imbalance.subsume(imbalance); } - // Track the number of payout ops to nominators. Note: - // `WeightInfo::payout_stakers_alive_staked` always assumes at least a validator is paid + // Track the number of payout ops to nominators. + // + // Notes: + // - `WeightInfo::payout_stakers_alive_staked` always assumes at least a validator is paid // out, so we do not need to count their payout op. + // - this logic does not count payouts for `PayoutDestination::Forgo`. let mut nominator_payout_count: u32 = 0; // Lets now calculate how this is split to the nominators. @@ -305,29 +308,52 @@ impl Pallet { fn make_payout( stash: &T::AccountId, amount: BalanceOf, - ) -> Option<(PositiveImbalanceOf, RewardDestination)> { - let dest = Self::payee(StakingAccount::Stash(stash.clone())); - let maybe_imbalance = match dest { - RewardDestination::Controller => Self::bonded(stash) - .map(|controller| T::Currency::deposit_creating(&controller, amount)), - RewardDestination::Stash => T::Currency::deposit_into_existing(stash, amount).ok(), - RewardDestination::Staked => Self::ledger(Stash(stash.clone())) + ) -> Option<(PositiveImbalanceOf, CheckedPayoutDestination)> { + // NOTE: temporary getter while `Payee` -> `Payees` lazy migration is taking place. + // Tracking issue: + // Can replace with `dest = Self:payees(stash);` once migration is done. + // + // let dest = Self::payee(StakingAccount::Stash(stash.clone())); + + let dest = Self::bonded(stash) + .and_then(|c| Some(Self::get_payout_destination_migrate(stash, c)))?; + + // Closure to handle the `Stake` payout destination, used in `Stake` and `Split` variants. + let payout_destination_stake = |a: BalanceOf| -> Option> { + Self::ledger(Stash(stash.clone())) .and_then(|mut ledger| { ledger.active += amount; ledger.total += amount; - let r = T::Currency::deposit_into_existing(stash, amount).ok(); - let _ = ledger .update() .defensive_proof("ledger fetched from storage, so it exists; qed."); + let r = T::Currency::deposit_into_existing(stash, amount).ok(); Ok(r) }) - .unwrap_or_default(), - RewardDestination::Account(dest_account) => - Some(T::Currency::deposit_creating(&dest_account, amount)), - RewardDestination::None => None, + .unwrap_or_default() }; + + let maybe_imbalance = match dest { + PayoutDestination::Stake => payout_destination_stake(amount), + PayoutDestination::Split((share, deposit_to)) => { + // `share` can never be 0% or 100%. + let amount_free = share * amount; + let amount_stake = amount.saturating_sub(amount_free); + let mut total_imbalance = PositiveImbalanceOf::::zero(); + + total_imbalance.subsume( + payout_destination_stake(amount_stake) + .unwrap_or(PositiveImbalanceOf::::zero()), + ); + total_imbalance.subsume(T::Currency::deposit_creating(&deposit_to, amount_free)); + Some(total_imbalance) + }, + PayoutDestination::Deposit(deposit_to) => + Some(T::Currency::deposit_creating(&deposit_to, amount)), + PayoutDestination::Forgo => None, + }; + maybe_imbalance .map(|imbalance| (imbalance, Self::payee(StakingAccount::Stash(stash.clone())))) } @@ -1036,6 +1062,28 @@ impl Pallet { DispatchClass::Mandatory, ); } + + /// Temporary getter for `Payees`. + /// + /// Migrates `Payee` to `Payees` if it has not been migrated already. + /// TODO: move to `StakingLedger`. + pub fn get_payout_destination_migrate( + stash: &T::AccountId, + controller: T::AccountId, + ) -> PayoutDestination { + if !Payees::::contains_key(stash) { + let current = PayoutDestination::from_reward_destination( + DeprecatedPayee::::get(stash), + stash.clone(), + controller, + ); + Payees::::insert(stash, current.clone()); + DeprecatedPayee::::remove(stash); + current.0 + } else { + Payees::::get(stash).0 + } + } } impl Pallet { @@ -1697,7 +1745,7 @@ impl StakingInterface for Pallet { Self::bond( RawOrigin::Signed(who.clone()).into(), value, - RewardDestination::Account(payee.clone()), + PayoutDestination::Deposit(payee.clone()), ) } diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index f084299be8e13..64eb2cf2258ad 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -46,10 +46,11 @@ mod impls; pub use impls::*; use crate::{ - slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout, - EraRewardPoints, Exposure, Forcing, MaxNominationsOf, NegativeImbalanceOf, Nominations, - NominationsQuota, PositiveImbalanceOf, RewardDestination, SessionInterface, StakingLedger, - UnappliedSlash, UnlockChunk, ValidatorPrefs, + slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, + CheckedPayoutDestination, EraPayout, EraRewardPoints, Exposure, Forcing, MaxNominationsOf, + NegativeImbalanceOf, Nominations, NominationsQuota, PayoutDestination, PositiveImbalanceOf, + RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk, + ValidatorPrefs, }; // The speculative number of spans are used as an input of the weight annotation of @@ -325,11 +326,27 @@ pub mod pallet { /// Where the reward payment should be made. Keyed by stash. /// + /// NOTE: Being lazily migrated and deprecated in favour of `Payees`. + /// Tracking at /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. #[pallet::storage] - pub type Payee = + #[pallet::storage_prefix = "Payee"] + pub type DeprecatedPayee = StorageMap<_, Twox64Concat, T::AccountId, RewardDestination, ValueQuery>; + /// Where the reward payment should be made. Keyed by stash. + /// + /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. + #[pallet::storage] + #[pallet::getter(fn payees)] + pub type Payees = CountedStorageMap< + _, + Twox64Concat, + T::AccountId, + CheckedPayoutDestination, + ValueQuery, + >; + /// The map from (wannabe) validator stash key to the preferences of that validator. /// /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. @@ -631,7 +648,7 @@ pub mod pallet { frame_support::assert_ok!(>::bond( T::RuntimeOrigin::from(Some(stash.clone()).into()), balance, - RewardDestination::Staked, + PayoutDestination::Stake, )); frame_support::assert_ok!(match status { crate::StakerStatus::Validator => >::validate( @@ -668,7 +685,7 @@ pub mod pallet { /// The nominator has been rewarded by this amount to this destination. Rewarded { stash: T::AccountId, - dest: RewardDestination, + dest: CheckedPayoutDestination, amount: BalanceOf, }, /// A staker (validator or nominator) has been slashed by the given amount. @@ -827,11 +844,6 @@ pub mod pallet { /// The dispatch origin for this call must be _Signed_ by the stash account. /// /// Emits `Bonded`. - /// ## Complexity - /// - Independent of the arguments. Moderate complexity. - /// - O(1). - /// - Three extra DB entries. - /// /// NOTE: Two of the storage writes (`Self::bonded`, `Self::payee`) are _never_ cleaned /// unless the `origin` falls below _existential deposit_ and gets removed as dust. #[pallet::call_index(0)] @@ -839,7 +851,7 @@ pub mod pallet { pub fn bond( origin: OriginFor, #[pallet::compact] value: BalanceOf, - payee: RewardDestination, + payee: PayoutDestination, ) -> DispatchResult { let stash = ensure_signed(origin)?; @@ -854,6 +866,10 @@ pub mod pallet { frame_system::Pallet::::inc_consumers(&stash).map_err(|_| Error::::BadState)?; + // Ensure a 100% or 0% `Split` variant is updated to a `Deposit` or `Stake` variant + // respectively. + let checked_payee = PayoutDestination::to_checked(payee); + let current_era = CurrentEra::::get().unwrap_or(0); let history_depth = T::HistoryDepth::get(); let last_reward_era = current_era.saturating_sub(history_depth); @@ -874,7 +890,7 @@ pub mod pallet { // You're auto-bonded forever, here. We might improve this by only bonding when // you actually validate/nominate and remove once you unbond __everything__. - ledger.bond(payee)?; + ledger.bond(checked_payee)?; Ok(()) } @@ -889,10 +905,6 @@ pub mod pallet { /// any limitation on the amount that can be added. /// /// Emits `Bonded`. - /// - /// ## Complexity - /// - Independent of the arguments. Insignificant complexity. - /// - O(1). #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::bond_extra())] pub fn bond_extra( @@ -1053,10 +1065,6 @@ pub mod pallet { /// slashing spans associated with the stash account in the [`SlashingSpans`] storage type, /// otherwise the call will fail. The call weight is directly propotional to /// `num_slashing_spans`. - /// - /// ## Complexity - /// O(S) where S is the number of slashing spans to remove - /// NOTE: Weight annotation is the kill scenario, we refund otherwise. #[pallet::call_index(3)] #[pallet::weight(T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans))] pub fn withdraw_unbonded( @@ -1112,11 +1120,6 @@ pub mod pallet { /// Effects will be felt at the beginning of the next era. /// /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. - /// - /// ## Complexity - /// - The transaction's complexity is proportional to the size of `targets` (N) - /// which is capped at CompactAssignments::LIMIT (T::MaxNominations). - /// - Both the reads and writes follow a similar pattern. #[pallet::call_index(5)] #[pallet::weight(T::WeightInfo::nominate(targets.len() as u32))] pub fn nominate( @@ -1184,11 +1187,6 @@ pub mod pallet { /// Effects will be felt at the beginning of the next era. /// /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. - /// - /// ## Complexity - /// - Independent of the arguments. Insignificant complexity. - /// - Contains one read. - /// - Writes are limited to the `origin` account key. #[pallet::call_index(6)] #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor) -> DispatchResult { @@ -1205,23 +1203,20 @@ pub mod pallet { /// Effects will be felt instantly (as soon as this function is completed successfully). /// /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. - /// - /// ## Complexity - /// - O(1) - /// - Independent of the arguments. Insignificant complexity. - /// - Contains a limited number of reads. - /// - Writes are limited to the `origin` account key. - /// --------- #[pallet::call_index(7)] #[pallet::weight(T::WeightInfo::set_payee())] pub fn set_payee( origin: OriginFor, - payee: RewardDestination, + payee: PayoutDestination, ) -> DispatchResult { + // Ensure a 100% or 0% `Split` variant is updated to a `Deposit` or `Stake` variant + // respectively. + let checked_payee = PayoutDestination::to_checked(payee); + let controller = ensure_signed(origin)?; let ledger = Self::ledger(Controller(controller))?; let _ = ledger - .set_payee(payee) + .set_payee(checked_payee) .defensive_proof("ledger was retrieved from storage, thus its bonded; qed."); Ok(()) @@ -1235,12 +1230,6 @@ pub mod pallet { /// Effects will be felt instantly (as soon as this function is completed successfully). /// /// The dispatch origin for this call must be _Signed_ by the stash, not the controller. - /// - /// ## Complexity - /// O(1) - /// - Independent of the arguments. Insignificant complexity. - /// - Contains a limited number of reads. - /// - Writes are limited to the `origin` account key. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::set_controller())] pub fn set_controller(origin: OriginFor) -> DispatchResult { @@ -1268,9 +1257,6 @@ pub mod pallet { /// Sets the ideal number of validators. /// /// The dispatch origin must be Root. - /// - /// ## Complexity - /// O(1) #[pallet::call_index(9)] #[pallet::weight(T::WeightInfo::set_validator_count())] pub fn set_validator_count( @@ -1292,9 +1278,6 @@ pub mod pallet { /// `ElectionProviderBase::MaxWinners`. /// /// The dispatch origin must be Root. - /// - /// ## Complexity - /// Same as [`Self::set_validator_count`]. #[pallet::call_index(10)] #[pallet::weight(T::WeightInfo::set_validator_count())] pub fn increase_validator_count( @@ -1317,9 +1300,6 @@ pub mod pallet { /// `ElectionProviderBase::MaxWinners`. /// /// The dispatch origin must be Root. - /// - /// ## Complexity - /// Same as [`Self::set_validator_count`]. #[pallet::call_index(11)] #[pallet::weight(T::WeightInfo::set_validator_count())] pub fn scale_validator_count(origin: OriginFor, factor: Percent) -> DispatchResult { @@ -1345,10 +1325,6 @@ pub mod pallet { /// The election process starts multiple blocks before the end of the era. /// Thus the election process may be ongoing when this is called. In this case the /// election will continue until the next era is triggered. - /// - /// ## Complexity - /// - No arguments. - /// - Weight: O(1) #[pallet::call_index(12)] #[pallet::weight(T::WeightInfo::force_no_eras())] pub fn force_no_eras(origin: OriginFor) -> DispatchResult { @@ -1367,10 +1343,6 @@ pub mod pallet { /// The election process starts multiple blocks before the end of the era. /// If this is called just before a new era is triggered, the election process may not /// have enough blocks to get a result. - /// - /// ## Complexity - /// - No arguments. - /// - Weight: O(1) #[pallet::call_index(13)] #[pallet::weight(T::WeightInfo::force_new_era())] pub fn force_new_era(origin: OriginFor) -> DispatchResult { @@ -1471,9 +1443,6 @@ pub mod pallet { /// /// The origin of this call must be _Signed_. Any account can call this function, even if /// it is not one of the stakers. - /// - /// ## Complexity - /// - At most O(MaxNominatorRewardedPerValidator). #[pallet::call_index(18)] #[pallet::weight(T::WeightInfo::payout_stakers_alive_staked( T::MaxNominatorRewardedPerValidator::get() @@ -1490,10 +1459,6 @@ pub mod pallet { /// Rebond a portion of the stash scheduled to be unlocked. /// /// The dispatch origin must be signed by the controller. - /// - /// ## Complexity - /// - Time complexity: O(L), where L is unlocking chunks - /// - Bounded by `MaxUnlockingChunks`. #[pallet::call_index(19)] #[pallet::weight(T::WeightInfo::rebond(T::MaxUnlockingChunks::get() as u32))] pub fn rebond( @@ -1536,7 +1501,7 @@ pub mod pallet { /// 2. or, the `ledger.total` of the stash is below existential deposit. /// /// The former can happen in cases like a slash; the latter when a fully unbonded account - /// is still receiving staking rewards in `RewardDestination::Staked`. + /// is still receiving staking rewards in `PayoutDestination::Stake`. /// /// It can be called by anyone, as long as `stash` meets the above requirements. /// @@ -1779,6 +1744,39 @@ pub mod pallet { MinCommission::::put(new); Ok(()) } + + /// Migrates an account's `RewardDestination` in `Payee` to `PayoutDestination` in `Payees` + /// if a record exists and if it has not already been migrated. + /// + /// Effects will be felt instantly (as soon as this function is completed successfully). + /// + /// This will waive the transaction fee if the payee is successfully migrated. + #[pallet::call_index(26)] + #[pallet::weight(0)] + pub fn update_payee(origin: OriginFor) -> DispatchResultWithPostInfo { + let controller = ensure_signed(origin)?; + let ledger = Self::ledger(Controller(controller.clone()))?; + + if Payees::::contains_key(&ledger.stash) && + !DeprecatedPayee::::contains_key(&ledger.stash) + { + return Ok(Pays::Yes.into()) + } + + Payees::::insert( + ledger.stash.clone(), + PayoutDestination::from_reward_destination( + DeprecatedPayee::::get(&ledger.stash), + ledger.stash.clone(), + controller, + ), + ); + DeprecatedPayee::::remove(&ledger.stash); + // TODO: is this update really needed? + let _ = ledger.update(); + + Ok(Pays::No.into()) + } } } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index cb620f89f12c0..b9dd96a993682 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -25,7 +25,7 @@ use frame_election_provider_support::{ }; use frame_support::{ assert_noop, assert_ok, assert_storage_noop, - dispatch::{extract_actual_weight, GetDispatchInfo, WithPostDispatchInfo}, + dispatch::{extract_actual_weight, GetDispatchInfo, PostDispatchInfo, WithPostDispatchInfo}, pallet_prelude::*, traits::{Currency, Get, ReservableCurrency}, }; @@ -42,6 +42,7 @@ use sp_staking::{ }; use sp_std::prelude::*; use substrate_test_utils::assert_eq_uvec; +use testing_utils::{create_stash_controller, create_unique_stash_controller}; #[test] fn set_staking_configs_works() { @@ -238,7 +239,7 @@ fn change_controller_works() { let (stash, controller) = testing_utils::create_unique_stash_controller::( 0, 100, - RewardDestination::Staked, + PayoutRoute::Direct(PayoutDestination::Stake), false, ) .unwrap(); @@ -297,9 +298,9 @@ fn rewards_should_work() { let init_balance_101 = Balances::total_balance(&101); // Set payees - Payee::::insert(11, RewardDestination::Controller); - Payee::::insert(21, RewardDestination::Controller); - Payee::::insert(101, RewardDestination::Controller); + Payees::::insert(11, CheckedPayoutDestination(PayoutDestination::Deposit(11))); + Payees::::insert(21, CheckedPayoutDestination(PayoutDestination::Deposit(21))); + Payees::::insert(101, CheckedPayoutDestination(PayoutDestination::Deposit(101))); Pallet::::reward_by_ids(vec![(11, 50)]); Pallet::::reward_by_ids(vec![(11, 50)]); @@ -416,7 +417,7 @@ fn staking_should_work() { // --- Block 2: start_session(2); // add a new candidate for being a validator. account 3 controlled by 4. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, PayoutDestination::Deposit(3))); assert_ok!(Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(3), @@ -586,19 +587,19 @@ fn nominating_and_rewards_should_work() { // Set payee to controller. assert_ok!(Staking::set_payee( RuntimeOrigin::signed(11), - RewardDestination::Controller + PayoutDestination::Deposit(11) )); assert_ok!(Staking::set_payee( RuntimeOrigin::signed(21), - RewardDestination::Controller + PayoutDestination::Deposit(21) )); assert_ok!(Staking::set_payee( RuntimeOrigin::signed(31), - RewardDestination::Controller + PayoutDestination::Deposit(31) )); assert_ok!(Staking::set_payee( RuntimeOrigin::signed(41), - RewardDestination::Controller + PayoutDestination::Deposit(41) )); // give the man some money @@ -611,14 +612,14 @@ fn nominating_and_rewards_should_work() { assert_ok!(Staking::bond( RuntimeOrigin::signed(1), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(1) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 21, 31])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(3) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![11, 21, 41])); @@ -708,6 +709,108 @@ fn nominating_and_rewards_should_work() { }); } +#[test] +fn set_payee_also_updates_payee_destination() { + ExtBuilder::default().build_and_execute(|| { + // Given + let (stash, _) = + create_stash_controller::(12, 13, PayoutRoute::Direct(PayoutDestination::Stake)) + .unwrap(); + Payees::::remove(stash); + // Value to be migrated + DeprecatedPayee::::insert(stash, PayoutDestination::Deposit(stash)); + + // When + assert_ok!(Staking::set_payee( + RuntimeOrigin::signed(stash), + PayoutDestination::Deposit(11) + )); + + // Then + assert!(!DeprecatedPayee::::contains_key(stash)); + assert_eq!( + Payees::::get(stash), + CheckedPayoutDestination(PayoutDestination::Deposit(11)) + ); + }); +} + +#[test] +fn update_payee_works() { + ExtBuilder::default().build_and_execute(|| { + // `update_payee` is used to lazily migrate `Payee` records into `Payees` records. + + // fails when invalid controller is given. + assert_noop!( + Staking::update_payee(RuntimeOrigin::signed(1), 1), + Error::::NotController, + ); + + // Given + let (stash, controller) = + create_stash_controller::(12, 13, PayoutRoute::Direct(PayoutDestination::Stake)) + .unwrap(); + Payees::::remove(stash); + // Value to be migrated + DeprecatedPayee::::insert(stash, PayoutDestination::Stake); + + // When + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(controller), PayoutDestination::Stake)); + assert!(!DeprecatedPayee::::contains_key(stash)); + assert!(Payees::::contains_key(stash)); + + // Then + assert_ok!(Staking::update_payee(RuntimeOrigin::signed(stash), controller)); + }); +} + +#[test] +fn update_payee_charges_on_invalid_migration() { + ExtBuilder::default().build_and_execute(|| { + // `update_payee` is used to lazily migrate `Payee` records into `Payees` records. + + // Given + let (stash, controller) = + create_stash_controller::(12, 13, PayoutRoute::Direct(PayoutDestination::Stake)) + .unwrap(); + Payees::::remove(stash); + + // When + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(controller), PayoutDestination::Stake)); + assert!(!DeprecatedPayee::::contains_key(stash)); + assert!(Payees::::contains_key(stash)); + + // Then + assert_eq!( + Staking::update_payee(RuntimeOrigin::signed(stash), controller), + Ok(PostDispatchInfo { actual_weight: None, pays_fee: Pays::Yes }) + ); + }); +} + +#[test] +fn get_destination_payout_migrates_payee() { + ExtBuilder::default().build_and_execute(|| { + // `get_destination_payout` is used as a getter of `Payees`, and also migrates `Payee` + // records if they have not already been. + + // Given + let (stash, controller) = + create_stash_controller::(12, 13, PayoutRoute::Direct(PayoutDestination::Stake)) + .unwrap(); + Payees::::remove(stash); + DeprecatedPayee::::insert(stash, PayoutDestination::Stake); + + // When + let dest = Staking::get_payout_destination_migrate(&stash, controller); + + // Then + assert_eq!(dest, PayoutDestination::Stake,); + assert!(!DeprecatedPayee::::contains_key(stash)); + assert!(Payees::::contains_key(stash)); + }); +} + #[test] fn nominators_also_get_slashed_pro_rata() { ExtBuilder::default().build_and_execute(|| { @@ -773,7 +876,7 @@ fn double_staking_should_fail() { let (stash, controller) = testing_utils::create_unique_stash_controller::( 0, arbitrary_value, - RewardDestination::default(), + PayoutRoute::Direct(PayoutDestination::Stake), false, ) .unwrap(); @@ -783,7 +886,7 @@ fn double_staking_should_fail() { Staking::bond( RuntimeOrigin::signed(stash), arbitrary_value.into(), - RewardDestination::default() + PayoutDestination::default() ), Error::::AlreadyBonded, ); @@ -807,7 +910,7 @@ fn double_controlling_attempt_should_fail() { let (stash, _) = testing_utils::create_unique_stash_controller::( 0, arbitrary_value, - RewardDestination::default(), + PayoutRoute::Direct(PayoutDestination::Stake), false, ) .unwrap(); @@ -817,7 +920,7 @@ fn double_controlling_attempt_should_fail() { Staking::bond( RuntimeOrigin::signed(stash), arbitrary_value.into(), - RewardDestination::default() + PayoutDestination::default() ), Error::::AlreadyBonded, ); @@ -1037,8 +1140,8 @@ fn cannot_reserve_staked_balance() { } #[test] -fn reward_destination_works() { - // Rewards go to the correct destination as determined in Payee +fn payout_destination_works() { + // Rewards go to the correct destination as determined in Payees ExtBuilder::default().nominate(false).build_and_execute(|| { // Check that account 11 is a validator assert!(Session::validators().contains(&11)); @@ -1062,11 +1165,14 @@ fn reward_destination_works() { let total_payout_0 = current_total_payout_for_duration(reward_time_per_era()); Pallet::::reward_by_ids(vec![(11, 1)]); + // Check that PayoutDestination is Staked (default) + assert_eq!(Staking::payees(&11), CheckedPayoutDestination(PayoutDestination::Stake)); + mock::start_active_era(1); mock::make_all_reward_payment(0); - // Check that RewardDestination is Staked (default) - assert_eq!(Staking::payee(11.into()), RewardDestination::Staked); + // Check that PayoutDestination is Stake (default) + assert_eq!(Staking::payee(11.into()), PayoutDestination::Stake); // Check that reward went to the stash account of validator assert_eq!(Balances::free_balance(11), 1000 + total_payout_0); // Check that amount at stake increased accordingly @@ -1080,19 +1186,16 @@ fn reward_destination_works() { claimed_rewards: bounded_vec![0], } ); - - // Change RewardDestination to Stash - >::insert(&11, RewardDestination::Stash); - + // Change PayoutDestination to Stash + >::insert(&11, CheckedPayoutDestination(PayoutDestination::Deposit(11))); // Compute total payout now for whole duration as other parameter won't change let total_payout_1 = current_total_payout_for_duration(reward_time_per_era()); Pallet::::reward_by_ids(vec![(11, 1)]); mock::start_active_era(2); mock::make_all_reward_payment(1); - - // Check that RewardDestination is Stash - assert_eq!(Staking::payee(11.into()), RewardDestination::Stash); + // Check that PayoutDestination is Stash + assert_eq!(Staking::payees(&11), CheckedPayoutDestination(PayoutDestination::Deposit(11))); // Check that reward went to the stash account assert_eq!(Balances::free_balance(11), 1000 + total_payout_0 + total_payout_1); // Check that amount at stake is NOT increased @@ -1107,8 +1210,8 @@ fn reward_destination_works() { } ); - // Change RewardDestination to Controller - >::insert(&11, RewardDestination::Controller); + // Change PayoutDestination to Controller + >::insert(&11, CheckedPayoutDestination(PayoutDestination::Deposit(11))); // Check controller balance assert_eq!(Balances::free_balance(11), 23150); @@ -1120,8 +1223,8 @@ fn reward_destination_works() { mock::start_active_era(3); mock::make_all_reward_payment(2); - // Check that RewardDestination is Controller - assert_eq!(Staking::payee(11.into()), RewardDestination::Controller); + // Check that PayoutDestination is Controller + assert_eq!(Staking::payees(&11), CheckedPayoutDestination(PayoutDestination::Deposit(11))); // Check that reward went to the controller account assert_eq!(Balances::free_balance(11), 23150 + total_payout_2); // Check that amount at stake is NOT increased @@ -1146,10 +1249,9 @@ fn validator_payment_prefs_work() { ExtBuilder::default().build_and_execute(|| { let commission = Perbill::from_percent(40); >::insert(&11, ValidatorPrefs { commission, ..Default::default() }); - // Reward controller so staked ratio doesn't change. - >::insert(&11, RewardDestination::Controller); - >::insert(&101, RewardDestination::Controller); + >::insert(&11, CheckedPayoutDestination(PayoutDestination::Stake)); + >::insert(&101, CheckedPayoutDestination(PayoutDestination::Stake)); mock::start_active_era(1); mock::make_all_reward_payment(0); @@ -1239,7 +1341,7 @@ fn bond_extra_and_withdraw_unbonded_works() { // * Once the unbonding period is done, it can actually take the funds out of the stash. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), PayoutDestination::Deposit(11))); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1450,7 +1552,7 @@ fn rebond_works() { // * it can re-bond a portion of the funds scheduled to unlock. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), PayoutDestination::Deposit(11))); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1576,7 +1678,7 @@ fn rebond_is_fifo() { // Rebond should proceed by reversing the most recent bond operations. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), PayoutDestination::Deposit(11))); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1672,7 +1774,7 @@ fn rebond_emits_right_value_in_event() { // and the rebond event emits the actual value rebonded. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), PayoutDestination::Deposit(11))); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1797,7 +1899,7 @@ fn reap_stash_works() { assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); - assert!(>::contains_key(&11)); + assert!(>::contains_key(&11)); // stash is not reapable assert_noop!( @@ -1816,7 +1918,7 @@ fn reap_stash_works() { assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); - assert!(!>::contains_key(&11)); + assert!(!>::contains_key(&11)); }); } @@ -1829,7 +1931,7 @@ fn switching_roles() { for i in &[11, 21] { assert_ok!(Staking::set_payee( RuntimeOrigin::signed(*i), - RewardDestination::Controller + PayoutDestination::Deposit(i) )); } @@ -1841,14 +1943,14 @@ fn switching_roles() { } // add 2 nominators - assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 2000, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 2000, PayoutDestination::Deposit(1))); assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 5])); - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, PayoutDestination::Deposit(3))); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21, 1])); // add a new validator candidate - assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1000, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1000, PayoutDestination::Deposit(5))); assert_ok!(Staking::validate(RuntimeOrigin::signed(5), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(5), @@ -1919,11 +2021,11 @@ fn bond_with_no_staked_value() { .build_and_execute(|| { // Can't bond with 1 assert_noop!( - Staking::bond(RuntimeOrigin::signed(1), 1, RewardDestination::Controller), + Staking::bond(RuntimeOrigin::signed(1), 1, PayoutDestination::Deposit(1)), Error::::InsufficientBond, ); // bonded with absolute minimum value possible. - assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 5, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 5, PayoutDestination::Deposit(1))); assert_eq!(Balances::locks(&1)[0].amount, 5); // unbonding even 1 will cause all to be unbonded. @@ -1967,13 +2069,13 @@ fn bond_with_little_staked_value_bounded() { assert_ok!(Staking::chill(RuntimeOrigin::signed(31))); assert_ok!(Staking::set_payee( RuntimeOrigin::signed(11), - RewardDestination::Controller + PayoutDestination::Deposit(11) )); let init_balance_1 = Balances::free_balance(&1); let init_balance_11 = Balances::free_balance(&11); // Stingy validator. - assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 1, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 1, PayoutDestination::Deposit(11))); assert_ok!(Staking::validate(RuntimeOrigin::signed(1), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(1), @@ -2052,14 +2154,14 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { assert_ok!(Staking::bond( RuntimeOrigin::signed(1), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(1) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 11, 11, 21, 31])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(1) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21, 31])); @@ -2105,14 +2207,14 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { assert_ok!(Staking::bond( RuntimeOrigin::signed(1), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(1) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 11, 11, 21])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(3) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21])); @@ -2196,7 +2298,7 @@ fn reward_validator_slashing_validator_does_not_overflow() { let _ = Balances::make_free_balance_be(&2, stake); // only slashes out of bonded stake are applied. without this line, it is 0. - Staking::bond(RuntimeOrigin::signed(2), stake - 1, RewardDestination::default()).unwrap(); + Staking::bond(RuntimeOrigin::signed(2), stake - 1, PayoutDestination::default()).unwrap(); // Override exposure of 11 ErasStakers::::insert( 0, @@ -3519,8 +3621,8 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { let part_for_101 = Perbill::from_rational::(125, 1125); // Check state - Payee::::insert(11, RewardDestination::Controller); - Payee::::insert(101, RewardDestination::Controller); + Payees::::insert(11, PayoutDestination::Deposit(11)); + Payees::::insert(101, PayoutDestination::Deposit(101)); Pallet::::reward_by_ids(vec![(11, 1)]); // Compute total payout now for whole duration as other parameter won't change @@ -3681,7 +3783,7 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( assert_ok!(Staking::bond( RuntimeOrigin::signed(stash), balance, - RewardDestination::Stash + PayoutDestination::Deposit(stash) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![11])); } @@ -3756,8 +3858,16 @@ fn test_payout_stakers() { staking_events_since_last_call().as_slice(), &[ .., - Event::Rewarded { stash: 1037, dest: RewardDestination::Controller, amount: 108 }, - Event::Rewarded { stash: 1036, dest: RewardDestination::Controller, amount: 108 } + Event::Rewarded { + stash: 1037, + dest: PayoutDestination::Deposit(1037), + amount: 108 + }, + Event::Rewarded { + stash: 1036, + dest: PayoutDestination::Deposit(1036), + amount: 108 + } ] )); @@ -4188,7 +4298,7 @@ fn payout_creates_controller() { let (stash, controller) = testing_utils::create_unique_stash_controller::( 0, 100, - RewardDestination::Controller, + PayoutRoute::Direct(PayoutDestination::Stake), false, ) .unwrap(); @@ -4222,7 +4332,7 @@ fn payout_to_any_account_works() { bond_nominator(1234, 100, vec![11]); // Update payout location - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(1234), RewardDestination::Account(42))); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(1234), PayoutDestination::Deposit(42))); // Reward Destination account doesn't exist assert_eq!(Balances::free_balance(42), 0); @@ -4954,7 +5064,7 @@ fn min_bond_checks_work() { .min_validator_bond(1_500) .build_and_execute(|| { // 500 is not enough for any role - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, PayoutDestination::Deposit(3))); assert_noop!( Staking::nominate(RuntimeOrigin::signed(3), vec![1]), Error::::InsufficientBond @@ -5019,7 +5129,7 @@ fn chill_other_works() { assert_ok!(Staking::bond( RuntimeOrigin::signed(a), 1000, - RewardDestination::Controller + PayoutDestination::Deposit(a) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(a), vec![1])); @@ -5027,7 +5137,7 @@ fn chill_other_works() { assert_ok!(Staking::bond( RuntimeOrigin::signed(b), 1500, - RewardDestination::Controller + PayoutDestination::Deposit(b) )); assert_ok!(Staking::validate(RuntimeOrigin::signed(b), ValidatorPrefs::default())); } @@ -5175,7 +5285,7 @@ fn capped_stakers_works() { let (_, controller) = testing_utils::create_stash_controller::( i + 10_000_000, 100, - RewardDestination::Controller, + PayoutDestination::Deposit(i), ) .unwrap(); assert_ok!(Staking::validate( @@ -5189,7 +5299,7 @@ fn capped_stakers_works() { let (_, last_validator) = testing_utils::create_stash_controller::( 1337, 100, - RewardDestination::Controller, + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); @@ -5204,7 +5314,7 @@ fn capped_stakers_works() { let (_, controller) = testing_utils::create_stash_controller::( i + 20_000_000, 100, - RewardDestination::Controller, + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![1])); @@ -5215,7 +5325,7 @@ fn capped_stakers_works() { let (_, last_nominator) = testing_utils::create_stash_controller::( 30_000_000, 100, - RewardDestination::Controller, + PayoutDestination::Deposit(100), ) .unwrap(); assert_noop!( @@ -5776,7 +5886,7 @@ fn pre_bonding_era_cannot_be_claimed() { mock::start_active_era(current_era); // add a new candidate for being a validator. account 3 controlled by 4. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, PayoutDestination::Deposit(3))); let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); @@ -5840,7 +5950,7 @@ fn reducing_history_depth_abrupt() { mock::start_active_era(current_era); // add a new candidate for being a staker. account 3 controlled by 3. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, PayoutDestination::Deposit(3))); // all previous era before the bonding action should be marked as // claimed. @@ -5878,7 +5988,7 @@ fn reducing_history_depth_abrupt() { ); // new stakers can still bond - assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1200, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1200, PayoutDestination::Deposit(5))); // new staking ledgers created will be bounded by the current history depth let last_reward_era = current_era - 1; @@ -5909,7 +6019,7 @@ fn reducing_max_unlocking_chunks_abrupt() { // given a staker at era=10 and MaxUnlockChunks set to 2 MaxUnlockingChunks::set(2); start_active_era(10); - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 300, RewardDestination::Staked)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 300, PayoutDestination::Stake)); assert!(matches!(Staking::ledger(3.into()), Ok(_))); // when staker unbonds @@ -6130,7 +6240,7 @@ mod ledger { assert_ok!(Staking::bond( RuntimeOrigin::signed(10), 100, - RewardDestination::Controller + PayoutDestination::Deposit(10) )); assert_eq!(>::get(&10), Some(10)); @@ -6206,12 +6316,12 @@ mod ledger { assert!(>::get(&42).is_none()); let mut ledger: StakingLedger = StakingLedger::default_from(42); - let reward_dest = RewardDestination::Account(10); + let reward_dest = PayoutDestination::Deposit(10); assert_ok!(ledger.clone().bond(reward_dest)); assert!(StakingLedger::::is_bonded(StakingAccount::Stash(42))); assert!(>::get(&42).is_some()); - assert_eq!(>::get(&42), reward_dest); + assert_eq!(>::get(&42), reward_dest); // cannot bond again. assert!(ledger.clone().bond(reward_dest).is_err()); diff --git a/substrate/frame/staking/src/weights.rs b/substrate/frame/staking/src/weights.rs index f2c65e677cac8..dca332d3b5b7f 100644 --- a/substrate/frame/staking/src/weights.rs +++ b/substrate/frame/staking/src/weights.rs @@ -62,6 +62,7 @@ pub trait WeightInfo { fn nominate(n: u32, ) -> Weight; fn chill() -> Weight; fn set_payee() -> Weight; + fn update_payee() -> Weight; fn set_controller() -> Weight; fn set_validator_count() -> Weight; fn force_no_eras() -> Weight; @@ -339,6 +340,23 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } + /// Storage: `Staking::Ledger` (r:1 w:0) + /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) + /// Storage: `Staking::Payees` (r:1 w:1) + /// Proof: `Staking::Payees` (`max_values`: None, `max_size`: Some(77), added: 2552, mode: `MaxEncodedLen`) + /// Storage: `Staking::Payee` (r:1 w:1) + /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) + /// Storage: `Staking::CounterForPayees` (r:1 w:1) + /// Proof: `Staking::CounterForPayees` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + fn update_payee() -> Weight { + // Proof Size summary in bytes: + // Measured: `989` + // Estimated: `4556` + // Minimum execution time: 29_795_000 picoseconds. + Weight::from_parts(30_505_000, 4556) + .saturating_add(T::DbWeight::get().reads(4_u64)) + .saturating_add(T::DbWeight::get().writes(3_u64)) + } /// Storage: Staking Bonded (r:1 w:1) /// Proof: Staking Bonded (max_values: None, max_size: Some(72), added: 2547, mode: MaxEncodedLen) /// Storage: Staking Ledger (r:2 w:2) @@ -1049,6 +1067,23 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } + /// Storage: `Staking::Ledger` (r:1 w:0) + /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) + /// Storage: `Staking::Payees` (r:1 w:1) + /// Proof: `Staking::Payees` (`max_values`: None, `max_size`: Some(77), added: 2552, mode: `MaxEncodedLen`) + /// Storage: `Staking::Payee` (r:1 w:1) + /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) + /// Storage: `Staking::CounterForPayees` (r:1 w:1) + /// Proof: `Staking::CounterForPayees` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + fn update_payee() -> Weight { + // Proof Size summary in bytes: + // Measured: `989` + // Estimated: `4556` + // Minimum execution time: 29_795_000 picoseconds. + Weight::from_parts(30_505_000, 4556) + .saturating_add(RocksDbWeight::get().reads(4_u64)) + .saturating_add(RocksDbWeight::get().writes(3_u64)) + } /// Storage: Staking Bonded (r:1 w:1) /// Proof: Staking Bonded (max_values: None, max_size: Some(72), added: 2547, mode: MaxEncodedLen) /// Storage: Staking Ledger (r:2 w:2) From d12d298d7fe46e12bcfba742819283ce4e77be44 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 15 Nov 2023 10:11:13 +0000 Subject: [PATCH 3/5] fmt --- substrate/frame/staking/src/lib.rs | 4 ++-- substrate/frame/staking/src/pallet/impls.rs | 7 ++++--- substrate/frame/staking/src/pallet/mod.rs | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 910b0a8af2b3e..ee1ac535d0f1b 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -322,8 +322,8 @@ use sp_runtime::{ }; use sp_staking::{ offence::{Offence, OffenceError, ReportOffence}, - EraIndex, ExposurePage, PayoutDestinationAlias, OnStakingUpdate, Page, PagedExposureMetadata, SessionIndex, - StakingAccount, + EraIndex, ExposurePage, OnStakingUpdate, Page, PagedExposureMetadata, PayoutDestinationAlias, + SessionIndex, StakingAccount, }; pub use sp_staking::{Exposure, IndividualExposure, StakerStatus}; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 554b26807bc80..bf60ff638b528 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -50,8 +50,8 @@ use sp_std::prelude::*; use crate::{ election_size_tracker::StaticTracker, log, slashing, weights::WeightInfo, ActiveEraInfo, BalanceOf, EraInfo, EraPayout, Exposure, ExposureOf, Forcing, IndividualExposure, - MaxNominationsOf, MaxWinnersOf, Nominations, NominationsQuota, PositiveImbalanceOf, - PayoutDestination, SessionInterface, StakingLedger, ValidatorPrefs, + MaxNominationsOf, MaxWinnersOf, Nominations, NominationsQuota, PayoutDestination, + PositiveImbalanceOf, SessionInterface, StakingLedger, ValidatorPrefs, }; use super::pallet::*; @@ -338,7 +338,6 @@ impl Pallet { if amount.is_zero() { return None } - // NOTE: temporary getter while `Payee` -> `Payees` lazy migration is taking place. // Tracking issue: // Can replace with `dest = Self:payees(stash);` once migration is done. @@ -1115,6 +1114,8 @@ impl Pallet { } else { Payees::::get(stash).0 } + } + /// Returns full exposure of a validator for a given era. /// /// History note: This used to be a getter for old storage item `ErasStakers` deprecated in v14. diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index de2e89a24be3e..a8cf58ecf66cd 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -49,7 +49,7 @@ pub use impls::*; use crate::{ slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout, EraRewardPoints, Exposure, ExposurePage, Forcing, MaxNominationsOf, NegativeImbalanceOf, - Nominations, NominationsQuota, PositiveImbalanceOf, PayoutDestination, SessionInterface, + Nominations, NominationsQuota, PayoutDestination, PositiveImbalanceOf, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk, ValidatorPrefs, }; From 6495e7b93dc8851d27db1fa1817a181f77d88c29 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 15 Nov 2023 10:39:15 +0000 Subject: [PATCH 4/5] fix test errors --- substrate/frame/staking/src/lib.rs | 2 +- substrate/frame/staking/src/mock.rs | 2 +- substrate/frame/staking/src/pallet/impls.rs | 12 ++--- substrate/frame/staking/src/pallet/mod.rs | 18 ++++---- substrate/frame/staking/src/testing_utils.rs | 41 ++++++++++++----- substrate/frame/staking/src/tests.rs | 46 ++++++++++++-------- 6 files changed, 73 insertions(+), 48 deletions(-) diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index ee1ac535d0f1b..456de8d08155f 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -325,7 +325,7 @@ use sp_staking::{ EraIndex, ExposurePage, OnStakingUpdate, Page, PagedExposureMetadata, PayoutDestinationAlias, SessionIndex, StakingAccount, }; -pub use sp_staking::{Exposure, IndividualExposure, StakerStatus}; +pub use sp_staking::{Exposure, IndividualExposure, PayoutSplitOpt, StakerStatus}; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; pub use weights::WeightInfo; diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index d2afd8f26e241..861e34ee72d5f 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -594,7 +594,7 @@ pub(crate) fn current_era() -> EraIndex { pub(crate) fn bond(who: AccountId, val: Balance) { let _ = Balances::make_free_balance_be(&who, val); - assert_ok!(Staking::bond(RuntimeOrigin::signed(who), val, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(who), val, PayoutDestination::Deposit(who))); } pub(crate) fn bond_validator(who: AccountId, val: Balance) { diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index bf60ff638b528..99e1fc946cc35 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -49,9 +49,9 @@ use sp_std::prelude::*; use crate::{ election_size_tracker::StaticTracker, log, slashing, weights::WeightInfo, ActiveEraInfo, - BalanceOf, EraInfo, EraPayout, Exposure, ExposureOf, Forcing, IndividualExposure, - MaxNominationsOf, MaxWinnersOf, Nominations, NominationsQuota, PayoutDestination, - PositiveImbalanceOf, SessionInterface, StakingLedger, ValidatorPrefs, + BalanceOf, CheckedPayoutDestination, EraInfo, EraPayout, Exposure, ExposureOf, Forcing, + IndividualExposure, MaxNominationsOf, MaxWinnersOf, Nominations, NominationsQuota, + PayoutDestination, PositiveImbalanceOf, SessionInterface, StakingLedger, ValidatorPrefs, }; use super::pallet::*; @@ -351,13 +351,13 @@ impl Pallet { let payout_destination_stake = |a: BalanceOf| -> Option> { Self::ledger(Stash(stash.clone())) .and_then(|mut ledger| { - ledger.active += amount; - ledger.total += amount; + ledger.active += a; + ledger.total += a; let _ = ledger .update() .defensive_proof("ledger fetched from storage, so it exists; qed."); - let r = T::Currency::deposit_into_existing(stash, amount).ok(); + let r = T::Currency::deposit_into_existing(stash, a).ok(); Ok(r) }) .unwrap_or_default() diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index a8cf58ecf66cd..e5ef5220d7349 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -47,10 +47,11 @@ mod impls; pub use impls::*; use crate::{ - slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, EraPayout, - EraRewardPoints, Exposure, ExposurePage, Forcing, MaxNominationsOf, NegativeImbalanceOf, - Nominations, NominationsQuota, PayoutDestination, PositiveImbalanceOf, SessionInterface, - StakingLedger, UnappliedSlash, UnlockChunk, ValidatorPrefs, + slashing, weights::WeightInfo, AccountIdLookupOf, ActiveEraInfo, BalanceOf, + CheckedPayoutDestination, EraPayout, EraRewardPoints, Exposure, ExposurePage, Forcing, + MaxNominationsOf, NegativeImbalanceOf, Nominations, NominationsQuota, PayoutDestination, + PositiveImbalanceOf, RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, + UnlockChunk, ValidatorPrefs, }; // The speculative number of spans are used as an input of the weight annotation of @@ -946,10 +947,6 @@ pub mod pallet { // respectively. let checked_payee = PayoutDestination::to_checked(payee); - let current_era = CurrentEra::::get().unwrap_or(0); - let history_depth = T::HistoryDepth::get(); - let last_reward_era = current_era.saturating_sub(history_depth); - let stash_balance = T::Currency::free_balance(&stash); let value = value.min(stash_balance); Self::deposit_event(Event::::Bonded { stash: stash.clone(), amount: value }); @@ -1852,7 +1849,10 @@ pub mod pallet { /// This will waive the transaction fee if the payee is successfully migrated. #[pallet::call_index(27)] #[pallet::weight(0)] - pub fn update_payee(origin: OriginFor) -> DispatchResultWithPostInfo { + pub fn update_payee( + origin: OriginFor, + controller: T::AccountId, + ) -> DispatchResultWithPostInfo { let controller = ensure_signed(origin)?; let ledger = Self::ledger(Controller(controller.clone()))?; diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index 28e08230d701d..f79363dd538a1 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -74,11 +74,15 @@ pub fn create_funded_user_with_balance( pub fn create_stash_controller( n: u32, balance_factor: u32, - destination: RewardDestination, + destination: PayoutRoute, ) -> Result<(T::AccountId, T::AccountId), &'static str> { let staker = create_funded_user::("stash", n, balance_factor); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); - Staking::::bond(RawOrigin::Signed(staker.clone()).into(), amount, destination)?; + Staking::::bond( + RawOrigin::Signed(staker.clone()).into(), + amount, + PayoutDestination::from_route(destination, &staker, &staker), + )?; Ok((staker.clone(), staker)) } @@ -86,7 +90,7 @@ pub fn create_stash_controller( pub fn create_unique_stash_controller( n: u32, balance_factor: u32, - destination: RewardDestination, + destination: PayoutRoute, dead_controller: bool, ) -> Result<(T::AccountId, T::AccountId), &'static str> { let stash = create_funded_user::("stash", n, balance_factor); @@ -97,7 +101,11 @@ pub fn create_unique_stash_controller( create_funded_user::("controller", n, balance_factor) }; let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); - Staking::::bond(RawOrigin::Signed(stash.clone()).into(), amount, destination)?; + Staking::::bond( + RawOrigin::Signed(stash.clone()).into(), + amount, + PayoutDestination::from_route(destination, &stash, &controller), + )?; // update ledger to be a *different* controller to stash if let Some(l) = Ledger::::take(&stash) { @@ -113,7 +121,7 @@ pub fn create_unique_stash_controller( pub fn create_stash_controller_with_balance( n: u32, balance: crate::BalanceOf, - destination: RewardDestination, + destination: PayoutDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { let staker = create_funded_user_with_balance::("stash", n, balance); Staking::::bond(RawOrigin::Signed(staker.clone()).into(), balance, destination)?; @@ -133,7 +141,7 @@ pub fn create_stash_and_dead_payee( Staking::::bond( RawOrigin::Signed(staker.clone()).into(), amount, - RewardDestination::Account(payee), + PayoutDestination::Deposit(payee), )?; Ok((staker.clone(), staker)) } @@ -154,8 +162,11 @@ pub fn create_validators_with_seed( ) -> Result>, &'static str> { let mut validators: Vec> = Vec::with_capacity(max as usize); for i in 0..max { - let (stash, controller) = - create_stash_controller::(i + seed, balance_factor, RewardDestination::Staked)?; + let (stash, controller) = create_stash_controller::( + i + seed, + balance_factor, + PayoutRoute::Direct(PayoutDestination::Stake), + )?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(controller).into(), validator_prefs)?; @@ -195,8 +206,11 @@ pub fn create_validators_with_nominators_for_era( // Create validators for i in 0..validators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (v_stash, v_controller) = - create_stash_controller::(i, balance_factor, RewardDestination::Staked)?; + let (v_stash, v_controller) = create_stash_controller::( + i, + balance_factor, + PayoutRoute::Direct(PayoutDestination::Stake), + )?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(v_controller.clone()).into(), validator_prefs)?; @@ -210,8 +224,11 @@ pub fn create_validators_with_nominators_for_era( // Create nominators for j in 0..nominators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (_n_stash, n_controller) = - create_stash_controller::(u32::MAX - j, balance_factor, RewardDestination::Staked)?; + let (_n_stash, n_controller) = create_stash_controller::( + u32::MAX - j, + balance_factor, + PayoutRoute::Direct(PayoutDestination::Stake), + )?; // Have them randomly validate let mut available_validators = validator_chosen.clone(); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index d645597854c0e..6c4debe491d56 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -237,7 +237,7 @@ fn basic_setup_works() { #[test] fn change_controller_works() { ExtBuilder::default().build_and_execute(|| { - let (stash, controller) = testing_utils::create_unique_stash_controller::( + let (stash, controller) = create_unique_stash_controller::( 0, 100, PayoutRoute::Direct(PayoutDestination::Stake), @@ -719,7 +719,7 @@ fn set_payee_also_updates_payee_destination() { .unwrap(); Payees::::remove(stash); // Value to be migrated - DeprecatedPayee::::insert(stash, PayoutDestination::Deposit(stash)); + DeprecatedPayee::::insert(stash, RewardDestination::Staked); // When assert_ok!(Staking::set_payee( @@ -753,7 +753,7 @@ fn update_payee_works() { .unwrap(); Payees::::remove(stash); // Value to be migrated - DeprecatedPayee::::insert(stash, PayoutDestination::Stake); + DeprecatedPayee::::insert(stash, RewardDestination::Staked); // When assert_ok!(Staking::set_payee(RuntimeOrigin::signed(controller), PayoutDestination::Stake)); @@ -800,7 +800,7 @@ fn get_destination_payout_migrates_payee() { create_stash_controller::(12, 13, PayoutRoute::Direct(PayoutDestination::Stake)) .unwrap(); Payees::::remove(stash); - DeprecatedPayee::::insert(stash, PayoutDestination::Stake); + DeprecatedPayee::::insert(stash, RewardDestination::Staked); // When let dest = Staking::get_payout_destination_migrate(&stash, controller); @@ -874,7 +874,7 @@ fn double_staking_should_fail() { // * an account already bonded as controller can nominate. ExtBuilder::default().build_and_execute(|| { let arbitrary_value = 5; - let (stash, controller) = testing_utils::create_unique_stash_controller::( + let (stash, controller) = create_unique_stash_controller::( 0, arbitrary_value, PayoutRoute::Direct(PayoutDestination::Stake), @@ -908,7 +908,7 @@ fn double_controlling_attempt_should_fail() { // account. ExtBuilder::default().build_and_execute(|| { let arbitrary_value = 5; - let (stash, _) = testing_utils::create_unique_stash_controller::( + let (stash, _) = create_unique_stash_controller::( 0, arbitrary_value, PayoutRoute::Direct(PayoutDestination::Stake), @@ -1173,7 +1173,7 @@ fn payout_destination_works() { mock::make_all_reward_payment(0); // Check that PayoutDestination is Stake (default) - assert_eq!(Staking::payee(11.into()), PayoutDestination::Stake); + assert_eq!(Staking::payee(11.into()), CheckedPayoutDestination(PayoutDestination::Stake)); // Check that reward went to the stash account of validator assert_eq!(Balances::free_balance(11), 1000 + total_payout_0); // Check that amount at stake increased accordingly @@ -1942,7 +1942,7 @@ fn switching_roles() { for i in &[11, 21] { assert_ok!(Staking::set_payee( RuntimeOrigin::signed(*i), - PayoutDestination::Deposit(i) + PayoutDestination::Deposit(*i) )); } @@ -3634,8 +3634,8 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { let part_for_101 = Perbill::from_rational::(125, 1125); // Check state - Payees::::insert(11, PayoutDestination::Deposit(11)); - Payees::::insert(101, PayoutDestination::Deposit(101)); + Payees::::insert(11, CheckedPayoutDestination(PayoutDestination::Deposit(11))); + Payees::::insert(101, CheckedPayoutDestination(PayoutDestination::Deposit(101))); Pallet::::reward_by_ids(vec![(11, 1)]); // Compute total payout now for whole duration as other parameter won't change @@ -3839,7 +3839,7 @@ fn test_nominators_are_rewarded_for_all_exposure_page() { assert_ok!(Staking::bond( RuntimeOrigin::signed(stash), balance, - RewardDestination::Stash + PayoutDestination::Deposit(stash) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![11])); } @@ -3926,12 +3926,12 @@ fn test_multi_page_payout_stakers_by_page() { .., Event::Rewarded { stash: 1063, - dest: PayoutDestination::Deposit(1037), + dest: CheckedPayoutDestination(PayoutDestination::Deposit(1037)), amount: 111 }, Event::Rewarded { stash: 1064, - dest: PayoutDestination::Deposit(1036), + dest: CheckedPayoutDestination(PayoutDestination::Deposit(1036)), amount: 111 } ] @@ -3955,8 +3955,16 @@ fn test_multi_page_payout_stakers_by_page() { events.as_slice(), &[ Event::PayoutStarted { era_index: 1, validator_stash: 11 }, - Event::Rewarded { stash: 1065, dest: RewardDestination::Controller, amount: 111 }, - Event::Rewarded { stash: 1066, dest: RewardDestination::Controller, amount: 111 }, + Event::Rewarded { + stash: 1065, + dest: CheckedPayoutDestination(PayoutDestination::Deposit(_)), + amount: 111 + }, + Event::Rewarded { + stash: 1066, + dest: CheckedPayoutDestination(PayoutDestination::Deposit(_)), + amount: 111 + }, .. ] )); @@ -4805,7 +4813,7 @@ fn payout_creates_controller() { bond_validator(11, balance); // create a stash/controller pair and nominate - let (stash, controller) = testing_utils::create_unique_stash_controller::( + let (stash, controller) = create_unique_stash_controller::( 0, 100, PayoutRoute::Direct(PayoutDestination::Stake), @@ -5795,7 +5803,7 @@ fn capped_stakers_works() { let (_, controller) = testing_utils::create_stash_controller::( i + 10_000_000, 100, - PayoutDestination::Deposit(i), + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); assert_ok!(Staking::validate( @@ -5835,7 +5843,7 @@ fn capped_stakers_works() { let (_, last_nominator) = testing_utils::create_stash_controller::( 30_000_000, 100, - PayoutDestination::Deposit(100), + PayoutRoute::Direct(PayoutDestination::Stake), ) .unwrap(); assert_noop!( @@ -6959,7 +6967,7 @@ mod ledger { assert!(>::get(&42).is_none()); let mut ledger: StakingLedger = StakingLedger::default_from(42); - let reward_dest = PayoutDestination::Deposit(10); + let reward_dest = CheckedPayoutDestination(PayoutDestination::Deposit(10)); assert_ok!(ledger.clone().bond(reward_dest)); assert!(StakingLedger::::is_bonded(StakingAccount::Stash(42))); From b47573b98489973a6a307f6239eb7693ca2a0e22 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 15 Nov 2023 11:46:43 +0000 Subject: [PATCH 5/5] fixes --- substrate/frame/staking/src/pallet/mod.rs | 2 +- substrate/frame/staking/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index e5ef5220d7349..db36c4c679b97 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1853,7 +1853,7 @@ pub mod pallet { origin: OriginFor, controller: T::AccountId, ) -> DispatchResultWithPostInfo { - let controller = ensure_signed(origin)?; + let _ = ensure_signed(origin)?; let ledger = Self::ledger(Controller(controller.clone()))?; if Payees::::contains_key(&ledger.stash) && diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 6c4debe491d56..b9382a2b57d0d 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -2172,7 +2172,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, - PayoutDestination::Deposit(1) + PayoutDestination::Deposit(3) )); assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21, 31]));