From 7c39004de967f6e4c934c03d3b47bebac48b4c66 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 01:22:20 +0700 Subject: [PATCH 01/12] fix: enable spork21 for feature_llmq_is_retroactive.py and fix a test failure This commit is a partial revert of 71ef10a8d168f5424c1fc6a12afb6b920b662ae6 --- .../functional/feature_llmq_is_retroactive.py | 26 +++++++++++-------- .../test_framework/test_framework.py | 7 +++-- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/test/functional/feature_llmq_is_retroactive.py b/test/functional/feature_llmq_is_retroactive.py index c8454ea72982..2b3b0901c2a9 100755 --- a/test/functional/feature_llmq_is_retroactive.py +++ b/test/functional/feature_llmq_is_retroactive.py @@ -29,7 +29,9 @@ def assert_no_instantlock(self, txid, node): assert not node.getrawtransaction(txid, True)["instantlock"] def sleep_and_assert_no_instantlock(self, txid, node, sleep=5): - time.sleep(sleep) + for _ in range(sleep): + time.sleep(1) + self.bump_mocktime(1) self.assert_no_instantlock(txid, node) # random delay before tx is actually send by network could take up to 30 seconds @@ -50,6 +52,7 @@ def create_fund_sign_tx(self): def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) + self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) # Turn mempool IS signing off self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 1) self.wait_for_sporks_same() @@ -63,7 +66,7 @@ def run_test(self): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) # 3 nodes should be enough to create an IS lock even if nodes 4 and 5 (which have no tx itself) # are the only "neighbours" in intra-quorum connections for one of them. - self.bump_mocktime(30) + self.bump_mocktime(10) self.sleep_and_assert_no_instantlock(txid, self.nodes[0]) # Have to disable ChainLocks to avoid signing a block with a "safe" tx too early self.nodes[0].sporkupdate("SPORK_19_CHAINLOCKS_ENABLED", 4000000000) @@ -84,7 +87,7 @@ def run_test(self): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) # 3 nodes should be enough to create an IS lock even if nodes 4 and 5 (which have no tx itself) # are the only "neighbours" in intra-quorum connections for one of them. - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_instantlock(txid, self.nodes[0]) block = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] self.wait_for_chainlocked_block_all_nodes(block) @@ -94,17 +97,18 @@ def run_test(self): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) # Make sure nodes 1 and 2 received the TX before we continue, # otherwise it might announce the TX to node 3 when reconnecting - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_tx(txid, self.nodes[1]) self.wait_for_tx(txid, self.nodes[2]) self.reconnect_isolated_node(3, 0) # Make sure nodes actually try re-connecting quorum connections - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_mnauth(self.nodes[3], 2) # node 3 fully reconnected but the TX wasn't relayed to it, so there should be no IS lock self.sleep_and_assert_no_instantlock(txid, self.nodes[0]) # push the tx directly via rpc self.nodes[3].sendrawtransaction(self.nodes[0].getrawtransaction(txid)) + self.bump_mocktime(10) # node 3 should vote on a tx now since it became aware of it via sendrawtransaction # and this should be enough to complete an IS lock self.wait_for_instantlock(txid, self.nodes[0]) @@ -125,12 +129,12 @@ def run_test(self): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) # Make sure nodes 1 and 2 received the TX before we continue, # otherwise it might announce the TX to node 3 when reconnecting - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_tx(txid, self.nodes[1]) self.wait_for_tx(txid, self.nodes[2]) self.reconnect_isolated_node(3, 0) # Make sure nodes actually try re-connecting quorum connections - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_mnauth(self.nodes[3], 2) # node 3 fully reconnected but the TX wasn't relayed to it, so there should be no IS lock self.sleep_and_assert_no_instantlock(txid, self.nodes[0]) @@ -157,7 +161,7 @@ def test_session_timeout(self, do_cycle_llmqs): txid_single_node = self.nodes[3].sendrawtransaction(rawtx_1) # Make sure nodes 1 and 2 received the TX before we continue - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_tx(txid_all_nodes, self.nodes[1]) self.wait_for_tx(txid_all_nodes, self.nodes[2]) # Make sure signing is done on nodes 1 and 2 (it's async) @@ -167,15 +171,15 @@ def test_session_timeout(self, do_cycle_llmqs): time.sleep(2) # make sure Cleanup() is called self.reconnect_isolated_node(3, 0) # Make sure nodes actually try re-connecting quorum connections - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_mnauth(self.nodes[3], 2) self.nodes[0].sendrawtransaction(rawtx_1) # Make sure nodes 1 and 2 received the TX - self.bump_mocktime(30) + self.bump_mocktime(10) self.wait_for_tx(txid_single_node, self.nodes[1]) self.wait_for_tx(txid_single_node, self.nodes[2]) - self.bump_mocktime(30) + self.bump_mocktime(10) # Make sure signing is done on nodes 1 and 2 (it's async) time.sleep(5) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index d9ad16b25459..d27e509758a2 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1929,22 +1929,25 @@ def wait_for_instantlock(self, txid, node, timeout=60): def check_instantlock(): try: + self.bump_mocktime(1) return node.getrawtransaction(txid, True)["instantlock"] except: return False self.log.info(f"Expecting InstantLock for {txid}") - self.wait_until(check_instantlock, timeout=timeout) + self.wait_until(check_instantlock, timeout=timeout, sleep=1) def wait_for_chainlocked_block(self, node, block_hash, expected=True, timeout=15): def check_chainlocked_block(): try: + self.bump_mocktime(1) block = node.getblock(block_hash) return block["confirmations"] > 0 and block["chainlock"] except: return False + self.log.info(f"Expecting ChainLock for {block_hash}") - if self.wait_until(check_chainlocked_block, timeout=timeout, do_assert=expected) and not expected: + if self.wait_until(check_chainlocked_block, timeout=timeout, sleep=1, do_assert=expected) and not expected: raise AssertionError("waiting unexpectedly succeeded") def wait_for_chainlocked_block_all_nodes(self, block_hash, timeout=15): From c96395ba67e001e592f19f7089915c62443ecb2c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 7 Nov 2025 00:23:46 +0700 Subject: [PATCH 02/12] refactor: drop spork21 as always active Downside of having spork21: - llmq code run in 2 different modes depending it is active or not active. It complicates implementation while we not actually need a case of "inactive" - it increase scope of test coverage; functional test to test it takes 2 minutes to run and it increase cost of CI, making CI slower, making local run of functional tests noticeable slower See previuos commit: functional test feature_llmq_is_retroactive.py actually had been broken for awhile, but disabled spork21 masked it --- src/llmq/options.cpp | 5 +- src/llmq/signing_shares.cpp | 2 +- src/spork.cpp | 7 +- src/spork.h | 5 +- test/functional/feature_llmq_connections.py | 4 - test/functional/feature_llmq_data_recovery.py | 1 - .../functional/feature_llmq_is_retroactive.py | 1 - test/functional/feature_llmq_signing.py | 113 ++++++++---------- test/functional/feature_llmq_simplepose.py | 4 - .../test_framework/test_framework.py | 6 +- test/functional/test_runner.py | 1 - 11 files changed, 57 insertions(+), 92 deletions(-) diff --git a/src/llmq/options.cpp b/src/llmq/options.cpp index 6ce3bdf8ff66..61774501f94a 100644 --- a/src/llmq/options.cpp +++ b/src/llmq/options.cpp @@ -31,10 +31,7 @@ static bool EvalSpork(const Consensus::LLMQType llmqType, const int64_t spork_va return false; } -bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) -{ - return EvalSpork(llmqType, sporkman.GetSporkValue(SPORK_21_QUORUM_ALL_CONNECTED)); -} +bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { return true; } bool IsQuorumPoseEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index c5e8c87e4be1..b684419b5151 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -238,7 +238,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms // non-masternodes are not interested in sigshares if (m_mn_activeman.GetProTxHash().IsNull()) return; - if (m_sporkman.IsSporkActive(SPORK_21_QUORUM_ALL_CONNECTED) && msg_type == NetMsgType::QSIGSHARE) { + if (msg_type == NetMsgType::QSIGSHARE) { std::vector receivedSigShares; vRecv >> receivedSigShares; diff --git a/src/spork.cpp b/src/spork.cpp index 9842e44226f6..a8c18ad1c931 100644 --- a/src/spork.cpp +++ b/src/spork.cpp @@ -262,12 +262,7 @@ SporkValue CSporkManager::GetSporkValue(SporkId nSporkID) const { // Harden all sporks on Mainnet if (!Params().IsTestChain()) { - switch (nSporkID) { - case SPORK_21_QUORUM_ALL_CONNECTED: - return 1; - default: - return 0; - } + return 0; } LOCK(cs); diff --git a/src/spork.h b/src/spork.h index 2351460f12af..90e4c2d0eb65 100644 --- a/src/spork.h +++ b/src/spork.h @@ -41,7 +41,7 @@ enum SporkId : int32_t { SPORK_9_SUPERBLOCKS_ENABLED = 10008, SPORK_17_QUORUM_DKG_ENABLED = 10016, SPORK_19_CHAINLOCKS_ENABLED = 10018, - SPORK_21_QUORUM_ALL_CONNECTED = 10020, + SPORK_21_DEPRECATED = 10020, SPORK_23_QUORUM_POSE = 10022, // SPORK_24_DEPRECATED = 10023, @@ -69,13 +69,12 @@ struct CSporkDef }; #define MAKE_SPORK_DEF(name, defaultValue) CSporkDef{name, defaultValue, #name} -[[maybe_unused]] static constexpr std::array sporkDefs = { +[[maybe_unused]] static constexpr std::array sporkDefs = { MAKE_SPORK_DEF(SPORK_2_INSTANTSEND_ENABLED, 4070908800ULL), // OFF MAKE_SPORK_DEF(SPORK_3_INSTANTSEND_BLOCK_FILTERING, 4070908800ULL), // OFF MAKE_SPORK_DEF(SPORK_9_SUPERBLOCKS_ENABLED, 4070908800ULL), // OFF MAKE_SPORK_DEF(SPORK_17_QUORUM_DKG_ENABLED, 4070908800ULL), // OFF MAKE_SPORK_DEF(SPORK_19_CHAINLOCKS_ENABLED, 4070908800ULL), // OFF - MAKE_SPORK_DEF(SPORK_21_QUORUM_ALL_CONNECTED, 4070908800ULL), // OFF MAKE_SPORK_DEF(SPORK_23_QUORUM_POSE, 4070908800ULL), // OFF }; #undef MAKE_SPORK_DEF diff --git a/test/functional/feature_llmq_connections.py b/test/functional/feature_llmq_connections.py index a9acd24885a4..806ab9ab9e37 100755 --- a/test/functional/feature_llmq_connections.py +++ b/test/functional/feature_llmq_connections.py @@ -68,10 +68,6 @@ def run_test(self): for mn in self.get_quorum_masternodes(q): self.wait_until(lambda: self.get_mn_probe_count(mn.get_node(self), q, True) == 4) - self.log.info("Activating SPORK_21_QUORUM_ALL_CONNECTED") - self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) - self.wait_for_sporks_same() - self.check_reconnects(4) self.nodes[0].sporkupdate("SPORK_23_QUORUM_POSE", 4070908800) diff --git a/test/functional/feature_llmq_data_recovery.py b/test/functional/feature_llmq_data_recovery.py index 76a92e234027..1f1bd6d51937 100755 --- a/test/functional/feature_llmq_data_recovery.py +++ b/test/functional/feature_llmq_data_recovery.py @@ -158,7 +158,6 @@ def run_test(self): node = self.nodes[0] node.sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) - node.sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) self.wait_for_sporks_same() logger.info("Test automated DGK data recovery") diff --git a/test/functional/feature_llmq_is_retroactive.py b/test/functional/feature_llmq_is_retroactive.py index 2b3b0901c2a9..fce71f6d8256 100755 --- a/test/functional/feature_llmq_is_retroactive.py +++ b/test/functional/feature_llmq_is_retroactive.py @@ -52,7 +52,6 @@ def create_fund_sign_tx(self): def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) - self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) # Turn mempool IS signing off self.nodes[0].sporkupdate("SPORK_2_INSTANTSEND_ENABLED", 1) self.wait_for_sporks_same() diff --git a/test/functional/feature_llmq_signing.py b/test/functional/feature_llmq_signing.py index 82782e743cad..77e45d97f109 100755 --- a/test/functional/feature_llmq_signing.py +++ b/test/functional/feature_llmq_signing.py @@ -25,21 +25,14 @@ def set_test_params(self): self.set_dash_test_params(6, 5) self.set_dash_llmq_test_params(5, 3) - def add_options(self, parser): - parser.add_argument("--spork21", dest="spork21", default=False, action="store_true", - help="Test with spork21 enabled") - def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) - if self.options.spork21: - self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) self.wait_for_sporks_same() self.mine_quorum() - if self.options.spork21: - assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size + assert self.mninfo[0].get_node(self).getconnectioncount() == self.llmq_size id = "0000000000000000000000000000000000000000000000000000000000000001" msgHash = "0000000000000000000000000000000000000000000000000000000000000002" @@ -75,42 +68,37 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): quorumHash = self.mninfo[1].get_node(self).quorum("selectquorum", q_type, id)["quorumHash"] assert self.mninfo[1].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash) assert_sigs_nochange(False, False, False, 3) - # Sign third share and test optional submit parameter if spork21 is enabled, should result in recovered sig + # Sign third share and test optional submit parameter should result in recovered sig # and conflict for msgHashConflict - if self.options.spork21: - # 1. Providing an invalid quorum hash and set submit=false, should throw an error - assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False) - # 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object - sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False) - sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False) - assert_equal(sig_share_rpc_1, sig_share_rpc_2) - assert_sigs_nochange(False, False, False, 3) - # 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result - # in a recovered sig - sig_share = CSigShare() - sig_share.llmqType = int(sig_share_rpc_1["llmqType"]) - sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16) - sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"]) - sig_share.id = int(sig_share_rpc_1["id"], 16) - sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16) - sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"]) - for mn in self.mninfo: # type: MasternodeInfo - assert mn.get_node(self).getconnectioncount() == self.llmq_size - # Get the current recovery member of the quorum - q = self.nodes[0].quorum('selectquorum', q_type, id) - mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0]) - # Open a P2P connection to it - p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface()) - # Send the last required QSIGSHARE message to the recovery member - p2p_interface.send_message(msg_qsigshare([sig_share])) - else: - # If spork21 is not enabled just sign regularly - self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash) + # 1. Providing an invalid quorum hash and set submit=false, should throw an error + assert_raises_rpc_error(-8, 'quorum not found', self.mninfo[2].get_node(self).quorum, "sign", q_type, id, msgHash, id, False) + # 2. Providing a valid quorum hash and set submit=false, should return a valid sigShare object + sig_share_rpc_1 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, quorumHash, False) + sig_share_rpc_2 = self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash, "", False) + assert_equal(sig_share_rpc_1, sig_share_rpc_2) + assert_sigs_nochange(False, False, False, 3) + # 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result + # in a recovered sig + sig_share = CSigShare() + sig_share.llmqType = int(sig_share_rpc_1["llmqType"]) + sig_share.quorumHash = int(sig_share_rpc_1["quorumHash"], 16) + sig_share.quorumMember = int(sig_share_rpc_1["quorumMember"]) + sig_share.id = int(sig_share_rpc_1["id"], 16) + sig_share.msgHash = int(sig_share_rpc_1["msgHash"], 16) + sig_share.sigShare = bytes.fromhex(sig_share_rpc_1["signature"]) + for mn in self.mninfo: # type: MasternodeInfo + assert mn.get_node(self).getconnectioncount() == self.llmq_size + # Get the current recovery member of the quorum + q = self.nodes[0].quorum('selectquorum', q_type, id) + mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0]) + # Open a P2P connection to it + p2p_interface = mn.get_node(self).add_p2p_connection(P2PInterface()) + # Send the last required QSIGSHARE message to the recovery member + p2p_interface.send_message(msg_qsigshare([sig_share])) wait_for_sigs(True, False, True, 15) - if self.options.spork21: - mn.get_node(self).disconnect_p2ps() + mn.get_node(self).disconnect_p2ps() # Test `quorum verify` rpc node = self.mninfo[0].get_node(self) @@ -175,29 +163,28 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash) wait_for_sigs(True, False, True, 15) - if self.options.spork21: - id = uint256_to_string(request_id + 1) - - # Isolate the node that is responsible for the recovery of a signature and assert that recovery fails - q = self.nodes[0].quorum('selectquorum', q_type, id) - mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0]) - mn.get_node(self).setnetworkactive(False) - self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0) - for i in range(4): - self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash) - assert_sigs_nochange(False, False, False, 3) - # Need to re-connect so that it later gets the recovered sig - mn.get_node(self).setnetworkactive(True) - self.connect_nodes(mn.nodeIdx, 0) - force_finish_mnsync(mn.get_node(self)) - # Make sure intra-quorum connections were also restored - self.bump_mocktime(1) # need this to bypass quorum connection retry timeout - self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10) - mn.get_node(self).ping() - self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo())) - # Let 2 seconds pass so that the next node is used for recovery, which should succeed - self.bump_mocktime(2) - wait_for_sigs(True, False, True, 2) + id = uint256_to_string(request_id + 1) + + # Isolate the node that is responsible for the recovery of a signature and assert that recovery fails + q = self.nodes[0].quorum('selectquorum', q_type, id) + mn: MasternodeInfo = self.get_mninfo(q['recoveryMembers'][0]) + mn.get_node(self).setnetworkactive(False) + self.wait_until(lambda: mn.get_node(self).getconnectioncount() == 0) + for i in range(4): + self.mninfo[i].get_node(self).quorum("sign", q_type, id, msgHash) + assert_sigs_nochange(False, False, False, 3) + # Need to re-connect so that it later gets the recovered sig + mn.get_node(self).setnetworkactive(True) + self.connect_nodes(mn.nodeIdx, 0) + force_finish_mnsync(mn.get_node(self)) + # Make sure intra-quorum connections were also restored + self.bump_mocktime(1) # need this to bypass quorum connection retry timeout + self.wait_until(lambda: mn.get_node(self).getconnectioncount() == self.llmq_size, timeout=10) + mn.get_node(self).ping() + self.wait_until(lambda: all('pingwait' not in peer for peer in mn.get_node(self).getpeerinfo())) + # Let 2 seconds pass so that the next node is used for recovery, which should succeed + self.bump_mocktime(2) + wait_for_sigs(True, False, True, 2) if __name__ == '__main__': LLMQSigningTest().main() diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 0b3cfcb3953a..0f752de02e31 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -48,10 +48,6 @@ def run_test(self): self.test_banning(self.isolate_mn, 2) self.repair_masternodes(False) - - self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0) - self.wait_for_sporks_same() - self.reset_probe_timeouts() if not self.options.disable_spork23: diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index d27e509758a2..5a12cba5e710 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -2104,11 +2104,10 @@ def move_blocks(self, nodes, num_blocks): self.generate(self.nodes[0], num_blocks, sync_fun=lambda: self.sync_blocks(nodes)) def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None, skip_maturity=False): - spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1 spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1 if expected_connections is None: - expected_connections = (self.llmq_size - 1) if spork21_active else 2 + expected_connections = self.llmq_size - 1 if expected_members is None: expected_members = self.llmq_size if expected_contributions is None: @@ -2194,13 +2193,12 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec return new_quorum def mine_cycle_quorum(self): - spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1 spork23_active = self.nodes[0].spork('show')['SPORK_23_QUORUM_POSE'] <= 1 llmq_type_name="llmq_test_dip0024" llmq_type=103 llmq_cycle_len = 24 - expected_connections = (self.llmq_size_dip0024 - 1) if spork21_active else 2 + expected_connections = self.llmq_size_dip0024 - 1 expected_members = self.llmq_size_dip0024 expected_contributions = self.llmq_size_dip0024 expected_commitments = self.llmq_size_dip0024 diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index d63f866f4bca..7f9ffa241395 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -130,7 +130,6 @@ 'feature_dip3_deterministicmns.py --legacy-wallet', # NOTE: needs dash_hash to pass 'feature_dip3_deterministicmns.py --descriptors', # NOTE: needs dash_hash to pass 'feature_llmq_signing.py', # NOTE: needs dash_hash to pass - 'feature_llmq_signing.py --spork21', # 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_is_cl_conflicts.py', # NOTE: needs dash_hash to pass From 0da31c21828b3e7d52dc2e678d0d1bc74db555ac Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 7 Nov 2025 20:47:49 +0700 Subject: [PATCH 03/12] refactor: remove handlers and sender of QGETSIGSHARES and related methods from llmq/signing_shares Careful reviewing implementation is showing that there's no relevant code. Logic which prepares & creates data to be sent by QGETSIGSHARES is `CollectSigSharesToRequest` in the loop over nodeState.sessions and it does nothing if spork21 is enabled --- src/llmq/signing_shares.cpp | 166 +----------------------------------- src/llmq/signing_shares.h | 4 - 2 files changed, 1 insertion(+), 169 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index b684419b5151..585e0c6e5178 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -280,18 +280,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms return; } } else if (msg_type == NetMsgType::QGETSIGSHARES) { - std::vector msgs; - vRecv >> msgs; - if (msgs.size() > MAX_MSGS_CNT_QGETSIGSHARES) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QGETSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QGETSIGSHARES, pfrom.GetId()); - BanNode(pfrom.GetId()); - return; - } - if (!ranges::all_of(msgs, - [this, &pfrom](const auto& inv){ return ProcessMessageGetSigShares(pfrom, inv); })) { - BanNode(pfrom.GetId()); - return; - } + return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } else if (msg_type == NetMsgType::QBSIGSHARES) { std::vector msgs; vRecv >> msgs; @@ -387,36 +376,6 @@ bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSi return true; } -bool CSigSharesManager::ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) -{ - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId, sessionInfo)) { - return true; - } - - if (!VerifySigSharesInv(sessionInfo.llmqType, inv)) { - return false; - } - - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo.signHash.Get())) { - return true; - } - - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), inv.ToString(), pfrom.GetId()); - - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { - return true; - } - session->requested.Merge(inv); - session->knows.Merge(inv); - return true; -} - bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) { CSigSharesNodeState::SessionInfo sessionInfo; @@ -961,103 +920,6 @@ void CSigSharesManager::NotifyRecoveredSig(const std::shared_ptrGetHash()); } -void CSigSharesManager::CollectSigSharesToRequest(std::unordered_map>& sigSharesToRequest) -{ - AssertLockHeld(cs); - - int64_t now = GetTime().count(); - const size_t maxRequestsForNode = 32; - - // avoid requesting from same nodes all the time - std::vector shuffledNodeIds; - shuffledNodeIds.reserve(nodeStates.size()); - for (const auto& [nodeId, nodeState] : nodeStates) { - if (nodeState.sessions.empty()) { - continue; - } - shuffledNodeIds.emplace_back(nodeId); - } - Shuffle(shuffledNodeIds.begin(), shuffledNodeIds.end(), rnd); - - for (const auto& nodeId : shuffledNodeIds) { - auto& nodeState = nodeStates[nodeId]; - - if (nodeState.banned) { - continue; - } - - nodeState.requestedSigShares.EraseIf([&now, &nodeId](const SigShareKey& k, int64_t t) { - if (now - t >= SIG_SHARE_REQUEST_TIMEOUT) { - // timeout while waiting for this one, so retry it with another node - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::CollectSigSharesToRequest -- timeout while waiting for %s-%d, node=%d\n", - k.first.ToString(), k.second, nodeId); - return true; - } - return false; - }); - - decltype(sigSharesToRequest.begin()->second)* invMap = nullptr; - - for (auto& [signHash, session] : nodeState.sessions) { - if (IsAllMembersConnectedEnabled(session.llmqType, m_sporkman)) { - continue; - } - - if (sigman.HasRecoveredSigForSession(signHash)) { - continue; - } - - for (const auto i : irange::range(session.announced.inv.size())) { - if (!session.announced.inv[i]) { - continue; - } - auto k = std::make_pair(signHash, (uint16_t) i); - if (sigShares.Has(k)) { - // we already have it - session.announced.inv[i] = false; - continue; - } - if (nodeState.requestedSigShares.Size() >= maxRequestsForNode) { - // too many pending requests for this node - break; - } - if (auto *const p = sigSharesRequested.Get(k)) { - if (now - p->second >= SIG_SHARE_REQUEST_TIMEOUT && nodeId != p->first) { - // other node timed out, re-request from this node - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- other node timeout while waiting for %s-%d, re-request from=%d, node=%d\n", __func__, - k.first.ToString(), k.second, nodeId, p->first); - } else { - continue; - } - } - // if we got this far we should do a request - - // track when we initiated the request so that we can detect timeouts - nodeState.requestedSigShares.Add(k, now); - - // don't request it from other nodes until a timeout happens - auto& r = sigSharesRequested.GetOrAdd(k); - r.first = nodeId; - r.second = now; - - if (invMap == nullptr) { - invMap = &sigSharesToRequest[nodeId]; - } - auto& inv = (*invMap)[signHash]; - if (inv.inv.empty()) { - const auto& llmq_params_opt = Params().GetLLMQ(session.llmqType); - assert(llmq_params_opt.has_value()); - inv.Init(llmq_params_opt->size); - } - inv.inv[k.second] = true; - - // don't request it again from this node - session.announced.inv[i] = false; - } - } - } -} - void CSigSharesManager::CollectSigSharesToSend(std::unordered_map>& sigSharesToSend) { AssertLockHeld(cs); @@ -1212,7 +1074,6 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map> sigSharesToRequest; std::unordered_map> sigShareBatchesToSend; std::unordered_map> sigSharesToSend; std::unordered_map> sigSharesToAnnounce; @@ -1242,16 +1103,10 @@ bool CSigSharesManager::SendMessages() const CConnman::NodesSnapshot snap{m_connman, /* cond = */ CConnman::FullyConnectedOnly}; { LOCK(cs); - CollectSigSharesToRequest(sigSharesToRequest); CollectSigSharesToSend(sigShareBatchesToSend); CollectSigSharesToAnnounce(sigSharesToAnnounce); CollectSigSharesToSendConcentrated(sigSharesToSend, snap.Nodes()); - for (auto& [nodeId, sigShareMap] : sigSharesToRequest) { - for (auto& [hash, sigShareInv] : sigShareMap) { - sigShareInv.sessionId = addSigSesAnnIfNeeded(nodeId, hash); - } - } for (auto& [nodeId, sigShareBatchesMap] : sigShareBatchesToSend) { for (auto& [hash, sigShareBatch] : sigShareBatchesMap) { sigShareBatch.sessionId = addSigSesAnnIfNeeded(nodeId, hash); @@ -1288,25 +1143,6 @@ bool CSigSharesManager::SendMessages() } } - if (const auto it = sigSharesToRequest.find(pnode->GetId()); it != sigSharesToRequest.end()) { - std::vector msgs; - for (const auto& [signHash, inv] : it->second) { - assert(inv.CountSet() != 0); - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::SendMessages -- QGETSIGSHARES signHash=%s, inv={%s}, node=%d\n", - signHash.ToString(), inv.ToString(), pnode->GetId()); - msgs.emplace_back(inv); - if (msgs.size() == MAX_MSGS_CNT_QGETSIGSHARES) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); - msgs.clear(); - didSend = true; - } - } - if (!msgs.empty()) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QGETSIGSHARES, msgs)); - didSend = true; - } - } - if (const auto jt = sigShareBatchesToSend.find(pnode->GetId()); jt != sigShareBatchesToSend.end()) { size_t totalSigsCount = 0; std::vector msgs; diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index f49c927ed068..b92a52c68962 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -367,7 +367,6 @@ class CSigSharesManager : public CRecoveredSigsListener // we try to keep total message size below 10k static constexpr size_t MAX_MSGS_CNT_QSIGSESANN{100}; - static constexpr size_t MAX_MSGS_CNT_QGETSIGSHARES{200}; static constexpr size_t MAX_MSGS_CNT_QSIGSHARESINV{200}; // 400 is the maximum quorum size, so this is also the maximum number of sigs we need to support static constexpr size_t MAX_MSGS_TOTAL_BATCHED_SIGS{400}; @@ -454,7 +453,6 @@ class CSigSharesManager : public CRecoveredSigsListener // all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages) bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) EXCLUSIVE_LOCKS_REQUIRED(!cs); void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -488,8 +486,6 @@ class CSigSharesManager : public CRecoveredSigsListener void BanNode(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool SendMessages() EXCLUSIVE_LOCKS_REQUIRED(!cs); - void CollectSigSharesToRequest(std::unordered_map>& sigSharesToRequest) - EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToSend(std::unordered_map>& sigSharesToSend) EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs); From ce58eeed8db929c46fada67f7f0cbefe9f70142e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 08:11:20 +0700 Subject: [PATCH 04/12] refactor: remove handlers and sender of QBSIGSHARES and related methods from llmq/signing_shares Careful reviewing implementation is showing that there's no relevant code after removing spork21 Logic which prepares & creates data to be sent by QBSIGSHARES is `CollectSigSharesToSend` in the loop over nodeState.sessions and it does nothing if spork21 is enabled --- src/llmq/signing_shares.cpp | 203 +----------------------------------- src/llmq/signing_shares.h | 24 ----- 2 files changed, 1 insertion(+), 226 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 585e0c6e5178..a1d25e50af4f 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -88,17 +88,6 @@ void CSigSharesInv::SetAll(bool v) std::fill(inv.begin(), inv.end(), v); } -std::string CBatchedSigShares::ToInvString() const -{ - CSigSharesInv inv; - // we use 400 here no matter what the real size is. We don't really care about that size as we just want to call ToString() - inv.Init(400); - for (const auto& sigShare : sigShares) { - inv.inv[sigShare.first] = true; - } - return inv.ToString(); -} - static void InitSession(CSigSharesNodeState::Session& s, const llmq::SignHash& signHash, CSigBase from) { const auto& llmq_params_opt = Params().GetLLMQ(from.getLlmqType()); @@ -282,22 +271,7 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms } else if (msg_type == NetMsgType::QGETSIGSHARES) { return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } else if (msg_type == NetMsgType::QBSIGSHARES) { - std::vector msgs; - vRecv >> msgs; - size_t totalSigsCount = 0; - for (const auto& bs : msgs) { - totalSigsCount += bs.sigShares.size(); - } - if (totalSigsCount > MAX_MSGS_TOTAL_BATCHED_SIGS) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many sigs in QBSIGSHARES message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_TOTAL_BATCHED_SIGS, pfrom.GetId()); - BanNode(pfrom.GetId()); - return; - } - if (!ranges::all_of(msgs, - [this, &pfrom](const auto& bs){ return ProcessMessageBatchedSigShares(pfrom, bs); })) { - BanNode(pfrom.GetId()); - return; - } + return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } } @@ -376,60 +350,6 @@ bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSi return true; } -bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) -{ - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), batchedSigShares.sessionId, sessionInfo)) { - return true; - } - - if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, sessionInfo, batchedSigShares, ban)) { - return !ban; - } - - std::vector sigSharesToProcess; - sigSharesToProcess.reserve(batchedSigShares.sigShares.size()); - - { - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - - for (const auto& sigSharetmp : batchedSigShares.sigShares) { - CSigShare sigShare = RebuildSigShare(sessionInfo, sigSharetmp); - nodeState.requestedSigShares.Erase(sigShare.GetKey()); - - // TODO track invalid sig shares received for PoSe? - // It's important to only skip seen *valid* sig shares here. If a node sends us a - // batch of mostly valid sig shares with a single invalid one and thus batched - // verification fails, we'd skip the valid ones in the future if received from other nodes - if (sigShares.Has(sigShare.GetKey())) { - continue; - } - - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) { - continue; - } - - sigSharesToProcess.emplace_back(sigShare); - } - } - - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), batchedSigShares.sigShares.size(), sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); - - if (sigSharesToProcess.empty()) { - return true; - } - - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - for (const auto& s : sigSharesToProcess) { - nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); - } - return true; -} - void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) { auto quorum = qman.GetQuorum(sigShare.getLlmqType(), sigShare.getQuorumHash()); @@ -481,48 +401,6 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& s sigShare.GetSignHash().ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), sigShare.getQuorumMember(), fromId); } -bool CSigSharesManager::PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager, - const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan) -{ - retBan = false; - - if (!IsQuorumActive(session.llmqType, quorum_manager, session.quorum->qc->quorumHash)) { - // quorum is too old - return false; - } - if (!session.quorum->IsMember(mn_activeman.GetProTxHash())) { - // we're not a member so we can't verify it (we actually shouldn't have received it) - return false; - } - if (!session.quorum->HasVerificationVector()) { - // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible.\n", __func__, - session.quorumHash.ToString()); - return false; - } - - std::unordered_set dupMembers; - - for (const auto& [quorumMember, _] : batchedSigShares.sigShares) { - if (!dupMembers.emplace(quorumMember).second) { - retBan = true; - return false; - } - - if (quorumMember >= session.quorum->members.size()) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember out of bounds\n", __func__); - retBan = true; - return false; - } - if (!session.quorum->qc->validMembers[quorumMember]) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember not valid\n", __func__); - retBan = true; - return false; - } - } - return true; -} - bool CSigSharesManager::CollectPendingSigSharesToVerify( size_t maxUniqueSessions, std::unordered_map>& retSigShares, std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums) @@ -920,56 +798,6 @@ void CSigSharesManager::NotifyRecoveredSig(const std::shared_ptrGetHash()); } -void CSigSharesManager::CollectSigSharesToSend(std::unordered_map>& sigSharesToSend) -{ - AssertLockHeld(cs); - - for (auto& [nodeId, nodeState] : nodeStates) { - if (nodeState.banned) { - continue; - } - - decltype(sigSharesToSend.begin()->second)* sigSharesToSend2 = nullptr; - - for (auto& [signHash, session] : nodeState.sessions) { - if (IsAllMembersConnectedEnabled(session.llmqType, m_sporkman)) { - continue; - } - - if (sigman.HasRecoveredSigForSession(signHash)) { - continue; - } - - CBatchedSigShares batchedSigShares; - - for (const auto i : irange::range(session.requested.inv.size())) { - if (!session.requested.inv[i]) { - continue; - } - session.requested.inv[i] = false; - - auto k = std::make_pair(signHash, (uint16_t)i); - const CSigShare* sigShare = sigShares.Get(k); - if (sigShare == nullptr) { - // he requested something we don't have - session.requested.inv[i] = false; - continue; - } - - batchedSigShares.sigShares.emplace_back((uint16_t)i, sigShare->sigShare); - } - - if (!batchedSigShares.sigShares.empty()) { - if (sigSharesToSend2 == nullptr) { - // only create the map if we actually add a batched sig - sigSharesToSend2 = &sigSharesToSend[nodeId]; - } - sigSharesToSend2->try_emplace(signHash, std::move(batchedSigShares)); - } - } - } -} - void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes) { AssertLockHeld(cs); @@ -1074,7 +902,6 @@ void CSigSharesManager::CollectSigSharesToAnnounce(std::unordered_map> sigShareBatchesToSend; std::unordered_map> sigSharesToSend; std::unordered_map> sigSharesToAnnounce; std::unordered_map> sigSessionAnnouncements; @@ -1103,15 +930,9 @@ bool CSigSharesManager::SendMessages() const CConnman::NodesSnapshot snap{m_connman, /* cond = */ CConnman::FullyConnectedOnly}; { LOCK(cs); - CollectSigSharesToSend(sigShareBatchesToSend); CollectSigSharesToAnnounce(sigSharesToAnnounce); CollectSigSharesToSendConcentrated(sigSharesToSend, snap.Nodes()); - for (auto& [nodeId, sigShareBatchesMap] : sigShareBatchesToSend) { - for (auto& [hash, sigShareBatch] : sigShareBatchesMap) { - sigShareBatch.sessionId = addSigSesAnnIfNeeded(nodeId, hash); - } - } for (auto& [nodeId, sigShareMap] : sigSharesToAnnounce) { for (auto& [hash, sigShareInv] : sigShareMap) { sigShareInv.sessionId = addSigSesAnnIfNeeded(nodeId, hash); @@ -1143,28 +964,6 @@ bool CSigSharesManager::SendMessages() } } - if (const auto jt = sigShareBatchesToSend.find(pnode->GetId()); jt != sigShareBatchesToSend.end()) { - size_t totalSigsCount = 0; - std::vector msgs; - for (const auto& [signHash, inv] : jt->second) { - assert(!inv.sigShares.empty()); - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::SendMessages -- QBSIGSHARES signHash=%s, inv={%s}, node=%d\n", - signHash.ToString(), inv.ToInvString(), pnode->GetId()); - if (totalSigsCount + inv.sigShares.size() > MAX_MSGS_TOTAL_BATCHED_SIGS) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, msgs)); - msgs.clear(); - totalSigsCount = 0; - didSend = true; - } - totalSigsCount += inv.sigShares.size(); - msgs.emplace_back(inv); - } - if (!msgs.empty()) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QBSIGSHARES, std::move(msgs))); - didSend = true; - } - } - if (const auto kt = sigSharesToAnnounce.find(pnode->GetId()); kt != sigSharesToAnnounce.end()) { std::vector msgs; for (const auto& [signHash, inv] : kt->second) { diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b92a52c68962..3f4aab8cb0a2 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -138,22 +138,6 @@ class CSigSharesInv [[nodiscard]] std::string ToString() const; }; -// sent through the message QBSIGSHARES as a vector of multiple batches -class CBatchedSigShares -{ -public: - uint32_t sessionId{UNINITIALIZED_SESSION_ID}; - std::vector> sigShares; - -public: - SERIALIZE_METHODS(CBatchedSigShares, obj) - { - READWRITE(VARINT(obj.sessionId), obj.sigShares); - } - - [[nodiscard]] std::string ToInvString() const; -}; - template class SigShareMap { @@ -368,8 +352,6 @@ class CSigSharesManager : public CRecoveredSigsListener // we try to keep total message size below 10k static constexpr size_t MAX_MSGS_CNT_QSIGSESANN{100}; static constexpr size_t MAX_MSGS_CNT_QSIGSHARESINV{200}; - // 400 is the maximum quorum size, so this is also the maximum number of sigs we need to support - static constexpr size_t MAX_MSGS_TOTAL_BATCHED_SIGS{400}; static constexpr int64_t EXP_SEND_FOR_RECOVERY_TIMEOUT{2000}; static constexpr int64_t MAX_SEND_FOR_RECOVERY_TIMEOUT{10000}; @@ -453,13 +435,9 @@ class CSigSharesManager : public CRecoveredSigsListener // all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages) bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) - EXCLUSIVE_LOCKS_REQUIRED(!cs); void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) EXCLUSIVE_LOCKS_REQUIRED(!cs); static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv); - static bool PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager, - const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan); bool CollectPendingSigSharesToVerify( size_t maxUniqueSessions, std::unordered_map>& retSigShares, @@ -486,8 +464,6 @@ class CSigSharesManager : public CRecoveredSigsListener void BanNode(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(!cs); bool SendMessages() EXCLUSIVE_LOCKS_REQUIRED(!cs); - void CollectSigSharesToSend(std::unordered_map>& sigSharesToSend) - EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs); void CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce) EXCLUSIVE_LOCKS_REQUIRED(cs); From d3d42e11590ef6b32d2d14bf907ac6617e2f3680 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 08:27:18 +0700 Subject: [PATCH 05/12] refactor: remove function ForceReAnnouncement which is used only when spork21 is disabled --- src/llmq/signing_shares.cpp | 31 ------------------------------- src/llmq/signing_shares.h | 2 -- 2 files changed, 33 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index a1d25e50af4f..04da1d64308d 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -784,10 +784,6 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning } } - if (allowReSign) { - // make us re-announce all known shares (other nodes might have run into a timeout) - ForceReAnnouncement(*quorum, llmqType, id, msgHash); - } AsyncSign(std::move(quorum), id, msgHash); return true; @@ -1342,33 +1338,6 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorum& quorum return sigShare; } -// causes all known sigShares to be re-announced -void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) -{ - if (IsAllMembersConnectedEnabled(llmqType, m_sporkman)) { - return; - } - - LOCK(cs); - auto signHash = SignHash(llmqType, quorum.qc->quorumHash, id, msgHash).Get(); - if (const auto *const sigs = sigShares.GetAllForSignHash(signHash)) { - for (const auto& [quorumMemberIndex, _] : *sigs) { - // re-announce every sigshare to every node - sigSharesQueuedToAnnounce.Add(std::make_pair(signHash, quorumMemberIndex), true); - } - } - for (auto& [_, nodeState] : nodeStates) { - auto* session = nodeState.GetSessionBySignHash(signHash); - if (session == nullptr) { - continue; - } - // pretend that the other node doesn't know about any shares so that we re-announce everything - session->knows.SetAll(false); - // we need to use a new session id as we don't know if the other node has run into a timeout already - session->sendSessionId = UNINITIALIZED_SESSION_ID; - } -} - MessageProcessingResult CSigSharesManager::HandleNewRecoveredSig(const llmq::CRecoveredSig& recoveredSig) { auto signHash = recoveredSig.buildSignHash().Get(); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 3f4aab8cb0a2..f409b4538f74 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -417,8 +417,6 @@ class CSigSharesManager : public CRecoveredSigsListener EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs); std::optional CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, - const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override EXCLUSIVE_LOCKS_REQUIRED(!cs); From 182283526ee1b911dbd9c61b0e38ed98e3c8f35b Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 08:29:47 +0700 Subject: [PATCH 06/12] refactor: remove member sigSharesRequested which is always empty now --- src/llmq/signing_shares.cpp | 22 ---------------------- src/llmq/signing_shares.h | 1 - 2 files changed, 23 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 04da1d64308d..b521bcbb61f5 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1139,13 +1139,6 @@ void CSigSharesManager::Cleanup() if (it == nodeStates.end()) { continue; } - // remove global requested state to force a re-request from another node - // TODO: remove NO_THREAD_SAFETY_ANALYSIS - // using here template ForEach makes impossible to use lock annotation - it->second.requestedSigShares.ForEach([this](const SigShareKey& k, bool) NO_THREAD_SAFETY_ANALYSIS { - AssertLockHeld(cs); - sigSharesRequested.Erase(k); - }); nodeStates.erase(nodeId); } @@ -1160,7 +1153,6 @@ void CSigSharesManager::RemoveSigSharesForSession(const uint256& signHash) nodeState.RemoveSession(signHash); } - sigSharesRequested.EraseAllForSignHash(signHash); sigSharesQueuedToAnnounce.EraseAllForSignHash(signHash); sigShares.EraseAllForSignHash(signHash); signedSessions.erase(signHash); @@ -1174,13 +1166,6 @@ void CSigSharesManager::RemoveBannedNodeStates() LOCK(cs); for (auto it = nodeStates.begin(); it != nodeStates.end();) { if (m_peerman.IsBanned(it->first)) { - // re-request sigshares from other nodes - // TODO: remove NO_THREAD_SAFETY_ANALYSIS - // using here template ForEach makes impossible to use lock annotation - it->second.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS { - AssertLockHeld(cs); - sigSharesRequested.Erase(k); - }); it = nodeStates.erase(it); } else { ++it; @@ -1203,13 +1188,6 @@ void CSigSharesManager::BanNode(NodeId nodeId) } auto& nodeState = it->second; - // Whatever we requested from him, let's request it from someone else now - // TODO: remove NO_THREAD_SAFETY_ANALYSIS - // using here template ForEach makes impossible to use lock annotation - nodeState.requestedSigShares.ForEach([this](const SigShareKey& k, int64_t) NO_THREAD_SAFETY_ANALYSIS { - AssertLockHeld(cs); - sigSharesRequested.Erase(k); - }); nodeState.requestedSigShares.Clear(); nodeState.banned = true; } diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index f409b4538f74..bcea75f27379 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -369,7 +369,6 @@ class CSigSharesManager : public CRecoveredSigsListener Uint256HashMap timeSeenForSessions GUARDED_BY(cs); std::unordered_map nodeStates GUARDED_BY(cs); - SigShareMap> sigSharesRequested GUARDED_BY(cs); SigShareMap sigSharesQueuedToAnnounce GUARDED_BY(cs); struct PendingSignatureData { From e53983a3450916aae0c46c50dd39a03cbd893353 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 08:33:31 +0700 Subject: [PATCH 07/12] refactor: remove member sigSharesQueuedToAnnounce as always empty when spork21 is active --- src/llmq/signing_shares.cpp | 58 +------------------------------------ src/llmq/signing_shares.h | 1 - 2 files changed, 1 insertion(+), 58 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index b521bcbb61f5..81614f63010e 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -584,9 +584,6 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum if (!sigShares.Add(sigShare.GetKey(), sigShare)) { return; } - if (!isAllMembersConnectedEnabled) { - sigSharesQueuedToAnnounce.Add(sigShare.GetKey(), true); - } // Update the time we've seen the last sigShare timeSeenForSessions[sigShare.GetSignHash()] = GetTime().count(); @@ -841,59 +838,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToAnnounce) { - AssertLockHeld(cs); - - std::unordered_map, std::unordered_set, StaticSaltedHasher> quorumNodesMap; - - // TODO: remove NO_THREAD_SAFETY_ANALYSIS - // using here template ForEach makes impossible to use lock annotation - sigSharesQueuedToAnnounce.ForEach([this, &quorumNodesMap, &sigSharesToAnnounce](const SigShareKey& sigShareKey, - bool) NO_THREAD_SAFETY_ANALYSIS { - AssertLockHeld(cs); - const auto& signHash = sigShareKey.first; - auto quorumMember = sigShareKey.second; - const CSigShare* sigShare = sigShares.Get(sigShareKey); - if (sigShare == nullptr) { - return; - } - - // announce to the nodes which we know through the intra-quorum-communication system - auto quorumKey = std::make_pair(sigShare->getLlmqType(), sigShare->getQuorumHash()); - auto it = quorumNodesMap.find(quorumKey); - if (it == quorumNodesMap.end()) { - auto nodeIds = m_connman.GetMasternodeQuorumNodes(quorumKey.first, quorumKey.second); - it = quorumNodesMap.emplace(std::piecewise_construct, std::forward_as_tuple(quorumKey), std::forward_as_tuple(nodeIds.begin(), nodeIds.end())).first; - } - - const auto& quorumNodes = it->second; - - for (const auto& nodeId : quorumNodes) { - auto& nodeState = nodeStates[nodeId]; - - if (nodeState.banned) { - continue; - } - - auto& session = nodeState.GetOrCreateSessionFromShare(*sigShare); - - if (session.knows.inv[quorumMember]) { - // he already knows that one - continue; - } - - auto& inv = sigSharesToAnnounce[nodeId][signHash]; - if (inv.inv.empty()) { - const auto& llmq_params_opt = Params().GetLLMQ(sigShare->getLlmqType()); - assert(llmq_params_opt.has_value()); - inv.Init(llmq_params_opt->size); - } - inv.inv[quorumMember] = true; - session.knows.inv[quorumMember] = true; - } - }); - - // don't announce these anymore - sigSharesQueuedToAnnounce.Clear(); + // Do nothing } bool CSigSharesManager::SendMessages() @@ -1153,7 +1098,6 @@ void CSigSharesManager::RemoveSigSharesForSession(const uint256& signHash) nodeState.RemoveSession(signHash); } - sigSharesQueuedToAnnounce.EraseAllForSignHash(signHash); sigShares.EraseAllForSignHash(signHash); signedSessions.erase(signHash); timeSeenForSessions.erase(signHash); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index bcea75f27379..b7f2fe225821 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -369,7 +369,6 @@ class CSigSharesManager : public CRecoveredSigsListener Uint256HashMap timeSeenForSessions GUARDED_BY(cs); std::unordered_map nodeStates GUARDED_BY(cs); - SigShareMap sigSharesQueuedToAnnounce GUARDED_BY(cs); struct PendingSignatureData { const CQuorumCPtr quorum; From 3c650589edbbe30ed34dab795b78f372b16aa87f Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 08:42:10 +0700 Subject: [PATCH 08/12] refactor: remove functions related to QSIGSHARESINV p2p message which could be send only if spork21 is disabled --- src/llmq/signing_shares.cpp | 276 +----------------------------------- src/llmq/signing_shares.h | 67 +-------- 2 files changed, 6 insertions(+), 337 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 81614f63010e..af6de92de83e 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -33,12 +33,6 @@ void CSigShare::UpdateKey() key.second = quorumMember; } -std::string CSigSesAnn::ToString() const -{ - return strprintf("sessionId=%d, llmqType=%d, quorumHash=%s, id=%s, msgHash=%s", - sessionId, ToUnderlying(getLlmqType()), getQuorumHash().ToString(), getId().ToString(), getMsgHash().ToString()); -} - void CSigSharesInv::Merge(const CSigSharesInv& inv2) { for (const auto i : irange::range(inv.size())) { @@ -88,79 +82,9 @@ void CSigSharesInv::SetAll(bool v) std::fill(inv.begin(), inv.end(), v); } -static void InitSession(CSigSharesNodeState::Session& s, const llmq::SignHash& signHash, CSigBase from) -{ - const auto& llmq_params_opt = Params().GetLLMQ(from.getLlmqType()); - assert(llmq_params_opt.has_value()); - const auto& llmq_params = llmq_params_opt.value(); - - s.llmqType = from.getLlmqType(); - s.quorumHash = from.getQuorumHash(); - s.id = from.getId(); - s.msgHash = from.getMsgHash(); - s.signHash = signHash; - s.announced.Init((size_t)llmq_params.size); - s.requested.Init((size_t)llmq_params.size); - s.knows.Init((size_t)llmq_params.size); -} - -CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromShare(const llmq::CSigShare& sigShare) -{ - auto& s = sessions[sigShare.GetSignHash()]; - if (s.announced.inv.empty()) { - InitSession(s, sigShare.buildSignHash(), sigShare); - } - return s; -} - -CSigSharesNodeState::Session& CSigSharesNodeState::GetOrCreateSessionFromAnn(const llmq::CSigSesAnn& ann) -{ - auto signHash = ann.buildSignHash(); - auto& s = sessions[signHash.Get()]; - if (s.announced.inv.empty()) { - InitSession(s, signHash, ann); - } - return s; -} - -CSigSharesNodeState::Session* CSigSharesNodeState::GetSessionBySignHash(const uint256& signHash) -{ - auto it = sessions.find(signHash); - if (it == sessions.end()) { - return nullptr; - } - return &it->second; -} - -CSigSharesNodeState::Session* CSigSharesNodeState::GetSessionByRecvId(uint32_t sessionId) -{ - auto it = sessionByRecvId.find(sessionId); - if (it == sessionByRecvId.end()) { - return nullptr; - } - return it->second; -} - -bool CSigSharesNodeState::GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo) -{ - const auto* s = GetSessionByRecvId(sessionId); - if (s == nullptr) { - return false; - } - retInfo.llmqType = s->llmqType; - retInfo.quorumHash = s->quorumHash; - retInfo.id = s->id; - retInfo.msgHash = s->msgHash; - retInfo.signHash = s->signHash; - retInfo.quorum = s->quorum; - - return true; -} - void CSigSharesNodeState::RemoveSession(const uint256& signHash) { if (const auto it = sessions.find(signHash); it != sessions.end()) { - sessionByRecvId.erase(it->second.recvSessionId); sessions.erase(it); } requestedSigShares.EraseAllForSignHash(signHash); @@ -240,34 +164,10 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms for (const auto& sigShare : receivedSigShares) { ProcessMessageSigShare(pfrom.GetId(), sigShare); } - } - - if (msg_type == NetMsgType::QSIGSESANN) { - std::vector msgs; - vRecv >> msgs; - if (msgs.size() > MAX_MSGS_CNT_QSIGSESANN) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many announcements in QSIGSESANN message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSESANN, pfrom.GetId()); - BanNode(pfrom.GetId()); - return; - } - if (!ranges::all_of(msgs, - [this, &pfrom](const auto& ann){ return ProcessMessageSigSesAnn(pfrom, ann); })) { - BanNode(pfrom.GetId()); - return; - } + } else if (msg_type == NetMsgType::QSIGSESANN) { + return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } else if (msg_type == NetMsgType::QSIGSHARESINV) { - std::vector msgs; - vRecv >> msgs; - if (msgs.size() > MAX_MSGS_CNT_QSIGSHARESINV) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- too many invs in QSIGSHARESINV message. cnt=%d, max=%d, node=%d\n", __func__, msgs.size(), MAX_MSGS_CNT_QSIGSHARESINV, pfrom.GetId()); - BanNode(pfrom.GetId()); - return; - } - if (!ranges::all_of(msgs, - [this, &pfrom](const auto& inv){ return ProcessMessageSigSharesInv(pfrom, inv); })) { - BanNode(pfrom.GetId()); - return; - } + return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } else if (msg_type == NetMsgType::QGETSIGSHARES) { return; // Do nothing: this message is not expected to be received once spork21 is hardened as active } else if (msg_type == NetMsgType::QBSIGSHARES) { @@ -275,81 +175,6 @@ void CSigSharesManager::ProcessMessage(const CNode& pfrom, const std::string& ms } } -bool CSigSharesManager::ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) -{ - auto llmqType = ann.getLlmqType(); - if (!Params().GetLLMQ(llmqType).has_value()) { - return false; - } - if (ann.getSessionId() == UNINITIALIZED_SESSION_ID || ann.getQuorumHash().IsNull() || ann.getId().IsNull() || ann.getMsgHash().IsNull()) { - return false; - } - - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- ann={%s}, node=%d\n", __func__, ann.ToString(), pfrom.GetId()); - - auto quorum = qman.GetQuorum(llmqType, ann.getQuorumHash()); - if (!quorum) { - // TODO should we ban here? - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorum %s not found, node=%d\n", __func__, - ann.getQuorumHash().ToString(), pfrom.GetId()); - return true; // let's still try other announcements from the same message - } - - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto& session = nodeState.GetOrCreateSessionFromAnn(ann); - nodeState.sessionByRecvId.erase(session.recvSessionId); - nodeState.sessionByRecvId.erase(ann.getSessionId()); - session.recvSessionId = ann.getSessionId(); - session.quorum = quorum; - nodeState.sessionByRecvId.try_emplace(ann.getSessionId(), &session); - - return true; -} - -bool CSigSharesManager::VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv) -{ - const auto& llmq_params_opt = Params().GetLLMQ(llmqType); - return llmq_params_opt.has_value() && (inv.inv.size() == size_t(llmq_params_opt->size)); -} - -bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) -{ - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId, sessionInfo)) { - return true; - } - - if (!VerifySigSharesInv(sessionInfo.llmqType, inv)) { - return false; - } - - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo.signHash.Get())) { - return true; - } - - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), inv.ToString(), pfrom.GetId()); - - if (!sessionInfo.quorum->HasVerificationVector()) { - // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, - sessionInfo.quorumHash.ToString(), pfrom.GetId()); - return true; - } - - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { - return true; - } - session->announced.Merge(inv); - session->knows.Merge(inv); - return true; -} - void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) { auto quorum = qman.GetQuorum(sigShare.getLlmqType(), sigShare.getQuorumHash()); @@ -590,12 +415,8 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum // don't announce and wait for other nodes to request this share and directly send it to them // there is no way the other nodes know about this share as this is the one created on this node - for (auto otherNodeId : quorumNodes) { - auto& nodeState = nodeStates[otherNodeId]; - auto& session = nodeState.GetOrCreateSessionFromShare(sigShare); - session.quorum = quorum; - session.requested.Set(sigShare.getQuorumMember(), true); - session.knows.Set(sigShare.getQuorumMember(), true); + for (auto _: quorumNodes) { + // quorumNodes is always empty because isAllMembersConnectedEnabled is always true } size_t sigShareCount = sigShares.CountForSignHash(sigShare.GetSignHash()); @@ -836,49 +657,14 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToAnnounce) -{ - // Do nothing -} - bool CSigSharesManager::SendMessages() { std::unordered_map> sigSharesToSend; - std::unordered_map> sigSharesToAnnounce; - std::unordered_map> sigSessionAnnouncements; - - auto addSigSesAnnIfNeeded = [&](NodeId nodeId, const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs) { - AssertLockHeld(cs); - auto& nodeState = nodeStates[nodeId]; - auto* session = nodeState.GetSessionBySignHash(signHash); - assert(session); - while (session->sendSessionId == UNINITIALIZED_SESSION_ID) { - const uint32_t session_id{GetRand()}; - if (ranges::all_of(nodeState.sessions, - [&session_id](const auto& s) { return s.second.sendSessionId != session_id; })) { - // No session is using this id yet - session->sendSessionId = session_id; - sigSessionAnnouncements[nodeId].emplace_back( - CSigSesAnn(/*sessionId=*/session->sendSessionId, /*llmqType=*/session->llmqType, - /*quorumHash=*/session->quorumHash, /*id=*/session->id, /*msgHash=*/session->msgHash)); - } - // It's very unlikely that there is a session with the same id, - // but if there is one we just start over and pick another id - } - return session->sendSessionId; - }; const CConnman::NodesSnapshot snap{m_connman, /* cond = */ CConnman::FullyConnectedOnly}; { LOCK(cs); - CollectSigSharesToAnnounce(sigSharesToAnnounce); CollectSigSharesToSendConcentrated(sigSharesToSend, snap.Nodes()); - - for (auto& [nodeId, sigShareMap] : sigSharesToAnnounce) { - for (auto& [hash, sigShareInv] : sigShareMap) { - sigShareInv.sessionId = addSigSesAnnIfNeeded(nodeId, hash); - } - } } bool didSend = false; @@ -886,44 +672,6 @@ bool CSigSharesManager::SendMessages() for (auto& pnode : snap.Nodes()) { CNetMsgMaker msgMaker(pnode->GetCommonVersion()); - if (const auto it1 = sigSessionAnnouncements.find(pnode->GetId()); it1 != sigSessionAnnouncements.end()) { - std::vector msgs; - msgs.reserve(it1->second.size()); - for (auto& sigSesAnn : it1->second) { - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::SendMessages -- QSIGSESANN signHash=%s, sessionId=%d, node=%d\n", - sigSesAnn.buildSignHash().ToString(), sigSesAnn.getSessionId(), pnode->GetId()); - msgs.emplace_back(sigSesAnn); - if (msgs.size() == MAX_MSGS_CNT_QSIGSESANN) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); - msgs.clear(); - didSend = true; - } - } - if (!msgs.empty()) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSESANN, msgs)); - didSend = true; - } - } - - if (const auto kt = sigSharesToAnnounce.find(pnode->GetId()); kt != sigSharesToAnnounce.end()) { - std::vector msgs; - for (const auto& [signHash, inv] : kt->second) { - assert(inv.CountSet() != 0); - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::SendMessages -- QSIGSHARESINV signHash=%s, inv={%s}, node=%d\n", - signHash.ToString(), inv.ToString(), pnode->GetId()); - msgs.emplace_back(inv); - if (msgs.size() == MAX_MSGS_CNT_QSIGSHARESINV) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); - msgs.clear(); - didSend = true; - } - } - if (!msgs.empty()) { - m_connman.PushMessage(pnode, msgMaker.Make(NetMsgType::QSIGSHARESINV, msgs)); - didSend = true; - } - } - auto lt = sigSharesToSend.find(pnode->GetId()); if (lt != sigSharesToSend.end()) { std::vector msgs; @@ -947,20 +695,6 @@ bool CSigSharesManager::SendMessages() return didSend; } -bool CSigSharesManager::GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) -{ - LOCK(cs); - return nodeStates[nodeId].GetSessionInfoByRecvId(sessionId, retInfo); -} - -CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair& in) -{ - const auto& [member, sig] = in; - CSigShare sigShare(session.llmqType, session.quorumHash, session.id, session.msgHash, member, sig); - sigShare.UpdateKey(); - return sigShare; -} - void CSigSharesManager::Cleanup() { int64_t now = GetTime().count(); diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index b7f2fe225821..c8c3abb1b032 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -85,34 +85,6 @@ class CSigShare : virtual public CSigBase } }; -// Nodes will first announce a signing session with a sessionId to be used in all future P2P messages related to that -// session. We locally keep track of the mapping for each node. We also assign new sessionIds for outgoing sessions -// and send QSIGSESANN messages appropriately. All values except the max value for uint32_t are valid as sessionId -class CSigSesAnn : virtual public CSigBase -{ -private: - uint32_t sessionId{UNINITIALIZED_SESSION_ID}; - -public: - CSigSesAnn(uint32_t _sessionId, Consensus::LLMQType _llmqType, const uint256& _quorumHash, const uint256& _id, - const uint256& _msgHash) : CSigBase(_llmqType, _quorumHash, _id, _msgHash), sessionId(_sessionId) {}; - // ONLY FOR SERIALIZATION - CSigSesAnn() = default; - - - - [[nodiscard]] auto getSessionId() const { - return sessionId; - } - - SERIALIZE_METHODS(CSigSesAnn, obj) - { - READWRITE(VARINT(obj.sessionId), obj.llmqType, obj.quorumHash, obj.id, obj.msgHash); - } - - [[nodiscard]] std::string ToString() const; -}; - class CSigSharesInv { public: @@ -286,23 +258,8 @@ class SigShareMap class CSigSharesNodeState { public: - // Used to avoid holding locks too long - struct SessionInfo - { - Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; - uint256 quorumHash; - uint256 id; - uint256 msgHash; - llmq::SignHash signHash; - - CQuorumCPtr quorum; - }; - struct Session { - uint32_t recvSessionId{UNINITIALIZED_SESSION_ID}; - uint32_t sendSessionId{UNINITIALIZED_SESSION_ID}; - - Consensus::LLMQType llmqType; + Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; uint256 quorumHash; uint256 id; uint256 msgHash; @@ -317,19 +274,11 @@ class CSigSharesNodeState // TODO limit number of sessions per node Uint256HashMap sessions; - std::unordered_map sessionByRecvId; - SigShareMap pendingIncomingSigShares; SigShareMap requestedSigShares; bool banned{false}; - Session& GetOrCreateSessionFromShare(const CSigShare& sigShare); - Session& GetOrCreateSessionFromAnn(const CSigSesAnn& ann); - Session* GetSessionBySignHash(const uint256& signHash); - Session* GetSessionByRecvId(uint32_t sessionId); - bool GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo); - void RemoveSession(const uint256& signHash); }; @@ -349,10 +298,6 @@ class CSigSharesManager : public CRecoveredSigsListener static constexpr int64_t SESSION_NEW_SHARES_TIMEOUT{60}; static constexpr int64_t SIG_SHARE_REQUEST_TIMEOUT{5}; - // we try to keep total message size below 10k - static constexpr size_t MAX_MSGS_CNT_QSIGSESANN{100}; - static constexpr size_t MAX_MSGS_CNT_QSIGSHARESINV{200}; - static constexpr int64_t EXP_SEND_FOR_RECOVERY_TIMEOUT{2000}; static constexpr int64_t MAX_SEND_FOR_RECOVERY_TIMEOUT{10000}; static constexpr size_t MAX_MSGS_SIG_SHARES{32}; @@ -429,12 +374,8 @@ class CSigSharesManager : public CRecoveredSigsListener private: // all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages) - bool ProcessMessageSigSesAnn(const CNode& pfrom, const CSigSesAnn& ann) EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!cs); void ProcessMessageSigShare(NodeId fromId, const CSigShare& sigShare) EXCLUSIVE_LOCKS_REQUIRED(!cs); - static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv); - bool CollectPendingSigSharesToVerify( size_t maxUniqueSessions, std::unordered_map>& retSigShares, std::unordered_map, CQuorumCPtr, StaticSaltedHasher>& retQuorums) @@ -449,10 +390,6 @@ class CSigSharesManager : public CRecoveredSigsListener void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs); void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) - EXCLUSIVE_LOCKS_REQUIRED(!cs); - static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair& in); - void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs); void RemoveSigSharesForSession(const uint256& signHash) EXCLUSIVE_LOCKS_REQUIRED(cs); void RemoveBannedNodeStates() EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -461,8 +398,6 @@ class CSigSharesManager : public CRecoveredSigsListener bool SendMessages() EXCLUSIVE_LOCKS_REQUIRED(!cs); void CollectSigSharesToSendConcentrated(std::unordered_map>& sigSharesToSend, const std::vector& vNodes) EXCLUSIVE_LOCKS_REQUIRED(cs); - void CollectSigSharesToAnnounce(std::unordered_map>& sigSharesToAnnounce) - EXCLUSIVE_LOCKS_REQUIRED(cs); void SignPendingSigShares() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs); void WorkThreadMain() EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns, !cs); }; From 115d6954fc41f8df2cf0cca57aa22dbe1d5078a9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 09:12:45 +0700 Subject: [PATCH 09/12] refactor: removed unused class CSigSharesInv --- src/llmq/signing_shares.cpp | 49 ------------------------------------- src/llmq/signing_shares.h | 29 ---------------------- 2 files changed, 78 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index af6de92de83e..b6dcc0d29aeb 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -33,55 +33,6 @@ void CSigShare::UpdateKey() key.second = quorumMember; } -void CSigSharesInv::Merge(const CSigSharesInv& inv2) -{ - for (const auto i : irange::range(inv.size())) { - if (inv2.inv[i]) { - inv[i] = inv2.inv[i]; - } - } -} - -size_t CSigSharesInv::CountSet() const -{ - return (size_t)std::count(inv.begin(), inv.end(), true); -} - -std::string CSigSharesInv::ToString() const -{ - std::string str = "("; - bool first = true; - for (const auto i : irange::range(inv.size())) { - if (!inv[i]) { - continue; - } - - if (!first) { - str += ","; - } - first = false; - str += strprintf("%d", i); - } - str += ")"; - return str; -} - -void CSigSharesInv::Init(size_t size) -{ - inv.resize(size, false); -} - -void CSigSharesInv::Set(uint16_t quorumMember, bool v) -{ - assert(quorumMember < inv.size()); - inv[quorumMember] = v; -} - -void CSigSharesInv::SetAll(bool v) -{ - std::fill(inv.begin(), inv.end(), v); -} - void CSigSharesNodeState::RemoveSession(const uint256& signHash) { if (const auto it = sessions.find(signHash); it != sessions.end()) { diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index c8c3abb1b032..58f41268339f 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -85,31 +85,6 @@ class CSigShare : virtual public CSigBase } }; -class CSigSharesInv -{ -public: - uint32_t sessionId{UNINITIALIZED_SESSION_ID}; - std::vector inv; - -public: - SERIALIZE_METHODS(CSigSharesInv, obj) - { - uint64_t invSize = obj.inv.size(); - READWRITE(VARINT(obj.sessionId), COMPACTSIZE(invSize)); - autobitset_t bitset = std::make_pair(obj.inv, (size_t)invSize); - READWRITE(AUTOBITSET(bitset)); - SER_READ(obj, obj.inv = bitset.first); - } - - void Init(size_t size); - void Set(uint16_t quorumMember, bool v); - void SetAll(bool v); - void Merge(const CSigSharesInv& inv2); - - [[nodiscard]] size_t CountSet() const; - [[nodiscard]] std::string ToString() const; -}; - template class SigShareMap { @@ -266,10 +241,6 @@ class CSigSharesNodeState llmq::SignHash signHash; CQuorumCPtr quorum; - - CSigSharesInv announced; - CSigSharesInv requested; - CSigSharesInv knows; }; // TODO limit number of sessions per node Uint256HashMap sessions; From a714bfdd6bf677cd72f5ab8106c201a04f08fca4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 09:27:35 +0700 Subject: [PATCH 10/12] refactor: remove GetMasternodeQuorumNodes as it was used for disabled spork21 only --- src/llmq/signing_shares.cpp | 15 --------------- src/net.cpp | 26 -------------------------- src/net.h | 2 -- 3 files changed, 43 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index b6dcc0d29aeb..41c7613bdf4c 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -341,15 +341,6 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum auto llmqType = quorum->params.type; bool canTryRecovery = false; - const bool isAllMembersConnectedEnabled = IsAllMembersConnectedEnabled(llmqType, m_sporkman); - - // prepare node set for direct-push in case this is our sig share - std::vector quorumNodes; - if (!isAllMembersConnectedEnabled && - sigShare.getQuorumMember() == quorum->GetMemberIndex(m_mn_activeman.GetProTxHash())) { - quorumNodes = m_connman.GetMasternodeQuorumNodes(sigShare.getLlmqType(), sigShare.getQuorumHash()); - } - if (sigman.HasRecoveredSigForId(llmqType, sigShare.getId())) { return; } @@ -364,12 +355,6 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum // Update the time we've seen the last sigShare timeSeenForSessions[sigShare.GetSignHash()] = GetTime().count(); - // don't announce and wait for other nodes to request this share and directly send it to them - // there is no way the other nodes know about this share as this is the one created on this node - for (auto _: quorumNodes) { - // quorumNodes is always empty because isAllMembersConnectedEnabled is always true - } - size_t sigShareCount = sigShares.CountForSignHash(sigShare.GetSignHash()); if (sigShareCount >= size_t(quorum->params.threshold)) { canTryRecovery = true; diff --git a/src/net.cpp b/src/net.cpp index 7c120ff66833..e52aa3c5fade 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4318,32 +4318,6 @@ Uint256HashSet CConnman::GetMasternodeQuorums(Consensus::LLMQType llmqType) cons return result; } -std::vector CConnman::GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const -{ - READ_LOCK(m_nodes_mutex); - LOCK(cs_vPendingMasternodes); - auto it = masternodeQuorumNodes.find(std::make_pair(llmqType, quorumHash)); - if (it == masternodeQuorumNodes.end()) { - return {}; - } - const auto& proRegTxHashes = it->second; - - std::vector nodes; - - auto IsMasternodeQuorumNode = [&](const CNode* n) { - if (n->fDisconnect) return false; - const auto h = n->GetVerifiedProRegTxHash(); - return n->qwatch || (!h.IsNull() && proRegTxHashes.contains(h)); - }; - - for (NodeId id : m_nodes - | std::views::filter(IsMasternodeQuorumNode) - | std::views::transform([](const CNode* n){ return n->GetId(); })) { - nodes.push_back(id); - } - return nodes; -} - void CConnman::RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) { LOCK(cs_vPendingMasternodes); diff --git a/src/net.h b/src/net.h index b2e457fef01d..331708ca1c32 100644 --- a/src/net.h +++ b/src/net.h @@ -1468,8 +1468,6 @@ friend class CNode; void SetMasternodeQuorumRelayMembers(Consensus::LLMQType llmqType, const uint256& quorumHash, const Uint256HashSet& proTxHashes) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool HasMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const; Uint256HashSet GetMasternodeQuorums(Consensus::LLMQType llmqType) const; - // also returns QWATCH nodes - std::vector GetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); void RemoveMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash); bool IsMasternodeQuorumNode(const CNode* pnode, const CDeterministicMNList& tip_mn_list) const; bool IsMasternodeQuorumRelayMember(const uint256& protxHash); From 5891e44545a9733748b05f1afd9ff0cdeaf60771 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 20:01:36 +0700 Subject: [PATCH 11/12] refactor: drop leftover usages of IsAllMembersConnectedEnabled and unused references to spork manager --- src/llmq/context.cpp | 2 +- src/llmq/dkgsession.cpp | 3 +-- src/llmq/dkgsessionhandler.cpp | 2 +- src/llmq/options.cpp | 2 -- src/llmq/options.h | 1 - src/llmq/quorums.cpp | 7 +++---- src/llmq/quorums.h | 4 +--- src/llmq/signing_shares.cpp | 24 +++++++-------------- src/llmq/signing_shares.h | 5 +---- src/llmq/utils.cpp | 35 ++++++++++++++----------------- src/llmq/utils.h | 4 ++-- src/masternode/active/context.cpp | 2 +- src/rpc/quorums.cpp | 5 ++--- 13 files changed, 37 insertions(+), 59 deletions(-) diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 9e8cb27dea71..91bf5646bff8 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -28,7 +28,7 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d mn_metaman, *quorum_block_processor, *qsnapman, mn_activeman, sporkman, db_params)}, qman{std::make_unique(*bls_worker, chainman.ActiveChainstate(), dmnman, *qdkgsman, evo_db, - *quorum_block_processor, *qsnapman, mn_activeman, mn_sync, sporkman, + *quorum_block_processor, *qsnapman, mn_activeman, mn_sync, db_params)}, sigman{std::make_unique(chainman.ActiveChainstate(), *qman, db_params)}, clhandler{std::make_unique(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)}, diff --git a/src/llmq/dkgsession.cpp b/src/llmq/dkgsession.cpp index 062040a819a5..641ca48ee81c 100644 --- a/src/llmq/dkgsession.cpp +++ b/src/llmq/dkgsession.cpp @@ -478,13 +478,12 @@ void CDKGSession::VerifyConnectionAndMinProtoVersions(CConnman& connman) const protoMap.emplace(verifiedProRegTxHash, pnode->nVersion); }); - bool fShouldAllMembersBeConnected = IsAllMembersConnectedEnabled(params.type, m_sporkman); for (const auto& m : members) { if (m->dmn->proTxHash == myProTxHash) { continue; } if (auto it = protoMap.find(m->dmn->proTxHash); it == protoMap.end()) { - m->badConnection = fShouldAllMembersBeConnected; + m->badConnection = true; if (m->badConnection) { logger.Batch("%s is not connected to us, badConnection=1", m->dmn->proTxHash.ToString()); } diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 1a200be7cf34..13be7975ea14 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -563,7 +563,7 @@ void CDKGSessionHandler::HandleDKGRound(CConnman& connman, PeerManager& peerman) } const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - utils::EnsureQuorumConnections(params, connman, m_dmnman, m_sporkman, m_qsnapman, tip_mn_list, pQuorumBaseBlockIndex, + utils::EnsureQuorumConnections(params, connman, m_dmnman, m_qsnapman, tip_mn_list, pQuorumBaseBlockIndex, curSession->myProTxHash, /* is_masternode = */ m_mn_activeman != nullptr); if (curSession->AreWeMember()) { utils::AddQuorumProbeConnections(params, connman, m_dmnman, m_mn_metaman, m_qsnapman, m_sporkman, tip_mn_list, diff --git a/src/llmq/options.cpp b/src/llmq/options.cpp index 61774501f94a..e32b48c59cde 100644 --- a/src/llmq/options.cpp +++ b/src/llmq/options.cpp @@ -31,8 +31,6 @@ static bool EvalSpork(const Consensus::LLMQType llmqType, const int64_t spork_va return false; } -bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { return true; } - bool IsQuorumPoseEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman) { return EvalSpork(llmqType, sporkman.GetSporkValue(SPORK_23_QUORUM_POSE)); diff --git a/src/llmq/options.h b/src/llmq/options.h index b8c02804bde2..e43a1c3a5a6c 100644 --- a/src/llmq/options.h +++ b/src/llmq/options.h @@ -34,7 +34,6 @@ static constexpr bool DEFAULT_ENABLE_QUORUM_DATA_RECOVERY{true}; // If true, we will connect to all new quorums and watch their communication static constexpr bool DEFAULT_WATCH_QUORUMS{false}; -bool IsAllMembersConnectedEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman); bool IsQuorumPoseEnabled(const Consensus::LLMQType llmqType, const CSporkManager& sporkman); bool IsQuorumRotationEnabled(const Consensus::LLMQParams& llmqParams, gsl::not_null pindex); diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 3c467aca2530..c7b800aa2058 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -211,7 +211,7 @@ CQuorumManager::CQuorumManager(CBLSWorker& _blsWorker, CChainState& chainstate, CDKGSessionManager& _dkgManager, CEvoDB& _evoDb, CQuorumBlockProcessor& _quorumBlockProcessor, CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, - const CSporkManager& sporkman, const util::DbWrapperParams& db_params) : + const util::DbWrapperParams& db_params) : db{util::MakeDbWrapper( {db_params.path / "llmq" / "quorumdb", db_params.memory, db_params.wipe, /*cache_size=*/1 << 20})}, blsWorker{_blsWorker}, @@ -221,8 +221,7 @@ CQuorumManager::CQuorumManager(CBLSWorker& _blsWorker, CChainState& chainstate, quorumBlockProcessor{_quorumBlockProcessor}, m_qsnapman{qsnapman}, m_mn_activeman{mn_activeman}, - m_mn_sync{mn_sync}, - m_sporkman{sporkman} + m_mn_sync{mn_sync} { utils::InitQuorumsCache(mapQuorumsCache, false); quorumThreadInterrupt.reset(); @@ -366,7 +365,7 @@ void CQuorumManager::CheckQuorumConnections(CConnman& connman, const Consensus:: }); for (const auto& quorum : lastQuorums) { - if (utils::EnsureQuorumConnections(llmqParams, connman, m_dmnman, m_sporkman, m_qsnapman, + if (utils::EnsureQuorumConnections(llmqParams, connman, m_dmnman, m_qsnapman, m_dmnman.GetListAtChainTip(), quorum->m_quorum_base_block_index, myProTxHash, /* is_masternode = */ m_mn_activeman != nullptr)) { if (connmanQuorumsToDelete.erase(quorum->qc->quorumHash) > 0) { diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index f1dc5df1849f..121ecd2255d2 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -36,7 +36,6 @@ class CDBWrapper; class CEvoDB; class CMasternodeSync; class CNode; -class CSporkManager; namespace util { struct DbWrapperParams; } // namespace util @@ -243,7 +242,6 @@ class CQuorumManager CQuorumSnapshotManager& m_qsnapman; const CActiveMasternodeManager* const m_mn_activeman; const CMasternodeSync& m_mn_sync; - const CSporkManager& m_sporkman; mutable Mutex cs_map_quorums; mutable std::map> mapQuorumsCache GUARDED_BY(cs_map_quorums); @@ -269,7 +267,7 @@ class CQuorumManager CDKGSessionManager& _dkgManager, CEvoDB& _evoDb, CQuorumBlockProcessor& _quorumBlockProcessor, CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync, - const CSporkManager& sporkman, const util::DbWrapperParams& db_params); + const util::DbWrapperParams& db_params); ~CQuorumManager(); void Start(); diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 41c7613bdf4c..1886d573e458 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -46,14 +45,13 @@ void CSigSharesNodeState::RemoveSession(const uint256& signHash) CSigSharesManager::CSigSharesManager(CConnman& connman, CChainState& chainstate, CSigningManager& _sigman, PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, - const CQuorumManager& _qman, const CSporkManager& sporkman) : + const CQuorumManager& _qman) : m_connman{connman}, m_chainstate{chainstate}, sigman{_sigman}, m_peerman{peerman}, m_mn_activeman{mn_activeman}, - qman{_qman}, - m_sporkman{sporkman} + qman{_qman} { workInterrupt.reset(); } @@ -564,10 +562,6 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map().count(); for (auto& [_, signedSession] : signedSessions) { - if (!IsAllMembersConnectedEnabled(signedSession.quorum->params.type, m_sporkman)) { - continue; - } - if (signedSession.attempt >= signedSession.quorum->params.recoveryMembers) { continue; } @@ -848,14 +842,12 @@ void CSigSharesManager::SignPendingSigShares() auto sigShare = *opt_sigShare; ProcessSigShare(sigShare, pQuorum); - if (IsAllMembersConnectedEnabled(pQuorum->params.type, m_sporkman)) { - LOCK(cs); - auto& session = signedSessions[sigShare.GetSignHash()]; - session.sigShare = sigShare; - session.quorum = pQuorum; - session.nextAttemptTime = 0; - session.attempt = 0; - } + LOCK(cs); + auto& session = signedSessions[sigShare.GetSignHash()]; + session.sigShare = sigShare; + session.quorum = pQuorum; + session.nextAttemptTime = 0; + session.attempt = 0; } } } diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 58f41268339f..4d09550163a5 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -30,8 +30,6 @@ class CActiveMasternodeManager; class CNode; class CConnman; -class CDeterministicMN; -class CSporkManager; class PeerManager; namespace llmq @@ -305,7 +303,6 @@ class CSigSharesManager : public CRecoveredSigsListener PeerManager& m_peerman; const CActiveMasternodeManager& m_mn_activeman; const CQuorumManager& qman; - const CSporkManager& m_sporkman; int64_t lastCleanupTime{0}; std::atomic recoveredSigsCounter{0}; @@ -316,7 +313,7 @@ class CSigSharesManager : public CRecoveredSigsListener CSigSharesManager& operator=(const CSigSharesManager&) = delete; explicit CSigSharesManager(CConnman& connman, CChainState& chainstate, CSigningManager& _sigman, PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, - const CQuorumManager& _qman, const CSporkManager& sporkman); + const CQuorumManager& _qman); ~CSigSharesManager() override; void StartWorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 09d2d4fddc7b..b175355e03ab 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -730,29 +730,26 @@ uint256 DeterministicOutboundConnection(const uint256& proTxHash1, const uint256 } Uint256HashSet GetQuorumConnections(const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, - CQuorumSnapshotManager& qsnapman, const CSporkManager& sporkman, + CQuorumSnapshotManager& qsnapman, gsl::not_null pQuorumBaseBlockIndex, const uint256& forMember, bool onlyOutbound) { - if (IsAllMembersConnectedEnabled(llmqParams.type, sporkman)) { - auto mns = GetAllQuorumMembers(llmqParams.type, dmnman, qsnapman, pQuorumBaseBlockIndex); - Uint256HashSet result; + auto mns = GetAllQuorumMembers(llmqParams.type, dmnman, qsnapman, pQuorumBaseBlockIndex); + Uint256HashSet result; - for (const auto& dmn : mns) { - if (dmn->proTxHash == forMember) { - continue; - } - // Determine which of the two MNs (forMember vs dmn) should initiate the outbound connection and which - // one should wait for the inbound connection. We do this in a deterministic way, so that even when we - // end up with both connecting to each other, we know which one to disconnect - uint256 deterministicOutbound = DeterministicOutboundConnection(forMember, dmn->proTxHash); - if (!onlyOutbound || deterministicOutbound == dmn->proTxHash) { - result.emplace(dmn->proTxHash); - } + for (const auto& dmn : mns) { + if (dmn->proTxHash == forMember) { + continue; + } + // Determine which of the two MNs (forMember vs dmn) should initiate the outbound connection and which + // one should wait for the inbound connection. We do this in a deterministic way, so that even when we + // end up with both connecting to each other, we know which one to disconnect + uint256 deterministicOutbound = DeterministicOutboundConnection(forMember, dmn->proTxHash); + if (!onlyOutbound || deterministicOutbound == dmn->proTxHash) { + result.emplace(dmn->proTxHash); } - return result; } - return GetQuorumRelayMembers(llmqParams, dmnman, qsnapman, pQuorumBaseBlockIndex, forMember, onlyOutbound); + return result; } Uint256HashSet GetQuorumRelayMembers(const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, @@ -834,7 +831,7 @@ std::set CalcDeterministicWatchConnections(Consensus::LLMQType llmqType, } bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman& connman, - CDeterministicMNManager& dmnman, const CSporkManager& sporkman, + CDeterministicMNManager& dmnman, CQuorumSnapshotManager& qsnapman, const CDeterministicMNList& tip_mn_list, gsl::not_null pQuorumBaseBlockIndex, const uint256& myProTxHash, bool is_masternode) @@ -860,7 +857,7 @@ bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman& Uint256HashSet connections; Uint256HashSet relayMembers; if (isMember) { - connections = GetQuorumConnections(llmqParams, dmnman, qsnapman, sporkman, pQuorumBaseBlockIndex, myProTxHash, + connections = GetQuorumConnections(llmqParams, dmnman, qsnapman, pQuorumBaseBlockIndex, myProTxHash, true); relayMembers = GetQuorumRelayMembers(llmqParams, dmnman, qsnapman, pQuorumBaseBlockIndex, myProTxHash, true); } else { diff --git a/src/llmq/utils.h b/src/llmq/utils.h index 88ce5d4ee704..a2cc3a0a7c1e 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -40,7 +40,7 @@ std::vector GetAllQuorumMembers(Consensus::LLMQType llmqTy uint256 DeterministicOutboundConnection(const uint256& proTxHash1, const uint256& proTxHash2); Uint256HashSet GetQuorumConnections(const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, - CQuorumSnapshotManager& qsnapman, const CSporkManager& sporkman, + CQuorumSnapshotManager& qsnapman, gsl::not_null pQuorumBaseBlockIndex, const uint256& forMember, bool onlyOutbound); Uint256HashSet GetQuorumRelayMembers(const Consensus::LLMQParams& llmqParams, CDeterministicMNManager& dmnman, @@ -50,7 +50,7 @@ Uint256HashSet GetQuorumRelayMembers(const Consensus::LLMQParams& llmqParams, CD std::set CalcDeterministicWatchConnections(Consensus::LLMQType llmqType, gsl::not_null pQuorumBaseBlockIndex, size_t memberCount, size_t connectionCount); bool EnsureQuorumConnections(const Consensus::LLMQParams& llmqParams, CConnman& connman, - CDeterministicMNManager& dmnman, const CSporkManager& sporkman, + CDeterministicMNManager& dmnman, CQuorumSnapshotManager& qsnapman, const CDeterministicMNList& tip_mn_list, gsl::not_null pQuorumBaseBlockIndex, const uint256& myProTxHash, bool is_masternode); diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index c737d8a03d0b..5bf277f1b984 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -27,7 +27,7 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe mn_activeman, mn_sync, *llmq_ctx.isman)}, gov_signer{std::make_unique(connman, dmnman, govman, mn_activeman, chainman, mn_sync)}, shareman{std::make_unique(connman, chainman.ActiveChainstate(), *llmq_ctx.sigman, peerman, - mn_activeman, *llmq_ctx.qman, sporkman)}, + mn_activeman, *llmq_ctx.qman)}, ehf_sighandler{ std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *shareman, *llmq_ctx.qman)}, cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index fb837dd0f1c3..e7df85f92bbd 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -322,7 +322,6 @@ static RPCHelpMan quorum_dkgstatus() const ChainstateManager& chainman = EnsureChainman(node); const LLMQContext& llmq_ctx = EnsureLLMQContext(node); const CConnman& connman = EnsureConnman(node); - CHECK_NONFATAL(node.sporkman); int detailLevel = 0; if (!request.params[0].isNull()) { @@ -364,10 +363,10 @@ static RPCHelpMan quorum_dkgstatus() obj.pushKV("pindexTip", pindexTip->nHeight); auto allConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, - *llmq_ctx.qsnapman, *node.sporkman, + *llmq_ctx.qsnapman, pQuorumBaseBlockIndex, proTxHash, false); auto outboundConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, - *llmq_ctx.qsnapman, *node.sporkman, + *llmq_ctx.qsnapman, pQuorumBaseBlockIndex, proTxHash, true); std::map foundConnections; connman.ForEachNode([&](const CNode* pnode) { From d0e52c3fc5fc55603f340023a21ad28c2f10583e Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Thu, 13 Nov 2025 20:14:20 +0700 Subject: [PATCH 12/12] refactor: final cleanup of left-over of unused members in signing_shares --- src/llmq/signing_shares.cpp | 6 +----- src/llmq/signing_shares.h | 34 ---------------------------------- 2 files changed, 1 insertion(+), 39 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 1886d573e458..4adf7da7cbdf 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -34,10 +34,7 @@ void CSigShare::UpdateKey() void CSigSharesNodeState::RemoveSession(const uint256& signHash) { - if (const auto it = sessions.find(signHash); it != sessions.end()) { - sessions.erase(it); - } - requestedSigShares.EraseAllForSignHash(signHash); + sessions.erase(signHash); pendingIncomingSigShares.EraseAllForSignHash(signHash); } @@ -796,7 +793,6 @@ void CSigSharesManager::BanNode(NodeId nodeId) } auto& nodeState = it->second; - nodeState.requestedSigShares.Clear(); nodeState.banned = true; } diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index 4d09550163a5..92ea411cf928 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -137,16 +137,6 @@ class SigShareMap return &jt->second; } - T& GetOrAdd(const SigShareKey& k) - { - T* v = Get(k); - if (!v) { - Add(k, T()); - v = Get(k); - } - return *v; - } - const T* GetFirst() const { if (internalMap.empty()) { @@ -192,28 +182,6 @@ class SigShareMap internalMap.erase(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; - } - } - } - template void ForEach(F&& f) { @@ -244,7 +212,6 @@ class CSigSharesNodeState Uint256HashMap sessions; SigShareMap pendingIncomingSigShares; - SigShareMap requestedSigShares; bool banned{false}; @@ -265,7 +232,6 @@ class CSigSharesManager : public CRecoveredSigsListener { private: static constexpr int64_t SESSION_NEW_SHARES_TIMEOUT{60}; - static constexpr int64_t SIG_SHARE_REQUEST_TIMEOUT{5}; static constexpr int64_t EXP_SEND_FOR_RECOVERY_TIMEOUT{2000}; static constexpr int64_t MAX_SEND_FOR_RECOVERY_TIMEOUT{10000};