Skip to content

BIP352: fix loop range issue in get_pubkey_from_input function #1806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

BIP352: fix loop range issue in get_pubkey_from_input function #1806

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions bip-0352/reference.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,54 +32,51 @@ def get_pubkey_from_input(vin: VinInfo) -> ECPubKey:
# skip the first 3 op_codes and grab the 20 byte hash
# from the scriptPubKey
spk_hash = vin.prevout[3:3 + 20]
for i in range(len(vin.scriptSig), 0, -1):
if i - 33 >= 0:
for i in range(len(vin.scriptSig) - 33, 0, -1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure what issue is being fixed here. Before this loop, we have already determined that we are looking at a P2PKH input. The P2PKH input script consists of a signature and the pubkey. The pubkey bytes are the last 33 bytes of the input script. However, P2PKH is malleable and could have additional junk bytes after the pubkey. This loop starts by reading the last 33 bytes and in each iteration goes forward one byte until it finds the pubkey.

If I understand this code and the change right, the changed code skips reading the last 33 bytes which means that it breaks support for P2PKH. I may misunderstand what’s going on, if that is so, please elaborate what problem was discovered that this fixes.

if i >= 33:
# starting from the back, we move over the scriptSig with a 33 byte
# window (to match a compressed pubkey). we hash this and check if it matches
# the 20 byte has from the scriptPubKey. for standard scriptSigs, this will match
# the 20 byte hash from the scriptPubKey. for standard scriptSigs, this will match
# right away because the pubkey is the last item in the scriptSig.
# if its a non-standard (malleated) scriptSig, we will still find the pubkey if its
# if it's a non-standard (malleated) scriptSig, we will still find the pubkey if it's
# a compressed pubkey.
#
# note: this is an incredibly inefficient implementation, for demonstration purposes only.
pubkey_bytes = vin.scriptSig[i - 33:i]
pubkey_hash = hash160(pubkey_bytes)
if pubkey_hash == spk_hash:
pubkey = ECPubKey().set(pubkey_bytes)
if (pubkey.valid) & (pubkey.compressed):
if pubkey.valid and pubkey.compressed:
return pubkey
if is_p2sh(vin.prevout):
redeem_script = vin.scriptSig[1:]
if is_p2wpkh(redeem_script):
pubkey = ECPubKey().set(vin.txinwitness.scriptWitness.stack[-1])
if (pubkey.valid) & (pubkey.compressed):
if pubkey.valid and pubkey.compressed:
return pubkey
if is_p2wpkh(vin.prevout):
txin = vin.txinwitness
pubkey = ECPubKey().set(txin.scriptWitness.stack[-1])
if (pubkey.valid) & (pubkey.compressed):
if pubkey.valid and pubkey.compressed:
return pubkey
if is_p2tr(vin.prevout):
witnessStack = vin.txinwitness.scriptWitness.stack
if (len(witnessStack) >= 1):
if (len(witnessStack) > 1 and witnessStack[-1][0] == 0x50):
if len(witnessStack) >= 1:
if len(witnessStack) > 1 and witnessStack[-1][0] == 0x50:
# Last item is annex
witnessStack.pop()

if (len(witnessStack) > 1):
if len(witnessStack) > 1:
# Script-path spend
control_block = witnessStack[-1]
# control block is <control byte> <32 byte internal key> and 0 or more <32 byte hash>
# control block is <control byte> <32 byte internal key> and 0 or more <32 byte hash>
internal_key = control_block[1:33]
if (internal_key == NUMS_H.to_bytes(32, 'big')):
if internal_key == NUMS_H.to_bytes(32, 'big'):
# Skip if NUMS_H
return ECPubKey()

pubkey = ECPubKey().set(vin.prevout[2:])
if (pubkey.valid) & (pubkey.compressed):
if pubkey.valid and pubkey.compressed:
return pubkey


return ECPubKey()


Expand Down