From 81e3228fcb33e8ed32d8b9fbe917444ba080073a Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 11 Nov 2016 16:23:17 -0800 Subject: [PATCH] Make CTransaction actually immutable --- src/net_processing.cpp | 3 +- src/primitives/transaction.cpp | 27 ++---- src/primitives/transaction.h | 152 +++++++++++++++++--------------- src/script/bitcoinconsensus.cpp | 3 +- src/test/bloom_tests.cpp | 6 +- src/test/script_tests.cpp | 12 +-- src/test/serialize_tests.cpp | 6 +- src/test/sighash_tests.cpp | 6 +- src/test/transaction_tests.cpp | 50 +++++------ src/validation.cpp | 8 +- 10 files changed, 132 insertions(+), 141 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 747167264b..b1283c236c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1594,8 +1594,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, deque vWorkQueue; vector vEraseQueue; - CTransaction tx; - vRecv >> tx; + CTransaction tx(deserialize, vRecv); CInv inv(MSG_TX, tx.GetHash()); pfrom->AddInventoryKnown(inv); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 91f4d29488..11d7eace55 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -62,9 +62,9 @@ uint256 CMutableTransaction::GetHash() const return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); } -void CTransaction::UpdateHash() const +uint256 CTransaction::ComputeHash() const { - *const_cast(&hash) = SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); + return SerializeHash(*this, SER_GETHASH, SERIALIZE_TRANSACTION_NO_WITNESS); } uint256 CTransaction::GetWitnessHash() const @@ -72,25 +72,10 @@ uint256 CTransaction::GetWitnessHash() const return SerializeHash(*this, SER_GETHASH, 0); } -CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), vin(), vout(), nLockTime(0) { } - -CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), vin(tx.vin), vout(tx.vout), wit(tx.wit), nLockTime(tx.nLockTime) { - UpdateHash(); -} - -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) { - UpdateHash(); -} - -CTransaction& CTransaction::operator=(const CTransaction &tx) { - *const_cast(&nVersion) = tx.nVersion; - *const_cast*>(&vin) = tx.vin; - *const_cast*>(&vout) = tx.vout; - *const_cast(&wit) = tx.wit; - *const_cast(&nLockTime) = tx.nLockTime; - *const_cast(&hash) = tx.hash; - return *this; -} +/* 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()) {} CAmount CTransaction::GetValueOut() const { diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 0fa85a1519..66fefafef5 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -286,73 +286,81 @@ struct CMutableTransaction; * - CTxWitness wit; * - uint32_t nLockTime */ -template -inline void SerializeTransaction(TxType& tx, Stream& s, Operation ser_action) { +template +inline void UnserializeTransaction(TxType& tx, Stream& s) { const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS); - READWRITE(*const_cast(&tx.nVersion)); + s >> tx.nVersion; unsigned char flags = 0; - if (ser_action.ForRead()) { - const_cast*>(&tx.vin)->clear(); - const_cast*>(&tx.vout)->clear(); - const_cast(&tx.wit)->SetNull(); - /* Try to read the vin. In case the dummy is there, this will be read as an empty vector. */ - READWRITE(*const_cast*>(&tx.vin)); - if (tx.vin.size() == 0 && fAllowWitness) { - /* We read a dummy or an empty vin. */ - READWRITE(flags); - if (flags != 0) { - READWRITE(*const_cast*>(&tx.vin)); - READWRITE(*const_cast*>(&tx.vout)); - } - } else { - /* We read a non-empty vin. Assume a normal vout follows. */ - READWRITE(*const_cast*>(&tx.vout)); - } - if ((flags & 1) && fAllowWitness) { - /* The witness flag is present, and we support witnesses. */ - flags ^= 1; - const_cast(&tx.wit)->vtxinwit.resize(tx.vin.size()); - READWRITE(tx.wit); - } - if (flags) { - /* Unknown flag in the serialization */ - throw std::ios_base::failure("Unknown transaction optional data"); + 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) { + /* We read a dummy or an empty vin. */ + s >> flags; + if (flags != 0) { + s >> tx.vin; + s >> tx.vout; } } else { - // Consistency check - assert(tx.wit.vtxinwit.size() <= tx.vin.size()); - if (fAllowWitness) { - /* Check whether witnesses need to be serialized. */ - if (!tx.wit.IsNull()) { - flags |= 1; - } - } - if (flags) { - /* Use extended format in case witnesses are to be serialized. */ - std::vector vinDummy; - READWRITE(vinDummy); - READWRITE(flags); - } - READWRITE(*const_cast*>(&tx.vin)); - READWRITE(*const_cast*>(&tx.vout)); - if (flags & 1) { - const_cast(&tx.wit)->vtxinwit.resize(tx.vin.size()); - READWRITE(tx.wit); + /* We read a non-empty vin. Assume a normal vout follows. */ + s >> tx.vout; + } + 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; + } + if (flags) { + /* Unknown flag in the serialization */ + throw std::ios_base::failure("Unknown transaction optional data"); + } + s >> tx.nLockTime; +} + +template +inline void SerializeTransaction(const TxType& tx, Stream& s) { + const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS); + + 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()) { + flags |= 1; } } - READWRITE(*const_cast(&tx.nLockTime)); + if (flags) { + /* Use extended format in case witnesses are to be serialized. */ + std::vector vinDummy; + s << vinDummy; + s << flags; + } + s << tx.vin; + 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.nLockTime; } + /** The basic transaction that is broadcasted on the network and contained in * blocks. A transaction can contain multiple inputs and outputs. */ class CTransaction { -private: - /** Memory only. */ - const uint256 hash; - public: // Default transaction version. static const int32_t CURRENT_VERSION=1; @@ -374,6 +382,13 @@ public: CTxWitness wit; // Not const: can change without invalidating the txid cache const uint32_t nLockTime; +private: + /** Memory only. */ + const uint256 hash; + + uint256 ComputeHash() const; + +public: /** Construct a CTransaction that qualifies as IsNull() */ CTransaction(); @@ -381,18 +396,13 @@ public: CTransaction(const CMutableTransaction &tx); CTransaction(CMutableTransaction &&tx); - CTransaction& operator=(const CTransaction& tx); - - ADD_SERIALIZE_METHODS; - - template - inline void SerializationOp(Stream& s, Operation ser_action) { - SerializeTransaction(*this, s, ser_action); - if (ser_action.ForRead()) { - UpdateHash(); - } + template + inline void Serialize(Stream& s) const { + SerializeTransaction(*this, s); } + /** This deserializing constructor is provided instead of an Unserialize method. + * Unserialize is not possible, since it would require overwriting const fields. */ template CTransaction(deserialize_type, Stream& s) : CTransaction(CMutableTransaction(deserialize, s)) {} @@ -417,7 +427,7 @@ public: // Compute modified tx size for priority calculation (optionally given tx size) unsigned int CalculateModifiedSize(unsigned int nTxSize=0) const; - + /** * Get the total transaction size in bytes, including witness data. * "Total Size" defined in BIP141 and BIP144. @@ -441,8 +451,6 @@ public: } std::string ToString() const; - - void UpdateHash() const; }; /** A mutable version of CTransaction. */ @@ -457,11 +465,15 @@ struct CMutableTransaction CMutableTransaction(); CMutableTransaction(const CTransaction& tx); - ADD_SERIALIZE_METHODS; + template + inline void Serialize(Stream& s) const { + SerializeTransaction(*this, s); + } - template - inline void SerializationOp(Stream& s, Operation ser_action) { - SerializeTransaction(*this, s, ser_action); + + template + inline void Unserialize(Stream& s) { + UnserializeTransaction(*this, s); } template diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 069ac55bfb..036d6ca7bf 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -85,8 +85,7 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP } try { TxInputStream stream(SER_NETWORK, PROTOCOL_VERSION, txTo, txToLen); - CTransaction tx; - stream >> tx; + CTransaction tx(deserialize, stream); if (nIn >= tx.vin.size()) return set_error(err, bitcoinconsensus_ERR_TX_INDEX); if (GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) != txToLen) diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index ffa1fb6ad0..b15ff9e44b 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -114,16 +114,14 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_key) BOOST_AUTO_TEST_CASE(bloom_match) { // Random real transaction (b4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b) - CTransaction tx; CDataStream stream(ParseHex("01000000010b26e9b7735eb6aabdf358bab62f9816a21ba9ebdb719d5299e88607d722c190000000008b4830450220070aca44506c5cef3a16ed519d7c3c39f8aab192c4e1c90d065f37b8a4af6141022100a8e160b856c2d43d27d8fba71e5aef6405b8643ac4cb7cb3c462aced7f14711a0141046d11fee51b0e60666d5049a9101a72741df480b96ee26488a4d3466b95c9a40ac5eeef87e10a5cd336c19a84565f80fa6c547957b7700ff4dfbdefe76036c339ffffffff021bff3d11000000001976a91404943fdd508053c75000106d3bc6e2754dbcff1988ac2f15de00000000001976a914a266436d2965547608b9e15d9032a7b9d64fa43188ac00000000"), SER_DISK, CLIENT_VERSION); - stream >> tx; + CTransaction tx(deserialize, stream); // and one which spends it (e2769b09e784f32f62ef849763d4f45b98e07ba658647343b915ff832b110436) unsigned char ch[] = {0x01, 0x00, 0x00, 0x00, 0x01, 0x6b, 0xff, 0x7f, 0xcd, 0x4f, 0x85, 0x65, 0xef, 0x40, 0x6d, 0xd5, 0xd6, 0x3d, 0x4f, 0xf9, 0x4f, 0x31, 0x8f, 0xe8, 0x20, 0x27, 0xfd, 0x4d, 0xc4, 0x51, 0xb0, 0x44, 0x74, 0x01, 0x9f, 0x74, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x8c, 0x49, 0x30, 0x46, 0x02, 0x21, 0x00, 0xda, 0x0d, 0xc6, 0xae, 0xce, 0xfe, 0x1e, 0x06, 0xef, 0xdf, 0x05, 0x77, 0x37, 0x57, 0xde, 0xb1, 0x68, 0x82, 0x09, 0x30, 0xe3, 0xb0, 0xd0, 0x3f, 0x46, 0xf5, 0xfc, 0xf1, 0x50, 0xbf, 0x99, 0x0c, 0x02, 0x21, 0x00, 0xd2, 0x5b, 0x5c, 0x87, 0x04, 0x00, 0x76, 0xe4, 0xf2, 0x53, 0xf8, 0x26, 0x2e, 0x76, 0x3e, 0x2d, 0xd5, 0x1e, 0x7f, 0xf0, 0xbe, 0x15, 0x77, 0x27, 0xc4, 0xbc, 0x42, 0x80, 0x7f, 0x17, 0xbd, 0x39, 0x01, 0x41, 0x04, 0xe6, 0xc2, 0x6e, 0xf6, 0x7d, 0xc6, 0x10, 0xd2, 0xcd, 0x19, 0x24, 0x84, 0x78, 0x9a, 0x6c, 0xf9, 0xae, 0xa9, 0x93, 0x0b, 0x94, 0x4b, 0x7e, 0x2d, 0xb5, 0x34, 0x2b, 0x9d, 0x9e, 0x5b, 0x9f, 0xf7, 0x9a, 0xff, 0x9a, 0x2e, 0xe1, 0x97, 0x8d, 0xd7, 0xfd, 0x01, 0xdf, 0xc5, 0x22, 0xee, 0x02, 0x28, 0x3d, 0x3b, 0x06, 0xa9, 0xd0, 0x3a, 0xcf, 0x80, 0x96, 0x96, 0x8d, 0x7d, 0xbb, 0x0f, 0x91, 0x78, 0xff, 0xff, 0xff, 0xff, 0x02, 0x8b, 0xa7, 0x94, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x19, 0x76, 0xa9, 0x14, 0xba, 0xde, 0xec, 0xfd, 0xef, 0x05, 0x07, 0x24, 0x7f, 0xc8, 0xf7, 0x42, 0x41, 0xd7, 0x3b, 0xc0, 0x39, 0x97, 0x2d, 0x7b, 0x88, 0xac, 0x40, 0x94, 0xa8, 0x02, 0x00, 0x00, 0x00, 0x00, 0x19, 0x76, 0xa9, 0x14, 0xc1, 0x09, 0x32, 0x48, 0x3f, 0xec, 0x93, 0xed, 0x51, 0xf5, 0xfe, 0x95, 0xe7, 0x25, 0x59, 0xf2, 0xcc, 0x70, 0x43, 0xf9, 0x88, 0xac, 0x00, 0x00, 0x00, 0x00, 0x00}; vector vch(ch, ch + sizeof(ch) -1); CDataStream spendStream(vch, SER_DISK, CLIENT_VERSION); - CTransaction spendingTx; - spendStream >> spendingTx; + CTransaction spendingTx(deserialize, spendStream); CBloomFilter filter(10, 0.000001, 0, BLOOM_UPDATE_ALL); filter.insert(uint256S("0xb4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b")); diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 3169afb13a..eb324d5a6b 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -276,7 +276,7 @@ private: //! The Witness embedded script CScript witscript; CScriptWitness scriptWitness; - CTransaction creditTx; + CTransactionRef creditTx; CMutableTransaction spendTx; bool havePush; std::vector push; @@ -319,8 +319,8 @@ public: redeemscript = scriptPubKey; scriptPubKey = CScript() << OP_HASH160 << ToByteVector(CScriptID(redeemscript)) << OP_EQUAL; } - creditTx = BuildCreditingTransaction(scriptPubKey, nValue); - spendTx = BuildSpendingTransaction(CScript(), CScriptWitness(), creditTx); + creditTx = MakeTransactionRef(BuildCreditingTransaction(scriptPubKey, nValue)); + spendTx = BuildSpendingTransaction(CScript(), CScriptWitness(), *creditTx); } TestBuilder& ScriptError(ScriptError_t err) @@ -421,7 +421,7 @@ public: { TestBuilder copy = *this; // Make a copy so we can rollback the push. DoPush(); - DoTest(creditTx.vout[0].scriptPubKey, spendTx.vin[0].scriptSig, scriptWitness, flags, comment, scriptError, nValue); + DoTest(creditTx->vout[0].scriptPubKey, spendTx.vin[0].scriptSig, scriptWitness, flags, comment, scriptError, nValue); *this = copy; return *this; } @@ -447,7 +447,7 @@ public: array.push_back(wit); } array.push_back(FormatScript(spendTx.vin[0].scriptSig)); - array.push_back(FormatScript(creditTx.vout[0].scriptPubKey)); + array.push_back(FormatScript(creditTx->vout[0].scriptPubKey)); array.push_back(FormatScriptFlags(flags)); array.push_back(FormatScriptError((ScriptError_t)scriptError)); array.push_back(comment); @@ -461,7 +461,7 @@ public: const CScript& GetScriptPubKey() { - return creditTx.vout[0].scriptPubKey; + return creditTx->vout[0].scriptPubKey; } }; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index bbadf57957..1dc86eb116 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -21,10 +21,10 @@ protected: bool boolval; std::string stringval; const char* charstrval; - CTransaction txval; + CTransactionRef txval; public: CSerializeMethodsTestSingle() = default; - CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const char* charstrvalin, CTransaction txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), charstrval(charstrvalin), txval(txvalin){} + CSerializeMethodsTestSingle(int intvalin, bool boolvalin, std::string stringvalin, const char* charstrvalin, CTransaction txvalin) : intval(intvalin), boolval(boolvalin), stringval(std::move(stringvalin)), charstrval(charstrvalin), txval(MakeTransactionRef(txvalin)){} ADD_SERIALIZE_METHODS; template @@ -42,7 +42,7 @@ public: boolval == rhs.boolval && \ stringval == rhs.stringval && \ strcmp(charstrval, rhs.charstrval) == 0 && \ - txval == rhs.txval; + *txval == *rhs.txval; } }; diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 38d58e7ab0..a524f5b946 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -184,7 +184,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) std::string raw_tx, raw_script, sigHashHex; int nIn, nHashType; uint256 sh; - CTransaction tx; + CTransactionRef tx; CScript scriptCode = CScript(); try { @@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) stream >> tx; CValidationState state; - BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); + BOOST_CHECK_MESSAGE(CheckTransaction(*tx, state), strTest); BOOST_CHECK(state.IsValid()); std::vector raw = ParseHex(raw_script); @@ -209,7 +209,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) continue; } - sh = SignatureHash(scriptCode, tx, nIn, nHashType, 0, SIGVERSION_BASE); + sh = SignatureHash(scriptCode, *tx, nIn, nHashType, 0, SIGVERSION_BASE); BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest); } } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 7b4763669a..ae5406ef7e 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -150,8 +150,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) string transaction = test[1].get_str(); CDataStream stream(ParseHex(transaction), SER_NETWORK, PROTOCOL_VERSION); - CTransaction tx; - stream >> tx; + CTransaction tx(deserialize, stream); CValidationState state; BOOST_CHECK_MESSAGE(CheckTransaction(tx, state), strTest); @@ -236,8 +235,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) string transaction = test[1].get_str(); CDataStream stream(ParseHex(transaction), SER_NETWORK, PROTOCOL_VERSION ); - CTransaction tx; - stream >> tx; + CTransaction tx(deserialize, stream); CValidationState state; fValid = CheckTransaction(tx, state) && state.IsValid(); @@ -346,7 +344,7 @@ BOOST_AUTO_TEST_CASE(test_Get) BOOST_CHECK_EQUAL(coins.GetValueIn(t1), (50+21+22)*CENT); } -void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, CTransaction& output, CMutableTransaction& input, bool success = true) +void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true) { CMutableTransaction outputm; outputm.nVersion = 1; @@ -360,22 +358,22 @@ void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, C CDataStream ssout(SER_NETWORK, PROTOCOL_VERSION); ssout << outputm; ssout >> output; - assert(output.vin.size() == 1); - 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); + assert(output->vin.size() == 1); + 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.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(); - bool ret = SignSignature(keystore, output, inputm, 0, SIGHASH_ALL); + bool ret = SignSignature(keystore, *output, inputm, 0, SIGHASH_ALL); assert(ret == success); CDataStream ssin(SER_NETWORK, PROTOCOL_VERSION); ssin << inputm; @@ -393,11 +391,11 @@ void CreateCreditAndSpend(const CKeyStore& keystore, const CScript& outscript, C } } -void CheckWithFlag(const CTransaction& output, const CMutableTransaction& input, int flags, bool success) +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.wit.vtxinwit.size() > 0 ? &inputi.wit.vtxinwit[0].scriptWitness : NULL, flags, TransactionSignatureChecker(&inputi, 0, output->vout[0].nValue), &error); assert(ret == success); } @@ -466,10 +464,10 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { assert(hashSigned); } - CTransaction tx; CDataStream ssout(SER_NETWORK, PROTOCOL_VERSION); - WithOrVersion(&ssout, 0) << mtx; - WithOrVersion(&ssout, 0) >> tx; + auto vstream = WithOrVersion(&ssout, 0); + vstream << mtx; + CTransaction tx(deserialize, vstream); // check all inputs concurrently, with the cache PrecomputedTransactionData txdata(tx); @@ -547,7 +545,7 @@ BOOST_AUTO_TEST_CASE(test_witness) keystore2.AddCScript(GetScriptForWitness(scriptMulti)); keystore2.AddKeyPubKey(key3, pubkey3); - CTransaction output1, output2; + CTransactionRef output1, output2; CMutableTransaction input1, input2; SignatureData sigdata; @@ -639,8 +637,8 @@ BOOST_AUTO_TEST_CASE(test_witness) CheckWithFlag(output1, input1, 0, false); CreateCreditAndSpend(keystore2, scriptMulti, output2, input2, false); CheckWithFlag(output2, input2, 0, false); - BOOST_CHECK(output1 == output2); - UpdateTransaction(input1, 0, CombineSignatures(output1.vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1.vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); + BOOST_CHECK(*output1 == *output2); + UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); // P2SH 2-of-2 multisig @@ -650,8 +648,8 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore2, GetScriptForDestination(CScriptID(scriptMulti)), output2, input2, false); CheckWithFlag(output2, input2, 0, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, false); - BOOST_CHECK(output1 == output2); - UpdateTransaction(input1, 0, CombineSignatures(output1.vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1.vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); + BOOST_CHECK(*output1 == *output2); + UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); @@ -662,8 +660,8 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore2, GetScriptForWitness(scriptMulti), output2, input2, false); CheckWithFlag(output2, input2, 0, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); - BOOST_CHECK(output1 == output2); - UpdateTransaction(input1, 0, CombineSignatures(output1.vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1.vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); + BOOST_CHECK(*output1 == *output2); + UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); @@ -674,8 +672,8 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore2, GetScriptForDestination(CScriptID(GetScriptForWitness(scriptMulti))), output2, input2, false); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); - BOOST_CHECK(output1 == output2); - UpdateTransaction(input1, 0, CombineSignatures(output1.vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1.vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); + BOOST_CHECK(*output1 == *output2); + UpdateTransaction(input1, 0, CombineSignatures(output1->vout[0].scriptPubKey, MutableTransactionSignatureChecker(&input1, 0, output1->vout[0].nValue), DataFromTransaction(input1, 0), DataFromTransaction(input2, 0))); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); } diff --git a/src/validation.cpp b/src/validation.cpp index fe34565131..7eef14af2b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2859,8 +2859,9 @@ std::vector GenerateCoinbaseCommitment(CBlock& block, const CBloc out.scriptPubKey[5] = 0xed; memcpy(&out.scriptPubKey[6], witnessroot.begin(), 32); commitment = std::vector(out.scriptPubKey.begin(), out.scriptPubKey.end()); - const_cast*>(&block.vtx[0]->vout)->push_back(out); - block.vtx[0]->UpdateHash(); + CMutableTransaction tx(*block.vtx[0]); + tx.vout.push_back(out); + block.vtx[0] = MakeTransactionRef(std::move(tx)); } } UpdateUncommittedBlockStructures(block, pindexPrev, consensusParams); @@ -4090,10 +4091,9 @@ bool LoadMempool(void) file >> num; double prioritydummy = 0; while (num--) { - CTransaction tx; int64_t nTime; int64_t nFeeDelta; - file >> tx; + CTransaction tx(deserialize, file); file >> nTime; file >> nFeeDelta;