From 79da0ad3bee6257620de8425801c1eaa391215e9 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 11:23:20 -0300 Subject: [PATCH 01/18] feat: storage identity --- .../Trezor/TrezorKnownDeviceMatching.swift | 62 +++++ .../Trezor/TrezorKnownDeviceStorage.swift | 73 +++++- .../Views/Trezor/TrezorDeviceListView.swift | 2 +- .../TrezorKnownDeviceMatchingTests.swift | 228 ++++++++++++++++++ 4 files changed, 359 insertions(+), 6 deletions(-) create mode 100644 Bitkit/Services/Trezor/TrezorKnownDeviceMatching.swift create mode 100644 BitkitTests/TrezorKnownDeviceMatchingTests.swift diff --git a/Bitkit/Services/Trezor/TrezorKnownDeviceMatching.swift b/Bitkit/Services/Trezor/TrezorKnownDeviceMatching.swift new file mode 100644 index 000000000..afb26318c --- /dev/null +++ b/Bitkit/Services/Trezor/TrezorKnownDeviceMatching.swift @@ -0,0 +1,62 @@ +import Foundation + +/// Decides which stored entry a fresh connect refreshes, and which entries it supersedes. +/// +/// A passphrase wallet is a separate identity on the same physical device, so the transport id +/// alone no longer identifies an entry: matching by it would overwrite another identity or blend +/// two identities' xpubs into one record. Shared key material is the identity. +enum TrezorKnownDeviceMatching { + /// The entry this connect refreshed: among the entries of this transport, the one whose xpubs + /// overlap the freshly read set. Only an entry stored before any xpub was captured has no + /// identity to conflict with and can be adopted instead. Anything else is a new identity. + static func previous( + in devices: [TrezorKnownDevice], + deviceId: String, + fetchedXpubs: [String: String] + ) -> TrezorKnownDevice? { + let candidates = devices.filter { $0.id == deviceId } + let fetched = Set(fetchedXpubs.values) + if let overlapping = candidates.first(where: { !Set($0.xpubs.values).isDisjoint(with: fetched) }) { + return overlapping + } + guard candidates.count == 1, let only = candidates.first, only.xpubs.isEmpty else { return nil } + return only + } + + /// The entry a new record inherits its Bitkit-side label from. Labels are set for the wallet, + /// not for the transport it happens to be reached over, so a wallet showing up on a new path + /// keeps the name the user gave it instead of falling back to the device's own. + static func named( + in devices: [TrezorKnownDevice], + previous: TrezorKnownDevice?, + walletKey: String + ) -> TrezorKnownDevice? { + previous ?? devices.first { $0.walletKey == walletKey } + } + + /// The stored list after `known` supersedes what it replaces. + static func merged( + _ devices: [TrezorKnownDevice], + with known: TrezorKnownDevice, + refreshed: TrezorKnownDevice? + ) -> [TrezorKnownDevice] { + devices.filter { !isReplaced($0, by: known, refreshed: refreshed) } + [known] + } + + /// Whether a stored entry gives way to the one just read. That covers the identity it holds and + /// the entry this connect refreshed, since reading a previously rejected address type changes + /// the wallet key and matching on the new key alone would leave the old entry behind as a + /// duplicate. Wallets of a seed the device no longer carries go too: nothing would ever + /// supersede them by key material. An unknown device id proves nothing, so those are left alone. + private static func isReplaced( + _ entry: TrezorKnownDevice, + by known: TrezorKnownDevice, + refreshed: TrezorKnownDevice? + ) -> Bool { + guard entry.id == known.id else { return false } + if entry.walletKey == known.walletKey { return true } + if let refreshed, entry.walletKey == refreshed.walletKey { return true } + guard let knownTrezorId = known.trezorDeviceId, let entryTrezorId = entry.trezorDeviceId else { return false } + return entryTrezorId != knownTrezorId + } +} diff --git a/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift b/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift index e2407b598..c365b062a 100644 --- a/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift +++ b/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift @@ -1,6 +1,8 @@ import Foundation -/// Represents a previously connected Trezor device +/// One wallet identity a Trezor holds. A device with passphrase protection carries its standard +/// wallet plus one entry per passphrase (hidden) wallet, so `id` — the transport-level device id — +/// is shared by several entries and no longer identifies one on its own. struct TrezorKnownDevice: Codable, Identifiable { let id: String let name: String @@ -15,6 +17,17 @@ struct TrezorKnownDevice: Codable, Identifiable { /// User-set name applied while managing the wallet in Bitkit; nil until renamed. Takes priority /// over the device's own `label`/`model` when resolving the display name. var customLabel: String? + /// bitkit-core wallet id of this identity. Absent on entries stored before hidden wallets + /// existed, where `resolvedWalletId` derives it from `xpubs` instead. + var walletId: String? + /// Whether this entry is a passphrase (hidden) wallet. Nothing else in the record can tell one + /// apart from the standard wallet — the xpubs are opaque and the selected mode only lives in + /// memory, so reconnects would silently fall back to the standard wallet without this. The + /// passphrase itself is never persisted. + var passphraseProtected: Bool + /// The Trezor's own device id, which it regenerates when wiped. Entries of the same transport + /// reporting a different one belong to a seed the device can no longer sign for. + var trezorDeviceId: String? init( id: String, @@ -25,7 +38,10 @@ struct TrezorKnownDevice: Codable, Identifiable { model: String? = nil, lastConnectedAt: Date, xpubs: [String: String] = [:], - customLabel: String? = nil + customLabel: String? = nil, + walletId: String? = nil, + passphraseProtected: Bool = false, + trezorDeviceId: String? = nil ) { self.id = id self.name = name @@ -36,6 +52,9 @@ struct TrezorKnownDevice: Codable, Identifiable { self.lastConnectedAt = lastConnectedAt self.xpubs = xpubs self.customLabel = customLabel + self.walletId = walletId + self.passphraseProtected = passphraseProtected + self.trezorDeviceId = trezorDeviceId } init(from decoder: any Decoder) throws { @@ -49,6 +68,34 @@ struct TrezorKnownDevice: Codable, Identifiable { lastConnectedAt = try container.decode(Date.self, forKey: .lastConnectedAt) xpubs = try container.decodeIfPresent([String: String].self, forKey: .xpubs) ?? [:] customLabel = try container.decodeIfPresent(String.self, forKey: .customLabel) + walletId = try container.decodeIfPresent(String.self, forKey: .walletId) + passphraseProtected = try container.decodeIfPresent(Bool.self, forKey: .passphraseProtected) ?? false + trezorDeviceId = try container.decodeIfPresent(String.self, forKey: .trezorDeviceId) + } +} + +extension TrezorKnownDevice { + /// Identity of the key material this entry holds: entries sharing it are the same wallet, on + /// this device or on another transport. An entry read before any xpub was captured has no key + /// material to compare, so it falls back to its transport id. + var walletKey: String { + TrezorKnownDevice.walletKey(for: xpubs, fallback: id) + } + + static func walletKey(for xpubs: [String: String], fallback: String) -> String { + xpubs.isEmpty ? fallback : xpubs.values.sorted().joined(separator: "\u{1f}") + } + + /// Wallet id of this identity: the stored one, or derived from the xpubs for entries written + /// before it was persisted. The derivation is unchanged, so those keep the id they always had. + var resolvedWalletId: String? { + if let walletId, !walletId.isEmpty { return walletId } + return try? HwWalletId.derive(xpubs: xpubs) + } + + /// Stable key for lists and diffing, since `id` is shared by every identity of one device. + var entryId: String { + "\(id)\u{1f}\(walletKey)" } } @@ -64,10 +111,12 @@ enum TrezorKnownDeviceStorage { return devices.sorted { $0.lastConnectedAt > $1.lastConnectedAt } } - /// Save or update a known device + /// Save or update one wallet identity. Scoped to the identity rather than to the transport it + /// was reached over, so a passphrase wallet is stored next to the device's standard wallet + /// instead of replacing it. static func save(_ device: TrezorKnownDevice) { var devices = loadAll() - devices.removeAll { $0.id == device.id } + devices.removeAll { $0.id == device.id && $0.walletKey == device.walletKey } devices.insert(device, at: 0) if let data = try? JSONEncoder().encode(devices) { UserDefaults.standard.set(data, forKey: key) @@ -82,7 +131,12 @@ enum TrezorKnownDeviceStorage { } } - /// Remove a known device by ID + /// Entries tracking one wallet identity. + static func loadAll(walletId: String) -> [TrezorKnownDevice] { + loadAll().filter { $0.resolvedWalletId == walletId } + } + + /// Forget every identity of a device, whichever wallets it holds. static func remove(id: String) { var devices = loadAll() devices.removeAll { $0.id == id } @@ -91,6 +145,15 @@ enum TrezorKnownDeviceStorage { } } + /// Forget a single wallet identity, leaving the device's other wallets paired. + static func remove(walletId: String) { + var devices = loadAll() + devices.removeAll { $0.resolvedWalletId == walletId } + if let data = try? JSONEncoder().encode(devices) { + UserDefaults.standard.set(data, forKey: key) + } + } + /// Remove all remembered Trezor devices. static func removeAll() { UserDefaults.standard.removeObject(forKey: key) diff --git a/Bitkit/Views/Trezor/TrezorDeviceListView.swift b/Bitkit/Views/Trezor/TrezorDeviceListView.swift index 56eb2833b..83d4e003c 100644 --- a/Bitkit/Views/Trezor/TrezorDeviceListView.swift +++ b/Bitkit/Views/Trezor/TrezorDeviceListView.swift @@ -41,7 +41,7 @@ struct TrezorDeviceListView: View { .font(.system(size: 14, weight: .medium)) .foregroundColor(.white.opacity(0.6)) - ForEach(trezorManager.knownDevices) { device in + ForEach(trezorManager.knownDevices, id: \.entryId) { device in KnownDeviceRow( device: device, isConnecting: connectingDevicePath == device.path diff --git a/BitkitTests/TrezorKnownDeviceMatchingTests.swift b/BitkitTests/TrezorKnownDeviceMatchingTests.swift new file mode 100644 index 000000000..94a7da740 --- /dev/null +++ b/BitkitTests/TrezorKnownDeviceMatchingTests.swift @@ -0,0 +1,228 @@ +@testable import Bitkit +import XCTest + +/// Covers how a connect resolves which stored entry it refreshes and which entries it supersedes, +/// now that one physical device can hold a standard wallet plus its passphrase (hidden) wallets. +final class TrezorKnownDeviceMatchingTests: XCTestCase { + // MARK: - previous(in:deviceId:fetchedXpubs:) + + func testRefreshesTheEntrySharingKeyMaterial() { + let standard = makeDevice(xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden") + + let previous = TrezorKnownDeviceMatching.previous( + in: [standard, hidden], + deviceId: "dev1", + fetchedXpubs: ["nativeSegwit": "zHidden", "taproot": "zHiddenTR"] + ) + + XCTAssertEqual(previous?.walletId, "trezor:hidden") + } + + /// A passphrase wallet read for the first time overlaps nothing, so it must not adopt the + /// standard wallet's entry — that would blend two seeds' xpubs into one record. + func testTreatsUnseenKeyMaterialAsANewIdentity() { + let standard = makeDevice(xpubs: ["nativeSegwit": "zStandard"]) + + let previous = TrezorKnownDeviceMatching.previous( + in: [standard], + deviceId: "dev1", + fetchedXpubs: ["nativeSegwit": "zHidden"] + ) + + XCTAssertNil(previous) + } + + func testAdoptsALoneEntryStoredBeforeAnyXpubWasCaptured() { + let bare = makeDevice(xpubs: [:], customLabel: "My Trezor") + + let previous = TrezorKnownDeviceMatching.previous( + in: [bare], + deviceId: "dev1", + fetchedXpubs: ["nativeSegwit": "zStandard"] + ) + + XCTAssertEqual(previous?.customLabel, "My Trezor") + } + + func testIgnoresEntriesOfAnotherDevice() { + let other = makeDevice(id: "dev2", xpubs: ["nativeSegwit": "zStandard"]) + + let previous = TrezorKnownDeviceMatching.previous( + in: [other], + deviceId: "dev1", + fetchedXpubs: ["nativeSegwit": "zStandard"] + ) + + XCTAssertNil(previous) + } + + // MARK: - named(in:previous:walletKey:) + + /// The wallet reappears on a fresh transport path, so nothing matches by device id — but it is + /// the same key material, and the user's label belongs to the wallet, not to the path. + func testInheritsTheLabelOfTheSameWalletOnAnotherPath() { + let previouslyPaired = makeDevice(id: "old-path", xpubs: ["nativeSegwit": "zStandard"], customLabel: "Savings") + + let named = TrezorKnownDeviceMatching.named( + in: [previouslyPaired], + previous: nil, + walletKey: TrezorKnownDevice.walletKey(for: ["nativeSegwit": "zStandard"], fallback: "dev1") + ) + + XCTAssertEqual(named?.customLabel, "Savings") + } + + func testPrefersTheRefreshedEntryForTheLabel() { + let refreshed = makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Refreshed") + let sameKey = makeDevice(id: "old-path", xpubs: ["nativeSegwit": "zStandard"], customLabel: "Stale") + + let named = TrezorKnownDeviceMatching.named( + in: [sameKey, refreshed], + previous: refreshed, + walletKey: refreshed.walletKey + ) + + XCTAssertEqual(named?.customLabel, "Refreshed") + } + + // MARK: - merged(_:with:refreshed:) + + func testKeepsTheStandardWalletWhenAPassphraseWalletIsAdded() { + let standard = makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:standard") + let hidden = makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden", passphraseProtected: true) + + let merged = TrezorKnownDeviceMatching.merged([standard], with: hidden, refreshed: nil) + + XCTAssertEqual(merged.map(\.walletId), ["trezor:standard", "trezor:hidden"]) + } + + func testReplacesTheEntryHoldingTheSameIdentity() { + let stored = makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Old") + let known = makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "New") + + let merged = TrezorKnownDeviceMatching.merged([stored], with: known, refreshed: stored) + + XCTAssertEqual(merged.map(\.customLabel), ["New"]) + } + + /// Reading a previously rejected address type changes the wallet key, so matching on the new + /// key alone would leave the entry this connect refreshed behind as a duplicate. + func testReplacesTheRefreshedEntryWhenReadingMoreAccountsChangesItsKey() { + let partial = makeDevice(xpubs: ["nativeSegwit": "zStandard"]) + let complete = makeDevice(xpubs: ["nativeSegwit": "zStandard", "taproot": "zTaproot"]) + + let merged = TrezorKnownDeviceMatching.merged([partial], with: complete, refreshed: partial) + + XCTAssertEqual(merged.count, 1) + XCTAssertEqual(merged[0].xpubs.count, 2) + } + + func testSupersedesWalletsOfASeedTheDeviceNoLongerCarries() { + let wiped = makeDevice(xpubs: ["nativeSegwit": "zOldSeed"], trezorDeviceId: "trezor-before-wipe") + let known = makeDevice(xpubs: ["nativeSegwit": "zNewSeed"], trezorDeviceId: "trezor-after-wipe") + + let merged = TrezorKnownDeviceMatching.merged([wiped], with: known, refreshed: nil) + + XCTAssertEqual(merged.map(\.xpubs), [["nativeSegwit": "zNewSeed"]]) + } + + /// Two identities of one device report the same Trezor device id, so the wipe rule must not + /// sweep away the sibling wallet. + func testKeepsAnotherIdentityOfTheSameDevice() { + let standard = makeDevice( + xpubs: ["nativeSegwit": "zStandard"], + walletId: "trezor:standard", + trezorDeviceId: "trezor-id" + ) + let hidden = makeDevice( + xpubs: ["nativeSegwit": "zHidden"], + walletId: "trezor:hidden", + passphraseProtected: true, + trezorDeviceId: "trezor-id" + ) + + let merged = TrezorKnownDeviceMatching.merged([standard], with: hidden, refreshed: nil) + + XCTAssertEqual(merged.map(\.walletId), ["trezor:standard", "trezor:hidden"]) + } + + func testLeavesEntriesOfAnotherDeviceAlone() { + let other = makeDevice(id: "dev2", xpubs: ["nativeSegwit": "zOther"], trezorDeviceId: "other-trezor") + let known = makeDevice(xpubs: ["nativeSegwit": "zStandard"], trezorDeviceId: "trezor-id") + + let merged = TrezorKnownDeviceMatching.merged([other], with: known, refreshed: nil) + + XCTAssertEqual(merged.count, 2) + } + + // MARK: - Identity helpers + + func testWalletKeyIsIndependentOfAddressTypeKeys() { + let a = makeDevice(xpubs: ["nativeSegwit": "zA", "taproot": "zB"]) + let b = makeDevice(xpubs: ["taproot": "zA", "nativeSegwit": "zB"]) + + XCTAssertEqual(a.walletKey, b.walletKey) + } + + func testWalletKeyFallsBackToTheTransportIdWithoutXpubs() { + XCTAssertEqual(makeDevice(xpubs: [:]).walletKey, "dev1") + } + + func testEntryIdSeparatesTwoIdentitiesOfOneDevice() { + let standard = makeDevice(xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(xpubs: ["nativeSegwit": "zHidden"]) + + XCTAssertNotEqual(standard.entryId, hidden.entryId) + } + + func testResolvedWalletIdPrefersTheStoredValue() { + let device = makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:stored") + + XCTAssertEqual(device.resolvedWalletId, "trezor:stored") + } + + // MARK: - Decoding entries stored before hidden wallets existed + + func testDecodesLegacyEntriesAndDerivesTheirWalletId() throws { + let legacy = """ + { + "id": "dev1", + "name": "Trezor", + "path": "ble://dev1", + "transportType": "bluetooth", + "lastConnectedAt": 0, + "xpubs": { "nativeSegwit": "zStandard" } + } + """ + + let decoded = try JSONDecoder().decode(TrezorKnownDevice.self, from: Data(legacy.utf8)) + + XCTAssertNil(decoded.walletId) + XCTAssertFalse(decoded.passphraseProtected) + XCTAssertNil(decoded.trezorDeviceId) + XCTAssertEqual(decoded.resolvedWalletId, try HwWalletId.derive(xpubs: ["nativeSegwit": "zStandard"])) + } + + private func makeDevice( + id: String = "dev1", + xpubs: [String: String], + customLabel: String? = nil, + walletId: String? = nil, + passphraseProtected: Bool = false, + trezorDeviceId: String? = nil + ) -> TrezorKnownDevice { + TrezorKnownDevice( + id: id, + name: "Trezor", + path: "ble://\(id)", + transportType: "bluetooth", + lastConnectedAt: Date(timeIntervalSince1970: 0), + xpubs: xpubs, + customLabel: customLabel, + walletId: walletId, + passphraseProtected: passphraseProtected, + trezorDeviceId: trezorDeviceId + ) + } +} From 7751e9c02e6484345be76ef90313c47039f2b700 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 13:08:11 -0300 Subject: [PATCH 02/18] feat: replace device id with wallet id --- Bitkit/MainNavView.swift | 8 +- Bitkit/Managers/HwWalletManager.swift | 188 +++++++++--------- Bitkit/Managers/TrezorManager.swift | 66 +++++- Bitkit/Models/HwWallet.swift | 14 +- Bitkit/ViewModels/HwFundingSigner.swift | 44 ++-- Bitkit/ViewModels/NavigationViewModel.swift | 8 +- Bitkit/ViewModels/SheetViewModel.swift | 2 +- Bitkit/ViewModels/TransferViewModel.swift | 104 +++++----- Bitkit/Views/Home/HomeWalletView.swift | 2 +- .../HardwareWalletsSettingsScreen.swift | 2 +- .../Sheets/RenameHardwareWalletSheet.swift | 8 +- .../Transfer/Hardware/SpendingAmountHw.swift | 10 +- .../Transfer/Hardware/SpendingHwSign.swift | 8 +- Bitkit/Views/Transfer/SpendingIntroView.swift | 6 +- .../Views/Wallets/HardwareWalletScreen.swift | 8 +- BitkitTests/HwFundingSignerTests.swift | 24 +-- BitkitTests/HwTransferMocks.swift | 29 +-- BitkitTests/HwWalletManagerFundingTests.swift | 23 ++- BitkitTests/HwWalletManagerTests.swift | 128 +++++++++--- BitkitTests/TransferViewModelHwTests.swift | 73 +++---- 20 files changed, 436 insertions(+), 319 deletions(-) diff --git a/Bitkit/MainNavView.swift b/Bitkit/MainNavView.swift index 380311134..533a659a8 100644 --- a/Bitkit/MainNavView.swift +++ b/Bitkit/MainNavView.swift @@ -420,17 +420,17 @@ struct MainNavView: View { case .buyBitcoin: BuyBitcoinView() case .savingsWallet: SavingsWalletScreen() case .spendingWallet: SpendingWalletScreen() - case let .hardwareWallet(deviceId): HardwareWalletScreen(deviceId: deviceId) + case let .hardwareWallet(walletId): HardwareWalletScreen(walletId: walletId) case .scanner: ScannerScreen() // Transfer case .transferIntro: TransferIntroView() case .fundingOptions: FundingOptions() case .spendingIntro: SpendingIntroView() - case let .spendingIntroHw(deviceId): SpendingIntroView(deviceId: deviceId) + case let .spendingIntroHw(walletId): SpendingIntroView(walletId: walletId) case .spendingAmount: SpendingAmount() - case let .spendingAmountHw(deviceId): SpendingAmountHw(deviceId: deviceId) - case let .spendingHwSign(deviceId): SpendingHwSign(deviceId: deviceId) + case let .spendingAmountHw(walletId): SpendingAmountHw(walletId: walletId) + case let .spendingHwSign(walletId): SpendingHwSign(walletId: walletId) case .spendingHwSigned: SpendingHwSigned() case let .spendingConfirm(order): SpendingConfirm(order: order) case let .spendingAdvanced(order): SpendingAdvancedView(order: order) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 2d2540003..68cd2da6d 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -2,12 +2,15 @@ import BitkitCore import Combine import Foundation -/// Production hardware-wallet business layer. Tracks paired Trezor devices as watch-only -/// balances by running one on-chain xpub watcher per (device, address type), aggregating the -/// per-device balance in memory, and persisting each device's on-chain activity into -/// bitkit-core scoped by a derived `walletId` (core 0.3.x wallet-scoped storage). +/// Production hardware-wallet business layer. Tracks paired Trezor wallets as watch-only +/// balances by running one on-chain xpub watcher per (wallet, address type), aggregating the +/// per-wallet balance in memory, and persisting each wallet's on-chain activity into +/// bitkit-core scoped by its `walletId` (core 0.3.x wallet-scoped storage). /// -/// Fully decoupled from `TrezorManager`: it receives the paired-device snapshot through +/// Keyed by wallet identity, not by device: a Trezor with passphrase protection holds its standard +/// wallet plus one identity per hidden wallet, all reached over the same transport id. +/// +/// Fully decoupled from `TrezorManager`: it receives the paired-wallet snapshot through /// `updateDevices(...)`, fed by the composition root (`AppScene`). Adapts bitkit-android's /// `HwWalletRepo`. iOS supports Bluetooth only, so the cross-transport (BLE+USB) dedup is reduced /// to a plain xpub-based identity and USB-specific reconnect handling is omitted. @@ -22,7 +25,7 @@ final class HwWalletManager { // MARK: - Published state - /// Paired hardware wallets, one per physical device, with aggregated balance. + /// Paired hardware wallets, one per wallet identity, with aggregated balance. private(set) var wallets: [HwWallet] = [] /// Sum of every paired wallet's balance. @@ -54,16 +57,10 @@ final class HwWalletManager { private var knownDevices: [TrezorKnownDevice] = [] private var connectedDeviceId: String? + private var connectedWalletId: String? private var watcherData: [String: HwWatcherData] = [:] private var activeWatchers: Set = [] private var activeWatcherElectrumUrls: [String: String] = [:] - - /// Xpub each active watcher was started with. The watcher id is only `deviceId|addressType`, so - /// the same physical device re-saved with a different xpub for that type (e.g. a passphrase/ - /// hidden wallet, or re-fetched accounts) keeps the same watcher id and derives a new wallet id. - /// Tracked here so `syncWatchers()` restarts the watcher on the new xpub instead of leaving the - /// old one feeding the old wallet's balance/activity under the new wallet id. - private var activeWatcherXpubs: [String: String] = [:] private var retryingWatcherStarts: Set = [] /// Watchers whose async start is dispatched but not yet confirmed in `activeWatchers`. @@ -124,10 +121,18 @@ final class HwWalletManager { /// Update the device snapshot and reconcile watchers. This is the manager's sole input: the /// composition root (`AppScene`) feeds it the current Trezor device list, so this type stays /// fully decoupled from `TrezorManager`. Also the test seam — tests drive it directly. - func updateDevices(knownDevices: [TrezorKnownDevice], connectedDeviceId: String?) { + /// + /// `connectedWalletId` is the identity the live session opened. A device holds one wallet open + /// at a time, so it is what decides which tile shows as connected. + func updateDevices( + knownDevices: [TrezorKnownDevice], + connectedDeviceId: String?, + connectedWalletId: String? = nil + ) { let previousWalletIds = hwWalletIds self.knownDevices = knownDevices self.connectedDeviceId = connectedDeviceId + self.connectedWalletId = connectedWalletId walletsLoaded = true syncWatchers() @@ -143,20 +148,17 @@ final class HwWalletManager { // MARK: - Control - /// Stop watching a paired hardware wallet and delete its stored activities. The caller is - /// responsible for forgetting the device entries (via `TrezorManager.forgetDevice`); the next - /// `updateDevices(...)` push then drops it from the tile list. - func removeDevice(id deviceId: String) { - let group = deviceGroups().first { $0.ids.contains(deviceId) } - let ids = group?.ids ?? [deviceId] - for watcherId in activeWatchers where ids.contains(self.deviceId(fromWatcherId: watcherId)) { + /// Stop watching a paired hardware wallet and delete its stored activities. Other wallets on the + /// same physical device are left untouched. The caller is responsible for forgetting the stored + /// entries (via `TrezorManager`); the next `updateDevices(...)` push then drops it from the tile + /// list. + func removeDevice(walletId: String) { + for watcherId in activeWatchers where self.walletId(fromWatcherId: watcherId) == walletId { _ = stopActiveWatcher(watcherId) } - if let group { - delete(walletId: group.walletId) - lastPersisted[group.walletId] = nil - } - if let device = knownDevices.first(where: { $0.id == deviceId }) { + delete(walletId: walletId) + lastPersisted[walletId] = nil + for device in knownDevices where device.resolvedWalletId == walletId { walletIdCache[xpubsSignature(device.xpubs)] = nil } recomputeDerivedState() @@ -181,27 +183,17 @@ final class HwWalletManager { ) } - /// Removes a hardware wallet and forgets every device entry that belongs to its wallet identity. + /// Removes a hardware wallet and forgets every stored entry that belongs to its wallet identity. func removeWallet( _ wallet: HwWallet, forgetDevice: (String) async -> Void ) async { - removeDevice(id: wallet.id) + removeDevice(walletId: wallet.id) for deviceId in wallet.deviceIds { await forgetDevice(deviceId) } } - /// The bitkit-core wallet id scoping a paired device's activities. Throws when the device is - /// unknown or has no captured xpubs, so callers that must write wallet-scoped data (e.g. the - /// pending Transfer To Spending activity) fail loudly rather than writing to the wrong wallet. - func walletId(forDevice deviceId: String) throws -> String { - guard let group = deviceGroups().first(where: { $0.ids.contains(deviceId) }) else { - throw AppError(message: "Unknown hardware wallet", debugMessage: "No wallet id for device '\(deviceId)'") - } - return group.walletId - } - // MARK: - Watcher orchestration /// Reconcile watchers in response to a settings change, but only when the monitored address @@ -226,9 +218,7 @@ final class HwWalletManager { // The next sync after it completes reconciles any electrum-url change. if pendingWatcherStarts.contains(spec.watcherId) { continue } let isActive = activeWatchers.contains(spec.watcherId) - if isActive, - activeWatcherElectrumUrls[spec.watcherId] == spec.electrumUrl, - activeWatcherXpubs[spec.watcherId] == spec.xpub { continue } + if isActive, activeWatcherElectrumUrls[spec.watcherId] == spec.electrumUrl { continue } if isActive, !stopActiveWatcher(spec.watcherId) { continue } startWatcher(spec) } @@ -245,8 +235,8 @@ final class HwWalletManager { } /// Build the watcher specs the current device/settings snapshot wants running: one per - /// (device, monitored address type), deduped by (addressType, xpub) and scoped to the - /// device's derived wallet id (devices without xpubs are skipped). + /// (wallet identity, monitored address type). Deduped by watcher id, which is what collapses the + /// same wallet stored more than once into a single watcher. Entries without xpubs are skipped. private func desiredWatcherSpecs() -> [WatcherSpec] { let monitored = monitoredTypesProvider() let electrumUrl = electrumUrlProvider() @@ -254,10 +244,11 @@ final class HwWalletManager { var seen = Set() var specs: [WatcherSpec] = [] for device in knownDevices { - guard let walletId = walletId(for: device.xpubs) else { continue } + guard let walletId = resolvedWalletId(for: device) else { continue } for (addressType, xpub) in device.xpubs where monitored.contains(addressType) { - guard seen.insert(dedupKey(addressType: addressType, xpub: xpub)).inserted else { continue } - specs.append(WatcherSpec(deviceId: device.id, walletId: walletId, addressType: addressType, xpub: xpub, electrumUrl: electrumUrl)) + let spec = WatcherSpec(walletId: walletId, addressType: addressType, xpub: xpub, electrumUrl: electrumUrl) + guard seen.insert(spec.watcherId).inserted else { continue } + specs.append(spec) } } return specs @@ -269,6 +260,14 @@ final class HwWalletManager { "\(addressType)\u{1}\(xpub)" } + /// The wallet identity a stored entry belongs to: the id it was saved with, or one derived from + /// its xpubs for entries written before the id was persisted. Returns nil when neither is + /// available (no captured xpubs), so callers skip the entry. + private func resolvedWalletId(for device: TrezorKnownDevice) -> String? { + if let walletId = device.walletId, !walletId.isEmpty { return walletId } + return walletId(for: device.xpubs) + } + /// Derive (and memoize) the wallet id for a device's xpubs. Returns nil when derivation fails /// (e.g. no captured xpubs — `HwWalletId.derive` throws on empty), so callers skip the device. private func walletId(for xpubs: [String: String]) -> String? { @@ -317,7 +316,6 @@ final class HwWalletManager { pendingWatcherStarts.remove(spec.watcherId) activeWatchers.insert(spec.watcherId) activeWatcherElectrumUrls[spec.watcherId] = spec.electrumUrl - activeWatcherXpubs[spec.watcherId] = spec.xpub retryingWatcherStarts.remove(spec.watcherId) syncWatchers() } catch { @@ -335,7 +333,6 @@ final class HwWalletManager { try watcherService.stopWatcher(watcherId: watcherId) activeWatchers.remove(watcherId) activeWatcherElectrumUrls[watcherId] = nil - activeWatcherXpubs[watcherId] = nil watcherData[watcherId] = nil listeners[watcherId] = nil return true @@ -358,20 +355,20 @@ final class HwWalletManager { /// Update aggregated state from a watcher event. The first event after a watcher starts /// delivers the full history (baseline); only later inbound txs are surfaced as received. /// Core builds the persistence-ready activities (core 0.3.4 watch-only watcher); the manager - /// stores, aggregates, and scopes them to the device. + /// stores, aggregates, and scopes them to the wallet. func handleWatcherEvent(watcherId: String, event: WatcherEvent) { guard case let .transactionsChanged(activities, transactionDetails, balance, _, _, _) = event else { return } - let deviceId = deviceId(fromWatcherId: watcherId) + let walletId = walletId(fromWatcherId: watcherId) let previous = watcherData[watcherId] watcherData[watcherId] = HwWatcherData( - deviceId: deviceId, + walletId: walletId, balanceSats: balance.total, activities: activities, transactionDetails: transactionDetails ) let groups = deviceGroups() recomputeDerivedState(groups: groups) - persistGroupSnapshot(forDevice: deviceId, groups: groups) + persistGroupSnapshot(forWallet: walletId, groups: groups) emitReceivedTxs(previous: previous, activities: activities) } @@ -395,9 +392,9 @@ final class HwWalletManager { // MARK: - Persistence - private func persistGroupSnapshot(forDevice deviceId: String, groups: [DeviceGroup]? = nil) { + private func persistGroupSnapshot(forWallet walletId: String, groups: [DeviceGroup]? = nil) { let groups = groups ?? deviceGroups() - guard let group = groups.first(where: { $0.ids.contains(deviceId) }) else { return } + guard let group = groups.first(where: { $0.walletId == walletId }) else { return } let missing = missingWatcherIds(for: group) let snapshot = mergedSnapshot(for: group, isComplete: missing.isEmpty) @@ -494,7 +491,7 @@ final class HwWalletManager { /// highest-ordered watcherId — rather than depending on dictionary iteration order. private func mergedSnapshot(for group: DeviceGroup, isComplete: Bool) -> HwWalletSnapshot { let watchers = watcherData - .filter { group.ids.contains($0.value.deviceId) } + .filter { $0.value.walletId == group.walletId } .sorted { $0.key < $1.key } .map(\.value) @@ -537,16 +534,20 @@ final class HwWalletManager { wallets = groups.map { group in let connectedDevice = group.devices.first { $0.id == connectedDeviceId } let device = connectedDevice ?? group.representative - let deviceWatchers = watcherData.values.filter { group.ids.contains($0.deviceId) } + let walletWatchers = watcherData.values.filter { $0.walletId == group.walletId } return HwWallet( - id: device.id, + id: group.walletId, walletId: group.walletId, name: device.displayName, model: device.model, - isConnected: connectedDevice != nil, - balanceSats: deviceWatchers.reduce(UInt64(0)) { $0.saturatingAdd($1.balanceSats) }, + // A device holding several passphrase wallets only has a session for one of them, + // and only that identity can sign; mark the others disconnected. A session opened + // before its identity was resolved reports none and stays inclusive. + isConnected: connectedDevice != nil && (connectedWalletId == nil || connectedWalletId == group.walletId), + balanceSats: walletWatchers.reduce(UInt64(0)) { $0.saturatingAdd($1.balanceSats) }, fundingBalanceSats: fundingBalance(group: group, addressType: hwFundingDefaultAddressType), - deviceIds: group.ids + deviceIds: group.ids, + passphraseProtected: group.devices.contains { $0.passphraseProtected } ) } @@ -554,13 +555,14 @@ final class HwWalletManager { hwWalletIds = Set(groups.map(\.walletId)) } - /// Group device entries sharing an xpub identity (same physical device over different - /// transports), preserving first-seen order. Entries without captured xpubs are skipped. + /// Group stored entries by wallet identity, preserving first-seen order. A passphrase wallet + /// derives different xpubs from its device's standard wallet, so it groups on its own. Entries + /// without captured xpubs are skipped. private func deviceGroups() -> [DeviceGroup] { var order: [String] = [] var grouped: [String: [TrezorKnownDevice]] = [:] for device in knownDevices where !device.xpubs.isEmpty { - guard let walletId = walletId(for: device.xpubs) else { continue } + guard let walletId = resolvedWalletId(for: device) else { continue } if grouped[walletId] == nil { order.append(walletId) } grouped[walletId, default: []].append(device) } @@ -572,39 +574,40 @@ final class HwWalletManager { // MARK: - Funding (transfer to spending) - /// The watch-only balance available to fund a transfer to spending from `deviceId`, sourced from + /// The watch-only balance available to fund a transfer to spending from `walletId`, sourced from /// the given address-type account only (v1: native segwit). Does not require a connected device. - func fundingBalance(deviceId: String, addressType: AddressScriptType = hwFundingDefaultAddressType) -> UInt64 { - guard let group = deviceGroups().first(where: { $0.ids.contains(deviceId) }) else { return 0 } - return fundingBalance(group: group, addressType: addressType) + func fundingBalance(walletId: String, addressType: AddressScriptType = hwFundingDefaultAddressType) -> UInt64 { + watcherData + .filter { $0.value.walletId == walletId && self.addressType(fromWatcherId: $0.key) == addressType.stringValue } + .reduce(UInt64(0)) { $0.saturatingAdd($1.value.balanceSats) } } private func fundingBalance(group: DeviceGroup, addressType: AddressScriptType) -> UInt64 { - watcherData - .filter { group.ids.contains($0.value.deviceId) && self.addressType(fromWatcherId: $0.key) == addressType.stringValue } - .reduce(UInt64(0)) { $0.saturatingAdd($1.value.balanceSats) } + fundingBalance(walletId: group.walletId, addressType: addressType) } - /// Resolve the funding account (xpub + watch-only balance) for a paired device. Does not require - /// a connected device — the xpub is read from the stored known-device record and the balance - /// from the running watchers. + /// Resolve the funding account (xpub + watch-only balance) for a paired wallet. Does not require + /// a connected device — the xpub is read from the stored entry and the balance from the running + /// watchers. On a device holding several wallets, the entry that actually carries the requested + /// account wins. func getFundingAccount( - deviceId: String, + walletId: String, addressType: AddressScriptType = hwFundingDefaultAddressType ) throws -> HwFundingAccount { - guard let device = knownDevices.first(where: { $0.id == deviceId }) else { - throw AppError(message: "Unknown hardware wallet", debugMessage: "No known device '\(deviceId)'") + let entries = knownDevices.filter { $0.resolvedWalletId == walletId } + guard !entries.isEmpty else { + throw AppError(message: "Unknown hardware wallet", debugMessage: "No known wallet '\(walletId)'") } - guard let xpub = device.xpubs[addressType.stringValue] else { + guard let xpub = entries.compactMap({ $0.xpubs[addressType.stringValue] }).first else { throw AppError( message: "Missing account", - debugMessage: "Device '\(deviceId)' has no '\(addressType.stringValue)' account xpub" + debugMessage: "Wallet '\(walletId)' has no '\(addressType.stringValue)' account xpub" ) } return HwFundingAccount( xpub: xpub, addressType: addressType, - balanceSats: fundingBalance(deviceId: deviceId, addressType: addressType) + balanceSats: fundingBalance(walletId: walletId, addressType: addressType) ) } @@ -613,12 +616,12 @@ final class HwWalletManager { /// required for signing — so this mirrors the software wallet's max-sendable estimate. /// `destinationAddress` is a fee-estimation destination only (never broadcast). func maxSpendableFunding( - deviceId: String, + walletId: String, destinationAddress: String, satsPerVByte: UInt64, addressType: AddressScriptType = hwFundingDefaultAddressType ) async throws -> UInt64 { - let account = try getFundingAccount(deviceId: deviceId, addressType: addressType) + let account = try getFundingAccount(walletId: walletId, addressType: addressType) let params = ComposeParams( wallet: WalletParams( extendedKey: account.xpub, @@ -650,7 +653,7 @@ final class HwWalletManager { /// Requires the device to be connected (the fingerprint drives the PSBT derivation paths); the /// caller must ensure the Trezor is connected first (via `TrezorManager`). func composeFundingTransaction( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64, @@ -658,7 +661,7 @@ final class HwWalletManager { ) async throws -> HwFundingTransaction { let fingerprint = try await TrezorService.shared.getDeviceFingerprint() return try await composeFundingTransactionInternal( - deviceId: deviceId, + walletId: walletId, address: address, sats: sats, satsPerVByte: satsPerVByte, @@ -669,14 +672,14 @@ final class HwWalletManager { /// Offline coin-selection for the exact funding amount; returns the mining fee only. func estimateOfflineFundingMiningFee( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64, addressType: AddressScriptType = hwFundingDefaultAddressType ) async throws -> UInt64 { let funding = try await composeFundingTransactionInternal( - deviceId: deviceId, + walletId: walletId, address: address, sats: sats, satsPerVByte: satsPerVByte, @@ -687,14 +690,14 @@ final class HwWalletManager { } private func composeFundingTransactionInternal( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64, fingerprint: String?, addressType: AddressScriptType ) async throws -> HwFundingTransaction { - let account = try getFundingAccount(deviceId: deviceId, addressType: addressType) + let account = try getFundingAccount(walletId: walletId, addressType: addressType) let network = networkProvider() let params = ComposeParams( wallet: WalletParams( @@ -734,7 +737,7 @@ final class HwWalletManager { /// `TrezorManager.disconnectStaleSession`). Broadcasting is a separate step so a device-signing /// timeout is never conflated with an in-flight broadcast. func signFunding( - deviceId _: String, + walletId _: String, funding: HwFundingTransaction ) async throws -> HwFundingSignedTx { let network = networkProvider() @@ -757,7 +760,7 @@ final class HwWalletManager { // MARK: - Helpers - private func deviceId(fromWatcherId watcherId: String) -> String { + private func walletId(fromWatcherId watcherId: String) -> String { guard let range = watcherId.range(of: Constants.watcherIdSeparator) else { return watcherId } return String(watcherId[.. Bool + ) { let trimmed = String(newName.trimmingCharacters(in: .whitespacesAndNewlines).prefix(Self.deviceLabelMaxLength)) let customLabel = trimmed.isEmpty ? nil : trimmed let updated = devices.map { device -> TrezorKnownDevice in - let sameGroup = device.id == id || (!target.xpubs.isEmpty && device.xpubs == target.xpubs) - guard sameGroup else { return device } + guard isTarget(device) else { return device } var copy = device copy.customLabel = customLabel return copy } TrezorKnownDeviceStorage.saveAll(updated) loadKnownDevices() - trezorLog("Renamed device \(id) to \(customLabel ?? "")") } /// Captures the connected device's account xpubs so watch-only balances/activity stay available @@ -791,3 +810,44 @@ final class TrezorManager { TrezorErrorPresenter.userMessage(from: error) } } + +// MARK: - Wallet-addressed session access + +/// The transfer flow addresses a hardware wallet by its identity, not by the transport it happens to +/// be reachable over: one device can hold a standard wallet plus its passphrase wallets, all sharing +/// a transport id. Resolving the identity to a transport id happens here so the session APIs stay +/// device-level. +extension TrezorManager: HwTransferConnecting { + func ensureConnected(walletId: String) async throws { + try await ensureConnected(deviceId: requireTransportDeviceId(forWallet: walletId)) + } + + func disconnectStaleSession(walletId: String) async { + guard let deviceId = transportDeviceId(forWallet: walletId) else { return } + await disconnectStaleSession(deviceId: deviceId) + } + + func isKnownBluetoothDevice(walletId: String) -> Bool { + guard let deviceId = transportDeviceId(forWallet: walletId) else { return false } + return isKnownBluetoothDevice(deviceId: deviceId) + } + + func warmUpConnection(walletId: String) { + guard let deviceId = transportDeviceId(forWallet: walletId) else { return } + warmUpConnection(deviceId: deviceId) + } + + /// Transport id to reach `walletId` with: the connected entry, else the most recently used one. + private func transportDeviceId(forWallet walletId: String) -> String? { + let entries = knownDevices.filter { $0.resolvedWalletId == walletId } + if let connected = entries.first(where: { $0.id == connectedDevice?.id }) { return connected.id } + return entries.max(by: { $0.lastConnectedAt < $1.lastConnectedAt })?.id + } + + private func requireTransportDeviceId(forWallet walletId: String) throws -> String { + guard let deviceId = transportDeviceId(forWallet: walletId) else { + throw AppError(message: "Unknown hardware wallet", debugMessage: "No paired device for wallet '\(walletId)'") + } + return deviceId + } +} diff --git a/Bitkit/Models/HwWallet.swift b/Bitkit/Models/HwWallet.swift index c752c8d56..4915ed5d5 100644 --- a/Bitkit/Models/HwWallet.swift +++ b/Bitkit/Models/HwWallet.swift @@ -1,13 +1,16 @@ import BitkitCore import Foundation -/// A paired hardware wallet tracked as a watch-only balance. +/// A paired hardware wallet tracked as a watch-only balance. One per wallet identity: a Trezor with +/// passphrase protection contributes its standard wallet plus one of these per hidden wallet. /// /// Activities are NOT held here — they are persisted in bitkit-core scoped by `walletId` /// and read back through the normal activity pipeline (see `HwWalletManager`). struct HwWallet: Identifiable { + /// The wallet identity, equal to `walletId`. Routes and every wallet-scoped API key off it, + /// since the transport-level device id is shared by all of a device's wallets. let id: String - /// bitkit-core wallet id scoping this device's activities (see `HwWalletId`). + /// bitkit-core wallet id scoping this wallet's activities (see `HwWalletId`). let walletId: String let name: String let model: String? @@ -17,7 +20,10 @@ struct HwWallet: Identifiable { /// (v1 funds from native-segwit; other address types are watched but not spent). Defaults to /// `balanceSats` when not computed separately. let fundingBalanceSats: UInt64 + /// Transport-level ids this wallet is reachable over. Bluetooth-only on iOS, so effectively one. let deviceIds: Set + /// Whether reaching this wallet needs a passphrase, i.e. it is a hidden wallet. + let passphraseProtected: Bool init( id: String, @@ -27,7 +33,8 @@ struct HwWallet: Identifiable { isConnected: Bool, balanceSats: UInt64, fundingBalanceSats: UInt64? = nil, - deviceIds: Set? = nil + deviceIds: Set? = nil, + passphraseProtected: Bool = false ) { self.id = id self.walletId = walletId @@ -37,6 +44,7 @@ struct HwWallet: Identifiable { self.balanceSats = balanceSats self.fundingBalanceSats = fundingBalanceSats ?? balanceSats self.deviceIds = deviceIds ?? [id] + self.passphraseProtected = passphraseProtected } } diff --git a/Bitkit/ViewModels/HwFundingSigner.swift b/Bitkit/ViewModels/HwFundingSigner.swift index 18876f655..6fef0d4f1 100644 --- a/Bitkit/ViewModels/HwFundingSigner.swift +++ b/Bitkit/ViewModels/HwFundingSigner.swift @@ -34,22 +34,22 @@ struct HwFundingSigner { /// Resolve the device balance and the amount available to fund. Prefers the real max-sendable /// (same coin-selection fee as the software wallet), falling back to the reserve estimate. func availability( - deviceId: String, + walletId: String, addressType: AddressScriptType = hwFundingDefaultAddressType ) async throws -> Availability { - let account = try funding.getFundingAccount(deviceId: deviceId, addressType: addressType) - let available = await maxSpendable(deviceId: deviceId, balanceSats: account.balanceSats, addressType: addressType) + let account = try funding.getFundingAccount(walletId: walletId, addressType: addressType) + let available = await maxSpendable(walletId: walletId, balanceSats: account.balanceSats, addressType: addressType) return Availability(balanceSats: account.balanceSats, available: available) } /// The amount available to fund. Computes the exact max-sendable via a `sendMax` compose at the /// target fee rate; when the fee rate, address, or compose is unavailable, falls back to the /// conservative reserve clamp. - private func maxSpendable(deviceId: String, balanceSats: UInt64, addressType: AddressScriptType) async -> UInt64 { + private func maxSpendable(walletId: String, balanceSats: UInt64, addressType: AddressScriptType) async -> UInt64 { if let satsPerVByte = await feeRateProvider(), let address = try? await addressProvider(), let spendable = try? await funding.maxSpendableFunding( - deviceId: deviceId, + walletId: walletId, destinationAddress: address, satsPerVByte: satsPerVByte, addressType: addressType @@ -70,15 +70,15 @@ struct HwFundingSigner { /// Reconnects, composes and signs the funding transaction without broadcasting it. func prepareSignedFunding( order: IBtOrder, - deviceId: String, + walletId: String, address: String, onComposed: (HwFundingTransaction) -> Void = { _ in } ) async throws -> HwFundingSignedTx { - try await ensureConnected(deviceId: deviceId) + try await ensureConnected(walletId: walletId) let satsPerVByte = await resolvedSatsPerVByte() - let tx = try await compose(deviceId: deviceId, address: address, sats: order.feeSat, satsPerVByte: satsPerVByte) + let tx = try await compose(walletId: walletId, address: address, sats: order.feeSat, satsPerVByte: satsPerVByte) onComposed(tx) - return try await signStep(deviceId: deviceId, funding: tx) + return try await signStep(walletId: walletId, funding: tx) } /// Broadcasts a signed funding transaction without requiring the hardware device. @@ -94,15 +94,15 @@ struct HwFundingSigner { /// Best-effort pre-connect of the device before signing (fire-and-forget). Delegates to the /// device-session capability, which no-ops unless it's a known BLE device that isn't connected. - func warmUp(deviceId: String) { - connecting.warmUpConnection(deviceId: deviceId) + func warmUp(walletId: String) { + connecting.warmUpConnection(walletId: walletId) } /// Offline compose for the exact order amount; does not require a connected device. - func estimateOfflineFundingMiningFee(deviceId: String, address: String, sats: UInt64) async throws -> UInt64 { + func estimateOfflineFundingMiningFee(walletId: String, address: String, sats: UInt64) async throws -> UInt64 { let satsPerVByte = await resolvedSatsPerVByte() return try await funding.estimateOfflineFundingMiningFee( - deviceId: deviceId, + walletId: walletId, address: address, sats: sats, satsPerVByte: satsPerVByte, @@ -110,22 +110,22 @@ struct HwFundingSigner { ) } - private func ensureConnected(deviceId: String) async throws { + private func ensureConnected(walletId: String) async throws { do { try await withTimeout(timeouts.reconnect) { - try await connecting.ensureConnected(deviceId: deviceId) + try await connecting.ensureConnected(walletId: walletId) } } catch is CancellationError { throw CancellationError() } catch { if error.isTrezorUserCancellation() { throw error } if error.isTrezorDeviceBusy() { throw HwTransferError.deviceBusy } - throw HwTransferError.reconnect(isBluetooth: connecting.isKnownBluetoothDevice(deviceId: deviceId)) + throw HwTransferError.reconnect(isBluetooth: connecting.isKnownBluetoothDevice(walletId: walletId)) } } private func compose( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64 @@ -133,7 +133,7 @@ struct HwFundingSigner { do { return try await withTimeout(timeouts.compose) { try await funding.composeFundingTransaction( - deviceId: deviceId, + walletId: walletId, address: address, sats: sats, satsPerVByte: satsPerVByte, @@ -143,7 +143,7 @@ struct HwFundingSigner { } catch is CancellationError { throw CancellationError() } catch is Timeout { - await connecting.disconnectStaleSession(deviceId: deviceId) + await connecting.disconnectStaleSession(walletId: walletId) throw HwTransferError.signingTimeout } catch { let message = (error as? AppError)?.debugMessage ?? (error as? AppError)?.message ?? error.localizedDescription @@ -151,15 +151,15 @@ struct HwFundingSigner { } } - private func signStep(deviceId: String, funding tx: HwFundingTransaction) async throws -> HwFundingSignedTx { + private func signStep(walletId: String, funding tx: HwFundingTransaction) async throws -> HwFundingSignedTx { do { return try await withTimeout(timeouts.sign) { - try await funding.signFunding(deviceId: deviceId, funding: tx) + try await funding.signFunding(walletId: walletId, funding: tx) } } catch is CancellationError { throw CancellationError() } catch is Timeout { - await connecting.disconnectStaleSession(deviceId: deviceId) + await connecting.disconnectStaleSession(walletId: walletId) throw HwTransferError.signingTimeout } // Any other (real signing) error propagates to the caller's generic handler. diff --git a/Bitkit/ViewModels/NavigationViewModel.swift b/Bitkit/ViewModels/NavigationViewModel.swift index a1554b4be..c861c3bef 100644 --- a/Bitkit/ViewModels/NavigationViewModel.swift +++ b/Bitkit/ViewModels/NavigationViewModel.swift @@ -5,7 +5,7 @@ import SwiftUI enum Route: Hashable { case savingsWallet case spendingWallet - case hardwareWallet(deviceId: String) + case hardwareWallet(walletId: String) case activityList case activityDetail(Activity) case activityExplorer(Activity) @@ -31,10 +31,10 @@ enum Route: Hashable { case transferIntro case fundingOptions case spendingIntro - case spendingIntroHw(deviceId: String) + case spendingIntroHw(walletId: String) case spendingAmount - case spendingAmountHw(deviceId: String) - case spendingHwSign(deviceId: String) + case spendingAmountHw(walletId: String) + case spendingHwSign(walletId: String) case spendingHwSigned case spendingConfirm(order: IBtOrder) case spendingAdvanced(order: IBtOrder) diff --git a/Bitkit/ViewModels/SheetViewModel.swift b/Bitkit/ViewModels/SheetViewModel.swift index e9dda2f3e..7ddfbe0be 100644 --- a/Bitkit/ViewModels/SheetViewModel.swift +++ b/Bitkit/ViewModels/SheetViewModel.swift @@ -332,7 +332,7 @@ class SheetViewModel: ObservableObject { get { guard let config = activeSheetConfiguration, config.id == .renameHardwareWallet else { return nil } guard let data = config.data as? RenameHardwareWalletConfig else { return nil } - return RenameHardwareWalletSheetItem(deviceId: data.deviceId, currentName: data.currentName) + return RenameHardwareWalletSheetItem(walletId: data.walletId, currentName: data.currentName) } set { if newValue == nil { diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index c85a4dda1..3babad03a 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -28,14 +28,14 @@ struct HwSpendingState: Equatable { private struct PendingHwFundingBroadcast { let orderId: String - let deviceId: String + let walletId: String let address: String let amountSats: UInt64 let signedTx: HwFundingSignedTx - func matches(order: IBtOrder, deviceId: String, address: String) -> Bool { + func matches(order: IBtOrder, walletId: String, address: String) -> Bool { orderId == order.id && - self.deviceId == deviceId && + self.walletId == walletId && self.address == address && amountSats == order.feeSat } @@ -63,18 +63,15 @@ enum HwTransferError: Error, Equatable { /// declared as a protocol so the flow stays testable. @MainActor protocol HwTransferFunding: Sendable { - /// bitkit-core wallet id scoping the device's activities, so a transfer funded from it is - /// recorded against that wallet rather than the normal Bitkit wallet. - func walletId(forDevice deviceId: String) throws -> String - func getFundingAccount(deviceId: String, addressType: AddressScriptType) throws -> HwFundingAccount + func getFundingAccount(walletId: String, addressType: AddressScriptType) throws -> HwFundingAccount func maxSpendableFunding( - deviceId: String, + walletId: String, destinationAddress: String, satsPerVByte: UInt64, addressType: AddressScriptType ) async throws -> UInt64 func composeFundingTransaction( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64, @@ -82,28 +79,29 @@ protocol HwTransferFunding: Sendable { ) async throws -> HwFundingTransaction /// Offline coin-selection estimate for the exact funding amount (`fingerprint: nil`); fee only. func estimateOfflineFundingMiningFee( - deviceId: String, + walletId: String, address: String, sats: UInt64, satsPerVByte: UInt64, addressType: AddressScriptType ) async throws -> UInt64 - func signFunding(deviceId: String, funding: HwFundingTransaction) async throws -> HwFundingSignedTx + func signFunding(walletId: String, funding: HwFundingTransaction) async throws -> HwFundingSignedTx func broadcastFunding(serializedTx: String) async throws -> String } -/// The device-session capability the transfer flow needs for on-device signing. Implemented by -/// `TrezorManager`. +/// The device-session capability the transfer flow needs for on-device signing, addressed by wallet +/// identity: a device holds one wallet open at a time, so reaching a given wallet is more than +/// reaching its transport. Implemented by `TrezorManager`. @MainActor protocol HwTransferConnecting: Sendable { - func ensureConnected(deviceId: String) async throws - func disconnectStaleSession(deviceId: String) async - /// Whether the device is a known Bluetooth device, so a reconnect failure can show the softer - /// BLE "check that it is unlocked and try again" toast instead of the generic reconnect error. - func isKnownBluetoothDevice(deviceId: String) -> Bool + func ensureConnected(walletId: String) async throws + func disconnectStaleSession(walletId: String) async + /// Whether the wallet is reachable over a known Bluetooth device, so a reconnect failure can show + /// the softer BLE "check that it is unlocked and try again" toast instead of the generic error. + func isKnownBluetoothDevice(walletId: String) -> Bool /// Best-effort pre-connect when the sign screen appears, so tapping Open Trezor Connect is less /// likely to hit a cold reconnect. Fire-and-forget. - func warmUpConnection(deviceId: String) + func warmUpConnection(walletId: String) } @MainActor @@ -145,7 +143,7 @@ class TransferViewModel: ObservableObject { private var refreshTask: Task? private var hwSignTask: Task? private var pendingHwFundingBroadcast: PendingHwFundingBroadcast? - private var activeHwTransferDeviceId: String? + private var activeHwTransferWalletId: String? private let retryInterval: TimeInterval = 60 // 1 min private let giveUpInterval: TimeInterval = 30 * 60 // 30 min @@ -534,7 +532,7 @@ class TransferViewModel: ObservableObject { /// the device's native-segwit balance minus an on-chain fee reserve, then the shared /// spending-limit calculation clamps it to the LSP receiving cap. func updateHwLimits( - deviceId: String, + walletId: String, blocktankInfo: IBtInfo?, estimateOrderFee: @escaping (_ clientBalance: UInt64, _ lspBalance: UInt64) async throws -> (networkFeeSat: UInt64, serviceFeeSat: UInt64) @@ -545,7 +543,7 @@ class TransferViewModel: ObservableObject { let availability: HwFundingSigner.Availability do { - availability = try await hwSigner.availability(deviceId: deviceId) + availability = try await hwSigner.availability(walletId: walletId) } catch { hwSpending = HwSpendingState(isLoading: false) hwTransferError = .generic((error as? AppError)?.message ?? error.localizedDescription) @@ -572,24 +570,24 @@ class TransferViewModel: ObservableObject { } /// Best-effort offline mining-fee estimate for the Sign screen (`fingerprint: nil` compose). - func updateHwFundingFeeEstimate(order: IBtOrder, deviceId: String) async { + func updateHwFundingFeeEstimate(order: IBtOrder, walletId: String) async { guard let hwSigner else { return } guard !hwSpending.hasPendingBroadcast else { return } guard let address = order.payment?.onchain?.address, !address.isEmpty else { return } do { hwSpending.miningFeeSats = try await hwSigner.estimateOfflineFundingMiningFee( - deviceId: deviceId, + walletId: walletId, address: address, sats: order.feeSat ) } catch { - Logger.debug("Skipped offline hardware funding fee estimate for '\(deviceId)'", context: "TransferViewModel") + Logger.debug("Skipped offline hardware funding fee estimate for '\(walletId)'", context: "TransferViewModel") } } /// Pay for the order by composing and signing the funding send on the Trezor (via the signer), /// then record and watch it. Coordination only — the device orchestration lives in `HwFundingSigner`. - func onTransferToSpendingHwConfirm(order: IBtOrder, deviceId: String) { + func onTransferToSpendingHwConfirm(order: IBtOrder, walletId: String) { guard !hwSpending.isSigning else { return } guard let hwSigner else { hwTransferError = .generic(t("common__error")) @@ -600,7 +598,7 @@ class TransferViewModel: ObservableObject { return } - activeHwTransferDeviceId = deviceId + activeHwTransferWalletId = walletId hwSpending.isSigning = true hwTransferError = nil @@ -612,25 +610,21 @@ class TransferViewModel: ObservableObject { } do { - // Resolved before signing: without it the transfer activity would be recorded - // against the wrong wallet, so fail before anything is broadcast. - let activityWalletId = try hwSigner.funding.walletId(forDevice: deviceId) - let signedTx: HwFundingSignedTx - if let pending = pendingHwFundingBroadcast, pending.matches(order: order, deviceId: deviceId, address: address) { + if let pending = pendingHwFundingBroadcast, pending.matches(order: order, walletId: walletId, address: address) { signedTx = pending.signedTx hwSpending.miningFeeSats = signedTx.miningFeeSats } else { signedTx = try await hwSigner.prepareSignedFunding( order: order, - deviceId: deviceId, + walletId: walletId, address: address ) { [weak self] funding in self?.hwSpending.miningFeeSats = funding.miningFeeSats } pendingHwFundingBroadcast = PendingHwFundingBroadcast( orderId: order.id, - deviceId: deviceId, + walletId: walletId, address: address, amountSats: order.feeSat, signedTx: signedTx @@ -645,21 +639,22 @@ class TransferViewModel: ObservableObject { createTransferActivity: true, fee: result.miningFeeSats, feeRate: result.feeRate, - activityWalletId: activityWalletId + // The wallet being spent from is the wallet the transfer is recorded against. + activityWalletId: walletId ) - activeHwTransferDeviceId = nil + activeHwTransferWalletId = nil hwFundingComplete = true hwSignedEvent += 1 } catch is CancellationError { // User dismissed the flow — no toast. } catch let error as HwTransferError { - self.handleHardwareTransferFailure(error, deviceId: deviceId) + self.handleHardwareTransferFailure(error, walletId: walletId) } catch { if error.isTrezorUserCancellation() { - Logger.info("Hardware transfer cancelled on device '\(deviceId)'", context: "TransferViewModel") + Logger.info("Hardware transfer cancelled on device for '\(walletId)'", context: "TransferViewModel") return } - handleRawHardwareTransferFailure(error, deviceId: deviceId) + handleRawHardwareTransferFailure(error, walletId: walletId) } } } @@ -667,9 +662,9 @@ class TransferViewModel: ObservableObject { /// Pre-connect the hardware device when the sign screen appears, mirroring Android's warm-up, so /// tapping Open Trezor Connect is less likely to hit a cold reconnect. Best-effort no-op without /// the HW capabilities. - func warmUpHardwareConnection(deviceId: String) { + func warmUpHardwareConnection(walletId: String) { guard !hwSpending.hasPendingBroadcast else { return } - hwSigner?.warmUp(deviceId: deviceId) + hwSigner?.warmUp(walletId: walletId) } /// Cancel an in-flight hardware signing task when the user abandons the sign flow, so a later @@ -677,14 +672,14 @@ class TransferViewModel: ObservableObject { /// is awaiting broadcast retry. func cancelHwSigning() { guard pendingHwFundingBroadcast == nil else { return } - let deviceId = activeHwTransferDeviceId + let walletId = activeHwTransferWalletId hwSignTask?.cancel() hwSignTask = nil hwSpending.isSigning = false - activeHwTransferDeviceId = nil - if let deviceId, let hwConnecting { + activeHwTransferWalletId = nil + if let walletId, let hwConnecting { Task { - await hwConnecting.disconnectStaleSession(deviceId: deviceId) + await hwConnecting.disconnectStaleSession(walletId: walletId) } } } @@ -698,29 +693,29 @@ class TransferViewModel: ObservableObject { hwSpending.hasPendingBroadcast = false } - private func handleHardwareTransferFailure(_ error: HwTransferError, deviceId: String) { + private func handleHardwareTransferFailure(_ error: HwTransferError, walletId: String) { switch error { case .reconnect: - Logger.error("Failed to reconnect hardware device '\(deviceId)'", context: "TransferViewModel") + Logger.error("Failed to reconnect hardware device '\(walletId)'", context: "TransferViewModel") case .signingTimeout: - Logger.warn("Timed out hardware transfer signing for '\(deviceId)'", context: "TransferViewModel") + Logger.warn("Timed out hardware transfer signing for '\(walletId)'", context: "TransferViewModel") case .broadcastUncertain: - Logger.warn("Hardware funding broadcast timed out (uncertain) for '\(deviceId)'", context: "TransferViewModel") + Logger.warn("Hardware funding broadcast timed out (uncertain) for '\(walletId)'", context: "TransferViewModel") case .broadcastConnectivity: - Logger.warn("Hardware funding broadcast connectivity failure for '\(deviceId)'", context: "TransferViewModel") + Logger.warn("Hardware funding broadcast connectivity failure for '\(walletId)'", context: "TransferViewModel") case .deviceBusy: - Logger.warn("Blocked hardware transfer for locked or busy Trezor '\(deviceId)'", context: "TransferViewModel") + Logger.warn("Blocked hardware transfer for locked or busy Trezor '\(walletId)'", context: "TransferViewModel") case .firmwareReconnect: - Logger.warn("Received Trezor firmware error for '\(deviceId)'", context: "TransferViewModel") + Logger.warn("Received Trezor firmware error for '\(walletId)'", context: "TransferViewModel") case let .funding(message): - Logger.warn("Failed to compose hardware funding for '\(deviceId)': \(message ?? "")", context: "TransferViewModel") + Logger.warn("Failed to compose hardware funding for '\(walletId)': \(message ?? "")", context: "TransferViewModel") case .generic: break } hwTransferError = error } - private func handleRawHardwareTransferFailure(_ error: Error, deviceId: String) { + private func handleRawHardwareTransferFailure(_ error: Error, walletId: String) { if error.isTrezorDeviceBusy() { hwTransferError = .deviceBusy return @@ -1512,4 +1507,3 @@ actor ChannelPendingCapture { // MARK: - Hardware transfer capability conformances extension HwWalletManager: HwTransferFunding {} -extension TrezorManager: HwTransferConnecting {} diff --git a/Bitkit/Views/Home/HomeWalletView.swift b/Bitkit/Views/Home/HomeWalletView.swift index f237f6206..136ce6e56 100644 --- a/Bitkit/Views/Home/HomeWalletView.swift +++ b/Bitkit/Views/Home/HomeWalletView.swift @@ -54,7 +54,7 @@ struct HomeWalletView: View { if !hwWalletManager.wallets.isEmpty { HardwareWalletsGrid(wallets: hwWalletManager.wallets) { hwWallet in - navigation.navigate(.hardwareWallet(deviceId: hwWallet.id)) + navigation.navigate(.hardwareWallet(walletId: hwWallet.id)) } .padding(.bottom, 32) } diff --git a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift index 618ec1854..e6b6a8b3c 100644 --- a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift +++ b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift @@ -80,7 +80,7 @@ struct HardwareWalletsSettingsScreen: View { onRename: { sheets.showSheet( .renameHardwareWallet, - data: RenameHardwareWalletConfig(deviceId: wallet.id, currentName: wallet.name) + data: RenameHardwareWalletConfig(walletId: wallet.id, currentName: wallet.name) ) }, onRemove: { pendingRemoval = wallet } diff --git a/Bitkit/Views/Sheets/RenameHardwareWalletSheet.swift b/Bitkit/Views/Sheets/RenameHardwareWalletSheet.swift index d13e4e75a..8f5282622 100644 --- a/Bitkit/Views/Sheets/RenameHardwareWalletSheet.swift +++ b/Bitkit/Views/Sheets/RenameHardwareWalletSheet.swift @@ -1,19 +1,19 @@ import SwiftUI struct RenameHardwareWalletConfig { - let deviceId: String + let walletId: String let currentName: String } struct RenameHardwareWalletSheetItem: SheetItem, Equatable { let id: SheetID = .renameHardwareWallet let size: SheetSize = .small - let deviceId: String + let walletId: String let currentName: String } /// Renames a paired hardware wallet: a single NAME field pre-filled with the current name and a Save -/// button. Persists the custom name via `TrezorManager.renameDevice`, which re-pushes the device +/// button. Persists the custom name via `TrezorManager.renameWallet`, which re-pushes the device /// snapshot so `HwWallet.name` updates everywhere. struct RenameHardwareWalletSheet: View { @Environment(TrezorManager.self) private var trezorManager @@ -67,7 +67,7 @@ struct RenameHardwareWalletSheet: View { private func save() { guard !trimmedName.isEmpty else { return } - trezorManager.renameDevice(id: config.deviceId, newName: trimmedName) + trezorManager.renameWallet(walletId: config.walletId, newName: trimmedName) sheets.hideSheet() } } diff --git a/Bitkit/Views/Transfer/Hardware/SpendingAmountHw.swift b/Bitkit/Views/Transfer/Hardware/SpendingAmountHw.swift index 145a205fd..eaaf2f0b3 100644 --- a/Bitkit/Views/Transfer/Hardware/SpendingAmountHw.swift +++ b/Bitkit/Views/Transfer/Hardware/SpendingAmountHw.swift @@ -6,7 +6,7 @@ import SwiftUI /// `SpendingAmount`, but the available/MAX/quarter limits come from the device's native-segwit /// balance via `TransferViewModel.updateHwLimits`, and Continue advances to the on-device Sign step. struct SpendingAmountHw: View { - let deviceId: String + let walletId: String @EnvironmentObject var app: AppViewModel @EnvironmentObject var blocktank: BlocktankViewModel @@ -29,14 +29,14 @@ struct SpendingAmountHw: View { /// Inputs the limit calculation depends on. Keying the `.task` on this reruns `updateHwLimits` /// when Blocktank info arrives after the screen opens (the LSP caps are otherwise nil → 0 limits). private struct HwLimitInputs: Equatable { - let deviceId: String + let walletId: String let maxChannelSizeSat: UInt64? let maxClientBalanceSat: UInt64? } private var hwLimitInputs: HwLimitInputs { HwLimitInputs( - deviceId: deviceId, + walletId: walletId, maxChannelSizeSat: blocktank.info?.options.maxChannelSizeSat, maxClientBalanceSat: blocktank.info?.options.maxClientBalanceSat ) @@ -110,7 +110,7 @@ struct SpendingAmountHw: View { .offlineOverlay(title: t("lightning__transfer__nav_title")) .task(id: hwLimitInputs) { await transfer.updateHwLimits( - deviceId: deviceId, + walletId: walletId, blocktankInfo: blocktank.info, estimateOrderFee: { clientBalance, lspBalance in let estimate = try await blocktank.estimateOrderFee(clientBalance: clientBalance, lspBalance: lspBalance) @@ -190,7 +190,7 @@ struct SpendingAmountHw: View { let order = try await blocktank.createOrder(clientBalance: amountSats, lspBalance: lspBalance) transfer.onOrderCreated(order: order) - navigation.navigate(.spendingHwSign(deviceId: deviceId)) + navigation.navigate(.spendingHwSign(walletId: walletId)) } catch { let appError = AppError(error: error) app.toast(type: .error, title: appError.message, description: appError.debugMessage) diff --git a/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift b/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift index c659def61..ef350a4fe 100644 --- a/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift +++ b/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift @@ -5,7 +5,7 @@ import SwiftUI /// transaction on the Trezor. Reuses the existing Learn More / Advanced controls; on-device signing /// replaces the local swipe-to-pay. Advances to the Signed screen on success. struct SpendingHwSign: View { - let deviceId: String + let walletId: String @EnvironmentObject var app: AppViewModel @EnvironmentObject var navigation: NavigationViewModel @@ -47,8 +47,8 @@ struct SpendingHwSign: View { .padding(.horizontal, 16) .bottomSafeAreaPadding() .task(id: order.id) { - transfer.warmUpHardwareConnection(deviceId: deviceId) - await transfer.updateHwFundingFeeEstimate(order: order, deviceId: deviceId) + transfer.warmUpHardwareConnection(walletId: walletId) + await transfer.updateHwFundingFeeEstimate(order: order, walletId: walletId) } .onChange(of: transfer.hwSignedEvent) { navigation.navigate(.spendingHwSigned) @@ -126,7 +126,7 @@ struct SpendingHwSign: View { isDisabled: transfer.hwSpending.isSigning, isLoading: transfer.hwSpending.isSigning ) { - transfer.onTransferToSpendingHwConfirm(order: order, deviceId: deviceId) + transfer.onTransferToSpendingHwConfirm(order: order, walletId: walletId) } .accessibilityIdentifier("HardwareTransferOpenTrezorConnect") } diff --git a/Bitkit/Views/Transfer/SpendingIntroView.swift b/Bitkit/Views/Transfer/SpendingIntroView.swift index 3b34ed8d2..5b9ea5f00 100644 --- a/Bitkit/Views/Transfer/SpendingIntroView.swift +++ b/Bitkit/Views/Transfer/SpendingIntroView.swift @@ -2,7 +2,7 @@ import SwiftUI struct SpendingIntroView: View { /// When set, this intro is for a hardware-wallet transfer; Continue routes to the HW amount flow. - var deviceId: String? + var walletId: String? @EnvironmentObject var app: AppViewModel @EnvironmentObject var navigation: NavigationViewModel @@ -16,8 +16,8 @@ struct SpendingIntroView: View { buttonText: t("lightning__spending_intro__button"), onButtonPress: { app.hasSeenTransferToSpendingIntro = true - if let deviceId { - navigation.navigate(.spendingAmountHw(deviceId: deviceId)) + if let walletId { + navigation.navigate(.spendingAmountHw(walletId: walletId)) } else { navigation.navigate(.spendingAmount) } diff --git a/Bitkit/Views/Wallets/HardwareWalletScreen.swift b/Bitkit/Views/Wallets/HardwareWalletScreen.swift index 3e2d6bbb6..2172bd607 100644 --- a/Bitkit/Views/Wallets/HardwareWalletScreen.swift +++ b/Bitkit/Views/Wallets/HardwareWalletScreen.swift @@ -6,7 +6,7 @@ import SwiftUI /// device's on-chain activity grouped by date (blue hardware icons), a Transfer-To-Spending /// placeholder on funded devices, and a Remove action. Ports bitkit-android's `HardwareWalletScreen`. struct HardwareWalletScreen: View { - let deviceId: String + let walletId: String @EnvironmentObject var activity: ActivityListViewModel @EnvironmentObject var app: AppViewModel @@ -18,7 +18,7 @@ struct HardwareWalletScreen: View { @State private var showRemoveDialog = false private var wallet: HwWallet? { - hwWalletManager.wallets.first { $0.deviceIds.contains(deviceId) } + hwWalletManager.wallets.first { $0.id == walletId } } var body: some View { @@ -133,9 +133,9 @@ struct HardwareWalletScreen: View { isDisabled: GeoService.shared.isGeoBlocked ) { if app.hasSeenTransferToSpendingIntro { - navigation.navigate(.spendingAmountHw(deviceId: deviceId)) + navigation.navigate(.spendingAmountHw(walletId: walletId)) } else { - navigation.navigate(.spendingIntroHw(deviceId: deviceId)) + navigation.navigate(.spendingIntroHw(walletId: walletId)) } } .accessibilityIdentifier("HardwareTransferToSpending") diff --git a/BitkitTests/HwFundingSignerTests.swift b/BitkitTests/HwFundingSignerTests.swift index 8e2e4ed49..5cb939934 100644 --- a/BitkitTests/HwFundingSignerTests.swift +++ b/BitkitTests/HwFundingSignerTests.swift @@ -47,7 +47,7 @@ final class HwFundingSignerTests: XCTestCase { funding.maxSpendable = 990_000 let signer = makeSigner(funding: funding, connecting: MockHwConnecting(), feeRate: 2) - let availability = try await signer.availability(deviceId: "dev1") + let availability = try await signer.availability(walletId: "trezor:wallet") XCTAssertEqual(availability.balanceSats, 1_000_000) XCTAssertEqual(availability.available, 990_000, "available comes from the real sendMax estimate") @@ -61,7 +61,7 @@ final class HwFundingSignerTests: XCTestCase { funding.maxSpendable = 990_000 let signer = makeSigner(funding: funding, connecting: MockHwConnecting(), feeRate: 2) - let availability = try await signer.availability(deviceId: "dev1") + let availability = try await signer.availability(walletId: "trezor:wallet") XCTAssertEqual(availability.available, 800_000, "available is clamped to the device balance") } @@ -72,7 +72,7 @@ final class HwFundingSignerTests: XCTestCase { funding.maxSpendableError = MockHwFunding.TestError() let signer = makeSigner(funding: funding, connecting: MockHwConnecting(), feeRate: 2) - let availability = try await signer.availability(deviceId: "dev1") + let availability = try await signer.availability(walletId: "trezor:wallet") XCTAssertEqual(availability.available, 1_000_000 - 2 * 1200, "falls back to the reserve estimate") } @@ -82,7 +82,7 @@ final class HwFundingSignerTests: XCTestCase { funding.account = HwFundingAccount(xpub: "zpubNS", addressType: .nativeSegwit, balanceSats: 1_000_000) let signer = makeSigner(funding: funding, connecting: MockHwConnecting(), feeRate: 2, address: nil) - let availability = try await signer.availability(deviceId: "dev1") + let availability = try await signer.availability(walletId: "trezor:wallet") XCTAssertTrue(funding.maxSpendableCalls.isEmpty, "no estimate without a destination address") XCTAssertEqual(availability.available, 1_000_000 - 2 * 1200) @@ -98,7 +98,7 @@ final class HwFundingSignerTests: XCTestCase { let signed = try await signer.prepareSignedFunding( order: order, - deviceId: "dev1", + walletId: "trezor:wallet", address: XCTUnwrap(order.payment?.onchain?.address), onComposed: { composedMiningFee = $0.miningFeeSats } ) @@ -119,7 +119,7 @@ final class HwFundingSignerTests: XCTestCase { let signer = makeSigner(funding: funding, connecting: connecting) await assertThrowsAsync { - _ = try await signer.prepareSignedFunding(order: .mock(), deviceId: "dev1", address: "bc1q...") + _ = try await signer.prepareSignedFunding(order: .mock(), walletId: "trezor:wallet", address: "bc1q...") } _: { error in XCTAssertEqual(error as? HwTransferError, .reconnect(isBluetooth: false)) } @@ -133,7 +133,7 @@ final class HwFundingSignerTests: XCTestCase { let signer = makeSigner(funding: funding, connecting: MockHwConnecting()) await assertThrowsAsync { - _ = try await signer.prepareSignedFunding(order: .mock(), deviceId: "dev1", address: "bc1q...") + _ = try await signer.prepareSignedFunding(order: .mock(), walletId: "trezor:wallet", address: "bc1q...") } _: { error in if case .funding = error as? HwTransferError {} else { XCTFail("expected .funding, got \(error)") } } @@ -147,11 +147,11 @@ final class HwFundingSignerTests: XCTestCase { let signer = makeSigner(funding: funding, connecting: connecting, timeouts: (reconnect: 5, compose: 5, sign: 0.05, broadcast: 5)) await assertThrowsAsync { - _ = try await signer.prepareSignedFunding(order: .mock(), deviceId: "dev1", address: "bc1q...") + _ = try await signer.prepareSignedFunding(order: .mock(), walletId: "trezor:wallet", address: "bc1q...") } _: { error in XCTAssertEqual(error as? HwTransferError, .signingTimeout) } - XCTAssertEqual(connecting.staleDisconnects, ["dev1"]) + XCTAssertEqual(connecting.staleDisconnects, ["trezor:wallet"]) XCTAssertEqual(funding.signCalls, 1) } @@ -209,11 +209,11 @@ final class HwFundingSignerTests: XCTestCase { let signer = makeSigner(funding: funding, connecting: connecting, timeouts: (reconnect: 5, compose: 0.05, sign: 5, broadcast: 5)) await assertThrowsAsync { - _ = try await signer.prepareSignedFunding(order: .mock(), deviceId: "dev1", address: "bc1q...") + _ = try await signer.prepareSignedFunding(order: .mock(), walletId: "trezor:wallet", address: "bc1q...") } _: { error in XCTAssertEqual(error as? HwTransferError, .signingTimeout) } - XCTAssertEqual(connecting.staleDisconnects, ["dev1"], "a compose timeout must tear down the stale session") + XCTAssertEqual(connecting.staleDisconnects, ["trezor:wallet"], "a compose timeout must tear down the stale session") XCTAssertEqual(funding.signCalls, 0, "signing must not run after a compose timeout") } @@ -224,7 +224,7 @@ final class HwFundingSignerTests: XCTestCase { let signer = makeSigner(funding: funding, connecting: connecting) await assertThrowsAsync { - _ = try await signer.prepareSignedFunding(order: .mock(), deviceId: "dev1", address: "bc1q...") + _ = try await signer.prepareSignedFunding(order: .mock(), walletId: "trezor:wallet", address: "bc1q...") } _: { error in XCTAssertTrue(error is MockHwFunding.TestError, "a real signing error must propagate unwrapped") XCTAssertNil(error as? HwTransferError) diff --git a/BitkitTests/HwTransferMocks.swift b/BitkitTests/HwTransferMocks.swift index e05250c02..4dd25a18f 100644 --- a/BitkitTests/HwTransferMocks.swift +++ b/BitkitTests/HwTransferMocks.swift @@ -20,8 +20,6 @@ final class MockHwFunding: HwTransferFunding { var funding = HwFundingTransaction(psbt: "psbt", miningFeeSats: 141, feeRate: 1, totalSpent: 43186, satsPerVByte: 1) var signedTx = HwFundingSignedTx(serializedTx: "rawtx", miningFeeSats: 141, feeRate: 1, totalSpent: 43186) var broadcastTxId = "txid" - var walletId = "trezor:wallet" - var walletIdError: Error? private(set) var composeCalls: [(address: String, sats: UInt64, satsPerVByte: UInt64)] = [] private(set) var estimateCalls: [(address: String, sats: UInt64, satsPerVByte: UInt64)] = [] @@ -29,18 +27,13 @@ final class MockHwFunding: HwTransferFunding { private(set) var signCalls = 0 private(set) var broadcastCalls = 0 - func walletId(forDevice _: String) throws -> String { - if let walletIdError { throw walletIdError } - return walletId - } - - func getFundingAccount(deviceId _: String, addressType _: AddressScriptType) throws -> HwFundingAccount { + func getFundingAccount(walletId _: String, addressType _: AddressScriptType) throws -> HwFundingAccount { if let accountError { throw accountError } return account } func maxSpendableFunding( - deviceId _: String, + walletId _: String, destinationAddress: String, satsPerVByte: UInt64, addressType _: AddressScriptType @@ -51,7 +44,7 @@ final class MockHwFunding: HwTransferFunding { } func composeFundingTransaction( - deviceId _: String, + walletId _: String, address: String, sats: UInt64, satsPerVByte: UInt64, @@ -64,7 +57,7 @@ final class MockHwFunding: HwTransferFunding { } func estimateOfflineFundingMiningFee( - deviceId _: String, + walletId _: String, address: String, sats: UInt64, satsPerVByte: UInt64, @@ -75,7 +68,7 @@ final class MockHwFunding: HwTransferFunding { return funding.miningFeeSats } - func signFunding(deviceId _: String, funding _: HwFundingTransaction) async throws -> HwFundingSignedTx { + func signFunding(walletId _: String, funding _: HwFundingTransaction) async throws -> HwFundingSignedTx { signCalls += 1 if signDelay > 0 { try await Task.sleep(nanoseconds: UInt64(signDelay * 1_000_000_000)) } if let signError { throw signError } @@ -98,20 +91,20 @@ final class MockHwConnecting: HwTransferConnecting { private(set) var staleDisconnects: [String] = [] private(set) var warmUpCalls: [String] = [] - func ensureConnected(deviceId _: String) async throws { + func ensureConnected(walletId _: String) async throws { ensureCalls += 1 if let connectError { throw connectError } } - func isKnownBluetoothDevice(deviceId _: String) -> Bool { + func isKnownBluetoothDevice(walletId _: String) -> Bool { isBluetooth } - func warmUpConnection(deviceId: String) { - warmUpCalls.append(deviceId) + func warmUpConnection(walletId: String) { + warmUpCalls.append(walletId) } - func disconnectStaleSession(deviceId: String) async { - staleDisconnects.append(deviceId) + func disconnectStaleSession(walletId: String) async { + staleDisconnects.append(walletId) } } diff --git a/BitkitTests/HwWalletManagerFundingTests.swift b/BitkitTests/HwWalletManagerFundingTests.swift index 8f993c8cc..275645ff8 100644 --- a/BitkitTests/HwWalletManagerFundingTests.swift +++ b/BitkitTests/HwWalletManagerFundingTests.swift @@ -28,7 +28,8 @@ final class HwWalletManagerFundingTests: XCTestCase { } private func makeDevice(id: String, xpubs: [String: String]) -> TrezorKnownDevice { - TrezorKnownDevice( + xpubsByDeviceId[id] = xpubs + return TrezorKnownDevice( id: id, name: id, path: "ble:\(id)", @@ -54,8 +55,16 @@ final class HwWalletManagerFundingTests: XCTestCase { ) } + /// Watcher ids are keyed by wallet identity, so they are derived from the device's xpubs. + /// `makeDevice` records them here so tests can keep naming devices by their transport id. + private var xpubsByDeviceId: [String: [String: String]] = [:] + private func watcherId(_ deviceId: String, _ addressType: String) -> String { - "\(deviceId)|\(addressType)" + "\(walletId(deviceId))|\(addressType)" + } + + private func walletId(_ deviceId: String) -> String { + (try? HwWalletId.derive(xpubs: xpubsByDeviceId[deviceId] ?? [:])) ?? deviceId } func testFundingBalanceIsNativeSegwitOnly() throws { @@ -69,8 +78,8 @@ final class HwWalletManagerFundingTests: XCTestCase { let wallet = try XCTUnwrap(manager.wallets.first) XCTAssertEqual(wallet.balanceSats, 80000, "aggregate balance spans all address types") XCTAssertEqual(wallet.fundingBalanceSats, 50000, "funding balance is native-segwit only") - XCTAssertEqual(manager.fundingBalance(deviceId: "dev1"), 50000) - XCTAssertEqual(manager.fundingBalance(deviceId: "dev1", addressType: .taproot), 30000) + XCTAssertEqual(manager.fundingBalance(walletId: walletId("dev1")), 50000) + XCTAssertEqual(manager.fundingBalance(walletId: walletId("dev1"), addressType: .taproot), 30000) } func testGetFundingAccountReturnsNativeSegwitXpubAndBalance() throws { @@ -79,7 +88,7 @@ final class HwWalletManagerFundingTests: XCTestCase { manager.updateDevices(knownDevices: [device], connectedDeviceId: "dev1") manager.handleWatcherEvent(watcherId: watcherId("dev1", "nativeSegwit"), event: makeEvent(total: 42000)) - let account = try manager.getFundingAccount(deviceId: "dev1") + let account = try manager.getFundingAccount(walletId: walletId("dev1")) XCTAssertEqual(account.xpub, "zpubNS") XCTAssertEqual(account.addressType, .nativeSegwit) XCTAssertEqual(account.accountType, .nativeSegwit) @@ -89,13 +98,13 @@ final class HwWalletManagerFundingTests: XCTestCase { func testGetFundingAccountThrowsForUnknownDevice() { let manager = makeManager() manager.updateDevices(knownDevices: [], connectedDeviceId: nil) - XCTAssertThrowsError(try manager.getFundingAccount(deviceId: "nope")) + XCTAssertThrowsError(try manager.getFundingAccount(walletId: walletId("nope"))) } func testGetFundingAccountThrowsWhenNativeSegwitAccountMissing() { let device = makeDevice(id: "dev1", xpubs: ["taproot": "zpubTR"]) let manager = makeManager() manager.updateDevices(knownDevices: [device], connectedDeviceId: "dev1") - XCTAssertThrowsError(try manager.getFundingAccount(deviceId: "dev1")) + XCTAssertThrowsError(try manager.getFundingAccount(walletId: walletId("dev1"))) } } diff --git a/BitkitTests/HwWalletManagerTests.swift b/BitkitTests/HwWalletManagerTests.swift index 3e8ae89ea..d782755b9 100644 --- a/BitkitTests/HwWalletManagerTests.swift +++ b/BitkitTests/HwWalletManagerTests.swift @@ -88,6 +88,7 @@ final class HwWalletManagerTests: XCTestCase { deleted = [] receivedTxs = [] cancellables = [] + xpubsByDeviceId = [:] } // MARK: - Factories @@ -117,7 +118,8 @@ final class HwWalletManagerTests: XCTestCase { model: String? = "Safe 5", lastConnectedAt: Date = Date(timeIntervalSince1970: 1000) ) -> TrezorKnownDevice { - TrezorKnownDevice( + xpubsByDeviceId[id] = xpubs + return TrezorKnownDevice( id: id, name: id, path: "ble:\(id)", @@ -209,8 +211,20 @@ final class HwWalletManagerTests: XCTestCase { ) } + /// Watcher ids are keyed by wallet identity, so they are derived from the device's xpubs. + /// `makeDevice` records them here so tests can keep naming devices by their transport id. + private var xpubsByDeviceId: [String: [String: String]] = [:] + private func watcherId(_ deviceId: String, _ addressType: String) -> String { - "\(deviceId)|\(addressType)" + let derived = (try? HwWalletId.derive(xpubs: xpubsByDeviceId[deviceId] ?? [:])) ?? deviceId + return "\(derived)|\(addressType)" + } + + /// Needed when one device id holds several identities, where the registry above can only + /// remember the last one written for it. + private func watcherId(_ device: TrezorKnownDevice, _ addressType: String) -> String { + let derived = (try? HwWalletId.derive(xpubs: device.xpubs)) ?? device.id + return "\(derived)|\(addressType)" } // MARK: - Tests @@ -227,7 +241,7 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(vm.wallets.count, 1) let wallet = vm.wallets[0] - XCTAssertEqual(wallet.id, "dev1") + XCTAssertEqual(wallet.id, wallet.walletId, "a wallet is identified by its wallet id, not by its transport") XCTAssertEqual(wallet.balanceSats, 50000) XCTAssertEqual(wallet.name, "Trezor Safe 5") XCTAssertTrue(wallet.isConnected) @@ -256,8 +270,8 @@ final class HwWalletManagerTests: XCTestCase { func testSamePhysicalDeviceDedupedByXpub() { // Same xpubs, two device entries (e.g. re-paired) → one wallet, one walletId. let xpubs = ["nativeSegwit": "zpubShared"] - let ble = makeDevice(id: "ble1", xpubs: xpubs, lastConnectedAt: Date(timeIntervalSince1970: 1000)) - let usb = makeDevice(id: "usb1", xpubs: xpubs, lastConnectedAt: Date(timeIntervalSince1970: 2000)) + let ble = makeDevice(id: "ble1", xpubs: xpubs, label: "Older", lastConnectedAt: Date(timeIntervalSince1970: 1000)) + let usb = makeDevice(id: "usb1", xpubs: xpubs, label: "Newer", lastConnectedAt: Date(timeIntervalSince1970: 2000)) let vm = makeViewModel() vm.updateDevices(knownDevices: [ble, usb], connectedDeviceId: nil) @@ -267,8 +281,9 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(vm.wallets.count, 1) XCTAssertEqual(vm.wallets[0].deviceIds, ["ble1", "usb1"]) - // Representative is the most recently connected entry. - XCTAssertEqual(vm.wallets[0].id, "usb1") + XCTAssertEqual(vm.wallets[0].balanceSats, 70000, "one watcher feeds the single identity") + // The entries share an identity, so the most recently connected one names it. + XCTAssertEqual(vm.wallets[0].name, "Newer") XCTAssertEqual(vm.hwWalletIds.count, 1) } @@ -363,15 +378,6 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(persistedSnapshots.count, 2) } - func testWalletIdForDeviceMatchesDerivedIdAndThrowsForUnknownDevice() throws { - let xpubs = ["nativeSegwit": "zpubNS"] - let vm = makeViewModel() - vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) - - XCTAssertEqual(try vm.walletId(forDevice: "dev1"), try HwWalletId.derive(xpubs: xpubs)) - XCTAssertThrowsError(try vm.walletId(forDevice: "unknown")) - } - func testUnchangedWatcherEventDoesNotRepersist() async { let device = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zpubNS"]) let vm = makeViewModel() @@ -592,24 +598,26 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(params?.accountType, .nativeSegwit) } - func testWatcherRestartsWhenXpubChangesForSameDeviceAndType() async { + func testWatcherMovesToTheNewIdWhenTheWalletIdChanges() async { let mock = MockWatcherService() + let original = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "z"]) let vm = makeViewModel(watcherService: mock, monitored: ["nativeSegwit"]) - vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: ["nativeSegwit": "z"])], connectedDeviceId: nil) + vm.updateDevices(knownDevices: [original], connectedDeviceId: nil) await waitUntil { mock.startedParams.count == 1 } - vm.handleWatcherEvent(watcherId: watcherId("dev1", "nativeSegwit"), event: makeEvent( + vm.handleWatcherEvent(watcherId: watcherId(original, "nativeSegwit"), event: makeEvent( [makeActivity(txId: "t1", value: 40000, txType: .received)], total: 40000 )) let originalWalletId = vm.wallets.first?.walletId - // Same device id + address type, new xpub (e.g. a passphrase/hidden wallet, or re-fetched - // accounts): the watcher id is unchanged but the watched key — and the derived wallet id — - // differ, so the old watcher must be torn down and a new one started on the new xpub - // instead of feeding the old wallet's balance under the new wallet id. - vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: ["nativeSegwit": "z2"])], connectedDeviceId: nil) + // Same device id + address type, new xpub (e.g. re-fetched accounts): the wallet id derives + // from the key material, so this is a different identity and a different watcher id. The old + // watcher must be torn down rather than left feeding the old balance under the new identity. + let rekeyed = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "z2"]) + vm.updateDevices(knownDevices: [rekeyed], connectedDeviceId: nil) await waitUntil { mock.startedParams.count == 2 } - XCTAssertTrue(mock.stoppedWatcherIds.contains(watcherId("dev1", "nativeSegwit"))) + XCTAssertTrue(mock.stoppedWatcherIds.contains(watcherId(original, "nativeSegwit"))) + XCTAssertEqual(mock.startedParams.last?.watcherId, watcherId(rekeyed, "nativeSegwit")) XCTAssertEqual(mock.startedParams.last?.extendedKey, "z2") XCTAssertNotEqual(vm.wallets.first?.walletId, originalWalletId) XCTAssertEqual(vm.wallets.first?.balanceSats, 0, "stale old-xpub balance is dropped until the new watcher reports") @@ -700,16 +708,74 @@ final class HwWalletManagerTests: XCTestCase { func testConnectedEntryWinsRepresentativeIdentity() { // Same xpub over two entries; the more recent is `ble1`, but `usb1` is connected. let xpubs = ["nativeSegwit": "shared"] - let ble = makeDevice(id: "ble1", xpubs: xpubs, lastConnectedAt: Date(timeIntervalSince1970: 2000)) - let usb = makeDevice(id: "usb1", xpubs: xpubs, lastConnectedAt: Date(timeIntervalSince1970: 1000)) + let ble = makeDevice(id: "ble1", xpubs: xpubs, label: "Ble", lastConnectedAt: Date(timeIntervalSince1970: 2000)) + let usb = makeDevice(id: "usb1", xpubs: xpubs, label: "Usb", lastConnectedAt: Date(timeIntervalSince1970: 1000)) let vm = makeViewModel() vm.updateDevices(knownDevices: [ble, usb], connectedDeviceId: "usb1") XCTAssertEqual(vm.wallets.count, 1) - XCTAssertEqual(vm.wallets[0].id, "usb1") + XCTAssertEqual(vm.wallets[0].name, "Usb", "the connected entry names the wallet") XCTAssertTrue(vm.wallets[0].isConnected) } + // MARK: - Several wallet identities on one device + + func testPassphraseWalletIsWatchedNextToTheStandardWalletOfTheSameDevice() { + let standard = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zHidden"]) + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [standard, hidden], connectedDeviceId: "dev1") + + vm.handleWatcherEvent(watcherId: watcherId(standard, "nativeSegwit"), event: makeEvent([], total: 30000)) + vm.handleWatcherEvent(watcherId: watcherId(hidden, "nativeSegwit"), event: makeEvent([], total: 20000)) + + XCTAssertEqual(vm.wallets.count, 2, "one tile per identity, not per device") + XCTAssertEqual(vm.wallets.map(\.balanceSats), [30000, 20000], "each identity counts its own balance") + XCTAssertEqual(vm.totalSats, 50000) + XCTAssertEqual(vm.hwWalletIds.count, 2) + } + + /// A device only holds one wallet open at a time, and only that identity can sign. + func testOnlyTheIdentityHoldingTheSessionShowsAsConnected() throws { + let standard = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zHidden"]) + let hiddenWalletId = try HwWalletId.derive(xpubs: hidden.xpubs) + let vm = makeViewModel(monitored: ["nativeSegwit"]) + + vm.updateDevices(knownDevices: [standard, hidden], connectedDeviceId: "dev1", connectedWalletId: hiddenWalletId) + + let connected = vm.wallets.filter(\.isConnected) + XCTAssertEqual(connected.map(\.id), [hiddenWalletId]) + } + + /// A session opened before its identity could be resolved reports none and stays inclusive. + func testUnresolvedSessionLeavesEveryIdentityOnTheDeviceConnected() { + let standard = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zHidden"]) + let vm = makeViewModel(monitored: ["nativeSegwit"]) + + vm.updateDevices(knownDevices: [standard, hidden], connectedDeviceId: "dev1", connectedWalletId: nil) + + XCTAssertEqual(vm.wallets.filter(\.isConnected).count, 2) + } + + func testRemovingOneIdentityLeavesTheOtherWatched() async throws { + let mock = MockWatcherService() + let standard = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zStandard"]) + let hidden = makeDevice(id: "dev1", xpubs: ["nativeSegwit": "zHidden"]) + let hiddenWalletId = try HwWalletId.derive(xpubs: hidden.xpubs) + let vm = makeViewModel(watcherService: mock, monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [standard, hidden], connectedDeviceId: nil) + await waitUntil { mock.startedParams.count == 2 } + + vm.removeDevice(walletId: hiddenWalletId) + await vm.drainPendingPersists() + + XCTAssertEqual(mock.stoppedWatcherIds, [watcherId(hidden, "nativeSegwit")], "only the removed identity stops watching") + XCTAssertEqual(deleted, [hiddenWalletId], "only the removed identity's activities are deleted") + XCTAssertTrue(vm.wallets.contains { $0.id == (try? HwWalletId.derive(xpubs: standard.xpubs)) }) + } + func testTotalSatsSaturatesInsteadOfOverflowing() { let d1 = makeDevice(id: "d1", xpubs: ["nativeSegwit": "a"]) let d2 = makeDevice(id: "d2", xpubs: ["nativeSegwit": "b"]) @@ -734,7 +800,7 @@ final class HwWalletManagerTests: XCTestCase { vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) await waitUntil { mock.startedParams.count == 1 } - vm.handleWatcherEvent(watcherId: watcherId("dev1", "nativeSegwit"), event: makeEvent( + vm.handleWatcherEvent(watcherId: watcherId(device, "nativeSegwit"), event: makeEvent( [makeActivity(txId: "t1", value: 40000, txType: .received)], total: 40000 )) XCTAssertEqual(vm.wallets.first?.balanceSats, 40000) @@ -742,7 +808,7 @@ final class HwWalletManagerTests: XCTestCase { // Stop fails → the watcher must stay active and keep feeding its balance. mock.stopShouldFail = true vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: [:])], connectedDeviceId: nil) - XCTAssertTrue(mock.stoppedWatcherIds.contains(watcherId("dev1", "nativeSegwit"))) + XCTAssertTrue(mock.stoppedWatcherIds.contains(watcherId(device, "nativeSegwit"))) // Stop now succeeds → next sync removes it. mock.stopShouldFail = false @@ -761,7 +827,7 @@ final class HwWalletManagerTests: XCTestCase { [makeActivity(txId: "t1", value: 1000, txType: .received)], total: 1000 )) - vm.removeDevice(id: "dev1") + try vm.removeDevice(walletId: HwWalletId.derive(xpubs: xpubs)) XCTAssertTrue(mock.stoppedWatcherIds.contains(watcherId("dev1", "nativeSegwit"))) await vm.drainPendingPersists() diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index 87e0c2700..e0c6f874e 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -30,7 +30,7 @@ final class TransferViewModelHwTests: XCTestCase { func testConfirmWithoutHwCapabilitiesSurfacesGenericError() { let vm = TransferViewModel() // no signer injected - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") } XCTAssertFalse(vm.hwSpending.isSigning) } @@ -40,7 +40,7 @@ final class TransferViewModelHwTests: XCTestCase { funding.accountError = MockHwFunding.TestError() let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) - await vm.updateHwLimits(deviceId: "dev1", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) + await vm.updateHwLimits(walletId: "trezor:wallet", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") } XCTAssertFalse(vm.hwSpending.isLoading) @@ -52,7 +52,7 @@ final class TransferViewModelHwTests: XCTestCase { let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) vm.hwSpending.maxAllowedToSend = 999_999 // stale cap from a previously-selected device - await vm.updateHwLimits(deviceId: "dev1", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) + await vm.updateHwLimits(walletId: "trezor:wallet", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) XCTAssertEqual(vm.hwSpending.maxAllowedToSend, 0, "a reload must not keep the previous device's cap") } @@ -63,7 +63,7 @@ final class TransferViewModelHwTests: XCTestCase { connecting.connectError = MockHwFunding.TestError() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(vm.hwTransferError, .reconnect(isBluetooth: false)) @@ -77,7 +77,7 @@ final class TransferViewModelHwTests: XCTestCase { connecting.isBluetooth = true let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(vm.hwTransferError, .reconnect(isBluetooth: true), "a known BLE device gets the softer INFO reconnect toast") @@ -89,30 +89,13 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error, got \(String(describing: vm.hwTransferError))") } XCTAssertTrue(connecting.staleDisconnects.isEmpty) } - func testUnresolvableWalletIdFailsBeforeSigningOrBroadcasting() async { - let funding = MockHwFunding() - funding.walletIdError = MockHwFunding.TestError() - let connecting = MockHwConnecting() - let vm = makeViewModel(funding: funding, connecting: connecting) - - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") - await awaitSigningComplete(vm) - - // Recording the transfer against the wrong wallet is worse than not transferring at all, - // so the flow aborts before the device is asked to sign. - if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error, got \(String(describing: vm.hwTransferError))") } - XCTAssertEqual(funding.signCalls, 0) - XCTAssertEqual(funding.broadcastCalls, 0) - XCTAssertFalse(vm.hwFundingComplete) - } - func testBroadcastFailureRetainsSignedTransactionForRetry() async { let funding = MockHwFunding() funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") @@ -120,7 +103,7 @@ final class TransferViewModelHwTests: XCTestCase { let vm = makeViewModel(funding: funding, connecting: connecting) let order = IBtOrder.mock() - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) @@ -129,7 +112,7 @@ final class TransferViewModelHwTests: XCTestCase { XCTAssertEqual(funding.broadcastCalls, 1) XCTAssertEqual(vm.hwTransferError, .broadcastConnectivity) - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) @@ -150,11 +133,11 @@ final class TransferViewModelHwTests: XCTestCase { let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) var order = IBtOrder.mock() - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) order.payment?.onchain?.address = "bc1qnewdestination" - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(funding.signCalls, 2) @@ -167,7 +150,7 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertNil(vm.hwTransferError, "a device cancel must not surface a toast") @@ -186,7 +169,7 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertNil(vm.hwTransferError, "a wrapped device cancel must not surface a toast") @@ -200,7 +183,7 @@ final class TransferViewModelHwTests: XCTestCase { connecting.connectError = TrezorError.UserCancelled let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertNil(vm.hwTransferError, "a reconnect-step cancel must not surface a toast") @@ -213,9 +196,9 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.warmUpHardwareConnection(deviceId: "dev1") + vm.warmUpHardwareConnection(walletId: "trezor:wallet") - XCTAssertEqual(connecting.warmUpCalls, ["dev1"]) + XCTAssertEqual(connecting.warmUpCalls, ["trezor:wallet"]) } func testCancelHwSigningStopsInFlightSign() async { @@ -224,7 +207,7 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") while !vm.hwSpending.isSigning { await Task.yield() } @@ -236,7 +219,7 @@ final class TransferViewModelHwTests: XCTestCase { XCTAssertFalse(vm.hwSpending.isSigning) XCTAssertEqual(vm.hwSignedEvent, 0, "a cancelled sign must not advance the flow") - XCTAssertEqual(connecting.staleDisconnects, ["dev1"], "cancelling during sign must tear down the stale session") + XCTAssertEqual(connecting.staleDisconnects, ["trezor:wallet"], "cancelling during sign must tear down the stale session") } func testDeviceBusyMapsToDeviceBusyError() async { @@ -245,7 +228,7 @@ final class TransferViewModelHwTests: XCTestCase { connecting.connectError = TrezorError.DeviceBusy let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(vm.hwTransferError, .deviceBusy) @@ -260,7 +243,7 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(vm.hwTransferError, .firmwareReconnect) @@ -272,7 +255,7 @@ final class TransferViewModelHwTests: XCTestCase { let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) let order = IBtOrder.mock() - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) @@ -284,7 +267,7 @@ final class TransferViewModelHwTests: XCTestCase { funding.broadcastError = Bitkit.AppError(message: "rejected", debugMessage: "invalid tx") let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertFalse(vm.hwSpending.hasPendingBroadcast) @@ -298,7 +281,7 @@ final class TransferViewModelHwTests: XCTestCase { ) let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertFalse(vm.hwSpending.hasPendingBroadcast) @@ -312,11 +295,11 @@ final class TransferViewModelHwTests: XCTestCase { funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") let order = IBtOrder.mock() - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) - await vm.updateHwLimits(deviceId: "dev1", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) + await vm.updateHwLimits(walletId: "trezor:wallet", blocktankInfo: nil, estimateOrderFee: { _, _ in (0, 0) }) XCTAssertFalse(vm.hwSpending.hasPendingBroadcast) vm.cancelHwSigning() @@ -328,7 +311,7 @@ final class TransferViewModelHwTests: XCTestCase { let vm = makeViewModel(funding: funding, connecting: MockHwConnecting()) let order = IBtOrder.mock() - await vm.updateHwFundingFeeEstimate(order: order, deviceId: "dev1") + await vm.updateHwFundingFeeEstimate(order: order, walletId: "trezor:wallet") XCTAssertEqual(vm.hwSpending.miningFeeSats, funding.funding.miningFeeSats) XCTAssertEqual(funding.estimateCalls.count, 1) @@ -341,9 +324,9 @@ final class TransferViewModelHwTests: XCTestCase { let connecting = MockHwConnecting() let vm = makeViewModel(funding: funding, connecting: connecting) - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") // Second call while the first is still signing must be ignored. - vm.onTransferToSpendingHwConfirm(order: .mock(), deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") await awaitSigningComplete(vm) XCTAssertEqual(connecting.ensureCalls, 1, "only the first confirm should run") @@ -357,7 +340,7 @@ final class TransferViewModelHwTests: XCTestCase { var order = IBtOrder.mock() order.payment?.onchain?.address = "" - vm.onTransferToSpendingHwConfirm(order: order, deviceId: "dev1") + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") } XCTAssertFalse(vm.hwSpending.isSigning) From 2e0b0d0a37f72f4746bbef7a2be4abdc94db2c33 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 13:26:45 -0300 Subject: [PATCH 03/18] TrezorManager session identity + Bridge fix --- Bitkit/AppScene.swift | 3 +- Bitkit/Managers/HwDeviceSessioning.swift | 47 ++++ Bitkit/Managers/TrezorManager.swift | 216 ++++++++++++++---- .../Trezor/TrezorBridgeTransport.swift | 7 + .../TrezorKnownDeviceStorageTests.swift | 120 ++++++++++ 5 files changed, 347 insertions(+), 46 deletions(-) create mode 100644 Bitkit/Managers/HwDeviceSessioning.swift create mode 100644 BitkitTests/TrezorKnownDeviceStorageTests.swift diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift index a48b22375..60422a0cf 100644 --- a/Bitkit/AppScene.swift +++ b/Bitkit/AppScene.swift @@ -903,7 +903,8 @@ struct AppScene: View { private func pushHardwareDevices() { hwWalletManager.updateDevices( knownDevices: trezorManager.knownDevices, - connectedDeviceId: trezorManager.connectedDevice?.id + connectedDeviceId: trezorManager.connectedDevice?.id, + connectedWalletId: trezorManager.connectedWalletId ) } diff --git a/Bitkit/Managers/HwDeviceSessioning.swift b/Bitkit/Managers/HwDeviceSessioning.swift new file mode 100644 index 000000000..e8b96c57d --- /dev/null +++ b/Bitkit/Managers/HwDeviceSessioning.swift @@ -0,0 +1,47 @@ +import BitkitCore + +/// The live Trezor session and its stored entries, as the watch-only layer needs them. +/// +/// Implemented by `TrezorManager` and injected into `HwWalletManager`, so the watch-only layer can +/// reason about which wallet a session belongs to without either manager referencing the other. +/// Everything here is device-level on purpose: resolving a wallet identity to the transport it is +/// reachable over is the watch-only layer's job, since it owns the wallet grouping. +@MainActor +protocol HwDeviceSessioning: AnyObject, Sendable { + /// Stored entries read fresh. A connect that just wrote one lands here before the + /// `updateDevices(...)` push does, so session operations must not read the pushed snapshot. + var storedDevices: [TrezorKnownDevice] { get } + var connectedDeviceId: String? { get } + /// Identity the live session opened; nil when no session is open or none could be resolved. + var connectedWalletId: String? { get } + var connectedFeatures: TrezorFeatures? { get } + + func ensureConnected(deviceId: String) async throws + /// Opens `deviceId` with an explicit wallet selection, with or without a live session. + @discardableResult + func connectWithWalletMode( + deviceId: String, + mode: TrezorWalletMode, + passphrase: String + ) async throws -> TrezorFeatures + func disconnectStaleSession(deviceId: String) async + func isKnownBluetoothDevice(deviceId: String) -> Bool + func warmUpConnection(deviceId: String) + /// Forgets every stored entry of `walletId`, keeping transport credentials while another + /// identity of the same device remains paired. + func forgetWallet(walletId: String) async +} + +extension TrezorManager: HwDeviceSessioning { + var storedDevices: [TrezorKnownDevice] { + knownDevices + } + + var connectedDeviceId: String? { + connectedDevice?.id + } + + var connectedFeatures: TrezorFeatures? { + deviceFeatures + } +} diff --git a/Bitkit/Managers/TrezorManager.swift b/Bitkit/Managers/TrezorManager.swift index 0a3c3c187..66f61f748 100644 --- a/Bitkit/Managers/TrezorManager.swift +++ b/Bitkit/Managers/TrezorManager.swift @@ -34,6 +34,17 @@ final class TrezorManager { didSet { devicesRevision &+= 1 } } + /// Wallet identity the live session was opened for. A device can hold a standard wallet plus + /// several passphrase wallets, and only the one the session opened can sign. Nil while no + /// session is open, or before its accounts could be read. + private(set) var connectedWalletId: String? { + didSet { devicesRevision &+= 1 } + } + + /// Set while a session is deliberately being torn down and reopened for a chosen wallet, so a + /// background reconnect can't race in and open the standard wallet under a hidden selection. + private var isOpeningSession = false + /// Bumped whenever the device list or connection state changes, so observers (e.g. the /// composition root that feeds `HwWalletManager`) can react without those types coupling. private(set) var devicesRevision: Int = 0 @@ -166,6 +177,7 @@ final class TrezorManager { func clearDisconnectedDeviceState(errorMessage: String? = nil) { connectedDevice = nil + connectedWalletId = nil deviceFeatures = nil clearWalletDerivedState() error = errorMessage @@ -263,16 +275,19 @@ final class TrezorManager { // MARK: - Connection - func connect(device: TrezorDeviceInfo) async { + /// Opens a session on `device`. `mode` is the wallet selection to open it with; passing nil keeps + /// whatever selection is already recorded on the handler, which is how a passphrase wallet is + /// reopened. Every other caller passes `.standard` explicitly, because a passphrase selection + /// left over from a previously connected device must never silently apply to a newly picked one. + func connect(device: TrezorDeviceInfo, mode: TrezorWalletMode? = .standard) async { error = nil suppressNextAutoReconnect = false showPairingCode = false - // Explicit user-initiated connect always opens the standard wallet — a - // passphrase/on-device selection left over from a previously connected device - // must not silently apply to a newly selected one. - uiHandler.setWalletMode(.standard) - walletMode = .standard + if let mode { + uiHandler.setWalletMode(mode) + walletMode = mode + } trezorLog("=== Connecting to device: \(device.path) ===") @@ -288,6 +303,9 @@ final class TrezorManager { connectedDevice = device deviceFeatures = features showConfirmOnDevice = false + // Unresolved until this session's accounts are read: reporting the previous session's + // identity would mark the wrong wallet as the one that can sign. + connectedWalletId = nil let savedComplete = await saveCurrentDeviceAsKnown() if savedComplete { @@ -381,51 +399,81 @@ final class TrezorManager { await setWalletMode(.passphraseDevice) } - /// Switch between wallet modes. The Trezor caches the passphrase for the whole - /// session, so switching requires a fresh session: this records the desired mode, - /// then disconnects and reconnects by path. Mirrors bitkit-android's setWalletMode. + /// Switch the live session between wallet modes, surfacing failures on `error` for the dev + /// dashboard. Requires a connected device; `connectWithWalletMode` is the throwing variant that + /// also works from cold. func setWalletMode(_ mode: TrezorWalletMode, passphrase: String = "") async { guard let device = connectedDevice else { error = "Not connected to a Trezor" return } - error = nil - trezorLog("=== Switching wallet mode to \(mode); resetting session ===") - - // Reset the session. We call the service directly (not the manager's disconnect()) - // so connectedDevice/deviceFeatures stay populated for the reconnect. do { - try await trezorService.disconnect() + _ = try await connectWithWalletMode(deviceId: device.id, mode: mode, passphrase: passphrase) } catch { - trezorLog("Disconnect before wallet-mode switch failed: \(error)", level: "warn") + trezorLog("Reconnect after wallet-mode switch failed: \(error)", level: "error") } + } - // Results derived from the previous wallet are no longer valid once the - // session has been reset for a different wallet mode. - clearWalletDerivedState() + /// Opens `deviceId` with an explicit wallet selection, whether or not a session is live. + /// + /// The Trezor binds a passphrase when the session is created and caches it for the session's + /// lifetime, so an existing session is torn down first; with none — the app was restarted, or a + /// wrong passphrase closed it — the device is reconnected from its stored entry instead. + @discardableResult + func connectWithWalletMode( + deviceId: String, + mode: TrezorWalletMode, + passphrase: String = "" + ) async throws -> TrezorFeatures { + isOpeningSession = true + defer { isOpeningSession = false } + + error = nil + let hadSession = connectedDevice != nil + trezorLog("=== Opening \(mode) session for \(deviceId); hadSession=\(hadSession) ===") - // Brief settle delay before reconnecting (matches Android's reconnect delay). - try? await Task.sleep(nanoseconds: 300_000_000) + if hadSession { + // The service is called directly (not the manager's disconnect()) so + // connectedDevice/deviceFeatures stay populated for the reconnect. + do { + try await trezorService.disconnect() + } catch { + trezorLog("Disconnect before wallet-mode switch failed: \(error)", level: "warn") + } + // Results derived from the previous wallet no longer hold once the session is reset. + clearWalletDerivedState() + // Brief settle delay before reconnecting (matches Android's reconnect delay). + try? await Task.sleep(nanoseconds: 300_000_000) + } - // Record the selection AFTER the disconnect so it survives into the new session. - // THP reads it via currentSelection() to bind the passphrase at session creation; - // non-THP devices re-request it mid-operation and are answered from the same value. + // Record the selection last: disconnect resets it. THP reads it via currentSelection() to + // bind the passphrase at session creation; non-THP devices re-request it mid-operation and + // are answered from the same value. uiHandler.setWalletMode(mode, hostPassphrase: passphrase) walletMode = mode - do { - let features = try await trezorService.connect(deviceId: device.path, selection: uiHandler.currentSelection()) - connectedDevice = device - deviceFeatures = features - showConfirmOnDevice = false - trezorLog("Reconnected with wallet mode \(mode)") + if hadSession, let target = connectedDevice ?? knownDeviceInfo(deviceId) { + // Reconnect by path without a scan: a scan right after a disconnect usually finds + // nothing, whereas the cached handle still works. + await connect(device: target, mode: nil) + } else { + // Nothing cached to reconnect to, so take the known-device path with its scan and + // bluetooth fallback. + try await reconnectKnownDevice(deviceId: deviceId, mode: nil) + } - await saveCurrentDeviceAsKnown() - } catch { - clearDisconnectedDeviceState(errorMessage: errorMessage(from: error)) - trezorLog("Reconnect after wallet-mode switch failed: \(error)", level: "error") + guard connectedDevice?.id == deviceId, let features = deviceFeatures else { + let message = error ?? "Failed to open wallet on '\(deviceId)'" + clearDisconnectedDeviceState(errorMessage: message) + throw AppError(message: "Reconnect Hardware Device", debugMessage: message) } + trezorLog("Opened \(mode) session for \(deviceId)") + return features + } + + private func knownDeviceInfo(_ deviceId: String) -> TrezorDeviceInfo? { + knownDevices.first { $0.id == deviceId }.map { deviceInfo(from: $0) } } func submitPairingCode(_ code: String) { @@ -515,8 +563,12 @@ final class TrezorManager { @discardableResult func saveCurrentDeviceAsKnown() async -> Bool { guard let device = connectedDevice else { return false } - let previous = TrezorKnownDeviceStorage.loadAll().first { $0.id == device.id } + let stored = TrezorKnownDeviceStorage.loadAll() let (fetched, transientFailures) = await fetchAccountXpubs() + // Not matched by transport id alone: a passphrase wallet is a separate identity on the same + // device, so that would overwrite another identity or blend two identities' xpubs into one + // record. Shared key material is the identity. + let previous = TrezorKnownDeviceMatching.previous(in: stored, deviceId: device.id, fetchedXpubs: fetched) let mergedXpubs = (previous?.xpubs ?? [:]).merging(fetched) { _, new in new } guard !mergedXpubs.isEmpty else { @@ -533,6 +585,11 @@ final class TrezorManager { return false } + // The label belongs to the wallet, not to the transport it happens to be reached over, so a + // wallet showing up on a new path keeps the name the user gave it. + let identityKey = TrezorKnownDevice.walletKey(for: mergedXpubs, fallback: device.id) + let named = TrezorKnownDeviceMatching.named(in: stored, previous: previous, walletKey: identityKey) + let known = TrezorKnownDevice( id: device.id, name: device.name ?? "Trezor", @@ -542,14 +599,44 @@ final class TrezorManager { model: device.model ?? deviceFeatures?.model, lastConnectedAt: Date(), xpubs: mergedXpubs, - customLabel: previous?.customLabel + customLabel: named?.customLabel, + walletId: resolvedWalletId(previous: previous, identityKey: identityKey, xpubs: mergedXpubs, in: stored), + passphraseProtected: passphraseProtection(previous: previous), + trezorDeviceId: deviceFeatures?.deviceId ?? previous?.trezorDeviceId ) - TrezorKnownDeviceStorage.save(known) + TrezorKnownDeviceStorage.saveAll(TrezorKnownDeviceMatching.merged(stored, with: known, refreshed: previous)) loadKnownDevices() + connectedWalletId = known.resolvedWalletId trezorLog("Saved known device: \(known.name) with \(mergedXpubs.count) xpubs") return true } + private func resolvedWalletId( + previous: TrezorKnownDevice?, + identityKey: String, + xpubs: [String: String], + in stored: [TrezorKnownDevice] + ) -> String? { + if let carried = previous?.walletId ?? stored.first(where: { $0.walletKey == identityKey })?.walletId, + !carried.isEmpty + { + return carried + } + return try? HwWalletId.derive(xpubs: xpubs) + } + + /// The selection that derived these keys is authoritative, so a wallet wrongly marked hidden is + /// corrected the next time it is opened rather than staying gated behind a passphrase forever. + /// On-device entry cannot say which wallet was opened, so it keeps what the entry already knew + /// and assumes hidden only for one it has never seen. + private func passphraseProtection(previous: TrezorKnownDevice?) -> Bool { + switch uiHandler.currentSelection() { + case .standard: false + case .hidden: true + case .onDevice: previous?.passphraseProtected ?? true + } + } + private static let maxXpubFetchAttempts = 3 private static let xpubFetchRetryDelayNanos: UInt64 = 300_000_000 private static let deviceLabelMaxLength = 50 @@ -610,17 +697,13 @@ final class TrezorManager { return (result, transientFailures) } + /// Forget every wallet a device holds. Used by the dev device list, where the unit is the device. func forgetDevice(id: String) async { let known = knownDevices.first(where: { $0.id == id }) let isActiveSession = connectedDevice?.id == id || (known.map { connectedDevice?.path == $0.path } ?? false) if let device = known { - do { - try await trezorService.clearCredentials(deviceId: device.path) - } catch { - trezorLog("Failed to clear credentials for forgotten device: \(error)", level: "warn") - } - TrezorCredentialStorage.delete(deviceId: device.path) + await clearCredentials(path: device.path) } TrezorKnownDeviceStorage.remove(id: id) loadKnownDevices() @@ -631,11 +714,52 @@ final class TrezorManager { } } + /// Forget one wallet identity, leaving the device's other wallets paired. Transport and session + /// credentials are keyed by path and shared by every identity of a device, so they are only + /// cleared once none remains — dropping them while a sibling is still paired would leave that + /// wallet unable to reconnect. + func forgetWallet(walletId: String) async { + let stored = TrezorKnownDeviceStorage.loadAll() + let forgotten = stored.filter { $0.resolvedWalletId == walletId } + guard !forgotten.isEmpty else { + trezorLog("Nothing to forget for hardware wallet '\(walletId)'", level: "warn") + return + } + let remaining = stored.filter { $0.resolvedWalletId != walletId } + + for entry in forgotten where !remaining.contains(where: { $0.id == entry.id }) { + await clearCredentials(path: entry.path) + } + + TrezorKnownDeviceStorage.saveAll(remaining) + loadKnownDevices() + trezorLog("Forgot hardware wallet: \(walletId)") + + // Only the session of what is being forgotten may be torn down: the device can hold another + // identity open, and that wallet is still paired and still signing. + let ownsSession = forgotten.contains { $0.id == connectedDevice?.id || $0.path == connectedDevice?.path } + if ownsSession, connectedWalletId == nil || connectedWalletId == walletId { + await disconnect() + } + } + + private func clearCredentials(path: String) async { + do { + try await trezorService.clearCredentials(deviceId: path) + } catch { + trezorLog("Failed to clear credentials for forgotten device: \(error)", level: "warn") + } + TrezorCredentialStorage.delete(deviceId: path) + } + // MARK: - Auto-Reconnect func autoReconnect() async { guard !knownDevices.isEmpty else { return } guard !isAutoReconnecting else { return } + // A deliberate wallet-mode open is mid-flight; reconnecting now would race it and open the + // standard wallet under a hidden selection. + guard !isOpeningSession else { return } guard connectedDevice == nil else { trezorLog("Auto-reconnect: skipped, device already connected") return @@ -706,7 +830,7 @@ final class TrezorManager { } } - private func reconnectKnownDevice(deviceId: String) async throws { + private func reconnectKnownDevice(deviceId: String, mode: TrezorWalletMode? = .standard) async throws { await startScan(clearExisting: true) let target: TrezorDeviceInfo @@ -720,7 +844,7 @@ final class TrezorManager { throw AppError(message: "Reconnect Hardware Device", debugMessage: "Device '\(deviceId)' not found nearby") } - await connect(device: target) + await connect(device: target, mode: mode) guard connectedDevice?.id == deviceId, await trezorService.isConnected() else { throw AppError(message: "Reconnect Hardware Device", debugMessage: error ?? "Failed to reconnect '\(deviceId)'") @@ -739,6 +863,8 @@ final class TrezorManager { func warmUpConnection(deviceId: String) { guard connectedDevice?.id != deviceId else { return } guard !isScanning else { return } + // Would race a deliberate wallet-mode open and land on the standard wallet. + guard !isOpeningSession else { return } guard isKnownBluetoothDevice(deviceId: deviceId) else { return } Task { do { diff --git a/Bitkit/Services/Trezor/TrezorBridgeTransport.swift b/Bitkit/Services/Trezor/TrezorBridgeTransport.swift index 93b1c3223..bdbf093ea 100644 --- a/Bitkit/Services/Trezor/TrezorBridgeTransport.swift +++ b/Bitkit/Services/Trezor/TrezorBridgeTransport.swift @@ -93,6 +93,13 @@ final class TrezorBridgeTransport { func closeDevice(path: String) -> TrezorTransportWriteResult { sessionLock.lock() let session = openSessions.removeValue(forKey: path) + // The enumerated session is the one being released here, so leaving it cached would offer a + // released id as the previous session on the next acquire and the bridge answers "wrong + // previous session". Switching to a passphrase wallet closes and reopens the session, so + // that would block every passphrase pairing after the first. + if let session, enumeratedSessions[path] == session { + enumeratedSessions.removeValue(forKey: path) + } sessionLock.unlock() guard let session else { diff --git a/BitkitTests/TrezorKnownDeviceStorageTests.swift b/BitkitTests/TrezorKnownDeviceStorageTests.swift new file mode 100644 index 000000000..70565e799 --- /dev/null +++ b/BitkitTests/TrezorKnownDeviceStorageTests.swift @@ -0,0 +1,120 @@ +@testable import Bitkit +import XCTest + +/// Covers identity-scoped reads and writes: one physical device can hold a standard wallet plus its +/// passphrase wallets, so `id` no longer identifies a stored entry on its own. +final class TrezorKnownDeviceStorageTests: XCTestCase { + private static let storageKey = "trezor.knownDevices" + private var savedDefaults: Data? + + override func setUp() { + super.setUp() + savedDefaults = UserDefaults.standard.data(forKey: Self.storageKey) + TrezorKnownDeviceStorage.removeAll() + } + + override func tearDown() { + if let savedDefaults { + UserDefaults.standard.set(savedDefaults, forKey: Self.storageKey) + } else { + TrezorKnownDeviceStorage.removeAll() + } + super.tearDown() + } + + func testSavingAPassphraseWalletKeepsTheStandardWalletOfTheSameDevice() { + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:standard")) + TrezorKnownDeviceStorage.save(makeDevice( + xpubs: ["nativeSegwit": "zHidden"], + walletId: "trezor:hidden", + passphraseProtected: true + )) + + let stored = TrezorKnownDeviceStorage.loadAll() + XCTAssertEqual(Set(stored.compactMap(\.walletId)), ["trezor:standard", "trezor:hidden"]) + XCTAssertEqual(stored.filter(\.passphraseProtected).count, 1) + } + + func testSavingTheSameIdentityAgainReplacesIt() { + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Old")) + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "New")) + + XCTAssertEqual(TrezorKnownDeviceStorage.loadAll().map(\.customLabel), ["New"]) + } + + func testRemovingOneWalletLeavesTheDevicesOtherWalletsPaired() { + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:standard")) + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden")) + + TrezorKnownDeviceStorage.remove(walletId: "trezor:hidden") + + XCTAssertEqual(TrezorKnownDeviceStorage.loadAll().compactMap(\.walletId), ["trezor:standard"]) + XCTAssertTrue(TrezorKnownDeviceStorage.isKnown(id: "dev1"), "the device itself stays paired") + } + + /// Entries written before the wallet id was persisted resolve it from their xpubs. + func testRemovingAWalletMatchesEntriesWithoutAStoredWalletId() throws { + let legacy = makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: nil) + TrezorKnownDeviceStorage.save(legacy) + + try TrezorKnownDeviceStorage.remove(walletId: HwWalletId.derive(xpubs: legacy.xpubs)) + + XCTAssertTrue(TrezorKnownDeviceStorage.loadAll().isEmpty) + } + + func testRemovingByDeviceIdForgetsEveryWalletItHolds() { + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:standard")) + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden")) + TrezorKnownDeviceStorage.save(makeDevice(id: "dev2", xpubs: ["nativeSegwit": "zOther"], walletId: "trezor:other")) + + TrezorKnownDeviceStorage.remove(id: "dev1") + + XCTAssertEqual(TrezorKnownDeviceStorage.loadAll().compactMap(\.walletId), ["trezor:other"]) + } + + func testLoadingByWalletIdReturnsOnlyThatIdentitysEntries() { + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zStandard"], walletId: "trezor:standard")) + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden")) + + let entries = TrezorKnownDeviceStorage.loadAll(walletId: "trezor:hidden") + + XCTAssertEqual(entries.count, 1) + XCTAssertEqual(entries.first?.xpubs, ["nativeSegwit": "zHidden"]) + } + + func testNewFieldsSurviveAStorageRoundTrip() { + TrezorKnownDeviceStorage.save(makeDevice( + xpubs: ["nativeSegwit": "zHidden"], + walletId: "trezor:hidden", + passphraseProtected: true, + trezorDeviceId: "trezor-id" + )) + + let stored = TrezorKnownDeviceStorage.loadAll().first + XCTAssertEqual(stored?.walletId, "trezor:hidden") + XCTAssertTrue(stored?.passphraseProtected == true) + XCTAssertEqual(stored?.trezorDeviceId, "trezor-id") + } + + private func makeDevice( + id: String = "dev1", + xpubs: [String: String], + customLabel: String? = nil, + walletId: String? = nil, + passphraseProtected: Bool = false, + trezorDeviceId: String? = nil + ) -> TrezorKnownDevice { + TrezorKnownDevice( + id: id, + name: "Trezor", + path: "ble://\(id)", + transportType: "bluetooth", + lastConnectedAt: Date(timeIntervalSince1970: 0), + xpubs: xpubs, + customLabel: customLabel, + walletId: walletId, + passphraseProtected: passphraseProtected, + trezorDeviceId: trezorDeviceId + ) + } +} From 41689b7e13e463957f068e5d1c8be19c464ea16d Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 13:47:18 -0300 Subject: [PATCH 04/18] feat: identity ops on HwWalletManager --- Bitkit/AppScene.swift | 4 +- Bitkit/Managers/HwWalletManager.swift | 177 +++++++- Bitkit/Managers/TrezorManager.swift | 41 -- Bitkit/ViewModels/HwFundingSigner.swift | 4 + Bitkit/ViewModels/TransferViewModel.swift | 5 + .../HardwareWalletsSettingsScreen.swift | 4 +- .../Views/Wallets/HardwareWalletScreen.swift | 11 +- BitkitTests/HwTransferMocks.swift | 14 + .../HwWalletManagerPassphraseTests.swift | 380 ++++++++++++++++++ BitkitTests/HwWalletManagerTests.swift | 9 +- 10 files changed, 580 insertions(+), 69 deletions(-) create mode 100644 BitkitTests/HwWalletManagerPassphraseTests.swift diff --git a/Bitkit/AppScene.swift b/Bitkit/AppScene.swift index 60422a0cf..92dd2a859 100644 --- a/Bitkit/AppScene.swift +++ b/Bitkit/AppScene.swift @@ -84,13 +84,13 @@ struct AppScene: View { // Created ahead of `transfer` so the hardware-wallet transfer flow can reach the funding // (compose/sign/broadcast) and device-session (reconnect) capabilities. let trezorManager = TrezorManager() - let hwWalletManager = HwWalletManager() + let hwWalletManager = HwWalletManager(session: trezorManager) _transfer = StateObject(wrappedValue: TransferViewModel( transferService: transferService, sheetViewModel: sheetViewModel, hwFunding: hwWalletManager, - hwConnecting: trezorManager, + hwConnecting: hwWalletManager, hwFeeRateProvider: { guard let rates = await feeEstimatesManager.getEstimates() else { return nil } return UInt64(TransactionSpeed.fast.getFeeRate(from: rates)) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 68cd2da6d..ca7ff05f6 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -10,10 +10,15 @@ import Foundation /// Keyed by wallet identity, not by device: a Trezor with passphrase protection holds its standard /// wallet plus one identity per hidden wallet, all reached over the same transport id. /// -/// Fully decoupled from `TrezorManager`: it receives the paired-wallet snapshot through -/// `updateDevices(...)`, fed by the composition root (`AppScene`). Adapts bitkit-android's -/// `HwWalletRepo`. iOS supports Bluetooth only, so the cross-transport (BLE+USB) dedup is reduced -/// to a plain xpub-based identity and USB-specific reconnect handling is omitted. +/// Tile and watcher state come solely from `updateDevices(...)`, fed by the composition root +/// (`AppScene`). The identity-aware session operations — opening a passphrase wallet, proving the +/// live session belongs to the wallet being spent from — additionally read the device through the +/// injected `HwDeviceSessioning` seam, and read the stored entries fresh from it: a connect that +/// just wrote one lands there before the push does. Never references `TrezorManager` concretely. +/// +/// Adapts bitkit-android's `HwWalletRepo`. iOS supports Bluetooth only, so the cross-transport +/// (BLE+USB) dedup is reduced to a plain xpub-based identity and USB-specific reconnect handling +/// is omitted. @Observable @MainActor final class HwWalletManager { @@ -49,6 +54,11 @@ final class HwWalletManager { private let persistSnapshot: @MainActor (HwWalletSnapshot) async throws -> Void private let deleteActivities: @MainActor (String) async throws -> Void + /// The live device session. Only the identity-aware operations need it; tile and watcher state + /// still come solely from `updateDevices(...)`. Nil in previews and in tests that don't reach + /// the device. + private weak var session: HwDeviceSessioning? + /// One chain per wallet id, shared by both writes: a snapshot landing after the delete it was /// racing would resurrect the wallet `removeDevice` just wiped. private let persistQueue = SnapshotPersistQueue() @@ -87,6 +97,7 @@ final class HwWalletManager { private var listeners: [String: TrezorEventListener] = [:] init( + session: HwDeviceSessioning? = nil, watcherService: OnChainWatcherServicing = OnChainHwService.shared, monitoredTypes: (() -> Set)? = nil, electrumUrl: (() -> String)? = nil, @@ -94,6 +105,7 @@ final class HwWalletManager { persistSnapshot: (@MainActor (HwWalletSnapshot) async throws -> Void)? = nil, deleteActivities: (@MainActor (String) async throws -> Void)? = nil ) { + self.session = session self.watcherService = watcherService networkProvider = network ?? { OnChainHwService.appDefaultCoinType } monitoredTypesProvider = monitoredTypes ?? { @@ -184,14 +196,139 @@ final class HwWalletManager { } /// Removes a hardware wallet and forgets every stored entry that belongs to its wallet identity. - func removeWallet( - _ wallet: HwWallet, - forgetDevice: (String) async -> Void - ) async { - removeDevice(walletId: wallet.id) - for deviceId in wallet.deviceIds { - await forgetDevice(deviceId) + /// Other wallets of the same physical device stay paired. + func removeWallet(walletId: String) async { + removeDevice(walletId: walletId) + await session?.forgetWallet(walletId: walletId) + } + + // MARK: - Wallet identity & the device session + + /// Stored entries tracking one wallet identity, read fresh: a connect that just wrote one lands + /// there before the `updateDevices(...)` push does. + private func entries(for walletId: String) -> [TrezorKnownDevice] { + (session?.storedDevices ?? knownDevices).filter { $0.resolvedWalletId == walletId } + } + + /// Transport id to reach `walletId` with: the connected entry, else the most recently used one. + private func transportDeviceId(for walletId: String) -> String? { + let entries = entries(for: walletId) + if let connected = entries.first(where: { $0.id == session?.connectedDeviceId }) { return connected.id } + return entries.max(by: { $0.lastConnectedAt < $1.lastConnectedAt })?.id + } + + private func requireTransportDeviceId(for walletId: String) throws -> String { + guard let deviceId = transportDeviceId(for: walletId) else { + throw AppError(message: "Unknown hardware wallet", debugMessage: "No paired device for wallet '\(walletId)'") + } + return deviceId + } + + /// A session opened before its identity could be resolved reports none and stays usable. + private func isIdentity(_ sessionWalletId: String?, of walletId: String) -> Bool { + sessionWalletId == nil || sessionWalletId == walletId + } + + private func watchedWalletIds() -> Set { + Set((session?.storedDevices ?? knownDevices).compactMap(\.resolvedWalletId)) + } + + /// Opens the passphrase (hidden) wallet of an already paired device and starts watching it as + /// its own identity, returning its wallet id. The passphrase is bound to a fresh Trezor session + /// and is never persisted; re-entering it is what makes the wallet reachable again. + func connectWithPassphrase(deviceId: String, passphrase: String) async throws -> String { + guard let session else { + throw AppError(message: "Unavailable", debugMessage: "No device session to open a passphrase wallet with") + } + // A device with passphrase protection turned off ignores the passphrase and simply reopens + // the standard wallet, which would surface as "already added" and leave the user retyping a + // passphrase that can never take effect. + guard session.connectedFeatures?.passphraseProtection == true else { + throw HwPassphraseError.protectionDisabled + } + + let watchedBefore = watchedWalletIds() + try await session.connectWithWalletMode(deviceId: deviceId, mode: .passphraseHost, passphrase: passphrase) + guard let opened = session.connectedWalletId else { + throw AppError( + message: "Couldn't read the passphrase wallet", + debugMessage: "No accounts resolved for the passphrase wallet on '\(deviceId)'" + ) + } + guard !watchedBefore.contains(opened) else { throw HwPassphraseError.alreadyAdded } + return opened + } + + /// Makes the device session belong to `walletId`, not merely to its transport. A device holds one + /// identity open at a time, so a session opened for another wallet on the same device would + /// otherwise be accepted and sign with the wrong seed. The standard wallet needs no secret to + /// reopen; a passphrase wallet does, which the caller has to collect. + func ensureConnected(walletId: String) async throws { + guard let session else { + throw AppError(message: "Unavailable", debugMessage: "No device session for wallet '\(walletId)'") + } + let deviceId = try requireTransportDeviceId(for: walletId) + try await session.ensureConnected(deviceId: deviceId) + if isIdentity(session.connectedWalletId, of: walletId) { return } + + Logger.info("Reopening '\(walletId)': session belongs to another identity", context: "HwWalletManager") + guard !entries(for: walletId).contains(where: \.passphraseProtected) else { + throw HwPassphraseError.required + } + try await session.connectWithWalletMode(deviceId: deviceId, mode: .standard, passphrase: "") + guard isIdentity(session.connectedWalletId, of: walletId) else { throw HwPassphraseError.required } + } + + /// Whether reaching `walletId` needs the passphrase again. The device only holds one hidden + /// wallet open at a time and forgets the passphrase with the session, so a passphrase wallet that + /// is not the live session cannot be reconnected — or signed with — without it. + func needsPassphrase(walletId: String) -> Bool { + entries(for: walletId).contains(where: \.passphraseProtected) && session?.connectedWalletId != walletId + } + + func disconnectStaleSession(walletId: String) async { + guard let deviceId = transportDeviceId(for: walletId) else { return } + await session?.disconnectStaleSession(deviceId: deviceId) + } + + func isKnownBluetoothDevice(walletId: String) -> Bool { + guard let deviceId = transportDeviceId(for: walletId) else { return false } + return session?.isKnownBluetoothDevice(deviceId: deviceId) ?? false + } + + func warmUpConnection(walletId: String) { + guard let deviceId = transportDeviceId(for: walletId) else { return } + session?.warmUpConnection(deviceId: deviceId) + } + + /// Reopens a watched passphrase wallet for signing. A wrong passphrase is not rejected by the + /// device — it silently derives a different wallet — so the reopened session is only accepted when + /// its accounts resolve back to `walletId`; anything else is torn down again and reported as + /// `HwPassphraseError.mismatch` rather than signing from the wrong wallet. + func reconnectWithPassphrase(walletId: String, passphrase: String) async throws { + guard let session else { + throw AppError(message: "Unavailable", debugMessage: "No device session for wallet '\(walletId)'") } + let deviceId = try requireTransportDeviceId(for: walletId) + let watchedBefore = watchedWalletIds() + // Not `ensureConnected`: the session this reopens is usually already gone, either because the + // app restarted or because a wrong passphrase closed it. + try await session.connectWithWalletMode(deviceId: deviceId, mode: .passphraseHost, passphrase: passphrase) + + let opened = session.connectedWalletId + if opened == walletId { return } + + Logger.warn( + "Rejected hardware session for '\(walletId)': opened wallet '\(opened ?? "unknown")'", + context: "HwWalletManager" + ) + // Reading the accounts of the wrong wallet already stored it; a mistyped passphrase must not + // leave a stray watch-only wallet behind. + if let opened, !watchedBefore.contains(opened) { + await removeWallet(walletId: opened) + } + await session.disconnectStaleSession(deviceId: deviceId) + throw HwPassphraseError.mismatch } // MARK: - Watcher orchestration @@ -737,9 +874,12 @@ final class HwWalletManager { /// `TrezorManager.disconnectStaleSession`). Broadcasting is a separate step so a device-signing /// timeout is never conflated with an in-flight broadcast. func signFunding( - walletId _: String, + walletId: String, funding: HwFundingTransaction ) async throws -> HwFundingSignedTx { + // The session can change between connecting and signing, and signing from the wrong seed + // would produce signatures that do not match the inputs being spent. + guard isIdentity(session?.connectedWalletId, of: walletId) else { throw HwPassphraseError.required } let network = networkProvider() let signed = try await TrezorService.shared.signTxFromPsbt(psbtBase64: funding.psbt, network: network) return HwFundingSignedTx( @@ -828,3 +968,16 @@ final class SnapshotPersistQueue { } } } + +/// Failures specific to passphrase (hidden) wallets, mirroring bitkit-android's `HwPassphrase*Error` +/// types. The passphrase itself never appears in any of them. +enum HwPassphraseError: Error, Equatable { + /// The device has passphrase protection turned off, so it cannot open a hidden wallet at all. + case protectionDisabled + /// The entered passphrase resolves to a wallet Bitkit already watches. + case alreadyAdded + /// The session belongs to another identity, and only this wallet's passphrase reopens it. + case required + /// The entered passphrase opened a different wallet than the one being signed from. + case mismatch +} diff --git a/Bitkit/Managers/TrezorManager.swift b/Bitkit/Managers/TrezorManager.swift index 66f61f748..6de4f2b56 100644 --- a/Bitkit/Managers/TrezorManager.swift +++ b/Bitkit/Managers/TrezorManager.swift @@ -936,44 +936,3 @@ final class TrezorManager { TrezorErrorPresenter.userMessage(from: error) } } - -// MARK: - Wallet-addressed session access - -/// The transfer flow addresses a hardware wallet by its identity, not by the transport it happens to -/// be reachable over: one device can hold a standard wallet plus its passphrase wallets, all sharing -/// a transport id. Resolving the identity to a transport id happens here so the session APIs stay -/// device-level. -extension TrezorManager: HwTransferConnecting { - func ensureConnected(walletId: String) async throws { - try await ensureConnected(deviceId: requireTransportDeviceId(forWallet: walletId)) - } - - func disconnectStaleSession(walletId: String) async { - guard let deviceId = transportDeviceId(forWallet: walletId) else { return } - await disconnectStaleSession(deviceId: deviceId) - } - - func isKnownBluetoothDevice(walletId: String) -> Bool { - guard let deviceId = transportDeviceId(forWallet: walletId) else { return false } - return isKnownBluetoothDevice(deviceId: deviceId) - } - - func warmUpConnection(walletId: String) { - guard let deviceId = transportDeviceId(forWallet: walletId) else { return } - warmUpConnection(deviceId: deviceId) - } - - /// Transport id to reach `walletId` with: the connected entry, else the most recently used one. - private func transportDeviceId(forWallet walletId: String) -> String? { - let entries = knownDevices.filter { $0.resolvedWalletId == walletId } - if let connected = entries.first(where: { $0.id == connectedDevice?.id }) { return connected.id } - return entries.max(by: { $0.lastConnectedAt < $1.lastConnectedAt })?.id - } - - private func requireTransportDeviceId(forWallet walletId: String) throws -> String { - guard let deviceId = transportDeviceId(forWallet: walletId) else { - throw AppError(message: "Unknown hardware wallet", debugMessage: "No paired device for wallet '\(walletId)'") - } - return deviceId - } -} diff --git a/Bitkit/ViewModels/HwFundingSigner.swift b/Bitkit/ViewModels/HwFundingSigner.swift index 6fef0d4f1..27a1a2938 100644 --- a/Bitkit/ViewModels/HwFundingSigner.swift +++ b/Bitkit/ViewModels/HwFundingSigner.swift @@ -119,6 +119,10 @@ struct HwFundingSigner { throw CancellationError() } catch { if error.isTrezorUserCancellation() { throw error } + // Swift has no cause chain, so this must be rethrown explicitly: the catch-all below + // would otherwise bury "this wallet needs its passphrase" under a reconnect failure and + // the prompt would never open. + if let passphrase = error as? HwPassphraseError { throw passphrase } if error.isTrezorDeviceBusy() { throw HwTransferError.deviceBusy } throw HwTransferError.reconnect(isBluetooth: connecting.isKnownBluetoothDevice(walletId: walletId)) } diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index 3babad03a..7b15e3e5e 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -102,6 +102,10 @@ protocol HwTransferConnecting: Sendable { /// Best-effort pre-connect when the sign screen appears, so tapping Open Trezor Connect is less /// likely to hit a cold reconnect. Fire-and-forget. func warmUpConnection(walletId: String) + /// Whether the wallet's passphrase has to be collected again before the device can sign for it. + func needsPassphrase(walletId: String) -> Bool + /// Reopens a hidden wallet for signing, refusing a session that resolves to a different wallet. + func reconnectWithPassphrase(walletId: String, passphrase: String) async throws } @MainActor @@ -1507,3 +1511,4 @@ actor ChannelPendingCapture { // MARK: - Hardware transfer capability conformances extension HwWalletManager: HwTransferFunding {} +extension HwWalletManager: HwTransferConnecting {} diff --git a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift index e6b6a8b3c..b9266cf21 100644 --- a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift +++ b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift @@ -102,9 +102,7 @@ struct HardwareWalletsSettingsScreen: View { private func remove(_ wallet: HwWallet) async { pendingRemoval = nil - await hwWalletManager.removeWallet(wallet) { deviceId in - await trezorManager.forgetDevice(id: deviceId) - } + await hwWalletManager.removeWallet(walletId: wallet.id) } } diff --git a/Bitkit/Views/Wallets/HardwareWalletScreen.swift b/Bitkit/Views/Wallets/HardwareWalletScreen.swift index 2172bd607..12279add9 100644 --- a/Bitkit/Views/Wallets/HardwareWalletScreen.swift +++ b/Bitkit/Views/Wallets/HardwareWalletScreen.swift @@ -175,15 +175,12 @@ struct HardwareWalletScreen: View { } } - /// Stop watching and forget every entry for this wallet (the same device may be paired over - /// multiple transports). `removeDevice` stops the watchers and deletes the persisted activities; - /// `forgetDevice` clears credentials and drops the known-device entry, which re-pushes the device - /// snapshot and removes the tile. The reactive auto-pop above then leaves the screen. + /// Stop watching this wallet and forget its stored entries, leaving the device paired for any + /// other wallet it holds. Dropping the entries re-pushes the device snapshot and removes the + /// tile, and the reactive auto-pop above then leaves the screen. private func removeWallet() async { guard let wallet else { return } - await hwWalletManager.removeWallet(wallet) { deviceId in - await trezorManager.forgetDevice(id: deviceId) - } + await hwWalletManager.removeWallet(walletId: wallet.id) } } diff --git a/BitkitTests/HwTransferMocks.swift b/BitkitTests/HwTransferMocks.swift index 4dd25a18f..f06160013 100644 --- a/BitkitTests/HwTransferMocks.swift +++ b/BitkitTests/HwTransferMocks.swift @@ -87,15 +87,29 @@ final class MockHwFunding: HwTransferFunding { final class MockHwConnecting: HwTransferConnecting { var connectError: Error? var isBluetooth = false + /// Wallets whose passphrase the device no longer holds, so signing has to ask for it again. + var walletsNeedingPassphrase: Set = [] + var reconnectError: Error? private(set) var ensureCalls = 0 private(set) var staleDisconnects: [String] = [] private(set) var warmUpCalls: [String] = [] + private(set) var reconnectCalls: [(walletId: String, passphrase: String)] = [] func ensureConnected(walletId _: String) async throws { ensureCalls += 1 if let connectError { throw connectError } } + func needsPassphrase(walletId: String) -> Bool { + walletsNeedingPassphrase.contains(walletId) + } + + func reconnectWithPassphrase(walletId: String, passphrase: String) async throws { + reconnectCalls.append((walletId, passphrase)) + if let reconnectError { throw reconnectError } + walletsNeedingPassphrase.remove(walletId) + } + func isKnownBluetoothDevice(walletId _: String) -> Bool { isBluetooth } diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift new file mode 100644 index 000000000..251d704db --- /dev/null +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -0,0 +1,380 @@ +@testable import Bitkit +import BitkitCore +import XCTest + +/// Covers the identity-aware session operations on `HwWalletManager`, adapting the passphrase cases +/// in bitkit-android's `HwWalletRepoTest`. The device is a fake session, so the rules that stand +/// between a mistyped passphrase and a signature from the wrong seed are exercised without hardware. +@MainActor +final class HwWalletManagerPassphraseTests: XCTestCase { + // MARK: - Fake session + + private final class MockHwDeviceSession: HwDeviceSessioning { + var storedDevices: [TrezorKnownDevice] = [] + var connectedDeviceId: String? + var connectedWalletId: String? + var connectedFeatures: TrezorFeatures? + + /// Wallet id the next `connectWithWalletMode` resolves to, per mode. A hidden open that is + /// not listed here resolves to `openedWalletIdOnHidden`, standing in for the different wallet + /// a wrong passphrase silently derives. + var openedWalletIdOnHidden: String? + var openedWalletIdOnStandard: String? + /// An entry the device writes when a hidden open reads a wallet Bitkit has never seen, + /// mirroring how reading accounts persists the wallet before anything can reject it. + var writesEntryOnHiddenOpen: TrezorKnownDevice? + + var ensureConnectedError: Error? + var connectWithWalletModeError: Error? + + private(set) var ensureCalls: [String] = [] + private(set) var openCalls: [(deviceId: String, mode: TrezorWalletMode, passphrase: String)] = [] + private(set) var staleDisconnects: [String] = [] + private(set) var forgottenWalletIds: [String] = [] + private(set) var warmUpCalls: [String] = [] + + func ensureConnected(deviceId: String) async throws { + ensureCalls.append(deviceId) + if let ensureConnectedError { throw ensureConnectedError } + } + + @discardableResult + func connectWithWalletMode( + deviceId: String, + mode: TrezorWalletMode, + passphrase: String + ) async throws -> TrezorFeatures { + openCalls.append((deviceId, mode, passphrase)) + if let connectWithWalletModeError { throw connectWithWalletModeError } + connectedDeviceId = deviceId + switch mode { + case .standard: + connectedWalletId = openedWalletIdOnStandard + case .passphraseHost, .passphraseDevice: + connectedWalletId = openedWalletIdOnHidden + if let entry = writesEntryOnHiddenOpen { + storedDevices.append(entry) + writesEntryOnHiddenOpen = nil + } + } + return connectedFeatures ?? makeFeatures() + } + + func disconnectStaleSession(deviceId: String) async { + staleDisconnects.append(deviceId) + } + + func isKnownBluetoothDevice(deviceId _: String) -> Bool { + true + } + + func warmUpConnection(deviceId: String) { + warmUpCalls.append(deviceId) + } + + func forgetWallet(walletId: String) async { + forgottenWalletIds.append(walletId) + storedDevices.removeAll { $0.resolvedWalletId == walletId } + if connectedWalletId == walletId { connectedWalletId = nil } + } + } + + private final class NoopWatcher: OnChainWatcherServicing, @unchecked Sendable { + func startWatcher(params _: WatcherParams, listener _: EventListener) async throws {} + func stopWatcher(watcherId _: String) throws {} + func stopAllWatchers() {} + } + + private var session = MockHwDeviceSession() + private var deletedWalletIds: [String] = [] + + override func setUp() { + super.setUp() + session = MockHwDeviceSession() + deletedWalletIds = [] + } + + // MARK: - connectWithPassphrase + + func testOpensTheHiddenWalletAndReturnsItsIdentity() async throws { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedFeatures = makeFeatures(passphraseProtection: true) + session.openedWalletIdOnHidden = hiddenWalletId + let manager = makeManager() + + let opened = try await manager.connectWithPassphrase(deviceId: "dev1", passphrase: "correct horse") + + XCTAssertEqual(opened, hiddenWalletId) + XCTAssertEqual(session.openCalls.map(\.mode), [.passphraseHost]) + XCTAssertEqual(session.openCalls.first?.passphrase, "correct horse") + } + + /// A device with passphrase protection off ignores the passphrase and reopens the standard + /// wallet, which would surface as "already added" and leave the user retyping something that + /// can never take effect. + func testRefusesADeviceThatCannotOpenHiddenWallets() async { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedFeatures = makeFeatures(passphraseProtection: false) + let manager = makeManager() + + await assertThrows(HwPassphraseError.protectionDisabled) { + _ = try await manager.connectWithPassphrase(deviceId: "dev1", passphrase: "anything") + } + XCTAssertTrue(session.openCalls.isEmpty, "the device is never asked to open a hidden wallet") + } + + func testReportsAPassphraseWalletThatIsAlreadyWatched() async { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedFeatures = makeFeatures(passphraseProtection: true) + session.openedWalletIdOnHidden = hiddenWalletId + let manager = makeManager() + + await assertThrows(HwPassphraseError.alreadyAdded) { + _ = try await manager.connectWithPassphrase(deviceId: "dev1", passphrase: "already used") + } + } + + // MARK: - ensureConnected + + func testAcceptsASessionAlreadyOpenOnTheRequestedWallet() async throws { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedWalletId = standardWalletId + let manager = makeManager() + + try await manager.ensureConnected(walletId: standardWalletId) + + XCTAssertEqual(session.ensureCalls, ["dev1"]) + XCTAssertTrue(session.openCalls.isEmpty, "no reopen is needed") + } + + /// A session opened before its identity could be resolved reports none and stays usable. + func testAcceptsASessionWhoseIdentityIsNotYetResolved() async throws { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedWalletId = nil + let manager = makeManager() + + try await manager.ensureConnected(walletId: standardWalletId) + + XCTAssertTrue(session.openCalls.isEmpty) + } + + func testReopensTheStandardWalletWhenAnotherIdentityHoldsTheSession() async throws { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedWalletId = hiddenWalletId + session.openedWalletIdOnStandard = standardWalletId + let manager = makeManager() + + try await manager.ensureConnected(walletId: standardWalletId) + + XCTAssertEqual(session.openCalls.map(\.mode), [.standard]) + } + + /// Only the passphrase reopens a hidden wallet, so there is nothing to try automatically. + func testDemandsThePassphraseWhenAnotherIdentityHoldsTheSession() async { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedWalletId = standardWalletId + let manager = makeManager() + + await assertThrows(HwPassphraseError.required) { + try await manager.ensureConnected(walletId: hiddenWalletId) + } + XCTAssertTrue(session.openCalls.isEmpty) + } + + // MARK: - needsPassphrase + + func testNeedsThePassphraseOnlyWhileTheHiddenWalletIsNotTheLiveSession() { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + let manager = makeManager() + + session.connectedWalletId = standardWalletId + XCTAssertTrue(manager.needsPassphrase(walletId: hiddenWalletId)) + XCTAssertFalse(manager.needsPassphrase(walletId: standardWalletId), "the standard wallet needs no secret") + + session.connectedWalletId = hiddenWalletId + XCTAssertFalse(manager.needsPassphrase(walletId: hiddenWalletId), "its session is already open") + } + + // MARK: - reconnectWithPassphrase + + func testReopensAHiddenWalletWithNoLiveSession() async throws { + session.storedDevices = [ + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = nil + session.openedWalletIdOnHidden = hiddenWalletId + let manager = makeManager() + + try await manager.reconnectWithPassphrase(walletId: hiddenWalletId, passphrase: "correct horse") + + XCTAssertEqual(session.openCalls.map(\.mode), [.passphraseHost]) + XCTAssertTrue(session.staleDisconnects.isEmpty) + } + + /// The device does not reject a wrong passphrase — it silently derives another wallet — so the + /// mismatch is what stands between a typo and a signature from the wrong seed. + func testRefusesAPassphraseThatOpensADifferentWallet() async { + let stray = makeDevice(xpubs: ["nativeSegwit": "zStray"], walletId: strayWalletId, passphraseProtected: true) + session.storedDevices = [ + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.openedWalletIdOnHidden = strayWalletId + session.writesEntryOnHiddenOpen = stray + let manager = makeManager() + + await assertThrows(HwPassphraseError.mismatch) { + try await manager.reconnectWithPassphrase(walletId: hiddenWalletId, passphrase: "wrong") + } + await manager.drainPendingPersists() + + XCTAssertEqual(session.forgottenWalletIds, [strayWalletId], "the wallet a typo opened is dropped") + XCTAssertEqual(deletedWalletIds, [strayWalletId], "its activities go with it") + XCTAssertEqual(session.staleDisconnects, ["dev1"], "the session it opened is torn down") + } + + /// A wallet that was already watched before the reopen is not a stray, so it must survive. + func testKeepsAnAlreadyWatchedWalletWhenThePassphraseOpensIt() async { + session.storedDevices = [ + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + makeDevice(xpubs: ["nativeSegwit": "zOther"], walletId: strayWalletId, passphraseProtected: true), + ] + session.openedWalletIdOnHidden = strayWalletId + let manager = makeManager() + + await assertThrows(HwPassphraseError.mismatch) { + try await manager.reconnectWithPassphrase(walletId: hiddenWalletId, passphrase: "the other one") + } + await manager.drainPendingPersists() + + XCTAssertTrue(session.forgottenWalletIds.isEmpty) + XCTAssertTrue(deletedWalletIds.isEmpty) + } + + // MARK: - signFunding + + func testRefusesToSignWhenTheSessionBelongsToAnotherIdentity() async { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedWalletId = standardWalletId + let manager = makeManager() + + await assertThrows(HwPassphraseError.required) { + _ = try await manager.signFunding(walletId: hiddenWalletId, funding: makeFunding()) + } + } + + // MARK: - removeWallet + + func testRemovingAWalletForgetsOnlyThatIdentity() async { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + let manager = makeManager() + + await manager.removeWallet(walletId: hiddenWalletId) + await manager.drainPendingPersists() + + XCTAssertEqual(session.forgottenWalletIds, [hiddenWalletId]) + XCTAssertEqual(deletedWalletIds, [hiddenWalletId]) + XCTAssertEqual(session.storedDevices.compactMap(\.resolvedWalletId), [standardWalletId]) + } + + // MARK: - Helpers + + private let standardWalletId = "trezor:standard" + private let hiddenWalletId = "trezor:hidden" + private let strayWalletId = "trezor:stray" + + private func makeManager() -> HwWalletManager { + HwWalletManager( + session: session, + watcherService: NoopWatcher(), + monitoredTypes: { ["nativeSegwit"] }, + electrumUrl: { "ssl://test:1" }, + network: { .regtest }, + persistSnapshot: { _ in }, + deleteActivities: { [weak self] in self?.deletedWalletIds.append($0) } + ) + } + + private func makeDevice( + id: String = "dev1", + xpubs: [String: String] = ["nativeSegwit": "zStandard"], + walletId: String, + passphraseProtected: Bool = false + ) -> TrezorKnownDevice { + TrezorKnownDevice( + id: id, + name: "Trezor", + path: "ble://\(id)", + transportType: "bluetooth", + model: "Safe 5", + lastConnectedAt: Date(timeIntervalSince1970: 1000), + xpubs: xpubs, + walletId: walletId, + passphraseProtected: passphraseProtected + ) + } + + private func makeFunding() -> HwFundingTransaction { + HwFundingTransaction(psbt: "psbt", miningFeeSats: 141, feeRate: 1, totalSpent: 43186, satsPerVByte: 1) + } + + private func assertThrows( + _ expected: HwPassphraseError, + file: StaticString = #filePath, + line: UInt = #line, + _ operation: () async throws -> Void + ) async { + do { + try await operation() + XCTFail("expected \(expected)", file: file, line: line) + } catch let error as HwPassphraseError { + XCTAssertEqual(error, expected, file: file, line: line) + } catch { + XCTFail("expected \(expected), got \(error)", file: file, line: line) + } + } +} + +private func makeFeatures(passphraseProtection: Bool? = nil) -> TrezorFeatures { + TrezorFeatures( + vendor: "trezor.io", + model: "Safe 5", + label: "Trezor", + deviceId: "trezor-id", + majorVersion: 2, + minorVersion: 8, + patchVersion: 0, + pinProtection: false, + unlocked: true, + passphraseProtection: passphraseProtection, + initialized: true, + needsBackup: false, + passphraseEntryCapable: false + ) +} diff --git a/BitkitTests/HwWalletManagerTests.swift b/BitkitTests/HwWalletManagerTests.swift index d782755b9..263703fc4 100644 --- a/BitkitTests/HwWalletManagerTests.swift +++ b/BitkitTests/HwWalletManagerTests.swift @@ -834,7 +834,10 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(deleted, try [HwWalletId.derive(xpubs: xpubs)]) } - func testRemoveWalletForgetsEveryDeviceEntry() async throws { + /// The same wallet stored under two entries is one identity, so removing it deletes its + /// activities once. Forgetting the stored entries is the session's job — see + /// `HwWalletManagerPassphraseTests`. + func testRemoveWalletDeletesTheIdentitysActivitiesOnce() async throws { let xpubs = ["nativeSegwit": "z"] let devices = [ makeDevice(id: "dev1", xpubs: xpubs), @@ -843,11 +846,9 @@ final class HwWalletManagerTests: XCTestCase { let vm = makeViewModel(monitored: ["nativeSegwit"]) vm.updateDevices(knownDevices: devices, connectedDeviceId: nil) let wallet = try XCTUnwrap(vm.wallets.first) - var forgottenDeviceIds: [String] = [] - await vm.removeWallet(wallet) { forgottenDeviceIds.append($0) } + await vm.removeWallet(walletId: wallet.id) - XCTAssertEqual(Set(forgottenDeviceIds), wallet.deviceIds) await vm.drainPendingPersists() XCTAssertEqual(deleted, try [HwWalletId.derive(xpubs: xpubs)]) } From e807a05f38e4c3166363e63c6fec2f5edb4b68d0 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 14:00:48 -0300 Subject: [PATCH 05/18] feat: connect-flow UI for pairing a hidden wallet --- .../Localization/en.lproj/Localizable.strings | 11 ++ .../Trezor/HwConnectViewModel.swift | 174 ++++++++++++++-- .../Sheets/HardwareConnect/HwPairedView.swift | 57 +++++- .../HardwareConnect/HwPassphraseView.swift | 109 +++++++++++ .../Views/Sheets/HardwareConnectSheet.swift | 36 +++- BitkitTests/HwConnectViewModelTests.swift | 185 ++++++++++++++++-- 6 files changed, 526 insertions(+), 46 deletions(-) create mode 100644 Bitkit/Views/Sheets/HardwareConnect/HwPassphraseView.swift diff --git a/Bitkit/Resources/Localization/en.lproj/Localizable.strings b/Bitkit/Resources/Localization/en.lproj/Localizable.strings index 5998a5b5b..cca7c2ce9 100644 --- a/Bitkit/Resources/Localization/en.lproj/Localizable.strings +++ b/Bitkit/Resources/Localization/en.lproj/Localizable.strings @@ -61,6 +61,17 @@ "hardware__paired_text" = "Bitkit found funds on your device and added these to your balance."; "hardware__paired_label" = "Label Funds"; "hardware__paired_finish" = "Finish"; +"hardware__passphrase_title" = "Passphrase"; +"hardware__passphrase_button" = "Passphrase"; +"hardware__passphrase_header" = "Enter passphrase"; +"hardware__passphrase_text" = "If you have funds protected by a passphrase, enter it below to add these funds to your wallet balance as well."; +"hardware__passphrase_paired_header" = "Passphrase funds found"; +"hardware__passphrase_paired_text" = "Bitkit found funds behind a passphrase, and added these to your wallet balance."; +"hardware__passphrase_sign_text" = "Enter the passphrase of this wallet so your hardware device can sign the transfer."; +"hardware__passphrase_disabled" = "Passphrase protection is turned off on this hardware device. Enable it in Trezor Suite, then try again."; +"hardware__passphrase_duplicate" = "You are already watching this passphrase wallet."; +"hardware__passphrase_error" = "Could not open the passphrase wallet. Make sure your hardware device is unlocked and try again."; +"hardware__passphrase_mismatch" = "That passphrase opens a different wallet. Enter the one you paired this wallet with."; "hardware__pairing_title" = "Pair Device"; "hardware__pairing_text" = "Enter the 6-digit code shown on your hardware device."; "hardware__remove_button" = "Remove {name}"; diff --git a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift index 038da78cd..45f74b0a2 100644 --- a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift +++ b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift @@ -1,10 +1,11 @@ import BitkitCore import Foundation -/// Result of a successful hardware-wallet connect: the persisted known-device id and its resolved -/// display name (from the device's own label/model). +/// Result of a successful hardware-wallet connect: the persisted known-device id, the wallet +/// identity the session opened (nil until its accounts resolve), and its resolved display name. struct HwConnectResult: Equatable { let deviceId: String + let walletId: String? let name: String } @@ -13,9 +14,13 @@ struct HwConnectResult: Equatable { /// without the BLE stack. @MainActor protocol HwConnectServicing { - func scanForUnpairedDevices() async throws -> [TrezorDeviceInfo] + /// Reachable devices, unpaired first. Discovery normally hides paired devices; one is offered as + /// a fallback so its passphrase wallets can be added after the initial pairing. + func scanForDevices() async throws -> [TrezorDeviceInfo] func connect(to device: TrezorDeviceInfo) async throws -> HwConnectResult - func setDeviceLabel(id: String, label: String) + /// Opens the hidden wallet the passphrase unlocks and starts watching it; returns its wallet id. + func connectWithPassphrase(deviceId: String, passphrase: String) async throws -> String + func setWalletLabel(walletId: String, label: String) func cancelPairingCode() } @@ -25,6 +30,9 @@ protocol HwConnectServicing { /// connect, is surfaced inline by moving to `.pairCode`. Reactivity to `showPairingCode`/`wallets` /// lives in the sheet (idiomatic `.onChange`), which forwards changes via `onPairingCodeRequested()` /// / `onWalletsUpdated(_:)`. +/// +/// From the paired step the user can add the passphrase (hidden) wallets of the same device, each +/// becoming its own watched identity with its own label and balance. @Observable @MainActor final class HwConnectViewModel { @@ -33,6 +41,8 @@ final class HwConnectViewModel { case searching case found case paired + case passphrase + case passphrasePaired case pairCode } @@ -46,9 +56,14 @@ final class HwConnectViewModel { private(set) var foundDevice: TrezorDeviceInfo? private(set) var foundDeviceModel = "" private(set) var pairedDeviceId: String? + /// Identity paired on `pairedDeviceId`; resolved once its watch-only wallet is known. + private(set) var pairedWalletId: String? private(set) var deviceName = "" private(set) var balanceSats: UInt64 = 0 private(set) var labelInput = "" + /// Held only until the device answers; the passphrase is never persisted or logged. + private(set) var passphraseInput = "" + private(set) var isSubmittingPassphrase = false private(set) var errorMessage: String? /// Invoked when the user taps Finish after the label is persisted, so the host can dismiss the @@ -80,7 +95,7 @@ final class HwConnectViewModel { searchTask = Task { [weak self] in while let self, !Task.isCancelled { do { - let devices = try await service.scanForUnpairedDevices() + let devices = try await service.scanForDevices() if Task.isCancelled { return } errorMessage = nil if let device = devices.first { @@ -134,11 +149,14 @@ final class HwConnectViewModel { private func onConnected(_ result: HwConnectResult) { isConnecting = false pairedDeviceId = result.deviceId + // The device may hold several identities, so take the one this session opened rather than + // any wallet sharing its transport id. + pairedWalletId = result.walletId deviceName = result.name - if !labelInitialized { - labelInput = result.name - } - labelInitialized = true + labelInput = result.name + // Until the identity resolves, the prefill is only the device's own name; let a wallet + // emission refine it. + labelInitialized = result.walletId != nil errorMessage = nil phase = .paired } @@ -158,10 +176,21 @@ final class HwConnectViewModel { // MARK: - Paired - /// The connected wallet's aggregated balance/name landed; reflect it on the Paired step. + /// The paired wallet's aggregated balance/name landed; reflect it on the Paired step. func onWalletsUpdated(_ wallets: [HwWallet]) { guard let deviceId = pairedDeviceId else { return } - guard let wallet = wallets.first(where: { $0.id == deviceId || $0.deviceIds.contains(deviceId) }) else { return } + let wallet: HwWallet? = if let pairedWalletId { + // The store publishes a newly watched identity asynchronously: wait for it rather than + // falling back to another wallet of the same device and reporting its name, balance and + // label as this one's. + wallets.first { $0.id == pairedWalletId } + } else { + wallets.first { $0.deviceIds.contains(deviceId) && $0.isConnected } + ?? wallets.first { $0.deviceIds.contains(deviceId) } + } + guard let wallet else { return } + + pairedWalletId = wallet.id deviceName = wallet.name balanceSats = wallet.balanceSats if !labelInitialized { @@ -171,16 +200,98 @@ final class HwConnectViewModel { } func onLabelChange(_ value: String) { + // Once the user types, the field is theirs: a wallet emission arriving late must not + // overwrite what they entered. + labelInitialized = true labelInput = String(value.prefix(Self.deviceLabelMaxLength)) } - func onFinish() { - if let deviceId = pairedDeviceId { - service.setDeviceLabel(id: deviceId, label: labelInput) + // MARK: - Passphrase (hidden) wallets + + /// Each identity is labelled on its own paired step, so the one being left is persisted before + /// the next passphrase wallet takes over the field. + func onPassphraseClick() { + persistLabel() + passphraseInput = "" + errorMessage = nil + phase = .passphrase + } + + func onPassphraseChange(_ value: String) { + passphraseInput = value + } + + /// Leaves the passphrase step without keeping what was typed. + func onPassphraseBack() { + passphraseInput = "" + errorMessage = nil + phase = .paired + } + + /// Opens the hidden wallet the entered passphrase unlocks and watches it as its own identity. + /// The passphrase is dropped from state as soon as the device answers: it lives in the Trezor + /// session, never in Bitkit. + func onPassphraseSubmit() { + guard let deviceId = pairedDeviceId, !passphraseInput.isEmpty, connectTask == nil else { return } + let passphrase = passphraseInput + isSubmittingPassphrase = true + errorMessage = nil + + connectTask = Task { [weak self] in + guard let self else { return } + do { + let walletId = try await service.connectWithPassphrase(deviceId: deviceId, passphrase: passphrase) + if Task.isCancelled { return } + onPassphraseWalletAdded(walletId) + } catch { + if Task.isCancelled { return } + onPassphraseFailed(error) + } + connectTask = nil + } + } + + private func onPassphraseWalletAdded(_ walletId: String) { + isSubmittingPassphrase = false + passphraseInput = "" + pairedWalletId = walletId + balanceSats = 0 + // Fall back to the device name until the new wallet is published, and let that emission + // refine the prefill; once it resolves the field is the user's to edit. + labelInitialized = false + labelInput = deviceName + phase = .passphrasePaired + } + + private func onPassphraseFailed(_ error: Error) { + isSubmittingPassphrase = false + passphraseInput = "" + errorMessage = Self.passphraseErrorMessage(for: error) + } + + private static func passphraseErrorMessage(for error: Error) -> String { + switch error { + case HwPassphraseError.protectionDisabled: t("hardware__passphrase_disabled") + case HwPassphraseError.alreadyAdded: t("hardware__passphrase_duplicate") + default: error.isTrezorDeviceBusy() + ? TrezorErrorPresenter.userMessage(from: error) + : t("hardware__passphrase_error") } + } + + func onFinish() { + persistLabel() onFinished?() } + private func persistLabel() { + guard let walletId = pairedWalletId else { + Logger.warn("Finished pairing before its identity resolved; label not saved", context: "HwConnectViewModel") + return + } + service.setWalletLabel(walletId: walletId, label: labelInput) + } + // MARK: - Teardown /// Cancels a pending connect/pairing-code request when the user backs out mid-connect. @@ -196,6 +307,8 @@ final class HwConnectViewModel { searchTask?.cancel() searchTask = nil cancelConnect() + passphraseInput = "" + isSubmittingPassphrase = false } } @@ -206,13 +319,18 @@ final class HwConnectViewModel { @MainActor struct TrezorHwConnectService: HwConnectServicing { let trezorManager: TrezorManager + let hwWalletManager: HwWalletManager - func scanForUnpairedDevices() async throws -> [TrezorDeviceInfo] { + func scanForDevices() async throws -> [TrezorDeviceInfo] { await trezorManager.startScan() if let error = trezorManager.error { throw AppError(message: error, debugMessage: nil) } - return trezorManager.devices.filter { !TrezorKnownDeviceStorage.isKnown(id: $0.id) } + // A device that is already paired is only offered once no new one is found, so its + // passphrase wallets can be added afterwards — otherwise Add Hardware Wallet would search + // forever on the only device in range. + let (paired, unpaired) = trezorManager.devices.partitioned { TrezorKnownDeviceStorage.isKnown(id: $0.id) } + return unpaired + paired } func connect(to device: TrezorDeviceInfo) async throws -> HwConnectResult { @@ -220,18 +338,34 @@ struct TrezorHwConnectService: HwConnectServicing { guard let connected = trezorManager.connectedDevice, connected.id == device.id else { throw AppError(message: trezorManager.error ?? t("hardware__connect_error"), debugMessage: nil) } - let name = resolveHwWalletName( + let walletId = trezorManager.connectedWalletId + // Show the name it was already saved under, so re-pairing doesn't appear to rename it. + let stored = walletId.flatMap { id in hwWalletManager.wallets.first { $0.id == id } } + let name = stored?.name ?? resolveHwWalletName( label: connected.label ?? trezorManager.deviceFeatures?.label, model: connected.model ?? trezorManager.deviceFeatures?.model ) - return HwConnectResult(deviceId: connected.id, name: name) + return HwConnectResult(deviceId: connected.id, walletId: walletId, name: name) } - func setDeviceLabel(id: String, label: String) { - trezorManager.renameDevice(id: id, newName: label) + func connectWithPassphrase(deviceId: String, passphrase: String) async throws -> String { + try await hwWalletManager.connectWithPassphrase(deviceId: deviceId, passphrase: passphrase) + } + + func setWalletLabel(walletId: String, label: String) { + trezorManager.renameWallet(walletId: walletId, newName: label) } func cancelPairingCode() { trezorManager.cancelPairingCode() } } + +private extension Array { + /// Splits into (matching, rest), preserving order within each group. + func partitioned(by isMatch: (Element) -> Bool) -> (matching: [Element], rest: [Element]) { + reduce(into: ([Element](), [Element]())) { result, element in + if isMatch(element) { result.0.append(element) } else { result.1.append(element) } + } + } +} diff --git a/Bitkit/Views/Sheets/HardwareConnect/HwPairedView.swift b/Bitkit/Views/Sheets/HardwareConnect/HwPairedView.swift index 8e91098cd..c41365bfe 100644 --- a/Bitkit/Views/Sheets/HardwareConnect/HwPairedView.swift +++ b/Bitkit/Views/Sheets/HardwareConnect/HwPairedView.swift @@ -1,16 +1,32 @@ import SwiftUI -/// Paired step: the device's watch-only balance plus an editable "Label Funds" field, over the coin -/// illustration. +/// Paired step, shared by the standard wallet and by a passphrase wallet found afterwards: both +/// confirm the watched balance and its Bitkit-side label over the coin illustration, and both can add +/// another passphrase wallet from the same device before finishing. struct HwPairedView: View { let deviceName: String let balanceSats: UInt64 @Binding var labelText: String + let onPassphrase: () -> Void let onFinish: () -> Void + /// Set for the step confirming a passphrase wallet, which says so in its own words. + var isPassphraseWallet = false /// Coins illustration width as a fraction of the sheet — the 256-wide Visual in the 375-wide Figma frame. private let coinsWidthRatio: CGFloat = 256.0 / 375.0 + private var header: String { + isPassphraseWallet ? t("hardware__passphrase_paired_header") : t("hardware__paired_header") + } + + private var text: String { + isPassphraseWallet ? t("hardware__passphrase_paired_text") : t("hardware__paired_text") + } + + private var screenIdentifier: String { + isPassphraseWallet ? "HardwareWalletPassphrasePairedScreen" : "HardwareWalletPairedScreen" + } + var body: some View { ZStack(alignment: .bottom) { GeometryReader { geo in @@ -28,9 +44,9 @@ struct HwPairedView: View { .padding(.horizontal, 16) VStack(alignment: .leading, spacing: 0) { - DisplayText(t("hardware__paired_header"), accentColor: .blueAccent) + DisplayText(header, accentColor: .blueAccent) - BodyMText(t("hardware__paired_text")) + BodyMText(text) .padding(.top, 8) HwPairedBalanceView(name: deviceName, sats: balanceSats) @@ -50,16 +66,23 @@ struct HwPairedView: View { Spacer(minLength: 0) - CustomButton(title: t("hardware__paired_finish"), shouldExpand: true) { - onFinish() + HStack(spacing: 16) { + CustomButton(title: t("hardware__passphrase_button"), variant: .secondary, shouldExpand: true) { + onPassphrase() + } + .accessibilityIdentifier("HardwareWalletPairedPassphrase") + + CustomButton(title: t("hardware__paired_finish"), shouldExpand: true) { + onFinish() + } + .accessibilityIdentifier("HardwareWalletPairedFinish") } - .accessibilityIdentifier("HardwareWalletPairedFinish") .padding(.horizontal, 32) .padding(.bottom, 16) } } .accessibilityElement(children: .contain) - .accessibilityIdentifier("HardwareWalletPairedScreen") + .accessibilityIdentifier(screenIdentifier) } } @@ -85,11 +108,12 @@ private struct HwPairedBalanceView: View { } } -#Preview { +#Preview("Paired") { HwPairedView( deviceName: "Trezor Safe 3", balanceSats: 10_562_411, labelText: .constant("Trezor Safe 3"), + onPassphrase: {}, onFinish: {} ) .frame(maxWidth: .infinity, maxHeight: .infinity) @@ -97,3 +121,18 @@ private struct HwPairedBalanceView: View { .environmentObject(CurrencyViewModel()) .preferredColorScheme(.dark) } + +#Preview("Passphrase funds found") { + HwPairedView( + deviceName: "Trezor Safe 3", + balanceSats: 5_214_983, + labelText: .constant("Trezor Safe 3"), + onPassphrase: {}, + onFinish: {}, + isPassphraseWallet: true + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Color.black) + .environmentObject(CurrencyViewModel()) + .preferredColorScheme(.dark) +} diff --git a/Bitkit/Views/Sheets/HardwareConnect/HwPassphraseView.swift b/Bitkit/Views/Sheets/HardwareConnect/HwPassphraseView.swift new file mode 100644 index 000000000..c39815078 --- /dev/null +++ b/Bitkit/Views/Sheets/HardwareConnect/HwPassphraseView.swift @@ -0,0 +1,109 @@ +import SwiftUI + +/// Optional step of the connect flow: the passphrase that unlocks a hidden wallet on the paired +/// device. Bitkit binds it to a fresh Trezor session to read that wallet's accounts and never stores +/// it, so it is asked for again whenever the session has to be rebuilt. +struct HwPassphraseView: View { + @Binding var passphrase: String + let isSubmitting: Bool + let errorMessage: String? + let onBack: () -> Void + let onContinue: () -> Void + + /// Shield width as a fraction of the sheet, matching the other steps' illustration sizing. + private let shieldWidthRatio: CGFloat = 256.0 / 375.0 + + var body: some View { + VStack(spacing: 0) { + SheetHeader(title: t("hardware__passphrase_title")) + .padding(.horizontal, 16) + + VStack(alignment: .leading, spacing: 0) { + DisplayText(t("hardware__passphrase_header"), accentColor: .blueAccent) + + BodyMText(t("hardware__passphrase_text")) + .padding(.top, 8) + + TextField( + t("hardware__passphrase_title"), + text: $passphrase, + testIdentifier: "HardwareWalletPassphraseInput" + ) + // A passphrase is case- and character-exact: never let the keyboard alter it. + .textInputAutocapitalization(.never) + .autocorrectionDisabled() + .padding(.top, 32) + + if let errorMessage { + BodyMText(errorMessage, textColor: .redAccent) + .padding(.top, 16) + .accessibilityIdentifier("HwPassphraseError") + } + } + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.horizontal, 32) + + GeometryReader { geo in + Image("shield-figure") + .resizable() + .scaledToFit() + .frame(width: geo.size.width * shieldWidthRatio) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + .allowsHitTesting(false) + + HStack(spacing: 16) { + CustomButton( + title: t("common__back"), + variant: .secondary, + isDisabled: isSubmitting, + shouldExpand: true + ) { + onBack() + } + .accessibilityIdentifier("HardwareWalletPassphraseBack") + + CustomButton( + title: t("common__continue"), + isDisabled: passphrase.isEmpty || isSubmitting, + isLoading: isSubmitting, + shouldExpand: true + ) { + onContinue() + } + .accessibilityIdentifier("HardwareWalletPassphraseContinue") + } + .padding(.horizontal, 32) + .padding(.bottom, 16) + } + .screenshotPreventMask(true) + .accessibilityElement(children: .contain) + .accessibilityIdentifier("HardwareWalletPassphraseScreen") + } +} + +#Preview("Entering") { + HwPassphraseView( + passphrase: .constant("satoshirulestheworld"), + isSubmitting: false, + errorMessage: nil, + onBack: {}, + onContinue: {} + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Color.black) + .preferredColorScheme(.dark) +} + +#Preview("Rejected") { + HwPassphraseView( + passphrase: .constant(""), + isSubmitting: false, + errorMessage: t("hardware__passphrase_duplicate"), + onBack: {}, + onContinue: {} + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(Color.black) + .preferredColorScheme(.dark) +} diff --git a/Bitkit/Views/Sheets/HardwareConnectSheet.swift b/Bitkit/Views/Sheets/HardwareConnectSheet.swift index 6b6147a52..0cc8bbf52 100644 --- a/Bitkit/Views/Sheets/HardwareConnectSheet.swift +++ b/Bitkit/Views/Sheets/HardwareConnectSheet.swift @@ -9,11 +9,12 @@ struct HardwareConnectSheetItem: SheetItem { /// Entry point for the Connect Hardware flow. struct HardwareConnectSheet: View { @Environment(TrezorManager.self) private var trezorManager + @Environment(HwWalletManager.self) private var hwWalletManager let config: HardwareConnectSheetItem var body: some View { HardwareConnectFlow( - service: TrezorHwConnectService(trezorManager: trezorManager), + service: TrezorHwConnectService(trezorManager: trezorManager, hwWalletManager: hwWalletManager), config: config ) } @@ -96,8 +97,26 @@ private struct HardwareConnectFlow: View { deviceName: viewModel.deviceName, balanceSats: viewModel.balanceSats, labelText: labelBinding, + onPassphrase: viewModel.onPassphraseClick, onFinish: viewModel.onFinish ) + case .passphrase: + HwPassphraseView( + passphrase: passphraseBinding, + isSubmitting: viewModel.isSubmittingPassphrase, + errorMessage: viewModel.errorMessage, + onBack: viewModel.onPassphraseBack, + onContinue: viewModel.onPassphraseSubmit + ) + case .passphrasePaired: + HwPairedView( + deviceName: viewModel.deviceName, + balanceSats: viewModel.balanceSats, + labelText: labelBinding, + onPassphrase: viewModel.onPassphraseClick, + onFinish: viewModel.onFinish, + isPassphraseWallet: true + ) case .pairCode: HwPairCodeView(onSubmit: { trezorManager.submitPairingCode($0) }) .id(trezorManager.pairingCodeRequestID) @@ -168,13 +187,18 @@ private struct HardwareConnectFlow: View { Binding(get: { viewModel.labelInput }, set: { viewModel.onLabelChange($0) }) } - /// Changes whenever the paired wallet's name or balance changes, so the balance shown on the - /// Paired step tracks incoming watcher updates. + private var passphraseBinding: Binding { + Binding(get: { viewModel.passphraseInput }, set: { viewModel.onPassphraseChange($0) }) + } + + /// Changes whenever the wallet list changes for the paired device, so the Paired step picks up + /// its balance and its newly watched passphrase wallets as they land. private var connectedWalletKey: String { guard let deviceId = viewModel.pairedDeviceId else { return "" } - guard let wallet = hwWalletManager.wallets.first(where: { $0.id == deviceId || $0.deviceIds.contains(deviceId) }) - else { return "" } - return "\(wallet.name)\u{1}\(wallet.balanceSats)" + return hwWalletManager.wallets + .filter { $0.deviceIds.contains(deviceId) } + .map { "\($0.id)\u{1}\($0.name)\u{1}\($0.balanceSats)" } + .joined(separator: "\u{1f}") } private var isBluetoothUsable: Bool { diff --git a/BitkitTests/HwConnectViewModelTests.swift b/BitkitTests/HwConnectViewModelTests.swift index 0d7f31c07..0c6c0b1dc 100644 --- a/BitkitTests/HwConnectViewModelTests.swift +++ b/BitkitTests/HwConnectViewModelTests.swift @@ -48,7 +48,7 @@ final class HwConnectViewModelTests: XCTestCase { func testOnConnectConnectsFoundDeviceAndAdvancesToPaired() async { await givenDeviceFound() - service.connectResult = .success(HwConnectResult(deviceId: "dev1", name: "Trezor Safe 3")) + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: standardWalletId, name: "Trezor Safe 3")) sut.onConnect() @@ -93,7 +93,7 @@ final class HwConnectViewModelTests: XCTestCase { func testPairingCodeRequestSurfacesInlinePairCodeStepWhileConnecting() async { await givenDeviceFound() - service.connectResult = .success(HwConnectResult(deviceId: "dev1", name: "Trezor Safe 3")) + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: standardWalletId, name: "Trezor Safe 3")) // onConnect flips isConnecting synchronously; the connect Task is queued but not yet run, // so the pairing-code request lands mid-connect exactly as it would on device. @@ -114,7 +114,7 @@ final class HwConnectViewModelTests: XCTestCase { func testConnectedWalletUpdatesBalanceOnPairedStep() async { await givenDevicePaired() - sut.onWalletsUpdated([makeWallet(id: "dev1", name: "Trezor Safe 3", balance: 10_562_411)]) + sut.onWalletsUpdated([makeWallet(id: standardWalletId, name: "Trezor Safe 3", balance: 10_562_411)]) XCTAssertEqual(sut.balanceSats, 10_562_411) XCTAssertEqual(sut.deviceName, "Trezor Safe 3") @@ -134,11 +134,150 @@ final class HwConnectViewModelTests: XCTestCase { sut.onFinish() XCTAssertEqual(service.setLabelCalls.count, 1) - XCTAssertEqual(service.setLabelCalls.first?.id, "dev1") + XCTAssertEqual(service.setLabelCalls.first?.walletId, standardWalletId) XCTAssertEqual(service.setLabelCalls.first?.label, "My Cold Wallet") XCTAssertTrue(finished) } + // MARK: - Passphrase wallets + + func testPassphraseSubmitWatchesTheHiddenWalletAndAdvances() async { + await givenDevicePaired() + service.passphraseResult = .success(hiddenWalletId) + sut.onPassphraseClick() + sut.onPassphraseChange("correct horse") + + sut.onPassphraseSubmit() + + await waitUntil { self.sut.phase == .passphrasePaired } + XCTAssertEqual(service.passphraseCalls.first?.passphrase, "correct horse") + XCTAssertEqual(sut.pairedWalletId, hiddenWalletId) + XCTAssertTrue(sut.passphraseInput.isEmpty, "the passphrase lives in the device session, never in Bitkit") + XCTAssertEqual(sut.balanceSats, 0, "the new identity starts empty until its watcher reports") + } + + func testPassphraseFailureReportsInlineAndKeepsNoPassphrase() async { + await givenDevicePaired() + service.passphraseResult = .failure(HwPassphraseError.alreadyAdded) + sut.onPassphraseClick() + sut.onPassphraseChange("already used") + + sut.onPassphraseSubmit() + + await waitUntil { self.sut.errorMessage != nil } + XCTAssertEqual(sut.phase, .passphrase) + XCTAssertEqual(sut.errorMessage, t("hardware__passphrase_duplicate")) + XCTAssertTrue(sut.passphraseInput.isEmpty) + XCTAssertFalse(sut.isSubmittingPassphrase) + } + + func testPassphraseProtectionDisabledIsReportedInItsOwnWords() async { + await givenDevicePaired() + service.passphraseResult = .failure(HwPassphraseError.protectionDisabled) + sut.onPassphraseClick() + sut.onPassphraseChange("anything") + + sut.onPassphraseSubmit() + + await waitUntil { self.sut.errorMessage != nil } + XCTAssertEqual(sut.errorMessage, t("hardware__passphrase_disabled")) + } + + /// Each identity is labelled on its own paired step, so the one being left must be saved first. + func testMovingToThePassphraseStepPersistsTheLabelOfTheWalletBeingLeft() async { + await givenDevicePaired() + sut.onLabelChange("Standard Funds") + + sut.onPassphraseClick() + + XCTAssertEqual(sut.phase, .passphrase) + XCTAssertEqual(service.setLabelCalls.count, 1) + XCTAssertEqual(service.setLabelCalls.first?.walletId, standardWalletId) + XCTAssertEqual(service.setLabelCalls.first?.label, "Standard Funds") + } + + func testBackFromThePassphraseStepDropsWhatWasTyped() async { + await givenDevicePaired() + sut.onPassphraseClick() + sut.onPassphraseChange("secret") + + sut.onPassphraseBack() + + XCTAssertEqual(sut.phase, .paired) + XCTAssertTrue(sut.passphraseInput.isEmpty) + } + + func testDismissingTheSheetDropsTheEnteredPassphrase() async { + await givenDevicePaired() + sut.onPassphraseClick() + sut.onPassphraseChange("secret") + + sut.reset() + + XCTAssertTrue(sut.passphraseInput.isEmpty) + } + + /// A device holding several wallets shares one transport id, so the paired step must follow the + /// identity being paired and wait for it rather than adopting a sibling's name and balance. + func testPairedStepWaitsForTheIdentityBeingPaired() async { + await givenDevicePaired() + service.passphraseResult = .success(hiddenWalletId) + sut.onPassphraseClick() + sut.onPassphraseChange("correct horse") + sut.onPassphraseSubmit() + await waitUntil { self.sut.phase == .passphrasePaired } + + // The standard wallet is still the only one published. + sut.onWalletsUpdated([makeWallet(id: standardWalletId, name: "Standard", balance: 30000)]) + XCTAssertEqual(sut.balanceSats, 0, "a sibling wallet's balance is not this wallet's") + + sut.onWalletsUpdated([ + makeWallet(id: standardWalletId, name: "Standard", balance: 30000), + makeWallet(id: hiddenWalletId, name: "Hidden", balance: 20000, isConnected: true), + ]) + XCTAssertEqual(sut.balanceSats, 20000) + XCTAssertEqual(sut.deviceName, "Hidden") + } + + func testTypedLabelSurvivesAWalletEmissionArrivingAfterwards() async { + await givenDevicePaired() + sut.onLabelChange("My Cold Wallet") + + sut.onWalletsUpdated([makeWallet(id: standardWalletId, name: "Trezor Safe 3", balance: 42000)]) + + XCTAssertEqual(sut.labelInput, "My Cold Wallet") + } + + func testFinishingLabelsTheIdentityThatWasPaired() async { + await givenDevicePaired() + service.passphraseResult = .success(hiddenWalletId) + sut.onPassphraseClick() + sut.onPassphraseChange("correct horse") + sut.onPassphraseSubmit() + await waitUntil { self.sut.phase == .passphrasePaired } + sut.onLabelChange("Hidden Funds") + + sut.onFinish() + + XCTAssertEqual(service.setLabelCalls.last?.walletId, hiddenWalletId) + XCTAssertEqual(service.setLabelCalls.last?.label, "Hidden Funds") + } + + /// The device is paired either way, so the flow finishes instead of dropping out of it. + func testFinishingCompletesEvenWhenNoIdentityResolved() async { + await givenDeviceFound() + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: nil, name: "Trezor Safe 3")) + sut.onConnect() + await waitUntil { self.sut.phase == .paired } + var finished = false + sut.onFinished = { finished = true } + + sut.onFinish() + + XCTAssertTrue(service.setLabelCalls.isEmpty) + XCTAssertTrue(finished) + } + // MARK: - Helpers private func givenDeviceFound() async { @@ -149,7 +288,7 @@ final class HwConnectViewModelTests: XCTestCase { private func givenDevicePaired() async { await givenDeviceFound() - service.connectResult = .success(HwConnectResult(deviceId: "dev1", name: "Trezor Safe 3")) + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: standardWalletId, name: "Trezor Safe 3")) sut.onConnect() await waitUntil { self.sut.phase == .paired } } @@ -166,8 +305,25 @@ final class HwConnectViewModelTests: XCTestCase { ) } - private func makeWallet(id: String, name: String, balance: UInt64) -> HwWallet { - HwWallet(id: id, walletId: "trezor:\(id)", name: name, model: nil, isConnected: true, balanceSats: balance) + private let standardWalletId = "trezor:standard" + private let hiddenWalletId = "trezor:hidden" + + private func makeWallet( + id: String, + name: String, + balance: UInt64, + deviceIds: Set = ["dev1"], + isConnected: Bool = true + ) -> HwWallet { + HwWallet( + id: id, + walletId: id, + name: name, + model: nil, + isConnected: isConnected, + balanceSats: balance, + deviceIds: deviceIds + ) } private func waitUntil(timeout: TimeInterval = 2, _ condition: () -> Bool) async { @@ -187,13 +343,15 @@ private final class FakeHwConnectService: HwConnectServicing { var nearbyDevices: [TrezorDeviceInfo] = [] var scanError: Error? var connectResult: Result = .failure(TestError.stub) + var passphraseResult: Result = .failure(TestError.stub) private(set) var scanCount = 0 private(set) var connectedDeviceIds: [String] = [] - private(set) var setLabelCalls: [(id: String, label: String)] = [] + private(set) var passphraseCalls: [(deviceId: String, passphrase: String)] = [] + private(set) var setLabelCalls: [(walletId: String, label: String)] = [] private(set) var cancelPairingCount = 0 - func scanForUnpairedDevices() async throws -> [TrezorDeviceInfo] { + func scanForDevices() async throws -> [TrezorDeviceInfo] { scanCount += 1 if let scanError { throw scanError } return nearbyDevices @@ -204,8 +362,13 @@ private final class FakeHwConnectService: HwConnectServicing { return try connectResult.get() } - func setDeviceLabel(id: String, label: String) { - setLabelCalls.append((id, label)) + func connectWithPassphrase(deviceId: String, passphrase: String) async throws -> String { + passphraseCalls.append((deviceId, passphrase)) + return try passphraseResult.get() + } + + func setWalletLabel(walletId: String, label: String) { + setLabelCalls.append((walletId, label)) } func cancelPairingCode() { From a47815d89468d0253d670f580d94af42faef5239 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 14:13:35 -0300 Subject: [PATCH 06/18] feat: transfer sign with passphrase --- Bitkit/ViewModels/AppViewModel.swift | 2 + Bitkit/ViewModels/TransferViewModel.swift | 68 ++++++++++++ .../Hardware/HwPassphrasePromptSheet.swift | 80 +++++++++++++ .../Transfer/Hardware/SpendingHwSign.swift | 16 +++ BitkitTests/TransferViewModelHwTests.swift | 105 ++++++++++++++++++ changelog.d/next/659.added.md | 1 + 6 files changed, 272 insertions(+) create mode 100644 Bitkit/Views/Transfer/Hardware/HwPassphrasePromptSheet.swift create mode 100644 changelog.d/next/659.added.md diff --git a/Bitkit/ViewModels/AppViewModel.swift b/Bitkit/ViewModels/AppViewModel.swift index a8b8c4ecd..df4e0b427 100644 --- a/Bitkit/ViewModels/AppViewModel.swift +++ b/Bitkit/ViewModels/AppViewModel.swift @@ -339,6 +339,8 @@ extension AppViewModel { title: t("lightning__transfer_hw__reconnect_error_title"), description: t("lightning__transfer_hw__reconnect_error_description") ) + case .passphraseMismatch: + toast(type: .error, title: t("common__error"), description: t("hardware__passphrase_mismatch")) case let .funding(message): toast(type: .error, title: t("common__error"), description: message ?? t("common__error_body")) case let .generic(message): diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index 7b15e3e5e..986b92214 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -20,6 +20,9 @@ struct HwSpendingState: Equatable { var isLoading = false var isSigning = false var hasPendingBroadcast = false + /// The hidden wallet needs its passphrase before the device can sign for it. + var isPassphraseRequired = false + var isVerifyingPassphrase = false var miningFeeSats: UInt64 = 0 var maxAllowedToSend: UInt64 = 0 var balanceAfterFee: UInt64 = 0 @@ -55,6 +58,8 @@ enum HwTransferError: Error, Equatable { case deviceBusy /// Firmware error (code 99) — user must reconnect the device. case firmwareReconnect + /// The entered passphrase opened a different wallet than the one being spent from. + case passphraseMismatch case funding(String?) case generic(String?) } @@ -146,6 +151,7 @@ class TransferViewModel: ObservableObject { private var refreshTimer: Timer? private var refreshTask: Task? private var hwSignTask: Task? + private var hwPassphraseTask: Task? private var pendingHwFundingBroadcast: PendingHwFundingBroadcast? private var activeHwTransferWalletId: String? @@ -601,6 +607,12 @@ class TransferViewModel: ObservableObject { hwTransferError = .generic(t("common__error")) return } + // A hidden wallet whose session is gone can only be reopened with its passphrase, and the + // device would otherwise sign from whichever wallet the current session holds. + if hwConnecting?.needsPassphrase(walletId: walletId) == true { + hwSpending.isPassphraseRequired = true + return + } activeHwTransferWalletId = walletId hwSpending.isSigning = true @@ -663,6 +675,49 @@ class TransferViewModel: ObservableObject { } } + /// Reopens the hidden wallet with the entered passphrase and, once its accounts prove it is the + /// wallet the transfer is for, continues into signing. The passphrase is passed straight through + /// to the device session; it is never kept in view state. + func onHwPassphraseSubmit(order: IBtOrder, walletId: String, passphrase: String) { + guard !passphrase.isEmpty, let hwConnecting, hwPassphraseTask == nil else { return } + + hwSpending.isVerifyingPassphrase = true + hwTransferError = nil + + // Tracked separately from `hwSignTask`: on success this hands over to the confirm below, + // which installs its own signing task that this one's cleanup must not tear down. + hwPassphraseTask = Task { @MainActor [weak self] in + guard let self else { return } + defer { + self.hwSpending.isVerifyingPassphrase = false + self.hwPassphraseTask = nil + } + do { + try await hwConnecting.reconnectWithPassphrase(walletId: walletId, passphrase: passphrase) + // The prompt can be dismissed while the device is still reopening the wallet, and + // the confirm below would start a signing task a late dismiss could not reach. + guard hwSpending.isPassphraseRequired else { return } + hwSpending.isPassphraseRequired = false + onTransferToSpendingHwConfirm(order: order, walletId: walletId) + } catch is CancellationError { + // User dismissed the prompt — no toast. + } catch HwPassphraseError.mismatch { + Logger.warn("Rejected wrong passphrase for hardware wallet '\(walletId)'", context: "TransferViewModel") + hwTransferError = .passphraseMismatch + } catch { + handleRawHardwareTransferFailure(error, walletId: walletId) + } + } + } + + /// Backing out of the prompt also drops the reopen it started, so no signature is requested. + func onHwPassphraseDismiss() { + hwPassphraseTask?.cancel() + hwPassphraseTask = nil + hwSpending.isPassphraseRequired = false + hwSpending.isVerifyingPassphrase = false + } + /// Pre-connect the hardware device when the sign screen appears, mirroring Android's warm-up, so /// tapping Open Trezor Connect is less likely to hit a cold reconnect. Best-effort no-op without /// the HW capabilities. @@ -711,6 +766,8 @@ class TransferViewModel: ObservableObject { Logger.warn("Blocked hardware transfer for locked or busy Trezor '\(walletId)'", context: "TransferViewModel") case .firmwareReconnect: Logger.warn("Received Trezor firmware error for '\(walletId)'", context: "TransferViewModel") + case .passphraseMismatch: + Logger.warn("Rejected wrong passphrase for hardware wallet '\(walletId)'", context: "TransferViewModel") case let .funding(message): Logger.warn("Failed to compose hardware funding for '\(walletId)': \(message ?? "")", context: "TransferViewModel") case .generic: @@ -720,6 +777,17 @@ class TransferViewModel: ObservableObject { } private func handleRawHardwareTransferFailure(_ error: Error, walletId: String) { + // The device is open on another identity and only this wallet's passphrase reopens it, so + // raise the prompt instead of reporting a failure the user can do nothing about. + if case HwPassphraseError.required = error { + Logger.info("Asking for the passphrase to reopen hardware wallet '\(walletId)'", context: "TransferViewModel") + hwSpending.isPassphraseRequired = true + return + } + if case HwPassphraseError.mismatch = error { + hwTransferError = .passphraseMismatch + return + } if error.isTrezorDeviceBusy() { hwTransferError = .deviceBusy return diff --git a/Bitkit/Views/Transfer/Hardware/HwPassphrasePromptSheet.swift b/Bitkit/Views/Transfer/Hardware/HwPassphrasePromptSheet.swift new file mode 100644 index 000000000..939c1cfb3 --- /dev/null +++ b/Bitkit/Views/Transfer/Hardware/HwPassphrasePromptSheet.swift @@ -0,0 +1,80 @@ +import SwiftUI + +/// Asks for the passphrase of the hidden wallet a transfer signs from. Bitkit never stores it, so it +/// is needed again whenever the Trezor session that held it is gone. What is typed stays local to +/// this sheet and is handed straight to the device session. +struct HwPassphrasePromptSheet: View { + let isVerifying: Bool + let onSubmit: (String) -> Void + let onCancel: () -> Void + + @State private var passphrase = "" + @FocusState private var isFocused: Bool + + var body: some View { + VStack(spacing: 0) { + SheetHeader(title: t("hardware__passphrase_title")) + + VStack(alignment: .leading, spacing: 0) { + DisplayText(t("hardware__passphrase_header"), accentColor: .blueAccent) + + BodyMText(t("hardware__passphrase_sign_text")) + .padding(.top, 8) + + TextField( + t("hardware__passphrase_title"), + text: $passphrase, + testIdentifier: "HwTransferPassphraseInput" + ) + // A passphrase is case- and character-exact: never let the keyboard alter it. + .textInputAutocapitalization(.never) + .autocorrectionDisabled() + .focused($isFocused) + .padding(.top, 24) + } + .frame(maxWidth: .infinity, alignment: .leading) + + Spacer(minLength: 16) + + HStack(spacing: 16) { + CustomButton( + title: t("common__cancel"), + variant: .secondary, + isDisabled: isVerifying, + shouldExpand: true + ) { + onCancel() + } + .accessibilityIdentifier("HwTransferPassphraseCancel") + + CustomButton( + title: t("common__continue"), + isDisabled: passphrase.isEmpty || isVerifying, + isLoading: isVerifying, + shouldExpand: true + ) { + onSubmit(passphrase) + } + .accessibilityIdentifier("HwTransferPassphraseContinue") + } + .padding(.bottom, 16) + } + .padding(.horizontal, 16) + .sheetBackground() + .presentationDetents([.height(420)]) + .presentationCornerRadius(32) + .presentationDragIndicator(.visible) + .screenshotPreventMask(true) + .task { isFocused = true } + .accessibilityElement(children: .contain) + .accessibilityIdentifier("HwTransferPassphraseSheet") + } +} + +#Preview { + Color.black + .sheet(isPresented: .constant(true)) { + HwPassphrasePromptSheet(isVerifying: false, onSubmit: { _ in }, onCancel: {}) + } + .preferredColorScheme(.dark) +} diff --git a/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift b/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift index ef350a4fe..7d2a4dbef 100644 --- a/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift +++ b/Bitkit/Views/Transfer/Hardware/SpendingHwSign.swift @@ -58,6 +58,15 @@ struct SpendingHwSign: View { app.toast(error) transfer.hwTransferError = nil } + // A local sheet, not a route: it belongs to this order and this wallet, and swiping it away + // must take the same path as Cancel. + .sheet(isPresented: passphrasePromptBinding) { + HwPassphrasePromptSheet( + isVerifying: transfer.hwSpending.isVerifyingPassphrase, + onSubmit: { transfer.onHwPassphraseSubmit(order: order, walletId: walletId, passphrase: $0) }, + onCancel: { transfer.onHwPassphraseDismiss() } + ) + } .onDisappear { // Cancel an in-flight sign only when the user truly leaves the flow (back/reset), not when // pushing deeper (Learn More / Advanced / Signed) which keeps this route in the path. @@ -68,6 +77,13 @@ struct SpendingHwSign: View { } } + private var passphrasePromptBinding: Binding { + Binding( + get: { transfer.hwSpending.isPassphraseRequired }, + set: { if !$0 { transfer.onHwPassphraseDismiss() } } + ) + } + private func belowNav(order: IBtOrder) -> some View { VStack(alignment: .leading, spacing: 0) { DisplayText( diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index e0c6f874e..4b407882d 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -28,6 +28,111 @@ final class TransferViewModelHwTests: XCTestCase { } } + // MARK: - Passphrase (hidden) wallets + + func testConfirmAsksForThePassphraseWhenTheSessionIsGone() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + let vm = makeViewModel(funding: funding, connecting: connecting) + + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") + await awaitSigningComplete(vm) + + XCTAssertTrue(vm.hwSpending.isPassphraseRequired) + XCTAssertFalse(vm.hwSpending.isSigning) + XCTAssertEqual(connecting.ensureCalls, 0, "the device is not reached before the passphrase is known") + XCTAssertEqual(funding.signCalls, 0) + } + + /// The reopen can also be demanded mid-flight, when the session turns out to hold another wallet. + func testAPassphraseDemandedDuringSigningRaisesThePromptInsteadOfToasting() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.connectError = HwPassphraseError.required + let vm = makeViewModel(funding: funding, connecting: connecting) + + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") + await awaitSigningComplete(vm) + + XCTAssertTrue(vm.hwSpending.isPassphraseRequired) + XCTAssertNil(vm.hwTransferError, "the user can act on this, so it is a prompt and not an error") + XCTAssertEqual(funding.signCalls, 0) + } + + func testSubmittingTheRightPassphraseReopensTheWalletAndSigns() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + let vm = makeViewModel(funding: funding, connecting: connecting) + let order = IBtOrder.mock() + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + + vm.onHwPassphraseSubmit(order: order, walletId: "trezor:wallet", passphrase: "correct horse") + await awaitPassphraseVerified(vm) + await awaitSigningComplete(vm) + + XCTAssertEqual(connecting.reconnectCalls.first?.passphrase, "correct horse") + XCTAssertFalse(vm.hwSpending.isPassphraseRequired) + XCTAssertEqual(funding.signCalls, 1) + XCTAssertEqual(funding.broadcastCalls, 1) + } + + func testAPassphraseThatOpensAnotherWalletIsRejectedWithoutSigning() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + connecting.reconnectError = HwPassphraseError.mismatch + let vm = makeViewModel(funding: funding, connecting: connecting) + let order = IBtOrder.mock() + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + + vm.onHwPassphraseSubmit(order: order, walletId: "trezor:wallet", passphrase: "wrong") + await awaitPassphraseVerified(vm) + + XCTAssertEqual(vm.hwTransferError, .passphraseMismatch) + XCTAssertEqual(funding.signCalls, 0) + XCTAssertEqual(funding.broadcastCalls, 0) + XCTAssertTrue(vm.hwSpending.isPassphraseRequired, "the prompt stays up so the user can retry") + } + + /// The prompt can be swiped away while the device is still reopening the wallet. + func testDismissingThePromptStopsTheReopenFromRequestingASignature() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + let vm = makeViewModel(funding: funding, connecting: connecting) + let order = IBtOrder.mock() + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + + vm.onHwPassphraseSubmit(order: order, walletId: "trezor:wallet", passphrase: "correct horse") + vm.onHwPassphraseDismiss() + await awaitPassphraseVerified(vm) + await awaitSigningComplete(vm) + + XCTAssertFalse(vm.hwSpending.isPassphraseRequired) + XCTAssertEqual(funding.signCalls, 0, "nothing is signed after the user backs out") + XCTAssertEqual(funding.broadcastCalls, 0) + } + + func testAnEmptyPassphraseIsNotSubmitted() { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + let vm = makeViewModel(funding: funding, connecting: connecting) + + vm.onHwPassphraseSubmit(order: .mock(), walletId: "trezor:wallet", passphrase: "") + + XCTAssertTrue(connecting.reconnectCalls.isEmpty) + XCTAssertFalse(vm.hwSpending.isVerifyingPassphrase) + } + + private func awaitPassphraseVerified(_ vm: TransferViewModel, timeout: Double = 3) async { + let deadline = Date().addingTimeInterval(timeout) + while vm.hwSpending.isVerifyingPassphrase, Date() < deadline { + try? await Task.sleep(nanoseconds: 10_000_000) + } + } + func testConfirmWithoutHwCapabilitiesSurfacesGenericError() { let vm = TransferViewModel() // no signer injected vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") diff --git a/changelog.d/next/659.added.md b/changelog.d/next/659.added.md new file mode 100644 index 000000000..54ed99bef --- /dev/null +++ b/changelog.d/next/659.added.md @@ -0,0 +1 @@ +Passphrase-protected (hidden) Trezor wallets can now be paired from the connect flow, each appearing as its own watch-only balance with its own label, activity and removal, and asking for its passphrase again when a transfer needs signing. From 2d0984c056f7211c3508c76dd26c7d35db375553 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 11 Aug 2026 14:50:45 -0300 Subject: [PATCH 07/18] doc: changelog --- changelog.d/next/{659.added.md => 662.added.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{659.added.md => 662.added.md} (100%) diff --git a/changelog.d/next/659.added.md b/changelog.d/next/662.added.md similarity index 100% rename from changelog.d/next/659.added.md rename to changelog.d/next/662.added.md From f52fc4ee81062c56f559cb7a5b57dbad6b0b310a Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 08:38:50 -0300 Subject: [PATCH 08/18] fix: stale session cleanup and device name selection --- Bitkit/Managers/TrezorManager.swift | 6 +- .../Trezor/TrezorBridgeTransport.swift | 57 +++++++++++++------ .../Trezor/HwConnectViewModel.swift | 37 +++++++++--- BitkitTests/HwConnectViewModelTests.swift | 24 ++++++++ 4 files changed, 98 insertions(+), 26 deletions(-) diff --git a/Bitkit/Managers/TrezorManager.swift b/Bitkit/Managers/TrezorManager.swift index 6de4f2b56..ed17326a8 100644 --- a/Bitkit/Managers/TrezorManager.swift +++ b/Bitkit/Managers/TrezorManager.swift @@ -463,7 +463,11 @@ final class TrezorManager { try await reconnectKnownDevice(deviceId: deviceId, mode: nil) } - guard connectedDevice?.id == deviceId, let features = deviceFeatures else { + // `connect(device:)` reports failure on `error` and leaves the previous session's device and + // features in place, so identity alone would accept a failed reopen and let the caller read + // the wallet the old session had opened. The live session is the only proof. + let isLive = await trezorService.isConnected() + guard connectedDevice?.id == deviceId, isLive, let features = deviceFeatures else { let message = error ?? "Failed to open wallet on '\(deviceId)'" clearDisconnectedDeviceState(errorMessage: message) throw AppError(message: "Reconnect Hardware Device", debugMessage: message) diff --git a/Bitkit/Services/Trezor/TrezorBridgeTransport.swift b/Bitkit/Services/Trezor/TrezorBridgeTransport.swift index bdbf093ea..3c1dad3c3 100644 --- a/Bitkit/Services/Trezor/TrezorBridgeTransport.swift +++ b/Bitkit/Services/Trezor/TrezorBridgeTransport.swift @@ -17,6 +17,10 @@ final class TrezorBridgeTransport { /// tied to a single message type, so every `/call` gets the long window. Bridge is dev/E2E-only /// and management calls return immediately regardless. private static let callReadTimeout: TimeInterval = 120 + /// What the bridge calls the absence of a held session in an acquire path. + private static let noSession = "null" + /// The bridge's answer when the session offered as the previous one is not the one it holds. + private static let wrongPreviousSession = "wrong previous session" private let decoder = JSONDecoder() private let sessionLock = NSLock() @@ -71,35 +75,52 @@ final class TrezorBridgeTransport { let rawPath = Self.rawBridgePath(path) sessionLock.lock() - let previousSession = openSessions.removeValue(forKey: path) ?? enumeratedSessions[path] ?? "null" + let previousSession = openSessions.removeValue(forKey: path) ?? enumeratedSessions[path] ?? Self.noSession sessionLock.unlock() do { - let response = try post(path: "/acquire/\(Self.encode(rawPath))/\(Self.encode(previousSession))") - let bridgeSession = try decoder.decode(BridgeSession.self, from: Data(response.utf8)) - + try acquire(path: path, rawPath: rawPath, previousSession: previousSession) + } catch { + // The remembered session goes stale in both directions: a release the bridge applied but + // never confirmed, and one that never reached it at all. Rather than trust the cache, + // ask which session it holds and try once more. + guard error.localizedDescription.localizedCaseInsensitiveContains(Self.wrongPreviousSession) else { + debugLog("openDevice FAILED: \(error.localizedDescription)") + return TrezorTransportWriteResult(success: false, error: error.localizedDescription, errorCode: nil) + } + debugLog("openDevice: refreshing the session held for \(path) after a stale acquire") + _ = enumerateDevices() sessionLock.lock() - openSessions[path] = bridgeSession.session + let held = enumeratedSessions[path] ?? Self.noSession sessionLock.unlock() - - debugLog("openDevice: \(path)") - return TrezorTransportWriteResult(success: true, error: "", errorCode: nil) - } catch { - debugLog("openDevice FAILED: \(error.localizedDescription)") - return TrezorTransportWriteResult(success: false, error: error.localizedDescription, errorCode: nil) + do { + try acquire(path: path, rawPath: rawPath, previousSession: held) + } catch { + debugLog("openDevice FAILED: \(error.localizedDescription)") + return TrezorTransportWriteResult(success: false, error: error.localizedDescription, errorCode: nil) + } } + + debugLog("openDevice: \(path)") + return TrezorTransportWriteResult(success: true, error: "", errorCode: nil) + } + + private func acquire(path: String, rawPath: String, previousSession: String) throws { + let response = try post(path: "/acquire/\(Self.encode(rawPath))/\(Self.encode(previousSession))") + let bridgeSession = try decoder.decode(BridgeSession.self, from: Data(response.utf8)) + sessionLock.lock() + openSessions[path] = bridgeSession.session + sessionLock.unlock() } func closeDevice(path: String) -> TrezorTransportWriteResult { sessionLock.lock() let session = openSessions.removeValue(forKey: path) - // The enumerated session is the one being released here, so leaving it cached would offer a - // released id as the previous session on the next acquire and the bridge answers "wrong - // previous session". Switching to a passphrase wallet closes and reopens the session, so - // that would block every passphrase pairing after the first. - if let session, enumeratedSessions[path] == session { - enumeratedSessions.removeValue(forKey: path) - } + // Whatever was cached from the last enumerate is stale once a release is attempted: offering + // it as the previous session makes the bridge answer "wrong previous session". Cleared even + // when the release below fails, since a failed release leaves it just as untrustworthy — the + // retry in `openDevice` re-reads the session the bridge actually holds either way. + enumeratedSessions.removeValue(forKey: path) sessionLock.unlock() guard let session else { diff --git a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift index 45f74b0a2..5b004eabb 100644 --- a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift +++ b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift @@ -6,7 +6,19 @@ import Foundation struct HwConnectResult: Equatable { let deviceId: String let walletId: String? + /// Name of the identity this session opened — its Bitkit-side label once it has one. let name: String + /// The device's own name, from its label/model. A passphrase wallet has no label of its own + /// until the user gives it one, so this is what its step is prefilled with — the label of the + /// identity that happened to be open before it is not its name. + let deviceDefaultName: String + + init(deviceId: String, walletId: String?, name: String, deviceDefaultName: String? = nil) { + self.deviceId = deviceId + self.walletId = walletId + self.name = name + self.deviceDefaultName = deviceDefaultName ?? name + } } /// Device discovery/connection seam the Connect Hardware flow drives. `TrezorHwConnectService` is @@ -59,6 +71,9 @@ final class HwConnectViewModel { /// Identity paired on `pairedDeviceId`; resolved once its watch-only wallet is known. private(set) var pairedWalletId: String? private(set) var deviceName = "" + /// The paired device's own name, kept apart from `deviceName` so a wallet the user renamed does + /// not lend its label to the next identity opened on the same device. + private(set) var deviceDefaultName = "" private(set) var balanceSats: UInt64 = 0 private(set) var labelInput = "" /// Held only until the device answers; the passphrase is never persisted or logged. @@ -153,6 +168,7 @@ final class HwConnectViewModel { // any wallet sharing its transport id. pairedWalletId = result.walletId deviceName = result.name + deviceDefaultName = result.deviceDefaultName labelInput = result.name // Until the identity resolves, the prefill is only the device's own name; let a wallet // emission refine it. @@ -256,10 +272,12 @@ final class HwConnectViewModel { passphraseInput = "" pairedWalletId = walletId balanceSats = 0 - // Fall back to the device name until the new wallet is published, and let that emission - // refine the prefill; once it resolves the field is the user's to edit. + // A brand-new identity carries no label of its own, so it shows the device's name until the + // wallet is published and that emission refines the prefill; once it resolves the field is + // the user's to edit. + deviceName = deviceDefaultName labelInitialized = false - labelInput = deviceName + labelInput = deviceDefaultName phase = .passphrasePaired } @@ -339,13 +357,18 @@ struct TrezorHwConnectService: HwConnectServicing { throw AppError(message: trezorManager.error ?? t("hardware__connect_error"), debugMessage: nil) } let walletId = trezorManager.connectedWalletId - // Show the name it was already saved under, so re-pairing doesn't appear to rename it. - let stored = walletId.flatMap { id in hwWalletManager.wallets.first { $0.id == id } } - let name = stored?.name ?? resolveHwWalletName( + let deviceDefaultName = resolveHwWalletName( label: connected.label ?? trezorManager.deviceFeatures?.label, model: connected.model ?? trezorManager.deviceFeatures?.model ) - return HwConnectResult(deviceId: connected.id, walletId: walletId, name: name) + // Show the name it was already saved under, so re-pairing doesn't appear to rename it. + let stored = walletId.flatMap { id in hwWalletManager.wallets.first { $0.id == id } } + return HwConnectResult( + deviceId: connected.id, + walletId: walletId, + name: stored?.name ?? deviceDefaultName, + deviceDefaultName: deviceDefaultName + ) } func connectWithPassphrase(deviceId: String, passphrase: String) async throws -> String { diff --git a/BitkitTests/HwConnectViewModelTests.swift b/BitkitTests/HwConnectViewModelTests.swift index 0c6c0b1dc..df8206aa3 100644 --- a/BitkitTests/HwConnectViewModelTests.swift +++ b/BitkitTests/HwConnectViewModelTests.swift @@ -156,6 +156,30 @@ final class HwConnectViewModelTests: XCTestCase { XCTAssertEqual(sut.balanceSats, 0, "the new identity starts empty until its watcher reports") } + /// A brand-new identity has no label of its own, and the label of whichever wallet happened to be + /// open before it is not its name. + func testPassphraseWalletIsPrefilledWithTheDeviceNameNotTheRenamedWalletsLabel() async { + await givenDeviceFound() + service.connectResult = .success(HwConnectResult( + deviceId: "dev1", + walletId: standardWalletId, + name: "Standard Funds", // the standard wallet was renamed by the user + deviceDefaultName: "Trezor Safe 3" + )) + sut.onConnect() + await waitUntil { self.sut.phase == .paired } + XCTAssertEqual(sut.labelInput, "Standard Funds", "the paired wallet keeps its own label") + + service.passphraseResult = .success(hiddenWalletId) + sut.onPassphraseClick() + sut.onPassphraseChange("correct horse") + sut.onPassphraseSubmit() + await waitUntil { self.sut.phase == .passphrasePaired } + + XCTAssertEqual(sut.deviceName, "Trezor Safe 3") + XCTAssertEqual(sut.labelInput, "Trezor Safe 3") + } + func testPassphraseFailureReportsInlineAndKeepsNoPassphrase() async { await givenDevicePaired() service.passphraseResult = .failure(HwPassphraseError.alreadyAdded) From 0ef02cec92ff951e28146a7c3923743f9550a4c9 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 10:09:06 -0300 Subject: [PATCH 09/18] fix: isIdentity separates unresolved case by whether the target needs a secret --- Bitkit/Managers/HwWalletManager.swift | 12 +++++-- .../HwWalletManagerPassphraseTests.swift | 36 +++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index ca7ff05f6..f7578e13b 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -224,9 +224,17 @@ final class HwWalletManager { return deviceId } - /// A session opened before its identity could be resolved reports none and stays usable. + /// Whether the live session can be treated as `walletId`'s. + /// + /// A session whose accounts could not be read reports no identity. The standard wallet tolerates + /// that: it is reachable without a secret, so refusing would demand a passphrase that does not + /// exist. A hidden wallet is only ever opened by proving its identity — `connectWithPassphrase` + /// and `reconnectWithPassphrase` both require the open to resolve — so an unresolved session is + /// never one of them, and accepting it would compose and sign against whichever seed is loaded. private func isIdentity(_ sessionWalletId: String?, of walletId: String) -> Bool { - sessionWalletId == nil || sessionWalletId == walletId + if sessionWalletId == walletId { return true } + guard sessionWalletId == nil else { return false } + return !entries(for: walletId).contains(where: \.passphraseProtected) } private func watchedWalletIds() -> Set { diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 251d704db..5773327d1 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -154,8 +154,9 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertTrue(session.openCalls.isEmpty, "no reopen is needed") } - /// A session opened before its identity could be resolved reports none and stays usable. - func testAcceptsASessionWhoseIdentityIsNotYetResolved() async throws { + /// The standard wallet needs no secret, so refusing an unresolved session would demand a + /// passphrase that does not exist. + func testAcceptsAnUnresolvedSessionForTheStandardWallet() async throws { session.storedDevices = [makeDevice(walletId: standardWalletId)] session.connectedDeviceId = "dev1" session.connectedWalletId = nil @@ -166,6 +167,22 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertTrue(session.openCalls.isEmpty) } + /// A hidden wallet is only ever opened by proving its identity, so a session that resolved to + /// none is never one — accepting it would compose and sign against whichever seed is loaded. + func testDemandsThePassphraseWhenTheSessionResolvedToNoIdentity() async { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedWalletId = nil + let manager = makeManager() + + await assertThrows(HwPassphraseError.required) { + try await manager.ensureConnected(walletId: hiddenWalletId) + } + } + func testReopensTheStandardWalletWhenAnotherIdentityHoldsTheSession() async throws { session.storedDevices = [ makeDevice(walletId: standardWalletId), @@ -286,6 +303,21 @@ final class HwWalletManagerPassphraseTests: XCTestCase { } } + /// An account read that failed leaves a live session reporting no identity. Signing then would + /// hand the device a transaction derived from another wallet's keys. + func testRefusesToSignForAHiddenWalletWhenTheSessionResolvedToNoIdentity() async { + session.storedDevices = [ + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedDeviceId = "dev1" + session.connectedWalletId = nil + let manager = makeManager() + + await assertThrows(HwPassphraseError.required) { + _ = try await manager.signFunding(walletId: hiddenWalletId, funding: makeFunding()) + } + } + // MARK: - removeWallet func testRemovingAWalletForgetsOnlyThatIdentity() async { From 9e630e320e7fbc6907ccfe27dabc359e1be38e26 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 11:13:25 -0300 Subject: [PATCH 10/18] fix: connectWithWalletMode reopens whatever device holds de sections instead of the requested deviceId --- Bitkit/Managers/TrezorManager.swift | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Bitkit/Managers/TrezorManager.swift b/Bitkit/Managers/TrezorManager.swift index ed17326a8..fdb8ea5b4 100644 --- a/Bitkit/Managers/TrezorManager.swift +++ b/Bitkit/Managers/TrezorManager.swift @@ -453,13 +453,15 @@ final class TrezorManager { uiHandler.setWalletMode(mode, hostPassphrase: passphrase) walletMode = mode - if hadSession, let target = connectedDevice ?? knownDeviceInfo(deviceId) { - // Reconnect by path without a scan: a scan right after a disconnect usually finds - // nothing, whereas the cached handle still works. - await connect(device: target, mode: nil) + // Only the device just disconnected can be reopened from its cached handle, and only when it + // is the one being asked for: a scan right after a disconnect usually finds nothing, whereas + // that handle still works. Reaching for whatever held the session would open a *different* + // device with the selection recorded above — handing it a passphrase meant for another one. + // Any other device has no such handle, so it takes the known-device path with its scan and + // bluetooth fallback. + if hadSession, let reopening = connectedDevice, reopening.id == deviceId { + await connect(device: reopening, mode: nil) } else { - // Nothing cached to reconnect to, so take the known-device path with its scan and - // bluetooth fallback. try await reconnectKnownDevice(deviceId: deviceId, mode: nil) } @@ -476,10 +478,6 @@ final class TrezorManager { return features } - private func knownDeviceInfo(_ deviceId: String) -> TrezorDeviceInfo? { - knownDevices.first { $0.id == deviceId }.map { deviceInfo(from: $0) } - } - func submitPairingCode(_ code: String) { showPairingCode = false transport.submitPairingCode(code) From 020eb041c53bb60d1a04ca713bfdce932eb5ad60 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 11:18:10 -0300 Subject: [PATCH 11/18] fix: dont request passphrase for broadcast retry --- Bitkit/ViewModels/TransferViewModel.swift | 8 +++- BitkitTests/TransferViewModelHwTests.swift | 46 ++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index 986b92214..d1665fd72 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -608,8 +608,12 @@ class TransferViewModel: ObservableObject { return } // A hidden wallet whose session is gone can only be reopened with its passphrase, and the - // device would otherwise sign from whichever wallet the current session holds. - if hwConnecting?.needsPassphrase(walletId: walletId) == true { + // device would otherwise sign from whichever wallet the current session holds. A signed + // transaction awaiting a broadcast retry is the exception: broadcasting never reaches the + // device, so holding the retry behind a passphrase would strand funds the user already + // approved — the same reason `cancelHwSigning` leaves the device alone while one is pending. + let isBroadcastRetry = pendingHwFundingBroadcast?.matches(order: order, walletId: walletId, address: address) == true + if !isBroadcastRetry, hwConnecting?.needsPassphrase(walletId: walletId) == true { hwSpending.isPassphraseRequired = true return } diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index 4b407882d..a3815b619 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -115,6 +115,52 @@ final class TransferViewModelHwTests: XCTestCase { XCTAssertEqual(funding.broadcastCalls, 0) } + /// Broadcasting never reaches the device, so a signed transaction awaiting retry must not be held + /// behind a passphrase — the session it would reopen is not needed to send it. + func testBroadcastRetryIsNotBlockedByTheRequiredPassphrase() async { + let funding = MockHwFunding() + funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") + let connecting = MockHwConnecting() + let vm = makeViewModel(funding: funding, connecting: connecting) + let order = IBtOrder.mock() + + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + await awaitSigningComplete(vm) + XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) + + // The device session is gone by the time the user retries. + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + funding.broadcastError = nil + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + await awaitSigningComplete(vm) + + XCTAssertFalse(vm.hwSpending.isPassphraseRequired) + XCTAssertEqual(funding.signCalls, 1, "the retry reuses the signed transaction") + XCTAssertEqual(funding.broadcastCalls, 2) + XCTAssertTrue(vm.hwFundingComplete) + } + + /// Only the transaction already signed for this order is exempt; anything else needs the device. + func testADifferentOrderStillAsksForThePassphraseWhileABroadcastIsPending() async { + let funding = MockHwFunding() + funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") + let connecting = MockHwConnecting() + let vm = makeViewModel(funding: funding, connecting: connecting) + + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") + await awaitSigningComplete(vm) + XCTAssertTrue(vm.hwSpending.hasPendingBroadcast) + + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + var other = IBtOrder.mock() + other.id = "another-order" + vm.onTransferToSpendingHwConfirm(order: other, walletId: "trezor:wallet") + await awaitSigningComplete(vm) + + XCTAssertTrue(vm.hwSpending.isPassphraseRequired) + XCTAssertEqual(funding.signCalls, 1, "nothing new is signed while the passphrase is unknown") + } + func testAnEmptyPassphraseIsNotSubmitted() { let funding = MockHwFunding() let connecting = MockHwConnecting() From 11d4e2361aa5ea86c0342f49dd8a49577e955746 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 11:26:40 -0300 Subject: [PATCH 12/18] fix: cancelHwSigning must drop reopen too --- Bitkit/ViewModels/TransferViewModel.swift | 5 +++ BitkitTests/TransferViewModelHwTests.swift | 43 ++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index d1665fd72..57c23588e 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -734,6 +734,11 @@ class TransferViewModel: ObservableObject { /// on-device approval can't still sign/broadcast/record. Idempotent. No-op while a signed tx /// is awaiting broadcast retry. func cancelHwSigning() { + // The prompt and the reopen it started belong to the screen being left. Neither touches a + // signed transaction waiting to be broadcast, so they are dropped before the guard below — + // otherwise leaving mid-verify would leave the reopen running and the prompt set to reappear. + onHwPassphraseDismiss() + guard pendingHwFundingBroadcast == nil else { return } let walletId = activeHwTransferWalletId hwSignTask?.cancel() diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index a3815b619..e9ab2cf84 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -161,6 +161,49 @@ final class TransferViewModelHwTests: XCTestCase { XCTAssertEqual(funding.signCalls, 1, "nothing new is signed while the passphrase is unknown") } + /// Leaving the sign flow has to drop the reopen too, or a device answering afterwards would run + /// into a signature request for a screen the user already left. + func testLeavingTheSignFlowCancelsAnInFlightReopen() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + let vm = makeViewModel(funding: funding, connecting: connecting) + let order = IBtOrder.mock() + vm.onTransferToSpendingHwConfirm(order: order, walletId: "trezor:wallet") + vm.onHwPassphraseSubmit(order: order, walletId: "trezor:wallet", passphrase: "correct horse") + + vm.cancelHwSigning() + await awaitPassphraseVerified(vm) + await awaitSigningComplete(vm) + + XCTAssertFalse(vm.hwSpending.isPassphraseRequired) + XCTAssertFalse(vm.hwSpending.isVerifyingPassphrase) + XCTAssertEqual(funding.signCalls, 0) + XCTAssertEqual(funding.broadcastCalls, 0) + } + + /// The prompt is dropped even while a signed transaction is held for retry, which the guard in + /// `cancelHwSigning` otherwise returns before reaching. + func testLeavingTheSignFlowClearsThePromptWhileABroadcastIsPending() async { + let funding = MockHwFunding() + funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") + let connecting = MockHwConnecting() + let vm = makeViewModel(funding: funding, connecting: connecting) + + vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") + await awaitSigningComplete(vm) + connecting.walletsNeedingPassphrase = ["trezor:wallet"] + var other = IBtOrder.mock() + other.id = "another-order" + vm.onTransferToSpendingHwConfirm(order: other, walletId: "trezor:wallet") + XCTAssertTrue(vm.hwSpending.isPassphraseRequired) + + vm.cancelHwSigning() + + XCTAssertFalse(vm.hwSpending.isPassphraseRequired) + XCTAssertTrue(vm.hwSpending.hasPendingBroadcast, "the signed transaction is still retained") + } + func testAnEmptyPassphraseIsNotSubmitted() { let funding = MockHwFunding() let connecting = MockHwConnecting() From 3b9eeed6834710e241dda4adc2236e69ea9e136b Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 11:36:54 -0300 Subject: [PATCH 13/18] fix: isIdentity guard on ensureConnected now throws AppError instead of HwPassphraseError.required, for prevend request passpbrase for the standard wallet --- Bitkit/Managers/HwWalletManager.swift | 12 ++++++++++- .../HwWalletManagerPassphraseTests.swift | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index f7578e13b..88d2ec3de 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -284,7 +284,17 @@ final class HwWalletManager { throw HwPassphraseError.required } try await session.connectWithWalletMode(deviceId: deviceId, mode: .standard, passphrase: "") - guard isIdentity(session.connectedWalletId, of: walletId) else { throw HwPassphraseError.required } + guard isIdentity(session.connectedWalletId, of: walletId) else { + // Deliberately not `.required`: this wallet is reachable without a secret, so the prompt + // that error raises would ask for a passphrase that cannot open it and every entry would + // come back a mismatch. The device is simply not holding this wallet — a different seed, + // or accounts that no longer resolve to it — which is a reconnect failure. + throw AppError( + message: "Reconnect Hardware Device", + debugMessage: "Standard session on '\(deviceId)' opened " + + "'\(session.connectedWalletId ?? "no wallet")', not '\(walletId)'" + ) + } } /// Whether reaching `walletId` needs the passphrase again. The device only holds one hidden diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 5773327d1..990018d8d 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -214,6 +214,26 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertTrue(session.openCalls.isEmpty) } + /// The device is not holding this wallet at all — a different seed, or accounts that no longer + /// resolve to it. Reporting it as a missing passphrase would raise a prompt that cannot open a + /// wallet which needs no secret, and every entry would come back a mismatch. + func testReportsAReconnectFailureWhenTheStandardWalletCannotBeReopened() async { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedWalletId = strayWalletId + session.openedWalletIdOnStandard = strayWalletId + let manager = makeManager() + + do { + try await manager.ensureConnected(walletId: standardWalletId) + XCTFail("expected a reconnect failure") + } catch is HwPassphraseError { + XCTFail("a wallet with no passphrase must not be reported as needing one") + } catch { + XCTAssertEqual(session.openCalls.map(\.mode), [.standard], "the standard reopen was attempted first") + } + } + // MARK: - needsPassphrase func testNeedsThePassphraseOnlyWhileTheHiddenWalletIsNotTheLiveSession() { From c5cb845f7dbcbbd1e49aa6d25f4efe8da623df83 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 11:48:54 -0300 Subject: [PATCH 14/18] fix: separate passprase mismatch and passphrase error reading cases on reconnectWithPassphrase --- Bitkit/Managers/HwWalletManager.swift | 18 ++++++++++++----- .../HwWalletManagerPassphraseTests.swift | 20 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 88d2ec3de..627ea433c 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -336,13 +336,21 @@ final class HwWalletManager { let opened = session.connectedWalletId if opened == walletId { return } - Logger.warn( - "Rejected hardware session for '\(walletId)': opened wallet '\(opened ?? "unknown")'", - context: "HwWalletManager" - ) + guard let opened else { + // Not a mismatch: the session opened but its accounts could not be read, so nothing is + // known about which wallet it holds. Reporting a wrong passphrase would send the user to + // re-enter one that may well have been right. + await session.disconnectStaleSession(deviceId: deviceId) + throw AppError( + message: "Couldn't read the passphrase wallet", + debugMessage: "No accounts resolved for the wallet reopened on '\(deviceId)'" + ) + } + + Logger.warn("Rejected hardware session for '\(walletId)': opened wallet '\(opened)'", context: "HwWalletManager") // Reading the accounts of the wrong wallet already stored it; a mistyped passphrase must not // leave a stray watch-only wallet behind. - if let opened, !watchedBefore.contains(opened) { + if !watchedBefore.contains(opened) { await removeWallet(walletId: opened) } await session.disconnectStaleSession(deviceId: deviceId) diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 990018d8d..e91350d35 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -289,6 +289,26 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertEqual(session.staleDisconnects, ["dev1"], "the session it opened is torn down") } + /// An account read that failed says nothing about which wallet the session holds, so calling it a + /// wrong passphrase would send the user to re-enter one that may well have been right. + func testAnUnreadableReopenIsNotReportedAsAWrongPassphrase() async { + session.storedDevices = [ + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.openedWalletIdOnHidden = nil // the open succeeded, its accounts did not resolve + let manager = makeManager() + + do { + try await manager.reconnectWithPassphrase(walletId: hiddenWalletId, passphrase: "correct horse") + XCTFail("expected the read failure to be reported") + } catch is HwPassphraseError { + XCTFail("an unreadable session must not be reported as a wrong passphrase") + } catch { + XCTAssertEqual(session.staleDisconnects, ["dev1"], "the unusable session is torn down") + XCTAssertTrue(session.forgottenWalletIds.isEmpty, "there is no stray wallet to drop") + } + } + /// A wallet that was already watched before the reopen is not a stray, so it must survive. func testKeepsAnAlreadyWatchedWalletWhenThePassphraseOpensIt() async { session.storedDevices = [ From aa958e2ad04abf6a10760209beebe73296c5bda0 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 13:06:21 -0300 Subject: [PATCH 15/18] fix: a dropped session may be reported as off passphrase --- Bitkit/Managers/HwWalletManager.swift | 14 ++++++++++++-- .../HwWalletManagerPassphraseTests.swift | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 627ea433c..1189abe23 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -248,10 +248,20 @@ final class HwWalletManager { guard let session else { throw AppError(message: "Unavailable", debugMessage: "No device session to open a passphrase wallet with") } + // Absent features mean there is nothing to read the setting from — a session that dropped + // between pairing and this call — which is a reconnect problem and not a device that refuses + // hidden wallets. + guard let features = session.connectedFeatures else { + throw AppError( + message: "Reconnect Hardware Device", + debugMessage: "No live session on '\(deviceId)' to open a passphrase wallet with" + ) + } // A device with passphrase protection turned off ignores the passphrase and simply reopens // the standard wallet, which would surface as "already added" and leave the user retyping a - // passphrase that can never take effect. - guard session.connectedFeatures?.passphraseProtection == true else { + // passphrase that can never take effect. A device that does not report the setting at all is + // treated the same way, since attempting the open would fail just as silently. + guard features.passphraseProtection == true else { throw HwPassphraseError.protectionDisabled } diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index e91350d35..dfde61fa3 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -125,6 +125,25 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertTrue(session.openCalls.isEmpty, "the device is never asked to open a hidden wallet") } + /// A session that dropped between pairing and this call is a reconnect problem; telling the user + /// to enable passphrase protection they already have on would send them to Trezor Suite for + /// nothing. + func testASessionThatDroppedIsNotReportedAsProtectionBeingOff() async { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedFeatures = nil + let manager = makeManager() + + do { + _ = try await manager.connectWithPassphrase(deviceId: "dev1", passphrase: "correct horse") + XCTFail("expected a reconnect failure") + } catch is HwPassphraseError { + XCTFail("a missing session must not be reported as passphrase protection being off") + } catch { + XCTAssertTrue(session.openCalls.isEmpty, "the device is never asked to open a hidden wallet") + } + } + func testReportsAPassphraseWalletThatIsAlreadyWatched() async { session.storedDevices = [ makeDevice(walletId: standardWalletId), From dc4cac6de3ee9272f54e94902edd06fb432434fe Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 13:50:50 -0300 Subject: [PATCH 16/18] fix: do not wamup hidden wallet whose session is gone --- Bitkit/Managers/HwWalletManager.swift | 5 ++++ .../HwWalletManagerPassphraseTests.swift | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 1189abe23..387976a6a 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -325,6 +325,11 @@ final class HwWalletManager { } func warmUpConnection(walletId: String) { + // There is nothing to warm up for a hidden wallet whose session is gone: opening it needs the + // passphrase, which a warm-up has no way to ask for, and connecting without one opens the + // standard wallet instead — a session the user did not ask for, on the device the transfer is + // about to need. The prompt reopens it properly a moment later. + guard !needsPassphrase(walletId: walletId) else { return } guard let deviceId = transportDeviceId(for: walletId) else { return } session?.warmUpConnection(deviceId: deviceId) } diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index dfde61fa3..23e1289bb 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -271,6 +271,32 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertFalse(manager.needsPassphrase(walletId: hiddenWalletId), "its session is already open") } + // MARK: - warmUpConnection + + /// A warm-up cannot ask for a passphrase, so warming up a hidden wallet would open the standard + /// wallet on the very device the transfer is about to need. + func testDoesNotWarmUpAHiddenWalletWhoseSessionIsGone() { + session.storedDevices = [ + makeDevice(walletId: standardWalletId), + makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), + ] + session.connectedWalletId = nil + let manager = makeManager() + + manager.warmUpConnection(walletId: hiddenWalletId) + + XCTAssertTrue(session.warmUpCalls.isEmpty) + } + + func testWarmsUpAWalletThatNeedsNoPassphrase() { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + let manager = makeManager() + + manager.warmUpConnection(walletId: standardWalletId) + + XCTAssertEqual(session.warmUpCalls, ["dev1"]) + } + // MARK: - reconnectWithPassphrase func testReopensAHiddenWalletWithNoLiveSession() async throws { From 4fedfed78ce7f55c09f5cb74244020ae59248cae Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 12 Aug 2026 14:14:01 -0300 Subject: [PATCH 17/18] fix: prevent binding to an arbitrary identity when session isn't resolved --- .../Trezor/HwConnectViewModel.swift | 30 ++++++++---- BitkitTests/HwConnectViewModelTests.swift | 46 +++++++++++++++++++ 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift index 5b004eabb..e79e20068 100644 --- a/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift +++ b/Bitkit/ViewModels/Trezor/HwConnectViewModel.swift @@ -195,16 +195,7 @@ final class HwConnectViewModel { /// The paired wallet's aggregated balance/name landed; reflect it on the Paired step. func onWalletsUpdated(_ wallets: [HwWallet]) { guard let deviceId = pairedDeviceId else { return } - let wallet: HwWallet? = if let pairedWalletId { - // The store publishes a newly watched identity asynchronously: wait for it rather than - // falling back to another wallet of the same device and reporting its name, balance and - // label as this one's. - wallets.first { $0.id == pairedWalletId } - } else { - wallets.first { $0.deviceIds.contains(deviceId) && $0.isConnected } - ?? wallets.first { $0.deviceIds.contains(deviceId) } - } - guard let wallet else { return } + guard let wallet = pairedWallet(in: wallets, deviceId: deviceId) else { return } pairedWalletId = wallet.id deviceName = wallet.name @@ -215,6 +206,25 @@ final class HwConnectViewModel { labelInitialized = true } + /// The wallet the paired step is showing. + private func pairedWallet(in wallets: [HwWallet], deviceId: String) -> HwWallet? { + if let pairedWalletId { + // The store publishes a newly watched identity asynchronously: wait for it rather than + // falling back to another wallet of the same device and reporting its name, balance and + // label as this one's. + return wallets.first { $0.id == pairedWalletId } + } + // The connect could not resolve which identity it opened. One wallet reading as connected + // means it resolved afterwards; failing that, a device holding a single identity is + // unambiguous. Anything else is a guess, and guessing here shows a sibling wallet's balance + // and renames that wallet on Finish — a device with an unresolved session reports every one + // of its identities as connected, so there is nothing to tell them apart by. + let onDevice = wallets.filter { $0.deviceIds.contains(deviceId) } + let connected = onDevice.filter(\.isConnected) + if connected.count == 1 { return connected.first } + return onDevice.count == 1 ? onDevice.first : nil + } + func onLabelChange(_ value: String) { // Once the user types, the field is theirs: a wallet emission arriving late must not // overwrite what they entered. diff --git a/BitkitTests/HwConnectViewModelTests.swift b/BitkitTests/HwConnectViewModelTests.swift index df8206aa3..3026fe72b 100644 --- a/BitkitTests/HwConnectViewModelTests.swift +++ b/BitkitTests/HwConnectViewModelTests.swift @@ -120,6 +120,52 @@ final class HwConnectViewModelTests: XCTestCase { XCTAssertEqual(sut.deviceName, "Trezor Safe 3") } + /// A device whose session did not resolve reports every one of its identities as connected, so + /// adopting one would show a sibling's balance and rename it on Finish. + func testDoesNotAdoptAnIdentityWhenTheDeviceHoldsSeveralAndTheSessionIsUnresolved() async { + await givenDeviceFound() + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: nil, name: "Trezor Safe 3")) + sut.onConnect() + await waitUntil { self.sut.phase == .paired } + + sut.onWalletsUpdated([ + makeWallet(id: standardWalletId, name: "Standard", balance: 30000), + makeWallet(id: hiddenWalletId, name: "Hidden", balance: 20000), + ]) + + XCTAssertNil(sut.pairedWalletId) + XCTAssertEqual(sut.balanceSats, 0) + XCTAssertEqual(sut.deviceName, "Trezor Safe 3", "the device's own name stands until an identity resolves") + } + + func testAdoptsTheOnlyIdentityOfTheDeviceWhenTheSessionIsUnresolved() async { + await givenDeviceFound() + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: nil, name: "Trezor Safe 3")) + sut.onConnect() + await waitUntil { self.sut.phase == .paired } + + sut.onWalletsUpdated([makeWallet(id: standardWalletId, name: "Standard", balance: 30000)]) + + XCTAssertEqual(sut.pairedWalletId, standardWalletId) + XCTAssertEqual(sut.balanceSats, 30000) + } + + /// One identity reading as connected means the session resolved after the connect returned. + func testAdoptsTheIdentityThatResolvedAfterTheConnect() async { + await givenDeviceFound() + service.connectResult = .success(HwConnectResult(deviceId: "dev1", walletId: nil, name: "Trezor Safe 3")) + sut.onConnect() + await waitUntil { self.sut.phase == .paired } + + sut.onWalletsUpdated([ + makeWallet(id: standardWalletId, name: "Standard", balance: 30000, isConnected: false), + makeWallet(id: hiddenWalletId, name: "Hidden", balance: 20000, isConnected: true), + ]) + + XCTAssertEqual(sut.pairedWalletId, hiddenWalletId) + XCTAssertEqual(sut.balanceSats, 20000) + } + func testOnLabelChangeCapsTheLabelInput() { sut.onLabelChange(String(repeating: "a", count: 51)) XCTAssertEqual(sut.labelInput, String(repeating: "a", count: 50)) From 7c199f0f4afabf6d10a581fe6d032b3cdfab5587 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 13 Aug 2026 06:45:59 -0300 Subject: [PATCH 18/18] fix: isIdentity always falling back to default wallet --- Bitkit/Managers/HwWalletManager.swift | 49 ++++++++++--------- .../HwWalletManagerPassphraseTests.swift | 29 +++++++++-- 2 files changed, 50 insertions(+), 28 deletions(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 387976a6a..19332f75c 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -224,17 +224,25 @@ final class HwWalletManager { return deviceId } - /// Whether the live session can be treated as `walletId`'s. + /// Fails unless the live session is provably `walletId`'s. /// - /// A session whose accounts could not be read reports no identity. The standard wallet tolerates - /// that: it is reachable without a secret, so refusing would demand a passphrase that does not - /// exist. A hidden wallet is only ever opened by proving its identity — `connectWithPassphrase` - /// and `reconnectWithPassphrase` both require the open to resolve — so an unresolved session is - /// never one of them, and accepting it would compose and sign against whichever seed is loaded. - private func isIdentity(_ sessionWalletId: String?, of walletId: String) -> Bool { - if sessionWalletId == walletId { return true } - guard sessionWalletId == nil else { return false } - return !entries(for: walletId).contains(where: \.passphraseProtected) + /// A session reports no identity precisely when its accounts could not be read, which says + /// nothing about which seed it holds — a device that opened a hidden wallet and then failed the + /// read looks exactly like one that opened nothing. So an unresolved session is refused rather + /// than tolerated, and the caller either reopens the wallet or reports that it is out of reach. + /// + /// Only the passphrase reopens a hidden wallet, so that is what a hidden target asks for. For any + /// other wallet the device is simply not holding it, which no passphrase can fix. + private func requireIdentity(of walletId: String) throws { + let opened = session?.connectedWalletId + guard opened != walletId else { return } + guard !entries(for: walletId).contains(where: \.passphraseProtected) else { + throw HwPassphraseError.required + } + throw AppError( + message: "Reconnect Hardware Device", + debugMessage: "The live session holds '\(opened ?? "no resolved wallet")', not '\(walletId)'" + ) } private func watchedWalletIds() -> Set { @@ -287,24 +295,17 @@ final class HwWalletManager { } let deviceId = try requireTransportDeviceId(for: walletId) try await session.ensureConnected(deviceId: deviceId) - if isIdentity(session.connectedWalletId, of: walletId) { return } + if session.connectedWalletId == walletId { return } - Logger.info("Reopening '\(walletId)': session belongs to another identity", context: "HwWalletManager") + Logger.info("Reopening '\(walletId)': the session is not provably this wallet's", context: "HwWalletManager") guard !entries(for: walletId).contains(where: \.passphraseProtected) else { throw HwPassphraseError.required } + // A wallet that needs no secret can simply be reopened, including when the session reported + // no identity at all — reopening is what makes it provable, and it re-reads the accounts that + // failed to resolve in the first place. try await session.connectWithWalletMode(deviceId: deviceId, mode: .standard, passphrase: "") - guard isIdentity(session.connectedWalletId, of: walletId) else { - // Deliberately not `.required`: this wallet is reachable without a secret, so the prompt - // that error raises would ask for a passphrase that cannot open it and every entry would - // come back a mismatch. The device is simply not holding this wallet — a different seed, - // or accounts that no longer resolve to it — which is a reconnect failure. - throw AppError( - message: "Reconnect Hardware Device", - debugMessage: "Standard session on '\(deviceId)' opened " - + "'\(session.connectedWalletId ?? "no wallet")', not '\(walletId)'" - ) - } + try requireIdentity(of: walletId) } /// Whether reaching `walletId` needs the passphrase again. The device only holds one hidden @@ -920,7 +921,7 @@ final class HwWalletManager { ) async throws -> HwFundingSignedTx { // The session can change between connecting and signing, and signing from the wrong seed // would produce signatures that do not match the inputs being spent. - guard isIdentity(session?.connectedWalletId, of: walletId) else { throw HwPassphraseError.required } + try requireIdentity(of: walletId) let network = networkProvider() let signed = try await TrezorService.shared.signTxFromPsbt(psbtBase64: funding.psbt, network: network) return HwFundingSignedTx( diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 23e1289bb..8a68b3255 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -173,17 +173,19 @@ final class HwWalletManagerPassphraseTests: XCTestCase { XCTAssertTrue(session.openCalls.isEmpty, "no reopen is needed") } - /// The standard wallet needs no secret, so refusing an unresolved session would demand a - /// passphrase that does not exist. - func testAcceptsAnUnresolvedSessionForTheStandardWallet() async throws { + /// A session reporting no identity may be holding any seed the device has open, so it is not + /// accepted on trust. A wallet that needs no secret can simply be reopened, which re-reads the + /// accounts that failed to resolve. + func testReopensTheStandardWalletWhenTheSessionReportedNoIdentity() async throws { session.storedDevices = [makeDevice(walletId: standardWalletId)] session.connectedDeviceId = "dev1" session.connectedWalletId = nil + session.openedWalletIdOnStandard = standardWalletId let manager = makeManager() try await manager.ensureConnected(walletId: standardWalletId) - XCTAssertTrue(session.openCalls.isEmpty) + XCTAssertEqual(session.openCalls.map(\.mode), [.standard]) } /// A hidden wallet is only ever opened by proving its identity, so a session that resolved to @@ -388,6 +390,25 @@ final class HwWalletManagerPassphraseTests: XCTestCase { } } + /// The device may have opened a hidden wallet and then failed the account read, which looks + /// exactly like a session that opened nothing — so signing for the standard wallet on an + /// unresolved session would hand the device a transaction derived from another seed's keys. + func testRefusesToSignForTheStandardWalletWhenTheSessionResolvedToNoIdentity() async { + session.storedDevices = [makeDevice(walletId: standardWalletId)] + session.connectedDeviceId = "dev1" + session.connectedWalletId = nil + let manager = makeManager() + + do { + _ = try await manager.signFunding(walletId: standardWalletId, funding: makeFunding()) + XCTFail("expected the unprovable session to be refused") + } catch is HwPassphraseError { + XCTFail("a wallet with no passphrase must not be reported as needing one") + } catch { + // A reconnect failure: the device is not provably holding this wallet. + } + } + /// An account read that failed leaves a live session reporting no identity. Signing then would /// hand the device a transaction derived from another wallet's keys. func testRefusesToSignForAHiddenWalletWhenTheSessionResolvedToNoIdentity() async {