From 87ba5a6a1368726cf0d19e2ffe1c835c4c5c5753 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Mon, 18 Sep 2023 15:33:57 +0000
Subject: [PATCH] feat(gov): handle panics when executing x/gov proposals
 (backport #17780) (#17790)

Co-authored-by: Robert Zaremba <robert@zaremba.ch>
Co-authored-by: Julien Robert <julien@rbrt.fr>
---
 CHANGELOG.md                |  1 +
 x/gov/abci.go               | 16 ++++++++++++++--
 x/gov/abci_internal_test.go | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 x/gov/abci_internal_test.go

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c62b7ddeaae..52eb686e59ae 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvments
 
+* (x/gov) [#17780](https://github.com/cosmos/cosmos-sdk/pull/17780) Recover panics and turn them into errors when executing x/gov proposals.
 * (types/module) [#17554](https://github.com/cosmos/cosmos-sdk/pull/17554) Introduce `HasABCIGenesis` which is implemented by a module only when a validatorset update needs to be returned.
 * (baseapp) [#17667](https://github.com/cosmos/cosmos-sdk/pull/17667) Close databases opened by SDK in `baseApp.Close()`.
 
diff --git a/x/gov/abci.go b/x/gov/abci.go
index 0be4c0f5d384..e6ff70c00cc5 100644
--- a/x/gov/abci.go
+++ b/x/gov/abci.go
@@ -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"
@@ -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
 				}
@@ -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
+}
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)
+}