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

Extrinsic to restore corrupt staking ledgers #3706

Merged
merged 34 commits into from
Mar 27, 2024

Conversation

gpestana
Copy link
Contributor

@gpestana gpestana commented Mar 14, 2024

This PR adds a new extrinsic Call::restore_ledger gated by StakingAdmin origin that restores a corrupted staking ledger. This extrinsic will be used to recover ledgers that were affected by the issue discussed in #3245.

The extrinsic will re-write the storage items associated with a stash account provided as input parameter. The data used to reset the ledger can be either i) fetched on-chain or ii) partially/totally set by the input parameters of the call.

In order to use on-chain data to restore the staking locks, we need a way to read the current lock in the balances pallet. This PR adds a InspectLockableCurrency trait and implements it in the pallet balances. An alternative would be to tightly couple staking with the pallet balances but that's inelegant (an example of how it would look like in this branch).

More details on the type of corruptions and corresponding fixes https://hackmd.io/DLb5jEYWSmmvqXC9ae4yRg?view#/

We verified that the Call::restore_ledger does fix all current corrupted ledgers in Polkadot and Kusama. You can verify it here https://hackmd.io/v-XNrEoGRpe7APR-EZGhOA.

Changes introduced

  • Adds Call::restore_ledger extrinsic to recover a corrupted ledger;
  • Adds trait frame_support::traits::currency::InspectLockableCurrency to allow external pallets to read current locks given an account and lock ID;
  • Implements the InspectLockableCurrency in the pallet-balances.
  • Adds staking locks try-runtime checks (Add try-state check for staking locks #3751)

Todo

Related to #3245
Closes #3751

@gpestana gpestana self-assigned this Mar 14, 2024
@gpestana gpestana marked this pull request as draft March 14, 2024 22:42
@gpestana gpestana requested a review from Ank4n March 14, 2024 22:43
@gpestana gpestana changed the title wip: Extrinsic to clean corrupted ledger Extrinsic to recover corrupted ledger Mar 21, 2024
@gpestana gpestana changed the title Extrinsic to recover corrupted ledger Extrinsic to recover corrupt staking ledgers Mar 21, 2024
@gpestana gpestana added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Mar 21, 2024
@gpestana gpestana marked this pull request as ready for review March 21, 2024 09:54
@gpestana gpestana requested a review from a team as a code owner March 21, 2024 09:54
@gpestana gpestana added the D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. label Mar 21, 2024
@gpestana gpestana added T2-pallets This PR/Issue is related to a particular pallet. and removed T1-FRAME This PR/Issue is related to core FRAME, the framework. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Mar 21, 2024
@gpestana
Copy link
Contributor Author

bot bench substrate-pallet --pallet=pallet_staking

…=dev --target_dir=substrate --pallet=pallet_staking
@command-bot
Copy link

command-bot bot commented Mar 27, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_staking has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5668119 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5668119/artifacts/download.

/// storage.
/// * The ledger's controller and stash matches the associated `Bonded` tuple.
/// * Staking locked funds for every bonded stash should be the same as its ledger's total.
/// * Staking ledger and bond are not corrupted.
fn check_ledgers() -> Result<(), TryRuntimeError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the running time for this try state check? Don't mind putting this in release for now if it is not super long but want to point out that you are reading some storages multiple times and it can be optimized to be much faster.
Ideally we read any storage only once and keep it in memory for further checks.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team March 27, 2024 10:46
@gpestana
Copy link
Contributor Author

bot bench polkadot-pallet --runtime=westend --pallet=pallet_staking

@command-bot
Copy link

command-bot bot commented Mar 27, 2024

@gpestana https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5671993 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=pallet_staking. 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 2-d8bd9239-bd33-4fcf-bf1b-3829eddd07dc to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Mar 27, 2024

@gpestana Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=pallet_staking has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5671993 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5671993/artifacts/download.

gpestana added a commit that referenced this pull request Mar 27, 2024
Backport for 1.7:
- #3639
- #3706

Relevant Issues:
- #3245
@gpestana gpestana added this pull request to the merge queue Mar 27, 2024
Merged via the queue into master with commit bbdbeb7 Mar 27, 2024
133 of 135 checks passed
@gpestana gpestana deleted the gpestana/ledger-badstate-clean branch March 27, 2024 17:44
gpestana added a commit that referenced this pull request Mar 27, 2024
bkchr pushed a commit that referenced this pull request Mar 28, 2024
Backports for 1.7: 
- #3639
- #3706

Relevant Issues:
- #3245
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Apr 9, 2024
This PR adds a new extrinsic `Call::restore_ledger ` gated by
`StakingAdmin` origin that restores a corrupted staking ledger. This
extrinsic will be used to recover ledgers that were affected by the
issue discussed in
paritytech#3245.

The extrinsic will re-write the storage items associated with a stash
account provided as input parameter. The data used to reset the ledger
can be either i) fetched on-chain or ii) partially/totally set by the
input parameters of the call.

In order to use on-chain data to restore the staking locks, we need a
way to read the current lock in the balances pallet. This PR adds a
`InspectLockableCurrency` trait and implements it in the pallet
balances. An alternative would be to tightly couple staking with the
pallet balances but that's inelegant (an example of how it would look
like in [this
branch](https://github.com/paritytech/polkadot-sdk/tree/gpestana/ledger-badstate-clean_tightly)).

More details on the type of corruptions and corresponding fixes
https://hackmd.io/DLb5jEYWSmmvqXC9ae4yRg?view#/

We verified that the `Call::restore_ledger` does fix all current
corrupted ledgers in Polkadot and Kusama. You can verify it here
https://hackmd.io/v-XNrEoGRpe7APR-EZGhOA.

**Changes introduced**
- Adds `Call::restore_ledger ` extrinsic to recover a corrupted ledger;
- Adds trait `frame_support::traits::currency::InspectLockableCurrency`
to allow external pallets to read current locks given an account and
lock ID;
- Implements the `InspectLockableCurrency` in the pallet-balances.
- Adds staking locks try-runtime checks
(paritytech#3751)

**Todo**
- [x] benchmark `Call::restore_ledger`
- [x] throughout testing of all ledger recovering cases
- [x] consider adding the staking locks try-runtime checks to this PR
(paritytech#3751)
- [x] simulate restoring all ledgers
(https://hackmd.io/Dsa2tvhISNSs7zcqriTaxQ?view) in Polkadot and Kusama
using chopsticks -- https://hackmd.io/v-XNrEoGRpe7APR-EZGhOA

Related to paritytech#3245
Closes paritytech#3751

---------

Co-authored-by: command-bot <>
@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

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
Status: Done
Development

Successfully merging this pull request may close these issues.

Add try-state check for staking locks
4 participants