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

Support for not emitting unpopulated fields when proto json marshaling #13727

Open
colin-axner opened this issue Nov 2, 2022 · 1 comment
Open
Labels
C: Proto Proto definition and proto release

Comments

@colin-axner
Copy link
Contributor

Summary

When adding new fields to IBC packet data, we would like the ability to not emit (marshal) unpopulated fields. This would allow for new fields to be added without requiring a full network upgrade.

Problem Definition

Currently the ProtoMarshalJSON function sets EmitDefaults to true which will emit unpopulated fields and thus cause unmarshal errors on the packet's receiving side.

Proposal

We need this now, so we have decided to fork this function solely for marshaling ics20 packets.

We anticipate the need for this again and would appreciate the flexibility of controlling the EmitDefaults value. I could see a number of possible solutions to this issue (potentially utilizing the proto codec to indicate the EmitDefaults value). It appears the current justification for setting EmitDefaults to true is for cli purposes, so maybe it should be clear when a proto codec is used for cli vs the state machine.

I don't see any security considerations with regards to setting EmitDefaults to false, as it should marshal less information, but it will be state machine breaking.

Since we have decided to make the change on our end, there is no immediate demand for this feature, but it would be greatly appreciated so we could remove our forked code 🙏

@alexanderbez
Copy link
Contributor

alexanderbez commented Nov 2, 2022

Spoke with @aaronc and he stated there is no harm in making EmitDefaults configurable 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Proto Proto definition and proto release
Projects
None yet
Development

No branches or pull requests

3 participants