Fix e/o bug in miniscript threshold correctness rules#349
Merged
Conversation
This will not affect existing users of rust-miniscript as we were previously more strict in the miniscript that we accepted. This allows more more scripts to parsed as miniscripts. This allows correctly tagging thresh z, o values as per the spec. For example, miniscript with thresh(2,older(9),older(10)) to be tagged as `z` and thresh(2,pk(),older()) as `o` Found while doing a line to line comparison with c++ codebase and spec
We were marking extra miniscripts as non-malleable. Note that this change is previously marked non-malleable miniscripts as malleable. In particular, this affects miniscripts that are have a thresh child that does not have the `s` property. (Meaning that the thresh can be satisfied without signatures
Parallel to sipa/miniscript#117 This is a severe bug reporting that needs backporting as it affects the correctness properties of miniscript
sanket1729
force-pushed
the
type_system_bugs
branch
from
703842d to
c74934b
Compare
apoelstra
approved these changes
Apr 19, 2022
heap-coder
added a commit
to heap-coder/rust-miniscript
that referenced
this pull request
Sep 27, 2025
…eshold correctness rules c74934bf0864a463c42c5a44502f803bf61eb57c Fix compiler test cases for new typing rules (sanket1729) 9766e30c94f85562e3687f00b84a281ac76d2eff Fix bug in exec stack elements calculation (sanket1729) db97c39afa4053c2c3917f04392f6e24964b3972 Remove `u` property from `d` (sanket1729) 77d7d796aab10373a3dd5e49a232a920f59c05b4 Fix malleability rules according to website (sanket1729) 6a1ceac81dfd275aecc7ee6594d2f2e88de37473 Fix e/o bug in miniscript threshold correctness rules (sanket1729) Pull request description: Multiple type system bugs: The first two bugs are not severe: They relax rules so existing systems should not be affected. However the third commit is a fix that will be backported. Pasting description #348 > The value it leaves on the stack depends on the last element on the stack. However, we can't make sure this element is OP_1 (which would give us the 'u' property) without the MINIMALIF rule. MINIMALIF is only policy for P2WSH, therefore giving 'd:' the 'u' property breaks consensus soundness: it makes it possible (by consensus but not policy) for instance to satisfy a thresh() without satisfying at least k of its subs. This bug was found and reported by Andrew Poelstra rust-bitcoin/rust-miniscript#341. ACKs for top commit: apoelstra: ACK c74934bf0864a463c42c5a44502f803bf61eb57c Tree-SHA512: 274a3c2f93eb56b8cda3bf8f9befd9c93494f398d1564b90716330e1c73fbb503e7c1dcc1ffd232bcbae8f1c4e316bfbc705b3d5fc02b9491de1fcdb8c3dbe79
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.
Multiple type system bugs:
The first two bugs are not severe: They relax rules so existing systems should not be affected. However the third commit is a fix that will be backported.
Pasting description #348
This bug was found and reported by Andrew Poelstra #341.