From 453b4813ebc74859864803e9972b58e4be76a4d6 Mon Sep 17 00:00:00 2001
From: stickies-v <stickies-v@protonmail.com>
Date: Thu, 9 Nov 2023 17:51:20 +0100
Subject: [PATCH 01/15] [refactor] Add helper for iterating through mempool
 entries

Instead of reaching into the mapTx data structure, use a helper method
that provides the required vector of CTxMemPoolEntry pointers.
---
 src/kernel/mempool_entry.h |  2 ++
 src/node/interfaces.cpp    |  2 +-
 src/rpc/mempool.cpp        |  5 ++---
 src/txmempool.cpp          | 12 ++++++++++++
 src/txmempool.h            |  1 +
 5 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index 1f175a5ccf9..edd95e8d53a 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -176,4 +176,6 @@ public:
     mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms
 };
 
+using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef;
+
 #endif // BITCOIN_KERNEL_MEMPOOL_ENTRY_H
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index f6dbe4f0087..39302807974 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -806,7 +806,7 @@ public:
     {
         if (!m_node.mempool) return;
         LOCK2(::cs_main, m_node.mempool->cs);
-        for (const CTxMemPoolEntry& entry : m_node.mempool->mapTx) {
+        for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) {
             notifications.transactionAddedToMempool(entry.GetSharedTx());
         }
     }
diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 23cef42d30e..43691b7a091 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -344,14 +344,13 @@ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempoo
         }
         LOCK(pool.cs);
         UniValue o(UniValue::VOBJ);
-        for (const CTxMemPoolEntry& e : pool.mapTx) {
-            const uint256& hash = e.GetTx().GetHash();
+        for (const CTxMemPoolEntry& e : pool.entryAll()) {
             UniValue info(UniValue::VOBJ);
             entryToJSON(pool, info, e);
             // Mempool has unique entries so there is no advantage in using
             // UniValue::pushKV, which checks if the key already exists in O(N).
             // UniValue::pushKVEnd is used instead which currently is O(1).
-            o.pushKVEnd(hash.ToString(), info);
+            o.pushKVEnd(e.GetTx().GetHash().ToString(), info);
         }
         return o;
     } else {
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 461662ad93a..8b744698baa 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -836,6 +836,18 @@ static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator
     return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()};
 }
 
+std::vector<CTxMemPoolEntryRef> CTxMemPool::entryAll() const
+{
+    AssertLockHeld(cs);
+
+    std::vector<CTxMemPoolEntryRef> ret;
+    ret.reserve(mapTx.size());
+    for (const auto& it : GetSortedDepthAndScore()) {
+        ret.emplace_back(*it);
+    }
+    return ret;
+}
+
 std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
 {
     LOCK(cs);
diff --git a/src/txmempool.h b/src/txmempool.h
index cbeabb31fa6..fd7006ab440 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -695,6 +695,7 @@ public:
     /** Returns info for a transaction if its entry_sequence < last_sequence */
     TxMempoolInfo info_for_relay(const GenTxid& gtxid, uint64_t last_sequence) const;
 
+    std::vector<CTxMemPoolEntryRef> entryAll() const EXCLUSIVE_LOCKS_REQUIRED(cs);
     std::vector<TxMempoolInfo> infoAll() const;
 
     size_t DynamicMemoryUsage() const;

From 1c6a73abbd1fb773c7d0036beb952b95dde8e38b Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Thu, 2 Nov 2023 15:50:45 +0100
Subject: [PATCH 02/15] [refactor] Add helper for retrieving mempool entry

In places where the iterator is only needed for accessing the actual
entry, it should not be required to first retrieve the iterator.
---
 src/node/interfaces.cpp      |  5 +++--
 src/test/miniminer_tests.cpp | 20 ++++++++++----------
 src/txmempool.cpp            |  7 +++++++
 src/txmempool.h              |  2 ++
 src/validation.cpp           | 10 ++++------
 5 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 39302807974..153b4728ab9 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -648,8 +648,9 @@ public:
     {
         if (!m_node.mempool) return false;
         LOCK(m_node.mempool->cs);
-        auto it = m_node.mempool->GetIter(txid);
-        return it && (*it)->GetCountWithDescendants() > 1;
+        const auto entry{m_node.mempool->GetEntry(Txid::FromUint256(txid))};
+        if (entry == nullptr) return false;
+        return entry->GetCountWithDescendants() > 1;
     }
     bool broadcastTransaction(const CTransactionRef& tx,
         const CAmount& max_tx_fee,
diff --git a/src/test/miniminer_tests.cpp b/src/test/miniminer_tests.cpp
index 76c079a6e76..2eb82ae7489 100644
--- a/src/test/miniminer_tests.cpp
+++ b/src/test/miniminer_tests.cpp
@@ -94,7 +94,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup)
     const CFeeRate feerate_zero(0);
     mini_miner_target0.BuildMockTemplate(feerate_zero);
     // Check the quit condition:
-    BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetIter(tx_mod_negative->GetHash()).value()->GetTxSize()));
+    BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(Assert(pool.GetEntry(tx_mod_negative->GetHash()))->GetTxSize()));
     BOOST_CHECK(mini_miner_target0.GetMockTemplateTxids().empty());
 
     // With no target feerate, the template includes all transactions, even negative feerate ones.
