fix: possible assert call if nHeight in CDeterministicMNList is higher then Tip (#5590)

## Issue being fixed or feature implemented
fix: possible assert call if nHeight in CDeterministicMNListDiff is
higher than Tip
    
Example of new log:
```
2023-09-28T17:35:50Z GetProjectedMNPayeesAtChainTip WARNING pindex is nullptr due to height=914160 chain height=914159
```
    
instead assert call:
```
...
     #6  0x00007ffff7a33b86 in __assert_fail (assertion=0x55555783afd2 "pindex", file=0x5555577f2ed8 "llmq/utils.cpp", line=730,
            function=0x5555577f2448 "bool llmq::utils::IsMNRewardReallocationActive(const CBlockIndex*)") at ./assert/assert.c:101
     #7  0x0000555555ab7daf in llmq::utils::IsMNRewardReallocationActive (pindex=<optimized out>) at llmq/utils.cpp:730
     #8  0x00005555559458ad in CDeterministicMNList::GetProjectedMNPayees (this=this@entry=0x7fffffffc690, pindex=0x0, nCount=<optimized out>, nCount@entry=2147483647)
            at evo/deterministicmns.cpp:231
     #9  0x000055555594614f in CDeterministicMNList::GetProjectedMNPayeesAtChainTip (this=this@entry=0x7fffffffc690, nCount=nCount@entry=2147483647) at evo/deterministicmns.cpp:216
     #10 0x00005555558c9f51 in MasternodeList::updateDIP3List (this=this@entry=0x55555908cfd0) at qt/masternodelist.cpp:194
     #11 0x00005555558ca9a0 in MasternodeList::updateDIP3ListScheduled (this=0x55555908cfd0) at qt/masternodelist.cpp:157
     #12 0x000055555684a60f in void doActivate<false>(QObject*, int, void**) ()
     #13 0x00005555568525b1 in QTimer::timerEvent(QTimerEvent*) ()
     #14 0x0000555556844ce5 in QObject::event(QEvent*) ()
     #15 0x0000555556ac3252 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
     #16 0x000055555681e6b8 in QCoreApplication::sendEvent(QObject*, QEvent*) ()
     #17 0x000055555686de2a in QTimerInfoList::activateTimers() ()
     #18 0x000055555686be84 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #19 0x00005555569bf8a2 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #20 0x000055555681caf6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #21 0x0000555556825f8a in QCoreApplication::exec() ()
...
```

## What was done?
ClientModel returns now a pair: MNList and CBlockIndex; so, we always
know the which one has been used even if current chain is switched.

## How Has This Been Tested?
Run on my localhost from `c034ff0c2606142ba3e8894bc74f693b87374e5c` -
aborted with backtrace like above.
With both of commit - no assert more.


## Breaking Changes
N/A

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: UdjinM6 <[email protected]>

Author: knst

src/evo/deterministicmns.cpp

@@ -214,38 +214,35 @@ CDeterministicMNCPtr CDeterministicMNList::GetMNPayee(const CBlockIndex* pIndex)
     return best;
 }
 
