Skip to content

Commit

Permalink
nomination-pools: add permissionless condition to chill (parityte…
Browse files Browse the repository at this point in the history
…ch#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 paritytech#2350

Polkadot address: 16FqwPZ8GRC5U5D4Fu7W33nA55ZXzXGWHwmbnE1eT6pxuqcT

---------

Co-authored-by: Ankan <[email protected]>
Co-authored-by: Gonçalo Pestana <[email protected]>
Co-authored-by: command-bot <>
  • Loading branch information
3 people authored Mar 7, 2024
1 parent 4d07d57 commit 354c20a
Show file tree
Hide file tree
Showing 4 changed files with 354 additions and 128 deletions.
39 changes: 37 additions & 2 deletions substrate/frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
}

Expand Down Expand Up @@ -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())
}

Expand Down
42 changes: 42 additions & 0 deletions substrate/frame/nomination-pools/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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));
})
}
}
Loading

0 comments on commit 354c20a

Please sign in to comment.