From fefbe27ee49c471936569da3412bdca9741299a9 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 22:04:39 +0000 Subject: [PATCH 01/14] refactor: replace `Sign` functions with direct activeman call We are discarding the value of `ObjName::Sign()` so we are assuming it is always valid, let's codify that assumption with an assert, something we generate should hold valid to ourselves. --- src/coinjoin/coinjoin.cpp | 25 ------------------------- src/coinjoin/coinjoin.h | 11 +---------- src/coinjoin/server.cpp | 6 +++--- src/governance/governance.cpp | 4 ++-- src/governance/object.cpp | 10 ++-------- src/governance/object.h | 5 +++-- src/governance/vote.cpp | 11 ----------- src/governance/vote.h | 3 --- src/masternode/node.cpp | 8 ++++++++ src/masternode/node.h | 1 + src/test/coinjoin_queue_tests.cpp | 2 +- 11 files changed, 21 insertions(+), 65 deletions(-) diff --git a/src/coinjoin/coinjoin.cpp b/src/coinjoin/coinjoin.cpp index 07ddb2018090f..d48239927f1aa 100644 --- a/src/coinjoin/coinjoin.cpp +++ b/src/coinjoin/coinjoin.cpp @@ -16,7 +16,6 @@ #include #include #include -#include #include #include @@ -45,18 +44,6 @@ uint256 CCoinJoinQueue::GetSignatureHash() const } uint256 CCoinJoinQueue::GetHash() const { return SerializeHash(*this, SER_NETWORK, PROTOCOL_VERSION); } -bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman) -{ - uint256 hash = GetSignatureHash(); - CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false); - if (!sig.IsValid()) { - return false; - } - vchSig = sig.ToByteVector(false); - - return true; -} - bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const { if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { @@ -84,18 +71,6 @@ uint256 CCoinJoinBroadcastTx::GetSignatureHash() const return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION); } -bool CCoinJoinBroadcastTx::Sign(const CActiveMasternodeManager& mn_activeman) -{ - uint256 hash = GetSignatureHash(); - CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false); - if (!sig.IsValid()) { - return false; - } - vchSig = sig.ToByteVector(false); - - return true; -} - bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const { if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) { diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index d69d6c12d0511..90d1fe2fe4a90 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -22,7 +22,6 @@ #include #include -class CActiveMasternodeManager; class CChainState; class CBLSPublicKey; class CBlockIndex; @@ -209,14 +208,7 @@ class CCoinJoinQueue [[nodiscard]] uint256 GetHash() const; [[nodiscard]] uint256 GetSignatureHash() const; - /** Sign this mixing transaction - * return true if all conditions are met: - * 1) we have an active Masternode, - * 2) we have a valid Masternode private key, - * 3) we signed the message successfully, and - * 4) we verified the message successfully - */ - bool Sign(const CActiveMasternodeManager& mn_activeman); + /// Check if we have a valid Masternode address [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const; @@ -284,7 +276,6 @@ class CCoinJoinBroadcastTx [[nodiscard]] uint256 GetSignatureHash() const; - bool Sign(const CActiveMasternodeManager& mn_activeman); [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const; // Used only for unit tests diff --git a/src/coinjoin/server.cpp b/src/coinjoin/server.cpp index 70c5b4772109f..86c8e67bc5cb9 100644 --- a/src/coinjoin/server.cpp +++ b/src/coinjoin/server.cpp @@ -345,7 +345,7 @@ void CCoinJoinServer::CommitFinalTransaction() if (!m_dstxman.GetDSTX(hashTx)) { CCoinJoinBroadcastTx dstxNew(finalTransaction, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(), GetAdjustedTime()); - dstxNew.Sign(m_mn_activeman); + dstxNew.vchSig = m_mn_activeman.SignBasic(dstxNew.GetSignatureHash()); m_dstxman.AddDSTX(dstxNew); } @@ -504,7 +504,7 @@ void CCoinJoinServer::CheckForCompleteQueue() GetAdjustedTime(), true); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */ "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size()); - dsq.Sign(m_mn_activeman); + dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash()); m_peerman.RelayDSQ(dsq); WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq)); } @@ -714,7 +714,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(), GetAdjustedTime(), false); LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString()); - dsq.Sign(m_mn_activeman); + dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash()); m_peerman.RelayDSQ(dsq); LOCK(cs_vecqueue); vecCoinJoinQueue.push_back(dsq); diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 3cc1d9ac34e81..cf3f2604c1d1f 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -732,7 +732,7 @@ std::optional CGovernanceManager::CreateGovernanceTrigg return std::nullopt; } gov_sb.SetMasternodeOutpoint(mn_activeman.GetOutPoint()); - gov_sb.Sign(mn_activeman); + gov_sb.SetSignature(mn_activeman.SignBasic(gov_sb.GetSignatureHash())); if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), m_chainman, strError, true)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError); @@ -841,7 +841,7 @@ bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_out { CGovernanceVote vote(mn_activeman.GetOutPoint(), nHash, VOTE_SIGNAL_FUNDING, outcome); vote.SetTime(GetAdjustedTime()); - vote.Sign(mn_activeman); + vote.SetSignature(mn_activeman.SignBasic(vote.GetSignatureHash())); CGovernanceException exception; if (!ProcessVoteAndRelay(vote, exception, connman, peerman)) { diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 8945ec62f7fae..2e4dbfbe43e38 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -244,14 +243,9 @@ void CGovernanceObject::SetMasternodeOutpoint(const COutPoint& outpoint) m_obj.masternodeOutpoint = outpoint; } -bool CGovernanceObject::Sign(const CActiveMasternodeManager& mn_activeman) +void CGovernanceObject::SetSignature(Span sig) { - CBLSSignature sig = mn_activeman.Sign(GetSignatureHash(), false); - if (!sig.IsValid()) { - return false; - } - m_obj.vchSig = sig.ToByteVector(false); - return true; + m_obj.vchSig.assign(sig.begin(), sig.end()); } bool CGovernanceObject::CheckSignature(const CBLSPublicKey& pubKey) const diff --git a/src/governance/object.h b/src/governance/object.h index 66324a75d62b0..34ebaa4efa993 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -11,9 +11,10 @@ #include #include +#include + #include -class CActiveMasternodeManager; class CBLSPublicKey; class CDeterministicMNList; class CGovernanceManager; @@ -221,7 +222,7 @@ class CGovernanceObject // Signature related functions void SetMasternodeOutpoint(const COutPoint& outpoint); - bool Sign(const CActiveMasternodeManager& mn_activeman); + void SetSignature(Span sig); bool CheckSignature(const CBLSPublicKey& pubKey) const; uint256 GetSignatureHash() const; diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index 57c6e5d69ffdd..a20d2afc246de 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -158,16 +157,6 @@ bool CGovernanceVote::CheckSignature(const CKeyID& keyID) const return true; } -bool CGovernanceVote::Sign(const CActiveMasternodeManager& mn_activeman) -{ - CBLSSignature sig = mn_activeman.Sign(GetSignatureHash(), false); - if (!sig.IsValid()) { - return false; - } - vchSig = sig.ToByteVector(false); - return true; -} - bool CGovernanceVote::CheckSignature(const CBLSPublicKey& pubKey) const { CBLSSignature sig; diff --git a/src/governance/vote.h b/src/governance/vote.h index 6d2388a132afd..25f62a4408a92 100644 --- a/src/governance/vote.h +++ b/src/governance/vote.h @@ -10,7 +10,6 @@ #include #include -class CActiveMasternodeManager; class CBLSPublicKey; class CDeterministicMNList; class CGovernanceVote; @@ -97,9 +96,7 @@ class CGovernanceVote void SetSignature(const std::vector& vchSigIn) { vchSig = vchSigIn; } - bool Sign(const CKey& key, const CKeyID& keyID); bool CheckSignature(const CKeyID& keyID) const; - bool Sign(const CActiveMasternodeManager& mn_activeman); bool CheckSignature(const CBLSPublicKey& pubKey) const; bool IsValid(const CDeterministicMNList& tip_mn_list, bool useVotingKey) const; std::string GetSignatureString() const diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 5399ab47ba147..2f0f692388715 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -276,6 +276,14 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec return WITH_READ_LOCK(cs, return m_info.blsKeyOperator.Sign(hash, is_legacy)); } +[[nodiscard]] std::vector CActiveMasternodeManager::SignBasic(const uint256& hash) const +{ + AssertLockNotHeld(cs); + auto sig = Sign(hash, /*is_legacy=*/false); + assert(sig.IsValid()); + return sig.ToByteVector(/*specificLegacyScheme=*/false); +} + // We need to pass a copy as opposed to a const ref because CBLSPublicKeyVersionWrapper // does not accept a const ref in its construction args [[nodiscard]] CBLSPublicKey CActiveMasternodeManager::GetPubKey() const diff --git a/src/masternode/node.h b/src/masternode/node.h index 931233c42f72f..656fafd77e95d 100644 --- a/src/masternode/node.h +++ b/src/masternode/node.h @@ -66,6 +66,7 @@ class CActiveMasternodeManager [[nodiscard]] bool Decrypt(const EncryptedObj& obj, size_t idx, Obj& ret_obj, int version) const EXCLUSIVE_LOCKS_REQUIRED(!cs); [[nodiscard]] CBLSSignature Sign(const uint256& hash, const bool is_legacy) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + [[nodiscard]] std::vector SignBasic(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); /* TODO: Reconsider external locking */ [[nodiscard]] COutPoint GetOutPoint() const { READ_LOCK(cs); return m_info.outpoint; } diff --git a/src/test/coinjoin_queue_tests.cpp b/src/test/coinjoin_queue_tests.cpp index 55323a5a37ad4..fda668146477c 100644 --- a/src/test/coinjoin_queue_tests.cpp +++ b/src/test/coinjoin_queue_tests.cpp @@ -34,7 +34,7 @@ BOOST_AUTO_TEST_CASE(queue_sign_and_verify) q.fReady = false; // Sign and verify with corresponding pubkey - BOOST_CHECK(q.Sign(mn_activeman)); + q.vchSig = mn_activeman.SignBasic(q.GetSignatureHash()); const CBLSPublicKey pub = mn_activeman.GetPubKey(); BOOST_CHECK(q.CheckSignature(pub)); } From 2b74e152cf77bbdb26386d7d666340b2d8617dac Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 3 Oct 2025 11:47:11 +0000 Subject: [PATCH 02/14] refactor: move masternode mode logic to `governance/signing.cpp` Review with `git log -p -n1 --color-moved=dimmed_zebra`. --- src/Makefile.am | 1 + src/governance/governance.cpp | 270 ------------------------------- src/governance/signing.cpp | 288 ++++++++++++++++++++++++++++++++++ 3 files changed, 289 insertions(+), 270 deletions(-) create mode 100644 src/governance/signing.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 9ad66b87a276d..e2a817d65680d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -510,6 +510,7 @@ libbitcoin_node_a_SOURCES = \ governance/exceptions.cpp \ governance/governance.cpp \ governance/object.cpp \ + governance/signing.cpp \ governance/validators.cpp \ governance/vote.cpp \ governance/votedb.cpp \ diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index cf3f2604c1d1f..af1451ea38353 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -592,276 +592,6 @@ struct sortProposalsByVotes { } }; -std::optional CGovernanceManager::CreateSuperblockCandidate(int nHeight) const -{ - if (!IsValid()) return std::nullopt; - if (!m_mn_sync.IsSynced()) return std::nullopt; - if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; - if (HasAlreadyVotedFundingTrigger()) return std::nullopt; - - // A proposal is considered passing if (YES votes) >= (Total Weight of Masternodes / 10), - // count total valid (ENABLED) masternodes to determine passing threshold. - const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); - const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); - const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); - - // Use std::vector of std::shared_ptr because CGovernanceObject doesn't support move operations (needed for sorting the vector later) - std::vector> approvedProposals; - - { - LOCK(cs); - for (const auto& [unused, object] : mapObjects) { - // Skip all non-proposals objects - if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; - - const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - // Skip non-passing proposals - if (absYesCount < nAbsVoteReq) continue; - - approvedProposals.emplace_back(std::make_shared(object)); - } - } // cs - - // Sort approved proposals by absolute Yes votes descending - std::sort(approvedProposals.begin(), approvedProposals.end(), [tip_mn_list](std::shared_ptr a, std::shared_ptr b) { - const auto a_yes = a->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - const auto b_yes = b->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - return a_yes == b_yes ? UintToArith256(a->GetHash()) > UintToArith256(b->GetHash()) : a_yes > b_yes; - }); - - if (approvedProposals.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d empty approvedProposals\n", __func__, nHeight); - return std::nullopt; - } - - std::vector payments; - int nLastSuperblock; - int nNextSuperblock; - - CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock); - auto SBEpochTime = static_cast(GetTime().count() + (nNextSuperblock - nHeight) * 2.62 * 60); - auto governanceBudget = CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock); - - CAmount budgetAllocated{}; - for (const auto& proposal : approvedProposals) { - // Extract payment address and amount from proposal - UniValue jproposal = proposal->GetJSONObject(); - - CTxDestination dest = DecodeDestination(jproposal["payment_address"].getValStr()); - if (!IsValidDestination(dest)) continue; - - CAmount nAmount{}; - try { - nAmount = ParsePaymentAmount(jproposal["payment_amount"].getValStr()); - } - catch (const std::runtime_error& e) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d Skipping payment exception:%s\n", __func__,nHeight, e.what()); - continue; - } - - // Construct CGovernancePayment object and make sure it is valid - CGovernancePayment payment(dest, nAmount, proposal->GetHash()); - if (!payment.IsValid()) continue; - - // Skip proposals that are too expensive - if (budgetAllocated + payment.nAmount > governanceBudget) continue; - - int64_t windowStart = jproposal["start_epoch"].getInt() - GOVERNANCE_FUDGE_WINDOW; - int64_t windowEnd = jproposal["end_epoch"].getInt() + GOVERNANCE_FUDGE_WINDOW; - - // Skip proposals if the SB isn't within the proposal time window - if (SBEpochTime < windowStart) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowStart:%d\n", __func__,nHeight, SBEpochTime, windowStart); - continue; - } - if (SBEpochTime > windowEnd) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowEnd:%d\n", __func__,nHeight, SBEpochTime, windowEnd); - continue; - } - - // Keep track of total budget allocation - budgetAllocated += payment.nAmount; - - // Add the payment - payments.push_back(payment); - } - - // No proposals made the cut - if (payments.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s CreateSuperblockCandidate nHeight:%d empty payments\n", __func__, nHeight); - return std::nullopt; - } - - // Sort by proposal hash descending - std::sort(payments.begin(), payments.end(), [](const CGovernancePayment& a, const CGovernancePayment& b) { - return UintToArith256(a.proposalHash) > UintToArith256(b.proposalHash); - }); - - // Create Superblock - return CSuperblock(nNextSuperblock, std::move(payments)); -} - -std::optional CGovernanceManager::CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman) -{ - // no sb_opt, no trigger - if (!sb_opt.has_value()) return std::nullopt; - - //TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct - LOCK2(::cs_main, cs); - - // Check if identical trigger (equal DataHash()) is already created (signed by other masternode) - CGovernanceObject gov_sb(uint256(), 1, GetAdjustedTime(), uint256(), sb_opt.value().GetHexStrData()); - if (auto identical_sb = FindGovernanceObjectByDataHash(gov_sb.GetDataHash())) { - // Somebody submitted a trigger with the same data, support it instead of submitting a duplicate - return std::make_optional(*identical_sb); - } - - // Nobody submitted a trigger we'd like to see, so let's do it but only if we are the payee - const CBlockIndex* tip = m_chainman.ActiveChain().Tip(); - const auto mnList = Assert(m_dmnman)->GetListForBlock(tip); - const auto mn_payees = mnList.GetProjectedMNPayees(tip); - - if (mn_payees.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s payee list is empty\n", __func__); - return std::nullopt; - } - - if (mn_payees.front()->proTxHash != mn_activeman.GetProTxHash()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__); - return std::nullopt; - } - gov_sb.SetMasternodeOutpoint(mn_activeman.GetOutPoint()); - gov_sb.SetSignature(mn_activeman.SignBasic(gov_sb.GetSignatureHash())); - - if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), m_chainman, strError, true)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError); - return std::nullopt; - } - - if (!MasternodeRateCheck(gov_sb)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Trigger rejected because of rate check failure hash(%s)\n", __func__, gov_sb.GetHash().ToString()); - return std::nullopt; - } - - // The trigger we just created looks good, submit it - AddGovernanceObject(gov_sb, peerman); - return std::make_optional(gov_sb); -} - -void CGovernanceManager::VoteGovernanceTriggers(const std::optional& trigger_opt, CConnman& connman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman) -{ - // only active masternodes can vote on triggers - if (mn_activeman.GetProTxHash().IsNull()) return; - - LOCK2(::cs_main, cs); - - if (trigger_opt.has_value()) { - // We should never vote "yes" on another trigger or the same trigger twice - assert(!votedFundingYesTriggerHash.has_value()); - // Vote YES-FUNDING for the trigger we like, unless we already did - const uint256 gov_sb_hash = trigger_opt.value().GetHash(); - bool voted_already{false}; - if (vote_rec_t voteRecord; trigger_opt.value().GetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { - const auto& strFunc = __func__; - // Let's see if there is a VOTE_SIGNAL_FUNDING vote from us already - voted_already = ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair) { - if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { - if (voteInstancePair.second.eOutcome == VOTE_OUTCOME_YES) { - votedFundingYesTriggerHash = gov_sb_hash; - } - LogPrint(BCLog::GOBJECT, /* Continued */ - "CGovernanceManager::%s " - "Not voting YES-FUNDING for trigger:%s, we voted %s for it already\n", - strFunc, gov_sb_hash.ToString(), - CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); - return true; - } - return false; - }); - } - if (!voted_already) { - // No previous VOTE_SIGNAL_FUNDING was found, vote now - if (VoteFundingTrigger(gov_sb_hash, VOTE_OUTCOME_YES, connman, peerman, mn_activeman)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting YES-FUNDING for new trigger:%s success\n", - __func__, gov_sb_hash.ToString()); - votedFundingYesTriggerHash = gov_sb_hash; - } else { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting YES-FUNDING for new trigger:%s failed\n", - __func__, gov_sb_hash.ToString()); - // this should never happen, bail out - return; - } - } - } - - // Vote NO-FUNDING for the rest of the active triggers - const auto activeTriggers = GetActiveTriggers(); - for (const auto& trigger : activeTriggers) { - const auto govobj = FindGovernanceObject(trigger->GetGovernanceObjHash()); - 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()); - continue; - } - if (trigger_hash == votedFundingYesTriggerHash) { - // Skip actual trigger - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); - continue; - } - if (vote_rec_t voteRecord; govobj->GetCurrentMNVotes(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, /* 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; - } - } - 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 - continue; - } - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting NO-FUNDING for trigger:%s success\n", __func__, trigger_hash.ToString()); - } -} - -bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman) -{ - CGovernanceVote vote(mn_activeman.GetOutPoint(), nHash, VOTE_SIGNAL_FUNDING, outcome); - vote.SetTime(GetAdjustedTime()); - vote.SetSignature(mn_activeman.SignBasic(vote.GetSignatureHash())); - - CGovernanceException exception; - if (!ProcessVoteAndRelay(vote, exception, connman, peerman)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Vote FUNDING %d for trigger:%s failed:%s\n", __func__, outcome, nHash.ToString(), exception.what()); - return false; - } - - return true; -} - -bool CGovernanceManager::HasAlreadyVotedFundingTrigger() const -{ - return votedFundingYesTriggerHash.has_value(); -} - -void CGovernanceManager::ResetVotedFundingTrigger() -{ - votedFundingYesTriggerHash = std::nullopt; -} - void CGovernanceManager::DoMaintenance(CConnman& connman) { if (!IsValid()) return; diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp new file mode 100644 index 0000000000000..0c6a79ea1391a --- /dev/null +++ b/src/governance/signing.cpp @@ -0,0 +1,288 @@ +// Copyright (c) 2014-2025 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +std::optional CGovernanceManager::CreateSuperblockCandidate(int nHeight) const +{ + if (!IsValid()) return std::nullopt; + if (!m_mn_sync.IsSynced()) return std::nullopt; + if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; + if (HasAlreadyVotedFundingTrigger()) return std::nullopt; + + // A proposal is considered passing if (YES votes) >= (Total Weight of Masternodes / 10), + // count total valid (ENABLED) masternodes to determine passing threshold. + const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); + const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); + const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); + + // Use std::vector of std::shared_ptr because CGovernanceObject doesn't support move operations (needed for sorting the vector later) + std::vector> approvedProposals; + + { + LOCK(cs); + for (const auto& [unused, object] : mapObjects) { + // Skip all non-proposals objects + if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; + + const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + // Skip non-passing proposals + if (absYesCount < nAbsVoteReq) continue; + + approvedProposals.emplace_back(std::make_shared(object)); + } + } // cs + + // Sort approved proposals by absolute Yes votes descending + std::sort(approvedProposals.begin(), approvedProposals.end(), [tip_mn_list](std::shared_ptr a, std::shared_ptr b) { + const auto a_yes = a->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + const auto b_yes = b->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + return a_yes == b_yes ? UintToArith256(a->GetHash()) > UintToArith256(b->GetHash()) : a_yes > b_yes; + }); + + if (approvedProposals.empty()) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d empty approvedProposals\n", __func__, nHeight); + return std::nullopt; + } + + std::vector payments; + int nLastSuperblock; + int nNextSuperblock; + + CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock); + auto SBEpochTime = static_cast(GetTime().count() + (nNextSuperblock - nHeight) * 2.62 * 60); + auto governanceBudget = CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock); + + CAmount budgetAllocated{}; + for (const auto& proposal : approvedProposals) { + // Extract payment address and amount from proposal + UniValue jproposal = proposal->GetJSONObject(); + + CTxDestination dest = DecodeDestination(jproposal["payment_address"].getValStr()); + if (!IsValidDestination(dest)) continue; + + CAmount nAmount{}; + try { + nAmount = ParsePaymentAmount(jproposal["payment_amount"].getValStr()); + } + catch (const std::runtime_error& e) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d Skipping payment exception:%s\n", __func__,nHeight, e.what()); + continue; + } + + // Construct CGovernancePayment object and make sure it is valid + CGovernancePayment payment(dest, nAmount, proposal->GetHash()); + if (!payment.IsValid()) continue; + + // Skip proposals that are too expensive + if (budgetAllocated + payment.nAmount > governanceBudget) continue; + + int64_t windowStart = jproposal["start_epoch"].getInt() - GOVERNANCE_FUDGE_WINDOW; + int64_t windowEnd = jproposal["end_epoch"].getInt() + GOVERNANCE_FUDGE_WINDOW; + + // Skip proposals if the SB isn't within the proposal time window + if (SBEpochTime < windowStart) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowStart:%d\n", __func__,nHeight, SBEpochTime, windowStart); + continue; + } + if (SBEpochTime > windowEnd) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowEnd:%d\n", __func__,nHeight, SBEpochTime, windowEnd); + continue; + } + + // Keep track of total budget allocation + budgetAllocated += payment.nAmount; + + // Add the payment + payments.push_back(payment); + } + + // No proposals made the cut + if (payments.empty()) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s CreateSuperblockCandidate nHeight:%d empty payments\n", __func__, nHeight); + return std::nullopt; + } + + // Sort by proposal hash descending + std::sort(payments.begin(), payments.end(), [](const CGovernancePayment& a, const CGovernancePayment& b) { + return UintToArith256(a.proposalHash) > UintToArith256(b.proposalHash); + }); + + // Create Superblock + return CSuperblock(nNextSuperblock, std::move(payments)); +} + +std::optional CGovernanceManager::CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, + const CActiveMasternodeManager& mn_activeman) +{ + // no sb_opt, no trigger + if (!sb_opt.has_value()) return std::nullopt; + + //TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct + LOCK2(::cs_main, cs); + + // Check if identical trigger (equal DataHash()) is already created (signed by other masternode) + CGovernanceObject gov_sb(uint256(), 1, GetAdjustedTime(), uint256(), sb_opt.value().GetHexStrData()); + if (auto identical_sb = FindGovernanceObjectByDataHash(gov_sb.GetDataHash())) { + // Somebody submitted a trigger with the same data, support it instead of submitting a duplicate + return std::make_optional(*identical_sb); + } + + // Nobody submitted a trigger we'd like to see, so let's do it but only if we are the payee + const CBlockIndex* tip = m_chainman.ActiveChain().Tip(); + const auto mnList = Assert(m_dmnman)->GetListForBlock(tip); + const auto mn_payees = mnList.GetProjectedMNPayees(tip); + + if (mn_payees.empty()) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s payee list is empty\n", __func__); + return std::nullopt; + } + + if (mn_payees.front()->proTxHash != mn_activeman.GetProTxHash()) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__); + return std::nullopt; + } + gov_sb.SetMasternodeOutpoint(mn_activeman.GetOutPoint()); + gov_sb.SetSignature(mn_activeman.SignBasic(gov_sb.GetSignatureHash())); + + if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), m_chainman, strError, true)) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError); + return std::nullopt; + } + + if (!MasternodeRateCheck(gov_sb)) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Trigger rejected because of rate check failure hash(%s)\n", __func__, gov_sb.GetHash().ToString()); + return std::nullopt; + } + + // The trigger we just created looks good, submit it + AddGovernanceObject(gov_sb, peerman); + return std::make_optional(gov_sb); +} + +void CGovernanceManager::VoteGovernanceTriggers(const std::optional& trigger_opt, CConnman& connman, PeerManager& peerman, + const CActiveMasternodeManager& mn_activeman) +{ + // only active masternodes can vote on triggers + if (mn_activeman.GetProTxHash().IsNull()) return; + + LOCK2(::cs_main, cs); + + if (trigger_opt.has_value()) { + // We should never vote "yes" on another trigger or the same trigger twice + assert(!votedFundingYesTriggerHash.has_value()); + // Vote YES-FUNDING for the trigger we like, unless we already did + const uint256 gov_sb_hash = trigger_opt.value().GetHash(); + bool voted_already{false}; + if (vote_rec_t voteRecord; trigger_opt.value().GetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { + const auto& strFunc = __func__; + // Let's see if there is a VOTE_SIGNAL_FUNDING vote from us already + voted_already = ranges::any_of(voteRecord.mapInstances, [&](const auto& voteInstancePair) { + if (voteInstancePair.first == VOTE_SIGNAL_FUNDING) { + if (voteInstancePair.second.eOutcome == VOTE_OUTCOME_YES) { + votedFundingYesTriggerHash = gov_sb_hash; + } + LogPrint(BCLog::GOBJECT, /* Continued */ + "CGovernanceManager::%s " + "Not voting YES-FUNDING for trigger:%s, we voted %s for it already\n", + strFunc, gov_sb_hash.ToString(), + CGovernanceVoting::ConvertOutcomeToString(voteInstancePair.second.eOutcome)); + return true; + } + return false; + }); + } + if (!voted_already) { + // No previous VOTE_SIGNAL_FUNDING was found, vote now + if (VoteFundingTrigger(gov_sb_hash, VOTE_OUTCOME_YES, connman, peerman, mn_activeman)) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting YES-FUNDING for new trigger:%s success\n", + __func__, gov_sb_hash.ToString()); + votedFundingYesTriggerHash = gov_sb_hash; + } else { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting YES-FUNDING for new trigger:%s failed\n", + __func__, gov_sb_hash.ToString()); + // this should never happen, bail out + return; + } + } + } + + // Vote NO-FUNDING for the rest of the active triggers + const auto activeTriggers = GetActiveTriggers(); + for (const auto& trigger : activeTriggers) { + const auto govobj = FindGovernanceObject(trigger->GetGovernanceObjHash()); + 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()); + continue; + } + if (trigger_hash == votedFundingYesTriggerHash) { + // Skip actual trigger + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); + continue; + } + if (vote_rec_t voteRecord; govobj->GetCurrentMNVotes(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, /* 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; + } + } + 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 + continue; + } + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Voting NO-FUNDING for trigger:%s success\n", __func__, trigger_hash.ToString()); + } +} + +bool CGovernanceManager::VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman, PeerManager& peerman, + const CActiveMasternodeManager& mn_activeman) +{ + CGovernanceVote vote(mn_activeman.GetOutPoint(), nHash, VOTE_SIGNAL_FUNDING, outcome); + vote.SetTime(GetAdjustedTime()); + vote.SetSignature(mn_activeman.SignBasic(vote.GetSignatureHash())); + + CGovernanceException exception; + if (!ProcessVoteAndRelay(vote, exception, connman, peerman)) { + LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Vote FUNDING %d for trigger:%s failed:%s\n", __func__, outcome, nHash.ToString(), exception.what()); + return false; + } + + return true; +} + +bool CGovernanceManager::HasAlreadyVotedFundingTrigger() const +{ + return votedFundingYesTriggerHash.has_value(); +} + +void CGovernanceManager::ResetVotedFundingTrigger() +{ + votedFundingYesTriggerHash = std::nullopt; +} From bd6af835fff8958fd69cff6784d26f2646c7729d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 22:09:20 +0000 Subject: [PATCH 03/14] refactor: separate masternode mode logic into dedicated signer class --- src/Makefile.am | 3 +- src/dsnotificationinterface.cpp | 4 +- src/dsnotificationinterface.h | 3 - src/governance/governance.cpp | 23 +-- src/governance/governance.h | 51 +++---- src/governance/signing.cpp | 141 +++++++++++------- src/governance/signing.h | 56 +++++++ src/init.cpp | 8 +- src/masternode/active/context.cpp | 9 +- src/masternode/active/context.h | 9 +- .../active/notificationinterface.cpp | 2 + src/node/interfaces.cpp | 1 + src/qt/governancelist.h | 9 +- src/wallet/interfaces.cpp | 1 - test/functional/feature_governance.py | 2 +- 15 files changed, 204 insertions(+), 118 deletions(-) create mode 100644 src/governance/signing.h diff --git a/src/Makefile.am b/src/Makefile.am index e2a817d65680d..639a7d9eff83d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -222,11 +222,12 @@ BITCOIN_CORE_H = \ evo/specialtxman.h \ evo/types.h \ dsnotificationinterface.h \ - governance/governance.h \ governance/classes.h \ governance/common.h \ + governance/governance.h \ governance/exceptions.h \ governance/object.h \ + governance/signing.h \ governance/validators.h \ governance/vote.h \ governance/votedb.h \ diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 95066be54658c..4f8c7ed0358af 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -26,7 +26,6 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman, CGovernanceManager& govman, PeerManager& peerman, const ChainstateManager& chainman, - const CActiveMasternodeManager* const mn_activeman, const std::unique_ptr& dmnman, const std::unique_ptr& llmq_ctx, const std::unique_ptr& cj_ctx) @@ -35,7 +34,6 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman, m_govman(govman), m_peerman(peerman), m_chainman(chainman), - m_mn_activeman(mn_activeman), m_dmnman(dmnman), m_llmq_ctx(llmq_ctx), m_cj_ctx(cj_ctx) {} @@ -94,7 +92,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con m_llmq_ctx->qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload); if (m_govman.IsValid()) { - m_govman.UpdatedBlockTip(pindexNew, m_connman, m_peerman, m_mn_activeman); + m_govman.UpdatedBlockTip(pindexNew, m_peerman); } } diff --git a/src/dsnotificationinterface.h b/src/dsnotificationinterface.h index 6ca8634e2cc1c..9ae958be5f628 100644 --- a/src/dsnotificationinterface.h +++ b/src/dsnotificationinterface.h @@ -7,7 +7,6 @@ #include -class CActiveMasternodeManager; class CConnman; class CDeterministicMNManager; class CGovernanceManager; @@ -25,7 +24,6 @@ class CDSNotificationInterface : public CValidationInterface CGovernanceManager& govman, PeerManager& peerman, const ChainstateManager& chainman, - const CActiveMasternodeManager* const mn_activeman, const std::unique_ptr& dmnman, const std::unique_ptr& llmq_ctx, const std::unique_ptr& cj_ctx); @@ -54,7 +52,6 @@ class CDSNotificationInterface : public CValidationInterface CGovernanceManager& m_govman; PeerManager& m_peerman; const ChainstateManager& m_chainman; - const CActiveMasternodeManager* const m_mn_activeman; const std::unique_ptr& m_dmnman; const std::unique_ptr& m_llmq_ctx; const std::unique_ptr& m_cj_ctx; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index af1451ea38353..3e6d78b7e0454 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -75,6 +74,7 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi m_mn_sync{mn_sync}, nTimeLastDiff(0), nCachedBlockHeight(0), + mapPostponedObjects(), fRateChecksEnabled(true), votedFundingYesTriggerHash(std::nullopt), mapTrigger{} @@ -140,6 +140,12 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& return cmapVoteToObject.Get(nHash, pGovobj) && pGovobj->GetVoteFile().SerializeVoteToStream(nHash, ss); } +void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) +{ + LOCK(cs); + mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); +} + MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) { if (!IsValid()) return {}; @@ -998,6 +1004,11 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter)); } +void CGovernanceManager::AddInvalidVote(const CGovernanceVote& vote) +{ + cmapInvalidVotes.Insert(vote.GetHash(), vote); +} + int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const { const std::vector vNodeCopy{&peer}; @@ -1246,7 +1257,7 @@ UniValue CGovernanceManager::ToJson() const return jsonObj; } -void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, CConnman& connman, PeerManager& peerman, const CActiveMasternodeManager* const mn_activeman) +void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman) { // Note this gets called from ActivateBestChain without cs_main being held // so it should be safe to lock our mutex here without risking a deadlock @@ -1257,12 +1268,6 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, CConnman& co return; } - if (mn_activeman) { - const auto sb_opt = CreateSuperblockCandidate(pindex->nHeight); - const auto trigger_opt = CreateGovernanceTrigger(sb_opt, peerman, *mn_activeman); - VoteGovernanceTriggers(trigger_opt, connman, peerman, *mn_activeman); - } - nCachedBlockHeight = pindex->nHeight; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight); @@ -1655,11 +1660,9 @@ void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_m // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. pSuperblock->SetExecuted(); - ResetVotedFundingTrigger(); } } - bool AreSuperblocksEnabled(const CSporkManager& sporkman) { return sporkman.IsSporkActive(SPORK_9_SUPERBLOCKS_ENABLED); diff --git a/src/governance/governance.h b/src/governance/governance.h index 2657c94602c02..9ecd9b391ff7c 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -5,27 +5,33 @@ #ifndef BITCOIN_GOVERNANCE_GOVERNANCE_H #define BITCOIN_GOVERNANCE_GOVERNANCE_H -#include -#include - #include #include #include -#include +#include -#include +#include +#include +#include +#include +#include #include +#include class CBloomFilter; class CBlockIndex; +class CChain; class CConnman; +class ChainstateManager; template class CFlatDB; class CInv; +class CNode; class PeerManager; class CDeterministicMNList; class CDeterministicMNManager; +class CGovernanceException; class CGovernanceManager; class CGovernanceObject; class CGovernanceVote; @@ -33,6 +39,14 @@ class CMasternodeMetaMan; class CMasternodeSync; class CNetFulfilledRequestManager; class CSporkManager; +class CSuperblock; +class GovernanceSigner; + +class UniValue; + +using CDeterministicMNListPtr = std::shared_ptr; +using CSuperblock_sptr = std::shared_ptr; +using vote_time_pair_t = std::pair; static constexpr int RATE_BUFFER_SIZE = 5; static constexpr bool DEFAULT_GOVERNANCE_ENABLE{true}; @@ -275,10 +289,6 @@ class CGovernanceManager : public GovernanceStore [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv); -private: - void ResetVotedFundingTrigger(); - -public: void DoMaintenance(CConnman& connman); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -296,7 +306,7 @@ class CGovernanceManager : public GovernanceStore UniValue ToJson() const; - void UpdatedBlockTip(const CBlockIndex* pindex, CConnman& connman, PeerManager& peerman, const CActiveMasternodeManager* const mn_activeman); + void UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman); int64_t GetLastDiffTime() const { return nTimeLastDiff; } void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } @@ -313,11 +323,7 @@ class CGovernanceManager : public GovernanceStore bool SerializeVoteForHash(const uint256& nHash, CDataStream& ss) const; - void AddPostponedObject(const CGovernanceObject& govobj) - { - LOCK(cs); - mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); - } + void AddPostponedObject(const CGovernanceObject& govobj); void MasternodeRateUpdate(const CGovernanceObject& govobj); @@ -375,21 +381,9 @@ class CGovernanceManager : public GovernanceStore bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); - std::optional CreateSuperblockCandidate(int nHeight) const; - std::optional CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman); - void VoteGovernanceTriggers(const std::optional& trigger_opt, CConnman& connman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman); - bool VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome, CConnman& connman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman); - bool HasAlreadyVotedFundingTrigger() const; - void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const; - void AddInvalidVote(const CGovernanceVote& vote) - { - cmapInvalidVotes.Insert(vote.GetHash(), vote); - } + void AddInvalidVote(const CGovernanceVote& vote); bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman); @@ -408,6 +402,7 @@ class CGovernanceManager : public GovernanceStore void RemoveInvalidVotes(); + friend class GovernanceSigner; }; bool AreSuperblocksEnabled(const CSporkManager& sporkman); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 0c6a79ea1391a..86b9ae5137bfd 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -2,10 +2,11 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include +#include #include #include +#include #include #include @@ -17,16 +18,31 @@ #include -std::optional CGovernanceManager::CreateSuperblockCandidate(int nHeight) const +GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, CGovernanceManager& govman, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const ChainstateManager& chainman, const CMasternodeSync& mn_sync) : + m_connman{connman}, + m_dmnman{dmnman}, + m_govman{govman}, + m_peerman{peerman}, + m_mn_activeman{mn_activeman}, + m_chainman{chainman}, + m_mn_sync{mn_sync} +{ +} + +GovernanceSigner::~GovernanceSigner() = default; + +std::optional GovernanceSigner::CreateSuperblockCandidate(int nHeight) const { - if (!IsValid()) return std::nullopt; + if (!m_govman.IsValid()) return std::nullopt; if (!m_mn_sync.IsSynced()) return std::nullopt; if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; if (HasAlreadyVotedFundingTrigger()) return std::nullopt; // A proposal is considered passing if (YES votes) >= (Total Weight of Masternodes / 10), // count total valid (ENABLED) masternodes to determine passing threshold. - const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); + const auto tip_mn_list = m_dmnman.GetListAtChainTip(); const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); @@ -34,8 +50,8 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i std::vector> approvedProposals; { - LOCK(cs); - for (const auto& [unused, object] : mapObjects) { + LOCK(m_govman.cs); + for (const auto& [unused, object] : m_govman.mapObjects) { // Skip all non-proposals objects if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; @@ -55,7 +71,7 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i }); if (approvedProposals.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d empty approvedProposals\n", __func__, nHeight); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d empty approvedProposals\n", __func__, nHeight); return std::nullopt; } @@ -80,7 +96,7 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i nAmount = ParsePaymentAmount(jproposal["payment_amount"].getValStr()); } catch (const std::runtime_error& e) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d Skipping payment exception:%s\n", __func__,nHeight, e.what()); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d Skipping payment exception:%s\n", __func__,nHeight, e.what()); continue; } @@ -96,11 +112,11 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i // Skip proposals if the SB isn't within the proposal time window if (SBEpochTime < windowStart) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowStart:%d\n", __func__,nHeight, SBEpochTime, windowStart); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowStart:%d\n", __func__,nHeight, SBEpochTime, windowStart); continue; } if (SBEpochTime > windowEnd) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s nHeight:%d SB:%d windowEnd:%d\n", __func__,nHeight, SBEpochTime, windowEnd); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowEnd:%d\n", __func__,nHeight, SBEpochTime, windowEnd); continue; } @@ -113,7 +129,7 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i // No proposals made the cut if (payments.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s CreateSuperblockCandidate nHeight:%d empty payments\n", __func__, nHeight); + LogPrint(BCLog::GOBJECT, "%s -- CreateSuperblockCandidate nHeight:%d empty payments\n", __func__, nHeight); return std::nullopt; } @@ -126,61 +142,59 @@ std::optional CGovernanceManager::CreateSuperblockCandidate(i return CSuperblock(nNextSuperblock, std::move(payments)); } -std::optional CGovernanceManager::CreateGovernanceTrigger(const std::optional& sb_opt, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman) +std::optional GovernanceSigner::CreateGovernanceTrigger(const std::optional& sb_opt) { // no sb_opt, no trigger if (!sb_opt.has_value()) return std::nullopt; //TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct - LOCK2(::cs_main, cs); + LOCK2(::cs_main, m_govman.cs); // Check if identical trigger (equal DataHash()) is already created (signed by other masternode) CGovernanceObject gov_sb(uint256(), 1, GetAdjustedTime(), uint256(), sb_opt.value().GetHexStrData()); - if (auto identical_sb = FindGovernanceObjectByDataHash(gov_sb.GetDataHash())) { + if (auto identical_sb = m_govman.FindGovernanceObjectByDataHash(gov_sb.GetDataHash())) { // Somebody submitted a trigger with the same data, support it instead of submitting a duplicate return std::make_optional(*identical_sb); } // Nobody submitted a trigger we'd like to see, so let's do it but only if we are the payee const CBlockIndex* tip = m_chainman.ActiveChain().Tip(); - const auto mnList = Assert(m_dmnman)->GetListForBlock(tip); + const auto mnList = m_dmnman.GetListForBlock(tip); const auto mn_payees = mnList.GetProjectedMNPayees(tip); if (mn_payees.empty()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s payee list is empty\n", __func__); + LogPrint(BCLog::GOBJECT, "%s -- payee list is empty\n", __func__); return std::nullopt; } - if (mn_payees.front()->proTxHash != mn_activeman.GetProTxHash()) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s we are not the payee, skipping\n", __func__); + if (mn_payees.front()->proTxHash != m_mn_activeman.GetProTxHash()) { + LogPrint(BCLog::GOBJECT, "%s -- we are not the payee, skipping\n", __func__); return std::nullopt; } - gov_sb.SetMasternodeOutpoint(mn_activeman.GetOutPoint()); - gov_sb.SetSignature(mn_activeman.SignBasic(gov_sb.GetSignatureHash())); + gov_sb.SetMasternodeOutpoint(m_mn_activeman.GetOutPoint()); + gov_sb.SetSignature(m_mn_activeman.SignBasic(gov_sb.GetSignatureHash())); - if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), m_chainman, strError, true)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError); + if (std::string strError; !gov_sb.IsValidLocally(m_dmnman.GetListAtChainTip(), m_chainman, strError, true)) { + LogPrint(BCLog::GOBJECT, "%s -- Created trigger is invalid:%s\n", __func__, strError); return std::nullopt; } - if (!MasternodeRateCheck(gov_sb)) { - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Trigger rejected because of rate check failure hash(%s)\n", __func__, gov_sb.GetHash().ToString()); + if (!m_govman.MasternodeRateCheck(gov_sb)) { + LogPrint(BCLog::GOBJECT, "%s -- Trigger rejected because of rate check failure hash(%s)\n", __func__, gov_sb.GetHash().ToString()); return std::nullopt; } // The trigger we just created looks good, submit it - AddGovernanceObject(gov_sb, peerman); + m_govman.AddGovernanceObject(gov_sb, m_peerman); return std::make_optional(gov_sb); } -void CGovernanceManager::VoteGovernanceTriggers(const std::optional& trigger_opt, CConnman& connman, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman) +void GovernanceSigner::VoteGovernanceTriggers(const std::optional& trigger_opt) { // only active masternodes can vote on triggers - if (mn_activeman.GetProTxHash().IsNull()) return; + if (m_mn_activeman.GetProTxHash().IsNull()) return; - LOCK2(::cs_main, cs); + LOCK2(::cs_main, m_govman.cs); if (trigger_opt.has_value()) { // We should never vote "yes" on another trigger or the same trigger twice @@ -188,7 +202,7 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalGetGovernanceObjHash()); + const auto govobj = m_govman.FindGovernanceObject(trigger->GetGovernanceObjHash()); const uint256 trigger_hash = govobj->GetHash(); - if (trigger->GetBlockHeight() <= nCachedBlockHeight) { + if (trigger->GetBlockHeight() <= m_govman.nCachedBlockHeight) { // ignore triggers from the past - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for outdated trigger:%s\n", __func__, trigger_hash.ToString()); + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for outdated trigger:%s\n", __func__, trigger_hash.ToString()); continue; } if (trigger_hash == votedFundingYesTriggerHash) { // Skip actual trigger - LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); continue; } - if (vote_rec_t voteRecord; govobj->GetCurrentMNVotes(mn_activeman.GetOutPoint(), voteRecord)) { + if (vote_rec_t voteRecord; govobj->GetCurrentMNVotes(m_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, /* Continued */ - "CGovernanceManager::%s " - "Not voting NO-FUNDING for trigger:%s, we voted %s for it already\n", + "%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; @@ -252,37 +264,50 @@ void CGovernanceManager::VoteGovernanceTriggers(const std::optionalnHeight); + const auto trigger_opt = CreateGovernanceTrigger(sb_opt); + VoteGovernanceTriggers(trigger_opt); + { + LOCK(m_govman.cs); + CSuperblock_sptr pSuperblock; + if (m_govman.GetBestSuperblock(m_dmnman.GetListAtChainTip(), pSuperblock, pindex->nHeight)) { + ResetVotedFundingTrigger(); + } + } +} diff --git a/src/governance/signing.h b/src/governance/signing.h new file mode 100644 index 0000000000000..77d6260145dcc --- /dev/null +++ b/src/governance/signing.h @@ -0,0 +1,56 @@ +// Copyright (c) 2014-2025 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_GOVERNANCE_SIGNING_H +#define BITCOIN_GOVERNANCE_SIGNING_H + +#include +#include + +#include + +#include + +class CActiveMasternodeManager; +class CBlockIndex; +class CConnman; +class CDeterministicMNManager; +class CGovernanceManager; +class ChainstateManager; +class CMasternodeSync; +class PeerManager; +enum vote_outcome_enum_t : int; + +class GovernanceSigner +{ +private: + CConnman& m_connman; + CDeterministicMNManager& m_dmnman; + CGovernanceManager& m_govman; + PeerManager& m_peerman; + const CActiveMasternodeManager& m_mn_activeman; + const ChainstateManager& m_chainman; + const CMasternodeSync& m_mn_sync; + +private: + std::optional votedFundingYesTriggerHash{std::nullopt}; + +public: + explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, CGovernanceManager& govman, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const ChainstateManager& chainman, const CMasternodeSync& mn_sync); + ~GovernanceSigner(); + + void UpdatedBlockTip(const CBlockIndex* pindex); + +private: + bool HasAlreadyVotedFundingTrigger() const; + bool VoteFundingTrigger(const uint256& nHash, const vote_outcome_enum_t outcome); + std::optional CreateGovernanceTrigger(const std::optional& sb_opt); + std::optional CreateSuperblockCandidate(int nHeight) const; + void ResetVotedFundingTrigger(); + void VoteGovernanceTriggers(const std::optional& trigger_opt); +}; + +#endif // BITCOIN_GOVERNANCE_SIGNING_H diff --git a/src/init.cpp b/src/init.cpp index 55bb5fb72bcb7..8b82d9a9e8dd7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2140,7 +2140,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) RegisterValidationInterface(node.peerman.get()); g_ds_notification_interface = std::make_unique( - *node.connman, *node.mn_sync, *node.govman, *node.peerman, chainman, node.mn_activeman.get(), node.dmnman, node.llmq_ctx, node.cj_ctx + *node.connman, *node.mn_sync, *node.govman, *node.peerman, chainman, node.dmnman, node.llmq_ctx, node.cj_ctx ); RegisterValidationInterface(g_ds_notification_interface.get()); @@ -2154,9 +2154,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.active_ctx); assert(!g_active_notification_interface); if (node.mn_activeman) { - node.active_ctx = std::make_unique(chainman, *node.connman, *node.dmnman, *node.cj_ctx->dstxman, *node.mn_metaman, *node.mnhf_manager, - *node.llmq_ctx, *node.sporkman, *node.mempool, *node.peerman, *node.mn_activeman, - *node.mn_sync); + node.active_ctx = std::make_unique(chainman, *node.connman, *node.dmnman, *node.cj_ctx->dstxman, *node.govman, *node.mn_metaman, + *node.mnhf_manager, *node.sporkman, *node.mempool, *node.llmq_ctx, *node.peerman, + *node.mn_activeman, *node.mn_sync); g_active_notification_interface = std::make_unique(*node.active_ctx, *node.mn_activeman); RegisterValidationInterface(g_active_notification_interface.get()); } diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 3fdfa75bad39a..1cc39f8485e4a 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -14,9 +15,10 @@ #include ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman, - CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, - LLMQContext& llmq_ctx, CSporkManager& sporkman, CTxMemPool& mempool, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync) : + CDSTXManager& dstxman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman, + CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, LLMQContext& llmq_ctx, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const CMasternodeSync& mn_sync) : m_llmq_ctx{llmq_ctx}, cl_signer{std::make_unique(chainman.ActiveChainstate(), *llmq_ctx.clhandler, *llmq_ctx.sigman, *llmq_ctx.shareman, sporkman, mn_sync)}, @@ -25,6 +27,7 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe *llmq_ctx.qman, sporkman, mempool, mn_sync)}, cj_server{std::make_unique(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman, mn_activeman, mn_sync, *llmq_ctx.isman)}, + gov_signer{std::make_unique(connman, dmnman, govman, peerman, mn_activeman, chainman, mn_sync)}, ehf_sighandler{std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *llmq_ctx.shareman, *llmq_ctx.qman)} { diff --git a/src/masternode/active/context.h b/src/masternode/active/context.h index 7dd04a507b47e..ca1cf697ecc9c 100644 --- a/src/masternode/active/context.h +++ b/src/masternode/active/context.h @@ -13,11 +13,13 @@ class CCoinJoinServer; class CConnman; class CDeterministicMNManager; class CDSTXManager; +class CGovernanceManager; class CMasternodeMetaMan; class CMasternodeSync; class CMNHFManager; class CSporkManager; class CTxMemPool; +class GovernanceSigner; class PeerManager; struct LLMQContext; namespace chainlock { @@ -46,9 +48,9 @@ struct ActiveContext { ActiveContext() = delete; ActiveContext(const ActiveContext&) = delete; ActiveContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman, - CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, LLMQContext& llmq_ctx, - CSporkManager& sporkman, CTxMemPool& mempool, PeerManager& peerman, - const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync); + CDSTXManager& dstxman, CGovernanceManager& govman, CMasternodeMetaMan& mn_metaman, + CMNHFManager& mnhfman, CSporkManager& sporkman, CTxMemPool& mempool, LLMQContext& llmq_ctx, + PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const CMasternodeSync& mn_sync); ~ActiveContext(); /* @@ -57,6 +59,7 @@ struct ActiveContext { * TODO: Move CActiveMasternodeManager here when dependents have been migrated */ const std::unique_ptr cj_server; + const std::unique_ptr gov_signer; const std::unique_ptr ehf_sighandler; }; diff --git a/src/masternode/active/notificationinterface.cpp b/src/masternode/active/notificationinterface.cpp index 215458a9e9dd5..aee9f1917afcb 100644 --- a/src/masternode/active/notificationinterface.cpp +++ b/src/masternode/active/notificationinterface.cpp @@ -4,6 +4,7 @@ #include +#include #include #include #include @@ -22,6 +23,7 @@ void ActiveNotificationInterface::UpdatedBlockTip(const CBlockIndex* pindexNew, m_mn_activeman.UpdatedBlockTip(pindexNew, pindexFork, fInitialDownload); m_active_ctx.ehf_sighandler->UpdatedBlockTip(pindexNew); + m_active_ctx.gov_signer->UpdatedBlockTip(pindexNew); } std::unique_ptr g_active_notification_interface; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 420dd9f6baf37..14177a6b3a13e 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include diff --git a/src/qt/governancelist.h b/src/qt/governancelist.h index 1f696d7f16697..dd79fe51e5712 100644 --- a/src/qt/governancelist.h +++ b/src/qt/governancelist.h @@ -5,13 +5,14 @@ #ifndef BITCOIN_QT_GOVERNANCELIST_H #define BITCOIN_QT_GOVERNANCELIST_H -#include -#include #include +#include #include #include #include +#include + #include #include #include @@ -28,12 +29,14 @@ namespace Ui { class GovernanceList; } -class CDeterministicMNList; class ClientModel; class ProposalModel; class WalletModel; class ProposalWizard; +class CDeterministicMNList; +enum vote_outcome_enum_t : int; + /** Governance Manager page widget */ class GovernanceList : public QWidget { diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 324bea3952a11..4c134c76220ea 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include diff --git a/test/functional/feature_governance.py b/test/functional/feature_governance.py index 9ed1e59d49919..82025d06d6cdc 100755 --- a/test/functional/feature_governance.py +++ b/test/functional/feature_governance.py @@ -317,7 +317,7 @@ def sync_gov(node): self.bump_mocktime(GOVERNANCE_UPDATE_MIN + 1, update_schedulers=False) self.log.info("Move another block inside the Superblock maturity window") - with self.nodes[1].assert_debug_log(["CGovernanceManager::VoteGovernanceTriggers"]): + with self.nodes[1].assert_debug_log(["VoteGovernanceTriggers --"]): self.bump_mocktime(1) self.generate(self.nodes[0], 1, sync_fun=self.sync_blocks()) From 757ded31dd715796d7546500b21c37a35db9e698 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 18:10:15 +0000 Subject: [PATCH 04/14] refactor: remove need for access to private members --- src/governance/governance.cpp | 81 ++++++++++++++++++++++++++++------- src/governance/governance.h | 22 +++++++--- src/governance/signing.cpp | 46 +++----------------- 3 files changed, 87 insertions(+), 62 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 3e6d78b7e0454..448baaa477f04 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -505,21 +505,25 @@ const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uin CGovernanceObject* CGovernanceManager::FindGovernanceObject(const uint256& nHash) { - AssertLockHeld(cs); + AssertLockNotHeld(cs); + LOCK(cs); + return FindGovernanceObjectInternal(nHash); +} +CGovernanceObject* CGovernanceManager::FindGovernanceObjectInternal(const uint256& nHash) +{ + AssertLockHeld(cs); if (mapObjects.count(nHash)) return &mapObjects[nHash]; - return nullptr; } CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint256 &nDataHash) { - AssertLockHeld(cs); - + AssertLockNotHeld(cs); + LOCK(cs); for (const auto& [nHash, object] : mapObjects) { if (object.GetDataHash() == nDataHash) return &mapObjects[nHash]; } - return nullptr; } @@ -1392,7 +1396,7 @@ bool CGovernanceManager::AddNewTrigger(uint256 nHash) CSuperblock_sptr pSuperblock; try { - const CGovernanceObject* pGovObj = FindGovernanceObject(nHash); + const CGovernanceObject* pGovObj = FindGovernanceObjectInternal(nHash); if (!pGovObj) { throw std::runtime_error("CSuperblock: Failed to find Governance Object"); } @@ -1434,7 +1438,7 @@ void CGovernanceManager::CleanAndRemoveTriggers() LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- nullptr superblock\n", __func__); remove = true; } else { - pObj = FindGovernanceObject(it->first); + pObj = FindGovernanceObjectInternal(it->first); if (!pObj || pObj->GetObjectType() != GovernanceObject::TRIGGER) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- Unknown or non-trigger superblock\n", __func__); pSuperblock->SetStatus(SeenObjectStatus::ErrorInvalid); @@ -1487,8 +1491,14 @@ void CGovernanceManager::CleanAndRemoveTriggers() * - Look through triggers and scan for active ones * - Return the triggers in a list */ - std::vector CGovernanceManager::GetActiveTriggers() const +{ + AssertLockNotHeld(cs); + LOCK(cs); + return GetActiveTriggersInternal(); +} + +std::vector CGovernanceManager::GetActiveTriggersInternal() const { AssertLockHeld(cs); std::vector vecResults; @@ -1513,7 +1523,7 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m LOCK(cs); // GET ALL ACTIVE TRIGGERS - std::vector vecTriggers = GetActiveTriggers(); + std::vector vecTriggers = GetActiveTriggersInternal(); LogPrint(BCLog::GOBJECT, "IsSuperblockTriggered -- vecTriggers.size() = %d\n", vecTriggers.size()); @@ -1523,7 +1533,7 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m continue; } - CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); + CGovernanceObject* pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); if (!pObj) { LogPrintf("IsSuperblockTriggered -- pObj == nullptr, continuing\n"); continue; @@ -1556,16 +1566,23 @@ bool CGovernanceManager::IsSuperblockTriggered(const CDeterministicMNList& tip_m return false; } - bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) +{ + AssertLockNotHeld(cs); + LOCK(cs); + return GetBestSuperblockInternal(tip_mn_list, pSuperblockRet, nBlockHeight); +} + +bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) { if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; } AssertLockHeld(cs); - std::vector vecTriggers = GetActiveTriggers(); + std::vector vecTriggers = GetActiveTriggersInternal(); int nYesCount = 0; for (const auto& pSuperblock : vecTriggers) { @@ -1573,7 +1590,7 @@ bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_li continue; } - const CGovernanceObject* pObj = FindGovernanceObject(pSuperblock->GetGovernanceObjHash()); + const CGovernanceObject* pObj = FindGovernanceObjectInternal(pSuperblock->GetGovernanceObjHash()); if (!pObj) { continue; } @@ -1598,7 +1615,7 @@ bool CGovernanceManager::GetSuperblockPayments(const CDeterministicMNList& tip_m // GET THE BEST SUPERBLOCK FOR THIS BLOCK HEIGHT CSuperblock_sptr pSuperblock; - if (!GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + if (!GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { LogPrint(BCLog::GOBJECT, "GetSuperblockPayments -- Can't find superblock for height %d\n", nBlockHeight); return false; } @@ -1644,7 +1661,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe LOCK(cs); CSuperblock_sptr pSuperblock; - if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { return pSuperblock->IsValid(active_chain, txNew, nBlockHeight, blockReward); } @@ -1656,13 +1673,45 @@ void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_m LOCK(cs); CSuperblock_sptr pSuperblock; - if (GetBestSuperblock(tip_mn_list, pSuperblock, nBlockHeight)) { + if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { // All checks are done in CSuperblock::IsValid via IsBlockValueValid and IsBlockPayeeValid, // tip wouldn't be updated if anything was wrong. Mark this trigger as executed. pSuperblock->SetExecuted(); } } +std::vector> CGovernanceManager::GetApprovedProposals(const CDeterministicMNList& tip_mn_list) +{ + AssertLockNotHeld(cs); + + // Use std::vector of std::shared_ptr because CGovernanceObject doesn't support move operations (needed for sorting the vector later) + std::vector> ret{}; + + // A proposal is considered passing if (YES votes) >= (Total Weight of Masternodes / 10), + // count total valid (ENABLED) masternodes to determine passing threshold. + const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); + const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); + + LOCK(cs); + for (const auto& [unused, object] : mapObjects) { + // Skip all non-proposals objects + if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; + // Skip non-passing proposals + const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + if (absYesCount < nAbsVoteReq) continue; + ret.emplace_back(std::make_shared(object)); + } + + // Sort approved proposals by absolute Yes votes descending + std::sort(ret.begin(), ret.end(), [&tip_mn_list](auto& lhs, auto& rhs) { + const auto lhs_yes = lhs->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + const auto rhs_yes = rhs->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); + return lhs_yes == rhs_yes ? UintToArith256(lhs->GetHash()) > UintToArith256(rhs->GetHash()) : lhs_yes > rhs_yes; + }); + + return ret; +} + bool AreSuperblocksEnabled(const CSporkManager& sporkman) { return sporkman.IsSporkActive(SPORK_9_SUPERBLOCKS_ENABLED); diff --git a/src/governance/governance.h b/src/governance/governance.h index 9ecd9b391ff7c..4e350e9feb427 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -292,8 +292,8 @@ class CGovernanceManager : public GovernanceStore void DoMaintenance(CConnman& connman); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObject(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) EXCLUSIVE_LOCKS_REQUIRED(cs); + CGovernanceObject* FindGovernanceObject(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); + CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); void DeleteGovernanceObject(const uint256& nHash); // These commands are only used in RPC @@ -352,7 +352,7 @@ class CGovernanceManager : public GovernanceStore * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - std::vector> GetActiveTriggers() const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetActiveTriggers() const EXCLUSIVE_LOCKS_REQUIRED(!cs); bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -376,11 +376,21 @@ class CGovernanceManager : public GovernanceStore bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); -private: - void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + + std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) + EXCLUSIVE_LOCKS_REQUIRED(!cs); + +private: + //! Internal functions that require locks to be held + CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); + bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); + void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); + void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const; void AddInvalidVote(const CGovernanceVote& vote); @@ -401,8 +411,6 @@ class CGovernanceManager : public GovernanceStore void CleanOrphanObjects(); void RemoveInvalidVotes(); - - friend class GovernanceSigner; }; bool AreSuperblocksEnabled(const CSporkManager& sporkman); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 86b9ae5137bfd..184a3fcbf6003 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -40,36 +40,7 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; if (HasAlreadyVotedFundingTrigger()) return std::nullopt; - // A proposal is considered passing if (YES votes) >= (Total Weight of Masternodes / 10), - // count total valid (ENABLED) masternodes to determine passing threshold. - const auto tip_mn_list = m_dmnman.GetListAtChainTip(); - const int nWeightedMnCount = tip_mn_list.GetValidWeightedMNsCount(); - const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10); - - // Use std::vector of std::shared_ptr because CGovernanceObject doesn't support move operations (needed for sorting the vector later) - std::vector> approvedProposals; - - { - LOCK(m_govman.cs); - for (const auto& [unused, object] : m_govman.mapObjects) { - // Skip all non-proposals objects - if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue; - - const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - // Skip non-passing proposals - if (absYesCount < nAbsVoteReq) continue; - - approvedProposals.emplace_back(std::make_shared(object)); - } - } // cs - - // Sort approved proposals by absolute Yes votes descending - std::sort(approvedProposals.begin(), approvedProposals.end(), [tip_mn_list](std::shared_ptr a, std::shared_ptr b) { - const auto a_yes = a->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - const auto b_yes = b->GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING); - return a_yes == b_yes ? UintToArith256(a->GetHash()) > UintToArith256(b->GetHash()) : a_yes > b_yes; - }); - + const auto approvedProposals = m_govman.GetApprovedProposals(m_dmnman.GetListAtChainTip()); if (approvedProposals.empty()) { LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d empty approvedProposals\n", __func__, nHeight); return std::nullopt; @@ -148,7 +119,7 @@ std::optional GovernanceSigner::CreateGovernanceTrigger if (!sb_opt.has_value()) return std::nullopt; //TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct - LOCK2(::cs_main, m_govman.cs); + LOCK(::cs_main); // Check if identical trigger (equal DataHash()) is already created (signed by other masternode) CGovernanceObject gov_sb(uint256(), 1, GetAdjustedTime(), uint256(), sb_opt.value().GetHexStrData()); @@ -194,7 +165,7 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetGovernanceObjHash()); const uint256 trigger_hash = govobj->GetHash(); - if (trigger->GetBlockHeight() <= m_govman.nCachedBlockHeight) { + if (trigger->GetBlockHeight() <= m_govman.GetCachedBlockHeight()) { // ignore triggers from the past LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for outdated trigger:%s\n", __func__, trigger_hash.ToString()); continue; @@ -303,11 +274,8 @@ void GovernanceSigner::UpdatedBlockTip(const CBlockIndex* pindex) const auto sb_opt = CreateSuperblockCandidate(pindex->nHeight); const auto trigger_opt = CreateGovernanceTrigger(sb_opt); VoteGovernanceTriggers(trigger_opt); - { - LOCK(m_govman.cs); - CSuperblock_sptr pSuperblock; - if (m_govman.GetBestSuperblock(m_dmnman.GetListAtChainTip(), pSuperblock, pindex->nHeight)) { - ResetVotedFundingTrigger(); - } + CSuperblock_sptr pSuperblock; + if (m_govman.GetBestSuperblock(m_dmnman.GetListAtChainTip(), pSuperblock, pindex->nHeight)) { + ResetVotedFundingTrigger(); } } From 63448ff1cc50e5363e205b7554309f826bda9fb3 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 12 Sep 2025 14:02:12 +0000 Subject: [PATCH 05/14] refactor: abstract away parent implementation from signer --- src/governance/governance.h | 26 +++++++++++++++----------- src/governance/signing.cpp | 3 +-- src/governance/signing.h | 28 +++++++++++++++++++++++++--- src/masternode/active/context.cpp | 1 + 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/governance/governance.h b/src/governance/governance.h index 4e350e9feb427..9e1219a1458c8 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -7,12 +7,16 @@ #include #include +#include + #include #include +#include #include #include #include +#include #include #include #include @@ -236,7 +240,7 @@ class GovernanceStore // // Governance Manager : Contains all proposals for the budget // -class CGovernanceManager : public GovernanceStore +class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent { friend class CGovernanceObject; @@ -275,7 +279,7 @@ class CGovernanceManager : public GovernanceStore bool LoadCache(bool load_cache); - bool IsValid() const { return is_valid; } + bool IsValid() const override { return is_valid; } /** * This is called by AlreadyHave in net_processing.cpp as part of the inventory @@ -292,15 +296,15 @@ class CGovernanceManager : public GovernanceStore void DoMaintenance(CConnman& connman); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); - CGovernanceObject* FindGovernanceObject(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); - CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); + CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); + CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); void DeleteGovernanceObject(const uint256& nHash); // These commands are only used in RPC std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; - void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr); + void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) override; void CheckAndRemove(); @@ -310,7 +314,7 @@ class CGovernanceManager : public GovernanceStore int64_t GetLastDiffTime() const { return nTimeLastDiff; } void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } - int GetCachedBlockHeight() const { return nCachedBlockHeight; } + int GetCachedBlockHeight() const override { return nCachedBlockHeight; } // Accessors for thread-safe access to maps bool HaveObjectForHash(const uint256& nHash) const; @@ -327,11 +331,11 @@ class CGovernanceManager : public GovernanceStore void MasternodeRateUpdate(const CGovernanceObject& govobj); - bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false); + bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override; bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); - bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman); + bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) override; void CheckPostponedObjects(PeerManager& peerman); @@ -352,7 +356,7 @@ class CGovernanceManager : public GovernanceStore * - Track governance objects which are triggers * - After triggers are activated and executed, they can be removed */ - std::vector> GetActiveTriggers() const EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::vector> GetActiveTriggers() const override EXCLUSIVE_LOCKS_REQUIRED(!cs); bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs); @@ -376,10 +380,10 @@ class CGovernanceManager : public GovernanceStore bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); - bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) + bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) + std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs); private: diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 184a3fcbf6003..3374752df9495 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include @@ -18,7 +17,7 @@ #include -GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, CGovernanceManager& govman, +GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CMasternodeSync& mn_sync) : m_connman{connman}, diff --git a/src/governance/signing.h b/src/governance/signing.h index 77d6260145dcc..044d103157d03 100644 --- a/src/governance/signing.h +++ b/src/governance/signing.h @@ -10,24 +10,46 @@ #include +#include #include +#include class CActiveMasternodeManager; class CBlockIndex; class CConnman; +class CDeterministicMNList; class CDeterministicMNManager; -class CGovernanceManager; +class CGovernanceException; +class CGovernanceVote; class ChainstateManager; class CMasternodeSync; +class CNode; class PeerManager; enum vote_outcome_enum_t : int; +class GovernanceSignerParent +{ +public: + virtual ~GovernanceSignerParent() = default; + + virtual bool IsValid() const = 0; + virtual bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, std::shared_ptr& pSuperblockRet, int nBlockHeight) = 0; + virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0; + virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) = 0; + virtual int GetCachedBlockHeight() const = 0; + virtual CGovernanceObject* FindGovernanceObject(const uint256& nHash) = 0; + virtual CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0; + virtual std::vector> GetActiveTriggers() const = 0; + virtual std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) = 0; + virtual void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) = 0; +}; + class GovernanceSigner { private: CConnman& m_connman; CDeterministicMNManager& m_dmnman; - CGovernanceManager& m_govman; + GovernanceSignerParent& m_govman; PeerManager& m_peerman; const CActiveMasternodeManager& m_mn_activeman; const ChainstateManager& m_chainman; @@ -37,7 +59,7 @@ class GovernanceSigner std::optional votedFundingYesTriggerHash{std::nullopt}; public: - explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, CGovernanceManager& govman, + explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CMasternodeSync& mn_sync); ~GovernanceSigner(); diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 1cc39f8485e4a..7b8df853e7ea8 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include From 38119244a01ee05162c8a2eca76fca1fbf1bbdd8 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Fri, 12 Sep 2025 10:04:43 +0000 Subject: [PATCH 06/14] governance: drop `Relay()` from `CGovernance{Object,Vote}` --- src/governance/governance.cpp | 40 +++++++++++++++++++++++++++++------ src/governance/governance.h | 3 +++ src/governance/object.cpp | 12 ----------- src/governance/object.h | 3 --- src/governance/vote.cpp | 17 --------------- src/governance/vote.h | 2 -- src/node/interfaces.cpp | 2 +- src/rpc/governance.cpp | 2 +- 8 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 448baaa477f04..b9f594956e717 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -98,6 +98,34 @@ bool CGovernanceManager::LoadCache(bool load_cache) return is_valid; } +void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const +{ + if (!m_mn_sync.IsSynced()) { + LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); + return; + } + + CInv inv(MSG_GOVERNANCE_OBJECT, obj.GetHash()); + peerman.RelayInv(inv); +} + +void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const +{ + if (!m_mn_sync.IsSynced()) { + LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); + return; + } + + const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip(); + auto dmn = tip_mn_list.GetMNByCollateral(vote.GetMasternodeOutpoint()); + if (!dmn) { + return; + } + + CInv inv(MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash()); + peerman.RelayInv(inv); +} + // Accessors for thread-safe access to maps bool CGovernanceManager::HaveObjectForHash(const uint256& nHash) const { @@ -273,7 +301,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman if (ProcessVote(&peer, vote, exception, connman)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- %s new\n", strHash); m_mn_sync.BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE"); - vote.Relay(peerman, m_mn_sync, tip_mn_list); + RelayMessage(peerman, vote); } else { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Rejected vote, error = %s\n", exception.what()); if ((exception.GetNodePenalty() != 0) && m_mn_sync.IsSynced()) { @@ -305,7 +333,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager if (pairVote.second < nNow) { fRemove = true; } else if (govobj.ProcessVote(m_mn_metaman, *this, tip_mn_list, vote, e)) { - vote.Relay(peerman, m_mn_sync, tip_mn_list); + RelayMessage(peerman, vote); fRemove = true; } if (fRemove) { @@ -359,7 +387,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana } LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, pfrom ? pfrom->GetLogString() : "nullptr"); - govobj.Relay(peerman, m_mn_sync); + RelayMessage(peerman, govobj); // Update the rate buffer MasternodeRateUpdate(govobj); @@ -852,9 +880,9 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) { - bool fOK = ProcessVote(/* pfrom = */ nullptr, vote, exception, connman); + bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); if (fOK) { - vote.Relay(peerman, m_mn_sync, Assert(m_dmnman)->GetListAtChainTip()); + RelayMessage(peerman, vote); } return fOK; } @@ -962,7 +990,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) if (fValid) { if (fReady) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- additional relay: hash = %s\n", govobj.GetHash().ToString()); - govobj.Relay(peerman, m_mn_sync); + RelayMessage(peerman, govobj); } else { it++; continue; diff --git a/src/governance/governance.h b/src/governance/governance.h index 9e1219a1458c8..8705e2fd44375 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -281,6 +281,9 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool IsValid() const override { return is_valid; } + void RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const; + void RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const; + /** * This is called by AlreadyHave in net_processing.cpp as part of the inventory * retrieval process. Returns true if we want to retrieve the object, otherwise diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 2e4dbfbe43e38..65f8a164bd44c 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -588,18 +588,6 @@ bool CGovernanceObject::GetCurrentMNVotes(const COutPoint& mnCollateralOutpoint, return true; } -void CGovernanceObject::Relay(PeerManager& peerman, const CMasternodeSync& mn_sync) const -{ - // Do not relay until fully synced - if (!mn_sync.IsSynced()) { - LogPrint(BCLog::GOBJECT, "CGovernanceObject::Relay -- won't relay until fully synced\n"); - return; - } - - CInv inv(MSG_GOVERNANCE_OBJECT, GetHash()); - peerman.RelayInv(inv); -} - void CGovernanceObject::UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list) { // CALCULATE MINIMUM SUPPORT LEVELS REQUIRED diff --git a/src/governance/object.h b/src/governance/object.h index 34ebaa4efa993..b3866cfa3aff7 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -24,7 +24,6 @@ class ChainstateManager; class CMasternodeMetaMan; class CMasternodeSync; class CNode; -class PeerManager; extern RecursiveMutex cs_main; @@ -253,8 +252,6 @@ class CGovernanceObject UniValue GetJSONObject() const; - void Relay(PeerManager& peerman, const CMasternodeSync& mn_sync) const; - uint256 GetHash() const; uint256 GetDataHash() const; diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index a20d2afc246de..828f46ddeaef4 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -101,23 +101,6 @@ std::string CGovernanceVote::ToString(const CDeterministicMNList& tip_mn_list) c voteWeight); } -void CGovernanceVote::Relay(PeerManager& peerman, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list) const -{ - // Do not relay until fully synced - if (!mn_sync.IsSynced()) { - LogPrint(BCLog::GOBJECT, "CGovernanceVote::Relay -- won't relay until fully synced\n"); - return; - } - - auto dmn = tip_mn_list.GetMNByCollateral(masternodeOutpoint); - if (!dmn) { - return; - } - - CInv inv(MSG_GOVERNANCE_OBJECT_VOTE, GetHash()); - peerman.RelayInv(inv); -} - void CGovernanceVote::UpdateHash() const { // Note: doesn't match serialization diff --git a/src/governance/vote.h b/src/governance/vote.h index 25f62a4408a92..65c05703f0981 100644 --- a/src/governance/vote.h +++ b/src/governance/vote.h @@ -16,7 +16,6 @@ class CGovernanceVote; class CMasternodeSync; class CKey; class CKeyID; -class PeerManager; // INTENTION OF MASTERNODES REGARDING ITEM enum vote_outcome_enum_t : int { @@ -106,7 +105,6 @@ class CGovernanceVote ::ToString(nVoteOutcome) + "|" + ::ToString(nTime); } - void Relay(PeerManager& peerman, const CMasternodeSync& mn_sync, const CDeterministicMNList& tip_mn_list) const; const COutPoint& GetMasternodeOutpoint() const { return masternodeOutpoint; } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 14177a6b3a13e..2acb653160584 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -238,7 +238,7 @@ class GOVImpl : public GOV PeerManager& peerman = EnsurePeerman(context()); if (fMissingConfirmations) { context().govman->AddPostponedObject(govobj); - govobj.Relay(peerman, *Assert(context().mn_sync)); + context().govman->RelayMessage(peerman, govobj); } else { context().govman->AddGovernanceObject(govobj, peerman); } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index ea66057b457ca..e6b1a4df0ea29 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -396,7 +396,7 @@ static RPCHelpMan gobject_submit() PeerManager& peerman = EnsurePeerman(node); if (fMissingConfirmations) { node.govman->AddPostponedObject(govobj); - govobj.Relay(peerman, *CHECK_NONFATAL(node.mn_sync)); + node.govman->RelayMessage(peerman, govobj); } else { node.govman->AddGovernanceObject(govobj, peerman); } From df589c7ab2d735a4bd598dcd9befbbf64175db30 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 22:04:58 +0000 Subject: [PATCH 07/14] governance: introduce task for relaying governance objects --- src/dsnotificationinterface.cpp | 4 +-- src/dsnotificationinterface.h | 3 -- src/governance/governance.cpp | 59 ++++++++++++++++++++----------- src/governance/governance.h | 22 +++++++----- src/governance/signing.cpp | 9 +++-- src/governance/signing.h | 8 ++--- src/init.cpp | 3 +- src/masternode/active/context.cpp | 2 +- src/net_processing.cpp | 2 +- src/node/interfaces.cpp | 9 +++-- src/rpc/governance.cpp | 11 +++--- 11 files changed, 72 insertions(+), 60 deletions(-) diff --git a/src/dsnotificationinterface.cpp b/src/dsnotificationinterface.cpp index 4f8c7ed0358af..484a564039ab7 100644 --- a/src/dsnotificationinterface.cpp +++ b/src/dsnotificationinterface.cpp @@ -24,7 +24,6 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman, CMasternodeSync& mn_sync, CGovernanceManager& govman, - PeerManager& peerman, const ChainstateManager& chainman, const std::unique_ptr& dmnman, const std::unique_ptr& llmq_ctx, @@ -32,7 +31,6 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman, : m_connman(connman), m_mn_sync(mn_sync), m_govman(govman), - m_peerman(peerman), m_chainman(chainman), m_dmnman(dmnman), m_llmq_ctx(llmq_ctx), @@ -92,7 +90,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con m_llmq_ctx->qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload); if (m_govman.IsValid()) { - m_govman.UpdatedBlockTip(pindexNew, m_peerman); + m_govman.UpdatedBlockTip(pindexNew); } } diff --git a/src/dsnotificationinterface.h b/src/dsnotificationinterface.h index 9ae958be5f628..bcb614006b6f8 100644 --- a/src/dsnotificationinterface.h +++ b/src/dsnotificationinterface.h @@ -12,7 +12,6 @@ class CDeterministicMNManager; class CGovernanceManager; class ChainstateManager; class CMasternodeSync; -class PeerManager; struct CJContext; struct LLMQContext; @@ -22,7 +21,6 @@ class CDSNotificationInterface : public CValidationInterface explicit CDSNotificationInterface(CConnman& connman, CMasternodeSync& mn_sync, CGovernanceManager& govman, - PeerManager& peerman, const ChainstateManager& chainman, const std::unique_ptr& dmnman, const std::unique_ptr& llmq_ctx, @@ -50,7 +48,6 @@ class CDSNotificationInterface : public CValidationInterface CConnman& m_connman; CMasternodeSync& m_mn_sync; CGovernanceManager& m_govman; - PeerManager& m_peerman; const ChainstateManager& m_chainman; const std::unique_ptr& m_dmnman; const std::unique_ptr& m_llmq_ctx; diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index b9f594956e717..d203c25ef7e3c 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -87,6 +88,22 @@ CGovernanceManager::~CGovernanceManager() m_db->Store(*this); } +void CGovernanceManager::Schedule(CScheduler& scheduler, PeerManager& peerman) +{ + assert(IsValid()); + + scheduler.scheduleEvery( + [this, &peerman]() -> void { + LOCK(cs_relay); + for (const auto& inv : m_relay_invs) { + peerman.RelayInv(inv); + } + m_relay_invs.clear(); + }, + // Tests need tighter timings to avoid timeouts, use more relaxed pacing otherwise + Params().IsMockableChain() ? std::chrono::seconds{1} : std::chrono::seconds{5}); +} + bool CGovernanceManager::LoadCache(bool load_cache) { assert(m_db != nullptr); @@ -98,18 +115,18 @@ bool CGovernanceManager::LoadCache(bool load_cache) return is_valid; } -void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const +void CGovernanceManager::RelayObject(const CGovernanceObject& obj) { if (!m_mn_sync.IsSynced()) { LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); return; } - CInv inv(MSG_GOVERNANCE_OBJECT, obj.GetHash()); - peerman.RelayInv(inv); + LOCK(cs_relay); + m_relay_invs.emplace_back(MSG_GOVERNANCE_OBJECT, obj.GetHash()); } -void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const +void CGovernanceManager::RelayVote(const CGovernanceVote& vote) { if (!m_mn_sync.IsSynced()) { LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); @@ -122,8 +139,8 @@ void CGovernanceManager::RelayMessage(PeerManager& peerman, const CGovernanceVot return; } - CInv inv(MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash()); - peerman.RelayInv(inv); + LOCK(cs_relay); + m_relay_invs.emplace_back(MSG_GOVERNANCE_OBJECT_VOTE, vote.GetHash()); } // Accessors for thread-safe access to maps @@ -174,7 +191,7 @@ void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); } -MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv) +MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) { if (!IsValid()) return {}; if (!m_mn_sync.IsBlockchainSynced()) return {}; @@ -267,7 +284,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman return ret; } - AddGovernanceObject(govobj, peerman, &peer); + AddGovernanceObject(govobj, &peer); return ret; } @@ -301,7 +318,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman if (ProcessVote(&peer, vote, exception, connman)) { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- %s new\n", strHash); m_mn_sync.BumpAssetLastTime("MNGOVERNANCEOBJECTVOTE"); - RelayMessage(peerman, vote); + RelayVote(vote); } else { LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECTVOTE -- Rejected vote, error = %s\n", exception.what()); if ((exception.GetNodePenalty() != 0) && m_mn_sync.IsSynced()) { @@ -316,7 +333,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman return {}; } -void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman) +void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) { uint256 nHash = govobj.GetHash(); std::vector vecVotePairs; @@ -333,7 +350,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager if (pairVote.second < nNow) { fRemove = true; } else if (govobj.ProcessVote(m_mn_metaman, *this, tip_mn_list, vote, e)) { - RelayMessage(peerman, vote); + RelayVote(vote); fRemove = true; } if (fRemove) { @@ -342,7 +359,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, PeerManager } } -void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom) +void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom) { uint256 nHash = govobj.GetHash(); std::string strHash = nHash.ToString(); @@ -387,7 +404,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana } LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- %s new, received from peer %s\n", strHash, pfrom ? pfrom->GetLogString() : "nullptr"); - RelayMessage(peerman, govobj); + RelayObject(govobj); // Update the rate buffer MasternodeRateUpdate(govobj); @@ -396,7 +413,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana // WE MIGHT HAVE PENDING/ORPHAN VOTES FOR THIS OBJECT - CheckOrphanVotes(govobj, peerman); + CheckOrphanVotes(govobj); // SEND NOTIFICATION TO SCRIPT/ZMQ GetMainSignals().NotifyGovernanceObject(std::make_shared(govobj.Object()), nHash.ToString()); @@ -878,11 +895,11 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo return false; } -bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) +bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); if (fOK) { - RelayMessage(peerman, vote); + RelayVote(vote); } return fOK; } @@ -940,7 +957,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, return fOk; } -void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) +void CGovernanceManager::CheckPostponedObjects() { if (!m_mn_sync.IsSynced()) return; @@ -957,7 +974,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) bool fMissingConfirmations; if (govobj.IsCollateralValid(m_chainman, strError, fMissingConfirmations)) { if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), m_chainman, strError, false)) { - AddGovernanceObject(govobj, peerman); + AddGovernanceObject(govobj); } else { LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString()); } @@ -990,7 +1007,7 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman) if (fValid) { if (fReady) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- additional relay: hash = %s\n", govobj.GetHash().ToString()); - RelayMessage(peerman, govobj); + RelayObject(govobj); } else { it++; continue; @@ -1289,7 +1306,7 @@ UniValue CGovernanceManager::ToJson() const return jsonObj; } -void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman) +void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) { // Note this gets called from ActivateBestChain without cs_main being held // so it should be safe to lock our mutex here without risking a deadlock @@ -1307,7 +1324,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& RemoveInvalidVotes(); } - CheckPostponedObjects(peerman); + CheckPostponedObjects(); ExecuteBestSuperblock(Assert(m_dmnman)->GetListAtChainTip(), pindex->nHeight); } diff --git a/src/governance/governance.h b/src/governance/governance.h index 8705e2fd44375..d7f26b393a85f 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -31,6 +31,7 @@ template class CFlatDB; class CInv; class CNode; +class CScheduler; class PeerManager; class CDeterministicMNList; @@ -271,18 +272,23 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::optional votedFundingYesTriggerHash; std::map> mapTrigger; + mutable Mutex cs_relay; + std::vector m_relay_invs GUARDED_BY(cs_relay); + public: explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman, const ChainstateManager& chainman, const std::unique_ptr& dmnman, CMasternodeSync& mn_sync); ~CGovernanceManager(); + void Schedule(CScheduler& scheduler, PeerManager& peerman); + bool LoadCache(bool load_cache); bool IsValid() const override { return is_valid; } - void RelayMessage(PeerManager& peerman, const CGovernanceObject& obj) const; - void RelayMessage(PeerManager& peerman, const CGovernanceVote& vote) const; + void RelayObject(const CGovernanceObject& obj); + void RelayVote(const CGovernanceVote& vote); /** * This is called by AlreadyHave in net_processing.cpp as part of the inventory @@ -294,7 +300,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; - [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv); void DoMaintenance(CConnman& connman); @@ -307,13 +313,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; - void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) override; + void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override; void CheckAndRemove(); UniValue ToJson() const; - void UpdatedBlockTip(const CBlockIndex* pindex, PeerManager& peerman); + void UpdatedBlockTip(const CBlockIndex* pindex); int64_t GetLastDiffTime() const { return nTimeLastDiff; } void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } @@ -338,9 +344,9 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); - bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) override; + bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override; - void CheckPostponedObjects(PeerManager& peerman); + void CheckPostponedObjects(); bool AreRateChecksEnabled() const { @@ -407,7 +413,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent /// Called to indicate a requested object or vote has been received bool AcceptMessage(const uint256& nHash); - void CheckOrphanVotes(CGovernanceObject& govobj, PeerManager& peerman); + void CheckOrphanVotes(CGovernanceObject& govobj); void RebuildIndexes(); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 3374752df9495..1da2b9bc5311e 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -18,12 +18,11 @@ #include GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, - PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, - const ChainstateManager& chainman, const CMasternodeSync& mn_sync) : + const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, + const CMasternodeSync& mn_sync) : m_connman{connman}, m_dmnman{dmnman}, m_govman{govman}, - m_peerman{peerman}, m_mn_activeman{mn_activeman}, m_chainman{chainman}, m_mn_sync{mn_sync} @@ -155,7 +154,7 @@ std::optional GovernanceSigner::CreateGovernanceTrigger } // The trigger we just created looks good, submit it - m_govman.AddGovernanceObject(gov_sb, m_peerman); + m_govman.AddGovernanceObject(gov_sb); return std::make_optional(gov_sb); } @@ -250,7 +249,7 @@ bool GovernanceSigner::VoteFundingTrigger(const uint256& nHash, const vote_outco vote.SetSignature(m_mn_activeman.SignBasic(vote.GetSignatureHash())); CGovernanceException exception; - if (!m_govman.ProcessVoteAndRelay(vote, exception, m_connman, m_peerman)) { + if (!m_govman.ProcessVoteAndRelay(vote, exception, m_connman)) { LogPrint(BCLog::GOBJECT, "%s -- Vote FUNDING %d for trigger:%s failed:%s\n", __func__, outcome, nHash.ToString(), exception.what()); return false; } diff --git a/src/governance/signing.h b/src/governance/signing.h index 044d103157d03..e50d12f8651d8 100644 --- a/src/governance/signing.h +++ b/src/governance/signing.h @@ -24,7 +24,6 @@ class CGovernanceVote; class ChainstateManager; class CMasternodeSync; class CNode; -class PeerManager; enum vote_outcome_enum_t : int; class GovernanceSignerParent @@ -35,13 +34,13 @@ class GovernanceSignerParent virtual bool IsValid() const = 0; virtual bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, std::shared_ptr& pSuperblockRet, int nBlockHeight) = 0; virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0; - virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman, PeerManager& peerman) = 0; + virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) = 0; virtual int GetCachedBlockHeight() const = 0; virtual CGovernanceObject* FindGovernanceObject(const uint256& nHash) = 0; virtual CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0; virtual std::vector> GetActiveTriggers() const = 0; virtual std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) = 0; - virtual void AddGovernanceObject(CGovernanceObject& govobj, PeerManager& peerman, const CNode* pfrom = nullptr) = 0; + virtual void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) = 0; }; class GovernanceSigner @@ -50,7 +49,6 @@ class GovernanceSigner CConnman& m_connman; CDeterministicMNManager& m_dmnman; GovernanceSignerParent& m_govman; - PeerManager& m_peerman; const CActiveMasternodeManager& m_mn_activeman; const ChainstateManager& m_chainman; const CMasternodeSync& m_mn_sync; @@ -60,7 +58,7 @@ class GovernanceSigner public: explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, - PeerManager& peerman, const CActiveMasternodeManager& mn_activeman, + const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CMasternodeSync& mn_sync); ~GovernanceSigner(); diff --git a/src/init.cpp b/src/init.cpp index 8b82d9a9e8dd7..231efc9abf09c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2140,7 +2140,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) RegisterValidationInterface(node.peerman.get()); g_ds_notification_interface = std::make_unique( - *node.connman, *node.mn_sync, *node.govman, *node.peerman, chainman, node.dmnman, node.llmq_ctx, node.cj_ctx + *node.connman, *node.mn_sync, *node.govman, chainman, node.dmnman, node.llmq_ctx, node.cj_ctx ); RegisterValidationInterface(g_ds_notification_interface.get()); @@ -2262,6 +2262,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.scheduler->scheduleEvery(std::bind(&CDeterministicMNManager::DoMaintenance, std::ref(*node.dmnman)), std::chrono::seconds{10}); if (node.govman->IsValid()) { + node.govman->Schedule(*node.scheduler, *node.peerman); node.scheduler->scheduleEvery(std::bind(&CGovernanceManager::DoMaintenance, std::ref(*node.govman), std::ref(*node.connman)), std::chrono::minutes{5}); } diff --git a/src/masternode/active/context.cpp b/src/masternode/active/context.cpp index 7b8df853e7ea8..23ff1e24ba3d4 100644 --- a/src/masternode/active/context.cpp +++ b/src/masternode/active/context.cpp @@ -28,7 +28,7 @@ ActiveContext::ActiveContext(ChainstateManager& chainman, CConnman& connman, CDe *llmq_ctx.qman, sporkman, mempool, mn_sync)}, cj_server{std::make_unique(chainman, connman, dmnman, dstxman, mn_metaman, mempool, peerman, mn_activeman, mn_sync, *llmq_ctx.isman)}, - gov_signer{std::make_unique(connman, dmnman, govman, peerman, mn_activeman, chainman, mn_sync)}, + gov_signer{std::make_unique(connman, dmnman, govman, mn_activeman, chainman, mn_sync)}, ehf_sighandler{std::make_unique(chainman, mnhfman, *llmq_ctx.sigman, *llmq_ctx.shareman, *llmq_ctx.qman)} { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bee912118e31a..63d5aac9657b0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5380,7 +5380,7 @@ void PeerManagerImpl::ProcessMessage( } PostProcessMessage(m_sporkman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId()); m_mn_sync.ProcessMessage(pfrom, msg_type, vRecv); - PostProcessMessage(m_govman.ProcessMessage(pfrom, m_connman, *this, msg_type, vRecv), pfrom.GetId()); + PostProcessMessage(m_govman.ProcessMessage(pfrom, m_connman, msg_type, vRecv), pfrom.GetId()); PostProcessMessage(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, m_mn_activeman, m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom.GetId()); PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId()); PostProcessMessage(m_llmq_ctx->qdkgsman->ProcessMessage(pfrom, is_masternode, msg_type, vRecv), pfrom.GetId()); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2acb653160584..312378ffa9ea9 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -148,9 +148,9 @@ class GOVImpl : public GOV } bool processVoteAndRelay(const CGovernanceVote& vote, std::string& error) override { - if (context().govman != nullptr && context().connman != nullptr && context().peerman != nullptr) { + if (context().govman != nullptr && context().connman != nullptr) { CGovernanceException exception; - bool result = context().govman->ProcessVoteAndRelay(vote, exception, *context().connman, *context().peerman); + bool result = context().govman->ProcessVoteAndRelay(vote, exception, *context().connman); if (!result) { error = exception.GetMessage(); } @@ -235,12 +235,11 @@ class GOVImpl : public GOV } } if (!Assert(context().govman)->MasternodeRateCheck(govobj)) { error = "Object creation rate limit exceeded"; return false; } - PeerManager& peerman = EnsurePeerman(context()); if (fMissingConfirmations) { context().govman->AddPostponedObject(govobj); - context().govman->RelayMessage(peerman, govobj); + context().govman->RelayObject(govobj); } else { - context().govman->AddGovernanceObject(govobj, peerman); + context().govman->AddGovernanceObject(govobj); } out_object_hash = govobj.GetHash().ToString(); return true; diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index e6b1a4df0ea29..f9ee9f7c69b54 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -393,12 +393,11 @@ static RPCHelpMan gobject_submit() LogPrintf("gobject(submit) -- Adding locally created governance object - %s\n", strHash); - PeerManager& peerman = EnsurePeerman(node); if (fMissingConfirmations) { node.govman->AddPostponedObject(govobj); - node.govman->RelayMessage(peerman, govobj); + node.govman->RelayObject(govobj); } else { - node.govman->AddGovernanceObject(govobj, peerman); + node.govman->AddGovernanceObject(govobj); } return govobj.GetHash().ToString(); @@ -456,8 +455,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet CGovernanceException exception; CConnman& connman = EnsureConnman(node); - PeerManager& peerman = EnsurePeerman(node); - if (node.govman->ProcessVoteAndRelay(vote, exception, connman, peerman)) { + if (node.govman->ProcessVoteAndRelay(vote, exception, connman)) { nSuccessful++; statusObj.pushKV("result", "success"); } else { @@ -961,10 +959,9 @@ static RPCHelpMan voteraw() } CConnman& connman = EnsureConnman(node); - PeerManager& peerman = EnsurePeerman(node); CGovernanceException exception; - if (node.govman->ProcessVoteAndRelay(vote, exception, connman, peerman)) { + if (node.govman->ProcessVoteAndRelay(vote, exception, connman)) { return "Voted successfully"; } else { throw JSONRPCError(RPC_INTERNAL_ERROR, "Error voting : " + exception.GetMessage()); From e2249912879c1b6dbdf5f2319675c6dbce471b70 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 7 Sep 2025 22:22:24 +0000 Subject: [PATCH 08/14] governance: add lock annotations for `cs_relay` --- src/governance/governance.cpp | 8 ++++++++ src/governance/governance.h | 19 +++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index d203c25ef7e3c..b215d0dec07b2 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -117,6 +117,7 @@ bool CGovernanceManager::LoadCache(bool load_cache) void CGovernanceManager::RelayObject(const CGovernanceObject& obj) { + AssertLockNotHeld(cs_relay); if (!m_mn_sync.IsSynced()) { LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); return; @@ -128,6 +129,7 @@ void CGovernanceManager::RelayObject(const CGovernanceObject& obj) void CGovernanceManager::RelayVote(const CGovernanceVote& vote) { + AssertLockNotHeld(cs_relay); if (!m_mn_sync.IsSynced()) { LogPrint(BCLog::GOBJECT, "%s -- won't relay until fully synced\n", __func__); return; @@ -193,6 +195,7 @@ void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) { + AssertLockNotHeld(cs_relay); if (!IsValid()) return {}; if (!m_mn_sync.IsBlockchainSynced()) return {}; @@ -335,6 +338,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) { + AssertLockNotHeld(cs_relay); uint256 nHash = govobj.GetHash(); std::vector vecVotePairs; cmmapOrphanVotes.GetAll(nHash, vecVotePairs); @@ -361,6 +365,7 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj) void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom) { + AssertLockNotHeld(cs_relay); uint256 nHash = govobj.GetHash(); std::string strHash = nHash.ToString(); @@ -897,6 +902,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) { + AssertLockNotHeld(cs_relay); bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman); if (fOK) { RelayVote(vote); @@ -959,6 +965,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, void CGovernanceManager::CheckPostponedObjects() { + AssertLockNotHeld(cs_relay); if (!m_mn_sync.IsSynced()) return; LOCK2(::cs_main, cs); @@ -1308,6 +1315,7 @@ UniValue CGovernanceManager::ToJson() const void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex) { + AssertLockNotHeld(cs_relay); // Note this gets called from ActivateBestChain without cs_main being held // so it should be safe to lock our mutex here without risking a deadlock // On the other hand it should be safe for us to access pindex without holding a lock diff --git a/src/governance/governance.h b/src/governance/governance.h index d7f26b393a85f..8e5f50d0e0b25 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -287,8 +287,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool IsValid() const override { return is_valid; } - void RelayObject(const CGovernanceObject& obj); - void RelayVote(const CGovernanceVote& vote); + void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); /** * This is called by AlreadyHave in net_processing.cpp as part of the inventory @@ -300,7 +300,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; - [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv); + [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void DoMaintenance(CConnman& connman); @@ -313,13 +314,14 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent std::vector GetCurrentVotes(const uint256& nParentHash, const COutPoint& mnCollateralOutpointFilter) const; void GetAllNewerThan(std::vector& objs, int64_t nMoreThanTime) const; - void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override; + void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) override + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void CheckAndRemove(); UniValue ToJson() const; - void UpdatedBlockTip(const CBlockIndex* pindex); + void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); int64_t GetLastDiffTime() const { return nTimeLastDiff; } void UpdateLastDiffTime(int64_t nTimeIn) { nTimeLastDiff = nTimeIn; } @@ -344,9 +346,10 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed); - bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override; + bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override + EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - void CheckPostponedObjects(); + void CheckPostponedObjects() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); bool AreRateChecksEnabled() const { @@ -413,7 +416,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent /// Called to indicate a requested object or vote has been received bool AcceptMessage(const uint256& nHash); - void CheckOrphanVotes(CGovernanceObject& govobj); + void CheckOrphanVotes(CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); void RebuildIndexes(); From 4d96f5fcc40b39baa7d970341a7c728af59d547e Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Wed, 10 Sep 2025 06:40:35 +0000 Subject: [PATCH 09/14] refactor: move `DoMaintenance()` inside `Schedule()` --- src/governance/governance.cpp | 29 ++++++++++++++--------------- src/governance/governance.h | 4 +--- src/init.cpp | 3 +-- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index b215d0dec07b2..54bf2a5dd2132 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -88,10 +88,23 @@ CGovernanceManager::~CGovernanceManager() m_db->Store(*this); } -void CGovernanceManager::Schedule(CScheduler& scheduler, PeerManager& peerman) +void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman) { assert(IsValid()); + scheduler.scheduleEvery( + [this, &connman]() -> void { + if (!m_mn_sync.IsSynced()) return; + + // CHECK OBJECTS WE'VE ASKED FOR, REMOVE OLD ENTRIES + CleanOrphanObjects(); + RequestOrphanObjects(connman); + + // CHECK AND REMOVE - REPROCESS GOVERNANCE OBJECTS + CheckAndRemove(); + }, + std::chrono::minutes{5}); + scheduler.scheduleEvery( [this, &peerman]() -> void { LOCK(cs_relay); @@ -652,20 +665,6 @@ struct sortProposalsByVotes { } }; -void CGovernanceManager::DoMaintenance(CConnman& connman) -{ - if (!IsValid()) return; - if (!m_mn_sync.IsSynced()) return; - if (ShutdownRequested()) return; - - // CHECK OBJECTS WE'VE ASKED FOR, REMOVE OLD ENTRIES - CleanOrphanObjects(); - RequestOrphanObjects(connman); - - // CHECK AND REMOVE - REPROCESS GOVERNANCE OBJECTS - CheckAndRemove(); -} - bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) { // do not request objects until it's time to sync diff --git a/src/governance/governance.h b/src/governance/governance.h index 8e5f50d0e0b25..0747baddc149b 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -281,7 +281,7 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent const std::unique_ptr& dmnman, CMasternodeSync& mn_sync); ~CGovernanceManager(); - void Schedule(CScheduler& scheduler, PeerManager& peerman); + void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman); bool LoadCache(bool load_cache); @@ -303,8 +303,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); - void DoMaintenance(CConnman& connman); - const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/init.cpp b/src/init.cpp index 231efc9abf09c..4ecda7683d7f7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2262,8 +2262,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.scheduler->scheduleEvery(std::bind(&CDeterministicMNManager::DoMaintenance, std::ref(*node.dmnman)), std::chrono::seconds{10}); if (node.govman->IsValid()) { - node.govman->Schedule(*node.scheduler, *node.peerman); - node.scheduler->scheduleEvery(std::bind(&CGovernanceManager::DoMaintenance, std::ref(*node.govman), std::ref(*node.connman)), std::chrono::minutes{5}); + node.govman->Schedule(*node.scheduler, *node.connman, *node.peerman); } if (node.mn_activeman) { From 17779190fc3fff8e0e2960c6203d3e851522326b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 5 Oct 2025 08:42:48 +0000 Subject: [PATCH 10/14] refactor: use `std::chrono` for governance time constants --- src/governance/governance.cpp | 35 ++++++++++++++++++++--------------- src/governance/governance.h | 4 ---- src/governance/object.h | 6 ------ src/governance/signing.cpp | 9 +++++++-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 54bf2a5dd2132..15efc08684669 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -30,10 +30,13 @@ int nSubmittedFinalBudget; const std::string GovernanceStore::SERIALIZATION_VERSION_STRING = "CGovernanceManager-Version-16"; -const int CGovernanceManager::MAX_TIME_FUTURE_DEVIATION = 60 * 60; -const int CGovernanceManager::RELIABLE_PROPAGATION_TIME = 60; namespace { +constexpr std::chrono::seconds GOVERNANCE_DELETION_DELAY{10min}; +constexpr std::chrono::seconds GOVERNANCE_ORPHAN_EXPIRATION_TIME{10min}; +constexpr std::chrono::seconds MAX_TIME_FUTURE_DEVIATION{1h}; +constexpr std::chrono::seconds RELIABLE_PROPAGATION_TIME{1min}; + class ScopedLockBool { bool& ref; @@ -466,7 +469,7 @@ void CGovernanceManager::CheckAndRemove() CleanAndRemoveTriggers(); auto it = mapObjects.begin(); - int64_t nNow = GetTime().count(); + const auto nNow = GetTime(); while (it != mapObjects.end()) { CGovernanceObject* pObj = &((*it).second); @@ -485,10 +488,10 @@ void CGovernanceManager::CheckAndRemove() // IF DELETE=TRUE, THEN CLEAN THE MESS UP! - int64_t nTimeSinceDeletion = nNow - pObj->GetDeletionTime(); + const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj->GetDeletionTime()}; LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- Checking object for deletion: %s, deletion time = %d, time since deletion = %d, delete flag = %d, expired flag = %d\n", - strHash, pObj->GetDeletionTime(), nTimeSinceDeletion, pObj->IsSetCachedDelete(), pObj->IsSetExpired()); + strHash, pObj->GetDeletionTime(), nTimeSinceDeletion.count(), pObj->IsSetCachedDelete(), pObj->IsSetExpired()); if ((pObj->IsSetCachedDelete() || pObj->IsSetExpired()) && (nTimeSinceDeletion >= GOVERNANCE_DELETION_DELAY)) { @@ -515,7 +518,9 @@ void CGovernanceManager::CheckAndRemove() nTimeExpired = std::numeric_limits::max(); } else { int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing; - nTimeExpired = pObj->GetCreationTime() + 2 * nSuperblockCycleSeconds + GOVERNANCE_DELETION_DELAY; + nTimeExpired = (std::chrono::seconds{pObj->GetCreationTime()} + + std::chrono::seconds{2 * nSuperblockCycleSeconds} + + GOVERNANCE_DELETION_DELAY).count(); } mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired)); @@ -525,7 +530,7 @@ void CGovernanceManager::CheckAndRemove() CProposalValidator validator(pObj->GetDataAsHexString()); if (!validator.Validate()) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", strHash); - pObj->PrepareDeletion(nNow); + pObj->PrepareDeletion(nNow.count()); } } ++it; @@ -535,7 +540,7 @@ void CGovernanceManager::CheckAndRemove() // forget about expired deleted objects auto s_it = mapErasedGovernanceObjects.begin(); while (s_it != mapErasedGovernanceObjects.end()) { - if (s_it->second < nNow) { + if (s_it->second < nNow.count()) { mapErasedGovernanceObjects.erase(s_it++); } else { ++s_it; @@ -545,7 +550,7 @@ void CGovernanceManager::CheckAndRemove() // forget about expired requests auto r_it = m_requested_hash_time.begin(); while (r_it != m_requested_hash_time.end()) { - if (r_it->second < std::chrono::seconds(nNow)) { + if (r_it->second < nNow) { m_requested_hash_time.erase(r_it++); } else { ++r_it; @@ -695,7 +700,7 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv) return false; } - const auto valid_until = GetTime() + std::chrono::seconds(RELIABLE_PROPAGATION_TIME); + const auto valid_until = GetTime() + RELIABLE_PROPAGATION_TIME; const auto& [_itr, inserted] = m_requested_hash_time.emplace(inv.hash, valid_until); if (inserted) { @@ -821,7 +826,7 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj) int64_t nTimestamp = govobj.GetCreationTime(); it->second.triggerBuffer.AddTimestamp(nTimestamp); - if (nTimestamp > GetTime() + MAX_TIME_FUTURE_DEVIATION - RELIABLE_PROPAGATION_TIME) { + if (nTimestamp > GetTime() + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)) { // schedule additional relay for the object setAdditionalRelayObjects.insert(govobj.GetHash()); } @@ -862,7 +867,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo return false; } - if (nTimestamp > nNow + MAX_TIME_FUTURE_DEVIATION) { + if (nTimestamp > nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION)) { LogPrint(BCLog::GOBJECT, "CGovernanceManager::MasternodeRateCheck -- object %s rejected due to too new (future) timestamp, masternode = %s, timestamp = %d, current time = %d\n", strHash, masternodeOutpoint.ToStringShort(), nTimestamp, nNow); return false; @@ -936,7 +941,7 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, std::string msg{strprintf("CGovernanceManager::%s -- Unknown parent object %s, MN outpoint = %s", __func__, nHashGovobj.ToString(), vote.GetMasternodeOutpoint().ToStringShort())}; exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING); - if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, GetTime().count() + GOVERNANCE_ORPHAN_EXPIRATION_TIME))) { + if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, count_seconds(GetTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME)))) { LEAVE_CRITICAL_SECTION(cs); RequestGovernanceObject(pfrom, nHashGovobj, connman); LogPrint(BCLog::GOBJECT, "%s\n", msg); @@ -1007,8 +1012,8 @@ void CGovernanceManager::CheckPostponedObjects() int64_t nTimestamp = govobj.GetCreationTime(); - bool fValid = (nTimestamp <= nNow + MAX_TIME_FUTURE_DEVIATION) && (nTimestamp >= nNow - 2 * nSuperblockCycleSeconds); - bool fReady = (nTimestamp <= nNow + MAX_TIME_FUTURE_DEVIATION - RELIABLE_PROPAGATION_TIME); + bool fValid = (nTimestamp <= nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION)) && (nTimestamp >= nNow - 2 * nSuperblockCycleSeconds); + bool fReady = (nTimestamp <= nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)); if (fValid) { if (fReady) { diff --git a/src/governance/governance.h b/src/governance/governance.h index 0747baddc149b..4b246cc6a5639 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -248,10 +248,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent private: using db_type = CFlatDB; -private: - static const int MAX_TIME_FUTURE_DEVIATION; - static const int RELIABLE_PROPAGATION_TIME; - private: const std::unique_ptr m_db; bool is_valid{false}; diff --git a/src/governance/object.h b/src/governance/object.h index b3866cfa3aff7..1804878e268ad 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -28,16 +28,10 @@ class CNode; extern RecursiveMutex cs_main; static constexpr double GOVERNANCE_FILTER_FP_RATE = 0.001; - - static constexpr CAmount GOVERNANCE_PROPOSAL_FEE_TX = (1 * COIN); - static constexpr int64_t GOVERNANCE_FEE_CONFIRMATIONS = 6; static constexpr int64_t GOVERNANCE_MIN_RELAY_FEE_CONFIRMATIONS = 1; static constexpr int64_t GOVERNANCE_UPDATE_MIN = 60 * 60; -static constexpr int64_t GOVERNANCE_DELETION_DELAY = 10 * 60; -static constexpr int64_t GOVERNANCE_ORPHAN_EXPIRATION_TIME = 10 * 60; -static constexpr int64_t GOVERNANCE_FUDGE_WINDOW = 60 * 60 * 2; // FOR SEEN MAP ARRAYS - GOVERNANCE OBJECTS AND VOTES enum class SeenObjectStatus { diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 1da2b9bc5311e..50bcaa698c38c 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -13,10 +13,15 @@ #include #include #include +#include #include #include +namespace { +constexpr std::chrono::seconds GOVERNANCE_FUDGE_WINDOW{2h}; +} // anonymous namespace + GovernanceSigner::GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, const CMasternodeSync& mn_sync) : @@ -76,8 +81,8 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int // Skip proposals that are too expensive if (budgetAllocated + payment.nAmount > governanceBudget) continue; - int64_t windowStart = jproposal["start_epoch"].getInt() - GOVERNANCE_FUDGE_WINDOW; - int64_t windowEnd = jproposal["end_epoch"].getInt() + GOVERNANCE_FUDGE_WINDOW; + int64_t windowStart = jproposal["start_epoch"].getInt() - count_seconds(GOVERNANCE_FUDGE_WINDOW); + int64_t windowEnd = jproposal["end_epoch"].getInt() + count_seconds(GOVERNANCE_FUDGE_WINDOW); // Skip proposals if the SB isn't within the proposal time window if (SBEpochTime < windowStart) { From d19e1f33156c99db0d6dfad9acb464a5068bf4c1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 4 Oct 2025 19:01:16 +0000 Subject: [PATCH 11/14] fix: add `nullptr` check before using `FindGovernanceObject()` retval --- src/governance/signing.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 50bcaa698c38c..a44971b0a5513 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -212,6 +212,12 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetGovernanceObjHash()); + if (!govobj) { + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for unknown trigger %s\n", __func__, + trigger->GetGovernanceObjHash().ToString()); + continue; + } + const uint256 trigger_hash = govobj->GetHash(); if (trigger->GetBlockHeight() <= m_govman.GetCachedBlockHeight()) { // ignore triggers from the past From c516fd353764ce6f53ec1958e6240d47e9fb7248 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sat, 4 Oct 2025 19:02:53 +0000 Subject: [PATCH 12/14] refactor: apply review suggestions --- src/governance/governance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 15efc08684669..30eefb514452b 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -206,7 +206,7 @@ bool CGovernanceManager::SerializeVoteForHash(const uint256& nHash, CDataStream& void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) { LOCK(cs); - mapPostponedObjects.insert(std::make_pair(govobj.GetHash(), govobj)); + mapPostponedObjects.emplace(govobj.GetHash(), govobj); } MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) From 700e069f3af38210d27ca0f42217a4b2cb5b833d Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 22:44:39 +0000 Subject: [PATCH 13/14] refactor: cleanup headers and forward decls --- src/coinjoin/coinjoin.h | 1 - src/governance/object.cpp | 9 +++++---- src/governance/object.h | 2 -- src/governance/signing.cpp | 1 - src/governance/vote.cpp | 5 +++-- src/governance/vote.h | 2 -- 6 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/coinjoin/coinjoin.h b/src/coinjoin/coinjoin.h index 90d1fe2fe4a90..b0c13beebbdbd 100644 --- a/src/coinjoin/coinjoin.h +++ b/src/coinjoin/coinjoin.h @@ -28,7 +28,6 @@ class CBlockIndex; class ChainstateManager; class CMasternodeSync; class CTxMemPool; -class TxValidationState; namespace llmq { class CChainLocksHandler; diff --git a/src/governance/object.cpp b/src/governance/object.cpp index 65f8a164bd44c..8bea754eea50c 100644 --- a/src/governance/object.cpp +++ b/src/governance/object.cpp @@ -5,15 +5,16 @@ #include #include -#include -#include #include #include #include -#include #include #include -#include + +#include +#include +#include +#include #include #include #include diff --git a/src/governance/object.h b/src/governance/object.h index 1804878e268ad..5e0374520859e 100644 --- a/src/governance/object.h +++ b/src/governance/object.h @@ -22,8 +22,6 @@ class CGovernanceObject; class CGovernanceVote; class ChainstateManager; class CMasternodeMetaMan; -class CMasternodeSync; -class CNode; extern RecursiveMutex cs_main; diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index a44971b0a5513..53322c7ffee20 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include diff --git a/src/governance/vote.cpp b/src/governance/vote.cpp index 828f46ddeaef4..2a013465e38c6 100644 --- a/src/governance/vote.cpp +++ b/src/governance/vote.cpp @@ -5,12 +5,13 @@ #include #include -#include #include #include #include #include -#include + +#include +#include #include #include diff --git a/src/governance/vote.h b/src/governance/vote.h index 65c05703f0981..18664f8b97ec5 100644 --- a/src/governance/vote.h +++ b/src/governance/vote.h @@ -13,8 +13,6 @@ class CBLSPublicKey; class CDeterministicMNList; class CGovernanceVote; -class CMasternodeSync; -class CKey; class CKeyID; // INTENTION OF MASTERNODES REGARDING ITEM From b51cd1d39c0638d43bc34a4af31b1c40cf03bcf7 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 6 Oct 2025 22:33:16 +0000 Subject: [PATCH 14/14] lint: apply most `clang-format` suggestions, update circular allowlist --- src/governance/governance.cpp | 23 +++++++++------- src/governance/governance.h | 12 ++++----- src/governance/signing.cpp | 35 +++++++++++++++---------- src/governance/signing.h | 10 ++++--- test/lint/lint-circular-dependencies.py | 6 ++++- 5 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/governance/governance.cpp b/src/governance/governance.cpp index 30eefb514452b..9d193de7036cb 100644 --- a/src/governance/governance.cpp +++ b/src/governance/governance.cpp @@ -209,7 +209,8 @@ void CGovernanceManager::AddPostponedObject(const CGovernanceObject& govobj) mapPostponedObjects.emplace(govobj.GetHash(), govobj); } -MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) +MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) { AssertLockNotHeld(cs_relay); if (!IsValid()) return {}; @@ -519,8 +520,8 @@ void CGovernanceManager::CheckAndRemove() } else { int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing; nTimeExpired = (std::chrono::seconds{pObj->GetCreationTime()} + - std::chrono::seconds{2 * nSuperblockCycleSeconds} + - GOVERNANCE_DELETION_DELAY).count(); + std::chrono::seconds{2 * nSuperblockCycleSeconds} + GOVERNANCE_DELETION_DELAY) + .count(); } mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired)); @@ -941,7 +942,8 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, std::string msg{strprintf("CGovernanceManager::%s -- Unknown parent object %s, MN outpoint = %s", __func__, nHashGovobj.ToString(), vote.GetMasternodeOutpoint().ToStringShort())}; exception = CGovernanceException(msg, GOVERNANCE_EXCEPTION_WARNING); - if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, count_seconds(GetTime() + GOVERNANCE_ORPHAN_EXPIRATION_TIME)))) { + if (cmmapOrphanVotes.Insert(nHashGovobj, vote_time_pair_t(vote, count_seconds(GetTime() + + GOVERNANCE_ORPHAN_EXPIRATION_TIME)))) { LEAVE_CRITICAL_SECTION(cs); RequestGovernanceObject(pfrom, nHashGovobj, connman); LogPrint(BCLog::GOBJECT, "%s\n", msg); @@ -1012,8 +1014,10 @@ void CGovernanceManager::CheckPostponedObjects() int64_t nTimestamp = govobj.GetCreationTime(); - bool fValid = (nTimestamp <= nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION)) && (nTimestamp >= nNow - 2 * nSuperblockCycleSeconds); - bool fReady = (nTimestamp <= nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)); + bool fValid = (nTimestamp <= nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION)) && + (nTimestamp >= nNow - 2 * nSuperblockCycleSeconds); + bool fReady = (nTimestamp <= + nNow + count_seconds(MAX_TIME_FUTURE_DEVIATION) - count_seconds(RELIABLE_PROPAGATION_TIME)); if (fValid) { if (fReady) { @@ -1631,8 +1635,8 @@ bool CGovernanceManager::GetBestSuperblock(const CDeterministicMNList& tip_mn_li return GetBestSuperblockInternal(tip_mn_list, pSuperblockRet, nBlockHeight); } -bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, - int nBlockHeight) +bool CGovernanceManager::GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, + CSuperblock_sptr& pSuperblockRet, int nBlockHeight) { if (!CSuperblock::IsValidBlockHeight(nBlockHeight)) { return false; @@ -1737,7 +1741,8 @@ void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_m } } -std::vector> CGovernanceManager::GetApprovedProposals(const CDeterministicMNList& tip_mn_list) +std::vector> CGovernanceManager::GetApprovedProposals( + const CDeterministicMNList& tip_mn_list) { AssertLockNotHeld(cs); diff --git a/src/governance/governance.h b/src/governance/governance.h index 4b246cc6a5639..43d3979bdcd4b 100644 --- a/src/governance/governance.h +++ b/src/governance/governance.h @@ -296,8 +296,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent [[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman); [[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const; - [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, CDataStream& vRecv) - EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); + [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type, + CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay); const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs); CGovernanceObject* FindGovernanceObject(const uint256& nHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -386,8 +386,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); - bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) override - EXCLUSIVE_LOCKS_REQUIRED(!cs); + bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) override EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -396,8 +396,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent //! Internal functions that require locks to be held CGovernanceObject* FindGovernanceObjectInternal(const uint256& nHash) EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector> GetActiveTriggersInternal() const EXCLUSIVE_LOCKS_REQUIRED(cs); - bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, int nBlockHeight) - EXCLUSIVE_LOCKS_REQUIRED(cs); + bool GetBestSuperblockInternal(const CDeterministicMNList& tip_mn_list, CSuperblock_sptr& pSuperblockRet, + int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight); diff --git a/src/governance/signing.cpp b/src/governance/signing.cpp index 53322c7ffee20..13dd74d886372 100644 --- a/src/governance/signing.cpp +++ b/src/governance/signing.cpp @@ -39,7 +39,9 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int { if (!m_govman.IsValid()) return std::nullopt; if (!m_mn_sync.IsSynced()) return std::nullopt; - if (nHeight % Params().GetConsensus().nSuperblockCycle < Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) return std::nullopt; + if (nHeight % Params().GetConsensus().nSuperblockCycle < + Params().GetConsensus().nSuperblockCycle - Params().GetConsensus().nSuperblockMaturityWindow) + return std::nullopt; if (HasAlreadyVotedFundingTrigger()) return std::nullopt; const auto approvedProposals = m_govman.GetApprovedProposals(m_dmnman.GetListAtChainTip()); @@ -53,7 +55,8 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int int nNextSuperblock; CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock); - auto SBEpochTime = static_cast(GetTime().count() + (nNextSuperblock - nHeight) * 2.62 * 60); + auto SBEpochTime = static_cast(GetTime().count() + + (nNextSuperblock - nHeight) * 2.62 * 60); auto governanceBudget = CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock); CAmount budgetAllocated{}; @@ -67,9 +70,8 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int CAmount nAmount{}; try { nAmount = ParsePaymentAmount(jproposal["payment_amount"].getValStr()); - } - catch (const std::runtime_error& e) { - LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d Skipping payment exception:%s\n", __func__,nHeight, e.what()); + } catch (const std::runtime_error& e) { + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d Skipping payment exception:%s\n", __func__, nHeight, e.what()); continue; } @@ -85,11 +87,12 @@ std::optional GovernanceSigner::CreateSuperblockCandidate(int // Skip proposals if the SB isn't within the proposal time window if (SBEpochTime < windowStart) { - LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowStart:%d\n", __func__,nHeight, SBEpochTime, windowStart); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowStart:%d\n", __func__, nHeight, SBEpochTime, + windowStart); continue; } if (SBEpochTime > windowEnd) { - LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowEnd:%d\n", __func__,nHeight, SBEpochTime, windowEnd); + LogPrint(BCLog::GOBJECT, "%s -- nHeight:%d SB:%d windowEnd:%d\n", __func__, nHeight, SBEpochTime, windowEnd); continue; } @@ -120,7 +123,7 @@ std::optional GovernanceSigner::CreateGovernanceTrigger // no sb_opt, no trigger if (!sb_opt.has_value()) return std::nullopt; - //TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct + // TODO: Check if nHashParentIn, nRevision and nCollateralHashIn are correct LOCK(::cs_main); // Check if identical trigger (equal DataHash()) is already created (signed by other masternode) @@ -153,7 +156,8 @@ std::optional GovernanceSigner::CreateGovernanceTrigger } if (!m_govman.MasternodeRateCheck(gov_sb)) { - LogPrint(BCLog::GOBJECT, "%s -- Trigger rejected because of rate check failure hash(%s)\n", __func__, gov_sb.GetHash().ToString()); + LogPrint(BCLog::GOBJECT, "%s -- Trigger rejected because of rate check failure hash(%s)\n", __func__, + gov_sb.GetHash().ToString()); return std::nullopt; } @@ -220,12 +224,14 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optionalGetHash(); if (trigger->GetBlockHeight() <= m_govman.GetCachedBlockHeight()) { // ignore triggers from the past - LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for outdated trigger:%s\n", __func__, trigger_hash.ToString()); + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for outdated trigger:%s\n", __func__, + trigger_hash.ToString()); continue; } if (trigger_hash == votedFundingYesTriggerHash) { // Skip actual trigger - LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", __func__, trigger_hash.ToString()); + LogPrint(BCLog::GOBJECT, "%s -- Not voting NO-FUNDING for trigger:%s, we voted yes for it already\n", + __func__, trigger_hash.ToString()); continue; } if (vote_rec_t voteRecord; govobj->GetCurrentMNVotes(m_mn_activeman.GetOutPoint(), voteRecord)) { @@ -233,8 +239,8 @@ void GovernanceSigner::VoteGovernanceTriggers(const std::optional& pSuperblockRet, int nBlockHeight) = 0; + virtual bool GetBestSuperblock(const CDeterministicMNList& tip_mn_list, + std::shared_ptr& pSuperblockRet, int nBlockHeight) = 0; virtual bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) = 0; virtual bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) = 0; virtual int GetCachedBlockHeight() const = 0; virtual CGovernanceObject* FindGovernanceObject(const uint256& nHash) = 0; virtual CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) = 0; virtual std::vector> GetActiveTriggers() const = 0; - virtual std::vector> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) = 0; + virtual std::vector> GetApprovedProposals( + const CDeterministicMNList& tip_mn_list) = 0; virtual void AddGovernanceObject(CGovernanceObject& govobj, const CNode* pfrom = nullptr) = 0; }; @@ -58,8 +60,8 @@ class GovernanceSigner public: explicit GovernanceSigner(CConnman& connman, CDeterministicMNManager& dmnman, GovernanceSignerParent& govman, - const CActiveMasternodeManager& mn_activeman, - const ChainstateManager& chainman, const CMasternodeSync& mn_sync); + const CActiveMasternodeManager& mn_activeman, const ChainstateManager& chainman, + const CMasternodeSync& mn_sync); ~GovernanceSigner(); void UpdatedBlockTip(const CBlockIndex* pindex); diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 4934b002e1a93..266fd02574dbf 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -40,6 +40,8 @@ "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> consensus/tx_verify", "consensus/tx_verify -> evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> consensus/tx_verify", "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> evo/smldiff -> core_io", + "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> masternode/active/context -> governance/signing -> governance/classes -> core_io", + "core_io -> evo/mnhftx -> llmq/signing -> net_processing -> masternode/active/context -> governance/signing -> governance/object -> core_io", "evo/assetlocktx -> llmq/commitment -> validation -> txmempool -> evo/assetlocktx", "evo/chainhelper -> evo/specialtxman -> validation -> evo/chainhelper", "evo/deterministicmns -> index/txindex -> validation -> evo/deterministicmns", @@ -47,8 +49,10 @@ "evo/netinfo -> evo/providertx -> evo/netinfo", "evo/smldiff -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/smldiff", "evo/specialtxman -> validation -> evo/specialtxman", - "governance/governance -> governance/object -> governance/governance", + "governance/classes -> governance/object -> governance/governance -> governance/classes", + "governance/governance -> governance/signing -> governance/object -> governance/governance", "governance/governance -> masternode/sync -> governance/governance", + "governance/governance -> net_processing -> masternode/active/context -> governance/governance", "governance/governance -> net_processing -> governance/governance", "instantsend/instantsend -> net_processing -> instantsend/instantsend", "instantsend/instantsend -> net_processing -> llmq/context -> instantsend/instantsend",