Skip to content

Conversation

@charlie-dalio
Copy link

This PR implements end-to-end Taproot script‐path support in bp-derive, including:

  • Full stack-based Merkle-tree logic in TapTree::merkle_root and TapTree::merkle_path

  • Integration of the new TapCode op-codes from PR #120 to build leaf scripts

  • Enhanced ControlBlockFactory to track and pop a vector of Merkle paths per leaf

  • Comprehensive unit tests (taptree_tests, negative_tests, control_block_factory_tests) covering:

    • Single-leaf, two-leaf, and unbalanced-tree merkle roots/paths
    • Edge cases: empty tree panic, depth overflow, duplicate leaves, builder errors
  • Dependency upgrades:

    • bitcoin = "0.29", secp256k1 = "0.30.0" (with rand), plus matching secp256k1-sys
    • Pinned bp-core and related crates to specific Git commits for reproducible builds

Key Changes

  1. TapTree Merkle Logic

    • Replaced todo!() with a generic stack‐merge algorithm that:

      • Pushes each (depth, hash)
      • While top two items share the same depth, pops them, hashes into a parent, and re-pushes at depth–1
    • merkle_path(index) uses a stack of (depth, hash, path_vec, is_target) to collect only the sibling branch hashes for the chosen leaf.

  2. TapCode Leaf Construction

    • Switched all tests to call TapScript::push_opcode(op) on the new TapCode enum, ensuring the PR #120 op-codes are actually used.
  3. ControlBlockFactory

    • Switched from a single blank path field to merkle_paths: Vec<TapMerklePath>
    • On Iterator::next(), pops one leaf and its matching path, then constructs ControlBlock::with(version, internal_pk, parity, path).
  4. Unit & Negative Tests

    • taptree_tests: validates correct roots/paths for various tree shapes

    • negative_tests:

      • merkle_path(empty_tree) now deliberately panics with "unbalanced tap tree" on misuse—this is consistent with our policy to catch invalid tree use at development time rather than silently returning bogus data.
      • Builder error when no leaves or overflow depth.
    • control_block_factory_tests: ensures each ControlBlock carries its correct Merkle branch and script version.

  5. Dependencies

    • Upgraded to latest bitcoin_hashes, bitcoin, and secp256k1 versions; pinned bp-core et al. to Git commit for consistency.
    • Enabled rand feature for secp256k1 in workspace and derive crates.

Behavioral & Compatibility Notes

  • Empty-tree panic (merkle_path on TapTree(vec![])) is intentional: an empty tap tree is invalid by spec, so a debug assertion and panic surface programming errors early.
  • All existing public APIs (TapTree, ControlBlockFactory::with, etc.) retain their signatures.
  • Performance remains roughly O(n) in leaves; heavy cloning only occurs in tests or single-threaded builder paths.

Testing

  • cargo test -p bp-derive (all 16 tests passing)
  • ✅ Local lib and bin builds including WASM targets
  • ✅ Dependency resolution via cargo check & CI

I welcome any feedback—happy to iterate on panic-vs-error behavior or naming refinements at your direction!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant