From 04cad368247c28ad7ff782890db1ba94e622afa4 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 13:53:28 -0300 Subject: [PATCH 1/7] feat: store methods --- .../Trezor/TrezorKnownDeviceStorage.swift | 146 ++++++++++++++++-- 1 file changed, 130 insertions(+), 16 deletions(-) diff --git a/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift b/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift index c365b062a..bb455d791 100644 --- a/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift +++ b/Bitkit/Services/Trezor/TrezorKnownDeviceStorage.swift @@ -1,3 +1,4 @@ +import Combine import Foundation /// One wallet identity a Trezor holds. A device with passphrase protection carries its standard @@ -99,10 +100,23 @@ extension TrezorKnownDevice { } } +/// A pending-name change to apply together with a device-list write; a nil `name` drops the entry. +struct PendingHwWalletName: Equatable { + let walletId: String + let name: String? +} + /// Persists known Trezor device metadata in UserDefaults /// THP credentials remain in Keychain via TrezorCredentialStorage enum TrezorKnownDeviceStorage { + /// Fires when the set of hardware wallet names changes, so the metadata backup can be marked + /// stale. Every connect rewrites the device list to refresh `lastConnectedAt`, and reconnect + /// traffic must not re-upload the whole envelope, so this only fires on a real name change. + static let namesChangedPublisher = namesChangedSubject.eraseToAnyPublisher() + private static let key = "trezor.knownDevices" + private static let pendingNamesKey = "trezor.pendingWalletNames" + private static let namesChangedSubject = PassthroughSubject() /// Load all known devices, sorted by most recently connected static func loadAll() -> [TrezorKnownDevice] { @@ -118,17 +132,23 @@ enum TrezorKnownDeviceStorage { var devices = loadAll() 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) - } + saveAll(devices) } /// Persist the full device list as-is. Used for bulk updates (e.g. renaming every entry of a /// device shared across transports) without per-device reordering. - static func saveAll(_ devices: [TrezorKnownDevice]) { - if let data = try? JSONEncoder().encode(devices) { - UserDefaults.standard.set(data, forKey: key) + /// + /// - Parameter pendingName: a pending-name change to apply in the same call, or nil to leave the + /// pending names alone. It is written *first*: crashing between the two writes then leaves a name + /// recorded for a wallet that is still paired, which the next pairing masks away, rather than a + /// forgotten wallet whose name was recorded nowhere. + static func saveAll(_ devices: [TrezorKnownDevice], pendingName: PendingHwWalletName? = nil) { + let previousNames = backupSnapshot() + if let pendingName { + writePendingName(pendingName) } + writeDevices(devices) + notifyIfNamesChanged(from: previousNames) } /// Entries tracking one wallet identity. @@ -138,29 +158,123 @@ enum TrezorKnownDeviceStorage { /// Forget every identity of a device, whichever wallets it holds. static func remove(id: String) { - var devices = loadAll() - devices.removeAll { $0.id == id } - if let data = try? JSONEncoder().encode(devices) { - UserDefaults.standard.set(data, forKey: key) - } + let devices = loadAll() + forget(devices.filter { $0.id == id }, keeping: devices.filter { $0.id != id }) } /// 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) - } + let devices = loadAll() + forget( + devices.filter { $0.resolvedWalletId == walletId }, + keeping: devices.filter { $0.resolvedWalletId != walletId } + ) } /// Remove all remembered Trezor devices. static func removeAll() { + let previousNames = backupSnapshot() UserDefaults.standard.removeObject(forKey: key) + UserDefaults.standard.removeObject(forKey: pendingNamesKey) + notifyIfNamesChanged(from: previousNames) } /// Check if a device is known static func isKnown(id: String) -> Bool { loadAll().contains { $0.id == id } } + + // MARK: - Hardware wallet names + + /// Names of wallets that no device entry carries: restored from a backup before the device was + /// paired again, or kept when the wallet was removed. + /// + /// A wallet the device list already names is masked out rather than pruned, so pairing consumes + /// a pending name by simply adopting it — no second write that could be lost on its own. + static func loadPendingNames() -> [String: String] { + let paired = pairedNames() + return storedPendingNames().filter { paired[$0.key] == nil } + } + + /// Stores the name of a wallet with no device entry, or drops it when `name` is nil or blank. + static func setPendingName(walletId: String, name: String?) { + let previousNames = backupSnapshot() + writePendingName(PendingHwWalletName(walletId: walletId, name: name)) + notifyIfNamesChanged(from: previousNames) + } + + /// Every hardware wallet name this wallet knows, keyed by wallet id: the pending ones overlaid + /// with the name of each paired wallet. A paired name wins because it is what the user currently + /// sees. Entries without a wallet id are skipped — only a device stored before any account key + /// was captured has none, and such an entry is filtered out of the wallet list anyway, so it can + /// never have been named. + static func backupSnapshot() -> [String: String] { + storedPendingNames().merging(pairedNames()) { _, paired in paired } + } + + /// Merges backed up names into the pending ones, so each is adopted the next time its wallet is + /// paired. Names already held locally win: they were set on this device after the backup was + /// written. Never clears — an envelope without names predates the field and must not drop what is + /// stored. + static func restoreNames(_ names: [String: String]) { + guard !names.isEmpty else { return } + let previousNames = backupSnapshot() + writePendingNames(names.merging(storedPendingNames()) { _, local in local }) + notifyIfNamesChanged(from: previousNames) + } + + // MARK: - Storage + + /// Drop `forgotten` from the device list and with it any name kept for the wallets it held: a + /// removal that wanted to keep a name writes it back through `saveAll(_:pendingName:)` instead. + private static func forget(_ forgotten: [TrezorKnownDevice], keeping remaining: [TrezorKnownDevice]) { + let previousNames = backupSnapshot() + let remainingWalletIds = Set(remaining.compactMap(\.resolvedWalletId)) + var pending = storedPendingNames() + for walletId in forgotten.compactMap(\.resolvedWalletId) where !remainingWalletIds.contains(walletId) { + pending[walletId] = nil + } + writePendingNames(pending) + writeDevices(remaining) + notifyIfNamesChanged(from: previousNames) + } + + private static func writeDevices(_ devices: [TrezorKnownDevice]) { + guard let data = try? JSONEncoder().encode(devices) else { return } + UserDefaults.standard.set(data, forKey: key) + } + + private static func storedPendingNames() -> [String: String] { + UserDefaults.standard.dictionary(forKey: pendingNamesKey) as? [String: String] ?? [:] + } + + private static func writePendingName(_ update: PendingHwWalletName) { + guard !update.walletId.isEmpty else { return } + var pending = storedPendingNames() + pending[update.walletId] = update.name.flatMap { $0.isEmpty ? nil : $0 } + writePendingNames(pending) + } + + private static func writePendingNames(_ names: [String: String]) { + if names.isEmpty { + UserDefaults.standard.removeObject(forKey: pendingNamesKey) + } else { + UserDefaults.standard.set(names, forKey: pendingNamesKey) + } + } + + private static func pairedNames() -> [String: String] { + var names: [String: String] = [:] + for device in loadAll() { + guard let walletId = device.resolvedWalletId, !walletId.isEmpty else { continue } + guard let label = device.customLabel, !label.isEmpty else { continue } + names[walletId] = label + } + return names + } + + private static func notifyIfNamesChanged(from previousNames: [String: String]) { + guard backupSnapshot() != previousNames else { return } + namesChangedSubject.send() + } } From ae34b3e4815237170d84549d0f63b7ccb338df23 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 13:55:38 -0300 Subject: [PATCH 2/7] feat: backup wiring --- Bitkit/Models/BackupPayloads.swift | 22 ++++++++++++++++++++++ Bitkit/Services/BackupService.swift | 28 ++++++++++++++++++++++++++-- Bitkit/Services/CoreService.swift | 27 +++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/Bitkit/Models/BackupPayloads.swift b/Bitkit/Models/BackupPayloads.swift index e3f38fef1..48e09e16b 100644 --- a/Bitkit/Models/BackupPayloads.swift +++ b/Bitkit/Models/BackupPayloads.swift @@ -20,6 +20,28 @@ struct MetadataBackupV1: Codable { let cache: AppCacheData let pubkySession: PubkySessionBackupV1? let pubkyContactProfileOverrides: [String: PubkyProfileData]? + /// User-set hardware wallet names, keyed by bitkit-core wallet id. Nil in envelopes written + /// before this field, and in envelopes from an app that knows no names. Shared with + /// bitkit-android, which spells the key the same way — see its `MetadataBackupV1`. + let hwWalletNames: [String: String]? + + init( + version: Int, + createdAt: UInt64, + tagMetadata: [PreActivityMetadata], + cache: AppCacheData, + pubkySession: PubkySessionBackupV1?, + pubkyContactProfileOverrides: [String: PubkyProfileData]?, + hwWalletNames: [String: String]? = nil + ) { + self.version = version + self.createdAt = createdAt + self.tagMetadata = tagMetadata + self.cache = cache + self.pubkySession = pubkySession + self.pubkyContactProfileOverrides = pubkyContactProfileOverrides + self.hwWalletNames = hwWalletNames + } } struct PubkySessionBackupV1: Codable, Equatable { diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift index 628d824b2..82a27728f 100644 --- a/Bitkit/Services/BackupService.swift +++ b/Bitkit/Services/BackupService.swift @@ -264,10 +264,18 @@ class BackupService { } ContactsManager.restoreContactProfileOverrides(payload.pubkyContactProfileOverrides) + // App-owned, so it takes no part in the core field migration above and never sets + // needsRewrite. Restored names wait as pending ones until each wallet is paired again. + TrezorKnownDeviceStorage.restoreNames(payload.hwWalletNames ?? [:]) + // Force address rotation by clearing onchain address UserDefaults.standard.set("", forKey: "onchainAddress") - Logger.debug("Restored caches, \(payload.tagMetadata.count) pre-activity metadata", context: "BackupService") + Logger.debug( + "Restored caches, \(payload.tagMetadata.count) pre-activity metadata, " + + "\(payload.hwWalletNames?.count ?? 0) hardware wallet names", + context: "BackupService" + ) } if didRestoreWalletBackup { @@ -420,6 +428,16 @@ class BackupService { } .store(in: &cancellables) + // METADATA (hardware wallet names). Scoped to the names alone: the known-device store is also + // rewritten by every connect, and reconnect traffic must not re-upload the whole envelope. + TrezorKnownDeviceStorage.namesChangedPublisher + .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) + .sink { [weak self] _ in + guard let self, !self.shouldSkipBackup() else { return } + markBackupRequired(category: .metadata) + } + .store(in: &cancellables) + // APP STATE (UserDefaults changes, etc.) Task { @MainActor in SettingsViewModel.shared.appStatePublisher @@ -771,13 +789,19 @@ class BackupService { // side to land on. let preActivityMetadata = HwActivityTagBackup.deduplicated(hardwareTagMetadata + storedPreActivityMetadata) + // A UserDefaults read that cannot fail, so unlike the tags above there is no partial-read + // case to guard against. Nil rather than an empty map when nothing is named, so an + // envelope this app writes stays byte-comparable with one bitkit-android writes. + let hwWalletNames = TrezorKnownDeviceStorage.backupSnapshot() + let payload = MetadataBackupV1( version: 1, createdAt: currentTime, tagMetadata: preActivityMetadata, cache: cache, pubkySession: pubkySession, - pubkyContactProfileOverrides: pubkyContactProfileOverrides + pubkyContactProfileOverrides: pubkyContactProfileOverrides, + hwWalletNames: hwWalletNames.isEmpty ? nil : hwWalletNames ) return try JSONEncoder().encode(payload) diff --git a/Bitkit/Services/CoreService.swift b/Bitkit/Services/CoreService.swift index b0096448f..cd0b10bc8 100644 --- a/Bitkit/Services/CoreService.swift +++ b/Bitkit/Services/CoreService.swift @@ -1495,6 +1495,30 @@ class ActivityService { } } + /// The slice of the metadata backup's tag data that belongs to `walletId`, built the same way the + /// envelope builds it so a caller can preserve a wallet's tags across a deletion. + /// + /// Both sources are needed: core drops a wallet's stored `PreActivityMetadata` along with its + /// activities, and those rows are not covered by the rendered set, which only shapes tags that + /// already reached an activity. + func tagMetadata(forWallet walletId: String) async throws -> [BitkitCore.PreActivityMetadata] { + try await ServiceQueue.background(.core) { () throws -> [BitkitCore.PreActivityMetadata] in + let stored = try BitkitCore.getAllPreActivityMetadata().filter { $0.walletId == walletId } + let hardwareTags = try BitkitCore.getAllActivitiesTags().filter { $0.walletId == walletId } + let rendered = try hardwareTags.isEmpty + ? [] + : HwActivityTagBackup.preActivityMetadata( + activities: Self.storedOnchainActivities(walletId: walletId), + tags: hardwareTags + ) + + // Rendered first, matching the envelope build and for the same reason: a stored row can + // outlive the activity it was meant for and hold tags the user has since edited, and + // keeping a wallet's tags means keeping what the user currently sees. + return HwActivityTagBackup.deduplicated(rendered + stored) + } + } + func upsertTags(_ activityTags: [ActivityTags]) async throws { try await ServiceQueue.background(.core) { try BitkitCore.upsertTags(activityTags: activityTags) @@ -1549,6 +1573,9 @@ class ActivityService { func upsertPreActivityMetadata(_ preActivityMetadata: [BitkitCore.PreActivityMetadata]) async throws { try await ServiceQueue.background(.core) { try BitkitCore.upsertPreActivityMetadata(preActivityMetadata: preActivityMetadata) + // Rows written back after a hardware wallet's delete cascade have to reach the next + // envelope; a restore's own upsert is inert here, since `shouldSkipBackup` gates it. + self.metadataChangedSubject.send() } } From b165d3a2dce5299f77369cc8217ad6fbcd00618d Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 13:58:21 -0300 Subject: [PATCH 3/7] feat: create and consume pending names --- Bitkit/Managers/HwDeviceSessioning.swift | 6 +++++- Bitkit/Managers/HwWalletManager.swift | 2 +- Bitkit/Managers/TrezorManager.swift | 21 +++++++++++++++---- .../HwWalletManagerPassphraseTests.swift | 5 ++++- 4 files changed, 27 insertions(+), 7 deletions(-) diff --git a/Bitkit/Managers/HwDeviceSessioning.swift b/Bitkit/Managers/HwDeviceSessioning.swift index e8b96c57d..4b063e4fe 100644 --- a/Bitkit/Managers/HwDeviceSessioning.swift +++ b/Bitkit/Managers/HwDeviceSessioning.swift @@ -29,7 +29,11 @@ protocol HwDeviceSessioning: AnyObject, Sendable { 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 + /// + /// - Parameter pendingName: a name to keep for the wallet being forgotten, so re-pairing the + /// device restores it, or nil to drop any name kept for it. It rides the same store write that + /// forgets the entries, so the device list is never published while the name is missing. + func forgetWallet(walletId: String, pendingName: PendingHwWalletName?) async } extension TrezorManager: HwDeviceSessioning { diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index 19332f75c..bd4f918b2 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -199,7 +199,7 @@ final class HwWalletManager { /// Other wallets of the same physical device stay paired. func removeWallet(walletId: String) async { removeDevice(walletId: walletId) - await session?.forgetWallet(walletId: walletId) + await session?.forgetWallet(walletId: walletId, pendingName: nil) } // MARK: - Wallet identity & the device session diff --git a/Bitkit/Managers/TrezorManager.swift b/Bitkit/Managers/TrezorManager.swift index fdb8ea5b4..fc3ddde97 100644 --- a/Bitkit/Managers/TrezorManager.swift +++ b/Bitkit/Managers/TrezorManager.swift @@ -550,6 +550,12 @@ final class TrezorManager { copy.customLabel = customLabel return copy } + // Cleared before the label is written, while the entry still masks it: a pending name left + // behind would resurface the moment the user clears this label, resurrecting a name they + // replaced. Safe to drop first — the entry already carries whatever it adopted. + for walletId in Set(devices.filter(isTarget).compactMap(\.resolvedWalletId)) { + TrezorKnownDeviceStorage.setPendingName(walletId: walletId, name: nil) + } TrezorKnownDeviceStorage.saveAll(updated) loadKnownDevices() } @@ -592,6 +598,13 @@ final class TrezorManager { let identityKey = TrezorKnownDevice.walletKey(for: mergedXpubs, fallback: device.id) let named = TrezorKnownDeviceMatching.named(in: stored, previous: previous, walletKey: identityKey) + // A name restored from a backup, or kept when this wallet was removed, waits as a pending one + // until the wallet is paired again — which is here. A name set locally wins: it was chosen on + // this device, after the backup was written. Adopting it is all the consuming needed, since + // `loadPendingNames` masks out wallets the device list already names. + let walletId = resolvedWalletId(previous: previous, identityKey: identityKey, xpubs: mergedXpubs, in: stored) + let pendingName = walletId.flatMap { TrezorKnownDeviceStorage.loadPendingNames()[$0] } + let known = TrezorKnownDevice( id: device.id, name: device.name ?? "Trezor", @@ -601,8 +614,8 @@ final class TrezorManager { model: device.model ?? deviceFeatures?.model, lastConnectedAt: Date(), xpubs: mergedXpubs, - customLabel: named?.customLabel, - walletId: resolvedWalletId(previous: previous, identityKey: identityKey, xpubs: mergedXpubs, in: stored), + customLabel: named?.customLabel ?? pendingName, + walletId: walletId, passphraseProtected: passphraseProtection(previous: previous), trezorDeviceId: deviceFeatures?.deviceId ?? previous?.trezorDeviceId ) @@ -720,7 +733,7 @@ final class TrezorManager { /// 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 { + func forgetWallet(walletId: String, pendingName: PendingHwWalletName? = nil) async { let stored = TrezorKnownDeviceStorage.loadAll() let forgotten = stored.filter { $0.resolvedWalletId == walletId } guard !forgotten.isEmpty else { @@ -733,7 +746,7 @@ final class TrezorManager { await clearCredentials(path: entry.path) } - TrezorKnownDeviceStorage.saveAll(remaining) + TrezorKnownDeviceStorage.saveAll(remaining, pendingName: pendingName) loadKnownDevices() trezorLog("Forgot hardware wallet: \(walletId)") diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 8a68b3255..5febdaba9 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -60,6 +60,8 @@ final class HwWalletManagerPassphraseTests: XCTestCase { return connectedFeatures ?? makeFeatures() } + var forgottenPendingNames: [PendingHwWalletName?] = [] + func disconnectStaleSession(deviceId: String) async { staleDisconnects.append(deviceId) } @@ -72,8 +74,9 @@ final class HwWalletManagerPassphraseTests: XCTestCase { warmUpCalls.append(deviceId) } - func forgetWallet(walletId: String) async { + func forgetWallet(walletId: String, pendingName: PendingHwWalletName?) async { forgottenWalletIds.append(walletId) + forgottenPendingNames.append(pendingName) storedDevices.removeAll { $0.resolvedWalletId == walletId } if connectedWalletId == walletId { connectedWalletId = nil } } From 5ee1cf53a0777cd79a19d19de9b38d7bf908c746 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 14:03:17 -0300 Subject: [PATCH 4/7] feat: wallet removal flow --- Bitkit/Managers/HwWalletManager.swift | 103 ++++++++++++++++-- .../HardwareWalletsSettingsScreen.swift | 2 +- .../Views/Wallets/HardwareWalletScreen.swift | 2 +- .../HwWalletManagerPassphraseTests.swift | 4 +- BitkitTests/HwWalletManagerTests.swift | 2 +- 5 files changed, 99 insertions(+), 14 deletions(-) diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index bd4f918b2..ff1b8dcdb 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -53,6 +53,8 @@ final class HwWalletManager { private let networkProvider: () -> TrezorCoinType private let persistSnapshot: @MainActor (HwWalletSnapshot) async throws -> Void private let deleteActivities: @MainActor (String) async throws -> Void + private let readTagMetadata: @MainActor (String) async throws -> [PreActivityMetadata] + private let writeTagMetadata: @MainActor ([PreActivityMetadata]) 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 @@ -93,6 +95,11 @@ final class HwWalletManager { /// content is byte-identical — that write is what applies the deletions the partial ones deferred. private var lastPersisted: [String: HwWalletSnapshot] = [:] + /// Tag metadata a removal asked to keep, held until the wallet's activities are gone and it can + /// be written back. Read at the moment each delete runs rather than captured, so a re-pair or a + /// second removal that keeps nothing cancels a repair that has not happened yet. + private var keptBackupMetadata: [String: [PreActivityMetadata]] = [:] + private var emittedReceivedTxIds: Set = [] private var listeners: [String: TrezorEventListener] = [:] @@ -103,7 +110,9 @@ final class HwWalletManager { electrumUrl: (() -> String)? = nil, network: (() -> TrezorCoinType)? = nil, persistSnapshot: (@MainActor (HwWalletSnapshot) async throws -> Void)? = nil, - deleteActivities: (@MainActor (String) async throws -> Void)? = nil + deleteActivities: (@MainActor (String) async throws -> Void)? = nil, + readTagMetadata: (@MainActor (String) async throws -> [PreActivityMetadata])? = nil, + writeTagMetadata: (@MainActor ([PreActivityMetadata]) async throws -> Void)? = nil ) { self.session = session self.watcherService = watcherService @@ -126,6 +135,12 @@ final class HwWalletManager { self.deleteActivities = deleteActivities ?? { walletId in _ = try await CoreService.shared.activity.deleteByWalletId(walletId) } + self.readTagMetadata = readTagMetadata ?? { walletId in + try await CoreService.shared.activity.tagMetadata(forWallet: walletId) + } + self.writeTagMetadata = writeTagMetadata ?? { records in + try await CoreService.shared.activity.upsertPreActivityMetadata(records) + } } // MARK: - Device input @@ -162,9 +177,13 @@ final class HwWalletManager { /// 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) { + /// entries (via `TrezorManager`). + /// + /// - Parameter keptMetadata: tag metadata to re-apply after each delete of this wallet, or empty + /// to keep nothing. Passed on every call so a removal that keeps nothing clears what an earlier + /// one left behind. + func removeDevice(walletId: String, keptMetadata: [PreActivityMetadata] = []) { + keptBackupMetadata[walletId] = keptMetadata.isEmpty ? nil : keptMetadata for watcherId in activeWatchers where self.walletId(fromWatcherId: watcherId) == walletId { _ = stopActiveWatcher(watcherId) } @@ -173,6 +192,11 @@ final class HwWalletManager { for device in knownDevices where device.resolvedWalletId == walletId { walletIdCache[xpubsSignature(device.xpubs)] = nil } + // Dropped here rather than left to the next `updateDevices(...)` push. Until the wallet leaves + // `hwWalletIds`, the push's own cleanup deletes its activities a second time — after any kept + // metadata was written back — and `deviceGroups()` still yields the group, so a watcher event + // arriving in that window re-persists the activities this just deleted. + knownDevices.removeAll { $0.resolvedWalletId == walletId } recomputeDerivedState() } @@ -197,9 +221,37 @@ final class HwWalletManager { /// Removes a hardware wallet and forgets every stored entry that belongs to its wallet identity. /// Other wallets of the same physical device stay paired. - func removeWallet(walletId: String) async { - removeDevice(walletId: walletId) - await session?.forgetWallet(walletId: walletId, pendingName: nil) + /// + /// - Parameter keepBackupData: whether to carry the wallet's name and tags in the backup, so + /// re-pairing the device restores them. Core deletes a wallet's activities, its activity tags and + /// its pre-activity metadata in one cascade, which is both of the sources the metadata envelope + /// draws hardware tags from, so without this a removal silently empties the backup of them. + func removeWallet(walletId: String, keepBackupData: Bool) async throws { + // Everything here reads; nothing has been deleted yet, so a failure leaves the wallet whole. + var keptName: String? + var keptMetadata: [PreActivityMetadata] = [] + if keepBackupData { + keptName = entries(for: walletId) + .lazy + .compactMap { $0.customLabel?.trimmingCharacters(in: .whitespacesAndNewlines) } + .first { !$0.isEmpty } + do { + keptMetadata = try await readTagMetadata(walletId) + } catch { + // Refused rather than reported as a failed removal: the wallet is untouched, so the + // choice stays with the user — retry, or remove it without keeping the data. + Logger.error("Failed to read tag metadata of HW wallet '\(walletId)': \(error)", context: "HwWalletManager") + throw HwWalletRemovalError.backupDataUnreadable + } + } + + removeDevice(walletId: walletId, keptMetadata: keptMetadata) + // The name rides the same store write that forgets the entries carrying it. A nil name is + // passed deliberately when keeping nothing: it drops a name an earlier removal kept. + await session?.forgetWallet( + walletId: walletId, + pendingName: PendingHwWalletName(walletId: walletId, name: keptName) + ) } // MARK: - Wallet identity & the device session @@ -367,7 +419,11 @@ final class HwWalletManager { // Reading the accounts of the wrong wallet already stored it; a mistyped passphrase must not // leave a stray watch-only wallet behind. if !watchedBefore.contains(opened) { - await removeWallet(walletId: opened) + // The wallet is a real one the user owns, and reading its accounts already stored it — + // consuming any name restored for it into the entry about to be forgotten. Keeping its + // data puts that name back where re-pairing will find it. Failing here must not replace + // the mismatch the caller is waiting for. + try? await removeWallet(walletId: opened, keepBackupData: true) } await session.disconnectStaleSession(deviceId: deviceId) throw HwPassphraseError.mismatch @@ -469,6 +525,9 @@ final class HwWalletManager { let liveSignatures = Set(knownDevices.filter { !$0.xpubs.isEmpty }.map { xpubsSignature($0.xpubs) }) walletIdCache = walletIdCache.filter { liveSignatures.contains($0.key) } lastPersisted = lastPersisted.filter { hwWalletIds.contains($0.key) } + // A wallet that is paired again owns its metadata through its watcher; a pending repair from + // an earlier removal would only re-apply rows core has since re-attached anyway. + keptBackupMetadata = keptBackupMetadata.filter { !hwWalletIds.contains($0.key) } } private func startWatcher(_ spec: WatcherSpec) { @@ -609,11 +668,29 @@ final class HwWalletManager { private func delete(walletId: String) { persistQueue.enqueue(walletId: walletId) { [weak self] in + guard let self else { return } do { - try await self?.deleteActivities(walletId) + try await deleteActivities(walletId) } catch { Logger.error("Failed to delete activities for HW wallet '\(walletId)': \(error)", context: "HwWalletManager") } + await restoreKeptMetadata(walletId: walletId) + } + } + + /// Re-apply the tag metadata a removal asked to keep, as the tail of the delete that took it. + /// Core drops a wallet's pre-activity metadata along with its activities whether or not any + /// matched, so this belongs to every delete of the wallet rather than to the removal alone — a + /// later cleanup pass then repairs itself instead of destroying the kept rows. Core re-attaches + /// them once a watcher recreates the activities, so re-pairing the device brings the tags back. + private func restoreKeptMetadata(walletId: String) async { + guard let kept = keptBackupMetadata[walletId], !kept.isEmpty else { return } + do { + try await writeTagMetadata(kept) + } catch { + // The activities are already gone and the watchers already stopped, so there is nothing + // to roll back to and reporting a failed removal would be false. The tags are lost. + Logger.error("Failed to keep tag metadata of HW wallet '\(walletId)': \(error)", context: "HwWalletManager") } } @@ -1011,6 +1088,14 @@ final class SnapshotPersistQueue { } } +/// Failures of a hardware-wallet removal the user can act on. +enum HwWalletRemovalError: Error, Equatable { + /// The removal asked to keep the wallet's backup data, but its tags could not be read. Raised + /// before anything is deleted, so the wallet is untouched and the removal can be retried or + /// repeated without keeping the data. + case backupDataUnreadable +} + /// 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 { diff --git a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift index b9266cf21..0328e91a7 100644 --- a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift +++ b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift @@ -102,7 +102,7 @@ struct HardwareWalletsSettingsScreen: View { private func remove(_ wallet: HwWallet) async { pendingRemoval = nil - await hwWalletManager.removeWallet(walletId: wallet.id) + try? await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: true) } } diff --git a/Bitkit/Views/Wallets/HardwareWalletScreen.swift b/Bitkit/Views/Wallets/HardwareWalletScreen.swift index 12279add9..17a23dcaa 100644 --- a/Bitkit/Views/Wallets/HardwareWalletScreen.swift +++ b/Bitkit/Views/Wallets/HardwareWalletScreen.swift @@ -180,7 +180,7 @@ struct HardwareWalletScreen: View { /// tile, and the reactive auto-pop above then leaves the screen. private func removeWallet() async { guard let wallet else { return } - await hwWalletManager.removeWallet(walletId: wallet.id) + try? await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: true) } } diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 5febdaba9..81042ce08 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -429,14 +429,14 @@ final class HwWalletManagerPassphraseTests: XCTestCase { // MARK: - removeWallet - func testRemovingAWalletForgetsOnlyThatIdentity() async { + func testRemovingAWalletForgetsOnlyThatIdentity() async throws { session.storedDevices = [ makeDevice(walletId: standardWalletId), makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: hiddenWalletId, passphraseProtected: true), ] let manager = makeManager() - await manager.removeWallet(walletId: hiddenWalletId) + try await manager.removeWallet(walletId: hiddenWalletId, keepBackupData: false) await manager.drainPendingPersists() XCTAssertEqual(session.forgottenWalletIds, [hiddenWalletId]) diff --git a/BitkitTests/HwWalletManagerTests.swift b/BitkitTests/HwWalletManagerTests.swift index 263703fc4..d792e9ac0 100644 --- a/BitkitTests/HwWalletManagerTests.swift +++ b/BitkitTests/HwWalletManagerTests.swift @@ -847,7 +847,7 @@ final class HwWalletManagerTests: XCTestCase { vm.updateDevices(knownDevices: devices, connectedDeviceId: nil) let wallet = try XCTUnwrap(vm.wallets.first) - await vm.removeWallet(walletId: wallet.id) + try await vm.removeWallet(walletId: wallet.id, keepBackupData: false) await vm.drainPendingPersists() XCTAssertEqual(deleted, try [HwWalletId.derive(xpubs: xpubs)]) From 289d2e165736faabd80cd6aeeea04ebe80ce9597 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 14:15:29 -0300 Subject: [PATCH 5/7] feat: UI handling --- Bitkit/Components/RemoveHwWalletDialog.swift | 111 ++++++++++++++++++ .../Localization/en.lproj/Localizable.strings | 3 + .../HardwareWalletsSettingsScreen.swift | 38 +++--- .../Views/Wallets/HardwareWalletScreen.swift | 29 +++-- 4 files changed, 154 insertions(+), 27 deletions(-) create mode 100644 Bitkit/Components/RemoveHwWalletDialog.swift diff --git a/Bitkit/Components/RemoveHwWalletDialog.swift b/Bitkit/Components/RemoveHwWalletDialog.swift new file mode 100644 index 000000000..b8ff6bf62 --- /dev/null +++ b/Bitkit/Components/RemoveHwWalletDialog.swift @@ -0,0 +1,111 @@ +import SwiftUI + +/// Confirms removing a paired hardware wallet, offering to carry its name and tags in the backup so +/// re-pairing the device restores them. Shared by the wallet screen and the hardware wallet settings. +/// +/// A card rather than a native `.alert`, which cannot hold the switch. Ports bitkit-android's +/// `RemoveHwWalletDialog`. +struct RemoveHwWalletDialog: View { + let walletName: String + @Binding var keepBackupData: Bool + let onConfirm: () -> Void + let onDismiss: () -> Void + + var body: some View { + ZStack { + Color.black.opacity(0.6) + .ignoresSafeArea() + .onTapGesture(perform: onDismiss) + .accessibilityHidden(true) + + VStack(alignment: .leading, spacing: 0) { + SubtitleText(t("hardware__remove_dialog_title", variables: ["name": walletName])) + .padding(.bottom, 8) + + BodyMText(t("hardware__remove_dialog_text")) + .padding(.bottom, 16) + + keepBackupDataRow + .padding(.bottom, 24) + + HStack(spacing: 16) { + CustomButton(title: t("common__dialog_cancel"), variant: .secondary, shouldExpand: true) { + onDismiss() + } + .accessibilityIdentifier("DialogCancel") + + CustomButton(title: t("common__remove"), shouldExpand: true) { + onConfirm() + } + .accessibilityIdentifier("DialogConfirm") + } + } + .padding(24) + .background(Color.gray6) + .clipShape(RoundedRectangle(cornerRadius: 16)) + .padding(.horizontal, 32) + .frame(maxWidth: 400) + } + .accessibilityElement(children: .contain) + .accessibilityIdentifier("RemoveHwWalletDialog") + } + + private var keepBackupDataRow: some View { + HStack(spacing: 16) { + BodyMSBText(t("hardware__remove_dialog_keep")) + .frame(maxWidth: .infinity, alignment: .leading) + + Toggle("", isOn: $keepBackupData) + .toggleStyle(SwitchToggleStyle(tint: .brandAccent)) + .labelsHidden() + .accessibilityLabel(t("hardware__remove_dialog_keep")) + .accessibilityIdentifier("HwRemoveKeepBackupToggle") + } + .contentShape(Rectangle()) + .onTapGesture { keepBackupData.toggle() } + } +} + +extension RemoveHwWalletDialog { + /// Message for a failed removal. An unreadable tag read leaves the wallet untouched, so it names + /// the way through instead of asking for a retry that would repeat the same failure. + static func errorDescription(for error: Error) -> String { + if let removalError = error as? HwWalletRemovalError, removalError == .backupDataUnreadable { + return t("hardware__remove_keep_error") + } + return t("hardware__remove_error") + } +} + +extension View { + /// Overlays `RemoveHwWalletDialog` while `walletName` is non-nil. Nil dismisses it, so the caller + /// keeps the wallet being removed in one piece of state rather than two that can disagree. + func removeHwWalletDialog( + walletName: String?, + keepBackupData: Binding, + onConfirm: @escaping () -> Void, + onDismiss: @escaping () -> Void + ) -> some View { + overlay { + if let walletName { + RemoveHwWalletDialog( + walletName: walletName, + keepBackupData: keepBackupData, + onConfirm: onConfirm, + onDismiss: onDismiss + ) + } + } + } +} + +#Preview { + Color.black + .removeHwWalletDialog( + walletName: "Trezor Safe 3", + keepBackupData: .constant(true), + onConfirm: {}, + onDismiss: {} + ) + .preferredColorScheme(.dark) +} diff --git a/Bitkit/Resources/Localization/en.lproj/Localizable.strings b/Bitkit/Resources/Localization/en.lproj/Localizable.strings index b31175674..5f743f207 100644 --- a/Bitkit/Resources/Localization/en.lproj/Localizable.strings +++ b/Bitkit/Resources/Localization/en.lproj/Localizable.strings @@ -77,6 +77,9 @@ "hardware__remove_button" = "Remove {name}"; "hardware__remove_dialog_title" = "Remove {name}"; "hardware__remove_dialog_text" = "Don't worry, your funds are safe and your coins won't be deleted. Bitkit will simply stop displaying the amounts in the wallet."; +"hardware__remove_dialog_keep" = "Keep name and tags in backup"; +"hardware__remove_error" = "Could not remove the hardware wallet. Please try again."; +"hardware__remove_keep_error" = "Could not keep this wallet's tags in your backup. Try again, or remove it without keeping them."; "cards__buyBitcoin__title" = "Buy"; "cards__buyBitcoin__description" = "Buy some bitcoin"; "cards__btFailed__title" = "Failed"; diff --git a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift index 0328e91a7..06c453714 100644 --- a/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift +++ b/Bitkit/Views/Settings/General/HardwareWalletsSettingsScreen.swift @@ -7,9 +7,11 @@ import SwiftUI struct HardwareWalletsSettingsScreen: View { @Environment(HwWalletManager.self) private var hwWalletManager @Environment(TrezorManager.self) private var trezorManager + @EnvironmentObject private var app: AppViewModel @EnvironmentObject private var sheets: SheetViewModel @State private var pendingRemoval: HwWallet? + @State private var keepBackupDataOnRemoval = true private var wallets: [HwWallet] { hwWalletManager.wallets @@ -39,21 +41,15 @@ struct HardwareWalletsSettingsScreen: View { .navigationBarHidden(true) .accessibilityElement(children: .contain) .accessibilityIdentifier("HardwareWalletsScreen") - .alert( - t("hardware__remove_dialog_title", variables: ["name": pendingRemoval?.name ?? ""]), - isPresented: Binding(get: { pendingRemoval != nil }, set: { if !$0 { pendingRemoval = nil } }) - ) { - Button(t("common__remove"), role: .destructive) { + .removeHwWalletDialog( + walletName: pendingRemoval?.name, + keepBackupData: $keepBackupDataOnRemoval, + onConfirm: { guard let wallet = pendingRemoval else { return } Task { await remove(wallet) } - } - .accessibilityIdentifier("DialogConfirm") - - Button(t("common__dialog_cancel"), role: .cancel) {} - .accessibilityIdentifier("DialogCancel") - } message: { - Text(t("hardware__remove_dialog_text")) - } + }, + onDismiss: { pendingRemoval = nil } + ) } private var emptyState: some View { @@ -83,7 +79,12 @@ struct HardwareWalletsSettingsScreen: View { data: RenameHardwareWalletConfig(walletId: wallet.id, currentName: wallet.name) ) }, - onRemove: { pendingRemoval = wallet } + // Reset on open rather than on dismiss, so a cancel, a failed removal or + // another wallet picked from the list all start from the default. + onRemove: { + keepBackupDataOnRemoval = true + pendingRemoval = wallet + } ) CustomDivider() } @@ -101,8 +102,13 @@ struct HardwareWalletsSettingsScreen: View { } private func remove(_ wallet: HwWallet) async { + let keepBackupData = keepBackupDataOnRemoval pendingRemoval = nil - try? await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: true) + do { + try await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: keepBackupData) + } catch { + app.toast(type: .error, title: t("common__error"), description: RemoveHwWalletDialog.errorDescription(for: error)) + } } } @@ -172,6 +178,7 @@ private struct HwConnectionBadge: View { HardwareWalletsSettingsScreen() .environment(HwWalletManager()) .environment(TrezorManager()) + .environmentObject(AppViewModel()) .environmentObject(SheetViewModel()) .environmentObject(NavigationViewModel()) .environmentObject(CurrencyViewModel()) @@ -185,6 +192,7 @@ private struct HwConnectionBadge: View { HardwareWalletsSettingsScreen() .environment(HwWalletManager()) .environment(TrezorManager()) + .environmentObject(AppViewModel()) .environmentObject(SheetViewModel()) .environmentObject(NavigationViewModel()) .environmentObject(CurrencyViewModel()) diff --git a/Bitkit/Views/Wallets/HardwareWalletScreen.swift b/Bitkit/Views/Wallets/HardwareWalletScreen.swift index 17a23dcaa..987315b96 100644 --- a/Bitkit/Views/Wallets/HardwareWalletScreen.swift +++ b/Bitkit/Views/Wallets/HardwareWalletScreen.swift @@ -16,6 +16,7 @@ struct HardwareWalletScreen: View { @State private var activities: [Activity] = [] @State private var showRemoveDialog = false + @State private var keepBackupDataOnRemoval = true private var wallet: HwWallet? { hwWalletManager.wallets.first { $0.id == walletId } @@ -52,17 +53,12 @@ struct HardwareWalletScreen: View { navigation.navigateBack() } } - .alert( - t("hardware__remove_dialog_title", variables: ["name": wallet?.name ?? ""]), - isPresented: $showRemoveDialog - ) { - Button(t("common__remove"), role: .destructive) { - Task { await removeWallet() } - } - Button(t("common__dialog_cancel"), role: .cancel) {} - } message: { - Text(t("hardware__remove_dialog_text")) - } + .removeHwWalletDialog( + walletName: showRemoveDialog ? wallet?.name : nil, + keepBackupData: $keepBackupDataOnRemoval, + onConfirm: { Task { await removeWallet() } }, + onDismiss: { showRemoveDialog = false } + ) } private func content(for wallet: HwWallet) -> some View { @@ -146,6 +142,9 @@ struct HardwareWalletScreen: View { title: t("hardware__remove_button", variables: ["name": wallet.name]), variant: .tertiary ) { + // Reset on open rather than on dismiss, so a cancel or a failed removal starts from the + // default rather than from the last choice. + keepBackupDataOnRemoval = true showRemoveDialog = true } .accessibilityIdentifier("RemoveHardwareWallet") @@ -180,7 +179,13 @@ struct HardwareWalletScreen: View { /// tile, and the reactive auto-pop above then leaves the screen. private func removeWallet() async { guard let wallet else { return } - try? await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: true) + let keepBackupData = keepBackupDataOnRemoval + showRemoveDialog = false + do { + try await hwWalletManager.removeWallet(walletId: wallet.id, keepBackupData: keepBackupData) + } catch { + app.toast(type: .error, title: t("common__error"), description: RemoveHwWalletDialog.errorDescription(for: error)) + } } } From a1e03472fd04468377004d28da3955580e368da9 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 14:51:53 -0300 Subject: [PATCH 6/7] test: HW name persistense tests --- Bitkit/Managers/HwWalletManager.swift | 13 +- .../HwWalletManagerPassphraseTests.swift | 7 + BitkitTests/HwWalletManagerTests.swift | 209 +++++++++++++++++- BitkitTests/PubkyProfileManagerTests.swift | 37 ++++ .../TrezorKnownDeviceStorageTests.swift | 109 ++++++++- changelog.d/next/680.added.md | 1 + 6 files changed, 367 insertions(+), 9 deletions(-) create mode 100644 changelog.d/next/680.added.md diff --git a/Bitkit/Managers/HwWalletManager.swift b/Bitkit/Managers/HwWalletManager.swift index ff1b8dcdb..1e6e012f1 100644 --- a/Bitkit/Managers/HwWalletManager.swift +++ b/Bitkit/Managers/HwWalletManager.swift @@ -95,9 +95,13 @@ final class HwWalletManager { /// content is byte-identical — that write is what applies the deletions the partial ones deferred. private var lastPersisted: [String: HwWalletSnapshot] = [:] - /// Tag metadata a removal asked to keep, held until the wallet's activities are gone and it can - /// be written back. Read at the moment each delete runs rather than captured, so a re-pair or a - /// second removal that keeps nothing cancels a repair that has not happened yet. + /// Tag metadata a removal asked to keep, re-applied after every delete of its wallet. Read at the + /// moment each delete runs rather than captured, so a later removal replaces it — or clears it, + /// when that one keeps nothing. + /// + /// Deliberately outlives the removal: a cleanup delete can arrive long afterwards, from a push + /// that re-added the wallet and then dropped it again, and re-applying is what keeps it from + /// taking the rows with it. private var keptBackupMetadata: [String: [PreActivityMetadata]] = [:] private var emittedReceivedTxIds: Set = [] @@ -525,9 +529,6 @@ final class HwWalletManager { let liveSignatures = Set(knownDevices.filter { !$0.xpubs.isEmpty }.map { xpubsSignature($0.xpubs) }) walletIdCache = walletIdCache.filter { liveSignatures.contains($0.key) } lastPersisted = lastPersisted.filter { hwWalletIds.contains($0.key) } - // A wallet that is paired again owns its metadata through its watcher; a pending repair from - // an earlier removal would only re-apply rows core has since re-attached anyway. - keptBackupMetadata = keptBackupMetadata.filter { !hwWalletIds.contains($0.key) } } private func startWatcher(_ spec: WatcherSpec) { diff --git a/BitkitTests/HwWalletManagerPassphraseTests.swift b/BitkitTests/HwWalletManagerPassphraseTests.swift index 81042ce08..7d126355c 100644 --- a/BitkitTests/HwWalletManagerPassphraseTests.swift +++ b/BitkitTests/HwWalletManagerPassphraseTests.swift @@ -337,6 +337,13 @@ final class HwWalletManagerPassphraseTests: XCTestCase { 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") + // The stray is a real wallet the user owns, and reading its accounts already consumed any + // name restored for it into the entry being forgotten, so the name has to go back. + XCTAssertEqual( + session.forgottenPendingNames.compactMap { $0 }.map(\.walletId), + [strayWalletId], + "its backup data is kept" + ) } /// An account read that failed says nothing about which wallet the session holds, so calling it a diff --git a/BitkitTests/HwWalletManagerTests.swift b/BitkitTests/HwWalletManagerTests.swift index d792e9ac0..51aa5fb19 100644 --- a/BitkitTests/HwWalletManagerTests.swift +++ b/BitkitTests/HwWalletManagerTests.swift @@ -77,6 +77,20 @@ final class HwWalletManagerTests: XCTestCase { private var receivedTxs: [HwWalletReceivedTx] = [] private var cancellables: Set = [] + /// The metadata a removal asked to keep, per wallet id, and whether reading it should fail. + private var tagMetadataByWallet: [String: [PreActivityMetadata]] = [:] + private var tagMetadataReadError: Error? + private var tagMetadataWriteError: Error? + + /// Deletes and metadata write-backs in the order core saw them, so ordering is assertable and + /// not just the call sets. + private enum CoreOp: Equatable { + case delete(String) + case upsert([String]) + } + + private var coreOps: [CoreOp] = [] + /// The activity sets handed to core, oldest first — most assertions only care about these. private var persisted: [[Activity]] { persistedSnapshots.map(\.activities) @@ -89,6 +103,10 @@ final class HwWalletManagerTests: XCTestCase { receivedTxs = [] cancellables = [] xpubsByDeviceId = [:] + tagMetadataByWallet = [:] + tagMetadataReadError = nil + tagMetadataWriteError = nil + coreOps = [] } // MARK: - Factories @@ -103,7 +121,19 @@ final class HwWalletManagerTests: XCTestCase { electrumUrl: { "ssl://test:1" }, network: { .regtest }, persistSnapshot: { [weak self] in self?.persistedSnapshots.append($0) }, - deleteActivities: { [weak self] in self?.deleted.append($0) } + deleteActivities: { [weak self] walletId in + self?.deleted.append(walletId) + self?.coreOps.append(.delete(walletId)) + }, + readTagMetadata: { [weak self] walletId in + guard let self else { return [] } + if let tagMetadataReadError { throw tagMetadataReadError } + return tagMetadataByWallet[walletId] ?? [] + }, + writeTagMetadata: { [weak self] records in + self?.coreOps.append(.upsert(records.map(\.paymentId))) + if let error = self?.tagMetadataWriteError { throw error } + } ) vm.receivedTxPublisher .sink { [weak self] in self?.receivedTxs.append($0) } @@ -1147,8 +1177,179 @@ final class HwWalletManagerTests: XCTestCase { XCTAssertEqual(order.first, "B1") } + // MARK: - Removal keeping backup data + + /// Core drops a wallet's pre-activity metadata along with its activities, so the rows a removal + /// kept can only go back once the delete has run. + func testRemovingAWalletKeepingItsDataWritesTheTagsBackAfterTheDelete() async throws { + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + tagMetadataByWallet[walletId] = [makeTagMetadata(walletId: walletId, paymentId: "p1")] + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: true) + await vm.drainPendingPersists() + + XCTAssertEqual(coreOps, [.delete(walletId), .upsert(["p1"])]) + } + + /// The push that follows `forgetWallet` used to delete the wallet's activities a second time, + /// taking the kept rows with them. The delete now carries its own repair, so even if one runs it + /// leaves the metadata behind. + func testACleanupDeleteAfterRemovalReappliesTheKeptTags() async throws { + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + let device = makeDevice(id: "dev1", xpubs: xpubs) + tagMetadataByWallet[walletId] = [makeTagMetadata(walletId: walletId, paymentId: "p1")] + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: true) + // A stale push still holding the wallet, then one without it — the window `forgetWallet` + // opens while it awaits `clearCredentials`. + vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) + vm.updateDevices(knownDevices: [], connectedDeviceId: nil) + await vm.drainPendingPersists() + + XCTAssertEqual(coreOps.last, .upsert(["p1"]), "the last thing core saw must be the kept rows") + for (index, op) in coreOps.enumerated() where op == .delete(walletId) { + XCTAssertEqual(coreOps[safeIndex: index + 1], .upsert(["p1"]), "every delete repairs itself") + } + } + + /// The wallet leaves the tile list inside `removeDevice`, so the push that follows has nothing + /// left to clean up. + func testRemovingAWalletDoesNotDeleteItsActivitiesAgainOnTheNextPush() async throws { + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: false) + vm.updateDevices(knownDevices: [], connectedDeviceId: nil) + await vm.drainPendingPersists() + + XCTAssertEqual(deleted, [walletId]) + XCTAssertTrue(vm.wallets.isEmpty) + } + + func testRemovingAWalletWithoutKeepingItsDataReadsAndWritesNothing() async throws { + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + tagMetadataByWallet[walletId] = [makeTagMetadata(walletId: walletId, paymentId: "p1")] + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: false) + await vm.drainPendingPersists() + + XCTAssertEqual(coreOps, [.delete(walletId)]) + } + + /// Raised before anything is deleted, so the wallet has to come through it untouched. + func testAnUnreadableTagSnapshotRefusesTheRemovalAndTouchesNothing() async throws { + struct ReadFailure: Error {} + let mock = MockWatcherService() + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + tagMetadataReadError = ReadFailure() + let vm = makeViewModel(watcherService: mock, monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) + await waitUntil { mock.startedParams.count == 1 } + + do { + try await vm.removeWallet(walletId: walletId, keepBackupData: true) + XCTFail("Expected the removal to be refused") + } catch { + XCTAssertEqual(error as? HwWalletRemovalError, .backupDataUnreadable) + } + await vm.drainPendingPersists() + + XCTAssertTrue(coreOps.isEmpty, "nothing was deleted") + XCTAssertTrue(mock.stoppedWatcherIds.isEmpty, "the wallet is still watched") + XCTAssertEqual(vm.wallets.map(\.id), [walletId], "the wallet is still paired") + } + + /// The activities are already gone by then, so there is nothing to roll back to and reporting a + /// failed removal would be false. + func testAFailedTagWriteBackStillCompletesTheRemoval() async throws { + struct WriteFailure: Error {} + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + tagMetadataByWallet[walletId] = [makeTagMetadata(walletId: walletId, paymentId: "p1")] + tagMetadataWriteError = WriteFailure() + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [makeDevice(id: "dev1", xpubs: xpubs)], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: true) + await vm.drainPendingPersists() + + XCTAssertEqual(deleted, [walletId]) + XCTAssertTrue(vm.wallets.isEmpty) + } + + /// A later removal that keeps nothing is what disarms the repair — otherwise the rows the user + /// asked to drop would come back. + func testRemovingAWalletAgainWithoutKeepingItsDataDropsTheEarlierRepair() async throws { + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + let device = makeDevice(id: "dev1", xpubs: xpubs) + tagMetadataByWallet[walletId] = [makeTagMetadata(walletId: walletId, paymentId: "p1")] + let vm = makeViewModel(monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) + + try await vm.removeWallet(walletId: walletId, keepBackupData: true) + await vm.drainPendingPersists() + vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) + coreOps = [] + + try await vm.removeWallet(walletId: walletId, keepBackupData: false) + await vm.drainPendingPersists() + + XCTAssertEqual(coreOps, [.delete(walletId)]) + } + + /// `deviceGroups()` used to keep yielding the removed group until the next push, so an event + /// still in flight re-persisted the activities the removal had just deleted. + func testAWatcherEventAfterRemovalDoesNotRePersistTheWallet() async throws { + let mock = MockWatcherService() + let xpubs = ["nativeSegwit": "z"] + let walletId = try HwWalletId.derive(xpubs: xpubs) + let device = makeDevice(id: "dev1", xpubs: xpubs) + let vm = makeViewModel(watcherService: mock, monitored: ["nativeSegwit"]) + vm.updateDevices(knownDevices: [device], connectedDeviceId: nil) + await waitUntil { mock.startedParams.count == 1 } + + try await vm.removeWallet(walletId: walletId, keepBackupData: false) + persistedSnapshots = [] + vm.handleWatcherEvent( + watcherId: watcherId(device, "nativeSegwit"), + event: makeEvent([makeActivity(txId: "t1", value: 1000, txType: .received)], total: 1000) + ) + await vm.drainPendingPersists() + + XCTAssertTrue(persistedSnapshots.isEmpty) + } + // MARK: - Helpers + private func makeTagMetadata(walletId: String, paymentId: String) -> PreActivityMetadata { + PreActivityMetadata( + walletId: walletId, + paymentId: paymentId, + tags: ["coffee"], + paymentHash: nil, + txId: paymentId, + address: nil, + isReceive: false, + feeRate: 0, + isTransfer: false, + channelId: nil, + createdAt: 1_700_000_000_000 + ) + } + private func waitUntil(timeout: TimeInterval = 2, _ condition: () -> Bool) async { let deadline = Date().addingTimeInterval(timeout) while !condition(), Date() < deadline { @@ -1156,3 +1357,9 @@ final class HwWalletManagerTests: XCTestCase { } } } + +private extension Array { + subscript(safeIndex index: Int) -> Element? { + indices.contains(index) ? self[index] : nil + } +} diff --git a/BitkitTests/PubkyProfileManagerTests.swift b/BitkitTests/PubkyProfileManagerTests.swift index 45b39210c..43be620ac 100644 --- a/BitkitTests/PubkyProfileManagerTests.swift +++ b/BitkitTests/PubkyProfileManagerTests.swift @@ -595,6 +595,43 @@ final class PubkyProfileManagerTests: XCTestCase { XCTAssertEqual(decoded.cache.dismissedSuggestions, []) } + func testMetadataBackupV1RoundTripsHwWalletNames() throws { + let payload = MetadataBackupV1( + version: 1, + createdAt: 123, + tagMetadata: [], + cache: makeAppCacheData(), + pubkySession: nil, + pubkyContactProfileOverrides: nil, + hwWalletNames: ["trezor:standard": "Cold Storage"] + ) + + let encoded = try JSONEncoder().encode(payload) + let decoded = try JSONDecoder().decode(MetadataBackupV1.self, from: encoded) + + XCTAssertEqual(decoded.hwWalletNames, ["trezor:standard": "Cold Storage"]) + } + + /// The envelope is shared with bitkit-android, which wrote it without this field before it + /// existed — and still omits it when no wallet is named. + func testMetadataBackupV1DecodesWithoutHwWalletNamesField() throws { + let payload = MetadataBackupV1( + version: 1, + createdAt: 123, + tagMetadata: [], + cache: makeAppCacheData(), + pubkySession: nil, + pubkyContactProfileOverrides: nil + ) + + let encoded = try JSONEncoder().encode(payload) + let json = try XCTUnwrap(JSONSerialization.jsonObject(with: encoded) as? [String: Any]) + XCTAssertNil(json["hwWalletNames"], "a nil map must not be written as an explicit null") + + let decoded = try JSONDecoder().decode(MetadataBackupV1.self, from: encoded) + XCTAssertNil(decoded.hwWalletNames) + } + // MARK: - Profile Link Input Model func testProfileLinkInputHasUniqueIds() { diff --git a/BitkitTests/TrezorKnownDeviceStorageTests.swift b/BitkitTests/TrezorKnownDeviceStorageTests.swift index 70565e799..9e0f1eb00 100644 --- a/BitkitTests/TrezorKnownDeviceStorageTests.swift +++ b/BitkitTests/TrezorKnownDeviceStorageTests.swift @@ -1,23 +1,32 @@ @testable import Bitkit +import Combine 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 static let pendingNamesKey = "trezor.pendingWalletNames" private var savedDefaults: Data? + private var savedPendingNames: [String: String]? + private var cancellables: Set = [] override func setUp() { super.setUp() savedDefaults = UserDefaults.standard.data(forKey: Self.storageKey) + savedPendingNames = UserDefaults.standard.dictionary(forKey: Self.pendingNamesKey) as? [String: String] + cancellables = [] TrezorKnownDeviceStorage.removeAll() } override func tearDown() { + cancellables = [] + TrezorKnownDeviceStorage.removeAll() if let savedDefaults { UserDefaults.standard.set(savedDefaults, forKey: Self.storageKey) - } else { - TrezorKnownDeviceStorage.removeAll() + } + if let savedPendingNames { + UserDefaults.standard.set(savedPendingNames, forKey: Self.pendingNamesKey) } super.tearDown() } @@ -96,6 +105,102 @@ final class TrezorKnownDeviceStorageTests: XCTestCase { XCTAssertEqual(stored?.trezorDeviceId, "trezor-id") } + // MARK: - Hardware wallet names + + func testAPendingNameAndTheDeviceListAreWrittenTogether() { + let device = makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Cold", walletId: "trezor:standard") + TrezorKnownDeviceStorage.save(device) + + TrezorKnownDeviceStorage.saveAll([], pendingName: PendingHwWalletName(walletId: "trezor:standard", name: "Cold")) + + XCTAssertTrue(TrezorKnownDeviceStorage.loadAll().isEmpty) + XCTAssertEqual(TrezorKnownDeviceStorage.loadPendingNames(), ["trezor:standard": "Cold"]) + } + + /// Adoption on pairing consumes a pending name by masking rather than by a second write, so a + /// wallet the device list already names must not report one. + func testAPendingNameIsMaskedOnceTheWalletIsPairedAndNamed() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: "Cold") + TrezorKnownDeviceStorage.save( + makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Cold", walletId: "trezor:standard") + ) + + XCTAssertTrue(TrezorKnownDeviceStorage.loadPendingNames().isEmpty) + XCTAssertEqual(TrezorKnownDeviceStorage.backupSnapshot(), ["trezor:standard": "Cold"]) + } + + func testTheNameOfAPairedWalletWinsOverAPendingOne() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: "Restored") + TrezorKnownDeviceStorage.save( + makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Renamed", walletId: "trezor:standard") + ) + + XCTAssertEqual(TrezorKnownDeviceStorage.backupSnapshot(), ["trezor:standard": "Renamed"]) + } + + func testSettingAPendingNameToNilDropsIt() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: "Cold") + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: nil) + + XCTAssertTrue(TrezorKnownDeviceStorage.backupSnapshot().isEmpty) + } + + func testRestoringNamesLetsALocalNameWinAndNeverClears() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: "Local") + + TrezorKnownDeviceStorage.restoreNames(["trezor:standard": "Backed up", "trezor:hidden": "Hidden"]) + XCTAssertEqual( + TrezorKnownDeviceStorage.backupSnapshot(), + ["trezor:standard": "Local", "trezor:hidden": "Hidden"] + ) + + // An envelope written before the field carries no names, and must not drop what is stored. + TrezorKnownDeviceStorage.restoreNames([:]) + XCTAssertEqual( + TrezorKnownDeviceStorage.backupSnapshot(), + ["trezor:standard": "Local", "trezor:hidden": "Hidden"] + ) + } + + func testForgettingAWalletDropsTheNameKeptForIt() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:hidden", name: "Hidden") + TrezorKnownDeviceStorage.save(makeDevice(xpubs: ["nativeSegwit": "zHidden"], walletId: "trezor:hidden")) + + TrezorKnownDeviceStorage.remove(walletId: "trezor:hidden") + + XCTAssertTrue(TrezorKnownDeviceStorage.backupSnapshot().isEmpty) + } + + func testRemoveAllClearsPendingNamesToo() { + TrezorKnownDeviceStorage.setPendingName(walletId: "trezor:standard", name: "Cold") + + TrezorKnownDeviceStorage.removeAll() + + XCTAssertTrue(TrezorKnownDeviceStorage.backupSnapshot().isEmpty) + } + + /// Every connect rewrites the device list to refresh `lastConnectedAt`; only a name change may + /// mark the metadata backup stale. + func testTheNameSignalFiresOnARenameButNotOnAReconnect() { + let device = makeDevice(xpubs: ["nativeSegwit": "zStandard"], customLabel: "Cold", walletId: "trezor:standard") + TrezorKnownDeviceStorage.save(device) + + var fires = 0 + TrezorKnownDeviceStorage.namesChangedPublisher + .sink { fires += 1 } + .store(in: &cancellables) + + var reconnected = device + reconnected.lastConnectedAt = Date(timeIntervalSince1970: 5000) + TrezorKnownDeviceStorage.saveAll([reconnected]) + XCTAssertEqual(fires, 0, "a reconnect must not re-upload the metadata envelope") + + var renamed = reconnected + renamed.customLabel = "Vault" + TrezorKnownDeviceStorage.saveAll([renamed]) + XCTAssertEqual(fires, 1) + } + private func makeDevice( id: String = "dev1", xpubs: [String: String], diff --git a/changelog.d/next/680.added.md b/changelog.d/next/680.added.md new file mode 100644 index 000000000..451e94270 --- /dev/null +++ b/changelog.d/next/680.added.md @@ -0,0 +1 @@ +The name you give a hardware wallet is now included in your backup and comes back when you pair the device again, and removing a hardware wallet asks first whether to keep its name and tags in your backup. From 4ec04f925ae7a2d0774bdf0269dc4252f4efa7a4 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 14:58:59 -0300 Subject: [PATCH 7/7] chore: rename changelog --- changelog.d/next/{680.added.md => 681.added.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{680.added.md => 681.added.md} (100%) diff --git a/changelog.d/next/680.added.md b/changelog.d/next/681.added.md similarity index 100% rename from changelog.d/next/680.added.md rename to changelog.d/next/681.added.md