From f5857e5cb5fb03bee9c05d1dd6ba2621cac49179 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 4 May 2013 19:32:33 +0200 Subject: [PATCH] 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 | 146 +++++++++++++++++++++++++----------- src/test/Makefile.am | 2 +- src/test/multisig_tests.cpp | 2 +- src/test/script_tests.cpp | 2 +- src/test/sighash_tests.cpp | 120 +++++++++++++++++++++++++++++ 5 files changed, 224 insertions(+), 48 deletions(-) create mode 100644 src/test/sighash_tests.cpp diff --git a/src/script.cpp b/src/script.cpp index 0fe295354..ad2d35d94 100644 --- a/src/script.cpp +++ b/src/script.cpp @@ -971,62 +971,118 @@ bool EvalScript(vector >& 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 + 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 + 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 + 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 + 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); diff --git a/src/test/Makefile.am b/src/test/Makefile.am index d193b729f..bedc5d0c2 100644 --- a/src/test/Makefile.am +++ b/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) diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 9ef932b5b..7f6f141c6 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -19,7 +19,7 @@ using namespace boost::assign; typedef vector 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) diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index dfa5529b8..32be91441 100644 --- a/src/test/script_tests.cpp +++ b/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; diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp new file mode 100644 index 000000000..f098d4618 --- /dev/null +++ b/src/test/sighash_tests.cpp @@ -0,0 +1,120 @@ +#include + +#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