[ZIP 143] Transaction Signature Verification for Overwinter #129

str4d · 2018-01-08T22:08:19Z

No description provided.

str4d · 2018-01-09T00:03:48Z

WRT replay protection, we decided in a meeting today that the branch ID in the transaction's block serialization format would be used for replay protection, so its presence in the personalization field is no longer specifically needed for replay protection. But we will leave it there because it is good practice to personalize the hash function over the desired domain (in our case, the branch "epoch" as referred to in #128).

str4d · 2018-01-09T11:19:20Z

drafts/str4d-overwinter-sighash/draft1.rst

+    2. hashPrevouts (32-byte hash)
+    3. hashSequence (32-byte hash)
+    4. hashOutputs (32-byte hash)
+    5. hashJoinSplits (32-byte hash)


(in reply to str4d@3d29239#r26720276)

@daira I was thinking that we might end up having separate hashes for input and output Sapling circuits, ie:

5. hashJoinSplits (32-byte hash) 6. hashShieldedInputs (32-byte hash) 7. hashShieldedOutputs (32-byte hash)

I'm not sure whether we need this kind of flexibility though, in the way that it is useful for the different transparent sighash types (because I expect we'd want to implement those in a zero-knowledge way, which doing inside a visible signature would not be).

Note that we will need to have another ZIP with a newer SigHash specification anyway when we address transaction malleability.

We discussed this in a pairing, and decided that we don't need to add anything for Sapling in advance; we can conditionally add it to the algorithm (gated on transaction version) when we add Sapling.

str4d · 2018-01-09T11:22:11Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+2: ``hashPrevouts``
+```````````````````
+* If the ``ANYONECANPAY`` flag is not set, ``hashPrevouts`` is the double SHA256 of the serialization of all


(in reply to str4d@3d29239#r26723101)

@daira these are the inner hashes. Do we want to change these to BLAKE2 as well? And if so, we should personalize them too.

str4d · 2018-01-09T11:25:41Z

drafts/str4d-overwinter-sighash/draft1.rst

+5: ``hashJoinSplits``
+`````````````````````
+* If ``vjoinsplits`` is non-empty, ``hashJoinSplits`` is the double SHA256 of the serialization of all
+  JoinSplits concatenated with the joinSplitPubKey;


(in reply to str4d@3d29239#r26723268)

@daira I don't think we want this encoded as in a transaction:

The transaction uses a std::vector encoding for vin, vout, and vjoinsplit, whereas the other hash* in this proposal (of data from vin and vout) use simple concatenation. We should follow the latter here for consistency with vjoinsplit.

In Sprout we serialized a "null" joinSplitSig. I see no need to do that here.

str4d · 2018-01-09T11:28:24Z

drafts/str4d-overwinter-sighash/draft1.rst

+  inside CTxOuts);
+
+* If sighash type is ``SINGLE`` and the input index is smaller than the number of outputs, ``hashOutputs`` is
+  the double SHA256 of the output amount with ``scriptPubKey`` of the same index as the input;


