Added optional packet metadata to the packet and message types (backp…

…ort #2305) (#2504)

* Added optional packet metadata to the packet and message types (#2305)

* added optional packet metadata to the packet and message types

* added docs

* breaking the api (backports should add a utility function for this)

* adding nil metadata on all the calls

* added metadata to the cli

* added events

* breaking api for FungibleTokenPacketData

* hex encoding metadata

* added abstraction

* fixed bad merge

* added tests with metadata

* added missing metadata to packet for recv

* cleaning up metadata on every test

* reset metadata

* added metadata flag

* lint

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

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* fixed bad call in tests

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit 82397d6)

# Conflicts:
#	docs/apps/transfer/messages.md
#	docs/ibc/proto-docs.md
#	go.mod
#	go.sum
#	modules/apps/29-fee/transfer_test.go
#	modules/apps/transfer/ibc_module.go
#	modules/apps/transfer/keeper/mbt_relay_test.go
#	modules/apps/transfer/keeper/relay_test.go
#	modules/apps/transfer/spec/05_events.md
#	modules/apps/transfer/transfer_test.go
#	modules/apps/transfer/types/packet.pb.go
#	modules/apps/transfer/types/tx.pb.go
#	proto/ibc/applications/interchain_accounts/controller/v1/tx.proto

* fix most merge conflicts, remove API breaking changes

* chore: fix remaining merge conflicts

* chore: revert all unnecessary changes

* fix docs

Co-authored-by: Nicolas Lara <nicolaslara@gmail.com>
Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Carlos Rodriguez <crodveg@gmail.com>

docs/ibc/proto-docs.md

@@ -699,6 +699,7 @@ https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transf
 | `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 nanoseconds) relative to the current block timestamp. The timeout is disabled when set to 0. |
+| `metadata` | [bytes](#bytes) |  | optional metadata |
 
 
 
@@ -760,6 +761,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 |
+| `metadata` | [bytes](#bytes) |  | optional metadata |
 
 
 

go.mod

@@ -24,7 +24,6 @@ require (
 	google.golang.org/genproto v0.0.0-20220725144611-272f38e5d71b
 	google.golang.org/grpc v1.48.0
 	google.golang.org/protobuf v1.28.0
-	gopkg.in/yaml.v2 v2.4.0 // indirect
 )
 
 require (
@@ -119,6 +118,7 @@ require (
 	golang.org/x/term v0.0.0-20220722155259-a9ba230a4035 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	gopkg.in/ini.v1 v1.66.6 // indirect
+	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	nhooyr.io/websocket v1.8.6 // indirect
 )

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

@@ -22,6 +22,7 @@ const (
 	flagPacketTimeoutHeight    = "packet-timeout-height"
 	flagPacketTimeoutTimestamp = "packet-timeout-timestamp"
 	flagAbsoluteTimeouts       = "absolute-timeouts"
+	flagMetadata               = "metadata"
 )
 
 // NewTransferTxCmd returns the command to create a NewMsgTransfer transaction
@@ -76,6 +77,11 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 				return err
 			}
 
+			metadataStr, err := cmd.Flags().GetString(flagMetadata)
+			if err != nil {
+				return err
+			}
+
 			// if the timeouts are not absolute, retrieve latest block height and block timestamp
 			// for the consensus state connected to the destination port/channel
 			if !absoluteTimeouts {
@@ -113,13 +119,16 @@ corresponding to the counterparty channel. Any timeout set to 0 is disabled.`),
 			msg := types.NewMsgTransfer(
 				srcPort, srcChannel, coin, sender, receiver, timeoutHeight, timeoutTimestamp,
 			)
+			msg.Metadata = []byte(metadataStr)
+
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},
 	}
 
 	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. 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.")
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd

modules/apps/transfer/keeper/msg_server.go

@@ -20,7 +20,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)
 	if err != nil {
 		return nil, err
 	}

modules/apps/transfer/keeper/msg_server_test.go

@@ -55,6 +55,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() {
 			coin, suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(),
 			suite.chainB.GetTimeoutHeight(), 0, // only use timeout height
 		)
+		msg.Metadata = []byte("custom metadata")
 
 		tc.malleate()
 

modules/apps/transfer/keeper/relay.go

@@ -69,6 +69,7 @@ func (k Keeper) SendTransfer(
 		receiver,
 		timeoutHeight,
 		timeoutTimestamp,
+		nil,
 	)
 	return err
 }
