diff --git a/src/chainlock/handler.cpp b/src/chainlock/handler.cpp index c20ef531bf53..2201f47a4efe 100644 --- a/src/chainlock/handler.cpp +++ b/src/chainlock/handler.cpp @@ -47,7 +47,7 @@ ChainlockHandler::ChainlockHandler(chainlock::Chainlocks& chainlocks, Chainstate scheduler{std::make_unique()}, scheduler_thread{ std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))}, - seenChainLocks{SEEN_CHAINLOCKS_RETAINED_SIZE, SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE} + seenChainLocks{SEEN_CHAINLOCKS_CUTOFF_SIZE, SEEN_CHAINLOCKS_MAX_SIZE} { } @@ -89,18 +89,6 @@ size_t ChainlockHandler::SeenChainLockCacheSizeForTesting() const return seenChainLocks.size(); } -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); diff --git a/src/chainlock/handler.h b/src/chainlock/handler.h index 8d50dd2867dc..d369113eeb4f 100644 --- a/src/chainlock/handler.h +++ b/src/chainlock/handler.h @@ -54,16 +54,8 @@ class ChainlockHandler final : public CValidationInterface std::atomic tryLockChainTipScheduled{false}; std::atomic isEnabled{false}; - //! 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 partitions 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 full-map partition - //! passes 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}; + static constexpr size_t SEEN_CHAINLOCKS_CUTOFF_SIZE{1024}; + static constexpr size_t SEEN_CHAINLOCKS_MAX_SIZE{2 * SEEN_CHAINLOCKS_CUTOFF_SIZE}; const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs){nullptr}; Uint256HashMap txFirstSeenTime GUARDED_BY(cs); @@ -86,10 +78,6 @@ 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); - //! 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, diff --git a/src/limitedmap.h b/src/limitedmap.h index ee04db0de8ee..3cb1fe642f83 100644 --- a/src/limitedmap.h +++ b/src/limitedmap.h @@ -27,25 +27,22 @@ class unordered_limitedmap protected: std::unordered_map map; typedef typename std::unordered_map::iterator iterator; + size_type nCutoffSize; size_type nMaxSize; - 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 partition 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 partitioning is amortised over a batch of evictions instead. - explicit unordered_limitedmap(size_type nMaxSizeIn, size_type nPruneAfterSizeIn = 0) + //! nCutoffSizeIn is the number of elements retained after a prune. nMaxSizeIn is the size the + //! map may grow to before a prune; it defaults to nCutoffSizeIn. + explicit unordered_limitedmap(size_type nCutoffSizeIn, size_type nMaxSizeIn = 0) { - assert(nMaxSizeIn > 0); - nMaxSize = nMaxSizeIn; - if (nPruneAfterSizeIn == 0) { - nPruneAfterSize = nMaxSize; + assert(nCutoffSizeIn > 0); + nCutoffSize = nCutoffSizeIn; + if (nMaxSizeIn == 0) { + nMaxSize = nCutoffSize; } else { - nPruneAfterSize = nPruneAfterSizeIn; + nMaxSize = nMaxSizeIn; } - assert(nPruneAfterSize >= nMaxSize); + assert(nMaxSize >= nCutoffSize); } const_iterator begin() const { return map.begin(); } const_iterator end() const { return map.end(); } @@ -74,26 +71,25 @@ class unordered_limitedmap itTarget->second = v; } //! Number of elements retained after a prune. + size_type cutoff_size() const { return nCutoffSize; } + //! Size the map is allowed to grow to before a prune is triggered. Always >= cutoff_size(). 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) + size_type max_size(size_type nCutoffSizeIn, size_type nMaxSizeIn = 0) { - assert(nMaxSizeIn > 0); - nMaxSize = nMaxSizeIn; - if (nPruneAfterSizeIn == 0) { - nPruneAfterSize = nMaxSize; + assert(nCutoffSizeIn > 0); + nCutoffSize = nCutoffSizeIn; + if (nMaxSizeIn == 0) { + nMaxSize = nCutoffSize; } else { - nPruneAfterSize = nPruneAfterSizeIn; + nMaxSize = nMaxSizeIn; } - assert(nPruneAfterSize >= nMaxSize); + assert(nMaxSize >= nCutoffSize); prune(); return nMaxSize; } void prune() { - if (map.size() <= nPruneAfterSize) { + if (map.size() <= nMaxSize) { return; } @@ -102,8 +98,8 @@ class unordered_limitedmap for (auto it = map.begin(); it != map.end(); ++it) { iterators.emplace_back(it); } - size_type tooMuch = map.size() - nMaxSize; - // nPruneAfterSize >= nMaxSize > 0 keeps tooMuch inside the vector, which nth_element relies on + size_type tooMuch = map.size() - nCutoffSize; + // nMaxSize >= nCutoffSize > 0 keeps tooMuch inside the vector, which nth_element relies on assert(tooMuch > 0 && tooMuch < iterators.size()); // Only the entries below the eviction boundary have to be identified, their relative order does not matter std::nth_element(iterators.begin(), iterators.begin() + tooMuch, iterators.end(), diff --git a/src/test/limitedmap_tests.cpp b/src/test/limitedmap_tests.cpp index 999a42139096..0edc7e4a21b9 100644 --- a/src/test/limitedmap_tests.cpp +++ b/src/test/limitedmap_tests.cpp @@ -99,41 +99,41 @@ 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 +// A map constructed with a max size larger than its cutoff size must not prune on +// every insertion past the cutoff size. Instead it is allowed to grow up to the max +// size and is then pruned back down to the cutoff size in a single batch. This amortises the // cost of prune() -- which partitions every element -- over many insertions. -BOOST_AUTO_TEST_CASE(limitedmap_prune_after_size_test) +BOOST_AUTO_TEST_CASE(limitedmap_max_size_test) { - constexpr int RETAINED_SIZE{10}; - constexpr int PRUNE_AFTER_SIZE{2 * RETAINED_SIZE}; + constexpr int CUTOFF_SIZE{10}; + constexpr int MAX_SIZE{2 * CUTOFF_SIZE}; - unordered_limitedmap map(RETAINED_SIZE, PRUNE_AFTER_SIZE); + unordered_limitedmap map(CUTOFF_SIZE, MAX_SIZE); - // max_size() reports the retained size, not the temporary prune threshold - BOOST_CHECK_EQUAL(map.max_size(), static_cast(RETAINED_SIZE)); + BOOST_CHECK_EQUAL(map.cutoff_size(), static_cast(CUTOFF_SIZE)); + BOOST_CHECK_EQUAL(map.max_size(), static_cast(MAX_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 + // fill the map up to (and including) the max size. No prune may happen along the + // way, so every insertion is retained -- in particular size() exceeds the cutoff size // without anything being evicted. - for (int i = 0; i < PRUNE_AFTER_SIZE; i++) { + for (int i = 0; i < MAX_SIZE; i++) { map.insert(std::pair(i, i)); BOOST_CHECK_EQUAL(map.size(), static_cast(i) + 1U); } - // nothing has been evicted yet, even though we are well past the retained size - BOOST_CHECK_EQUAL(map.size(), static_cast(PRUNE_AFTER_SIZE)); - for (int i = 0; i < PRUNE_AFTER_SIZE; i++) { + // nothing has been evicted yet, even though we are well past the cutoff size + BOOST_CHECK_EQUAL(map.size(), static_cast(MAX_SIZE)); + for (int i = 0; i < MAX_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(PRUNE_AFTER_SIZE, PRUNE_AFTER_SIZE)); - BOOST_CHECK_EQUAL(map.size(), static_cast(RETAINED_SIZE)); + // crossing the max size prunes back down to the cutoff size in one batch + map.insert(std::pair(MAX_SIZE, MAX_SIZE)); + BOOST_CHECK_EQUAL(map.size(), static_cast(CUTOFF_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) { + // the entries retained after pruning are the ones with the highest values + for (int i = 0; i <= MAX_SIZE; i++) { + if (i <= MAX_SIZE - CUTOFF_SIZE) { BOOST_CHECK(map.count(i) == 0); } else { BOOST_CHECK(map.count(i) == 1); @@ -141,32 +141,33 @@ BOOST_AUTO_TEST_CASE(limitedmap_prune_after_size_test) } // 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++) { + // once more exceeds the max size, at which point it drops back to the cutoff size + int next_key{MAX_SIZE + 1}; + for (int i = 0; i < MAX_SIZE - CUTOFF_SIZE; i++, next_key++) { map.insert(std::pair(next_key, next_key)); - BOOST_CHECK_EQUAL(map.size(), static_cast(RETAINED_SIZE + i) + 1U); + BOOST_CHECK_EQUAL(map.size(), static_cast(CUTOFF_SIZE + i) + 1U); } - BOOST_CHECK_EQUAL(map.size(), static_cast(PRUNE_AFTER_SIZE)); + BOOST_CHECK_EQUAL(map.size(), static_cast(MAX_SIZE)); map.insert(std::pair(next_key, next_key)); - BOOST_CHECK_EQUAL(map.size(), static_cast(RETAINED_SIZE)); + BOOST_CHECK_EQUAL(map.size(), static_cast(CUTOFF_SIZE)); } -// Without an explicit prune-after size the map prunes as soon as it exceeds the retained size, +// Without an explicit max size the map prunes as soon as it exceeds the cutoff size, // so it never holds more than that many elements. -BOOST_AUTO_TEST_CASE(limitedmap_default_prune_after_size_test) +BOOST_AUTO_TEST_CASE(limitedmap_default_max_size_test) { - constexpr int RETAINED_SIZE{10}; + constexpr int CUTOFF_SIZE{10}; - unordered_limitedmap map(RETAINED_SIZE); - BOOST_CHECK_EQUAL(map.max_size(), static_cast(RETAINED_SIZE)); + unordered_limitedmap map(CUTOFF_SIZE); + BOOST_CHECK_EQUAL(map.cutoff_size(), static_cast(CUTOFF_SIZE)); + BOOST_CHECK_EQUAL(map.max_size(), static_cast(CUTOFF_SIZE)); - for (int i = 0; i < 4 * RETAINED_SIZE; i++) { + for (int i = 0; i < 4 * CUTOFF_SIZE; i++) { map.insert(std::pair(i, i)); - BOOST_CHECK_LE(map.size(), static_cast(RETAINED_SIZE)); + BOOST_CHECK_LE(map.size(), static_cast(CUTOFF_SIZE)); } - BOOST_CHECK_EQUAL(map.size(), static_cast(RETAINED_SIZE)); + BOOST_CHECK_EQUAL(map.size(), static_cast(CUTOFF_SIZE)); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index fa48e6ac94a3..008a5c261b1f 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -26,11 +26,17 @@ #include #include +#include using chainlock::ChainLockSig; using namespace llmq; using namespace llmq::testutils; +namespace { +constexpr size_t MAX_SEEN_CHAINLOCKS{2500}; +constexpr size_t RECENT_CHAINLOCKS_TO_RETAIN{2}; +} // namespace + BOOST_AUTO_TEST_SUITE(llmq_chainlock_tests) BOOST_AUTO_TEST_CASE(chainlock_construction_test) @@ -207,35 +213,28 @@ BOOST_FIXTURE_TEST_CASE(seen_chainlock_cache_is_bounded, TestingSetup) { m_node.clhandler->CheckActiveState(); - 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(i), GetTestBlockHash(static_cast(2000 + i))); + SetMockTime(std::chrono::seconds{100000 + static_cast(i)}); + auto clsig = CreateChainLock(100, GetTestBlockHash(static_cast(2000 + i))); + const auto hash = ::SerializeHash(clsig); [[maybe_unused]] const auto result = - m_node.clhandler->ProcessNewChainLock(/*from=*/-1, clsig, *m_node.llmq_ctx->qman, ::SerializeHash(clsig)); + m_node.clhandler->ProcessNewChainLock(/*from=*/-1, clsig, *m_node.llmq_ctx->qman, hash); + return hash; }; - // 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); + std::vector recent_hashes; + for (size_t i = 0; i <= MAX_SEEN_CHAINLOCKS; ++i) { + const auto hash = process(i); + if (i >= MAX_SEEN_CHAINLOCKS + 1 - RECENT_CHAINLOCKS_TO_RETAIN) { + recent_hashes.emplace_back(hash); + } + BOOST_CHECK_LE(m_node.clhandler->SeenChainLockCacheSizeForTesting(), MAX_SEEN_CHAINLOCKS); } - 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. + for (const auto& hash : recent_hashes) { + BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, hash})); + } + SetMockTime(0s); } BOOST_FIXTURE_TEST_CASE(best_chainlock_is_already_have_after_seen_cache_eviction, TestingSetup) @@ -247,18 +246,13 @@ 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 prune_after_size = m_node.clhandler->SeenChainLockCachePruneAfterSizeForTesting(); - BOOST_REQUIRE_GT(prune_after_size, 0U); - // 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) { + for (size_t i = 0; i <= MAX_SEEN_CHAINLOCKS; ++i) { auto clsig = CreateChainLock(static_cast(101 + i), GetTestBlockHash(static_cast(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(), prune_after_size); + BOOST_CHECK_LE(m_node.clhandler->SeenChainLockCacheSizeForTesting(), MAX_SEEN_CHAINLOCKS); } - BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(), - m_node.clhandler->SeenChainLockCacheRetainedSizeForTesting()); BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, best_hash})); }