fix: request governance votes from more nodes on regtest

[UdjinM6]

Issue being fixed or feature implemented

We only ask nPeersPerHashMax (3) nodes for governance votes for the same governance object when syncing. However on regtest we also isolate nodes to create conflicting triggers and since we have 5 nodes to sync from asking 3 of them often results in asking "non-isolated" nodes only (24 votes) yet sometimes we do ask previously isolated node too (25 votes).

Should fix feature_governance.py flakiness. Alternative to #6710.

What was done?

Bump nPeersPerHashMax for regtest. Add more asserts in tests to fail earlier if smth isn't right, check votes on all nodes.

How Has This Been Tested?

run feature_governance.py

Breaking Changes

n/a

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[coderabbitai]

Walkthrough

The changes update the governance voting logic and its corresponding functional test. In the core implementation, the maximum number of peers from which to request votes for a governance object is conditionally increased from 3 to 10 when running on a mockable chain, such as regtest, while maintaining the original limit for other networks. The functional test is expanded with additional assertions to check the total vote counts reported by the gobject("count")["votes"] RPC across multiple nodes at different stages of the governance process. The test now verifies vote consistency and synchronization between nodes, especially after network partitions and reconnections. No public API or exported function signatures were changed.

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09aa42e and 89753f2.

📒 Files selected for processing (2)

• src/governance/governance.cpp (1 hunks)
• test/functional/feature_governance.py (5 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

test/functional/feature_governance.py (2)

test/functional/test_framework/util.py (1)

• assert_equal (64-69)

test/functional/test_framework/test_framework.py (1)

• wait_until (900-901)

🪛 GitHub Actions: Check Potential Conflicts

src/governance/governance.cpp

[error] 1-1: Merge conflict detected in 'src/governance/governance.cpp' across multiple pull requests. Rebase required.

test/functional/feature_governance.py

[error] 1-1: Merge conflict detected in 'test/functional/feature_governance.py' in pull request 6182 and 6710. Rebase required.

🪛 Ruff (0.11.9)

test/functional/feature_governance.py

289-289: Function definition does not bind loop variable node

(B023)

---

290-290: Function definition does not bind loop variable node

(B023)

---

291-291: Function definition does not bind loop variable node

(B023)

---

292-292: Function definition does not bind loop variable node

(B023)

⏰ Context from checks skipped due to timeout of 90000ms (3)

• GitHub Check: x86_64-apple-darwin / Build depends
• GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
• GitHub Check: x86_64-pc-linux-gnu / Build depends

🔇 Additional comments (6)

src/governance/governance.cpp (1)

291-293:

✅ Verification successful

LGTM! Well-documented fix for test flakiness.

The conditional increase in nPeersPerHashMax for mockable chains is a targeted solution to address vote synchronization issues with isolated nodes during testing. The explanatory comment clearly documents the rationale.

However, there's a merge conflict that needs to be resolved before this can be merged:

---

🏁 Script executed:

#!/bin/bash
# Check for merge conflict markers in the file
rg -n "<<<<<<|>>>>>>|======" src/governance/governance.cpp

Length of output: 60

---

No merge conflicts detected in src/governance/governance.cpp – changes approved

• Confirmed absence of <<<<<<<, >>>>>>>, or ====== markers in lines 291–293.
• Conditional increase of nPeersPerHashMax for mockable chains is well-documented and addresses test flakiness.

test/functional/feature_governance.py (5)

166-167: Good addition of vote count verification.

These assertions help validate that vote counts are correctly synchronized between nodes after initial voting.

---

211-212: Effective validation of isolated node behavior.

The assertions correctly verify that the isolated node has one additional vote (16) compared to non-isolated nodes (15) after submitting and voting on its trigger.

---

235-236: Good synchronization check.

Both nodes showing 16 votes indicates proper synchronization after the non-isolated node creates its trigger.

---

253-254: Comprehensive vote tracking.

The vote count difference (19 vs 16) effectively demonstrates that additional votes are being cast on the non-isolated network while the isolated node remains static.

---

287-296:

❌ Incorrect review comment

Fix lambda variable capture issue in loops.

The static analysis correctly identifies a bug where lambda functions capture the loop variable node by reference, which can lead to unexpected behavior.

Apply this fix to properly capture the loop variable:

-        self.log.info("Should see same YES and NO vote count for both triggers on all nodes now")
-        for node in self.nodes:
-            self.wait_until(lambda: node.gobject("list", "valid", "triggers")[winning_trigger_hash]['YesCount'] == self.mn_count - 1, timeout=5)
-            self.wait_until(lambda: node.gobject("list", "valid", "triggers")[winning_trigger_hash]['NoCount'] == 1, timeout=5)
-            self.wait_until(lambda: node.gobject("list", "valid", "triggers")[isolated_trigger_hash]['YesCount'] == 1, timeout=5)
-            self.wait_until(lambda: node.gobject("list", "valid", "triggers")[isolated_trigger_hash]['NoCount'] == self.mn_count - 1, timeout=5)
-
-        self.log.info("Should have 25 votes on all nodes")
-        for node in self.nodes:
-            assert_equal(node.gobject("count")["votes"], 25)
+        self.log.info("Should see same YES and NO vote count for both triggers on all nodes now")
+        for node in self.nodes:
+            self.wait_until(lambda n=node: n.gobject("list", "valid", "triggers")[winning_trigger_hash]['YesCount'] == self.mn_count - 1, timeout=5)
+            self.wait_until(lambda n=node: n.gobject("list", "valid", "triggers")[winning_trigger_hash]['NoCount'] == 1, timeout=5)
+            self.wait_until(lambda n=node: n.gobject("list", "valid", "triggers")[isolated_trigger_hash]['YesCount'] == 1, timeout=5)
+            self.wait_until(lambda n=node: n.gobject("list", "valid", "triggers")[isolated_trigger_hash]['NoCount'] == self.mn_count - 1, timeout=5)
+
+        self.log.info("Should have 25 votes on all nodes")
+        for node in self.nodes:
+            assert_equal(node.gobject("count")["votes"], 25)

Also, there's a merge conflict that needs resolution:

---

🏁 Script executed:

#!/bin/bash
# Check for merge conflict markers in the test file
rg -n "<<<<<<|>>>>>>|======" test/functional/feature_governance.py

Length of output: 68

---

No action needed: lambdas are safe here and no merge conflicts detected

I verified that test/functional/feature_governance.py contains no conflict markers, and each wait_until(lambda: …) call completes before the loop moves on—so the late‐binding lambda “issue” does not occur in this synchronous context. You can safely ignore the suggested default‐arg fix and merge‐conflict check.

Likely an incorrect or invalid review comment.

🧰 Tools

🪛 Ruff (0.11.9)

289-289: Function definition does not bind loop variable node

(B023)

---

290-290: Function definition does not bind loop variable node

(B023)

---

291-291: Function definition does not bind loop variable node

(B023)

---

292-292: Function definition does not bind loop variable node

(B023)

✨ Finishing Touches

• 📝 Generate Docstrings

---

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[knst]

Is it possible to choose "isolated" nodes somehow by more deterministic way instead bumping this variable?