@@ -179,9 +179,9 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
     };
     std::map<uint256, TxDimensions> tx_dims;
     for (const auto& tx : all_transactions) {
-        const auto it = pool.GetIter(tx->GetHash()).value();
-        tx_dims.emplace(tx->GetHash(), TxDimensions{it->GetTxSize(), it->GetModifiedFee(),
-                                              CFeeRate(it->GetModifiedFee(), it->GetTxSize())});
+        const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
+        tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(),
+                                              CFeeRate(entry.GetModifiedFee(), entry.GetTxSize())});
     }
 
     const std::vector<CFeeRate> various_normal_feerates({CFeeRate(0), CFeeRate(500), CFeeRate(999),
@@ -447,15 +447,15 @@ BOOST_FIXTURE_TEST_CASE(miniminer_overlap, TestChain100Setup)
     // tx3's feerate is lower than tx2's. same fee, different weight.
     BOOST_CHECK(tx2_feerate > tx3_feerate);
     const auto tx3_anc_feerate = CFeeRate(low_fee + med_fee + high_fee + high_fee, tx_vsizes[0] + tx_vsizes[1] + tx_vsizes[2] + tx_vsizes[3]);
-    const auto tx3_iter = pool.GetIter(tx3->GetHash());
-    BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_iter.value()->GetModFeesWithAncestors(), tx3_iter.value()->GetSizeWithAncestors()));
+    const auto& tx3_entry{*Assert(pool.GetEntry(tx3->GetHash()))};
+    BOOST_CHECK(tx3_anc_feerate == CFeeRate(tx3_entry.GetModFeesWithAncestors(), tx3_entry.GetSizeWithAncestors()));
     const auto tx4_feerate = CFeeRate(high_fee, tx_vsizes[4]);
     const auto tx6_anc_feerate = CFeeRate(high_fee + low_fee + med_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[6]);
-    const auto tx6_iter = pool.GetIter(tx6->GetHash());
-    BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_iter.value()->GetModFeesWithAncestors(), tx6_iter.value()->GetSizeWithAncestors()));
+    const auto& tx6_entry{*Assert(pool.GetEntry(tx6->GetHash()))};
+    BOOST_CHECK(tx6_anc_feerate == CFeeRate(tx6_entry.GetModFeesWithAncestors(), tx6_entry.GetSizeWithAncestors()));
     const auto tx7_anc_feerate = CFeeRate(high_fee + low_fee + high_fee, tx_vsizes[4] + tx_vsizes[5] + tx_vsizes[7]);
-    const auto tx7_iter = pool.GetIter(tx7->GetHash());
-    BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_iter.value()->GetModFeesWithAncestors(), tx7_iter.value()->GetSizeWithAncestors()));
+    const auto& tx7_entry{*Assert(pool.GetEntry(tx7->GetHash()))};
+    BOOST_CHECK(tx7_anc_feerate == CFeeRate(tx7_entry.GetModFeesWithAncestors(), tx7_entry.GetSizeWithAncestors()));
     BOOST_CHECK(tx4_feerate > tx6_anc_feerate);
     BOOST_CHECK(tx4_feerate > tx7_anc_feerate);
 
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 8b744698baa..e39f8975448 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -862,6 +862,13 @@ std::vector<TxMempoolInfo> CTxMemPool::infoAll() const
     return ret;
 }
 
+const CTxMemPoolEntry* CTxMemPool::GetEntry(const Txid& txid) const
+{
+    AssertLockHeld(cs);
+    const auto i = mapTx.find(txid);
+    return i == mapTx.end() ? nullptr : &(*i);
+}
+
 CTransactionRef CTxMemPool::get(const uint256& hash) const
 {
     LOCK(cs);
diff --git a/src/txmempool.h b/src/txmempool.h
index fd7006ab440..d4418fa4072 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -684,6 +684,8 @@ public:
         return (mapTx.count(gtxid.GetHash()) != 0);
     }
 
