refactor: adapting transfer metadata bytes field to memo string (#2595)

* adapting transfer metadata bytes field to memo string

* updating changelog

(cherry picked from commit 05685b3)

# Conflicts:
#	CHANGELOG.md
#	docs/ibc/proto-docs.md
#	go.mod
#	go.sum
#	modules/apps/29-fee/transfer_test.go
#	modules/apps/transfer/client/cli/tx.go
#	modules/apps/transfer/ibc_module.go
#	modules/apps/transfer/keeper/mbt_relay_test.go
#	modules/apps/transfer/keeper/msg_server_test.go
#	modules/apps/transfer/keeper/relay.go
#	modules/apps/transfer/keeper/relay_test.go
#	modules/apps/transfer/transfer_test.go
#	modules/apps/transfer/types/msgs.go
#	modules/apps/transfer/types/msgs_test.go
#	modules/apps/transfer/types/packet.go
#	modules/apps/transfer/types/packet.pb.go
#	modules/apps/transfer/types/packet_test.go
#	modules/apps/transfer/types/tx.pb.go

CHANGELOG.md

@@ -48,7 +48,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+<<<<<<< HEAD
 * (apps/transfer) [\#2305](https://github.com/cosmos/ibc-go/pull/2305) Added optional metadata field to `FungibleTokenPacketData` and `MsgTransfer`.
+=======
+* (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) [\#2595](https://github.com/cosmos/ibc-go/pull/2595) Adding optional memo field to `FungibleTokenPacketData` and `MsgTransfer`.
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 
 ### Bug Fixes
 

docs/ibc/proto-docs.md

@@ -1099,13 +1099,24 @@ identifier fields.
 
 | Field | Type | Label | Description |
 | ----- | ---- | ----- | ----------- |
+<<<<<<< HEAD
 | `state` | [State](#ibc.core.channel.v1.State) |  | current state of the channel end |
 | `ordering` | [Order](#ibc.core.channel.v1.Order) |  | whether the channel is ordered or unordered |
 | `counterparty` | [Counterparty](#ibc.core.channel.v1.Counterparty) |  | counterparty channel end |
 | `connection_hops` | [string](#string) | repeated | list of connection identifiers, in order, along which packets sent on this channel will travel |
 | `version` | [string](#string) |  | opaque channel version, which is agreed upon during the handshake |
 | `port_id` | [string](#string) |  | port identifier |
 | `channel_id` | [string](#string) |  | channel identifier |
+=======
+| `source_port` | [string](#string) |  | the port on which the packet will be sent |
+| `source_channel` | [string](#string) |  | the channel by which the packet will be sent |
+| `token` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) |  | the tokens to be transferred |
+| `sender` | [string](#string) |  | the sender address |
+| `receiver` | [string](#string) |  | the recipient address on the destination chain |
+| `timeout_height` | [ibc.core.client.v1.Height](#ibc.core.client.v1.Height) |  | Timeout height relative to the current block height. The timeout is disabled when set to 0. |
+| `timeout_timestamp` | [uint64](#uint64) |  | Timeout timestamp in absolute nanoseconds since unix epoch. The timeout is disabled when set to 0. |
+| `memo` | [string](#string) |  | optional memo |
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 
 
 
@@ -1168,6 +1179,16 @@ Order defines if a channel is ORDERED or UNORDERED
 | ORDER_UNORDERED | 1 | packets can be delivered in any order, which may differ from the order in which they were sent. |
 | ORDER_ORDERED | 2 | packets are delivered exactly in the order which they were sent |
 
+<<<<<<< HEAD
+=======
+| Field | Type | Label | Description |
+| ----- | ---- | ----- | ----------- |
+| `denom` | [string](#string) |  | the token denomination to be transferred |
+| `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) |  | optional memo |
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 
 
 <a name="ibc.core.channel.v1.State"></a>

go.mod

@@ -35,6 +35,10 @@ require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/bgentry/speakeasy v0.1.0 // indirect
 	github.com/btcsuite/btcd v0.22.1 // indirect
+<<<<<<< HEAD
+=======
+	github.com/cenkalti/backoff/v4 v4.1.1 // indirect
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 	github.com/cespare/xxhash v1.1.0 // indirect
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
 	github.com/coinbase/rosetta-sdk-go v0.7.0 // indirect

go.sum

@@ -130,6 +130,11 @@ github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46f
 github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ=
 github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4=
 github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
+<<<<<<< HEAD
+=======
+github.com/cenkalti/backoff/v4 v4.1.1 h1:G2HAfAmvm/GcKan2oOQpBXOd2tT2G57ZnZGWa1PxPBQ=
+github.com/cenkalti/backoff/v4 v4.1.1/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/cespare/cp v0.1.0/go.mod h1:SOGHArjBr4JWaSDEVpWpo/hNg6RoKrls6Oh40hiwW+s=
 github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=

modules/apps/29-fee/transfer_test.go

@@ -0,0 +1,70 @@
+package fee_test
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/v6/modules/apps/29-fee/types"
+	transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	ibctesting "github.com/cosmos/ibc-go/v6/testing"
+)
+
+// Integration test to ensure ics29 works with ics20
+func (suite *FeeTestSuite) TestFeeTransfer() {
+	path := ibctesting.NewPath(suite.chainA, suite.chainB)
+	feeTransferVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: transfertypes.Version}))
+	path.EndpointA.ChannelConfig.Version = feeTransferVersion
+	path.EndpointB.ChannelConfig.Version = feeTransferVersion
+	path.EndpointA.ChannelConfig.PortID = transfertypes.PortID
+	path.EndpointB.ChannelConfig.PortID = transfertypes.PortID
+
+	suite.coordinator.Setup(path)
+
+	// set up coin & ics20 packet
+	coin := ibctesting.TestCoin
+	fee := types.Fee{
+		RecvFee:    defaultRecvFee,
+		AckFee:     defaultAckFee,
+		TimeoutFee: defaultTimeoutFee,
+	}
+
+	msgs := []sdk.Msg{
+		types.NewMsgPayPacketFee(fee, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, suite.chainA.SenderAccount.GetAddress().String(), nil),
+		transfertypes.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), clienttypes.NewHeight(1, 100), 0, ""),
+	}
+	res, err := suite.chainA.SendMsgs(msgs...)
+	suite.Require().NoError(err) // message committed
+
+	// after incentivizing the packets
+	originalChainASenderAccountBalance := sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom))
+
+	packet, err := ibctesting.ParsePacketFromEvents(res.GetEvents())
+	suite.Require().NoError(err)
+
+	// register counterparty address on chainB
+	// relayerAddress is address of sender account on chainB, but we will use it on chainA
+	// to differentiate from the chainA.SenderAccount for checking successful relay payouts
+	relayerAddress := suite.chainB.SenderAccount.GetAddress()
+
+	msgRegister := types.NewMsgRegisterCounterpartyPayee(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String())
+	_, err = suite.chainB.SendMsgs(msgRegister)
+	suite.Require().NoError(err) // message committed
+
+	// relay packet
+	err = path.RelayPacket(packet)
+	suite.Require().NoError(err) // relay committed
+
+	// ensure relayers got paid
+	// relayer for forward relay: chainB.SenderAccount
+	// relayer for reverse relay: chainA.SenderAccount
+
+	// check forward relay balance
+	suite.Require().Equal(
+		fee.RecvFee,
+		sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainB.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)),
+	)
+
+	suite.Require().Equal(
+		fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded
+		sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)).Sub(originalChainASenderAccountBalance[0]))
+}

modules/apps/transfer/client/cli/tx.go

@@ -22,7 +22,7 @@ const (
 	flagPacketTimeoutHeight    = "packet-timeout-height"
 	flagPacketTimeoutTimestamp = "packet-timeout-timestamp"
 	flagAbsoluteTimeouts       = "absolute-timeouts"
-	flagMetadata               = "metadata"
+	flagMemo                   = "memo"
 )
 
 // NewTransferTxCmd returns the command to create a NewMsgTransfer transaction
@@ -77,7 +77,7 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 				return err
 			}
 
-			metadataStr, err := cmd.Flags().GetString(flagMetadata)
+			memo, err := cmd.Flags().GetString(flagMemo)
 			if err != nil {
 				return err
 			}
@@ -117,7 +117,11 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 			}
 
 			msg := types.NewMsgTransfer(
+<<<<<<< HEAD
 				srcPort, srcChannel, coin, sender, receiver, timeoutHeight, timeoutTimestamp,
+=======
+				srcPort, srcChannel, coin, sender, receiver, timeoutHeight, timeoutTimestamp, memo,
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 			)
 			msg.Metadata = []byte(metadataStr)
 
@@ -128,7 +132,7 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 	cmd.Flags().String(flagPacketTimeoutHeight, types.DefaultRelativePacketTimeoutHeight, "Packet timeout block height. The timeout is disabled when set to 0-0.")
 	cmd.Flags().Uint64(flagPacketTimeoutTimestamp, types.DefaultRelativePacketTimeoutTimestamp, "Packet timeout timestamp in nanoseconds from now. Default is 10 minutes. The timeout is disabled when set to 0.")
 	cmd.Flags().Bool(flagAbsoluteTimeouts, false, "Timeout flags are used as absolute timeouts.")
-	cmd.Flags().String(flagMetadata, "", "Metadata to be sent along with the packet. The CLI accepts only strings here but you can construct a packet with arbitrary bytes via code.")
+	cmd.Flags().String(flagMemo, "", "Memo to be sent along with the packet.")
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd

modules/apps/transfer/ibc_module.go

@@ -1,7 +1,6 @@
 package transfer
 
 import (
-	"encoding/hex"
 	"fmt"
 	"math"
 
@@ -187,6 +186,23 @@ func (im IBCModule) OnRecvPacket(
 		}
 	}
 
+<<<<<<< HEAD
+=======
+	eventAttributes := []sdk.Attribute{
+		sdk.NewAttribute(sdk.AttributeKeyModule, types.ModuleName),
+		sdk.NewAttribute(sdk.AttributeKeySender, data.Sender),
+		sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
+		sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
+		sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
+		sdk.NewAttribute(types.AttributeKeyMemo, data.Memo),
+		sdk.NewAttribute(types.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())),
+	}
+
+	if ackErr != nil {
+		eventAttributes = append(eventAttributes, sdk.NewAttribute(types.AttributeKeyAckError, ackErr.Error()))
+	}
+
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 	ctx.EventManager().EmitEvent(
 		sdk.NewEvent(
 			types.EventTypePacket,
@@ -232,7 +248,7 @@ func (im IBCModule) OnAcknowledgementPacket(
 			sdk.NewAttribute(types.AttributeKeyReceiver, data.Receiver),
 			sdk.NewAttribute(types.AttributeKeyDenom, data.Denom),
 			sdk.NewAttribute(types.AttributeKeyAmount, data.Amount),
-			sdk.NewAttribute(types.AttributeKeyMetadata, hex.EncodeToString(data.Metadata)),
+			sdk.NewAttribute(types.AttributeKeyMemo, data.Memo),
 			sdk.NewAttribute(types.AttributeKeyAck, ack.String()),
 		),
 	)
@@ -279,7 +295,7 @@ func (im IBCModule) OnTimeoutPacket(
 			sdk.NewAttribute(types.AttributeKeyRefundReceiver, data.Sender),
 			sdk.NewAttribute(types.AttributeKeyRefundDenom, data.Denom),
 			sdk.NewAttribute(types.AttributeKeyRefundAmount, data.Amount),
-			sdk.NewAttribute(types.AttributeKeyMetadata, hex.EncodeToString(data.Metadata)),
+			sdk.NewAttribute(types.AttributeKeyMemo, data.Memo),
 		),
 	)
 

modules/apps/transfer/keeper/mbt_relay_test.go

@@ -145,7 +145,12 @@ func FungibleTokenPacketFromTla(packet TlaFungibleTokenPacket) FungibleTokenPack
 			DenomFromTla(packet.Data.Denom),
 			packet.Data.Amount,
 			AddressFromString(packet.Data.Sender),
+<<<<<<< HEAD
 			AddressFromString(packet.Data.Receiver)),
+=======
+			AddressFromString(packet.Data.Receiver),
+			""),
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 	}
 }
 
@@ -344,8 +349,17 @@ func (suite *KeeperTestSuite) TestModelBasedRelay() {
 							sdk.NewCoin(denom, amount),
 							sender,
 							tc.packet.Data.Receiver,
+<<<<<<< HEAD
 							clienttypes.NewHeight(0, 110),
 							0)
+=======
+							suite.chainA.GetTimeoutHeight(), 0, // only use timeout height
+							"",
+						)
+
+						_, err = suite.chainB.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(suite.chainB.GetContext()), msg)
+
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 					}
 				case "OnRecvPacket":
 					err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, tc.packet.Data)

modules/apps/transfer/keeper/msg_server.go

@@ -21,7 +21,7 @@ func (k Keeper) Transfer(goCtx context.Context, msg *types.MsgTransfer) (*types.
 
 	sequence, err := k.sendTransfer(
 		ctx, msg.SourcePort, msg.SourceChannel, msg.Token, sender, msg.Receiver, msg.TimeoutHeight, msg.TimeoutTimestamp,
-		msg.Metadata)
+		msg.Memo)
 	if err != nil {
 		return nil, err
 	}

modules/apps/transfer/keeper/msg_server_test.go

@@ -48,6 +48,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 		path := NewTransferPath(suite.chainA, suite.chainB)
 		suite.coordinator.Setup(path)
 
+<<<<<<< HEAD
 		coin := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
 		msg = types.NewMsgTransfer(
 			path.EndpointA.ChannelConfig.PortID,
@@ -56,6 +57,16 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 			suite.chainB.GetTimeoutHeight(), 0, // only use timeout height
 		)
 		msg.Metadata = []byte("custom metadata")
+=======
+			coin := sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
+			msg = types.NewMsgTransfer(
+				path.EndpointA.ChannelConfig.PortID,
+				path.EndpointA.ChannelID,
+				coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(),
+				suite.chainB.GetTimeoutHeight(), 0, // only use timeout height
+				"memo",
+			)
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 
 		tc.malleate()
 

modules/apps/transfer/keeper/relay.go

@@ -84,7 +84,7 @@ func (k Keeper) sendTransfer(
 	receiver string,
 	timeoutHeight clienttypes.Height,
 	timeoutTimestamp uint64,
-	metadata []byte,
+	memo string,
 ) (uint64, error) {
 	if !k.GetSendEnabled(ctx) {
 		return 0, types.ErrSendDisabled
@@ -175,6 +175,7 @@ func (k Keeper) sendTransfer(
 	}
 
 	packetData := types.NewFungibleTokenPacketData(
+<<<<<<< HEAD
 		fullDenomPath, token.Amount.String(), sender.String(), receiver,
 	)
 	packetData.Metadata = metadata
@@ -188,6 +189,9 @@ func (k Keeper) sendTransfer(
 		destinationChannel,
 		timeoutHeight,
 		timeoutTimestamp,
+=======
+		fullDenomPath, token.Amount.String(), sender.String(), receiver, memo,
+>>>>>>> 05685b3 (refactor: adapting transfer metadata bytes field to memo string (#2595))
 	)
 
 	if err := k.ics4Wrapper.SendPacket(ctx, channelCap, packet); err != nil {