diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ac9cab5e..58415b9f1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,10 @@ - Added Ownable2Step as an Account Component ([#2572](https://github.com/0xMiden/protocol/pull/2572)) - [BREAKING] Introduced `PrivateNoteHeader` for output notes and removed `RawOutputNote::Header` variant ([#2569](https://github.com/0xMiden/protocol/pull/2569)). +### Fixes + +- Fixed `PartialAccountTree::track_account` rejecting provably-empty leaves in sparse trees by handling `SmtLeaf::Empty` correctly ([#2598](https://github.com/0xMiden/protocol/pull/2598)). + ## 0.13.3 (2026-01-27) - Fixed `CLAIM` note creation to use `NetworkAccountTarget` attachment ([#2352](https://github.com/0xMiden/miden-base/pull/2352)). diff --git a/crates/miden-protocol/src/block/account_tree/partial.rs b/crates/miden-protocol/src/block/account_tree/partial.rs index f663bf5405..4530f77cf0 100644 --- a/crates/miden-protocol/src/block/account_tree/partial.rs +++ b/crates/miden-protocol/src/block/account_tree/partial.rs @@ -111,15 +111,20 @@ impl PartialAccountTree { let id_prefix = witness.id().prefix(); let id_key = AccountIdKey::from(witness.id()).as_word(); - // If a leaf with the same prefix is already tracked by this partial tree, consider it an + // If there exists a tracked leaf with a non-empty entry whose key differs from the one + // we're about to track, then two different account IDs share the same prefix, which is an // error. // - // We return an error even for empty leaves, because tracking the same ID prefix twice - // indicates that different IDs are attempted to be tracked. It would technically not - // violate the invariant of the tree that it only tracks zero or one entries per leaf, but - // since tracking the same ID twice should practically never happen, we return an error, out - // of an abundance of caution. - if self.smt.get_leaf(&id_key).is_ok() { + // Note that if the leaf is empty, that's fine: `PartialSmt::get_leaf` returns + // `Ok(SmtLeaf::Empty)` for any leaf position reachable through provably-empty subtrees, + // even if no proof was explicitly added for that position. In a sparse tree this covers + // most of the leaf space, so treating empty leaves as duplicates would reject nearly every + // second witness. + // + // Also note that the multiple variant cannot occur by construction of the account tree. + if let Ok(SmtLeaf::Single((existing_key, _))) = self.smt.get_leaf(&id_key) + && id_key != existing_key + { return Err(AccountTreeError::DuplicateIdPrefix { duplicate_prefix: id_prefix }); } @@ -275,6 +280,45 @@ mod tests { Ok(()) } + /// Verifies that tracking multiple witnesses succeeds in a sparse tree, where most leaf + /// positions are reachable through provably-empty subtrees, including `SmtLeaf::Empty` + /// leaves that are provably empty but not actually occupied. + #[test] + fn track_succeeds_for_multiple_witnesses_in_sparse_tree() -> anyhow::Result<()> { + let id0 = AccountIdBuilder::default().build_with_seed([10; 32]); + let id1 = AccountIdBuilder::default().build_with_seed([11; 32]); + let id2 = AccountIdBuilder::default().build_with_seed([12; 32]); + + let commitment0 = Word::from([1, 2, 3, 4u32]); + let commitment1 = Word::from([5, 6, 7, 8u32]); + + // Create a tree with only one account (very sparse). + let account_tree = AccountTree::with_entries([(id0, commitment0)])?; + + // Get witnesses for one existing and two new (empty) accounts. + let witness0 = account_tree.open(id0); + let witness1 = account_tree.open(id1); + let witness2 = account_tree.open(id2); + + // Building a partial tree from all three witnesses should succeed: + // id1 and id2 have empty leaves that are provably empty via the sparse tree structure, + // but they are NOT duplicates of id0. + let mut partial_tree = + PartialAccountTree::with_witnesses([witness0, witness1.clone(), witness2])?; + + // Adding the same witness again should also succeed. + partial_tree.track_account(witness1)?; + + // Verify the existing account has its commitment. + assert_eq!(partial_tree.get(id0)?, commitment0); + + // We should be able to insert new state commitments for the new accounts. + partial_tree.upsert_state_commitments([(id1, commitment1)])?; + assert_eq!(partial_tree.get(id1)?, commitment1); + + Ok(()) + } + #[test] fn track_fails_on_duplicate_prefix() { // Use a raw Smt since an account tree would not allow us to get the witnesses for two diff --git a/crates/miden-testing/src/kernel_tests/block/header_errors.rs b/crates/miden-testing/src/kernel_tests/block/header_errors.rs index 37ae0d579f..ffc2e1d2c6 100644 --- a/crates/miden-testing/src/kernel_tests/block/header_errors.rs +++ b/crates/miden-testing/src/kernel_tests/block/header_errors.rs @@ -439,10 +439,10 @@ async fn block_building_fails_on_creating_account_with_duplicate_account_id_pref let err = block.into_header_and_body().unwrap_err(); - // This should fail when we try to _track_ the same two prefixes in the partial tree. + // This should fail when we try to _insert_ the same two prefixes in the partial tree. assert_matches!( err, - ProposedBlockError::AccountWitnessTracking { + ProposedBlockError::AccountIdPrefixDuplicate { source: AccountTreeError::DuplicateIdPrefix { duplicate_prefix } } if duplicate_prefix == id0.prefix() );