bitcoindevkit / bitcoindevkit/bdk_wallet
[dsl] Handle `ThresholdError` in `fragment!` macro
- Dominant language
- Rust
- Stars
- 59
- Forks
- 105
- Avg merge
- 10d 9h
- Merged PRs (30d)
- 1
Description
**Describe the bug**
The thresh_vec, multi_vec, and multi_a_vec fragments of the `descriptor!`/`fragment!` macros accept `$thresh:expr` and `$keys:expr` runtime values, with thresh_vec explicitly documented for building "dynamic descriptors" from a runtime Vec. All three call `Threshold::new` and `.expect("valid threshold and pks collection")` on the result. `Threshold::new` returns Err for k == 0, k > n, or n > MAX, so invalid runtime parameters crash the consumer process even though the macro's normal error channel is `Result<_, DescriptorError>`.
**To Reproduce**
```rust
// `k` (3) is greater than the number of items (2), which is an invalid threshold.
let result = descriptor!(wsh(thresh(3, pk(key1), pk(key2))));
// Same invalid-threshold scenario, but going through the `multi()` fragment
// which builds a `Terminal::Multi` threshold via `make_multi`.
let result = descriptor!(wsh(multi(3, key1, key2)));
```
**Build environment**
- BDK tag/commit: 49f7955c34d1f558ca3566656faa8b17acffba1c
- OS+version:
- Rust/Cargo version:
- Rust/Cargo target:
**Is this blocking production use?**
- [ ] Yes
- [x] No
**Additional context**
Suggested fix
Propagate the `ThresholdError` as a `DescriptorError` variant instead of `.expect()` in all three arms, and add a test asserting Err (not panic) for k > n via multi_vec.
Contributor guide
Research direction
Locate the descriptor!/fragment! macro arms for thresh_vec, multi_vec, and multi_a_vec, then trace Threshold::new and the multi() path through make_multi. Replace the panic path with the normal DescriptorError result and add a test showing that multi_vec with k greater than the item count returns Err rather than panicking.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 74/100