Bip352: Add consensus-validity requirement to Scanning requirements #2339

pull Eunovo wants to merge 2 commits into bitcoin:master from Eunovo:bip352-invalid-control-block-vector changing 2 files +8 −0

Eunovo commented at 5:10 PM on October 25, 2026: contributor

Add a consensus-validity requirement to Scanning requirements, and update the reference implementation to assert valid control block sizes; see BIP341 Script Validation Rules.
Eunovo renamed this:
~~Bip352 invalid control block vector~~
Bip352: add invalid control block vector
on Oct 25, 2026
fametrano referenced this in commit 59b6589294 on Oct 25, 2026
fametrano referenced this in commit 0e75a328fc on Oct 25, 2026
fametrano referenced this in commit 7515be6781 on Oct 25, 2026
fametrano referenced this in commit c907ca57b6 on Oct 25, 2026
theStack commented at 10:01 AM on October 26, 2026: contributor

IIUC, this would be the first test for a consensus-invalid transaction, and it's not clear to me whether we should have test vectors for those in the first place. An alternative could be to state more clearly in the BIP that scanning assumes consensus-valid transactions and add the control block size validations as asserts in the reference implementation?
jonatack added the label Tests on Oct 26, 2026
Eunovo commented at 1:13 PM on October 26, 2026: contributor

IIUC, this would be the first test for a consensus-invalid transaction, and it's not clear to me whether we should have test vectors for those in the first place. An alternative could be to state more clearly in the BIP that scanning assumes consensus-valid transactions and add the control block size validations as asserts in the reference implementation?

After some deliberation, I agree that we should limit the scope of the test vectors to consensus-valid transactions. I'll update the BIP to state that scanning assumes consensus-valid transactions.
BIP352: validate control block size in reference impl f7b78df024

BIP352: add consensus validity scanning requirements

Silent Payments transaction scanning assumes that the transaction is consensus-valid. The test vectors do not include consensus-invalid transactions.

86b6ef4452

Eunovo force-pushed on Oct 26, 2026
Eunovo renamed this:
~~Bip352: add invalid control block vector~~
Bip352: Add consensus-validity requirement to Scanning requirements
on Oct 26, 2026
in bip-0352/reference.py:88 in f7b78df024
```
  82 | @@ -79,6 +83,9 @@ def get_pubkey_from_input(vin: VinInfo) -> GE:
  83 |                  # Script-path spend
  84 |                  control_block = witnessStack[-1]
  85 |                  #  control block is <control byte> <32 byte internal key> and 0 or more <32 byte hash>
  86 | +                assert len(control_block) >= TAPROOT_CONTROL_BASE_SIZE
  87 | +                assert len(control_block) <= TAPROOT_CONTROL_MAX_SIZE
  88 | +                assert (len(control_block) - TAPROOT_CONTROL_BASE_SIZE) % TAPROOT_CONTROL_NODE_SIZE == 0
```
theStack commented at 10:48 AM on October 30, 2026:

it doesn't hurt to have those detailed checks, but for the sake of the pubkey extraction only the first one (>= TAPROOT_CONTROL_BASE_SIZE) is really relevant to avoid a out-of-bound access

Eunovo commented at 10:13 AM on November 2, 2026:

From https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules:

The last stack element is called the control block c, and must have length 33 + 32m, for a value of m that is an integer between 0 and 128[6], inclusive. Fail if it does not have such a length.

I think the reference implementation should demonstrate proper validation following the rules.
theStack commented at 10:49 AM on October 30, 2026: contributor

ACK modulo diff-could-be-smaller nit
fametrano commented at 6:22 PM on November 1, 2026: none
An input in bip-0352/send_and_receive_test_vectors.json is not consensus-valid, the scope this thread settled on for the vectors: in vector 19 ("taproot input with NUMS point"), input 2 has control byte c1, but its output key has even y, so BIP341 requires c0.

I suggest fixing it here, although the file is outside this diff: on lines 2390 and 2453, f1f85dac21c150929b becomes f1f85dac21c050929b. Expected values stay the same, as reference.py never reads this byte. From the repository root, this prints True True 1 (x matches, y even, parity bit odd):
```
PYTHONPATH=bip-0352/secp256k1lab/src python3 -c '
import json
from secp256k1lab.secp256k1 import G,GE
from secp256k1lab.util import tagged_hash as h
v=json.load(open("bip-0352/send_and_receive_test_vectors.json"))[19]["sending"][0]["given"]["vin"][2]
w=bytes.fromhex(v["txinwitness"]);c=w[-33:]
Q=GE.from_bytes_xonly(c[1:])+int.from_bytes(h("TapTweak",c[1:]+h("TapLeaf",b"\xc0\x22"+w[-68:-34])),"big")*G
print(Q.to_bytes_xonly().hex()==v["prevout"]["scriptPubKey"]["hex"][4:],Q.has_even_y(),c[0]&1)'
```
This byte is not the only gap, and I am not asking to change the others here: 20 of the file's 34 distinct inputs have ECDSA signatures with no sighash byte, so they fail BIP66, and I removed from my #2330 the inputs that cannot be consensus-valid.

Contributors

Labels

Linked (view graph)

#2330 BIP-352: add test vectors for skipped inputs and label edge cases