From d84ec6adff0374f98b78c9d74a8e3c31e0465a39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Wed, 6 Sep 2023 18:34:13 +0100 Subject: [PATCH 1/2] Refactor staking ledger 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. Lift and shift from https://github.com/paritytech/substrate/pull/14582 --- .../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 | 16 +- substrate/frame/staking/src/ledger.rs | 258 +++++++++ substrate/frame/staking/src/lib.rs | 40 +- substrate/frame/staking/src/mock.rs | 17 +- substrate/frame/staking/src/pallet/impls.rs | 196 +++---- substrate/frame/staking/src/pallet/mod.rs | 147 +++--- substrate/frame/staking/src/slashing.rs | 18 +- substrate/frame/staking/src/tests.rs | 496 +++++++++++------- substrate/primitives/staking/src/lib.rs | 17 + 12 files changed, 812 insertions(+), 407 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 3ede1c908802..f075ce5ca737 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 e40bac3e9fc4..1d3f4712b1d6 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 722bb14fb56e..84258d84994f 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 ce5c35524c74..a82a9a0127af 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,13 @@ 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(), + T::Currency::minimum_balance() - One::one(), + Default::default(), + 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 000000000000..6599ffd9a477 --- /dev/null +++ b/substrate/frame/staking/src/ledger.rs @@ -0,0 +1,258 @@ +// 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, UnlockChunk, + STAKING_ID, +}; + +#[cfg(any(feature = "runtime-benchmarks", test))] +use { + codec::{Decode, Encode, MaxEncodedLen}, + scale_info::TypeInfo, + sp_runtime::traits::Zero, +}; + +impl StakingLedger { + #[cfg(any(feature = "runtime-benchmarks", test))] + pub fn default_from(stash: T::AccountId) -> Self { + Self { + stash, + total: Zero::zero(), + active: Zero::zero(), + unlocking: Default::default(), + claimed_rewards: Default::default(), + controller: Default::default(), + } + } + + /// 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, + active_stake: BalanceOf, + total_stake: BalanceOf, + unlocking: BoundedVec>, T::MaxUnlockingChunks>, + claimed_rewards: BoundedVec, + ) -> Self { + Self { + stash: stash.clone(), + active: active_stake, + total: total_stake, + unlocking, + 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_else(|| 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(|| 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(()) + })? + } +} + +// 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 e59b2a3324a6..96b76f43053e 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, } } @@ -921,7 +927,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 cf08f8be1f27..e013eeaaebfc 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, @@ -777,6 +780,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 e0f5c9558781..4ef7f0308a83 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,18 +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) = - 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, - amount: imbalance.peek(), - }); + Self::deposit_event(Event::::Rewarded { stash, amount: imbalance.peek() }); total_imbalance.subsume(imbalance); } @@ -274,14 +288,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); @@ -294,20 +300,24 @@ impl Pallet { /// Actually make a payment to a staker. This uses the currency's reward function /// to pay the right payee for the given staker account. fn make_payout(stash: &T::AccountId, amount: BalanceOf) -> Option> { - let dest = Self::payee(stash); + let dest = Self::payee(StakingAccount::Stash(stash.clone())); 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, @@ -400,7 +410,6 @@ impl Pallet { } /// Start a new era. It does: - /// /// * Increment `active_era.index`, /// * reset `active_era.start`, /// * update `BondedEras` and apply slashes. @@ -659,18 +668,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)?; + // removes controller from `Bonded` and staking ledger from `Ledger`, as well as reward + // setting of the stash in `Payee`. + StakingLedger::::kill(&stash)?; - >::remove(stash); - >::remove(&controller); + Self::do_remove_validator(&stash); + Self::do_remove_nominator(&stash); - >::remove(stash); - Self::do_remove_validator(stash); - Self::do_remove_nominator(stash); - - frame_system::Pallet::::dec_consumers(stash); + frame_system::Pallet::::dec_consumers(&stash); Ok(()) } @@ -1116,13 +1123,13 @@ 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, + stake, + Default::default(), + Default::default(), + ), ); Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false }); @@ -1134,13 +1141,13 @@ 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, + stake, + Default::default(), + Default::default(), + ), ); Self::do_add_validator( &target, @@ -1175,13 +1182,13 @@ 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, + stake, + Default::default(), + Default::default(), + ), ); Self::do_add_validator( &v, @@ -1196,13 +1203,13 @@ 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, + stake, + Default::default(), + Default::default(), + ), ); Self::do_add_nominator( &v, @@ -1452,9 +1459,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; @@ -1645,9 +1652,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 { @@ -1665,10 +1672,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 { @@ -1693,7 +1699,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) @@ -1720,8 +1726,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()) } @@ -1875,7 +1880,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 c6c75326b80c..9e132481c832 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>; @@ -837,16 +838,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()) @@ -854,11 +850,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); @@ -866,19 +857,23 @@ 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, + value, + Default::default(), + (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(()) } @@ -904,8 +899,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) { @@ -919,11 +913,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 }); @@ -959,9 +952,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. @@ -977,8 +969,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, @@ -994,9 +987,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() @@ -1020,15 +1013,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 { @@ -1085,7 +1077,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; @@ -1131,7 +1123,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; @@ -1198,7 +1191,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(()) } @@ -1222,9 +1217,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(()) } @@ -1246,18 +1243,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. @@ -1405,11 +1408,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(()) } @@ -1498,7 +1499,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; @@ -1511,16 +1512,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()) } @@ -1552,13 +1555,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()) } @@ -1578,7 +1579,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 @@ -1687,7 +1688,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 bb02da73f6e5..0d84d503733e 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 fd7dabac74d8..9fce490d6a55 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, @@ -1808,13 +1809,7 @@ fn reap_stash_works() { // instead. Ledger::::insert( 11, - StakingLedger { - stash: 11, - total: 5, - active: 5, - unlocking: Default::default(), - claimed_rewards: bounded_vec![], - }, + StakingLedger::::new(11, 5, 5, bounded_vec![], bounded_vec![]), ); // reap-able @@ -1937,14 +1932,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 +1947,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 +2039,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 +2091,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 +2977,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 +2992,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 +3027,7 @@ fn staker_cannot_bail_deferred_slash() { assert_eq!( Ledger::::get(101).unwrap(), - StakingLedger { + StakingLedgerInspect { active: 0, total: 500, stash: 101, @@ -3772,14 +3767,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 { @@ -3803,14 +3798,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; @@ -3836,14 +3831,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. @@ -3851,8 +3846,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, @@ -3864,7 +3859,7 @@ fn test_payout_stakers() { 69, expected_last_reward_era ] - }) + } ); }); } @@ -4063,26 +4058,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 @@ -4091,8 +4086,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, @@ -4101,7 +4096,7 @@ fn bond_during_era_correctly_populates_claimed_rewards() { .collect::>() .try_into() .unwrap(), - }) + } ); }); } @@ -4190,6 +4185,7 @@ fn payout_creates_controller() { false, ) .unwrap(); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![11])); // kill controller @@ -4346,8 +4342,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, @@ -4360,8 +4356,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, @@ -4386,8 +4382,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, @@ -4400,8 +4396,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, @@ -4427,8 +4423,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, @@ -4444,8 +4440,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, @@ -5520,16 +5516,12 @@ fn force_apply_min_commission_works() { #[test] fn proportional_slash_stop_slashing_if_remaining_zero() { let c = |era, value| UnlockChunk:: { era, value }; - // 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![], - }; + // we have some chunks, but they are not affected. + let unlocking = bounded_vec![c(1, 10), c(2, 10)]; + + // Given + let mut ledger = StakingLedger::::new(123, 40, 20, unlocking, bounded_vec![]); assert_eq!(BondingDuration::get(), 3); // should not slash more than the amount requested, by accidentally slashing the first chunk. @@ -5540,13 +5532,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, 10, bounded_vec![], bounded_vec![]); assert_eq!(BondingDuration::get(), 3); // When we slash a ledger with no unlocking chunks @@ -5785,8 +5771,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, @@ -5813,7 +5799,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!( @@ -5851,8 +5837,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, @@ -5890,8 +5876,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, @@ -5914,7 +5900,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)); @@ -5923,8 +5909,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)); @@ -5935,8 +5921,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)); @@ -6124,3 +6110,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 1621af164b37..d412cd032591 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))] From 1f8a4d9b7099a7396f9951e303f6edd78d96d74a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Mon, 18 Sep 2023 14:00:18 +0200 Subject: [PATCH 2/2] Improves based on latest review comments --- substrate/frame/staking/src/benchmarking.rs | 2 -- substrate/frame/staking/src/ledger.rs | 39 +++++++++++---------- substrate/frame/staking/src/pallet/impls.rs | 32 +++-------------- substrate/frame/staking/src/pallet/mod.rs | 2 -- substrate/frame/staking/src/tests.rs | 12 +++---- 5 files changed, 30 insertions(+), 57 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index a82a9a0127af..f94d9bf4b328 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -687,8 +687,6 @@ benchmarks! { let l = StakingLedger::::new( stash.clone(), T::Currency::minimum_balance() - One::one(), - T::Currency::minimum_balance() - One::one(), - Default::default(), Default::default(), ); Ledger::::insert(&controller, l); diff --git a/substrate/frame/staking/src/ledger.rs b/substrate/frame/staking/src/ledger.rs index 6599ffd9a477..cf9b4635bf55 100644 --- a/substrate/frame/staking/src/ledger.rs +++ b/substrate/frame/staking/src/ledger.rs @@ -40,27 +40,22 @@ use sp_staking::{EraIndex, StakingAccount}; use sp_std::prelude::*; use crate::{ - BalanceOf, Bonded, Config, Error, Ledger, Payee, RewardDestination, StakingLedger, UnlockChunk, - STAKING_ID, + BalanceOf, Bonded, Config, Error, Ledger, Payee, RewardDestination, StakingLedger, STAKING_ID, }; #[cfg(any(feature = "runtime-benchmarks", test))] -use { - codec::{Decode, Encode, MaxEncodedLen}, - scale_info::TypeInfo, - sp_runtime::traits::Zero, -}; +use sp_runtime::traits::Zero; impl StakingLedger { #[cfg(any(feature = "runtime-benchmarks", test))] pub fn default_from(stash: T::AccountId) -> Self { Self { - stash, + stash: stash.clone(), total: Zero::zero(), active: Zero::zero(), unlocking: Default::default(), claimed_rewards: Default::default(), - controller: Default::default(), + controller: Some(stash), } } @@ -73,16 +68,14 @@ impl StakingLedger { /// controller account. pub fn new( stash: T::AccountId, - active_stake: BalanceOf, - total_stake: BalanceOf, - unlocking: BoundedVec>, T::MaxUnlockingChunks>, + stake: BalanceOf, claimed_rewards: BoundedVec, ) -> Self { Self { stash: stash.clone(), - active: active_stake, - total: total_stake, - unlocking, + active: stake, + total: stake, + unlocking: Default::default(), claimed_rewards, // controllers are deprecated and mapped 1-1 to stashes. controller: Some(stash), @@ -129,7 +122,7 @@ impl StakingLedger { ledger.controller = Some(controller.clone()); ledger }) - .ok_or_else(|| Error::::NotController) + .ok_or(Error::::NotController) } /// Returns the reward destination of a staking ledger, stored in [`Payee`]. @@ -160,9 +153,10 @@ impl StakingLedger { /// [`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(|| Self::paired_account(StakingAccount::Stash(self.stash.clone()))) + 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. @@ -229,6 +223,13 @@ impl StakingLedger { } } +#[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)] diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 4ef7f0308a83..a6bbcc0f4054 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1123,13 +1123,7 @@ impl ElectionDataProvider for Pallet { >::insert(voter.clone(), voter.clone()); >::insert( voter.clone(), - StakingLedger::::new( - voter.clone(), - stake, - stake, - Default::default(), - Default::default(), - ), + StakingLedger::::new(voter.clone(), stake, Default::default()), ); Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false }); @@ -1141,13 +1135,7 @@ impl ElectionDataProvider for Pallet { >::insert(target.clone(), target.clone()); >::insert( target.clone(), - StakingLedger::::new( - target.clone(), - stake, - stake, - Default::default(), - Default::default(), - ), + StakingLedger::::new(target.clone(), stake, Default::default()), ); Self::do_add_validator( &target, @@ -1182,13 +1170,7 @@ impl ElectionDataProvider for Pallet { >::insert(v.clone(), v.clone()); >::insert( v.clone(), - StakingLedger::::new( - v.clone(), - stake, - stake, - Default::default(), - Default::default(), - ), + StakingLedger::::new(v.clone(), stake, Default::default()), ); Self::do_add_validator( &v, @@ -1203,13 +1185,7 @@ impl ElectionDataProvider for Pallet { >::insert(v.clone(), v.clone()); >::insert( v.clone(), - StakingLedger::::new( - v.clone(), - stake, - stake, - Default::default(), - Default::default(), - ), + StakingLedger::::new(v.clone(), stake, Default::default()), ); Self::do_add_nominator( &v, diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 9e132481c832..ef7c54b7d297 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -860,8 +860,6 @@ pub mod pallet { let ledger = StakingLedger::::new( stash.clone(), value, - value, - Default::default(), (last_reward_era..current_era) .try_collect() // Since last_reward_era is calculated as `current_era - diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 9fce490d6a55..99ed651c4e24 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -1807,10 +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::::new(11, 5, 5, bounded_vec![], bounded_vec![]), - ); + Ledger::::insert(11, StakingLedger::::new(11, 5, bounded_vec![])); // reap-able assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0)); @@ -5521,7 +5518,10 @@ fn proportional_slash_stop_slashing_if_remaining_zero() { let unlocking = bounded_vec![c(1, 10), c(2, 10)]; // Given - let mut ledger = StakingLedger::::new(123, 40, 20, unlocking, bounded_vec![]); + let mut ledger = StakingLedger::::new(123, 20, bounded_vec![]); + ledger.total = 40; + ledger.unlocking = unlocking; + assert_eq!(BondingDuration::get(), 3); // should not slash more than the amount requested, by accidentally slashing the first chunk. @@ -5532,7 +5532,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::::new(123, 10, 10, bounded_vec![], 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