-
Notifications
You must be signed in to change notification settings - Fork 80
fix: correctly set track_latest on MMR
#1633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
077be9f
5997955
0cf8350
84a8c01
10e4cd6
8b9910d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,17 +106,22 @@ pub(crate) fn adjust_merkle_path_for_forest( | |
| "Can't adjust merkle path for a forest that does not include the block number" | ||
| ); | ||
|
|
||
| let expected_depth = forest | ||
| .leaf_to_corresponding_tree(block_num.as_usize()) | ||
| .expect("forest includes block number") as usize; | ||
|
||
| let rightmost_index = InOrderIndex::from_leaf_pos(forest.num_leaves() - 1); | ||
|
|
||
| let mut idx = InOrderIndex::from_leaf_pos(block_num.as_usize()); | ||
| let mut path_nodes = vec![]; | ||
| for node in merkle_path.iter() { | ||
| idx = idx.sibling(); | ||
| // Rightmost index is always the biggest value, so if the path contains any node | ||
| // past it, we can discard it for our version of the forest | ||
| if idx <= rightmost_index { | ||
| path_nodes.push((idx, *node)); | ||
| let mut path_nodes = Vec::with_capacity(expected_depth); | ||
| for node in merkle_path.nodes().iter().take(expected_depth) { | ||
| let sibling = idx.sibling(); | ||
| // NOTE: For a leaf that's in a smaller forest, all siblings within the tree depth should be | ||
| // in-bounds so this check and break are mostly redundant/should never be hit | ||
| // (iterating up to `expected_depth` handles it) | ||
| if sibling > rightmost_index { | ||
| break; | ||
| } | ||
| path_nodes.push((sibling, *node)); | ||
| idx = idx.parent(); | ||
| } | ||
|
||
|
|
||
|
|
@@ -146,3 +151,116 @@ pub(crate) async fn fetch_block_header( | |
|
|
||
| Ok((block_header, path_nodes)) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use miden_objects::block::BlockNumber; | ||
| use miden_objects::crypto::merkle::{Forest, InOrderIndex, MerklePath, Mmr, PartialMmr}; | ||
| use miden_objects::{Felt, Word}; | ||
|
|
||
| use super::adjust_merkle_path_for_forest; | ||
|
|
||
| fn word(n: u64) -> Word { | ||
| Word::new([Felt::new(n), Felt::new(0), Felt::new(0), Felt::new(0)]) | ||
| } | ||
|
|
||
| #[test] | ||
| fn adjust_merkle_path_truncates_to_forest_bounds() { | ||
| let forest = Forest::new(5); | ||
| // Forest 5 <=> block 4 is rightmost leaf | ||
| let block_num = BlockNumber::from(4u32); | ||
| let path = MerklePath::new(vec![word(1), word(2), word(3)]); | ||
|
|
||
| let adjusted = adjust_merkle_path_for_forest(&path, block_num, forest); | ||
| // Block 4 conforms a single leaf tree so it should be empty | ||
| assert!(adjusted.is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn adjust_merkle_path_keeps_proof_valid_for_smaller_forest() { | ||
| // Build a proof in a larger forest and ensure truncation does not keep siblings from a | ||
| // different tree in the smaller forest, which would invalidate the proof. | ||
| let mut mmr = Mmr::new(); | ||
| for value in 0u64..8 { | ||
| mmr.add(word(value)); | ||
| } | ||
|
|
||
| let large_forest = Forest::new(8); | ||
| let small_forest = Forest::new(5); | ||
| let leaf_pos = 4usize; | ||
| let block_num = BlockNumber::from(u32::try_from(leaf_pos).unwrap()); | ||
|
|
||
| let proof = mmr.open_at(leaf_pos, large_forest).expect("valid proof"); | ||
| let adjusted_nodes = | ||
| adjust_merkle_path_for_forest(&proof.merkle_path, block_num, small_forest); | ||
| let adjusted_path = MerklePath::new(adjusted_nodes.iter().map(|(_, n)| *n).collect()); | ||
|
|
||
| let peaks = mmr.peaks_at(small_forest).unwrap(); | ||
| let mut partial = PartialMmr::from_peaks(peaks); | ||
| let leaf = mmr.get(leaf_pos).expect("leaf exists"); | ||
|
|
||
| partial | ||
| .track(leaf_pos, leaf, &adjusted_path) | ||
| .expect("adjusted path should verify against smaller forest peaks"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn adjust_merkle_path_correct_indices() { | ||
| // Forest 6 has trees of size 2 and 4 | ||
| let forest = Forest::new(6); | ||
| // Block 1 is on tree with size 4 (merkle path should have 2 nodes) | ||
| let block_num = BlockNumber::from(1u32); | ||
| let nodes = vec![word(10), word(11), word(12), word(13)]; | ||
| let path = MerklePath::new(nodes.clone()); | ||
|
|
||
| let adjusted = adjust_merkle_path_for_forest(&path, block_num, forest); | ||
|
|
||
| assert_eq!(adjusted.len(), 2); | ||
| assert_eq!(adjusted[0].1, nodes[0]); | ||
| assert_eq!(adjusted[1].1, nodes[1]); | ||
|
|
||
| let mut idx = InOrderIndex::from_leaf_pos(1); | ||
| let expected0 = idx.sibling(); | ||
| idx = idx.parent(); | ||
| let expected1 = idx.sibling(); | ||
|
|
||
| assert_eq!(adjusted[0].0, expected0); | ||
| assert_eq!(adjusted[1].0, expected1); | ||
| } | ||
|
|
||
| #[test] | ||
| fn adjust_path_limit_correct_when_siblings_in_bounds() { | ||
| // Ensure the expected depth limit matters even when the next sibling | ||
| // is "in-bounds" (but not part of the leaf's subtree for that forest) | ||
| let large_leaves = 8usize; | ||
| let small_leaves = 7usize; | ||
| let leaf_pos = 2usize; | ||
| let mut mmr = Mmr::new(); | ||
| for value in 0u64..large_leaves as u64 { | ||
| mmr.add(word(value)); | ||
| } | ||
|
|
||
| let small_forest = Forest::new(small_leaves); | ||
| let proof = mmr.open_at(leaf_pos, Forest::new(large_leaves)).expect("valid proof"); | ||
| let expected_depth = | ||
| small_forest.leaf_to_corresponding_tree(leaf_pos).expect("leaf is in forest") as usize; | ||
|
|
||
| // Confirm the next sibling after the expected depth is still in bounds, which would | ||
| // create an overlong path without the depth cap. | ||
| let mut idx = InOrderIndex::from_leaf_pos(leaf_pos); | ||
| for _ in 0..expected_depth { | ||
| idx = idx.parent(); | ||
| } | ||
| let next_sibling = idx.sibling(); | ||
| let rightmost = InOrderIndex::from_leaf_pos(small_leaves - 1); | ||
| assert!(next_sibling <= rightmost); | ||
| assert!(proof.merkle_path.depth() as usize > expected_depth); | ||
|
|
||
| let adjusted = adjust_merkle_path_for_forest( | ||
| &proof.merkle_path, | ||
| BlockNumber::from(u32::try_from(leaf_pos).unwrap()), | ||
| small_forest, | ||
| ); | ||
| assert_eq!(adjusted.len(), expected_depth); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also change the CI job to try and catch if debug assertions fail