From 4bcacc5e0f23d649ea98dc55e6bd2f54baac4c3a Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 6 Jun 2025 15:02:13 +0300 Subject: [PATCH 1/3] fix: request governance votes from more nodes on regtest --- src/governance/governance.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 06bb466e9de4..b0650d77b4c6 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -1287,7 +1287,9 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& int64_t nNow = GetTime(); int nTimeout = 60 * 60; - size_t nPeersPerHashMax = 3; + // We isolate nodes on regtest during tests so let's use extra nodes to make sure + // votes from isolated nodes are requested by non-isolated nodes correctly. + size_t nPeersPerHashMax = Params().IsMockableChain() ? 10 : 3; std::vector vTriggerObjHashes; std::vector vOtherObjHashes; From c16a62fe2d4083a645afc320705252153c34d458 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Fri, 6 Jun 2025 15:02:58 +0300 Subject: [PATCH 2/3] test: adjust feature_governance.py --- test/functional/feature_governance.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index be828d3b8936..34fc28634fe4 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -166,6 +166,8 @@ def run_test(self): self.wait_until(lambda: self.nodes[1].gobject("get", self.p2_hash)["FundingResult"]["NoCount"] == 2, timeout = 5) assert_equal(len(self.nodes[0].gobject("list", "valid", "triggers")), 0) + assert_equal(self.nodes[0].gobject("count")["votes"], 15) + assert_equal(self.nodes[1].gobject("count")["votes"], 15) block_count = self.nodes[0].getblockcount() @@ -209,6 +211,8 @@ def run_test(self): 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) + assert_equal(self.nodes[0].gobject("count")["votes"], 15) + assert_equal(isolated.gobject("count")["votes"], 16) self.log.info("Move 1 block enabling the Superblock maturity window on non-isolated nodes") self.bump_mocktime(1) @@ -231,6 +235,8 @@ def run_test(self): 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) + assert_equal(self.nodes[0].gobject("count")["votes"], 16) + assert_equal(isolated.gobject("count")["votes"], 16) self.log.info("Make sure amounts aren't trimmed") payment_amounts_expected = [str(satoshi_round(str(self.p0_amount))), str(satoshi_round(str(self.p1_amount))), str(satoshi_round(str(self.p2_amount)))] @@ -247,6 +253,8 @@ def run_test(self): 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) + assert_equal(self.nodes[0].gobject("count")["votes"], 19) + assert_equal(isolated.gobject("count")["votes"], 16) self.reconnect_isolated_node(payee_idx, 0) # self.connect_nodes(0, payee_idx) @@ -279,11 +287,16 @@ def sync_gov(node): self.bump_mocktime(1) self.generate(self.nodes[0], 1, sync_fun=self.sync_blocks()) - self.log.info("Should see NO votes on both triggers now") - self.wait_until(lambda: self.nodes[0].gobject("list", "valid", "triggers")[winning_trigger_hash]['NoCount'] == 1, timeout=5) - self.wait_until(lambda: self.nodes[0].gobject("list", "valid", "triggers")[isolated_trigger_hash]['NoCount'] == self.mn_count - 1, timeout=5) - self.log.info("Should wait until all 24 votes are counted for success on next stages") - self.wait_until(lambda: self.nodes[1].gobject("count")["votes"] == 24, timeout=5) + 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("Remember vote count") before = self.nodes[1].gobject("count")["votes"] From bd2aa8051e8ecca68fa4af7e36b683b7f21c84a2 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sat, 16 Aug 2025 11:47:12 +0300 Subject: [PATCH 3/3] refactor: use named constants, tweak and explain chosen numbers --- src/governance/governance.cpp | 12 +++++++++--- test/functional/feature_governance.py | 14 +++++++++++++- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index b0650d77b4c6..7f7ddaf4fa38 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -1282,14 +1282,20 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector& const PeerManager& peerman) const { static std::map > mapAskedRecently; + // Maximum number of nodes to request votes from for the same object hash on real networks + // (mainnet, testnet, devnets). Keep this low to avoid unnecessary bandwidth usage. + static constexpr size_t REALNET_PEERS_PER_HASH{3}; + // Maximum number of nodes to request votes from for the same object hash on regtest. + // During testing, nodes are isolated to create conflicting triggers. Using the real + // networks limit of 3 nodes often results in querying only "non-isolated" nodes, missing the + // isolated ones we need to test. This high limit ensures all available nodes are queried. + static constexpr size_t REGTEST_PEERS_PER_HASH{std::numeric_limits::max()}; if (vNodesCopy.empty()) return -1; int64_t nNow = GetTime(); int nTimeout = 60 * 60; - // We isolate nodes on regtest during tests so let's use extra nodes to make sure - // votes from isolated nodes are requested by non-isolated nodes correctly. - size_t nPeersPerHashMax = Params().IsMockableChain() ? 10 : 3; + size_t nPeersPerHashMax = Params().IsMockableChain() ? REGTEST_PEERS_PER_HASH : REALNET_PEERS_PER_HASH; std::vector vTriggerObjHashes; std::vector vOtherObjHashes; diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 34fc28634fe4..9ed1e59d4991 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -166,6 +166,7 @@ def run_test(self): self.wait_until(lambda: self.nodes[1].gobject("get", self.p2_hash)["FundingResult"]["NoCount"] == 2, timeout = 5) assert_equal(len(self.nodes[0].gobject("list", "valid", "triggers")), 0) + # 5 nodes voted on 3 proposals so we expect to see 15 votes total assert_equal(self.nodes[0].gobject("count")["votes"], 15) assert_equal(self.nodes[1].gobject("count")["votes"], 15) @@ -211,8 +212,10 @@ def run_test(self): 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) - assert_equal(self.nodes[0].gobject("count")["votes"], 15) + # 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) + assert_equal(self.nodes[0].gobject("count")["votes"], 15) self.log.info("Move 1 block enabling the Superblock maturity window on non-isolated nodes") self.bump_mocktime(1) @@ -235,7 +238,9 @@ def run_test(self): 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) + # 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) assert_equal(isolated.gobject("count")["votes"], 16) self.log.info("Make sure amounts aren't trimmed") @@ -253,7 +258,9 @@ def run_test(self): 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) + # 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) assert_equal(isolated.gobject("count")["votes"], 16) self.reconnect_isolated_node(payee_idx, 0) @@ -295,6 +302,11 @@ def sync_gov(node): 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") + # All 4 non-isolated nodes voted NO for a trigger created by a now reconnected node. + # They also see 1 YES vote for this trigger the reconnected node created earlier. + # The reconnected node received earlier votes from non-isolated ones and + # voted NO vote for the trigger non-isolated node created. + # So everyone should be on the same page now with 25 votes total. for node in self.nodes: assert_equal(node.gobject("count")["votes"], 25)