chore: Misc followups for block validation

bin/remote-prover/src/api/prover.rs

@@ -172,16 +172,17 @@ impl ProverRpcApi {
             return Err(Status::unimplemented("Block prover is not enabled"));
         };
         let BlockProofRequest { tx_batches, block_header, block_inputs } = proof_request;
-        let block_proof = prover
-            .try_lock()
-            .map_err(|_| Status::resource_exhausted("Server is busy handling another request"))?
-            .prove(tx_batches, block_header.clone(), block_inputs)
-            .map_err(internal_error)?;
 
         // Record the commitment of the block in the current tracing span.
         let block_id = block_header.commitment();
         tracing::Span::current().record("block_id", tracing::field::display(&block_id));
 
+        let block_proof = prover
+            .try_lock()
+            .map_err(|_| Status::resource_exhausted("Server is busy handling another request"))?
+            .prove(tx_batches, block_header, block_inputs)
+            .map_err(internal_error)?;
+
         Ok(Response::new(proto::remote_prover::Proof { payload: block_proof.to_bytes() }))
     }
 }

crates/block-producer/src/block_builder/mod.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 use futures::FutureExt;
 use futures::never::Never;
 use miden_block_prover::LocalBlockProver;
+use miden_lib::block::build_block;
 use miden_node_utils::tracing::OpenTelemetrySpanExt;
 use miden_objects::MIN_PROOF_SECURITY_LEVEL;
 use miden_objects::batch::{OrderedBatches, ProvenBatch};
@@ -27,7 +28,7 @@ use url::Url;
 use crate::errors::BuildBlockError;
 use crate::mempool::SharedMempool;
 use crate::store::StoreClient;
-use crate::validator::BlockProducerValidatorClient;
+use crate::validator::{BlockProducerValidatorClient, BodyDiff, HeaderDiff, ValidatorError};
 use crate::{COMPONENT, TelemetryInjectorExt};
 
 // BLOCK BUILDER
@@ -230,16 +231,43 @@ impl BlockBuilder {
         proposed_block: ProposedBlock,
         block_inputs: BlockInputs,
     ) -> Result<(OrderedBatches, BlockInputs, BlockHeader, BlockBody), BuildBlockError> {
-        let response = self
+        // Concurrently build the block and validate it via the validator.
+        let build_result = tokio::task::spawn_blocking({
+            let proposed_block = proposed_block.clone();
+            move || build_block(proposed_block)
+        });
+        let (header, body) = self
             .validator
-            .validate_block(proposed_block.clone())
+            .sign_block(proposed_block.clone())
+            .await
+            .map_err(|err| BuildBlockError::ValidateBlockFailed(err.into()))?;
+        let (expected_header, expected_body) = build_result
             .await
-            .map_err(BuildBlockError::ValidateBlockFailed)?;
+            .map_err(|err| BuildBlockError::other(format!("task join error: {err}")))?
+            .map_err(BuildBlockError::ProposeBlockFailed)?;
 
-        // TODO: Check that the returned header and body match the proposed block.
+        // Check that the header and body returned from the validator is consistent with the
+        // proposed block.
+        // TODO(sergerad): Update Eq implementation once signatures are part of the header.
+        if header != expected_header {
+            let diff = HeaderDiff {
+                validator_header: header,
+                expected_header,
+            }
+            .into();
+            return Err(BuildBlockError::ValidateBlockFailed(
+                ValidatorError::HeaderMismatch(diff).into(),
+            ));
+        }
+        if body != expected_body {
+            let diff = BodyDiff { validator_body: body, expected_body }.into();
+            return Err(BuildBlockError::ValidateBlockFailed(
+                ValidatorError::BodyMismatch(diff).into(),
+            ));
+        }
 
         let (ordered_batches, ..) = proposed_block.into_parts();
-        Ok((ordered_batches, block_inputs, response.header, response.body))
+        Ok((ordered_batches, block_inputs, header, body))
     }
 
     #[instrument(target = COMPONENT, name = "block_builder.prove_block", skip_all, err)]
