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<StakingLedger>`. 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<T>`, `Payee<T>` and `Bonded<T>` 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

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::<T>(&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::<T>::new(
+			stash.clone(),
+			T::Currency::minimum_balance() - One::one(),
+			Default::default(),
+		);
 		Ledger::<T>::insert(&controller, l);
 
 		assert!(Bonded::<T>::contains_key(&stash));

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<T: Config> StakingLedger<T> {
+	#[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<T>,
+		claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>,
+	) -> 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<T::AccountId>) -> Option<T::AccountId> {
+		match account {
+			StakingAccount::Stash(stash) => <Bonded<T>>::get(stash),
+			StakingAccount::Controller(controller) =>
+				<Ledger<T>>::get(&controller).map(|ledger| ledger.stash),
+		}
+	}
+
+	/// Returns whether a given account is bonded.
+	pub(crate) fn is_bonded(account: StakingAccount<T::AccountId>) -> bool {
+		match account {
+			StakingAccount::Stash(stash) => <Bonded<T>>::contains_key(stash),
+			StakingAccount::Controller(controller) => <Ledger<T>>::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<T::AccountId>) -> Result<StakingLedger<T>, Error<T>> {
+		let controller = match account {
+			StakingAccount::Stash(stash) => <Bonded<T>>::get(stash).ok_or(Error::<T>::NotStash),
+			StakingAccount::Controller(controller) => Ok(controller),
+		}?;
+
+		<Ledger<T>>::get(&controller)
+			.map(|mut ledger| {
+				ledger.controller = Some(controller.clone());
+				ledger
+			})
+			.ok_or(Error::<T>::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<T::AccountId>,
+	) -> RewardDestination<T::AccountId> {
+		let stash = match account {
+			StakingAccount::Stash(stash) => Some(stash),
+			StakingAccount::Controller(controller) =>
+				Self::paired_account(StakingAccount::Controller(controller)),
+		};
+
+		if let Some(stash) = stash {
+			<Payee<T>>::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<T::AccountId> {
+		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<T>> {
+		if !<Bonded<T>>::contains_key(&self.stash) {
+			return Err(Error::<T>::NotStash)
+		}
+
+		T::Currency::set_lock(STAKING_ID, &self.stash, self.total, WithdrawReasons::all());
+		Ledger::<T>::insert(
+			&self.controller().ok_or_else(|| {
+				defensive!("update called on a ledger that is not bonded.");
+				Error::<T>::NotController
+			})?,
+			&self,
+		);
+
+		Ok(())
+	}
+
+	/// Bonds a ledger.
+	///
+	/// It sets the reward preferences for the bonded stash.
+	pub(crate) fn bond(self, payee: RewardDestination<T::AccountId>) -> Result<(), Error<T>> {
+		if <Bonded<T>>::contains_key(&self.stash) {
+			Err(Error::<T>::AlreadyBonded)
+		} else {
+			<Payee<T>>::insert(&self.stash, payee);
+			<Bonded<T>>::insert(&self.stash, &self.stash);
+			self.update()
+		}
+	}
+
+	/// Sets the ledger Payee.
+	pub(crate) fn set_payee(self, payee: RewardDestination<T::AccountId>) -> Result<(), Error<T>> {
+		if !<Bonded<T>>::contains_key(&self.stash) {
+			Err(Error::<T>::NotStash)
+		} else {
+			<Payee<T>>::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<T>> {
+		let controller = <Bonded<T>>::get(stash).ok_or(Error::<T>::NotStash)?;
+
+		<Ledger<T>>::get(&controller).ok_or(Error::<T>::NotController).map(|ledger| {
+			T::Currency::remove_lock(STAKING_ID, &ledger.stash);
+			Ledger::<T>::remove(controller);
+
+			<Bonded<T>>::remove(&stash);
+			<Payee<T>>::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<T: Config> {
+	pub stash: T::AccountId,
+	#[codec(compact)]
+	pub total: BalanceOf<T>,
+	#[codec(compact)]
+	pub active: BalanceOf<T>,
+	pub unlocking: BoundedVec<UnlockChunk<BalanceOf<T>>, T::MaxUnlockingChunks>,
+	pub claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>,
+}
+
+#[cfg(test)]
+impl<T: Config> PartialEq<StakingLedgerInspect<T>> for StakingLedger<T> {
+	fn eq(&self, other: &StakingLedgerInspect<T>) -> 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<T: Config> codec::EncodeLike<StakingLedger<T>> for StakingLedgerInspect<T> {}

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<Balance: HasCompact + MaxEncodedLen> {
 }
 
 /// 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
+/// <https://github.com/paritytech/substrate/issues/14749>.
 #[derive(
 	PartialEqNoBound,
 	EqNoBound,
@@ -462,20 +472,15 @@ pub struct StakingLedger<T: Config> {
 	/// List of eras for which the stakers behind a validator have claimed rewards. Only updated
 	/// for validators.
 	pub claimed_rewards: BoundedVec<EraIndex, T::HistoryDepth>,
+	/// 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<T::AccountId>,
 }
 
 impl<T: Config> StakingLedger<T> {
-	/// 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<T: Config> StakingLedger<T> {
 			active: self.active,
 			unlocking,
 			claimed_rewards: self.claimed_rewards,
+			controller: self.controller,
 		}
 	}
 
@@ -927,7 +933,7 @@ pub struct StashOf<T>(sp_std::marker::PhantomData<T>);
 
 impl<T: Config> Convert<T::AccountId, Option<T::AccountId>> for StashOf<T> {
 	fn convert(controller: T::AccountId) -> Option<T::AccountId> {
-		<Pallet<T>>::ledger(&controller).map(|l| l.stash)
+		StakingLedger::<T>::paired_account(StakingAccount::Controller(controller))
 	}
 }
 

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::<Test>::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> {
+	<Bonded<Test>>::get(&stash).map_or(Ok(()), |_| Err("stash already bonded"))?;
+	<Ledger<Test>>::get(&controller).map_or(Ok(()), |_| Err("controller already bonded"))?;
+
+	<Bonded<Test>>::insert(stash, controller);
+	<Ledger<Test>>::insert(controller, StakingLedger::<Test>::default_from(stash));
+
+	Ok(())
+}
+
 #[macro_export]
 macro_rules! assert_session_era {
 	($session:expr, $era:expr) => {

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<T: Config> Pallet<T> {
+	/// Fetches the ledger associated with a controller or stash account, if any.
+	pub fn ledger(account: StakingAccount<T::AccountId>) -> Result<StakingLedger<T>, Error<T>> {
+		StakingLedger::<T>::get(account)
+	}
+
+	pub fn payee(account: StakingAccount<T::AccountId>) -> RewardDestination<T::AccountId> {
+		StakingLedger::<T>::reward_destination(account)
+	}
+
+	/// Fetches the controller bonded to a stash account, if any.
+	pub fn bonded(stash: &T::AccountId) -> Option<T::AccountId> {
+		StakingLedger::<T>::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<T> {
 		// 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<T: Config> Pallet<T> {
 		controller: &T::AccountId,
 		num_slashing_spans: u32,
 	) -> Result<Weight, DispatchError> {
-		let mut ledger = Self::ledger(&controller).ok_or(Error::<T>::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<T: Config> Pallet<T> {
 
 		// `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::<T>::Withdrawn { stash, amount: value });
 		}
 
@@ -163,10 +178,15 @@ impl<T: Config> Pallet<T> {
 				.with_weight(T::WeightInfo::payout_stakers_alive_staked(0))
 		})?;
 
-		let controller = Self::bonded(&validator_stash).ok_or_else(|| {
-			Error::<T>::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::<T>::is_bonded(account) {
+				Err(Error::<T>::NotController.into())
+			} else {
+				Err(Error::<T>::NotStash.with_weight(T::WeightInfo::payout_stakers_alive_staked(0)))
+			}
 		})?;
-		let mut ledger = <Ledger<T>>::get(&controller).ok_or(Error::<T>::NotController)?;
+		let stash = ledger.stash.clone();
 
 		ledger
 			.claimed_rewards
@@ -185,11 +205,11 @@ impl<T: Config> Pallet<T> {
 				.defensive_map_err(|_| Error::<T>::BoundNotMet)?,
 		}
 
-		let exposure = <ErasStakersClipped<T>>::get(&era, &ledger.stash);
+		let exposure = <ErasStakersClipped<T>>::get(&era, &stash);
 
 		// Input data seems good, no errors allowed after this point
 
-		<Ledger<T>>::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<T: Config> Pallet<T> {
 
 		let era_reward_points = <ErasRewardPoints<T>>::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<T: Config> Pallet<T> {
 
 		Self::deposit_event(Event::<T>::PayoutStarted {
 			era_index: era,
-			validator_stash: ledger.stash.clone(),
+			validator_stash: stash.clone(),
 		});
 
 		let mut total_imbalance = PositiveImbalanceOf::<T>::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::<T>::Rewarded {
-				stash: ledger.stash,
-				dest,
-				amount: imbalance.peek(),
-			});
+			Self::deposit_event(Event::<T>::Rewarded { stash, dest, amount: imbalance.peek() });
 			total_imbalance.subsume(imbalance);
 		}
 
@@ -278,14 +291,6 @@ impl<T: Config> Pallet<T> {
 		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>) {
-		T::Currency::set_lock(STAKING_ID, &ledger.stash, ledger.total, WithdrawReasons::all());
-		<Ledger<T>>::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<T: Config> Pallet<T> {
 		stash: &T::AccountId,
 		amount: BalanceOf<T>,
 	) -> Option<(PositiveImbalanceOf<T>, RewardDestination<T::AccountId>)> {
-		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<T: Config> Pallet<T> {
 	}
 
 	/// Start a new era. It does:
-	///
 	/// * Increment `active_era.index`,
 	/// * reset `active_era.start`,
 	/// * update `BondedEras` and apply slashes.
@@ -666,18 +676,16 @@ impl<T: Config> Pallet<T> {
 	/// - 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 = <Bonded<T>>::get(stash).ok_or(Error::<T>::NotStash)?;
+		slashing::clear_stash_metadata::<T>(&stash, num_slashing_spans)?;
 
-		slashing::clear_stash_metadata::<T>(stash, num_slashing_spans)?;
+		// removes controller from `Bonded` and staking ledger from `Ledger`, as well as reward
+		// setting of the stash in `Payee`.
+		StakingLedger::<T>::kill(&stash)?;
 
-		<Bonded<T>>::remove(stash);
-		<Ledger<T>>::remove(&controller);
+		Self::do_remove_validator(&stash);
+		Self::do_remove_nominator(&stash);
 
-		<Payee<T>>::remove(stash);
-		Self::do_remove_validator(stash);
-		Self::do_remove_nominator(stash);
-
-		frame_system::Pallet::<T>::dec_consumers(stash);
+		frame_system::Pallet::<T>::dec_consumers(&stash);
 
 		Ok(())
 	}
@@ -1123,13 +1131,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 		<Bonded<T>>::insert(voter.clone(), voter.clone());
 		<Ledger<T>>::insert(
 			voter.clone(),
-			StakingLedger {
-				stash: voter.clone(),
-				active: stake,
-				total: stake,
-				unlocking: Default::default(),
-				claimed_rewards: Default::default(),
-			},
+			StakingLedger::<T>::new(voter.clone(), stake, Default::default()),
 		);
 
 		Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false });
@@ -1141,13 +1143,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 		<Bonded<T>>::insert(target.clone(), target.clone());
 		<Ledger<T>>::insert(
 			target.clone(),
-			StakingLedger {
-				stash: target.clone(),
-				active: stake,
-				total: stake,
-				unlocking: Default::default(),
-				claimed_rewards: Default::default(),
-			},
+			StakingLedger::<T>::new(target.clone(), stake, Default::default()),
 		);
 		Self::do_add_validator(
 			&target,
@@ -1182,13 +1178,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 			<Bonded<T>>::insert(v.clone(), v.clone());
 			<Ledger<T>>::insert(
 				v.clone(),
-				StakingLedger {
-					stash: v.clone(),
-					active: stake,
-					total: stake,
-					unlocking: Default::default(),
-					claimed_rewards: Default::default(),
-				},
+				StakingLedger::<T>::new(v.clone(), stake, Default::default()),
 			);
 			Self::do_add_validator(
 				&v,
@@ -1203,13 +1193,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {
 			<Bonded<T>>::insert(v.clone(), v.clone());
 			<Ledger<T>>::insert(
 				v.clone(),
-				StakingLedger {
-					stash: v.clone(),
-					active: stake,
-					total: stake,
-					unlocking: Default::default(),
-					claimed_rewards: Default::default(),
-				},
+				StakingLedger::<T>::new(v.clone(), stake, Default::default()),
 			);
 			Self::do_add_nominator(
 				&v,
@@ -1459,9 +1443,9 @@ impl<T: Config> ScoreProvider<T::AccountId> for Pallet<T> {
 		// this will clearly results in an inconsistent state, but it should not matter for a
 		// benchmark.
 		let active: BalanceOf<T> = 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<T: Config> StakingInterface for Pallet<T> {
 	}
 
 	fn stash_by_ctrl(controller: &Self::AccountId) -> Result<Self::AccountId, DispatchError> {
-		Self::ledger(controller)
+		Self::ledger(Controller(controller.clone()))
 			.map(|l| l.stash)
-			.ok_or(Error::<T>::NotController.into())
+			.map_err(|e| e.into())
 	}
 
 	fn is_exposed_in_era(who: &Self::AccountId, era: &EraIndex) -> bool {
@@ -1672,10 +1656,9 @@ impl<T: Config> StakingInterface for Pallet<T> {
 	}
 
 	fn stake(who: &Self::AccountId) -> Result<Stake<BalanceOf<T>>, 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::<T>::NotStash.into())
+			.map_err(|e| e.into())
 	}
 
 	fn bond_extra(who: &Self::AccountId, extra: Self::Balance) -> DispatchResult {
@@ -1700,7 +1683,7 @@ impl<T: Config> StakingInterface for Pallet<T> {
 		who: Self::AccountId,
 		num_slashing_spans: u32,
 	) -> Result<bool, DispatchError> {
-		let ctrl = Self::bonded(who).ok_or(Error::<T>::NotStash)?;
+		let ctrl = Self::bonded(&who).ok_or(Error::<T>::NotStash)?;
 		Self::withdraw_unbonded(RawOrigin::Signed(ctrl.clone()).into(), num_slashing_spans)
 			.map(|_| !Ledger::<T>::contains_key(&ctrl))
 			.map_err(|with_post| with_post.error)
@@ -1727,8 +1710,7 @@ impl<T: Config> StakingInterface for Pallet<T> {
 	fn status(
 		who: &Self::AccountId,
 	) -> Result<sp_staking::StakerStatus<Self::AccountId>, DispatchError> {
-		let is_bonded = Self::bonded(who).is_some();
-		if !is_bonded {
+		if !StakingLedger::<T>::is_bonded(StakingAccount::Stash(who.clone())) {
 			return Err(Error::<T>::NotStash.into())
 		}
 
@@ -1882,7 +1864,8 @@ impl<T: Config> Pallet<T> {
 
 	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<T> =
 			ledger.unlocking.iter().fold(ledger.active, |a, c| a + c.value);
 		ensure!(real_total == ledger.total, "ledger.total corrupt");

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<T: Config> = 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<T: Config> = 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<T: Config> = StorageMap<_, Blake2_128Concat, T::AccountId, StakingLedger<T>>;
 
 	/// 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<T: Config> =
 		StorageMap<_, Twox64Concat, T::AccountId, RewardDestination<T::AccountId>, ValueQuery>;
 
@@ -841,16 +842,11 @@ pub mod pallet {
 			payee: RewardDestination<T::AccountId>,
 		) -> DispatchResult {
 			let stash = ensure_signed(origin)?;
-			let controller_to_be_deprecated = stash.clone();
 
-			if <Bonded<T>>::contains_key(&stash) {
+			if StakingLedger::<T>::is_bonded(StakingAccount::Stash(stash.clone())) {
 				return Err(Error::<T>::AlreadyBonded.into())
 			}
 
-			if <Ledger<T>>::contains_key(&controller_to_be_deprecated) {
-				return Err(Error::<T>::AlreadyPaired.into())
-			}
-
 			// Reject a bond which is considered to be _dust_.
 			if value < T::Currency::minimum_balance() {
 				return Err(Error::<T>::InsufficientBond.into())
@@ -858,11 +854,6 @@ pub mod pallet {
 
 			frame_system::Pallet::<T>::inc_consumers(&stash).map_err(|_| Error::<T>::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__.
-			<Bonded<T>>::insert(&stash, &stash);
-			<Payee<T>>::insert(&stash, payee);
-
 			let current_era = CurrentEra::<T>::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::<T>::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::<T>::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::<T>::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::<T>::NotStash)?;
-			let mut ledger = Self::ledger(&controller).ok_or(Error::<T>::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::<T>::Bonded { stash, amount: extra });
@@ -963,9 +954,8 @@ pub mod pallet {
 			#[pallet::compact] value: BalanceOf<T>,
 		) -> DispatchResultWithPostInfo {
 			let controller = ensure_signed(origin)?;
-			let unlocking = Self::ledger(&controller)
-				.map(|l| l.unlocking.len())
-				.ok_or(Error::<T>::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::<T>::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::<T>::contains_key(&ledger.stash) {
+				let min_active_bond = if Nominators::<T>::contains_key(&stash) {
 					MinNominatorBond::<T>::get()
-				} else if Validators::<T>::contains_key(&ledger.stash) {
+				} else if Validators::<T>::contains_key(&stash) {
 					MinValidatorBond::<T>::get()
 				} else {
 					Zero::zero()
@@ -1024,15 +1015,14 @@ pub mod pallet {
 						.map_err(|_| Error::<T>::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::<T>::Unbonded { stash: ledger.stash, amount: value });
+				Self::deposit_event(Event::<T>::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<T>, prefs: ValidatorPrefs) -> DispatchResult {
 			let controller = ensure_signed(origin)?;
 
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
+			let ledger = Self::ledger(Controller(controller))?;
 
 			ensure!(ledger.active >= MinValidatorBond::<T>::get(), Error::<T>::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::<T>::NotController)?;
+			let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?;
+
 			ensure!(ledger.active >= MinNominatorBond::<T>::get(), Error::<T>::InsufficientBond);
 			let stash = &ledger.stash;
 
@@ -1202,7 +1193,9 @@ pub mod pallet {
 		#[pallet::weight(T::WeightInfo::chill())]
 		pub fn chill(origin: OriginFor<T>) -> DispatchResult {
 			let controller = ensure_signed(origin)?;
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
+
+			let ledger = Self::ledger(StakingAccount::Controller(controller))?;
+
 			Self::chill_stash(&ledger.stash);
 			Ok(())
 		}
@@ -1226,9 +1219,11 @@ pub mod pallet {
 			payee: RewardDestination<T::AccountId>,
 		) -> DispatchResult {
 			let controller = ensure_signed(origin)?;
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
-			let stash = &ledger.stash;
-			<Payee<T>>::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<T>) -> DispatchResult {
 			let stash = ensure_signed(origin)?;
-			let old_controller = Self::bonded(&stash).ok_or(Error::<T>::NotStash)?;
 
-			if <Ledger<T>>::contains_key(&stash) {
-				return Err(Error::<T>::AlreadyPaired.into())
-			}
-			if old_controller != stash {
-				<Bonded<T>>::insert(&stash, &stash);
-				if let Some(l) = <Ledger<T>>::take(&old_controller) {
-					<Ledger<T>>::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::<T>::NotController)?;
+
+				if controller == stash {
+					// stash is already its own controller.
+					return Err(Error::<T>::AlreadyPaired.into())
 				}
-			}
-			Ok(())
+				// update bond and ledger.
+				<Ledger<T>>::remove(controller);
+				<Bonded<T>>::insert(&stash, &stash);
+				<Ledger<T>>::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<T>,
 		) -> DispatchResultWithPostInfo {
 			let controller = ensure_signed(origin)?;
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
+			let ledger = Self::ledger(Controller(controller))?;
 			ensure!(!ledger.unlocking.is_empty(), Error::<T>::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::<T>::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::<T>::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<T>, who: Vec<AccountIdLookupOf<T>>) -> DispatchResult {
 			let controller = ensure_signed(origin)?;
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::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<T>, controller: T::AccountId) -> DispatchResult {
 			// Anyone can call this function.
 			let caller = ensure_signed(origin)?;
-			let ledger = Self::ledger(&controller).ok_or(Error::<T>::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:

substrate/frame/staking/src/slashing.rs

@@ -597,15 +597,11 @@ pub fn do_slash<T: Config>(
 	slashed_imbalance: &mut NegativeImbalanceOf<T>,
 	slash_era: EraIndex,
 ) {
-	let controller = match <Pallet<T>>::bonded(stash).defensive() {
-		None => return,
-		Some(c) => c,
-	};
-
-	let mut ledger = match <Pallet<T>>::ledger(&controller) {
-		Some(ledger) => ledger,
-		None => return, // nothing to do.
-	};
+	let mut ledger =
+		match Pallet::<T>::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<T: Config>(
 			*reward_payout = reward_payout.saturating_sub(missing);
 		}
 
-		<Pallet<T>>::update_ledger(&controller, &ledger);
+		let _ = ledger
+			.update()
+			.defensive_proof("ledger fetched from storage so it exists in storage; qed.");
 
 		// trigger the event
 		<Pallet<T>>::deposit_event(super::Event::<T>::Slashed {