From f6fb7acda4aefd01b8ef6cd77063bfc0c4f4ab36 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 4 Aug 2016 02:49:16 +0200 Subject: [PATCH] Move CTxInWitness inside CTxIn --- src/bench/verify_script.cpp | 5 +- src/bitcoin-tx.cpp | 2 +- src/core_memusage.h | 30 +++------- src/core_write.cpp | 4 +- src/miner.cpp | 4 +- src/net_processing.cpp | 4 +- src/policy/policy.cpp | 8 +-- src/primitives/transaction.cpp | 10 ++-- src/primitives/transaction.h | 99 +++++++++++---------------------- src/rpc/rawtransaction.cpp | 11 ++-- src/script/bitcoinconsensus.cpp | 3 +- src/script/script.h | 2 + src/script/sign.cpp | 9 +-- src/test/script_tests.cpp | 3 +- src/test/sigopcount_tests.cpp | 32 ++++------- src/test/transaction_tests.cpp | 17 ++---- src/validation.cpp | 23 ++++---- src/wallet/wallet.cpp | 6 +- 18 files changed, 96 insertions(+), 176 deletions(-) diff --git a/src/bench/verify_script.cpp b/src/bench/verify_script.cpp index dc3940cdbd..4aca1c7eb3 100644 --- a/src/bench/verify_script.cpp +++ b/src/bench/verify_script.cpp @@ -36,7 +36,6 @@ static CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, co txSpend.nLockTime = 0; txSpend.vin.resize(1); txSpend.vout.resize(1); - txSpend.wit.vtxinwit.resize(1); txSpend.vin[0].prevout.hash = txCredit.GetHash(); txSpend.vin[0].prevout.n = 0; txSpend.vin[0].scriptSig = scriptSig; @@ -68,7 +67,7 @@ static void VerifyScriptBench(benchmark::State& state) CScript witScriptPubkey = CScript() << OP_DUP << OP_HASH160 << ToByteVector(pubkeyHash) << OP_EQUALVERIFY << OP_CHECKSIG; CTransaction txCredit = BuildCreditingTransaction(scriptPubKey); CMutableTransaction txSpend = BuildSpendingTransaction(scriptSig, txCredit); - CScriptWitness& witness = txSpend.wit.vtxinwit[0].scriptWitness; + CScriptWitness& witness = txSpend.vin[0].scriptWitness; witness.stack.emplace_back(); key.Sign(SignatureHash(witScriptPubkey, txSpend, 0, SIGHASH_ALL, txCredit.vout[0].nValue, SIGVERSION_WITNESS_V0), witness.stack.back(), 0); witness.stack.back().push_back(static_cast(SIGHASH_ALL)); @@ -80,7 +79,7 @@ static void VerifyScriptBench(benchmark::State& state) bool success = VerifyScript( txSpend.vin[0].scriptSig, txCredit.vout[0].scriptPubKey, - &txSpend.wit.vtxinwit[0].scriptWitness, + &txSpend.vin[0].scriptWitness, flags, MutableTransactionSignatureChecker(&txSpend, 0, txCredit.vout[0].nValue), &err); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 1ed1449f03..816687a45d 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -494,7 +494,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) sigdata = CombineSignatures(prevPubKey, MutableTransactionSignatureChecker(&mergedTx, i, amount), sigdata, DataFromTransaction(txv, i)); UpdateTransaction(mergedTx, i, sigdata); - if (!VerifyScript(txin.scriptSig, prevPubKey, mergedTx.wit.vtxinwit.size() > i ? &mergedTx.wit.vtxinwit[i].scriptWitness : NULL, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) + if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker(&mergedTx, i, amount))) fComplete = false; } diff --git a/src/core_memusage.h b/src/core_memusage.h index 0dcc24c40c..80f6216ee2 100644 --- a/src/core_memusage.h +++ b/src/core_memusage.h @@ -18,35 +18,19 @@ static inline size_t RecursiveDynamicUsage(const COutPoint& out) { } static inline size_t RecursiveDynamicUsage(const CTxIn& in) { - return RecursiveDynamicUsage(in.scriptSig) + RecursiveDynamicUsage(in.prevout); + size_t mem = RecursiveDynamicUsage(in.scriptSig) + RecursiveDynamicUsage(in.prevout) + memusage::DynamicUsage(in.scriptWitness.stack); + for (std::vector >::const_iterator it = in.scriptWitness.stack.begin(); it != in.scriptWitness.stack.end(); it++) { + mem += memusage::DynamicUsage(*it); + } + return mem; } static inline size_t RecursiveDynamicUsage(const CTxOut& out) { return RecursiveDynamicUsage(out.scriptPubKey); } -static inline size_t RecursiveDynamicUsage(const CScriptWitness& scriptWit) { - size_t mem = memusage::DynamicUsage(scriptWit.stack); - for (std::vector >::const_iterator it = scriptWit.stack.begin(); it != scriptWit.stack.end(); it++) { - mem += memusage::DynamicUsage(*it); - } - return mem; -} - -static inline size_t RecursiveDynamicUsage(const CTxInWitness& txinwit) { - return RecursiveDynamicUsage(txinwit.scriptWitness); -} - -static inline size_t RecursiveDynamicUsage(const CTxWitness& txwit) { - size_t mem = memusage::DynamicUsage(txwit.vtxinwit); - for (std::vector::const_iterator it = txwit.vtxinwit.begin(); it != txwit.vtxinwit.end(); it++) { - mem += RecursiveDynamicUsage(*it); - } - return mem; -} - static inline size_t RecursiveDynamicUsage(const CTransaction& tx) { - size_t mem = memusage::DynamicUsage(tx.vin) + memusage::DynamicUsage(tx.vout) + RecursiveDynamicUsage(tx.wit); + size_t mem = memusage::DynamicUsage(tx.vin) + memusage::DynamicUsage(tx.vout); for (std::vector::const_iterator it = tx.vin.begin(); it != tx.vin.end(); it++) { mem += RecursiveDynamicUsage(*it); } @@ -57,7 +41,7 @@ static inline size_t RecursiveDynamicUsage(const CTransaction& tx) { } static inline size_t RecursiveDynamicUsage(const CMutableTransaction& tx) { - size_t mem = memusage::DynamicUsage(tx.vin) + memusage::DynamicUsage(tx.vout) + RecursiveDynamicUsage(tx.wit); + size_t mem = memusage::DynamicUsage(tx.vin) + memusage::DynamicUsage(tx.vout); for (std::vector::const_iterator it = tx.vin.begin(); it != tx.vin.end(); it++) { mem += RecursiveDynamicUsage(*it); } diff --git a/src/core_write.cpp b/src/core_write.cpp index ea01ddc10d..1125718597 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -168,9 +168,9 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry) o.pushKV("asm", ScriptToAsmStr(txin.scriptSig, true)); o.pushKV("hex", HexStr(txin.scriptSig.begin(), txin.scriptSig.end())); in.pushKV("scriptSig", o); - if (!tx.wit.IsNull() && i < tx.wit.vtxinwit.size() && !tx.wit.vtxinwit[i].IsNull()) { + if (!tx.vin[i].scriptWitness.IsNull()) { UniValue txinwitness(UniValue::VARR); - for (const auto& item : tx.wit.vtxinwit[i].scriptWitness.stack) { + for (const auto& item : tx.vin[i].scriptWitness.stack) { txinwitness.push_back(HexStr(item.begin(), item.end())); } in.pushKV("txinwitness", txinwitness); diff --git a/src/miner.cpp b/src/miner.cpp index e80e8a2656..b48b9cee49 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -245,7 +245,7 @@ bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& packa BOOST_FOREACH (const CTxMemPool::txiter it, package) { if (!IsFinalTx(it->GetTx(), nHeight, nLockTimeCutoff)) return false; - if (!fIncludeWitness && !it->GetTx().wit.IsNull()) + if (!fIncludeWitness && it->GetTx().HasWitness()) return false; if (fNeedSizeAccounting) { uint64_t nTxSize = ::GetSerializeSize(it->GetTx(), SER_NETWORK, PROTOCOL_VERSION); @@ -554,7 +554,7 @@ void BlockAssembler::addPriorityTxs() } // cannot accept witness transactions into a non-witness block - if (!fIncludeWitness && !iter->GetTx().wit.IsNull()) + if (!fIncludeWitness && iter->GetTx().HasWitness()) continue; // If tx is dependent on other mempool txs which haven't yet been included diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c37faf49f0..3ff5ccc18f 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1666,7 +1666,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, // Probably non-standard or insufficient fee/priority LogPrint("mempool", " removed orphan tx %s\n", orphanHash.ToString()); vEraseQueue.push_back(orphanHash); - if (orphanTx.wit.IsNull() && !stateDummy.CorruptionPossible()) { + if (!orphanTx.HasWitness() && !stateDummy.CorruptionPossible()) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. @@ -1708,7 +1708,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, LogPrint("mempool", "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); } } else { - if (tx.wit.IsNull() && !state.CorruptionPossible()) { + if (!tx.HasWitness() && !state.CorruptionPossible()) { // Do not use rejection cache for witness transactions or // witness-stripped transactions, as they can have been malleated. // See https://github.com/bitcoin/bitcoin/issues/8279 for details. diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 0c71a079fb..d318a0997c 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -163,7 +163,7 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) { // We don't care if witness for this input is empty, since it must not be bloated. // If the script is invalid without witness, it would be caught sooner or later during validation. - if (tx.wit.vtxinwit[i].IsNull()) + if (tx.vin[i].scriptWitness.IsNull()) continue; const CTxOut &prev = mapInputs.GetOutputFor(tx.vin[i]); @@ -192,13 +192,13 @@ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) // Check P2WSH standard limits if (witnessversion == 0 && witnessprogram.size() == 32) { - if (tx.wit.vtxinwit[i].scriptWitness.stack.back().size() > MAX_STANDARD_P2WSH_SCRIPT_SIZE) + if (tx.vin[i].scriptWitness.stack.back().size() > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return false; - size_t sizeWitnessStack = tx.wit.vtxinwit[i].scriptWitness.stack.size() - 1; + size_t sizeWitnessStack = tx.vin[i].scriptWitness.stack.size() - 1; if (sizeWitnessStack > MAX_STANDARD_P2WSH_STACK_ITEMS) return false; for (unsigned int j = 0; j < sizeWitnessStack; j++) { - if (tx.wit.vtxinwit[i].scriptWitness.stack[j].size() > MAX_STANDARD_P2WSH_STACK_ITEM_SIZE) + if (tx.vin[i].scriptWitness.stack[j].size() > MAX_STANDARD_P2WSH_STACK_ITEM_SIZE) return false; } } diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 11d7eace55..faf3637d74 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -55,7 +55,7 @@ std::string CTxOut::ToString() const } CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {} -CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), wit(tx.wit), nLockTime(tx.nLockTime) {} +CMutableTransaction::CMutableTransaction(const CTransaction& tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime) {} uint256 CMutableTransaction::GetHash() const { @@ -74,8 +74,8 @@ uint256 CTransaction::GetWitnessHash() const /* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */ CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0), hash() {} -CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), wit(tx.wit), nLockTime(tx.nLockTime), hash(ComputeHash()) {} -CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), vin(std::move(tx.vin)), vout(std::move(tx.vout)), wit(std::move(tx.wit)), nLockTime(tx.nLockTime), hash(ComputeHash()) {} +CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), hash(ComputeHash()) {} +CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), vin(std::move(tx.vin)), vout(std::move(tx.vout)), nLockTime(tx.nLockTime), hash(ComputeHash()) {} CAmount CTransaction::GetValueOut() const { @@ -131,8 +131,8 @@ std::string CTransaction::ToString() const nLockTime); for (unsigned int i = 0; i < vin.size(); i++) str += " " + vin[i].ToString() + "\n"; - for (unsigned int i = 0; i < wit.vtxinwit.size(); i++) - str += " " + wit.vtxinwit[i].scriptWitness.ToString() + "\n"; + for (unsigned int i = 0; i < vin.size(); i++) + str += " " + vin[i].scriptWitness.ToString() + "\n"; for (unsigned int i = 0; i < vout.size(); i++) str += " " + vout[i].ToString() + "\n"; return str; diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 66fefafef5..79393fa949 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -65,6 +65,7 @@ public: COutPoint prevout; CScript scriptSig; uint32_t nSequence; + CScriptWitness scriptWitness; //! Only serialized through CTransaction /* Setting nSequence to this value for every input in a transaction * disables nLockTime. */ @@ -211,62 +212,6 @@ public: std::string ToString() const; }; -class CTxInWitness -{ -public: - CScriptWitness scriptWitness; - - ADD_SERIALIZE_METHODS; - - template - inline void SerializationOp(Stream& s, Operation ser_action) - { - READWRITE(scriptWitness.stack); - } - - bool IsNull() const { return scriptWitness.IsNull(); } - - CTxInWitness() { } -}; - -class CTxWitness -{ -public: - /** In case vtxinwit is missing, all entries are treated as if they were empty CTxInWitnesses */ - std::vector vtxinwit; - - ADD_SERIALIZE_METHODS; - - bool IsEmpty() const { return vtxinwit.empty(); } - - bool IsNull() const - { - for (size_t n = 0; n < vtxinwit.size(); n++) { - if (!vtxinwit[n].IsNull()) { - return false; - } - } - return true; - } - - void SetNull() - { - vtxinwit.clear(); - } - - template - inline void SerializationOp(Stream& s, Operation ser_action) - { - for (size_t n = 0; n < vtxinwit.size(); n++) { - READWRITE(vtxinwit[n]); - } - if (IsNull()) { - /* It's illegal to encode a witness when all vtxinwit entries are empty. */ - throw std::ios_base::failure("Superfluous witness record"); - } - } -}; - struct CMutableTransaction; /** @@ -294,7 +239,6 @@ inline void UnserializeTransaction(TxType& tx, Stream& s) { unsigned char flags = 0; tx.vin.clear(); tx.vout.clear(); - tx.wit.SetNull(); /* Try to read the vin. In case the dummy is there, this will be read as an empty vector. */ s >> tx.vin; if (tx.vin.size() == 0 && fAllowWitness) { @@ -311,8 +255,9 @@ inline void UnserializeTransaction(TxType& tx, Stream& s) { if ((flags & 1) && fAllowWitness) { /* The witness flag is present, and we support witnesses. */ flags ^= 1; - tx.wit.vtxinwit.resize(tx.vin.size()); - s >> tx.wit; + for (size_t i = 0; i < tx.vin.size(); i++) { + s >> tx.vin[i].scriptWitness.stack; + } } if (flags) { /* Unknown flag in the serialization */ @@ -328,10 +273,9 @@ inline void SerializeTransaction(const TxType& tx, Stream& s) { s << tx.nVersion; unsigned char flags = 0; // Consistency check - assert(tx.wit.vtxinwit.size() <= tx.vin.size()); if (fAllowWitness) { /* Check whether witnesses need to be serialized. */ - if (!tx.wit.IsNull()) { + if (tx.HasWitness()) { flags |= 1; } } @@ -345,11 +289,7 @@ inline void SerializeTransaction(const TxType& tx, Stream& s) { s << tx.vout; if (flags & 1) { for (size_t i = 0; i < tx.vin.size(); i++) { - if (i < tx.wit.vtxinwit.size()) { - s << tx.wit.vtxinwit[i]; - } else { - s << CTxInWitness(); - } + s << tx.vin[i].scriptWitness.stack; } } s << tx.nLockTime; @@ -379,7 +319,6 @@ public: const int32_t nVersion; const std::vector vin; const std::vector vout; - CTxWitness wit; // Not const: can change without invalidating the txid cache const uint32_t nLockTime; private: @@ -451,6 +390,16 @@ public: } std::string ToString() const; + + bool HasWitness() const + { + for (size_t i = 0; i < vin.size(); i++) { + if (!vin[i].scriptWitness.IsNull()) { + return true; + } + } + return false; + } }; /** A mutable version of CTransaction. */ @@ -459,7 +408,6 @@ struct CMutableTransaction int32_t nVersion; std::vector vin; std::vector vout; - CTxWitness wit; uint32_t nLockTime; CMutableTransaction(); @@ -485,6 +433,21 @@ struct CMutableTransaction * fly, as opposed to GetHash() in CTransaction, which uses a cached result. */ uint256 GetHash() const; + + friend bool operator==(const CMutableTransaction& a, const CMutableTransaction& b) + { + return a.GetHash() == b.GetHash(); + } + + bool HasWitness() const + { + for (size_t i = 0; i < vin.size(); i++) { + if (!vin[i].scriptWitness.IsNull()) { + return true; + } + } + return false; + } }; typedef std::shared_ptr CTransactionRef; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 48769a5335..b13aa4de34 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -82,16 +82,13 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry) o.push_back(Pair("hex", HexStr(txin.scriptSig.begin(), txin.scriptSig.end()))); in.push_back(Pair("scriptSig", o)); } - if (!tx.wit.IsNull()) { - if (!tx.wit.vtxinwit[i].IsNull()) { + if (tx.HasWitness()) { UniValue txinwitness(UniValue::VARR); - for (unsigned int j = 0; j < tx.wit.vtxinwit[i].scriptWitness.stack.size(); j++) { - std::vector item = tx.wit.vtxinwit[i].scriptWitness.stack[j]; + for (unsigned int j = 0; j < tx.vin[i].scriptWitness.stack.size(); j++) { + std::vector item = tx.vin[i].scriptWitness.stack[j]; txinwitness.push_back(HexStr(item.begin(), item.end())); } in.push_back(Pair("txinwitness", txinwitness)); - } - } in.push_back(Pair("sequence", (int64_t)txin.nSequence)); vin.push_back(in); @@ -840,7 +837,7 @@ UniValue signrawtransaction(const JSONRPCRequest& request) UpdateTransaction(mergedTx, i, sigdata); ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, mergedTx.wit.vtxinwit.size() > i ? &mergedTx.wit.vtxinwit[i].scriptWitness : NULL, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { + if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) { TxInErrorToJSON(txin, vErrors, ScriptErrorString(serror)); } } diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 036d6ca7bf..be7fa5d3b7 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -93,8 +93,9 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP // Regardless of the verification result, the tx did not error. set_error(err, bitcoinconsensus_ERR_OK); + PrecomputedTransactionData txdata(tx); - return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), nIn < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[nIn].scriptWitness : NULL, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata), NULL); + return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), &tx.vin[nIn].scriptWitness, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata), NULL); } catch (const std::exception&) { return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing } diff --git a/src/script/script.h b/src/script/script.h index 278774d32e..76419c1495 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -656,6 +656,8 @@ struct CScriptWitness bool IsNull() const { return stack.empty(); } + void SetNull() { stack.clear(); stack.shrink_to_fit(); } + std::string ToString() const; }; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index f552ad5bba..b008df2591 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -194,9 +194,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI SignatureData data; assert(tx.vin.size() > nIn); data.scriptSig = tx.vin[nIn].scriptSig; - if (tx.wit.vtxinwit.size() > nIn) { - data.scriptWitness = tx.wit.vtxinwit[nIn].scriptWitness; - } + data.scriptWitness = tx.vin[nIn].scriptWitness; return data; } @@ -204,10 +202,7 @@ void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const Signatur { assert(tx.vin.size() > nIn); tx.vin[nIn].scriptSig = data.scriptSig; - if (!data.scriptWitness.IsNull() || tx.wit.vtxinwit.size() > nIn) { - tx.wit.vtxinwit.resize(tx.vin.size()); - tx.wit.vtxinwit[nIn].scriptWitness = data.scriptWitness; - } + tx.vin[nIn].scriptWitness = data.scriptWitness; } bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType) diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index eb324d5a6b..84498c5f10 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -145,8 +145,7 @@ CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CSc txSpend.nLockTime = 0; txSpend.vin.resize(1); txSpend.vout.resize(1); - txSpend.wit.vtxinwit.resize(1); - txSpend.wit.vtxinwit[0].scriptWitness = scriptWitness; + txSpend.vin[0].scriptWitness = scriptWitness; txSpend.vin[0].prevout.hash = txCredit.GetHash(); txSpend.vin[0].prevout.n = 0; txSpend.vin[0].scriptSig = scriptSig; diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index 2e974cc612..3488a0befc 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -73,7 +73,7 @@ ScriptError VerifyWithFlag(const CTransaction& output, const CMutableTransaction { ScriptError error; CTransaction inputi(input); - bool ret = VerifyScript(inputi.vin[0].scriptSig, output.vout[0].scriptPubKey, inputi.wit.vtxinwit.size() > 0 ? &inputi.wit.vtxinwit[0].scriptWitness : NULL, flags, TransactionSignatureChecker(&inputi, 0, output.vout[0].nValue), &error); + bool ret = VerifyScript(inputi.vin[0].scriptSig, output.vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output.vout[0].nValue), &error); BOOST_CHECK((ret == true) == (error == SCRIPT_ERR_OK)); return error; @@ -84,13 +84,12 @@ ScriptError VerifyWithFlag(const CTransaction& output, const CMutableTransaction * and witness such that spendingTx spends output zero of creationTx. * Also inserts creationTx's output into the coins view. */ -void BuildTxs(CMutableTransaction& spendingTx, CCoinsViewCache& coins, CMutableTransaction& creationTx, const CScript& scriptPubKey, const CScript& scriptSig, const CTxInWitness& witness) +void BuildTxs(CMutableTransaction& spendingTx, CCoinsViewCache& coins, CMutableTransaction& creationTx, const CScript& scriptPubKey, const CScript& scriptSig, const CScriptWitness& witness) { creationTx.nVersion = 1; creationTx.vin.resize(1); creationTx.vin[0].prevout.SetNull(); creationTx.vin[0].scriptSig = CScript(); - creationTx.wit.vtxinwit.resize(1); creationTx.vout.resize(1); creationTx.vout[0].nValue = 1; creationTx.vout[0].scriptPubKey = scriptPubKey; @@ -100,8 +99,7 @@ void BuildTxs(CMutableTransaction& spendingTx, CCoinsViewCache& coins, CMutableT spendingTx.vin[0].prevout.hash = creationTx.GetHash(); spendingTx.vin[0].prevout.n = 0; spendingTx.vin[0].scriptSig = scriptSig; - spendingTx.wit.vtxinwit.resize(1); - spendingTx.wit.vtxinwit[0] = witness; + spendingTx.vin[0].scriptWitness = witness; spendingTx.vout.resize(1); spendingTx.vout[0].nValue = 1; spendingTx.vout[0].scriptPubKey = CScript(); @@ -133,7 +131,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // Do not use a valid signature to avoid using wallet operations. CScript scriptSig = CScript() << OP_0 << OP_0; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CTxInWitness()); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CScriptWitness()); // Legacy counting only includes signature operations in scriptSigs and scriptPubKeys // of a transaction and does not take the actual executed sig operations into account. // spendingTx in itself does not contain a signature operation. @@ -151,7 +149,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CScript scriptPubKey = GetScriptForDestination(CScriptID(redeemScript)); CScript scriptSig = CScript() << OP_0 << OP_0 << ToByteVector(redeemScript); - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CTxInWitness()); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, CScriptWitness()); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2 * WITNESS_SCALE_FACTOR); assert(VerifyWithFlag(creationTx, spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY); } @@ -161,14 +159,12 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CScript p2pk = CScript() << ToByteVector(pubkey) << OP_CHECKSIG; CScript scriptPubKey = GetScriptForWitness(p2pk); CScript scriptSig = CScript(); - CTxInWitness witness; CScriptWitness scriptWitness; scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(0)); - witness.scriptWitness = scriptWitness; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1); // No signature operations if we don't verify the witness. assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0); @@ -177,10 +173,10 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) // The sig op cost for witness version != 0 is zero. assert(scriptPubKey[0] == 0x00); scriptPubKey[0] = 0x51; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 0); scriptPubKey[0] = 0x00; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); // The witness of a coinbase transaction is not taken into account. spendingTx.vin[0].prevout.SetNull(); @@ -193,13 +189,11 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CScript scriptSig = GetScriptForWitness(p2pk); CScript scriptPubKey = GetScriptForDestination(CScriptID(scriptSig)); scriptSig = CScript() << ToByteVector(scriptSig); - CTxInWitness witness; CScriptWitness scriptWitness; scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(0)); - witness.scriptWitness = scriptWitness; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 1); assert(VerifyWithFlag(creationTx, spendingTx, flags) == SCRIPT_ERR_EQUALVERIFY); } @@ -209,14 +203,12 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CScript witnessScript = CScript() << 1 << ToByteVector(pubkey) << ToByteVector(pubkey) << 2 << OP_CHECKMULTISIGVERIFY; CScript scriptPubKey = GetScriptForWitness(witnessScript); CScript scriptSig = CScript(); - CTxInWitness witness; CScriptWitness scriptWitness; scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(witnessScript.begin(), witnessScript.end())); - witness.scriptWitness = scriptWitness; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags & ~SCRIPT_VERIFY_WITNESS) == 0); assert(VerifyWithFlag(creationTx, spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY); @@ -228,14 +220,12 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CScript redeemScript = GetScriptForWitness(witnessScript); CScript scriptPubKey = GetScriptForDestination(CScriptID(redeemScript)); CScript scriptSig = CScript() << ToByteVector(redeemScript); - CTxInWitness witness; CScriptWitness scriptWitness; scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(0)); scriptWitness.stack.push_back(vector(witnessScript.begin(), witnessScript.end())); - witness.scriptWitness = scriptWitness; - BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, witness); + BuildTxs(spendingTx, coins, creationTx, scriptPubKey, scriptSig, scriptWitness); assert(GetTransactionSigOpCost(CTransaction(spendingTx), coins, flags) == 2); assert(VerifyWithFlag(creationTx, spendingTx, flags) == SCRIPT_ERR_CHECKMULTISIGVERIFY); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index ae5406ef7e..cd9294c7a9 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -170,7 +170,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) amount = mapprevOutValues[tx.vin[i].prevout]; } unsigned int verify_flags = ParseScriptFlags(test[2].get_str()); - const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; + const CScriptWitness *witness = &tx.vin[i].scriptWitness; BOOST_CHECK_MESSAGE(VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, txdata), &err), strTest); @@ -254,7 +254,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) if (mapprevOutValues.count(tx.vin[i].prevout)) { amount = mapprevOutValues[tx.vin[i].prevout]; } - const CScriptWitness *witness = (i < tx.wit.vtxinwit.size()) ? &tx.wit.vtxinwit[i].scriptWitness : NULL; + const CScriptWitness *witness = &tx.vin[i].scriptWitness; fValid = VerifyScript(tx.vin[i].scriptSig, mapprevOutScriptPubKeys[tx.vin[i].prevout], witness, verify_flags, TransactionSignatureChecker(&tx, i, amount, txdata), &err); } @@ -351,7 +351,6 @@ void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, C outputm.vin.resize(1); outputm.vin[0].prevout.SetNull(); outputm.vin[0].scriptSig = CScript(); - outputm.wit.vtxinwit.resize(1); outputm.vout.resize(1); outputm.vout[0].nValue = 1; outputm.vout[0].scriptPubKey = outscript; @@ -362,14 +361,12 @@ void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, C assert(output->vin[0] == outputm.vin[0]); assert(output->vout.size() == 1); assert(output->vout[0] == outputm.vout[0]); - assert(output->wit.vtxinwit.size() == 0); CMutableTransaction inputm; inputm.nVersion = 1; inputm.vin.resize(1); inputm.vin[0].prevout.hash = output->GetHash(); inputm.vin[0].prevout.n = 0; - inputm.wit.vtxinwit.resize(1); inputm.vout.resize(1); inputm.vout[0].nValue = 1; inputm.vout[0].scriptPubKey = CScript(); @@ -382,20 +379,14 @@ void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, C assert(input.vin[0] == inputm.vin[0]); assert(input.vout.size() == 1); assert(input.vout[0] == inputm.vout[0]); - if (inputm.wit.IsNull()) { - assert(input.wit.IsNull()); - } else { - assert(!input.wit.IsNull()); - assert(input.wit.vtxinwit.size() == 1); - assert(input.wit.vtxinwit[0].scriptWitness.stack == inputm.wit.vtxinwit[0].scriptWitness.stack); - } + assert(input.vin[0].scriptWitness.stack == inputm.vin[0].scriptWitness.stack); } void CheckWithFlag(const CTransactionRef& output, const CMutableTransaction& input, int flags, bool success) { ScriptError error; CTransaction inputi(input); - bool ret = VerifyScript(inputi.vin[0].scriptSig, output->vout[0].scriptPubKey, inputi.wit.vtxinwit.size() > 0 ? &inputi.wit.vtxinwit[0].scriptWitness : NULL, flags, TransactionSignatureChecker(&inputi, 0, output->vout[0].nValue), &error); + bool ret = VerifyScript(inputi.vin[0].scriptSig, output->vout[0].scriptPubKey, &inputi.vin[0].scriptWitness, flags, TransactionSignatureChecker(&inputi, 0, output->vout[0].nValue), &error); assert(ret == success); } diff --git a/src/validation.cpp b/src/validation.cpp index a541978c85..9a08018113 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -446,7 +446,7 @@ int64_t GetTransactionSigOpCost(const CTransaction& tx, const CCoinsViewCache& i for (unsigned int i = 0; i < tx.vin.size(); i++) { const CTxOut &prevout = inputs.GetOutputFor(tx.vin[i]); - nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, i < tx.wit.vtxinwit.size() ? &tx.wit.vtxinwit[i].scriptWitness : NULL, flags); + nSigOps += CountWitnessSigOps(tx.vin[i].scriptSig, prevout.scriptPubKey, &tx.vin[i].scriptWitness, flags); } return nSigOps; } @@ -550,7 +550,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // Reject transactions with witness before segregated witness activates (override with -prematurewitness) bool witnessEnabled = IsWitnessEnabled(chainActive.Tip(), Params().GetConsensus()); - if (!GetBoolArg("-prematurewitness",false) && !tx.wit.IsNull() && !witnessEnabled) { + if (!GetBoolArg("-prematurewitness",false) && tx.HasWitness() && !witnessEnabled) { return state.DoS(0, false, REJECT_NONSTANDARD, "no-witness-yet", true); } @@ -672,7 +672,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C return state.Invalid(false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs"); // Check for non-standard witness in P2WSH - if (!tx.wit.IsNull() && fRequireStandard && !IsWitnessStandard(tx, view)) + if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, view)) return state.DoS(0, false, REJECT_NONSTANDARD, "bad-witness-nonstandard", true); int64_t nSigOpsCost = GetTransactionSigOpCost(tx, view, STANDARD_SCRIPT_VERIFY_FLAGS); @@ -912,7 +912,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // SCRIPT_VERIFY_CLEANSTACK requires SCRIPT_VERIFY_WITNESS, so we // need to turn both off, and compare against just turning off CLEANSTACK // to see if the failure is specifically due to witness validation. - if (tx.wit.IsNull() && CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) && + if (!tx.HasWitness() && CheckInputs(tx, state, view, true, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, txdata) && !CheckInputs(tx, state, view, true, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, txdata)) { // Only the witness is missing, so the transaction itself may be fine. state.SetCorruptionPossible(); @@ -1304,7 +1304,7 @@ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight) bool CScriptCheck::operator()() { const CScript &scriptSig = ptxTo->vin[nIn].scriptSig; - const CScriptWitness *witness = (nIn < ptxTo->wit.vtxinwit.size()) ? &ptxTo->wit.vtxinwit[nIn].scriptWitness : NULL; + const CScriptWitness *witness = &ptxTo->vin[nIn].scriptWitness; if (!VerifyScript(scriptSig, scriptPubKey, witness, nFlags, CachingTransactionSignatureChecker(ptxTo, nIn, amount, cacheStore, *txdata), &error)) { return false; } @@ -2846,11 +2846,10 @@ void UpdateUncommittedBlockStructures(CBlock& block, const CBlockIndex* pindexPr { int commitpos = GetWitnessCommitmentIndex(block); static const std::vector nonce(32, 0x00); - if (commitpos != -1 && IsWitnessEnabled(pindexPrev, consensusParams) && block.vtx[0]->wit.IsEmpty()) { + if (commitpos != -1 && IsWitnessEnabled(pindexPrev, consensusParams) && !block.vtx[0]->HasWitness()) { CMutableTransaction tx(*block.vtx[0]); - tx.wit.vtxinwit.resize(1); - tx.wit.vtxinwit[0].scriptWitness.stack.resize(1); - tx.wit.vtxinwit[0].scriptWitness.stack[0] = nonce; + tx.vin[0].scriptWitness.stack.resize(1); + tx.vin[0].scriptWitness.stack[0] = nonce; block.vtx[0] = MakeTransactionRef(std::move(tx)); } } @@ -2958,10 +2957,10 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co // The malleation check is ignored; as the transaction tree itself // already does not permit it, it is impossible to trigger in the // witness tree. - if (block.vtx[0]->wit.vtxinwit.size() != 1 || block.vtx[0]->wit.vtxinwit[0].scriptWitness.stack.size() != 1 || block.vtx[0]->wit.vtxinwit[0].scriptWitness.stack[0].size() != 32) { + if (block.vtx[0]->vin[0].scriptWitness.stack.size() != 1 || block.vtx[0]->vin[0].scriptWitness.stack[0].size() != 32) { return state.DoS(100, false, REJECT_INVALID, "bad-witness-nonce-size", true, strprintf("%s : invalid witness nonce size", __func__)); } - CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->wit.vtxinwit[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); + CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->vin[0].scriptWitness.stack[0][0], 32).Finalize(hashWitness.begin()); if (memcmp(hashWitness.begin(), &block.vtx[0]->vout[commitpos].scriptPubKey[6], 32)) { return state.DoS(100, false, REJECT_INVALID, "bad-witness-merkle-match", true, strprintf("%s : witness merkle commitment mismatch", __func__)); } @@ -2972,7 +2971,7 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co // No witness data is allowed in blocks that don't commit to witness data, as this would otherwise leave room for spam if (!fHaveWitness) { for (size_t i = 0; i < block.vtx.size(); i++) { - if (!block.vtx[i]->wit.IsNull()) { + if (block.vtx[i]->HasWitness()) { return state.DoS(100, false, REJECT_INVALID, "unexpected-witness", true, strprintf("%s : unexpected witness data found", __func__)); } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 638fca9917..2ef4802a10 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2301,7 +2301,6 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt nChangePosInOut = nChangePosRequest; txNew.vin.clear(); txNew.vout.clear(); - txNew.wit.SetNull(); wtxNew.fFromMe = true; bool fFirst = true; @@ -2488,9 +2487,10 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // Remove scriptSigs if we used dummy signatures for fee calculation if (!sign) { - BOOST_FOREACH (CTxIn& vin, txNew.vin) + BOOST_FOREACH (CTxIn& vin, txNew.vin) { vin.scriptSig = CScript(); - txNew.wit.SetNull(); + vin.scriptWitness.SetNull(); + } } // Embed the constructed transaction data in wtxNew.