Skip to content

LLMQ: Bound seen ChainLock cache #1933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
navidR wants to merge 1 commit into
base: master
Choose a base branch
from
+103 −6
Open

LLMQ: Bound seen ChainLock cache #1933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/llmq/quorums_chainlocks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ std::string CChainLockSig::ToString() const
}

CChainLocksHandler::CChainLocksHandler(CScheduler* _scheduler) :
scheduler(_scheduler)
scheduler(_scheduler),
seenChainLocks(MAX_SEEN_CHAINLOCKS)
{
}

Expand Down Expand Up @@ -61,7 +62,8 @@ void CChainLocksHandler::Stop()
bool CChainLocksHandler::AlreadyHave(const CInv& inv)
{
LOCK(cs);
return seenChainLocks.count(inv.hash) != 0;
return seenChainLocks.count(inv.hash) != 0 ||
(bestChainLock.nHeight != -1 && inv.hash == bestChainLockHash);
}

bool CChainLocksHandler::GetChainLockByHash(const uint256& hash, llmq::CChainLockSig& ret)
Expand Down Expand Up @@ -101,9 +103,10 @@ void CChainLocksHandler::ProcessNewChainLock(NodeId from, const llmq::CChainLock

{
LOCK(cs);
if (!seenChainLocks.emplace(hash, GetTimeMillis()).second) {
if (seenChainLocks.count(hash) != 0) {
return;
}
seenChainLocks.insert(std::make_pair(hash, GetTimeMillis()));

@codeant-ai codeant-ai Bot Aug 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Invalid CLSIGs consume cache entries before signature verification, allowing peers to evict valid entries and force repeated expensive BLS verification. [security]

Assessment: 🟠 Major · 🔁 Occurrence: Sometimes

Use CodeAnt Skill Fix in Cursor Fix in VSCode Claude

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/llmq/quorums_chainlocks.cpp
**Line:** 109:109
**Comment:**
	*Security: Invalid CLSIGs consume cache entries before signature verification, allowing peers to evict valid entries and force repeated expensive BLS verification.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎


if (bestChainLock.nHeight != -1 && clsig.nHeight <= bestChainLock.nHeight) {
// no need to process/relay older CLSIGs
Expand Down Expand Up @@ -687,7 +690,9 @@ void CChainLocksHandler::Cleanup()

for (auto it = seenChainLocks.begin(); it != seenChainLocks.end(); ) {
if (GetTimeMillis() - it->second >= CLEANUP_SEEN_TIMEOUT) {
it = seenChainLocks.erase(it);
const uint256 hash = it->first;
++it;
seenChainLocks.erase(hash);
} else {
++it;
}
Expand Down Expand Up @@ -735,4 +740,3 @@ bool IsChainlocksEnabled()
if (!llmq::chainLocksHandler) return false;
return llmq::chainLocksHandler->IsChainlocksEnabled();
}

8 changes: 7 additions & 1 deletion src/llmq/quorums_chainlocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "llmq/quorums.h"
#include "llmq/quorums_signing.h"

#include "limitedmap.h"
#include "net.h"
#include "chainparams.h"

Expand All @@ -20,6 +21,8 @@ class CScheduler;
namespace llmq
{

struct CChainLocksHandlerTestAccess;

class CChainLockSig
{
public:
Expand All @@ -43,8 +46,11 @@ class CChainLockSig

class CChainLocksHandler : public CRecoveredSigsListener
{
friend struct CChainLocksHandlerTestAccess;

static const int64_t CLEANUP_INTERVAL = 1000 * 30;
static const int64_t CLEANUP_SEEN_TIMEOUT = 24 * 60 * 60 * 1000;
static constexpr size_t MAX_SEEN_CHAINLOCKS{1024};

// how long to wait for ixlocks until we consider a block with non-ixlocked TXs to be safe to sign
static const int64_t WAIT_FOR_ISLOCK_TIMEOUT = 10 * 60;
Expand Down Expand Up @@ -72,7 +78,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
BlockTxs blockTxs;
std::unordered_map<uint256, int64_t> txFirstSeenTime;

std::map<uint256, int64_t> seenChainLocks;
limitedmap<uint256, int64_t> seenChainLocks;

int64_t lastCleanupTime{0};

Expand Down
1 change: 1 addition & 0 deletions src/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ add_executable(test_firo
${CMAKE_CURRENT_SOURCE_DIR}/key_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/dbwrapper_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/limitedmap_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/llmq_chainlock_cache_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/logging_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/llmq_signing_shares_tests.cpp
${CMAKE_CURRENT_SOURCE_DIR}/main_tests.cpp
Expand Down
86 changes: 86 additions & 0 deletions src/test/llmq_chainlock_cache_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// Copyright (c) 2026 The Firo developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include "llmq/quorums_chainlocks.h"
#include "test/test_bitcoin.h"

#include <boost/test/unit_test.hpp>

#include <vector>

namespace llmq
{

struct CChainLocksHandlerTestAccess
{
static size_t SeenCount(CChainLocksHandler& handler)
{
LOCK(handler.cs);
return handler.seenChainLocks.size();
}

static bool SetSeenTime(CChainLocksHandler& handler, const uint256& hash, int64_t time)
{
LOCK(handler.cs);
auto it = handler.seenChainLocks.find(hash);
if (it == handler.seenChainLocks.end()) {
return false;
}
handler.seenChainLocks.update(it, time);
return true;
}

static void SetBest(CChainLocksHandler& handler, const CChainLockSig& clsig, const uint256& hash)
{
LOCK(handler.cs);
handler.bestChainLock = clsig;
handler.bestChainLockHash = hash;
}
};

} // namespace llmq

BOOST_FIXTURE_TEST_SUITE(llmq_chainlock_cache_tests, TestingSetup)

BOOST_AUTO_TEST_CASE(stale_chainlock_cache_is_bounded)
{
constexpr size_t MAX_SEEN_CHAINLOCKS{1024};

llmq::CChainLockSig bestChainLock;
bestChainLock.nHeight = 100;
bestChainLock.blockHash = uint256S("1");
const uint256 bestHash = ::SerializeHash(bestChainLock);
llmq::CChainLocksHandlerTestAccess::SetBest(*llmq::chainLocksHandler, bestChainLock, bestHash);

BOOST_CHECK(llmq::chainLocksHandler->AlreadyHave(CInv(MSG_CLSIG, bestHash)));

uint256 firstStaleHash;
std::vector<uint256> recentStaleHashes;
for (size_t i = 0; i <= MAX_SEEN_CHAINLOCKS; ++i) {
llmq::CChainLockSig staleChainLock;
staleChainLock.nHeight = bestChainLock.nHeight;
staleChainLock.blockHash = uint256S(strprintf("%064x", static_cast<unsigned int>(i + 2)));
const uint256 hash = ::SerializeHash(staleChainLock);
if (i == 0) {
firstStaleHash = hash;
}
if (i >= MAX_SEEN_CHAINLOCKS - 1) {
recentStaleHashes.emplace_back(hash);
}

llmq::chainLocksHandler->ProcessNewChainLock(-1, staleChainLock, hash);
BOOST_REQUIRE(llmq::CChainLocksHandlerTestAccess::SetSeenTime(
*llmq::chainLocksHandler, hash, static_cast<int64_t>(i)));
BOOST_CHECK_LE(llmq::CChainLocksHandlerTestAccess::SeenCount(*llmq::chainLocksHandler), MAX_SEEN_CHAINLOCKS);
}

BOOST_CHECK_EQUAL(llmq::CChainLocksHandlerTestAccess::SeenCount(*llmq::chainLocksHandler), MAX_SEEN_CHAINLOCKS);
BOOST_CHECK(!llmq::chainLocksHandler->AlreadyHave(CInv(MSG_CLSIG, firstStaleHash)));
for (const uint256& hash : recentStaleHashes) {
BOOST_CHECK(llmq::chainLocksHandler->AlreadyHave(CInv(MSG_CLSIG, hash)));
}
BOOST_CHECK(llmq::chainLocksHandler->AlreadyHave(CInv(MSG_CLSIG, bestHash)));
}

BOOST_AUTO_TEST_SUITE_END()
Loading