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.

Sign up for GitHub

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

Jump to bottom

x/upgrade: remove alias.go usage #6382

Merged
merged 5 commits into from
Jun 10, 2020
Merged

x/upgrade: remove alias.go usage #6382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9f0d4a
x/upgrade: remove alias.go usage
dauTT Jun 9, 2020
69a08ed
Merge branch 'master' into remove-alias-x-upgrade
alexanderbez Jun 10, 2020
c689e6b
Simplify code
dauTT Jun 10, 2020
4e0e3ad
Merge branch 'master' into remove-alias-x-upgrade
fedekunze Jun 10, 2020
3a7878f
Merge branch 'master' into remove-alias-x-upgrade
mergify[bot] Jun 10, 2020
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
12 changes: 7 additions & 5 deletions simapp/app.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

This file was deleted.

12 changes: 7 additions & 5 deletions x/upgrade/handler.go
View file
Open in desktop
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