fix(contains): treat unresolvable LabelOnly as 'maybe present', not absent

[mwaddip]

Bug

AVLTree::contains_recursive (used by BatchAVLProver::removed_nodes) returns false when the walk reaches any non-Internal node whose label doesn't match the target. That catch-all fires for two distinct cases:

• Leaf with non-matching label — terminal, target genuinely absent. false is correct.
• LabelOnly placeholder the resolver couldn't materialize — we cannot conclude anything about the unresolved subtree. false is unsafe.

removed_nodes() interprets false as "definitely not in tree → mark for deletion":

pub fn removed_nodes(&mut self) -> Vec<NodeId> {
    for cn in &self.base.changed_nodes_buffer_to_check {
        if !self.contains(cn) {
            self.base.changed_nodes_buffer.push(cn.clone())
        }
    }
    ...
}

For a persistent backend that lazily materializes nodes (most of the in-memory tree is LabelOnly after a few flushes), a candidate-for-deletion's key path frequently crosses an unresolved subtree. Pre-fix, those candidates were marked for deletion even when they remained reachable through the unresolved branch. Subsequent walks into that branch hit a LabelOnly whose digest the resolver can no longer find and bail at _ => bail!("Should never reach this point. ...") (modify_helper line 382 of master).

Reproduction

A Rust full node implementing the Ergo mainnet on top of this crate (with the persistence backend from #10) ran a fresh UTXO snapshot bootstrap to mainnet tip and 250 blocks of steady-state validation. A direct on-disk scan of the resulting NODES_TABLE:

Reachable from META_TOP_NODE_HASH:  6,353,404
Missing references (parent in storage, child digest absent): 135
Orphan nodes (in storage but unreachable from root):     378,274

The 135 dangling references are the over-deletions caused by this bug. Each one eventually triggers an apply failure when block validation walks into the missing subtree.

Fix

Distinguish Leaf (terminal, return false) from LabelOnly (couldn't determine, return true to fail safe). At worst this leaks orphan nodes; never silently corrupts.

Refactored to use a Kind enum so the immutable borrow that does the discrimination is scoped separately from the mutable borrow driving the Internal walk (avoids RefCell already mutably borrowed).

Tests

All 22 existing tests pass unchanged. No new behaviour change in the no-storage case (where every walk lands on a real Internal/Leaf).

Relationship to other PRs

• Independent of Change Resolver from fn pointer to Arc<dyn Fn> for storage backends #10 (Resolver type) and feat: Implement disk-based persistence with Redb (Bounty #7) #12 (persistence backend).
• feat: Implement disk-based persistence with Redb (Bounty #7) #12's per-key undo with prior values avoids one related deletion bug (rollback's blind delete of inserted_labels) but doesn't address the forward-path contains() issue here. Anyone using prover.removed_nodes() with a persistent backend is affected regardless of which storage layer they use.

🤖 Generated with Claude Code

[a-shannon]

Great catch and an elegant fix!

As discussed over in PR #12, failing safe and treating unresolved LabelOnly nodes as 'maybe present' is absolutely critical to protect lazily-materialized database backends from false garbage collection during tree rebalancing.

The logic is perfectly sound, and the scoped RefCell borrow workaround with the Kind enum to avoid runtime panics is very clean, idiomatic Rust. LGTM! 🚀