[stable2509] Backport #9926 (#9970)

Backport #9926 into `stable2509` from kianenigma.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: Paolo La Camera <[email protected]>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: 4 people

prdoc/pr_9926.prdoc

@@ -0,0 +1,14 @@
+title: 'Staking-Async: Chill stakers should not have a score'
+doc:
+- audience: Runtime Dev
+  description: |-
+    Async: Chill stakers should not have a score.
+
+    While no severe consequence, this bug could cause non-validator and non-nominator stakers to retain a spot in the bags-list pallet, preventing other legit nominators/validators from taking their place.
+
+    Note that previously, this was not a possibility, because `staking` would always issue a `T::VoterList::on_remove` when someone `chill`s, ensuring they are removed from the list. Moreover, an older version of `pallet_bags_list::Pallet::rebag` didn't allow new nodes to be added, only the score of existing nodes to be adjusted.
+
+    But, in recent versions of `bags-list`, we added a `Lock` ability that would block any changes to the bags list (during the election snapshot phase). This also had us update the `rebag` transaction to add or remove nodes from the list, which opened the door to this issue.
+crates:
+- name: pallet-staking-async
+  bump: patch

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

@@ -1343,12 +1343,20 @@ impl<T: Config> ScoreProvider<T::AccountId> for Pallet<T> {
 
 	fn score(who: &T::AccountId) -> Option<Self::Score> {
 		Self::ledger(Stash(who.clone()))
-			.map(|l| l.active)
+			.ok()
+			.and_then(|l| {
+				if Nominators::<T>::contains_key(&l.stash) ||
+					Validators::<T>::contains_key(&l.stash)
+				{
+					Some(l.active)
+				} else {
+					None
+				}
+			})
 			.map(|a| {
 				let issuance = asset::total_issuance::<T>();
 				T::CurrencyToVote::to_vote(a, issuance)
 			})
-			.ok()
 	}
 
 	#[cfg(feature = "runtime-benchmarks")]
@@ -1364,6 +1372,9 @@ impl<T: Config> ScoreProvider<T::AccountId> for Pallet<T> {
 
 		<Ledger<T>>::insert(who, ledger);
 		<Bonded<T>>::insert(who, who);
+		// we also need to appoint this staker to be validator or nominator, such that their score
+		// is actually there. Note that `fn score` above checks the role.
+		<Validators<T>>::insert(who, ValidatorPrefs::default());
 
 		// also, we play a trick to make sure that a issuance based-`CurrencyToVote` behaves well:
 		// This will make sure that total issuance is zero, thus the currency to vote will be a 1-1

substrate/frame/staking-async/src/tests/election_data_provider.rs

@@ -622,7 +622,7 @@ mod paged_snapshot {
 	}
 
 	#[test]
-	fn target_snaposhot_multi_page_redundant() {
+	fn target_snapshot_multi_page_redundant() {
 		ExtBuilder::default().build_and_execute(|| {
 			let all_targets = vec![31, 21, 11];
 			assert_eq_uvec!(<Test as Config>::TargetList::iter().collect::<Vec<_>>(), all_targets,);
@@ -858,6 +858,67 @@ mod paged_snapshot {
 	}
 }
 
+mod score_provider {
+	use frame_election_provider_support::ScoreProvider;
+
+	use super::*;
+
+	#[test]
+	fn no_score_for_chilled_stakers() {
+		ExtBuilder::default().build_and_execute(|| {
+			// given 41 being a chilled staker
+			assert!(
+				Ledger::<Test>::get(41).is_some() &&
+					!Validators::<Test>::contains_key(41) &&
+					!Nominators::<Test>::contains_key(41)
+			);
+
+			// then they will not have a score when bags-list wants to update it.
+			assert!(<Staking as ScoreProvider<_>>::score(&41).is_none());
+		});
+	}
+
+	#[test]
+	fn no_score_for_non_stakers() {
+		ExtBuilder::default().build_and_execute(|| {
+			// given 777 being neither a nominator nor a validator in this pallet.
+			assert!(
+				!Ledger::<Test>::get(777).is_some() &&
+					!Validators::<Test>::contains_key(777) &&
+					!Nominators::<Test>::contains_key(777)
+			);
+
+			// then it will not have a score when bags-list wants to update it.
+			assert!(<Staking as ScoreProvider<_>>::score(&777).is_none());
+		});
+	}
+
+	#[test]
+	fn score_for_validators_nominators() {
+		ExtBuilder::default().nominate(true).build_and_execute(|| {
+			// Given 101 being a nominator
+			assert!(
+				Ledger::<Test>::get(101).unwrap().active == 500 &&
+					!Validators::<Test>::contains_key(101) &&
+					Nominators::<Test>::contains_key(101)
+			);
+
+			// then it will have a score.
+			assert_eq!(<Staking as ScoreProvider<_>>::score(&101), Some(500));
+
+			// given 11 being a validator
+			assert!(
+				Ledger::<Test>::get(11).unwrap().active == 1000 &&
+					Validators::<Test>::contains_key(11) &&
+					!Nominators::<Test>::contains_key(11)
+			);
+
+			// then it will have a score.
+			assert_eq!(<Staking as ScoreProvider<_>>::score(&11), Some(1000));
+		});
+	}
+}
+
 #[test]
 fn from_most_staked_to_least_staked() {
 	ExtBuilder::default()