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

ibc-proto planning issue #13

Closed
5 tasks
greg-szabo opened this issue Nov 27, 2020 · 11 comments
Closed
5 tasks

ibc-proto planning issue #13

greg-szabo opened this issue Nov 27, 2020 · 11 comments
Labels
question Further information is requested

Comments

@greg-szabo
Copy link
Member

Crate

ibc-proto

Summary

New requests came in to enrich the ibc-proto crate with cosmos-SDK related protobuf structs or rename it or separate it.

This issue tries to coordinate among these requests to find out what would be the best possible future for the ibc-proto crate.

Problem Definition

ibc-proto only implements a subset of the Cosmos SDK protobuf definitions. Notably, the ones for IBC.

It would be nice to have one go-to protobuf crate for Cosmos SDK protobuf structs in Rust. But that might require its own repository (as well as better version control as other issues already point that out).

Proposal

(Will be updated based on the discussion.)


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@greg-szabo
Copy link
Member Author

greg-szabo commented Nov 27, 2020

How does this sound like:
a separate cosmos-sdk-proto crate that encompasses all the structs from cosmos/cosmos-sdk. Version control and recompilation options would be kept in the (separate from ibc-rs) repository too. (proto-compiler or similar)

@jkilpatr
Copy link

ibc-proto as it exists includes some Cosmos protos that are more generic, BaseAccount for example. Obviously it's not complete in that aspect.

I like the pattern in ibc-proto and tendermint-proto and I've talked with the ICF about moving them upstream and gotten the unimaginatively named https://github.com/cosmos/rust-monorepo repo for the purpose.

Right now I think just moving the proto definitions there, making them more complete, and creating a definitive location for the maintenance of proto data would be a big win. We can work on having a standard client library and other such functionality later.

@CharlyCst
Copy link

I was about to open an issue for a feature request, but it seems that it would be more appropriate to post it here:
I'm building a mock node for testing purpose and I rely on ibc-proto for proto definitions, however proto-compiler only builds client-related definitions (see this line). For now I simply forked the crate but I would very much appreciate having all the definitions in a single place.

I'm not very familiar with ibc-proto and the needs of the cosmos ecosystem in general, but if someone is willing to provide some guidance I can help on moving thins forward.

@adizere
Copy link
Contributor

adizere commented Jan 28, 2021

Also FYI there is a proto/raw type lingering around in the modules: informalsystems/hermes#565 (comment)

@adizere
Copy link
Contributor

adizere commented Feb 1, 2022

The need for a canonical set of proto files for IBC (and separately for Cosmos) still exists. But the ecosystem-level dependencies have not stabilized and it's not clear if this issue should be tracked here.

@adizere adizere closed this as completed Feb 1, 2022
@mzabaluev
Copy link
Contributor

Revisited this with an attempt to use cosmos-sdk-proto 0.12.0 replacing the bulk of ibc_proto.

The immediate problems I have encountered are:

  • ibc_proto derives serde, JSON schema, and other traits in addition to the default code for many of the types, which cosmos_sdk_proto does not.
  • Some types that are generated for ibc_proto are missing from cosmos_sdk_proto, e.g. cosmos::auth::v1beta1::query_client::QueryClient.

It appears that for ibc-rs purposes, generating homespun Rust code in the ibc_proto crate is more viable, at least in the near term. To generate from cosmos-sdk 0.46.x proto files, I'll try to make use of buf and protoc-gen-prost to avoid replicating the dependency management.

@tarcieri
Copy link

tarcieri commented May 4, 2022

ibc_proto derives serde, JSON schema, and other traits in addition to the default code for many of the types, which cosmos_sdk_proto does not.

We’ve been looking for upstream guidance from tendermint-rs. Things are a mess of incompatibilities right now, for ibc-rs as well:

informalsystems/tendermint-rs#1053

Some types that are generated for ibc_proto are missing from cosmos_sdk_proto, e.g. cosmos::auth::v1beta1::query_client::QueryClient.

This looks like a regression in the latest v0.12 release, likely owing to the prostv0.10 and tonic v0.7 upgrades. It was available in the v0.11 release:

https://docs.rs/cosmos-sdk-proto/0.11.0/cosmos_sdk_proto/cosmos/auth/v1beta1/query_client/index.html

Please open tracking issues for these things as you discover them on https://github.com/cosmos/cosmos-rust

@mzabaluev
Copy link
Contributor

@tarcieri Is there any plan on dealing with serde and other derives? The code generation settings in ibc-proto do a great deal to customize human-readable serialization for the proto-derived data structures, and I guess these serialization settings are applied only within the relayer.

As to other customizations, I don't currently know the purpose of deriving the JSON schema impls, and I suspect we can live without the Eq impls (prost-build always derives PartialEq which should be enough for practical purposes).

@tarcieri
Copy link

tarcieri commented May 5, 2022

@mzabaluev there are a few open issues on that. I linked informalsystems/tendermint-rs#1053 because it discusses serde serialization of Protobuf WKTs and suggests prost-wkt as a possible solution.

There's also this older, more general issue: informalsystems/tendermint-rs#654

Definitely open to solutions in this area, and if the issue isn't going to be addressed upstream in tendermint-rs soon, perhaps we can look at doing something in the meantime: cosmos/cosmos-rust#83

@tarcieri
Copy link

tarcieri commented May 5, 2022

I made a missing auth::v1beta1::query_client::QueryClient: cosmos/cosmos-rust#219

Not sure what happened there as the other gRPC clients are still present.

@tony-iqlusion
Copy link
Member

cosmos/cosmos-rust#219 was indeed a bug in cosmos-sdk-proto: there were protobufs that were clashing between cosmos-sdk and ibc.

This has been corrected in the v0.12.1 release, which now once again has a auth::v1beta1::query_client::QueryClient.

@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants