gui: Fix unintialized WalletView::progressDialog #18062

1. 
   promag commented at 8:05 PM on February 3, 2020: member

   #17911 shows that it's possible to read the unintialized progressDialog in https://github.com/bitcoin/bitcoin/blob/f32564f0a73c5ad1a107dd112e40516f39d1a51e/src/qt/walletview.cpp#L296-L297.

   And the debugger shows

   (gdb) bt
   [#0](/github-metadata-backup-bitcoin-bitcoin/0/)  0x0000555556687c60 in QProgressDialog::wasCanceled() const ()
   [#1](/github-metadata-backup-bitcoin-bitcoin/1/)  0x000055555572989f in WalletView::showProgress (this=0x5555577d7a70, 
       title=..., nProgress=1) at qt/walletview.cpp:322
   

   Closes #17911.

2. gui: Fix unintialized WalletView::progressDialog acf8abc7f3
3. 
   promag commented at 8:07 PM on February 3, 2020: member

   Needs backport. @practicalswift any idea how this was unnoticed?

4. DrahtBot added the label GUI on Feb 3, 2020
5. 
   elichai commented at 4:06 PM on February 4, 2020: contributor

   So the actual problem this solves is here https://github.com/bitcoin/bitcoin/blob/ef8e2cee9f5d157eeb3139b64e9c3a5fa4bf36f3/src/qt/bitcoingui.cpp#L1337 and line 1342. right? it's because we check if the pointer is null or not but we never set it to null, so it either contains a real pointer or uninitialized values.

6. 
   promag commented at 4:07 PM on February 4, 2020: member

   @elichai that is correct.

7. elichai approved
8. 
   elichai commented at 4:18 PM on February 4, 2020: contributor

   utACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582

9. kristapsk approved
10. 
    kristapsk commented at 7:43 PM on February 4, 2020: contributor

    ACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582

11. 
    promag commented at 10:32 PM on February 4, 2020: member

    FWIW @bitcoinhodler tested this, see #17911 (comment).

12. 
    fanquake commented at 10:32 PM on February 4, 2020: member

    @promag Can you add the explanation of what this is fixing and how it's fixing it to the PR description; given it's not in there already and not in the commit message either.

13. 
    promag commented at 10:46 PM on February 4, 2020: member

    @elichai actually the code you point out is wrong, see the updated OP.

    The code you point is fine because the dialog is already initialized.

14. 
    elichai commented at 9:27 AM on February 5, 2020: contributor

    @elichai actually the code you point out is wrong, see the updated OP.

    The code you point is fine because the dialog is already initialized.

    Why? both have an initialization if nProgress == 0, and both have an else if (progressDialog) how do they differ?

15. 
    promag commented at 9:57 AM on February 5, 2020: member

    @elichai sorry for not being clear, the code you point out is fine, just that it's not where the bug is.

    Note that BitcoinGUI::progressDialog is already initialized: https://github.com/bitcoin/bitcoin/blob/a064e005fa6b2b9f2c7f01e45a04a4d740f4605d/src/qt/bitcoingui.h#L125

16. 
    MarcoFalke commented at 11:57 AM on February 5, 2020: member

    ACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582

    Nice find. Remind me that we should finally get the functional tests run with the gui.

    Does this need backport?

17. 
    promag commented at 12:01 PM on February 5, 2020: member

    @MarcoFalke yes.

18. fanquake added the label Needs backport (0.19) on Feb 5, 2020
19. MarcoFalke added this to the milestone 0.19.1 on Feb 5, 2020
20. hebasto approved
21. 
    hebasto commented at 9:47 PM on February 5, 2020: member

    ACK acf8abc7f3cf7efa418a46f9f69f23f1a5035582, I have reviewed the code and it looks OK, I agree it can be merged.

22. fanquake referenced this in commit 23fab1a3df on Feb 6, 2020
23. fanquake merged this on Feb 6, 2020
24. fanquake closed this on Feb 6, 2020

    ---

25. promag deleted the branch on Feb 6, 2020
26. promag referenced this in commit b4e5363ccc on Feb 6, 2020
27. promag cross-referenced this on Feb 6, 2020 from issue 0.19: gui: Fix unintialized WalletView::progressDialog by promag
28. laanwj removed the label Needs backport (0.19) on Feb 6, 2020
29. fanquake referenced this in commit 7d53995ff2 on Feb 6, 2020
30. MarkLTZ referenced this in commit d99bc96496 on Feb 13, 2020
31. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
32. Fabcien referenced this in commit 88b93a4375 on Aug 24, 2021
33. Munkybooty referenced this in commit 5b48477e16 on Dec 9, 2021
34. Munkybooty referenced this in commit 38e606465d on Dec 9, 2021
35. Munkybooty referenced this in commit 7f8344fd86 on Dec 9, 2021
36. Munkybooty referenced this in commit 8f846bdab9 on Dec 23, 2021
37. bitcoin locked this on Feb 15, 2022

Milestone
0.19.1