gui: Fix race in WalletModel::pollBalanceChanged

Poll function was wrongly setting cached height to the current chain height
instead of the chain height at the time of polling.

This bug could cause balances to appear out of date, and was first introduced
a0704a8996 (r378452145)
Before that commit, there wasn't a problem because cs_main was held during the
poll update.

Currently, the problem should be rare. But if
8937d99ce81a27ae5e1012a28323c0e26d89c50b from #17954 were merged, the problem
would get worse, because the wrong cachedNumBlocks value would be set if the
wallet was polled in the interval between a block being connected and it
processing the BlockConnected notification.

MarcoFalke <falke.marco@gmail.com> also points out that a0704a8996 could lead
to GUI hangs as well, because previously the pollBalanceChanged method, which
runs on the GUI thread, would only make a nonblocking TRY_LOCK(cs_main) call,
but after could make blocking LOCK(cs_main) calls, potentially locking up the
GUI.

Thanks to John Newbery <john@johnnewbery.com> for finding this bug this while
reviewing https://github.com/bitcoin/bitcoin/pull/17954.

Github-Pull: #18123
Rebased-From: bf36a3ccc212ad4d7c5cb8f26d7a22e279fe3cec
This commit is contained in:
Russell Yanofsky 2020-02-11 16:53:53 -05:00 committed by fanquake
parent 1964561a3a
commit 48fef5ebae
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1

View file

@ -81,12 +81,12 @@ void WalletModel::pollBalanceChanged()
return;
}
if(fForceCheckBalanceChanged || m_node.getNumBlocks() != cachedNumBlocks)
if(fForceCheckBalanceChanged || numBlocks != cachedNumBlocks)
{
fForceCheckBalanceChanged = false;
// Balance and number of transactions might have changed
cachedNumBlocks = m_node.getNumBlocks();
cachedNumBlocks = numBlocks;
checkBalanceChanged(new_balances);
if(transactionTableModel)