Add minor improvements to chill_other test (#7553)

# Description

open-web3-stack/polkadot-ecosystem-tests#174
showed the test for the `pallet_staking::chill_other` extrinsic could be
more exhaustive.

This PR adds those checks, and also a few more to another test related
to `chill_other`,
`pallet_staking::tests::change_of_absolute_max_nominations`.

## Integration

N/A

## Review Notes

N/A

---------

Co-authored-by: Bastian Köcher <[email protected]>

Author: rockbmb

substrate/frame/staking/src/tests.rs

@@ -5789,6 +5789,21 @@ fn chill_other_works() {
 			//
 			// If any of these are missing, we do not have enough information to allow the
 			// `chill_other` to succeed from one user to another.
+			//
+			// Out of 8 possible cases, only one will allow the use of `chill_other`, which is
+			// when all 3 conditions are met.
+
+			// 1. No limits whatsoever
+			assert_ok!(Staking::set_staking_configs(
+				RuntimeOrigin::root(),
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+			));
 
 			// Can't chill these users
 			assert_noop!(
@@ -5800,15 +5815,15 @@ fn chill_other_works() {
 				Error::<Test>::CannotChillOther
 			);
 
-			// Change the minimum bond... but no limits.
+			// 2. Change only the minimum bonds.
 			assert_ok!(Staking::set_staking_configs(
 				RuntimeOrigin::root(),
 				ConfigOp::Set(1_500),
 				ConfigOp::Set(2_000),
-				ConfigOp::Remove,
-				ConfigOp::Remove,
-				ConfigOp::Remove,
-				ConfigOp::Remove,
+				ConfigOp::Noop,
+				ConfigOp::Noop,
+				ConfigOp::Noop,
+				ConfigOp::Noop,
 				ConfigOp::Noop,
 			));
 
@@ -5822,11 +5837,11 @@ fn chill_other_works() {
 				Error::<Test>::CannotChillOther
 			);
 
-			// Add limits, but no threshold
+			// 3. Add nominator/validator count limits, but no other threshold.
 			assert_ok!(Staking::set_staking_configs(
 				RuntimeOrigin::root(),
-				ConfigOp::Noop,
-				ConfigOp::Noop,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
 				ConfigOp::Set(10),
 				ConfigOp::Set(10),
 				ConfigOp::Noop,
@@ -5844,15 +5859,59 @@ fn chill_other_works() {
 				Error::<Test>::CannotChillOther
 			);
 
-			// Add threshold, but no limits
+			// 4. Add chil threshold, but no other limits
 			assert_ok!(Staking::set_staking_configs(
 				RuntimeOrigin::root(),
 				ConfigOp::Noop,
 				ConfigOp::Noop,
 				ConfigOp::Remove,
 				ConfigOp::Remove,
+				ConfigOp::Set(Percent::from_percent(75)),
 				ConfigOp::Noop,
 				ConfigOp::Noop,
+			));
+
+			// Still can't chill these users
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 0),
+				Error::<Test>::CannotChillOther
+			);
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 2),
+				Error::<Test>::CannotChillOther
+			);
+
+			// 5. Add bond and count limits, but no threshold
+			assert_ok!(Staking::set_staking_configs(
+				RuntimeOrigin::root(),
+				ConfigOp::Set(1_500),
+				ConfigOp::Set(2_000),
+				ConfigOp::Set(10),
+				ConfigOp::Set(10),
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+			));
+
+			// Still can't chill these users
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 0),
+				Error::<Test>::CannotChillOther
+			);
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 2),
+				Error::<Test>::CannotChillOther
+			);
+
+			// 6. Add bond and threshold limits, but no count limits
+			assert_ok!(Staking::set_staking_configs(
+				RuntimeOrigin::root(),
+				ConfigOp::Noop,
+				ConfigOp::Noop,
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Set(Percent::from_percent(75)),
+				ConfigOp::Noop,
 				ConfigOp::Noop,
 			));
 
@@ -5866,11 +5925,33 @@ fn chill_other_works() {
 				Error::<Test>::CannotChillOther
 			);
 
