From 25f87b9434b98a524d38a97d9fe580acc0fa47ce Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 26 May 2021 13:24:01 +0300 Subject: [PATCH] Merge bitcoin-core/gui#121: Early subscribe core signals in transaction table model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cafef080a2e59c2bcae6baeee3c3c7e187e931ae qt: Refactor to remove unnecessary block in DispatchNotifications (João Barbosa) 57785fb7f61e51e8a8a459486a594443665ea8c9 qt: Early subscribe core signals in transaction table model (João Barbosa) c6cbdf1a90a253fef0259b365a782bf88cd437f2 qt: Refactor ShowProgress to DispatchNotifications (João Barbosa) 3bccd50ad2f384e6c8c97c7f44bda7ae0d777696 qt: Set flag after inital load on transaction table model (João Barbosa) Pull request description: This fixes the case where transaction notifications arrive between `getWalletTxs` and `subscribeToCoreSignals`. Basically notifications are queued until `getWalletTxs` and wallet rescan complete. This is also a requirement to call `getWalletTxs` in a background thread. Motivated by https://github.com/bitcoin/bitcoin/issues/20241. ACKs for top commit: jonatack: tACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae ryanofsky: Code review ACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae. Only change since last review is splitting commits and replacing m_progress with m_loading. meshcollider: Code review ACK cafef080a2e59c2bcae6baeee3c3c7e187e931ae Tree-SHA512: 003caab2f2ae3522619711c8d02d521d2b8f7f280a467f6c3d08abf37ca81cc66b4b9fa10acfdf34e5fe250da7b696cfeec435f72b53c1ea97ccda96d8b4be33 --- src/qt/transactiontablemodel.cpp | 66 ++++++++++++++++---------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index c08c7db2f9..a738428c6c 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -95,21 +95,23 @@ public: */ QList cachedWallet; - bool fQueueNotifications = false; + /** True when model finishes loading all wallet transactions on start */ + bool m_loaded = false; + /** True when transactions are being notified, for instance when scanning */ + bool m_loading = false; std::vector< TransactionNotification > vQueueNotifications; void NotifyTransactionChanged(const uint256 &hash, ChangeType status); void NotifyAddressBookChanged(const CTxDestination &address, const std::string &label, bool isMine, const std::string &purpose, ChangeType status); - void ShowProgress(const std::string &title, int nProgress); + void DispatchNotifications(); /* Query entire wallet anew from core. */ void refreshWallet(interfaces::Wallet& wallet) { - qDebug() << "TransactionTablePriv::refreshWallet"; parent->beginResetModel(); + assert(!m_loaded); try { - cachedWallet.clear(); for (const auto& wtx : wallet.getWalletTxs()) { if (TransactionRecord::showTransaction()) { cachedWallet.append(TransactionRecord::decomposeTransaction(wallet, wtx)); @@ -119,6 +121,8 @@ public: QMessageBox::critical(nullptr, PACKAGE_NAME, QString("Failed to refresh wallet table: ") + QString::fromStdString(e.what())); } parent->endResetModel(); + m_loaded = true; + DispatchNotifications(); } /* Update our model of the wallet incrementally, to synchronize our model of the wallet @@ -267,12 +271,12 @@ TransactionTableModel::TransactionTableModel(WalletModel *parent): fProcessingQueuedTransactions(false), cachedChainLockHeight(-1) { + subscribeToCoreSignals(); + columns << QString() << QString() << tr("Date") << tr("Type") << tr("Address / Label") << BitcoinUnits::getAmountColumnTitle(walletModel->getOptionsModel()->getDisplayUnit()); priv->refreshWallet(walletModel->wallet()); connect(walletModel->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &TransactionTableModel::updateDisplayUnit); - - subscribeToCoreSignals(); } TransactionTableModel::~TransactionTableModel() @@ -793,7 +797,7 @@ void TransactionTablePriv::NotifyTransactionChanged(const uint256 &hash, ChangeT TransactionNotification notification(hash, status, showTransaction); - if (fQueueNotifications) + if (!m_loaded || m_loading) { vQueueNotifications.push_back(notification); return; @@ -812,35 +816,30 @@ void TransactionTablePriv::NotifyAddressBookChanged(const CTxDestination &addres assert(invoked); } -void TransactionTablePriv::ShowProgress(const std::string &title, int nProgress) +void TransactionTablePriv::DispatchNotifications() { - if (nProgress == 0) - fQueueNotifications = true; + if (!m_loaded || m_loading) return; - if (nProgress == 100) - { - fQueueNotifications = false; - if (vQueueNotifications.size() < 10000) { - if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons - bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); - assert(invoked); - } - for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) - { - if (vQueueNotifications.size() - i <= 10) { - bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); - assert(invoked); - } - - vQueueNotifications[i].invoke(parent); - } - } else { - // it's much faster to just refresh the whole thing instead - bool invoked = QMetaObject::invokeMethod(parent, "refreshWallet", Qt::QueuedConnection); + if (vQueueNotifications.size() < 10000) { + if (vQueueNotifications.size() > 10) { // prevent balloon spam, show maximum 10 balloons + bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, true)); assert(invoked); } - vQueueNotifications.clear(); + for (unsigned int i = 0; i < vQueueNotifications.size(); ++i) + { + if (vQueueNotifications.size() - i <= 10) { + bool invoked = QMetaObject::invokeMethod(parent, "setProcessingQueuedTransactions", Qt::QueuedConnection, Q_ARG(bool, false)); + assert(invoked); + } + + vQueueNotifications[i].invoke(parent); + } + } else { + // it's much faster to just refresh the whole thing instead + bool invoked = QMetaObject::invokeMethod(parent, "refreshWallet", Qt::QueuedConnection); + assert(invoked); } + vQueueNotifications.clear(); } void TransactionTableModel::subscribeToCoreSignals() @@ -848,7 +847,10 @@ void TransactionTableModel::subscribeToCoreSignals() // Connect signals to wallet m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(std::bind(&TransactionTablePriv::NotifyTransactionChanged, priv, std::placeholders::_1, std::placeholders::_2)); m_handler_address_book_changed = walletModel->wallet().handleAddressBookChanged(std::bind(&TransactionTablePriv::NotifyAddressBookChanged, priv, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5)); - m_handler_show_progress = walletModel->wallet().handleShowProgress(std::bind(&TransactionTablePriv::ShowProgress, priv, std::placeholders::_1, std::placeholders::_2)); + m_handler_show_progress = walletModel->wallet().handleShowProgress([this](const std::string&, int progress) { + priv->m_loading = progress < 100; + priv->DispatchNotifications(); + }); } void TransactionTableModel::unsubscribeFromCoreSignals()