From 0adb8b279ae56344cbc587e5a9a9c279a5835c83 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 10:49:13 -0300 Subject: [PATCH 01/17] feat: data layer fo handling pending names --- .../main/java/to/bitkit/data/HwWalletStore.kt | 53 +++++++++++++++++++ .../java/to/bitkit/models/BackupPayloads.kt | 2 + 2 files changed, 55 insertions(+) diff --git a/app/src/main/java/to/bitkit/data/HwWalletStore.kt b/app/src/main/java/to/bitkit/data/HwWalletStore.kt index 08cd6ab191..01c81a1d21 100644 --- a/app/src/main/java/to/bitkit/data/HwWalletStore.kt +++ b/app/src/main/java/to/bitkit/data/HwWalletStore.kt @@ -38,6 +38,39 @@ class HwWalletStore @Inject constructor( Unit } + suspend fun loadPendingNames(): Map = withContext(ioDispatcher) { + store.data.first().pendingNames + } + + /** Stores the name of a wallet with no device entry, or drops it when [name] is null. */ + suspend fun setPendingName(walletId: String, name: String?) = withContext(ioDispatcher) { + if (walletId.isBlank()) return@withContext + store.updateData { data -> + val pendingNames = when { + name.isNullOrBlank() -> data.pendingNames - walletId + else -> data.pendingNames + (walletId to name) + } + data.copy(pendingNames = pendingNames) + } + Unit + } + + suspend fun backupSnapshot(): Map = withContext(ioDispatcher) { + store.data.first().hwWalletNames() + } + + /** + * Merges backed up names into the pending ones, so they are adopted the next time each 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 this field, and must not drop what is stored. + */ + suspend fun restoreNames(names: Map) = withContext(ioDispatcher) { + if (names.isEmpty()) return@withContext + store.updateData { it.copy(pendingNames = names + it.pendingNames) } + Unit + } + suspend fun reset() = withContext(ioDispatcher) { store.updateData { HwWalletData() } Unit @@ -47,4 +80,24 @@ class HwWalletStore @Inject constructor( @Serializable data class HwWalletData( val knownDevices: List = emptyList(), + /** + * Names of wallets with no [KnownDevice] entry, keyed by wallet id: restored from a backup before + * the device was paired again, or kept when the wallet was removed. Pairing consumes an entry into + * the device's own `customLabel`. + */ + val pendingNames: Map = emptyMap(), ) + +/** + * Every hardware wallet name this wallet knows, keyed by wallet id: the name of each paired wallet, + * overlaid on the pending ones. 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, so it can never have been named. + */ +internal fun HwWalletData.hwWalletNames(): Map = + pendingNames + knownDevices.mapNotNull { device -> + val walletId = device.walletId.takeIf { it.isNotBlank() } ?: return@mapNotNull null + val name = device.customLabel?.takeIf { it.isNotBlank() } ?: return@mapNotNull null + walletId to name + } diff --git a/app/src/main/java/to/bitkit/models/BackupPayloads.kt b/app/src/main/java/to/bitkit/models/BackupPayloads.kt index 04dfeb68bb..d16bdab076 100644 --- a/app/src/main/java/to/bitkit/models/BackupPayloads.kt +++ b/app/src/main/java/to/bitkit/models/BackupPayloads.kt @@ -34,6 +34,8 @@ data class MetadataBackupV1( val cache: AppCacheData, val pubkySession: PubkySessionBackupV1? = null, val pubkyContactProfileOverrides: Map? = null, + /** User-set hardware wallet names, keyed by wallet id. Null in envelopes written before this field. */ + val hwWalletNames: Map? = null, ) @Serializable From dce16694aea182fe19b703a54da2d8ffa5f48e45 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 10:54:22 -0300 Subject: [PATCH 02/17] feat: observe name changes and mark them as backup required --- .../java/to/bitkit/repositories/BackupRepo.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index c0ef32ee19..bd7b55116f 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -31,11 +31,13 @@ import to.bitkit.R import to.bitkit.async.appScope import to.bitkit.data.AppDb import to.bitkit.data.CacheStore +import to.bitkit.data.HwWalletStore import to.bitkit.data.SettingsStore import to.bitkit.data.WatchOnlyAccountStore import to.bitkit.data.WidgetsStore import to.bitkit.data.backup.VssBackupClient import to.bitkit.data.backup.VssBackupClientLdk +import to.bitkit.data.hwWalletNames import to.bitkit.data.resetPin import to.bitkit.di.IoDispatcher import to.bitkit.di.json @@ -91,6 +93,7 @@ class BackupRepo @Inject constructor( private val widgetsStore: WidgetsStore, private val watchOnlyAccountStore: WatchOnlyAccountStore, private val watchOnlyAccountRepo: WatchOnlyAccountRepo, + private val hwWalletStore: HwWalletStore, private val blocktankRepo: BlocktankRepo, private val activityRepo: ActivityRepo, private val pubkyRepo: PubkyRepo, @@ -298,6 +301,20 @@ class BackupRepo @Inject constructor( } dataListenerJobs.add(preActivityMetadataJob) + // METADATA - Observe hardware wallet names only: the store is also rewritten by every connect, + // and reconnect traffic must not re-upload the whole metadata envelope. + val hwWalletNamesJob = scope.launch { + hwWalletStore.data + .map { it.hwWalletNames() } + .distinctUntilChanged() + .drop(1) + .collect { + if (shouldSkipBackup()) return@collect + markBackupRequired(BackupCategory.METADATA) + } + } + dataListenerJobs.add(hwWalletNamesJob) + dataListenerJobs.add(observeBackupChanges(pubkyRepo.backupStateVersion, BackupCategory.METADATA)) dataListenerJobs.add(observeBackupChanges(privatePaykitRepo.get().backupStateVersion, BackupCategory.WALLET)) dataListenerJobs.add(observeBackupChanges(paykitSdkService.backupStateVersion, BackupCategory.WALLET)) @@ -548,6 +565,9 @@ class BackupRepo @Inject constructor( val hardwareTagMetadata = activityRepo.getHardwareTagsAsPreActivityMetadata().getOrThrow() val tagMetadata = (preActivityMetadata + hardwareTagMetadata) .distinctBy { it.walletId to it.paymentId } + // Like the tags above, this envelope is the only copy of the names, so a read failure must + // propagate and fail the backup rather than upload an empty set over the stored ones. + val hwWalletNames = hwWalletStore.backupSnapshot().takeIf { it.isNotEmpty() } val cacheData = cacheStore.data.first() val pubkySession = pubkyRepo.snapshotSessionBackupState().getOrDefault(null) val pubkyContactProfileOverrides = pubkyRepo.snapshotContactProfileOverrides().getOrDefault(null) @@ -558,6 +578,7 @@ class BackupRepo @Inject constructor( cache = cacheData, pubkySession = pubkySession, pubkyContactProfileOverrides = pubkyContactProfileOverrides, + hwWalletNames = hwWalletNames, ) json.encodeToString(payload).toByteArray() @@ -661,7 +682,11 @@ class BackupRepo @Inject constructor( .onFailure { Logger.warn("Failed to restore pubky contact profile overrides", it, context = TAG) } + // 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. + hwWalletStore.restoreNames(parsed.hwWalletNames.orEmpty()) Logger.debug("Restored ${parsed.tagMetadata.size} pre-activity metadata", TAG) + Logger.debug("Restored ${parsed.hwWalletNames.orEmpty().size} hardware wallet names", TAG) return RestoredCoreBackup(createdAt = parsed.createdAt, needsRewrite = migration.changed && persisted) } From c9b38654eb7908c9ee38a76485f93d81e56fd515 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:13:04 -0300 Subject: [PATCH 03/17] feat: hoist resolvedWalletId and consume stored name --- .../java/to/bitkit/repositories/TrezorRepo.kt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt index 2c66ba607a..ab74dd3cd7 100644 --- a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt @@ -1141,6 +1141,15 @@ class TrezorRepo @Inject constructor( // must keep the name the user gave it instead of falling back to the device's own. val identityKey = walletKey(xpubs, deviceInfo.id) val named = previous ?: knownDevices.firstOrNull { it.walletKey == identityKey } + val resolvedWalletId = previous?.walletId?.takeIf { it.isNotBlank() } + ?: knownDevices.findHardwareWalletId(xpubs, fallback = deviceInfo.id) + // A name restored from a backup, or kept when this wallet was removed, belongs to the wallet + // identity rather than to any device entry, so adopt it the first time the identity is paired + // again. An entry that already carries one was named on this device more recently. + val pendingName = resolvedWalletId.takeIf { it.isNotBlank() } + ?.let { hwWalletStore.loadPendingNames()[it] } + ?.takeIf { it.isNotBlank() } + val customLabel = named?.customLabel ?: pendingName val known = KnownDevice( id = deviceInfo.id, name = deviceInfo.name, @@ -1150,9 +1159,8 @@ class TrezorRepo @Inject constructor( model = features.model ?: deviceInfo.model, lastConnectedAt = clock.nowMs(), xpubs = xpubs, - customLabel = named?.customLabel, - walletId = previous?.walletId?.takeIf { it.isNotBlank() } - ?: knownDevices.findHardwareWalletId(xpubs, fallback = deviceInfo.id), + customLabel = customLabel, + walletId = resolvedWalletId, // 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 @@ -1166,6 +1174,11 @@ class TrezorRepo @Inject constructor( ) val updated = knownDevices.filterNot { it.isReplacedBy(known, refreshed = previous) } + known saveKnownDevices(updated) + // Consumed, so the name lives on the entry alone: leaving it would resurrect a name the user + // later clears, since the entry would then fall back to the pending one again. + if (pendingName != null) { + hwWalletStore.setPendingName(resolvedWalletId, null) + } _state.update { it.copy(knownDevices = updated.toImmutableList()) } return known } From 8c83ead9f892e3c1a6ddec0ae1785e3bdd71b566 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:17:29 -0300 Subject: [PATCH 04/17] feat: update remove device flow for adding keep device data as metadata support --- .../to/bitkit/repositories/ActivityRepo.kt | 21 ++++++++++++++ .../to/bitkit/repositories/HwWalletRepo.kt | 28 ++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt index 6385f5c1d8..f5460b37a5 100644 --- a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt @@ -901,6 +901,27 @@ class ActivityRepo @Inject constructor( } } + /** + * 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 the stored [PreActivityMetadata] of a wallet along with its + * activities, and those rows are not covered by [getHardwareTagsAsPreActivityMetadata], which only + * renders tags that already reached an activity. + */ + suspend fun getTagMetadataForWallet(walletId: String): Result> = + withContext(bgDispatcher) { + runSuspendCatching { + val stored = coreService.activity.getAllPreActivityMetadata() + .filter { it.walletId == walletId } + val rendered = getHardwareTagsAsPreActivityMetadata().getOrThrow() + .filter { it.walletId == walletId } + (stored + rendered).distinctBy { it.walletId to it.paymentId } + }.onFailure { + Logger.error("Failed to read tag metadata of '$walletId'", it, context = TAG) + } + } + /** * Fill in wallet ids missing from a backup envelope's `activities` slice, letting Core migrate its own * model JSON before the app decodes it. diff --git a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt index 4e368f709e..0305f9e05b 100644 --- a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt @@ -77,6 +77,7 @@ import kotlin.time.Duration.Companion.seconds class HwWalletRepo @Inject constructor( private val trezorRepo: TrezorRepo, private val activityRepo: ActivityRepo, + private val preActivityMetadataRepo: PreActivityMetadataRepo, private val hwWalletStore: HwWalletStore, private val settingsStore: SettingsStore, @IoDispatcher private val ioDispatcher: CoroutineDispatcher, @@ -444,8 +445,15 @@ class HwWalletRepo @Inject constructor( * is stored once per transport but shares an xpub-derived identity, so forgetting a single id * would leave the tile reappearing through the other transport. Other identities on the same * device — the standard wallet, or another passphrase wallet — are left paired. + * + * @param keepBackupData whether to carry the wallet's name and tags in the backup, so re-pairing + * the device restores them. Off by default: only a user removing a wallet is asked, and internal + * cleanup of a wallet the user never meant to watch must not leave its data behind. */ - suspend fun removeDevice(walletId: String): Result = withContext(ioDispatcher) { + suspend fun removeDevice( + walletId: String, + keepBackupData: Boolean = false, + ): Result = withContext(ioDispatcher) { runSuspendCatching { watcherMutex.withLock { val knownDevices = hwWalletStore.loadKnownDevices() @@ -453,6 +461,13 @@ class HwWalletRepo @Inject constructor( // Without an entry there is nothing to forget, and the check below would pass on an // empty set: report the failure instead of telling the user the wallet was removed. require(targets.isNotEmpty()) { "Unknown hardware wallet '$walletId'" } + // Read before the deletion below, which takes the tags with the activities they are on. + val keptName = targets.firstNotNullOfOrNull { it.customLabel?.takeIf(String::isNotBlank) } + .takeIf { keepBackupData } + val keptTagMetadata = when { + keepBackupData -> activityRepo.getTagMetadataForWallet(walletId).getOrThrow() + else -> emptyList() + } activeWatchers.toList() .filter { it.toWalletId() == walletId } .forEach { @@ -461,6 +476,17 @@ class HwWalletRepo @Inject constructor( } } activityRepo.deleteForWallet(walletId).getOrThrow() + // Written back only now: the deletion above drops the wallet's stored tag metadata + // along with its activities. Core re-attaches these once the watcher recreates them, + // so re-pairing the device brings the tags back. + if (keptTagMetadata.isNotEmpty()) { + // Nothing to roll back to at this point, and the wallet is already half removed, + // so a failure here loses the tags rather than failing the removal. + preActivityMetadataRepo.upsertPreActivityMetadata(keptTagMetadata) + } + // Before forgetting the entries below, so the name is never absent from the backup: + // dropping the entries takes their label with them. + hwWalletStore.setPendingName(walletId, keptName) trackedWalletIds -= walletId lastPersistedHwSnapshots -= walletId val failures = targets.mapNotNull { From d83f2f8103ffd847f6e80845368ea182b56498c9 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:29:43 -0300 Subject: [PATCH 05/17] feat: display switch for keep device data and implement shared dialog --- .../java/to/bitkit/repositories/TrezorRepo.kt | 38 +++++--- .../screens/wallets/HardwareWalletScreen.kt | 14 +-- .../ui/screens/wallets/HwWalletViewModel.kt | 14 ++- .../screens/wallets/RemoveHwWalletDialog.kt | 92 +++++++++++++++++++ .../general/HardwareWalletsSettingsScreen.kt | 13 +-- app/src/main/res/values/strings.xml | 1 + 6 files changed, 143 insertions(+), 29 deletions(-) create mode 100644 app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt diff --git a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt index ab74dd3cd7..f5d0319b1f 100644 --- a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt @@ -1143,12 +1143,7 @@ class TrezorRepo @Inject constructor( val named = previous ?: knownDevices.firstOrNull { it.walletKey == identityKey } val resolvedWalletId = previous?.walletId?.takeIf { it.isNotBlank() } ?: knownDevices.findHardwareWalletId(xpubs, fallback = deviceInfo.id) - // A name restored from a backup, or kept when this wallet was removed, belongs to the wallet - // identity rather than to any device entry, so adopt it the first time the identity is paired - // again. An entry that already carries one was named on this device more recently. - val pendingName = resolvedWalletId.takeIf { it.isNotBlank() } - ?.let { hwWalletStore.loadPendingNames()[it] } - ?.takeIf { it.isNotBlank() } + val pendingName = pendingNameFor(resolvedWalletId) val customLabel = named?.customLabel ?: pendingName val known = KnownDevice( id = deviceInfo.id, @@ -1161,15 +1156,7 @@ class TrezorRepo @Inject constructor( xpubs = xpubs, customLabel = customLabel, walletId = resolvedWalletId, - // 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. - passphraseProtected = when (selection) { - WalletSelection.Standard -> false - is WalletSelection.Hidden -> true - WalletSelection.OnDevice -> previous?.passphraseProtected ?: true - }, + passphraseProtected = selection.isPassphraseProtected(previous), trezorDeviceId = features.deviceId ?: previous?.trezorDeviceId, ) val updated = knownDevices.filterNot { it.isReplacedBy(known, refreshed = previous) } + known @@ -1183,6 +1170,27 @@ class TrezorRepo @Inject constructor( return known } + /** + * The selection that derived a device's 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 fun WalletSelection.isPassphraseProtected(previous: KnownDevice?): Boolean = when (this) { + WalletSelection.Standard -> false + is WalletSelection.Hidden -> true + WalletSelection.OnDevice -> previous?.passphraseProtected ?: true + } + + /** + * The name a wallet identity carries while it has no device entry: restored from a backup, or kept + * when the wallet was removed. Adopted the first time the identity is paired again. + */ + private suspend fun pendingNameFor(walletId: String): String? = walletId + .takeIf { it.isNotBlank() } + ?.let { hwWalletStore.loadPendingNames()[it] } + ?.takeIf { it.isNotBlank() } + /** * Reads account-level extended public keys for every supported address type so a * watch-only balance can be tracked later without the device present. Permanent diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/HardwareWalletScreen.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/HardwareWalletScreen.kt index b1d49cac5c..8de971c067 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/HardwareWalletScreen.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/HardwareWalletScreen.kt @@ -49,7 +49,6 @@ import to.bitkit.ui.components.TabBar import to.bitkit.ui.components.TertiaryButton import to.bitkit.ui.components.TopBarSpacer import to.bitkit.ui.components.VerticalSpacer -import to.bitkit.ui.scaffold.AppAlertDialog import to.bitkit.ui.scaffold.AppTopBar import to.bitkit.ui.scaffold.DrawerNavIcon import to.bitkit.ui.screens.wallets.activity.components.activityListGroupedItems @@ -87,6 +86,8 @@ fun HardwareWalletScreen( onConfirmRemove = { viewModel.removeDevice(walletId) }, onDismissRemoveDialog = viewModel::onDismissRemoveDialog, onBackClick = onBackClick, + keepBackupData = uiState.keepBackupDataOnRemoval, + onKeepBackupDataChange = viewModel::onKeepBackupDataChange, ) } } @@ -101,6 +102,8 @@ private fun HardwareWalletContent( onConfirmRemove: () -> Unit, onDismissRemoveDialog: () -> Unit, onBackClick: () -> Unit, + keepBackupData: Boolean = true, + onKeepBackupDataChange: (Boolean) -> Unit = {}, ) { val hasFunds = wallet.balanceSats > 0uL val hasFundingFunds = wallet.fundingBalanceSats > 0uL @@ -215,11 +218,10 @@ private fun HardwareWalletContent( } if (showRemoveDialog) { - AppAlertDialog( - title = stringResource(R.string.hardware__remove_dialog_title, wallet.name), - text = stringResource(R.string.hardware__remove_dialog_text), - confirmText = stringResource(R.string.common__remove), - dismissText = stringResource(R.string.common__cancel), + RemoveHwWalletDialog( + walletName = wallet.name, + keepBackupData = keepBackupData, + onKeepBackupDataChange = onKeepBackupDataChange, onConfirm = onConfirmRemove, onDismiss = onDismissRemoveDialog, ) diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt index bf71316d5b..109576cca3 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt @@ -34,7 +34,13 @@ class HwWalletViewModel @Inject constructor( private var renameSessionId = 0L - fun onRemoveClick(wallet: HwWallet) = _uiState.update { it.copy(isPendingRemoval = wallet) } + // Reset on open rather than on dismiss, so a back press, a failed removal or another wallet picked + // from the list all start from the default rather than from the last choice. + fun onRemoveClick(wallet: HwWallet) = _uiState.update { + it.copy(isPendingRemoval = wallet, keepBackupDataOnRemoval = true) + } + + fun onKeepBackupDataChange(value: Boolean) = _uiState.update { it.copy(keepBackupDataOnRemoval = value) } fun onDismissRemoveDialog() = _uiState.update { it.copy(isPendingRemoval = null) } @@ -112,9 +118,12 @@ class HwWalletViewModel @Inject constructor( renameSessionId == sessionId && isPendingRename?.id == walletId fun removeDevice(walletId: String) { + // Read before the update below clears the pending state this choice was made in. + val keepBackupData = _uiState.value.keepBackupDataOnRemoval + viewModelScope.launch { _uiState.update { it.copy(isPendingRemoval = null) } - hwWalletRepo.removeDevice(walletId).onFailure { + hwWalletRepo.removeDevice(walletId, keepBackupData).onFailure { ToastEventBus.send( type = Toast.ToastType.ERROR, title = context.getString(R.string.common__error), @@ -128,6 +137,7 @@ class HwWalletViewModel @Inject constructor( @Immutable data class HwWalletDetailUiState( val isPendingRemoval: HwWallet? = null, + val keepBackupDataOnRemoval: Boolean = true, val isPendingRename: HwWallet? = null, val labelInput: String = "", val isSavingLabel: Boolean = false, diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt new file mode 100644 index 0000000000..47c129fe84 --- /dev/null +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt @@ -0,0 +1,92 @@ +package to.bitkit.ui.screens.wallets + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.material3.Switch +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.tooling.preview.Preview +import androidx.compose.ui.unit.dp +import to.bitkit.R +import to.bitkit.ui.components.BodyM +import to.bitkit.ui.components.BodyMSB +import to.bitkit.ui.components.VerticalSpacer +import to.bitkit.ui.scaffold.AppAlertDialog +import to.bitkit.ui.shared.modifiers.clickableAlpha +import to.bitkit.ui.theme.AppSwitchDefaults +import to.bitkit.ui.theme.AppThemeSurface +import to.bitkit.ui.theme.Colors + +/** + * 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. + */ +@Composable +fun RemoveHwWalletDialog( + walletName: String, + keepBackupData: Boolean, + onKeepBackupDataChange: (Boolean) -> Unit, + onConfirm: () -> Unit, + onDismiss: () -> Unit, + modifier: Modifier = Modifier, +) { + AppAlertDialog( + title = stringResource(R.string.hardware__remove_dialog_title, walletName), + confirmText = stringResource(R.string.common__remove), + dismissText = stringResource(R.string.common__cancel), + onConfirm = onConfirm, + onDismiss = onDismiss, + modifier = modifier.testTag("RemoveHwWalletDialog") + ) { + Column { + BodyM(text = stringResource(R.string.hardware__remove_dialog_text), color = Colors.White64) + VerticalSpacer(16.dp) + KeepBackupDataRow( + keepBackupData = keepBackupData, + onKeepBackupDataChange = onKeepBackupDataChange, + ) + } + } +} + +@Composable +private fun KeepBackupDataRow( + keepBackupData: Boolean, + onKeepBackupDataChange: (Boolean) -> Unit, + modifier: Modifier = Modifier, +) { + Row( + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + modifier = modifier + .fillMaxWidth() + .clickableAlpha { onKeepBackupDataChange(!keepBackupData) } + .testTag("HwRemoveKeepBackupToggle") + ) { + BodyMSB(text = stringResource(R.string.hardware__remove_dialog_keep)) + Switch( + checked = keepBackupData, + onCheckedChange = null, // handled by parent + colors = AppSwitchDefaults.colors, + ) + } +} + +@Preview(showBackground = true) +@Composable +private fun Preview() { + AppThemeSurface { + RemoveHwWalletDialog( + walletName = "Trezor Safe 3", + keepBackupData = true, + onKeepBackupDataChange = {}, + onConfirm = {}, + onDismiss = {}, + ) + } +} diff --git a/app/src/main/java/to/bitkit/ui/settings/general/HardwareWalletsSettingsScreen.kt b/app/src/main/java/to/bitkit/ui/settings/general/HardwareWalletsSettingsScreen.kt index 3e00fe24b2..bcc35c01c1 100644 --- a/app/src/main/java/to/bitkit/ui/settings/general/HardwareWalletsSettingsScreen.kt +++ b/app/src/main/java/to/bitkit/ui/settings/general/HardwareWalletsSettingsScreen.kt @@ -63,13 +63,13 @@ import to.bitkit.ui.components.MoneySSB import to.bitkit.ui.components.PrimaryButton import to.bitkit.ui.components.TextInput import to.bitkit.ui.components.VerticalSpacer -import to.bitkit.ui.scaffold.AppAlertDialog import to.bitkit.ui.scaffold.AppTopBar import to.bitkit.ui.scaffold.DrawerNavIcon import to.bitkit.ui.scaffold.ScreenColumn import to.bitkit.ui.scaffold.SheetTopBar import to.bitkit.ui.screens.wallets.HwWalletDetailUiState import to.bitkit.ui.screens.wallets.HwWalletViewModel +import to.bitkit.ui.screens.wallets.RemoveHwWalletDialog import to.bitkit.ui.shared.modifiers.clickableAlpha import to.bitkit.ui.shared.modifiers.sheetHeight import to.bitkit.ui.shared.util.gradientBackground @@ -96,6 +96,7 @@ fun HardwareWalletsSettingsScreen( onRemoveClick = viewModel::onRemoveClick, onConfirmRemove = { viewModel.removeDevice(it.id) }, onDismissRemoveDialog = viewModel::onDismissRemoveDialog, + onKeepBackupDataChange = viewModel::onKeepBackupDataChange, onRenameClick = viewModel::onRenameClick, onDismissRenameSheet = viewModel::onDismissRenameSheet, onLabelChange = viewModel::onLabelChange, @@ -112,6 +113,7 @@ private fun Content( onRemoveClick: (HwWallet) -> Unit = {}, onConfirmRemove: (HwWallet) -> Unit = {}, onDismissRemoveDialog: () -> Unit = {}, + onKeepBackupDataChange: (Boolean) -> Unit = {}, onRenameClick: (HwWallet) -> Unit = {}, onDismissRenameSheet: () -> Unit = {}, onLabelChange: (String) -> Unit = {}, @@ -178,11 +180,10 @@ private fun Content( } uiState.isPendingRemoval?.let { wallet -> - AppAlertDialog( - title = stringResource(R.string.hardware__remove_dialog_title, wallet.name), - text = stringResource(R.string.hardware__remove_dialog_text), - confirmText = stringResource(R.string.common__remove), - dismissText = stringResource(R.string.common__cancel), + RemoveHwWalletDialog( + walletName = wallet.name, + keepBackupData = uiState.keepBackupDataOnRemoval, + onKeepBackupDataChange = onKeepBackupDataChange, onConfirm = { onConfirmRemove(wallet) }, onDismiss = onDismissRemoveDialog, ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 357583ed8f..5137fefb40 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -204,6 +204,7 @@ If you have funds protected by a passphrase, enter it below to add these funds to your wallet balance as well. Passphrase Remove %1$s + Keep name and tags in backup Don\'t worry, your funds are safe and your coins won\'t be deleted. Bitkit will simply stop displaying the amounts in the wallet. Remove %1$s Could not remove the hardware wallet. Please try again. From 174fefe24bff67a5a66968b314d2e6ca89184003 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:44:31 -0300 Subject: [PATCH 06/17] fix: failing to read one must not stop the device being paired --- app/src/main/java/to/bitkit/repositories/TrezorRepo.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt index f5d0319b1f..e2838dfbb2 100644 --- a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt @@ -1188,7 +1188,8 @@ class TrezorRepo @Inject constructor( */ private suspend fun pendingNameFor(walletId: String): String? = walletId .takeIf { it.isNotBlank() } - ?.let { hwWalletStore.loadPendingNames()[it] } + // Only a name: failing to read one must not stop the device being paired. + ?.let { runSuspendCatching { hwWalletStore.loadPendingNames()[it] }.getOrNull() } ?.takeIf { it.isNotBlank() } /** From e63f4f34957895fb4a4f8a16a7a595ed92ad46cd Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:44:59 -0300 Subject: [PATCH 07/17] test: HW name and tag persistence tests --- .../bitkit/repositories/ActivityRepoTest.kt | 48 +++++++ .../to/bitkit/repositories/BackupRepoTest.kt | 133 +++++++++++++++++- .../bitkit/repositories/HwWalletRepoTest.kt | 116 +++++++++++++++ .../to/bitkit/repositories/TrezorRepoTest.kt | 93 ++++++++++++ .../screens/wallets/HwWalletViewModelTest.kt | 44 +++++- 5 files changed, 429 insertions(+), 5 deletions(-) diff --git a/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt b/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt index 728a5721f8..bf8bec153e 100644 --- a/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt @@ -7,6 +7,7 @@ import com.synonym.bitkitcore.IcJitEntry import com.synonym.bitkitcore.LightningActivity import com.synonym.bitkitcore.OnchainActivity import com.synonym.bitkitcore.PaymentType +import com.synonym.bitkitcore.PreActivityMetadata import com.synonym.bitkitcore.SortDirection import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.MutableStateFlow @@ -857,6 +858,53 @@ class ActivityRepoTest : BaseUnitTest() { assertEquals(emptyList(), result) } + @Test + fun `getTagMetadataForWallet unions the stored and the rendered tag metadata`() = test { + stubHardwareTagLookup(hardwareOnchainActivity(txType = PaymentType.SENT)) + val stored = PreActivityMetadata( + walletId = HARDWARE_WALLET_ID, + paymentId = "not-yet-seen", + tags = listOf("gift"), + paymentHash = null, + txId = "not-yet-seen", + address = null, + isReceive = false, + feeRate = 0uL, + isTransfer = false, + channelId = null, + createdAt = 1uL, + ) + whenever { coreService.activity.getAllPreActivityMetadata() }.thenReturn(listOf(stored)) + + val result = sut.getTagMetadataForWallet(HARDWARE_WALLET_ID).getOrThrow() + + // Core drops both on delete, and only the rendered half comes back from the activity tags. + assertEquals(listOf("not-yet-seen", "hw-txid"), result.map { it.paymentId }) + } + + @Test + fun `getTagMetadataForWallet ignores the metadata of other wallets`() = test { + stubHardwareTagLookup(hardwareOnchainActivity(txType = PaymentType.SENT)) + val otherWallet = PreActivityMetadata( + walletId = WalletScope.default, + paymentId = "default-payment", + tags = listOf("daily"), + paymentHash = null, + txId = null, + address = null, + isReceive = true, + feeRate = 0uL, + isTransfer = false, + channelId = null, + createdAt = 1uL, + ) + whenever { coreService.activity.getAllPreActivityMetadata() }.thenReturn(listOf(otherWallet)) + + val result = sut.getTagMetadataForWallet(HARDWARE_WALLET_ID).getOrThrow() + + assertEquals(listOf("hw-txid"), result.map { it.paymentId }) + } + private suspend fun stubHardwareTagLookup(activity: Activity.Onchain) { whenever { coreService.activity.getAllActivitiesTags() } .thenReturn(listOf(ActivityTags(HARDWARE_WALLET_ID, "hw-activity", listOf("cold")))) diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index 164cf9d144..a2e4b03189 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -29,6 +29,8 @@ import org.mockito.kotlin.whenever import to.bitkit.data.AppCacheData import to.bitkit.data.AppDb import to.bitkit.data.CacheStore +import to.bitkit.data.HwWalletData +import to.bitkit.data.HwWalletStore import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.WatchOnlyAccountAllocationState @@ -45,7 +47,9 @@ import to.bitkit.di.json import to.bitkit.models.ActivityBackupV1 import to.bitkit.models.BackupCategory import to.bitkit.models.BackupItemStatus +import to.bitkit.models.KnownDevice import to.bitkit.models.MetadataBackupV1 +import to.bitkit.models.TransportType import to.bitkit.models.WalletBackupV1 import to.bitkit.models.WalletScope import to.bitkit.models.WatchOnlyAccountRecord @@ -57,12 +61,14 @@ import to.bitkit.utils.AppError import javax.inject.Provider import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Clock import kotlin.time.ExperimentalTime import kotlin.time.Instant @OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class) +@Suppress("LargeClass") class BackupRepoTest : BaseUnitTest() { private val context = mock() private val cacheStore = mock() @@ -72,6 +78,7 @@ class BackupRepoTest : BaseUnitTest() { private val widgetsStore = mock() private val watchOnlyAccountStore = mock() private val watchOnlyAccountRepo = mock() + private val hwWalletStore = mock() private val blocktankRepo = mock() private val activityRepo = mock() private val pubkyRepo = mock() @@ -86,6 +93,7 @@ class BackupRepoTest : BaseUnitTest() { private val cacheData = MutableStateFlow(AppCacheData()) private val settingsData = MutableStateFlow(SettingsData()) private val widgetsData = MutableStateFlow(WidgetsData()) + private val hwWalletData = MutableStateFlow(HwWalletData()) private lateinit var sut: BackupRepo @@ -105,6 +113,9 @@ class BackupRepoTest : BaseUnitTest() { whenever { watchOnlyAccountStore.backupSnapshot() }.thenReturn( WatchOnlyAccountBackupSnapshot(emptyList(), WatchOnlyAccountAllocationState()) ) + whenever(hwWalletStore.data).thenReturn(hwWalletData) + whenever { hwWalletStore.backupSnapshot() }.thenReturn(emptyMap()) + whenever { hwWalletStore.restoreNames(any()) }.thenReturn(Unit) whenever { vssBackupClient.getObject(any()) }.thenReturn(Result.success(null)) whenever { vssBackupClient.putObject(any(), any()) } .thenReturn(Result.success(VssItem(key = BackupCategory.SETTINGS.name, value = byteArrayOf(), version = 1))) @@ -459,6 +470,102 @@ class BackupRepoTest : BaseUnitTest() { assertEquals(listOf(preActivityMetadata(), hardwareTagMetadata), payload.tagMetadata) } + @Test + fun `metadata backup carries the hardware wallet names`() = test { + stubMetadataBackupReads() + whenever { hwWalletStore.backupSnapshot() }.thenReturn(mapOf(HARDWARE_WALLET_ID to "Cold Storage")) + val dataCaptor = argumentCaptor() + + sut.triggerBackup(BackupCategory.METADATA) + + verifyBlocking(vssBackupClient) { + putObject(eq(BackupCategory.METADATA.name), dataCaptor.capture()) + } + val payload = json.decodeFromString(dataCaptor.firstValue.decodeToString()) + assertEquals(mapOf(HARDWARE_WALLET_ID to "Cold Storage"), payload.hwWalletNames) + } + + @Test + fun `metadata backup omits the hardware wallet names when none are set`() = test { + stubMetadataBackupReads() + val dataCaptor = argumentCaptor() + + sut.triggerBackup(BackupCategory.METADATA) + + verifyBlocking(vssBackupClient) { + putObject(eq(BackupCategory.METADATA.name), dataCaptor.capture()) + } + val payload = json.decodeFromString(dataCaptor.firstValue.decodeToString()) + assertNull(payload.hwWalletNames) + } + + @Test + fun `metadata backup fails when the hardware wallet names cannot be read`() = test { + stubMetadataBackupReads() + whenever { hwWalletStore.backupSnapshot() } + .doSuspendableAnswer { throw BackupRepoTestError("store unavailable") } + + val result = sut.triggerBackup(BackupCategory.METADATA) + + assertTrue(result.isFailure) + // This envelope is the only copy of the names, so uploading without them would drop them. + verify(vssBackupClient, never()).putObject(eq(BackupCategory.METADATA.name), any()) + } + + @Test + fun `metadata restore applies the backed up hardware wallet names`() = test { + val names = mapOf(HARDWARE_WALLET_ID to "Cold Storage") + stubMetadataRestore( + envelope = metadataEnvelope(metadata = listOf(preActivityMetadata()), hwWalletNames = names), + ) + + sut.performFullRestoreFromLatestBackup() + + verifyBlocking(hwWalletStore) { restoreNames(names) } + } + + @Test + fun `metadata restore keeps the stored names when the envelope carries none`() = test { + stubMetadataRestore(envelope = metadataEnvelope(metadata = listOf(preActivityMetadata()))) + + sut.performFullRestoreFromLatestBackup() + + // An envelope written before this field must not clear what this wallet already holds. + verifyBlocking(hwWalletStore) { restoreNames(emptyMap()) } + } + + @Test + fun `renaming a hardware wallet triggers a metadata backup`() = test { + stubMetadataBackupReads() + stubBackupObservers() + stubBackupStatuses( + MutableStateFlow(emptyMap()), + CompletableDeferred().apply { complete(Unit) }, + ) {} + + try { + sut.startObservingBackups() + runCurrent() + + // Reconnecting rewrites the entry without touching its name. + hwWalletData.update { HwWalletData(knownDevices = listOf(knownDevice())) } + runCurrent() + advanceTimeBy(10_000) + runCurrent() + + verify(vssBackupClient, never()).putObject(eq(BackupCategory.METADATA.name), any()) + + hwWalletData.update { HwWalletData(knownDevices = listOf(knownDevice(customLabel = "Cold Storage"))) } + runCurrent() + advanceTimeBy(10_000) + runCurrent() + + verifyBlocking(vssBackupClient) { putObject(eq(BackupCategory.METADATA.name), any()) } + } finally { + sut.stopObservingBackups() + } + } + @Test fun `activity traffic alone does not trigger a metadata backup`() = test { val activitiesChanged = MutableStateFlow(0L) @@ -643,8 +750,16 @@ class BackupRepoTest : BaseUnitTest() { ) ) - private fun metadataEnvelope(metadata: List = emptyList()) = json.encodeToString( - MetadataBackupV1(createdAt = 123, tagMetadata = metadata, cache = AppCacheData()) + private fun metadataEnvelope( + metadata: List = emptyList(), + hwWalletNames: Map? = null, + ) = json.encodeToString( + MetadataBackupV1( + createdAt = 123, + tagMetadata = metadata, + cache = AppCacheData(), + hwWalletNames = hwWalletNames, + ) ) private fun envelopeWithRawField(base: String, field: String, rawJson: String): String { @@ -659,6 +774,19 @@ class BackupRepoTest : BaseUnitTest() { tags = listOf("coffee"), ) + private fun knownDevice(customLabel: String? = null) = KnownDevice( + id = "dev1", + name = "Trezor", + path = "usb-1", + transportType = TransportType.USB, + label = null, + model = "Safe 3", + lastConnectedAt = 1, + xpubs = mapOf("nativeSegwit" to "zpubNS"), + customLabel = customLabel, + walletId = HARDWARE_WALLET_ID, + ) + private fun preActivityMetadata() = PreActivityMetadata( walletId = WalletScope.default, paymentId = "p1", @@ -741,6 +869,7 @@ class BackupRepoTest : BaseUnitTest() { widgetsStore = widgetsStore, watchOnlyAccountStore = watchOnlyAccountStore, watchOnlyAccountRepo = watchOnlyAccountRepo, + hwWalletStore = hwWalletStore, blocktankRepo = blocktankRepo, activityRepo = activityRepo, pubkyRepo = pubkyRepo, diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index 5eb92f8b6e..da5ed5f3a7 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -5,6 +5,7 @@ import com.synonym.bitkitcore.Activity import com.synonym.bitkitcore.ComposeResult import com.synonym.bitkitcore.OnchainActivity import com.synonym.bitkitcore.PaymentType +import com.synonym.bitkitcore.PreActivityMetadata import com.synonym.bitkitcore.TransactionDetails import com.synonym.bitkitcore.TrezorException import com.synonym.bitkitcore.TrezorFeatures @@ -61,6 +62,7 @@ class HwWalletRepoTest : BaseUnitTest() { private val trezorRepo = mock() private val activityRepo = mock() + private val preActivityMetadataRepo = mock() private val hwWalletStore = mock() private val settingsStore = mock() @@ -113,6 +115,9 @@ class HwWalletRepoTest : BaseUnitTest() { } whenever { activityRepo.getWalletIds() }.thenReturn(Result.success(emptySet())) whenever { activityRepo.deleteForWallet(any()) }.thenReturn(Result.success(Unit)) + whenever { activityRepo.getTagMetadataForWallet(any()) }.thenReturn(Result.success(emptyList())) + whenever { preActivityMetadataRepo.upsertPreActivityMetadata(any()) }.thenReturn(Result.success(Unit)) + whenever { hwWalletStore.setPendingName(any(), anyOrNull()) }.thenReturn(Unit) } private fun passphraseCapableFeatures(): TrezorFeatures = @@ -121,6 +126,7 @@ class HwWalletRepoTest : BaseUnitTest() { private fun createRepo() = HwWalletRepo( trezorRepo = trezorRepo, activityRepo = activityRepo, + preActivityMetadataRepo = preActivityMetadataRepo, hwWalletStore = hwWalletStore, settingsStore = settingsStore, ioDispatcher = testDispatcher, @@ -1372,6 +1378,102 @@ class HwWalletRepoTest : BaseUnitTest() { verify(trezorRepo).forgetDevice("dev1", "zpubNS") } + @Test + fun `removeDevice keeping backup data rewrites the tag metadata after deleting the activities`() = test { + val named = device.copy(customLabel = "Cold Storage") + val tagMetadata = listOf(preActivityMetadata()) + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) + whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } + .thenReturn(Result.success(tagMetadata)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + assertTrue(result.isSuccess) + // Core drops the wallet's tag metadata with its activities, so the rewrite must follow the delete. + inOrder(activityRepo, preActivityMetadataRepo) { + verify(activityRepo).deleteForWallet(HARDWARE_WALLET_ID) + verify(preActivityMetadataRepo).upsertPreActivityMetadata(tagMetadata) + } + } + + @Test + fun `removeDevice keeping backup data stores the wallet name before forgetting the device`() = test { + val named = device.copy(customLabel = "Cold Storage") + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + assertTrue(result.isSuccess) + // Forgetting the entry takes its label, so the name must be stored while it is still there. + inOrder(hwWalletStore, trezorRepo) { + verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, "Cold Storage") + verify(trezorRepo).forgetDevice("dev1", "zpubNS") + } + } + + @Test + fun `removeDevice keeping backup data stores no name for a wallet that was never renamed`() = test { + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + assertTrue(result.isSuccess) + verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + } + + @Test + fun `removeDevice without keeping backup data drops the name and the tag metadata`() = test { + val named = device.copy(customLabel = "Cold Storage") + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = false) + + assertTrue(result.isSuccess) + verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(activityRepo, never()).getTagMetadataForWallet(any()) + verify(preActivityMetadataRepo, never()).upsertPreActivityMetadata(any()) + } + + @Test + fun `removeDevice keeps nothing by default`() = test { + val named = device.copy(customLabel = "Cold Storage") + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID) + + assertTrue(result.isSuccess) + verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(preActivityMetadataRepo, never()).upsertPreActivityMetadata(any()) + } + + @Test + fun `removeDevice still removes the wallet when the tag metadata cannot be rewritten`() = test { + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) + whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } + .thenReturn(Result.success(listOf(preActivityMetadata()))) + whenever { preActivityMetadataRepo.upsertPreActivityMetadata(any()) } + .thenReturn(Result.failure(AppError("core unavailable"))) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + // The activities are already gone and the watchers stopped, so there is nothing to roll back to: + // the tags are lost rather than the removal reported as failed. + assertTrue(result.isSuccess) + verify(trezorRepo).forgetDevice("dev1", "zpubNS") + } + @Test fun `removeDevice fails when forget reports credential cleanup failure despite the device being gone`() = test { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) @@ -1823,6 +1925,20 @@ class HwWalletRepoTest : BaseUnitTest() { assertEquals("My Cold Wallet", sut.wallets.value.single().name) } + private fun preActivityMetadata() = PreActivityMetadata( + walletId = HARDWARE_WALLET_ID, + paymentId = "hw-txid", + tags = listOf("cold"), + paymentHash = null, + txId = "hw-txid", + address = null, + isReceive = false, + feeRate = 0uL, + isTransfer = false, + channelId = null, + createdAt = 1uL, + ) + private suspend fun wheneverStartWatcher() = whenever( trezorRepo.startWatcher( any(), diff --git a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt index 3a2419e8bd..ba552d9e75 100644 --- a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt @@ -112,6 +112,8 @@ class TrezorRepoTest : BaseUnitTest() { whenever(context.getString(R.string.hardware__connect_error)).thenReturn("Could not connect to your Trezor.") whenever(context.getString(R.string.hardware__device_busy)).thenReturn(DEVICE_BUSY_MESSAGE) whenever { hwWalletStore.loadKnownDevices() }.thenReturn(emptyList()) + whenever { hwWalletStore.loadPendingNames() }.thenReturn(emptyMap()) + whenever { hwWalletStore.setPendingName(any(), anyOrNull()) }.thenReturn(Unit) stubAccountXpubFetch() } @@ -832,6 +834,97 @@ class TrezorRepoTest : BaseUnitTest() { assertEquals("standard-wallet", added.walletId) } + @Test + fun `connect adopts the pending name of a wallet paired again`() = test { + // Restored from a backup, or kept when the wallet was removed: pairing takes the name over. + val sharedKey = "shared-native-xpub" + val unnamed = mockKnownDevice( + id = DEVICE_ID, + xpubs = ALL_ADDRESS_TYPE_KEYS.associateWith { sharedKey }, + customLabel = null, + walletId = "standard-wallet", + ) + val features = mockFeatures() + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(unnamed)) + whenever { hwWalletStore.loadPendingNames() }.thenReturn(mapOf("standard-wallet" to "Cold Storage")) + whenever(trezorService.connect(eq(DEVICE_ID), any())).thenReturn(features) + whenever(trezorService.scan()).thenReturn(listOf(mockDeviceInfo())) + whenever( + trezorService.getPublicKey(path = any(), coin = anyOrNull(), showOnTrezor = eq(false)) + ).thenAnswer { mockPublicKeyResponse(xpub = sharedKey, path = it.getArgument(0)) } + sut = createSut() + + sut.scan() + val result = sut.connect(DEVICE_ID) + + assertTrue(result.isSuccess) + val captor = argumentCaptor>() + verify(hwWalletStore).saveKnownDevices(captor.capture()) + assertEquals("Cold Storage", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) + // Consumed, so clearing the name later cannot fall back to it again. + verify(hwWalletStore).setPendingName("standard-wallet", null) + } + + @Test + fun `connect prefers the stored custom label over a pending name`() = test { + val sharedKey = "shared-native-xpub" + val stored = mockKnownDevice( + id = DEVICE_ID, + xpubs = ALL_ADDRESS_TYPE_KEYS.associateWith { sharedKey }, + customLabel = "Renamed Here", + walletId = "standard-wallet", + ) + val features = mockFeatures() + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(stored)) + whenever { hwWalletStore.loadPendingNames() }.thenReturn(mapOf("standard-wallet" to "Cold Storage")) + whenever(trezorService.connect(eq(DEVICE_ID), any())).thenReturn(features) + whenever(trezorService.scan()).thenReturn(listOf(mockDeviceInfo())) + whenever( + trezorService.getPublicKey(path = any(), coin = anyOrNull(), showOnTrezor = eq(false)) + ).thenAnswer { mockPublicKeyResponse(xpub = sharedKey, path = it.getArgument(0)) } + sut = createSut() + + sut.scan() + val result = sut.connect(DEVICE_ID) + + assertTrue(result.isSuccess) + val captor = argumentCaptor>() + verify(hwWalletStore).saveKnownDevices(captor.capture()) + assertEquals("Renamed Here", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) + // The pending name lost, so it is stale: dropping it keeps a later rename from falling back to it. + verify(hwWalletStore).setPendingName("standard-wallet", null) + } + + @Test + fun `connect leaves the pending name of another identity on the device alone`() = test { + val sharedKey = "shared-native-xpub" + val unnamed = mockKnownDevice( + id = DEVICE_ID, + xpubs = ALL_ADDRESS_TYPE_KEYS.associateWith { sharedKey }, + customLabel = null, + walletId = "standard-wallet", + ) + val features = mockFeatures() + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(unnamed)) + // A passphrase wallet on the same device derives its own keys, so its name is its own. + whenever { hwWalletStore.loadPendingNames() }.thenReturn(mapOf("hidden-wallet" to "Hidden Stash")) + whenever(trezorService.connect(eq(DEVICE_ID), any())).thenReturn(features) + whenever(trezorService.scan()).thenReturn(listOf(mockDeviceInfo())) + whenever( + trezorService.getPublicKey(path = any(), coin = anyOrNull(), showOnTrezor = eq(false)) + ).thenAnswer { mockPublicKeyResponse(xpub = sharedKey, path = it.getArgument(0)) } + sut = createSut() + + sut.scan() + val result = sut.connect(DEVICE_ID) + + assertTrue(result.isSuccess) + val captor = argumentCaptor>() + verify(hwWalletStore).saveKnownDevices(captor.capture()) + assertNull(captor.firstValue.single { it.id == DEVICE_ID }.customLabel) + verify(hwWalletStore, never()).setPendingName(any(), anyOrNull()) + } + @Test fun `connect supersedes entries of a seed the device no longer carries`() = test { // A wiped and restored device reports a new device id and different keys, so nothing would diff --git a/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt index 3a052033da..3bce2d280a 100644 --- a/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Before import org.junit.Test +import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -98,20 +99,57 @@ class HwWalletViewModelTest : BaseUnitTest() { @Test fun `removeDevice delegates to the repo and clears the pending device`() = test { - whenever { hwWalletRepo.removeDevice("dev1") }.thenReturn(Result.success(Unit)) + whenever { hwWalletRepo.removeDevice("dev1", true) }.thenReturn(Result.success(Unit)) val sut = createSut() sut.onRemoveClick(wallet) sut.removeDevice("dev1") advanceUntilIdle() - verify(hwWalletRepo).removeDevice("dev1") + verify(hwWalletRepo).removeDevice("dev1", true) assertNull(sut.uiState.value.isPendingRemoval) } + @Test + fun `onRemoveClick offers to keep the backup data`() = test { + val sut = createSut() + + sut.onRemoveClick(wallet) + + assertEquals(true, sut.uiState.value.keepBackupDataOnRemoval) + } + + @Test + fun `onRemoveClick restores the default after a wallet was removed without keeping its data`() = test { + whenever { hwWalletRepo.removeDevice(any(), any()) }.thenReturn(Result.success(Unit)) + val sut = createSut() + sut.onRemoveClick(wallet) + sut.onKeepBackupDataChange(false) + sut.removeDevice("dev1") + advanceUntilIdle() + + // Reopening starts from the default rather than from the last choice. + sut.onRemoveClick(otherWallet) + + assertEquals(true, sut.uiState.value.keepBackupDataOnRemoval) + } + + @Test + fun `removeDevice forwards the choice to keep nothing`() = test { + whenever { hwWalletRepo.removeDevice("dev1", false) }.thenReturn(Result.success(Unit)) + val sut = createSut() + sut.onRemoveClick(wallet) + sut.onKeepBackupDataChange(false) + + sut.removeDevice("dev1") + advanceUntilIdle() + + verify(hwWalletRepo).removeDevice("dev1", false) + } + @Test fun `removeDevice sends an error toast on failure`() = test { - whenever { hwWalletRepo.removeDevice("dev1") }.thenReturn(Result.failure(AppError("nope"))) + whenever { hwWalletRepo.removeDevice(any(), any()) }.thenReturn(Result.failure(AppError("nope"))) val sut = createSut() val toasts = mutableListOf() From da40800c8141d7162968457fd9231ae1f7654e79 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 20 Aug 2026 11:49:31 -0300 Subject: [PATCH 08/17] doc: changelog --- changelog.d/next/1173.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/next/1173.added.md diff --git a/changelog.d/next/1173.added.md b/changelog.d/next/1173.added.md new file mode 100644 index 0000000000..451e942706 --- /dev/null +++ b/changelog.d/next/1173.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 e10f84a642eece463443030268f3be93b1d6e1ab Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 09:04:22 -0300 Subject: [PATCH 09/17] fix: prevent a typo on passphrase from removing a saved wallet --- .../to/bitkit/repositories/HwWalletRepo.kt | 6 ++- .../bitkit/repositories/HwWalletRepoTest.kt | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt index 0305f9e05b..76e546dba0 100644 --- a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt @@ -298,9 +298,11 @@ class HwWalletRepo @Inject constructor( Logger.warn("Rejected hardware session for '$walletId': opened wallet '$opened'", context = TAG) // Reading the accounts of the wrong wallet already stored it; a mistyped passphrase - // must not leave a stray watch-only wallet behind. + // must not leave a stray watch-only wallet behind. Its backup data is kept: the wallet + // is a real one the user owns, and storing it has already consumed any name restored + // for it into the entry about to be forgotten. if (opened !in watchedBefore) { - removeDevice(opened) + removeDevice(opened, keepBackupData = true) .onFailure { Logger.warn("Failed to drop unwatched wallet '$opened'", it, context = TAG) } } trezorRepo.disconnectStaleSession(deviceId) diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index da5ed5f3a7..076d757bbb 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -1248,6 +1248,43 @@ class HwWalletRepoTest : BaseUnitTest() { verify(trezorRepo).disconnectStaleSession("dev1") } + @Test + fun `reconnectWithPassphrase keeps the backup data of the wallet a wrong passphrase opened`() = test { + // The wallet is a real one the user owns, and storing it consumed the name restored for it, + // so dropping it here would erase a backed up name a typo was never meant to touch. + val strayWallet = device.copy( + xpubs = mapOf("nativeSegwit" to "zpubStray"), + walletId = "stray-wallet", + customLabel = "Hidden Stash", + passphraseProtected = true, + ) + val strayTagMetadata = listOf(preActivityMetadata().copy(walletId = "stray-wallet")) + var stored = listOf(device, hiddenWallet) + whenever { hwWalletStore.loadKnownDevices() }.thenAnswer { stored } + whenever { activityRepo.getTagMetadataForWallet("stray-wallet") } + .thenReturn(Result.success(strayTagMetadata)) + whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenAnswer { + stored = stored.filterNot { it.walletId == "stray-wallet" } + Result.success(Unit) + } + whenever { trezorRepo.connectWithWalletMode("dev1", TrezorWalletMode.PASSPHRASE_HOST, "wrong") } + .thenAnswer { + stored = stored + strayWallet + trezorState.value = TrezorState( + connected = ConnectedTrezorDevice(id = "dev1", features = mock(), walletId = "stray-wallet"), + ) + Result.success(mock()) + } + val sut = createRepo() + + val result = sut.reconnectWithPassphrase(HIDDEN_WALLET_ID, "wrong") + + assertTrue(result.exceptionOrNull() is HwPassphraseMismatchError) + verify(hwWalletStore).setPendingName("stray-wallet", "Hidden Stash") + verify(preActivityMetadataRepo).upsertPreActivityMetadata(strayTagMetadata) + } + @Test fun `funding account resolves the requested identity on a shared device`() = test { storeData.value = HwWalletData(knownDevices = listOf(device, hiddenWallet)) From a17ea06923fd58a8f8814234a3151f11a6229b29 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 09:13:36 -0300 Subject: [PATCH 10/17] fix: prevent a failure on restoreNames skip restoreMetadataBackup --- .../java/to/bitkit/repositories/BackupRepo.kt | 6 ++++-- .../to/bitkit/repositories/BackupRepoTest.kt | 20 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index bd7b55116f..7c2c5b699d 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -683,8 +683,10 @@ class BackupRepo @Inject constructor( Logger.warn("Failed to restore pubky contact profile overrides", it, context = TAG) } // 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. - hwWalletStore.restoreNames(parsed.hwWalletNames.orEmpty()) + // Restored names wait as pending ones until each wallet is paired again. Failing to store them + // must not discard the rest of this envelope, which has already been applied by here. + runSuspendCatching { hwWalletStore.restoreNames(parsed.hwWalletNames.orEmpty()) } + .onFailure { Logger.warn("Failed to restore hardware wallet names", it, context = TAG) } Logger.debug("Restored ${parsed.tagMetadata.size} pre-activity metadata", TAG) Logger.debug("Restored ${parsed.hwWalletNames.orEmpty().size} hardware wallet names", TAG) diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index a2e4b03189..b8a88abf98 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -534,6 +534,26 @@ class BackupRepoTest : BaseUnitTest() { verifyBlocking(hwWalletStore) { restoreNames(emptyMap()) } } + @Test + fun `metadata restore records the category when the hardware wallet names cannot be stored`() = test { + stubWalletBackup() + stubMetadataRestore( + envelope = metadataEnvelope( + metadata = listOf(preActivityMetadata()), + hwWalletNames = mapOf(HARDWARE_WALLET_ID to "Cold Storage"), + ), + ) + whenever { hwWalletStore.restoreNames(any()) } + .doSuspendableAnswer { throw BackupRepoTestError("store unavailable") } + + val result = sut.performFullRestoreFromLatestBackup() + + assertTrue(result.isSuccess) + // The names are the last and the least of this envelope: the caches and tags above them were + // already applied, so losing the whole category's synced marker over a name would be wrong. + verifyBlocking(cacheStore) { updateBackupStatus(eq(BackupCategory.METADATA), any()) } + } + @Test fun `renaming a hardware wallet triggers a metadata backup`() = test { stubMetadataBackupReads() From 83a5edbba60732c9f5b971464a708d686223261b Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 09:26:29 -0300 Subject: [PATCH 11/17] fix: make delete pending name and persist the used one a single action --- .../main/java/to/bitkit/data/HwWalletStore.kt | 19 ++++++- .../java/to/bitkit/repositories/TrezorRepo.kt | 16 +++--- .../to/bitkit/repositories/TrezorRepoTest.kt | 50 +++++++++---------- 3 files changed, 48 insertions(+), 37 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/HwWalletStore.kt b/app/src/main/java/to/bitkit/data/HwWalletStore.kt index 01c81a1d21..85289c78cf 100644 --- a/app/src/main/java/to/bitkit/data/HwWalletStore.kt +++ b/app/src/main/java/to/bitkit/data/HwWalletStore.kt @@ -33,8 +33,23 @@ class HwWalletStore @Inject constructor( store.data.first().knownDevices } - suspend fun saveKnownDevices(devices: List) = withContext(ioDispatcher) { - store.updateData { it.copy(knownDevices = devices) } + /** + * @param consumedPendingName the wallet whose pending name one of [devices] has just adopted, dropped + * in the same write. Splitting the two would let the entry carrying the name fail to save while the + * pending copy is deleted anyway, leaving the name nowhere. + */ + suspend fun saveKnownDevices( + devices: List, + consumedPendingName: String? = null, + ) = withContext(ioDispatcher) { + store.updateData { data -> + data.copy( + knownDevices = devices, + pendingNames = consumedPendingName + ?.let { data.pendingNames - it } + ?: data.pendingNames, + ) + } Unit } diff --git a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt index e2838dfbb2..40c05f7585 100644 --- a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt @@ -1160,12 +1160,10 @@ class TrezorRepo @Inject constructor( trezorDeviceId = features.deviceId ?: previous?.trezorDeviceId, ) val updated = knownDevices.filterNot { it.isReplacedBy(known, refreshed = previous) } + known - saveKnownDevices(updated) - // Consumed, so the name lives on the entry alone: leaving it would resurrect a name the user - // later clears, since the entry would then fall back to the pending one again. - if (pendingName != null) { - hwWalletStore.setPendingName(resolvedWalletId, null) - } + // The pending name is consumed in the same write as the entry that adopted it, so the name + // lives in exactly one place: leaving it pending would resurrect it once the user clears the + // entry's own label, and dropping it separately would lose it if saving the entry failed. + saveKnownDevices(updated, consumedPendingName = resolvedWalletId.takeIf { pendingName != null }) _state.update { it.copy(knownDevices = updated.toImmutableList()) } return known } @@ -1254,9 +1252,9 @@ class TrezorRepo @Inject constructor( Logger.error("Failed to load known devices", it, context = TAG) }.getOrDefault(emptyList()) - private suspend fun saveKnownDevices(devices: List) { - runCatching { - hwWalletStore.saveKnownDevices(devices) + private suspend fun saveKnownDevices(devices: List, consumedPendingName: String? = null) { + runSuspendCatching { + hwWalletStore.saveKnownDevices(devices, consumedPendingName) }.onFailure { Logger.error("Failed to save known devices", it, context = TAG) } } diff --git a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt index ba552d9e75..af891c8c58 100644 --- a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt @@ -28,6 +28,7 @@ import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq +import org.mockito.kotlin.isNull import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times @@ -113,7 +114,6 @@ class TrezorRepoTest : BaseUnitTest() { whenever(context.getString(R.string.hardware__device_busy)).thenReturn(DEVICE_BUSY_MESSAGE) whenever { hwWalletStore.loadKnownDevices() }.thenReturn(emptyList()) whenever { hwWalletStore.loadPendingNames() }.thenReturn(emptyMap()) - whenever { hwWalletStore.setPendingName(any(), anyOrNull()) }.thenReturn(Unit) stubAccountXpubFetch() } @@ -241,7 +241,7 @@ class TrezorRepoTest : BaseUnitTest() { val result = sut.initialize() assertTrue(result.isSuccess) - verify(hwWalletStore, never()).saveKnownDevices(any()) + verify(hwWalletStore, never()).saveKnownDevices(any(), anyOrNull()) assertEquals("", sut.state.value.knownDevices.single().walletId) } @@ -721,7 +721,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) val saved = captor.firstValue.single() assertEquals(DEVICE_ID, saved.id) assertEquals(TransportType.USB, saved.transportType) @@ -767,7 +767,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals(setOf(walletId), captor.firstValue.map { it.walletId }.toSet()) } @@ -789,7 +789,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) val saved = captor.firstValue assertEquals(2, saved.size) assertEquals(standard, saved.first()) @@ -828,7 +828,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) val added = captor.firstValue.single { it.id == DEVICE_ID } assertEquals("No Pass", added.customLabel) assertEquals("standard-wallet", added.walletId) @@ -859,10 +859,10 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + // Consumed in the same write as the entry that adopted it, so a failed save cannot lose it, + // and clearing the name later cannot fall back to it again. + verify(hwWalletStore).saveKnownDevices(captor.capture(), eq("standard-wallet")) assertEquals("Cold Storage", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) - // Consumed, so clearing the name later cannot fall back to it again. - verify(hwWalletStore).setPendingName("standard-wallet", null) } @Test @@ -889,10 +889,9 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) - assertEquals("Renamed Here", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) // The pending name lost, so it is stale: dropping it keeps a later rename from falling back to it. - verify(hwWalletStore).setPendingName("standard-wallet", null) + verify(hwWalletStore).saveKnownDevices(captor.capture(), eq("standard-wallet")) + assertEquals("Renamed Here", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) } @Test @@ -920,9 +919,8 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), isNull()) assertNull(captor.firstValue.single { it.id == DEVICE_ID }.customLabel) - verify(hwWalletStore, never()).setPendingName(any(), anyOrNull()) } @Test @@ -944,7 +942,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) val saved = captor.firstValue.single() assertEquals("new-device-id", saved.trezorDeviceId) assertTrue(saved.xpubs.values.none { it == "old-seed-xpub" }) @@ -969,7 +967,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals(2, captor.firstValue.size) assertEquals(standard, captor.firstValue.first()) } @@ -993,7 +991,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertFalse(captor.firstValue.single().passphraseProtected) } @@ -1017,7 +1015,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertTrue(captor.firstValue.single().passphraseProtected) } @@ -1035,7 +1033,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) val saved = captor.firstValue.single() assertFalse(saved.passphraseProtected) } @@ -1075,7 +1073,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals( mapOf( "nativeSegwit" to "native-xpub", @@ -1101,7 +1099,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals("Cold Storage", captor.lastValue.single().customLabel) } @@ -1230,7 +1228,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isFailure) assertEquals(DEVICE_BUSY_MESSAGE, sut.state.value.error) assertNull(sut.state.value.connectedDevice()) - verify(hwWalletStore, never()).saveKnownDevices(any()) + verify(hwWalletStore, never()).saveKnownDevices(any(), anyOrNull()) } @Test @@ -1277,7 +1275,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isFailure) assertNull(sut.state.value.connectedDevice()) - verify(hwWalletStore, never()).saveKnownDevices(any()) + verify(hwWalletStore, never()).saveKnownDevices(any(), anyOrNull()) } @Test @@ -1301,7 +1299,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isFailure) assertEquals("DeviceDisconnected", sut.state.value.error) assertNull(sut.state.value.connectedDevice()) - verify(hwWalletStore, never()).saveKnownDevices(any()) + verify(hwWalletStore, never()).saveKnownDevices(any(), anyOrNull()) } @Test @@ -2084,7 +2082,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals(emptyList(), captor.lastValue) assertTrue(sut.state.value.knownDevices.isEmpty()) } @@ -2220,7 +2218,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() - verify(hwWalletStore).saveKnownDevices(captor.capture()) + verify(hwWalletStore).saveKnownDevices(captor.capture(), anyOrNull()) assertEquals(listOf(keptWhenStored), captor.lastValue) } From 5202103ae2cfb364d69abc3deea22e426596048d Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 09:55:33 -0300 Subject: [PATCH 12/17] fix: improve tag reading error message --- .../to/bitkit/repositories/HwWalletRepo.kt | 12 +++++++++++- .../ui/screens/wallets/HwWalletViewModel.kt | 9 ++++++++- app/src/main/res/values/strings.xml | 1 + .../to/bitkit/repositories/HwWalletRepoTest.kt | 15 +++++++++++++++ .../screens/wallets/HwWalletViewModelTest.kt | 18 ++++++++++++++++++ 5 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt index 76e546dba0..4eaf299e41 100644 --- a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt @@ -466,8 +466,11 @@ class HwWalletRepo @Inject constructor( // Read before the deletion below, which takes the tags with the activities they are on. val keptName = targets.firstNotNullOfOrNull { it.customLabel?.takeIf(String::isNotBlank) } .takeIf { keepBackupData } + // Nothing has been deleted yet, so failing here costs nothing and keeps the choice with + // the user: retry, or remove the wallet without keeping its data. val keptTagMetadata = when { - keepBackupData -> activityRepo.getTagMetadataForWallet(walletId).getOrThrow() + keepBackupData -> activityRepo.getTagMetadataForWallet(walletId) + .getOrElse { throw HwBackupDataUnreadableError(it) } else -> emptyList() } activeWatchers.toList() @@ -890,6 +893,13 @@ class HwPassphraseRequiredError : AppError("Passphrase needed to reopen this wal /** The entered passphrase opened a different wallet than the one being signed from. */ class HwPassphraseMismatchError : AppError("Passphrase opened a different wallet") +/** + * A 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. + */ +class HwBackupDataUnreadableError(cause: Throwable) : AppError("Could not read the backup data", cause) + private data class HwWatcherData( val walletId: String, val addressType: String, diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt index 109576cca3..fe199c38a7 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/HwWalletViewModel.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.launch import to.bitkit.R import to.bitkit.models.HwWallet import to.bitkit.models.Toast +import to.bitkit.repositories.HwBackupDataUnreadableError import to.bitkit.repositories.HwWalletRepo import to.bitkit.repositories.HwWalletRepo.Companion.DEVICE_LABEL_MAX_LENGTH import to.bitkit.ui.shared.toast.ToastEventBus @@ -124,10 +125,16 @@ class HwWalletViewModel @Inject constructor( viewModelScope.launch { _uiState.update { it.copy(isPendingRemoval = null) } hwWalletRepo.removeDevice(walletId, keepBackupData).onFailure { + // The wallet is untouched when its backup data could not be read, so say what failed + // and point at the way through instead of asking for a retry that repeats it. + val description = when (it) { + is HwBackupDataUnreadableError -> R.string.hardware__remove_keep_error + else -> R.string.hardware__remove_error + } ToastEventBus.send( type = Toast.ToastType.ERROR, title = context.getString(R.string.common__error), - description = context.getString(R.string.hardware__remove_error), + description = context.getString(description), ) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5137fefb40..1f236296fb 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -208,6 +208,7 @@ Don\'t worry, your funds are safe and your coins won\'t be deleted. Bitkit will simply stop displaying the amounts in the wallet. Remove %1$s Could not remove the hardware wallet. Please try again. + Could not keep this wallet\'s tags in your backup. Try again, or remove it without keeping them. Could not rename the hardware wallet. Please try again. Could not search for hardware wallets. Check your connection and try again. Funds transfer to savings is usually instant, but settlement may take up to <accent>14 days</accent> under certain network conditions. diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index 076d757bbb..a66db7008f 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -1435,6 +1435,21 @@ class HwWalletRepoTest : BaseUnitTest() { } } + @Test + fun `removeDevice keeping backup data reports unreadable data without touching the wallet`() = test { + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device)) + whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } + .thenReturn(Result.failure(AppError("core unavailable"))) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + // Raised before anything is deleted, so the wallet survives and the user keeps the choice. + assertTrue(result.exceptionOrNull() is HwBackupDataUnreadableError) + verify(activityRepo, never()).deleteForWallet(any()) + verify(trezorRepo, never()).forgetDevice(any(), anyOrNull()) + } + @Test fun `removeDevice keeping backup data stores the wallet name before forgetting the device`() = test { val named = device.copy(customLabel = "Cold Storage") diff --git a/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt index 3bce2d280a..987a0b8ae1 100644 --- a/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/wallets/HwWalletViewModelTest.kt @@ -19,6 +19,7 @@ import to.bitkit.R import to.bitkit.models.HwWallet import to.bitkit.models.Toast import to.bitkit.models.TransportType +import to.bitkit.repositories.HwBackupDataUnreadableError import to.bitkit.repositories.HwWalletRepo import to.bitkit.repositories.HwWalletRepo.Companion.DEVICE_LABEL_MAX_LENGTH import to.bitkit.test.BaseUnitTest @@ -63,6 +64,7 @@ class HwWalletViewModelTest : BaseUnitTest() { whenever(hwWalletRepo.walletsLoaded).thenReturn(MutableStateFlow(true)) whenever(context.getString(R.string.common__error)).thenReturn("Error") whenever(context.getString(R.string.hardware__remove_error)).thenReturn("Could not remove") + whenever(context.getString(R.string.hardware__remove_keep_error)).thenReturn("Could not keep the tags") whenever(context.getString(R.string.hardware__rename_error)).thenReturn("Could not rename") } @@ -161,6 +163,22 @@ class HwWalletViewModelTest : BaseUnitTest() { collectJob.cancel() } + @Test + fun `removeDevice explains an unreadable backup data failure`() = test { + whenever { hwWalletRepo.removeDevice(any(), any()) } + .thenReturn(Result.failure(HwBackupDataUnreadableError(AppError("core unavailable")))) + val sut = createSut() + + val toasts = mutableListOf() + val collectJob = launch { ToastEventBus.events.collect { toasts.add(it) } } + sut.removeDevice("dev1") + advanceUntilIdle() + + // The generic retry message would hide that removing without keeping the data still works. + assertEquals("Could not keep the tags", toasts.single().description) + collectJob.cancel() + } + @Test fun `onRenameClick opens the rename sheet with the current name`() = test { val sut = createSut() From 1128ea55ac096c5b7cbef701f45e969787aa2b7e Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 10:19:16 -0300 Subject: [PATCH 13/17] test: removeDevice keeping backup data keeps the tags of a wallet that was never renamed --- .../bitkit/repositories/HwWalletRepoTest.kt | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index a66db7008f..a624c934f9 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -48,6 +48,7 @@ import to.bitkit.test.BaseUnitTest import to.bitkit.utils.AppError import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Duration.Companion.seconds @@ -1479,6 +1480,25 @@ class HwWalletRepoTest : BaseUnitTest() { verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) } + @Test + fun `removeDevice keeping backup data keeps the tags of a wallet that was never renamed`() = test { + // The name and the tags are kept independently: a wallet is far more likely to carry tags than + // a name the user bothered to set, and having no name must not cost it its tags. + val tagMetadata = listOf(preActivityMetadata()) + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) + whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } + .thenReturn(Result.success(tagMetadata)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + + assertTrue(result.isSuccess) + assertNull(device.customLabel) + verify(preActivityMetadataRepo).upsertPreActivityMetadata(tagMetadata) + verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + } + @Test fun `removeDevice without keeping backup data drops the name and the tag metadata`() = test { val named = device.copy(customLabel = "Cold Storage") From d1086f33773796877a577a157d2f716d73d3b41a Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 10:29:12 -0300 Subject: [PATCH 14/17] fix: line break --- .../to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt index 47c129fe84..9883ab546f 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/RemoveHwWalletDialog.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.unit.dp import to.bitkit.R import to.bitkit.ui.components.BodyM import to.bitkit.ui.components.BodyMSB +import to.bitkit.ui.components.HorizontalSpacer import to.bitkit.ui.components.VerticalSpacer import to.bitkit.ui.scaffold.AppAlertDialog import to.bitkit.ui.shared.modifiers.clickableAlpha @@ -68,7 +69,11 @@ private fun KeepBackupDataRow( .clickableAlpha { onKeepBackupDataChange(!keepBackupData) } .testTag("HwRemoveKeepBackupToggle") ) { - BodyMSB(text = stringResource(R.string.hardware__remove_dialog_keep)) + BodyMSB( + text = stringResource(R.string.hardware__remove_dialog_keep), + modifier = Modifier.weight(1f) + ) + HorizontalSpacer(16.dp) Switch( checked = keepBackupData, onCheckedChange = null, // handled by parent From 5c176025c1c5154f868a4b67ba64678ffea7c19a Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 11:15:08 -0300 Subject: [PATCH 15/17] fix: make garbage collector only consider wallets that actually have persisted activities --- .../to/bitkit/repositories/HwWalletRepo.kt | 14 ++++++--- .../bitkit/repositories/HwWalletRepoTest.kt | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt index 4eaf299e41..562a8d065c 100644 --- a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt @@ -688,15 +688,21 @@ class HwWalletRepo @Inject constructor( knownDevices: List, watcherSettings: WatcherSettings, ) { - val persistedWalletIds = activityRepo.getWalletIds().getOrDefault(emptySet()) - .filterNot { it == WalletScope.default } - .toSet() watcherMutex.withLock { + // Read under the lock: a removal deletes a wallet's activities while holding it, and this + // set decides what to delete. Reading it first would let a removal complete in between and + // then be undone here, taking the tag metadata it deliberately kept with it. + val persistedWalletIds = activityRepo.getWalletIds().getOrDefault(emptySet()) + .filterNot { it == WalletScope.default } + .toSet() val specs = knownDevices.toWatcherSpecs(watcherSettings.electrumUrl) val desiredIds = specs.map { it.watcherId }.toSet() val knownWalletIds = knownDevices.mapNotNull { it.resolvedWalletId() }.toSet() trackedWalletIds += persistedWalletIds - val removedWalletIds = trackedWalletIds - knownWalletIds + // Only wallets that still have activities to clear. Core drops a wallet's tag metadata + // along with its activities whether or not any matched, so cleaning up a wallet that has + // none is not a no-op: it takes the metadata a removal deliberately kept. + val removedWalletIds = (trackedWalletIds - knownWalletIds).intersect(persistedWalletIds) trackedWalletIds += knownWalletIds specs.forEach { spec -> diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index a624c934f9..46f5c49152 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -1346,6 +1346,8 @@ class HwWalletRepoTest : BaseUnitTest() { @Test fun `store removal deletes the hardware wallet activity scope`() = test { + // Only a wallet with activities left behind is cleaned up, so it must have some to clean. + whenever { activityRepo.getWalletIds() }.thenReturn(Result.success(setOf(HARDWARE_WALLET_ID))) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) createRepo() @@ -1373,6 +1375,33 @@ class HwWalletRepoTest : BaseUnitTest() { verify(activityRepo, never()).deleteForWallet(HARDWARE_WALLET_ID) } + @Test + fun `removing a wallet deletes its activities exactly once`() = test { + // A second delete would run Core's cascade again and take the tag metadata the removal + // deliberately kept. The interleaving that caused this in the field — the cleanup deciding from + // a scope set read before it takes the lock — needs real concurrency and is covered by manual + // QA; this pins the simpler invariant that the cleanup adds no delete of its own. + var persisted = setOf(HARDWARE_WALLET_ID) + whenever { activityRepo.getWalletIds() }.thenAnswer { Result.success(persisted) } + whenever { activityRepo.deleteForWallet(any()) }.thenAnswer { + persisted = emptySet() + Result.success(Unit) + } + whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) + wheneverStartWatcher().thenReturn(Result.success(Unit)) + whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + val sut = createRepo() + runCurrent() + + val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) + storeData.value = HwWalletData(knownDevices = emptyList()) + runCurrent() + + assertTrue(result.isSuccess) + verify(activityRepo, times(1)).deleteForWallet(HARDWARE_WALLET_ID) + } + @Test fun `resetState clears store and stops active watchers`() = test { storeData.value = HwWalletData( From 2ae9aaede4e2957c0f82bb51191345da7b68acdd Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 12:53:50 -0300 Subject: [PATCH 16/17] fix: make the name change ride the same store write that forgets the device, so the store never publishes a device list and a name set that disagree --- .../main/java/to/bitkit/data/HwWalletStore.kt | 24 ++++-- .../to/bitkit/repositories/HwWalletRepo.kt | 15 ++-- .../java/to/bitkit/repositories/TrezorRepo.kt | 18 +++-- .../bitkit/repositories/HwWalletRepoTest.kt | 77 ++++++++++--------- .../to/bitkit/repositories/TrezorRepoTest.kt | 5 +- 5 files changed, 83 insertions(+), 56 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/HwWalletStore.kt b/app/src/main/java/to/bitkit/data/HwWalletStore.kt index 85289c78cf..d65fa6ff47 100644 --- a/app/src/main/java/to/bitkit/data/HwWalletStore.kt +++ b/app/src/main/java/to/bitkit/data/HwWalletStore.kt @@ -34,20 +34,18 @@ class HwWalletStore @Inject constructor( } /** - * @param consumedPendingName the wallet whose pending name one of [devices] has just adopted, dropped - * in the same write. Splitting the two would let the entry carrying the name fail to save while the - * pending copy is deleted anyway, leaving the name nowhere. + * @param pendingName a pending-name change to apply in the same write, or null to leave them alone. + * Splitting the two would publish a device list without its matching name change, which restarts a + * watcher for a wallet already being removed and can leave a name in both places or in neither. */ suspend fun saveKnownDevices( devices: List, - consumedPendingName: String? = null, + pendingName: PendingNameUpdate? = null, ) = withContext(ioDispatcher) { store.updateData { data -> data.copy( knownDevices = devices, - pendingNames = consumedPendingName - ?.let { data.pendingNames - it } - ?: data.pendingNames, + pendingNames = pendingName?.applyTo(data.pendingNames) ?: data.pendingNames, ) } Unit @@ -92,6 +90,18 @@ class HwWalletStore @Inject constructor( } } +/** A pending-name change applied together with a device list write; a null [name] drops the entry. */ +data class PendingNameUpdate( + val walletId: String, + val name: String?, +) { + fun applyTo(pendingNames: Map): Map = when { + walletId.isBlank() -> pendingNames + name.isNullOrBlank() -> pendingNames - walletId + else -> pendingNames + (walletId to name) + } +} + @Serializable data class HwWalletData( val knownDevices: List = emptyList(), diff --git a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt index 562a8d065c..3cb350284c 100644 --- a/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/HwWalletRepo.kt @@ -33,6 +33,7 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import to.bitkit.async.appScope import to.bitkit.data.HwWalletStore +import to.bitkit.data.PendingNameUpdate import to.bitkit.data.SettingsStore import to.bitkit.di.IoDispatcher import to.bitkit.env.Env @@ -489,13 +490,17 @@ class HwWalletRepo @Inject constructor( // so a failure here loses the tags rather than failing the removal. preActivityMetadataRepo.upsertPreActivityMetadata(keptTagMetadata) } - // Before forgetting the entries below, so the name is never absent from the backup: - // dropping the entries takes their label with them. - hwWalletStore.setPendingName(walletId, keptName) trackedWalletIds -= walletId lastPersistedHwSnapshots -= walletId - val failures = targets.mapNotNull { - trezorRepo.forgetDevice(it.id, walletKey = it.walletKey).exceptionOrNull() + // The name is stored in the same write that forgets the entries carrying it, so the + // store never publishes a device list still holding this wallet. A separate write would, + // and a reconcile reading it restarts the watcher of the wallet being removed. + val failures = targets.mapNotNull { device -> + trezorRepo.forgetDevice( + device.id, + walletKey = device.walletKey, + pendingName = PendingNameUpdate(walletId, keptName), + ).exceptionOrNull() } val remaining = hwWalletStore.loadKnownDevices() failures.firstOrNull()?.let { throw it } diff --git a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt index 40c05f7585..a0e252bf77 100644 --- a/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TrezorRepo.kt @@ -53,6 +53,7 @@ import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import to.bitkit.async.appScope import to.bitkit.data.HwWalletStore +import to.bitkit.data.PendingNameUpdate import to.bitkit.data.SettingsStore import to.bitkit.di.IoDispatcher import to.bitkit.env.Env @@ -889,7 +890,11 @@ class TrezorRepo @Inject constructor( * credentials are only cleared once no identity of the device remains, so removing one hidden * wallet does not unpair the device for the others. */ - suspend fun forgetDevice(deviceId: String, walletKey: String? = null): Result = withContext(ioDispatcher) { + suspend fun forgetDevice( + deviceId: String, + walletKey: String? = null, + pendingName: PendingNameUpdate? = null, + ): Result = withContext(ioDispatcher) { runSuspendCatching { TrezorDebugLog.log("FORGET", "forgetDevice called for: $deviceId") // The store is the source of truth here: labels are written straight to it, so a @@ -938,7 +943,7 @@ class TrezorRepo @Inject constructor( TrezorDebugLog.log("FORGET", "Keeping credentials, another wallet still uses $deviceId") Result.success(Unit) } - saveKnownDevices(updated) + saveKnownDevices(updated, pendingName) _state.update { it.copy(knownDevices = updated.toImmutableList()) } clearCredentialsResult.getOrThrow() disconnectResult.onFailure { @@ -1163,7 +1168,10 @@ class TrezorRepo @Inject constructor( // The pending name is consumed in the same write as the entry that adopted it, so the name // lives in exactly one place: leaving it pending would resurrect it once the user clears the // entry's own label, and dropping it separately would lose it if saving the entry failed. - saveKnownDevices(updated, consumedPendingName = resolvedWalletId.takeIf { pendingName != null }) + saveKnownDevices( + updated, + pendingName = pendingName?.let { PendingNameUpdate(resolvedWalletId, name = null) }, + ) _state.update { it.copy(knownDevices = updated.toImmutableList()) } return known } @@ -1252,9 +1260,9 @@ class TrezorRepo @Inject constructor( Logger.error("Failed to load known devices", it, context = TAG) }.getOrDefault(emptyList()) - private suspend fun saveKnownDevices(devices: List, consumedPendingName: String? = null) { + private suspend fun saveKnownDevices(devices: List, pendingName: PendingNameUpdate? = null) { runSuspendCatching { - hwWalletStore.saveKnownDevices(devices, consumedPendingName) + hwWalletStore.saveKnownDevices(devices, pendingName) }.onFailure { Logger.error("Failed to save known devices", it, context = TAG) } } diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index 46f5c49152..637f9f15d9 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -31,6 +31,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import to.bitkit.data.HwWalletData import to.bitkit.data.HwWalletStore +import to.bitkit.data.PendingNameUpdate import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.env.Env @@ -940,7 +941,7 @@ class HwWalletRepoTest : BaseUnitTest() { val result = sut.removeDevice("unknown-wallet") assertTrue(result.isFailure) - verify(trezorRepo, never()).forgetDevice(any(), anyOrNull()) + verify(trezorRepo, never()).forgetDevice(any(), anyOrNull(), anyOrNull()) verify(activityRepo, never()).deleteForWallet("unknown-wallet") } @@ -950,14 +951,14 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device, hiddenWallet), listOf(device)) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() runCurrent() val result = sut.removeDevice(HIDDEN_WALLET_ID) assertTrue(result.isSuccess) - verify(trezorRepo).forgetDevice("dev1", "zpubHidden") + verify(trezorRepo).forgetDevice(eq("dev1"), eq("zpubHidden"), anyOrNull()) verify(trezorRepo).stopWatcher("$HIDDEN_WALLET_ID|nativeSegwit") verify(trezorRepo, never()).stopWatcher("$HARDWARE_WALLET_ID|nativeSegwit") verify(activityRepo).deleteForWallet(HIDDEN_WALLET_ID) @@ -1227,7 +1228,7 @@ class HwWalletRepoTest : BaseUnitTest() { var stored = listOf(device, hiddenWallet) whenever { hwWalletStore.loadKnownDevices() }.thenAnswer { stored } whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenAnswer { + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenAnswer { stored = stored.filterNot { it.walletId == "stray-wallet" } Result.success(Unit) } @@ -1245,7 +1246,7 @@ class HwWalletRepoTest : BaseUnitTest() { val result = sut.reconnectWithPassphrase(HIDDEN_WALLET_ID, "wrong") assertTrue(result.exceptionOrNull() is HwPassphraseMismatchError) - verify(trezorRepo).forgetDevice("dev1", "zpubStray") + verify(trezorRepo).forgetDevice(eq("dev1"), eq("zpubStray"), anyOrNull()) verify(trezorRepo).disconnectStaleSession("dev1") } @@ -1265,7 +1266,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever { activityRepo.getTagMetadataForWallet("stray-wallet") } .thenReturn(Result.success(strayTagMetadata)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenAnswer { + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenAnswer { stored = stored.filterNot { it.walletId == "stray-wallet" } Result.success(Unit) } @@ -1282,7 +1283,7 @@ class HwWalletRepoTest : BaseUnitTest() { val result = sut.reconnectWithPassphrase(HIDDEN_WALLET_ID, "wrong") assertTrue(result.exceptionOrNull() is HwPassphraseMismatchError) - verify(hwWalletStore).setPendingName("stray-wallet", "Hidden Stash") + verify(trezorRepo).forgetDevice(any(), anyOrNull(), eq(PendingNameUpdate("stray-wallet", "Hidden Stash"))) verify(preActivityMetadataRepo).upsertPreActivityMetadata(strayTagMetadata) } @@ -1390,7 +1391,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() runCurrent() @@ -1433,7 +1434,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() runCurrent() @@ -1442,7 +1443,7 @@ class HwWalletRepoTest : BaseUnitTest() { assertEquals(true, result.isSuccess) verify(trezorRepo).stopWatcher("hardware-wallet|nativeSegwit") verify(activityRepo).deleteForWallet(HARDWARE_WALLET_ID) - verify(trezorRepo).forgetDevice("dev1", "zpubNS") + verify(trezorRepo).forgetDevice(eq("dev1"), eq("zpubNS"), anyOrNull()) } @Test @@ -1452,7 +1453,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } .thenReturn(Result.success(tagMetadata)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) @@ -1477,36 +1478,38 @@ class HwWalletRepoTest : BaseUnitTest() { // Raised before anything is deleted, so the wallet survives and the user keeps the choice. assertTrue(result.exceptionOrNull() is HwBackupDataUnreadableError) verify(activityRepo, never()).deleteForWallet(any()) - verify(trezorRepo, never()).forgetDevice(any(), anyOrNull()) + verify(trezorRepo, never()).forgetDevice(any(), anyOrNull(), anyOrNull()) } @Test fun `removeDevice keeping backup data stores the wallet name before forgetting the device`() = test { val named = device.copy(customLabel = "Cold Storage") whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) assertTrue(result.isSuccess) - // Forgetting the entry takes its label, so the name must be stored while it is still there. - inOrder(hwWalletStore, trezorRepo) { - verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, "Cold Storage") - verify(trezorRepo).forgetDevice("dev1", "zpubNS") - } + // Carried by the write that forgets the entry, so the store never publishes a device list + // still holding this wallet, which would restart the watcher of the wallet being removed. + verify(trezorRepo).forgetDevice( + eq("dev1"), + eq("zpubNS"), + eq(PendingNameUpdate(HARDWARE_WALLET_ID, "Cold Storage")), + ) } @Test fun `removeDevice keeping backup data stores no name for a wallet that was never renamed`() = test { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) assertTrue(result.isSuccess) - verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(trezorRepo).forgetDevice(any(), anyOrNull(), eq(PendingNameUpdate(HARDWARE_WALLET_ID, null))) } @Test @@ -1517,7 +1520,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) whenever { activityRepo.getTagMetadataForWallet(HARDWARE_WALLET_ID) } .thenReturn(Result.success(tagMetadata)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) @@ -1525,20 +1528,20 @@ class HwWalletRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) assertNull(device.customLabel) verify(preActivityMetadataRepo).upsertPreActivityMetadata(tagMetadata) - verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(trezorRepo).forgetDevice(any(), anyOrNull(), eq(PendingNameUpdate(HARDWARE_WALLET_ID, null))) } @Test fun `removeDevice without keeping backup data drops the name and the tag metadata`() = test { val named = device.copy(customLabel = "Cold Storage") whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = false) assertTrue(result.isSuccess) - verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(trezorRepo).forgetDevice(any(), anyOrNull(), eq(PendingNameUpdate(HARDWARE_WALLET_ID, null))) verify(activityRepo, never()).getTagMetadataForWallet(any()) verify(preActivityMetadataRepo, never()).upsertPreActivityMetadata(any()) } @@ -1547,13 +1550,13 @@ class HwWalletRepoTest : BaseUnitTest() { fun `removeDevice keeps nothing by default`() = test { val named = device.copy(customLabel = "Cold Storage") whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(named), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID) assertTrue(result.isSuccess) - verify(hwWalletStore).setPendingName(HARDWARE_WALLET_ID, null) + verify(trezorRepo).forgetDevice(any(), anyOrNull(), eq(PendingNameUpdate(HARDWARE_WALLET_ID, null))) verify(preActivityMetadataRepo, never()).upsertPreActivityMetadata(any()) } @@ -1564,7 +1567,7 @@ class HwWalletRepoTest : BaseUnitTest() { .thenReturn(Result.success(listOf(preActivityMetadata()))) whenever { preActivityMetadataRepo.upsertPreActivityMetadata(any()) } .thenReturn(Result.failure(AppError("core unavailable"))) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID, keepBackupData = true) @@ -1572,19 +1575,19 @@ class HwWalletRepoTest : BaseUnitTest() { // The activities are already gone and the watchers stopped, so there is nothing to roll back to: // the tags are lost rather than the removal reported as failed. assertTrue(result.isSuccess) - verify(trezorRepo).forgetDevice("dev1", "zpubNS") + verify(trezorRepo).forgetDevice(eq("dev1"), eq("zpubNS"), anyOrNull()) } @Test fun `removeDevice fails when forget reports credential cleanup failure despite the device being gone`() = test { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.failure(AppError("clear failed"))) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.failure(AppError("clear failed"))) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID) assertEquals(true, result.isFailure) - verify(trezorRepo).forgetDevice("dev1", "zpubNS") + verify(trezorRepo).forgetDevice(eq("dev1"), eq("zpubNS"), anyOrNull()) } @Test @@ -1599,7 +1602,7 @@ class HwWalletRepoTest : BaseUnitTest() { assertEquals(true, result.isFailure) verify(trezorRepo).stopWatcher("hardware-wallet|nativeSegwit") - verify(trezorRepo, never()).forgetDevice(any(), anyOrNull()) + verify(trezorRepo, never()).forgetDevice(any(), anyOrNull(), anyOrNull()) } @Test @@ -1613,7 +1616,7 @@ class HwWalletRepoTest : BaseUnitTest() { assertTrue(result.isFailure) verify(activityRepo).deleteForWallet(HARDWARE_WALLET_ID) - verify(trezorRepo, never()).forgetDevice(any(), anyOrNull()) + verify(trezorRepo, never()).forgetDevice(any(), anyOrNull(), anyOrNull()) } @Test @@ -1624,21 +1627,21 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(bleEntry, usbEntry), emptyList()) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() runCurrent() sut.removeDevice(HARDWARE_WALLET_ID) verify(trezorRepo).stopWatcher("hardware-wallet|nativeSegwit") - verify(trezorRepo).forgetDevice("ble1", "zpubNS") - verify(trezorRepo).forgetDevice("usb1", "zpubNS") + verify(trezorRepo).forgetDevice(eq("ble1"), eq("zpubNS"), anyOrNull()) + verify(trezorRepo).forgetDevice(eq("usb1"), eq("zpubNS"), anyOrNull()) } @Test fun `removeDevice fails when the device is still present afterwards`() = test { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), listOf(device)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID) @@ -1651,7 +1654,7 @@ class HwWalletRepoTest : BaseUnitTest() { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), listOf(device)) wheneverStartWatcher().thenReturn(Result.success(Unit)) whenever { trezorRepo.stopWatcher(any()) }.thenReturn(Result.success(Unit)) - whenever { trezorRepo.forgetDevice(any(), anyOrNull()) }.thenReturn(Result.success(Unit)) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.success(Unit)) val sut = createRepo() runCurrent() diff --git a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt index af891c8c58..07d6156208 100644 --- a/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/TrezorRepoTest.kt @@ -36,6 +36,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import to.bitkit.R import to.bitkit.data.HwWalletStore +import to.bitkit.data.PendingNameUpdate import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.env.Env @@ -861,7 +862,7 @@ class TrezorRepoTest : BaseUnitTest() { val captor = argumentCaptor>() // Consumed in the same write as the entry that adopted it, so a failed save cannot lose it, // and clearing the name later cannot fall back to it again. - verify(hwWalletStore).saveKnownDevices(captor.capture(), eq("standard-wallet")) + verify(hwWalletStore).saveKnownDevices(captor.capture(), eq(PendingNameUpdate("standard-wallet", name = null))) assertEquals("Cold Storage", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) } @@ -890,7 +891,7 @@ class TrezorRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) val captor = argumentCaptor>() // The pending name lost, so it is stale: dropping it keeps a later rename from falling back to it. - verify(hwWalletStore).saveKnownDevices(captor.capture(), eq("standard-wallet")) + verify(hwWalletStore).saveKnownDevices(captor.capture(), eq(PendingNameUpdate("standard-wallet", name = null))) assertEquals("Renamed Here", captor.firstValue.single { it.id == DEVICE_ID }.customLabel) } From 065b44fdf1fe816b94d929bbe2f95f343e59a6e6 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 21 Aug 2026 13:12:13 -0300 Subject: [PATCH 17/17] chore: lint --- app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt index 637f9f15d9..85424a167a 100644 --- a/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/HwWalletRepoTest.kt @@ -1581,7 +1581,8 @@ class HwWalletRepoTest : BaseUnitTest() { @Test fun `removeDevice fails when forget reports credential cleanup failure despite the device being gone`() = test { whenever(hwWalletStore.loadKnownDevices()).thenReturn(listOf(device), emptyList()) - whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) }.thenReturn(Result.failure(AppError("clear failed"))) + whenever { trezorRepo.forgetDevice(any(), anyOrNull(), anyOrNull()) } + .thenReturn(Result.failure(AppError("clear failed"))) val sut = createRepo() val result = sut.removeDevice(HARDWARE_WALLET_ID)