Skip to content

Conversation

darosior
Copy link
Member

The type system was incorrectly relying on a standardness rule to be sound.

This bug was found and reported by Andrew Poelstra based on a question from Aman Kumar Kashyap.

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.
@DrahtBot DrahtBot added the Tests label Apr 18, 2022
@sipa
Copy link
Member

sipa commented Apr 18, 2022

ACK 7417594

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24860 (Miniscript integration follow-ups by darosior)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake
Copy link
Member

cc @sanket1729 @apoelstra

@apoelstra
Copy link
Contributor

utACK 7417594

@achow101
Copy link
Member

ACK 7417594

@hebasto hebasto merged commit 8103fff into bitcoin:master Apr 19, 2022
@sanket1729
Copy link
Contributor

ACK 7417594

@darosior darosior deleted the miniscript_fix branch April 19, 2022 18:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 19, 2022
7417594 miniscript: the 'd:' wrapper must not be 'u' (Antoine Poinsot)

Pull request description:

  The type system was incorrectly relying on a standardness rule to be sound.

  This bug was found and reported by Andrew Poelstra [based on a question from Aman Kumar Kashyap](rust-bitcoin/rust-miniscript#341).

ACKs for top commit:
  sipa:
    ACK 7417594
  apoelstra:
    utACK 7417594
  achow101:
    ACK 7417594

Tree-SHA512: af68c1df1c40e40dd105ef54544c226f560524dd8e35248fa0305dbef966e96ec1fa6ff2fe50fb8f2792ac310761a29c55ea81dd7b6d122a0de0a68b135e5aaa
@bitcoin bitcoin locked and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants