Skip to content

Commit

Permalink
x/upgrade: remove alias.go usage (#6382)
Browse files Browse the repository at this point in the history 
* x/upgrade: remove alias.go usage

* Simplify code

Co-authored-by: Alexander Bezobchuk <[email protected]>
Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@dauTT @alexanderbez
@fedekunze @mergify
4 people authored Jun 10, 2020
1 parent 79c308a commit 82afd52
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 89 deletions.
12 changes: 7 additions & 5 deletions simapp/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import (
"github.com/cosmos/cosmos-sdk/x/staking"
"github.com/cosmos/cosmos-sdk/x/upgrade"
upgradeclient "github.com/cosmos/cosmos-sdk/x/upgrade/client"
upgradekeeper "github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

const appName = "SimApp"
Expand Down Expand Up @@ -119,7 +121,7 @@ type SimApp struct {
DistrKeeper distr.Keeper
GovKeeper gov.Keeper
CrisisKeeper crisis.Keeper
UpgradeKeeper upgrade.Keeper
UpgradeKeeper upgradekeeper.Keeper
ParamsKeeper params.Keeper
IBCKeeper *ibc.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
EvidenceKeeper evidence.Keeper
Expand Down Expand Up @@ -152,7 +154,7 @@ func NewSimApp(
keys := sdk.NewKVStoreKeys(
auth.StoreKey, bank.StoreKey, staking.StoreKey,
mint.StoreKey, distr.StoreKey, slashing.StoreKey,
gov.StoreKey, params.StoreKey, ibc.StoreKey, upgrade.StoreKey,
gov.StoreKey, params.StoreKey, ibc.StoreKey, upgradetypes.StoreKey,
evidence.StoreKey, transfer.StoreKey, capability.StoreKey,
)
tkeys := sdk.NewTransientStoreKeys(params.TStoreKey)
Expand Down Expand Up @@ -212,14 +214,14 @@ func NewSimApp(
app.CrisisKeeper = crisis.NewKeeper(
app.subspaces[crisis.ModuleName], invCheckPeriod, app.BankKeeper, auth.FeeCollectorName,
)
app.UpgradeKeeper = upgrade.NewKeeper(skipUpgradeHeights, keys[upgrade.StoreKey], appCodec, homePath)
app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath)

// register the proposal types
govRouter := gov.NewRouter()
govRouter.AddRoute(gov.RouterKey, gov.ProposalHandler).
AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)).
AddRoute(distr.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)).
AddRoute(upgrade.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper))
AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper))
app.GovKeeper = gov.NewKeeper(
appCodec, keys[gov.StoreKey], app.subspaces[gov.ModuleName], app.AccountKeeper, app.BankKeeper,
&stakingKeeper, govRouter,
Expand Down Expand Up @@ -286,7 +288,7 @@ func NewSimApp(
// CanWithdrawInvariant invariant.
// NOTE: staking module is required if HistoricalEntries param > 0
app.mm.SetOrderBeginBlockers(
upgrade.ModuleName, mint.ModuleName, distr.ModuleName, slashing.ModuleName,
upgradetypes.ModuleName, mint.ModuleName, distr.ModuleName, slashing.ModuleName,
evidence.ModuleName, staking.ModuleName, ibc.ModuleName,
)
app.mm.SetOrderEndBlockers(crisis.ModuleName, gov.ModuleName, staking.ModuleName)
Expand Down
3 changes: 2 additions & 1 deletion x/upgrade/abci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
abci "github.com/tendermint/tendermint/abci/types"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
)

// BeginBlock will check if there is a scheduled plan and if it is ready to be executed.
Expand All @@ -16,7 +17,7 @@ import (
// The purpose is to ensure the binary is switched EXACTLY at the desired block, and to allow
// a migration to be executed if needed upon this switch (migration defined in the new binary)
// skipUpgradeHeightArray is a set of block heights for which the upgrade must be skipped
func BeginBlocker(k Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {
func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {
plan, found := k.GetUpgradePlan(ctx)
if !found {
return
Expand Down
58 changes: 30 additions & 28 deletions x/upgrade/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import (
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/x/gov"
"github.com/cosmos/cosmos-sdk/x/upgrade"
"github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
"github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

type TestSuite struct {
module module.AppModule
keeper upgrade.Keeper
keeper keeper.Keeper
querier sdk.Querier
handler gov.Handler
ctx sdk.Context
Expand Down Expand Up @@ -61,36 +63,36 @@ func setupTest(height int64, skip map[int64]bool) TestSuite {
func TestRequireName(t *testing.T) {
s := setupTest(10, map[int64]bool{})

err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{}})
require.NotNil(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestRequireFutureTime(t *testing.T) {
s := setupTest(10, map[int64]bool{})
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: s.ctx.BlockHeader().Time}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: s.ctx.BlockHeader().Time}})
require.NotNil(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestRequireFutureBlock(t *testing.T) {
s := setupTest(10, map[int64]bool{})
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight()}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight()}})
require.NotNil(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestCantSetBothTimeAndHeight(t *testing.T) {
s := setupTest(10, map[int64]bool{})
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now(), Height: s.ctx.BlockHeight() + 1}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: time.Now(), Height: s.ctx.BlockHeight() + 1}})
require.NotNil(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}

func TestDoTimeUpgrade(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify can schedule an upgrade")
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: time.Now()}})
require.Nil(t, err)

