diff --git a/src/chainlock/handler.cpp b/src/chainlock/handler.cpp index b62a92b3d39c..1bed836c12b5 100644 --- a/src/chainlock/handler.cpp +++ b/src/chainlock/handler.cpp @@ -46,7 +46,8 @@ ChainlockHandler::ChainlockHandler(chainlock::Chainlocks& chainlocks, Chainstate m_mn_sync{mn_sync}, scheduler{std::make_unique()}, scheduler_thread{ - std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))} + std::make_unique(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))}, + seenChainLocks{MAX_SEEN_CHAINLOCKS} { } @@ -70,9 +71,28 @@ void ChainlockHandler::Start() void ChainlockHandler::Stop() { scheduler->stop(); } bool ChainlockHandler::AlreadyHave(const CInv& inv) const +{ + { + LOCK(cs); + if (seenChainLocks.count(inv.hash) != 0) { + return true; + } + } + + chainlock::ChainLockSig clsig; + return m_chainlocks.GetChainLockByHash(inv.hash, clsig); +} + +size_t ChainlockHandler::SeenChainLockCacheSizeForTesting() const +{ + LOCK(cs); + return seenChainLocks.size(); +} + +size_t ChainlockHandler::SeenChainLockCacheMaxSizeForTesting() const { LOCK(cs); - return seenChainLocks.count(inv.hash) != 0; + return seenChainLocks.max_size(); } void ChainlockHandler::UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) @@ -91,13 +111,16 @@ MessageProcessingResult ChainlockHandler::ProcessNewChainLock(const NodeId from, { LOCK(cs); - if (!seenChainLocks.emplace(hash, GetTime()).second) { + if (seenChainLocks.count(hash) != 0) { return {}; } + seenChainLocks.insert({hash, GetTime()}); - // height is expect to check twice: preliminary (for optimization) and inside UpdateBestsChainlock (as mutex is not kept during validation) + // Height is checked twice: preliminary (for optimization) and inside + // UpdateBestChainlock, as this mutex is not kept during validation. if (clsig.getHeight() <= m_chainlocks.GetBestChainLockHeight()) { - // no need to process older/same CLSIGs + // Remember the hash so AlreadyHave() suppresses repeated requests + // for stale CLSIG announcements. return {}; } } @@ -295,7 +318,9 @@ void ChainlockHandler::Cleanup() LOCK(cs); for (auto it = seenChainLocks.begin(); it != seenChainLocks.end();) { if (GetTime() - it->second >= CLEANUP_SEEN_TIMEOUT) { - it = seenChainLocks.erase(it); + const auto hash = it->first; + ++it; + seenChainLocks.erase(hash); } else { ++it; } diff --git a/src/chainlock/handler.h b/src/chainlock/handler.h index 140325c007c6..53de74deb03c 100644 --- a/src/chainlock/handler.h +++ b/src/chainlock/handler.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_CHAINLOCK_HANDLER_H #define BITCOIN_CHAINLOCK_HANDLER_H +#include #include #include #include @@ -53,10 +54,12 @@ class ChainlockHandler final : public CValidationInterface std::atomic tryLockChainTipScheduled{false}; std::atomic isEnabled{false}; + static constexpr size_t MAX_SEEN_CHAINLOCKS{1024}; + const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs){nullptr}; Uint256HashMap txFirstSeenTime GUARDED_BY(cs); - std::map seenChainLocks GUARDED_BY(cs); + unordered_limitedmap seenChainLocks GUARDED_BY(cs); CleanupThrottler cleanupThrottler; @@ -73,6 +76,8 @@ 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); [[nodiscard]] MessageProcessingResult ProcessNewChainLock(NodeId from, const chainlock::ChainLockSig& clsig, const llmq::CQuorumManager& qman, diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index b718cba9a342..0a7c7357475c 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -9,6 +9,10 @@ #include #include +#include +#include +#include +#include #include @@ -16,7 +20,7 @@ using chainlock::ChainLockSig; using namespace llmq; using namespace llmq::testutils; -BOOST_FIXTURE_TEST_SUITE(llmq_chainlock_tests, BasicTestingSetup) +BOOST_AUTO_TEST_SUITE(llmq_chainlock_tests) BOOST_AUTO_TEST_CASE(chainlock_construction_test) { @@ -167,4 +171,65 @@ BOOST_AUTO_TEST_CASE(chainlock_malformed_data_test) } } +BOOST_FIXTURE_TEST_CASE(stale_chainlocks_are_remembered_for_duplicate_suppression, TestingSetup) +{ + m_node.clhandler->CheckActiveState(); + + auto best_clsig = CreateChainLock(100, GetTestBlockHash(1)); + BOOST_REQUIRE(m_node.chainlocks->UpdateBestChainlock(::SerializeHash(best_clsig), best_clsig, /*pindex=*/nullptr)); + + BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(), 0U); + + for (uint32_t i = 0; i < 10; ++i) { + auto stale_clsig = CreateChainLock(100, GetTestBlockHash(1000 + i)); + const auto hash = ::SerializeHash(stale_clsig); + + [[maybe_unused]] const auto result = + m_node.clhandler->ProcessNewChainLock(/*from=*/0, stale_clsig, *m_node.llmq_ctx->qman, hash); + + BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, hash})); + BOOST_CHECK_EQUAL(m_node.clhandler->SeenChainLockCacheSizeForTesting(), static_cast(i) + 1U); + } +} + +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) { + auto clsig = CreateChainLock(static_cast(i), GetTestBlockHash(static_cast(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); + } + } +} + +BOOST_FIXTURE_TEST_CASE(best_chainlock_is_already_have_after_seen_cache_eviction, TestingSetup) +{ + m_node.clhandler->CheckActiveState(); + + auto best_clsig = CreateChainLock(100, GetTestBlockHash(1)); + const auto best_hash = ::SerializeHash(best_clsig); + 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); + + for (size_t i = 0; i < max_size + 1; ++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(), max_size); + } + + BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, best_hash})); +} + BOOST_AUTO_TEST_SUITE_END()