fix(net): amortize ChainLock seen-cache pruning

src/chainlock/handler.cpp

@@ -47,7 +47,7 @@ ChainlockHandler::ChainlockHandler(chainlock::Chainlocks& chainlocks, Chainstate
     scheduler{std::make_unique<CScheduler>()},
     scheduler_thread{
         std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))},
-    seenChainLocks{MAX_SEEN_CHAINLOCKS}
+    seenChainLocks{SEEN_CHAINLOCKS_RETAINED_SIZE, SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE}
 {
 }
 
@@ -89,12 +89,18 @@ size_t ChainlockHandler::SeenChainLockCacheSizeForTesting() const
     return seenChainLocks.size();
 }
 
-size_t ChainlockHandler::SeenChainLockCacheMaxSizeForTesting() const
+size_t ChainlockHandler::SeenChainLockCacheRetainedSizeForTesting() const
 {
     LOCK(cs);
     return seenChainLocks.max_size();
 }
 
+size_t ChainlockHandler::SeenChainLockCachePruneAfterSizeForTesting() const
+{
+    LOCK(cs);
+    return seenChainLocks.prune_after_size();
+}
+
 void ChainlockHandler::UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time)
 {
     AssertLockNotHeld(cs);

src/chainlock/handler.h

@@ -54,7 +54,16 @@ class ChainlockHandler final : public CValidationInterface
     std::atomic<bool> tryLockChainTipScheduled{false};
     std::atomic<bool> isEnabled{false};
 
-    static constexpr size_t MAX_SEEN_CHAINLOCKS{1024};
+    //! Number of recently seen CLSIG hashes retained once `seenChainLocks` is pruned.
+    static constexpr size_t SEEN_CHAINLOCKS_RETAINED_SIZE{1024};
+    //! Size `seenChainLocks` may grow to before the next insertion prunes it back down to
+    //! SEEN_CHAINLOCKS_RETAINED_SIZE. Pruning sorts every entry, and CLSIG hashes are recorded
+    //! before the signature is verified, so pruning on each insertion past the retained size
+    //! lets a peer turn a stream of unique CLSIG hashes into a stream of O(n log n) sorts under
+    //! `cs`. Pruning only after twice the retained size amortises that cost over the entries
+    //! dropped in a single batch, at the price of a larger transient cache. The 2x ratio matches
+    //! the default in unordered_lru_cache.
+    static constexpr size_t SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE{2 * SEEN_CHAINLOCKS_RETAINED_SIZE};
 
     const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs){nullptr};
     Uint256HashMap<std::chrono::seconds> txFirstSeenTime GUARDED_BY(cs);
@@ -77,7 +86,10 @@ class ChainlockHandler final : public CValidationInterface
     bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     size_t SeenChainLockCacheSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    size_t SeenChainLockCacheMaxSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    //! Number of entries retained after the seen cache is pruned.
+    size_t SeenChainLockCacheRetainedSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    //! Size the seen cache may grow to before it is pruned back to the retained size.
+    size_t SeenChainLockCachePruneAfterSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     [[nodiscard]] MessageProcessingResult ProcessNewChainLock(NodeId from, const chainlock::ChainLockSig& clsig,
                                                               const llmq::CQuorumManager& qman,

src/limitedmap.h

@@ -31,6 +31,11 @@ class unordered_limitedmap
     size_type nPruneAfterSize;
 
 public:
+    //! nMaxSizeIn is the number of elements retained after a prune. nPruneAfterSizeIn is the size
+    //! the map may grow to before the next insertion prunes it; it defaults to nMaxSizeIn, which
+    //! means prune() -- and therefore a sort of every element -- runs on *every* insertion past
+    //! nMaxSizeIn. Callers whose keys are attacker-supplied should pass a larger value (e.g.
+    //! 2 * nMaxSizeIn) so that sorting is amortised over a batch of evictions instead.
     explicit unordered_limitedmap(size_type nMaxSizeIn, size_type nPruneAfterSizeIn = 0)
     {
         assert(nMaxSizeIn > 0);
@@ -68,7 +73,11 @@ class unordered_limitedmap
             return;
         itTarget->second = v;
     }
+    //! Number of elements retained after a prune.
     size_type max_size() const { return nMaxSize; }
+    //! Size the map is allowed to grow to before a prune is triggered. Always >= max_size();
+    //! when larger, the map temporarily holds more than max_size() elements between prunes.
+    size_type prune_after_size() const { return nPruneAfterSize; }
     size_type max_size(size_type nMaxSizeIn, size_type nPruneAfterSizeIn = 0)
     {
         assert(nMaxSizeIn > 0);

src/test/limitedmap_tests.cpp

@@ -99,4 +99,74 @@ BOOST_AUTO_TEST_CASE(limitedmap_test)
     BOOST_CHECK(map.empty());
 }
 
+// A map constructed with a prune-after size larger than its retained size must not prune on
+// every insertion past the retained size. Instead it is allowed to grow up to the prune-after
+// size and is then pruned back down to the retained size in a single batch. This amortises the
+// cost of prune() -- which sorts every element -- over many insertions.
+BOOST_AUTO_TEST_CASE(limitedmap_prune_after_size_test)
+{
+    constexpr int RETAINED_SIZE{10};
+    constexpr int PRUNE_AFTER_SIZE{2 * RETAINED_SIZE};
+
+    unordered_limitedmap<int, int> map(RETAINED_SIZE, PRUNE_AFTER_SIZE);
+
+    // max_size() reports the retained size, not the temporary prune threshold
+    BOOST_CHECK_EQUAL(map.max_size(), static_cast<size_t>(RETAINED_SIZE));
+
+    // fill the map up to (and including) the prune-after size. No prune may happen along the
+    // way, so every insertion is retained -- in particular size() exceeds the retained size
+    // without anything being evicted.
+    for (int i = 0; i < PRUNE_AFTER_SIZE; i++) {
+        map.insert(std::pair<int, int>(i, i));
+        BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(i) + 1U);
+    }
+
+    // nothing has been evicted yet, even though we are well past the retained size
+    BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(PRUNE_AFTER_SIZE));
+    for (int i = 0; i < PRUNE_AFTER_SIZE; i++) {
+        BOOST_CHECK(map.count(i) == 1);
+    }
+
+    // crossing the prune-after size prunes back down to the retained size in one batch
+    map.insert(std::pair<int, int>(PRUNE_AFTER_SIZE, PRUNE_AFTER_SIZE));
+    BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(RETAINED_SIZE));
+
+    // the retained entries are the ones with the highest values
+    for (int i = 0; i <= PRUNE_AFTER_SIZE; i++) {
+        if (i <= PRUNE_AFTER_SIZE - RETAINED_SIZE) {
+            BOOST_CHECK(map.count(i) == 0);
+        } else {
+            BOOST_CHECK(map.count(i) == 1);
+        }
+    }
+
+    // the cycle repeats: after a prune the map grows again without evicting anything until it
+    // once more exceeds the prune-after size, at which point it drops back to the retained size
+    int next_key{PRUNE_AFTER_SIZE + 1};
+    for (int i = 0; i < PRUNE_AFTER_SIZE - RETAINED_SIZE; i++, next_key++) {
+        map.insert(std::pair<int, int>(next_key, next_key));
+        BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(RETAINED_SIZE + i) + 1U);
+    }
+    BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(PRUNE_AFTER_SIZE));
+
+    map.insert(std::pair<int, int>(next_key, next_key));
+    BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(RETAINED_SIZE));
+}
+
+// Without an explicit prune-after size the map prunes as soon as it exceeds the retained size,
+// so it never holds more than that many elements.
+BOOST_AUTO_TEST_CASE(limitedmap_default_prune_after_size_test)
+{
+    constexpr int RETAINED_SIZE{10};
+
+    unordered_limitedmap<int, int> map(RETAINED_SIZE);
+    BOOST_CHECK_EQUAL(map.max_size(), static_cast<size_t>(RETAINED_SIZE));
+
+    for (int i = 0; i < 4 * RETAINED_SIZE; i++) {
+        map.insert(std::pair<int, int>(i, i));
+        BOOST_CHECK_LE(map.size(), static_cast<size_t>(RETAINED_SIZE));
+    }
+    BOOST_CHECK_EQUAL(map.size(), static_cast<size_t>(RETAINED_SIZE));
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/llmq_chainlock_tests.cpp

@@ -196,18 +196,35 @@ BOOST_FIXTURE_TEST_CASE(seen_chainlock_cache_is_bounded, TestingSetup)
 {
     m_node.clhandler->CheckActiveState();
 
-    const size_t max_size = m_node.clhandler->SeenChainLockCacheMaxSizeForTesting();
-    BOOST_REQUIRE_GT(max_size, 0U);
-
-    for (size_t i = 0; i < max_size + 1; ++i) {
+    const size_t retained_size = m_node.clhandler->SeenChainLockCacheRetainedSizeForTesting();
+    const size_t prune_after_size = m_node.clhandler->SeenChainLockCachePruneAfterSizeForTesting();
+    BOOST_REQUIRE_GT(retained_size, 0U);
+    // The cache prunes with hysteresis: it is allowed to grow past the retained size and is only
+    // pruned back down once it exceeds the (larger) prune-after size. Pruning sorts every entry,
+    // so pruning on each insertion past the retained size would be a peer-triggered CPU
+    // amplification path.
+    BOOST_REQUIRE_GT(prune_after_size, retained_size);
+
+    const auto process = [&](size_t i) {
         auto clsig = CreateChainLock(static_cast<int32_t>(i), GetTestBlockHash(static_cast<uint32_t>(2000 + i)));
         [[maybe_unused]] const auto result =
             m_node.clhandler->ProcessNewChainLock(/*from=*/-1, clsig, *m_node.llmq_ctx->qman, ::SerializeHash(clsig));
-        BOOST_CHECK_LE(m_node.clhandler->SeenChainLockCacheSizeForTesting(), max_size);
-        if (i == 0) {
-            BOOST_CHECK_GT(m_node.clhandler->SeenChainLockCacheSizeForTesting(), 0U);
-        }
+    };
+
+    // Growing up to the prune-after size must not evict anything, so no prune (and no sort) has
+    // run yet -- in particular there is no strict cap at the retained size.
+    for (size_t i = 0; i < prune_after_size; ++i) {
+        process(i);
+        BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(), i + 1U);
     }
+    BOOST_CHECK_GT(m_node.clhandler->SeenChainLockCacheSizeForTesting(), retained_size);
+
+    // Crossing the prune-after size prunes back down to the retained size in a single batch.
+    process(prune_after_size);
+    BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(), retained_size);
+
+    // Repeated prune cycles are covered generically by limitedmap_prune_after_size_test; this
+    // case only pins down how ChainlockHandler wires the cache up.
 }
 
 BOOST_FIXTURE_TEST_CASE(best_chainlock_is_already_have_after_seen_cache_eviction, TestingSetup)
