Skip to content

Commit fe46832

Browse files
dharjeezyAnk4ngpestana
authored
Full Unbond in Staking (#3811)
closes #414 Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW --------- Co-authored-by: Ankan <[email protected]> Co-authored-by: command-bot <> Co-authored-by: Gonçalo Pestana <[email protected]> Co-authored-by: Ankan <[email protected]>
1 parent 1b76f99 commit fe46832

File tree

6 files changed

+172
-104
lines changed

6 files changed

+172
-104
lines changed

prdoc/pr_3811.prdoc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
title: "Implicit chill when full unbounding in [pallet_staking]"
2+
3+
doc:
4+
- audience: Runtime Dev
5+
description: |
6+
Modifies the `unbond` extrinsic to forcefully chill stash when unbonding
7+
if the value to be unbonded is the total of what is bonded
8+
9+
crates:
10+
- name: pallet-staking
11+
bump: minor

substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ mod mock;
2323

2424
pub(crate) const LOG_TARGET: &str = "tests::e2e-epm";
2525

26-
use frame_support::{assert_err, assert_noop, assert_ok};
26+
use frame_support::{assert_err, assert_ok};
2727
use mock::*;
2828
use pallet_timestamp::Now;
2929
use sp_core::Get;
@@ -253,18 +253,8 @@ fn ledger_consistency_active_balance_below_ed() {
253253
execute_with(ext, || {
254254
assert_eq!(Staking::ledger(11.into()).unwrap().active, 1000);
255255

256-
// unbonding total of active stake fails because the active ledger balance would fall
257-
// below the `MinNominatorBond`.
258-
assert_noop!(
259-
Staking::unbond(RuntimeOrigin::signed(11), 1000),
260-
Error::<Runtime>::InsufficientBond
261-
);
262-
263-
// however, chilling works as expected.
264-
assert_ok!(Staking::chill(RuntimeOrigin::signed(11)));
265-
266-
// now unbonding the full active balance works, since remainder of the active balance is
267-
// not enforced to be below `MinNominatorBond` if the stash has been chilled.
256+
// unbonding total of active stake passes because chill occurs implicitly when unbonding
257+
// full amount.
268258
assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1000));
269259

270260
// the active balance of the ledger entry is 0, while total balance is 1000 until

substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,6 @@ fn pool_lifecycle_e2e() {
164164
]
165165
);
166166

167-
// as soon as all members have left, the depositor can try to unbond, but since the
168-
// min-nominator intention is set, they must chill first.
169-
assert_noop!(
170-
Pools::unbond(RuntimeOrigin::signed(10), 10, 50),
171-
pallet_staking::Error::<Runtime>::InsufficientBond
172-
);
173-
174-
assert_ok!(Pools::chill(RuntimeOrigin::signed(10), 1));
175167
assert_ok!(Pools::unbond(RuntimeOrigin::signed(10), 10, 50));
176168

177169
assert_eq!(
@@ -183,10 +175,7 @@ fn pool_lifecycle_e2e() {
183175
);
184176
assert_eq!(
185177
pool_events_since_last_call(),
186-
vec![
187-
PoolsEvent::PoolNominatorChilled { pool_id: 1, caller: 10 },
188-
PoolsEvent::Unbonded { member: 10, pool_id: 1, points: 50, balance: 50, era: 6 }
189-
]
178+
vec![PoolsEvent::Unbonded { member: 10, pool_id: 1, points: 50, balance: 50, era: 6 },]
190179
);
191180

192181
// waiting another bonding duration:

substrate/frame/staking/src/pallet/impls.rs

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ use crate::{
5353
BalanceOf, EraInfo, EraPayout, Existence, ExistenceOrLegacyExposure, Exposure, Forcing,
5454
IndividualExposure, LedgerIntegrityState, MaxNominationsOf, MaxWinnersOf, Nominations,
5555
NominationsQuota, PositiveImbalanceOf, RewardDestination, SessionInterface, StakingLedger,
56-
ValidatorPrefs, STAKING_ID,
56+
UnlockChunk, ValidatorPrefs, STAKING_ID,
5757
};
5858
use alloc::{boxed::Box, vec, vec::Vec};
5959

@@ -1376,6 +1376,92 @@ impl<T: Config> Pallet<T> {
13761376

13771377
consumed_weight
13781378
}
1379+
1380+
/// Unbonds a controller.
1381+
pub(crate) fn do_unbond(
1382+
controller: T::AccountId,
1383+
value: BalanceOf<T>,
1384+
) -> Result<Option<Weight>, DispatchError> {
1385+
let unlocking = Self::ledger(Controller(controller.clone())).map(|l| l.unlocking.len())?;
1386+
1387+
// if there are no unlocking chunks available, try to withdraw chunks older than
1388+
// `BondingDuration` to proceed with the unbonding.
1389+
let maybe_withdraw_weight = {
1390+
if unlocking == T::MaxUnlockingChunks::get() as usize {
1391+
let real_num_slashing_spans =
1392+
SlashingSpans::<T>::get(&controller).map_or(0, |s| s.iter().count());
1393+
Some(Self::do_withdraw_unbonded(&controller, real_num_slashing_spans as u32)?)
1394+
} else {
1395+
None
1396+
}
1397+
};
1398+
1399+
// we need to fetch the ledger again because it may have been mutated in the call
1400+
// to `Self::do_withdraw_unbonded` above.
1401+
let mut ledger = Self::ledger(Controller(controller))?;
1402+
let mut value = value.min(ledger.active);
1403+
let stash = ledger.stash.clone();
1404+
1405+
ensure!(
1406+
ledger.unlocking.len() < T::MaxUnlockingChunks::get() as usize,
1407+
Error::<T>::NoMoreChunks,
1408+
);
1409+
1410+
if !value.is_zero() {
1411+
ledger.active -= value;
1412+
1413+
// Avoid there being a dust balance left in the staking system.
1414+
if ledger.active < asset::existential_deposit::<T>() {
1415+
value += ledger.active;
1416+
ledger.active = Zero::zero();
1417+
}
1418+
1419+
let min_active_bond = if Nominators::<T>::contains_key(&stash) {
1420+
MinNominatorBond::<T>::get()
1421+
} else if Validators::<T>::contains_key(&stash) {
1422+
MinValidatorBond::<T>::get()
1423+
} else {
1424+
Zero::zero()
1425+
};
1426+
1427+
// Make sure that the user maintains enough active bond for their role.
1428+
// If a user runs into this error, they should chill first.
1429+
ensure!(ledger.active >= min_active_bond, Error::<T>::InsufficientBond);
1430+
1431+
// Note: in case there is no current era it is fine to bond one era more.
1432+
let era = CurrentEra::<T>::get()
1433+
.unwrap_or(0)
1434+
.defensive_saturating_add(T::BondingDuration::get());
1435+
if let Some(chunk) = ledger.unlocking.last_mut().filter(|chunk| chunk.era == era) {
1436+
// To keep the chunk count down, we only keep one chunk per era. Since
1437+
// `unlocking` is a FiFo queue, if a chunk exists for `era` we know that it will
1438+
// be the last one.
1439+
chunk.value = chunk.value.defensive_saturating_add(value)
1440+
} else {
1441+
ledger
1442+
.unlocking
1443+
.try_push(UnlockChunk { value, era })
1444+
.map_err(|_| Error::<T>::NoMoreChunks)?;
1445+
};
1446+
// NOTE: ledger must be updated prior to calling `Self::weight_of`.
1447+
ledger.update()?;
1448+
1449+
// update this staker in the sorted list, if they exist in it.
1450+
if T::VoterList::contains(&stash) {
1451+
let _ = T::VoterList::on_update(&stash, Self::weight_of(&stash)).defensive();
1452+
}
1453+
1454+
Self::deposit_event(Event::<T>::Unbonded { stash, amount: value });
1455+
}
1456+
1457+
let actual_weight = if let Some(withdraw_weight) = maybe_withdraw_weight {
1458+
Some(T::WeightInfo::unbond().saturating_add(withdraw_weight))
1459+
} else {
1460+
Some(T::WeightInfo::unbond())
1461+
};
1462+
1463+
Ok(actual_weight)
1464+
}
13791465
}
13801466

13811467
impl<T: Config> Pallet<T> {

substrate/frame/staking/src/pallet/mod.rs

Lines changed: 15 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use frame_support::{
2929
hold::{Balanced as FunHoldBalanced, Mutate as FunHoldMutate},
3030
Mutate as FunMutate,
3131
},
32-
Contains, Defensive, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get,
33-
InspectLockableCurrency, Nothing, OnUnbalanced, UnixTime,
32+
Contains, Defensive, EnsureOrigin, EstimateNextNewSession, Get, InspectLockableCurrency,
33+
Nothing, OnUnbalanced, UnixTime,
3434
},
3535
weights::Weight,
3636
BoundedVec,
@@ -1240,6 +1240,8 @@ pub mod pallet {
12401240
/// period ends. If this leaves an amount actively bonded less than
12411241
/// [`asset::existential_deposit`], then it is increased to the full amount.
12421242
///
1243+
/// The stash may be chilled if the ledger total amount falls to 0 after unbonding.
1244+
///
12431245
/// The dispatch origin for this call must be _Signed_ by the controller, not the stash.
12441246
///
12451247
/// Once the unlock period is done, you can call `withdraw_unbonded` to actually move
@@ -1257,93 +1259,28 @@ pub mod pallet {
12571259
/// See also [`Call::withdraw_unbonded`].
12581260
#[pallet::call_index(2)]
12591261
#[pallet::weight(
1260-
T::WeightInfo::withdraw_unbonded_kill(SPECULATIVE_NUM_SPANS).saturating_add(T::WeightInfo::unbond()))
1262+
T::WeightInfo::withdraw_unbonded_kill(SPECULATIVE_NUM_SPANS).saturating_add(T::WeightInfo::unbond()).saturating_add(T::WeightInfo::chill()))
12611263
]
12621264
pub fn unbond(
12631265
origin: OriginFor<T>,
12641266
#[pallet::compact] value: BalanceOf<T>,
12651267
) -> DispatchResultWithPostInfo {
12661268
let controller = ensure_signed(origin)?;
1267-
let unlocking =
1268-
Self::ledger(Controller(controller.clone())).map(|l| l.unlocking.len())?;
1269-
1270-
// if there are no unlocking chunks available, try to withdraw chunks older than
1271-
// `BondingDuration` to proceed with the unbonding.
1272-
let maybe_withdraw_weight = {
1273-
if unlocking == T::MaxUnlockingChunks::get() as usize {
1274-
let real_num_slashing_spans =
1275-
SlashingSpans::<T>::get(&controller).map_or(0, |s| s.iter().count());
1276-
Some(Self::do_withdraw_unbonded(&controller, real_num_slashing_spans as u32)?)
1277-
} else {
1278-
None
1279-
}
1280-
};
12811269

1282-
// we need to fetch the ledger again because it may have been mutated in the call
1283-
// to `Self::do_withdraw_unbonded` above.
1284-
let mut ledger = Self::ledger(Controller(controller))?;
1285-
let mut value = value.min(ledger.active);
1286-
let stash = ledger.stash.clone();
1287-
1288-
ensure!(
1289-
ledger.unlocking.len() < T::MaxUnlockingChunks::get() as usize,
1290-
Error::<T>::NoMoreChunks,
1291-
);
1292-
1293-
if !value.is_zero() {
1294-
ledger.active -= value;
1295-
1296-
// Avoid there being a dust balance left in the staking system.
1297-
if ledger.active < asset::existential_deposit::<T>() {
1298-
value += ledger.active;
1299-
ledger.active = Zero::zero();
1300-
}
1301-
1302-
let min_active_bond = if Nominators::<T>::contains_key(&stash) {
1303-
MinNominatorBond::<T>::get()
1304-
} else if Validators::<T>::contains_key(&stash) {
1305-
MinValidatorBond::<T>::get()
1306-
} else {
1307-
Zero::zero()
1308-
};
1309-
1310-
// Make sure that the user maintains enough active bond for their role.
1311-
// If a user runs into this error, they should chill first.
1312-
ensure!(ledger.active >= min_active_bond, Error::<T>::InsufficientBond);
1313-
1314-
// Note: in case there is no current era it is fine to bond one era more.
1315-
let era = CurrentEra::<T>::get()
1316-
.unwrap_or(0)
1317-
.defensive_saturating_add(T::BondingDuration::get());
1318-
if let Some(chunk) = ledger.unlocking.last_mut().filter(|chunk| chunk.era == era) {
1319-
// To keep the chunk count down, we only keep one chunk per era. Since
1320-
// `unlocking` is a FiFo queue, if a chunk exists for `era` we know that it will
1321-
// be the last one.
1322-
chunk.value = chunk.value.defensive_saturating_add(value)
1323-
} else {
1324-
ledger
1325-
.unlocking
1326-
.try_push(UnlockChunk { value, era })
1327-
.map_err(|_| Error::<T>::NoMoreChunks)?;
1328-
};
1329-
// NOTE: ledger must be updated prior to calling `Self::weight_of`.
1330-
ledger.update()?;
1331-
1332-
// update this staker in the sorted list, if they exist in it.
1333-
if T::VoterList::contains(&stash) {
1334-
let _ = T::VoterList::on_update(&stash, Self::weight_of(&stash)).defensive();
1335-
}
1336-
1337-
Self::deposit_event(Event::<T>::Unbonded { stash, amount: value });
1338-
}
1270+
let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?;
13391271

1340-
let actual_weight = if let Some(withdraw_weight) = maybe_withdraw_weight {
1341-
Some(T::WeightInfo::unbond().saturating_add(withdraw_weight))
1272+
let mut total_weight = if value >= ledger.total {
1273+
Self::chill_stash(&ledger.stash);
1274+
T::WeightInfo::chill()
13421275
} else {
1343-
Some(T::WeightInfo::unbond())
1276+
Zero::zero()
13441277
};
13451278

1346-
Ok(actual_weight.into())
1279+
if let Some(withdraw_weight) = Self::do_unbond(controller, value)? {
1280+
total_weight.saturating_accrue(withdraw_weight);
1281+
}
1282+
1283+
Ok(Some(total_weight).into())
13471284
}
13481285

13491286
/// Remove any unlocked chunks from the `unlocking` queue from our management.

substrate/frame/staking/src/tests.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3638,6 +3638,61 @@ fn test_multi_page_payout_stakers_by_page() {
36383638
});
36393639
}
36403640

