Skip to content

Commit

Permalink
Change host relay tests to check error returned (cosmos#4161)
Browse files Browse the repository at this point in the history 
* Add failure case for msg failing on ValidateBasic.

* Move testing for host relay to check for error returns.

* Use similar error checking as other tests.

* Use NoError, wrap err in error message.

* fix: relay test expected result

---------

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
  • Loading branch information
@DimitrisJim @colin-axner @damiannolan
3 people committed Oct 5, 2023
1 parent 2fb6171 commit a0ac240
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 33 deletions.
101 changes: 69 additions & 32 deletions modules/apps/27-interchain-accounts/host/keeper/relay_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v8/testing"
)

Expand All @@ -33,7 +34,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
testCases := []struct {
msg string
malleate func(encoding string)
expPass bool
expErr error
}{
{
"interchain account successfully executes an arbitrary message type using the * (allow all message types) param",
Expand Down Expand Up @@ -77,7 +78,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{"*"})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes banktypes.MsgSend",
Expand All @@ -104,7 +105,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes stakingtypes.MsgDelegate",
Expand Down Expand Up @@ -132,7 +133,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes stakingtypes.MsgDelegate and stakingtypes.MsgUndelegate sequentially",
Expand Down Expand Up @@ -166,7 +167,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msgDelegate), sdk.MsgTypeURL(msgUndelegate)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes govtypes.MsgSubmitProposal",
Expand Down Expand Up @@ -201,7 +202,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes govtypes.MsgVote",
Expand Down Expand Up @@ -245,7 +246,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes disttypes.MsgFundCommunityPool",
Expand All @@ -271,7 +272,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes disttypes.MsgSetWithdrawAddress",
Expand All @@ -297,7 +298,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes transfertypes.MsgTransfer",
Expand Down Expand Up @@ -332,7 +333,41 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"Msg fails its ValidateBasic: MsgTransfer has an empty receiver",
func(encoding string) {
transferPath := ibctesting.NewTransferPath(suite.chainB, suite.chainC)
suite.coordinator.Setup(transferPath)

interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID)
suite.Require().True(found)

msg := &transfertypes.MsgTransfer{
SourcePort: transferPath.EndpointA.ChannelConfig.PortID,
SourceChannel: transferPath.EndpointA.ChannelID,
Token: sdk.NewCoin(sdk.DefaultBondDenom, sdkmath.NewInt(100)),
Sender: interchainAccountAddr,
Receiver: "",
TimeoutHeight: suite.chainB.GetTimeoutHeight(),
TimeoutTimestamp: uint64(0),
}

data, err := icatypes.SerializeCosmosTx(suite.chainA.GetSimApp().AppCodec(), []proto.Message{msg}, encoding)
suite.Require().NoError(err)

icaPacketData := icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: data,
}

packetData = icaPacketData.GetBytes()

params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
ibcerrors.ErrInvalidAddress,
},
{
"unregistered sdk.Msg",
Expand All @@ -352,14 +387,14 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{"/" + proto.MessageName(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
false,
icatypes.ErrUnknownDataType,
},
{
"cannot unmarshal interchain account packet data",
func(encoding string) {
packetData = []byte{}
},
false,
icatypes.ErrUnknownDataType,
},
{
"cannot deserialize interchain account packet data messages",
Expand All @@ -373,7 +408,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

packetData = icaPacketData.GetBytes()
},
false,
icatypes.ErrUnknownDataType,
},
{
"invalid packet type - UNSPECIFIED",
Expand All @@ -388,7 +423,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

packetData = icaPacketData.GetBytes()
},
false,
icatypes.ErrUnknownDataType,
},
{
"unauthorised: interchain account not found for controller port ID",
Expand All @@ -405,7 +440,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

packetData = icaPacketData.GetBytes()
},
false,
icatypes.ErrInterchainAccountNotFound,
},
{
"unauthorised: message type not allowed", // NOTE: do not update params to explicitly force the error
Expand All @@ -426,7 +461,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

packetData = icaPacketData.GetBytes()
},
false,
ibcerrors.ErrUnauthorized,
},
{
"unauthorised: signer address is not the interchain account associated with the controller portID",
Expand All @@ -450,7 +485,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
false,
ibcerrors.ErrUnauthorized,
},
}

