From 623892e00c867b65c231d28c741be5c436c6fc3c Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 5 Aug 2026 08:48:30 -0300 Subject: [PATCH 1/2] fix: recover hw transfer after re-pairing --- .../to/bitkit/repositories/ActivityRepo.kt | 2 + .../to/bitkit/repositories/TransferRepo.kt | 29 ++++- .../java/to/bitkit/services/CoreService.kt | 49 +++++-- .../bitkit/repositories/ActivityRepoTest.kt | 50 ++++++- .../bitkit/repositories/TransferRepoTest.kt | 87 +++++++++++++ .../to/bitkit/services/CoreServiceTest.kt | 122 +++++++++++++++++- changelog.d/next/1130.fixed.md | 1 + 7 files changed, 325 insertions(+), 15 deletions(-) create mode 100644 changelog.d/next/1130.fixed.md diff --git a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt index 7e2b6c217..73375d5f0 100644 --- a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt @@ -225,10 +225,12 @@ class ActivityRepo @Inject constructor( transactionDetails: List, ): Result> = withContext(bgDispatcher) { runSuspendCatching { + val transferChannelIds = transferRepo.getChannelIdsByFundingTxId().getOrDefault(emptyMap()) val persistedActivities = coreService.activity.replaceHwSnapshot( walletId = walletId, activities = activities, transactionDetails = transactionDetails, + transferChannelIdsByFundingTxId = transferChannelIds, ) notifyActivitiesChanged() persistedActivities diff --git a/app/src/main/java/to/bitkit/repositories/TransferRepo.kt b/app/src/main/java/to/bitkit/repositories/TransferRepo.kt index 350e51c86..981922925 100644 --- a/app/src/main/java/to/bitkit/repositories/TransferRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/TransferRepo.kt @@ -91,8 +91,10 @@ class TransferRepo @Inject constructor( } } - // TODO maybe replace with delete, or call delete once activity item was augmented with the transfer's data. - // Likely no clear reason to keep persisting transfers afterwards. + // Settled transfers must be kept: [getChannelIdsByFundingTxId] recovers the transfer flag for a + // hardware wallet's funding tx after the device is removed and re-paired, and removal deletes the + // activities but not these records. Deleting on settle, or wiring up TransferDao.deleteOldSettled, + // regresses synonymdev/bitkit-android#1130. suspend fun markSettled(id: String): Result = withContext(bgDispatcher) { runCatching { val settledAt = clock.now().epochSeconds @@ -138,6 +140,29 @@ class TransferRepo @Inject constructor( } } + /** + * Maps funding tx id to channel id for every transfer Bitkit recorded itself. + * + * Removing a hardware wallet deletes its activities but never these records, so they are what + * lets a re-paired wallet's rediscovered funding tx read as a transfer again instead of a plain + * send. Settled transfers are included on purpose: [syncTransferStates] only re-marks transfers + * that are still active, and a settled one is exactly the case that regresses. + */ + suspend fun getChannelIdsByFundingTxId(): Result> = withContext(bgDispatcher) { + runSuspendCatching { + transferDao.getAll() + .mapNotNull { transfer -> + val fundingTxId = transfer.fundingTxId ?: return@mapNotNull null + val channelId = transfer.channelId ?: return@mapNotNull null + fundingTxId to channelId + } + .distinctBy { it.first } + .toMap() + }.onFailure { + Logger.warn("Failed to load channel ids by funding txid", it, context = TAG) + } + } + @Suppress("CyclomaticComplexMethod") suspend fun syncTransferStates(): Result = withContext(bgDispatcher) { runCatching { diff --git a/app/src/main/java/to/bitkit/services/CoreService.kt b/app/src/main/java/to/bitkit/services/CoreService.kt index 35b612d14..caaa68a76 100644 --- a/app/src/main/java/to/bitkit/services/CoreService.kt +++ b/app/src/main/java/to/bitkit/services/CoreService.kt @@ -246,27 +246,49 @@ internal data class HwSnapshotMerge( val toUpsert: List, ) +/** + * Builds the delete/upsert plan for a hardware wallet's on-chain snapshot. + * + * @param transferChannelIdsByFundingTxId funding tx id to channel id for transfers Bitkit recorded + * itself. Re-pairing a wallet that was removed leaves nothing to carry forward, because removal + * deleted its activities, and [to.bitkit.repositories.TransferRepo.syncTransferStates] only re-marks + * transfers that are still unsettled, so a completed transfer would come back from the watcher as a + * plain send. The funding tx id is still recorded against the Bitkit-side transfer, which removal + * does not touch. Has no default so a caller cannot drop the recovery silently. + */ internal fun mergeHwSnapshot( existing: List, incoming: List, + transferChannelIdsByFundingTxId: Map, ): HwSnapshotMerge { val incomingIds = incoming.map { it.rawId() }.toSet() val toDelete = existing.filter { !it.v1.isTransfer && it.v1.id !in incomingIds } val existingByTxId = existing.associateBy { it.v1.txId } val toUpsert = incoming.map { activity -> val onchain = activity as? Activity.Onchain ?: return@map activity - val stored = existingByTxId[onchain.v1.txId]?.v1 ?: return@map activity - Activity.Onchain( - onchain.v1.copy( - isTransfer = onchain.v1.isTransfer || stored.isTransfer, - channelId = onchain.v1.channelId ?: stored.channelId, - transferTxId = onchain.v1.transferTxId ?: stored.transferTxId, - ) - ) + val merged = onchain.v1 + .mergedWith(existingByTxId[onchain.v1.txId]?.v1) + .withRecoveredTransfer(transferChannelIdsByFundingTxId[onchain.v1.txId]) + Activity.Onchain(merged) } return HwSnapshotMerge(toDelete = toDelete, toUpsert = toUpsert) } +private fun OnchainActivity.mergedWith(stored: OnchainActivity?): OnchainActivity = when (stored) { + null -> this + else -> copy( + isTransfer = isTransfer || stored.isTransfer, + channelId = channelId ?: stored.channelId, + transferTxId = transferTxId ?: stored.transferTxId, + ) +} + +private fun OnchainActivity.withRecoveredTransfer(recoveredChannelId: String?): OnchainActivity = + when { + isTransfer || recoveredChannelId == null -> this + else -> copy(isTransfer = true, channelId = channelId ?: recoveredChannelId) + } + @Suppress("LargeClass", "TooManyFunctions") class ActivityService( @Suppress("unused") private val coreService: CoreService, // used to ensure CoreService inits first @@ -321,11 +343,16 @@ class ActivityService( * * Callers must merge every watcher for the wallet before invoking this method. The current * hardware-wallet integration has one supported watcher address type per wallet. + * + * [transferChannelIdsByFundingTxId] carries the Bitkit-side transfer records used to recover + * transfer metadata for transactions that have no stored activity left, e.g. after re-pairing a + * removed device. Pass an empty map when none are known. */ suspend fun replaceHwSnapshot( walletId: String, activities: List, transactionDetails: List, + transferChannelIdsByFundingTxId: Map, ): List = ServiceQueue.CORE.background { val existingActivities = getActivities( walletId = walletId, @@ -338,7 +365,11 @@ class ActivityService( limit = null, sortDirection = null, ).filterIsInstance() - val merge = mergeHwSnapshot(existing = existingActivities, incoming = activities) + val merge = mergeHwSnapshot( + existing = existingActivities, + incoming = activities, + transferChannelIdsByFundingTxId = transferChannelIdsByFundingTxId, + ) merge.toDelete.forEach { deleteActivityById(walletId = walletId, activityId = it.v1.id) deleteTransactionDetails(walletId = walletId, txId = it.v1.txId) diff --git a/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt b/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt index 0fcd4e611..f560fb88b 100644 --- a/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/ActivityRepoTest.kt @@ -33,6 +33,7 @@ import to.bitkit.ext.mock import to.bitkit.models.WalletScope import to.bitkit.services.CoreService import to.bitkit.test.BaseUnitTest +import to.bitkit.utils.AppError import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse @@ -131,6 +132,7 @@ class ActivityRepoTest : BaseUnitTest() { whenever(clock.now()).thenReturn(Clock.System.now()) whenever(lightningRepo.lightningState).thenReturn(MutableStateFlow(LightningState())) whenever(blocktankRepo.blocktankState).thenReturn(MutableStateFlow(BlocktankState())) + whenever { transferRepo.getChannelIdsByFundingTxId() }.thenReturn(Result.success(emptyMap())) sut = ActivityRepo( bgDispatcher = testDispatcher, @@ -296,13 +298,59 @@ class ActivityRepoTest : BaseUnitTest() { walletId = walletId, activities = listOf(activity), transactionDetails = emptyList(), + transferChannelIdsByFundingTxId = emptyMap(), ) ).thenReturn(listOf(activity)) val result = sut.persistHwSnapshot(walletId, listOf(activity), emptyList()) assertEquals(listOf(activity), result.getOrThrow()) - verify(coreService.activity).replaceHwSnapshot(walletId, listOf(activity), emptyList()) + verify(coreService.activity).replaceHwSnapshot(walletId, listOf(activity), emptyList(), emptyMap()) + } + + @Test + fun `persistHwSnapshot forwards known transfer channel ids to the merge`() = test { + val walletId = "hardware-wallet" + val activity = createOnchainActivity().copy( + v1 = baseOnchainActivity.copy(walletId = walletId) + ) + val channelIds = mapOf("base_tx_id" to "channel-1") + whenever(transferRepo.getChannelIdsByFundingTxId()).thenReturn(Result.success(channelIds)) + whenever( + coreService.activity.replaceHwSnapshot( + walletId = walletId, + activities = listOf(activity), + transactionDetails = emptyList(), + transferChannelIdsByFundingTxId = channelIds, + ) + ).thenReturn(listOf(activity)) + + val result = sut.persistHwSnapshot(walletId, listOf(activity), emptyList()) + + assertEquals(listOf(activity), result.getOrThrow()) + verify(coreService.activity).replaceHwSnapshot(walletId, listOf(activity), emptyList(), channelIds) + } + + @Test + fun `persistHwSnapshot still persists when transfer lookup fails`() = test { + val walletId = "hardware-wallet" + val activity = createOnchainActivity().copy( + v1 = baseOnchainActivity.copy(walletId = walletId) + ) + whenever(transferRepo.getChannelIdsByFundingTxId()).thenReturn(Result.failure(AppError("db down"))) + whenever( + coreService.activity.replaceHwSnapshot( + walletId = walletId, + activities = listOf(activity), + transactionDetails = emptyList(), + transferChannelIdsByFundingTxId = emptyMap(), + ) + ).thenReturn(listOf(activity)) + + val result = sut.persistHwSnapshot(walletId, listOf(activity), emptyList()) + + assertEquals(listOf(activity), result.getOrThrow()) + verify(coreService.activity).replaceHwSnapshot(walletId, listOf(activity), emptyList(), emptyMap()) } @Test diff --git a/app/src/test/java/to/bitkit/repositories/TransferRepoTest.kt b/app/src/test/java/to/bitkit/repositories/TransferRepoTest.kt index ce87680cd..4d2466390 100644 --- a/app/src/test/java/to/bitkit/repositories/TransferRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/TransferRepoTest.kt @@ -235,6 +235,93 @@ class TransferRepoTest : BaseUnitTest() { assertEquals(exception, result.exceptionOrNull()) } + @Test + fun `markSettled keeps the transfer record for hardware re-pair recovery`() = test { + setupClockNowMock() + whenever(transferDao.markSettled(any(), any())).thenReturn(Unit) + + val result = sut.markSettled(ID_TRANSFER) + + assertTrue(result.isSuccess) + verify(transferDao).markSettled(eq(ID_TRANSFER), any()) + verify(transferDao, never()).deleteOldSettled(any()) + } + + // MARK: - getChannelIdsByFundingTxId + + @Test + fun `getChannelIdsByFundingTxId includes settled transfers`() = test { + whenever(transferDao.getAll()).thenReturn( + listOf( + transferEntity( + id = ID_TRANSFER, + fundingTxId = "funding-tx", + channelId = ID_CHANNEL, + isSettled = true, + ) + ) + ) + + val result = sut.getChannelIdsByFundingTxId() + + assertEquals(mapOf("funding-tx" to ID_CHANNEL), result.getOrThrow()) + } + + @Test + fun `getChannelIdsByFundingTxId skips transfers without funding tx id or channel id`() = test { + whenever(transferDao.getAll()).thenReturn( + listOf( + transferEntity(id = "no-funding-tx", fundingTxId = null, channelId = ID_CHANNEL), + transferEntity(id = "no-channel", fundingTxId = "funding-tx", channelId = null), + transferEntity(id = ID_TRANSFER, fundingTxId = "kept-tx", channelId = ID_CHANNEL), + ) + ) + + val result = sut.getChannelIdsByFundingTxId() + + assertEquals(mapOf("kept-tx" to ID_CHANNEL), result.getOrThrow()) + } + + @Test + fun `getChannelIdsByFundingTxId keeps the first entry for duplicated funding tx ids`() = test { + whenever(transferDao.getAll()).thenReturn( + listOf( + transferEntity(id = "first", fundingTxId = "funding-tx", channelId = "channel-1"), + transferEntity(id = "second", fundingTxId = "funding-tx", channelId = "channel-2"), + ) + ) + + val result = sut.getChannelIdsByFundingTxId() + + assertEquals(mapOf("funding-tx" to "channel-1"), result.getOrThrow()) + } + + @Test + fun `getChannelIdsByFundingTxId returns failure when dao throws`() = test { + val exception = AppError("Database error") + whenever(transferDao.getAll()).thenAnswer { throw exception } + + val result = sut.getChannelIdsByFundingTxId() + + assertTrue(result.isFailure) + assertEquals(exception, result.exceptionOrNull()) + } + + private fun transferEntity( + id: String, + fundingTxId: String?, + channelId: String?, + isSettled: Boolean = false, + ) = TransferEntity( + id = id, + type = TransferType.TO_SPENDING, + amountSats = 50000L, + channelId = channelId, + fundingTxId = fundingTxId, + isSettled = isSettled, + createdAt = 1000L, + ) + // MARK: - syncTransferStates @Test diff --git a/app/src/test/java/to/bitkit/services/CoreServiceTest.kt b/app/src/test/java/to/bitkit/services/CoreServiceTest.kt index 82e124cb6..b90c7f85e 100644 --- a/app/src/test/java/to/bitkit/services/CoreServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/CoreServiceTest.kt @@ -1,12 +1,15 @@ package to.bitkit.services import com.synonym.bitkitcore.Activity +import com.synonym.bitkitcore.LightningActivity import com.synonym.bitkitcore.OnchainActivity +import com.synonym.bitkitcore.PaymentState import com.synonym.bitkitcore.PaymentType import org.junit.Test import to.bitkit.ext.create import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull import kotlin.test.assertTrue class CoreServiceTest { @@ -21,7 +24,7 @@ class CoreServiceTest { ) val incoming = activity(id = "transfer") - val result = mergeHwSnapshot(existing = listOf(existing), incoming = listOf(incoming)) + val result = mergePlan(existing = listOf(existing), incoming = listOf(incoming)) assertTrue(result.toDelete.isEmpty()) assertEquals(1, result.toUpsert.size) @@ -37,7 +40,7 @@ class CoreServiceTest { val transfer = activity(id = "transfer", isTransfer = true) val incoming = activity(id = "current") - val result = mergeHwSnapshot( + val result = mergePlan( existing = listOf(stale, transfer), incoming = listOf(incoming), ) @@ -47,8 +50,109 @@ class CoreServiceTest { assertFalse(result.toDelete.single().v1.isTransfer) } + @Test + fun `merge hw snapshot recovers transfer from known funding tx when no stored row remains`() { + val result = mergePlan( + existing = emptyList(), + incoming = listOf(activity(id = "fundingTx")), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + val recovered = result.upserted("fundingTx") + assertEquals(true, recovered?.isTransfer) + assertEquals("channel-1", recovered?.channelId) + } + + @Test + fun `merge hw snapshot leaves unrelated transaction unmarked`() { + val result = mergePlan( + existing = emptyList(), + incoming = listOf(activity(id = "someOtherTx")), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + val untouched = result.upserted("someOtherTx") + assertEquals(false, untouched?.isTransfer) + assertNull(untouched?.channelId) + } + + @Test + fun `merge hw snapshot keeps stored channel id over recovered channel id`() { + val result = mergePlan( + existing = listOf(activity(id = "fundingTx", isTransfer = true, channelId = "stored-channel")), + incoming = listOf(activity(id = "fundingTx")), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + val merged = result.upserted("fundingTx") + assertEquals(true, merged?.isTransfer) + assertEquals("stored-channel", merged?.channelId) + } + + @Test + fun `merge hw snapshot fills missing channel id on stored transfer`() { + val result = mergePlan( + existing = listOf(activity(id = "fundingTx", isTransfer = false, channelId = null)), + incoming = listOf(activity(id = "fundingTx")), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + val merged = result.upserted("fundingTx") + assertEquals(true, merged?.isTransfer) + assertEquals("channel-1", merged?.channelId) + } + + @Test + fun `merge hw snapshot recovers transfer matching on tx id not activity id`() { + val result = mergePlan( + existing = emptyList(), + incoming = listOf(activity(id = "rebuilt-activity-id", txId = "fundingTx")), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + val recovered = result.upserted("rebuilt-activity-id") + assertEquals(true, recovered?.isTransfer) + assertEquals("channel-1", recovered?.channelId) + } + + @Test + fun `merge hw snapshot leaves activities unchanged without known transfers`() { + val result = mergePlan(existing = emptyList(), incoming = listOf(activity(id = "fundingTx"))) + + val untouched = result.upserted("fundingTx") + assertEquals(false, untouched?.isTransfer) + assertNull(untouched?.channelId) + } + + @Test + fun `merge hw snapshot leaves lightning activities untouched`() { + val lightning = lightningActivity(id = "fundingTx") + + val result = mergePlan( + existing = emptyList(), + incoming = listOf(lightning), + transferChannelIdsByFundingTxId = mapOf("fundingTx" to "channel-1"), + ) + + assertEquals(listOf(lightning), result.toUpsert) + } + + private fun mergePlan( + existing: List, + incoming: List, + transferChannelIdsByFundingTxId: Map = emptyMap(), + ) = mergeHwSnapshot( + existing = existing, + incoming = incoming, + transferChannelIdsByFundingTxId = transferChannelIdsByFundingTxId, + ) + + private fun HwSnapshotMerge.upserted(id: String): OnchainActivity? = + toUpsert.filterIsInstance().firstOrNull { it.v1.id == id }?.v1 + private fun activity( id: String, + txId: String = id, isTransfer: Boolean = false, channelId: String? = null, transferTxId: String? = null, @@ -57,7 +161,7 @@ class CoreServiceTest { walletId = "hardware-wallet", id = id, txType = PaymentType.RECEIVED, - txId = id, + txId = txId, value = 1uL, fee = 0uL, address = "", @@ -67,4 +171,16 @@ class CoreServiceTest { transferTxId = transferTxId, ) ) + + private fun lightningActivity(id: String) = Activity.Lightning( + LightningActivity.create( + walletId = "hardware-wallet", + id = id, + txType = PaymentType.RECEIVED, + status = PaymentState.SUCCEEDED, + value = 1uL, + invoice = "", + timestamp = 1uL, + ) + ) } diff --git a/changelog.d/next/1130.fixed.md b/changelog.d/next/1130.fixed.md new file mode 100644 index 000000000..127b84707 --- /dev/null +++ b/changelog.d/next/1130.fixed.md @@ -0,0 +1 @@ +Restored the transfer label on hardware wallet transactions after removing and re-adding a device. From befd2957db398cd0b9a300774d905c28b4cc044b Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 5 Aug 2026 08:58:10 -0300 Subject: [PATCH 2/2] chore: rename changelog fragment --- changelog.d/next/{1130.fixed.md => 1133.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{1130.fixed.md => 1133.fixed.md} (100%) diff --git a/changelog.d/next/1130.fixed.md b/changelog.d/next/1133.fixed.md similarity index 100% rename from changelog.d/next/1130.fixed.md rename to changelog.d/next/1133.fixed.md