Skip to content

Commit 828cad5

Browse files
kwvgkwvg
committed
trivial: use structured bindings where possible, limit iterator scope
1 parent 0bc28c3 commit 828cad5

File tree

3 files changed

+66
-85
lines changed

3 files changed

+66
-85
lines changed

‎src/governance/governance.cpp‎

Lines changed: 63 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,10 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj)
377377
int64_t nNow = GetAdjustedTime();
378378
const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip();
379379
for (const auto& pairVote : vecVotePairs) {
380+
const auto& [vote, time] = pairVote;
380381
bool fRemove = false;
381-
const CGovernanceVote& vote = pairVote.first;
382382
CGovernanceException e;
383-
if (pairVote.second < nNow) {
383+
if (time < nNow) {
384384
fRemove = true;
385385
} else if (govobj.ProcessVote(m_mn_metaman, *this, tip_mn_list, vote, e)) {
386386
RelayVote(vote);
@@ -392,50 +392,51 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj)
392392
}
393393
}
394394

395-
void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& govobj, const CNode* pfrom)
395+
void CGovernanceManager::AddGovernanceObjectInternal(CGovernanceObject& insert_obj, const CNode* pfrom)
396396
{
397397
AssertLockHeld(::cs_main);
398398
AssertLockHeld(cs_store);
399399
AssertLockNotHeld(cs_relay);
400400

401-
uint256 nHash = govobj.GetHash();
401+
uint256 nHash = insert_obj.GetHash();
402402
std::string strHash = nHash.ToString();
403403

404404
const auto tip_mn_list = Assert(m_dmnman)->GetListAtChainTip();
405405

406406
// UPDATE CACHED VARIABLES FOR THIS OBJECT AND ADD IT TO OUR MANAGED DATA
407407

408-
govobj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object
408+
insert_obj.UpdateSentinelVariables(tip_mn_list); //this sets local vars in object
409409

410410
std::string strError;
411411

412412
// MAKE SURE THIS OBJECT IS OK
413413

414-
if (!govobj.IsValidLocally(tip_mn_list, m_chainman, strError, true)) {
414+
if (!insert_obj.IsValidLocally(tip_mn_list, m_chainman, strError, true)) {
415415
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- invalid governance object - %s - (nCachedBlockHeight %d) \n", strError, nCachedBlockHeight);
416416
return;
417417
}
418418

419419
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Adding object: hash = %s, type = %d\n", nHash.ToString(),
420-
ToUnderlying(govobj.GetObjectType()));
420+
ToUnderlying(insert_obj.GetObjectType()));
421421

422422
// INSERT INTO OUR GOVERNANCE OBJECT MEMORY
423423
// IF WE HAVE THIS OBJECT ALREADY, WE DON'T WANT ANOTHER COPY
424-
auto objpair = mapObjects.emplace(nHash, govobj);
424+
auto [emplace_ret, emplace_status] = mapObjects.emplace(nHash, insert_obj);
425425

426-
if (!objpair.second) {
426+
if (!emplace_status) {
427427
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- already have governance object %s\n", nHash.ToString());
428428
return;
429429
}
430430

431431
// SHOULD WE ADD THIS OBJECT TO ANY OTHER MANAGERS?
432432

433+
auto& [_, govobj] = *emplace_ret;
433434
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- Before trigger block, GetDataAsPlainString = %s, nObjectType = %d\n",
434435
govobj.GetDataAsPlainString(), ToUnderlying(govobj.GetObjectType()));
435436

436437
if (govobj.GetObjectType() == GovernanceObject::TRIGGER && !AddNewTrigger(nHash)) {
437438
LogPrint(BCLog::GOBJECT, "CGovernanceManager::AddGovernanceObject -- undo adding invalid trigger object: hash = %s\n", nHash.ToString());
438-
objpair.first->second.PrepareDeletion(GetTime<std::chrono::seconds>().count());
439+
govobj.PrepareDeletion(GetTime<std::chrono::seconds>().count());
439440
return;
440441
}
441442

@@ -491,41 +492,36 @@ void CGovernanceManager::CheckAndRemove()
491492
// Clean up any expired or invalid triggers
492493
CleanAndRemoveTriggers();
493494

494-
auto it = mapObjects.begin();
495495
const auto nNow = GetTime<std::chrono::seconds>();
496-
497-
while (it != mapObjects.end()) {
498-
CGovernanceObject* pObj = &((*it).second);
499-
500-
uint256 nHash = it->first;
496+
for (auto it = mapObjects.begin(); it != mapObjects.end();) {
497+
auto [nHash, pObj] = *it;
501498
std::string strHash = nHash.ToString();
502499

503500
// IF CACHE IS NOT DIRTY, WHY DO THIS?
504-
if (pObj->IsSetDirtyCache()) {
501+
if (pObj.IsSetDirtyCache()) {
505502
// UPDATE LOCAL VALIDITY AGAINST CRYPTO DATA
506-
pObj->UpdateLocalValidity(tip_mn_list, m_chainman);
503+
pObj.UpdateLocalValidity(tip_mn_list, m_chainman);
507504

508505
// UPDATE SENTINEL SIGNALING VARIABLES
509-
pObj->UpdateSentinelVariables(tip_mn_list);
506+
pObj.UpdateSentinelVariables(tip_mn_list);
510507
}
511508

512509
// IF DELETE=TRUE, THEN CLEAN THE MESS UP!
513510

514-
const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj->GetDeletionTime()};
511+
const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj.GetDeletionTime()};
515512

516513
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- Checking object for deletion: %s, deletion time = %d, time since deletion = %d, delete flag = %d, expired flag = %d\n",
517-
strHash, pObj->GetDeletionTime(), nTimeSinceDeletion.count(), pObj->IsSetCachedDelete(), pObj->IsSetExpired());
514+
strHash, pObj.GetDeletionTime(), nTimeSinceDeletion.count(), pObj.IsSetCachedDelete(), pObj.IsSetExpired());
518515

