[refactor] TxOrphanage::HaveTx only by wtxid

Author: glozow

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;

src/test/fuzz/txorphan.cpp

@@ -112,21 +112,20 @@ 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
                     {
                         bool add_tx = orphanage.AddTx(tx, peer_id);
                         // 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()));

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)

src/txorphanage.h

@@ -23,8 +23,8 @@ class TxOrphanage {
     /** 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.