Remove SigVersion from sighash computation.

Summary: Trigger BIP143 on bit 6 in nHashType. It's not an issue, because it is illegal as this point in time. This will be extended to ensure replay protection. The bit is called FORKID. In a future patch, we will add a fork id in the computation of the sighash in order to ensure replay protection. Test Plan: make check ../qa/pull-tester/rpc-tests.py Also updated expectation for sighash tests so that the do not use bit 6. Reviewers: freetrader, sickpig, #bitcoin_abc Reviewed By: freetrader, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D64
Bitcoin-ABC · Jun 6, 2017 · 1f50d97 · 1f50d97
1 parent ab51494
commit 1f50d97
Show file tree

Hide file tree

Showing 8 changed files with 282 additions and 285 deletions.
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
@@ -1229,10 +1229,10 @@ PrecomputedTransactionData::PrecomputedTransactionData(
 }
 
 uint256 SignatureHash(const CScript &scriptCode, const CTransaction &txTo,
-                      unsigned int nIn, int nHashType, const CAmount &amount,
-                      SigVersion sigversion,
+                      unsigned int nIn, uint32_t nHashType,
+                      const CAmount &amount,
                       const PrecomputedTransactionData *cache) {
-    if (sigversion == SIGVERSION_WITNESS_V0) {
+    if (nHashType & SIGHASH_FORKID) {
         uint256 hashPrevouts;
         uint256 hashSequence;
         uint256 hashOutputs;
@@ -1324,8 +1324,8 @@ bool TransactionSignatureChecker::CheckSig(
     int nHashType = vchSig.back();
     vchSig.pop_back();
 
-    uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount,
-                                    sigversion, this->txdata);
+    uint256 sighash =
+        SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, this->txdata);
 
     if (!VerifySignature(vchSig, pubkey, sighash)) return false;
 

diff --git a/src/script/interpreter.h b/src/script/interpreter.h
@@ -23,6 +23,7 @@ enum {
     SIGHASH_ALL = 1,
     SIGHASH_NONE = 2,
     SIGHASH_SINGLE = 3,
+    SIGHASH_FORKID = 0x40,
     SIGHASH_ANYONECANPAY = 0x80,
 };
 
@@ -128,8 +129,8 @@ enum SigVersion {
 };
 
 uint256 SignatureHash(const CScript &scriptCode, const CTransaction &txTo,
-                      unsigned int nIn, int nHashType, const CAmount &amount,
-                      SigVersion sigversion,
+                      unsigned int nIn, uint32_t nHashType,
+                      const CAmount &amount,
                       const PrecomputedTransactionData *cache = NULL);
 
 class BaseSignatureChecker {

diff --git a/src/script/sign.cpp b/src/script/sign.cpp
@@ -29,8 +29,7 @@ bool TransactionSignatureCreator::CreateSig(std::vector<unsigned char> &vchSig,
     CKey key;
     if (!keystore->GetKey(address, key)) return false;
 
-    uint256 hash =
-        SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion);
+    uint256 hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount);
     if (!key.Sign(hash, vchSig)) return false;
     vchSig.push_back((unsigned char)nHashType);
     return true;

diff --git a/src/test/data/sighash.json b/src/test/data/sighash.json
diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp
@@ -22,8 +22,8 @@ BOOST_FIXTURE_TEST_SUITE(multisig_tests, BasicTestingSetup)
 
 CScript sign_multisig(CScript scriptPubKey, std::vector<CKey> keys,
                       CTransaction transaction, int whichIn) {
-    uint256 hash = SignatureHash(scriptPubKey, transaction, whichIn,
-                                 SIGHASH_ALL, 0, SIGVERSION_BASE);
+    uint256 hash =
+        SignatureHash(scriptPubKey, transaction, whichIn, SIGHASH_ALL, 0);
 
     CScript result;
     // CHECKMULTISIG bug workaround

diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
@@ -352,10 +352,8 @@ class TestBuilder {
 
     TestBuilder &PushSig(const CKey &key, int nHashType = SIGHASH_ALL,
                          unsigned int lenR = 32, unsigned int lenS = 32,
-                         SigVersion sigversion = SIGVERSION_BASE,
                          CAmount amount = 0) {
-        uint256 hash =
-            SignatureHash(script, spendTx, 0, nHashType, amount, sigversion);
+        uint256 hash = SignatureHash(script, spendTx, 0, nHashType, amount);
         std::vector<unsigned char> vchSig, r, s;
         uint32_t iter = 0;
         do {
@@ -1162,8 +1160,7 @@ BOOST_AUTO_TEST_CASE(script_PushData) {
 
 CScript sign_multisig(CScript scriptPubKey, std::vector<CKey> keys,
                       CTransaction transaction) {
-    uint256 hash = SignatureHash(scriptPubKey, transaction, 0, SIGHASH_ALL, 0,
-                                 SIGVERSION_BASE);
+    uint256 hash = SignatureHash(scriptPubKey, transaction, 0, SIGHASH_ALL, 0);
 
     CScript result;
     //
@@ -1433,18 +1430,15 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) {
 
     // A couple of partially-signed versions:
     std::vector<unsigned char> sig1;
-    uint256 hash1 =
-        SignatureHash(scriptPubKey, txTo, 0, SIGHASH_ALL, 0, SIGVERSION_BASE);
+    uint256 hash1 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_ALL, 0);
     BOOST_CHECK(keys[0].Sign(hash1, sig1));
     sig1.push_back(SIGHASH_ALL);
     std::vector<unsigned char> sig2;
-    uint256 hash2 =
-        SignatureHash(scriptPubKey, txTo, 0, SIGHASH_NONE, 0, SIGVERSION_BASE);
+    uint256 hash2 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_NONE, 0);
     BOOST_CHECK(keys[1].Sign(hash2, sig2));
     sig2.push_back(SIGHASH_NONE);
     std::vector<unsigned char> sig3;
-    uint256 hash3 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_SINGLE, 0,
-                                  SIGVERSION_BASE);
+    uint256 hash3 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_SINGLE, 0);
     BOOST_CHECK(keys[2].Sign(hash3, sig3));
     sig3.push_back(SIGHASH_SINGLE);
 

diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp
@@ -133,6 +133,10 @@ BOOST_AUTO_TEST_CASE(sighash_test) {
 #endif
     for (int i = 0; i < nRandomTests; i++) {
         int nHashType = insecure_rand();
+
+        // Clear forkid
+        nHashType &= ~SIGHASH_FORKID;
+
         CMutableTransaction txTo;
         RandomTransaction(txTo, (nHashType & 0x1f) == SIGHASH_SINGLE);
         CScript scriptCode;
@@ -141,8 +145,7 @@ BOOST_AUTO_TEST_CASE(sighash_test) {
 
         uint256 sh, sho;
         sho = SignatureHashOld(scriptCode, txTo, nIn, nHashType);
-        sh =
-            SignatureHash(scriptCode, txTo, nIn, nHashType, 0, SIGVERSION_BASE);
+        sh = SignatureHash(scriptCode, txTo, nIn, nHashType, 0);
 #if defined(PRINT_SIGHASH_JSON)
         CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
         ss << txTo;
@@ -209,7 +212,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) {
             continue;
         }
 
-        sh = SignatureHash(scriptCode, *tx, nIn, nHashType, 0, SIGVERSION_BASE);
+        sh = SignatureHash(scriptCode, *tx, nIn, nHashType, 0);
         BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest);
     }
 }

diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
@@ -47,8 +47,8 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) {
 
         // Sign:
         std::vector<unsigned char> vchSig;
-        uint256 hash = SignatureHash(scriptPubKey, spends[i], 0, SIGHASH_ALL, 0,
-                                     SIGVERSION_BASE);
+        uint256 hash =
+            SignatureHash(scriptPubKey, spends[i], 0, SIGHASH_ALL, 0);
         BOOST_CHECK(coinbaseKey.Sign(hash, vchSig));
         vchSig.push_back((unsigned char)SIGHASH_ALL);
         spends[i].vin[0].scriptSig << vchSig;