[p2p] bump DEFAULT_MAX_ORPHAN_ANNOUNCEMENTS to 3000

For the default number of peers (125), allows each to relay a default
descendant package of transactions (up to 25-1=24 can be missing inputs)
out of order.

Functional tests aren't changed to check for a cap for 3000
because it would make the runtime too long.

Also deletes the now-unused DEFAULT_MAX_ORPHAN_TRANSACTIONS.

Author: glozow

src/node/txorphanage.h

@@ -19,11 +19,9 @@ namespace node {
 /** Default value for TxOrphanage::m_reserved_usage_per_peer. */
 static constexpr int64_t DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER{404'000};
 /** Default value for TxOrphanage::m_max_global_announcements. */
-static constexpr unsigned int DEFAULT_MAX_ORPHAN_ANNOUNCEMENTS{100};
+static constexpr unsigned int DEFAULT_MAX_ORPHAN_ANNOUNCEMENTS{3000};
 /** Minimum NodeId for lower_bound lookups (in practice, NodeIds start at 0). */
 static constexpr NodeId MIN_PEER{std::numeric_limits<NodeId>::min()};
-/** Default maximum number of orphan transactions kept in memory */
-static const uint32_t DEFAULT_MAX_ORPHAN_TRANSACTIONS{100};
 
 /** 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 amount of

src/test/fuzz/txdownloadman.cpp

@@ -278,12 +278,9 @@ FUZZ_TARGET(txdownloadman, .init = initialize)
 // peer without tracking anything (this is only for the txdownload_impl target).
 static bool HasRelayPermissions(NodeId peer) { return peer == 0; }
 
-static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl, size_t max_orphan_count)
+static void CheckInvariants(const node::TxDownloadManagerImpl& txdownload_impl)
 {
-    // Orphanage usage should never exceed what is allowed
-    Assert(txdownload_impl.m_orphanage->CountUniqueOrphans() <= max_orphan_count);
     txdownload_impl.m_orphanage->SanityCheck();
-
     // We should never have more than the maximum in-flight requests out for a peer.
     for (NodeId peer = 0; peer < NUM_PEERS; ++peer) {
         if (!HasRelayPermissions(peer)) {
@@ -302,7 +299,6 @@ 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 = node::DEFAULT_MAX_ORPHAN_TRANSACTIONS;
     FastRandomContext det_rand{true};
     node::TxDownloadManagerImpl txdownload_impl{node::TxDownloadOptions{pool, det_rand, true}};
 
@@ -436,7 +432,7 @@ FUZZ_TARGET(txdownloadman_impl, .init = initialize)
         if (fuzzed_data_provider.ConsumeBool()) time_skip *= -1;
         time += time_skip;
     }
-    CheckInvariants(txdownload_impl, max_orphan_count);
+    CheckInvariants(txdownload_impl);
     // Disconnect everybody, check that all data structures are empty.
     for (NodeId nodeid = 0; nodeid < NUM_PEERS; ++nodeid) {
         txdownload_impl.DisconnectedPeer(nodeid);

src/test/fuzz/txorphan.cpp

@@ -50,7 +50,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
 
     std::vector<CTransactionRef> tx_history;
 
-    LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * node::DEFAULT_MAX_ORPHAN_TRANSACTIONS)
+    LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 1000)
     {
         // construct transaction
         const CTransactionRef tx = [&] {
@@ -97,7 +97,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
             }
         }
         // trigger orphanage functions
-        LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * node::DEFAULT_MAX_ORPHAN_TRANSACTIONS)
+        LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 1000)
         {
             NodeId peer_id = fuzzed_data_provider.ConsumeIntegral<NodeId>();
             const auto total_bytes_start{orphanage->TotalOrphanUsage()};
@@ -217,7 +217,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     // test mocktime and expiry
                     SetMockTime(ConsumeTime(fuzzed_data_provider));
                     orphanage->LimitOrphans();
-                    Assert(orphanage->CountUniqueOrphans() <= node::DEFAULT_MAX_ORPHAN_TRANSACTIONS);
                 });
         }
 
@@ -270,7 +269,7 @@ FUZZ_TARGET(txorphan_protected, .init = initialize_orphanage)
     // These are honest peer's live announcements. We expect them to be protected from eviction.
     std::set<Wtxid> protected_wtxids;
 
-    LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 10 * global_announcement_limit)
+    LIMITED_WHILE(outpoints.size() < 200'000 && fuzzed_data_provider.ConsumeBool(), 1000)
     {
         // construct transaction
         const CTransactionRef tx = [&] {

test/functional/p2p_invalid_tx.py

@@ -147,14 +147,14 @@ def run_test(self):
         self.wait_until(lambda: 1 == len(node.getpeerinfo()), timeout=12)  # p2ps[1] is no longer connected
         assert_equal(expected_mempool, set(node.getrawmempool()))
 
-        self.log.info('Test orphan pool overflow')
+        self.log.info('Test orphanage can store more than 100 transactions')
         orphan_tx_pool = [CTransaction() for _ in range(101)]
         for i in range(len(orphan_tx_pool)):
             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(['orphanage overflow, removed 1 tx']):
-            node.p2ps[0].send_txs_and_test(orphan_tx_pool, node, success=False)
+        node.p2ps[0].send_txs_and_test(orphan_tx_pool, node, success=False)
+        self.wait_until(lambda: len(node.getorphantxs()) >= 101)
 
         self.log.info('Test orphan with rejected parents')
         rejected_parent = CTransaction()
@@ -165,8 +165,8 @@ def run_test(self):
             node.p2ps[0].send_txs_and_test([rejected_parent], node, success=False)
 
         self.log.info('Test that a peer disconnection causes erase its transactions from the orphan pool')
-        with node.assert_debug_log(['Erased 100 orphan transaction(s) from peer=26']):
-            self.reconnect_p2p(num_connections=1)
+        self.reconnect_p2p(num_connections=1)
+        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
         self.log.info('Test that a transaction in the orphan pool is included in a new tip block causes erase this transaction from the orphan pool')
         tx_withhold_until_block_A = CTransaction()

test/functional/p2p_orphan_handling.py

@@ -43,8 +43,6 @@
 # for one peer and y seconds for another, use specific values instead.
 TXREQUEST_TIME_SKIP = NONPREF_PEER_TX_DELAY + TXID_RELAY_DELAY + OVERLOADED_PEER_TX_DELAY + 1
 
-DEFAULT_MAX_ORPHAN_TRANSACTIONS = 100
-
 def cleanup(func):
     # Time to fastfoward (using setmocktime) in between subtests to ensure they do not interfere with
     # one another, in seconds. Equal to 12 hours, which is enough to expire anything that may exist
@@ -593,46 +591,6 @@ def test_orphan_txid_inv(self):
         assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"])
         self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
-    @cleanup
-    def test_max_orphan_amount(self):
-        self.log.info("Check that we never exceed our storage limits for orphans")
-
-        node = self.nodes[0]
-        self.generate(self.wallet, 1)
-        peer_1 = node.add_p2p_connection(P2PInterface())
-
-        self.log.info("Check that orphanage is empty on start of test")
-        assert len(node.getorphantxs()) == 0
-
-        self.log.info("Filling up orphanage with " + str(DEFAULT_MAX_ORPHAN_TRANSACTIONS) + "(DEFAULT_MAX_ORPHAN_TRANSACTIONS) orphans")
-        orphans = []
-        parent_orphans = []
-        for _ in range(DEFAULT_MAX_ORPHAN_TRANSACTIONS):
-            tx_parent_1 = self.wallet.create_self_transfer()
-            tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"])
-            parent_orphans.append(tx_parent_1["tx"])
-            orphans.append(tx_child_1["tx"])
-            peer_1.send_without_ping(msg_tx(tx_child_1["tx"]))
-
-        peer_1.sync_with_ping()
-        orphanage = node.getorphantxs()
-        self.wait_until(lambda: len(node.getorphantxs()) == DEFAULT_MAX_ORPHAN_TRANSACTIONS)
-
-        for orphan in orphans:
-            assert tx_in_orphanage(node, orphan)
-
-        self.log.info("Check that we do not add more than the max orphan amount")
-        tx_parent_1 = self.wallet.create_self_transfer()
-        tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"])
-        peer_1.send_and_ping(msg_tx(tx_child_1["tx"]))
-        parent_orphans.append(tx_parent_1["tx"])
-        orphanage = node.getorphantxs()
-        assert_equal(len(orphanage), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
-
-        self.log.info("Clearing the orphanage")
-        for index, parent_orphan in enumerate(parent_orphans):
-            peer_1.send_and_ping(msg_tx(parent_orphan))
-        self.wait_until(lambda: len(node.getorphantxs()) == 0)
 
     @cleanup
     def test_orphan_handling_prefer_outbound(self):
@@ -820,7 +778,6 @@ def run_test(self):
         self.test_same_txid_orphan()
         self.test_same_txid_orphan_of_orphan()
         self.test_orphan_txid_inv()
-        self.test_max_orphan_amount()
         self.test_orphan_handling_prefer_outbound()
         self.test_announcers_before_and_after()
         self.test_parents_change()