P2SH-P2WPKH redeem script must begin with 0x00

mmgen commented at 9:11 am on July 16, 2017: contributor

This fixes the current description of the P2SH-P2WPKH redeem script, which incorrectly prepends 0x16 to the script. This contradicts the correct description in the Segwit Wallet Development Guide:

The P2SH redeemScript is always 22 bytes. It starts with a OP_0, followed by a canonical push of the keyhash (i.e. 0x0014{20-byte keyhash})

The prepended 0x16 also results in the script being a single data push: the witness mechanism is not triggered, the script evaluates to TRUE and the TX is accepted by the network with no signature, creating a potentially dangerous gotcha for wallet developers.

This fixes the current description of the P2SH-P2WPKH redeem script, which incorrectly prepends 0x16 to the script. This contradicts the correct description in https://bitcoincore.org/en/segwit_wallet_dev/:

    The P2SH redeemScript is always 22 bytes. It starts with a OP_0, followed by a canonical push of the keyhash (i.e. 0x0014{20-byte keyhash})

The prepended 0x16 also results in the script being a single data push: the witness mechanism is not triggered, the script evaluates to TRUE and the TX is accepted by the network with no signature, creating a potentially dangerous gotcha for wallet developers.

5c4674e47f

dabura667 commented at 9:49 am on July 16, 2017: none

NACK

You are misunderstanding what a redeemscript is.

Perhaps it needs to be made clear in the parenthesis that “Or, written as a raw hex scriptSig”

The writing is correct as-is. Though I do agree it might be a bit confusing and may cause someone to implement it wrong.

However, your fix is also just as misleading and just as easy to misunderstand. So your fix does not help.

Therefore NACK.

mmgen commented at 10:41 am on July 16, 2017: contributor

@dabura667 The fix is correct. The as-is description is incorrect. There’s no byte 0x16 at the beginning of a Segwit P2SH-P2WPKH redeem script, which MUST begin with byte 0x00 as described in the Development Guide linked above. Prepending byte 0x16 creates a non-Segwit P2SH script redeemable without a signature. I know. I’ve tried it. I encountered this problem while in the process of adding Segwit support to my wallet.

The fix is correctly formed and not misleading in any way. The removal of the angle brackets signifying a data push is equivalent to the removal of the 0x16 in the hex-encoded example that follows.

dabura667 commented at 1:18 pm on July 16, 2017: none

0x16 is not part of the redeemscript, it’s the pushbyte PUSHING the redeemscript on the stack in the scriptSig.

The part you changed is unclear at worst. Notice how they don’t write “redeemscript” on the left, but instead “scriptSig”…

that’s because the script sig is as follows:

0scriptSig:    <0 <20-byte-key-hash>>
1              (0x160014{20-byte-key-hash})
2
31. 0x16 (Push 22 bytes on the stack)
42. 0x0014{20-byte-key-hash} (the 22 bytes to be pushed on the stack by step 1)

Usually when writing Script, people use <> to denote “push this on the stack”

Which is why the scriptSig is just one piece of data pushed on the stack.

to make things less confusing how about saying:

0      scriptSig:   <P2WPKH-script> (This data is the redeemscript in this case)
1  P2WPKH-script:   0 <20-byte-key-hash>
2scriptSig (hex):   0x16 0x0014{20-byte-key-hash}

I repeat. It is not incorrect. Just confusing.

mmgen commented at 2:31 pm on July 16, 2017: contributor

@dabura667 Thanks for the clarification. I like your suggestion. It prevents any confusion by making it absolutely clear that the push byte doesn’t belong to the redeem script. How to go about submitting this as a patch? Close this pull request and open a new one?

My wallet uses bitcoind as a back end, so I didn’t have to craft the scriptSig, only the redeem script. That’s what got me into trouble. Who knows, there might be other front-end wallets out there whose devs could fall into the same trap.

mmgen closed this on Jul 16, 2017

mmgen reopened this on Jul 16, 2017

mmgen closed this on Jul 16, 2017

mmgen deleted the branch on Jul 16, 2017

P2SH-P2WPKH redeem script must begin with 0x00 #558