From 4a70db8d01874eeaf555a811a95b030dbf17fc54 Mon Sep 17 00:00:00 2001 From: chandiniv1 <117723967+chandiniv1@users.noreply.github.com> Date: Fri, 26 Jul 2024 17:24:35 +0530 Subject: [PATCH] Add error for indicating Mashal/Unmarshaling failures (#6904) * Add error for indicating Mashal/Unmarshaling failures * nit * replace ErrUnmarshalFailed to existing ErrInvalidType * fix test --------- Co-authored-by: Carlos Rodriguez --- .../controller/ibc_middleware_test.go | 5 +++-- .../controller/keeper/handshake_test.go | 10 +++++----- .../27-interchain-accounts/host/ibc_module_test.go | 2 +- .../host/keeper/handshake_test.go | 5 +++-- .../apps/27-interchain-accounts/host/keeper/relay.go | 2 +- .../27-interchain-accounts/host/keeper/relay_test.go | 10 +++++----- modules/apps/27-interchain-accounts/types/codec.go | 8 +++++--- .../apps/27-interchain-accounts/types/codec_test.go | 7 ++++--- modules/apps/27-interchain-accounts/types/metadata.go | 3 ++- 9 files changed, 29 insertions(+), 23 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 420771e8f5c..afaf5c487ce 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -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" ) @@ -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() { @@ -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() { diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index f7cc1b1bd31..35bac8a80c3 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -142,7 +142,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.SetChannel(*channel) channel.Version = "invalid-metadata-bytestring" }, - icatypes.ErrUnknownDataType, + ibcerrors.ErrInvalidType, }, { "unsupported encoding format", @@ -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", @@ -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", diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index cce0a8c76ee..cba05393f9b 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -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", }, } diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go index 4fe6856cbd2..9140d948695 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go @@ -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" ) @@ -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", diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay.go b/modules/apps/27-interchain-accounts/host/keeper/relay.go index 0c18707a98a..91c03a75ee1 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay.go @@ -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) diff --git a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go index 9770e78096b..b4cbacfcf01 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/relay_test.go @@ -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", @@ -418,7 +418,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { packetData = icaPacketData.GetBytes() }, - icatypes.ErrUnknownDataType, + ibcerrors.ErrInvalidType, }, { "invalid packet type - UNSPECIFIED", @@ -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", @@ -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, }, } diff --git a/modules/apps/27-interchain-accounts/types/codec.go b/modules/apps/27-interchain-accounts/types/codec.go index ddb092ce030..1f63cb9d1d2 100644 --- a/modules/apps/27-interchain-accounts/types/codec.go +++ b/modules/apps/27-interchain-accounts/types/codec.go @@ -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 @@ -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) @@ -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) diff --git a/modules/apps/27-interchain-accounts/types/codec_test.go b/modules/apps/27-interchain-accounts/types/codec_test.go index ca808a64dac..388bc7a4cde 100644 --- a/modules/apps/27-interchain-accounts/types/codec_test.go +++ b/modules/apps/27-interchain-accounts/types/codec_test.go @@ -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 @@ -409,7 +410,7 @@ func (suite *TypesTestSuite) TestJSONDeserializeCosmosTx() { []proto.Message{ &mockSdkMsg{}, }, - types.ErrUnknownDataType, + ibcerrors.ErrInvalidType, }, { "failure: multiple unregistered msg types", @@ -419,13 +420,13 @@ func (suite *TypesTestSuite) TestJSONDeserializeCosmosTx() { &mockSdkMsg{}, &mockSdkMsg{}, }, - types.ErrUnknownDataType, + ibcerrors.ErrInvalidType, }, { "failure: empty bytes", []byte{}, nil, - types.ErrUnknownDataType, + ibcerrors.ErrInvalidType, }, } diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 7f639d537f5..13cc5c30fc2 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -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 ( @@ -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 }