feat: pair trezor passphrase hidden wallets - #662
Draft
jvsena42 wants to merge 7 commits into
Draft
Conversation
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