Allow nomination pools to chill + fix dismantle scenario (#11426)

* make pool roles optional

* undo lock file changes?

* add migration

* add the ability for pools to chill themselves

* boilerplate of tests

* somewhat stable, but I think I found another bug as well

* Fix it all

* Add more more sophisticated test + capture one more bug.

* Update frame/staking/src/lib.rs

* reduce the diff a little bit

* add some test for the slashing bug

* cleanup

* fix lock file?

* Fix

* fmt

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/nomination-pools/src/mock.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix build

* fix some fishy tests..

* add one last integrity check for MinCreateBond

* remove bad assertion -- needs to be dealt with later

* nits

* fix tests and add benchmarks for chill

* remove stuff

* fix benchmarks

* cargo run --quiet --profile=production  --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark pallet --chain=dev --steps=50 --repeat=20 --pallet=pallet_nomination_pools --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/nomination-pools/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* remove defensive

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Parity Bot <admin@parity.io>

substrate/frame/staking/src/lib.rs

@@ -438,7 +438,7 @@ pub struct UnlockChunk<Balance: HasCompact> {
 }
 
 /// The ledger of a (bonded) stash.
-#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo)]
 #[scale_info(skip_type_params(T))]
 pub struct StakingLedger<T: Config> {
 	/// The stash account whose balance is actually locked and at stake.
@@ -607,12 +607,12 @@ impl<T: Config> StakingLedger<T> {
 		let mut slashed_unlocking = BTreeMap::<_, _>::new();
 		for i in slash_chunks_priority {
 			if let Some(chunk) = self.unlocking.get_mut(i).defensive() {
-				slash_out_of(&mut chunk.value, &mut remaining_slash);
-				// write the new slashed value of this chunk to the map.
-				slashed_unlocking.insert(chunk.era, chunk.value);
 				if remaining_slash.is_zero() {
 					break
 				}
+				slash_out_of(&mut chunk.value, &mut remaining_slash);
+				// write the new slashed value of this chunk to the map.
+				slashed_unlocking.insert(chunk.era, chunk.value);
 			} else {
 				break
 			}

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

@@ -1414,17 +1414,17 @@ impl<T: Config> StakingInterface for Pallet<T> {
 		Self::unbond(RawOrigin::Signed(controller).into(), value)
 	}
 
+	fn chill(controller: Self::AccountId) -> DispatchResult {
+		Self::chill(RawOrigin::Signed(controller).into())
+	}
+
 	fn withdraw_unbonded(
 		controller: Self::AccountId,
 		num_slashing_spans: u32,
-	) -> Result<u64, DispatchError> {
-		Self::withdraw_unbonded(RawOrigin::Signed(controller).into(), num_slashing_spans)
-			.map(|post_info| {
-				post_info
-					.actual_weight
-					.unwrap_or(T::WeightInfo::withdraw_unbonded_kill(num_slashing_spans))
-			})
-			.map_err(|err_with_post_info| err_with_post_info.error)
+	) -> Result<bool, DispatchError> {
+		Self::withdraw_unbonded(RawOrigin::Signed(controller.clone()).into(), num_slashing_spans)
+			.map(|_| !Ledger::<T>::contains_key(&controller))
+			.map_err(|with_post| with_post.error)
 	}
 
 	fn bond(
@@ -1445,4 +1445,9 @@ impl<T: Config> StakingInterface for Pallet<T> {
 		let targets = targets.into_iter().map(T::Lookup::unlookup).collect::<Vec<_>>();
 		Self::nominate(RawOrigin::Signed(controller).into(), targets)
 	}
+
+	#[cfg(feature = "runtime-benchmarks")]
+	fn nominations(who: Self::AccountId) -> Option<Vec<T::AccountId>> {
+		Nominators::<T>::get(who).map(|n| n.targets.into_inner())
+	}
 }

substrate/frame/staking/src/tests.rs

@@ -4930,6 +4930,25 @@ fn force_apply_min_commission_works() {
 	});
 }
 
+#[test]
+fn proportional_slash_stop_slashing_if_remaining_zero() {
+	let c = |era, value| UnlockChunk::<Balance> { era, value };
+	// Given
+	let mut ledger = StakingLedger::<Test> {
+		stash: 123,
+		total: 40,
+		active: 20,
+		// we have some chunks, but they are not affected.
+		unlocking: bounded_vec![c(1, 10), c(2, 10)],
+		claimed_rewards: vec![],
+	};
+
+	assert_eq!(BondingDuration::get(), 3);
+
+	// should not slash more than the amount requested, by accidentally slashing the first chunk.
+	assert_eq!(ledger.slash(18, 1, 0), 18);
+}
+
 #[test]
 fn proportional_ledger_slash_works() {
 	let c = |era, value| UnlockChunk::<Balance> { era, value };