3641+
#[test]
3642+
fn unbond_with_chill_works() {
3643+
// Should test:
3644+
// * Given a bunded account
3645+
// * it can full unbond all portion of its funds from the stash account.
3646+
ExtBuilder::default().nominate(false).build_and_execute(|| {
3647+
// Set payee to stash.
3648+
assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Stash));
3649+
3650+
// Give account 11 some large free balance greater than total
3651+
let _ = Balances::make_free_balance_be(&11, 1000000);
3652+
3653+
// confirm that 10 is a normal validator and gets paid at the end of the era.
3654+
mock::start_active_era(1);
3655+
3656+
// Initial state of 11
3657+
assert_eq!(
3658+
Staking::ledger(11.into()).unwrap(),
3659+
StakingLedgerInspect {
3660+
stash: 11,
3661+
total: 1000,
3662+
active: 1000,
3663+
unlocking: Default::default(),
3664+
legacy_claimed_rewards: bounded_vec![],
3665+
}
3666+
);
3667+
3668+
assert!(Validators::<Test>::contains_key(11));
3669+
3670+
mock::start_active_era(2);
3671+
assert_eq!(active_era(), 2);
3672+
3673+
assert_eq!(Validators::<Test>::count(), 3);
3674+
3675+
// Unbond all amount by ensuring chilling
3676+
assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1000));
3677+
3678+
assert!(matches!(
3679+
staking_events_since_last_call().as_slice(),
3680+
&[
3681+
Event::StakersElected,
3682+
Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 },
3683+
Event::StakersElected,
3684+
Event::EraPaid { era_index: 1, validator_payout: 11075, remainder: 33225 },
3685+
Event::Chilled { stash: 11 },
3686+
Event::Unbonded { stash: 11, amount: 1000 }
3687+
]
3688+
));
3689+
assert!(!Validators::<Test>::contains_key(11));
3690+
3691+
assert!(Nominators::<Test>::get(11).is_none());
3692+
assert_eq!(Validators::<Test>::count(), 2);
3693+
})
3694+
}
3695+
36413696
#[test]
36423697
fn test_multi_page_payout_stakers_backward_compatible() {
36433698
// Test that payout_stakers work in general and that it pays the correct amount of reward.

0 commit comments

Comments
 (0)