Properly forbid multipath keys in definite descriptor keys #830
Conversation
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
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.