From bbcc29fcb0e0f5ce1ee54c94e762f751e030ffa7 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 09:55:42 +0200 Subject: [PATCH 1/7] feat(gov): handle panics when executing x/gov proposals --- x/gov/abci.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index f376afd47587..1ef42ccb96c3 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -5,7 +5,9 @@ import ( "time" "cosmossdk.io/collections" + "google.golang.org/protobuf/proto" + "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 +135,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 +224,16 @@ 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 poposal msg [%s] PANICED: %v", msg, r) + } + }() + res, err = handler(ctx, msg) + return +} From 8f6110d8836603381346da2473dafb46a0476fad Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 10:21:56 +0200 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 761f8b1ae538..f6b43f4ef44f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) Introduce a new message type, `MsgBurn `, to burn coins. * (genutil) [#17571](https://github.com/cosmos/cosmos-sdk/pull/17571) Allow creation of `AppGenesis` without a file lookup. * (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) Handle panics when executing x/gov proposals. ### Improvements From e08fc65f8f5889edd70ecffa5f020d6b867ad23f Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 14:38:39 +0200 Subject: [PATCH 3/7] TestSafeExecuteHandler --- x/gov/abci_internal_test.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 x/gov/abci_internal_test.go diff --git a/x/gov/abci_internal_test.go b/x/gov/abci_internal_test.go new file mode 100644 index 000000000000..0a00c5876201 --- /dev/null +++ b/x/gov/abci_internal_test.go @@ -0,0 +1,32 @@ +package gov + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/stretchr/testify/require" +) + +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) +} From 1c1555ef80cf89d79e62390ab4ae0e86ce0ff198 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 14:39:26 +0200 Subject: [PATCH 4/7] typo --- x/gov/abci.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 1ef42ccb96c3..43338ff6bf4a 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -5,7 +5,6 @@ import ( "time" "cosmossdk.io/collections" - "google.golang.org/protobuf/proto" "github.com/cosmos/cosmos-sdk/baseapp" "github.com/cosmos/cosmos-sdk/telemetry" @@ -231,7 +230,7 @@ func safeExecuteHandler(ctx sdk.Context, msg sdk.Msg, handler baseapp.MsgService defer func() { if r := recover(); r != nil { - err = fmt.Errorf("handling x/gov poposal msg [%s] PANICED: %v", msg, r) + err = fmt.Errorf("handling x/gov proposal msg [%s] PANICED: %v", msg, r) } }() res, err = handler(ctx, msg) From 08ea4d06133ef325a967cb09db27f9c91ae9f11f Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 14:41:29 +0200 Subject: [PATCH 5/7] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6b43f4ef44f..c109c35463ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [#17569](https://github.com/cosmos/cosmos-sdk/pull/17569) Introduce a new message type, `MsgBurn `, to burn coins. * (genutil) [#17571](https://github.com/cosmos/cosmos-sdk/pull/17571) Allow creation of `AppGenesis` without a file lookup. * (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) Handle panics when executing x/gov proposals. +* (x/gov) [#17780](https://github.com/cosmos/cosmos-sdk/pull/17780) Recover panics and turn them into errors when executing x/gov proposals. ### Improvements From 43699351b282c635c5f305125664e24bdb894b04 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 15:51:58 +0200 Subject: [PATCH 6/7] lint --- x/gov/abci_internal_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gov/abci_internal_test.go b/x/gov/abci_internal_test.go index 0a00c5876201..1421a81b5cb6 100644 --- a/x/gov/abci_internal_test.go +++ b/x/gov/abci_internal_test.go @@ -3,9 +3,9 @@ package gov import ( "testing" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" ) func failingHandler(_ sdk.Context, _ sdk.Msg) (*sdk.Result, error) { From 7c074ba93b220a24419186ad558e9e75e911c1be Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Mon, 18 Sep 2023 16:32:04 +0200 Subject: [PATCH 7/7] typo --- x/gov/abci.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/gov/abci.go b/x/gov/abci.go index 43338ff6bf4a..88ed1dc42449 100644 --- a/x/gov/abci.go +++ b/x/gov/abci.go @@ -227,10 +227,9 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error { // 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] PANICED: %v", msg, r) + err = fmt.Errorf("handling x/gov proposal msg [%s] PANICKED: %v", msg, r) } }() res, err = handler(ctx, msg)