Skip to content

Commit e8a1e8b

Browse files
kwvgkwvg
committed
refactor: add cs annotations for private functions
1 parent 9f5cf4a commit e8a1e8b

File tree

2 files changed

+79
-42
lines changed

2 files changed

+79
-42
lines changed

‎src/governance/governance.cpp‎

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ void CGovernanceManager::Schedule(CScheduler& scheduler, CConnman& connman, Peer
119119

120120
bool CGovernanceManager::LoadCache(bool load_cache)
121121
{
122+
AssertLockNotHeld(cs);
122123
assert(m_db != nullptr);
123124
is_valid = load_cache ? m_db->Load(*this) : m_db->Store(*this);
124125
if (is_valid && load_cache) {
@@ -215,6 +216,7 @@ void CGovernanceManager::AddPostponedObjectInternal(const CGovernanceObject& gov
215216
MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type,
216217
CDataStream& vRecv)
217218
{
219+
AssertLockNotHeld(cs);
218220
AssertLockNotHeld(cs_relay);
219221
if (!IsValid()) return {};
220222
if (!m_mn_sync.IsBlockchainSynced()) return {};
@@ -233,6 +235,7 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
233235
vRecv >> filter;
234236

235237
LogPrint(BCLog::GOBJECT, "MNGOVERNANCESYNC -- syncing governance objects to our peer %s\n", peer.GetLogString());
238+
LOCK(cs);
236239
if (nProp == uint256()) {
237240
return SyncObjects(peer, connman);
238241
} else {
@@ -288,7 +291,8 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
288291
bool fMissingConfirmations = false;
289292
bool fIsValid = govobj.IsValidLocally(tip_mn_list, m_chainman, strError, fMissingConfirmations, true);
290293

291-
if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true)) {
294+
bool unused_rcb;
295+
if (fRateCheckBypassed && fIsValid && !MasternodeRateCheck(govobj, true, true, unused_rcb)) {
292296
LogPrint(BCLog::GOBJECT, "MNGOVERNANCEOBJECT -- masternode rate check failed (after signature verification) - %s - (current block height %d)\n", strHash, nCachedBlockHeight);
293297
return ret;
294298
}
@@ -358,9 +362,9 @@ MessageProcessingResult CGovernanceManager::ProcessMessage(CNode& peer, CConnman
358362

359363
void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj)
360364
{
361-
LOCK(cs);
362-
365+
AssertLockHeld(cs);
363366
AssertLockNotHeld(cs_relay);
367+
364368
uint256 nHash = govobj.GetHash();
365369
std::vector<vote_time_pair_t> vecVotePairs;
366370
cmmapOrphanVotes.GetAll(nHash, vecVotePairs);
@@ -448,6 +452,7 @@ void CGovernanceManager::AddGovernanceObject(CGovernanceObject& govobj, const CN
448452

449453
void CGovernanceManager::CheckAndRemove()
450454
{
455+
AssertLockNotHeld(cs);
451456
assert(m_mn_metaman.IsValid());
452457

453458
// Return on initial sync, spammed the debug.log and provided no use
@@ -707,15 +712,14 @@ bool CGovernanceManager::ConfirmInventoryRequest(const CInv& inv)
707712

708713
MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman)
709714
{
715+
AssertLockHeld(cs);
710716
// do not provide any data until our node is synced
711717
if (!m_mn_sync.IsSynced()) return {};
712718

713719
// SYNC GOVERNANCE OBJECTS WITH OTHER CLIENT
714720

715721
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing single object to peer=%d, nProp = %s\n", __func__, peer.GetId(), nProp.ToString());
716722

717-
LOCK(cs);
718-
719723
// single valid object and its valid votes
720724
auto it = mapObjects.find(nProp);
721725
if (it == mapObjects.end()) {
@@ -758,6 +762,7 @@ MessageProcessingResult CGovernanceManager::SyncSingleObjVotes(CNode& peer, cons
758762

759763
MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& connman) const
760764
{
765+
AssertLockHeld(cs);
761766
assert(m_netfulfilledman.IsValid());
762767

763768
// do not provide any data until our node is synced
@@ -774,8 +779,6 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c
774779

775780
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- syncing all objects to peer=%d\n", __func__, peer.GetId());
776781

777-
LOCK(cs);
778-
779782
// all valid objects, no votes
780783
MessageProcessingResult ret{};
781784
for (const auto& objPair : mapObjects) {
@@ -806,9 +809,9 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c
806809

807810
void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj)
808811
{
809-
if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return;
812+
AssertLockHeld(cs);
810813

811-
LOCK(cs);
814+
if (govobj.GetObjectType() != GovernanceObject::TRIGGER) return;
812815

813816
const COutPoint& masternodeOutpoint = govobj.GetMasternodeOutpoint();
814817
auto it = mapLastMasternodeObject.find(masternodeOutpoint);
@@ -830,13 +833,14 @@ void CGovernanceManager::MasternodeRateUpdate(const CGovernanceObject& govobj)
830833

831834
bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus)
832835
{
836+
LOCK(cs);
833837
bool fRateCheckBypassed;
834838
return MasternodeRateCheck(govobj, fUpdateFailStatus, true, fRateCheckBypassed);
835839
}
836840

837841
bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed)
838842
{
839-
LOCK(cs);
843+
AssertLockHeld(cs);
840844

841845
fRateCheckBypassed = false;
842846

@@ -900,6 +904,7 @@ bool CGovernanceManager::MasternodeRateCheck(const CGovernanceObject& govobj, bo
900904

901905
bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
902906
{
907+
AssertLockNotHeld(cs);
903908
AssertLockNotHeld(cs_relay);
904909
bool fOK = ProcessVote(/*pfrom=*/nullptr, vote, exception, connman);
905910
if (fOK) {
@@ -910,6 +915,7 @@ bool CGovernanceManager::ProcessVoteAndRelay(const CGovernanceVote& vote, CGover
910915

911916
bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
912917
{
918+
AssertLockNotHeld(cs);
913919
ENTER_CRITICAL_SECTION(cs);
914920
uint256 nHashVote = vote.GetHash();
915921
uint256 nHashGovobj = vote.GetParentHash();
@@ -964,10 +970,11 @@ bool CGovernanceManager::ProcessVote(CNode* pfrom, const CGovernanceVote& vote,
964970

965971
void CGovernanceManager::CheckPostponedObjects()
966972
{
973+
AssertLockHeld(cs);
967974
AssertLockNotHeld(cs_relay);
968975
if (!m_mn_sync.IsSynced()) return;
969976

970-
LOCK2(::cs_main, cs);
977+
LOCK(::cs_main);
971978

972979
// Check postponed proposals
973980
for (auto it = mapPostponedObjects.begin(); it != mapPostponedObjects.end();) {
@@ -1032,6 +1039,7 @@ void CGovernanceManager::CheckPostponedObjects()
10321039

10331040
void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter) const
10341041
{
1042+
AssertLockNotHeld(cs);
10351043
if (!pfrom) {
10361044
return;
10371045
}
@@ -1186,6 +1194,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>&
11861194

11871195
bool CGovernanceManager::AcceptMessage(const uint256& nHash)
11881196
{
1197+
AssertLockNotHeld(cs);
11891198
LOCK(cs);
11901199
auto it = m_requested_hash_time.find(nHash);
11911200
if (it == m_requested_hash_time.end()) {
@@ -1199,7 +1208,7 @@ bool CGovernanceManager::AcceptMessage(const uint256& nHash)
11991208

12001209
void CGovernanceManager::RebuildIndexes()
12011210
{
1202-
LOCK(cs);
1211+
AssertLockHeld(cs);
12031212

12041213
cmapVoteToObject.Clear();
12051214
for (auto& objPair : mapObjects) {
@@ -1213,7 +1222,7 @@ void CGovernanceManager::RebuildIndexes()
12131222

12141223
void CGovernanceManager::AddCachedTriggers()
12151224
{
1216-
LOCK(cs);
1225+
AssertLockHeld(cs);
12171226

12181227
int64_t nNow = GetTime<std::chrono::seconds>().count();
12191228

@@ -1325,6 +1334,7 @@ UniValue CGovernanceManager::ToJson() const
13251334

13261335
void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex)
13271336
{
1337+
AssertLockNotHeld(cs);
13281338
AssertLockNotHeld(cs_relay);
13291339
// Note this gets called from ActivateBestChain without cs_main being held
13301340
// so it should be safe to lock our mutex here without risking a deadlock
@@ -1338,6 +1348,7 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex)
13381348
nCachedBlockHeight = pindex->nHeight;
13391349
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdatedBlockTip -- nCachedBlockHeight: %d\n", nCachedBlockHeight);
13401350

1351+
LOCK(cs);
13411352
if (DeploymentDIP0003Enforced(pindex->nHeight, Params().GetConsensus())) {
13421353
RemoveInvalidVotes();
13431354
}
@@ -1349,12 +1360,13 @@ void CGovernanceManager::UpdatedBlockTip(const CBlockIndex* pindex)
13491360

13501361
void CGovernanceManager::RequestOrphanObjects(CConnman& connman)
13511362
{
1363+
AssertLockNotHeld(cs);
13521364
const CConnman::NodesSnapshot snap{connman, /* cond = */ CConnman::FullyConnectedOnly};
13531365

13541366
std::vector<uint256> vecHashesFiltered;
13551367
{
1356-
std::vector<uint256> vecHashes;
13571368
LOCK(cs);
1369+
std::vector<uint256> vecHashes;
13581370
cmmapOrphanVotes.GetKeys(vecHashes);
13591371
for (const uint256& nHash : vecHashes) {
13601372
if (mapObjects.find(nHash) == mapObjects.end()) {
@@ -1394,12 +1406,12 @@ void CGovernanceManager::CleanOrphanObjects()
13941406

13951407
void CGovernanceManager::RemoveInvalidVotes()
13961408
{
1409+
AssertLockHeld(cs);
1410+
13971411
if (!m_mn_sync.IsSynced()) {
13981412
return;
13991413
}
14001414

1401-
LOCK(cs);
1402-
14031415
const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip();
14041416
auto diff = lastMNListForVotingKeys->BuildDiff(tip_mn_list);
14051417

@@ -1733,7 +1745,7 @@ bool CGovernanceManager::IsValidSuperblock(const CChain& active_chain, const CDe
17331745

17341746
void CGovernanceManager::ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight)
17351747
{
1736-
LOCK(cs);
1748+
AssertLockHeld(cs);
17371749

17381750
CSuperblock_sptr pSuperblock;
17391751
if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) {

‎src/governance/governance.h‎

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,13 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
277277

278278
// Basic initialization and querying
279279
bool IsValid() const override { return is_valid; }
280-
bool LoadCache(bool load_cache);
280+
bool LoadCache(bool load_cache)
281+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
281282
std::string ToString() const;
282283
UniValue ToJson() const;
283284
void Clear();
284-
void CheckAndRemove();
285+
void CheckAndRemove()
286+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
285287
void Schedule(CScheduler& scheduler, CConnman& connman, PeerManager& peerman);
286288

287289
// CGovernanceObject
@@ -303,20 +305,23 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
303305
*/
304306
bool ConfirmInventoryRequest(const CInv& inv);
305307
bool ProcessVoteAndRelay(const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman) override
306-
EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
308+
EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay);
307309
int RequestGovernanceObjectVotes(CNode& peer, CConnman& connman, const PeerManager& peerman) const;
308310
int RequestGovernanceObjectVotes(const std::vector<CNode*>& vNodesCopy, CConnman& connman,
309311
const PeerManager& peerman) const;
310312
[[nodiscard]] MessageProcessingResult ProcessMessage(CNode& peer, CConnman& connman, std::string_view msg_type,
311-
CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
313+
CDataStream& vRecv)
314+
EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay);
312315
void RelayObject(const CGovernanceObject& obj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
313316
void RelayVote(const CGovernanceVote& vote) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
314317

315318
// Notification interface trigger
316-
void UpdatedBlockTip(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
319+
void UpdatedBlockTip(const CBlockIndex* pindex)
320+
EXCLUSIVE_LOCKS_REQUIRED(!cs, !cs_relay);
317321

318322
// Signer interface
319-
bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override;
323+
bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus = false) override
324+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
320325
CGovernanceObject* FindGovernanceObjectByDataHash(const uint256& nDataHash) override EXCLUSIVE_LOCKS_REQUIRED(!cs);
321326
std::vector<std::shared_ptr<const CGovernanceObject>> GetApprovedProposals(const CDeterministicMNList& tip_mn_list) override
322327
EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -356,8 +361,11 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
356361

357362
private:
358363
// Branches of ProcessMessage
359-
[[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const;
360-
[[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter, CConnman& connman);
364+
[[nodiscard]] MessageProcessingResult SyncObjects(CNode& peer, CConnman& connman) const
365+
EXCLUSIVE_LOCKS_REQUIRED(cs);
366+
[[nodiscard]] MessageProcessingResult SyncSingleObjVotes(CNode& peer, const uint256& nProp, const CBloomFilter& filter,
367+
CConnman& connman)
368+
EXCLUSIVE_LOCKS_REQUIRED(cs);
361369

362370
// Internal counterparts to "Thread-safe accessors"
363371
void AddPostponedObjectInternal(const CGovernanceObject& govobj)
@@ -370,44 +378,61 @@ class CGovernanceManager : public GovernanceStore, public GovernanceSignerParent
370378
std::vector<std::shared_ptr<CSuperblock>> GetActiveTriggersInternal() const
371379
EXCLUSIVE_LOCKS_REQUIRED(cs);
372380

373-
const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const EXCLUSIVE_LOCKS_REQUIRED(cs);
381+
const CGovernanceObject* FindConstGovernanceObjectInternal(const uint256& nHash) const
382+
EXCLUSIVE_LOCKS_REQUIRED(cs);
374383

375-
void MasternodeRateUpdate(const CGovernanceObject& govobj);
384+
void MasternodeRateUpdate(const CGovernanceObject& govobj)
385+
EXCLUSIVE_LOCKS_REQUIRED(cs);
376386

377-
bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed);
387+
bool MasternodeRateCheck(const CGovernanceObject& govobj, bool fUpdateFailStatus, bool fForce, bool& fRateCheckBypassed)
388+
EXCLUSIVE_LOCKS_REQUIRED(cs);
378389

379-
void CheckPostponedObjects() EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
390+
void CheckPostponedObjects()
391+
EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay);
380392

381-
void InitOnLoad();
393+
void InitOnLoad()
394+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
382395

383396
/*
384397
* Trigger Management (formerly CGovernanceTriggerManager)
385398
* - Track governance objects which are triggers
386399
* - After triggers are activated and executed, they can be removed
387400
*/
388-
bool AddNewTrigger(uint256 nHash) EXCLUSIVE_LOCKS_REQUIRED(cs);
389-
void CleanAndRemoveTriggers() EXCLUSIVE_LOCKS_REQUIRED(cs);
401+
bool AddNewTrigger(uint256 nHash)
402+
EXCLUSIVE_LOCKS_REQUIRED(cs);
403+
void CleanAndRemoveTriggers()
404+
EXCLUSIVE_LOCKS_REQUIRED(cs);
390405

391-
void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight);
406+
void ExecuteBestSuperblock(const CDeterministicMNList& tip_mn_list, int nBlockHeight)
407+
EXCLUSIVE_LOCKS_REQUIRED(cs);
392408

393-
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const;
409+
void RequestGovernanceObject(CNode* pfrom, const uint256& nHash, CConnman& connman, bool fUseFilter = false) const
410+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
394411

395-
bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman);
412+
bool ProcessVote(CNode* pfrom, const CGovernanceVote& vote, CGovernanceException& exception, CConnman& connman)
413+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
396414

397415
/// Called to indicate a requested object or vote has been received
398-
bool AcceptMessage(const uint256& nHash);
416+
bool AcceptMessage(const uint256& nHash)
417+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
399418

400-
void CheckOrphanVotes(CGovernanceObject& govobj) EXCLUSIVE_LOCKS_REQUIRED(!cs_relay);
419+
void CheckOrphanVotes(CGovernanceObject& govobj)
420+
EXCLUSIVE_LOCKS_REQUIRED(cs, !cs_relay);
401421

402-
void RebuildIndexes();
422+
void RebuildIndexes()
423+
EXCLUSIVE_LOCKS_REQUIRED(cs);
403424

404-
void AddCachedTriggers();
425+
void AddCachedTriggers()
426+
EXCLUSIVE_LOCKS_REQUIRED(cs);
405427

406-
void RequestOrphanObjects(CConnman& connman);
428+
void RequestOrphanObjects(CConnman& connman)
429+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
407430

408-
void CleanOrphanObjects();
431+
void CleanOrphanObjects()
432+
EXCLUSIVE_LOCKS_REQUIRED(!cs);
409433

410-
void RemoveInvalidVotes();
434+
void RemoveInvalidVotes()
435+
EXCLUSIVE_LOCKS_REQUIRED(cs);
411436
};
412437

413438
bool AreSuperblocksEnabled(const CSporkManager& sporkman);

0 commit comments

Comments
 (0)