feedback

- directly call from upgrade handlers - add test coverage for bad state
zeta-chain · May 14, 2024 · f3e2743 · f3e2743
1 parent a6c3fe1
commit f3e2743
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 15 deletions.
diff --git a/app/setup_handlers.go b/app/setup_handlers.go
@@ -107,10 +107,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
@@ -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
@@ -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)
+}