+    const CTxMemPoolEntry* GetEntry(const Txid& txid) const LIFETIMEBOUND EXCLUSIVE_LOCKS_REQUIRED(cs);
+
     CTransactionRef get(const uint256& hash) const;
     txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs)
     {
diff --git a/src/validation.cpp b/src/validation.cpp
index 8d7e3661254..ec99d927014 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1485,9 +1485,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             // transactions that are already in the mempool, and only call AcceptMultipleTransactions() with
             // the new transactions. This ensures we don't double-count transaction counts and sizes when
             // checking ancestor/descendant limits, or double-count transaction fees for fee-related policy.
-            auto iter = m_pool.GetIter(txid);
-            assert(iter != std::nullopt);
-            results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee()));
+            const auto& entry{*Assert(m_pool.GetEntry(txid))};
+            results_final.emplace(wtxid, MempoolAcceptResult::MempoolTx(entry.GetTxSize(), entry.GetFee()));
         } else if (m_pool.exists(GenTxid::Txid(txid))) {
             // Transaction with the same non-witness data but different witness (same txid,
             // different wtxid) already exists in the mempool.
@@ -1496,10 +1495,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
             // transaction for the mempool one. Note that we are ignoring the validity of the
             // package transaction passed in.
             // TODO: allow witness replacement in packages.
-            auto iter = m_pool.GetIter(txid);
-            assert(iter != std::nullopt);
+            const auto& entry{*Assert(m_pool.GetEntry(txid))};
             // Provide the wtxid of the mempool tx so that the caller can look it up in the mempool.
-            results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(iter.value()->GetTx().GetWitnessHash()));
+            results_final.emplace(wtxid, MempoolAcceptResult::MempoolTxDifferentWitness(entry.GetTx().GetWitnessHash()));
         } else {
             // Transaction does not already exist in the mempool.
             // Try submitting the transaction on its own.

From 14804699e59794e61dcfb02ff1971db96e9a06ce Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 12:31:14 +0100
Subject: [PATCH 03/15] [refactor] remove access to mapTx from policy/rbf.cpp

---
 src/policy/rbf.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp
index d032b740088..76ab2b1a969 100644
--- a/src/policy/rbf.cpp
+++ b/src/policy/rbf.cpp
@@ -12,6 +12,7 @@
 #include <tinyformat.h>
 #include <txmempool.h>
 #include <uint256.h>
+#include <util/check.h>
 #include <util/moneystr.h>
 #include <util/rbf.h>
 
@@ -35,7 +36,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
 
     // If all the inputs have nSequence >= maxint-1, it still might be
     // signaled for RBF if any unconfirmed parents have signaled.
-    const CTxMemPoolEntry entry{*pool.mapTx.find(tx.GetHash())};
+    const auto& entry{*Assert(pool.GetEntry(tx.GetHash()))};
     auto ancestors{pool.AssumeCalculateMemPoolAncestors(__func__, entry, CTxMemPool::Limits::NoLimits(),
                                                         /*fSearchForParents=*/false)};
 

From 9cd8cafb77563243af19c95e396c2fb4fc3758df Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 15:53:56 +0100
Subject: [PATCH 04/15] [refactor] use exists() instead of mapTx.find()

---
 src/validation.cpp | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index ec99d927014..e1838a85892 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -362,9 +362,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
         // If the transaction spends any coinbase outputs, it must be mature.
         if (it->GetSpendsCoinbase()) {
             for (const CTxIn& txin : tx.vin) {
-                auto it2 = m_mempool->mapTx.find(txin.prevout.hash);
-                if (it2 != m_mempool->mapTx.end())
-                    continue;
+                if (m_mempool->exists(GenTxid::Txid(txin.prevout.hash))) continue;
                 const Coin& coin{CoinsTip().AccessCoin(txin.prevout)};
                 assert(!coin.IsSpent());
                 const auto mempool_spend_height{m_chain.Tip()->nHeight + 1};

From fad61aa56189f98d97af1d6f70c4eb46b8f98bf0 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 25 Aug 2023 15:15:10 +0100
Subject: [PATCH 05/15] [refactor] get wtxid from entry instead of vTxHashes

---
 src/rpc/mempool.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index 43691b7a091..a97bf0b112b 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -289,7 +289,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
     info.pushKV("descendantsize", e.GetSizeWithDescendants());
     info.pushKV("ancestorcount", e.GetCountWithAncestors());
     info.pushKV("ancestorsize", e.GetSizeWithAncestors());
-    info.pushKV("wtxid", pool.vTxHashes[e.vTxHashesIdx].first.ToString());
+    info.pushKV("wtxid", e.GetTx().GetWitnessHash().ToString());
 
     UniValue fees(UniValue::VOBJ);
     fees.pushKV("base", ValueFromAmount(e.GetFee()));

From 8892d6b744e3cbda2cf93721f573ffa7017bd898 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 12:29:56 +0100
Subject: [PATCH 06/15] [refactor] remove access to mapTx from rpc/mempool.cpp

---
 src/rpc/mempool.cpp | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp
index a97bf0b112b..dbef5b07646 100644
--- a/src/rpc/mempool.cpp
+++ b/src/rpc/mempool.cpp
@@ -315,9 +315,7 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
     info.pushKV("depends", depends);
 
     UniValue spent(UniValue::VARR);
-    const CTxMemPool::txiter& it = pool.mapTx.find(tx.GetHash());
-    const CTxMemPoolEntry::Children& children = it->GetMemPoolChildrenConst();
-    for (const CTxMemPoolEntry& child : children) {
+    for (const CTxMemPoolEntry& child : e.GetMemPoolChildrenConst()) {
         spent.push_back(child.GetTx().GetHash().ToString());
     }
 
@@ -459,12 +457,12 @@ static RPCHelpMan getmempoolancestors()
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
 
-    CTxMemPool::txiter it = mempool.mapTx.find(hash);
-    if (it == mempool.mapTx.end()) {
+    const auto entry{mempool.GetEntry(Txid::FromUint256(hash))};
+    if (entry == nullptr) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
     }
 
-    auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *it, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)};
+    auto ancestors{mempool.AssumeCalculateMemPoolAncestors(self.m_name, *entry, CTxMemPool::Limits::NoLimits(), /*fSearchForParents=*/false)};
 
     if (!fVerbose) {
         UniValue o(UniValue::VARR);
@@ -520,15 +518,15 @@ static RPCHelpMan getmempooldescendants()
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
 
-    CTxMemPool::txiter it = mempool.mapTx.find(hash);
-    if (it == mempool.mapTx.end()) {
+    const auto it{mempool.GetIter(hash)};
+    if (!it) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
     }
 
     CTxMemPool::setEntries setDescendants;
-    mempool.CalculateDescendants(it, setDescendants);
+    mempool.CalculateDescendants(*it, setDescendants);
     // CTxMemPool::CalculateDescendants will include the given tx
-    setDescendants.erase(it);
+    setDescendants.erase(*it);
 
     if (!fVerbose) {
         UniValue o(UniValue::VARR);
@@ -572,14 +570,13 @@ static RPCHelpMan getmempoolentry()
     const CTxMemPool& mempool = EnsureAnyMemPool(request.context);
     LOCK(mempool.cs);
 
-    CTxMemPool::txiter it = mempool.mapTx.find(hash);
-    if (it == mempool.mapTx.end()) {
+    const auto entry{mempool.GetEntry(Txid::FromUint256(hash))};
+    if (entry == nullptr) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not in mempool");
     }
 
-    const CTxMemPoolEntry &e = *it;
     UniValue info(UniValue::VOBJ);
-    entryToJSON(mempool, info, e);
+    entryToJSON(mempool, info, *entry);
     return info;
 },
     };

From f80909e7a31523d8f197fd650b138b9f228cd13f Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 15:40:49 +0100
Subject: [PATCH 07/15] [refactor] remove access to mapTx in
 blockencodings_tests.cpp

---
 src/test/blockencodings_tests.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 4348a20886f..8709e0c6231 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
 
     LOCK2(cs_main, pool.cs);
     pool.addUnchecked(entry.FromTx(block.vtx[2]));
-    BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
+    BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0);
 
     // Do a simple ShortTxIDs RT
     {
@@ -80,7 +80,7 @@ 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.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1);
 
         size_t poolSize = pool.size();
         pool.removeRecursive(*block.vtx[2], MemPoolRemovalReason::REPLACED);
@@ -145,7 +145,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
 
     LOCK2(cs_main, pool.cs);
     pool.addUnchecked(entry.FromTx(block.vtx[2]));
-    BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
+    BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 0);
 
     uint256 txhash;
 
