Skip to content

Commit 1731272

Browse files
committed
refactor: protect GovernanceStore::cs, add accessor to remove ext. use
1 parent 423c1eb commit 1731272

3 files changed

Lines changed: 15 additions & 10 deletions

File tree

src/governance/governance.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,12 @@ void CGovernanceManager::CheckAndRemove()
568568
}
569569

570570
const CGovernanceObject* CGovernanceManager::FindConstGovernanceObject(const uint256& nHash) const
571+
{
572+
LOCK(cs);
573+
return FindConstGovernanceObjectInternal(nHash);
574+
}
575+
576+
const CGovernanceObject* CGovernanceManager::FindConstGovernanceObjectInternal(const uint256& nHash) const
571577
{
572578
AssertLockHeld(cs);
573579

@@ -1039,7 +1045,7 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
10391045
size_t nVoteCount = 0;
10401046
if (fUseFilter) {
10411047
LOCK(cs);
1042-
const CGovernanceObject* pObj = FindConstGovernanceObject(nHash);
1048+
const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash);
10431049

10441050
if (pObj) {
10451051
filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRand<int>(/*nMax=*/999999), BLOOM_UPDATE_ALL);
@@ -1562,7 +1568,7 @@ std::vector<CSuperblock_sptr> CGovernanceManager::GetActiveTriggersInternal() co
15621568

15631569
// LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS
15641570
for (const auto& pair : mapTrigger) {
1565-
const CGovernanceObject* pObj = FindConstGovernanceObject(pair.first);
1571+
const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(pair.first);
15661572
if (pObj) {
15671573
vecResults.push_back(pair.second);
15681574
}

src/governance/governance.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,10 @@ class GovernanceStore
177177
static constexpr int MAX_CACHE_SIZE = 1000000;
178178
static const std::string SERIALIZATION_VERSION_STRING;
179179

180-
public:
180+
protected:
181181
// critical section to protect the inner data structures
182182
mutable RecursiveMutex cs;
183183

184-
protected:
185184
// keep track of the scanning errors
186185
std::map<uint256, CGovernanceObject> mapObjects GUARDED_BY(cs);
187186
// mapErasedGovernanceObjects contains key-value pairs, where
@@ -298,7 +297,6 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
298297
[[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type,
299298
CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
300299

301-
const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs);
302300
CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs);
303301

304302
// These commands are only used in RPC
@@ -339,6 +337,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
339337
void AddPostponedObject(const CGovernanceObject& govobj)
340338
EXCLUSIVE_LOCKS_REQUIRED(!cs);
341339

340+
const CGovernanceObject* FindConstGovernanceObject(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
341+
342342
// Thread-safe accessors for trigger management
343343
std::vector<std::shared_ptr<CSuperblock>> GetActiveTriggers() const override
344344
EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -409,6 +409,8 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
409409
std::vector<std::shared_ptr<CSuperblock>> GetActiveTriggersInternal() const
410410
EXCLUSIVE_LOCKS_REQUIRED(cs);
411411

412+
const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs);
413+
412414
void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight);
413415

414416
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const;

src/rpc/governance.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,6 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const CWallet
414414
const NodeContext& node = EnsureAnyNodeContext(request.context);
415415
CHECK_NONFATAL(node.govman);
416416
{
417-
LOCK(node.govman->cs);
418417
const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hash);
419418
if (!pGovObj) {
420419
throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found");
@@ -604,7 +603,7 @@ static UniValue ListObjects(CGovernanceManager& govman, const CDeterministicMNLi
604603
g_txindex->BlockUntilSyncedToCurrentChain();
605604
}
606605

607-
LOCK2(cs_main, govman.cs);
606+
LOCK(cs_main);
608607

609608
std::vector<CGovernanceObject> objs;
610609
govman.GetAllNewerThan(objs, nStartTime);
@@ -729,7 +728,7 @@ static RPCHelpMan gobject_get()
729728
const ChainstateManager& chainman = EnsureChainman(node);
730729

731730
CHECK_NONFATAL(node.govman);
732-
LOCK2(cs_main, node.govman->cs);
731+
LOCK(cs_main);
733732
const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash);
734733

735734
if (pGovObj == nullptr) {
@@ -828,7 +827,6 @@ static RPCHelpMan gobject_getcurrentvotes()
828827
const NodeContext& node = EnsureAnyNodeContext(request.context);
829828

830829
CHECK_NONFATAL(node.govman);
831-
LOCK(node.govman->cs);
832830
const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash);
833831

834832
if (pGovObj == nullptr) {
@@ -924,7 +922,6 @@ static RPCHelpMan voteraw()
924922
const NodeContext& node = EnsureAnyNodeContext(request.context);
925923
CHECK_NONFATAL(node.govman);
926924
GovernanceObject govObjType = [&]() {
927-
LOCK(node.govman->cs);
928925
const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hashGovObj);
929926
if (!pGovObj) {
930927
throw JSONRPCError(RPC_INVALID_PARAMETER, "Governance object not found");

0 commit comments

Comments
 (0)