From 9d5313df7eedad8562c822f5477747e924929fd3 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 00:55:54 +1000
Subject: [PATCH 01/14] move-only: Add txorphanage module

This module captures orphan tracking code for tx relay.

Can be reviewed with --color-moved=dimmed-zebra
---
 src/Makefile.am                    |   6 +-
 src/init.cpp                       |   1 +
 src/net_processing.cpp             | 123 +----------------------------
 src/net_processing.h               |   1 -
 src/test/denialofservice_tests.cpp |  10 +--
 src/test/fuzz/process_message.cpp  |   1 +
 src/test/fuzz/process_messages.cpp |   1 +
 src/txorphanage.cpp                | 107 +++++++++++++++++++++++++
 src/txorphanage.h                  |  54 +++++++++++++
 9 files changed, 170 insertions(+), 134 deletions(-)
 create mode 100644 src/txorphanage.cpp
 create mode 100644 src/txorphanage.h

diff --git a/src/Makefile.am b/src/Makefile.am
index eae226b1d4d..a797075d07d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -224,8 +224,9 @@ BITCOIN_CORE_H = \
   timedata.h \
   torcontrol.h \
   txdb.h \
-  txrequest.h \
   txmempool.h \
+  txorphanage.h \
+  txrequest.h \
   undo.h \
   util/asmap.h \
   util/bip32.h \
@@ -347,8 +348,9 @@ libbitcoin_server_a_SOURCES = \
   timedata.cpp \
   torcontrol.cpp \
   txdb.cpp \
-  txrequest.cpp \
   txmempool.cpp \
+  txorphanage.cpp \
+  txrequest.cpp \
   validation.cpp \
   validationinterface.cpp \
   versionbits.cpp \
diff --git a/src/init.cpp b/src/init.cpp
index befba2eb2d9..99a88ffa29f 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -52,6 +52,7 @@
 #include <torcontrol.h>
 #include <txdb.h>
 #include <txmempool.h>
+#include <txorphanage.h>
 #include <util/asmap.h>
 #include <util/check.h>
 #include <util/moneystr.h>
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index c97f7ced463..d842dca95b6 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -26,6 +26,7 @@
 #include <streams.h>
 #include <tinyformat.h>
 #include <txmempool.h>
+#include <txorphanage.h>
 #include <txrequest.h>
 #include <util/check.h> // For NDEBUG compile time check
 #include <util/strencodings.h>
@@ -35,10 +36,6 @@
 #include <memory>
 #include <typeinfo>
 
-/** Expiration time for orphan transactions in seconds */
-static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
-/** Minimum time between orphan transactions expire time checks in seconds */
-static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
 /** How long to cache transactions in mapRelay for normal relay */
 static constexpr std::chrono::seconds RELAY_TX_CACHE_TIME = std::chrono::minutes{15};
 /** How long a transaction has to be in the mempool before it can unconditionally be relayed (even when not in mapRelay). */
@@ -148,25 +145,6 @@ static constexpr uint32_t MAX_GETCFHEADERS_SIZE = 2000;
 /** the maximum percentage of addresses from our addrman to return in response to a getaddr message. */
 static constexpr size_t MAX_PCT_ADDR_TO_SEND = 23;
 
-struct COrphanTx {
-    // When modifying, adapt the copy of this definition in tests/DoS_tests.
-    CTransactionRef tx;
-    NodeId fromPeer;
-    int64_t nTimeExpire;
-    size_t list_pos;
-};
-
-/** Guards orphan transactions and extra txs for compact blocks */
-RecursiveMutex g_cs_orphans;
-/** Map from txid to orphan transaction record. Limited by
- *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
-std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
-/** Index from wtxid into the mapOrphanTransactions to lookup orphan
- *  transactions using their witness ids. */
-std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
-
-void EraseOrphansFor(NodeId peer);
-
 // Internal stuff
 namespace {
 /** Blocks that are in flight, and that are in the queue to be downloaded. */
@@ -487,21 +465,6 @@ namespace {
     /** Number of preferable block download peers. */
     int nPreferredDownload GUARDED_BY(cs_main) = 0;
 
-    struct IteratorComparator
-    {
-        template<typename I>
-        bool operator()(const I& a, const I& b) const
-        {
-            return &(*a) < &(*b);
-        }
-    };
-
-    /** Index from the parents' COutPoint into the mapOrphanTransactions. Used
-     *  to remove orphan transactions from the mapOrphanTransactions */
-    std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
-    /** Orphan transactions in vector for quick random eviction */
-    std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
-
     /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction.
      *  The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of
      *  these are kept in a ring buffer */
@@ -1169,90 +1132,6 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE
     return true;
 }
 
-int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
-{
-    std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash);
-    if (it == mapOrphanTransactions.end())
-        return 0;
-    for (const CTxIn& txin : it->second.tx->vin)
-    {
-        auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
-        if (itPrev == mapOrphanTransactionsByPrev.end())
-            continue;
-        itPrev->second.erase(it);
-        if (itPrev->second.empty())
-            mapOrphanTransactionsByPrev.erase(itPrev);
-    }
-
-    size_t old_pos = it->second.list_pos;
-    assert(g_orphan_list[old_pos] == it);
-    if (old_pos + 1 != g_orphan_list.size()) {
-        // Unless we're deleting the last entry in g_orphan_list, move the last
-        // entry to the position we're deleting.
-        auto it_last = g_orphan_list.back();
-        g_orphan_list[old_pos] = it_last;
-        it_last->second.list_pos = old_pos;
-    }
-    g_orphan_list.pop_back();
-    g_orphans_by_wtxid.erase(it->second.tx->GetWitnessHash());
-
-    mapOrphanTransactions.erase(it);
-    return 1;
-}
-
-void EraseOrphansFor(NodeId peer)
-{
-    LOCK(g_cs_orphans);
-    int nErased = 0;
-    std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
-    while (iter != mapOrphanTransactions.end())
-    {
-        std::map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
-        if (maybeErase->second.fromPeer == peer)
-        {
-            nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
-        }
-    }
-    if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
-}
-
-
-unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
-{
-    LOCK(g_cs_orphans);
-
-    unsigned int nEvicted = 0;
-    static int64_t nNextSweep;
-    int64_t nNow = GetTime();
-    if (nNextSweep <= nNow) {
-        // Sweep out expired orphan pool entries:
-        int nErased = 0;
-        int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
-        std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
-        while (iter != mapOrphanTransactions.end())
-        {
-            std::map<uint256, COrphanTx>::iterator maybeErase = iter++;
-            if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
-            } else {
-                nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
-            }
-        }
-        // Sweep again 5 minutes after the next entry that expires in order to batch the linear scan.
-        nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
-        if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased);
-    }
-    FastRandomContext rng;
-    while (mapOrphanTransactions.size() > nMaxOrphans)
-    {
-        // Evict a random orphan:
-        size_t randompos = rng.randrange(g_orphan_list.size());
-        EraseOrphanTx(g_orphan_list[randompos]->first);
-        ++nEvicted;
-    }
-    return nEvicted;
-}
-
 void PeerManagerImpl::Misbehaving(const NodeId pnode, const int howmuch, const std::string& message)
 {
     assert(howmuch > 0);
diff --git a/src/net_processing.h b/src/net_processing.h
index d7be453df5a..3b099074432 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -15,7 +15,6 @@ class CTxMemPool;
 class ChainstateManager;
 
 extern RecursiveMutex cs_main;
-extern RecursiveMutex g_cs_orphans;
 
 /** Default for -maxorphantx, maximum number of orphan transactions kept in memory */
 static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100;
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 0d480e35ea1..e2e38b3f03e 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -14,6 +14,7 @@
 #include <script/signingprovider.h>
 #include <script/standard.h>
 #include <serialize.h>
+#include <txorphanage.h>
 #include <util/memory.h>
 #include <util/string.h>
 #include <util/system.h>
@@ -45,15 +46,6 @@ struct CConnmanTest : public CConnman {
 
 // Tests these internal-to-net_processing.cpp methods:
 extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer);
-extern void EraseOrphansFor(NodeId peer);
-extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
-
-struct COrphanTx {
-    CTransactionRef tx;
-    NodeId fromPeer;
-    int64_t nTimeExpire;
-};
-extern std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
 
 static CService ip(uint32_t i)
 {
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index 442e32d4ca9..cea724344e6 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -18,6 +18,7 @@
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
+#include <txorphanage.h>
 #include <util/memory.h>
 #include <validationinterface.h>
 #include <version.h>
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index ef45196671e..12186f4f25d 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -13,6 +13,7 @@
 #include <test/util/net.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
+#include <txorphanage.h>
 #include <util/memory.h>
 #include <validation.h>
 #include <validationinterface.h>
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
new file mode 100644
index 00000000000..fb03914681e
--- /dev/null
+++ b/src/txorphanage.cpp
@@ -0,0 +1,107 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <txorphanage.h>
+
+#include <consensus/validation.h>
+#include <logging.h>
+#include <policy/policy.h>
+
+#include <cassert>
+
+/** Minimum time between orphan transactions expire time checks in seconds */
+static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
+
+RecursiveMutex g_cs_orphans;
+
+std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
+std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
+
+    std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
+
+    std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
+
+int EraseOrphanTx(uint256 hash)
+{
+    std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash);
+    if (it == mapOrphanTransactions.end())
+        return 0;
+    for (const CTxIn& txin : it->second.tx->vin)
+    {
+        auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
+        if (itPrev == mapOrphanTransactionsByPrev.end())
+            continue;
+        itPrev->second.erase(it);
+        if (itPrev->second.empty())
+            mapOrphanTransactionsByPrev.erase(itPrev);
+    }
+
+    size_t old_pos = it->second.list_pos;
+    assert(g_orphan_list[old_pos] == it);
+    if (old_pos + 1 != g_orphan_list.size()) {
+        // Unless we're deleting the last entry in g_orphan_list, move the last
+        // entry to the position we're deleting.
+        auto it_last = g_orphan_list.back();
+        g_orphan_list[old_pos] = it_last;
+        it_last->second.list_pos = old_pos;
+    }
+    g_orphan_list.pop_back();
+    g_orphans_by_wtxid.erase(it->second.tx->GetWitnessHash());
+
+    mapOrphanTransactions.erase(it);
+    return 1;
+}
+
+void EraseOrphansFor(NodeId peer)
+{
+    LOCK(g_cs_orphans);
+    int nErased = 0;
+    std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
+    while (iter != mapOrphanTransactions.end())
+    {
+        std::map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
+        if (maybeErase->second.fromPeer == peer)
+        {
+            nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
+        }
+    }
+    if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
+}
+
+unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
+{
+    LOCK(g_cs_orphans);
+
+    unsigned int nEvicted = 0;
+    static int64_t nNextSweep;
+    int64_t nNow = GetTime();
+    if (nNextSweep <= nNow) {
+        // Sweep out expired orphan pool entries:
+        int nErased = 0;
+        int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
+        std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
+        while (iter != mapOrphanTransactions.end())
+        {
+            std::map<uint256, COrphanTx>::iterator maybeErase = iter++;
+            if (maybeErase->second.nTimeExpire <= nNow) {
+                nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
+            } else {
+                nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
+            }
+        }
+        // Sweep again 5 minutes after the next entry that expires in order to batch the linear scan.
+        nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
+        if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased);
+    }
+    FastRandomContext rng;
+    while (mapOrphanTransactions.size() > nMaxOrphans)
+    {
+        // Evict a random orphan:
+        size_t randompos = rng.randrange(g_orphan_list.size());
+        EraseOrphanTx(g_orphan_list[randompos]->first);
+        ++nEvicted;
+    }
+    return nEvicted;
+}
+
diff --git a/src/txorphanage.h b/src/txorphanage.h
new file mode 100644
index 00000000000..d97b4b1f43b
--- /dev/null
+++ b/src/txorphanage.h
@@ -0,0 +1,54 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_TXORPHANAGE_H
+#define BITCOIN_TXORPHANAGE_H
+
+#include <net.h>
+#include <primitives/transaction.h>
+#include <sync.h>
+
+/** Expiration time for orphan transactions in seconds */
+static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
+
+/** Guards orphan transactions and extra txs for compact blocks */
+extern RecursiveMutex g_cs_orphans;
+
+struct COrphanTx {
+    // When modifying, adapt the copy of this definition in tests/DoS_tests.
+    CTransactionRef tx;
+    NodeId fromPeer;
+    int64_t nTimeExpire;
+    size_t list_pos;
+};
+
+int EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+void EraseOrphansFor(NodeId peer);
+unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
+
+/** Map from txid to orphan transaction record. Limited by
+ *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
+extern std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
+
+/** Index from wtxid into the mapOrphanTransactions to lookup orphan
+ *  transactions using their witness ids. */
+extern std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
+
+    struct IteratorComparator
+    {
+        template<typename I>
+        bool operator()(const I& a, const I& b) const
+        {
+            return &(*a) < &(*b);
+        }
+    };
+
+    /** Index from the parents' COutPoint into the mapOrphanTransactions. Used
+     *  to remove orphan transactions from the mapOrphanTransactions */
+    extern std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
+
+    /** Orphan transactions in vector for quick random eviction */
+    extern std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
+
+#endif // BITCOIN_TXORPHANAGE_H

From 81dd57e5b1ab1afa7e59468e30ef41bd34f0c8d7 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Mon, 15 Feb 2021 23:04:21 +1000
Subject: [PATCH 02/14] txorphanage: Pass uint256 by reference instead of value

---
 src/txorphanage.cpp | 4 ++--
 src/txorphanage.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index fb03914681e..d418ed93231 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -22,9 +22,9 @@ std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUA
 
     std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
 
-int EraseOrphanTx(uint256 hash)
+int EraseOrphanTx(const uint256& txid)
 {
-    std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(hash);
+    std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(txid);
     if (it == mapOrphanTransactions.end())
         return 0;
     for (const CTxIn& txin : it->second.tx->vin)
diff --git a/src/txorphanage.h b/src/txorphanage.h
index d97b4b1f43b..343f12c9c5f 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -23,7 +23,7 @@ struct COrphanTx {
     size_t list_pos;
 };
 
-int EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void EraseOrphansFor(NodeId peer);
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
 

From 38a11c355acfc15134c682571b3d92f66b0e7c3c Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Mon, 1 Feb 2021 01:35:08 +1000
Subject: [PATCH 03/14] txorphanage: Add lock annotations

EraseOrphansFor was called both with and without g_cs_orphans held,
correct that so that it's always called with it already held.

LimitOrphanTxSize was always called with g_cs_orphans held, so
add annotations and don't lock it a second time.
---
 src/net_processing.cpp | 2 +-
 src/txorphanage.cpp    | 5 +++--
 src/txorphanage.h      | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index d842dca95b6..01effa92458 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1003,7 +1003,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim
     for (const QueuedBlock& entry : state->vBlocksInFlight) {
         mapBlocksInFlight.erase(entry.hash);
     }
-    EraseOrphansFor(nodeid);
+    WITH_LOCK(g_cs_orphans, EraseOrphansFor(nodeid));
     m_txrequest.DisconnectedPeer(nodeid);
     nPreferredDownload -= state->fPreferredDownload;
     nPeersWithValidatedDownloads -= (state->nBlocksInFlightValidHeaders != 0);
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index d418ed93231..c883aaa57fd 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -55,7 +55,8 @@ int EraseOrphanTx(const uint256& txid)
 
 void EraseOrphansFor(NodeId peer)
 {
-    LOCK(g_cs_orphans);
+    AssertLockHeld(g_cs_orphans);
+
     int nErased = 0;
     std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
     while (iter != mapOrphanTransactions.end())
@@ -71,7 +72,7 @@ void EraseOrphansFor(NodeId peer)
 
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
 {
-    LOCK(g_cs_orphans);
+    AssertLockHeld(g_cs_orphans);
 
     unsigned int nEvicted = 0;
     static int64_t nNextSweep;
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 343f12c9c5f..d8dfaec8a00 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -24,8 +24,8 @@ struct COrphanTx {
 };
 
 int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-void EraseOrphansFor(NodeId peer);
-unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans);
+void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
 /** Map from txid to orphan transaction record. Limited by
  *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */

From ee135c8d5b39b0cb8b301a83e286285ab926dca7 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 20:36:24 +1000
Subject: [PATCH 04/14] txorphanage: Extract AddChildrenToWorkSet

Extract some common code into AddChildrenToWorkSet function.

(It's a hard knock life)
---
 src/net_processing.cpp | 18 ++----------------
 src/txorphanage.cpp    | 13 +++++++++++++
 src/txorphanage.h      |  1 +
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 01effa92458..181e64c77e7 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2121,14 +2121,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             LogPrint(BCLog::MEMPOOL, "   accepted orphan tx %s\n", orphanHash.ToString());
             RelayTransaction(orphanHash, porphanTx->GetWitnessHash(), m_connman);
-            for (unsigned int i = 0; i < porphanTx->vout.size(); i++) {
-                auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(orphanHash, i));
-                if (it_by_prev != mapOrphanTransactionsByPrev.end()) {
-                    for (const auto& elem : it_by_prev->second) {
-                        orphan_work_set.insert(elem->first);
-                    }
-                }
-            }
+            AddChildrenToWorkSet(*porphanTx, orphan_work_set);
             EraseOrphanTx(orphanHash);
             for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
@@ -3147,14 +3140,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             m_txrequest.ForgetTxHash(tx.GetHash());
             m_txrequest.ForgetTxHash(tx.GetWitnessHash());
             RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman);
-            for (unsigned int i = 0; i < tx.vout.size(); i++) {
-                auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(txid, i));
-                if (it_by_prev != mapOrphanTransactionsByPrev.end()) {
-                    for (const auto& elem : it_by_prev->second) {
-                        peer->m_orphan_work_set.insert(elem->first);
-                    }
-                }
-            }
+            AddChildrenToWorkSet(tx, peer->m_orphan_work_set);
 
             pfrom.nLastTXTime = GetTime();
 
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index c883aaa57fd..c1443115f0c 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -106,3 +106,16 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
     return nEvicted;
 }
 
+void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set)
+{
+    AssertLockHeld(g_cs_orphans);
+    for (unsigned int i = 0; i < tx.vout.size(); i++) {
+        const auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(tx.GetHash(), i));
+        if (it_by_prev != mapOrphanTransactionsByPrev.end()) {
+            for (const auto& elem : it_by_prev->second) {
+                orphan_work_set.insert(elem->first);
+            }
+        }
+    }
+}
+
diff --git a/src/txorphanage.h b/src/txorphanage.h
index d8dfaec8a00..c57249265e7 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -26,6 +26,7 @@ struct COrphanTx {
 int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
 /** Map from txid to orphan transaction record. Limited by
  *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */

From 83679ffc600305ec0926fd195ee31c11de2ed613 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 21:24:08 +1000
Subject: [PATCH 05/14] txorphanage: Extract HaveOrphanTx

Extract some common code into HaveOrphanTx function.
---
 src/net_processing.cpp |  9 +--------
 src/txorphanage.cpp    | 10 ++++++++++
 src/txorphanage.h      |  1 +
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 181e64c77e7..cda177dfe39 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1509,14 +1509,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
 
     const uint256& hash = gtxid.GetHash();
 
-    {
-        LOCK(g_cs_orphans);
-        if (!gtxid.IsWtxid() && mapOrphanTransactions.count(hash)) {
-            return true;
-        } else if (gtxid.IsWtxid() && g_orphans_by_wtxid.count(hash)) {
-            return true;
-        }
-    }
+    if (HaveOrphanTx(gtxid)) return true;
 
     {
         LOCK(m_recent_confirmed_transactions_mutex);
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index c1443115f0c..473abd50445 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -119,3 +119,13 @@ void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work
     }
 }
 
+bool HaveOrphanTx(const GenTxid& gtxid)
+{
+    LOCK(g_cs_orphans);
+    if (gtxid.IsWtxid()) {
+        return g_orphans_by_wtxid.count(gtxid.GetHash());
+    } else {
+        return mapOrphanTransactions.count(gtxid.GetHash());
+    }
+}
+
diff --git a/src/txorphanage.h b/src/txorphanage.h
index c57249265e7..ab4960be693 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -27,6 +27,7 @@ int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+bool HaveOrphanTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 
 /** Map from txid to orphan transaction record. Limited by
  *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */

From f294da727413210fda279afdc206a4dd12046d56 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 21:59:57 +1000
Subject: [PATCH 06/14] txorphanage: Extract GetOrphanTx

Extract orphan lookup code into GetOrphanTx function.
---
 src/net_processing.cpp | 9 ++++-----
 src/txorphanage.cpp    | 8 ++++++++
 src/txorphanage.h      | 1 +
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index cda177dfe39..4ac50dc1f0b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2104,10 +2104,9 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         const uint256 orphanHash = *orphan_work_set.begin();
         orphan_work_set.erase(orphan_work_set.begin());
 
-        auto orphan_it = mapOrphanTransactions.find(orphanHash);
-        if (orphan_it == mapOrphanTransactions.end()) continue;
+        const auto [porphanTx, from_peer] = GetOrphanTx(orphanHash);
+        if (porphanTx == nullptr) continue;
 
-        const CTransactionRef porphanTx = orphan_it->second.tx;
         const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), m_mempool, porphanTx, false /* bypass_limits */);
         const TxValidationState& state = result.m_state;
 
@@ -2124,10 +2123,10 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
             if (state.IsInvalid()) {
                 LogPrint(BCLog::MEMPOOL, "   invalid orphan tx %s from peer=%d. %s\n",
                     orphanHash.ToString(),
-                    orphan_it->second.fromPeer,
+                    from_peer,
                     state.ToString());
                 // Maybe punish peer that gave us an invalid orphan tx
-                MaybePunishNodeForTx(orphan_it->second.fromPeer, state);
+                MaybePunishNodeForTx(from_peer, state);
             }
             // Has inputs but not accepted to mempool
             // Probably non-standard or insufficient fee
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 473abd50445..6baea9f69ec 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -129,3 +129,11 @@ bool HaveOrphanTx(const GenTxid& gtxid)
     }
 }
 
+std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid)
+{
+    AssertLockHeld(g_cs_orphans);
+
+    const auto it = mapOrphanTransactions.find(txid);
+    if (it == mapOrphanTransactions.end()) return {nullptr, -1};
+    return {it->second.tx, it->second.fromPeer};
+}
diff --git a/src/txorphanage.h b/src/txorphanage.h
index ab4960be693..1adc6f1d6c3 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -28,6 +28,7 @@ void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 bool HaveOrphanTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
+std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
 /** Map from txid to orphan transaction record. Limited by
  *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */

From 1041616d7eb66281bb4de51ffbc83df0923b2f7e Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 23:12:24 +1000
Subject: [PATCH 07/14] txorphanage: Extract OrphanageAddTx

Extract code from AddOrphanTx into OrphanageAddTx.
---
 src/net_processing.cpp | 27 +--------------------------
 src/txorphanage.cpp    | 38 ++++++++++++++++++++++++++++++++++++++
 src/txorphanage.h      |  4 +---
 3 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 4ac50dc1f0b..89586beba5d 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1098,37 +1098,12 @@ static void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_L
 
 bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
 {
-    const uint256& hash = tx->GetHash();
-    if (mapOrphanTransactions.count(hash))
+    if (!OrphanageAddTx(tx, peer)) {
         return false;
-
-    // Ignore big transactions, to avoid a
-    // send-big-orphans memory exhaustion attack. If a peer has a legitimate
-    // large transaction with a missing parent then we assume
-    // it will rebroadcast it later, after the parent transaction(s)
-    // have been mined or received.
-    // 100 orphans, each of which is at most 100,000 bytes big is
-    // at most 10 megabytes of orphans and somewhat more byprev index (in the worst case):
-    unsigned int sz = GetTransactionWeight(*tx);
-    if (sz > MAX_STANDARD_TX_WEIGHT)
-    {
-        LogPrint(BCLog::MEMPOOL, "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString());
-        return false;
-    }
-
-    auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size()});
-    assert(ret.second);
-    g_orphan_list.push_back(ret.first);
-    // Allow for lookups in the orphan pool by wtxid, as well as txid
-    g_orphans_by_wtxid.emplace(tx->GetWitnessHash(), ret.first);
-    for (const CTxIn& txin : tx->vin) {
-        mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first);
     }
 
     AddToCompactExtraTransactions(tx);
 
-    LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u outsz %u)\n", hash.ToString(),
-             mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size());
     return true;
 }
 
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 6baea9f69ec..1527bdf5eac 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -10,6 +10,8 @@
 
 #include <cassert>
 
