From 1a733537b0f2843b7277dca9a7adb2f351b8c0b5 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 28 Oct 2026 07:29:49 +0200 Subject: [PATCH 1/5] fix: keep send recovery checks off main --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 104 +++++-- .../to/bitkit/repositories/BackupRepoTest.kt | 44 ++- .../viewmodels/AppViewModelSendFlowTest.kt | 261 ++++++++++++++++++ changelog.d/next/1725.fixed.md | 1 + 4 files changed, 381 insertions(+), 29 deletions(-) create mode 100644 changelog.d/next/1725.fixed.md diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 8e023e93d2..bf966af790 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -44,6 +44,7 @@ import kotlinx.collections.immutable.toImmutableMap import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Deferred import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable @@ -93,7 +94,6 @@ import to.bitkit.data.CacheStore 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.data.resetPin import to.bitkit.di.BgDispatcher import to.bitkit.domain.commands.NotifyChannelReady @@ -360,7 +360,10 @@ class AppViewModel @Inject constructor( private val _sendEffect = MutableSharedFlow(extraBufferCapacity = 1) val sendEffect = _sendEffect.asSharedFlow() - private fun setSendEffect(effect: SendEffect, canPresent: () -> Boolean = { true }) = viewModelScope.launch { + private fun setSendEffect( + effect: SendEffect, + canPresent: suspend () -> Boolean = { true }, + ) = viewModelScope.launch { if (canPresent()) _sendEffect.emit(effect) } @@ -371,7 +374,7 @@ class AppViewModel @Inject constructor( val mainScreenEffect = _mainScreenEffect.asSharedFlow() private fun mainScreenEffect( effect: MainScreenEffect, - canPresent: () -> Boolean = { true } + canPresent: suspend () -> Boolean = { true }, ) = viewModelScope.launch { if (canPresent()) _mainScreenEffect.emit(effect) } @@ -2203,10 +2206,14 @@ class AppViewModel @Inject constructor( ?.cancel() synchronized(deferredScanLock) { if (targetsRequest(deferredScan?.contactPaymentContext)) { + val admission = deferredScan?.admission deferredScan = null + admission?.cancel() } if (targetsRequest(scanAfterPubkyAuth?.contactPaymentContext)) { + val admission = scanAfterPubkyAuth?.admission scanAfterPubkyAuth = null + admission?.cancel() } } val shouldHideRequestSendSheet = if (requestId != null) { @@ -3356,13 +3363,17 @@ class AppViewModel @Inject constructor( allowPubkyAuth: Boolean = isMainScanner, suppressQuickPay: Boolean = false, walletGeneration: Long? = connectionRecovery.currentGeneration(), + receiptAdmission: Deferred? = null, ): Job? { - if (!canPrepareSend(walletGeneration)) return null + if (!connectionRecovery.isCurrentGeneration(walletGeneration)) return null if (contactPaymentContext == null && isUsdtPaymentInput(data.trim()) && hasActiveUsdtSend() ) { return null } + val admission = receiptAdmission ?: viewModelScope.async(start = CoroutineStart.LAZY) { + isWalletReadyForSend(walletGeneration) + } val previousPreparation = paymentRequestPreparation.value?.takeUnless { it.request.id == contactPaymentContext?.incomingPaymentRequest?.id } @@ -3375,9 +3386,24 @@ class AppViewModel @Inject constructor( allowPubkyAuth = allowPubkyAuth, suppressQuickPay = suppressQuickPay, walletGeneration = walletGeneration, + admission = admission, ) + if (receiptAdmission == null) { + admission.invokeOnCompletion { + viewModelScope.launch { + if (admission.isCancelled || !admission.await()) { + synchronized(deferredScanLock) { + if (deferredScan?.admission === admission) deferredScan = null + if (scanAfterPubkyAuth?.admission === admission) scanAfterPubkyAuth = null + } + flushDeferredScan() + } + } + } + } if (!_isAuthenticated.value) { enqueueDeferredScan(deferrable) + admission.start() return null } @@ -3390,21 +3416,25 @@ class AppViewModel @Inject constructor( (scheduled.contactPaymentContext == contactPaymentContext || contactPaymentContext == null) if (isSameActiveScan) { Logger.info("Skipping duplicate scan from '${source.label}': '$scanId'", context = TAG) + if (receiptAdmission == null) admission.cancel() return null } if (scheduled?.job?.isActive == true && scheduled.mustComplete) { enqueueDeferredScan(deferrable) + admission.start() return null } if (isPubkyAuthSheetShownOrPending() && !deferrable.isPubkyAuth) { enqueueDeferredScan(deferrable) + admission.start() return null } val previousJob = scheduled?.job val nextJob = viewModelScope.launch(start = CoroutineStart.LAZY) { + if (!admission.await() || !canPrepareSend(walletGeneration)) return@launch clearPaymentRequestPreparation(previousPreparation) scanMutex.withLock { try { @@ -3437,6 +3467,7 @@ class AppViewModel @Inject constructor( scheduledScan = nextScheduledScan nextJob.invokeOnCompletion { + admission.cancel() if (scheduledScan === nextScheduledScan) scheduledScan = null if (nextJob.isCancelled) return@invokeOnCompletion clearCancelledContactContext(contactPaymentContext) viewModelScope.launch { @@ -3585,7 +3616,10 @@ class AppViewModel @Inject constructor( if (queued?.walletGeneration == scan.walletGeneration && queued?.data?.removeLightningSchemes() == normalized ) { - deferredScan = queued.mergeDuplicate(scan, scanId) + val merged = queued.mergeDuplicate(scan, scanId) + deferredScan = merged + if (merged.admission !== queued.admission) queued.admission.cancel() + if (merged.admission !== scan.admission) scan.admission.cancel() return } if (!scan.isPubkyAuth && (queued?.isPubkyAuth == true || scanAfterPubkyAuth != null)) { @@ -3593,10 +3627,14 @@ class AppViewModel @Inject constructor( if (followUp?.walletGeneration == scan.walletGeneration && followUp?.data?.removeLightningSchemes() == normalized ) { - scanAfterPubkyAuth = followUp.mergeDuplicate(scan, scanId) + val merged = followUp.mergeDuplicate(scan, scanId) + scanAfterPubkyAuth = merged + if (merged.admission !== followUp.admission) followUp.admission.cancel() + if (merged.admission !== scan.admission) scan.admission.cancel() return } scanAfterPubkyAuth = scan + followUp?.admission?.cancel() Logger.info("Queuing '${scan.source.label}' scan after a sign-in link: '$scanId'", context = TAG) return } @@ -3614,6 +3652,7 @@ class AppViewModel @Inject constructor( ) } deferredScan = scan + queued?.admission?.cancel() } Logger.info("Queuing '${scan.source.label}' scan for deferred handling: '$scanId'", context = TAG) } @@ -3666,6 +3705,7 @@ class AppViewModel @Inject constructor( allowPubkyAuth = pending.allowPubkyAuth, suppressQuickPay = pending.suppressQuickPay, walletGeneration = pending.walletGeneration, + receiptAdmission = pending.admission, ) } @@ -3783,8 +3823,9 @@ class AppViewModel @Inject constructor( walletGeneration: Long?, ) { requestUsdtEnable(onCancel = { cancelUsdtContactPayment(payment) }) { - val currentPayment = synchronized(contactPaymentContextLock) { activeContactPaymentContext } - if (canPrepareSend(walletGeneration) && currentPayment == payment) { + if (canPrepareSend(walletGeneration) && + synchronized(contactPaymentContextLock) { activeContactPaymentContext } == payment + ) { openPaykitEndpoints(payment, fromMainScanner, walletGeneration) } } @@ -4661,7 +4702,7 @@ class AppViewModel @Inject constructor( } } - private fun handleUsdtPaymentInput(input: String, walletGeneration: Long?) { + private suspend fun handleUsdtPaymentInput(input: String, walletGeneration: Long?) { if (!canPrepareSend(walletGeneration)) return val request = if (Env.isUsdtEnabled && activeHardwareWalletId == null && !usdtRepo.isSendPresented) { parseUsdtPaymentRequest(input).getOrNull() @@ -5525,7 +5566,7 @@ class AppViewModel @Inject constructor( showSheet(Sheet.LnurlAuth(domain = data.domain, lnurl = data.uri, k1 = data.k1)) } - private fun navigateToSendRoute( + private suspend fun navigateToSendRoute( fromMainScanner: Boolean, route: SendRoute, effect: SendEffect, @@ -6860,7 +6901,7 @@ class AppViewModel @Inject constructor( private fun refreshOnchainSendIfNeeded( walletGeneration: Long? = connectionRecovery.currentGeneration(), ): Job? { - if (!canPrepareSend(walletGeneration)) return null + if (!connectionRecovery.isCurrentGeneration(walletGeneration)) return null val generation = sendStateGeneration val currentState = _sendUiState.value if (currentState.isRetainedHardwarePayment) return null @@ -6871,11 +6912,11 @@ class AppViewModel @Inject constructor( return null } - updateOnchainFeeUi { it.copy(isLoading = true) } onchainSendRefreshJob?.cancel() val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launch + updateOnchainFeeUi { it.copy(isLoading = true) } val settings = settingsStore.data.first() requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launch @@ -7105,16 +7146,25 @@ class AppViewModel @Inject constructor( if (!canPrepareSend(walletGeneration)) throw CancellationException("Payment preparation owner changed") } - private fun canPrepareSend(walletGeneration: Long?): Boolean { + private suspend fun canPrepareSend(walletGeneration: Long?): Boolean { + val generation = sendStateGeneration + val preparation = paymentRequestPreparation.value + val payment = synchronized(contactPaymentContextLock) { activeContactPaymentContext } + return isWalletReadyForSend(walletGeneration) && + generation == sendStateGeneration && preparation === paymentRequestPreparation.value && + synchronized(contactPaymentContextLock) { activeContactPaymentContext } == payment + } + + private suspend fun isWalletReadyForSend(walletGeneration: Long?): Boolean { + currentCoroutineContext().ensureActive() if (!connectionRecovery.isCurrentGeneration(walletGeneration)) return false - return try { - !keychain.hasPendingWalletRestore() - } catch (cancelled: CancellationException) { - throw cancelled - } catch (error: Throwable) { - Logger.warn("Unable to read required wallet recovery before payment entry", error, context = TAG) - false - } + val pending = runSuspendCatching { backupRepo.hasPendingWalletRestore() } + .onFailure { + Logger.warn("Unable to read required wallet recovery before payment entry", it, context = TAG) + } + .getOrDefault(true) + currentCoroutineContext().ensureActive() + return !pending && connectionRecovery.isCurrentGeneration(walletGeneration) } // endregion @@ -7258,14 +7308,13 @@ class AppViewModel @Inject constructor( } } - fun showSheet(sheetType: Sheet, canPresent: () -> Boolean = { true }) { - if (!canPresent()) return - timedSheetManager.cancelPendingCheck() - if (sheetType !== paymentRequestPreparation.value?.sheet) clearPaymentRequestPreparation() + fun showSheet(sheetType: Sheet, canPresent: suspend () -> Boolean = { true }) { val previousJob = sheetTransitionJob transitionSheet = sheetType val nextJob = viewModelScope.launch(start = CoroutineStart.LAZY) { if (!canPresent()) return@launch + timedSheetManager.cancelPendingCheck() + if (sheetType !== paymentRequestPreparation.value?.sheet) clearPaymentRequestPreparation() receiveSheetContext = null if (_currentSheet.value != null && !replacesSubscriptionReview(sheetType)) { val replacesTimedSheet = _currentSheet.value is Sheet.TimedSheet @@ -8103,15 +8152,15 @@ class AppViewModel @Inject constructor( tags: ImmutableList, ) { val walletGeneration = connectionRecovery.currentGeneration() - if (!canPrepareSend(walletGeneration)) return + if (!connectionRecovery.isCurrentGeneration(walletGeneration)) return if (!_isRetryingInitialSubscriptionPayment.compareAndSet(false, true)) { toast(PaykitPaymentRequestError.OperationInProgress) return } - clearActiveContactPaymentContext() viewModelScope.launch { try { if (!canPrepareSend(walletGeneration)) return@launch + clearActiveContactPaymentContext() refreshIncomingPaykitPaymentRequests(forceFresh = true) if (!canPrepareSend(walletGeneration)) return@launch val request = paykitPaymentRequestRepo.pendingRequest(id) ?: run { @@ -8585,6 +8634,7 @@ private data class DeferredScan( val allowPubkyAuth: Boolean, val suppressQuickPay: Boolean, val walletGeneration: Long?, + val admission: Deferred, ) { val isPubkyAuth: Boolean get() = allowPubkyAuth && PubkyAuthRequest.isProtocolUrl(data) } diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index 4848b75228..f666fed4c0 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -6,7 +6,10 @@ import com.synonym.bitkitcore.PreActivityMetadata import com.synonym.paykit.PaykitException import com.synonym.vssclient.VssItem import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.flow.MutableSharedFlow @@ -17,6 +20,8 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.jsonObject import org.junit.Before @@ -75,15 +80,20 @@ import to.bitkit.services.PaykitSdkService import to.bitkit.test.BaseUnitTest import to.bitkit.test.connectionRecoveryFixture import to.bitkit.utils.AppError +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import javax.inject.Provider import kotlin.coroutines.cancellation.CancellationException import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Clock import kotlin.time.Duration.Companion.minutes +import kotlin.time.Duration.Companion.seconds import kotlin.time.ExperimentalTime import kotlin.time.Instant @@ -194,6 +204,36 @@ class BackupRepoTest : BaseUnitTest() { sut = createSut() } + @Test + fun `restore marker storage reads leave Main responsive`() = test { + val entered = CompletableDeferred() + val release = CountDownLatch(1) + val mainThread = Thread.currentThread() + whenever(keychain.exists(Keychain.Key.PAYKIT_PENDING_BACKUP_RESTORE.name)).thenAnswer { + entered.complete(Thread.currentThread()) + check(release.await(5, TimeUnit.SECONDS)) { "Marker read was not released" } + false + } + Executors.newSingleThreadExecutor().asCoroutineDispatcher().use { ioDispatcher -> + sut = createSut(ioDispatcher) + try { + val read = async(Dispatchers.Main) { sut.hasPendingWalletRestore() } + val storageThread = withContext(Dispatchers.Default) { + withTimeout(5.seconds) { entered.await() } + } + assertNotEquals(mainThread, storageThread) + assertTrue(async(Dispatchers.Main) { true }.await()) + assertFalse(read.isCompleted) + + release.countDown() + + assertFalse(read.await()) + } finally { + release.countDown() + } + } + } + @Test fun `wallet backup waits for submission and includes the latest pending state`() = test { val statuses = MutableStateFlow( @@ -2175,9 +2215,9 @@ class BackupRepoTest : BaseUnitTest() { verify(vssBackupClient, never()).getObject(any()) } - private fun createSut() = BackupRepo( + private fun createSut(ioDispatcher: CoroutineDispatcher = testDispatcher) = BackupRepo( context = context, - ioDispatcher = testDispatcher, + ioDispatcher = ioDispatcher, cacheStore = cacheStore, vssBackupClient = vssBackupClient, vssBackupClientLdk = vssBackupClientLdk, diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 56c5f144fe..84580a6d73 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -108,6 +108,7 @@ import to.bitkit.data.dto.PlatformDetails import to.bitkit.data.dto.Platforms import to.bitkit.data.dto.ReleaseInfoDTO import to.bitkit.data.keychain.Keychain +import to.bitkit.data.keychain.hasPendingWalletRestore import to.bitkit.domain.commands.NotifyChannelReadyHandler import to.bitkit.domain.commands.NotifyPaymentReceived import to.bitkit.domain.commands.NotifyPaymentReceivedHandler @@ -706,6 +707,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { whenever(walletRepo.getOnchainAddress()).thenAnswer { walletState.value.onchainAddress } whenever(walletRepo.walletExists()).thenReturn(true) whenever(backupRepo.isRestoring).thenReturn(MutableStateFlow(false)) + whenever { backupRepo.hasPendingWalletRestore() }.thenAnswer { keychain.hasPendingWalletRestore() } stubSettingsStore() whenever(cacheStore.data).thenReturn(flowOf(AppCacheData())) whenever { quickPayRepo.canApply(any()) }.thenReturn(Result.success(false)) @@ -4304,6 +4306,254 @@ class AppViewModelSendFlowTest : BaseUnitTest() { verify(toastManager, never()).enqueue(any()) } + @Test + fun `slow recovery reads yield Main and withhold send until clear`() = test { + whenever(coreService.validateBitcoinAddress(REGTEST_ADDRESS)).thenReturn( + ValidationResult(REGTEST_ADDRESS, NetworkType.REGTEST, AddressType.P2WPKH), + ) + balanceState.value = BalanceState(maxSendOnchainSats = 100_000u) + sut.setIsAuthenticated(true) + val input = "bitcoin:$REGTEST_ADDRESS?amount=0.0001" + whenever(coreService.decode(input)).thenReturn( + Scanner.OnChain(OnChainInvoice(REGTEST_ADDRESS, 10_000u, null, null, null)), + ) + for (outcome in listOf(Result.success(false), Result.success(true), Result.failure(AppError("Unavailable")))) { + sut.hideSheet() + val entered = CompletableDeferred() + val release = CompletableDeferred() + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + entered.complete(Unit) + release.await() + outcome.getOrThrow() + } + clearInvocations(blocktankRepo) + + sut.onScanResult(input) + runCurrent() + + assertTrue(entered.isCompleted) + var mainProgressed = false + launch(Dispatchers.Main) { mainProgressed = true } + runCurrent() + assertTrue(mainProgressed) + assertNull(sut.currentSheet.value) + verify(blocktankRepo, never()).refreshInfo() + + release.complete(Unit) + advanceUntilIdle() + + if (outcome.getOrNull() == false) { + assertTrue(sut.currentSheet.value is Sheet.Send) + } else { + assertNull(sut.currentSheet.value) + verify(blocktankRepo, never()).refreshInfo() + } + } + } + + @Test + fun `new scan cancels an older pending recovery read`() = test { + val olderInput = "lnbcrt1olderrecoveryread" + val newerInput = "lnbcrt1newerrecoveryread" + sut.setIsAuthenticated(true) + stubLightningScan(bolt11 = newerInput, amountSats = 600u) + val entered = CompletableDeferred() + val release = CompletableDeferred() + var firstRead = true + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (firstRead) { + firstRead = false + entered.complete(Unit) + release.await() + } + false + } + val older = sut.openContactPayment(olderInput, testPublicKey) + runCurrent() + assertTrue(entered.isCompleted) + + sut.onScanResult(newerInput) + release.complete(Unit) + advanceUntilIdle() + + assertTrue(older?.isCancelled == true) + verify(coreService, never()).decode(olderInput) + assertEquals(600u, sut.sendUiState.value.amount) + assertTrue(sut.currentSheet.value is Sheet.Send) + } + + @Test + fun `queued duplicate cannot replace the original recovery admission`() = test { + val input = "lnbcrt1blockedqueuedduplicate" + sut.setIsAuthenticated(false) + val entered = CompletableDeferred() + val release = CompletableDeferred() + var firstRead = true + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (firstRead) { + firstRead = false + entered.complete(Unit) + release.await() + true + } else { + false + } + } + sut.openContactPayment(input, testPublicKey) + runCurrent() + assertTrue(entered.isCompleted) + sut.onScanResult(input) + sut.setIsAuthenticated(true) + + release.complete(Unit) + advanceUntilIdle() + + verify(coreService, never()).decode(input) + assertNull(sut.currentSheet.value) + } + + @Test + fun `showing the same request during a recovery read preserves fee preparation`() = test { + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + val request = paymentRequest() + val input = "lnbcrt1visiblerequestrecoveryread" + stubLightningScan(bolt11 = input, amountSats = 0u) + balanceState.value = BalanceState(maxSendLightningSats = 100_000u) + whenever(lightningRepo.canSend(request.amount.atomic)).thenReturn(true) + var feesEstimated = false + whenever(lightningRepo.estimateRoutingFeesForAmount(input, request.amount.atomic)).doSuspendableAnswer { + feesEstimated = true + Result.success(17uL) + } + sut.setIsAuthenticated(true) + runCurrent() + val entered = CompletableDeferred() + val release = CompletableDeferred() + var held = false + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (!held && feesEstimated) { + held = true + entered.complete(Unit) + release.await() + } + false + } + val scan = sut.openContactPayment(input, testPublicKey, incomingPaymentRequest = request) + var cancellation: Throwable? = null + scan?.invokeOnCompletion { cancellation = it } + advanceUntilIdle() + assertTrue(entered.isCompleted) + assertEquals(Sheet.Send(SendRoute.Confirm), sut.currentSheet.value) + assertFalse(scan?.isCancelled == true, "Cancelled before visibility: $cancellation") + + sut.onSheetVisible(sut.currentSheet.value) + release.complete(Unit) + advanceUntilIdle() + + assertFalse(scan?.isCancelled == true, cancellation?.stackTraceToString()) + assertEquals(request, activeContactPaymentContext()?.incomingPaymentRequest) + assertEquals(17L, sut.sendUiState.value.lightningFeeSats) + } + + @Test + fun `wallet replacement during recovery read refuses send initialization`() = test { + val entered = CompletableDeferred() + val release = CompletableDeferred() + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + entered.complete(Unit) + release.await() + false + } + clearInvocations(blocktankRepo) + val before = sut.sendUiState.value + val reset = async { sut.resetSendState() } + runCurrent() + assertTrue(entered.isCompleted) + + walletGeneration++ + release.complete(Unit) + + assertFalse(reset.await()) + assertEquals(before, sut.sendUiState.value) + verify(blocktankRepo, never()).refreshInfo() + } + + @Test + fun `older recovery read cannot replace a newer send request`() = test { + val entered = CompletableDeferred() + val release = CompletableDeferred() + var firstRead = true + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + if (firstRead) { + firstRead = false + entered.complete(Unit) + release.await() + } + false + } + val older = async { sut.resetSendState(paymentRequestNote = "Older request") } + runCurrent() + assertTrue(entered.isCompleted) + assertTrue(sut.resetSendState(paymentRequestNote = "Newer request")) + val newer = sut.sendUiState.value + + release.complete(Unit) + + assertFalse(older.await()) + assertEquals(newer, sut.sendUiState.value) + } + + @Test + fun `cancelling a recovery read prevents send initialization`() = test { + val entered = CompletableDeferred() + val release = CompletableDeferred() + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + entered.complete(Unit) + release.await() + false + } + clearInvocations(blocktankRepo) + val before = sut.sendUiState.value + val reset = async { sut.resetSendState() } + runCurrent() + assertTrue(entered.isCompleted) + + reset.cancelAndJoin() + release.complete(Unit) + advanceUntilIdle() + + assertTrue(reset.isCancelled) + assertEquals(before, sut.sendUiState.value) + verify(blocktankRepo, never()).refreshInfo() + } + + @Test + fun `replacing a sheet cancels its pending admission`() = test { + val entered = CompletableDeferred() + val release = CompletableDeferred() + var cancelled = false + sut.showSheet(Sheet.Send()) { + entered.complete(Unit) + try { + release.await() + true + } finally { + cancelled = true + } + } + runCurrent() + assertTrue(entered.isCompleted) + assertNull(sut.currentSheet.value) + + sut.showScannerSheet() + release.complete(Unit) + advanceUntilIdle() + + assertTrue(cancelled) + assertTrue(sut.currentSheet.value is Sheet.QrScanner) + } + @Test fun `required wallet recovery rejects a payment deeplink before fee lookup`() = test { sut.setIsAuthenticated(true) @@ -9449,6 +9699,13 @@ class AppViewModelSendFlowTest : BaseUnitTest() { fun `new scan waits for an active locked replay`() = test { val lockedInvoice = "lnbcrt1activelockedreplay" val newInvoice = "lnbcrt1afteractivelockedreplay" + val markerReadStarted = CompletableDeferred() + val releaseMarkerRead = CompletableDeferred() + whenever(backupRepo.hasPendingWalletRestore()).doSuspendableAnswer { + markerReadStarted.complete(Unit) + releaseMarkerRead.await() + false + } val lockedScanStarted = CompletableDeferred() val finishLockedScan = CompletableDeferred() var newScanDecodeCount = 0 @@ -9465,7 +9722,11 @@ class AppViewModelSendFlowTest : BaseUnitTest() { whenever(lightningRepo.canSend(any())).thenReturn(true) sut.onScanResult(lockedInvoice) + runCurrent() + assertTrue(markerReadStarted.isCompleted) sut.setIsAuthenticated(true) + assertFalse(lockedScanStarted.isCompleted) + releaseMarkerRead.complete(Unit) lockedScanStarted.await() sut.onScanResult(newInvoice) runCurrent() diff --git a/changelog.d/next/1725.fixed.md b/changelog.d/next/1725.fixed.md new file mode 100644 index 0000000000..e43355706f --- /dev/null +++ b/changelog.d/next/1725.fixed.md @@ -0,0 +1 @@ +Keep payment entry responsive while checking wallet recovery. From 9f2d2b0010435d3b557328fff95182cb4a00f94f Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 28 Oct 2026 07:51:22 +0200 Subject: [PATCH 2/5] fix: keep hardware fees loading during recovery --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 2 +- .../viewmodels/AppViewModelSendFlowTest.kt | 49 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index bf966af790..adfff36b07 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -6912,11 +6912,11 @@ class AppViewModel @Inject constructor( return null } + updateOnchainFeeUi { it.copy(isLoading = true) } onchainSendRefreshJob?.cancel() val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launch - updateOnchainFeeUi { it.copy(isLoading = true) } val settings = settingsStore.data.first() requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launch diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 84580a6d73..ae71ccdfc7 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -6821,6 +6821,55 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertFalse(sut.sendUiState.value.shouldConfirmPay) } + @Test + fun `hardware confirmation keeps fees loading while recovery is checked`() = test { + val readStarted = CompletableDeferred() + val finishRead = CompletableDeferred() + var initialFeeEstimated = false + var holdRead = true + whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) }.doSuspendableAnswer { + initialFeeEstimated = true + Result.success(250uL) + } + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (initialFeeEstimated && !sut.sendUiState.value.isLoading && holdRead) { + holdRead = false + readStarted.complete(Unit) + finishRead.await() + } + false + } + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + isAmountInputValid = true, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), + ) + ) + + sut.sendEffect.test { + sut.setSendEvent(SendEvent.AmountContinue) + readStarted.await() + assertEquals(SendEffect.NavigateToConfirm, awaitItem()) + assertFalse(sut.sendUiState.value.isLoading) + assertTrue( + sut.sendUiState.value.onchainFeeUi.isLoading, + "Confirmation fee must stay loading during recovery" + ) + + finishRead.complete(Unit) + advanceUntilIdle() + + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + } + } + @Test fun `missing hardware fee does not block confirmation`() = test { settingsData.value = SettingsData(coinSelectAuto = false) From 618b09869b0a1cb6318cb37817a1e85b896cd882 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 28 Oct 2026 11:12:44 +0200 Subject: [PATCH 3/5] fix: recover refused hardware fee refreshes --- .../screens/wallets/send/SendConfirmScreen.kt | 2 +- .../java/to/bitkit/viewmodels/AppViewModel.kt | 43 ++++- .../viewmodels/AppViewModelSendFlowTest.kt | 156 ++++++++++++++++++ 3 files changed, 194 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/send/SendConfirmScreen.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/send/SendConfirmScreen.kt index 6d64183220..7d7015b19c 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/send/SendConfirmScreen.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/send/SendConfirmScreen.kt @@ -499,7 +499,7 @@ private fun ContentRunning( color = accentColor, enabled = !isPreparing && uiState.isAmountInputValid && !uiState.isFundingSourceLoading && - !isHardwareFeeLoading, + uiState.isHardwareFeeReady, loading = isPreparing || isLoading, confirmed = !isPreparing && isLoading, progress = swipeProgress, diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index adfff36b07..39478bd870 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -4016,7 +4016,7 @@ class AppViewModel @Inject constructor( fun setTransactionSpeed(speed: TransactionSpeed) { if (_sendUiState.value.isRetainedHardwarePayment) return - onchainSendRefreshJob?.cancel() + cancelOnchainSendRefresh() val previous = _sendUiState.value _sendUiState.update { it.copy( @@ -4082,7 +4082,7 @@ class AppViewModel @Inject constructor( val nextSource = nextFundingSource(current, requested) ?: return val canReviewInPlace = canRebindBitcoinRequestTerms(current, nextSource) val generation = ++sendStateGeneration - onchainSendRefreshJob?.cancel() + cancelOnchainSendRefresh() val target = if (nextSource == SendFundingSource.Usdt) PaykitAsset.USDT else PaykitAsset.BTC val converted = try { paymentAmountForSource(current, nextSource) @@ -4412,6 +4412,7 @@ class AppViewModel @Inject constructor( rate = FeeRate.fromSpeed(state.speed), sats = miningFeeSats.toLong(), isLoading = false, + requiresRefresh = false, ) } return true @@ -5738,7 +5739,7 @@ class AppViewModel @Inject constructor( private fun onSwipeToPay() { Logger.debug("Swipe to pay event, checking send confirmation conditions", context = TAG) - if (!_sendUiState.value.isAmountInputValid) return + if (!_sendUiState.value.isAmountInputValid || !_sendUiState.value.isHardwareFeeReady) return viewModelScope.launch { if (!prepareCoinSelection()) return@launch val amount = _sendUiState.value.amount @@ -6897,6 +6898,12 @@ class AppViewModel @Inject constructor( } } + private fun cancelOnchainSendRefresh() { + val job = onchainSendRefreshJob + onchainSendRefreshJob = null + job?.cancel() + } + /** Reselect utxos for current amount & speed then refresh fees using updated utxos */ private fun refreshOnchainSendIfNeeded( walletGeneration: Long? = connectionRecovery.currentGeneration(), @@ -6913,7 +6920,7 @@ class AppViewModel @Inject constructor( } updateOnchainFeeUi { it.copy(isLoading = true) } - onchainSendRefreshJob?.cancel() + cancelOnchainSendRefresh() val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launch @@ -6946,7 +6953,23 @@ class AppViewModel @Inject constructor( refreshOnchainFeeUi(walletGeneration) } onchainSendRefreshJob = job - job.invokeOnCompletion { + job.invokeOnCompletion { cause -> + if (cause != null) { + _sendUiState.update { + if (onchainSendRefreshJob !== job || generation != sendStateGeneration || + !connectionRecovery.isCurrentGeneration(walletGeneration) || it.isRetainedHardwarePayment + ) { + it + } else { + it.copy( + onchainFeeUi = OnchainFeeUi( + rate = FeeRate.fromSpeed(it.speed), + requiresRefresh = true, + ), + ) + } + } + } if (onchainSendRefreshJob === job) onchainSendRefreshJob = null } job.start() @@ -7578,7 +7601,11 @@ class AppViewModel @Inject constructor( private fun onConfirmPay() { Logger.debug("Payment checks confirmed, proceeding…", context = TAG) - if (isSubmittingPaymentRequest || isSoftwarePaymentSubmissionActive) return + if (isSubmittingPaymentRequest || isSoftwarePaymentSubmissionActive || + !_sendUiState.value.isHardwareFeeReady + ) { + return + } val contactPaymentContext = synchronized(contactPaymentContextLock) { activeContactPaymentContext } if (_sendUiState.value.isPaymentRequest && contactPaymentContext?.incomingPaymentRequest == null) { @@ -8697,6 +8724,9 @@ data class SendUiState( it > 0u && !paykitUsesUsdt && payMethod == SendMethod.LIGHTNING && lnurl == null } + val isHardwareFeeReady: Boolean + get() = hardwareWalletId == null || (!onchainFeeUi.isLoading && !onchainFeeUi.requiresRefresh) + val isAmountEditable: Boolean get() = !isPaymentRequest && fixedLightningAmount == null @@ -8711,6 +8741,7 @@ data class OnchainFeeUi( val sats: Long? = null, val estimates: ImmutableMap = persistentMapOf(), val isLoading: Boolean = false, + val requiresRefresh: Boolean = false, ) enum class SanityWarning(@StringRes val message: Int, val testTag: String) { diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index ae71ccdfc7..7cea3852ca 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -6870,6 +6870,162 @@ class AppViewModelSendFlowTest : BaseUnitTest() { } } + @Test + fun `refused hardware fee refresh ends loading and can be retried`() = test { + val refusals = listOf(Result.success(true), Result.failure(IllegalStateException("marker unreadable"))) + for (refusal in refusals) { + val readStarted = CompletableDeferred() + val finishRead = CompletableDeferred() + var recoveryClear = false + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (!recoveryClear && sut.sendUiState.value.hardwareAvailableSats == 48_000uL) { + readStarted.complete(Unit) + finishRead.await() + refusal.getOrThrow() + } else { + false + } + } + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } + .thenReturn(Result.success(48_000uL)) + whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } + .thenReturn(Result.success(250uL)) + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + isAmountInputValid = true, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), + onchainFeeUi = OnchainFeeUi(sats = 100L), + ) + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + readStarted.await() + assertTrue(sut.sendUiState.value.onchainFeeUi.isLoading) + + finishRead.complete(Unit) + advanceUntilIdle() + + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertNull(sut.sendUiState.value.onchainFeeUi.sats) + assertFalse(sut.sendUiState.value.isHardwareFeeReady) + sut.sendEffect.test { + sut.setSendEvent(SendEvent.SwipeToPay) + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + assertFalse(sut.sendUiState.value.shouldConfirmPay) + expectNoEvents() + } + + recoveryClear = true + sut.setTransactionSpeed(TransactionSpeed.Medium) + advanceUntilIdle() + + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + assertTrue(sut.sendUiState.value.isHardwareFeeReady) + } + } + + @Test + fun `superseded hardware fee refresh cannot clear newer loading`() = test { + val firstRead = CompletableDeferred() + val finishFirst = CompletableDeferred() + val secondRead = CompletableDeferred() + val finishSecond = CompletableDeferred() + var reads = 0 + var availableSats = 48_000uL + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } + .doSuspendableAnswer { Result.success(availableSats) } + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (sut.sendUiState.value.hardwareAvailableSats != availableSats) return@doSuspendableAnswer false + when (reads++) { + 0 -> withContext(NonCancellable) { + firstRead.complete(Unit) + finishFirst.await() + true + } + 1 -> { + secondRead.complete(Unit) + finishSecond.await() + false + } + else -> false + } + } + whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } + .thenReturn(Result.success(250uL)) + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), + ) + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + firstRead.await() + availableSats = 46_000uL + sut.setTransactionSpeed(TransactionSpeed.Medium) + secondRead.await() + finishFirst.complete(Unit) + advanceUntilIdle() + + assertTrue(sut.sendUiState.value.onchainFeeUi.isLoading) + finishSecond.complete(Unit) + advanceUntilIdle() + + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + } + + @Test + fun `obsolete hardware fee refresh preserves replacement send state`() = test { + for (replaceWallet in listOf(false, true)) { + val readStarted = CompletableDeferred() + val finishRead = CompletableDeferred() + var holdRead = true + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } + .thenReturn(Result.success(48_000uL)) + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (holdRead && sut.sendUiState.value.hardwareAvailableSats == 48_000uL) { + holdRead = false + readStarted.complete(Unit) + finishRead.await() + true + } else { + false + } + } + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + ) + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + readStarted.await() + if (replaceWallet) walletGeneration++ else assertTrue(sut.resetSendState()) + val replacement = SendUiState(onchainFeeUi = OnchainFeeUi(sats = 99L, isLoading = true)) + setSendState(replacement) + finishRead.complete(Unit) + advanceUntilIdle() + + assertEquals(replacement, sut.sendUiState.value) + } + } + @Test fun `missing hardware fee does not block confirmation`() = test { settingsData.value = SettingsData(coinSelectAuto = false) From 60f141f7f044d93be921961f911b7cb8e10da2a6 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 28 Oct 2026 13:16:24 +0200 Subject: [PATCH 4/5] fix: recover fee row retries after refusal --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 52 +++- .../viewmodels/AppViewModelSendFlowTest.kt | 267 ++++++++++++------ 2 files changed, 223 insertions(+), 96 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 39478bd870..a76fa7d296 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -73,6 +73,7 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update +import kotlinx.coroutines.job import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -2990,9 +2991,15 @@ class AppViewModel @Inject constructor( SendEvent.SpeedAndFee -> { if (_sendUiState.value.isRetainedHardwarePayment) return@collect if (_sendUiState.value.onchainFeeUi.estimates.isEmpty()) { - viewModelScope.launch { - refreshOnchainFeeUi() - setSendEffect(SendEffect.NavigateToFee) + val generation = sendStateGeneration + val walletGeneration = connectionRecovery.currentGeneration() + launchOnchainFeeRefresh(generation, walletGeneration) { + refreshOnchainFeeUi(walletGeneration) + val refreshJob = currentCoroutineContext().job + setSendEffect(SendEffect.NavigateToFee) { + refreshJob.isActive && generation == sendStateGeneration && + connectionRecovery.isCurrentGeneration(walletGeneration) + }.join() } } else { setSendEffect(SendEffect.NavigateToFee) @@ -4444,11 +4451,19 @@ class AppViewModel @Inject constructor( } private suspend fun onCoinSelectionContinue(utxos: List) { + val generation = sendStateGeneration + val walletGeneration = connectionRecovery.currentGeneration() _sendUiState.update { it.copy(selectedUtxos = utxos.toImmutableList()) } - refreshOnchainFeeUi() - setSendEffect(SendEffect.NavigateToConfirm) + launchOnchainFeeRefresh(generation, walletGeneration) { + refreshOnchainFeeUi(walletGeneration) + val refreshJob = currentCoroutineContext().job + setSendEffect(SendEffect.NavigateToConfirm) { + refreshJob.isActive && generation == sendStateGeneration && + connectionRecovery.isCurrentGeneration(walletGeneration) + }.join() + }?.join() } private suspend fun validateAmount( @@ -6908,10 +6923,8 @@ class AppViewModel @Inject constructor( private fun refreshOnchainSendIfNeeded( walletGeneration: Long? = connectionRecovery.currentGeneration(), ): Job? { - if (!connectionRecovery.isCurrentGeneration(walletGeneration)) return null val generation = sendStateGeneration val currentState = _sendUiState.value - if (currentState.isRetainedHardwarePayment) return null if (currentState.payMethod != SendMethod.ONCHAIN || currentState.amount == 0uL || currentState.address.isEmpty() @@ -6919,14 +6932,12 @@ class AppViewModel @Inject constructor( return null } - updateOnchainFeeUi { it.copy(isLoading = true) } - cancelOnchainSendRefresh() - val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { + return launchOnchainFeeRefresh(generation, walletGeneration) { requireCurrentPaymentPreparation(walletGeneration) - if (generation != sendStateGeneration) return@launch + if (generation != sendStateGeneration) return@launchOnchainFeeRefresh val settings = settingsStore.data.first() requireCurrentPaymentPreparation(walletGeneration) - if (generation != sendStateGeneration) return@launch + if (generation != sendStateGeneration) return@launchOnchainFeeRefresh // preselect utxos for deterministic fee estimation if ( currentState.hardwareWalletId == null && @@ -6949,9 +6960,24 @@ class AppViewModel @Inject constructor( } } requireCurrentPaymentPreparation(walletGeneration) - if (generation != sendStateGeneration) return@launch + if (generation != sendStateGeneration) return@launchOnchainFeeRefresh refreshOnchainFeeUi(walletGeneration) } + } + + private fun launchOnchainFeeRefresh( + generation: Long, + walletGeneration: Long?, + refresh: suspend () -> Unit, + ): Job? { + if (generation != sendStateGeneration || !connectionRecovery.isCurrentGeneration(walletGeneration) || + _sendUiState.value.isRetainedHardwarePayment + ) { + return null + } + updateOnchainFeeUi { it.copy(isLoading = true) } + cancelOnchainSendRefresh() + val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { refresh() } onchainSendRefreshJob = job job.invokeOnCompletion { cause -> if (cause != null) { diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 7cea3852ca..1fbf4c66cd 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -58,6 +58,7 @@ import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain import kotlinx.coroutines.withContext import kotlinx.coroutines.yield import org.junit.After @@ -6877,8 +6878,12 @@ class AppViewModelSendFlowTest : BaseUnitTest() { val readStarted = CompletableDeferred() val finishRead = CompletableDeferred() var recoveryClear = false + var refuseAfterEstimate = false + var feeEstimated = false whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (!recoveryClear && sut.sendUiState.value.hardwareAvailableSats == 48_000uL) { + if (!recoveryClear && sut.sendUiState.value.hardwareAvailableSats == 48_000uL && + (!refuseAfterEstimate || feeEstimated) + ) { readStarted.complete(Unit) finishRead.await() refusal.getOrThrow() @@ -6889,7 +6894,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } .thenReturn(Result.success(48_000uL)) whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } - .thenReturn(Result.success(250uL)) + .doSuspendableAnswer { + feeEstimated = true + Result.success(250uL) + } setSendState( SendUiState( address = REGTEST_ADDRESS, @@ -6919,91 +6927,63 @@ class AppViewModelSendFlowTest : BaseUnitTest() { advanceUntilIdle() assertFalse(sut.sendUiState.value.shouldConfirmPay) expectNoEvents() - } - recoveryClear = true - sut.setTransactionSpeed(TransactionSpeed.Medium) - advanceUntilIdle() + sut.setSendEvent(SendEvent.SpeedAndFee) + advanceUntilIdle() + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertFalse(sut.sendUiState.value.isHardwareFeeReady) + expectNoEvents() - assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) - assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) - assertTrue(sut.sendUiState.value.isHardwareFeeReady) - } - } + refuseAfterEstimate = true + sut.setSendEvent(SendEvent.SpeedAndFee) + advanceUntilIdle() + assertTrue(feeEstimated) + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertNull(sut.sendUiState.value.onchainFeeUi.sats) + assertFalse(sut.sendUiState.value.isHardwareFeeReady) + expectNoEvents() - @Test - fun `superseded hardware fee refresh cannot clear newer loading`() = test { - val firstRead = CompletableDeferred() - val finishFirst = CompletableDeferred() - val secondRead = CompletableDeferred() - val finishSecond = CompletableDeferred() - var reads = 0 - var availableSats = 48_000uL - whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } - .doSuspendableAnswer { Result.success(availableSats) } - whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (sut.sendUiState.value.hardwareAvailableSats != availableSats) return@doSuspendableAnswer false - when (reads++) { - 0 -> withContext(NonCancellable) { - firstRead.complete(Unit) - finishFirst.await() - true - } - 1 -> { - secondRead.complete(Unit) - finishSecond.await() - false - } - else -> false + recoveryClear = true + sut.setSendEvent(SendEvent.SpeedAndFee) + advanceUntilIdle() + + assertEquals(SendEffect.NavigateToFee, awaitItem()) + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + assertTrue(sut.sendUiState.value.isHardwareFeeReady) } } - whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } - .thenReturn(Result.success(250uL)) - setSendState( - SendUiState( - address = REGTEST_ADDRESS, - amount = 1_000uL, - hardwareWalletId = HARDWARE_WALLET_ID, - hardwareAvailableSats = 50_000uL, - payMethod = SendMethod.ONCHAIN, - feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), - ) - ) - - sut.setTransactionSpeed(TransactionSpeed.Fast) - firstRead.await() - availableSats = 46_000uL - sut.setTransactionSpeed(TransactionSpeed.Medium) - secondRead.await() - finishFirst.complete(Unit) - advanceUntilIdle() - - assertTrue(sut.sendUiState.value.onchainFeeUi.isLoading) - finishSecond.complete(Unit) - advanceUntilIdle() - - assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) - assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) } @Test - fun `obsolete hardware fee refresh preserves replacement send state`() = test { - for (replaceWallet in listOf(false, true)) { - val readStarted = CompletableDeferred() - val finishRead = CompletableDeferred() - var holdRead = true + fun `superseded hardware fee refresh cannot clear newer loading`() = test { + for (feeRow in listOf(false, true)) { + val firstRead = CompletableDeferred() + val finishFirst = CompletableDeferred() + val secondRead = CompletableDeferred() + val finishSecond = CompletableDeferred() + var reads = 0 + var availableSats = if (feeRow) 50_000uL else 48_000uL whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } - .thenReturn(Result.success(48_000uL)) + .doSuspendableAnswer { Result.success(availableSats) } whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (holdRead && sut.sendUiState.value.hardwareAvailableSats == 48_000uL) { - holdRead = false - readStarted.complete(Unit) - finishRead.await() - true - } else { - false + if (sut.sendUiState.value.hardwareAvailableSats != availableSats) return@doSuspendableAnswer false + when (reads++) { + 0 -> withContext(NonCancellable) { + firstRead.complete(Unit) + finishFirst.await() + true + } + 1 -> { + secondRead.complete(Unit) + finishSecond.await() + false + } + else -> false } } + whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } + .thenReturn(Result.success(250uL)) setSendState( SendUiState( address = REGTEST_ADDRESS, @@ -7011,18 +6991,139 @@ class AppViewModelSendFlowTest : BaseUnitTest() { hardwareWalletId = HARDWARE_WALLET_ID, hardwareAvailableSats = 50_000uL, payMethod = SendMethod.ONCHAIN, + feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), ) ) - sut.setTransactionSpeed(TransactionSpeed.Fast) - readStarted.await() - if (replaceWallet) walletGeneration++ else assertTrue(sut.resetSendState()) - val replacement = SendUiState(onchainFeeUi = OnchainFeeUi(sats = 99L, isLoading = true)) - setSendState(replacement) - finishRead.complete(Unit) - advanceUntilIdle() + if (feeRow) sut.setSendEvent(SendEvent.SpeedAndFee) else sut.setTransactionSpeed(TransactionSpeed.Fast) + firstRead.await() + availableSats = 46_000uL + sut.setTransactionSpeed(TransactionSpeed.Medium) + secondRead.await() + sut.sendEffect.test { + finishFirst.complete(Unit) + advanceUntilIdle() + + assertTrue(sut.sendUiState.value.onchainFeeUi.isLoading) + finishSecond.complete(Unit) + advanceUntilIdle() + + assertFalse(sut.sendUiState.value.onchainFeeUi.isLoading) + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + expectNoEvents() + } + } + } + + @Test + fun `obsolete hardware fee refresh preserves replacement send state`() = test { + for (feeRow in listOf(false, true)) { + for (replaceWallet in listOf(false, true)) { + val readStarted = CompletableDeferred() + val finishRead = CompletableDeferred() + var holdRead = true + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } + .thenReturn(Result.success(48_000uL)) + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (holdRead && sut.sendUiState.value.hardwareAvailableSats == if (feeRow) 50_000uL else 48_000uL) { + holdRead = false + readStarted.complete(Unit) + finishRead.await() + true + } else { + false + } + } + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + ) + ) - assertEquals(replacement, sut.sendUiState.value) + if (feeRow) sut.setSendEvent(SendEvent.SpeedAndFee) else sut.setTransactionSpeed(TransactionSpeed.Fast) + readStarted.await() + if (replaceWallet) walletGeneration++ else assertTrue(sut.resetSendState()) + val replacement = SendUiState(onchainFeeUi = OnchainFeeUi(sats = 99L, isLoading = true)) + setSendState(replacement) + sut.sendEffect.test { + finishRead.complete(Unit) + advanceUntilIdle() + + assertEquals(replacement, sut.sendUiState.value) + expectNoEvents() + } + } + } + } + + @Test + fun `successful fee refresh navigation respects replacement ownership`() = test { + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } + .thenReturn(Result.success(48_000uL)) + for (feeRow in listOf(true, false)) { + for (replacement in listOf("send", "wallet", "refresh", "none")) { + var estimates = 0 + var replacementQueued = false + whenever { hwWalletRepo.estimateFundingMiningFee(any(), any(), any(), any()) } + .doSuspendableAnswer { + estimates++ + Result.success(250uL) + } + whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { + if (estimates == 3 && !replacementQueued) { + replacementQueued = true + launch(Dispatchers.Main) { + assertEquals(250L, sut.sendUiState.value.onchainFeeUi.sats) + when (replacement) { + "send" -> assertTrue(sut.resetSendState()) + "wallet" -> walletGeneration++ + "refresh" -> sut.setTransactionSpeed(TransactionSpeed.Fast) + } + } + } + false + } + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = 1_000uL, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareAvailableSats = 50_000uL, + payMethod = SendMethod.ONCHAIN, + feeRates = FeeRates(fast = 5u, mid = 3u, slow = 1u), + ) + ) + + Dispatchers.setMain(StandardTestDispatcher(testScheduler)) + try { + sut.sendEffect.test { + val event = if (feeRow) { + SendEvent.SpeedAndFee + } else { + SendEvent.CoinSelectionContinue(listOf(mock())) + } + sut.setSendEvent(event) + advanceUntilIdle() + + assertTrue(replacementQueued) + if (replacement == "none") { + assertEquals( + if (feeRow) SendEffect.NavigateToFee else SendEffect.NavigateToConfirm, + awaitItem() + ) + } else if (replacement == "refresh") { + assertEquals(SendEffect.PopBack(SendRoute.Confirm), awaitItem()) + } + expectNoEvents() + } + } finally { + Dispatchers.setMain(testDispatcher) + } + } } } From 2643b493657c37ab52d42a4fe0cae2edbed29559 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 29 Oct 2026 01:25:05 +0200 Subject: [PATCH 5/5] fix: own preliminary hardware fee preparation --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 25 ++++++++++++++----- .../viewmodels/AppViewModelSendFlowTest.kt | 16 +++++++----- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index c3c165b4dc..976432e3a2 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -4123,7 +4123,9 @@ class AppViewModel @Inject constructor( if (!reviewPaykitMethodChange(previous)) { setSendEffect(SendEffect.PopBack(SendRoute.Confirm)) } - viewModelScope.launch { + val generation = sendStateGeneration + val walletGeneration = connectionRecovery.currentGeneration() + refreshOnchainSendIfNeeded(walletGeneration) { val shouldResetUtxos = when (settingsStore.data.first().coinSelectAuto) { true -> { val currentSatsPerVByte = previous.feeRates?.getSatsPerVByteFor(previous.speed) @@ -4137,8 +4139,10 @@ class AppViewModel @Inject constructor( previous.amount > 0uL && previous.amount == previous.hardwareAvailableSats val hardwareAvailableSats = previous.hardwareWalletId?.let { walletId -> - hardwareMaxSpendable(walletId, previous.address, speed) + hardwareMaxSpendable(walletId, previous.address, speed, walletGeneration) } ?: previous.hardwareAvailableSats + requireCurrentPaymentPreparation(walletGeneration) + if (generation != sendStateGeneration) return@refreshOnchainSendIfNeeded _sendUiState.update { it.copy( amount = if (wasHardwareMax) hardwareAvailableSats else it.amount, @@ -4147,7 +4151,6 @@ class AppViewModel @Inject constructor( ) } updateCanSwitchWallet() - refreshOnchainSendIfNeeded() ?: updateOnchainFeeUi { it.copy(isLoading = false) } } } @@ -7015,19 +7018,29 @@ class AppViewModel @Inject constructor( /** Reselect utxos for current amount & speed then refresh fees using updated utxos */ private fun refreshOnchainSendIfNeeded( walletGeneration: Long? = connectionRecovery.currentGeneration(), + prepare: (suspend () -> Unit)? = null, ): Job? { val generation = sendStateGeneration val currentState = _sendUiState.value - if (currentState.payMethod != SendMethod.ONCHAIN || - currentState.amount == 0uL || - currentState.address.isEmpty() + if (prepare == null && ( + currentState.payMethod != SendMethod.ONCHAIN || + currentState.amount == 0uL || currentState.address.isEmpty() + ) ) { return null } return launchOnchainFeeRefresh(generation, walletGeneration) { + prepare?.invoke() requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launchOnchainFeeRefresh + val currentState = _sendUiState.value + if (currentState.payMethod != SendMethod.ONCHAIN || + currentState.amount == 0uL || currentState.address.isEmpty() + ) { + updateOnchainFeeUi { it.copy(isLoading = false) } + return@launchOnchainFeeRefresh + } val settings = settingsStore.data.first() requireCurrentPaymentPreparation(walletGeneration) if (generation != sendStateGeneration) return@launchOnchainFeeRefresh diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 47e98366c9..8a6279be03 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -6897,14 +6897,15 @@ class AppViewModelSendFlowTest : BaseUnitTest() { @Test fun `refused hardware fee refresh ends loading and can be retried`() = test { val refusals = listOf(Result.success(true), Result.failure(IllegalStateException("marker unreadable"))) - for (refusal in refusals) { + for ((refusal, availableAtRefusal) in refusals.flatMap { listOf(it to 50_000uL, it to 48_000uL) }) { val readStarted = CompletableDeferred() val finishRead = CompletableDeferred() var recoveryClear = false var refuseAfterEstimate = false var feeEstimated = false whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (!recoveryClear && sut.sendUiState.value.hardwareAvailableSats == 48_000uL && + if (!recoveryClear && + (sut.sendUiState.value.hardwareAvailableSats == availableAtRefusal || refuseAfterEstimate) && (!refuseAfterEstimate || feeEstimated) ) { readStarted.complete(Unit) @@ -6980,7 +6981,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { @Test fun `superseded hardware fee refresh cannot clear newer loading`() = test { - for (feeRow in listOf(false, true)) { + for ((feeRow, preliminary) in listOf(false to false, true to false, false to true)) { val firstRead = CompletableDeferred() val finishFirst = CompletableDeferred() val secondRead = CompletableDeferred() @@ -6990,7 +6991,9 @@ class AppViewModelSendFlowTest : BaseUnitTest() { whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } .doSuspendableAnswer { Result.success(availableSats) } whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (sut.sendUiState.value.hardwareAvailableSats != availableSats) return@doSuspendableAnswer false + if (!preliminary && sut.sendUiState.value.hardwareAvailableSats != availableSats) { + return@doSuspendableAnswer false + } when (reads++) { 0 -> withContext(NonCancellable) { firstRead.complete(Unit) @@ -7040,7 +7043,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { @Test fun `obsolete hardware fee refresh preserves replacement send state`() = test { - for (feeRow in listOf(false, true)) { + for ((feeRow, preliminary) in listOf(false to false, true to false, false to true)) { for (replaceWallet in listOf(false, true)) { val readStarted = CompletableDeferred() val finishRead = CompletableDeferred() @@ -7048,7 +7051,8 @@ class AppViewModelSendFlowTest : BaseUnitTest() { whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) } .thenReturn(Result.success(48_000uL)) whenever { backupRepo.hasPendingWalletRestore() }.doSuspendableAnswer { - if (holdRead && sut.sendUiState.value.hardwareAvailableSats == if (feeRow) 50_000uL else 48_000uL) { + val availableAtRead = if (feeRow || preliminary) 50_000uL else 48_000uL + if (holdRead && sut.sendUiState.value.hardwareAvailableSats == availableAtRead) { holdRead = false readStarted.complete(Unit) finishRead.await()