Skip to content

BIP-352: add test vectors for malleated p2pkh pubkey extraction - #2323

Open
fametrano wants to merge 2 commits into
bitcoin:masterfrom
fametrano:bip352-malleated-p2pkh-vector
Open

fametrano wants to merge 2 commits into
bitcoin:masterfrom
fametrano:bip352-malleated-p2pkh-vector

Conversation

@fametrano

@fametrano fametrano commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

This adds two test vectors, each with a p2pkh input whose scriptSig also pushes another valid pubkey.

The first uses the scriptSig from the test in bitcoin/bitcoin#36338 by @theStack: <sig> <pubkey> <bogus_sig> OP_OVER OP_CHECKSIG OP_IF <other_pubkey> OP_ENDIF. Evaluating this scriptSig with a checker that accepts any signature leaves <other_pubkey> on top of the stack. The existing malleated p2pkh vector does not cover this.

The second is the <dummy> OP_DROP <sig> <pubkey> case @jonatack suggested in that PR, with a valid pubkey as the dummy. It catches an extractor that takes the first 33-byte push without checking its hash.

The expected values come from reference.py. btclib-wallet's vector tests also pass on the new file.

I ran Bitcoin Core's bip352_tests with the new file in place of its vendored copy:

  • at the head of #36338 (cdfaa534), all 30 vectors pass;
  • on master's code, the first new vector fails;
  • on #36338 without its Hash160 check, the second new vector fails.

Core's test does not compare input_pub_keys, so each failure shows on the receiving side: the wrong key gives a wrong tweak, and no output is found.

Made with my usual tools: a computer, the Internet and an LLM. The mistakes, as usual, are all mine.

Add two p2pkh inputs whose scriptSig also pushes another valid pubkey.

In `<sig> <pubkey> <bogus_sig> OP_OVER OP_CHECKSIG OP_IF <other_pubkey>
OP_ENDIF`, evaluating the scriptSig with a checker that accepts any
signature leaves <other_pubkey> on top of the stack. This is the
scriptSig of the test added in bitcoin/bitcoin#36338.

In `<other_pubkey> OP_DROP <sig> <pubkey>`, taking the first 33-byte
push without checking its hash finds <other_pubkey>.
@fametrano

Copy link
Copy Markdown
Contributor Author

With bitcoin/bitcoin#36338 merged, Core's master (db0bde16) passes all 30 vectors in this file, so Core and reference.py now agree on them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant