bitcoindevkit / bitcoindevkit/bdk-python
test: expand offline descriptor test coverage to include all BIP descriptor constructors
- 主要言語
- Python
- スター
- 11
- フォーク
- 12
- 平均マージ
- 2日 13時間
- マージ済み PR(30日)
- 2
説明
### 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
---
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
調査の方向性
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.
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- python
- 領域
- testing
- issue の種類
- 機能追加
- 難易度
- 3/5
- 見積もり時間
- 1〜2日
- 活発さ
- 活発
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 84/100