nomination-pools: add permissionless condition to chill (#3453)

Currently, pool member funds cannot be unbonded if the depositor's stake
is less than `MinNominatorBond`. This usually happens after
`T::MinNominatorBond` is increased.

To fix this, the above mentioned condition is added as a case for
permissionless dispatch of `chill`. After pool is chilled, pool members
can unbond their funds since pool won't be nominating anymore.

Consequently, same check is added to `nominate` call, i.e pool can not
start nominating if it's depositor does not have `MinNominatorBond`

cc @Ank4n @kianenigma 

closes #2350

Polkadot address: 16FqwPZ8GRC5U5D4Fu7W33nA55ZXzXGWHwmbnE1eT6pxuqcT

---------

Co-authored-by: Ankan <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: command-bot <>

prdoc/pr_3453.prdoc

@@ -0,0 +1,13 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[pallet-nomination-pools]: `chill` is permissionless if depositor's stake is less than `min_nominator_bond`"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Nomination pools currently have an issue whereby member funds cannot be unbonded if the depositor bonded amount is less than `MinNominatorBond`.
+      This PR makes the `chill` function permissionless if this condition is met.
+      Consequently, `nominate` function also checks for `depositor` to have at least `MinNominatorBond`, and returns `MinimumBondNotMet` error if not.
+crates:
+  - name: pallet-nomination-pools

substrate/frame/nomination-pools/src/lib.rs

@@ -2401,6 +2401,11 @@ pub mod pallet {
 		///
 		/// This directly forward the call to the staking pallet, on behalf of the pool bonded
 		/// account.
+		///
+		/// # Note
+		///
+		/// In addition to a `root` or `nominator` role of `origin`, pool's depositor needs to have
+		/// at least `depositor_min_bond` in the pool to start nominating.
 		#[pallet::call_index(8)]
 		#[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))]
 		pub fn nominate(
@@ -2411,6 +2416,16 @@ pub mod pallet {
 			let who = ensure_signed(origin)?;
 			let bonded_pool = BondedPool::<T>::get(pool_id).ok_or(Error::<T>::PoolNotFound)?;
 			ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
+
+			let depositor_points = PoolMembers::<T>::get(&bonded_pool.roles.depositor)
+				.ok_or(Error::<T>::PoolMemberNotFound)?
+				.active_points();
+
+			ensure!(
+				bonded_pool.points_to_balance(depositor_points) >= Self::depositor_min_bond(),
+				Error::<T>::MinimumBondNotMet
+			);
+
 			T::Staking::nominate(&bonded_pool.bonded_account(), validators)
 		}
 
@@ -2573,17 +2588,37 @@ pub mod pallet {
 
 		/// Chill on behalf of the pool.
 		///
-		/// The dispatch origin of this call must be signed by the pool nominator or the pool
+		/// The dispatch origin of this call can be signed by the pool nominator or the pool
 		/// root role, same as [`Pallet::nominate`].
 		///
+		/// Under certain conditions, this call can be dispatched permissionlessly (i.e. by any
+		/// account).
+		///
+		/// # Conditions for a permissionless dispatch:
+		/// * When pool depositor has less than `MinNominatorBond` staked, otherwise  pool members
+		///   are unable to unbond.
+		///
+		/// # Conditions for permissioned dispatch:
+		/// * The caller has a nominator or root role of the pool.
 		/// This directly forward the call to the staking pallet, on behalf of the pool bonded
 		/// account.
 		#[pallet::call_index(13)]
 		#[pallet::weight(T::WeightInfo::chill())]
 		pub fn chill(origin: OriginFor<T>, pool_id: PoolId) -> DispatchResult {
 			let who = ensure_signed(origin)?;
+
 			let bonded_pool = BondedPool::<T>::get(pool_id).ok_or(Error::<T>::PoolNotFound)?;
-			ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
+
+			let depositor_points = PoolMembers::<T>::get(&bonded_pool.roles.depositor)
+				.ok_or(Error::<T>::PoolMemberNotFound)?
+				.active_points();
+
+			if bonded_pool.points_to_balance(depositor_points) >=
+				T::Staking::minimum_nominator_bond()
+			{
+				ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
+			}
+
 			T::Staking::chill(&bonded_pool.bonded_account())
 		}
 

substrate/frame/nomination-pools/src/tests.rs

@@ -4848,6 +4848,18 @@ mod nominate {
 				Error::<Runtime>::NotNominator
 			);
 
+			// if `depositor` stake is less than the `MinimumNominatorBond`, they can't nominate
+			StakingMinBond::set(20);
+
+			// Can't nominate if depositor's stake is less than the `MinimumNominatorBond`
+			assert_noop!(
+				Pools::nominate(RuntimeOrigin::signed(900), 1, vec![21]),
+				Error::<Runtime>::MinimumBondNotMet
+			);
+
+			// restore `MinimumNominatorBond`
+			StakingMinBond::set(10);
+
 			// Root can nominate
 			assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![21]));
 			assert_eq!(Nominations::get().unwrap(), vec![21]);
@@ -7338,3 +7350,33 @@ mod slash {
 		});
 	}
 }
+
+mod chill {
+	use super::*;
+
+	#[test]
+	fn chill_works() {
+		ExtBuilder::default().build_and_execute(|| {
+			// only nominator or root can chill
+			assert_noop!(
+				Pools::chill(RuntimeOrigin::signed(10), 1),
+				Error::<Runtime>::NotNominator
+			);
+
+			// root can chill and re-nominate
+			assert_ok!(Pools::chill(RuntimeOrigin::signed(900), 1));
+			assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![31]));
+
+			// nominator can chill and re-nominate
+			assert_ok!(Pools::chill(RuntimeOrigin::signed(901), 1));
+			assert_ok!(Pools::nominate(RuntimeOrigin::signed(901), 1, vec![31]));
+
+			// if `depositor` stake is less than the `MinimumNominatorBond`, then this call
+			// becomes permissionless;
+			StakingMinBond::set(20);
+
+			// any account can chill
+			assert_ok!(Pools::chill(RuntimeOrigin::signed(10), 1));
+		})
+	}
+}