fix(ics-23): Auto-derive serde::Serialize for CommitmentPrefix

[seanchen1991]

Closes: #1229

Description

---

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

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 67.33%. Comparing base (86a0ee0) to head (c3cbca4).

Files	Patch %	Lines
ibc-core/ics23-commitment/types/src/commitment.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1230      +/-   ##
==========================================
+ Coverage   67.31%   67.33%   +0.01%     
==========================================
  Files         234      234              
  Lines       23553    23547       -6     
==========================================
  Hits        15855    15855              
+ Misses       7698     7692       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[soareschen]

I think this is a reasonable fix. However, we may want to consider whether it is fine for breaking compatibility in this case. Let's wait for @rnbguy and @romac to review and discuss the changes.

[romac]

I think that's fine, better to break compatibility now and have consistent serialization than the reverse.

[soareschen]

Sounds good. @seanchen1991 let's add a changelog to indicate the breaking change?

[seanchen1991]

@soareschen, you mentioned that you opened a fork that manually implements deserialization from String. Would that route be preferable than opting to auto-derive Serialize?

[seanchen1991]

Oh, I just saw this comment, which makes me think we should just opt to auto-derive Serialize instead of manually implement Deserialize.

[soareschen]

Yes, the custom fix is at https://github.com/cosmos/ibc-rs/tree/soares/fix-commitment-prefix-deserialize. But we should serialize and deserialize it as bytes, as the current behavior for serializing non-UTF8 commitment prefix is incorrect.