Fix tests for JoinSplit signatures

src/main.cpp

@@ -956,8 +956,8 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state)
                 return state.DoS(10, error("CheckTransaction(): prevout is null"),
                                  REJECT_INVALID, "bad-txns-prevout-null");
 
-        // TODO: #966.
         if (tx.vpour.size() > 0) {
+            // TODO: #966.
             static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
             // Empty output script.
             CScript scriptCode;
@@ -967,18 +967,20 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state)
                                 REJECT_INVALID, "error-computing-signature-hash");
             }
 
-            // Add the signature
-            tx.joinSplitPubKey.Verify(dataToBeSigned, tx.joinSplitSig);
-        }
-
-        // Ensure that zk-SNARKs verify
+            // Verify the signature
+            if (!tx.joinSplitPubKey.Verify(dataToBeSigned, tx.joinSplitSig)) {
+                return state.DoS(100, error("CheckTransaction(): JoinSplit signature does not verify"),
+                                REJECT_INVALID, "invalid-joinsplit-signature");
+            }
 
-        if (state.PerformPourVerification()) {
-            BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
-                uint256 pubKeyHash = tx.joinSplitPubKey.GetZcashHash();
-                if (!pour.Verify(*pzcashParams, pubKeyHash)) {
-                    return state.DoS(100, error("CheckTransaction(): pour does not verify"),
-                                     REJECT_INVALID, "bad-txns-pour-verification-failed");
+            // Ensure that zk-SNARKs verify
+            uint256 pubKeyHash = tx.joinSplitPubKey.GetZcashHash();
+            if (state.PerformPourVerification()) {
+                BOOST_FOREACH(const CPourTx &pour, tx.vpour) {
+                    if (!pour.Verify(*pzcashParams, pubKeyHash)) {
+                        return state.DoS(100, error("CheckTransaction(): pour does not verify"),
+                                        REJECT_INVALID, "bad-txns-pour-verification-failed");
+                    }
                 }
             }
         }

src/primitives/transaction.cpp

@@ -110,7 +110,7 @@ std::string CTxOut::ToString() const
 }
 
 CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
-CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vpour(tx.vpour) {}
+CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vpour(tx.vpour), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig) {}
 
 uint256 CMutableTransaction::GetHash() const
 {
@@ -122,9 +122,9 @@ void CTransaction::UpdateHash() const
     *const_cast<uint256*>(&hash) = SerializeHash(*this);
 }
 
-CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0), vpour() { }
+CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0), vpour(), joinSplitPubKey(), joinSplitSig() { }
 
-CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vpour(tx.vpour) {
+CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), vpour(tx.vpour), joinSplitPubKey(tx.joinSplitPubKey), joinSplitSig(tx.joinSplitSig) {
     UpdateHash();
 }
 
@@ -134,6 +134,8 @@ CTransaction& CTransaction::operator=(const CTransaction &tx) {
     *const_cast<std::vector<CTxOut>*>(&vout) = tx.vout;
     *const_cast<unsigned int*>(&nLockTime) = tx.nLockTime;
     *const_cast<std::vector<CPourTx>*>(&vpour) = tx.vpour;
+    *const_cast<CCompressedPubKey*>(&joinSplitPubKey) = tx.joinSplitPubKey;
+    *const_cast<std::vector<unsigned char>*>(&joinSplitSig) = tx.joinSplitSig;
     *const_cast<uint256*>(&hash) = tx.hash;
     return *this;
 }

src/pubkey.h

@@ -215,7 +215,6 @@ public:
     void Serialize(Stream& s, int nType, int nVersion) const
     {
         unsigned int len = pubKey.size();
-        assert(len == 33);
         s.write((char*)pubKey.begin(), len);
     }
 
@@ -236,8 +235,6 @@ public:
         = {'Z','c','a','s','h','E','C','D','S','A','P','u','b','K','e','y'};
 
         uint256 hash;
-        assert(pubKey[0] == 2 || pubKey[0] == 3);
-        assert(pubKey.size() == 33);
         if (crypto_generichash_blake2b_salt_personal(hash.begin(), 32,
                                                     pubKey.begin(), pubKey.size(),
                                                     NULL, 0, // No key.

src/script/interpreter.cpp

@@ -1085,6 +1085,8 @@ public:
             ::Serialize(s, txTo.vpour, nType, nVersion);
             if (txTo.vpour.size() > 0) {
                 ::Serialize(s, txTo.joinSplitPubKey, nType, nVersion);
+                std::vector<unsigned char> nullSig = {};
+                ::Serialize(s, nullSig, nType, nVersion);
             }
         }
     }

src/test/sighash_tests.cpp

@@ -13,6 +13,7 @@
 #include "util.h"
 #include "version.h"
 #include "sodium.h"
+#include "key.h"
 
 #include <iostream>
 
@@ -81,6 +82,8 @@ uint256 static SignatureHashOld(CScript scriptCode, const CTransaction& txTo, un
         txTmp.vin.resize(1);
     }
 
+    txTmp.joinSplitSig = {};
+
     // Serialize and hash
     CHashWriter ss(SER_GETHASH, 0);
     ss << txTmp << nHashType;
@@ -139,6 +142,21 @@ void static RandomTransaction(CMutableTransaction &tx, bool fSingle) {
 
             tx.vpour.push_back(pourtx);
         }
+
+        CKey joinSplitPrivKey;
+        joinSplitPrivKey.MakeNewKey(true);
+        CCompressedPubKey joinSplitPubKey(joinSplitPrivKey.GetPubKey());
+        tx.joinSplitPubKey = joinSplitPubKey;
+        CTransaction signTx(tx);
+
+        // TODO: #966
+        static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
+        CScript scriptCode;
+        uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL);
+        BOOST_CHECK(dataToBeSigned != one);
+
+        // Add the signature
+        joinSplitPrivKey.Sign(dataToBeSigned, tx.joinSplitSig);
     }
 }
 

src/test/transaction_tests.cpp

@@ -396,7 +396,27 @@ BOOST_AUTO_TEST_CASE(test_simple_pour_invalidity)
         pourtx->serials[0] = GetRandHash();
         pourtx->serials[1] = GetRandHash();
 
-        BOOST_CHECK_MESSAGE(CheckTransaction(newTx, state), state.GetRejectReason());
+        BOOST_CHECK(!CheckTransaction(newTx, state));
+        BOOST_CHECK(state.GetRejectReason() == "invalid-joinsplit-signature");
+
+
+        CKey joinSplitPrivKey;
+        joinSplitPrivKey.MakeNewKey(true);
+        CCompressedPubKey joinSplitPubKey(joinSplitPrivKey.GetPubKey());
+
+        newTx.joinSplitPubKey = joinSplitPubKey;
+        CTransaction signTx(newTx);
+
+        // TODO: #966
+        static const uint256 one(uint256S("0000000000000000000000000000000000000000000000000000000000000001"));
+        CScript scriptCode;
+        uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL);
+        BOOST_CHECK(dataToBeSigned != one);
+
+        // Add the signature
+        joinSplitPrivKey.Sign(dataToBeSigned, newTx.joinSplitSig);
+
+        BOOST_CHECK(CheckTransaction(newTx, state));
     }
     {
         // Ensure that values within the pour are well-formed.