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

Patches Call::Staking.restore_ledger to ensure a restored ledger has enough free balance to cover staking locks #5066

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Jul 18, 2024

The Call::restore_ledger call was introduced to restore corrupted ledgers based on as much on-chain data as possible.

One of the corruption cases that it covers is when a ledger is wiped out due to the corruption. In this case, if there are staking locks left behind associated with a stash that has a ledger wiped out, the new, restored, ledger.total will be taken from the lingering staking locks. However, at the time of the restore, the lingering locks may be higher than the current stash free balance. In that case, we cannot set the new ledger.total as the lock amount.

To solve this issue, Call::restore_ledger will basically chill and kill the corrupted stash/ledger.

Note: Although we may use a different strat to restore the ledgers at this point (see this approach or this), we should nevertheless ensure that this code is correct.

@gpestana gpestana added R0-silent Changes should not be mentioned in any release notes and removed R0-silent Changes should not be mentioned in any release notes labels Jul 18, 2024
@gpestana gpestana self-assigned this Jul 18, 2024
@gpestana gpestana marked this pull request as draft July 18, 2024 13:11
@gpestana gpestana changed the title Improvements to Call::Staking.restore_ledger Patches Call::Staking.restore_ledger to ensure a restored ledger has enough free balance to cover staking locks Jul 30, 2024
@gpestana gpestana added the T2-pallets This PR/Issue is related to a particular pallet. label Jul 30, 2024
@gpestana gpestana marked this pull request as ready for review July 30, 2024 16:44
@gpestana
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

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

Comment bot cancel 4-c4dd97cd-5f48-453f-b988-be3b9adb642f to cancel this command or bot cancel to cancel all commands in this pull request.

Copy link
Contributor

We are migrating the command bot to be a GitHub Action

Please, see the documentation on how to use it

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6864164 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6864164/artifacts/download.

…polkadot-sdk into gpestana/restore_ledger_kill
@gpestana
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

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

Comment bot cancel 5-787c2e29-c85f-4e1c-b7cc-b6b0399e2198 to cancel this command or bot cancel to cancel all commands in this pull request.

Copy link
Contributor

We are migrating the command bot to be a GitHub Action

Please, see the documentation on how to use it

@command-bot
Copy link

command-bot bot commented Jul 30, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6864474 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6864474/artifacts/download.

@gpestana gpestana requested a review from gui1117 August 22, 2024 11:02
@gpestana
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Aug 22, 2024

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

Comment bot cancel 5-b357781a-95ca-420c-b853-40511b6aaade to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Aug 22, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7103117 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7103117/artifacts/download.

@Polkadot-Forum
Copy link

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

https://forum.polkadot.network/t/recover-corrupted-staking-ledgers-in-polkadot-and-kusama/9796/1

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7162019

fellowship-merge-bot bot pushed a commit to polkadot-fellows/runtimes that referenced this pull request Oct 2, 2024
…Kusama (#447)

Note: for more details on the corrupted ledgers issue and recovery steps
check https://hackmd.io/m_h9DRutSZaUqCwM9tqZ3g?view.

This PR adds a migration in Polkadot and Kusama runtimes to recover the
current corrupted ledgers in Polkadot and Kusama. A migration consists
of:

1. Call into `pallet_staking::Pallet::<T>::restore_ledger` for each of
the "whitelisted" stashes as `Root` origin.
2. Performs a check that ensures the restored ledger's stake does not
overflow the current stash's free balance. If that's the case, force
unstake the ledger. This check is currently missing in
polkadot-sdk/pallet-staking ([PR with patch
here](paritytech/polkadot-sdk#5066)).

The reason to restore the corrupted ledgers as migrations implemented in
the fellowship runtimes is twofold:

1. The call to `pallet_staking::Pallet::<T>::restore_ledger` and check +
`force_unstake` must be done atomically (thus a ledger can't be safely
restored with a set of two distinct extrinsic calls, so it's not
possible to use referenda to this fx).
2. To speed up the whole process and avoid having to wait for 1. merge
and releases of paritytech/polkadot-sdk#5066 and
2. referenda to call into `Call::restore_ledger` for both Polkadot and
Kusama.

Alternatively, we could add a new temporary pallet directly in the
fellowship runtime which would expose an extrinsic to restore the
ledgers and perform the extra missing check. See this [PR as an
example](gpestana#2).

---
- [x] on-runtime-upgrade tests against Polkadot and Kusama
- [x] staking try-state checks passing after all migrations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants