Switch CTransaction storage in mempool to std::shared_ptr

(cherry picked from commit bitcoin/bitcoin@8d39d7a2cf)

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<uint256> vtxid;
+                auto vtxinfo = mempool.infoAll();
-                mempool.queryHashes(vtxid);
                 pto->fSendMempool = false;
 
                 LOCK(pto->cs_filter);
 
-                int currentHeight = GetHeight();
+                for (const auto& txinfo : vtxinfo) {
-                for (const uint256& hash : vtxid) {
+                    const uint256& hash = txinfo.tx->GetHash();
-                    CTransaction tx;
-                    bool fInMemPool = mempool.lookup(hash, tx);
-                    if (fInMemPool && IsExpiringSoonTx(tx, currentHeight + 1)) {
-                        continue;
-                    }
-
                     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(*txinfo.tx)) continue;
-                        if (!pto->pfilter->IsRelevantAndUpdate(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;
+                    auto txinfo = mempool.info(hash);
-                    if (!mempool.lookup(hash, tx)) continue;
+                    if (!txinfo.tx) {
-                    if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(tx)) continue;
+                        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));
                         }

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<CTransaction>(_tx)), nFee(_nFee), nTime(_nTime), dPriority(_dPriority), nHeight(_nHeight),
     hadNoDependencies(poolHasNoInputsOf),
     spendsCoinbase(_spendsCoinbase), sigOpCount(_sigOps), nBranchId(_nBranchId)
 {
-    nTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
+    nTxSize = ::GetSerializeSize(_tx, SER_NETWORK, PROTOCOL_VERSION);
-    nModSize = tx.CalculateModifiedSize(nTxSize);
+    nModSize = _tx.CalculateModifiedSize(nTxSize);
-    nUsageSize = RecursiveDynamicUsage(tx);
+    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 CTxMemPool::indexed_transaction_set::const_iterator& a, const CTxMemPool::indexed_transaction_set::const_iterator& b)
-    bool operator()(const uint256& a, const uint256& b) { return mp->CompareDepthAndScore(a, 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::indexed_transaction_set::const_iterator> CTxMemPool::GetSortedDepthAndScore() const
+{
+    std::vector<indexed_transaction_set::const_iterator> 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<uint256>& vtxid)
 {
-    vtxid.clear();
-
     LOCK(cs);
-    vtxid.reserve(mapTx.size());
+    auto iters = GetSortedDepthAndScore();
-    for (indexed_transaction_set::iterator mi = mapTx.begin(); mi != mapTx.end(); ++mi)
-        vtxid.push_back(mi->GetTx().GetHash());
 
-    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<TxMempoolInfo> CTxMemPool::infoAll() const
+{
+    LOCK(cs);
+    auto iters = GetSortedDepthAndScore();
 
-bool CTxMemPool::lookup(uint256 hash, CTransaction& result, int64_t& time) const
+    std::vector<TxMempoolInfo> 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<const CTransaction> CTxMemPool::get(const uint256& hash) const
 {
     LOCK(cs);
     indexed_transaction_set::const_iterator i = mapTx.find(hash);
-    if (i == mapTx.end()) return false;
+    if (i == mapTx.end())
-    result = i->GetTx();
+        return nullptr;
-    time = i->GetTime();
+    return i->GetSharedTx();
-    return true;
 }
 
 bool CTxMemPool::lookup(uint256 hash, CTransaction& result) const
 {
-    int64_t time;
+    auto tx = get(hash);
-    return CTxMemPool::lookup(hash, result, time);
+    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

src/txmempool.h

@@ -7,6 +7,7 @@
 #define BITCOIN_TXMEMPOOL_H
 
 #include <list>
+#include <memory>
 
 #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<const CTransaction> 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<const CTransaction> 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<const CTransaction> 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<CSpentIndexKey, CSpentIndexValue, CSpentIndexKeyCompare> mapSpent;
     std::map<uint256, std::vector<CSpentIndexKey>> mapSpentInserted;
 
+    std::vector<indexed_transaction_set::const_iterator> GetSortedDepthAndScore() const;
+
 public:
     std::map<COutPoint, CInPoint> mapNextTx;
     std::map<uint256, std::pair<double, CAmount> > 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<const CTransaction> get(const uint256& hash) const;
+    TxMempoolInfo info(const uint256& hash) const;
+    std::vector<TxMempoolInfo> infoAll() const;
 
     /** Estimate fee rate needed to get into the next nBlocks */
     CFeeRate estimateFee(int nBlocks) const;