[refactor] rewrite vTxHashes as a vector of CTransactionRef

vTxHashes exposes a complex mapTx iterator type that its external users
don't need. Directly populate it with CTransactionRef instead.

4 files changed, 10 insertions, 8 deletions

diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 211b4740be..9b8f49cbc8 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -107,12 +107,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
     std::vector<bool> have_txn(txn_available.size());
     {
     LOCK(pool->cs);
-    for (size_t i = 0; i < pool->vTxHashes.size(); i++) {
-        uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first);
+    for (const auto& tx : pool->vTxHashes) {
+        uint64_t shortid = cmpctblock.GetShortID(tx->GetWitnessHash());
         std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid);
         if (idit != shorttxids.end()) {
             if (!have_txn[idit->second]) {
-                txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx();
+                txn_available[idit->second] = tx;
                 have_txn[idit->second]  = true;
                 mempool_count++;
             } else {
diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 8709e0c623..33894fd076 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -51,8 +51,8 @@ static CBlock BuildBlockTestCase() {
 }
 
 // Number of shared use_counts we expect for a tx we haven't touched
-// (block + mempool + our copy from the GetSharedTx call)
-constexpr long SHARED_TX_OFFSET{3};
+// (block + mempool entry + mempool vTxHashes + our copy from the GetSharedTx call)
+constexpr long SHARED_TX_OFFSET{4};
 
 BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
 {
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index e39f897544..14f4d3c032 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -480,7 +480,7 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
         minerPolicyEstimator->processTransaction(entry, validFeeEstimate);
     }
 
-    vTxHashes.emplace_back(tx.GetWitnessHash(), newit);
+    vTxHashes.emplace_back(newit->GetSharedTx());
     newit->vTxHashesIdx = vTxHashes.size() - 1;
 
     TRACE3(mempool, added,
@@ -518,8 +518,10 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
     RemoveUnbroadcastTx(hash, true /* add logging because unchecked */ );
 
     if (vTxHashes.size() > 1) {
+        // Update vTxHashesIdx of the to-be-moved entry.
+        Assert(GetEntry(vTxHashes.back()->GetHash()))->vTxHashesIdx = it->vTxHashesIdx;
+        // Remove entry from vTxHashes by replacing it with the back and deleting the back.
         vTxHashes[it->vTxHashesIdx] = std::move(vTxHashes.back());
-        vTxHashes[it->vTxHashesIdx].second->vTxHashesIdx = it->vTxHashesIdx;
         vTxHashes.pop_back();
         if (vTxHashes.size() * 2 < vTxHashes.capacity())
             vTxHashes.shrink_to_fit();
diff --git a/src/txmempool.h b/src/txmempool.h
index d4418fa407..fa1c62217b 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -392,7 +392,7 @@ public:
     indexed_transaction_set mapTx GUARDED_BY(cs);
 
     using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;
-    std::vector<std::pair<uint256, txiter>> vTxHashes GUARDED_BY(cs); //!< All tx witness hashes/entries in mapTx, in random order
+    std::vector<CTransactionRef> vTxHashes GUARDED_BY(cs); //!< All transactions in mapTx, in random order
 
     typedef std::set<txiter, CompareIteratorByHash> setEntries;