migration docs; clippy fixes

substrate/frame/staking/src/migrations/v13_stake_tracker/mod.rs

@@ -50,16 +50,38 @@ impl Default for Processing {
 
 /// V13 Multi-block migration to introduce the stake-tracker pallet.
 ///
-/// A step of the migration consists of processing one nominator in the [`Nominators`] list. All
-/// nominatior's target nominations are processed per step (bound by upper bound of the max
-/// nominations).
+/// A step of the migration consists of processing one nominator in the [`Nominators`] list or one
+/// validators in the [`Validators`] list, depending on the cursor's state.
+///
+/// The migration has two phases:
+/// * [`Processing::Nominators`]: iterates over the nominators list and updates the approvals stake
+/// of the nominated targets associated with each of the nominators.
+/// * [`Processing::Validators`]: iterates over the validators list and, if the validator does not
+/// exist in the target list, add it with self-stake as score.
+///
+/// First, the migration processes all the nominators and then the validators. When a validator is
+/// added to the target list during the [`Processing::Validators`], it indicates that the validator
+/// has no nominations.
 ///
 /// The goals of the migration are:
-/// - Insert all the nominated targets into the [`SortedListProvider`] target list.
-/// - Ensure the target score (total stake) is the sum of the self stake and all its nominations
+/// * Insert all the nominated targets into the [`SortedListProvider`] target list.
+/// * Ensure the target score (total stake) is the sum of the self stake and all its nominations
 /// stake.
-/// - Ensure the new targets in the list are sorted per total stake (as per the underlying
+/// * Ensure the new targets in the list are sorted per total stake (as per the underlying
 ///   [`SortedListProvider`]).
+/// * Ensure that there is no duplicate nominations per nominator.
+///
+/// ## Potential changes to nominations state during the migration.
+///
+/// When migrating a nominator, we need to ensure that the nominator is not nominating duplicate
+/// targets. In addition, this migration also "cleans" the nominations by dropping all nominations
+/// of targets that are not active validators. This logic is implemented by
+/// [`MigrationV13::clean_nominations`]. In sum, the followinf invariants hold true after
+/// processing each nominator:
+///
+/// 1. A nominator has no duplicate nominations;
+/// 2. A nominators is nominating only active validators;
+/// 3. A nominator has at least one nomination, otherwise it is chilled.
 pub struct MigrationV13<T: Config, W: weights::WeightInfo>(PhantomData<(T, W)>);
 impl<T: Config, W: weights::WeightInfo> SteppedMigration for MigrationV13<T, W> {
 	// nominator cursor and validator cursor.

substrate/frame/staking/stake-tracker/src/lib.rs

@@ -79,9 +79,6 @@
 //!   be removed onced all the voters stop nominating the unbonded account (i.e. the target's score
 //!   drops to 0).
 //!
-//! For further details on the target list invariante, refer to [`Self`::do_try_state_approvals`]
-//! and [`Self::do_try_state_target_sorting`].
-//!
 //! ## Event emitter ordering and staking ledger state updates
 //!
 //! It is important to ensure that the events are emitted from staking (i.e. the calls into

substrate/frame/staking/stake-tracker/src/mock.rs

@@ -461,10 +461,6 @@ pub(crate) fn voter_bags_events() -> Vec<pallet_bags_list::Event<Test, VoterBags
 		.collect::<Vec<_>>()
 }
 
-parameter_types! {
-	pub static DisableTryRuntimeChecks: bool = false;
-}
-
 #[derive(Default, Copy, Clone)]
 pub struct ExtBuilder {
 	populate_lists: bool,
@@ -481,12 +477,6 @@ impl ExtBuilder {
 		self
 	}
 
-	#[allow(dead_code)]
-	pub fn try_state(self, enable: bool) -> Self {
-		DisableTryRuntimeChecks::set(!enable);
-		self
-	}
-
 	pub fn build(self) -> sp_io::TestExternalities {
 		sp_tracing::try_init_simple();
 		let storage = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
@@ -506,13 +496,5 @@ impl ExtBuilder {
 			System::set_block_number(1);
 		});
 		ext.execute_with(test);
-
-		if !DisableTryRuntimeChecks::get() {
-			ext.execute_with(|| {
-				StakeTracker::do_try_state()
-					.map_err(|err| println!(" 🕵️‍♂️  try_state failure: {:?}", err))
-					.unwrap();
-			});
-		}
 	}
 }

substrate/frame/staking/stake-tracker/src/tests.rs

@@ -60,11 +60,10 @@ fn update_target_score_works() {
 
 		let current_score = TargetBagsList::get_score(&10).unwrap();
 		crate::Pallet::<Test>::update_target_score(&10, StakeImbalance::Negative(current_score));
-		assert_eq!(TargetBagsList::get_score(&10), Ok(0));
 
-		// disables the try runtime checks since the score of 10 was updated manually, so the target
-		// list was not updated accordingly.
-		DisableTryRuntimeChecks::set(true);
+		// score dropped to 0, node is removed.
+		assert!(!TargetBagsList::contains(&10));
+		assert!(TargetBagsList::get_score(&10).is_err());
 	})
 }
 
@@ -446,13 +445,7 @@ mod staking_integration {
 
 			chill_staker(2);
 			assert_eq!(StakingMock::status(&2), Ok(StakerStatus::Idle));
-			// a chilled validator is kepts in the target list.
-			assert!(TargetBagsList::contains(&2));
-			assert!(!VoterBagsList::contains(&2));
-
-			remove_staker(2);
-			assert!(StakingMock::status(&2).is_err());
-			// a chilled validator is kepts in the target list if its score is 0.
+			// a chilled validator is dropped from the target list if its score is 0.
 			assert!(!TargetBagsList::contains(&2));
 			assert!(!VoterBagsList::contains(&2));
 		})