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

Fix migrations (#7340) #7349

Closed
wants to merge 4 commits into from
Closed

Fix migrations (#7340) #7349

wants to merge 4 commits into from

Conversation

chevdor
Copy link
Contributor

@chevdor chevdor commented Jun 9, 2023

Backport of #7340

cc @ggwpez

@ggwpez @chevdor
Fix migrations (#7340)
38ba56d 
Okay this was stupid 🤦

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@chevdor chevdor added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. T1-runtime This PR/Issue is related to the topic “runtime”. B1-note_worthy Changes should be noted in the release notes A6-backport Pull request is already reviewed well in another branch. labels Jun 9, 2023
ggwpez
ggwpez approved these changes Jun 9, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks!
Lets double check the CI output here. Will probably still fail with max code size error until #7108

@chevdor
Copy link
Contributor Author

chevdor commented Jun 9, 2023

Westend is now on 9430 already.
Would it still be helpful to make a new Westend v9431 to test the new runtimes ?

@ggwpez
Copy link
Member

ggwpez commented Jun 9, 2023

Westend is now on 9430 already.
Would it still be helpful to make a new Westend v9431 to test the new runtimes ?

It will need to be reverted though. Not sure what best release-engineering practice is in this case 😆

@chevdor
Copy link
Contributor Author

chevdor commented Jun 9, 2023

The problem with reverting is that we won't undo the migrations already applied.
So ss long as the version is lower than 9430 after a downgrade, we could re-upgrade using a new 9430 or simply upgrade again with a runtime v9431 without a downgrade that won't bring much.

@ggwpez
Copy link
Member

ggwpez commented Jun 9, 2023

AFAIK it is fine to use .43 again, since devops should be able to censor out the Sudo::set_code transactions.
So it will be as if .43 got never enacted on Westend.
But you can also do .431 I have no strong opinion on this.

@chevdor
Copy link
Contributor Author

chevdor commented Jun 9, 2023

Let's keep it simple then. We will get a new set of 9430 runtimes once this PR is merged.

@ggwpez
Copy link
Member

ggwpez commented Jun 9, 2023

AFAIK it is fine to use .43 again, since devops should be able to censor out the Sudo::set_code transactions.
So it will be as if .43 got never enacted on Westend.
But you can also do .431 I have no strong opinion on this.

This should work, or @bkchr ?

@ggwpez
Copy link
Member

ggwpez commented Jun 12, 2023

bot fmt

@command-bot
Copy link

command-bot bot commented Jun 12, 2023 •
edited
Loading

@ggwpez https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2969546 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 16-10013b56-0544-4b0d-9c77-4f33f57ff7f7 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jun 12, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2969546 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot/-/jobs/2969546/artifacts/download.

@bkchr
Copy link
Member

bkchr commented Jun 12, 2023

This should work, or @bkchr ?

Yes that is right, if we revert. Then 9430 was never enacted on chain.

ggwpez
ggwpez suggested changes Jun 12, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Going to push a temporary DNM commit to check that the try-runtime CI turns green.

@ggwpez
DNM bump MAX_CODE_SIZE
97e9e7e 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Revert "DNM bump MAX_CODE_SIZE"
b0e80dd 
This reverts commit 97e9e7e.
ggwpez
ggwpez approved these changes Jun 12, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Okay all ✔️ and the DNM commit is reverted.

@coderobe
Copy link
Contributor

merged in ba42b9c

@coderobe coderobe closed this Jun 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@coderobe coderobe coderobe approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. A6-backport Pull request is already reviewed well in another branch. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. E0-runtime_migration PR introduces code that might require downstream chains to run a runtime upgrade. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chevdor @ggwpez @bkchr @coderobe