From b1e3a3850d0b64fe901bd977f3789b28f307850a Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 1 Feb 2021 01:25:57 -0300 Subject: [PATCH 1/3] Switch GetTransaction to returning a CTransactionRef --- src/budget/budgetmanager.cpp | 12 ++++++------ src/masternode.cpp | 6 +++--- src/rest.cpp | 4 ++-- src/rpc/blockchain.cpp | 4 ++-- src/rpc/rawtransaction.cpp | 10 +++++----- src/stakeinput.cpp | 4 ++-- src/validation.cpp | 28 ++++++++++++++-------------- src/validation.h | 4 ++-- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp index 64dc151a5ed1..f8d7bee8a347 100644 --- a/src/budget/budgetmanager.cpp +++ b/src/budget/budgetmanager.cpp @@ -1224,23 +1224,23 @@ bool CheckCollateralConfs(const uint256& nTxCollateralHash, int nCurrentHeight, bool CheckCollateral(const uint256& nTxCollateralHash, const uint256& nExpectedHash, std::string& strError, int64_t& nTime, int nCurrentHeight, bool fBudgetFinalization) { - CTransaction txCollateral; + CTransactionRef txCollateral; uint256 nBlockHash; if (!GetTransaction(nTxCollateralHash, txCollateral, nBlockHash, true)) { strError = strprintf("Can't find collateral tx %s", nTxCollateralHash.ToString()); return false; } - if (txCollateral.vout.size() < 1) return false; - if (txCollateral.nLockTime != 0) return false; + if (txCollateral->vout.size() < 1) return false; + if (txCollateral->nLockTime != 0) return false; CScript findScript; findScript << OP_RETURN << ToByteVector(nExpectedHash); bool foundOpReturn = false; - for (const CTxOut &o : txCollateral.vout) { + for (const CTxOut &o : txCollateral->vout) { if (!o.scriptPubKey.IsNormalPaymentScript() && !o.scriptPubKey.IsUnspendable()) { - strError = strprintf("Invalid Script %s", txCollateral.ToString()); + strError = strprintf("Invalid Script %s", txCollateral->ToString()); return false; } if (fBudgetFinalization) { @@ -1267,7 +1267,7 @@ bool CheckCollateral(const uint256& nTxCollateralHash, const uint256& nExpectedH } if (!foundOpReturn) { - strError = strprintf("Couldn't find opReturn %s in %s", nExpectedHash.ToString(), txCollateral.ToString()); + strError = strprintf("Couldn't find opReturn %s in %s", nExpectedHash.ToString(), txCollateral->ToString()); return false; } diff --git a/src/masternode.cpp b/src/masternode.cpp index f2dfd0c89832..8b2b0ace4e60 100644 --- a/src/masternode.cpp +++ b/src/masternode.cpp @@ -191,10 +191,10 @@ bool CMasternode::IsInputAssociatedWithPubkey() const CScript payee; payee = GetScriptForDestination(pubKeyCollateralAddress.GetID()); - CTransaction txVin; + CTransactionRef txVin; uint256 hash; if(GetTransaction(vin.prevout.hash, txVin, hash, true)) { - for (CTxOut out : txVin.vout) { + for (CTxOut out : txVin->vout) { if (out.nValue == 10000 * COIN && out.scriptPubKey == payee) return true; } } @@ -517,7 +517,7 @@ bool CMasternodeBroadcast::CheckInputsAndAdd(int& nDoS) // verify that sig time is legit in past // should be at least not earlier than block when 1000 PIV tx got MASTERNODE_MIN_CONFIRMATIONS uint256 hashBlock = UINT256_ZERO; - CTransaction tx2; + CTransactionRef tx2; GetTransaction(vin.prevout.hash, tx2, hashBlock, true); BlockMap::iterator mi = mapBlockIndex.find(hashBlock); if (mi != mapBlockIndex.end() && (*mi).second) { diff --git a/src/rest.cpp b/src/rest.cpp index 8804758f3dc6..ae8c164ea720 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -357,7 +357,7 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart) if (!ParseHashStr(hashStr, hash)) return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr); - CTransaction tx; + CTransactionRef tx; uint256 hashBlock = uint256(); if (!GetTransaction(hash, tx, hashBlock, true)) return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); @@ -382,7 +382,7 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart) case RF_JSON: { UniValue objTx(UniValue::VOBJ); - TxToJSON(tx, hashBlock, objTx); + TxToJSON(*tx, hashBlock, objTx); std::string strJSON = objTx.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); req->WriteReply(HTTP_OK, strJSON); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f060a1b14e59..9c5e099dec21 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1351,11 +1351,11 @@ UniValue getblockindexstats(const JSONRPCRequest& request) { // Transparent inputs for (unsigned int j = 0; j < tx.vin.size(); j++) { COutPoint prevout = tx.vin[j].prevout; - CTransaction txPrev; + CTransactionRef txPrev; uint256 hashBlock; if(!GetTransaction(prevout.hash, txPrev, hashBlock, true)) throw JSONRPCError(RPC_DATABASE_ERROR, "failed to read tx from disk"); - nValueIn += txPrev.vout[prevout.n].nValue; + nValueIn += txPrev->vout[prevout.n].nValue; } // Shield inputs nValueIn += tx.GetShieldedValueIn(); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 2f5cd90ead82..d30d3fa3eec9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -221,7 +221,7 @@ UniValue getrawtransaction(const JSONRPCRequest& request) in_active_chain = chainActive.Contains(blockindex); } - CTransaction tx; + CTransactionRef tx; uint256 hash_block; if (!GetTransaction(hash, tx, hash_block, true, blockindex)) { std::string errmsg; @@ -239,12 +239,12 @@ UniValue getrawtransaction(const JSONRPCRequest& request) } if (!fVerbose) { - return EncodeHexTx(tx); + return EncodeHexTx(*tx); } UniValue result(UniValue::VOBJ); if (blockindex) result.pushKV("in_active_chain", in_active_chain); - TxToJSON(tx, hash_block, result); + TxToJSON(*tx, hash_block, result); return result; } @@ -678,11 +678,11 @@ UniValue signrawtransaction(const JSONRPCRequest& request) if (Params().IsRegTestNet()) { for (const CTxIn &txbase : mergedTx.vin) { - CTransaction tempTx; + CTransactionRef tempTx; uint256 hashBlock; if (GetTransaction(txbase.prevout.hash, tempTx, hashBlock, true)) { // Copy results into mapPrevOut: - mapPrevOut[txbase.prevout] = std::make_pair(tempTx.vout[txbase.prevout.n].scriptPubKey, tempTx.vout[txbase.prevout.n].nValue); + mapPrevOut[txbase.prevout] = std::make_pair(tempTx->vout[txbase.prevout.n].scriptPubKey, tempTx->vout[txbase.prevout.n].nValue); } } } diff --git a/src/stakeinput.cpp b/src/stakeinput.cpp index dd6f241b2566..cef92e7854ca 100644 --- a/src/stakeinput.cpp +++ b/src/stakeinput.cpp @@ -18,7 +18,7 @@ CPivStake* CPivStake::NewPivStake(const CTxIn& txin) // Find the previous transaction in database uint256 hashBlock; - CTransaction txPrev; + CTransactionRef txPrev; if (!GetTransaction(txin.prevout.hash, txPrev, hashBlock, true)) { error("%s : INFO: read txPrev failed, tx id prev: %s", __func__, txin.prevout.hash.GetHex()); return nullptr; @@ -36,7 +36,7 @@ CPivStake* CPivStake::NewPivStake(const CTxIn& txin) return nullptr; } - return new CPivStake(txPrev.vout[txin.prevout.n], + return new CPivStake(txPrev->vout[txin.prevout.n], txin.prevout, pindexFrom); } diff --git a/src/validation.cpp b/src/validation.cpp index b5c6be80f066..7b5df9b9360c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -634,20 +634,20 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa bool GetOutput(const uint256& hash, unsigned int index, CValidationState& state, CTxOut& out) { - CTransaction txPrev; + CTransactionRef txPrev; uint256 hashBlock; if (!GetTransaction(hash, txPrev, hashBlock, true)) { return state.DoS(100, error("Output not found")); } - if (index > txPrev.vout.size()) { + if (index > txPrev->vout.size()) { return state.DoS(100, error("Output not found, invalid index %d for %s",index, hash.GetHex())); } - out = txPrev.vout[index]; + out = txPrev->vout[index]; return true; } /** Return transaction in tx, and if it was found inside a block, its hash is placed in hashBlock */ -bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock, bool fAllowSlow, CBlockIndex* blockIndex) +bool GetTransaction(const uint256& hash, CTransactionRef& txOut, uint256& hashBlock, bool fAllowSlow, CBlockIndex* blockIndex) { CBlockIndex* pindexSlow = blockIndex; @@ -657,7 +657,7 @@ bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock CTransactionRef ptx = mempool.get(hash); if (ptx) { - txOut = *ptx; + txOut = ptx; return true; } @@ -676,7 +676,7 @@ bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock return error("%s : Deserialize or I/O error - %s", __func__, e.what()); } hashBlock = header.GetHash(); - if (txOut.GetHash() != hash) + if (txOut->GetHash() != hash) return error("%s : txid mismatch", __func__); return true; } @@ -696,7 +696,7 @@ bool GetTransaction(const uint256& hash, CTransaction& txOut, uint256& hashBlock if (ReadBlockFromDisk(block, pindexSlow)) { for (const auto& tx : block.vtx) { if (tx->GetHash() == hash) { - txOut = *tx; + txOut = tx; hashBlock = pindexSlow->GetBlockHash(); return true; } @@ -1037,15 +1037,15 @@ void AddInvalidSpendsToMap(const CBlock& block) if (zerocoinDB->ReadCoinSpend(bnActualSerial, txHash)) { mapInvalidSerials[bnActualSerial] = spend->getDenomination() * COIN; - CTransaction txPrev; + CTransactionRef txPrev; uint256 hashBlock; if (!GetTransaction(txHash, txPrev, hashBlock, true)) continue; //Record all txouts from txPrev as invalid - for (unsigned int i = 0; i < txPrev.vout.size(); i++) { + for (unsigned int i = 0; i < txPrev->vout.size(); i++) { //map to an empty outpoint to represent that this is the first in the chain of bad outs - mapInvalidOutPoints[COutPoint(txPrev.GetHash(), i)] = COutPoint(); + mapInvalidOutPoints[COutPoint(txPrev->GetHash(), i)] = COutPoint(); } } @@ -2669,10 +2669,10 @@ bool CheckColdStakeFreeOutput(const CTransaction& tx, const int nHeight) // so we check that the staker is not transferring value to the free output if (!masternodeSync.IsSynced()) { // First try finding the previous transaction in database - CTransaction txPrev; uint256 hashBlock; + CTransactionRef txPrev; uint256 hashBlock; if (!GetTransaction(tx.vin[0].prevout.hash, txPrev, hashBlock, true)) return error("%s : read txPrev failed: %s", __func__, tx.vin[0].prevout.hash.GetHex()); - CAmount amtIn = txPrev.vout[tx.vin[0].prevout.n].nValue + GetBlockValue(nHeight); + CAmount amtIn = txPrev->vout[tx.vin[0].prevout.n].nValue + GetBlockValue(nHeight); CAmount amtOut = 0; for (unsigned int i = 1; i < outs-1; i++) amtOut += tx.vout[i].nValue; if (amtOut != amtIn) @@ -2988,7 +2988,7 @@ bool IsBlockHashInChain(const uint256& hashBlock) return chainActive.Contains(mapBlockIndex[hashBlock]); } -bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransaction& tx) +bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransactionRef& tx) { uint256 hashBlock; if (!GetTransaction(txId, tx, hashBlock, true)) @@ -3002,7 +3002,7 @@ bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransaction& tx) bool IsTransactionInChain(const uint256& txId, int& nHeightTx) { - CTransaction tx; + CTransactionRef tx; return IsTransactionInChain(txId, nHeightTx, tx); } diff --git a/src/validation.h b/src/validation.h index 27c6c17b007e..a8ed972cd324 100644 --- a/src/validation.h +++ b/src/validation.h @@ -197,7 +197,7 @@ void ThreadScriptCheck(); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload(); /** Retrieve a transaction (from memory pool, or from disk, if possible) */ -bool GetTransaction(const uint256& hash, CTransaction& tx, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr); +bool GetTransaction(const uint256& hash, CTransactionRef& tx, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr); /** Retrieve an output (from memory pool, or from disk, if possible) */ bool GetOutput(const uint256& hash, unsigned int index, CValidationState& state, CTxOut& out); @@ -254,7 +254,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState& state, const CCoinsVi /** Apply the effects of this transaction on the UTXO set represented by view */ void UpdateCoins(const CTransaction& tx, CCoinsViewCache& inputs, int nHeight); -bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransaction& tx); +bool IsTransactionInChain(const uint256& txId, int& nHeightTx, CTransactionRef& tx); bool IsTransactionInChain(const uint256& txId, int& nHeightTx); bool IsBlockHashInChain(const uint256& hashBlock); bool ValidOutPoint(const COutPoint& out, int nHeight); From 896b5e2f115e49e881f606ab548363bef079dab3 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 1 Feb 2021 01:33:52 -0300 Subject: [PATCH 2/3] Make DecodeHexTx return a CMutableTransaction --- src/core_io.h | 3 ++- src/core_read.cpp | 2 +- src/pivx-tx.cpp | 6 ++---- src/rpc/rawtransaction.cpp | 18 +++++++++--------- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/core_io.h b/src/core_io.h index 21426e50a31e..7a1ae92321b0 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -12,13 +12,14 @@ class CBlock; class CScript; class CTransaction; +class CMutableTransaction; class uint256; class UniValue; // core_read.cpp extern CScript ParseScript(std::string s); extern std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); -extern bool DecodeHexTx(CTransaction& tx, const std::string& strHexTx); +extern bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx); extern bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); extern uint256 ParseHashUV(const UniValue& v, const std::string& strName); extern uint256 ParseHashStr(const std::string&, const std::string& strName); diff --git a/src/core_read.cpp b/src/core_read.cpp index 4a98773b930a..5bd659881da5 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -74,7 +74,7 @@ CScript ParseScript(std::string s) return result; } -bool DecodeHexTx(CTransaction& tx, const std::string& strHexTx) +bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx) { if (!IsHex(strHexTx)) return false; diff --git a/src/pivx-tx.cpp b/src/pivx-tx.cpp index 21ef09d3ef38..89040fe6bbaf 100644 --- a/src/pivx-tx.cpp +++ b/src/pivx-tx.cpp @@ -592,7 +592,7 @@ static int CommandLineRawTx(int argc, char* argv[]) argv++; } - CTransaction txDecodeTmp; + CMutableTransaction tx; int startArg; if (!fCreateBlank) { @@ -605,15 +605,13 @@ static int CommandLineRawTx(int argc, char* argv[]) if (strHexTx == "-") // "-" implies standard input strHexTx = readStdin(); - if (!DecodeHexTx(txDecodeTmp, strHexTx)) + if (!DecodeHexTx(tx, strHexTx)) throw std::runtime_error("invalid transaction encoding"); startArg = 2; } else startArg = 1; - CMutableTransaction tx(txDecodeTmp); - for (int i = startArg; i < argc; i++) { std::string arg = argv[i]; std::string key, value; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d30d3fa3eec9..1afdabe60772 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -406,13 +406,13 @@ UniValue decoderawtransaction(const JSONRPCRequest& request) LOCK(cs_main); RPCTypeCheck(request.params, {UniValue::VSTR}); - CTransaction tx; + CMutableTransaction mtx; - if (!DecodeHexTx(tx, request.params[0].get_str())) + if (!DecodeHexTx(mtx, request.params[0].get_str())) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); UniValue result(UniValue::VOBJ); - TxToJSON(tx, UINT256_ZERO, result); + TxToJSON(CTransaction(std::move(mtx)), UINT256_ZERO, result); return result; } @@ -563,7 +563,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) } // parse hex string from parameter - CTransaction origTx; + CMutableTransaction origTx; if (!DecodeHexTx(origTx, request.params[0].get_str())) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); @@ -902,10 +902,10 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VBOOL}); // parse hex string from parameter - CTransaction tx; - if (!DecodeHexTx(tx, request.params[0].get_str())) + CMutableTransaction mtx; + if (!DecodeHexTx(mtx, request.params[0].get_str())) throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed"); - uint256 hashTx = tx.GetHash(); + const uint256& hashTx = mtx.GetHash(); bool fOverrideFees = false; if (request.params.size() > 1) @@ -914,7 +914,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) AssertLockNotHeld(cs_main); CCoinsViewCache& view = *pcoinsTip; bool fHaveChain = false; - for (size_t o = 0; !fHaveChain && o < tx.vout.size(); o++) { + for (size_t o = 0; !fHaveChain && o < mtx.vout.size(); o++) { const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o)); fHaveChain = !existingCoin.IsSpent(); } @@ -924,7 +924,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) bool fMissingInputs; { LOCK(cs_main); - if (!AcceptToMemoryPool(mempool, state, MakeTransactionRef(std::move(tx)), true, &fMissingInputs, false, !fOverrideFees)) { + if (!AcceptToMemoryPool(mempool, state, MakeTransactionRef(std::move(mtx)), true, &fMissingInputs, false, !fOverrideFees)) { if (state.IsInvalid()) { throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); } else { From 4e1f27082f7b84d493fce020e308cab741f7eec1 Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 1 Feb 2021 20:05:44 -0300 Subject: [PATCH 3/3] Make CTransaction actually immutable Adaptation coming from btc@81e3228fcb33e8ed32d8b9fbe917444ba080073a --- src/core_io.h | 2 +- src/net_processing.cpp | 5 +- src/primitives/transaction.cpp | 28 ++------ src/primitives/transaction.h | 70 ++++++++++++------- src/sapling/sapling_operation.cpp | 16 +++-- src/sapling/sapling_operation.h | 5 +- src/script/bitcoinconsensus.cpp | 3 +- src/script/sign.h | 2 +- src/test/bloom_tests.cpp | 6 +- src/test/coins_tests.cpp | 24 +++---- src/test/librust/sapling_rpc_wallet_tests.cpp | 3 +- src/test/script_tests.cpp | 17 ++--- src/test/serialize_tests.cpp | 6 +- src/test/sighash_tests.cpp | 14 ++-- src/test/transaction_tests.cpp | 9 +-- 15 files changed, 101 insertions(+), 109 deletions(-) diff --git a/src/core_io.h b/src/core_io.h index 7a1ae92321b0..486915464dd2 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -12,7 +12,7 @@ class CBlock; class CScript; class CTransaction; -class CMutableTransaction; +struct CMutableTransaction; class uint256; class UniValue; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5f308867b767..f22163bd0a98 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1395,9 +1395,8 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR else if (strCommand == NetMsgType::TX) { std::vector vWorkQueue; std::vector vEraseQueue; - CTransactionRef ptx; - vRecv >> ptx; - const CTransaction& tx = *ptx; + CTransaction tx(deserialize, vRecv); + CTransactionRef ptx = MakeTransactionRef(tx); CInv inv(MSG_TX, tx.GetHash()); pfrom->AddInventoryKnown(inv); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index f2bbd03880ea..b8b149921efd 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -121,9 +121,9 @@ uint256 CMutableTransaction::GetHash() const return SerializeHash(*this); } -void CTransaction::UpdateHash() const +uint256 CTransaction::ComputeHash() const { - *const_cast(&hash) = SerializeHash(*this); + return SerializeHash(*this); } size_t CTransaction::DynamicMemoryUsage() const @@ -131,26 +131,10 @@ size_t CTransaction::DynamicMemoryUsage() const return memusage::RecursiveDynamicUsage(vin) + memusage::RecursiveDynamicUsage(vout); } -CTransaction::CTransaction() : nVersion(CTransaction::CURRENT_VERSION), nType(TxType::NORMAL), vin(), vout(), nLockTime(0) { } - -CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), nType(tx.nType), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), sapData(tx.sapData), extraPayload(tx.extraPayload) { - UpdateHash(); -} - -CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), nType(tx.nType), vin(std::move(tx.vin)), vout(std::move(tx.vout)), nLockTime(tx.nLockTime), sapData(tx.sapData), extraPayload(tx.extraPayload) { - UpdateHash(); -} - -CTransaction& CTransaction::operator=(const CTransaction &tx) { - *const_cast(&nVersion) = tx.nVersion; - *const_cast(&nType) = tx.nType; - *const_cast*>(&vin) = tx.vin; - *const_cast*>(&vout) = tx.vout; - *const_cast(&nLockTime) = tx.nLockTime; - *const_cast(&hash) = tx.hash; - *const_cast*>(&sapData) = tx.sapData; - 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), nType(TxType::NORMAL), vin(), vout(), nLockTime(0), hash() {} +CTransaction::CTransaction(const CMutableTransaction &tx) : nVersion(tx.nVersion), nType(tx.nType), vin(tx.vin), vout(tx.vout), nLockTime(tx.nLockTime), sapData(tx.sapData), extraPayload(tx.extraPayload), hash(ComputeHash()) {} +CTransaction::CTransaction(CMutableTransaction &&tx) : nVersion(tx.nVersion), nType(tx.nType), vin(std::move(tx.vin)), vout(std::move(tx.vout)), nLockTime(tx.nLockTime), sapData(tx.sapData), extraPayload(tx.extraPayload), hash(ComputeHash()) {} bool CTransaction::HasZerocoinSpendInputs() const { diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index 4df084dbcc04..f275c78d2e0c 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -221,18 +221,35 @@ struct CMutableTransaction; * - Optional sapData * - Optional> extraPayload */ -template -inline void SerializeTransaction(TxType& tx, Stream& s, Operation ser_action) { - READWRITE(*const_cast(&tx.nVersion)); - READWRITE(*const_cast(&tx.nType)); - READWRITE(*const_cast*>(&tx.vin)); - READWRITE(*const_cast*>(&tx.vout)); - READWRITE(*const_cast(&tx.nLockTime)); +template +inline void UnserializeTransaction(TxType& tx, Stream& s) { + tx.vin.clear(); + tx.vout.clear(); + + s >> tx.nVersion; + s >> tx.nType; + s >> tx.vin; + s >> tx.vout; + s >> tx.nLockTime; + if (tx.isSaplingVersion()) { + s >> tx.sapData; + if (!tx.IsNormalType()) { + s >> tx.extraPayload; + } + } +} +template +inline void SerializeTransaction(const TxType& tx, Stream& s) { + s << tx.nVersion; + s << tx.nType; + s << tx.vin; + s << tx.vout; + s << tx.nLockTime; if (tx.isSaplingVersion()) { - READWRITE(*const_cast*>(&tx.sapData)); + s << tx.sapData; if (!tx.IsNormalType()) { - READWRITE(*const_cast>*>(&tx.extraPayload)); + s << tx.extraPayload; } } } @@ -242,11 +259,6 @@ inline void SerializeTransaction(TxType& tx, Stream& s, Operation ser_action) { */ class CTransaction { -private: - /** Memory only. */ - const uint256 hash; - void UpdateHash() const; - public: /** Transaction Versions */ enum TxVersion: int16_t { @@ -283,18 +295,14 @@ class CTransaction CTransaction(CMutableTransaction &&tx); CTransaction(const CTransaction& tx) = default; - 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)) {} @@ -400,6 +408,11 @@ class CTransaction std::string ToString() const; size_t DynamicMemoryUsage() const; + +private: + /** Memory only. */ + const uint256 hash; + uint256 ComputeHash() const; }; /** A mutable version of CTransaction. */ @@ -416,11 +429,14 @@ 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/sapling/sapling_operation.cpp b/src/sapling/sapling_operation.cpp index c42a8614aa70..f89a14a093bb 100644 --- a/src/sapling/sapling_operation.cpp +++ b/src/sapling/sapling_operation.cpp @@ -177,12 +177,13 @@ OperationResult SaplingOperation::build() if (!opTx) { return errorOut("Failed to build transaction: " + txResult.GetError()); } - finalTx = *opTx; + finalTx.reset(); + finalTx = MakeTransactionRef(*opTx); // Now check fee - bool isShielded = finalTx.IsShieldedTx(); - const CAmount& nFeeNeeded = isShielded ? GetShieldedTxMinFee(finalTx) : - GetMinRelayFee(finalTx.GetTotalSize(), false); + bool isShielded = finalTx->IsShieldedTx(); + const CAmount& nFeeNeeded = isShielded ? GetShieldedTxMinFee(*finalTx) : + GetMinRelayFee(finalTx->GetTotalSize(), false); if (nFeeNeeded <= nFeeRet) { // Check that the fee is not too high. CAmount nMaxFee = nFeeNeeded * (isShielded ? 100 : 10000); @@ -222,19 +223,20 @@ OperationResult SaplingOperation::build() if (!opTx) { return errorOut("Failed to build transaction: " + txResult.GetError()); } - finalTx = *opTx; + finalTx.reset(); + finalTx = MakeTransactionRef(*opTx); return OperationResult(true); } OperationResult SaplingOperation::send(std::string& retTxHash) { - CWalletTx wtx(pwalletMain, MakeTransactionRef(finalTx)); + CWalletTx wtx(pwalletMain, finalTx); const CWallet::CommitResult& res = pwalletMain->CommitTransaction(wtx, tkeyChange, g_connman.get()); if (res.status != CWallet::CommitStatus::OK) { return errorOut(res.ToString()); } - retTxHash = finalTx.GetHash().ToString(); + retTxHash = finalTx->GetHash().ToString(); return OperationResult(true); } diff --git a/src/sapling/sapling_operation.h b/src/sapling/sapling_operation.h index 9c8114dcb3c9..c1c4cb74bc11 100644 --- a/src/sapling/sapling_operation.h +++ b/src/sapling/sapling_operation.h @@ -104,7 +104,8 @@ class SaplingOperation { SaplingOperation* setCoinControl(const CCoinControl* _coinControl) { coinControl = _coinControl; return this; } CAmount getFee() { return fee; } - CTransaction getFinalTx() { return finalTx; } + CTransaction getFinalTx() { return *finalTx; } + CTransactionRef getFinalTxRef() { return finalTx; } private: FromAddress fromAddress; @@ -124,7 +125,7 @@ class SaplingOperation { // Builder TransactionBuilder txBuilder; - CTransaction finalTx; + CTransactionRef finalTx; OperationResult loadUtxos(TxValues& values); OperationResult loadUtxos(TxValues& txValues, const std::vector& selectedUTXO, const CAmount selectedUTXOAmount); diff --git a/src/script/bitcoinconsensus.cpp b/src/script/bitcoinconsensus.cpp index 8294e50352af..6d79158198e2 100644 --- a/src/script/bitcoinconsensus.cpp +++ b/src/script/bitcoinconsensus.cpp @@ -78,8 +78,7 @@ int bitcoinconsensus_verify_script(const unsigned char *scriptPubKey, unsigned i { 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/script/sign.h b/src/script/sign.h index 713ad8b91c16..f231a4581425 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -46,7 +46,7 @@ class TransactionSignatureCreator : public BaseSignatureCreator { }; class MutableTransactionSignatureCreator : public TransactionSignatureCreator { - CTransaction tx; + const 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) {} diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index 048daf9c687a..d8646534f4c9 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -111,16 +111,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}; std::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(uint256("0xb4749f017444b051c44dfd2720e88f314ff94f3dd6d56d40ef65854fcd7fff6b")); diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index a57735a7d237..6acf1ad809b5 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -172,7 +172,7 @@ class CCoinsViewTest : public CCoinsView class TxWithNullifiers { public: - CTransaction tx; + CTransactionRef tx; uint256 saplingNullifier; TxWithNullifiers() @@ -183,7 +183,7 @@ class TxWithNullifiers SpendDescription sd; sd.nullifier = saplingNullifier; mutableTx.sapData->vShieldedSpend.push_back(sd); - tx = CTransaction(mutableTx); + tx = MakeTransactionRef(CTransaction(mutableTx)); } }; @@ -235,12 +235,12 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) TxWithNullifiers txWithNullifiers; // Insert a nullifier into the base. - cache1.SetNullifiers(txWithNullifiers.tx, true); + cache1.SetNullifiers(*txWithNullifiers.tx, true); checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifiers(txWithNullifiers.tx, false); + cache1.SetNullifiers(*txWithNullifiers.tx, false); // The nullifier now should be `false`. checkNullifierCache(cache1, txWithNullifiers, false); @@ -254,12 +254,12 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) TxWithNullifiers txWithNullifiers; // Insert a nullifier into the base. - cache1.SetNullifiers(txWithNullifiers.tx, true); + cache1.SetNullifiers(*txWithNullifiers.tx, true); checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Flush to base. // Remove the nullifier from cache - cache1.SetNullifiers(txWithNullifiers.tx, false); + cache1.SetNullifiers(*txWithNullifiers.tx, false); cache1.Flush(); // Flush to base. // The nullifier now should be `false`. @@ -273,7 +273,7 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) // Insert a nullifier into the base. TxWithNullifiers txWithNullifiers; - cache1.SetNullifiers(txWithNullifiers.tx, true); + cache1.SetNullifiers(*txWithNullifiers.tx, true); checkNullifierCache(cache1, txWithNullifiers, true); cache1.Flush(); // Empties cache. @@ -282,7 +282,7 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) // Remove the nullifier. CCoinsViewCache cache2(&cache1); checkNullifierCache(cache2, txWithNullifiers, true); - cache1.SetNullifiers(txWithNullifiers.tx, false); + cache1.SetNullifiers(*txWithNullifiers.tx, false); cache2.Flush(); // Empties cache, flushes to cache1. } @@ -297,14 +297,14 @@ BOOST_AUTO_TEST_CASE(nullifier_regression_test) // Insert a nullifier into the base. TxWithNullifiers txWithNullifiers; - cache1.SetNullifiers(txWithNullifiers.tx, true); + cache1.SetNullifiers(*txWithNullifiers.tx, true); cache1.Flush(); // Empties cache. // Create cache on top. { // Remove the nullifier. CCoinsViewCache cache2(&cache1); - cache2.SetNullifiers(txWithNullifiers.tx, false); + cache2.SetNullifiers(*txWithNullifiers.tx, false); cache2.Flush(); // Empties cache, flushes to cache1. } @@ -485,14 +485,14 @@ BOOST_AUTO_TEST_CASE(nullifiers_test) TxWithNullifiers txWithNullifiers; checkNullifierCache(cache, txWithNullifiers, false); - cache.SetNullifiers(txWithNullifiers.tx, true); + cache.SetNullifiers(*txWithNullifiers.tx, true); checkNullifierCache(cache, txWithNullifiers, true); cache.Flush(); CCoinsViewCache cache2(&base); checkNullifierCache(cache2, txWithNullifiers, true); - cache2.SetNullifiers(txWithNullifiers.tx, false); + cache2.SetNullifiers(*txWithNullifiers.tx, false); checkNullifierCache(cache2, txWithNullifiers, false); cache2.Flush(); diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index 3c2f56f71311..9c825733217d 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -449,8 +449,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) // Test mode does not send the transaction to the network. auto hexTx = EncodeHexTx(operation.getFinalTx()); CDataStream ss(ParseHex(hexTx), SER_NETWORK, PROTOCOL_VERSION); - CTransaction tx; - ss >> tx; + CTransaction tx(deserialize, ss); BOOST_ASSERT(!tx.sapData->vShieldedOutput.empty()); // We shouldn't be able to decrypt with the empty ovk diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index ede0f23e9349..cb96d36e391e 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -15,15 +15,12 @@ #include "script/script.h" #include "script/script_error.h" #include "script/sign.h" -#include "util.h" #if defined(HAVE_CONSENSUS_LIB) #include "script/bitcoinconsensus.h" #endif #include -#include -#include #include #include @@ -183,7 +180,7 @@ class TestBuilder { private: CScript scriptPubKey; - CTransaction creditTx; + CTransactionRef creditTx; CMutableTransaction spendTx; bool havePush; std::vector push; @@ -209,11 +206,11 @@ class TestBuilder TestBuilder(const CScript& redeemScript, const std::string& comment_, int flags_, bool P2SH = false) : scriptPubKey(redeemScript), havePush(false), comment(comment_), flags(flags_) { if (P2SH) { - creditTx = BuildCreditingTransaction(CScript() << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL); + creditTx = MakeTransactionRef(BuildCreditingTransaction(CScript() << OP_HASH160 << ToByteVector(CScriptID(redeemScript)) << OP_EQUAL)); } else { - creditTx = BuildCreditingTransaction(redeemScript); + creditTx = MakeTransactionRef(BuildCreditingTransaction(redeemScript)); } - spendTx = BuildSpendingTransaction(CScript(), creditTx); + spendTx = BuildSpendingTransaction(CScript(), *creditTx); } TestBuilder& Add(const CScript& script) @@ -290,7 +287,7 @@ class TestBuilder { TestBuilder copy = *this; // Make a copy so we can rollback the push. DoPush(); - DoTest(creditTx.vout[0].scriptPubKey, spendTx.vin[0].scriptSig, flags, expect, comment); + DoTest(creditTx->vout[0].scriptPubKey, spendTx.vin[0].scriptSig, flags, expect, comment); *this = copy; return *this; } @@ -300,7 +297,7 @@ class TestBuilder DoPush(); UniValue array(UniValue::VARR); 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(comment); return array; @@ -313,7 +310,7 @@ class TestBuilder 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 62857a92f89e..b5bd8eb163b6 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -40,10 +40,10 @@ class CSerializeMethodsTestSingle 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 @@ -61,7 +61,7 @@ class CSerializeMethodsTestSingle 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 e4fe46bfeebc..59c669587e1b 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -216,7 +216,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 { @@ -232,7 +232,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) stream >> tx; CValidationState state; - BOOST_CHECK_MESSAGE(CheckTransaction(tx, false, false, state), strTest); + BOOST_CHECK_MESSAGE(CheckTransaction(*tx, false, false, state), strTest); BOOST_CHECK(state.IsValid()); std::vector raw = ParseHex(raw_script); @@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(sighash_from_data) continue; } - sh = SignatureHash(scriptCode, tx, nIn, nHashType, 0, tx.GetRequiredSigVersion()); + sh = SignatureHash(scriptCode, *tx, nIn, nHashType, 0, tx->GetRequiredSigVersion()); BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest); } @@ -386,17 +386,17 @@ BOOST_AUTO_TEST_CASE(malleated_tx) mtx = _tx; while (mtx.vout[i].nValue == tx.vout[i].nValue) mtx.vout[i].nValue = InsecureRandRange(100000000); - tx2 = CTransaction(mtx); + CTransaction tx3 = CTransaction(mtx); for (int nIn = 0; nIn < (int) tx.vin.size(); nIn++) { - BOOST_CHECK(vsh[nIn] != SignatureHash(vScriptCode[nIn], tx2, nIn, SIGHASH_ALL, 0, tx2.GetRequiredSigVersion())); + BOOST_CHECK(vsh[nIn] != SignatureHash(vScriptCode[nIn], tx3, nIn, SIGHASH_ALL, 0, tx3.GetRequiredSigVersion())); } mtx = _tx; while (mtx.vout[i].scriptPubKey == tx.vout[i].scriptPubKey) RandomScript(mtx.vout[i].scriptPubKey); - tx2 = CTransaction(mtx); + CTransaction tx4 = CTransaction(mtx); for (int nIn = 0; nIn < (int) tx.vin.size(); nIn++) { - BOOST_CHECK(vsh[nIn] != SignatureHash(vScriptCode[nIn], tx2, nIn, SIGHASH_ALL, 0, tx2.GetRequiredSigVersion())); + BOOST_CHECK(vsh[nIn] != SignatureHash(vScriptCode[nIn], tx4, nIn, SIGHASH_ALL, 0, tx4.GetRequiredSigVersion())); } } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index daf75f48070c..b6632b56f08b 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -130,8 +130,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) std::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, false, false, state), strTest); @@ -207,8 +206,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) std::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, false, false, state) && state.IsValid(); @@ -362,10 +360,9 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) { assert(hashSigned); } - CTransaction tx; CDataStream ssout(SER_NETWORK, PROTOCOL_VERSION); ssout << mtx; - ssout >> tx; + CTransaction tx(deserialize, ssout); // check all inputs concurrently, with the cache PrecomputedTransactionData precomTxData(tx);