Don't use boost range adaptors in CDeterministicMNList (#2327)

These cause crashes when used in for loops. Reason is very likely that
temporary CDeterministicMNList objects are destroyed before entering the
loop while the adaptors rely on these objects.

Discovered in one of our devnets while calling "protx list"

src/evo/deterministicmns.cpp

@@ -174,10 +174,11 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee() const
         return nullptr;
 
     CDeterministicMNCPtr best;
-    for (const auto& dmn : valid_range()) {
+    ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) {
-        if (!best || CompareByLastPaid(dmn, best))
+        if (!best || CompareByLastPaid(dmn, best)) {
             best = dmn;
         }
+    });
 
     return best;
 }
@@ -310,8 +311,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
     evoDb.Write(std::make_pair(DB_LIST_DIFF, diff.blockHash), diff);
     if ((nHeight % SNAPSHOT_LIST_PERIOD) == 0) {
         evoDb.Write(std::make_pair(DB_LIST_SNAPSHOT, diff.blockHash), newList);
-        LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.size=%d\n",
+        LogPrintf("CDeterministicMNManager::%s -- Wrote snapshot. nHeight=%d, mapCurMNs.allMNsCount=%d\n",
-                  __func__, nHeight, newList.size());
+                  __func__, nHeight, newList.GetAllMNsCount());
     }
 
     if (nHeight == GetSpork15Value()) {
@@ -373,8 +374,8 @@ bool CDeterministicMNManager::BuildNewListFromBlock(const CBlock& block, const C
             if (dmn && dmn->nCollateralIndex == in.prevout.n) {
                 newList.RemoveMN(proTxHash);
 
-                LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was spent. nHeight=%d, mapCurMNs.size=%d\n",
+                LogPrintf("CDeterministicMNManager::%s -- MN %s removed from list because collateral was spent. nHeight=%d, mapCurMNs.allMNsCount=%d\n",
-                          __func__, proTxHash.ToString(), nHeight, newList.size());
+                          __func__, proTxHash.ToString(), nHeight, newList.GetAllMNsCount());
             }
         }
 

src/evo/deterministicmns.h

@@ -15,9 +15,6 @@
 
 #include <map>
 
-#include <boost/range/adaptors.hpp>
-#include <boost/range/any_range.hpp>
-
 class CBlock;
 class CBlockIndex;
 class CValidationState;
@@ -220,41 +217,18 @@ public:
 
 public:
 
-    size_t size() const
+    size_t GetAllMNsCount() const
     {
         return mnMap.size();
     }
 
-    typedef boost::any_range<const CDeterministicMNCPtr&, boost::forward_traversal_tag> range_type;
+    template<typename Callback>
-
+    void ForEachMN(bool onlyValid, Callback&& cb) const {
-    range_type all_range() const
-    {
-        return boost::adaptors::transform(mnMap, [] (const MnMap::value_type& p) -> const CDeterministicMNCPtr& {
-            return p.second;
-        });
-    }
-
-    range_type valid_range() const
-    {
-        return boost::adaptors::filter(all_range(), [&] (const CDeterministicMNCPtr& dmn) -> bool {
-            return IsMNValid(dmn);
-        });
-    }
-
-    size_t all_count() const
-    {
-        return mnMap.size();
-    }
-
-    size_t valid_count() const
-    {
-        size_t c = 0;
         for (const auto& p : mnMap) {
-            if (IsMNValid(p.second)) {
+            if (!onlyValid || IsMNValid(p.second)) {
-                c++;
+                cb(p.second);
             }
         }
-        return c;
     }
 
 public:

src/evo/simplifiedmns.cpp

@@ -56,11 +56,11 @@ CSimplifiedMNList::CSimplifiedMNList(const std::vector<CSimplifiedMNListEntry>&
 
 CSimplifiedMNList::CSimplifiedMNList(const CDeterministicMNList& dmnList)
 {
-    mnList.reserve(dmnList.all_count());
+    mnList.reserve(dmnList.GetAllMNsCount());
 
-    for (const auto& dmn : dmnList.all_range()) {
+    dmnList.ForEachMN(false, [this](const CDeterministicMNCPtr& dmn) {
         mnList.emplace_back(*dmn);
-    }
+    });
 
     std::sort(mnList.begin(), mnList.end(), [&](const CSimplifiedMNListEntry& a, const CSimplifiedMNListEntry& b) {
         return a.proRegTxHash.Compare(b.proRegTxHash) < 0;

src/masternodeman.cpp

@@ -381,7 +381,7 @@ void CMasternodeMan::AddDeterministicMasternodes()
         unsigned int oldMnCount = mapMasternodes.size();
 
         auto mnList = deterministicMNManager->GetListAtChainTip();
-        for (const auto& dmn : mnList.valid_range()) {
+        mnList.ForEachMN(true, [this](const CDeterministicMNCPtr& dmn) {
             // call Find() on each deterministic MN to force creation of CMasternode object
             auto mn = Find(COutPoint(dmn->proTxHash, dmn->nCollateralIndex));
             assert(mn);
@@ -395,7 +395,7 @@ void CMasternodeMan::AddDeterministicMasternodes()
 
             // If it appeared in the valid list, it is enabled no matter what
             mn->nActiveState = CMasternode::MASTERNODE_ENABLED;
-        }
+        });
 
         added = oldMnCount != mapMasternodes.size();
     }
@@ -415,9 +415,9 @@ void CMasternodeMan::RemoveNonDeterministicMasternodes()
         LOCK(cs);
         std::set<COutPoint> mnSet;
         auto mnList = deterministicMNManager->GetListAtChainTip();
-        for (const auto& dmn : mnList.valid_range()) {
+        mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) {
             mnSet.insert(COutPoint(dmn->proTxHash, dmn->nCollateralIndex));
-        }
+        });
         auto it = mapMasternodes.begin();
         while (it != mapMasternodes.end()) {
             if (!mnSet.count(it->second.outpoint)) {
@@ -455,10 +455,11 @@ int CMasternodeMan::CountMasternodes(int nProtocolVersion)
 
     if (deterministicMNManager->IsDeterministicMNsSporkActive()) {
         auto mnList = deterministicMNManager->GetListAtChainTip();
-        for (const auto& dmn : mnList.valid_range()) {
+        mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) {
-            if (dmn->pdmnState->nProtocolVersion < nProtocolVersion) continue;
+            if (dmn->pdmnState->nProtocolVersion >= nProtocolVersion) {
                 nCount++;
             }
+        });
     } else {
         for (const auto& mnpair : mapMasternodes) {
             if(mnpair.second.nProtocolVersion < nProtocolVersion) continue;
@@ -1693,7 +1694,7 @@ std::string CMasternodeMan::ToString() const
 
     if (deterministicMNManager->IsDeterministicMNsSporkActive()) {
         info << "Masternodes: masternode object count: " << (int)mapMasternodes.size() <<
-                ", deterministic masternode count: " << deterministicMNManager->GetListAtChainTip().size() <<
+                ", deterministic masternode count: " << deterministicMNManager->GetListAtChainTip().GetAllMNsCount() <<
                 ", nDsqCount: " << (int)nDsqCount;
     } else {
         info << "Masternodes: " << (int)mapMasternodes.size() <<

src/rpc/governance.cpp

@@ -579,7 +579,7 @@ UniValue gobject_vote_many(const JSONRPCRequest& request)
     // management
     if (deterministicMNManager->IsDeterministicMNsSporkActive()) {
         auto mnList = deterministicMNManager->GetListAtChainTip();
-        for (const auto &dmn : mnList.valid_range()) {
+        mnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) {
             bool found = false;
             for (const auto &mne : entries) {
                 uint256 nTxHash;
@@ -595,10 +595,7 @@ UniValue gobject_vote_many(const JSONRPCRequest& request)
                     break;
                 }
             }
-            if (found) {
+            if (!found) {
-                continue;
-            }
-
                 CKey ownerKey;
                 if (pwalletMain->GetKey(dmn->pdmnState->keyIDVoting, ownerKey)) {
                     CBitcoinSecret secret(ownerKey);
@@ -606,6 +603,7 @@ UniValue gobject_vote_many(const JSONRPCRequest& request)
                     entries.push_back(mne);
                 }
             }
+        });
     }
 #endif
 

src/rpc/rpcevo.cpp

@@ -519,7 +519,7 @@ UniValue protx_list(const JSONRPCRequest& request)
             setOutpts.emplace(outpt.hash);
         }
 
-        for (const auto& dmn : deterministicMNManager->GetListAtChainTip().all_range()) {
+        deterministicMNManager->GetListAtChainTip().ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) {
             if (setOutpts.count(dmn->proTxHash) ||
                 pwalletMain->HaveKey(dmn->pdmnState->keyIDOwner) ||
                 pwalletMain->HaveKey(dmn->pdmnState->keyIDOperator) ||
@@ -528,7 +528,7 @@ UniValue protx_list(const JSONRPCRequest& request)
                 CheckWalletOwnsScript(dmn->pdmnState->scriptOperatorPayout)) {
                 ret.push_back(BuildDMNListEntry(dmn, detailed));
             }
-        }
+        });
     } else if (type == "valid" || type == "registered") {
         if (request.params.size() > 4)
             protx_list_help();
@@ -542,16 +542,10 @@ UniValue protx_list(const JSONRPCRequest& request)
         bool detailed = request.params.size() > 3 ? ParseBoolV(request.params[3], "detailed") : false;
 
         CDeterministicMNList mnList = deterministicMNManager->GetListForBlock(chainActive[height]->GetBlockHash());
-        CDeterministicMNList::range_type range;
+        bool onlyValid = type == "valid";
-
+        mnList.ForEachMN(onlyValid, [&](const CDeterministicMNCPtr& dmn) {
-        if (type == "valid") {
-            range = mnList.valid_range();
-        } else if (type == "registered") {
-            range = mnList.all_range();
-        }
-        for (const auto& dmn : range) {
             ret.push_back(BuildDMNListEntry(dmn, detailed));
-        }
+        });
     } else {
         throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid type specified");
     }

src/test/evo_deterministicmns_tests.cpp

@@ -199,11 +199,15 @@ static CDeterministicMNCPtr FindPayoutDmn(const CBlock& block)
 {
     auto dmnList = deterministicMNManager->GetListAtChainTip();
 
-    for (auto txout : block.vtx[0]->vout) {
+    for (const auto& txout : block.vtx[0]->vout) {
-        for (auto& dmn : dmnList.valid_range()) {
+        CDeterministicMNCPtr found;
-            if (txout.scriptPubKey == dmn->pdmnState->scriptPayout) {
+        dmnList.ForEachMN(true, [&](const CDeterministicMNCPtr& dmn) {
-                return dmn;
+            if (found == nullptr && txout.scriptPubKey == dmn->pdmnState->scriptPayout) {
+                found = dmn;
             }
+        });
+        if (found != nullptr) {
+            return found;
         }
     }
     return nullptr;