A small overhaul of the way MN list/stats UI and data are tied together (#2696)

* Do not access wallet directly from masternodelist ui, use wallet model instead

* Don't access deterministicMNManager from UI directly, use client model instead

* Send just a general signal to UI elements when mn list has changed and let them handle it in their specific ways

* Drop mn list update timers and use signals instead

* some cleanup

* Move initial UI update to init.cpp

* Refactor getMasternodeList()

* Rename setMasternodeCount to updateMasternodeCount

* Drop legacy code in comments

* Drop NotifyMasternodeListChanged from uiInterface and use NotifyBlockTip instead

src/evo/deterministicmns.cpp

@@ -9,6 +9,7 @@
 #include "chainparams.h"
 #include "core_io.h"
 #include "script/standard.h"
+#include "ui_interface.h"
 #include "validation.h"
 #include "validationinterface.h"
 

src/qt/clientmodel.cpp

@@ -23,8 +23,6 @@
 #include "masternode-sync.h"
 #include "privatesend.h"
 
-#include "evo/deterministicmns.h"
-
 #include <stdint.h>
 
 #include <QDebug>
@@ -40,7 +38,6 @@ ClientModel::ClientModel(OptionsModel *_optionsModel, QObject *parent) :
     QObject(parent),
     optionsModel(_optionsModel),
     peerTableModel(0),
-    cachedMasternodeCountString(""),
     banTableModel(0),
     pollTimer(0)
 {
@@ -52,11 +49,6 @@ ClientModel::ClientModel(OptionsModel *_optionsModel, QObject *parent) :
     connect(pollTimer, SIGNAL(timeout()), this, SLOT(updateTimer()));
     pollTimer->start(MODEL_UPDATE_DELAY);
 
-    pollMnTimer = new QTimer(this);
-    connect(pollMnTimer, SIGNAL(timeout()), this, SLOT(updateMnTimer()));
-    // no need to update as frequent as data for balances/txes/blocks
-    pollMnTimer->start(MODEL_UPDATE_DELAY * 4);
-
     subscribeToCoreSignals();
 }
 
@@ -81,16 +73,26 @@ int ClientModel::getNumConnections(unsigned int flags) const
     return 0;
 }
 
-QString ClientModel::getMasternodeCountString() const
+void ClientModel::setMasternodeList(const CDeterministicMNList& mnList)
 {
-    // return tr("Total: %1 (PS compatible: %2 / Enabled: %3) (IPv4: %4, IPv6: %5, TOR: %6)").arg(QString::number((int)mnodeman.size()))
+    LOCK(cs_mnlinst);
-    auto mnList = deterministicMNManager->GetListAtChainTip();
+    if (mnListCached.GetBlockHash() == mnList.GetBlockHash()) {
-    return tr("Total: %1 (Enabled: %2)")
+        return;
-            .arg(QString::number((int)mnList.GetAllMNsCount()))
+    }
-            .arg(QString::number((int)mnList.GetValidMNsCount()));
+    mnListCached = mnList;
-            // .arg(QString::number((int)mnodeman.CountByIP(NET_IPV4)))
+    Q_EMIT masternodeListChanged();
-            // .arg(QString::number((int)mnodeman.CountByIP(NET_IPV6)))
+}
-            // .arg(QString::number((int)mnodeman.CountByIP(NET_TOR)));
+
+CDeterministicMNList ClientModel::getMasternodeList() const
+{
+    LOCK(cs_mnlinst);
+    return mnListCached;
+}
+
+void ClientModel::refreshMasternodeList()
+{
+    LOCK(cs_mnlinst);
+    setMasternodeList(deterministicMNManager->GetListAtChainTip());
 }
 
 int ClientModel::getNumBlocks() const
@@ -178,18 +180,6 @@ void ClientModel::updateTimer()
     Q_EMIT bytesChanged(getTotalBytesRecv(), getTotalBytesSent());
 }
 
-void ClientModel::updateMnTimer()
-{
-    QString newMasternodeCountString = getMasternodeCountString();
-
-    if (cachedMasternodeCountString != newMasternodeCountString)
-    {
-        cachedMasternodeCountString = newMasternodeCountString;
-
-        Q_EMIT strMasternodesChanged(cachedMasternodeCountString);
-    }
-}
-
 void ClientModel::updateNumConnections(int numConnections)
 {
     Q_EMIT numConnectionsChanged(numConnections);
@@ -361,6 +351,14 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, const CB
                                   Q_ARG(bool, fHeader));
         nLastUpdateNotification = now;
     }
