Add a timer to check for changes in immature or unconfirmed balances, #1559

pull sje397 wants to merge 1 commits into bitcoin:master from sje397:BalanceSignalFix changing 3 files +55 −14

sje397 commented at 4:10 PM on July 5, 2012: contributor

when these are non-zero. Fixed a minor mem leak.

Relates to #837 and #1475.
Diapolo commented at 4:33 PM on July 5, 2012: none

Thanks for starting the real fix :), before I test it, can you rebase this to be on top of #1478? #1778 has an ACK, so to not have any merge conflicts this would be better (the real changes are rather small).
laanwj commented at 4:48 PM on July 5, 2012: member

Code looks good. Haven't tested yet.
Add a timer to check for changes in immature or unconfirmed balances,
when these are non-zero. Fixed a minor mem leak.
6c83a8419b
sje397 commented at 5:08 PM on July 5, 2012: contributor

@Diapolo - I had already made those same changes :)
in src/qt/walletmodel.cpp:None in 6c83a8419b
```
 116 | -    cachedNumTransactions = newNumTransactions;
 117 | +    // Balance and number of transactions might have changed
 118 | +    checkBalanceChanged();
 119 | +
 120 | +    if(cachedUnconfirmedBalance || cachedImmatureBalance)
 121 | +        pollTimer->start();
```
laanwj commented at 6:48 AM on July 6, 2012:

You may want to check for the timer already active here with isActive "If the timer is already running, it will be stopped and restarted." so the start() postpones it. This is not needed in pollBalangeChanged as you already know that the timer triggered there.

sje397 commented at 6:58 AM on July 6, 2012:

The timer is single-shot. I was thinking that the half-second period is pretty fast already...if we get a bunch of transactions at a time, it would make sense to delay wouldn't it?

laanwj commented at 7:11 AM on July 6, 2012:

Every half a second is not fast, even in human terms. I'd prefer the simple semantics of "if there is outstanding immature (or unconfirmed) balance, poll every half a second".

sje397 commented at 7:40 AM on July 6, 2012:

On second thought, it does call 'checkBalanceChanged' immediately before starting or restarting the timer - so it won't be delayed, but might actually be a bit costly if a bunch of transactions come in very quickly. Perhaps it would be better to remove the call to 'checkBalanceChanged', and remove the condition that is there, and just check 'isActive' like you suggest. I.e.

// checkBalanceChanged(); if(!pollTimer->isActive()) pollTimer->start();

Would that make more sense?

laanwj commented at 8:00 AM on July 6, 2012:

Nah, for simplicity's sake and to prevent problems like this in the future, I prefer balance to be recomputed instantly when a transaction comes in. Some caching also happens inside Bitcoin, so computing the balance is not a very expensive operation. Also remember that transactions can also deduct from the balance, which happens instantly and not after maturing/confirmation.

Also mind that updateTransaction can also be called if a transaction is updated (CT_UPDATE), so there is no need to always trigger the timer. It is neater to have it triggered when there is actual unconfirmed/immature balance.

sje397 commented at 11:30 AM on July 6, 2012:

Ok - so leave it as is? Since 'checkBalanceChanged' is already called for each 'updateTransaction', restarting the timer if it's running seems to be sensible.

laanwj commented at 12:43 PM on July 6, 2012:

Yeah, it's fine...
laanwj commented at 6:59 AM on July 6, 2012: member

Good idea to only start the timer when there is immature or unconfirmed balance!
laanwj commented at 11:36 AM on July 7, 2012: member

ACK
laanwj referenced this in commit 9502ee3b5f on Jul 7, 2012
laanwj merged this on Jul 7, 2012
laanwj closed this on Jul 7, 2012
nifgraup referenced this in commit 14716e7ba5 on Mar 30, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#837 Added 'immature balance' for miners. Only displayed if the balance is > 0 #1475 GUI: fix immature display in GUI / add balance updates on block change