Replace tools/proto-compiler with buf gen and protoc-gen-prost
#2184
Comments
|
"This is going to bite us eventually, so the sooner we do it, the better." The way it's going to bite us is that eventually we might have a divergence in parts of the protos that we actually do use, because our vendored go protos are not kept up to date, but buf does dependency management. So, then we would have a problem. |
|
Picking this up. Given the recent crate reorg, our plan is currently:
First attempt in branch |
|
Updated the feature branch to use generated protos for go and rust, entirely via buf/prost plugins. All hardcoded vendored protos have been removed. The major regression right now is that gRPC server reflection is broken. Haven't yet figured out how to cobble together the file descriptor info and expose that to tonic during service construction, but it seems to be possible. Will try on that a bit longer. |
|
Sick! I think if we can't get reflection working via the same proto descriptor file approach as before it would be an OK regression, at least temporarily — and maybe we could upstream changes required to restore it later. I don't think anything is currently critically reliant on it, it's just a nice to have. |
Overhauls the protobuf codegen by standardizing on `buf`,
along with associated tooling like `protoc-gen-prost` and
`proto-gen-tonic`. The output dirs for the supported langs are now:
* Rust: ./crates/proto/src/gen/
* Go: ./proto/go/gen/
We've removed the proto-compiler tool from the repo. Also, we no longer
vendor upstream protos by committing directly to our repo; instead,
we declare dependencies in the buf yaml files and offload the resolution
to `buf generate`.
One major regression is that gRPC server reflection no longer works.
These changes remove the `proto_descriptor.bin` build.rs logic, with no
replacement in the new buf logic. As of right now, preview supports
reflection:
❯ grpcurl grpc.testnet-preview.penumbra.zone:443 list
cosmos.base.tendermint.v1beta1.Service
grpc.reflection.v1alpha.ServerReflection
penumbra.client.v1alpha1.ObliviousQueryService
penumbra.client.v1alpha1.SpecificQueryService
penumbra.client.v1alpha1.TendermintProxyService
penumbra.custody.v1alpha1.CustodyProtocolService
penumbra.narsil.ledger.v1alpha1.LedgerService
penumbra.view.v1alpha1.ViewAuthService
penumbra.view.v1alpha1.ViewProtocolService
but this code does not:
❯ grpcurl -plaintext localhost:8080 list
Failed to list services: server does not support the reflection API
We'll circle back on reflection support.
Notably the tonic rpc defs are now in their own files, a side-effect of
using the `prost-gen-tonic` plugin for buf. The original proto files,
without the `.tonic.proto` suffix, now automatically `include!` their
tonic neighbors, so the interface hasn't changed.
Refs #2184.
Overhauls the protobuf codegen by standardizing on `buf`,
along with associated tooling like `protoc-gen-prost` and
`proto-gen-tonic`. The output dirs for the supported langs are now:
* Rust: ./crates/proto/src/gen/
* Go: ./proto/go/gen/
We've removed the proto-compiler tool from the repo. Also, we no longer
vendor upstream protos by committing directly to our repo; instead,
we declare dependencies in the buf yaml files and offload the resolution
to `buf generate`.
One major regression is that gRPC server reflection no longer works.
These changes remove the `proto_descriptor.bin` build.rs logic, with no
replacement in the new buf logic. As of right now, preview supports
reflection:
❯ grpcurl grpc.testnet-preview.penumbra.zone:443 list
cosmos.base.tendermint.v1beta1.Service
grpc.reflection.v1alpha.ServerReflection
penumbra.client.v1alpha1.ObliviousQueryService
penumbra.client.v1alpha1.SpecificQueryService
penumbra.client.v1alpha1.TendermintProxyService
penumbra.custody.v1alpha1.CustodyProtocolService
penumbra.narsil.ledger.v1alpha1.LedgerService
penumbra.view.v1alpha1.ViewAuthService
penumbra.view.v1alpha1.ViewProtocolService
but this code does not:
❯ grpcurl -plaintext localhost:8080 list
Failed to list services: server does not support the reflection API
We'll circle back on reflection support.
Notably the tonic rpc defs are now in their own files, a side-effect of
using the `prost-gen-tonic` plugin for buf. The original proto files,
without the `.tonic.proto` suffix, now automatically `include!` their
tonic neighbors, so the interface hasn't changed.
Refs #2184.
Overhauls the protobuf codegen by standardizing on `buf`,
along with associated tooling like `protoc-gen-prost` and
`proto-gen-tonic`. The output dirs for the supported langs are now:
* Rust: ./crates/proto/src/gen/
* Go: ./proto/go/gen/
We've removed the proto-compiler tool from the repo. Also, we no longer
vendor upstream protos by committing directly to our repo; instead,
we declare dependencies in the buf yaml files and offload the resolution
to `buf generate`.
One major regression is that gRPC server reflection no longer works.
These changes remove the `proto_descriptor.bin` build.rs logic, with no
replacement in the new buf logic. As of right now, preview supports
reflection:
❯ grpcurl grpc.testnet-preview.penumbra.zone:443 list
cosmos.base.tendermint.v1beta1.Service
grpc.reflection.v1alpha.ServerReflection
penumbra.client.v1alpha1.ObliviousQueryService
penumbra.client.v1alpha1.SpecificQueryService
penumbra.client.v1alpha1.TendermintProxyService
penumbra.custody.v1alpha1.CustodyProtocolService
penumbra.narsil.ledger.v1alpha1.LedgerService
penumbra.view.v1alpha1.ViewAuthService
penumbra.view.v1alpha1.ViewProtocolService
but this code does not:
❯ grpcurl -plaintext localhost:8080 list
Failed to list services: server does not support the reflection API
We'll circle back on reflection support.
Notably the tonic rpc defs are now in their own files, a side-effect of
using the `prost-gen-tonic` plugin for buf. The original proto files,
without the `.tonic.proto` suffix, now automatically `include!` their
tonic neighbors, so the interface hasn't changed.
Refs #2184.
|
Filed follow-up in #2629, closing. |
github-project-automation
bot
moved this from In Progress
to Testnet 53: Himalia
in Testnets
|
Reopening to try again, since we backed out the incomplete changes in #2639. |
|
Fallback position here is to retain some directories of vendored protos, but standardize on the use of |
Updates the protobuf build process to use `buf` to pull in the relevant upstream proto definitions from BSR. We intentionally pin older versions of the cosmos deps, cosmos-sdk & ibc, for compatibility. The most recent version of cosmos/ibc doesn't integrate with our existing tooling, due to introduction of new "extension" and "extension_signature" attributes on the tendermint::vote type, but we'll tackle updating that later. Only a single change was made to our proto compiler tool: excising a manual import of a tendermint "query.rs" codegen file, which is no longer exported from the sdk protos. Removing it did not break the build, so we weren't using the functionality in that file. The rest of the diff to proto-compiler is updating relpaths. The proto specs now live at a top-level `proto/` dir, and alongside the vendored protos. Those directories are used as inputs to the build. Relevant docs have been updated. Towards #2184.
Updates the protobuf build process to use `buf` to pull in the relevant upstream proto definitions from BSR. We intentionally pin older versions of the cosmos deps, cosmos-sdk & ibc, for compatibility. The most recent version of cosmos/ibc doesn't integrate with our existing tooling, due to introduction of new "extension" and "extension_signature" attributes on the tendermint::vote type, but we'll tackle updating that later. Only a single change was made to our proto compiler tool: excising a manual import of a tendermint "query.rs" codegen file, which is no longer exported from the sdk protos. Removing it did not break the build, so we weren't using the functionality in that file. The rest of the diff to proto-compiler is updating relpaths. The proto specs now live at a top-level `proto/` dir, and alongside the vendored protos. Those directories are used as inputs to the build. Relevant docs have been updated. Towards #2184.
Updates the protobuf build process to use `buf` to pull in the relevant upstream proto definitions from BSR. We intentionally pin older versions of the cosmos deps, cosmos-sdk & ibc, for compatibility. The most recent version of cosmos/ibc doesn't integrate with our existing tooling, due to introduction of new "extension" and "extension_signature" attributes on the tendermint::vote type, but we'll tackle updating that later. Only a single change was made to our proto compiler tool: excising a manual import of a tendermint "query.rs" codegen file, which is no longer exported from the sdk protos. Removing it did not break the build, so we weren't using the functionality in that file. The rest of the diff to proto-compiler is updating relpaths. The proto specs now live at a top-level `proto/` dir, and alongside the vendored protos. Those directories are used as inputs to the build. Relevant docs have been updated. Towards #2184.
|
The primary goal of this ticket is not accomplished—we're still using two different tools to perform codegen—but we've made solid progress in terms of terms of shoring up our codegen and using explicit versions consistently across all targets. For now, I'm moving this ticket back into Next because I'd like to revisit it, but I'm calling the recent gains good enough at present, and will focus on other tasks next sprint. |
|
This is worth revisiting after we move to CometBFT 0.38 and fully support Vote Extensions. Until then, kicking into next. Also, we should consider removing the generate go code from the repo entirely, and allow other people like SL to maintain go codegen where it's needed. |
We originally added the golang codegen to provide first-class support to downstream integrators. This is no longer necessary, as those same integrators, e.g. Strangelove, are performing their own codegen in the respective repos. These codepaths in the Penumbra monorepo are unused and largely unmaintained, so I'm culling them. Refs #2184. Moots and therefore closes #2731.
We originally added the golang codegen to provide first-class support to downstream integrators. This is no longer necessary, as those same integrators, e.g. Strangelove, are performing their own codegen in the respective repos. These codepaths in the Penumbra monorepo are unused and largely unmaintained, so I'm culling them. Refs #2184. Moots and therefore closes #2731.
We originally added the golang codegen to provide first-class support to downstream integrators. This is no longer necessary, as those same integrators, e.g. Strangelove, are performing their own codegen in the respective repos. These codepaths in the Penumbra monorepo are unused and largely unmaintained, so I'm culling them. Refs #2184. Moots and therefore closes #2731.
We originally added the golang codegen to provide first-class support to downstream integrators. This is no longer necessary, as those same integrators, e.g. Strangelove, are performing their own codegen in the respective repos. These codepaths in the Penumbra monorepo are unused and largely unmaintained, so I'm culling them. Refs #2184. Moots and therefore closes #2731.
Is your feature request related to a problem? Please describe.
As described in #1867, we have two different variants of protobuf generation tooling, one using Buf and the other using prost/tonic. That issue described a strategy for trying to hack together
prostand the Buf cache. An almost-certainly better alternative would be to use Buf to generate both kinds of code, and instruct it on how to generate Rust code using https://github.com/neoeinstein/protoc-gen-prost (details on Buf integration here: https://github.com/neoeinstein/protoc-gen-prost/blob/main/protoc-gen-prost-serde/README.md#usage-with-buf )Describe the solution you'd like
tools/proto-compilerwith Buf generateprotoc-gen-prost(replicating our existing configs with the options it provides)protoc-gen-prost-serde(replicating our invocation ofpbjson-build)protoc-gen-tonic(replicating our existing configs, in particular replicating the cfg-module options we use to feature-gate the tonic server code)include!paths where we arrange the generated code into the module tree rather than trying to exactly match what we currently do withtools/proto-compilerThe text was updated successfully, but these errors were encountered: