refactor: Convert mini_miner from uint256 to Txid

src/node/mini_miner.cpp

@@ -16,6 +16,7 @@
 
 #include <algorithm>
 #include <numeric>
+#include <ranges>
 #include <utility>
 
 namespace node {
@@ -61,12 +62,8 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
     if (m_requested_outpoints_by_txid.empty()) return;
 
     // Calculate the cluster and construct the entry map.
-    std::vector<uint256> txids_needed;
-    txids_needed.reserve(m_requested_outpoints_by_txid.size());
-    for (const auto& [txid, _]: m_requested_outpoints_by_txid) {
-        txids_needed.push_back(txid);
-    }
-    const auto cluster = mempool.GatherClusters(txids_needed);
+    auto txids_needed{m_requested_outpoints_by_txid | std::views::keys};
+    const auto cluster = mempool.GatherClusters({txids_needed.begin(), txids_needed.end()});
     if (cluster.empty()) {
         // An empty cluster means that at least one of the transactions is missing from the mempool
         // (should not be possible given processing above) or DoS limit was hit.
@@ -286,7 +283,7 @@ void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
         }
         // Track the order in which transactions were selected.
         for (const auto& ancestor : ancestors) {
-            m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
+            m_inclusion_order.emplace(ancestor->first, sequence_num);
         }
         DeleteAncestorPackage(ancestors);
         SanityCheck();
@@ -409,7 +406,7 @@ std::optional<CAmount> MiniMiner::CalculateTotalBumpFees(const CFeeRate& target_
         ancestors.insert(iter);
     }
 
-    std::set<uint256> has_been_processed;
+    std::set<Txid> has_been_processed;
     while (!to_process.empty()) {
         auto iter = to_process.begin();
         const CTransaction& tx = (*iter)->second.GetTx();

src/node/mini_miner.h

@@ -83,12 +83,12 @@ class MiniMiner
 
     // Set once per lifetime, fill in during initialization.
     // txids of to-be-replaced transactions
-    std::set<uint256> m_to_be_replaced;
+    std::set<Txid> m_to_be_replaced;
 
     // If multiple argument outpoints correspond to the same transaction, cache them together in
     // a single entry indexed by txid. Then we can just work with txids since all outpoints from
     // the same tx will have the same bumpfee. Excludes non-mempool transactions.
-    std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
+    std::map<Txid, std::vector<COutPoint>> m_requested_outpoints_by_txid;
 
     // Txid to a number representing the order in which this transaction was included (smaller
     // number = included earlier).  Transactions included in an ancestor set together have the same
@@ -98,21 +98,21 @@ class MiniMiner
     std::map<COutPoint, CAmount> m_bump_fees;
 
     // The constructed block template
-    std::set<uint256> m_in_block;
+    std::set<Txid> m_in_block;
 
     // Information on the current status of the block
     CAmount m_total_fees{0};
     int32_t m_total_vsize{0};
 
     /** Main data structure holding the entries, can be indexed by txid */
-    std::map<uint256, MiniMinerMempoolEntry> m_entries_by_txid;
+    std::map<Txid, MiniMinerMempoolEntry> m_entries_by_txid;
     using MockEntryMap = decltype(m_entries_by_txid);
 
     /** Vector of entries, can be sorted by ancestor feerate. */
     std::vector<MockEntryMap::iterator> m_entries;
 
     /** Map of txid to its descendants. Should be inclusive. */
-    std::map<uint256, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
+    std::map<Txid, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
 
     /** Consider this ancestor package "mined" so remove all these entries from our data structures. */
     void DeleteAncestorPackage(const std::set<MockEntryMap::iterator, IteratorComparator>& ancestors);
@@ -129,7 +129,7 @@ public:
     void BuildMockTemplate(std::optional<CFeeRate> target_feerate);
 
     /** Returns set of txids in the block template if one has been constructed. */
-    std::set<uint256> GetMockTemplateTxids() const { return m_in_block; }
+    std::set<Txid> GetMockTemplateTxids() const { return m_in_block; }
 
     /** Constructor that takes a list of outpoints that may or may not belong to transactions in the
      * mempool. Copies out information about the relevant transactions in the mempool into

src/test/miniminer_tests.cpp

@@ -103,7 +103,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_negative, TestChain100Setup)
     mini_miner_no_target.BuildMockTemplate(std::nullopt);
     const auto template_txids{mini_miner_no_target.GetMockTemplateTxids()};
     BOOST_CHECK_EQUAL(template_txids.size(), 1);
-    BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash().ToUint256()) > 0);
+    BOOST_CHECK(template_txids.count(tx_mod_negative->GetHash()) > 0);
 }
 
 BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
@@ -177,7 +177,7 @@ BOOST_FIXTURE_TEST_CASE(miniminer_1p1c, TestChain100Setup)
     struct TxDimensions {
         int32_t vsize; CAmount mod_fee; CFeeRate feerate;
     };
-    std::map<uint256, TxDimensions> tx_dims;
+    std::map<Txid, TxDimensions> tx_dims;
     for (const auto& tx : all_transactions) {
         const auto& entry{*Assert(pool.GetEntry(tx->GetHash()))};
         tx_dims.emplace(tx->GetHash(), TxDimensions{entry.GetTxSize(), entry.GetModifiedFee(),
@@ -590,14 +590,6 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
     CTxMemPool& pool = *Assert(m_node.mempool);
     LOCK2(cs_main, pool.cs);
 
-    // TODO this can be removed once the mempool interface uses Txid, Wtxid
-    auto convert_to_uint256_vec = [](const std::vector<Txid>& vec) -> std::vector<uint256> {
-        std::vector<uint256> out;
-        std::transform(vec.begin(), vec.end(), std::back_inserter(out),
-                       [](const Txid& txid) { return txid.ToUint256(); });
-        return out;
-    };
-
     // Add chain of size 500
     TestMemPoolEntryHelper entry;
     std::vector<Txid> chain_txids;
@@ -611,7 +603,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
     const auto cluster_500tx = pool.GatherClusters({lasttx->GetHash()});
     CTxMemPool::setEntries cluster_500tx_set{cluster_500tx.begin(), cluster_500tx.end()};
     BOOST_CHECK_EQUAL(cluster_500tx.size(), cluster_500tx_set.size());
-    const auto vec_iters_500 = pool.GetIterVec(convert_to_uint256_vec(chain_txids));
+    const auto vec_iters_500 = pool.GetIterVec(chain_txids);
     for (const auto& iter : vec_iters_500) BOOST_CHECK(cluster_500tx_set.count(iter));
 
     // GatherClusters stops at 500 transactions.
@@ -637,7 +629,7 @@ BOOST_FIXTURE_TEST_CASE(calculate_cluster, TestChain100Setup)
         AddToMempool(pool, entry.Fee(CENT).FromTx(txc));
         zigzag_txids.push_back(txc->GetHash());
     }
-    const auto vec_iters_zigzag = pool.GetIterVec(convert_to_uint256_vec(zigzag_txids));
+    const auto vec_iters_zigzag = pool.GetIterVec(zigzag_txids);
     // It doesn't matter which tx we calculate cluster for, everybody is in it.
     const std::vector<size_t> indices{0, 22, 72, zigzag_txids.size() - 1};
     for (const auto index : indices) {

src/txmempool.cpp

@@ -983,13 +983,13 @@ CTxMemPool::setEntries CTxMemPool::GetIterSet(const std::set<Txid>& hashes) cons
     return ret;
 }
 
-std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<uint256>& txids) const
+std::vector<CTxMemPool::txiter> CTxMemPool::GetIterVec(const std::vector<Txid>& txids) const
 {
     AssertLockHeld(cs);
     std::vector<txiter> ret;
     ret.reserve(txids.size());
     for (const auto& txid : txids) {
-        const auto it{GetIter(Txid::FromUint256(txid))};
+        const auto it{GetIter(txid)};
         if (!it) return {};
         ret.push_back(*it);
     }
@@ -1227,7 +1227,7 @@ void CTxMemPool::SetLoadTried(bool load_tried)
     m_load_tried = load_tried;
 }
 
-std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<uint256>& txids) const
+std::vector<CTxMemPool::txiter> CTxMemPool::GatherClusters(const std::vector<Txid>& txids) const
 {
     AssertLockHeld(cs);
     std::vector<txiter> clustered_txs{GetIterVec(txids)};

src/txmempool.h

@@ -491,7 +491,7 @@ public:
     /** Translate a list of hashes into a list of mempool iterators to avoid repeated lookups.
      * The nth element in txids becomes the nth element in the returned vector. If any of the txids
      * don't actually exist in the mempool, returns an empty vector. */
-    std::vector<txiter> GetIterVec(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::vector<txiter> GetIterVec(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** UpdateTransactionsFromBlock is called when adding transactions from a
      * disconnected block back to the mempool, new mempool entries may have
@@ -548,7 +548,7 @@ public:
      * All txids must correspond to transaction entries in the mempool, otherwise this returns an
      * empty vector. This call will also exit early and return an empty vector if it collects 500 or
      * more transactions as a DoS protection. */
-    std::vector<txiter> GatherClusters(const std::vector<uint256>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
+    std::vector<txiter> GatherClusters(const std::vector<Txid>& txids) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     /** Calculate all in-mempool ancestors of a set of transactions not already in the mempool and
      * check ancestor and descendant limits. Heuristics are used to estimate the ancestor and