Merge #2717: [GUI][Bug] Don't set a proposal's status to PASSING_NOT_…

…FUNDED prematurely

8a8e089 GUI: Differentiate between not passing and over budget in mini-card (Fuzzbawls)
1d28002 GUI: Don't set a proposal's status to PASSING_NOT_FUNDED prematurely (Fuzzbawls)
2964387 Refactor: clang-tidy governancemodel.h and proposalcard.h (Fuzzbawls)
7582276 Refactor: Initialize default values for ProposalInfo struct (Fuzzbawls)
d5aa3e2 Build: Fix sign comparison in governancemodel.cpp (Fuzzbawls)

Pull request description:

  Simply checking that there are more "yes" votes than "no" votes isn't
  enough context to determine if a proposal should be in a passing state
  (`PASSING` or `PASSING_NOT_FUNDED`)

  Also, Set a unique label text for proposals that are passing but there aren't
  enough funds left in the budget to find them.

  -----------

  Also includes some very light refactoring/cleanup in related files and a compile-time warning fix.

  There is some follow-up refactoring to be done, but will do that in a separate PR as it falls out-of-scope for this PR.

ACKs for top commit:
  random-zebra:
    ACK 8a8e089
  furszy:
    ACK 8a8e089 and merging..

Tree-SHA512: efd9384c8b00bd579ff6c7a41c1abf29225c77c5318c1f0ad5a0f9c89d2e1ef0bb58527a40cf26f31f8a4239a3886c04a3802dddcf23417deb739d21d6e19c5e

src/qt/clientmodel.cpp

@@ -75,18 +75,19 @@ int ClientModel::getNumConnections(unsigned int flags) const
     return 0;
 }
 
-QString ClientModel::getMasternodeCountString() const
+QString ClientModel::getMasternodeCountString()
 {
     const auto& info = mnodeman.getMNsInfo();
     int unknown = std::max(0, info.total - info.ipv4 - info.ipv6 - info.onion);
+    m_cached_masternodes_count = info.total;
     return tr("Total: %1 (IPv4: %2 / IPv6: %3 / Tor: %4 / Unknown: %5)").arg(QString::number(info.total))
                                                                         .arg(QString::number(info.ipv4))
                                                                         .arg(QString::number(info.ipv6))
                                                                         .arg(QString::number(info.onion))
                                                                         .arg(QString::number(unknown));
 }
 
-QString ClientModel::getMasternodesCount()
+QString ClientModel::getMasternodesCountString()
 {
     if (!cachedMasternodeCountString.isEmpty()) {
         return cachedMasternodeCountString;

src/qt/clientmodel.h

@@ -13,6 +13,7 @@
 #include <QObject>
 #include <QDateTime>
 
+#include <atomic>
 #include <memory>
 
 class AddressTableModel;
@@ -106,7 +107,8 @@ class ClientModel : public QObject
     void stopMasternodesTimer();
     // Force a MN count update calling mnmanager directly locking its internal mutex.
     // Future todo: implement an event based update and remove the lock requirement.
-    QString getMasternodesCount();
+    QString getMasternodesCountString();
+    int getMasternodesCount() const { return m_cached_masternodes_count; }
 
     // Return the specific chain amount value for the MN collateral output.
     CAmount getMNCollateralRequiredAmount();
@@ -120,7 +122,7 @@ class ClientModel : public QObject
     std::unique_ptr<interfaces::Handler> m_handler_banned_list_changed;
     std::unique_ptr<interfaces::Handler> m_handler_notify_block_tip;
 
-    QString getMasternodeCountString() const;
+    QString getMasternodeCountString();
     OptionsModel* optionsModel;
     PeerTableModel* peerTableModel;
     BanTableModel *banTableModel;
@@ -136,6 +138,8 @@ class ClientModel : public QObject
     QTimer* pollTimer;
     QTimer* pollMnTimer;
 
+    std::atomic_int m_cached_masternodes_count{0};
+
     void subscribeToCoreSignals();
     void unsubscribeFromCoreSignals();
 

src/qt/pivx/governancemodel.cpp

@@ -8,7 +8,6 @@
 #include "budget/budgetutil.h"
 #include "destination_io.h"
 #include "guiconstants.h"
-#include "script/standard.h"
 #include "qt/transactiontablemodel.h"
 #include "qt/transactionrecord.h"
 #include "qt/pivx/mnmodel.h"
@@ -54,6 +53,7 @@ ProposalInfo GovernanceModel::buildProposalInfo(const CBudgetProposal* prop, boo
     // Calculate status
     int votesYes = prop->GetYeas();
     int votesNo = prop->GetNays();
+    int mnCount = clientModel->getMasternodesCount();
     int remainingPayments = prop->GetRemainingPaymentCount(clientModel->getLastBlockProcessedHeight());
     ProposalInfo::Status status;
 
@@ -65,7 +65,7 @@ ProposalInfo GovernanceModel::buildProposalInfo(const CBudgetProposal* prop, boo
             status = ProposalInfo::FINISHED;
         } else if (isPassing) {
             status = ProposalInfo::PASSING;
-        } else if (votesYes > votesNo) {
+        } else if (votesYes - votesNo > mnCount / 10) {
             status = ProposalInfo::PASSING_NOT_FUNDED;
         } else {
             status = ProposalInfo::NOT_PASSING;
@@ -182,7 +182,7 @@ std::vector<VoteInfo> GovernanceModel::getLocalMNsVotesForProposal(const Proposa
 
 OperationResult GovernanceModel::validatePropName(const QString& name) const
 {
-    if ((int) name.toUtf8().size() > PROP_NAME_MAX_SIZE) { // limit
+    if (name.toUtf8().size() > (int)PROP_NAME_MAX_SIZE) { // limit
         return {false, _("Invalid name, maximum size exceeded")};
     }
     return {true};

src/qt/pivx/governancemodel.h

@@ -30,22 +30,22 @@ struct ProposalInfo {
     uint256 id;
     std::string name;
     std::string url;
-    int votesYes;
-    int votesNo;
+    int votesYes{0};
+    int votesNo{0};
     /** Payment script destination */
     std::string recipientAdd;
     /** Amount of PIV paid per month */
-    CAmount amount;
+    CAmount amount{0};
     /** Amount of times that the proposal will be paid */
-    int totalPayments;
+    int totalPayments{0};
     /** Amount of times that the proposal was paid already */
-    int remainingPayments;
+    int remainingPayments{0};
     /** Proposal state */
-    Status status;
+    Status status{WAITING_FOR_APPROVAL};
     /** Start superblock height */
-    int startBlock;
+    int startBlock{0};
     /** End superblock height */
-    int endBlock;
+    int endBlock{0};
 
     ProposalInfo() {}
     explicit ProposalInfo(const uint256& _id, std::string  _name, std::string  _url,
@@ -58,19 +58,19 @@ struct ProposalInfo {
             endBlock(_endBlock) {}
 
     bool operator==(const ProposalInfo& prop2) const { return id == prop2.id; }
-    bool isFinished() { return status == Status::FINISHED; }
+    bool isFinished() const { return status == Status::FINISHED; }
     std::string statusToStr() const;
 };
 
 struct VoteInfo {
     enum VoteDirection {
-        ABSTAIN=0,
-        YES=1,
-        NO=2
+        ABSTAIN = 0,
+        YES     = 1,
+        NO      = 2
     };
 
     explicit VoteInfo(const COutPoint _mnId, VoteDirection _vote, std::string _mnAlias, int64_t _time) :
-        mnVoter(_mnId), vote(_vote), mnAlias(_mnAlias), time(_time) {}
+        mnVoter(_mnId), vote(_vote), mnAlias(std::move(_mnAlias)), time(_time) {}
     COutPoint mnVoter;
     VoteDirection vote;
     std::string mnAlias;

src/qt/pivx/proposalcard.cpp

@@ -50,7 +50,7 @@ void ProposalCard::setProposal(const ProposalInfo& _proposalInfo)
     ui->labelYes->setText(tr("Yes") + " " + QString::number(percentageYes) + "%");
 
     QString cssClassStatus;
-    if (proposalInfo.status == ProposalInfo::WAITING_FOR_APPROVAL){
+    if (proposalInfo.status == ProposalInfo::WAITING_FOR_APPROVAL) {
         cssClassStatus = "card-status-no-votes";
         setStatusAndVotes(tr("Waiting"), 50);
     } else if (proposalInfo.status == ProposalInfo::FINISHED) {
@@ -59,10 +59,12 @@ void ProposalCard::setProposal(const ProposalInfo& _proposalInfo)
     } else if (totalVotes == 0) {
         cssClassStatus = "card-status-no-votes";
         setStatusAndVotes(tr("No Votes"), 50);
-    } else if (proposalInfo.status == ProposalInfo::NOT_PASSING ||
-        proposalInfo.status == ProposalInfo::PASSING_NOT_FUNDED) {
+    } else if (proposalInfo.status == ProposalInfo::NOT_PASSING) {
         cssClassStatus = "card-status-not-passing";
         setStatusAndVotes(tr("Not Passing"), (int)percentageNo);
+    } else if (proposalInfo.status == ProposalInfo::PASSING_NOT_FUNDED) {
+        cssClassStatus = "card-status-not-passing";
+        setStatusAndVotes(tr("Over Budget"), (int)percentageNo);
     } else if (proposalInfo.status == ProposalInfo::PASSING) {
         cssClassStatus = "card-status-passing";
         setStatusAndVotes(tr("Passing"), (int)percentageNo);

src/qt/pivx/proposalcard.h

@@ -8,7 +8,6 @@
 #include "qt/pivx/governancemodel.h"
 
 #include <QGridLayout>
-#include <QProgressBar>
 #include <QWidget>
 
 namespace Ui {
@@ -21,14 +20,14 @@ class ProposalCard : public QWidget
 
 public:
     explicit ProposalCard(QWidget *parent = nullptr);
-    ~ProposalCard();
+    ~ProposalCard() override;
 
     void setProposal(const ProposalInfo& _proposalInfo);
     ProposalInfo getProposal() { return proposalInfo; }
 
     // Update-only functions
     void setNeedsUpdate(bool _update) { needsUpdate = _update; }
-    bool isUpdateNeeded() { return needsUpdate; }
+    bool isUpdateNeeded() const { return needsUpdate; }
 
 public Q_SLOTS:
     void onCopyUrlClicked();

src/qt/pivx/settings/settingsinformationwidget.cpp

@@ -204,7 +204,7 @@ void SettingsInformationWidget::run(int type)
 {
     if (type == REQUEST_UPDATE_COUNTS) {
         QMetaObject::invokeMethod(this, "setMasternodeCount",
-                                  Qt::QueuedConnection, Q_ARG(QString, clientModel->getMasternodesCount()));
+                                  Qt::QueuedConnection, Q_ARG(QString, clientModel->getMasternodesCountString()));
         QMetaObject::invokeMethod(this, "setNumBlocks",
                                   Qt::QueuedConnection, Q_ARG(int, clientModel->getLastBlockProcessedHeight()));
     }