Merge dashpay#7176: perf: do linear lookup instead building 2 heavy Hash-Maps

6ffe808 refactor: apply review suggestions (Konstantin Akimov)
bb6facd perf: do linear lookup when it's needed instead building 2 heavy Hash-Maps which used almost never (Konstantin Akimov)
9d26734 refactor: remove unused getObjLocalValidity (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Currently, for each update of masternode list 3 list are generated: list of shared_pointers, hash-map by service, hash-map by protx.

  Creating these hash-maps creates overhead for every user for every masternode update. It causes excessive battery use, excessive RAM usage while these lookups happens in rare conditions (only for masternode owners during voting; only when "Peer Detail" is open).

  It's better to do linear lookup _once_ over all masternode in the specific scenario, rather than build these heavy objects for _every_ update of list.

  Lookup during voting may looks scary due to potential O(N^2). Though, it's not N^2, but N*M (where M is amount of masternodes that belongs to single Owner, M<<N). Secondly, N is relatively small and _voting_ is not assumed to be rapid-fast action (milliseconds); it's okay if user will wait extra 10ms in the worst case scenario for each its vote.

  ## What was done?
  Code is updated to do linear lookup instead building heavy hash-maps for every update.

  Current implementation (in develop) has a bug with dangling pointer and potentially could cause UB: dashpay#7118 (comment)
  Fixed by this PR

  ## How Has This Been Tested?
  N/A

  ## 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 _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 6ffe808
  kwvg:
    utACK 6ffe808

Tree-SHA512: b401d399767825a0363edfe8aa89dd6f1c44184874ec40aeebcde36917b643d69bc8a670f3b6e6aaa9a35af88d30810e55413137a8c02955103fca89a46e0c0a

Author: PastaPastaPasta

src/Makefile.qt.include

@@ -200,7 +200,6 @@ BITCOIN_QT_H = \
   qt/transactionrecord.h \
   qt/transactiontablemodel.h \
   qt/transactionview.h \
-  qt/util.h \
   qt/utilitydialog.h \
   qt/walletcontroller.h \
   qt/walletframe.h \

src/interfaces/node.h

@@ -148,7 +148,6 @@ class GOV
         uint16_t m_evo{0};
     };
     virtual UniqueVoters getObjUniqueVoters(const CGovernanceObject& obj, vote_signal_enum_t vote_signal) = 0;
-    virtual bool getObjLocalValidity(const CGovernanceObject& obj, std::string& error, bool check_collateral) = 0;
     virtual bool existsObj(const uint256& hash) = 0;
     virtual bool isEnabled() = 0;
     virtual bool processVoteAndRelay(const CGovernanceVote& vote, std::string& error) = 0;

src/node/interfaces.cpp

@@ -263,14 +263,6 @@ class GOVImpl : public GOV
         }
         return false;
     }
-    bool getObjLocalValidity(const CGovernanceObject& obj, std::string& error, bool check_collateral) override
-    {
-        if (context().govman != nullptr && context().chainman != nullptr && context().dmnman != nullptr) {
-            LOCK(cs_main);
-            return obj.IsValidLocally(context().dmnman->GetListAtChainTip(), *(context().chainman), error, check_collateral);
-        }
-        return false;
-    }
     bool isEnabled() override
     {
         if (context().govman != nullptr) {

src/qt/clientfeeds.cpp

@@ -14,7 +14,6 @@
 #include <qt/clientmodel.h>
 #include <qt/masternodemodel.h>
 #include <qt/proposalmodel.h>
-#include <qt/util.h>
 
 #include <QDebug>
 #include <QThread>
@@ -188,9 +187,7 @@ void MasternodeFeed::fetch()
             nNextPayment = nextPaymentIt->second;
         }
         auto entry = std::make_shared<MasternodeEntry>(dmn, collateralStr, nNextPayment);
-        ret->m_by_protx[dmn->getProTxHash()] = entry.get();
-        ret->m_by_service[util::make_array(dmn->getNetInfoPrimary().GetKey())] = entry.get();
-        ret->m_entries.push_back(std::move(entry));
+        ret->m_entries.emplace_back(std::move(entry));
     });
 
     ret->m_valid = true;

src/qt/clientfeeds.h

@@ -151,10 +151,8 @@ class InstantSendFeed : public Feed<InstantSendData> {
 struct MasternodeData {
     bool m_valid{false};
     int m_list_height{0};
-    interfaces::MnList::Counts m_counts{};
-    QHash<QByteArray, const MasternodeEntry*> m_by_service{};
-    std::vector<std::shared_ptr<MasternodeEntry>> m_entries{};
-    Uint256HashMap<const MasternodeEntry*> m_by_protx{};
+    interfaces::MnList::Counts m_counts;
+    std::vector<std::shared_ptr<MasternodeEntry>> m_entries;
 };
 
 class MasternodeFeed : public Feed<MasternodeData> {

src/qt/guiutil.h

@@ -531,6 +531,11 @@ namespace GUIUtil
         return false;
     }
 
+    template <typename T1>
+    inline QByteArray MakeQByteArray(const T1& data)
+    {
+        return QByteArray(reinterpret_cast<const char*>(data.data()), data.size());
+    }
 } // namespace GUIUtil
 
 #endif // BITCOIN_QT_GUIUTIL_H

src/qt/proposallist.cpp

@@ -576,16 +576,19 @@ void ProposalList::voteForProposal(vote_outcome_enum_t outcome)
     // Vote with each masternode
     for (const auto& [proTxHash, votingKeyID] : votableMasternodes) {
         // Find the masternode
-        const auto* dmn = [&]() -> const MasternodeEntry* {
-            if (const auto it = data_mn->m_by_protx.find(proTxHash); it != data_mn->m_by_protx.end() && !it->second->isBanned()) {
-                return it->second;
+        QString protx_hash{QString::fromStdString(proTxHash.ToString())};
+        const auto dmn = [&]() -> const std::shared_ptr<MasternodeEntry> {
+            for (const auto& mn : data_mn->m_entries) {
+                if (mn->proTxHash() == protx_hash) {
+                    return mn->isBanned() ? nullptr : mn;
+                }
             }
             return nullptr;
         }();
 
         if (!dmn) {
             nFailed++;
-            failedMessages.append(tr("Masternode %1 not found").arg(QString::fromStdString(proTxHash.ToString())));
+            failedMessages.append(tr("Masternode %1 not found").arg(protx_hash));
             continue;
         }
 
@@ -595,8 +598,7 @@ void ProposalList::voteForProposal(vote_outcome_enum_t outcome)
         // Sign vote via wallet interface
         if (!walletModel->wallet().signGovernanceVote(votingKeyID, vote)) {
             nFailed++;
-            failedMessages.append(
-                tr("Failed to sign vote for masternode %1").arg(QString::fromStdString(proTxHash.ToString())));
+            failedMessages.append(tr("Failed to sign vote for masternode %1").arg(protx_hash));
             continue;
         }
 

src/qt/rpcconsole.cpp

@@ -20,7 +20,6 @@
 #include <qt/masternodemodel.h>
 #include <qt/networkwidget.h>
 #include <qt/peertablesortproxy.h>
-#include <qt/util.h>
 #include <qt/walletcontroller.h>
 #include <qt/walletmodel.h>
 
@@ -1291,12 +1290,12 @@ void RPCConsole::updateDetailWidget()
     }
     ui->peerMappedAS->setText(stats->nodeStats.m_mapped_as != 0 ? QString::number(stats->nodeStats.m_mapped_as) : ts.na);
 
-    const auto addr_key{util::make_array(stats->nodeStats.addr.GetKey())};
-    const MasternodeEntry* dmn = [&]() -> const MasternodeEntry* {
+    const auto addr_key{GUIUtil::MakeQByteArray(stats->nodeStats.addr.GetKey())};
+    const std::shared_ptr<MasternodeEntry> dmn = [&]() -> const std::shared_ptr<MasternodeEntry> {
         if (m_feed_masternode) {
             if (const auto data{m_feed_masternode->data()}; data) {
-                if (auto it = data->m_by_service.find(addr_key); it != data->m_by_service.end()) {
-                    return it.value();
+                for (const auto& mn : data->m_entries) {
+                    if (mn->serviceKey() == addr_key) return mn;
                 }
             }
         }

src/qt/util.h

@@ -42,7 +42,6 @@ src/qt/proposalinfo.*
 src/qt/proposallist.*
 src/qt/proposalmodel.*
 src/qt/proposalresume.*
-src/qt/util.h
 src/rpc/coinjoin.cpp
 src/rpc/evo.cpp
 src/rpc/evo_util.*