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

Fix locked deposits of Nicks pallet #4656

Merged
merged 6 commits into from
Jan 7, 2022

Conversation

kianenigma
Copy link
Contributor

@kianenigma kianenigma commented Jan 4, 2022

The nicks pallet was removed at block 569325 from the runtime, without any consideration of the
fact that numerous accounts had reserved funds in this pallet. This migration is the outcome of
an investigation that tries to refund all of the accounts that had a nick set for them prior to
removal, if they still have the amount in their reserved balance. Otherwise, we ignore the
refund.

closes #3243

For the sake of posterity, the code that's used to generate this fix is here: https://gist.github.com/kianenigma/a34bcc2efcf753d986c199da62adffbd


Release Note Addition

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

This runtime upgrade contains a custom migration that fixes the locked deposit participants of the historical Nicks pallet, which was only used temporarily in the Kusama runtime. 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
@michalisFr
Copy link

I have found a subset of @kianenigma's accounts, the ones that have a 10 KSM reserve and some that have 0.01 KSM (I wasn't looking for 0.01 KSM reserves, as I wasn't aware the reserved amount had changed). My results for the accounts with 10 KSM reserve agree with Kian's results, as do the accounts with 0.01 KSM that I found.

My approach can be found here:
#3243
but Kian's methodology is more solid.

The only account that potentially has a reserve from nicks but we couldn't determine it with certainty is this one:
CmD9vaMYoiKe7HiFnfkftwvhKbxN9bhyjcDrfFRGbifJEG8
and so it isn't included in this PR.

Comment on lines +1655 to +1657
116, 157, 220, 147, 166, 93, 254, 195, 175, 39, 204, 116, 120, 33, 44, 183,
212, 176, 192, 53, 127, 239, 53, 160, 22, 57, 102, 171, 83, 51, 183, 87,
],
Copy link
Member

Choose a reason for hiding this comment

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

would like to have the account id written as a comment above these different hexs for easier audit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good idea actually. I will add them, please manually check a few of them? they are all auto generated, so if one is correct, they all should be fine.

132, 250, 52, 33, 111, 141, 173, 223, 29, 75, 206, 46, 56, 72, 71, 124, 222,
140, 211, 139, 226, 179, 165, 161, 38, 254, 36, 111, 96, 86, 124, 60,
],
10000000000000,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that some accounts reserved 10KSM and some 10mKSM because of how the logic of the pallet changed, so variability here is expected.

@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 be0b5d3 into master Jan 7, 2022
@paritytech-processbot paritytech-processbot bot deleted the kiz-fix-nick-deposit-loss branch January 7, 2022 15:05
chevdor pushed a commit that referenced this pull request Jan 7, 2022
* refund the lost deposit of historical nick module

* Fix doc

* add comments etc.

* guard it better

* more log

* fix build
@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
chevdor added a commit that referenced this pull request Jan 15, 2022
This reverts commit 4530f66.
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
* refund the lost deposit of historical nick module

* Fix doc

* add comments etc.

* guard it better

* more log

* fix build
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. 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
Development

Successfully merging this pull request may close these issues.

Reserved KSM by nicks pallet can't be released
4 participants