Follow BIP379 for the thresh "e" rule

[guggero]

Prompted by bitcoin/bips#2240 (comment).
Related: bitcoin/bitcoin#36028 and bitcoin/bips#2267.

Description

BIP379 computes the "e" (expressive) property of a threshold as e=all are s.
This crate additionally requires every child to be "e". Bitcoin Core did the
same until bitcoin/bitcoin#36028, which aligns it with the BIP; this does the
same for rust-miniscript.

Why it doesn't change anything a caller can act on

The extra condition is already part of non_malleable, and non-malleability is
conjunctive, so a threshold whose children are all "s" but not all "e" is
malleable either way — as is every expression containing one. Wherever an m
rule consumes a child's e it also requires that child's m, so no m value
changes anywhere, and Descriptor::from_str accepts and rejects exactly what it
did before. Only the type of an expression that was already malleable changes.

Why it matters anyway

BIP379's test vectors are being contributed from the corpora in ms_tests.rs
(bitcoin/bips#2240), and exact type vectors mean the specification and the
implementations have to agree on the type of a malleable expression too.

To check that this is the only rule where they disagree, I implemented the
BIP379 correctness and malleability tables as an independent reference and ran
it over the five corpora with recorded types — 36,370 expressions, including the
malleable and timelock-conflicting ones:

Reference thresh e rule	Mismatches vs ms_tests.rs
all are s (BIP text)	40, every one an added "e" on a thresh
all are e and all are s (this crate, before)	0

So this crate matched the BIP in every other correctness and malleability rule.
(Core additionally differed in or_b's "e" and in two vacuous d: rules, which
is the rest of #36028.)

A note on the property description

BIP379 describes "e" as requiring "a unique unconditional dissatisfaction to
exist", and that does not hold under its own rule: or_i(pk(A),pk(B)) is a legal
thresh child which is "s" but not "e", and spending
or_d(thresh(2,or_i(pk(A),pk(B)),a:or_i(pk(C),pk(D))),pk(E)) through its second
branch has four consensus-valid witnesses that differ only in the two
branch-selector bytes. The BIP is being amended to note that the property
descriptions only bind for expressions meeting the malleability requirements.

[apoelstra]

Interesting. Okay, I think this is reasonable. The claim is that, assuming that all children are s,

• either a thresh's children are all e, in which case the thresh itself is e
• or the thresh is malleable, and e's promises are moot

Therefore we can just always set e for the thresh and not bother looking at the children's es.

The alternative (fix the BIP to match the implementation) might work, but it'd be upholding a stronger promise than we intended to make and would require far more extensive analysis for little benefit. See bitcoin/bips#2267 (comment)

[apoelstra]

Now I'm kinda musing about changing the types::Malleability type to just erase the f/e/s properties if something is malleable. So instead of

pub struct Malleability {
    pub dissat: Dissat,
    pub signed: bool,
    pub non_malleable: bool,
}

we'd have

pub enum Malleability {
    Malleable,
    NonMalleable {
        dissat: Dissat,
        signed: bool,
    },
}

But I guess doing this would break our test vectors for little benefit. If we ever provide a "user-facing API" for the type system maybe we can do something like this.

[apoelstra]

In 439be85:

lmao "cannot mislead a caller"

Let's tighten up this comment to say "Intuitively, Dissat::Unique should require all children to be Dissat::Unique. But this does not match BIP 379. Notice that if any child is not Dissat::Unique, the entire thresh will be malleable so the dissat property has no meaning anyway. See PR 1032 and BIPs PR 2267 for more discussion."

[guggero]

Closing in favor of #1037, which seems to be the preferred option.