Skip to content

Commit 38d9364

Browse files
fix(llmq): purge pending recovered sigs in BanNode
Widen BanNode to drop CSigningManager::pendingRecoveredSigs for the banned node, and route the two raw recsig PeerMisbehaving(100) sites (invalid QSIGREC llmqType, bad BLS batch source) through BanNode so every NetSigning score-100 path purges eagerly like shares already do via MarkAsBanned. Remove the 5s PeerIsBanned pending-recsig sweep entirely: that predicate was a ~100ms one-shot and never reliably ran. Queue reclaim for banned peers is now eager on BanNode. Caps from #7402 remain the bound for peers that disconnect without misbehavior. Keep m_sig_manager.Cleanup() for DB age.
1 parent 5c44379 commit 38d9364

2 files changed

Lines changed: 7 additions & 8 deletions

File tree

‎src/llmq/net_signing.cpp‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ void NetSigning::ProcessMessage(CNode& pfrom, const std::string& msg_type, CData
5454
recoveredSig->GetHash()}));
5555

5656
if (!Params().GetLLMQ(recoveredSig->getLlmqType()).has_value()) {
57-
m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100);
57+
BanNode(pfrom.GetId());
5858
return;
5959
}
6060

@@ -264,7 +264,7 @@ bool NetSigning::ProcessPendingRecoveredSigs()
264264
for (const auto& [nodeId, v] : recSigsByNode) {
265265
if (batchVerifier.badSources.count(nodeId)) {
266266
LogPrint(BCLog::LLMQ, "NetSigning::%s -- invalid recSig from other node, banning peer=%d\n", __func__, nodeId);
267-
m_peer_manager->PeerMisbehaving(nodeId, 100);
267+
BanNode(nodeId);
268268
continue;
269269
}
270270

@@ -288,10 +288,6 @@ void NetSigning::WorkThreadSigning()
288288
constexpr auto CLEANUP_INTERVAL{5s};
289289
if (cleanupThrottler.TryCleanup(CLEANUP_INTERVAL)) {
290290
m_sig_manager.Cleanup();
291-
// Drop pending recovered sigs queued by banned peers so a flood's backlog does not
292-
// persist after the peer is banned (RemoveBannedNodeStates only cleans the sig-shares
293-
// subsystem, not m_sig_manager's pending recovered sigs).
294-
m_sig_manager.RemoveNodesIf([this](NodeId node_id) { return m_peer_manager->PeerIsBanned(node_id); });
295291
}
296292

297293
// TODO Wakeup when pending signing is needed?
@@ -313,6 +309,9 @@ void NetSigning::BanNode(NodeId nodeId)
313309
if (nodeId == -1) return;
314310

315311
m_peer_manager->PeerMisbehaving(nodeId, 100);
312+
// Drop any not-yet-verified recovered sigs still queued for this peer so a flood's backlog
313+
// does not keep burning the single recsig worker after we have already decided to ban.
314+
m_sig_manager.RemoveNodesIf([nodeId](NodeId id) { return id == nodeId; });
316315
if (m_shares_manager) {
317316
m_shares_manager->MarkAsBanned(nodeId);
318317
}

‎src/llmq/signing.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,8 @@ class CSigningManager
217217
size_t maxUniqueSessions, std::unordered_map<NodeId, std::list<std::shared_ptr<const CRecoveredSig>>>& retSigShares,
218218
std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CBLSPublicKey, StaticSaltedHasher>& ret_pubkeys)
219219
EXCLUSIVE_LOCKS_REQUIRED(!cs_pending);
220-
// Drop the pending (not-yet-verified) recovered sigs of any node matching the predicate, e.g.
221-
// banned peers. Without this, a flooded peer's backlog would persist even after it is banned.
220+
// Drop pending (not-yet-verified) recovered sigs for matching node ids (used by BanNode).
221+
// Does not touch pendingReconstructedRecoveredSigs (local, node id -1).
222222
void RemoveNodesIf(const std::function<bool(NodeId)>& predicate) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending);
223223
[[nodiscard]] std::vector<CRecoveredSigsListener*> GetListeners() const EXCLUSIVE_LOCKS_REQUIRED(!cs_listeners);
224224
// Returns true if recovered sigs should be send to listeners

0 commit comments

Comments
 (0)