Fix calculation of balances and available coins.

[morcos]

No longer consider coins which aren't in our mempool as adding to our spendable but unconfirmed balance. Add testing of this in abandonconflict.py.
Fixes #7690 (sort of)

This PR only changes the result of GetUnconfirmedBalance and AvailableCoins. Balances returned via GetAccountBalance such as the rpc call getbalance "" 0 were already unreliable in 0.11 and suffered a further regression in 0.12. It is recommended to use rpc call getunconfirmedbalance to get the unconfirmed portion of the balance.

Summary of effect on balances for various transaction types:
✅ - Reduces balance for coins spent and increases balance for coins received.
☑️ - Reduces balance for coins spent and increases unconfirmed balance for coins received
⬇️ - Reduces balance for coins spent, does not affect balance for any coins received
😨 - Increases unconfirmed balance for coins received, but does not reduce balance for coins spent
➖ - no effect on balance

tx status	0.11	0.12	#7715
Confirmed	✅	✅	✅
Unconfirmed, in mempool	☑️	☑️	☑️
Unconfirmed, not in mempool	➖	☑️	⬇️
Unconfirmed, not in mempool, abandoned	➖	😨	➖
Unconfirmed, not in mempool, known to conflict	➖	➖	➖
Unconfirmed, not in mempool, non-final	😨	☑️	⬇️
Unconfirmed, not in mempool, non-final, abandoned	😨	😨	➖
Unconfirmed, not in mempool, non-final, known to conflict	😨	😨	➖

[morcos]

@laanwj I think this PR is small enough and a clear enough improvement to a regression that it should be backported for 0.12.1.

[bittylicious]

I can confirm that this no longer shows the transaction that were abandoned on my client, so the patch seems to work as expected - thank you.

As you stated, getbalance doesn't work, and I can confirm "listaccounts 0" also doesn't work (it still shows this abandoned amount). Is there any possibility of allowing these commands to discount the abandoned transactions in another patch as I sadly rely on some deprecated accounting functionality at the moment (yes, I know I shouldn't).

[laanwj]

utACK 68d4282

[morcos]

@bittylicious Unfortunately I don't see the situation improving for the account balance calculations. It's not related to abandontransaction technically, which was never meant to affect whether or not new coins are considered received, but it is related to whether newly received coins are countable in unconfirmed balance or not based on whether they are in the mempool which changed for 0.12. It's very difficult, maybe even impossible, to do account balance calculations properly, so I would expect them to just get more unreliable until they finally disappear. This particular problem should only affect balances for 0 confirms.

[laanwj]

Unfortunately I don't see the situation improving for the account balance calculations

Unfortunately, I don't see so either.

Going forward on the account deprecation is #7729, which adds a label API. Labels behave almost as accounts do, except that they have no balance. I'd like to include that for 0.13. Then at some later time (possibly 0.14), the old account API, along with everything to do with account balance computation is going to be removed.

There's not much chance of anyone maintaining it in the mean time - some of the reasons for removing it in the first place are that the code is a mess, no one knows exactly how it behaves (nor should behave), it has some long-running problems, and thus no one wants to maintain it.

Accounts have been marked DEPRECATED since 0.11, and there was talk of doing so far before that. I'd urge you to start working on an implementation that doesn't need them.

If there is specific functionality that doesn't involve computing balances, but that you feel you cannot achieve without making use of accounts, do let us know (comment in #7729 or so).

[bittylicious]

Thanks @morcos and @laanwj for the analysis on this one. Naturally it's a shame the unconfirmed balance will seemingly always be off, but it's not a showstopper.

Thanks also for the estimated timescale for accounts being removed. I'm well aware of the deprecation of this functionality and there is an internal accounting mechanism there. It just needs more testing as it has a few bugs.

Ultimately, accounts are used indeed mainly for their balance. It also serves as a last chance emergency stop in case a user tries to send more coins than they have, enforced by bitcoind because the account balance would drop to zero. These can, of course, both be coded for outside of bitcoind, but it's always been nice having this emergency. Issues like this one though do show how the accounting functionality has suffered bit rot though so I understand the need to remove it eventually.

[maflcko]

Backported in 19866c1; Removing label; Assuming this is not meant to be backported to 0.11?