diff --git a/crates/rpc/Cargo.toml b/crates/rpc/Cargo.toml index 8c483ac319..9d2902612f 100644 --- a/crates/rpc/Cargo.toml +++ b/crates/rpc/Cargo.toml @@ -23,6 +23,7 @@ miden-node-proto = { workspace = true } miden-node-proto-build = { workspace = true } miden-node-utils = { workspace = true } miden-protocol = { default-features = true, workspace = true } +miden-standards = { workspace = true } miden-tx = { features = ["concurrent"], workspace = true } miden-tx-batch-prover = { workspace = true } semver = { version = "1.0" } @@ -41,7 +42,6 @@ url = { workspace = true } miden-node-store = { features = ["rocksdb"], workspace = true } miden-node-utils = { features = ["testing", "tracing-forest"], workspace = true } miden-protocol = { default-features = true, features = ["testing"], workspace = true } -miden-standards = { workspace = true } reqwest = { workspace = true } rstest = { workspace = true } tempfile = { workspace = true } diff --git a/crates/rpc/src/server/api.rs b/crates/rpc/src/server/api.rs index 4a11d17a54..3a39006c21 100644 --- a/crates/rpc/src/server/api.rs +++ b/crates/rpc/src/server/api.rs @@ -24,6 +24,7 @@ use miden_node_utils::limiter::{ QueryParamNullifierPrefixLimit, QueryParamStorageMapKeyTotalLimit, }; +use miden_protocol::account::delta::AccountUpdateDetails; use miden_protocol::batch::{ProposedBatch, ProvenBatch}; use miden_protocol::block::{BlockHeader, BlockNumber}; use miden_protocol::transaction::{ @@ -34,6 +35,7 @@ use miden_protocol::transaction::{ }; use miden_protocol::utils::serde::{Deserializable, Serializable}; use miden_protocol::{MIN_PROOF_SECURITY_LEVEL, Word}; +use miden_standards::account::auth::NetworkAccountNoteAllowlist; use miden_tx::TransactionVerifier; use miden_tx_batch_prover::LocalBatchProver; use tonic::{IntoRequest, Request, Response, Status}; @@ -404,6 +406,7 @@ impl api_server::Api for RpcService { "Network transactions may not be submitted by users yet", )); } + reject_existing_non_network_account_becoming_network(&tx)?; let tx_verifier = TransactionVerifier::new(MIN_PROOF_SECURITY_LEVEL); @@ -471,6 +474,7 @@ impl api_server::Api for RpcService { "Network transactions may not be submitted by users yet", )); } + reject_existing_non_network_account_becoming_network(tx)?; } // Verify batch transaction proofs. @@ -627,6 +631,29 @@ fn strip_output_note_decorators<'a>( }) } +fn reject_existing_non_network_account_becoming_network( + tx: &ProvenTransaction, +) -> Result<(), Status> { + let is_existing_account = !tx.account_update().initial_state_commitment().is_empty(); + if !is_existing_account || tx.account_id().is_network() { + return Ok(()); + } + + let AccountUpdateDetails::Delta(delta) = tx.account_update().details() else { + return Ok(()); + }; + + if delta.storage().get(NetworkAccountNoteAllowlist::slot_name()).is_some() { + return Err(Status::invalid_argument(format!( + "Existing non-network account {} cannot add the network account allowlist storage slot {}", + tx.account_id(), + NetworkAccountNoteAllowlist::slot_name() + ))); + } + + Ok(()) +} + // LIMIT HELPERS // ================================================================================================ @@ -640,6 +667,95 @@ fn check(n: usize) -> Result<(), Status> { ::check(n).map_err(out_of_range_error) } +#[cfg(test)] +mod tests { + use super::*; + + use miden_node_utils::fee::test_fee; + use miden_protocol::account::delta::AccountStorageDelta; + use miden_protocol::account::{ + AccountDelta, AccountId, AccountIdVersion, AccountStorageMode, AccountType, + AccountVaultDelta, StorageMapKey, + }; + use miden_protocol::transaction::InputNoteCommitment; + use miden_protocol::vm::ExecutionProof; + use miden_protocol::{Felt, Word}; + + fn proven_tx_with_delta( + account_id: AccountId, + initial_state_commitment: Word, + delta: AccountDelta, + ) -> ProvenTransaction { + let account_update = TxAccountUpdate::new( + account_id, + initial_state_commitment, + Word::from([2, 0, 0, 0u32]), + delta.to_commitment(), + AccountUpdateDetails::Delta(delta), + ) + .expect("account update should be valid"); + + ProvenTransaction::new( + account_update, + Vec::::new(), + Vec::::new(), + 0.into(), + Word::default(), + test_fee(), + u32::MAX.into(), + ExecutionProof::new_dummy(), + ) + .expect("proven transaction should be valid") + } + + fn account_id(storage_mode: AccountStorageMode) -> AccountId { + AccountId::dummy( + [0; 15], + AccountIdVersion::Version0, + AccountType::RegularAccountImmutableCode, + storage_mode, + ) + } + + fn delta_with_network_account_allowlist_slot(account_id: AccountId) -> AccountDelta { + let mut storage_delta = AccountStorageDelta::new(); + storage_delta + .set_map_item( + NetworkAccountNoteAllowlist::slot_name().clone(), + StorageMapKey::empty(), + Word::from([1, 0, 0, 0u32]), + ) + .expect("network account allowlist slot should accept map updates"); + + AccountDelta::new(account_id, storage_delta, AccountVaultDelta::default(), Felt::ONE) + .expect("account delta should be valid") + } + + #[test] + fn rejects_existing_non_network_account_adding_network_allowlist_slot() { + let account_id = account_id(AccountStorageMode::Public); + let delta = delta_with_network_account_allowlist_slot(account_id); + let tx = proven_tx_with_delta(account_id, Word::from([1, 0, 0, 0u32]), delta); + + let err = reject_existing_non_network_account_becoming_network(&tx).unwrap_err(); + + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert!( + err.message().contains("cannot add the network account allowlist storage slot"), + "unexpected error: {err}" + ); + } + + #[test] + fn permits_existing_network_account_with_network_allowlist_slot() { + let account_id = account_id(AccountStorageMode::Network); + let delta = delta_with_network_account_allowlist_slot(account_id); + let tx = proven_tx_with_delta(account_id, Word::from([1, 0, 0, 0u32]), delta); + + reject_existing_non_network_account_becoming_network(&tx).unwrap(); + } +} + /// Helper to build an [`EndpointLimits`](proto::rpc::EndpointLimits) from (name, limit) pairs. fn endpoint_limits(params: &[(&str, usize)]) -> proto::rpc::EndpointLimits { proto::rpc::EndpointLimits { diff --git a/crates/store/src/state/apply_block.rs b/crates/store/src/state/apply_block.rs index d8aab21e70..52a8efc23d 100644 --- a/crates/store/src/state/apply_block.rs +++ b/crates/store/src/state/apply_block.rs @@ -130,12 +130,16 @@ impl State { } // compute update for nullifier tree - let nullifier_tree_update = inner - .nullifier_tree - .compute_mutations( - body.created_nullifiers().iter().map(|nullifier| (*nullifier, block_num)), - ) - .map_err(InvalidBlockError::NewBlockNullifierAlreadySpent)?; + let nullifier_tree_update = tokio::task::block_in_place(|| { + inner + .nullifier_tree + .compute_mutations( + body.created_nullifiers() + .iter() + .map(|nullifier| (*nullifier, block_num)), + ) + .map_err(InvalidBlockError::NewBlockNullifierAlreadySpent) + })?; if nullifier_tree_update.as_mutation_set().root() != header.nullifier_root() { // We do our best here to notify the serve routine, if it doesn't care (dropped the @@ -147,21 +151,25 @@ impl State { } // compute update for account tree - let account_tree_update = inner - .account_tree - .compute_mutations( - body.updated_accounts() - .iter() - .map(|update| (update.account_id(), update.final_state_commitment())), - ) - .map_err(|e| match e { - HistoricalError::AccountTreeError(err) => { - InvalidBlockError::NewBlockDuplicateAccountIdPrefix(err) - }, - HistoricalError::MerkleError(_) => { - panic!("Unexpected MerkleError during account tree mutation computation") - }, - })?; + let account_tree_update = tokio::task::block_in_place(|| { + inner + .account_tree + .compute_mutations( + body.updated_accounts() + .iter() + .map(|update| (update.account_id(), update.final_state_commitment())), + ) + .map_err(|e| match e { + HistoricalError::AccountTreeError(err) => { + InvalidBlockError::NewBlockDuplicateAccountIdPrefix(err) + }, + HistoricalError::MerkleError(_) => { + panic!( + "Unexpected MerkleError during account tree mutation computation" + ) + }, + }) + })?; if account_tree_update.as_mutation_set().root() != header.account_root() { let _ = self.termination_ask.try_send(ApplyBlockError::InvalidBlockError( @@ -275,15 +283,20 @@ impl State { .await? .map_err(|err| ApplyBlockError::DbUpdateTaskFailed(err.as_report()))?; - // Update the in-memory data structures after successful commit of the DB transaction - inner - .nullifier_tree - .apply_mutations(nullifier_tree_update) - .expect("Unreachable: old nullifier tree root must be checked before this step"); - inner - .account_tree - .apply_mutations(account_tree_update) - .expect("Unreachable: old account tree root must be checked before this step"); + // Update the in-memory data structures after successful commit of the DB transaction. + // These write to RocksDB and must run on a blocking-capable thread. + tokio::task::block_in_place(|| { + inner + .nullifier_tree + .apply_mutations(nullifier_tree_update) + .expect( + "Unreachable: old nullifier tree root must be checked before this step", + ); + inner + .account_tree + .apply_mutations(account_tree_update) + .expect("Unreachable: old account tree root must be checked before this step"); + }); inner.blockchain.push(block_commitment); diff --git a/crates/store/src/state/mod.rs b/crates/store/src/state/mod.rs index 74a41f1807..3fad120957 100644 --- a/crates/store/src/state/mod.rs +++ b/crates/store/src/state/mod.rs @@ -265,11 +265,13 @@ impl State { #[instrument(level = "debug", target = COMPONENT, skip_all, ret)] pub async fn check_nullifiers(&self, nullifiers: &[Nullifier]) -> Vec { let inner = self.inner.read().await; - nullifiers - .iter() - .map(|n| inner.nullifier_tree.open(n)) - .map(NullifierWitness::into_proof) - .collect() + tokio::task::block_in_place(|| { + nullifiers + .iter() + .map(|n| inner.nullifier_tree.open(n)) + .map(NullifierWitness::into_proof) + .collect() + }) } /// Queries a list of notes from the database. @@ -552,19 +554,27 @@ impl State { ); // Fetch witnesses for all accounts. - let account_witnesses = account_ids - .iter() - .copied() - .map(|account_id| (account_id, inner.account_tree.open_latest(account_id))) - .collect::>(); + // open_latest() reads from RocksDB for deep subtrees — use block_in_place. + let account_witnesses: BTreeMap = + tokio::task::block_in_place(|| { + account_ids + .iter() + .copied() + .map(|account_id| (account_id, inner.account_tree.open_latest(account_id))) + .collect() + }); // Fetch witnesses for all nullifiers. We don't check whether the nullifiers are spent or // not as this is done as part of proposing the block. - let nullifier_witnesses: BTreeMap = nullifiers - .iter() - .copied() - .map(|nullifier| (nullifier, inner.nullifier_tree.open(&nullifier))) - .collect(); + // open() reads from RocksDB for deep subtrees — use block_in_place. + let nullifier_witnesses: BTreeMap = + tokio::task::block_in_place(|| { + nullifiers + .iter() + .copied() + .map(|nullifier| (nullifier, inner.nullifier_tree.open(&nullifier))) + .collect() + }); Ok((latest_block_number, account_witnesses, nullifier_witnesses, partial_mmr)) } @@ -581,10 +591,13 @@ impl State { let inner = self.inner.read().await; - let account_commitment = inner.account_tree.get_latest_commitment(account_id); + let account_commitment = + tokio::task::block_in_place(|| inner.account_tree.get_latest_commitment(account_id)); let new_account_id_prefix_is_unique = if account_commitment.is_empty() { - Some(!inner.account_tree.contains_account_id_prefix_in_latest(account_id.prefix())) + Some(!tokio::task::block_in_place(|| { + inner.account_tree.contains_account_id_prefix_in_latest(account_id.prefix()) + })) } else { None }; @@ -597,13 +610,15 @@ impl State { }); } - let nullifiers = nullifiers - .iter() - .map(|nullifier| NullifierInfo { - nullifier: *nullifier, - block_num: inner.nullifier_tree.get_block_num(nullifier).unwrap_or_default(), - }) - .collect(); + let nullifiers = tokio::task::block_in_place(|| { + nullifiers + .iter() + .map(|nullifier| NullifierInfo { + nullifier: *nullifier, + block_num: inner.nullifier_tree.get_block_num(nullifier).unwrap_or_default(), + }) + .collect() + }); let found_unauthenticated_notes = self .db @@ -691,24 +706,28 @@ impl State { self.inner.read().instrument(tracing::info_span!("acquire_inner_state")).await; // Determine which block to query - let (block_num, witness) = if let Some(requested_block) = block_num { - // Historical query: use the account tree with history - let witness = - inner_state.account_tree.open_at(account_id, requested_block).ok_or_else(|| { - let latest_block = inner_state.account_tree.block_number_latest(); - if requested_block > latest_block { - GetAccountError::UnknownBlock(requested_block) - } else { - GetAccountError::BlockPruned(requested_block) - } - })?; - (requested_block, witness) - } else { - // Latest query: use the latest state - let block_num = inner_state.account_tree.block_number_latest(); - let witness = inner_state.account_tree.open_latest(account_id); - (block_num, witness) - }; + let (block_num, witness) = tokio::task::block_in_place(|| { + if let Some(requested_block) = block_num { + // Historical query: use the account tree with history + let witness = inner_state + .account_tree + .open_at(account_id, requested_block) + .ok_or_else(|| { + let latest_block = inner_state.account_tree.block_number_latest(); + if requested_block > latest_block { + GetAccountError::UnknownBlock(requested_block) + } else { + GetAccountError::BlockPruned(requested_block) + } + })?; + Ok::<_, GetAccountError>((requested_block, witness)) + } else { + // Latest query: use the latest state + let block_num = inner_state.account_tree.block_number_latest(); + let witness = inner_state.account_tree.open_latest(account_id); + Ok((block_num, witness)) + } + })?; Ok((block_num, witness)) }