From a342b84b1bb9d8872bc297bf09307eced269c4dc Mon Sep 17 00:00:00 2001
From: Alex Gartner <alexg@zetachain.com>
Date: Wed, 15 May 2024 09:23:54 -0700
Subject: [PATCH] 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)
 }