Skip to content

Commit

Permalink
feat(gov): handle panics when executing x/gov proposals (#17780)
Browse files Browse the repository at this point in the history
(cherry picked from commit a0b39a1)

# Conflicts:
#	CHANGELOG.md
  • Loading branch information
robert-zaremba authored and mergify[bot] committed Sep 18, 2023
1 parent 2196eda commit 00a55ad
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (baseapp & types) [#17712](https://github.com/cosmos/cosmos-sdk/pull/17712) Introduce `PreBlock`, which runs before begin blocker other modules, and allows to modify consensus parameters, and the changes are visible to the following state machine logics. Additionally it can be used for vote extensions.
* (x/bank) [#14224](https://github.com/cosmos/cosmos-sdk/pull/14224) Allow injection of restrictions on transfers using `AppendSendRestriction` or `PrependSendRestriction`.
* (genutil) [#17571](https://github.com/cosmos/cosmos-sdk/pull/17571) Allow creation of `AppGenesis` without a file lookup.
<<<<<<< HEAD
=======
* (server) [#17094](https://github.com/cosmos/cosmos-sdk/pull/17094) Add duration `shutdown-grace` for resource clean up (closing database handles) before exit.
* (x/gov) [#17780](https://github.com/cosmos/cosmos-sdk/pull/17780) Recover panics and turn them into errors when executing x/gov proposals.
>>>>>>> a0b39a1d3 (feat(gov): handle panics when executing x/gov proposals (#17780))
### Improvments

Expand Down
16 changes: 14 additions & 2 deletions x/gov/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"cosmossdk.io/collections"

"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"
Expand Down Expand Up @@ -133,9 +134,8 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
// execute all messages
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
}
Expand Down Expand Up @@ -223,3 +223,15 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
}
return nil
}

// 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
}
32 changes: 32 additions & 0 deletions x/gov/abci_internal_test.go
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit 00a55ad

Please sign in to comment.