ics26_routing: Impl Protobuf<Any> for MsgEnvelope

[kevinji]

Closes: #XXX

Description

Implement Protobuf<Any> for MsgEnvelope so that serialization of MsgEnvelope types is possible. This matches the already-existing TryFrom<Any> implementation.

Using msg.encode_vec().unwrap() is safe because .encode_vec() will never return an error, as the buffer will always be constructed to be the correct size. I will submit a follow-up PR to ibc-proto to update the return type of encode_vec().

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests.
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).
• Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• Reviewed Files changed in the GitHub PR explorer.
• Manually tested (in case integration/unit/mock tests are absent).

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.10 ⚠️

Comparison is base (20d682f) 72.85% compared to head (27672ba) 72.76%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #586      +/-   ##
==========================================
- Coverage   72.85%   72.76%   -0.10%     
==========================================
  Files         126      126              
  Lines       15705    15725      +20     
==========================================
  Hits        11442    11442              
- Misses       4263     4283      +20     

Impacted Files	Coverage Δ
crates/ibc/src/core/ics26_routing/msgs.rs	0.95% <0.00%> (-0.23%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kevinji]

Reference to upstream PR: cosmos/ibc-proto-rs#73

[Farhad-Shabani]

Thanks for this improvement suggestion.

Note that since each message type provides a to_any method, we can extract the domain message from MsgEnvelop and apply domain_msg.to_any()

However, from what I've seen, the typical conversion flow to work with the IBC-rs implementation is from Any to MsgEnvelop, and the reverse direction may be rarely used which makes me to think there is no need to retain it in the IBC-rs boundary. Nonetheless, the follow-up PR to ibc-proto may still work and if your project requires that conversion, please let us know what it is used for. we are happy to help.

[Farhad-Shabani]

And a quick request. I’d appreciate for such cases if you open a GitHub issue in advance, so we could keep our changelog and plannings consistent. This helps us to search, track and prioritize our issues in a more efficient way. 🙏🏻 🌷

[kevinji]

Will do regarding issues! To respond to the use case: I need to serialize MsgEnvelope within my code so I can create a client that passes them to our IBC handler, so I thought that this would be the easiest way to do so. If you would prefer not to include this serialization I can just copy-paste what I have here directly into my code.

[Farhad-Shabani]

Will do regarding issues! To respond to the use case: I need to serialize MsgEnvelope within my code so I can create a client that passes them to our IBC handler, so I thought that this would be the easiest way to do so. If you would prefer not to include this serialization I can just copy-paste what I have here directly into my code.

In fact, this comment applies here as well. Since we are going towards making our msg structs private (issue #324), we'd prefer only to maintain necessary parts/conversions needed to work with the IBC-rs implementation.

[kevinji]

I think this should be fine as long as there's some way to call dispatch after the change to make things private happens. Is the idea to depend solely on the ibc-proto-generated raw message types then?

[plafer]

Is the idea to depend solely on the ibc-proto-generated raw message types then?

That's right. We need to make changes to the domain types from time to time, which is a breaking change every time (which won't work when we're on version 1.x).

[kevinji]

Got it, in that case I'll close this PR and await the API changes once the msg structs are private.