fix: correctly propagate msg errors in gov (cosmos#13918)

* fix: correctly propagate msg errors in gov * chore: update changelog * fix: correctly check proposal status in tests * chore: delete unused var
provenance-io · Nov 18, 2022 · 5581f7f · 5581f7f
1 parent eb4afa7
commit 5581f7f
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -195,6 +195,7 @@ extension interfaces. `module.Manager.Modules` is now of type `map[string]interf
 * (snapshot) [#13400](https://github.com/cosmos/cosmos-sdk/pull/13400) Fix snapshot checksum issue in golang 1.19.
 * (server) [#13778](https://github.com/cosmos/cosmos-sdk/pull/13778) Set Cosmos SDK default endpoints to localhost to avoid unknown exposure of endpoints.
 * (x/auth) [#13877](https://github.com/cosmos/cosmos-sdk/pull/13877) Handle missing account numbers during `InitGenesis`.
+* (x/gov) [#13918](https://github.com/cosmos/cosmos-sdk/pull/13918) Fix propagation of message errors when executing a proposal.
 
 ### Deprecated
 

diff --git a/x/gov/abci.go b/x/gov/abci.go
@@ -72,7 +72,9 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) {
 			if err == nil {
 				for idx, msg = range messages {
 					handler := keeper.Router().Handler(msg)
-					res, err := handler(cacheCtx, msg)
+
+					var res *sdk.Result
+					res, err = handler(cacheCtx, msg)
 					if err != nil {
 						break
 					}

diff --git a/x/gov/abci_test.go b/x/gov/abci_test.go
@@ -11,6 +11,8 @@ import (
 
 	simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"github.com/cosmos/cosmos-sdk/x/gov"
 	"github.com/cosmos/cosmos-sdk/x/gov/keeper"
 	"github.com/cosmos/cosmos-sdk/x/gov/types"
@@ -347,10 +349,8 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) {
 	createValidators(t, stakingMsgSvr, ctx, []sdk.ValAddress{valAddr}, []int64{10})
 	staking.EndBlocker(ctx, suite.StakingKeeper)
 
-	// Create a proposal where the handler will pass for the test proposal
-	// because the value of contextKeyBadProposal is true.
-	ctx = ctx.WithValue(contextKeyBadProposal, true)
-	proposal, err := suite.GovKeeper.SubmitProposal(ctx, []sdk.Msg{mkTestLegacyContent(t)}, "")
+	msg := banktypes.NewMsgSend(authtypes.NewModuleAddress(types.ModuleName), addrs[0], sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100000))))
+	proposal, err := suite.GovKeeper.SubmitProposal(ctx, []sdk.Msg{msg}, "")
 	require.NoError(t, err)
 
 	proposalCoins := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 10)))
@@ -368,12 +368,12 @@ func TestEndBlockerProposalHandlerFailed(t *testing.T) {
 	newHeader.Time = ctx.BlockHeader().Time.Add(*suite.GovKeeper.GetParams(ctx).MaxDepositPeriod).Add(*suite.GovKeeper.GetParams(ctx).VotingPeriod)
 	ctx = ctx.WithBlockHeader(newHeader)
 
-	// Set the contextKeyBadProposal value to false so that the handler will fail
-	// during the processing of the proposal in the EndBlocker.
-	ctx = ctx.WithValue(contextKeyBadProposal, false)
-
 	// validate that the proposal fails/has been rejected
 	gov.EndBlocker(ctx, suite.GovKeeper)
+
+	proposal, ok := suite.GovKeeper.GetProposal(ctx, proposal.Id)
+	require.True(t, ok)
+	require.Equal(t, v1.StatusFailed, proposal.Status)
 }
 
 func createValidators(t *testing.T, stakingMsgSvr stakingtypes.MsgServer, ctx sdk.Context, addrs []sdk.ValAddress, powerAmt []int64) {

diff --git a/x/gov/common_test.go b/x/gov/common_test.go
@@ -91,8 +91,6 @@ func SortByteArrays(src [][]byte) [][]byte {
 	return sorted
 }
 
-const contextKeyBadProposal = "contextKeyBadProposal"
-
 var pubkeys = []cryptotypes.PubKey{
 	ed25519.GenPrivKey().PubKey(),
 	ed25519.GenPrivKey().PubKey(),