+
+    static int64_t nLastMasternodeUpdateNotification = 0;
+    // if we are in-sync, update the UI regardless of last update time
+    // no need to refresh masternode list/stats as often as blocks etc.
+    if (!initialSync || now - nLastMasternodeUpdateNotification > MODEL_UPDATE_DELAY*4*5) {
+        clientmodel->refreshMasternodeList();
+        nLastMasternodeUpdateNotification = now;
+    }
 }
 
 static void NotifyAdditionalDataSyncProgressChanged(ClientModel *clientmodel, double nSyncProgress)

src/qt/clientmodel.h

@@ -6,6 +6,9 @@
 #ifndef BITCOIN_QT_CLIENTMODEL_H
 #define BITCOIN_QT_CLIENTMODEL_H
 
+#include "evo/deterministicmns.h"
+#include "sync.h"
+
 #include <QObject>
 #include <QDateTime>
 
@@ -53,7 +56,6 @@ public:
 
     //! Return number of connections, default is in- and outbound (total)
     int getNumConnections(unsigned int flags = CONNECTIONS_ALL) const;
-    QString getMasternodeCountString() const;
     int getNumBlocks() const;
     int getHeaderTipHeight() const;
     int64_t getHeaderTipTime() const;
@@ -61,6 +63,10 @@ public:
     long getMempoolSize() const;
     //! Return the dynamic memory usage of the mempool
     size_t getMempoolDynamicUsage() const;
+
+    void setMasternodeList(const CDeterministicMNList& mnList);
+    CDeterministicMNList getMasternodeList() const;
+    void refreshMasternodeList();
     
     quint64 getTotalBytesRecv() const;
     quint64 getTotalBytesSent() const;
@@ -92,18 +98,22 @@ public:
 private:
     OptionsModel *optionsModel;
     PeerTableModel *peerTableModel;
-    QString cachedMasternodeCountString;
     BanTableModel *banTableModel;
 
     QTimer *pollTimer;
-    QTimer *pollMnTimer;
+
+    // The cache for mn list is not technically needed because CDeterministicMNManager
+    // caches it internally for recent blocks but it's not enough to get consistent
+    // representation of the list in UI during initial sync/reindex, so we cache it here too.
+    mutable CCriticalSection cs_mnlinst; // protects mnListCached
+    CDeterministicMNList mnListCached;
 
     void subscribeToCoreSignals();
     void unsubscribeFromCoreSignals();
 
 Q_SIGNALS:
     void numConnectionsChanged(int count);
-    void strMasternodesChanged(const QString &strMasternodes);
+    void masternodeListChanged() const;
     void numBlocksChanged(int count, const QDateTime& blockDate, double nVerificationProgress, bool header);
     void additionalDataSyncProgressChanged(double nSyncProgress);
     void mempoolSizeChanged(long count, size_t mempoolSizeInBytes);
@@ -119,7 +129,6 @@ Q_SIGNALS:
 
 public Q_SLOTS:
     void updateTimer();
-    void updateMnTimer();
     void updateNumConnections(int numConnections);
     void updateNetworkActive(bool networkActive);
     void updateAlert(const QString &hash, int status);

src/qt/masternodelist.cpp

@@ -34,7 +34,9 @@ MasternodeList::MasternodeList(const PlatformStyle* platformStyle, QWidget* pare
     QWidget(parent),
     ui(new Ui::MasternodeList),
     clientModel(0),
-    walletModel(0)
+    walletModel(0),
+    fFilterUpdatedDIP3(true),
+    nTimeFilterUpdatedDIP3(0)
 {
     ui->setupUi(this);
 
@@ -74,12 +76,8 @@ MasternodeList::MasternodeList(const PlatformStyle* platformStyle, QWidget* pare
     connect(copyCollateralOutpointAction, SIGNAL(triggered()), this, SLOT(copyCollateralOutpoint_clicked()));
 
     timer = new QTimer(this);
-    connect(timer, SIGNAL(timeout()), this, SLOT(updateDIP3List()));
+    connect(timer, SIGNAL(timeout()), this, SLOT(updateDIP3ListScheduled()));
     timer->start(1000);
-
-    fFilterUpdatedDIP3 = false;
-    nTimeFilterUpdatedDIP3 = GetTime();
-    updateDIP3List();
 }
 
 MasternodeList::~MasternodeList()
@@ -92,7 +90,7 @@ void MasternodeList::setClientModel(ClientModel* model)
     this->clientModel = model;
     if (model) {
         // try to update list when masternode count changes
-        connect(clientModel, SIGNAL(strMasternodesChanged(QString)), this, SLOT(updateNodeList()));
+        connect(clientModel, SIGNAL(masternodeListChanged()), this, SLOT(updateDIP3ListForced()));
     }
 }
 
@@ -107,40 +105,36 @@ void MasternodeList::showContextMenuDIP3(const QPoint& point)
     if (item) contextMenuDIP3->exec(QCursor::pos());
 }
 
-static bool CheckWalletOwnsScript(const CScript& script)
+void MasternodeList::updateDIP3ListScheduled()
 {
-    CTxDestination dest;
+    updateDIP3List(false);
-    if (ExtractDestination(script, dest)) {
-        if ((boost::get<CKeyID>(&dest) && pwalletMain->HaveKey(*boost::get<CKeyID>(&dest))) || (boost::get<CScriptID>(&dest) && pwalletMain->HaveCScript(*boost::get<CScriptID>(&dest)))) {
-            return true;
-        }
-    }
-    return false;
 }
 
-void MasternodeList::updateDIP3List()
+void MasternodeList::updateDIP3ListForced()
 {
-    if (ShutdownRequested()) {
+    updateDIP3List(true);
+}
+
+void MasternodeList::updateDIP3List(bool fForce)
+{
+    if (!clientModel || ShutdownRequested()) {
         return;
     }
 
     TRY_LOCK(cs_dip3list, fLockAcquired);
     if (!fLockAcquired) return;
 
-    static int64_t nTimeListUpdated = GetTime();
+    // To prevent high cpu usage update only once in MASTERNODELIST_FILTER_COOLDOWN_SECONDS seconds
+    // after filter was last changed unless we want to force the update.
+    if (!fForce) {
+        if (!fFilterUpdatedDIP3) return;
 
-    // to prevent high cpu usage update only once in MASTERNODELIST_UPDATE_SECONDS seconds
+        int64_t nSecondsToWait = nTimeFilterUpdatedDIP3 - GetTime() + MASTERNODELIST_FILTER_COOLDOWN_SECONDS;
-    // or MASTERNODELIST_FILTER_COOLDOWN_SECONDS seconds after filter was last changed
-    int64_t nSecondsToWait = fFilterUpdatedDIP3
-                             ? nTimeFilterUpdatedDIP3 - GetTime() + MASTERNODELIST_FILTER_COOLDOWN_SECONDS
-                             : nTimeListUpdated - GetTime() + MASTERNODELIST_UPDATE_SECONDS;
-
-    if (fFilterUpdatedDIP3) {
         ui->countLabelDIP3->setText(QString::fromStdString(strprintf("Please wait... %d", nSecondsToWait)));
-    }
-    if (nSecondsToWait > 0) return;
 
-    nTimeListUpdated = GetTime();
+        if (nSecondsToWait > 0) return;
+    }
+
     fFilterUpdatedDIP3 = false;
 
     QString strToFilter;
@@ -149,7 +143,7 @@ void MasternodeList::updateDIP3List()
     ui->tableWidgetMasternodesDIP3->clearContents();
     ui->tableWidgetMasternodesDIP3->setRowCount(0);
 
-    auto mnList = deterministicMNManager->GetListAtChainTip();
+    auto mnList = clientModel->getMasternodeList();
     auto projectedPayees = mnList.GetProjectedMNPayees(mnList.GetValidMNsCount());
     std::map<uint256, int> nextPayments;
     for (size_t i = 0; i < projectedPayees.size(); i++) {
@@ -158,23 +152,21 @@ void MasternodeList::updateDIP3List()
     }
 
     std::set<COutPoint> setOutpts;
-    if (pwalletMain && ui->checkBoxMyMasternodesOnly->isChecked()) {
+    if (walletModel && ui->checkBoxMyMasternodesOnly->isChecked()) {
-        LOCK(pwalletMain->cs_wallet);
         std::vector<COutPoint> vOutpts;
-        pwalletMain->ListProTxCoins(vOutpts);
+        walletModel->listProTxCoins(vOutpts);
         for (const auto& outpt : vOutpts) {
             setOutpts.emplace(outpt);
         }
     }
 
     mnList.ForEachMN(false, [&](const CDeterministicMNCPtr& dmn) {
-        if (pwalletMain && ui->checkBoxMyMasternodesOnly->isChecked()) {
+        if (walletModel && ui->checkBoxMyMasternodesOnly->isChecked()) {
-            LOCK(pwalletMain->cs_wallet);
             bool fMyMasternode = setOutpts.count(dmn->collateralOutpoint) ||
-                pwalletMain->HaveKey(dmn->pdmnState->keyIDOwner) ||
+                walletModel->havePrivKey(dmn->pdmnState->keyIDOwner) ||
-                pwalletMain->HaveKey(dmn->pdmnState->keyIDVoting) ||
+                walletModel->havePrivKey(dmn->pdmnState->keyIDVoting) ||
-                CheckWalletOwnsScript(dmn->pdmnState->scriptPayout) ||
+                walletModel->havePrivKey(dmn->pdmnState->scriptPayout) ||
-                CheckWalletOwnsScript(dmn->pdmnState->scriptOperatorPayout);
+                walletModel->havePrivKey(dmn->pdmnState->scriptOperatorPayout);
             if (!fMyMasternode) return;
         }
         // populate list
@@ -261,6 +253,10 @@ void MasternodeList::on_checkBoxMyMasternodesOnly_stateChanged(int state)
 
 CDeterministicMNCPtr MasternodeList::GetSelectedDIP3MN()
 {
+    if (!clientModel) {
+        return nullptr;
+    }
+
     std::string strProTxHash;
     {
         LOCK(cs_dip3list);
@@ -278,7 +274,7 @@ CDeterministicMNCPtr MasternodeList::GetSelectedDIP3MN()
     uint256 proTxHash;
     proTxHash.SetHex(strProTxHash);
 
-    auto mnList = deterministicMNManager->GetListAtChainTip();
+    auto mnList = clientModel->getMasternodeList();
     return mnList.GetMN(proTxHash);
 }
 

src/qt/masternodelist.h

@@ -6,7 +6,7 @@
 #include "sync.h"
 #include "util.h"
 
-#include <evo/deterministicmns.h>
+#include "evo/deterministicmns.h"
 
 #include <QMenu>
 #include <QTimer>
@@ -38,20 +38,12 @@ public:
 
     void setClientModel(ClientModel* clientModel);
     void setWalletModel(WalletModel* walletModel);
-    CDeterministicMNCPtr GetSelectedDIP3MN();
 
 private:
     QMenu* contextMenuDIP3;
     int64_t nTimeFilterUpdatedDIP3;
     bool fFilterUpdatedDIP3;
 
-public Q_SLOTS:
-    void updateDIP3List();
-
-Q_SIGNALS:
-    void doubleClicked(const QModelIndex&);
-
-private:
     QTimer* timer;
     Ui::MasternodeList* ui;
     ClientModel* clientModel;
@@ -62,6 +54,13 @@ private:
 
     QString strCurrentFilterDIP3;
 
+    CDeterministicMNCPtr GetSelectedDIP3MN();
+
+    void updateDIP3List(bool fForce);
+
+Q_SIGNALS:
+    void doubleClicked(const QModelIndex&);
+
 private Q_SLOTS:
     void showContextMenuDIP3(const QPoint&);
     void on_filterLineEditDIP3_textChanged(const QString& strFilterIn);
@@ -70,5 +69,8 @@ private Q_SLOTS:
     void extraInfoDIP3_clicked();
     void copyProTxHash_clicked();
     void copyCollateralOutpoint_clicked();
+
+    void updateDIP3ListScheduled();
+    void updateDIP3ListForced();
 };
 #endif // MASTERNODELIST_H

src/qt/rpcconsole.cpp

@@ -556,8 +556,8 @@ void RPCConsole::setClientModel(ClientModel *model)
         updateNetworkState();
         connect(model, SIGNAL(networkActiveChanged(bool)), this, SLOT(setNetworkActive(bool)));
 
-        setMasternodeCount(model->getMasternodeCountString());
+        connect(model, SIGNAL(masternodeListChanged()), this, SLOT(updateMasternodeCount()));
-        connect(model, SIGNAL(strMasternodesChanged(QString)), this, SLOT(setMasternodeCount(QString)));
+        clientModel->refreshMasternodeList();
 
         updateTrafficStats(model->getTotalBytesRecv(), model->getTotalBytesSent());
         connect(model, SIGNAL(bytesChanged(quint64,quint64)), this, SLOT(updateTrafficStats(quint64, quint64)));
@@ -884,9 +884,16 @@ void RPCConsole::setNumBlocks(int count, const QDateTime& blockDate, double nVer
     }
 }
 
-void RPCConsole::setMasternodeCount(const QString &strMasternodes)
+void RPCConsole::updateMasternodeCount()
 {
-    ui->masternodeCount->setText(strMasternodes);
+    if (!clientModel) {
+        return;
+    }
+    auto mnList = clientModel->getMasternodeList();
+    QString strMasternodeCount = tr("Total: %1 (Enabled: %2)")
+        .arg(QString::number(mnList.GetAllMNsCount()))
+        .arg(QString::number(mnList.GetValidMNsCount()));
+    ui->masternodeCount->setText(strMasternodeCount);
 }
 
 void RPCConsole::setMempoolSize(long numberOfTxs, size_t dynUsage)

src/qt/rpcconsole.h

@@ -105,8 +105,8 @@ public Q_SLOTS:
     void setNumConnections(int count);
     /** Set network state shown in the UI */
     void setNetworkActive(bool networkActive);
-    /** Set number of masternodes shown in the UI */
+    /** Update number of masternodes shown in the UI */
-    void setMasternodeCount(const QString &strMasternodes);
+    void updateMasternodeCount();
     /** Set number of blocks and last block date shown in the UI */
     void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool headers);
     /** Set size (number of transactions and memory usage) of the mempool in the UI */

src/qt/walletmodel.cpp

@@ -645,6 +645,17 @@ bool WalletModel::havePrivKey(const CKeyID &address) const
     return wallet->HaveKey(address);
 }
 
+bool WalletModel::havePrivKey(const CScript& script) const
+{
+    CTxDestination dest;
+    if (ExtractDestination(script, dest)) {
+        if ((boost::get<CKeyID>(&dest) && wallet->HaveKey(*boost::get<CKeyID>(&dest))) || (boost::get<CScriptID>(&dest) && wallet->HaveCScript(*boost::get<CScriptID>(&dest)))) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool WalletModel::getPrivKey(const CKeyID &address, CKey& vchPrivKeyOut) const
 {
     return wallet->GetKey(address, vchPrivKeyOut);
@@ -732,6 +743,12 @@ void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts)
     wallet->ListLockedCoins(vOutpts);
 }
 
+void WalletModel::listProTxCoins(std::vector<COutPoint>& vOutpts)
+{
+    LOCK2(cs_main, wallet->cs_wallet);
+    wallet->ListProTxCoins(vOutpts);
+}
+
 void WalletModel::loadReceiveRequests(std::vector<std::string>& vReceiveRequests)
 {
     LOCK(wallet->cs_wallet);

src/qt/walletmodel.h

@@ -202,6 +202,7 @@ public:
 
     bool getPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const;
     bool havePrivKey(const CKeyID &address) const;
+    bool havePrivKey(const CScript& script) const;
     bool getPrivKey(const CKeyID &address, CKey& vchPrivKeyOut) const;
     void getOutputs(const std::vector<COutPoint>& vOutpoints, std::vector<COutput>& vOutputs);
     bool isSpent(const COutPoint& outpoint) const;
@@ -212,6 +213,8 @@ public:
     void unlockCoin(COutPoint& output);
     void listLockedCoins(std::vector<COutPoint>& vOutpts);
 
+    void listProTxCoins(std::vector<COutPoint>& vOutpts);
+
     void loadReceiveRequests(std::vector<std::string>& vReceiveRequests);
     bool saveReceiveRequest(const std::string &sAddress, const int64_t nId, const std::string &sRequest);
 

src/ui_interface.h

@@ -16,6 +16,7 @@ class CBasicKeyStore;
 class CWallet;
 class uint256;
 class CBlockIndex;
+class CDeterministicMNList;
 
 /** General change type (added, updated, removed). */
 enum ChangeType
@@ -88,9 +89,6 @@ public:
     /** Network activity state changed. */
     boost::signals2::signal<void (bool networkActive)> NotifyNetworkActiveChanged;
 
-    /** Number of masternodes changed. */
-    boost::signals2::signal<void (int newNumMasternodes)> NotifyStrMasternodeCountChanged;
-
     /**
      * New, updated or cancelled alert.
      * @note called with lock cs_mapAlerts held.