Skip to content

Commit 390a1c4

Browse files
kwvgkwvg
committed
trivial: use structured bindings where possible, limit iterator scope
1 parent 3654052 commit 390a1c4

File tree

2 files changed

+65
-81
lines changed

2 files changed

+65
-81
lines changed

‎src/governance/governance.cpp‎

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

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

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

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

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

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

411411
std::string strError;
412412

413413
// MAKE SURE THIS OBJECT IS OK
414414

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

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

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

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

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

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

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

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

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

504501
// IF CACHE IS NOT DIRTY, WHY DO THIS?
505-
if (pObj->IsSetDirtyCache()) {
502+
if (pObj.IsSetDirtyCache()) {
506503
// UPDATE LOCAL VALIDITY AGAINST CRYPTO DATA
507-
pObj->UpdateLocalValidity(tip_mn_list, m_chainman);
504+
pObj.UpdateLocalValidity(tip_mn_list, m_chainman);
508505

509506
// UPDATE SENTINEL SIGNALING VARIABLES
510-
pObj->UpdateSentinelVariables(tip_mn_list);
507+
pObj.UpdateSentinelVariables(tip_mn_list);
511508
}
512509

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

515-
const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj->GetDeletionTime()};
512+
const auto nTimeSinceDeletion = nNow - std::chrono::seconds{pObj.GetDeletionTime()};
516513

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

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

525522
// Remove vote references
526523
const object_ref_cm_t::list_t& listItems = cmapVoteToObject.GetItemList();
527-
auto lit = listItems.begin();
528-
while (lit != listItems.end()) {
529-
if (lit->value == pObj) {
524+
for (auto lit = listItems.begin(); lit != listItems.end();) {
525+
if (lit->value == &pObj) {
530526
uint256 nKey = lit->key;
531527
++lit;
532528
cmapVoteToObject.Erase(nKey);
@@ -537,33 +533,32 @@ void CGovernanceManager::CheckAndRemove()
537533

538534
int64_t nTimeExpired{0};
539535

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

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

564560
// forget about expired deleted objects
565-
auto s_it = mapErasedGovernanceObjects.begin();
566-
while (s_it != mapErasedGovernanceObjects.end()) {
561+
for (auto s_it = mapErasedGovernanceObjects.begin(); s_it != mapErasedGovernanceObjects.end();) {
567562
if (s_it->second < nNow.count()) {
568563
mapErasedGovernanceObjects.erase(s_it++);
569564
} else {
@@ -572,8 +567,7 @@ void CGovernanceManager::CheckAndRemove()
572567
}
573568

574569
// forget about expired requests
575-
auto r_it = m_requested_hash_time.begin();
576-
while (r_it != m_requested_hash_time.end()) {
570+
for (auto r_it = m_requested_hash_time.begin(); r_it != m_requested_hash_time.end();) {
577571
if (r_it->second < nNow) {
578572
m_requested_hash_time.erase(r_it++);
579573
} else {
@@ -620,8 +614,8 @@ CGovernanceObject* CGovernanceManager::FindGovernanceObjectByDataHash(const uint
620614
{
621615
AssertLockNotHeld(cs_store);
622616
LOCK(cs_store);
623-
for (const auto& [nHash, object] : mapObjects) {
624-
if (object.GetDataHash() == nDataHash) return &mapObjects[nHash];
617+
for (const auto& [nHash, govobj] : mapObjects) {
618+
if (govobj.GetDataHash() == nDataHash) return &mapObjects[nHash];
625619
}
626620
return nullptr;
627621
}
@@ -650,13 +644,13 @@ std::vector<CGovernanceVote> CGovernanceManager::GetCurrentVotes(const uint256&
650644
}
651645

652646
// Loop through each MN collateral outpoint and get the votes for the `nParentHash` governance object
653-
for (const auto& mnpair : mapMasternodes) {
647+
for (const auto& [outpoint, _] : mapMasternodes) {
654648
// get a vote_rec_t from the govobj
655649
vote_rec_t voteRecord;
656-
if (!govobj.GetCurrentMNVotes(mnpair.first, voteRecord)) continue;
650+
if (!govobj.GetCurrentMNVotes(outpoint, voteRecord)) continue;
657651

658652
for (const auto& [signal, vote_instance] : voteRecord.mapInstances) {
659-
CGovernanceVote vote = CGovernanceVote(mnpair.first, nParentHash, (vote_signal_enum_t)signal,
653+
CGovernanceVote vote = CGovernanceVote(outpoint, nParentHash, (vote_signal_enum_t)signal,
660654
vote_instance.eOutcome);
661655
vote.SetTime(vote_instance.nCreationTime);
662656
vecResult.push_back(vote);
@@ -670,14 +664,14 @@ void CGovernanceManager::GetAllNewerThan(std::vector<CGovernanceObject>& objs, i
670664
{
671665
LOCK(cs_store);
672666

673-
for (const auto& objPair : mapObjects) {
667+
for (const auto& [_, govobj] : mapObjects) {
674668
// IF THIS OBJECT IS OLDER THAN TIME, CONTINUE
675-
if (objPair.second.GetCreationTime() < nMoreThanTime) {
669+
if (govobj.GetCreationTime() < nMoreThanTime) {
676670
continue;
677671
}
678672

679673
// ADD GOVERNANCE OBJECT TO LIST
680-
objs.push_back(objPair.second);
674+
objs.push_back(govobj);
681675
}
682676
}
683677

@@ -800,11 +794,8 @@ MessageProcessingResult CGovernanceManager::SyncObjects(CNode& peer, CConnman& c
800794

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

810801
if (govobj.IsSetCachedDelete() || govobj.IsSetExpired()) {
@@ -1142,8 +1133,7 @@ int CGovernanceManager::RequestGovernanceObjectVotes(const std::vector<CNode*>&
11421133
for (const auto& [nHash, govobj] : mapObjects) {
11431134
if (govobj.IsSetCachedDelete()) continue;
11441135
if (mapAskedRecently.count(nHash)) {
1145-
auto it = mapAskedRecently[nHash].begin();
1146-
while (it != mapAskedRecently[nHash].end()) {
1136+
for (auto it = mapAskedRecently[nHash].begin(); it != mapAskedRecently[nHash].end();) {
11471137
if (it->second < nNow) {
11481138
mapAskedRecently[nHash].erase(it++);
11491139
} else {
@@ -1231,8 +1221,7 @@ void CGovernanceManager::RebuildIndexes()
12311221
AssertLockHeld(cs_store);
12321222

12331223
cmapVoteToObject.Clear();
1234-
for (auto& objPair : mapObjects) {
1235-
CGovernanceObject& govobj = objPair.second;
1224+
for (auto& [_, govobj] : mapObjects) {
12361225
std::vector<CGovernanceVote> vecVotes = WITH_LOCK(govobj.cs, return govobj.GetVoteFile().GetVotes());
12371226
for (const auto& vecVote : vecVotes) {
12381227
cmapVoteToObject.Insert(vecVote.GetHash(), &govobj);
@@ -1246,9 +1235,7 @@ void CGovernanceManager::AddCachedTriggers()
12461235

12471236
int64_t nNow = GetTime<std::chrono::seconds>().count();
12481237

1249-
for (auto& objpair : mapObjects) {
1250-
CGovernanceObject& govobj = objpair.second;
1251-
1238+
for (auto& [_, govobj] : mapObjects) {
12521239
if (govobj.GetObjectType() != GovernanceObject::TRIGGER) {
12531240
continue;
12541241
}
@@ -1299,8 +1286,8 @@ std::string GovernanceStore::ToString() const
12991286
int nTriggerCount = 0;
13001287
int nOtherCount = 0;
13011288

1302-
for (const auto& objPair : mapObjects) {
1303-
switch (objPair.second.GetObjectType()) {
1289+
for (const auto& [_, govobj] : mapObjects) {
1290+
switch (govobj.GetObjectType()) {
13041291
case GovernanceObject::PROPOSAL:
13051292
nProposalCount++;
13061293
break;
@@ -1332,8 +1319,8 @@ UniValue CGovernanceManager::ToJson() const
13321319
int nTriggerCount = 0;
13331320
int nOtherCount = 0;
13341321

1335-
for (const auto& objpair : mapObjects) {
1336-
switch (objpair.second.GetObjectType()) {
1322+
for (const auto& [_, govobj] : mapObjects) {
1323+
switch (govobj.GetObjectType()) {
13371324
case GovernanceObject::PROPOSAL:
13381325
nProposalCount++;
13391326
break;
@@ -1417,12 +1404,11 @@ void CGovernanceManager::CleanOrphanObjects()
14171404

14181405
int64_t nNow = GetTime<std::chrono::seconds>().count();
14191406

1420-
auto it = items.begin();
1421-
while (it != items.end()) {
1407+
for (auto it = items.begin(); it != items.end();) {
14221408
auto prevIt = it;
14231409
++it;
1424-
const vote_time_pair_t& pairVote = prevIt->value;
1425-
if (pairVote.second < nNow) {
1410+
const auto& [vote, time] = prevIt->value;
1411+
if (time < nNow) {
14261412
cmmapOrphanVotes.Erase(prevIt->key, prevIt->value);
14271413
}
14281414
}
@@ -1440,14 +1426,14 @@ void CGovernanceManager::RemoveInvalidVotes()
14401426
auto diff = lastMNListForVotingKeys->BuildDiff(tip_mn_list);
14411427

14421428
std::vector<COutPoint> changedKeyMNs;
1443-
for (const auto& p : diff.updatedMNs) {
1444-
auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(p.first);
1429+
for (const auto& [internalId, pdmnState] : diff.updatedMNs) {
1430+
auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId);
14451431
// BuildDiff will construct itself with MNs that we already have knowledge
14461432
// of, meaning that fetch operations should never fail.
14471433
assert(oldDmn);
1448-
if ((p.second.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && p.second.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
1434+
if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
14491435
changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
1450-
} else if ((p.second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && p.second.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
1436+
} else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
14511437
changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
14521438
}
14531439
}
@@ -1460,8 +1446,8 @@ void CGovernanceManager::RemoveInvalidVotes()
14601446
}
14611447

14621448
for (const auto& outpoint : changedKeyMNs) {
1463-
for (auto& p : mapObjects) {
1464-
auto removed = p.second.RemoveInvalidVotes(tip_mn_list, outpoint);
1449+
for (auto& [_, govobj] : mapObjects) {
1450+
auto removed = govobj.RemoveInvalidVotes(tip_mn_list, outpoint);
14651451
if (removed.empty()) {
14661452
continue;
14671453
}
@@ -1528,8 +1514,7 @@ void CGovernanceManager::CleanAndRemoveTriggers()
15281514
// Remove triggers that are invalid or expired
15291515
LogPrint(BCLog::GOBJECT, "CGovernanceManager::%s -- mapTrigger.size() = %d\n", __func__, mapTrigger.size());
15301516

1531-
auto it = mapTrigger.begin();
1532-
while (it != mapTrigger.end()) {
1517+
for (auto it = mapTrigger.begin(); it != mapTrigger.end();) {
15331518
bool remove = false;
15341519
CGovernanceObject* pObj = nullptr;
15351520
const CSuperblock_sptr& pSuperblock = it->second;
@@ -1603,10 +1588,10 @@ std::vector<CSuperblock_sptr> CGovernanceManager::GetActiveTriggersInternal() co
16031588
std::vector<CSuperblock_sptr> vecResults;
16041589

16051590
// LOOK AT THESE OBJECTS AND COMPILE A VALID LIST OF TRIGGERS
1606-
for (const auto& pair : mapTrigger) {
1607-
const CGovernanceObject* pObj = InternalFindConstGovernanceObject(pair.first);
1591+
for (const auto& [nHash, pSuperblock] : mapTrigger) {
1592+
const CGovernanceObject* pObj = InternalFindConstGovernanceObject(nHash);
16081593
if (pObj) {
1609-
vecResults.push_back(pair.second);
1594+
vecResults.push_back(pSuperblock);
16101595
}
16111596
}
16121597

@@ -1794,13 +1779,13 @@ std::vector<std::shared_ptr<const CGovernanceObject>> CGovernanceManager::GetApp
17941779
const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10);
17951780

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

18061791
// 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
@@ -535,13 +535,12 @@ int CGovernanceObject::CountMatchingVotes(const CDeterministicMNList& tip_mn_lis
535535
LOCK(cs);
536536

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

0 commit comments

Comments
 (0)