refactor: governance constification and deglobalization (#5572)

## Issue being fixed or feature implemented
Some relatively simple refactoring; inspired by reviewing #5569; adds
some constification and some deglobalization

## What was done?
Partial deglobalization and constification

## How Has This Been Tested?
Building

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
This commit is contained in:
PastaPastaPasta 2023-09-10 14:05:49 -05:00 committed by GitHub
parent 13c12406ae
commit 38e70430b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 61 additions and 42 deletions

View File

@ -209,7 +209,7 @@ std::vector<CSuperblock_sptr> CGovernanceTriggerManager::GetActiveTriggers()
// LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS
for (const auto& pair : mapTrigger) {
const CGovernanceObject* pObj = governance->FindGovernanceObject(pair.first);
const CGovernanceObject* pObj = governance->FindConstGovernanceObject(pair.first);
if (pObj) {
vecResults.push_back(pair.second);
}

View File

@ -421,6 +421,16 @@ void CGovernanceManager::UpdateCachesAndClean()
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- %s\n", ToString());
}
const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const
{
AssertLockHeld(cs);
auto it = mapObjects.find(nHash);
if (it != mapObjects.end()) return &(it->second);
return nullptr;
}
CGovernanceObject* CGovernanceManager::FindGovernanceObject(const uint256& nHash)
{
AssertLockHeld(cs);
@ -1108,7 +1118,7 @@ void CGovernanceManager::CheckPostponedObjects(CConnman& connman)
}
}
void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter)
void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter) const
{
if (!pfrom) {
return;
@ -1123,7 +1133,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
size_t nVoteCount = 0;
if (fUseFilter) {
LOCK(cs);
const CGovernanceObject* pObj = FindGovernanceObject(nHash);
const CGovernanceObject* pObj = FindConstGovernanceObject(nHash);
if (pObj) {
filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRandInt(999999), BLOOM_UPDATE_ALL);
@ -1139,13 +1149,13 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::MNGOVERNANCESYNC, nHash, filter));
}
int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& connman)
int CGovernanceManager::RequestGovernanceObjectVotes(CNode& peer, CConnman& connman) const
{
std::array<CNode*, 1> nodeCopy{&peer};
return RequestGovernanceObjectVotes(nodeCopy, connman);
}
int CGovernanceManager::RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CConnman& connman)
int CGovernanceManager::RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CConnman& connman) const
{
static std::map<uint256, std::map<CService, int64_t> > mapAskedRecently;

View File

@ -248,6 +248,7 @@ public:
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);
void DeleteGovernanceObject(const uint256& nHash);
@ -359,11 +360,11 @@ public:
void InitOnLoad();
int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman);
int RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CConnman& connman);
int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman) const;
int RequestGovernanceObjectVotes(Span<CNode*> vNodesCopy, CConnman& connman) const;
private:
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false);
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const;
void AddInvalidVote(const CGovernanceVote& vote)
{

View File

@ -1694,15 +1694,16 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
node.chainman = &g_chainman;
ChainstateManager& chainman = *Assert(node.chainman);
::governance = std::make_unique<CGovernanceManager>();
assert(!node.peerman);
node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(),
*node.scheduler, chainman, *node.mempool, node.llmq_ctx, ignores_incoming_txs);
*node.scheduler, chainman, *node.mempool, node.llmq_ctx, *::governance, ignores_incoming_txs);
RegisterValidationInterface(node.peerman.get());
::governance = std::make_unique<CGovernanceManager>();
assert(!::sporkManager);
::sporkManager = std::make_unique<CSporkManager>();
::masternodeSync = std::make_unique<CMasternodeSync>(*node.connman);
::masternodeSync = std::make_unique<CMasternodeSync>(*node.connman, *::governance);
std::vector<std::string> vSporkAddresses;
if (args.IsArgSet("-sporkaddr")) {

View File

@ -16,10 +16,11 @@
class CMasternodeSync;
std::unique_ptr<CMasternodeSync> masternodeSync;
CMasternodeSync::CMasternodeSync(CConnman& _connman) :
CMasternodeSync::CMasternodeSync(CConnman& _connman, const CGovernanceManager& govman) :
nTimeAssetSyncStarted(GetTime()),
nTimeLastBumped(GetTime()),
connman(_connman)
connman(_connman),
m_govman(govman)
{
}
@ -138,7 +139,7 @@ void CMasternodeSync::ProcessTick()
// gradually request the rest of the votes after sync finished
if(IsSynced()) {
governance->RequestGovernanceObjectVotes(vNodesCopy, connman);
m_govman.RequestGovernanceObjectVotes(vNodesCopy, connman);
connman.ReleaseNodeVector(vNodesCopy);
return;
}
@ -262,7 +263,7 @@ void CMasternodeSync::ProcessTick()
if(!netfulfilledman.HasFulfilledRequest(pnode->addr, "governance-sync")) {
continue; // to early for this node
}
int nObjsLeftToAsk = governance->RequestGovernanceObjectVotes(*pnode, connman);
int nObjsLeftToAsk = m_govman.RequestGovernanceObjectVotes(*pnode, connman);
// check for data
if(nObjsLeftToAsk == 0) {
static int64_t nTimeNoObjectsLeft = 0;
@ -275,7 +276,7 @@ void CMasternodeSync::ProcessTick()
// make sure the condition below is checked only once per tick
if(nLastTick == nTick) continue;
if(GetTime() - nTimeNoObjectsLeft > MASTERNODE_SYNC_TIMEOUT_SECONDS &&
governance->GetVoteCount() - nLastVotes < std::max(int(0.0001 * nLastVotes), MASTERNODE_SYNC_TICK_SECONDS)
m_govman.GetVoteCount() - nLastVotes < std::max(int(0.0001 * nLastVotes), MASTERNODE_SYNC_TICK_SECONDS)
) {
// We already asked for all objects, waited for MASTERNODE_SYNC_TIMEOUT_SECONDS
// after that and less then 0.01% or MASTERNODE_SYNC_TICK_SECONDS
@ -289,7 +290,7 @@ void CMasternodeSync::ProcessTick()
return;
}
nLastTick = nTick;
nLastVotes = governance->GetVoteCount();
nLastVotes = m_govman.GetVoteCount();
}
}
@ -297,7 +298,7 @@ void CMasternodeSync::ProcessTick()
connman.ReleaseNodeVector(vNodesCopy);
}
void CMasternodeSync::SendGovernanceSyncRequest(CNode* pnode)
void CMasternodeSync::SendGovernanceSyncRequest(CNode* pnode) const
{
CNetMsgMaker msgMaker(pnode->GetSendVersion());

View File

@ -13,6 +13,7 @@ class CBlockIndex;
class CConnman;
class CNode;
class CDataStream;
class CGovernanceManager;
static constexpr int MASTERNODE_SYNC_BLOCKCHAIN = 1;
static constexpr int MASTERNODE_SYNC_GOVERNANCE = 4;
@ -49,11 +50,12 @@ private:
std::atomic<int64_t> nTimeLastUpdateBlockTip{0};
CConnman& connman;
const CGovernanceManager& m_govman;
public:
explicit CMasternodeSync(CConnman& _connman);
explicit CMasternodeSync(CConnman& _connman, const CGovernanceManager& govman);
void SendGovernanceSyncRequest(CNode* pnode);
void SendGovernanceSyncRequest(CNode* pnode) const;
bool IsBlockchainSynced() const { return nCurrentAsset > MASTERNODE_SYNC_BLOCKCHAIN; }
bool IsSynced() const { return nCurrentAsset == MASTERNODE_SYNC_FINISHED; }

View File

@ -220,7 +220,7 @@ class PeerManagerImpl final : public PeerManager
public:
PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman,
BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman,
CTxMemPool& pool, const std::unique_ptr<LLMQContext>& llmq_ctx, bool ignore_incoming_txs);
CTxMemPool& pool, const std::unique_ptr<LLMQContext>& llmq_ctx, CGovernanceManager& govman, bool ignore_incoming_txs);
/** Overridden from CValidationInterface. */
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override;
@ -310,6 +310,7 @@ private:
ChainstateManager& m_chainman;
CTxMemPool& m_mempool;
const std::unique_ptr<LLMQContext>& m_llmq_ctx;
CGovernanceManager& m_govman;
/** The height of the best chain */
std::atomic<int> m_best_height{-1};
@ -1542,14 +1543,14 @@ bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex, const Conse
std::unique_ptr<PeerManager> PeerManager::make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman,
CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool,
const std::unique_ptr<LLMQContext>& llmq_ctx, bool ignore_incoming_txs)
const std::unique_ptr<LLMQContext>& llmq_ctx, CGovernanceManager& govman, bool ignore_incoming_txs)
{
return std::make_unique<PeerManagerImpl>(chainparams, connman, addrman, banman, scheduler, chainman, pool, llmq_ctx, ignore_incoming_txs);
return std::make_unique<PeerManagerImpl>(chainparams, connman, addrman, banman, scheduler, chainman, pool, llmq_ctx, govman, ignore_incoming_txs);
}
PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman, BanMan* banman,
CScheduler &scheduler, ChainstateManager& chainman, CTxMemPool& pool,
const std::unique_ptr<LLMQContext>& llmq_ctx, bool ignore_incoming_txs)
const std::unique_ptr<LLMQContext>& llmq_ctx, CGovernanceManager& govman, bool ignore_incoming_txs)
: m_chainparams(chainparams),
m_connman(connman),
m_addrman(addrman),
@ -1557,6 +1558,7 @@ PeerManagerImpl::PeerManagerImpl(const CChainParams& chainparams, CConnman& conn
m_chainman(chainman),
m_mempool(pool),
m_llmq_ctx(llmq_ctx),
m_govman(govman),
m_stale_tip_check_time(0),
m_ignore_incoming_txs(ignore_incoming_txs)
{
@ -1858,7 +1860,7 @@ bool PeerManagerImpl::AlreadyHave(const CInv& inv)
case MSG_GOVERNANCE_OBJECT:
case MSG_GOVERNANCE_OBJECT_VOTE:
return ! governance->ConfirmInventoryRequest(inv);
return !m_govman.ConfirmInventoryRequest(inv);
case MSG_QUORUM_FINAL_COMMITMENT:
return m_llmq_ctx->quorum_block_processor->HasMineableCommitment(inv.hash);
@ -2166,9 +2168,9 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
if (!push && inv.type == MSG_GOVERNANCE_OBJECT) {
CDataStream ss(SER_NETWORK, pfrom.GetSendVersion());
bool topush = false;
if (governance->HaveObjectForHash(inv.hash)) {
if (m_govman.HaveObjectForHash(inv.hash)) {
ss.reserve(1000);
if (governance->SerializeObjectForHash(inv.hash, ss)) {
if (m_govman.SerializeObjectForHash(inv.hash, ss)) {
topush = true;
}
}
@ -2181,9 +2183,9 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic
if (!push && inv.type == MSG_GOVERNANCE_OBJECT_VOTE) {
CDataStream ss(SER_NETWORK, pfrom.GetSendVersion());
bool topush = false;
if (governance->HaveVoteForHash(inv.hash)) {
if (m_govman.HaveVoteForHash(inv.hash)) {
ss.reserve(1000);
if (governance->SerializeVoteForHash(inv.hash, ss)) {
if (m_govman.SerializeVoteForHash(inv.hash, ss)) {
topush = true;
}
}
@ -4319,7 +4321,7 @@ void PeerManagerImpl::ProcessMessage(
coinJoinServer->ProcessMessage(pfrom, *this, msg_type, vRecv);
sporkManager->ProcessMessage(pfrom, *this, m_connman, msg_type, vRecv);
::masternodeSync->ProcessMessage(pfrom, msg_type, vRecv);
governance->ProcessMessage(pfrom, *this, m_connman, msg_type, vRecv);
m_govman.ProcessMessage(pfrom, *this, m_connman, msg_type, vRecv);
CMNAuth::ProcessMessage(pfrom, *this, m_connman, msg_type, vRecv);
m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv);
m_llmq_ctx->qdkgsman->ProcessMessage(pfrom, *m_llmq_ctx->qman, msg_type, vRecv);

View File

@ -16,6 +16,7 @@ class CAddrMan;
class CTxMemPool;
class ChainstateManager;
struct LLMQContext;
class CGovernanceManager;
extern RecursiveMutex cs_main;
extern RecursiveMutex g_cs_orphans;
@ -39,7 +40,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
public:
static std::unique_ptr<PeerManager> make(const CChainParams& chainparams, CConnman& connman, CAddrMan& addrman,
BanMan* banman, CScheduler &scheduler, ChainstateManager& chainman,
CTxMemPool& pool, const std::unique_ptr<LLMQContext>& llmq_ctx, bool ignore_incoming_txs);
CTxMemPool& pool, const std::unique_ptr<LLMQContext>& llmq_ctx, CGovernanceManager& govman, bool ignore_incoming_txs);
virtual ~PeerManager() { }
/** Get statistics from node state */

View File

@ -398,7 +398,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma
const NodeContext& node = EnsureAnyNodeContext(request.context);
{
LOCK(governance->cs);
CGovernanceObject *pGovObj = governance->FindGovernanceObject(hash);
const CGovernanceObject *pGovObj = governance->FindConstGovernanceObject(hash);
if (!pGovObj) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found");
}
@ -731,7 +731,7 @@ static UniValue gobject_get(const JSONRPCRequest& request)
// FIND THE GOVERNANCE OBJECT THE USER IS LOOKING FOR
LOCK2(cs_main, governance->cs);
CGovernanceObject* pGovObj = governance->FindGovernanceObject(hash);
const CGovernanceObject* pGovObj = governance->FindConstGovernanceObject(hash);
if (pGovObj == nullptr) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown governance object");
@ -829,7 +829,7 @@ static UniValue gobject_getcurrentvotes(const JSONRPCRequest& request)
LOCK(governance->cs);
CGovernanceObject* pGovObj = governance->FindGovernanceObject(hash);
const CGovernanceObject* pGovObj = governance->FindConstGovernanceObject(hash);
if (pGovObj == nullptr) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Unknown governance-hash");
@ -966,7 +966,7 @@ static UniValue voteraw(const JSONRPCRequest& request)
GovernanceObject govObjType = WITH_LOCK(governance->cs, return [&](){
AssertLockHeld(governance->cs);
CGovernanceObject *pGovObj = governance->FindGovernanceObject(hashGovObj);
const CGovernanceObject *pGovObj = governance->FindConstGovernanceObject(hashGovObj);
if (!pGovObj) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found");
}

View File

@ -18,6 +18,7 @@
#include <util/system.h>
#include <util/time.h>
#include <validation.h>
#include <governance/governance.h>
#include <test/util/setup_common.h>
@ -80,7 +81,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
const CChainParams& chainparams = Params();
auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false);
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance, false);
// Mock an outbound peer
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
@ -152,7 +153,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management)
const CChainParams& chainparams = Params();
auto connman = std::make_unique<CConnmanTest>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, *m_node.scheduler,
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false);
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance, false);
const Consensus::Params& consensusParams = Params().GetConsensus();
constexpr int max_outbound_full_relay = MAX_OUTBOUND_FULL_RELAY_CONNECTIONS;
@ -226,7 +227,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false);
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance, false);
banman->ClearBanned();
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
@ -273,7 +274,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false);
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance, false);
banman->ClearBanned();
gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number
@ -318,7 +319,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
auto banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
auto connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman);
auto peerLogic = PeerManager::make(chainparams, *connman, *m_node.addrman, banman.get(), *m_node.scheduler,
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, false);
*m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance, false);
banman->ClearBanned();
int64_t nStartTime = GetTime();

View File

@ -204,7 +204,7 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
::sporkManager = std::make_unique<CSporkManager>();
::governance = std::make_unique<CGovernanceManager>();
::masternodeSync = std::make_unique<CMasternodeSync>(*m_node.connman);
::masternodeSync = std::make_unique<CMasternodeSync>(*m_node.connman, *::governance);
#ifdef ENABLE_WALLET
::coinJoinClientQueueManager = std::make_unique<CCoinJoinClientQueueManager>(*m_node.connman, *::masternodeSync);
#endif // ENABLE_WALLET
@ -261,7 +261,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
m_node.banman = std::make_unique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
m_node.peerman = PeerManager::make(chainparams, *m_node.connman, *m_node.addrman, m_node.banman.get(),
*m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx,
*m_node.scheduler, *m_node.chainman, *m_node.mempool, m_node.llmq_ctx, *governance,
false);
{
CConnman::Options options;