Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change host relay tests to check error returned #4161

Merged
merged 8 commits into from
Oct 5, 2023
99 changes: 68 additions & 31 deletions modules/apps/27-interchain-accounts/host/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
ibcerrors "github.com/cosmos/ibc-go/v7/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v7/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",
DimitrisJim marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -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,
ibcerrors.ErrUnauthorized,
},
}

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
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
Loading