519-
if ((pObj->IsSetCachedDelete() || pObj->IsSetExpired()) &&
516+
if ((pObj.IsSetCachedDelete() || pObj.IsSetExpired()) &&
520517
(nTimeSinceDeletion >= GOVERNANCE_DELETION_DELAY)) {
521-
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- erase obj %s\n", (*it).first.ToString());
522-
m_mn_metaman.RemoveGovernanceObject(pObj->GetHash());
518+
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- erase obj %s\n", nHash.ToString());
519+
m_mn_metaman.RemoveGovernanceObject(pObj.GetHash());
523520

524521
// Remove vote references
525522
const object_ref_cm_t::list_t& listItems = cmapVoteToObject.GetItemList();
526-
auto lit = listItems.begin();
527-
while (lit != listItems.end()) {
528-
if (lit->value == pObj) {
523+
for (auto lit = listItems.begin(); lit != listItems.end();) {
524+
if (lit->value == &pObj) {
529525
uint256 nKey = lit->key;
530526
++lit;
531527
cmapVoteToObject.Erase(nKey);
@@ -536,33 +532,32 @@ void CGovernanceManager::CheckAndRemove()
536532

537533
int64_t nTimeExpired{0};
538534

539-
if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) {
535+
if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) {
540536
// keep hashes of deleted proposals forever
541537
nTimeExpired = std::numeric_limits<int64_t>::max();
542538
} else {
543539
int64_t nSuperblockCycleSeconds = Params().GetConsensus().nSuperblockCycle * Params().GetConsensus().nPowTargetSpacing;
544-
nTimeExpired = (std::chrono::seconds{pObj->GetCreationTime()} +
540+
nTimeExpired = (std::chrono::seconds{pObj.GetCreationTime()} +
545541
std::chrono::seconds{2 * nSuperblockCycleSeconds} + GOVERNANCE_DELETION_DELAY)
546542
.count();
547543
}
548544

549545
mapErasedGovernanceObjects.insert(std::make_pair(nHash, nTimeExpired));
550546
mapObjects.erase(it++);
551547
} else {
552-
if (pObj->GetObjectType() == GovernanceObject::PROPOSAL) {
553-
CProposalValidator validator(pObj->GetDataAsHexString());
548+
if (pObj.GetObjectType() == GovernanceObject::PROPOSAL) {
549+
CProposalValidator validator(pObj.GetDataAsHexString());
554550
if (!validator.Validate()) {
555551
LogPrint(BCLog::GOBJECT, "CGovernanceManager::UpdateCachesAndClean -- set for deletion expired obj %s\n", strHash);
556-
pObj->PrepareDeletion(nNow.count());
552+
pObj.PrepareDeletion(nNow.count());
557553
}
558554
}
559555
++it;
560556
}
561557
}
562558

563559
// forget about expired deleted objects
564-
auto s_it = mapErasedGovernanceObjects.begin();
565-
while (s_it != mapErasedGovernanceObjects.end()) {
560+
for (auto s_it = mapErasedGovernanceObjects.begin(); s_it != mapErasedGovernanceObjects.end();) {
566561
if (s_it->second < nNow.count()) {
567562
mapErasedGovernanceObjects.erase(s_it++);
568563
} else {
@@ -571,8 +566,7 @@ void CGovernanceManager::CheckAndRemove()
571566
}
572567

573568
// forget about expired requests
574-
auto r_it = m_requested_hash_time.begin();
575-
while (r_it != m_requested_hash_time.end()) {
569+
for (auto r_it = m_requested_hash_time.begin(); r_it != m_requested_hash_time.end();) {
576570
if (r_it->second < nNow) {
577571
m_requested_hash_time.erase(r_it++);
578572
} else {
@@ -619,8 +613,8 @@ CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint
619613
{
620614
AssertLockNotHeld(cs_store);
621615
LOCK(cs_store);
622-
for (const auto& [nHash, object] : mapObjects) {
623-
if (object.GetDataHash() == nDataHash) return &mapObjects[nHash];
616+
for (const auto& [nHash, govobj] : mapObjects) {
617+
if (govobj.GetDataHash() == nDataHash) return &mapObjects[nHash];
624618
}
625619
return nullptr;
626620
}
@@ -649,13 +643,13 @@ std::vector<CGovernanceVote> CGovernanceManager::GetCurrentVotes(const uint256&
649643
}
650644

651645
// Loop through each MN collateral outpoint and get the votes for the `nParentHash` governance object
652-
for (const auto& mnpair : mapMasternodes) {
646+
for (const auto& [outpoint, _] : mapMasternodes) {
653647
// get a vote_rec_t from the govobj
654648
vote_rec_t voteRecord;
655-
if (!govobj.GetCurrentMNVotes(mnpair.first, voteRecord)) continue;
649+
if (!govobj.GetCurrentMNVotes(outpoint, voteRecord)) continue;
656650

657651
for (const auto& [signal, vote_instance] : voteRecord.mapInstances) {
658-
CGovernanceVote vote = CGovernanceVote(mnpair.first, nParentHash, (vote_signal_enum_t)signal,
652+
CGovernanceVote vote = CGovernanceVote(outpoint, nParentHash, (vote_signal_enum_t)signal,
659653
vote_instance.eOutcome);
660654
vote.SetTime(vote_instance.nCreationTime);
661655
vecResult.push_back(vote);
@@ -669,14 +663,14 @@ void CGovernanceManager::GetAllNewerThan(std::vector<CGovernanceObject>& objs, i
669663
{
670664
LOCK(cs_store);
671665

672-
for (const auto& objPair : mapObjects) {
666+
for (const auto& [_, govobj] : mapObjects) {
673667
// IF THIS OBJECT IS OLDER THAN TIME, CONTINUE
674-
if (objPair.second.GetCreationTime() < nMoreThanTime) {
668+
if (govobj.GetCreationTime() < nMoreThanTime) {
675669
continue;
676670
}
677671

678672
// ADD GOVERNANCE OBJECT TO LIST
679-
objs.push_back(objPair.second);
673+
objs.push_back(govobj);
680674
}
681675
}
682676

@@ -799,11 +793,8 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c
799793

800794
// all valid objects, no votes
801795
MessageProcessingResult ret{};
802-
for (const auto& objPair : mapObjects) {
803-
uint256 nHash = objPair.first;
804-
const CGovernanceObject& govobj = objPair.second;
796+
for (const auto& [nHash, govobj] : mapObjects) {
805797
std::string strHash = nHash.ToString();
806-
807798
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- attempting to sync govobj: %s, peer=%d\n", __func__, strHash, peer.GetId());
808799

809800
if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) {
@@ -1140,8 +1131,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>&
11401131
for (const auto& [nHash, govobj] : mapObjects) {
11411132
if (govobj.IsSetCachedDelete()) continue;
11421133
if (mapAskedRecently.count(nHash)) {
1143-
auto it = mapAskedRecently[nHash].begin();
1144-
while (it != mapAskedRecently[nHash].end()) {
1134+
for (auto it = mapAskedRecently[nHash].begin(); it != mapAskedRecently[nHash].end();) {
11451135
if (it->second < nNow) {
11461136
mapAskedRecently[nHash].erase(it++);
11471137
} else {
@@ -1229,8 +1219,7 @@ void CGovernanceManager::RebuildIndexes()
12291219
AssertLockHeld(cs_store);
12301220

12311221
cmapVoteToObject.Clear();
1232-
for (auto& objPair : mapObjects) {
1233-
CGovernanceObject& govobj = objPair.second;
1222+
for (auto& [_, govobj] : mapObjects) {
12341223
std::vector<CGovernanceVote> vecVotes = WITH_LOCK(govobj.cs, return govobj.GetVoteFile().GetVotes());
12351224
for (const auto& vecVote : vecVotes) {
12361225
cmapVoteToObject.Insert(vecVote.GetHash(), &govobj);
@@ -1244,9 +1233,7 @@ void CGovernanceManager::AddCachedTriggers()
12441233

12451234
int64_t nNow = GetTime<std::chrono::seconds>().count();
12461235

1247-
for (auto& objpair : mapObjects) {
1248-
CGovernanceObject& govobj = objpair.second;
1249-
1236+
for (auto& [_, govobj] : mapObjects) {
12501237
if (govobj.GetObjectType() != GovernanceObject::TRIGGER) {
12511238
continue;
12521239
}
@@ -1297,8 +1284,8 @@ std::string GovernanceStore::ToString() const
12971284
int nTriggerCount = 0;
12981285
int nOtherCount = 0;
12991286

1300-
for (const auto& objPair : mapObjects) {
1301-
switch (objPair.second.GetObjectType()) {
1287+
for (const auto& [_, govobj] : mapObjects) {
1288+
switch (govobj.GetObjectType()) {
13021289
case GovernanceObject::PROPOSAL:
13031290
nProposalCount++;
13041291
break;
@@ -1330,8 +1317,8 @@ UniValue CGovernanceManager::ToJson() const
13301317
int nTriggerCount = 0;
13311318
int nOtherCount = 0;
13321319

1333-
for (const auto& objpair : mapObjects) {
1334-
switch (objpair.second.GetObjectType()) {
1320+
for (const auto& [_, govobj] : mapObjects) {
1321+
switch (govobj.GetObjectType()) {
13351322
case GovernanceObject::PROPOSAL:
13361323
nProposalCount++;
13371324
break;
@@ -1415,12 +1402,11 @@ void CGovernanceManager::CleanOrphanObjects()
14151402

14161403
int64_t nNow = GetTime<std::chrono::seconds>().count();
14171404

1418-
auto it = items.begin();
1419-
while (it != items.end()) {
1405+
for (auto it = items.begin(); it != items.end();) {
14201406
auto prevIt = it;
14211407
++it;
1422-
const vote_time_pair_t& pairVote = prevIt->value;
1423-
if (pairVote.second < nNow) {
1408+
const auto& [vote, time] = prevIt->value;
1409+
if (time < nNow) {
14241410
cmmapOrphanVotes.Erase(prevIt->key, prevIt->value);
14251411
}
14261412
}
@@ -1438,14 +1424,14 @@ void CGovernanceManager::RemoveInvalidVotes()
14381424
auto diff = lastMNListForVotingKeys->BuildDiff(tip_mn_list);
14391425

14401426
std::vector<COutPoint> changedKeyMNs;
1441-
for (const auto& p : diff.updatedMNs) {
1442-
auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(p.first);
1427+
for (const auto& [internalId, pdmnState] : diff.updatedMNs) {
1428+
auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId);
14431429
// BuildDiff will construct itself with MNs that we already have knowledge
14441430
// of, meaning that fetch operations should never fail.
14451431
assert(oldDmn);
1446-
if ((p.second.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && p.second.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
1432+
if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
14471433
changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
1448-
} else if ((p.second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && p.second.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
1434+
} else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
14491435
changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
14501436
}
14511437
}
@@ -1458,8 +1444,8 @@ void CGovernanceManager::RemoveInvalidVotes()
14581444
}
14591445

14601446
for (const auto& outpoint : changedKeyMNs) {
1461-
for (auto& p : mapObjects) {
1462-
auto removed = p.second.RemoveInvalidVotes(tip_mn_list, outpoint);
1447+
for (auto& [_, govobj] : mapObjects) {
1448+
auto removed = govobj.RemoveInvalidVotes(tip_mn_list, outpoint);
14631449
if (removed.empty()) {
14641450
continue;
14651451
}
@@ -1526,8 +1512,7 @@ void CGovernanceManager::CleanAndRemoveTriggers()
15261512
// Remove triggers that are invalid or expired
15271513
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- mapTrigger.size() = %d\n", __func__, mapTrigger.size());
15281514

1529-
auto it = mapTrigger.begin();
1530-
while (it != mapTrigger.end()) {
1515+
for (auto it = mapTrigger.begin(); it != mapTrigger.end();) {
15311516
bool remove = false;
15321517
CGovernanceObject* pObj = nullptr;
15331518
const CSuperblock_sptr& pSuperblock = it->second;
@@ -1601,10 +1586,10 @@ std::vector<CSuperblock_sptr> CGovernanceManager::GetActiveTriggersInternal() co
16011586
std::vector<CSuperblock_sptr> vecResults;
16021587

16031588
// LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS
1604-
for (const auto& pair : mapTrigger) {
1605-
const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(pair.first);
1589+
for (const auto& [nHash, pSuperblock] : mapTrigger) {
1590+
const CGovernanceObject* pObj = FindConstGovernanceObjectInternal(nHash);
16061591
if (pObj) {
1607-
vecResults.push_back(pair.second);
1592+
vecResults.push_back(pSuperblock);
16081593
}
16091594
}
16101595

@@ -1792,13 +1777,13 @@ std::vector<std::shared_ptr<const CGovernanceObject>> CGovernanceManager::GetApp
17921777
const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10);
17931778

17941779
LOCK(cs_store);
1795-
for (const auto& [unused, object] : mapObjects) {
1780+
for (const auto& [_, govobj] : mapObjects) {
17961781
// Skip all non-proposals objects
1797-
if (object.GetObjectType() != GovernanceObject::PROPOSAL) continue;
1782+
if (govobj.GetObjectType() != GovernanceObject::PROPOSAL) continue;
17981783
// Skip non-passing proposals
1799-
const int absYesCount = object.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING);
1784+
const int absYesCount = govobj.GetAbsoluteYesCount(tip_mn_list, VOTE_SIGNAL_FUNDING);
18001785
if (absYesCount < nAbsVoteReq) continue;
1801-
ret.emplace_back(std::make_shared<const CGovernanceObject>(object));
1786+
ret.emplace_back(std::make_shared<const CGovernanceObject>(govobj));
18021787
}
18031788

18041789
// Sort approved proposals by absolute Yes votes descending

‎src/governance/object.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -534,13 +534,12 @@ int CGovernanceObject::CountMatchingVotes(const CDeterministicMNList& tip_mn_lis
534534
LOCK(cs);
535535

536536
int nCount = 0;
537-
for (const auto& votepair : mapCurrentMNVotes) {
538-
const vote_rec_t& recVote = votepair.second;
537+
for (const auto& [outpoint, recVote] : mapCurrentMNVotes) {
539538
auto it2 = recVote.mapInstances.find(eVoteSignalIn);
540539
if (it2 != recVote.mapInstances.end() && it2->second.eOutcome == eVoteOutcomeIn) {
541540
// 4x times weight vote for EvoNode owners.
542541
// No need to check if v19 is active since no EvoNode are allowed to register before v19s
543-
auto dmn = tip_mn_list.GetMNByCollateral(votepair.first);
542+
auto dmn = tip_mn_list.GetMNByCollateral(outpoint);
544543
if (dmn != nullptr) nCount += GetMnType(dmn->nType).voting_weight;
545544
}
546545
}

0 commit comments

Comments
 (0)