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

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

Merged
merged 6 commits into from
Oct 14, 2022
Merged
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
2 changes: 2 additions & 0 deletions docs/ibc/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |



Expand Down Expand Up @@ -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 |



Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
)
9 changes: 9 additions & 0 deletions modules/apps/transfer/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/transfer/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions modules/apps/transfer/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
3 changes: 3 additions & 0 deletions modules/apps/transfer/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (k Keeper) SendTransfer(
receiver,
timeoutHeight,
timeoutTimestamp,
nil,
)
return err
}
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand Down
22 changes: 16 additions & 6 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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",
Expand Down Expand Up @@ -149,6 +149,7 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
trace types.DenomTrace
amount sdk.Int
receiver string
metadata []byte
)

testCases := []struct {
Expand All @@ -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()
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions modules/apps/transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package transfer

import (
"context"
"encoding/hex"
"encoding/json"
"fmt"
"math"
Expand Down Expand Up @@ -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())),
),
)
Expand Down Expand Up @@ -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()),
),
)
Expand Down Expand Up @@ -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)),
),
)

Expand Down
1 change: 1 addition & 0 deletions modules/apps/transfer/spec/04_messages.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type MsgTransfer struct {
Receiver string
TimeoutHeight ibcexported.Height
TimeoutTimestamp uint64
Metadata []byte
}
```

Expand Down
4 changes: 4 additions & 0 deletions modules/apps/transfer/spec/05_events.md
Original file line number Diff line number Diff line change
Expand Up @@ -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} |
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
| denomination_trace | trace_hash | {hex_hash} |

## OnAcknowledgePacket callback
Expand All @@ -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
Expand All @@ -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} |
1 change: 1 addition & 0 deletions modules/apps/transfer/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ const (
AttributeKeyAck = "acknowledgement"
AttributeKeyAckError = "error"
AttributeKeyTraceHash = "trace_hash"
AttributeKeyMetadata = "metadata"
)
1 change: 1 addition & 0 deletions modules/apps/transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
)

// NewMsgTransfer creates a new MsgTransfer instance
//
//nolint:interfacer
func NewMsgTransfer(
sourcePort, sourceChannel string,
Expand Down
88 changes: 72 additions & 16 deletions modules/apps/transfer/types/packet.pb.go

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

Loading