From e552783b262a27f6e9f90905627d131cad408934 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 21 Oct 2026 10:05:49 -0600 Subject: [PATCH] perf: keep request creation ahead of refreshes --- .../repositories/PaykitPaymentRequestRepo.kt | 3 + .../PaykitPaymentRequestRepoTest.kt | 134 ++++++++++++++++++ 2 files changed, 137 insertions(+) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index 415f98d332..b8750b79d2 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -738,6 +738,7 @@ class PaykitPaymentRequestRepo @Inject constructor( ): Result = withContext(ioDispatcher) { runSuspendCatching { if (!creationMutex.tryLock()) throw PaykitPaymentRequestError.OperationInProgress + var endReview: (() -> Unit)? = null _isCreatingRequest.update { true } try { operationMutex.withLock { @@ -747,6 +748,7 @@ class PaykitPaymentRequestRepo @Inject constructor( if (draft.amount.atomic == 0uL || !isAvailable()) throw PaykitPaymentRequestError.RequestUnavailable if (draft.expiresAt <= proposalDate) throw PaykitPaymentRequestError.RequestExpired + endReview = paykitSdkService.beginPaymentReview() val selectedSavedKey = savedPublicKeys.firstOrNull { PubkyPublicKeyFormat.matches(it, target.publicKey) } @@ -785,6 +787,7 @@ class PaykitPaymentRequestRepo @Inject constructor( publishCreatedRequest(request, generation, expectedIdentity) } } finally { + endReview?.invoke() _isCreatingRequest.update { false } creationMutex.unlock() } diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 4ebe74447c..13844200cc 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -258,6 +258,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { paymentSubmissionActive.value = it.getArgument(0) Unit } + doAnswer { {} }.whenever(paykitSdkService).beginPaymentReview() stubMessageProcessing() whenever(settingsStore.isPaykitEnabled).thenReturn(flowOf(true)) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(sharesPrivatePaykitEndpoints = true))) @@ -2189,6 +2190,121 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { assertTrue(sut.eligibleTargets.value.isEmpty()) } + @Test + fun `creation holds review through eligibility and targeted delivery without ending another review`() = test { + val activity = stubPaymentReview() + val record = paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE).copy(proposalOutboundMessageId = 7uL) + val target = stubProposal(record) + val draft = PaykitPaymentRequestDraft(PaykitAmount(PaykitAsset.BTC, 1uL), "Lunch", clock.now().plus(60.seconds)) + val finishIdentity = CompletableDeferred() + val finishDelivery = CompletableDeferred() + whenever(paykitSdkService.identityStatus()).doSuspendableAnswer { + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + finishIdentity.await() + IdentityStatus(LOCAL_IDENTITY, PubkyIdentityCapability.PRIVATE_LINK_CAPABLE) + } + whenever(paykitSdkService.proposePaymentRequest(any(), any(), eq(LOCAL_IDENTITY))).doSuspendableAnswer { + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + record + } + whenever(paykitSdkService.processOutboundPrivateMessages(COUNTERPARTY)).doSuspendableAnswer { + finishDelivery.await() + OutboundPrivateSendReport(listOf(7uL), listOf(7uL), emptyList(), emptyList(), emptyList()) + } + val proposal = async { sut.propose(draft, target, listOf(COUNTERPARTY)).getOrThrow() } + runCurrent() + assertTrue(sut.isCreatingRequest.value) + assertFalse(activity.isPaymentSubmissionActive.value) + activity.checkRefreshAdmission(Priority.Ordered) + activity.checkRefreshAdmission(Priority.Interactive) + finishIdentity.complete(Unit) + runCurrent() + verify(paykitSdkService).processOutboundPrivateMessages(COUNTERPARTY, Priority.Interactive) + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + val finishOther = activity.beginPaymentReview() + finishDelivery.complete(Unit) + assertEquals(PaykitPaymentRequestDeliveryStatus.Sent, proposal.await().request.deliveryStatus) + assertFalse(sut.isCreatingRequest.value) + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + finishOther() + activity.checkRefreshAdmission(Priority.Background) + sut.propose(draft, target, listOf(COUNTERPARTY)).getOrThrow() + activity.checkRefreshAdmission(Priority.Background) + verify(paykitSdkService, times(2)).proposePaymentRequest(any(), any(), eq(LOCAL_IDENTITY)) + verify(paykitSdkService, never()).processPendingPrivateMessages() + } + + @Test + fun `cancelled creation releases review during eligibility or delivery and permits another creation`() = test { + val activity = stubPaymentReview() + val record = paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE).copy(proposalOutboundMessageId = 7uL) + val draft = PaykitPaymentRequestDraft(PaykitAmount(PaykitAsset.BTC, 1uL), "Lunch", clock.now().plus(60.seconds)) + for (duringDelivery in listOf(false, true)) { + var block = true + val target = stubProposal(record) + if (duringDelivery) { + whenever(paykitSdkService.processOutboundPrivateMessages(COUNTERPARTY)).doSuspendableAnswer { + if (block) awaitCancellation() + OutboundPrivateSendReport(emptyList(), emptyList(), emptyList(), emptyList(), emptyList()) + } + } else { + whenever(paykitSdkService.identityStatus()).doSuspendableAnswer { + if (block) awaitCancellation() + IdentityStatus(LOCAL_IDENTITY, PubkyIdentityCapability.PRIVATE_LINK_CAPABLE) + } + } + val proposal = async { sut.propose(draft, target, listOf(COUNTERPARTY)).getOrThrow() } + runCurrent() + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + proposal.cancel() + assertFailsWith { proposal.await() } + proposal.join() + assertFalse(sut.isCreatingRequest.value) + activity.checkRefreshAdmission(Priority.Background) + + block = false + stubProposal(record) + sut.propose(draft, target, listOf(COUNTERPARTY)).getOrThrow() + activity.checkRefreshAdmission(Priority.Background) + } + } + + @Test + fun `creation releases review when fresh eligibility rejects the target`() = test { + val activity = stubPaymentReview() + val record = paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE) + val target = stubProposal(record) + val draft = PaykitPaymentRequestDraft(PaykitAmount(PaykitAsset.BTC, 1uL), "Lunch", clock.now().plus(60.seconds)) + whenever(paykitSdkService.identityStatus()).doSuspendableAnswer { + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } + IdentityStatus(SECOND_IDENTITY, PubkyIdentityCapability.PRIVATE_LINK_CAPABLE) + } + assertSame( + PaykitPaymentRequestError.RequestUnavailable, + sut.propose(draft, target, listOf(COUNTERPARTY)).exceptionOrNull(), + ) + assertFalse(sut.isCreatingRequest.value) + activity.checkRefreshAdmission(Priority.Background) + verify(paykitSdkService, never()).proposePaymentRequest(any(), any(), any()) + doReturn(IdentityStatus(LOCAL_IDENTITY, PubkyIdentityCapability.PRIVATE_LINK_CAPABLE)) + .whenever(paykitSdkService).identityStatus() + stubProposal(record) + sut.propose(draft, target, listOf(COUNTERPARTY)).getOrThrow() + activity.checkRefreshAdmission(Priority.Background) + } + @Test fun `proposal uses canonical terms and accepts available selected methods`() = test { val target = @@ -2273,6 +2389,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { @Test fun `proposal skips status lookup only after its durable send failure`() = test { + val activity = stubPaymentReview() val record = paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE).copy(proposalOutboundMessageId = 7uL) val target = stubProposal(record) val error = mock { @@ -2300,6 +2417,8 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { listOf(COUNTERPARTY), ).getOrThrow() + activity.checkRefreshAdmission(Priority.Background) + assertFalse(sut.isCreatingRequest.value) val expected = if (failedMessageId == 7uL) { PaykitPaymentRequestDeliveryStatus.Queued } else { @@ -2312,6 +2431,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { @Test fun `proposal remains created after status read failure and propagates status read cancellation`() = test { + val activity = stubPaymentReview() val record = paymentRequestRecord(role = PaymentRequestLocalRole.PAYEE).copy(proposalOutboundMessageId = 7uL) val target = stubProposal(record) val draft = PaykitPaymentRequestDraft(PaykitAmount(PaykitAsset.BTC, 1uL), "Lunch", clock.now().plus(60.seconds)) @@ -2325,10 +2445,12 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { assertEquals(PaykitPaymentRequestDeliveryStatus.Queued, creation.request.deliveryStatus) assertEquals(listOf(creation.request), sut.paymentRequestHistory.value) + activity.checkRefreshAdmission(Priority.Background) cancelled = true assertFailsWith { sut.propose(draft, target, listOf(COUNTERPARTY)) } verify(paykitSdkService, times(2)).proposePaymentRequest(any(), any(), eq(LOCAL_IDENTITY)) assertFalse(sut.isCreatingRequest.value) + activity.checkRefreshAdmission(Priority.Background) } @Test @@ -2407,6 +2529,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { @Test fun `identity switch keeps a committed proposal out of the replacement identity state`() = test { + val activity = stubPaymentReview() val target = PaykitPaymentRequestTarget(COUNTERPARTY) val proposalStarted = CompletableDeferred() val finishProposal = CompletableDeferred() @@ -2438,12 +2561,17 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { ).getOrThrow() } proposalStarted.await() + assertFailsWith { + activity.checkRefreshAdmission(Priority.Background) + } val activation = async { sut.activate(SECOND_IDENTITY) } runCurrent() finishProposal.complete(Unit) val creation = proposal.await() activation.await() + activity.checkRefreshAdmission(Priority.Background) + assertFalse(sut.isCreatingRequest.value) assertEquals(LOCAL_IDENTITY, creation.creatorIdentity) assertFalse(creation.wasPublishedToActiveState) @@ -3021,6 +3149,12 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { assertTrue(!sut.isPending(request)) } + private fun stubPaymentReview(): PaykitSdkService { + val activity = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { mock() } + doAnswer { activity.beginPaymentReview() }.whenever(paykitSdkService).beginPaymentReview() + return activity + } + private suspend fun stubProposal(record: PaymentRequestRecord): PaykitPaymentRequestTarget { whenever(paykitSdkService.identityStatus()).thenReturn( IdentityStatus(LOCAL_IDENTITY, PubkyIdentityCapability.PRIVATE_LINK_CAPABLE),