feat: hash AssetVaultKey before insertion into asset vault SMT#2912
Open
partylikeits1983 wants to merge 8 commits into
Open
feat: hash AssetVaultKey before insertion into asset vault SMT#2912partylikeits1983 wants to merge 8 commits into
AssetVaultKey before insertion into asset vault SMT#2912partylikeits1983 wants to merge 8 commits into
Conversation
partylikeits1983
added a commit
that referenced
this pull request
May 12, 2026
partylikeits1983
changed the title
AssetVaultKey before insertion into asset vault SMT
Contributor
|
This is something we need to do but it'll also complicate asset retrieval from the |
Contributor
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
There was a problem hiding this comment.
I looked at parts of the asset vault related changes and left some comments.
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<AssetVaultKey, Word> 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.
partylikeits1983
added
the
pr-from-maintainers
- 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<Word>` 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).
partylikeits1983
force-pushed
the
ajl-claude-hash-asset-vault-key-smt
branch
from
cbb11d4 to
59521ea
Compare
- 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 }`.
…masm Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
…-vault-key-smt # Conflicts: # crates/miden-protocol/src/asset/vault/mod.rs # crates/miden-protocol/src/asset/vault/vault_key.rs # crates/miden-protocol/src/errors/mod.rs
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).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2518.
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 degrading tree balance.This PR mirrors the existing raw-key/hashed-key pattern already used by
StorageMapin this crate:AssetVaultKey::to_smt_key()returns the Poseidon2 hash of the raw key.to_leaf_indexis rerouted through it.AssetVaultandPartialVaultnow store the SMT keyed by hashed keys, plus aBTreeMap<AssetVaultKey, Word>of raw keys for iteration and proof reconstruction.AssetWitnesscarries both theSmtProofand the raw key(s) it covers;newvalidates each(key, value)pair against the proof.hash_asset_keyhelper (exec.poseidon2::hash) and injects it before everysmt::set/smt::get/smt::peekinasset_vault.masm(same pattern ashash_map_keyinaccount.masm).read_vault_asset/read_vault_asset_witnessesupdated for the new layout.A regression test (
two_non_fungible_assets_from_same_faucet_use_different_leaves) covers the original bug end-to-end.