utACK 76fb300

Maybe reject 0x50 too as that's reserved for the annex.

Maybe reject 0x50 too as that's reserved for the annex.

I don't think the annex byte has any effect on the leaf version. AFAICT, a future leaf version could b 0x50 too and it wouldn't conflict with the annex.

From BIP341:

What constraints are there on the leaf version? First, the leaf version cannot be odd as c[0] & 0xfe will always be even, and cannot be 0x50 as that would result in ambiguity with the annex. In addition, in order to support some forms of static analysis that rely on being able to identify script spends without access to the output being spent, it is recommended to avoid using any leaf versions that would conflict with a valid first byte of either a valid P2WPKH pubkey or a valid P2WSH script (that is, both v and v | 1 should be an undefined, invalid or disabled opcode or an opcode that is not valid as the first opcode). The values that comply to this rule are the 32 even values between 0xc0 and 0xfe and also 0x66, 0x7e, 0x80, 0x84, 0x96, 0x98, 0xba, 0xbc, 0xbe. Note also that this constraint implies that leaf versions should be shared amongst different witness versions, as knowing the witness version requires access to the output being spent.

I guess it's in theory possible to use 0x50 as leaf version, but it would be only be usable in spends that also have an annex.

If we want to add direct PSBT support for annexes later, presumably in the form of additional keytype's, it may save us some headache if we disallow 0x50 as a leaf version here. The downside is that if in some far future we end up having to introduce leaf version 0x50, we (or the AI programmer) have to remember to remove the throw.

https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki

I don't see how annexes are at all related to a field which only contains leaf scripts. We wouldn't be adding annexes to this field - they would be in a separate field.

@achow101 The leaf version is the first byte of the last witness stack element in a P2TR script path spend, after skipping over the annex field if present, which would follow the control block.

So while there is no actual consensus rule against leaf version 0x50, constructing a spend for it would imply the spend must have an annex as well, because otherwise the control block would be interpreted as an annex.

For this reason, the BIP does state that leaf version cannot be 0x50.

I get that, but I don't see why we should be checking that in a leaf script deserializer. Sure, it may not be a good idea and we shouldn't consider signing scripts with such a leaf version unless there is an annex, but that doesn't mean that the deserializer should be enforcing it. Frankly, I don't think we should even be adding these depth and leaf version checks in the first place, and the only reason I'm adding them is to avoid hitting asserts that exist elsewhere. If the user wants to do something stupid like using bad leaf versions or having things be too deep, then the signer should be dealing with it, not the deserializer. And even if they do, if those paths are never revealed or used, then it doesn't matter.

@achow101 Fair point, I see where you're coming from. There is perhaps another reason why depth checking makes sense too, as a DoS prevention mechanism (e.g. put script paths with 1000000 levels in a PSBT, which would hurt the users passing around such PSBTs, even if those script paths are unused).

utACK 76fb300