Add typed descriptor builder

[praveenperera]

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling and clearer messages for invalid derivation paths and fingerprints.

• New Features

  • Better support for multiple BIP coin-type derivation standards (e.g., BIP44/49/84/86) and more robust parsing/normalization of fingerprints and paths.

• Refactor

  • Centralized descriptor construction to simplify and stabilize descriptor generation.

• Tests

  • Expanded test coverage across multiple derivation path standards.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 51ba125e-f462-4b4d-9b65-2305eb11a7af

📥 Commits

Reviewing files that changed from the base of the PR and between 10fce65 and 3658538.

📒 Files selected for processing (3)

• src/descriptor.rs
• src/descriptor/builder.rs
• src/descriptor/script_type.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• src/descriptor/builder.rs
• src/descriptor/script_type.rs
• src/descriptor.rs

---

📝 Walkthrough

Walkthrough

Descriptor construction was centralized behind a new public DescriptorBuilder. ScriptType gained hardened derivation helpers (purpose, account_derivation_path_for_coin_type). Descriptor creation and JSON conversions were refactored to use the builder; parsing and fingerprint errors were added and multipath splitting/parsing were encapsulated.

Changes

Descriptor Construction Refactoring

Layer / File(s)	Summary
Data Shape / API src/descriptor/builder.rs, src/descriptor/script_type.rs, src/descriptor.rs	Adds DescriptorBuilder (public) and re-exports it; ScriptType adds purpose() and account_derivation_path_for_coin_type(u32); Error enum adds InvalidDerivationPath, InvalidFingerprint, and InvalidChildNumber.
Core Implementation src/descriptor/builder.rs	DescriptorBuilder::new, account_xpub_for_coin_type, and build create DescriptorPublicKey::MultiXPub with origin (fingerprint + derivation) and multipath change derivations, then build appropriate Miniscript descriptor (PKH/SH-WPKH/WPKH/TR).
Integration / Call Sites src/descriptor.rs	Refactors Descriptors::try_from_line, try_from_single_sig, try_from_child_xpub_with_coin_type, try_from_key_expression, TryFrom<WasabiJson>, and TryFrom<ElectrumJson> to parse fingerprints/derivation paths (via Fingerprint::from_str and parse_derivation_path) and delegate construction to DescriptorBuilder.
Helpers / Parsing src/descriptor.rs	Introduces split_multipath_descriptor(...) to encapsulate multipath splitting/validation and parse_derivation_path(...) to normalize/parse derivation strings.
Tests src/descriptor.rs (test module)	Adds tests and helpers asserting DescriptorBuilder::account_xpub_for_coin_type produces expected external/internal descriptors for BIP44/49/84/86 and an arbitrary coin type across networks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• bitcoinppl/pubport#6 — Modifies descriptor construction/parsing and ScriptType APIs touching the same codepaths.
• bitcoinppl/pubport#9 — Changes coin-type-aware derivation path logic and child-xpub handling related to this refactor.

Poem

🐰 A builder hops with tidy paws,
Paths hardened without a pause,
Fingerprints snug, xpubs aligned,
Multipath branches well-defined,
Descriptors bloom — neat rabbit applause!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: addition of a new DescriptorBuilder struct and API that centralizes descriptor construction behind a typed builder pattern.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch typed-descriptor-builder

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR replaces string-interpolated descriptor construction with a typed DescriptorBuilder that operates directly on Fingerprint, DerivationPath, and Xpub BIP32 types, eliminating an entire class of silent formatting bugs. Error handling is improved with dedicated InvalidDerivationPath and InvalidFingerprint variants, and new tests cover all four BIP derivation standards (44/49/84/86) on both mainnet and testnet coin types.

Confidence Score: 5/5

Safe to merge — clean refactor with no behavioral regressions and good test coverage.

No P0 or P1 findings. The builder correctly uses typed BIP32 structures, split_multipath_descriptor stays private (child modules access parent-private items in Rust without extra qualifiers), parse_derivation_path is strictly more correct than the old replace approach, and coin_type is evaluated before into_bip32() consumes the wrapper.

No files require special attention.

Important Files Changed

Filename	Overview
src/descriptor/builder.rs	New typed builder that constructs miniscript descriptors directly from BIP32 types, replacing the string-interpolation approach
src/descriptor/script_type.rs	Replaced string-returning path methods with typed purpose() and account_derivation_path_for_coin_type(), removing wrap_with() and descriptor_derivation_path()
src/descriptor.rs	Refactored all construction call sites to use DescriptorBuilder; added InvalidDerivationPath / InvalidFingerprint error variants and parse_derivation_path helper; tests extended to cover all BIP derivation paths

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Callers] --> B[account_xpub_for_coin_type]
    A --> C[DescriptorBuilder::new]
    B --> D[account_derivation_path_for_coin_type]
    D --> C
    C --> E[build]
    E --> F[MultiXPub key with multipath change derivations]
    F --> G{script_type}
    G -->|P2pkh| H[new_pkh]
    G -->|P2shP2wpkh| I[new_sh_wpkh]
    G -->|P2wpkh| J[new_wpkh]
    G -->|P2tr| K[new_tr]
    H & I & J & K --> L[split_multipath_descriptor]
    L --> M[Descriptors: external and internal]

Loading

_{Reviews (2): Last reviewed commit: "Address descriptor review comments" | Re-trigger Greptile}