diff --git a/.github/workflows/dependencies-review.yml b/.github/workflows/dependencies-review.yml index 52b58c3a486f..7503a9718030 100644 --- a/.github/workflows/dependencies-review.yml +++ b/.github/workflows/dependencies-review.yml @@ -10,7 +10,7 @@ jobs: steps: - uses: actions/setup-go@v3 with: - go-version: "1.19" + go-version: "1.21" check-latest: true - name: "Checkout Repository" uses: actions/checkout@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4874bb816b71..38df5e2fea63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Improvements + +* (x/gov) [#17780](https://github.com/cosmos/cosmos-sdk/pull/17780) Recover panics and turn them into errors when executing x/gov proposals. + ### Bug Fixes * (config) [#17649](https://github.com/cosmos/cosmos-sdk/pull/17649) Fix `mempool.max-txs` configuration is invalid in `app.config`. diff --git a/x/gov/abci.go b/x/gov/abci.go index fd0df2981779..8fc06069f886 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -4,6 +4,7 @@ import ( "fmt" "time" + "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/telemetry" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov/keeper" @@ -78,9 +79,8 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) { if err == nil { for idx, msg = range messages { handler := keeper.Router().Handler(msg) - var res *sdk.Result - res, err = handler(cacheCtx, msg) + res, err = safeExecuteHandler(cacheCtx, msg, handler) if err != nil { break } @@ -136,3 +136,15 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) { return false }) } + +// executes handle(msg) and recovers from panic. +func safeExecuteHandler(ctx sdk.Context, msg sdk.Msg, handler baseapp.MsgServiceHandler, +) (res *sdk.Result, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("handling x/gov proposal msg [%s] PANICKED: %v", msg, r) + } + }() + res, err = handler(ctx, msg) + return +} diff --git a/x/gov/abci_internal_test.go b/x/gov/abci_internal_test.go new file mode 100644 index 000000000000..1421a81b5cb6 --- /dev/null +++ b/x/gov/abci_internal_test.go @@ -0,0 +1,32 @@ +package gov + +import ( + "testing" + + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func failingHandler(_ sdk.Context, _ sdk.Msg) (*sdk.Result, error) { + panic("test-fail") +} + +func okHandler(_ sdk.Context, _ sdk.Msg) (*sdk.Result, error) { + return new(sdk.Result), nil +} + +func TestSafeExecuteHandler(t *testing.T) { + t.Parallel() + + require := require.New(t) + var ctx sdk.Context + + r, err := safeExecuteHandler(ctx, nil, failingHandler) + require.ErrorContains(err, "test-fail") + require.Nil(r) + + r, err = safeExecuteHandler(ctx, nil, okHandler) + require.Nil(err) + require.NotNil(r) +}