+/** Expiration time for orphan transactions in seconds */
+static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
 /** Minimum time between orphan transactions expire time checks in seconds */
 static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
 
@@ -22,6 +24,42 @@ std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUA
 
     std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
 
+bool OrphanageAddTx(const CTransactionRef& tx, NodeId peer)
+{
+    AssertLockHeld(g_cs_orphans);
+
+    const uint256& hash = tx->GetHash();
+    if (mapOrphanTransactions.count(hash))
+        return false;
+
+    // Ignore big transactions, to avoid a
+    // send-big-orphans memory exhaustion attack. If a peer has a legitimate
+    // large transaction with a missing parent then we assume
+    // it will rebroadcast it later, after the parent transaction(s)
+    // have been mined or received.
+    // 100 orphans, each of which is at most 100,000 bytes big is
+    // at most 10 megabytes of orphans and somewhat more byprev index (in the worst case):
+    unsigned int sz = GetTransactionWeight(*tx);
+    if (sz > MAX_STANDARD_TX_WEIGHT)
+    {
+        LogPrint(BCLog::MEMPOOL, "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString());
+        return false;
+    }
+
+    auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size()});
+    assert(ret.second);
+    g_orphan_list.push_back(ret.first);
+    // Allow for lookups in the orphan pool by wtxid, as well as txid
+    g_orphans_by_wtxid.emplace(tx->GetWitnessHash(), ret.first);
+    for (const CTxIn& txin : tx->vin) {
+        mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first);
+    }
+
+    LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u outsz %u)\n", hash.ToString(),
+             mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size());
+    return true;
+}
+
 int EraseOrphanTx(const uint256& txid)
 {
     std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(txid);
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 1adc6f1d6c3..1a72af175c1 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -9,9 +9,6 @@
 #include <primitives/transaction.h>
 #include <sync.h>
 
-/** Expiration time for orphan transactions in seconds */
-static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60;
-
 /** Guards orphan transactions and extra txs for compact blocks */
 extern RecursiveMutex g_cs_orphans;
 
@@ -29,6 +26,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRE
 void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 bool HaveOrphanTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+bool OrphanageAddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
 /** Map from txid to orphan transaction record. Limited by
  *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */

From 26d1a6ccd5fcc7abec737c0d8c67238561627d59 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Mon, 1 Feb 2021 02:00:14 +1000
Subject: [PATCH 08/14] denialofservices_tests: check txorphanage's AddTx

Rather than checking net_processing's internal implementation of
AddOrphanTx, test txorphanage's exported AddTx interface. Note that
this means AddToCompactExtraTransactions is no longer tested here.
---
 src/test/denialofservice_tests.cpp | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index e2e38b3f03e..83b2016cd4d 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -44,9 +44,6 @@ struct CConnmanTest : public CConnman {
     }
 };
 
-// Tests these internal-to-net_processing.cpp methods:
-extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer);
-
 static CService ip(uint32_t i)
 {
     struct in_addr s;
@@ -320,6 +317,8 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     FillableSigningProvider keystore;
     BOOST_CHECK(keystore.AddKey(key));
 
+    LOCK(g_cs_orphans);
+
     // 50 orphan transactions:
     for (int i = 0; i < 50; i++)
     {
@@ -332,7 +331,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vout[0].nValue = 1*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
-        AddOrphanTx(MakeTransactionRef(tx), i);
+        OrphanageAddTx(MakeTransactionRef(tx), i);
     }
 
     // ... and 50 that depend on other orphans:
@@ -349,7 +348,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
 
-        AddOrphanTx(MakeTransactionRef(tx), i);
+        OrphanageAddTx(MakeTransactionRef(tx), i);
     }
 
     // This really-big orphan should be ignored:
@@ -373,10 +372,9 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         for (unsigned int j = 1; j < tx.vin.size(); j++)
             tx.vin[j].scriptSig = tx.vin[0].scriptSig;
 
-        BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i));
+        BOOST_CHECK(!OrphanageAddTx(MakeTransactionRef(tx), i));
     }
 
-    LOCK2(cs_main, g_cs_orphans);
     // Test EraseOrphansFor:
     for (NodeId i = 0; i < 3; i++)
     {

From 3c4c3c2fdda3a361e3802e97bc3566f815b75de1 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sat, 27 Feb 2021 00:30:08 +1000
Subject: [PATCH 09/14] net_processing: drop AddOrphanTx

All the interesting functionality of AddOrphanTx is already in other
functions, so call those functions directly in the one place that
AddOrphanTx was used.
---
 src/net_processing.cpp | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 89586beba5d..67a0c0e3d20 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1096,17 +1096,6 @@ static void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_L
     vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % max_extra_txn;
 }
 
-bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
-{
-    if (!OrphanageAddTx(tx, peer)) {
-        return false;
-    }
-
-    AddToCompactExtraTransactions(tx);
-
-    return true;
-}
-
 void PeerManagerImpl::Misbehaving(const NodeId pnode, const int howmuch, const std::string& message)
 {
     assert(howmuch > 0);
@@ -3156,7 +3145,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     pfrom.AddKnownTx(parent_txid);
                     if (!AlreadyHaveTx(gtxid)) AddTxAnnouncement(pfrom, gtxid, current_time);
                 }
-                AddOrphanTx(ptx, pfrom.GetId());
+
+                if (OrphanageAddTx(ptx, pfrom.GetId())) {
+                    AddToCompactExtraTransactions(ptx);
+                }
 
                 // Once added to the orphan pool, a tx is considered AlreadyHave, and we shouldn't request it anymore.
                 m_txrequest.ForgetTxHash(tx.GetHash());

From 03257b832debcb1470420d8657d30ba30f4be770 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 23:37:41 +1000
Subject: [PATCH 10/14] txorphanage: Extract EraseOrphansForBlock

Extract code that erases orphans when a new block is found into
EraseOrphansForBlock.
---
 src/net_processing.cpp | 32 ++------------------------------
 src/txorphanage.cpp    | 31 +++++++++++++++++++++++++++++++
 src/txorphanage.h      |  2 ++
 3 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 67a0c0e3d20..f1b5c6f38d8 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1261,37 +1261,9 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn
  */
 void PeerManagerImpl::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
 {
-    {
-        LOCK(g_cs_orphans);
+    EraseOrphansForBlock(*pblock);
+    m_last_tip_update = GetTime();
 
-        std::vector<uint256> vOrphanErase;
-
-        for (const CTransactionRef& ptx : pblock->vtx) {
-            const CTransaction& tx = *ptx;
-
-            // Which orphan pool entries must we evict?
-            for (const auto& txin : tx.vin) {
-                auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
-                if (itByPrev == mapOrphanTransactionsByPrev.end()) continue;
-                for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) {
-                    const CTransaction& orphanTx = *(*mi)->second.tx;
-                    const uint256& orphanHash = orphanTx.GetHash();
-                    vOrphanErase.push_back(orphanHash);
-                }
-            }
-        }
-
-        // Erase orphan transactions included or precluded by this block
-        if (vOrphanErase.size()) {
-            int nErased = 0;
-            for (const uint256& orphanHash : vOrphanErase) {
-                nErased += EraseOrphanTx(orphanHash);
-            }
-            LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased);
-        }
-
-        m_last_tip_update = GetTime();
-    }
     {
         LOCK(m_recent_confirmed_transactions_mutex);
         for (const auto& ptx : pblock->vtx) {
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 1527bdf5eac..45bc0885fa3 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -175,3 +175,34 @@ std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid)
     if (it == mapOrphanTransactions.end()) return {nullptr, -1};
     return {it->second.tx, it->second.fromPeer};
 }
+
+void EraseOrphansForBlock(const CBlock& block)
+{
+    LOCK(g_cs_orphans);
+
+    std::vector<uint256> vOrphanErase;
+
+    for (const CTransactionRef& ptx : block.vtx) {
+        const CTransaction& tx = *ptx;
+
+        // Which orphan pool entries must we evict?
+        for (const auto& txin : tx.vin) {
+            auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
+            if (itByPrev == mapOrphanTransactionsByPrev.end()) continue;
+            for (auto mi = itByPrev->second.begin(); mi != itByPrev->second.end(); ++mi) {
+                const CTransaction& orphanTx = *(*mi)->second.tx;
+                const uint256& orphanHash = orphanTx.GetHash();
+                vOrphanErase.push_back(orphanHash);
+            }
+        }
+    }
+
+    // Erase orphan transactions included or precluded by this block
+    if (vOrphanErase.size()) {
+        int nErased = 0;
+        for (const uint256& orphanHash : vOrphanErase) {
+            nErased += EraseOrphanTx(orphanHash);
+        }
+        LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased);
+    }
+}
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 1a72af175c1..f2ffbf3d671 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -6,6 +6,7 @@
 #define BITCOIN_TXORPHANAGE_H
 
 #include <net.h>
+#include <primitives/block.h>
 #include <primitives/transaction.h>
 #include <sync.h>
 
@@ -22,6 +23,7 @@ struct COrphanTx {
 
 int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+void EraseOrphansForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 bool HaveOrphanTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);

From 6bd4963c069bfd0af420e8a3fb724c3b693a1e76 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 23:42:00 +1000
Subject: [PATCH 11/14] txorphanage: Move functions and data into class

Collects all the orphan handling globals into a single member var in
net_processing, and ensures access is encapuslated into the interface
functions. Also adds doxygen comments for methods.
---
 src/net_processing.cpp             | 46 +++++++-------------
 src/test/denialofservice_tests.cpp | 53 +++++++++++++----------
 src/txorphanage.cpp                | 32 ++++++--------
 src/txorphanage.h                  | 69 ++++++++++++++++++++----------
 test/functional/p2p_invalid_tx.py  |  2 +-
 5 files changed, 106 insertions(+), 96 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index f1b5c6f38d8..98d9709eb2b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -457,11 +457,12 @@ private:
     /** Number of peers from which we're downloading blocks. */
     int nPeersWithValidatedDownloads GUARDED_BY(cs_main) = 0;
 
+    /** Storage for orphan information */
+    TxOrphanage m_orphanage;
 };
 } // namespace
 
 namespace {
-
     /** Number of preferable block download peers. */
     int nPreferredDownload GUARDED_BY(cs_main) = 0;
 
@@ -1003,7 +1004,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node, bool& fUpdateConnectionTim
     for (const QueuedBlock& entry : state->vBlocksInFlight) {
         mapBlocksInFlight.erase(entry.hash);
     }
-    WITH_LOCK(g_cs_orphans, EraseOrphansFor(nodeid));
+    WITH_LOCK(g_cs_orphans, m_orphanage.EraseForPeer(nodeid));
     m_txrequest.DisconnectedPeer(nodeid);
     nPreferredDownload -= state->fPreferredDownload;
     nPeersWithValidatedDownloads -= (state->nBlocksInFlightValidHeaders != 0);
@@ -1080,11 +1081,6 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats)
     return true;
 }
 
-//////////////////////////////////////////////////////////////////////////////
-//
-// mapOrphanTransactions
-//
-
 static void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
 {
     size_t max_extra_txn = gArgs.GetArg("-blockreconstructionextratxn", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN);
@@ -1255,13 +1251,13 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn
 }
 
 /**
- * Evict orphan txn pool entries (EraseOrphanTx) based on a newly connected
+ * Evict orphan txn pool entries based on a newly connected
  * block, remember the recently confirmed transactions, and delete tracked
  * announcements for them. Also save the time of the last tip update.
  */
 void PeerManagerImpl::BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
 {
-    EraseOrphansForBlock(*pblock);
+    m_orphanage.EraseForBlock(*pblock);
     m_last_tip_update = GetTime();
 
     {
@@ -1445,7 +1441,7 @@ bool PeerManagerImpl::AlreadyHaveTx(const GenTxid& gtxid)
 
     const uint256& hash = gtxid.GetHash();
 
-    if (HaveOrphanTx(gtxid)) return true;
+    if (m_orphanage.HaveTx(gtxid)) return true;
 
     {
         LOCK(m_recent_confirmed_transactions_mutex);
@@ -2040,7 +2036,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         const uint256 orphanHash = *orphan_work_set.begin();
         orphan_work_set.erase(orphan_work_set.begin());
 
-        const auto [porphanTx, from_peer] = GetOrphanTx(orphanHash);
+        const auto [porphanTx, from_peer] = m_orphanage.GetTx(orphanHash);
         if (porphanTx == nullptr) continue;
 
         const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), m_mempool, porphanTx, false /* bypass_limits */);
@@ -2049,8 +2045,8 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             LogPrint(BCLog::MEMPOOL, "   accepted orphan tx %s\n", orphanHash.ToString());
             RelayTransaction(orphanHash, porphanTx->GetWitnessHash(), m_connman);
-            AddChildrenToWorkSet(*porphanTx, orphan_work_set);
-            EraseOrphanTx(orphanHash);
+            m_orphanage.AddChildrenToWorkSet(*porphanTx, orphan_work_set);
+            m_orphanage.EraseTx(orphanHash);
             for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
             }
@@ -2097,7 +2093,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
                     recentRejects->insert(porphanTx->GetHash());
                 }
             }
-            EraseOrphanTx(orphanHash);
+            m_orphanage.EraseTx(orphanHash);
             break;
         }
     }
@@ -3068,7 +3064,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             m_txrequest.ForgetTxHash(tx.GetHash());
             m_txrequest.ForgetTxHash(tx.GetWitnessHash());
             RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), m_connman);
-            AddChildrenToWorkSet(tx, peer->m_orphan_work_set);
+            m_orphanage.AddChildrenToWorkSet(tx, peer->m_orphan_work_set);
 
             pfrom.nLastTXTime = GetTime();
 
@@ -3118,7 +3114,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                     if (!AlreadyHaveTx(gtxid)) AddTxAnnouncement(pfrom, gtxid, current_time);
                 }
 
-                if (OrphanageAddTx(ptx, pfrom.GetId())) {
+                if (m_orphanage.AddTx(ptx, pfrom.GetId())) {
                     AddToCompactExtraTransactions(ptx);
                 }
 
@@ -3126,11 +3122,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 m_txrequest.ForgetTxHash(tx.GetHash());
                 m_txrequest.ForgetTxHash(tx.GetWitnessHash());
 
-                // DoS prevention: do not allow mapOrphanTransactions to grow unbounded (see CVE-2012-3789)
+                // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
                 unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, gArgs.GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS));
-                unsigned int nEvicted = LimitOrphanTxSize(nMaxOrphanTx);
+                unsigned int nEvicted = m_orphanage.LimitOrphans(nMaxOrphanTx);
                 if (nEvicted > 0) {
-                    LogPrint(BCLog::MEMPOOL, "mapOrphan overflow, removed %u tx\n", nEvicted);
+                    LogPrint(BCLog::MEMPOOL, "orphanage overflow, removed %u tx\n", nEvicted);
                 }
             } else {
                 LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString());
@@ -4744,15 +4740,3 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
     return true;
 }
 
-class CNetProcessingCleanup
-{
-public:
-    CNetProcessingCleanup() {}
-    ~CNetProcessingCleanup() {
-        // orphan transactions
-        mapOrphanTransactions.clear();
-        mapOrphanTransactionsByPrev.clear();
-        g_orphans_by_wtxid.clear();
-    }
-};
-static CNetProcessingCleanup instance_of_cnetprocessingcleanup;
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 83b2016cd4d..35f16aa69da 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -284,15 +284,23 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     peerLogic->FinalizeNode(dummyNode, dummy);
 }
 
-static CTransactionRef RandomOrphan()
+class TxOrphanageTest : public TxOrphanage
 {
-    std::map<uint256, COrphanTx>::iterator it;
-    LOCK2(cs_main, g_cs_orphans);
-    it = mapOrphanTransactions.lower_bound(InsecureRand256());
-    if (it == mapOrphanTransactions.end())
-        it = mapOrphanTransactions.begin();
-    return it->second.tx;
-}
+public:
+    inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
+    {
+        return mapOrphanTransactions.size();
+    }
+
+    CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
+    {
+        std::map<uint256, COrphanTx>::iterator it;
+        it = mapOrphanTransactions.lower_bound(InsecureRand256());
+        if (it == mapOrphanTransactions.end())
+            it = mapOrphanTransactions.begin();
+        return it->second.tx;
+    }
+};
 
 static void MakeNewKeyWithFastRandomContext(CKey& key)
 {
@@ -312,6 +320,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     // signature's R and S values have leading zeros.
     g_insecure_rand_ctx = FastRandomContext(ArithToUint256(arith_uint256(33)));
 
+    TxOrphanageTest orphanage;
     CKey key;
     MakeNewKeyWithFastRandomContext(key);
     FillableSigningProvider keystore;
@@ -331,13 +340,13 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vout[0].nValue = 1*CENT;
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
 
-        OrphanageAddTx(MakeTransactionRef(tx), i);
+        orphanage.AddTx(MakeTransactionRef(tx), i);
     }
 
     // ... and 50 that depend on other orphans:
     for (int i = 0; i < 50; i++)
     {
-        CTransactionRef txPrev = RandomOrphan();
+        CTransactionRef txPrev = orphanage.RandomOrphan();
 
         CMutableTransaction tx;
         tx.vin.resize(1);
@@ -348,13 +357,13 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey()));
         BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
 
-        OrphanageAddTx(MakeTransactionRef(tx), i);
+        orphanage.AddTx(MakeTransactionRef(tx), i);
     }
 
     // This really-big orphan should be ignored:
     for (int i = 0; i < 10; i++)
     {
-        CTransactionRef txPrev = RandomOrphan();
+        CTransactionRef txPrev = orphanage.RandomOrphan();
 
         CMutableTransaction tx;
         tx.vout.resize(1);
@@ -372,24 +381,24 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         for (unsigned int j = 1; j < tx.vin.size(); j++)
             tx.vin[j].scriptSig = tx.vin[0].scriptSig;
 
-        BOOST_CHECK(!OrphanageAddTx(MakeTransactionRef(tx), i));
+        BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), i));
     }
 
     // Test EraseOrphansFor:
     for (NodeId i = 0; i < 3; i++)
     {
-        size_t sizeBefore = mapOrphanTransactions.size();
-        EraseOrphansFor(i);
-        BOOST_CHECK(mapOrphanTransactions.size() < sizeBefore);
+        size_t sizeBefore = orphanage.CountOrphans();
+        orphanage.EraseForPeer(i);
+        BOOST_CHECK(orphanage.CountOrphans() < sizeBefore);
     }
 
     // Test LimitOrphanTxSize() function:
-    LimitOrphanTxSize(40);
-    BOOST_CHECK(mapOrphanTransactions.size() <= 40);
-    LimitOrphanTxSize(10);
-    BOOST_CHECK(mapOrphanTransactions.size() <= 10);
-    LimitOrphanTxSize(0);
-    BOOST_CHECK(mapOrphanTransactions.empty());
+    orphanage.LimitOrphans(40);
+    BOOST_CHECK(orphanage.CountOrphans() <= 40);
+    orphanage.LimitOrphans(10);
+    BOOST_CHECK(orphanage.CountOrphans() <= 10);
+    orphanage.LimitOrphans(0);
+    BOOST_CHECK(orphanage.CountOrphans() == 0);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index 45bc0885fa3..f758107b828 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -17,14 +17,7 @@ static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60;
 
 RecursiveMutex g_cs_orphans;
 
-std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
-std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
-
-    std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
-
-    std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
-
-bool OrphanageAddTx(const CTransactionRef& tx, NodeId peer)
+bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     AssertLockHeld(g_cs_orphans);
 
@@ -60,8 +53,9 @@ bool OrphanageAddTx(const CTransactionRef& tx, NodeId peer)
     return true;
 }
 
-int EraseOrphanTx(const uint256& txid)
+int TxOrphanage::EraseTx(const uint256& txid)
 {
+    AssertLockHeld(g_cs_orphans);
     std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(txid);
     if (it == mapOrphanTransactions.end())
         return 0;
@@ -91,7 +85,7 @@ int EraseOrphanTx(const uint256& txid)
     return 1;
 }
 
-void EraseOrphansFor(NodeId peer)
+void TxOrphanage::EraseForPeer(NodeId peer)
 {
     AssertLockHeld(g_cs_orphans);
 
@@ -102,13 +96,13 @@ void EraseOrphansFor(NodeId peer)
         std::map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
         if (maybeErase->second.fromPeer == peer)
         {
-            nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
+            nErased += EraseTx(maybeErase->second.tx->GetHash());
         }
     }
     if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
 }
 
-unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
+unsigned int TxOrphanage::LimitOrphans(unsigned int nMaxOrphans)
 {
     AssertLockHeld(g_cs_orphans);
 
@@ -124,7 +118,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
         {
             std::map<uint256, COrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
-                nErased += EraseOrphanTx(maybeErase->second.tx->GetHash());
+                nErased += EraseTx(maybeErase->second.tx->GetHash());
             } else {
                 nMinExpTime = std::min(maybeErase->second.nTimeExpire, nMinExpTime);
             }
@@ -138,13 +132,13 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(g_orphan_list.size());
-        EraseOrphanTx(g_orphan_list[randompos]->first);
+        EraseTx(g_orphan_list[randompos]->first);
         ++nEvicted;
     }
     return nEvicted;
 }
 
-void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set)
+void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const
 {
     AssertLockHeld(g_cs_orphans);
     for (unsigned int i = 0; i < tx.vout.size(); i++) {
@@ -157,7 +151,7 @@ void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work
     }
 }
 
-bool HaveOrphanTx(const GenTxid& gtxid)
+bool TxOrphanage::HaveTx(const GenTxid& gtxid) const
 {
     LOCK(g_cs_orphans);
     if (gtxid.IsWtxid()) {
@@ -167,7 +161,7 @@ bool HaveOrphanTx(const GenTxid& gtxid)
     }
 }
 
-std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid)
+std::pair<CTransactionRef, NodeId> TxOrphanage::GetTx(const uint256& txid) const
 {
     AssertLockHeld(g_cs_orphans);
 
@@ -176,7 +170,7 @@ std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid)
     return {it->second.tx, it->second.fromPeer};
 }
 
-void EraseOrphansForBlock(const CBlock& block)
+void TxOrphanage::EraseForBlock(const CBlock& block)
 {
     LOCK(g_cs_orphans);
 
@@ -201,7 +195,7 @@ void EraseOrphansForBlock(const CBlock& block)
     if (vOrphanErase.size()) {
         int nErased = 0;
         for (const uint256& orphanHash : vOrphanErase) {
-            nErased += EraseOrphanTx(orphanHash);
+            nErased += EraseTx(orphanHash);
         }
         LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx included or conflicted by block\n", nErased);
     }
diff --git a/src/txorphanage.h b/src/txorphanage.h
index f2ffbf3d671..6b9837815b8 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -13,30 +13,48 @@
 /** Guards orphan transactions and extra txs for compact blocks */
 extern RecursiveMutex g_cs_orphans;
 
-struct COrphanTx {
-    // When modifying, adapt the copy of this definition in tests/DoS_tests.
-    CTransactionRef tx;
-    NodeId fromPeer;
-    int64_t nTimeExpire;
-    size_t list_pos;
-};
+/** Data structure to keep track of orphan transactions
+ */
+class TxOrphanage {
+public:
+    /** Add a new orphan transaction */
+    bool AddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
-int EraseOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-void EraseOrphansForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
-unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-bool HaveOrphanTx(const GenTxid& gtxid) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
-std::pair<CTransactionRef, NodeId> GetOrphanTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
-bool OrphanageAddTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+    /** Check if we already have an orphan transaction (by txid or wtxid) */
+    bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 
-/** Map from txid to orphan transaction record. Limited by
- *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
-extern std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
+    /** Get the details of an orphan transaction (returns nullptr if not found) */
+    std::pair<CTransactionRef, NodeId> GetTx(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
-/** Index from wtxid into the mapOrphanTransactions to lookup orphan
- *  transactions using their witness ids. */
-extern std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
+    /** Erase an orphan by txid */
+    int EraseTx(const uint256& txid) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+
+    /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
+    void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+
+    /** Erase all orphans included in / invalidated by a new block */
+    void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
+
+    /** Limit the orphanage to the given maximum */
+    unsigned int LimitOrphans(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+
+    /** Add any orphans that list a particular tx as a parent into a peer's work set
+     * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */
+    void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+
+protected:
+    struct COrphanTx {
+        CTransactionRef tx;
+        NodeId fromPeer;
+        int64_t nTimeExpire;
+        size_t list_pos;
+    };
+
+    /** Map from txid to orphan transaction record. Limited by
+     *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
+    std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
+
+    using OrphanMap = decltype(mapOrphanTransactions);
 
     struct IteratorComparator
     {
@@ -49,9 +67,14 @@ extern std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wt
 
     /** Index from the parents' COutPoint into the mapOrphanTransactions. Used
      *  to remove orphan transactions from the mapOrphanTransactions */
-    extern std::map<COutPoint, std::set<std::map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
+    std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
 
     /** Orphan transactions in vector for quick random eviction */
-    extern std::vector<std::map<uint256, COrphanTx>::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
+    std::vector<OrphanMap::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
+
+    /** Index from wtxid into the mapOrphanTransactions to lookup orphan
+     *  transactions using their witness ids. */
+    std::map<uint256, OrphanMap::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
+};
 
 #endif // BITCOIN_TXORPHANAGE_H
diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py
index cca7390ae38..8783c244c3f 100755
--- a/test/functional/p2p_invalid_tx.py
+++ b/test/functional/p2p_invalid_tx.py
@@ -154,7 +154,7 @@ class InvalidTxRequestTest(BitcoinTestFramework):
             orphan_tx_pool[i].vin.append(CTxIn(outpoint=COutPoint(i, 333)))
             orphan_tx_pool[i].vout.append(CTxOut(nValue=11 * COIN, scriptPubKey=SCRIPT_PUB_KEY_OP_TRUE))
 
-        with node.assert_debug_log(['mapOrphan overflow, removed 1 tx']):
+        with node.assert_debug_log(['orphanage overflow, removed 1 tx']):
             node.p2ps[0].send_txs_and_test(orphan_tx_pool, node, success=False)
 
         rejected_parent = CTransaction()

From f8c0688b9490c8d4902530ba3c3b6fbd8b48e0de Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Mon, 15 Feb 2021 22:53:06 +1000
Subject: [PATCH 12/14] scripted-diff: Update txorphanage naming convention

-BEGIN VERIFY SCRIPT-
sed -i 's/mapOrphanTransactionsByPrev/m_outpoint_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/mapOrphanTransactions/m_orphans/g' src/txorphanage.h src/txorphanage.cpp src/net_processing.cpp src/test/denialofservice_tests.cpp
sed -i 's/g_orphan_list/m_orphan_list/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/g_orphans_by_wtxid/m_wtxid_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/nMaxOrphans/max_orphans/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/COrphanTx/OrphanTx/g' src/txorphanage.h src/txorphanage.cpp src/test/denialofservice_tests.cpp
-END VERIFY SCRIPT-
---
 src/test/denialofservice_tests.cpp | 10 ++--
 src/txorphanage.cpp                | 74 +++++++++++++++---------------
 src/txorphanage.h                  | 20 ++++----
 3 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 35f16aa69da..5906913b58b 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -289,15 +289,15 @@ class TxOrphanageTest : public TxOrphanage
 public:
     inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
     {
-        return mapOrphanTransactions.size();
+        return m_orphans.size();
     }
 
     CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
     {
-        std::map<uint256, COrphanTx>::iterator it;
-        it = mapOrphanTransactions.lower_bound(InsecureRand256());
-        if (it == mapOrphanTransactions.end())
-            it = mapOrphanTransactions.begin();
+        std::map<uint256, OrphanTx>::iterator it;
+        it = m_orphans.lower_bound(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 f758107b828..ed4783f1a5c 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -22,7 +22,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
     AssertLockHeld(g_cs_orphans);
 
     const uint256& hash = tx->GetHash();
-    if (mapOrphanTransactions.count(hash))
+    if (m_orphans.count(hash))
         return false;
 
     // Ignore big transactions, to avoid a
@@ -39,49 +39,49 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
         return false;
     }
 
-    auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size()});
+    auto ret = m_orphans.emplace(hash, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()});
     assert(ret.second);
-    g_orphan_list.push_back(ret.first);
+    m_orphan_list.push_back(ret.first);
     // Allow for lookups in the orphan pool by wtxid, as well as txid
-    g_orphans_by_wtxid.emplace(tx->GetWitnessHash(), ret.first);
+    m_wtxid_to_orphan_it.emplace(tx->GetWitnessHash(), ret.first);
     for (const CTxIn& txin : tx->vin) {
-        mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first);
+        m_outpoint_to_orphan_it[txin.prevout].insert(ret.first);
     }
 
     LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u outsz %u)\n", hash.ToString(),
-             mapOrphanTransactions.size(), mapOrphanTransactionsByPrev.size());
+             m_orphans.size(), m_outpoint_to_orphan_it.size());
     return true;
 }
 
 int TxOrphanage::EraseTx(const uint256& txid)
 {
     AssertLockHeld(g_cs_orphans);
-    std::map<uint256, COrphanTx>::iterator it = mapOrphanTransactions.find(txid);
-    if (it == mapOrphanTransactions.end())
+    std::map<uint256, OrphanTx>::iterator it = m_orphans.find(txid);
+    if (it == m_orphans.end())
         return 0;
     for (const CTxIn& txin : it->second.tx->vin)
     {
-        auto itPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
-        if (itPrev == mapOrphanTransactionsByPrev.end())
+        auto itPrev = m_outpoint_to_orphan_it.find(txin.prevout);
+        if (itPrev == m_outpoint_to_orphan_it.end())
             continue;
         itPrev->second.erase(it);
         if (itPrev->second.empty())
-            mapOrphanTransactionsByPrev.erase(itPrev);
+            m_outpoint_to_orphan_it.erase(itPrev);
     }
 
     size_t old_pos = it->second.list_pos;
-    assert(g_orphan_list[old_pos] == it);
-    if (old_pos + 1 != g_orphan_list.size()) {
-        // Unless we're deleting the last entry in g_orphan_list, move the last
+    assert(m_orphan_list[old_pos] == it);
+    if (old_pos + 1 != m_orphan_list.size()) {
+        // Unless we're deleting the last entry in m_orphan_list, move the last
         // entry to the position we're deleting.
-        auto it_last = g_orphan_list.back();
-        g_orphan_list[old_pos] = it_last;
+        auto it_last = m_orphan_list.back();
+        m_orphan_list[old_pos] = it_last;
         it_last->second.list_pos = old_pos;
     }
-    g_orphan_list.pop_back();
-    g_orphans_by_wtxid.erase(it->second.tx->GetWitnessHash());
+    m_orphan_list.pop_back();
+    m_wtxid_to_orphan_it.erase(it->second.tx->GetWitnessHash());
 
-    mapOrphanTransactions.erase(it);
+    m_orphans.erase(it);
     return 1;
 }
 
@@ -90,10 +90,10 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     AssertLockHeld(g_cs_orphans);
 
     int nErased = 0;
-    std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
-    while (iter != mapOrphanTransactions.end())
+    std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin();
+    while (iter != m_orphans.end())
     {
-        std::map<uint256, COrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
+        std::map<uint256, OrphanTx>::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid
         if (maybeErase->second.fromPeer == peer)
         {
             nErased += EraseTx(maybeErase->second.tx->GetHash());
@@ -102,7 +102,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
 }
 
-unsigned int TxOrphanage::LimitOrphans(unsigned int nMaxOrphans)
+unsigned int TxOrphanage::LimitOrphans(unsigned int max_orphans)
 {
     AssertLockHeld(g_cs_orphans);
 
@@ -113,10 +113,10 @@ unsigned int TxOrphanage::LimitOrphans(unsigned int nMaxOrphans)
         // Sweep out expired orphan pool entries:
         int nErased = 0;
         int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL;
-        std::map<uint256, COrphanTx>::iterator iter = mapOrphanTransactions.begin();
-        while (iter != mapOrphanTransactions.end())
+        std::map<uint256, OrphanTx>::iterator iter = m_orphans.begin();
+        while (iter != m_orphans.end())
         {
-            std::map<uint256, COrphanTx>::iterator maybeErase = iter++;
+            std::map<uint256, OrphanTx>::iterator maybeErase = iter++;
             if (maybeErase->second.nTimeExpire <= nNow) {
                 nErased += EraseTx(maybeErase->second.tx->GetHash());
             } else {
@@ -128,11 +128,11 @@ unsigned int TxOrphanage::LimitOrphans(unsigned int nMaxOrphans)
         if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased);
     }
     FastRandomContext rng;
-    while (mapOrphanTransactions.size() > nMaxOrphans)
+    while (m_orphans.size() > max_orphans)
     {
         // Evict a random orphan:
-        size_t randompos = rng.randrange(g_orphan_list.size());
-        EraseTx(g_orphan_list[randompos]->first);
+        size_t randompos = rng.randrange(m_orphan_list.size());
+        EraseTx(m_orphan_list[randompos]->first);
         ++nEvicted;
     }
     return nEvicted;
@@ -142,8 +142,8 @@ void TxOrphanage::AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>
 {
     AssertLockHeld(g_cs_orphans);
     for (unsigned int i = 0; i < tx.vout.size(); i++) {
-        const auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(tx.GetHash(), i));
-        if (it_by_prev != mapOrphanTransactionsByPrev.end()) {
+        const auto it_by_prev = m_outpoint_to_orphan_it.find(COutPoint(tx.GetHash(), i));
+        if (it_by_prev != m_outpoint_to_orphan_it.end()) {
             for (const auto& elem : it_by_prev->second) {
                 orphan_work_set.insert(elem->first);
             }
@@ -155,9 +155,9 @@ bool TxOrphanage::HaveTx(const GenTxid& gtxid) const
 {
     LOCK(g_cs_orphans);
     if (gtxid.IsWtxid()) {
-        return g_orphans_by_wtxid.count(gtxid.GetHash());
+        return m_wtxid_to_orphan_it.count(gtxid.GetHash());
     } else {
-        return mapOrphanTransactions.count(gtxid.GetHash());
+        return m_orphans.count(gtxid.GetHash());
     }
 }
 
@@ -165,8 +165,8 @@ std::pair<CTransactionRef, NodeId> TxOrphanage::GetTx(const uint256& txid) const
 {
     AssertLockHeld(g_cs_orphans);
 
-    const auto it = mapOrphanTransactions.find(txid);
-    if (it == mapOrphanTransactions.end()) return {nullptr, -1};
+    const auto it = m_orphans.find(txid);
+    if (it == m_orphans.end()) return {nullptr, -1};
     return {it->second.tx, it->second.fromPeer};
 }
 
@@ -181,8 +181,8 @@ void TxOrphanage::EraseForBlock(const CBlock& block)
 
         // Which orphan pool entries must we evict?
         for (const auto& txin : tx.vin) {
-            auto itByPrev = mapOrphanTransactionsByPrev.find(txin.prevout);
-            if (itByPrev == mapOrphanTransactionsByPrev.end()) continue;
+            auto itByPrev = m_outpoint_to_orphan_it.find(txin.prevout);
+            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();
diff --git a/src/txorphanage.h b/src/txorphanage.h
index 6b9837815b8..a5e6764d75c 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -36,14 +36,14 @@ public:
     void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 
     /** Limit the orphanage to the given maximum */
-    unsigned int LimitOrphans(unsigned int nMaxOrphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+    unsigned int LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
     /** Add any orphans that list a particular tx as a parent into a peer's work set
      * (ie orphans that may have found their final missing parent, and so should be reconsidered for the mempool) */
     void AddChildrenToWorkSet(const CTransaction& tx, std::set<uint256>& orphan_work_set) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
 protected:
-    struct COrphanTx {
+    struct OrphanTx {
         CTransactionRef tx;
         NodeId fromPeer;
         int64_t nTimeExpire;
@@ -52,9 +52,9 @@ protected:
 
     /** Map from txid to orphan transaction record. Limited by
      *  -maxorphantx/DEFAULT_MAX_ORPHAN_TRANSACTIONS */
-    std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
+    std::map<uint256, OrphanTx> m_orphans GUARDED_BY(g_cs_orphans);
 
-    using OrphanMap = decltype(mapOrphanTransactions);
+    using OrphanMap = decltype(m_orphans);
 
     struct IteratorComparator
     {
@@ -65,16 +65,16 @@ protected:
         }
     };
 
-    /** Index from the parents' COutPoint into the mapOrphanTransactions. Used
-     *  to remove orphan transactions from the mapOrphanTransactions */
-    std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(g_cs_orphans);
+    /** Index from the parents' COutPoint into the m_orphans. Used
+     *  to remove orphan transactions from the m_orphans */
+    std::map<COutPoint, std::set<OrphanMap::iterator, IteratorComparator>> m_outpoint_to_orphan_it GUARDED_BY(g_cs_orphans);
 
     /** Orphan transactions in vector for quick random eviction */
-    std::vector<OrphanMap::iterator> g_orphan_list GUARDED_BY(g_cs_orphans);
+    std::vector<OrphanMap::iterator> m_orphan_list GUARDED_BY(g_cs_orphans);
 
-    /** Index from wtxid into the mapOrphanTransactions to lookup orphan
+    /** Index from wtxid into the m_orphans to lookup orphan
      *  transactions using their witness ids. */
-    std::map<uint256, OrphanMap::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans);
+    std::map<uint256, OrphanMap::iterator> m_wtxid_to_orphan_it GUARDED_BY(g_cs_orphans);
 };
 
 #endif // BITCOIN_TXORPHANAGE_H

From eeeafb324ef6057f40b5c5fdd8464110e809b0f7 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Sun, 31 Jan 2021 23:50:53 +1000
Subject: [PATCH 13/14] net_processing: move AddToCompactExtraTransactions into
 PeerManagerImpl

Allows making vExtraTxnForCompact and vExtraTxnForCompactIt member vars
instead of globals.
---
 src/net_processing.cpp | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 98d9709eb2b..af61e7064e5 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -459,19 +459,21 @@ private:
 
     /** Storage for orphan information */
     TxOrphanage m_orphanage;
+
+    void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
+
+    /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction.
+     *  The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of
+     *  these are kept in a ring buffer */
+    std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_cs_orphans);
+    /** Offset into vExtraTxnForCompact to insert the next tx */
+    size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0;
 };
 } // namespace
 
 namespace {
     /** Number of preferable block download peers. */
     int nPreferredDownload GUARDED_BY(cs_main) = 0;
-
-    /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction.
-     *  The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of
-     *  these are kept in a ring buffer */
-    static std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact GUARDED_BY(g_cs_orphans);
-    /** Offset into vExtraTxnForCompact to insert the next tx */
-    static size_t vExtraTxnForCompactIt GUARDED_BY(g_cs_orphans) = 0;
 } // namespace
 
 namespace {
@@ -1081,7 +1083,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats)
     return true;
 }
 
-static void AddToCompactExtraTransactions(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans)
+void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx)
 {
     size_t max_extra_txn = gArgs.GetArg("-blockreconstructionextratxn", DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN);
     if (max_extra_txn <= 0)

From 5e50e2d1b95e7ca7709a9671ab21f1164b8d0cb8 Mon Sep 17 00:00:00 2001
From: Anthony Towns <aj@erisian.com.au>
Date: Tue, 2 Mar 2021 19:36:48 +1000
Subject: [PATCH 14/14] txorphanage: comment improvements

---
 src/txorphanage.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/txorphanage.h b/src/txorphanage.h
index a5e6764d75c..df55cdb3bef 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -13,7 +13,10 @@
 /** Guards orphan transactions and extra txs for compact blocks */
 extern RecursiveMutex g_cs_orphans;
 
-/** Data structure to keep track of orphan transactions
+/** A class to track orphan transactions (failed on TX_MISSING_INPUTS)
+ * Since we cannot distinguish orphans from bad transactions with
+ * non-existent inputs, we heavily limit the number of orphans
+ * we keep and the duration we keep them for.
  */
 class TxOrphanage {
 public:
@@ -23,7 +26,9 @@ public:
     /** Check if we already have an orphan transaction (by txid or wtxid) */
     bool HaveTx(const GenTxid& gtxid) const EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 
-    /** Get the details of an orphan transaction (returns nullptr if not found) */
+    /** Get an orphan transaction and its orginating peer
+     * (Transaction ref will be nullptr if not found)
+     */
     std::pair<CTransactionRef, NodeId> GetTx(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
     /** Erase an orphan by txid */
@@ -32,7 +37,7 @@ public:
     /** Erase all orphans announced by a peer (eg, after that peer disconnects) */
     void EraseForPeer(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans);
 
-    /** Erase all orphans included in / invalidated by a new block */
+    /** Erase all orphans included in or invalidated by a new block */
     void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!g_cs_orphans);
 
     /** Limit the orphanage to the given maximum */