gui: show watch-only balance in send screen #17587

1. 
   Sjors commented at 10:39 am on November 25, 2019: member

   Now that we can create a PSBT from a watch-only wallet (#16944), we should also display the watch-only balance on the send screen.

   Before:

   After:

   I added a test to check the balance on the send screen, but it only covers regular wallets. A better would add a watch-only only wallet.

2. DrahtBot added the label GUI on Nov 25, 2019
3. DrahtBot added the label Tests on Nov 25, 2019
4. 
   promag commented at 2:31 pm on November 25, 2019: member

   ACK 0a0f3676d0fe755c91693ecfdb1cb48608fb1fdc.

   A better would add a watch-only only wallet.

   Indeed, and it would be even better if added in this PR. I can write it for you.

5. 
   Sjors commented at 2:47 pm on November 25, 2019: member

   @promag I created #17588 as a reminder, but if you feel like it…
6. 
   promag commented at 2:50 pm on November 25, 2019: member

   Ok, considering that issue I think this is ready to merge. The only nit I can think of is to change the bottom label to “watch only balance” or something like that.
7. 
   Sjors commented at 3:03 pm on November 25, 2019: member

   Maybe. But there’s already a watch-only icon (the eye) right below the balance, and this feature is exclusively for watch-only wallets.
8. 
   jonasschnelli commented at 2:20 am on November 26, 2019: contributor

   What if the wallet has watch only and hot balances? Maybe display both in a such case and if the wallet only has a watch-only balance, label it correctly?
9. 
   gwillen commented at 9:38 am on November 26, 2019: contributor

   Whoops, I went to go make this change and discovered this PR open! :-) @Sjors, I am hoping to start breaking my old giant offline branch into manageable pieces and start PRing it on top of your work – we should coordinate so we aren’t both trying to do it.

   I do think the label should change; I think displaying both balances (which is what I had done in my old branch) is less compatible with the way this was handled in Sjors’ other PR, since all the new logic only triggers if the wallet has private keys disabled, which forbids having a hot balance.

10. [test] qt: add send screen balance test 2689c8fd71
11. 
    Sjors commented at 10:43 am on November 26, 2019: member

    I renamed the balance label. @jonasschnelli there’s no PSBT functionality in wallets with private keys, hence if (model->privateKeysDisabled()). Those wallets can’t spend from their watch-only address in the GUI, and so it makes sense to not display the watch-only balance on the send screen (it’s shown on the Overview screen).

    In general I think the plan is to encourage creating separate wallets. @gwillen I’ve been taking bits and pieces from your offline project, see also #17509.

12. [gui] send: show watch-only balance in send screen 4a96e459d7
13. Sjors force-pushed on Nov 26, 2019
14. 
    laanwj commented at 12:07 pm on November 26, 2019: member

    Concept ACK.

    there’s no PSBT functionality in wallets with private keys, hence if (model->privateKeysDisabled())

    I really like @Sjors keeping this simple and displaying only one balance (the most relevant one for the purpose). For a full list of balances there’s the overview page.

15. 
    gwillen commented at 6:24 pm on November 26, 2019: contributor

    @Sjors maybe we could chat on IRC about it, I am back from my sabbatical and hoping to resume this work myself. Sorry for leaving it hanging for so long!
16. 
    meshcollider commented at 8:30 am on November 27, 2019: contributor

    utACK 4a96e459d733f1b6427221aaa1874ea00f79988a
17. in src/qt/test/wallettests.cpp:174 in 2689c8fd71 outdated

    169@@ -170,6 +170,16 @@ void TestGUI(interfaces::Node& node)
    170     sendCoinsDialog.setModel(&walletModel);
    171     transactionView.setModel(&walletModel);
    172 
    173+    {
    174+        // Check balance in send dialog
    

    ---

    ---

    

    instagibbs commented at 3:03 pm on November 27, 2019:

    just noting this test won’t catch the current behavior, that’s ok for now

    ---

    

    promag commented at 10:11 pm on November 28, 2019:

    Yup #17588.
18. instagibbs approved
19. 
    instagibbs commented at 3:05 pm on November 27, 2019: member

    code review and light test ACK https://github.com/bitcoin/bitcoin/pull/17587/commits/4a96e459d733f1b6427221aaa1874ea00f79988a

    single, most-relevant balance is the best way to go. I’m doing it for all my other related PRs.

20. 
    jb55 commented at 4:35 pm on November 27, 2019: member

    utACK 4a96e459d733f1b6427221aaa1874ea00f79988a
21. 
    promag commented at 10:13 pm on November 28, 2019: member

    reACK 4a96e45, rebased and label change since last review.
22. meshcollider referenced this in commit abb30de63f on Nov 29, 2019
23. meshcollider merged this on Nov 29, 2019
24. meshcollider closed this on Nov 29, 2019

    ---

25. Sjors deleted the branch on Nov 29, 2019
26. MarkLTZ referenced this in commit a53a2cb9a1 on Nov 29, 2019
27. sidhujag referenced this in commit a289b8cba8 on Nov 30, 2019
28. luke-jr referenced this in commit fd158a64b9 on Jan 5, 2020
29. jasonbcox referenced this in commit a690e66ad2 on Oct 23, 2020
30. sidhujag referenced this in commit b9a1d85c52 on Nov 10, 2020
31. MarcoFalke locked this on Dec 16, 2021

Contributors
Sjors promag jonasschnelli gwillen laanwj meshcollider instagibbs jb55

Labels
GUI Tests