search

cometbft / cometbft-rs

Rust client framework for interacting with CometBFT
Apache License 2.0
8 stars 9 forks source link

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

Open mzabaluev opened 11 months ago

mzabaluev commented 11 months ago

Pre-requisites

Description

The CometBFT protobuf files, complete with versioned definition variants corresponding to past releases, are published on Buf Schema Registry as per https://github.com/cometbft/cometbft/issues/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 commented 10 months ago

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 commented 10 months ago

Switching to neoenstein-prost-serde to generate the bulk of serde impls is not possible now due to https://github.com/neoeinstein/protoc-gen-prost/issues/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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

Filed https://github.com/cometbft/cometbft-rs/pull/16 to capture the architectural aspects.