From b9c9360dbe83d1d3b551d5c60b91db3ea74e764b Mon Sep 17 00:00:00 2001 From: Ali Zahid Raja Date: Wed, 28 Sep 2022 02:22:00 +0500 Subject: [PATCH 1/2] chore: add memo in ics20 packet data (cosmos#2411) --- docs/ibc/proto-docs.md | 1 + .../apps/transfer/keeper/mbt_relay_test.go | 4 +- modules/apps/transfer/keeper/relay.go | 4 +- modules/apps/transfer/keeper/relay_test.go | 12 +++-- modules/apps/transfer/types/packet.go | 2 + modules/apps/transfer/types/packet.pb.go | 53 ++++++++++++++++++- modules/apps/transfer/types/packet_test.go | 19 +++---- .../ibc/applications/transfer/v2/packet.proto | 1 + 8 files changed, 80 insertions(+), 16 deletions(-) diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index 8183045f4fc..8fb9d60be2a 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -2312,6 +2312,7 @@ https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transf | `amount` | [string](#string) | | the token amount to be transferred | | `sender` | [string](#string) | | the sender address | | `receiver` | [string](#string) | | the recipient address on the destination chain | +| `memo` | [string](#string) | | memo string field | diff --git a/modules/apps/transfer/keeper/mbt_relay_test.go b/modules/apps/transfer/keeper/mbt_relay_test.go index 83a7be7c0a7..e0248c0772e 100644 --- a/modules/apps/transfer/keeper/mbt_relay_test.go +++ b/modules/apps/transfer/keeper/mbt_relay_test.go @@ -33,6 +33,7 @@ type TlaFungibleTokenPacketData struct { Receiver string `json:"receiver"` Amount string `json:"amount"` Denom []string `json:"denom"` + Memo string `json:"memo"` } type TlaFungibleTokenPacket struct { @@ -146,7 +147,8 @@ func FungibleTokenPacketFromTla(packet TlaFungibleTokenPacket) FungibleTokenPack DenomFromTla(packet.Data.Denom), packet.Data.Amount, AddressFromString(packet.Data.Sender), - AddressFromString(packet.Data.Receiver)), + AddressFromString(packet.Data.Receiver), + packet.Data.Memo), } } diff --git a/modules/apps/transfer/keeper/relay.go b/modules/apps/transfer/keeper/relay.go index 653e5daeeab..97ac56ee073 100644 --- a/modules/apps/transfer/keeper/relay.go +++ b/modules/apps/transfer/keeper/relay.go @@ -148,8 +148,10 @@ func (k Keeper) SendTransfer( } } + // TODO: take memo field as a parameter in SendTransfer + memo := "memo" packetData := types.NewFungibleTokenPacketData( - fullDenomPath, token.Amount.String(), sender.String(), receiver, + fullDenomPath, token.Amount.String(), sender.String(), receiver, memo, ) packet := channeltypes.NewPacket( diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index c467459c5c3..36f8c1bbe71 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -122,11 +122,12 @@ func (suite *KeeperTestSuite) TestSendTransfer() { // send coin from chainB to chainA coinFromBToA := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100)) transferMsg := types.NewMsgTransfer(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, coinFromBToA, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 110), 0) + memo := "memo" _, err = suite.chainB.SendMsgs(transferMsg) suite.Require().NoError(err) // message committed // receive coin on chainA from chainB - fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + fungibleTokenPacket := types.NewFungibleTokenPacketData(coinFromBToA.Denom, coinFromBToA.Amount.String(), suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String(), memo) packet := channeltypes.NewPacket(fungibleTokenPacket.GetBytes(), 1, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, clienttypes.NewHeight(1, 110), 0) // get proof of packet commitment from chainB @@ -212,6 +213,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { path := NewTransferPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) receiver = suite.chainB.SenderAccount.GetAddress().String() // must be explicitly changed in malleate + memo := "memo" amount = sdk.NewInt(100) // must be explicitly changed in malleate seq := uint64(1) @@ -244,7 +246,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() { tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), receiver) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), receiver, memo) packet := channeltypes.NewPacket(data.GetBytes(), seq, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data) @@ -314,10 +316,11 @@ func (suite *KeeperTestSuite) TestOnAcknowledgementPacket() { path = NewTransferPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) amount = sdk.NewInt(100) // must be explicitly changed + memo := "memo" tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String()) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), memo) packet := channeltypes.NewPacket(data.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) @@ -410,10 +413,11 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { suite.coordinator.Setup(path) amount = sdk.NewInt(100) // must be explicitly changed sender = suite.chainA.SenderAccount.GetAddress().String() + memo := "memo" tc.malleate() - data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), sender, suite.chainB.SenderAccount.GetAddress().String()) + data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), sender, suite.chainB.SenderAccount.GetAddress().String(), memo) packet := channeltypes.NewPacket(data.GetBytes(), 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(1, 100), 0) preCoin := suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), trace.IBCDenom()) diff --git a/modules/apps/transfer/types/packet.go b/modules/apps/transfer/types/packet.go index 6203cdbfa05..4708990310b 100644 --- a/modules/apps/transfer/types/packet.go +++ b/modules/apps/transfer/types/packet.go @@ -25,12 +25,14 @@ var ( func NewFungibleTokenPacketData( denom string, amount string, sender, receiver string, + memo string, ) FungibleTokenPacketData { return FungibleTokenPacketData{ Denom: denom, Amount: amount, Sender: sender, Receiver: receiver, + Memo: memo, } } diff --git a/modules/apps/transfer/types/packet.pb.go b/modules/apps/transfer/types/packet.pb.go index 688f51cbab2..59c70f8245c 100644 --- a/modules/apps/transfer/types/packet.pb.go +++ b/modules/apps/transfer/types/packet.pb.go @@ -34,6 +34,7 @@ type FungibleTokenPacketData struct { Sender string `protobuf:"bytes,3,opt,name=sender,proto3" json:"sender,omitempty"` // the recipient address on the destination chain Receiver string `protobuf:"bytes,4,opt,name=receiver,proto3" json:"receiver,omitempty"` + Memo string `protobuf:"bytes,5,opt,name=memo,proto3" json:"memo,omitempty"` } func (m *FungibleTokenPacketData) Reset() { *m = FungibleTokenPacketData{} } @@ -97,6 +98,13 @@ func (m *FungibleTokenPacketData) GetReceiver() string { return "" } +func (m *FungibleTokenPacketData) GetMemo() string { + if m != nil { + return m.Memo + } + return "" +} + func init() { proto.RegisterType((*FungibleTokenPacketData)(nil), "ibc.applications.transfer.v2.FungibleTokenPacketData") } @@ -145,6 +153,13 @@ func (m *FungibleTokenPacketData) MarshalToSizedBuffer(dAtA []byte) (int, error) _ = i var l int _ = l + if len(m.Memo) > 0 { + i -= len(m.Memo) + copy(dAtA[i:], m.Memo) + i = encodeVarintPacket(dAtA, i, uint64(len(m.Memo))) + i-- + dAtA[i] = 0x2a + } if len(m.Receiver) > 0 { i -= len(m.Receiver) copy(dAtA[i:], m.Receiver) @@ -209,6 +224,10 @@ func (m *FungibleTokenPacketData) Size() (n int) { if l > 0 { n += 1 + l + sovPacket(uint64(l)) } + l = len(m.Memo) + if l > 0 { + n += 1 + l + sovPacket(uint64(l)) + } return n } @@ -375,6 +394,38 @@ func (m *FungibleTokenPacketData) Unmarshal(dAtA []byte) error { } m.Receiver = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex + case 5: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Memo", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowPacket + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthPacket + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthPacket + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Memo = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipPacket(dAtA[iNdEx:]) @@ -479,4 +530,4 @@ var ( ErrInvalidLengthPacket = fmt.Errorf("proto: negative length found during unmarshaling") ErrIntOverflowPacket = fmt.Errorf("proto: integer overflow") ErrUnexpectedEndOfGroupPacket = fmt.Errorf("proto: unexpected end of group") -) +) \ No newline at end of file diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index e5d21d648d2..155f7710e1d 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -11,6 +11,7 @@ const ( amount = "100" largeAmount = "18446744073709551616" // one greater than largest uint64 (^uint64(0)) invalidLargeAmount = "115792089237316195423570985008687907853269984665640564039457584007913129639936" // 2^256 + memo = "memo" ) // TestFungibleTokenPacketDataValidateBasic tests ValidateBasic for FungibleTokenPacketData @@ -20,15 +21,15 @@ func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { packetData FungibleTokenPacketData expPass bool }{ - {"valid packet", NewFungibleTokenPacketData(denom, amount, addr1, addr2), true}, - {"valid packet with large amount", NewFungibleTokenPacketData(denom, largeAmount, addr1, addr2), true}, - {"invalid denom", NewFungibleTokenPacketData("", amount, addr1, addr2), false}, - {"invalid empty amount", NewFungibleTokenPacketData(denom, "", addr1, addr2), false}, - {"invalid zero amount", NewFungibleTokenPacketData(denom, "0", addr1, addr2), false}, - {"invalid negative amount", NewFungibleTokenPacketData(denom, "-1", addr1, addr2), false}, - {"invalid large amount", NewFungibleTokenPacketData(denom, invalidLargeAmount, addr1, addr2), false}, - {"missing sender address", NewFungibleTokenPacketData(denom, amount, emptyAddr, addr2), false}, - {"missing recipient address", NewFungibleTokenPacketData(denom, amount, addr1, emptyAddr), false}, + {"valid packet", NewFungibleTokenPacketData(denom, amount, addr1, addr2, memo), true}, + {"valid packet with large amount", NewFungibleTokenPacketData(denom, largeAmount, addr1, addr2, memo), true}, + {"invalid denom", NewFungibleTokenPacketData("", amount, addr1, addr2, memo), false}, + {"invalid empty amount", NewFungibleTokenPacketData(denom, "", addr1, addr2, memo), false}, + {"invalid zero amount", NewFungibleTokenPacketData(denom, "0", addr1, addr2, memo), false}, + {"invalid negative amount", NewFungibleTokenPacketData(denom, "-1", addr1, addr2, memo), false}, + {"invalid large amount", NewFungibleTokenPacketData(denom, invalidLargeAmount, addr1, addr2, memo), false}, + {"missing sender address", NewFungibleTokenPacketData(denom, amount, emptyAddr, addr2, memo), false}, + {"missing recipient address", NewFungibleTokenPacketData(denom, amount, addr1, emptyAddr, memo), false}, } for i, tc := range testCases { diff --git a/proto/ibc/applications/transfer/v2/packet.proto b/proto/ibc/applications/transfer/v2/packet.proto index 684afcfe3de..b7c8bcfa84d 100644 --- a/proto/ibc/applications/transfer/v2/packet.proto +++ b/proto/ibc/applications/transfer/v2/packet.proto @@ -16,4 +16,5 @@ message FungibleTokenPacketData { string sender = 3; // the recipient address on the destination chain string receiver = 4; + string memo = 5; } From f087a6a5fad77b3fa726eba61bf6cf5850a602ba Mon Sep 17 00:00:00 2001 From: Ali Zahid Raja Date: Wed, 28 Sep 2022 02:26:31 +0500 Subject: [PATCH 2/2] updated changelog.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54fe9f93246..33140df5ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (apps/27-interchain-accounts) [\#2147](https://github.com/cosmos/ibc-go/pull/2147) Adding a `SubmitTx` gRPC endpoint for the ICS27 Controller module which allows owners of interchain accounts to submit transactions. This replaces the previously existing need for authentication modules to implement this standard functionality. * (testing/simapp) [\#2190](https://github.com/cosmos/ibc-go/pull/2190) Adding the new `x/group` cosmos-sdk module to simapp. +* (apps/transfer) [\#2411](https://github.com/cosmos/ibc-go/pull/2415) Adding memo field in ICS20 packet data proto. ### Bug Fixes