refactor(proto): deprecate tonic-prost generated gRPC server types (#2263)

Author: kkovaacs

Cargo.lock

@@ -1,5 +1,5 @@
 use anyhow::Context;
-use miden_node_proto::generated::ntx_builder::api_server;
+use miden_node_proto::server::ntx_builder_api;
 use miden_node_proto_build::ntx_builder_api_descriptor;
 use miden_node_utils::panic::{CatchPanicLayer, catch_panic_layer_fn};
 use miden_node_utils::tracing::grpc::grpc_trace_fn;
@@ -32,7 +32,7 @@ impl NtxBuilderRpcServer {
 
     /// Starts the gRPC server on the given listener.
     pub async fn serve(self, listener: TcpListener) -> anyhow::Result<()> {
-        let api_service = api_server::ApiServer::new(self);
+        let api_service = ntx_builder_api::service(self);
         let reflection_service = server::Builder::configure()
             .register_file_descriptor_set(ntx_builder_api_descriptor())
             .build_v1()

bin/ntx-builder/src/server.rs

@@ -1,8 +1,7 @@
 use std::num::NonZeroUsize;
 
 use anyhow::Context;
-use miden_node_proto::generated::remote_prover::api_server::ApiServer;
-use miden_node_proto::generated::remote_prover::worker_status_api_server::WorkerStatusApiServer;
+use miden_node_proto::server::{remote_prover_api, remote_prover_worker_status_api};
 use miden_node_utils::cors::cors_for_grpc_web_layer;
 use miden_node_utils::logging::OpenTelemetry;
 use miden_node_utils::panic::catch_panic_layer_fn;
@@ -74,9 +73,10 @@ impl Server {
             "proof server listening"
         );
 
-        let status_service = WorkerStatusApiServer::new(status::StatusService::new(self.kind));
+        let status_service =
+            remote_prover_worker_status_api::service(status::StatusService::new(self.kind));
         let prover_service = ProverService::with_capacity(self.kind, self.capacity);
-        let prover_service = ApiServer::new(prover_service);
+        let prover_service = remote_prover_api::service(prover_service);
 
         let reflection_service = tonic_reflection::server::Builder::configure()
             .register_file_descriptor_set(miden_node_proto_build::remote_prover_api_descriptor())
@@ -88,7 +88,12 @@ impl Server {
         let (health_reporter, health_service) = tonic_health::server::health_reporter();
 
         // Mark the service as serving
-        health_reporter.set_serving::<ApiServer<ProverService>>().await;
+        health_reporter
+            .set_service_status(
+                remote_prover_api::service_name(),
+                tonic_health::ServingStatus::Serving,
+            )
+            .await;
 
         let server = tonic::transport::Server::builder()
             .accept_http1(true)

bin/remote-prover/src/server/mod.rs

@@ -2,7 +2,7 @@ use std::net::SocketAddr;
 use std::num::NonZeroUsize;
 
 use anyhow::Context;
-use miden_node_proto::generated::validator::api_server;
+use miden_node_proto::server::validator_api;
 use miden_node_proto_build::validator_api_descriptor;
 use miden_node_store::BlockStore;
 use miden_node_utils::clap::GrpcOptionsInternal;
@@ -94,7 +94,7 @@ impl ValidatorServer {
             .layer(CatchPanicLayer::custom(catch_panic_layer_fn))
             .layer(TraceLayer::new_for_grpc().make_span_with(grpc_trace_fn))
             .timeout(self.grpc_options.request_timeout)
-            .add_service(api_server::ApiServer::new(
+            .add_service(validator_api::service(
                 ValidatorService::new(
                     self.signer,
                     db,

bin/validator/src/server/mod.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
-use miden_node_proto::generated::validator::api_server;
 use miden_node_proto::generated::{self as proto};
+use miden_node_proto::server::validator_api;
 use miden_node_store::{BlockStore, GenesisState};
 use miden_node_utils::fee::test_fee_params;
 use miden_protocol::Word;
@@ -50,11 +50,11 @@ impl TestValidator {
     async fn call_sign_block(
         &self,
         proposed_block: &ProposedBlock,
-    ) -> Result<tonic::Response<proto::blockchain::SignBlockResponse>, tonic::Status> {
+    ) -> Result<proto::blockchain::SignBlockResponse, tonic::Status> {
         let request = tonic::Request::new(proto::blockchain::ProposedBlock {
             proposed_block: proposed_block.to_bytes(),
         });
-        api_server::Api::sign_block(&self.server, request).await
+        validator_api::SignBlock::full(&self.server, request).await
     }
 
     /// Opens a block subscription starting from `block_from`.
@@ -64,10 +64,9 @@ impl TestValidator {
     ) -> <ValidatorService as proto::server::validator_api::BlockSubscription>::ItemStream {
         let request =
             tonic::Request::new(proto::validator::BlockSubscriptionRequest { block_from });
-        api_server::Api::block_subscription(&self.server, request)
+        validator_api::BlockSubscription::full(&self.server, request)
             .await
             .expect("subscription should open")
-            .into_inner()
     }
 
     /// Loads the current chain tip from the validator's database.
@@ -163,11 +162,7 @@ async fn sign_block_returns_signed_commitment() {
     let tv = TestValidator::new().await;
 
     let proposed = tv.propose_empty_block();
-    let response = tv
-        .call_sign_block(&proposed)
-        .await
-        .expect("block should be signed")
-        .into_inner();
+    let response = tv.call_sign_block(&proposed).await.expect("block should be signed");
 
     let (header, _) = proposed.into_header_and_body().unwrap();
     let returned: Word = response

bin/validator/src/server/validator_service/tests.rs

@@ -26,6 +26,7 @@ prost                       = { workspace = true }
 thiserror                   = { workspace = true }
 tonic                       = { default-features = true, workspace = true }
 tonic-prost                 = { workspace = true }
+tower                       = { workspace = true }
 url                         = { workspace = true }
 
 [dev-dependencies]
@@ -44,4 +45,10 @@ tonic-prost-build      = { workspace = true }
 [package.metadata.cargo-machete]
 ignored = [
   "tonic-prost", # used in generated OUT_DIR code
+  "tower",       # used in generated OUT_DIR code
+]
+
+[package.metadata.cargo-shear]
+ignored = [
+  "tower", # used in generated OUT_DIR code
 ]

crates/proto/Cargo.toml

@@ -32,7 +32,7 @@ fn main() -> miette::Result<()> {
     ];
 
     for file_descriptors in &descriptor_sets {
-        generate_bindings(file_descriptors.clone(), &dst_dir)?;
+        generate_bindings(file_descriptors, &dst_dir)?;
     }
 
     let server_dst_dir = dst_dir.join("server");
@@ -54,14 +54,19 @@ fn main() -> miette::Result<()> {
 
 /// Generates protobuf bindings from the given file descriptor set and stores them in the given
 /// destination directory.
-fn generate_bindings(file_descriptors: FileDescriptorSet, dst_dir: &Path) -> miette::Result<()> {
+fn generate_bindings(file_descriptors: &FileDescriptorSet, dst_dir: &Path) -> miette::Result<()> {
     let mut prost_config = tonic_prost_build::Config::new();
     prost_config.skip_debug(["AccountId", "Digest"]);
 
     // Generate the stub of the user facing server from its proto file
     tonic_prost_build::configure()
+        .server_mod_attribute(".", "#[allow(deprecated, clippy::mixed_attributes_style)]")
+        .server_attribute(
+            ".",
+            r#"#[deprecated(note = "use the service constructors in `miden_node_proto::server` instead")]"#,
+        )
         .out_dir(dst_dir)
-        .compile_fds_with_config(file_descriptors, prost_config)
+        .compile_fds_with_config(file_descriptors.clone(), prost_config)
         .into_diagnostic()
         .wrap_err("compiling protobufs")?;
 
@@ -216,6 +221,8 @@ impl Service {
     fn generate(&self) -> Module {
         let mut module = Module::new(&self.name);
 
+        module.push_fn(self.service_constructor());
+        module.push_fn(self.service_name());
         module.push_trait(self.service_trait());
         module.push_impl(self.blanket_impl());
         module.push_impl(self.tonic_impl());
@@ -302,12 +309,7 @@ impl Service {
     /// }
     /// ```
     fn tonic_impl(&self) -> Impl {
-        let tonic_path = format!(
-            "crate::generated::{}::{}_server::{}",
-            self.package,
-            to_snake_case(&self.name),
-            self.name
-        );
+        let tonic_path = self.tonic_trait_path();
 
         let mut ret = Impl::new("T");
         ret.generic("T")
@@ -329,6 +331,60 @@ impl Service {
 
         ret
     }
+
+    /// Constructs the underlying tonic server for this service behind an opaque tower service type.
+    fn service_constructor(&self) -> Function {
+        let mut ret = Function::new("service");
+        ret.vis("pub")
+            .attr("allow(deprecated)")
+            .generic("T")
+            .arg("service", "T")
+            .ret(
+                "impl tower::Service<
+    http::Request<tonic::body::Body>,
+    Response = http::Response<tonic::body::Body>,
+    Error = std::convert::Infallible,
+    Future: Send + 'static,
+> + tonic::server::NamedService
+  + Clone
+  + Send
+  + Sync
+  + 'static",
+            )
+            .bound("T", self.service_trait().ty())
+            .bound("T", "Send")
+            .bound("T", "Sync")
+            .bound("T", "'static")
+            .line(format!("{}::new(service)", self.tonic_server_path()));
+
+        ret
+    }
+
+    /// Returns the gRPC service name used by tonic routing and health reporting.
+    fn service_name(&self) -> Function {
+        let mut ret = Function::new("service_name");
+        ret.vis("pub").attr("allow(deprecated)").ret("&'static str").line(format!(
+            "<{}::<()> as tonic::server::NamedService>::NAME",
+            self.tonic_server_path()
+        ));
+
+        ret
+    }
+
+    /// Returns the full path to the service's trait.
+    fn tonic_trait_path(&self) -> String {
+        format!(
+            "crate::generated::{}::{}_server::{}",
+            self.package,
+            to_snake_case(&self.name),
+            self.name
+        )
+    }
+
+    /// Returns the full path to the generated server struct.
+    fn tonic_server_path(&self) -> String {
+        format!("{}Server", self.tonic_trait_path())
+    }
 }
 
 impl UnaryMethod {

crates/proto/build.rs

@@ -12,4 +12,5 @@ pub mod generated;
 pub use domain::account::AccountWitnessRecord;
 pub use domain::proof_request::BlockProofRequest;
 pub use domain::{convert, try_convert};
+pub use generated::server;
 pub use prost;

crates/proto/src/lib.rs

@@ -5,7 +5,7 @@ use accept::AcceptHeaderLayer;
 use anyhow::Context;
 use miden_node_block_producer::{BlockProducerApi, RpcReadiness, RpcSync};
 use miden_node_proto::clients::{NtxBuilderClient, RpcClient as SourceRpcClient, ValidatorClient};
-use miden_node_proto::generated::rpc::api_server;
+use miden_node_proto::server::rpc_api;
 use miden_node_proto_build::rpc_api_descriptor;
 use miden_node_store::state::State;
 use miden_node_utils::clap::GrpcOptionsExternal;
@@ -105,7 +105,7 @@ impl Rpc {
 
         api.set_genesis_commitment(genesis.commitment())?;
 
-        let api_service = api_server::ApiServer::new(api);
+        let api_service = rpc_api::service(api);
 
         info!(target: COMPONENT, endpoint=?self.listener, mode=?self.mode, "Server initialized");
 
@@ -115,11 +115,19 @@ impl Rpc {
         let (health_reporter, health_service) = tonic_health::server::health_reporter();
         match self.mode {
             RpcMode::Sequencer { .. } => {
-                health_reporter.set_serving::<api_server::ApiServer<api::RpcService>>().await;
+                health_reporter
+                    .set_service_status(
+                        rpc_api::service_name(),
+                        tonic_health::ServingStatus::Serving,
+                    )
+                    .await;
             },
             RpcMode::FullNode { source_rpc, readiness_threshold } => {
                 health_reporter
-                    .set_not_serving::<api_server::ApiServer<api::RpcService>>()
+                    .set_service_status(
+                        rpc_api::service_name(),
+                        tonic_health::ServingStatus::NotServing,
+                    )
                     .await;
                 let readiness = RpcReadiness::new(health_reporter, readiness_threshold);
                 tasks.spawn(

crates/rpc/src/server/mod.rs

@@ -15,10 +15,10 @@ use miden_node_proto::clients::{
     RpcClient,
     ValidatorClient,
 };
-use miden_node_proto::generated::ntx_builder::api_server::ApiServer as NtxBuilderApiServer;
 use miden_node_proto::generated::rpc::api_client::ApiClient as ProtoClient;
-use miden_node_proto::generated::rpc::api_server::{Api, ApiServer as RpcApiServer};
+use miden_node_proto::generated::rpc::api_server::Api;
 use miden_node_proto::generated::{self as proto};
+use miden_node_proto::server::{ntx_builder_api, rpc_api};
 use miden_node_store::genesis::config::GenesisConfig;
 use miden_node_store::state::State;
 use miden_node_utils::clap::{GrpcOptionsExternal, StorageOptions};
@@ -502,19 +502,34 @@ struct FixedNtxBuilder {
 }
 
 #[tonic::async_trait]
-impl proto::ntx_builder::api_server::Api for FixedNtxBuilder {
-    async fn get_network_note_status(
+impl ntx_builder_api::GetNetworkNoteStatus for FixedNtxBuilder {
+    type Input = proto::note::NoteId;
+    type Output = proto::rpc::GetNetworkNoteStatusResponse;
+
+    fn decode(request: proto::note::NoteId) -> tonic::Result<Self::Input> {
+        Ok(request)
+    }
+
+    fn encode(output: Self::Output) -> tonic::Result<proto::rpc::GetNetworkNoteStatusResponse> {
+        Ok(output)
+    }
+
+    async fn handle(&self, _input: Self::Input) -> tonic::Result<Self::Output> {
+        Ok(self.response.clone())
+    }
+
+    async fn full(
         &self,
         request: Request<proto::note::NoteId>,
-    ) -> Result<tonic::Response<proto::rpc::GetNetworkNoteStatusResponse>, tonic::Status> {
+    ) -> tonic::Result<proto::rpc::GetNetworkNoteStatusResponse> {
         self.call_count.fetch_add(1, Ordering::SeqCst);
         let accept = request
             .metadata()
             .get(ACCEPT.as_str())
             .and_then(|value| value.to_str().ok())
             .map(str::to_string);
         *self.last_accept.lock().expect("last_accept mutex should not be poisoned") = accept;
-        Ok(tonic::Response::new(self.response.clone()))
+        self.handle(request.into_inner()).await.and_then(Self::encode)
     }
 }
 
@@ -533,7 +548,7 @@ async fn start_ntx_builder(
 
     task::spawn(async move {
         tonic::transport::Server::builder()
-            .add_service(NtxBuilderApiServer::new(service))
+            .add_service(ntx_builder_api::service(service))
             .serve_with_incoming(TcpListenerStream::new(listener))
             .await
             .expect("Failed to serve ntx-builder");
@@ -583,7 +598,7 @@ async fn start_source_rpc(ntx_builder: NtxBuilderClient) -> (RpcClient, TestStor
         );
 
         tonic::transport::Server::builder()
-            .add_service(RpcApiServer::new(source_rpc))
+            .add_service(rpc_api::service(source_rpc))
             .serve_with_incoming(TcpListenerStream::new(listener))
             .await
             .expect("Failed to serve source RPC");