From 585b68ec70b7ab48ac323c98d0490a2177f909b2 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Mon, 15 Apr 2019 12:25:04 -0400 Subject: [PATCH] Merge PR #4100: Return correct error on invalid messages --- ...ownRequest-in-message-handlers-for-unknown | 2 ++ x/bank/handler.go | 6 +++++- x/bank/handler_test.go | 18 ++++++++++++++++++ x/crisis/handler.go | 7 +++++-- x/crisis/handler_test.go | 10 ++++++++++ x/distribution/handler.go | 8 +++++++- x/gov/handler.go | 5 ++++- x/gov/handler_test.go | 19 +++++++++++++++++++ x/ibc/handler.go | 6 +++++- x/ibc/handler_test.go | 19 +++++++++++++++++++ x/slashing/handler.go | 6 +++++- x/slashing/handler_test.go | 10 ++++++++++ x/staking/handler.go | 9 ++++++++- x/staking/handler_test.go | 10 ++++++++++ 14 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 .pending/improvements/sdk/3978-Return-ErrUnknownRequest-in-message-handlers-for-unknown create mode 100644 x/bank/handler_test.go create mode 100644 x/gov/handler_test.go create mode 100644 x/ibc/handler_test.go diff --git a/.pending/improvements/sdk/3978-Return-ErrUnknownRequest-in-message-handlers-for-unknown b/.pending/improvements/sdk/3978-Return-ErrUnknownRequest-in-message-handlers-for-unknown new file mode 100644 index 0000000000..413689c05e --- /dev/null +++ b/.pending/improvements/sdk/3978-Return-ErrUnknownRequest-in-message-handlers-for-unknown @@ -0,0 +1,2 @@ +#3978 Return ErrUnknownRequest in message handlers for unknown +or invalid routed messages. diff --git a/x/bank/handler.go b/x/bank/handler.go index ca8654feb6..3afff836ae 100644 --- a/x/bank/handler.go +++ b/x/bank/handler.go @@ -1,6 +1,8 @@ package bank import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/bank/tags" ) @@ -11,10 +13,12 @@ func NewHandler(k Keeper) sdk.Handler { switch msg := msg.(type) { case MsgSend: return handleMsgSend(ctx, k, msg) + case MsgMultiSend: return handleMsgMultiSend(ctx, k, msg) + default: - errMsg := "Unrecognized bank Msg type: %s" + msg.Type() + errMsg := fmt.Sprintf("unrecognized bank message type: %T", msg) return sdk.ErrUnknownRequest(errMsg).Result() } } diff --git a/x/bank/handler_test.go b/x/bank/handler_test.go new file mode 100644 index 0000000000..69021d30b4 --- /dev/null +++ b/x/bank/handler_test.go @@ -0,0 +1,18 @@ +package bank + +import ( + "strings" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/stretchr/testify/require" +) + +func TestInvalidMsg(t *testing.T) { + h := NewHandler(nil) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized bank message type")) +} diff --git a/x/crisis/handler.go b/x/crisis/handler.go index 6373949500..da30d976aa 100644 --- a/x/crisis/handler.go +++ b/x/crisis/handler.go @@ -1,6 +1,8 @@ package crisis import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/crisis/tags" ) @@ -13,12 +15,13 @@ const ( func NewHandler(k Keeper) sdk.Handler { return func(ctx sdk.Context, msg sdk.Msg) sdk.Result { - switch msg := msg.(type) { case MsgVerifyInvariant: return handleMsgVerifyInvariant(ctx, msg, k) + default: - return sdk.ErrTxDecode("invalid message parse in crisis module").Result() + errMsg := fmt.Sprintf("unrecognized crisis message type: %T", msg) + return sdk.ErrUnknownRequest(errMsg).Result() } } } diff --git a/x/crisis/handler_test.go b/x/crisis/handler_test.go index 84c3d5c455..acc920e4a1 100644 --- a/x/crisis/handler_test.go +++ b/x/crisis/handler_test.go @@ -3,6 +3,7 @@ package crisis import ( "errors" "fmt" + "strings" "testing" "github.com/stretchr/testify/require" @@ -99,3 +100,12 @@ func TestHandleMsgVerifyInvariantWithInvariantNotBroken(t *testing.T) { res := handleMsgVerifyInvariant(ctx, msg, crisisKeeper) require.True(t, res.IsOK()) } + +func TestInvalidMsg(t *testing.T) { + k := Keeper{} + h := NewHandler(k) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized crisis message type")) +} diff --git a/x/distribution/handler.go b/x/distribution/handler.go index 3cb1a40c8d..64f824b9f8 100644 --- a/x/distribution/handler.go +++ b/x/distribution/handler.go @@ -1,6 +1,8 @@ package distribution import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/distribution/keeper" "github.com/cosmos/cosmos-sdk/x/distribution/tags" @@ -13,12 +15,16 @@ func NewHandler(k keeper.Keeper) sdk.Handler { switch msg := msg.(type) { case types.MsgSetWithdrawAddress: return handleMsgModifyWithdrawAddress(ctx, msg, k) + case types.MsgWithdrawDelegatorReward: return handleMsgWithdrawDelegatorReward(ctx, msg, k) + case types.MsgWithdrawValidatorCommission: return handleMsgWithdrawValidatorCommission(ctx, msg, k) + default: - return sdk.ErrTxDecode("invalid message parse in distribution module").Result() + errMsg := fmt.Sprintf("unrecognized distribution message type: %T", msg) + return sdk.ErrUnknownRequest(errMsg).Result() } } } diff --git a/x/gov/handler.go b/x/gov/handler.go index 8a60cb9ed8..a4a6494c33 100644 --- a/x/gov/handler.go +++ b/x/gov/handler.go @@ -13,12 +13,15 @@ func NewHandler(keeper Keeper) sdk.Handler { switch msg := msg.(type) { case MsgDeposit: return handleMsgDeposit(ctx, keeper, msg) + case MsgSubmitProposal: return handleMsgSubmitProposal(ctx, keeper, msg) + case MsgVote: return handleMsgVote(ctx, keeper, msg) + default: - errMsg := fmt.Sprintf("Unrecognized gov msg type: %T", msg) + errMsg := fmt.Sprintf("unrecognized gov message type: %T", msg) return sdk.ErrUnknownRequest(errMsg).Result() } } diff --git a/x/gov/handler_test.go b/x/gov/handler_test.go new file mode 100644 index 0000000000..8b8ad17cb5 --- /dev/null +++ b/x/gov/handler_test.go @@ -0,0 +1,19 @@ +package gov + +import ( + "strings" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/stretchr/testify/require" +) + +func TestInvalidMsg(t *testing.T) { + k := Keeper{} + h := NewHandler(k) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized gov message type")) +} diff --git a/x/ibc/handler.go b/x/ibc/handler.go index bdb871ada5..b1b958218a 100644 --- a/x/ibc/handler.go +++ b/x/ibc/handler.go @@ -1,6 +1,8 @@ package ibc import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -9,10 +11,12 @@ func NewHandler(ibcm Mapper, ck BankKeeper) sdk.Handler { switch msg := msg.(type) { case MsgIBCTransfer: return handleIBCTransferMsg(ctx, ibcm, ck, msg) + case MsgIBCReceive: return handleIBCReceiveMsg(ctx, ibcm, ck, msg) + default: - errMsg := "Unrecognized IBC Msg type: " + msg.Type() + errMsg := fmt.Sprintf("unrecognized IBC message type: %T", msg) return sdk.ErrUnknownRequest(errMsg).Result() } } diff --git a/x/ibc/handler_test.go b/x/ibc/handler_test.go new file mode 100644 index 0000000000..fca5634747 --- /dev/null +++ b/x/ibc/handler_test.go @@ -0,0 +1,19 @@ +package ibc + +import ( + "strings" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/stretchr/testify/require" +) + +func TestInvalidMsg(t *testing.T) { + m := Mapper{} + h := NewHandler(m, nil) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized IBC message type")) +} diff --git a/x/slashing/handler.go b/x/slashing/handler.go index 7e5594c44c..d3562fe05d 100644 --- a/x/slashing/handler.go +++ b/x/slashing/handler.go @@ -1,6 +1,8 @@ package slashing import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/slashing/tags" ) @@ -11,8 +13,10 @@ func NewHandler(k Keeper) sdk.Handler { switch msg := msg.(type) { case MsgUnjail: return handleMsgUnjail(ctx, msg, k) + default: - return sdk.ErrTxDecode("invalid message parse in staking module").Result() + errMsg := fmt.Sprintf("unrecognized slashing message type: %T", msg) + return sdk.ErrUnknownRequest(errMsg).Result() } } } diff --git a/x/slashing/handler_test.go b/x/slashing/handler_test.go index 18e03fba26..93a7057fb5 100644 --- a/x/slashing/handler_test.go +++ b/x/slashing/handler_test.go @@ -1,6 +1,7 @@ package slashing import ( + "strings" "testing" "time" @@ -121,3 +122,12 @@ func TestJailedValidatorDelegations(t *testing.T) { got = NewHandler(slashingKeeper)(ctx, NewMsgUnjail(valAddr)) require.True(t, got.IsOK(), "expected jailed validator to be able to unjail, got: %v", got) } + +func TestInvalidMsg(t *testing.T) { + k := Keeper{} + h := NewHandler(k) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized slashing message type")) +} diff --git a/x/staking/handler.go b/x/staking/handler.go index d51413d5cc..bfff581f05 100644 --- a/x/staking/handler.go +++ b/x/staking/handler.go @@ -1,6 +1,7 @@ package staking import ( + "fmt" "time" abci "github.com/tendermint/tendermint/abci/types" @@ -19,16 +20,22 @@ func NewHandler(k keeper.Keeper) sdk.Handler { switch msg := msg.(type) { case types.MsgCreateValidator: return handleMsgCreateValidator(ctx, msg, k) + case types.MsgEditValidator: return handleMsgEditValidator(ctx, msg, k) + case types.MsgDelegate: return handleMsgDelegate(ctx, msg, k) + case types.MsgBeginRedelegate: return handleMsgBeginRedelegate(ctx, msg, k) + case types.MsgUndelegate: return handleMsgUndelegate(ctx, msg, k) + default: - return sdk.ErrTxDecode("invalid message parse in staking module").Result() + errMsg := fmt.Sprintf("unrecognized staking message type: %T", msg) + return sdk.ErrUnknownRequest(errMsg).Result() } } } diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 19865aeba1..bdf0ab4699 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -1,6 +1,7 @@ package staking import ( + "strings" "testing" "time" @@ -1272,3 +1273,12 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { validator, _ = keeper.GetValidator(ctx, valA) require.Equal(t, validator.GetStatus(), sdk.Unbonding) } + +func TestInvalidMsg(t *testing.T) { + k := keep.Keeper{} + h := NewHandler(k) + + res := h(sdk.Context{}, sdk.NewTestMsg()) + require.False(t, res.IsOK()) + require.True(t, strings.Contains(res.Log, "unrecognized staking message type")) +}