From 1662b437b33b7ec5a1723f6ae6187d3bdd06f593 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 10 Nov 2016 17:26:00 -0800 Subject: [PATCH] Make CBlock::vtx a vector of shared_ptr --- src/blockencodings.cpp | 14 ++--- src/blockencodings.h | 10 ++-- src/chainparams.cpp | 2 +- src/consensus/merkle.cpp | 6 +-- src/core_memusage.h | 4 +- src/main.cpp | 87 +++++++++++++++--------------- src/merkleblock.cpp | 6 +-- src/miner.cpp | 12 ++--- src/primitives/block.cpp | 2 +- src/primitives/block.h | 2 +- src/rpc/blockchain.cpp | 6 +-- src/rpc/mining.cpp | 5 +- src/rpc/rawtransaction.cpp | 4 +- src/test/blockencodings_tests.cpp | 61 ++++++++++++--------- src/test/mempool_tests.cpp | 6 +-- src/test/merkle_tests.cpp | 8 +-- src/test/miner_tests.cpp | 33 ++++++------ src/test/pmt_tests.cpp | 4 +- src/test/policyestimator_tests.cpp | 8 +-- src/test/test_bitcoin.cpp | 8 +-- src/test/test_bitcoin.h | 4 +- src/txmempool.cpp | 14 ++--- src/txmempool.h | 2 +- src/wallet/wallet.cpp | 2 +- 24 files changed, 161 insertions(+), 149 deletions(-) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index dbed90583..fac52474e 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -24,7 +24,7 @@ CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block, bool f //TODO: Use our mempool prior to block acceptance to predictively fill more than just the coinbase prefilledtxn[0] = {0, block.vtx[0]}; for (size_t i = 1; i < block.vtx.size(); i++) { - const CTransaction& tx = block.vtx[i]; + const CTransaction& tx = *block.vtx[i]; shorttxids[i - 1] = GetShortID(fUseWTXID ? tx.GetWitnessHash() : tx.GetHash()); } } @@ -59,7 +59,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c int32_t lastprefilledindex = -1; for (size_t i = 0; i < cmpctblock.prefilledtxn.size(); i++) { - if (cmpctblock.prefilledtxn[i].tx.IsNull()) + if (cmpctblock.prefilledtxn[i].tx->IsNull()) return READ_STATUS_INVALID; lastprefilledindex += cmpctblock.prefilledtxn[i].index + 1; //index is a uint16_t, so cant overflow here @@ -71,7 +71,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c // have neither a prefilled txn or a shorttxid! return READ_STATUS_INVALID; } - txn_available[lastprefilledindex] = std::make_shared(cmpctblock.prefilledtxn[i].tx); + txn_available[lastprefilledindex] = cmpctblock.prefilledtxn[i].tx; } prefilled_count = cmpctblock.prefilledtxn.size(); @@ -142,7 +142,7 @@ bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const { return txn_available[index] ? true : false; } -ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector& vtx_missing) const { +ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector>& vtx_missing) const { assert(!header.IsNull()); block = header; block.vtx.resize(txn_available.size()); @@ -154,7 +154,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< return READ_STATUS_INVALID; block.vtx[i] = vtx_missing[tx_missing_offset++]; } else - block.vtx[i] = *txn_available[i]; + block.vtx[i] = txn_available[i]; } if (vtx_missing.size() != tx_missing_offset) return READ_STATUS_INVALID; @@ -172,8 +172,8 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector< LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", header.GetHash().ToString(), prefilled_count, mempool_count, vtx_missing.size()); if (vtx_missing.size() < 5) { - for(const CTransaction& tx : vtx_missing) - LogPrint("cmpctblock", "Reconstructed block %s required tx %s\n", header.GetHash().ToString(), tx.GetHash().ToString()); + for (const auto& tx : vtx_missing) + LogPrint("cmpctblock", "Reconstructed block %s required tx %s\n", header.GetHash().ToString(), tx->GetHash().ToString()); } return READ_STATUS_OK; diff --git a/src/blockencodings.h b/src/blockencodings.h index 1f9491867..ffe189a94 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -14,9 +14,9 @@ class CTxMemPool; // Dumb helper to handle CTransaction compression at serialize-time struct TransactionCompressor { private: - CTransaction& tx; + std::shared_ptr& tx; public: - TransactionCompressor(CTransaction& txIn) : tx(txIn) {} + TransactionCompressor(std::shared_ptr& txIn) : tx(txIn) {} ADD_SERIALIZE_METHODS; @@ -72,7 +72,7 @@ class BlockTransactions { public: // A BlockTransactions message uint256 blockhash; - std::vector txn; + std::vector> txn; BlockTransactions() {} BlockTransactions(const BlockTransactionsRequest& req) : @@ -104,7 +104,7 @@ struct PrefilledTransaction { // Used as an offset since last prefilled tx in CBlockHeaderAndShortTxIDs, // as a proper transaction-in-block-index in PartiallyDownloadedBlock uint16_t index; - CTransaction tx; + std::shared_ptr tx; ADD_SERIALIZE_METHODS; @@ -202,7 +202,7 @@ public: ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock); bool IsTxAvailable(size_t index) const; - ReadStatus FillBlock(CBlock& block, const std::vector& vtx_missing) const; + ReadStatus FillBlock(CBlock& block, const std::vector>& vtx_missing) const; }; #endif diff --git a/src/chainparams.cpp b/src/chainparams.cpp index a57ab632e..77eb586b6 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -31,7 +31,7 @@ static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesi genesis.nBits = nBits; genesis.nNonce = nNonce; genesis.nVersion = nVersion; - genesis.vtx.push_back(txNew); + genesis.vtx.push_back(std::make_shared(std::move(txNew))); genesis.hashPrevBlock.SetNull(); genesis.hashMerkleRoot = BlockMerkleRoot(genesis); return genesis; diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index 35f7d2e05..6fa96ddf4 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -160,7 +160,7 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) std::vector leaves; leaves.resize(block.vtx.size()); for (size_t s = 0; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s].GetHash(); + leaves[s] = block.vtx[s]->GetHash(); } return ComputeMerkleRoot(leaves, mutated); } @@ -171,7 +171,7 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) leaves.resize(block.vtx.size()); leaves[0].SetNull(); // The witness hash of the coinbase is 0. for (size_t s = 1; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s].GetWitnessHash(); + leaves[s] = block.vtx[s]->GetWitnessHash(); } return ComputeMerkleRoot(leaves, mutated); } @@ -181,7 +181,7 @@ std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) std::vector leaves; leaves.resize(block.vtx.size()); for (size_t s = 0; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s].GetHash(); + leaves[s] = block.vtx[s]->GetHash(); } return ComputeMerkleBranch(leaves, position); } diff --git a/src/core_memusage.h b/src/core_memusage.h index b8e0f08bb..0dcc24c40 100644 --- a/src/core_memusage.h +++ b/src/core_memusage.h @@ -69,8 +69,8 @@ static inline size_t RecursiveDynamicUsage(const CMutableTransaction& tx) { static inline size_t RecursiveDynamicUsage(const CBlock& block) { size_t mem = memusage::DynamicUsage(block.vtx); - for (std::vector::const_iterator it = block.vtx.begin(); it != block.vtx.end(); it++) { - mem += RecursiveDynamicUsage(*it); + for (const auto& tx : block.vtx) { + mem += memusage::DynamicUsage(tx) + RecursiveDynamicUsage(*tx); } return mem; } diff --git a/src/main.cpp b/src/main.cpp index 263421aea..7b84ab7ba 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1682,9 +1682,9 @@ bool GetTransaction(const uint256 &hash, CTransaction &txOut, const Consensus::P if (pindexSlow) { CBlock block; if (ReadBlockFromDisk(block, pindexSlow, consensusParams)) { - BOOST_FOREACH(const CTransaction &tx, block.vtx) { - if (tx.GetHash() == hash) { - txOut = tx; + for (const auto& tx : block.vtx) { + if (tx->GetHash() == hash) { + txOut = *tx; hashBlock = pindexSlow->GetBlockHash(); return true; } @@ -2223,7 +2223,7 @@ bool DisconnectBlock(const CBlock& block, CValidationState& state, const CBlockI // undo transactions in reverse order for (int i = block.vtx.size() - 1; i >= 0; i--) { - const CTransaction &tx = block.vtx[i]; + const CTransaction &tx = *(block.vtx[i]); uint256 hash = tx.GetHash(); // Check that all outputs are available and match the outputs in the block itself @@ -2417,8 +2417,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == chainparams.GetConsensus().BIP34Hash)); if (fEnforceBIP30) { - BOOST_FOREACH(const CTransaction& tx, block.vtx) { - const CCoins* coins = view.AccessCoins(tx.GetHash()); + for (const auto& tx : block.vtx) { + const CCoins* coins = view.AccessCoins(tx->GetHash()); if (coins && !coins->IsPruned()) return state.DoS(100, error("ConnectBlock(): tried to overwrite transaction"), REJECT_INVALID, "bad-txns-BIP30"); @@ -2474,7 +2474,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin txdata.reserve(block.vtx.size()); // Required so that pointers to individual PrecomputedTransactionData don't get invalidated for (unsigned int i = 0; i < block.vtx.size(); i++) { - const CTransaction &tx = block.vtx[i]; + const CTransaction &tx = *(block.vtx[i]); nInputs += tx.vin.size(); @@ -2544,10 +2544,10 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin LogPrint("bench", " - Connect %u transactions: %.2fms (%.3fms/tx, %.3fms/txin) [%.2fs]\n", (unsigned)block.vtx.size(), 0.001 * (nTime3 - nTime2), 0.001 * (nTime3 - nTime2) / block.vtx.size(), nInputs <= 1 ? 0 : 0.001 * (nTime3 - nTime2) / (nInputs-1), nTimeConnect * 0.000001); CAmount blockReward = nFees + GetBlockSubsidy(pindex->nHeight, chainparams.GetConsensus()); - if (block.vtx[0].GetValueOut() > blockReward) + if (block.vtx[0]->GetValueOut() > blockReward) return state.DoS(100, error("ConnectBlock(): coinbase pays too much (actual=%d vs limit=%d)", - block.vtx[0].GetValueOut(), blockReward), + block.vtx[0]->GetValueOut(), blockReward), REJECT_INVALID, "bad-cb-amount"); if (!control.Wait()) @@ -2590,7 +2590,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // Watch for changes to the previous coinbase transaction. static uint256 hashPrevBestCoinBase; GetMainSignals().UpdatedTransaction(hashPrevBestCoinBase); - hashPrevBestCoinBase = block.vtx[0].GetHash(); + hashPrevBestCoinBase = block.vtx[0]->GetHash(); // Erase orphan transactions include or precluded by this block if (vOrphanErase.size()) { @@ -2807,7 +2807,8 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara if (!fBare) { // Resurrect mempool transactions from the disconnected block. std::vector vHashUpdate; - BOOST_FOREACH(const CTransaction &tx, block.vtx) { + for (const auto& it : block.vtx) { + const CTransaction& tx = *it; // ignore validation errors in resurrected transactions CValidationState stateDummy; if (tx.IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL, true)) { @@ -2828,8 +2829,8 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara UpdateTip(pindexDelete->pprev, chainparams); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: - BOOST_FOREACH(const CTransaction &tx, block.vtx) { - GetMainSignals().SyncTransaction(tx, pindexDelete->pprev, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); + for (const auto& tx : block.vtx) { + GetMainSignals().SyncTransaction(*tx, pindexDelete->pprev, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } return true; } @@ -2844,7 +2845,7 @@ static int64_t nTimePostConnect = 0; * Connect a new block to chainActive. pblock is either NULL or a pointer to a CBlock * corresponding to pindexNew, to bypass loading it again from disk. */ -bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::vector> &txConflicted, std::vector> &txChanged) +bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const CBlock* pblock, std::vector> &txConflicted, std::vector,CBlockIndex*,int>> &txChanged) { assert(pindexNew->pprev == chainActive.Tip()); // Read block from disk. @@ -2884,7 +2885,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, // Update chainActive & related variables. UpdateTip(pindexNew, chainparams); - for(unsigned int i=0; i < pblock->vtx.size(); i++) + for (unsigned int i=0; i < pblock->vtx.size(); i++) txChanged.emplace_back(pblock->vtx[i], pindexNew, i); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; @@ -2967,7 +2968,7 @@ static void PruneBlockIndexCandidates() { * Try to make some progress towards making pindexMostWork the active block. * pblock is either NULL or a pointer to a CBlock corresponding to pindexMostWork. */ -static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound, std::vector>& txConflicted, std::vector>& txChanged) +static bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const CBlock* pblock, bool& fInvalidFound, std::vector>& txConflicted, std::vector,CBlockIndex*,int>>& txChanged) { AssertLockHeld(cs_main); const CBlockIndex *pindexOldTip = chainActive.Tip(); @@ -3068,7 +3069,7 @@ static void NotifyHeaderTip() { bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, const CBlock *pblock) { CBlockIndex *pindexMostWork = NULL; CBlockIndex *pindexNewTip = NULL; - std::vector> txChanged; + std::vector,CBlockIndex*,int>> txChanged; if (pblock) txChanged.reserve(pblock->vtx.size()); do { @@ -3109,13 +3110,13 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // throw all transactions though the signal-interface // while _not_ holding the cs_main lock - for(std::shared_ptr tx : txConflicted) + for (const auto& tx : txConflicted) { GetMainSignals().SyncTransaction(*tx, pindexNewTip, CMainSignals::SYNC_TRANSACTION_NOT_IN_BLOCK); } // ... and about transactions that got confirmed: - for(unsigned int i = 0; i < txChanged.size(); i++) - GetMainSignals().SyncTransaction(std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); + for (unsigned int i = 0; i < txChanged.size(); i++) + GetMainSignals().SyncTransaction(*std::get<0>(txChanged[i]), std::get<1>(txChanged[i]), std::get<2>(txChanged[i])); // Notify external listeners about the new tip. GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload); @@ -3454,22 +3455,22 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P return state.DoS(100, false, REJECT_INVALID, "bad-blk-length", false, "size limits failed"); // First transaction must be coinbase, the rest must not be - if (block.vtx.empty() || !block.vtx[0].IsCoinBase()) + if (block.vtx.empty() || !block.vtx[0]->IsCoinBase()) return state.DoS(100, false, REJECT_INVALID, "bad-cb-missing", false, "first tx is not coinbase"); for (unsigned int i = 1; i < block.vtx.size(); i++) - if (block.vtx[i].IsCoinBase()) + if (block.vtx[i]->IsCoinBase()) return state.DoS(100, false, REJECT_INVALID, "bad-cb-multiple", false, "more than one coinbase"); // Check transactions for (const auto& tx : block.vtx) - if (!CheckTransaction(tx, state, false)) + if (!CheckTransaction(*tx, state, false)) return state.Invalid(false, state.GetRejectCode(), state.GetRejectReason(), - strprintf("Transaction check failed (tx hash %s) %s", tx.GetHash().ToString(), state.GetDebugMessage())); + strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), state.GetDebugMessage())); unsigned int nSigOps = 0; for (const auto& tx : block.vtx) { - nSigOps += GetLegacySigOpCount(tx); + nSigOps += GetLegacySigOpCount(*tx); } if (nSigOps * WITNESS_SCALE_FACTOR > MAX_BLOCK_SIGOPS_COST) return state.DoS(100, false, REJECT_INVALID, "bad-blk-sigops", false, "out-of-bounds SigOpCount"); @@ -3505,8 +3506,8 @@ bool IsWitnessEnabled(const CBlockIndex* pindexPrev, const Consensus::Params& pa static int GetWitnessCommitmentIndex(const CBlock& block) { int commitpos = -1; - for (size_t o = 0; o < block.vtx[0].vout.size(); o++) { - if (block.vtx[0].vout[o].scriptPubKey.size() >= 38 && block.vtx[0].vout[o].scriptPubKey[0] == OP_RETURN && block.vtx[0].vout[o].scriptPubKey[1] == 0x24 && block.vtx[0].vout[o].scriptPubKey[2] == 0xaa && block.vtx[0].vout[o].scriptPubKey[3] == 0x21 && block.vtx[0].vout[o].scriptPubKey[4] == 0xa9 && block.vtx[0].vout[o].scriptPubKey[5] == 0xed) { + for (size_t o = 0; o < block.vtx[0]->vout.size(); o++) { + if (block.vtx[0]->vout[o].scriptPubKey.size() >= 38 && block.vtx[0]->vout[o].scriptPubKey[0] == OP_RETURN && block.vtx[0]->vout[o].scriptPubKey[1] == 0x24 && block.vtx[0]->vout[o].scriptPubKey[2] == 0xaa && block.vtx[0]->vout[o].scriptPubKey[3] == 0x21 && block.vtx[0]->vout[o].scriptPubKey[4] == 0xa9 && block.vtx[0]->vout[o].scriptPubKey[5] == 0xed) { commitpos = o; } } @@ -3517,10 +3518,12 @@ 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()) { - block.vtx[0].wit.vtxinwit.resize(1); - block.vtx[0].wit.vtxinwit[0].scriptWitness.stack.resize(1); - block.vtx[0].wit.vtxinwit[0].scriptWitness.stack[0] = nonce; + if (commitpos != -1 && IsWitnessEnabled(pindexPrev, consensusParams) && block.vtx[0]->wit.IsEmpty()) { + 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; + block.vtx[0] = std::make_shared(std::move(tx)); } } @@ -3530,7 +3533,7 @@ std::vector GenerateCoinbaseCommitment(CBlock& block, const CBloc int commitpos = GetWitnessCommitmentIndex(block); bool fHaveWitness = false; for (size_t t = 1; t < block.vtx.size(); t++) { - if (!block.vtx[t].wit.IsNull()) { + if (!block.vtx[t]->wit.IsNull()) { fHaveWitness = true; break; } @@ -3551,8 +3554,8 @@ 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(); + const_cast*>(&block.vtx[0]->vout)->push_back(out); + block.vtx[0]->UpdateHash(); } } UpdateUncommittedBlockStructures(block, pindexPrev, consensusParams); @@ -3601,7 +3604,7 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co // Check that all transactions are finalized for (const auto& tx : block.vtx) { - if (!IsFinalTx(tx, nHeight, nLockTimeCutoff)) { + if (!IsFinalTx(*tx, nHeight, nLockTimeCutoff)) { return state.DoS(10, false, REJECT_INVALID, "bad-txns-nonfinal", false, "non-final transaction"); } } @@ -3610,8 +3613,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co if (nHeight >= consensusParams.BIP34Height) { CScript expect = CScript() << nHeight; - if (block.vtx[0].vin[0].scriptSig.size() < expect.size() || - !std::equal(expect.begin(), expect.end(), block.vtx[0].vin[0].scriptSig.begin())) { + if (block.vtx[0]->vin[0].scriptSig.size() < expect.size() || + !std::equal(expect.begin(), expect.end(), block.vtx[0]->vin[0].scriptSig.begin())) { return state.DoS(100, false, REJECT_INVALID, "bad-cb-height", false, "block height mismatch in coinbase"); } } @@ -3633,11 +3636,11 @@ 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]->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) { 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()); - if (memcmp(hashWitness.begin(), &block.vtx[0].vout[commitpos].scriptPubKey[6], 32)) { + CHash256().Write(hashWitness.begin(), 32).Write(&block.vtx[0]->wit.vtxinwit[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__)); } fHaveWitness = true; @@ -3647,7 +3650,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]->wit.IsNull()) { return state.DoS(100, false, REJECT_INVALID, "unexpected-witness", true, strprintf("%s : unexpected witness data found", __func__)); } } @@ -4953,7 +4956,7 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam // however we MUST always provide at least what the remote peer needs typedef std::pair PairType; BOOST_FOREACH(PairType& pair, merkleBlock.vMatchedTxn) - connman.PushMessageWithFlag(pfrom, SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, block.vtx[pair.first]); + connman.PushMessageWithFlag(pfrom, SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::TX, *block.vtx[pair.first]); } // else // no response diff --git a/src/merkleblock.cpp b/src/merkleblock.cpp index 31332526a..882717ac5 100644 --- a/src/merkleblock.cpp +++ b/src/merkleblock.cpp @@ -23,8 +23,8 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, CBloomFilter& filter) for (unsigned int i = 0; i < block.vtx.size(); i++) { - const uint256& hash = block.vtx[i].GetHash(); - if (filter.IsRelevantAndUpdate(block.vtx[i])) + const uint256& hash = block.vtx[i]->GetHash(); + if (filter.IsRelevantAndUpdate(*block.vtx[i])) { vMatch.push_back(true); vMatchedTxn.push_back(make_pair(i, hash)); @@ -49,7 +49,7 @@ CMerkleBlock::CMerkleBlock(const CBlock& block, const std::set& txids) for (unsigned int i = 0; i < block.vtx.size(); i++) { - const uint256& hash = block.vtx[i].GetHash(); + const uint256& hash = block.vtx[i]->GetHash(); if (txids.count(hash)) vMatch.push_back(true); else diff --git a/src/miner.cpp b/src/miner.cpp index 6ad63207c..2b65cbc65 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -134,7 +134,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc pblock = &pblocktemplate->block; // pointer for convenience // Add dummy coinbase tx as first transaction - pblock->vtx.push_back(CTransaction()); + pblock->vtx.emplace_back(); pblocktemplate->vTxFees.push_back(-1); // updated at end pblocktemplate->vTxSigOpsCost.push_back(-1); // updated at end @@ -178,7 +178,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc coinbaseTx.vout[0].scriptPubKey = scriptPubKeyIn; coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus()); coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; - pblock->vtx[0] = coinbaseTx; + pblock->vtx[0] = std::make_shared(std::move(coinbaseTx)); pblocktemplate->vchCoinbaseCommitment = GenerateCoinbaseCommitment(*pblock, pindexPrev, chainparams.GetConsensus()); pblocktemplate->vTxFees[0] = -nFees; @@ -190,7 +190,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus()); pblock->nNonce = 0; - pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(pblock->vtx[0]); + pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]); CValidationState state; if (!TestBlockValidity(state, chainparams, *pblock, pindexPrev, false, false)) { @@ -312,7 +312,7 @@ bool BlockAssembler::TestForBlock(CTxMemPool::txiter iter) void BlockAssembler::AddToBlock(CTxMemPool::txiter iter) { - pblock->vtx.push_back(iter->GetTx()); + pblock->vtx.emplace_back(iter->GetSharedTx()); pblocktemplate->vTxFees.push_back(iter->GetFee()); pblocktemplate->vTxSigOpsCost.push_back(iter->GetSigOpCost()); if (fNeedSizeAccounting) { @@ -601,10 +601,10 @@ void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned } ++nExtraNonce; unsigned int nHeight = pindexPrev->nHeight+1; // Height first in coinbase required for block.version=2 - CMutableTransaction txCoinbase(pblock->vtx[0]); + CMutableTransaction txCoinbase(*pblock->vtx[0]); txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS; assert(txCoinbase.vin[0].scriptSig.size() <= 100); - pblock->vtx[0] = txCoinbase; + pblock->vtx[0] = std::make_shared(std::move(txCoinbase)); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); } diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index 0e6ab4dd7..95bd2211f 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -27,7 +27,7 @@ std::string CBlock::ToString() const vtx.size()); for (unsigned int i = 0; i < vtx.size(); i++) { - s << " " << vtx[i].ToString() << "\n"; + s << " " << vtx[i]->ToString() << "\n"; } return s.str(); } diff --git a/src/primitives/block.h b/src/primitives/block.h index d148aec1e..45b4895eb 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -73,7 +73,7 @@ class CBlock : public CBlockHeader { public: // network and disk - std::vector vtx; + std::vector> vtx; // memory only mutable bool fChecked; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8caea14ad..154107c0d 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -119,16 +119,16 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx result.push_back(Pair("versionHex", strprintf("%08x", block.nVersion))); result.push_back(Pair("merkleroot", block.hashMerkleRoot.GetHex())); UniValue txs(UniValue::VARR); - BOOST_FOREACH(const CTransaction&tx, block.vtx) + for(const auto& tx : block.vtx) { if(txDetails) { UniValue objTx(UniValue::VOBJ); - TxToJSON(tx, uint256(), objTx); + TxToJSON(*tx, uint256(), objTx); txs.push_back(objTx); } else - txs.push_back(tx.GetHash().GetHex()); + txs.push_back(tx->GetHash().GetHex()); } result.push_back(Pair("tx", txs)); result.push_back(Pair("time", block.GetBlockTime())); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index ad545bdf0..6b0e52a30 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -557,7 +557,8 @@ UniValue getblocktemplate(const JSONRPCRequest& request) UniValue transactions(UniValue::VARR); map setTxIndex; int i = 0; - BOOST_FOREACH (CTransaction& tx, pblock->vtx) { + for (const auto& it : pblock->vtx) { + const CTransaction& tx = *it; uint256 txHash = tx.GetHash(); setTxIndex[txHash] = i++; @@ -662,7 +663,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request) result.push_back(Pair("previousblockhash", pblock->hashPrevBlock.GetHex())); result.push_back(Pair("transactions", transactions)); result.push_back(Pair("coinbaseaux", aux)); - result.push_back(Pair("coinbasevalue", (int64_t)pblock->vtx[0].vout[0].nValue)); + result.push_back(Pair("coinbasevalue", (int64_t)pblock->vtx[0]->vout[0].nValue)); result.push_back(Pair("longpollid", chainActive.Tip()->GetBlockHash().GetHex() + i64tostr(nTransactionsUpdatedLast))); result.push_back(Pair("target", hashTarget.GetHex())); result.push_back(Pair("mintime", (int64_t)pindexPrev->GetMedianTimePast()+1)); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0656a6175..b9b81600b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -288,8 +288,8 @@ UniValue gettxoutproof(const JSONRPCRequest& request) throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); unsigned int ntxFound = 0; - BOOST_FOREACH(const CTransaction&tx, block.vtx) - if (setTxids.count(tx.GetHash())) + for (const auto& tx : block.vtx) + if (setTxids.count(tx->GetHash())) ntxFound++; if (ntxFound != setTxids.size()) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "(Not all) transactions not found in specified block"); diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 0ed5d62ef..6899bd632 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -26,21 +26,21 @@ static CBlock BuildBlockTestCase() { tx.vout[0].nValue = 42; block.vtx.resize(3); - block.vtx[0] = tx; + block.vtx[0] = std::make_shared(tx); block.nVersion = 42; block.hashPrevBlock = GetRandHash(); block.nBits = 0x207fffff; tx.vin[0].prevout.hash = GetRandHash(); tx.vin[0].prevout.n = 0; - block.vtx[1] = tx; + block.vtx[1] = std::make_shared(tx); tx.vin.resize(10); for (size_t i = 0; i < tx.vin.size(); i++) { tx.vin[i].prevout.hash = GetRandHash(); tx.vin[i].prevout.n = 0; } - block.vtx[2] = tx; + block.vtx[2] = std::make_shared(tx); bool mutated; block.hashMerkleRoot = BlockMerkleRoot(block, &mutated); @@ -59,8 +59,8 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); - pool.addUnchecked(block.vtx[2].GetHash(), entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2])); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); // Do a simple ShortTxIDs RT { @@ -78,14 +78,14 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest) BOOST_CHECK(!partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); std::vector> removed; - pool.removeRecursive(block.vtx[2], &removed); + pool.removeRecursive(*block.vtx[2], &removed); BOOST_CHECK_EQUAL(removed.size(), 1); CBlock block2; - std::vector vtx_missing; + std::vector> vtx_missing; BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_INVALID); // No transactions vtx_missing.push_back(block.vtx[2]); // Wrong transaction @@ -152,8 +152,10 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); - pool.addUnchecked(block.vtx[2].GetHash(), entry.FromTx(block.vtx[2])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2])); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + + uint256 txhash; // Test with pre-forwarding tx 1, but not coinbase { @@ -161,8 +163,8 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) shortIDs.prefilledtxn.resize(1); shortIDs.prefilledtxn[0] = {1, block.vtx[1]}; shortIDs.shorttxids.resize(2); - shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[0].GetHash()); - shortIDs.shorttxids[1] = shortIDs.GetShortID(block.vtx[2].GetHash()); + shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[0]->GetHash()); + shortIDs.shorttxids[1] = shortIDs.GetShortID(block.vtx[2]->GetHash()); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; @@ -176,10 +178,10 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); CBlock block2; - std::vector vtx_missing; + std::vector> vtx_missing; BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_INVALID); // No transactions vtx_missing.push_back(block.vtx[1]); // Wrong transaction @@ -194,9 +196,13 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest) BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString()); BOOST_CHECK(!mutated); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + txhash = block.vtx[2]->GetHash(); + block.vtx.clear(); + block2.vtx.clear(); + block3.vtx.clear(); + BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); } - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); } BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) @@ -205,8 +211,10 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) TestMemPoolEntryHelper entry; CBlock block(BuildBlockTestCase()); - pool.addUnchecked(block.vtx[1].GetHash(), entry.FromTx(block.vtx[1])); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(*block.vtx[1])); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + + uint256 txhash; // Test with pre-forwarding coinbase + tx 2 with tx 1 in mempool { @@ -215,7 +223,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) shortIDs.prefilledtxn[0] = {0, block.vtx[0]}; shortIDs.prefilledtxn[1] = {1, block.vtx[2]}; // id == 1 as it is 1 after index 1 shortIDs.shorttxids.resize(1); - shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[1].GetHash()); + shortIDs.shorttxids[0] = shortIDs.GetShortID(block.vtx[1]->GetHash()); CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); stream << shortIDs; @@ -229,19 +237,22 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest) BOOST_CHECK( partialBlock.IsTxAvailable(1)); BOOST_CHECK( partialBlock.IsTxAvailable(2)); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); CBlock block2; - std::vector vtx_missing; + std::vector> vtx_missing; BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString()); bool mutated; BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString()); BOOST_CHECK(!mutated); - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); + txhash = block.vtx[1]->GetHash(); + block.vtx.clear(); + block2.vtx.clear(); + BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); } - BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1].GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); + BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0); } BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) @@ -255,7 +266,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) CBlock block; block.vtx.resize(1); - block.vtx[0] = coinbase; + block.vtx[0] = std::make_shared(std::move(coinbase)); block.nVersion = 42; block.hashPrevBlock = GetRandHash(); block.nBits = 0x207fffff; @@ -280,7 +291,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest) BOOST_CHECK(partialBlock.IsTxAvailable(0)); CBlock block2; - std::vector vtx_missing; + std::vector> vtx_missing; BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_OK); BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString()); BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString()); diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index a73dbe725..4a6a06001 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -410,8 +410,8 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) CheckSort(pool, sortedOrder); /* after tx6 is mined, tx7 should move up in the sort */ - std::vector vtx; - vtx.push_back(tx6); + std::vector> vtx; + vtx.push_back(std::make_shared(tx6)); pool.removeForBlock(vtx, 1, NULL, false); sortedOrder.erase(sortedOrder.begin()+1); @@ -546,7 +546,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(tx5.GetHash(), entry.Fee(1000LL).FromTx(tx5, &pool)); pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7, &pool)); - std::vector vtx; + std::vector> vtx; SetMockTime(42); SetMockTime(42 + CTxMemPool::ROLLING_FEE_HALFLIFE); BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), maxFeeRateRemoved.GetFeePerK() + 1000); diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 706d30f48..8aea571e1 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -15,8 +15,8 @@ static uint256 BlockBuildMerkleTree(const CBlock& block, bool* fMutated, std::ve { vMerkleTree.clear(); vMerkleTree.reserve(block.vtx.size() * 2 + 16); // Safe upper bound for the number of total nodes. - for (std::vector::const_iterator it(block.vtx.begin()); it != block.vtx.end(); ++it) - vMerkleTree.push_back(it->GetHash()); + for (std::vector>::const_iterator it(block.vtx.begin()); it != block.vtx.end(); ++it) + vMerkleTree.push_back((*it)->GetHash()); int j = 0; bool mutated = false; for (int nSize = block.vtx.size(); nSize > 1; nSize = (nSize + 1) / 2) @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(merkle_test) for (int j = 0; j < ntx; j++) { CMutableTransaction mtx; mtx.nLockTime = j; - block.vtx[j] = mtx; + block.vtx[j] = std::make_shared(mtx); } // Compute the root of the block before mutating it. bool unmutatedMutated = false; @@ -126,7 +126,7 @@ BOOST_AUTO_TEST_CASE(merkle_test) std::vector newBranch = BlockMerkleBranch(block, mtx); std::vector oldBranch = BlockGetMerkleBranch(block, merkleTree, mtx); BOOST_CHECK(oldBranch == newBranch); - BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx].GetHash(), newBranch, mtx) == oldRoot); + BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx]->GetHash(), newBranch, mtx) == oldRoot); } } } diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 1ef70c343..56dab347f 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -77,7 +77,7 @@ bool TestSequenceLocks(const CTransaction &tx, int flags) // Implemented as an additional function, rather than a separate test case, // to allow reusing the blockchain created in CreateNewBlock_validity. // Note that this test assumes blockprioritysize is 0. -void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, std::vector& txFirst) +void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, std::vector>& txFirst) { // Test the ancestor feerate transaction selection. TestMemPoolEntryHelper entry; @@ -108,9 +108,9 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, mempool.addUnchecked(hashHighFeeTx, entry.Fee(50000).Time(GetTime()).SpendsCoinbase(false).FromTx(tx)); std::unique_ptr pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); - BOOST_CHECK(pblocktemplate->block.vtx[1].GetHash() == hashParentTx); - BOOST_CHECK(pblocktemplate->block.vtx[2].GetHash() == hashHighFeeTx); - BOOST_CHECK(pblocktemplate->block.vtx[3].GetHash() == hashMediumFeeTx); + BOOST_CHECK(pblocktemplate->block.vtx[1]->GetHash() == hashParentTx); + BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx); + BOOST_CHECK(pblocktemplate->block.vtx[3]->GetHash() == hashMediumFeeTx); // Test that a package below the min relay fee doesn't get included tx.vin[0].prevout.hash = hashHighFeeTx; @@ -130,8 +130,8 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); // Verify that the free tx and the low fee tx didn't get selected for (size_t i=0; iblock.vtx.size(); ++i) { - BOOST_CHECK(pblocktemplate->block.vtx[i].GetHash() != hashFreeTx); - BOOST_CHECK(pblocktemplate->block.vtx[i].GetHash() != hashLowFeeTx); + BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx); + BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashLowFeeTx); } // Test that packages above the min relay fee do get included, even if one @@ -142,8 +142,8 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, hashLowFeeTx = tx.GetHash(); mempool.addUnchecked(hashLowFeeTx, entry.Fee(feeToUse+2).FromTx(tx)); pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); - BOOST_CHECK(pblocktemplate->block.vtx[4].GetHash() == hashFreeTx); - BOOST_CHECK(pblocktemplate->block.vtx[5].GetHash() == hashLowFeeTx); + BOOST_CHECK(pblocktemplate->block.vtx[4]->GetHash() == hashFreeTx); + BOOST_CHECK(pblocktemplate->block.vtx[5]->GetHash() == hashLowFeeTx); // Test that transaction selection properly updates ancestor fee // calculations as ancestor transactions get included in a block. @@ -166,8 +166,8 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, // Verify that this tx isn't selected. for (size_t i=0; iblock.vtx.size(); ++i) { - BOOST_CHECK(pblocktemplate->block.vtx[i].GetHash() != hashFreeTx2); - BOOST_CHECK(pblocktemplate->block.vtx[i].GetHash() != hashLowFeeTx2); + BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashFreeTx2); + BOOST_CHECK(pblocktemplate->block.vtx[i]->GetHash() != hashLowFeeTx2); } // This tx will be mineable, and should cause hashLowFeeTx2 to be selected @@ -176,7 +176,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, tx.vout[0].nValue = 100000000 - 10000; // 10k satoshi fee mempool.addUnchecked(tx.GetHash(), entry.Fee(10000).FromTx(tx)); pblocktemplate = BlockAssembler(chainparams).CreateNewBlock(scriptPubKey); - BOOST_CHECK(pblocktemplate->block.vtx[8].GetHash() == hashLowFeeTx2); + BOOST_CHECK(pblocktemplate->block.vtx[8]->GetHash() == hashLowFeeTx2); } // NOTE: These tests rely on CreateNewBlock doing its own self-validation! @@ -203,23 +203,23 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) // We can't make transactions until we have inputs // Therefore, load 100 blocks :) int baseheight = 0; - std::vectortxFirst; + std::vector> txFirst; for (unsigned int i = 0; i < sizeof(blockinfo)/sizeof(*blockinfo); ++i) { CBlock *pblock = &pblocktemplate->block; // pointer for convenience pblock->nVersion = 1; pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; - CMutableTransaction txCoinbase(pblock->vtx[0]); + CMutableTransaction txCoinbase(*pblock->vtx[0]); txCoinbase.nVersion = 1; txCoinbase.vin[0].scriptSig = CScript(); txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); txCoinbase.vout[0].scriptPubKey = CScript(); - pblock->vtx[0] = CTransaction(txCoinbase); + pblock->vtx[0] = std::make_shared(std::move(txCoinbase)); if (txFirst.size() == 0) baseheight = chainActive.Height(); if (txFirst.size() < 4) - txFirst.push_back(new CTransaction(pblock->vtx[0])); + txFirst.push_back(pblock->vtx[0]); pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; BOOST_CHECK(ProcessNewBlock(chainparams, pblock, true, NULL, NULL)); @@ -485,9 +485,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) TestPackageSelection(chainparams, scriptPubKey, txFirst); - BOOST_FOREACH(CTransaction *_tx, txFirst) - delete _tx; - fCheckpointsEnabled = true; } diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp index c77312964..1552cb4ba 100644 --- a/src/test/pmt_tests.cpp +++ b/src/test/pmt_tests.cpp @@ -45,14 +45,14 @@ BOOST_AUTO_TEST_CASE(pmt_test1) for (unsigned int j=0; j(tx)); } // calculate actual merkle root and height uint256 merkleRoot1 = BlockMerkleRoot(block); std::vector vTxid(nTx, uint256()); for (unsigned int j=0; jGetHash(); int nHeight = 1, nTx_ = nTx; while (nTx_ > 1) { nTx_ = (nTx_+1)/2; diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp index 38aaaba26..08e5e774e 100644 --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -45,7 +45,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) CFeeRate baseRate(basefee, GetVirtualTransactionSize(tx)); // Create a fake block - std::vector block; + std::vector> block; int blocknum = 0; // Loop through 200 blocks @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) while (txHashes[9-h].size()) { std::shared_ptr ptx = mpool.get(txHashes[9-h].back()); if (ptx) - block.push_back(*ptx); + block.push_back(ptx); txHashes[9-h].pop_back(); } } @@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) while(txHashes[j].size()) { std::shared_ptr ptx = mpool.get(txHashes[j].back()); if (ptx) - block.push_back(*ptx); + block.push_back(ptx); txHashes[j].pop_back(); } } @@ -165,7 +165,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates) mpool.addUnchecked(hash, entry.Fee(feeV[j]).Time(GetTime()).Priority(0).Height(blocknum).FromTx(tx, &mpool)); std::shared_ptr ptx = mpool.get(hash); if (ptx) - block.push_back(*ptx); + block.push_back(ptx); } } diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp index 3f7416f23..f932cbe23 100644 --- a/src/test/test_bitcoin.cpp +++ b/src/test/test_bitcoin.cpp @@ -101,7 +101,7 @@ TestChain100Setup::TestChain100Setup() : TestingSetup(CBaseChainParams::REGTEST) { std::vector noTxns; CBlock b = CreateAndProcessBlock(noTxns, scriptPubKey); - coinbaseTxns.push_back(b.vtx[0]); + coinbaseTxns.push_back(*b.vtx[0]); } } @@ -119,7 +119,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector& // Replace mempool-selected txns with just coinbase plus passed-in txns: block.vtx.resize(1); BOOST_FOREACH(const CMutableTransaction& tx, txns) - block.vtx.push_back(tx); + block.vtx.push_back(std::make_shared(tx)); // IncrementExtraNonce creates a valid coinbase and merkleRoot unsigned int extraNonce = 0; IncrementExtraNonce(&block, chainActive.Tip(), extraNonce); @@ -137,12 +137,12 @@ TestChain100Setup::~TestChain100Setup() } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPool *pool) { +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction &tx, CTxMemPool *pool) { CTransaction txn(tx); return FromTx(txn, pool); } -CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CTransaction &txn, CTxMemPool *pool) { +CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransaction &txn, CTxMemPool *pool) { bool hasNoDependencies = pool ? pool->HasNoInputsOf(txn) : hadNoDependencies; // Hack to assume either its completely dependent on other mempool txs or not at all CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0; diff --git a/src/test/test_bitcoin.h b/src/test/test_bitcoin.h index 9819a7097..3dea20445 100644 --- a/src/test/test_bitcoin.h +++ b/src/test/test_bitcoin.h @@ -79,8 +79,8 @@ struct TestMemPoolEntryHelper nFee(0), nTime(0), dPriority(0.0), nHeight(1), hadNoDependencies(false), spendsCoinbase(false), sigOpCost(4) { } - CTxMemPoolEntry FromTx(CMutableTransaction &tx, CTxMemPool *pool = NULL); - CTxMemPoolEntry FromTx(CTransaction &tx, CTxMemPool *pool = NULL); + CTxMemPoolEntry FromTx(const CMutableTransaction &tx, CTxMemPool *pool = NULL); + CTxMemPoolEntry FromTx(const CTransaction &tx, CTxMemPool *pool = NULL); // Change the default value TestMemPoolEntryHelper &Fee(CAmount _fee) { nFee = _fee; return *this; } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 45135a5f7..fff20a609 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -596,29 +596,29 @@ void CTxMemPool::removeConflicts(const CTransaction &tx, std::vector& vtx, unsigned int nBlockHeight, +void CTxMemPool::removeForBlock(const std::vector>& vtx, unsigned int nBlockHeight, std::vector>* conflicts, bool fCurrentEstimate) { LOCK(cs); std::vector entries; - BOOST_FOREACH(const CTransaction& tx, vtx) + for (const auto& tx : vtx) { - uint256 hash = tx.GetHash(); + uint256 hash = tx->GetHash(); indexed_transaction_set::iterator i = mapTx.find(hash); if (i != mapTx.end()) entries.push_back(*i); } - BOOST_FOREACH(const CTransaction& tx, vtx) + for (const auto& tx : vtx) { - txiter it = mapTx.find(tx.GetHash()); + txiter it = mapTx.find(tx->GetHash()); if (it != mapTx.end()) { setEntries stage; stage.insert(it); RemoveStaged(stage, true); } - removeConflicts(tx, conflicts); - ClearPrioritisation(tx.GetHash()); + removeConflicts(*tx, conflicts); + ClearPrioritisation(tx->GetHash()); } // After the txs in the new block have been removed from the mempool, update policy estimates minerPolicyEstimator->processBlock(nBlockHeight, entries, fCurrentEstimate); diff --git a/src/txmempool.h b/src/txmempool.h index 9b0ca4655..6952cc900 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -530,7 +530,7 @@ public: void removeRecursive(const CTransaction &tx, std::vector>* removed = NULL); void removeForReorg(const CCoinsViewCache *pcoins, unsigned int nMemPoolHeight, int flags); void removeConflicts(const CTransaction &tx, std::vector>* removed = NULL); - void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight, + void removeForBlock(const std::vector>& vtx, unsigned int nBlockHeight, std::vector>* conflicts = NULL, bool fCurrentEstimate = true); void clear(); void _clear(); //lock free diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3e18cb702..2e512fd57 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1492,7 +1492,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) int posInBlock; for (posInBlock = 0; posInBlock < (int)block.vtx.size(); posInBlock++) { - if (AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate)) + if (AddToWalletIfInvolvingMe(*block.vtx[posInBlock], pindex, posInBlock, fUpdate)) ret++; } pindex = chainActive.Next(pindex);