rpc: Add balances RPC

[maflcko]

This exposes the CWallet::GetBalance() struct over RPC.

In the future, incorrectly named rpcs such as getunconfirmedbalance or rpcs redundant to this such as getbalance could be removed.

[laanwj]

Concept ACK

or rpcs redundant to this such as getbalance could be removed.

Dunno, I think having a dedicated call to get the balance is useful. If it's not broken (like mis-named methods) I'd prefer keeping it.

[instagibbs]

concept ACK

[jnewbery]

Looks good. I think this needs release notes to describe the new return object and document that getunconfirmedbalance is deprecated.

I think having a dedicated call to get the balance is useful.

How about we have a getbalances RPC that just returns the balances object added in this PR.

[maflcko]

How about we have a getbalances RPC that just returns the balances object added in this PR.

Then I'd rather not add the same struct to getwalletinfo. Imo a single place where to get this is enough.

[jnewbery]

I don't have an issue with the same data appearing in multiple RPC methods, but if you think that's an issue, then I'd have a slight preference for a getbalances RPC method to be separate from getwalletinfo. I think either is an improvement though.

I also think all wallet get- and list- methods should return a best block hash, so it's clear at what point the returned values are valid.

[jonasschnelli]

Nice!
Concept ACK.

I agree with @jnewbery about returning the new object through getbalance. Why?
getwalletinfo, for historical and logical reasons, has the is_min is_spendable balance value in it and I think removing it makes little sense. We already have the same data type in getbalance and to me it would just be logical to switch from a single balance value to the new obect (in both calls).

I agree about depracating / removing getunconfirmedbalance

[jonatack]

Concept ACK

Initial manual testing, to be completed later:

test/functional/wallet_balance.py passes locally -> EDIT: all unit + extended functional tests pass locally

getwalletinfo

  "balances": {
    "mine": {
      "trusted": 0.00000000,
      "untrusted_pending": 0.00000000,
      "immature": 0.00000000
    }
  },

help getwalletinfo

getwalletinfo
Returns an object containing various wallet state info.

Result:
{
  "walletname": xxxxx,               (string) the wallet name
  "walletversion": xxxxx,            (numeric) the wallet version
  "balances": {                      (object) the wallet balances in BTC
    "mine": {                        (object) balances from outputs that the wallet can sign
      "trusted": xxx                 (numeric) trusted balance (outputs created by the wallet or confirmed outputs)
      "untrusted_pending": xxx       (numeric) untrusted pending balance (outputs created by others that are in the mempool)
      "immature": xxx                (numeric) balance from immature coinbase outputs
    },
    "watchonly": {                   (object) watchonly balances (not present if wallet does not watch anything)
      "trusted": xxx                 (numeric) trusted balance (outputs created by the wallet or confirmed outputs)
      "untrusted_pending": xxx       (numeric) untrusted pending balance (outputs created by others that are in the mempool)
      "immature": xxx                (numeric) balance from immature coinbase outputs
    },
  },
  "balance": xxxxxxx,                (numeric) Identical to balances.mine.trusted
  "unconfirmed_balance": xxx,        (numeric) Identical to balances.mine.untrusted_pending
  "immature_balance": xxxxxx,        (numeric) Identical to balances.mine.immature
  "txcount": xxxxxxx,                (numeric) the total number of transactions in the wallet
  "keypoololdest": xxxxxx,           (numeric) the timestamp (seconds since Unix epoch) of the oldest pre-generated key in the key pool
  "keypoolsize": xxxx,               (numeric) how many new keys are pre-generated (only counts external keys)
  "keypoolsize_hd_internal": xxxx,   (numeric) how many new keys are pre-generated for internal use (used for change outputs, only appears if the wallet is using this feature, otherwise external keys are used)
  "unlocked_until": ttt,             (numeric) the timestamp in seconds since epoch (midnight Jan 1 1970 GMT) that the wallet is unlocked for transfers, or 0 if the wallet is locked
  "paytxfee": x.xxxx,                (numeric) the transaction fee configuration, set in BTC/kB
  "hdseedid": "<hash160>"            (string, optional) the Hash160 of the HD seed (only present when HD is enabled)
  "private_keys_enabled": true|false (boolean) false if privatekeys are disabled for this wallet (enforced watch-only wallet)
}

Examples:
> bitcoin-cli getwalletinfo 
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getwalletinfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

getunconfirmedbalance

0.00000000

help getunconfirmedbalance

getunconfirmedbalance
DEPRECATED
Identical to getwalletinfo().balances.mine.untrusted_pending

getunconfirmedbalance true

error code: -1
error message:
getunconfirmedbalance
DEPRECATED
Identical to getwalletinfo().balances.mine.untrusted_pending

[promag]

Concept ACK either way (getbalances or here). I tend to prefer getbalances though.

[luke-jr]

This seems redundant with JSON-RPC batching?

[maflcko]

@luke-jr It would be horribly redundant to provide a separate rpc for each balance type, when it comes at no cost to return them all.

[luke-jr]

Perhaps just a getbalances as @jnewbery suggested then?

[promag]

Having a separate RPC makes getwalletinfo constant (after removing deprecated fields).

[maflcko]

Ok, going to rework to make this a fresh getbalances call (not to be confused with getbalance)

[jnewbery]

This seems redundant with JSON-RPC batching?

Note that batched RPCs are not atomic (and nor could they ever be, since the RPC server can't take the main lock).

[promag]

I also think all wallet get- and list- methods should return a best block hash, so it's clear at what point the returned values are valid.

Agree with @jnewbery, it can be useful.

[jnewbery]

utACK eeee149

[luke-jr]

PR title/description needs update

[jnewbery]

utACK facfb41

Only change is adding "DEPRECATED" to the balance descriptions in getwalletinfo and replacing the despicable * with a glorious - in the release notes.

[laanwj]

lightly tested ACK facfb41

[laanwj]

Good call on referring to the RPC help for details. This is what the release notes should do instead of substitute for documentation 👍

[jonatack]

Post-merge tested ACK a3d2d6b

[promag]

utACK facfb41.