diff --git a/CHANGELOG.md b/CHANGELOG.md index 135f24209..c69a0d96a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * Incremented the limits for various RPC calls to accommodate larger data sets ([#1621](https://github.com/0xMiden/miden-client/pull/1621)). * [BREAKING] Introduced named storage slots, changed `FilesystemKeystore` to not be generic over RNG ([#1626](https://github.com/0xMiden/miden-client/pull/1626)). * Added `submit_new_transaction_with_prover` to the Rust client and `submitNewTransactionWithProver` to the WebClient([#1622](https://github.com/0xMiden/miden-client/pull/1622)). +* Fixed MMR reconstruction code and fixed how block authentication paths are adjusted ([#1633](https://github.com/0xMiden/miden-client/pull/1633)). * Added WebClient bindings and RPC helpers for additional account, note, and validation workflows ([#1638](https://github.com/0xMiden/miden-client/pull/1638)). * [BREAKING] Modified JS binding for `AccountComponent::compile` which now takes an `AccountComponentCode` built with the newly added binding `CodeBuilder::compile_account_component_code` ([#1627](https://github.com/0xMiden/miden-client/pull/1627)). diff --git a/Makefile b/Makefile index 146c74933..9eecea5cc 100644 --- a/Makefile +++ b/Makefile @@ -149,6 +149,14 @@ integration-test-full: ## Run the integration test binary with ignored tests inc TEST_WITH_NOTE_TRANSPORT=1 cargo nextest run --workspace $(EXCLUDE_WASM_PACKAGES) --exclude testing-remote-prover --release --test=integration cargo nextest run --workspace $(EXCLUDE_WASM_PACKAGES) --exclude testing-remote-prover --release --test=integration --run-ignored ignored-only -- import_genesis_accounts_can_be_used_for_transactions +.PHONY: test-dev +test-dev: ## Run tests with debug assertions enabled via test-dev profile + cargo nextest run --workspace $(EXCLUDE_WASM_PACKAGES) --exclude testing-remote-prover --cargo-profile test-dev --lib $(FEATURES_CLIENT) + +.PHONY: integration-test-dev +integration-test-dev: ## Run integration tests with debug assertions enabled via test-dev profile + cargo nextest run --workspace $(EXCLUDE_WASM_PACKAGES) --exclude testing-remote-prover --cargo-profile test-dev --test=integration + .PHONY: integration-test-binary integration-test-binary: ## Run the integration tests using the standalone binary (requires note transport service) TEST_WITH_NOTE_TRANSPORT=1 cargo run --package miden-client-integration-tests --release --locked diff --git a/crates/rust-client/src/store/mod.rs b/crates/rust-client/src/store/mod.rs index 634ffb3fa..65bdaddd5 100644 --- a/crates/rust-client/src/store/mod.rs +++ b/crates/rust-client/src/store/mod.rs @@ -438,8 +438,12 @@ pub trait Store: Send + Sync { let has_client_notes = has_client_notes.into(); current_partial_mmr.add(current_block.commitment(), has_client_notes); + // Only track the latest leaf if it is relevant (it has client notes) _and_ the forest + // actually has a single leaf tree bit + let track_latest = has_client_notes && current_partial_mmr.forest().has_single_leaf_tree(); + let current_partial_mmr = - PartialMmr::from_parts(current_partial_mmr.peaks(), tracked_nodes, has_client_notes); + PartialMmr::from_parts(current_partial_mmr.peaks(), tracked_nodes, track_latest); Ok(current_partial_mmr) } diff --git a/crates/rust-client/src/sync/block_header.rs b/crates/rust-client/src/sync/block_header.rs index 52ad818f3..af1560a9c 100644 --- a/crates/rust-client/src/sync/block_header.rs +++ b/crates/rust-client/src/sync/block_header.rs @@ -100,22 +100,15 @@ pub(crate) fn adjust_merkle_path_for_forest( block_num: BlockNumber, forest: Forest, ) -> Vec<(InOrderIndex, Word)> { - assert!( - forest.num_leaves() > block_num.as_usize(), - "Can't adjust merkle path for a forest that does not include the block number" - ); - - let rightmost_index = InOrderIndex::from_leaf_pos(forest.num_leaves() - 1); + let expected_path_len = forest + .leaf_to_corresponding_tree(block_num.as_usize()) + .expect("forest includes block number") as usize; 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_path_len); + + for node in merkle_path.nodes().iter().take(expected_path_len) { + path_nodes.push((idx.sibling(), *node)); idx = idx.parent(); } @@ -145,3 +138,117 @@ pub(crate) async fn fetch_block_header( Ok((block_header, path_nodes)) } + +#[cfg(test)] +mod tests { + use miden_protocol::block::BlockNumber; + use miden_protocol::crypto::merkle::MerklePath; + use miden_protocol::crypto::merkle::mmr::{Forest, InOrderIndex, Mmr, PartialMmr}; + use miden_protocol::{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); + } +}