bitcoindevkit / bitcoindevkit/bdk-python

test: expand offline descriptor test coverage to include all BIP descriptor constructors

Open
#51 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
11
Forks
12
Avg merge
2d 13h
Merged PRs (30d)
2

Description

### What is missing

`tests/test_offline_descriptor.py` currently contains a single test:

```
test_descriptor_bip86 — tests Descriptor.new_bip86() for the external keychain only
```

`bdkpython` exposes 8 BIP descriptor template constructors (from `bdk_wallet::template`)
across 4 standards, each with a private key variant (`DescriptorSecretKey`) and a
watch-only public key variant (`DescriptorPublicKey`), plus 3 miniscript multisig
constructors. None of the following have test coverage today:

| Constructor | Standard | Script type | Tested? |
|---|---|---|---|
| `Descriptor.new_bip44()` | BIP44 | `pkh(...)` | No |
| `Descriptor.new_bip44_public()` | BIP44 | `pkh(...)` | No |
| `Descriptor.new_bip49()` | BIP49 | `sh(wpkh(...))` | No |
| `Descriptor.new_bip49_public()` | BIP49 | `sh(wpkh(...))` | No |
| `Descriptor.new_bip84()` | BIP84 | `wpkh(...)` | No |
| `Descriptor.new_bip84_public()` | BIP84 | `wpkh(...)` | No |
| `Descriptor.new_bip86()` internal | BIP86 | `tr(...)` | No |
| `Descriptor.new_bip86_public()` | BIP86 | `tr(...)` | No |
| `Descriptor.new_wsh_sortedmulti()` | miniscript | `wsh(sortedmulti(...))` | No |
| `Descriptor.new_sh_sortedmulti()` | miniscript | `sh(sortedmulti(...))` | No |
| `Descriptor.new_sh_wsh_sortedmulti()` | miniscript | `sh(wsh(sortedmulti(...)))` | No |

### What I propose to add

13 new tests in `tests/test_offline_descriptor.py` covering:

- **BIP44** — external keychain, internal keychain, and watch-only (`new_bip44_public`) variant
- **BIP49** — external keychain, internal keychain, and watch-only (`new_bip49_public`) variant
- **BIP84** — external keychain, internal keychain, and watch-only (`new_bip84_public`) variant
- **BIP86** — internal keychain and watch-only (`new_bip86_public`) variant (external already exists)
- **Multisig** — `new_wsh_sortedmulti` and `new_sh_sortedmulti` 2-of-3

I am intentionally leaving `new_sh_wsh_sortedmulti` out of this PR to keep the scope
focused, and can follow up with a separate issue if the maintainers consider it worthwhile.

All tests will be:
- Fully **offline** — no network access required
- **Deterministic** — fixed test mnemonic so expected outputs are independently verifiable
with any compatible BIP32/BIP44/BIP49/BIP84/BIP86 implementation
- Following the **same pattern** as the existing test: fixed mnemonic → derive key → assert full descriptor string including checksum

### Why this matters

Descriptor construction is the first thing every `bdkpython` user does — it is required
to create a `Wallet`. Without test coverage for these constructors, a regression in the
FFI layer (e.g. wrong derivation path, wrong script type, wrong network prefix, wrong
checksum) would go undetected until a user reports it in production.

Each test pins the full descriptor output string to a known-correct value, which means
any future change to the FFI type conversion or the `bdk_wallet::template` behavior
that silently produces wrong output will immediately fail CI.

This is a pure Python addition — no changes to the Rust layer.

### Checklist before I open a PR

- [ ] Maintainer confirms this contribution is welcome
- [ ] I will follow the existing file structure in `tests/test_offline_descriptor.py`
- [ ] I will verify all expected values against the fixed mnemonic before submitting

---

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with tests/test_offline_descriptor.py and the existing test_descriptor_bip86 test to follow its fixed-mnemonic and full-descriptor-string pattern. Add coverage for the listed BIP44, BIP49, BIP84, BIP86, new_wsh_sortedmulti, and new_sh_sortedmulti constructors, then run the offline descriptor tests; done means all expected descriptor strings and checksums pass without network access.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.