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

migration: unlock/unreserve Gov v1 balances, remove Gov V1 pallets from polkadot runtime, and remove Gov V1 pallet key/values from storage #7314

Merged
merged 39 commits into from
Aug 18, 2023

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented May 31, 2023

Partial paritytech/polkadot-sdk#485

Checklist

Migration summary

Network Module Total Accounts Total Stake to Unlock Total Deposit to Unreserve USD Total
Kusama Democracy 5364 1,179,551 KSM 25 KSM $30,609,997
Kusama ElectionPhragmen 748 1,496,706 KSM 157 KSM $38,843,594
Kusama Tip 3 N/A 0.21 KSM $5
Polkadot Democracy 5228 44,984,449 DOT 800 DOT $260,914,444
Polkadot ElectionPhragmen 326 235,329,403 DOT 5,649 DOT $1,364,943,301
Polkadot Tip 7 N/A 16 DOT $92

assuming DOT: $5.8, KSM: $25.95

Questions / to figure out

  • Total to unstake amounts seem quite high, but not necessarily invalid. Are there any dashboards we can cross-check them against?~
  • What's the best way to disable calls to Gov V1 pallets, incl. if they're made as part of batch requests?
  • ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer error logs

@liamaharon liamaharon added A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. B1-note_worthy Changes should be noted in the release notes labels May 31, 2023
@liamaharon liamaharon added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels May 31, 2023
@liamaharon liamaharon marked this pull request as ready for review May 31, 2023 10:30
@paritytech-ci paritytech-ci requested review from a team May 31, 2023 10:30
@liamaharon liamaharon added the T1-runtime This PR/Issue is related to the topic “runtime”. label May 31, 2023
@paritytech-ci paritytech-ci requested a review from a team June 13, 2023 12:37
@ggwpez ggwpez added the E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. label Jun 13, 2023
@ggwpez
Copy link
Member

ggwpez commented Jun 13, 2023

What is this in the Kusama try-runtime log? Looks like a few hundreds of those.

ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer

@liamaharon
Copy link
Contributor Author

liamaharon commented Jun 13, 2023

What is this in the Kusama try-runtime log? Looks like a few hundreds of those.

ERROR main runtime::system: Logic error: Unexpected underflow in reducing consumer

It occurs when a lock is removed on an account that has zero consumers set, it is benign and can safely be ignored.

@gpestana suggested in this comment that it's likely to be related to paritytech/polkadot-sdk#234.

I was unable to log the offending accounts to check their state exactly (see the comment), but I did confirm that all the accounts "Unexpected underflow in reducing consumer" occurs for have real locks in Balances, so it's necessary to T::Currency::remove_lock on those accounts even if it triggers that log, otherwise it'll leave hanging locks in the Balances.

node/service/src/chain_spec.rs Outdated Show resolved Hide resolved
runtime/kusama/src/governance/old.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
runtime/kusama/src/lib.rs Outdated Show resolved Hide resolved
@liamaharon
Copy link
Contributor Author

Needs paritytech/substrate#14779. Then, should be no more blockers 🤞

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Ty! Much better approach! :)

@liamaharon liamaharon changed the title migration: unlock/unreserve Gov v1 balances and prepare storage removal migration: unlock/unreserve Gov v1 balances and remove kvs Aug 18, 2023
Comment on lines -205 to -209
// Check which one has more yes votes.
(
OriginCaller::Council(pallet_collective::RawOrigin::Members(l_yes_votes, l_count)),
OriginCaller::Council(pallet_collective::RawOrigin::Members(r_yes_votes, r_count)),
) => Some((l_yes_votes * r_count).cmp(&(r_yes_votes * l_count))),
Copy link
Contributor Author

@liamaharon liamaharon Aug 18, 2023

Choose a reason for hiding this comment

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

@paritytech-cicd-pr
Copy link

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

@liamaharon
Copy link
Contributor Author

bot merge force

@paritytech-processbot paritytech-processbot bot merged commit a877cc5 into master Aug 18, 2023
6 checks passed
@paritytech-processbot paritytech-processbot bot deleted the liam-kusama-gov-v1-storage-migration branch August 18, 2023 09:03
@liamaharon liamaharon changed the title migration: unlock/unreserve Gov v1 balances and remove kvs migration: unlock/unreserve Gov v1 balances, remove Gov V1 pallets from polkadot runtime, and remove Gov V1 pallet key/values from storage Aug 18, 2023
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. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants