miniscript: make typing rule match BIP-379 exactly #36028

sipa commented at 8:42 PM on August 19, 2026: member

The BIP-379 text specifies rule for reasoning about correctness and malleability through a number of type properties. These rules are implemented in Bitcoin Core's miniscript implementation.

There are multiple equivalent ways of writing these. For example, the BIP text says that an or_b fragment always has the "e" (uniquely dissatisfiable) property. The Bitcoin Core implementation however only assigns it the "e" property when both child expressions have "e" as well. This is an irrelevant distinction, because the "e" property is only used for reasoning about malleability, and an or_b whose children do not have "e" is always malleable. Thus, for all cases where it matters, "always e" and "e when children have e" are equivalent.

In the context of https://github.com/bitcoin/bips/pull/2240, which adds test vectors to BIP-379, it is helpful however to make the implementation match the specification exactly, even for these cases where it does not matter.

Draft until these is some agreement on this approach in https://github.com/bitcoin/bips/pull/2240.

miniscript: make typing rule match BIP-379 exactly 2631ece7b5

DrahtBot added the label Descriptors on Aug 19, 2026

DrahtBot commented at 8:42 PM on August 19, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36028.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	guggero

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

in src/script/miniscript.cpp:121 in 2631ece7b5

 117 | @@ -118,13 +118,11 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector<Ty
 118 |              "us"_mst; // u, s
 119 |          case Fragment::WRAP_D: return
 120 |              "B"_mst.If(x << "Vz"_mst) | // B=V_x*z_x
 121 | -            "o"_mst.If(x << "z"_mst) | // o=z_x

guggero commented at 1:18 PM on August 22, 2026:

These two deleted lines were no-ops in the first place, which may be worth stating for reviewers who want to convince themselves the change is behavior-preserving:

d: o=z_x -> o: d: requires its child to be Vz, so z_x always holds.
d: e=f_x -> e: every V expression has "f". By induction — v: and or_c are unconditionally "f"; and_v(X,Y) has f=s_X+f_Y with Y a V; andor(X,Y,Z) has f=f_Z(s_X+f_Y) with Y,Z both V; or_i(X,Z) has f=f_X f_Z with both V.

in src/script/miniscript.cpp:253 in 2631ece7b5

 249 | @@ -252,7 +250,7 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector<Ty
 250 |              return "Bdu"_mst |
 251 |                     "z"_mst.If(args == 0) | // z=all z
 252 |                     "o"_mst.If(args == 1) | // o=all z except one o
 253 | -                   "e"_mst.If(all_e && num_s == n_subs) | // e=all e and all s
 254 | +                   "e"_mst.If(num_s == n_subs) | // e=all s

guggero commented at 1:25 PM on August 22, 2026:

More context for other reviewers: This is the only hunk with an observable delta, and it cannot change what Core accepts: "m" still requires every child to be "e", and wherever an "m" rule consumes a child's "e" it also requires that child's "m", so no "m" value moves and descriptor acceptance is unchanged. Only the type of an already-malleable expression differs. The vector set does discriminate (40 of the 36,370 lines) which is how this surfaced.

One thing you may want to point at from here: after this PR, Core assigns "e" to expressions whose dissatisfaction is provably not unique, which contradicts the BIP's own description of the property ("this requires a unique unconditional dissatisfaction to exist"). or_i(pk(A),pk(B)) is a legal thresh child that 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, none of them needing a signature from A..D. The threshold is malleable either way, so nothing is wrong with the rule, but the description reads as a guarantee. bitcoin/bips#2267 adds a sentence saying the descriptions only bind for expressions that meet the malleability requirements, which would keep this PR from looking like it contradicts the spec it is matching.

guggero commented at 1:26 PM on August 22, 2026: contributor

Concept ACK 2631ece.

I agree on the approach and the Go implementation in btcsuite/btcd#2568 and the test vector BIP PR are now updated. Assuming the rust-miniscript PR is also accepted and merged, the three implementations would then be aligned.

guggero referenced this in commit a320fe8d7e on Aug 26, 2026

guggero commented at 7:22 AM on August 26, 2026: contributor

After the discussion in https://github.com/bitcoin/bips/pull/2267 and the new approach proposed in https://github.com/rust-bitcoin/rust-miniscript/pull/1037, I took the liberty of instructing an LLM to implement the same changes on top of this PR. Feel free to take whatever you find useful from this commit (assuming this is the direction that finds agreement): https://github.com/guggero/bitcoin/commit/576eb5f3252bd4159c302ae2bfe70b6ae71f6e33