Skip to content

test: segregate wallet-mode coverage and deduplicate functional test runs #7497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 6 commits into from
Aug 1, 2026
Merged

test: segregate wallet-mode coverage and deduplicate functional test runs #7497

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b052e85
test: add wallet_dash_rpcs.py covering wallet-mode-divergent Dash RPC…
PastaPastaPasta Jul 30, 2026
60f0c38
test: run governance functional tests in a single wallet mode
PastaPastaPasta Jul 30, 2026
6980795
test: run feature_dip3_deterministicmns in a single wallet mode
PastaPastaPasta Jul 30, 2026
436bcfa
test: merge spork21 variant of feature_llmq_signing into a single run
PastaPastaPasta Jul 30, 2026
95996ba
test: skip non-spork23-gated banning section in simplepose spork23 va…
PastaPastaPasta Jul 30, 2026
850aeba
test: address review feedback on wallet-mode segregation
UdjinM6 Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 74 additions & 64 deletions test/functional/feature_llmq_signing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,18 @@ def set_test_params(self):

def add_options(self, parser):
self.add_wallet_options(parser)
parser.add_argument("--spork21", dest="spork21", default=False, action="store_true",
help="Test with spork21 enabled")

def run_test(self):

# The first part of this test runs with spork21 off, the second part
# enables it mid-test and exercises the spork21-only paths on quorums
# mined after that. spork21 being active from the very first DKG is
# covered by feature_llmq_connections.py.

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore first-DKG spork21 coverage

Keep a test where SPORK_21 is active before the first DKG: the cited replacement does not cover that scenario, because feature_llmq_connections.py mines three llmq_test quorums at lines 36, 58, and 70 before enabling SPORK_21 at lines 74-76, then proceeds with a rotating quorum. Consequently, regressions specific to initializing or forming the first quorum with the spork already active are no longer detected after removing the --spork21 variant.

AGENTS.md reference: AGENTS.md:L164-L164

Useful? React with 👍 / 👎.

@PastaPastaPasta PastaPastaPasta Jul 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right that feature_llmq_connections.py was the wrong citation — it mines three llmq_test quorums (lines 36, 58, 70) before enabling SPORK_21 at line 75, so it does not cover "spork21 active before the first DKG" for that quorum type. The PR description has been corrected.

However, the permutation itself is still covered in every CI job: feature_llmq_data_recovery.py enables SPORK_21_QUORUM_ALL_CONNECTED at the very top of run_test (line 164), on a fresh chain before its first mine_quorum() (line 173), and then forms both llmq_test and llmq_test_v17 quorums every DKG cycle with the spork active from before the first DKG ever. That exercises first-quorum formation under all-connected topology at least as thoroughly as the removed --spork21 variant did (two quorum types concurrently vs one). Additionally, feature_llmq_connections.py does cover the chain's first rotation (dip0024) DKGs under spork21 via mine_cycle_quorum() after the spork is enabled.

The only delta vs the removed variant is quorum size (5 members there vs 4/3 in data_recovery); the spork21 path (EnsureQuorumConnections all-connected + probes) has no first-DKG-specific branching that depends on member count, so we are not restoring the variant.

@thepastaclaw thepastaclaw Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 Nitpick: Correct the stale first-DKG coverage citation

feature_llmq_connections.py mines three llmq_test quorums at lines 36, 58, and 70 before enabling SPORK_21_QUORUM_ALL_CONNECTED at lines 74-76, so it does not cover SPORK21 being active for the first DKG. The actual replacement coverage is in feature_llmq_data_recovery.py, which enables SPORK21 at line 164 before its first mine_quorum() at line 173. Update this comment and amend the matching rationale in commit 436bcfaf339 so the source and commit history accurately explain why removing the original permutation is safe; the functional coverage itself remains intact.

Suggested change
# mined after that. spork21 being active from the very first DKG is
# covered by feature_llmq_connections.py.
# mined after that. spork21 being active from the very first DKG is
# covered by feature_llmq_data_recovery.py.

source: ['codex']

@thepastaclaw thepastaclaw Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in 850aeba — Correct the stale first-DKG coverage citation no longer present.

Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.

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
self.assert_qsendrecsigs_symmetric()

id = "0000000000000000000000000000000000000000000000000000000000000001"
msgHash = "0000000000000000000000000000000000000000000000000000000000000002"
msgHashConflict = "0000000000000000000000000000000000000000000000000000000000000003"
Expand Down Expand Up @@ -77,43 +73,12 @@ 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
# 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)
# Sign third share regularly, should result in recovered sig and conflict for msgHashConflict.
# The optional submit parameter is spork21-only and is exercised after spork21 is enabled below.
self.mninfo[2].get_node(self).quorum("sign", q_type, id, msgHash)

wait_for_sigs(True, False, True, 15)

if self.options.spork21:
mn.get_node(self).disconnect_p2ps()

