Show watchonly balance only for Legacy wallets #653
Conversation
Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets.
hebasto
changed the title
|
Hmm, maybe we should hide the "Balance" for watch-only descriptor wallets, and show the right thing as watch-only? |
|
eg --- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -702,7 +702,9 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances)
if (model->wallet().hasExternalSigner()) {
ui->labelBalanceName->setText(tr("External balance:"));
} else if (model->wallet().privateKeysDisabled()) {
- balance = balances.watch_only_balance;
+ if (model->wallet().isLegacy()) {
+ balance = balances.watch_only_balance;
+ }
ui->labelBalanceName->setText(tr("Watch-only balance:"));
}
ui->labelBalance->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), balance)); |
There was a problem hiding this comment.
Tested ACK (using signet).
Reproduced the issue (watchonly descritor wallets with a balance show watch-only balance 0 on the Send window):
I've verified that it has been fixed with this change (now it shows balance with the total amount on the Send window matching the one shown on the overview window):
Checked also on a descriptor watch-only blank wallet and using importdescriptors function to have some balance within.
As @luke-jr, we could be still showing "watch-only balance" for watch-only descriptor wallets with the balance amount as shown in the overview window instead of the current solution, reafirming that we are on a watch-only wallet.
On @hernanmarino's comment, I think it makes sense to grey out the Receive button when the wallets can't generate addresses avoiding displaying the whole thing:
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. |
There was a problem hiding this comment.
@achow101 are you planning to move ahead with this? any stoppers?
Just waiting for reviewers. |
There was a problem hiding this comment.
ACK fdb8dc8
Inlines the balance presented in the send screen with the balance presented in the overview screen.
Not for this PR but would be good to have a visual distinction between wallets with private keys enabled vs wallets without private keys enabled (The previous "watch-only" wording wasn't that bad to fulfill the purpose).
…llets fdb8dc8 gui: Show watchonly balance only for Legacy wallets (Andrew Chow) Pull request description: Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets. The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC" ACKs for top commit: johnny9: tACK fdb8dc8 furszy: ACK fdb8dc8 hebasto: ACK fdb8dc8 Tree-SHA512: e5c0703a62d25c881c8dadfb9cffd482791f3d437a4ec5ae0088ce1a2069c2455ad6d3ec6c95a4404a3b55fbd727f92694529c35052236951553ca90c4ed31b5
Descriptor wallets do not have a watchonly balance as wallets are designated watchonly or not. Thus we should not be displaying the empty watchonly balance for descriptor wallets.
The result is that instead of the send page showing "Watch-only balance: 0.00000000 BTC" for watchonly descriptor wallets, we see the actual balance as "Balance: 10.00000000 BTC"