@@ -83,6 +84,7 @@ func (k Keeper) sendTransfer(
 	receiver string,
 	timeoutHeight clienttypes.Height,
 	timeoutTimestamp uint64,
+	metadata []byte,
 ) (uint64, error) {
 	if !k.GetSendEnabled(ctx) {
 		return 0, types.ErrSendDisabled
@@ -175,6 +177,7 @@ func (k Keeper) sendTransfer(
 	packetData := types.NewFungibleTokenPacketData(
 		fullDenomPath, token.Amount.String(), sender.String(), receiver,
 	)
+	packetData.Metadata = metadata
 
 	packet := channeltypes.NewPacket(
 		packetData.GetBytes(),

modules/apps/transfer/keeper/relay_test.go

@@ -34,13 +34,15 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 				suite.coordinator.CreateTransferChannels(path)
 				amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
 			}, true, true},
-		{"successful transfer with coin from counterparty chain",
+		{
+			"successful transfer with coin from counterparty chain",
 			func() {
 				// send coin from chainA back to chainB
 				suite.coordinator.CreateTransferChannels(path)
 				amount = types.GetTransferCoin(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.DefaultBondDenom, sdk.NewInt(100))
 			}, false, true},
-		{"source channel not found",
+		{
+			"source channel not found",
 			func() {
 				// channel references wrong ID
 				suite.coordinator.CreateTransferChannels(path)
@@ -59,16 +61,14 @@ func (suite *KeeperTestSuite) TestSendTransfer() {
 				)
 				suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 				amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
-			}, true, false,
-		},
+			}, true, false},
 		{
 			"transfer failed - sender account is blocked",
 			func() {
 				suite.coordinator.CreateTransferChannels(path)
 				amount = sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))
 				sender = suite.chainA.GetSimApp().AccountKeeper.GetModuleAddress(types.ModuleName)
-			}, true, false,
-		},
+			}, true, false},
 		// createOutgoingPacket tests
 		// - source chain
 		{"send coin failed",
@@ -149,6 +149,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 		trace    types.DenomTrace
 		amount   sdk.Int
 		receiver string
+		metadata []byte
 	)
 
 	testCases := []struct {
@@ -158,7 +159,13 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 		expPass      bool
 	}{
 		{"success receive on source chain", func() {}, true, true},
+		{"success receive on source chain with metadata", func() {
+			metadata = []byte("metadata")
+		}, true, true},
 		{"success receive with coin from another chain as source", func() {}, false, true},
+		{"success receive with coin from another chain as source with metadata", func() {
+			metadata = []byte("metadata")
+		}, false, true},
 		{"empty coin", func() {
 			trace = types.DenomTrace{}
 			amount = sdk.ZeroInt()
@@ -199,6 +206,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 			suite.coordinator.Setup(path)
 			receiver = suite.chainB.SenderAccount.GetAddress().String() // must be explicitly changed in malleate
 
+			metadata = []byte{}      // can be explicitly changed in malleate
 			amount = sdk.NewInt(100) // must be explicitly changed in malleate
 			seq := uint64(1)
 
@@ -226,12 +234,14 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
 
 			// send coin from chainA to chainB
 			transferMsg := types.NewMsgTransfer(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, sdk.NewCoin(trace.IBCDenom(), amount), suite.chainA.SenderAccount.GetAddress().String(), receiver, clienttypes.NewHeight(0, 110), 0)
+			transferMsg.Metadata = metadata
 			_, err := suite.chainA.SendMsgs(transferMsg)
 			suite.Require().NoError(err) // message committed
 
 			tc.malleate()
 
 			data := types.NewFungibleTokenPacketData(trace.GetFullDenomPath(), amount.String(), suite.chainA.SenderAccount.GetAddress().String(), receiver)
+			data.Metadata = metadata
 			packet := channeltypes.NewPacket(data.GetBytes(), seq, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 100), 0)
 
 			err = suite.chainB.GetSimApp().TransferKeeper.OnRecvPacket(suite.chainB.GetContext(), packet, data)

modules/apps/transfer/module.go

@@ -2,6 +2,7 @@ package transfer
 
 import (
 	"context"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"math"
@@ -354,6 +355,7 @@ func (am AppModule) OnRecvPacket(
 			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.AttributeKeyAckSuccess, fmt.Sprintf("%t", ack.Success())),
 		),
 	)
@@ -390,6 +392,7 @@ func (am AppModule) 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.AttributeKeyAck, ack.String()),
 		),
 	)
@@ -436,6 +439,7 @@ func (am AppModule) 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)),
 		),
 	)
 

modules/apps/transfer/spec/04_messages.md

@@ -17,6 +17,7 @@ type MsgTransfer struct {
   Receiver          string
   TimeoutHeight     ibcexported.Height
   TimeoutTimestamp  uint64
+  Metadata          []byte
 }
 ```
 

modules/apps/transfer/spec/05_events.md

@@ -22,6 +22,7 @@ order: 5
 | fungible_token_packet | denom         | {denom}         |
 | fungible_token_packet | amount        | {amount}        |
 | fungible_token_packet | success       | {ackSuccess}    |
+| fungible_token_packet | metadata      | {metadata}      |
 | denomination_trace    | trace_hash    | {hex_hash}      |
 
 ## OnAcknowledgePacket callback
@@ -32,6 +33,8 @@ order: 5
 | fungible_token_packet | receiver        | {receiver}        |
 | fungible_token_packet | denom           | {denom}           |
 | fungible_token_packet | amount          | {amount}          |
+| fungible_token_packet | metadata        | {metadata}        |
+| fungible_token_packet | acknowledgement | {ack.String()}    |
 | fungible_token_packet | success | error | {ack.Response}    |
 
 ## OnTimeoutPacket callback
@@ -42,3 +45,4 @@ order: 5
 | fungible_token_packet | refund_receiver | {receiver}      |
 | fungible_token_packet | denom           | {denom}         |
 | fungible_token_packet | amount          | {amount}        |
+| fungible_token_packet | metadata        | {metadata}      |

modules/apps/transfer/types/events.go

@@ -18,4 +18,5 @@ const (
 	AttributeKeyAck            = "acknowledgement"
 	AttributeKeyAckError       = "error"
 	AttributeKeyTraceHash      = "trace_hash"
+	AttributeKeyMetadata       = "metadata"
 )

modules/apps/transfer/types/msgs.go

@@ -15,6 +15,7 @@ const (
 )
 
 // NewMsgTransfer creates a new MsgTransfer instance
+//
 //nolint:interfacer
 func NewMsgTransfer(
 	sourcePort, sourceChannel string,