diff --git a/CHANGELOG.md b/CHANGELOG.md index befd00df0..8d19867df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Renamed card's names in the `miden-network-monitor` binary ([#1441](https://github.com/0xMiden/miden-node/pull/1441)). - Improved tracing in `miden-network-monitor` binary ([#1366](https://github.com/0xMiden/miden-node/pull/1366)). - Integrated RPC stack with Validator component for transaction validation ([#1457](https://github.com/0xMiden/miden-node/pull/1457)). +- Added validated transactions check to block validation logc in Validator ([#1460](https://github.com/0xMiden/miden-node/pull/1460)). - Added explorer status to the `miden-network-monitor` binary ([#1450](https://github.com/0xMiden/miden-node/pull/1450)). - Added `GetLimits` endpoint to the RPC server ([#1410](https://github.com/0xMiden/miden-node/pull/1410)). diff --git a/bin/node/src/commands/bundled.rs b/bin/node/src/commands/bundled.rs index 6745db617..a51c191eb 100644 --- a/bin/node/src/commands/bundled.rs +++ b/bin/node/src/commands/bundled.rs @@ -102,9 +102,10 @@ pub enum BundledCommand { #[arg( long = "validator.insecure.secret-key", env = ENV_VALIDATOR_INSECURE_SECRET_KEY, - value_name = "VALIDATOR_INSECURE_SECRET_KEY" + value_name = "VALIDATOR_INSECURE_SECRET_KEY", + default_value = INSECURE_VALIDATOR_KEY_HEX )] - validator_insecure_secret_key: Option, + validator_insecure_secret_key: String, }, } @@ -137,9 +138,8 @@ impl BundledCommand { grpc_timeout, validator_insecure_secret_key, } => { - let secret_key_hex = - validator_insecure_secret_key.unwrap_or(INSECURE_VALIDATOR_KEY_HEX.into()); - let signer = SecretKey::read_from_bytes(hex::decode(secret_key_hex)?.as_ref())?; + let secret_key_bytes = hex::decode(validator_insecure_secret_key)?; + let signer = SecretKey::read_from_bytes(&secret_key_bytes)?; Self::start( rpc_url, data_directory, diff --git a/crates/ntx-builder/src/actor/execute.rs b/crates/ntx-builder/src/actor/execute.rs index 3c5eeb702..83c1d09c9 100644 --- a/crates/ntx-builder/src/actor/execute.rs +++ b/crates/ntx-builder/src/actor/execute.rs @@ -412,25 +412,23 @@ impl DataStore for NtxDataStore { &self, script_root: Word, ) -> impl FutureMaybeSend, DataStoreError>> { - let store = self.store.clone(); - let mut cache = self.script_cache.clone(); - async move { // Attempt to retrieve the script from the cache. - if let Some(cached_script) = cache.get(&script_root).await { + if let Some(cached_script) = self.script_cache.get(&script_root).await { return Ok(Some(cached_script)); } // Retrieve the script from the store. - let maybe_script = store.get_note_script_by_root(script_root).await.map_err(|err| { - DataStoreError::Other { - error_msg: "failed to retrieve note script from store".to_string().into(), - source: Some(err.into()), - } - })?; + let maybe_script = + self.store.get_note_script_by_root(script_root).await.map_err(|err| { + DataStoreError::Other { + error_msg: "failed to retrieve note script from store".to_string().into(), + source: Some(err.into()), + } + })?; // Handle response. if let Some(script) = maybe_script { - cache.put(script_root, script.clone()).await; + self.script_cache.put(script_root, script.clone()).await; Ok(Some(script)) } else { Ok(None) diff --git a/crates/rpc/src/server/api.rs b/crates/rpc/src/server/api.rs index 7dc17e196..86bb35e59 100644 --- a/crates/rpc/src/server/api.rs +++ b/crates/rpc/src/server/api.rs @@ -25,7 +25,7 @@ use miden_protocol::utils::serde::{Deserializable, Serializable}; use miden_protocol::{MIN_PROOF_SECURITY_LEVEL, Word}; use miden_tx::TransactionVerifier; use tonic::{IntoRequest, Request, Response, Status}; -use tracing::{debug, info, instrument, warn}; +use tracing::{debug, info, instrument}; use url::Url; use crate::COMPONENT; @@ -392,23 +392,7 @@ impl api_server::Api for RpcService { // If transaction inputs are provided, re-execute the transaction to validate it. if request.transaction_inputs.is_some() { // Re-execute the transaction via the Validator. - match self.validator.clone().submit_proven_transaction(request.clone()).await { - Ok(_) => { - debug!( - target = COMPONENT, - tx_id = %tx.id().to_hex(), - "Transaction validation successful" - ); - }, - Err(e) => { - warn!( - target = COMPONENT, - tx_id = %tx.id().to_hex(), - error = %e, - "Transaction validation failed, but continuing with submission" - ); - }, - } + self.validator.clone().submit_proven_transaction(request.clone()).await?; } block_producer.clone().submit_proven_transaction(request).await diff --git a/crates/utils/src/lru_cache.rs b/crates/utils/src/lru_cache.rs index de325bf10..7e6751529 100644 --- a/crates/utils/src/lru_cache.rs +++ b/crates/utils/src/lru_cache.rs @@ -26,7 +26,7 @@ where } /// Puts a value into the cache. - pub async fn put(&mut self, key: K, value: V) { + pub async fn put(&self, key: K, value: V) { self.0.lock().await.put(key, value); } } diff --git a/crates/validator/src/block_validation/mod.rs b/crates/validator/src/block_validation/mod.rs new file mode 100644 index 000000000..68744010f --- /dev/null +++ b/crates/validator/src/block_validation/mod.rs @@ -0,0 +1,51 @@ +use std::sync::Arc; + +use miden_protocol::ProposedBlockError; +use miden_protocol::block::{BlockNumber, BlockSigner, ProposedBlock}; +use miden_protocol::crypto::dsa::ecdsa_k256_keccak::Signature; +use miden_protocol::transaction::TransactionId; + +use crate::server::ValidatedTransactions; + +// BLOCK VALIDATION ERROR +// ================================================================================================ + +#[derive(thiserror::Error, Debug)] +pub enum BlockValidationError { + #[error("transaction {0} in block {1} has not been validated")] + TransactionNotValidated(TransactionId, BlockNumber), + #[error("failed to build block")] + BlockBuildingFailed(#[from] ProposedBlockError), +} + +// BLOCK VALIDATION +// ================================================================================================ + +/// Validates a block by checking that all transactions in the proposed block have been processed by +/// the validator in the past. +/// +/// Removes the validated transactions from the cache upon success. +pub async fn validate_block( + proposed_block: ProposedBlock, + signer: &S, + validated_transactions: Arc, +) -> Result { + // Check that all transactions in the proposed block have been validated + for tx_header in proposed_block.transactions() { + let tx_id = tx_header.id(); + if validated_transactions.get(&tx_id).await.is_none() { + return Err(BlockValidationError::TransactionNotValidated( + tx_id, + proposed_block.block_num(), + )); + } + } + + // Build the block header. + let (header, _) = proposed_block.into_header_and_body()?; + + // Sign the header. + let signature = signer.sign(&header); + + Ok(signature) +} diff --git a/crates/validator/src/lib.rs b/crates/validator/src/lib.rs index 065098d8a..a45112d27 100644 --- a/crates/validator/src/lib.rs +++ b/crates/validator/src/lib.rs @@ -1,3 +1,4 @@ +mod block_validation; mod server; mod tx_validation; diff --git a/crates/validator/src/server/mod.rs b/crates/validator/src/server/mod.rs index 439573436..0c48de745 100644 --- a/crates/validator/src/server/mod.rs +++ b/crates/validator/src/server/mod.rs @@ -1,4 +1,6 @@ use std::net::SocketAddr; +use std::num::NonZeroUsize; +use std::sync::Arc; use std::time::Duration; use anyhow::Context; @@ -6,11 +8,17 @@ use miden_node_proto::generated::validator::api_server; use miden_node_proto::generated::{self as proto}; use miden_node_proto_build::validator_api_descriptor; use miden_node_utils::ErrorReport; +use miden_node_utils::lru_cache::LruCache; use miden_node_utils::panic::catch_panic_layer_fn; use miden_node_utils::tracing::grpc::grpc_trace_fn; use miden_protocol::block::{BlockSigner, ProposedBlock}; -use miden_protocol::transaction::{ProvenTransaction, TransactionInputs}; -use miden_protocol::utils::{Deserializable, Serializable}; +use miden_protocol::transaction::{ + ProvenTransaction, + TransactionHeader, + TransactionId, + TransactionInputs, +}; +use miden_tx::utils::{Deserializable, Serializable}; use tokio::net::TcpListener; use tokio_stream::wrappers::TcpListenerStream; use tonic::Status; @@ -18,8 +26,15 @@ use tower_http::catch_panic::CatchPanicLayer; use tower_http::trace::TraceLayer; use crate::COMPONENT; +use crate::block_validation::validate_block; use crate::tx_validation::validate_transaction; +/// Number of transactions to keep in the validated transactions cache. +const NUM_VALIDATED_TRANSACTIONS: NonZeroUsize = NonZeroUsize::new(7000).unwrap(); + +/// A type alias for a LRU cache that stores validated transactions. +pub type ValidatedTransactions = LruCache; + // VALIDATOR // ================================================================================ @@ -69,7 +84,7 @@ impl Validator { .layer(CatchPanicLayer::custom(catch_panic_layer_fn)) .layer(TraceLayer::new_for_grpc().make_span_with(grpc_trace_fn)) .timeout(self.grpc_timeout) - .add_service(api_server::ApiServer::new(ValidatorServer { signer: self.signer })) + .add_service(api_server::ApiServer::new(ValidatorServer::new(self.signer))) .add_service(reflection_service) .add_service(reflection_service_alpha) .serve_with_incoming(TcpListenerStream::new(listener)) @@ -86,6 +101,15 @@ impl Validator { /// Implements the gRPC API for the validator. struct ValidatorServer { signer: S, + validated_transactions: Arc, +} + +impl ValidatorServer { + fn new(signer: S) -> Self { + let validated_transactions = + Arc::new(ValidatedTransactions::new(NUM_VALIDATED_TRANSACTIONS)); + Self { signer, validated_transactions } + } } #[tonic::async_trait] @@ -122,9 +146,14 @@ impl api_server::Api for ValidatorServer })?; // Validate the transaction. - validate_transaction(proven_tx, tx_inputs).await.map_err(|err| { - Status::invalid_argument(err.as_report_context("Invalid transaction")) - })?; + let validated_tx_header = + validate_transaction(proven_tx, tx_inputs).await.map_err(|err| { + Status::invalid_argument(err.as_report_context("Invalid transaction")) + })?; + + // Register the validated transaction. + let tx_id = validated_tx_header.id(); + self.validated_transactions.put(tx_id, validated_tx_header).await; Ok(tonic::Response::new(())) } @@ -144,11 +173,13 @@ impl api_server::Api for ValidatorServer )) })?; - // Build and sign header. - let (header, _body) = proposed_block - .into_header_and_body() - .map_err(|err| tonic::Status::internal(format!("Failed to build block: {err}")))?; - let signature = self.signer.sign(&header); + // Validate the block. + let signature = + validate_block(proposed_block, &self.signer, self.validated_transactions.clone()) + .await + .map_err(|err| { + tonic::Status::invalid_argument(format!("Failed to validate block: {err}",)) + })?; // Send the signature. let response = proto::blockchain::BlockSignature { signature: signature.to_bytes() }; diff --git a/crates/validator/src/tx_validation/mod.rs b/crates/validator/src/tx_validation/mod.rs index 024f7f124..95419c392 100644 --- a/crates/validator/src/tx_validation/mod.rs +++ b/crates/validator/src/tx_validation/mod.rs @@ -27,10 +27,12 @@ pub enum TransactionValidationError { /// Validates a transaction by verifying its proof, executing it and comparing its header with the /// provided proven transaction. +/// +/// Returns the header of the executed transaction if successful. pub async fn validate_transaction( proven_tx: ProvenTransaction, tx_inputs: TransactionInputs, -) -> Result<(), TransactionValidationError> { +) -> Result { // First, verify the transaction proof let tx_verifier = TransactionVerifier::new(MIN_PROOF_SECURITY_LEVEL); tx_verifier.verify(&proven_tx)?; @@ -50,7 +52,7 @@ pub async fn validate_transaction( let executed_tx_header: TransactionHeader = (&executed_tx).into(); let proven_tx_header: TransactionHeader = (&proven_tx).into(); if executed_tx_header == proven_tx_header { - Ok(()) + Ok(executed_tx_header) } else { Err(TransactionValidationError::Mismatch { proven_tx_header: proven_tx_header.into(),