From 8564ba2a1a96e4d1ac56c4b084143f619968d48a Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 20 Oct 2026 19:52:53 -0600 Subject: [PATCH] fix: keep incoming request refreshes together --- .../repositories/PaykitPaymentRequestRepo.kt | 14 ++ .../to/bitkit/services/PaykitSdkService.kt | 100 +++++++++---- .../java/to/bitkit/viewmodels/AppViewModel.kt | 12 +- ...aykitPaymentRequestRepoSubscriptionTest.kt | 7 + .../PaykitPaymentRequestRepoTest.kt | 87 +++++++++++ .../bitkit/services/PaykitSdkServiceTest.kt | 139 ++++++++++++++++++ .../viewmodels/AppViewModelSendFlowTest.kt | 65 +++++++- changelog.d/next/1645.fixed.md | 1 + journeys/payment-requests/README.md | 13 ++ .../automatic-presentation.xml | 2 + journeys/payment-requests/review-refresh.xml | 4 +- 11 files changed, 410 insertions(+), 34 deletions(-) create mode 100644 changelog.d/next/1645.fixed.md diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index b6dc749756..415f98d332 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -602,6 +602,7 @@ class PaykitPaymentRequestRepo @Inject constructor( priority, if (forceFresh) priority else messagePriority, observationOnly = true, + combineReceiveObservation = !forceFresh, ) } operationMutex.withLock { @@ -1245,8 +1246,21 @@ class PaykitPaymentRequestRepo @Inject constructor( priority: Priority = Priority.Ordered, messagePriority: Priority = Priority.Ordered, observationOnly: Boolean = false, + combineReceiveObservation: Boolean = false, ): RequestSnapshot { if (mode == PaykitPaymentRequestRefreshMode.FULL) processPendingMessages(priority = messagePriority) + if (combineReceiveObservation && observationOnly && mode != PaykitPaymentRequestRefreshMode.STORED && + priority == Priority.Background && messagePriority == Priority.Background + ) { + val observation = paykitSdkService.receiveAndObserveState( + requireNotNull(expectedIdentity), + ::logIntakeFailures, + ) + return RequestSnapshot( + observation.paymentRequests, + observation.linkedPeers.filter { it.state == LinkedPeerState.BLOCKED }, + ) + } if (mode != PaykitPaymentRequestRefreshMode.STORED) { paykitSdkService.receivePrivateMessagesFromLinkedPeers(messagePriority).also(::logIntakeFailures) } diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index de0f10b33c..99ebe62b72 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -967,6 +967,50 @@ class PaykitSdkService @Inject constructor( } } + internal suspend fun receiveAndObserveState( + expectedIdentity: String, + onReceive: (List) -> Unit, + ): PaykitStateObservation { + isSetup.await() + val generation = runtimeGeneration + return operationLock.withLock(Priority.Background) { + check(runtimeGeneration == generation) { "Paykit runtime changed before inbox refresh" } + checkRefreshAdmission(Priority.Background) + withPaykitKey { handle -> + val key = cachedPaykitKey + check(key == null || PubkyPublicKeyFormat.matches(key.publicKey, expectedIdentity)) { + "Paykit inbox identity changed" + } + val reports = withStateRevisionTracking( + handle, + beforeOperation = { + operationLock.withoutLock { + check(runtimeGeneration == generation && sdk === handle && cachedPaykitKey == key) { + "Paykit runtime changed before inbox receive" + } + checkRefreshAdmission(Priority.Background) + } + }, + ) { completeSdkCall { handle.receivePrivateMessagesFromLinkedPeers() } } + onReceive(reports) + operationLock.withoutLock { + check(runtimeGeneration == generation && sdk === handle && cachedPaykitKey == key) { + "Paykit runtime changed during inbox refresh" + } + checkRefreshAdmission(Priority.Background) + val observation = completeSdkCall { handle.observeState() } + check(PubkyPublicKeyFormat.matches(observation.publicKey, expectedIdentity)) { + "Paykit observation identity changed" + } + key?.let { + check(observation.keyGeneration == it.generation) { "Paykit observation key changed" } + } + observation + } + } + } + } + suspend fun receivePrivateMessages(counterparty: String) = receivePrivateMessages(counterparty, Priority.Ordered) internal suspend fun receivePrivateMessages(counterparty: String, priority: Priority) = run { @@ -1601,32 +1645,38 @@ class PaykitSdkService @Inject constructor( beforeOperation: suspend () -> Unit = {}, block: suspend (PaykitSdk) -> T, ): T = withPaykitKey { handle -> - withPaykitBackupStateTracking( - readRevision = { - val started = TimeSource.Monotonic.markNow() - try { - runSuspendCatching { completeSdkCall { handle.backupStateRevision() } } - .onFailure(::invalidatePaykitKeyIfNeeded) - .getOrThrow() - } finally { - if (BuildConfig.DEBUG) { - Logger.debug("Read Paykit backup revision duration='${started.elapsedNow()}'", context = TAG) - } - } - }, - readStateRevision = { handle.stateRevision() }, - readObservedSnapshot = { - handle.observedBackupStateRevision()?.let { - PaykitBackupStateSnapshot(it.stateRevision, it.backupRevision) + withStateRevisionTracking(handle, beforeOperation) { block(handle) } + } + + private suspend fun withStateRevisionTracking( + handle: PaykitSdk, + beforeOperation: suspend () -> Unit, + block: suspend () -> T, + ): T = withPaykitBackupStateTracking( + readRevision = { + val started = TimeSource.Monotonic.markNow() + try { + runSuspendCatching { completeSdkCall { handle.backupStateRevision() } } + .onFailure(::invalidatePaykitKeyIfNeeded) + .getOrThrow() + } finally { + if (BuildConfig.DEBUG) { + Logger.debug("Read Paykit backup revision duration='${started.elapsedNow()}'", context = TAG) } - }, - cachedSnapshot = cachedBackupState, - onSnapshot = { cachedBackupState = it }, - onChange = ::notifyBackupStateChanged, - beforeOperation = beforeOperation, - ) { - block(handle) - } + } + }, + readStateRevision = { handle.stateRevision() }, + readObservedSnapshot = { + handle.observedBackupStateRevision()?.let { + PaykitBackupStateSnapshot(it.stateRevision, it.backupRevision) + } + }, + cachedSnapshot = cachedBackupState, + onSnapshot = { cachedBackupState = it }, + onChange = ::notifyBackupStateChanged, + beforeOperation = beforeOperation, + ) { + block() } private suspend fun handle(): PaykitSdk = handleMutex.withLock { diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 8c3e702d35..9655350fd6 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -951,7 +951,11 @@ class AppViewModel @Inject constructor( refreshPrivateOnlyPaykitApp("contact sync", onlyIfNeeded = !identityChanged) if (!state.contactsLoaded || !synchronizeSavedPaykitContacts(state.contactKeys, isStillCurrent)) return if (isPaymentRequestPollingStopped.value) return - val result = refreshIncomingPaykitPaymentRequests(forceFresh = true, onlyWhileActive = true) + val result = refreshIncomingPaykitPaymentRequests( + forceFresh = true, + refreshPriority = Priority.Background, + onlyWhileActive = true, + ) if (!isPaymentRequestPollingStopped.value) refreshPaymentRequestTargets(force = true) result } finally { @@ -1032,7 +1036,11 @@ class AppViewModel @Inject constructor( isStillCurrent = isStillCurrent, ) if (isPaymentRequestPollingStopped.value) return - refreshIncomingPaykitPaymentRequests(forceFresh = true, onlyWhileActive = true) + refreshIncomingPaykitPaymentRequests( + forceFresh = true, + refreshPriority = Priority.Background, + onlyWhileActive = true, + ) if (!isPaymentRequestPollingStopped.value) refreshPaymentRequestTargets(force = true) } diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt index ae94869244..0ba6138ace 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt @@ -21,6 +21,7 @@ import com.synonym.paykit.PaymentRequestRecord import com.synonym.paykit.PaymentRequestRecurrence import com.synonym.paykit.PaymentRequestTerms import com.synonym.paykit.PrivateJsonObject +import com.synonym.paykit.PrivateStreamCounterpartyIntakeReport import com.synonym.paykit.PubkyIdentityCapability import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -53,6 +54,7 @@ import to.bitkit.models.PaykitAmount import to.bitkit.models.PaykitAsset import to.bitkit.models.PaykitRequestPricing import to.bitkit.services.PaykitPaymentRequestProposalTerms +import to.bitkit.services.PaykitSdkOperationLock.Priority import to.bitkit.services.PaykitSdkService import to.bitkit.test.BaseUnitTest import kotlin.test.assertEquals @@ -116,6 +118,11 @@ class PaykitPaymentRequestRepoSubscriptionTest : BaseUnitTest(StandardTestDispat whenever(paykitSdkService.receivePrivateMessagesFromLinkedPeers(any())).doSuspendableAnswer { paykitSdkService.receivePrivateMessagesFromLinkedPeers() } + whenever(paykitSdkService.receiveAndObserveState(any(), any())).doSuspendableAnswer { + val reports = paykitSdkService.receivePrivateMessagesFromLinkedPeers(Priority.Background) + it.getArgument<(List) -> Unit>(1)(reports) + paykitSdkService.observeState(it.getArgument(0), Priority.Background) + } stubRequestReads() whenever(paykitSdkService.processOutboundPrivateMessages(any(), any())).doSuspendableAnswer { paykitSdkService.processOutboundPrivateMessages(it.getArgument(0)) diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 50914dd245..4ebe74447c 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -312,6 +312,11 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { whenever(paykitSdkService.receivePrivateMessagesFromLinkedPeers(any())).doSuspendableAnswer { paykitSdkService.receivePrivateMessagesFromLinkedPeers() } + whenever(paykitSdkService.receiveAndObserveState(any(), any())).doSuspendableAnswer { + val reports = paykitSdkService.receivePrivateMessagesFromLinkedPeers(Priority.Background) + it.getArgument<(List) -> Unit>(1)(reports) + paykitSdkService.observeState(it.getArgument(0), Priority.Background) + } stubRequestReads() whenever(paykitSdkService.processOutboundPrivateMessages(any(), any())).doSuspendableAnswer { paykitSdkService.processOutboundPrivateMessages(it.getArgument(0)) @@ -493,6 +498,58 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { verify(paykitSdkService, never()).linkedPeers(any()) verify(paykitSdkService).processPendingPrivateMessages(Priority.Background) verify(paykitSdkService, times(2)).receivePrivateMessagesFromLinkedPeers(Priority.Background) + verify(paykitSdkService, times(2)).receiveAndObserveState(eq(LOCAL_IDENTITY), any()) + } + + @Test + fun `forced and non-background refresh retain separate receive and observation`() = test { + for (mode in listOf(PaykitPaymentRequestRefreshMode.INBOX, PaykitPaymentRequestRefreshMode.FULL)) { + for (priority in listOf(Priority.Background, Priority.Ordered, Priority.Interactive)) { + clearInvocations(paykitSdkService) + if (priority == Priority.Background) { + sut.refreshAfterStateChange(mode, priority).getOrThrow() + } else { + sut.refresh(mode, priority).getOrThrow() + } + + verify(paykitSdkService).receivePrivateMessagesFromLinkedPeers(priority) + verify(paykitSdkService).observeState(LOCAL_IDENTITY, Priority.Background) + verify(paykitSdkService, never()).receiveAndObserveState(any(), any()) + } + } + } + + @Test + fun `automatic full refresh finishes pending drain before receive observation and stops on deferral`() = test { + for (deferred in listOf(false, true)) { + clearInvocations(paykitSdkService) + val release = CompletableDeferred() + doSuspendableAnswer { + release.await() + if (deferred) throw PaykitPaymentRequestError.OperationInProgress + emptyList() + }.whenever(paykitSdkService).processPendingPrivateMessages(Priority.Background) + val refresh = async { sut.refresh() } + runCurrent() + verify(paykitSdkService).processPendingPrivateMessages(Priority.Background) + verify(paykitSdkService, never()).receiveAndObserveState(any(), any()) + verify(paykitSdkService, never()).receivePrivateMessagesFromLinkedPeers(any()) + verify(paykitSdkService, never()).observeState(any(), any()) + release.complete(Unit) + val result = refresh.await() + + if (deferred) { + assertSame(PaykitPaymentRequestError.OperationInProgress, result.exceptionOrNull()) + verify(paykitSdkService, never()).receiveAndObserveState(any(), any()) + verify(paykitSdkService, never()).receivePrivateMessagesFromLinkedPeers(any()) + verify(paykitSdkService, never()).observeState(any(), any()) + } else { + result.getOrThrow() + verify(paykitSdkService).receiveAndObserveState(eq(LOCAL_IDENTITY), any()) + verify(paykitSdkService).receivePrivateMessagesFromLinkedPeers(Priority.Background) + verify(paykitSdkService).observeState(LOCAL_IDENTITY, Priority.Background) + } + } } @Test @@ -1118,6 +1175,36 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { } } + @Test + fun `inbox reports intake failures once before observation fails and keeps warm requests`() = test { + for (failure in listOf( + PaykitPaymentRequestError.OperationInProgress, + PaykitException.SharedStateBusy("shared_state_busy", "Locked"), + )) { + stubRequestReads() + val record = paymentRequestRecord() + whenever(paykitSdkService.allPaymentRequests(anyOrNull())).thenReturn(listOf(record)) + sut.refresh(PaykitPaymentRequestRefreshMode.STORED).getOrThrow() + val pending = sut.pendingRequests.value + val intakeError = mock { + on { redactedContext() } doReturn "transport failure" + } + whenever(paykitSdkService.receivePrivateMessagesFromLinkedPeers()).thenReturn( + listOf(PrivateStreamCounterpartyIntakeReport(COUNTERPARTY, null, intakeError)), + ) + whenever(paykitSdkService.observeState(any(), any())).doSuspendableAnswer { throw failure } + clearInvocations(paykitSdkService) + + val result = sut.refresh(PaykitPaymentRequestRefreshMode.INBOX) + + assertSame(failure, result.exceptionOrNull()) + assertEquals(pending, sut.pendingRequests.value) + verify(intakeError).redactedContext() + verify(paykitSdkService).receiveAndObserveState(eq(LOCAL_IDENTITY), any()) + verify(paykitSdkService).receivePrivateMessagesFromLinkedPeers() + } + } + @Test fun `peer intake failure does not drop received requests`() = test { val record = paymentRequestRecord() diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index 11cc4b4326..22870cbf72 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -497,6 +497,145 @@ class PaykitSdkServiceTest { } } + @Test + fun `inbox observation precedes queued maintenance and accounts only for receive`() = runTest { + for (changed in listOf(false, true)) { + for (observeFails in listOf(false, true)) { + val sdk = mock() + val release = CompletableDeferred() + val calls = mutableListOf() + var revision = "before" + whenever(sdk.backupStateRevision()).thenAnswer { revision } + whenever(sdk.receivePrivateMessagesFromLinkedPeers()).doSuspendableAnswer { + calls += "receive" + release.await() + if (changed) revision = "after" + emptyList() + } + whenever(sdk.observeState()).thenAnswer { + calls += "observe" + if (observeFails) throw PaykitException.SharedStateBusy("shared_state_busy", "Locked") + PaykitStateObservation(RING_PUBKY, 1uL, "now", emptyList(), emptyList(), emptyList(), emptyList()) + } + whenever(sdk.processPendingPrivateMessages()).thenAnswer { + calls += "maintenance" + emptyList() + } + val service = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { sdk } + val refresh = async { + runSuspendCatching { + service.receiveAndObserveState(RING_PUBKY) { + calls += "received" + assertEquals(if (changed) 1L else 0L, service.backupStateVersion.value) + } + } + } + runCurrent() + val maintenance = async { service.processPendingPrivateMessages(Priority.Background) } + runCurrent() + assertEquals(listOf("receive"), calls) + release.complete(Unit) + val result = refresh.await() + maintenance.await() + + assertEquals(observeFails, result.isFailure) + assertEquals(listOf("receive", "received", "observe", "maintenance"), calls) + assertEquals(if (changed) 1L else 0L, service.backupStateVersion.value) + verify(sdk).receivePrivateMessagesFromLinkedPeers() + verify(sdk).observeState() + } + } + } + + @Test + fun `inbox observation yields after receive to payment review cancellation and wipe`() = runTest { + for (boundary in listOf("payment", "review", "cancellation", "wipe")) { + val sdk = mock() + val release = CompletableDeferred() + var revision = "before" + var reports = 0 + whenever(sdk.backupStateRevision()).thenAnswer { revision } + whenever(sdk.receivePrivateMessagesFromLinkedPeers()).doSuspendableAnswer { + release.await() + revision = "after" + emptyList() + } + whenever(sdk.contactRecords()).thenReturn(emptyList()) + val service = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { sdk } + val refresh = async { runSuspendCatching { service.receiveAndObserveState(RING_PUBKY) { reports++ } } } + runCurrent() + val endReview = if (boundary == "review") service.beginPaymentReview() else null + if (boundary == "payment") service.setPaymentSubmissionActive(true) + if (boundary == "cancellation") refresh.cancel() + val wipe = if (boundary == "wipe") async { service.withWalletWipe {} } else null + val authority = if (wipe == null) async { service.contactRecords() } else null + runCurrent() + assertEquals(0L, service.backupStateVersion.value) + verify(sdk, never()).contactRecords() + release.complete(Unit) + + if (boundary == "cancellation") { + assertFailsWith { refresh.await() } + } else { + val failure = refresh.await().exceptionOrNull() + if (boundary == "wipe") { + assertIs(failure) + } else { + assertSame(PaykitPaymentRequestError.OperationInProgress, failure) + } + } + wipe?.await() + authority?.await() + assertEquals(if (boundary == "cancellation") 0 else 1, reports) + assertEquals(1L, service.backupStateVersion.value) + verify(sdk).receivePrivateMessagesFromLinkedPeers() + verify(sdk, never()).observeState() + endReview?.invoke() + service.setPaymentSubmissionActive(false) + } + } + + @Test + fun `inbox receive failure dirties backup once and never observes or replays`() = runTest { + val sdk = mock() + whenever(sdk.backupStateRevision()).thenReturn("before") + val failure = PaykitException.SharedStateBusy("shared_state_busy", "Locked") + whenever(sdk.receivePrivateMessagesFromLinkedPeers()).thenThrow(failure) + val service = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { sdk } + var reports = 0 + + val result = runSuspendCatching { service.receiveAndObserveState(RING_PUBKY) { reports++ } } + + assertSame(failure, result.exceptionOrNull()) + assertEquals(0, reports) + assertEquals(1L, service.backupStateVersion.value) + verify(sdk).receivePrivateMessagesFromLinkedPeers() + verify(sdk, never()).observeState() + } + + @Test + fun `queued inbox refresh rejects a replaced runtime before receiving`() = runTest { + val sdk = mock() + val release = CompletableDeferred() + whenever(sdk.contactRecords()).doSuspendableAnswer { + release.await() + emptyList() + } + val service = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { sdk } + val holder = async { service.contactRecords() } + runCurrent() + val reset = async { service.clearState() } + val refresh = async { runSuspendCatching { service.receiveAndObserveState(RING_PUBKY) {} } } + runCurrent() + release.complete(Unit) + holder.await() + reset.await() + + assertIs(refresh.await().exceptionOrNull()) + verify(sdk, never()).receivePrivateMessagesFromLinkedPeers() + verify(sdk, never()).observeState() + } + @Test fun `queued state observation is rejected after runtime replacement`() = runTest { val sdk = mock() diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 52177e81f7..adccdba6e2 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -2310,8 +2310,9 @@ class AppViewModelSendFlowTest : BaseUnitTest() { val otherRequest = targetRequest.copy(paymentRequestId = "other-subscription") surfacedPaykitPaymentRequestIds += targetRequest.id surfacedPaykitPaymentRequestIds += otherRequest.id - whenever(paykitPaymentRequestRepo.refreshAfterStateChange(PaykitPaymentRequestRefreshMode.FULL)) - .thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) + whenever( + paykitPaymentRequestRepo.refreshAfterStateChange(PaykitPaymentRequestRefreshMode.FULL, Priority.Background), + ).thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) whenever(privatePaykitRepo.beginPaymentRequest(targetRequest)).thenReturn( Result.success(PublicPaykitPaymentResult.WaitingForUpdatedPaymentList) ) @@ -2324,7 +2325,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { pubkyPublicKey.value = testPublicKey runCurrent() - verify(paykitPaymentRequestRepo).refreshAfterStateChange(PaykitPaymentRequestRefreshMode.FULL) + verify(paykitPaymentRequestRepo).refreshAfterStateChange( + PaykitPaymentRequestRefreshMode.FULL, + Priority.Background, + ) assertEquals(targetRequest.id, sut.requestedPaymentRequestId.value) assertNull(sut.currentSheet.value) verify(privatePaykitRepo, never()).beginPaymentRequest(any()) @@ -13398,11 +13402,52 @@ class AppViewModelSendFlowTest : BaseUnitTest() { verify(privatePaykitRepo).pruneUnsavedContactState(eq(emptySet()), any()) inOrder(privatePaykitRepo, paykitPaymentRequestRepo).apply { verify(privatePaykitRepo).removeSavedContacts(eq(setOf(contact.publicKey)), any()) - verify(paykitPaymentRequestRepo).refreshAfterStateChange() + verify(paykitPaymentRequestRepo).refreshAfterStateChange( + PaykitPaymentRequestRefreshMode.FULL, + Priority.Background, + ) } verify(paykitPaymentRequestRepo, never()).refresh(any()) } + @Test + fun `automatic contact refresh defers without losing the next poll`() = test { + enablePaykitUi() + pubkyPublicKey.update { testPublicKey } + runCurrent() + whenever(paykitPaymentRequestRepo.refresh(any())).thenReturn(Result.success(Unit)) + whenever( + paykitPaymentRequestRepo.refreshAfterStateChange(PaykitPaymentRequestRefreshMode.FULL, Priority.Background), + ).thenReturn(Result.failure(PaykitPaymentRequestError.OperationInProgress)) + sut.startPaykitPaymentRequestPolling() + try { + runCurrent() + for (trigger in listOf("contacts", "foreground")) { + clearInvocations(paykitPaymentRequestRepo, activityRepo) + when (trigger) { + "contacts" -> pubkyContactsLoadVersion.update { it + 1L } + else -> sut.refreshPrivatePaykitEndpoints() + } + runCurrent() + + verify(paykitPaymentRequestRepo).refreshAfterStateChange( + PaykitPaymentRequestRefreshMode.FULL, + Priority.Background, + ) + verify(paykitPaymentRequestRepo, never()).refreshAfterStateChange(any()) + verify(paykitPaymentRequestRepo).refreshEligibleTargets(any(), eq(true)) + verify(activityRepo, never()).backfillPaykitContacts() + assertEquals(testPublicKey, pubkyPublicKey.value) + + verifyPaymentRequestPollingInterval() + verify(paykitPaymentRequestRepo).refresh(PaykitPaymentRequestRefreshMode.INBOX, Priority.Background) + verify(activityRepo).backfillPaykitContacts() + } + } finally { + sut.stopPaykitPaymentRequestPolling() + } + } + @Test fun `automatic request refresh waits for resume when contact work finishes in background`() = test { enablePaykitUi() @@ -13429,6 +13474,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { contactWork.complete(Unit) runCurrent() verify(paykitPaymentRequestRepo, never()).refreshAfterStateChange(any()) + verify(paykitPaymentRequestRepo, never()).refreshAfterStateChange(any(), any()) verify(paykitPaymentRequestRepo, never()).refreshEligibleTargets(any(), any()) sut.startPaykitPaymentRequestPolling() @@ -13511,13 +13557,17 @@ class AppViewModelSendFlowTest : BaseUnitTest() { verify(privatePaykitRepo, never()).scheduleSavedContactPreparation(any>(), any()) verify(paykitPaymentRequestRepo, never()).refresh(any(), any()) verify(paykitPaymentRequestRepo, never()).refreshAfterStateChange(any()) + verify(paykitPaymentRequestRepo, never()).refreshAfterStateChange(any(), any()) verify(pubkyRepo, never()).refreshContacts() verify(pubkyRepo, never()).importShopContacts() isProfileDeletionInProgress.update { false } runCurrent() verify(privatePaykitRepo).scheduleSavedContactPreparation(eq(emptySet()), any()) - verify(paykitPaymentRequestRepo).refreshAfterStateChange() + verify(paykitPaymentRequestRepo).refreshAfterStateChange( + PaykitPaymentRequestRefreshMode.FULL, + Priority.Background, + ) verify(pubkyRepo).refreshContacts() sut.stopPaykitPaymentRequestPolling() } @@ -13591,7 +13641,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { inOrder(privatePaykitRepo, paykitPaymentRequestRepo).apply { verify(privatePaykitRepo).retryPendingEndpointRemoval(any()) - verify(paykitPaymentRequestRepo).refreshAfterStateChange() + verify(paykitPaymentRequestRepo).refreshAfterStateChange( + PaykitPaymentRequestRefreshMode.FULL, + Priority.Background, + ) } verify(paykitPaymentRequestRepo, never()).refresh(any()) } diff --git a/changelog.d/next/1645.fixed.md b/changelog.d/next/1645.fixed.md new file mode 100644 index 0000000000..c0ad1ba6de --- /dev/null +++ b/changelog.d/next/1645.fixed.md @@ -0,0 +1 @@ +Improve how incoming payment requests refresh without interrupting payment review. diff --git a/journeys/payment-requests/README.md b/journeys/payment-requests/README.md index 0916655715..46c72690d7 100644 --- a/journeys/payment-requests/README.md +++ b/journeys/payment-requests/README.md @@ -192,3 +192,16 @@ the boundary at 546 sats, manual retry, and preparation of a new request for 547 Closing during preparation leaves the request pending and suppresses automatic reopening for the current identity's app session; Pay from the request list or details explicitly retries it. An unfunded wallet can verify loading followed by native rejection, not an enabled payment control. + +## Automatic refresh during review + +`automatic-presentation.xml` starts after a foreground resume and checks that each received request +appears once. `review-refresh.xml` sends a second request while the first confirmation is open, +then checks that the original review remains usable and the second request arrives after cancellation. +Allow ordinary foreground polling and synchronization to finish; these steps set no arrival deadline +and do not require an immediate refresh when review closes. + +The exact overlap of a saved-contact callback, message receipt, and a queued SDK operation requires +controlled callback timing outside the journey capabilities. Repository tests cover that ordering, +automatic forced-refresh deferral, and later polling. These journeys check the visible request and +payment safeguards without claiming that a specific callback race occurred. diff --git a/journeys/payment-requests/automatic-presentation.xml b/journeys/payment-requests/automatic-presentation.xml index cc7ea79edc..7fb8850237 100644 --- a/journeys/payment-requests/automatic-presentation.xml +++ b/journeys/payment-requests/automatic-presentation.xml @@ -4,6 +4,7 @@ On the payer, leave Bitkit unlocked and in the foreground on Home + Background and reopen the payer app, complete app unlock if needed, and leave Home visible while automatic contact and Payment Request refreshes resume On the requester, send the payer contact a Payment Request for 21,000 sats with the note "First request" While the payer prepares the request, verify the existing Payment Request confirmation sheet (id "PaymentRequestConfirm") shows the requester's contact name, 21,000 sats and "First request", with the payment swipe control (id "GRAB") disabled and showing progress Close the confirmation while preparation is still pending @@ -20,6 +21,7 @@ Close Receive Verify Payment Request confirmation (id "PaymentRequestConfirm") appears automatically with 5,000 sats and For shows "Second request" Close the confirmation without paying + Open Payment Requests and verify "First request" and "Second request" each appear once with their original amounts and remain unpaid, then close the list Open Receive and close it again Verify Home remains visible and neither reviewed request reopens automatically On the payer, open the Home menu (id "HeaderMenu") before the next request arrives diff --git a/journeys/payment-requests/review-refresh.xml b/journeys/payment-requests/review-refresh.xml index b1333e95e7..e06f3cfe6b 100644 --- a/journeys/payment-requests/review-refresh.xml +++ b/journeys/payment-requests/review-refresh.xml @@ -7,10 +7,12 @@ On the payer, verify the Payment Request confirmation screen (id "PaymentRequestConfirm") shows 1,000 sats and Savings is selected Leave the confirmation open for 30 seconds Verify the amount remains 1,000 sats and payment authorization has not started + On the requester, send the payer a second on-chain-only Bitcoin Payment Request for 2,000 sats + Verify the first confirmation still shows 1,000 sats and is not replaced by the second request Swipe to pay using the control with id "GRAB", complete any sending-warning confirmation, and wait on PIN entry for 30 seconds without authorizing Cancel PIN entry, then close the send sheet without paying Verify Activity contains no outgoing payment for this request - On the requester, send the payer a second on-chain-only Bitcoin Payment Request for 2,000 sats + Leave the payer unlocked and in the foreground while ordinary Payment Request synchronization resumes; do not require the next request to arrive immediately when review closes On the payer, verify the second request becomes visible without restarting the app Close the second request without paying, then reopen the first request from the pending-request list Verify the confirmation still shows the original 1,000 sats