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

ICS27: Define De/Serialization format #634

Open
Tracked by #987
ethanfrey opened this issue Jan 4, 2022 · 1 comment
Open
Tracked by #987

ICS27: Define De/Serialization format #634

ethanfrey opened this issue Jan 4, 2022 · 1 comment
Labels
app Application layer.

Comments

@ethanfrey
Copy link
Contributor

I read that there is one type EXECUTE_TX, which is fine.

And when received, we should parse the bytes into multiple messages: https://github.com/cosmos/ibc/tree/master/spec/app/ics-027-interchain-accounts#packet-relay

However, the serialization format is never defined in the spec. This make building a compatible implementation impossible. Please define this format explicitly here: https://github.com/cosmos/ibc/tree/master/spec/app/ics-027-interchain-accounts#packet-data

Also, I must admit that seeing go code lightly masquerading as the typescript spec code (just changed the name from func to function otherwise Go syntax) is a bit surprising.

@ethanfrey ethanfrey changed the title Define De/Serialization format for EXECUTE_TX ics27: Define De/Serialization format Jan 4, 2022
@mpoke mpoke changed the title ics27: Define De/Serialization format ICS27: Define De/Serialization format Mar 17, 2022
@mpoke mpoke added the app Application layer. label Mar 17, 2022
@colin-axner
Copy link
Contributor

Great suggestion, sorry we haven't addressed it yet.

In ibc-go we currently use proto3 json which results in a json string like such:

{"type":"TYPE_EXECUTE_TX","data":"ZGF0YQ==","memo":"memo"}

We could switch to json which would look like (the "type" encoding changes):

{"type":1,"data":"ZGF0YQ==","memo":"memo"}

If we made the switch, we would continue using proto3 json to unmarshal because the standard json library would fail to unmarshal the proto3 json encoding (which may be sent by a chain on an existing version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app Application layer.
Projects
None yet
Development

No branches or pull requests

3 participants