From 87cee81cb1e737cacc2e4ef519232c01766a605d Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Wed, 30 Sep 2026 10:28:02 +0200 Subject: [PATCH 1/9] fix: keep rn channel state through restore A seed restore was opening the node before the React Native channel backup finished, and a late channel file or early tag was discarded. Co-authored-by: Cursor --- .../to/bitkit/services/MigrationService.kt | 31 +++++-- .../java/to/bitkit/viewmodels/AppViewModel.kt | 37 ++++---- .../to/bitkit/viewmodels/WalletViewModel.kt | 11 ++- .../bitkit/services/MigrationServiceTest.kt | 27 ++++++ .../java/to/bitkit/ui/WalletViewModelTest.kt | 89 +++++++++++++++++++ changelog.d/next/1258.fixed.md | 1 + 6 files changed, 170 insertions(+), 26 deletions(-) create mode 100644 changelog.d/next/1258.fixed.md diff --git a/app/src/main/java/to/bitkit/services/MigrationService.kt b/app/src/main/java/to/bitkit/services/MigrationService.kt index 24696ceb1a..cbaf8bc655 100644 --- a/app/src/main/java/to/bitkit/services/MigrationService.kt +++ b/app/src/main/java/to/bitkit/services/MigrationService.kt @@ -328,6 +328,10 @@ class MigrationService @Inject constructor( Logger.debug("Cannot cleanup: pending metadata/transfers/boosts exists", context = TAG) return false } + if (peekPendingChannelMigration() != null) { + Logger.debug("Cannot cleanup: pending channel migration exists", context = TAG) + return false + } return true } @@ -941,13 +945,14 @@ class MigrationService @Inject constructor( } } - private suspend fun applyRNMetadata(metadata: RNMetadata) { + private suspend fun applyRNMetadata(metadata: RNMetadata): Set { val tags = metadata.tags if (tags.isNullOrEmpty()) { Logger.debug("No tags to apply in metadata", context = TAG) - return + return emptySet() } + val unapplied = mutableSetOf() var applied = 0 val allTags = tags.mapNotNull { (activityId, tagList) -> val onchain = activityRepo.getOnchainActivityByTxId(activityId) @@ -968,7 +973,8 @@ class MigrationService @Inject constructor( tags = tagList, ) } else { - Logger.warn("Activity not found for tags: id=$activityId", context = TAG) + Logger.warn("Activity not found for tags: id='$activityId'", context = TAG) + unapplied += activityId null } } @@ -979,9 +985,11 @@ class MigrationService @Inject constructor( coreService.activity.upsertTags(allTags) Logger.info("Applied $applied/${tags.size} pending tags", context = TAG) }.onFailure { - Logger.error("Failed to upsert tags: $it", it, context = TAG) + Logger.error("Failed to upsert tags", it, context = TAG) + unapplied += tags.keys } } + return unapplied } private suspend fun applyRNTodos(todos: RNTodos) { @@ -1720,8 +1728,12 @@ class MigrationService @Inject constructor( // Apply remote metadata (tags) AFTER activities are created pendingRemoteMetadata?.let { metadata -> Logger.info("Applying remote metadata (tags: ${metadata.tags?.size})", context = TAG) - applyRNMetadata(metadata) - clearPersistedMetadata() + val remaining = unappliedRnMetadata(metadata, applyRNMetadata(metadata)) + if (remaining == null) { + clearPersistedMetadata() + } else { + persistMetadata(remaining) + } } var blocktankFetchFailed = false @@ -2201,6 +2213,13 @@ data class RNMetadata( val lastUsedTags: List? = null, ) +internal fun unappliedRnMetadata(metadata: RNMetadata, unappliedActivityIds: Set): RNMetadata? { + if (unappliedActivityIds.isEmpty()) return null + val remaining = metadata.tags.orEmpty().filterKeys { it in unappliedActivityIds } + if (remaining.isEmpty()) return null + return metadata.copy(tags = remaining, lastUsedTags = null) +} + @Serializable data class RNTodos( val hide: Map? = null, diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index a66e2fe255..31726c5103 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1525,23 +1525,7 @@ class AppViewModel @Inject constructor( } private suspend fun completeRNRemoteBackupRestore() { - val channelMigration = buildChannelMigrationIfAvailable() - - if (channelMigration != null) { - lightningRepo.stop().onFailure { - Logger.error("Failed to stop node during remote restore restart", it, context = TAG) - } - delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) - lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) - .onSuccess { - migrationService.consumePendingChannelMigration() - walletRepo.syncNodeAndWallet() - walletRepo.syncBalances() - } - .onFailure { e -> - Logger.error("Failed to restart node after remote restore: $e", e, context = TAG) - } - } + applyPendingChannelMigration() lightningRepo.getPayments().onSuccess { activityRepo.syncLdkNodePayments(it) } migrationService.reapplyMetadataAfterSync() @@ -1566,6 +1550,23 @@ class AppViewModel @Inject constructor( ) } + private suspend fun applyPendingChannelMigration() { + val channelMigration = buildChannelMigrationIfAvailable() ?: return + lightningRepo.stop().onFailure { + Logger.error("Failed to stop node during remote restore restart", it, context = TAG) + } + delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) + lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) + .onSuccess { + migrationService.consumePendingChannelMigration() + walletRepo.syncNodeAndWallet() + walletRepo.syncBalances() + } + .onFailure { e -> + Logger.error("Failed to restart node after remote restore", e, context = TAG) + } + } + private suspend fun completeMigration() { if (isCompletingMigration) return isCompletingMigration = true @@ -1578,7 +1579,7 @@ class AppViewModel @Inject constructor( } activityRepo.markAllUnseenActivitiesAsSeen() - migrationService.consumePendingChannelMigration() + applyPendingChannelMigration() walletRepo.syncNodeAndWallet() .onSuccess { finishMigrationSuccessfully() } diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index b643365cfc..5333a73a34 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -342,9 +342,16 @@ class WalletViewModel @Inject constructor( private suspend fun waitForRestoreIfNeeded() { if (!_restoreState.value.isOngoing()) return - withTimeoutOrNull(TIMEOUT_RESTORE_WAIT) { + val finished = withTimeoutOrNull(TIMEOUT_RESTORE_WAIT) { _restoreState.first { !it.isOngoing() } - } ?: Logger.warn("waitForRestoreIfNeeded timeout, proceeding anyway", context = TAG) + } + if (finished != null) return + if (migrationService.isRestoringFromRNRemoteBackup.value) { + Logger.warn("RN remote restore still running, waiting for channel state", context = TAG) + _restoreState.first { !it.isOngoing() } + return + } + Logger.warn("waitForRestoreIfNeeded timeout, proceeding anyway", context = TAG) } private fun buildChannelMigrationIfAvailable(): ChannelDataMigration? = diff --git a/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt b/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt index 40cdd4572f..6d28f8cc57 100644 --- a/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt @@ -51,4 +51,31 @@ class MigrationServiceTest : BaseUnitTest() { assertTrue(requests.isEmpty()) assertTrue(result.isEmpty()) } + + @Test + fun `unapplied metadata keeps only tags whose activity is missing`() { + val metadata = RNMetadata( + tags = mapOf("known" to listOf("sent"), "missing" to listOf("received")), + lastUsedTags = listOf("sent"), + ) + + val remaining = unappliedRnMetadata(metadata, setOf("missing")) + + assertEquals(mapOf("missing" to listOf("received")), remaining?.tags) + assertEquals(null, remaining?.lastUsedTags) + } + + @Test + fun `unapplied metadata is dropped when every tag was applied`() { + val metadata = RNMetadata(tags = mapOf("known" to listOf("sent")), lastUsedTags = listOf("sent")) + + assertEquals(null, unappliedRnMetadata(metadata, emptySet())) + } + + @Test + fun `unapplied metadata is dropped when there are no tags`() { + val metadata = RNMetadata(tags = null, lastUsedTags = listOf("sent")) + + assertEquals(null, unappliedRnMetadata(metadata, setOf("missing"))) + } } diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 88f4ee7d27..2c8575bc47 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Before import org.junit.Test @@ -46,6 +47,7 @@ import kotlin.coroutines.cancellation.CancellationException import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.seconds @OptIn(ExperimentalCoroutinesApi::class) class WalletViewModelTest : BaseUnitTest() { @@ -75,6 +77,8 @@ class WalletViewModelTest : BaseUnitTest() { whenever(lightningRepo.lightningState).thenReturn(lightningState) whenever(migrationService.isMigrationChecked()).thenReturn(true) whenever(migrationService.isChannelRecoveryChecked()).thenReturn(true) + whenever(migrationService.isRestoringFromRNRemoteBackup).thenReturn(MutableStateFlow(false)) + whenever(migrationService.isShowingMigrationLoading).thenReturn(MutableStateFlow(false)) whenever(migrationService.tryFetchMigrationPeersFromBackup()).thenReturn(emptyList()) whenever { migrationService.getRNRemoteBackupTimestamp() }.thenReturn(null) whenever { backupRepo.hasPendingWalletRestore() }.thenReturn(false) @@ -661,6 +665,91 @@ class WalletViewModelTest : BaseUnitTest() { verify(testWalletRepo, never()).refreshBip21() } + @Test + fun `start waits out an rn remote restore instead of opening the node`() = test { + val testWalletRepo: WalletRepo = mock() + val testLightningRepo: LightningRepo = mock() + val testWalletState = MutableStateFlow(WalletState(walletExists = true)) + + whenever(migrationService.isRestoringFromRNRemoteBackup).thenReturn(MutableStateFlow(true)) + whenever(testWalletRepo.walletState).thenReturn(testWalletState) + whenever(testWalletRepo.balanceState).thenReturn(balanceState) + whenever(testWalletRepo.walletExists()).thenReturn(true) + whenever(testWalletRepo.restoreWallet(any(), anyOrNull())).thenReturn(Result.success(Unit)) + whenever(testLightningRepo.lightningState).thenReturn(lightningState) + whenever(testLightningRepo.isRecoveryMode).thenReturn(isRecoveryMode) + stubSuccessfulNodeStart(testLightningRepo) + + val testSut = WalletViewModel( + context = context, + bgDispatcher = testDispatcher, + walletRepo = testWalletRepo, + lightningRepo = testLightningRepo, + settingsStore = settingsStore, + backupRepo = backupRepo, + blocktankRepo = blocktankRepo, + pubkyRepo = pubkyRepo, + migrationService = migrationService, + connectivityRepo = connectivityRepo, + boltzService = boltzService, + ) + + testSut.restoreWallet("mnemonic", null) + testSut.start() + advanceTimeBy(31.seconds) + + verifyNodeNotStarted(testLightningRepo) + + testSut.onRestoreContinue() + advanceUntilIdle() + + verifyNodeStarted(testLightningRepo) + } + + private suspend fun stubSuccessfulNodeStart(repo: LightningRepo) { + whenever( + repo.start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + any(), + ), + ).thenReturn(Result.success(Unit)) + } + + private suspend fun verifyNodeNotStarted(repo: LightningRepo) { + verify( + repo, + never(), + ).start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + any(), + ) + } + + private suspend fun verifyNodeStarted(repo: LightningRepo) { + verify(repo).start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + any(), + ) + } + private fun stubSettingsUpdate(): MutableStateFlow { val settingsData = MutableStateFlow(SettingsData()) whenever { settingsStore.update(any()) }.thenAnswer { diff --git a/changelog.d/next/1258.fixed.md b/changelog.d/next/1258.fixed.md new file mode 100644 index 0000000000..8c9aeb3380 --- /dev/null +++ b/changelog.d/next/1258.fixed.md @@ -0,0 +1 @@ +Restoring a React Native wallet waits for its Lightning channel backup, and activity tags that are not ready yet are kept for a later sync. From e750fe0458c5c2e4601eca75a588b0f313eb160a Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Wed, 30 Sep 2026 10:39:02 +0200 Subject: [PATCH 2/9] fix: surface a failed rn channel restart A failed restart after stopping the node was only logged, so migration continued with the node down and no warning. Co-authored-by: Cursor --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 31726c5103..6899aae308 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1525,7 +1525,10 @@ class AppViewModel @Inject constructor( } private suspend fun completeRNRemoteBackupRestore() { - applyPendingChannelMigration() + if (!applyPendingChannelMigration()) { + finishMigrationWithError() + return + } lightningRepo.getPayments().onSuccess { activityRepo.syncLdkNodePayments(it) } migrationService.reapplyMetadataAfterSync() @@ -1550,21 +1553,24 @@ class AppViewModel @Inject constructor( ) } - private suspend fun applyPendingChannelMigration() { - val channelMigration = buildChannelMigrationIfAvailable() ?: return + private suspend fun applyPendingChannelMigration(): Boolean { + val channelMigration = buildChannelMigrationIfAvailable() ?: return true lightningRepo.stop().onFailure { Logger.error("Failed to stop node during remote restore restart", it, context = TAG) } delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) + var applied = false lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) .onSuccess { migrationService.consumePendingChannelMigration() walletRepo.syncNodeAndWallet() walletRepo.syncBalances() + applied = true } .onFailure { e -> Logger.error("Failed to restart node after remote restore", e, context = TAG) } + return applied } private suspend fun completeMigration() { @@ -1579,7 +1585,10 @@ class AppViewModel @Inject constructor( } activityRepo.markAllUnseenActivitiesAsSeen() - applyPendingChannelMigration() + if (!applyPendingChannelMigration()) { + finishMigrationWithError() + return@runCatching + } walletRepo.syncNodeAndWallet() .onSuccess { finishMigrationSuccessfully() } From 34d4e33e447e46ee826ce490368a4513d56afa58 Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Wed, 30 Sep 2026 10:49:14 +0200 Subject: [PATCH 3/9] fix: keep a channel file if the node does not stop A failed stop left the node running, and the following start could succeed without loading the channel file and then discard it. Co-authored-by: Cursor --- app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 6899aae308..0fc426c64c 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1558,6 +1558,10 @@ class AppViewModel @Inject constructor( lightningRepo.stop().onFailure { Logger.error("Failed to stop node during remote restore restart", it, context = TAG) } + if (lightningRepo.lightningState.value.nodeLifecycleState.isRunningOrStarting()) { + Logger.error("Node still running, pending channel migration was not applied", context = TAG) + return false + } delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) var applied = false lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) From e56a39480b285bb61c977e265e3636ceb6b56e43 Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Wed, 30 Sep 2026 11:03:17 +0200 Subject: [PATCH 4/9] test: keep channel file when node stays running Co-authored-by: Cursor --- .../viewmodels/AppViewModelSendFlowTest.kt | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 8e06bb881b..0919b4064f 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -164,6 +164,7 @@ import to.bitkit.services.ActivityService import to.bitkit.services.AppUpdaterService import to.bitkit.services.CoreService import to.bitkit.services.MigrationService +import to.bitkit.services.PendingChannelMigration import to.bitkit.services.NodeServiceFgState import to.bitkit.services.PubkyService import to.bitkit.test.BaseUnitTest @@ -4833,6 +4834,34 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertTrue(settingsData.value.pendingRestoreActivitySeen) } + @Test + fun `pending channel migration is kept when the node stays running after stop`() = test { + val nodeState = MutableStateFlow(LightningState(nodeLifecycleState = NodeLifecycleState.Running)) + whenever(lightningRepo.lightningState).thenReturn(nodeState) + whenever(lightningRepo.getPayments()).thenReturn(Result.success(emptyList())) + whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) + whenever(activityRepo.markAllUnseenActivitiesAsSeen()).thenReturn(Result.success(Unit)) + whenever(migrationService.needsPostMigrationSync()).thenReturn(true) + whenever(migrationService.peekPendingChannelMigration()).thenReturn( + PendingChannelMigration(byteArrayOf(1), listOf(byteArrayOf(2))), + ) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + verify(lightningRepo, never()).start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + any(), + ) + verify(migrationService, never()).consumePendingChannelMigration() + } + @Test fun `onchain sync without a pending restore leaves unseen activities untouched`() = test { emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) From 57bf4241f2f6a3da4107233a8b61ab4a3cb393fe Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Wed, 30 Sep 2026 19:11:43 +0200 Subject: [PATCH 5/9] chore: rename changelog fragment Co-authored-by: Cursor --- changelog.d/next/{1258.fixed.md => 1396.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{1258.fixed.md => 1396.fixed.md} (100%) diff --git a/changelog.d/next/1258.fixed.md b/changelog.d/next/1396.fixed.md similarity index 100% rename from changelog.d/next/1258.fixed.md rename to changelog.d/next/1396.fixed.md From 824ab61cc06bed1e40d037df9df530cd7bd3f588 Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Fri, 9 Oct 2026 10:33:50 +0200 Subject: [PATCH 6/9] fix: separate migration retries from completion --- .../to/bitkit/services/MigrationService.kt | 20 ++-- .../java/to/bitkit/viewmodels/AppViewModel.kt | 99 ++++++++++++------- .../to/bitkit/viewmodels/WalletViewModel.kt | 9 +- .../viewmodels/AppViewModelSendFlowTest.kt | 43 +++++++- 4 files changed, 126 insertions(+), 45 deletions(-) diff --git a/app/src/main/java/to/bitkit/services/MigrationService.kt b/app/src/main/java/to/bitkit/services/MigrationService.kt index cbaf8bc655..5b0edb53d2 100644 --- a/app/src/main/java/to/bitkit/services/MigrationService.kt +++ b/app/src/main/java/to/bitkit/services/MigrationService.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.update +import kotlinx.coroutines.sync.Mutex import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable import kotlinx.serialization.json.JsonObject @@ -112,6 +113,12 @@ class MigrationService @Inject constructor( internal const val BLOCKTANK_ORDER_IDS_CHUNK = 20 } + private val channelMigrationMutex = Mutex() + + suspend fun lockChannelMigration() = channelMigrationMutex.lock() + + fun unlockChannelMigration() = channelMigrationMutex.unlock() + private val rnMigrationStore = context.rnMigrationDataStore private inline fun decodeBackupData(data: ByteArray): T { @@ -318,6 +325,11 @@ class MigrationService @Inject constructor( Logger.debug("Cleared all persisted migration data", context = TAG) } + suspend fun hasPendingMigrationRetries(): Boolean { + loadPersistedMigrationData() + return !canCleanupAfterMigration + } + val canCleanupAfterMigration: Boolean get() { if (pendingBlocktankOrderIds != null || pendingRemotePaidOrders != null) { @@ -1680,7 +1692,7 @@ class MigrationService @Inject constructor( loadPersistedMigrationData() // Handle MMKV (local) migration data - apply activities FIRST, then metadata - if (hasRNMmkvData()) { + if (needsPostMigrationSync() && hasRNMmkvData()) { loadRNMmkvData()?.let { mmkvData -> extractRNActivities(mmkvData)?.let { activities -> Logger.info("Applying ${activities.size} MMKV activities", context = TAG) @@ -1697,12 +1709,6 @@ class MigrationService @Inject constructor( applyBoostTransactions(boosts) } } - - // Apply MMKV metadata (tags) AFTER activities are created - extractRNMetadata(mmkvData)?.let { metadata -> - Logger.info("Applying MMKV metadata (tags: ${metadata.tags?.size})", context = TAG) - applyRNMetadata(metadata) - } } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 272f065cab..31dabb0103 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1841,7 +1841,7 @@ class AppViewModel @Inject constructor( when { (isShowingLoading || needsPostMigrationSync) && !isCompletingMigration -> completeMigration() - isRestoringRemote -> completeRNRemoteBackupRestore() + isRestoringRemote && !isCompletingMigration -> completeRNRemoteBackupRestore() pendingPrune -> { settingsStore.update { it.copy(pendingRestoreAddressTypePrune = false) } delay(POST_RESTORE_PRUNE_DELAY_MS) @@ -1849,7 +1849,10 @@ class AppViewModel @Inject constructor( walletRepo.debounceSyncByEvent() } - !isShowingLoading && !needsPostMigrationSync && !isCompletingMigration -> walletRepo.debounceSyncByEvent() + !isShowingLoading && !needsPostMigrationSync && !isCompletingMigration -> { + retryPendingMigrationData() + walletRepo.debounceSyncByEvent() + } else -> Unit } @@ -1863,6 +1866,17 @@ class AppViewModel @Inject constructor( } } + private suspend fun retryPendingMigrationData() { + isCompletingMigration = true + try { + if (!migrationService.hasPendingMigrationRetries()) return + migrationService.reapplyMetadataAfterSync() + if (migrationService.canCleanupAfterMigration) migrationService.cleanupAfterMigration() + } finally { + isCompletingMigration = false + } + } + private suspend fun completePendingRestoreActivitySeen(syncedBlockHeight: UInt) { // Claimed before the first suspension, so a later sync completing while this sweep runs cannot record its // higher tip and silence a new receive confirmed in between. @@ -1888,23 +1902,29 @@ class AppViewModel @Inject constructor( } private suspend fun completeRNRemoteBackupRestore() { - if (!applyPendingChannelMigration()) { - finishMigrationWithError() - return - } + isCompletingMigration = true + try { + if (!applyPendingChannelMigration()) { + finishMigrationWithError() + return + } - lightningRepo.getPayments().onSuccess { activityRepo.syncLdkNodePayments(it) } - migrationService.reapplyMetadataAfterSync() - activityRepo.syncActivities() - walletRepo.syncBalances() + lightningRepo.getPayments().onSuccess { activityRepo.syncLdkNodePayments(it) } + migrationService.reapplyMetadataAfterSync() + activityRepo.syncActivities() + walletRepo.syncBalances() - if (migrationService.canCleanupAfterMigration) { - migrationService.cleanupAfterMigration() + migrationService.setNeedsPostMigrationSync(false) migrationService.setRestoringFromRNRemoteBackup(false) - migrationService.setShowingMigrationLoading(false) - } else { - Logger.info("Post-migration sync incomplete (remote restore), will retry on next sync", context = TAG) - migrationService.setShowingMigrationLoading(false) + if (migrationService.canCleanupAfterMigration) { + migrationService.cleanupAfterMigration() + migrationService.setShowingMigrationLoading(false) + } else { + Logger.info("Post-migration sync incomplete (remote restore), will retry on next sync", context = TAG) + migrationService.setShowingMigrationLoading(false) + } + } finally { + isCompletingMigration = false } } @@ -1917,27 +1937,32 @@ class AppViewModel @Inject constructor( } private suspend fun applyPendingChannelMigration(): Boolean { - val channelMigration = buildChannelMigrationIfAvailable() ?: return true - lightningRepo.stop().onFailure { - Logger.error("Failed to stop node during remote restore restart", it, context = TAG) - } - if (lightningRepo.lightningState.value.nodeLifecycleState.isRunningOrStarting()) { - Logger.error("Node still running, pending channel migration was not applied", context = TAG) - return false - } - delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) - var applied = false - lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) - .onSuccess { - migrationService.consumePendingChannelMigration() - walletRepo.syncNodeAndWallet() - walletRepo.syncBalances() - applied = true + migrationService.lockChannelMigration() + try { + val channelMigration = buildChannelMigrationIfAvailable() ?: return true + lightningRepo.stop().onFailure { + Logger.error("Failed to stop node during remote restore restart", it, context = TAG) } - .onFailure { e -> - Logger.error("Failed to restart node after remote restore", e, context = TAG) + if (lightningRepo.lightningState.value.nodeLifecycleState.isRunningOrStarting()) { + Logger.error("Node still running, pending channel migration was not applied", context = TAG) + return false } - return applied + delay(REMOTE_RESTORE_NODE_RESTART_DELAY_MS) + var applied = false + lightningRepo.start(channelMigration = channelMigration, shouldRetry = false) + .onSuccess { + migrationService.consumePendingChannelMigration() + walletRepo.syncNodeAndWallet() + walletRepo.syncBalances() + applied = true + } + .onFailure { e -> + Logger.error("Failed to restart node after remote restore", e, context = TAG) + } + return applied + } finally { + migrationService.unlockChannelMigration() + } } private suspend fun completeMigration() { @@ -1978,6 +2003,8 @@ class AppViewModel @Inject constructor( transferRepo.syncTransferStates() migrationService.reapplyMetadataAfterSync() + migrationService.setNeedsPostMigrationSync(false) + migrationService.setRestoringFromRNRemoteBackup(false) if (migrationService.canCleanupAfterMigration) { migrationService.cleanupAfterMigration() migrationService.setShowingMigrationLoading(false) @@ -1997,6 +2024,8 @@ class AppViewModel @Inject constructor( transferRepo.syncTransferStates() migrationService.reapplyMetadataAfterSync() + migrationService.setNeedsPostMigrationSync(false) + migrationService.setRestoringFromRNRemoteBackup(false) if (migrationService.canCleanupAfterMigration) { migrationService.cleanupAfterMigration() migrationService.setShowingMigrationLoading(false) diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index abcc87effc..0366c420d7 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -332,8 +332,13 @@ class WalletViewModel @Inject constructor( waitForRestoreIfNeeded() - val channelMigration = buildChannelMigrationIfAvailable() - startNode(walletIndex, channelMigration) + migrationService.lockChannelMigration() + try { + val channelMigration = buildChannelMigrationIfAvailable() + startNode(walletIndex, channelMigration) + } finally { + migrationService.unlockChannelMigration() + } } finally { isStarting = false } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index ef53b33f5b..7a03726a21 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -174,9 +174,9 @@ import to.bitkit.services.ActivityService import to.bitkit.services.AppUpdaterService import to.bitkit.services.CoreService import to.bitkit.services.MigrationService -import to.bitkit.services.PendingChannelMigration import to.bitkit.services.NodeServiceFgState import to.bitkit.services.PaykitSdkOperationLock.Priority +import to.bitkit.services.PendingChannelMigration import to.bitkit.services.PubkyService import to.bitkit.test.BaseUnitTest import to.bitkit.ui.Routes @@ -6189,6 +6189,47 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertTrue(settingsData.value.pendingRestoreActivitySeen) } + @Test + fun `background tag retry does not sweep new activities or restart the node`() = test { + whenever(migrationService.isRestoringFromRNRemoteBackup).thenReturn(MutableStateFlow(false)) + whenever(migrationService.needsPostMigrationSync()).thenReturn(false) + whenever(migrationService.hasPendingMigrationRetries()).thenReturn(true) + whenever(migrationService.canCleanupAfterMigration).thenReturn(false) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + verify(migrationService).reapplyMetadataAfterSync() + verify(activityRepo, never()).markAllUnseenActivitiesAsSeen(anyOrNull()) + verify(lightningRepo, never()).stop() + verify(migrationService, never()).cleanupAfterMigration() + } + + @Test + fun `migration completion rechecks pending channels after startup releases the lock`() = test { + whenever(migrationService.isRestoringFromRNRemoteBackup).thenReturn(MutableStateFlow(false)) + val startup = CompletableDeferred() + whenever(migrationService.lockChannelMigration()).doSuspendableAnswer { startup.await() } + whenever(migrationService.needsPostMigrationSync()).thenReturn(true) + whenever(lightningRepo.getPayments()).thenReturn(Result.success(emptyList())) + whenever(activityRepo.markAllUnseenActivitiesAsSeen()).thenReturn(Result.success(Unit)) + whenever(migrationService.peekPendingChannelMigration()).thenReturn( + PendingChannelMigration(byteArrayOf(1), listOf(byteArrayOf(2))), + ) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + runCurrent() + verify(lightningRepo, never()).stop() + whenever(migrationService.peekPendingChannelMigration()).thenReturn(null) + whenever(walletRepo.syncNodeAndWallet()).thenReturn(Result.success(Unit)) + startup.complete(Unit) + advanceUntilIdle() + + verify(lightningRepo, never()).stop() + verify(migrationService).unlockChannelMigration() + verify(migrationService).setNeedsPostMigrationSync(false) + } + @Test fun `pending channel migration is kept when the node stays running after stop`() = test { val nodeState = MutableStateFlow(LightningState(nodeLifecycleState = NodeLifecycleState.Running)) From 10f14359f25ff97f4b52588996b7c7e6bdfdb6bb Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Fri, 9 Oct 2026 10:59:37 +0200 Subject: [PATCH 7/9] fix: retain unfinished migration markers --- .../to/bitkit/services/MigrationService.kt | 37 +++++---- .../bitkit/services/MigrationServiceTest.kt | 76 +++++++++++++++++++ 2 files changed, 99 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/to/bitkit/services/MigrationService.kt b/app/src/main/java/to/bitkit/services/MigrationService.kt index 5b0edb53d2..ebbc5a81c3 100644 --- a/app/src/main/java/to/bitkit/services/MigrationService.kt +++ b/app/src/main/java/to/bitkit/services/MigrationService.kt @@ -48,6 +48,7 @@ import to.bitkit.data.keychain.Keychain import to.bitkit.data.resetPin import to.bitkit.di.json import to.bitkit.env.Env +import to.bitkit.ext.runSuspendCatching import to.bitkit.models.BitcoinDisplayUnit import to.bitkit.models.CoinSelectionPreference import to.bitkit.models.DEFAULT_ADDRESS_TYPE_STRING @@ -1702,11 +1703,11 @@ class MigrationService @Inject constructor( extractRNWalletBackup(mmkvData)?.let { (transfers, boosts) -> if (transfers.isNotEmpty()) { Logger.info("Applying ${transfers.size} local transfer markers", context = TAG) - applyRemoteTransfers(transfers) + persistTransfers(pendingRemoteTransfers.orEmpty() + transfers) } if (boosts.isNotEmpty()) { Logger.info("Applying ${boosts.size} local boost markers", context = TAG) - applyBoostTransactions(boosts) + persistBoosts(pendingRemoteBoosts.orEmpty() + boosts) } } } @@ -1721,14 +1722,14 @@ class MigrationService @Inject constructor( pendingRemoteTransfers?.let { transfers -> Logger.info("Applying ${transfers.size} remote transfer markers", context = TAG) - applyRemoteTransfers(transfers) - clearPersistedTransfers() + val remaining = applyRemoteTransfers(transfers) + if (remaining.isEmpty()) clearPersistedTransfers() else persistTransfers(remaining) } pendingRemoteBoosts?.let { boosts -> Logger.info("Applying ${boosts.size} remote boost markers", context = TAG) - applyBoostTransactions(boosts) - clearPersistedBoosts() + val remaining = applyBoostTransactions(boosts) + if (remaining.isEmpty()) clearPersistedBoosts() else persistBoosts(remaining) } // Apply remote metadata (tags) AFTER activities are created @@ -1793,15 +1794,20 @@ class MigrationService @Inject constructor( } } - private suspend fun applyRemoteTransfers(transfers: Map) { + internal suspend fun applyRemoteTransfers(transfers: Map): Map { + val remaining = transfers.toMutableMap() transfers.forEach { (txId, channelId) -> val onchain = activityRepo.getOnchainActivityByTxId(txId) ?: return@forEach val updated = onchain.copy(isTransfer = true, channelId = channelId) - activityRepo.updateActivity(onchain.id, Activity.Onchain(updated)) + activityRepo.updateActivity(onchain.id, Activity.Onchain(updated)).onSuccess { + remaining.remove(txId) + } } + return remaining } - private suspend fun applyBoostTransactions(boosts: Map) { + internal suspend fun applyBoostTransactions(boosts: Map): Map { + val remaining = boosts.toMutableMap() var applied = 0 boosts.forEach { (oldTxId, newTxId) -> @@ -1825,9 +1831,10 @@ class MigrationService @Inject constructor( boostTxIds = newOnchain.boostTxIds.filter { it != oldTxId }, ) - runCatching { - activityRepo.updateActivity(parentOnchain.id, Activity.Onchain(parentOnchain)) - activityRepo.updateActivity(updatedNewOnchain.id, Activity.Onchain(updatedNewOnchain)) + runSuspendCatching { + activityRepo.updateActivity(parentOnchain.id, Activity.Onchain(parentOnchain)).getOrThrow() + activityRepo.updateActivity(updatedNewOnchain.id, Activity.Onchain(updatedNewOnchain)).getOrThrow() + remaining.remove(oldTxId) applied++ }.onFailure { e -> Logger.error( @@ -1847,8 +1854,9 @@ class MigrationService @Inject constructor( boostTxIds = updatedBoostTxIds, ) - runCatching { - activityRepo.updateActivity(updated.id, Activity.Onchain(updated)) + runSuspendCatching { + activityRepo.updateActivity(updated.id, Activity.Onchain(updated)).getOrThrow() + remaining.remove(oldTxId) applied++ }.onFailure { e -> Logger.error("Failed to apply RBF boost for tx $newTxId: $e", e, context = TAG) @@ -1857,6 +1865,7 @@ class MigrationService @Inject constructor( } Logger.info("Applied $applied/${boosts.size} boost markers", context = TAG) + return remaining } private suspend fun applyBoostedParents(boostedParents: List, txId: String) { diff --git a/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt b/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt index 6d28f8cc57..ec2eb68d8c 100644 --- a/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/MigrationServiceTest.kt @@ -1,16 +1,92 @@ package to.bitkit.services +import android.content.Context +import com.synonym.bitkitcore.Activity import com.synonym.bitkitcore.IBtOrder +import com.synonym.bitkitcore.OnchainActivity +import com.synonym.bitkitcore.PaymentType import kotlinx.coroutines.test.runTest import org.junit.Test +import org.mockito.kotlin.any import org.mockito.kotlin.doReturn import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import to.bitkit.ext.create +import to.bitkit.repositories.ActivityRepo import to.bitkit.test.BaseUnitTest +import to.bitkit.utils.AppError import kotlin.test.assertEquals import kotlin.test.assertTrue class MigrationServiceTest : BaseUnitTest() { + @Test + fun `missing transfer markers remain pending until their activity exists`() = test { + val activityRepo = mock() + val service = createService(activityRepo) + val markers = mapOf("transfer" to "channel") + + val pending = service.applyRemoteTransfers(markers) + assertEquals(markers, pending) + verify(activityRepo, never()).updateActivity(any(), any(), any()) + + val activity = onchain("transfer") + whenever(activityRepo.getOnchainActivityByTxId("transfer")).thenReturn(activity) + whenever(activityRepo.updateActivity(any(), any(), any())).thenReturn(Result.success(Unit)) + assertTrue(service.applyRemoteTransfers(pending).isEmpty()) + verify(activityRepo).updateActivity( + "transfer", + Activity.Onchain(activity.copy(isTransfer = true, channelId = "channel")), + ) + } + + @Test + fun `missing boost markers remain pending until their activities exist`() = test { + val activityRepo = mock() + val service = createService(activityRepo) + val markers = mapOf("parent" to "child") + val pending = service.applyBoostTransactions(markers) + assertEquals(markers, pending) + + whenever(activityRepo.getOnchainActivityByTxId("parent")).thenReturn(onchain("parent")) + whenever(activityRepo.getOnchainActivityByTxId("child")).thenReturn(onchain("child")) + whenever(activityRepo.updateActivity(any(), any(), any())).thenReturn(Result.success(Unit)) + assertTrue(service.applyBoostTransactions(pending).isEmpty()) + } + + @Test + fun `failed transfer and boost writes remain pending`() = test { + val activityRepo = mock() + val service = createService(activityRepo) + whenever(activityRepo.getOnchainActivityByTxId(any(), any())).thenReturn(onchain("activity")) + whenever(activityRepo.updateActivity(any(), any(), any())).thenReturn(Result.failure(AppError("write failed"))) + + val transfer = mapOf("transfer" to "channel") + val boost = mapOf("parent" to "child") + assertEquals(transfer, service.applyRemoteTransfers(transfer)) + assertEquals(boost, service.applyBoostTransactions(boost)) + } + + private fun createService(activityRepo: ActivityRepo): MigrationService { + val context = mock() + whenever(context.applicationContext).thenReturn(context) + return MigrationService(context, mock(), mock(), mock(), mock(), activityRepo, mock(), mock(), mock(), mock()) + } + + private fun onchain(id: String) = OnchainActivity.create( + walletId = "wallet0", + id = id, + txType = PaymentType.SENT, + txId = id, + value = 1000uL, + fee = 100uL, + feeRate = 1uL, + address = "address", + timestamp = 1uL, + ) + @Test fun `fetchOrdersInChunks should split ids into chunks and concatenate results`() = runTest { val orderIds = (1..25).map { "order$it" } From 4b82c1d9656c7d6904729755b46f3768cf90e398 Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Fri, 9 Oct 2026 11:34:07 +0200 Subject: [PATCH 8/9] fix: preserve channels through restore retries --- .../to/bitkit/viewmodels/WalletViewModel.kt | 26 +++- .../java/to/bitkit/ui/WalletViewModelTest.kt | 111 ++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 0366c420d7..baab3876f1 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -181,8 +181,13 @@ class WalletViewModel @Inject constructor( pendingWalletStart = true return@launch } - val channelMigration = buildChannelMigrationIfAvailable() - startNode(0, channelMigration) + migrationService.lockChannelMigration() + try { + val channelMigration = buildChannelMigrationIfAvailable() + startNode(0, channelMigration) + } finally { + migrationService.unlockChannelMigration() + } } else { migrationService.setShowingMigrationLoading(false) } @@ -289,7 +294,22 @@ class WalletViewModel @Inject constructor( fun onRestoreRetry() = viewModelScope.launch(bgDispatcher) { _restoreState.update { it.countRetry() } setInitNodeLifecycleState() - lightningRepo.restartNode() + migrationService.lockChannelMigration() + try { + val channelMigration = buildChannelMigrationIfAvailable() + if (channelMigration == null) { + lightningRepo.restartNode() + } else { + lightningRepo.stop().onSuccess { + startNode(0, channelMigration) + }.onFailure { + Logger.error("Failed to stop node during restore retry", it, context = TAG) + ToastEventBus.send(it) + } + } + } finally { + migrationService.unlockChannelMigration() + } } fun onBackupRestoreRetry() = viewModelScope.launch(bgDispatcher) { diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 2c8575bc47..c05b533ef2 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -11,9 +11,11 @@ import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import org.junit.Before import org.junit.Test +import org.lightningdevkit.ldknode.ChannelDataMigration import org.lightningdevkit.ldknode.PeerDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock @@ -39,6 +41,7 @@ import to.bitkit.repositories.WalletRepo import to.bitkit.repositories.WalletState import to.bitkit.services.BoltzService import to.bitkit.services.MigrationService +import to.bitkit.services.PendingChannelMigration import to.bitkit.test.BaseUnitTest import to.bitkit.utils.AppError import to.bitkit.viewmodels.RestoreState @@ -706,6 +709,114 @@ class WalletViewModelTest : BaseUnitTest() { verifyNodeStarted(testLightningRepo) } + @Test + fun `restore retry rebuilds the node with the pending channel file`() = test { + val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) + whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) + whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) + stubSuccessfulNodeStart(lightningRepo) + + sut.onRestoreRetry() + advanceUntilIdle() + + verify(lightningRepo, never()).restartNode() + val migration = argumentCaptor() + verify(lightningRepo).start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + migration.capture(), + any(), + ) + assertEquals(listOf(1.toUByte()), migration.firstValue.channelManager) + assertEquals(listOf(listOf(2.toUByte())), migration.firstValue.channelMonitors) + inOrder(migrationService) { + verify(migrationService).lockChannelMigration() + verify(migrationService).peekPendingChannelMigration() + verify(migrationService).consumePendingChannelMigration() + verify(migrationService).unlockChannelMigration() + } + } + + @Test + fun `restore retry keeps pending channels when stop fails`() = test { + val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) + whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) + whenever(lightningRepo.stop()).thenReturn(Result.failure(AppError("stop failed"))) + + sut.onRestoreRetry() + advanceUntilIdle() + + verifyNodeNotStarted(lightningRepo) + verify(lightningRepo, never()).restartNode() + verify(migrationService, never()).consumePendingChannelMigration() + verify(migrationService).unlockChannelMigration() + } + + @Test + fun `restore retry keeps pending channels when start fails`() = test { + val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) + whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) + whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) + whenever( + lightningRepo.start( + any(), + anyOrNull(), + any(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + any(), + ), + ).thenReturn(Result.failure(AppError("start failed"))) + + sut.onRestoreRetry() + advanceUntilIdle() + + verifyNodeStarted(lightningRepo) + verify(lightningRepo, never()).restartNode() + verify(migrationService, never()).consumePendingChannelMigration() + verify(migrationService).unlockChannelMigration() + } + + @Test + fun `rn local migration start holds the channel migration lock until consumption`() = test { + advanceUntilIdle() + val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) + whenever(migrationService.isMigrationChecked()).thenReturn(false) + whenever(migrationService.hasRNWalletData()).thenReturn(true) + whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) + whenever(walletRepo.walletExists()).thenReturn(true) + stubSuccessfulNodeStart(lightningRepo) + + WalletViewModel( + context = context, + bgDispatcher = testDispatcher, + walletRepo = walletRepo, + lightningRepo = lightningRepo, + settingsStore = settingsStore, + backupRepo = backupRepo, + blocktankRepo = blocktankRepo, + pubkyRepo = pubkyRepo, + migrationService = migrationService, + connectivityRepo = connectivityRepo, + boltzService = boltzService, + ) + advanceUntilIdle() + + verifyNodeStarted(lightningRepo) + inOrder(migrationService) { + verify(migrationService).lockChannelMigration() + verify(migrationService).peekPendingChannelMigration() + verify(migrationService).consumePendingChannelMigration() + verify(migrationService).unlockChannelMigration() + } + } + private suspend fun stubSuccessfulNodeStart(repo: LightningRepo) { whenever( repo.start( From eab911c87c1ef918a19c52a2cca49ee91c851c1e Mon Sep 17 00:00:00 2001 From: Piotr Stachyra Date: Fri, 9 Oct 2026 11:54:40 +0200 Subject: [PATCH 9/9] fix: preserve background stops during restore retry --- .../to/bitkit/repositories/LightningRepo.kt | 4 +- .../to/bitkit/viewmodels/WalletViewModel.kt | 18 +++-- .../bitkit/repositories/LightningRepoTest.kt | 79 +++++++++++++++++++ .../java/to/bitkit/ui/WalletViewModelTest.kt | 45 +++-------- 4 files changed, 104 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index 2ac1ab04d6..6c5297b164 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt @@ -2204,13 +2204,13 @@ class LightningRepo @Inject constructor( */ suspend fun restartNodeDetached(): Result = scope.async { restartNode() }.await() - suspend fun restartNode(): Result = withContext(bgDispatcher) { + suspend fun restartNode(channelMigration: ChannelDataMigration? = null): Result = withContext(bgDispatcher) { Logger.info("Restarting node", context = TAG) stop().onFailure { Logger.error("Failed to stop node during restart", it, context = TAG) return@withContext Result.failure(it) } - startNode(shouldRetryYieldToStop = true).onFailure { + startNode(channelMigration = channelMigration, shouldRetryYieldToStop = true).onFailure { if (it is NodeStartYieldedToStopError) { Logger.info("Deferred node restart to a requested stop", context = TAG) } else { diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index baab3876f1..a64e01ea46 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -39,6 +39,7 @@ import to.bitkit.repositories.BlocktankRepo import to.bitkit.repositories.ConnectivityRepo import to.bitkit.repositories.ConnectivityState import to.bitkit.repositories.LightningRepo +import to.bitkit.repositories.NodeStartYieldedToStopError import to.bitkit.repositories.PubkyRepo import to.bitkit.repositories.RecoveryModeError import to.bitkit.repositories.SyncSource @@ -300,12 +301,7 @@ class WalletViewModel @Inject constructor( if (channelMigration == null) { lightningRepo.restartNode() } else { - lightningRepo.stop().onSuccess { - startNode(0, channelMigration) - }.onFailure { - Logger.error("Failed to stop node during restore retry", it, context = TAG) - ToastEventBus.send(it) - } + startNode(0, channelMigration, isRestoreRetry = true) } } finally { migrationService.unlockChannelMigration() @@ -390,8 +386,14 @@ class WalletViewModel @Inject constructor( private suspend fun startNode( walletIndex: Int = 0, channelMigration: ChannelDataMigration?, + isRestoreRetry: Boolean = false, ) { - lightningRepo.start(walletIndex, channelMigration = channelMigration) + val result = if (isRestoreRetry) { + lightningRepo.restartNode(channelMigration) + } else { + lightningRepo.start(walletIndex, channelMigration = channelMigration) + } + result .onSuccess { if (channelMigration != null) { migrationService.consumePendingChannelMigration() @@ -407,7 +409,7 @@ class WalletViewModel @Inject constructor( // checkForOrphanedChannelMonitorRecovery() } .onFailure { - if (it is RecoveryModeError || it is WipeInProgressError) { + if (it is RecoveryModeError || it is WipeInProgressError || it is NodeStartYieldedToStopError) { Logger.debug("Skipped node start: '${it.message}'", context = TAG) return@onFailure } diff --git a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt index e76eb0c097..a83b903709 100644 --- a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt @@ -29,6 +29,7 @@ import org.junit.Before import org.junit.Test import org.lightningdevkit.ldknode.AddressTypeBalance import org.lightningdevkit.ldknode.BalanceDetails +import org.lightningdevkit.ldknode.ChannelDataMigration import org.lightningdevkit.ldknode.ChannelDetails import org.lightningdevkit.ldknode.Event import org.lightningdevkit.ldknode.Node @@ -741,6 +742,41 @@ class LightningRepoTest : BaseUnitTest() { verifyBlocking(lightningService, times(2)) { start(anyOrNull(), any()) } } + @Test + fun `channel migration restart skips the start retry when a stop is requested during the failed attempt`() = test { + stubNodeForRestart() + val error = AppError("Feerate estimation update timeout") + var attempts = 0 + whenever(lightningService.start(anyOrNull(), any())).thenAnswer { + attempts++ + if (attempts == 1) { + sut.stopDebounced() + throw error + } + Unit + } + + val result = sut.restartNode(ChannelDataMigration(listOf(1.toUByte()), listOf(listOf(2.toUByte())))) + + val yieldError = assertIs(result.exceptionOrNull()) + assertEquals(error, yieldError.cause) + assertEquals(NodeLifecycleState.Stopped, sut.lightningState.value.nodeLifecycleState) + verifyBlocking(lightningService, times(1)) { start(anyOrNull(), any()) } + verify(lightningService, times(1)).stop() + + testScheduler.advanceUntilIdle() + + verify(lightningService, times(2)).stop() + verifyBlocking(lightningService, times(1)) { start(anyOrNull(), any()) } + assertEquals(NodeLifecycleState.Stopped, sut.lightningState.value.nodeLifecycleState) + + val foregroundResult = sut.start() + + assertTrue(foregroundResult.isSuccess) + assertEquals(NodeLifecycleState.Running, sut.lightningState.value.nodeLifecycleState) + verifyBlocking(lightningService, times(2)) { start(anyOrNull(), any()) } + } + @Test fun `restartNode retries when a foreground cancels the stop requested during the failed attempt`() = test { stubNodeForRestart() @@ -794,6 +830,49 @@ class LightningRepoTest : BaseUnitTest() { assertEquals(NodeLifecycleState.Stopped, sut.lightningState.value.nodeLifecycleState) } + @Test + fun `channel migration restart keeps the backup across bounded start attempts`() = test { + stubNodeForRestart() + whenever(lightningService.node).thenReturn(null) + val migration = ChannelDataMigration(listOf(1.toUByte()), listOf(listOf(2.toUByte()))) + var attempts = 0 + whenever(lightningService.start(anyOrNull(), any())).thenAnswer { + attempts++ + if (attempts == 1) throw AppError("transient start failure") + Unit + } + + val result = sut.restartNode(migration) + + assertTrue(result.isSuccess) + verifyBlocking(lightningService, times(2)) { + setup(any(), anyOrNull(), anyOrNull(), anyOrNull(), eq(migration)) + } + assertEquals(NodeLifecycleState.Running, sut.lightningState.value.nodeLifecycleState) + } + + @Test + fun `channel migration restart retry does not cancel a stop requested during the retry`() = test { + stubNodeForRestart() + var attempts = 0 + whenever(lightningService.start(anyOrNull(), any())).thenAnswer { + attempts++ + if (attempts == 1) throw AppError("Feerate estimation update timeout") + sut.stopDebounced() + Unit + } + + val result = sut.restartNode(ChannelDataMigration(listOf(1.toUByte()), listOf(listOf(2.toUByte())))) + + assertTrue(result.isSuccess) + verifyBlocking(lightningService, times(2)) { start(anyOrNull(), any()) } + + testScheduler.advanceUntilIdle() + + verify(lightningService, times(2)).stop() + assertEquals(NodeLifecycleState.Stopped, sut.lightningState.value.nodeLifecycleState) + } + // Regression #1125: a caller cancelled inside the retry delay must not drop the bounded retry @Test fun `restartNodeDetached completes the start retry after the caller is cancelled`() = test { diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index c05b533ef2..942b155b75 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -35,6 +35,7 @@ import to.bitkit.repositories.ConnectivityRepo import to.bitkit.repositories.ConnectivityState import to.bitkit.repositories.LightningRepo import to.bitkit.repositories.LightningState +import to.bitkit.repositories.NodeStartYieldedToStopError import to.bitkit.repositories.PubkyRepo import to.bitkit.repositories.SyncSource import to.bitkit.repositories.WalletRepo @@ -713,24 +714,14 @@ class WalletViewModelTest : BaseUnitTest() { fun `restore retry rebuilds the node with the pending channel file`() = test { val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) - whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) - stubSuccessfulNodeStart(lightningRepo) + whenever(lightningRepo.restartNode(anyOrNull())).thenReturn(Result.success(Unit)) sut.onRestoreRetry() advanceUntilIdle() - verify(lightningRepo, never()).restartNode() + verifyNodeNotStarted(lightningRepo) val migration = argumentCaptor() - verify(lightningRepo).start( - any(), - anyOrNull(), - any(), - anyOrNull(), - anyOrNull(), - anyOrNull(), - migration.capture(), - any(), - ) + verify(lightningRepo).restartNode(migration.capture()) assertEquals(listOf(1.toUByte()), migration.firstValue.channelManager) assertEquals(listOf(listOf(2.toUByte())), migration.firstValue.channelMonitors) inOrder(migrationService) { @@ -742,43 +733,33 @@ class WalletViewModelTest : BaseUnitTest() { } @Test - fun `restore retry keeps pending channels when stop fails`() = test { + fun `restore retry keeps pending channels when restart fails`() = test { val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) - whenever(lightningRepo.stop()).thenReturn(Result.failure(AppError("stop failed"))) + whenever(lightningRepo.restartNode(anyOrNull())).thenReturn(Result.failure(AppError("restart failed"))) sut.onRestoreRetry() advanceUntilIdle() verifyNodeNotStarted(lightningRepo) - verify(lightningRepo, never()).restartNode() + verify(lightningRepo).restartNode(anyOrNull()) verify(migrationService, never()).consumePendingChannelMigration() verify(migrationService).unlockChannelMigration() } @Test - fun `restore retry keeps pending channels when start fails`() = test { + fun `restore retry keeps pending channels when restart yields to background stop`() = test { val pending = PendingChannelMigration(channelManager = byteArrayOf(1), channelMonitors = listOf(byteArrayOf(2))) whenever(migrationService.peekPendingChannelMigration()).thenReturn(pending) - whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) - whenever( - lightningRepo.start( - any(), - anyOrNull(), - any(), - anyOrNull(), - anyOrNull(), - anyOrNull(), - anyOrNull(), - any(), - ), - ).thenReturn(Result.failure(AppError("start failed"))) + whenever(lightningRepo.restartNode(anyOrNull())).thenReturn( + Result.failure(NodeStartYieldedToStopError(AppError("start failed"))), + ) sut.onRestoreRetry() advanceUntilIdle() - verifyNodeStarted(lightningRepo) - verify(lightningRepo, never()).restartNode() + verifyNodeNotStarted(lightningRepo) + verify(lightningRepo).restartNode(anyOrNull()) verify(migrationService, never()).consumePendingChannelMigration() verify(migrationService).unlockChannelMigration() }