From 023e63df78b847812040bf6958c97476606dfbfd Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 28 Oct 2014 19:52:21 +0100 Subject: [PATCH] qt: Move transaction notification to transaction table model Move transaction new/update notification to TransactionTableModel. This moves the concerns to where they're actually handled. No need to bounce this through wallet model. - Do wallet transaction preprocessing on signal handler side; avoids locking cs_main/cs_wallet on notification in GUI thread (except for new transactions) --- src/qt/transactiontablemodel.cpp | 208 +++++++++++++++++++++---------- src/qt/transactiontablemodel.h | 10 +- src/qt/walletmodel.cpp | 43 +------ src/qt/walletmodel.h | 6 +- src/qt/walletview.cpp | 4 +- 5 files changed, 160 insertions(+), 111 deletions(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index e34d77681..79cb4a629 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -91,87 +91,80 @@ public: Call with transaction that was added, removed or changed. */ - void updateWallet(const uint256 &hash, int status) + void updateWallet(const uint256 &hash, int status, bool showTransaction) { qDebug() << "TransactionTablePriv::updateWallet : " + QString::fromStdString(hash.ToString()) + " " + QString::number(status); + + // Find bounds of this transaction in model + QList::iterator lower = qLowerBound( + cachedWallet.begin(), cachedWallet.end(), hash, TxLessThan()); + QList::iterator upper = qUpperBound( + cachedWallet.begin(), cachedWallet.end(), hash, TxLessThan()); + int lowerIndex = (lower - cachedWallet.begin()); + int upperIndex = (upper - cachedWallet.begin()); + bool inModel = (lower != upper); + + if(status == CT_UPDATED) { - LOCK2(cs_main, wallet->cs_wallet); + if(showTransaction && !inModel) + status = CT_NEW; /* Not in model, but want to show, treat as new */ + if(!showTransaction && inModel) + status = CT_DELETED; /* In model, but want to hide, treat as deleted */ + } - // Find transaction in wallet - std::map::iterator mi = wallet->mapWallet.find(hash); - bool inWallet = mi != wallet->mapWallet.end(); + qDebug() << " inModel=" + QString::number(inModel) + + " Index=" + QString::number(lowerIndex) + "-" + QString::number(upperIndex) + + " showTransaction=" + QString::number(showTransaction) + " derivedStatus=" + QString::number(status); - // Find bounds of this transaction in model - QList::iterator lower = qLowerBound( - cachedWallet.begin(), cachedWallet.end(), hash, TxLessThan()); - QList::iterator upper = qUpperBound( - cachedWallet.begin(), cachedWallet.end(), hash, TxLessThan()); - int lowerIndex = (lower - cachedWallet.begin()); - int upperIndex = (upper - cachedWallet.begin()); - bool inModel = (lower != upper); - - // Determine whether to show transaction or not - bool showTransaction = (inWallet && TransactionRecord::showTransaction(mi->second)); - - if(status == CT_UPDATED) + switch(status) + { + case CT_NEW: + if(inModel) { - if(showTransaction && !inModel) - status = CT_NEW; /* Not in model, but want to show, treat as new */ - if(!showTransaction && inModel) - status = CT_DELETED; /* In model, but want to hide, treat as deleted */ + qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_NEW, but transaction is already in model"; + break; } - - qDebug() << " inWallet=" + QString::number(inWallet) + " inModel=" + QString::number(inModel) + - " Index=" + QString::number(lowerIndex) + "-" + QString::number(upperIndex) + - " showTransaction=" + QString::number(showTransaction) + " derivedStatus=" + QString::number(status); - - switch(status) + if(showTransaction) { - case CT_NEW: - if(inModel) - { - qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_NEW, but transaction is already in model"; - break; - } - if(!inWallet) + LOCK2(cs_main, wallet->cs_wallet); + // Find transaction in wallet + std::map::iterator mi = wallet->mapWallet.find(hash); + if(mi == wallet->mapWallet.end()) { qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_NEW, but transaction is not in wallet"; break; } - if(showTransaction) + // Added -- insert at the right position + QList toInsert = + TransactionRecord::decomposeTransaction(wallet, mi->second); + if(!toInsert.isEmpty()) /* only if something to insert */ { - // Added -- insert at the right position - QList toInsert = - TransactionRecord::decomposeTransaction(wallet, mi->second); - if(!toInsert.isEmpty()) /* only if something to insert */ + parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1); + int insert_idx = lowerIndex; + foreach(const TransactionRecord &rec, toInsert) { - parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex+toInsert.size()-1); - int insert_idx = lowerIndex; - foreach(const TransactionRecord &rec, toInsert) - { - cachedWallet.insert(insert_idx, rec); - insert_idx += 1; - } - parent->endInsertRows(); + cachedWallet.insert(insert_idx, rec); + insert_idx += 1; } + parent->endInsertRows(); } - break; - case CT_DELETED: - if(!inModel) - { - qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_DELETED, but transaction is not in model"; - break; - } - // Removed -- remove entire transaction from table - parent->beginRemoveRows(QModelIndex(), lowerIndex, upperIndex-1); - cachedWallet.erase(lower, upper); - parent->endRemoveRows(); - break; - case CT_UPDATED: - // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for - // visible transactions. + } + break; + case CT_DELETED: + if(!inModel) + { + qWarning() << "TransactionTablePriv::updateWallet : Warning: Got CT_DELETED, but transaction is not in model"; break; } + // Removed -- remove entire transaction from table + parent->beginRemoveRows(QModelIndex(), lowerIndex, upperIndex-1); + cachedWallet.erase(lower, upper); + parent->endRemoveRows(); + break; + case CT_UPDATED: + // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for + // visible transactions. + break; } } @@ -230,16 +223,20 @@ TransactionTableModel::TransactionTableModel(CWallet* wallet, WalletModel *paren QAbstractTableModel(parent), wallet(wallet), walletModel(parent), - priv(new TransactionTablePriv(wallet, this)) + priv(new TransactionTablePriv(wallet, this)), + fProcessingQueuedTransactions(false) { columns << QString() << QString() << tr("Date") << tr("Type") << tr("Address") << BitcoinUnits::getAmountColumnTitle(walletModel->getOptionsModel()->getDisplayUnit()); priv->refreshWallet(); connect(walletModel->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); + + subscribeToCoreSignals(); } TransactionTableModel::~TransactionTableModel() { + unsubscribeFromCoreSignals(); delete priv; } @@ -250,12 +247,12 @@ void TransactionTableModel::updateAmountColumnTitle() emit headerDataChanged(Qt::Horizontal,Amount,Amount); } -void TransactionTableModel::updateTransaction(const QString &hash, int status) +void TransactionTableModel::updateTransaction(const QString &hash, int status, bool showTransaction) { uint256 updated; updated.SetHex(hash.toStdString()); - priv->updateWallet(updated, status); + priv->updateWallet(updated, status, showTransaction); } void TransactionTableModel::updateConfirmations() @@ -649,3 +646,82 @@ void TransactionTableModel::updateDisplayUnit() updateAmountColumnTitle(); emit dataChanged(index(0, Amount), index(priv->size()-1, Amount)); } + +// queue notifications to show a non freezing progress dialog e.g. for rescan +struct TransactionNotification +{ +public: + TransactionNotification() {} + TransactionNotification(uint256 hash, ChangeType status, bool showTransaction): + hash(hash), status(status), showTransaction(showTransaction) {} + + void invoke(QObject *ttm) + { + QString strHash = QString::fromStdString(hash.GetHex()); + qDebug() << "NotifyTransactionChanged : " + strHash + " status= " + QString::number(status); + QMetaObject::invokeMethod(ttm, "updateTransaction", Qt::QueuedConnection, + Q_ARG(QString, strHash), + Q_ARG(int, status), + Q_ARG(bool, showTransaction)); + } +private: + uint256 hash; + ChangeType status; + bool showTransaction; +}; + +static bool fQueueNotifications = false; +static std::vector< TransactionNotification > vQueueNotifications; + +static void NotifyTransactionChanged(TransactionTableModel *ttm, CWallet *wallet, const uint256 &hash, ChangeType status) +{ + // Find transaction in wallet + std::map::iterator mi = wallet->mapWallet.find(hash); + // Determine whether to show transaction or not (determine this here so that no relocking is needed in GUI thread) + bool inWallet = mi != wallet->mapWallet.end(); + bool showTransaction = (inWallet && TransactionRecord::showTransaction(mi->second)); + + TransactionNotification notification(hash, status, showTransaction); + + if (fQueueNotifications) + { + vQueueNotifications.push_back(notification); + return; + } + notification.invoke(ttm); +} + +static void ShowProgress(TransactionTableModel *ttm, const std::string &title, int nProgress) +{ + if (nProgress == 0) + fQueueNotifications = true; + + if (nProgress == 100) + { + fQueueNotifications = false; + if (vQueueNotifications.size() > 10) // prevent balloon spam, show maximum 10 balloons + QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); + for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) + { + if (vQueueNotifications.size() - i <= 10) + QMetaObject::invokeMethod(ttm, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + + vQueueNotifications[i].invoke(ttm); + } + std::vector().swap(vQueueNotifications); // clear + } +} + +void TransactionTableModel::subscribeToCoreSignals() +{ + // Connect signals to wallet + wallet->NotifyTransactionChanged.connect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); + wallet->ShowProgress.connect(boost::bind(ShowProgress, this, _1, _2)); +} + +void TransactionTableModel::unsubscribeFromCoreSignals() +{ + // Disconnect signals from wallet + wallet->NotifyTransactionChanged.disconnect(boost::bind(NotifyTransactionChanged, this, _1, _2, _3)); + wallet->ShowProgress.disconnect(boost::bind(ShowProgress, this, _1, _2)); +} diff --git a/src/qt/transactiontablemodel.h b/src/qt/transactiontablemodel.h index 413f3f9bf..bb517a969 100644 --- a/src/qt/transactiontablemodel.h +++ b/src/qt/transactiontablemodel.h @@ -72,12 +72,17 @@ public: QVariant data(const QModelIndex &index, int role) const; QVariant headerData(int section, Qt::Orientation orientation, int role) const; QModelIndex index(int row, int column, const QModelIndex & parent = QModelIndex()) const; + bool processingQueuedTransactions() { return fProcessingQueuedTransactions; } private: CWallet* wallet; WalletModel *walletModel; QStringList columns; TransactionTablePriv *priv; + bool fProcessingQueuedTransactions; + + void subscribeToCoreSignals(); + void unsubscribeFromCoreSignals(); QString lookupAddress(const std::string &address, bool tooltip) const; QVariant addressColor(const TransactionRecord *wtx) const; @@ -92,11 +97,14 @@ private: QVariant txAddressDecoration(const TransactionRecord *wtx) const; public slots: - void updateTransaction(const QString &hash, int status); + /* New transaction, or transaction changed status */ + void updateTransaction(const QString &hash, int status, bool showTransaction); void updateConfirmations(); void updateDisplayUnit(); /** Updates the column title to "Amount (DisplayUnit)" and emits headerDataChanged() signal for table headers to react. */ void updateAmountColumnTitle(); + /* Needed to update fProcessingQueuedTransactions through a QueuedConnection */ + void setProcessingQueuedTransactions(bool value) { fProcessingQueuedTransactions = value; } friend class TransactionTablePriv; }; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index b4733d369..f7b1552f3 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -34,7 +34,6 @@ WalletModel::WalletModel(CWallet *wallet, OptionsModel *optionsModel, QObject *p cachedEncryptionStatus(Unencrypted), cachedNumBlocks(0) { - fProcessingQueuedTransactions = false; fHaveWatchOnly = wallet->HaveWatchOnly(); fForceCheckBalanceChanged = false; @@ -164,11 +163,8 @@ void WalletModel::checkBalanceChanged() } } -void WalletModel::updateTransaction(const QString &hash, int status) +void WalletModel::updateTransaction() { - if(transactionTableModel) - transactionTableModel->updateTransaction(hash, status); - // Balance and number of transactions might have changed fForceCheckBalanceChanged = true; } @@ -455,45 +451,16 @@ static void NotifyAddressBookChanged(WalletModel *walletmodel, CWallet *wallet, Q_ARG(int, status)); } -// queue notifications to show a non freezing progress dialog e.g. for rescan -static bool fQueueNotifications = false; -static std::vector > vQueueNotifications; static void NotifyTransactionChanged(WalletModel *walletmodel, CWallet *wallet, const uint256 &hash, ChangeType status) { - if (fQueueNotifications) - { - vQueueNotifications.push_back(make_pair(hash, status)); - return; - } - - QString strHash = QString::fromStdString(hash.GetHex()); - - qDebug() << "NotifyTransactionChanged : " + strHash + " status= " + QString::number(status); - QMetaObject::invokeMethod(walletmodel, "updateTransaction", Qt::QueuedConnection, - Q_ARG(QString, strHash), - Q_ARG(int, status)); + Q_UNUSED(wallet); + Q_UNUSED(hash); + Q_UNUSED(status); + QMetaObject::invokeMethod(walletmodel, "updateTransaction", Qt::QueuedConnection); } static void ShowProgress(WalletModel *walletmodel, const std::string &title, int nProgress) { - if (nProgress == 0) - fQueueNotifications = true; - - if (nProgress == 100) - { - fQueueNotifications = false; - if (vQueueNotifications.size() > 10) // prevent balloon spam, show maximum 10 balloons - QMetaObject::invokeMethod(walletmodel, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); - for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) - { - if (vQueueNotifications.size() - i <= 10) - QMetaObject::invokeMethod(walletmodel, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); - - NotifyTransactionChanged(walletmodel, NULL, vQueueNotifications[i].first, vQueueNotifications[i].second); - } - std::vector >().swap(vQueueNotifications); // clear - } - // emits signal "showProgress" QMetaObject::invokeMethod(walletmodel, "showProgress", Qt::QueuedConnection, Q_ARG(QString, QString::fromStdString(title)), diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index b1d0f28f1..fe91e9d9f 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -133,7 +133,6 @@ public: CAmount getWatchUnconfirmedBalance() const; CAmount getWatchImmatureBalance() const; EncryptionStatus getEncryptionStatus() const; - bool processingQueuedTransactions() { return fProcessingQueuedTransactions; } // Check address for validity bool validateAddress(const QString &address); @@ -197,7 +196,6 @@ public: private: CWallet *wallet; - bool fProcessingQueuedTransactions; bool fHaveWatchOnly; bool fForceCheckBalanceChanged; @@ -254,15 +252,13 @@ public slots: /* Wallet status might have changed */ void updateStatus(); /* New transaction, or transaction changed status */ - void updateTransaction(const QString &hash, int status); + void updateTransaction(); /* New, updated or removed address book entry */ void updateAddressBook(const QString &address, const QString &label, bool isMine, const QString &purpose, int status); /* Watch-only added */ void updateWatchOnlyFlag(bool fHaveWatchonly); /* Current, immature or unconfirmed balance might have changed - emit 'balanceChanged' if so */ void pollBalanceChanged(); - /* Needed to update fProcessingQueuedTransactions through a QueuedConnection */ - void setProcessingQueuedTransactions(bool value) { fProcessingQueuedTransactions = value; } }; #endif // WALLETMODEL_H diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index eff50593b..3b8fdd7e5 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -137,10 +137,12 @@ void WalletView::setWalletModel(WalletModel *walletModel) void WalletView::processNewTransaction(const QModelIndex& parent, int start, int /*end*/) { // Prevent balloon-spam when initial block download is in progress - if (!walletModel || walletModel->processingQueuedTransactions() || !clientModel || clientModel->inInitialBlockDownload()) + if (!walletModel || !clientModel || clientModel->inInitialBlockDownload()) return; TransactionTableModel *ttm = walletModel->getTransactionTableModel(); + if (!ttm || ttm->processingQueuedTransactions()) + return; QString date = ttm->index(start, TransactionTableModel::Date, parent).data().toString(); qint64 amount = ttm->index(start, TransactionTableModel::Amount, parent).data(Qt::EditRole).toULongLong();