Skip to content

Commit

Permalink
fix(cosmos): don't rerun store migrations on upgrade (#9683)
Browse files Browse the repository at this point in the history
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](https://github.com/Agoric/agoric-sdk/tree/mhofman/9682-test-double-upgrade)
tests applying primary followed by secondary upgrade

### Upgrade Considerations
Fixes cosmos store upgrade logic during chain software upgrades
  • Loading branch information
mhofman authored Jul 10, 2024
1 parent 1c6bbde commit 12b78e3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
9 changes: 7 additions & 2 deletions golang/cosmos/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -862,18 +862,23 @@ func NewAgoricApp(
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)

for name := range upgradeNamesOfThisVersion {
for _, name := range upgradeNamesOfThisVersion {
app.UpgradeKeeper.SetUpgradeHandler(
name,
upgrade16Handler(app, name),
)
}

// At this point we don't have a way to read from the store, so we have to
// rely on data saved by the x/upgrade module in the previous software.
upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk()
if err != nil {
panic(err)
}
if upgradeNamesOfThisVersion[upgradeInfo.Name] && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
// Store migrations can only run once, so we use a notion of "primary upgrade
// name" to trigger them. Testnets may end up upgrading from one rc to
// another, which shouldn't re-run store upgrades.
if isPrimaryUpgradeName(upgradeInfo.Name) && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) {
storeUpgrades := storetypes.StoreUpgrades{
Added: []string{
packetforwardtypes.ModuleName, // Added PFM
Expand Down
27 changes: 23 additions & 4 deletions golang/cosmos/app/upgrade.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,32 @@
package gaia

import (
"fmt"

"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
swingsetkeeper "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/keeper"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

var upgradeNamesOfThisVersion = map[string]bool{
"agoric-upgrade-16": true,
"agoric-upgrade-16-2": true,
var upgradeNamesOfThisVersion = []string{
"agoric-upgrade-16",
"agoric-upgrade-16-2",
}

// isPrimaryUpgradeName returns wether the provided plan name is considered a
// primary for the purpose of applying store migrations for the first upgrade
// of this version.
// It is expected that only primary plan names are used for non testing chains.
func isPrimaryUpgradeName(name string) bool {
return upgradeNamesOfThisVersion[0] == name
}

// isFirstTimeUpgradeOfThisVersion looks up in the upgrade store whether no
// upgrade plan name of this version have previously been applied.
func isFirstTimeUpgradeOfThisVersion(app *GaiaApp, ctx sdk.Context) bool {
for name := range upgradeNamesOfThisVersion {
for _, name := range upgradeNamesOfThisVersion {
if app.UpgradeKeeper.GetDoneHeight(ctx, name) != 0 {
return false
}
Expand Down Expand Up @@ -49,6 +61,13 @@ func upgrade16Handler(app *GaiaApp, targetUpgrade string) func(sdk.Context, upgr
"@agoric/builders/scripts/vats/init-transfer.js",
),
}

// The storeUpgrades defined in app.go only execute for the primary upgrade name
// If we got here and this first upgrade of this version does not use the
// primary upgrade name, stores have not been initialized correctly.
if !isPrimaryUpgradeName(plan.Name) {
return module.VersionMap{}, fmt.Errorf("cannot run %s as first upgrade", plan.Name)
}
}

app.upgradeDetails = &upgradeDetails{
Expand Down

0 comments on commit 12b78e3

Please sign in to comment.