gui: make sure to update the UI when deleting a transaction #16952

1. 
   jonasschnelli commented at 9:19 AM on September 24, 2019: contributor

   CWallet::ZapSelectTx removes transactions from the internal model, but leaves the UI in the dark. Adding a NotifyTransactionChanged() should avoid having invalid transactions in the GUI.

   Fixes #16950

2. make sure to update the UI when deleting a transaction addaf8af82
3. jonasschnelli added this to the milestone 0.19.0 on Sep 24, 2019
4. jonasschnelli added the label GUI on Sep 24, 2019
5. fanquake renamed this:
   make sure to update the UI when deleting a transaction
   gui: make sure to update the UI when deleting a transaction
   on Sep 24, 2019
6. kristapsk approved
7. 
   kristapsk commented at 8:30 PM on September 24, 2019: contributor

   ACK addaf8af8268d918973883a304025d40af5a33c1 (tested both with and without this change)

8. 
   promag commented at 10:58 PM on September 25, 2019: member

   Looks like the UI model is not updated if fQueueNotifications, which happens if anything is in progress: https://github.com/bitcoin/bitcoin/blob/4c4ff4911a6dbf1555fb4cfc05092cb8fe2f9eba/src/qt/transactiontablemodel.cpp#L707-L722

   Couldn't we trigger the crash in this case?

9. 
   fanquake commented at 11:27 AM on September 26, 2019: member

   ACK addaf8af8268d918973883a304025d40af5a33c1 - tested that this fixes #16950

   Couldn't we trigger the crash in this case?

   Yes you can. Using this branch (addaf8af8268d918973883a304025d40af5a33c1), I triggered a rescan, called removeprunedfunds and tried opening the info dialog of the tx in the GUI:

   * thread [#1](/bitcoin-bitcoin/1/), name = 'bitcoin-main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x18)
       frame [#0](/bitcoin-bitcoin/0/): 0x00000001000bc638 bitcoin-qt`TransactionDesc::toHTML(interfaces::Node&, interfaces::Wallet&, TransactionRecord*, int) [inlined] std::__1::vector<CTxOut, std::__1::allocator<CTxOut> >::begin(this=0x0000000000000018 size=0) const at vector:1519:30 [opt]
      1516	typename vector<_Tp, _Allocator>::const_iterator
      1517	vector<_Tp, _Allocator>::begin() const _NOEXCEPT
      1518	{
   -> 1519	    return __make_iter(this->__begin_);
      1520	}
      1521	
      1522	template <class _Tp, class _Allocator>
   Target 0: (bitcoin-qt) stopped.
   

   However, I'm not sure fixing that (less likely to hit, and likely more involved to fix) case has to block this PR.

10. 
    promag commented at 11:45 AM on September 26, 2019: member

    Well maybe deletions shouldn't be queued?

    Agree it's unlikely. Thoughts Jonas?

11. fanquake added the label Waiting for author on Sep 27, 2019
12. 
    Sjors commented at 5:01 PM on September 27, 2019: member

    Perhaps removeprunedfunds should fail if a rescan is in progress (etc), but I think that can wait for a separate PR.

    tACK addaf8a: tested with an unpruned wallet by calling removeprunedfunds on an RBF-replaced transaction. It neatly disappears from the UI.

13. 
    promag commented at 9:11 PM on September 27, 2019: member

    Perhaps removeprunedfunds should fail if a rescan is in progress (etc), but I think that can wait for a separate PR.

    Just to avoid the problem in the GUI? AFAIK there's no problem calling removeprunedfunds while rescanning in bitcoind.

14. 
    Sjors commented at 8:10 AM on September 28, 2019: member

    Just in general so that concurrency is easier to reason about. But if it already works, don't fix it :-)

15. laanwj added the label Bug on Sep 30, 2019
16. laanwj referenced this in commit 8d39c636aa on Oct 2, 2019
17. laanwj merged this on Oct 2, 2019
18. laanwj closed this on Oct 2, 2019

    ---

19. sidhujag referenced this in commit 1e10122858 on Oct 2, 2019
20. laanwj removed the label Waiting for author on Oct 24, 2019
21. deadalnix referenced this in commit b1728a65f1 on Jul 28, 2020
22. PastaPastaPasta referenced this in commit 83607a5f83 on Jun 27, 2021
23. PastaPastaPasta referenced this in commit 27422a8a37 on Jun 28, 2021
24. PastaPastaPasta referenced this in commit 7a9f66f583 on Jun 29, 2021
25. PastaPastaPasta referenced this in commit da0cb2c727 on Jul 1, 2021
26. PastaPastaPasta referenced this in commit ac4e42225f on Jul 1, 2021
27. PastaPastaPasta referenced this in commit 828adbc249 on Jul 12, 2021
28. PastaPastaPasta referenced this in commit fc0279ad80 on Jul 13, 2021
29. PastaPastaPasta referenced this in commit ecf642bb8a on Jul 13, 2021
30. DrahtBot locked this on Dec 16, 2021

Contributors

jonasschnellikristapskpromagfanquakeSjors

Milestone
0.19.0