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

Fix locked deposit of council voters #4655

Merged
merged 2 commits into from
Jan 7, 2022
Merged

Fix locked deposit of council voters #4655

merged 2 commits into from
Jan 7, 2022

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Jan 4, 2022 •
edited
Loading

Closes #4160

See the linked issue and comments for more detail.

Affected accounts:

  '14udY48vCbPLtF2VEpUmqfJLHesmAZWi5ShzeKgnqkuigYaR',
  '14iGCDW2cEphmcXH22RhbuQBxjAzPP8UJAX8EQw4p5T2ZwsW',
  '14FrHPF7UWDZCe811yBUXMch8VS7hE4CvV26UR92Pvz5BUia',
  '14BpBd1EhsbzSAarjKJmpNHA2evTj1B4FjryWACycBCqTxDQ',
  '13Smi6KLQxEfBNYNPAbiMPiVFVPRpu8WM2qQv42jkmZKnkfo',
  '1A6Tb4x8T9WbN2Rvq1kQFdFoFQxbNv6GBhc5GAMY9ACQV4j',
  '14cstG1jBDJuPGcAf41cmX6QWuF2AxN3sMWaxtPac9D5cToJ',
  '165gfcRhaqPSiwhhw3Vsb6S4iWxALtQtEK5MbtrfJsgDEHQi',
  '1gNBZtAbG75p4cVcdCJNh6NE8YB7X1F7jr67J8fCBeV96cp',
  '1EP1PHmnjwpL2X53CToLFJg8UuXqzbuAv3ySXQjkJaCw3Y6',
  '14v1h36tNjUkxaMGugURtXZncrXns47pkQ86gJcxyPtknsN',
  '14dQWQ6URVDwApf58KWd61FjGcQ4UPujw7gsz5o8Mcitp24y',
  '15Xx6ToS88iUagZ6BNJsEMKDbo4V7RcjdJk1FGfdxLtgqVs4',
  '121YKGZBCaLyCkZjrwfqEkZHHVns1Kt5Q1g6gEfdU6MgbkFy',
  '14pXP1DJVJnZDt6CcPkuwfT6NHh9nQ22J8LnsUCvkk2LRH65',
  '14tye3sQ1L3z3p4DSKhvhbXFMW6jQfFQYFd5xi4LBhJ2TJem',
  '14gUmUjQooUqAgbkP8zEdz5tkLyCqPSz8GZ41AnLLyFzhVw2',
  '16c552whYN6dr8jJtNdRLm8FKyxMpYwFDP5f5enw26VquJk5',
  '16UJBPHVqQ3xYXnmhEpaQtvSRnrP9k1XeE7WxoyCxsrL9AvV',
  '16aHHHXnTYAV72qngKmEDVGXwRw8cknqgioY75BsTtMNJEg3',
  '16X6fA9y2kPoUDN3wo2Y5PhAfYCd7R8b8soH9jKo754TEoHh',
  '14M7jxZGVjp8DKNgmqCRPmrJyVpFSNvrUG6CfX7TGcS48JZw',
  '16kbrXtdXwQKEPrswWQXLtBTao1PqiGhRcXFvyh4ysF5pmoK',
  '1LWcBbCTEc1E1vvVSSPXXiMfGdX9ZLPv9su3cWN3m9xWEjE',
  '1n1B9ZPkg9cv152c6UwtQrXuNwR2tXPXvwKDX9cYeMh4fNQ',
  '1RVaJptRF1XompgRCXuda3Vs8Cxjt22ww5ndSRSvaW7WJBf',
  '1243tzEb446NSpWzPcaeMGpJh2YZ4TwMb4B85yVCt4275fD8',
  '14XetcVhGUWtf2W1tbtA6wxP3MwWxTquLsLhZMgKGBG4W7R5',
  '15rqGn147Drr6nY5Xvyzqo6NgDWJoQzooiqaJHEzkcavFryy',
  '1629Shw6w88GnyXyyUbRtX7YFipQnjScGKcWr1BaRiMhvmAg'

Release Note Addition

Please make sure this is added to the release note of 9150 runtime for transparency:

This runtime contains a custom migration that fixes the locked deposit of a number of accounts who have voted for the Polkadot council in the past. See the list of affected accounts and more description here.

@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. labels Jan 4, 2022
bkchr
bkchr approved these changes Jan 4, 2022
View reviewed changes
runtime/polkadot/src/lib.rs

impl OnRuntimeUpgrade for FixCouncilDepositMigration {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
if VERSION.spec_version == 9150 {
Copy link
Member

Choose a reason for hiding this comment

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

I mean this if isn't "problematic", but do we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am worried of accidentally running this migration twice, in case we forgot to remove it. Since this is not part of any pallet, it is even more likely to happen.

Copy link
Member

Choose a reason for hiding this comment

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

The same could be said about every migration :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

most other migrations are guarded by at least some storage item (i.e. PalletVersion, StorageVersion), but not this one, therefore I guarded it with the spec version.

runtime/polkadot/src/lib.rs Outdated Show resolved Hide resolved
@michalisFr
Copy link

I came to the exact same results using a different (more manual) method. So, I believe the accounts included in this PR are all that have been affected by this bug.

@shawntabrizi
Copy link
Member

shawntabrizi commented Jan 4, 2022 •
edited
Loading

Can this not be fixed via an extrinsic voted by governance?

Feels like the more appropriate process here.

@michalisFr
Copy link

Can this not be fixed via an extrinsic voted by governance?

Feels like the more appropriate process here.

You're probably right, but we felt this is more straightforward and fast, especially now that Raul is OOO. And since the runtime upgrade needs to be upvoted by governance anyway, we're hitting two (or three with the nicks fix #4656) birds with one stone.

If you feel strongly about it, though, personally I don't mind doing it with an extrinsic.

@kianenigma kianenigma added this to the v0.9.15 milestone Jan 5, 2022
@kianenigma
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit f8558e7 into master Jan 7, 2022
@paritytech-processbot paritytech-processbot bot deleted the kiz-fix-council-deposits branch January 7, 2022 15:05
chevdor pushed a commit that referenced this pull request Jan 7, 2022
@kianenigma @chevdor
Fix locked deposit of council voters (#4655)
d4dc967 
* Fix locked deposit of council voters

* add account ids as comment
@chevdor chevdor modified the milestones: v0.9.15, v0.9.16 Jan 11, 2022
chevdor pushed a commit that referenced this pull request Jan 11, 2022
@kianenigma @chevdor
Merge council PR #4655
351134d
@Dengjianping Dengjianping mentioned this pull request Jan 21, 2022
Merged
12 tasks
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
@kianenigma @Wizdave97
Fix locked deposit of council voters (paritytech#4655)
12d70dd 
* Fix locked deposit of council voters

* add account ids as comment
@librelois librelois mentioned this pull request Feb 7, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@shawntabrizi shawntabrizi shawntabrizi approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade.
Projects
None yet
Milestone
v0.9.16
Development

Successfully merging this pull request may close these issues.

Unjustifiable reserved amount. Bug?
5 participants
@kianenigma @michalisFr @shawntabrizi @bkchr @chevdor