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

fix(cosmos): don't rerun store migrations on upgrade #9683

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Jul 10, 2024

closes: #9682

Description

Add logic to check if the upgrade plan name is a "primary name", and only run store migrations for those.
Check in the upgrade handler that the first upgrade is using a primary name to ensure store migrations were run.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

a3p test for primary upgrade will run on this PR
mhofman/9682-test-double-upgrade tests applying primary followed by secondary upgrade

Upgrade Considerations

Fixes cosmos store upgrade logic during chain software upgrades

@mhofman mhofman requested a review from gibson042 July 10, 2024 18:15
@mhofman mhofman added the force:integration Force integration tests to run on PR label Jul 10, 2024
@mhofman
Copy link
Member Author

mhofman commented Jul 10, 2024

@mhofman
Copy link
Member Author

mhofman commented Jul 10, 2024

Both sets of a3p tests have passed, and we confirmed that this change fixes the problem on emerynet validators.

@mhofman mhofman merged commit 12b78e3 into dev-upgrade-16 Jul 10, 2024
85 of 86 checks passed
@mhofman mhofman deleted the mhofman/9682-fix-double-upgrade branch July 10, 2024 19:22
mhofman added a commit that referenced this pull request Jul 10, 2024
## Packages that have NEWS.md updates

```diff
--- a/golang/cosmos/CHANGELOG.md
+++ b/golang/cosmos/CHANGELOG.md
@@ -3,6 +3,15 @@
 All notable changes to this project will be documented in this file.
 See [Conventional Commits](https://conventionalcommits.org) for commit guidelines.
 
+## [0.35.0-u16.2](https://github.com/Agoric/agoric-sdk/compare/@agoric/cosmos@0.35.0-u16.1...@agoric/cosmos@0.35.0-u16.2) (2024-07-10)
+
+
+### Bug Fixes
+
+* **cosmos:** don't rerun store migrations on upgrade ([#9683](#9683)) ([12b78e3](12b78e3)), closes [#9682](#9682)
+
+
+
 ## [0.35.0-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/cosmos@0.35.0-u16.0...@agoric/cosmos@0.35.0-u16.1) (2024-07-10)
 
 **Note:** Version bump only for package @agoric/cosmos
```
@mhofman mhofman mentioned this pull request Jul 12, 2024
mergify bot added a commit that referenced this pull request Aug 31, 2024
closes: #9682

## Description
Ports the change from #9683 to master, adding some extra checks since master has a more complex upgrade name structure.
Update some docs following the refactor in #9575

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
This PR originally added an a3p-integration test, but it cannot work without modification to the agd binary just for the purpose of testing this (if the original binary already contains support for the upgrade name, it attempts an upgrade in place, which fails as we don't intend to support this)

### Upgrade Considerations
Ensure we have the ability to release and apply multiple release candidates for the same version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants