More tests and checks confirming that ledger.controller is always c…

…orrect. (paritytech#2599)

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 <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>

substrate/frame/staking/src/pallet/impls.rs

@@ -1853,7 +1853,17 @@ impl<T: Config> Pallet<T> {
 
 	fn check_ledgers() -> Result<(), TryRuntimeError> {
 		Bonded::<T>::iter()
-			.map(|(_, ctrl)| Self::ensure_ledger_consistent(ctrl))
+			.map(|(stash, ctrl)| {
+				// `ledger.controller` is never stored in raw storage.
+				let raw = Ledger::<T>::get(stash).unwrap_or_else(|| {
+					Ledger::<T>::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::<Result<Vec<_>, _>>()?;
 		Ok(())
 	}

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 = <Ledger<Test>>::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()),