feat(gov): handle panics when executing x/gov proposals (backport cos…

…mos#17780) (cosmos#17793) Co-authored-by: Robert Zaremba <[email protected]> Co-authored-by: Julien Robert <[email protected]>
mmsqe · Oct 9, 2023 · 26ea71b · 26ea71b
1 parent c4a439e
commit 26ea71b
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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)
+}