@@ -268,13 +296,17 @@ impl BlockBuilder {
         body: BlockBody,
         block_proof: BlockProof,
     ) -> Result<ProvenBlock, BuildBlockError> {
+        // SAFETY: The header and body are assumed valid and consistent with the proof.
         let proven_block = ProvenBlock::new_unchecked(header, body, block_proof);
         if proven_block.proof_security_level() < MIN_PROOF_SECURITY_LEVEL {
             return Err(BuildBlockError::SecurityLevelTooLow(
                 proven_block.proof_security_level(),
                 MIN_PROOF_SECURITY_LEVEL,
             ));
         }
+        // TODO(sergerad): Consider removing this validation. Once block proving is implemented,
+        // this would be replaced with verifying the proof returned from the prover against
+        // the block header.
         validate_tx_headers(&proven_block, &ordered_batches.to_transactions())?;
 
         Ok(proven_block)
@@ -464,7 +496,7 @@ impl BlockProver {
 /// passed in the proposed block.
 ///
 /// This expects that transactions from the proposed block and proven block are in the same
-/// order, as define by [`OrderedTransactionHeaders`].
+/// order, as defined by [`OrderedTransactionHeaders`].
 fn validate_tx_headers(
     proven_block: &ProvenBlock,
     proposed_txs: &OrderedTransactionHeaders,

crates/block-producer/src/errors.rs

@@ -209,7 +209,7 @@ pub enum BuildBlockError {
     #[error("failed to propose block")]
     ProposeBlockFailed(#[source] ProposedBlockError),
     #[error("failed to validate block")]
-    ValidateBlockFailed(#[source] ValidatorError),
+    ValidateBlockFailed(#[source] Box<ValidatorError>),
     #[error("failed to prove block")]
     ProveBlockFailed(#[source] BlockProverError),
     /// We sometimes randomly inject errors into the batch building process to test our failure

crates/block-producer/src/lib.rs

@@ -9,7 +9,7 @@ mod block_builder;
 mod domain;
 mod mempool;
 pub mod store;
-pub mod validator;
+mod validator;
 
 #[cfg(feature = "testing")]
 pub mod errors;

crates/block-producer/src/validator/mod.rs

@@ -1,4 +1,7 @@
+use std::fmt::{Display, Formatter};
+
 use miden_node_proto::clients::{Builder, ValidatorClient};
+use miden_node_proto::errors::{ConversionError, MissingFieldHelper};
 use miden_node_proto::generated as proto;
 use miden_objects::block::{BlockBody, BlockHeader, ProposedBlock};
 use miden_objects::utils::{Deserializable, Serializable};
@@ -15,25 +18,61 @@ use crate::COMPONENT;
 pub enum ValidatorError {
     #[error("gRPC transport error: {0}")]
     Transport(#[from] tonic::Status),
-    #[error("Failed to convert header: {0}")]
+    #[error("response content error: {0}")]
+    ResponseContent(#[from] ConversionError),
+    #[error("failed to convert header: {0}")]
     HeaderConversion(String),
-    #[error("Failed to deserialize body: {0}")]
+    #[error("failed to deserialize body: {0}")]
     BodyDeserialization(String),
+    #[error("validator header does not match the request: {0}")]
+    HeaderMismatch(Box<HeaderDiff>),
+    #[error("validator body does not match the request: {0}")]
+    BodyMismatch(Box<BodyDiff>),
 }
 
-// VALIDATE BLOCK RESPONSE
+// VALIDATION DIFF TYPES
 // ================================================================================================
 
+/// Represents a difference between validator and expected block headers
+#[derive(Debug, Clone)]
+pub struct HeaderDiff {
+    pub validator_header: BlockHeader,
+    pub expected_header: BlockHeader,
+}
+
+impl Display for HeaderDiff {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "Expected Header:")?;
+        writeln!(f, "{:?}", self.expected_header)?;
+        writeln!(f, "============================")?;
+        writeln!(f, "Validator Header:")?;
+        writeln!(f, "{:?}", self.validator_header)?;
+        Ok(())
+    }
+}
+
+/// Represents a difference between validator and expected block bodies
 #[derive(Debug, Clone)]
-pub struct ValidateBlockResponse {
-    pub header: BlockHeader,
-    pub body: BlockBody,
+pub struct BodyDiff {
+    pub validator_body: BlockBody,
+    pub expected_body: BlockBody,
+}
+
+impl Display for BodyDiff {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        writeln!(f, "Expected Body:")?;
+        writeln!(f, "{:?}", self.expected_body)?;
+        writeln!(f, "============================")?;
+        writeln!(f, "Validator Body:")?;
+        writeln!(f, "{:?}", self.validator_body)?;
+        Ok(())
+    }
 }
 
 // VALIDATOR CLIENT
 // ================================================================================================
 
-/// Interface to the validator's block-producer gRPC API.
+/// Interface to the validator's gRPC API.
 ///
 /// Essentially just a thin wrapper around the generated gRPC client which improves type safety.
 #[derive(Clone, Debug)]
@@ -58,28 +97,34 @@ impl BlockProducerValidatorClient {
     }
 
     #[instrument(target = COMPONENT, name = "validator.client.validate_block", skip_all, err)]
-    pub async fn validate_block(
+    pub async fn sign_block(
         &self,
         proposed_block: ProposedBlock,
-    ) -> Result<ValidateBlockResponse, ValidatorError> {
+    ) -> Result<(BlockHeader, BlockBody), ValidatorError> {
         // Send request and receive response.
         let message = proto::blockchain::ProposedBlock {
             proposed_block: proposed_block.to_bytes(),
         };
         let request = tonic::Request::new(message);
-        let response = self.client.clone().validate_block(request).await?;
-        let response = response.into_inner();
+        let response = self.client.clone().sign_block(request).await?;
+        let signed_block = response.into_inner();
 
-        // Extract header from response (should always be present).
-        let header_proto = response.header.expect("validator always returns a header");
+        // Extract header from response.
+        let header_proto = signed_block
+            .header
+            .ok_or(miden_node_proto::generated::blockchain::BlockHeader::missing_field("header"))
+            .map_err(ValidatorError::ResponseContent)?;
         let header = BlockHeader::try_from(header_proto)
             .map_err(|err| ValidatorError::HeaderConversion(err.to_string()))?;
 
-        // Extract body from response (should always be present).
-        let body_proto = response.body.expect("validator always returns a body");
+        // Extract body from response.
+        let body_proto = signed_block
+            .body
+            .ok_or(miden_node_proto::generated::blockchain::BlockBody::missing_field("body"))
+            .map_err(ValidatorError::ResponseContent)?;
         let body = BlockBody::read_from_bytes(&body_proto.block_body)
             .map_err(|err| ValidatorError::BodyDeserialization(err.to_string()))?;
 
-        Ok(ValidateBlockResponse { header, body })
+        Ok((header, body))
     }
 }

crates/proto/src/generated/validator.rs

@@ -9,13 +9,13 @@ pub struct ValidatorStatus {
     #[prost(string, tag = "2")]
     pub status: ::prost::alloc::string::String,
 }
-/// Response message for ValidateBlock RPC.
+/// Response message for SignBlock RPC.
 #[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
-pub struct ValidateBlockResponse {
-    /// The block header (required - always present).
+pub struct SignedBlock {
+    /// The block header.
     #[prost(message, optional, tag = "1")]
     pub header: ::core::option::Option<super::blockchain::BlockHeader>,
-    /// The block body (required - always present).
+    /// The block body.
     #[prost(message, optional, tag = "2")]
     pub body: ::core::option::Option<super::blockchain::BlockBody>,
 }
@@ -158,13 +158,10 @@ pub mod api_client {
             self.inner.unary(req, path, codec).await
         }
         /// Validates a proposed block and returns the block header and body.
-        pub async fn validate_block(
+        pub async fn sign_block(
             &mut self,
             request: impl tonic::IntoRequest<super::super::blockchain::ProposedBlock>,
-        ) -> std::result::Result<
-            tonic::Response<super::ValidateBlockResponse>,
-            tonic::Status,
-        > {
+        ) -> std::result::Result<tonic::Response<super::SignedBlock>, tonic::Status> {
             self.inner
                 .ready()
                 .await
@@ -174,12 +171,9 @@ pub mod api_client {
                     )
                 })?;
             let codec = tonic_prost::ProstCodec::default();
-            let path = http::uri::PathAndQuery::from_static(
-                "/validator.Api/ValidateBlock",
-            );
+            let path = http::uri::PathAndQuery::from_static("/validator.Api/SignBlock");
             let mut req = request.into_request();
-            req.extensions_mut()
-                .insert(GrpcMethod::new("validator.Api", "ValidateBlock"));
+            req.extensions_mut().insert(GrpcMethod::new("validator.Api", "SignBlock"));
             self.inner.unary(req, path, codec).await
         }
     }
@@ -208,13 +202,10 @@ pub mod api_server {
             request: tonic::Request<super::super::transaction::ProvenTransaction>,
         ) -> std::result::Result<tonic::Response<()>, tonic::Status>;
         /// Validates a proposed block and returns the block header and body.
-        async fn validate_block(
+        async fn sign_block(
             &self,
             request: tonic::Request<super::super::blockchain::ProposedBlock>,
-        ) -> std::result::Result<
-            tonic::Response<super::ValidateBlockResponse>,
-            tonic::Status,
-        >;
+        ) -> std::result::Result<tonic::Response<super::SignedBlock>, tonic::Status>;
     }
     /// Validator API for the Validator component.
     #[derive(Debug)]
@@ -380,15 +371,15 @@ pub mod api_server {
                     };
                     Box::pin(fut)
                 }
-                "/validator.Api/ValidateBlock" => {
+                "/validator.Api/SignBlock" => {
                     #[allow(non_camel_case_types)]
-                    struct ValidateBlockSvc<T: Api>(pub Arc<T>);
+                    struct SignBlockSvc<T: Api>(pub Arc<T>);
                     impl<
                         T: Api,
                     > tonic::server::UnaryService<
                         super::super::blockchain::ProposedBlock,
-                    > for ValidateBlockSvc<T> {
-                        type Response = super::ValidateBlockResponse;
+                    > for SignBlockSvc<T> {
+                        type Response = super::SignedBlock;
                         type Future = BoxFuture<
                             tonic::Response<Self::Response>,
                             tonic::Status,
@@ -401,7 +392,7 @@ pub mod api_server {
                         ) -> Self::Future {
                             let inner = Arc::clone(&self.0);
                             let fut = async move {
-                                <T as Api>::validate_block(&inner, request).await
+                                <T as Api>::sign_block(&inner, request).await
                             };
                             Box::pin(fut)
                         }
@@ -412,7 +403,7 @@ pub mod api_server {
                     let max_encoding_message_size = self.max_encoding_message_size;
                     let inner = self.inner.clone();
                     let fut = async move {
-                        let method = ValidateBlockSvc(inner);
+                        let method = SignBlockSvc(inner);
                         let codec = tonic_prost::ProstCodec::default();
                         let mut grpc = tonic::server::Grpc::new(codec)
                             .apply_compression_config(

crates/validator/src/server/mod.rs

@@ -103,10 +103,10 @@ impl api_server::Api for ValidatorServer {
     }
 
     /// Validates a proposed block and returns the block header and body.
-    async fn validate_block(
+    async fn sign_block(
         &self,
         request: tonic::Request<proto::blockchain::ProposedBlock>,
-    ) -> Result<tonic::Response<proto::validator::ValidateBlockResponse>, tonic::Status> {
+    ) -> Result<tonic::Response<proto::validator::SignedBlock>, tonic::Status> {
         let proposed_block_bytes = request.into_inner().proposed_block;
 
         // Deserialize the proposed block.
@@ -126,7 +126,7 @@ impl api_server::Api for ValidatorServer {
         let body_proto = proto::blockchain::BlockBody { block_body: body.to_bytes() };
 
         // Both header and body are required fields and must always be populated
-        let response = proto::validator::ValidateBlockResponse {
+        let response = proto::validator::SignedBlock {
             header: Some(header_proto),
             body: Some(body_proto),
         };