ZIP 143 test vectors #5

str4d · 2018-06-15T23:02:12Z

The Zcash output is almost identical to the existing sighash.json output, except that amounts are provided.

str4d · 2018-06-15T23:04:44Z

Here is the patch required to run these test vectors in place of sighash.json:

diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
index a73bd6f..ee71af6 100644
--- a/src/test/sighash_tests.cpp
+++ b/src/test/sighash_tests.cpp
@@ -281,6 +281,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
 
         std::string raw_tx, raw_script, sigHashHex;
         int nIn, nHashType;
+        CAmount amount;
         uint32_t consensusBranchId;
         uint256 sh;
         CTransaction tx;
@@ -292,13 +293,15 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
           raw_script = test[1].get_str();
           nIn = test[2].get_int();
           nHashType = test[3].get_int();
-          consensusBranchId = test[4].get_int();
-          sigHashHex = test[5].get_str();
+          assert(-1 == NOT_AN_INPUT);
+          amount = test[4].get_int64();
+          consensusBranchId = test[5].get_int();
+          sigHashHex = test[6].get_str();
 
           uint256 sh;
           CDataStream stream(ParseHex(raw_tx), SER_NETWORK, PROTOCOL_VERSION);
           stream >> tx;
-
+/*
           CValidationState state;
           if (tx.fOverwintered) {
               // Note that OVERWINTER_MIN_CURRENT_VERSION and OVERWINTER_MAX_CURRENT_VERSION
@@ -321,7 +324,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
               BOOST_CHECK_MESSAGE(CheckTransactionWithoutProofVerification(tx, state), strTest);
               BOOST_CHECK(state.IsValid());
           }
-
+*/
           std::vector<unsigned char> raw = ParseHex(raw_script);
           scriptCode.insert(scriptCode.end(), raw.begin(), raw.end());
         } catch (...) {
@@ -329,7 +332,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
           continue;
         }
 
-        sh = SignatureHash(scriptCode, tx, nIn, nHashType, 0, consensusBranchId);
+        sh = SignatureHash(scriptCode, tx, nIn, nHashType, amount, consensusBranchId);
         BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest);
     }
 }

daira

The comment about a length field before a scriptPubKey is blocking.

daira · 2018-06-16T11:08:13Z

transaction.py

+
+
+def pack_g1(p):
+    return struct.pack('b', 0x02 | (1 if p[0] else 0)) + p[1]


This should say that it's the BN-254 encoding, not the BLS12-381 encoding. Also document that p[1] is big-endian (I know it doesn't make a difference for random proofs, but still).

Format character b means signed char; I guess it doesn't make a difference here because the high bit of the byte is not set, but I would use B.

daira · 2018-06-16T11:19:35Z

transaction.py

+        self.g_C = (rand.bool(), rand.b(32))
+        self.g_C_prime = (rand.bool(), rand.b(32))
+        self.g_K = (rand.bool(), rand.b(32))
+        self.g_H = (rand.bool(), rand.b(32))


I'm not sure it matters much yet, but these field element encodings might be out of range.

This is fine, it's not what we're testing.

daira · 2018-06-16T11:38:51Z

transaction.py

+        self.scriptPubKey = Script(rand)
+
+    def __bytes__(self):
+        return struct.pack('<Q', self.nValue) + bytes(self.scriptPubKey)


The scriptPubKey is preceded by a CompactSize length: https://en.bitcoin.it/wiki/Transaction#General_format_.28inside_a_block.29_of_each_output_of_a_transaction_-_Txout

Yep, that's already being added by Script.__bytes__().

daira · 2018-06-16T11:41:36Z

transaction.py

+            for i in range(rand.u8() % 3):
+                self.vJoinSplit.append(JoinSplit(rand))
+            if len(self.vJoinSplit):
+                self.joinSplitPubKey = rand.b(32)


This might also be an invalid key encoding (not a problem, but add comment).

daira · 2018-06-16T11:46:22Z

transaction.py

+            self.nVersionGroupId == OVERWINTER_VERSION_GROUP_ID and \
+            self.nVersion == OVERWINTER_TX_VERSION
+
+        ret += struct.pack('b', len(self.vin))


