Wallet: Functions to enable adding used balance to GUI overview page #28776
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. ConflictsNo conflicts as of last run. |
|
Concept ACK |
There was a problem hiding this comment.
Concept ACK
Light CR, left a suggestion if I understand it correctly.
Also, since this is a very small change on the wallet interface, perhaps makes more sense to add this commit first on the GUI PR #775 and would be easier to test the gui as well.
| @@ -410,6 +410,11 @@ class WalletImpl : public Wallet | |||
| result.unconfirmed_watch_only_balance = bal.m_watchonly_untrusted_pending; | |||
| result.immature_watch_only_balance = bal.m_watchonly_immature; | |||
| } | |||
| if (m_wallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { | |||
| const auto full_bal = GetBalance(*m_wallet, 0, false); | |||
There was a problem hiding this comment.
GetBalance receives bool avoid_reuse in the last argument, should not be true or just remove the if condition directly and pass the result of IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE) entirely.
| const auto full_bal = GetBalance(*m_wallet, 0, false); | |
| const auto full_bal = GetBalance(*m_wallet, 0, true); |
There was a problem hiding this comment.
That parameter is slightly misleading. When set to true, it will ignore the used balance, so it needs to be false here in order to get the full balance to calculate the used balance. This is what getbalances does.
However, I think it would be better to move all of this into GetBalance itself and just have it always compute the used balance for us rather than having the caller do this extra computation.
There was a problem hiding this comment.
Sorry, it's clear, I was incorrect in my statement, but what I meant is we could use the flag instead of false (e.g.: GetBalance(*m_wallet, 0, !(m_wallet->IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)));) and remove the if.
I agree with moving this piece into GetBalance, the other only place where this is called with this argument is in coins.cpp.
| @@ -280,6 +280,9 @@ class Wallet | |||
| //! Return whether is a legacy wallet | |||
| virtual bool isLegacy() = 0; | |||
|
|
|||
| //Return whether wallet has 'avoid_reuse' flag set | |||
| virtual bool isAvoidReuseEnabled() = 0; | |||
There was a problem hiding this comment.
Interface functions that are newly introduced or are only called by the GUI can be handled in the GUI repo. Closing for now, this can be moved into the GUI pull.
First Part: Fixes bitcoin-core/gui#769
Functions to enable adding used balance to the overview page for wallets with the avoid_reuse flag
Dependency