diff --git a/CHANGELOG.md b/CHANGELOG.md index 25c3f02177ca..b52ced580e20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -186,6 +186,11 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf ### Bug Fixes +<<<<<<< HEAD +======= +* (x/upgrade) [#13936](https://github.com/cosmos/cosmos-sdk/pull/13936) Make downgrade verification work again +* (x/group) [#13742](https://github.com/cosmos/cosmos-sdk/pull/13742) Fix `validate-genesis` when group policy accounts exist. +>>>>>>> b585d17e7 (fix: make downgrade verification work again (#13936)) * (x/auth) [#13838](https://github.com/cosmos/cosmos-sdk/pull/13838) Fix calling `String()` and `MarshalYAML` panics when pubkey is set on a `BaseAccount`. * (rosetta) [#13583](https://github.com/cosmos/cosmos-sdk/pull/13583) Misc fixes for cosmos-rosetta. * (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) Fix evidence query API to decode the hash properly. diff --git a/x/upgrade/abci.go b/x/upgrade/abci.go index 91795b9f709e..5f05af61dd64 100644 --- a/x/upgrade/abci.go +++ b/x/upgrade/abci.go @@ -25,6 +25,28 @@ func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) { plan, found := k.GetUpgradePlan(ctx) + if !k.DowngradeVerified() { + k.SetDowngradeVerified(true) + // This check will make sure that we are using a valid binary. + // It'll panic in these cases if there is no upgrade handler registered for the last applied upgrade. + // 1. If there is no scheduled upgrade. + // 2. If the plan is not ready. + // 3. If the plan is ready and skip upgrade height is set for current height. + if !found || !plan.ShouldExecute(ctx) || (plan.ShouldExecute(ctx) && k.IsSkipHeight(ctx.BlockHeight())) { + lastAppliedPlan, _ := k.GetLastCompletedUpgrade(ctx) + if lastAppliedPlan != "" && !k.HasHandler(lastAppliedPlan) { + var appVersion uint64 + + cp := ctx.ConsensusParams() + if cp != nil && cp.Version != nil { + appVersion = cp.Version.App + } + + panic(fmt.Sprintf("Wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan)) + } + } + } + if !found { return } @@ -70,28 +92,6 @@ func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) { ctx.Logger().Error(downgradeMsg) panic(downgradeMsg) } - - if !k.DowngradeVerified() { - k.SetDowngradeVerified(true) - lastAppliedPlan, _ := k.GetLastCompletedUpgrade(ctx) - // This check will make sure that we are using a valid binary. - // It'll panic in these cases if there is no upgrade handler registered for the last applied upgrade. - // 1. If there is no scheduled upgrade. - // 2. If the plan is not ready. - // 3. If the plan is ready and skip upgrade height is set for current height. - if !found || !plan.ShouldExecute(ctx) || (plan.ShouldExecute(ctx) && k.IsSkipHeight(ctx.BlockHeight())) { - if lastAppliedPlan != "" && !k.HasHandler(lastAppliedPlan) { - var appVersion uint64 - - cp := ctx.ConsensusParams() - if cp != nil && cp.Version != nil { - appVersion = cp.Version.App - } - - panic(fmt.Sprintf("Wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan)) - } - } - } } // BuildUpgradeNeededMsg prints the message that notifies that an upgrade is needed. diff --git a/x/upgrade/abci_test.go b/x/upgrade/abci_test.go index 7f6490f1277f..f08a1ad181f7 100644 --- a/x/upgrade/abci_test.go +++ b/x/upgrade/abci_test.go @@ -483,3 +483,91 @@ func TestBinaryVersion(t *testing.T) { } } } + +func TestDowngradeVerification(t *testing.T) { + // could not use setupTest() here, because we have to use the same key + // for the two keepers. + encCfg := moduletestutil.MakeTestEncodingConfig(upgrade.AppModuleBasic{}) + key := sdk.NewKVStoreKey(types.StoreKey) + testCtx := testutil.DefaultContextWithDB(s.T(), key, sdk.NewTransientStoreKey("transient_test")) + ctx := testCtx.Ctx.WithBlockHeader(tmproto.Header{Time: time.Now(), Height: 10}) + + skip := map[int64]bool{} + tempDir := t.TempDir() + k := keeper.NewKeeper(skip, key, encCfg.Codec, tempDir, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + m := upgrade.NewAppModule(k) + handler := upgrade.NewSoftwareUpgradeProposalHandler(k) + + // submit a plan. + planName := "downgrade" + err := handler(ctx, &types.SoftwareUpgradeProposal{Title: "test", Plan: types.Plan{Name: planName, Height: ctx.BlockHeight() + 1}}) + require.NoError(t, err) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + // set the handler. + k.SetUpgradeHandler(planName, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }) + + // successful upgrade. + req := abci.RequestBeginBlock{Header: ctx.BlockHeader()} + require.NotPanics(t, func() { + m.BeginBlock(ctx, req) + }) + ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1) + + testCases := map[string]struct{ + preRun func(keeper.Keeper, sdk.Context, string) + expectPanic bool + }{ + "valid binary": { + preRun: func(k keeper.Keeper, ctx sdk.Context, name string) { + k.SetUpgradeHandler(planName, func(ctx sdk.Context, plan types.Plan, vm module.VersionMap) (module.VersionMap, error) { + return vm, nil + }) + }, + }, + "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.BlockHeight() + 1}}) + require.NoError(t, err, name) + }, + expectPanic: true, + }, + "downgrade without any active plan": { + expectPanic: true, + }, + } + + for name, tc := range testCases { + ctx, _ := ctx.CacheContext() + + // downgrade. now keeper does not have the handler. + k := keeper.NewKeeper(skip, key, encCfg.Codec, tempDir, nil, authtypes.NewModuleAddress(govtypes.ModuleName).String()) + m := upgrade.NewAppModule(k) + + // assertions + lastAppliedPlan, _ := k.GetLastCompletedUpgrade(ctx) + require.Equal(t, planName, lastAppliedPlan) + require.False(t, k.HasHandler(planName)) + require.False(t, k.DowngradeVerified()) + _, found := k.GetUpgradePlan(ctx) + require.False(t, found) + + if tc.preRun != nil { + tc.preRun(k, ctx, name) + } + + req := abci.RequestBeginBlock{Header: ctx.BlockHeader()} + if tc.expectPanic { + require.Panics(t, func() { + m.BeginBlock(ctx, req) + }, name) + } else { + require.NotPanics(t, func() { + m.BeginBlock(ctx, req) + }, name) + } + } +}