From daf044a1eb7512dd69acad6b21527a07387c8ac4 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 24 Apr 2020 21:09:59 -0300 Subject: [PATCH 01/10] Reduce unnecessary hashing in signrawtransaction Coming from btc@bd0f41387783ee91623d7fac15e89e57db37df82 --- src/rpc/rawtransaction.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index e20e0efd112d..f005f9e959e1 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -734,6 +734,9 @@ UniValue signrawtransaction(const JSONRPCRequest& request) // Script verification errors UniValue vErrors(UniValue::VARR); + // Use CTransaction for the constant parts of the + // transaction to avoid rehashing. + const CTransaction txConst(mergedTx); // Sign what we can: for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { CTxIn& txin = mergedTx.vin[i]; @@ -767,10 +770,10 @@ UniValue signrawtransaction(const JSONRPCRequest& request) // ... and merge in other signatures: for (const CMutableTransaction& txv : txVariants) { - txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, txin.scriptSig, txv.vin[i].scriptSig); + txin.scriptSig = CombineSignatures(prevPubKey, txConst, i, txin.scriptSig, txv.vin[i].scriptSig); } ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i), &serror)) { + if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i), &serror)) { TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); } } From dccc3c632e10fe6f346dd8a4011bdf568954527c Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 25 Apr 2020 15:36:59 -0300 Subject: [PATCH 02/10] Refactor script validation to observe amounts This is a preparation for BIP143 support. -- Edited for PIVX merge -- Coming from btc@0ef1dd3e11dd573b6e443852ef0c72e34093ac68 --- src/kernel.cpp | 2 +- src/main.cpp | 2 +- src/main.h | 5 ++++- src/pivx-tx.cpp | 3 ++- src/rpc/rawtransaction.cpp | 9 +++++---- src/script/interpreter.h | 4 ++-- src/script/sigcache.h | 2 +- src/script/sign.cpp | 5 +++-- src/test/multisig_tests.cpp | 17 +++++++++-------- src/test/script_P2SH_tests.cpp | 2 +- src/test/script_tests.cpp | 29 +++++++++++++++-------------- src/test/transaction_tests.cpp | 6 ++++-- 12 files changed, 48 insertions(+), 38 deletions(-) diff --git a/src/kernel.cpp b/src/kernel.cpp index 2feb26d4e3ce..5501578dc4b5 100644 --- a/src/kernel.cpp +++ b/src/kernel.cpp @@ -185,7 +185,7 @@ bool CheckProofOfStake(const CBlock& block, std::string& strError, const CBlockI const CTxIn& txin = tx.vin[0]; ScriptError serror; if (!VerifyScript(txin.scriptSig, stakePrevout.scriptPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, - TransactionSignatureChecker(&tx, 0), &serror)) { + TransactionSignatureChecker(&tx, 0, stakePrevout.nValue), &serror)) { strError = strprintf("signature fails: %s", serror ? ScriptErrorString(serror) : ""); return false; } diff --git a/src/main.cpp b/src/main.cpp index ccd159229c6a..12ccecfcc6fe 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1735,7 +1735,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache &inputs, int nHeight) bool CScriptCheck::operator()() { const CScript& scriptSig = ptxTo->vin[nIn].scriptSig; - return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, cacheStore), &error); + return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore), &error); } std::map mapInvalidOutPoints; diff --git a/src/main.h b/src/main.h index bf1a665d229f..02eb4c2cea95 100644 --- a/src/main.h +++ b/src/main.h @@ -308,6 +308,7 @@ class CScriptCheck { private: CScript scriptPubKey; + CAmount amount; const CTransaction* ptxTo; unsigned int nIn; unsigned int nFlags; @@ -315,8 +316,9 @@ class CScriptCheck ScriptError error; public: - CScriptCheck() : ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} + CScriptCheck() : amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), + amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) {} bool operator()(); @@ -325,6 +327,7 @@ class CScriptCheck { scriptPubKey.swap(check.scriptPubKey); std::swap(ptxTo, check.ptxTo); + std::swap(amount, check.amount); std::swap(nIn, check.nIn); std::swap(nFlags, check.nFlags); std::swap(cacheStore, check.cacheStore); diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index 08e6e5196371..d5e57af57737 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -421,6 +421,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) continue; } const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey; + const CAmount& amount = coins->vout[txin.prevout.n].nValue; txin.scriptSig.clear(); // Only sign SIGHASH_SINGLE if there's a corresponding output: @@ -431,7 +432,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) for (const CTransaction& txv : txVariants) { txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, txin.scriptSig, txv.vin[i].scriptSig); } - if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i))) + if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) fComplete = false; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index f005f9e959e1..0ab3c2acbb7a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -616,7 +616,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) CMutableTransaction mergedTx(txVariants[0]); // Fetch previous transactions (inputs): - std::map mapPrevOut; + std::map> mapPrevOut; // todo: check why do we have this for regtest.. if (Params().IsRegTestNet()) { for (const CTxIn &txbase : mergedTx.vin) { @@ -624,7 +624,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) uint256 hashBlock; if (GetTransaction(txbase.prevout.hash, tempTx, hashBlock, true)) { // Copy results into mapPrevOut: - mapPrevOut[txbase.prevout] = tempTx.vout[txbase.prevout.n].scriptPubKey; + mapPrevOut[txbase.prevout] = std::make_pair(tempTx.vout[txbase.prevout.n].scriptPubKey, tempTx.vout[txbase.prevout.n].nValue); } } } @@ -753,7 +753,8 @@ UniValue signrawtransaction(const JSONRPCRequest& request) continue; } } - const CScript& prevPubKey = (Params().IsRegTestNet() && mapPrevOut.count(txin.prevout) != 0 ? mapPrevOut[txin.prevout] : coins->vout[txin.prevout.n].scriptPubKey); + const CScript& prevPubKey = (Params().IsRegTestNet() && mapPrevOut.count(txin.prevout) != 0 ? mapPrevOut[txin.prevout].first : coins->vout[txin.prevout.n].scriptPubKey); + const CAmount& amount = (Params().IsRegTestNet() && mapPrevOut.count(txin.prevout) != 0 ? mapPrevOut[txin.prevout].second : coins->vout[txin.prevout.n].nValue); txin.scriptSig.clear(); @@ -773,7 +774,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) txin.scriptSig = CombineSignatures(prevPubKey, txConst, i, txin.scriptSig, txv.vin[i].scriptSig); } ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i), &serror)) { + if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); } } diff --git a/src/script/interpreter.h b/src/script/interpreter.h index c504ae76be72..3868a63665ff 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -119,7 +119,7 @@ class TransactionSignatureChecker : public BaseSignatureChecker virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn) : txTo(txToIn), nIn(nInIn) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : txTo(txToIn), nIn(nInIn) {} bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode) const override; bool CheckLockTime(const CScriptNum& nLockTime) const override; bool CheckColdStake(const CScript& script) const override { @@ -133,7 +133,7 @@ class MutableTransactionSignatureChecker : public TransactionSignatureChecker const CTransaction txTo; public: - MutableTransactionSignatureChecker(const CMutableTransaction* txToIn, unsigned int nInIn) : TransactionSignatureChecker(&txTo, nInIn), txTo(*txToIn) {} + MutableTransactionSignatureChecker(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : TransactionSignatureChecker(&txTo, nInIn, amount), txTo(*txToIn) {} }; bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = NULL); diff --git a/src/script/sigcache.h b/src/script/sigcache.h index d0d9d07059d9..f01c198954e4 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -46,7 +46,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, bool storeIn=true) : TransactionSignatureChecker(txToIn, nInIn), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn=true) : TransactionSignatureChecker(txToIn, nInIn, amount), store(storeIn) {} bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; }; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 7e3eb74e989d..be7d55bd0524 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -16,7 +16,8 @@ typedef std::vector valtype; -TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), checker(txTo, nIn) {} +static const CAmount amountZero = 0; +TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), checker(txTo, nIn, amountZero) {} bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& address, const CScript& scriptCode) const { @@ -278,7 +279,7 @@ static CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatur CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CScript& scriptSig1, const CScript& scriptSig2) { - TransactionSignatureChecker checker(&txTo, nIn); + TransactionSignatureChecker checker(&txTo, nIn, amountZero); return CombineSignatures(scriptPubKey, checker, scriptSig1, scriptSig2); } diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 20e7896c9b94..171bc327e310 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -47,6 +47,7 @@ BOOST_AUTO_TEST_CASE(multisig_verify) ScriptError err; CKey key[4]; + CAmount amount = 0; for (int i = 0; i < 4; i++) key[i].MakeNewKey(true); @@ -83,7 +84,7 @@ BOOST_AUTO_TEST_CASE(multisig_verify) keys.push_back(key[0]); keys.push_back(key[1]); s = sign_multisig(a_and_b, keys, txTo[0], 0); - BOOST_CHECK(VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0), &err)); + BOOST_CHECK(VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); for (int i = 0; i < 4; i++) @@ -91,14 +92,14 @@ BOOST_AUTO_TEST_CASE(multisig_verify) keys.clear(); keys.push_back(key[i]); s = sign_multisig(a_and_b, keys, txTo[0], 0); - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0), &err), strprintf("a&b 1: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 1: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err)); keys.clear(); keys.push_back(key[1]); keys.push_back(key[i]); s = sign_multisig(a_and_b, keys, txTo[0], 0); - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0), &err), strprintf("a&b 2: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 2: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } @@ -110,18 +111,18 @@ BOOST_AUTO_TEST_CASE(multisig_verify) s = sign_multisig(a_or_b, keys, txTo[1], 0); if (i == 0 || i == 1) { - BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0), &err), strprintf("a|b: %d", i)); + BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } else { - BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0), &err), strprintf("a|b: %d", i)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } } s.clear(); s << OP_0 << OP_1; - BOOST_CHECK(!VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0), &err)); + BOOST_CHECK(!VerifyScript(s, a_or_b, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_SIG_DER, ScriptErrorString(err)); @@ -134,12 +135,12 @@ BOOST_AUTO_TEST_CASE(multisig_verify) s = sign_multisig(escrow, keys, txTo[2], 0); if (i < j && i < 3 && j < 3) { - BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, flags, MutableTransactionSignatureChecker(&txTo[2], 0), &err), strprintf("escrow 1: %d %d", i, j)); + BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 1: %d %d", i, j)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } else { - BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, flags, MutableTransactionSignatureChecker(&txTo[2], 0), &err), strprintf("escrow 2: %d %d", i, j)); + BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 2: %d %d", i, j)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } } diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 50e3cd8ebee3..ec0f7d3d53d0 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -46,7 +46,7 @@ Verify(const CScript& scriptSig, const CScript& scriptPubKey, bool fStrict, Scri txTo.vin[0].scriptSig = scriptSig; txTo.vout[0].nValue = 1; - return VerifyScript(scriptSig, scriptPubKey, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0), &err); + return VerifyScript(scriptSig, scriptPubKey, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0, txFrom.vout[0].nValue), &err); } BOOST_FIXTURE_TEST_SUITE(script_P2SH_tests, TestingSetup) diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 2a67fc925d51..f5dac3facbb6 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -96,7 +96,8 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, int flags, bo ScriptError err; CMutableTransaction tx = BuildSpendingTransaction(scriptSig, BuildCreditingTransaction(scriptPubKey)); CMutableTransaction tx2 = tx; - BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0), &err) == expect, message); + static const CAmount amountZero = 0; + BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0, amountZero), &err) == expect, message); BOOST_CHECK_MESSAGE(expect == (err == SCRIPT_ERR_OK), std::string(ScriptErrorString(err)) + ": " + message); #if defined(HAVE_CONSENSUS_LIB) CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); @@ -742,18 +743,18 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12) CMutableTransaction txTo12 = BuildSpendingTransaction(CScript(), txFrom12); CScript goodsig1 = sign_multisig(scriptPubKey12, key1, txTo12); - BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0), &err)); + BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); txTo12.vout[0].nValue = 2; - BOOST_CHECK(!VerifyScript(goodsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0), &err)); + BOOST_CHECK(!VerifyScript(goodsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); CScript goodsig2 = sign_multisig(scriptPubKey12, key2, txTo12); - BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0), &err)); + BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); CScript badsig1 = sign_multisig(scriptPubKey12, key3, txTo12); - BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey12, flags, MutableTransactionSignatureChecker(&txTo12, 0, txFrom12.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); } @@ -775,54 +776,54 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23) std::vector keys; keys.push_back(key1); keys.push_back(key2); CScript goodsig1 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(VerifyScript(goodsig1, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key1); keys.push_back(key3); CScript goodsig2 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(VerifyScript(goodsig2, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key3); CScript goodsig3 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(VerifyScript(goodsig3, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(VerifyScript(goodsig3, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key2); // Can't re-use sig CScript badsig1 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig1, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key2); keys.push_back(key1); // sigs must be in correct order CScript badsig2 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig2, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig2, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key3); keys.push_back(key2); // sigs must be in correct order CScript badsig3 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig3, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig3, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key4); keys.push_back(key2); // sigs must match pubkeys CScript badsig4 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig4, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig4, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); keys.push_back(key1); keys.push_back(key4); // sigs must match pubkeys CScript badsig5 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig5, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig5, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err)); keys.clear(); // Must have signatures CScript badsig6 = sign_multisig(scriptPubKey23, keys, txTo23); - BOOST_CHECK(!VerifyScript(badsig6, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0), &err)); + BOOST_CHECK(!VerifyScript(badsig6, scriptPubKey23, flags, MutableTransactionSignatureChecker(&txTo23, 0, txFrom23.vout[0].nValue), &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err)); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 1fcb6dba24a3..1c2d0cc953fc 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -148,9 +148,10 @@ BOOST_AUTO_TEST_CASE(tx_valid) break; } + CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i), &err), + verify_flags, TransactionSignatureChecker(&tx, i, amount), &err), strTest); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -222,9 +223,10 @@ BOOST_AUTO_TEST_CASE(tx_invalid) break; } + CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i), &err); + verify_flags, TransactionSignatureChecker(&tx, i, amount), &err); } BOOST_CHECK_MESSAGE(!fValid, strTest); BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err)); From d2dd54732466c2a264bc454bd217235d300a302b Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 26 Apr 2020 21:56:24 -0300 Subject: [PATCH 03/10] BIP143: Verification logic. --- src/pivx-tx.cpp | 2 +- src/policy/policy.cpp | 2 +- src/rpc/rawtransaction.cpp | 11 +++++- src/script/interpreter.cpp | 75 ++++++++++++++++++++++++++++++++----- src/script/interpreter.h | 19 +++++++--- src/script/sign.cpp | 26 ++++++++----- src/script/sign.h | 5 ++- src/test/multisig_tests.cpp | 2 +- src/test/script_tests.cpp | 57 ++++++++++++++-------------- src/test/sighash_tests.cpp | 4 +- 10 files changed, 141 insertions(+), 62 deletions(-) diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index d5e57af57737..e4db2c135def 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -430,7 +430,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) // ... and merge in other signatures: for (const CTransaction& txv : txVariants) { - txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, txin.scriptSig, txv.vin[i].scriptSig); + txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, amount, txin.scriptSig, txv.vin[i].scriptSig); } if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) fComplete = false; diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 8a253eedf3c7..05ab68e0b8aa 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -172,7 +172,7 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) // IsStandard() will have already returned false // and this method isn't called. std::vector > stack; - if (!EvalScript(stack, tx.vin[i].scriptSig, false, BaseSignatureChecker())) + if (!EvalScript(stack, tx.vin[i].scriptSig, false, BaseSignatureChecker(), SIGVERSION_BASE)) return false; if (whichType == TX_SCRIPTHASH) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0ab3c2acbb7a..66f6689f764d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -721,7 +721,14 @@ UniValue signrawtransaction(const JSONRPCRequest& request) int nHashType = SIGHASH_ALL; if (request.params.size() > 3 && !request.params[3].isNull()) { static std::map mapSigHashValues = - boost::assign::map_list_of(std::string("ALL"), int(SIGHASH_ALL))(std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL | SIGHASH_ANYONECANPAY))(std::string("NONE"), int(SIGHASH_NONE))(std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE | SIGHASH_ANYONECANPAY))(std::string("SINGLE"), int(SIGHASH_SINGLE))(std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE | SIGHASH_ANYONECANPAY)); + boost::assign::map_list_of + (std::string("ALL"), int(SIGHASH_ALL)) + (std::string("ALL|ANYONECANPAY"), int(SIGHASH_ALL|SIGHASH_ANYONECANPAY)) + (std::string("NONE"), int(SIGHASH_NONE)) + (std::string("NONE|ANYONECANPAY"), int(SIGHASH_NONE|SIGHASH_ANYONECANPAY)) + (std::string("SINGLE"), int(SIGHASH_SINGLE)) + (std::string("SINGLE|ANYONECANPAY"), int(SIGHASH_SINGLE|SIGHASH_ANYONECANPAY)) + ; std::string strHashType = request.params[3].get_str(); if (mapSigHashValues.count(strHashType)) nHashType = mapSigHashValues[strHashType]; @@ -771,7 +778,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) // ... and merge in other signatures: for (const CMutableTransaction& txv : txVariants) { - txin.scriptSig = CombineSignatures(prevPubKey, txConst, i, txin.scriptSig, txv.vin[i].scriptSig); + txin.scriptSig = CombineSignatures(prevPubKey, txConst, i, amount, txin.scriptSig, txv.vin[i].scriptSig); } ScriptError serror = SCRIPT_ERR_OK; if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 3cd54cc09e78..4523fdf0c0a6 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -235,7 +235,7 @@ bool static CheckMinimalPush(const valtype& data, opcodetype opcode) { return true; } -bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror) +bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* serror) { static const CScriptNum bnZero(0); static const CScriptNum bnOne(1); @@ -847,7 +847,7 @@ bool EvalScript(std::vector >& stack, const CScript& //serror is set return false; } - bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode); + bool fSuccess = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion); popstack(stack); popstack(stack); @@ -915,7 +915,7 @@ bool EvalScript(std::vector >& stack, const CScript& } // Check signature - bool fOk = checker.CheckSig(vchSig, vchPubKey, scriptCode); + bool fOk = checker.CheckSig(vchSig, vchPubKey, scriptCode, sigversion); if (fOk) { isig++; @@ -1087,13 +1087,70 @@ class CTransactionSignatureSerializer { } // anon namespace -uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType) +uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion) { if (nIn >= txTo.vin.size()) { // nIn out of range return UINT256_ONE; } + if (sigversion == SIGVERSION_WITNESS_V0) { + + uint256 hashPrevouts; + uint256 hashSequence; + uint256 hashOutputs; + + if (!(nHashType & SIGHASH_ANYONECANPAY)) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].prevout; + } + hashPrevouts = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + } + + if (!(nHashType & SIGHASH_ANYONECANPAY) && (nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].nSequence; + } + hashSequence = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + } + + if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vout.size(); n++) { + ss << txTo.vout[n]; + } + hashOutputs = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + } else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) { + CHashWriter ss(SER_GETHASH, 0); + ss << txTo.vout[nIn]; + hashOutputs = ss.GetHash(); + } + + CHashWriter ss(SER_GETHASH, 0); + // Version + ss << txTo.nVersion; + // Input prevouts/nSequence (none/all, depending on flags) + ss << hashPrevouts; + ss << hashSequence; + // The input being signed (replacing the scriptSig with scriptCode + amount) + // The prevout may already be contained in hashPrevout, and the nSequence + // may already be contained in hashSequence. + ss << txTo.vin[nIn].prevout; + ss << static_cast(scriptCode); + ss << amount; + ss << txTo.vin[nIn].nSequence; + // Outputs (none/one/all, depending on flags) + ss << hashOutputs; + // Locktime + ss << txTo.nLockTime; + // Sighash type + ss << nHashType; + + return ss.GetHash(); + } + // Check for invalid use of SIGHASH_SINGLE if ((nHashType & 0x1f) == SIGHASH_SINGLE) { if (nIn >= txTo.vout.size()) { @@ -1116,7 +1173,7 @@ bool TransactionSignatureChecker::VerifySignature(const std::vector& vchSigIn, const std::vector& vchPubKey, const CScript& scriptCode) const +bool TransactionSignatureChecker::CheckSig(const std::vector& vchSigIn, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const { CPubKey pubkey(vchPubKey); if (!pubkey.IsValid()) @@ -1129,7 +1186,7 @@ bool TransactionSignatureChecker::CheckSig(const std::vector& vch int nHashType = vchSig.back(); vchSig.pop_back(); - uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion); if (!VerifySignature(vchSig, pubkey, sighash)) return false; @@ -1183,12 +1240,12 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne } std::vector > stack, stackCopy; - if (!EvalScript(stack, scriptSig, flags, checker, serror)) + if (!EvalScript(stack, scriptSig, flags, checker, SIGVERSION_BASE, serror)) // serror is set return false; if (flags & SCRIPT_VERIFY_P2SH) stackCopy = stack; - if (!EvalScript(stack, scriptPubKey, flags, checker, serror)) + if (!EvalScript(stack, scriptPubKey, flags, checker, SIGVERSION_BASE, serror)) // serror is set return false; if (stack.empty()) @@ -1213,7 +1270,7 @@ bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigne CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end()); popstack(stackCopy); - if (!EvalScript(stackCopy, pubKey2, flags, checker, serror)) + if (!EvalScript(stackCopy, pubKey2, flags, checker, SIGVERSION_BASE, serror)) // serror is set return false; if (stackCopy.empty()) diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 3868a63665ff..817b2459c246 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -86,12 +86,18 @@ enum bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); -uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType); +enum SigVersion +{ + SIGVERSION_BASE = 0, + SIGVERSION_WITNESS_V0 = 1, +}; + +uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion); class BaseSignatureChecker { public: - virtual bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode) const + virtual bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const { return false; } @@ -114,13 +120,14 @@ class TransactionSignatureChecker : public BaseSignatureChecker private: const CTransaction* txTo; unsigned int nIn; + const CAmount amount; protected: virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : txTo(txToIn), nIn(nInIn) {} - bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode) const override; + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn) {} + bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override ; bool CheckLockTime(const CScriptNum& nLockTime) const override; bool CheckColdStake(const CScript& script) const override { return txTo->CheckColdStake(script); @@ -136,7 +143,7 @@ class MutableTransactionSignatureChecker : public TransactionSignatureChecker MutableTransactionSignatureChecker(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount) : TransactionSignatureChecker(&txTo, nInIn, amount), txTo(*txToIn) {} }; -bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = NULL); -bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* error = NULL); +bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptError* error = NULL); +bool VerifyScript(const CScript& scriptSig, const CScript& scriptPubKey, unsigned int flags, const BaseSignatureChecker& checker, ScriptError* serror = NULL); #endif // BITCOIN_SCRIPT_INTERPRETER_H diff --git a/src/script/sign.cpp b/src/script/sign.cpp index be7d55bd0524..9f20111e5fd9 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -16,8 +16,7 @@ typedef std::vector valtype; -static const CAmount amountZero = 0; -TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), checker(txTo, nIn, amountZero) {} +TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {} bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& address, const CScript& scriptCode) const { @@ -25,7 +24,13 @@ bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, if (!keystore->GetKey(address, key)) return false; - uint256 hash = SignatureHash(scriptCode, *txTo, nIn, nHashType); + uint256 hash; + try { + hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, SIGVERSION_BASE); + } catch (std::logic_error ex) { + return false; + } + if (!key.Sign(hash, vchSig)) return false; vchSig.push_back((unsigned char)nHashType); @@ -151,7 +156,8 @@ bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutabl CTxIn& txin = txTo.vin[nIn]; CTransaction txToConst(txTo); - TransactionSignatureCreator creator(&keystore, &txToConst, nIn, nHashType); + static CAmount amount = 0; // receive amount + TransactionSignatureCreator creator(&keystore, &txToConst, nIn, amount, nHashType); return ProduceSignature(creator, fromPubKey, txin.scriptSig, fColdStake); } @@ -204,7 +210,7 @@ static CScript CombineMultisig(const CScript& scriptPubKey, const BaseSignatureC if (sigs.count(pubkey)) continue; // Already got a sig for this pubkey - if (checker.CheckSig(sig, pubkey, scriptPubKey)) + if (checker.CheckSig(sig, pubkey, scriptPubKey, SIGVERSION_BASE)) { sigs[pubkey] = sig; break; @@ -276,10 +282,10 @@ static CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatur return CScript(); } -CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, +CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CAmount& amount, const CScript& scriptSig1, const CScript& scriptSig2) { - TransactionSignatureChecker checker(&txTo, nIn, amountZero); + TransactionSignatureChecker checker(&txTo, nIn, amount); return CombineSignatures(scriptPubKey, checker, scriptSig1, scriptSig2); } @@ -291,9 +297,9 @@ CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecke Solver(scriptPubKey, txType, vSolutions); std::vector stack1; - EvalScript(stack1, scriptSig1, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker()); + EvalScript(stack1, scriptSig1, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SIGVERSION_BASE); std::vector stack2; - EvalScript(stack2, scriptSig2, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker()); + EvalScript(stack2, scriptSig2, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SIGVERSION_BASE); return CombineSignatures(scriptPubKey, checker, txType, vSolutions, stack1, stack2); } @@ -305,7 +311,7 @@ class DummySignatureChecker : public BaseSignatureChecker public: DummySignatureChecker() {} - bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode) const + bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const { return true; } diff --git a/src/script/sign.h b/src/script/sign.h index f11f9be89c79..e53d2f38bafa 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -36,10 +36,11 @@ class TransactionSignatureCreator : public BaseSignatureCreator { const CTransaction* txTo; unsigned int nIn; int nHashType; + CAmount amount; const TransactionSignatureChecker checker; public: - TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, int nHashTypeIn=SIGHASH_ALL); + TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL); const BaseSignatureChecker& Checker() const { return checker; } bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode) const; }; @@ -63,6 +64,6 @@ bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CMutab CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const CScript& scriptSig1, const CScript& scriptSig2); /** Combine two script signatures on transactions. */ -CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CScript& scriptSig1, const CScript& scriptSig2); +CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CAmount& amount, const CScript& scriptSig1, const CScript& scriptSig2); #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 171bc327e310..2bab76081f1b 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -27,7 +27,7 @@ BOOST_FIXTURE_TEST_SUITE(multisig_tests, TestingSetup) CScript sign_multisig(CScript scriptPubKey, std::vector keys, CTransaction transaction, int whichIn) { - uint256 hash = SignatureHash(scriptPubKey, transaction, whichIn, SIGHASH_ALL); + uint256 hash = SignatureHash(scriptPubKey, transaction, whichIn, SIGHASH_ALL, 0, SIGVERSION_BASE); CScript result; result << OP_0; // CHECKMULTISIG bug workaround diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index f5dac3facbb6..8cd35d1f4ffc 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -243,7 +243,7 @@ class TestBuilder TestBuilder& PushSig(const CKey& key, int nHashType = SIGHASH_ALL, unsigned int lenR = 32, unsigned int lenS = 32) { - uint256 hash = SignatureHash(scriptPubKey, spendTx, 0, nHashType); + uint256 hash = SignatureHash(scriptPubKey, spendTx, 0, nHashType, 0, SIGVERSION_BASE); std::vector vchSig, r, s; uint32_t iter = 0; do { @@ -677,21 +677,21 @@ BOOST_AUTO_TEST_CASE(script_PushData) ScriptError err; std::vector > directStack; - BOOST_CHECK(EvalScript(directStack, CScript(&direct[0], &direct[sizeof(direct)]), true, BaseSignatureChecker(), &err)); + BOOST_CHECK(EvalScript(directStack, CScript(&direct[0], &direct[sizeof(direct)]), true, BaseSignatureChecker(), SIGVERSION_BASE, &err)); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); std::vector > pushdata1Stack; - BOOST_CHECK(EvalScript(pushdata1Stack, CScript(&pushdata1[0], &pushdata1[sizeof(pushdata1)]), true, BaseSignatureChecker(), &err)); + BOOST_CHECK(EvalScript(pushdata1Stack, CScript(&pushdata1[0], &pushdata1[sizeof(pushdata1)]), true, BaseSignatureChecker(), SIGVERSION_BASE, &err)); BOOST_CHECK(pushdata1Stack == directStack); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); std::vector > pushdata2Stack; - BOOST_CHECK(EvalScript(pushdata2Stack, CScript(&pushdata2[0], &pushdata2[sizeof(pushdata2)]), true, BaseSignatureChecker(), &err)); + BOOST_CHECK(EvalScript(pushdata2Stack, CScript(&pushdata2[0], &pushdata2[sizeof(pushdata2)]), true, BaseSignatureChecker(), SIGVERSION_BASE, &err)); BOOST_CHECK(pushdata2Stack == directStack); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); std::vector > pushdata4Stack; - BOOST_CHECK(EvalScript(pushdata4Stack, CScript(&pushdata4[0], &pushdata4[sizeof(pushdata4)]), true, BaseSignatureChecker(), &err)); + BOOST_CHECK(EvalScript(pushdata4Stack, CScript(&pushdata4[0], &pushdata4[sizeof(pushdata4)]), true, BaseSignatureChecker(), SIGVERSION_BASE, &err)); BOOST_CHECK(pushdata4Stack == directStack); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -699,7 +699,7 @@ BOOST_AUTO_TEST_CASE(script_PushData) CScript sign_multisig(CScript scriptPubKey, std::vector keys, CTransaction transaction) { - uint256 hash = SignatureHash(scriptPubKey, transaction, 0, SIGHASH_ALL); + uint256 hash = SignatureHash(scriptPubKey, transaction, 0, SIGHASH_ALL, 0, SIGVERSION_BASE); CScript result; // @@ -830,6 +830,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23) BOOST_AUTO_TEST_CASE(script_combineSigs) { // Test the CombineSignatures function + CAmount amount = 0; CBasicKeyStore keystore; std::vector keys; std::vector pubkeys; @@ -848,19 +849,19 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) CScript& scriptSig = txTo.vin[0].scriptSig; CScript empty; - CScript combined = CombineSignatures(scriptPubKey, txTo, 0, empty, empty); + CScript combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, empty); BOOST_CHECK(combined.empty()); // Single signature case: SignSignature(keystore, txFrom, txTo, 0); // changes scriptSig - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSig, empty); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, empty, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); BOOST_CHECK(combined == scriptSig); CScript scriptSigCopy = scriptSig; // Signing again will give a different, valid signature: SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSigCopy, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); BOOST_CHECK(combined == scriptSigCopy || combined == scriptSig); // P2SH, single-signature case: @@ -868,41 +869,41 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) keystore.AddCScript(pkSingle); scriptPubKey = GetScriptForDestination(CScriptID(pkSingle)); SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSig, empty); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, empty, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); BOOST_CHECK(combined == scriptSig); scriptSigCopy = scriptSig; SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSigCopy, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); BOOST_CHECK(combined == scriptSigCopy || combined == scriptSig); // dummy scriptSigCopy with placeholder, should always choose non-placeholder: scriptSigCopy = CScript() << OP_0 << std::vector(pkSingle.begin(), pkSingle.end()); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSigCopy, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSig, scriptSigCopy); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, scriptSigCopy); BOOST_CHECK(combined == scriptSig); // Hardest case: Multisig 2-of-3 scriptPubKey = GetScriptForMultisig(2, pubkeys); keystore.AddCScript(scriptPubKey); SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, scriptSig, empty); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, empty, scriptSig); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); BOOST_CHECK(combined == scriptSig); // A couple of partially-signed versions: std::vector sig1; - uint256 hash1 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_ALL); + uint256 hash1 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_ALL, 0, SIGVERSION_BASE); BOOST_CHECK(keys[0].Sign(hash1, sig1)); sig1.push_back(SIGHASH_ALL); std::vector sig2; - uint256 hash2 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_NONE); + uint256 hash2 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_NONE, 0, SIGVERSION_BASE); BOOST_CHECK(keys[1].Sign(hash2, sig2)); sig2.push_back(SIGHASH_NONE); std::vector sig3; - uint256 hash3 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_SINGLE); + uint256 hash3 = SignatureHash(scriptPubKey, txTo, 0, SIGHASH_SINGLE, 0, SIGVERSION_BASE); BOOST_CHECK(keys[2].Sign(hash3, sig3)); sig3.push_back(SIGHASH_SINGLE); @@ -918,21 +919,21 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) CScript complete13 = CScript() << OP_0 << sig1 << sig3; CScript complete23 = CScript() << OP_0 << sig2 << sig3; - combined = CombineSignatures(scriptPubKey, txTo, 0, partial1a, partial1b); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1a, partial1b); BOOST_CHECK(combined == partial1a); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial1a, partial2a); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1a, partial2a); BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial2a, partial1a); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial2a, partial1a); BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial1b, partial2b); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1b, partial2b); BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial3b, partial1b); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial1b); BOOST_CHECK(combined == complete13); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial2a, partial3a); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial2a, partial3a); BOOST_CHECK(combined == complete23); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial3b, partial2b); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial2b); BOOST_CHECK(combined == complete23); - combined = CombineSignatures(scriptPubKey, txTo, 0, partial3b, partial3a); + combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial3a); BOOST_CHECK(combined == partial3c); } diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 318bd5c9ff45..bee1ed5d13b6 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE(sighash_test) uint256 sh, sho; sho = SignatureHashOld(scriptCode, txTo, nIn, nHashType); - sh = SignatureHash(scriptCode, txTo, nIn, nHashType); + sh = SignatureHash(scriptCode, txTo, nIn, nHashType, 0, SIGVERSION_BASE); #if defined(PRINT_SIGHASH_JSON) CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); ss << txTo; @@ -207,7 +207,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) continue; } - sh = SignatureHash(scriptCode, tx, nIn, nHashType); + sh = SignatureHash(scriptCode, tx, nIn, nHashType, 0, SIGVERSION_BASE); BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest); } } From a5170f0b7180fea75a4bde6ae3f8ccbff1f2d20e Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Apr 2020 16:44:09 -0300 Subject: [PATCH 04/10] BIP143: Signing logic. --- src/pivx-tx.cpp | 35 +++++- src/rpc/rawtransaction.cpp | 16 ++- src/script/sign.cpp | 211 ++++++++++++++++++++------------- src/script/sign.h | 33 ++++-- src/test/DoS_tests.cpp | 4 +- src/test/multisig_tests.cpp | 2 +- src/test/script_P2SH_tests.cpp | 10 +- src/test/script_tests.cpp | 89 +++++++------- src/wallet/wallet.cpp | 20 +++- src/wallet/wallet_ismine.cpp | 4 +- src/wallet/wallet_zerocoin.cpp | 19 ++- 11 files changed, 279 insertions(+), 164 deletions(-) diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index e4db2c135def..27d7d9823f6d 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -315,6 +315,24 @@ uint256 ParseHashUO(std::map& o, std::string strKey) return ParseHashUV(o[strKey], strKey); } +static inline int64_t roundint64(double d) +{ + return (int64_t)(d > 0 ? d + 0.5 : d - 0.5); +} + +static CAmount AmountFromValue(const UniValue& value) +{ + if (!value.isNum() && !value.isStr()) + throw std::runtime_error("Amount is not a number or string"); + double dAmount = value.get_real(); + if (dAmount <= 0.0 || dAmount > 21000000.0) + throw std::runtime_error("Invalid amount"); + CAmount nAmount = roundint64(dAmount * COIN); + if (!Params().GetConsensus().MoneyRange(nAmount)) + throw std::runtime_error("Amount out of range"); + return nAmount; +} + std::vector ParseHexUO(std::map& o, std::string strKey) { if (!o.count(strKey)) { @@ -393,7 +411,10 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) if ((unsigned int)nOut >= coins->vout.size()) coins->vout.resize(nOut + 1); coins->vout[nOut].scriptPubKey = scriptPubKey; - coins->vout[nOut].nValue = 0; // we don't know the actual output value + coins->vout[nOut].nValue = 0; + if (prevOut.exists("amount")) { + coins->vout[nOut].nValue = AmountFromValue(prevOut["amount"]); + } } // if redeemScript given and private keys given, @@ -423,15 +444,21 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey; const CAmount& amount = coins->vout[txin.prevout.n].nValue; - txin.scriptSig.clear(); + SignatureData sigdata; // Only sign SIGHASH_SINGLE if there's a corresponding output: if (!fHashSingle || (i < mergedTx.vout.size())) - SignSignature(keystore, prevPubKey, mergedTx, i, nHashType); + ProduceSignature( + MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType), + prevPubKey, + sigdata, + false // no cold stake + ); // ... and merge in other signatures: for (const CTransaction& txv : txVariants) { - txin.scriptSig = CombineSignatures(prevPubKey, mergedTx, i, amount, txin.scriptSig, txv.vin[i].scriptSig); + sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i)); } + UpdateTransaction(mergedTx, i, sigdata); if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) fComplete = false; } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 66f6689f764d..54df5d8c68a7 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -553,6 +553,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) " \"vout\":n, (numeric, required) The output number\n" " \"scriptPubKey\": \"hex\", (string, required) script key\n" " \"redeemScript\": \"hex\" (string, required for P2SH) redeem script\n" + " \"amount\": value (numeric, required) The amount spent\n" " }\n" " ,...\n" " ]\n" @@ -695,7 +696,10 @@ UniValue signrawtransaction(const JSONRPCRequest& request) if ((unsigned int)nOut >= coins->vout.size()) coins->vout.resize(nOut + 1); coins->vout[nOut].scriptPubKey = scriptPubKey; - coins->vout[nOut].nValue = 0; // we don't know the actual output value + coins->vout[nOut].nValue = 0; + if (prevOut.exists("amount")) { + coins->vout[nOut].nValue = AmountFromValue(find_value(prevOut, "amount")); + } } // if redeemScript given and not using the local wallet (private keys @@ -763,8 +767,6 @@ UniValue signrawtransaction(const JSONRPCRequest& request) const CScript& prevPubKey = (Params().IsRegTestNet() && mapPrevOut.count(txin.prevout) != 0 ? mapPrevOut[txin.prevout].first : coins->vout[txin.prevout.n].scriptPubKey); const CAmount& amount = (Params().IsRegTestNet() && mapPrevOut.count(txin.prevout) != 0 ? mapPrevOut[txin.prevout].second : coins->vout[txin.prevout.n].nValue); - txin.scriptSig.clear(); - // if this is a P2CS script, select which key to use bool fColdStake = false; if (prevPubKey.IsPayToColdStaking()) { @@ -772,14 +774,18 @@ UniValue signrawtransaction(const JSONRPCRequest& request) fColdStake = !bool(IsMine(keystore, prevPubKey) & ISMINE_SPENDABLE_DELEGATED); } + SignatureData sigdata; // Only sign SIGHASH_SINGLE if there's a corresponding output: if (!fHashSingle || (i < mergedTx.vout.size())) - SignSignature(keystore, prevPubKey, mergedTx, i, nHashType, fColdStake); + ProduceSignature(MutableTransactionSignatureCreator(&keystore, &mergedTx, i, amount, nHashType), prevPubKey, sigdata, fColdStake); // ... and merge in other signatures: for (const CMutableTransaction& txv : txVariants) { - txin.scriptSig = CombineSignatures(prevPubKey, txConst, i, amount, txin.scriptSig, txv.vin[i].scriptSig); + sigdata = CombineSignatures(prevPubKey, TransactionSignatureChecker(&txConst, i, amount), sigdata, DataFromTransaction(txv, i)); } + + UpdateTransaction(mergedTx, i, sigdata); + ScriptError serror = SCRIPT_ERR_OK; if (!VerifyScript(txin.scriptSig, prevPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 9f20111e5fd9..f6e19e8852ca 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -18,7 +18,7 @@ typedef std::vector valtype; TransactionSignatureCreator::TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : BaseSignatureCreator(keystoreIn), txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {} -bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& address, const CScript& scriptCode) const +bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const { CKey key; if (!keystore->GetKey(address, key)) @@ -26,7 +26,7 @@ bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, uint256 hash; try { - hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, SIGVERSION_BASE); + hash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion); } catch (std::logic_error ex) { return false; } @@ -37,16 +37,16 @@ bool TransactionSignatureCreator::CreateSig(std::vector& vchSig, return true; } -static bool Sign1(const CKeyID& address, const BaseSignatureCreator& creator, const CScript& scriptCode, CScript& scriptSigRet) +static bool Sign1(const CKeyID& address, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector& ret, SigVersion sigversion) { std::vector vchSig; - if (!creator.CreateSig(vchSig, address, scriptCode)) + if (!creator.CreateSig(vchSig, address, scriptCode, sigversion)) return false; - scriptSigRet << vchSig; + ret.emplace_back(vchSig); return true; } -static bool SignN(const std::vector& multisigdata, const BaseSignatureCreator& creator, const CScript& scriptCode, CScript& scriptSigRet) +static bool SignN(const std::vector& multisigdata, const BaseSignatureCreator& creator, const CScript& scriptCode, std::vector& ret, SigVersion sigversion) { int nSigned = 0; int nRequired = multisigdata.front()[0]; @@ -54,7 +54,7 @@ static bool SignN(const std::vector& multisigdata, const BaseSignatureC { const valtype& pubkey = multisigdata[i]; CKeyID keyID = CPubKey(pubkey).GetID(); - if (Sign1(keyID, creator, scriptCode, scriptSigRet)) + if (Sign1(keyID, creator, scriptCode, ret, sigversion)) ++nSigned; } return nSigned==nRequired; @@ -67,9 +67,11 @@ static bool SignN(const std::vector& multisigdata, const BaseSignatureC * Returns false if scriptPubKey could not be completely satisfied. */ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptPubKey, - CScript& scriptSigRet, txnouttype& whichTypeRet, bool fColdStake = false) + std::vector& ret, txnouttype& whichTypeRet, SigVersion sigversion, bool fColdStake) { - scriptSigRet.clear(); + CScript scriptRet; + uint160 h160; + ret.clear(); std::vector vSolutions; if (!Solver(scriptPubKey, whichTypeRet, vSolutions)) @@ -85,24 +87,28 @@ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptP return false; case TX_PUBKEY: keyID = CPubKey(vSolutions[0]).GetID(); - return Sign1(keyID, creator, scriptPubKey, scriptSigRet); + return Sign1(keyID, creator, scriptPubKey, ret, sigversion); case TX_PUBKEYHASH: keyID = CKeyID(uint160(vSolutions[0])); - if (!Sign1(keyID, creator, scriptPubKey, scriptSigRet)) + if (!Sign1(keyID, creator, scriptPubKey, ret, sigversion)) return false; else { CPubKey vch; creator.KeyStore().GetPubKey(keyID, vch); - scriptSigRet << ToByteVector(vch); + ret.push_back(ToByteVector(vch)); } return true; case TX_SCRIPTHASH: - return creator.KeyStore().GetCScript(uint160(vSolutions[0]), scriptSigRet); + if (creator.KeyStore().GetCScript(uint160(vSolutions[0]), scriptRet)) { + ret.push_back(std::vector(scriptRet.begin(), scriptRet.end())); + return true; + } + return false; case TX_MULTISIG: - scriptSigRet << OP_0; // workaround CHECKMULTISIG bug - return (SignN(vSolutions, creator, scriptPubKey, scriptSigRet)); + ret.push_back(valtype()); // workaround CHECKMULTISIG bug + return (SignN(vSolutions, creator, scriptPubKey, ret, sigversion)); case TX_COLDSTAKE: if (fColdStake) { @@ -112,54 +118,89 @@ static bool SignStep(const BaseSignatureCreator& creator, const CScript& scriptP // sign with the owner key keyID = CKeyID(uint160(vSolutions[1])); } - if (!Sign1(keyID, creator, scriptPubKey, scriptSigRet)) + if (!Sign1(keyID, creator, scriptPubKey, ret, sigversion)) return error("*** %s: failed to sign with the %s key.", __func__, fColdStake ? "cold staker" : "owner"); CPubKey vch; if (!creator.KeyStore().GetPubKey(keyID, vch)) return error("%s : Unable to get public key from keyID", __func__); - scriptSigRet << (fColdStake ? (int)OP_TRUE : OP_FALSE) << ToByteVector(vch); + + valtype oper; + oper.reserve(4); + oper.emplace_back((fColdStake ? (int) OP_TRUE : OP_FALSE)); + ret.emplace_back(oper); + ret.emplace_back(ToByteVector(vch)); return true; } LogPrintf("*** solver no case met \n"); return false; } -bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPubKey, CScript& scriptSig, bool fColdStake) +static CScript PushAll(const std::vector& values) +{ + CScript result; + for (const valtype& v : values) { + if (v.size() == 0) { + result << OP_0; + } else if (v.size() == 1 && v[0] >= 1 && v[0] <= 16) { + result << CScript::EncodeOP_N(v[0]); + } else { + result << v; + } + } + return result; +} + +bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata, bool fColdStake, ScriptError* serror) { + CScript script = fromPubKey; + bool solved = true; + std::vector result; txnouttype whichType; - if (!SignStep(creator, fromPubKey, scriptSig, whichType, fColdStake)) - return false; + solved = SignStep(creator, script, result, whichType, SIGVERSION_BASE, fColdStake); + CScript subscript; - if (whichType == TX_SCRIPTHASH) + if (solved && whichType == TX_SCRIPTHASH) { - // Solver returns the subscript that need to be evaluated; + // Solver returns the subscript that needs to be evaluated; // the final scriptSig is the signatures from that // and then the serialized subscript: - CScript subscript = scriptSig; - - txnouttype subType; - bool fSolved = - SignStep(creator, subscript, scriptSig, subType, fColdStake) && subType != TX_SCRIPTHASH; - // Append serialized subscript whether or not it is completely signed: - scriptSig << valtype(subscript.begin(), subscript.end()); - if (!fSolved) return false; + script = subscript = CScript(result[0].begin(), result[0].end()); + solved = solved && SignStep(creator, script, result, whichType, SIGVERSION_BASE, fColdStake) && whichType != TX_SCRIPTHASH; + result.push_back(std::vector(subscript.begin(), subscript.end())); } + sigdata.scriptSig = PushAll(result); + // Test solution - return VerifyScript(scriptSig, fromPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker()); + return solved && VerifyScript(sigdata.scriptSig, fromPubKey, STANDARD_SCRIPT_VERIFY_FLAGS, creator.Checker(), serror); +} + +SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn) +{ + SignatureData data; + assert(tx.vin.size() > nIn); + data.scriptSig = tx.vin[nIn].scriptSig; + return data; +} + +void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data) +{ + assert(tx.vin.size() > nIn); + tx.vin[nIn].scriptSig = data.scriptSig; } -bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, int nHashType, bool fColdStake) +bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, bool fColdStake) { assert(nIn < txTo.vin.size()); - CTxIn& txin = txTo.vin[nIn]; CTransaction txToConst(txTo); - static CAmount amount = 0; // receive amount TransactionSignatureCreator creator(&keystore, &txToConst, nIn, amount, nHashType); - return ProduceSignature(creator, fromPubKey, txin.scriptSig, fColdStake); + SignatureData sigdata; + bool ret = ProduceSignature(creator, fromPubKey, sigdata, fColdStake); + UpdateTransaction(txTo, nIn, sigdata); + return ret; } bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, bool fColdStake) @@ -169,20 +210,12 @@ bool SignSignature(const CKeyStore &keystore, const CTransaction& txFrom, CMutab assert(txin.prevout.n < txFrom.vout.size()); const CTxOut& txout = txFrom.vout[txin.prevout.n]; - return SignSignature(keystore, txout.scriptPubKey, txTo, nIn, nHashType, fColdStake); -} - -static CScript PushAll(const std::vector& values) -{ - CScript result; - for (const valtype& v : values) - result << v; - return result; + return SignSignature(keystore, txout.scriptPubKey, txTo, nIn, txout.nValue, nHashType, fColdStake); } -static CScript CombineMultisig(const CScript& scriptPubKey, const BaseSignatureChecker& checker, +static std::vector CombineMultisig(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const std::vector& vSolutions, - const std::vector& sigs1, const std::vector& sigs2) + const std::vector& sigs1, const std::vector& sigs2, SigVersion sigversion) { // Combine all the signatures we've got: std::set allsigs; @@ -210,7 +243,7 @@ static CScript CombineMultisig(const CScript& scriptPubKey, const BaseSignatureC if (sigs.count(pubkey)) continue; // Already got a sig for this pubkey - if (checker.CheckSig(sig, pubkey, scriptPubKey, SIGVERSION_BASE)) + if (checker.CheckSig(sig, pubkey, scriptPubKey, sigversion)) { sigs[pubkey] = sig; break; @@ -219,25 +252,45 @@ static CScript CombineMultisig(const CScript& scriptPubKey, const BaseSignatureC } // Now build a merged CScript: unsigned int nSigsHave = 0; - CScript result; result << OP_0; // pop-one-too-many workaround + std::vector result; result.push_back(valtype()); // pop-one-too-many workaround for (unsigned int i = 0; i < nPubKeys && nSigsHave < nSigsRequired; i++) { if (sigs.count(vSolutions[i+1])) { - result << sigs[vSolutions[i+1]]; + result.push_back(sigs[vSolutions[i+1]]); ++nSigsHave; } } // Fill any missing with OP_0: for (unsigned int i = nSigsHave; i < nSigsRequired; i++) - result << OP_0; + result.push_back(valtype()); return result; } -static CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, +namespace +{ +struct Stacks +{ + std::vector script; + + Stacks() {} + explicit Stacks(const std::vector& scriptSigStack_) : script(scriptSigStack_) {} + explicit Stacks(const SignatureData& data) { + EvalScript(script, data.scriptSig, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SIGVERSION_BASE); + } + + SignatureData Output() const { + SignatureData result; + result.scriptSig = PushAll(script); + return result; + } +}; +} + +static Stacks CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const txnouttype txType, const std::vector& vSolutions, - std::vector& sigs1, std::vector& sigs2) + Stacks sigs1, Stacks sigs2, SigVersion sigversion) { switch (txType) { @@ -245,63 +298,51 @@ static CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatur case TX_NULL_DATA: case TX_ZEROCOINMINT: // Don't know anything about this, assume bigger one is correct: - if (sigs1.size() >= sigs2.size()) - return PushAll(sigs1); - return PushAll(sigs2); + if (sigs1.script.size() >= sigs2.script.size()) + return sigs1; + return sigs2; case TX_PUBKEY: case TX_PUBKEYHASH: case TX_COLDSTAKE: // Signatures are bigger than placeholders or empty scripts: - if (sigs1.empty() || sigs1[0].empty()) - return PushAll(sigs2); - return PushAll(sigs1); + if (sigs1.script.empty() || sigs1.script[0].empty()) + return sigs2; + return sigs1; case TX_SCRIPTHASH: - if (sigs1.empty() || sigs1.back().empty()) - return PushAll(sigs2); - else if (sigs2.empty() || sigs2.back().empty()) - return PushAll(sigs1); + if (sigs1.script.empty() || sigs1.script.back().empty()) + return sigs2; + else if (sigs2.script.empty() || sigs2.script.back().empty()) + return sigs1; else { // Recur to combine: - valtype spk = sigs1.back(); + valtype spk = sigs1.script.back(); CScript pubKey2(spk.begin(), spk.end()); txnouttype txType2; std::vector > vSolutions2; Solver(pubKey2, txType2, vSolutions2); - sigs1.pop_back(); - sigs2.pop_back(); - CScript result = CombineSignatures(pubKey2, checker, txType2, vSolutions2, sigs1, sigs2); - result << spk; + sigs1.script.pop_back(); + sigs2.script.pop_back(); + Stacks result = CombineSignatures(pubKey2, checker, txType2, vSolutions2, sigs1, sigs2, sigversion); + result.script.push_back(spk); return result; } case TX_MULTISIG: - return CombineMultisig(scriptPubKey, checker, vSolutions, sigs1, sigs2); + return Stacks(CombineMultisig(scriptPubKey, checker, vSolutions, sigs1.script, sigs2.script, sigversion)); } - return CScript(); + return Stacks(); } -CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CAmount& amount, - const CScript& scriptSig1, const CScript& scriptSig2) -{ - TransactionSignatureChecker checker(&txTo, nIn, amount); - return CombineSignatures(scriptPubKey, checker, scriptSig1, scriptSig2); -} - -CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, - const CScript& scriptSig1, const CScript& scriptSig2) +SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, + const SignatureData& scriptSig1, const SignatureData& scriptSig2) { txnouttype txType; std::vector > vSolutions; Solver(scriptPubKey, txType, vSolutions); - std::vector stack1; - EvalScript(stack1, scriptSig1, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SIGVERSION_BASE); - std::vector stack2; - EvalScript(stack2, scriptSig2, SCRIPT_VERIFY_STRICTENC, BaseSignatureChecker(), SIGVERSION_BASE); - - return CombineSignatures(scriptPubKey, checker, txType, vSolutions, stack1, stack2); + return CombineSignatures(scriptPubKey, checker, txType, vSolutions, Stacks(scriptSig1), Stacks(scriptSig2), SIGVERSION_BASE).Output(); } namespace { @@ -324,7 +365,7 @@ const BaseSignatureChecker& DummySignatureCreator::Checker() const return dummyChecker; } -bool DummySignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode) const +bool DummySignatureCreator::CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const { // Create a dummy signature that is a valid DER-encoding vchSig.assign(72, '\000'); diff --git a/src/script/sign.h b/src/script/sign.h index e53d2f38bafa..7e6a3651dd0a 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -28,7 +28,7 @@ class BaseSignatureCreator { virtual const BaseSignatureChecker& Checker() const =0; /** Create a singular (non-script) signature. */ - virtual bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode) const =0; + virtual bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0; }; /** A signature creator for transactions. */ @@ -42,7 +42,14 @@ class TransactionSignatureCreator : public BaseSignatureCreator { public: TransactionSignatureCreator(const CKeyStore* keystoreIn, const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn=SIGHASH_ALL); const BaseSignatureChecker& Checker() const { return checker; } - bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode) const; + bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const; +}; + +class MutableTransactionSignatureCreator : public TransactionSignatureCreator { + CTransaction tx; + +public: + MutableTransactionSignatureCreator(const CKeyStore* keystoreIn, const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amount, int nHashTypeIn) : TransactionSignatureCreator(keystoreIn, &tx, nInIn, amount, nHashTypeIn), tx(*txToIn) {} }; /** A signature creator that just produces 72-byte empty signatyres. */ @@ -50,20 +57,28 @@ class DummySignatureCreator : public BaseSignatureCreator { public: DummySignatureCreator(const CKeyStore* keystoreIn) : BaseSignatureCreator(keystoreIn) {} const BaseSignatureChecker& Checker() const; - bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode) const; + bool CreateSig(std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const; +}; + +struct SignatureData { + CScript scriptSig; + + SignatureData() {} + explicit SignatureData(const CScript& script) : scriptSig(script) {} }; /** Produce a script signature using a generic signature creator. */ -bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& scriptPubKey, CScript& scriptSig, bool fColdStake); +bool ProduceSignature(const BaseSignatureCreator& creator, const CScript& fromPubKey, SignatureData& sigdata, bool fColdStake, ScriptError* serror = nullptr); /** Produce a script signature for a transaction. */ -bool SignSignature(const CKeyStore& keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL, bool fColdStake = false); -bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType=SIGHASH_ALL, bool fColdStake = false); +bool SignSignature(const CKeyStore& keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, bool fColdStake = false); +bool SignSignature(const CKeyStore& keystore, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, bool fColdStake = false); /** Combine two script signatures using a generic signature checker, intelligently, possibly with OP_0 placeholders. */ -CScript CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const CScript& scriptSig1, const CScript& scriptSig2); +SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignatureChecker& checker, const SignatureData& scriptSig1, const SignatureData& scriptSig2); -/** Combine two script signatures on transactions. */ -CScript CombineSignatures(const CScript& scriptPubKey, const CTransaction& txTo, unsigned int nIn, const CAmount& amount, const CScript& scriptSig1, const CScript& scriptSig2); +/** Extract signature data from a transaction, and insert it. */ +SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn); +void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data); #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index b2e5cc3fc0e5..ff1af3c30f6c 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) tx.vout.resize(1); tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); - SignSignature(keystore, txPrev, tx, 0); + SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); AddOrphanTx(tx, i); } @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) tx.vin[j].prevout.n = j; tx.vin[j].prevout.hash = txPrev.GetHash(); } - SignSignature(keystore, txPrev, tx, 0); + SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); // Re-use same signature for other inputs // (they don't have to be valid for this test) for (unsigned int j = 1; j < tx.vin.size(); j++) diff --git a/src/test/multisig_tests.cpp b/src/test/multisig_tests.cpp index 2bab76081f1b..c241240748bc 100644 --- a/src/test/multisig_tests.cpp +++ b/src/test/multisig_tests.cpp @@ -317,7 +317,7 @@ BOOST_AUTO_TEST_CASE(multisig_Sign) for (int i = 0; i < 3; i++) { - BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0), strprintf("SignSignature %d", i)); + BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0, SIGHASH_ALL), strprintf("SignSignature %d", i)); } } diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index ec0f7d3d53d0..0ada24fe070f 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -107,7 +107,7 @@ BOOST_AUTO_TEST_CASE(sign) } for (int i = 0; i < 8; i++) { - BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0), strprintf("SignSignature %d", i)); + BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0, SIGHASH_ALL), strprintf("SignSignature %d", i)); } // All of the above should be OK, and the txTos have valid signatures // Check to make sure signature verification fails if we use the wrong ScriptSig: @@ -204,7 +204,7 @@ BOOST_AUTO_TEST_CASE(set) } for (int i = 0; i < 4; i++) { - BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0), strprintf("SignSignature %d", i)); + BOOST_CHECK_MESSAGE(SignSignature(keystore, txFrom, txTo[i], 0, SIGHASH_ALL), strprintf("SignSignature %d", i)); BOOST_CHECK_MESSAGE(IsStandardTx(txTo[i], reason), strprintf("txTo[%d].IsStandard", i)); } } @@ -333,9 +333,9 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txTo.vin[i].prevout.n = i; txTo.vin[i].prevout.hash = txFrom.GetHash(); } - BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 0)); - BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 1)); - BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2)); + BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL)); + BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 1, SIGHASH_ALL)); + BOOST_CHECK(SignSignature(keystore, txFrom, txTo, 2, SIGHASH_ALL)); // SignSignature doesn't know how to sign these. We're // not testing validating signatures, so just create // dummy signatures that DO include the correct P2SH scripts: diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 8cd35d1f4ffc..f85c0e1339cb 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -848,50 +848,50 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) CScript& scriptPubKey = txFrom.vout[0].scriptPubKey; CScript& scriptSig = txTo.vin[0].scriptSig; - CScript empty; - CScript combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, empty); - BOOST_CHECK(combined.empty()); + SignatureData empty; + SignatureData combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, empty); + BOOST_CHECK(combined.scriptSig.empty()); // Single signature case: - SignSignature(keystore, txFrom, txTo, 0); // changes scriptSig - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); - BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); - BOOST_CHECK(combined == scriptSig); + SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); // changes scriptSig + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty); + BOOST_CHECK(combined.scriptSig == scriptSig); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSig); CScript scriptSigCopy = scriptSig; // Signing again will give a different, valid signature: - SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); - BOOST_CHECK(combined == scriptSigCopy || combined == scriptSig); + SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSigCopy || combined.scriptSig == scriptSig); // P2SH, single-signature case: CScript pkSingle; pkSingle << ToByteVector(keys[0].GetPubKey()) << OP_CHECKSIG; keystore.AddCScript(pkSingle); scriptPubKey = GetScriptForDestination(CScriptID(pkSingle)); - SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); - BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); - BOOST_CHECK(combined == scriptSig); + SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty); + BOOST_CHECK(combined.scriptSig == scriptSig); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSig); scriptSigCopy = scriptSig; - SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); - BOOST_CHECK(combined == scriptSigCopy || combined == scriptSig); + SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSigCopy || combined.scriptSig == scriptSig); // dummy scriptSigCopy with placeholder, should always choose non-placeholder: scriptSigCopy = CScript() << OP_0 << std::vector(pkSingle.begin(), pkSingle.end()); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSigCopy, scriptSig); - BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, scriptSigCopy); - BOOST_CHECK(combined == scriptSig); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSigCopy), SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSig); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), SignatureData(scriptSigCopy)); + BOOST_CHECK(combined.scriptSig == scriptSig); // Hardest case: Multisig 2-of-3 scriptPubKey = GetScriptForMultisig(2, pubkeys); keystore.AddCScript(scriptPubKey); - SignSignature(keystore, txFrom, txTo, 0); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, scriptSig, empty); - BOOST_CHECK(combined == scriptSig); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, empty, scriptSig); - BOOST_CHECK(combined == scriptSig); + SignSignature(keystore, txFrom, txTo, 0, SIGHASH_ALL); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(scriptSig), empty); + BOOST_CHECK(combined.scriptSig == scriptSig); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), empty, SignatureData(scriptSig)); + BOOST_CHECK(combined.scriptSig == scriptSig); // A couple of partially-signed versions: std::vector sig1; @@ -919,24 +919,25 @@ BOOST_AUTO_TEST_CASE(script_combineSigs) CScript complete13 = CScript() << OP_0 << sig1 << sig3; CScript complete23 = CScript() << OP_0 << sig2 << sig3; - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1a, partial1b); - BOOST_CHECK(combined == partial1a); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1a, partial2a); - BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial2a, partial1a); - BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial1b, partial2b); - BOOST_CHECK(combined == complete12); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial1b); - BOOST_CHECK(combined == complete13); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial2a, partial3a); - BOOST_CHECK(combined == complete23); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial2b); - BOOST_CHECK(combined == complete23); - combined = CombineSignatures(scriptPubKey, txTo, 0, amount, partial3b, partial3a); - BOOST_CHECK(combined == partial3c); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1a), SignatureData(partial1b)); + BOOST_CHECK(combined.scriptSig == partial1a); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1a), SignatureData(partial2a)); + BOOST_CHECK(combined.scriptSig == complete12); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial2a), SignatureData(partial1a)); + BOOST_CHECK(combined.scriptSig == complete12); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial1b), SignatureData(partial2b)); + BOOST_CHECK(combined.scriptSig == complete12); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial1b)); + BOOST_CHECK(combined.scriptSig == complete13); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial2a), SignatureData(partial3a)); + BOOST_CHECK(combined.scriptSig == complete23); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial2b)); + BOOST_CHECK(combined.scriptSig == complete23); + combined = CombineSignatures(scriptPubKey, MutableTransactionSignatureChecker(&txTo, 0, amount), SignatureData(partial3b), SignatureData(partial3a)); + BOOST_CHECK(combined.scriptSig == partial3c); } + BOOST_AUTO_TEST_CASE(script_standard_push) { ScriptError err; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e8a737fba033..79ae5d21f7d3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2676,15 +2676,25 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, for (const PAIRTYPE(const CWalletTx*, unsigned int) & coin : setCoins) { bool signSuccess; const CScript& scriptPubKey = coin.first->vout[coin.second].scriptPubKey; - CScript& scriptSigRes = txNew.vin[nIn].scriptSig; - if (sign) - signSuccess = ProduceSignature(TransactionSignatureCreator(this, &txNewConst, nIn, SIGHASH_ALL), scriptPubKey, scriptSigRes, false); - else - signSuccess = ProduceSignature(DummySignatureCreator(this), scriptPubKey, scriptSigRes, false); + SignatureData sigdata; + bool haveKey = coin.first->GetStakeDelegationCredit() > 0; + if (sign) { + signSuccess = ProduceSignature( + TransactionSignatureCreator(this, &txNewConst, nIn, coin.first->vout[coin.second].nValue, SIGHASH_ALL), + scriptPubKey, + sigdata, + !haveKey // fColdStake = false + ); + } else { + signSuccess = ProduceSignature( + DummySignatureCreator(this), scriptPubKey, sigdata, false); + } if (!signSuccess) { strFailReason = _("Signing transaction failed"); return false; + } else { + UpdateTransaction(txNew, nIn, sigdata); } nIn++; } diff --git a/src/wallet/wallet_ismine.cpp b/src/wallet/wallet_ismine.cpp index 092146b4927f..2d96b95aea18 100644 --- a/src/wallet/wallet_ismine.cpp +++ b/src/wallet/wallet_ismine.cpp @@ -100,8 +100,8 @@ isminetype IsMine(const CKeyStore& keystore, const CScript& scriptPubKey) if (keystore.HaveWatchOnly(scriptPubKey)) { // TODO: This could be optimized some by doing some work after the above solver - CScript scriptSig; - return ProduceSignature(DummySignatureCreator(&keystore), scriptPubKey, scriptSig, false) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE; + SignatureData sigdata; + return ProduceSignature(DummySignatureCreator(&keystore), scriptPubKey, sigdata, false) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE; } return ISMINE_NO; diff --git a/src/wallet/wallet_zerocoin.cpp b/src/wallet/wallet_zerocoin.cpp index 3fa402e0d634..9b9831921aca 100644 --- a/src/wallet/wallet_zerocoin.cpp +++ b/src/wallet/wallet_zerocoin.cpp @@ -335,11 +335,26 @@ bool CWallet::CreateZerocoinMintTransaction(const CAmount nValue, // Sign int nIn = 0; - for (const std::pair& coin : setCoins) { - if (!SignSignature(*this, *coin.first, txNew, nIn++)) { + CTransaction txNewConst(txNew); + for (const PAIRTYPE(const CWalletTx*, unsigned int) & coin : setCoins) { + bool signSuccess; + const CScript& scriptPubKey = coin.first->vout[coin.second].scriptPubKey; + SignatureData sigdata; + signSuccess = ProduceSignature( + TransactionSignatureCreator(this, &txNewConst, nIn, coin.first->vout[coin.second].nValue, SIGHASH_ALL), + scriptPubKey, + sigdata, + false // fColdStake = false + ); + + if (!signSuccess) { strFailReason = _("Signing transaction failed"); return false; + } else { + UpdateTransaction(txNew, nIn, sigdata); } + + nIn++; } return true; From dfd24eb6f386ca819b12779634636ffbecef2d9a Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Apr 2020 16:46:01 -0300 Subject: [PATCH 05/10] Update wallet_txn_close.py test: Removing deprecated accounts checks and the hardcoded serialization. --- test/functional/wallet_txn_clone.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index e5faed66ba60..2d63fae78033 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -4,8 +4,11 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet accounts properly when there are cloned transactions with malleated scriptsigs.""" +import io from test_framework.test_framework import PivxTestFramework from test_framework.util import * +from test_framework.messages import CTransaction, COIN + class TxnMallTest(PivxTestFramework): def set_test_params(self): @@ -44,8 +47,9 @@ def run_test(self): # Coins are sent to node1_address node1_address = self.nodes[1].getnewaddress() + tx1_amount = 40 * 5 # Send tx1, and another transaction tx2 that won't be cloned - txid1 = self.nodes[0].sendtoaddress(node1_address, (40 * 5)) + txid1 = self.nodes[0].sendtoaddress(node1_address, tx1_amount) txid2 = self.nodes[0].sendtoaddress(node1_address, (20 * 5)) # Construct a clone of tx1, to be malleated @@ -57,20 +61,19 @@ def run_test(self): clone_raw = self.nodes[0].createrawtransaction(clone_inputs, clone_outputs, clone_locktime) # createrawtransaction randomizes the order of its outputs, so swap them if necessary. - # output 0 is at version+#inputs+input+sigstub+sequence+#outputs - # 40 BTC serialized is 00286bee00000000 - pos0 = 2*(4+1+36+1+4+1) - hex40 = "00286bee00000000" - output_len = 16 + 2 + 2 * int("0x" + clone_raw[pos0 + 16: pos0 + 16 + 2], 0) - if (rawtx1["vout"][0]["value"] == 40 and clone_raw[pos0: pos0 + 16] != hex40 or - rawtx1["vout"][0]["value"] != 40 and clone_raw[pos0: pos0 + 16] == hex40): - output0 = clone_raw[pos0: pos0 + output_len] - output1 = clone_raw[pos0 + output_len: pos0 + 2 * output_len] - clone_raw = clone_raw[:pos0] + output1 + output0 + clone_raw[pos0 + 2 * output_len:] + clone_tx = CTransaction() + clone_tx.deserialize(io.BytesIO(bytes.fromhex(clone_raw))) + + if (rawtx1["vout"][0]["value"] == tx1_amount and clone_tx.vout[0].nValue != tx1_amount*COIN or rawtx1["vout"][0]["value"] != tx1_amount and clone_tx.vout[0].nValue == tx1_amount*COIN): + (clone_tx.vout[0], clone_tx.vout[1]) = (clone_tx.vout[1], clone_tx.vout[0]) + + if (rawtx1 == clone_raw): + print("## !! Equal hex!!") + assert(False) # Use a different signature hash type to sign. This creates an equivalent but malleated clone. # Don't send the clone anywhere yet - tx1_clone = self.nodes[0].signrawtransaction(clone_raw, None, None, "ALL|ANYONECANPAY") + tx1_clone = self.nodes[0].signrawtransaction(clone_tx.serialize().hex(), None, None, "ALL|ANYONECANPAY") assert_equal(tx1_clone["complete"], True) # Have node0 mine a block, if requested: @@ -130,7 +133,6 @@ def run_test(self): expected -= 250 assert_equal(self.nodes[0].getbalance(), expected) - if __name__ == '__main__': TxnMallTest().main() From 446d340cc7463570f353d59b56ee35912f2cc34a Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Apr 2020 18:32:32 -0300 Subject: [PATCH 06/10] Precompute sighashes Coming from btc@d2c5d044d00ec805957ab246a7863d83ca075805 --- src/main.cpp | 33 ++++++++++++++------- src/main.h | 10 +++++-- src/miner.cpp | 3 +- src/script/interpreter.cpp | 53 +++++++++++++++++++++++----------- src/script/interpreter.h | 14 +++++++-- src/script/sigcache.h | 2 +- src/test/script_P2SH_tests.cpp | 10 ++++--- src/test/transaction_tests.cpp | 6 ++-- src/txmempool.cpp | 6 ++-- 9 files changed, 95 insertions(+), 42 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 12ccecfcc6fe..af1fac4e4232 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1058,7 +1058,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa int flags = STANDARD_SCRIPT_VERIFY_FLAGS; if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - if (!CheckInputs(tx, state, view, true, flags, true)) { + + CachedHashes cachedHashes(tx); + if (!CheckInputs(tx, state, view, true, flags, true, cachedHashes)) { return false; } @@ -1074,9 +1076,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa flags = MANDATORY_SCRIPT_VERIFY_FLAGS; if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - if (!CheckInputs(tx, state, view, true, flags, true)) { + if (!CheckInputs(tx, state, view, true, flags, true, cachedHashes)) { return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", - __func__, hash.ToString(), FormatStateMessage(state)); + __func__, hash.ToString(), FormatStateMessage(state)); } // Store transaction in memory @@ -1283,7 +1285,9 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact int flags = STANDARD_SCRIPT_VERIFY_FLAGS; if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - if (!CheckInputs(tx, state, view, false, flags, true)) { + + CachedHashes cachedHashes(tx); + if (!CheckInputs(tx, state, view, false, flags, true, cachedHashes)) { return error("AcceptableInputs: : ConnectInputs failed %s", hash.ToString()); } @@ -1297,7 +1301,7 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. // for any real tx this will be checked on AcceptToMemoryPool anyway - // if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + // if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) // { // return error("AcceptableInputs: : BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); // } @@ -1735,7 +1739,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache &inputs, int nHeight) bool CScriptCheck::operator()() { const CScript& scriptSig = ptxTo->vin[nIn].scriptSig; - return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore), &error); + return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *cachedHashes), &error); } std::map mapInvalidOutPoints; @@ -1858,7 +1862,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } }// namespace Consensus -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks) { if (!tx.IsCoinBase() && !tx.HasZerocoinSpendInputs()) { @@ -1882,7 +1886,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi assert(coins); // Verify signature - CScriptCheck check(*coins, tx, i, flags, cacheStore); + CScriptCheck check(*coins, tx, i, flags, cacheStore, &cachedHashes); if (pvChecks) { pvChecks->push_back(CScriptCheck()); check.swap(pvChecks->back()); @@ -1895,7 +1899,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // avoid splitting the network between upgraded and // non-upgraded nodes. CScriptCheck check(*coins, tx, i, - flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore); + flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes); if (check()) return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } @@ -2238,6 +2242,9 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin unsigned int nMaxBlockSigOps = MAX_BLOCK_SIGOPS_CURRENT; std::vector vSpendsInBlock; uint256 hashBlock = block.GetHash(); + + std::vector cachedHashes; + cachedHashes.reserve(block.vtx.size()); // Required so that pointers to individual CachedHashes don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction& tx = block.vtx[i]; @@ -2344,6 +2351,12 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin if (nSigOps > nMaxBlockSigOps) return state.DoS(100, error("ConnectBlock() : too many sigops"), REJECT_INVALID, "bad-blk-sigops"); + } + + // Cache the sig ser hashes + cachedHashes.emplace_back(tx); + + if (!tx.IsCoinBase()) { if (!tx.IsCoinStake()) nFees += view.GetValueIn(tx) - tx.GetValueOut(); nValueIn += view.GetValueIn(tx); @@ -2354,7 +2367,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ - if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, nScriptCheckThreads ? &vChecks : NULL)) + if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, cachedHashes[i], nScriptCheckThreads ? &vChecks : NULL)) return error("%s: Check inputs on %s failed with %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index 02eb4c2cea95..66e9cca8d267 100644 --- a/src/main.h +++ b/src/main.h @@ -59,6 +59,7 @@ class CScriptCheck; class CValidationInterface; class CValidationState; +struct CachedHashes; struct CBlockTemplate; struct CNodeStateStats; @@ -281,7 +282,7 @@ CAmount GetMinRelayFee(const CTransaction& tx, const CTxMemPool& pool, unsigned * This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it * instead of being performed inline. */ -bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, std::vector* pvChecks = NULL); +bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector* pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); @@ -314,12 +315,14 @@ class CScriptCheck unsigned int nFlags; bool cacheStore; ScriptError error; + CachedHashes *cachedHashes; public: CScriptCheck() : amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} - CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), + CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, CachedHashes* cachedHashesIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), - ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR) {} + ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), + cachedHashes(cachedHashesIn) {} bool operator()(); @@ -332,6 +335,7 @@ class CScriptCheck std::swap(nFlags, check.nFlags); std::swap(cacheStore, check.cacheStore); std::swap(error, check.error); + std::swap(cachedHashes, check.cachedHashes); } ScriptError GetScriptError() const { return error; } diff --git a/src/miner.cpp b/src/miner.cpp index c3fd8fddc449..4991f08868d2 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -367,7 +367,8 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn, CWallet* pwallet, // create only contains transactions that are valid in new blocks. CValidationState state; - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true)) + CachedHashes cachedHashes(tx); + if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) continue; UpdateCoins(tx, view, nHeight); diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 4523fdf0c0a6..30c272a94dd5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1085,9 +1085,40 @@ class CTransactionSignatureSerializer { } }; +uint256 GetPrevoutHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].prevout; + } + return ss.GetHash(); +} + +uint256 GetSequenceHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vin.size(); n++) { + ss << txTo.vin[n].nSequence; + } + return ss.GetHash(); +} + +uint256 GetOutputsHash(const CTransaction& txTo) { + CHashWriter ss(SER_GETHASH, 0); + for (unsigned int n = 0; n < txTo.vout.size(); n++) { + ss << txTo.vout[n]; + } + return ss.GetHash(); +} + } // anon namespace -uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion) +CachedHashes::CachedHashes(const CTransaction& txTo) +{ + hashPrevouts = GetPrevoutHash(txTo); + hashSequence = GetSequenceHash(txTo); + hashOutputs = GetOutputsHash(txTo); +} + +uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache) { if (nIn >= txTo.vin.size()) { // nIn out of range @@ -1101,27 +1132,15 @@ uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsig uint256 hashOutputs; if (!(nHashType & SIGHASH_ANYONECANPAY)) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].prevout; - } - hashPrevouts = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashPrevouts = cache ? cache->hashPrevouts : GetPrevoutHash(txTo); } if (!(nHashType & SIGHASH_ANYONECANPAY) && (nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vin.size(); n++) { - ss << txTo.vin[n].nSequence; - } - hashSequence = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashSequence = cache ? cache->hashSequence : GetSequenceHash(txTo); } if ((nHashType & 0x1f) != SIGHASH_SINGLE && (nHashType & 0x1f) != SIGHASH_NONE) { - CHashWriter ss(SER_GETHASH, 0); - for (unsigned int n = 0; n < txTo.vout.size(); n++) { - ss << txTo.vout[n]; - } - hashOutputs = ss.GetHash(); // TODO: cache this value for all signatures in a transaction + hashOutputs = cache ? cache->hashOutputs : GetOutputsHash(txTo); } else if ((nHashType & 0x1f) == SIGHASH_SINGLE && nIn < txTo.vout.size()) { CHashWriter ss(SER_GETHASH, 0); ss << txTo.vout[nIn]; @@ -1186,7 +1205,7 @@ bool TransactionSignatureChecker::CheckSig(const std::vector& vch int nHashType = vchSig.back(); vchSig.pop_back(); - uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->cachedHashes); if (!VerifySignature(vchSig, pubkey, sighash)) return false; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 817b2459c246..ee0f626dcd75 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -86,13 +86,20 @@ enum bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); +struct CachedHashes +{ + uint256 hashPrevouts, hashSequence, hashOutputs; + + CachedHashes(const CTransaction& tx); +}; + enum SigVersion { SIGVERSION_BASE = 0, SIGVERSION_WITNESS_V0 = 1, }; -uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion); +uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache = nullptr); class BaseSignatureChecker { @@ -121,12 +128,15 @@ class TransactionSignatureChecker : public BaseSignatureChecker const CTransaction* txTo; unsigned int nIn; const CAmount amount; + const CachedHashes* cachedHashes; protected: virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(nullptr) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const CachedHashes& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(&cachedHashesIn) {} + bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override ; bool CheckLockTime(const CScriptNum& nLockTime) const override; bool CheckColdStake(const CScript& script) const override { diff --git a/src/script/sigcache.h b/src/script/sigcache.h index f01c198954e4..701fe4b16a7f 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -46,7 +46,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn=true) : TransactionSignatureChecker(txToIn, nInIn, amount), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, CachedHashes& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {} bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; }; diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 0ada24fe070f..3cac1a58ae80 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -111,18 +111,20 @@ BOOST_AUTO_TEST_CASE(sign) } // All of the above should be OK, and the txTos have valid signatures // Check to make sure signature verification fails if we use the wrong ScriptSig: - for (int i = 0; i < 8; i++) - for (int j = 0; j < 8; j++) - { + for (int i = 0; i < 8; i++) { + CachedHashes cachedHashes(txTo[i]); + for (int j = 0; j < 8; j++) { CScript sigSave = txTo[i].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; - bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, false)(); + bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, + false, &cachedHashes)(); if (i == j) BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); else BOOST_CHECK_MESSAGE(!sigOK, strprintf("VerifySignature %d %d", i, j)); txTo[i].vin[0].scriptSig = sigSave; } + } } BOOST_AUTO_TEST_CASE(norecurse) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 1c2d0cc953fc..c2135d4bd971 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -140,6 +140,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) BOOST_CHECK_MESSAGE(CheckTransaction(tx, false, false, state), strTest); BOOST_CHECK(state.IsValid()); + CachedHashes cachedHashes(tx); for (unsigned int i = 0; i < tx.vin.size(); i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -151,7 +152,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i, amount), &err), + verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err), strTest); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -215,6 +216,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CValidationState state; fValid = CheckTransaction(tx, false, false, state) && state.IsValid(); + CachedHashes cachedHashes(tx); for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -226,7 +228,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i, amount), &err); + verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err); } BOOST_CHECK_MESSAGE(!fValid, strTest); BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err)); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 95ab98a11c69..1cca32d8184f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -662,7 +662,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const waitingOnDependants.push_back(&(*it)); else { CValidationState state; - assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, NULL)); + CachedHashes cachedHashes(tx); + assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, cachedHashes, NULL)); UpdateCoins(tx, mempoolDuplicate, 1000000); } } @@ -677,7 +678,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const stepsSinceLastRemove++; assert(stepsSinceLastRemove < waitingOnDependants.size()); } else { - assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, NULL)); + CachedHashes cachedHashes(entry->GetTx()); + assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, cachedHashes, NULL)); UpdateCoins(entry->GetTx(), mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; } From 2ef387244c5fddea81479711505a7b634ea8e007 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 24 Apr 2020 20:54:01 -0300 Subject: [PATCH 07/10] Report non-mandatory script failures correctly. Coming from btc@7ef8f3c072a8750c72a3a1cdc727b5c1d173bac8 --- src/main.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index af1fac4e4232..1c1806fe8fac 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1898,9 +1898,9 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // arguments; if so, don't trigger DoS protection to // avoid splitting the network between upgraded and // non-upgraded nodes. - CScriptCheck check(*coins, tx, i, + CScriptCheck check2(*coins, tx, i, flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes); - if (check()) + if (check2()) return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } // Failures of other flags indicate a transaction that is From b4b181ba863a91bd4e016d6e977ba6d5b3315e4d Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Apr 2020 18:38:49 -0300 Subject: [PATCH 08/10] Unit test for sighash caching Coming from btc@ab48c5e72156b34300db4a6521cb3c9969be3937 --- src/test/transaction_tests.cpp | 82 ++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index c2135d4bd971..60c693b892da 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -9,12 +9,14 @@ #include "consensus/tx_verify.h" #include "clientversion.h" +#include "checkqueue.h" #include "key.h" #include "keystore.h" #include "main.h" #include "policy/policy.h" #include "script/script.h" #include "script/script_error.h" +#include "script/sign.h" #include "core_io.h" #include "test_pivx.h" @@ -324,6 +326,86 @@ BOOST_AUTO_TEST_CASE(test_Get) BOOST_CHECK(!AreInputsStandard(t1, coins)); } +BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { + CMutableTransaction mtx; + mtx.nVersion = 2; // Sapling future version + + CKey key; + key.MakeNewKey(false); + CBasicKeyStore keystore; + keystore.AddKeyPubKey(key, key.GetPubKey()); + CKeyID hash = key.GetPubKey().GetID(); + CScript scriptPubKey = GetScriptForDestination(hash); + + std::vector sigHashes; + sigHashes.push_back(SIGHASH_NONE | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_SINGLE | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_ALL | SIGHASH_ANYONECANPAY); + sigHashes.push_back(SIGHASH_NONE); + sigHashes.push_back(SIGHASH_SINGLE); + sigHashes.push_back(SIGHASH_ALL); + + // create a big transaction of 4500 inputs signed by the same key + for(uint32_t ij = 0; ij < 4500; ij++) { + uint32_t i = mtx.vin.size(); + uint256 prevId; + prevId.SetHex("0000000000000000000000000000000000000000000000000000000000000100"); + COutPoint outpoint(prevId, i); + + mtx.vin.resize(mtx.vin.size() + 1); + mtx.vin[i].prevout = outpoint; + mtx.vin[i].scriptSig = CScript(); + + mtx.vout.resize(mtx.vout.size() + 1); + mtx.vout[i].nValue = 1000; + mtx.vout[i].scriptPubKey = CScript() << OP_1; + } + + // sign all inputs + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + bool hashSigned = SignSignature(keystore, scriptPubKey, mtx, i, 1000, sigHashes.at(i % sigHashes.size())); + assert(hashSigned); + } + + CTransaction tx; + CDataStream ssout(SER_NETWORK, PROTOCOL_VERSION); + ssout << mtx; + ssout >> tx; + + // check all inputs concurrently, with the cache + CachedHashes cachedHashes(tx); + boost::thread_group threadGroup; + CCheckQueue scriptcheckqueue(128); + CCheckQueueControl control(&scriptcheckqueue); + + for (int i=0; i<20; i++) + threadGroup.create_thread(boost::bind(&CCheckQueue::Thread, boost::ref(scriptcheckqueue))); + + CCoins coins; + coins.nVersion = 1; + coins.fCoinBase = false; + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + CTxOut txout; + txout.nValue = 1000; + txout.scriptPubKey = scriptPubKey; + coins.vout.push_back(txout); + } + + for(uint32_t i = 0; i < mtx.vin.size(); i++) { + std::vector vChecks; + CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH, false, &cachedHashes); + vChecks.push_back(CScriptCheck()); + check.swap(vChecks.back()); + control.Add(vChecks); + } + + bool controlCheck = control.Wait(); + assert(controlCheck); + + threadGroup.interrupt_all(); + threadGroup.join_all(); +} + BOOST_AUTO_TEST_CASE(test_IsStandard) { LOCK(cs_main); From a034daf3af5b80d81d3192e7c0bf01965c3a77eb Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Apr 2020 18:48:46 -0300 Subject: [PATCH 09/10] Rename to PrecomputedTransactionData Coming from btc@35fe0393f216aa6020fc929272118eade5628636 --- src/main.cpp | 28 ++++++++++++++-------------- src/main.h | 12 ++++++------ src/miner.cpp | 4 ++-- src/script/interpreter.cpp | 6 +++--- src/script/interpreter.h | 12 ++++++------ src/script/sigcache.h | 2 +- src/test/script_P2SH_tests.cpp | 4 ++-- src/test/transaction_tests.cpp | 12 ++++++------ src/txmempool.cpp | 8 ++++---- 9 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 1c1806fe8fac..16f82436140d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1059,8 +1059,8 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - CachedHashes cachedHashes(tx); - if (!CheckInputs(tx, state, view, true, flags, true, cachedHashes)) { + PrecomputedTransactionData precomTxData(tx); + if (!CheckInputs(tx, state, view, true, flags, true, precomTxData)) { return false; } @@ -1076,7 +1076,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa flags = MANDATORY_SCRIPT_VERIFY_FLAGS; if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - if (!CheckInputs(tx, state, view, true, flags, true, cachedHashes)) { + if (!CheckInputs(tx, state, view, true, flags, true, precomTxData)) { return error("%s: BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); } @@ -1286,8 +1286,8 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact if (fCLTVIsActivated) flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; - CachedHashes cachedHashes(tx); - if (!CheckInputs(tx, state, view, false, flags, true, cachedHashes)) { + PrecomputedTransactionData precomTxData(tx); + if (!CheckInputs(tx, state, view, false, flags, true, precomTxData)) { return error("AcceptableInputs: : ConnectInputs failed %s", hash.ToString()); } @@ -1301,7 +1301,7 @@ bool AcceptableInputs(CTxMemPool& pool, CValidationState& state, const CTransact // invalid blocks, however allowing such transactions into the mempool // can be exploited as a DoS attack. // for any real tx this will be checked on AcceptToMemoryPool anyway - // if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) + // if (!CheckInputs(tx, state, view, false, MANDATORY_SCRIPT_VERIFY_FLAGS, true, precomTxData)) // { // return error("AcceptableInputs: : BUG! PLEASE REPORT THIS! ConnectInputs failed against MANDATORY but not STANDARD flags %s", hash.ToString()); // } @@ -1739,7 +1739,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache &inputs, int nHeight) bool CScriptCheck::operator()() { const CScript& scriptSig = ptxTo->vin[nIn].scriptSig; - return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *cachedHashes), &error); + return VerifyScript(scriptSig, scriptPubKey, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *precomTxData), &error); } std::map mapInvalidOutPoints; @@ -1862,7 +1862,7 @@ bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoins } }// namespace Consensus -bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector *pvChecks) +bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& precomTxData, std::vector *pvChecks) { if (!tx.IsCoinBase() && !tx.HasZerocoinSpendInputs()) { @@ -1886,7 +1886,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi assert(coins); // Verify signature - CScriptCheck check(*coins, tx, i, flags, cacheStore, &cachedHashes); + CScriptCheck check(*coins, tx, i, flags, cacheStore, &precomTxData); if (pvChecks) { pvChecks->push_back(CScriptCheck()); check.swap(pvChecks->back()); @@ -1899,7 +1899,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi // avoid splitting the network between upgraded and // non-upgraded nodes. CScriptCheck check2(*coins, tx, i, - flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &cachedHashes); + flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheStore, &precomTxData); if (check2()) return state.Invalid(false, REJECT_NONSTANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(check.GetScriptError()))); } @@ -2243,8 +2243,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::vector vSpendsInBlock; uint256 hashBlock = block.GetHash(); - std::vector cachedHashes; - cachedHashes.reserve(block.vtx.size()); // Required so that pointers to individual CachedHashes don't get invalidated + std::vector precomTxData; + precomTxData.reserve(block.vtx.size()); // Required so that pointers to individual precomTxData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { const CTransaction& tx = block.vtx[i]; @@ -2354,7 +2354,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } // Cache the sig ser hashes - cachedHashes.emplace_back(tx); + precomTxData.emplace_back(tx); if (!tx.IsCoinBase()) { if (!tx.IsCoinStake()) @@ -2367,7 +2367,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin flags |= SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY; bool fCacheResults = fJustCheck; /* Don't cache results if we're actually connecting blocks (still consult the cache, though) */ - if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, cachedHashes[i], nScriptCheckThreads ? &vChecks : NULL)) + if (!CheckInputs(tx, state, view, fScriptChecks, flags, fCacheResults, precomTxData[i], nScriptCheckThreads ? &vChecks : NULL)) return error("%s: Check inputs on %s failed with %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); control.Add(vChecks); } diff --git a/src/main.h b/src/main.h index 66e9cca8d267..ea980ede797a 100644 --- a/src/main.h +++ b/src/main.h @@ -59,7 +59,7 @@ class CScriptCheck; class CValidationInterface; class CValidationState; -struct CachedHashes; +struct PrecomputedTransactionData; struct CBlockTemplate; struct CNodeStateStats; @@ -282,7 +282,7 @@ CAmount GetMinRelayFee(const CTransaction& tx, const CTxMemPool& pool, unsigned * This does not modify the UTXO set. If pvChecks is not NULL, script checks are pushed onto it * instead of being performed inline. */ -bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, CachedHashes& cachedHashes, std::vector* pvChecks = NULL); +bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& view, bool fScriptChecks, unsigned int flags, bool cacheStore, PrecomputedTransactionData& precomTxData, std::vector* pvChecks = NULL); /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); @@ -315,14 +315,14 @@ class CScriptCheck unsigned int nFlags; bool cacheStore; ScriptError error; - CachedHashes *cachedHashes; + PrecomputedTransactionData *precomTxData; public: CScriptCheck() : amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {} - CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, CachedHashes* cachedHashesIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), + CScriptCheck(const CCoins& txFromIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* cachedHashesIn) : scriptPubKey(txFromIn.vout[txToIn.vin[nInIn].prevout.n].scriptPubKey), amount(txFromIn.vout[txToIn.vin[nInIn].prevout.n].nValue), ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), - cachedHashes(cachedHashesIn) {} + precomTxData(cachedHashesIn) {} bool operator()(); @@ -335,7 +335,7 @@ class CScriptCheck std::swap(nFlags, check.nFlags); std::swap(cacheStore, check.cacheStore); std::swap(error, check.error); - std::swap(cachedHashes, check.cachedHashes); + std::swap(precomTxData, check.precomTxData); } ScriptError GetScriptError() const { return error; } diff --git a/src/miner.cpp b/src/miner.cpp index 4991f08868d2..c335a8d427a9 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -367,8 +367,8 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn, CWallet* pwallet, // create only contains transactions that are valid in new blocks. CValidationState state; - CachedHashes cachedHashes(tx); - if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, cachedHashes)) + PrecomputedTransactionData precomTxData(tx); + if (!CheckInputs(tx, state, view, true, MANDATORY_SCRIPT_VERIFY_FLAGS, true, precomTxData)) continue; UpdateCoins(tx, view, nHeight); diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 30c272a94dd5..b7b5b230e288 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1111,14 +1111,14 @@ uint256 GetOutputsHash(const CTransaction& txTo) { } // anon namespace -CachedHashes::CachedHashes(const CTransaction& txTo) +PrecomputedTransactionData::PrecomputedTransactionData(const CTransaction& txTo) { hashPrevouts = GetPrevoutHash(txTo); hashSequence = GetSequenceHash(txTo); hashOutputs = GetOutputsHash(txTo); } -uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache) +uint256 SignatureHash(const CScript& scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache) { if (nIn >= txTo.vin.size()) { // nIn out of range @@ -1205,7 +1205,7 @@ bool TransactionSignatureChecker::CheckSig(const std::vector& vch int nHashType = vchSig.back(); vchSig.pop_back(); - uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->cachedHashes); + uint256 sighash = SignatureHash(scriptCode, *txTo, nIn, nHashType, amount, sigversion, this->precomTxData); if (!VerifySignature(vchSig, pubkey, sighash)) return false; diff --git a/src/script/interpreter.h b/src/script/interpreter.h index ee0f626dcd75..e7e932c098d8 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -86,11 +86,11 @@ enum bool CheckSignatureEncoding(const std::vector &vchSig, unsigned int flags, ScriptError* serror); -struct CachedHashes +struct PrecomputedTransactionData { uint256 hashPrevouts, hashSequence, hashOutputs; - CachedHashes(const CTransaction& tx); + PrecomputedTransactionData(const CTransaction& tx); }; enum SigVersion @@ -99,7 +99,7 @@ enum SigVersion SIGVERSION_WITNESS_V0 = 1, }; -uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const CachedHashes* cache = nullptr); +uint256 SignatureHash(const CScript &scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType, const CAmount& amount, SigVersion sigversion, const PrecomputedTransactionData* cache = nullptr); class BaseSignatureChecker { @@ -128,14 +128,14 @@ class TransactionSignatureChecker : public BaseSignatureChecker const CTransaction* txTo; unsigned int nIn; const CAmount amount; - const CachedHashes* cachedHashes; + const PrecomputedTransactionData* precomTxData; protected: virtual bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; public: - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(nullptr) {} - TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const CachedHashes& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), cachedHashes(&cachedHashesIn) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), precomTxData(nullptr) {} + TransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& cachedHashesIn) : txTo(txToIn), nIn(nInIn), amount(amountIn), precomTxData(&cachedHashesIn) {} bool CheckSig(const std::vector& scriptSig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override ; bool CheckLockTime(const CScriptNum& nLockTime) const override; diff --git a/src/script/sigcache.h b/src/script/sigcache.h index 701fe4b16a7f..e36323e386ea 100644 --- a/src/script/sigcache.h +++ b/src/script/sigcache.h @@ -46,7 +46,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker bool store; public: - CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, CachedHashes& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {} + CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amount, bool storeIn, PrecomputedTransactionData& cachedHashesIn) : TransactionSignatureChecker(txToIn, nInIn, amount, cachedHashesIn), store(storeIn) {} bool VerifySignature(const std::vector& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const; }; diff --git a/src/test/script_P2SH_tests.cpp b/src/test/script_P2SH_tests.cpp index 3cac1a58ae80..382ebeea60b4 100644 --- a/src/test/script_P2SH_tests.cpp +++ b/src/test/script_P2SH_tests.cpp @@ -112,12 +112,12 @@ BOOST_AUTO_TEST_CASE(sign) // All of the above should be OK, and the txTos have valid signatures // Check to make sure signature verification fails if we use the wrong ScriptSig: for (int i = 0; i < 8; i++) { - CachedHashes cachedHashes(txTo[i]); + PrecomputedTransactionData precomTxData(txTo[i]); for (int j = 0; j < 8; j++) { CScript sigSave = txTo[i].vin[0].scriptSig; txTo[i].vin[0].scriptSig = txTo[j].vin[0].scriptSig; bool sigOK = CScriptCheck(CCoins(txFrom, 0), txTo[i], 0, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC, - false, &cachedHashes)(); + false, &precomTxData)(); if (i == j) BOOST_CHECK_MESSAGE(sigOK, strprintf("VerifySignature %d %d", i, j)); else diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 60c693b892da..0db74a41535f 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -142,7 +142,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) BOOST_CHECK_MESSAGE(CheckTransaction(tx, false, false, state), strTest); BOOST_CHECK(state.IsValid()); - CachedHashes cachedHashes(tx); + PrecomputedTransactionData precomTxData(tx); for (unsigned int i = 0; i < tx.vin.size(); i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err), + verify_flags, TransactionSignatureChecker(&tx, i, amount, precomTxData), &err), strTest); BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err)); } @@ -218,7 +218,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CValidationState state; fValid = CheckTransaction(tx, false, false, state) && state.IsValid(); - CachedHashes cachedHashes(tx); + PrecomputedTransactionData precomTxData(tx); for (unsigned int i = 0; i < tx.vin.size() && fValid; i++) { if (!mapprevOutScriptPubKeys.count(tx.vin[i].prevout)) @@ -230,7 +230,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) CAmount amount = 0; unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], - verify_flags, TransactionSignatureChecker(&tx, i, amount, cachedHashes), &err); + verify_flags, TransactionSignatureChecker(&tx, i, amount, precomTxData), &err); } BOOST_CHECK_MESSAGE(!fValid, strTest); BOOST_CHECK_MESSAGE(err != SCRIPT_ERR_OK, ScriptErrorString(err)); @@ -373,7 +373,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { ssout >> tx; // check all inputs concurrently, with the cache - CachedHashes cachedHashes(tx); + PrecomputedTransactionData precomTxData(tx); boost::thread_group threadGroup; CCheckQueue scriptcheckqueue(128); CCheckQueueControl control(&scriptcheckqueue); @@ -393,7 +393,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { for(uint32_t i = 0; i < mtx.vin.size(); i++) { std::vector vChecks; - CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH, false, &cachedHashes); + CScriptCheck check(coins, tx, i, SCRIPT_VERIFY_P2SH, false, &precomTxData); vChecks.push_back(CScriptCheck()); check.swap(vChecks.back()); control.Add(vChecks); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 1cca32d8184f..219a5eb7136a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -662,8 +662,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const waitingOnDependants.push_back(&(*it)); else { CValidationState state; - CachedHashes cachedHashes(tx); - assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, cachedHashes, NULL)); + PrecomputedTransactionData precomTxData(tx); + assert(CheckInputs(tx, state, mempoolDuplicate, false, 0, false, precomTxData, NULL)); UpdateCoins(tx, mempoolDuplicate, 1000000); } } @@ -678,8 +678,8 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const stepsSinceLastRemove++; assert(stepsSinceLastRemove < waitingOnDependants.size()); } else { - CachedHashes cachedHashes(entry->GetTx()); - assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, cachedHashes, NULL)); + PrecomputedTransactionData precomTxData(entry->GetTx()); + assert(CheckInputs(entry->GetTx(), state, mempoolDuplicate, false, 0, false, precomTxData, NULL)); UpdateCoins(entry->GetTx(), mempoolDuplicate, 1000000); stepsSinceLastRemove = 0; } From d1d15c8d4d1f7e624c53d3ff4bd2ff583168cd34 Mon Sep 17 00:00:00 2001 From: furszy Date: Sun, 2 Aug 2020 18:25:52 -0300 Subject: [PATCH 10/10] Fix missing sigverion in main_test.cpp CreateDummyScriptSigWithKey. --- src/test/main_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/main_tests.cpp b/src/test/main_tests.cpp index 343caa848cf4..cef6cfecca5e 100644 --- a/src/test/main_tests.cpp +++ b/src/test/main_tests.cpp @@ -33,7 +33,7 @@ std::vector CreateDummyScriptSigWithKey(CPubKey pubKey) { std::vector vchSig; const CScript scriptCode; - DummySignatureCreator(nullptr).CreateSig(vchSig, pubKey.GetID(), scriptCode); + DummySignatureCreator(nullptr).CreateSig(vchSig, pubKey.GetID(), scriptCode, SIGVERSION_BASE); return vchSig; }