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

Duplicate protos between ibc-rs and tendermint-rs #284

Closed
1 of 4 tasks
ancazamfir opened this issue Oct 5, 2020 · 3 comments · Fixed by #292
Closed
1 of 4 tasks

Duplicate protos between ibc-rs and tendermint-rs #284

ancazamfir opened this issue Oct 5, 2020 · 3 comments · Fixed by #292
Assignees
Milestone

Comments

@ancazamfir
Copy link
Collaborator

Summary of Bug

Some IBC proto files (e.g. ibc.tendermint.rs::Header) need to reference some other proto files and structures (e.g. tendermint.types.rs::SignedHeader or tendermint.types.rs::ValidatorSet) that belong to tendermint-rs where we expect the conversions between raw and domain types are implemented.
Presently the ibc-proto files point to duplicate tendermint files in ibc-proto, in particular tendermint.types.rs for the examples above. But I believe all tendermint files in IBC proto are dups of the ones in tendermint-rs

Version

master

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir
Copy link
Collaborator Author

This is blocking progress for #277 where I get errors like:

^^^^ the trait std::convert::From<ibc_proto::tendermint::types::SignedHeader> is not implemented for tendermint::block::signed_header::SignedHeader

I would expect at most something like:
^^^^ the trait std::convert::From<tendermint_proto::types::SignedHeader> is not implemented for tendermint::block::signed_header::SignedHeader
if the conversion is not implemented yet in tendermint-rs.

@greg-szabo
Copy link
Member

greg-szabo commented Oct 7, 2020

The problem is that the Cosmos SDK is copying Tendermint messages from the Tendermint repository's protobuf message folder. Here's a note claiming that: https://github.com/tendermint/tendermint/blob/10f30f8a995d058a76f5f2c5b1ec809f58e8464a/proto/tendermint/abci/types.proto#L15
Here's an example:
https://github.com/cosmos/cosmos-sdk/blob/master/third_party/proto/tendermint/abci/types.proto
https://github.com/tendermint/tendermint/blob/master/proto/tendermint/abci/types.proto
(The two files are the same.)

Because we compile the messages separately in Prost, the IBC Prost compiler doesn't know about a separate set of messages in the Tendermint repository and compiles them as it sees them.

We have to figure out if we can merge the two protobuf message folders together either at the source or during compilation into Rust structs. Or as a third option, somehow separate the Tendermint-related messages in the Cosmos SDK.
I'm not sure, but the copy of Tendermint messages is definitely problematic.

@greg-szabo
Copy link
Member

#292 solves the problem but it requires a rebuild of the IBC protobuf files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants