CW-1624: Trezor native integration cleanup (connect, restore, send) - #3631
Open
sethforprivacy wants to merge 30 commits into
Open
sethforprivacy wants to merge 30 commits into
sethforprivacy wants to merge 30 commits into
Conversation
…ingSettings` state in pairing flow
…stead-of-app # Conflicts: # pubspec.lock # res/values/strings_ar.arb # res/values/strings_bg.arb # res/values/strings_cs.arb # res/values/strings_es.arb # res/values/strings_fa.arb # res/values/strings_fr.arb # res/values/strings_gn.arb # res/values/strings_ha.arb # res/values/strings_hi.arb # res/values/strings_hr.arb # res/values/strings_hy.arb # res/values/strings_id.arb # res/values/strings_it.arb # res/values/strings_ja.arb # res/values/strings_ko.arb # res/values/strings_my.arb # res/values/strings_nl.arb # res/values/strings_pl.arb # res/values/strings_pt.arb # res/values/strings_ru.arb # res/values/strings_th.arb # res/values/strings_tl.arb # res/values/strings_tr.arb # res/values/strings_uk.arb # res/values/strings_ur.arb # res/values/strings_vi.arb # res/values/strings_yo.arb # res/values/strings_zh.arb
…d of re-asking Follow-up to #3601 (CW-1624). That PR correctly moves passphrase entry to connect time and removes the per-operation session, but sending (and key-image sync / swaps) still prompted for the passphrase inside the app: those flows push ConnectDevicePage whenever the Trezor is not on a live BLE session, and connectDevice() always ran the "Almost ready" settings sheet again. Nothing persisted the choice, so every reconnect re-asked, and the sheet defaulted to app-side entry. What changes: - HardwareWalletViewModel gains two no-op hooks: prepareReconnect(wallet) and rememberWalletSettings(wallet). - ConnectDevicePageParams gains an optional reconnectWallet. The page calls prepareReconnect(reconnectWallet) right before connectDevice(). Restore and new-wallet flows leave it null, so they still show the settings sheet. - TrezorConnectViewModel: * persists the TrezorDeviceSettings a session was created with, per wallet, in SecureStorage (mode "device" / "app" / "none" under com.cakewallet.trezor/passphrase_mode/<type>_<name>; app-side passphrase, when chosen, under com.cakewallet.trezor/passphrase/<type>_<name>); * on a reconnect for a known wallet, feeds those settings straight into createSession() and skips the in-app sheet. "device" mode re-creates the session with TrezorPassphrase.onDevice() (the device asks once, as it must), "app" mode with the stored value (silent), "none" creates no passphrase session; * saves the settings in initWallet() and rememberWalletSettings(), and clears them when a connect fails. - The preset is consumed at the top of connectDevice() so a failed attempt can never leak it into an unrelated later connect. - WalletHardwareRestoreViewModel.process() calls rememberWalletSettings() so a freshly restored wallet is covered before its first send. - Every reconnect call site passes reconnectWallet: new-UI send page, key-image sync sheet, swap confirm sheet, exchange trade page, legacy send page. - The settings sheet now defaults "Enter passphrase on device" to on when the passphrase toggle is enabled, matching the intent of #3601. Net effect on a Trezor Safe 7 Monero wallet with a passphrase: after the first setup, tapping Send no longer shows the in-app settings/passphrase sheet. If the BLE session is still up nothing is asked at all; if it has to reconnect, the device prompts once (device mode) or the stored passphrase is applied silently (app mode). Not covered here (separate issues): the missing timeout on THP requests that turns a dropped reply into a permanent "Proceed on device", and the Ledger-only signing-progress state for Monero. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…CW-1624) Follows up PR #3601 and the "reuse passphrase settings on reconnect" patch. Connect / pairing (TrezorConnectViewModel, ConnectDevicePage, pairing sheet): - Start every connection attempt from a fresh THP state rebuilt from the persisted pairing credentials. The in-memory state kept phase=paired across connects, so trezor_flutter skipped the handshake and talked on a channel the device had dropped -> one-off ThpUnallocatedChannel. A failed attempt no longer wipes the credentials from memory either. - Retry and exit now go through the pending connectDevice call (retryPairing / cancelPairing). Before, "Try again" fired a second connectDevice whose success never reached the page, and exiting the sheet left connectDevice awaiting a completer forever with isConnecting stuck. - Do not create a second passphrase session when the device has "passphrase always on device": createChannel() already bound one via the on-device prompt. Hide the passphrase options in that case and skip the settings sheet entirely when nothing is left to configure. - Only show the "Passphrase entry" state when a passphrase session is actually being created. - List already-connected BLE devices on the connect page (a connected peripheral does not advertise) and skip the page in the restore flow when the device is already connected; de-duplicate scan results. - Real copy for the auto-connect toggle subtitle; Trezor errors are surfaced via interpretErrorCode instead of the generic Ledger message. Restore: - SelectHardwareWalletAccountPage / MoneroHardwareWalletOptionsPage popped themselves on any Trezor error (all mapped to ledger_connection_error) and then showed the alert on a disposed context, dropping the user back to the device list with no message. Show the reason first. - Pin the Monero refresh height on hardware (from-keys / from-device) restores like the seed restore does; restoring "1 day ago" landed ~15k blocks back. Fix the no-op `date..subtract` margin and the -1 sentinel. Send: - Trezor commit threw a bare empty String read from the wrong FFI object, producing an empty "Transaction Error" banner; read Wallet_errorString and throw a typed exception. Fallback text when an error is empty. - "Max" on a 2nd+ recipient: the send page's per-output reactions were bound to output 0 because the selected index was not observable. Resync sheet: new copy and a persisted "Do not show me this again" option (shouldShowTrezorResyncInfo). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
changes in dependencies |
Review follow-ups on #3631: - Cancellation now has its own signal (_cancelCompleter) that every awaited stage of a connect attempt races against, including the BLE connect that runs before any PIN/settings completer exists. A link that comes up after the user exited is disconnected, and the attempt re-checks the flag between stages so it can never report success (or park on a new completer) after the sheet has closed. - getAutoPairingCredentials(): cancellation is re-thrown instead of being swallowed by the "auto-pairing is optional" catch. - Send page "Max": re-resolve the tapped recipient's index after the async balance lookup so removing/re-ordering recipients meanwhile cannot hit a stale controller index. - Connect page: de-duplicate devices by transport id instead of name, so two devices sharing a name both stay selectable (HardwareWalletDevice.id). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…anup The SDK disconnects by device id through a shared connection manager, so the late disconnect of a link that came up after the user cancelled could tear down the link of the next attempt on the same device. The late connect and its disconnect are now tracked as a pending cleanup that the next attempt awaits before connecting. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nally; keep app passphrase under on-device mode; pop restore pages on Trezor link loss Panel review follow-ups on #3631: - proceed_on_device_sheet: if the sheet route is destroyed without its own buttons (e.g. the app locks and the unlock navigation clears the stack), dispose() cancels a still-pending connect so isConnecting cannot stay stuck for the rest of the session. - Restore pages treated only the Ledger connection string as "link lost"; since Trezor errors now have their own strings, they never went back to the connect page. isHardwareWalletConnectionError covers all of them. - A wallet set up with an app-side passphrase keeps that record when the device is later switched to "passphrase always on device"; previously a reconnect rewrote the mode to "device" and deleted the stored secret. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… auto-submit pairing code, Trezor how-to-connect steps - Restoring another wallet while the Trezor is already connected skipped straight to the accounts and silently reused the live session, so there was no way to pick a (different or no) passphrase. The scan is still skipped, but prepareNewWalletSession() now shows the settings sheet and binds a fresh session with the chosen passphrase (explicitly empty when none), or lets the device ask when it forces on-device entry. - The pairing code is submitted as soon as the sixth digit is typed; the arrow stays as a fallback and a new code request resets the field. - "How to connect" shows Trezor-specific steps (menu, Pair & Connect, Pair new device) instead of the Ledger ones. Strings added to all locales. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
universal_ble's peripheral plugin instantiated CBPeripheralManager in its initializer, i.e. during plugin registration on every launch, so iOS asked for Bluetooth permission before the user had touched anything Bluetooth related. Pin universal_ble to the cake-tech fork commit that backports the upstream fix (defer CBPeripheralManager creation until the peripheral API is used). The prompt now appears only when the hardware wallet connect page starts scanning. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nishes syncing A freshly restored Monero wallet on a Trezor is view-only on the phone, so until the key images are imported from the device every output looks unspent and the balance is too high. The sync used to be offered only when the user tried to send. Offer it once per wallet load as soon as a sync completes with unknown key images, e.g. right after a restore. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The 24pt icon was centred on the text line box while the glyphs sit lower in it, so the full-height Trezor icon floated above the name. Align the icon to the text baseline and draw it at 20pt. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the resync page a Material ancestor - WalletInfoBar: lay the icon out inline (WidgetSpan, middle alignment) so the font metrics position it instead of the Row's line box; verified by rendering the widget with the app theme and fonts. Row-level baseline alignment did not propagate through the AnimatedSwitcher and left the icon top-aligned. - SyncKeyImagesSheet is pushed as a bare route; without a Material ancestor its texts rendered in the red/yellow fallback style. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ssphrase records Security review follow-ups on #3631: - The live Trezor session is now attributed to a wallet (_sessionWalletKey). Already-connected spend/sync/buy flows call initWallet(), which used to attach whatever session was open and persist its settings under the current wallet. With one device and two Cake wallets that could sign or import key images with wallet A's passphrase while wallet B was open, and copy A's passphrase into B's stored record. initWallet() now rebinds the session for the wallet first (silently from its stored settings, or via the settings sheet) and refuses to persist settings for a wallet the session does not belong to. - Per-wallet settings (mode + app-side passphrase) moved into TrezorWalletSettingsStorage and are deleted when the wallet is removed and re-keyed when it is renamed, so a passphrase no longer outlives its wallet or gets applied to a later wallet reusing the same name. - _presetSettings is consumed before connectDevice's early returns so a rejected call cannot leave a preset for an unrelated later connect. - TrezorDeviceSettings moved to core alongside the storage. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ing; rebind unattributed sessions A Bitcoin passphrase wallet could reach signing with a session bound to the wrong (empty) passphrase and fail with a script/pubkey mismatch: the stored settings were wrong, or the live session had never been attributed to a wallet (abandoned restore) and was taken as this wallet's. - A session that is not attributed to the wallet being used, including an unattributed one, is now rebound first. - After (re)binding, the device is asked for the account key (Bitcoin: account xpub at the wallet's derivation path; Monero: primary address, without showing it on the device) and compared with the wallet. On a mismatch the passphrase settings sheet is shown once and the check repeated; a persistent mismatch aborts with a clear error instead of a wrong-wallet signature. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…es each time, and show step-specific device instructions
- The per-wallet record now stores only how the passphrase is supplied
(none / on device / in app); the app-side passphrase itself is never
persisted any more (earlier records are scrubbed). Reconnecting an
app-passphrase wallet shows a passphrase field; a device-passphrase
wallet shows "enter it on your Trezor"; the full settings sheet only
appears when nothing is known or the keys do not match.
- Every hardware-wallet connect callback now awaits initWallet() before
continuing. It performs device round-trips (session rebind and key
verification) since the last change, and callers that fired it without
awaiting proceeded to sign with no hardware service attached
("type 'Null' is not a subtype of type 'BitcoinHardwareWalletService'").
- Replace the catch-all "Proceed on your device" with what the Trezor
actually asks for at each step: Bluetooth pairing-code match and pairing
consent before the security code, auto-connect consent, passphrase on
device, watch-only credential export (Monero restore), key image sync
confirmation, and transaction review (each recipient, then fee/total).
Strings added to all locales.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…reword the signing instruction - The in-app passphrase screen now uses the same BaseTextFormField, reveal toggle, hint and illustration as the hot-wallet passphrase sheet. - Signing instruction copy: "Review all transaction details on your Trezor match." Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
konstantinullrich
changed the base branch from
dev
to
CW-1624-trezor-passphrase-on-trezor-device-instead-of-app
September 17, 2026 11:56
konstantinullrich
changed the base branch from
CW-1624-trezor-passphrase-on-trezor-device-instead-of-app
to
dev
September 17, 2026 11:57
…ed key-image sync recoverable - Pin trezor_flutter/trezor_usb_transport to the cake-tech branch that ignores notifications on a disposed gateway and drops malformed fragments instead of failing with "Bad state: Not enough bytes to read" (seen by QA when the device timed out and auto-reconnected during the key-image sync flow). - Key-image sync failure no longer leaves the resync sheet stuck on the syncing state: it returns to the explanation, shows the reason, and resets a dead client so the next attempt reconnects cleanly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dev now carries the squash of #3601 (the branch this work was built on), so every conflict was between our follow-up changes and content that is already an ancestor here. Resolved by keeping this branch's side: - trezor_connect_view_model.dart / proceed_on_device_sheet.dart: dev's copies are byte-identical to the #3601 tip in our history. - res/values/*.arb: keep the added `auto_connect_desc` next to `auto_connect`. - pubspec_base.yaml / pubspec_overrides.yaml / pubspec.lock: keep the trezor_flutter ref 73e7045 (descends from dev's 0277176). - cw_monero wallet_manager.dart: keep the guarded setRefreshFromBlockHeight and its comment; cw_core get_height_by_date.dart: same fix, dev's `const`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e conversations QA: after several Monero sends, a block landed while a transaction was being signed on the Trezor. The wallet flipped to "synced" with an unknown-key-image change output, so the automatic key-image sync offer was pushed on top of the send flow. Continuing it started a second THP conversation while the signing one was still on the device, and the Trezor stopped responding until it was power-cycled and re-paired. - MoneroTrezorService now owns a static device mutex; signing, key-image sync, the watch-only export and the session address check all run under it, so two device conversations can no longer interleave. Exposed as `Monero.isTrezorBusy()`. - The sync-status reaction only offers the key-image sync when the user is on the dashboard and the device is idle. Otherwise it waits (MobX `when` on the current route) and offers once the user is back on the dashboard, instead of consuming the once-per-wallet offer or interrupting a flow. The pending offer is dropped when the active wallet changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ardown, recover the key-image sheet; restore dev hunks dropped in the merge Review round 3 (Astra) on d712076. 1. A pairing sheet that had already succeeded could cancel the corrective rebind that _ensureSessionFor starts right after it: its dispose() runs after the pop animation and read the view model's *current* isConnecting / paringState. The sheet now captures `pairingAttempt` when it opens, records its own success, and `cancelPairing(attempt:)` ignores a sheet that is not the current attempt. 2. Cancelling a session rebind at the in-app passphrase stage makes the SDK tear the BLE link down asynchronously, but that teardown was never registered in _pendingCleanup (rebind had a no-op onFailure), so the next connect could bring up a link while the old one was still going down. cancelPairing now chains the SDK cancel into _pendingCleanup, and a rebind whose link died resets the client. 3. The key-image sheet awaited initWallet without handling the new TrezorSessionMismatchException, leaving it on "Proceed on device" with no Continue button. Session bind + sync is now one recoverable step: back to the initial state on failure or back-out; the view model shows the reason (unless the user cancelled) so every initWallet caller gets the same message without local handling. TrezorSessionMismatchException carries `cancelled`. 4. Merge ee4c21b used `git checkout --ours` on conflicted files, which also discarded dev's auto-merged hunks in them. Restored: the guarded setRefreshFromBlockHeight in restoreWalletFromSpendKeySync, the cake_backup pin reverted by #3634 (3aba867), and the unused import removal in get_height_by_date.dart. Those files now differ from dev only by this branch's intended changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ound instead of leaking the exception Review round 3 follow-up. Send, Swap, the buy providers and the dEuro savings page awaited initWallet() with no handling for TrezorSessionMismatchException, so a user simply exiting the passphrase prompt for a wallet the device was not yet bound to bubbled up to the application error handler and could show the generic report-an-error dialog. The same flows also re-checked only "is the device still connected?" after the connect page, which is not the same as "is the session bound to this wallet". - HardwareWalletViewModel.tryInitWallet(): typed result; false when the session could not be bound (the user has been told why unless they cancelled). Other failures still propagate. - ensureHardwareWalletReady(context, vm, wallet): one place for the connect-page-then-bind dance that every flow duplicated; returns false on back-out or bind failure and never treats a connected-but-unbound device as ready. Push and pop use the same NavigatorState. - All six flows use it and stop on false. The dEuro page's _requireHardwareWallet now returns that result and its actions honour it. - TrezorSessionMismatchException moves to the base view model file so the base can consume it without a circular import. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
dev's #3638 (thread-safety and BLE support for Ledger and Trezor) landed the same ideas this branch already carried, so the merge reconciles two implementations rather than picking sides: - MoneroTrezorService: dev's `runBlocking` mutex naming is kept; this branch's `isBusy` flag is added on top because the key-image offer gate reads it. The session address check keeps using the same lock. - HardwareWalletViewModel: dev's `getConnectedBleDevices` replaces this branch's `getConnectedDevices`; the connect page keeps this branch's loader (de-duplicates by device id) on dev's method name, and dev's duplicate loader is dropped. - TrezorConnectViewModel: dev's BLE link-state listener is kept, registered in `_connect` where the interface is chosen (dev registered it in the old connectDevice body this branch had already restructured). The listener now also clears the session attribution when it drops the client, and its log line says Trezor rather than Ledger. dev's `close()` is kept as is. - trezor_flutter: dev's pin 591d4b8 supersedes this branch's fork ref 73e7045 (same gateway fix plus client safeguards). pubspec.lock refreshed with `flutter pub get`. Every conflicted file was diffed against dev after resolution; the only remaining differences are this branch's intended changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
4 participants
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.
Follow-up to #3601 (this branch includes its commits and Vik's "reuse passphrase settings on reconnect" patch; the diff shrinks to the last commit once #3601 merges).
Fixes for the CW-1624 test findings on Trezor Safe 7 (Monero + Bitcoin):
Connect / pairing
ThpUnallocatedChannel: the view model reused its in-memory THP state (phase=paired) across connects, so trezor_flutter skipped the handshake and used a channel the device had dropped. Every attempt now rebuilds state from the persisted pairing credentials; a failed attempt no longer wipes them.connectDevicecall. Before, "Try again" fired a second connect whose success never reached the page (you were left on the device list), and exiting the sheet leftconnectDeviceawaiting a completer forever withisConnectingstuck.createChannel()already bound one via the on-device prompt). Passphrase options are hidden in that mode; the settings sheet is skipped when nothing is left to configure. "Passphrase entry" state only shows when a session is actually created.interpretErrorCodeinstead of the Ledger connection message.Restore
SelectHardwareWalletAccountPage/MoneroHardwareWalletOptionsPagepopped themselves on any Trezor error and showed the alert on a disposed context (Bitcoin "dropped back to device list" with no message). The reason is shown first.date..subtractmargin and the-1height sentinel.Send
Stringfrom the wrong FFI object → blank "Transaction Error". It now readsWallet_errorStringand throws a typed exception; the banner has a fallback text.Resync sheet: new copy + persisted "Do not show me this again" (
shouldShowTrezorResyncInfo).Strings added to all 31 locales via
tool/append_translation.dart.Not addressed here: the multi-recipient Monero Trezor signing failure itself (error is now visible; likely in trezor_flutter's range-proof batching /
commitTrezor(0)), THP request timeout, Ledger-only signing progress UI.Not yet tested on hardware.
🤖 Generated with Claude Code