This depends on self.vin being short. I suggest writing a general CompactSize encoding function; it'll be clearer in intent as well as more general.

daira · 2018-06-16T16:21:33Z

zip_0143.py

+
+
+# Currently assumes the nHashType is SIGHASHALL
+# and that there are no joinSplits


Is this comment correct? It does seem to implement hashing for JoinSplits.

Oops, forgot to update it

daira · 2018-06-16T16:24:04Z

zip_0143.py

+    if not nHashType & SIGHASH_ANYONECANPAY:
+        hashPrevouts = getHashPrevouts(tx)
+
+    if (not nHashType & SIGHASH_ANYONECANPAY) and \


This is correct, but write (not (nHashType & SIGHASH_ANYONECANPAY)) so that readers don't have to look up the precedence :-)

daira · 2018-06-16T16:27:56Z

transaction.py

+            self.randomSeed + \
+            b''.join(self.macs) + \
+            bytes(self.proof) + \
+            b''.join(self.ciphertexts)


Style nit: prefer parens to line continuations, i.e.

return (struct.pack(...) + struct.pack(...) + ... + b''.join(self.ciphertexts))

or

return ( struct.pack(...) + struct.pack(...) + ... + b''.join(self.ciphertexts) )

daira · 2018-06-16T16:37:28Z

zip_0143.py

+            SIGHASH_ALL,
+            SIGHASH_NONE,
+            SIGHASH_SINGLE,
+        ])


This doesn't test SIGHASH_ANYONECANPAY.

daira · 2018-06-16T16:39:33Z

zip_0143.py

+            SIGHASH_NONE,
+            SIGHASH_SINGLE,
+        ])
+        amount = rand.u64() % MAX_MONEY


This is very pedantic: rand.u64() % (MAX_MONEY+1).

(This would mainly act as documentation that the range is 0..MAX_MONEY inclusive.)

daira

The error in write_compact_size is blocking. Everything else looks good.

daira · 2018-08-03T13:19:32Z

zc_utils.py

+def write_compact_size(n):
+    if n < 253:
+        return struct.pack('B', n)
+    elif n < 0xFFFF:


