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”
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.
eg
0--- a/src/qt/sendcoinsdialog.cpp
1+++ b/src/qt/sendcoinsdialog.cpp
2@@ -702,7 +702,9 @@ void SendCoinsDialog::setBalance(const interfaces::WalletBalances& balances)
3 if (model->wallet().hasExternalSigner()) {
4 ui->labelBalanceName->setText(tr("External balance:"));
5 } else if (model->wallet().privateKeysDisabled()) {
6- balance = balances.watch_only_balance;
7+ if (model->wallet().isLegacy()) {
8+ balance = balances.watch_only_balance;
9+ }
10 ui->labelBalanceName->setText(tr("Watch-only balance:"));
11 }
12 ui->labelBalance->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), balance));
tACK.
This might be slightly off-topic, but shouldn’t the whole “Receive” dialog be disabled when the wallet is unable to generate addresses ? Currently, only the receive button is disabled.
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:
tACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
Tested on signet by creating a no-private-key descriptor wallet and then using the importdescriptor command to add an address with funds on it.
on master (65de8eeeca29e71378aa34602b287ab921b040e4): Send dialog shows invalid watch-only balance
PR (fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e): Send dialog shows correct balance
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | johnny9, furszy, hebasto |
| Concept ACK | hernanmarino, pablomartin4btc |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
ACK fdb8dc8a
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).
ACK fdb8dc8a5a10c39927dc4b8ddc5e0038a633c50e
Comments about further UX improving could be addressed in follow ups.