chore: refine upgrade module for code quality (#144)

x/upgrade/keeper/keeper.go

@@ -27,7 +27,7 @@ type Keeper struct {
 	cdc                codec.BinaryCodec                   // App-wide binary codec
 	upgradeHandlers    map[string]types.UpgradeHandler     // map of plan name to upgrade handler
 	upgradeInitializer map[string]types.UpgradeInitializer // map of plan name to upgrade initializer
-	upgradeConfig      *types.UpgradeConfig
+	upgradeConfig      *types.UpgradeConfig                // upgrade config for upcoming upgrade plan or upgraded plan
 }
 
 // NewKeeper constructs an upgrade Keeper which requires the following arguments:
@@ -178,24 +178,6 @@ func (k Keeper) ScheduleUpgrade(ctx sdk.Context, plan types.Plan) error {
 	return nil
 }
 
-// SetUpgradedClient sets the expected upgraded client for the next version of this chain at the last height the current chain will commit.
-func (k Keeper) SetUpgradedClient(ctx sdk.Context, planHeight int64, bz []byte) error {
-	store := ctx.KVStore(k.storeKey)
-	store.Set(types.UpgradedClientKey(planHeight), bz)
-	return nil
-}
-
-// GetUpgradedClient gets the expected upgraded client for the next version of this chain
-func (k Keeper) GetUpgradedClient(ctx sdk.Context, height int64) ([]byte, bool) {
-	store := ctx.KVStore(k.storeKey)
-	bz := store.Get(types.UpgradedClientKey(height))
-	if len(bz) == 0 {
-		return nil, false
-	}
-
-	return bz, true
-}
-
 // SetUpgradedConsensusState set the expected upgraded consensus state for the next version of this chain
 // using the last height committed on this chain.
 func (k Keeper) SetUpgradedConsensusState(ctx sdk.Context, planHeight int64, bz []byte) error {
@@ -262,7 +244,6 @@ func (k Keeper) GetDoneHeight(ctx sdk.Context, name string) int64 {
 func (k Keeper) ClearIBCState(ctx sdk.Context, lastHeight int64) {
 	// delete IBC client and consensus state from store if this is IBC plan
 	store := ctx.KVStore(k.storeKey)
-	store.Delete(types.UpgradedClientKey(lastHeight))
 	store.Delete(types.UpgradedConsStateKey(lastHeight))
 }
 

x/upgrade/keeper/keeper_option.go

@@ -5,6 +5,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
+// convertUpgradeConfig converts serverconfig.UpgradeConfig to types.UpgradeConfig
 func convertUpgradeConfig(chainID string, plans []serverconfig.UpgradeConfig) (*types.UpgradeConfig, error) {
 	upgradeConfig := types.NewUpgradeConfig()
 	if chainID == types.MainnetChainID {

x/upgrade/keeper/keeper_test.go

@@ -157,49 +157,6 @@ func (s *KeeperTestSuite) TestScheduleUpgrade() {
 	}
 }
 
-func (s *KeeperTestSuite) TestSetUpgradedClient() {
-	cs := []byte("IBC client state")
-
-	cases := []struct {
-		name   string
-		height int64
-		setup  func()
-		exists bool
-	}{
-		{
-			name:   "no upgraded client exists",
-			height: 10,
-			setup:  func() {},
-			exists: false,
-		},
-		{
-			name:   "success",
-			height: 10,
-			setup: func() {
-				s.app.UpgradeKeeper.SetUpgradedClient(s.ctx, 10, cs)
-			},
-			exists: true,
-		},
-	}
-
-	for _, tc := range cases {
-		// reset suite
-		s.SetupTest()
-
-		// setup test case
-		tc.setup()
-
-		gotCs, exists := s.app.UpgradeKeeper.GetUpgradedClient(s.ctx, tc.height)
-		if tc.exists {
-			s.Require().Equal(cs, gotCs, "valid case: %s did not retrieve correct client state", tc.name)
-			s.Require().True(exists, "valid case: %s did not retrieve client state", tc.name)
-		} else {
-			s.Require().Nil(gotCs, "invalid case: %s retrieved valid client state", tc.name)
-			s.Require().False(exists, "invalid case: %s retrieved valid client state", tc.name)
-		}
-	}
-}
-
 // Tests that the underlying state of x/upgrade is set correctly after
 // an upgrade.
 func (s *KeeperTestSuite) TestMigrations() {

x/upgrade/types/keys.go

@@ -26,20 +26,10 @@ const (
 	// KeyUpgradedIBCState is the key under which upgraded ibc state is stored in the upgrade store
 	KeyUpgradedIBCState = "upgradedIBCState"
 
-	// KeyUpgradedClient is the sub-key under which upgraded client state will be stored
-	KeyUpgradedClient = "upgradedClient"
-
 	// KeyUpgradedConsState is the sub-key under which upgraded consensus state will be stored
 	KeyUpgradedConsState = "upgradedConsState"
 )
 
-// UpgradedClientKey is the key under which the upgraded client state is saved
-// Connecting IBC chains can verify against the upgraded client in this path before
-// upgrading their clients
-func UpgradedClientKey(height int64) []byte {
-	return []byte(fmt.Sprintf("%s/%d/%s", KeyUpgradedIBCState, height, KeyUpgradedClient))
-}
-
 // UpgradedConsStateKey is the key under which the upgraded consensus state is saved
 // Connecting IBC chains can verify against the upgraded consensus state in this path before
 // upgrading their clients.

x/upgrade/types/upgrade_config.go

@@ -10,9 +10,11 @@ import (
 // )
 
 const (
+	// EnablePublicDelegationUpgrade is the upgrade name for enabling public delegation
 	EnablePublicDelegationUpgrade = "EnablePublicDelegationUpgrade"
 )
 
+// The default upgrade config for networks
 var (
 	MainnetChainID = "greenfield_9000-1"
 	MainnetConfig  = NewUpgradeConfig()
@@ -35,11 +37,13 @@ type key struct {
 	height int64
 }
 
+// UpgradeConfig is a list of upgrade plans
 type UpgradeConfig struct {
 	keys     map[string]*key
 	elements map[int64][]*Plan
 }
 
+// SetPlan sets a new upgrade plan
 func (c *UpgradeConfig) SetPlan(plan *Plan) *UpgradeConfig {
 	if key, ok := c.keys[plan.Name]; ok {
 		if c.elements[key.height][key.index].Height == plan.Height {
@@ -56,13 +60,15 @@ func (c *UpgradeConfig) SetPlan(plan *Plan) *UpgradeConfig {
 	return c
 }
 
+// Clear removes all upgrade plans at a given height
 func (c *UpgradeConfig) Clear(height int64) {
 	for _, plan := range c.elements[height] {
 		delete(c.keys, plan.Name)
 	}
 	c.elements[height] = nil
 }
 
+// GetPlan returns the upgrade plan at a given height
 func (c *UpgradeConfig) GetPlan(height int64) []*Plan {
 	plans, exist := c.elements[height]
 	if exist && len(plans) != 0 {