From 2bb36af4bbc03ccb9a3a7bcdbd08dad0338d9098 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:23:17 +0200 Subject: [PATCH 1/3] fix: guard wallet operations while required restore is pending --- .../onboarding/WalletRestoreErrorViewTest.kt | 64 +++++++++++ .../data/keychain/WalletRestoreState.kt | 14 +++ .../java/to/bitkit/repositories/BackupRepo.kt | 3 +- .../to/bitkit/repositories/LightningRepo.kt | 62 +++++++++-- .../to/bitkit/services/LightningService.kt | 12 ++ app/src/main/java/to/bitkit/ui/ContentView.kt | 3 + .../ui/onboarding/WalletRestoreErrorView.kt | 5 +- .../to/bitkit/viewmodels/WalletViewModel.kt | 12 +- .../bitkit/repositories/LightningRepoTest.kt | 104 ++++++++++++++++++ .../bitkit/services/LightningServiceTest.kt | 47 +++++++- .../java/to/bitkit/ui/WalletViewModelTest.kt | 85 +++++++++++++- .../next/pending-wallet-restore.fixed.md | 1 + .../pending-wallet-recovery.xml | 21 ++++ 13 files changed, 410 insertions(+), 23 deletions(-) create mode 100644 app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt create mode 100644 app/src/main/java/to/bitkit/data/keychain/WalletRestoreState.kt create mode 100644 changelog.d/next/pending-wallet-restore.fixed.md create mode 100644 journeys/backup-restore/pending-wallet-recovery.xml diff --git a/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt b/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt new file mode 100644 index 0000000000..3e0ac7f8fa --- /dev/null +++ b/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt @@ -0,0 +1,64 @@ +package to.bitkit.ui.onboarding + +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.platform.LocalInspectionMode +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import org.junit.Rule +import org.junit.Test +import to.bitkit.test.annotations.ComposeUi +import to.bitkit.ui.theme.AppThemeSurface +import kotlin.test.assertEquals + +@ComposeUi +class WalletRestoreErrorViewTest { + @get:Rule + val composeTestRule = createComposeRule() + + @Test + fun pendingRecoveryOnlyOffersRetryAfterRepeatedFailures() { + var retries = 0 + composeTestRule.setContent { + AppThemeSurface { + CompositionLocalProvider(LocalInspectionMode provides true) { + WalletRestoreErrorView( + retryCount = 3, + allowProceedWithoutRestore = false, + onRetry = { retries++ }, + onProceedWithoutRestore = { error("Pending recovery must not be dismissed") }, + ) + } + } + } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + composeTestRule.onNodeWithTag("TryAgainButton").assertIsDisplayed().performClick() + composeTestRule.runOnIdle { assertEquals(1, retries) } + } + + @Test + fun pendingRecoveryRemovesAnAlreadyOpenProceedDialog() { + var allowed by mutableStateOf(true) + composeTestRule.setContent { + AppThemeSurface { + CompositionLocalProvider(LocalInspectionMode provides true) { + WalletRestoreErrorView( + retryCount = 3, + allowProceedWithoutRestore = allowed, + onRetry = {}, + onProceedWithoutRestore = { error("Pending recovery must not be dismissed") }, + ) + } + } + } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").performClick() + composeTestRule.onNodeWithTag("ProceedWithoutBackupDialog").assertIsDisplayed() + composeTestRule.runOnIdle { allowed = false } + composeTestRule.onNodeWithTag("ProceedWithoutBackupDialog").assertDoesNotExist() + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + } +} diff --git a/app/src/main/java/to/bitkit/data/keychain/WalletRestoreState.kt b/app/src/main/java/to/bitkit/data/keychain/WalletRestoreState.kt new file mode 100644 index 0000000000..27e3d08a1e --- /dev/null +++ b/app/src/main/java/to/bitkit/data/keychain/WalletRestoreState.kt @@ -0,0 +1,14 @@ +package to.bitkit.data.keychain + +import to.bitkit.utils.AppError + +internal fun Keychain.hasPendingWalletRestore(): Boolean = + exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name) || exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name) + +internal fun Keychain.requireWalletRestoreComplete() { + if (hasPendingWalletRestore()) throw WalletBackupRestorePendingError() +} + +class WalletBackupRestorePendingError : AppError( + "Wallet backup restoration is incomplete. Retry the restore before continuing." +) diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index 55e88e56c1..dabef786d4 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -48,6 +48,7 @@ import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.entities.TransferEntity import to.bitkit.data.hwWalletNames import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.hasPendingWalletRestore import to.bitkit.data.resetPin import to.bitkit.di.IoDispatcher import to.bitkit.di.json @@ -186,7 +187,7 @@ class BackupRepo @Inject constructor( } suspend fun hasPendingWalletRestore(): Boolean = withContext(ioDispatcher) { - keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name) || isUsdtRestorePending() + keychain.hasPendingWalletRestore() } private fun currentTimeMillis(): Long = nowMillis(clock) diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index d1752edce9..b6b0a7805e 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt @@ -66,6 +66,7 @@ import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.backup.VssBackupClientLdk import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.requireWalletRestoreComplete import to.bitkit.di.BgDispatcher import to.bitkit.env.Defaults import to.bitkit.env.Env @@ -284,6 +285,10 @@ class LightningRepo @Inject constructor( return@withContext executeOperation(operationName, operation) } + private fun requireWalletRestoreComplete() { + keychain.requireWalletRestoreComplete() + } + private suspend fun executeOperation( operationName: String, operation: suspend () -> Result, @@ -1274,16 +1279,19 @@ class LightningRepo @Inject constructor( } suspend fun newAddress(): Result = executeWhenNodeRunning("newAddress") { + requireWalletRestoreComplete() runCatching { lightningService.newAddress() } } suspend fun newAddressForType(addressType: AddressType): Result = executeWhenNodeRunning("newAddressForType") { + requireWalletRestoreComplete() runCatching { lightningService.newAddressForType(addressType) } } suspend fun newAddressInfoForType(addressType: AddressType): Result = executeWhenNodeRunning("newAddressInfoForType") { + requireWalletRestoreComplete() runSuspendCatching { lightningService.newAddressInfoForType(addressType) } } @@ -1312,7 +1320,9 @@ class LightningRepo @Inject constructor( description: String, expirySeconds: UInt = Defaults.bolt11ExpirySec, ): Result = executeWhenNodeRunning("createInvoice") { + requireWalletRestoreComplete() updateGeoBlockState() + requireWalletRestoreComplete() runCatching { lightningService.receive(amountSats, description, expirySeconds) } } @@ -1321,7 +1331,9 @@ class LightningRepo @Inject constructor( description: String, expirySeconds: UInt = Defaults.bolt11ExpirySec, ): Result = executeWhenNodeRunning("createInvoiceMsats") { + requireWalletRestoreComplete() updateGeoBlockState() + requireWalletRestoreComplete() runCatching { lightningService.receiveMsats(amountMsats, description, expirySeconds) } } @@ -1350,6 +1362,7 @@ class LightningRepo @Inject constructor( callback: String, paymentRequest: String, ): Result = executeWhenNodeRunning("requestLnurlWithdraw") { + requireWalletRestoreComplete() val callbackUrl = createWithdrawCallbackUrl(k1, callback, paymentRequest) Logger.debug("handleLnurlWithdraw callbackUrl generated: '$callbackUrl'", context = TAG) lnurlService.requestLnurlWithdraw(callbackUrl) @@ -1360,6 +1373,7 @@ class LightningRepo @Inject constructor( callback: String, nodeId: String, ): Result = executeWhenNodeRunning("requestLnurlChannel") { + requireWalletRestoreComplete() val url = createChannelRequestUrl( k1 = k1, callback = callback, @@ -1417,8 +1431,11 @@ class LightningRepo @Inject constructor( maxTotalRoutingFeeMsat: ULong?, onBeforeSend: suspend () -> Boolean, ): Result = executeWhenNodeRunning("payInvoice") { + requireWalletRestoreComplete() waitForUsableChannels() + requireWalletRestoreComplete() if (!onBeforeSend()) return@executeWhenNodeRunning Result.failure(PaymentAbortedBeforeSend()) + requireWalletRestoreComplete() runCatching { lightningService.send(bolt11, sats, paymentDeadlineAt, maxTotalRoutingFeeMsat) }.also { syncState() } @@ -1519,6 +1536,7 @@ class LightningRepo @Inject constructor( paymentDeadlineAt: Instant? = null, contactPublicKey: String? = null, ): Result = executeWhenNodeRunning("sendOnChain") { + requireWalletRestoreComplete() require(address.isNotEmpty()) { "Send address cannot be empty" } // Recovery must remain reachable when observing the original transaction makes sync fail. @@ -1551,6 +1569,7 @@ class LightningRepo @Inject constructor( Logger.debug("UTXOs selected to spend: $utxosForSend", context = TAG) val attempt = runSuspendCatching { + requireWalletRestoreComplete() onchainSendAttemptStore.admit( walletId = WalletScope.default, requestId = requestId, @@ -1577,6 +1596,7 @@ class LightningRepo @Inject constructor( } val preparedResult = try { runSuspendCatching { + requireWalletRestoreComplete() lightningService.prepareOnchainSend( address, sats, @@ -1625,7 +1645,9 @@ class LightningRepo @Inject constructor( prepared.receipt.txid, broadcast = { beforeDispatch -> prepared.broadcast { + requireWalletRestoreComplete() beforeDispatch() + requireWalletRestoreComplete() dispatchStarted = true } }, @@ -1717,14 +1739,17 @@ class LightningRepo @Inject constructor( val selected = inputs.map { input -> SpendableUtxo(OutPoint(input.txid, input.vout), 0uL) } - return lightningService.prepareOnchainSend( - attempt.address, - attempt.amountSats, - feeRateSatsPerVByte, - selected, - false, - attempt.walletIndex, - paymentDeadlineAt, + requireWalletRestoreComplete() + return guardPreparedBroadcast( + lightningService.prepareOnchainSend( + attempt.address, + attempt.amountSats, + feeRateSatsPerVByte, + selected, + false, + attempt.walletIndex, + paymentDeadlineAt, + ) ) } }, @@ -1740,6 +1765,7 @@ class LightningRepo @Inject constructor( approvePrepared: suspend (OnchainPreparedReceipt) -> Unit = {}, authorizeOriginal: suspend (OnchainSendAttempt) -> Unit, ): Result = executeWhenNodeRunning("retryOriginalOnchainSend") { + requireWalletRestoreComplete() val result = recoveryCoordinator.retryOriginal( attemptId, walletId, @@ -1913,8 +1939,20 @@ class LightningRepo @Inject constructor( override suspend fun originalCoinSpentElsewhere(attempt: OnchainSendAttempt): Boolean = lightningService.originalCoinSpentElsewhere(attempt) - override suspend fun prepareRelease(attempt: OnchainSendAttempt): PreparedOnchainSend = - lightningService.prepareRelease(attempt) + override suspend fun prepareRelease(attempt: OnchainSendAttempt): PreparedOnchainSend { + requireWalletRestoreComplete() + return guardPreparedBroadcast(lightningService.prepareRelease(attempt)) + } + + private fun guardPreparedBroadcast(prepared: PreparedOnchainSend): PreparedOnchainSend = + PreparedOnchainSend(prepared.receipt) { beforeDispatch -> + requireWalletRestoreComplete() + prepared.broadcast { + requireWalletRestoreComplete() + beforeDispatch() + requireWalletRestoreComplete() + } + } suspend fun recordOnchainRelease(expected: OnchainSendAttempt, txid: String): OnchainSendAttempt? = onchainSendAttemptStore.recordRelease(expected, txid) @@ -2086,6 +2124,7 @@ class LightningRepo @Inject constructor( pushToCounterpartySats: ULong? = null, channelConfig: ChannelConfig? = null, ): Result = executeWhenNodeRunning("openChannel") { + requireWalletRestoreComplete() lightningService.openChannel(peer, channelAmountSats, pushToCounterpartySats, channelConfig).also { syncState() } @@ -2096,6 +2135,7 @@ class LightningRepo @Inject constructor( force: Boolean = false, forceCloseReason: String? = null, ): Result = executeWhenNodeRunning("closeChannel") { + requireWalletRestoreComplete() runCatching { lightningService.closeChannel(channel, force, forceCloseReason) }.also { syncState() } @@ -2279,6 +2319,7 @@ class LightningRepo @Inject constructor( originalTxId: Txid, satsPerVByte: ULong, ): Result = executeWhenNodeRunning("bumpFeeByRbf") { + requireWalletRestoreComplete() runCatching { require(!originalTxId.isBlank()) { "originalTxId is null or empty: $originalTxId" } require(satsPerVByte > 0u) { "satsPerVByte invalid: $satsPerVByte" } @@ -2309,6 +2350,7 @@ class LightningRepo @Inject constructor( satsPerVByte: ULong, destinationAddress: Address, ): Result = executeWhenNodeRunning("accelerateByCpfp") { + requireWalletRestoreComplete() runCatching { require(!originalTxId.isBlank()) { "originalTxId is null or empty: $originalTxId" } require(!destinationAddress.isBlank()) { "destinationAddress is null or empty: $destinationAddress" } diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt index 78a272c4c9..a30dadb996 100644 --- a/app/src/main/java/to/bitkit/services/LightningService.kt +++ b/app/src/main/java/to/bitkit/services/LightningService.kt @@ -58,6 +58,7 @@ import to.bitkit.data.SettingsStore import to.bitkit.data.WatchOnlyAccountStore import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.requireWalletRestoreComplete import to.bitkit.data.withRequiredNativeSegwitMonitoring import to.bitkit.di.BgDispatcher import to.bitkit.di.IoDispatcher @@ -676,6 +677,7 @@ class LightningService internal constructor( val node = this.node ?: throw ServiceError.NodeNotSetup() return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() node.onchainPayment().newAddress() } } @@ -689,6 +691,7 @@ class LightningService internal constructor( val node = this.node ?: throw ServiceError.NodeNotSetup() return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() val addressInfo = node.onchainPayment().newAddressInfoForType(addressType.toLdkAddressType()) AddressDerivationInfo(address = addressInfo.address, index = addressInfo.index.toInt()) } @@ -838,6 +841,7 @@ class LightningService internal constructor( val node = this.node ?: throw ServiceError.NodeNotSetup() return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() try { val pushToCounterpartyMsat = pushToCounterpartySats?.let { it * 1000u } Logger.debug("Initiating channel open (sats: '$channelAmountSats') with: '${peer.uri}'", context = TAG) @@ -886,6 +890,7 @@ class LightningService internal constructor( try { ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() Logger.debug("Initiating channel close (force=$force): '$channelId'", context = TAG) if (force) { node.forceCloseChannel(userChannelId, counterpartyNodeId, forceCloseReason.orEmpty()) @@ -954,6 +959,7 @@ class LightningService internal constructor( val message = description return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() val bolt11Invoice: Bolt11Invoice = if (amountMsat != null) { node.bolt11Payment() .receive( @@ -983,6 +989,7 @@ class LightningService internal constructor( walletIndex: Int = currentWalletIndex, paymentDeadlineAt: Instant? = null, ): PreparedOnchainSend = callOnchainSend { + keychain.requireWalletRestoreComplete() ensurePaymentDeadline(paymentDeadlineAt) require(satsPerVByte > 0uL && satsPerVByte <= UInt.MAX_VALUE.toULong()) if (currentWalletIndex != walletIndex) throw ServiceError.NodeNotSetup() @@ -1012,8 +1019,10 @@ class LightningService internal constructor( callOnchainSend { // Dispatch only through the handle that signed the original receipt. if (currentWalletIndex != walletIndex || node !== originalNode) throw ServiceError.NodeNotSetup() + keychain.requireWalletRestoreComplete() ensurePaymentDeadline(paymentDeadlineAt) beforeDispatch() + keychain.requireWalletRestoreComplete() when (val result = prepared.broadcast()) { is OnchainSendResult.Accepted -> OnchainSendOutcome.Accepted(result.txid) is OnchainSendResult.Rejected -> OnchainSendOutcome.Rejected(result.txid, result.reason) @@ -1131,6 +1140,7 @@ class LightningService internal constructor( .getOrElse { throw LdkError(it as NodeException) } return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() ensurePaymentDeadline(paymentDeadlineAt) runCatching { val routeParameters = maxTotalRoutingFeeMsat?.let(::routeParametersWithFeeCap) @@ -1311,6 +1321,7 @@ class LightningService internal constructor( Logger.info("RBF for txid='$txid' using satsPerVByte='$satsPerVByte'", context = TAG) return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() return@background try { node.onchainPayment().bumpFeeByRbf( txid = txid, @@ -1332,6 +1343,7 @@ class LightningService internal constructor( Logger.info("CPFP for txid='$txid' using satsPerVByte='$satsPerVByte', to address='$toAddress'", context = TAG) return ServiceQueue.LDK.background(ldkQueue) { + keychain.requireWalletRestoreComplete() return@background try { node.onchainPayment().accelerateByCpfp( txid = txid, diff --git a/app/src/main/java/to/bitkit/ui/ContentView.kt b/app/src/main/java/to/bitkit/ui/ContentView.kt index fff4a35de1..d26508edee 100644 --- a/app/src/main/java/to/bitkit/ui/ContentView.kt +++ b/app/src/main/java/to/bitkit/ui/ContentView.kt @@ -419,10 +419,13 @@ fun ContentView( val backupRestoreFailed = restoreState is RestoreState.BackupFailed && nodeLifecycleState == NodeLifecycleState.Running + val canProceedWithoutRestore = (restoreState as? RestoreState.BackupFailed)?.hasPendingWalletRestore != true + if (walletIsInitializing || backupRestoreFailed) { if (nodeLifecycleState is NodeLifecycleState.ErrorStarting || backupRestoreFailed) { WalletRestoreErrorView( retryCount = restoreState.retryCount(), + allowProceedWithoutRestore = canProceedWithoutRestore, hazeState = hazeState, onRetry = { if (backupRestoreFailed) { diff --git a/app/src/main/java/to/bitkit/ui/onboarding/WalletRestoreErrorView.kt b/app/src/main/java/to/bitkit/ui/onboarding/WalletRestoreErrorView.kt index f12db70684..4c442937ce 100644 --- a/app/src/main/java/to/bitkit/ui/onboarding/WalletRestoreErrorView.kt +++ b/app/src/main/java/to/bitkit/ui/onboarding/WalletRestoreErrorView.kt @@ -38,6 +38,7 @@ fun WalletRestoreErrorView( onRetry: () -> Unit, onProceedWithoutRestore: () -> Unit, modifier: Modifier = Modifier, + allowProceedWithoutRestore: Boolean = true, hazeState: HazeState = rememberHazeState(), ) { var showDialog by remember { mutableStateOf(false) } @@ -75,7 +76,7 @@ fun WalletRestoreErrorView( modifier = Modifier.testTag("TryAgainButton") ) - if (retryCount > 1) { + if (allowProceedWithoutRestore && retryCount > 1) { VerticalSpacer(12.dp) SecondaryButton( text = stringResource(R.string.onboarding__restore_no_backup_button), @@ -86,7 +87,7 @@ fun WalletRestoreErrorView( VerticalSpacer(16.dp) - if (showDialog) { + if (showDialog && allowProceedWithoutRestore) { AppAlertDialog( title = stringResource(R.string.common__are_you_sure), text = stringResource(R.string.onboarding__restore_no_backup_warn), diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index c37b95d466..114798b9d0 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -111,6 +111,7 @@ class WalletViewModel @Inject constructor( val isRefreshing = _isRefreshing.asStateFlow() private val retryLightningPaymentMutex = Mutex() + private val backupRestoreMutex = Mutex() private val _isRetryingLightningPayment = MutableStateFlow(false) val isRetryingLightningPayment = _isRetryingLightningPayment.asStateFlow() @@ -226,6 +227,7 @@ class WalletViewModel @Inject constructor( } private suspend fun restoreFromBackup(resumePendingWalletBackup: Boolean = false) { + if (!backupRestoreMutex.tryLock()) return val retryCount = _restoreState.value.backupRetryCount() _restoreState.update { RestoreState.InProgress.Metadata } try { @@ -242,10 +244,12 @@ class WalletViewModel @Inject constructor( }.onFailure { Logger.error("Restore from backup failed", it, context = TAG) ToastEventBus.send(it) - _restoreState.update { RestoreState.BackupFailed(retryCount + 1) } + val pendingRestore = runSuspendCatching { backupRepo.hasPendingWalletRestore() }.getOrDefault(true) + _restoreState.update { RestoreState.BackupFailed(retryCount + 1, pendingRestore) } } } finally { backupRepo.setRestorePending(false) + backupRestoreMutex.unlock() } } @@ -323,11 +327,13 @@ class WalletViewModel @Inject constructor( @Suppress("ForbiddenComment") fun onProceedWithoutRestore(onDone: () -> Unit) = viewModelScope.launch { + if (runSuspendCatching { backupRepo.hasPendingWalletRestore() }.getOrDefault(true)) return@launch // TODO start LDK without trying to restore backup state from VSS if possible if (lightningState.value.nodeLifecycleState is NodeLifecycleState.ErrorStarting) { lightningRepo.stop() delay(LOADING_MS.milliseconds) } + if (runSuspendCatching { backupRepo.hasPendingWalletRestore() }.getOrDefault(true)) return@launch _restoreState.update { RestoreState.Settled } onDone() } @@ -720,7 +726,7 @@ sealed interface RestoreState { } data class Retry(val count: Int) : RestoreState - data class BackupFailed(val count: Int) : RestoreState + data class BackupFailed(val count: Int, val hasPendingWalletRestore: Boolean = true) : RestoreState data object Completed : RestoreState data object Settled : RestoreState @@ -733,7 +739,7 @@ sealed interface RestoreState { fun backupRetryCount() = (this as? BackupFailed)?.count ?: 0 fun countRetry(): RestoreState = when (this) { is Retry -> Retry(count + 1) - is BackupFailed -> BackupFailed(count + 1) + is BackupFailed -> copy(count = count + 1) else -> Retry(1) } fun isOngoing() = this is InProgress diff --git a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt index a06dcd724b..cbf4849701 100644 --- a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt @@ -67,6 +67,7 @@ import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.backup.VssBackupClientLdk import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.WalletBackupRestorePendingError import to.bitkit.ext.createChannelDetails import to.bitkit.ext.of import to.bitkit.models.ActiveOnchainAttemptBackup @@ -1082,6 +1083,109 @@ class LightningRepoTest : BaseUnitTest() { verify(lightningService).resetNetworkGraph(0) } + @Test + fun `pending wallet recovery blocks new receiving data while node remains available`() = test { + startNodeForTesting() + whenever(lightningService.newAddress()).thenReturn("address") + for (marker in listOf(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE, Keychain.Key.USDT_PENDING_BACKUP_RESTORE)) { + whenever(keychain.exists(marker.name)).thenReturn(true) + assertIs(sut.newAddress().exceptionOrNull()) + assertIs(sut.newAddressForType(AddressType.P2WPKH).exceptionOrNull()) + assertIs(sut.newAddressInfoForType(AddressType.P2WPKH).exceptionOrNull()) + assertIs(sut.createInvoice(description = "test").exceptionOrNull()) + assertIs(sut.createInvoiceMsats(1000uL, "test").exceptionOrNull()) + assertTrue(sut.revealReceiveAddresses(505, AddressType.P2WPKH).isSuccess) + assertTrue(sut.sync().isSuccess) + whenever(keychain.exists(marker.name)).thenReturn(false) + } + verify(lightningService, never()).newAddress() + verify(lightningService, never()).newAddressForType(any()) + verify(lightningService, never()).newAddressInfoForType(any()) + verify(lightningService, never()).receive(anyOrNull(), any(), any()) + verify(lightningService, never()).receiveMsats(any(), any(), any()) + assertEquals("address", sut.newAddress().getOrThrow()) + } + + @Test + fun `pending wallet recovery prevents ordinary payment admission`() = test { + startNodeForTesting() + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + whenever(lightningService.channels).thenReturn(listOf(createChannelDetails().copy(isUsable = true))) + var beforeSend = false + sut.payInvoice("invoice", null, onBeforeSend = { + beforeSend = true + true + }) + assertFalse(beforeSend) + assertIs(sut.sendOnChain("address", 1000uL).exceptionOrNull()) + verify(onchainSendAttemptStore, never()).current() + verify(lightningService, never()).send(any(), anyOrNull(), anyOrNull(), anyOrNull()) + } + + @Test + fun `restore beginning during payment authorization prevents dispatch`() = test { + startNodeForTesting() + val result = sut.payInvoice("invoice", null, onBeforeSend = { + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + true + }) + assertIs(result.exceptionOrNull()) + verify(lightningService, never()).send(any(), anyOrNull(), anyOrNull(), anyOrNull()) + } + + @Test + fun `unreadable restore admission fails closed`() = test { + startNodeForTesting() + val failure = IllegalStateException("Keychain unavailable") + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenThrow(failure) + assertEquals(failure, sut.newAddress().exceptionOrNull()) + verify(lightningService, never()).newAddress() + } + + @Test + fun `recreated repository retains durable recovery admission`() = test { + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + repeat(2) { + setUp() + startNodeForTesting() + assertIs(sut.newAddress().exceptionOrNull()) + } + verify(lightningService, never()).newAddress() + } + + @Test + fun `pending restore blocks explicit recovery channel close and fee mutations`() = test { + startNodeForTesting() + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + val results = listOf( + sut.retryOriginalOnchainSend("attempt", "wallet", 1uL, authorizeOriginal = { error("Must not authorize") }), + sut.closeChannel(createChannelDetails()), + sut.openChannel(mock(), 1000uL), + sut.bumpFeeByRbf("txid", 1uL), + sut.accelerateByCpfp("txid", 1uL, "address"), + ) + results.forEach { assertIs(it.exceptionOrNull()) } + assertFailsWith { sut.prepareRelease(pendingSendAttempt()) } + verify(lightningService, never()).prepareRelease(any()) + } + + @Test + fun `restore beginning after release preparation prevents broadcast`() = test { + val attempt = pendingSendAttempt() + var broadcasts = 0 + val native = PreparedOnchainSend(preparedOutcome(OnchainSendOutcome.Accepted("release")).receipt) { + broadcasts++ + OnchainSendOutcome.Accepted("release") + } + whenever(lightningService.prepareRelease(attempt)).thenReturn(native) + val prepared = sut.prepareRelease(attempt) + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + repeat(2) { + assertFailsWith { prepared.broadcast() } + } + assertEquals(0, broadcasts) + } + @Test fun `newAddress should fail when node is not running`() = test { val result = sut.newAddress() diff --git a/app/src/test/java/to/bitkit/services/LightningServiceTest.kt b/app/src/test/java/to/bitkit/services/LightningServiceTest.kt index 2e9736f816..517ba4c656 100644 --- a/app/src/test/java/to/bitkit/services/LightningServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/LightningServiceTest.kt @@ -1,14 +1,19 @@ package to.bitkit.services import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.async import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.delay import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.Json @@ -51,6 +56,7 @@ import to.bitkit.data.WatchOnlyAccountReconciliationState import to.bitkit.data.WatchOnlyAccountStore import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.WalletBackupRestorePendingError import to.bitkit.env.Env import to.bitkit.ext.createChannelDetails import to.bitkit.models.ActiveOnchainAttemptBackup @@ -103,7 +109,11 @@ class LightningServiceTest : BaseUnitTest() { @Before fun setUp() { - sut = LightningService( + sut = createService() + } + + private fun createService(ldkDispatcher: CoroutineDispatcher = testDispatcher): LightningService = + LightningService( bgDispatcher = testDispatcher, ioDispatcher = testDispatcher, keychain = keychain, @@ -112,10 +122,39 @@ class LightningServiceTest : BaseUnitTest() { watchOnlyAccountStore = watchOnlyAccountStore, loggerLdk = loggerLdk, watchOnlyAccountLifecycleCoordinator = watchOnlyAccountLifecycleCoordinator, - ldkQueue = testDispatcher, + ldkQueue = ldkDispatcher, onchainFeeRateFactory = { sendFeeRate }, - ) - sut.node = node + ).also { it.node = node } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `queued address allocation rechecks recovery before native mutation`() = test { + whenever(node.onchainPayment()).thenReturn(onchainPayment) + whenever(onchainPayment.newAddress()).thenReturn("bcrt1qnew") + val queued = createService(StandardTestDispatcher(testScheduler)) + val result = async { runCatching { queued.newAddress() } } + assertFalse(result.isCompleted) + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + runCurrent() + assertTrue(result.await().exceptionOrNull()?.cause is WalletBackupRestorePendingError) + verify(onchainPayment, never()).newAddress() + } + + @Test + fun `prepared native broadcast rechecks recovery after caller suspension`() = test { + val txid = "ab".repeat(32) + val native = preparedNative(txid) + whenever(node.onchainPayment()).thenReturn(onchainPayment) + whenever(onchainPayment.prepareSendToAddress("address", 1_000uL, sendFeeRate, null)).thenReturn(native) + whenever(native.broadcast()).thenReturn(OnchainSendResult.Accepted(txid)) + val prepared = sut.prepareOnchainSend("address", 1_000uL, 1uL) + val result = runCatching { + prepared.broadcast { + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + } + } + assertTrue(result.exceptionOrNull() is WalletBackupRestorePendingError) + verify(native, never()).broadcast() } @Test diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 898a9e08a6..91cdfe0477 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent import org.junit.Before import org.junit.Test import org.lightningdevkit.ldknode.ChannelDataMigration @@ -334,7 +335,7 @@ class WalletViewModelTest : BaseUnitTest() { sut.restoreWallet("mnemonic", "passphrase") walletState.value = walletState.value.copy(walletExists = true) advanceUntilIdle() - assertEquals(RestoreState.BackupFailed(1), sut.restoreState.value) + assertEquals(RestoreState.BackupFailed(1, hasPendingWalletRestore = false), sut.restoreState.value) sut.onBackupRestoreRetry() advanceUntilIdle() @@ -381,7 +382,7 @@ class WalletViewModelTest : BaseUnitTest() { sut.restoreWallet("mnemonic", "passphrase") walletState.value = walletState.value.copy(walletExists = true) advanceUntilIdle() - assertEquals(RestoreState.BackupFailed(1), sut.restoreState.value) + assertEquals(RestoreState.BackupFailed(1, hasPendingWalletRestore = false), sut.restoreState.value) sut.onProceedWithoutRestore(onDone = {}) advanceUntilIdle() @@ -390,6 +391,84 @@ class WalletViewModelTest : BaseUnitTest() { verify(lightningRepo, never()).stop() } + @Test + fun `proceed cannot dismiss pending wallet recovery`() = test { + whenever(backupRepo.hasPendingWalletRestore()).thenReturn(true) + walletState.value = walletState.value.copy(walletExists = true) + lightningState.value = lightningState.value.copy(nodeLifecycleState = NodeLifecycleState.Running) + advanceUntilIdle() + val pendingState = sut.restoreState.value + var dismissed = false + + repeat(3) { + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + } + + assertEquals(pendingState, sut.restoreState.value) + assertFalse(dismissed) + verify(lightningRepo, never()).stop() + } + + @Test + fun `post restore profile failure allows proceeding only while safety markers stay clear`() = test { + whenever(backupRepo.performFullRestoreFromLatestBackup(any())).thenReturn(Result.success(Unit)) + whenever(pubkyRepo.initialize()).doSuspendableAnswer { throw AppError("profile cache unavailable") } + sut.onBackupRestoreRetry() + advanceUntilIdle() + assertEquals(RestoreState.BackupFailed(1, hasPendingWalletRestore = false), sut.restoreState.value) + + whenever(backupRepo.hasPendingWalletRestore()).thenReturn(true) + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + assertTrue(sut.restoreState.value is RestoreState.BackupFailed) + + whenever(backupRepo.hasPendingWalletRestore()).thenReturn(false) + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertTrue(dismissed) + assertEquals(RestoreState.Settled, sut.restoreState.value) + verify(backupRepo, times(1)).performFullRestoreFromLatestBackup(any()) + } + + @Test + fun `failed safety marker inspection keeps restore failure closed`() = test { + whenever(backupRepo.performFullRestoreFromLatestBackup(any())).thenReturn(Result.failure(AppError("offline"))) + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { throw AppError("keychain unavailable") } + sut.onBackupRestoreRetry() + advanceUntilIdle() + assertEquals(RestoreState.BackupFailed(1), sut.restoreState.value) + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + assertEquals(RestoreState.BackupFailed(1), sut.restoreState.value) + } + + @Test + fun `overlapping backup retries perform one restore`() = test { + val finishRestore = CompletableDeferred() + var calls = 0 + whenever(backupRepo.performFullRestoreFromLatestBackup(any())).doSuspendableAnswer { + calls++ + finishRestore.await() + Result.success(Unit) + } + sut.onBackupRestoreRetry() + sut.onBackupRestoreRetry() + runCurrent() + try { + assertEquals(1, calls) + assertEquals(RestoreState.InProgress.Metadata, sut.restoreState.value) + } finally { + finishRestore.complete(Unit) + } + advanceUntilIdle() + assertEquals(RestoreState.Completed, sut.restoreState.value) + } + @Test fun `pending wallet restore is offered after wallet state reloads`() = test { whenever(backupRepo.hasPendingWalletRestore()).thenReturn(true) @@ -412,7 +491,7 @@ class WalletViewModelTest : BaseUnitTest() { sut.restoreWallet("mnemonic", "passphrase") walletState.value = walletState.value.copy(walletExists = true) advanceUntilIdle() - assertEquals(RestoreState.BackupFailed(1), sut.restoreState.value) + assertEquals(RestoreState.BackupFailed(1, hasPendingWalletRestore = false), sut.restoreState.value) verify(migrationService).restoreFromRNRemoteBackup() verify(backupRepo).restoreUsdtRecoveryAfterLegacyMigration() verify(backupRepo, never()).performFullRestoreFromLatestBackup(any()) diff --git a/changelog.d/next/pending-wallet-restore.fixed.md b/changelog.d/next/pending-wallet-restore.fixed.md new file mode 100644 index 0000000000..c51025be15 --- /dev/null +++ b/changelog.d/next/pending-wallet-restore.fixed.md @@ -0,0 +1 @@ +Keep payments and new receive addresses unavailable until required wallet recovery finishes, while allowing safe access after unrelated profile loading fails. diff --git a/journeys/backup-restore/pending-wallet-recovery.xml b/journeys/backup-restore/pending-wallet-recovery.xml new file mode 100644 index 0000000000..633cfd8acb --- /dev/null +++ b/journeys/backup-restore/pending-wallet-recovery.xml @@ -0,0 +1,21 @@ + + + Use a disposable test-network recovery fixture with synthetic funds and a complete wallet backup + containing an unresolved payment attempt and private address reservation ceilings. A VSS control + must fail or delay wallet download/import without preventing node sync, and a separate control + must fail Pubky profile caching after wallet restoration succeeds. These full-app fault controls + are not currently provisioned; repository tests and Compose component tests are separate evidence. + iOS already keeps its normal startup behind the durable restore gate and presents retry-only + recovery failure. Android keeps node sync and reservation reveal available because restore needs them. + Never reset a real wallet or use live funds. + + + Start restoring the fixture wallet and fail required wallet backup download + Retry more than twice and verify Try Again remains available without Proceed Without Backup + Attempt ordinary BTC and Lightning sends, a new receive address and a prepared broadcast through the fixture; verify no new operation is signed or broadcast while required recovery is pending + Restart the app with the same pending restore and verify the same admission restrictions remain + Restore backup availability, trigger overlapping retries and verify one restoration completes before wallet actions become available + Repeat with successful wallet recovery followed by profile cache failure; after repeated failures verify Proceed is available only while safety markers are clear + Set a pending restore marker before confirming Proceed and verify the current state is rechecked and recovery cannot be dismissed + + From 837aa1c7257fdee2b21340b09942993e38c5ab01 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:24:43 +0200 Subject: [PATCH 2/3] chore: rename changelog fragment --- .../next/{pending-wallet-restore.fixed.md => 1685.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{pending-wallet-restore.fixed.md => 1685.fixed.md} (100%) diff --git a/changelog.d/next/pending-wallet-restore.fixed.md b/changelog.d/next/1685.fixed.md similarity index 100% rename from changelog.d/next/pending-wallet-restore.fixed.md rename to changelog.d/next/1685.fixed.md From 8d72053c4fd480c5b21ec33a8285b966672bf6c5 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 27 Oct 2026 01:48:13 +0200 Subject: [PATCH 3/3] fix: preserve ordinary startup recovery and editor cancellation --- .../onboarding/WalletRestoreErrorViewTest.kt | 47 ++ .../bitkit/data/WalletConnectionRecovery.kt | 12 + app/src/main/java/to/bitkit/ui/ContentView.kt | 7 +- .../to/bitkit/viewmodels/WalletViewModel.kt | 297 ++++++++---- .../bitkit/test/ConnectionRecoveryFixture.kt | 3 +- .../java/to/bitkit/ui/WalletViewModelTest.kt | 454 +++++++++++++++++- changelog.d/next/1685.fixed.md | 2 +- 7 files changed, 725 insertions(+), 97 deletions(-) diff --git a/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt b/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt index 3e0ac7f8fa..73d0dd8915 100644 --- a/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt +++ b/app/src/androidTest/java/to/bitkit/ui/onboarding/WalletRestoreErrorViewTest.kt @@ -8,11 +8,17 @@ import androidx.compose.ui.platform.LocalInspectionMode import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.test.platform.app.InstrumentationRegistry import org.junit.Rule import org.junit.Test +import to.bitkit.R import to.bitkit.test.annotations.ComposeUi import to.bitkit.ui.theme.AppThemeSurface +import to.bitkit.viewmodels.ConnectionStartupFailure +import to.bitkit.viewmodels.RestoreState +import to.bitkit.viewmodels.canProceedWithoutRestore import kotlin.test.assertEquals @ComposeUi @@ -20,6 +26,47 @@ class WalletRestoreErrorViewTest { @get:Rule val composeTestRule = createComposeRule() + @Test + fun ordinaryStartupFailureUsesProductionGateAndDismissesOnlyAfterConfirmation() { + var failure by mutableStateOf(ConnectionStartupFailure("storage unavailable", true)) + var restore by mutableStateOf(RestoreState.Retry(0)) + var dismissed = 0 + composeTestRule.setContent { + AppThemeSurface { + CompositionLocalProvider(LocalInspectionMode provides true) { + if (failure != null || restore is RestoreState.BackupFailed) { + WalletRestoreErrorView( + retryCount = restore.retryCount(), + allowProceedWithoutRestore = canProceedWithoutRestore(restore, failure?.canProceedWithoutRestore), + onRetry = {}, + onProceedWithoutRestore = { + failure = null + restore = RestoreState.Settled + dismissed++ + }, + ) + } + } + } + } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + composeTestRule.runOnIdle { restore = RestoreState.Retry(1) } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + composeTestRule.runOnIdle { restore = RestoreState.Retry(2) } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertIsDisplayed().performClick() + composeTestRule.onNodeWithTag("ProceedWithoutBackupDialog").assertIsDisplayed() + composeTestRule.runOnIdle { failure = failure?.copy(canProceedWithoutRestore = false) } + composeTestRule.onNodeWithTag("ProceedWithoutBackupDialog").assertDoesNotExist() + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + composeTestRule.runOnIdle { failure = failure?.copy(canProceedWithoutRestore = true) } + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").performClick() + val confirm = InstrumentationRegistry.getInstrumentation().targetContext.getString(R.string.common__yes_proceed) + composeTestRule.onNodeWithText(confirm).performClick() + composeTestRule.onNodeWithTag("ProceedWithoutBackupButton").assertDoesNotExist() + composeTestRule.onNodeWithTag("TryAgainButton").assertDoesNotExist() + composeTestRule.runOnIdle { assertEquals(1, dismissed) } + } + @Test fun pendingRecoveryOnlyOffersRetryAfterRepeatedFailures() { var retries = 0 diff --git a/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt b/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt index c332155ec1..b4b64f937b 100644 --- a/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt +++ b/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt @@ -158,6 +158,18 @@ class WalletConnectionRecovery @Inject constructor( checkOwner(owner) } + /** Publish local UI state only while this original owner still holds admission. */ + internal suspend fun commitCurrentState(block: () -> T): T { + checkCurrent() + return synchronized(stateLock) { + job.ensureActive() + check(active) { "Connection recovery operation ended" } + checkAdmission() + if (owner.generation != generation) throw CancellationException("Wallet recovery owner changed") + block() + } + } + private suspend fun read(): ConnectionChoices? { checkCurrent() val encoded = keychain.loadString(KEY, owner.walletIndex) ?: return null diff --git a/app/src/main/java/to/bitkit/ui/ContentView.kt b/app/src/main/java/to/bitkit/ui/ContentView.kt index dbf65bed05..206b02fa0e 100644 --- a/app/src/main/java/to/bitkit/ui/ContentView.kt +++ b/app/src/main/java/to/bitkit/ui/ContentView.kt @@ -253,6 +253,7 @@ import to.bitkit.viewmodels.SettingsViewModel import to.bitkit.viewmodels.TransferEffect import to.bitkit.viewmodels.TransferViewModel import to.bitkit.viewmodels.WalletViewModel +import to.bitkit.viewmodels.canProceedWithoutRestore internal fun shouldFinishWalletInitialization( nodeLifecycleState: NodeLifecycleState, @@ -421,8 +422,10 @@ fun ContentView( val connectionFailure by walletViewModel.connectionStartupError.collectAsStateWithLifecycle() val connectionRetryBusy by walletViewModel.connectionRetryBusy.collectAsStateWithLifecycle() - val canProceedWithoutRestore = connectionFailure == null && - (restoreState as? RestoreState.BackupFailed)?.hasPendingWalletRestore != true + val canProceedWithoutRestore = canProceedWithoutRestore( + restoreState, + connectionFailure?.canProceedWithoutRestore, + ) val connectionEditor by walletViewModel.connectionEditor.collectAsStateWithLifecycle() if (connectionEditor != null) { diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 2e32ea85f4..ffbcc7a9c3 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.withTimeoutOrNull import org.lightningdevkit.ldknode.ChannelDataMigration import org.lightningdevkit.ldknode.PeerDetails import to.bitkit.R +import to.bitkit.data.ConnectionRecoveryOwnerChanged import to.bitkit.data.SettingsStore import to.bitkit.data.WalletConnectionRecovery import to.bitkit.di.BgDispatcher @@ -127,9 +128,10 @@ class WalletViewModel @Inject constructor( val connectionEditor = _connectionEditor.asStateFlow() private val connectionEditorGeneration = AtomicLong() private var connectionRetryJob: Job? = null + private val startupAttempt = AtomicLong() private val _connectionRetryBusy = MutableStateFlow(false) val connectionRetryBusy = _connectionRetryBusy.asStateFlow() - private val _connectionStartupError = MutableStateFlow(null) + private val _connectionStartupError = MutableStateFlow(null) val connectionStartupError = _connectionStartupError.asStateFlow() private var connectionEditorJob: Job? = null @@ -137,22 +139,40 @@ class WalletViewModel @Inject constructor( if (_connectionEditor.value != null) return val id = connectionEditorGeneration.incrementAndGet() _connectionEditor.update { ConnectionRecoveryUiState(id = id, busy = true) } - connectionEditorJob = viewModelScope.launch(bgDispatcher) { - runSuspendCatching { - val owner = connectionRecovery.captureOwner() - connectionRecovery.withOperation(owner) { session -> - session.replay() - settingsStore.data.first() - }.also { - updateConnectionEditor(id) { _ -> + launchConnectionEditorOperation(id) { + val owner = connectionRecovery.captureOwner() + connectionRecovery.withOperation(owner) { session -> + session.replay() + val settings = settingsStore.data.first() + session.commitCurrentState { + updateConnectionEditor(id) { ConnectionRecoveryUiState( - id = id, owner = owner, electrum = it.electrumServer, - rgs = it.rgsServerUrl.orEmpty() + id = id, + owner = owner, + electrum = settings.electrumServer, + rgs = settings.rgsServerUrl.orEmpty() ) } } - }.onFailure { error -> - updateConnectionEditor(id) { it.copy(busy = false, error = error.message) } + } + } + } + + private fun launchConnectionEditorOperation(id: Long, operation: suspend () -> Unit) { + connectionEditorJob = viewModelScope.launch(bgDispatcher) { + try { + runSuspendCatching { operation() }.onFailure { error -> + if (error is ConnectionRecoveryOwnerChanged) { + invalidateConnectionEditor(id, error) + } else { + updateConnectionEditor(id) { it.copy(error = error.message) } + } + } + } catch (error: CancellationException) { + invalidateConnectionEditor(id, error) + throw error + } finally { + updateConnectionEditor(id) { it.copy(busy = false) } } } } @@ -179,35 +199,31 @@ class WalletViewModel @Inject constructor( val owner = state.owner ?: return if (state.busy) return _connectionEditor.update { it?.copy(busy = true, saved = false, error = null) } - connectionEditorJob = viewModelScope.launch(bgDispatcher) { - runSuspendCatching { - connectionRecovery.withOperation(owner) { session -> - if (electrum) { - val url = state.electrum.trim() - lightningRepo.validateElectrumServer(url).getOrThrow() - session.save(electrum = url) - } else { - val input = state.rgs.trim() - val url = when { - input.isBlank() -> Env.ldkRgsServerUrl - input.startsWith("https://") || input.startsWith("http://") -> input - else -> "https://$input" - } - url?.let { lightningRepo.validateRgsUrl(it).getOrThrow() } - session.save(rgs = WalletConnectionRecovery.RgsChoice(url)) + launchConnectionEditorOperation(state.id) { + connectionRecovery.withOperation(owner) { session -> + val settings = if (electrum) { + val url = state.electrum.trim() + lightningRepo.validateElectrumServer(url).getOrThrow() + session.save(electrum = url) + } else { + val input = state.rgs.trim() + val url = when { + input.isBlank() -> Env.ldkRgsServerUrl + input.startsWith("https://") || input.startsWith("http://") -> input + else -> "https://$input" } + url?.let { lightningRepo.validateRgsUrl(it).getOrThrow() } + session.save(rgs = WalletConnectionRecovery.RgsChoice(url)) } - }.onSuccess { settings -> - updateConnectionEditor(state.id) { - it.copy( - busy = false, - saved = true, - electrum = if (electrum) settings.electrumServer else it.electrum, - rgs = if (!electrum) settings.rgsServerUrl.orEmpty() else it.rgs, - ) + session.commitCurrentState { + updateConnectionEditor(state.id) { + it.copy( + saved = true, + electrum = if (electrum) settings.electrumServer else it.electrum, + rgs = if (!electrum) settings.rgsServerUrl.orEmpty() else it.rgs, + ) + } } - }.onFailure { error -> - updateConnectionEditor(state.id) { it.copy(busy = false, error = error.message) } } } } @@ -232,27 +248,80 @@ class WalletViewModel @Inject constructor( ): Job { connectionRetryJob?.takeIf { it.isActive }?.let { return it } _connectionRetryBusy.update { true } + val attempt = startupAttempt.incrementAndGet() return viewModelScope.launch(bgDispatcher) { + var attemptOwner = owner try { runSuspendCatching { - retryRecovery(owner ?: connectionRecovery.captureRuntimeOwner(), backupOnly) - }.onSuccess { editorId?.let { updateConnectionEditor(it) { null } } } - .onFailure { error -> - if (editorId == null) { - _connectionStartupError.update { - error.message ?: context.getString(R.string.common__error_body) - } - ToastEventBus.send(error) - } else { - updateConnectionEditor(editorId) { it.copy(busy = false, error = error.message) } - } + val admittedOwner = attemptOwner ?: connectionRecovery.captureRuntimeOwner().also { + attemptOwner = it } + retryRecovery(admittedOwner, backupOnly, attempt) + }.onSuccess { editorId?.let { updateConnectionEditor(it) { null } } } + .onFailure { error -> reportStartupFailure(attemptOwner, error, attempt, editorId) } + } catch (error: CancellationException) { + editorId?.let { invalidateConnectionEditor(it, error) } + throw error } finally { + editorId?.let { updateConnectionEditor(it) { state -> state.copy(busy = false) } } _connectionRetryBusy.update { false } } }.also { connectionRetryJob = it } } + private suspend fun reportStartupFailure( + owner: WalletConnectionRecovery.Owner?, + error: Throwable, + attempt: Long, + editorId: Long? = null, + ) { + if (error is ConnectionRecoveryOwnerChanged) { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + editorId?.let { invalidateConnectionEditor(it, error) } + return + } + if (error is RecoveryModeError || error is WipeInProgressError || error is NodeStartYieldedToStopError) return + Logger.error("Wallet connection recovery startup failed", error, context = TAG) + if (startupAttempt.get() != attempt) return + if (owner == null) { + _connectionStartupError.update { + ConnectionStartupFailure(error.message ?: context.getString(R.string.common__error_body)) + } + editorId?.let { updateConnectionEditor(it) { state -> state.copy(busy = false, error = error.message) } } + ToastEventBus.send(error) + return + } + runSuspendCatching { + connectionRecovery.withOperation(owner) { session -> + val markersClear = runSuspendCatching { !backupRepo.hasPendingWalletRestore() }.getOrDefault(false) + val published = session.commitCurrentState { + if (startupAttempt.get() != attempt) return@commitCurrentState false + _connectionStartupError.update { + ConnectionStartupFailure( + message = error.message ?: context.getString(R.string.common__error_body), + canProceedWithoutRestore = markersClear, + owner = owner, + ) + } + editorId?.let { + updateConnectionEditor( + it + ) { state -> state.copy(busy = false, error = error.message) } + } + true + } + if (published) ToastEventBus.send(error) + } + }.onFailure { + if (it is ConnectionRecoveryOwnerChanged) editorId?.let { id -> invalidateConnectionEditor(id, it) } + Logger.warn("Startup failure owner is no longer available", it, context = TAG) + } + } + + private fun invalidateConnectionEditor(id: Long, error: Throwable) = updateConnectionEditor(id) { + it.copy(owner = null, busy = false, error = error.message) + } + private var syncJob: Job? = null private var pendingWalletStart = false @@ -477,8 +546,11 @@ class WalletViewModel @Inject constructor( fun onRestoreRetry() = requestRecoveryRetry(owner = null, editorId = null) - private suspend fun retryRecovery(owner: WalletConnectionRecovery.Owner, backupOnly: Boolean) { + private suspend fun retryRecovery(owner: WalletConnectionRecovery.Owner, backupOnly: Boolean, attempt: Long) { connectionRecovery.withOperation(owner) { session -> + session.commitCurrentState { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + } val revision = session.replay() if (!backupOnly || revision != null) { if (!backupRepo.hasPendingWalletRestore()) _restoreState.update { it.countRetry() } @@ -498,12 +570,16 @@ class WalletViewModel @Inject constructor( restoreFromBackup(resumePendingWalletBackup = true, connectionSession = session) if (_restoreState.value !is RestoreState.Completed) { // Restore failures carry their own required-state gate; optional profile work is not a node error. - _connectionStartupError.update { null } + session.commitCurrentState { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + } return@withOperation } } session.complete(revision) - _connectionStartupError.update { null } + session.commitCurrentState { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + } } } @@ -511,15 +587,46 @@ class WalletViewModel @Inject constructor( @Suppress("ForbiddenComment") fun onProceedWithoutRestore(onDone: () -> Unit) = viewModelScope.launch { - if (runSuspendCatching { backupRepo.hasPendingWalletRestore() }.getOrDefault(true)) return@launch - // TODO start LDK without trying to restore backup state from VSS if possible - if (lightningState.value.nodeLifecycleState is NodeLifecycleState.ErrorStarting) { - lightningRepo.stop() - delay(LOADING_MS.milliseconds) + val failure = _connectionStartupError.value + val attempt = startupAttempt.get() + if (_connectionRetryBusy.value || isStarting) return@launch + if (failure?.canProceedWithoutRestore == false) return@launch + if (failure == null && _restoreState.value !is RestoreState.BackupFailed) return@launch + runSuspendCatching { + val owner = failure?.owner ?: connectionRecovery.captureRuntimeOwner() + connectionRecovery.withOperation(owner) { session -> + if (!canStillProceed(session, failure, attempt)) return@withOperation + // TODO start LDK without trying to restore backup state from VSS if possible + if (lightningState.value.nodeLifecycleState is NodeLifecycleState.ErrorStarting) { + lightningRepo.stop().getOrThrow() + delay(LOADING_MS.milliseconds) + } + if (!canStillProceed(session, failure, attempt)) return@withOperation + session.commitCurrentState { + if (startupAttempt.get() != attempt || _connectionStartupError.value !== failure) { + return@commitCurrentState + } + _connectionStartupError.update { null } + _restoreState.update { RestoreState.Settled } + onDone() + } + } + }.onFailure { Logger.error("Proceed without restore admission failed", it, context = TAG) } + } + + private suspend fun canStillProceed( + session: WalletConnectionRecovery.Session, + failure: ConnectionStartupFailure?, + attempt: Long, + ): Boolean { + if (runSuspendCatching { !backupRepo.hasPendingWalletRestore() }.getOrDefault(false)) return true + session.commitCurrentState { + if (startupAttempt.get() == attempt && _connectionStartupError.value === failure) { + _connectionStartupError.update { it?.copy(canProceedWithoutRestore = false) } + _restoreState.update { (it as? RestoreState.BackupFailed)?.copy(hasPendingWalletRestore = true) ?: it } + } } - if (runSuspendCatching { backupRepo.hasPendingWalletRestore() }.getOrDefault(true)) return@launch - _restoreState.update { RestoreState.Settled } - onDone() + return false } fun setInitNodeLifecycleState() = lightningRepo.setInitNodeLifecycleState() @@ -575,30 +682,34 @@ class WalletViewModel @Inject constructor( ) } - private suspend fun startNode(walletIndex: Int = 0): Result = runSuspendCatching { - val owner = connectionRecovery.captureRuntimeOwner() - require(owner.walletIndex == walletIndex) { "Wallet startup owner changed" } - val revision = connectionRecovery.withStartup(owner) { session -> - val revision = session.replay() - migrationService.lockChannelMigration() - try { - // A skipped start of a running node cannot acknowledge a newly saved configuration. - startNodeOwned(walletIndex, buildChannelMigrationIfAvailable(), isRestoreRetry = revision != null) - .getOrThrow() - } finally { - migrationService.unlockChannelMigration() + private suspend fun startNode(walletIndex: Int = 0): Result { + val attempt = startupAttempt.incrementAndGet() + var owner: WalletConnectionRecovery.Owner? = null + return runSuspendCatching { + val admittedOwner = connectionRecovery.captureRuntimeOwner().also { owner = it } + require(admittedOwner.walletIndex == walletIndex) { "Wallet startup owner changed" } + val revision = connectionRecovery.withStartup(admittedOwner) { session -> + session.commitCurrentState { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + } + val revision = session.replay() + migrationService.lockChannelMigration() + try { + // A skipped start of a running node cannot acknowledge a newly saved configuration. + startNodeOwned(walletIndex, buildChannelMigrationIfAvailable(), isRestoreRetry = revision != null) + .getOrThrow() + } finally { + migrationService.unlockChannelMigration() + } + revision } - revision - } - connectionRecovery.withOperation(owner) { it.complete(revision) } - _connectionStartupError.update { null } - }.onFailure { - if (it is RecoveryModeError || it is WipeInProgressError || it is NodeStartYieldedToStopError) { - return@onFailure - } - Logger.error("Wallet connection recovery startup failed", it, context = TAG) - _connectionStartupError.update { _ -> it.message ?: context.getString(R.string.common__error_body) } - ToastEventBus.send(it) + connectionRecovery.withOperation(admittedOwner) { session -> + session.complete(revision) + session.commitCurrentState { + if (startupAttempt.get() == attempt) _connectionStartupError.update { null } + } + } + }.onFailure { reportStartupFailure(owner, it, attempt) } } private suspend fun startNodeOwned( @@ -626,14 +737,6 @@ class WalletViewModel @Inject constructor( ensureSwapUpdatesRunning() // checkForOrphanedChannelMonitorRecovery() } - .onFailure { - if (it is RecoveryModeError || it is WipeInProgressError || it is NodeStartYieldedToStopError) { - Logger.debug("Skipped node start: '${it.message}'", context = TAG) - return@onFailure - } - Logger.error("Node startup error", it, context = TAG) - ToastEventBus.send(it) - } } private var swapEventsCollected = false @@ -966,3 +1069,13 @@ data class ConnectionRecoveryUiState( val saved: Boolean = false, val error: String? = null, ) + +@Immutable +data class ConnectionStartupFailure( + val message: String, + val canProceedWithoutRestore: Boolean = false, + internal val owner: WalletConnectionRecovery.Owner? = null, +) + +fun canProceedWithoutRestore(restoreState: RestoreState, startupClearance: Boolean?): Boolean = + startupClearance ?: ((restoreState as? RestoreState.BackupFailed)?.hasPendingWalletRestore == false) diff --git a/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt b/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt index e9987a6780..5f8ccf5859 100644 --- a/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt +++ b/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt @@ -14,11 +14,12 @@ import to.bitkit.data.keychain.Keychain fun connectionRecoveryFixture( settings: SettingsStore, keychain: Keychain = mock(), + currentBinding: () -> String = { "test-wallet-binding" }, ): WalletConnectionRecovery { val db = mock() whenever(db.configDao()).thenReturn(mock()) whenever(db.configDao().getAll()).thenReturn(MutableStateFlow(listOf(ConfigEntity()))) val binding = mock() - whenever(binding.getBackupWalletBinding(any())).thenReturn("test-wallet-binding") + whenever(binding.getBackupWalletBinding(any())).thenAnswer { currentBinding() } return WalletConnectionRecovery(keychain, db, binding, settings) } diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 75e8318af2..b47ca98c7b 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -1,16 +1,21 @@ package to.bitkit.ui import android.content.Context +import app.cash.turbine.test import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.withContext import org.junit.Before import org.junit.Test import org.lightningdevkit.ldknode.ChannelDataMigration @@ -31,6 +36,7 @@ import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.WalletConnectionRecovery import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.hasPendingWalletRestore import to.bitkit.ext.of import to.bitkit.models.BalanceState import to.bitkit.models.NodeLifecycleState @@ -50,9 +56,11 @@ import to.bitkit.services.MigrationService import to.bitkit.services.PendingChannelMigration import to.bitkit.test.BaseUnitTest import to.bitkit.test.connectionRecoveryFixture +import to.bitkit.ui.shared.toast.ToastEventBus import to.bitkit.utils.AppError import to.bitkit.viewmodels.RestoreState import to.bitkit.viewmodels.WalletViewModel +import to.bitkit.viewmodels.canProceedWithoutRestore import kotlin.coroutines.cancellation.CancellationException import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -69,6 +77,7 @@ class WalletViewModelTest : BaseUnitTest() { private val settingsStore = mock() private val recoveryKeychain = mock() private lateinit var connectionRecovery: WalletConnectionRecovery + private var currentBinding = "test-wallet-binding" private val backupRepo = mock() private val blocktankRepo = mock() private val pubkyRepo = mock() @@ -88,6 +97,7 @@ class WalletViewModelTest : BaseUnitTest() { whenever(walletRepo.walletState).thenReturn(walletState) whenever(walletRepo.refreshReceiveState()).thenReturn(Result.success(Unit)) whenever(lightningRepo.lightningState).thenReturn(lightningState) + whenever(lightningRepo.stop()).thenReturn(Result.success(Unit)) whenever(migrationService.isMigrationChecked()).thenReturn(true) whenever(migrationService.isChannelRecoveryChecked()).thenReturn(true) whenever(migrationService.isRestoringFromRNRemoteBackup).thenReturn(MutableStateFlow(false)) @@ -100,7 +110,7 @@ class WalletViewModelTest : BaseUnitTest() { whenever(settingsStore.data).thenReturn(flowOf(SettingsData())) whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(1uL)) - connectionRecovery = connectionRecoveryFixture(settingsStore, recoveryKeychain) + connectionRecovery = connectionRecoveryFixture(settingsStore, recoveryKeychain) { currentBinding } sut = WalletViewModel( context = context, bgDispatcher = testDispatcher, @@ -354,6 +364,448 @@ class WalletViewModelTest : BaseUnitTest() { verify(lightningRepo, never()).restartNode() } + @Test + fun `replacement cancels editor opening without leaving it busy`() = test { + val reading = CompletableDeferred() + whenever(settingsStore.data).thenReturn( + flow { + reading.complete(Unit) + awaitCancellation() + } + ) + sut.openConnectionRecovery() + reading.await() + connectionRecovery.withIdentityChange { } + advanceUntilIdle() + val cancelled = checkNotNull(sut.connectionEditor.value) + assertFalse(cancelled.busy) + assertEquals(null, cancelled.owner) + whenever(settingsStore.data).thenReturn(flowOf(SettingsData())) + sut.retryRecoveryConnection() + advanceUntilIdle() + assertFalse(checkNotNull(sut.connectionEditor.value).busy) + assertTrue(sut.connectionEditor.value?.owner != null) + } + + @Test + fun `replacement cancels editor save without retaining its old owner`() = test { + sut.openConnectionRecovery() + advanceUntilIdle() + sut.editRecoveryElectrum("ssl://electrum.example.com:50002") + val validating = CompletableDeferred() + whenever(lightningRepo.validateElectrumServer(any())).doSuspendableAnswer { + validating.complete(Unit) + awaitCancellation() + } + sut.saveRecoveryConnection(electrum = true) + validating.await() + connectionRecovery.withIdentityChange { } + advanceUntilIdle() + val cancelled = checkNotNull(sut.connectionEditor.value) + assertFalse(cancelled.busy) + assertEquals(null, cancelled.owner) + assertFalse(cancelled.saved) + sut.retryRecoveryConnection() + advanceUntilIdle() + assertTrue(sut.connectionEditor.value?.owner != null) + verify(lightningRepo, never()).restartNode(anyOrNull(), any()) + } + + @Test + fun `cancelled save cleanup cannot overwrite a newly opened editor`() = test { + sut.openConnectionRecovery() + advanceUntilIdle() + val original = checkNotNull(sut.connectionEditor.value) + sut.editRecoveryElectrum("ssl://electrum.example.com:50002") + val validating = CompletableDeferred() + val release = CompletableDeferred() + whenever(lightningRepo.validateElectrumServer(any())).doSuspendableAnswer { + validating.complete(Unit) + withContext(NonCancellable) { release.await() } + Result.success(Unit) + } + sut.saveRecoveryConnection(electrum = true) + validating.await() + sut.closeConnectionRecovery() + sut.openConnectionRecovery() + runCurrent() + val replacement = checkNotNull(sut.connectionEditor.value) + assertTrue(replacement.id != original.id) + assertTrue(replacement.busy) + release.complete(Unit) + advanceUntilIdle() + val completed = checkNotNull(sut.connectionEditor.value) + assertEquals(replacement.id, completed.id) + assertFalse(completed.busy) + assertFalse(completed.saved) + assertTrue(completed.owner != null) + assertEquals(null, completed.error) + } + + @Test + fun `ordinary startup failure can proceed only after affirmative marker clearance`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + repeat(2) { + sut.onRestoreRetry() + advanceUntilIdle() + } + val failure = checkNotNull(sut.connectionStartupError.value) + assertEquals(2, sut.restoreState.value.retryCount()) + assertTrue(canProceedWithoutRestore(sut.restoreState.value, failure.canProceedWithoutRestore)) + var dismissed = 0 + sut.onProceedWithoutRestore { dismissed++ } + advanceUntilIdle() + assertEquals(1, dismissed) + assertEquals(null, sut.connectionStartupError.value) + assertEquals(RestoreState.Settled, sut.restoreState.value) + } + + @Test + fun `pending or unreadable markers never grant ordinary startup clearance`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + for (unreadable in listOf(false, true)) { + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (unreadable) throw AppError("keychain unavailable") else true + } + sut.onRestoreRetry() + advanceUntilIdle() + assertFalse(checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + } + verify(lightningRepo, never()).stop() + } + + @Test + fun `failed node stop cannot acknowledge proceeding without restore`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + sut.onRestoreRetry() + advanceUntilIdle() + val failure = sut.connectionStartupError.value + lightningState.value = lightningState.value.copy( + nodeLifecycleState = NodeLifecycleState.ErrorStarting(AppError("storage")) + ) + whenever(lightningRepo.stop()).thenReturn(Result.failure(AppError("stop persistence failed"))) + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + assertEquals(failure, sut.connectionStartupError.value) + } + + @Test + fun `marker appearing during proceed delay retains failure and prevents dismissal`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + sut.onRestoreRetry() + advanceUntilIdle() + val failure = sut.connectionStartupError.value + lightningState.value = lightningState.value.copy( + nodeLifecycleState = NodeLifecycleState.ErrorStarting(AppError("storage")) + ) + var pending = false + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { pending } + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + runCurrent() + pending = true + advanceUntilIdle() + assertFalse(dismissed) + assertEquals(failure?.message, sut.connectionStartupError.value?.message) + assertFalse(checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + verify(lightningRepo).stop() + } + + @Test + fun `replacement during failure marker read cannot publish old clearance`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + var reads = 0 + val reading = CompletableDeferred() + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + reads++ + if (reads > 1) { + reading.complete(Unit) + awaitCancellation() + } + false + } + ToastEventBus.events.test { + sut.onRestoreRetry() + reading.await() + connectionRecovery.withIdentityChange { } + advanceUntilIdle() + assertEquals(null, sut.connectionStartupError.value) + assertFalse(sut.connectionRetryBusy.value) + expectNoEvents() + } + } + + @Test + fun `replacement drains proceed stop without old dismissal`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + sut.onRestoreRetry() + advanceUntilIdle() + val failure = sut.connectionStartupError.value + lightningState.value = lightningState.value.copy( + nodeLifecycleState = NodeLifecycleState.ErrorStarting(AppError("storage")) + ) + val stopping = CompletableDeferred() + val release = CompletableDeferred() + whenever(lightningRepo.stop()).doSuspendableAnswer { + stopping.complete(Unit) + withContext(NonCancellable) { release.await() } + Result.success(Unit) + } + var dismissed = false + var replaced = false + sut.onProceedWithoutRestore { dismissed = true } + stopping.await() + val replacement = launch { connectionRecovery.withIdentityChange { replaced = true } } + runCurrent() + assertFalse(replaced) + release.complete(Unit) + replacement.join() + advanceUntilIdle() + assertTrue(replaced) + assertFalse(dismissed) + assertEquals(failure, sut.connectionStartupError.value) + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + } + + @Test + fun `editor retry reports one toast and inline error without stale underlying clearance`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + sut.openConnectionRecovery() + advanceUntilIdle() + ToastEventBus.events.test { + sut.retryRecoveryConnection() + advanceUntilIdle() + assertEquals("storage unavailable", awaitItem().description) + expectNoEvents() + assertEquals("storage unavailable", sut.connectionEditor.value?.error) + assertFalse(checkNotNull(sut.connectionEditor.value).busy) + assertTrue(checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + sut.closeConnectionRecovery() + assertTrue(checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + } + } + + @Test + fun `editor control and cancellation release busy state without toast or clearance`() = test { + ToastEventBus.events.test { + for (error in listOf(NodeStartYieldedToStopError(null), CancellationException("cancelled"))) { + sut.openConnectionRecovery() + advanceUntilIdle() + whenever(lightningRepo.restartNode(anyOrNull(), any())).thenReturn(Result.failure(error)) + sut.retryRecoveryConnection() + advanceUntilIdle() + assertFalse(checkNotNull(sut.connectionEditor.value).busy) + assertFalse(sut.connectionRetryBusy.value) + assertEquals(null, sut.connectionStartupError.value) + expectNoEvents() + sut.closeConnectionRecovery() + } + } + } + + @Test + fun `changed credential owner invalidates editor and permits explicit reopening`() = test { + sut.openConnectionRecovery() + advanceUntilIdle() + val originalOwner = checkNotNull(sut.connectionEditor.value?.owner) + currentBinding = "replacement-wallet-binding" + ToastEventBus.events.test { + sut.retryRecoveryConnection() + advanceUntilIdle() + val invalidated = checkNotNull(sut.connectionEditor.value) + assertFalse(invalidated.busy) + assertEquals(null, invalidated.owner) + assertTrue(invalidated.error.orEmpty().contains("wallet changed")) + assertEquals(null, sut.connectionStartupError.value) + expectNoEvents() + } + sut.retryRecoveryConnection() + advanceUntilIdle() + val reopened = checkNotNull(sut.connectionEditor.value) + assertFalse(reopened.busy) + assertTrue(reopened.owner != null && reopened.owner !== originalOwner) + verify(lightningRepo, never()).restartNode(anyOrNull(), any()) + } + + @Test + fun `each real keychain restore marker separately denies startup clearance`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + val markers = mutableSetOf() + whenever(recoveryKeychain.exists(any())).thenAnswer { markers.contains(it.getArgument(0)) } + whenever( + backupRepo.hasPendingWalletRestore() + ).doSuspendableAnswer { recoveryKeychain.hasPendingWalletRestore() } + for (pending in listOf( + setOf(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name), + setOf(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name), + setOf(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name, Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name), + emptySet(), + )) { + markers.clear() + markers.addAll(pending) + sut.onRestoreRetry() + advanceUntilIdle() + assertEquals(pending.isEmpty(), checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + } + } + + @Test + fun `unavailable final marker read revokes proceed without dismissing`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("storage unavailable"))) + sut.onRestoreRetry() + advanceUntilIdle() + lightningState.value = lightningState.value.copy( + nodeLifecycleState = NodeLifecycleState.ErrorStarting(AppError("storage")) + ) + var reads = 0 + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (++reads > 1) throw AppError("keychain unavailable") + false + } + var dismissed = false + sut.onProceedWithoutRestore { dismissed = true } + advanceUntilIdle() + assertFalse(dismissed) + assertFalse(checkNotNull(sut.connectionStartupError.value).canProceedWithoutRestore) + } + + @Test + fun `admitted editor cancellation permits explicit fresh owner capture`() = test { + val starting = CompletableDeferred() + whenever(lightningRepo.restartNode(anyOrNull(), any())).doSuspendableAnswer { + starting.complete(Unit) + awaitCancellation() + } + sut.openConnectionRecovery() + advanceUntilIdle() + val originalOwner = sut.connectionEditor.value?.owner + sut.retryRecoveryConnection() + starting.await() + connectionRecovery.withIdentityChange { } + advanceUntilIdle() + val cancelled = checkNotNull(sut.connectionEditor.value) + assertFalse(cancelled.busy) + assertEquals(null, cancelled.owner) + sut.retryRecoveryConnection() + advanceUntilIdle() + assertTrue(sut.connectionEditor.value?.owner != null && sut.connectionEditor.value?.owner !== originalOwner) + verify(lightningRepo, times(1)).restartNode(anyOrNull(), any()) + } + + @Test + fun `newer retry prevents held older failure from publishing`() = test { + walletState.value = walletState.value.copy(walletExists = true) + advanceUntilIdle() + whenever( + lightningRepo.start(any(), anyOrNull(), any(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), any()) + ) + .thenReturn(Result.failure(AppError("older failure"))) + whenever(lightningRepo.restartNode(anyOrNull(), any())).thenReturn(Result.success(Unit)) + val reading = CompletableDeferred() + val release = CompletableDeferred() + var first = true + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (first) { + first = false + reading.complete(Unit) + release.await() + } + false + } + ToastEventBus.events.test { + sut.start() + reading.await() + sut.onRestoreRetry() + runCurrent() + release.complete(Unit) + advanceUntilIdle() + assertEquals(null, sut.connectionStartupError.value) + expectNoEvents() + } + } + + @Test + fun `control failure retry has no ordinary toast or clearance`() = test { + ToastEventBus.events.test { + for (error in listOf(NodeStartYieldedToStopError(null), CancellationException("cancelled"))) { + whenever(lightningRepo.restartNode(anyOrNull(), any())).thenReturn(Result.failure(error)) + sut.onRestoreRetry() + advanceUntilIdle() + assertEquals(null, sut.connectionStartupError.value) + assertFalse(sut.connectionRetryBusy.value) + expectNoEvents() + } + } + } + + @Test + fun `unknown restore state is not affirmative proceed permission`() { + assertFalse(canProceedWithoutRestore(RestoreState.Initial, null)) + assertFalse(canProceedWithoutRestore(RestoreState.Retry(2), null)) + assertFalse(canProceedWithoutRestore(RestoreState.BackupFailed(2), null)) + assertTrue(canProceedWithoutRestore(RestoreState.BackupFailed(2, false), null)) + assertFalse(canProceedWithoutRestore(RestoreState.BackupFailed(2, false), false)) + assertTrue(canProceedWithoutRestore(RestoreState.Retry(2), true)) + } + + @Test + fun `ordinary node startup failure emits one error toast`() = test { + whenever(walletRepo.walletExists()).thenReturn(true) + walletState.value = walletState.value.copy(walletExists = true) + whenever( + lightningRepo.start(any(), anyOrNull(), any(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), any()), + ).thenReturn(Result.failure(AppError("node storage unavailable"))) + + ToastEventBus.events.test { + sut.start() + advanceUntilIdle() + assertEquals("node storage unavailable", awaitItem().description) + expectNoEvents() + } + } + + @Test + fun `ordinary node retry failure emits one error toast per attempt`() = test { + whenever( + lightningRepo.restartNode(anyOrNull(), any()) + ).thenReturn(Result.failure(AppError("node storage unavailable"))) + + ToastEventBus.events.test { + repeat(2) { + sut.onRestoreRetry() + advanceUntilIdle() + assertEquals("node storage unavailable", awaitItem().description) + expectNoEvents() + } + } + } + @Test fun `node restore retry remains separate from backup recovery`() = test { sut.onRestoreRetry() diff --git a/changelog.d/next/1685.fixed.md b/changelog.d/next/1685.fixed.md index f75f618af1..63303e553f 100644 --- a/changelog.d/next/1685.fixed.md +++ b/changelog.d/next/1685.fixed.md @@ -1 +1 @@ -Keep payments and new receive addresses unavailable until required wallet recovery finishes, allow connection settings to be corrected safely across retries and restart, and preserve access after unrelated profile loading fails. +Keep payments and new receive addresses unavailable until required wallet recovery finishes, allow connection settings to be corrected safely across retries and restart, and preserve access after unrelated profile loading fails, with a single error notice and safe recovery after ordinary startup failures.