wizard: handle UserFacingException in WCWalletPasswordHardware
What the developer wrote
Authored by f321x
wizard: handle UserFacingException in WCWalletPasswordHardware
Handles `UserFacingException` in the `WCWalletPasswordHardware` step of
the hardware wallet wizard flow. This fixes the previous FIXME and
prevents the crash reporter from getting triggered if the the user e.g.
disconnects his hardware wallet during the wallet encryption step.
What changed, and why it matters
This commit fixes a bug in Electrum's wallet setup wizard where unplugging a hardware wallet during the encryption step could crash the application by opening an error-report dialog. The fix catches the expected error gracefully, shows a friendly message, and lets the user retry instead of triggering the crash reporter.
No immediate user action required; this is a quality/defensive fix. Users relying on hardware-wallet encryption should update to a release containing this commit. Reviewers may want to verify that self.error is surfaced correctly in the wizard UI and that the busy indicator is always cleared.
Security signals we found
Unhandled exception path removed from user-facing wizard flow
UserFacingException now caught instead of propagating to crash reporter
Null/None client dereference risk removed
Background threading introduced for hardware wallet I/O
FIXME comment referencing disconnect-related crash removed
Evidence from the diff
In WCWalletPasswordHardware, the previous code retrieved the hardware-derived storage-encryption password synchronously inside apply(), without handling a None client or exceptions from a just-disconnected device. The patch moves password retrieval into an on_ready() background thread, catches UserFacingException, sets self.error and self.valid=False, and skips applying encryption data when invalid. This prevents an unhandled exception from reaching the crash reporter.
Changed components
electrum/gui/qt/wizard/wallet.pyWCWalletPasswordHardware wizard componentHardware wallet setup/encryption flowInspect captured patch +29 / −8
diff --git a/electrum/gui/qt/wizard/wallet.py b/electrum/gui/qt/wizard/wallet.py
index 6d0926b..8397b7b 100644
--- a/electrum/gui/qt/wizard/wallet.py
+++ b/electrum/gui/qt/wizard/wallet.py
@@ -1275,19 +1275,40 @@ class WCWalletPasswordHardware(WalletWizardComponent):
self.layout().addLayout(self.playout.layout())
self.layout().addStretch(1)
- self._valid = True
+ self._hw_password = None # type: Optional[str]
+ self._valid = False
+
+ def on_ready(self):
+ _name, info = self.wizard_data['hardware_device']
+ device_id = info.device.id_
+ client = self.plugins.device_manager.client_by_id(device_id, scan_now=False)
+ if client is None:
+ self.valid = False
+ self.error = _("Client for hardware device was unpaired.")
+ return
+
+ def retrieve_password_task():
+ try:
+ self._hw_password = client.get_password_for_storage_encryption()
+ self.valid = True
+ except UserFacingException as e:
+ self.error = str(e)
+ self.valid = False
+ finally:
+ self.busy = False
+
+ self.busy = True
+ t = threading.Thread(target=retrieve_password_task, daemon=True)
+ t.start()
def apply(self):
+ if not self.valid:
+ return
self.wizard_data['encrypt'] = True
if self.playout.should_encrypt_storage_with_xpub():
self.wizard_data['xpub_encrypt'] = True
- _name, _info = self.wizard_data['hardware_device']
- device_id = _info.device.id_
- client = self.plugins.device_manager.client_by_id(device_id, scan_now=False)
- # client.handler = self.plugin.create_handler(self.wizard)
- # FIXME client can be None if it was recently disconnected.
- # also, even if not None, this might raise (e.g. if it disconnected *just now*):
- self.wizard_data['password'] = client.get_password_for_storage_encryption()
+ assert self._hw_password
+ self.wizard_data['password'] = self._hw_password
else:
self.wizard_data['xpub_encrypt'] = False
self.wizard_data['password'] = self.playout.new_password()
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.