Skip to content

fix: limit signing share sessions per peer #7351

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
thepastaclaw wants to merge 5 commits into
base: develop
Choose a base branch
from
+135 −2
Open

fix: limit signing share sessions per peer #7351

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5e8d1e
fix: limit signing share sessions per peer
thepastaclaw Jun 8, 2026
135f8ce
fix: refine signing share session cap
thepastaclaw Jun 9, 2026
6595948
test: avoid signing share Makefile conflict
thepastaclaw Jun 9, 2026
f6aa12a
fix: preserve signing share session expiry on refresh
thepastaclaw Jun 9, 2026
03dddee
fix: count only announced signing share sessions
thepastaclaw Jun 10, 2026
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
41 changes: 40 additions & 1 deletion src/llmq/signing_shares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,21 @@

#include <cxxtimer.hpp>

#include <algorithm>
#include <ranges>

namespace llmq
{
namespace {
constexpr size_t MAX_SESSIONS_PER_PEER_FACTOR{4};
constexpr size_t MIN_SESSIONS_PER_PEER{100};

size_t GetMaxSessionsForPeer(const Consensus::LLMQParams& params)
{
return std::max<size_t>(size_t(params.size) * MAX_SESSIONS_PER_PEER_FACTOR, MIN_SESSIONS_PER_PEER);
}
} // namespace

void CSigShare::UpdateKey()
{
key.first = this->buildSignHash().Get();
Expand Down Expand Up @@ -133,6 +144,21 @@ CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromAnn(con
return s;
}

bool CSigSharesNodeState::CanCreateSessionFromAnn(const llmq::CSigSesAnn& ann, size_t maxSessions) const
{
return sessions.count(ann.buildSignHash().Get()) != 0 || GetSessionCount(ann.getLlmqType()) < maxSessions;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 9, 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.

P2 Badge Count only announcement-created sessions in the cap

When this node already has maxSessions sessions for a peer that were created locally for outgoing traffic (for example CollectSigSharesToAnnounce calls GetOrCreateSessionFromShare for that peer before SendMessages assigns a send session), this count rejects the peer's first unrelated QSIGSESANN even though the new limit is meant to constrain sessions created from announcements. NetSigning::ProcessMessage treats this false return as a ban, so under a busy signing backlog an honest peer can be banned because our own send-side sessions filled the per-peer count; the cap should exclude send-only sessions or track announcement-created sessions separately.

Useful? React with 👍 / 👎.

@thepastaclaw thepastaclaw Jun 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 03dddee: the session cap now counts only sessions that were actually introduced by QSIGSESANN announcements. Send-only sessions created locally via sig shares no longer consume the announcement-session budget, while existing-session announcements are still accepted and then marked as announcement-backed.

Validation:

  • git diff --check
  • make -C src -j8 test/test_dash
  • src/test/test_dash --run_test=llmq_utils_tests --catch_system_errors=no

}

size_t CSigSharesNodeState::GetSessionCount() const
{
return sessions.size();
}

size_t CSigSharesNodeState::GetSessionCount(Consensus::LLMQType llmqType) const
{
return std::ranges::count_if(sessions, [&](const auto& kv) { return kv.second.llmqType == llmqType; });
}

CSigSharesNodeState::Session* CSigSharesNodeState::GetSessionBySignHash(const uint256& signHash)
{
auto it = sessions.find(signHash);
Expand Down Expand Up @@ -206,7 +232,8 @@ void CSigSharesManager::UnregisterRecoveryInterface()
bool CSigSharesManager::ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann)
{
auto llmqType = ann.getLlmqType();
if (!Params().GetLLMQ(llmqType).has_value()) {
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
if (!llmq_params_opt.has_value()) {
return false;
}
if (ann.getSessionId() == UNINITIALIZED_SESSION_ID || ann.getQuorumHash().IsNull() || ann.getId().IsNull() || ann.getMsgHash().IsNull()) {
Expand All @@ -225,7 +252,14 @@ bool CSigSharesManager::ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSe

LOCK(cs);
auto& nodeState = nodeStates[pfrom.GetId()];
const size_t maxSessions = GetMaxSessionsForPeer(*llmq_params_opt);
if (!nodeState.CanCreateSessionFromAnn(ann, maxSessions)) {
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many sessions. cnt=%d, max=%d, llmqType=%d, node=%d\n",
__func__, nodeState.GetSessionCount(llmqType), maxSessions, static_cast<int>(llmqType), pfrom.GetId());
return false;
}
auto& session = nodeState.GetOrCreateSessionFromAnn(ann);
timeSeenForSessions.insert_or_assign(ann.buildSignHash().Get(), GetTime<std::chrono::seconds>().count());

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 9, 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.

P2 Badge Preserve announcement-only session expiry on refresh

When a peer periodically re-announces an already-known QSIGSESANN before SESSION_NEW_SHARES_TIMEOUT, insert_or_assign refreshes timeSeenForSessions, and cleanup only expires sessions by comparing the current time to that stored value. That lets announcement-only sessions stay alive indefinitely, so an attacker can keep its capped session slots and the corresponding per-peer state pinned forever; seed this timestamp only when the signHash is first inserted rather than on refresh.

Useful? React with 👍 / 👎.

@thepastaclaw thepastaclaw Jun 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in f6aa12a: QSIGSESANN announcements now seed timeSeenForSessions only on first insert (try_emplace) instead of refreshing it on repeat announcements. Actual sig-share receipt still refreshes the timestamp via ProcessSigShare, so announcement-only sessions expire normally.

nodeState.sessionByRecvId.erase(session.recvSessionId);
nodeState.sessionByRecvId.erase(ann.getSessionId());
session.recvSessionId = ann.getSessionId();
Expand Down Expand Up @@ -1247,6 +1281,11 @@ void CSigSharesManager::Cleanup()
doneSessions.emplace(sigShare.GetSignHash());
}
});
for (const auto& [signHash, _] : timeSeenForSessions) {
if (doneSessions.count(signHash) == 0 && sigman.HasRecoveredSigForSession(signHash)) {
doneSessions.emplace(signHash);
}
}
for (const auto& signHash : doneSessions) {
RemoveSigSharesForSession(signHash);
}
Expand Down
4 changes: 3 additions & 1 deletion src/llmq/signing_shares.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ class CSigSharesNodeState
CSigSharesInv requested;
CSigSharesInv knows;
};
// TODO limit number of sessions per node
Uint256HashMap<Session> sessions;

