From 940a49978c70453e1aaf2c4a0bcb382872b844a5 Mon Sep 17 00:00:00 2001
From: dergoegge <n.goeggi@gmail.com>
Date: Wed, 19 Jul 2023 15:56:37 +0200
Subject: [PATCH] Use type-safe txid types in orphanage

---
 src/net_processing.cpp       |  4 ++--
 src/test/orphanage_tests.cpp |  5 +++--
 src/txorphanage.cpp          | 33 +++++++++++++++++----------------
 src/txorphanage.h            | 10 +++++-----
 4 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 58bf3ec5d1e..6391ecc934c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2918,8 +2918,8 @@ bool PeerManagerImpl::ProcessOrphanTx(Peer& peer)
     while (CTransactionRef porphanTx = m_orphanage.GetTxToReconsider(peer.m_id)) {
         const MempoolAcceptResult result = m_chainman.ProcessTransaction(porphanTx);
         const TxValidationState& state = result.m_state;
-        const uint256& orphanHash = porphanTx->GetHash();
-        const uint256& orphan_wtxid = porphanTx->GetWitnessHash();
+        const Txid& orphanHash = porphanTx->GetHash();
+        const Wtxid& orphan_wtxid = porphanTx->GetWitnessHash();
 
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             LogPrint(BCLog::TXPACKAGES, "   accepted orphan tx %s (wtxid=%s)\n", orphanHash.ToString(), orphan_wtxid.ToString());
diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index af53737fecb..d374497a45c 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <arith_uint256.h>
+#include <primitives/transaction.h>
 #include <pubkey.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
@@ -29,8 +30,8 @@ public:
     CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
         LOCK(m_mutex);
-        std::map<uint256, OrphanTx>::iterator it;
-        it = m_orphans.lower_bound(InsecureRand256());
+        std::map<Txid, OrphanTx>::iterator it;
+        it = m_orphans.lower_bound(Txid::FromUint256(InsecureRand256()));
         if (it == m_orphans.end())
             it = m_orphans.begin();
         return it->second.tx;
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 7455d914e80..16f54644c20 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -7,6 +7,7 @@
 #include <consensus/validation.h>
 #include <logging.h>
 #include <policy/policy.h>
+#include <primitives/transaction.h>
 
 #include <cassert>
 
@@ -20,8 +21,8 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     LOCK(m_mutex);
 
-    const uint256& hash = tx->GetHash();
-    const uint256& wtxid = tx->GetWitnessHash();
+    const Txid& hash = tx->GetHash();
+    const Wtxid& wtxid = tx->GetWitnessHash();
     if (m_orphans.count(hash))
         return false;
 
@@ -53,16 +54,16 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
     return true;
 }
 
-int TxOrphanage::EraseTx(const uint256& txid)
+int TxOrphanage::EraseTx(const Txid& txid)
 {
     LOCK(m_mutex);
     return EraseTxNoLock(txid);
 }
 