-			// Add threshold and limits
+			// 7. Add count limits and a chill threshold, but no bond limits
 			assert_ok!(Staking::set_staking_configs(
 				RuntimeOrigin::root(),
+				ConfigOp::Remove,
+				ConfigOp::Remove,
+				ConfigOp::Set(10),
+				ConfigOp::Set(10),
+				ConfigOp::Set(Percent::from_percent(75)),
 				ConfigOp::Noop,
 				ConfigOp::Noop,
+			));
+
+			// Still can't chill these users
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 0),
+				Error::<Test>::CannotChillOther
+			);
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1337), 2),
+				Error::<Test>::CannotChillOther
+			);
+
+			// 8. Add all limits
+			assert_ok!(Staking::set_staking_configs(
+				RuntimeOrigin::root(),
+				ConfigOp::Set(1_500),
+				ConfigOp::Set(2_000),
 				ConfigOp::Set(10),
 				ConfigOp::Set(10),
 				ConfigOp::Set(Percent::from_percent(75)),
@@ -5888,7 +5969,9 @@ fn chill_other_works() {
 				let b = 4 * i;
 				let d = 4 * i + 2;
 				assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), b));
+				assert_eq!(*staking_events().last().unwrap(), Event::Chilled { stash: b });
 				assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), d));
+				assert_eq!(*staking_events().last().unwrap(), Event::Chilled { stash: d });
 			}
 
 			// chill a nominator. Limit is not reached, not chill-able
@@ -6090,6 +6173,14 @@ fn change_of_absolute_max_nominations() {
 			);
 			assert_eq!(Staking::electing_voters(bounds).unwrap().len(), 3 + 3);
 
+			// No one can be chilled on account of non-decodable keys.
+			for k in Nominators::<Test>::iter_keys() {
+				assert_noop!(
+					Staking::chill_other(RuntimeOrigin::signed(1), k),
+					Error::<Test>::CannotChillOther
+				);
+			}
+
 			// abrupt change from 4 to 3, everyone should be fine.
 			AbsoluteMaxNominations::set(3);
 
@@ -6101,8 +6192,16 @@ fn change_of_absolute_max_nominations() {
 			);
 			assert_eq!(Staking::electing_voters(bounds).unwrap().len(), 3 + 3);
 
+			// As before, no one can be chilled on account of non-decodable keys.
+			for k in Nominators::<Test>::iter_keys() {
+				assert_noop!(
+					Staking::chill_other(RuntimeOrigin::signed(1), k),
+					Error::<Test>::CannotChillOther
+				);
+			}
+
 			// abrupt change from 3 to 2, this should cause some nominators to be non-decodable, and
-			// thus non-existent unless if they update.
+			// thus non-existent unless they update.
 			AbsoluteMaxNominations::set(2);
 
 			assert_eq!(
@@ -6111,7 +6210,16 @@ fn change_of_absolute_max_nominations() {
 					.collect::<Vec<_>>(),
 				vec![(101, 2), (61, 1)]
 			);
-			// 70 is still in storage..
+
+			// 101 and 61 still cannot be chilled by someone else.
+			for k in [101, 61].iter() {
+				assert_noop!(
+					Staking::chill_other(RuntimeOrigin::signed(1), *k),
+					Error::<Test>::CannotChillOther
+				);
+			}
+
+			// 71 is still in storage..
 			assert!(Nominators::<Test>::contains_key(71));
 			// but its value cannot be decoded and default is returned.
 			assert!(Nominators::<Test>::get(71).is_none());
@@ -6120,7 +6228,7 @@ fn change_of_absolute_max_nominations() {
 			assert!(Nominators::<Test>::contains_key(101));
 
 			// abrupt change from 2 to 1, this should cause some nominators to be non-decodable, and
-			// thus non-existent unless if they update.
+			// thus non-existent unless they update.
 			AbsoluteMaxNominations::set(1);
 
 			assert_eq!(
@@ -6129,6 +6237,13 @@ fn change_of_absolute_max_nominations() {
 					.collect::<Vec<_>>(),
 				vec![(61, 1)]
 			);
+
+			// 61 *still* cannot be chilled by someone else.
+			assert_noop!(
+				Staking::chill_other(RuntimeOrigin::signed(1), 61),
+				Error::<Test>::CannotChillOther
+			);
+
 			assert!(Nominators::<Test>::contains_key(71));
 			assert!(Nominators::<Test>::contains_key(61));
 			assert!(Nominators::<Test>::get(71).is_none());
@@ -6148,6 +6263,7 @@ fn change_of_absolute_max_nominations() {
 			assert!(Nominators::<Test>::contains_key(101));
 			assert!(Nominators::<Test>::get(101).is_none());
 			assert_ok!(Staking::chill_other(RuntimeOrigin::signed(71), 101));
+			assert_eq!(*staking_events().last().unwrap(), Event::Chilled { stash: 101 });
 			assert!(!Nominators::<Test>::contains_key(101));
 			assert!(Nominators::<Test>::get(101).is_none());
 		})