Inline signature serializer

Instead of building a full copy of a CTransaction being signed, and
then modifying bits and pieces until its fits the form necessary
for computing the signature hash, use a wrapper serializer that
only serializes the necessary bits on-the-fly.

This makes it easier to see which data is actually being hash,
reduces load on the heap, and also marginally improves performances
(around 3-4us/sigcheck here). The performance improvements are much
larger for large transactions, though.

The old implementation of SignatureHash is moved to a unit tests,
to test whether the old and new algorithm result in the same value
for randomly-constructed transactions.

src/script.cpp

@@ -971,62 +971,118 @@ bool EvalScript(vector<vector<unsigned char> >& stack, const CScript& script, co
 
 
 
+namespace {
+/** Wrapper that serializes like CTransaction, but with the modifications
+ *  required for the signature hash done in-place
+ */
+class CTransactionSignatureSerializer {
+private:
+    const CTransaction &txTo;  // reference to the spending transaction (the one being serialized)
+    const CScript &scriptCode; // output script being consumed
+    const unsigned int nIn;    // input index of txTo being signed
+    const bool fAnyoneCanPay;  // whether the hashtype has the SIGHASH_ANYONECANPAY flag set
+    const bool fHashSingle;    // whether the hashtype is SIGHASH_SINGLE
+    const bool fHashNone;      // whether the hashtype is SIGHASH_NONE
 
-
-uint256 SignatureHash(CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType)
-{
-    if (nIn >= txTo.vin.size())
-    {
-        LogPrintf("ERROR: SignatureHash() : nIn=%d out of range\n", nIn);
-        return 1;
+public:
+    CTransactionSignatureSerializer(const CTransaction &txToIn, const CScript &scriptCodeIn, unsigned int nInIn, int nHashTypeIn) :
+        txTo(txToIn), scriptCode(scriptCodeIn), nIn(nInIn),
+        fAnyoneCanPay(!!(nHashTypeIn & SIGHASH_ANYONECANPAY)),
+        fHashSingle((nHashTypeIn & 0x1f) == SIGHASH_SINGLE),
+        fHashNone((nHashTypeIn & 0x1f) == SIGHASH_NONE) {}
+
+    /** Serialize the passed scriptCode, skipping OP_CODESEPARATORs */
+    template<typename S>
+    void SerializeScriptCode(S &s, int nType, int nVersion) const {
+        CScript::const_iterator it = scriptCode.begin();
+        CScript::const_iterator itBegin = it;
+        opcodetype opcode;
+        unsigned int nCodeSeparators = 0;
+        while (scriptCode.GetOp(it, opcode)) {
+            if (opcode == OP_CODESEPARATOR)
+                nCodeSeparators++;
+        }
+        ::WriteCompactSize(s, scriptCode.size() - nCodeSeparators);
+        it = itBegin;
+        while (scriptCode.GetOp(it, opcode)) {
+            if (opcode == OP_CODESEPARATOR) {
+                s.write((char*)&itBegin[0], it-itBegin-1);
+                itBegin = it;
+            }
+        }
+        s.write((char*)&itBegin[0], it-itBegin);
     }
-    CTransaction txTmp(txTo);
 
-    // In case concatenating two scripts ends up with two codeseparators,
-    // or an extra one at the end, this prevents all those possible incompatibilities.
-    scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
+    /** Serialize an input of txTo */
+    template<typename S>
+    void SerializeInput(S &s, unsigned int nInput, int nType, int nVersion) const {
+        // In case of SIGHASH_ANYONECANPAY, only the input being signed is serialized
+        if (fAnyoneCanPay)
+            nInput = nIn;
+        // Serialize the prevout
+        ::Serialize(s, txTo.vin[nInput].prevout, nType, nVersion);
+        // Serialize the script
+        if (nInput != nIn)
+            // Blank out other inputs' signatures
+            ::Serialize(s, CScript(), nType, nVersion);
+        else
+            SerializeScriptCode(s, nType, nVersion);
+        // Serialize the nSequence
+        if (nInput != nIn && (fHashSingle || fHashNone))
+            // let the others update at will
+            ::Serialize(s, (int)0, nType, nVersion);
+        else
+            ::Serialize(s, txTo.vin[nInput].nSequence, nType, nVersion);
+    }
 
-    // Blank out other inputs' signatures
-    for (unsigned int i = 0; i < txTmp.vin.size(); i++)
-        txTmp.vin[i].scriptSig = CScript();
-    txTmp.vin[nIn].scriptSig = scriptCode;
+    /** Serialize an output of txTo */
+    template<typename S>
+    void SerializeOutput(S &s, unsigned int nOutput, int nType, int nVersion) const {
+        if (fHashSingle && nOutput != nIn)
+            // Do not lock-in the txout payee at other indices as txin
+            ::Serialize(s, CTxOut(), nType, nVersion);
+        else
+            ::Serialize(s, txTo.vout[nOutput], nType, nVersion);
+    }
 
-    // Blank out some of the outputs
-    if ((nHashType & 0x1f) == SIGHASH_NONE)
-    {
-        // Wildcard payee
-        txTmp.vout.clear();
+    /** Serialize txTo */
+    template<typename S>
+    void Serialize(S &s, int nType, int nVersion) const {
+        // Serialize nVersion
+        ::Serialize(s, txTo.nVersion, nType, nVersion);
+        // Serialize vin
+        unsigned int nInputs = fAnyoneCanPay ? 1 : txTo.vin.size();
+        ::WriteCompactSize(s, nInputs);
+        for (unsigned int nInput = 0; nInput < nInputs; nInput++)
+             SerializeInput(s, nInput, nType, nVersion);
+        // Serialize vout
+        unsigned int nOutputs = fHashNone ? 0 : (fHashSingle ? nIn+1 : txTo.vout.size());
+        ::WriteCompactSize(s, nOutputs);
+        for (unsigned int nOutput = 0; nOutput < nOutputs; nOutput++)
+             SerializeOutput(s, nOutput, nType, nVersion);
+        // Serialie nLockTime
+        ::Serialize(s, txTo.nLockTime, nType, nVersion);
+    }
+};
+}
 
-        // Let the others update at will
-        for (unsigned int i = 0; i < txTmp.vin.size(); i++)
-            if (i != nIn)
-                txTmp.vin[i].nSequence = 0;
+uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType)
+{
+    if (nIn >= txTo.vin.size()) {
+        LogPrintf("ERROR: SignatureHash() : nIn=%d out of range\n", nIn);
+        return 1;
     }
-    else if ((nHashType & 0x1f) == SIGHASH_SINGLE)
-    {
-        // Only lock-in the txout payee at same index as txin
-        unsigned int nOut = nIn;
-        if (nOut >= txTmp.vout.size())
-        {
-            LogPrintf("ERROR: SignatureHash() : nOut=%d out of range\n", nOut);
+
+    // Check for invalid use of SIGHASH_SINGLE
+    if ((nHashType & 0x1f) == SIGHASH_SINGLE) {
+        if (nIn >= txTo.vout.size()) {
+            LogPrintf("ERROR: SignatureHash() : nOut=%d out of range\n", nIn);
             return 1;
         }
-        txTmp.vout.resize(nOut+1);
-        for (unsigned int i = 0; i < nOut; i++)
-            txTmp.vout[i].SetNull();
-
-        // Let the others update at will
-        for (unsigned int i = 0; i < txTmp.vin.size(); i++)
-            if (i != nIn)
-                txTmp.vin[i].nSequence = 0;
     }
 
-    // Blank out other inputs completely, not recommended for open transactions
-    if (nHashType & SIGHASH_ANYONECANPAY)
-    {
-        txTmp.vin[0] = txTmp.vin[nIn];
-        txTmp.vin.resize(1);
-    }
+    // Wrapper to serialize only the necessary parts of the transaction being signed
+    CTransactionSignatureSerializer txTmp(txTo, scriptCode, nIn, nHashType);
 
     // Serialize and hash
     CHashWriter ss(SER_GETHASH, 0);

src/test/Makefile.am

@@ -34,7 +34,7 @@ test_bitcoin_SOURCES = accounting_tests.cpp alert_tests.cpp \
   netbase_tests.cpp pmt_tests.cpp rpc_tests.cpp script_P2SH_tests.cpp \
   script_tests.cpp serialize_tests.cpp sigopcount_tests.cpp test_bitcoin.cpp \
   transaction_tests.cpp uint160_tests.cpp uint256_tests.cpp util_tests.cpp \
-  wallet_tests.cpp $(JSON_TEST_FILES) $(RAW_TEST_FILES)
+  wallet_tests.cpp sighash_tests.cpp $(JSON_TEST_FILES) $(RAW_TEST_FILES)
 
 nodist_test_bitcoin_SOURCES = $(BUILT_SOURCES)
 

src/test/multisig_tests.cpp

@@ -19,7 +19,7 @@ using namespace boost::assign;
 
 typedef vector<unsigned char> valtype;
 
-extern uint256 SignatureHash(CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
+extern uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
 
 BOOST_AUTO_TEST_SUITE(multisig_tests)
 

src/test/script_tests.cpp

@@ -21,7 +21,7 @@ using namespace std;
 using namespace json_spirit;
 using namespace boost::algorithm;
 
-extern uint256 SignatureHash(CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
+extern uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
 
 static const unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC;
 

src/test/sighash_tests.cpp

@@ -0,0 +1,120 @@
+#include <boost/test/unit_test.hpp>
+
+#include "main.h"
+#include "util.h"
+
+extern uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType);
+
+// Old script.cpp SignatureHash function
+uint256 static SignatureHashOld(CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType)
+{
+    if (nIn >= txTo.vin.size())
+    {
+        printf("ERROR: SignatureHash() : nIn=%d out of range\n", nIn);
+        return 1;
+    }
+    CTransaction txTmp(txTo);
+
+    // In case concatenating two scripts ends up with two codeseparators,
+    // or an extra one at the end, this prevents all those possible incompatibilities.
+    scriptCode.FindAndDelete(CScript(OP_CODESEPARATOR));
+
+    // Blank out other inputs' signatures
+    for (unsigned int i = 0; i < txTmp.vin.size(); i++)
+        txTmp.vin[i].scriptSig = CScript();
+    txTmp.vin[nIn].scriptSig = scriptCode;
+
+    // Blank out some of the outputs
+    if ((nHashType & 0x1f) == SIGHASH_NONE)
+    {
+        // Wildcard payee
+        txTmp.vout.clear();
+
+        // Let the others update at will
+        for (unsigned int i = 0; i < txTmp.vin.size(); i++)
+            if (i != nIn)
+                txTmp.vin[i].nSequence = 0;
+    }
+    else if ((nHashType & 0x1f) == SIGHASH_SINGLE)
+    {
+        // Only lock-in the txout payee at same index as txin
+        unsigned int nOut = nIn;
+        if (nOut >= txTmp.vout.size())
+        {
+            printf("ERROR: SignatureHash() : nOut=%d out of range\n", nOut);
+            return 1;
+        }
+        txTmp.vout.resize(nOut+1);
+        for (unsigned int i = 0; i < nOut; i++)
+            txTmp.vout[i].SetNull();
+
+        // Let the others update at will
+        for (unsigned int i = 0; i < txTmp.vin.size(); i++)
+            if (i != nIn)
+                txTmp.vin[i].nSequence = 0;
+    }
+
+    // Blank out other inputs completely, not recommended for open transactions
+    if (nHashType & SIGHASH_ANYONECANPAY)
+    {
+        txTmp.vin[0] = txTmp.vin[nIn];
+        txTmp.vin.resize(1);
+    }
+
+    // Serialize and hash
+    CHashWriter ss(SER_GETHASH, 0);
+    ss << txTmp << nHashType;
+    return ss.GetHash();
+}
+
+void static RandomScript(CScript &script) {
+    static const opcodetype oplist[] = {OP_FALSE, OP_1, OP_2, OP_3, OP_CHECKSIG, OP_IF, OP_VERIF, OP_RETURN, OP_CODESEPARATOR};
+    script = CScript();
+    int ops = (insecure_rand() % 10);
+    for (int i=0; i<ops; i++)
+        script << oplist[insecure_rand() % (sizeof(oplist)/sizeof(oplist[0]))];
+}
+
+void static RandomTransaction(CTransaction &tx, bool fSingle) {
+    tx.nVersion = insecure_rand();
+    tx.vin.clear();
+    tx.vout.clear();
+    tx.nLockTime = (insecure_rand() % 2) ? insecure_rand() : 0;
+    int ins = (insecure_rand() % 4) + 1;
+    int outs = fSingle ? ins : (insecure_rand() % 4) + 1;
+    for (int in = 0; in < ins; in++) {
+        tx.vin.push_back(CTxIn());
+        CTxIn &txin = tx.vin.back();
+        txin.prevout.hash = GetRandHash();
+        txin.prevout.n = insecure_rand() % 4;
+        RandomScript(txin.scriptSig);
+        txin.nSequence = (insecure_rand() % 2) ? insecure_rand() : (unsigned int)-1;
+    }
+    for (int out = 0; out < outs; out++) {
+        tx.vout.push_back(CTxOut());
+        CTxOut &txout = tx.vout.back();
+        txout.nValue = insecure_rand() % 100000000;
+        RandomScript(txout.scriptPubKey);
+    }
+}
+
+BOOST_AUTO_TEST_SUITE(sighash_tests)
+
+BOOST_AUTO_TEST_CASE(sighash_test)
+{
+    seed_insecure_rand(false);
+
+    for (int i=0; i<50000; i++) {
+        int nHashType = insecure_rand();
+        CTransaction txTo;
+        RandomTransaction(txTo, (nHashType & 0x1f) == SIGHASH_SINGLE);
+        CScript scriptCode;
+        RandomScript(scriptCode);
+        int nIn = insecure_rand() % txTo.vin.size();
+        BOOST_CHECK(SignatureHash(scriptCode, txTo, nIn, nHashType) ==
+        SignatureHashOld(scriptCode, txTo, nIn, nHashType));
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+