Expand Down Expand Up @@ -498,11 +533,12 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

txResponse, err := suite.chainB.GetSimApp().ICAHostKeeper.OnRecvPacket(suite.chainB.GetContext(), packet)

if tc.expPass {
expPass := tc.expErr == nil
if expPass {
suite.Require().NoError(err)
suite.Require().NotNil(txResponse)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
suite.Require().Nil(txResponse)
}
})
Expand All @@ -520,7 +556,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
testCases := []struct {
msg string
malleate func(icaAddress string)
expPass bool
expErr error
}{
{
"interchain account successfully executes an arbitrary message type using the * (allow all message types) param",
Expand Down Expand Up @@ -564,7 +600,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{"*"})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes banktypes.MsgSend",
Expand All @@ -589,7 +625,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*banktypes.MsgSend)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes govtypes.MsgSubmitProposal",
Expand Down Expand Up @@ -618,7 +654,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*govtypes.MsgSubmitProposal)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes govtypes.MsgVote",
Expand Down Expand Up @@ -660,7 +696,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*govtypes.MsgVote)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes govtypes.MsgSubmitProposal, govtypes.MsgDeposit, and then govtypes.MsgVote sequentially",
Expand Down Expand Up @@ -701,7 +737,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*govtypes.MsgSubmitProposal)(nil)), sdk.MsgTypeURL((*govtypes.MsgDeposit)(nil)), sdk.MsgTypeURL((*govtypes.MsgVote)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"interchain account successfully executes transfertypes.MsgTransfer",
Expand Down Expand Up @@ -734,7 +770,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*transfertypes.MsgTransfer)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
true,
nil,
},
{
"unregistered sdk.Msg",
Expand All @@ -750,7 +786,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{"*"})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
false,
icatypes.ErrUnknownDataType,
},
{
"message type not allowed banktypes.MsgSend",
Expand All @@ -775,7 +811,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*transfertypes.MsgTransfer)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
false,
ibcerrors.ErrUnauthorized,
},
{
"unauthorised: signer address is not the interchain account associated with the controller portID",
Expand All @@ -784,7 +820,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
"messages": [
{
"@type": "/cosmos.bank.v1beta1.MsgSend",
"from_address": "` + ibctesting.InvalidID + `",
"from_address": "` + suite.chainB.SenderAccount.GetAddress().String() + `", // unexpected signer
"to_address": "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs",
"amount": [{ "denom": "stake", "amount": "100" }]
}
Expand All @@ -800,7 +836,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*banktypes.MsgSend)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
false,
icatypes.ErrUnknownDataType,
},
}

Expand Down Expand Up @@ -840,11 +876,12 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {

txResponse, err := suite.chainB.GetSimApp().ICAHostKeeper.OnRecvPacket(suite.chainB.GetContext(), packet)

if tc.expPass {
expPass := tc.expErr == nil
if expPass {
suite.Require().NoError(err)
suite.Require().NotNil(txResponse)
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
suite.Require().Nil(txResponse)
}
})
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/types/codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func DeserializeCosmosTx(cdc codec.Codec, data []byte, encoding string) ([]sdk.M
switch encoding {
case EncodingProtobuf:
if err := cdc.Unmarshal(data, &cosmosTx); err != nil {
return nil, errorsmod.Wrapf(err, "cannot unmarshal CosmosTx with protobuf")
return nil, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal CosmosTx with protobuf: %v", err)
}
case EncodingProto3JSON:
if err := cdc.UnmarshalJSON(data, &cosmosTx); err != nil {
Expand Down

0 comments on commit a0ac240

Please sign in to comment.