Add GetFullBalance() to compute used balance in getbalances RPC and on GUI [GUI PR #775](https://github.com/bitcoin-core/gui/pull/775)
Check #28776 (review) by @achow101
However, I think it would be better to move all of this into
GetBalanceitself and just have it always compute the used balance for us rather than having the caller do this extra computation.
used balance from GetBalance(...)
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/29062.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | pablomartin4btc |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
302@@ -303,18 +303,23 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
303 const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)};
304 const int tx_depth{wallet.GetTxDepthInMainChain(wtx)};
305 const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | reuse_filter)};
306+ const CAmount tx_credit_mine_used{
307+ avoid_reuse ? CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | ISMINE_USED) : tx_credit_mine};
have it always compute the used balance for us rather than having the caller do this extra computation.
My understanding of this part of @achow101 comment was to move the calculation for the used balance to GetBalance(...)
I have updated the PR and added GetFullBalance() which adds used balance to GetBalance(...)
289@@ -290,6 +290,15 @@ bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx)
290 return CachedTxIsTrusted(wallet, wtx, trusted_parents);
291 }
292
293+Balance GetFullBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) {
GetBalance include m_mine_used. It’s up to the caller to figure out what to do with that information.
GetBalance to include m_mine_used
303@@ -304,17 +304,23 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
304 const int tx_depth{wallet.GetTxDepthInMainChain(wtx)};
305 const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | reuse_filter)};
306 const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_WATCH_ONLY | reuse_filter)};
307+ const CAmount tx_credit_mine_used{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | ISMINE_USED)};
308 if (is_trusted && tx_depth >= min_depth) {
309 ret.m_mine_trusted += tx_credit_mine;
310 ret.m_watchonly_trusted += tx_credit_watchonly;
311 }
312+ if (is_trusted) {
if so that the min_depth check is still being done.
My understanding from the above line is min_depth should be zero when calculating full_balance hence I didn’t include the statement below in the above if since it uses min_depth provided by the caller
I didn’t include the check in this if since tx_depth is always >= 0
m_mine_used without respecting min_depth. The original code passes 0 for min_depth because that is the default value but it needs to pass the avoid_reuse flag. It is not because used balance should always calculated with a min depth of 0. Ignoring it is a layer violation here.
318+ ret.m_mine_used += tx_credit_mine_used;
319 }
320 ret.m_mine_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE);
321 ret.m_watchonly_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_WATCH_ONLY);
322 }
323+ ret.m_mine_used -= (ret.m_mine_trusted + ret.m_mine_untrusted_pending);
309+ ret.m_mine_used += tx_credit_mine_used;
310+ }
311 if (is_trusted && tx_depth >= min_depth) {
312 ret.m_mine_trusted += tx_credit_mine;
313 ret.m_watchonly_trusted += tx_credit_watchonly;
314+ ret.m_mine_used -= tx_credit_mine; // remove overlap from used_balance
avoid_reuse flag is passed, tx_credit_mine and tx_credit_mine_used will be the same so the returned used_balance will be 0. That seems incorrect.
