fix: clear stale leaves and inner nodes in set_subtree

[sashass1315]

What was wrong?

set_subtree()

crypto/miden-crypto/src/merkle/smt/simple/mod.rs

Line 292 in bbf7493

pub fn set_subtree<const SUBTREE_DEPTH: u8>(

did not remove existing leaves and inner nodes in the insertion region before adding new subtree data. This caused stale data to remain in the tree, breaking the invariant leaves.is_empty() == (root == EMPTY_ROOT)

crypto/miden-crypto/src/merkle/smt/simple/mod.rs

Lines 186 to 190 in bbf7493

	/// Returns a boolean value indicating whether the SMT is empty.
	pub fn is_empty(&self) -> bool {
	debug_assert_eq!(self.leaves.is_empty(), self.root == Self::EMPTY_ROOT);
	self.root == Self::EMPTY_ROOT
	}

and causing num_leaves()

crypto/miden-crypto/src/merkle/smt/simple/mod.rs

Lines 144 to 147 in bbf7493

	/// Returns the number of non-empty leaves in this tree.
	pub fn num_leaves(&self) -> usize {
	self.leaves.len()
	}

to return incorrect values. In debug mode, calling is_empty()

crypto/miden-crypto/src/merkle/smt/simple/mod.rs

Lines 186 to 190 in bbf7493

	/// Returns a boolean value indicating whether the SMT is empty.
	pub fn is_empty(&self) -> bool {
	debug_assert_eq!(self.leaves.is_empty(), self.root == Self::EMPTY_ROOT);
	self.root == Self::EMPTY_ROOT
	}

after inserting an empty subtree would panic.

What changed?

Added cleanup logic to set_subtree() that removes all leaves and inner nodes within the insertion region before adding the new subtree's data. Also added tests to verify the fix.

[huitseeker]

The new tests cover empty subtree insertion well, but don't verify behavior when inserting a non-empty subtree over existing data. This is the main scenario the stale data fix targets. Could we add a test that inserts a non-empty subtree at depth > 1 into a populated tree and verifies the root matches a fresh build?

[huitseeker]

The if let Ok(node_index) = NodeIndex::new(node_depth, node_value) pattern here silently skips any indices that fail validation. Is this hiding an error condition?

Looking at the loop logic:

• node_depth ranges from subtree_root_insertion_depth + 1 to DEPTH
• num_nodes_at_depth = 2^depth_offset where depth_offset = node_depth - subtree_root_insertion_depth
• node_value ranges from node_value_offset to node_value_offset + num_nodes_at_depth - 1

For a valid NodeIndex, position must be less than 2^depth. Here that means node_value < 2^(subtree_root_insertion_depth + depth_offset) which equals subtree_insertion_index * 2^depth_offset + 2^depth_offset = (subtree_insertion_index + 1) * num_nodes_at_depth.

Since subtree_insertion_index is validated at line 307 to be < 2^subtree_root_insertion_depth, the math should always work out. So if NodeIndex::new returns an error, it would indicate a bug in the calculation logic that should probably not be silently ignored.

Would it make sense to use expect() here instead, similar to line 357? Or is there a case where this can legitimately fail?

[huitseeker]

The CI changelog check is failing. Could you add an entry for this PR under the ## 0.23.0 (TBD) section in CHANGELOG.md? Suggested entry:

• Fixed SimpleSmt::set_subtree to clear stale leaves and inner nodes in the insertion region.