From efcc5930175f31b685adb4627a038d9f0848eb1f Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Fri, 10 May 2024 10:55:39 +0100
Subject: [PATCH] [refactor] TxOrphanage::HaveTx only by wtxid

---
 src/net_processing.cpp     |  4 ++--
 src/test/fuzz/txorphan.cpp | 11 +++++------
 src/txorphanage.cpp        |  8 ++------
 src/txorphanage.h          |  4 ++--
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 985b41b160f..0b8b8969872 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2297,7 +2297,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconside
 
     if (gtxid.IsWtxid()) {
         // Normal query by wtxid.
-        if (m_orphanage.HaveTx(gtxid)) return true;
+        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
     } else {
         // Never query by txid: it is possible that the transaction in the orphanage has the same
         // txid but a different witness, which would give us a false positive result. If we decided
@@ -2307,7 +2307,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid, bool include_reconside
         // While we won't query by txid, we can try to "guess" what the wtxid is based on the txid.
         // A non-segwit transaction's txid == wtxid. Query this txid "casted" to a wtxid. This will
         // help us find non-segwit transactions, saving bandwidth, and should have no false positives.
-        if (m_orphanage.HaveTx(GenTxid::Wtxid(hash))) return true;
+        if (m_orphanage.HaveTx(Wtxid::FromUint256(hash))) return true;
     }
 
     if (include_reconsiderable && m_recent_rejects_reconsiderable.contains(hash)) return true;
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
index a44f47b00d3..5e9034e7f74 100644
--- a/src/test/fuzz/txorphan.cpp
+++ b/src/test/fuzz/txorphan.cpp
@@ -112,13 +112,12 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     {
                         CTransactionRef ref = orphanage.GetTxToReconsider(peer_id);
                         if (ref) {
-                            bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetWitnessHash()));
-                            Assert(have_tx);
+                            Assert(orphanage.HaveTx(ref->GetWitnessHash()));
                         }
                     }
                 },
                 [&] {
-                    bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // AddTx should return false if tx is too big or already have it
                     // tx weight is unknown, we only check when tx is already in orphanage
                     {
@@ -126,7 +125,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                         // have_tx == true -> add_tx == false
                         Assert(!have_tx || !add_tx);
                     }
-                    have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     {
                         bool add_tx = orphanage.AddTx(tx, peer_id);
                         // if have_tx is still false, it must be too big
@@ -135,12 +134,12 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     }
                 },
                 [&] {
-                    bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    bool have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // EraseTx should return 0 if m_orphans doesn't have the tx
                     {
                         Assert(have_tx == orphanage.EraseTx(tx->GetHash()));
                     }
-                    have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash()));
+                    have_tx = orphanage.HaveTx(tx->GetWitnessHash());
                     // have_tx should be false and EraseTx should fail
                     {
                         Assert(!have_tx && !orphanage.EraseTx(tx->GetHash()));
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 82206e56c32..e10f01c693f 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -169,14 +169,10 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx)
     }
 }
 
-bool TxOrphanage::HaveTx(const GenTxid& gtxid) const
+bool TxOrphanage::HaveTx(const Wtxid& wtxid) const
 {
     LOCK(m_mutex);
-    if (gtxid.IsWtxid()) {
-        return m_wtxid_to_orphan_it.count(Wtxid::FromUint256(gtxid.GetHash()));
-    } else {
-        return m_orphans.count(Txid::FromUint256(gtxid.GetHash()));
-    }
+    return m_wtxid_to_orphan_it.count(wtxid);
 }
 
 CTransactionRef TxOrphanage::GetTxToReconsider(NodeId peer)
diff --git a/src/txorphanage.h b/src/txorphanage.h
index a3c8edaa2ac..33e41ff5b7e 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -23,8 +23,8 @@ public:
     /** Add a new orphan transaction */
     bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
-    /** Check if we already have an orphan transaction (by txid or wtxid) */
-    bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    /** Check if we already have an orphan transaction (by wtxid only) */
+    bool HaveTx(const Wtxid& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Extract a transaction from a peer's work set
      *  Returns nullptr if there are no transactions to work on.