Skip to content

feat: Add ProtoJSON-compatible Serialize and Deserialize instances on all Protobuf definitions via pbjson #146

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Apr 26, 2023

Conversation

romac
Copy link
Contributor

@romac romac commented Apr 22, 2023

Closes: #144
Supersedes: #145

@romac romac changed the title Derive Serialize and Deserialize on all Protobuf definitions via pbjson Add ProtoJSON-compatible Serialize and Deserialize on all Protobuf definitions via pbjson Apr 22, 2023
@romac romac changed the title Add ProtoJSON-compatible Serialize and Deserialize on all Protobuf definitions via pbjson rust: Add ProtoJSON-compatible Serialize and Deserialize instances on all Protobuf definitions via pbjson Apr 22, 2023
@romac romac changed the title rust: Add ProtoJSON-compatible Serialize and Deserialize instances on all Protobuf definitions via pbjson feat: Add ProtoJSON-compatible Serialize and Deserialize instances on all Protobuf definitions via pbjson Apr 22, 2023
@romac romac added the rust Issues pertaining to the Rust implementation label Apr 22, 2023
@codecov
Copy link

codecov bot commented Apr 22, 2023

Codecov Report

Patch coverage: 41.09% and project coverage change: +14.72 🎉

Comparison is base (d28ea7a) 50.54% compared to head (a5d2bde) 65.27%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #146       +/-   ##
===========================================
+ Coverage   50.54%   65.27%   +14.72%     
===========================================
  Files          23        8       -15     
  Lines        8034     3551     -4483     
  Branches       86        0       -86     
===========================================
- Hits         4061     2318     -1743     
+ Misses       3616     1233     -2383     
+ Partials      357        0      -357     
Flag Coverage Δ
go ?
rust 65.27% <41.09%> (-26.88%) ⬇️
typescript ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rust/src/lib.rs 100.00% <ø> (ø)
rust/src/cosmos.ics23.v1.serde.rs 40.32% <40.32%> (ø)
rust/src/api.rs 97.21% <100.00%> (+0.10%) ⬆️

... and 16 files with indirect coverage changes

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.

Copy link

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be great, and would unblock us on Penumbra.

@tac0turtle tac0turtle merged commit 58722c1 into master Apr 26, 2023
@tac0turtle tac0turtle deleted the romac/pbjson branch April 26, 2023 08:01
romac pushed a commit that referenced this pull request Aug 16, 2023
Closes: #156

Addresses the `no_std` compatibility issue with `serde` feature in `ibc-rs`. This is caused by the recent implementation of ProtoJSON serialization and deserialization [0] for the `ics23` Protobuf definitions using `pbjson`, and then re-exporting the ics23 type [1] in `ibc-proto-rs`. Some of our users by then (starting from IBC-rs v0.41.0) are experiencing compilation errors. [2]

To meet this immediate need [3] and the lack of activity in the `pbjson` crate for months, we have taken the initiative to feature `no_std` support in the `informalsystems-pbjson` crate and have it published.

[0] #146
[1] cosmos/ibc-proto-rs#92
[2] informalsystems/ibc-rs#741
[3] cosmos/ibc-proto-rs#98 (comment)

---

* feat: enable no_std support for pbjson

* fix: get serde feature work with no-std

* deps: use informalsystems-pbjson v0.6.0

* deps: use informalsystems-pbjson v0.6.0
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
rust Issues pertaining to the Rust implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rust: Add serde feature to derive Serialize and Deserialize on all Protobuf definitions
3 participants