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

chore: add memo in ics20 packet data (cosmos#2411) #2415

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,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

Expand Down
1 change: 1 addition & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |



Expand Down
4 changes: 3 additions & 1 deletion modules/apps/transfer/keeper/mbt_relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}
}

Expand Down
4 changes: 3 additions & 1 deletion modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 8 additions & 4 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down
2 changes: 2 additions & 0 deletions modules/apps/transfer/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
53 changes: 52 additions & 1 deletion modules/apps/transfer/types/packet.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 10 additions & 9 deletions modules/apps/transfer/types/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions proto/ibc/applications/transfer/v2/packet.proto
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ message FungibleTokenPacketData {
string sender = 3;
// the recipient address on the destination chain
string receiver = 4;
string memo = 5;
}