Skip to content

Commit ed96e0f

Browse files
committed
feat(SmtForest): Make the entries iterator fallible
This changes `LargeSmtForest::entries` to return an iterator over items that are `Result<TreeEntry>` instead of bare `TreeEntry`, ensuring that the potential failure of iteration in the backend is communicated clearly to the caller performing the iteration. We also add benchmarks to the iteration in order to confirm that the changes incur no performance impact.
1 parent 5f3dee0 commit ed96e0f

File tree

14 files changed

+243
-122
lines changed

14 files changed

+243
-122
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
- [BREAKING] Removed `RpoRandomCoin` and `RpxRandomCoin` and introduced a Poseidon2-based `RandomCoin` ([#871](https://github.com/0xMiden/crypto/pull/871)).
2727
- Harden MerkleStore deserialization and fuzz coverage ([#878](https://github.com/0xMiden/crypto/pull/878)).
2828
- [BREAKING] Upgraded Plonky3 from 0.4.2 to 0.5.0 and replaced `p3-miden-air`, `p3-miden-fri`, and `p3-miden-prover` with the unified `p3-miden-lifted-stark` crate. The `stark` module now re-exports the Lifted STARK proving system from [p3-miden](https://github.com/0xMiden/p3-miden).
29+
- [BREAKING] Changed the `LargeSmtForest::entries` iterator to be fallible by explicitly returning `Result<TreeEntry>` as the iterator item.
2930

3031
## 0.22.4 (2026-03-03)
3132

miden-crypto/benches/large_smt_forest.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,56 @@ benchmark_with_setup_data! {
134134
},
135135
}
136136

137+
// Measures iteration over the latest version of a tree (the WithoutHistory fast path).
138+
benchmark_with_setup_data! {
139+
large_smt_forest_persistent_entries_current_tree,
140+
DEFAULT_MEASUREMENT_TIME,
141+
DEFAULT_SAMPLE_SIZE,
142+
"large_smt_forest_persistent_entries_current_tree",
143+
|| {
144+
let mut setup = ForestSetup::new_persistent();
145+
let batch = generate_tree_update_batch(BATCH_SIZE);
146+
let lineage = LineageId::new([0x42; 32]);
147+
let version = 0;
148+
setup.forest.add_lineage(lineage, version, batch).unwrap();
149+
let tree = TreeId::new(lineage, version);
150+
(setup, tree)
151+
},
152+
|b: &mut criterion::Bencher, (setup, tree): &(ForestSetup<_>, TreeId)| {
153+
b.iter(|| {
154+
hint::black_box(
155+
setup.forest.entries(*tree).unwrap().map(|e| e.unwrap()).collect::<Vec<_>>()
156+
);
157+
})
158+
}
159+
}
160+
161+
// Measures iteration over a historical version of a tree (the WithHistory state machine path).
162+
benchmark_with_setup_data! {
163+
large_smt_forest_persistent_entries_historical_tree,
164+
DEFAULT_MEASUREMENT_TIME,
165+
DEFAULT_SAMPLE_SIZE,
166+
"large_smt_forest_persistent_entries_historical_tree",
167+
|| {
168+
let mut setup = ForestSetup::new_persistent();
169+
let initial_batch = generate_tree_update_batch(BATCH_SIZE);
170+
let lineage = LineageId::new([0x42; 32]);
171+
let version = 0;
172+
setup.forest.add_lineage(lineage, version, initial_batch).unwrap();
173+
let update_batch = generate_tree_update_batch(BATCH_SIZE);
174+
setup.forest.update_tree(lineage, version + 1, update_batch).unwrap();
175+
let tree = TreeId::new(lineage, version);
176+
(setup, tree)
177+
},
178+
|b: &mut criterion::Bencher, (setup, tree): &(ForestSetup<_>, TreeId)| {
179+
b.iter(|| {
180+
hint::black_box(
181+
setup.forest.entries(*tree).unwrap().map(|e| e.unwrap()).collect::<Vec<_>>()
182+
);
183+
})
184+
},
185+
}
186+
137187
// Roughly equivalent to large_smt::large_smt_insert_batch_to_empty_tree in functionality.
138188
benchmark_batch! {
139189
large_smt_forest_persistent_add_lineage,
@@ -220,6 +270,8 @@ criterion_group!(
220270
large_smt_forest_persistent_group,
221271
large_smt_forest_persistent_open_full_tree,
222272
large_smt_forest_persistent_open_historical_tree,
273+
large_smt_forest_persistent_entries_current_tree,
274+
large_smt_forest_persistent_entries_historical_tree,
223275
large_smt_forest_persistent_add_lineage,
224276
large_smt_forest_persistent_update_tree,
225277
large_smt_forest_persistent_update_forest,

miden-crypto/src/merkle/smt/large_forest/backend/memory/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ impl Backend for InMemoryBackend {
113113
///
114114
/// - [`BackendError::UnknownLineage`] if the provided `lineage` is one not known by this
115115
/// backend.
116-
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = TreeEntry>> {
116+
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = Result<TreeEntry>>> {
117117
let tree = self.trees.get(&lineage).ok_or(BackendError::UnknownLineage(lineage))?;
118-
Ok(tree.tree.entries().map(|(k, v)| TreeEntry { key: *k, value: *v }))
118+
Ok(tree.tree.entries().map(|(k, v)| Ok(TreeEntry { key: *k, value: *v })))
119119
}
120120

121121
/// Adds the provided `lineage` to the forest.

miden-crypto/src/merkle/smt/large_forest/backend/memory/property_tests.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,10 @@ proptest! {
226226
let backend_entries = backend
227227
.entries(target_lineage)
228228
.map_err(to_fail)?
229-
.map(|e| (e.key, e.value))
229+
.map(|e| e.map(|e| (e.key, e.value)))
230+
.collect::<Result<Vec<_>, _>>()
231+
.map_err(to_fail)?
232+
.into_iter()
230233
.sorted()
231234
.collect_vec();
232235
let tree_entries = tree.entries().copied().sorted().collect_vec();

miden-crypto/src/merkle/smt/large_forest/backend/memory/tests.rs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
//! existing [`Smt`] implementation, comparing the results of the in-memory backend against it
66
//! wherever relevant.
77
8+
use alloc::vec::Vec;
9+
810
use assert_matches::assert_matches;
911
use itertools::Itertools;
1012

@@ -385,22 +387,11 @@ fn entries() -> Result<()> {
385387
backend.update_tree(lineage_1, version, operations)?;
386388

387389
// Now, the iterator should yield the expected three items.
388-
assert_eq!(backend.entries(lineage_1)?.count(), 3);
389-
assert!(
390-
backend
391-
.entries(lineage_1)?
392-
.contains(&TreeEntry { key: key_1_1, value: value_1_1 }),
393-
);
394-
assert!(
395-
backend
396-
.entries(lineage_1)?
397-
.contains(&TreeEntry { key: key_1_2, value: value_1_2 }),
398-
);
399-
assert!(
400-
backend
401-
.entries(lineage_1)?
402-
.contains(&TreeEntry { key: key_1_3, value: value_1_3 }),
403-
);
390+
let entries = backend.entries(lineage_1)?.collect::<Result<Vec<_>>>()?;
391+
assert_eq!(entries.len(), 3);
392+
assert!(entries.contains(&TreeEntry { key: key_1_1, value: value_1_1 }));
393+
assert!(entries.contains(&TreeEntry { key: key_1_2, value: value_1_2 }));
394+
assert!(entries.contains(&TreeEntry { key: key_1_3, value: value_1_3 }));
404395

405396
Ok(())
406397
}

miden-crypto/src/merkle/smt/large_forest/backend/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,17 @@ where
121121
///
122122
/// The iterator may yield entries in any arbitrary order, but must not yield entries for which
123123
/// the value is the empty word.
124-
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = TreeEntry>>;
124+
///
125+
/// # Expected Behavior
126+
///
127+
/// Implementations must guarantee the following behavior in addition to the global invariants:
128+
///
129+
/// - If any kind of error occurs during iteration that should be signaled to the user, the
130+
/// iterator must return `Some(Err(...))`. The caller should stop iteration after receiving an
131+
/// error as the iterator state is no longer valid.
132+
/// - `None` will be returned upon successful completion, or at any time after an error has been
133+
/// returned.
134+
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = Result<TreeEntry>>>;
125135

126136
// SINGLE-TREE MODIFIERS
127137
// ============================================================================================

miden-crypto/src/merkle/smt/large_forest/backend/persistent/iterator.rs

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl<'db> PersistentBackendEntriesIterator<'db> {
3939
/// Constructs a new such iterator in the starting state.
4040
///
4141
/// The provided `iterator` must yield items where the key decodes to a `LeafKey` and the value
42-
/// decodes to an `SmtLeaf`. If this is not the case, iteration will panic.
42+
/// decodes to an `SmtLeaf`. If this is not the case, iteration will yield an error.
4343
///
4444
/// For performance, this iterator should be passed a prefix iterator over the database with the
4545
/// correct prefix (corresponding to the provided `lineage`) set, but it will still function
@@ -61,26 +61,38 @@ enum PersistentBackendEntriesIteratorState {
6161
/// The iterator over the current leaf's entries.
6262
leaf_entries: Box<dyn Iterator<Item = (Word, Word)>>,
6363
},
64+
65+
/// The iterator has encountered an error and will yield no further items.
66+
Faulted,
6467
}
6568

6669
impl<'db> Iterator for PersistentBackendEntriesIterator<'db> {
67-
type Item = TreeEntry;
70+
type Item = super::Result<TreeEntry>;
6871

6972
/// Advances the iterator and returns the next item if present.
70-
///
71-
/// # Panics
72-
///
73-
/// - If unable to read from the underlying database.
7473
fn next(&mut self) -> Option<Self::Item> {
7574
loop {
7675
match &mut self.state {
76+
PersistentBackendEntriesIteratorState::Faulted => return None,
7777
PersistentBackendEntriesIteratorState::NotInLeaf => {
7878
// Here we are not in a leaf of the targeted tree, so we have to see if we _can_
7979
// be.
8080
if let Some(entry) = self.iterator.next() {
81-
let (key_bytes, value_bytes) = entry.expect("Able to read from the DB");
82-
let key = LeafKey::read_from_bytes(&key_bytes)
83-
.expect("Leaf key data read from disk is not corrupt");
81+
let (key_bytes, value_bytes) = match entry {
82+
Ok((key_bytes, value_bytes)) => (key_bytes, value_bytes),
83+
Err(e) => {
84+
self.state = PersistentBackendEntriesIteratorState::Faulted;
85+
return Some(Err(e.into()));
86+
},
87+
};
88+
89+
let key = match LeafKey::read_from_bytes(&key_bytes) {
90+
Ok(key) => key,
91+
Err(e) => {
92+
self.state = PersistentBackendEntriesIteratorState::Faulted;
93+
return Some(Err(e.into()));
94+
},
95+
};
8496

8597
// If the key isn't for the correct lineage (which can happen even with the
8698
// bloom filter), we need to advance by returning to the loop.
@@ -90,8 +102,13 @@ impl<'db> Iterator for PersistentBackendEntriesIterator<'db> {
90102

91103
// If the key is valid, we need to read out the leaf itself and then start
92104
// iterating over that.
93-
let leaf = SmtLeaf::read_from_bytes(&value_bytes)
94-
.expect("Leaf data read from disk is not corrupt");
105+
let leaf = match SmtLeaf::read_from_bytes(&value_bytes) {
106+
Ok(leaf) => leaf,
107+
Err(e) => {
108+
self.state = PersistentBackendEntriesIteratorState::Faulted;
109+
return Some(Err(e.into()));
110+
},
111+
};
95112
let mut leaf_entries = leaf.into_entries();
96113
leaf_entries.sort_by_key(|(k, _)| *k);
97114

@@ -106,7 +123,7 @@ impl<'db> Iterator for PersistentBackendEntriesIterator<'db> {
106123
PersistentBackendEntriesIteratorState::InLeaf { leaf_entries } => {
107124
if let Some((key, value)) = leaf_entries.next() {
108125
// Here we have an entry in the leaf, so we simply need to return it.
109-
return Some(TreeEntry { key, value });
126+
return Some(Ok(TreeEntry { key, value }));
110127
} else {
111128
// If we've run out of entries in the leaf itself, we need to see if there
112129
// is another valid leaf. We do this by changing state and looping to use

miden-crypto/src/merkle/smt/large_forest/backend/persistent/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ impl Backend for PersistentBackend {
340340
///
341341
/// - [`BackendError::UnknownLineage`] if the provided `lineage` is not one known by this
342342
/// backend.
343-
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = TreeEntry>> {
343+
fn entries(&self, lineage: LineageId) -> Result<impl Iterator<Item = Result<TreeEntry>>> {
344344
if !self.lineages.contains_key(&lineage) {
345345
return Err(BackendError::UnknownLineage(lineage));
346346
}

miden-crypto/src/merkle/smt/large_forest/backend/persistent/property_tests.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,9 @@ proptest! {
217217
// And we should have the same number of entries in each.
218218
let backend_entries = backend
219219
.entries(target_lineage)?
220-
.map(|e| (e.key, e.value))
220+
.map(|e| e.map(|e| (e.key, e.value)))
221+
.collect::<std::result::Result<Vec<_>, _>>()?
222+
.into_iter()
221223
.sorted()
222224
.collect_vec();
223225
let tree_entries = tree.entries().copied().sorted().collect_vec();

miden-crypto/src/merkle/smt/large_forest/backend/persistent/tests.rs

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
//! existing [`Smt`] implementation, comparing the results of the persistent backend against it
66
//! wherever relevant.
77
8+
use alloc::vec::Vec;
9+
810
use assert_matches::assert_matches;
911
use itertools::Itertools;
1012
use tempfile::{TempDir, tempdir};
@@ -126,15 +128,25 @@ fn load_extant() -> Result<()> {
126128
assert_eq!(l2_value, Some(t2_value));
127129

128130
// ...and entries.
129-
let l1_entries = backend.entries(lineage_1)?.sorted().collect_vec();
131+
let l1_entries = backend
132+
.entries(lineage_1)?
133+
.collect::<std::result::Result<Vec<_>, _>>()?
134+
.into_iter()
135+
.sorted()
136+
.collect_vec();
130137
let t1_entries = tree_1
131138
.entries()
132139
.sorted()
133140
.map(|(k, v)| TreeEntry { key: *k, value: *v })
134141
.collect_vec();
135142
assert_eq!(l1_entries, t1_entries);
136143

137-
let l2_entries = backend.entries(lineage_2)?.sorted().collect_vec();
144+
let l2_entries = backend
145+
.entries(lineage_2)?
146+
.collect::<std::result::Result<Vec<_>, _>>()?
147+
.into_iter()
148+
.sorted()
149+
.collect_vec();
138150
let t2_entries = tree_2
139151
.entries()
140152
.sorted()
@@ -493,22 +505,11 @@ fn entries() -> Result<()> {
493505
backend.update_tree(lineage_1, version, operations)?;
494506

495507
// Now, the iterator should yield the expected three items.
496-
assert_eq!(backend.entries(lineage_1)?.count(), 3);
497-
assert!(
498-
backend
499-
.entries(lineage_1)?
500-
.contains(&TreeEntry { key: key_1_1, value: value_1_1 }),
501-
);
502-
assert!(
503-
backend
504-
.entries(lineage_1)?
505-
.contains(&TreeEntry { key: key_1_2, value: value_1_2 }),
506-
);
507-
assert!(
508-
backend
509-
.entries(lineage_1)?
510-
.contains(&TreeEntry { key: key_1_3, value: value_1_3 }),
511-
);
508+
let entries = backend.entries(lineage_1)?.collect::<Result<Vec<_>>>()?;
509+
assert_eq!(entries.len(), 3);
510+
assert!(entries.contains(&TreeEntry { key: key_1_1, value: value_1_1 }));
511+
assert!(entries.contains(&TreeEntry { key: key_1_2, value: value_1_2 }));
512+
assert!(entries.contains(&TreeEntry { key: key_1_3, value: value_1_3 }));
512513

513514
Ok(())
514515
}

0 commit comments

Comments
 (0)