Skip to content

Commit 21cc12c

Browse files
kwvgkwvg
committed
refactor: drop usage of chainstate globals in governance logic
1 parent a475f5f commit 21cc12c

File tree

14 files changed

+75
-62
lines changed

14 files changed

+75
-62
lines changed

‎src/evo/chainhelper.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
#include <masternode/payments.h>
1010

1111
CChainstateHelper::CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman, CMNHFManager& mnhfman, CGovernanceManager& govman,
12-
llmq::CQuorumBlockProcessor& qblockman, const Consensus::Params& consensus_params,
12+
llmq::CQuorumBlockProcessor& qblockman, const ChainstateManager& chainman, const Consensus::Params& consensus_params,
1313
const CMasternodeSync& mn_sync, const CSporkManager& sporkman, const llmq::CChainLocksHandler& clhandler,
1414
const llmq::CQuorumManager& qman)
15-
: mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, govman, consensus_params, mn_sync, sporkman)},
15+
: mn_payments{std::make_unique<CMNPaymentsProcessor>(dmnman, govman, chainman, consensus_params, mn_sync, sporkman)},
1616
special_tx{std::make_unique<CSpecialTxProcessor>(cpoolman, dmnman, mnhfman, qblockman, consensus_params, clhandler, qman)}
1717
{}
1818

‎src/evo/chainhelper.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
class CCreditPoolManager;
1111
class CDeterministicMNManager;
12+
class ChainstateManager;
1213
class CMNHFManager;
1314
class CMNPaymentsProcessor;
1415
class CMasternodeSync;
@@ -27,7 +28,7 @@ class CChainstateHelper
2728
{
2829
public:
2930
explicit CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman, CMNHFManager& mnhfman, CGovernanceManager& govman,
30-
llmq::CQuorumBlockProcessor& qblockman, const Consensus::Params& consensus_params,
31+
llmq::CQuorumBlockProcessor& qblockman, const ChainstateManager& chainman, const Consensus::Params& consensus_params,
3132
const CMasternodeSync& mn_sync, const CSporkManager& sporkman, const llmq::CChainLocksHandler& clhandler,
3233
const llmq::CQuorumManager& qman);
3334
~CChainstateHelper();

‎src/governance/classes.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -362,14 +362,14 @@ bool CSuperblockManager::GetSuperblockPayments(CGovernanceManager& govman, const
362362
return true;
363363
}
364364

365-
bool CSuperblockManager::IsValid(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
365+
bool CSuperblockManager::IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
366366
{
367367
// GET BEST SUPERBLOCK, SHOULD MATCH
368368
LOCK(govman.cs);
369369

370370
CSuperblock_sptr pSuperblock;
371371
if (CSuperblockManager::GetBestSuperblock(govman, tip_mn_list, pSuperblock, nBlockHeight)) {
372-
return pSuperblock->IsValid(govman, txNew, nBlockHeight, blockReward);
372+
return pSuperblock->IsValid(govman, active_chain, txNew, nBlockHeight, blockReward);
373373
}
374374

375375
return false;
@@ -482,15 +482,15 @@ void CSuperblock::GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuper
482482
}
483483
}
484484

485-
CAmount CSuperblock::GetPaymentsLimit(int nBlockHeight)
485+
CAmount CSuperblock::GetPaymentsLimit(const CChain& active_chain, int nBlockHeight)
486486
{
487487
const Consensus::Params& consensusParams = Params().GetConsensus();
488488

489489
if (!IsValidBlockHeight(nBlockHeight)) {
490490
return 0;
491491
}
492492

493-
const CBlockIndex* pindex = ::ChainActive().Tip();
493+
const CBlockIndex* pindex = active_chain.Tip();
494494
if (pindex->nHeight > nBlockHeight) pindex = pindex->GetAncestor(nBlockHeight);
495495

496496
const auto v20_state = g_versionbitscache.State(pindex, consensusParams, Consensus::DEPLOYMENT_V20);
@@ -612,7 +612,7 @@ CAmount CSuperblock::GetPaymentsTotalAmount()
612612
* - Does this transaction match the superblock?
613613
*/
614614

615-
bool CSuperblock::IsValid(CGovernanceManager& govman, const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
615+
bool CSuperblock::IsValid(CGovernanceManager& govman, const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward)
616616
{
617617
// TODO : LOCK(cs);
618618
// No reason for a lock here now since this method only accesses data
@@ -646,7 +646,7 @@ bool CSuperblock::IsValid(CGovernanceManager& govman, const CTransaction& txNew,
646646

647647
// payments should not exceed limit
648648
CAmount nPaymentsTotalAmount = GetPaymentsTotalAmount();
649-
CAmount nPaymentsLimit = GetPaymentsLimit(nBlockHeight);
649+
CAmount nPaymentsLimit = GetPaymentsLimit(active_chain, nBlockHeight);
650650
if (nPaymentsTotalAmount > nPaymentsLimit) {
651651
LogPrintf("CSuperblock::IsValid -- ERROR: Block invalid, payments limit exceeded: payments %lld, limit %lld\n", nPaymentsTotalAmount, nPaymentsLimit);
652652
return false;

‎src/governance/classes.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010
#include <script/standard.h>
1111
#include <uint256.h>
1212

13-
class CTxOut;
14-
class CTransaction;
15-
16-
class CSuperblock;
13+
class CChain;
1714
class CGovernanceManager;
15+
class CSuperblock;
1816
class CSuperblockManager;
17+
class CTxOut;
18+
class CTransaction;
1919

2020
using CSuperblock_sptr = std::shared_ptr<CSuperblock>;
2121

@@ -38,7 +38,7 @@ class CSuperblockManager
3838
static bool GetSuperblockPayments(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector<CTxOut>& voutSuperblockRet);
3939
static void ExecuteBestSuperblock(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, int nBlockHeight);
4040

41-
static bool IsValid(CGovernanceManager& govman, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
41+
static bool IsValid(CGovernanceManager& govman, const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
4242
};
4343

4444
/**
@@ -105,7 +105,7 @@ class CSuperblock : public CGovernanceObject
105105

106106
static bool IsValidBlockHeight(int nBlockHeight);
107107
static void GetNearestSuperblocksHeights(int nBlockHeight, int& nLastSuperblockRet, int& nNextSuperblockRet);
108-
static CAmount GetPaymentsLimit(int nBlockHeight);
108+
static CAmount GetPaymentsLimit(const CChain& active_chain, int nBlockHeight);
109109

110110
SeenObjectStatus GetStatus() const { return nStatus; }
111111
void SetStatus(SeenObjectStatus nStatusIn) { nStatus = nStatusIn; }
@@ -126,7 +126,7 @@ class CSuperblock : public CGovernanceObject
126126
bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet);
127127
CAmount GetPaymentsTotalAmount();
128128

129-
bool IsValid(CGovernanceManager& govman, const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
129+
bool IsValid(CGovernanceManager& govman, const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward);
130130
bool IsExpired(const CGovernanceManager& govman) const;
131131

132132
std::vector<uint256> GetProposalHashes() const;

‎src/governance/governance.cpp‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,12 @@ GovernanceStore::GovernanceStore() :
4545
}
4646

4747
CGovernanceManager::CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman,
48-
const std::unique_ptr<CDeterministicMNManager>& dmnman,
48+
const ChainstateManager& chainman, const std::unique_ptr<CDeterministicMNManager>& dmnman,
4949
const std::unique_ptr<CMasternodeSync>& mn_sync) :
5050
m_db{std::make_unique<db_type>("governance.dat", "magicGovernanceCache")},
5151
m_mn_metaman{mn_metaman},
5252
m_netfulfilledman{netfulfilledman},
53+
m_chainman{chainman},
5354
m_dmnman{dmnman},
5455
m_mn_sync{mn_sync},
5556
nTimeLastDiff(0),
@@ -194,7 +195,7 @@ PeerMsgRet CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, Pe
194195
// CHECK OBJECT AGAINST LOCAL BLOCKCHAIN
195196

196197
bool fMissingConfirmations = false;
197-
bool fIsValid = govobj.IsValidLocally(tip_mn_list, strError, fMissingConfirmations, true);
198+
bool fIsValid = govobj.IsValidLocally(tip_mn_list, m_chainman, strError, fMissingConfirmations, true);
198199

199200
if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true)) {
200201
LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight);
@@ -303,7 +304,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, PeerMana
303304

304305
// MAKE SURE THIS OBJECT IS OK
305306

306-
if (!govobj.IsValidLocally(tip_mn_list, strError, true)) {
307+
if (!govobj.IsValidLocally(tip_mn_list, m_chainman, strError, true)) {
307308
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- invalid governance object - %s - (nCachedBlockHeight %d) \n", strError, nCachedBlockHeight);
308309
return;
309310
}
@@ -387,7 +388,7 @@ void CGovernanceManager::CheckAndRemove()
387388
// IF CACHE IS NOT DIRTY, WHY DO THIS?
388389
if (pObj->IsSetDirtyCache()) {
389390
// UPDATE LOCAL VALIDITY AGAINST CRYPTO DATA
390-
pObj->UpdateLocalValidity(tip_mn_list);
391+
pObj->UpdateLocalValidity(tip_mn_list, m_chainman);
391392

392393
// UPDATE SENTINEL SIGNALING VARIABLES
393394
pObj->UpdateSentinelVariables(tip_mn_list);
@@ -612,7 +613,7 @@ std::optional<const CSuperblock> CGovernanceManager::CreateSuperblockCandidate(i
612613

613614
CSuperblock::GetNearestSuperblocksHeights(nHeight, nLastSuperblock, nNextSuperblock);
614615
auto SBEpochTime = static_cast<int64_t>(GetTime<std::chrono::seconds>().count() + (nNextSuperblock - nHeight) * 2.62 * 60);
615-
auto governanceBudget = CSuperblock::GetPaymentsLimit(nNextSuperblock);
616+
auto governanceBudget = CSuperblock::GetPaymentsLimit(m_chainman.ActiveChain(), nNextSuperblock);
616617

617618
CAmount budgetAllocated{};
618619
for (const auto& proposal : approvedProposals) {
@@ -690,7 +691,7 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
690691
}
691692

692693
// Nobody submitted a trigger we'd like to see, so let's do it but only if we are the payee
693-
const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
694+
const CBlockIndex *tip = WITH_LOCK(::cs_main, return m_chainman.ActiveChain().Tip());
694695
const auto mnList = Assert(m_dmnman)->GetListForBlock(tip);
695696
const auto mn_payees = mnList.GetProjectedMNPayees(tip);
696697

@@ -706,7 +707,7 @@ std::optional<const CGovernanceObject> CGovernanceManager::CreateGovernanceTrigg
706707
gov_sb.SetMasternodeOutpoint(mn_activeman.GetOutPoint());
707708
gov_sb.Sign(mn_activeman);
708709

709-
if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), strError, true)) {
710+
if (std::string strError; !gov_sb.IsValidLocally(m_dmnman->GetListAtChainTip(), m_chainman, strError, true)) {
710711
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s Created trigger is invalid:%s\n", __func__, strError);
711712
return std::nullopt;
712713
}
@@ -1140,8 +1141,8 @@ void CGovernanceManager::CheckPostponedObjects(PeerManager& peerman)
11401141

11411142
std::string strError;
11421143
bool fMissingConfirmations;
1143-
if (govobj.IsCollateralValid(strError, fMissingConfirmations)) {
1144-
if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), strError, false)) {
1144+
if (govobj.IsCollateralValid(m_chainman, strError, fMissingConfirmations)) {
1145+
if (govobj.IsValidLocally(Assert(m_dmnman)->GetListAtChainTip(), m_chainman, strError, false)) {
11451146
AddGovernanceObject(govobj, peerman);
11461147
} else {
11471148
LogPrint(BCLog::GOBJECT, "CGovernanceManager::CheckPostponedObjects -- %s invalid\n", nHash.ToString());

‎src/governance/governance.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ class CGovernanceManager : public GovernanceStore
261261

262262
CMasternodeMetaMan& m_mn_metaman;
263263
CNetFulfilledRequestManager& m_netfulfilledman;
264+
const ChainstateManager& m_chainman;
264265
const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
265266
const std::unique_ptr<CMasternodeSync>& m_mn_sync;
266267

@@ -276,7 +277,7 @@ class CGovernanceManager : public GovernanceStore
276277
std::map<uint256, std::shared_ptr<CSuperblock>> mapTrigger;
277278

278279
public:
279-
explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman,
280+
explicit CGovernanceManager(CMasternodeMetaMan& mn_metaman, CNetFulfilledRequestManager& netfulfilledman, const ChainstateManager& chainman,
280281
const std::unique_ptr<CDeterministicMNManager>& dmnman,
281282
const std::unique_ptr<CMasternodeSync>& mn_sync);
282283
~CGovernanceManager();

‎src/governance/object.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -397,22 +397,22 @@ UniValue CGovernanceObject::ToJson() const
397397
return m_obj.ToJson();
398398
}
399399

400-
void CGovernanceObject::UpdateLocalValidity(const CDeterministicMNList& tip_mn_list)
400+
void CGovernanceObject::UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman)
401401
{
402402
AssertLockHeld(cs_main);
403403
// THIS DOES NOT CHECK COLLATERAL, THIS IS CHECKED UPON ORIGINAL ARRIVAL
404-
fCachedLocalValidity = IsValidLocally(tip_mn_list, strLocalValidityError, false);
404+
fCachedLocalValidity = IsValidLocally(tip_mn_list, chainman, strLocalValidityError, false);
405405
}
406406

407407

408-
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, std::string& strError, bool fCheckCollateral) const
408+
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const
409409
{
410410
bool fMissingConfirmations = false;
411411

412-
return IsValidLocally(tip_mn_list, strError, fMissingConfirmations, fCheckCollateral);
412+
return IsValidLocally(tip_mn_list, chainman, strError, fMissingConfirmations, fCheckCollateral);
413413
}
414414

415-
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const
415+
bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const
416416
{
417417
AssertLockHeld(cs_main);
418418

@@ -433,7 +433,7 @@ bool CGovernanceObject::IsValidLocally(const CDeterministicMNList& tip_mn_list,
433433
strError = strprintf("Invalid proposal data, error messages: %s", validator.GetErrorMessages());
434434
return false;
435435
}
436-
if (fCheckCollateral && !IsCollateralValid(strError, fMissingConfirmations)) {
436+
if (fCheckCollateral && !IsCollateralValid(chainman, strError, fMissingConfirmations)) {
437437
strError = "Invalid proposal collateral";
438438
return false;
439439
}
@@ -483,7 +483,7 @@ CAmount CGovernanceObject::GetMinCollateralFee() const
483483
}
484484
}
485485

486-
bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingConfirmations) const
486+
bool CGovernanceObject::IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const
487487
{
488488
AssertLockHeld(cs_main);
489489

@@ -547,9 +547,9 @@ bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingC
547547
AssertLockHeld(cs_main);
548548
int nConfirmationsIn = 0;
549549
if (nBlockHash != uint256()) {
550-
const CBlockIndex* pindex = g_chainman.m_blockman.LookupBlockIndex(nBlockHash);
551-
if (pindex && ::ChainActive().Contains(pindex)) {
552-
nConfirmationsIn += ::ChainActive().Height() - pindex->nHeight + 1;
550+
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(nBlockHash);
551+
if (pindex && chainman.ActiveChain().Contains(pindex)) {
552+
nConfirmationsIn += chainman.ActiveChain().Height() - pindex->nHeight + 1;
553553
}
554554
}
555555

‎src/governance/object.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ class CDeterministicMNList;
1919
class CGovernanceManager;
2020
class CGovernanceObject;
2121
class CGovernanceVote;
22+
class ChainstateManager;
2223
class CMasternodeMetaMan;
2324
class CMasternodeSync;
2425
class CNode;
@@ -227,14 +228,14 @@ class CGovernanceObject
227228

228229
// CORE OBJECT FUNCTIONS
229230

230-
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, std::string& strError, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
231+
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
231232

232-
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
233+
bool IsValidLocally(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations, bool fCheckCollateral) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
233234

234235
/// Check the collateral transaction for the budget proposal/finalized budget
235-
bool IsCollateralValid(std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
236+
bool IsCollateralValid(const ChainstateManager& chainman, std::string& strError, bool& fMissingConfirmations) const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
236237

237-
void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list);
238+
void UpdateLocalValidity(const CDeterministicMNList& tip_mn_list, const ChainstateManager& chainman);
238239

239240
void UpdateSentinelVariables(const CDeterministicMNList& tip_mn_list);
240241

‎src/init.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
17351735
*/
17361736
const bool is_governance_enabled{!args.GetBoolArg("-disablegovernance", !DEFAULT_GOVERNANCE_ENABLE)};
17371737
assert(!node.govman);
1738-
node.govman = std::make_unique<CGovernanceManager>(*node.mn_metaman, *node.netfulfilledman, node.dmnman, node.mn_sync);
1738+
node.govman = std::make_unique<CGovernanceManager>(*node.mn_metaman, *node.netfulfilledman, *node.chainman, node.dmnman, node.mn_sync);
17391739

17401740
assert(!node.sporkman);
17411741
node.sporkman = std::make_unique<CSporkManager>();
@@ -2021,7 +2021,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
20212021
node.llmq_ctx->Start();
20222022

20232023
node.chain_helper.reset();
2024-
node.chain_helper = std::make_unique<CChainstateHelper>(*node.cpoolman, *node.dmnman, *node.mnhf_manager, *node.govman, *(node.llmq_ctx->quorum_block_processor),
2024+
node.chain_helper = std::make_unique<CChainstateHelper>(*node.cpoolman, *node.dmnman, *node.mnhf_manager, *node.govman, *(node.llmq_ctx->quorum_block_processor), *node.chainman,
20252025
chainparams.GetConsensus(), *node.mn_sync, *node.sporkman, *(node.llmq_ctx->clhandler), *(node.llmq_ctx->qman));
20262026

20272027
if (fReset) {

0 commit comments

Comments
 (0)