Backport wallet policy fixes to 13.x - #1042
Open
trevarj wants to merge 18 commits into
Open
Conversation
BIP-388 key information items are bare KEY expressions and the derivation comes from the template's key placeholders, but the translator returned the key information item verbatim, so wpkh(@0/**) materialized as wpkh(xpub...) instead of wpkh(xpub.../<0;1>/*). - take the derivation path and wildcard from the placeholder when materializing a descriptor key - reject a key information item that is not an extended key, since there is nothing to append a derivation to Fixes rust-bitcoin#1014
BIP-388 says a placeholder @i must never appear for the first time before an occurrence of @j for some j < i, and that reuses of one placeholder must have pairwise disjoint paths. The check compared only adjacent placeholders, so it rejected valid templates: wsh(multi(2,@0/**,@1/**,@1/<2;3>/*)) failed because @1 followed @1, and the @0 reuse variant failed because @0 followed @1.
key_info was a Vec<DescriptorPublicKey>, which can hold a plain public key or a key carrying its own derivation, neither of which BIP-388 allows. It also held one entry per key occurrence, so a descriptor reusing one key with disjoint paths, such as sh(multi(1,K/<0;1>/*,K/<2;3>/*)), produced the template sh(multi(1,@0/**,@1/<2;3>/*)) and a key vector holding the same xpub twice, rather than sh(multi(1,@0/**,@0/<2;3>/*)) with one entry. - add `KeyInfo`, an extended public key plus optional origin. - take `Vec<KeyInfo>` in `set_key_info`, matching how the crate takes owned collections elsewhere, and add a `key_info` getter, now that the items cannot be malformed.
…istinct BIP-388 requires the deserialized keys of the key information vector to be pairwise distinct. This was not checked, so one key could fill two placeholders, turning wsh(multi(2,K,K)) into a 2-of-2 that a single keyholder satisfies alone.
The template parser required a key placeholder to be followed by "/**" or "/<NUM;NUM>/*", but from_descriptor built key expressions straight from descriptor keys and never checked their shape, so the checked constructor emitted templates that WalletPolicy::from_str rejects: wpkh(xpub.../0/*) became wpkh(@0/0/*) and a bare wpkh(xpub...) became wpkh(@0).
… grammar - parse the placeholder grammar directly: "@N/**" or "@N/<NUM;NUM>/*" with two distinct canonical NUMs, sharing the digits-only, no leading zeros rule between the key index and the NUMs via parse_canonical_num - collapse only a whole "/<0;1>/*" derivation to "/**" on display, instead of a global replace
- keep release-13.x wallet policy tests aligned with available descriptor grammar Assisted-by: OpenAI Codex GPT-5.6 Sol
…criptor - drop from_descriptor_unchecked and validate in from_descriptor - add n_keys(), which is the number of key placeholders in the template
BIP-388 requires at least one key placeholder and the corresponding key. - reject a template with no key placeholder - reject Descriptor::Bare in validate
- use the concrete bitcoin hash types, exactly as DescriptorPublicKey does, so garbage hex fails inside the miniscript parser
`Miniscript::translate_pk` and `Descriptor::translate_pk`, along with the concrete and semantic policy equivalents, drove their reconstruction from a right-to-left post-order iterator. That existed only so a node could be rebuilt as `Terminal::AndB(stack.pop().unwrap(), stack.pop().unwrap())`. - iterate with `post_order_iter` everywhere `rtl_post_order_iter` was used - add a crate-private `StackExt` trait on `Vec<T>`; `pop2`/`pop3` pop the top two or three elements and pass them, left to right, to a constructor or closure - `pop_n` drains the top `n` elements in order, for the `Vec`-shaped policy nodes, and `pop_thresh` rebuilds a threshold with the same `k` Fixes rust-bitcoin#1022
- add an infallible constructor for the single-path case
- the fields and validate() are now private to the submodule, so every construction path validates - the TryFrom<&str> template branch goes through a private from_template instead of a struct literal
With `translate_pk` now walking the descriptor in left-to-right order, `from_descriptor` no longer needs a separate `iter_pk` pass to number the keys in textual order. - drop the `iter_pk` pre-pass in `from_descriptor` - number each distinct key at its first occurrence in the translator
Every construction and mutation of a `WalletPolicy` now runs through the same checker. - replace `validate` with a `check_policy` fn that the constructors call before building the struct - `set_key_info` drops its own count check, rejects an empty argument and revalidates through `check_policy` before assigning
- keep validated state construction and mutation in the private policy module - move translation and public API implementations to the parent module
- preserve textual key order for stateful translators - cover wallet policies with distinct internal and script keys Assisted-by: OpenAI Codex GPT-5.6 Sol
trevarj
marked this pull request as ready for review
September 4, 2026 14:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports BIP-388 wallet policy fixes and required translation-order fixes to release-13.x.