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

Finish removing staking delay exits migration #665

Merged
merged 1 commit into from
Aug 17, 2021

Conversation

4meta5
Copy link
Contributor

@4meta5 4meta5 commented Aug 4, 2021

Add this after the migration in #662 is executed by all networks

@4meta5 4meta5 added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes labels Aug 4, 2021
@JoshOrndorff
Copy link
Contributor

In the future, the migration code itself should kill this storage item if the runtime upgrade has already executed. This is possible but wasn't implemented in the original migration (my bad!)

Is that possible? Isn't the migration code what set this value in the first place? One option to do this in fewer PRs would be to combine this PR with the one in #662 without using the strongly typed wrapper, and instead kill the storage item using https://crates.parity.io/frame_support/storage/migration/fn.take_storage_value.html

This will do the take operation with keys that you supply manually. Therefore the storage item type is no longer necessary.

@4meta5
Copy link
Contributor Author

4meta5 commented Aug 5, 2021

Your suggestion is the only way.

I thought I could kill the storage value in the else branch of if the migration already ran, but killing the storage item would set it to bool::default() which is false and then the migration could run accidentally again (defeating the purpose of the storage item in the first place).

@4meta5
Copy link
Contributor Author

4meta5 commented Aug 16, 2021

We'll know this can be merged when moonriver parachain_staking::DelatNominationExitsMigration == false because the previous migration (#662 ) kills that storage item and this migration removes it.

As of now, this is not the case (so the prerequisite migration has not yet been executed on moonriver)

@4meta5 4meta5 merged commit a93f9e9 into master Aug 17, 2021
@4meta5 4meta5 deleted the amar-finish-removing-staking-delay-exits-migration branch August 17, 2021 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants