Electron-cash: 'Used' list pops open on incoming txes (intermittent/hard to reproduce)

[markblundeberg]

@kyuupichan noticed this the other day, and I noticed it today as well. @cculianu says it is not happening on Mac.

Steps to reproduce:

• Have Addresses tab open with a wallet that has used addresses (mine has addresses 0-14 and 16-24 used), with used addresses hidden.
• Receive a tx on an unused address, while you have said address selected.
• Observe that the used addresses list gets unhidden.

[cculianu]

This doesn't happen on macOS which is why I didn't know about it. I would have fixed it if it had.

Here is an example -- I kept it selected, sent from 1 wallet to this one and no problem:

Must be a Linux thing. I'll boot up my Linux and try it on there and see if I can figure it out.

Thanks Mark.

[kyuupichan]

I run Mac; so it happens on Mac

[cculianu]

Well then it's not Mac-specific but I can't even reproduce it on Linux or Mac now.

Here is a sample movie you can download that shows you I can't get it to happen:

https://c3-soft.com/downloads/BitcoinCash/ec_used_bug_how_2_reproduce.mov

If you guys can figure out what circumstances it happens in, it would help.

If not I'll continue to study this code that tries to preserve the address-list state, and see if I can see any obvious errors in certain corner cases.

[cculianu]

One more movie with me futzing around trying to reproduce it. If you can figure out exactly what combination of used/unused addresses I need to do it, please do let me know!

https://c3-soft.com/downloads/BitcoinCash/ec_address_bug_cant_reproduce_2.mov

[cculianu]

@kyuupichan @markblundeberg

I created a PR that I think should at least correctly enforce the rule that the previous tree state (expanded/collapsed) cannot be violated by a refresh. The previous code didn't always guarantee that if the current item moved around in the tree: setCurrentItem() would sometimes auto-expand parent tree items.

[cculianu]

Please try this PR #1043 and let me know if it addresses the issues you've observed.

[markblundeberg]

Yup seems to be fixed now.

[cculianu]

Aye. Awesome! Thanks for checking.

I can conclude the problem was whomever wrote that previous code likely wasn't aware of the (perhaps unintuitive) fact that setCurrentItem() on a collapsed QTreeWidgetItem parent would cause the parent to expand out as a "convenience"... so their usage wasn't maintaining the invariant they thought they were maintaining.

Good to know it's solved.