Merge bitcoin#29031: fuzz: Improve fuzzing stability for txorphan harness

15f5a0d fuzz: Improve fuzzing stability for txorphan harness (dergoegge)

Pull request description:

  The `txorphan` harness has low stability as eviction of orphan txs is entirely random at the moment.

  Fix this by passing the rng to `LimitOrphans`, which can be deterministic in tests.

  Also see bitcoin#29018.

ACKs for top commit:
  maflcko:
    lgtm ACK 15f5a0d
  brunoerg:
    utACK 15f5a0d

Tree-SHA512: 854ec34b3a0f16f26db6dc419096c6e7a380e8400119534aa278d6b1d54c253b572aa2fad13c383c796c431d8ff4263956e6f60326e99f8bf6abd16d9a280e97

Author: fanquake

src/net_processing.cpp

@@ -4762,7 +4762,7 @@ void PeerManagerImpl::ProcessMessage(
 
                 // DoS prevention: do not allow m_orphans to grow unbounded (see CVE-2012-3789)
                 unsigned int nMaxOrphanTxSize = (unsigned int)std::max((int64_t)0, gArgs.GetIntArg("-maxorphantxsize", DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE)) * 1000000;
-                m_orphanage.LimitOrphans(nMaxOrphanTxSize);
+                m_orphanage.LimitOrphans(nMaxOrphanTxSize, m_rng);
             } else {
                 LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString());
                 // We will continue to reject this tx since it has rejected

src/test/fuzz/txorphan.cpp

@@ -36,6 +36,7 @@ void initialize_orphanage()
 FUZZ_TARGET(txorphan, .init = initialize_orphanage)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
+    FastRandomContext limit_orphans_rng{/*fDeterministic=*/true};
     SetMockTime(ConsumeTime(fuzzed_data_provider));
 
     TxOrphanage orphanage;
@@ -140,7 +141,7 @@ 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.LimitOrphans(limit, limit_orphans_rng);
                     Assert(orphanage.Size() <= limit);
                 });
         }

src/test/orphanage_tests.cpp

@@ -126,11 +126,12 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     }
 
     // Test LimitOrphanTxSize() function:
-    orphanage.LimitOrphans(40);
+    FastRandomContext rng{/*fDeterministic=*/true};
+    orphanage.LimitOrphans(40, rng);
     BOOST_CHECK(orphanage.CountOrphans() <= 40);
-    orphanage.LimitOrphans(10);
+    orphanage.LimitOrphans(10, rng);
     BOOST_CHECK(orphanage.CountOrphans() <= 10);
-    orphanage.LimitOrphans(0);
+    orphanage.LimitOrphans(0, rng);
     BOOST_CHECK(orphanage.CountOrphans() == 0);
 }
 

src/txorphanage.cpp

@@ -116,7 +116,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx from peer=%d\n", nErased, peer);
 }
 
-void TxOrphanage::LimitOrphans(unsigned int max_orphans_size)
+void TxOrphanage::LimitOrphans(unsigned int max_orphans_size, FastRandomContext& rng)
 {
     LOCK(m_mutex);
 
@@ -141,7 +141,6 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans_size)
         nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
         if (nErased > 0) LogPrint(BCLog::MEMPOOL, "Erased %d orphan tx due to expiration\n", nErased);
     }
-    FastRandomContext rng;
     while (!m_orphans.empty() && m_orphan_tx_size > max_orphans_size)
     {
         // Evict a random orphan:

src/txorphanage.h

@@ -51,7 +51,7 @@ class TxOrphanage {
     bool HaveMoreWork(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Limit the orphanage to the given maximum */
-    void LimitOrphans(unsigned int max_orphans_size) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    void LimitOrphans(unsigned int max_orphans_size, FastRandomContext& rng) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Add any orphans that list a particular tx as a parent into a peer's work set */
     void AddChildrenToWorkSet(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);