Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install Wizard: Add derivation path scanner #2199

Merged
merged 3 commits into from Mar 20, 2021

Conversation

jonas-lundqvist
Copy link

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.
time.sleep(0.1)
num_tx = len(wallet.get_history())
wallet.clear_history()
self.progress_cb(i, str(num_tx))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey man; this is touching the GUI from a thread that is not the main thread. That's UB.

The typical way to do this is you leverage Qt's signal/slot mechanism to emit a signal which gets delivered to the main thread. An example of this can be found in main_window.py where you have def on_network(): which is executed as a callback in the network thread, and all that does is immediately emit a signal that gets caught in the main thread in def on_network_qt():...

Here is more background on how to work with Qt across threads: https://doc.qt.io/qt-5/threads-qobject.html

Although QObject is reentrant, the GUI classes, notably QWidget and all its subclasses, are not reentrant. They can only be used from the main thread. As noted earlier, QCoreApplication::exec() must also be called from that thread.

In practice, the impossibility of using GUI classes in other threads than the main thread can easily be worked around ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey man; this is touching the GUI from a thread that is not the main thread. That's UB.
Oh, I see. That shouldn't be too hard to work around and I'll see if I get time for it this weekend.
Thanks for the review.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively I could also work on this and "finish it up" for you to handle this. I like this feature.. and I think it's a valuable addition. So. whomever makes it first wins I guess :)

The callback will not emit a signal and the table update will be done in
the main thread.

Also add early return if there isn't any network so we don't have to
mess around with any keys, wallets or stuff like that if we can't really
scan.
@cculianu
Copy link
Collaborator

Nice solution. Elegant. I love this feature. Thanks man!

@cculianu cculianu added the Qt label Mar 20, 2021
@cculianu cculianu merged commit 2372899 into Electron-Cash:master Mar 20, 2021
jcramer pushed a commit to simpleledger/Electron-Cash-SLP that referenced this pull request Mar 21, 2021
* Install Wizard: Add derivation path scanner

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.

* Use global network instance

* Do not touch GUI stuff from other thread

The callback will not emit a signal and the table update will be done in
the main thread.

Also add early return if there isn't any network so we don't have to
mess around with any keys, wallets or stuff like that if we can't really
scan.
PiRK pushed a commit to PiRK/ElectrumABC that referenced this pull request Apr 27, 2021
* Install Wizard: Add derivation path scanner

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.

* Use global network instance

* Do not touch GUI stuff from other thread

The callback will not emit a signal and the table update will be done in
the main thread.

Also add early return if there isn't any network so we don't have to
mess around with any keys, wallets or stuff like that if we can't really
scan.
PiRK pushed a commit to PiRK/ElectrumABC that referenced this pull request May 21, 2021
* Install Wizard: Add derivation path scanner

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.

* Use global network instance

* Do not touch GUI stuff from other thread

The callback will not emit a signal and the table update will be done in
the main thread.

Also add early return if there isn't any network so we don't have to
mess around with any keys, wallets or stuff like that if we can't really
scan.
PiRK pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jun 16, 2021
* Install Wizard: Add derivation path scanner

This allows a user to scan a set of known derivation paths for
transactions when restoring a wallet from a seed.

* Use global network instance

* Do not touch GUI stuff from other thread

The callback will not emit a signal and the table update will be done in
the main thread.

Also add early return if there isn't any network so we don't have to
mess around with any keys, wallets or stuff like that if we can't really
scan.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants