From 37b5b07580f21e5902ceaf357a4ddccd13d0396a Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 12 May 2026 17:40:18 -0400 Subject: [PATCH 1/6] feat: hash AssetVaultKey before insertion into asset vault SMT Two non-fungible assets issued by the same faucet share the third element of their raw AssetVaultKey (the faucet ID suffix), which the SMT uses to determine leaf membership. Without hashing, they always landed in the same leaf, risking the per-leaf cap and harming tree balance. Mirrors the existing raw-key/hashed-key pattern used by StorageMap: - AssetVaultKey::to_smt_key() returns Poseidon2 hash of the raw key. - AssetVault and PartialVault store the SMT keyed by hashed keys, plus a BTreeMap of raw keys for iteration and proofs. - AssetWitness carries both the SmtProof and the raw key(s) it covers. - The MASM kernel injects exec.poseidon2::hash before every smt::set, smt::get, smt::peek in asset_vault.masm (same as hash_map_key in account.masm). Closes #2518. --- .../kernels/transaction/lib/asset_vault.masm | 51 ++++- .../src/asset/vault/asset_witness.rs | 184 +++++++++++------ crates/miden-protocol/src/asset/vault/mod.rs | 151 +++++++++----- .../miden-protocol/src/asset/vault/partial.rs | 187 ++++++++++-------- .../src/asset/vault/vault_key.rs | 20 +- crates/miden-protocol/src/errors/mod.rs | 2 + .../src/transaction/inputs/mod.rs | 7 +- .../src/transaction/kernel/advice_inputs.rs | 5 +- docs/src/asset.md | 2 + 9 files changed, 402 insertions(+), 207 deletions(-) diff --git a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm index 51444b0773..f43efd75fa 100644 --- a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm +++ b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm @@ -1,9 +1,26 @@ use miden::core::collections::smt +use miden::core::crypto::hashes::poseidon2 use $kernel::asset use $kernel::fungible_asset use $kernel::non_fungible_asset +# ASSET KEY HASHING +# ================================================================================================= + +#! Hashes the raw asset vault key into the key used by the asset vault SMT. +#! +#! See [`AssetVaultKey::to_smt_key`] in Rust for the rationale: hashing the raw key gives a uniform +#! distribution across SMT leaves and in particular prevents non-fungible assets issued by the same +#! faucet from sharing a leaf. +#! +#! Inputs: [ASSET_KEY] +#! Outputs: [HASHED_ASSET_KEY] +proc hash_asset_key + exec.poseidon2::hash + # => [HASHED_ASSET_KEY] +end + # ERRORS # ================================================================================================= @@ -28,12 +45,16 @@ const ERR_VAULT_NON_FUNGIBLE_ASSET_TO_REMOVE_NOT_FOUND="failed to remove non-exi #! - ASSET_KEY is the asset vault key of the asset to fetch. #! - ASSET_VALUE is the value of the asset from the vault, which can be the EMPTY_WORD if it isn't present. pub proc get_asset + # hash the asset vault key before using it as the SMT key + exec.hash_asset_key + # => [HASHED_ASSET_KEY, vault_root_ptr] + # load the asset vault root from memory padw movup.8 mem_loadw_le - # => [ASSET_VAULT_ROOT, ASSET_KEY] + # => [ASSET_VAULT_ROOT, HASHED_ASSET_KEY] swapw - # => [ASSET_KEY, ASSET_VAULT_ROOT] + # => [HASHED_ASSET_KEY, ASSET_VAULT_ROOT] # lookup asset exec.smt::get swapw dropw @@ -64,16 +85,20 @@ end #! - ASSET_KEY is the asset vault key of the asset to fetch. #! - ASSET_VALUE is the retrieved asset. pub proc peek_asset + # hash the asset vault key before using it as the SMT key + exec.hash_asset_key + # => [HASHED_ASSET_KEY, vault_root_ptr] + # load the asset vault root from memory padw movup.8 mem_loadw_le - # => [ASSET_VAULT_ROOT, ASSET_KEY] + # => [ASSET_VAULT_ROOT, HASHED_ASSET_KEY] swapw - # => [ASSET_KEY, ASSET_VAULT_ROOT] + # => [HASHED_ASSET_KEY, ASSET_VAULT_ROOT] # lookup asset exec.smt::peek - # OS => [ASSET_KEY, ASSET_VAULT_ROOT] + # OS => [HASHED_ASSET_KEY, ASSET_VAULT_ROOT] # AS => [ASSET_VALUE] dropw @@ -153,6 +178,10 @@ pub proc add_fungible_asset movdnw.2 # => [MERGED_ASSET_VALUE, ASSET_KEY, VAULT_ROOT, CUR_VAULT_VALUE, MERGED_ASSET_VALUE] + # hash the asset key before using it as the SMT key + swapw exec.hash_asset_key swapw + # => [MERGED_ASSET_VALUE, HASHED_ASSET_KEY, VAULT_ROOT, CUR_VAULT_VALUE, MERGED_ASSET_VALUE] + # update asset in vault exec.smt::set # => [PREV_VAULT_VALUE, NEW_VAULT_ROOT, CUR_VAULT_VALUE, MERGED_ASSET_VALUE] @@ -192,6 +221,10 @@ pub proc add_non_fungible_asset dupw.2 # => [ASSET_VALUE, ASSET_KEY, VAULT_ROOT, ASSET_VALUE, vault_root_ptr] + # hash the asset key before using it as the SMT key + swapw exec.hash_asset_key swapw + # => [ASSET_VALUE, HASHED_ASSET_KEY, VAULT_ROOT, ASSET_VALUE, vault_root_ptr] + # insert asset into vault exec.smt::set # => [OLD_VAL, VAULT_ROOT', ASSET_VALUE, vault_root_ptr] @@ -299,6 +332,10 @@ pub proc remove_fungible_asset movdnw.2 # => [REMAINING_ASSET_VALUE, ASSET_KEY, VAULT_ROOT, PEEKED_ASSET_VALUE, vault_root_ptr] + # hash the asset key before using it as the SMT key + swapw exec.hash_asset_key swapw + # => [REMAINING_ASSET_VALUE, HASHED_ASSET_KEY, VAULT_ROOT, PEEKED_ASSET_VALUE, vault_root_ptr] + # update asset in vault and assert the old value is equivalent to the peeked value provided # via peek_asset exec.smt::set @@ -343,6 +380,10 @@ pub proc remove_non_fungible_asset swapw padw # => [EMPTY_WORD, ASSET_KEY, VAULT_ROOT, ASSET_VALUE, vault_root_ptr] + # hash the asset key before using it as the SMT key + swapw exec.hash_asset_key swapw + # => [EMPTY_WORD, HASHED_ASSET_KEY, VAULT_ROOT, ASSET_VALUE, vault_root_ptr] + # insert empty word into the vault to remove the asset exec.smt::set # => [REMOVED_ASSET_VALUE, NEW_VAULT_ROOT, ASSET_VALUE, vault_root_ptr] diff --git a/crates/miden-protocol/src/asset/vault/asset_witness.rs b/crates/miden-protocol/src/asset/vault/asset_witness.rs index f289b3e92a..31cb0ad06f 100644 --- a/crates/miden-protocol/src/asset/vault/asset_witness.rs +++ b/crates/miden-protocol/src/asset/vault/asset_witness.rs @@ -1,10 +1,13 @@ use alloc::boxed::Box; +use alloc::collections::BTreeMap; use alloc::string::ToString; +use alloc::vec::Vec; use miden_crypto::merkle::InnerNodeInfo; -use miden_crypto::merkle::smt::{SmtLeaf, SmtProof}; +use miden_crypto::merkle::smt::SmtProof; use super::vault_key::AssetVaultKey; +use crate::Word; use crate::asset::Asset; use crate::errors::AssetError; use crate::utils::serde::{ @@ -18,93 +21,150 @@ use crate::utils::serde::{ /// A witness of an asset in an [`AssetVault`](super::AssetVault). /// /// It proves inclusion of a certain asset in the vault. +/// +/// ## Guarantees +/// +/// This type guarantees that the raw key-value pairs it contains are all present in the +/// contained SMT proof (under their hashed form). Note that the inverse is not necessarily true: +/// the proof may contain more entries than the witness because to prove inclusion of a given raw +/// key A an [`SmtLeaf::Multiple`](miden_crypto::merkle::smt::SmtLeaf::Multiple) may be present +/// that contains both keys hash(A) and hash(B). However, B may not be present in the key-value +/// pairs and this is a valid state. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct AssetWitness(SmtProof); +pub struct AssetWitness { + proof: SmtProof, + /// Raw [`AssetVaultKey`]s -> asset value words, kept consistent with the proof's leaf entries. + entries: BTreeMap, +} impl AssetWitness { // CONSTRUCTORS // -------------------------------------------------------------------------------------------- - /// Creates a new [`AssetWitness`] from an SMT proof. + /// Creates a new [`AssetWitness`] from an SMT proof and a set of raw vault keys. + /// + /// For each key, looks up its hashed form in the proof and records the resulting value. /// /// # Errors /// /// Returns an error if: - /// - any of the key value pairs in the SMT leaf do not form a valid asset. - pub fn new(smt_proof: SmtProof) -> Result { - for (vault_key, asset_value) in smt_proof.leaf().entries() { - // This ensures that vault key and value are consistent. - Asset::from_key_value_words(*vault_key, *asset_value) - .map_err(|err| AssetError::AssetWitnessInvalid(Box::new(err)))?; + /// - any key's hashed form is not present in the proof. + /// - any of the resulting `(vault_key, value)` pairs do not form a valid asset. + pub fn new( + proof: SmtProof, + keys: impl IntoIterator, + ) -> Result { + let mut entries = BTreeMap::new(); + + for key in keys { + let value = + proof.get(&key.to_smt_key()).ok_or(AssetError::AssetWitnessMissingKey { key })?; + + // Validate that the (key, value) pair forms a valid asset (and skip empty entries). + if !value.is_empty() { + Asset::from_key_value(key, value) + .map_err(|err| AssetError::AssetWitnessInvalid(Box::new(err)))?; + } + + entries.insert(key, value); } - Ok(Self(smt_proof)) + Ok(Self { proof, entries }) } - /// Creates a new [`AssetWitness`] from an SMT proof without checking that the proof contains - /// valid assets. + /// Creates a new [`AssetWitness`] from an SMT proof and a set of key-value pairs without + /// validating that the pairs form valid assets. /// - /// Prefer [`AssetWitness::new`] whenever possible. - pub fn new_unchecked(smt_proof: SmtProof) -> Self { - Self(smt_proof) + /// Prefer [`AssetWitness::new`] whenever possible. See the type-level docs for the invariants + /// callers must uphold. + pub fn new_unchecked( + proof: SmtProof, + key_values: impl IntoIterator, + ) -> Self { + Self { + proof, + entries: key_values.into_iter().collect(), + } } // PUBLIC ACCESSORS // -------------------------------------------------------------------------------------------- + /// Returns a reference to the underlying [`SmtProof`]. + pub fn proof(&self) -> &SmtProof { + &self.proof + } + /// Returns `true` if this [`AssetWitness`] authenticates the provided [`AssetVaultKey`], i.e. /// if its leaf index matches, `false` otherwise. pub fn authenticates_asset_vault_key(&self, vault_key: AssetVaultKey) -> bool { - self.0.leaf().index() == vault_key.to_leaf_index() + self.proof.leaf().index() == vault_key.to_leaf_index() } /// Searches for an [`Asset`] in the witness with the given `vault_key`. pub fn find(&self, vault_key: AssetVaultKey) -> Option { - self.assets().find(|asset| asset.vault_key() == vault_key) + let value = self.entries.get(&vault_key).copied()?; + if value.is_empty() { + None + } else { + Some( + Asset::from_key_value(vault_key, value) + .expect("asset witness should track valid assets"), + ) + } } /// Returns an iterator over the [`Asset`]s in this witness. - pub fn assets(&self) -> impl Iterator { - // TODO: Avoid cloning the vector by not calling SmtLeaf::entries. - // Once SmtLeaf::entries returns a slice (i.e. once - // https://github.com/0xMiden/crypto/pull/521 is available), replace this match statement. - let entries = match self.0.leaf() { - SmtLeaf::Empty(_) => &[], - SmtLeaf::Single(kv_pair) => core::slice::from_ref(kv_pair), - SmtLeaf::Multiple(kv_pairs) => kv_pairs, - }; - - entries.iter().map(|(key, value)| { - Asset::from_key_value_words(*key, *value) - .expect("asset witness should track valid assets") + pub fn assets(&self) -> impl Iterator + '_ { + self.entries.iter().filter_map(|(key, value)| { + if value.is_empty() { + None + } else { + Some( + Asset::from_key_value(*key, *value) + .expect("asset witness should track valid assets"), + ) + } }) } + /// Returns an iterator over the raw `(vault_key, value)` pairs tracked by this witness. + pub fn entries(&self) -> impl Iterator { + self.entries.iter() + } + /// Returns an iterator over every inner node of this witness' merkle path. pub fn authenticated_nodes(&self) -> impl Iterator + '_ { - self.0 + self.proof .path() - .authenticated_nodes(self.0.leaf().index().position(), self.0.leaf().hash()) + .authenticated_nodes(self.proof.leaf().index().position(), self.proof.leaf().hash()) .expect("leaf index is u64 and should be less than 2^SMT_DEPTH") } } impl From for SmtProof { fn from(witness: AssetWitness) -> Self { - witness.0 + witness.proof } } impl Serializable for AssetWitness { fn write_into(&self, target: &mut W) { - self.0.write_into(target); + target.write(&self.proof); + target.write_usize(self.entries.len()); + target.write_many(self.entries.keys()); } } impl Deserializable for AssetWitness { fn read_from(source: &mut R) -> Result { - let proof = SmtProof::read_from(source)?; - Self::new(proof).map_err(|err| DeserializationError::InvalidValue(err.to_string())) + let proof: SmtProof = source.read()?; + let num_keys: usize = source.read()?; + let keys: Vec = (0..num_keys) + .map(|_| source.read::()) + .collect::>()?; + + Self::new(proof, keys).map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } @@ -114,49 +174,55 @@ impl Deserializable for AssetWitness { #[cfg(test)] mod tests { use assert_matches::assert_matches; - use miden_crypto::merkle::smt::Smt; use super::*; - use crate::Word; use crate::asset::{AssetVault, FungibleAsset, NonFungibleAsset}; use crate::testing::account_id::{ ACCOUNT_ID_NETWORK_FUNGIBLE_FAUCET, ACCOUNT_ID_PRIVATE_FUNGIBLE_FAUCET, }; - /// Tests that constructing an asset witness fails if any asset in the smt proof is invalid. + /// Tests that constructing an asset witness fails if the (vault_key, value) pair stored in the + /// proof is inconsistent (here: a non-fungible value under a fungible vault key). #[test] - fn create_asset_witness_fails_on_invalid_asset() -> anyhow::Result<()> { - let invalid_asset = Word::from([0, 0, 0, 5u32]); - let smt = Smt::with_entries([(invalid_asset, invalid_asset)])?; - let proof = smt.open(&invalid_asset); + fn create_asset_witness_fails_on_vault_key_mismatch() -> anyhow::Result<()> { + let fungible_asset = FungibleAsset::mock(500); + let non_fungible_asset = NonFungibleAsset::mock(&[1]); - let err = AssetWitness::new(proof).unwrap_err(); + // Manually build a proof at the fungible asset's hashed key but with a non-fungible value. + let fungible_key = fungible_asset.vault_key(); + let inconsistent_smt = miden_crypto::merkle::smt::Smt::with_entries([( + fungible_key.to_smt_key(), + non_fungible_asset.to_value_word(), + )])?; + let proof = inconsistent_smt.open(&fungible_key.to_smt_key()); + + let err = AssetWitness::new(proof, [fungible_key]).unwrap_err(); assert_matches!(err, AssetError::AssetWitnessInvalid(source) => { - assert_matches!(*source, AssetError::InvalidFaucetAccountId(_)); + assert_matches!(*source, AssetError::FungibleAssetValueMostSignificantElementsMustBeZero(_)); }); Ok(()) } - /// Tests that constructing an asset witness fails if the vault key is from a fungible asset and - /// the asset is a non-fungible one. + /// Tests that constructing an asset witness fails if the provided raw key is not actually + /// present (in hashed form) in the SMT proof. #[test] - fn create_asset_witness_fails_on_vault_key_mismatch() -> anyhow::Result<()> { - let fungible_asset = FungibleAsset::mock(500); - let non_fungible_asset = NonFungibleAsset::mock(&[1]); - - let smt = Smt::with_entries([( - fungible_asset.vault_key().into(), - non_fungible_asset.to_value_word(), - )])?; - let proof = smt.open(&fungible_asset.vault_key().into()); + fn create_asset_witness_fails_on_missing_key() -> anyhow::Result<()> { + let asset_in_vault = + FungibleAsset::new(ACCOUNT_ID_NETWORK_FUNGIBLE_FAUCET.try_into()?, 200)?; + let other_key = + FungibleAsset::new(ACCOUNT_ID_PRIVATE_FUNGIBLE_FAUCET.try_into()?, 100)?.vault_key(); - let err = AssetWitness::new(proof).unwrap_err(); + let vault = AssetVault::new(&[asset_in_vault.into()])?; + let proof = vault.open(asset_in_vault.vault_key()).proof().clone(); - assert_matches!(err, AssetError::AssetWitnessInvalid(source) => { - assert_matches!(*source, AssetError::FungibleAssetValueMostSignificantElementsMustBeZero(_)); + // The proof was opened at `asset_in_vault`'s hashed key, so a separate `other_key` won't + // be found in it. + let err = AssetWitness::new(proof, [other_key]).unwrap_err(); + assert_matches!(err, AssetError::AssetWitnessMissingKey { key } => { + assert_eq!(key, other_key); }); Ok(()) diff --git a/crates/miden-protocol/src/asset/vault/mod.rs b/crates/miden-protocol/src/asset/vault/mod.rs index 9ba8c652e8..85c285e6e6 100644 --- a/crates/miden-protocol/src/asset/vault/mod.rs +++ b/crates/miden-protocol/src/asset/vault/mod.rs @@ -1,3 +1,4 @@ +use alloc::collections::BTreeMap; use alloc::string::ToString; use alloc::vec::Vec; @@ -37,17 +38,24 @@ pub use asset_id::AssetId; /// A container for an unlimited number of assets. /// /// An asset vault can contain an unlimited number of assets. The assets are stored in a Sparse -/// Merkle tree as follows: -/// - For fungible assets, the index of a node is defined by the issuing faucet ID, and the value of -/// the node is the asset itself. Thus, for any fungible asset there will be only one node in the -/// tree. -/// - For non-fungible assets, the index is defined by the asset itself, and the asset is also the -/// value of the node. +/// Merkle Tree, keyed by the hash of the [`AssetVaultKey`] (see [`AssetVaultKey::to_smt_key`]). +/// Hashing the raw key gives a uniform leaf distribution: in particular it prevents non-fungible +/// assets issued by the same faucet from sharing a leaf, which would otherwise happen because +/// their raw vault keys share their third element (the faucet ID suffix) - the element the SMT +/// uses to determine leaf membership. +/// +/// The raw (unhashed) [`AssetVaultKey`]s are retained alongside the SMT to allow iteration and +/// proof reconstruction. This mirrors the +/// [`StorageMap`](crate::account::StorageMap)/[`StorageMapKey`](crate::account::StorageMapKey) +/// pattern used elsewhere in this crate. /// /// An asset vault can be reduced to a single hash which is the root of the Sparse Merkle Tree. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct AssetVault { + /// SMT keyed by hashed [`AssetVaultKey`]s. asset_tree: Smt, + /// Raw [`AssetVaultKey`]s -> asset value words, kept in sync with `asset_tree`. + entries: BTreeMap, } impl AssetVault { @@ -62,12 +70,19 @@ impl AssetVault { /// Returns a new [AssetVault] initialized with the provided assets. pub fn new(assets: &[Asset]) -> Result { - Ok(Self { - asset_tree: Smt::with_entries( - assets.iter().map(|asset| (asset.vault_key().to_word(), asset.to_value_word())), - ) - .map_err(AssetVaultError::DuplicateAsset)?, - }) + let asset_tree = Smt::with_entries( + assets + .iter() + .map(|asset| (asset.vault_key().to_smt_key(), asset.to_value_word())), + ) + .map_err(AssetVaultError::DuplicateAsset)?; + + // `Smt::with_entries` already errored on duplicate keys, so collecting into a `BTreeMap` + // here cannot silently drop assets. + let entries = + assets.iter().map(|asset| (asset.vault_key(), asset.to_value_word())).collect(); + + Ok(Self { asset_tree, entries }) } // PUBLIC ACCESSORS @@ -81,7 +96,7 @@ impl AssetVault { /// Returns the asset corresponding to the provided asset vault key, or `None` if the asset /// doesn't exist. pub fn get(&self, asset_vault_key: AssetVaultKey) -> Option { - let asset_value = self.asset_tree.get_value(&asset_vault_key.to_word()); + let asset_value = self.entries.get(&asset_vault_key).copied().unwrap_or_default(); if asset_value.is_empty() { None @@ -95,11 +110,7 @@ impl AssetVault { /// Returns true if the specified non-fungible asset is stored in this vault. pub fn has_non_fungible_asset(&self, asset: NonFungibleAsset) -> Result { - // check if the asset is stored in the vault - match self.asset_tree.get_value(&asset.vault_key().to_word()) { - asset if asset == Smt::EMPTY_VALUE => Ok(false), - _ => Ok(true), - } + Ok(self.entries.contains_key(&asset.vault_key())) } /// Returns the balance of the asset issued by the specified faucet. If the vault does not @@ -114,7 +125,7 @@ impl AssetVault { let vault_key = AssetVaultKey::new_fungible(faucet_id).expect("faucet ID should be of type fungible"); - let asset_value = self.asset_tree.get_value(&vault_key.to_word()); + let asset_value = self.entries.get(&vault_key).copied().unwrap_or_default(); let asset = FungibleAsset::from_key_value(vault_key, asset_value) .expect("asset vault should only store valid assets"); @@ -123,10 +134,9 @@ impl AssetVault { /// Returns an iterator over the assets stored in the vault. pub fn assets(&self) -> impl Iterator + '_ { - // SAFETY: The asset tree tracks only valid assets. - self.asset_tree.entries().map(|(key, value)| { - Asset::from_key_value_words(*key, *value) - .expect("asset vault should only store valid assets") + // SAFETY: The entries map only tracks valid assets. + self.entries.iter().map(|(key, value)| { + Asset::from_key_value(*key, *value).expect("asset vault should only store valid assets") }) } @@ -139,9 +149,12 @@ impl AssetVault { /// /// The `vault_key` can be obtained with [`Asset::vault_key`]. pub fn open(&self, vault_key: AssetVaultKey) -> AssetWitness { - let smt_proof = self.asset_tree.open(&vault_key.to_word()); - // SAFETY: The asset vault should only contain valid assets. - AssetWitness::new_unchecked(smt_proof) + let smt_proof = self.asset_tree.open(&vault_key.to_smt_key()); + let value = self.entries.get(&vault_key).copied().unwrap_or_default(); + + // SAFETY: The key-value pair is guaranteed to be present in the proof since we open its + // hashed form, and the asset vault only contains valid assets. + AssetWitness::new_unchecked(smt_proof, [(vault_key, value)]) } /// Returns a bool indicating whether the vault is empty. @@ -230,18 +243,16 @@ impl AssetVault { &mut self, other_asset: FungibleAsset, ) -> Result { - let current_asset_value = self.asset_tree.get_value(&other_asset.vault_key().to_word()); - let current_asset = - FungibleAsset::from_key_value(other_asset.vault_key(), current_asset_value) - .expect("asset vault should store valid assets"); + let vault_key = other_asset.vault_key(); + let current_asset_value = self.entries.get(&vault_key).copied().unwrap_or_default(); + let current_asset = FungibleAsset::from_key_value(vault_key, current_asset_value) + .expect("asset vault should store valid assets"); let new_asset = current_asset .add(other_asset) .map_err(AssetVaultError::AddFungibleAssetBalanceError)?; - self.asset_tree - .insert(new_asset.vault_key().to_word(), new_asset.to_value_word()) - .map_err(AssetVaultError::MaxLeafEntriesExceeded)?; + self.insert_entry(new_asset.vault_key(), new_asset.to_value_word())?; Ok(new_asset) } @@ -255,11 +266,7 @@ impl AssetVault { &mut self, asset: NonFungibleAsset, ) -> Result { - // add non-fungible asset to the vault - let old = self - .asset_tree - .insert(asset.vault_key().to_word(), asset.to_value_word()) - .map_err(AssetVaultError::MaxLeafEntriesExceeded)?; + let old = self.insert_entry(asset.vault_key(), asset.to_value_word())?; // if the asset already exists, return an error if old != Smt::EMPTY_VALUE { @@ -306,10 +313,10 @@ impl AssetVault { &mut self, other_asset: FungibleAsset, ) -> Result { - let current_asset_value = self.asset_tree.get_value(&other_asset.vault_key().to_word()); - let current_asset = - FungibleAsset::from_key_value(other_asset.vault_key(), current_asset_value) - .expect("asset vault should store valid assets"); + let vault_key = other_asset.vault_key(); + let current_asset_value = self.entries.get(&vault_key).copied().unwrap_or_default(); + let current_asset = FungibleAsset::from_key_value(vault_key, current_asset_value) + .expect("asset vault should store valid assets"); // If the asset's amount is 0, we consider it absent from the vault. if current_asset.amount() == 0 { @@ -330,9 +337,7 @@ impl AssetVault { } } - self.asset_tree - .insert(new_asset.vault_key().to_word(), new_asset.to_value_word()) - .map_err(AssetVaultError::MaxLeafEntriesExceeded)?; + self.insert_entry(new_asset.vault_key(), new_asset.to_value_word())?; Ok(new_asset) } @@ -346,11 +351,7 @@ impl AssetVault { &mut self, asset: NonFungibleAsset, ) -> Result<(), AssetVaultError> { - // remove the asset from the vault. - let old = self - .asset_tree - .insert(asset.vault_key().to_word(), Smt::EMPTY_VALUE) - .map_err(AssetVaultError::MaxLeafEntriesExceeded)?; + let old = self.insert_entry(asset.vault_key(), Smt::EMPTY_VALUE)?; // return an error if the asset did not exist in the vault. if old == Smt::EMPTY_VALUE { @@ -359,6 +360,25 @@ impl AssetVault { Ok(()) } + + /// Inserts the given `(vault_key, value)` pair into both the SMT and the raw-entry map. + /// + /// Returns the previous SMT value at the hashed key (the empty word if no entry existed). + fn insert_entry( + &mut self, + vault_key: AssetVaultKey, + value: Word, + ) -> Result { + if value == Smt::EMPTY_VALUE { + self.entries.remove(&vault_key); + } else { + self.entries.insert(vault_key, value); + } + + self.asset_tree + .insert(vault_key.to_smt_key(), value) + .map_err(AssetVaultError::MaxLeafEntriesExceeded) + } } // SERIALIZATION @@ -409,4 +429,35 @@ mod tests { let err = vault.remove_asset(FungibleAsset::mock(50)).unwrap_err(); assert_matches!(err, AssetVaultError::FungibleAssetNotFound(_)); } + + /// Two non-fungible assets issued by the same faucet share their third raw-key element (the + /// faucet ID suffix), which historically caused them to land in the same SMT leaf because the + /// SMT uses element 3 for leaf membership. Hashing the vault key before insertion fixes that: + /// the assets must end up in different leaves. + /// + /// Regression test for . + #[test] + fn two_non_fungible_assets_from_same_faucet_use_different_leaves() -> anyhow::Result<()> { + let asset0 = NonFungibleAsset::mock(&[1, 2, 3]); + let asset1 = NonFungibleAsset::mock(&[4, 5, 6]); + + // Sanity check: the assets share their faucet but have distinct raw vault keys (different + // asset IDs). + assert_eq!(asset0.vault_key().faucet_id(), asset1.vault_key().faucet_id()); + assert_ne!(asset0.vault_key(), asset1.vault_key()); + + // Without hashing, both raw vault keys would share their element-3 (the faucet ID suffix) + // and the SMT would route them into a single leaf. Sanity-check that pre-condition. + assert_eq!(asset0.vault_key().to_word()[2], asset1.vault_key().to_word()[2]); + assert_eq!(asset0.vault_key().to_word()[3], asset1.vault_key().to_word()[3]); + + // With hashing, the hashed leaf indices differ, so they live in different SMT leaves. + assert_ne!(asset0.vault_key().to_leaf_index(), asset1.vault_key().to_leaf_index()); + + let vault = AssetVault::new(&[asset0, asset1])?; + assert_eq!(vault.num_leaves(), 2); + assert_eq!(vault.num_assets(), 2); + + Ok(()) + } } diff --git a/crates/miden-protocol/src/asset/vault/partial.rs b/crates/miden-protocol/src/asset/vault/partial.rs index 970d3c8508..53bfea3f1e 100644 --- a/crates/miden-protocol/src/asset/vault/partial.rs +++ b/crates/miden-protocol/src/asset/vault/partial.rs @@ -1,6 +1,7 @@ +use alloc::collections::BTreeMap; use alloc::string::ToString; -use miden_crypto::merkle::smt::{PartialSmt, SmtLeaf, SmtProof}; +use miden_crypto::merkle::smt::{LeafIndex, PartialSmt, SMT_DEPTH, SmtLeaf, SmtProof}; use miden_crypto::merkle::{InnerNodeInfo, MerkleError}; use super::{AssetVault, AssetVaultKey}; @@ -20,10 +21,21 @@ use crate::utils::serde::{ /// Partial vault is used to provide verifiable access to specific assets in a vault /// without the need to provide the full vault data. It contains all required data for loading /// vault data into the transaction kernel for transaction execution. +/// +/// ## Guarantees +/// +/// This type guarantees that the raw key-value pairs it contains are all present in the contained +/// partial SMT (under their hashed form). Note that the inverse is not necessarily true: the SMT +/// may contain more entries than the map because to prove inclusion of a given raw key A an +/// [`SmtLeaf::Multiple`] may be present that contains both keys hash(A) and hash(B). However, B +/// may not be present in the key-value pairs and this is a valid state. #[derive(Clone, Debug, PartialEq, Eq, Default)] pub struct PartialVault { - /// An SMT with a partial view into an account's full [`AssetVault`]. + /// An SMT with a partial view into an account's full [`AssetVault`], keyed by hashed + /// [`AssetVaultKey`]s. partial_smt: PartialSmt, + /// Raw [`AssetVaultKey`]s -> asset value words, kept consistent with `partial_smt`. + entries: BTreeMap, } impl PartialVault { @@ -34,17 +46,36 @@ impl PartialVault { /// /// For conversion from an [`AssetVault`], prefer [`Self::new_minimal`] to be more explicit. pub fn new(root: Word) -> Self { - PartialVault { partial_smt: PartialSmt::new(root) } + PartialVault { + partial_smt: PartialSmt::new(root), + entries: BTreeMap::new(), + } + } + + /// Returns a new [`PartialVault`] with all provided witnesses added to it. + pub fn with_witnesses( + witnesses: impl IntoIterator, + ) -> Result { + let mut entries = BTreeMap::new(); + + let partial_smt = PartialSmt::from_proofs(witnesses.into_iter().map(|witness| { + entries.extend(witness.entries().map(|(key, value)| (*key, *value))); + SmtProof::from(witness) + })) + .map_err(PartialAssetVaultError::FailedToAddProof)?; + + Ok(PartialVault { partial_smt, entries }) } /// Converts an [`AssetVault`] into a partial vault representation. /// - /// The resulting [`PartialVault`] will contain the _full_ merkle paths of the original asset - /// vault. + /// The resulting [`PartialVault`] will contain the _full_ merkle paths and entries of the + /// original asset vault. pub fn new_full(vault: AssetVault) -> Self { let partial_smt = PartialSmt::from(vault.asset_tree); + let entries = vault.entries; - PartialVault { partial_smt } + PartialVault { partial_smt, entries } } /// Converts an [`AssetVault`] into a partial vault representation. @@ -71,11 +102,14 @@ impl PartialVault { self.partial_smt.inner_nodes() } - /// Returns an iterator over all leaves in the Sparse Merkle Tree proofs. - /// - /// Each item returned is a tuple containing the leaf index and a reference to the leaf. - pub fn leaves(&self) -> impl Iterator { - self.partial_smt.leaves().map(|(_, leaf)| leaf) + /// Returns an iterator over all leaves of the underlying [`PartialSmt`]. + pub fn leaves(&self) -> impl Iterator, &SmtLeaf)> { + self.partial_smt.leaves() + } + + /// Returns an iterator over the raw `(vault_key, value)` pairs tracked by this partial vault. + pub fn entries(&self) -> impl Iterator { + self.entries.iter() } /// Returns an opening of the leaf associated with `vault_key`. @@ -89,10 +123,13 @@ impl PartialVault { pub fn open(&self, vault_key: AssetVaultKey) -> Result { let smt_proof = self .partial_smt - .open(&vault_key.into()) + .open(&vault_key.to_smt_key()) .map_err(PartialAssetVaultError::UntrackedAsset)?; - // SAFETY: The partial vault should only contain valid assets. - Ok(AssetWitness::new_unchecked(smt_proof)) + let value = self.entries.get(&vault_key).copied().unwrap_or_default(); + + // SAFETY: The key-value pair is guaranteed to be present in the proof since we open its + // hashed form, and the partial vault only tracks valid assets. + Ok(AssetWitness::new_unchecked(smt_proof, [(vault_key, value)])) } /// Returns the [`Asset`] associated with the given `vault_key`. @@ -104,18 +141,15 @@ impl PartialVault { /// Returns an error if: /// - the key is not tracked by this partial SMT. pub fn get(&self, vault_key: AssetVaultKey) -> Result, MerkleError> { - self.partial_smt.get_value(&vault_key.into()).map(|asset_value| { - if asset_value.is_empty() { - None - } else { - // SAFETY: If this returned a non-empty word, then it should be a valid asset, - // because the vault should only track valid ones. - Some( - Asset::from_key_value(vault_key, asset_value) - .expect("partial vault should only track valid assets"), - ) - } - }) + let value = self.partial_smt.get_value(&vault_key.to_smt_key())?; + if value.is_empty() { + Ok(None) + } else { + Ok(Some( + Asset::from_key_value(vault_key, value) + .expect("partial vault should only track valid assets"), + )) + } } // MUTATORS @@ -129,63 +163,45 @@ impl PartialVault { /// - the new root after the insertion of the leaf and the path does not match the existing root /// (except when the first leaf is added). pub fn add(&mut self, witness: AssetWitness) -> Result<(), PartialAssetVaultError> { - let proof = SmtProof::from(witness); + self.entries.extend(witness.entries().map(|(key, value)| (*key, *value))); self.partial_smt - .add_proof(proof) + .add_proof(SmtProof::from(witness)) .map_err(PartialAssetVaultError::FailedToAddProof) } - - // HELPER FUNCTIONS - // -------------------------------------------------------------------------------------------- - - /// Validates that the provided entries are valid vault keys and assets. - /// - /// For brevity, the error conditions are only mentioned on the public methods that use this - /// function. - fn validate_entries<'a>( - entries: impl IntoIterator, - ) -> Result<(), PartialAssetVaultError> { - for (vault_key, asset_value) in entries { - // This ensures that vault key and value are consistent. - Asset::from_key_value_words(*vault_key, *asset_value).map_err(|source| { - PartialAssetVaultError::InvalidAssetInSmt { entry: *asset_value, source } - })?; - } - - Ok(()) - } -} - -impl TryFrom for PartialVault { - type Error = PartialAssetVaultError; - - /// Returns a new instance of a partial vault from the provided partial SMT. - /// - /// # Errors - /// - /// Returns an error if: - /// - the provided SMT does not track only valid [`Asset`]s. - /// - the vault key at which the asset is stored does not match the vault key derived from the - /// asset. - fn try_from(partial_smt: PartialSmt) -> Result { - Self::validate_entries(partial_smt.entries())?; - - Ok(PartialVault { partial_smt }) - } } impl Serializable for PartialVault { fn write_into(&self, target: &mut W) { - target.write(&self.partial_smt) + target.write(&self.partial_smt); + target.write_usize(self.entries.len()); + target.write_many(self.entries.keys()); } } impl Deserializable for PartialVault { fn read_from(source: &mut R) -> Result { let partial_smt: PartialSmt = source.read()?; + let num_entries: usize = source.read()?; + let mut entries = BTreeMap::new(); + + for _ in 0..num_entries { + let key: AssetVaultKey = source.read()?; + let value = partial_smt.get_value(&key.to_smt_key()).map_err(|err| { + DeserializationError::InvalidValue(alloc::format!( + "failed to find vault key {key} in partial SMT: {err}" + )) + })?; + + // Validate the (key, value) pair forms a valid asset (or is empty). + if !value.is_empty() { + Asset::from_key_value(key, value) + .map_err(|err| DeserializationError::InvalidValue(err.to_string()))?; + } - PartialVault::try_from(partial_smt) - .map_err(|err| DeserializationError::InvalidValue(err.to_string())) + entries.insert(key, value); + } + + Ok(PartialVault { partial_smt, entries }) } } @@ -195,36 +211,35 @@ impl Deserializable for PartialVault { #[cfg(test)] mod tests { use assert_matches::assert_matches; - use miden_crypto::merkle::smt::Smt; use super::*; use crate::asset::FungibleAsset; #[test] - fn partial_vault_ensures_asset_validity() -> anyhow::Result<()> { - let invalid_asset = Word::from([0, 0, 0, 5u32]); - let smt = Smt::with_entries([(invalid_asset, invalid_asset)])?; - let proof = smt.open(&invalid_asset); - let partial_smt = PartialSmt::from_proofs([proof.clone()])?; + fn partial_vault_open_returns_correct_asset_after_full_conversion() -> anyhow::Result<()> { + let asset = FungibleAsset::mock(500); + let vault = AssetVault::new(&[asset])?; + let partial = PartialVault::new_full(vault.clone()); + + let key = asset.vault_key(); + let witness = partial.open(key)?; - let err = PartialVault::try_from(partial_smt).unwrap_err(); - assert_matches!(err, PartialAssetVaultError::InvalidAssetInSmt { entry, .. } => { - assert_eq!(entry, invalid_asset); - }); + assert!(witness.authenticates_asset_vault_key(key)); + assert_eq!(witness.find(key), Some(asset)); + assert_eq!(partial.root(), vault.root()); Ok(()) } #[test] - fn partial_vault_ensures_asset_vault_key_matches() -> anyhow::Result<()> { + fn partial_vault_open_fails_for_untracked_key() -> anyhow::Result<()> { let asset = FungibleAsset::mock(500); - let invalid_vault_key = Word::from([0, 1, 2, 3u32]); - let smt = Smt::with_entries([(invalid_vault_key, asset.to_value_word())])?; - let proof = smt.open(&invalid_vault_key); - let partial_smt = PartialSmt::from_proofs([proof.clone()])?; + let vault = AssetVault::new(&[asset])?; + // `new_minimal` carries the root but no entries. + let partial = PartialVault::new_minimal(&vault); - let err = PartialVault::try_from(partial_smt).unwrap_err(); - assert_matches!(err, PartialAssetVaultError::InvalidAssetInSmt { .. }); + let err = partial.open(asset.vault_key()).unwrap_err(); + assert_matches!(err, PartialAssetVaultError::UntrackedAsset(_)); Ok(()) } diff --git a/crates/miden-protocol/src/asset/vault/vault_key.rs b/crates/miden-protocol/src/asset/vault/vault_key.rs index 2204114f6c..978d39e278 100644 --- a/crates/miden-protocol/src/asset/vault/vault_key.rs +++ b/crates/miden-protocol/src/asset/vault/vault_key.rs @@ -17,7 +17,7 @@ use crate::utils::serde::{ DeserializationError, Serializable, }; -use crate::{Felt, Word}; +use crate::{Felt, Hasher, Word}; /// The unique identifier of an [`Asset`] in the [`AssetVault`](crate::asset::AssetVault). /// @@ -30,6 +30,10 @@ use crate::{Felt, Word}; /// faucet_id_prefix (64 bits) /// ] /// ``` +/// +/// This raw key is hashed via [`Self::to_smt_key`] before being used as the key in the asset +/// vault's underlying SMT. Hashing ensures a uniform distribution across leaves regardless of how +/// faucet IDs or asset IDs are chosen. #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct AssetVaultKey { /// The asset ID of the vault key. @@ -149,9 +153,19 @@ impl AssetVaultKey { } } - /// Returns the leaf index of a vault key. + /// Returns the hashed [`Word`] used as the key in the asset vault's underlying SMT. + /// + /// Hashing the raw key ensures a uniform distribution across SMT leaves. In particular it + /// prevents non-fungible assets issued by the same faucet from sharing a leaf: their raw + /// vault keys share their third element (the faucet ID suffix), which the SMT uses to + /// determine leaf membership. + pub fn to_smt_key(&self) -> Word { + Hasher::hash_elements(self.to_word().as_elements()) + } + + /// Returns the leaf index of a vault key in the asset vault's underlying SMT. pub fn to_leaf_index(&self) -> LeafIndex { - LeafIndex::::from(self.to_word()) + LeafIndex::::from(self.to_smt_key()) } } diff --git a/crates/miden-protocol/src/errors/mod.rs b/crates/miden-protocol/src/errors/mod.rs index 2ff09d4215..e37a80fd9b 100644 --- a/crates/miden-protocol/src/errors/mod.rs +++ b/crates/miden-protocol/src/errors/mod.rs @@ -486,6 +486,8 @@ pub enum AssetError { NonFungibleFaucetIdTypeMismatch(AccountId), #[error("smt proof in asset witness contains invalid key or value")] AssetWitnessInvalid(#[source] Box), + #[error("vault key {key} is not present in the provided asset witness SMT proof")] + AssetWitnessMissingKey { key: AssetVaultKey }, #[error("invalid native asset callbacks encoding: {0}")] InvalidAssetCallbackFlag(u8), } diff --git a/crates/miden-protocol/src/transaction/inputs/mod.rs b/crates/miden-protocol/src/transaction/inputs/mod.rs index f38e780000..0659e7157d 100644 --- a/crates/miden-protocol/src/transaction/inputs/mod.rs +++ b/crates/miden-protocol/src/transaction/inputs/mod.rs @@ -300,7 +300,7 @@ impl TransactionInputs { // Construct SMT proof and witness. let smt_proof = SmtProof::new(sparse_path, smt_leaf)?; - let asset_witness = AssetWitness::new(smt_proof)?; + let asset_witness = AssetWitness::new(smt_proof, [vault_key])?; asset_witnesses.push(asset_witness); } Ok(asset_witnesses) @@ -353,11 +353,12 @@ impl TransactionInputs { .ok_or(TransactionInputsExtractionError::MissingVaultRoot)?; let smt_leaf = SmtLeaf::try_from_elements(smt_leaf_elements, smt_index)?; - // Find the asset in the SMT leaf + // Find the asset in the SMT leaf. Leaves are keyed by the hashed form of the vault key. + let hashed_key = asset_key.to_smt_key(); let asset = smt_leaf .entries() .iter() - .find(|(key, _value)| key == &asset_key.to_word()) + .find(|(key, _value)| key == &hashed_key) .map(|(_key, value)| Asset::from_key_value(asset_key, *value)) .transpose()?; diff --git a/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs b/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs index e6f1bc4524..8751fcb63c 100644 --- a/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs +++ b/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs @@ -285,7 +285,10 @@ impl TransactionAdviceInputs { // populate Merkle store and advice map with nodes info needed to access vault assets self.extend_merkle_store(account.vault().inner_nodes()); self.extend_map( - account.vault().leaves().map(|leaf| (leaf.hash(), leaf.to_elements().collect())), + account + .vault() + .leaves() + .map(|(_, leaf)| (leaf.hash(), leaf.to_elements().collect())), ); } diff --git a/docs/src/asset.md b/docs/src/asset.md index 756e0fdebf..580ac824f2 100644 --- a/docs/src/asset.md +++ b/docs/src/asset.md @@ -56,6 +56,8 @@ Non-fungible assets are encoded by hashing the `Asset` data into 32 bytes and pl [Accounts](./account) and [notes](note) have vaults used to store assets. Accounts use a sparse Merkle tree as a vault while notes use a simple list. This enables an account to store a practically unlimited number of assets while a note can only store up to 64 assets. +Asset vault keys are hashed before being used as keys in the underlying sparse Merkle tree. Hashing the raw key ensures a uniform leaf distribution: in particular, it prevents non-fungible assets issued by the same faucet from sharing an SMT leaf (their raw vault keys share the third element that the SMT uses to determine leaf membership). +

