From f8e0f11f729459f3eedaca6be8cc65ac712e7ddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 7 Oct 2021 10:46:48 +0200 Subject: [PATCH 1/4] remove ICA TxBody type, use repeated Any in packet data --- docs/ibc/proto-docs.md | 20 +- .../27-interchain-accounts/keeper/keeper.go | 18 +- .../27-interchain-accounts/keeper/relay.go | 39 ++- .../27-interchain-accounts/types/types.pb.go | 282 ++++-------------- .../interchain_accounts/v1/types.proto | 6 +- 5 files changed, 79 insertions(+), 286 deletions(-) diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index 986ffd334f8..eb03b0887c6 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -17,7 +17,6 @@ - [Query](#ibc.applications.interchain_accounts.v1.Query) - [ibc/applications/interchain_accounts/v1/types.proto](#ibc/applications/interchain_accounts/v1/types.proto) - - [IBCTxBody](#ibc.applications.interchain_accounts.v1.IBCTxBody) - [InterchainAccountPacketData](#ibc.applications.interchain_accounts.v1.InterchainAccountPacketData) - [Type](#ibc.applications.interchain_accounts.v1.Type) @@ -398,31 +397,16 @@ Query defines the gRPC querier service. - - -### IBCTxBody -Body of a tx for an ics27 IBC packet - - -| Field | Type | Label | Description | -| ----- | ---- | ----- | ----------- | -| `messages` | [google.protobuf.Any](#google.protobuf.Any) | repeated | | - - - - - - ### InterchainAccountPacketData -InterchainAccountPacketData is comprised of araw transaction,type of transaction and optional memo field. +InterchainAccountPacketData is comprised of a raw transaction, type of transaction and optional memo field. | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | | `type` | [Type](#ibc.applications.interchain_accounts.v1.Type) | | | -| `data` | [bytes](#bytes) | | | +| `messages` | [google.protobuf.Any](#google.protobuf.Any) | repeated | | | `memo` | [string](#string) | | | diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go index 9730f5a9da7..98e6418faa5 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper.go @@ -57,28 +57,18 @@ func NewKeeper( } } -// SerializeCosmosTx marshals data to bytes using the provided codec -func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) ([]byte, error) { - msgAnys := make([]*codectypes.Any, len(msgs)) +// SerializeCosmosTx packs a slice of sdk.Msg into a slice of Any's. +func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (msgAnys []*codectypes.Any, err error) { + msgAnys = make([]*codectypes.Any, len(msgs)) for i, msg := range msgs { - var err error msgAnys[i], err = codectypes.NewAnyWithValue(msg) if err != nil { return nil, err } } - txBody := &types.IBCTxBody{ - Messages: msgAnys, - } - - bz, err := cdc.Marshal(txBody) - if err != nil { - return nil, err - } - - return bz, nil + return msgAnys, nil } // Logger returns the application logger, scoped to the associated module diff --git a/modules/apps/27-interchain-accounts/keeper/relay.go b/modules/apps/27-interchain-accounts/keeper/relay.go index 964bfb5cd20..5d3e861dec0 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/keeper/relay.go @@ -3,6 +3,7 @@ package keeper import ( "encoding/binary" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/tendermint/tendermint/crypto/tmhash" @@ -47,13 +48,13 @@ func (k Keeper) createOutgoingPacket( } var ( - txBytes []byte - err error + msgs []*codectypes.Any + err error ) switch data := data.(type) { case []sdk.Msg: - txBytes, err = k.SerializeCosmosTx(k.cdc, data) + msgs, err = k.SerializeCosmosTx(k.cdc, data) default: return nil, sdkerrors.Wrapf(types.ErrInvalidOutgoingData, "message type %T is not supported", data) } @@ -74,9 +75,9 @@ func (k Keeper) createOutgoingPacket( } packetData := types.InterchainAccountPacketData{ - Type: types.EXECUTE_TX, - Data: txBytes, - Memo: memo, + Type: types.EXECUTE_TX, + Messages: msgs, + Memo: memo, } // timeoutTimestamp is set to be a max number here so that we never recieve a timeout @@ -94,30 +95,26 @@ func (k Keeper) createOutgoingPacket( timeoutTimestamp, ) - return k.ComputeVirtualTxHash(packetData.Data, packet.Sequence), k.channelKeeper.SendPacket(ctx, channelCap, packet) + return k.ComputeVirtualTxHash(packetData.Messages, packet.Sequence), k.channelKeeper.SendPacket(ctx, channelCap, packet) } // DeserializeCosmosTx unmarshals and unpacks a slice of transaction bytes // into a slice of sdk.Msg's. -func (k Keeper) DeserializeCosmosTx(_ sdk.Context, txBytes []byte) ([]sdk.Msg, error) { - var txBody types.IBCTxBody +func (k Keeper) DeserializeCosmosTx(_ sdk.Context, anys []*codectypes.Any) ([]sdk.Msg, error) { + msgs := make([]sdk.Msg, len(anys)) - if err := k.cdc.Unmarshal(txBytes, &txBody); err != nil { - return nil, err - } - - anys := txBody.Messages - res := make([]sdk.Msg, len(anys)) for i, any := range anys { var msg sdk.Msg + err := k.cdc.UnpackAny(any, &msg) if err != nil { return nil, err } - res[i] = msg + + msgs[i] = msg } - return res, nil + return msgs, nil } func (k Keeper) AuthenticateTx(ctx sdk.Context, msgs []sdk.Msg, portId string) error { @@ -204,7 +201,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error switch data.Type { case types.EXECUTE_TX: - msgs, err := k.DeserializeCosmosTx(ctx, data.Data) + msgs, err := k.DeserializeCosmosTx(ctx, data.Messages) if err != nil { return err } @@ -224,12 +221,12 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac switch ack.Response.(type) { case *channeltypes.Acknowledgement_Error: if k.hook != nil { - k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Data, packet.Sequence), data.Data) + k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Messages, packet.Sequence), data.Messages) } return nil case *channeltypes.Acknowledgement_Result: if k.hook != nil { - k.hook.OnTxSucceeded(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Data, packet.Sequence), data.Data) + k.hook.OnTxSucceeded(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Messages, packet.Sequence), data.Messages) } return nil default: @@ -241,7 +238,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.InterchainAccountPacketData) error { if k.hook != nil { - k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Data, packet.Sequence), data.Data) + k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, k.ComputeVirtualTxHash(data.Messages, packet.Sequence), data.Messages) } return nil diff --git a/modules/apps/27-interchain-accounts/types/types.pb.go b/modules/apps/27-interchain-accounts/types/types.pb.go index 38aa8249c28..73cded7005f 100644 --- a/modules/apps/27-interchain-accounts/types/types.pb.go +++ b/modules/apps/27-interchain-accounts/types/types.pb.go @@ -50,63 +50,18 @@ func (Type) EnumDescriptor() ([]byte, []int) { return fileDescriptor_39bab93e18d89799, []int{0} } -// Body of a tx for an ics27 IBC packet -type IBCTxBody struct { - Messages []*types.Any `protobuf:"bytes,1,rep,name=messages,proto3" json:"messages,omitempty"` -} - -func (m *IBCTxBody) Reset() { *m = IBCTxBody{} } -func (m *IBCTxBody) String() string { return proto.CompactTextString(m) } -func (*IBCTxBody) ProtoMessage() {} -func (*IBCTxBody) Descriptor() ([]byte, []int) { - return fileDescriptor_39bab93e18d89799, []int{0} -} -func (m *IBCTxBody) XXX_Unmarshal(b []byte) error { - return m.Unmarshal(b) -} -func (m *IBCTxBody) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { - if deterministic { - return xxx_messageInfo_IBCTxBody.Marshal(b, m, deterministic) - } else { - b = b[:cap(b)] - n, err := m.MarshalToSizedBuffer(b) - if err != nil { - return nil, err - } - return b[:n], nil - } -} -func (m *IBCTxBody) XXX_Merge(src proto.Message) { - xxx_messageInfo_IBCTxBody.Merge(m, src) -} -func (m *IBCTxBody) XXX_Size() int { - return m.Size() -} -func (m *IBCTxBody) XXX_DiscardUnknown() { - xxx_messageInfo_IBCTxBody.DiscardUnknown(m) -} - -var xxx_messageInfo_IBCTxBody proto.InternalMessageInfo - -func (m *IBCTxBody) GetMessages() []*types.Any { - if m != nil { - return m.Messages - } - return nil -} - -// InterchainAccountPacketData is comprised of araw transaction,type of transaction and optional memo field. +// InterchainAccountPacketData is comprised of a raw transaction, type of transaction and optional memo field. type InterchainAccountPacketData struct { - Type Type `protobuf:"varint,1,opt,name=type,proto3,enum=ibc.applications.interchain_accounts.v1.Type" json:"type,omitempty"` - Data []byte `protobuf:"bytes,2,opt,name=data,proto3" json:"data,omitempty"` - Memo string `protobuf:"bytes,3,opt,name=memo,proto3" json:"memo,omitempty"` + Type Type `protobuf:"varint,1,opt,name=type,proto3,enum=ibc.applications.interchain_accounts.v1.Type" json:"type,omitempty"` + Messages []*types.Any `protobuf:"bytes,2,rep,name=messages,proto3" json:"messages,omitempty"` + Memo string `protobuf:"bytes,3,opt,name=memo,proto3" json:"memo,omitempty"` } func (m *InterchainAccountPacketData) Reset() { *m = InterchainAccountPacketData{} } func (m *InterchainAccountPacketData) String() string { return proto.CompactTextString(m) } func (*InterchainAccountPacketData) ProtoMessage() {} func (*InterchainAccountPacketData) Descriptor() ([]byte, []int) { - return fileDescriptor_39bab93e18d89799, []int{1} + return fileDescriptor_39bab93e18d89799, []int{0} } func (m *InterchainAccountPacketData) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -142,9 +97,9 @@ func (m *InterchainAccountPacketData) GetType() Type { return EXECUTE_TX } -func (m *InterchainAccountPacketData) GetData() []byte { +func (m *InterchainAccountPacketData) GetMessages() []*types.Any { if m != nil { - return m.Data + return m.Messages } return nil } @@ -158,7 +113,6 @@ func (m *InterchainAccountPacketData) GetMemo() string { func init() { proto.RegisterEnum("ibc.applications.interchain_accounts.v1.Type", Type_name, Type_value) - proto.RegisterType((*IBCTxBody)(nil), "ibc.applications.interchain_accounts.v1.IBCTxBody") proto.RegisterType((*InterchainAccountPacketData)(nil), "ibc.applications.interchain_accounts.v1.InterchainAccountPacketData") } @@ -167,34 +121,33 @@ func init() { } var fileDescriptor_39bab93e18d89799 = []byte{ - // 380 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x91, 0xcf, 0xaa, 0xd3, 0x40, - 0x18, 0xc5, 0x33, 0xde, 0x20, 0xde, 0x51, 0x8a, 0x84, 0x2e, 0x62, 0x0a, 0x21, 0x74, 0x63, 0x10, - 0x32, 0x63, 0xd3, 0x85, 0xab, 0x2e, 0xfa, 0x27, 0x42, 0x36, 0x52, 0x62, 0x0a, 0xd5, 0x4d, 0x98, - 0x4c, 0xc7, 0x74, 0xb0, 0xc9, 0x84, 0xce, 0xa4, 0x98, 0x37, 0x28, 0xae, 0x7c, 0x01, 0x57, 0xbe, - 0x8c, 0xcb, 0x2e, 0x5d, 0x4a, 0xfb, 0x22, 0x92, 0x04, 0x5b, 0x17, 0x2e, 0xee, 0xee, 0xf0, 0xcd, - 0xfc, 0x0e, 0xdf, 0xf9, 0x0e, 0x1c, 0xf3, 0x94, 0x62, 0x52, 0x96, 0x3b, 0x4e, 0x89, 0xe2, 0xa2, - 0x90, 0x98, 0x17, 0x8a, 0xed, 0xe9, 0x96, 0xf0, 0x22, 0x21, 0x94, 0x8a, 0xaa, 0x50, 0x12, 0x1f, - 0x46, 0x58, 0xd5, 0x25, 0x93, 0xa8, 0xdc, 0x0b, 0x25, 0x8c, 0x97, 0x3c, 0xa5, 0xe8, 0x5f, 0x08, - 0xfd, 0x07, 0x42, 0x87, 0x91, 0xf5, 0x22, 0x13, 0x22, 0xdb, 0x31, 0xdc, 0x62, 0x69, 0xf5, 0x09, - 0x93, 0xa2, 0xee, 0x3c, 0xac, 0x7e, 0x26, 0x32, 0xd1, 0x4a, 0xdc, 0xa8, 0x6e, 0x3a, 0x9c, 0xc0, - 0xfb, 0x70, 0x36, 0x8f, 0xbf, 0xcc, 0xc4, 0xa6, 0x36, 0x5e, 0xc3, 0x27, 0x39, 0x93, 0x92, 0x64, - 0x4c, 0x9a, 0xc0, 0xb9, 0x73, 0x9f, 0xfa, 0x7d, 0xd4, 0x19, 0xa2, 0xbf, 0x86, 0x68, 0x5a, 0xd4, - 0xd1, 0xf5, 0xd7, 0xf0, 0x08, 0xe0, 0x20, 0xbc, 0xae, 0x32, 0xed, 0x36, 0x59, 0x12, 0xfa, 0x99, - 0xa9, 0x05, 0x51, 0xc4, 0x98, 0x42, 0xbd, 0xc9, 0x61, 0x02, 0x07, 0xb8, 0x3d, 0xdf, 0x43, 0x0f, - 0xcc, 0x81, 0xe2, 0xba, 0x64, 0x51, 0x8b, 0x1a, 0x06, 0xd4, 0x37, 0x44, 0x11, 0xf3, 0x91, 0x03, - 0xdc, 0x67, 0x51, 0xab, 0x9b, 0x59, 0xce, 0x72, 0x61, 0xde, 0x39, 0xc0, 0xbd, 0x8f, 0x5a, 0xfd, - 0x6a, 0x02, 0xf5, 0x86, 0x32, 0x30, 0x1c, 0xc4, 0x1f, 0x96, 0x41, 0x12, 0xac, 0x83, 0xf9, 0x2a, - 0x0e, 0x92, 0x78, 0x9d, 0xac, 0xde, 0xbd, 0x5f, 0x06, 0xf3, 0xf0, 0x6d, 0x18, 0x2c, 0x9e, 0x6b, - 0x56, 0xef, 0xeb, 0x77, 0x07, 0xde, 0x5e, 0x2d, 0xfd, 0xf8, 0xc3, 0xd6, 0x66, 0xc9, 0xcf, 0xb3, - 0x0d, 0x4e, 0x67, 0x1b, 0xfc, 0x3e, 0xdb, 0xe0, 0xdb, 0xc5, 0xd6, 0x4e, 0x17, 0x5b, 0xfb, 0x75, - 0xb1, 0xb5, 0x8f, 0x41, 0xc6, 0xd5, 0xb6, 0x4a, 0x11, 0x15, 0x39, 0xa6, 0x42, 0xe6, 0x42, 0x62, - 0x9e, 0x52, 0x2f, 0x13, 0xf8, 0xe0, 0xe3, 0x5c, 0x6c, 0xaa, 0x1d, 0x93, 0x4d, 0xa3, 0x12, 0xfb, - 0x6f, 0xbc, 0x5b, 0x1e, 0xef, 0x5a, 0x66, 0xdb, 0x64, 0xfa, 0xb8, 0x3d, 0xe1, 0xf8, 0x4f, 0x00, - 0x00, 0x00, 0xff, 0xff, 0x69, 0x0c, 0xd9, 0x88, 0x01, 0x02, 0x00, 0x00, + // 356 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x91, 0x31, 0x4b, 0xf3, 0x40, + 0x1c, 0xc6, 0x93, 0xb7, 0xe1, 0xe5, 0x7d, 0x4f, 0x28, 0x12, 0x3a, 0xd4, 0x14, 0x42, 0x70, 0xb1, + 0x08, 0xb9, 0xb3, 0xe9, 0xe0, 0xe4, 0x50, 0xdb, 0x08, 0x5d, 0xa4, 0xd4, 0x14, 0xaa, 0x4b, 0xb8, + 0x9c, 0xe7, 0xf5, 0xb0, 0xc9, 0x85, 0xde, 0xa5, 0x90, 0x6f, 0x20, 0x4e, 0x7e, 0x01, 0x27, 0xc1, + 0xcf, 0xe2, 0xd8, 0xd1, 0x51, 0xda, 0x2f, 0x22, 0x49, 0xb0, 0x75, 0x70, 0x70, 0x7b, 0xb8, 0xfb, + 0x3f, 0x0f, 0xbf, 0x87, 0x07, 0x74, 0x79, 0x44, 0x10, 0x4e, 0xd3, 0x39, 0x27, 0x58, 0x71, 0x91, + 0x48, 0xc4, 0x13, 0x45, 0x17, 0x64, 0x86, 0x79, 0x12, 0x62, 0x42, 0x44, 0x96, 0x28, 0x89, 0x96, + 0x1d, 0xa4, 0xf2, 0x94, 0x4a, 0x98, 0x2e, 0x84, 0x12, 0xe6, 0x11, 0x8f, 0x08, 0xfc, 0x6e, 0x82, + 0x3f, 0x98, 0xe0, 0xb2, 0x63, 0x1d, 0x30, 0x21, 0xd8, 0x9c, 0xa2, 0xd2, 0x16, 0x65, 0x77, 0x08, + 0x27, 0x79, 0x95, 0x61, 0x35, 0x98, 0x60, 0xa2, 0x94, 0xa8, 0x50, 0xd5, 0xeb, 0xe1, 0xab, 0x0e, + 0x5a, 0xc3, 0x6d, 0x56, 0xaf, 0x8a, 0x1a, 0x61, 0x72, 0x4f, 0xd5, 0x00, 0x2b, 0x6c, 0xf6, 0x80, + 0x51, 0x80, 0x34, 0x75, 0x47, 0x6f, 0xd7, 0x3d, 0x17, 0xfe, 0x12, 0x04, 0x06, 0x79, 0x4a, 0xc7, + 0xa5, 0xd5, 0x3c, 0x01, 0xff, 0x62, 0x2a, 0x25, 0x66, 0x54, 0x36, 0xff, 0x38, 0xb5, 0xf6, 0x9e, + 0xd7, 0x80, 0x15, 0x26, 0xfc, 0xc2, 0x84, 0xbd, 0x24, 0x1f, 0x6f, 0xaf, 0x4c, 0x13, 0x18, 0x31, + 0x8d, 0x45, 0xb3, 0xe6, 0xe8, 0xed, 0xff, 0xe3, 0x52, 0x1f, 0x9f, 0x01, 0xa3, 0xc8, 0x34, 0x11, + 0x68, 0x05, 0xd7, 0x23, 0x3f, 0xf4, 0xa7, 0x7e, 0x7f, 0x12, 0xf8, 0x61, 0x30, 0x0d, 0x27, 0x97, + 0x57, 0x23, 0xbf, 0x3f, 0xbc, 0x18, 0xfa, 0x83, 0x7d, 0xcd, 0xaa, 0x3f, 0x3e, 0x3b, 0x60, 0xf7, + 0x6b, 0x19, 0x0f, 0x2f, 0xb6, 0x76, 0x1e, 0xbe, 0xad, 0x6d, 0x7d, 0xb5, 0xb6, 0xf5, 0x8f, 0xb5, + 0xad, 0x3f, 0x6d, 0x6c, 0x6d, 0xb5, 0xb1, 0xb5, 0xf7, 0x8d, 0xad, 0xdd, 0xf8, 0x8c, 0xab, 0x59, + 0x16, 0x41, 0x22, 0x62, 0x44, 0x84, 0x8c, 0x85, 0x44, 0x3c, 0x22, 0x2e, 0x13, 0x68, 0xe9, 0xa1, + 0x58, 0xdc, 0x66, 0x73, 0x2a, 0x8b, 0xc1, 0x24, 0xf2, 0x4e, 0xdd, 0x5d, 0x5b, 0x77, 0xbb, 0x55, + 0x39, 0x54, 0xf4, 0xb7, 0xec, 0xd2, 0xfd, 0x0c, 0x00, 0x00, 0xff, 0xff, 0x59, 0xf8, 0x0d, 0xe4, + 0xe0, 0x01, 0x00, 0x00, } -func (m *IBCTxBody) Marshal() (dAtA []byte, err error) { +func (m *InterchainAccountPacketData) Marshal() (dAtA []byte, err error) { size := m.Size() dAtA = make([]byte, size) n, err := m.MarshalToSizedBuffer(dAtA[:size]) @@ -204,16 +157,23 @@ func (m *IBCTxBody) Marshal() (dAtA []byte, err error) { return dAtA[:n], nil } -func (m *IBCTxBody) MarshalTo(dAtA []byte) (int, error) { +func (m *InterchainAccountPacketData) MarshalTo(dAtA []byte) (int, error) { size := m.Size() return m.MarshalToSizedBuffer(dAtA[:size]) } -func (m *IBCTxBody) MarshalToSizedBuffer(dAtA []byte) (int, error) { +func (m *InterchainAccountPacketData) MarshalToSizedBuffer(dAtA []byte) (int, error) { i := len(dAtA) _ = i var l int _ = l + if len(m.Memo) > 0 { + i -= len(m.Memo) + copy(dAtA[i:], m.Memo) + i = encodeVarintTypes(dAtA, i, uint64(len(m.Memo))) + i-- + dAtA[i] = 0x1a + } if len(m.Messages) > 0 { for iNdEx := len(m.Messages) - 1; iNdEx >= 0; iNdEx-- { { @@ -225,46 +185,9 @@ func (m *IBCTxBody) MarshalToSizedBuffer(dAtA []byte) (int, error) { i = encodeVarintTypes(dAtA, i, uint64(size)) } i-- - dAtA[i] = 0xa + dAtA[i] = 0x12 } } - return len(dAtA) - i, nil -} - -func (m *InterchainAccountPacketData) Marshal() (dAtA []byte, err error) { - size := m.Size() - dAtA = make([]byte, size) - n, err := m.MarshalToSizedBuffer(dAtA[:size]) - if err != nil { - return nil, err - } - return dAtA[:n], nil -} - -func (m *InterchainAccountPacketData) MarshalTo(dAtA []byte) (int, error) { - size := m.Size() - return m.MarshalToSizedBuffer(dAtA[:size]) -} - -func (m *InterchainAccountPacketData) MarshalToSizedBuffer(dAtA []byte) (int, error) { - i := len(dAtA) - _ = i - var l int - _ = l - if len(m.Memo) > 0 { - i -= len(m.Memo) - copy(dAtA[i:], m.Memo) - i = encodeVarintTypes(dAtA, i, uint64(len(m.Memo))) - i-- - dAtA[i] = 0x1a - } - if len(m.Data) > 0 { - i -= len(m.Data) - copy(dAtA[i:], m.Data) - i = encodeVarintTypes(dAtA, i, uint64(len(m.Data))) - i-- - dAtA[i] = 0x12 - } if m.Type != 0 { i = encodeVarintTypes(dAtA, i, uint64(m.Type)) i-- @@ -284,34 +207,21 @@ func encodeVarintTypes(dAtA []byte, offset int, v uint64) int { dAtA[offset] = uint8(v) return base } -func (m *IBCTxBody) Size() (n int) { +func (m *InterchainAccountPacketData) Size() (n int) { if m == nil { return 0 } var l int _ = l + if m.Type != 0 { + n += 1 + sovTypes(uint64(m.Type)) + } if len(m.Messages) > 0 { for _, e := range m.Messages { l = e.Size() n += 1 + l + sovTypes(uint64(l)) } } - return n -} - -func (m *InterchainAccountPacketData) Size() (n int) { - if m == nil { - return 0 - } - var l int - _ = l - if m.Type != 0 { - n += 1 + sovTypes(uint64(m.Type)) - } - l = len(m.Data) - if l > 0 { - n += 1 + l + sovTypes(uint64(l)) - } l = len(m.Memo) if l > 0 { n += 1 + l + sovTypes(uint64(l)) @@ -325,90 +235,6 @@ func sovTypes(x uint64) (n int) { func sozTypes(x uint64) (n int) { return sovTypes(uint64((x << 1) ^ uint64((int64(x) >> 63)))) } -func (m *IBCTxBody) Unmarshal(dAtA []byte) error { - l := len(dAtA) - iNdEx := 0 - for iNdEx < l { - preIndex := iNdEx - var wire uint64 - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowTypes - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - wire |= uint64(b&0x7F) << shift - if b < 0x80 { - break - } - } - fieldNum := int32(wire >> 3) - wireType := int(wire & 0x7) - if wireType == 4 { - return fmt.Errorf("proto: IBCTxBody: wiretype end group for non-group") - } - if fieldNum <= 0 { - return fmt.Errorf("proto: IBCTxBody: illegal tag %d (wire type %d)", fieldNum, wire) - } - switch fieldNum { - case 1: - if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field Messages", wireType) - } - var msglen int - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowTypes - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - msglen |= int(b&0x7F) << shift - if b < 0x80 { - break - } - } - if msglen < 0 { - return ErrInvalidLengthTypes - } - postIndex := iNdEx + msglen - if postIndex < 0 { - return ErrInvalidLengthTypes - } - if postIndex > l { - return io.ErrUnexpectedEOF - } - m.Messages = append(m.Messages, &types.Any{}) - if err := m.Messages[len(m.Messages)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { - return err - } - iNdEx = postIndex - default: - iNdEx = preIndex - skippy, err := skipTypes(dAtA[iNdEx:]) - if err != nil { - return err - } - if (skippy < 0) || (iNdEx+skippy) < 0 { - return ErrInvalidLengthTypes - } - if (iNdEx + skippy) > l { - return io.ErrUnexpectedEOF - } - iNdEx += skippy - } - } - - if iNdEx > l { - return io.ErrUnexpectedEOF - } - return nil -} func (m *InterchainAccountPacketData) Unmarshal(dAtA []byte) error { l := len(dAtA) iNdEx := 0 @@ -459,9 +285,9 @@ func (m *InterchainAccountPacketData) Unmarshal(dAtA []byte) error { } case 2: if wireType != 2 { - return fmt.Errorf("proto: wrong wireType = %d for field Data", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field Messages", wireType) } - var byteLen int + var msglen int for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowTypes @@ -471,24 +297,24 @@ func (m *InterchainAccountPacketData) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - byteLen |= int(b&0x7F) << shift + msglen |= int(b&0x7F) << shift if b < 0x80 { break } } - if byteLen < 0 { + if msglen < 0 { return ErrInvalidLengthTypes } - postIndex := iNdEx + byteLen + postIndex := iNdEx + msglen if postIndex < 0 { return ErrInvalidLengthTypes } if postIndex > l { return io.ErrUnexpectedEOF } - m.Data = append(m.Data[:0], dAtA[iNdEx:postIndex]...) - if m.Data == nil { - m.Data = []byte{} + m.Messages = append(m.Messages, &types.Any{}) + if err := m.Messages[len(m.Messages)-1].Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err } iNdEx = postIndex case 3: diff --git a/proto/ibc/applications/interchain_accounts/v1/types.proto b/proto/ibc/applications/interchain_accounts/v1/types.proto index 758c2539072..77192f333f3 100644 --- a/proto/ibc/applications/interchain_accounts/v1/types.proto +++ b/proto/ibc/applications/interchain_accounts/v1/types.proto @@ -5,10 +5,6 @@ import "google/protobuf/any.proto"; import "gogoproto/gogo.proto"; option go_package = "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"; -// Body of a tx for an ics27 IBC packet -message IBCTxBody { - repeated google.protobuf.Any messages = 1; -} // The different types of interchain account transactions // EXECUTE_TX is used when sending a TX from the controller side to the host side. The host side will execute the tx on @@ -22,6 +18,6 @@ enum Type { // InterchainAccountPacketData is comprised of a raw transaction, type of transaction and optional memo field. message InterchainAccountPacketData { Type type = 1; - bytes data = 2; + repeated google.protobuf.Any messages = 2; string memo = 3; } From 7436f4fe2cbfc0291b27aa960748727d4b58b512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Fri, 8 Oct 2021 13:53:25 +0200 Subject: [PATCH 2/4] adjust SerializeCosmosTx, fix tests --- .../27-interchain-accounts/keeper/keeper.go | 19 +++- .../keeper/relay_test.go | 99 +++++++++---------- 2 files changed, 63 insertions(+), 55 deletions(-) diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go index 98e6418faa5..d730b8a8c94 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper.go @@ -57,9 +57,11 @@ func NewKeeper( } } -// SerializeCosmosTx packs a slice of sdk.Msg into a slice of Any's. -func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (msgAnys []*codectypes.Any, err error) { - msgAnys = make([]*codectypes.Any, len(msgs)) +// SerializeCosmosTx serializes a slice of sdk.Msg's using the CosmosTx type. The sdk.Msg's are +// packed into Any's and inserted into the Messages field of a CosmosTx. The proto marshaled CosmosTx +// bytes are returned. +func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (bz []byte, err error) { + msgAnys := make([]*codectypes.Any, len(msgs)) for i, msg := range msgs { msgAnys[i], err = codectypes.NewAnyWithValue(msg) @@ -68,7 +70,16 @@ func (k Keeper) SerializeCosmosTx(cdc codec.BinaryCodec, msgs []sdk.Msg) (msgAny } } - return msgAnys, nil + cosmosTx := &types.CosmosTx{ + Messages: msgAnys, + } + + bz, err = cdc.Marshal(cosmosTx) + if err != nil { + return nil, err + } + + return bz, nil } // Logger returns the application logger, scoped to the associated module diff --git a/modules/apps/27-interchain-accounts/keeper/relay_test.go b/modules/apps/27-interchain-accounts/keeper/relay_test.go index 8b421941ed8..952391fd039 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/keeper/relay_test.go @@ -3,7 +3,6 @@ package keeper_test import ( "fmt" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" @@ -16,9 +15,9 @@ import ( func (suite *KeeperTestSuite) TestTrySendTx() { var ( - path *ibctesting.Path - msg interface{} - portID string + path *ibctesting.Path + icaPacketData types.InterchainAccountPacketData + portID string ) testCases := []struct { @@ -28,9 +27,6 @@ func (suite *KeeperTestSuite) TestTrySendTx() { }{ { "success", func() { - amount, _ := sdk.ParseCoinsNormalized("100stake") - interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) - msg = []sdk.Msg{&banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount}} }, true, }, { @@ -39,45 +35,30 @@ func (suite *KeeperTestSuite) TestTrySendTx() { interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) msg1 := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} msg2 := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} - msg = []sdk.Msg{msg1, msg2} + data, err := suite.chainB.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg1, msg2}) + suite.Require().NoError(err) + icaPacketData.Data = data }, true, }, { - "incorrect outgoing data", func() { - msg = []byte{} - }, false, - }, - { - "incorrect outgoing data - []sdk.Msg is not used", func() { - amount, _ := sdk.ParseCoinsNormalized("100stake") - interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) - msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} + "data is nil", func() { + icaPacketData.Data = nil }, false, }, { "active channel not found", func() { - amount, _ := sdk.ParseCoinsNormalized("100stake") - interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) - msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} portID = "incorrect portID" }, false, }, { "channel does not exist", func() { - amount, _ := sdk.ParseCoinsNormalized("100stake") - interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) - msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), portID, "channel-100") }, false, }, { - "data is nil", func() { - msg = nil - }, false, - }, - { - "data is not an SDK message", func() { - msg = "not an sdk message" + "SendPacket fails - channel closed", func() { + err := path.EndpointA.SetChannelClosed() + suite.Require().NoError(err) }, false, }, } @@ -89,16 +70,28 @@ func (suite *KeeperTestSuite) TestTrySendTx() { suite.SetupTest() // reset path = NewICAPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) - memo := "memo" err := suite.SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) portID = path.EndpointA.ChannelConfig.PortID + amount, _ := sdk.ParseCoinsNormalized("100stake") + interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) + msg := &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} + data, err := suite.chainB.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainB.GetSimApp().AppCodec(), []sdk.Msg{msg}) + suite.Require().NoError(err) + + // default packet data, must be modified in malleate for test cases expected to fail + icaPacketData = types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: data, + Memo: "memo", + } + tc.malleate() - _, err = suite.chainA.GetSimApp().ICAKeeper.TrySendTx(suite.chainA.GetContext(), portID, msg, memo) + _, err = suite.chainA.GetSimApp().ICAKeeper.TrySendTx(suite.chainA.GetContext(), portID, icaPacketData) if tc.expPass { suite.Require().NoError(err) @@ -129,37 +122,40 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { interchainAccountAddr, _ := suite.chainB.GetSimApp().ICAKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID) msg = &banktypes.MsgSend{FromAddress: interchainAccountAddr, ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} // build packet data - msgs, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg}) + data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg}) suite.Require().NoError(err) - data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX, - Messages: msgs, + icaPacketData := types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: data, } - packetData = data.GetBytes() + packetData = icaPacketData.GetBytes() }, true, }, { "Cannot deserialize packet data messages", func() { - // DeserializeCosmosTx expects Any's unpacked into sdk.MSg - any, err := codectypes.NewAnyWithValue(&types.InterchainAccountPacketData{}) - suite.Require().NoError(err) - msgs := []*codectypes.Any{any} + data := []byte("invalid packet data") - data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX, - Messages: msgs, + icaPacketData := types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: data, } - packetData = data.GetBytes() + packetData = icaPacketData.GetBytes() }, false, }, { "Invalid packet type", func() { - msgs := []*codectypes.Any{} + // build packet data + data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{&banktypes.MsgSend{}}) + suite.Require().NoError(err) + // Type here is an ENUM // Valid type is types.EXECUTE_TX - data := types.InterchainAccountPacketData{Type: 100, - Messages: msgs, + icaPacketData := types.InterchainAccountPacketData{ + Type: 100, + Data: data, } - packetData = data.GetBytes() + packetData = icaPacketData.GetBytes() }, false, }, { @@ -179,12 +175,13 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { // Incorrect FromAddress msg = &banktypes.MsgSend{FromAddress: suite.chainB.SenderAccount.GetAddress().String(), ToAddress: suite.chainB.SenderAccount.GetAddress().String(), Amount: amount} // build packet data - msgs, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg}) + data, err := suite.chainA.GetSimApp().ICAKeeper.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{msg}) suite.Require().NoError(err) - data := types.InterchainAccountPacketData{Type: types.EXECUTE_TX, - Messages: msgs, + icaPacketData := types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: data, } - packetData = data.GetBytes() + packetData = icaPacketData.GetBytes() }, false, }, } From 9c8de88382bd1dbfc0735001afae92a719eeeada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Fri, 8 Oct 2021 14:09:14 +0200 Subject: [PATCH 3/4] apply self nits --- modules/apps/27-interchain-accounts/keeper/relay.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/apps/27-interchain-accounts/keeper/relay.go b/modules/apps/27-interchain-accounts/keeper/relay.go index 26254361b51..26937cd6cf1 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/keeper/relay.go @@ -12,7 +12,7 @@ import ( // TODO: implement middleware functionality, this will allow us to use capabilities to // manage helper module access to owner addresses they do not have capabilities for -func (k Keeper) TrySendTx(ctx sdk.Context, portID string, packetData types.InterchainAccountPacketData) (uint64, error) { +func (k Keeper) TrySendTx(ctx sdk.Context, portID string, icaPacketData types.InterchainAccountPacketData) (uint64, error) { // Check for the active channel activeChannelId, found := k.GetActiveChannel(ctx, portID) if !found { @@ -27,7 +27,7 @@ func (k Keeper) TrySendTx(ctx sdk.Context, portID string, packetData types.Inter destinationPort := sourceChannelEnd.GetCounterparty().GetPortID() destinationChannel := sourceChannelEnd.GetCounterparty().GetChannelID() - return k.createOutgoingPacket(ctx, portID, activeChannelId, destinationPort, destinationChannel, packetData) + return k.createOutgoingPacket(ctx, portID, activeChannelId, destinationPort, destinationChannel, icaPacketData) } func (k Keeper) createOutgoingPacket( @@ -36,9 +36,9 @@ func (k Keeper) createOutgoingPacket( sourceChannel, destinationPort, destinationChannel string, - packetData types.InterchainAccountPacketData, + icaPacketData types.InterchainAccountPacketData, ) (uint64, error) { - if err := packetData.ValidateBasic(); err != nil { + if err := icaPacketData.ValidateBasic(); err != nil { return 0, sdkerrors.Wrap(err, "invalid interchain account packet data") } @@ -58,7 +58,7 @@ func (k Keeper) createOutgoingPacket( const timeoutTimestamp = ^uint64(0) >> 1 // Shift the unsigned bit to satisfy hermes relayer timestamp conversion packet := channeltypes.NewPacket( - packetData.GetBytes(), + icaPacketData.GetBytes(), sequence, sourcePort, sourceChannel, From d25ea6716a24128afa827b7d0637f360e3643c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 12 Oct 2021 12:09:15 +0200 Subject: [PATCH 4/4] add memo length validation --- modules/apps/27-interchain-accounts/types/packet.go | 6 ++++++ .../apps/27-interchain-accounts/types/packet_test.go | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/modules/apps/27-interchain-accounts/types/packet.go b/modules/apps/27-interchain-accounts/types/packet.go index da3dd035783..d69dfebd30a 100644 --- a/modules/apps/27-interchain-accounts/types/packet.go +++ b/modules/apps/27-interchain-accounts/types/packet.go @@ -6,12 +6,18 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) +const MaxMemoCharLength = 256 + // ValidateBasic performs basic validation of the interchain account packet data. // The memo may be empty. func (iapd InterchainAccountPacketData) ValidateBasic() error { if iapd.Data == nil { return sdkerrors.Wrap(ErrInvalidOutgoingData, "packet data cannot be empty") } + + if len(iapd.Memo) > MaxMemoCharLength { + return sdkerrors.Wrapf(ErrInvalidOutgoingData, "packet data memo cannot be greater than %d characters", MaxMemoCharLength) + } // TODO: add type validation when data type enum supports unspecified type return nil diff --git a/modules/apps/27-interchain-accounts/types/packet_test.go b/modules/apps/27-interchain-accounts/types/packet_test.go index 9dc8eebfb34..ce2fab2f5ca 100644 --- a/modules/apps/27-interchain-accounts/types/packet_test.go +++ b/modules/apps/27-interchain-accounts/types/packet_test.go @@ -4,6 +4,8 @@ import ( "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" ) +var largeMemo = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum" + func (suite *TypesTestSuite) TestValidateBasic() { testCases := []struct { name string @@ -36,6 +38,15 @@ func (suite *TypesTestSuite) TestValidateBasic() { }, false, }, + { + "memo too large", + types.InterchainAccountPacketData{ + Type: types.EXECUTE_TX, + Data: []byte("data"), + Memo: largeMemo, + }, + false, + }, } for _, tc := range testCases {