From a6320865c46b2af0ec89c830c868e7aeec96d187 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Thu, 25 Jul 2024 12:31:44 +0300 Subject: [PATCH 1/4] fix: avoid voting for the same trigger multiple times --- src/governance/governance.cpp | 39 ++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 89336190ed..208cb7e5bd 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -743,21 +744,36 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalGetGovernanceObject(*this)->GetHash(); + const auto govobj = trigger->GetGovernanceObject(*this); + const uint256 trigger_hash = govobj->GetHash(); if (trigger->GetBlockHeight() <= nCachedBlockHeight) { // ignore triggers from the past LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for outdated trigger:%s\n", __func__, trigger_hash.ToString()); @@ -768,6 +784,19 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalGetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { + const auto& strFunc = __func__; + if (ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair){ + if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for trigger:%s, we voted %s for it already\n", + strFunc, trigger_hash.ToString(), CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); + return true; + } + return false; + })) { + continue; + } + } if (!VoteFundingTrigger(trigger_hash, VOTE_OUTCOME_NO, connman, peerman, mn_activeman)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting NO-FUNDING for trigger:%s failed\n", __func__, trigger_hash.ToString()); // failing here is ok-ish From ec1392c6dec3f945d889561a789fe3a769906b7f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 5 Aug 2024 16:21:16 +0300 Subject: [PATCH 2/4] chore: make clang-format and linter happy --- src/governance/governance.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 208cb7e5bd..72709d951a 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -748,19 +748,23 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalGetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { const auto& strFunc = __func__; - if (ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair){ - if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for trigger:%s, we voted %s for it already\n", - strFunc, trigger_hash.ToString(), CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); - return true; - } - return false; - })) { + if (ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair) { + if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { + LogPrint(BCLog::GOBJECT, /* Continued */ + "CGovernanceManager::%s " + "Not voting NO-FUNDING for trigger:%s, we voted %s for it already\n", + strFunc, trigger_hash.ToString(), + CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); + return true; + } + return false; + })) { continue; } } From 3d75390e4ec590dc06de8554b16d1d10735dd23a Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 14 Aug 2024 11:41:41 +0300 Subject: [PATCH 3/4] fix: correct conditions for YES voting --- src/governance/governance.cpp | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 72709d951a..7aa10f6b03 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -746,9 +746,11 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optional Date: Thu, 22 Aug 2024 23:18:17 +0300 Subject: [PATCH 4/4] test: make sure MNs don't vote twice even when they are allowed to --- test/functional/feature_governance.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index e98cf6ba1b..9ec599dc96 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -11,6 +11,8 @@ from test_framework.test_framework import DashTestFramework from test_framework.governance import have_trigger_for_height, prepare_object from test_framework.util import assert_equal, satoshi_round, set_node_times, wait_until_helper +GOVERNANCE_UPDATE_MIN = 60 * 60 # src/governance/object.h + class DashGovernanceTest (DashTestFramework): def set_test_params(self): self.v20_start_time = 1417713500 + 80 @@ -283,6 +285,23 @@ class DashGovernanceTest (DashTestFramework): 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) + # Remember vote count + before = self.nodes[1].gobject("count")["votes"] + + # Bump mocktime to let MNs vote again + self.bump_mocktime(GOVERNANCE_UPDATE_MIN + 1) + + # Move another block inside the Superblock maturity window + with self.nodes[1].assert_debug_log(["CGovernanceManager::VoteGovernanceTriggers"]): + self.nodes[0].generate(1) + self.bump_mocktime(1) + self.sync_blocks() + + # Vote count should not change even though MNs are allowed to vote again + assert_equal(before, self.nodes[1].gobject("count")["votes"]) + # Revert mocktime back to avoid issues in tests below + self.bump_mocktime(GOVERNANCE_UPDATE_MIN * -1) + block_count = self.nodes[0].getblockcount() n = sb_cycle - block_count % sb_cycle