-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
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
fix: handle RunMigrationBeginBlock error #17372
Conversation
|
||
if app.beginBlocker != nil { | ||
ctx := app.finalizeBlockState.ctx | ||
if app.migrationModuleManager != nil && app.migrationModuleManager.RunMigrationBeginBlock(ctx) { | ||
cp := ctx.ConsensusParams() | ||
// Manager skips this step if Block is non-nil since upgrade module is expected to set this params | ||
// and consensus parameters should not be overwritten. | ||
if cp.Block == nil { | ||
if cp = app.GetConsensusParams(ctx); cp.Block != nil { | ||
ctx = ctx.WithConsensusParams(cp) | ||
if app.migrationModuleManager != nil { | ||
if success, err := app.migrationModuleManager.RunMigrationBeginBlock(ctx); success { | ||
cp := ctx.ConsensusParams() | ||
// Manager skips this step if Block is non-nil since upgrade module is expected to set this params | ||
// and consensus parameters should not be overwritten. | ||
if cp.Block == nil { | ||
if cp = app.GetConsensusParams(ctx); cp.Block != nil { | ||
ctx = ctx.WithConsensusParams(cp) | ||
} | ||
} | ||
} else if err != nil { | ||
return sdk.BeginBlock{}, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change potentially affects state.
Call sequence:
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).beginBlock (baseapp/baseapp.go:681)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/baseapp.go:658)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, only changelog nits.
I'll cherry pick this in the mergify pr instead of adding a flag, otherwise it is annoying.
@@ -61,7 +61,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
* (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm". | |||
* (x/bank) [#17170](https://github.com/cosmos/cosmos-sdk/pull/17170) Avoid empty spendable error message on send coins. | |||
* (x/distribution) [#17236](https://github.com/cosmos/cosmos-sdk/pull/17236) Using "validateCommunityTax" in "Params.ValidateBasic", preventing panic when field "CommunityTax" is nil. | |||
* (migrations) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration. | |||
* (x/upgrade) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is (types)
@@ -61,7 +61,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
* (x/group) [#17146](https://github.com/cosmos/cosmos-sdk/pull/17146) Rename x/group legacy ORM package's error codespace from "orm" to "legacy_orm", preventing collisions with ORM's error codespace "orm". | |||
* (x/bank) [#17170](https://github.com/cosmos/cosmos-sdk/pull/17170) Avoid empty spendable error message on send coins. | |||
* (x/distribution) [#17236](https://github.com/cosmos/cosmos-sdk/pull/17236) Using "validateCommunityTax" in "Params.ValidateBasic", preventing panic when field "CommunityTax" is nil. | |||
* (migrations) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration. | |||
* (x/upgrade) [#16583](https://github.com/cosmos/cosmos-sdk/pull/16583) Add `MigrationModuleManager` to handle migration of upgrade module before other modules, ensuring access to the updated context with consensus parameters within the same block that executes the migration. | |||
* (x/upgrade) [#17372](https://github.com/cosmos/cosmos-sdk/pull/17372) Stop state machine when `RunMigrationBeginBlock` has error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is (baseapp)
Description
This change should have been included in 0c1f6fc
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
make lint
andmake test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change