tendermint-proto: (re-)add grpc feature and server definitions
#1227
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1227 +/- ##
=======================================
- Coverage 64.2% 64.1% -0.2%
=======================================
Files 244 244
Lines 21364 21411 +47
=======================================
- Hits 13734 13730 -4
- Misses 7630 7681 +51
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
tony-iqlusion
force-pushed
the
tendermint-proto/grpc
branch
from
474b1bc
to
8aa7ecc
Compare
tony-iqlusion
changed the title
grpc feature and server definitionsgrpc feature and server definitions
Adds a "grpc" feature to the crate along with gRPC server definitions for the "privval" interface used by e.g. TMKMS to support remote signing. These were originally added in #1137 but lost in the shuffle of Tendermint v0.35-related changes. They have been updated to use `tonic` v0.8, which the `tendermint-proto` crate updated to after the original #1137 PR landed.
tony-iqlusion
force-pushed
the
tendermint-proto/grpc
branch
from
8aa7ecc
to
bf0f890
Compare
There was a problem hiding this comment.
The current main seems to be based on v0.34.x: https://github.com/informalsystems/tendermint-rs/blob/main/proto/src/tendermint.rs#L65 which doesn't have the gRPC privval support (those service definitions don't exist here: https://github.com/tendermint/tendermint/blob/v0.34.23/proto/tendermint/privval/types.proto).
I'm not sure whether there's a plan to backport it to 0.34 and have a release with it.
|
Oh wow, so it is. That's unfortunate |
|
@thanethomson any idea if/when the gRPC privval interface will get re-added upstream in Tendermint? |
It's in our prioritized backlog. We have a bunch of burning, high-priority issues to deal with this quarter (see this board), so those things take precedence. As we have capacity we pick things off the prioritized backlog to work on. |
|
Aha cool, here's the relevant issue: tendermint/tendermint#9256 |
| @@ -1,6 +1,6 @@ | |||
| //! tendermint-proto library gives the developer access to the Tendermint proto-defined structs. | |||
|
|
|||
| #![no_std] | |||
| #![cfg_attr(not(feature = "grpc"), no_std)] | |||
There was a problem hiding this comment.
This effect could be made explicitly seen also in Cargo.toml by making this conditional on the "std" feature, which "grpc" would depend on. In practice though, "grpc" is the only feature that requires std for now.
| .build_client(false) | ||
| .build_server(true) |
There was a problem hiding this comment.
What if we want to enable client-side gRPC bindings later, and feature-gate client and server sides independently? I think it's possible to introduce "grpc-server" and "grpc-client" features later, with "grpc" dependent on both, so this would be backward compatible.
|
If this is still needed, can you rebase it and fix the conflicts? |
|
@mzabaluev it's blocked on tendermint/tendermint#9256 (which I now see was closed as stale). I would definitely still love to see all of this happen, if possible though. |
|
Should this be closed in favor of #1338? |
|
Sure |
See #1225.
Adds a "grpc" feature to the crate along with gRPC server definitions for the "privval" interface used by e.g. TMKMS to support remote signing.
These were originally added in #1137 by @tomtau but lost in the shuffle of Tendermint v0.35-related changes.
They have been updated to use
tonicv0.8, which thetendermint-protocrate updated to after the original #1137 PR landed..changelog/