From a8929500af7eab05356c01c9cde0a11eb5a6aada Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 19 Nov 2023 12:49:39 -0500 Subject: [PATCH 01/10] Implement GUI machinery for poll expiry notification --- src/qt/bitcoingui.cpp | 31 ++++++++++++- src/qt/bitcoingui.h | 1 + src/qt/forms/optionsdialog.ui | 33 +++++++++++++- src/qt/optionsdialog.cpp | 86 +++++++++++++++++++++++++++++++---- src/qt/optionsdialog.h | 4 ++ src/qt/optionsmodel.cpp | 12 +++++ src/qt/optionsmodel.h | 5 +- src/qt/voting/votingmodel.cpp | 30 +++++++++++- src/qt/voting/votingmodel.h | 17 +++++++ 9 files changed, 205 insertions(+), 14 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 60444b4fe7..bd0b96456f 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -1932,10 +1932,20 @@ void BitcoinGUI::handleNewPoll() overviewPage->setCurrentPollTitle(votingModel->getCurrentPollTitle()); } -void BitcoinGUI::handleExpiredPoll() +//! +//! \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) { - // The only difference between this and handleNewPoll() is no call to the event notifier. + for (const auto& expiring_poll : expiring_polls) { + notification += expiring_poll + "\n"; + } +} +void BitcoinGUI::handleExpiredPoll() +{ if (!clientModel || !clientModel->getOptionsModel()) { return; } @@ -1944,6 +1954,23 @@ void BitcoinGUI::handleExpiredPoll() return; } + 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()); } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index eec17f954f..adbd9216e9 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -295,6 +295,7 @@ private slots: QString GetEstimatedStakingFrequency(unsigned int nEstimateTime); void handleNewPoll(); + void extracted(QStringList& expiring_polls, QString& notification); void handleExpiredPoll(); }; diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index ee3632e12c..9e95f8c909 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -369,6 +369,37 @@ + + + + + + Hours before poll expiry reminder + + + + + + + Valid values are between 0.25 and 24.0 hours. + + + + + + + Qt::Horizontal + + + + 80 + 20 + + + + + + @@ -585,7 +616,7 @@ QValidatedLineEdit QLineEdit -
qvalidatedlineedit.h
+
qvalidatedlineedit.h
 QValueComboBox diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 697c489eaf..316f1e4a58 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -1,4 +1,5 @@ #include "optionsdialog.h" +#include "qevent.h" #include "ui_optionsdialog.h" #include "netbase.h" @@ -16,13 +17,16 @@ #include OptionsDialog::OptionsDialog(QWidget* parent) - : QDialog(parent) - , ui(new Ui::OptionsDialog) - , model(nullptr) - , mapper(nullptr) - , fRestartWarningDisplayed_Proxy(false) - , fRestartWarningDisplayed_Lang(false) - , fProxyIpValid(true) + : QDialog(parent) + , ui(new Ui::OptionsDialog) + , model(nullptr) + , mapper(nullptr) + , fRestartWarningDisplayed_Proxy(false) + , fRestartWarningDisplayed_Lang(false) + , fProxyIpValid(true) + , fStakingEfficiencyValid(true) + , fMinStakeSplitValueValid(true) + , fPollExpireNotifyValid(true) { ui->setupUi(this); @@ -44,6 +48,7 @@ OptionsDialog::OptionsDialog(QWidget* parent) ui->proxyIp->installEventFilter(this); ui->stakingEfficiency->installEventFilter(this); ui->minPostSplitOutputValue->installEventFilter(this); + ui->pollExpireNotifyLineEdit->installEventFilter(this); /* Window elements init */ #ifdef Q_OS_MAC @@ -103,17 +108,24 @@ OptionsDialog::OptionsDialog(QWidget* parent) connect(this, &OptionsDialog::proxyIpValid, this, &OptionsDialog::handleProxyIpValid); connect(this, &OptionsDialog::stakingEfficiencyValid, this, &OptionsDialog::handleStakingEfficiencyValid); connect(this, &OptionsDialog::minStakeSplitValueValid, this, &OptionsDialog::handleMinStakeSplitValueValid); + /** setup/change UI elements when poll expiry notification time window is valid/invalid */ + connect(this, &OptionsDialog::pollExpireNotifyValid, this, &OptionsDialog::handlePollExpireNotifyValid); if (fTestNet) ui->disableUpdateCheck->setHidden(true); ui->gridcoinAtStartupMinimised->setHidden(!ui->gridcoinAtStartup->isChecked()); ui->limitTxnDisplayDateEdit->setHidden(!ui->limitTxnDisplayCheckBox->isChecked()); + ui->pollExpireNotifyLabel->setHidden(ui->disablePollNotifications->isChecked()); + ui->pollExpireNotifyLineEdit->setHidden(ui->disablePollNotifications->isChecked()); + connect(ui->gridcoinAtStartup, &QCheckBox::toggled, this, &OptionsDialog::hideStartMinimized); connect(ui->gridcoinAtStartupMinimised, &QCheckBox::toggled, this, &OptionsDialog::hideStartMinimized); connect(ui->limitTxnDisplayCheckBox, &QCheckBox::toggled, this, &OptionsDialog::hideLimitTxnDisplayDate); + connect(ui->disablePollNotifications, &QCheckBox::toggled, this , &OptionsDialog::hidePollExpireNotify); + bool stake_split_enabled = ui->enableStakeSplit->isChecked(); ui->stakingEfficiencyLabel->setHidden(!stake_split_enabled); @@ -180,6 +192,7 @@ void OptionsDialog::setMapper() /* Window */ mapper->addMapping(ui->disableTransactionNotifications, OptionsModel::DisableTrxNotifications); mapper->addMapping(ui->disablePollNotifications, OptionsModel::DisablePollNotifications); + mapper->addMapping(ui->pollExpireNotifyLineEdit, OptionsModel::PollExpireNotification); #ifndef Q_OS_MAC if (QSystemTrayIcon::isSystemTrayAvailable()) { mapper->addMapping(ui->minimizeToTray, OptionsModel::MinimizeToTray); @@ -194,7 +207,7 @@ void OptionsDialog::setMapper() mapper->addMapping(ui->styleComboBox, OptionsModel::WalletStylesheet,"currentData"); mapper->addMapping(ui->limitTxnDisplayCheckBox, OptionsModel::LimitTxnDisplay); mapper->addMapping(ui->limitTxnDisplayDateEdit, OptionsModel::LimitTxnDate); - mapper->addMapping(ui->displayAddresses, OptionsModel::DisplayAddresses); + mapper->addMapping(ui->displayAddresses, OptionsModel::DisplayAddresses); } void OptionsDialog::enableApplyButton() @@ -298,6 +311,14 @@ void OptionsDialog::hideLimitTxnDisplayDate() } } +void OptionsDialog::hidePollExpireNotify() +{ + if (model) { + ui->pollExpireNotifyLabel->setHidden(ui->disablePollNotifications->isChecked()); + ui->pollExpireNotifyLineEdit->setHidden(ui->disablePollNotifications->isChecked()); + } +} + void OptionsDialog::hideStakeSplitting() { if (model) @@ -368,9 +389,40 @@ void OptionsDialog::handleMinStakeSplitValueValid(QValidatedLineEdit *object, bo } } +void OptionsDialog::handlePollExpireNotifyValid(QValidatedLineEdit *object, bool fState) +{ + // this is used in a check before re-enabling the save buttons + fPollExpireNotifyValid = fState; + + if (fPollExpireNotifyValid) { + enableSaveButtons(); + ui->statusLabel->clear(); + } else { + disableSaveButtons(); + object->setValid(fPollExpireNotifyValid); + ui->statusLabel->setStyleSheet("QLabel { color: red; }"); + ui->statusLabel->setText(tr("The supplied time for notification before poll expires must " + "be between 0.25 and 24 hours.")); + } +} + bool OptionsDialog::eventFilter(QObject *object, QEvent *event) { - if (event->type() == QEvent::FocusOut) + bool filter_event = false; + + if (event->type() == QEvent::FocusOut) { + filter_event = true; + } + + if (event->type() == QEvent::KeyPress) { + QKeyEvent *keyEvent = static_cast(event); + + if (keyEvent->key() == Qt::Key_Enter || keyEvent->key() == Qt::Key_Return) { + filter_event = true; + } + } + + if (filter_event) { if (object == ui->proxyIp) { @@ -423,6 +475,22 @@ bool OptionsDialog::eventFilter(QObject *object, QEvent *event) } } } + + if (object == ui->pollExpireNotifyLineEdit) { + bool ok = false; + double hours = ui->pollExpireNotifyLineEdit->text().toDouble(&ok); + + if (!ok) { + emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, false); + } else { + if (hours >= 0.25 && hours <= 24.0) { + emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, true); + } else { + emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, false); + } + } + } } + return QDialog::eventFilter(object, event); } diff --git a/src/qt/optionsdialog.h b/src/qt/optionsdialog.h index 1687c00f72..ae7d2adf6e 100644 --- a/src/qt/optionsdialog.h +++ b/src/qt/optionsdialog.h @@ -47,14 +47,17 @@ private slots: void hideStartMinimized(); void hideLimitTxnDisplayDate(); void hideStakeSplitting(); + void hidePollExpireNotify(); void handleProxyIpValid(QValidatedLineEdit *object, bool fState); void handleStakingEfficiencyValid(QValidatedLineEdit *object, bool fState); void handleMinStakeSplitValueValid(QValidatedLineEdit *object, bool fState); + void handlePollExpireNotifyValid(QValidatedLineEdit *object, bool fState); signals: void proxyIpValid(QValidatedLineEdit *object, bool fValid); void stakingEfficiencyValid(QValidatedLineEdit *object, bool fValid); void minStakeSplitValueValid(QValidatedLineEdit *object, bool fValid); + void pollExpireNotifyValid(QValidatedLineEdit *object, bool fValid); private: Ui::OptionsDialog *ui; @@ -65,6 +68,7 @@ private slots: bool fProxyIpValid; bool fStakingEfficiencyValid; bool fMinStakeSplitValueValid; + bool fPollExpireNotifyValid; }; #endif // BITCOIN_QT_OPTIONSDIALOG_H diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index f435c2369f..ce76357a0b 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -57,6 +57,7 @@ void OptionsModel::Init() fLimitTxnDisplay = settings.value("fLimitTxnDisplay", false).toBool(); fMaskValues = settings.value("fMaskValues", false).toBool(); limitTxnDate = settings.value("limitTxnDate", QDate()).toDate(); + pollExpireNotification = settings.value("pollExpireNotification", 1.0).toDouble(); nReserveBalance = settings.value("nReserveBalance").toLongLong(); language = settings.value("language", "").toString(); walletStylesheet = settings.value("walletStylesheet", "dark").toString(); @@ -142,6 +143,8 @@ QVariant OptionsModel::data(const QModelIndex & index, int role) const return QVariant(fMaskValues); case LimitTxnDate: return QVariant(limitTxnDate); + case PollExpireNotification: + return QVariant(pollExpireNotification); case DisableUpdateCheck: return QVariant(gArgs.GetBoolArg("-disableupdatecheck", false)); case DataDir: @@ -284,6 +287,10 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in limitTxnDate = value.toDate(); settings.setValue("limitTxnDate", limitTxnDate); break; + case PollExpireNotification: + pollExpireNotification = value.toDouble(); + settings.setValue("pollExpireNotification", pollExpireNotification); + break; case DisableUpdateCheck: gArgs.ForceSetArg("-disableupdatecheck", value.toBool() ? "1" : "0"); settings.setValue("fDisableUpdateCheck", value.toBool()); @@ -380,6 +387,11 @@ int64_t OptionsModel::getLimitTxnDateTime() return limitTxnDateTime.toMSecsSinceEpoch() / 1000; } +double OptionsModel::getPollExpireNotification() +{ + return pollExpireNotification; +} + bool OptionsModel::getStartAtStartup() { return fStartAtStartup; diff --git a/src/qt/optionsmodel.h b/src/qt/optionsmodel.h index f9d94bc834..d80009e66f 100644 --- a/src/qt/optionsmodel.h +++ b/src/qt/optionsmodel.h @@ -43,6 +43,7 @@ class OptionsModel : public QAbstractListModel EnableStakeSplit, // bool StakingEfficiency, // double MinStakeSplitValue, // int + PollExpireNotification, // double ContractChangeToInput, // bool MaskValues, // bool OptionIDRowCount @@ -71,6 +72,7 @@ class OptionsModel : public QAbstractListModel bool getMaskValues(); QDate getLimitTxnDate(); int64_t getLimitTxnDateTime(); + double getPollExpireNotification(); QString getLanguage() { return language; } QString getCurrentStyle(); QString getDataDir(); @@ -87,13 +89,14 @@ class OptionsModel : public QAbstractListModel bool fStartMin; bool fDisableTrxNotifications; bool fDisablePollNotifications; - bool bDisplayAddresses; + bool bDisplayAddresses; bool fMinimizeOnClose; bool fConfirmOnClose; bool fCoinControlFeatures; bool fLimitTxnDisplay; bool fMaskValues; QDate limitTxnDate; + double pollExpireNotification; QString language; QString walletStylesheet; QString dataDir; diff --git a/src/qt/voting/votingmodel.cpp b/src/qt/voting/votingmodel.cpp index 769374f64d..ffdc671de0 100644 --- a/src/qt/voting/votingmodel.cpp +++ b/src/qt/voting/votingmodel.cpp @@ -15,6 +15,7 @@ #include "gridcoin/voting/payloads.h" #include "logging.h" #include "main.h" +#include "optionsmodel.h" #include "qt/clientmodel.h" #include "qt/voting/votingmodel.h" #include "qt/walletmodel.h" @@ -159,6 +160,8 @@ VotingModel::VotingModel( m_last_poll_time = std::max(m_last_poll_time, iter->Ref().Time()); } } + + m_poll_expire_warning = static_cast(m_options_model.getPollExpireNotification() * 3600.0 * 1000.0); } VotingModel::~VotingModel() @@ -248,7 +251,25 @@ QStringList VotingModel::getActiveProjectUrls() const } return Urls; +} + +QStringList VotingModel::getExpiringPollsNotNotified() +{ + QStringList expiring_polls; + + QDateTime now = QDateTime::fromMSecsSinceEpoch(GetAdjustedTime() * 1000); + + // Populate the list and mark the poll items included in the list m_expire_notified true. + for (auto& poll : m_pollitems) { + if (now.msecsTo(poll.second.m_expiration) <= m_poll_expire_warning + && !poll.second.m_expire_notified + && !poll.second.m_self_voted) { + expiring_polls << poll.second.m_title; + poll.second.m_expire_notified = true; + } + } + return expiring_polls; } std::vector VotingModel::buildPollTable(const PollFilterFlag flags) @@ -271,6 +292,7 @@ std::vector VotingModel::buildPollTable(const PollFilterFlag flags) // poll item into the results and move on. bool pollitem_needs_rebuild = true; + bool pollitem_expire_notified = false; auto pollitems_iter = m_pollitems.find(iter->Ref().Txid()); // Note that the NewVoteReceived core signal will also be fired during reorgs where votes are reverted, @@ -281,6 +303,10 @@ std::vector VotingModel::buildPollTable(const PollFilterFlag flags) // Not stale... the cache entry is good. Insert into items to return and go to the next one. items.push_back(pollitems_iter->second); pollitem_needs_rebuild = false; + } else { + // Retain state for expire notification in the case of a stale poll item that needs to be + // refreshed. + pollitem_expire_notified = pollitems_iter->second.m_expire_notified; } } @@ -302,7 +328,9 @@ std::vector VotingModel::buildPollTable(const PollFilterFlag flags) try { if (std::optional item = BuildPollItem(iter)) { // This will replace any stale existing entry in the cache with the freshly built item. - // It will also correctly add a new entry for a new item. + // It will also correctly add a new entry for a new item. The state of the pending expiry + // notification is retained from the stale entry to the refreshed one. + item->m_expire_notified = pollitem_expire_notified; m_pollitems[iter->Ref().Txid()] = *item; items.push_back(std::move(*item)); } diff --git a/src/qt/voting/votingmodel.h b/src/qt/voting/votingmodel.h index 68896d44c0..e489e967f6 100644 --- a/src/qt/voting/votingmodel.h +++ b/src/qt/voting/votingmodel.h @@ -88,6 +88,7 @@ class PollItem GRC::PollResult::VoteDetail m_self_vote_detail; bool m_stale = true; + bool m_expire_notified = false; }; //! @@ -134,6 +135,20 @@ class VotingModel : public QObject QString getCurrentPollTitle() const; QStringList getActiveProjectNames() const; QStringList getActiveProjectUrls() const; + + //! + //! \brief getExpiringPollsNotNotified. This method populates a QStringList with + //! the polls in the pollitems cache that are within the m_poll_expire_warning window + //! and which have not previously been notified to the user. Since this method is + //! to be used to have the GUI immediately provide notification to the user, it also + //! marks each of the polls in the QStringList m_expire_notified = true so that they + //! will not appear again on this list (unless the wallet is restarted). This accomplishes + //! a single shot notification for each poll that is about to expire. + //! + //! \return QStringList of polls that are about to expire (within m_poll_expire_warning of + //! expiration), and which have not previously been included on the list (i.e. notified). + //! + QStringList getExpiringPollsNotNotified(); std::vector buildPollTable(const GRC::PollFilterFlag flags); CAmount estimatePollFee() const; @@ -158,6 +173,8 @@ class VotingModel : public QObject void newVoteReceived(QString poll_txid_string); private: + qint64 m_poll_expire_warning; + GRC::PollRegistry& m_registry; ClientModel& m_client_model; OptionsModel& m_options_model; From ef0f716a34ad44dfd889ee954e5e8462ba28448d Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 19 Nov 2023 23:50:59 -0500 Subject: [PATCH 02/10] Wire up Poll notification GUI machinery to signals Also address wallet restart with existing current polls. --- src/gridcoin/voting/result.cpp | 39 ++++++++++++++++++++++++++++---- src/gridcoin/voting/result.h | 7 ++++++ src/qt/bitcoingui.cpp | 35 +++++++++++++++++++--------- src/qt/bitcoingui.h | 1 + src/qt/voting/pollcardview.cpp | 10 ++++---- src/qt/voting/pollcardview.h | 2 +- src/qt/voting/polltab.cpp | 39 ++++++++++++++++++-------------- src/qt/voting/polltab.h | 5 +++- src/qt/voting/polltablemodel.cpp | 14 ++++++++---- src/qt/voting/polltablemodel.h | 5 +++- src/qt/voting/votingpage.cpp | 13 +++++++++++ src/qt/voting/votingpage.h | 1 + 12 files changed, 126 insertions(+), 45 deletions(-) diff --git a/src/gridcoin/voting/result.cpp b/src/gridcoin/voting/result.cpp index 1cdd746eff..98a8b72dee 100644 --- a/src/gridcoin/voting/result.cpp +++ b/src/gridcoin/voting/result.cpp @@ -864,18 +864,38 @@ class VoteCounter { CTransaction tx; - if (!m_txdb.ReadDiskTx(txid, tx)) { - LogPrint(LogFlags::VOTE, "%s: failed to read vote tx", __func__); + bool read_tx_success = false; + + // This is very ugly. In testing for implement poll expiration reminders PR2716, there is an issue with ReadDiskTx + // on very fast machines, where upon receipt of a vote on an existing poll, the poll builder tests for the transaction + // BEFORE it is committed to disk. This retry loop is essentially zero overhead for an immediate success, but does + // up to 10 tries over up to 2.5 seconds total to "wait" for the transaction to appear in leveldb. + for (unsigned int i = 0; i < 10; ++i) { + if (m_txdb.ReadDiskTx(txid, tx)) { + read_tx_success = true; + break; + } else { + LogPrintf("WARN: %s: failed to read vote tx in try %u", __func__, i + 1); + } + + if (!MilliSleep(250)) { + // Interrupt with throw if MilliSleep interrupted by op sys signal. + throw InvalidVoteError(); + } + } + + if (!read_tx_success) { + LogPrintf("WARN: %s: failed to read vote tx after 10 tries", __func__); throw InvalidVoteError(); } 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 +905,7 @@ class VoteCounter } if (!contract.WellFormed()) { - LogPrint(LogFlags::VOTE, "%s: skipped bad contract", __func__); + LogPrintf("WARN: %s: skipped bad contract", __func__); continue; } @@ -1259,6 +1279,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; diff --git a/src/gridcoin/voting/result.h b/src/gridcoin/voting/result.h index 758a7bbcf9..5ca0cda601 100644 --- a/src/gridcoin/voting/result.h +++ b/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. //! diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index bd0b96456f..aa7c86315f 100644 --- a/src/qt/bitcoingui.cpp +++ b/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" @@ -1946,7 +1948,11 @@ void BitcoinGUI::extracted(QStringList& expiring_polls, QString& notification) void BitcoinGUI::handleExpiredPoll() { - if (!clientModel || !clientModel->getOptionsModel()) { + if (!clientModel) { + return; + } + + if (!clientModel->getOptionsModel()) { return; } @@ -1954,20 +1960,27 @@ void BitcoinGUI::handleExpiredPoll() return; } - if (!clientModel->getOptionsModel()->getDisablePollNotifications()) { - QStringList expiring_polls = votingModel->getExpiringPollsNotNotified(); + // Only do if in sync. + if (researcherModel && !researcherModel->outOfSync() && votingPage->getActiveTab()) { + + // First refresh the active poll tab and underlying table + votingPage->getActiveTab()->refresh(); - if (!expiring_polls.isEmpty()) { - QString notification = tr("The following poll(s) are about to expire:\n"); + if (!clientModel->getOptionsModel()->getDisablePollNotifications()) { + QStringList expiring_polls = votingModel->getExpiringPollsNotNotified(); - extracted(expiring_polls, notification); + if (!expiring_polls.isEmpty()) { + QString notification = tr("The following poll(s) are about to expire:\n"); - notification += tr("Open Gridcoin to vote."); + extracted(expiring_polls, notification); - notificator->notify( - Notificator::Information, - tr("Poll(s) about to expire"), - notification); + notification += tr("Open Gridcoin to vote."); + + notificator->notify( + Notificator::Information, + tr("Poll(s) about to expire"), + notification); + } } } diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index adbd9216e9..5b1413cbbf 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -20,6 +20,7 @@ class WalletModel; class ResearcherModel; class MRCModel; class VotingModel; +class PollTableModel; class TransactionView; class OverviewPage; class FavoritesPage; diff --git a/src/qt/voting/pollcardview.cpp b/src/qt/voting/pollcardview.cpp index 6c1e7798b3..c720183851 100644 --- a/src/qt/voting/pollcardview.cpp +++ b/src/qt/voting/pollcardview.cpp @@ -33,7 +33,7 @@ PollCardView::~PollCardView() void PollCardView::setModel(PollTableModel* model) { - m_model = model; + m_polltable_model = model; if (!model) { return; @@ -41,7 +41,7 @@ void PollCardView::setModel(PollTableModel* model) connect(model, &PollTableModel::layoutChanged, this, &PollCardView::redraw); - if (!m_refresh_timer && m_model->includesActivePolls()) { + if (!m_refresh_timer && m_polltable_model->includesActivePolls()) { m_refresh_timer.reset(new QTimer(this)); m_refresh_timer->setTimerType(Qt::VeryCoarseTimer); @@ -76,15 +76,15 @@ void PollCardView::redraw() // sorting and filtering. Hook up model events for these operations. clear(); - if (!m_model) { + if (!m_polltable_model) { return; } const QDateTime now = QDateTime::currentDateTimeUtc(); const QModelIndex dummy_parent; - for (int i = 0; i < m_model->rowCount(dummy_parent); ++i) { - if (const PollItem* poll_item = m_model->rowItem(i)) { + for (int i = 0; i < m_polltable_model->rowCount(dummy_parent); ++i) { + if (const PollItem* poll_item = m_polltable_model->rowItem(i)) { PollCard* card = new PollCard(*poll_item, this); card->updateRemainingTime(now); card->updateIcons(m_theme); diff --git a/src/qt/voting/pollcardview.h b/src/qt/voting/pollcardview.h index d85efa6503..58da557f7f 100644 --- a/src/qt/voting/pollcardview.h +++ b/src/qt/voting/pollcardview.h @@ -43,7 +43,7 @@ public slots: private: Ui::PollCardView* ui; - PollTableModel* m_model; + PollTableModel* m_polltable_model; std::unique_ptr m_refresh_timer; QString m_theme; diff --git a/src/qt/voting/polltab.cpp b/src/qt/voting/polltab.cpp index d08d9b1e26..672ab759ac 100644 --- a/src/qt/voting/polltab.cpp +++ b/src/qt/voting/polltab.cpp @@ -134,7 +134,7 @@ private slots: PollTab::PollTab(QWidget* parent) : QWidget(parent) , ui(new Ui::PollTab) - , m_model(new PollTableModel(this)) + , m_polltable_model(new PollTableModel(this)) , m_no_result(new NoResult(this)) , m_loading(new LoadingBar(this)) { @@ -152,7 +152,12 @@ PollTab::PollTab(QWidget* parent) connect(ui->cards, &PollCardView::detailsRequested, this, &PollTab::showDetailsRowDialog); connect(ui->table, &QAbstractItemView::doubleClicked, this, &PollTab::showPreferredDialog); connect(ui->table, &QWidget::customContextMenuRequested, this, &PollTab::showTableContextMenu); - connect(m_model.get(), &PollTableModel::layoutChanged, this, &PollTab::finishRefresh); + connect(m_polltable_model.get(), &PollTableModel::layoutChanged, this, &PollTab::finishRefresh); + + // Forward the polltable model signal to the Poll Tab signal to avoid having to directly include the PollTableModel + // in the voting page. + connect(m_polltable_model.get(), &PollTableModel::newVoteReceivedAndPollMarkedDirty, + this, &PollTab::newVoteReceivedAndPollMarkedDirty); } PollTab::~PollTab() @@ -163,15 +168,15 @@ PollTab::~PollTab() void PollTab::setVotingModel(VotingModel* model) { m_voting_model = model; - m_model->setModel(model); + m_polltable_model->setModel(model); - ui->cards->setModel(m_model.get()); - ui->table->setModel(m_model.get()); + ui->cards->setModel(m_polltable_model.get()); + ui->table->setModel(m_polltable_model.get()); } void PollTab::setPollFilterFlags(PollFilterFlag flags) { - m_model->setPollFilterFlags(flags); + m_polltable_model->setPollFilterFlags(flags); } void PollTab::changeViewMode(const ViewId view_id) @@ -181,26 +186,26 @@ void PollTab::changeViewMode(const ViewId view_id) void PollTab::refresh() { - if (m_model->empty()) { + if (m_polltable_model->empty()) { m_no_result->showDefaultLoadingTitle(); m_no_result->contentWidgetAs()->setText(WaitMessage()); } m_loading->start(); - m_model->refresh(); + m_polltable_model->refresh(); } void PollTab::filter(const QString& needle) { if (needle != m_last_filter) { - m_model->changeTitleFilter(needle); + m_polltable_model->changeTitleFilter(needle); m_last_filter = needle; } } void PollTab::sort(const int column) { - const Qt::SortOrder order = m_model->sort(column); + const Qt::SortOrder order = m_polltable_model->sort(column); ui->table->horizontalHeader()->setSortIndicator(column, order); } @@ -215,7 +220,7 @@ const PollItem* PollTab::selectedTableItem() const return nullptr; } - return m_model->rowItem( + return m_polltable_model->rowItem( ui->table->selectionModel()->selectedIndexes().first().row()); } @@ -228,10 +233,10 @@ void PollTab::resizeEvent(QResizeEvent* event) void PollTab::finishRefresh() { m_loading->finish(); - ui->stack->setVisible(!m_model->empty()); - m_no_result->setVisible(m_model->empty()); + ui->stack->setVisible(!m_polltable_model->empty()); + m_no_result->setVisible(m_polltable_model->empty()); - if (m_model->empty()) { + if (m_polltable_model->empty()) { m_no_result->showDefaultNoResultTitle(); m_no_result->contentWidgetAs()->setText(FullRefreshMessage()); } @@ -239,7 +244,7 @@ void PollTab::finishRefresh() void PollTab::showVoteRowDialog(int row) { - if (const PollItem* const poll_item = m_model->rowItem(row)) { + if (const PollItem* const poll_item = m_polltable_model->rowItem(row)) { showVoteDialog(*poll_item); } } @@ -251,7 +256,7 @@ void PollTab::showVoteDialog(const PollItem& poll_item) void PollTab::showDetailsRowDialog(int row) { - if (const PollItem* const poll_item = m_model->rowItem(row)) { + if (const PollItem* const poll_item = m_polltable_model->rowItem(row)) { showDetailsDialog(*poll_item); } } @@ -263,7 +268,7 @@ void PollTab::showDetailsDialog(const PollItem& poll_item) void PollTab::showPreferredDialog(const QModelIndex& index) { - if (const PollItem* const poll_item = m_model->rowItem(index.row())) { + if (const PollItem* const poll_item = m_polltable_model->rowItem(index.row())) { if (poll_item->m_finished) { showDetailsDialog(*poll_item); } else { diff --git a/src/qt/voting/polltab.h b/src/qt/voting/polltab.h index 93e68252b4..0034f26682 100644 --- a/src/qt/voting/polltab.h +++ b/src/qt/voting/polltab.h @@ -50,6 +50,9 @@ class PollTab : public QWidget void setVotingModel(VotingModel* voting_model); void setPollFilterFlags(GRC::PollFilterFlag flags); +signals: + void newVoteReceivedAndPollMarkedDirty(); + public slots: void changeViewMode(const ViewId view_id); void refresh(); @@ -60,7 +63,7 @@ public slots: private: Ui::PollTab* ui; VotingModel* m_voting_model; - std::unique_ptr m_model; + std::unique_ptr m_polltable_model; std::unique_ptr m_no_result; std::unique_ptr m_loading; QString m_last_filter; diff --git a/src/qt/voting/polltablemodel.cpp b/src/qt/voting/polltablemodel.cpp index 14f3bbe8ed..e169ae8dbb 100644 --- a/src/qt/voting/polltablemodel.cpp +++ b/src/qt/voting/polltablemodel.cpp @@ -5,6 +5,7 @@ #include "qt/guiutil.h" #include "qt/voting/polltablemodel.h" #include "qt/voting/votingmodel.h" +#include "logging.h" #include #include @@ -212,11 +213,11 @@ PollTableModel::~PollTableModel() void PollTableModel::setModel(VotingModel* model) { - m_model = model; + m_voting_model = model; // Connect poll stale handler to newVoteReceived signal from voting model, which propagates // from the core. - connect(m_model, &VotingModel::newVoteReceived, this, &PollTableModel::handlePollStaleFlag); + connect(m_voting_model, &VotingModel::newVoteReceived, this, &PollTableModel::handlePollStaleFlag); } void PollTableModel::setPollFilterFlags(PollFilterFlag flags) @@ -254,13 +255,16 @@ const PollItem* PollTableModel::rowItem(int row) const void PollTableModel::refresh() { - if (!m_model || !m_refresh_mutex.tryLock()) { + if (!m_voting_model || !m_refresh_mutex.tryLock()) { + LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: m_refresh_mutex is already taken, so tryLock failed", + __func__); + return; } QtConcurrent::run([this]() { static_cast(m_data_model.get()) - ->reload(m_model->buildPollTable(m_filter_flags)); + ->reload(m_voting_model->buildPollTable(m_filter_flags)); m_refresh_mutex.unlock(); }); @@ -269,6 +273,8 @@ void PollTableModel::refresh() void PollTableModel::handlePollStaleFlag(QString poll_txid_string) { m_data_model->handlePollStaleFlag(poll_txid_string); + + emit newVoteReceivedAndPollMarkedDirty(); } void PollTableModel::changeTitleFilter(const QString& pattern) diff --git a/src/qt/voting/polltablemodel.h b/src/qt/voting/polltablemodel.h index 38620d3bab..5f3b1ac406 100644 --- a/src/qt/voting/polltablemodel.h +++ b/src/qt/voting/polltablemodel.h @@ -74,6 +74,9 @@ class PollTableModel : public QSortFilterProxyModel QString columnName(int offset) const; const PollItem* rowItem(int row) const; +signals: + void newVoteReceivedAndPollMarkedDirty(); + public slots: void refresh(); void changeTitleFilter(const QString& pattern); @@ -82,7 +85,7 @@ public slots: void handlePollStaleFlag(QString poll_txid_string); private: - VotingModel* m_model; + VotingModel* m_voting_model; std::unique_ptr m_data_model; GRC::PollFilterFlag m_filter_flags; QMutex m_refresh_mutex; diff --git a/src/qt/voting/votingpage.cpp b/src/qt/voting/votingpage.cpp index a592ee2576..dc2e22403e 100644 --- a/src/qt/voting/votingpage.cpp +++ b/src/qt/voting/votingpage.cpp @@ -129,8 +129,17 @@ void VotingPage::setVotingModel(VotingModel* model) return; } + // Now that PollItem caching is available, automatically refresh current poll tab on receipt of new poll or vote. connect(model, &VotingModel::newPollReceived, [this]() { ui->pollReceivedLabel->show(); + getActiveTab()->refresh(); + ui->pollReceivedLabel->hide(); + }); + + // Using the newVoteReceivedAndPollMarkedDirty instead of newVoteReceived insures the poll staleness flag in the appropriate + // poll item has been marked dirty before the refresh is called. + connect(getActiveTab(), &PollTab::newVoteReceivedAndPollMarkedDirty, [this]() { + getActiveTab()->refresh(); }); } @@ -149,6 +158,10 @@ PollTab& VotingPage::currentTab() return *qobject_cast(ui->tabWidget->currentWidget()); } +PollTab* VotingPage::getActiveTab() +{ + return m_tabs[0]; +} void VotingPage::updateIcons(const QString& theme) { m_filter_action->setIcon(QIcon(":/icons/" + theme + "_search")); diff --git a/src/qt/voting/votingpage.h b/src/qt/voting/votingpage.h index 93e7ce4b65..9ac6ab8b5e 100644 --- a/src/qt/voting/votingpage.h +++ b/src/qt/voting/votingpage.h @@ -35,6 +35,7 @@ class VotingPage : public QWidget void setOptionsModel(OptionsModel* model); PollTab& currentTab(); + PollTab* getActiveTab(); private: Ui::VotingPage* ui; From 44e832cded807e3d0188821109982dbacfffec5b Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 25 Nov 2023 01:11:53 -0500 Subject: [PATCH 03/10] Correct vote display in poll card for walletholder vote(s) The handling for last vote did not really work correctly in the case where the walletholder votes multiple times. Because of the way the voting system handles duplicate outputs in the vote tallying, using the last vote only does not necessarily contain the entire weight or all the choices for the walletholder. This commit implements showing each choice that has weight and the weight on each one. --- src/gridcoin/voting/result.cpp | 22 +++++++++++++++++++++- src/qt/forms/voting/pollcard.ui | 4 ++-- src/qt/voting/pollcard.cpp | 9 ++++++++- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/gridcoin/voting/result.cpp b/src/gridcoin/voting/result.cpp index 98a8b72dee..ef523a2cd3 100644 --- a/src/gridcoin/voting/result.cpp +++ b/src/gridcoin/voting/result.cpp @@ -1248,7 +1248,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) { @@ -1258,6 +1262,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)); diff --git a/src/qt/forms/voting/pollcard.ui b/src/qt/forms/voting/pollcard.ui index 2bad4f8d81..10312b84d6 100644 --- a/src/qt/forms/voting/pollcard.ui +++ b/src/qt/forms/voting/pollcard.ui @@ -201,14 +201,14 @@ - Your Last Vote: + Your Vote(s): - Your Vote Weight: + Your Vote Weight(s): diff --git a/src/qt/voting/pollcard.cpp b/src/qt/voting/pollcard.cpp index 6680209bdd..1071192d00 100644 --- a/src/qt/voting/pollcard.cpp +++ b/src/qt/voting/pollcard.cpp @@ -39,6 +39,7 @@ PollCard::PollCard(const PollItem& poll_item, QWidget* parent) ui->myPercentAVWLabel->setText("N/A"); } else { QString choices_str; + QString weights_str; int64_t my_total_weight = 0; @@ -49,11 +50,17 @@ PollCard::PollCard(const PollItem& poll_item, QWidget* parent) choices_str = QString(poll_item.m_choices[choice.first].m_label); } + if (!weights_str.isEmpty()) { + weights_str += ", " + QString::number(choice.second / COIN); + } else { + weights_str = QString::number(choice.second / COIN); + } + my_total_weight += choice.second / COIN; } ui->myLastVoteAnswerLabel->setText(choices_str); - ui->myVoteWeightLabel->setText(QString::number(my_total_weight)); + ui->myVoteWeightLabel->setText(weights_str); if (poll_item.m_active_weight) ui->myPercentAVWLabel->setText(QString::number((double) my_total_weight / (double) poll_item.m_active_weight * (double) 100.0, 'f', 4) + '\%'); From 3dd263a9b9cb6fbff8ce0b7e5347c572b019fe26 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Thu, 30 Nov 2023 14:07:50 -0500 Subject: [PATCH 04/10] Implement lock for GridcoinConnectBlock to queued txindex write. --- src/gridcoin/voting/result.cpp | 17 ++++++++++--- src/main.cpp | 1 + src/main.h | 1 + src/validation.cpp | 45 ++++++++++++++++++++-------------- 4 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/gridcoin/voting/result.cpp b/src/gridcoin/voting/result.cpp index ef523a2cd3..51047e546a 100644 --- a/src/gridcoin/voting/result.cpp +++ b/src/gridcoin/voting/result.cpp @@ -864,6 +864,7 @@ class VoteCounter { CTransaction tx; + /* bool read_tx_success = false; // This is very ugly. In testing for implement poll expiration reminders PR2716, there is an issue with ReadDiskTx @@ -884,11 +885,21 @@ class VoteCounter } } - if (!read_tx_success) { - LogPrintf("WARN: %s: failed to read vote tx after 10 tries", __func__); - throw InvalidVoteError(); + if (!read_tx_success) { + LogPrintf("WARN: %s: failed to read vote tx after 10 tries", __func__); + throw InvalidVoteError(); + } + */ + + { + LOCK(cs_tx_val_commit_to_disk); + + if (!m_txdb.ReadDiskTx(txid, tx)) { + LogPrintf("WARN: %s: failed to read vote tx.", __func__); + } } + if (tx.nTime < m_poll.m_timestamp) { LogPrintf("WARN: %s: tx earlier than poll", __func__); throw InvalidVoteError(); diff --git a/src/main.cpp b/src/main.cpp index 4fbe0e2740..137a345a12 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -62,6 +62,7 @@ CCriticalSection cs_setpwalletRegistered; set setpwalletRegistered; CCriticalSection cs_main; +CCriticalSection cs_tx_val_commit_to_disk; CTxMemPool mempool; diff --git a/src/main.h b/src/main.h index 336333f4a3..5af4e3e47a 100644 --- a/src/main.h +++ b/src/main.h @@ -75,6 +75,7 @@ typedef std::unordered_map 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; diff --git a/src/validation.cpp b/src/validation.cpp index d9b12eb165..e397f59962 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1641,30 +1641,39 @@ bool ConnectBlock(CBlock& block, CTxDB& txdb, CBlockIndex* pindex, bool fJustChe mapQueuedChanges[hashTx] = CTxIndex(posThisTx, tx.vout.size()); } - if (IsResearchAgeEnabled(pindex->nHeight) - && !GridcoinConnectBlock(block, pindex, txdb, stake_value_in, nStakeReward, nFees)) { - return false; - } + // 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 below. 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); + + if (IsResearchAgeEnabled(pindex->nHeight) + && !GridcoinConnectBlock(block, pindex, txdb, stake_value_in, nStakeReward, nFees)) + { + return false; + } - pindex->nMoneySupply = ReturnCurrentMoneySupply(pindex) + nValueOut - nValueIn; + pindex->nMoneySupply = ReturnCurrentMoneySupply(pindex) + nValueOut - nValueIn; - if (!txdb.WriteBlockIndex(CDiskBlockIndex(pindex))) - return error("%s: WriteBlockIndex for pindex failed", __func__); + if (!txdb.WriteBlockIndex(CDiskBlockIndex(pindex))) + return error("%s: WriteBlockIndex for pindex failed", __func__); - if (!OutOfSyncByAge()) - { - fColdBoot = false; - } + if (!OutOfSyncByAge()) + { + fColdBoot = false; + } - if (fJustCheck) - return true; + if (fJustCheck) + return true; - // Write queued txindex changes - for (const auto& [hash, index] : mapQueuedChanges) - { - if (!txdb.UpdateTxIndex(hash, index)) - return error("%s: UpdateTxIndex failed", __func__); + // Write queued txindex changes + for (const auto& [hash, index] : mapQueuedChanges) + { + if (!txdb.UpdateTxIndex(hash, index)) + return error("%s: UpdateTxIndex failed", __func__); + } } // Update block index on disk without changing it in memory. From 1926b0668a80429d9228974a1d0390519c25720e Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Fri, 1 Dec 2023 15:15:52 -0500 Subject: [PATCH 05/10] Add logging to debug transaction write to poll refresh issue --- src/gridcoin/voting/result.cpp | 3 +++ src/qt/voting/polltablemodel.cpp | 10 ++++++++++ src/validation.cpp | 3 +++ 3 files changed, 16 insertions(+) diff --git a/src/gridcoin/voting/result.cpp b/src/gridcoin/voting/result.cpp index 51047e546a..8ef7fd3f1f 100644 --- a/src/gridcoin/voting/result.cpp +++ b/src/gridcoin/voting/result.cpp @@ -893,10 +893,13 @@ class VoteCounter { 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__); } diff --git a/src/qt/voting/polltablemodel.cpp b/src/qt/voting/polltablemodel.cpp index e169ae8dbb..8f4a467c8e 100644 --- a/src/qt/voting/polltablemodel.cpp +++ b/src/qt/voting/polltablemodel.cpp @@ -6,6 +6,8 @@ #include "qt/voting/polltablemodel.h" #include "qt/voting/votingmodel.h" #include "logging.h" +#include "util.h" +#include "util/threadnames.h" #include #include @@ -260,13 +262,21 @@ void PollTableModel::refresh() __func__); return; + } else { + LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: m_refresh_mutex trylock succeeded.", + __func__); } QtConcurrent::run([this]() { + RenameThread("PollTableModel_refresh"); + util::ThreadSetInternalName("PollTableModel_refresh"); + static_cast(m_data_model.get()) ->reload(m_voting_model->buildPollTable(m_filter_flags)); m_refresh_mutex.unlock(); + LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: m_refresh_mutex lock released.", + __func__); }); } diff --git a/src/validation.cpp b/src/validation.cpp index e397f59962..98830e5b36 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1648,6 +1648,7 @@ bool ConnectBlock(CBlock& block, CTxDB& txdb, CBlockIndex* pindex, bool fJustChe // 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 (IsResearchAgeEnabled(pindex->nHeight) && !GridcoinConnectBlock(block, pindex, txdb, stake_value_in, nStakeReward, nFees)) @@ -1674,6 +1675,8 @@ bool ConnectBlock(CBlock& block, CTxDB& txdb, CBlockIndex* pindex, bool fJustChe if (!txdb.UpdateTxIndex(hash, index)) return error("%s: UpdateTxIndex failed", __func__); } + + LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk unlocked", __func__); } // Update block index on disk without changing it in memory. From c49b2f2bca056638800cb076564b3fbf961bf49b Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 2 Dec 2023 19:44:16 -0500 Subject: [PATCH 06/10] cs_tx_val_commit_to_disk critical section changes --- src/main.cpp | 138 ++++++++++++++++++++++++--------------------- src/validation.cpp | 48 ++++++---------- 2 files changed, 93 insertions(+), 93 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 137a345a12..8aa9e05018 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1156,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 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 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(); + 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(); - 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 diff --git a/src/validation.cpp b/src/validation.cpp index 98830e5b36..d9b12eb165 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1641,42 +1641,30 @@ bool ConnectBlock(CBlock& block, CTxDB& txdb, CBlockIndex* pindex, bool fJustChe mapQueuedChanges[hashTx] = CTxIndex(posThisTx, tx.vout.size()); } + if (IsResearchAgeEnabled(pindex->nHeight) + && !GridcoinConnectBlock(block, pindex, txdb, stake_value_in, nStakeReward, nFees)) { - // 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 below. 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 (IsResearchAgeEnabled(pindex->nHeight) - && !GridcoinConnectBlock(block, pindex, txdb, stake_value_in, nStakeReward, nFees)) - { - return false; - } - - pindex->nMoneySupply = ReturnCurrentMoneySupply(pindex) + nValueOut - nValueIn; + return false; + } - if (!txdb.WriteBlockIndex(CDiskBlockIndex(pindex))) - return error("%s: WriteBlockIndex for pindex failed", __func__); + pindex->nMoneySupply = ReturnCurrentMoneySupply(pindex) + nValueOut - nValueIn; - if (!OutOfSyncByAge()) - { - fColdBoot = false; - } + if (!txdb.WriteBlockIndex(CDiskBlockIndex(pindex))) + return error("%s: WriteBlockIndex for pindex failed", __func__); - if (fJustCheck) - return true; + if (!OutOfSyncByAge()) + { + fColdBoot = false; + } - // Write queued txindex changes - for (const auto& [hash, index] : mapQueuedChanges) - { - if (!txdb.UpdateTxIndex(hash, index)) - return error("%s: UpdateTxIndex failed", __func__); - } + if (fJustCheck) + return true; - LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk unlocked", __func__); + // Write queued txindex changes + for (const auto& [hash, index] : mapQueuedChanges) + { + if (!txdb.UpdateTxIndex(hash, index)) + return error("%s: UpdateTxIndex failed", __func__); } // Update block index on disk without changing it in memory. From ef559d6bf55b13f9312cc938aad5e48637f0c527 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 2 Dec 2023 21:34:21 -0500 Subject: [PATCH 07/10] Remove commented out for loop hack and add commenting on new lock --- src/gridcoin/voting/result.cpp | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/src/gridcoin/voting/result.cpp b/src/gridcoin/voting/result.cpp index 8ef7fd3f1f..95f0be85fe 100644 --- a/src/gridcoin/voting/result.cpp +++ b/src/gridcoin/voting/result.cpp @@ -864,34 +864,11 @@ class VoteCounter { CTransaction tx; - /* - bool read_tx_success = false; - - // This is very ugly. In testing for implement poll expiration reminders PR2716, there is an issue with ReadDiskTx - // on very fast machines, where upon receipt of a vote on an existing poll, the poll builder tests for the transaction - // BEFORE it is committed to disk. This retry loop is essentially zero overhead for an immediate success, but does - // up to 10 tries over up to 2.5 seconds total to "wait" for the transaction to appear in leveldb. - for (unsigned int i = 0; i < 10; ++i) { - if (m_txdb.ReadDiskTx(txid, tx)) { - read_tx_success = true; - break; - } else { - LogPrintf("WARN: %s: failed to read vote tx in try %u", __func__, i + 1); - } - - if (!MilliSleep(250)) { - // Interrupt with throw if MilliSleep interrupted by op sys signal. - throw InvalidVoteError(); - } - } - - if (!read_tx_success) { - LogPrintf("WARN: %s: failed to read vote tx after 10 tries", __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__); @@ -902,7 +879,6 @@ class VoteCounter LogPrint(BCLog::LogFlags::VOTE, "INFO: %s: cs_tx_val_commit_to_disk unlocked", __func__); } - if (tx.nTime < m_poll.m_timestamp) { LogPrintf("WARN: %s: tx earlier than poll", __func__); throw InvalidVoteError(); From adaa443b44e7f3a3fbe7d76a0e1a5a8021a6e2d7 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 3 Dec 2023 00:49:05 -0500 Subject: [PATCH 08/10] Change upper poll expiry notification limit to 168 hours (7 days). --- src/qt/forms/optionsdialog.ui | 2 +- src/qt/optionsdialog.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 9e95f8c909..acd91271bd 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -381,7 +381,7 @@ - Valid values are between 0.25 and 24.0 hours. + Valid values are between 0.25 and 168.0 hours. diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 316f1e4a58..aecbd97885 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -483,7 +483,7 @@ bool OptionsDialog::eventFilter(QObject *object, QEvent *event) if (!ok) { emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, false); } else { - if (hours >= 0.25 && hours <= 24.0) { + if (hours >= 0.25 && hours <= 24.0 * 7.0) { emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, true); } else { emit pollExpireNotifyValid(ui->pollExpireNotifyLineEdit, false); From 6fcd4e9c35544e220f4b169899da0bf747f5351a Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 3 Dec 2023 01:45:11 -0500 Subject: [PATCH 09/10] Corrections for getExpiringPollsNotNotified() in votingmodel --- src/qt/voting/votingmodel.cpp | 6 +++--- src/qt/voting/votingmodel.h | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/qt/voting/votingmodel.cpp b/src/qt/voting/votingmodel.cpp index ffdc671de0..9df0716b5f 100644 --- a/src/qt/voting/votingmodel.cpp +++ b/src/qt/voting/votingmodel.cpp @@ -160,8 +160,6 @@ VotingModel::VotingModel( m_last_poll_time = std::max(m_last_poll_time, iter->Ref().Time()); } } - - m_poll_expire_warning = static_cast(m_options_model.getPollExpireNotification() * 3600.0 * 1000.0); } VotingModel::~VotingModel() @@ -259,9 +257,11 @@ QStringList VotingModel::getExpiringPollsNotNotified() QDateTime now = QDateTime::fromMSecsSinceEpoch(GetAdjustedTime() * 1000); + qint64 poll_expire_warning = static_cast(m_options_model.getPollExpireNotification() * 3600.0 * 1000.0); + // Populate the list and mark the poll items included in the list m_expire_notified true. for (auto& poll : m_pollitems) { - if (now.msecsTo(poll.second.m_expiration) <= m_poll_expire_warning + if (now.msecsTo(poll.second.m_expiration) <= poll_expire_warning && !poll.second.m_expire_notified && !poll.second.m_self_voted) { expiring_polls << poll.second.m_title; diff --git a/src/qt/voting/votingmodel.h b/src/qt/voting/votingmodel.h index e489e967f6..f207948ecc 100644 --- a/src/qt/voting/votingmodel.h +++ b/src/qt/voting/votingmodel.h @@ -173,8 +173,6 @@ class VotingModel : public QObject void newVoteReceived(QString poll_txid_string); private: - qint64 m_poll_expire_warning; - GRC::PollRegistry& m_registry; ClientModel& m_client_model; OptionsModel& m_options_model; From 4946c5412e410aa4c0aeeaffb7a6ea5062e731de Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sun, 10 Dec 2023 11:42:16 -0500 Subject: [PATCH 10/10] Change default notification window for new poll from 1 to 8 hours --- src/qt/optionsmodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index ce76357a0b..666b60371b 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -57,7 +57,7 @@ void OptionsModel::Init() fLimitTxnDisplay = settings.value("fLimitTxnDisplay", false).toBool(); fMaskValues = settings.value("fMaskValues", false).toBool(); limitTxnDate = settings.value("limitTxnDate", QDate()).toDate(); - pollExpireNotification = settings.value("pollExpireNotification", 1.0).toDouble(); + pollExpireNotification = settings.value("pollExpireNotification", 8.0).toDouble(); nReserveBalance = settings.value("nReserveBalance").toLongLong(); language = settings.value("language", "").toString(); walletStylesheet = settings.value("walletStylesheet", "dark").toString();