@@ -170,7 +170,7 @@ 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); // +1 because of partialBlock
+        BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 1); // +1 because of partialBlock
 
         CBlock block2;
         {
@@ -185,7 +185,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
             partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that
             partialBlock = tmp;
         }
-        BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2
+        BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 2); // +2 because of partialBlock and block2
         bool mutated;
         BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated));
 
@@ -196,15 +196,15 @@ 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 + 3); // +2 because of partialBlock and block2 and block3
+        BOOST_CHECK_EQUAL(pool.get(block.vtx[2]->GetHash()).use_count(), SHARED_TX_OFFSET + 3); // +2 because of partialBlock and block2 and block3
 
         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 - 1); // + 1 because of partialBlock; -1 because of block.
+        BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block.
     }
-    BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block
+    BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block
 }
 
 BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
@@ -215,7 +215,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
 
     LOCK2(cs_main, pool.cs);
     pool.addUnchecked(entry.FromTx(block.vtx[1]));
-    BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
+    BOOST_CHECK_EQUAL(pool.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 0);
 
     uint256 txhash;
 
@@ -240,7 +240,7 @@ 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.get(block.vtx[1]->GetHash()).use_count(), SHARED_TX_OFFSET + 1);
 
         CBlock block2;
         PartiallyDownloadedBlock partialBlockCopy = partialBlock;
