From 8ee4042c3bc86d4232e51a9bdb5ec1ae5ca444ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Sun, 15 Oct 2023 22:50:07 +0200 Subject: [PATCH] Refactor staking ledger (#1484) This PR refactors the staking ledger logic to encapsulate all reads and mutations of `Ledger`, `Bonded`, `Payee` and stake locks within the `StakingLedger` struct implementation. With these changes, all the reads and mutations to the `Ledger`, `Payee` and `Bonded` storage map should be done through the methods exposed by StakingLedger to ensure the data and lock consistency of the operations. The new introduced methods that mutate and read Ledger are: - `ledger.update()`: inserts/updates a staking ledger in storage; updates staking locks accordingly (and ledger.bond(), which is synthatic sugar for ledger.update()) - `ledger.kill()`: removes all Bonded and StakingLedger related data for a given ledger; updates staking locks accordingly; `StakingLedger::get(account)`: queries both the `Bonded` and `Ledger` storages and returns a `Option`. The pallet impl exposes fn ledger(account) as synthatic sugar for `StakingLedger::get(account)`. Retrieving a ledger with `StakingLedger::get()` can be done by providing either a stash or controller account. The input must be wrapped in a `StakingAccount` variant (Stash or Controller) which is treated accordingly. This simplifies the caller API but will eventually be deprecated once we completely get rid of the controller account in staking. However, this refactor will help with the work necessary when completely removing the controller. Other goals: - No logical changes have been introduced in this PR; - No breaking changes or updates in wallets required; - No new storage items or need to perform storage migrations; - Centralise the changes to bonds and ledger updates to simplify the OnStakingUpdate updates to the target list (related to https://github.com/paritytech/polkadot-sdk/issues/443) Note: it would be great to prevent or at least raise a warning if `Ledger`, `Payee` and `Bonded` storage types are accessed outside the `StakingLedger` implementation. This PR should not get blocked by that feature, but there's a tracking issue here https://github.com/paritytech/polkadot-sdk/issues/149 Related and step towards https://github.com/paritytech/polkadot-sdk/issues/443 --- .../src/disputes/slashing/benchmarking.rs | 2 +- .../test-staking-e2e/src/lib.rs | 10 +- .../frame/session/benchmarking/src/lib.rs | 2 +- substrate/frame/staking/src/benchmarking.rs | 14 +- substrate/frame/staking/src/ledger.rs | 259 +++++++++ substrate/frame/staking/src/lib.rs | 40 +- substrate/frame/staking/src/mock.rs | 17 +- substrate/frame/staking/src/pallet/impls.rs | 177 +++---- substrate/frame/staking/src/pallet/mod.rs | 145 +++-- substrate/frame/staking/src/slashing.rs | 18 +- substrate/frame/staking/src/tests.rs | 500 +++++++++++------- substrate/primitives/staking/src/lib.rs | 17 + 12 files changed, 790 insertions(+), 411 deletions(-) create mode 100644 substrate/frame/staking/src/ledger.rs diff --git a/polkadot/runtime/parachains/src/disputes/slashing/benchmarking.rs b/polkadot/runtime/parachains/src/disputes/slashing/benchmarking.rs index 3ede1c9088025..f075ce5ca737d 100644 --- a/polkadot/runtime/parachains/src/disputes/slashing/benchmarking.rs +++ b/polkadot/runtime/parachains/src/disputes/slashing/benchmarking.rs @@ -51,7 +51,7 @@ where use rand::{RngCore, SeedableRng}; let validator = T::Lookup::lookup(who).unwrap(); - let controller = pallet_staking::Pallet::::bonded(validator).unwrap(); + let controller = pallet_staking::Pallet::::bonded(&validator).unwrap(); let keys = { const SESSION_KEY_LEN: usize = 32; diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs index e40bac3e9fc44..1d3f4712b1d60 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs @@ -338,7 +338,7 @@ fn ledger_consistency_active_balance_below_ed() { ExtBuilder::default().staking(StakingExtBuilder::default()).build_offchainify(); ext.execute_with(|| { - assert_eq!(Staking::ledger(&11).unwrap().active, 1000); + assert_eq!(Staking::ledger(11.into()).unwrap().active, 1000); // unbonding total of active stake fails because the active ledger balance would fall // below the `MinNominatorBond`. @@ -356,13 +356,13 @@ fn ledger_consistency_active_balance_below_ed() { // the active balance of the ledger entry is 0, while total balance is 1000 until // `withdraw_unbonded` is called. - assert_eq!(Staking::ledger(&11).unwrap().active, 0); - assert_eq!(Staking::ledger(&11).unwrap().total, 1000); + assert_eq!(Staking::ledger(11.into()).unwrap().active, 0); + assert_eq!(Staking::ledger(11.into()).unwrap().total, 1000); // trying to withdraw the unbonded balance won't work yet because not enough bonding // eras have passed. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); - assert_eq!(Staking::ledger(&11).unwrap().total, 1000); + assert_eq!(Staking::ledger(11.into()).unwrap().total, 1000); // tries to reap stash after chilling, which fails since the stash total balance is // above ED. @@ -384,6 +384,6 @@ fn ledger_consistency_active_balance_below_ed() { pool_state, ); assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); - assert_eq!(Staking::ledger(&11), None); + assert!(Staking::ledger(11.into()).is_err()); }); } diff --git a/substrate/frame/session/benchmarking/src/lib.rs b/substrate/frame/session/benchmarking/src/lib.rs index 722bb14fb56ed..84258d84994f4 100644 --- a/substrate/frame/session/benchmarking/src/lib.rs +++ b/substrate/frame/session/benchmarking/src/lib.rs @@ -134,7 +134,7 @@ fn check_membership_proof_setup( use rand::{RngCore, SeedableRng}; let validator = T::Lookup::lookup(who).unwrap(); - let controller = pallet_staking::Pallet::::bonded(validator).unwrap(); + let controller = pallet_staking::Pallet::::bonded(&validator).unwrap(); let keys = { let mut keys = [0u8; 128]; diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index ce5c35524c747..f94d9bf4b328a 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -29,7 +29,7 @@ use frame_support::{ }; use sp_runtime::{ traits::{Bounded, One, StaticLookup, TrailingZeroInput, Zero}, - Perbill, Percent, + Perbill, Percent, Saturating, }; use sp_staking::{currency_to_vote::CurrencyToVote, SessionIndex}; use sp_std::prelude::*; @@ -684,13 +684,11 @@ benchmarks! { let stash = scenario.origin_stash1; add_slashing_spans::(&stash, s); - let l = StakingLedger { - stash: stash.clone(), - active: T::Currency::minimum_balance() - One::one(), - total: T::Currency::minimum_balance() - One::one(), - unlocking: Default::default(), - claimed_rewards: Default::default(), - }; + let l = StakingLedger::::new( + stash.clone(), + T::Currency::minimum_balance() - One::one(), + Default::default(), + ); Ledger::::insert(&controller, l); assert!(Bonded::::contains_key(&stash)); diff --git a/substrate/frame/staking/src/ledger.rs b/substrate/frame/staking/src/ledger.rs new file mode 100644 index 0000000000000..cf9b4635bf55a --- /dev/null +++ b/substrate/frame/staking/src/ledger.rs @@ -0,0 +1,259 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! A Ledger implementation for stakers. +//! +//! A [`StakingLedger`] encapsulates all the state and logic related to the stake of bonded +//! stakers, namely, it handles the following storage items: +//! * [`Bonded`]: mutates and reads the state of the controller <> stash bond map (to be deprecated +//! soon); +//! * [`Ledger`]: mutates and reads the state of all the stakers. The [`Ledger`] storage item stores +//! instances of [`StakingLedger`] keyed by the staker's controller account and should be mutated +//! and read through the [`StakingLedger`] API; +//! * [`Payee`]: mutates and reads the reward destination preferences for a bonded stash. +//! * Staking locks: mutates the locks for staking. +//! +//! NOTE: All the storage operations related to the staking ledger (both reads and writes) *MUST* be +//! performed through the methods exposed by the [`StakingLedger`] implementation in order to ensure +//! state consistency. + +use frame_support::{ + defensive, + traits::{LockableCurrency, WithdrawReasons}, + BoundedVec, +}; +use sp_staking::{EraIndex, StakingAccount}; +use sp_std::prelude::*; + +use crate::{ + BalanceOf, Bonded, Config, Error, Ledger, Payee, RewardDestination, StakingLedger, STAKING_ID, +}; + +#[cfg(any(feature = "runtime-benchmarks", test))] +use sp_runtime::traits::Zero; + +impl StakingLedger { + #[cfg(any(feature = "runtime-benchmarks", test))] + pub fn default_from(stash: T::AccountId) -> Self { + Self { + stash: stash.clone(), + total: Zero::zero(), + active: Zero::zero(), + unlocking: Default::default(), + claimed_rewards: Default::default(), + controller: Some(stash), + } + } + + /// Returns a new instance of a staking ledger. + /// + /// The [`Ledger`] storage is not mutated. In order to store, `StakingLedger::update` must be + /// called on the returned staking ledger. + /// + /// Note: as the controller accounts are being deprecated, the stash account is the same as the + /// controller account. + pub fn new( + stash: T::AccountId, + stake: BalanceOf, + claimed_rewards: BoundedVec, + ) -> Self { + Self { + stash: stash.clone(), + active: stake, + total: stake, + unlocking: Default::default(), + claimed_rewards, + // controllers are deprecated and mapped 1-1 to stashes. + controller: Some(stash), + } + } + + /// Returns the paired account, if any. + /// + /// A "pair" refers to the tuple (stash, controller). If the input is a + /// [`StakingAccount::Stash`] variant, its pair account will be of type + /// [`StakingAccount::Controller`] and vice-versa. + /// + /// This method is meant to abstract from the runtime development the difference between stash + /// and controller. This will be deprecated once the controller is fully deprecated as well. + pub(crate) fn paired_account(account: StakingAccount) -> Option { + match account { + StakingAccount::Stash(stash) => >::get(stash), + StakingAccount::Controller(controller) => + >::get(&controller).map(|ledger| ledger.stash), + } + } + + /// Returns whether a given account is bonded. + pub(crate) fn is_bonded(account: StakingAccount) -> bool { + match account { + StakingAccount::Stash(stash) => >::contains_key(stash), + StakingAccount::Controller(controller) => >::contains_key(controller), + } + } + + /// Returns a staking ledger, if it is bonded and it exists in storage. + /// + /// This getter can be called with either a controller or stash account, provided that the + /// account is properly wrapped in the respective [`StakingAccount`] variant. This is meant to + /// abstract the concept of controller/stash accounts from the caller. + pub(crate) fn get(account: StakingAccount) -> Result, Error> { + let controller = match account { + StakingAccount::Stash(stash) => >::get(stash).ok_or(Error::::NotStash), + StakingAccount::Controller(controller) => Ok(controller), + }?; + + >::get(&controller) + .map(|mut ledger| { + ledger.controller = Some(controller.clone()); + ledger + }) + .ok_or(Error::::NotController) + } + + /// Returns the reward destination of a staking ledger, stored in [`Payee`]. + /// + /// Note: if the stash is not bonded and/or does not have an entry in [`Payee`], it returns the + /// default reward destination. + pub(crate) fn reward_destination( + account: StakingAccount, + ) -> RewardDestination { + let stash = match account { + StakingAccount::Stash(stash) => Some(stash), + StakingAccount::Controller(controller) => + Self::paired_account(StakingAccount::Controller(controller)), + }; + + if let Some(stash) = stash { + >::get(stash) + } else { + defensive!("fetched reward destination from unbonded stash {}", stash); + RewardDestination::default() + } + } + + /// Returns the controller account of a staking ledger. + /// + /// Note: it will fallback into querying the [`Bonded`] storage with the ledger stash if the + /// controller is not set in `self`, which most likely means that self was fetched directly from + /// [`Ledger`] instead of through the methods exposed in [`StakingLedger`]. If the ledger does + /// not exist in storage, it returns `None`. + pub(crate) fn controller(&self) -> Option { + self.controller.clone().or_else(|| { + defensive!("fetched a controller on a ledger instance without it."); + Self::paired_account(StakingAccount::Stash(self.stash.clone())) + }) + } + + /// Inserts/updates a staking ledger account. + /// + /// Bonds the ledger if it is not bonded yet, signalling that this is a new ledger. The staking + /// locks of the stash account are updated accordingly. + /// + /// Note: To ensure lock consistency, all the [`Ledger`] storage updates should be made through + /// this helper function. + pub(crate) fn update(self) -> Result<(), Error> { + if !>::contains_key(&self.stash) { + return Err(Error::::NotStash) + } + + T::Currency::set_lock(STAKING_ID, &self.stash, self.total, WithdrawReasons::all()); + Ledger::::insert( + &self.controller().ok_or_else(|| { + defensive!("update called on a ledger that is not bonded."); + Error::::NotController + })?, + &self, + ); + + Ok(()) + } + + /// Bonds a ledger. + /// + /// It sets the reward preferences for the bonded stash. + pub(crate) fn bond(self, payee: RewardDestination) -> Result<(), Error> { + if >::contains_key(&self.stash) { + Err(Error::::AlreadyBonded) + } else { + >::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> { + if !>::contains_key(&self.stash) { + Err(Error::::NotStash) + } else { + >::insert(&self.stash, payee); + Ok(()) + } + } + + /// Clears all data related to a staking ledger and its bond in both [`Ledger`] and [`Bonded`] + /// storage items and updates the stash staking lock. + pub(crate) fn kill(stash: &T::AccountId) -> Result<(), Error> { + let controller = >::get(stash).ok_or(Error::::NotStash)?; + + >::get(&controller).ok_or(Error::::NotController).map(|ledger| { + T::Currency::remove_lock(STAKING_ID, &ledger.stash); + Ledger::::remove(controller); + + >::remove(&stash); + >::remove(&stash); + + Ok(()) + })? + } +} + +#[cfg(test)] +use { + crate::UnlockChunk, + codec::{Decode, Encode, MaxEncodedLen}, + scale_info::TypeInfo, +}; + +// This structs makes it easy to write tests to compare staking ledgers fetched from storage. This +// is required because the controller field is not stored in storage and it is private. +#[cfg(test)] +#[derive(frame_support::DebugNoBound, Clone, Encode, Decode, TypeInfo, MaxEncodedLen)] +pub struct StakingLedgerInspect { + pub stash: T::AccountId, + #[codec(compact)] + pub total: BalanceOf, + #[codec(compact)] + pub active: BalanceOf, + pub unlocking: BoundedVec>, T::MaxUnlockingChunks>, + pub claimed_rewards: BoundedVec, +} + +#[cfg(test)] +impl PartialEq> for StakingLedger { + fn eq(&self, other: &StakingLedgerInspect) -> bool { + self.stash == other.stash && + self.total == other.total && + self.active == other.active && + self.unlocking == other.unlocking && + self.claimed_rewards == other.claimed_rewards + } +} + +#[cfg(test)] +impl codec::EncodeLike> for StakingLedgerInspect {} diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index dcf57a4643233..227326763a9bd 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -91,7 +91,7 @@ //! #### Nomination //! //! A **nominator** does not take any _direct_ role in maintaining the network, instead, it votes on -//! a set of validators to be elected. Once interest in nomination is stated by an account, it +//! a set of validators to be elected. Once interest in nomination is stated by an account, it //! takes effect at the next election round. The funds in the nominator's stash account indicate the //! _weight_ of its vote. Both the rewards and any punishment that a validator earns are shared //! between the validator and its nominators. This rule incentivizes the nominators to NOT vote for @@ -294,6 +294,7 @@ mod tests; pub mod election_size_tracker; pub mod inflation; +pub mod ledger; pub mod migrations; pub mod slashing; pub mod weights; @@ -302,26 +303,27 @@ mod pallet; use codec::{Decode, Encode, HasCompact, MaxEncodedLen}; use frame_support::{ - traits::{ConstU32, Currency, Defensive, Get}, + traits::{ConstU32, Currency, Defensive, Get, LockIdentifier}, weights::Weight, BoundedVec, CloneNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound, }; use scale_info::TypeInfo; use sp_runtime::{ curve::PiecewiseLinear, - traits::{AtLeast32BitUnsigned, Convert, Saturating, StaticLookup, Zero}, - Perbill, Perquintill, Rounding, RuntimeDebug, + traits::{AtLeast32BitUnsigned, Convert, StaticLookup, Zero}, + Perbill, Perquintill, Rounding, RuntimeDebug, Saturating, }; pub use sp_staking::StakerStatus; use sp_staking::{ offence::{Offence, OffenceError, ReportOffence}, - EraIndex, OnStakingUpdate, SessionIndex, + EraIndex, OnStakingUpdate, SessionIndex, StakingAccount, }; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; pub use weights::WeightInfo; pub use pallet::{pallet::*, UseNominatorsAndValidatorsMap, UseValidatorsMap}; +pub(crate) const STAKING_ID: LockIdentifier = *b"staking "; pub(crate) const LOG_TARGET: &str = "runtime::staking"; // syntactic sugar for logging. @@ -433,6 +435,14 @@ pub struct UnlockChunk { } /// The ledger of a (bonded) stash. +/// +/// Note: All the reads and mutations to the [`Ledger`], [`Bonded`] and [`Payee`] storage items +/// *MUST* be performed through the methods exposed by this struct, to ensure the consistency of +/// ledger's data and corresponding staking lock +/// +/// TODO: move struct definition and full implementation into `/src/ledger.rs`. Currently +/// leaving here to enforce a clean PR diff, given how critical this logic is. Tracking issue +/// . #[derive( PartialEqNoBound, EqNoBound, @@ -462,20 +472,15 @@ pub struct StakingLedger { /// List of eras for which the stakers behind a validator have claimed rewards. Only updated /// for validators. pub claimed_rewards: BoundedVec, + /// The controller associated with this ledger's stash. + /// + /// This is not stored on-chain, and is only bundled when the ledger is read from storage. + /// Use [`controller`] function to get the controller associated with the ledger. + #[codec(skip)] + controller: Option, } impl StakingLedger { - /// Initializes the default object using the given `validator`. - pub fn default_from(stash: T::AccountId) -> Self { - Self { - stash, - total: Zero::zero(), - active: Zero::zero(), - unlocking: Default::default(), - claimed_rewards: Default::default(), - } - } - /// Remove entries from `unlocking` that are sufficiently old and reduce the /// total by the sum of their balances. fn consolidate_unlocked(self, current_era: EraIndex) -> Self { @@ -503,6 +508,7 @@ impl StakingLedger { active: self.active, unlocking, claimed_rewards: self.claimed_rewards, + controller: self.controller, } } @@ -927,7 +933,7 @@ pub struct StashOf(sp_std::marker::PhantomData); impl Convert> for StashOf { fn convert(controller: T::AccountId) -> Option { - >::ledger(&controller).map(|l| l.stash) + StakingLedger::::paired_account(StakingAccount::Controller(controller)) } } diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index c41144278f2c1..26d05d3a8c878 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -39,7 +39,10 @@ use sp_runtime::{ traits::{IdentityLookup, Zero}, BuildStorage, }; -use sp_staking::offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}; +use sp_staking::{ + offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}, + OnStakingUpdate, +}; pub const INIT_TIMESTAMP: u64 = 30_000; pub const BLOCK_TIME: u64 = 1000; @@ -77,7 +80,7 @@ impl sp_runtime::BoundToRuntimeAppPublic for OtherSessionHandler { } pub fn is_disabled(controller: AccountId) -> bool { - let stash = Staking::ledger(&controller).unwrap().stash; + let stash = Ledger::::get(&controller).unwrap().stash; let validator_index = match Session::validators().iter().position(|v| *v == stash) { Some(index) => index as u32, None => return false, @@ -778,6 +781,16 @@ pub(crate) fn make_all_reward_payment(era: EraIndex) { } } +pub(crate) fn bond_controller_stash(controller: AccountId, stash: AccountId) -> Result<(), String> { + >::get(&stash).map_or(Ok(()), |_| Err("stash already bonded"))?; + >::get(&controller).map_or(Ok(()), |_| Err("controller already bonded"))?; + + >::insert(stash, controller); + >::insert(controller, StakingLedger::::default_from(stash)); + + Ok(()) +} + #[macro_export] macro_rules! assert_session_era { ($session:expr, $era:expr) => { diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 3d9b1157ca87a..ad2de1d59315a 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -27,8 +27,8 @@ use frame_support::{ dispatch::WithPostDispatchInfo, pallet_prelude::*, traits::{ - Currency, Defensive, DefensiveResult, EstimateNextNewSession, Get, Imbalance, - LockableCurrency, OnUnbalanced, TryCollect, UnixTime, WithdrawReasons, + Currency, Defensive, DefensiveResult, EstimateNextNewSession, Get, Imbalance, OnUnbalanced, + TryCollect, UnixTime, }, weights::Weight, }; @@ -41,7 +41,9 @@ use sp_runtime::{ use sp_staking::{ currency_to_vote::CurrencyToVote, offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}, - EraIndex, SessionIndex, Stake, StakingInterface, + EraIndex, SessionIndex, Stake, + StakingAccount::{self, Controller, Stash}, + StakingInterface, }; use sp_std::prelude::*; @@ -52,7 +54,7 @@ use crate::{ SessionInterface, StakingLedger, ValidatorPrefs, }; -use super::{pallet::*, STAKING_ID}; +use super::pallet::*; #[cfg(feature = "try-runtime")] use frame_support::ensure; @@ -68,10 +70,24 @@ use sp_runtime::TryRuntimeError; const NPOS_MAX_ITERATIONS_COEFFICIENT: u32 = 2; impl Pallet { + /// Fetches the ledger associated with a controller or stash account, if any. + pub fn ledger(account: StakingAccount) -> Result, Error> { + StakingLedger::::get(account) + } + + pub fn payee(account: StakingAccount) -> RewardDestination { + StakingLedger::::reward_destination(account) + } + + /// Fetches the controller bonded to a stash account, if any. + pub fn bonded(stash: &T::AccountId) -> Option { + StakingLedger::::paired_account(Stash(stash.clone())) + } + /// The total balance that can be slashed from a stash account as of right now. pub fn slashable_balance_of(stash: &T::AccountId) -> BalanceOf { // Weight note: consider making the stake accessible through stash. - Self::bonded(stash).and_then(Self::ledger).map(|l| l.active).unwrap_or_default() + Self::ledger(Stash(stash.clone())).map(|l| l.active).unwrap_or_default() } /// Internal impl of [`Self::slashable_balance_of`] that returns [`VoteWeight`]. @@ -105,25 +121,24 @@ impl Pallet { controller: &T::AccountId, num_slashing_spans: u32, ) -> Result { - let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let mut ledger = Self::ledger(Controller(controller.clone()))?; let (stash, old_total) = (ledger.stash.clone(), ledger.total); if let Some(current_era) = Self::current_era() { ledger = ledger.consolidate_unlocked(current_era) } + let new_total = ledger.total; let used_weight = if ledger.unlocking.is_empty() && ledger.active < T::Currency::minimum_balance() { // This account must have called `unbond()` with some value that caused the active // portion to fall below existential deposit + will have no more unlocking chunks // left. We can now safely remove all staking-related information. - Self::kill_stash(&stash, num_slashing_spans)?; - // Remove the lock. - T::Currency::remove_lock(STAKING_ID, &stash); + Self::kill_stash(&ledger.stash, num_slashing_spans)?; T::WeightInfo::withdraw_unbonded_kill(num_slashing_spans) } else { // This was the consequence of a partial unbond. just update the ledger and move on. - Self::update_ledger(&controller, &ledger); + ledger.update()?; // This is only an update, so we use less overall weight. T::WeightInfo::withdraw_unbonded_update(num_slashing_spans) @@ -131,9 +146,9 @@ impl Pallet { // `old_total` should never be less than the new total because // `consolidate_unlocked` strictly subtracts balance. - if ledger.total < old_total { + if new_total < old_total { // Already checked that this won't overflow by entry condition. - let value = old_total - ledger.total; + let value = old_total - new_total; Self::deposit_event(Event::::Withdrawn { stash, amount: value }); } @@ -163,10 +178,15 @@ impl Pallet { .with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) })?; - let controller = Self::bonded(&validator_stash).ok_or_else(|| { - Error::::NotStash.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)) + let account = StakingAccount::Stash(validator_stash.clone()); + let mut ledger = Self::ledger(account.clone()).or_else(|_| { + if StakingLedger::::is_bonded(account) { + Err(Error::::NotController.into()) + } else { + Err(Error::::NotStash.with_weight(T::WeightInfo::payout_stakers_alive_staked(0))) + } })?; - let mut ledger = >::get(&controller).ok_or(Error::::NotController)?; + let stash = ledger.stash.clone(); ledger .claimed_rewards @@ -185,11 +205,11 @@ impl Pallet { .defensive_map_err(|_| Error::::BoundNotMet)?, } - let exposure = >::get(&era, &ledger.stash); + let exposure = >::get(&era, &stash); // Input data seems good, no errors allowed after this point - >::insert(&controller, &ledger); + ledger.update()?; // Get Era reward points. It has TOTAL and INDIVIDUAL // Find the fraction of the era reward that belongs to the validator @@ -200,11 +220,8 @@ impl Pallet { let era_reward_points = >::get(&era); let total_reward_points = era_reward_points.total; - let validator_reward_points = era_reward_points - .individual - .get(&ledger.stash) - .copied() - .unwrap_or_else(Zero::zero); + let validator_reward_points = + era_reward_points.individual.get(&stash).copied().unwrap_or_else(Zero::zero); // Nothing to do if they have no reward points. if validator_reward_points.is_zero() { @@ -231,19 +248,15 @@ impl Pallet { Self::deposit_event(Event::::PayoutStarted { era_index: era, - validator_stash: ledger.stash.clone(), + validator_stash: stash.clone(), }); let mut total_imbalance = PositiveImbalanceOf::::zero(); // We can now make total validator payout: if let Some((imbalance, dest)) = - Self::make_payout(&ledger.stash, validator_staking_payout + validator_commission_payout) + Self::make_payout(&stash, validator_staking_payout + validator_commission_payout) { - Self::deposit_event(Event::::Rewarded { - stash: ledger.stash, - dest, - amount: imbalance.peek(), - }); + Self::deposit_event(Event::::Rewarded { stash, dest, amount: imbalance.peek() }); total_imbalance.subsume(imbalance); } @@ -278,14 +291,6 @@ impl Pallet { Ok(Some(T::WeightInfo::payout_stakers_alive_staked(nominator_payout_count)).into()) } - /// Update the ledger for a controller. - /// - /// This will also update the stash lock. - pub(crate) fn update_ledger(controller: &T::AccountId, ledger: &StakingLedger) { - T::Currency::set_lock(STAKING_ID, &ledger.stash, ledger.total, WithdrawReasons::all()); - >::insert(controller, ledger); - } - /// Chill a stash account. pub(crate) fn chill_stash(stash: &T::AccountId) { let chilled_as_validator = Self::do_remove_validator(stash); @@ -301,24 +306,30 @@ impl Pallet { stash: &T::AccountId, amount: BalanceOf, ) -> Option<(PositiveImbalanceOf, RewardDestination)> { - let maybe_imbalance = match Self::payee(stash) { + 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::bonded(stash) - .and_then(|c| Self::ledger(&c).map(|l| (c, l))) - .and_then(|(controller, mut l)| { - l.active += amount; - l.total += amount; + RewardDestination::Staked => 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(); - Self::update_ledger(&controller, &l); - r - }), + + let _ = ledger + .update() + .defensive_proof("ledger fetched from storage, so it exists; qed."); + + Ok(r) + }) + .unwrap_or_default(), RewardDestination::Account(dest_account) => Some(T::Currency::deposit_creating(&dest_account, amount)), RewardDestination::None => None, }; - maybe_imbalance.map(|imbalance| (imbalance, Self::payee(stash))) + maybe_imbalance + .map(|imbalance| (imbalance, Self::payee(StakingAccount::Stash(stash.clone())))) } /// Plan a new session potentially trigger a new era. @@ -407,7 +418,6 @@ impl Pallet { } /// Start a new era. It does: - /// /// * Increment `active_era.index`, /// * reset `active_era.start`, /// * update `BondedEras` and apply slashes. @@ -666,18 +676,16 @@ impl Pallet { /// - after a `withdraw_unbonded()` call that frees all of a stash's bonded balance. /// - through `reap_stash()` if the balance has fallen to zero (through slashing). pub(crate) fn kill_stash(stash: &T::AccountId, num_slashing_spans: u32) -> DispatchResult { - let controller = >::get(stash).ok_or(Error::::NotStash)?; - - slashing::clear_stash_metadata::(stash, num_slashing_spans)?; + slashing::clear_stash_metadata::(&stash, num_slashing_spans)?; - >::remove(stash); - >::remove(&controller); + // removes controller from `Bonded` and staking ledger from `Ledger`, as well as reward + // setting of the stash in `Payee`. + StakingLedger::::kill(&stash)?; - >::remove(stash); - Self::do_remove_validator(stash); - Self::do_remove_nominator(stash); + Self::do_remove_validator(&stash); + Self::do_remove_nominator(&stash); - frame_system::Pallet::::dec_consumers(stash); + frame_system::Pallet::::dec_consumers(&stash); Ok(()) } @@ -1123,13 +1131,7 @@ impl ElectionDataProvider for Pallet { >::insert(voter.clone(), voter.clone()); >::insert( voter.clone(), - StakingLedger { - stash: voter.clone(), - active: stake, - total: stake, - unlocking: Default::default(), - claimed_rewards: Default::default(), - }, + StakingLedger::::new(voter.clone(), stake, Default::default()), ); Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false }); @@ -1141,13 +1143,7 @@ impl ElectionDataProvider for Pallet { >::insert(target.clone(), target.clone()); >::insert( target.clone(), - StakingLedger { - stash: target.clone(), - active: stake, - total: stake, - unlocking: Default::default(), - claimed_rewards: Default::default(), - }, + StakingLedger::::new(target.clone(), stake, Default::default()), ); Self::do_add_validator( &target, @@ -1182,13 +1178,7 @@ impl ElectionDataProvider for Pallet { >::insert(v.clone(), v.clone()); >::insert( v.clone(), - StakingLedger { - stash: v.clone(), - active: stake, - total: stake, - unlocking: Default::default(), - claimed_rewards: Default::default(), - }, + StakingLedger::::new(v.clone(), stake, Default::default()), ); Self::do_add_validator( &v, @@ -1203,13 +1193,7 @@ impl ElectionDataProvider for Pallet { >::insert(v.clone(), v.clone()); >::insert( v.clone(), - StakingLedger { - stash: v.clone(), - active: stake, - total: stake, - unlocking: Default::default(), - claimed_rewards: Default::default(), - }, + StakingLedger::::new(v.clone(), stake, Default::default()), ); Self::do_add_nominator( &v, @@ -1459,9 +1443,9 @@ impl ScoreProvider for Pallet { // this will clearly results in an inconsistent state, but it should not matter for a // benchmark. let active: BalanceOf = weight.try_into().map_err(|_| ()).unwrap(); - let mut ledger = match Self::ledger(who) { - None => StakingLedger::default_from(who.clone()), - Some(l) => l, + let mut ledger = match Self::ledger(StakingAccount::Stash(who.clone())) { + Ok(l) => l, + Err(_) => StakingLedger::default_from(who.clone()), }; ledger.active = active; @@ -1652,9 +1636,9 @@ impl StakingInterface for Pallet { } fn stash_by_ctrl(controller: &Self::AccountId) -> Result { - Self::ledger(controller) + Self::ledger(Controller(controller.clone())) .map(|l| l.stash) - .ok_or(Error::::NotController.into()) + .map_err(|e| e.into()) } fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool { @@ -1672,10 +1656,9 @@ impl StakingInterface for Pallet { } fn stake(who: &Self::AccountId) -> Result>, DispatchError> { - Self::bonded(who) - .and_then(|c| Self::ledger(c)) + Self::ledger(Stash(who.clone())) .map(|l| Stake { total: l.total, active: l.active }) - .ok_or(Error::::NotStash.into()) + .map_err(|e| e.into()) } fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult { @@ -1700,7 +1683,7 @@ impl StakingInterface for Pallet { who: Self::AccountId, num_slashing_spans: u32, ) -> Result { - let ctrl = Self::bonded(who).ok_or(Error::::NotStash)?; + let ctrl = Self::bonded(&who).ok_or(Error::::NotStash)?; Self::withdraw_unbonded(RawOrigin::Signed(ctrl.clone()).into(), num_slashing_spans) .map(|_| !Ledger::::contains_key(&ctrl)) .map_err(|with_post| with_post.error) @@ -1727,8 +1710,7 @@ impl StakingInterface for Pallet { fn status( who: &Self::AccountId, ) -> Result, DispatchError> { - let is_bonded = Self::bonded(who).is_some(); - if !is_bonded { + if !StakingLedger::::is_bonded(StakingAccount::Stash(who.clone())) { return Err(Error::::NotStash.into()) } @@ -1882,7 +1864,8 @@ impl Pallet { fn ensure_ledger_consistent(ctrl: T::AccountId) -> Result<(), TryRuntimeError> { // ensures ledger.total == ledger.active + sum(ledger.unlocking). - let ledger = Self::ledger(ctrl.clone()).ok_or("Not a controller.")?; + let ledger = Self::ledger(StakingAccount::Controller(ctrl.clone()))?; + let real_total: BalanceOf = ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value); ensure!(real_total == ledger.total, "ledger.total corrupt"); diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 0bcf932d90b44..f084299be8e13 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -25,8 +25,7 @@ use frame_support::{ pallet_prelude::*, traits::{ Currency, Defensive, DefensiveResult, DefensiveSaturating, EnsureOrigin, - EstimateNextNewSession, Get, LockIdentifier, LockableCurrency, OnUnbalanced, TryCollect, - UnixTime, + EstimateNextNewSession, Get, LockableCurrency, OnUnbalanced, TryCollect, UnixTime, }, weights::Weight, BoundedVec, @@ -36,7 +35,10 @@ use sp_runtime::{ traits::{CheckedSub, SaturatedConversion, StaticLookup, Zero}, ArithmeticError, Perbill, Percent, }; -use sp_staking::{EraIndex, SessionIndex}; +use sp_staking::{ + EraIndex, SessionIndex, + StakingAccount::{self, Controller, Stash}, +}; use sp_std::prelude::*; mod impls; @@ -50,7 +52,6 @@ use crate::{ UnappliedSlash, UnlockChunk, ValidatorPrefs, }; -const STAKING_ID: LockIdentifier = *b"staking "; // The speculative number of spans are used as an input of the weight annotation of // [`Call::unbond`], as the post dipatch weight may depend on the number of slashing span on the // account which is not provided as an input. The value set should be conservative but sensible. @@ -295,7 +296,6 @@ pub mod pallet { /// /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. #[pallet::storage] - #[pallet::getter(fn bonded)] pub type Bonded = StorageMap<_, Twox64Concat, T::AccountId, T::AccountId>; /// The minimum active bond to become and maintain the role of a nominator. @@ -317,15 +317,16 @@ pub mod pallet { pub type MinCommission = StorageValue<_, Perbill, ValueQuery>; /// Map from all (unlocked) "controller" accounts to the info regarding the staking. + /// + /// Note: All the reads and mutations to this storage *MUST* be done through the methods exposed + /// by [`StakingLedger`] to ensure data and lock consistency. #[pallet::storage] - #[pallet::getter(fn ledger)] pub type Ledger = StorageMap<_, Blake2_128Concat, T::AccountId, StakingLedger>; /// Where the reward payment should be made. Keyed by stash. /// /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. #[pallet::storage] - #[pallet::getter(fn payee)] pub type Payee = StorageMap<_, Twox64Concat, T::AccountId, RewardDestination, ValueQuery>; @@ -841,16 +842,11 @@ pub mod pallet { payee: RewardDestination, ) -> DispatchResult { let stash = ensure_signed(origin)?; - let controller_to_be_deprecated = stash.clone(); - if >::contains_key(&stash) { + if StakingLedger::::is_bonded(StakingAccount::Stash(stash.clone())) { return Err(Error::::AlreadyBonded.into()) } - if >::contains_key(&controller_to_be_deprecated) { - return Err(Error::::AlreadyPaired.into()) - } - // Reject a bond which is considered to be _dust_. if value < T::Currency::minimum_balance() { return Err(Error::::InsufficientBond.into()) @@ -858,11 +854,6 @@ pub mod pallet { frame_system::Pallet::::inc_consumers(&stash).map_err(|_| Error::::BadState)?; - // You're auto-bonded forever, here. We might improve this by only bonding when - // you actually validate/nominate and remove once you unbond __everything__. - >::insert(&stash, &stash); - >::insert(&stash, 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); @@ -870,19 +861,21 @@ pub mod pallet { let stash_balance = T::Currency::free_balance(&stash); let value = value.min(stash_balance); Self::deposit_event(Event::::Bonded { stash: stash.clone(), amount: value }); - let item = StakingLedger { - stash: stash.clone(), - total: value, - active: value, - unlocking: Default::default(), - claimed_rewards: (last_reward_era..current_era) + let ledger = StakingLedger::::new( + stash.clone(), + value, + (last_reward_era..current_era) .try_collect() // Since last_reward_era is calculated as `current_era - // HistoryDepth`, following bound is always expected to be // satisfied. .defensive_map_err(|_| Error::::BoundNotMet)?, - }; - Self::update_ledger(&controller_to_be_deprecated, &item); + ); + + // 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)?; + Ok(()) } @@ -908,8 +901,7 @@ pub mod pallet { ) -> DispatchResult { let stash = ensure_signed(origin)?; - let controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; - let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let mut ledger = Self::ledger(StakingAccount::Stash(stash.clone()))?; let stash_balance = T::Currency::free_balance(&stash); if let Some(extra) = stash_balance.checked_sub(&ledger.total) { @@ -923,11 +915,10 @@ pub mod pallet { ); // NOTE: ledger must be updated prior to calling `Self::weight_of`. - Self::update_ledger(&controller, &ledger); + ledger.update()?; // update this staker in the sorted list, if they exist in it. if T::VoterList::contains(&stash) { - let _ = - T::VoterList::on_update(&stash, Self::weight_of(&ledger.stash)).defensive(); + let _ = T::VoterList::on_update(&stash, Self::weight_of(&stash)).defensive(); } Self::deposit_event(Event::::Bonded { stash, amount: extra }); @@ -963,9 +954,8 @@ pub mod pallet { #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let controller = ensure_signed(origin)?; - let unlocking = Self::ledger(&controller) - .map(|l| l.unlocking.len()) - .ok_or(Error::::NotController)?; + let unlocking = + Self::ledger(Controller(controller.clone())).map(|l| l.unlocking.len())?; // if there are no unlocking chunks available, try to withdraw chunks older than // `BondingDuration` to proceed with the unbonding. @@ -981,8 +971,9 @@ pub mod pallet { // we need to fetch the ledger again because it may have been mutated in the call // to `Self::do_withdraw_unbonded` above. - let mut ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let mut ledger = Self::ledger(Controller(controller))?; let mut value = value.min(ledger.active); + let stash = ledger.stash.clone(); ensure!( ledger.unlocking.len() < T::MaxUnlockingChunks::get() as usize, @@ -998,9 +989,9 @@ pub mod pallet { ledger.active = Zero::zero(); } - let min_active_bond = if Nominators::::contains_key(&ledger.stash) { + let min_active_bond = if Nominators::::contains_key(&stash) { MinNominatorBond::::get() - } else if Validators::::contains_key(&ledger.stash) { + } else if Validators::::contains_key(&stash) { MinValidatorBond::::get() } else { Zero::zero() @@ -1024,15 +1015,14 @@ pub mod pallet { .map_err(|_| Error::::NoMoreChunks)?; }; // NOTE: ledger must be updated prior to calling `Self::weight_of`. - Self::update_ledger(&controller, &ledger); + ledger.update()?; // update this staker in the sorted list, if they exist in it. - if T::VoterList::contains(&ledger.stash) { - let _ = T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)) - .defensive(); + if T::VoterList::contains(&stash) { + let _ = T::VoterList::on_update(&stash, Self::weight_of(&stash)).defensive(); } - Self::deposit_event(Event::::Unbonded { stash: ledger.stash, amount: value }); + Self::deposit_event(Event::::Unbonded { stash, amount: value }); } let actual_weight = if let Some(withdraw_weight) = maybe_withdraw_weight { @@ -1089,7 +1079,7 @@ pub mod pallet { pub fn validate(origin: OriginFor, prefs: ValidatorPrefs) -> DispatchResult { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let ledger = Self::ledger(Controller(controller))?; ensure!(ledger.active >= MinValidatorBond::::get(), Error::::InsufficientBond); let stash = &ledger.stash; @@ -1135,7 +1125,8 @@ pub mod pallet { ) -> DispatchResult { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?; + ensure!(ledger.active >= MinNominatorBond::::get(), Error::::InsufficientBond); let stash = &ledger.stash; @@ -1202,7 +1193,9 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor) -> DispatchResult { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + + let ledger = Self::ledger(StakingAccount::Controller(controller))?; + Self::chill_stash(&ledger.stash); Ok(()) } @@ -1226,9 +1219,11 @@ pub mod pallet { payee: RewardDestination, ) -> DispatchResult { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; - let stash = &ledger.stash; - >::insert(stash, payee); + let ledger = Self::ledger(Controller(controller))?; + let _ = ledger + .set_payee(payee) + .defensive_proof("ledger was retrieved from storage, thus its bonded; qed."); + Ok(()) } @@ -1250,18 +1245,24 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::set_controller())] pub fn set_controller(origin: OriginFor) -> DispatchResult { let stash = ensure_signed(origin)?; - let old_controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; - if >::contains_key(&stash) { - return Err(Error::::AlreadyPaired.into()) - } - if old_controller != stash { - >::insert(&stash, &stash); - if let Some(l) = >::take(&old_controller) { - >::insert(&stash, l); + // the bonded map and ledger are mutated directly as this extrinsic is related to a + // (temporary) passive migration. + Self::ledger(StakingAccount::Stash(stash.clone())).map(|ledger| { + let controller = ledger.controller() + .defensive_proof("ledger was fetched used the StakingInterface, so controller field must exist; qed.") + .ok_or(Error::::NotController)?; + + if controller == stash { + // stash is already its own controller. + return Err(Error::::AlreadyPaired.into()) } - } - Ok(()) + // update bond and ledger. + >::remove(controller); + >::insert(&stash, &stash); + >::insert(&stash, ledger); + Ok(()) + })? } /// Sets the ideal number of validators. @@ -1409,11 +1410,9 @@ pub mod pallet { ) -> DispatchResult { ensure_root(origin)?; - // Remove all staking-related information. + // Remove all staking-related information and lock. Self::kill_stash(&stash, num_slashing_spans)?; - // Remove the lock. - T::Currency::remove_lock(STAKING_ID, &stash); Ok(()) } @@ -1502,7 +1501,7 @@ pub mod pallet { #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let ledger = Self::ledger(Controller(controller))?; ensure!(!ledger.unlocking.is_empty(), Error::::NoUnlockChunk); let initial_unlocking = ledger.unlocking.len() as u32; @@ -1515,16 +1514,18 @@ pub mod pallet { amount: rebonded_value, }); + let stash = ledger.stash.clone(); + let final_unlocking = ledger.unlocking.len(); + // NOTE: ledger must be updated prior to calling `Self::weight_of`. - Self::update_ledger(&controller, &ledger); - if T::VoterList::contains(&ledger.stash) { - let _ = T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)) - .defensive(); + ledger.update()?; + if T::VoterList::contains(&stash) { + let _ = T::VoterList::on_update(&stash, Self::weight_of(&stash)).defensive(); } let removed_chunks = 1u32 // for the case where the last iterated chunk is not removed .saturating_add(initial_unlocking) - .saturating_sub(ledger.unlocking.len() as u32); + .saturating_sub(final_unlocking as u32); Ok(Some(T::WeightInfo::rebond(removed_chunks)).into()) } @@ -1556,13 +1557,11 @@ pub mod pallet { let ed = T::Currency::minimum_balance(); let reapable = T::Currency::total_balance(&stash) < ed || - Self::ledger(Self::bonded(stash.clone()).ok_or(Error::::NotStash)?) - .map(|l| l.total) - .unwrap_or_default() < ed; + Self::ledger(Stash(stash.clone())).map(|l| l.total).unwrap_or_default() < ed; ensure!(reapable, Error::::FundedTarget); + // Remove all staking-related information and lock. Self::kill_stash(&stash, num_slashing_spans)?; - T::Currency::remove_lock(STAKING_ID, &stash); Ok(Pays::No.into()) } @@ -1582,7 +1581,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::kick(who.len() as u32))] pub fn kick(origin: OriginFor, who: Vec>) -> DispatchResult { let controller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let ledger = Self::ledger(Controller(controller))?; let stash = &ledger.stash; for nom_stash in who @@ -1691,7 +1690,7 @@ pub mod pallet { pub fn chill_other(origin: OriginFor, controller: T::AccountId) -> DispatchResult { // Anyone can call this function. let caller = ensure_signed(origin)?; - let ledger = Self::ledger(&controller).ok_or(Error::::NotController)?; + let ledger = Self::ledger(Controller(controller.clone()))?; let stash = ledger.stash; // In order for one user to chill another user, the following conditions must be met: diff --git a/substrate/frame/staking/src/slashing.rs b/substrate/frame/staking/src/slashing.rs index bb02da73f6e5d..0d84d503733e6 100644 --- a/substrate/frame/staking/src/slashing.rs +++ b/substrate/frame/staking/src/slashing.rs @@ -597,15 +597,11 @@ pub fn do_slash( slashed_imbalance: &mut NegativeImbalanceOf, slash_era: EraIndex, ) { - let controller = match >::bonded(stash).defensive() { - None => return, - Some(c) => c, - }; - - let mut ledger = match >::ledger(&controller) { - Some(ledger) => ledger, - None => return, // nothing to do. - }; + let mut ledger = + match Pallet::::ledger(sp_staking::StakingAccount::Stash(stash.clone())).defensive() { + Ok(ledger) => ledger, + Err(_) => return, // nothing to do. + }; let value = ledger.slash(value, T::Currency::minimum_balance(), slash_era); @@ -618,7 +614,9 @@ pub fn do_slash( *reward_payout = reward_payout.saturating_sub(missing); } - >::update_ledger(&controller, &ledger); + let _ = ledger + .update() + .defensive_proof("ledger fetched from storage so it exists in storage; qed."); // trigger the event >::deposit_event(super::Event::::Slashed { diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 78183cfde9296..cb620f89f12c0 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -18,6 +18,7 @@ //! Tests for the module. use super::{ConfigOp, Event, *}; +use crate::ledger::StakingLedgerInspect; use frame_election_provider_support::{ bounds::{DataProviderBounds, ElectionBoundsBuilder}, ElectionProvider, SortedListProvider, Support, @@ -33,7 +34,7 @@ use pallet_balances::Error as BalancesError; use sp_runtime::{ assert_eq_error_rate, bounded_vec, traits::{BadOrigin, Dispatchable}, - Perbill, Percent, Rounding, TokenError, + Perbill, Percent, Perquintill, Rounding, TokenError, }; use sp_staking::{ offence::{DisableStrategy, OffenceDetails, OnOffenceHandler}, @@ -151,8 +152,8 @@ fn basic_setup_works() { // Account 11 controls its own stash, which is 100 * balance_factor units assert_eq!( - Staking::ledger(&11).unwrap(), - StakingLedger { + Ledger::get(&11).unwrap(), + StakingLedgerInspect:: { stash: 11, total: 1000, active: 1000, @@ -162,17 +163,17 @@ fn basic_setup_works() { ); // Account 21 controls its own stash, which is 200 * balance_factor units assert_eq!( - Staking::ledger(&21), - Some(StakingLedger { + Ledger::get(&21).unwrap(), + StakingLedgerInspect:: { stash: 21, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Account 1 does not control any stash - assert_eq!(Staking::ledger(&1), None); + assert!(Staking::ledger(1.into()).is_err()); // ValidatorPrefs are default assert_eq_uvec!( @@ -185,14 +186,14 @@ fn basic_setup_works() { ); assert_eq!( - Staking::ledger(101), - Some(StakingLedger { + Staking::ledger(101.into()).unwrap(), + StakingLedgerInspect { stash: 101, total: 500, active: 500, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); @@ -265,7 +266,7 @@ fn change_controller_works() { #[test] fn change_controller_already_paired_once_stash() { ExtBuilder::default().build_and_execute(|| { - // 10 and 11 are bonded as controller and stash respectively. + // 11 and 11 are bonded as controller and stash respectively. assert_eq!(Staking::bonded(&11), Some(11)); // 11 is initially a validator. @@ -460,14 +461,14 @@ fn staking_should_work() { // Note: the stashed value of 4 is still lock assert_eq!( - Staking::ledger(&3), - Some(StakingLedger { + Staking::ledger(3.into()).unwrap(), + StakingLedgerInspect { stash: 3, total: 1500, active: 1500, unlocking: Default::default(), claimed_rewards: bounded_vec![0], - }) + } ); // e.g. it cannot reserve more than 500 that it has free from the total 2000 assert_noop!(Balances::reserve(&3, 501), BalancesError::::LiquidityRestrictions); @@ -717,9 +718,9 @@ fn nominators_also_get_slashed_pro_rata() { assert_eq!(initial_exposure.others.first().unwrap().who, 101); // staked values; - let nominator_stake = Staking::ledger(101).unwrap().active; + let nominator_stake = Staking::ledger(101.into()).unwrap().active; let nominator_balance = balances(&101).0; - let validator_stake = Staking::ledger(11).unwrap().active; + let validator_stake = Staking::ledger(11.into()).unwrap().active; let validator_balance = balances(&11).0; let exposed_stake = initial_exposure.total; let exposed_validator = initial_exposure.own; @@ -732,8 +733,8 @@ fn nominators_also_get_slashed_pro_rata() { ); // both stakes must have been decreased. - assert!(Staking::ledger(101).unwrap().active < nominator_stake); - assert!(Staking::ledger(11).unwrap().active < validator_stake); + assert!(Staking::ledger(101.into()).unwrap().active < nominator_stake); + assert!(Staking::ledger(11.into()).unwrap().active < validator_stake); let slash_amount = slash_percent * exposed_stake; let validator_share = @@ -746,8 +747,8 @@ fn nominators_also_get_slashed_pro_rata() { assert!(nominator_share > 0); // both stakes must have been decreased pro-rata. - assert_eq!(Staking::ledger(101).unwrap().active, nominator_stake - nominator_share); - assert_eq!(Staking::ledger(11).unwrap().active, validator_stake - validator_share); + assert_eq!(Staking::ledger(101.into()).unwrap().active, nominator_stake - nominator_share); + assert_eq!(Staking::ledger(11.into()).unwrap().active, validator_stake - validator_share); assert_eq!( balances(&101).0, // free balance nominator_balance - nominator_share, @@ -1047,14 +1048,14 @@ fn reward_destination_works() { assert_eq!(Balances::free_balance(11), 1000); // Check how much is at stake assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Compute total payout now for whole duration as other parameter won't change @@ -1065,19 +1066,19 @@ fn reward_destination_works() { mock::make_all_reward_payment(0); // Check that RewardDestination is Staked (default) - assert_eq!(Staking::payee(&11), RewardDestination::Staked); + assert_eq!(Staking::payee(11.into()), RewardDestination::Staked); // 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 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + total_payout_0, active: 1000 + total_payout_0, unlocking: Default::default(), claimed_rewards: bounded_vec![0], - }) + } ); // Change RewardDestination to Stash @@ -1091,19 +1092,19 @@ fn reward_destination_works() { mock::make_all_reward_payment(1); // Check that RewardDestination is Stash - assert_eq!(Staking::payee(&11), RewardDestination::Stash); + assert_eq!(Staking::payee(11.into()), RewardDestination::Stash); // 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 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + total_payout_0, active: 1000 + total_payout_0, unlocking: Default::default(), claimed_rewards: bounded_vec![0, 1], - }) + } ); // Change RewardDestination to Controller @@ -1120,19 +1121,19 @@ fn reward_destination_works() { mock::make_all_reward_payment(2); // Check that RewardDestination is Controller - assert_eq!(Staking::payee(&11), RewardDestination::Controller); + assert_eq!(Staking::payee(11.into()), RewardDestination::Controller); // 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 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + total_payout_0, active: 1000 + total_payout_0, unlocking: Default::default(), claimed_rewards: bounded_vec![0, 1, 2], - }) + } ); }); } @@ -1185,14 +1186,14 @@ fn bond_extra_works() { assert_eq!(Staking::bonded(&11), Some(11)); // Check how much is at stake assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Give account 11 some large free balance greater than total @@ -1202,28 +1203,28 @@ fn bond_extra_works() { assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(11), 100)); // There should be 100 more `total` and `active` in the ledger assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 1000 + 100, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Call the bond_extra function with a large number, should handle it assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(11), Balance::max_value())); // The full amount of the funds should now be in the total and active assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000000, active: 1000000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); }); } @@ -1254,14 +1255,14 @@ fn bond_extra_and_withdraw_unbonded_works() { // Initial state of 11 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); assert_eq!( Staking::eras_stakers(active_era(), 11), @@ -1272,14 +1273,14 @@ fn bond_extra_and_withdraw_unbonded_works() { Staking::bond_extra(RuntimeOrigin::signed(11), 100).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 1000 + 100, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Exposure is a snapshot! only updated after the next era update. assert_ne!( @@ -1293,14 +1294,14 @@ fn bond_extra_and_withdraw_unbonded_works() { // ledger should be the same. assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 1000 + 100, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Exposure is now updated. assert_eq!( @@ -1311,27 +1312,27 @@ fn bond_extra_and_withdraw_unbonded_works() { // Unbond almost all of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 1000).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 100, unlocking: bounded_vec![UnlockChunk { value: 1000, era: 2 + 3 }], claimed_rewards: bounded_vec![], - }), + }, ); // Attempting to free the balances now will fail. 2 eras need to pass. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 100, unlocking: bounded_vec![UnlockChunk { value: 1000, era: 2 + 3 }], claimed_rewards: bounded_vec![], - }), + }, ); // trigger next era. @@ -1340,14 +1341,14 @@ fn bond_extra_and_withdraw_unbonded_works() { // nothing yet assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000 + 100, active: 100, unlocking: bounded_vec![UnlockChunk { value: 1000, era: 2 + 3 }], claimed_rewards: bounded_vec![], - }), + }, ); // trigger next era. @@ -1356,14 +1357,14 @@ fn bond_extra_and_withdraw_unbonded_works() { assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); // Now the value is free and the staking ledger is updated. assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 100, active: 100, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }), + }, ); }) } @@ -1390,7 +1391,7 @@ fn many_unbond_calls_should_work() { // `BondingDuration` == 3). assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); assert_eq!( - Staking::ledger(&11).map(|l| l.unlocking.len()).unwrap(), + Staking::ledger(11.into()).map(|l| l.unlocking.len()).unwrap(), <::MaxUnlockingChunks as Get>::get() as usize ); @@ -1405,7 +1406,7 @@ fn many_unbond_calls_should_work() { // only slots within last `BondingDuration` are filled. assert_eq!( - Staking::ledger(&11).map(|l| l.unlocking.len()).unwrap(), + Staking::ledger(11.into()).map(|l| l.unlocking.len()).unwrap(), <::BondingDuration>::get() as usize ); }) @@ -1459,14 +1460,14 @@ fn rebond_works() { // Initial state of 11 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(2); @@ -1478,66 +1479,66 @@ fn rebond_works() { // Unbond almost all of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 100, unlocking: bounded_vec![UnlockChunk { value: 900, era: 2 + 3 }], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond all the funds unbonded. Staking::rebond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Unbond almost all of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 100, unlocking: bounded_vec![UnlockChunk { value: 900, era: 5 }], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond part of the funds unbonded. Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 600, unlocking: bounded_vec![UnlockChunk { value: 400, era: 5 }], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond the remainder of the funds unbonded. Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Unbond parts of the funds in stash. @@ -1545,27 +1546,27 @@ fn rebond_works() { Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 100, unlocking: bounded_vec![UnlockChunk { value: 900, era: 5 }], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond part of the funds unbonded. Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 600, unlocking: bounded_vec![UnlockChunk { value: 400, era: 5 }], claimed_rewards: bounded_vec![], - }) + } ); }) } @@ -1585,14 +1586,14 @@ fn rebond_is_fifo() { // Initial state of 10 assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(2); @@ -1600,14 +1601,14 @@ fn rebond_is_fifo() { // Unbond some of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 400).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 600, unlocking: bounded_vec![UnlockChunk { value: 400, era: 2 + 3 }], claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(3); @@ -1615,8 +1616,8 @@ fn rebond_is_fifo() { // Unbond more of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 300, @@ -1625,7 +1626,7 @@ fn rebond_is_fifo() { UnlockChunk { value: 300, era: 3 + 3 }, ], claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(4); @@ -1633,8 +1634,8 @@ fn rebond_is_fifo() { // Unbond yet more of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 200).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 100, @@ -1644,14 +1645,14 @@ fn rebond_is_fifo() { UnlockChunk { value: 200, era: 4 + 3 }, ], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond half of the unbonding funds. Staking::rebond(RuntimeOrigin::signed(11), 400).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 500, @@ -1660,7 +1661,7 @@ fn rebond_is_fifo() { UnlockChunk { value: 100, era: 3 + 3 }, ], claimed_rewards: bounded_vec![], - }) + } ); }) } @@ -1682,27 +1683,27 @@ fn rebond_emits_right_value_in_event() { // Unbond almost all of the funds in stash. Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 100, unlocking: bounded_vec![UnlockChunk { value: 900, era: 1 + 3 }], claimed_rewards: bounded_vec![], - }) + } ); // Re-bond less than the total Staking::rebond(RuntimeOrigin::signed(11), 100).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 200, unlocking: bounded_vec![UnlockChunk { value: 800, era: 1 + 3 }], claimed_rewards: bounded_vec![], - }) + } ); // Event emitted should be correct assert_eq!(*staking_events().last().unwrap(), Event::Bonded { stash: 11, amount: 100 }); @@ -1710,14 +1711,14 @@ fn rebond_emits_right_value_in_event() { // Re-bond way more than available Staking::rebond(RuntimeOrigin::signed(11), 100_000).unwrap(); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); // Event emitted should be correct, only 800 assert_eq!(*staking_events().last().unwrap(), Event::Bonded { stash: 11, amount: 800 }); @@ -1747,7 +1748,7 @@ fn reward_to_stake_works() { ErasStakers::::insert(0, 21, Exposure { total: 69, own: 69, others: vec![] }); >::insert( &20, - StakingLedger { + StakingLedgerInspect { stash: 21, total: 69, active: 69, @@ -1806,16 +1807,7 @@ fn reap_stash_works() { // no easy way to cause an account to go below ED, we tweak their staking ledger // instead. - Ledger::::insert( - 11, - StakingLedger { - stash: 11, - total: 5, - active: 5, - unlocking: Default::default(), - claimed_rewards: bounded_vec![], - }, - ); + Ledger::::insert(11, StakingLedger::::new(11, 5, bounded_vec![])); // reap-able assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0)); @@ -1937,14 +1929,14 @@ fn bond_with_no_staked_value() { // unbonding even 1 will cause all to be unbonded. assert_ok!(Staking::unbond(RuntimeOrigin::signed(1), 1)); assert_eq!( - Staking::ledger(1), - Some(StakingLedger { + Staking::ledger(1.into()).unwrap(), + StakingLedgerInspect { stash: 1, active: 0, total: 5, unlocking: bounded_vec![UnlockChunk { value: 5, era: 3 }], claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(1); @@ -1952,14 +1944,14 @@ fn bond_with_no_staked_value() { // not yet removed. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(1), 0)); - assert!(Staking::ledger(1).is_some()); + assert!(Staking::ledger(1.into()).is_ok()); assert_eq!(Balances::locks(&1)[0].amount, 5); mock::start_active_era(3); // poof. Account 1 is removed from the staking system. assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(1), 0)); - assert!(Staking::ledger(1).is_none()); + assert!(Staking::ledger(1.into()).is_err()); assert_eq!(Balances::locks(&1).len(), 0); }); } @@ -2044,7 +2036,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { // ensure all have equal stake. assert_eq!( >::iter() - .map(|(v, _)| (v, Staking::ledger(v).unwrap().total)) + .map(|(v, _)| (v, Staking::ledger(v.into()).unwrap().total)) .collect::>(), vec![(31, 1000), (21, 1000), (11, 1000)], ); @@ -2096,7 +2088,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { // ensure all have equal stake. assert_eq!( >::iter() - .map(|(v, _)| (v, Staking::ledger(v).unwrap().total)) + .map(|(v, _)| (v, Staking::ledger(v.into()).unwrap().total)) .collect::>(), vec![(31, 1000), (21, 1000), (11, 1000)], ); @@ -2982,7 +2974,7 @@ fn retroactive_deferred_slashes_one_before() { mock::start_active_era(4); - assert_eq!(Staking::ledger(11).unwrap().total, 1000); + assert_eq!(Staking::ledger(11.into()).unwrap().total, 1000); // slash happens after the next line. mock::start_active_era(5); @@ -2997,9 +2989,9 @@ fn retroactive_deferred_slashes_one_before() { )); // their ledger has already been slashed. - assert_eq!(Staking::ledger(11).unwrap().total, 900); + assert_eq!(Staking::ledger(11.into()).unwrap().total, 900); assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1000)); - assert_eq!(Staking::ledger(11).unwrap().total, 900); + assert_eq!(Staking::ledger(11.into()).unwrap().total, 900); }) } @@ -3032,7 +3024,7 @@ fn staker_cannot_bail_deferred_slash() { assert_eq!( Ledger::::get(101).unwrap(), - StakingLedger { + StakingLedgerInspect { active: 0, total: 500, stash: 101, @@ -3782,14 +3774,14 @@ fn test_payout_stakers() { // We track rewards in `claimed_rewards` vec assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![1] - }) + } ); for i in 3..16 { @@ -3813,14 +3805,14 @@ fn test_payout_stakers() { // We track rewards in `claimed_rewards` vec assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: (1..=14).collect::>().try_into().unwrap() - }) + } ); let last_era = 99; @@ -3846,14 +3838,14 @@ fn test_payout_stakers() { expected_last_reward_era )); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![expected_start_reward_era, expected_last_reward_era] - }) + } ); // Out of order claims works. @@ -3861,8 +3853,8 @@ fn test_payout_stakers() { assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 23)); assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 42)); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, @@ -3874,7 +3866,7 @@ fn test_payout_stakers() { 69, expected_last_reward_era ] - }) + } ); }); } @@ -4073,26 +4065,26 @@ fn bond_during_era_correctly_populates_claimed_rewards() { // Era = None bond_validator(9, 1000); assert_eq!( - Staking::ledger(&9), - Some(StakingLedger { + Staking::ledger(9.into()).unwrap(), + StakingLedgerInspect { stash: 9, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], - }) + } ); mock::start_active_era(5); bond_validator(11, 1000); assert_eq!( - Staking::ledger(&11), - Some(StakingLedger { + Staking::ledger(11.into()).unwrap(), + StakingLedgerInspect { stash: 11, total: 1000, active: 1000, unlocking: Default::default(), claimed_rewards: (0..5).collect::>().try_into().unwrap(), - }) + } ); // make sure only era upto history depth is stored @@ -4101,8 +4093,8 @@ fn bond_during_era_correctly_populates_claimed_rewards() { mock::start_active_era(current_era); bond_validator(13, 1000); assert_eq!( - Staking::ledger(&13), - Some(StakingLedger { + Staking::ledger(13.into()).unwrap(), + StakingLedgerInspect { stash: 13, total: 1000, active: 1000, @@ -4111,7 +4103,7 @@ fn bond_during_era_correctly_populates_claimed_rewards() { .collect::>() .try_into() .unwrap(), - }) + } ); }); } @@ -4200,6 +4192,7 @@ fn payout_creates_controller() { false, ) .unwrap(); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![11])); // kill controller @@ -4356,8 +4349,8 @@ fn cannot_rebond_to_lower_than_ed() { .build_and_execute(|| { // initial stuff. assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: 11 * 1000, active: 11 * 1000, @@ -4370,8 +4363,8 @@ fn cannot_rebond_to_lower_than_ed() { assert_ok!(Staking::chill(RuntimeOrigin::signed(21))); assert_ok!(Staking::unbond(RuntimeOrigin::signed(21), 11 * 1000)); assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: 11 * 1000, active: 0, @@ -4396,8 +4389,8 @@ fn cannot_bond_extra_to_lower_than_ed() { .build_and_execute(|| { // initial stuff. assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: 11 * 1000, active: 11 * 1000, @@ -4410,8 +4403,8 @@ fn cannot_bond_extra_to_lower_than_ed() { assert_ok!(Staking::chill(RuntimeOrigin::signed(21))); assert_ok!(Staking::unbond(RuntimeOrigin::signed(21), 11 * 1000)); assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: 11 * 1000, active: 0, @@ -4437,8 +4430,8 @@ fn do_not_die_when_active_is_ed() { .build_and_execute(|| { // given assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: 1000 * ed, active: 1000 * ed, @@ -4454,8 +4447,8 @@ fn do_not_die_when_active_is_ed() { // then assert_eq!( - Staking::ledger(&21).unwrap(), - StakingLedger { + Staking::ledger(21.into()).unwrap(), + StakingLedgerInspect { stash: 21, total: ed, active: ed, @@ -5530,15 +5523,14 @@ fn force_apply_min_commission_works() { #[test] fn proportional_slash_stop_slashing_if_remaining_zero() { let c = |era, value| UnlockChunk:: { era, value }; + + // we have some chunks, but they are not affected. + let unlocking = bounded_vec![c(1, 10), c(2, 10)]; + // Given - let mut ledger = StakingLedger:: { - stash: 123, - total: 40, - active: 20, - // we have some chunks, but they are not affected. - unlocking: bounded_vec![c(1, 10), c(2, 10)], - claimed_rewards: bounded_vec![], - }; + let mut ledger = StakingLedger::::new(123, 20, bounded_vec![]); + ledger.total = 40; + ledger.unlocking = unlocking; assert_eq!(BondingDuration::get(), 3); @@ -5550,13 +5542,7 @@ fn proportional_slash_stop_slashing_if_remaining_zero() { fn proportional_ledger_slash_works() { let c = |era, value| UnlockChunk:: { era, value }; // Given - let mut ledger = StakingLedger:: { - stash: 123, - total: 10, - active: 10, - unlocking: bounded_vec![], - claimed_rewards: bounded_vec![], - }; + let mut ledger = StakingLedger::::new(123, 10, bounded_vec![]); assert_eq!(BondingDuration::get(), 3); // When we slash a ledger with no unlocking chunks @@ -5795,8 +5781,8 @@ fn pre_bonding_era_cannot_be_claimed() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&3).unwrap(), - StakingLedger { + Staking::ledger(3.into()).unwrap(), + StakingLedgerInspect { stash: 3, total: 1500, active: 1500, @@ -5823,7 +5809,7 @@ fn pre_bonding_era_cannot_be_claimed() { // decoding will fail now since Staking Ledger is in corrupt state HistoryDepth::set(history_depth - 1); - assert_eq!(Staking::ledger(&4), None); + assert!(Staking::ledger(4.into()).is_err()); // make sure stakers still cannot claim rewards that they are not meant to assert_noop!( @@ -5861,8 +5847,8 @@ fn reducing_history_depth_abrupt() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&3).unwrap(), - StakingLedger { + Staking::ledger(3.into()).unwrap(), + StakingLedgerInspect { stash: 3, total: 1500, active: 1500, @@ -5900,8 +5886,8 @@ fn reducing_history_depth_abrupt() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&5).unwrap(), - StakingLedger { + Staking::ledger(5.into()).unwrap(), + StakingLedgerInspect { stash: 5, total: 1200, active: 1200, @@ -5924,7 +5910,7 @@ fn reducing_max_unlocking_chunks_abrupt() { MaxUnlockingChunks::set(2); start_active_era(10); assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 300, RewardDestination::Staked)); - assert!(matches!(Staking::ledger(3), Some(_))); + assert!(matches!(Staking::ledger(3.into()), Ok(_))); // when staker unbonds assert_ok!(Staking::unbond(RuntimeOrigin::signed(3), 20)); @@ -5933,8 +5919,8 @@ fn reducing_max_unlocking_chunks_abrupt() { // => 10 + 3 = 13 let expected_unlocking: BoundedVec, MaxUnlockingChunks> = bounded_vec![UnlockChunk { value: 20 as Balance, era: 13 as EraIndex }]; - assert!(matches!(Staking::ledger(3), - Some(StakingLedger { + assert!(matches!(Staking::ledger(3.into()), + Ok(StakingLedger { unlocking, .. }) if unlocking==expected_unlocking)); @@ -5945,8 +5931,8 @@ fn reducing_max_unlocking_chunks_abrupt() { // then another unlock chunk is added let expected_unlocking: BoundedVec, MaxUnlockingChunks> = bounded_vec![UnlockChunk { value: 20, era: 13 }, UnlockChunk { value: 50, era: 14 }]; - assert!(matches!(Staking::ledger(3), - Some(StakingLedger { + assert!(matches!(Staking::ledger(3.into()), + Ok(StakingLedger { unlocking, .. }) if unlocking==expected_unlocking)); @@ -6134,3 +6120,123 @@ mod staking_interface { }) } } + +mod ledger { + use super::*; + + #[test] + fn paired_account_works() { + ExtBuilder::default().build_and_execute(|| { + assert_ok!(Staking::bond( + RuntimeOrigin::signed(10), + 100, + RewardDestination::Controller + )); + + assert_eq!(>::get(&10), Some(10)); + assert_eq!( + StakingLedger::::paired_account(StakingAccount::Controller(10)), + Some(10) + ); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(10)), Some(10)); + + assert_eq!(>::get(&42), None); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Controller(42)), None); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(42)), None); + + // bond manually stash with different controller. This is deprecated but the migration + // has not been complete yet (controller: 100, stash: 200) + assert_ok!(bond_controller_stash(100, 200)); + assert_eq!(>::get(&200), Some(100)); + assert_eq!( + StakingLedger::::paired_account(StakingAccount::Controller(100)), + Some(200) + ); + assert_eq!( + StakingLedger::::paired_account(StakingAccount::Stash(200)), + Some(100) + ); + }) + } + + #[test] + fn get_ledger_works() { + ExtBuilder::default().build_and_execute(|| { + // stash does not exist + assert!(StakingLedger::::get(StakingAccount::Stash(42)).is_err()); + + // bonded and paired + assert_eq!(>::get(&11), Some(11)); + + match StakingLedger::::get(StakingAccount::Stash(11)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(11)); + assert_eq!(ledger.stash, 11); + }, + Err(_) => panic!("staking ledger must exist"), + }; + + // bond manually stash with different controller. This is deprecated but the migration + // has not been complete yet (controller: 100, stash: 200) + assert_ok!(bond_controller_stash(100, 200)); + assert_eq!(>::get(&200), Some(100)); + + match StakingLedger::::get(StakingAccount::Stash(200)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(100)); + assert_eq!(ledger.stash, 200); + }, + Err(_) => panic!("staking ledger must exist"), + }; + + match StakingLedger::::get(StakingAccount::Controller(100)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(100)); + assert_eq!(ledger.stash, 200); + }, + Err(_) => panic!("staking ledger must exist"), + }; + }) + } + + #[test] + fn bond_works() { + ExtBuilder::default().build_and_execute(|| { + assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); + assert!(>::get(&42).is_none()); + + let mut ledger: StakingLedger = StakingLedger::default_from(42); + let reward_dest = RewardDestination::Account(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); + + // cannot bond again. + assert!(ledger.clone().bond(reward_dest).is_err()); + + // once bonded, update works as expected. + ledger.claimed_rewards = bounded_vec![1]; + assert_ok!(ledger.update()); + }) + } + + #[test] + fn is_bonded_works() { + ExtBuilder::default().build_and_execute(|| { + assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); + assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); + + // adds entry to Bonded without Ledger pair (should not happen). + >::insert(42, 42); + assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); + + assert_eq!(>::get(&11), Some(11)); + assert!(StakingLedger::::is_bonded(StakingAccount::Stash(11))); + assert!(StakingLedger::::is_bonded(StakingAccount::Controller(11))); + + >::remove(42); // ensures try-state checks pass. + }) + } +} diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index 8b5797d7918fb..dfc18987d1525 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -37,6 +37,23 @@ pub type SessionIndex = u32; /// Counter for the number of eras that have passed. pub type EraIndex = u32; +/// Representation of a staking account, which may be a stash or controller account. +/// +/// Note: once the controller is completely deprecated, this enum can also be deprecated in favor of +/// the stash account. Tracking issue: . +#[derive(Clone, Debug)] +pub enum StakingAccount { + Stash(AccountId), + Controller(AccountId), +} + +#[cfg(feature = "std")] +impl From for StakingAccount { + fn from(account: AccountId) -> Self { + StakingAccount::Stash(account) + } +} + /// Representation of the status of a staker. #[derive(RuntimeDebug, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize, PartialEq, Eq, Clone))]