diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cb25ea9b5..422f5d5ed4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Breaking Changes +* [BREAKING][rust] `NodeRpcClient` implementations are now raw transports: the echo checks that verified node responses against the request moved from `GrpcClient` into the new `VerifyingRpcClient` wrapper. `ClientBuilder::grpc_client` and the network constructors wrap their gRPC client automatically; clients passed via `ClientBuilder::rpc` are used as provided, so wrap them in `VerifyingRpcClient` to keep verified responses ([#2278](https://github.com/0xMiden/rust-sdk/pull/2278)). * [BREAKING][rust][cli] Removed the client debug-mode toggle: `DebugMode`, `ClientBuilder::in_debug_mode`, `Client::in_debug_mode`, the CLI `--debug` flag, and the `MIDEN_DEBUG` environment variable are gone, along with the `debug_mode` parameter of `CliClient::new`/`CliClient::from_config`. Miden VM 0.24 replaced the flag-gated `debug.*` MASM decorators with `miden::core::debug` procedures whose output the transaction executor prints by default, so the toggle no longer had anything to gate. ([#2290](https://github.com/0xMiden/rust-sdk/pull/2290)). * [BREAKING][rust] Migrated to `miden-protocol` 0.16. Transaction-level fees were removed, block-level `FeeParameters` are unchanged. The relative `AccountDelta` model was replaced by the absolute `AccountPatch` model for account updates: `TransactionResult::account_delta` is now `account_patch`, `AccountUpdateDetails::Public` now carries an `AccountPatch`, account reconstruction is done via `Account::try_from(&AccountPatch)` or `Account::apply_patch` instead of previous `apply_delta`. Re-exports changed accordingly: `AccountStorageDelta` for `AccountStoragePatch`, `StorageMapDelta` for `StorageMapPatch`, and so on. Standards APIs were updated: `AuthMethod` was removed (use the concrete auth components), `create_fungible_faucet` was replaced by auth-specific variants such as `create_singlesig_user_fungible_faucet`. ([#2290](https://github.com/0xMiden/rust-sdk/pull/2290)). * [BREAKING][rust][cli] Fungible amounts in the public API now use `AssetAmount` instead of raw `u64`: `AccountReader::get_balance` returns `AssetAmount`, `TransactionRequestBuilder::build_pswap_consume` takes `AssetAmount` fill amounts, and `tokens_to_base_units`/`base_units_to_tokens` parse to and display from `AssetAmount` (amounts above `AssetAmount::MAX` are now rejected at parse time with `TokenParseError::InvalidAmount`) ([#2290](https://github.com/0xMiden/rust-sdk/pull/2290)). diff --git a/bin/integration-tests/src/tests/config.rs b/bin/integration-tests/src/tests/config.rs index 6a429afb8f..eb553b4e7b 100644 --- a/bin/integration-tests/src/tests/config.rs +++ b/bin/integration-tests/src/tests/config.rs @@ -13,7 +13,7 @@ use miden_client::note_transport::{ NOTE_TRANSPORT_DEVNET_ENDPOINT, NOTE_TRANSPORT_TESTNET_ENDPOINT, }; -use miden_client::rpc::{Endpoint, GrpcClient}; +use miden_client::rpc::{Endpoint, GrpcClient, VerifyingRpcClient}; use miden_client::testing::common::{FilesystemKeyStore, TestClient, create_test_store_path}; use miden_client::{Felt, RemoteTransactionProver}; use miden_client_sqlite_store::ClientBuilderSqliteExt; @@ -143,7 +143,8 @@ impl ClientConfig { format!("failed to create keystore at path: {}", auth_path.to_string_lossy()) })?; - let rpc_client = Arc::new(GrpcClient::new(&rpc_endpoint, rpc_timeout)); + let rpc_client = + Arc::new(VerifyingRpcClient::new(GrpcClient::new(&rpc_endpoint, rpc_timeout))); let mut builder = ClientBuilder::new() .rpc(rpc_client) diff --git a/bin/miden-bench/src/config.rs b/bin/miden-bench/src/config.rs index ad8a808f51..06a165be00 100644 --- a/bin/miden-bench/src/config.rs +++ b/bin/miden-bench/src/config.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use miden_client::builder::ClientBuilder; use miden_client::crypto::RandomCoin; use miden_client::keystore::FilesystemKeyStore; -use miden_client::rpc::{Endpoint, GrpcClient}; +use miden_client::rpc::{Endpoint, GrpcClient, VerifyingRpcClient}; use miden_client::{Client, Felt}; use miden_client_sqlite_store::ClientBuilderSqliteExt; use rand::RngExt; @@ -48,7 +48,7 @@ pub async fn create_client( let rng_coin = RandomCoin::new(coin_seed.map(Felt::new_unchecked).into()); let client = ClientBuilder::new() - .rpc(Arc::new(GrpcClient::new(endpoint, 30_000))) + .rpc(Arc::new(VerifyingRpcClient::new(GrpcClient::new(endpoint, 30_000)))) .rng(Box::new(rng_coin)) .sqlite_store(sqlite_path) .filesystem_keystore(keystore_path.to_str().expect("keystore path should be valid UTF-8"))? diff --git a/bin/miden-cli/src/commands/account.rs b/bin/miden-cli/src/commands/account.rs index f006fb77f8..2b7cf23f17 100644 --- a/bin/miden-cli/src/commands/account.rs +++ b/bin/miden-cli/src/commands/account.rs @@ -10,7 +10,7 @@ use miden_client::account::{ }; use miden_client::address::{Address, AddressInterface, NetworkId, RoutingParameters}; use miden_client::asset::Asset; -use miden_client::rpc::{GrpcClient, NodeRpcClient}; +use miden_client::rpc::{GrpcClient, NodeRpcClient, VerifyingRpcClient}; use miden_client::transaction::{AccountComponentInterface, AccountInterface}; use miden_client::utils::base_units_to_tokens; use miden_client::{Client, PrettyPrint, ZERO}; @@ -144,8 +144,10 @@ async fn show_account( } else { println!("Account {account_id} is not tracked by the client. Fetching from the network..."); - let rpc_client = - GrpcClient::new(&rpc_config.endpoint.clone().into(), rpc_config.timeout_ms); + let rpc_client = VerifyingRpcClient::new(GrpcClient::new( + &rpc_config.endpoint.clone().into(), + rpc_config.timeout_ms, + )); let fetched_account = rpc_client.get_account_details(account_id).await.map_err(|err| { CliError::Input(format!("Unable to fetch account {account_id} from the network: {err}")) diff --git a/bin/miden-cli/src/lib.rs b/bin/miden-cli/src/lib.rs index dbd73b5154..2297456276 100644 --- a/bin/miden-cli/src/lib.rs +++ b/bin/miden-cli/src/lib.rs @@ -9,7 +9,7 @@ use miden_client::account::AccountHeader; use miden_client::builder::ClientBuilder; use miden_client::keystore::{FilesystemKeyStore, Keystore}; use miden_client::note_transport::grpc::GrpcNoteTransportClient; -use miden_client::rpc::GrpcClient; +use miden_client::rpc::{GrpcClient, VerifyingRpcClient}; use miden_client::store::{NoteFilter as ClientNoteFilter, OutputNoteRecord}; use miden_client_sqlite_store::ClientBuilderSqliteExt; @@ -134,10 +134,10 @@ impl CliClient { CliKeyStore::new(config.secret_keys_directory.clone()).map_err(CliError::KeyStore)?; // Build client with the provided configuration - let rpc_client = Arc::new( + let rpc_client = Arc::new(VerifyingRpcClient::new( GrpcClient::new(&config.rpc.endpoint.clone().into(), config.rpc.timeout_ms) .with_max_decoding_message_size(CLI_MAX_RESPONSE_SIZE_BYTES), - ); + )); let mut builder = ClientBuilder::new() .sqlite_store(config.store_filepath.clone()) diff --git a/crates/rust-client/src/builder.rs b/crates/rust-client/src/builder.rs index c637954bfc..8ec03f875c 100644 --- a/crates/rust-client/src/builder.rs +++ b/crates/rust-client/src/builder.rs @@ -18,6 +18,8 @@ use crate::keystore::Keystore; use crate::note_transport::NoteTransportClient; use crate::pswap::PswapTransactionObserver; use crate::rpc::{Endpoint, NodeRpcClient}; +#[cfg(feature = "tonic")] +use crate::rpc::{GrpcClient, VerifyingRpcClient}; use crate::store::{Store, StoreError}; use crate::transaction::{TransactionObserver, TransactionProver}; use crate::{Client, ClientError, ClientRng, ClientRngBox, grpc_support}; @@ -198,10 +200,10 @@ where pub fn for_testnet() -> Self { let endpoint = Endpoint::testnet(); Self { - rpc_api: Some(Arc::new(crate::rpc::GrpcClient::new( + rpc_api: Some(Arc::new(VerifyingRpcClient::new(GrpcClient::new( &endpoint, DEFAULT_GRPC_TIMEOUT_MS, - ))), + )))), tx_prover: Some(Arc::new(RemoteTransactionProver::new( TESTNET_PROVER_ENDPOINT.to_string(), ))), @@ -242,10 +244,10 @@ where pub fn for_devnet() -> Self { let endpoint = Endpoint::devnet(); Self { - rpc_api: Some(Arc::new(crate::rpc::GrpcClient::new( + rpc_api: Some(Arc::new(VerifyingRpcClient::new(GrpcClient::new( &endpoint, DEFAULT_GRPC_TIMEOUT_MS, - ))), + )))), tx_prover: Some(Arc::new(RemoteTransactionProver::new( DEVNET_PROVER_ENDPOINT.to_string(), ))), @@ -286,10 +288,10 @@ where pub fn for_localhost() -> Self { let endpoint = Endpoint::localhost(); Self { - rpc_api: Some(Arc::new(crate::rpc::GrpcClient::new( + rpc_api: Some(Arc::new(VerifyingRpcClient::new(GrpcClient::new( &endpoint, DEFAULT_GRPC_TIMEOUT_MS, - ))), + )))), endpoint: Some(endpoint), ..Self::default() } @@ -307,20 +309,24 @@ where } /// Sets a custom RPC client directly. + /// + /// The client is used as provided: wrap it in + /// [`VerifyingRpcClient`] to have node responses verified against the requests. #[must_use] pub fn rpc(mut self, client: Arc) -> Self { self.rpc_api = Some(client); self } - /// Sets a gRPC client from the endpoint and optional timeout. + /// Sets a gRPC client from the endpoint and optional timeout, wrapped in a + /// [`VerifyingRpcClient`] so node responses are verified against the requests. #[must_use] #[cfg(feature = "tonic")] - pub fn grpc_client(mut self, endpoint: &crate::rpc::Endpoint, timeout_ms: Option) -> Self { - self.rpc_api = Some(Arc::new(crate::rpc::GrpcClient::new( + pub fn grpc_client(mut self, endpoint: &Endpoint, timeout_ms: Option) -> Self { + self.rpc_api = Some(Arc::new(VerifyingRpcClient::new(GrpcClient::new( endpoint, timeout_ms.unwrap_or(DEFAULT_GRPC_TIMEOUT_MS), - ))); + )))); self } diff --git a/crates/rust-client/src/lib.rs b/crates/rust-client/src/lib.rs index 1e244e03ea..036ff2c5c9 100644 --- a/crates/rust-client/src/lib.rs +++ b/crates/rust-client/src/lib.rs @@ -60,7 +60,7 @@ //! //! use miden_client::builder::ClientBuilder; //! use miden_client::keystore::FilesystemKeyStore; -//! use miden_client::rpc::{Endpoint, GrpcClient}; +//! use miden_client::rpc::{Endpoint, GrpcClient, VerifyingRpcClient}; //! use miden_client_sqlite_store::SqliteStore; //! //! # pub async fn create_test_client() -> Result<(), Box> { @@ -76,7 +76,7 @@ //! //! // Instantiate the client using the builder. //! let client = ClientBuilder::new() -//! .rpc(Arc::new(GrpcClient::new(&endpoint, 10_000))) +//! .rpc(Arc::new(VerifyingRpcClient::new(GrpcClient::new(&endpoint, 10_000)))) //! .store(store) //! .authenticator(Arc::new(keystore)) //! .build() diff --git a/crates/rust-client/src/rpc/mod.rs b/crates/rust-client/src/rpc/mod.rs index 940ec3263b..fb9202b325 100644 --- a/crates/rust-client/src/rpc/mod.rs +++ b/crates/rust-client/src/rpc/mod.rs @@ -17,13 +17,14 @@ //! ## Example //! //! ```no_run -//! # use miden_client::rpc::{Endpoint, NodeRpcClient, GrpcClient}; +//! # use miden_client::rpc::{Endpoint, NodeRpcClient, GrpcClient, VerifyingRpcClient}; //! # use miden_protocol::block::BlockNumber; //! # #[tokio::main] //! # async fn main() -> Result<(), Box> { -//! // Create a gRPC client instance (assumes default endpoint configuration). +//! // Create a gRPC client instance (assumes default endpoint configuration), wrapped so that +//! // node responses are verified against the requests. //! let endpoint = Endpoint::new("https".into(), "localhost".into(), Some(57291)); -//! let mut rpc_client = GrpcClient::new(&endpoint, 1000); +//! let rpc_client = VerifyingRpcClient::new(GrpcClient::new(&endpoint, 1000)); //! //! // Fetch the latest block header (by passing None). //! let (block_header, mmr_proof) = rpc_client.get_block_header_by_number(None, true).await?; @@ -99,6 +100,9 @@ mod tonic_client; #[cfg(feature = "tonic")] pub use tonic_client::GrpcClient; +mod verifying_client; +pub use verifying_client::VerifyingRpcClient; + use crate::rpc::domain::account_vault::AccountVaultInfo; use crate::rpc::domain::transaction::TransactionRecord; use crate::store::InputNoteRecord; @@ -121,7 +125,8 @@ pub enum AccountStateAt { /// /// The implementers are responsible for connecting to the Miden node, handling endpoint /// requests/responses, and translating responses into domain objects relevant for each of the -/// endpoints. +/// endpoints. Implementations do not check that responses correspond to the method's arguments. +/// Wrap a client in [`VerifyingRpcClient`] to reject mismatched responses. #[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)] #[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))] pub trait NodeRpcClient: Send + Sync { @@ -164,12 +169,8 @@ pub trait NodeRpcClient: Send + Sync { /// /// When `None` is provided, returns info regarding the latest block. /// - /// When `block_num` is `Some`, implementations must verify that the returned header's block - /// number matches the requested one and return [`RpcError::InvalidResponse`] otherwise. - /// - /// Errors: - /// - [`RpcError::InvalidResponse`] if the node returns a header whose block number does not - /// match the requested `block_num`. + /// The returned header is not verified against the requested `block_num`; + /// [`VerifyingRpcClient`] performs that check. async fn get_block_header_by_number( &self, block_num: Option, @@ -181,12 +182,8 @@ pub trait NodeRpcClient: Send + Sync { /// /// If `include_proof` is set to true, the block proof will be included in the response. /// - /// Implementations must verify that the returned block's number matches the requested - /// `block_num` and return [`RpcError::InvalidResponse`] otherwise. - /// - /// # Errors: - /// - [`RpcError::InvalidResponse`] if the node returns a block whose number does not match the - /// requested `block_num`. + /// The returned block is not verified against the requested `block_num`; + /// [`VerifyingRpcClient`] performs that check. async fn get_block_by_number( &self, block_num: BlockNumber, @@ -205,11 +202,8 @@ pub trait NodeRpcClient: Send + Sync { /// In both cases, a [`miden_protocol::note::NoteInclusionProof`] is returned so the caller can /// verify that each note is part of the block's note tree. /// - /// Implementations must verify that every returned note's ID was present in `note_ids` and - /// return [`RpcError::InvalidResponse`] otherwise. - /// - /// Errors: - /// - [`RpcError::InvalidResponse`] if the node returns a note whose ID was not requested. + /// Returned notes are not verified to be among the requested `note_ids`; + /// [`VerifyingRpcClient`] performs that check. async fn get_notes_by_id(&self, note_ids: &[NoteId]) -> Result, RpcError>; /// Fetches the MMR delta for a given block range using the `/SyncChainMmr` RPC endpoint. @@ -276,11 +270,8 @@ pub trait NodeRpcClient: Send + Sync { /// [`NodeRpcClient::sync_notes_with_content`] to also resolve their full metadata and /// fetch public note bodies in a single follow-up call. /// - /// Implementations must verify that every returned note's tag was present in `note_tags` and - /// return [`RpcError::InvalidResponse`] otherwise. - /// - /// # Errors - /// - [`RpcError::InvalidResponse`] if the node returns a note whose tag was not requested. + /// Returned notes are not verified to carry one of the requested `note_tags`; + /// [`VerifyingRpcClient`] performs that check. async fn sync_notes( &self, block_from: BlockNumber, @@ -389,12 +380,8 @@ pub trait NodeRpcClient: Send + Sync { /// - `block_from`: The starting block number for the range (inclusive). /// - `block_to`: The ending block number for the range (inclusive). /// - /// Implementations must verify that every returned nullifier's prefix was present in `prefix` - /// and return [`RpcError::InvalidResponse`] otherwise. - /// - /// # Errors - /// - [`RpcError::InvalidResponse`] if the node returns a nullifier whose prefix was not - /// requested. + /// Returned nullifiers are not verified to carry one of the requested prefixes; + /// [`VerifyingRpcClient`] performs that check. async fn sync_nullifiers( &self, prefix: &[u16], @@ -411,11 +398,12 @@ pub trait NodeRpcClient: Send + Sync { /// /// For a fully oversize-resolved account, use [`NodeRpcClient::get_account_details`]. /// - /// Errors if the account isn't found or the block number of the requested account doesn't match + /// The response block number is not verified against the requested one; + /// [`VerifyingRpcClient`] performs that check. + /// /// # Errors /// /// - If the account isn't found in the network - /// - If the response block number does not match the requested one async fn get_account( &self, account_id: AccountId, @@ -572,12 +560,8 @@ pub trait NodeRpcClient: Send + Sync { /// Fetches the note script with the specified root, returning `None` if the node has no script /// registered for that root. /// - /// Implementations must verify that a returned script's root matches the requested `root` and - /// return [`RpcError::InvalidResponse`] otherwise; callers may rely on this invariant. - /// - /// Errors: - /// - [`RpcError::InvalidResponse`] if the node returns a script whose root does not match the - /// requested `root`. + /// A returned script's root is not verified to match the requested `root`; + /// [`VerifyingRpcClient`] performs that check. async fn get_note_script_by_root(&self, root: Word) -> Result, RpcError>; /// Fetches storage map updates for specified account and storage slots within a block range, diff --git a/crates/rust-client/src/rpc/tonic_client/mod.rs b/crates/rust-client/src/rpc/tonic_client/mod.rs index 741b2e8236..f26f3a83d3 100644 --- a/crates/rust-client/src/rpc/tonic_client/mod.rs +++ b/crates/rust-client/src/rpc/tonic_client/mod.rs @@ -38,7 +38,7 @@ use super::domain::account::{ GetAccountRequest, StorageMapFetch, }; -use super::domain::note::{CommittedNote, FetchedNote, SyncNotesBlock}; +use super::domain::note::{FetchedNote, SyncNotesBlock}; use super::domain::nullifier::NullifierUpdate; use super::generated::rpc::AccountRequest; use super::generated::rpc::account_request::AccountDetailRequest; @@ -127,60 +127,6 @@ impl BlockPagination { } } -/// Returns [`RpcError::InvalidResponse`] if any update in the `sync_nullifiers` batch carries a -/// nullifier whose prefix was not requested. -fn ensure_requested_nullifiers( - requested_prefixes: &BTreeSet, - batch: &[NullifierUpdate], -) -> Result<(), RpcError> { - for update in batch { - let prefix = update.nullifier.prefix(); - if !requested_prefixes.contains(&prefix) { - let requested = requested_prefixes - .iter() - .map(ToString::to_string) - .collect::>() - .join(", "); - return Err(RpcError::InvalidResponse(format!( - "node returned nullifier with prefix {prefix} but [{requested}] were requested" - ))); - } - } - Ok(()) -} - -/// Returns an error if any note in a `sync_notes` response carries a tag that was not requested. -fn ensure_requested_tags( - requested: &BTreeSet, - returned: impl IntoIterator, -) -> Result<(), RpcError> { - for tag in returned { - if !requested.contains(&tag) { - let list = requested.iter().map(ToString::to_string).collect::>().join(", "); - return Err(RpcError::InvalidResponse(format!( - "node returned note with tag {tag} but [{list}] were requested" - ))); - } - } - Ok(()) -} - -/// Returns an error if any note in a `GetNotesById` response has an ID that was not requested. -fn ensure_requested_note_ids( - requested: &BTreeSet, - returned: impl IntoIterator, -) -> Result<(), RpcError> { - for id in returned { - if !requested.contains(&id) { - let list = requested.iter().map(ToString::to_string).collect::>().join(", "); - return Err(RpcError::InvalidResponse(format!( - "node returned note {id} but [{list}] were requested" - ))); - } - } - Ok(()) -} - // GRPC CLIENT // ================================================================================================ @@ -487,15 +433,6 @@ impl NodeRpcClient for GrpcClient { .ok_or(RpcError::ExpectedDataMissing("BlockHeader".into()))? .try_into()?; - if let Some(requested) = block_num - && block_header.block_num() != requested - { - return Err(RpcError::InvalidResponse(format!( - "node returned header for block {} but block {requested} was requested", - block_header.block_num(), - ))); - } - let mmr_proof = if include_mmr_proof { let forest = response .chain_length @@ -522,7 +459,6 @@ impl NodeRpcClient for GrpcClient { async fn get_notes_by_id(&self, note_ids: &[NoteId]) -> Result, RpcError> { let limits = self.get_rpc_limits().await?; - let requested_ids: BTreeSet = note_ids.iter().copied().collect(); let mut notes = Vec::with_capacity(note_ids.len()); for chunk in note_ids.chunks(limits.note_ids_limit as usize) { let request = proto::note::NoteIdList { @@ -543,8 +479,6 @@ impl NodeRpcClient for GrpcClient { .map(FetchedNote::try_from) .collect::, RpcConversionError>>()?; - ensure_requested_note_ids(&requested_ids, response_notes.iter().map(FetchedNote::id))?; - notes.extend(response_notes); } Ok(notes) @@ -579,7 +513,6 @@ impl NodeRpcClient for GrpcClient { /// This function will return an error if: /// /// - The requested Account isn't returned by the node. - /// - The block number of the requested Account doesn't match the response block number. /// - There was an error sending the request to the node. /// - The answer had a `None` for one of the expected fields. /// - There is an error during storage deserialization. @@ -644,16 +577,6 @@ impl NodeRpcClient for GrpcClient { .block_num .into(); - if let Some(requested) = block_num - && requested.block_num != response_block_num.as_u32() - { - return Err(RpcError::InvalidResponse(format!( - "node returned header for block {} but block {} was requested", - response_block_num.as_u32(), - requested.block_num - ))); - } - // For accounts with public state, details should be present when requested let headers = if account_witness.id().is_public() { let details = response @@ -696,7 +619,6 @@ impl NodeRpcClient for GrpcClient { for chunk in tags.chunks(limits.note_tags_limit as usize) { let proto_tags: Vec = chunk.iter().map(|&t| t.into()).collect(); - let requested_tags: BTreeSet = chunk.iter().copied().collect(); let mut pagination = BlockPagination::new(block_from, block_to); loop { @@ -724,10 +646,6 @@ impl NodeRpcClient for GrpcClient { for proto_block in response.blocks { let block: SyncNotesBlock = proto_block.try_into()?; - ensure_requested_tags( - &requested_tags, - block.notes.values().map(CommittedNote::tag), - )?; let bn = block.block_header.block_num(); if let Some(existing) = merged_blocks.get_mut(&bn) { for (id, note) in block.notes { @@ -761,7 +679,6 @@ impl NodeRpcClient for GrpcClient { // violating the RPC limit. for chunk in prefixes.chunks(limits.nullifiers_limit as usize) { let proto_prefixes: Vec = chunk.iter().map(|&x| u32::from(x)).collect(); - let requested_prefixes: BTreeSet = chunk.iter().copied().collect(); let mut pagination = BlockPagination::new(block_from, block_to); loop { @@ -789,7 +706,6 @@ impl NodeRpcClient for GrpcClient { .collect::, _>>() .map_err(|err| RpcError::InvalidResponse(err.to_string()))?; - ensure_requested_nullifiers(&requested_prefixes, &batch_nullifiers)?; all_nullifiers.extend(batch_nullifiers); let page = response.pagination_info.ok_or(RpcError::ExpectedDataMissing( @@ -827,13 +743,6 @@ impl NodeRpcClient for GrpcClient { "GetBlockByNumberResponse.block".to_string(), ))?)?; - if block.header().block_num() != block_num { - return Err(RpcError::InvalidResponse(format!( - "node returned header for block {} but block {block_num} was requested", - block.header().block_num(), - ))); - } - Ok(block) } @@ -852,13 +761,6 @@ impl NodeRpcClient for GrpcClient { }; let note_script = NoteScript::try_from(script)?; - let fetched_root = note_script.root(); - if Word::from(fetched_root) != root { - return Err(RpcError::InvalidResponse(format!( - "node returned note script with root {fetched_root} for requested root {root}", - ))); - } - Ok(Some(note_script)) } @@ -1096,24 +998,12 @@ impl From<&Status> for GrpcError { #[cfg(test)] mod tests { - use core::slice; use std::boxed::Box; - use std::collections::BTreeSet; + use miden_protocol::Word; use miden_protocol::block::BlockNumber; - use miden_protocol::note::{NoteId, NoteTag, Nullifier}; - use miden_protocol::{Felt, Word}; - - use super::{ - BlockPagination, - DEFAULT_MAX_RESPONSE_SIZE_BYTES, - GrpcClient, - NullifierUpdate, - PaginationResult, - ensure_requested_note_ids, - ensure_requested_nullifiers, - ensure_requested_tags, - }; + + use super::{BlockPagination, DEFAULT_MAX_RESPONSE_SIZE_BYTES, GrpcClient, PaginationResult}; use crate::alloc::string::ToString; use crate::rpc::{Endpoint, NodeRpcClient, RpcError}; @@ -1320,65 +1210,4 @@ mod tests { .expect("testnet status with caller auth header must succeed"); assert!(!status.version.is_empty(), "status must include a server version"); } - - fn nullifier_with_prefix(prefix: u16) -> Nullifier { - Nullifier::from_raw(Word::new([ - Felt::ZERO, - Felt::ZERO, - Felt::ZERO, - Felt::new_unchecked(u64::from(prefix) << 48), - ])) - } - - #[test] - fn verify_requested_nullifiers_rejects_unrequested_prefix() { - let requested = NullifierUpdate { - nullifier: nullifier_with_prefix(0x1234), - block_num: 1u32.into(), - }; - let unrequested = NullifierUpdate { - nullifier: nullifier_with_prefix(0xabcd), - block_num: 2u32.into(), - }; - - let requested_prefixes: BTreeSet = BTreeSet::from([0x1234]); - - ensure_requested_nullifiers(&requested_prefixes, slice::from_ref(&requested)) - .expect("requested prefix must be accepted"); - - let err = ensure_requested_nullifiers(&requested_prefixes, &[requested, unrequested]) - .expect_err("unrequested prefix must be rejected"); - assert!(matches!(err, RpcError::InvalidResponse(_))); - } - - #[test] - fn ensure_requested_tags_rejects_unrequested() { - let requested = NoteTag::new(1); - let other = NoteTag::new(2); - let requested_set = BTreeSet::from([requested]); - - ensure_requested_tags(&requested_set, [requested]).expect("requested tag must be accepted"); - - let err = ensure_requested_tags(&requested_set, [other]) - .expect_err("unrequested tag must be rejected"); - assert!(matches!(err, RpcError::InvalidResponse(_))); - } - - fn note_id(n: u32) -> NoteId { - NoteId::from_raw(Word::from([n, 0, 0, 0])) - } - - #[test] - fn ensure_requested_note_ids_rejects_unrequested() { - let requested = note_id(1); - let other = note_id(2); - let requested_set = BTreeSet::from([requested]); - - ensure_requested_note_ids(&requested_set, [requested]) - .expect("requested note id must be accepted"); - - let err = ensure_requested_note_ids(&requested_set, [other]) - .expect_err("unrequested note id must be rejected"); - assert!(matches!(err, RpcError::InvalidResponse(_))); - } } diff --git a/crates/rust-client/src/rpc/verifying_client.rs b/crates/rust-client/src/rpc/verifying_client.rs new file mode 100644 index 0000000000..16edcf3111 --- /dev/null +++ b/crates/rust-client/src/rpc/verifying_client.rs @@ -0,0 +1,569 @@ +use alloc::boxed::Box; +use alloc::collections::BTreeSet; +use alloc::string::ToString; +use alloc::vec::Vec; + +use miden_protocol::Word; +use miden_protocol::account::AccountId; +use miden_protocol::address::NetworkId; +use miden_protocol::batch::{ProposedBatch, ProvenBatch}; +use miden_protocol::block::{BlockHeader, BlockNumber, ProvenBlock}; +use miden_protocol::crypto::merkle::mmr::MmrProof; +use miden_protocol::note::{NoteId, NoteScript, NoteTag}; +use miden_protocol::transaction::{ProvenTransaction, TransactionInputs}; + +use super::domain::account::{AccountProof, GetAccountRequest}; +use super::domain::account_vault::AccountVaultInfo; +use super::domain::note::{CommittedNote, FetchedNote, SyncNotesBlock}; +use super::domain::nullifier::NullifierUpdate; +use super::domain::storage_map::StorageMapInfo; +use super::domain::sync::{ChainMmrInfo, SyncTarget}; +use super::domain::transaction::TransactionRecord; +use super::{ + AccountStateAt, + NetworkNoteStatusInfo, + NodeRpcClient, + RpcError, + RpcLimits, + RpcStatusInfo, +}; + +// RESPONSE VERIFICATION HELPERS +// ================================================================================================ + +/// Returns [`RpcError::InvalidResponse`] if `requested` is `Some` and `returned` does not equal it. +fn verify_block_num(requested: Option, returned: BlockNumber) -> Result<(), RpcError> { + if let Some(requested) = requested + && returned != requested + { + return Err(RpcError::InvalidResponse(format!( + "node returned block {returned} but block {requested} was requested" + ))); + } + Ok(()) +} + +/// Returns [`RpcError::InvalidResponse`] if any returned note ID was not in `requested`. +fn verify_note_ids( + requested: &BTreeSet, + returned: impl IntoIterator, +) -> Result<(), RpcError> { + for id in returned { + if !requested.contains(&id) { + let list = requested.iter().map(ToString::to_string).collect::>().join(", "); + return Err(RpcError::InvalidResponse(format!( + "node returned note {id} but [{list}] were requested" + ))); + } + } + Ok(()) +} + +/// Returns [`RpcError::InvalidResponse`] if any returned note tag was not in `requested`. +fn verify_note_tags( + requested: &BTreeSet, + returned: impl IntoIterator, +) -> Result<(), RpcError> { + for tag in returned { + if !requested.contains(&tag) { + let list = requested.iter().map(ToString::to_string).collect::>().join(", "); + return Err(RpcError::InvalidResponse(format!( + "node returned note with tag {tag} but [{list}] were requested" + ))); + } + } + Ok(()) +} + +/// Returns [`RpcError::InvalidResponse`] if any update carries a nullifier whose prefix was not in +/// `requested_prefixes`. +fn verify_nullifier_prefixes( + requested_prefixes: &BTreeSet, + batch: &[NullifierUpdate], +) -> Result<(), RpcError> { + for update in batch { + let prefix = update.nullifier.prefix(); + if !requested_prefixes.contains(&prefix) { + let requested = requested_prefixes + .iter() + .map(ToString::to_string) + .collect::>() + .join(", "); + return Err(RpcError::InvalidResponse(format!( + "node returned nullifier with prefix {prefix} but [{requested}] were requested" + ))); + } + } + Ok(()) +} + +/// Returns [`RpcError::InvalidResponse`] if `script`'s root does not equal the `requested` root. +fn verify_note_script_root(requested: Word, script: &NoteScript) -> Result<(), RpcError> { + let fetched_root = script.root(); + if Word::from(fetched_root) != requested { + return Err(RpcError::InvalidResponse(format!( + "node returned note script with root {fetched_root} for requested root {requested}" + ))); + } + Ok(()) +} + +// VERIFYING RPC CLIENT +// ================================================================================================ + +/// A [`NodeRpcClient`] wrapper that verifies that responses correspond to the method's arguments, +/// rejecting mismatches with [`RpcError::InvalidResponse`]: +/// +/// - [`get_block_header_by_number`](NodeRpcClient::get_block_header_by_number) and +/// [`get_block_by_number`](NodeRpcClient::get_block_by_number): the returned block's number must +/// match the requested one. +/// - [`get_notes_by_id`](NodeRpcClient::get_notes_by_id): every returned note's ID must have been +/// requested. +/// - [`sync_notes`](NodeRpcClient::sync_notes): every returned note's tag must have been requested. +/// - [`sync_nullifiers`](NodeRpcClient::sync_nullifiers): every returned nullifier's prefix must +/// have been requested. +/// - [`get_account`](NodeRpcClient::get_account): when the state at a specific block was requested, +/// the response must be for that block. +/// - [`get_note_script_by_root`](NodeRpcClient::get_note_script_by_root): a returned script's root +/// must match the requested one. +/// +/// All other methods delegate to the wrapped client unchanged. +pub struct VerifyingRpcClient(T); + +impl VerifyingRpcClient { + /// Wraps `client` so that its responses are verified against the request. + pub fn new(client: T) -> Self { + Self(client) + } +} + +#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)] +#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))] +impl NodeRpcClient for VerifyingRpcClient { + async fn set_genesis_commitment(&self, commitment: Word) -> Result<(), RpcError> { + self.0.set_genesis_commitment(commitment).await + } + + fn has_genesis_commitment(&self) -> Option { + self.0.has_genesis_commitment() + } + + async fn submit_proven_transaction( + &self, + proven_transaction: ProvenTransaction, + transaction_inputs: TransactionInputs, + ) -> Result { + self.0.submit_proven_transaction(proven_transaction, transaction_inputs).await + } + + async fn submit_proven_batch( + &self, + proven_batch: ProvenBatch, + proposed_batch: ProposedBatch, + transaction_inputs: Vec, + ) -> Result { + self.0 + .submit_proven_batch(proven_batch, proposed_batch, transaction_inputs) + .await + } + + async fn get_block_header_by_number( + &self, + block_num: Option, + include_mmr_proof: bool, + ) -> Result<(BlockHeader, Option), RpcError> { + let (header, mmr_proof) = + self.0.get_block_header_by_number(block_num, include_mmr_proof).await?; + verify_block_num(block_num, header.block_num())?; + Ok((header, mmr_proof)) + } + + async fn get_block_by_number( + &self, + block_num: BlockNumber, + include_proof: bool, + ) -> Result { + let block = self.0.get_block_by_number(block_num, include_proof).await?; + verify_block_num(Some(block_num), block.header().block_num())?; + Ok(block) + } + + async fn get_notes_by_id(&self, note_ids: &[NoteId]) -> Result, RpcError> { + let notes = self.0.get_notes_by_id(note_ids).await?; + let requested: BTreeSet = note_ids.iter().copied().collect(); + verify_note_ids(&requested, notes.iter().map(FetchedNote::id))?; + Ok(notes) + } + + async fn sync_chain_mmr( + &self, + current_block_height: BlockNumber, + upper_bound: SyncTarget, + ) -> Result { + self.0.sync_chain_mmr(current_block_height, upper_bound).await + } + + async fn sync_notes( + &self, + block_from: BlockNumber, + block_to: BlockNumber, + note_tags: &BTreeSet, + ) -> Result, RpcError> { + let blocks = self.0.sync_notes(block_from, block_to, note_tags).await?; + verify_note_tags( + note_tags, + blocks.iter().flat_map(|block| block.notes.values().map(CommittedNote::tag)), + )?; + Ok(blocks) + } + + async fn sync_nullifiers( + &self, + prefix: &[u16], + block_from: BlockNumber, + block_to: BlockNumber, + ) -> Result, RpcError> { + let nullifiers = self.0.sync_nullifiers(prefix, block_from, block_to).await?; + let requested: BTreeSet = prefix.iter().copied().collect(); + verify_nullifier_prefixes(&requested, &nullifiers)?; + Ok(nullifiers) + } + + async fn get_account( + &self, + account_id: AccountId, + request: GetAccountRequest, + ) -> Result<(BlockNumber, AccountProof), RpcError> { + let requested = match request.at { + AccountStateAt::Block(number) => Some(number), + AccountStateAt::ChainTip => None, + }; + let (block_num, proof) = self.0.get_account(account_id, request).await?; + verify_block_num(requested, block_num)?; + Ok((block_num, proof)) + } + + async fn get_note_script_by_root(&self, root: Word) -> Result, RpcError> { + let script = self.0.get_note_script_by_root(root).await?; + if let Some(script) = &script { + verify_note_script_root(root, script)?; + } + Ok(script) + } + + async fn sync_storage_maps( + &self, + block_from: BlockNumber, + block_to: BlockNumber, + account_id: AccountId, + ) -> Result { + self.0.sync_storage_maps(block_from, block_to, account_id).await + } + + async fn sync_account_vault( + &self, + block_from: BlockNumber, + block_to: BlockNumber, + account_id: AccountId, + ) -> Result { + self.0.sync_account_vault(block_from, block_to, account_id).await + } + + async fn sync_transactions( + &self, + block_from: BlockNumber, + block_to: BlockNumber, + account_ids: Vec, + ) -> Result, RpcError> { + self.0.sync_transactions(block_from, block_to, account_ids).await + } + + async fn get_network_id(&self) -> Result { + self.0.get_network_id().await + } + + async fn get_rpc_limits(&self) -> Result { + self.0.get_rpc_limits().await + } + + fn has_rpc_limits(&self) -> Option { + self.0.has_rpc_limits() + } + + async fn set_rpc_limits(&self, limits: RpcLimits) { + self.0.set_rpc_limits(limits).await; + } + + async fn get_status_unversioned(&self) -> Result { + self.0.get_status_unversioned().await + } + + async fn get_network_note_status( + &self, + note_id: NoteId, + ) -> Result { + self.0.get_network_note_status(note_id).await + } +} + +// TESTS +// ================================================================================================ + +#[cfg(test)] +mod tests { + use core::slice; + use std::boxed::Box; + use std::collections::BTreeSet; + use std::vec::Vec; + + use miden_protocol::account::AccountId; + use miden_protocol::address::NetworkId; + use miden_protocol::batch::{ProposedBatch, ProvenBatch}; + use miden_protocol::block::{BlockHeader, BlockNumber, ProvenBlock}; + use miden_protocol::crypto::merkle::mmr::MmrProof; + use miden_protocol::note::{NoteId, NoteScript, NoteTag, Nullifier}; + use miden_protocol::transaction::{ProvenTransaction, TransactionInputs}; + use miden_protocol::{Felt, Word}; + + use super::{VerifyingRpcClient, verify_note_ids, verify_note_tags, verify_nullifier_prefixes}; + use crate::rpc::domain::account::{AccountProof, GetAccountRequest}; + use crate::rpc::domain::account_vault::AccountVaultInfo; + use crate::rpc::domain::note::{FetchedNote, SyncNotesBlock}; + use crate::rpc::domain::nullifier::NullifierUpdate; + use crate::rpc::domain::storage_map::StorageMapInfo; + use crate::rpc::domain::sync::{ChainMmrInfo, SyncTarget}; + use crate::rpc::domain::transaction::TransactionRecord; + use crate::rpc::{NetworkNoteStatusInfo, NodeRpcClient, RpcError, RpcLimits, RpcStatusInfo}; + + fn nullifier_with_prefix(prefix: u16) -> Nullifier { + Nullifier::from_raw(Word::new([ + Felt::ZERO, + Felt::ZERO, + Felt::ZERO, + Felt::new_unchecked(u64::from(prefix) << 48), + ])) + } + + #[test] + fn verify_nullifier_prefixes_rejects_unrequested() { + let requested = NullifierUpdate { + nullifier: nullifier_with_prefix(0x1234), + block_num: 1u32.into(), + }; + let unrequested = NullifierUpdate { + nullifier: nullifier_with_prefix(0xabcd), + block_num: 2u32.into(), + }; + + let requested_prefixes: BTreeSet = BTreeSet::from([0x1234]); + + verify_nullifier_prefixes(&requested_prefixes, slice::from_ref(&requested)) + .expect("requested prefix must be accepted"); + + let err = verify_nullifier_prefixes(&requested_prefixes, &[requested, unrequested]) + .expect_err("unrequested prefix must be rejected"); + assert!(matches!(err, RpcError::InvalidResponse(_))); + } + + #[test] + fn verify_note_tags_rejects_unrequested() { + let requested = NoteTag::new(1); + let other = NoteTag::new(2); + let requested_set = BTreeSet::from([requested]); + + verify_note_tags(&requested_set, [requested]).expect("requested tag must be accepted"); + + let err = verify_note_tags(&requested_set, [other]) + .expect_err("unrequested tag must be rejected"); + assert!(matches!(err, RpcError::InvalidResponse(_))); + } + + fn note_id(n: u32) -> NoteId { + NoteId::from_raw(Word::from([n, 0, 0, 0])) + } + + #[test] + fn verify_note_ids_rejects_unrequested() { + let requested = note_id(1); + let other = note_id(2); + let requested_set = BTreeSet::from([requested]); + + verify_note_ids(&requested_set, [requested]).expect("requested note id must be accepted"); + + let err = verify_note_ids(&requested_set, [other]) + .expect_err("unrequested note id must be rejected"); + assert!(matches!(err, RpcError::InvalidResponse(_))); + } + + /// A transport whose `sync_nullifiers` always returns a fixed batch, regardless of the + /// requested prefixes. Every other method is unreachable in these tests. + struct FixedNullifiersTransport(Vec); + + #[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)] + #[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))] + impl NodeRpcClient for FixedNullifiersTransport { + async fn set_genesis_commitment(&self, _commitment: Word) -> Result<(), RpcError> { + unimplemented!() + } + + fn has_genesis_commitment(&self) -> Option { + unimplemented!() + } + + async fn submit_proven_transaction( + &self, + _proven_transaction: ProvenTransaction, + _transaction_inputs: TransactionInputs, + ) -> Result { + unimplemented!() + } + + async fn submit_proven_batch( + &self, + _proven_batch: ProvenBatch, + _proposed_batch: ProposedBatch, + _transaction_inputs: Vec, + ) -> Result { + unimplemented!() + } + + async fn get_block_header_by_number( + &self, + _block_num: Option, + _include_mmr_proof: bool, + ) -> Result<(BlockHeader, Option), RpcError> { + unimplemented!() + } + + async fn get_block_by_number( + &self, + _block_num: BlockNumber, + _include_proof: bool, + ) -> Result { + unimplemented!() + } + + async fn get_notes_by_id( + &self, + _note_ids: &[NoteId], + ) -> Result, RpcError> { + unimplemented!() + } + + async fn sync_chain_mmr( + &self, + _current_block_height: BlockNumber, + _upper_bound: SyncTarget, + ) -> Result { + unimplemented!() + } + + async fn sync_notes( + &self, + _block_from: BlockNumber, + _block_to: BlockNumber, + _note_tags: &BTreeSet, + ) -> Result, RpcError> { + unimplemented!() + } + + async fn sync_nullifiers( + &self, + _prefix: &[u16], + _block_from: BlockNumber, + _block_to: BlockNumber, + ) -> Result, RpcError> { + Ok(self.0.clone()) + } + + async fn get_account( + &self, + _account_id: AccountId, + _request: GetAccountRequest, + ) -> Result<(BlockNumber, AccountProof), RpcError> { + unimplemented!() + } + + async fn get_note_script_by_root( + &self, + _root: Word, + ) -> Result, RpcError> { + unimplemented!() + } + + async fn sync_storage_maps( + &self, + _block_from: BlockNumber, + _block_to: BlockNumber, + _account_id: AccountId, + ) -> Result { + unimplemented!() + } + + async fn sync_account_vault( + &self, + _block_from: BlockNumber, + _block_to: BlockNumber, + _account_id: AccountId, + ) -> Result { + unimplemented!() + } + + async fn sync_transactions( + &self, + _block_from: BlockNumber, + _block_to: BlockNumber, + _account_ids: Vec, + ) -> Result, RpcError> { + unimplemented!() + } + + async fn get_network_id(&self) -> Result { + unimplemented!() + } + + async fn get_rpc_limits(&self) -> Result { + unimplemented!() + } + + fn has_rpc_limits(&self) -> Option { + unimplemented!() + } + + async fn set_rpc_limits(&self, _limits: RpcLimits) { + unimplemented!() + } + + async fn get_status_unversioned(&self) -> Result { + unimplemented!() + } + + async fn get_network_note_status( + &self, + _note_id: NoteId, + ) -> Result { + unimplemented!() + } + } + + #[tokio::test] + async fn verifying_client_rejects_mismatched_responses() { + let update = NullifierUpdate { + nullifier: nullifier_with_prefix(0xabcd), + block_num: 1u32.into(), + }; + let client = VerifyingRpcClient::new(FixedNullifiersTransport(vec![update])); + + let nullifiers = client + .sync_nullifiers(&[0xabcd], 0u32.into(), 1u32.into()) + .await + .expect("requested prefix must be accepted"); + assert_eq!(nullifiers.len(), 1); + + let err = client + .sync_nullifiers(&[0x1234], 0u32.into(), 1u32.into()) + .await + .expect_err("unrequested prefix must be rejected"); + assert!(matches!(err, RpcError::InvalidResponse(_))); + } +}