Wallet: (Refactor) GetBalance to calculate used balance #29062
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, 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. |
BrandonOdiwuor
changed the title
src/wallet/receive.cpp
Outdated
| @@ -303,18 +303,23 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse) | |||
| const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)}; | |||
| const int tx_depth{wallet.GetTxDepthInMainChain(wtx)}; | |||
| const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | reuse_filter)}; | |||
| const CAmount tx_credit_mine_used{ | |||
| avoid_reuse ? CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | ISMINE_USED) : tx_credit_mine}; | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
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(...)
BrandonOdiwuor
force-pushed
the
getbalance-add-used
branch
from
19437c3
to
6a0272b
Compare
BrandonOdiwuor
changed the title
BrandonOdiwuor
force-pushed
the
getbalance-add-used
branch
from
6a0272b
to
c69dd7b
Compare
BrandonOdiwuor
changed the title
src/wallet/receive.cpp
Outdated
| if (is_trusted && tx_depth >= min_depth) { | ||
| ret.m_mine_trusted += tx_credit_mine; | ||
| ret.m_watchonly_trusted += tx_credit_watchonly; | ||
| } | ||
| if (is_trusted) { |
There was a problem hiding this comment.
This needs to be in the above if so that the min_depth check is still being done.
There was a problem hiding this comment.
bitcoin/src/wallet/rpc/coins.cpp
Line 480 in a46065e
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
src/wallet/receive.cpp
Outdated
| } | ||
| ret.m_mine_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE); | ||
| ret.m_watchonly_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_WATCH_ONLY); | ||
| } | ||
| ret.m_mine_used -= (ret.m_mine_trusted + ret.m_mine_untrusted_pending); |
There was a problem hiding this comment.
It would be a bit easier to understand if this subtraction was done with each addition during the loop, rather than all at the end. Also a comment for why this is being done would be useful, as it was not immediately obvious to me at first that the purpose of this is to remove the overlap.
There was a problem hiding this comment.
@achow101 do the added comments make the PR easy to follow?
BrandonOdiwuor
force-pushed
the
getbalance-add-used
branch
from
c69dd7b
to
1cc2a81
Compare
Add
GetFullBalance()to computeusedbalance ingetbalances RPCand on GUI GUI PR #775Check #28776 (comment) by @achow101
Update:
usedbalance fromGetBalance(...)