CMasternodeMan::GetMasternodeRanks removed unneeded MN extra validations and totally unnecessary extra vector load.

furszy · furszy · commit ea434a1ed811 · 2020-10-20T02:14:59.000-03:00
As this function is only used in listmasternodes RPC command, the checks are redundant, the MN thread only should check and update `vMasternodes`, removing any invalid masternode.
diff --git a/src/masternodeman.cpp b/src/masternodeman.cpp
@@ -616,41 +616,27 @@ int CMasternodeMan::GetMasternodeRank(const CTxIn& vin, int64_t nBlockHeight, in
     return -1;
 }
 
-std::vector<std::pair<int, CMasternode> > CMasternodeMan::GetMasternodeRanks(int64_t nBlockHeight, int minProtocol)
+std::vector<std::pair<int64_t, CMasternode>> CMasternodeMan::GetMasternodeRanks(int nBlockHeight)
 {
-    std::vector<std::pair<int64_t, CMasternode> > vecMasternodeScores;
-    std::vector<std::pair<int, CMasternode> > vecMasternodeRanks;
-
+    std::vector<std::pair<int64_t, CMasternode>> vecMasternodeScores;
     const uint256& hash = GetHashAtHeight(nBlockHeight - 1);
     // height outside range
-    if (!hash) return vecMasternodeRanks;
-
-    // scan for winner
-    for (CMasternode& mn : vMasternodes) {
-        mn.Check();
-
-        if (mn.protocolVersion < minProtocol) continue;
+    if (!hash) return vecMasternodeScores;
+    {
+        LOCK(cs);
+        // scan for winner
+        for (CMasternode& mn : vMasternodes) {
+            if (!mn.IsEnabled()) {
+                vecMasternodeScores.emplace_back(9999, mn);
+                continue;
+            }
 
-        if (!mn.IsEnabled()) {
-            vecMasternodeScores.emplace_back(9999, mn);
-            continue;
+            int64_t n2 = mn.CalculateScore(hash).GetCompact(false);
+            vecMasternodeScores.emplace_back(n2, mn);
         }
-
-        uint256 n = mn.CalculateScore(hash);
-        int64_t n2 = n.GetCompact(false);
-
-        vecMasternodeScores.emplace_back(n2, mn);
     }
-
     sort(vecMasternodeScores.rbegin(), vecMasternodeScores.rend(), CompareScoreMN());
-
-    int rank = 0;
-    for (PAIRTYPE(int64_t, CMasternode) & s : vecMasternodeScores) {
-        rank++;
-        vecMasternodeRanks.emplace_back(rank, s.second);
-    }
-
-    return vecMasternodeRanks;
+    return vecMasternodeScores;
 }
 
 void CMasternodeMan::ProcessMessage(CNode* pfrom, std::string& strCommand, CDataStream& vRecv)
diff --git a/src/masternodeman.h b/src/masternodeman.h
@@ -146,8 +146,8 @@ class CMasternodeMan
         Check();
         return vMasternodes;
     }
-
-    std::vector<std::pair<int, CMasternode> > GetMasternodeRanks(int64_t nBlockHeight, int minProtocol = 0);
+    // Retrieve the known masternodes ordered by scoring without checking them. (Only used for listmasternodes RPC call)
+    std::vector<std::pair<int64_t, CMasternode>> GetMasternodeRanks(int nBlockHeight);
     int GetMasternodeRank(const CTxIn& vin, int64_t nBlockHeight, int minProtocol = 0, bool fOnlyActive = true);
 
     void ProcessMessage(CNode* pfrom, std::string& strCommand, CDataStream& vRecv);
diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp
@@ -81,8 +81,9 @@ UniValue listmasternodes(const JSONRPCRequest& request)
     int nHeight = WITH_LOCK(cs_main, return chainActive.Height());
     if (nHeight < 0) return "[]";
 
-    std::vector<std::pair<int, CMasternode> > vMasternodeRanks = mnodeman.GetMasternodeRanks(nHeight);
-    for (PAIRTYPE(int, CMasternode) & s : vMasternodeRanks) {
+    std::vector<std::pair<int64_t, CMasternode>> vMasternodeRanks = mnodeman.GetMasternodeRanks(nHeight);
+    for (int pos=0; pos < (int) vMasternodeRanks.size(); pos++) {
+        const auto& s = vMasternodeRanks[pos];
         UniValue obj(UniValue::VOBJ);
         std::string strVin = s.second.vin.prevout.ToStringShort();
         std::string strTxHash = s.second.vin.prevout.hash.ToString();
@@ -103,7 +104,7 @@ UniValue listmasternodes(const JSONRPCRequest& request)
             LookupHost(strHost.c_str(), node, false);
             std::string strNetwork = GetNetworkName(node.GetNetwork());
 
-            obj.pushKV("rank", (strStatus == "ENABLED" ? s.first : 0));
+            obj.pushKV("rank", (strStatus == "ENABLED" ? pos : 0));
             obj.pushKV("network", strNetwork);
             obj.pushKV("txhash", strTxHash);
             obj.pushKV("outidx", (uint64_t)oIdx);