std::unordered_map<uint32_t, Session*> sessionByRecvId;
Expand All @@ -339,6 +338,9 @@ class CSigSharesNodeState

Session& GetOrCreateSessionFromShare(const CSigShare& sigShare);
Session& GetOrCreateSessionFromAnn(const CSigSesAnn& ann);
[[nodiscard]] bool CanCreateSessionFromAnn(const CSigSesAnn& ann, size_t maxSessions) const;
[[nodiscard]] size_t GetSessionCount() const;
[[nodiscard]] size_t GetSessionCount(Consensus::LLMQType llmqType) const;
Session* GetSessionBySignHash(const uint256& signHash);
Session* GetSessionByRecvId(uint32_t sessionId);
bool GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo);
Expand Down
52 changes: 52 additions & 0 deletions src/test/llmq_utils_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <consensus/params.h>
#include <llmq/params.h>
#include <llmq/signing_shares.h>
#include <llmq/utils.h>
#include <netaddress.h>
#include <random.h>
Expand All @@ -23,6 +24,57 @@ BOOST_FIXTURE_TEST_SUITE(llmq_utils_tests, BasicTestingSetup)

BOOST_AUTO_TEST_CASE(trivially_passes) { BOOST_CHECK(true); }

static CSigSesAnn MakeSigSesAnn(uint32_t session_id, uint32_t nonce, Consensus::LLMQType llmq_type = Consensus::LLMQType::LLMQ_50_60)
{
return CSigSesAnn{session_id, llmq_type, GetTestQuorumHash(1), GetTestQuorumHash(2), GetTestQuorumHash(nonce)};
}

BOOST_AUTO_TEST_CASE(sig_ses_ann_respects_session_limit_but_allows_refresh)
{
CSigSharesNodeState node_state;

const CSigSesAnn ann1{MakeSigSesAnn(1, 1)};
const CSigSesAnn ann2{MakeSigSesAnn(2, 2)};
const CSigSesAnn ann3{MakeSigSesAnn(3, 3)};
constexpr size_t max_sessions{2};

BOOST_CHECK(node_state.CanCreateSessionFromAnn(ann1, max_sessions));
node_state.GetOrCreateSessionFromAnn(ann1);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(), 1U);

BOOST_CHECK(node_state.CanCreateSessionFromAnn(ann2, max_sessions));
node_state.GetOrCreateSessionFromAnn(ann2);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(), max_sessions);

BOOST_CHECK(!node_state.CanCreateSessionFromAnn(ann3, max_sessions));

const CSigSesAnn ann1_refresh{4, Consensus::LLMQType::LLMQ_50_60, ann1.getQuorumHash(), ann1.getId(), ann1.getMsgHash()};
BOOST_CHECK(node_state.CanCreateSessionFromAnn(ann1_refresh, max_sessions));
node_state.GetOrCreateSessionFromAnn(ann1_refresh);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(), max_sessions);
}

BOOST_AUTO_TEST_CASE(sig_ses_ann_limit_is_per_llmq_type)
{
CSigSharesNodeState node_state;

constexpr size_t max_sessions{1};
const CSigSesAnn ann1{MakeSigSesAnn(1, 1)};
const CSigSesAnn ann2{MakeSigSesAnn(2, 2)};
const CSigSesAnn other_type_ann{MakeSigSesAnn(3, 3, Consensus::LLMQType::LLMQ_400_60)};

BOOST_CHECK(node_state.CanCreateSessionFromAnn(ann1, max_sessions));
node_state.GetOrCreateSessionFromAnn(ann1);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(), 1U);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(Consensus::LLMQType::LLMQ_50_60), 1U);

BOOST_CHECK(!node_state.CanCreateSessionFromAnn(ann2, max_sessions));
BOOST_CHECK(node_state.CanCreateSessionFromAnn(other_type_ann, max_sessions));
node_state.GetOrCreateSessionFromAnn(other_type_ann);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(), 2U);
BOOST_CHECK_EQUAL(node_state.GetSessionCount(Consensus::LLMQType::LLMQ_400_60), 1U);
}

BOOST_AUTO_TEST_CASE(deterministic_outbound_connection_test)
{
// Test deterministic behavior
Expand Down
Loading