descriptor: expose WalletPolicy template + key_info accessors#936
descriptor: expose WalletPolicy template + key_info accessors#936bg002h wants to merge 1 commit intorust-bitcoin:masterfrom
Conversation
…iscript#936 apoelstra's PR #2 closed in favor of rust-bitcoin/rust-miniscript#936 (true upstream). Same branch head, same content. Issue #934's cross-link to PR #2 was updated in the same maneuver to reference the new location, which removed the constraint that had previously held PR #2 in apoelstra's fork. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| /// `(AST position) -> (placeholder index)` mapping can iterate | ||
| /// `policy.template().iter_pk()` and read `ke.index.0` for each | ||
| /// yielded `KeyExpression`. For the resolved descriptor with | ||
| /// concrete keys, see [`Self::into_descriptor`] (consumes `self`). |
There was a problem hiding this comment.
Lol, why did the LLM feel the need to say this here?
| /// concrete keys, see [`Self::into_descriptor`] (consumes `self`). | |
| /// concrete keys, see [`Self::into_descriptor`]. |
There was a problem hiding this comment.
Lol, why did the LLM feel the need to say this here?
Maybe a prohibition against self consumption :)
|
I’m not sure if there is a use case outside of shared @n index, which 932 is addressing (https://github.com/rust-bitcoin/rust-miniscript/pull/932)… |
|
Can you dramatically reduce the amount of text in the commit description, PR description, and doccomments for the new methods? Concept ACK having these accessors. |
bg002h
force-pushed
the
feat/wallet-policy-template-accessor
branch
from
a95a61a to
83e8d36
Compare
|
Thanks. Trimmed:
— Brian (with Claude Opus 4.7) |
|
83e8d36 needs rebase |
Read accessors for the two private fields. The motivating use case is multipath-shared `@N` placeholders, e.g. `sh(multi(1,@0/**,@0/<2;3>/*))`, where `into_descriptor()` erases placeholder identity but the template's `KeyExpression::index` preserves it. Re-exports `KeyExpression` and `KeyIndex` so the accessor's return type is namable from outside the crate, and adds rustdoc on `KeyIndex` and `KeyExpression::is_disjoint` to satisfy `missing_docs`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bg002h
force-pushed
the
feat/wallet-policy-template-accessor
branch
from
83e8d36 to
489e799
Compare
|
Heh, thanks for the short and clear motivation. But as you say, #932 eliminates the proposed usecase. I think we should either
I'm happy to go in either direction. Though I guess we should lean toward closing, to avoid constraining our API. |
|
(Brian says): Theoretically these accessors could be useful for template inspection without specified keys, but my use case doesn't actually require that — #932 closed the concrete gap I was worried about. Perhaps it's wise to not expose these methods until a genuine use case arises from a real programmer, as their needs may hint at a better form or way of accomplishing the same goal than what I've sketched. Confirm closing as the wisest move at this stage of the game? |
|
+1 for closing if no immediate use-case |
3 participants
Read accessors for
WalletPolicy's privatetemplateandkey_infofields. Motivating case: multipath-shared@Nplaceholders (e.g.sh(multi(1,@0/**,@0/<2;3>/*))) whereinto_descriptor()erases placeholder identity but the template'sKeyExpression::indexpreserves it. Re-exportsKeyExpressionandKeyIndexso the return type is namable.Found while building md-codec, a BIP 388 wallet-policy encoder. Cross-link: #934 (about
set_key_infoAST-order correspondence; not a blocker for this PR).Authored by Claude Opus 4.7 under my direction.