-int TxOrphanage::EraseTxNoLock(const uint256& txid)
+int TxOrphanage::EraseTxNoLock(const Txid& txid)
 {
     AssertLockHeld(m_mutex);
-    std::map<uint256, OrphanTx>::iterator it = m_orphans.find(txid);
+    std::map<Txid, OrphanTx>::iterator it = m_orphans.find(txid);
     if (it == m_orphans.end())
         return 0;
     for (const CTxIn& txin : it->second.tx->vin)
@@ -100,10 +101,10 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     m_peer_work_set.erase(peer);
 
     int nErased = 0;
-    std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin();
+    std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
     while (iter != m_orphans.end())
     {
-        std::map<uint256, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
+        std::map<Txid, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
         if (maybeErase->second.fromPeer == peer)
         {
             nErased += EraseTxNoLock(maybeErase->second.tx->GetHash());
@@ -123,10 +124,10 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans)
         // Sweep out expired orphan pool entries:
         int nErased = 0;
         int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
-        std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin();
+        std::map<Txid, OrphanTx>::iterator iter = m_orphans.begin();
         while (iter != m_orphans.end())
         {
-            std::map<uint256, OrphanTx>::iterator maybeErase = iter++;
+            std::map<Txid, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
                 nErased += EraseTxNoLock(maybeErase->second.tx->GetHash());
             } else {
@@ -159,7 +160,7 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
             for (const auto& elem : it_by_prev->second) {
                 // Get this source peer's work set, emplacing an empty set if it didn't exist
                 // (note: if this peer wasn't still connected, we would have removed the orphan tx already)
-                std::set<uint256>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
+                std::set<Txid>& orphan_work_set = m_peer_work_set.try_emplace(elem->second.fromPeer).first->second;
                 // Add this tx to the work set
                 orphan_work_set.insert(elem->first);
                 LogPrint(BCLog::TXPACKAGES, "added %s (wtxid=%s) to peer %d workset\n",
@@ -173,9 +174,9 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const
 {
     LOCK(m_mutex);
     if (gtxid.IsWtxid()) {
-        return m_wtxid_to_orphan_it.count(gtxid.GetHash());
+        return m_wtxid_to_orphan_it.count(Wtxid::FromUint256(gtxid.GetHash()));
     } else {
-        return m_orphans.count(gtxid.GetHash());
+        return m_orphans.count(Txid::FromUint256(gtxid.GetHash()));
     }
 }
 
@@ -187,7 +188,7 @@ CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
     if (work_set_it != m_peer_work_set.end()) {
         auto& work_set = work_set_it->second;
         while (!work_set.empty()) {
-            uint256 txid = *work_set.begin();
+            Txid txid = *work_set.begin();
             work_set.erase(work_set.begin());
 
             const auto orphan_it = m_orphans.find(txid);
@@ -215,7 +216,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
 {
     LOCK(m_mutex);
 
-    std::vector<uint256> vOrphanErase;
+    std::vector<Txid> vOrphanErase;
 
     for (const CTransactionRef& ptx : block.vtx) {
         const CTransaction& tx = *ptx;
@@ -226,7 +227,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
             if (itByPrev == m_outpoint_to_orphan_it.end()) continue;
             for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) {
                 const CTransaction& orphanTx = *(*mi)->second.tx;
-                const uint256& orphanHash = orphanTx.GetHash();
+                const auto& orphanHash = orphanTx.GetHash();
                 vOrphanErase.push_back(orphanHash);
             }
         }
@@ -235,7 +236,7 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
     // Erase orphan transactions included or precluded by this block
     if (vOrphanErase.size()) {
         int nErased = 0;
-        for (const uint256& orphanHash : vOrphanErase) {
+        for (const auto& orphanHash : vOrphanErase) {
             nErased += EraseTxNoLock(orphanHash);
         }
         LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx included or conflicted by block\n", nErased);
diff --git a/src/txorphanage.h b/src/txorphanage.h
index a4705bf382b..2196ed4c85f 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -34,7 +34,7 @@ public:
     CTransactionRef GetTxToReconsider(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Erase an orphan by txid */
-    int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    int EraseTx(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
     void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
@@ -71,10 +71,10 @@ protected:
 
     /** Map from txid to orphan transaction record. Limited by
      *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
-    std::map<uint256, OrphanTx> m_orphans GUARDED_BY(m_mutex);
+    std::map<Txid, OrphanTx> m_orphans GUARDED_BY(m_mutex);
 
     /** Which peer provided the orphans that need to be reconsidered */
-    std::map<NodeId, std::set<uint256>> m_peer_work_set GUARDED_BY(m_mutex);
+    std::map<NodeId, std::set<Txid>> m_peer_work_set GUARDED_BY(m_mutex);
 
     using OrphanMap = decltype(m_orphans);
 
@@ -96,10 +96,10 @@ protected:
 
     /** Index from wtxid into the m_orphans to lookup orphan
      *  transactions using their witness ids. */
-    std::map<uint256, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex);
+    std::map<Wtxid, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(m_mutex);
 
     /** Erase an orphan by txid */
-    int EraseTxNoLock(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
+    int EraseTxNoLock(const Txid& txid) EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
 };
 
 #endif // BITCOIN_TXORPHANAGE_H