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.

Sign up for GitHub

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

Jump to bottom

Add metadata field to transfer packet data #842

Merged
merged 3 commits into from
Oct 4, 2022
Merged

Add metadata field to transfer packet data #842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea26100
add metadata field to transfer
AdityaSripal Sep 29, 2022
ffc61ec
changelog
AdityaSripal Sep 29, 2022
ee664f2
add further discussion on metadata field
AdityaSripal Sep 30, 2022
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,4 @@ Ref: https://keepachangelog.com/en/1.0.0/
- [\#804](https:/cosmos/ibc/pull/804) Increment upgrade sequence at the start of a new handshake rather than the end of a completed handshake
- [\#806](https:/cosmos/ibc/pull/806) Adds previous version to UpgradeInit and UpgradeTry callback arguments
- [\#807](https:/cosmos/ibc/pull/807) Upgrade keys will now prefix the channel path to align with the rest of ICS4 keys
- [\#842](https:/cosmos/ibc/pull/842) Adds metadata field to FungibleTokenPacketData
5 changes: 5 additions & 0 deletions spec/app/ics-020-fungible-token-transfer/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@ interface FungibleTokenPacketData {
amount: uint256
sender: string
receiver: string
metadata: bytes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be nice to have some reasoning for using bytes instead of string given that I think historically string is used over bytes. This is the only difference I can find between the two:

As described, a string must use UTF-8 character encoding. A string cannot exceed 2GB.

As described, bytes can store custom data types, up to 2GB in size.

source

Do we anticipate any not UTF-8 character encodings to cause problems? I think we already account for this in ibc-go for Any's and packet data - but probably worth noting for other implementations as this is a subtle issue

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so because the metadata is completely used outside of transfer application, any middleware or external user that uses metadata and expects UTF-8 encoding can enforce it on their end.

Otoh, since we are allowing arbitrary data to be parsed on the level above transfer, I don't want to make arbitrary limitations in the transfer level and block a potential usecase

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In interchain accounts we have a string memo in the InterchainAccountPacketData. Would it make sense to rename it also to metadata and make it bytes?

}
```

Note: Since earlier versions of this specification did not include a `metadata` field, implementations must ensure that the new packet data is still compatible with chains that expect the old packet data. A legacy implementation MUST be able to unmarshal a new packet data with a `nil` metadata into the legacy `FungibleTokenPacketData` struct. Similarly, an implementation supporting `metadata` must be able to unmarshal a legacy packet data into the current struct with the metadata field set to `nil`.

The `metadata` field is not used within transfer, however it may be used either for external off-chain users (i.e. a memo) or for middleware wrapping transfer that can parse and execute custom logic on the basis of the passed in metadata. Chains should ensure that there is some length limit on the entire packet data to ensure that the packet does not become a DOS vector. However, these do not need to be protocol-defined limits. If the receiver cannot accept a packet because of length limitations, this will lead to a timeout on the sender side.

As tokens are sent across chains using the ICS 20 protocol, they begin to accrue a record of channels for which they have been transferred across. This information is encoded into the `denom` field.

The ics20 token denominations are represented the form `{ics20Port}/{ics20Channel}/{denom}`, where `ics20Port` and `ics20Channel` are an ics20 port and channel on the current chain for which the funds exist. The prefixed port and channel pair indicate which channel the funds were previously sent through. If `{denom}` contains `/`, then it must also be in the ics20 form which indicates that this token has a multi-hop record. Note that this requires that the `/` (slash character) is prohibited in non-IBC token denomination names.
Expand Down