Repository navigation
Conversation
DefiniteDescriptorKey::new checks for wildcards, hardened steps and multipath derivation, but not for BIP32's maximum depth of 255. A key that passes these checks but derives past that depth makes rust-bitcoin return MaximumDepthExceeded, and derive_public_key turns this error into a panic labeled "cryptographically unreachable". A descriptor string can trigger it: an xpub with a long enough path parses, becomes definite, and panics when its script pubkey is computed. Return the new NonDefiniteKeyError::MaximumDepthExceeded from DefiniteDescriptorKey::new instead. Derivation at an index and into_definite go through it, so they return the error too. Only the steps after the extended key count, since its depth already includes its origin steps. Assisted-by: Claude Opus 5.5
A key expression that derives past BIP32's maximum depth of 255 can never produce a key. The previous commit rejects such a key when it becomes definite. Reject it when parsing too, so that the error points at the string that caused it. Count the steps after the extended key, plus one for a wildcard, for xpubs and xprvs, including multipath keys. Origin steps don't count. Assisted-by: Claude Opus 5.5
This was referenced Oct 12, 2026
Member
|
Can you backport to the |
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.
DefiniteDescriptorKey::newchecks for wildcards, hardened steps and multipath derivation, but not for BIP32's maximum depth of 255. A key that passes these checks but derives past that depth makes rust-bitcoin returnMaximumDepthExceeded, andderive_public_keyturns that error into a panic labeled "cryptographically unreachable". A descriptor string is enough to trigger it:tr(xpub/0/0/.../0)with 256 steps below a master xpub parses, passesinto_definite(), and panics inscript_pubkey().This is missing validation in miniscript. Deriving that deep used to overflow rust-bitcoin's depth counter (rust-bitcoin/rust-bitcoin#4308).
rust-bitcoin 0.32.6made it a recoverable error (rust-bitcoin/rust-bitcoin#4434), which miniscript doesn't handle.The first commit makes
DefiniteDescriptorKey::newreturn the newNonDefiniteKeyError::MaximumDepthExceeded.into_definite()and derivation at an index go through it, so they report the error too. Only the steps after the extended key count, since its depth already includes the steps of its origin.The second commit rejects such key expressions when parsing, with the new
MalformedKeyDataKind::MaximumDepthExceeded. It counts a wildcard as one more step and covers xprvs and multipath keys. It's a separate commit because some strings that parse today will stop parsing, even though no key can be derived from them. It can be dropped if parsing should stay unchanged.Together, they remove the deterministic depth panic. Derivation can still fail in BIP32's invalid-child case, which has a probability below 2^-127 and stays
unreachable!.Tests cover depths 255 and 256, a nonzero starting depth, wildcard derivation, multipath keys, xprvs and origin paths.
This pre-existing bug was found while working on #954.
🤖 AI disclosure: The patch, tests and this description were generated with Claude Opus 5.5. Codex reviewed the approach.