[ZIP 244] Changes to transparent component of signature digest

[str4d]

Closes #574.

[krnak]

Reviewed. I confirm this resolves #574. Good job! @str4d

[nuttycom]

One minor request for clarification.

[daira]

utACK

addressed

[dconnolly]

LGTM

[nuttycom]

ACK

[andrewkozlik]

I am wondering whether there is a difference between script_code, pk_script (used in S.2d) and scriptPubKey (used in T.2c). In Bitcoin script_code and scriptPubKey have different meanings, one reason is SegWit BIP141 and the other is due to OP_CODESEPARATOR, but I think neither of these applies in Zcash. If they all have the same meaning, then why not call them all scriptPubKey in this spec? And similarly rename script_codes_sig_digest to scriptpubkeys_sig_digest, so that the naming is more similar to BIP341.

[str4d]

All three names refer to the same field in CTxOut, and mean exactly the same thing: the transparent Bitcoin Script that the transaction output commits to. I've moved to calling it scriptCode (as Bitcoin did when they added SegWit), considering scriptPubKey to be the legacy inaccurate name (since it rarely if ever directly contained a public key in Bitcoin at the time Zcash forked, and in Zcash is almost exclusively either P2PKH or P2SH, containing only hash commitments). You're right that the ZIP shouldn't have both scriptPubKey and scriptCode; this should be fixed.

I don't know for certain why BIP 341 chose to revert from scriptCode back to scriptPubKey, but I suspect it's because Taproot is explicitly a combination of a public key and the hash of a script, so the field contains a public key directly again:

• Let q be the 32-byte array containing the witness program (the second push in the scriptPubKey) which represents a public key according to BIP340.

That rationale doesn't apply to Zcash, but even if we were deploying Taproot, I'd still prefer scriptCode over scriptPubKey.

pk_script is only mentioned because we're referencing the Bitcoin Developer Reference for the definition of CTxOut, which uses that name.

[nuttycom]

https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#cite_note-17, which we cite, refers to this as scriptPubKey, but we can refer to it as scriptCode internally and just keep the reference. Also, in looking at this I noticed that there's an off-by-one error in our cite_note links; I'm not sure how this happened but I'll fix it.

[andrewkozlik]

All three names refer to the same field in CTxOut, and mean exactly the same thing: the transparent Bitcoin Script that the transaction output commits to.

In than case, what you want is scriptPubKey.

I've moved to calling it scriptCode (as Bitcoin did when they added SegWit)

In Bitcoin scriptCode and scriptPubKey are two different things and Bitcoin never stopped using the term scriptPubKey, certainly not with SegWit, see BIP-143.

In addition to the two differences that I mentioned in my original comment, there is one more which is easy to illustrate. Consider a P2SH which consists of a 2-of-3 multisig. The scriptPubKey and scriptCode will look like this:
scriptPubKey: A9 14 <20-byte-scripthash> 87
scriptCode: 52 21 <pubkey1> 21 <pubkey2> 21 <pubkey3> 53 AE

I don't know for certain why BIP 341 chose to revert from scriptCode back to scriptPubKey

That never happened. BIP 341 ceased to use scriptCode as such.

even if we were deploying Taproot, I'd still prefer scriptCode over scriptPubKey.

The established terminology might not be perfect, but mixing it up will not help anyone.

[daira]

I believe @andrewkozlik is correct, and I'm inclined to revert those parts of 76433bb. We need to check whether there are other incorrect usages.

(The above doesn't depend on this, but I also believe that the origin of "scriptPubKey" is that scripts are analogous to public keys, not that they ever were directly public keys.)

[str4d]

We've just had a meeting to figure out what was going on here, where my understanding went wrong, and where the above comments were correct or not.

---

BIP 143 defined scriptCode as being one of two things:

• For P2WPKH, the equivalent of a scriptPubKey for P2PKH (the opcodes are AFAICT identical).
• For P2WSH, the equivalent of a redeemScript extracted from scriptSig for P2SH (if you look at the non-OP_CODESEPARATOR branch).

and did not alter non-SegWit sighashes (i.e. P2PKH or P2SH).

BIP 341 defines two sets of script-related components:

• At a transaction level, the scriptPubKey field for every input is digested (sha_scriptpubkeys).
• At an input level, the scriptPubKey field of that input.

The wording Bitcoin chose to use in their BIPs was incidentally very confusing, using "serialized as" (metaphor) instead of "serialized like" (simile) or the more precise "seralized using the encoding of":

• BIP 143: "scriptCode of the input (serialized as scripts inside CTxOuts)"
  • Refers only to the format of the encoding.
• BIP 341:
  • "the SHA256 of all spent outputs' scriptPubKeys, serialized as script inside CTxOut."
  • "scriptPubKey of the previous output spent by this input, serialized as script inside CTxOut."
    • Refers to the format of the encoding, but is also valid to interpret as the actual encoded field values in the outputs.

---

In ZIP 143 (and maintained in ZIP 243), we mapped the SegWit P2WPKH and P2WSH semantics to P2PKH and P2SH, and defined scriptCode as:

• redeemScript if P2SH.
• scriptPubKey in all other cases.

This commits to the same data as if it just used scriptPubKey in all cases, because the way Bitcoin implemented P2SH, the consensus code only reaches that point if the scriptPubKey has a very specific format in which everything about it ends up being checked prior to using redeemScript. I believe at the time we kept the SegWit formulation because we wanted to stick close to the SegWit semantics, but in hindsight it would have been simpler to just use scriptPubKey everywhere.

Meanwhile, in this PR to ZIP 244 we are attempting to map BIP 341's Taproot semantics to P2PKH and P2SH. If we were to follow them closely, we would need to use scriptPubKey in the digests at both the transaction level (for all inputs) and the input level (per-input). However, in this PR (and in ZIP 244 prior to this PR), at the input level we continue to use scriptCode (i.e. for P2SH we use redeemScript instead of scriptPubKey in its digest).

---

So, conclusions:

• I was partially incorrect to say that scriptPubKey, scriptCode, and pk_script were all the same in this context. They are all identical for P2PKH (and in fact for every script except P2SH), while for P2SH scriptPubKey and pk_script are identical but scriptCode is not.
• We should rename script_codes_sig_digest to scriptpubkeys_sig_digest, to match BIP 341.
• [Consensus change]: We change the per-input data to commit to scriptPubKey instead of scriptCode
  • For P2PKH this is a no-op.
  • For P2SH, this means committing to the Hash160 digest of the redeemScript instead of the actual redeemScript. We discussed this and believe it is fine, because:
    • An adversary trying to take a valid signature and use it with a different redeemScript would need to perform a preimage attack on the Hash160 digest, which doesn't have a square-root optimization anywhere.
    • Even if they could find a preimage, all that allows them to do is alter the input's scriptSig field; this doesn't alter the effecting data of the transaction, which by definition means the transaction has the same effect under consensus (spends the same inputs and produces the same outputs).

[nuttycom]

ACK

[daira]

NACK until the terminology issue is resolved.

[str4d]

Force-pushed to remove the incorrect scriptPubKey -> scriptCode rename. I'll fix the merge conflict and then make the correct fix.

[str4d]

Pushed the fixes.

[nuttycom]

re-ACK 13ef7b6

[daira]

ACK