diff --git a/bin/validator/src/server/validator_service/sign_block.rs b/bin/validator/src/server/validator_service/sign_block.rs index 5958a215e8..a7050d915b 100644 --- a/bin/validator/src/server/validator_service/sign_block.rs +++ b/bin/validator/src/server/validator_service/sign_block.rs @@ -1,7 +1,6 @@ use std::sync::atomic::Ordering; -use miden_node_proto::domain::protocol_config::ensure_protocol_config_is_present_and_matches_header; -use miden_node_proto::{BlockProofRequest, generated as grpc}; +use miden_node_proto::{SignBlockRequest, generated as grpc}; use miden_node_tracing::spawn::spawn_blocking_in_current_span; use miden_node_tracing::{ErrorReport, Instrument, info_span, miden_instrument}; use miden_protocol::Word; @@ -14,14 +13,14 @@ use crate::COMPONENT; #[tonic::async_trait] impl grpc::server::validator_api::SignBlock for ValidatorService { - type Input = grpc::block_proving::BlockProofRequest; + type Input = grpc::validator::SignBlockRequest; type Output = (Signature, Word, PublicKey); #[miden_instrument( target = COMPONENT, err, )] - fn decode(request: grpc::block_proving::BlockProofRequest) -> tonic::Result { + fn decode(request: grpc::validator::SignBlockRequest) -> tonic::Result { Ok(request) } @@ -62,18 +61,8 @@ impl grpc::server::validator_api::SignBlock for ValidatorService { let (proposed_block, protocol_config, protocol_config_commitment) = spawn_blocking_in_current_span(move || { - let mut request = request; - let supplied_protocol_config = request.protocol_config.take(); - let request = BlockProofRequest::try_from(request).map_err(tonic::Status::from)?; - let protocol_config = supplied_protocol_config - .map(|config| { - ensure_protocol_config_is_present_and_matches_header( - Some(config), - &request.block_header, - ) - }) - .transpose() - .map_err(tonic::Status::from)?; + let request = SignBlockRequest::try_from(request).map_err(tonic::Status::from)?; + let protocol_config = request.protocol_config; let protocol_config_commitment = request.block_header.protocol_config_commitment(); let proposed_block = ProposedBlock::new_at( request.block_inputs, diff --git a/bin/validator/src/server/validator_service/tests.rs b/bin/validator/src/server/validator_service/tests.rs index 6e0c6ea282..92029d59b3 100644 --- a/bin/validator/src/server/validator_service/tests.rs +++ b/bin/validator/src/server/validator_service/tests.rs @@ -1,12 +1,12 @@ use std::collections::BTreeMap; +use miden_node_proto::SignBlockRequest; use miden_node_proto::domain::encryption::{ TransactionEncryptionScheme, TrustedTransactionEncryptionState, transaction_inputs_associated_data, verify_transaction_encryption_key, }; -use miden_node_proto::domain::proof_request::BlockProofRequest; use miden_node_proto::generated::{self as proto}; use miden_node_proto::server::validator_api; use miden_node_store::{BlockStore, GenesisState}; @@ -184,13 +184,13 @@ impl TestValidator { BTreeMap::new(), ); let (block_header, _) = proposed_block.clone().into_header_and_body().unwrap(); - let mut request: proto::block_proving::BlockProofRequest = BlockProofRequest { + let request: proto::validator::SignBlockRequest = SignBlockRequest { tx_batches: OrderedBatches::new(proposed_block.batches().as_slice().to_vec()), block_header, block_inputs, + protocol_config: protocol_config.cloned(), } .into(); - request.protocol_config = protocol_config.map(Into::into); let request = tonic::Request::new(request); validator_api::SignBlock::full(&self.server, request).await } diff --git a/crates/block-producer/src/validator/mod.rs b/crates/block-producer/src/validator/mod.rs index 9d7a22c26c..ea2f28ee14 100644 --- a/crates/block-producer/src/validator/mod.rs +++ b/crates/block-producer/src/validator/mod.rs @@ -95,7 +95,7 @@ impl BlockProducerValidatorClient { block_inputs: &BlockInputs, protocol_config: &ProtocolConfig, ) -> Result, ValidatorError> { - let message = proto::block_proving::BlockProofRequest { + let message = proto::validator::SignBlockRequest { protocol_config: Some(protocol_config.into()), batches: proposed_block.batches().as_slice().iter().map(Into::into).collect(), block_inputs: Some(block_inputs.into()), diff --git a/crates/proto/src/domain/block_proposal.rs b/crates/proto/src/domain/block_proposal.rs new file mode 100644 index 0000000000..2eef60cf34 --- /dev/null +++ b/crates/proto/src/domain/block_proposal.rs @@ -0,0 +1,46 @@ +//! Shared decoding of block proposal fields. + +use miden_protocol::batch::{OrderedBatches, ProvenBatch}; +use miden_protocol::block::{BlockHeader, BlockInputs, ProposedBlock}; + +use crate::errors::ConversionError; +use crate::generated as proto; + +pub(super) struct DecodedBlockProposal { + pub tx_batches: OrderedBatches, + pub block_header: BlockHeader, + pub block_inputs: BlockInputs, +} + +pub(super) fn decode( + block_inputs: proto::block_proving::BlockInputs, + batches: Vec, + timestamp: u32, + next_validator_config: proto::blockchain::ValidatorConfig, + next_protocol_config: Option, +) -> Result { + let block_inputs: BlockInputs = block_inputs.try_into()?; + let batches = batches + .into_iter() + .enumerate() + .map(|(index, batch)| { + miden_objects::conversion::decode_standalone_proven_batch(batch) + .map_err(|error| ConversionError::from(error.context(format!("batches[{index}]")))) + }) + .collect::, _>>()?; + let next_validator_config = next_validator_config.try_into().map_err(ConversionError::from)?; + let next_protocol_config = next_protocol_config + .map(TryInto::try_into) + .transpose() + .map_err(ConversionError::from)?; + let proposed_block = ProposedBlock::new_at(block_inputs.clone(), batches.clone(), timestamp) + .map_err(ConversionError::new)? + .with_next_validator_config(next_validator_config) + .with_next_protocol_config(next_protocol_config); + let (block_header, _) = proposed_block.into_header_and_body().map_err(ConversionError::new)?; + Ok(DecodedBlockProposal { + tx_batches: OrderedBatches::new(batches), + block_header, + block_inputs, + }) +} diff --git a/crates/proto/src/domain/mod.rs b/crates/proto/src/domain/mod.rs index ef8d39e822..3476bdcd92 100644 --- a/crates/proto/src/domain/mod.rs +++ b/crates/proto/src/domain/mod.rs @@ -1,8 +1,11 @@ +mod block_proposal; + pub mod account; pub mod block; pub mod encryption; pub mod proof_request; pub mod protocol_config; +pub mod sign_block_request; pub mod submission; use miden_node_tracing::{RecordAttribute, Value}; diff --git a/crates/proto/src/domain/proof_request.rs b/crates/proto/src/domain/proof_request.rs index d5a138be11..438857e168 100644 --- a/crates/proto/src/domain/proof_request.rs +++ b/crates/proto/src/domain/proof_request.rs @@ -1,10 +1,10 @@ use std::collections::BTreeMap; use miden_protocol::account::AccountId; -use miden_protocol::batch::{OrderedBatches, ProvenBatch}; +use miden_protocol::batch::OrderedBatches; use miden_protocol::block::account_tree::AccountWitness; use miden_protocol::block::nullifier_tree::NullifierWitness; -use miden_protocol::block::{BlockHeader, BlockInputs, ProposedBlock}; +use miden_protocol::block::{BlockHeader, BlockInputs}; use miden_protocol::note::{NoteId, NoteInclusionProof, Nullifier}; use miden_protocol::transaction::PartialBlockchain; use miden_protocol::utils::serde::{ @@ -34,7 +34,6 @@ impl From<&BlockProofRequest> for proto::block_proving::BlockProofRequest { timestamp: value.block_header.timestamp(), next_validator_config: Some(value.block_header.validator_config().into()), next_protocol_config: value.block_header.next_protocol_config().map(Into::into), - protocol_config: None, } } } @@ -49,53 +48,27 @@ impl TryFrom for BlockProofRequest { type Error = ConversionError; fn try_from(value: proto::block_proving::BlockProofRequest) -> Result { - let block_inputs: BlockInputs = value - .block_inputs - .ok_or_else(|| { - ConversionError::missing_field::( - "block_inputs", - ) - })? - .try_into()?; - - let batches = value - .batches - .into_iter() - .enumerate() - .map(|(index, batch)| { - miden_objects::conversion::decode_standalone_proven_batch(batch).map_err(|error| { - ConversionError::from(error.context(format!("batches[{index}]"))) - }) - }) - .collect::, _>>()?; - - let next_validator_config = value - .next_validator_config - .ok_or_else(|| { - ConversionError::missing_field::( - "next_validator_config", - ) - })? - .try_into() - .map_err(ConversionError::from)?; - let next_protocol_config = value - .next_protocol_config - .map(TryInto::try_into) - .transpose() - .map_err(ConversionError::from)?; - - let proposed_block = - ProposedBlock::new_at(block_inputs.clone(), batches.clone(), value.timestamp) - .map_err(ConversionError::new)? - .with_next_validator_config(next_validator_config) - .with_next_protocol_config(next_protocol_config); - let (block_header, _) = - proposed_block.into_header_and_body().map_err(ConversionError::new)?; - - Ok(Self { - tx_batches: OrderedBatches::new(batches), - block_header, + let block_inputs = value.block_inputs.ok_or_else(|| { + ConversionError::missing_field::( + "block_inputs", + ) + })?; + let next_validator_config = value.next_validator_config.ok_or_else(|| { + ConversionError::missing_field::( + "next_validator_config", + ) + })?; + let decoded = super::block_proposal::decode( block_inputs, + value.batches, + value.timestamp, + next_validator_config, + value.next_protocol_config, + )?; + Ok(Self { + tx_batches: decoded.tx_batches, + block_header: decoded.block_header, + block_inputs: decoded.block_inputs, }) } } diff --git a/crates/proto/src/domain/sign_block_request.rs b/crates/proto/src/domain/sign_block_request.rs new file mode 100644 index 0000000000..58020cada4 --- /dev/null +++ b/crates/proto/src/domain/sign_block_request.rs @@ -0,0 +1,74 @@ +//! Validator signing request conversions. + +use miden_protocol::batch::OrderedBatches; +use miden_protocol::block::{BlockHeader, BlockInputs}; +use miden_protocol::protocol_config::ProtocolConfig; + +use super::protocol_config::ensure_protocol_config_is_present_and_matches_header; +use crate::errors::ConversionError; +use crate::generated as proto; + +/// The domain inputs needed to validate and sign a block. +#[derive(Debug)] +pub struct SignBlockRequest { + pub tx_batches: OrderedBatches, + pub block_header: BlockHeader, + pub block_inputs: BlockInputs, + pub protocol_config: Option, +} + +impl TryFrom for SignBlockRequest { + type Error = ConversionError; + + fn try_from(value: proto::validator::SignBlockRequest) -> Result { + let block_inputs = value.block_inputs.ok_or_else(|| { + ConversionError::missing_field::("block_inputs") + })?; + let next_validator_config = value.next_validator_config.ok_or_else(|| { + ConversionError::missing_field::( + "next_validator_config", + ) + })?; + let decoded = super::block_proposal::decode( + block_inputs, + value.batches, + value.timestamp, + next_validator_config, + value.next_protocol_config, + )?; + let protocol_config = value + .protocol_config + .map(|config| { + ensure_protocol_config_is_present_and_matches_header( + Some(config), + &decoded.block_header, + ) + }) + .transpose()?; + Ok(Self { + tx_batches: decoded.tx_batches, + block_header: decoded.block_header, + block_inputs: decoded.block_inputs, + protocol_config, + }) + } +} + +impl From<&SignBlockRequest> for proto::validator::SignBlockRequest { + fn from(value: &SignBlockRequest) -> Self { + Self { + batches: value.tx_batches.as_slice().iter().map(Into::into).collect(), + block_inputs: Some((&value.block_inputs).into()), + timestamp: value.block_header.timestamp(), + next_validator_config: Some(value.block_header.validator_config().into()), + next_protocol_config: value.block_header.next_protocol_config().map(Into::into), + protocol_config: value.protocol_config.as_ref().map(Into::into), + } + } +} + +impl From for proto::validator::SignBlockRequest { + fn from(value: SignBlockRequest) -> Self { + Self::from(&value) + } +} diff --git a/crates/proto/src/lib.rs b/crates/proto/src/lib.rs index 72534dab5f..b5b8d0ff37 100644 --- a/crates/proto/src/lib.rs +++ b/crates/proto/src/lib.rs @@ -10,6 +10,7 @@ pub mod generated; // ================================================================================================ pub use domain::proof_request::BlockProofRequest; +pub use domain::sign_block_request::SignBlockRequest; pub use domain::submission::{ProvenTransactionSubmission, TransactionBatchSubmission}; pub use domain::{convert, try_convert}; pub use generated::server; diff --git a/crates/proto/tests/node_conversions.rs b/crates/proto/tests/node_conversions.rs index b02c5c09e5..fb711a77e9 100644 --- a/crates/proto/tests/node_conversions.rs +++ b/crates/proto/tests/node_conversions.rs @@ -427,3 +427,111 @@ fn canonical_conversion_errors_map_to_invalid_argument() { assert_eq!(status.code(), tonic::Code::InvalidArgument); } + +fn signing_request() -> miden_node_proto::SignBlockRequest { + let proof = nonempty_block_request(); + miden_node_proto::SignBlockRequest { + tx_batches: proof.tx_batches, + block_header: proof.block_header, + block_inputs: proof.block_inputs, + protocol_config: None, + } +} + +#[test] +fn signing_roundtrip_preserves_proposal_and_matches_proving() { + let request = signing_request(); + let message = generated::validator::SignBlockRequest::from(&request); + let decoded = miden_node_proto::SignBlockRequest::try_from( + generated::validator::SignBlockRequest::decode(message.encode_to_vec().as_slice()).unwrap(), + ) + .unwrap(); + assert_eq!(decoded.block_header, request.block_header); + assert_eq!(decoded.tx_batches.as_slice(), request.tx_batches.as_slice()); + assert_eq!( + generated::block_proving::BlockInputs::from(&decoded.block_inputs), + generated::block_proving::BlockInputs::from(&request.block_inputs) + ); + assert!(decoded.protocol_config.is_none()); + let proof_message = generated::block_proving::BlockProofRequest { + batches: message.batches, + block_inputs: message.block_inputs, + timestamp: message.timestamp, + next_validator_config: message.next_validator_config, + next_protocol_config: message.next_protocol_config, + }; + let proof = BlockProofRequest::try_from(proof_message).unwrap(); + assert_eq!(decoded.block_header, proof.block_header); +} + +#[test] +fn signing_rejects_missing_fields_and_malformed_batches() { + let message = generated::validator::SignBlockRequest::from(&signing_request()); + for field in ["block_inputs", "next_validator_config"] { + let mut invalid = message.clone(); + if field == "block_inputs" { + invalid.block_inputs = None; + } else { + invalid.next_validator_config = None; + } + let error = miden_node_proto::SignBlockRequest::try_from(invalid).unwrap_err(); + assert!(error.to_string().contains(field)); + } + let mut invalid = message; + invalid.batches[0] = proto::transaction::ProvenBatch::default(); + let error = miden_node_proto::SignBlockRequest::try_from(invalid).unwrap_err(); + assert!(error.to_string().contains("batches[0]")); + assert!( + error + .source() + .unwrap() + .downcast_ref::() + .is_some() + ); +} + +#[test] +fn signing_rejects_duplicate_witnesses_and_preserves_absent_next_config() { + let mut message = generated::validator::SignBlockRequest::from(&signing_request()); + message.next_protocol_config = None; + let decoded = miden_node_proto::SignBlockRequest::try_from(message.clone()).unwrap(); + assert!(decoded.block_header.next_protocol_config().is_none()); + let witnesses = &mut message.block_inputs.as_mut().unwrap().nullifier_witnesses; + witnesses.push(witnesses[0].clone()); + let error = miden_node_proto::SignBlockRequest::try_from(message).unwrap_err(); + assert!(error.to_string().contains("duplicate nullifier")); +} + +#[test] +fn signing_roundtrip_validates_supplied_active_configuration() { + use miden_protocol::asset::AssetId; + use miden_protocol::protocol_config::ProtocolConfig; + use miden_protocol::testing::account_id::ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1; + + let config = ProtocolConfig::current(AssetId::new_fungible( + ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1.try_into().unwrap(), + )) + .unwrap(); + let proof = block_request_message(); + let mut message = generated::validator::SignBlockRequest { + batches: proof.batches, + block_inputs: proof.block_inputs, + timestamp: proof.timestamp, + next_validator_config: proof.next_validator_config, + next_protocol_config: proof.next_protocol_config, + protocol_config: Some((&config).into()), + }; + assert!(miden_node_proto::SignBlockRequest::try_from(message.clone()).is_err()); + message + .block_inputs + .as_mut() + .unwrap() + .prev_block_header + .as_mut() + .unwrap() + .protocol_config_commitment = Some(config.to_commitment().into()); + let decoded = miden_node_proto::SignBlockRequest::try_from(message.clone()).unwrap(); + assert_eq!(decoded.protocol_config, Some(config)); + let encoded = generated::validator::SignBlockRequest::from(decoded); + assert_eq!(encoded, message); +} diff --git a/crates/rpc/src/tests.rs b/crates/rpc/src/tests.rs index e28e2814cb..7658f5b7d3 100644 --- a/crates/rpc/src/tests.rs +++ b/crates/rpc/src/tests.rs @@ -1113,7 +1113,7 @@ impl validator_api::SignBlock for FixedValidator { type Input = (); type Output = proto::validator::SignBlockResponse; - fn decode(_request: proto::block_proving::BlockProofRequest) -> tonic::Result { + fn decode(_request: proto::validator::SignBlockRequest) -> tonic::Result { Ok(()) } diff --git a/proto/proto/README.md b/proto/proto/README.md index 9db9537585..8d7be16505 100644 --- a/proto/proto/README.md +++ b/proto/proto/README.md @@ -28,7 +28,7 @@ Public service files can import shared node-owned types and canonical object sch service files. This keeps internal services out of public service reflection. Keep service-specific wrappers with their service. For example, `rpc.proto` owns note query and compact note sync -messages. `internal/validator.proto` owns the signature response. Submission envelopes belong in +messages. `internal/validator.proto` owns the signing request and signature response. Submission envelopes belong in `types/submission.proto`. Block proving requests belong in `types/block_proving.proto`. See the [migration guidance](../README.md#canonical-protobuf-migration) before updating an existing client. diff --git a/proto/proto/internal/validator.proto b/proto/proto/internal/validator.proto index 4a5937abec..95d979717f 100644 --- a/proto/proto/internal/validator.proto +++ b/proto/proto/internal/validator.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package validator; import "block.proto"; +import "batch.proto"; import "primitives.proto"; import "protocol_config.proto"; import "types/block_proving.proto"; @@ -21,7 +22,7 @@ service Api { rpc SubmitProvenTransaction(submission.ProvenTransactionSubmission) returns (google.protobuf.Empty) {} // Validates and signs a proposed block. - rpc SignBlock(block_proving.BlockProofRequest) returns (SignBlockResponse) {} + rpc SignBlock(SignBlockRequest) returns (SignBlockResponse) {} // Streams signed blocks starting from the given block number (inclusive). // @@ -39,9 +40,27 @@ service Api { rpc GetTransactionEncryptionKey(google.protobuf.Empty) returns (submission.TransactionEncryptionKey) {} } -// SIGN BLOCK RESPONSE +// SIGN BLOCK REQUEST AND RESPONSE // ================================================================================================ +// The inputs required to validate and sign a proposed block. +message SignBlockRequest { + // The proven transaction batches in block order. + repeated transaction.ProvenBatch batches = 1; + // The chain state and witnesses required to reconstruct the proposed block. + block_proving.BlockInputs block_inputs = 2; + // The timestamp of the proposed block. + fixed32 timestamp = 3; + // The next validator configuration included in the proposed block header. + blockchain.ValidatorConfig next_validator_config = 4; + // The optional next protocol configuration included in the proposed block header. + optional blockchain.NextProtocolConfig next_protocol_config = 5; + // The active configuration for signing. The block producer always supplies this field. + // Other callers may omit this field if the validator already stores the configuration. + // The validator returns INVALID_ARGUMENT if the field is omitted and the configuration is not stored. + optional protocol_config.ProtocolConfig protocol_config = 6; +} + message SignBlockResponse { primitives.Signature signature = 1; primitives.Word block_commitment = 2; diff --git a/proto/proto/types/block_proving.proto b/proto/proto/types/block_proving.proto index a50549448d..20070a0645 100644 --- a/proto/proto/types/block_proving.proto +++ b/proto/proto/types/block_proving.proto @@ -7,7 +7,6 @@ import "block.proto"; import "note.proto"; import "partial_blockchain.proto"; import "primitives.proto"; -import "protocol_config.proto"; // The requested account ID paired with the canonical account-tree witness. message AccountWitnessRecord { @@ -52,9 +51,4 @@ message BlockProofRequest { // The optional next protocol configuration included in the proposed block header. optional blockchain.NextProtocolConfig next_protocol_config = 5; - // The active configuration for signing. The block producer always supplies this field. - // Other callers may omit this field if the validator already stores the configuration. - // The validator returns INVALID_ARGUMENT if the field is omitted and the configuration is not stored. - // Proving requests do not require this field. - optional protocol_config.ProtocolConfig protocol_config = 6; }