# Test `quorum verify` rpc
node = self.mninfo[0].get_node(self)
recsig = node.quorum("getrecsig", q_type, id, msgHash)
Expand Down Expand Up @@ -177,29 +142,74 @@ 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.log.info("Enable SPORK_21_QUORUM_ALL_CONNECTED and mine one more quorum")
self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()
self.mine_quorum()

for mn in self.mninfo: # type: MasternodeInfo
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)
self.assert_qsendrecsigs_symmetric()

self.log.info("Test the optional submit parameter and QSIGSHARE P2P submission")
id = uint256_to_string(request_id + 1)
# Sign first two shares regularly, using the optional quorumHash parameter for the second one
self.mninfo[0].get_node(self).quorum("sign", q_type, id, msgHash)
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 the optional submit parameter, should result in recovered sig
# and conflict for msgHashConflict
# 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"])
# 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)

mn.get_node(self).disconnect_p2ps()

id = uint256_to_string(request_id + 2)

# 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 = 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)

def assert_qsendrecsigs_symmetric(self):
# If only one direction's QSENDRECSIGS arrives, the receiving side keeps
Expand Down
18 changes: 15 additions & 3 deletions test/functional/feature_llmq_simplepose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,22 @@ def run_test(self):
self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0)
self.wait_for_sporks_same()

# Lets isolate MNs one by one and verify that punishment/banning happens
self.test_banning(self.isolate_mn, 2)
if not self.options.disable_spork23:
# Lets isolate MNs one by one and verify that punishment/banning happens
self.test_banning(self.isolate_mn, 2)
Comment on lines +48 to +50

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep contribution-miss coverage with spork 23 disabled

When SPORK_23_QUORUM_POSE is disabled, this branch now skips the only contribution-miss banning check, so the suite no longer proves that ordinary DKG misses remain punishable despite the spork being off. The enabled run cannot catch a regression that accidentally gates MarkBadMember/PoSePunish on SPORK_23, while the remaining disabled-spork cases assert the opposite behavior for connection and protocol failures; retain at least a small disabled-spork contribution-miss case.

AGENTS.md reference: AGENTS.md:L164-L175

Useful? React with 👍 / 👎.


self.repair_masternodes(False)
self.repair_masternodes(False)
else:
# The contribution-miss ban path (MarkBadMember -> PoSePunish) is not
# gated on spork23 (spork23 only gates connection/proto-version checks
# and probes), so it behaves identically with spork23 disabled and is
# already covered by the spork23-enabled run of this test.
self.log.info("Skipping contribution-miss banning, not affected by spork23")
# Mine one quorum in normal conditions so that the sections below start
# from the same state as in the spork23-enabled run: an existing quorum
# and all masternodes healthy.
self.reset_probe_timeouts()
self.mine_quorum()

self.nodes[0].sporkupdate("SPORK_21_QUORUM_ALL_CONNECTED", 0)
self.wait_for_sporks_same()
Expand Down
8 changes: 3 additions & 5 deletions test/functional/test_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,10 @@
'feature_llmq_is_retroactive.py', # NOTE: needs dash_hash to pass
'feature_llmq_chainlocks.py', # NOTE: needs dash_hash to pass
'feature_llmq_simplepose.py', # NOTE: needs dash_hash to pass
'feature_llmq_simplepose.py --disable-spork23', # NOTE: needs dash_hash to pass
'feature_dip3_deterministicmns.py --legacy-wallet', # NOTE: needs dash_hash to pass
'feature_dip3_deterministicmns.py --descriptors', # NOTE: needs dash_hash to pass
'feature_masternode_payout_shares.py',
'feature_llmq_signing.py', # NOTE: needs dash_hash to pass
'feature_llmq_signing.py --spork21', # NOTE: needs dash_hash to pass
'feature_llmq_simplepose.py --disable-spork23', # NOTE: needs dash_hash to pass
'feature_llmq_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
Expand All @@ -131,9 +129,7 @@
'feature_dip4_coinbasemerkleroots.py', # NOTE: needs dash_hash to pass
'feature_mnehf.py', # NOTE: needs dash_hash to pass
'feature_masternode_params.py', # NOTE: needs dash_hash to pass
'feature_governance.py --legacy-wallet',
'feature_governance.py --descriptors',
'feature_governance_cl.py --legacy-wallet',
'feature_governance_cl.py --descriptors',
'rpc_verifyislock.py',
'feature_notifications.py',
Expand All @@ -146,6 +142,8 @@
'wallet_import_rescan.py --legacy-wallet',
'wallet_backup.py --legacy-wallet',
'wallet_backup.py --descriptors',
'wallet_dash_rpcs.py --legacy-wallet',
'wallet_dash_rpcs.py --descriptors',
'p2p_tx_download.py',
'wallet_avoidreuse.py --legacy-wallet',
'wallet_avoidreuse.py --descriptors',
Expand Down
Loading