Skip to content

Commit

Permalink
fix: move downgrade verification after store migration (#12906)
Browse files Browse the repository at this point in the history
* move downgrade verification after store migration

otherwise, it panic because it expect the key is migrated
Closes: #12904

* changelog
  • Loading branch information
yihuang committed Aug 11, 2022
1 parent daca54d commit 6473e73
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/auth/tx) [#12474](https://github.com/cosmos/cosmos-sdk/pull/12474) Remove condition in GetTxsEvent that disallowed multiple equal signs, which would break event queries with base64 strings (i.e. query by signature).
* (store/rootmulti) [#12487](https://github.com/cosmos/cosmos-sdk/pull/12487) Fix non-deterministic map iteration.
* (x/group) [#12888](https://github.com/cosmos/cosmos-sdk/pull/12888) Fix event propagation to the current context of `x/group` message execution `[]sdk.Result`.
* (x/upgrade) [#12906](https://github.com/cosmos/cosmos-sdk/pull/12906) Fix upgrade failure by moving downgrade verification logic after store migration.

### Deprecated

Expand Down
44 changes: 22 additions & 22 deletions x/upgrade/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,6 @@ func BeginBlocker(k keeper.Keeper, ctx sdk.Context, _ abci.RequestBeginBlock) {

plan, found := k.GetUpgradePlan(ctx)

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.AppVersion
}

panic(fmt.Sprintf("Wrong app version %d, upgrade handler is missing for %s upgrade plan", appVersion, lastAppliedPlan))
}
}
}

if !found {
return
}
Expand Down Expand Up @@ -92,6 +70,28 @@ 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.AppVersion
}

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.
Expand Down

0 comments on commit 6473e73

Please sign in to comment.