Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refine upgrade module for code quality #144

Merged
merged 1 commit into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 0 additions & 146 deletions x/upgrade/doc.go

This file was deleted.

21 changes: 1 addition & 20 deletions x/upgrade/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}

Expand Down
1 change: 1 addition & 0 deletions x/upgrade/keeper/keeper_option.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
43 changes: 0 additions & 43 deletions x/upgrade/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
10 changes: 0 additions & 10 deletions x/upgrade/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions x/upgrade/types/upgrade_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down