From 15ceee9146640981b25a242ac37826b060a34b21 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 01:17:25 -0600 Subject: [PATCH 1/6] feat(llmq): vote against DKG members that persistently do not sign commitments PoSe only punishes members that are not valid in a mined commitment. A member that sends its contributions but never signs the commitment stays valid and is never punished. During the complaint phase, count from mined commitments of the last 4032 blocks how many each member was valid in and how many it signed, ignoring those before its registration or last revival. A member that signed at most a quarter of at least six of them gets a bad vote, so with enough votes the existing PoSe penalty applies. The counts are derived from chain data up to the start of the DKG cycle only, so all members agree on them. The check is gated by SPORK_23 like the other bad votes. Co-Authored-By: Claude Opus 5.5 --- src/active/dkgsession.cpp | 45 ++++++++++- src/active/dkgsession.h | 6 +- src/active/dkgsessionhandler.cpp | 2 +- src/llmq/blockprocessor.cpp | 77 ++++++++++++++++++ src/llmq/blockprocessor.h | 27 +++++++ src/llmq/dkgsession.cpp | 3 +- src/llmq/dkgsession.h | 3 + .../feature_llmq_simplepose_signing.py | 80 +++++++++++++++++++ test/functional/test_framework/simplepose.py | 4 +- test/functional/test_runner.py | 2 + 10 files changed, 242 insertions(+), 7 deletions(-) create mode 100755 test/functional/feature_llmq_simplepose_signing.py diff --git a/src/active/dkgsession.cpp b/src/active/dkgsession.cpp index 98e5422289d7..44c6dd3c3502 100644 --- a/src/active/dkgsession.cpp +++ b/src/active/dkgsession.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -23,13 +24,41 @@ #include namespace llmq { +namespace { +//! About a week of mainnet blocks +constexpr int SIGNING_HISTORY_WINDOW{4032}; +constexpr int SIGNING_HISTORY_MIN_COMMITMENTS{6}; + +//! Number of commitments a masternode was valid in since its last registration or revival, and how many it signed +std::pair CountSignedCommitments(const SigningRecords& records, const CDeterministicMNList& mn_list, + const uint256& proTxHash) +{ + auto it = records.find(proTxHash); + const auto dmn = mn_list.GetMN(proTxHash); + if (it == records.end() || !dmn) { + return {0, 0}; + } + const int cutoff{std::max(dmn->pdmnState->nRegisteredHeight, dmn->pdmnState->nPoSeRevivedHeight)}; + int valid{0}, signed_count{0}; + for (const auto& record : it->second) { + if (record.quorum_height > cutoff) { + ++valid; + signed_count += record.signed_commitment; + } + } + return {valid, signed_count}; +} +} // anonymous namespace + ActiveDKGSession::ActiveDKGSession(CBLSWorker& bls_worker, CDeterministicMNManager& dmnman, CDKGDebugManager& dkgdbgman, CDKGSessionManager& qdkgsman, CMasternodeMetaMan& mn_metaman, - CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager& mn_activeman, + const CQuorumBlockProcessor& qblockman, CQuorumSnapshotManager& qsnapman, + const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CSporkManager& sporkman, const CBlockIndex* base_block_index, const Consensus::LLMQParams& llmq_params) : CDKGSession(bls_worker, dmnman, dkgdbgman, qdkgsman, qsnapman, chainman, base_block_index, llmq_params), m_mn_metaman{mn_metaman}, + m_qblockman{qblockman}, m_mn_activeman{mn_activeman}, m_sporkman{sporkman}, m_use_legacy_bls{!DeploymentActiveAfter(m_quorum_base_block_index, Params().GetConsensus(), Consensus::DEPLOYMENT_V19)} @@ -285,6 +314,12 @@ void ActiveDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) co } }); + const int base_height{m_quorum_base_block_index->nHeight}; + const auto signing_records = m_qblockman.GetSigningRecords( + m_quorum_base_block_index->GetAncestor(base_height - base_height % params.dkgInterval), SIGNING_HISTORY_WINDOW); + // Members of a rotated quorum keep the state they had when their quarter was built + const auto mn_list = m_dmnman.GetListForBlock(m_quorum_base_block_index); + bool fShouldAllMembersBeConnected = IsAllMembersConnectedEnabled(params.type, m_sporkman); for (const auto& m : members) { if (m->dmn->proTxHash == myProTxHash) { @@ -315,6 +350,12 @@ void ActiveDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) co m->badConnection = true; logger.Batch("%s is Platform PoSe banned", m->dmn->proTxHash.ToString()); } + if (const auto [valid, signed_count] = CountSignedCommitments(*signing_records, mn_list, m->dmn->proTxHash); + valid >= SIGNING_HISTORY_MIN_COMMITMENTS && 4 * signed_count <= valid) { + m->badConnection = true; + logger.Batch("%s signed only %d of the %d recent commitments it was a valid member of", + m->dmn->proTxHash.ToString(), signed_count, valid); + } } } @@ -544,7 +585,7 @@ std::optional ActiveDKGSession::SendCommitment() return std::nullopt; } - if (ShouldSimulateError(DKGError::type::COMMIT_OMIT)) { + if (ShouldSimulateError(DKGError::type::COMMIT_OMIT) || ShouldSimulateError(DKGError::type::COMMIT_OMIT_ALL_TYPES)) { logger.Batch("omitting"); return std::nullopt; } diff --git a/src/active/dkgsession.h b/src/active/dkgsession.h index 6ea2f71a510d..61dedc427d82 100644 --- a/src/active/dkgsession.h +++ b/src/active/dkgsession.h @@ -14,10 +14,13 @@ class CSporkManager; class CMasternodeMetaMan; namespace llmq { +class CQuorumBlockProcessor; + class ActiveDKGSession final : public llmq::CDKGSession { private: CMasternodeMetaMan& m_mn_metaman; + const CQuorumBlockProcessor& m_qblockman; const CActiveMasternodeManager& m_mn_activeman; const CSporkManager& m_sporkman; const bool m_use_legacy_bls{false}; @@ -27,7 +30,8 @@ class ActiveDKGSession final : public llmq::CDKGSession ActiveDKGSession(const ActiveDKGSession&) = delete; ActiveDKGSession& operator=(const ActiveDKGSession&) = delete; ActiveDKGSession(CBLSWorker& bls_worker, CDeterministicMNManager& dmnman, CDKGDebugManager& dkgdbgman, - CDKGSessionManager& qdkgsman, CMasternodeMetaMan& mn_metaman, CQuorumSnapshotManager& qsnapman, + CDKGSessionManager& qdkgsman, CMasternodeMetaMan& mn_metaman, + const CQuorumBlockProcessor& qblockman, CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CSporkManager& sporkman, const CBlockIndex* base_block_index, const Consensus::LLMQParams& params); diff --git a/src/active/dkgsessionhandler.cpp b/src/active/dkgsessionhandler.cpp index 1d540275efda..7475abddfd3e 100644 --- a/src/active/dkgsessionhandler.cpp +++ b/src/active/dkgsessionhandler.cpp @@ -89,7 +89,7 @@ bool ActiveDKGSessionHandler::InitNewQuorum(gsl::not_null pQ } auto next_session = std::make_shared(m_bls_worker, m_dmnman, m_dkgdbgman, m_qdkgsman, - m_mn_metaman, m_qsnapman, m_mn_activeman, m_chainman, + m_mn_metaman, m_qblockman, m_qsnapman, m_mn_activeman, m_chainman, m_sporkman, pQuorumBaseBlockIndex, params); { diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 7a342c448646..955351c48e6b 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -27,6 +28,7 @@ #include #include +#include #include #include @@ -832,6 +834,81 @@ std::vector CQuorumBlockProcessor::GetMinedCommitmentsIndexe return ret; } +std::shared_ptr CQuorumBlockProcessor::GetValidMemberSigners( + Consensus::LLMQType llmqType, gsl::not_null pQuorumBaseBlockIndex) const +{ + const auto qc = GetMinedCommitment(llmqType, pQuorumBaseBlockIndex->GetBlockHash()).first; + if (qc.IsNull()) { + return nullptr; + } + const uint256 qc_hash{::SerializeHash(qc)}; + { + LOCK(m_signing_records_mutex); + if (auto it = m_valid_member_signers_cache.find(qc_hash); it != m_valid_member_signers_cache.end()) { + return it->second.second; + } + } + + const auto members = utils::GetAllQuorumMembers(llmqType, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}); + if (members.size() > qc.validMembers.size()) { + return nullptr; + } + auto signers = std::make_shared(); + for (size_t i = 0; i < members.size(); ++i) { + if (qc.validMembers[i]) { + signers->emplace_back(members[i]->proTxHash, qc.signers[i]); + } + } + + LOCK(m_signing_records_mutex); + m_valid_member_signers_cache.try_emplace(qc_hash, pQuorumBaseBlockIndex->nHeight, signers); + return signers; +} + +std::shared_ptr CQuorumBlockProcessor::GetSigningRecords(gsl::not_null pindex, + int window) const +{ + { + LOCK(m_signing_records_mutex); + if (std::shared_ptr cached; m_signing_records_cache.get(pindex->GetBlockHash(), cached)) { + return cached; + } + } + + const int min_height{pindex->nHeight - window}; + auto ret = std::make_shared(); + for (const auto& params : Params().GetConsensus().llmqs) { + if (params.is_single_member()) { + continue; + } + const bool rotation{IsQuorumRotationEnabled(params, pindex)}; + const size_t max_count{static_cast(window / params.dkgInterval + 1) * + static_cast(rotation ? params.signingActiveQuorumCount : 1)}; + const auto quorums{rotation ? GetMinedCommitmentsIndexedUntilBlock(params.type, pindex, max_count) + : GetMinedCommitmentsUntilBlock(params.type, pindex, max_count)}; + // Oldest first, so that the member lists of the active quorums stay in the shared caches + for (const auto* pQuorumBaseBlockIndex : quorums | std::views::reverse) { + if (pQuorumBaseBlockIndex->nHeight <= min_height) { + continue; + } + const auto signers = GetValidMemberSigners(params.type, pQuorumBaseBlockIndex); + if (!signers) { + continue; + } + for (const auto& [proTxHash, signed_commitment] : *signers) { + (*ret)[proTxHash].push_back({pQuorumBaseBlockIndex->nHeight, signed_commitment}); + } + } + } + + LOCK(m_signing_records_mutex); + // Sessions of types with a longer DKG interval gather at an older block + std::erase_if(m_valid_member_signers_cache, + [&](const auto& entry) { return entry.second.first <= min_height - window; }); + m_signing_records_cache.insert(pindex->GetBlockHash(), ret); + return ret; +} + // The returned quorums are in reversed order, so the most recent one is at index 0 std::map> CQuorumBlockProcessor::GetMinedAndActiveCommitmentsUntilBlock(gsl::not_null pindex) const { diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index f506a3c99deb..6126403de04f 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -47,6 +47,13 @@ using QcHashMap = std::map>; //! As above, but keyed by quorumIndex, for rotation-enabled types. using QcIndexedHashMap = std::map>; +//! Quorum base height of a mined commitment and whether a masternode that was valid in it also signed it. +struct SigningRecord { + int quorum_height; + bool signed_commitment; +}; +using SigningRecords = Uint256HashMap>; + /** Erase a mined commitment unless another chainstate still contains its block. */ bool EraseMinedCommitmentIfUnreferenced(CEvoDB& evo_db, const Chainstate& chainstate, gsl::not_null pindex, @@ -83,6 +90,15 @@ class CQuorumBlockProcessor mutable QcHashMap m_qc_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex); mutable QcIndexedHashMap m_qc_indexed_hashes_cached GUARDED_BY(m_qc_hashes_cache_mutex); + using ValidMemberSigners = std::vector>; + mutable Mutex m_signing_records_mutex; + //! By commitment hash, with the quorum base height + mutable Uint256HashMap>> m_valid_member_signers_cache + GUARDED_BY(m_signing_records_mutex); + //! By hash of the block the records were gathered at; DKG sessions of a cycle share it + mutable Uint256LruHashMap, 4> m_signing_records_cache + GUARDED_BY(m_signing_records_mutex); + //! A block's commitment payloads are needed by CheckSpecialTx, the masternode list update, //! ProcessBlock and the CbTx quorum merkle root to avoid duplicated decodes of BLS elements. mutable Mutex m_commitment_payload_mutex; @@ -155,7 +171,18 @@ class CQuorumBlockProcessor const CBlockIndex* pindex, size_t cycle) const; std::optional GetLastMinedCommitmentsByQuorumIndexUntilBlock(Consensus::LLMQType llmqType, const CBlockIndex* pindex, int quorumIndex, size_t cycle) const; + + /** + * Signing records of every masternode that was a valid member of a non-null commitment mined until pindex + * for a quorum based in (pindex->nHeight - window, pindex->nHeight]. + */ + std::shared_ptr GetSigningRecords(gsl::not_null pindex, int window) const + EXCLUSIVE_LOCKS_REQUIRED(!m_signing_records_mutex); private: + std::shared_ptr GetValidMemberSigners(Consensus::LLMQType llmqType, + gsl::not_null pQuorumBaseBlockIndex) const + EXCLUSIVE_LOCKS_REQUIRED(!m_signing_records_mutex); + //! Called from every site that writes or erases mined commitment state. void DropQcHashesCache() EXCLUSIVE_LOCKS_REQUIRED(!m_qc_hashes_cache_mutex); diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 0f511feb0d2b..bdc40e5d2059 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -54,7 +54,8 @@ double GetSimulatedErrorRate(DKGError::type type) bool CDKGSession::ShouldSimulateError(DKGError::type type) const { - if (params.type != Consensus::LLMQType::LLMQ_TEST) { + if (type == DKGError::type::COMMIT_OMIT_ALL_TYPES ? !Params().IsMockableChain() + : params.type != Consensus::LLMQType::LLMQ_TEST) { return false; } double rate = GetSimulatedErrorRate(type); diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index e116e55bcd16..320717591f00 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -71,6 +71,8 @@ class DKGError { JUSTIFY_LIE, CONTRIBUTION_INVALID, CONTRIBUTION_CLONE, + //! Like COMMIT_OMIT, but in the DKGs of every LLMQ type instead of only LLMQ_TEST + COMMIT_OMIT_ALL_TYPES, _COUNT }; static constexpr DKGError::type from_string(std::string_view in) { @@ -83,6 +85,7 @@ class DKGError { if (in == "justify-omit") return JUSTIFY_OMIT; if (in == "contribution-invalid") return CONTRIBUTION_INVALID; if (in == "contribution-clone") return CONTRIBUTION_CLONE; + if (in == "commit-omit-all-types") return COMMIT_OMIT_ALL_TYPES; return _COUNT; } }; diff --git a/test/functional/feature_llmq_simplepose_signing.py b/test/functional/feature_llmq_simplepose_signing.py new file mode 100755 index 000000000000..4c1af29f1dea --- /dev/null +++ b/test/functional/feature_llmq_simplepose_signing.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 The Dash Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +''' +feature_llmq_simplepose_signing.py + +Checks simple PoSe system based on LLMQ commitments: banning of masternodes +that stay valid quorum members but persistently do not sign the final +commitments. + +''' + +from test_framework.masternodes import check_banned, check_punished +from test_framework.simplepose import SimplePoSeMixin +from test_framework.test_framework import DashTestFramework +from test_framework.util import assert_equal + +NON_SIGNER_MSG = "recent commitments it was a valid member of" + + +class LLMQSimplePoSeSigningTest(SimplePoSeMixin, DashTestFramework): + def set_test_params(self): + self.set_simplepose_test_params() + + def run_test(self): + self.set_pose_sporks() + self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) + self.wait_for_sporks_same() + + mn = self.mninfo[0] + observer = self.mninfo[1].get_node(self) + self.reset_probe_timeouts() + self.mine_quorum() + + self.log.info("A masternode that misses signing a single commitment is not punished") + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) + self.mine_quorum_omitting_commitment() + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) + for _ in range(2): + self.reset_probe_timeouts() + with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): + self.mine_quorum() + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + + banned = not self.options.disable_spork23 + self.log.info(f"A masternode that never signs commitments is {'banned' if banned else 'not banned'}") + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) + rounds = 0 + with observer.assert_debug_log([f"{mn.proTxHash} signed only"] if banned else [], + unexpected_msgs=[] if banned else [NON_SIGNER_MSG]): + while rounds < 20 and not check_banned(self.nodes[0], mn): + rounds += 1 + self.log.info(f"Round {rounds} without a premature commitment") + self.mine_quorum_omitting_commitment() + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) + self.log.info(f"Rounds until banned: {rounds}") + assert_equal(check_banned(self.nodes[0], mn), banned) + if not banned: + assert not check_punished(self.nodes[0], mn) + return + + self.log.info("Reviving a masternode resets its signing history") + self.repair_masternodes(False) + self.reset_probe_timeouts() + with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): + self.mine_quorum() + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + + def mine_quorum_omitting_commitment(self): + self.reset_probe_timeouts() + members = len([m for m in self.mninfo if not check_banned(self.nodes[0], m)]) + self.mine_quorum_less_checks(members, self.mninfo, expected_commitments=members - 1) + + +if __name__ == '__main__': + LLMQSimplePoSeSigningTest().main() diff --git a/test/functional/test_framework/simplepose.py b/test/functional/test_framework/simplepose.py index 286c8d1b627d..3f575e66c097 100644 --- a/test/functional/test_framework/simplepose.py +++ b/test/functional/test_framework/simplepose.py @@ -40,7 +40,7 @@ def test_no_banning(self, invalidate_proc, expected_connections=None): self.log.info(f"Testing no PoSe banning in normal conditions {i + 1}/{iters}") self.mine_quorum(expected_connections=expected_connections) - def mine_quorum_less_checks(self, expected_members, mninfos_online): + def mine_quorum_less_checks(self, expected_members, mninfos_online, expected_commitments=None): # Unlike in mine_quorum we don't know how many complaints and justifications to expect: # the deaf masternode is a regular DKG participant until enough probes to it have failed, # so each phase waits for exactly the messages the members report having sent instead. @@ -78,7 +78,7 @@ def mine_quorum_less_checks(self, expected_members, mninfos_online): self.move_blocks(nodes, 2) self.log.info("Waiting for phase 5 (commit)") - self.wait_for_quorum_phase(q, 5, expected_members, "receivedPrematureCommitments", expected_members, mninfos_online, all_types=True) + self.wait_for_quorum_phase(q, 5, expected_members, "receivedPrematureCommitments", expected_members if expected_commitments is None else expected_commitments, mninfos_online, all_types=True) self.wait_for_offset_quorum_phase(q, 5, mninfos_online) self.move_blocks(nodes, 2) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index dbfe209d58a2..0b517bfdbc28 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -119,6 +119,7 @@ 'p2p_quorum_data.py', 'feature_llmq_simplepose.py', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose_capabilities.py', # NOTE: needs dash_hash to pass + 'feature_llmq_simplepose_signing.py', # NOTE: needs dash_hash to pass 'p2p_instantsend.py', 'feature_protx_version.py', 'feature_asset_locks.py', # NOTE: needs dash_hash to pass @@ -133,6 +134,7 @@ 'feature_llmq_signing.py --spork21', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose.py --disable-spork23', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose_capabilities.py --disable-spork23', # NOTE: needs dash_hash to pass + 'feature_llmq_simplepose_signing.py --disable-spork23', # NOTE: needs dash_hash to pass 'feature_llmq_rotation.py', # NOTE: needs dash_hash to pass 'feature_llmq_evo.py', # NOTE: needs dash_hash to pass 'feature_llmq_dkgerrors.py', # NOTE: needs dash_hash to pass From ca86d847f450da3c769f27c2026c14345ffb975e Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 13:25:37 -0600 Subject: [PATCH 2/6] test: fold the commitment-signing PoSe test into feature_llmq_simplepose_capabilities Move the non-signer scenarios into the existing capabilities test, which already starts from SPORK_21 on, an existing quorum and healthy masternodes, and drop the separate test file and its test_runner entries. Also drop comments that restate the code. Co-Authored-By: Claude Opus 5.5 --- src/active/dkgsession.cpp | 1 - src/llmq/blockprocessor.h | 1 - src/llmq/dkgsession.h | 1 - .../feature_llmq_simplepose_capabilities.py | 54 ++++++++++++- .../feature_llmq_simplepose_signing.py | 80 ------------------- test/functional/test_runner.py | 2 - 6 files changed, 53 insertions(+), 86 deletions(-) delete mode 100755 test/functional/feature_llmq_simplepose_signing.py diff --git a/src/active/dkgsession.cpp b/src/active/dkgsession.cpp index 44c6dd3c3502..6b6b3664f201 100644 --- a/src/active/dkgsession.cpp +++ b/src/active/dkgsession.cpp @@ -29,7 +29,6 @@ namespace { constexpr int SIGNING_HISTORY_WINDOW{4032}; constexpr int SIGNING_HISTORY_MIN_COMMITMENTS{6}; -//! Number of commitments a masternode was valid in since its last registration or revival, and how many it signed std::pair CountSignedCommitments(const SigningRecords& records, const CDeterministicMNList& mn_list, const uint256& proTxHash) { diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index 6126403de04f..60173fa69cb2 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -47,7 +47,6 @@ using QcHashMap = std::map>; //! As above, but keyed by quorumIndex, for rotation-enabled types. using QcIndexedHashMap = std::map>; -//! Quorum base height of a mined commitment and whether a masternode that was valid in it also signed it. struct SigningRecord { int quorum_height; bool signed_commitment; diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 320717591f00..44756760a170 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -71,7 +71,6 @@ class DKGError { JUSTIFY_LIE, CONTRIBUTION_INVALID, CONTRIBUTION_CLONE, - //! Like COMMIT_OMIT, but in the DKGs of every LLMQ type instead of only LLMQ_TEST COMMIT_OMIT_ALL_TYPES, _COUNT }; diff --git a/test/functional/feature_llmq_simplepose_capabilities.py b/test/functional/feature_llmq_simplepose_capabilities.py index 3f54a8592cc3..5f12a3942eae 100755 --- a/test/functional/feature_llmq_simplepose_capabilities.py +++ b/test/functional/feature_llmq_simplepose_capabilities.py @@ -7,7 +7,8 @@ feature_llmq_simplepose_capabilities.py Checks simple PoSe system based on LLMQ commitments: banning of masternodes -that run an outdated protocol or lack a required service. +that run an outdated protocol, lack a required service, or stay valid quorum +members but persistently do not sign the final commitments. ''' @@ -24,6 +25,8 @@ # See version.h MIN_MASTERNODE_PROTO_VERSION = 70242 +NON_SIGNER_MSG = "recent commitments it was a valid member of" + class MasternodePeer(P2PInterface): """Connects to a masternode the way another masternode does, advertising the given relay flag""" @@ -65,6 +68,8 @@ def run_test(self): self.repair_masternodes(True) self.reset_probe_timeouts() + self.test_non_signer() + def force_old_mn_proto(self, mn: MasternodeInfo): self.stop_node(mn.nodeIdx) self.start_masternode(mn, [f"-pushversion={MIN_MASTERNODE_PROTO_VERSION - 1}"]) @@ -181,6 +186,53 @@ def mine_quorum_no_service(self, mn, others, relay, services, expected_complaint assert not check_punished(self.nodes[0], m) assert not check_banned(self.nodes[0], m) + def test_non_signer(self): + mn = self.mninfo[0] + observer = self.mninfo[1].get_node(self) + self.reset_probe_timeouts() + self.mine_quorum() + + self.log.info("A masternode that misses signing a single commitment is not punished") + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) + self.mine_quorum_omitting_commitment() + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) + for _ in range(2): + self.reset_probe_timeouts() + with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): + self.mine_quorum() + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + + banned = not self.options.disable_spork23 + self.log.info(f"A masternode that never signs commitments is {'banned' if banned else 'not banned'}") + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) + rounds = 0 + with observer.assert_debug_log([f"{mn.proTxHash} signed only"] if banned else [], + unexpected_msgs=[] if banned else [NON_SIGNER_MSG]): + while rounds < 20 and not check_banned(self.nodes[0], mn): + rounds += 1 + self.log.info(f"Round {rounds} without a premature commitment") + self.mine_quorum_omitting_commitment() + mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) + self.log.info(f"Rounds until banned: {rounds}") + assert_equal(check_banned(self.nodes[0], mn), banned) + if not banned: + assert not check_punished(self.nodes[0], mn) + return + + self.log.info("Reviving a masternode resets its signing history") + self.repair_masternodes(False) + self.reset_probe_timeouts() + with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): + self.mine_quorum() + assert not check_punished(self.nodes[0], mn) + assert not check_banned(self.nodes[0], mn) + + def mine_quorum_omitting_commitment(self): + self.reset_probe_timeouts() + members = len([m for m in self.mninfo if not check_banned(self.nodes[0], m)]) + self.mine_quorum_less_checks(members, self.mninfo, expected_commitments=members - 1) + if __name__ == '__main__': LLMQSimplePoSeCapabilitiesTest().main() diff --git a/test/functional/feature_llmq_simplepose_signing.py b/test/functional/feature_llmq_simplepose_signing.py deleted file mode 100755 index 4c1af29f1dea..000000000000 --- a/test/functional/feature_llmq_simplepose_signing.py +++ /dev/null @@ -1,80 +0,0 @@ -#!/usr/bin/env python3 -# Copyright (c) 2026 The Dash Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. - -''' -feature_llmq_simplepose_signing.py - -Checks simple PoSe system based on LLMQ commitments: banning of masternodes -that stay valid quorum members but persistently do not sign the final -commitments. - -''' - -from test_framework.masternodes import check_banned, check_punished -from test_framework.simplepose import SimplePoSeMixin -from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal - -NON_SIGNER_MSG = "recent commitments it was a valid member of" - - -class LLMQSimplePoSeSigningTest(SimplePoSeMixin, DashTestFramework): - def set_test_params(self): - self.set_simplepose_test_params() - - def run_test(self): - self.set_pose_sporks() - self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) - self.wait_for_sporks_same() - - mn = self.mninfo[0] - observer = self.mninfo[1].get_node(self) - self.reset_probe_timeouts() - self.mine_quorum() - - self.log.info("A masternode that misses signing a single commitment is not punished") - mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) - self.mine_quorum_omitting_commitment() - mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) - for _ in range(2): - self.reset_probe_timeouts() - with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): - self.mine_quorum() - assert not check_punished(self.nodes[0], mn) - assert not check_banned(self.nodes[0], mn) - - banned = not self.options.disable_spork23 - self.log.info(f"A masternode that never signs commitments is {'banned' if banned else 'not banned'}") - mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 100) - rounds = 0 - with observer.assert_debug_log([f"{mn.proTxHash} signed only"] if banned else [], - unexpected_msgs=[] if banned else [NON_SIGNER_MSG]): - while rounds < 20 and not check_banned(self.nodes[0], mn): - rounds += 1 - self.log.info(f"Round {rounds} without a premature commitment") - self.mine_quorum_omitting_commitment() - mn.get_node(self).quorum('dkgsimerror', 'commit-omit-all-types', 0) - self.log.info(f"Rounds until banned: {rounds}") - assert_equal(check_banned(self.nodes[0], mn), banned) - if not banned: - assert not check_punished(self.nodes[0], mn) - return - - self.log.info("Reviving a masternode resets its signing history") - self.repair_masternodes(False) - self.reset_probe_timeouts() - with observer.assert_debug_log([], unexpected_msgs=[NON_SIGNER_MSG]): - self.mine_quorum() - assert not check_punished(self.nodes[0], mn) - assert not check_banned(self.nodes[0], mn) - - def mine_quorum_omitting_commitment(self): - self.reset_probe_timeouts() - members = len([m for m in self.mninfo if not check_banned(self.nodes[0], m)]) - self.mine_quorum_less_checks(members, self.mninfo, expected_commitments=members - 1) - - -if __name__ == '__main__': - LLMQSimplePoSeSigningTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 0b517bfdbc28..dbfe209d58a2 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -119,7 +119,6 @@ 'p2p_quorum_data.py', 'feature_llmq_simplepose.py', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose_capabilities.py', # NOTE: needs dash_hash to pass - 'feature_llmq_simplepose_signing.py', # NOTE: needs dash_hash to pass 'p2p_instantsend.py', 'feature_protx_version.py', 'feature_asset_locks.py', # NOTE: needs dash_hash to pass @@ -134,7 +133,6 @@ 'feature_llmq_signing.py --spork21', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose.py --disable-spork23', # NOTE: needs dash_hash to pass 'feature_llmq_simplepose_capabilities.py --disable-spork23', # NOTE: needs dash_hash to pass - 'feature_llmq_simplepose_signing.py --disable-spork23', # NOTE: needs dash_hash to pass 'feature_llmq_rotation.py', # NOTE: needs dash_hash to pass 'feature_llmq_evo.py', # NOTE: needs dash_hash to pass 'feature_llmq_dkgerrors.py', # NOTE: needs dash_hash to pass From 1789505d04d061ac3adce27cb92bbba1e6b4bf26 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 15:26:46 -0600 Subject: [PATCH 3/6] fix: also bound-check signers when collecting a commitment's valid-member signers Co-Authored-By: Claude Opus 5.5 --- src/llmq/blockprocessor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 955351c48e6b..9e7423455747 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -850,7 +850,7 @@ std::shared_ptr CQuorumBlockPro } const auto members = utils::GetAllQuorumMembers(llmqType, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}); - if (members.size() > qc.validMembers.size()) { + if (members.size() > qc.validMembers.size() || members.size() > qc.signers.size()) { return nullptr; } auto signers = std::make_shared(); From b98097e5b166b2302f44e470eef2d4891bd8d08f Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 18:48:32 -0600 Subject: [PATCH 4/6] test(llmq): cover commitment signing history regressions Verify that a valid truncated commitment cannot suppress a later richer commitment or manufacture non-signing history after it is mined. Cover window and mining-height bounds, quorum rotation, caches, masternode registration and revival cutoffs, and participation thresholds. Expose the existing count and pure participation predicate so unit tests exercise the same rules as ActiveDKGSession without private access. Co-Authored-By: GPT 6.1 Sol --- src/active/dkgsession.cpp | 9 +- src/active/dkgsession.h | 7 +- src/test/llmq_blockprocessor_tests.cpp | 228 +++++++++++++++++++++++++ src/test/llmq_dkg_tests.cpp | 57 +++++++ 4 files changed, 298 insertions(+), 3 deletions(-) diff --git a/src/active/dkgsession.cpp b/src/active/dkgsession.cpp index 6b6b3664f201..ba0dc70ce390 100644 --- a/src/active/dkgsession.cpp +++ b/src/active/dkgsession.cpp @@ -28,6 +28,7 @@ namespace { //! About a week of mainnet blocks constexpr int SIGNING_HISTORY_WINDOW{4032}; constexpr int SIGNING_HISTORY_MIN_COMMITMENTS{6}; +} // anonymous namespace std::pair CountSignedCommitments(const SigningRecords& records, const CDeterministicMNList& mn_list, const uint256& proTxHash) @@ -47,7 +48,11 @@ std::pair CountSignedCommitments(const SigningRecords& records, const } return {valid, signed_count}; } -} // anonymous namespace + +bool HasInsufficientSigningHistory(int valid, int signed_count) +{ + return valid >= SIGNING_HISTORY_MIN_COMMITMENTS && 4 * signed_count <= valid; +} ActiveDKGSession::ActiveDKGSession(CBLSWorker& bls_worker, CDeterministicMNManager& dmnman, CDKGDebugManager& dkgdbgman, CDKGSessionManager& qdkgsman, CMasternodeMetaMan& mn_metaman, @@ -350,7 +355,7 @@ void ActiveDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) co logger.Batch("%s is Platform PoSe banned", m->dmn->proTxHash.ToString()); } if (const auto [valid, signed_count] = CountSignedCommitments(*signing_records, mn_list, m->dmn->proTxHash); - valid >= SIGNING_HISTORY_MIN_COMMITMENTS && 4 * signed_count <= valid) { + HasInsufficientSigningHistory(valid, signed_count)) { m->badConnection = true; logger.Batch("%s signed only %d of the %d recent commitments it was a valid member of", m->dmn->proTxHash.ToString(), signed_count, valid); diff --git a/src/active/dkgsession.h b/src/active/dkgsession.h index 61dedc427d82..628a0867b997 100644 --- a/src/active/dkgsession.h +++ b/src/active/dkgsession.h @@ -5,16 +5,21 @@ #ifndef BITCOIN_ACTIVE_DKGSESSION_H #define BITCOIN_ACTIVE_DKGSESSION_H +#include #include #include +#include class CActiveMasternodeManager; +class CDeterministicMNList; class CSporkManager; class CMasternodeMetaMan; namespace llmq { -class CQuorumBlockProcessor; +std::pair CountSignedCommitments(const SigningRecords& records, const CDeterministicMNList& mn_list, + const uint256& proTxHash); +bool HasInsufficientSigningHistory(int valid, int signed_count); class ActiveDKGSession final : public llmq::CDKGSession { diff --git a/src/test/llmq_blockprocessor_tests.cpp b/src/test/llmq_blockprocessor_tests.cpp index f2f5907650ad..a2e48243f458 100644 --- a/src/test/llmq_blockprocessor_tests.cpp +++ b/src/test/llmq_blockprocessor_tests.cpp @@ -8,6 +8,8 @@ #include #include +#include +#include #include #include #include @@ -25,6 +27,9 @@ #include +#include +#include + using namespace llmq; using namespace llmq::testutils; @@ -37,6 +42,127 @@ CDataStream Serialized(const CFinalCommitment& qc) payload << qc; return payload; } + +struct SigningHistorySetup : TestChainV19Setup { + CDeterministicMNManager dmnman{*m_node.evodb, *m_node.mn_metaman}; + CQuorumBlockProcessor blockprocessor{*m_node.chainman, dmnman, *m_node.evodb, *m_node.llmq_ctx->qsnapman, 0}; + std::map operator_keys; + const bool legacy_scheme{bls::bls_legacy_scheme.load()}; + + SigningHistorySetup() + { + mineBlocks(132); + bls::bls_legacy_scheme.store(false); + CDeterministicMNList mn_list; + for (int i = 0; i < 8; ++i) { + auto mn = std::make_shared(i); + mn->proTxHash = GetTestQuorumHash(i + 1); + mn->collateralOutpoint = COutPoint(mn->proTxHash, 0); + auto state = std::make_shared(); + state->nVersion = ProTxVersion::BasicBLS; + state->netInfo = NetInfoInterface::MakeNetInfo(state->nVersion); + state->keyIDOwner.begin()[0] = i + 1; + state->nRegisteredHeight = 100; + state->UpdateConfirmedHash(mn->proTxHash, GetTestBlockHash(i + 1)); + auto& key = operator_keys[mn->proTxHash]; + key.MakeNewKey(); + state->pubKeyOperator.Set(key.GetPublicKey(), false); + mn->pdmnState = state; + mn_list.AddMN(mn); + } + // Read persisted lists with a fresh manager instead of replacing the node's cached lists. + for (int height = 100; height <= 240; ++height) { + mn_list.SetHeight(height); + mn_list.SetBlockHash(Index(height)->GetBlockHash()); + m_node.evodb->Write(std::make_pair(std::string{"dmn_S3"}, mn_list.GetBlockHash()), mn_list); + } + } + + ~SigningHistorySetup() { bls::bls_legacy_scheme.store(legacy_scheme); } + + const CBlockIndex* Index(int height) const + { + return WITH_LOCK(cs_main, return m_node.chainman->ActiveChain()[height]); + } + + std::vector Members(Consensus::LLMQType type, int height) + { + return utils::GetAllQuorumMembers(type, {dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, Index(height)}, + /*reset_cache=*/true); + } + + CFinalCommitment Commitment(Consensus::LLMQType type, int height, const std::vector& invalid_members = {}, + const std::vector& non_signers = {}) + { + const auto params = Params().GetLLMQ(type).value(); + const auto members = Members(type, height); + BOOST_REQUIRE_EQUAL(members.size(), static_cast(params.size)); + CFinalCommitment qc{params, Index(height)->GetBlockHash()}; + qc.nVersion = CFinalCommitment::GetVersion(IsQuorumRotationEnabled(params, Index(height)), true); + qc.quorumIndex = height % params.dkgInterval; + qc.validMembers.assign(params.size, true); + qc.signers.assign(params.size, true); + for (const auto i : invalid_members) + qc.validMembers[i] = false; + for (const auto i : non_signers) + qc.signers[i] = false; + qc.quorumVvecHash = GetRandHash(); + CBLSSecretKey quorum_key; + quorum_key.MakeNewKey(); + qc.quorumPublicKey = quorum_key.GetPublicKey(); + qc.quorumSig = quorum_key.Sign(BuildCommitmentHash(type, qc.quorumHash, qc.validMembers, qc.quorumPublicKey, + qc.quorumVvecHash), + false); + SignMembers(qc, members); + BOOST_REQUIRE(qc.Verify({dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, Index(height)}, + /*checkSigs=*/true)); + return qc; + } + + void SignMembers(CFinalCommitment& qc, const std::vector& members) + { + const auto hash = BuildCommitmentHash(qc.llmqType, qc.quorumHash, qc.validMembers, qc.quorumPublicKey, + qc.quorumVvecHash); + std::vector keys; + std::vector signatures; + for (size_t i = 0; i < members.size(); ++i) { + if (qc.signers[i]) { + const auto& key = operator_keys.at(members[i]->proTxHash); + keys.push_back(key.GetPublicKey()); + signatures.push_back(key.Sign(hash, false)); + } + } + qc.membersSig = CBLSSignature::AggregateSecure(signatures, keys, hash); + } + + void Store(const CFinalCommitment& qc, int mined_height, int base_height) + { + if (IsQuorumRotationEnabled(Params().GetLLMQ(qc.llmqType).value(), Index(base_height))) { + m_node.evodb->Write(std::make_pair(std::string{"q_mc"}, std::make_pair(qc.llmqType, qc.quorumHash)), + std::make_pair(qc, Index(mined_height)->GetBlockHash())); + m_node.evodb->Write(std::make_tuple(std::string{"q_mcihi"}, qc.llmqType, int{qc.quorumIndex}, + htobe32_internal(UINT32_MAX - mined_height)), + base_height); + } else { + WriteMinedCommitment(*m_node.evodb, qc, Index(mined_height)->GetBlockHash(), mined_height, base_height); + } + } +}; + +using NormalizedSigningRecords = std::map>>; + +NormalizedSigningRecords Normalize(const SigningRecords& records) +{ + NormalizedSigningRecords ret; + for (const auto& [protx, history] : records) { + auto& entries = ret[protx]; + for (const auto& record : history) { + entries.emplace_back(record.quorum_height, record.signed_commitment); + } + std::sort(entries.begin(), entries.end()); + } + return ret; +} } // namespace // A QFCOMMITMENT is only ever sent in reply to a GETDATA, so one the peer was never asked for must @@ -199,4 +325,106 @@ BOOST_FIXTURE_TEST_CASE(qfcommit_with_more_signers_replaces_stored_one, TestChai SetMockTime(0s); } +BOOST_FIXTURE_TEST_CASE(richer_commitment_preserves_member_signing_history, SigningHistorySetup) +{ + constexpr auto type{Consensus::LLMQType::LLMQ_TEST}; + const auto params = Params().GetLLMQ(type).value(); + const auto members = Members(type, 216); + const auto richer = Commitment(type, 216); + auto truncated = richer; + truncated.signers.back() = false; + SignMembers(truncated, members); + const UtilParameters util_params{dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, Index(216)}; + BOOST_REQUIRE(truncated.validMembers.back()); + BOOST_REQUIRE(truncated.Verify(util_params, /*checkSigs=*/true)); + BOOST_REQUIRE(richer.Verify(util_params, /*checkSigs=*/true)); + + auto peer = MakeTestPeer(71); + for (const auto& qc : {truncated, richer}) { + auto payload = Serialized(qc); + const auto result = blockprocessor.ProcessMessage(*peer, NetMsgType::QFCOMMITMENT, payload, + [](const CInv&) { return true; }); + BOOST_REQUIRE(!result.m_error.has_value()); + BOOST_CHECK_EQUAL(result.m_inventory.size(), 1U); + if (!result.m_inventory.empty()) { + BOOST_CHECK(result.m_inventory.front().hash == ::SerializeHash(qc)); + } + } + BOOST_CHECK(!blockprocessor.HasMineableCommitment(::SerializeHash(truncated))); + { + LOCK(cs_main); + const auto mineable = blockprocessor.GetMineableCommitments(params, 226); + BOOST_REQUIRE(mineable.has_value()); + BOOST_REQUIRE_EQUAL(mineable->size(), 1U); + BOOST_CHECK(::SerializeHash(mineable->front()) == ::SerializeHash(richer)); + + CBlock block; + for (const auto& enabled : GetEnabledQuorumParams(*m_node.chainman, Index(225))) { + blockprocessor.GetMineableCommitmentsTx(enabled, 226, block.vtx); + } + BlockValidationState state; + BOOST_REQUIRE_MESSAGE(blockprocessor.ProcessBlock(m_node.chainman->ActiveChainstate(), block, Index(226), state, + /*fJustCheck=*/false, /*fBLSChecks=*/true), + state.ToString()); + } + BOOST_CHECK(::SerializeHash(blockprocessor.GetMinedCommitment(type, richer.quorumHash).first) == + ::SerializeHash(richer)); + const auto records = blockprocessor.GetSigningRecords(Index(226), 4032); + const auto it = records->find(members.back()->proTxHash); + BOOST_REQUIRE(it != records->end()); + BOOST_REQUIRE_EQUAL(it->second.size(), 1U); + BOOST_CHECK_EQUAL(it->second.front().quorum_height, 216); + BOOST_CHECK(it->second.front().signed_commitment); +} + +BOOST_FIXTURE_TEST_CASE(signing_records_window_mining_height_rotation_and_cache, SigningHistorySetup) +{ + constexpr auto type{Consensus::LLMQType::LLMQ_TEST}; + constexpr auto rotated_type{Consensus::LLMQType::LLMQ_TEST_DIP0024}; + for (int height : {120, 144, 168}) { + Members(rotated_type, height); + } + BOOST_REQUIRE(IsQuorumRotationEnabled(Params().GetLLMQ(rotated_type).value(), Index(228))); + Store(Commitment(type, 192), 202, 192); + Store(Commitment(type, 216, /*invalid_members=*/{2}, /*non_signers=*/{1}), 226, 216); + Store(Commitment(rotated_type, 192), 204, 192); + Store(Commitment(rotated_type, 193), 205, 193); + Store(Commitment(rotated_type, 216), 228, 216); + Store(Commitment(rotated_type, 217), 229, 217); + + NormalizedSigningRecords expected; + const auto non_rotated_members = Members(type, 216); + const auto inside_members = Members(rotated_type, 193); + for (const auto& mn : inside_members) { + expected[mn->proTxHash].emplace_back(193, true); + } + const auto expected_before_non_rotated = expected; + expected[non_rotated_members[0]->proTxHash].emplace_back(216, true); + expected[non_rotated_members[1]->proTxHash].emplace_back(216, false); + const auto expected_before_mining = expected; + const auto rotated_members = Members(rotated_type, 216); + for (const auto& mn : rotated_members) { + expected[mn->proTxHash].emplace_back(216, true); + } + for (auto& [_, history] : expected) { + std::sort(history.begin(), history.end()); + } + + const auto cold = blockprocessor.GetSigningRecords(Index(228), 36); + BOOST_CHECK(Normalize(*cold) == expected); + const auto cached = blockprocessor.GetSigningRecords(Index(228), 36); + BOOST_CHECK(Normalize(*cached) == expected); + CQuorumBlockProcessor fresh{*m_node.chainman, dmnman, *m_node.evodb, *m_node.llmq_ctx->qsnapman, 0}; + BOOST_CHECK(Normalize(*fresh.GetSigningRecords(Index(228), 36)) == expected); + + BOOST_CHECK(Normalize(*blockprocessor.GetSigningRecords(Index(225), 33)) == expected_before_non_rotated); + const auto earlier = Normalize(*blockprocessor.GetSigningRecords(Index(227), 35)); + BOOST_CHECK(earlier == expected_before_mining); + const auto later = Normalize(*blockprocessor.GetSigningRecords(Index(229), 37)); + for (const auto& mn : Members(rotated_type, 217)) { + const auto& history = later.at(mn->proTxHash); + BOOST_CHECK(std::find(history.begin(), history.end(), std::make_pair(217, true)) != history.end()); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/llmq_dkg_tests.cpp b/src/test/llmq_dkg_tests.cpp index 6595cf457a02..1705e8ca2ec3 100644 --- a/src/test/llmq_dkg_tests.cpp +++ b/src/test/llmq_dkg_tests.cpp @@ -2,7 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include +#include #include #include #include @@ -155,4 +157,59 @@ BOOST_AUTO_TEST_CASE(contribution_records_vvec_scheme) bls::bls_legacy_scheme.store(legacy_scheme_before); } +BOOST_AUTO_TEST_CASE(signing_history_registration_and_revival_cutoffs) +{ + const auto protx = MakeTestHash(1); + const llmq::SigningRecords records{{protx, {{99, false}, {100, true}, {101, false}, {102, true}}}}; + const auto count = [&](int registered, int revived) { + auto state = std::make_shared(); + state->netInfo = NetInfoInterface::MakeNetInfo(state->nVersion); + state->nRegisteredHeight = registered; + state->nPoSeRevivedHeight = revived; + auto mn = std::make_shared(0); + mn->proTxHash = protx; + mn->collateralOutpoint = COutPoint(protx, 0); + state->keyIDOwner.begin()[0] = 1; + mn->pdmnState = state; + CDeterministicMNList mn_list; + mn_list.AddMN(mn); + BOOST_CHECK(llmq::CountSignedCommitments({}, mn_list, protx) == std::make_pair(0, 0)); + BOOST_CHECK(llmq::CountSignedCommitments(records, mn_list, MakeTestHash(2)) == std::make_pair(0, 0)); + return llmq::CountSignedCommitments(records, mn_list, protx); + }; + + BOOST_CHECK(count(99, -1) == std::make_pair(3, 2)); + BOOST_CHECK(count(100, -1) == std::make_pair(2, 1)); + BOOST_CHECK(count(100, 101) == std::make_pair(1, 1)); + BOOST_CHECK(count(101, 100) == std::make_pair(1, 1)); + BOOST_CHECK(count(102, 102) == std::make_pair(0, 0)); + BOOST_CHECK(llmq::CountSignedCommitments(records, {}, protx) == std::make_pair(0, 0)); +} + +BOOST_AUTO_TEST_CASE(signing_history_eligibility_and_quarter_threshold) +{ + struct TestCase { + int valid; + int signed_count; + bool insufficient; + }; + for (const auto& test : {TestCase{0, 0, false}, + {5, 0, false}, + {5, 1, false}, + {6, 0, true}, + {6, 1, true}, + {6, 2, false}, + {7, 2, false}, + {8, 2, true}, + {9, 2, true}, + {8, 3, false}, + {12, 3, true}, + {12, 4, false}}) { + BOOST_TEST_CONTEXT("valid=" << test.valid << ", signed=" << test.signed_count) + { + BOOST_CHECK_EQUAL(llmq::HasInsufficientSigningHistory(test.valid, test.signed_count), test.insufficient); + } + } +} + BOOST_AUTO_TEST_SUITE_END() From 99ea9483b51330db0771fbfb405728516c2dc91f Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 18:58:50 -0600 Subject: [PATCH 5/6] fix: key cached signing records by window as well as gathering block Co-Authored-By: Claude Opus 5.5 --- src/llmq/blockprocessor.cpp | 5 +++-- src/llmq/blockprocessor.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 9e7423455747..3f1c8069fa31 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -868,9 +868,10 @@ std::shared_ptr CQuorumBlockPro std::shared_ptr CQuorumBlockProcessor::GetSigningRecords(gsl::not_null pindex, int window) const { + const uint256 cache_key{::SerializeHash(std::make_pair(pindex->GetBlockHash(), window))}; { LOCK(m_signing_records_mutex); - if (std::shared_ptr cached; m_signing_records_cache.get(pindex->GetBlockHash(), cached)) { + if (std::shared_ptr cached; m_signing_records_cache.get(cache_key, cached)) { return cached; } } @@ -905,7 +906,7 @@ std::shared_ptr CQuorumBlockProcessor::GetSigningRecords(g // Sessions of types with a longer DKG interval gather at an older block std::erase_if(m_valid_member_signers_cache, [&](const auto& entry) { return entry.second.first <= min_height - window; }); - m_signing_records_cache.insert(pindex->GetBlockHash(), ret); + m_signing_records_cache.insert(cache_key, ret); return ret; } diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index 60173fa69cb2..c58f0235776b 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -94,7 +94,7 @@ class CQuorumBlockProcessor //! By commitment hash, with the quorum base height mutable Uint256HashMap>> m_valid_member_signers_cache GUARDED_BY(m_signing_records_mutex); - //! By hash of the block the records were gathered at; DKG sessions of a cycle share it + //! By hash of the gathering block and the window; DKG sessions of a cycle share it mutable Uint256LruHashMap, 4> m_signing_records_cache GUARDED_BY(m_signing_records_mutex); From d931e0c77d5864c0dfee9a7387da1c2a86f0f196 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 10 Nov 2026 20:45:34 -0600 Subject: [PATCH 6/6] fix(llmq): reject incomplete signing history views Return unavailable signing history instead of caching records from missing commitments or inconsistent members. Validate gathering-block membership, commitment ancestry, and unchanged quorum enumeration under cs_main after collecting members. Skip only the signing-history DKG check when history is unavailable, and cover disconnect/reconnect, incomplete views, cache recovery, and independent connection checks. Co-Authored-By: GPT 6.1 Sol --- src/active/dkgsession.cpp | 15 ++- src/llmq/blockprocessor.cpp | 55 +++++++--- src/llmq/blockprocessor.h | 5 +- src/test/llmq_blockprocessor_tests.cpp | 136 +++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 20 deletions(-) diff --git a/src/active/dkgsession.cpp b/src/active/dkgsession.cpp index ba0dc70ce390..7d75c55185fe 100644 --- a/src/active/dkgsession.cpp +++ b/src/active/dkgsession.cpp @@ -321,6 +321,9 @@ void ActiveDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) co const int base_height{m_quorum_base_block_index->nHeight}; const auto signing_records = m_qblockman.GetSigningRecords( m_quorum_base_block_index->GetAncestor(base_height - base_height % params.dkgInterval), SIGNING_HISTORY_WINDOW); + if (!signing_records) { + LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSession::%s -- signing history unavailable after chain change, skipping signing-history check\n", __func__); + } // Members of a rotated quorum keep the state they had when their quarter was built const auto mn_list = m_dmnman.GetListForBlock(m_quorum_base_block_index); @@ -354,11 +357,13 @@ void ActiveDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) co m->badConnection = true; logger.Batch("%s is Platform PoSe banned", m->dmn->proTxHash.ToString()); } - if (const auto [valid, signed_count] = CountSignedCommitments(*signing_records, mn_list, m->dmn->proTxHash); - HasInsufficientSigningHistory(valid, signed_count)) { - m->badConnection = true; - logger.Batch("%s signed only %d of the %d recent commitments it was a valid member of", - m->dmn->proTxHash.ToString(), signed_count, valid); + if (signing_records) { + if (const auto [valid, signed_count] = CountSignedCommitments(*signing_records, mn_list, m->dmn->proTxHash); + HasInsufficientSigningHistory(valid, signed_count)) { + m->badConnection = true; + logger.Batch("%s signed only %d of the %d recent commitments it was a valid member of", + m->dmn->proTxHash.ToString(), signed_count, valid); + } } } } diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 3f1c8069fa31..e12dd0fc37c3 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -835,12 +835,8 @@ std::vector CQuorumBlockProcessor::GetMinedCommitmentsIndexe } std::shared_ptr CQuorumBlockProcessor::GetValidMemberSigners( - Consensus::LLMQType llmqType, gsl::not_null pQuorumBaseBlockIndex) const + const CFinalCommitment& qc, gsl::not_null pQuorumBaseBlockIndex) const { - const auto qc = GetMinedCommitment(llmqType, pQuorumBaseBlockIndex->GetBlockHash()).first; - if (qc.IsNull()) { - return nullptr; - } const uint256 qc_hash{::SerializeHash(qc)}; { LOCK(m_signing_records_mutex); @@ -849,10 +845,14 @@ std::shared_ptr CQuorumBlockPro } } - const auto members = utils::GetAllQuorumMembers(llmqType, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}); + const auto members = utils::GetAllQuorumMembers(qc.llmqType, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}); if (members.size() > qc.validMembers.size() || members.size() > qc.signers.size()) { return nullptr; } + if (std::any_of(qc.validMembers.begin() + members.size(), qc.validMembers.end(), [](bool valid) { return valid; }) || + std::any_of(qc.signers.begin() + members.size(), qc.signers.end(), [](bool signed_member) { return signed_member; })) { + return nullptr; + } auto signers = std::make_shared(); for (size_t i = 0; i < members.size(); ++i) { if (qc.validMembers[i]) { @@ -878,30 +878,59 @@ std::shared_ptr CQuorumBlockProcessor::GetSigningRecords(g const int min_height{pindex->nHeight - window}; auto ret = std::make_shared(); + std::vector mined_block_hashes; + std::map> quorums_by_type; + const auto get_quorums = [&](const Consensus::LLMQParams& params) { + const bool rotation{IsQuorumRotationEnabled(params, pindex)}; + const size_t max_count{static_cast(window / params.dkgInterval + 1) * + static_cast(rotation ? params.signingActiveQuorumCount : 1)}; + return rotation ? GetMinedCommitmentsIndexedUntilBlock(params.type, pindex, max_count) + : GetMinedCommitmentsUntilBlock(params.type, pindex, max_count); + }; for (const auto& params : Params().GetConsensus().llmqs) { if (params.is_single_member()) { continue; } - const bool rotation{IsQuorumRotationEnabled(params, pindex)}; - const size_t max_count{static_cast(window / params.dkgInterval + 1) * - static_cast(rotation ? params.signingActiveQuorumCount : 1)}; - const auto quorums{rotation ? GetMinedCommitmentsIndexedUntilBlock(params.type, pindex, max_count) - : GetMinedCommitmentsUntilBlock(params.type, pindex, max_count)}; + const auto& quorums = quorums_by_type.emplace(params.type, get_quorums(params)).first->second; // Oldest first, so that the member lists of the active quorums stay in the shared caches for (const auto* pQuorumBaseBlockIndex : quorums | std::views::reverse) { if (pQuorumBaseBlockIndex->nHeight <= min_height) { continue; } - const auto signers = GetValidMemberSigners(params.type, pQuorumBaseBlockIndex); + const auto [qc, mined_block_hash] = GetMinedCommitment(params.type, pQuorumBaseBlockIndex->GetBlockHash()); + if (qc.IsNull()) { + return nullptr; + } + const auto signers = GetValidMemberSigners(qc, pQuorumBaseBlockIndex); if (!signers) { - continue; + return nullptr; } + mined_block_hashes.emplace_back(mined_block_hash); for (const auto& [proTxHash, signed_commitment] : *signers) { (*ret)[proTxHash].push_back({pQuorumBaseBlockIndex->nHeight, signed_commitment}); } } } + { + LOCK(::cs_main); + if (!m_chainman.ActiveChain().Contains(pindex)) { + return nullptr; + } + // A disconnect/reconnect can remove commitments before enumeration and restore them before this check. + for (const auto& params : Params().GetConsensus().llmqs) { + if (!params.is_single_member() && get_quorums(params) != quorums_by_type.at(params.type)) { + return nullptr; + } + } + for (const auto& hash : mined_block_hashes) { + const auto* mined_block = m_chainman.m_blockman.LookupBlockIndex(hash); + if (mined_block == nullptr || pindex->GetAncestor(mined_block->nHeight) != mined_block) { + return nullptr; + } + } + } + LOCK(m_signing_records_mutex); // Sessions of types with a longer DKG interval gather at an older block std::erase_if(m_valid_member_signers_cache, diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index c58f0235776b..e7f80aaf1a99 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -174,11 +174,12 @@ class CQuorumBlockProcessor /** * Signing records of every masternode that was a valid member of a non-null commitment mined until pindex * for a quorum based in (pindex->nHeight - window, pindex->nHeight]. + * Returns nullptr without caching the result if the view is incomplete or the chain changed underneath. */ std::shared_ptr GetSigningRecords(gsl::not_null pindex, int window) const - EXCLUSIVE_LOCKS_REQUIRED(!m_signing_records_mutex); + EXCLUSIVE_LOCKS_REQUIRED(!m_signing_records_mutex) LOCKS_EXCLUDED(::cs_main); private: - std::shared_ptr GetValidMemberSigners(Consensus::LLMQType llmqType, + std::shared_ptr GetValidMemberSigners(const CFinalCommitment& qc, gsl::not_null pQuorumBaseBlockIndex) const EXCLUSIVE_LOCKS_REQUIRED(!m_signing_records_mutex); diff --git a/src/test/llmq_blockprocessor_tests.cpp b/src/test/llmq_blockprocessor_tests.cpp index a2e48243f458..b68c2996776a 100644 --- a/src/test/llmq_blockprocessor_tests.cpp +++ b/src/test/llmq_blockprocessor_tests.cpp @@ -6,15 +6,19 @@ #include #include +#include +#include #include #include #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -22,6 +26,8 @@ #include #include #include +#include +#include #include #include @@ -427,4 +433,134 @@ BOOST_FIXTURE_TEST_CASE(signing_records_window_mining_height_rotation_and_cache, } } +BOOST_FIXTURE_TEST_CASE(signing_records_disconnect_reconnect, SigningHistorySetup) +{ + constexpr auto type{Consensus::LLMQType::LLMQ_TEST}; + CEvoDB evo_db{util::DbWrapperParams{.path = m_args.GetDataDirNet() / "signing-history", .memory = true, .wipe = true}}; + CQuorumBlockProcessor original{*m_node.chainman, dmnman, evo_db, *m_node.llmq_ctx->qsnapman, 0}; + CQuorumBlockProcessor recovering{*m_node.chainman, dmnman, evo_db, *m_node.llmq_ctx->qsnapman, 0}; + const auto qc = Commitment(type, 216, /*invalid_members=*/{2}, /*non_signers=*/{1}); + const auto mined_hash = Index(226)->GetBlockHash(); + WriteMinedCommitment(evo_db, qc, mined_hash, 226, 216); + auto* gathering = WITH_LOCK(cs_main, return m_node.chainman->ActiveChain()[240]); + NormalizedSigningRecords expected; + const auto members = Members(type, 216); + expected[members[0]->proTxHash].emplace_back(216, true); + expected[members[1]->proTxHash].emplace_back(216, false); + const auto records = original.GetSigningRecords(gathering, 48); + BOOST_REQUIRE(records); + BOOST_CHECK(Normalize(*records) == expected); + + auto& chainstate = m_node.chainman->ActiveChainstate(); + BlockValidationState state; + BOOST_REQUIRE(chainstate.InvalidateBlock(state, gathering)); + BOOST_REQUIRE(Index(240) == nullptr); + BOOST_CHECK(!recovering.GetSigningRecords(gathering, 48)); + evo_db.Erase(std::make_pair(std::string{"q_mc"}, std::make_pair(type, qc.quorumHash))); + BOOST_CHECK(!recovering.GetSigningRecords(gathering, 48)); + + WITH_LOCK(cs_main, chainstate.ResetBlockFailureFlags(gathering)); + BOOST_REQUIRE_MESSAGE(chainstate.ActivateBestChain(state), state.ToString()); + BOOST_REQUIRE(Index(240) == gathering); + BOOST_CHECK(!recovering.GetSigningRecords(gathering, 48)); + WriteMinedCommitment(evo_db, qc, mined_hash, 226, 216); + const auto cold = recovering.GetSigningRecords(gathering, 48); + BOOST_REQUIRE(cold); + BOOST_CHECK(Normalize(*cold) == expected); + const auto cached = recovering.GetSigningRecords(gathering, 48); + BOOST_REQUIRE(cached); + BOOST_CHECK(cached == cold); + BOOST_CHECK(Normalize(*cached) == expected); +} + +BOOST_FIXTURE_TEST_CASE(signing_records_incomplete_view_is_not_cached, SigningHistorySetup) +{ + constexpr auto type{Consensus::LLMQType::LLMQ_TEST}; + const auto qc = Commitment(type, 216); + Store(qc, 226, 216); + const auto key = std::make_pair(std::string{"q_mc"}, std::make_pair(type, qc.quorumHash)); + const auto* gathering = Index(228); + const auto check_incomplete = [&] { + CQuorumBlockProcessor fresh{*m_node.chainman, dmnman, *m_node.evodb, *m_node.llmq_ctx->qsnapman, 0}; + BOOST_CHECK(!fresh.GetSigningRecords(gathering, 36)); + Store(qc, 226, 216); + const auto cold = fresh.GetSigningRecords(gathering, 36); + BOOST_REQUIRE(cold); + NormalizedSigningRecords expected; + for (const auto& mn : Members(type, 216)) { + expected[mn->proTxHash].emplace_back(216, true); + } + BOOST_CHECK(Normalize(*cold) == expected); + const auto cached = fresh.GetSigningRecords(gathering, 36); + BOOST_REQUIRE(cached); + BOOST_CHECK(cached == cold); + BOOST_CHECK(Normalize(*cached) == expected); + }; + + m_node.evodb->Erase(key); + check_incomplete(); + for (const bool truncate_signers : {false, true}) { + auto malformed = qc; + (truncate_signers ? malformed.signers : malformed.validMembers).resize(1); + m_node.evodb->Write(key, std::make_pair(malformed, Index(226)->GetBlockHash())); + check_incomplete(); + } + for (const bool extend_signers : {false, true}) { + auto malformed = qc; + (extend_signers ? malformed.signers : malformed.validMembers).push_back(true); + m_node.evodb->Write(key, std::make_pair(malformed, Index(226)->GetBlockHash())); + check_incomplete(); + } + for (const auto& mined_hash : {uint256::ZERO, GetRandHash(), Index(229)->GetBlockHash()}) { + m_node.evodb->Write(key, std::make_pair(qc, mined_hash)); + check_incomplete(); + } +} + +BOOST_FIXTURE_TEST_CASE(unavailable_signing_history_preserves_other_connection_checks, SigningHistorySetup) +{ + constexpr auto type{Consensus::LLMQType::LLMQ_TEST}; + const auto qc = Commitment(type, 216); + Store(qc, 226, 216); + m_node.evodb->Erase(std::make_pair(std::string{"q_mc"}, std::make_pair(type, qc.quorumHash))); + BOOST_REQUIRE(!blockprocessor.GetSigningRecords(Index(240), 4032)); + BOOST_REQUIRE(m_node.mn_metaman->LoadCache(/*load_cache=*/false)); + + CSporkManager sporkman{m_args.GetDataDirNet()}; + CKey spork_key; + spork_key.MakeNewKey(/*fCompressed=*/true); + BOOST_REQUIRE(sporkman.SetSporkAddress(EncodeDestination(PKHash{spork_key.GetPubKey()}))); + CSporkMessage spork{SPORK_23_QUORUM_POSE, 0, NodeClock::now()}; + BOOST_REQUIRE(spork.Sign(spork_key)); + BOOST_REQUIRE(sporkman.ProcessSpork(spork)); + BOOST_REQUIRE(IsQuorumPoseEnabled(type, sporkman)); + BOOST_REQUIRE(!IsAllMembersConnectedEnabled(type, sporkman)); + + const auto members = Members(type, 240); + BOOST_REQUIRE_GE(members.size(), 2U); + const auto& own_protx = members[0]->proTxHash; + CActiveMasternodeManager nodeman{*m_node.connman, dmnman, operator_keys.at(own_protx)}; + CDKGDebugManager debugman{dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman}; + CDKGSessionManager dkgman{dmnman, *m_node.llmq_ctx->qsnapman, *m_node.chainman, sporkman, + util::DbWrapperParams{.path = m_args.GetDataDirNet(), .memory = true, .wipe = true}}; + const auto params = Params().GetLLMQ(type).value(); + ActiveDKGSession session{*m_node.llmq_ctx->bls_worker, dmnman, debugman, dkgman, *m_node.mn_metaman, + blockprocessor, *m_node.llmq_ctx->qsnapman, nodeman, *m_node.chainman, + sporkman, Index(240), params}; + BOOST_REQUIRE(session.Init(own_protx, 0)); + session.VerifyConnectionAndMinProtoVersions(*m_node.connman); + BOOST_CHECK(!session.SendComplaint()); + + const auto& failed_protx = members[1]->proTxHash; + for (int i = 0; i < 6; ++i) { + m_node.mn_metaman->SetLastOutboundAttempt(failed_protx, GetTime()); + } + BOOST_REQUIRE(m_node.mn_metaman->OutboundFailedTooManyTimes(failed_protx)); + session.VerifyConnectionAndMinProtoVersions(*m_node.connman); + const auto complaint = session.SendComplaint(); + BOOST_REQUIRE(complaint); + BOOST_CHECK_EQUAL(std::count(complaint->badMembers.begin(), complaint->badMembers.end(), true), 1); + BOOST_CHECK(complaint->badMembers[1]); +} + BOOST_AUTO_TEST_SUITE_END()