From 9db92115d023689581e8cf49edb9500b0dcced53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Pestana?= Date: Thu, 18 Jan 2024 23:05:59 +0100 Subject: [PATCH] More tests and checks confirming that `ledger.controller` is always correct. (#2599) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bonded ledger fetched with the `StakingLedger` implementation exposes a method `ledger.controller()` that returns the controller of the ledger. However, that controller is computed and stored under the `ledger.controller` field on the fly - i.e when the ledger is fetched from storage using the `StakingLedger::get` method. The controller field is never stored in storage. This PR add a few more tests checks and improves the ledger try-state checks to make sure these invariants hold true. --------- Co-authored-by: command-bot <> Co-authored-by: Bastian Köcher Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- substrate/frame/staking/src/pallet/impls.rs | 12 +++++++++++- substrate/frame/staking/src/tests.rs | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 093cdfdb9c..80b4079dbd 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -1853,7 +1853,17 @@ impl Pallet { fn check_ledgers() -> Result<(), TryRuntimeError> { Bonded::::iter() - .map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl)) + .map(|(stash, ctrl)| { + // `ledger.controller` is never stored in raw storage. + let raw = Ledger::::get(stash).unwrap_or_else(|| { + Ledger::::get(ctrl.clone()) + .expect("try_check: bonded stash/ctrl does not have an associated ledger") + }); + ensure!(raw.controller.is_none(), "raw storage controller should be None"); + + // ensure ledger consistency. + Self::ensure_ledger_consistent(ctrl) + }) .collect::, _>>()?; Ok(()) } diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 0e9be70ee7..f469ace0bc 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -255,6 +255,20 @@ fn change_controller_works() { assert_eq!(Staking::bonded(&stash), Some(stash)); mock::start_active_era(1); + // fetch the ledger from storage and check if the controller is correct. + let ledger = Staking::ledger(StakingAccount::Stash(stash)).unwrap(); + assert_eq!(ledger.controller(), Some(stash)); + + // same if we fetch the ledger by controller. + let ledger = Staking::ledger(StakingAccount::Controller(stash)).unwrap(); + assert_eq!(ledger.controller, Some(stash)); + assert_eq!(ledger.controller(), Some(stash)); + + // the raw storage ledger's controller is always `None`. however, we can still fetch the + // correct controller with `ledger.controler()`. + let raw_ledger = >::get(&stash).unwrap(); + assert_eq!(raw_ledger.controller, None); + // `controller` is no longer in control. `stash` is now controller. assert_noop!( Staking::validate(RuntimeOrigin::signed(controller), ValidatorPrefs::default()),