review feedback

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)
 		}

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{}

app/upgrade_tracker_test.go

@@ -54,15 +54,15 @@ 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)
 	r.Len(storeUpgrades.Deleted, 0)
 	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)
 }