bitcoindevkit / bitcoindevkit/bdk_wallet
Wallet signing may finalize invalid pre-seeded signatures and return Ok(true)
- Dominant language
- Rust
- Stars
- 59
- Forks
- 105
- Avg merge
- 10d 9h
- Merged PRs (30d)
- 1
Description
**Describe the bug**
`Wallet::sign_with_signers` skips signing when the PSBT already has a signature in the wallet key's slot, without verifying it. With default `SignOptions` (`try_finalize: true`), `finalize_psbt` consumes that signature, builds the final witness, and returns `Ok(true)`, although the transaction fails script validation.
Skip sites in `src/wallet/signer.rs`:
- ECDSA: `partial_sigs.contains_key(&pubkey)`
- Taproot key path: `tap_key_sig.is_none()`
- Taproot script path: `tap_script_sigs.contains_key(...)`
**To Reproduce**
1. Create a single-input `wpkh` PSBT.
2. Insert an invalid parseable signature for the wallet pubkey into `partial_sigs`.
3. `wallet.sign_with_signers(&mut psbt, &signers, SignOptions::default())` → `Ok(true)`, invalid sig lands in `final_script_witness`.
Same for Taproot: pre-seed `tap_key_sig` or `tap_script_sigs`.
**Expected behavior**
Overwrite wallet-controlled slots (or verify and overwrite invalid ones) on all three paths. Never `Ok(true)` with an invalid final witness.
**Build environment**
- BDK tag/commit: `6fc68462` (master)
- Rust/Cargo version: 1.96.0
**Which backend(s) are relevant (if any)?**
- [ ] Electrum
- [ ] Esplora
- [ ] Bitcoin Core RPC
- [x] None / not backend-related (e.g. `bdk_chain`, `bdk_core`)
- [ ] Other (please specify): `____`
**Is this blocking production use?**
- [ ] Yes
- [x] No
**Additional context**
`finalize_psbt` checks signature availability, not validity. The `allow_all_sighashes` guard is also bypassed: it checks the PSBT-level `sighash_type` field, not the sighash byte in an existing `partial_sigs` entry.
Contributor guide
Research direction
Start in src/wallet/signer.rs and inspect the ECDSA partial_sigs, Taproot key-path tap_key_sig, and Taproot script-path tap_script_sigs skip sites. Reproduce the invalid pre-seeded-signature cases for wpkh and Taproot, then verify that wallet-controlled invalid signatures are replaced and sign_with_signers does not return Ok(true) with an invalid final witness.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- cryptography
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100