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

Switch to buf to generate code in cometbft-proto #3

Open
1 task
mzabaluev opened this issue Dec 7, 2023 · 5 comments
Open
1 task

Switch to buf to generate code in cometbft-proto #3

mzabaluev opened this issue Dec 7, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@mzabaluev
Copy link
Contributor

mzabaluev commented Dec 7, 2023

Pre-requisites

Description

The CometBFT protobuf files, complete with versioned definition variants corresponding to past releases, are published on Buf Schema Registry as per cometbft/cometbft#1733.
The code generation method for protobuf bindings in cometbft-proto can now use the buf tooling, instead of the current cumbersome method of checking out the CometBFT repository and running tonic-build for multiple successive revisions.

Definition of "done"

The method of compiling protos is switched to using a buf plugin for tonic, tentatively neoenstein-tonic.
The hierarchy of generated modules in cometbft-proto is changed to match the versioned proto packages in buf.build/cometbft/cometbft.

@mzabaluev mzabaluev added the enhancement New feature or request label Dec 7, 2023
@mzabaluev mzabaluev self-assigned this Dec 7, 2023
@adizere adizere added this to CometBFT Jan 11, 2024
@github-project-automation github-project-automation bot moved this to Todo in CometBFT Jan 11, 2024
@adizere adizere added this to the 2024-Q1 milestone Jan 11, 2024
@mzabaluev
Copy link
Contributor Author

mzabaluev commented Jan 19, 2024

The doc for the neoenstein-prost-serde provides an approach to generate serde implementations for all structures of interest. I think this is the answer to our proto-types woes.

@mzabaluev
Copy link
Contributor Author

Switching to neoenstein-prost-serde to generate the bulk of serde impls is not possible now due to neoeinstein/protoc-gen-prost#84. We need at least a few custom impls (e.g. for PublicKey), for which the generation needs to be suppressed.

An alternative is not using the serde plugin and carrying over the serde attribute options that are used by proto-compiler.

@SuperFluffy
Copy link

SuperFluffy commented Jan 30, 2024

At https://github.com/astriaorg/astria we started out with buf + protoc-gen-prost. We ultimately dropped protoc-gen-prost because we wanted to migrate to protoc 0.12 (and tonic 0.10) which protoc-gen-prost still does not support.

We still use buf (among linting and pushing to the buf repo) for generating the Rust code via buf build --as-file-descriptor-set, which is then fed into tonic-build. We find this approach very ergonomic.

You can see the file descriptor set being generated here:
https://github.com/astriaorg/astria/blob/093e5794a6b66c7e163442127170263d4e2b2cf9/tools/protobuf-compiler/src/main.rs#L37-L41

tonic-build is then configured to use that descriptor set and skip the protoc step here:
https://github.com/astriaorg/astria/blob/093e5794a6b66c7e163442127170263d4e2b2cf9/tools/protobuf-compiler/src/main.rs#L76-L77

We enforce sync between our local proto spec and the generated Rust code through a github action here (the just command is just calling the linked protobuf compiler tool, cargo run --manifest-path tools/protobuf-compiler/Cargo.toml):
https://github.com/astriaorg/astria/blob/093e5794a6b66c7e163442127170263d4e2b2cf9/.github/workflows/test.yml#L34-L44

@mzabaluev
Copy link
Contributor Author

@SuperFluffy

We still use buf (among linting and pushing to the buf repo) for generating the Rust code via buf build --as-file-descriptor-set, which is then fed into tonic-build. We find this approach very ergonomic.

Thanks, good to know!
I see you don't generate serde impls on your proto structs, which add a lot of customization to our proto compilation process.

@mzabaluev
Copy link
Contributor Author

Filed #16 to capture the architectural aspects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

3 participants