From e469547f3d464b2570a554d6e155ef47459a1856 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 30 May 2016 17:06:24 +0200 Subject: [PATCH] Switch CTransaction storage in mempool to std::shared_ptr (cherry picked from commit bitcoin/bitcoin@8d39d7a2cf1559e0ba40681b0ab90f13ea6c0618) --- src/main.cpp | 35 +++++++++---------- src/txmempool.cpp | 87 +++++++++++++++++++++++++++++++++++------------ src/txmempool.h | 27 +++++++++++++-- 3 files changed, 107 insertions(+), 42 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a97c00b8a..1151ad346 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6148,10 +6148,11 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam } } if (!push && inv.type == MSG_TX) { - int64_t txtime; + auto txinfo = mempool.info(inv.hash); // To protect privacy, do not answer getdata using the mempool when // that TX couldn't have been INVed in reply to a MEMPOOL request. - if (mempool.lookup(inv.hash, tx, txtime) && txtime <= pfrom->timeLastMempoolReq && !IsExpiringSoonTx(tx, currentHeight + 1)) { + if (txinfo.tx && txinfo.nTime <= pfrom->timeLastMempoolReq && !IsExpiringSoonTx(*txinfo.tx, currentHeight + 1)) { + tx = *txinfo.tx; push = true; } } @@ -7446,27 +7447,22 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) if (!pto->fRelayTxes) pto->setInventoryTxToSend.clear(); } + int currentHeight = GetHeight(); + // Respond to BIP35 mempool requests if (fSendTrickle && pto->fSendMempool) { - std::vector vtxid; - mempool.queryHashes(vtxid); + auto vtxinfo = mempool.infoAll(); pto->fSendMempool = false; LOCK(pto->cs_filter); - int currentHeight = GetHeight(); - for (const uint256& hash : vtxid) { - CTransaction tx; - bool fInMemPool = mempool.lookup(hash, tx); - if (fInMemPool && IsExpiringSoonTx(tx, currentHeight + 1)) { - continue; - } - + for (const auto& txinfo : vtxinfo) { + const uint256& hash = txinfo.tx->GetHash(); CInv inv(MSG_TX, hash); pto->setInventoryTxToSend.erase(hash); + if (IsExpiringSoonTx(*txinfo.tx, currentHeight + 1)) continue; if (pto->pfilter) { - if (!fInMemPool) continue; // another thread removed since queryHashes, maybe... - if (!pto->pfilter->IsRelevantAndUpdate(tx)) continue; + if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; } pto->filterInventoryKnown.insert(hash); vInv.push_back(inv); @@ -7507,9 +7503,12 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) continue; } // Not in the mempool anymore? don't bother sending it. - CTransaction tx; - if (!mempool.lookup(hash, tx)) continue; - if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(tx)) continue; + auto txinfo = mempool.info(hash); + if (!txinfo.tx) { + continue; + } + if (IsExpiringSoonTx(*txinfo.tx, currentHeight + 1)) continue; + if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send vInv.push_back(CInv(MSG_TX, hash)); nRelayedTransactions++; @@ -7522,7 +7521,7 @@ bool SendMessages(const Consensus::Params& params, CNode* pto) vRelayExpiration.pop_front(); } - auto ret = mapRelay.insert(std::make_pair(hash, tx)); + auto ret = mapRelay.insert(std::make_pair(hash, *txinfo.tx)); if (ret.second) { vRelayExpiration.push_back(std::make_pair(GetTime() + 15 * 60, hash)); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 963b42527..63460bbae 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -32,13 +32,13 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, int64_t _nTime, double _dPriority, unsigned int _nHeight, bool poolHasNoInputsOf, bool _spendsCoinbase, unsigned int _sigOps, uint32_t _nBranchId): - tx(_tx), nFee(_nFee), nTime(_nTime), dPriority(_dPriority), nHeight(_nHeight), + tx(std::make_shared(_tx)), nFee(_nFee), nTime(_nTime), dPriority(_dPriority), nHeight(_nHeight), hadNoDependencies(poolHasNoInputsOf), spendsCoinbase(_spendsCoinbase), sigOpCount(_sigOps), nBranchId(_nBranchId) { - nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); - nModSize = tx.CalculateModifiedSize(nTxSize); - nUsageSize = RecursiveDynamicUsage(tx); + nTxSize = ::GetSerializeSize(_tx, SER_NETWORK, PROTOCOL_VERSION); + nModSize = _tx.CalculateModifiedSize(nTxSize); + nUsageSize = RecursiveDynamicUsage(*tx) + memusage::DynamicUsage(tx); feeRate = CFeeRate(nFee, nTxSize); feeDelta = 0; @@ -52,7 +52,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTxMemPoolEntry& other) double CTxMemPoolEntry::GetPriority(unsigned int currentHeight) const { - CAmount nValueIn = tx.GetValueOut()+nFee; + CAmount nValueIn = tx->GetValueOut()+nFee; double deltaPriority = ((double)(currentHeight-nHeight)*nValueIn)/nModSize; double dResult = dPriority + deltaPriority; return dResult; @@ -692,40 +692,85 @@ bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb namespace { class DepthAndScoreComparator { - CTxMemPool *mp; public: - DepthAndScoreComparator(CTxMemPool *mempool) : mp(mempool) {} - bool operator()(const uint256& a, const uint256& b) { return mp->CompareDepthAndScore(a, b); } + bool operator()(const CTxMemPool::indexed_transaction_set::const_iterator& a, const CTxMemPool::indexed_transaction_set::const_iterator& b) + { + // Same logic applies here as to CTxMemPool::CompareDepthAndScore(). + // As of https://github.com/bitcoin/bitcoin/pull/8126 this comparator + // duplicates that function (as it doesn't need to hold a dependency + // on the mempool). + return CompareTxMemPoolEntryByScore()(*a, *b); + } }; } +std::vector CTxMemPool::GetSortedDepthAndScore() const +{ + std::vector iters; + AssertLockHeld(cs); + + iters.reserve(mapTx.size()); + + for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) { + iters.push_back(mi); + } + std::sort(iters.begin(), iters.end(), DepthAndScoreComparator()); + return iters; +} + void CTxMemPool::queryHashes(std::vector& vtxid) { - vtxid.clear(); - LOCK(cs); - vtxid.reserve(mapTx.size()); - for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi) - vtxid.push_back(mi->GetTx().GetHash()); + auto iters = GetSortedDepthAndScore(); - std::sort(vtxid.begin(), vtxid.end(), DepthAndScoreComparator(this)); + vtxid.clear(); + vtxid.reserve(mapTx.size()); + + for (auto it : iters) { + vtxid.push_back(it->GetTx().GetHash()); + } } +std::vector CTxMemPool::infoAll() const +{ + LOCK(cs); + auto iters = GetSortedDepthAndScore(); -bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const + std::vector ret; + ret.reserve(mapTx.size()); + for (auto it : iters) { + ret.push_back(TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize())}); + } + + return ret; +} + +std::shared_ptr CTxMemPool::get(const uint256& hash) const { LOCK(cs); indexed_transaction_set::const_iterator i = mapTx.find(hash); - if (i == mapTx.end()) return false; - result = i->GetTx(); - time = i->GetTime(); - return true; + if (i == mapTx.end()) + return nullptr; + return i->GetSharedTx(); } bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const { - int64_t time; - return CTxMemPool::lookup(hash, result, time); + auto tx = get(hash); + if (tx) { + result = *tx; + return true; + } + return false; +} + +TxMempoolInfo CTxMemPool::info(const uint256& hash) const +{ + LOCK(cs); + indexed_transaction_set::const_iterator i = mapTx.find(hash); + if (i == mapTx.end()) + return TxMempoolInfo(); + return TxMempoolInfo{i->GetSharedTx(), i->GetTime(), CFeeRate(i->GetFee(), i->GetTxSize())}; } CFeeRate CTxMemPool::estimateFee(int nBlocks) const diff --git a/src/txmempool.h b/src/txmempool.h index e33d7d773..005d49afc 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -7,6 +7,7 @@ #define BITCOIN_TXMEMPOOL_H #include +#include #include "amount.h" #include "coins.h" @@ -45,7 +46,7 @@ static const unsigned int MEMPOOL_HEIGHT = 0x7FFFFFFF; class CTxMemPoolEntry { private: - CTransaction tx; + std::shared_ptr tx; CAmount nFee; //!< Cached to avoid expensive parent-transaction lookups size_t nTxSize; //!< ... and avoid recomputing tx size size_t nModSize; //!< ... and modified size for priority @@ -68,7 +69,8 @@ public: CTxMemPoolEntry(); CTxMemPoolEntry(const CTxMemPoolEntry& other); - const CTransaction& GetTx() const { return this->tx; } + const CTransaction& GetTx() const { return *this->tx; } + std::shared_ptr GetSharedTx() const { return this->tx; } double GetPriority(unsigned int currentHeight) const; const CAmount& GetFee() const { return nFee; } CFeeRate GetFeeRate() const { return feeRate; } @@ -152,6 +154,21 @@ public: size_t DynamicMemoryUsage() const { return 0; } }; +/** + * Information about a mempool transaction. + */ +struct TxMempoolInfo +{ + /** The transaction itself */ + std::shared_ptr tx; + + /** Time the transaction entered the mempool. */ + int64_t nTime; + + /** Feerate of the transaction. */ + CFeeRate feeRate; +}; + /** * CTxMemPool stores valid-according-to-the-current-best-chain * transactions that may be included in the next block. @@ -224,6 +241,8 @@ private: std::map mapSpent; std::map> mapSpentInserted; + std::vector GetSortedDepthAndScore() const; + public: std::map mapNextTx; std::map > mapDeltas; @@ -309,7 +328,9 @@ public: } bool lookup(uint256 hash, CTransaction& result) const; - bool lookup(uint256 hash, CTransaction& result, int64_t& time) const; + std::shared_ptr get(const uint256& hash) const; + TxMempoolInfo info(const uint256& hash) const; + std::vector infoAll() const; /** Estimate fee rate needed to get into the next nBlocks */ CFeeRate estimateFee(int nBlocks) const;