Merge pull request #2716 from jamescowens/implement_poll_expiration_r…

…eminders

gui, poll: Implement poll expiration reminders

src/gridcoin/voting/result.cpp

@@ -864,18 +864,28 @@ class VoteCounter
     {
         CTransaction tx;
 
-        if (!m_txdb.ReadDiskTx(txid, tx)) {
-            LogPrint(LogFlags::VOTE, "%s: failed to read vote tx", __func__);
-            throw InvalidVoteError();
+        {
+            // This lock is taken here to ensure that we wait on the leveldb batch write ("transaction commit") to finish
+            // in ReorganizeChain (which is essentially the ConnectBlock scope) and ensure that the voting transactions
+            // which correspond to the new vote signals sent from the contract handlers are actually present in leveldb when
+            // the below ReadDiskTx is called.
+            LOCK(cs_tx_val_commit_to_disk);
+            LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk locked", __func__);
+
+            if (!m_txdb.ReadDiskTx(txid, tx)) {
+                LogPrintf("WARN: %s: failed to read vote tx.", __func__);
+            }
+
+            LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk unlocked", __func__);
         }
 
         if (tx.nTime < m_poll.m_timestamp) {
-            LogPrint(LogFlags::VOTE, "%s: tx earlier than poll", __func__);
+            LogPrintf("WARN: %s: tx earlier than poll", __func__);
             throw InvalidVoteError();
         }
 
         if (m_poll.Expired(tx.nTime)) {
-            LogPrint(LogFlags::VOTE, "%s: tx exceeds expiration", __func__);
+            LogPrintf("WARN: %s: tx exceeds expiration", __func__);
             throw InvalidVoteError();
         }
 
@@ -885,7 +895,7 @@ class VoteCounter
             }
 
             if (!contract.WellFormed()) {
-                LogPrint(LogFlags::VOTE, "%s: skipped bad contract", __func__);
+                LogPrintf("WARN: %s: skipped bad contract", __func__);
                 continue;
             }
 
@@ -1228,7 +1238,11 @@ void PollResult::TallyVote(VoteDetail detail)
 
     if (detail.m_ismine != ISMINE_NO) {
         m_self_voted = true;
-        m_self_vote_detail = detail;
+
+        m_self_vote_detail.m_amount += detail.m_amount;
+        m_self_vote_detail.m_mining_id = detail.m_mining_id;
+        m_self_vote_detail.m_magnitude = detail.m_magnitude;
+        m_self_vote_detail.m_ismine = detail.m_ismine;
     }
 
     for (const auto& response_pair : detail.m_responses) {
@@ -1238,6 +1252,22 @@ void PollResult::TallyVote(VoteDetail detail)
         m_responses[response_offset].m_weight += response_weight;
         m_responses[response_offset].m_votes += 1.0 / detail.m_responses.size();
         m_total_weight += response_weight;
+
+        if (detail.m_ismine != ISMINE_NO) {
+            bool choice_found = false;
+
+            for (auto& choice : m_self_vote_detail.m_responses) {
+                if (choice.first == response_offset) {
+                    choice.second += response_weight;
+                    choice_found = true;
+                    break;
+                }
+            }
+
+            if (!choice_found) {
+                m_self_vote_detail.m_responses.push_back(std::make_pair(response_offset, response_weight));
+            }
+        }
     }
 
     m_votes.emplace_back(std::move(detail));
@@ -1259,6 +1289,15 @@ VoteDetail::VoteDetail() : m_amount(0), m_magnitude(Magnitude::Zero()), m_ismine
 {
 }
 
+VoteDetail::VoteDetail(const VoteDetail &original_votedetail)
+    : m_amount(original_votedetail.m_amount)
+      , m_mining_id(original_votedetail.m_mining_id)
+      , m_magnitude(original_votedetail.m_magnitude)
+      , m_ismine(original_votedetail.m_ismine)
+      , m_responses(original_votedetail.m_responses)
+{
+}
+
 bool VoteDetail::Empty() const
 {
     return m_amount == 0 && m_magnitude == 0;

src/gridcoin/voting/result.h

@@ -71,6 +71,13 @@ class PollResult
         //!
         VoteDetail();
 
+        //!
+        //! \brief User copy constructor.
+        //!
+        //! \param original_votedetail
+        //!
+        VoteDetail(const VoteDetail& original_votedetail);
+
         //!
         //! \brief Determine whether a vote contributes no weight.
         //!

src/main.cpp

@@ -62,6 +62,7 @@ CCriticalSection cs_setpwalletRegistered;
 set<CWallet*> setpwalletRegistered;
 
 CCriticalSection cs_main;
+CCriticalSection cs_tx_val_commit_to_disk;
 
 CTxMemPool mempool;
 
@@ -1155,84 +1156,96 @@ EXCLUSIVE_LOCKS_REQUIRED(cs_main)
             return error("%s: TxnBegin failed", __func__);
         }
 
-        if (pindexGenesisBlock == nullptr) {
-            if (hash != (!fTestNet ? hashGenesisBlock : hashGenesisBlockTestNet)) {
-                txdb.TxnAbort();
-                return error("%s: genesis block hash does not match", __func__);
-            }
-
-            pindexGenesisBlock = pindex;
-        } else {
-            assert(pindex->GetBlockHash()==block.GetHash(true));
-            assert(pindex->pprev == pindexBest);
+        {
+            // This lock protects the time period between the GridcoinConnectBlock, which also connects validated transaction
+            // contracts and causes contract handlers to fire, and the committing of the txindex changes to disk. Any contract
+            // handlers that generate signals whose downstream handlers make use of transaction data on disk via leveldb (txdb)
+            // on another thread need to take this lock to ensure that the write to leveldb and the access of the transaction data
+            // by the signal handlers is appropriately serialized.
+            LOCK(cs_tx_val_commit_to_disk);
+            LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk locked", __func__);
+
+            if (pindexGenesisBlock == nullptr) {
+                if (hash != (!fTestNet ? hashGenesisBlock : hashGenesisBlockTestNet)) {
+                    txdb.TxnAbort();
+                    return error("%s: genesis block hash does not match", __func__);
+                }
 
-            if (!ConnectBlock(block, txdb, pindex, false)) {
-                txdb.TxnAbort();
-                error("%s: ConnectBlock %s failed, Previous block %s",
-                      __func__,
-                      hash.ToString().c_str(),
-                      pindex->pprev->GetBlockHash().ToString());
-                InvalidChainFound(pindex);
-                return false;
+                pindexGenesisBlock = pindex;
+            } else {
+                assert(pindex->GetBlockHash()==block.GetHash(true));
+                assert(pindex->pprev == pindexBest);
+
+                if (!ConnectBlock(block, txdb, pindex, false)) {
+                    txdb.TxnAbort();
+                    error("%s: ConnectBlock %s failed, Previous block %s",
+                          __func__,
+                          hash.ToString().c_str(),
+                          pindex->pprev->GetBlockHash().ToString());
+                    InvalidChainFound(pindex);
+                    return false;
+                }
             }
-        }
 
-        // Delete redundant memory transactions
-        for (auto const& tx : block.vtx) {
-            mempool.remove(tx);
-            mempool.removeConflicts(tx);
-        }
+            // Delete redundant memory transactions
+            for (auto const& tx : block.vtx) {
+                mempool.remove(tx);
+                mempool.removeConflicts(tx);
+            }
 
-        // Remove stale MRCs in the mempool that are not in this new block. Remember the MRCs were initially validated in
-        // AcceptToMemoryPool. Here we just need to do a staleness check.
-        std::vector<CTransaction> to_be_erased;
+            // Remove stale MRCs in the mempool that are not in this new block. Remember the MRCs were initially validated in
+            // AcceptToMemoryPool. Here we just need to do a staleness check.
+            std::vector<CTransaction> to_be_erased;
 
-        for (const auto& [_, pool_tx] : mempool.mapTx) {
-            for (const auto& pool_tx_contract : pool_tx.GetContracts()) {
-                if (pool_tx_contract.m_type == GRC::ContractType::MRC) {
-                    GRC::MRC pool_tx_mrc = pool_tx_contract.CopyPayloadAs<GRC::MRC>();
+            for (const auto& [_, pool_tx] : mempool.mapTx) {
+                for (const auto& pool_tx_contract : pool_tx.GetContracts()) {
+                    if (pool_tx_contract.m_type == GRC::ContractType::MRC) {
+                        GRC::MRC pool_tx_mrc = pool_tx_contract.CopyPayloadAs<GRC::MRC>();
 
-                    if (pool_tx_mrc.m_last_block_hash != hashBestChain) {
-                        to_be_erased.push_back(pool_tx);
+                        if (pool_tx_mrc.m_last_block_hash != hashBestChain) {
+                            to_be_erased.push_back(pool_tx);
+                        }
                     }
                 }
             }
-        }
 
-        // TODO: Additional mempool removals for generic transactions based on txns...
-        // that satisfy lock time requirements,
-        // that are at least 30m old,
-        // that have been broadcast at least once min 5m ago,
-        // that had at least 45s to go in to the last block,
-        // and are still not in the txdb? (for the wallet itself, not mempool.)
-
-        for (const auto& tx : to_be_erased) {
-            LogPrintf("%s: Erasing stale transaction %s from mempool and wallet.", __func__, tx.GetHash().ToString());
-            mempool.remove(tx);
-            // If this transaction was in this wallet (i.e. erasure successful), then send signal for GUI.
-            if (pwalletMain->EraseFromWallet(tx.GetHash())) {
-                pwalletMain->NotifyTransactionChanged(pwalletMain, tx.GetHash(), CT_DELETED);
+            // TODO: Additional mempool removals for generic transactions based on txns...
+            // that satisfy lock time requirements,
+            // that are at least 30m old,
+            // that have been broadcast at least once min 5m ago,
+            // that had at least 45s to go in to the last block,
+            // and are still not in the txdb? (for the wallet itself, not mempool.)
+
+            for (const auto& tx : to_be_erased) {
+                LogPrintf("%s: Erasing stale transaction %s from mempool and wallet.", __func__, tx.GetHash().ToString());
+                mempool.remove(tx);
+                // If this transaction was in this wallet (i.e. erasure successful), then send signal for GUI.
+                if (pwalletMain->EraseFromWallet(tx.GetHash())) {
+                    pwalletMain->NotifyTransactionChanged(pwalletMain, tx.GetHash(), CT_DELETED);
+                }
             }
-        }
 
-        // Clean up spent outputs in wallet that are now not spent if mempool transactions erased above. This
-        // is ugly and heavyweight and should be replaced when the upstream wallet code is ported. Unlike the
-        // repairwallet rpc, this is silent.
-        if (!to_be_erased.empty()) {
-            int nMisMatchFound = 0;
-            CAmount nBalanceInQuestion = 0;
+            // Clean up spent outputs in wallet that are now not spent if mempool transactions erased above. This
+            // is ugly and heavyweight and should be replaced when the upstream wallet code is ported. Unlike the
+            // repairwallet rpc, this is silent.
+            if (!to_be_erased.empty()) {
+                int nMisMatchFound = 0;
+                CAmount nBalanceInQuestion = 0;
 
-            pwalletMain->FixSpentCoins(nMisMatchFound, nBalanceInQuestion);
-        }
+                pwalletMain->FixSpentCoins(nMisMatchFound, nBalanceInQuestion);
+            }
 
-        if (!txdb.WriteHashBestChain(pindex->GetBlockHash())) {
-            txdb.TxnAbort();
-            return error("%s: WriteHashBestChain failed", __func__);
-        }
+            if (!txdb.WriteHashBestChain(pindex->GetBlockHash())) {
+                txdb.TxnAbort();
+                return error("%s: WriteHashBestChain failed", __func__);
+            }
+
+            // Make sure it's successfully written to disk before changing memory structure
+            if (!txdb.TxnCommit()) {
+                return error("%s: TxnCommit failed", __func__);
+            }
 
-        // Make sure it's successfully written to disk before changing memory structure
-        if (!txdb.TxnCommit()) {
-            return error("%s: TxnCommit failed", __func__);
+            LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk unlocked", __func__);
         }
 
         // Add to current best branch

src/main.h

@@ -75,6 +75,7 @@ typedef std::unordered_map<uint256, CBlockIndex*, BlockHasher> BlockMap;
 
 extern CScript COINBASE_FLAGS;
 extern CCriticalSection cs_main;
+extern CCriticalSection cs_tx_val_commit_to_disk;
 extern BlockMap mapBlockIndex;
 extern CBlockIndex* pindexGenesisBlock;
 extern unsigned int nStakeMinAge;

src/qt/bitcoingui.cpp

@@ -20,6 +20,7 @@
 #include "signverifymessagedialog.h"
 #include "optionsdialog.h"
 #include "aboutdialog.h"
+#include "voting/polltab.h"
 #include "voting/votingpage.h"
 #include "clientmodel.h"
 #include "walletmodel.h"
@@ -43,6 +44,7 @@
 #include "univalue.h"
 #include "upgradeqt.h"
 #include "voting/votingmodel.h"
+#include "voting/polltablemodel.h"
 
 #ifdef Q_OS_MAC
 #include "macdockiconhandler.h"
@@ -1932,18 +1934,56 @@ void BitcoinGUI::handleNewPoll()
     overviewPage->setCurrentPollTitle(votingModel->getCurrentPollTitle());
 }
 
+//!
+//! \brief BitcoinGUI::extracted. Helper function to avoid container detach on range loop warning.
+//! \param expiring_polls
+//! \param notification
+//!
+void BitcoinGUI::extracted(QStringList& expiring_polls, QString& notification)
+{
+    for (const auto& expiring_poll : expiring_polls) {
+        notification += expiring_poll + "\n";
+    }
+}
+
 void BitcoinGUI::handleExpiredPoll()
 {
-    // The only difference between this and handleNewPoll() is no call to the event notifier.
+    if (!clientModel) {
+        return;
+    }
 
-    if (!clientModel || !clientModel->getOptionsModel()) {
+    if (!clientModel->getOptionsModel()) {
         return;
     }
 
     if (!votingModel) {
         return;
     }
 
+    // Only do if in sync.
+    if (researcherModel && !researcherModel->outOfSync() && votingPage->getActiveTab()) {
+
+        // First refresh the active poll tab and underlying table
+        votingPage->getActiveTab()->refresh();
+
+        if (!clientModel->getOptionsModel()->getDisablePollNotifications()) {
+            QStringList expiring_polls = votingModel->getExpiringPollsNotNotified();
+
+            if (!expiring_polls.isEmpty()) {
+                QString notification = tr("The following poll(s) are about to expire:\n");
+
+                extracted(expiring_polls, notification);
+
+                notification += tr("Open Gridcoin to vote.");
+
+                notificator->notify(
+                    Notificator::Information,
+                    tr("Poll(s) about to expire"),
+                    notification);
+            }
+        }
+    }
+
     overviewPage->setCurrentPollTitle(votingModel->getCurrentPollTitle());
 }
 

src/qt/bitcoingui.h

@@ -20,6 +20,7 @@ class WalletModel;
 class ResearcherModel;
 class MRCModel;
 class VotingModel;
+class PollTableModel;
 class TransactionView;
 class OverviewPage;
 class FavoritesPage;
@@ -295,6 +296,7 @@ private slots:
     QString GetEstimatedStakingFrequency(unsigned int nEstimateTime);
 
     void handleNewPoll();
+    void extracted(QStringList& expiring_polls, QString& notification);
     void handleExpiredPoll();
 };