From aef3dd73509215f42ef1981017c51d5e62c49f2f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 10 Apr 2023 19:16:08 +0000 Subject: [PATCH] merge bitcoin#17993: Balance/TxStatus polling update based on last block hash --- src/interfaces/node.cpp | 9 ++++++-- src/interfaces/node.h | 14 +++++++++++-- src/interfaces/wallet.cpp | 4 ++-- src/interfaces/wallet.h | 2 +- src/qt/clientmodel.cpp | 28 +++++++++++++++++-------- src/qt/clientmodel.h | 7 ++++++- src/qt/transactionrecord.cpp | 8 +++---- src/qt/transactionrecord.h | 10 ++++----- src/qt/transactiontablemodel.cpp | 8 +++---- src/qt/walletmodel.cpp | 14 ++++--------- src/qt/walletmodel.h | 5 +++-- test/lint/lint-circular-dependencies.sh | 1 + 12 files changed, 68 insertions(+), 42 deletions(-) diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 0b26159f46..63a9320201 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -297,6 +297,11 @@ public: LOCK(::cs_main); return ::ChainActive().Height(); } + uint256 getBestBlockHash() override + { + const CBlockIndex* tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip()); + return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash(); + } int64_t getLastBlockTime() override { LOCK(::cs_main); @@ -395,7 +400,7 @@ public: std::unique_ptr handleNotifyBlockTip(NotifyBlockTipFn fn) override { return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](bool initial_download, const CBlockIndex* block) { - fn(initial_download, block->nHeight, block->GetBlockTime(), block->GetBlockHash().ToString(), + fn(initial_download, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, GuessVerificationProgress(Params().TxData(), block)); })); } @@ -409,7 +414,7 @@ public: { return MakeHandler( ::uiInterface.NotifyHeaderTip_connect([fn](bool initial_download, const CBlockIndex* block) { - fn(initial_download, block->nHeight, block->GetBlockTime(), block->GetBlockHash().ToString(), + fn(initial_download, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()}, /* verification progress is unused when a header was received */ 0); })); } diff --git a/src/interfaces/node.h b/src/interfaces/node.h index 6d145f2a6a..8aee777821 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -38,6 +38,7 @@ struct NodeContext; namespace interfaces { class Handler; class WalletClient; +struct BlockTip; //! Interface for the src/evo part of a dash node (dashd process). class EVO @@ -229,6 +230,9 @@ public: //! Get num blocks. virtual int getNumBlocks() = 0; + //! Get best block hash. + virtual uint256 getBestBlockHash() = 0; + //! Get last block time. virtual int64_t getLastBlockTime() = 0; @@ -327,7 +331,7 @@ public: //! Register handler for block tip messages. using NotifyBlockTipFn = - std::function; + std::function; virtual std::unique_ptr handleNotifyBlockTip(NotifyBlockTipFn fn) = 0; //! Register handler for chainlock messages. @@ -337,7 +341,7 @@ public: //! Register handler for header tip messages. using NotifyHeaderTipFn = - std::function; + std::function; virtual std::unique_ptr handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0; //! Register handler for masternode list update messages. @@ -359,6 +363,12 @@ public: //! Return implementation of Node interface. std::unique_ptr MakeNode(NodeContext* context = nullptr); +//! Block tip (could be a header or not, depends on the subscribed signal). +struct BlockTip { + int block_height; + int64_t block_time; + uint256 block_hash; +}; } // namespace interfaces #endif // BITCOIN_INTERFACES_NODE_H diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 944b242938..1b025b89d1 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -417,13 +417,13 @@ public: } return result; } - bool tryGetBalances(WalletBalances& balances, int& num_blocks) override + bool tryGetBalances(WalletBalances& balances, uint256& block_hash) override { TRY_LOCK(m_wallet->cs_wallet, locked_wallet); if (!locked_wallet) { return false; } - num_blocks = m_wallet->GetLastBlockHeight(); + block_hash = m_wallet->GetLastBlockHash(); balances = getBalances(); return true; } diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index fba448f54c..2c2befad5a 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -229,7 +229,7 @@ public: virtual WalletBalances getBalances() = 0; //! Get balances if possible without blocking. - virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0; + virtual bool tryGetBalances(WalletBalances& balances, uint256& block_hash) = 0; //! Get balance. virtual CAmount getBalance() = 0; diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index bbe20f0615..ab0c05087d 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -145,6 +145,15 @@ int ClientModel::getNumBlocks() const return m_cached_num_blocks; } +uint256 ClientModel::getBestBlockHash() +{ + LOCK(m_cached_tip_mutex); + if (m_cached_tip_blocks.IsNull()) { + m_cached_tip_blocks = m_node.getBestBlockHash(); + } + return m_cached_tip_blocks; +} + void ClientModel::updateNumConnections(int numConnections) { Q_EMIT numConnectionsChanged(numConnections); @@ -266,7 +275,7 @@ static void BannedListChanged(ClientModel *clientmodel) assert(invoked); } -static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int height, int64_t blockTime, const std::string& strBlockHash, double verificationProgress, bool fHeader) +static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, interfaces::BlockTip tip, double verificationProgress, bool fHeader) { // lock free async UI updates in case we have a new block tip // during initial sync, only update the UI if the last update @@ -279,19 +288,20 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int heig if (fHeader) { // cache best headers time and height to reduce future cs_main locks - clientmodel->cachedBestHeaderHeight = height; - clientmodel->cachedBestHeaderTime = blockTime; + clientmodel->cachedBestHeaderHeight = tip.block_height; + clientmodel->cachedBestHeaderTime = tip.block_time; } else { - clientmodel->m_cached_num_blocks = height; + clientmodel->m_cached_num_blocks = tip.block_height; + WITH_LOCK(clientmodel->m_cached_tip_mutex, clientmodel->m_cached_tip_blocks = tip.block_hash;); } // During initial sync, block notifications, and header notifications from reindexing are both throttled. if (!initialSync || (fHeader && !clientmodel->node().getReindex()) || now - nLastUpdateNotification > MODEL_UPDATE_DELAY) { //pass an async signal to the UI thread bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection, - Q_ARG(int, height), - Q_ARG(QDateTime, QDateTime::fromTime_t(blockTime)), - Q_ARG(QString, QString::fromStdString(strBlockHash)), + Q_ARG(int, tip.block_height), + Q_ARG(QDateTime, QDateTime::fromTime_t(tip.block_time)), + Q_ARG(QString, QString::fromStdString(tip.block_hash.ToString())), Q_ARG(double, verificationProgress), Q_ARG(bool, fHeader)); assert(invoked); @@ -328,9 +338,9 @@ void ClientModel::subscribeToCoreSignals() m_handler_notify_network_active_changed = m_node.handleNotifyNetworkActiveChanged(std::bind(NotifyNetworkActiveChanged, this, std::placeholders::_1)); m_handler_notify_alert_changed = m_node.handleNotifyAlertChanged(std::bind(NotifyAlertChanged, this)); m_handler_banned_list_changed = m_node.handleBannedListChanged(std::bind(BannedListChanged, this)); - m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5, false)); + m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, false)); m_handler_notify_chainlock = m_node.handleNotifyChainLock(std::bind(NotifyChainLock, this, std::placeholders::_1, std::placeholders::_2)); - m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4, std::placeholders::_5, true)); + m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, true)); m_handler_notify_masternodelist_changed = m_node.handleNotifyMasternodeListChanged(std::bind(NotifyMasternodeListChanged, this, std::placeholders::_1)); m_handler_notify_additional_data_sync_progess_changed = m_node.handleNotifyAdditionalDataSyncProgressChanged(std::bind(NotifyAdditionalDataSyncProgressChanged, this, std::placeholders::_1)); } diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index ef9ec7bc66..8a22a3143a 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -14,6 +14,7 @@ #include #include +#include class BanTableModel; class OptionsModel; @@ -62,6 +63,7 @@ public: //! Return number of connections, default is in- and outbound (total) int getNumConnections(unsigned int flags = CONNECTIONS_ALL) const; int getNumBlocks() const; + uint256 getBestBlockHash(); int getHeaderTipHeight() const; int64_t getHeaderTipTime() const; @@ -85,11 +87,14 @@ public: bool getProxyInfo(std::string& ip_port) const; - // caches for the best header, number of blocks + // caches for the best header: hash, number of blocks and block time mutable std::atomic cachedBestHeaderHeight; mutable std::atomic cachedBestHeaderTime; mutable std::atomic m_cached_num_blocks{-1}; + Mutex m_cached_tip_mutex; + uint256 m_cached_tip_blocks GUARDED_BY(m_cached_tip_mutex){}; + private: interfaces::Node& m_node; std::unique_ptr m_handler_show_progress; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index db3f7ae850..bc481125df 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -257,7 +257,7 @@ QList TransactionRecord::decomposeTransaction(interfaces::Wal return parts; } -void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int numBlocks, int chainLockHeight, int64_t block_time) +void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, const uint256& block_hash, int numBlocks, int chainLockHeight, int64_t block_time) { // Determine transaction status @@ -269,7 +269,7 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int idx); status.countsForBalance = wtx.is_trusted && !(wtx.blocks_to_maturity > 0); status.depth = wtx.depth_in_main_chain; - status.cur_num_blocks = numBlocks; + status.m_cur_block_hash = block_hash; status.cachedChainLockHeight = chainLockHeight; status.lockedByChainLocks = wtx.is_chainlocked; status.lockedByInstantSend = wtx.is_islocked; @@ -331,9 +331,9 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, int status.needsUpdate = false; } -bool TransactionRecord::statusUpdateNeeded(int numBlocks, int chainLockHeight) const +bool TransactionRecord::statusUpdateNeeded(const uint256& block_hash, int chainLockHeight) const { - return status.cur_num_blocks != numBlocks || status.needsUpdate + return status.m_cur_block_hash != block_hash || status.needsUpdate || (!status.lockedByChainLocks && status.cachedChainLockHeight != chainLockHeight); } diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index a84eb3fff8..eef93b3bfd 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -26,7 +26,7 @@ class TransactionStatus public: TransactionStatus(): countsForBalance(false), lockedByInstantSend(false), lockedByChainLocks(false), sortKey(""), - matures_in(0), status(Unconfirmed), depth(0), open_for(0), cur_num_blocks(-1), + matures_in(0), status(Unconfirmed), depth(0), open_for(0), cachedChainLockHeight(-1), needsUpdate(false) { } @@ -67,8 +67,8 @@ public: finalization */ /**@}*/ - /** Current number of blocks (to know whether cached status is still valid) */ - int cur_num_blocks; + /** Current block hash (to know whether cached status is still valid) */ + uint256 m_cur_block_hash{}; //** Know when to update transaction for chainlocks **/ int cachedChainLockHeight; @@ -161,11 +161,11 @@ public: /** Update status from core wallet tx. */ - void updateStatus(const interfaces::WalletTxStatus& wtx, int numBlocks, int chainLockHeight, int64_t block_time); + void updateStatus(const interfaces::WalletTxStatus& wtx, const uint256& block_hash, int numBlocks, int chainLockHeight, int64_t block_time); /** Return whether a status update is needed. */ - bool statusUpdateNeeded(int numBlocks, int chainLockHeight) const; + bool statusUpdateNeeded(const uint256& block_hash, int chainLockHeight) const; /** Update label from address book. */ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 7c3bc28fcd..e2b69ef51a 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -188,7 +188,7 @@ public: return cachedWallet.size(); } - TransactionRecord *index(interfaces::Wallet& wallet, const int cur_num_blocks, const int idx) + TransactionRecord* index(interfaces::Wallet& wallet, const uint256& cur_block_hash, const int cur_num_blocks, const int idx) { if (idx >= 0 && idx < cachedWallet.size()) { TransactionRecord *rec = &cachedWallet[idx]; @@ -198,8 +198,8 @@ public: // Otherwise, simply re-use the cached status. interfaces::WalletTxStatus wtx; int64_t block_time; - if (rec->statusUpdateNeeded(cur_num_blocks, parent->getChainLockHeight()) && wallet.tryGetTxStatus(rec->hash, wtx, block_time)) { - rec->updateStatus(wtx, cur_num_blocks, parent->getChainLockHeight(), block_time); + if (rec->statusUpdateNeeded(cur_block_hash, parent->getChainLockHeight()) && wallet.tryGetTxStatus(rec->hash, wtx, block_time)) { + rec->updateStatus(wtx, cur_block_hash, cur_num_blocks, parent->getChainLockHeight(), block_time); } return rec; } @@ -726,7 +726,7 @@ QVariant TransactionTableModel::headerData(int section, Qt::Orientation orientat QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex &parent) const { Q_UNUSED(parent); - TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->clientModel().getNumBlocks(), row); + TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->clientModel().getBestBlockHash(), walletModel->clientModel().getNumBlocks(), row); if (data) { return createIndex(row, column, data); } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 9b76d7e276..71ffc2a7f6 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -46,7 +46,6 @@ WalletModel::WalletModel(std::unique_ptr wallet, ClientModel transactionTableModel(nullptr), recentRequestsTableModel(nullptr), cachedEncryptionStatus(Unencrypted), - cachedNumBlocks(-1), cachedNumISLocks(0), cachedCoinJoinRounds(0) { @@ -91,17 +90,17 @@ void WalletModel::pollBalanceChanged() // holding the locks for a longer time - for example, during a wallet // rescan. interfaces::WalletBalances new_balances; - int numBlocks = -1; - if (!m_wallet->tryGetBalances(new_balances, numBlocks)) { + uint256 block_hash; + if (!m_wallet->tryGetBalances(new_balances, block_hash)) { return; } - if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks || node().coinJoinOptions().getRounds() != cachedCoinJoinRounds) + if (fForceCheckBalanceChanged || block_hash != m_cached_last_update_tip || node().coinJoinOptions().getRounds() != cachedCoinJoinRounds) { fForceCheckBalanceChanged = false; // Balance and number of transactions might have changed - cachedNumBlocks = numBlocks; + m_cached_last_update_tip = block_hash; cachedCoinJoinRounds = node().coinJoinOptions().getRounds(); checkBalanceChanged(new_balances); @@ -137,11 +136,6 @@ void WalletModel::updateChainLockHeight(int chainLockHeight) fForceCheckBalanceChanged = true; } -int WalletModel::getNumBlocks() const -{ - return cachedNumBlocks; -} - int WalletModel::getNumISLocks() const { return cachedNumISLocks; diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 801e3b8656..8fe398714e 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -191,7 +191,6 @@ public: bool privateKeysDisabled() const; bool canGetAddresses() const; - int getNumBlocks() const; int getNumISLocks() const; int getRealOutpointCoinJoinRounds(const COutPoint& outpoint) const; @@ -236,10 +235,12 @@ private: // Cache some values to be able to detect changes interfaces::WalletBalances m_cached_balances; EncryptionStatus cachedEncryptionStatus; - int cachedNumBlocks; int cachedNumISLocks; int cachedCoinJoinRounds; + // Block hash denoting when the last balance update was done. + uint256 m_cached_last_update_tip{}; + void subscribeToCoreSignals(); void unsubscribeFromCoreSignals(); void checkBalanceChanged(const interfaces::WalletBalances& new_balances); diff --git a/test/lint/lint-circular-dependencies.sh b/test/lint/lint-circular-dependencies.sh index ed71040220..2dfd37fab1 100755 --- a/test/lint/lint-circular-dependencies.sh +++ b/test/lint/lint-circular-dependencies.sh @@ -55,6 +55,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=( "qt/bitcoingui -> qt/guiutil -> qt/bitcoingui" "qt/guiutil -> qt/optionsdialog -> qt/guiutil" "qt/guiutil -> qt/qvalidatedlineedit -> qt/guiutil" + "qt/clientmodel -> qt/guiutil -> qt/walletmodel -> qt/clientmodel" "core_io -> evo/cbtx -> evo/deterministicmns -> core_io" "core_io -> evo/cbtx -> evo/simplifiedmns -> core_io" "evo/simplifiedmns -> llmq/blockprocessor -> net_processing -> evo/simplifiedmns"