VerifyDoUpgrade(t)
Expand All @@ -99,7 +101,7 @@ func TestDoTimeUpgrade(t *testing.T) {
func TestDoHeightUpgrade(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify can schedule an upgrade")
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)

VerifyDoUpgrade(t)
Expand All @@ -108,9 +110,9 @@ func TestDoHeightUpgrade(t *testing.T) {
func TestCanOverwriteScheduleUpgrade(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Can overwrite plan")
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "bad_test", Height: s.ctx.BlockHeight() + 10}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "bad_test", Height: s.ctx.BlockHeight() + 10}})
require.Nil(t, err)
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)

VerifyDoUpgrade(t)
Expand All @@ -126,7 +128,7 @@ func VerifyDoUpgrade(t *testing.T) {
})

t.Log("Verify that the upgrade can be successfully applied with a handler")
s.keeper.SetUpgradeHandler("test", func(ctx sdk.Context, plan upgrade.Plan) {})
s.keeper.SetUpgradeHandler("test", func(ctx sdk.Context, plan types.Plan) {})
require.NotPanics(t, func() {
s.module.BeginBlock(newCtx, req)
})
Expand All @@ -142,7 +144,7 @@ func VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, proposalName strin
})

t.Log("Verify that the upgrade can be successfully applied with a handler")
s.keeper.SetUpgradeHandler(proposalName, func(ctx sdk.Context, plan upgrade.Plan) {})
s.keeper.SetUpgradeHandler(proposalName, func(ctx sdk.Context, plan types.Plan) {})
require.NotPanics(t, func() {
s.module.BeginBlock(newCtx, req)
})
Expand All @@ -154,7 +156,7 @@ func TestHaltIfTooNew(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify that we don't panic with registered plan not in database at all")
var called int
s.keeper.SetUpgradeHandler("future", func(ctx sdk.Context, plan upgrade.Plan) { called++ })
s.keeper.SetUpgradeHandler("future", func(ctx sdk.Context, plan types.Plan) { called++ })

newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now())
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
Expand All @@ -164,7 +166,7 @@ func TestHaltIfTooNew(t *testing.T) {
require.Equal(t, 0, called)

t.Log("Verify we panic if we have a registered handler ahead of time")
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "future", Height: s.ctx.BlockHeight() + 3}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "future", Height: s.ctx.BlockHeight() + 3}})
require.NoError(t, err)
require.Panics(t, func() {
s.module.BeginBlock(newCtx, req)
Expand All @@ -185,30 +187,30 @@ func TestHaltIfTooNew(t *testing.T) {

func VerifyCleared(t *testing.T, newCtx sdk.Context) {
t.Log("Verify that the upgrade plan has been cleared")
bz, err := s.querier(newCtx, []string{upgrade.QueryCurrent}, abci.RequestQuery{})
bz, err := s.querier(newCtx, []string{types.QueryCurrent}, abci.RequestQuery{})
require.NoError(t, err)
require.Nil(t, bz)
}

func TestCanClear(t *testing.T) {
s := setupTest(10, map[int64]bool{})
t.Log("Verify upgrade is scheduled")
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: time.Now()}})
require.Nil(t, err)

err = s.handler(s.ctx, &upgrade.CancelSoftwareUpgradeProposal{Title: "cancel"})
err = s.handler(s.ctx, &types.CancelSoftwareUpgradeProposal{Title: "cancel"})
require.Nil(t, err)

VerifyCleared(t, s.ctx)
}

func TestCantApplySameUpgradeTwice(t *testing.T) {
s := setupTest(10, map[int64]bool{})
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: time.Now()}})
require.Nil(t, err)
VerifyDoUpgrade(t)
t.Log("Verify an executed upgrade \"test\" can't be rescheduled")
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Time: time.Now()}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Time: time.Now()}})
require.NotNil(t, err)
require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err)
}
Expand All @@ -228,11 +230,11 @@ func TestPlanStringer(t *testing.T) {
require.Equal(t, `Upgrade Plan
Name: test
Time: 2020-01-01T00:00:00Z
Info: `, upgrade.Plan{Name: "test", Time: ti}.String())
Info: `, types.Plan{Name: "test", Time: ti}.String())
require.Equal(t, `Upgrade Plan
Name: test
Height: 100
Info: `, upgrade.Plan{Name: "test", Height: 100}.String())
Info: `, types.Plan{Name: "test", Height: 100}.String())
}

