feat: pair trezor passphrase hidden wallets - #662
Open
jvsena42 wants to merge 20 commits into
Open
Conversation
Member
Author
This test requires a physical device |
jvsena42
marked this pull request as ready for review
August 12, 2026 12:35
Greptile SummaryThe PR adds independently tracked Trezor hidden-wallet identities and passphrase-aware pairing and signing while rekeying hardware-wallet state from transport IDs to wallet IDs.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the current identity checks reject unresolved or mismatched sessions before signing, resolving both previously reported issues.
|
| Filename | Overview |
|---|---|
| Bitkit/Managers/HwWalletManager.swift | Rekeys watcher and funding behavior by wallet identity and enforces a resolved identity before signing. |
| Bitkit/Managers/TrezorManager.swift | Tracks the wallet opened by the live session and supports explicit standard or passphrase session reconstruction. |
| Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift | Persists and removes known-device entries using wallet-scoped identity semantics. |
| Bitkit/ViewModels/TransferViewModel.swift | Integrates passphrase prompting and identity-aware reconnection into hardware-funded transfers. |
| Bitkit/ViewModels/Trezor/HwConnectViewModel.swift | Adds the passphrase pairing step and tracks the wallet identity produced by connection. |
| BitkitTests/HwWalletManagerPassphraseTests.swift | Covers unresolved, mismatched, duplicate, and successfully reopened passphrase-wallet sessions. |
Sequence Diagram
sequenceDiagram
participant U as User
participant UI as Transfer UI
participant H as HwWalletManager
participant T as TrezorManager
participant D as Trezor
U->>UI: Sign from walletId
UI->>H: ensureConnected(walletId)
H->>T: Ensure transport connected
alt matching live identity
T-->>H: connectedWalletId matches
else hidden identity unavailable
H-->>UI: Passphrase required
U->>UI: Enter passphrase
UI->>H: reconnectWithPassphrase(walletId)
H->>T: Open passphrase session
T->>D: Read accounts
D-->>T: Derived wallet accounts
T-->>H: connectedWalletId
H->>H: Verify walletId matches
end
UI->>H: signFunding(walletId, transaction)
H->>H: Recheck connected identity
H->>D: Sign PSBT
D-->>UI: Signed transaction
Reviews (3): Last reviewed commit: "Merge branch 'master' into feat/hw-passp..." | Re-trigger Greptile
jvsena42
marked this pull request as draft
August 12, 2026 12:49
16 tasks
…instead of the requested deviceId
…of HwPassphraseError.required, for prevend request passpbrase for the standard wallet
…n reconnectWithPassphrase
jvsena42
marked this pull request as ready for review
August 12, 2026 17:30
jvsena42
marked this pull request as draft
August 13, 2026 17:00
Member
Author
|
Draft for investigating the CI failure Edit: it is just a flaky test |
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.
Closes #615
This PR ports bitkit-android #1142 to iOS:
Description
Trezor keeps no record of its hidden wallets and forgets a passphrase with the session, so Bitkit watches each one by its extended public keys and asks for the passphrase again whenever a session has to be rebuilt. The passphrase is never persisted or logged, and both entry screens block screenshots the same way the wallet's own mnemonic screens do.
A device previously mapped to a single wallet. It now maps to one identity per passphrase, keyed by the wallet id derived from its accounts rather than by the transport id, which two identities on the same device share. Watchers, balances, activity, labels, routes and removal all follow that identity, and removing one hidden wallet leaves the others paired. This is the bulk of the diff and lands ahead of the feature itself, because leaving the old keying in place would blend two seeds' extended public keys into one stored record, change the standard wallet's derived id and orphan its activity.
Pairing a hidden wallet adds an entry next to the standard one instead of replacing it. The device is matched by shared key material, so re-reading accounts updates the right entry while a different passphrase creates a new one. Only the identity that currently holds the device session shows as connected, since only that one can sign. Session credentials are shared by every wallet on a device, so they are only cleared once no identity of it remains.
Before signing a transfer, Bitkit reopens the hidden wallet and accepts the session only if its accounts resolve back to the wallet being spent from. A wrong passphrase is not rejected by the device — it silently derives another wallet — so the mismatch is caught, the stray wallet that reading its accounts created is dropped, and the transfer fails instead of signing from the wrong wallet. A session that is still open signs with no prompt.
Discovery skips devices that are already paired, which left the Add Hardware Wallet button searching forever and made the passphrase step unreachable after the first pairing. It now falls back to offering a known reachable device once no new one is found.
Separately, the Bridge transport used for the emulator kept a released session id cached and offered it as the previous one when re-acquiring, and the bridge answered
wrong previous session. Since switching to a passphrase wallet closes and reopens the session, this blocked every passphrase pairing after the first. Bridge is dev/E2E only, so this does not affect shipping builds, which are Bluetooth-only.The six commits are meant to be read in order: storage identity, then the device id → wallet id rekeying, then the session layer, then the identity operations, then the two pieces of UI.
Linked Issues/Tasks
QA Notes
Needs a Trezor with passphrase protection enabled. On the emulator start it with
TREZOR_PASSPHRASE_PROTECTION=true ../bitkit-docker/scripts/trezor-emulator startand confirm on the device for every account a passphrase session reads, otherwise the call blocks until it times out.Manual Tests
regression:Pair a device without using the Passphrase path: home tile, detail screen, rename, remove and Transfer To Spending behave as before.Automated Checks
BitkitTests/TrezorKnownDeviceMatchingTests.swift.BitkitTests/TrezorKnownDeviceStorageTests.swift.ensureConnectedoutcomes, refusing to sign for another identity, and the stray wallet a wrong passphrase leaves behind, inBitkitTests/HwWalletManagerPassphraseTests.swift.BitkitTests/HwWalletManagerTests.swift.BitkitTests/HwConnectViewModelTests.swift.BitkitTests/TransferViewModelHwTests.swift.BitkitTests/HwWalletManagerTests.swift,HwWalletManagerFundingTests.swift,HwFundingSignerTests.swift,HwTransferMocks.swiftandTransferViewModelHwTests.swift.walletId(forDevice:)failure test fromBitkitTests/TransferViewModelHwTests.swift, because the route now carries the wallet id and the lookup it guarded no longer exists.xcodebuild -project Bitkit.xcodeproj -scheme Bitkit -destination 'platform=iOS Simulator,id=<UDID>' ONLY_ACTIVE_ARCH=YES test -only-testing:BitkitTests/<hardware suites>. The full suite cannot complete locally —AddressTypeIntegrationTestsneeds the regtest/Electrum backend and crashes the runner before reaching the hardware suites, which is pre-existing and unrelated to this branch.node scripts/validate-translations.js: 0 errors.Known gaps
TrezorWalletMode.passphraseDeviceis wired end to end and stays available on the Trezor dev dashboard, but the production flow is host entry only, matching Android.TrezorManagerstill has no unit tests: it reachesTrezorService.shared,TrezorUiHandler.sharedand the static known-device store directly, so the session work in it is covered indirectly through the pure matching helpers and the storage tests. Adding an injection seam is worth a follow-up.Screenshot / Video
add-multiple-passphrase-wallets.mov
no-wallet-add-with-passphrase.mov
add-repeated-passphrase.mov
rename-and-delete-wallet.mov
passphrase-disabled.mov
transfer-current-session-wallet.mov
transfer-different-session.mov
transfer-base-wallet.mov