Asset storage

From b1486c8a79a87ca0d572ad92371cbad8b93652b3 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Tue, 12 May 2026 17:45:21 -0400 Subject: [PATCH 2/6] chore: add CHANGELOG entry for #2912 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da7242b828..707eae85f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ - [BREAKING] Renamed `note::build_recipient_hash` to `note::compute_recipient` and `note::build_recipient` to `note::compute_and_store_recipient` ([#2875](https://github.com/0xMiden/protocol/issues/2875)). - [BREAKING] Merged `BasicFungibleFaucet` and `NetworkFungibleFaucet` ([#2890](https://github.com/0xMiden/protocol/pull/2890)). - [BREAKING] Renamed `NoteMetadata` to `PartialNoteMetadata` and renamed `NoteMetadataHeader` to `NoteMetadata` ([#2887](https://github.com/0xMiden/protocol/pull/2887)). +- [BREAKING] Hashed `AssetVaultKey` before insertion into the asset vault SMT so non-fungible assets issued by the same faucet no longer share a leaf ([#2912](https://github.com/0xMiden/protocol/pull/2912)). - [BREAKING] Renamed account ID version 0 to version 1 and made encoded version 0 invalid ([#2842](https://github.com/0xMiden/protocol/issues/2842)). - [BREAKING] Changed note metadata version 1 to encode as `1`, leaving encoded version `0` invalid. - Documented the `miden::protocol::account_id` module in the protocol library docs ([#2607](https://github.com/0xMiden/protocol/issues/2607)). From 59521ea1a5a708786a9465291dc5f475b2ce4042 Mon Sep 17 00:00:00 2001 From: "Claude (Opus)" Date: Wed, 13 May 2026 14:32:44 -0400 Subject: [PATCH 3/6] fix: address review feedback on AssetVaultKey hashing - Make `PartialVault::add` atomic: extend `entries` only after SMT proof insertion succeeds, preserving the entries-subset-of-SMT invariant on error. - Filter zero-amount fungible assets out of `AssetVault::new`'s `entries` map so the map and the underlying SMT stay in sync (the SMT treats empty values as no-ops). - Correct the suffix/prefix wording: `LeafIndex` uses `value[3]`, which is the faucet ID prefix, not the suffix. Fixed in `vault_key.rs`, `vault/mod.rs` doc + test, and `docs/src/asset.md`. - Tighten `AssetWitness::new_unchecked`: document the proof-vs-pair precondition explicitly and `debug_assert!` it. - Add `PartialVault::with_witnesses` and `PartialVault::add` tests: happy-path (with round-trip serialization for `with_witnesses`) and root-mismatch failure paths (the `add` failure doubles as a regression test for the atomicity fix above). --- .../src/asset/vault/asset_witness.rs | 22 +++- crates/miden-protocol/src/asset/vault/mod.rs | 20 ++-- .../miden-protocol/src/asset/vault/partial.rs | 100 +++++++++++++++++- .../src/asset/vault/vault_key.rs | 2 +- docs/src/asset.md | 2 +- 5 files changed, 130 insertions(+), 16 deletions(-) diff --git a/crates/miden-protocol/src/asset/vault/asset_witness.rs b/crates/miden-protocol/src/asset/vault/asset_witness.rs index 31cb0ad06f..12472fd123 100644 --- a/crates/miden-protocol/src/asset/vault/asset_witness.rs +++ b/crates/miden-protocol/src/asset/vault/asset_witness.rs @@ -77,14 +77,30 @@ impl AssetWitness { /// /// Prefer [`AssetWitness::new`] whenever possible. See the type-level docs for the invariants /// callers must uphold. + /// + /// # Caller precondition + /// + /// For each `(key, value)` pair, `proof.get(&key.to_smt_key())` must return `Some(value)`. + /// In other words, each provided pair must agree with what the proof asserts at the hashed + /// key. Passing a mismatched pair lets downstream consumers of [`Self::find`] / + /// [`Self::assets`] disagree with consumers of the underlying [`SmtProof`]. This + /// precondition is checked in debug builds via [`debug_assert!`]. pub fn new_unchecked( proof: SmtProof, key_values: impl IntoIterator, ) -> Self { - Self { - proof, - entries: key_values.into_iter().collect(), + let entries: BTreeMap = key_values.into_iter().collect(); + + #[cfg(debug_assertions)] + for (key, value) in &entries { + debug_assert_eq!( + proof.get(&key.to_smt_key()), + Some(*value), + "AssetWitness::new_unchecked: (key, value) pair does not match the proof", + ); } + + Self { proof, entries } } // PUBLIC ACCESSORS diff --git a/crates/miden-protocol/src/asset/vault/mod.rs b/crates/miden-protocol/src/asset/vault/mod.rs index 85c285e6e6..765081f04e 100644 --- a/crates/miden-protocol/src/asset/vault/mod.rs +++ b/crates/miden-protocol/src/asset/vault/mod.rs @@ -41,7 +41,7 @@ pub use asset_id::AssetId; /// Merkle Tree, keyed by the hash of the [`AssetVaultKey`] (see [`AssetVaultKey::to_smt_key`]). /// Hashing the raw key gives a uniform leaf distribution: in particular it prevents non-fungible /// assets issued by the same faucet from sharing a leaf, which would otherwise happen because -/// their raw vault keys share their third element (the faucet ID suffix) - the element the SMT +/// their raw vault keys share their fourth element (the faucet ID prefix) - the element the SMT /// uses to determine leaf membership. /// /// The raw (unhashed) [`AssetVaultKey`]s are retained alongside the SMT to allow iteration and @@ -77,10 +77,14 @@ impl AssetVault { ) .map_err(AssetVaultError::DuplicateAsset)?; - // `Smt::with_entries` already errored on duplicate keys, so collecting into a `BTreeMap` - // here cannot silently drop assets. - let entries = - assets.iter().map(|asset| (asset.vault_key(), asset.to_value_word())).collect(); + // Filter empty values so the `entries` map stays in sync with the SMT, which treats + // empty values as no-ops. `Smt::with_entries` above already errored on duplicate keys, + // so collecting into a `BTreeMap` here cannot silently drop assets. + let entries = assets + .iter() + .filter(|asset| !asset.to_value_word().is_empty()) + .map(|asset| (asset.vault_key(), asset.to_value_word())) + .collect(); Ok(Self { asset_tree, entries }) } @@ -430,8 +434,8 @@ mod tests { assert_matches!(err, AssetVaultError::FungibleAssetNotFound(_)); } - /// Two non-fungible assets issued by the same faucet share their third raw-key element (the - /// faucet ID suffix), which historically caused them to land in the same SMT leaf because the + /// Two non-fungible assets issued by the same faucet share their fourth raw-key element (the + /// faucet ID prefix), which historically caused them to land in the same SMT leaf because the /// SMT uses element 3 for leaf membership. Hashing the vault key before insertion fixes that: /// the assets must end up in different leaves. /// @@ -446,7 +450,7 @@ mod tests { assert_eq!(asset0.vault_key().faucet_id(), asset1.vault_key().faucet_id()); assert_ne!(asset0.vault_key(), asset1.vault_key()); - // Without hashing, both raw vault keys would share their element-3 (the faucet ID suffix) + // Without hashing, both raw vault keys would share their element-3 (the faucet ID prefix) // and the SMT would route them into a single leaf. Sanity-check that pre-condition. assert_eq!(asset0.vault_key().to_word()[2], asset1.vault_key().to_word()[2]); assert_eq!(asset0.vault_key().to_word()[3], asset1.vault_key().to_word()[3]); diff --git a/crates/miden-protocol/src/asset/vault/partial.rs b/crates/miden-protocol/src/asset/vault/partial.rs index 53bfea3f1e..9b98077c18 100644 --- a/crates/miden-protocol/src/asset/vault/partial.rs +++ b/crates/miden-protocol/src/asset/vault/partial.rs @@ -163,10 +163,16 @@ impl PartialVault { /// - the new root after the insertion of the leaf and the path does not match the existing root /// (except when the first leaf is added). pub fn add(&mut self, witness: AssetWitness) -> Result<(), PartialAssetVaultError> { - self.entries.extend(witness.entries().map(|(key, value)| (*key, *value))); + // Collect entries first so that, if `add_proof` fails, no partial state escapes into + // `self.entries`. The type-level guarantee (entries are a subset of partial_smt) must + // hold even after an error. + let new_entries: alloc::vec::Vec<_> = + witness.entries().map(|(key, value)| (*key, *value)).collect(); self.partial_smt .add_proof(SmtProof::from(witness)) - .map_err(PartialAssetVaultError::FailedToAddProof) + .map_err(PartialAssetVaultError::FailedToAddProof)?; + self.entries.extend(new_entries); + Ok(()) } } @@ -210,10 +216,13 @@ impl Deserializable for PartialVault { #[cfg(test)] mod tests { + use alloc::vec::Vec; + use assert_matches::assert_matches; use super::*; - use crate::asset::FungibleAsset; + use crate::asset::{FungibleAsset, NonFungibleAsset}; + use crate::testing::account_id::ACCOUNT_ID_PRIVATE_FUNGIBLE_FAUCET; #[test] fn partial_vault_open_returns_correct_asset_after_full_conversion() -> anyhow::Result<()> { @@ -243,4 +252,89 @@ mod tests { Ok(()) } + + #[test] + fn partial_vault_with_witnesses_round_trips() -> anyhow::Result<()> { + let fungible = FungibleAsset::mock(500); + let non_fungible = NonFungibleAsset::mock(&[1, 2, 3]); + let vault = AssetVault::new(&[fungible, non_fungible])?; + + let witnesses = [vault.open(fungible.vault_key()), vault.open(non_fungible.vault_key())]; + let partial = PartialVault::with_witnesses(witnesses)?; + + assert_eq!(partial.root(), vault.root()); + assert_eq!(partial.entries().count(), 2); + + // Round-trip serialization preserves equality. + let bytes = partial.to_bytes(); + let roundtripped = PartialVault::read_from_bytes(&bytes)?; + assert_eq!(partial, roundtripped); + + Ok(()) + } + + #[test] + fn partial_vault_with_witnesses_fails_on_root_mismatch() -> anyhow::Result<()> { + // Two single-asset vaults rooted at different SMT roots. + let asset_a = FungibleAsset::mock(500); + let asset_b: Asset = + FungibleAsset::new(ACCOUNT_ID_PRIVATE_FUNGIBLE_FAUCET.try_into()?, 100)?.into(); + let vault_a = AssetVault::new(&[asset_a])?; + let vault_b = AssetVault::new(&[asset_b])?; + assert_ne!(vault_a.root(), vault_b.root()); + + let witness_a = vault_a.open(asset_a.vault_key()); + let witness_b = vault_b.open(asset_b.vault_key()); + + let err = PartialVault::with_witnesses([witness_a, witness_b]).unwrap_err(); + assert_matches!(err, PartialAssetVaultError::FailedToAddProof(_)); + + Ok(()) + } + + #[test] + fn partial_vault_add_extends_with_new_witness() -> anyhow::Result<()> { + let fungible = FungibleAsset::mock(500); + let non_fungible = NonFungibleAsset::mock(&[7, 8, 9]); + let vault = AssetVault::new(&[fungible, non_fungible])?; + + let mut partial = PartialVault::with_witnesses([vault.open(fungible.vault_key())])?; + assert_eq!(partial.entries().count(), 1); + + partial.add(vault.open(non_fungible.vault_key()))?; + + assert_eq!(partial.root(), vault.root()); + assert_eq!(partial.entries().count(), 2); + assert_eq!(partial.open(fungible.vault_key())?.find(fungible.vault_key()), Some(fungible)); + assert_eq!( + partial.open(non_fungible.vault_key())?.find(non_fungible.vault_key()), + Some(non_fungible), + ); + + Ok(()) + } + + #[test] + fn partial_vault_add_is_atomic_on_failure() -> anyhow::Result<()> { + // Build two distinct vaults so the second witness's root disagrees with the first. + let asset_a = FungibleAsset::mock(500); + let asset_b: Asset = + FungibleAsset::new(ACCOUNT_ID_PRIVATE_FUNGIBLE_FAUCET.try_into()?, 100)?.into(); + let vault_a = AssetVault::new(&[asset_a])?; + let vault_b = AssetVault::new(&[asset_b])?; + + let mut partial = PartialVault::with_witnesses([vault_a.open(asset_a.vault_key())])?; + let entries_before: Vec<_> = partial.entries().map(|(k, v)| (*k, *v)).collect(); + let root_before = partial.root(); + + let err = partial.add(vault_b.open(asset_b.vault_key())).unwrap_err(); + assert_matches!(err, PartialAssetVaultError::FailedToAddProof(_)); + + // Atomicity: failed `add` must not leak entries or shift the root. + let entries_after: Vec<_> = partial.entries().map(|(k, v)| (*k, *v)).collect(); + assert_eq!(entries_before, entries_after); + assert_eq!(partial.root(), root_before); + + Ok(()) + } } diff --git a/crates/miden-protocol/src/asset/vault/vault_key.rs b/crates/miden-protocol/src/asset/vault/vault_key.rs index 978d39e278..157628756b 100644 --- a/crates/miden-protocol/src/asset/vault/vault_key.rs +++ b/crates/miden-protocol/src/asset/vault/vault_key.rs @@ -157,7 +157,7 @@ impl AssetVaultKey { /// /// Hashing the raw key ensures a uniform distribution across SMT leaves. In particular it /// prevents non-fungible assets issued by the same faucet from sharing a leaf: their raw - /// vault keys share their third element (the faucet ID suffix), which the SMT uses to + /// vault keys share their fourth element (the faucet ID prefix), which the SMT uses to /// determine leaf membership. pub fn to_smt_key(&self) -> Word { Hasher::hash_elements(self.to_word().as_elements()) diff --git a/docs/src/asset.md b/docs/src/asset.md index 580ac824f2..1fee04e996 100644 --- a/docs/src/asset.md +++ b/docs/src/asset.md @@ -56,7 +56,7 @@ Non-fungible assets are encoded by hashing the `Asset` data into 32 bytes and pl [Accounts](./account) and [notes](note) have vaults used to store assets. Accounts use a sparse Merkle tree as a vault while notes use a simple list. This enables an account to store a practically unlimited number of assets while a note can only store up to 64 assets. -Asset vault keys are hashed before being used as keys in the underlying sparse Merkle tree. Hashing the raw key ensures a uniform leaf distribution: in particular, it prevents non-fungible assets issued by the same faucet from sharing an SMT leaf (their raw vault keys share the third element that the SMT uses to determine leaf membership). +Asset vault keys are hashed before being used as keys in the underlying sparse Merkle tree. Hashing the raw key ensures a uniform leaf distribution: in particular, it prevents non-fungible assets issued by the same faucet from sharing an SMT leaf (their raw vault keys share the fourth element - the faucet ID prefix - which the SMT uses to determine leaf membership).

Asset storage From a72aad7892e4a3cb363bc3706a1471757d05090b Mon Sep 17 00:00:00 2001 From: "Claude (Opus)" Date: Wed, 13 May 2026 14:56:03 -0400 Subject: [PATCH 4/6] refactor: address PhilippGackstatter's review on AssetVaultKey hashing - Rename `AssetVaultKey::to_smt_key` to `hash` and have it return a new `AssetVaultKeyHash` newtype (mirrors `StorageMapKey::hash` -> `StorageMapKeyHash`). Updates all call sites and the MASM `hash_asset_key` doc to point at `AssetVaultKey::hash`. - Restrict `AssetWitness::entries` to `pub(super)`; it is an internal helper used by `PartialVault` and not part of the public surface. - Rework `Deserializable for PartialVault` to use `read_many_iter` for `num_entries` (protects against unbounded length prefixes) and route through a new private `from_partial_smt_and_keys` constructor that performs the per-entry validation. Repurposes the previously-dead `PartialAssetVaultError::InvalidAssetInSmt` variant as `InvalidAssetForKey { key, value, source }`. --- .../kernels/transaction/lib/asset_vault.masm | 4 +- crates/miden-protocol/src/asset/mod.rs | 9 ++- .../src/asset/vault/asset_witness.rs | 15 ++--- crates/miden-protocol/src/asset/vault/mod.rs | 10 ++-- .../miden-protocol/src/asset/vault/partial.rs | 59 ++++++++++++------- .../src/asset/vault/vault_key.rs | 48 ++++++++++++--- crates/miden-protocol/src/errors/mod.rs | 9 ++- .../src/transaction/inputs/mod.rs | 2 +- 8 files changed, 108 insertions(+), 48 deletions(-) diff --git a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm index f43efd75fa..ebf36b7eda 100644 --- a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm +++ b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm @@ -10,9 +10,7 @@ use $kernel::non_fungible_asset #! Hashes the raw asset vault key into the key used by the asset vault SMT. #! -#! See [`AssetVaultKey::to_smt_key`] in Rust for the rationale: hashing the raw key gives a uniform -#! distribution across SMT leaves and in particular prevents non-fungible assets issued by the same -#! faucet from sharing a leaf. +#! See [`AssetVaultKey::hash`] in Rust for the rationale. #! #! Inputs: [ASSET_KEY] #! Outputs: [HASHED_ASSET_KEY] diff --git a/crates/miden-protocol/src/asset/mod.rs b/crates/miden-protocol/src/asset/mod.rs index 3f6cde41f1..98633bf2de 100644 --- a/crates/miden-protocol/src/asset/mod.rs +++ b/crates/miden-protocol/src/asset/mod.rs @@ -31,7 +31,14 @@ mod asset_callbacks_flag; pub use asset_callbacks_flag::AssetCallbackFlag; mod vault; -pub use vault::{AssetId, AssetVault, AssetVaultKey, AssetWitness, PartialVault}; +pub use vault::{ + AssetId, + AssetVault, + AssetVaultKey, + AssetVaultKeyHash, + AssetWitness, + PartialVault, +}; // ASSET // ================================================================================================ diff --git a/crates/miden-protocol/src/asset/vault/asset_witness.rs b/crates/miden-protocol/src/asset/vault/asset_witness.rs index 12472fd123..9b23ff43f4 100644 --- a/crates/miden-protocol/src/asset/vault/asset_witness.rs +++ b/crates/miden-protocol/src/asset/vault/asset_witness.rs @@ -57,8 +57,9 @@ impl AssetWitness { let mut entries = BTreeMap::new(); for key in keys { - let value = - proof.get(&key.to_smt_key()).ok_or(AssetError::AssetWitnessMissingKey { key })?; + let value = proof + .get(&key.hash().as_word()) + .ok_or(AssetError::AssetWitnessMissingKey { key })?; // Validate that the (key, value) pair forms a valid asset (and skip empty entries). if !value.is_empty() { @@ -80,7 +81,7 @@ impl AssetWitness { /// /// # Caller precondition /// - /// For each `(key, value)` pair, `proof.get(&key.to_smt_key())` must return `Some(value)`. + /// For each `(key, value)` pair, `proof.get(&key.hash().as_word())` must return `Some(value)`. /// In other words, each provided pair must agree with what the proof asserts at the hashed /// key. Passing a mismatched pair lets downstream consumers of [`Self::find`] / /// [`Self::assets`] disagree with consumers of the underlying [`SmtProof`]. This @@ -94,7 +95,7 @@ impl AssetWitness { #[cfg(debug_assertions)] for (key, value) in &entries { debug_assert_eq!( - proof.get(&key.to_smt_key()), + proof.get(&key.hash().as_word()), Some(*value), "AssetWitness::new_unchecked: (key, value) pair does not match the proof", ); @@ -145,7 +146,7 @@ impl AssetWitness { } /// Returns an iterator over the raw `(vault_key, value)` pairs tracked by this witness. - pub fn entries(&self) -> impl Iterator { + pub(super) fn entries(&self) -> impl Iterator { self.entries.iter() } @@ -208,10 +209,10 @@ mod tests { // Manually build a proof at the fungible asset's hashed key but with a non-fungible value. let fungible_key = fungible_asset.vault_key(); let inconsistent_smt = miden_crypto::merkle::smt::Smt::with_entries([( - fungible_key.to_smt_key(), + fungible_key.hash().as_word(), non_fungible_asset.to_value_word(), )])?; - let proof = inconsistent_smt.open(&fungible_key.to_smt_key()); + let proof = inconsistent_smt.open(&fungible_key.hash().as_word()); let err = AssetWitness::new(proof, [fungible_key]).unwrap_err(); diff --git a/crates/miden-protocol/src/asset/vault/mod.rs b/crates/miden-protocol/src/asset/vault/mod.rs index 765081f04e..608ae792da 100644 --- a/crates/miden-protocol/src/asset/vault/mod.rs +++ b/crates/miden-protocol/src/asset/vault/mod.rs @@ -27,7 +27,7 @@ mod asset_witness; pub use asset_witness::AssetWitness; mod vault_key; -pub use vault_key::AssetVaultKey; +pub use vault_key::{AssetVaultKey, AssetVaultKeyHash}; mod asset_id; pub use asset_id::AssetId; @@ -38,7 +38,7 @@ pub use asset_id::AssetId; /// A container for an unlimited number of assets. /// /// An asset vault can contain an unlimited number of assets. The assets are stored in a Sparse -/// Merkle Tree, keyed by the hash of the [`AssetVaultKey`] (see [`AssetVaultKey::to_smt_key`]). +/// Merkle Tree, keyed by the hash of the [`AssetVaultKey`] (see [`AssetVaultKey::hash`]). /// Hashing the raw key gives a uniform leaf distribution: in particular it prevents non-fungible /// assets issued by the same faucet from sharing a leaf, which would otherwise happen because /// their raw vault keys share their fourth element (the faucet ID prefix) - the element the SMT @@ -73,7 +73,7 @@ impl AssetVault { let asset_tree = Smt::with_entries( assets .iter() - .map(|asset| (asset.vault_key().to_smt_key(), asset.to_value_word())), + .map(|asset| (asset.vault_key().hash().as_word(), asset.to_value_word())), ) .map_err(AssetVaultError::DuplicateAsset)?; @@ -153,7 +153,7 @@ impl AssetVault { /// /// The `vault_key` can be obtained with [`Asset::vault_key`]. pub fn open(&self, vault_key: AssetVaultKey) -> AssetWitness { - let smt_proof = self.asset_tree.open(&vault_key.to_smt_key()); + let smt_proof = self.asset_tree.open(&vault_key.hash().as_word()); let value = self.entries.get(&vault_key).copied().unwrap_or_default(); // SAFETY: The key-value pair is guaranteed to be present in the proof since we open its @@ -380,7 +380,7 @@ impl AssetVault { } self.asset_tree - .insert(vault_key.to_smt_key(), value) + .insert(vault_key.hash().into(), value) .map_err(AssetVaultError::MaxLeafEntriesExceeded) } } diff --git a/crates/miden-protocol/src/asset/vault/partial.rs b/crates/miden-protocol/src/asset/vault/partial.rs index 9b98077c18..aff961b44e 100644 --- a/crates/miden-protocol/src/asset/vault/partial.rs +++ b/crates/miden-protocol/src/asset/vault/partial.rs @@ -86,6 +86,37 @@ impl PartialVault { PartialVault::new(vault.root()) } + /// Constructs a [`PartialVault`] from a [`PartialSmt`] and the raw [`AssetVaultKey`]s whose + /// values are looked up from the SMT. + /// + /// # Errors + /// + /// Returns an error if: + /// - any key's hashed form is not present in the partial SMT. + /// - any of the resulting `(vault_key, value)` pairs does not form a valid asset. + fn from_partial_smt_and_keys( + partial_smt: PartialSmt, + keys: impl IntoIterator, + ) -> Result { + let mut entries = BTreeMap::new(); + + for key in keys { + let value = partial_smt + .get_value(&key.hash().as_word()) + .map_err(PartialAssetVaultError::UntrackedAsset)?; + + if !value.is_empty() { + Asset::from_key_value(key, value).map_err(|source| { + PartialAssetVaultError::InvalidAssetForKey { key, value, source } + })?; + } + + entries.insert(key, value); + } + + Ok(Self { partial_smt, entries }) + } + // ACCESSORS // -------------------------------------------------------------------------------------------- @@ -123,7 +154,7 @@ impl PartialVault { pub fn open(&self, vault_key: AssetVaultKey) -> Result { let smt_proof = self .partial_smt - .open(&vault_key.to_smt_key()) + .open(&vault_key.hash().as_word()) .map_err(PartialAssetVaultError::UntrackedAsset)?; let value = self.entries.get(&vault_key).copied().unwrap_or_default(); @@ -141,7 +172,7 @@ impl PartialVault { /// Returns an error if: /// - the key is not tracked by this partial SMT. pub fn get(&self, vault_key: AssetVaultKey) -> Result, MerkleError> { - let value = self.partial_smt.get_value(&vault_key.to_smt_key())?; + let value = self.partial_smt.get_value(&vault_key.hash().as_word())?; if value.is_empty() { Ok(None) } else { @@ -188,26 +219,12 @@ impl Deserializable for PartialVault { fn read_from(source: &mut R) -> Result { let partial_smt: PartialSmt = source.read()?; let num_entries: usize = source.read()?; - let mut entries = BTreeMap::new(); - - for _ in 0..num_entries { - let key: AssetVaultKey = source.read()?; - let value = partial_smt.get_value(&key.to_smt_key()).map_err(|err| { - DeserializationError::InvalidValue(alloc::format!( - "failed to find vault key {key} in partial SMT: {err}" - )) - })?; - - // Validate the (key, value) pair forms a valid asset (or is empty). - if !value.is_empty() { - Asset::from_key_value(key, value) - .map_err(|err| DeserializationError::InvalidValue(err.to_string()))?; - } - - entries.insert(key, value); - } + let keys = source + .read_many_iter::(num_entries)? + .collect::, _>>()?; - Ok(PartialVault { partial_smt, entries }) + Self::from_partial_smt_and_keys(partial_smt, keys) + .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } diff --git a/crates/miden-protocol/src/asset/vault/vault_key.rs b/crates/miden-protocol/src/asset/vault/vault_key.rs index 157628756b..ab328674e7 100644 --- a/crates/miden-protocol/src/asset/vault/vault_key.rs +++ b/crates/miden-protocol/src/asset/vault/vault_key.rs @@ -1,8 +1,9 @@ use alloc::boxed::Box; -use alloc::string::ToString; +use alloc::string::{String, ToString}; use core::fmt; use miden_crypto::merkle::smt::LeafIndex; +use miden_crypto_derive::WordWrapper; use crate::account::AccountId; use crate::account::AccountType::{self}; @@ -31,9 +32,9 @@ use crate::{Felt, Hasher, Word}; /// ] /// ``` /// -/// This raw key is hashed via [`Self::to_smt_key`] before being used as the key in the asset -/// vault's underlying SMT. Hashing ensures a uniform distribution across leaves regardless of how -/// faucet IDs or asset IDs are chosen. +/// Use [`AssetVaultKey::hash`] to produce the corresponding [`AssetVaultKeyHash`] that is used as +/// the key in the asset vault's underlying SMT. Hashing ensures a uniform distribution across +/// leaves regardless of how faucet IDs or asset IDs are chosen. #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct AssetVaultKey { /// The asset ID of the vault key. @@ -153,19 +154,50 @@ impl AssetVaultKey { } } - /// Returns the hashed [`Word`] used as the key in the asset vault's underlying SMT. + /// Hashes this raw vault key to produce the [`AssetVaultKeyHash`] used as the key in the asset + /// vault's underlying SMT. /// /// Hashing the raw key ensures a uniform distribution across SMT leaves. In particular it /// prevents non-fungible assets issued by the same faucet from sharing a leaf: their raw /// vault keys share their fourth element (the faucet ID prefix), which the SMT uses to /// determine leaf membership. - pub fn to_smt_key(&self) -> Word { - Hasher::hash_elements(self.to_word().as_elements()) + pub fn hash(&self) -> AssetVaultKeyHash { + AssetVaultKeyHash::from_raw(Hasher::hash_elements(self.to_word().as_elements())) } /// Returns the leaf index of a vault key in the asset vault's underlying SMT. pub fn to_leaf_index(&self) -> LeafIndex { - LeafIndex::::from(self.to_smt_key()) + self.hash().to_leaf_index() + } +} + +// ASSET VAULT KEY HASH +// ================================================================================================ + +/// A hashed [`AssetVaultKey`]. +/// +/// This is produced by hashing an [`AssetVaultKey`] and is used as the actual key in the +/// underlying SMT. Wrapping the hashed key in a distinct type prevents accidentally using a raw +/// key where a hashed key is expected and vice-versa. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, WordWrapper)] +pub struct AssetVaultKeyHash(Word); + +impl AssetVaultKeyHash { + /// Returns the leaf index in the SMT for this hashed key. + pub fn to_leaf_index(&self) -> LeafIndex { + self.0.into() + } +} + +impl From for Word { + fn from(key: AssetVaultKeyHash) -> Self { + key.0 + } +} + +impl From for AssetVaultKeyHash { + fn from(key: AssetVaultKey) -> Self { + key.hash() } } diff --git a/crates/miden-protocol/src/errors/mod.rs b/crates/miden-protocol/src/errors/mod.rs index e37a80fd9b..2b5dabfcd8 100644 --- a/crates/miden-protocol/src/errors/mod.rs +++ b/crates/miden-protocol/src/errors/mod.rs @@ -600,8 +600,13 @@ pub enum AssetVaultError { #[derive(Debug, Error)] pub enum PartialAssetVaultError { - #[error("provided SMT entry {entry} is not a valid asset")] - InvalidAssetInSmt { entry: Word, source: AssetError }, + #[error("partial vault contains invalid asset value {value} at key {key}")] + InvalidAssetForKey { + key: AssetVaultKey, + value: Word, + #[source] + source: AssetError, + }, #[error("failed to add asset proof")] FailedToAddProof(#[source] MerkleError), #[error("asset is not tracked in the partial vault")] diff --git a/crates/miden-protocol/src/transaction/inputs/mod.rs b/crates/miden-protocol/src/transaction/inputs/mod.rs index 0659e7157d..5aad85a80e 100644 --- a/crates/miden-protocol/src/transaction/inputs/mod.rs +++ b/crates/miden-protocol/src/transaction/inputs/mod.rs @@ -354,7 +354,7 @@ impl TransactionInputs { let smt_leaf = SmtLeaf::try_from_elements(smt_leaf_elements, smt_index)?; // Find the asset in the SMT leaf. Leaves are keyed by the hashed form of the vault key. - let hashed_key = asset_key.to_smt_key(); + let hashed_key = asset_key.hash().as_word(); let asset = smt_leaf .entries() .iter() From 10256cd74f0e484f1865aacd434d7922713bffc5 Mon Sep 17 00:00:00 2001 From: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> Date: Wed, 13 May 2026 15:07:23 -0400 Subject: [PATCH 5/6] Update crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm Co-authored-by: Philipp Gackstatter --- .../asm/kernels/transaction/lib/asset_vault.masm | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm index ebf36b7eda..7ff7da38f8 100644 --- a/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm +++ b/crates/miden-protocol/asm/kernels/transaction/lib/asset_vault.masm @@ -6,11 +6,9 @@ use $kernel::fungible_asset use $kernel::non_fungible_asset # ASSET KEY HASHING -# ================================================================================================= - #! Hashes the raw asset vault key into the key used by the asset vault SMT. #! -#! See [`AssetVaultKey::hash`] in Rust for the rationale. +#! See [`AssetVaultKey::to_smt_key`] in Rust for the rationale. #! #! Inputs: [ASSET_KEY] #! Outputs: [HASHED_ASSET_KEY] From 03b12723b85092dad13dc9e071008d6de299f30f Mon Sep 17 00:00:00 2001 From: "Claude (Opus)" Date: Thu, 21 May 2026 17:57:19 -0400 Subject: [PATCH 6/6] fix: budget post-compute-fee cycles for vault key hashing Hashing the AssetVaultKey before the asset vault SMT set adds a fixed cost to the fee-removal smt::set in the epilogue, pushing the measured post-compute-fee cycle count (863) past the old estimate (858). Add a dedicated VAULT_KEY_HASH_CYCLES=50 constant (in epilogue.masm and the mirrored test) rather than inflating SMT_SET_ADDITIONAL_CYCLES, whose documented meaning is smt::set's worst-minus-best spread. The fee estimate stays a safe upper bound (608+250+50=908 > 863). --- .../asm/kernels/transaction/lib/epilogue.masm | 22 +++++++++++++++---- .../src/kernel_tests/tx/test_fee.rs | 3 ++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/crates/miden-protocol/asm/kernels/transaction/lib/epilogue.masm b/crates/miden-protocol/asm/kernels/transaction/lib/epilogue.masm index baf2ae2495..d29865116a 100644 --- a/crates/miden-protocol/asm/kernels/transaction/lib/epilogue.masm +++ b/crates/miden-protocol/asm/kernels/transaction/lib/epilogue.masm @@ -39,22 +39,36 @@ const EPILOGUE_AUTH_PROC_START_EVENT=event("miden::protocol::epilogue::auth_proc # Event emitted to signal that an execution of the authentication procedure has ended. const EPILOGUE_AUTH_PROC_END_EVENT=event("miden::protocol::epilogue::auth_proc_end") -# An additional number of cyclces to account for the number of cycles that smt::set will take when +# An additional number of cycles to account for the number of cycles that smt::set will take when # removing the computed fee from the asset vault. # Theoretically, this can safely be set to worst_case_cycles - best_case_cycles of smt::set. That's # because we can assume that the measured number of cycles already contains at least the best case # number of cycles, and so we only need to add the difference. const SMT_SET_ADDITIONAL_CYCLES=250 +# An upper-bound estimate of the cycles needed to hash the asset vault key before the fee-removing +# smt::set. This cost is incurred only when a fee is actually removed from the asset vault, and is +# kept separate from SMT_SET_ADDITIONAL_CYCLES (which models smt::set's own best/worst-case spread). +# It is additive rather than double-counted: the lowest-observed NUM_POST_COMPUTE_FEE_CYCLES below +# comes from a zero-fee transaction that skips fee removal entirely, and so excludes this hashing. +const VAULT_KEY_HASH_CYCLES=50 + # The number of cycles the epilogue is estimated to take after compute_fee has been executed, # including an unknown cycle number of the above-mentioned call to smt::set. It is safe to assume # that this includes at least smt::set's best case number of cycles. # This can be _estimated_ using the transaction measurements on ExecutedTransaction and can be set -# to the lowest observed value. +# to the lowest observed value (which comes from a zero-fee transaction that skips fee removal). const NUM_POST_COMPUTE_FEE_CYCLES=608 -# The number of cycles the epilogue is estimated to take after compute_fee has been executed. -const ESTIMATED_AFTER_COMPUTE_FEE_CYCLES=NUM_POST_COMPUTE_FEE_CYCLES+SMT_SET_ADDITIONAL_CYCLES +# The number of cycles the epilogue is estimated to take after compute_fee has been executed. This +# must remain an upper bound on the real post-compute_fee cycle count, otherwise the verification +# fee is undercharged. The most recently observed worst case is 863 cycles, leaving 45 cycles of +# margin against this 908-cycle budget; the margin is empirical, so re-measure when the epilogue or +# smt::set changes. The fee-removal smt::set cost cannot be inflated by an adversary: vault keys are +# hashed before insertion (see AssetVaultKey::hash), so leaf indices are uniformly distributed and +# entries cannot be concentrated into the fee asset's leaf without breaking the hash's collision +# resistance. The multi-leaf smt::set worst case is not yet exercised (see test_fee.rs TODO). +const ESTIMATED_AFTER_COMPUTE_FEE_CYCLES=NUM_POST_COMPUTE_FEE_CYCLES+SMT_SET_ADDITIONAL_CYCLES+VAULT_KEY_HASH_CYCLES # OUTPUT NOTES PROCEDURES # ================================================================================================= diff --git a/crates/miden-testing/src/kernel_tests/tx/test_fee.rs b/crates/miden-testing/src/kernel_tests/tx/test_fee.rs index e07c755902..08fc6e3535 100644 --- a/crates/miden-testing/src/kernel_tests/tx/test_fee.rs +++ b/crates/miden-testing/src/kernel_tests/tx/test_fee.rs @@ -97,11 +97,12 @@ async fn num_tx_cycles_after_compute_fee_are_less_than_estimated( // These constants should always be updated together with the equivalent constants in // epilogue.masm. const SMT_SET_ADDITIONAL_CYCLES: usize = 250; + const VAULT_KEY_HASH_CYCLES: usize = 50; const NUM_POST_COMPUTE_FEE_CYCLES: usize = 608; assert!( tx.measurements().after_tx_cycles_obtained - < NUM_POST_COMPUTE_FEE_CYCLES + SMT_SET_ADDITIONAL_CYCLES, + < NUM_POST_COMPUTE_FEE_CYCLES + SMT_SET_ADDITIONAL_CYCLES + VAULT_KEY_HASH_CYCLES, "estimated number of cycles is not larger than the measurements, so they need to be updated" );