diff --git a/CHANGELOG.md b/CHANGELOG.md index 77585748fd..50526b7ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ - Added standardized `NetworkAccountNoteAllowlist` slot for detecting network accounts ([#2883](https://github.com/0xMiden/protocol/pull/2883)). - [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. - [BREAKING] Added `NetworkAccount` wrapper for convenient network account identification ([#2915](https://github.com/0xMiden/protocol/pull/2915)). 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 1808579a32..e601b948c8 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,22 @@ 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. +#! +#! Inputs: [ASSET_KEY] +#! Outputs: [HASHED_ASSET_KEY] +proc hash_asset_key + exec.poseidon2::hash + # => [HASHED_ASSET_KEY] +end + # ERRORS # ================================================================================================= @@ -28,12 +41,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 +81,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 +174,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 +217,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] @@ -303,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 @@ -347,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/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-protocol/src/asset/mod.rs b/crates/miden-protocol/src/asset/mod.rs index 33c93d5570..0d8c4dbbca 100644 --- a/crates/miden-protocol/src/asset/mod.rs +++ b/crates/miden-protocol/src/asset/mod.rs @@ -33,7 +33,14 @@ mod asset_composition; pub use asset_composition::AssetComposition; 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 bfa54d185a..292c1106aa 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,167 @@ 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.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() { + 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. + /// + /// # Caller precondition + /// + /// 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 + /// precondition is checked in debug builds via [`debug_assert!`]. + pub fn new_unchecked( + proof: SmtProof, + key_values: impl IntoIterator, + ) -> Self { + let entries: BTreeMap = key_values.into_iter().collect(); + + #[cfg(debug_assertions)] + for (key, value) in &entries { + debug_assert_eq!( + proof.get(&key.hash().as_word()), + Some(*value), + "AssetWitness::new_unchecked: (key, value) pair does not match the proof", + ); + } + + Self { proof, entries } } // 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(super) 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 +191,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_PRIVATE_FUNGIBLE_FAUCET, ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_3, }; - /// 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]); + + // 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.hash().as_word(), + non_fungible_asset.to_value_word(), + )])?; + let proof = inconsistent_smt.open(&fungible_key.hash().as_word()); - let err = AssetWitness::new(proof).unwrap_err(); + 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_PUBLIC_FUNGIBLE_FAUCET_3.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 8c530a6b24..8563076662 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; @@ -27,7 +28,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,17 +39,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::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 +/// 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 { @@ -63,12 +71,23 @@ 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().hash().as_word(), asset.to_value_word())), + ) + .map_err(AssetVaultError::DuplicateAsset)?; + + // 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 }) } // PUBLIC ACCESSORS @@ -82,7 +101,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 @@ -96,11 +115,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 fungible asset identified by `vault_key`. @@ -119,7 +134,7 @@ impl AssetVault { }); } - 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"); @@ -128,10 +143,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") }) } @@ -144,9 +158,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.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 + // 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. @@ -235,18 +252,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) } @@ -260,11 +275,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 { @@ -311,10 +322,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() == AssetAmount::ZERO { @@ -335,9 +346,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) } @@ -351,11 +360,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 { @@ -364,6 +369,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.hash().into(), value) + .map_err(AssetVaultError::MaxLeafEntriesExceeded) + } } // SERIALIZATION @@ -414,4 +438,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 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. + /// + /// 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 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]); + + // 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..aff961b44e 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. @@ -55,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 // -------------------------------------------------------------------------------------------- @@ -71,11 +133,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 +154,13 @@ impl PartialVault { pub fn open(&self, vault_key: AssetVaultKey) -> Result { let smt_proof = self .partial_smt - .open(&vault_key.into()) + .open(&vault_key.hash().as_word()) .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 +172,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.hash().as_word())?; + 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,62 +194,36 @@ 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); + // 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(proof) - .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 } - })?; - } - + .add_proof(SmtProof::from(witness)) + .map_err(PartialAssetVaultError::FailedToAddProof)?; + self.entries.extend(new_entries); 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 keys = source + .read_many_iter::(num_entries)? + .collect::, _>>()?; - PartialVault::try_from(partial_smt) + Self::from_partial_smt_and_keys(partial_smt, keys) .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } } @@ -194,37 +233,124 @@ impl Deserializable for PartialVault { #[cfg(test)] mod tests { + use alloc::vec::Vec; + use assert_matches::assert_matches; - use miden_crypto::merkle::smt::Smt; 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_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 = partial.open(asset.vault_key()).unwrap_err(); + assert_matches!(err, PartialAssetVaultError::UntrackedAsset(_)); + + 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)?; - let err = PartialVault::try_from(partial_smt).unwrap_err(); - assert_matches!(err, PartialAssetVaultError::InvalidAssetInSmt { .. }); + 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 1ca7747fc3..333f7c2956 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::asset::vault::AssetId; @@ -16,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). /// @@ -33,6 +34,10 @@ use crate::{Felt, Word}; /// The composition is the discriminator between assets and so it is placed at a static offset much /// like the version in an account ID. This makes it slightly easier to change the asset metadata in /// the future without affecting identification of previous assets. +/// +/// 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. @@ -163,9 +168,50 @@ impl AssetVaultKey { self.composition } - /// Returns the leaf index of a vault key. + /// 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 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_word()) + 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 ac1f5b5fd8..dbeeab9bda 100644 --- a/crates/miden-protocol/src/errors/mod.rs +++ b/crates/miden-protocol/src/errors/mod.rs @@ -491,6 +491,8 @@ pub enum AssetError { FungibleAssetValueMostSignificantElementsMustBeZero(Word), #[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("unknown native asset callbacks encoding: {0}")] UnknownAssetCallbackFlag(u8), #[error("unknown asset composition encoding: {0}")] @@ -615,8 +617,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 1c3500ec83..6500138657 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.hash().as_word(); 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 cd0bdaeec0..67da862431 100644 --- a/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs +++ b/crates/miden-protocol/src/transaction/kernel/advice_inputs.rs @@ -287,7 +287,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/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" ); diff --git a/docs/src/asset.md b/docs/src/asset.md index 345d377826..7f837ecbfc 100644 --- a/docs/src/asset.md +++ b/docs/src/asset.md @@ -145,6 +145,8 @@ Examples of such assets include NFTs like a DevCon ticket. [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 fourth element - the faucet ID prefix - which the SMT uses to determine leaf membership). +

Asset storage