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

Improve the proto-compiler to generate canonical rust files #6

Closed
2 of 7 tasks
hu55a1n1 opened this issue Sep 21, 2021 · 3 comments
Closed
2 of 7 tasks

Improve the proto-compiler to generate canonical rust files #6

hu55a1n1 opened this issue Sep 21, 2021 · 3 comments

Comments

@hu55a1n1
Copy link
Contributor

hu55a1n1 commented Sep 21, 2021

Crate

ibc-proto & ibc-proto-compiler

Summary

Improve the proto-compiler to serve as a means for generating canonical Rust files by compiling SDK & IBC proto files.

Problem Definition

Proposal

Here's one possible solution to address these issues:

  • ibc-proto-compiler could use a rudimentary patch management system that automatically patches the generated files based on predefined rules. This should be relatively straightforward to implement using git and patch files, see quilt for a similar tool that is used in embedded linux build systems. (Update to tonic 0.9 and automatically patch the generated protos #78)
  • ibc-proto must contain generated server code.

Acceptance Criteria

ibc-proto files are usable by other ecosystem projects.


For Admin Use

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

adizere commented Oct 11, 2021

  • The rust files generated using SDK and IBC protos are not usable (i.e. they don't compile) out-of-the box and require a fix, see here.

This problem is now tracked in the SDK cosmos/cosmos-sdk#10323

@adizere
Copy link
Contributor

adizere commented Oct 15, 2021

One more serious problem we're seeing is that the output of proto compiler (i.e., the Rust-generated files) is not deterministic. Soares first spotted this problem while working on no_std. Mentioned in informalsystems/hermes#1439

Add ci/sync-protobuf.sh script to reliably re-generate the protobuf Rust definitions. Note however that the current protobuf-compiler is non-reproducible, so multiple calls to ci/sync-protobuf.sh may yield different results.

This is a problem because it makes reviewing difficult. The diff contains many changes, but most of the diff changes are irrelevant, spawning from re-ordering of types in the generated Rust files. For an example see this.

We don't have a solution to this yet. One idea suggested was that the problem is caused by tonic or prost using HashSet internally, which has non-deterministic ordering of elements, so the order of enums/structs/fns in the proto-compiler output is non-deterministic; the solution would be to replace the HashSet with something else. (@soareschen)

@romac
Copy link
Member

romac commented Aug 17, 2023

This has been fixed in #78

@romac romac closed this as completed Aug 17, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Closed in IBC-rs: the road to v1 Aug 17, 2023
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

No branches or pull requests

3 participants