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

Add Version Checks on Para Upgrade #2261

Merged
merged 4 commits into from
Mar 2, 2023

Conversation

joepetrowski
Copy link
Contributor

Adds checks to ensure that the spec version is increasing and that the spec name is the same. This will prevent accidentally upgrading a parachain to a different runtime.

Migration

Although this changes storage, I did not increase the storage version or provide a migration. The reason is that the upgrade itself will kill the affected storage item, and the next time an upgrade is authorized, the new type will be written. But this will break any front ends that show authorized upgrades, so let me know if I should bump the storage version for this.

Transaction Version

Unfortunately breaks it, although it's a Root call. Could also add a new function authorize_upgrade_without_checks as an alternative.

@joepetrowski joepetrowski added A0-please_review Pull request needs code review. 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. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”. labels Mar 1, 2023
@joepetrowski joepetrowski requested a review from bkchr March 1, 2023 12:53
pallets/parachain-system/src/lib.rs Outdated Show resolved Hide resolved
pallets/parachain-system/src/lib.rs Outdated Show resolved Hide resolved
pallets/parachain-system/src/lib.rs Outdated Show resolved Hide resolved
pallets/parachain-system/src/lib.rs Outdated Show resolved Hide resolved
@ruseinov ruseinov self-requested a review March 1, 2023 16:52
@joepetrowski joepetrowski changed the title Add Version Checks on Para Ugprade Add Version Checks on Para Upgrade Mar 1, 2023
@joepetrowski
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 3249186 into master Mar 2, 2023
@paritytech-processbot paritytech-processbot bot deleted the joe-check-para-upgrades branch March 2, 2023 15:48
@xlc
Copy link
Contributor

xlc commented Mar 2, 2023

Can we have some scary labels to ensure this PR is getting attentions by parachain teams? It is a breaking change and will impact scripts related to runtime upgrade.
It will be even better if you can just make it a non-breaking change.

@joepetrowski joepetrowski added the F3-breaks_API This PR changes public API; next release should be major. label Mar 3, 2023
@louismerlin louismerlin added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Mar 6, 2023
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. 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. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited. F3-breaks_API This PR changes public API; next release should be major. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants