[prep/test] have TxOrphanage remember its own limits in LimitOrphans

Move towards a model where TxOrphanage is initialized with limits that
it remembers throughout its lifetime.
Remove the param. Limiting by number of unique orphans will be removed
in a later commit.
Now that -maxorphantx is gone, this does not change the node behavior.
The parameter is only used in tests.

Author: glozow

src/net_processing.cpp

@@ -1925,7 +1925,7 @@ PeerManagerImpl::PeerManagerImpl(CConnman& connman, AddrMan& addrman,
       m_banman(banman),
       m_chainman(chainman),
       m_mempool(pool),
-      m_txdownloadman(node::TxDownloadOptions{pool, m_rng, opts.max_orphan_txs, opts.deterministic_rng}),
+      m_txdownloadman(node::TxDownloadOptions{pool, m_rng, opts.deterministic_rng}),
       m_warnings{warnings},
       m_opts{opts}
 {

src/net_processing.h

@@ -76,8 +76,6 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
         bool ignore_incoming_txs{DEFAULT_BLOCKSONLY};
         //! Whether transaction reconciliation protocol is enabled
         bool reconcile_txs{DEFAULT_TXRECONCILIATION_ENABLE};
-        //! Maximum number of orphan transactions kept in memory
-        uint32_t max_orphan_txs{node::DEFAULT_MAX_ORPHAN_TRANSACTIONS};
         //! Number of non-mempool transactions to keep around for block reconstruction. Includes
         //! orphan, replaced, and rejected transactions.
         uint32_t max_extra_txs{DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN};

src/node/txdownloadman.h

@@ -41,8 +41,6 @@ struct TxDownloadOptions {
     const CTxMemPool& m_mempool;
     /** RNG provided by caller. */
     FastRandomContext& m_rng;
-    /** Maximum number of transactions allowed in orphanage. */
-    const uint32_t m_max_orphan_txs;
     /** Instantiate TxRequestTracker as deterministic (used for tests). */
     bool m_deterministic_txrequest{false};
 };

src/node/txdownloadman_impl.cpp

@@ -422,7 +422,7 @@ node::RejectedTxTodo TxDownloadManagerImpl::MempoolRejectedTx(const CTransaction
                 // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
                 // Note that, if the orphanage reaches capacity, it's possible that we immediately evict
                 // the transaction we just added.
-                m_orphanage.LimitOrphans(m_opts.m_max_orphan_txs, m_opts.m_rng);
+                m_orphanage.LimitOrphans(m_opts.m_rng);
             } else {
                 unique_parents.clear();
                 LogDebug(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n",

src/node/txorphanage.cpp

@@ -141,7 +141,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     if (nErased > 0) LogDebug(BCLog::TXPACKAGES, "Erased %d orphan transaction(s) from peer=%d\n", nErased, peer);
 }
 
-void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
+void TxOrphanage::LimitOrphans(FastRandomContext& rng)
 {
     unsigned int nEvicted = 0;
     auto nNow{Now<NodeSeconds>()};
@@ -163,7 +163,7 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
         m_next_sweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
         if (nErased > 0) LogDebug(BCLog::TXPACKAGES, "Erased %d orphan tx due to expiration\n", nErased);
     }
-    while (m_orphans.size() > max_orphans)
+    while (m_orphans.size() > DEFAULT_MAX_ORPHAN_TRANSACTIONS)
     {
         // Evict a random orphan:
         size_t randompos = rng.randrange(m_orphan_list.size());

src/node/txorphanage.h

@@ -62,8 +62,8 @@ class TxOrphanage {
     /** Erase all orphans included in or invalidated by a new block */
     void EraseForBlock(const CBlock& block);
 
-    /** Limit the orphanage to the given maximum */
-    void LimitOrphans(unsigned int max_orphans, FastRandomContext& rng);
+    /** Limit the orphanage to DEFAULT_MAX_ORPHAN_TRANSACTIONS. */
+    void LimitOrphans(FastRandomContext& rng);
 
     /** Add any orphans that list a particular tx as a parent into the from peer's work set */
     void AddChildrenToWorkSet(const CTransaction& tx, FastRandomContext& rng);

src/test/fuzz/txdownloadman.cpp

@@ -173,9 +173,8 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
     // Initialize txdownloadman
     bilingual_str error;
     CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error};
-    const auto max_orphan_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 300);
     FastRandomContext det_rand{true};
-    node::TxDownloadManager txdownloadman{node::TxDownloadOptions{pool, det_rand, max_orphan_count, true}};
+    node::TxDownloadManager txdownloadman{node::TxDownloadOptions{pool, det_rand, true}};
 
     std::chrono::microseconds time{244466666};
 
@@ -304,9 +303,9 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
     // Initialize a TxDownloadManagerImpl
     bilingual_str error;
     CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error};
-    const auto max_orphan_count = fuzzed_data_provider.ConsumeIntegralInRange<unsigned int>(0, 300);
+    const auto max_orphan_count = node::DEFAULT_MAX_ORPHAN_TRANSACTIONS;
     FastRandomContext det_rand{true};
-    node::TxDownloadManagerImpl txdownload_impl{node::TxDownloadOptions{pool, det_rand, max_orphan_count, true}};
+    node::TxDownloadManagerImpl txdownload_impl{node::TxDownloadOptions{pool, det_rand, true}};
 
     std::chrono::microseconds time{244466666};
 

src/test/fuzz/txorphan.cpp

@@ -217,9 +217,8 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                 [&] {
                     // test mocktime and expiry
                     SetMockTime(ConsumeTime(fuzzed_data_provider));
-                    auto limit = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
-                    orphanage.LimitOrphans(limit, orphanage_rng);
-                    Assert(orphanage.Size() <= limit);
+                    orphanage.LimitOrphans(orphanage_rng);
+                    Assert(orphanage.Size() <= node::DEFAULT_MAX_ORPHAN_TRANSACTIONS);
                 });
 
         }

src/test/orphanage_tests.cpp

@@ -181,34 +181,23 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
 
     // Test LimitOrphanTxSize() function, nothing should timeout:
     FastRandomContext rng{/*fDeterministic=*/true};
-    orphanage.LimitOrphans(/*max_orphans=*/expected_num_orphans, rng);
+    orphanage.LimitOrphans(rng);
     BOOST_CHECK_EQUAL(orphanage.Size(), expected_num_orphans);
-    expected_num_orphans -= 1;
-    orphanage.LimitOrphans(/*max_orphans=*/expected_num_orphans, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), expected_num_orphans);
-    assert(expected_num_orphans > 40);
-    orphanage.LimitOrphans(40, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 40);
-    orphanage.LimitOrphans(10, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 10);
-    orphanage.LimitOrphans(0, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 0);
 
     // Add one more orphan, check timeout logic
     auto timeout_tx = MakeTransactionSpending(/*outpoints=*/{}, rng);
     orphanage.AddTx(timeout_tx, 0);
-    orphanage.LimitOrphans(1, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 1);
+    expected_num_orphans += 1;
+    BOOST_CHECK_EQUAL(orphanage.Size(), expected_num_orphans);
 
     // One second shy of expiration
     SetMockTime(now + node::ORPHAN_TX_EXPIRE_TIME - 1s);
-    orphanage.LimitOrphans(1, rng);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 1);
+    orphanage.LimitOrphans(rng);
+    BOOST_CHECK_EQUAL(orphanage.Size(), expected_num_orphans);
 
     // Jump one more second, orphan should be timed out on limiting
     SetMockTime(now + node::ORPHAN_TX_EXPIRE_TIME);
-    BOOST_CHECK_EQUAL(orphanage.Size(), 1);
-    orphanage.LimitOrphans(1, rng);
+    orphanage.LimitOrphans(rng);
     BOOST_CHECK_EQUAL(orphanage.Size(), 0);
 }
 

src/test/txdownload_tests.cpp

@@ -114,7 +114,7 @@ BOOST_FIXTURE_TEST_CASE(tx_rejection_types, TestChain100Setup)
 {
     CTxMemPool& pool = *Assert(m_node.mempool);
     FastRandomContext det_rand{true};
-    node::TxDownloadOptions DEFAULT_OPTS{pool, det_rand, node::DEFAULT_MAX_ORPHAN_TRANSACTIONS, true};
+    node::TxDownloadOptions DEFAULT_OPTS{pool, det_rand, true};
 
     // A new TxDownloadManagerImpl is created for each tx so we can just reuse the same one.
     TxValidationState state;
@@ -172,7 +172,7 @@ BOOST_FIXTURE_TEST_CASE(handle_missing_inputs, TestChain100Setup)
 {
     CTxMemPool& pool = *Assert(m_node.mempool);
     FastRandomContext det_rand{true};
-    node::TxDownloadOptions DEFAULT_OPTS{pool, det_rand, node::DEFAULT_MAX_ORPHAN_TRANSACTIONS, true};
+    node::TxDownloadOptions DEFAULT_OPTS{pool, det_rand, true};
     NodeId nodeid{1};
     node::TxDownloadConnectionInfo DEFAULT_CONN{/*m_preferred=*/false, /*m_relay_permissions=*/false, /*m_wtxid_relay=*/true};