Merge bitcoin#29031: fuzz: Improve fuzzing stability for txorphan har…

…ness 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
lateminer · Dec 11, 2023 · 40bc501 · 40bc501
2 parents ba5f16e + 15f5a0d
commit 40bc501
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 8 deletions.
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -4298,7 +4298,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 m_txrequest.ForgetTxHash(tx.GetWitnessHash());
 
                 // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789)
-                m_orphanage.LimitOrphans(m_opts.max_orphan_txs);
+                m_orphanage.LimitOrphans(m_opts.max_orphan_txs, m_rng);
             } else {
                 LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n",
                          tx.GetHash().ToString(),

diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
@@ -33,6 +33,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;
@@ -132,7 +133,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);
                 });
         }

diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
@@ -129,11 +129,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);
 }
 

diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
@@ -113,7 +113,7 @@ void TxOrphanage::EraseForPeer(NodeId peer)
     if (nErased > 0) LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx from peer=%d\n", nErased, peer);
 }
 
-void TxOrphanage::LimitOrphans(unsigned int max_orphans)
+void TxOrphanage::LimitOrphans(unsigned int max_orphans, FastRandomContext& rng)
 {
     LOCK(m_mutex);
 
@@ -138,7 +138,6 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans)
         nNextSweep = nMinExpTime + ORPHAN_TX_EXPIRE_INTERVAL;
         if (nErased > 0) LogPrint(BCLog::TXPACKAGES, "Erased %d orphan tx due to expiration\n", nErased);
     }
-    FastRandomContext rng;
     while (m_orphans.size() > max_orphans)
     {
         // Evict a random orphan:

diff --git a/src/txorphanage.h b/src/txorphanage.h
@@ -43,7 +43,7 @@ class TxOrphanage {
     void EraseForBlock(const CBlock& block) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Limit the orphanage to the given maximum */
-    void LimitOrphans(unsigned int max_orphans) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
+    void LimitOrphans(unsigned int max_orphans, FastRandomContext& rng) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
 
     /** Add any orphans that list a particular tx as a parent into the from peer's work set */
     void AddChildrenToWorkSet(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);;