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

Add pallet-im-online storage migration #114

Merged
merged 17 commits into from
Mar 23, 2023
Merged

Conversation

OnyxSkyscape
Copy link
Contributor

Aims to close #110.

DO NOT merge until tested on development chain.

@OnyxSkyscape OnyxSkyscape self-assigned this Mar 7, 2023
Copy link
Contributor

@PopcornPaws PopcornPaws left a comment

Choose a reason for hiding this comment

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

Cool! Excited to see this in action!

@PopcornPaws PopcornPaws marked this pull request as draft March 8, 2023 07:02
@PopcornPaws PopcornPaws self-requested a review March 9, 2023 09:52
Copy link
Contributor

@PopcornPaws PopcornPaws left a comment

Choose a reason for hiding this comment

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

Cool, however you should only include changes to files in

  • gn-node
  • gn-runtime
  • Cargo.toml

Changes to gn-client, gn-wasm and pallet-validator-manager shouldn't be included because they revert what's currently on main.

Great job otherwise! I guess this upgrade will be split into two PRs though. The first one will be version 102 which has () as an im-online "authority", then 103 will actually include the ValidatorManager pallet as the authority.

gn-pallets/pallet-validator-manager/src/lib.rs Outdated Show resolved Hide resolved
gn-pallets/pallet-validator-manager/src/migration.rs Outdated Show resolved Hide resolved
gn-pallets/pallet-validator-manager/src/test.rs Outdated Show resolved Hide resolved
integr_test.py Outdated Show resolved Hide resolved
gn-wasm/src/lib.rs Outdated Show resolved Hide resolved
gn-wasm/Cargo.toml Outdated Show resolved Hide resolved
gn-runtime/src/lib.rs Show resolved Hide resolved
@PopcornPaws PopcornPaws marked this pull request as ready for review March 21, 2023 07:44
@OnyxSkyscape
Copy link
Contributor Author

The issue above is caused by the combination of 1accee1 and 408bf06 (which was admittedly a mistake on my part), so I will most likely have to reset to a45502a and then rebase onto main (that latter of which should've been done in the first place instead of merging from main), because I can't see a good alternative for explicitly excluding these changes without having to resort to even more cursed and unorthodox methods.

gn-runtime/src/lib.rs Outdated Show resolved Hide resolved
gn-runtime/src/lib.rs Show resolved Hide resolved
@PopcornPaws
Copy link
Contributor

@OnyxSkyscape could you split this into two PRs: one for each runtime upgrade? I'm trying to write a bit about each runtime upgrade in the docs and I'd like to link to certain releases.

So we should merge this PR but only if it includes changes specific to upgrade 102. Then I'll add a release tag on main related to 102.

Then another PR should be opened for 103. What do you think?

@OnyxSkyscape
Copy link
Contributor Author

Runtime upgrade v102 tested and successfully rolled out into prod yesterday 6pm at commit 4423dcf, ready to merge.

@OnyxSkyscape OnyxSkyscape merged commit 3d46290 into main Mar 23, 2023
@PopcornPaws PopcornPaws deleted the I110-im-online-fix branch March 23, 2023 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImOnline runtime upgrade is broken
2 participants