@@ -253,9 +253,9 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
         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 - 1); // + 1 because of partialBlock; -1 because of block.
+        BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET + 1 - 1); // + 1 because of partialBlock; -1 because of block.
     }
-    BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET - 1); // -1 because of block
+    BOOST_CHECK_EQUAL(pool.get(txhash).use_count(), SHARED_TX_OFFSET - 1); // -1 because of block
 }
 
 BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest)

From dbc5bdbf595e9dd0330493645ebff0b8696192a3 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 15:48:51 +0100
Subject: [PATCH 08/15] [refactor] remove access to mapTx.find in
 mempool_tests.cpp

---
 src/test/mempool_tests.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index db58a0baec1..217e4a6d223 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     CheckSort<descendant_score>(pool, sortedOrder);
 
     CTxMemPool::setEntries setAncestors;
-    setAncestors.insert(pool.mapTx.find(tx6.GetHash()));
+    setAncestors.insert(pool.GetIter(tx6.GetHash()).value());
     CMutableTransaction tx7 = CMutableTransaction();
     tx7.vin.resize(1);
     tx7.vin[0].prevout = COutPoint(tx6.GetHash(), 0);
@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     tx8.vout.resize(1);
     tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL;
     tx8.vout[0].nValue = 10 * COIN;
-    setAncestors.insert(pool.mapTx.find(tx7.GetHash()));
+    setAncestors.insert(pool.GetIter(tx7.GetHash()).value());
     pool.addUnchecked(entry.Fee(0LL).Time(NodeSeconds{2s}).FromTx(tx8), setAncestors);
 
     // Now tx8 should be sorted low, but tx6/tx both high
@@ -247,8 +247,8 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
 
     std::vector<std::string> snapshotOrder = sortedOrder;
 
-    setAncestors.insert(pool.mapTx.find(tx8.GetHash()));
-    setAncestors.insert(pool.mapTx.find(tx9.GetHash()));
+    setAncestors.insert(pool.GetIter(tx8.GetHash()).value());
+    setAncestors.insert(pool.GetIter(tx9.GetHash()).value());
     /* tx10 depends on tx8 and tx9 and has a high fee*/
     CMutableTransaction tx10 = CMutableTransaction();
     tx10.vin.resize(2);
@@ -291,11 +291,11 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
     BOOST_CHECK_EQUAL(pool.size(), 10U);
 
     // Now try removing tx10 and verify the sort order returns to normal
-    pool.removeRecursive(pool.mapTx.find(tx10.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
+    pool.removeRecursive(*Assert(pool.get(tx10.GetHash())), REMOVAL_REASON_DUMMY);
     CheckSort<descendant_score>(pool, snapshotOrder);
 
-    pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
-    pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx(), REMOVAL_REASON_DUMMY);
+    pool.removeRecursive(*Assert(pool.get(tx9.GetHash())), REMOVAL_REASON_DUMMY);
+    pool.removeRecursive(*Assert(pool.get(tx8.GetHash())), REMOVAL_REASON_DUMMY);
 }
 
 BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)

