stake: revoke nomination => leave nominators if no nominations left (paritytech#294)

* done

* bump spec version

* make if else into ret early on edge case

* update comments

* update test (paritytech#296)

* update test

* update runtime

Co-authored-by: Antoine Estienne <[email protected]>

Author: 4meta5

pallets/parachain-staking/src/lib.rs

@@ -1079,9 +1079,18 @@ pub mod pallet {
 			collator: T::AccountId,
 		) -> DispatchResultWithPostInfo {
 			let mut nominator = <NominatorState<T>>::get(&acc).ok_or(Error::<T>::NominatorDNE)?;
+			let old_total = nominator.total;
 			let remaining = nominator
 				.rm_nomination(collator.clone())
 				.ok_or(Error::<T>::NominationDNE)?;
+			// edge case; if no nominations remaining, leave set of nominators
+			if nominator.nominations.0.len().is_zero() {
+				// leave the set of nominators because no nominations left
+				Self::nominator_leaves_collator(acc.clone(), collator)?;
+				<NominatorState<T>>::remove(&acc);
+				Self::deposit_event(Event::NominatorLeft(acc, old_total));
+				return Ok(().into());
+			}
 			ensure!(
 				remaining >= T::MinNominatorStk::get(),
 				Error::<T>::NomBondBelowMin

pallets/parachain-staking/src/tests.rs

@@ -735,21 +735,25 @@ fn revoke_nomination_or_leave_nominators() {
 			Stake::revoke_nomination(Origin::signed(6), 2),
 			Error::<Test>::NominationDNE
 		);
-		// must leave set of nominators if total bonds below MinNominatorStk
-		assert_noop!(
-			Stake::revoke_nomination(Origin::signed(6), 1),
-			Error::<Test>::NomBondBelowMin
-		);
 		assert_noop!(
 			Stake::leave_nominators(Origin::signed(1)),
 			Error::<Test>::NominatorDNE
 		);
-		assert_ok!(Stake::leave_nominators(Origin::signed(6)));
+		assert_ok!(Stake::nominate(Origin::signed(6), 2, 3));
+		assert_ok!(Stake::nominate(Origin::signed(6), 3, 3));
+		assert_ok!(Stake::revoke_nomination(Origin::signed(6), 1));
+		// cannot revoke nomination because would leave remaining total below MinNominatorStk
+		assert_noop!(
+			Stake::revoke_nomination(Origin::signed(6), 2),
+			Error::<Test>::NomBondBelowMin
+		);
 		assert_noop!(
-			Stake::revoke_nomination(Origin::signed(8), 2),
+			Stake::revoke_nomination(Origin::signed(6), 3),
 			Error::<Test>::NomBondBelowMin
 		);
-		assert_ok!(Stake::nominate(Origin::signed(8), 1, 10));
+		// can revoke both remaining by calling leave nominators
+		assert_ok!(Stake::leave_nominators(Origin::signed(6)));
+		// this leads to 8 leaving set of nominators
 		assert_ok!(Stake::revoke_nomination(Origin::signed(8), 2));
 	});
 }

runtime/src/lib.rs

@@ -385,15 +385,15 @@ parameter_types! {
 	pub const MinBlocksPerRound: u32 = 20;
 	/// Default BlocksPerRound is every hour (600 * 6 second block times)
 	pub const DefaultBlocksPerRound: u32 = 600;
-	/// Reward payments and validator exit requests are delayed by 2 hours (2 * 600 * block_time)
+	/// Reward payments and collator exit requests are delayed by 2 hours (2 * 600 * block_time)
 	pub const BondDuration: u32 = 2;
-	/// Maximum 8 valid block authors at any given time
+	/// Minimum 8 collators selected per round, default at genesis and minimum forever after
 	pub const MinSelectedCandidates: u32 = 8;
-	/// Maximum 10 nominators per validator
+	/// Maximum 10 nominators per collator
 	pub const MaxNominatorsPerCollator: u32 = 10;
-	/// The maximum percent a validator can take off the top of its rewards is 50%
+	/// The maximum percent a collator can take off the top of its rewards is 50%
 	pub const MaxFee: Perbill = Perbill::from_percent(50);
-	/// Minimum stake required to be reserved to be a validator is 1_000
+	/// Minimum stake required to be reserved to be a collator is 1_000
 	pub const MinCollatorStk: u128 = 1_000 * GLMR;
 	/// Minimum stake required to be reserved to be a nominator is 5
 	pub const MinNominatorStk: u128 = 5 * GLMR;

tools/staking-test-spec.json

@@ -185,7 +185,8 @@ async function test() {
 
   // Revoke Nomination
   const unsub3 = await polkadotApi.tx.parachainStaking
-    .leaveNominators()
+    .revokeNomination(GERALD) //TODO: when converting to test add .leaveNominators()
+    // that should produce the same behavior
     .signAndSend(alith, ({ events = [], status }) => {
       console.log(`Current status is ${status.type}`);