Merge bitcoin/bitcoin#30082: test: expand LimitOrphan and EraseForPeer coverage

172c1ad026cc38c6f52679e74c14579ecc77c48e test: expand LimitOrphan and EraseForPeer coverage (Greg Sanders)
28dbe218feef51cbc28051273334dd73ba4500c0 refactor: move orphanage constants to header file (Greg Sanders)

Pull request description:

  Inspired by refactorings in #30000 as the coverage appeared a bit sparse.

  Added some minimal border value testing, timeouts, and tightened existing assertions.

ACKs for top commit:
  achow101:
    ACK 172c1ad026cc38c6f52679e74c14579ecc77c48e
  rkrux:
    reACK [172c1ad](https://github.com/bitcoin/bitcoin/pull/30082/commits/172c1ad026cc38c6f52679e74c14579ecc77c48e)
  glozow:
    reACK 172c1ad026cc38c6f52679e74c14579ecc77c48e

Tree-SHA512: e8fa9b1de6a8617612bbe9b132c9c0c9b5a651ec94fd8c91042a34a8c91c5f9fa7ec4175b47e2b97d1320d452c23775be671a9970613533e68e81937539a7d70

3 files changed, 46 insertions, 13 deletions

diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp
index 082d090d7c..d2dab94526 100644
--- a/src/test/orphanage_tests.cpp
+++ b/src/test/orphanage_tests.cpp
@@ -112,6 +112,10 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
     FillableSigningProvider keystore;
     BOOST_CHECK(keystore.AddKey(key));
 
+    // Freeze time for length of test
+    auto now{GetTime<std::chrono::seconds>()};
+    SetMockTime(now);
+
     // 50 orphan transactions:
     for (int i = 0; i < 50; i++)
     {
@@ -170,22 +174,52 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), i));
     }
 
-    // Test EraseOrphansFor:
+    size_t expected_num_orphans = orphanage.CountOrphans();
+
+    // Non-existent peer; nothing should be deleted
+    orphanage.EraseForPeer(/*peer=*/-1);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), expected_num_orphans);
+
+    // Each of first three peers stored
+    // two transactions each.
     for (NodeId i = 0; i < 3; i++)
     {
-        size_t sizeBefore = orphanage.CountOrphans();
         orphanage.EraseForPeer(i);
-        BOOST_CHECK(orphanage.CountOrphans() < sizeBefore);
+        expected_num_orphans -= 2;
+        BOOST_CHECK(orphanage.CountOrphans() == expected_num_orphans);
     }
 
-    // Test LimitOrphanTxSize() function:
+    // Test LimitOrphanTxSize() function, nothing should timeout:
     FastRandomContext rng{/*fDeterministic=*/true};
+    orphanage.LimitOrphans(/*max_orphans=*/expected_num_orphans, rng);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), expected_num_orphans);
+    expected_num_orphans -= 1;
+    orphanage.LimitOrphans(/*max_orphans=*/expected_num_orphans, rng);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), expected_num_orphans);
+    assert(expected_num_orphans > 40);
     orphanage.LimitOrphans(40, rng);
-    BOOST_CHECK(orphanage.CountOrphans() <= 40);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 40);
     orphanage.LimitOrphans(10, rng);
-    BOOST_CHECK(orphanage.CountOrphans() <= 10);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 10);
     orphanage.LimitOrphans(0, rng);
-    BOOST_CHECK(orphanage.CountOrphans() == 0);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 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.CountOrphans(), 1);
+
+    // One second shy of expiration
+    SetMockTime(now + ORPHAN_TX_EXPIRE_TIME - 1s);
+    orphanage.LimitOrphans(1, rng);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 1);
+
+    // Jump one more second, orphan should be timed out on limiting
+    SetMockTime(now + ORPHAN_TX_EXPIRE_TIME);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 1);
+    orphanage.LimitOrphans(1, rng);
+    BOOST_CHECK_EQUAL(orphanage.CountOrphans(), 0);
 }
 
 BOOST_AUTO_TEST_CASE(same_txid_diff_witness)
diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp
index df9b96e64d..faab208333 100644
--- a/src/txorphanage.cpp
+++ b/src/txorphanage.cpp
@@ -12,12 +12,6 @@
 
 #include <cassert>
 
-/** Expiration time for orphan transactions */
-static constexpr auto ORPHAN_TX_EXPIRE_TIME{20min};
-/** Minimum time between orphan transactions expire time checks */
-static constexpr auto ORPHAN_TX_EXPIRE_INTERVAL{5min};
-
-
 bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer)
 {
     const Txid& hash = tx->GetHash();
diff --git a/src/txorphanage.h b/src/txorphanage.h
index f3f73ce0f2..2c53d1d40f 100644
--- a/src/txorphanage.h
+++ b/src/txorphanage.h
@@ -14,6 +14,11 @@
 #include <map>
 #include <set>
 
+/** Expiration time for orphan transactions */
+static constexpr auto ORPHAN_TX_EXPIRE_TIME{20min};
+/** Minimum time between orphan transactions expire time checks */
+static constexpr auto ORPHAN_TX_EXPIRE_INTERVAL{5min};
+
 /** 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 number of orphans