From 1bf4855016e777dd8b424fe01750f9e3e97931a2 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 16:08:58 +0100
Subject: [PATCH 09/15] [refactor] use CheckPackageLimits for checkChainLimits

The behavior is the same as CalculateMemPoolAncestors. The only
difference is the string returned, and the string is discarded anyway
since checkChainLimits only cares about pass/fail.
---
 src/node/interfaces.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 153b4728ab9..4c83df7eca3 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -703,9 +703,9 @@ public:
         if (!m_node.mempool) return true;
         LockPoints lp;
         CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp);
-        const CTxMemPool::Limits& limits{m_node.mempool->m_limits};
         LOCK(m_node.mempool->cs);
-        return m_node.mempool->CalculateMemPoolAncestors(entry, limits).has_value();
+        std::string err_string;
+        return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize(), err_string);
     }
     CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override
     {

From 333367a9407701b5077e2457b1a6aa8ff5e4934b Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 16:27:14 +0100
Subject: [PATCH 10/15] [txmempool] make CTxMemPoolEntry::lockPoints mutable

Allows calling UpdateLockPoints() with a (const) txiter. Note that this
was already possible for caller using mapTx.modify(txiter). The point
here is to not be accessing mapTx when doing so.
---
 src/kernel/mempool_entry.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index edd95e8d53a..c1478c0105c 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -83,7 +83,7 @@ private:
     const bool spendsCoinbase;      //!< keep track of transactions that spend a coinbase
     const int64_t sigOpCost;        //!< Total sigop cost
     CAmount m_modified_fee;         //!< Used for determining the priority of the transaction for mining in a block
-    LockPoints lockPoints;          //!< Track the height and time at which tx was final
+    mutable LockPoints lockPoints;  //!< Track the height and time at which tx was final
 
     // Information about descendants of this transaction that are in the
     // mempool; if we remove this transaction we must remove all of these
@@ -151,7 +151,7 @@ public:
     }
 
     // Update the LockPoints after a reorg
-    void UpdateLockPoints(const LockPoints& lp)
+    void UpdateLockPoints(const LockPoints& lp) const
     {
         lockPoints = lp;
     }

From 938643c3b2b8e7b9aec1df34a2f8a95d616d8dd5 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Wed, 30 Aug 2023 16:28:53 +0100
Subject: [PATCH 11/15] [refactor] remove access to mapTx in validation.cpp

---
 src/validation.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/validation.cpp b/src/validation.cpp
index e1838a85892..001359d4f35 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -353,7 +353,7 @@ void Chainstate::MaybeUpdateMempoolForReorg(
             const std::optional<LockPoints> new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)};
             if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) {
                 // Now update the mempool entry lockpoints as well.
-                m_mempool->mapTx.modify(it, [&new_lock_points](CTxMemPoolEntry& e) { e.UpdateLockPoints(*new_lock_points); });
+                it->UpdateLockPoints(*new_lock_points);
             } else {
                 return true;
             }

