This repository was archived by the owner on Oct 22, 2026. It is now read-only.
Repository navigation
Conversation
Miniscript::from_tree checks the global rules of the script context on every node. Move that loop into check_global_validity_recursive, so that the descriptor constructors can run the same checks. No behavior change. Assisted-by: Claude Opus 5.5
Parsing a descriptor checks the rules of the script context on every node of its Miniscript: which keys the context allows and its script size limits. Wsh::new, Sh::new and Bare::new run only the top-level checks, Tr::new checks only the internal key, and new_sortedmulti runs no checks at all since SortedMultiVec was removed in rust-bitcoin#915. Constructors like Miniscript::multi and Miniscript::pk don't check the context either, and Miniscript::from_ast checks only the node it builds. So the descriptor constructors accept Miniscript that parsing rejects. For example, Descriptor::new_wsh_sortedmulti accepts an x-only key. The result doesn't parse back from its Display, and into_definite panics when the translation checks the context again. Descriptor::new_sh_sortedmulti accepts 16 keys, which make a 547-byte redeem script, and address() then panics on its assertion that the size was checked. Descriptor::new_tr accepts a leaf with an uncompressed key, and into_definite panics on it too. Call check_global_validity_recursive in Wsh::new, Sh::new and Bare::new, and on every leaf in Tr::new. Build both new_sortedmulti constructors through Wsh::new and Sh::new. SortedMultiVec also ran the local checks of the context, but a multisig that passes the global checks passes the local ones too. Assisted-by: Claude Opus 5.5
Check that the descriptor constructors reject the keys and the script size that parsing rejects, also for a key below the root and in a Taproot leaf, and that the sortedmulti constructors build what parsing builds, with the keys sorted in the script. Assisted-by: Claude Opus 5.5
This was referenced Oct 14, 2026
|
Tested ACK eac11e6. The description's cases are accepted on master and rejected on this head, as Bitcoin Core 31.1 rejects them, and the new test fails with the fix reverted; details on request. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Before: the descriptor constructors skipped checks that parsing runs.
Wsh::new,Sh::newandBare::newchecked only the top level,Tr::newchecked only the internal key, andnew_sortedmultichecked nothing since #915. Sonew_wsh_sortedmultiaccepted an x-only key, andnew_sh_sortedmultiaccepted 16 keys, a 547-byte redeem script that madeaddress()panic.After: these constructors run the per-node checks that parsing runs.
Split out of #954. #991 doesn't cover
new_sortedmultior theTr::newleaves yet.🤖 AI disclosure: the patch was generated with Claude Opus 5.5 and reviewed with Codex.