From a7c392a83d7fa190e1f5df6fca395d19524aa44b Mon Sep 17 00:00:00 2001 From: Navid Rahimi Date: Mon, 24 Aug 2026 04:11:47 +0000 Subject: [PATCH] LLMQ: Bound pending signature shares Limit unverified pending shares per peer and globally. Keep signature-share size accounting constant-time. --- src/llmq/quorums_signing_shares.cpp | 28 ++- src/llmq/quorums_signing_shares.h | 168 ++++++++++++----- src/test/CMakeLists.txt | 1 + src/test/quorums_signing_pending_tests.cpp | 203 +++++++++++++++++++++ 4 files changed, 353 insertions(+), 47 deletions(-) create mode 100644 src/test/quorums_signing_pending_tests.cpp diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 0fb2b0fc1d..b4587c566f 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -538,11 +538,37 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(CNode* pfrom, const CBatc } auto& nodeState = it->second; for (auto& s : sigShares) { - nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); + TryAddPendingIncomingSigShare(pfrom->id, nodeState, s); } return true; } +bool CSigSharesManager::TryAddPendingIncomingSigShare(NodeId nodeId, CSigSharesNodeState& nodeState, const CSigShare& sigShare) +{ + AssertLockHeld(cs); + + if (nodeState.banned) { + return false; + } + if (nodeState.pendingIncomingSigShares.Size() >= MAX_PENDING_SIG_SHARES_PER_NODE) { + LogPrint("llmq-sigs", "CSigSharesManager::%s -- per-node pending sig shares cap reached (%d), dropping sigShare. node=%d\n", + __func__, MAX_PENDING_SIG_SHARES_PER_NODE, nodeId); + return false; + } + + size_t total{0}; + for (const auto& p : nodeStates) { + total += p.second.pendingIncomingSigShares.Size(); + } + if (total >= MAX_PENDING_SIG_SHARES_TOTAL) { + LogPrint("llmq-sigs", "CSigSharesManager::%s -- global pending sig shares cap reached (%d), dropping sigShare. node=%d\n", + __func__, MAX_PENDING_SIG_SHARES_TOTAL, nodeId); + return false; + } + + return nodeState.pendingIncomingSigShares.Add(sigShare.GetKey(), sigShare); +} + bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan) { retBan = false; diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index 53e0e1ab48..7ac68bdca7 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/src/llmq/quorums_signing_shares.h @@ -27,6 +27,8 @@ class CScheduler; namespace llmq { +struct CSigSharesManagerTestAccess; + // typedef std::pair SigShareKey; @@ -138,40 +140,125 @@ class CBatchedSigShares std::string ToInvString() const; }; -template -class SigShareMap +/** + * Two-level (signHash -> quorumMember) map with a running entry count, so Size() is O(1) + * instead of a fold over all sign hash buckets. All structural mutations go through the + * counted methods; Buckets() is for lookups and in-place value updates only. + */ +template +class CountedBucketMap { +public: + using BucketMap = std::unordered_map, StaticSaltedHasher>; + private: - std::unordered_map, StaticSaltedHasher> internalMap; + BucketMap m_data; + size_t m_num_entries{0}; public: - bool Add(const SigShareKey& k, const T& v) + BucketMap& Buckets() { - auto& m = internalMap[k.first]; - return m.emplace(k.second, v).second; + return m_data; + } + + const BucketMap& Buckets() const + { + return m_data; + } + + size_t Size() const + { + return m_num_entries; + } + + bool Emplace(const SigShareKey& k, const T& v) + { + if (!m_data[k.first].emplace(k.second, v).second) { + return false; + } + ++m_num_entries; + return true; } void Erase(const SigShareKey& k) { - auto it = internalMap.find(k.first); - if (it == internalMap.end()) { + auto it = m_data.find(k.first); + if (it == m_data.end()) { return; } - it->second.erase(k.second); + m_num_entries -= it->second.erase(k.second); if (it->second.empty()) { - internalMap.erase(it); + m_data.erase(it); + } + } + + void EraseBucket(const uint256& signHash) + { + auto it = m_data.find(signHash); + if (it == m_data.end()) { + return; + } + m_num_entries -= it->second.size(); + m_data.erase(it); + } + + template + void EraseIf(F&& f) + { + for (auto it = m_data.begin(); it != m_data.end();) { + SigShareKey k; + k.first = it->first; + for (auto jt = it->second.begin(); jt != it->second.end();) { + k.second = jt->first; + if (f(k, jt->second)) { + jt = it->second.erase(jt); + --m_num_entries; + } else { + ++jt; + } + } + if (it->second.empty()) { + it = m_data.erase(it); + } else { + ++it; + } } } void Clear() { - internalMap.clear(); + m_data.clear(); + m_num_entries = 0; + } +}; + +template +class SigShareMap +{ +private: + CountedBucketMap internalMap; + +public: + bool Add(const SigShareKey& k, const T& v) + { + return internalMap.Emplace(k, v); + } + + void Erase(const SigShareKey& k) + { + internalMap.Erase(k); + } + + void Clear() + { + internalMap.Clear(); } bool Has(const SigShareKey& k) const { - auto it = internalMap.find(k.first); - if (it == internalMap.end()) { + const auto& buckets = internalMap.Buckets(); + auto it = buckets.find(k.first); + if (it == buckets.end()) { return false; } return it->second.count(k.second) != 0; @@ -179,8 +266,9 @@ class SigShareMap T* Get(const SigShareKey& k) { - auto it = internalMap.find(k.first); - if (it == internalMap.end()) { + auto& buckets = internalMap.Buckets(); + auto it = buckets.find(k.first); + if (it == buckets.end()) { return nullptr; } @@ -204,25 +292,23 @@ class SigShareMap const T* GetFirst() const { - if (internalMap.empty()) { + const auto& buckets = internalMap.Buckets(); + if (buckets.empty()) { return nullptr; } - return &internalMap.begin()->second.begin()->second; + return &buckets.begin()->second.begin()->second; } size_t Size() const { - size_t s = 0; - for (auto& p : internalMap) { - s += p.second.size(); - } - return s; + return internalMap.Size(); } size_t CountForSignHash(const uint256& signHash) const { - auto it = internalMap.find(signHash); - if (it == internalMap.end()) { + const auto& buckets = internalMap.Buckets(); + auto it = buckets.find(signHash); + if (it == buckets.end()) { return 0; } return it->second.size(); @@ -230,13 +316,14 @@ class SigShareMap bool Empty() const { - return internalMap.empty(); + return internalMap.Buckets().empty(); } const std::unordered_map* GetAllForSignHash(const uint256& signHash) { - auto it = internalMap.find(signHash); - if (it == internalMap.end()) { + const auto& buckets = internalMap.Buckets(); + auto it = buckets.find(signHash); + if (it == buckets.end()) { return nullptr; } return &it->second; @@ -244,35 +331,19 @@ class SigShareMap void EraseAllForSignHash(const uint256& signHash) { - internalMap.erase(signHash); + internalMap.EraseBucket(signHash); } template void EraseIf(F&& f) { - for (auto it = internalMap.begin(); it != internalMap.end(); ) { - SigShareKey k; - k.first = it->first; - for (auto jt = it->second.begin(); jt != it->second.end(); ) { - k.second = jt->first; - if (f(k, jt->second)) { - jt = it->second.erase(jt); - } else { - ++jt; - } - } - if (it->second.empty()) { - it = internalMap.erase(it); - } else { - ++it; - } - } + internalMap.EraseIf(f); } template void ForEach(F&& f) { - for (auto& p : internalMap) { + for (auto& p : internalMap.Buckets()) { SigShareKey k; k.first = p.first; for (auto& p2 : p.second) { @@ -342,8 +413,12 @@ class CSigSharesNodeState class CSigSharesManager : public CRecoveredSigsListener { + friend struct CSigSharesManagerTestAccess; + static const int64_t SESSION_NEW_SHARES_TIMEOUT = 60; static const int64_t SIG_SHARE_REQUEST_TIMEOUT = 5; + static constexpr size_t MAX_PENDING_SIG_SHARES_PER_NODE{1000}; + static constexpr size_t MAX_PENDING_SIG_SHARES_TOTAL{10000}; // we try to keep total message size below 10k const size_t MAX_MSGS_CNT_QSIGSESANN = 100; @@ -421,6 +496,7 @@ class CSigSharesManager : public CRecoveredSigsListener private: bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo); CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, size_t idx); + bool TryAddPendingIncomingSigShare(NodeId nodeId, CSigSharesNodeState& nodeState, const CSigShare& sigShare); void Cleanup(); void RemoveSigSharesForSession(const uint256& signHash); diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 6408280c81..5183e06b25 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -94,6 +94,7 @@ add_executable(test_firo ${CMAKE_CURRENT_SOURCE_DIR}/evospork_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/evo_deterministicmns_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/evo_simplifiedmns_tests.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/quorums_signing_pending_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/progpow_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/bls_tests.cpp ${CMAKE_CURRENT_SOURCE_DIR}/sparkmessage_tests.cpp diff --git a/src/test/quorums_signing_pending_tests.cpp b/src/test/quorums_signing_pending_tests.cpp new file mode 100644 index 0000000000..88eab8502f --- /dev/null +++ b/src/test/quorums_signing_pending_tests.cpp @@ -0,0 +1,203 @@ +// 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_signing.h" +#include "llmq/quorums_signing_shares.h" +#include "test/test_bitcoin.h" + +#include + +namespace llmq +{ + +struct CSigSharesManagerTestAccess { + static bool AddPending(CSigSharesManager& manager, NodeId nodeId, const CSigShare& sigShare) + { + LOCK(manager.cs); + auto& nodeState = manager.nodeStates[nodeId]; + return manager.TryAddPendingIncomingSigShare(nodeId, nodeState, sigShare); + } + + static void ErasePending(CSigSharesManager& manager, NodeId nodeId, const SigShareKey& key) + { + LOCK(manager.cs); + manager.nodeStates.at(nodeId).pendingIncomingSigShares.Erase(key); + } + + static size_t PendingCount(CSigSharesManager& manager, NodeId nodeId) + { + LOCK(manager.cs); + return manager.nodeStates.at(nodeId).pendingIncomingSigShares.Size(); + } + + static size_t PendingTotal(CSigSharesManager& manager) + { + LOCK(manager.cs); + size_t total{0}; + for (const auto& p : manager.nodeStates) { + total += p.second.pendingIncomingSigShares.Size(); + } + return total; + } + + static bool IsBanned(CSigSharesManager& manager, NodeId nodeId) + { + LOCK(manager.cs); + return manager.nodeStates.at(nodeId).banned; + } + + static constexpr size_t MaxPerNode() + { + return CSigSharesManager::MAX_PENDING_SIG_SHARES_PER_NODE; + } + + static constexpr size_t MaxTotal() + { + return CSigSharesManager::MAX_PENDING_SIG_SHARES_TOTAL; + } +}; + +} // namespace llmq + +namespace +{ + +uint256 HashFromNonce(size_t nonce) +{ + return uint256S(strprintf("%064x", static_cast(nonce))); +} + +llmq::CSigShare MakeSigShare(size_t nonce, uint16_t quorumMember = 0) +{ + llmq::CSigShare sigShare; + sigShare.llmqType = Consensus::LLMQ_50_60; + sigShare.quorumHash = HashFromNonce(1); + sigShare.quorumMember = quorumMember; + sigShare.id = HashFromNonce(nonce + 2); + sigShare.msgHash = HashFromNonce(nonce + 3); + sigShare.UpdateKey(); + return sigShare; +} + +} // namespace + +BOOST_FIXTURE_TEST_SUITE(quorums_signing_pending_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(sig_share_map_size_tracks_mutations) +{ + llmq::SigShareMap sigShares; + const auto sigShare1 = MakeSigShare(1); + const auto sigShare2 = MakeSigShare(2); + + BOOST_CHECK(sigShares.Add(sigShare1.GetKey(), sigShare1)); + BOOST_CHECK(!sigShares.Add(sigShare1.GetKey(), sigShare1)); + BOOST_CHECK(sigShares.Add(sigShare2.GetKey(), sigShare2)); + BOOST_CHECK_EQUAL(sigShares.Size(), 2U); + + sigShares.Erase(sigShare1.GetKey()); + sigShares.Erase(sigShare1.GetKey()); + BOOST_CHECK_EQUAL(sigShares.Size(), 1U); + + sigShares.EraseAllForSignHash(sigShare2.GetSignHash()); + BOOST_CHECK(sigShares.Empty()); + BOOST_CHECK_EQUAL(sigShares.Size(), 0U); + + BOOST_CHECK(sigShares.Add(sigShare1.GetKey(), sigShare1)); + BOOST_CHECK(sigShares.Add(sigShare2.GetKey(), sigShare2)); + sigShares.EraseIf([&](const llmq::SigShareKey& key, const llmq::CSigShare&) { + return key == sigShare1.GetKey(); + }); + BOOST_CHECK_EQUAL(sigShares.Size(), 1U); + + sigShares.Clear(); + BOOST_CHECK(sigShares.Empty()); + BOOST_CHECK_EQUAL(sigShares.Size(), 0U); +} + +BOOST_AUTO_TEST_CASE(sig_share_map_bucket_erase_updates_size) +{ + llmq::SigShareMap sigShares; + const auto signHash = MakeSigShare(1).GetSignHash(); + + for (uint16_t member = 0; member < 5; ++member) { + const auto sigShare = MakeSigShare(1, member); + BOOST_CHECK(sigShare.GetSignHash() == signHash); + BOOST_CHECK(sigShares.Add(sigShare.GetKey(), sigShare)); + } + BOOST_CHECK_EQUAL(sigShares.Size(), 5U); + + sigShares.EraseAllForSignHash(signHash); + BOOST_CHECK(sigShares.Empty()); + BOOST_CHECK_EQUAL(sigShares.Size(), 0U); +} + +BOOST_AUTO_TEST_CASE(pending_sig_shares_session_removal_updates_count) +{ + llmq::CSigSharesNodeState nodeState; + const auto sigShare1 = MakeSigShare(1); + const auto sigShare2 = MakeSigShare(2); + + BOOST_CHECK(nodeState.pendingIncomingSigShares.Add(sigShare1.GetKey(), sigShare1)); + BOOST_CHECK(nodeState.pendingIncomingSigShares.Add(sigShare2.GetKey(), sigShare2)); + BOOST_CHECK_EQUAL(nodeState.pendingIncomingSigShares.Size(), 2U); + + nodeState.RemoveSession(sigShare1.GetSignHash()); + BOOST_CHECK_EQUAL(nodeState.pendingIncomingSigShares.Size(), 1U); + BOOST_CHECK(!nodeState.pendingIncomingSigShares.Has(sigShare1.GetKey())); + BOOST_CHECK(nodeState.pendingIncomingSigShares.Has(sigShare2.GetKey())); + + nodeState.RemoveSession(sigShare1.GetSignHash()); + nodeState.RemoveSession(MakeSigShare(3).GetSignHash()); + BOOST_CHECK_EQUAL(nodeState.pendingIncomingSigShares.Size(), 1U); +} + +BOOST_AUTO_TEST_CASE(pending_incoming_sig_shares_are_bounded) +{ + constexpr NodeId firstNode{1}; + const size_t maxPerNode = llmq::CSigSharesManagerTestAccess::MaxPerNode(); + const size_t maxTotal = llmq::CSigSharesManagerTestAccess::MaxTotal(); + + llmq::CSigSharesManager perNodeManager; + bool admittedAll{true}; + for (size_t i = 0; i < maxPerNode; ++i) { + admittedAll &= llmq::CSigSharesManagerTestAccess::AddPending(perNodeManager, firstNode, MakeSigShare(i)); + } + BOOST_REQUIRE(admittedAll); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingCount(perNodeManager, firstNode), maxPerNode); + + const auto excessPerNode = MakeSigShare(maxPerNode); + BOOST_CHECK(!llmq::CSigSharesManagerTestAccess::AddPending(perNodeManager, firstNode, excessPerNode)); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingCount(perNodeManager, firstNode), maxPerNode); + + const auto firstSigShare = MakeSigShare(0); + llmq::CSigSharesManagerTestAccess::ErasePending(perNodeManager, firstNode, firstSigShare.GetKey()); + BOOST_CHECK(llmq::CSigSharesManagerTestAccess::AddPending(perNodeManager, firstNode, excessPerNode)); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingCount(perNodeManager, firstNode), maxPerNode); + + perNodeManager.MarkNodeBanned(firstNode); + BOOST_CHECK(llmq::CSigSharesManagerTestAccess::IsBanned(perNodeManager, firstNode)); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingCount(perNodeManager, firstNode), 0U); + BOOST_CHECK(!llmq::CSigSharesManagerTestAccess::AddPending(perNodeManager, firstNode, MakeSigShare(maxPerNode + 1))); + + llmq::CSigSharesManager globalManager; + admittedAll = true; + for (size_t i = 0; i < maxTotal; ++i) { + const NodeId nodeId = static_cast(100 + i / maxPerNode); + admittedAll &= llmq::CSigSharesManagerTestAccess::AddPending(globalManager, nodeId, MakeSigShare(10000 + i)); + } + BOOST_REQUIRE(admittedAll); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingTotal(globalManager), maxTotal); + + constexpr NodeId excessNode{1000}; + const auto excessGlobal = MakeSigShare(10000 + maxTotal); + BOOST_CHECK(!llmq::CSigSharesManagerTestAccess::AddPending(globalManager, excessNode, excessGlobal)); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingTotal(globalManager), maxTotal); + + const auto globalFirstSigShare = MakeSigShare(10000); + llmq::CSigSharesManagerTestAccess::ErasePending(globalManager, 100, globalFirstSigShare.GetKey()); + BOOST_CHECK(llmq::CSigSharesManagerTestAccess::AddPending(globalManager, excessNode, excessGlobal)); + BOOST_CHECK_EQUAL(llmq::CSigSharesManagerTestAccess::PendingTotal(globalManager), maxTotal); +} + +BOOST_AUTO_TEST_SUITE_END()