diff --git a/CHANGELOG.md b/CHANGELOG.md index 1afd94bab..c5ee595fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (x/upgrade) [\#11551](https://github.com/cosmos/cosmos-sdk/pull/11551) Update `ScheduleUpgrade` for chains to schedule an automated upgrade on `BeginBlock` without having to go though governance. * (cli) [\#11548](https://github.com/cosmos/cosmos-sdk/pull/11548) Add Tendermint's `inspect` command to the `tendermint` sub-command. * (tx) [#\11533](https://github.com/cosmos/cosmos-sdk/pull/11533) Register [`EIP191`](https://eips.ethereum.org/EIPS/eip-191) as an available `SignMode` for chains to use. * (x/genutil) [\#11500](https://github.com/cosmos/cosmos-sdk/pull/11500) Fix GenTx validation and adjust error messages diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index c3f1ea6bd..8f3b8f52d 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -34,7 +34,6 @@ type TestSuite struct { var s TestSuite func setupTest(t *testing.T, height int64, skip map[int64]bool) TestSuite { - db := dbm.NewMemDB() app := simapp.NewSimappWithCustomOptions(t, false, simapp.SetupOptions{ Logger: log.NewNopLogger(), @@ -59,14 +58,14 @@ func TestRequireName(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{}}) - require.NotNil(t, err) + 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.BlockHeight()}}) - require.NotNil(t, err) + err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() - 1}}) + require.Error(t, err) require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } @@ -74,7 +73,7 @@ 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.BlockHeight() + 1}}) - require.Nil(t, err) + require.NoError(t, err) VerifyDoUpgrade(t) } @@ -83,9 +82,9 @@ 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.BlockHeight() + 10}}) - require.Nil(t, err) + require.NoError(t, err) err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}}) - require.Nil(t, err) + require.NoError(t, err) VerifyDoUpgrade(t) } @@ -132,7 +131,7 @@ func TestHaltIfTooNew(t *testing.T) { s := setupTest(t, 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 types.Plan, vm module.VersionMap) (module.VersionMap, error) { + s.keeper.SetUpgradeHandler("future", func(_ sdk.Context, _ types.Plan, vm module.VersionMap) (module.VersionMap, error) { called++ return vm, nil }) @@ -175,10 +174,10 @@ 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.BlockHeight() + 100}}) - require.Nil(t, err) + require.NoError(t, err) err = s.handler(s.ctx, &types.CancelSoftwareUpgradeProposal{Title: "cancel"}) - require.Nil(t, err) + require.NoError(t, err) VerifyCleared(t, s.ctx) } @@ -187,11 +186,11 @@ func TestCantApplySameUpgradeTwice(t *testing.T) { s := setupTest(t, 10, map[int64]bool{}) height := s.ctx.BlockHeader().Height + 1 err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: height}}) - require.Nil(t, err) + 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}}) - require.NotNil(t, err) + require.Error(t, err) require.True(t, errors.Is(sdkerrors.ErrInvalidRequest, err), err) } @@ -237,9 +236,7 @@ func VerifySet(t *testing.T, skipUpgradeHeights map[int64]bool) { } func TestContains(t *testing.T) { - var ( - skipOne int64 = 11 - ) + var skipOne int64 = 11 s := setupTest(t, 10, map[int64]bool{skipOne: true}) VerifySet(t, map[int64]bool{skipOne: true}) @@ -298,7 +295,7 @@ func TestUpgradeSkippingOne(t *testing.T) { req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}}) - require.Nil(t, err) + 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}) @@ -311,7 +308,7 @@ func TestUpgradeSkippingOne(t *testing.T) { 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}}) - require.Nil(t, err) + require.NoError(t, err) // Setting block height of proposal test2 newCtx = newCtx.WithBlockHeight(skipTwo) VerifyDoUpgradeWithCtx(t, newCtx, "test2") @@ -333,7 +330,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: skipOne}}) - require.Nil(t, err) + 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}) @@ -346,7 +343,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { // A new proposal with height in skipUpgradeHeights err = s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop2", Plan: types.Plan{Name: "test2", Height: skipTwo}}) - require.Nil(t, err) + require.NoError(t, err) // Setting block height of proposal test2 newCtx = newCtx.WithBlockHeight(skipTwo) require.NotPanics(t, func() { @@ -355,7 +352,7 @@ func TestUpgradeSkippingOnlyTwo(t *testing.T) { 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}}) - require.Nil(t, err) + require.NoError(t, err) newCtx = newCtx.WithBlockHeight(skipThree) VerifyDoUpgradeWithCtx(t, newCtx, "test3") @@ -370,7 +367,7 @@ func TestUpgradeWithoutSkip(t *testing.T) { newCtx := s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 1).WithBlockTime(time.Now()) req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "prop", Plan: types.Plan{Name: "test", Height: s.ctx.BlockHeight() + 1}}) - require.Nil(t, err) + require.NoError(t, err) t.Log("Verify if upgrade happens without skip upgrade") require.Panics(t, func() { s.module.BeginBlock(newCtx, req) @@ -437,7 +434,7 @@ func TestBinaryVersion(t *testing.T) { }) err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test0", Height: s.ctx.BlockHeight() + 2}}) - require.Nil(t, err) + require.NoError(t, err) newCtx := s.ctx.WithBlockHeight(12) s.keeper.ApplyUpgrade(newCtx, types.Plan{ @@ -454,7 +451,7 @@ func TestBinaryVersion(t *testing.T) { "test panic: upgrade needed", func() (sdk.Context, abci.RequestBeginBlock) { err := s.handler(s.ctx, &types.SoftwareUpgradeProposal{Title: "Upgrade test", Plan: types.Plan{Name: "test2", Height: 13}}) - require.Nil(t, err) + require.NoError(t, err) newCtx := s.ctx.WithBlockHeight(13) req := abci.RequestBeginBlock{Header: newCtx.BlockHeader()} diff --git a/x/upgrade/keeper/keeper.go b/x/upgrade/keeper/keeper.go index 52b47755b..a17530d09 100644 --- a/x/upgrade/keeper/keeper.go +++ b/x/upgrade/keeper/keeper.go @@ -167,14 +167,16 @@ func (k Keeper) getModuleVersion(ctx sdk.Context, name string) (uint64, bool) { // ScheduleUpgrade schedules an upgrade based on the specified plan. // If there is another Plan already scheduled, it will cancel and overwrite it. -// ScheduleUpgrade will also write the upgraded client to the upgraded client path -// if an upgraded client is specified in the plan +// ScheduleUpgrade will also write the upgraded IBC ClientState to the upgraded client +// path if it is specified in the plan. func (k Keeper) ScheduleUpgrade(ctx sdk.Context, plan types.Plan) error { if err := plan.ValidateBasic(); err != nil { return err } - if plan.Height <= ctx.BlockHeight() { + // NOTE: allow for the possibility of chains to schedule upgrades in begin block of the same block + // as a strategy for emergency hard fork recoveries + if plan.Height < ctx.BlockHeight() { return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "upgrade cannot be scheduled in the past") } @@ -364,7 +366,7 @@ func (k Keeper) DumpUpgradeInfoToDisk(height int64, p types.Plan) error { return err } - return os.WriteFile(upgradeInfoFilePath, info, 0600) + return os.WriteFile(upgradeInfoFilePath, info, 0o600) } // GetUpgradeInfoPath returns the upgrade info file path