UIP 6: app version safeguard: first draft #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Visit the preview URL for this PR (updated for commit 74c7d80): https://penumbra-uips--pr10-uip-6-76l77qwb.web.app (expires Tue, 19 Nov 2024 23:18:33 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 54ff667eaf61c617385479f19496d509f9179622 |
conorsch
approved these changes
Nov 12, 2024
2 tasks
conorsch
added a commit
to penumbra-zone/penumbra
that referenced
this pull request
Nov 15, 2024
## Describe your changes This implements UIP 6, creating an "app version safeguard", to try and prevent the wrong version of PD from being started against existing state, or running the wrong version of PD. This code should be immediately implementable as a non-breaking *point-release*, which should *immediately* provide a safeguard against forgetting to upgrade to the next major version of the software before the next migration. This should happen because nodes running the point release will start writing, to non-consensus state, the app version they have. The current migration in 0.80 will refuse to run unless this app version key is empty, or exactly the *previous version*, preventing forgetting to upgrade to the next version pre migration. Furthermore, the next migration should do the same, but with the next app version, so that it will not allow skipping the previous migration. For testing, I think we should: - test that we can take an existing node, and then start it with the new code, stop it, and then start it again - test that trying to run `pd migrate` _fails_ because of the newly added safeguard. ## Issue ticket number and link Closes #4793. Implements penumbra-zone/UIPs#10. ## Checklist before requesting a review - [x] I have added guiding text to explain how a reviewer should test these changes. - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > This code is sensitive to potentially breaking consensus, but the testing plan above should confirm that it is safe for a point release. If it is not safe for a point release, the code should be amended so that it is. --------- Co-authored-by: @erwanor <[email protected]> Co-authored-by: Conor Schaefer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A first draft of https://forum.penumbra.zone/t/pre-uip-version-aware-migrations-for-chain-upgrades/120