fix(btc-verification): bind inclusion proof to the block's Merkle tree

[vladb-ai]

Replicated from alpenlabs#175

Description

TxidInclusionProof::verify reconstructed the Merkle root by folding the supplied siblings into the leaf txid, but never checked those siblings against the tree the root actually commits to. It therefore proved a weaker statement than intended — "some fold reaches the root" rather than "the leaf sits at index i of an N-leaf tree". Two consequences, both covered by the added test:

• a zero-length proof passes off any single hash (e.g. an internal Merkle node) as a whole-block root; and
• an out-of-range position verifies against the real root, since only the low siblings.len() bits of the position feed left/right ordering.

The fix requires the caller to pass the block's transaction count and rejects any proof whose position is not a valid leaf index (< tx_count) or whose sibling count differs from the tree depth ceil(log2(tx_count)). The depth check is Bitcoin Core's standard mitigation against the 64-byte node/transaction ambiguity, where an internal node presented as a leaf yields a proof shorter than the true tree depth.

Notes to Reviewers

This is soundness hardening of the proof verifier, not a patch for an actively exploitable bug. The sole production caller (check_block_integrity) verifies the coinbase against a PoW-committed header.merkle_root and gates on is_coinbase(), and the guest always has the full block. A working forgery would therefore need either a full 256-bit preimage on the committed root, or a 64-byte string that is simultaneously a real block's internal Merkle node and a structurally valid coinbase — neither is feasible. The change makes verify sound on its own rather than relying on those external invariants, which matters because verify/compute_root are pub and reusable elsewhere.

Separately: the coinbase inclusion proof barely optimizes anything, since the segwit path already walks all of txdata to compute the witness root. Recomputing the txid Merkle root directly (as the non-segwit path does) would bind the coinbase fully and make the proof — and this whole bug class — unnecessary. Worth a follow-up ticket; out of scope here.

Type of Change

• Security fix

Checklist

• I have performed a self-review of my code.
• I have commented my code where necessary.
• My changes do not introduce new warnings.
• I have added tests that prove my changes are effective or that my feature works.
• New and existing tests pass with my changes.

[github-actions]

🔒 AI Security Review

Scope: crates/btc-verification/src/inclusion_proof.rs, crates/btc-verification/src/body_verification.rs

This PR is itself a security fix (binding TxidInclusionProof::verify to the block's actual Merkle tree via a caller-supplied tx_count, rejecting proofs whose sibling count doesn't match ceil(log2(tx_count)) or whose position is out of range). I reviewed the change and the surrounding code for correctness and for any new issues it might introduce.

Findings

None. No exploitable vulnerabilities are introduced or exposed by this diff.

Verification notes

• Depth formula correctness: merkle_tree_depth(n) = usize::BITS - (n-1).leading_zeros() for n >= 2, 0 for n <= 1. I checked this against the actual number of iterations TxidInclusionProof::generate performs (its while curr_level.len() > 1 loop halves-and-duplicates each level) — they match for all n, including the odd/duplicate-last-node cases. Verified against the added test_merkle_tree_depth cases by hand (n=1,2,3,4,5,8,9).
• Bound checks are sufficient: tx_count == 0 and position >= tx_count are rejected before any hashing occurs, and since a valid position < tx_count <= 2^depth, all higher bits of position beyond depth are guaranteed zero — so there's no leftover ambiguity in the left/right folding in compute_root. No integer overflow/underflow: n - 1 in merkle_tree_depth only executes on the n >= 2 match arm, and position as usize is a safe widening cast.
• Single call site, correctly wired: check_block_integrity (body_verification.rs:87-91) passes txdata.len() — the block's actual transaction count backing header.merkle_root — not anything attacker-influenced independent of the block itself. Grepped the whole tree for other TxidInclusionProof usages/constructions (worker, stf, prover, proof-statements, tests) — none call .verify() directly, so the signature change (adding the required tx_count param) didn't leave any other call site silently under-constrained.
• No regression in resource usage: the new length/position checks run before compute_root's hashing loop, so malformed proofs are now rejected cheaper than before, not more expensively.
• Out of scope / pre-existing, not introduced by this diff: Bitcoin's classic duplicate-last-node ambiguity (CVE-2012-2459-style) in generate()'s odd-level duplication, and any unbounded-length Vec<Buf32> exposure at SSZ decode time, both predate this change and are unaffected by it.

Overall the fix closes the described gap (zero-length proofs impersonating internal nodes as leaves, and out-of-range positions folding into the real root) without introducing new issues.