func VerifyNotDone(t *testing.T, newCtx sdk.Context, name string) {
Expand Down Expand Up @@ -279,7 +281,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) {
newCtx := s.ctx

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}})
require.NoError(t, err)

t.Log("Verify if skip upgrade flag clears upgrade plan in both cases")
Expand All @@ -291,7 +293,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) {
})

t.Log("Verify a second proposal also is being cleared")
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}})
require.NoError(t, err)

newCtx = newCtx.WithBlockHeight(skipTwo)
Expand All @@ -316,7 +318,7 @@ func TestUpgradeSkippingOne(t *testing.T) {
newCtx := s.ctx

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}})
require.Nil(t, err)

t.Log("Verify if skip upgrade flag clears upgrade plan in one case and does upgrade on another")
Expand All @@ -329,7 +331,7 @@ func TestUpgradeSkippingOne(t *testing.T) {
})

t.Log("Verify the second proposal is not skipped")
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}})
require.Nil(t, err)
// Setting block height of proposal test2
newCtx = newCtx.WithBlockHeight(skipTwo)
Expand All @@ -351,7 +353,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {
newCtx := s.ctx

req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: skipOne}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}})
require.Nil(t, err)

t.Log("Verify if skip upgrade flag clears upgrade plan in both cases and does third upgrade")
Expand All @@ -364,7 +366,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {
})

// A new proposal with height in skipUpgradeHeights
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop2", Plan: upgrade.Plan{Name: "test2", Height: skipTwo}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}})
require.Nil(t, err)
// Setting block height of proposal test2
newCtx = newCtx.WithBlockHeight(skipTwo)
Expand All @@ -373,7 +375,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) {
})

t.Log("Verify a new proposal is not skipped")
err = s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop3", Plan: upgrade.Plan{Name: "test3", Height: skipThree}})
err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop3", Plan: types.Plan{Name: "test3", Height: skipThree}})
require.Nil(t, err)
newCtx = newCtx.WithBlockHeight(skipThree)
VerifyDoUpgradeWithCtx(t, newCtx, "test3")
Expand All @@ -388,7 +390,7 @@ func TestUpgradeWithoutSkip(t *testing.T) {
s := setupTest(10, map[int64]bool{})
newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now())
req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()}
err := s.handler(s.ctx, &upgrade.SoftwareUpgradeProposal{Title: "prop", Plan: upgrade.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}})
require.Nil(t, err)
t.Log("Verify if upgrade happens without skip upgrade")
require.Panics(t, func() {
Expand Down
40 changes: 0 additions & 40 deletions x/upgrade/alias.go
View file

This file was deleted.

12 changes: 7 additions & 5 deletions x/upgrade/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,20 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/upgrade/keeper"
"github.com/cosmos/cosmos-sdk/x/upgrade/types"
)

// NewSoftwareUpgradeProposalHandler creates a governance handler to manage new proposal types.
// It enables SoftwareUpgradeProposal to propose an Upgrade, and CancelSoftwareUpgradeProposal
// to abort a previously voted upgrade.
func NewSoftwareUpgradeProposalHandler(k Keeper) govtypes.Handler {
func NewSoftwareUpgradeProposalHandler(k keeper.Keeper) govtypes.Handler {
return func(ctx sdk.Context, content govtypes.Content) error {
switch c := content.(type) {
case *SoftwareUpgradeProposal:
case *types.SoftwareUpgradeProposal:
return handleSoftwareUpgradeProposal(ctx, k, c)

case *CancelSoftwareUpgradeProposal:
case *types.CancelSoftwareUpgradeProposal:
return handleCancelSoftwareUpgradeProposal(ctx, k, c)

default:
Expand All @@ -24,11 +26,11 @@ func NewSoftwareUpgradeProposalHandler(k Keeper) govtypes.Handler {
}
}

func handleSoftwareUpgradeProposal(ctx sdk.Context, k Keeper, p *SoftwareUpgradeProposal) error {
func handleSoftwareUpgradeProposal(ctx sdk.Context, k keeper.Keeper, p *types.SoftwareUpgradeProposal) error {
return k.ScheduleUpgrade(ctx, p.Plan)
}

func handleCancelSoftwareUpgradeProposal(ctx sdk.Context, k Keeper, _ *CancelSoftwareUpgradeProposal) error {
func handleCancelSoftwareUpgradeProposal(ctx sdk.Context, k keeper.Keeper, _ *types.CancelSoftwareUpgradeProposal) error {
k.ClearUpgradePlan(ctx)
return nil
}
Loading

0 comments on commit 82afd52

Please sign in to comment.