From 96f9aaa84d128a82ea515cc92b02943e420fb775 Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 18 Oct 2026 15:08:49 -0500 Subject: [PATCH 1/9] test(framework): don't wait for connections to PoSe-banned members wait_for_quorum_connections() holds out for every outbound connection of each session to an online member. A masternode PoSe-banned after its selection into a quarter of a rotating quorum stays in that quorum's composition while the quarter is reused, so llmq_test_dip0024 sessions keep listing it, but it joins no DKG. wait_for_quorum_phase() already leaves such members out of its online set; wait_for_quorum_connections() did not. feature_llmq_simplepose.py hits this in the second closed-port round: the first closed-port masternode is banned but still in a reused dip0024 quarter, the second one has it as an outbound relay member, and neither accepts inbound connections, so the wait timed out after 240s (seen on develop, with "connection failed for masternode ... too many times" logged every second for the banned one). Compute the online set once, without the banned members, as wait_for_quorum_phase() does. Bans can't change during the wait, which mines no blocks. Co-Authored-By: Claude Opus 5.5 --- test/functional/test_framework/test_framework.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index c7c722da7ccf..744c63839159 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -2080,6 +2080,10 @@ def wait_for_quorum_connections(self, quorum_hash, expected_connections, mninfos # wait_proc bumps mocktime by a second, keep it from outrunning the wall clock wait_proc = throttled(wait_proc) + # As in wait_for_quorum_phase: a PoSe-banned masternode can stay listed in a reused quarter of a + # rotating quorum, but it joins no DKG, so no connection to it is needed + online = {mn.proTxHash for mn in mninfos if not check_banned(self.nodes[0], mn)} + def check_quorum_connections(): def ret(): if wait_proc is not None: @@ -2090,7 +2094,6 @@ def ret(): # up as well, or its contributions are lost and wait_for_quorum_phase never sees them; # expected_connections only applies to the requested type found = False - online = {mn.proTxHash for mn in mninfos} for mn in mninfos: s = mn.get_node(self).quorum("dkgstatus") for qs in s["session"]: From cd50dd9a5314dd299ac8cf8a40c21a1aa99e39c6 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 14 Oct 2026 23:13:19 -0500 Subject: [PATCH 2/9] test: replace fixed waits for absent governance events with sync points feature_governance.py had five negative checks written as wait_until(, timeout=5, do_assert=False). Each always runs to its timeout, which CI scales by --timeout-factor=4, so they cost 100s of the ~150s the test takes there. Masternodes create and vote on superblock triggers only in GovernanceSigner::UpdatedBlockTip, run from the ActiveContext validation callback. Once a node has the tip and has drained its validation queue (syncwithvalidationinterfacequeue) it has finished acting on that tip, and anything it created is in its own local state at once. Each check now syncs that point and then asserts on every relevant node's local state instead of polling node0 for relayed state: - isolated winner: no peers, so after its queue is drained its trigger must still have exactly one YES vote. - isolated payee's turn: no non-isolated node holds any valid trigger (was: node0 sees none within 5s). - new winner's trigger: exactly one non-isolated masternode has a vote on it, read per masternode with gobject getcurrentvotes on its own node (was: node0's YesCount stays 1 for 5s). - every MN voted: no non-isolated node holds a trigger other than the winning one (was: node0 sees no second trigger within 5s). - after the re-sync: every node holds exactly the two known triggers (was: node0 sees no third trigger within 5s). The fixed waits also gave relay time to finish before the next step, which the following steps silently relied on. Those preconditions are now explicit waits that fail on timeout: - all non-isolated nodes know the winning trigger before the next block, so they vote YES on it there; - all non-isolated nodes have the four YES votes and have flushed their once-a-second relay queue (mockscheduler(1), then a validation-queue sync, which runs after the advanced relay task) before the isolated node reconnects. Otherwise it can be offered those votes before the trigger; CGovernanceVote's operator< never returns true, so the orphan vote map keeps one vote per object and the rest are lost until a later vote sync, failing the YES count check; - all nodes hold both triggers and their YES votes before the block on which they vote NO on the other trigger. These counts can't change on that block, so checking them earlier is equivalent. feature_governance.py --descriptors --timeout-factor=4 on the same machine, interleaved: 135/139/137/137s before, 39/42/39/39s after. The vote-count waits that run right after the governance re-sync advance mocktime by a second on each poll. A node that missed votes during the re-sync fetches them from its sync tick (SyncManager::ProcessTick -> RequestGovernanceObjectVotes), which acts only once MASTERNODE_SYNC_TICK_SECONDS (6s) of mocktime have passed since it last did, and the original test only looked at these counts after a bump and a block. Without the bump the wait timed out in about 1 of 16 local runs and once in a CI ASAN run. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_governance.py | 70 +++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 6aedace59c95..e544988f9ba8 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -58,6 +58,26 @@ def check_superblock(self): assert_equal(payments_found, 2) + def sync_governance_signers(self, nodes): + # Masternodes create and vote on triggers only from the UpdatedBlockTip validation callback, + # so once every node is at the same tip and has drained its validation queue they are done + # acting on that tip. + self.sync_blocks(nodes) + for node in nodes: + node.syncwithvalidationinterfacequeue() + + @staticmethod + def valid_triggers(node): + return set(node.gobject("list", "valid", "triggers").keys()) + + def count_own_trigger_votes(self, mns, trigger_hash): + # Each masternode's own vote is visible on its own node, without relying on relay. + count = 0 + for mn in mns: + collateral = self.nodes[0].protx("info", mn.proTxHash) + count += len(mn.get_node(self).gobject("getcurrentvotes", trigger_hash, collateral["collateralHash"], str(collateral["collateralIndex"]))) + return count + def run_test(self): self.log.info("Start testing...") governance_info = self.nodes[0].getgovernanceinfo() @@ -212,8 +232,9 @@ def run_test(self): self.wait_until(lambda: len(isolated.gobject("list", "valid", "triggers")) == 1, timeout=5) isolated_trigger_hash = list(isolated.gobject("list", "valid", "triggers").keys())[0] self.wait_until(lambda: list(isolated.gobject("list", "valid", "triggers").values())[0]['YesCount'] == 1, timeout=5) - more_votes = self.wait_until(lambda: list(isolated.gobject("list", "valid", "triggers").values())[0]['YesCount'] > 1, timeout=5, do_assert=False) - assert_equal(more_votes, False) + # The isolated node has no peers, so its own signer is the only possible source of more votes + self.sync_governance_signers([isolated]) + assert_equal(list(isolated.gobject("list", "valid", "triggers").values())[0]['YesCount'], 1) # Isolated node created a trigger and voted YES for it (16 votes total) assert_equal(isolated.gobject("count")["votes"], 16) # Non-isolated nodes don't see this (still 15 votes total) @@ -227,8 +248,11 @@ def run_test(self): self.check_superblockbudget(False) self.log.info("The 'winner' should submit new trigger and vote for it, but it's isolated so no triggers should be found") - has_trigger = self.wait_until(lambda: len(self.nodes[0].gobject("list", "valid", "triggers")) >= 1, timeout=5, do_assert=False) - assert_equal(has_trigger, False) + non_isolated = [node for node in self.nodes if node is not isolated] + non_isolated_mns = [mn for mn in self.mninfo if mn.nodeIdx != payee_idx] + self.sync_governance_signers(non_isolated) + for node in non_isolated: + assert_equal(self.valid_triggers(node), set()) self.log.info("Move 1 block inside the Superblock maturity window on non-isolated nodes") self.bump_mocktime(1) @@ -238,8 +262,11 @@ def run_test(self): self.wait_until(lambda: len(self.nodes[0].gobject("list", "valid", "triggers")) == 1, timeout=5) winning_trigger_hash = list(self.nodes[0].gobject("list", "valid", "triggers").keys())[0] self.wait_until(lambda: list(self.nodes[0].gobject("list", "valid", "triggers").values())[0]['YesCount'] == 1, timeout=5) - more_votes = self.wait_until(lambda: list(self.nodes[0].gobject("list", "valid", "triggers").values())[0]['YesCount'] > 1, timeout=5, do_assert=False) - assert_equal(more_votes, False) + self.sync_governance_signers(non_isolated) + # Every masternode must know the trigger before the next block so that it votes for it there + self.wait_until(lambda: all(winning_trigger_hash in self.valid_triggers(node) for node in non_isolated), timeout=5) + # Only the trigger's creator may have voted on it at this height + assert_equal(self.count_own_trigger_votes(non_isolated_mns, winning_trigger_hash), 1) # Non-isolated node created a trigger and voted YES for it (16 votes total) assert_equal(self.nodes[0].gobject("count")["votes"], 16) # Isolated node don't see this (still 16 votes total) @@ -258,8 +285,17 @@ def run_test(self): self.log.info("Every non-isolated MN should vote for the same trigger now, no new triggers should be created") self.wait_until(lambda: list(self.nodes[0].gobject("list", "valid", "triggers").values())[0]['YesCount'] == self.mn_count - 1, timeout=5) - more_triggers = self.wait_until(lambda: len(self.nodes[0].gobject("list", "valid", "triggers")) > 1, timeout=5, do_assert=False) - assert_equal(more_triggers, False) + self.sync_governance_signers(non_isolated) + for node in non_isolated: + assert self.valid_triggers(node) <= {winning_trigger_hash} + + # Let these votes finish relaying before the isolated node reconnects: if it is offered a vote + # before the trigger itself, the vote is only kept as an orphan and may be dropped. + self.wait_until(lambda: all(node.gobject("get", winning_trigger_hash)["FundingResult"]["YesCount"] == self.mn_count - 1 for node in non_isolated), timeout=5) + for node in non_isolated: + # Run the pending once-a-second relay flush now; mockscheduler returns after a validation queue + # sync, which is scheduled after the advanced relay task and so runs after it + node.mockscheduler(1) # All 4 non-isolated nodes voted YES for a trigger created by a non-isolated node earlier (19 votes total) assert_equal(self.nodes[0].gobject("count")["votes"], 19) # Isolated node don't see this (still 16 votes total) @@ -288,8 +324,22 @@ def sync_gov(node): self.log.info("Should see two triggers now") self.wait_until(lambda: len(isolated.gobject("list", "valid", "triggers")) == 2, timeout=5) self.wait_until(lambda: len(self.nodes[0].gobject("list", "valid", "triggers")) == 2, timeout=5) - more_triggers = self.wait_until(lambda: len(self.nodes[0].gobject("list", "valid", "triggers")) > 2, timeout=5, do_assert=False) - assert_equal(more_triggers, False) + # Every masternode must know both triggers before the next block so that it votes on them there + both_triggers = {winning_trigger_hash, isolated_trigger_hash} + self.wait_until(lambda: all(both_triggers <= self.valid_triggers(node) for node in self.nodes), timeout=5) + + def yes_votes_known(node, trigger_hash, yes_count): + # A node that missed votes during the re-sync fetches them from its sync tick, which acts only once + # MASTERNODE_SYNC_TICK_SECONDS of mocktime have passed since it last did, so keep mocktime moving + self.bump_mocktime(1) + return node.gobject("get", trigger_hash)["FundingResult"]["YesCount"] == yes_count + + for node in self.nodes: + self.wait_until(lambda: yes_votes_known(node, winning_trigger_hash, self.mn_count - 1), timeout=5) + self.wait_until(lambda: yes_votes_known(node, isolated_trigger_hash, 1), timeout=5) + self.sync_governance_signers(self.nodes) + for node in self.nodes: + assert_equal(self.valid_triggers(node), both_triggers) self.log.info("Move another block inside the Superblock maturity window") self.bump_mocktime(1) From 192c719e658143c6fd65771f79c2ab868b907234 Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 18 Oct 2026 18:08:17 -0500 Subject: [PATCH 3/9] test(framework): keep the offset rotating session in step in mine_quorum With quorum rotation active, the index 1 session of a cycle is based on the block after q and runs one block behind the sessions of q. When the sessions of q have just entered phase N, it is still in phase N-1, and the next phase block moves it on as well. mine_quorum()'s waits (all_types included) only cover sessions based on q, so that session got whatever time the next sync_blocks() happened to leave. That used to be about 1s, but since bitcoin#36439 sync_blocks() polls every 50ms, which leaves it about 50ms. Three things can go wrong in that window: - A member whose worker is still handling a phase when the following one has already started aborts the session ("aborting due unexpected phase change"). feature_llmq_simplepose.py's no-service miner hit this locally: four blocks in about 70ms. - A member still missing a complaint, justification or premature commitment when its phase ends marks the sender bad, and the commitment then punishes a valid masternode. - The maturity blocks can cross that session's mining window before its commitment has reached node 0, so it is mined as a null commitment and the rotated quorum is skipped. Add two helpers, both of which pass at once when there is no offset session: - wait_for_offset_quorum_phase(q, N), called after the barrier of each phase N: waits for every member of the offset session to be in phase N-1 (wait_for_quorum_phase with all_types), and by default also: - N = 2: for its relay connections (wait_for_quorum_connections with all_types). A contribution is relayed only once, and only to the members connected at that moment. - N = 3..6: until every member holds every phase N-1 message its members report having sent. With messages=False only the phase is awaited, for callers with a member offline, whose relays can be lost for good. - wait_for_offset_quorum_commitments(q), called before the maturity blocks: waits until no member's offset session is short of finalization, then waits for the commitments they produced to reach the mining node. A member whose session has finished no longer reports it, so finalization is checked as "nothing still behind", not "everyone in phase 6", and aborted sessions, which produce no commitment, are skipped. mine_quorum() calls both. When a masternode is offline (all_types is off, e.g. simplepose's isolation rounds) it waits for the offset session's phase only (messages=False), because messages relayed through the offline member can be lost. feature_llmq_simplepose.py's mine_quorum_less_checks() keeps every masternode online and calls both with messages. mine_quorum_no_service() takes one masternode offline after phase 2, which breaks the relay graph of the offset session. It does the full phase-2 wait while every masternode is still online, then waits for the phase only (messages=False) on the remaining members, and for their offset commitments before its maturity blocks. These are only extra waits, and they still fail by timeout. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_llmq_simplepose.py | 14 +++++ .../test_framework/test_framework.py | 51 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 6fa9e8f3f1cb..62f45673a75e 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -179,6 +179,7 @@ def mine_quorum_no_service(self, mn, others, relay, services, expected_complaint self.log.info("Waiting for phase 2 (contribute)") self.wait_for_quorum_phase(q, 2, len(self.mninfo), "receivedContributions", self.llmq_size, self.mninfo, all_types=True) + self.wait_for_offset_quorum_phase(q, 2, self.mninfo) self.log.info("Replacing the connections to the masternode with stand-ins") mn.get_node(self).setnetworkactive(False) @@ -194,21 +195,25 @@ def mine_quorum_no_service(self, mn, others, relay, services, expected_complaint self.log.info("Waiting for phase 3 (complain)") self.wait_for_quorum_phase(q, 3, len(others), "receivedComplaints", expected_complaints, others) + self.wait_for_offset_quorum_phase(q, 3, others, messages=False) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 4 (justify)") self.wait_for_quorum_phase(q, 4, len(others), "receivedJustifications", 0, others) + self.wait_for_offset_quorum_phase(q, 4, others, messages=False) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 5 (commit)") self.wait_for_quorum_phase(q, 5, len(others), "receivedPrematureCommitments", len(others), others) + self.wait_for_offset_quorum_phase(q, 5, others, messages=False) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 6 (mining)") self.wait_for_quorum_phase(q, 6, len(others), None, 0, others) + self.wait_for_offset_quorum_phase(q, 6, others, messages=False) self.log.info("Waiting final commitment") self.wait_for_quorum_commitment(q, others) @@ -225,6 +230,8 @@ def mine_quorum_no_service(self, mn, others, relay, services, expected_complaint self.wait_for_quorum_list(q, nodes) assert_equal(q, self.nodes[0].quorum("list", 1)["llmq_test"][0]) + self.wait_for_offset_quorum_commitments(q, others) + # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes)) @@ -263,22 +270,27 @@ def mine_quorum_less_checks(self, expected_members, mninfos_online): self.log.info("Waiting for phase 2 (contribute)") self.wait_for_quorum_phase(q, 2, expected_members, "receivedContributions", expected_members, mninfos_online, all_types=True) + self.wait_for_offset_quorum_phase(q, 2, mninfos_online) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 3 (complain)") self.wait_for_quorum_phase(q, 3, expected_members, "receivedComplaints", None, mninfos_online, all_types=True) + self.wait_for_offset_quorum_phase(q, 3, mninfos_online) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 4 (justify)") self.wait_for_quorum_phase(q, 4, expected_members, "receivedJustifications", None, mninfos_online, all_types=True) + self.wait_for_offset_quorum_phase(q, 4, mninfos_online) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 5 (commit)") self.wait_for_quorum_phase(q, 5, expected_members, "receivedPrematureCommitments", expected_members, mninfos_online, all_types=True) + self.wait_for_offset_quorum_phase(q, 5, mninfos_online) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 6 (mining)") self.wait_for_quorum_phase(q, 6, expected_members, None, 0, mninfos_online, all_types=True) + self.wait_for_offset_quorum_phase(q, 6, mninfos_online) self.log.info("Waiting final commitment") self.wait_for_quorum_commitment(q, mninfos_online) @@ -298,6 +310,8 @@ def mine_quorum_less_checks(self, expected_members, mninfos_online): assert_equal(q, new_quorum) quorum_info = self.nodes[0].quorum("info", 100, new_quorum) + self.wait_for_offset_quorum_commitments(q, mninfos_online) + # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions self.bump_mocktime(8) self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes)) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 744c63839159..ee9bba3e7792 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -2247,6 +2247,51 @@ def check_dkg_session(): self.wait_until(check_dkg_session, timeout=timeout) + def offset_quorum_hash(self, quorum_hash): + """With rotation active, the index 1 session of a cycle is based on the block after quorum_hash and runs + one block behind the sessions of quorum_hash.""" + return self.nodes[0].getblockhash(self.nodes[0].getblock(quorum_hash)["height"] + 1) + + def wait_for_offset_quorum_phase(self, quorum_hash, phase, mninfos, messages=True): + """Called once the sessions of quorum_hash are in phase (2-6), when the offset session is still in + phase - 1 and the next phase block moves it on as well. A member whose worker has not caught up by then + aborts the session, so wait for every member of mninfos to be in phase - 1. With messages, also wait + until it is ready to move on: in phase 1 its relay connections must be up, because a contribution is + relayed only once and only to the members connected at that moment, and in phases 2-5 every member has + to hold every message of the phase that its members report having sent. Without messages (some member + is offline, so relays can be lost for good) only the phase is awaited. Passes at once without an offset + session.""" + offset_hash = self.offset_quorum_hash(quorum_hash) + received = {2: "receivedContributions", 3: "receivedComplaints", 4: "receivedJustifications", 5: "receivedPrematureCommitments"} + check_received = received.get(phase - 1) if messages else None + self.wait_for_quorum_phase(offset_hash, phase - 1, 0, check_received, None, mninfos, all_types=True) + if phase == 2 and messages: + # Only the rotating llmq_test_dip0024 has a session based on the offset block + has_offset_session = any(qs["status"]["quorumHash"] == offset_hash + for mn in mninfos for qs in mn.get_node(self).quorum("dkgstatus")["session"]) + if has_offset_session: + self.wait_for_quorum_connections(offset_hash, 0, mninfos, llmq_type_name="llmq_test_dip0024", + wait_proc=lambda: self.bump_mocktime(1), all_types=True) + + def wait_for_offset_quorum_commitments(self, quorum_hash, mninfos, timeout=15): + """The offset session finalizes with the block that mines the final commitments of quorum_hash, so wait + until no member's offset session is short of finalization and the commitments they produced reached + the mining node before mining past its window. A member whose session is done no longer reports it, and + an aborted session (e.g. with a member offline) produces no commitment. Passes at once without an offset + session.""" + offset_hash = self.offset_quorum_hash(quorum_hash) + + def check_finalized(): + for mn in mninfos: + for qs in mn.get_node(self).quorum("dkgstatus")["session"]: + status = qs["status"] + if status["quorumHash"] == offset_hash and not status["aborted"] and status["phase"] < 6: + return False + return True + + self.wait_until(check_finalized, timeout=timeout) + self.wait_for_quorum_commitments_on_miner(offset_hash, mninfos) + def node_quorum_commitment_types(self, node, quorum_hash): s = node.quorum("dkgstatus") return {c["llmqType"] for c in s.get("minableCommitments", []) @@ -2365,26 +2410,31 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec self.log.info("Waiting for phase 2 (contribute)") self.wait_for_quorum_phase(q, 2, expected_members, "receivedContributions", expected_contributions, mninfos_online, llmq_type_name=llmq_type_name, all_types=all_types) + self.wait_for_offset_quorum_phase(q, 2, mninfos_online, messages=all_types) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 3 (complain)") self.wait_for_quorum_phase(q, 3, expected_members, "receivedComplaints", expected_complaints, mninfos_online, llmq_type_name=llmq_type_name, all_types=all_types) + self.wait_for_offset_quorum_phase(q, 3, mninfos_online, messages=all_types) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 4 (justify)") self.wait_for_quorum_phase(q, 4, expected_members, "receivedJustifications", expected_justifications, mninfos_online, llmq_type_name=llmq_type_name, all_types=all_types) + self.wait_for_offset_quorum_phase(q, 4, mninfos_online, messages=all_types) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 5 (commit)") self.wait_for_quorum_phase(q, 5, expected_members, "receivedPrematureCommitments", expected_commitments, mninfos_online, llmq_type_name=llmq_type_name, all_types=all_types) + self.wait_for_offset_quorum_phase(q, 5, mninfos_online, messages=all_types) self.move_blocks(nodes, 2) self.log.info("Waiting for phase 6 (mining)") self.wait_for_quorum_phase(q, 6, expected_members, None, 0, mninfos_online, llmq_type_name=llmq_type_name, all_types=all_types) + self.wait_for_offset_quorum_phase(q, 6, mninfos_online, messages=all_types) self.log.info("Waiting final commitment") self.wait_for_quorum_commitment(q, mninfos_online, llmq_type=llmq_type) @@ -2407,6 +2457,7 @@ def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connec self.verify_upcoming_dkg_predictions(predictions, quorum_info) if not skip_maturity: + self.wait_for_offset_quorum_commitments(q, mninfos_online) # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions self.generate(self.nodes[0], 8, sync_fun=lambda: self.sync_blocks(nodes)) From f34caab97c85b63a130b668662ab1c421f674b0e Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 14 Oct 2026 23:41:14 -0500 Subject: [PATCH 4/9] test: share no-change windows in feature_llmq_signing.py assert_sigs_nochange() is a negative check that always runs to its timeout: 3s, which CI's --timeout-factor=4 turns into 12s. The test ran five of them (six with --spork21), 60s/72s of the 110s/123s it takes locally with the CI timeout factor. Every state it watches only moves one way here: a recovered sig or a conflict, once created, is kept until the age-based cleanup removes it, which this test only triggers deliberately by moving mocktime days ahead; and llmq_test sigs have no other removal path (only InstantSend sigs are truncated). So a change caused by a step is still visible in any later window, as long as no step in between legitimately changes that state. - The sub-threshold signing steps (first share, sign with a wrong quorum hash, second share and, with spork21, the submit=false shares) used to get one window each. Now each step is followed by an immediate check_sigs() and they share one window after the last of them, so every step is still watched for at least the full window before the threshold share is sent. - The window after the second quorum is mined (id 1 unchanged) is replaced by an immediate check, and id 1 is added to the window that already follows the fourth and fifth quorums. Nothing in between touches id 1, so its state is watched from the second quorum to the end of that window, longer than before. No check is dropped and each step still gets at least one full window. Locally with --timeout-factor=4: --spork21 123s -> 76s, plain 110s -> 76s. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_llmq_signing.py | 40 ++++++++++++++++--------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/test/functional/feature_llmq_signing.py b/test/functional/feature_llmq_signing.py index ee05f657adfb..7113b244ed6b 100755 --- a/test/functional/feature_llmq_signing.py +++ b/test/functional/feature_llmq_signing.py @@ -49,46 +49,56 @@ def run_test(self): msgHash = "0000000000000000000000000000000000000000000000000000000000000002" msgHashConflict = "0000000000000000000000000000000000000000000000000000000000000003" - def check_sigs(hasrecsigs, isconflicting1, isconflicting2): + def check_sigs(hasrecsigs, isconflicting1, isconflicting2, sig_id=None): + sig_id = sig_id or id for mn in self.mninfo: # type: MasternodeInfo - if mn.get_node(self).quorum("hasrecsig", q_type, id, msgHash) != hasrecsigs: + if mn.get_node(self).quorum("hasrecsig", q_type, sig_id, msgHash) != hasrecsigs: return False - if mn.get_node(self).quorum("isconflicting", q_type, id, msgHash) != isconflicting1: + if mn.get_node(self).quorum("isconflicting", q_type, sig_id, msgHash) != isconflicting1: return False - if mn.get_node(self).quorum("isconflicting", q_type, id, msgHashConflict) != isconflicting2: + if mn.get_node(self).quorum("isconflicting", q_type, sig_id, msgHashConflict) != isconflicting2: return False return True def wait_for_sigs(hasrecsigs, isconflicting1, isconflicting2, timeout): self.wait_until(lambda: check_sigs(hasrecsigs, isconflicting1, isconflicting2), timeout = timeout) - def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): - assert not self.wait_until(lambda: not check_sigs(hasrecsigs, isconflicting1, isconflicting2), timeout = timeout, do_assert = False) + def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout, other_ids=()): + def unchanged(): + return all(check_sigs(hasrecsigs, isconflicting1, isconflicting2, sig_id) for sig_id in (id, *other_ids)) + assert not self.wait_until(lambda: not unchanged(), timeout = timeout, do_assert = False) + + # Recovering, conflicting or removing a recovered sig can't be undone during this test without a + # deliberate step, so a change caused by any earlier step is still visible in a later window. A step + # that must cause no change is therefore checked at once, and its window is shared with the next + # step that must cause no change either. # Initial state wait_for_sigs(False, False, False, 1) # Sign first share without any optional parameter, should not result in recovered sig self.mninfo[0].get_node(self).quorum("sign", q_type, id, msgHash) - assert_sigs_nochange(False, False, False, 3) + assert check_sigs(False, False, False) # Sign second share and test optional quorumHash parameter, should not result in recovered sig # 1. Providing an invalid quorum hash should fail and cause no changes for sigs assert not self.mninfo[1].get_node(self).quorum("sign", q_type, id, msgHash, msgHash) - assert_sigs_nochange(False, False, False, 3) + assert check_sigs(False, False, False) # 2. Providing a valid quorum hash should succeed and cause no changes for sigss 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 - # and conflict for msgHashConflict + assert check_sigs(False, False, False) if self.options.spork21: + # Test optional submit parameter if spork21 is enabled # 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) + # Window shared by every step above + assert_sigs_nochange(False, False, False, 3) + # Sign third share, should result in recovered sig and conflict for msgHashConflict + if self.options.spork21: # 3. Sending the sig share received from RPC to the recovery member through P2P interface, should result # in a recovered sig sig_share = CSigShare() @@ -134,7 +144,9 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): # Mine one more quorum, so that we have 2 active ones, nothing should change self.mine_quorum() - assert_sigs_nochange(True, False, True, 3) + # The window for this is shared with the one after the next quorums below + assert check_sigs(True, False, True) + first_id = id # Create a recovered sig for the oldest quorum i.e. the active quorum which will be moved # out of the active set when a new quorum appears @@ -162,7 +174,7 @@ def assert_sigs_nochange(hasrecsigs, isconflicting1, isconflicting2, timeout): # Mine 2 more quorums, so that the one used for the the recovered sig should become inactive, nothing should change self.mine_quorum() self.mine_quorum() - assert_sigs_nochange(True, False, True, 3) + assert_sigs_nochange(True, False, True, 3, other_ids=[first_id]) # fast forward until 0.5 days before cleanup is expected, recovered sig should still be valid self.bump_mocktime(recsig_time + int(60 * 60 * 24 * 6.5) - self.mocktime, update_schedulers=False) From 934b8c9f1b0f335ffad36ac674f3589a24831c4b Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 14 Oct 2026 23:47:28 -0500 Subject: [PATCH 5/9] test: restart simplepose's repaired masternodes in parallel repair_masternodes stopped and started every banned or punished masternode one after the other. Submit all ProUpServTxes first, then restart those masternodes with restart_masternodes(), which stops them together, starts them in parallel and connects each to the controller, exactly what the loop did per masternode. The transactions only wait in the controller's mempool until the block mined below, so it does not matter whether a masternode was restarted before or after another one's transaction was created. Coverage: the same masternodes are repaired (same check_banned / check_punished selection, evaluated on the same chain state since nothing is mined in between), restarted with the same arguments and reconnected to the controller; all later assertions are unchanged. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_llmq_simplepose.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/functional/feature_llmq_simplepose.py b/test/functional/feature_llmq_simplepose.py index 62f45673a75e..5bd61fe61723 100755 --- a/test/functional/feature_llmq_simplepose.py +++ b/test/functional/feature_llmq_simplepose.py @@ -388,16 +388,16 @@ def test_banning(self, invalidate_proc, expected_connections=None): def repair_masternodes(self, restart): self.log.info("Repairing all banned and punished masternodes") - for mn in self.mninfo: # type: MasternodeInfo - if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn): - addr = self.nodes[0].getnewaddress() - self.nodes[0].sendtoaddress(addr, 0.1) - mn.update_service(self.nodes[0], submit=True, fundsAddr=addr) - if restart: - self.stop_node(mn.nodeIdx) - self.start_masternode(mn) - else: - mn.get_node(self).setnetworkactive(True) + repaired = [mn for mn in self.mninfo if check_banned(self.nodes[0], mn) or check_punished(self.nodes[0], mn)] + for mn in repaired: + addr = self.nodes[0].getnewaddress() + self.nodes[0].sendtoaddress(addr, 0.1) + mn.update_service(self.nodes[0], submit=True, fundsAddr=addr) + if restart: + self.restart_masternodes(mns=repaired) + else: + for mn in repaired: + mn.get_node(self).setnetworkactive(True) self.connect_nodes(mn.nodeIdx, 0) # syncing blocks only since node 0 has txes waiting to be mined From e7e2fe3d934b03e7890d129ccf42bcb879ade889 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 14 Oct 2026 23:51:11 -0500 Subject: [PATCH 6/9] test: replace data_recovery's fixed negative windows with barriers and state checks With recovery disabled the test slept 10 seconds after triggering recovery to show that the invalid members stay invalid, then waited 5 seconds (wait_until timeout, so 20 seconds with CI's --timeout-factor=4) to show that a new block gets no ChainLock. Both only observed that nothing happened within a window. Recovery: data recovery threads are only ever started by NetQuorum::TriggerQuorumDataRecoveryThreads (the only caller of StartSkShareRecoveryThread / TryStartVvecSyncThread), which runs in every block's UpdatedBlockTip validation callback and, unless -llmq-data-recovery=0 makes it return first, logs "TriggerQuorumDataRecoveryThreads -- Process block " before it starts any. Mine the trigger block (and the ChainLock block below), sync blocks and every node's validation interface queue so the callbacks have run, and assert on every recovery-disabled masternode that the callback reached the trigger for both blocks (it logs "llmqType[100] h[] keeping mn quorum connections" just before calling it) and that the trigger logged nothing. Then assert as before that their data is still missing. As a positive control, restart_mns now waits for every masternode restarted with recovery enabled to log the trigger line for its trigger block, so the negative check can not pass vacuously if the message or the trigger moves. ChainLock: mine the block behind the same barrier, wait until every masternode logged that it tries to sign it (ChainLockSigner), ask `quorum selectquorum` which llmq_test quorum signs its CLSIG request id (the same SelectQuorumForSigning the signers and the CLSIG verification use), assert that fewer of its members than the signing threshold still hold their secret key share (test_mn_quorum_data), wait until each of those has created its sig share for this block, and assert that no node has a ChainLock for it. Only members with a key share can create sig shares, and recovery needs threshold many, so no ChainLock can form later either; the old check only looked for one during the window. Saves the 10 s sleep and the 5 s (CI: 20 s) window. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_llmq_data_recovery.py | 72 +++++++++++++++++-- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/test/functional/feature_llmq_data_recovery.py b/test/functional/feature_llmq_data_recovery.py index 22737c44c9bc..35470e4b1233 100755 --- a/test/functional/feature_llmq_data_recovery.py +++ b/test/functional/feature_llmq_data_recovery.py @@ -3,8 +3,11 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. +from contextlib import ExitStack +import struct import time from typing import List, Optional +from test_framework.messages import hash256, ser_string from test_framework.p2p import logger from test_framework.test_framework import ( DashTestFramework, @@ -24,6 +27,9 @@ llmq_test_v17 = 102 llmq_type_strings = {llmq_test: 'llmq_test', llmq_test_v17: 'llmq_test_v17'} +# Logged by every block's recovery trigger, unless -llmq-data-recovery=0 turns it into a no-op +RECOVERY_TRIGGER_LOG = "TriggerQuorumDataRecoveryThreads -- Process block" + class QuorumDataRecoveryTest(DashTestFramework): def add_options(self, parser): @@ -56,7 +62,11 @@ def restart_mns(self, mns=None, exclude=None, reindex=False, qvvec_sync=None, qd if qdata_recovery_enabled: # trigger recovery threads and wait for them to start - self.generate(self.nodes[0], 1, sync_fun=self.no_op) + restarted = [mn for mn in (self.mninfo if mns is None else mns) if mn not in (exclude or [])] + log_positions = self.debug_log_positions(restarted) + block_hash = self.generate(self.nodes[0], 1, sync_fun=self.no_op)[0] + # The negative check in run_test relies on this message, so make sure it is the one the trigger logs + self.wait_for_log_line(restarted, log_positions, f"{RECOVERY_TRIGGER_LOG} {block_hash}") self.bump_mocktime(self.quorum_data_thread_request_timeout_seconds + 1) time.sleep(1) @@ -75,6 +85,24 @@ def test_mns(self, quorum_type_in, quorum_hash_in, valid_mns=None, all_mns=None, assert not self.test_mn_quorum_data(mn, quorum_type_in, quorum_hash_in, test_secret, False) self.wait_for_quorum_data(valid_mns, quorum_type_in, quorum_hash_in, test_secret, expect_secret, recover, timeout) + def debug_log_positions(self, mns): + return {mn.proTxHash: mn.get_node(self).debug_log_size(encoding="utf-8") for mn in mns} + + def wait_for_log_line(self, mns, log_positions, *parts): + """Wait until every one of mns logged a line containing all parts since log_positions was taken""" + def has_line(mn): + with open(mn.get_node(self).debug_log_path, encoding="utf-8", errors="replace") as debug_log: + debug_log.seek(log_positions[mn.proTxHash]) + return any(all(part in line for part in parts) for line in debug_log) + + self.wait_until(lambda: all(has_line(mn) for mn in mns)) + + def sync_validation(self): + """Every node has the same tip and has run its validation callbacks for it""" + self.sync_blocks() + for node in self.nodes: + node.syncwithvalidationinterfacequeue() + def get_mn(self, protx_hash) -> Optional[MasternodeInfo]: for mn in self.mninfo: # type: MasternodeInfo if mn.proTxHash == protx_hash: @@ -95,6 +123,27 @@ def get_subset_only_in_left(self, quorum_members_left, quorum_members_right) -> quorum_members_subset.remove(mn) return quorum_members_subset + def test_no_chainlock(self, node): + """Mine a block and show that the quorum that has to sign it can not: every masternode tried to sign it, + fewer members of the signing quorum than the signing threshold hold their secret key share, each of + those has signed the block, and no ChainLock formed""" + height = node.getblockcount() + 1 + request_id = hash256(ser_string(b"clsig") + struct.pack(" Date: Sun, 18 Oct 2026 18:08:17 -0500 Subject: [PATCH 7/9] test: mine a fresh block before expecting data_recovery's ChainLock After the recovery-enabled restart, test_mns(recover=True) mines a block on every other request expiry until every member holds its quorum data again. The test then waits for a ChainLock on the tip as it is. That tip can come from the recovery loop and be signed before the last members have their key shares back. Signing is not retried for the same tip, and nothing moves mocktime or mines another block during the wait, so no ChainLock forms. Seen locally: the members signed the last loop block while 2 of 4 had recovered, and the test saw 4/4 about 0.1s later. Since sync_blocks() polls every 50ms (bitcoin#36439), the loop returns sooner after its last block, which makes this likelier. Mine one block after recovery and wait for its ChainLock, as the comment above the check already describes ("Mining a block should result in a chainlock now"). Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_llmq_data_recovery.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_llmq_data_recovery.py b/test/functional/feature_llmq_data_recovery.py index 35470e4b1233..435783059061 100755 --- a/test/functional/feature_llmq_data_recovery.py +++ b/test/functional/feature_llmq_data_recovery.py @@ -252,7 +252,9 @@ def run_test(self): # which trigger CQuorumManger::TriggerQuorumDataRecoveryThreads() self.test_mns(llmq_test, quorum_hash_recover, valid_mns=member_mns_recover_test, recover=True) self.test_mns(llmq_test_v17, quorum_hash_recover, valid_mns=member_mns_recover_v17, recover=True) - # Mining a block should result in a chainlock now because the quorum should be healed + # Mining a block should result in a chainlock now because the quorum should be healed. The tip that the + # recovery loop mined last may have been signed before every member held its key share again. + self.generate(node, 1, sync_fun=self.sync_validation) self.wait_for_chainlocked_block_all_nodes(node.getbestblockhash()) logger.info("Test -llmq-qvvec-sync command line parameter") # Run with one type separated and then both possible (for regtest) together, both calls generate new quorums From aede93e949918e18498d02272f7770d308b5c7e2 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 27 Oct 2026 09:08:17 -0500 Subject: [PATCH 8/9] test: make dashd hang up promptly on feature_block's invalid blocks 51 of feature_block's block submissions expect the node to disconnect the peer that sent an invalid block. dashd marks the peer for disconnection right after rejecting the block, but its socket thread only acts on that flag once it wakes up, which on an otherwise idle node is its 500ms poll timeout. Each of these submissions therefore took ~0.5s of pure waiting (26s of the 61s local run). The helper peer now keeps sending pings (at most one per 50ms) while it waits for the disconnect. Each ping arriving wakes the node's socket thread, which closes the connection as soon as the disconnect flag is set. Coverage: the wait still requires the node to close the connection, with the same timeout, and send_blocks_and_test() still checks the reject reason in debug.log and that the tip did not move before the helper reconnects. The pings follow the block on the same connection, so the node processes the block first; once it has decided to disconnect, ProcessMessages() skips the peer's remaining messages, so the pings cannot affect the outcome. A node that wrongly accepted the block keeps the connection open and answers the pings, and the wait times out as before; the rate limit keeps that failure from flooding the logs with ping/pong traffic. The pings also wake P2PInterface.wait_until() early, since bitcoin#36440 made p2p_lock notify waiters on every delivered message. Co-Authored-By: Claude Opus 5.5 --- test/functional/feature_block.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 9f6023d963f0..406ba0af9bb0 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -22,6 +22,7 @@ CTxIn, CTxOut, SEQUENCE_FINAL, + msg_ping, uint256_from_compact, uint256_from_str, ) @@ -81,6 +82,28 @@ def normal_serialize(self): DUPLICATE_COINBASE_SCRIPT_SIG = b'\x01\x78' # Valid for block at height 120 +class P2PBlockStore(P2PDataStore): + def wait_for_disconnect(self, *, timeout=60): + # dashd's socket thread closes a peer marked for disconnection only once it wakes up, + # which takes up to its 500ms poll timeout when no data comes in. Keep pinging the node + # so it wakes up and hangs up as soon as it has decided to; the hang-up is still awaited. + # At most one ping per 50ms, so a node that wrongly stays connected is not flooded. + last_ping = 0 + + def disconnected(): + nonlocal last_ping + if not self.is_connected: + return True + if time.monotonic() - last_ping >= 0.05: + last_ping = time.monotonic() + try: + self.send_message(msg_ping(nonce=0)) + except IOError: + pass # closed in the meantime; seen on the next check + return False + self.wait_until(disconnected, timeout=timeout, check_connected=False) + + class FullBlockTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 @@ -1278,7 +1301,7 @@ def run_test(self): # Don't use v2transport for the large reorg, which is too slow with the unoptimized python ChaCha20 implementation if self.options.v2transport: self.nodes[0].disconnect_p2ps() - self.helper_peer = self.nodes[0].add_p2p_connection(P2PDataStore(), supports_v2_p2p=False) + self.helper_peer = self.nodes[0].add_p2p_connection(P2PBlockStore(), supports_v2_p2p=False) self.log.info("Test a re-org of ~2 days' worth of blocks (1088 blocks)") self.move_tip(88) @@ -1419,7 +1442,7 @@ def bootstrap_p2p(self, timeout=10): """Add a P2P connection to the node. Helper to connect and wait for version handshake.""" - self.helper_peer = self.nodes[0].add_p2p_connection(P2PDataStore()) + self.helper_peer = self.nodes[0].add_p2p_connection(P2PBlockStore()) # We need to wait for the initial getheaders from the peer before we # start populating our blockstore. If we don't, then we may run ahead # to the next subtest before we receive the getheaders. We'd then send From 611e8464394259f2056efbd609c0325082dcc3f2 Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 18 Oct 2026 17:01:11 -0500 Subject: [PATCH 9/9] test: let p2p_blocksonly's node take the relayed tx before moving mocktime The relay-permission check sends the tx from first_peer and immediately bumps mocktime by 60s so that the node sends its inv to second_peer. Sending over P2P is asynchronous, so the node can process the tx after the bump. When it does, the inv is queued behind an inv-send time that SendMessages() already scheduled from the bumped mocktime, mocktime never moves again, and wait_for_tx() times out. linux64_tsan-test hit this on all three attempts: the node logged "received: tx" at the bumped mocktime, one millisecond after setmocktime. Ping first_peer before the bump, so that the node has accepted the tx and queued its inv when mocktime moves. The RPC case above needs nothing: sendrawtransaction() returns only once the tx is in the mempool. Co-Authored-By: Claude Opus 5.5 --- test/functional/p2p_blocksonly.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index ebfdf87c393f..97a292258cec 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -71,6 +71,8 @@ def blocksonly_mode_tests(self): first_peer.send_message(msg_tx(tx)) self.log.info('Check that the peer with relay-permission is still connected after sending the transaction') assert_equal(first_peer.is_connected, True) + # Let the node take the tx before mocktime moves, or its relay is scheduled past the bump + first_peer.sync_with_ping() self.bump_mocktime(60) second_peer.wait_for_tx(txid) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1)