This is not correct; it is <= 0xFFFF (see https://bitcoin.org/en/developer-reference#compactsize-unsigned-integers ).

daira · 2018-08-03T13:19:49Z

zc_utils.py

+        return struct.pack('B', n)
+    elif n < 0xFFFF:
+        return struct.pack('B', 253) + struct.pack('<H', n)
+    elif n < 0xFFFFFFFF:


Similarly, <= 0xFFFFFFFF.

daira · 2018-08-03T13:20:50Z

zc_utils.py

+assert write_compact_size(252) == b'\xFC'
+assert write_compact_size(253) == b'\xFD\xFD\x00'
+assert write_compact_size(0xFFFE) == b'\xFD\xFE\xFF'
+assert write_compact_size(0xFFFF) == b'\xFE\xFF\xFF\x00\x00'


b'\FD\xFF\xFF'

daira · 2018-08-03T13:23:06Z

zc_utils.py

+assert write_compact_size(0xFFFE) == b'\xFD\xFE\xFF'
+assert write_compact_size(0xFFFF) == b'\xFE\xFF\xFF\x00\x00'
+assert write_compact_size(0xFFFFFFFE) == b'\xFE\xFE\xFF\xFF\xFF'
+assert write_compact_size(0xFFFFFFFF) == b'\xFF\xFF\xFF\xFF\xFF\x00\x00\x00\x00'


`b'\xFE\xFF\xFF\xFF\xFF'

Also add tests for 0x10000, 0x100000000 (8 '0's), and 0xFFFFFFFFFFFFFFFF (16 'F's).

daira

Flushing comments.

No support for JoinSplits yet. Co-authored-by: Jack Grigg <jack@z.cash>

str4d · 2018-08-20T11:29:45Z

Rebased on master to fix merge conflicts and address @daira's comments. I also squashed the commits from the second-to-last review round.

Used to convert a -1 for JSON to Option::None in Rust

str4d · 2018-08-21T19:29:19Z

Fixed some issues I had when trying to use the test vectors in Rust.

I have also manually confirmed that none of the previous test vectors' Rust output is altered by this PR.

daira

Premature ACK, ignore.

daira · 2018-08-21T21:24:40Z

transaction.py

+        return self._script
+
+    def __bytes__(self):
+        return struct.pack('b', len(self._script)) + self._script


Note to self: needs CompactSize.

daira · 2018-08-21T21:49:45Z

zip_0143.py

+SIGHASH_SINGLE = 3
+SIGHASH_ANYONECANPAY = 0x80
+
+NOT_AN_INPUT = -1 # For portability of the test vectors


I originally thought this should be NOT_AN_INPUT = 0xFFFFFFFF, but in fact the C++ code defines it as UINT_MAX regardless of the size of unsigned int, so there is no correct cross-platform value to use other than -1. As long as the JSON test vectors use -1, this will work.

daira

utACK

daira · 2018-08-21T21:58:33Z

transaction.py

+        self.g_C = (rand.bool(), rand.b(32))
+        self.g_C_prime = (rand.bool(), rand.b(32))
+        self.g_K = (rand.bool(), rand.b(32))
+        self.g_H = (rand.bool(), rand.b(32))


This is fine, it's not what we're testing.

str4d mentioned this pull request Jun 15, 2018

[ZIP 143] Add an example zcash/zips#140

Closed

daira requested changes Jun 16, 2018

View reviewed changes

str4d mentioned this pull request Jul 6, 2018

ZIP 143 updates zcash/zips#162

Merged

daira requested changes Aug 3, 2018

View reviewed changes

daira reviewed Aug 3, 2018

View reviewed changes

str4d and others added 5 commits August 20, 2018 12:04

Implement helper class for generating random values

fb4e0d6

Implement ZIP 143 test vector generation

704a2ac

No support for JoinSplits yet. Co-authored-by: Jack Grigg <jack@z.cash>

Implement JoinSplits in ZIP 143 test vectors

9448eee

Don't reverse 32-byte transactions in Zcash output

07b7a6e

Implement CompactSize helper

acdfe06

str4d force-pushed the zip-143 branch from 6ae774f to acdfe06 Compare August 20, 2018 11:29

str4d added 3 commits August 21, 2018 19:37

Use snake_case for ZIP 143 test vector part names

af98406

Render Vec<u8> data correctly

a9676cc

Refactor Rust output rendering to support reformatting

17d146b

Used to convert a -1 for JSON to Option::None in Rust

daira approved these changes Aug 21, 2018

View reviewed changes

str4d merged commit 4e8e742 into zcash:master Aug 22, 2018

str4d deleted the zip-143 branch August 22, 2018 19:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZIP 143 test vectors #5

ZIP 143 test vectors #5

str4d commented Jun 15, 2018

str4d commented Jun 15, 2018

daira left a comment

daira Jun 16, 2018

daira Jun 16, 2018

daira Aug 21, 2018

daira Jun 16, 2018

str4d Jun 22, 2018

daira Jun 16, 2018

daira Jun 16, 2018

daira Jun 16, 2018

str4d Jun 22, 2018

daira Jun 16, 2018 •

edited

daira Jun 16, 2018

daira Jun 16, 2018

daira Jun 16, 2018

daira left a comment

daira Aug 3, 2018

daira Aug 3, 2018

daira Aug 3, 2018

daira Aug 3, 2018

daira left a comment

str4d commented Aug 20, 2018

str4d commented Aug 21, 2018

daira left a comment •

edited

daira Aug 21, 2018

daira Aug 21, 2018

daira left a comment

daira Aug 21, 2018



		def pack_g1(p):
		return struct.pack('b', 0x02 \| (1 if p[0] else 0)) + p[1]



		# Currently assumes the nHashType is SIGHASHALL
		# and that there are no joinSplits

ZIP 143 test vectors #5

ZIP 143 test vectors #5

Conversation

str4d commented Jun 15, 2018

str4d commented Jun 15, 2018

daira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daira Jun 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

str4d commented Aug 20, 2018

str4d commented Aug 21, 2018

daira left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daira Jun 16, 2018 •

edited

daira left a comment •

edited