Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nomination-pools: add permissionless condition to chill #3453

Merged
merged 22 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
bdfcaf1
Add permissionless condition to chill
dastansam Feb 23, 2024
7a6bdea
Add pr doc
dastansam Feb 23, 2024
7d15d4c
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 23, 2024
76751e0
Add same check in nomnate
dastansam Feb 23, 2024
7363ebe
Merge branch 'fix/nomination-pools-chill' of github-dastansam:dastans…
dastansam Feb 23, 2024
280e3ed
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 26, 2024
0cffe10
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 26, 2024
3830963
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 27, 2024
a0bff6b
Merge branch 'master' into fix/nomination-pools-chill
Ank4n Feb 28, 2024
80f0444
Add doc comment to nominate
dastansam Feb 28, 2024
84ec112
Merge branch 'fix/nomination-pools-chill' of github-dastansam:dastans…
dastansam Feb 28, 2024
08d33ee
Update doc
dastansam Feb 28, 2024
d2b21fe
Apply suggestions
dastansam Feb 28, 2024
186f5c3
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 29, 2024
319f884
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 29, 2024
0d871c0
Update substrate/frame/nomination-pools/src/lib.rs
dastansam Feb 29, 2024
94b0462
Merge branch 'master' of https://github.com/paritytech/polkadot-sdk i…
Feb 29, 2024
927e267
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
Feb 29, 2024
4a11e0f
Merge branch 'master' into fix/nomination-pools-chill
dastansam Feb 29, 2024
6a00aa3
Merge branch 'master' into fix/nomination-pools-chill
dastansam Mar 2, 2024
31df111
Merge branch 'master' into fix/nomination-pools-chill
Ank4n Mar 7, 2024
dc1ae31
".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime…
Mar 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions prdoc/pr_3453.prdoc
Original file line number Diff line number Diff line change
@@ -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
47 changes: 43 additions & 4 deletions substrate/frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1576,7 +1576,7 @@ pub mod pallet {
use frame_system::{ensure_signed, pallet_prelude::*};
use sp_runtime::Perbill;

/// The in-code storage version.
/// The current storage version.
dastansam marked this conversation as resolved.
Show resolved Hide resolved
const STORAGE_VERSION: StorageVersion = StorageVersion::new(8);

#[pallet::pallet]
Expand Down 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
dastansam marked this conversation as resolved.
Show resolved Hide resolved
);

T::Staking::nominate(&bonded_pool.bonded_account(), validators)
}

Expand Down Expand Up @@ -2573,18 +2588,42 @@ 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.
///
dastansam marked this conversation as resolved.
Show resolved Hide resolved
/// 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);
T::Staking::chill(&bonded_pool.bonded_account())

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()
{
T::Staking::chill(&bonded_pool.bonded_account())
} else {
ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
T::Staking::chill(&bonded_pool.bonded_account())
}
dastansam marked this conversation as resolved.
Show resolved Hide resolved
}

/// `origin` bonds funds from `extra` for some pool member `member` into their respective
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
Loading