From 1d8fe940dfb022ee21e234df7d8e609680b6e139 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 5 Jul 2023 23:45:06 +0200 Subject: [PATCH 1/3] chore(upgrade): clean up handler and typo --- simapp/app.go | 3 +-- x/upgrade/abci_test.go | 55 +++++++++++++++----------------------- x/upgrade/client/cli/tx.go | 4 +-- x/upgrade/handler.go | 41 ---------------------------- x/upgrade/module.go | 5 +--- 5 files changed, 25 insertions(+), 83 deletions(-) delete mode 100644 x/upgrade/handler.go diff --git a/simapp/app.go b/simapp/app.go index eb6e47fd6dca..fc837aa1c29f 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -370,8 +370,7 @@ func NewSimApp( // See: https://docs.cosmos.network/main/modules/gov#proposal-messages govRouter := govv1beta1.NewRouter() govRouter.AddRoute(govtypes.RouterKey, govv1beta1.ProposalHandler). - AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). - AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper)) + AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)) govConfig := govtypes.DefaultConfig() /* Example of setting gov params: diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index f013081d478c..ee25fc774182 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -28,7 +28,6 @@ import ( moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - govtypesv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) type TestSuite struct { @@ -36,7 +35,6 @@ type TestSuite struct { module appmodule.HasBeginBlocker keeper *keeper.Keeper - handler govtypesv1beta1.Handler ctx sdk.Context baseApp *baseapp.BaseApp encCfg moduletestutil.TestEncodingConfig @@ -64,21 +62,12 @@ func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite { s.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now(), Height: height}) s.module = upgrade.NewAppModule(s.keeper, addresscodec.NewBech32Codec("cosmos")) - s.handler = upgrade.NewSoftwareUpgradeProposalHandler(s.keeper) return &s } -func TestRequireName(t *testing.T) { - s := setupTest(t, 10, map[int64]bool{}) - - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{}}) //nolint:staticcheck // we're testing deprecated code - require.Error(t, err) - require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) -} - func TestRequireFutureBlock(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height - 1}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height - 1}) require.Error(t, err) require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } @@ -86,7 +75,7 @@ func TestRequireFutureBlock(t *testing.T) { func TestDoHeightUpgrade(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) t.Log("Verify can schedule an upgrade") - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}) require.NoError(t, err) VerifyDoUpgrade(t) @@ -95,9 +84,9 @@ func TestDoHeightUpgrade(t *testing.T) { func TestCanOverwriteScheduleUpgrade(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) t.Log("Can overwrite plan") - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "bad_test", Height: s.ctx.HeaderInfo().Height + 10}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "bad_test", Height: s.ctx.HeaderInfo().Height + 10}) require.NoError(t, err) - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}) require.NoError(t, err) VerifyDoUpgrade(t) @@ -156,7 +145,7 @@ func TestHaltIfTooNew(t *testing.T) { require.Equal(t, 0, called) t.Log("Verify we error if we have a registered handler ahead of time") - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "future", Height: s.ctx.HeaderInfo().Height + 3}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "future", Height: s.ctx.HeaderInfo().Height + 3}) require.NoError(t, err) err = s.module.BeginBlock(newCtx) @@ -183,10 +172,10 @@ func VerifyCleared(t *testing.T, newCtx sdk.Context) { func TestCanClear(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) t.Log("Verify upgrade is scheduled") - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 100}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 100}) require.NoError(t, err) - err = s.handler(s.ctx, &types.CancelSoftwareUpgradeProposal{Title: "cancel"}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ClearUpgradePlan(s.ctx) require.NoError(t, err) VerifyCleared(t, s.ctx) @@ -195,11 +184,11 @@ func TestCanClear(t *testing.T) { func TestCantApplySameUpgradeTwice(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) height := s.ctx.HeaderInfo().Height + 1 - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: height}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: height}) require.NoError(t, err) VerifyDoUpgrade(t) t.Log("Verify an executed upgrade \"test\" can't be rescheduled") - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: height}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: height}) require.Error(t, err) require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } @@ -262,7 +251,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) { newCtx := s.ctx - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: skipOne}) require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in both cases") @@ -273,7 +262,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) { require.NoError(t, err) t.Log("Verify a second proposal also is being cleared") - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test2", Height: skipTwo}) require.NoError(t, err) newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo}) @@ -296,7 +285,7 @@ func TestUpgradeSkippingOne(t *testing.T) { newCtx := s.ctx - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: skipOne}) require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in one case and does upgrade on another") @@ -308,7 +297,7 @@ func TestUpgradeSkippingOne(t *testing.T) { require.NoError(t, err) t.Log("Verify the second proposal is not skipped") - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test2", Height: skipTwo}) require.NoError(t, err) // Setting block height of proposal test2 newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo}) @@ -329,7 +318,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { newCtx := s.ctx - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: skipOne}) require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in both cases and does third upgrade") @@ -341,7 +330,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { require.NoError(t, err) // A new proposal with height in skipUpgradeHeights - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test2", Height: skipTwo}) require.NoError(t, err) // Setting block height of proposal test2 newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo}) @@ -349,7 +338,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { require.NoError(t, err) t.Log("Verify a new proposal is not skipped") - err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop3", Plan: types.Plan{Name: "test3", Height: skipThree}}) //nolint:staticcheck // we're testing deprecated code + err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test3", Height: skipThree}) require.NoError(t, err) newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipThree}) VerifyDoUpgradeWithCtx(t, newCtx, "test3") @@ -363,7 +352,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { func TestUpgradeWithoutSkip(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) newCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 1, Time: time.Now()}) - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}) require.NoError(t, err) t.Log("Verify if upgrade happens without skip upgrade") @@ -429,7 +418,7 @@ func TestBinaryVersion(t *testing.T) { return vm, nil }) - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test0", Height: s.ctx.HeaderInfo().Height + 2}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test0", Height: s.ctx.HeaderInfo().Height + 2}) require.NoError(t, err) newCtx := s.ctx.WithHeaderInfo(header.Info{Height: 12}) @@ -445,7 +434,7 @@ func TestBinaryVersion(t *testing.T) { { "test panic: upgrade needed", func() sdk.Context { - err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test2", Height: 13}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test2", Height: 13}) require.NoError(t, err) newCtx := s.ctx.WithHeaderInfo(header.Info{Height: 13}) @@ -479,11 +468,10 @@ func TestDowngradeVerification(t *testing.T) { tempDir := t.TempDir() k := keeper.NewKeeper(skip, storeService, encCfg.Codec, tempDir, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) m := upgrade.NewAppModule(k, addresscodec.NewBech32Codec("cosmos")) - handler := upgrade.NewSoftwareUpgradeProposalHandler(k) // submit a plan. planName := "downgrade" - err := handler(ctx, &types.SoftwareUpgradeProposal{Title: "test", Plan: types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(ctx, types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1}) require.NoError(t, err) ctx = ctx.WithHeaderInfo(header.Info{Height: ctx.HeaderInfo().Height + 1}) @@ -509,8 +497,7 @@ func TestDowngradeVerification(t *testing.T) { }, "downgrade with an active plan": { preRun: func(k *keeper.Keeper, ctx sdk.Context, name string) { - handler := upgrade.NewSoftwareUpgradeProposalHandler(k) - err := handler(ctx, &types.SoftwareUpgradeProposal{Title: "test", Plan: types.Plan{Name: "another" + planName, Height: ctx.HeaderInfo().Height + 1}}) //nolint:staticcheck // we're testing deprecated code + err := s.keeper.ScheduleUpgrade(ctx, types.Plan{Name: "another" + planName, Height: ctx.HeaderInfo().Height + 1}) require.NoError(t, err, name) }, expectError: true, diff --git a/x/upgrade/client/cli/tx.go b/x/upgrade/client/cli/tx.go index 1f2681c69595..294538fdc6d4 100644 --- a/x/upgrade/client/cli/tx.go +++ b/x/upgrade/client/cli/tx.go @@ -110,7 +110,7 @@ func NewCmdSubmitUpgradeProposal(ac addresscodec.Codec) *cobra.Command { Plan: p, }, }); err != nil { - return fmt.Errorf("failed to create cancel upgrade message: %w", err) + return fmt.Errorf("failed to create submit upgrade proposal message: %w", err) } return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), proposal) @@ -164,7 +164,7 @@ func NewCmdSubmitCancelUpgradeProposal(ac addresscodec.Codec) *cobra.Command { Authority: authority, }, }); err != nil { - return fmt.Errorf("failed to create cancel upgrade message: %w", err) + return fmt.Errorf("failed to create cancel upgrade proposal message: %w", err) } return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), proposal) diff --git a/x/upgrade/handler.go b/x/upgrade/handler.go deleted file mode 100644 index fb17bef95ed7..000000000000 --- a/x/upgrade/handler.go +++ /dev/null @@ -1,41 +0,0 @@ -package upgrade - -import ( - errorsmod "cosmossdk.io/errors" - "cosmossdk.io/x/upgrade/keeper" - "cosmossdk.io/x/upgrade/types" - - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" -) - -// 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. -// -//nolint:staticcheck // we are intentionally using a deprecated proposal here. -func NewSoftwareUpgradeProposalHandler(k *keeper.Keeper) govtypes.Handler { - return func(ctx sdk.Context, content govtypes.Content) error { - switch c := content.(type) { - case *types.SoftwareUpgradeProposal: - return handleSoftwareUpgradeProposal(ctx, k, c) - - case *types.CancelSoftwareUpgradeProposal: - return handleCancelSoftwareUpgradeProposal(ctx, k, c) - - default: - return errorsmod.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized software upgrade proposal content type: %T", c) - } - } -} - -//nolint:staticcheck // we are intentionally using a deprecated proposal here. -func handleSoftwareUpgradeProposal(ctx sdk.Context, k *keeper.Keeper, p *types.SoftwareUpgradeProposal) error { - return k.ScheduleUpgrade(ctx, p.Plan) -} - -//nolint:staticcheck // we are intentionally using a deprecated proposal here. -func handleCancelSoftwareUpgradeProposal(ctx sdk.Context, k *keeper.Keeper, _ *types.CancelSoftwareUpgradeProposal) error { - return k.ClearUpgradePlan(ctx) -} diff --git a/x/upgrade/module.go b/x/upgrade/module.go index 5bbb52df2469..f7cfa767056a 100644 --- a/x/upgrade/module.go +++ b/x/upgrade/module.go @@ -31,7 +31,6 @@ import ( "github.com/cosmos/cosmos-sdk/types/module" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) func init() { @@ -195,7 +194,6 @@ type ModuleOutputs struct { UpgradeKeeper *keeper.Keeper Module appmodule.AppModule - GovHandler govv1beta1.HandlerRoute BaseAppOption runtime.BaseAppOption } @@ -225,9 +223,8 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k.SetVersionSetter(app) } m := NewAppModule(k, in.AddressCodec) - gh := govv1beta1.HandlerRoute{RouteKey: types.RouterKey, Handler: NewSoftwareUpgradeProposalHandler(k)} - return ModuleOutputs{UpgradeKeeper: k, Module: m, GovHandler: gh, BaseAppOption: baseappOpt} + return ModuleOutputs{UpgradeKeeper: k, Module: m, BaseAppOption: baseappOpt} } func PopulateVersionMap(upgradeKeeper *keeper.Keeper, modules map[string]appmodule.AppModule) { From 27d4aaca43d2aee1d23777ef721d42ec0e373323 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 5 Jul 2023 23:48:24 +0200 Subject: [PATCH 2/3] changelog --- x/upgrade/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/x/upgrade/CHANGELOG.md b/x/upgrade/CHANGELOG.md index b265582235a3..352cf30ce3df 100644 --- a/x/upgrade/CHANGELOG.md +++ b/x/upgrade/CHANGELOG.md @@ -32,6 +32,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes +* [#16845](https://github.com/cosmos/cosmos-sdk/pull/16845) Remove gov v1beta1 handler. Use gov v1 proposals directly, or replicate the handler in your app. * [#16511](https://github.com/cosmos/cosmos-sdk/pull/16511) `BinaryDownloadURLMap.ValidateBasic()` and `BinaryDownloadURLMap.CheckURLs` now both take a checksum parameter when willing to ensure a checksum is provided for each URL. * [#16511](https://github.com/cosmos/cosmos-sdk/pull/16511) `plan.DownloadURLWithChecksum` has been renamed to `plan.DownloadURL` and does not validate the URL anymore. Call `plan.ValidateURL` before calling `plan.DownloadURL` to validate the URL. * [#16511](https://github.com/cosmos/cosmos-sdk/pull/16511) `plan.DownloadUpgrade` does not validate URL anymore. Call `plan.ValidateURL` before calling `plan.DownloadUpgrade` to validate the URL. From ce65be79970e20592907e5d2c92b19f328dfe1e6 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 6 Jul 2023 15:53:39 +0200 Subject: [PATCH 3/3] remove weird global --- x/upgrade/abci_test.go | 192 ++++++++++++++++++++--------------------- 1 file changed, 95 insertions(+), 97 deletions(-) diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index ee25fc774182..7fce44b878f4 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -40,10 +40,78 @@ type TestSuite struct { encCfg moduletestutil.TestEncodingConfig } -var s TestSuite +func (s *TestSuite) VerifyDoUpgrade(t *testing.T) { + t.Helper() + t.Log("Verify that a panic happens at the upgrade height") + newCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 1, Time: time.Now()}) + + err := s.module.BeginBlock(newCtx) + require.ErrorContains(t, err, "UPGRADE \"test\" NEEDED at height: 11: ") + + t.Log("Verify that the upgrade can be successfully applied with a handler") + s.keeper.SetUpgradeHandler("test", func(ctx context.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }) + + err = s.module.BeginBlock(newCtx) + require.NoError(t, err) + + s.VerifyCleared(t, newCtx) +} + +func (s *TestSuite) VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, proposalName string) { + t.Helper() + t.Log("Verify that a panic happens at the upgrade height") + + err := s.module.BeginBlock(newCtx) + require.ErrorContains(t, err, "UPGRADE \""+proposalName+"\" NEEDED at height: ") + + t.Log("Verify that the upgrade can be successfully applied with a handler") + s.keeper.SetUpgradeHandler(proposalName, func(ctx context.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }) + + err = s.module.BeginBlock(newCtx) + require.NoError(t, err) + + s.VerifyCleared(t, newCtx) +} + +func (s *TestSuite) VerifyCleared(t *testing.T, newCtx sdk.Context) { + t.Helper() + t.Log("Verify that the upgrade plan has been cleared") + _, err := s.keeper.GetUpgradePlan(newCtx) + require.ErrorIs(t, err, types.ErrNoUpgradePlanFound) +} + +func (s *TestSuite) VerifyNotDone(t *testing.T, newCtx sdk.Context, name string) { + t.Helper() + t.Log("Verify that upgrade was not done") + height, err := s.keeper.GetDoneHeight(newCtx, name) + require.Zero(t, height) + require.NoError(t, err) +} + +func (s *TestSuite) VerifyDone(t *testing.T, newCtx sdk.Context, name string) { + t.Helper() + t.Log("Verify that the upgrade plan has been executed") + height, err := s.keeper.GetDoneHeight(newCtx, name) + require.NotZero(t, height) + require.NoError(t, err) +} + +func (s *TestSuite) VerifySet(t *testing.T, skipUpgradeHeights map[int64]bool) { + t.Helper() + t.Log("Verify if the skip upgrade has been set") + + for k := range skipUpgradeHeights { + require.True(t, s.keeper.IsSkipHeight(k)) + } +} func setupTest(t *testing.T, height int64, skip map[int64]bool) *TestSuite { t.Helper() + s := TestSuite{} s.encCfg = moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}) key := storetypes.NewKVStoreKey(types.StoreKey) storeService := runtime.NewKVStoreService(key) @@ -78,7 +146,7 @@ func TestDoHeightUpgrade(t *testing.T) { err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}) require.NoError(t, err) - VerifyDoUpgrade(t) + s.VerifyDoUpgrade(t) } func TestCanOverwriteScheduleUpgrade(t *testing.T) { @@ -89,44 +157,7 @@ func TestCanOverwriteScheduleUpgrade(t *testing.T) { err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: s.ctx.HeaderInfo().Height + 1}) require.NoError(t, err) - VerifyDoUpgrade(t) -} - -func VerifyDoUpgrade(t *testing.T) { - t.Helper() - t.Log("Verify that a panic happens at the upgrade height") - newCtx := s.ctx.WithHeaderInfo(header.Info{Height: s.ctx.HeaderInfo().Height + 1, Time: time.Now()}) - - err := s.module.BeginBlock(newCtx) - require.ErrorContains(t, err, "UPGRADE \"test\" NEEDED at height: 11: ") - - t.Log("Verify that the upgrade can be successfully applied with a handler") - s.keeper.SetUpgradeHandler("test", func(ctx context.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { - return vm, nil - }) - - err = s.module.BeginBlock(newCtx) - require.NoError(t, err) - - VerifyCleared(t, newCtx) -} - -func VerifyDoUpgradeWithCtx(t *testing.T, newCtx sdk.Context, proposalName string) { - t.Helper() - t.Log("Verify that a panic happens at the upgrade height") - - err := s.module.BeginBlock(newCtx) - require.ErrorContains(t, err, "UPGRADE \""+proposalName+"\" NEEDED at height: ") - - t.Log("Verify that the upgrade can be successfully applied with a handler") - s.keeper.SetUpgradeHandler(proposalName, func(ctx context.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { - return vm, nil - }) - - err = s.module.BeginBlock(newCtx) - require.NoError(t, err) - - VerifyCleared(t, newCtx) + s.VerifyDoUpgrade(t) } func TestHaltIfTooNew(t *testing.T) { @@ -159,14 +190,7 @@ func TestHaltIfTooNew(t *testing.T) { require.NoError(t, err) require.Equal(t, 1, called) - VerifyCleared(t, futCtx) -} - -func VerifyCleared(t *testing.T, newCtx sdk.Context) { - t.Helper() - t.Log("Verify that the upgrade plan has been cleared") - _, err := s.keeper.GetUpgradePlan(newCtx) - require.ErrorIs(t, err, types.ErrNoUpgradePlanFound) + s.VerifyCleared(t, futCtx) } func TestCanClear(t *testing.T) { @@ -178,7 +202,7 @@ func TestCanClear(t *testing.T) { err = s.keeper.ClearUpgradePlan(s.ctx) require.NoError(t, err) - VerifyCleared(t, s.ctx) + s.VerifyCleared(t, s.ctx) } func TestCantApplySameUpgradeTwice(t *testing.T) { @@ -186,7 +210,7 @@ func TestCantApplySameUpgradeTwice(t *testing.T) { height := s.ctx.HeaderInfo().Height + 1 err := s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: height}) require.NoError(t, err) - VerifyDoUpgrade(t) + s.VerifyDoUpgrade(t) t.Log("Verify an executed upgrade \"test\" can't be rescheduled") err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test", Height: height}) require.Error(t, err) @@ -205,36 +229,11 @@ func TestPlanStringer(t *testing.T) { require.Equal(t, `name:"test" time: height:100 `, (&types.Plan{Name: "test", Height: 100, Info: ""}).String()) } -func VerifyNotDone(t *testing.T, newCtx sdk.Context, name string) { - t.Helper() - t.Log("Verify that upgrade was not done") - height, err := s.keeper.GetDoneHeight(newCtx, name) - require.Zero(t, height) - require.NoError(t, err) -} - -func VerifyDone(t *testing.T, newCtx sdk.Context, name string) { - t.Helper() - t.Log("Verify that the upgrade plan has been executed") - height, err := s.keeper.GetDoneHeight(newCtx, name) - require.NotZero(t, height) - require.NoError(t, err) -} - -func VerifySet(t *testing.T, skipUpgradeHeights map[int64]bool) { - t.Helper() - t.Log("Verify if the skip upgrade has been set") - - for k := range skipUpgradeHeights { - require.True(t, s.keeper.IsSkipHeight(k)) - } -} - func TestContains(t *testing.T) { var skipOne int64 = 11 s := setupTest(t, 10, map[int64]bool{skipOne: true}) - VerifySet(t, map[int64]bool{skipOne: true}) + s.VerifySet(t, map[int64]bool{skipOne: true}) t.Log("case where array contains the element") require.True(t, s.keeper.IsSkipHeight(11)) @@ -255,7 +254,7 @@ func TestSkipUpgradeSkippingAll(t *testing.T) { require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in both cases") - VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) + s.VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne}) err = s.module.BeginBlock(newCtx) @@ -271,9 +270,9 @@ func TestSkipUpgradeSkippingAll(t *testing.T) { // To ensure verification is being done only after both upgrades are cleared t.Log("Verify if both proposals are cleared") - VerifyCleared(t, s.ctx) - VerifyNotDone(t, s.ctx, "test") - VerifyNotDone(t, s.ctx, "test2") + s.VerifyCleared(t, s.ctx) + s.VerifyNotDone(t, s.ctx, "test") + s.VerifyNotDone(t, s.ctx, "test2") } func TestUpgradeSkippingOne(t *testing.T) { @@ -289,7 +288,7 @@ func TestUpgradeSkippingOne(t *testing.T) { require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in one case and does upgrade on another") - VerifySet(t, map[int64]bool{skipOne: true}) + s.VerifySet(t, map[int64]bool{skipOne: true}) // Setting block height of proposal test newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne}) @@ -301,11 +300,11 @@ func TestUpgradeSkippingOne(t *testing.T) { require.NoError(t, err) // Setting block height of proposal test2 newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipTwo}) - VerifyDoUpgradeWithCtx(t, newCtx, "test2") + s.VerifyDoUpgradeWithCtx(t, newCtx, "test2") t.Log("Verify first proposal is cleared and second is done") - VerifyNotDone(t, s.ctx, "test") - VerifyDone(t, s.ctx, "test2") + s.VerifyNotDone(t, s.ctx, "test") + s.VerifyDone(t, s.ctx, "test2") } func TestUpgradeSkippingOnlyTwo(t *testing.T) { @@ -322,7 +321,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { require.NoError(t, err) t.Log("Verify if skip upgrade flag clears upgrade plan in both cases and does third upgrade") - VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) + s.VerifySet(t, map[int64]bool{skipOne: true, skipTwo: true}) // Setting block height of proposal test newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipOne}) @@ -341,12 +340,12 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { err = s.keeper.ScheduleUpgrade(s.ctx, types.Plan{Name: "test3", Height: skipThree}) require.NoError(t, err) newCtx = newCtx.WithHeaderInfo(header.Info{Height: skipThree}) - VerifyDoUpgradeWithCtx(t, newCtx, "test3") + s.VerifyDoUpgradeWithCtx(t, newCtx, "test3") t.Log("Verify two proposals are cleared and third is done") - VerifyNotDone(t, s.ctx, "test") - VerifyNotDone(t, s.ctx, "test2") - VerifyDone(t, s.ctx, "test3") + s.VerifyNotDone(t, s.ctx, "test") + s.VerifyNotDone(t, s.ctx, "test2") + s.VerifyDone(t, s.ctx, "test3") } func TestUpgradeWithoutSkip(t *testing.T) { @@ -359,8 +358,8 @@ func TestUpgradeWithoutSkip(t *testing.T) { err = s.module.BeginBlock(newCtx) require.ErrorContains(t, err, "UPGRADE \"test\" NEEDED at height:") - VerifyDoUpgrade(t) - VerifyDone(t, s.ctx, "test") + s.VerifyDoUpgrade(t) + s.VerifyDone(t, s.ctx, "test") } func TestDumpUpgradeInfoToFile(t *testing.T) { @@ -461,17 +460,16 @@ func TestDowngradeVerification(t *testing.T) { encCfg := moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}) key := storetypes.NewKVStoreKey(types.StoreKey) storeService := runtime.NewKVStoreService(key) - testCtx := testutil.DefaultContextWithDB(s.T(), key, storetypes.NewTransientStoreKey("transient_test")) + testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now(), Height: 10}) skip := map[int64]bool{} - tempDir := t.TempDir() - k := keeper.NewKeeper(skip, storeService, encCfg.Codec, tempDir, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) m := upgrade.NewAppModule(k, addresscodec.NewBech32Codec("cosmos")) // submit a plan. planName := "downgrade" - err := s.keeper.ScheduleUpgrade(ctx, types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1}) + err := k.ScheduleUpgrade(ctx, types.Plan{Name: planName, Height: ctx.HeaderInfo().Height + 1}) require.NoError(t, err) ctx = ctx.WithHeaderInfo(header.Info{Height: ctx.HeaderInfo().Height + 1}) @@ -497,7 +495,7 @@ func TestDowngradeVerification(t *testing.T) { }, "downgrade with an active plan": { preRun: func(k *keeper.Keeper, ctx sdk.Context, name string) { - err := s.keeper.ScheduleUpgrade(ctx, types.Plan{Name: "another" + planName, Height: ctx.HeaderInfo().Height + 1}) + err := k.ScheduleUpgrade(ctx, types.Plan{Name: "another" + planName, Height: ctx.HeaderInfo().Height + 1}) require.NoError(t, err, name) }, expectError: true, @@ -511,7 +509,7 @@ func TestDowngradeVerification(t *testing.T) { ctx, _ := ctx.CacheContext() // downgrade. now keeper does not have the handler. - k := keeper.NewKeeper(skip, storeService, encCfg.Codec, tempDir, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + k := keeper.NewKeeper(skip, storeService, encCfg.Codec, t.TempDir(), nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) m := upgrade.NewAppModule(k, addresscodec.NewBech32Codec("cosmos")) // assertions