[p2p] bump DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE to 3,000

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

This limit also gives acceptable bounds for worst case LimitOrphans iterations.

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

Also deletes the now-unused DEFAULT_MAX_ORPHAN_TRANSACTIONS.

Author: glozow

src/node/txorphanage.h

@@ -20,9 +20,7 @@ namespace node {
 static constexpr int64_t DEFAULT_RESERVED_ORPHAN_WEIGHT_PER_PEER{404'000};
 /** Default value for TxOrphanage::m_max_global_latency_score. Helps limit the maximum latency for operations like
  * EraseForBlock and LimitOrphans. */
-static constexpr unsigned int DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE{100};
-/** Default maximum number of orphan transactions kept in memory */
-static const uint32_t DEFAULT_MAX_ORPHAN_TRANSACTIONS{100};
+static constexpr unsigned int DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE{3000};
 
 /** 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

@@ -277,12 +277,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->Size() <= 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)) {
@@ -301,7 +298,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}};
 
@@ -434,7 +430,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

@@ -52,7 +52,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 = [&] {
@@ -100,7 +100,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()};
@@ -220,7 +220,6 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage)
                     // test mocktime and expiry
                     SetMockTime(ConsumeTime(fuzzed_data_provider));
                     orphanage->LimitOrphans();
-                    Assert(orphanage->Size() <= node::DEFAULT_MAX_ORPHAN_TRANSACTIONS);
                 });
 
         }

test/functional/p2p_invalid_tx.py

@@ -143,14 +143,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()
@@ -160,8 +160,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()