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 IsSuperblockTriggered()'s cached-funding
  condition.

Validation

python3 -m py_compile \
  test/functional/feature_governance.py \
  test/functional/feature_governance_cl.py \
  test/functional/test_framework/governance.py
CONFIG=/Users/claw/Projects/dash/worktrees/tracker-1176/test/config.ini
python3 test/functional/feature_governance.py \
  --configfile="$CONFIG" \
  --timeout-factor=4

• Functional test passed: Tests successful
• Pre-PR code-review gate: ship

[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]

✅ Review complete (commit fefdd42)

[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.
Description check	✅ Passed	The description is well-related to the changeset, explaining the timing/sync issue being fixed and detailing the changes made to stabilize the test.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[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 97ae2fb.

📒 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 fefdd42 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: fefdd42