Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[Staking] Active ledger balance may fall below ED if account chills before unbounding #14246

Closed
gpestana opened this issue May 27, 2023 · 1 comment · Fixed by #14186
Closed
Assignees
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.

Comments

@gpestana
Copy link
Contributor

gpestana commented May 27, 2023

Currently, there are some stashes in Polkadot (11) with ledger.active balance below the existential deposit. The call path for that to happen is to chill the stash before unbonding and not call withdraw_unbonded after the bonding period.

However, if the stash tries to call unbond with the full ledger.active balance, it will fail due to the checks enforcing that the active balance should be MinNominatorBond or MinValidatorBond at the end of the unbonding. In this case, the stash needs to call chill first to be able to unbond all the active balance (as described above).

Given that reap stash can be called only against stashes that have ledger.total balance below ED, the stashes that were chilled and do not call withdraw_unbonded after the unbonding period will have remain in the ledger with active balance = 0 without any mechanism to "clean" the ledger besides the stash calling withdraw_unbonded.

Another path for the ledger.active balance to be below MinBond is through slashing. However, in that case it is possible to call reap against the stash if the total (and thus active) balance is below ED.

Side effects

Currently, the staking try-state checks that inspect the ledger state consistency are triggering if there are ledger.active balances below the ED which, as laid out above, can happen (related to #14186).

Allow withdraw_unbonded to be called permissionlessly

From the reasoning above, it follows that some ledger entries may linger in the ledger indefinitely if the stash/controller does not call withdraw_unbonded after chilling and the unbonding period has passed. Currently, there is no way to enforce the withdrawal in those situations.

Perhaps we should consider allowing anyone to call withdraw_unbonded for a third-party stash which has unbonded chunks for longer than N eras and active balance below ED, e.g.:

pub fn withdraw_unbonded_other(
  origin: OriginFor<T>,
  controller: T::AccountId,
  num_slashing_spans: u32,
) -> DispatchResultWithPostInfo {
  let controller = ensure_signed(origin)?;
  let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
   
  let ed = T::Currency::minimum_balance();
  ensure!(ledger.active < ed && unlocking_chunks_older_than_N(ledger), Error::<T>::CannotWithdrawOther);

  Self::do_withdraw_unbonded(&controller, num_slashing_spans) // perhaps refund weight if successful
}

Or even better, we could just allow anyone to call withdraw_unbounded_other on any stash, without any extra check as in the code above.

After withdraw_unbonded_other, the ledger can may be killed if the balance goes to 0.


PR with E2E test reproducing the behaviour described above #14247

@gpestana gpestana added the C1-low PR touches the given topic and has a low impact on builders. label May 27, 2023
@gpestana gpestana self-assigned this May 27, 2023
@gpestana gpestana added the A0-please_review Pull request needs code review. label May 27, 2023
@kianenigma
Copy link
Contributor

permissionless withdraw_unbonded sounds generally reasonable. It is only there because the chain cannot do it itself on_initialize.

But, I think a more fundamental fix is to introduce MinBond which is enforced upon all stakers. In summary:

  1. If you are a chilled your active stake should be at least minBond or zero.
  2. If you are a nominator your active stake should be at least max(minBond, minNominatorBond) or zero.
  3. If you are a validator your active stake should be at least max(minBond, minValidatorBond) or zero.

Then, once a reasonable minBond is set into place, the above scenario will also be ameliorated because these lingering stashes must have at least some amount of active stake, or are forced to unbond entirely.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants