From 2bb36af4bbc03ccb9a3a7bcdbd08dad0338d9098 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:23:17 +0200 Subject: [PATCH 1/5] 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/5] 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/5] 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. From f9ff9d2716fa25c6d71be98582369ce16791d2e8 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 27 Oct 2026 16:16:02 +0200 Subject: [PATCH 4/5] fix: keep ordinary wallet navigation after startup failure --- app/src/main/java/to/bitkit/ui/ContentView.kt | 10 +++- .../to/bitkit/viewmodels/WalletViewModel.kt | 5 ++ .../test/java/to/bitkit/ui/ContentViewTest.kt | 12 ++++ .../java/to/bitkit/ui/WalletViewModelTest.kt | 58 +++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/ui/ContentView.kt b/app/src/main/java/to/bitkit/ui/ContentView.kt index 206b02fa0e..cd0abee0b5 100644 --- a/app/src/main/java/to/bitkit/ui/ContentView.kt +++ b/app/src/main/java/to/bitkit/ui/ContentView.kt @@ -246,6 +246,7 @@ import to.bitkit.viewmodels.ActivityListViewModel import to.bitkit.viewmodels.AppViewModel import to.bitkit.viewmodels.BackupsViewModel import to.bitkit.viewmodels.BlocktankViewModel +import to.bitkit.viewmodels.ConnectionStartupFailure import to.bitkit.viewmodels.CurrencyViewModel import to.bitkit.viewmodels.MainScreenEffect import to.bitkit.viewmodels.RestoreState @@ -255,6 +256,13 @@ import to.bitkit.viewmodels.TransferViewModel import to.bitkit.viewmodels.WalletViewModel import to.bitkit.viewmodels.canProceedWithoutRestore +internal fun shouldPresentWalletRecovery( + walletIsInitializing: Boolean, + restoreState: RestoreState, + connectionFailure: ConnectionStartupFailure?, +): Boolean = walletIsInitializing || restoreState is RestoreState.BackupFailed || + connectionFailure?.requiresRecovery == true + internal fun shouldFinishWalletInitialization( nodeLifecycleState: NodeLifecycleState, restoreState: RestoreState, @@ -442,7 +450,7 @@ fun ContentView( return } - if (walletIsInitializing || backupRestoreFailed || connectionFailure != null) { + if (shouldPresentWalletRecovery(walletIsInitializing, restoreState, connectionFailure)) { if (nodeLifecycleState is NodeLifecycleState.ErrorStarting || backupRestoreFailed || connectionFailure != null ) { diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 16b3cc805f..5039a5957c 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -302,12 +302,16 @@ class WalletViewModel @Inject constructor( runSuspendCatching { connectionRecovery.withOperation(owner) { session -> val markersClear = runSuspendCatching { !backupRepo.hasPendingWalletRestore() }.getOrDefault(false) + val requiresRecovery = !markersClear || runSuspendCatching { + session.pendingRevision() != null + }.getOrDefault(true) 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, + requiresRecovery = requiresRecovery, owner = owner, ) } @@ -1118,6 +1122,7 @@ data class ConnectionRecoveryUiState( data class ConnectionStartupFailure( val message: String, val canProceedWithoutRestore: Boolean = false, + val requiresRecovery: Boolean = true, internal val owner: WalletConnectionRecovery.Owner? = null, ) diff --git a/app/src/test/java/to/bitkit/ui/ContentViewTest.kt b/app/src/test/java/to/bitkit/ui/ContentViewTest.kt index cc75279ca8..e532a35af8 100644 --- a/app/src/test/java/to/bitkit/ui/ContentViewTest.kt +++ b/app/src/test/java/to/bitkit/ui/ContentViewTest.kt @@ -22,6 +22,7 @@ import to.bitkit.ui.components.Sheet import to.bitkit.ui.components.navigateIfNotCurrent import to.bitkit.ui.screens.wallets.receive.ReceiveRoute import to.bitkit.utils.AppError +import to.bitkit.viewmodels.ConnectionStartupFailure import to.bitkit.viewmodels.RestoreState import to.bitkit.viewmodels.TransferEffect import kotlin.test.assertEquals @@ -138,6 +139,17 @@ class ContentViewTest { assertEquals(listOf(false, false, false, false, true, false, false, true), completion) } + @Test + fun `ordinary foreground errors preserve navigation while initialization and required recovery stay restricted`() { + val ordinary = ConnectionStartupFailure("offline", requiresRecovery = false) + val required = ConnectionStartupFailure("restore incomplete") + assertFalse(shouldPresentWalletRecovery(false, RestoreState.Initial, ordinary)) + assertFalse(shouldPresentWalletRecovery(false, RestoreState.Settled, ordinary)) + assertTrue(shouldPresentWalletRecovery(true, RestoreState.Initial, ordinary)) + assertTrue(shouldPresentWalletRecovery(false, RestoreState.BackupFailed(1), ordinary)) + assertTrue(shouldPresentWalletRecovery(false, RestoreState.Initial, required)) + } + @Test fun `running wallet waits for backup and metadata restoration`() { val pendingRestores = listOf( diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index df302c1661..3791702cf6 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -789,6 +789,64 @@ class WalletViewModelTest : BaseUnitTest() { assertEquals("node storage unavailable", awaitItem().description) expectNoEvents() } + assertFalse(shouldPresentWalletRecovery(false, sut.restoreState.value, sut.connectionStartupError.value)) + } + + @Test + fun `foreground failure remains restricted when required recovery is pending or unreadable`() = 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 unavailable"))) + for (unreadable in listOf(false, true)) { + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (unreadable) throw AppError("keychain unavailable") else true + } + sut.start() + advanceUntilIdle() + val failure = checkNotNull(sut.connectionStartupError.value) + assertFalse(failure.canProceedWithoutRestore) + assertTrue(shouldPresentWalletRecovery(false, sut.restoreState.value, failure)) + } + } + + @Test + fun `foreground failure with a saved correction retains the recovery editor route`() = test { + stubRecoveryRecord() + savePendingCorrection() + sut.closeConnectionRecovery() + 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 unavailable"))) + + whenever(lightningRepo.restartNode(anyOrNull(), any())) + .thenReturn(Result.failure(AppError("node unavailable"))) + + sut.start() + advanceUntilIdle() + + val failure = checkNotNull(sut.connectionStartupError.value) + assertTrue(failure.canProceedWithoutRestore) + assertTrue(shouldPresentWalletRecovery(false, sut.restoreState.value, failure)) + assertTrue(recoveryKeychain.loadString(WalletConnectionRecovery.KEY, 0) != null) + } + + @Test + fun `unreadable correction during foreground startup keeps recovery reachable`() = test { + whenever(walletRepo.walletExists()).thenReturn(true) + walletState.value = walletState.value.copy(walletExists = true) + whenever(recoveryKeychain.loadString(WalletConnectionRecovery.KEY, 0)) + .thenAnswer { throw AppError("correction unavailable") } + + sut.start() + advanceUntilIdle() + + assertTrue(shouldPresentWalletRecovery(false, sut.restoreState.value, sut.connectionStartupError.value)) + verify(lightningRepo, never()) + .start(any(), anyOrNull(), any(), anyOrNull(), anyOrNull(), anyOrNull(), anyOrNull(), any()) } @Test From 2986fde93b896e75685b438c83c58a281a6ad2ff Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 27 Oct 2026 19:59:50 +0200 Subject: [PATCH 5/5] fix: release sends refused before native dispatch --- .../to/bitkit/repositories/LightningRepo.kt | 2 +- .../repositories/OnchainSendAttemptStore.kt | 62 +++-- .../repositories/OnchainSendCoordinator.kt | 4 + .../to/bitkit/services/LightningService.kt | 8 +- .../bitkit/services/LightningServiceTest.kt | 229 +++++++++++++++++- 5 files changed, 275 insertions(+), 30 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index bce9e7dbff..d65fccfb67 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt @@ -1656,7 +1656,7 @@ class LightningRepo @Inject constructor( if (error is OnchainSendNotDispatchedError) { runSuspendCatching { onchainSendAttemptStore.releaseBeforeDispatch(attempt.attemptId, attempt.walletIndex) - }.onFailure { Logger.warn("Failed to clear expired preparation", it, context = TAG) } + }.onFailure { Logger.warn("Failed to clear unsubmitted preparation", it, context = TAG) } return@executeWhenNodeRunning Result.failure(error) } val winnerResult = runSuspendCatching { onchainSendAttemptStore.current() } diff --git a/app/src/main/java/to/bitkit/repositories/OnchainSendAttemptStore.kt b/app/src/main/java/to/bitkit/repositories/OnchainSendAttemptStore.kt index 1290a4731c..d32cdc7d22 100644 --- a/app/src/main/java/to/bitkit/repositories/OnchainSendAttemptStore.kt +++ b/app/src/main/java/to/bitkit/repositories/OnchainSendAttemptStore.kt @@ -17,6 +17,7 @@ import org.lightningdevkit.ldknode.SpendableUtxo import to.bitkit.data.keychain.Keychain import to.bitkit.di.IoDispatcher import to.bitkit.ext.nowMillis +import to.bitkit.ext.runSuspendCatching import to.bitkit.models.ActiveOnchainAttemptBackup import to.bitkit.models.PaykitPaymentStateBackup import to.bitkit.models.WalletScope @@ -467,40 +468,49 @@ class OnchainSendAttemptStore @Inject constructor( if (current.hasPositiveEvidence) return@withLock OnchainSendOutcome.Accepted(requireNotNull(current.txid)) require(txid.lowercase() in current.candidateTxids) val firstSubmission = !current.initialDispatchAttempted + val localPreparation = attemptId in inFlightPreparations if (!firstSubmission) { persist( current.copy(txid = txid.lowercase(), evidence = OnchainSendEvidence.Pending, refusalReason = null) ) } try { - broadcast { - // Called on the native queue after wallet/deadline validation, while this mutex is owned. + runSuspendCatching { + broadcast { + // Called on the native queue after wallet/deadline validation, while this mutex is owned. + val live = loadWithRetainedAccepted(walletIndex) + check(live?.attemptId == attemptId && !live.hasPositiveEvidence) + persist(live.copy(initialDispatchAttempted = true)) + inFlightPreparations -= attemptId + } + }.getOrElse { error -> + val nativeRefused = error is PreparedOnchainSendRefused + if (!nativeRefused && error !is ServiceError.PaymentDeadlineExpired) throw error val live = loadWithRetainedAccepted(walletIndex) - check(live?.attemptId == attemptId && !live.hasPositiveEvidence) - persist(live.copy(initialDispatchAttempted = true)) - inFlightPreparations -= attemptId - } - } catch (error: ServiceError.PaymentDeadlineExpired) { - // The native queue checks the deadline before the durable dispatch boundary. - // Any earlier submission or imported backup remains uncertain and guarded. - if (!firstSubmission || loadWithRetainedAccepted(walletIndex)?.initialDispatchAttempted != false || - current.restoredFromBackup || current.evidence != OnchainSendEvidence.Pending || - current.candidateTxids.size != 1 - ) { - throw error - } - persist( - current.copy( - txid = null, - originalInputs = null, - candidateTxids = emptyList(), - candidateFeeRates = emptyMap(), - candidateMiningFees = emptyMap(), - preparationPending = current.requestId != null, + val samePreparation = live == current || nativeRefused && + live == current.copy(initialDispatchAttempted = true) + // Only the native handle can acknowledge refusal after the dispatch write-ahead. + // Earlier submissions, imported backups and changed journals remain guarded. + if (!firstSubmission || !localPreparation || !samePreparation || + walletIndex != lightningService.currentWalletIndex || current.restoredFromBackup || + current.evidence != OnchainSendEvidence.Pending || current.candidateTxids.size != 1 + ) { + throw error + } + persist( + current.copy( + txid = null, + originalInputs = null, + candidateTxids = emptyList(), + candidateFeeRates = emptyMap(), + candidateMiningFees = emptyMap(), + preparationPending = current.requestId != null, + initialDispatchAttempted = false, + ) ) - ) - // Shop proof cleanup still precedes guard deletion through the existing preparation path. - throw OnchainSendNotDispatchedError(error) + // Shop proof cleanup still precedes guard deletion through the existing preparation path. + throw OnchainSendNotDispatchedError(error) + } } finally { inFlightPreparations -= attemptId } diff --git a/app/src/main/java/to/bitkit/repositories/OnchainSendCoordinator.kt b/app/src/main/java/to/bitkit/repositories/OnchainSendCoordinator.kt index fb35f190c4..f8a34de714 100644 --- a/app/src/main/java/to/bitkit/repositories/OnchainSendCoordinator.kt +++ b/app/src/main/java/to/bitkit/repositories/OnchainSendCoordinator.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import to.bitkit.di.IoDispatcher import to.bitkit.ext.runSuspendCatching +import to.bitkit.utils.AppError import kotlin.time.Instant /** Rust converts sat/vB to sat/kwu by multiplying by 250; validate before the unchecked FFI factory. */ @@ -19,6 +20,9 @@ object OnchainRecoveryFeeRate { fun parse(input: String): ULong? = input.toULongOrNull()?.takeIf(::isValid) } +/** The prepared native handle refused this invocation before calling native broadcast. */ +internal class PreparedOnchainSendRefused(cause: Throwable) : AppError(cause) + /** One native prepared transaction; its direct submission is cached even if it fails. */ class PreparedOnchainSend( val receipt: OnchainPreparedReceipt, diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt index eab1894915..aa27587957 100644 --- a/app/src/main/java/to/bitkit/services/LightningService.kt +++ b/app/src/main/java/to/bitkit/services/LightningService.kt @@ -79,6 +79,7 @@ import to.bitkit.repositories.OnchainSendAttempt import to.bitkit.repositories.OnchainSendInput import to.bitkit.repositories.OnchainSendOutcome import to.bitkit.repositories.PreparedOnchainSend +import to.bitkit.repositories.PreparedOnchainSendRefused import to.bitkit.repositories.releaseInputSets import to.bitkit.repositories.releaseOutbids import to.bitkit.utils.AppError @@ -1021,8 +1022,11 @@ class LightningService internal constructor( if (currentWalletIndex != walletIndex || node !== originalNode) throw ServiceError.NodeNotSetup() keychain.requireWalletRestoreComplete() ensurePaymentDeadline(paymentDeadlineAt) - beforeDispatch() - keychain.requireWalletRestoreComplete() + runSuspendCatching { + beforeDispatch() + keychain.requireWalletRestoreComplete() + if (currentWalletIndex != walletIndex || node !== originalNode) throw ServiceError.NodeNotSetup() + }.getOrElse { throw PreparedOnchainSendRefused(it) } when (val result = prepared.broadcast()) { is OnchainSendResult.Accepted -> OnchainSendOutcome.Accepted(result.txid) is OnchainSendResult.Rejected -> OnchainSendOutcome.Rejected(result.txid, result.reason) diff --git a/app/src/test/java/to/bitkit/services/LightningServiceTest.kt b/app/src/test/java/to/bitkit/services/LightningServiceTest.kt index b9456239af..c02ab8c9f3 100644 --- a/app/src/test/java/to/bitkit/services/LightningServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/LightningServiceTest.kt @@ -61,6 +61,7 @@ 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.ext.runSuspendCatching import to.bitkit.models.ActiveOnchainAttemptBackup import to.bitkit.models.WATCH_ONLY_ACCOUNT_HIGHEST_PRE_REVEALED_ADDRESS_INDEX import to.bitkit.models.WalletScope @@ -71,12 +72,17 @@ import to.bitkit.repositories.OnchainSendAttempt import to.bitkit.repositories.OnchainSendAttemptStore import to.bitkit.repositories.OnchainSendCoordinator import to.bitkit.repositories.OnchainSendInput +import to.bitkit.repositories.OnchainSendNotDispatchedError import to.bitkit.repositories.OnchainSendOutcome import to.bitkit.repositories.OnchainTransferContext +import to.bitkit.repositories.PaykitPaymentRequestId import to.bitkit.repositories.PreparedOnchainSend +import to.bitkit.repositories.PreparedOnchainSendRefused import to.bitkit.test.BaseUnitTest import to.bitkit.utils.LoggerLdk import to.bitkit.utils.ServiceError +import java.io.File +import java.io.FileOutputStream import java.util.concurrent.CountDownLatch import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.assertEquals @@ -84,9 +90,11 @@ import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.time.Clock import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Instant import com.synonym.bitkitcore.AddressType as BitkitAddressType +import org.lightningdevkit.ldknode.PreparedOnchainSend as NativePreparedOnchainSend private const val VERIFY_TIMEOUT_MS = 2_000L private const val RELEASE_GATE_PROBE_MS = 200L @@ -155,10 +163,229 @@ class LightningServiceTest : BaseUnitTest() { whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) } } - assertTrue(result.exceptionOrNull() is WalletBackupRestorePendingError) + assertTrue(result.exceptionOrNull() is PreparedOnchainSendRefused) verify(native, never()).broadcast() } + @Test + fun `restore refusal after durable dispatch marker releases only local unsubmitted send`() = test { + for (marker in listOf(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE, Keychain.Key.USDT_PENDING_BACKUP_RESTORE)) { + val fixture = preparedJournal() + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) whenever(keychain.exists(marker.name)).thenReturn(true) + } + assertFailsWith { fixture.broadcast() } + verify(fixture.native, never()).broadcast() + fixture.store.releaseBeforeDispatch(fixture.attempt.attemptId, 0) + assertNull(fixture.store.current()) + whenever(keychain.exists(marker.name)).thenReturn(false) + fixture.onSave = {} + val next = preparedJournal(existing = fixture) + assertEquals(OnchainSendOutcome.Accepted(next.prepared.receipt.txid), next.broadcast()) + } + } + + @Test + fun `unreadable recovery after dispatch marker refuses without uncertain send`() = test { + val fixture = preparedJournal() + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) { + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)) + .thenThrow(IllegalStateException("unreadable")) + } + } + assertFailsWith { fixture.broadcast() } + verify(fixture.native, never()).broadcast() + fixture.store.releaseBeforeDispatch(fixture.attempt.attemptId, 0) + assertNull(fixture.store.current()) + } + + @Test + fun `refused Shop send retains guard until original proof cleanup succeeds`() = test { + val fixture = preparedJournal(shop = true) + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) { + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + } + } + assertFailsWith { fixture.broadcast() } + fixture.store.releaseBeforeDispatch(fixture.attempt.attemptId, 0) + val pending = requireNotNull(fixture.store.current()) + assertTrue(pending.preparationPending) + assertFalse(pending.initialDispatchAttempted) + assertTrue(pending.candidateTxids.isEmpty()) + assertFalse(fixture.store.releaseInterruptedShopPreparation { false }) + assertEquals(pending, fixture.store.current()) + assertTrue( + fixture.store.releaseInterruptedShopPreparation { + assertEquals(pending, fixture.read()) + true + } + ) + assertNull(fixture.store.current()) + verify(fixture.native, never()).broadcast() + } + + @Test + fun `failed refusal rollback keeps durable uncertain candidate`() = test { + val fixture = preparedJournal() + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) { + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + } else if (saved.candidateTxids.isEmpty()) { + error("rollback unavailable") + } + } + assertTrue(runSuspendCatching { fixture.broadcast() }.isFailure) + val retained = requireNotNull(fixture.read()) + assertTrue(retained.initialDispatchAttempted) + assertEquals(listOf(fixture.prepared.receipt.txid), retained.candidateTxids) + assertEquals(retained, fixture.store.current()) + verify(fixture.native, never()).broadcast() + } + + @Test + fun `wallet or journal replacement during dispatch marker never releases another guard`() = test { + for (replaceWallet in listOf(false, true)) { + setWalletIndex(0) + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(false) + val fixture = preparedJournal() + var replacement: OnchainSendAttempt? = null + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) { + if (replaceWallet) { + setWalletIndex(1) + } else { + replacement = saved.copy(attemptId = "another-attempt") + fixture.file.writeText(Json.encodeToString(replacement)) + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + } + } + } + assertTrue(runSuspendCatching { fixture.broadcast() }.isFailure) + verify(fixture.native, never()).broadcast() + val retained = requireNotNull(fixture.read()) + assertTrue(retained.initialDispatchAttempted) + if (replaceWallet) { + assertEquals(fixture.attempt.attemptId, retained.attemptId) + } else { + assertEquals(replacement, retained) + } + } + } + + @Test + fun `prior or imported candidate survives a locally refused native invocation`() = test { + for (imported in listOf(false, true)) { + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(false) + val fixture = preparedJournal() + val retained = requireNotNull(fixture.read()).copy( + initialDispatchAttempted = !imported, + restoredFromBackup = imported, + ) + fixture.file.writeText(Json.encodeToString(retained)) + fixture.onSave = { saved -> + if (saved.initialDispatchAttempted) { + whenever(keychain.exists(Keychain.Key.USDT_PENDING_BACKUP_RESTORE.name)).thenReturn(true) + } + } + assertTrue(runSuspendCatching { fixture.broadcast() }.isFailure) + verify(fixture.native, never()).broadcast() + assertEquals(retained.copy(initialDispatchAttempted = true), fixture.read()) + val reopened = OnchainSendAttemptStore(testDispatcher, keychain, sut, Clock.System) + assertEquals(fixture.read(), reopened.current()) + } + } + + @Test + fun `positive journal winner is retained without invoking native broadcast`() = test { + val fixture = preparedJournal() + val outcome = OnchainSendOutcome.Accepted(fixture.prepared.receipt.txid) + val winner = fixture.store.recordOutcome(fixture.attempt.attemptId, outcome, 0) + assertEquals(outcome, fixture.broadcast()) + assertEquals(winner, fixture.read()) + verify(fixture.native, never()).broadcast() + } + + @Test + fun `native error or unknown outcome retains dispatched journal`() = test { + for (throws in listOf(false, true)) { + val fixture = preparedJournal() + whenever(fixture.native.broadcast()).thenAnswer { + if (throws) throw WalletBackupRestorePendingError() + OnchainSendResult.Unknown(fixture.prepared.receipt.txid) + } + val result = runSuspendCatching { fixture.broadcast() } + if (throws) { + assertTrue(result.exceptionOrNull() is WalletBackupRestorePendingError) + } else { + assertEquals(OnchainSendOutcome.Unknown(fixture.prepared.receipt.txid), result.getOrThrow()) + } + verify(fixture.native).broadcast() + val retained = requireNotNull(fixture.read()) + assertTrue(retained.initialDispatchAttempted) + assertEquals(listOf(fixture.prepared.receipt.txid), retained.candidateTxids) + assertEquals(retained, fixture.store.current()) + } + } + + private fun setWalletIndex(index: Int) { + LightningService::class.java.getDeclaredField("currentWalletIndex").apply { + isAccessible = true + set(sut, index) + } + } + + private class PreparedJournal( + val store: OnchainSendAttemptStore, + val attempt: OnchainSendAttempt, + val prepared: PreparedOnchainSend, + val native: NativePreparedOnchainSend, + val file: File, + ) { + var onSave: (OnchainSendAttempt) -> Unit = {} + fun read(): OnchainSendAttempt? = file.readText().takeIf(String::isNotEmpty)?.let { + Json.decodeFromString(it) + } + suspend fun broadcast() = store.broadcastPreparedCandidate(attempt.attemptId, 0, prepared.receipt.txid) { + prepared.broadcast(it) + } + } + + private suspend fun preparedJournal(shop: Boolean = false, existing: PreparedJournal? = null): PreparedJournal { + val file = existing?.file ?: tempFolder.newFile() + val key = Keychain.Key.ONCHAIN_SEND_ATTEMPT.name + var fixture: PreparedJournal? = null + whenever(keychain.loadString(key, 0)).thenAnswer { file.readText().takeIf(String::isNotEmpty) } + whenever(keychain.upsertString(eq(key), any(), eq(0))).doSuspendableAnswer { + val value = it.getArgument(1) + val saved = Json.decodeFromString(value) + if (!saved.initialDispatchAttempted) fixture?.onSave?.invoke(saved) + FileOutputStream(file).use { output -> + output.write(value.toByteArray()) + output.fd.sync() + } + if (saved.initialDispatchAttempted) fixture?.onSave?.invoke(saved) + } + whenever(keychain.delete(key, 0)).doSuspendableAnswer { file.writeText("") } + val store = existing?.store ?: OnchainSendAttemptStore(testDispatcher, keychain, sut, Clock.System) + val attempt = store.admit( + walletId = WalletScope.default, + requestId = if (shop) PaykitPaymentRequestId("request", "payer") else null, + orderId = null, address = "address", amountSats = 1_000uL, isMaxAmount = false, + feeRateSatsPerVByte = 1uL, isTransfer = false, channelId = null, tags = emptyList(), + beforeSendAttempt = {}, payerIdentity = if (shop) "payer" else null, + ) + 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) + store.retainPreparedReceipt(attempt.attemptId, 0, prepared.receipt, false) + return PreparedJournal(store, attempt, prepared, native, file).also { fixture = it } + } + @Test fun `local transaction details cannot cross a wallet or node replacement`() = test { val txid = "ab".repeat(32)