Allow validators to block and kick their nominator set. (#7930)

* Allow validators to block and kick their nominator set.

* migration

* Test

* Better migration

* Fixes

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

* Update frame/staking/src/lib.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

frame/offences/benchmarking/src/lib.rs

@@ -109,6 +109,7 @@ fn create_offender<T: Config>(n: u32, nominators: u32) -> Result<Offender<T>, &'
 
 	let validator_prefs = ValidatorPrefs {
 		commission: Perbill::from_percent(50),
+		.. Default::default()
 	};
 	Staking::<T>::validate(RawOrigin::Signed(controller.clone()).into(), validator_prefs)?;
 

frame/staking/src/benchmarking.rs

@@ -63,6 +63,7 @@ pub fn create_validator_with_nominators<T: Config>(
 	let (v_stash, v_controller) = create_stash_controller::<T>(0, 100, destination.clone())?;
 	let validator_prefs = ValidatorPrefs {
 		commission: Perbill::from_percent(50),
+		.. Default::default()
 	};
 	Staking::<T>::validate(RawOrigin::Signed(v_controller).into(), validator_prefs)?;
 	let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(v_stash.clone());
@@ -198,6 +199,61 @@ benchmarks! {
 		assert!(Validators::<T>::contains_key(stash));
 	}
 
+	kick {
+		// scenario: we want to kick `k` nominators from nominating us (we are a validator).
+		// we'll assume that `k` is under 128 for the purposes of determining the slope.
+		// each nominator should have `MAX_NOMINATIONS` validators nominated, and our validator
+		// should be somewhere in there.
+		let k in 1 .. 128;
+
+		// these are the other validators; there are `MAX_NOMINATIONS - 1` of them, so there are a
+		// total of `MAX_NOMINATIONS` validators in the system.
+		let rest_of_validators = create_validators::<T>(MAX_NOMINATIONS as u32 - 1, 100)?;
+
+		// this is the validator that will be kicking.
+		let (stash, controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 - 1, 100, Default::default())?;
+		let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(stash.clone());
+
+		// they start validating.
+		Staking::<T>::validate(RawOrigin::Signed(controller.clone()).into(), Default::default())?;
+
+		// we now create the nominators. there will be `k` of them; each will nominate all
+		// validators. we will then kick each of the `k` nominators from the main validator.
+		let mut nominator_stashes = Vec::with_capacity(k as usize);
+		for i in 0 .. k {
+			// create a nominator stash.
+			let (n_stash, n_controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 + i, 100, Default::default())?;
+
+			// bake the nominations; we first clone them from the rest of the validators.
+			let mut nominations = rest_of_validators.clone();
+			// then insert "our" validator somewhere in there (we vary it) to avoid accidental
+			// optimisations/pessimisations.
+			nominations.insert(i as usize % (nominations.len() + 1), stash_lookup.clone());
+			// then we nominate.
+			Staking::<T>::nominate(RawOrigin::Signed(n_controller.clone()).into(), nominations)?;
+
+			nominator_stashes.push(n_stash);
+		}
+
+		// all nominators now should be nominating our validator...
+		for n in nominator_stashes.iter() {
+			assert!(Nominators::<T>::get(n).unwrap().targets.contains(&stash));
+		}
+
+		// we need the unlookuped version of the nominator stash for the kick.
+		let kicks = nominator_stashes.iter()
+			.map(|n| T::Lookup::unlookup(n.clone()))
+			.collect::<Vec<_>>();
+
+		whitelist_account!(controller);
+	}: _(RawOrigin::Signed(controller), kicks)
+	verify {
+		// all nominators now should *not* be nominating our validator...
+		for n in nominator_stashes.iter() {
+			assert!(!Nominators::<T>::get(n).unwrap().targets.contains(&stash));
+		}
+	}
+
 	// Worst case scenario, MAX_NOMINATIONS
 	nominate {
 		let n in 1 .. MAX_NOMINATIONS as u32;
@@ -814,6 +870,7 @@ mod tests {
 			assert_ok!(test_benchmark_withdraw_unbonded_update::<Test>());
 			assert_ok!(test_benchmark_withdraw_unbonded_kill::<Test>());
 			assert_ok!(test_benchmark_validate::<Test>());
+			assert_ok!(test_benchmark_kick::<Test>());
 			assert_ok!(test_benchmark_nominate::<Test>());
 			assert_ok!(test_benchmark_chill::<Test>());
 			assert_ok!(test_benchmark_set_payee::<Test>());

frame/staking/src/lib.rs

@@ -454,12 +454,17 @@ pub struct ValidatorPrefs {
 	/// nominators.
 	#[codec(compact)]
 	pub commission: Perbill,
+	/// Whether or not this validator is accepting more nominations. If `true`, then no nominator
+	/// who is not already nominating this validator may nominate them. By default, validators
+	/// are accepting nominations.
+	pub blocked: bool,
 }
 
 impl Default for ValidatorPrefs {
 	fn default() -> Self {
 		ValidatorPrefs {
 			commission: Default::default(),
+			blocked: false,
 		}
 	}
 }
@@ -896,11 +901,12 @@ enum Releases {
 	V2_0_0,
 	V3_0_0,
 	V4_0_0,
+	V5_0_0,
 }
 
 impl Default for Releases {
 	fn default() -> Self {
-		Releases::V4_0_0
+		Releases::V5_0_0
 	}
 }
 
@@ -1087,8 +1093,8 @@ decl_storage! {
 		/// True if network has been upgraded to this version.
 		/// Storage version of the pallet.
 		///
-		/// This is set to v3.0.0 for new networks.
-		StorageVersion build(|_: &GenesisConfig<T>| Releases::V4_0_0): Releases;
+		/// This is set to v5.0.0 for new networks.
+		StorageVersion build(|_: &GenesisConfig<T>| Releases::V5_0_0): Releases;
 	}
 	add_extra_genesis {
 		config(stakers):
@@ -1124,6 +1130,29 @@ decl_storage! {
 	}
 }
 
+pub mod migrations {
+	use super::*;
+
+	#[derive(Decode)]
+	struct OldValidatorPrefs {
+		#[codec(compact)]
+		pub commission: Perbill
+	}
+	impl OldValidatorPrefs {
+		fn upgraded(self) -> ValidatorPrefs {
+			ValidatorPrefs {
+				commission: self.commission,
+				.. Default::default()
+			}
+		}
+	}
+	pub fn migrate_to_blockable<T: Config>() -> frame_support::weights::Weight {
+		Validators::<T>::translate::<OldValidatorPrefs, _>(|_, p| Some(p.upgraded()));
+		ErasValidatorPrefs::<T>::translate::<OldValidatorPrefs, _>(|_, _, p| Some(p.upgraded()));
+		T::BlockWeights::get().max_block
+	}
+}
+
 decl_event!(
 	pub enum Event<T> where Balance = BalanceOf<T>, <T as frame_system::Config>::AccountId {
 		/// The era payout has been set; the first balance is the validator-payout; the second is
@@ -1152,6 +1181,8 @@ decl_event!(
 		/// An account has called `withdraw_unbonded` and removed unbonding chunks worth `Balance`
 		/// from the unlocking queue. \[stash, amount\]
 		Withdrawn(AccountId, Balance),
+		/// A nominator has been kicked from a validator. \[nominator, stash\]
+		Kicked(AccountId, AccountId),
 	}
 );
 
@@ -1225,6 +1256,10 @@ decl_error! {
 		IncorrectSlashingSpans,
 		/// Internal state has become somehow corrupted and the operation cannot continue.
 		BadState,
+		/// Too many nomination targets supplied.
+		TooManyTargets,
+		/// A nomination target was supplied that was blocked or otherwise not a validator.
+		BadTarget,
 	}
 }
 
@@ -1270,6 +1305,15 @@ decl_module! {
 
 		fn deposit_event() = default;
 
+		fn on_runtime_upgrade() -> frame_support::weights::Weight {
+			if StorageVersion::get() == Releases::V4_0_0 {
+				StorageVersion::put(Releases::V5_0_0);
+				migrations::migrate_to_blockable::<T>()
+			} else {
+				0
+			}
+		}
+
 		/// sets `ElectionStatus` to `Open(now)` where `now` is the block number at which the
 		/// election window has opened, if we are at the last session and less blocks than
 		/// `T::ElectionLookahead` is remaining until the next new session schedule. The offchain
@@ -1675,9 +1719,17 @@ decl_module! {
 			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
 			let stash = &ledger.stash;
 			ensure!(!targets.is_empty(), Error::<T>::EmptyTargets);
+			ensure!(targets.len() <= MAX_NOMINATIONS, Error::<T>::TooManyTargets);
+
+			let old = Nominators::<T>::get(stash).map_or_else(Vec::new, |x| x.targets);
+
 			let targets = targets.into_iter()
-				.take(MAX_NOMINATIONS)
-				.map(|t| T::Lookup::lookup(t))
+				.map(|t| T::Lookup::lookup(t).map_err(DispatchError::from))
+				.map(|n| n.and_then(|n| if old.contains(&n) || !Validators::<T>::get(&n).blocked {
+					Ok(n)
+				} else {
+					Err(Error::<T>::BadTarget.into())
+				}))
 				.collect::<result::Result<Vec<T::AccountId>, _>>()?;
 
 			let nominations = Nominations {
@@ -2168,6 +2220,42 @@ decl_module! {
 
 			Ok(adjustments)
 		}
+
+		/// Remove the given nominations from the calling validator.
+		///
+		/// Effects will be felt at the beginning of the next era.
+		///
+		/// The dispatch origin for this call must be _Signed_ by the controller, not the stash.
+		/// And, it can be only called when [`EraElectionStatus`] is `Closed`. The controller
+		/// account should represent a validator.
+		///
+		/// - `who`: A list of nominator stash accounts who are nominating this validator which
+		///   should no longer be nominating this validator.
+		///
+		/// Note: Making this call only makes sense if you first set the validator preferences to
+		/// block any further nominations.
+		#[weight = T::WeightInfo::kick(who.len() as u32)]
+		pub fn kick(origin, who: Vec<<T::Lookup as StaticLookup>::Source>) -> DispatchResult {
+			let controller = ensure_signed(origin)?;
+			ensure!(Self::era_election_status().is_closed(), Error::<T>::CallNotAllowed);
+			let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
+			let stash = &ledger.stash;
+
+			for nom_stash in who.into_iter()
+				.map(T::Lookup::lookup)
+				.collect::<Result<Vec<T::AccountId>, _>>()?
+				.into_iter()
+			{
+				Nominators::<T>::mutate(&nom_stash, |maybe_nom| if let Some(ref mut nom) = maybe_nom {
+					if let Some(pos) = nom.targets.iter().position(|v| v == stash) {
+						nom.targets.swap_remove(pos);
+						Self::deposit_event(RawEvent::Kicked(nom_stash.clone(), stash.clone()));
+					}
+				});
+			}
+
+			Ok(())
+		}
 	}
 }
 

frame/staking/src/offchain_election.rs

@@ -520,6 +520,9 @@ mod test {
 		fn submit_solution_better(v: u32, n: u32, a: u32, w: u32) -> Weight {
 			(0 * v + 0 * n + 1000 * a + 0 * w) as Weight
 		}
+		fn kick(w: u32) -> Weight {
+			unimplemented!()
+		}
 	}
 
 	#[test]

frame/staking/src/testing_utils.rs

@@ -92,6 +92,7 @@ pub fn create_validators<T: Config>(
 		let (stash, controller) = create_stash_controller::<T>(i, balance_factor, RewardDestination::Staked)?;
 		let validator_prefs = ValidatorPrefs {
 			commission: Perbill::from_percent(50),
+			.. Default::default()
 		};
 		Staking::<T>::validate(RawOrigin::Signed(controller).into(), validator_prefs)?;
 		let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(stash);
@@ -134,6 +135,7 @@ pub fn create_validators_with_nominators_for_era<T: Config>(
 		let (v_stash, v_controller) = create_stash_controller::<T>(i, balance_factor, RewardDestination::Staked)?;
 		let validator_prefs = ValidatorPrefs {
 			commission: Perbill::from_percent(50),
+			.. Default::default()
 		};
 		Staking::<T>::validate(RawOrigin::Signed(v_controller.clone()).into(), validator_prefs)?;
 		let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(v_stash.clone());

frame/staking/src/tests.rs

@@ -364,6 +364,30 @@ fn staking_should_work() {
 		});
 }
 
+#[test]
+fn blocking_and_kicking_works() {
+	ExtBuilder::default()
+		.minimum_validator_count(1)
+		.validator_count(4)
+		.nominate(true)
+		.num_validators(3)
+		.build()
+		.execute_with(|| {
+			// block validator 10/11
+			assert_ok!(Staking::validate(Origin::signed(10), ValidatorPrefs { blocked: true, .. Default::default() }));
+			// attempt to nominate from 100/101...
+			assert_ok!(Staking::nominate(Origin::signed(100), vec![11]));
+			// should have worked since we're already nominated them
+			assert_eq!(Nominators::<Test>::get(&101).unwrap().targets, vec![11]);
+			// kick the nominator
+			assert_ok!(Staking::kick(Origin::signed(10), vec![101]));
+			// should have been kicked now
+			assert!(Nominators::<Test>::get(&101).unwrap().targets.is_empty());
+			// attempt to nominate from 100/101...
+			assert_noop!(Staking::nominate(Origin::signed(100), vec![11]), Error::<Test>::BadTarget);
+		});
+}
+
 #[test]
 fn less_than_needed_candidates_works() {
 	ExtBuilder::default()
@@ -403,7 +427,7 @@ fn no_candidate_emergency_condition() {
 		.execute_with(|| {
 			// initial validators
 			assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]);
-			let prefs = ValidatorPrefs { commission: Perbill::one() };
+			let prefs = ValidatorPrefs { commission: Perbill::one(), .. Default::default() };
 			<Staking as crate::Store>::Validators::insert(11, prefs.clone());
 
 			// set the minimum validator count.
@@ -971,6 +995,7 @@ fn validator_payment_prefs_work() {
 		let commission = Perbill::from_percent(40);
 		<Validators<Test>>::insert(&11, ValidatorPrefs {
 			commission: commission.clone(),
+			.. Default::default()
 		});
 
 		// Reward controller so staked ratio doesn't change.