pack of small cleanup fixes / optimizations (#2334)

* remove vector, extra loop in cleanup function

This commit removes 2 loops and a vector which I don't believe are necessary in
CMasternode::FlagGovernanceItemsAsDirty. I could be missing something, but
can't think of any good reason why this was implemented this way.

* use range operator to range over vectors

* remove deprecated wire message types

* mn: simplify govobj map mgmt a bit

* remove extra semicolons

* invert if/else condition and drop else

* remove if/else logic from Qt

This is the entire purpose of the Get<X>String methods on MNP class.

* Revert "remove deprecated wire message types"

This reverts commit 9de88a3.

* Revert "remove if/else logic from Qt"

This reverts commit c0f43c9.

Author: nmarley

src/governance.cpp

@@ -270,9 +270,8 @@ void CGovernanceManager::CheckOrphanVotes(CGovernanceObject& govobj, CGovernance
     ScopedLockBool guard(cs, fRateChecksEnabled, false);
 
     int64_t nNow = GetAdjustedTime();
-    for (size_t i = 0; i < vecVotePairs.size(); ++i) {
+    for (auto& pairVote : vecVotePairs) {
         bool fRemove = false;
-        vote_time_pair_t& pairVote = vecVotePairs[i];
         CGovernanceVote& vote = pairVote.first;
         CGovernanceException exception;
         if (pairVote.second < nNow) {
@@ -368,8 +367,8 @@ void CGovernanceManager::UpdateCachesAndClean()
 
     LOCK2(cs_main, cs);
 
-    for (size_t i = 0; i < vecDirtyHashes.size(); ++i) {
-        object_m_it it = mapObjects.find(vecDirtyHashes[i]);
+    for (const uint256& nHash : vecDirtyHashes) {
+        object_m_it it = mapObjects.find(nHash);
         if (it == mapObjects.end()) {
             continue;
         }
@@ -1018,8 +1017,8 @@ void CGovernanceManager::RequestGovernanceObject(CNode* pfrom, const uint256& nH
             filter = CBloomFilter(Params().GetConsensus().nGovernanceFilterElements, GOVERNANCE_FILTER_FP_RATE, GetRandInt(999999), BLOOM_UPDATE_ALL);
             std::vector<CGovernanceVote> vecVotes = pObj->GetVoteFile().GetVotes();
             nVoteCount = vecVotes.size();
-            for (size_t i = 0; i < vecVotes.size(); ++i) {
-                filter.insert(vecVotes[i].GetHash());
+            for (const auto& vote : vecVotes) {
+                filter.insert(vote.GetHash());
             }
         }
     }
@@ -1296,19 +1295,16 @@ void CGovernanceManager::RequestOrphanObjects(CConnman& connman)
         std::vector<uint256> vecHashes;
         LOCK(cs);
         cmmapOrphanVotes.GetKeys(vecHashes);
-        for (size_t i = 0; i < vecHashes.size(); ++i) {
-            const uint256& nHash = vecHashes[i];
+        for (const uint256& nHash : vecHashes) {
             if (mapObjects.find(nHash) == mapObjects.end()) {
                 vecHashesFiltered.push_back(nHash);
             }
         }
     }
 
     LogPrint("gobject", "CGovernanceObject::RequestOrphanObjects -- number objects = %d\n", vecHashesFiltered.size());
-    for (size_t i = 0; i < vecHashesFiltered.size(); ++i) {
-        const uint256& nHash = vecHashesFiltered[i];
-        for (size_t j = 0; j < vNodesCopy.size(); ++j) {
-            CNode* pnode = vNodesCopy[j];
+    for (const uint256& nHash : vecHashesFiltered) {
+        for (CNode* pnode : vNodesCopy) {
             if (pnode->fMasternode) {
                 continue;
             }

src/masternode.cpp

@@ -993,20 +993,16 @@ std::string CMasternodePing::GetDaemonString() const
 
 void CMasternode::AddGovernanceVote(uint256 nGovernanceObjectHash)
 {
-    if(mapGovernanceObjectsVotedOn.count(nGovernanceObjectHash)) {
-        mapGovernanceObjectsVotedOn[nGovernanceObjectHash]++;
-    } else {
-        mapGovernanceObjectsVotedOn.insert(std::make_pair(nGovernanceObjectHash, 1));
-    }
+    // Insert a zero value, or not. Then increment the value regardless. This
+    // ensures the value is in the map.
+    const auto& pair = mapGovernanceObjectsVotedOn.emplace(nGovernanceObjectHash, 0);
+    pair.first->second++;
 }
 
 void CMasternode::RemoveGovernanceObject(uint256 nGovernanceObjectHash)
 {
-    std::map<uint256, int>::iterator it = mapGovernanceObjectsVotedOn.find(nGovernanceObjectHash);
-    if(it == mapGovernanceObjectsVotedOn.end()) {
-        return;
-    }
-    mapGovernanceObjectsVotedOn.erase(it);
+    // Whether or not the govobj hash exists in the map first is irrelevant.
+    mapGovernanceObjectsVotedOn.erase(nGovernanceObjectHash);
 }
 
 /**
@@ -1017,15 +1013,7 @@ void CMasternode::RemoveGovernanceObject(uint256 nGovernanceObjectHash)
 */
 void CMasternode::FlagGovernanceItemsAsDirty()
 {
-    std::vector<uint256> vecDirty;
-    {
-        std::map<uint256, int>::iterator it = mapGovernanceObjectsVotedOn.begin();
-        while(it != mapGovernanceObjectsVotedOn.end()) {
-            vecDirty.push_back(it->first);
-            ++it;
-        }
-    }
-    for(size_t i = 0; i < vecDirty.size(); ++i) {
-        mnodeman.AddDirtyGovernanceObjectHash(vecDirty[i]);
+    for (auto& govObjHashPair : mapGovernanceObjectsVotedOn) {
+        mnodeman.AddDirtyGovernanceObjectHash(govObjHashPair.first);
     }
 }

src/masternodeman.h

@@ -226,7 +226,7 @@ class CMasternodeMan
         LOCK(cs);
         std::vector<uint256> vecTmp = vecDirtyGovernanceObjectHashes;
         vecDirtyGovernanceObjectHashes.clear();
-        return vecTmp;;
+        return vecTmp;
     }
 
     bool IsSentinelPingActive();

src/privatesend-client.cpp

@@ -305,7 +305,7 @@ std::string CPrivateSendClientSession::GetStatus(bool fWaitForBlock)
             if(     nStatusMessageProgress % 70 <= 30) strSuffix = ".";
             else if(nStatusMessageProgress % 70 <= 50) strSuffix = "..";
             else if(nStatusMessageProgress % 70 <= 70) strSuffix = "...";
-            return strprintf(_("Submitted to masternode, waiting in queue %s"), strSuffix);;
+            return strprintf(_("Submitted to masternode, waiting in queue %s"), strSuffix);
         case POOL_STATE_ACCEPTING_ENTRIES:
             if(nEntriesCount == 0) {
                 nStatusMessageProgress = 0;

src/qt/masternodelist.cpp

@@ -474,10 +474,7 @@ void MasternodeList::ShowQRCode(std::string strAlias) {
     bool fFound = false;
 
     for (const auto& mne : masternodeConfig.getEntries()) {
-        if (strAlias != mne.getAlias()) {
-            continue;
-        }
-        else {
+        if (strAlias == mne.getAlias()) {
             strMNPrivKey = mne.getPrivKey();
             strCollateral = mne.getTxHash() + "-" + mne.getOutputIndex();
             strIP = mne.getIp();