@@ -219,15 +236,18 @@ BOOST_FIXTURE_TEST_CASE(best_chainlock_is_already_have_after_seen_cache_eviction
     BOOST_REQUIRE(m_node.chainlocks->UpdateBestChainlock(best_hash, best_clsig, /*pindex=*/nullptr));
     BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, best_hash}));
 
-    const size_t max_size = m_node.clhandler->SeenChainLockCacheMaxSizeForTesting();
-    BOOST_REQUIRE_GT(max_size, 0U);
+    const size_t prune_after_size = m_node.clhandler->SeenChainLockCachePruneAfterSizeForTesting();
+    BOOST_REQUIRE_GT(prune_after_size, 0U);
 
-    for (size_t i = 0; i < max_size + 1; ++i) {
+    // Insert enough unique CLSIGs to force at least one prune of the seen cache.
+    for (size_t i = 0; i < prune_after_size + 1; ++i) {
         auto clsig = CreateChainLock(static_cast<int32_t>(101 + i), GetTestBlockHash(static_cast<uint32_t>(3000 + i)));
         [[maybe_unused]] const auto result =
             m_node.clhandler->ProcessNewChainLock(/*from=*/-1, clsig, *m_node.llmq_ctx->qman, ::SerializeHash(clsig));
-        BOOST_CHECK_LE(m_node.clhandler->SeenChainLockCacheSizeForTesting(), max_size);
+        BOOST_CHECK_LE(m_node.clhandler->SeenChainLockCacheSizeForTesting(), prune_after_size);
     }
+    BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(),
+                      m_node.clhandler->SeenChainLockCacheRetainedSizeForTesting());
 
     BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, best_hash}));
 }