Fix fast-unstake for accounts with slashing

[kianenigma]

kinda subtle, but kinda ashamed that I made the mistake 🤦

[kianenigma]

bot fmt

[kianenigma]

/cmd queue -c fmt $ 1

[command-bot]

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2172044 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 116-bf25b68a-c4b2-49ff-a678-b2d6056d6f10 to cancel this command or /cmd cancel to cancel all commands in this pull request.

[command-bot]

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" 1 has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2172044 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2172044/artifacts/download.

[Ank4n]

Similar mistake here
https://github.com/paritytech/substrate/blob/master/frame/staking/src/pallet/mod.rs#L985

[kianenigma]

Similar mistake here https://github.com/paritytech/substrate/blob/master/frame/staking/src/pallet/mod.rs#L985

@gpestana can you add a test for this as well?

[kianenigma]

we should make sure this goes to the next possible Polkadot release, which will probably be in 2023, would be good to have your tests in here @gpestana soon nonetheless.

[melekes]

kinda subtle, but kinda ashamed that I made the mistake 🤦

can you explain what the issue was? neither issue's title does this nor there's an issue linked to this PR. Thanks 🙏

[melekes]

👍 (code-wise; don't know enough to reason about business logic)

[gpestana]

@kianenigma with the current flow, there's no path for the calculated real_num_slashing_spans to be used in the fn kill_stash called by fn do_withdraw_unbound. Thus, the slash + unbound + do_witdraw tests are not required/possible.

---

Explanation:

When calling Call::unbound(), if the controller's max_chunks are filled, the fn do_withdraw_unbond is called:

mod.rs#L984-L986

if unlocking == T::MaxUnlockingChunks::get() as usize {
    let real_num_slashing_spans = Self::slashing_spans(&controller).iter().count();
    Some(Self::do_withdraw_unbonded(&controller, real_num_slashing_spans as u32)?)
}

In the do_withdraw_unbond, the account is checked to see if all the staking related information should be removed (fn kill_stash). The condition is the following:

if ledger.unlocking.is_empty() && ledger.active < T::Currency::minimum_balance() {
    Self::kill_stash(&stash, num_slashing_spans)?;
    // ... 

i.e. the account should be killed iff there's no unlocking chunks and the balance is below the ED.

Thus, the path unbound() -> do_withdraw_unbond() -> kill_stash() is disjoint and can never happen, because the condition to call kill_stash() prevents the do_withdraw_unbond() to be called (no chunks in the slots).

Note: since withdraw_unbond() is an extrinsic, the fn kill_stash() may be called, just not through the fn unbond call.

[kianenigma]

kinda subtle, but kinda ashamed that I made the mistake 🤦

can you explain what the issue was? neither issue's title does this nor there's an issue linked to this PR. Thanks 🙏

fn slashing_spans returns Option<T> where T implemented Iterator and I expected it check the .count() of this iterator, but, Option<T> also implements Iterator, who's count is 0 or 1.

This is unrelated to any business logic.

[kianenigma]

bot rebase

[paritytech-processbot]

Rebased

[kianenigma]

/cmd queue -c fmt

[command-bot]

@kianenigma https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2188345 was started for your command "$PIPELINE_SCRIPTS_DIR/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment /cmd cancel 124-14dc6cef-2a84-414a-8b45-9c071519bfc3 to cancel this command or /cmd cancel to cancel all commands in this pull request.

[command-bot]

@kianenigma Command "$PIPELINE_SCRIPTS_DIR/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2188345 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2188345/artifacts/download.

[kianenigma]

bot merge

[Polkadot-Forum]

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/path-to-fast-unstake-in-polkadot-kusama/1539/1