Skip to content

Commit

Permalink
Add error for indicating Mashal/Unmarshaling failures (#6904)
Browse files Browse the repository at this point in the history
* Add error for indicating Mashal/Unmarshaling failures

* nit

* replace ErrUnmarshalFailed to existing ErrInvalidType

* fix test

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
  • Loading branch information
chandiniv1 and crodriguezvega authored Jul 26, 2024
1 parent 6ad9731 commit 4a70db8
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
ibcmock "github.com/cosmos/ibc-go/v9/testing/mock"
)
Expand Down Expand Up @@ -821,7 +822,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
{
"ICA OnChanUpgradeInit fails - invalid version", func() {
version = invalidVersion
}, icatypes.ErrUnknownDataType,
}, ibcerrors.ErrInvalidType,
},
{
"ICA auth module callback fails", func() {
Expand Down Expand Up @@ -949,7 +950,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
{
"ICA OnChanUpgradeAck fails - invalid version", func() {
counterpartyVersion = invalidVersion
}, icatypes.ErrUnknownDataType,
}, ibcerrors.ErrInvalidType,
},
{
"ICA auth module callback fails", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
path.EndpointA.SetChannel(*channel)
channel.Version = "invalid-metadata-bytestring"
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"unsupported encoding format",
Expand Down Expand Up @@ -586,14 +586,14 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
malleate: func() {
version = invalidVersion
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: cannot decode self version string",
malleate: func() {
path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { channel.Version = invalidVersion })
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: failed controller metadata validation, invalid encoding",
Expand Down Expand Up @@ -746,14 +746,14 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
malleate: func() {
counterpartyVersion = invalidVersion
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: cannot decode self version string",
malleate: func() {
path.EndpointA.UpdateChannel(func(channel *channeltypes.Channel) { channel.Version = invalidVersion })
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: channel not found",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
"ICA OnRecvPacket fails - cannot unmarshal packet data", func() {
packetData = []byte("invalid data")
}, false,
"cannot unmarshal ICS-27 interchain account packet data: unknown data type",
"cannot unmarshal ICS-27 interchain account packet data: invalid type",
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

Expand Down Expand Up @@ -515,14 +516,14 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
malleate: func() {
counterpartyVersion = "invalid-version"
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: cannot decode version string from channel",
malleate: func() {
path.EndpointB.UpdateChannel(func(channel *channeltypes.Channel) { channel.Version = "invalid-metadata-string" })
},
expError: icatypes.ErrUnknownDataType,
expError: ibcerrors.ErrInvalidType,
},
{
name: "failure: metadata encoding not supported",
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/host/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) ([]byt
err := data.UnmarshalJSON(packet.GetData())
if err != nil {
// UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks
return nil, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain account packet data")
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-27 interchain account packet data")
}

metadata, err := k.getAppMetadata(ctx, packet.DestinationPort, packet.DestinationChannel)
Expand Down
10 changes: 5 additions & 5 deletions modules/apps/27-interchain-accounts/host/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,14 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
params := types.NewParams(true, []string{"/" + proto.MessageName(msg)})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"cannot unmarshal interchain account packet data",
func(encoding string) {
packetData = []byte{}
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"cannot deserialize interchain account packet data messages",
Expand All @@ -418,7 +418,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {

packetData = icaPacketData.GetBytes()
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"invalid packet type - UNSPECIFIED",
Expand Down Expand Up @@ -782,7 +782,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{"*"})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"message type not allowed banktypes.MsgSend",
Expand Down Expand Up @@ -832,7 +832,7 @@ func (suite *KeeperTestSuite) TestJSONOnRecvPacket() {
params := types.NewParams(true, []string{sdk.MsgTypeURL((*banktypes.MsgSend)(nil))})
suite.chainB.GetSimApp().ICAHostKeeper.SetParams(suite.chainB.GetContext(), params)
},
icatypes.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
}

Expand Down
8 changes: 5 additions & 3 deletions modules/apps/27-interchain-accounts/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
)

// ModuleCdc references the global interchain accounts module codec. Note, the codec
Expand Down Expand Up @@ -60,7 +62,7 @@ func SerializeCosmosTx(cdc codec.Codec, msgs []proto.Message, encoding string) (
case EncodingProto3JSON:
bz, err = cdc.MarshalJSON(cosmosTx)
if err != nil {
return nil, errorsmod.Wrapf(ErrUnknownDataType, "cannot marshal CosmosTx with proto3 json")
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot marshal CosmosTx with proto3 json")
}
default:
return nil, errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", encoding)
Expand All @@ -84,11 +86,11 @@ 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(ErrUnknownDataType, "cannot unmarshal CosmosTx with protobuf: %v", err)
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal CosmosTx with protobuf: %v", err)
}
case EncodingProto3JSON:
if err := cdc.UnmarshalJSON(data, &cosmosTx); err != nil {
return nil, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal CosmosTx with proto3 json")
return nil, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal CosmosTx with proto3 json: %v", err)
}
default:
return nil, errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", encoding)
Expand Down
7 changes: 4 additions & 3 deletions modules/apps/27-interchain-accounts/types/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/cosmos/ibc-go/v9/modules/apps/27-interchain-accounts/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
)

// mockSdkMsg defines a mock struct, used for testing codec error scenarios
Expand Down Expand Up @@ -409,7 +410,7 @@ func (suite *TypesTestSuite) TestJSONDeserializeCosmosTx() {
[]proto.Message{
&mockSdkMsg{},
},
types.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"failure: multiple unregistered msg types",
Expand All @@ -419,13 +420,13 @@ func (suite *TypesTestSuite) TestJSONDeserializeCosmosTx() {
&mockSdkMsg{},
&mockSdkMsg{},
},
types.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
{
"failure: empty bytes",
[]byte{},
nil,
types.ErrUnknownDataType,
ibcerrors.ErrInvalidType,
},
}

Expand Down
3 changes: 2 additions & 1 deletion modules/apps/27-interchain-accounts/types/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

connectiontypes "github.com/cosmos/ibc-go/v9/modules/core/03-connection/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
)

const (
Expand Down Expand Up @@ -58,7 +59,7 @@ func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) s
func MetadataFromVersion(versionString string) (Metadata, error) {
var metadata Metadata
if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil {
return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata")
return Metadata{}, errorsmod.Wrapf(ibcerrors.ErrInvalidType, "cannot unmarshal ICS-27 interchain accounts metadata")
}
return metadata, nil
}
Expand Down

0 comments on commit 4a70db8

Please sign in to comment.