From 88065f95e501a8de6cd61972dd4008b9f0dd78f9 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Mon, 29 Apr 2024 12:27:31 -0700 Subject: [PATCH 1/7] feat(zetacore): add develop store upgrade tracker --- app/app.go | 1 + app/setup_handlers.go | 97 +++++++++++++++++++++++++------------ app/upgrade_tracker.go | 94 +++++++++++++++++++++++++++++++++++ app/upgrade_tracker_test.go | 86 ++++++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 32 deletions(-) create mode 100644 app/upgrade_tracker.go create mode 100644 app/upgrade_tracker_test.go diff --git a/app/app.go b/app/app.go index 94327c1faf..b88d2c01a7 100644 --- a/app/app.go +++ b/app/app.go @@ -850,6 +850,7 @@ func (app *App) InitChainer(ctx sdk.Context, req abci.RequestInitChain) abci.Res if err := tmjson.Unmarshal(req.AppStateBytes, &genesisState); err != nil { panic(err) } + app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) return app.mm.InitGenesis(ctx, app.appCodec, genesisState) } diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 7624a37a7e..99eafaf699 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -1,13 +1,16 @@ package app import ( + "os" + + "golang.org/x/exp/slices" + "github.com/cosmos/cosmos-sdk/baseapp" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types" crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types" distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -17,13 +20,12 @@ import ( slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cosmos/cosmos-sdk/x/upgrade/types" - ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" - ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" - "github.com/zeta-chain/zetacore/pkg/constant" emissionstypes "github.com/zeta-chain/zetacore/x/emissions/types" ibccrosschaintypes "github.com/zeta-chain/zetacore/x/ibccrosschain/types" ) +const releaseVersion = "v17" + func SetupHandlers(app *App) { // Set param key table for params module migration for _, subspace := range app.ParamsKeeper.GetSubspaces() { @@ -53,26 +55,66 @@ func SetupHandlers(app *App) { } } baseAppLegacySS := app.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(paramstypes.ConsensusParamsKeyTable()) - app.UpgradeKeeper.SetUpgradeHandler(constant.Version, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { - app.Logger().Info("Running upgrade handler for " + constant.Version) - // Migrate Tendermint consensus parameters from x/params module to a dedicated x/consensus module. - baseapp.MigrateParams(ctx, baseAppLegacySS, &app.ConsensusParamsKeeper) - // Updated version map to the latest consensus versions from each module - for m, mb := range app.mm.Modules { - if module, ok := mb.(module.HasConsensusVersion); ok { - vm[m] = module.ConsensusVersion() - } - } + needsForcedMigration := []string{ + authtypes.ModuleName, + banktypes.ModuleName, + stakingtypes.ModuleName, + distrtypes.ModuleName, + slashingtypes.ModuleName, + govtypes.ModuleName, + crisistypes.ModuleName, + emissionstypes.ModuleName, + } + allUpgrades := upgradeTracker{ + upgrades: []upgradeTrackerItem{ + { + index: 1714664193, + storeUpgrade: &storetypes.StoreUpgrades{ + Added: []string{consensustypes.ModuleName, crisistypes.ModuleName}, + }, + upgradeHandler: func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) { + // Migrate Tendermint consensus parameters from x/params module to a dedicated x/consensus module + // https://docs.cosmos.network/main/build/migrations/upgrading#xconsensus + baseapp.MigrateParams(ctx, baseAppLegacySS, &app.ConsensusParamsKeeper) - VersionMigrator{v: vm}.TriggerMigration(authtypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(banktypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(stakingtypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(distrtypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(slashingtypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(govtypes.ModuleName) - VersionMigrator{v: vm}.TriggerMigration(crisistypes.ModuleName) + // empty version map happens when upgrading from old versions which did not correctly call + // app.UpgradeKeeper.SetModuleVersionMap(ctx, app.mm.GetVersionMap()) in InitChainer. + // we must populate the version map if we detect this scenario + // + // this will only happen on the first upgrade. mainnet and testnet will not require this condition. + if len(vm) == 0 { + for m, mb := range app.mm.Modules { + if module, ok := mb.(module.HasConsensusVersion); ok { + if slices.Contains(needsForcedMigration, m) { + vm[m] = module.ConsensusVersion() - 1 + } else { + vm[m] = module.ConsensusVersion() + } + } + } + } + return vm, nil + }, + }, + }, + } - VersionMigrator{v: vm}.TriggerMigration(emissionstypes.ModuleName) + _, useDevelopTracker := os.LookupEnv("ZETACORED_USE_DEVELOP_UPGRADE_TRACKER") + upgradeHandlerFns, storeUpgrades, err := allUpgrades.getUpgrades(useDevelopTracker) + if err != nil { + panic(err) + } + + app.UpgradeKeeper.SetUpgradeHandler(releaseVersion, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + app.Logger().Info("Running upgrade handler for " + releaseVersion) + + var err error + for _, upgradeHandler := range upgradeHandlerFns { + vm, err = upgradeHandler(ctx, vm) + if err != nil { + return vm, err + } + } return app.mm.RunMigrations(ctx, app.configurator, vm) }) @@ -96,15 +138,6 @@ func SetupHandlers(app *App) { // it checks if version == upgradeHeight and applies store upgrades before loading the stores, // so that new stores start with the correct version (the current height of chain), // instead the default which is the latest version that store last committed i.e 0 for new stores. - app.SetStoreLoader(types.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades)) + app.SetStoreLoader(types.UpgradeStoreLoader(upgradeInfo.Height, storeUpgrades)) } } - -type VersionMigrator struct { - v module.VersionMap -} - -func (v VersionMigrator) TriggerMigration(moduleName string) module.VersionMap { - v.v[moduleName] = v.v[moduleName] - 1 - return v.v -} diff --git a/app/upgrade_tracker.go b/app/upgrade_tracker.go new file mode 100644 index 0000000000..1dec261ea5 --- /dev/null +++ b/app/upgrade_tracker.go @@ -0,0 +1,94 @@ +package app + +import ( + "fmt" + "os" + "path" + "strconv" + + storetypes "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" +) + +type upgradeHandlerFn func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) + +type upgradeTrackerItem struct { + // Monotonically increasing index to order and track migrations. Typically the current unix epoch timestamp. + index int64 + // Function that will run during the SetUpgradeHandler callback. The VersionMap must always be returned. + upgradeHandler upgradeHandlerFn + // StoreUpgrades that will be provided to UpgradeStoreLoader + storeUpgrade *storetypes.StoreUpgrades +} + +// upgradeTracker allows us to track needed upgrades/migrations across both release and develop builds +type upgradeTracker struct { + upgrades []upgradeTrackerItem + stateFileDir string +} + +func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { + neededUpgrades := &storetypes.StoreUpgrades{} + neededUpgradeHandlers := []upgradeHandlerFn{} + stateFilePath := path.Join(t.stateFileDir, "developupgradetracker") + + currentIndex := int64(0) + if stateFileContents, err := os.ReadFile(stateFilePath); err == nil { // #nosec G304 -- stateFilePath is not user controllable + currentIndex, err = strconv.ParseInt(string(stateFileContents), 10, 64) + if err != nil { + return nil, nil, fmt.Errorf("unable to decode upgrade tracker: %w", err) + } + } else { + fmt.Printf("unable to load upgrade tracker: %v\n", err) + } + + maxIndex := currentIndex + for _, item := range t.upgrades { + index := item.index + upgrade := item.storeUpgrade + versionModifier := item.upgradeHandler + if index <= currentIndex { + continue + } + if versionModifier != nil { + neededUpgradeHandlers = append(neededUpgradeHandlers, versionModifier) + } + if upgrade != nil { + neededUpgrades.Added = append(neededUpgrades.Added, upgrade.Added...) + neededUpgrades.Deleted = append(neededUpgrades.Deleted, upgrade.Deleted...) + neededUpgrades.Renamed = append(neededUpgrades.Renamed, upgrade.Renamed...) + } + maxIndex = index + } + err := os.WriteFile(stateFilePath, []byte(strconv.FormatInt(maxIndex, 10)), 0o600) + if err != nil { + return nil, nil, fmt.Errorf("unable to write upgrade state file: %w", err) + } + return neededUpgradeHandlers, neededUpgrades, nil +} + +func (t upgradeTracker) mergeAllUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { + upgrades := &storetypes.StoreUpgrades{} + upgradeHandlers := []upgradeHandlerFn{} + for _, item := range t.upgrades { + upgrade := item.storeUpgrade + versionModifier := item.upgradeHandler + if versionModifier != nil { + upgradeHandlers = append(upgradeHandlers, versionModifier) + } + if upgrade != nil { + upgrades.Added = append(upgrades.Added, upgrade.Added...) + upgrades.Deleted = append(upgrades.Deleted, upgrade.Deleted...) + upgrades.Renamed = append(upgrades.Renamed, upgrade.Renamed...) + } + } + return upgradeHandlers, upgrades, nil +} + +func (t upgradeTracker) getUpgrades(isDevelop bool) ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { + if isDevelop { + return t.getDevelopUpgrades() + } + return t.mergeAllUpgrades() +} diff --git a/app/upgrade_tracker_test.go b/app/upgrade_tracker_test.go new file mode 100644 index 0000000000..c8138d452b --- /dev/null +++ b/app/upgrade_tracker_test.go @@ -0,0 +1,86 @@ +package app + +import ( + "os" + "testing" + + storetypes "github.com/cosmos/cosmos-sdk/store/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/module" + "github.com/stretchr/testify/require" + authoritytypes "github.com/zeta-chain/zetacore/x/authority/types" + lightclienttypes "github.com/zeta-chain/zetacore/x/lightclient/types" +) + +func TestUpgradeTracker(t *testing.T) { + r := require.New(t) + + tmpdir, err := os.MkdirTemp("", "storeupgradetracker-*") + r.NoError(err) + + allUpgrades := upgradeTracker{ + upgrades: []upgradeTrackerItem{ + { + index: 1000, + storeUpgrade: &storetypes.StoreUpgrades{ + Added: []string{authoritytypes.ModuleName}, + }, + }, + { + index: 2000, + storeUpgrade: &storetypes.StoreUpgrades{ + Added: []string{lightclienttypes.ModuleName}, + }, + upgradeHandler: func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }, + }, + { + index: 3000, + upgradeHandler: func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }, + }, + }, + stateFileDir: tmpdir, + } + + upgradeHandlers, storeUpgrades, err := allUpgrades.mergeAllUpgrades() + r.NoError(err) + r.Len(storeUpgrades.Added, 2) + r.Len(storeUpgrades.Renamed, 0) + r.Len(storeUpgrades.Deleted, 0) + r.Len(upgradeHandlers, 2) + + // should return all migrations on first call + upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + r.NoError(err) + r.Len(storeUpgrades.Added, 2) + r.Len(storeUpgrades.Renamed, 0) + r.Len(storeUpgrades.Deleted, 0) + r.Len(upgradeHandlers, 2) + + // should return no upgrades on second call + upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + r.NoError(err) + r.Len(storeUpgrades.Added, 0) + r.Len(storeUpgrades.Renamed, 0) + r.Len(storeUpgrades.Deleted, 0) + r.Len(upgradeHandlers, 0) + + // now add a upgrade and ensure that it gets run without running + // the other upgrades + allUpgrades.upgrades = append(allUpgrades.upgrades, upgradeTrackerItem{ + index: 4000, + storeUpgrade: &storetypes.StoreUpgrades{ + Deleted: []string{"example"}, + }, + }) + + upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + r.NoError(err) + r.Len(storeUpgrades.Added, 0) + r.Len(storeUpgrades.Renamed, 0) + r.Len(storeUpgrades.Deleted, 1) + r.Len(upgradeHandlers, 0) +} From 01bb02a3229652e9e339af568e3b74e2d66caa29 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Mon, 13 May 2024 11:27:56 -0700 Subject: [PATCH 2/7] add ibc upgrade --- app/setup_handlers.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 99eafaf699..81f78c8620 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -11,6 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types" crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types" distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -20,6 +21,8 @@ import ( slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" "github.com/cosmos/cosmos-sdk/x/upgrade/types" + ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" + ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" emissionstypes "github.com/zeta-chain/zetacore/x/emissions/types" ibccrosschaintypes "github.com/zeta-chain/zetacore/x/ibccrosschain/types" ) @@ -96,6 +99,12 @@ func SetupHandlers(app *App) { return vm, nil }, }, + { + index: 1715624665, + storeUpgrade: &storetypes.StoreUpgrades{ + Added: []string{capabilitytypes.ModuleName, ibcexported.ModuleName, ibctransfertypes.ModuleName}, + }, + }, }, } From e9e86295af14329efe4e21d42dcce5c443c5df4d Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Mon, 13 May 2024 12:29:07 -0700 Subject: [PATCH 3/7] fix gosec annotation? --- app/upgrade_tracker.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/upgrade_tracker.go b/app/upgrade_tracker.go index 1dec261ea5..e484e5647d 100644 --- a/app/upgrade_tracker.go +++ b/app/upgrade_tracker.go @@ -34,7 +34,8 @@ func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.St stateFilePath := path.Join(t.stateFileDir, "developupgradetracker") currentIndex := int64(0) - if stateFileContents, err := os.ReadFile(stateFilePath); err == nil { // #nosec G304 -- stateFilePath is not user controllable + stateFileContents, err := os.ReadFile(stateFilePath) // #nosec G304 -- stateFilePath is not user controllable + if err == nil { currentIndex, err = strconv.ParseInt(string(stateFileContents), 10, 64) if err != nil { return nil, nil, fmt.Errorf("unable to decode upgrade tracker: %w", err) @@ -61,7 +62,7 @@ func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.St } maxIndex = index } - err := os.WriteFile(stateFilePath, []byte(strconv.FormatInt(maxIndex, 10)), 0o600) + err = os.WriteFile(stateFilePath, []byte(strconv.FormatInt(maxIndex, 10)), 0o600) if err != nil { return nil, nil, fmt.Errorf("unable to write upgrade state file: %w", err) } From 0537c5f8db4e3d1a50357659f9e756ff1513b927 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Tue, 14 May 2024 08:50:39 -0700 Subject: [PATCH 4/7] feedback - directly call from upgrade handlers - add test coverage for bad state --- app/setup_handlers.go | 13 ++++++++++--- app/upgrade_tracker.go | 15 +++++---------- app/upgrade_tracker_test.go | 25 +++++++++++++++++++++++-- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 81f78c8620..195a4eb794 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -108,10 +108,17 @@ func SetupHandlers(app *App) { }, } + var upgradeHandlerFns []upgradeHandlerFn + var storeUpgrades *storetypes.StoreUpgrades + var err error _, useDevelopTracker := os.LookupEnv("ZETACORED_USE_DEVELOP_UPGRADE_TRACKER") - upgradeHandlerFns, storeUpgrades, err := allUpgrades.getUpgrades(useDevelopTracker) - if err != nil { - panic(err) + if useDevelopTracker { + upgradeHandlerFns, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + if err != nil { + panic(err) + } + } else { + upgradeHandlerFns, storeUpgrades = allUpgrades.mergeAllUpgrades() } app.UpgradeKeeper.SetUpgradeHandler(releaseVersion, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { diff --git a/app/upgrade_tracker.go b/app/upgrade_tracker.go index e484e5647d..0bb895a785 100644 --- a/app/upgrade_tracker.go +++ b/app/upgrade_tracker.go @@ -11,6 +11,8 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" ) +const developUpgradeTrackerStateFile = "developupgradetracker" + type upgradeHandlerFn func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) type upgradeTrackerItem struct { @@ -31,7 +33,7 @@ type upgradeTracker struct { func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { neededUpgrades := &storetypes.StoreUpgrades{} neededUpgradeHandlers := []upgradeHandlerFn{} - stateFilePath := path.Join(t.stateFileDir, "developupgradetracker") + stateFilePath := path.Join(t.stateFileDir, developUpgradeTrackerStateFile) currentIndex := int64(0) stateFileContents, err := os.ReadFile(stateFilePath) // #nosec G304 -- stateFilePath is not user controllable @@ -69,7 +71,7 @@ func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.St return neededUpgradeHandlers, neededUpgrades, nil } -func (t upgradeTracker) mergeAllUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { +func (t upgradeTracker) mergeAllUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades) { upgrades := &storetypes.StoreUpgrades{} upgradeHandlers := []upgradeHandlerFn{} for _, item := range t.upgrades { @@ -84,12 +86,5 @@ func (t upgradeTracker) mergeAllUpgrades() ([]upgradeHandlerFn, *storetypes.Stor upgrades.Renamed = append(upgrades.Renamed, upgrade.Renamed...) } } - return upgradeHandlers, upgrades, nil -} - -func (t upgradeTracker) getUpgrades(isDevelop bool) ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { - if isDevelop { - return t.getDevelopUpgrades() - } - return t.mergeAllUpgrades() + return upgradeHandlers, upgrades } diff --git a/app/upgrade_tracker_test.go b/app/upgrade_tracker_test.go index c8138d452b..b5b3ad8afd 100644 --- a/app/upgrade_tracker_test.go +++ b/app/upgrade_tracker_test.go @@ -2,6 +2,7 @@ package app import ( "os" + "path" "testing" storetypes "github.com/cosmos/cosmos-sdk/store/types" @@ -17,6 +18,7 @@ func TestUpgradeTracker(t *testing.T) { tmpdir, err := os.MkdirTemp("", "storeupgradetracker-*") r.NoError(err) + defer os.RemoveAll(tmpdir) allUpgrades := upgradeTracker{ upgrades: []upgradeTrackerItem{ @@ -45,8 +47,7 @@ func TestUpgradeTracker(t *testing.T) { stateFileDir: tmpdir, } - upgradeHandlers, storeUpgrades, err := allUpgrades.mergeAllUpgrades() - r.NoError(err) + upgradeHandlers, storeUpgrades := allUpgrades.mergeAllUpgrades() r.Len(storeUpgrades.Added, 2) r.Len(storeUpgrades.Renamed, 0) r.Len(storeUpgrades.Deleted, 0) @@ -84,3 +85,23 @@ func TestUpgradeTracker(t *testing.T) { r.Len(storeUpgrades.Deleted, 1) r.Len(upgradeHandlers, 0) } + +func TestUpgradeTrackerBadState(t *testing.T) { + r := require.New(t) + + tmpdir, err := os.MkdirTemp("", "storeupgradetracker-*") + r.NoError(err) + defer os.RemoveAll(tmpdir) + + stateFilePath := path.Join(tmpdir, developUpgradeTrackerStateFile) + + err = os.WriteFile(stateFilePath, []byte("badstate"), 0o600) + r.NoError(err) + + allUpgrades := upgradeTracker{ + upgrades: []upgradeTrackerItem{}, + stateFileDir: tmpdir, + } + _, _, err = allUpgrades.getDevelopUpgrades() + r.Error(err) +} From 198bc74039b34f9bfc49e86a975a1f3daec7b65f Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Tue, 14 May 2024 10:25:17 -0700 Subject: [PATCH 5/7] ibc crosschain migration --- app/setup_handlers.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 195a4eb794..110f8c1df5 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -23,6 +23,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/upgrade/types" ibctransfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported" + "github.com/zeta-chain/zetacore/pkg/constant" emissionstypes "github.com/zeta-chain/zetacore/x/emissions/types" ibccrosschaintypes "github.com/zeta-chain/zetacore/x/ibccrosschain/types" ) @@ -105,6 +106,12 @@ func SetupHandlers(app *App) { Added: []string{capabilitytypes.ModuleName, ibcexported.ModuleName, ibctransfertypes.ModuleName}, }, }, + { + index: 1715707436, + storeUpgrade: &storetypes.StoreUpgrades{ + Added: []string{ibccrosschaintypes.ModuleName}, + }, + }, }, } @@ -140,16 +147,6 @@ func SetupHandlers(app *App) { panic(err) } if upgradeInfo.Name == constant.Version && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { - storeUpgrades := storetypes.StoreUpgrades{ - Added: []string{ - consensustypes.ModuleName, - crisistypes.ModuleName, - capabilitytypes.ModuleName, - ibcexported.ModuleName, - ibctransfertypes.ModuleName, - ibccrosschaintypes.ModuleName, - }, - } // Use upgrade store loader for the initial loading of all stores when app starts, // it checks if version == upgradeHeight and applies store upgrades before loading the stores, // so that new stores start with the correct version (the current height of chain), From df334cf3ea5b373e20fcf56e5c8d181ae79b4171 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Wed, 15 May 2024 09:23:54 -0700 Subject: [PATCH 6/7] review feedback --- app/setup_handlers.go | 7 ++++--- app/upgrade_tracker.go | 20 +++++++++++++------- app/upgrade_tracker_test.go | 10 +++++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 110f8c1df5..7221f7f680 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -113,14 +113,15 @@ func SetupHandlers(app *App) { }, }, }, + stateFileDir: DefaultNodeHome, } var upgradeHandlerFns []upgradeHandlerFn var storeUpgrades *storetypes.StoreUpgrades var err error - _, useDevelopTracker := os.LookupEnv("ZETACORED_USE_DEVELOP_UPGRADE_TRACKER") - if useDevelopTracker { - upgradeHandlerFns, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + _, useIncrementalTracker := os.LookupEnv("ZETACORED_USE_INCREMENTAL_UPGRADE_TRACKER") + if useIncrementalTracker { + upgradeHandlerFns, storeUpgrades, err = allUpgrades.getIncrementalUpgrades() if err != nil { panic(err) } diff --git a/app/upgrade_tracker.go b/app/upgrade_tracker.go index 0bb895a785..a467cb3e0c 100644 --- a/app/upgrade_tracker.go +++ b/app/upgrade_tracker.go @@ -11,7 +11,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" ) -const developUpgradeTrackerStateFile = "developupgradetracker" +const incrementalUpgradeTrackerStateFile = "incrementalupgradetracker" type upgradeHandlerFn func(ctx sdk.Context, vm module.VersionMap) (module.VersionMap, error) @@ -26,14 +26,18 @@ type upgradeTrackerItem struct { // upgradeTracker allows us to track needed upgrades/migrations across both release and develop builds type upgradeTracker struct { - upgrades []upgradeTrackerItem + upgrades []upgradeTrackerItem + // directory the incremental state file is stored stateFileDir string } -func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { +// getIncrementalUpgrades gets all upgrades that have not been been applied. This is typically +// used for developnet upgrades since we need to run migrations as the are committed rather than +// all at once during a release +func (t upgradeTracker) getIncrementalUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades, error) { neededUpgrades := &storetypes.StoreUpgrades{} neededUpgradeHandlers := []upgradeHandlerFn{} - stateFilePath := path.Join(t.stateFileDir, developUpgradeTrackerStateFile) + stateFilePath := path.Join(t.stateFileDir, incrementalUpgradeTrackerStateFile) currentIndex := int64(0) stateFileContents, err := os.ReadFile(stateFilePath) // #nosec G304 -- stateFilePath is not user controllable @@ -50,12 +54,12 @@ func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.St for _, item := range t.upgrades { index := item.index upgrade := item.storeUpgrade - versionModifier := item.upgradeHandler + upgradeHandler := item.upgradeHandler if index <= currentIndex { continue } - if versionModifier != nil { - neededUpgradeHandlers = append(neededUpgradeHandlers, versionModifier) + if upgradeHandler != nil { + neededUpgradeHandlers = append(neededUpgradeHandlers, upgradeHandler) } if upgrade != nil { neededUpgrades.Added = append(neededUpgrades.Added, upgrade.Added...) @@ -71,6 +75,8 @@ func (t upgradeTracker) getDevelopUpgrades() ([]upgradeHandlerFn, *storetypes.St return neededUpgradeHandlers, neededUpgrades, nil } +// mergeAllUpgrades unconditionally merges all upgrades. Typically used to gather the +// migrations used during a release upgrade. func (t upgradeTracker) mergeAllUpgrades() ([]upgradeHandlerFn, *storetypes.StoreUpgrades) { upgrades := &storetypes.StoreUpgrades{} upgradeHandlers := []upgradeHandlerFn{} diff --git a/app/upgrade_tracker_test.go b/app/upgrade_tracker_test.go index b5b3ad8afd..924ba4e053 100644 --- a/app/upgrade_tracker_test.go +++ b/app/upgrade_tracker_test.go @@ -54,7 +54,7 @@ func TestUpgradeTracker(t *testing.T) { r.Len(upgradeHandlers, 2) // should return all migrations on first call - upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + upgradeHandlers, storeUpgrades, err = allUpgrades.getIncrementalUpgrades() r.NoError(err) r.Len(storeUpgrades.Added, 2) r.Len(storeUpgrades.Renamed, 0) @@ -62,7 +62,7 @@ func TestUpgradeTracker(t *testing.T) { r.Len(upgradeHandlers, 2) // should return no upgrades on second call - upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + upgradeHandlers, storeUpgrades, err = allUpgrades.getIncrementalUpgrades() r.NoError(err) r.Len(storeUpgrades.Added, 0) r.Len(storeUpgrades.Renamed, 0) @@ -78,7 +78,7 @@ func TestUpgradeTracker(t *testing.T) { }, }) - upgradeHandlers, storeUpgrades, err = allUpgrades.getDevelopUpgrades() + upgradeHandlers, storeUpgrades, err = allUpgrades.getIncrementalUpgrades() r.NoError(err) r.Len(storeUpgrades.Added, 0) r.Len(storeUpgrades.Renamed, 0) @@ -93,7 +93,7 @@ func TestUpgradeTrackerBadState(t *testing.T) { r.NoError(err) defer os.RemoveAll(tmpdir) - stateFilePath := path.Join(tmpdir, developUpgradeTrackerStateFile) + stateFilePath := path.Join(tmpdir, incrementalUpgradeTrackerStateFile) err = os.WriteFile(stateFilePath, []byte("badstate"), 0o600) r.NoError(err) @@ -102,6 +102,6 @@ func TestUpgradeTrackerBadState(t *testing.T) { upgrades: []upgradeTrackerItem{}, stateFileDir: tmpdir, } - _, _, err = allUpgrades.getDevelopUpgrades() + _, _, err = allUpgrades.getIncrementalUpgrades() r.Error(err) } From a45ab9c12286a637154138ddd6d4f007b7dec4b7 Mon Sep 17 00:00:00 2001 From: Alex Gartner Date: Thu, 16 May 2024 10:34:34 -0700 Subject: [PATCH 7/7] fix version --- app/setup_handlers.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/setup_handlers.go b/app/setup_handlers.go index 7221f7f680..d178f5a599 100644 --- a/app/setup_handlers.go +++ b/app/setup_handlers.go @@ -28,8 +28,6 @@ import ( ibccrosschaintypes "github.com/zeta-chain/zetacore/x/ibccrosschain/types" ) -const releaseVersion = "v17" - func SetupHandlers(app *App) { // Set param key table for params module migration for _, subspace := range app.ParamsKeeper.GetSubspaces() { @@ -129,8 +127,8 @@ func SetupHandlers(app *App) { upgradeHandlerFns, storeUpgrades = allUpgrades.mergeAllUpgrades() } - app.UpgradeKeeper.SetUpgradeHandler(releaseVersion, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { - app.Logger().Info("Running upgrade handler for " + releaseVersion) + app.UpgradeKeeper.SetUpgradeHandler(constant.Version, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + app.Logger().Info("Running upgrade handler for " + constant.Version) var err error for _, upgradeHandler := range upgradeHandlerFns {