-std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayeesAtChainTip(int nCount) const
-{
-    return GetProjectedMNPayees(::ChainActive()[nHeight], nCount);
-}
-
 std::vector<CDeterministicMNCPtr> CDeterministicMNList::GetProjectedMNPayees(const CBlockIndex* const pindex, int nCount) const
 {
     if (nCount < 0 ) {
         return {};
     }
-    nCount = std::min(nCount, int(GetValidWeightedMNsCount()));
+    const auto weighted_count = GetValidWeightedMNsCount();
+    nCount = std::min(nCount, int(weighted_count));
 
     std::vector<CDeterministicMNCPtr> result;
-    result.reserve(nCount);
+    result.reserve(weighted_count);
 
-    auto remaining_evo_payments = 0;
-    CDeterministicMNCPtr evo_to_be_skipped = nullptr;
-    bool isMNRewardReallocation = llmq::utils::IsMNRewardReallocationActive(pindex);
-    ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
-        if (dmn->pdmnState->nLastPaidHeight == nHeight) {
-            // We found the last MN Payee.
-            // If the last payee is an EvoNode, we need to check its consecutive payments and pay him again if needed
-            if (!isMNRewardReallocation && dmn->nType == MnType::Evo && dmn->pdmnState->nConsecutivePayments < dmn_types::Evo.voting_weight) {
-                remaining_evo_payments = dmn_types::Evo.voting_weight - dmn->pdmnState->nConsecutivePayments;
-                for ([[maybe_unused]] auto _ : irange::range(remaining_evo_payments)) {
-                    result.emplace_back(dmn);
-                    evo_to_be_skipped = dmn;
+    int remaining_evo_payments{0};
+    CDeterministicMNCPtr evo_to_be_skipped{nullptr};
+    const bool isMNRewardReallocation = llmq::utils::IsMNRewardReallocationActive(pindex);
+    if (!isMNRewardReallocation) {
+        ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
+            if (dmn->pdmnState->nLastPaidHeight == nHeight) {
+                // We found the last MN Payee.
+                // If the last payee is an EvoNode, we need to check its consecutive payments and pay him again if needed
+                if (dmn->nType == MnType::Evo && dmn->pdmnState->nConsecutivePayments < dmn_types::Evo.voting_weight) {
+                    remaining_evo_payments = dmn_types::Evo.voting_weight - dmn->pdmnState->nConsecutivePayments;
+                    for ([[maybe_unused]] auto _ : irange::range(remaining_evo_payments)) {
+                        result.emplace_back(dmn);
+                        evo_to_be_skipped = dmn;
+                    }
                 }
             }
-        }
-        return;
-    });
+        });
+    }
 
     ForEachMNShared(true, [&](const CDeterministicMNCPtr& dmn) {
         if (dmn == evo_to_be_skipped) return;
@@ -647,7 +644,7 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
     // Don't hold cs while calling signals
     if (diff.HasChanges()) {
         GetMainSignals().NotifyMasternodeListChanged(false, oldList, diff, connman);
-        uiInterface.NotifyMasternodeListChanged(newList);
+        uiInterface.NotifyMasternodeListChanged(newList, pindex);
     }
 
     if (nHeight == consensusParams.DIP0003EnforcementHeight) {
@@ -688,7 +685,7 @@ bool CDeterministicMNManager::UndoBlock(const CBlockIndex* pindex)
     if (diff.HasChanges()) {
         auto inversedDiff = curList.BuildDiff(prevList);
         GetMainSignals().NotifyMasternodeListChanged(true, curList, inversedDiff, connman);
-        uiInterface.NotifyMasternodeListChanged(prevList);
+        uiInterface.NotifyMasternodeListChanged(prevList, pindex->pprev);
     }
 
     const auto& consensusParams = Params().GetConsensus();

src/evo/deterministicmns.h

@@ -348,7 +348,6 @@ class CDeterministicMNList
      * @return
      */
     [[nodiscard]] std::vector<CDeterministicMNCPtr> GetProjectedMNPayees(const CBlockIndex* const pindex, int nCount = std::numeric_limits<int>::max()) const;
-    [[nodiscard]] std::vector<CDeterministicMNCPtr> GetProjectedMNPayeesAtChainTip(int nCount = std::numeric_limits<int>::max()) const;
 
     /**
      * Calculate a quorum based on the modifier. The resulting list is deterministically sorted by score

src/governance/governance.cpp

@@ -676,8 +676,10 @@ void CGovernanceManager::CreateGovernanceTrigger(const CSuperblock& sb, CConnman
     if (identical_sb == nullptr) {
         // Nobody submitted a trigger we'd like to see,
         // so let's do it but only if we are the payee
-        auto mnList = deterministicMNManager->GetListAtChainTip();
-        auto mn_payees = mnList.GetProjectedMNPayeesAtChainTip();
+
+        const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
+        auto mnList = deterministicMNManager->GetListForBlock(tip);
+        auto mn_payees = mnList.GetProjectedMNPayees(tip);
         if (mn_payees.empty()) return;
         {
             LOCK(activeMasternodeInfoCs);

src/interfaces/node.h

@@ -21,6 +21,7 @@
 #include <vector>
 
 class BanMan;
+class CBlockIndex;
 class CCoinControl;
 class CDeterministicMNList;
 class CFeeRate;
@@ -45,7 +46,7 @@ class EVO
 {
 public:
     virtual ~EVO() {}
-    virtual CDeterministicMNList getListAtChainTip() = 0;
+    virtual std::pair<CDeterministicMNList, const CBlockIndex*> getListAtChainTip() = 0;
 };
 
 //! Interface for the src/governance part of a dash node (dashd process).
@@ -355,7 +356,8 @@ class Node
 
     //! Register handler for masternode list update messages.
     using NotifyMasternodeListChangedFn =
-        std::function<void(const CDeterministicMNList& newList)>;
+        std::function<void(const CDeterministicMNList& newList,
+                const CBlockIndex* pindex)>;
     virtual std::unique_ptr<Handler> handleNotifyMasternodeListChanged(NotifyMasternodeListChangedFn fn) = 0;
 
     //! Register handler for additional data sync progress update messages.

src/node/interfaces.cpp

@@ -79,9 +79,14 @@ namespace {
 class EVOImpl : public EVO
 {
 public:
-    CDeterministicMNList getListAtChainTip() override
+    std::pair<CDeterministicMNList, const CBlockIndex*> getListAtChainTip() override
     {
-        return deterministicMNManager == nullptr ? CDeterministicMNList() : deterministicMNManager->GetListAtChainTip();
+        const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
+        CDeterministicMNList mnList{};
+        if  (tip != nullptr && deterministicMNManager != nullptr) {
+            mnList = deterministicMNManager->GetListForBlock(tip);
+        }
+        return {std::move(mnList), tip};
     }
 };
 
@@ -499,8 +504,8 @@ class NodeImpl : public Node
     std::unique_ptr<Handler> handleNotifyMasternodeListChanged(NotifyMasternodeListChangedFn fn) override
     {
         return MakeHandler(
-            ::uiInterface.NotifyMasternodeListChanged_connect([fn](const CDeterministicMNList& newList) {
-                fn(newList);
+            ::uiInterface.NotifyMasternodeListChanged_connect([fn](const CDeterministicMNList& newList, const CBlockIndex* pindex) {
+                fn(newList, pindex);
             }));
     }
     std::unique_ptr<Handler> handleNotifyAdditionalDataSyncProgressChanged(NotifyAdditionalDataSyncProgressChangedFn fn) override

src/qt/clientmodel.cpp

@@ -82,26 +82,29 @@ int ClientModel::getNumConnections(unsigned int flags) const
     return m_node.getNodeCount(connections);
 }
 
-void ClientModel::setMasternodeList(const CDeterministicMNList& mnList)
+void ClientModel::setMasternodeList(const CDeterministicMNList& mnList, const CBlockIndex* tip)
 {
     LOCK(cs_mnlinst);
     if (mnListCached->GetBlockHash() == mnList.GetBlockHash()) {
         return;
     }
     mnListCached = std::make_shared<CDeterministicMNList>(mnList);
+    mnListTip = tip;
     Q_EMIT masternodeListChanged();
 }
 
-CDeterministicMNList ClientModel::getMasternodeList() const
+std::pair<CDeterministicMNList, const CBlockIndex*> ClientModel::getMasternodeList() const
 {
     LOCK(cs_mnlinst);
-    return *mnListCached;
+    return {*mnListCached, mnListTip};
 }
 
 void ClientModel::refreshMasternodeList()
 {
+    auto [mnList, tip] = m_node.evo().getListAtChainTip();
+
     LOCK(cs_mnlinst);
-    setMasternodeList(m_node.evo().getListAtChainTip());
+    setMasternodeList(mnList, tip);
 }
 
 int ClientModel::getHeaderTipHeight() const
@@ -332,9 +335,9 @@ static void NotifyChainLock(ClientModel *clientmodel, const std::string& bestCha
     assert(invoked);
 }
 
-static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDeterministicMNList& newList)
+static void NotifyMasternodeListChanged(ClientModel *clientmodel, const CDeterministicMNList& newList, const CBlockIndex* pindex)
 {
-    clientmodel->setMasternodeList(newList);
+    clientmodel->setMasternodeList(newList, pindex);
 }
 
 static void NotifyAdditionalDataSyncProgressChanged(ClientModel *clientmodel, double nSyncProgress)
@@ -355,7 +358,7 @@ void ClientModel::subscribeToCoreSignals()
     m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, false));
     m_handler_notify_chainlock = m_node.handleNotifyChainLock(std::bind(NotifyChainLock, this, std::placeholders::_1, std::placeholders::_2));
     m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, true));
-    m_handler_notify_masternodelist_changed = m_node.handleNotifyMasternodeListChanged(std::bind(NotifyMasternodeListChanged, this, std::placeholders::_1));
+    m_handler_notify_masternodelist_changed = m_node.handleNotifyMasternodeListChanged(std::bind(NotifyMasternodeListChanged, this, std::placeholders::_1, std::placeholders::_2));
     m_handler_notify_additional_data_sync_progess_changed = m_node.handleNotifyAdditionalDataSyncProgressChanged(std::bind(NotifyAdditionalDataSyncProgressChanged, this, std::placeholders::_1));
 }
 

src/qt/clientmodel.h

@@ -67,8 +67,8 @@ class ClientModel : public QObject
     int getHeaderTipHeight() const;
     int64_t getHeaderTipTime() const;
 
-    void setMasternodeList(const CDeterministicMNList& mnList);
-    CDeterministicMNList getMasternodeList() const;
+    void setMasternodeList(const CDeterministicMNList& mnList, const CBlockIndex* tip);
+    std::pair<CDeterministicMNList, const CBlockIndex*> getMasternodeList() const;
     void refreshMasternodeList();
 
     void getAllGovernanceObjects(std::vector<CGovernanceObject> &obj);
@@ -119,6 +119,7 @@ class ClientModel : public QObject
     // representation of the list in UI during initial sync/reindex, so we cache it here too.
     mutable RecursiveMutex cs_mnlinst; // protects mnListCached
     CDeterministicMNListPtr mnListCached;
+    const CBlockIndex* mnListTip;
 
     void subscribeToCoreSignals();
     void unsubscribeFromCoreSignals();

src/qt/governancelist.cpp

@@ -348,7 +348,7 @@ void GovernanceList::updateProposalList()
         // A proposal is considered passing if (YES votes - NO votes) >= (Total Weight of Masternodes / 10),
         // count total valid (ENABLED) masternodes to determine passing threshold.
         // Need to query number of masternodes here with access to clientModel.
-        const int nWeightedMnCount = clientModel->getMasternodeList().GetValidWeightedMNsCount();
+        const int nWeightedMnCount = clientModel->getMasternodeList().first.GetValidWeightedMNsCount();
         const int nAbsVoteReq = std::max(Params().GetConsensus().nGovernanceMinQuorum, nWeightedMnCount / 10);
         proposalModel->setVotingParams(nAbsVoteReq);
 

src/qt/masternodelist.cpp

@@ -166,7 +166,15 @@ void MasternodeList::updateDIP3List()
         return;
     }
 
-    auto mnList = clientModel->getMasternodeList();
+    auto [mnList, pindex] = clientModel->getMasternodeList();
+    auto projectedPayees = mnList.GetProjectedMNPayees(pindex);
+
+    if (projectedPayees.empty() && mnList.GetValidMNsCount() > 0) {
+        // GetProjectedMNPayees failed to provide results for a list with valid mns.
+        // Keep current list and let it try again later.
+        return;
+    }
+
     std::map<uint256, CTxDestination> mapCollateralDests;
 
     {
@@ -191,7 +199,6 @@ void MasternodeList::updateDIP3List()
 
     nTimeUpdatedDIP3 = GetTime();
 
-    auto projectedPayees = mnList.GetProjectedMNPayeesAtChainTip();
     std::map<uint256, int> nextPayments;
     for (size_t i = 0; i < projectedPayees.size(); i++) {
         const auto& dmn = projectedPayees[i];
@@ -222,7 +229,7 @@ void MasternodeList::updateDIP3List()
         QByteArray addr_ba(reinterpret_cast<const char*>(addr_key.data()), addr_key.size());
         QTableWidgetItem* addressItem = new CMasternodeListWidgetItem<QByteArray>(QString::fromStdString(dmn.pdmnState->addr.ToString()), addr_ba);
         QTableWidgetItem* typeItem = new QTableWidgetItem(QString::fromStdString(std::string(GetMnType(dmn.nType).description)));
-        QTableWidgetItem* statusItem = new QTableWidgetItem(mnList.IsMNValid(dmn) ? tr("ENABLED") : (mnList.IsMNPoSeBanned(dmn) ? tr("POSE_BANNED") : tr("UNKNOWN")));
+        QTableWidgetItem* statusItem = new QTableWidgetItem(dmn.pdmnState->IsBanned() ? tr("POSE_BANNED") : tr("ENABLED"));
         QTableWidgetItem* PoSeScoreItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nPoSePenalty), dmn.pdmnState->nPoSePenalty);
         QTableWidgetItem* registeredItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nRegisteredHeight), dmn.pdmnState->nRegisteredHeight);
         QTableWidgetItem* lastPaidItem = new CMasternodeListWidgetItem<int>(QString::number(dmn.pdmnState->nLastPaidHeight), dmn.pdmnState->nLastPaidHeight);
@@ -349,8 +356,7 @@ CDeterministicMNCPtr MasternodeList::GetSelectedDIP3MN()
     uint256 proTxHash;
     proTxHash.SetHex(strProTxHash);
 
-    auto mnList = clientModel->getMasternodeList();
-    return mnList.GetMN(proTxHash);
+    return clientModel->getMasternodeList().first.GetMN(proTxHash);;
 }
 
 void MasternodeList::extraInfoDIP3_clicked()

src/qt/rpcconsole.cpp

@@ -967,7 +967,7 @@ void RPCConsole::updateMasternodeCount()
     if (!clientModel) {
         return;
     }
-    auto mnList = clientModel->getMasternodeList();
+    auto mnList = clientModel->getMasternodeList().first;
     size_t total_mn_count = mnList.GetAllMNsCount();
     size_t total_enabled_mn_count = mnList.GetValidMNsCount();
     size_t total_evo_count = mnList.GetAllEvoCount();
@@ -1247,7 +1247,7 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats)
     ui->peerHeight->setText(QString::number(stats->nodeStats.nStartingHeight));
     ui->peerWhitelisted->setText(stats->nodeStats.m_legacyWhitelisted ? tr("Yes") : tr("No"));
     ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : tr("N/A"));
-    auto dmn = clientModel->getMasternodeList().GetMNByService(stats->nodeStats.addr);
+    auto dmn = clientModel->getMasternodeList().first.GetMNByService(stats->nodeStats.addr);
     if (dmn == nullptr) {
         ui->peerNodeType->setText(tr("Regular"));
         ui->peerPoSeScore->setText(tr("N/A"));

src/rpc/masternode.cpp

@@ -135,8 +135,9 @@ static UniValue masternode_count(const JSONRPCRequest& request)
 
 static UniValue GetNextMasternodeForPayment(int heightShift)
 {
-    auto mnList = deterministicMNManager->GetListAtChainTip();
-    auto payees = mnList.GetProjectedMNPayeesAtChainTip(heightShift);
+    const CBlockIndex *tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
+    auto mnList = deterministicMNManager->GetListForBlock(tip);
+    auto payees = mnList.GetProjectedMNPayees(tip, heightShift);
     if (payees.empty())
         return "unknown";
     auto payee = payees.back();

src/ui_interface.cpp

@@ -58,7 +58,7 @@ void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, b
 void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); }
 void CClientUIInterface::NotifyChainLock(const std::string& bestChainLockHash, int bestChainLockHeight) { return g_ui_signals.NotifyChainLock(bestChainLockHash, bestChainLockHeight); }
 void CClientUIInterface::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); }
-void CClientUIInterface::NotifyMasternodeListChanged(const CDeterministicMNList& list) { return g_ui_signals.NotifyMasternodeListChanged(list); }
+void CClientUIInterface::NotifyMasternodeListChanged(const CDeterministicMNList& list, const CBlockIndex* i) { return g_ui_signals.NotifyMasternodeListChanged(list, i); }
 void CClientUIInterface::NotifyAdditionalDataSyncProgressChanged(double nSyncProgress) { return g_ui_signals.NotifyAdditionalDataSyncProgressChanged(nSyncProgress); }
 void CClientUIInterface::BannedListChanged() { return g_ui_signals.BannedListChanged(); }
 

src/ui_interface.h

@@ -118,7 +118,7 @@ class CClientUIInterface
     ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, bool, const CBlockIndex*);
 
     /** Masternode list has changed */
-    ADD_SIGNALS_DECL_WRAPPER(NotifyMasternodeListChanged, void, const CDeterministicMNList&);
+    ADD_SIGNALS_DECL_WRAPPER(NotifyMasternodeListChanged, void, const CDeterministicMNList&, const CBlockIndex*);
 
     /** Additional data sync progress changed */
     ADD_SIGNALS_DECL_WRAPPER(NotifyAdditionalDataSyncProgressChanged, void, double nSyncProgress);