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

Remove the most recent staking migration #765

Merged
merged 4 commits into from
Aug 31, 2021

Conversation

4meta5
Copy link
Contributor

@4meta5 4meta5 commented Aug 31, 2021

Removes the migration from #757 and #748 and the associated tests. It also removes the hotfix_unreserve_nomination extrinsic and event.

It does NOT kill the storage bool, it just removes the type declaration from the code. There is a jira ticket to make a democracy proposal of frame_system::kill_storage call for the storage value FixBondLessMigrationExecuted.

@4meta5 4meta5 added A0-pleasereview Pull request needs code review. B0-silent Changes should not be mentioned in any release notes labels Aug 31, 2021
@notlesh
Copy link
Contributor

notlesh commented Aug 31, 2021

It's definitely not significant, but I wanted to point out that removing the storage item without deleting it will leave some junk in storage.

@notlesh notlesh closed this Aug 31, 2021
@notlesh notlesh reopened this Aug 31, 2021
@notlesh
Copy link
Contributor

notlesh commented Aug 31, 2021

Sorry, that Close button is way too close to the Comment button for me 😅

@4meta5
Copy link
Contributor Author

4meta5 commented Aug 31, 2021

It's definitely not significant, but I wanted to point out that removing the storage item without deleting it will leave some junk in storage.

Good point, I'd rather kill it. Inconveniently, now this requires a follow up migration to remove the on_runtime_upgrade block and storage bool.

Copy link
Contributor

@JoshOrndorff JoshOrndorff left a comment

Choose a reason for hiding this comment

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

I'm not fully familiar with all the fixes and migrations that went in. But this PR definitely just removes them, and adds a migration to remove the previous migration flag.

@notlesh
Copy link
Contributor

notlesh commented Aug 31, 2021

It's definitely not significant, but I wanted to point out that removing the storage item without deleting it will leave some junk in storage.

Good point, I'd rather kill it. Inconveniently, now this requires a follow up migration to remove the on_runtime_upgrade block and storage bool.

I say we remove through democracy using system -> killStorage()

@4meta5
Copy link
Contributor Author

4meta5 commented Aug 31, 2021

I say we remove through democracy using system -> killStorage()

Sure, we should make a jira ticket for it so we don't forget. It's nicer then because we can be done with this in 1 migration instead of 2.

@4meta5 4meta5 merged commit 0780f71 into master Aug 31, 2021
@4meta5 4meta5 deleted the amar-rm-staking-patch-migrations branch August 31, 2021 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants