Merge #18587: gui: Avoid wallet tryGetBalances calls in WalletModel::pollBalanceChanged

d3a56be77a Revert "gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged" (Russell Yanofsky)
bf0a510981 gui: Avoid wallet tryGetBalances calls before TransactionChanged or BlockTip notifications (Russell Yanofsky)
2bc9b92ed8 Cancel wallet balance timer when shutdown requested (Russell Yanofsky)
83f69fab3a Switch transaction table to use wallet height not node height (Russell Yanofsky)

Pull request description:

  Main commit `gui: Avoid wallet tryGetBalances calls` is one-line change to `WalletModel::pollBalanceChanged` that returns early if there hasn't been a new `TransactionChanged` or `BlockTip` notification since the previous poll call. This is the same behavior that was implemented in #18160, now implemented in a simpler way.

  The other commits are a straight revert of #18160, and two tweaks to avoid relying on `WalletModel::m_client_model` lifetime which were causing travis failures with earlier versions of this PR.

  Motivation for this change is to be able to revert #18160 and cut down on unnecessary cross-process calls that happen when #18160 is combined with #10102

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

ACKs for top commit:
  jonasschnelli:
    utACK d3a56be77a

Tree-SHA512: 3cd31ca515e77c3bd7160d3f1ea0dce5050d4038b2aa441b6f66b8599bd413d81ca5542a197806e773d6092dd1d26830932b1cecbc95298b1f1ab41099e2f12f
pull/19020/head
Jonas Schnelli 5 years ago
commit a587f85853
No known key found for this signature in database
GPG Key ID: 1EB776BB03C7922D

@ -351,14 +351,13 @@ public:
} }
return result; return result;
} }
bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override bool tryGetBalances(WalletBalances& balances, int& num_blocks) override
{ {
TRY_LOCK(m_wallet->cs_wallet, locked_wallet); TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) { if (!locked_wallet) {
return false; return false;
} }
num_blocks = m_wallet->GetLastBlockHeight(); num_blocks = m_wallet->GetLastBlockHeight();
if (!force && num_blocks == cached_num_blocks) return false;
balances = getBalances(); balances = getBalances();
return true; return true;
} }

@ -202,11 +202,8 @@ public:
//! Get balances. //! Get balances.
virtual WalletBalances getBalances() = 0; virtual WalletBalances getBalances() = 0;
//! Get balances if possible without waiting for chain and wallet locks. //! Get balances if possible without blocking.
virtual bool tryGetBalances(WalletBalances& balances, virtual bool tryGetBalances(WalletBalances& balances, int& num_blocks) = 0;
int& num_blocks,
bool force,
int cached_num_blocks) = 0;
//! Get balance. //! Get balance.
virtual CAmount getBalance() = 0; virtual CAmount getBalance() = 0;

@ -664,7 +664,7 @@ QVariant TransactionTableModel::headerData(int section, Qt::Orientation orientat
QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex &parent) const QModelIndex TransactionTableModel::index(int row, int column, const QModelIndex &parent) const
{ {
Q_UNUSED(parent); Q_UNUSED(parent);
TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->clientModel().getNumBlocks(), row); TransactionRecord *data = priv->index(walletModel->wallet(), walletModel->getNumBlocks(), row);
if(data) if(data)
{ {
return createIndex(row, column, data); return createIndex(row, column, data);

@ -39,14 +39,15 @@
WalletModel::WalletModel(std::unique_ptr<interfaces::Wallet> wallet, ClientModel& client_model, const PlatformStyle *platformStyle, QObject *parent) : WalletModel::WalletModel(std::unique_ptr<interfaces::Wallet> wallet, ClientModel& client_model, const PlatformStyle *platformStyle, QObject *parent) :
QObject(parent), QObject(parent),
m_wallet(std::move(wallet)), m_wallet(std::move(wallet)),
m_client_model(client_model), m_client_model(&client_model),
m_node(client_model.node()), m_node(client_model.node()),
optionsModel(client_model.getOptionsModel()), optionsModel(client_model.getOptionsModel()),
addressTableModel(nullptr), addressTableModel(nullptr),
transactionTableModel(nullptr), transactionTableModel(nullptr),
recentRequestsTableModel(nullptr), recentRequestsTableModel(nullptr),
cachedEncryptionStatus(Unencrypted), cachedEncryptionStatus(Unencrypted),
cachedNumBlocks(0) cachedNumBlocks(0),
timer(new QTimer(this))
{ {
fHaveWatchOnly = m_wallet->haveWatchOnly(); fHaveWatchOnly = m_wallet->haveWatchOnly();
addressTableModel = new AddressTableModel(this); addressTableModel = new AddressTableModel(this);
@ -64,11 +65,16 @@ WalletModel::~WalletModel()
void WalletModel::startPollBalance() void WalletModel::startPollBalance()
{ {
// This timer will be fired repeatedly to update the balance // This timer will be fired repeatedly to update the balance
QTimer* timer = new QTimer(this);
connect(timer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged); connect(timer, &QTimer::timeout, this, &WalletModel::pollBalanceChanged);
timer->start(MODEL_UPDATE_DELAY); timer->start(MODEL_UPDATE_DELAY);
} }
void WalletModel::setClientModel(ClientModel* client_model)
{
m_client_model = client_model;
if (!m_client_model) timer->stop();
}
void WalletModel::updateStatus() void WalletModel::updateStatus()
{ {
EncryptionStatus newEncryptionStatus = getEncryptionStatus(); EncryptionStatus newEncryptionStatus = getEncryptionStatus();
@ -80,24 +86,31 @@ void WalletModel::updateStatus()
void WalletModel::pollBalanceChanged() void WalletModel::pollBalanceChanged()
{ {
// Avoid recomputing wallet balances unless a TransactionChanged or
// BlockTip notification was received.
if (!fForceCheckBalanceChanged && cachedNumBlocks == m_client_model->getNumBlocks()) return;
// Try to get balances and return early if locks can't be acquired. This // Try to get balances and return early if locks can't be acquired. This
// avoids the GUI from getting stuck on periodical polls if the core is // avoids the GUI from getting stuck on periodical polls if the core is
// holding the locks for a longer time - for example, during a wallet // holding the locks for a longer time - for example, during a wallet
// rescan. // rescan.
interfaces::WalletBalances new_balances; interfaces::WalletBalances new_balances;
int numBlocks = -1; int numBlocks = -1;
if (!m_wallet->tryGetBalances(new_balances, numBlocks, fForceCheckBalanceChanged, cachedNumBlocks)) { if (!m_wallet->tryGetBalances(new_balances, numBlocks)) {
return; return;
} }
fForceCheckBalanceChanged = false; if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks)
{
fForceCheckBalanceChanged = false;
// Balance and number of transactions might have changed // Balance and number of transactions might have changed
cachedNumBlocks = numBlocks; cachedNumBlocks = numBlocks;
checkBalanceChanged(new_balances); checkBalanceChanged(new_balances);
if(transactionTableModel) if(transactionTableModel)
transactionTableModel->updateConfirmations(); transactionTableModel->updateConfirmations();
}
} }
void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances) void WalletModel::checkBalanceChanged(const interfaces::WalletBalances& new_balances)

@ -144,7 +144,8 @@ public:
interfaces::Node& node() const { return m_node; } interfaces::Node& node() const { return m_node; }
interfaces::Wallet& wallet() const { return *m_wallet; } interfaces::Wallet& wallet() const { return *m_wallet; }
ClientModel& clientModel() const { return m_client_model; } void setClientModel(ClientModel* client_model);
int getNumBlocks() const { return cachedNumBlocks; }
QString getWalletName() const; QString getWalletName() const;
QString getDisplayName() const; QString getDisplayName() const;
@ -161,7 +162,7 @@ private:
std::unique_ptr<interfaces::Handler> m_handler_show_progress; std::unique_ptr<interfaces::Handler> m_handler_show_progress;
std::unique_ptr<interfaces::Handler> m_handler_watch_only_changed; std::unique_ptr<interfaces::Handler> m_handler_watch_only_changed;
std::unique_ptr<interfaces::Handler> m_handler_can_get_addrs_changed; std::unique_ptr<interfaces::Handler> m_handler_can_get_addrs_changed;
ClientModel& m_client_model; ClientModel* m_client_model;
interfaces::Node& m_node; interfaces::Node& m_node;
bool fHaveWatchOnly; bool fHaveWatchOnly;
@ -179,6 +180,7 @@ private:
interfaces::WalletBalances m_cached_balances; interfaces::WalletBalances m_cached_balances;
EncryptionStatus cachedEncryptionStatus; EncryptionStatus cachedEncryptionStatus;
int cachedNumBlocks; int cachedNumBlocks;
QTimer* timer;
void subscribeToCoreSignals(); void subscribeToCoreSignals();
void unsubscribeFromCoreSignals(); void unsubscribeFromCoreSignals();

@ -97,6 +97,7 @@ void WalletView::setClientModel(ClientModel *_clientModel)
overviewPage->setClientModel(_clientModel); overviewPage->setClientModel(_clientModel);
sendCoinsPage->setClientModel(_clientModel); sendCoinsPage->setClientModel(_clientModel);
if (walletModel) walletModel->setClientModel(_clientModel);
} }
void WalletView::setWalletModel(WalletModel *_walletModel) void WalletView::setWalletModel(WalletModel *_walletModel)

Loading…
Cancel
Save