BIP346: reject truncated long-form TxFieldSelectors #2327

Description

The BIP-346 reference implementation in bip-0346/ref-impl accepts truncated long-form TxFieldSelectors.

calculate_txhash parsed the global byte and the input/output-fields byte, then treated the input and output selection bytes as optional: when absent it defaulted to an empty selection (NONE). As a result:

• a 2-byte selector [global, fields] was silently treated as input=NONE, output=NONE
• a 3-byte selector [global, fields, input-selector] was silently treated as output=NONE

This invents NONE semantics the spec does not define. bip-0346.md states ("For both inputs and then outputs, expect an additional byte") that both selection bytes belong to the long form, and the OP_TXHASH precondition requires the TxFieldSelector to be valid, failing otherwise.

Change

Make the input and output selection bytes mandatory in calculate_txhash, returning an error when they are missing. The generated test vectors (txhash_vectors.json) and the vector generator were updated to use explicit 4-byte long-form selectors; 0 of the 150 txhash values change because TXFS_CONTROL commits the padded selector bytes; the other 90 are unchanged.

Test plan

• Added reject_truncated_long_form_txfs regression test (2-byte and 3-byte selectors are rejected; an explicit 4-byte NONE/NONE selector is accepted).
• cargo test passes; cargo run regenerates the committed txhash_vectors.json byte for byte.

The description says all 150 txhash values are unchanged, but 60 of them change. They are the vectors whose selector gets padded (8000 becomes 80000000) and has TXFS_CONTROL (0x80) set, since that bit commits the selector bytes to the hash. The code is consistent: cargo run regenerates this PR's file byte for byte.

The PR also rejects 2- and 3-byte selectors, which were accepted until now. That changes which selectors are valid, so I think the BIP authors should confirm it.

The description says all 150 txhash values are unchanged, but 60 of them change. They are the vectors whose selector gets padded (8000 becomes 80000000) and has TXFS_CONTROL (0x80) set, since that bit commits the selector bytes to the hash. The code is consistent: cargo run regenerates this PR's file byte for byte.

The PR also rejects 2- and 3-byte selectors, which were accepted until now. That changes which selectors are valid, so I think the BIP authors should confirm it.

Thanks for catching this. I compared the vector files and confirmed that 60 of the 150 txhash values change; the other 90 do not. The changed hashes are from padded selectors with TXFS_CONTROL set, which commits the selector bytes. My PR description was incorrect, and I’ll update both the hash count and the cargo run wording. I also agree that rejecting 2- and 3-byte long-form selectors changes the accepted inputs of the reference implementation. I read “For both inputs and then outputs, expect an additional byte” as requiring both bytes, but this interpretation should be confirmed by the BIP authors before we treat the change as settled. Could the authors confirm the intended validity rule?

@stevenroose Could you confirm the intended validity rule here?

The long-form encoding says, “For both inputs and then outputs, expect an additional byte.” I interpreted that as requiring both selector bytes, which would make the currently accepted 2- and 3-byte forms invalid.

This PR therefore rejects those truncated forms and updates the generated vectors to explicit 4-byte selectors. As @fametrano pointed out, this is a semantic tightening of the reference implementation, so I'd prefer confirmation from the BIP authors before proceeding.

Also, I corrected the PR description: padding the selectors changes 60 TXFS_CONTROL hashes; the remaining 90 txhash values are unchanged.