(in reply to str4d@3d29239#r26723147)

@daira agreed, the wording from BIP 143 isn't clear. It means the same serialization as the first case, but only of the output corresponding to the input signature being created (instead of the whole vout).

str4d · 2018-01-09T11:29:42Z

drafts/str4d-overwinter-sighash/draft1.rst

+#. ``SINGLE`` does not commit to the input index. When ``ANYONECANPAY`` is not set, the semantics are
+   unchanged since ``hashPrevouts`` and ``outpoint`` together implictly commit to the input index. When
+   ``SINGLE`` is used with ``ANYONECANPAY``, omission of the index commitment allows permutation of the
+   input-output pairs, as long as each pair is located at an equivalent index.


(in reply to str4d@3d29239#r26723093)

@daira I don't think there's any security issue. It just allows shuffling of the order of transparent input/output pairs, in the same way Sapling will allow shuffling of the order of input circuits and output circuits (not in pairs).

bitcartel · 2018-01-17T08:16:44Z

drafts/str4d-overwinter-sighash/draft1.rst

+  Title: Transaction Signature Verification for Overwinter
+  Author: Jack Grigg <jack@z.cash>
+          Daira Hopwood <daira@z.cash>
+  Credits: Johnson Lau <jl2012@xbt.hk>


We should also credit BitcoinABC because they inspired ticket #2584 by writing up a spec https://github.com/Bitcoin-UAHF/spec/blob/master/uahf-technical-spec.md and https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/doc/abc/replay-protected-sighash.md demonstrating how to extract the relevant bits from BIP 143 etc.

bitcartel · 2018-01-17T08:17:37Z

drafts/str4d-overwinter-sighash/draft1.rst

+    4. hashSequence (32-byte hash)
+    5. hashOutputs (32-byte hash)
+    6. hashJoinSplits (32-byte hash)
+    7. nLockTime of the transaction (8-byte little endian)


nLockTime is uint32_t

bitcartel · 2018-01-17T08:18:01Z

drafts/str4d-overwinter-sighash/draft1.rst

+    5. hashOutputs (32-byte hash)
+    6. hashJoinSplits (32-byte hash)
+    7. nLockTime of the transaction (8-byte little endian)
+    8. nExpiryTime of the transaction (8-byte little endian)


Rename to nExpiryHeight. Also both nLockTime and nExpiryHeight are uint32_t so 4-byte little endian, not 8-byte.

bitcartel · 2018-01-17T08:22:30Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+  "ZcashSigHash" || BLOCK_BRANCH_ID
+
+``BLOCK_BRANCH_ID`` is the ``BRANCH_ID`` for the epoch of the transaction's parent block. [#ZIP-activation-mechanism]_


Can we think of terminology that better clarifies the different identifiers? e.g. FAMILY_BRANCH_ID (I think family, e.g. Zcash or some clone) whereas EPOCH_ID (epoch makes me think of time so then I start thinking that the ID will expire, makes me less confident/comfortable for some reason).

Above comment is probably not relevant given recent discussion.

Decided to name it CONSENSUS_BRANCH_ID to match usage in the code.

daira · 2018-01-25T23:27:59Z

The BLAKE2b personalization field for the inner hashes should be "Zcash_Inner_Hash".

Overwinter SignatureHash Implements zcash/zips#129. Includes code cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7276 - bitcoin/bitcoin#7976 - bitcoin/bitcoin#8118 - bitcoin/bitcoin#8149 - Only amount validation and SignatureHash commits. - bitcoin/bitcoin#6915 - Only the rework of `mempool.check()` calls that the next PR depends on. - bitcoin/bitcoin#8346 - bitcoin/bitcoin#8524 Part of #2254. Closes #1408 and #2584.

bitcartel · 2018-02-08T22:51:14Z

drafts/str4d-overwinter-sighash/draft1.rst

+A new transaction digest algorithm is defined, but only applicable from the Overwinter upgrade block height
+[#ZIP0000]_::
+  BLAKE2b-256 of the serialization of:
+    1. nVersion of the transaction (4-byte little endian)


Referred to in the transaction format zip as header. you could break this into compontents fOverwintered flag and nVersion.

bitcartel · 2018-02-08T22:51:36Z

drafts/str4d-overwinter-sighash/draft1.rst

+[#ZIP0000]_::
+  BLAKE2b-256 of the serialization of:
+    1. nVersion of the transaction (4-byte little endian)
+    2. nVersionBranch of the transaction (4-byte little endian)


Now: nVersionGroupId

bitcartel · 2018-02-08T22:52:54Z

drafts/str4d-overwinter-sighash/draft1.rst

+       c. value of the output spent by this input (8-byte little endian)
+       d. nSequence of the input (4-byte little endian)
+
+The BLAKE2b-256 personalization field is set to::


Add link/reference to blake2

bitcartel · 2018-02-08T22:54:15Z

drafts/str4d-overwinter-sighash/draft1.rst

+* Transaction creators MUST specify the epoch they want their transaction to be mined in. Across a network
+  upgrade, this means that if a transaction is not mined before the activation height, it will never be mined.
+
+* Transaction validators MUST NOT use ``nVersionBranch`` as the ``BLOCK_BRANCH_ID``: while these are both a


WIth nVersionGroupId, this bullet point is rendundant and could be removed.

bitcartel · 2018-02-08T22:56:28Z

drafts/str4d-overwinter-sighash/draft1.rst

+Field definitions
+-----------------
+
+The items 1, 7, 9, 10a, 10d have the same meaning as the original algorithm. [#wiki-checksig]_


Item 1 is now the header and not nversion.

bitcartel · 2018-02-08T22:56:51Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+The items 1, 7, 9, 10a, 10d have the same meaning as the original algorithm. [#wiki-checksig]_
+
+2: ``nVersionBranch``


nVersionGroupId - so this section should be reworked.

bitcartel · 2018-02-08T23:02:58Z

drafts/str4d-overwinter-sighash/draft1.rst

+The ``BRANCH_ID`` of the epoch in which the transaction format corresponding to ``nVersion`` was introduced.
+[#ZIP-overwinter-tx-format]_
+
+3: ``hashPrevouts``


So most of this document here is the same as BIP143 and this should be made clearer at start of ZIP.
It would be helpful if we clearly labelled which sections are Zcash only, which are pure BIP143 and which are BIP143+ZcashModification This will help folk who are familiar with that BIP.

Added a note about this being based on BIP 143. Note that none of the document is pure BIP 143 anymore, so while some code reuse is possible, it will likely be of the copy-paste form (e.g. hashPrevouts now uses BLAKE2b, not SHA256d, so a function for calculating that can't trivially be reused).

bitcartel · 2018-02-08T23:04:32Z

drafts/str4d-overwinter-sighash/draft1.rst

+  JoinSplits (in their canonical transaction serialization format) concatenated with the joinSplitPubKey;
+
+  * Note that the JoinSplit proofs are included in the signature hash, as with v1 and v2 transactions. In a
+    future transaction digest algorithm, the proofs will likely be omitted as authentication data, in the same


Remove "In a future..." as not relevant to current spec.

bitcartel · 2018-02-08T23:04:46Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+* Otherwise, ``hashJoinSplits`` is a ``uint256`` of ``0x0000......0000``.
+
+8: ``nExpiryTime``


nExpiryHeight

bitcartel · 2018-02-08T23:05:18Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+8: ``nExpiryTime``
+``````````````
+The block height or time at which the transaction becomes unilaterally invalid, and can never be mined.


Only block height, not time.
The last block height a transaction can be valid.

bitcartel · 2018-02-08T23:07:10Z

Added extra review comments, seems to be broken up by Github into two sections above.

str4d · 2018-02-09T16:15:49Z

@bitcartel addressed your comments.

randombit · 2018-02-12T18:31:59Z

drafts/str4d-overwinter-sighash/draft1.rst

+-----
+
+When generating ``hashPrevouts``, ``hashSequence``, ``hashOutputs``, and ``hashJoinSplits``, the BLAKE2b-256
+personalization field is set to ``Zcash_Inner_Hash``.


Is there a specific reason for sharing the personalization field across all 4 input types? It seems better to domain separate these different input types if possible. (To be clear I don't see any specific problem caused by this, and perhaps using a single string allows some optimization I've missed - but if so, maybe the text should mention said optimization.)

Yes, let's change these to:

ZcashPrevoutHash

ZcashSequencHash [spelling intended]

ZcashOutputsHash

ZcashJSplitsHash

daira · 2018-02-16T14:48:18Z

drafts/str4d-overwinter-sighash/draft1.rst

+  Credits: Johnson Lau <jl2012@xbt.hk>
+           Pieter Wuille <pieter.wuille@gmail.com>
+           Bitcoin-ABC
+  Category: Process


Category: Consensus

daira · 2018-02-16T16:58:23Z

drafts/str4d-overwinter-sighash/draft1.rst

+
+There are 4 ECDSA signature verification codes in the original Zcash script system: ``CHECKSIG``,
+``CHECKSIGVERIFY``, ``CHECKMULTISIG``, ``CHECKMULTISIGVERIFY`` ("sigops"). According to the sighash type
+(``ALL``, ``NONE``, or ``SINGLE``, possibly modified by ``ANYONECANPAY``), a transaction digest is generated


This is rst not Markdown, so single backticks for monospace throughout the document.

Per http://docutils.sourceforge.net/docs/user/rst/quickref.html#inline-markup double backticks is an inline literal rendered as monospace, while single backticks are interpreted text, which could be for explicit descriptive markup like program identifiers. Markdown is single ticks for code.

daira · 2018-02-16T17:00:32Z

drafts/str4d-overwinter-sighash/draft1.rst

+The new algorithm MUST be used for signatures created over the Overwinter transaction format.
+[#ZIP-overwinter-tx-format]_ Combined with the new consensus rule that v1 and v2 transaction formats will be
+invalid from the Overwinter upgrade, [#ZIP-overwinter-tx-format]_ this effectively means that all transactions
+signatures from the Overwinter activation height will use the new algorithm. [#ZIP0000]_


What is #ZIP0000 intended to reference?

I think I planned to link to whichever ZIP contained the activation heights.

daira · 2018-02-17T10:10:12Z

drafts/str4d-overwinter-sighash/draft1.rst

+  transaction. Therefore, data hashing grows in O(n\ :sup:`2`) as the number of sigops in a transaction
+  increases. While a 1 MB block would normally take 2 seconds to verify with an average computer in 2015, a
+  1MB transaction with 5569 sigops may take 25 seconds to verify. This could be fixed by optimizing the digest
+  algorithm by introducing some reusable “midstate”, so the time complexity becomes O(n). [#quadratic]_


Non-ASCII quotes are used here, plain ASCII quotes elsewhere (e.g. "cold wallet"). Unicode is allowed in ZIPs, but consistency is preferable. I would suggest just using plain quotes.

This wording leaves questions in a reader's mind: what is "midstate"? Does the proposal adopted in this ZIP use midstate?

This is the same wording as in BIP 143, so I'm fine leaving it as-is, unless you have an alternative suggestion.

daira · 2018-02-17T10:14:54Z

drafts/str4d-overwinter-sighash/draft1.rst

+    7. nLockTime of the transaction (4-byte little endian)
+    8. nExpiryHeight of the transaction (4-byte little endian)
+    9. sighash type of the signature (4-byte little endian)
+   10. If we are serializing an input (ie. this is not a JoinSplit signature hash):


s/ie./i.e./

daira · 2018-02-17T13:48:00Z

drafts/str4d-overwinter-sighash/draft1.rst

+      {'Z','c','a','s','h','O','u','t','p','u','t','s','H','a','s','h'};
+  const unsigned char ZCASH_JOINSPLITS_HASH_PERSONALIZATION[16] =
+      {'Z','c','a','s','h','J','S','p','l','i','t','s','H','a','s','h'};
+


Add a comment that the default values are zeros.

daira · 2018-02-17T13:50:44Z

drafts/str4d-overwinter-sighash/draft1.rst

+      hashJoinSplits = ss.GetHash();
+  }
+
+  uint32_t leConsensusBranchId = htole32(consensusBranchId);


Aside: I've never really liked htole32 and friends; they seem to me to be making a type error that is then compensated for by another type error at the memcpy. (Does not block.)

daira · 2018-02-17T21:23:30Z

drafts/str4d-overwinter-sighash/draft1.rst

+-----
+
+The ``hashPrevouts``, ``hashSequence``, ``hashOutputs``, and ``hashJoinSplits`` calculated in an earlier
+verification may be reused in other inputs of the same transaction, so that the time complexity of the whole


daira · 2018-02-17T21:25:11Z

drafts/str4d-overwinter-sighash/draft1.rst

+Deployment
+==========
+
+This proposal is deployed with the Overwinter network upgrade.


Reference ZIP 201 for the activation heights.

daira · 2018-02-17T21:25:56Z

drafts/str4d-overwinter-sighash/draft1.rst

+Example
+=======
+
+TBC


An example is not needed to merge this, but please file a ticket to add one.

Overwinter SignatureHash Implements zcash/zips#129. Includes code cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7276 - bitcoin/bitcoin#7976 - bitcoin/bitcoin#8118 - bitcoin/bitcoin#8149 - Only amount validation and SignatureHash commits. - bitcoin/bitcoin#6915 - Only the rework of `mempool.check()` calls that the next PR depends on. - bitcoin/bitcoin#8346 - bitcoin/bitcoin#8524 Part of #2074 and #2254. Closes #1408 and #2584.

str4d · 2018-02-20T13:17:00Z

In zcash/zcash#2584 (comment) @daira wrote:

Let's call it "Replay-Protected Scalable Lightweight-wallet-friendly hashing" or "RPSL hashing".

Do you still want this?

daira

ACK

Overwinter signature hashing draft 1

3d29239

str4d added protocol spec ZIP idea labels Jan 8, 2018

str4d commented Jan 9, 2018

View reviewed changes

str4d added 7 commits January 10, 2018 01:08

Use BLAKE2b-256 for the outer personalized hash

9f5dc6d

Add note about inclusion of JoinSplit proofs in the signature hash

7d20a43

Add note about replay protection effect of hash personalization

beaea49

Reference use of transaction digest algorithm for JoinSplit signatures

35854f6

Add transaction expiry value into transaction digest

2a4f012

Cleanups and clarity

8a4101b

Update draft based on latest transaction format specification draft

d769180

bitcartel reviewed Jan 17, 2018

View reviewed changes

arielgabizon mentioned this pull request Jan 23, 2018

Integrate quadhash fix zcash/zcash#2896

Closed

str4d mentioned this pull request Jan 25, 2018

Overwinter SignatureHash zcash/zcash#2903

Merged

str4d added Overwinter transaction parsing labels Jan 25, 2018

bitcartel suggested changes Feb 8, 2018

View reviewed changes

str4d added 4 commits February 9, 2018 14:19

Correct field sizes

1bfee9e

Update draft based on latest transaction format specification draft

cf28700

Remove unnecessary metadata

4998e99

Reference BIP 143 and BUIP-HF v1.2

4e02d73

str4d added 3 commits February 9, 2018 16:05

Fix typo

12d4e4d

Add reference for BLAKE2 personalization field

252c22b

Remove comment about future sighash algorithms

1921ce1

str4d requested review from bitcartel and daira February 9, 2018 16:15

str4d added 4 commits February 9, 2018 16:16

Fix rST formatting

59cea03

Clarify sentence

639c1de

Add link to reference implementation

6297ac5

Specify scriptCode

2d7401a

randombit reviewed Feb 12, 2018

View reviewed changes

daira changed the title ~~Overwinter signature hashing ZIP~~ [ZIP 143] Transaction Signature Verification for Overwinter Feb 16, 2018

Update inner hash personalizations

7fb2467

daira requested changes Feb 17, 2018

View reviewed changes

str4d added 6 commits February 26, 2018 22:47

Update category

abb6e74

Remove TODO (resolved in 2d7401a)

349b4cf

Cleanups

3d0c5ec

Update ZIP references

f39f59c

Address remaining comments

519ca18

Move ZIP 143 out of drafts

c4baac7

str4d force-pushed the 2584-zip-143 branch from f793311 to c4baac7 Compare February 28, 2018 22:40

Add terminology references

60c7424

bitcartel approved these changes Mar 2, 2018

View reviewed changes

daira approved these changes Mar 2, 2018

View reviewed changes

daira mentioned this pull request Mar 2, 2018

[ZIP 143] Add an example #140

Closed

daira merged commit ed7cef7 into zcash:master Mar 2, 2018

daira added the ZIP number assigned label Apr 10, 2018

str4d deleted the 2584-zip-143 branch November 3, 2022 09:37


		"ZcashSigHash" \|\| BLOCK_BRANCH_ID

		``BLOCK_BRANCH_ID`` is the ``BRANCH_ID`` for the epoch of the transaction's parent block. [#ZIP-activation-mechanism]_


		The items 1, 7, 9, 10a, 10d have the same meaning as the original algorithm. [#wiki-checksig]_

		2: ``nVersionBranch``


		* Otherwise, ``hashJoinSplits`` is a ``uint256`` of ``0x0000......0000``.

		8: ``nExpiryTime``

+              Example
+              =======
+              TBC

[ZIP 143] Transaction Signature Verification for Overwinter #129

[ZIP 143] Transaction Signature Verification for Overwinter #129

Uh oh!

Conversation

str4d commented Jan 8, 2018

Uh oh!

str4d commented Jan 9, 2018

Uh oh!

str4d Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitcartel Jan 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daira commented Jan 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitcartel commented Feb 8, 2018

Uh oh!

str4d commented Feb 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

str4d Jan 9, 2018 •

edited

Loading

bitcartel Jan 17, 2018 •

edited

Loading