diff --git a/CHANGELOG.md b/CHANGELOG.md index 33c53fdd2..b080df27d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,13 @@ ## 0.23.0 (TBD) - [BREAKING] `PartialMmr::open()` now returns `Option` instead of `Option` ([#787](https://github.com/0xMiden/crypto/pull/787)). -- Fixed `SmtForest` to remove nodes with zero reference count from store ([#821](https://github.com/0xMiden/crypto/pull/821)). - [BREAKING] Refactored BLAKE3 to use `Digest` struct, added `Digest192` type alias ([#811](https://github.com/0xMiden/crypto/pull/811)). - [BREAKING] Removed `hashbrown` dependency and `hashmaps` feature; `Map`/`Set` type aliases are now tied to the `std` feature ([#813](https://github.com/0xMiden/crypto/pull/813)). - [BREAKING] Renamed `NodeIndex::value()` to `NodeIndex::position()`, `NodeIndex::is_value_odd()` to `NodeIndex::is_position_odd()`, and `LeafIndex::value()` to `LeafIndex::position()` ([#814](https://github.com/0xMiden/crypto/pull/814)). -- Fixed tuple `min_serialized_size()` to exclude alignment padding, fixing `BudgetedReader` rejecting valid data ([#827](https://github.com/0xMiden/crypto/pull/827)). +- [BREAKING] Fix OOMs in Merkle/SMT deserialization ([#820](https://github.com/0xMiden/crypto/pull/820)). +- Fixed `SmtForest` to remove nodes with zero reference count from store ([#821](https://github.com/0xMiden/crypto/pull/821)). - Cross-checked RPO test vectors against the Python reference implementation after state layout change ([#822](https://github.com/0xMiden/crypto/pull/822)). +- Fixed tuple `min_serialized_size()` to exclude alignment padding, fixing `BudgetedReader` rejecting valid data ([#827](https://github.com/0xMiden/crypto/pull/827)). ## 0.22.2 (2026-02-01) diff --git a/miden-crypto/src/merkle/index.rs b/miden-crypto/src/merkle/index.rs index 522527e09..d5184acc9 100644 --- a/miden-crypto/src/merkle/index.rs +++ b/miden-crypto/src/merkle/index.rs @@ -210,6 +210,11 @@ impl Deserializable for NodeIndex { NodeIndex::new(depth, position) .map_err(|_| DeserializationError::InvalidValue("Invalid index".into())) } + + fn min_serialized_size() -> usize { + // u8 (depth) + u64 (value) + 9 + } } /// Implementation for [`NodeIndex::proof_indices()`]. diff --git a/miden-crypto/src/merkle/partial_mt/mod.rs b/miden-crypto/src/merkle/partial_mt/mod.rs index d0cfa1af3..d7fbd7157 100644 --- a/miden-crypto/src/merkle/partial_mt/mod.rs +++ b/miden-crypto/src/merkle/partial_mt/mod.rs @@ -88,22 +88,29 @@ impl PartialMerkleTree { /// /// # Errors /// Returns an error if: - /// - If the depth is 0 or is greater than 64. + /// - Any entry has depth 0 or is greater than 64. /// - The number of entries exceeds the maximum tree capacity, that is 2^{depth}. /// - The provided entries contain an insufficient set of nodes. /// - Any entry is an ancestor of another entry (creates hash ambiguity). + /// + /// An empty input returns an empty tree. pub fn with_leaves(entries: R) -> Result where R: IntoIterator, I: Iterator + ExactSizeIterator, { + let entries = entries.into_iter(); + if entries.len() == 0 { + return Ok(PartialMerkleTree::new()); + } + let mut layers: BTreeMap> = BTreeMap::new(); let mut leaves = BTreeSet::new(); let mut nodes = BTreeMap::new(); // add data to the leaves and nodes maps and also fill layers map, where the key is the // depth of the node and value is its index. - for (node_index, hash) in entries.into_iter() { + for (node_index, hash) in entries { leaves.insert(node_index); nodes.insert(node_index, hash); layers @@ -466,15 +473,14 @@ impl Serializable for PartialMerkleTree { impl Deserializable for PartialMerkleTree { fn read_from(source: &mut R) -> Result { - let leaves_len = source.read_u64()? as usize; - let mut leaf_nodes = Vec::with_capacity(leaves_len); - - // add leaf nodes to the vector - for _ in 0..leaves_len { - let index = NodeIndex::read_from(source)?; - let hash = Word::read_from(source)?; - leaf_nodes.push((index, hash)); - } + let leaves_len_u64 = source.read_u64()?; + let leaves_len = usize::try_from(leaves_len_u64).map_err(|_| { + DeserializationError::InvalidValue("PartialMerkleTree leaf count too large".into()) + })?; + + // Use read_many_iter to avoid eager allocation and respect BudgetedReader limits + let leaf_nodes: Vec<(NodeIndex, Word)> = + source.read_many_iter(leaves_len)?.collect::>()?; let pmt = PartialMerkleTree::with_leaves(leaf_nodes).map_err(|_| { DeserializationError::InvalidValue("Invalid data for PartialMerkleTree creation".into()) @@ -482,4 +488,9 @@ impl Deserializable for PartialMerkleTree { Ok(pmt) } + + /// Minimum serialized size: u64 length prefix (0 entries). + fn min_serialized_size() -> usize { + 8 + } } diff --git a/miden-crypto/src/merkle/partial_mt/tests.rs b/miden-crypto/src/merkle/partial_mt/tests.rs index 1a07aafdd..75f94fb8f 100644 --- a/miden-crypto/src/merkle/partial_mt/tests.rs +++ b/miden-crypto/src/merkle/partial_mt/tests.rs @@ -4,7 +4,7 @@ use super::{ super::{ MerkleError, MerkleTree, NodeIndex, PartialMerkleTree, int_to_node, store::MerkleStore, }, - Deserializable, InnerNodeInfo, MerkleProof, Serializable, Word, + Deserializable, DeserializationError, InnerNodeInfo, MerkleProof, Serializable, Word, }; // TEST DATA @@ -89,6 +89,37 @@ fn err_with_leaves() { assert!(PartialMerkleTree::with_leaves(leaf_nodes).is_err()); } +/// Checks that `with_leaves()` accepts an empty input and returns an empty tree. +#[test] +fn with_leaves_empty() { + let leaf_nodes: BTreeMap = BTreeMap::new(); + + let pmt = PartialMerkleTree::with_leaves(leaf_nodes).unwrap(); + + assert_eq!(PartialMerkleTree::new().root(), pmt.root()); + assert_eq!(0, pmt.max_depth()); +} + +/// Checks that `read_from_bytes_with_budget()` accepts an empty input. +#[test] +fn deserialize_empty_with_budget() { + let pmt = PartialMerkleTree::new(); + let bytes = pmt.to_bytes(); + + let parsed = PartialMerkleTree::read_from_bytes_with_budget(&bytes, bytes.len()).unwrap(); + assert_eq!(pmt, parsed); +} + +/// Checks that oversized leaf counts are rejected during deserialization. +#[test] +fn deserialize_rejects_oversized_length() { + let mut bytes = Vec::new(); + u64::MAX.write_into(&mut bytes); + + let result = PartialMerkleTree::read_from_bytes_with_budget(&bytes, bytes.len()); + assert!(matches!(result, Err(DeserializationError::InvalidValue(_)))); +} + /// Tests that `with_leaves()` returns `EntryIsNotLeaf` error when an entry /// is an ancestor of another entry. #[test] diff --git a/miden-crypto/src/merkle/smt/full/leaf.rs b/miden-crypto/src/merkle/smt/full/leaf.rs index 7ea1dcca9..fee6923fb 100644 --- a/miden-crypto/src/merkle/smt/full/leaf.rs +++ b/miden-crypto/src/merkle/smt/full/leaf.rs @@ -416,18 +416,18 @@ impl Deserializable for SmtLeaf { LeafIndex::new_max_depth(value) }; - // Read: entries - let mut entries: Vec<(Word, Word)> = Vec::new(); - for _ in 0..num_entries { - let key: Word = source.read()?; - let value: Word = source.read()?; - - entries.push((key, value)); - } + // Read: entries using read_many_iter to avoid eager allocation + let entries: Vec<(Word, Word)> = + source.read_many_iter(num_entries)?.collect::>()?; Self::new(entries, leaf_index) .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } + + /// Minimum serialized size: vint64 (num_entries) + u64 (leaf_index) with 0 entries. + fn min_serialized_size() -> usize { + 1 + 8 + } } // HELPER FUNCTIONS diff --git a/miden-crypto/src/merkle/smt/full/mod.rs b/miden-crypto/src/merkle/smt/full/mod.rs index efa23f7f4..36871ba4c 100644 --- a/miden-crypto/src/merkle/smt/full/mod.rs +++ b/miden-crypto/src/merkle/smt/full/mod.rs @@ -612,17 +612,19 @@ impl Deserializable for Smt { fn read_from(source: &mut R) -> Result { // Read the number of filled leaves for this Smt let num_filled_leaves = source.read_usize()?; - let mut entries = Vec::with_capacity(num_filled_leaves); - for _ in 0..num_filled_leaves { - let key = source.read()?; - let value = source.read()?; - entries.push((key, value)); - } + // Use read_many_iter to avoid eager allocation and respect BudgetedReader limits + let entries: Vec<(Word, Word)> = + source.read_many_iter(num_filled_leaves)?.collect::>()?; Self::with_entries(entries) .map_err(|err| DeserializationError::InvalidValue(err.to_string())) } + + /// Minimum serialized size: vint64 length prefix (0 entries). + fn min_serialized_size() -> usize { + 1 + } } // FUZZING diff --git a/miden-crypto/src/merkle/smt/full/tests.rs b/miden-crypto/src/merkle/smt/full/tests.rs index 703478445..f191492b7 100644 --- a/miden-crypto/src/merkle/smt/full/tests.rs +++ b/miden-crypto/src/merkle/smt/full/tests.rs @@ -709,6 +709,16 @@ fn test_smt_check_empty_root_constant() { assert_eq!(empty_root_64_depth, Smt::EMPTY_ROOT); } +/// Tests that empty SMT deserializes under a tight budget. +#[test] +fn test_empty_smt_deserialization_with_budget() { + let smt = Smt::default(); + let bytes = smt.to_bytes(); + + let parsed = Smt::read_from_bytes_with_budget(&bytes, bytes.len()).unwrap(); + assert_eq!(smt, parsed); +} + // SMT LEAF // -------------------------------------------------------------------------------------------- @@ -724,6 +734,15 @@ fn test_empty_smt_leaf_serialization() { assert_eq!(empty_leaf, deserialized); } +#[test] +fn test_empty_smt_leaf_deserialization_with_budget() { + let empty_leaf = SmtLeaf::new_empty(LeafIndex::new_max_depth(42)); + let bytes = empty_leaf.to_bytes(); + + let deserialized = SmtLeaf::read_from_bytes_with_budget(&bytes, bytes.len()).unwrap(); + assert_eq!(empty_leaf, deserialized); +} + #[test] fn test_single_smt_leaf_serialization() { let single_leaf = SmtLeaf::new_single( diff --git a/miden-crypto/src/word/mod.rs b/miden-crypto/src/word/mod.rs index e6fc2f270..20639b46e 100644 --- a/miden-crypto/src/word/mod.rs +++ b/miden-crypto/src/word/mod.rs @@ -688,6 +688,10 @@ impl Deserializable for Word { Ok(Self(inner)) } + + fn min_serialized_size() -> usize { + Self::SERIALIZED_SIZE + } } // ITERATORS