From a03aef9cec35b0d03aa63d7e8093f0420cd4b40b Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 25 Aug 2023 17:01:51 +0100
Subject: [PATCH 12/15] [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.
---
 src/blockencodings.cpp            | 6 +++---
 src/test/blockencodings_tests.cpp | 4 ++--
 src/txmempool.cpp                 | 6 ++++--
 src/txmempool.h                   | 2 +-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 211b4740be9..9b8f49cbc87 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 8709e0c6231..33894fd0765 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 e39f8975448..14f4d3c032c 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 d4418fa4072..fa1c62217b9 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;
 

From 55b0939cab49d50ca5bc59105b669e379d5e7f6c Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 1 Sep 2023 21:36:54 +0200
Subject: [PATCH 13/15] scripted-diff: rename vTxHashes to txns_randomized

-BEGIN VERIFY SCRIPT-
git grep -l "vTxHashesIdx" src | xargs sed -i "s/vTxHashesIdx/idx_randomized/g"
git grep -l "vTxHashes" src | xargs sed -i "s/vTxHashes/txns_randomized/g"
-END VERIFY SCRIPT-
---
 src/blockencodings.cpp            |  2 +-
 src/kernel/mempool_entry.h        |  2 +-
 src/test/blockencodings_tests.cpp |  2 +-
 src/txmempool.cpp                 | 24 ++++++++++++------------
 src/txmempool.h                   |  2 +-
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 9b8f49cbc87..29306b22290 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -107,7 +107,7 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
     std::vector<bool> have_txn(txn_available.size());
     {
     LOCK(pool->cs);
-    for (const auto& tx : pool->vTxHashes) {
+    for (const auto& tx : pool->txns_randomized) {
         uint64_t shortid = cmpctblock.GetShortID(tx->GetWitnessHash());
         std::unordered_map<uint64_t, uint16_t>::iterator idit = shorttxids.find(shortid);
         if (idit != shorttxids.end()) {
diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h
index c1478c0105c..7c905ca4f4a 100644
--- a/src/kernel/mempool_entry.h
+++ b/src/kernel/mempool_entry.h
@@ -172,7 +172,7 @@ public:
     Parents& GetMemPoolParents() const { return m_parents; }
     Children& GetMemPoolChildren() const { return m_children; }
 
-    mutable size_t vTxHashesIdx; //!< Index in mempool's vTxHashes
+    mutable size_t idx_randomized; //!< Index in mempool's txns_randomized
     mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms
 };
 
diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp
index 33894fd0765..e4ef019daf6 100644
--- a/src/test/blockencodings_tests.cpp
+++ b/src/test/blockencodings_tests.cpp
@@ -51,7 +51,7 @@ static CBlock BuildBlockTestCase() {
 }
 
 // Number of shared use_counts we expect for a tx we haven't touched
-// (block + mempool entry + mempool vTxHashes + our copy from the GetSharedTx call)
+// (block + mempool entry + mempool txns_randomized + 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 14f4d3c032c..efcb77f47cf 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -480,8 +480,8 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
         minerPolicyEstimator->processTransaction(entry, validFeeEstimate);
     }
 
-    vTxHashes.emplace_back(newit->GetSharedTx());
-    newit->vTxHashesIdx = vTxHashes.size() - 1;
+    txns_randomized.emplace_back(newit->GetSharedTx());
+    newit->idx_randomized = txns_randomized.size() - 1;
 
     TRACE3(mempool, added,
         entry.GetTx().GetHash().data(),
@@ -517,16 +517,16 @@ 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.pop_back();
-        if (vTxHashes.size() * 2 < vTxHashes.capacity())
-            vTxHashes.shrink_to_fit();
+    if (txns_randomized.size() > 1) {
+        // Update idx_randomized of the to-be-moved entry.
+        Assert(GetEntry(txns_randomized.back()->GetHash()))->idx_randomized = it->idx_randomized;
+        // Remove entry from txns_randomized by replacing it with the back and deleting the back.
+        txns_randomized[it->idx_randomized] = std::move(txns_randomized.back());
+        txns_randomized.pop_back();
+        if (txns_randomized.size() * 2 < txns_randomized.capacity())
+            txns_randomized.shrink_to_fit();
     } else
-        vTxHashes.clear();
+        txns_randomized.clear();
 
     totalTxSize -= it->GetTxSize();
     m_total_fee -= it->GetFee();
@@ -1054,7 +1054,7 @@ void CCoinsViewMemPool::Reset()
 size_t CTxMemPool::DynamicMemoryUsage() const {
     LOCK(cs);
     // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented.
-    return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage;
+    return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(txns_randomized) + cachedInnerUsage;
 }
 
 void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) {
diff --git a/src/txmempool.h b/src/txmempool.h
index fa1c62217b9..3b0b8cf519a 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<CTransactionRef> vTxHashes GUARDED_BY(cs); //!< All transactions in mapTx, in random order
+    std::vector<CTransactionRef> txns_randomized GUARDED_BY(cs); //!< All transactions in mapTx, in random order
 
     typedef std::set<txiter, CompareIteratorByHash> setEntries;
 

From d0cd2e804ec9b278ed9699c2ae48574b1c1613b1 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 29 Aug 2023 17:36:16 +0100
Subject: [PATCH 14/15] [refactor] rewrite BlockAssembler inBlock and failedTx
 as sets of txids

---
 src/node/miner.cpp | 14 +++++++-------
 src/node/miner.h   |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index caa29918193..ce5452d1f9d 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -188,7 +188,7 @@ void BlockAssembler::onlyUnconfirmed(CTxMemPool::setEntries& testSet)
 {
     for (CTxMemPool::setEntries::iterator iit = testSet.begin(); iit != testSet.end(); ) {
         // Only test txs not already in the block
-        if (inBlock.count(*iit)) {
+        if (inBlock.count((*iit)->GetSharedTx()->GetHash())) {
             testSet.erase(iit++);
         } else {
             iit++;
@@ -229,7 +229,7 @@ void BlockAssembler::AddToBlock(CTxMemPool::txiter iter)
     ++nBlockTx;
     nBlockSigOpsCost += iter->GetSigOpCost();
     nFees += iter->GetFee();
-    inBlock.insert(iter);
+    inBlock.insert(iter->GetSharedTx()->GetHash());
 
     bool fPrintPriority = gArgs.GetBoolArg("-printpriority", DEFAULT_PRINTPRIORITY);
     if (fPrintPriority) {
@@ -298,7 +298,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
     // because some of their txs are already in the block
     indexed_modified_transaction_set mapModifiedTx;
     // Keep track of entries that failed inclusion, to avoid duplicate work
-    CTxMemPool::setEntries failedTx;
+    std::set<Txid> failedTx;
 
     CTxMemPool::indexed_transaction_set::index<ancestor_score>::type::iterator mi = mempool.mapTx.get<ancestor_score>().begin();
     CTxMemPool::txiter iter;
@@ -326,7 +326,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
         if (mi != mempool.mapTx.get<ancestor_score>().end()) {
             auto it = mempool.mapTx.project<0>(mi);
             assert(it != mempool.mapTx.end());
-            if (mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it)) {
+            if (mapModifiedTx.count(it) || inBlock.count(it->GetSharedTx()->GetHash()) || failedTx.count(it->GetSharedTx()->GetHash())) {
                 ++mi;
                 continue;
             }
@@ -360,7 +360,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
 
         // We skip mapTx entries that are inBlock, and mapModifiedTx shouldn't
         // contain anything that is inBlock.
-        assert(!inBlock.count(iter));
+        assert(!inBlock.count(iter->GetSharedTx()->GetHash()));
 
         uint64_t packageSize = iter->GetSizeWithAncestors();
         CAmount packageFees = iter->GetModFeesWithAncestors();
@@ -382,7 +382,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
                 // we must erase failed entries so that we can consider the
                 // next best entry on the next loop iteration
                 mapModifiedTx.get<ancestor_score>().erase(modit);
-                failedTx.insert(iter);
+                failedTx.insert(iter->GetSharedTx()->GetHash());
             }
 
             ++nConsecutiveFailed;
@@ -404,7 +404,7 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
         if (!TestPackageTransactions(ancestors)) {
             if (fUsingModified) {
                 mapModifiedTx.get<ancestor_score>().erase(modit);
-                failedTx.insert(iter);
+                failedTx.insert(iter->GetSharedTx()->GetHash());
             }
             continue;
         }
diff --git a/src/node/miner.h b/src/node/miner.h
index 41735215854..06a917228dc 100644
--- a/src/node/miner.h
+++ b/src/node/miner.h
@@ -142,7 +142,7 @@ private:
     uint64_t nBlockTx;
     uint64_t nBlockSigOpsCost;
     CAmount nFees;
-    CTxMemPool::setEntries inBlock;
+    std::unordered_set<Txid, SaltedTxidHasher> inBlock;
 
     // Chain context for the block
     int nHeight;

From 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Wed, 1 Nov 2023 20:21:43 +0100
Subject: [PATCH 15/15] [refactor] remove access to mapTx in
 validation_block_tests

Use the helper function instead of reaching into the mapTx member
object.
---
 src/test/validation_block_tests.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp
index 64cb5522ebe..35e5c6a0372 100644
--- a/src/test/validation_block_tests.cpp
+++ b/src/test/validation_block_tests.cpp
@@ -283,8 +283,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
 
         // Check that all txs are in the pool
         {
-            LOCK(m_node.mempool->cs);
-            BOOST_CHECK_EQUAL(m_node.mempool->mapTx.size(), txs.size());
+            BOOST_CHECK_EQUAL(m_node.mempool->size(), txs.size());
         }
 
         // Run a thread that simulates an RPC caller that is polling while
@@ -295,7 +294,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
             // not some intermediate amount.
             while (true) {
                 LOCK(m_node.mempool->cs);
-                if (m_node.mempool->mapTx.size() == 0) {
+                if (m_node.mempool->size() == 0) {
                     // We are done with the reorg
                     break;
                 }
@@ -304,7 +303,7 @@ BOOST_AUTO_TEST_CASE(mempool_locks_reorg)
                 // be atomic. So the caller assumes that the returned mempool
                 // is consistent. That is, it has all txs that were there
                 // before the reorg.
-                assert(m_node.mempool->mapTx.size() == txs.size());
+                assert(m_node.mempool->size() == txs.size());
                 continue;
             }
             LOCK(cs_main);