fix: Refactor GetLastCompletedUpgrade [v0.45.x]

CHANGELOG.md

@@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (x/upgrade) [#12264](https://github.com/cosmos/cosmos-sdk/pull/12264) Fix `GetLastCompleteUpgrade` to properly return the latest upgrade.
 * (x/crisis) [#12208](https://github.com/cosmos/cosmos-sdk/pull/12208) Fix progress index of crisis invariant assertion logs.
 
 ## [v0.45.5](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.5) - 2022-06-09

x/upgrade/keeper/keeper.go

@@ -26,6 +26,12 @@ import (
 // UpgradeInfoFileName file to store upgrade information
 const UpgradeInfoFileName string = "upgrade-info.json"
 
+// upgrade defines a comparable structure for sorting upgrades.
+type upgrade struct {
+	Name        string
+	BlockHeight int64
+}
+
 type Keeper struct {
 	homePath           string                          // root directory of app config
 	skipUpgradeHeights map[int64]bool                  // map of heights to skip for an upgrade
@@ -236,8 +242,22 @@ func (k Keeper) GetUpgradedConsensusState(ctx sdk.Context, lastHeight int64) ([]
 func (k Keeper) GetLastCompletedUpgrade(ctx sdk.Context) (string, int64) {
 	iter := sdk.KVStoreReversePrefixIterator(ctx.KVStore(k.storeKey), []byte{types.DoneByte})
 	defer iter.Close()
-	if iter.Valid() {
-		return parseDoneKey(iter.Key()), int64(binary.BigEndian.Uint64(iter.Value()))
+
+	var upgrades []upgrade
+	for ; iter.Valid(); iter.Next() {
+		name := parseDoneKey(iter.Key())
+		value := int64(sdk.BigEndianToUint64(iter.Value()))
+
+		upgrades = append(upgrades, upgrade{Name: name, BlockHeight: value})
+	}
+
+	// sort upgrades in descending order by block height
+	sort.SliceStable(upgrades, func(i, j int) bool {
+		return upgrades[i].BlockHeight > upgrades[j].BlockHeight
+	})
+
+	if len(upgrades) > 0 {
+		return upgrades[0].Name, upgrades[0].BlockHeight
 	}
 
 	return "", 0

x/upgrade/keeper/keeper_test.go

@@ -241,33 +241,33 @@ func (s *KeeperTestSuite) TestLastCompletedUpgrade() {
 	require.Equal("", name)
 	require.Equal(int64(0), height)
 
-	keeper.SetUpgradeHandler("test0", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
+	keeper.SetUpgradeHandler("test-v0.9", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
 		return vm, nil
 	})
 
 	keeper.ApplyUpgrade(s.ctx, types.Plan{
-		Name:   "test0",
+		Name:   "test-v0.9",
 		Height: 10,
 	})
 
 	s.T().Log("verify valid upgrade name and height")
 	name, height = keeper.GetLastCompletedUpgrade(s.ctx)
-	require.Equal("test0", name)
+	require.Equal("test-v0.9", name)
 	require.Equal(int64(10), height)
 
-	keeper.SetUpgradeHandler("test1", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
+	keeper.SetUpgradeHandler("test-v0.10", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) {
 		return vm, nil
 	})
 
 	newCtx := s.ctx.WithBlockHeight(15)
 	keeper.ApplyUpgrade(newCtx, types.Plan{
-		Name:   "test1",
+		Name:   "test-v0.10",
 		Height: 15,
 	})
 
 	s.T().Log("verify valid upgrade name and height with multiple upgrades")
 	name, height = keeper.GetLastCompletedUpgrade(newCtx)
-	require.Equal("test1", name)
+	require.Equal("test-v0.10", name)
 	require.Equal(int64(15), height)
 }