hw_wallet/qt: reuse message dialog to fix lag between device prompts#10718
Open
sashazykov wants to merge 1 commit into
Open
hw_wallet/qt: reuse message dialog to fix lag between device prompts#10718sashazykov wants to merge 1 commit into
sashazykov wants to merge 1 commit into
Conversation
QtHandlerBase.message_dialog() used to tear down the current dialog (clear_dialog -> QDialog.accept) and build a brand-new WindowModalDialog on every device button request. A device emits one button request per output, so signing a tx with many outputs rebuilt the dialog once per output. On macOS a window-modal QDialog is shown as an animated "sheet", so the popup visibly slid closed and reopened for each output, and the GUI churn also competed with the single hardware-comms thread for the GIL, adding latency between device prompts. Reuse the open dialog and just update its label text when one is already showing (title and on_cancel are stable within a signing flow). This also smooths Ledger's repeated progress messages, which update the text on each call.
734aeed to
898a4c2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signing a transaction with many outputs on a Trezor is noticeably laggy: after I confirm one output on the device, there's a short pause before the next output shows up on the device screen.
What pointed me at Electrum's GUI rather than the device or the library: I have an Android signer (welldone-tech/satoshi-signer) built on the same
trezorlib, and it has no such delay — confirming an output brings up the next one instantly. Same library, same device, so the bottleneck is on Electrum's host/UI side.Symptom
When signing a tx with many outputs, each output confirmation on the device is preceded by a visible pause. On macOS you can see why: the "check your device" popup slides closed and a brand-new one slides open for every output, instead of staying put.
Root cause
QtHandlerBase.message_dialog()tore down the current dialog (clear_dialog()→QDialog.accept()) and built a freshWindowModalDialogon every device button request. A device emits one button request per output, so signing a tx with N outputs rebuilds the dialog N times. A window-modalQDialogis rendered as an animated "sheet" on macOS, so the popup visibly slides closed and reopens on each request. That GUI churn also competes with the (single) hardware-comms thread for the GIL, adding latency between device prompts.Fix
Reuse the already-open dialog and just update its label text when one is showing, instead of tearing it down and rebuilding. The window title and
on_cancelhandler are stable within a signing flow, so the only thing that changes between requests is the message text. If a new request comes in with a differenton_cancel, we still rebuild (the existing dialog's cancel wiring would otherwise be wrong).This also smooths Ledger's repeated progress messages (
Signing transaction... (phase2, i/N)), which update the label text on each call.Testing
Tested on macOS with a Trezor: signing a many-output transaction no longer slides the popup closed/open between outputs, and the lag between prompts is gone — outputs now appear back-to-back.
(I also have a small headless unit test for the dialog-reuse logic if you'd like the suite to start covering this — left out by default, since it would be the first
QtWidgets-based test in the tree and pulls in a newQT_QPA_PLATFORM=offscreensetup.)