getwalletinfo rpc return incorrect data

[leehyong]

hello guys, When I use getwalletinfo rpc to query data, I suspect that balance is equal to unconfirmed_balance according to the returned data, which may be wrong. So I lookup the code about getwalletinfo, which is as follows in the getWalletInfo function body:

return {
      walletid: wallet.id,
      walletversion: 6,
      balance: Amount.coin(balance.unconfirmed, true),
      unconfirmed_balance: Amount.coin(balance.unconfirmed, true),
      txcount: balance.tx,
      keypoololdest: 0,
      keypoolsize: 0,
      unlocked_until: wallet.master.until,
      paytxfee: Amount.coin(this.wdb.feeRate, true)
    };

Is this a bug ?

[pinheadmz]

So, the hsd RPC API is a port from bcoin, which was based on Bitcoin Core's RPC API from several years ago. A lot of the quirks in the RPC are due to managing compatibility with Bitcoin Core from that era. With that in mind, consider that bcoin and hsd have an unusual definition for "unconfirmed" balance. From the API docs:

In hsd balances, confirmed refers to the total balance of coins confirmed in the blockchain. unconfirmed refers to that total IN ADDITION to any transactions still unconfirmed in the mempool. Another way to think about it is your unconfirmed balance is the FUTURE total value of your wallet after everything is confirmed.

So I think this was actually by design to conform the bcoin definition of "unconfirmed" to sort of work with what is expected from the Bitcoin Core RPC.

I'm not opposed to changing this RPC output since we don't need to match anything from Bitcoin in hsd. Feel free to open a pull request and we can try to get community feedback on how this data should be returned.

[ca98am79]

This may be a separate discussion, but it would be nice to add available_balance as both balance and unconfirmed_balance take into account all HNS that is burned / locked up and unavailable to use in any way.

[ca98am79]

@pinheadmz it is by design that these two lines are exactly the same?

balance: Amount.coin(balance.unconfirmed, true),
unconfirmed_balance: Amount.coin(balance.unconfirmed, true),

it seems redundant

[pinheadmz]

Like I said above, it may have been implemented that way back in bcoin to resemble, somewhat, the expected behavior from Bitcoin Core, where the term "unconfirmed" means something different. Or it could be a mistake. Either way, if there is community consensus around an API change, a pull request here would be one line and easy to test.