Skip to content

refactor: split off governance masternode-only logic to GovernanceSigner, drop Relay()s and use periodic relay instead, minor cleanup #6838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 14 commits into from
Oct 20, 2025
Merged

refactor: split off governance masternode-only logic to GovernanceSigner, drop Relay()s and use periodic relay instead, minor cleanup #6838

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fefbe27
refactor: replace `Sign` functions with direct activeman call
kwvg Oct 6, 2025
2b74e15
refactor: move masternode mode logic to `governance/signing.cpp`
kwvg Oct 3, 2025
bd6af83
refactor: separate masternode mode logic into dedicated signer class
kwvg Oct 6, 2025
757ded3
refactor: remove need for access to private members
kwvg Oct 6, 2025
63448ff
refactor: abstract away parent implementation from signer
kwvg Sep 12, 2025
3811924
governance: drop `Relay()` from `CGovernance{Object,Vote}`
kwvg Sep 12, 2025
df589c7
governance: introduce task for relaying governance objects
kwvg Oct 6, 2025
e224991
governance: add lock annotations for `cs_relay`
kwvg Sep 7, 2025
4d96f5f
refactor: move `DoMaintenance()` inside `Schedule()`
kwvg Sep 10, 2025
1777919
refactor: use `std::chrono` for governance time constants
kwvg Oct 5, 2025
d19e1f3
fix: add `nullptr` check before using `FindGovernanceObject()` retval
kwvg Oct 4, 2025
c516fd3
refactor: apply review suggestions
kwvg Oct 4, 2025
700e069
refactor: cleanup headers and forward decls
kwvg Oct 6, 2025
b51cd1d
lint: apply most `clang-format` suggestions, update circular allowlist
kwvg Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
4 changes: 1 addition & 3 deletions src/dsnotificationinterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ CDSNotificationInterface::CDSNotificationInterface(CConnman& connman,
CGovernanceManager& govman,
PeerManager& peerman,
const ChainstateManager& chainman,
const CActiveMasternodeManager* const mn_activeman,
const std::unique_ptr<CDeterministicMNManager>& dmnman,
const std::unique_ptr<LLMQContext>& llmq_ctx,
const std::unique_ptr<CJContext>& cj_ctx)
Expand All @@ -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) {}
Expand Down Expand Up @@ -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);
}
}

Expand Down
3 changes: 0 additions & 3 deletions src/dsnotificationinterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <validationinterface.h>

class CActiveMasternodeManager;
class CConnman;
class CDeterministicMNManager;
class CGovernanceManager;
Expand All @@ -25,7 +24,6 @@ class CDSNotificationInterface : public CValidationInterface
CGovernanceManager& govman,
PeerManager& peerman,
const ChainstateManager& chainman,
const CActiveMasternodeManager* const mn_activeman,
const std::unique_ptr<CDeterministicMNManager>& dmnman,
const std::unique_ptr<LLMQContext>& llmq_ctx,
const std::unique_ptr<CJContext>& cj_ctx);
Expand Down Expand Up @@ -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<CDeterministicMNManager>& m_dmnman;
const std::unique_ptr<LLMQContext>& m_llmq_ctx;
const std::unique_ptr<CJContext>& m_cj_ctx;
Expand Down
23 changes: 13 additions & 10 deletions src/governance/governance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <governance/common.h>
#include <governance/validators.h>
#include <masternode/meta.h>
#include <masternode/node.h>
#include <masternode/sync.h>
#include <net_processing.h>
#include <netfulfilledman.h>
Expand Down Expand Up @@ -75,6 +74,7 @@ CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfi
m_mn_sync{mn_sync},
nTimeLastDiff(0),
nCachedBlockHeight(0),
mapPostponedObjects(),
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai is this line needed? shouldn't a map like this be automatically initialized properly?

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the declaration of mapPostponedObjects in the governance header/implementation files
rg -n "mapPostponedObjects" --type=cpp --type=h -C2 src/governance/

Length of output: 3326

Copy link
Copy Markdown
Collaborator Author

@kwvg kwvg Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was made while trying to trim the header count in governance/governance.h by moving the constructor's definition into the source file so we don't have to include heavier headers and can make do with a forward declaration.

It could probably be reversed but no strong feelings either way.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

fRateChecksEnabled(true),
votedFundingYesTriggerHash(std::nullopt),
mapTrigger{}
Expand Down Expand Up @@ -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 {};
Expand Down Expand Up @@ -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);
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta Oct 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: it appears we never use this as a map; only as a set. We should consider refactoring this.

}

int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const
{
const std::vector<CNode*> vNodeCopy{&peer};
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down Expand Up @@ -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);
Expand Down
51 changes: 23 additions & 28 deletions src/governance/governance.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,48 @@
#ifndef BITCOIN_GOVERNANCE_GOVERNANCE_H
#define BITCOIN_GOVERNANCE_GOVERNANCE_H

#include <governance/classes.h>
#include <governance/object.h>

#include <cachemap.h>
#include <cachemultimap.h>
#include <protocol.h>
#include <util/check.h>
#include <sync.h>

#include <optional>
#include <limits>
#include <map>
#include <memory>
#include <set>
#include <string>
#include <string_view>
#include <vector>

Comment thread
coderabbitai[bot] marked this conversation as resolved.
class CBloomFilter;
class CBlockIndex;
class CChain;
class CConnman;
class ChainstateManager;
template<typename T>
class CFlatDB;
class CInv;
class CNode;
class PeerManager;

class CDeterministicMNList;
class CDeterministicMNManager;
class CGovernanceException;
class CGovernanceManager;
class CGovernanceObject;
class CGovernanceVote;
class CMasternodeMetaMan;
class CMasternodeSync;
class CNetFulfilledRequestManager;
class CSporkManager;
class CSuperblock;
class GovernanceSigner;

class UniValue;

using CDeterministicMNListPtr = std::shared_ptr<CDeterministicMNList>;
using CSuperblock_sptr = std::shared_ptr<CSuperblock>;
using vote_time_pair_t = std::pair<CGovernanceVote, int64_t>;

static constexpr int RATE_BUFFER_SIZE = 5;
static constexpr bool DEFAULT_GOVERNANCE_ENABLE{true};
Expand Down Expand Up @@ -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);
Expand All @@ -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; }

Expand All @@ -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);

Expand Down Expand Up @@ -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<const CSuperblock> CreateSuperblockCandidate(int nHeight) const;
std::optional<const CGovernanceObject> CreateGovernanceTrigger(const std::optional<const CSuperblock>& sb_opt, PeerManager& peerman,
const CActiveMasternodeManager& mn_activeman);
void VoteGovernanceTriggers(const std::optional<const CGovernanceObject>& 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);

Expand All @@ -408,6 +402,7 @@ class CGovernanceManager : public GovernanceStore

void RemoveInvalidVotes();

friend class GovernanceSigner;
};

bool AreSuperblocksEnabled(const CSporkManager& sporkman);
Expand Down
Loading