test: stabilize governance superblock waits

[thepastaclaw]

Summary

Fixes #7328.

feature_governance.py can observe governance votes and triggers before every
node has finished updating the aggregate vote count, or before a trigger is in
the same funded state used by superblock mining. This made the test
intermittently fail on the final vote-count assertion and, locally, at the first
superblock payment check.

Changes

• Wait for each node's aggregate governance vote count to reach 25 before
  continuing.
• Extend have_trigger_for_height() so callers can wait on a specific
  governance vote signal.
• Add have_funded_trigger_for_height() and use it before mining superblocks,
  aligning the test wait with superblock processing's cached-funding condition.

Validation

PYTHONPYCACHEPREFIX=/private/tmp/tracker-2129-pycache \
  python3 -m py_compile \
    test/functional/feature_governance.py \
    test/functional/test_framework/governance.py

• feature_governance.py --descriptors: passed twice with PRNG seeds
  7310226623689214072 and 5676185504401363752.
• feature_governance.py --legacy-wallet: passed twice with PRNG seeds
  7488827763781849512 and 3282526354881486806.
• Pre-publication code-review gate: ship with zero findings.

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Final review complete — no blockers (commit d02864c)

[coderabbitai]

Walkthrough

This PR extends the governance testing framework to support multiple trigger signal types. The framework layer refactors have_trigger_for_height to accept a configurable signal parameter (defaulting to "valid") and introduces have_funded_trigger_for_height as a convenience helper for checking "funding" triggers. The feature test layer updates governance validation to use funded trigger checks at critical superblock and maturity-window heights, replaces a vote-count assertion with a polling loop, and adds explicit height computation before waiting for funded triggers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'test: stabilize governance superblock waits' is clear, concise, and directly describes the main change—making the governance test more robust to timing issues.
Linked Issues check	✅ Passed	The PR directly addresses the coding requirements of issue #7328: it waits for aggregate vote counts, extends have_trigger_for_height() to accept a signal parameter, and adds have_funded_trigger_for_height() to align with IsSuperblockTriggered()'s cached-funding condition.
Out of Scope Changes check	✅ Passed	All changes are scoped to addressing the intermittent test failures: vote-count waiting, trigger signal extension, and funded-trigger checks. No unrelated modifications are present.
Description check	✅ Passed	The description clearly matches the test stability and governance trigger-wait changes in the pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test/functional/feature_governance.py (1)

392-392: ⚡ Quick win

Bind the loop variable in the lambda to avoid closure issues.

The lambda captures sb_block_height by reference. While this works in practice because wait_until blocks the loop, it's safer to explicitly bind the variable to prevent potential issues if the code is refactored.

🔧 Proposed fix

-            self.wait_until(lambda: have_funded_trigger_for_height(self.nodes, sb_block_height))
+            self.wait_until(lambda sb_height=sb_block_height: have_funded_trigger_for_height(self.nodes, sb_height))

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/functional/feature_governance.py` at line 392, The lambda passed to
wait_until captures sb_block_height by reference which can cause closure issues;
change the lambda to bind the loop variable explicitly (e.g., give
sb_block_height as a default argument) so wait_until calls
have_funded_trigger_for_height with a bound value; update the call using
wait_until(lambda h=sb_block_height: have_funded_trigger_for_height(self.nodes,
h)) to ensure stable behavior when refactoring.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/functional/feature_governance.py`:
- Line 392: The lambda passed to wait_until captures sb_block_height by
reference which can cause closure issues; change the lambda to bind the loop
variable explicitly (e.g., give sb_block_height as a default argument) so
wait_until calls have_funded_trigger_for_height with a bound value; update the
call using wait_until(lambda h=sb_block_height:
have_funded_trigger_for_height(self.nodes, h)) to ensure stable behavior when
refactoring.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 279a4ccb-c991-4596-8357-10572720ff9d

📥 Commits

Reviewing files that changed from the base of the PR and between beca567 and 97ae2fb4ea2f72c04072dc3290067e80f69b312a.

📒 Files selected for processing (2)

• test/functional/feature_governance.py
• test/functional/test_framework/governance.py

[thepastaclaw]

Code Review

I verified the checked-out PR at fefdd42fc87ec7cc51b4cf97349390a8dc1b3b96 against the actual patch and surrounding test/RPC code. The change is consistent with Dash governance semantics: superblock mining uses the cached funding state, the new helper preserves the previous default valid behavior for existing callers, and I did not find any correctness issue that warrants a review comment.

Reviewed commit: fefdd42f

[thepastaclaw]

Addressed the CodeRabbit loop-capture suggestion in eddc30935c: bound the trigger wait lambda to the current sb_block_height and wrapped the call for readability. Pushed; CI is running again.

[thepastaclaw]

Closing during authored-PR cleanup. This governance wait hardening draft is not currently review-priority; can be revived if CI data shows this specific flake is still worth fixing.

[thepastaclaw]

Final validation — Codex + Sonnet

This PR fixes a real flake in feature_governance.py by waiting for the aggregate vote count to converge to 25 and by introducing have_funded_trigger_for_height, which polls gobject list ... funding — matching the fCachedFunding/IsSetCachedFunding() gate that governance/superblock.cpp:610 actually uses to select superblock triggers, rather than the AbsoluteYesCount-based 'valid' signal previously used. The default-argument closures (node=node, sb_height=sb_block_height) correctly avoid Python late-binding bugs, have_trigger_for_height stays backward compatible via its new signal='valid' default, and all reviewer lanes (Codex general/commit-history, Sonnet retry general/commit-history) independently verified the change and found no in-scope issues. No blocking, suggestion, or nitpick findings apply to this PR.

Note: GitHub does not allow authors to approve their own pull requests, so this exact verified APPROVE result is posted as a COMMENT.

Review provenance

• Codex reviewers: gpt-5.6-sol — general (completed), gpt-5.6-sol — dash-core-commit-history (completed)
• Verifier: claude-sonnet-5 — final-verifier
• Sonnet reviewers: claude-sonnet-5 — general (failed), claude-sonnet-5 — dash-core-commit-history (failed), claude-sonnet-5 — general (completed), claude-sonnet-5 — dash-core-commit-history (completed)