Merge #6835: perf(qt): reduce consequent calls of updateVotingCapability for each block

dacbacc perf: avoid calling updateVotingCapability for each block update (Konstantin Akimov)
1525731 refactor: use unique_ptr instead of shared_ptr in clientmodel for CDeterministicMNList (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Currently, there are 2 handlers for governance in Qt app: `updateProposalList` and `updateVotingCapability`.
  `updateVotingCapability` is calling for each block; `updateProposalList` is calling once in 10 seconds.
  Calling `updateVotingCapability` for each block is very inefficient because during reindex this handler takes uses one full CPU core on my laptop and make Qt app very irresponsible and lagging; it could take up to 10-60 seconds to see an input text in RPC console or react to click.

  ## What was done?
  Call `updateVotingCapability` for each call of `updateProposalList`, no more often.

  Minor improvement: use unique_ptr instead shared_ptr in ClientModel: it returns a copy of object anyway currently instead shared_ptr.

  ## How Has This Been Tested?
  This PR makes `updateVotingCapability` disappear from the top of profiler.

  ## 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

ACKs for top commit:
  UdjinM6:
    utACK dacbacc
  kwvg:
    utACK dacbacc

Tree-SHA512: b97e0bb700104e2dd6d9bc5407c7e36592b9b7c0e0ec2a329482fc8509da8f38e57f8c95f05d560541cc837531740df55c59c5b3fb2311a21eedeaeeb6854262

Author: PastaPastaPasta

src/qt/clientmodel.cpp

@@ -50,7 +50,7 @@ ClientModel::ClientModel(interfaces::Node& node, OptionsModel *_optionsModel, QO
     m_peer_table_sort_proxy->setSourceModel(peerTableModel);
 
     banTableModel = new BanTableModel(m_node, this);
-    mnListCached = std::make_shared<CDeterministicMNList>();
+    mnListCached = std::make_unique<CDeterministicMNList>();
 
     QTimer* timer = new QTimer;
     timer->setInterval(MODEL_UPDATE_DELAY);
@@ -101,26 +101,26 @@ int ClientModel::getNumConnections(unsigned int flags) const
 
 void ClientModel::setMasternodeList(const CDeterministicMNList& mnList, const CBlockIndex* tip)
 {
-    LOCK(cs_mnlinst);
+    LOCK(cs_mnlist);
     if (mnListCached->GetBlockHash() == mnList.GetBlockHash()) {
         return;
     }
-    mnListCached = std::make_shared<CDeterministicMNList>(mnList);
+    mnListCached = std::make_unique<CDeterministicMNList>(mnList);
     mnListTip = tip;
     Q_EMIT masternodeListChanged();
 }
 
 std::pair<CDeterministicMNList, const CBlockIndex*> ClientModel::getMasternodeList() const
 {
-    LOCK(cs_mnlinst);
+    LOCK(cs_mnlist);
     return {*mnListCached, mnListTip};
 }
 
 void ClientModel::refreshMasternodeList()
 {
     auto [mnList, tip] = m_node.evo().getListAtChainTip();
 
-    LOCK(cs_mnlinst);
+    LOCK(cs_mnlist);
     setMasternodeList(mnList, tip);
 }
 

src/qt/clientmodel.h

@@ -46,7 +46,6 @@ enum NumConnections {
 
 class CDeterministicMNList;
 class CGovernanceObject;
-typedef std::shared_ptr<CDeterministicMNList> CDeterministicMNListPtr;
 
 /** Model for Dash network client. */
 class ClientModel : public QObject
@@ -116,8 +115,8 @@ class ClientModel : public QObject
     // 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 RecursiveMutex cs_mnlinst; // protects mnListCached
-    CDeterministicMNListPtr mnListCached;
+    mutable RecursiveMutex cs_mnlist; // protects mnListCached
+    std::unique_ptr<CDeterministicMNList> mnListCached GUARDED_BY(cs_mnlist){};
     const CBlockIndex* mnListTip{nullptr};
 
     void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header);

src/qt/governancelist.cpp

@@ -7,6 +7,7 @@
 #include <qt/governancelist.h>
 #include <qt/proposalwizard.h>
 
+#include <chain.h>
 #include <chainparams.h>
 #include <chainparamsbase.h>
 #include <evo/deterministicmns.h>
@@ -360,16 +361,10 @@ GovernanceList::~GovernanceList() = default;
 void GovernanceList::setClientModel(ClientModel* model)
 {
     this->clientModel = model;
-    updateProposalList();
     if (model != nullptr) {
         connect(model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &GovernanceList::updateDisplayUnit);
 
-        // Update voting capability if we now have both client and wallet models
-        if (walletModel) {
-            updateVotingCapability();
-            // Update voting capability when masternode list changes
-            connect(clientModel, &ClientModel::masternodeListChanged, this, &GovernanceList::updateVotingCapability);
-        }
+        updateProposalList();
     }
 }
 
@@ -410,6 +405,11 @@ void GovernanceList::updateProposalList()
             newProposals.emplace_back(new Proposal(this->clientModel, govObj, proposalModel));
         }
         proposalModel->reconcile(newProposals);
+        // Update voting capability if we now have both client and wallet models
+
+        if (walletModel) {
+            updateVotingCapability();
+        }
     }
 
     // Schedule next update.
@@ -489,11 +489,11 @@ void GovernanceList::updateVotingCapability()
 {
     if (!walletModel || !clientModel) return;
 
-    votableMasternodes.clear();
-    auto [mnList, pindex] = clientModel->getMasternodeList();
+    auto [mn_list, pindex] = clientModel->getMasternodeList();
     if (!pindex) return;
 
-    mnList.ForEachMN(true, [&](const auto& dmn) {
+    votableMasternodes.clear();
+    mn_list.ForEachMN(true, [&](const auto& dmn) {
         // Check if wallet owns the voting key using the same logic as RPC
         const CScript script = GetScriptForDestination(PKHash(dmn.pdmnState->keyIDVoting));
         if (walletModel->wallet().isSpendable(script)) {