Properly forbid multipath keys in definite descriptor keys#830
Merged
Conversation
This uses `Self::XYZ` when matching on errors, rather than writing out the full name or using wildcard imports (which are dangerous because the compiler may misinterpret typos as being variable names) (although there will be a warning and CI should fail). This also collapses a bunch of fmt impls to only write the top-level error, in cases where we weren't adding any useful context. (We should revisit these and produce rich errors which have line number information and so forth; and I believe the correct way to describe the errors is the opposite of what I've done, where we are supposed to provide context but NOT the underlying error which can be accessed by the .source() method. But this simplifies the code without making things worse and we can do a proper fix later.)
When deriving keys we refuse to allow multipath keys. We should forbid them when directly constructing them at all.
apoelstra
added a commit
that referenced
this pull request
Jul 3, 2025
a3561f9 bump version to 12.3.3 (Andrew Poelstra) 08b446e descriptor: add unit tests for constructing multipath keys (Andrew Poelstra) 042cd08 DefiniteDescriptorKey: disallow multipath keys (Andrew Poelstra) Pull request description: Just the logic fix; the error refactoring doesn't apply. ACKs for top commit: sanket1729: utACK a3561f9 Tree-SHA512: e0ddaaa2313a791330d1bc1ca7a90604f223405b4219e66a8a840d9568a4b81789866ed80839c6e22d8453af6b5fb8cb808fc9ab2ed3a51d81df5ac14b5c626b
apoelstra
added a commit
that referenced
this pull request
Jul 4, 2025
cfa85be bump version to 11.2.2 (Andrew Poelstra) f652949 descriptor: add unit tests for constructing multipath keys (Andrew Poelstra) c07272a DefiniteDescriptorKey: disallow multipath keys (Andrew Poelstra) Pull request description: Just the logic fix; the error refactoring doesn't apply. ACKs for top commit: sanket1729: utACK cfa85be Tree-SHA512: 182c5b199c82460f80e8d9ae6bce9c0ea5905af36c372a92ad0ce189daff8b99fec7a7177f295e9101f78b5567cfcffb01bd4bfc921d8ed394878b22aa9309e2
apoelstra
added a commit
that referenced
this pull request
Jul 5, 2025
780d6bc bump version to 10.2.2 (Andrew Poelstra) 29808ae descriptor: add unit tests for constructing multipath keys (Andrew Poelstra) b41daad DefiniteDescriptorKey: disallow multipath keys (Andrew Poelstra) Pull request description: Just the logic fix; the error refactoring doesn't apply. ACKs for top commit: sanket1729: tACK 780d6bc. Tree-SHA512: 97ba6bd8c475650da03614e4f8477f568d984c71c09ba2cf4141e80a1facba70d8067f9a0a70fb614e66d0663c3653608f6b7a2df456bb5668adc14eebfb9e2c
heap-coder
added a commit
to heap-coder/rust-miniscript
that referenced
this pull request
Sep 27, 2025
…s in definite descriptor keys
b9ccd9d20c3e1bb54ce9f671180613d8626b1474 descriptor: add unit tests for constructing multipath keys (Andrew Poelstra)
dad35327c93f485f1fbb2313f92d3e980134e3c8 DefiniteDescriptorKey: disallow multipath keys (Andrew Poelstra)
258873f70646072f890180c06707ce080b5b193f key.rs: split "definite keys cannot have wildcards" into own error (Andrew Poelstra)
bd5db6a44cbc5545ce218c01c2449c7382c70a73 descriptor/key.rs: clean up error matching (Andrew Poelstra)
Pull request description:
This PR essentially just fixes a logic bug in `DefiniteDescriptorKey::new` but involves several commits that just rearrange error types.
I will backport the logic fix without the error refactoring, since for 12.x and earlier we didn't really have error types and were just returning `Error::Unexpected("arbitrary string")` style errors.
ACKs for top commit:
sanket1729:
ACK b9ccd9d20c3e1bb54ce9f671180613d8626b1474
Tree-SHA512: 44f5033732eb2b67792cf1f93d909bd255ff5be251c3937450d93265746aedb2944db57b256ee7dea9ae0140c921320cc570a5be54244c4c97970eb3fe09f1ee
This was referenced Mar 31, 2026
Closed
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
2 participants
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.
This PR essentially just fixes a logic bug in
DefiniteDescriptorKey::newbut involves several commits that just rearrange error types.I will backport the logic fix without the error refactoring, since for 12.x and earlier we didn't really have error types and were just returning
Error::Unexpected("arbitrary string")style errors.