getbalance comment incorrect #8183

1. 
   instagibbs commented at 2:21 pm on June 9, 2016: member

   https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpcwallet.cpp#L693

   // getbalance and “getbalance * 1 true” should return the same number

   This appears to not true at least in two cases, at least in regtest:

   • You send funds to yourself
   • You send funds to anyone with -spendzeroconfchange=0

   For for first case, getbalance includes the zero conf change.

   In the latter case, it seems to differ by the fee amount and coinbase subsidy. Perhaps due to how it accounts coinbase values?

   I unfortunately don’t understand the 2nd case well enough to make a proper edit to the comment. Is this known behavior?

   May be best to simply remove the line.

2. jonasschnelli added the label Wallet on Jun 9, 2016
3. 
   laanwj commented at 9:42 am on June 10, 2016: member

   I think in time we need to get rid of all those different ways to get the balance.
4. 
   MarcoFalke commented at 9:55 am on June 10, 2016: member

   Agree, we should just remove the opts to getbalance rpc call and return a dict, such that the keys denote how the balance was calculated and the value denotes the balance. (Similar to the emoji chart in #7715.)
5. 
   molxyz commented at 3:37 am on January 14, 2017: none

   Hi, in NTumbleBit the “Watch-only” part of Core wallet is used for escrowed multisig timelocked coins, they are part of the total balance but not spendable until later. So I saw @instagibbs mentioned this PR on IRC and out of curiosity I found that, from my wallet, “getbalance” only shows balance of the “Spendable” coins, but “getbalance * 1 true” returns the true balance of the wallet. Attached is a screenshot of the wallet:

   

6. 
   TheBlueMatt commented at 3:55 am on January 14, 2017: member

   Can we kill getbalance “*” for 0.14? It seems super broken (and is broken further by bumpfee, whether that makes it for 0.14 or not).
7. 
   TheBlueMatt commented at 0:28 am on January 16, 2017: member

   Oops, indeed, the 3rd parameter is include_watchonly - this seems to be the only way to get balance of watchonly addresses, so even if we remove the accounts stuff some part of it needs to stay.
8. 
   sipa commented at 0:40 am on January 16, 2017: member

   Can we make getbalance an alias for getbalance "*", and use the old getbalance algorithm for account="*"?
9. 
   morcos commented at 1:41 am on January 16, 2017: member

   +1 for @sipa’s idea…

   i think the question is whether its a bit late to be mucking around with this for 0.14 and risk something accidentally being worse

10. 
    morcos commented at 2:34 am on January 16, 2017: member

    After further review, I’m not sure there was any further regression in 0.14 even with #8456, so lets just fix this properly for 0.15, but perhaps throw an even bigger warning on it for 0.14?

    I’d suggest also making simple new RPC’s for getwatchonlybalance and getunconfirmedwatchonlybalance , similar to the existing getunconfirmedbalance.

11. 
    ryanofsky commented at 7:02 pm on January 25, 2017: member

    Agree, we should just remove the opts to getbalance rpc call and return a dict, such that the keys denote how the balance was calculated and the value denotes the balance. (Similar to the emoji chart in #7715.)

    The getwalletinfo RPC returns a dict with balance, unconfirmed balance, and immature balance. Maybe we should deprecate getbalance and getunconfirmedbalance RPCs in favor of this.

12. laanwj closed this on Mar 6, 2018

    ---

13. laanwj reopened this on Mar 6, 2018

    ---

14. 
    instagibbs commented at 7:48 pm on August 27, 2018: member

    closing this since accounts are getting nuked
15. instagibbs closed this on Aug 27, 2018

    ---

16. DrahtBot locked this on Sep 8, 2021

Contributors
instagibbs laanwj MarcoFalke molxyz TheBlueMatt sipa morcos ryanofsky

Labels
Wallet