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

Introduce Metadata field in ICS20 Packet data proto #2411

Closed
AdityaSripal opened this issue Sep 27, 2022 · 5 comments · Fixed by #2305
Closed

Introduce Metadata field in ICS20 Packet data proto #2411

AdityaSripal opened this issue Sep 27, 2022 · 5 comments · Fixed by #2305
Assignees
Milestone

Comments

@AdityaSripal
Copy link
Member

AdityaSripal commented Sep 27, 2022

Introduce Metadata field in ICS20 Packet Data proto. This should be done with new field number so that it is backwards compatible

@crodriguezvega crodriguezvega added this to the v6.0.0 milestone Sep 27, 2022
@alizahidraja
Copy link
Contributor

I can take this up

@alizahidraja
Copy link
Contributor

Lmk the versions for protoc & protoc-gen-go to generate updated pb.go

@colin-axner
Copy link
Contributor

colin-axner commented Sep 28, 2022

Some context:

Typically changing the packet data changes the packet commitment hash. This is actually not the primary concern of this change since the receiving chain will verify that the data bytes provided by the relayer were committed to by the sending chain. The concern is the receiving chains ability to decode the packet data. In this situation, a receiving chain without the memo field in the ICS20 packet data will fail to unmarshal the packet data and return an acknowledgement error. If the sending chain does not support the new field, it will also return an error if a user attempts to provide a MsgTransfer with the field included.

Protobuf will not encode fields if they are empty. Thus transfers to chains without the new packet data field can continue to successfully occur so long as the memo field is left empty

@nicolaslara
Copy link
Contributor

is there any reason to call it Memo over Metadata?

@AdityaSripal AdityaSripal changed the title Introduce Memo field in ICS20 Packet data proto Introduce Metadata field in ICS20 Packet data proto Sep 29, 2022
@crodriguezvega crodriguezvega moved this to In progress in ibc-go Oct 3, 2022
@crodriguezvega
Copy link
Contributor

is there any reason to call it Memo over Metadata?

PR is open in spec repo and calls it metadata.

Repository owner moved this from In progress to Done in ibc-go Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment