From 23dcb850fa34db4255ecd740b2da63a93d683ea4 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 09:23:42 -0500 Subject: [PATCH 1/5] fix: clean up deleted private contacts --- .../repositories/PaykitPaymentRequestRepo.kt | 49 +++++++----- .../bitkit/repositories/PrivatePaykitRepo.kt | 4 +- .../java/to/bitkit/repositories/PubkyRepo.kt | 34 ++++++--- .../to/bitkit/services/PaykitSdkService.kt | 29 +++++-- .../java/to/bitkit/services/PubkyService.kt | 3 +- ...aykitPaymentRequestRepoSubscriptionTest.kt | 1 + .../PaykitPaymentRequestRepoTest.kt | 27 +++++++ .../to/bitkit/repositories/PubkyRepoTest.kt | 21 ++++- .../bitkit/services/PaykitSdkServiceTest.kt | 76 +++++++++++++++++++ changelog.d/next/1372.fixed.md | 1 + journeys/payment-requests/README.md | 2 + .../delete-and-readd-contact.xml | 23 ++++++ 12 files changed, 233 insertions(+), 37 deletions(-) create mode 100644 changelog.d/next/1372.fixed.md create mode 100644 journeys/payment-requests/delete-and-readd-contact.xml diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index 26890757f7..c859b7ca0f 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -700,27 +700,29 @@ class PaykitPaymentRequestRepo @Inject constructor( return PaykitPaymentRequestCreation(request, creatorIdentity, wasPublishedToActiveState) } - suspend fun accept(request: PaykitPaymentRequest): Result { - if (!request.requiresAcceptance) { - return withContext(ioDispatcher) { - runSuspendCatching { - operationMutex.withLock { - if (_pendingRequests.value.none { it.id == request.id }) { - throw PaykitPaymentRequestError.RequestUnavailable - } + suspend fun accept(request: PaykitPaymentRequest): Result = withContext(ioDispatcher) { + runSuspendCatching { + if (paykitSdkService.linkedPeers().any { + it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, request.counterparty) && + it.counterpartyReceiverPath == request.counterpartyReceiverPath + }) { + throw PaykitPaymentRequestError.RequestUnavailable + } + if (!request.requiresAcceptance) { + operationMutex.withLock { + if (_pendingRequests.value.none { it.id == request.id }) { + throw PaykitPaymentRequestError.RequestUnavailable } } + } else { + updateRequest(request, PaymentRequestLifecycleState.ACCEPTED) { + paykitSdkService.acceptPaymentRequest( + counterparty = it.counterparty, + counterpartyReceiverPath = it.counterpartyReceiverPath, + paymentRequestId = it.paymentRequestId, + ) + }.getOrThrow() } - } - return updateRequest( - request = request, - resultingState = PaymentRequestLifecycleState.ACCEPTED, - ) { - paykitSdkService.acceptPaymentRequest( - counterparty = it.counterparty, - counterpartyReceiverPath = it.counterpartyReceiverPath, - paymentRequestId = it.paymentRequestId, - ) }.onFailure { Logger.warn("Failed to accept incoming Paykit payment request", it, context = TAG) } @@ -834,6 +836,13 @@ class PaykitPaymentRequestRepo @Inject constructor( paykitSdkService.receivePrivateMessagesFromLinkedPeers().also(::logIntakeFailures) val now = clock.now() val records = paykitSdkService.paymentRequests() + val blockedPeers = paykitSdkService.linkedPeers().filter { it.state == LinkedPeerState.BLOCKED } + val availableRecords = records.filterNot { record -> + blockedPeers.any { + PubkyPublicKeyFormat.matches(it.counterparty, record.counterparty) && + it.counterpartyReceiverPath == record.counterpartyReceiverPath + } + } val locallyCompletedProofKinds = expectedIdentity ?.let(paymentProofStore::completedRequestProofKindsAwaitingSubmission) .orEmpty() @@ -841,7 +850,7 @@ class PaykitPaymentRequestRepo @Inject constructor( val locallyInFlightRequestIds = expectedIdentity ?.let(paymentProofStore::inFlightRequestIds) .orEmpty() - val subscriptions = records.mapNotNull(PaymentRequestRecord::toPaykitSubscription) + val subscriptions = availableRecords.mapNotNull(PaymentRequestRecord::toPaykitSubscription) .map { it.withExpiredLifecycle(now) } val restoredAcceptances = subscriptions .filter { @@ -887,7 +896,7 @@ class PaykitPaymentRequestRepo @Inject constructor( else -> null } } - val oneTimeIncoming = records.mapNotNull { record -> + val oneTimeIncoming = availableRecords.mapNotNull { record -> when (val result = record.parseIncomingPaykitPaymentRequest(now)) { is PaykitPaymentRequestParseResult.Parsed -> result.request is PaykitPaymentRequestParseResult.Rejected -> { diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt index 83d8e0f582..e9b5a6eeba 100644 --- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt @@ -1364,11 +1364,13 @@ class PrivatePaykitRepo @Inject constructor( ).forEach { receiverPath -> runSuspendCatching { val report = paykitSdkService.clearPrivatePaymentList(publicKey, receiverPath) + ?: return@runSuspendCatching false if (report.failedToQueue.isNotEmpty() || report.failedToDeliver.isNotEmpty()) { throw PrivatePaykitError.PrivateUnavailable } + true }.onSuccess { - clearedRetryKeys += PrivateMessageDrainRetryKey(publicKey, receiverPath) + if (it) clearedRetryKeys += PrivateMessageDrainRetryKey(publicKey, receiverPath) }.onFailure { failedPublicKeys += publicKey firstError = firstError ?: it diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 9711ff9577..b394ab24ab 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -94,6 +94,8 @@ class PubkyRepo @Inject constructor( private val initializeMutex = Mutex() private val loadProfileMutex = Mutex() private val loadContactsMutex = Mutex() + private val contactsLock = Any() + private var contactsRevision = 0L private val adoptedSourceCheckMutex = Mutex() private var isServiceInitialized = false @@ -628,7 +630,7 @@ class PubkyRepo @Inject constructor( } pubkyStore.update { it.copy(contactProfileOverrides = emptyMap()) } notifyBackupStateChanged() - _contacts.update { emptyList() } + updateContacts { emptyList() } markContactsLoaded() Logger.info("Deleted all contacts", context = TAG) } @@ -680,6 +682,7 @@ class PubkyRepo @Inject constructor( val pk = _publicKey.value ?: return if (!loadContactsMutex.tryLock()) return + val revision = synchronized(contactsLock) { contactsRevision } _isLoadingContacts.update { true } var shouldMarkLoadCompleted = false try { @@ -711,7 +714,13 @@ class PubkyRepo @Inject constructor( Logger.debug("Skipped stale contacts load for '${redacted(pk)}'", context = TAG) return@onSuccess } - _contacts.update { loadedContacts } + synchronized(contactsLock) { + if (contactsRevision != revision) { + shouldMarkLoadCompleted = true + return@onSuccess + } + _contacts.update { loadedContacts } + } markContactsLoaded() shouldMarkLoadCompleted = true }.onFailure { @@ -751,8 +760,8 @@ class PubkyRepo @Inject constructor( val profile = existingProfile?.copy(publicKey = prefixedKey) ?: resolveContactProfile(prefixedKey).getOrThrow() ?: PubkyProfile.placeholder(prefixedKey) - pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey)) - _contacts.update { current -> + pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey), restorePrivateConnection = true) + updateContacts { current -> (current.filter { it.publicKey != prefixedKey } + profile) .sortedBy { it.name.lowercase() } } @@ -793,7 +802,7 @@ class PubkyRepo @Inject constructor( ) pubkyService.saveContact(prefixedKey, name) upsertContactProfileOverride(updatedProfile) - _contacts.update { current -> + updateContacts { current -> current.map { if (it.publicKey == prefixedKey) updatedProfile else it } .sortedBy { it.name.lowercase() } } @@ -807,7 +816,7 @@ class PubkyRepo @Inject constructor( val prefixedKey = publicKey.ensurePubkyPrefix() pubkyService.removeContact(prefixedKey) removeContactProfileOverride(prefixedKey) - _contacts.update { current -> current.filter { it.publicKey != prefixedKey } } + updateContacts { current -> current.filter { it.publicKey != prefixedKey } } markContactsLoaded() Logger.info("Removed contact '${redacted(prefixedKey)}'", context = TAG) } @@ -822,7 +831,7 @@ class PubkyRepo @Inject constructor( runSuspendCatching { val profile = resolveContactProfile(prefixedKey).getOrThrow() ?: PubkyProfile.placeholder(prefixedKey) - pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey)) + pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey), restorePrivateConnection = true) profile }.onFailure { Logger.warn("Failed to import contact '${redacted(prefixedKey)}'", it, context = TAG) @@ -830,7 +839,7 @@ class PubkyRepo @Inject constructor( } }.awaitAll().filterNotNull() } - _contacts.update { current -> + updateContacts { current -> val existing = current.map { it.publicKey }.toSet() (current + imported.filter { it.publicKey !in existing }) .sortedBy { it.name.lowercase() } @@ -1291,13 +1300,20 @@ class PubkyRepo @Inject constructor( runSuspendCatching { pubkyStore.reset() } _publicKey.update { null } _profile.update { null } - _contacts.update { emptyList() } + updateContacts { emptyList() } _contactsLoadVersion.update { 0L } _contactsLoadCompletionVersion.update { 0L } clearPendingImport() _sessionRestorationFailed.update { false } } + private fun updateContacts(transform: (List) -> List) { + synchronized(contactsLock) { + contactsRevision++ + _contacts.update(transform) + } + } + private fun markContactsLoaded() { _contactsLoadVersion.update { it + 1 } } diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index ecbfdffb84..1a77a4656a 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -510,13 +510,22 @@ class PaykitSdkService @Inject constructor( publicKey: String, label: String?, receiverPaths: List? = null, + restorePrivateConnection: Boolean = false, ): ContactRecord { isSetup.await() return operationMutex.withLock { withStateRevisionTracking { handle -> - val existingPaths = handle.contactRecord(publicKey)?.receiverPaths.orEmpty() + val existing = handle.contactRecord(publicKey) + check(restorePrivateConnection || existing != null) { "Contact no longer exists" } + val existingPaths = existing?.receiverPaths.orEmpty() val contactPaths = mergedReceiverPaths(existingPaths + receiverPaths.orEmpty()) - handle.saveContact(ContactUpdate(publicKey, contactPaths, label)) + handle.saveContact(ContactUpdate(publicKey, contactPaths, label)).also { + if (restorePrivateConnection) { + handle.linkedPeers().filter { + it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, publicKey) + }.forEach { handle.unblockPeer(it.counterparty, it.counterpartyReceiverPath) } + } + } } } } @@ -524,8 +533,12 @@ class PaykitSdkService @Inject constructor( suspend fun removeContact(publicKey: String): ContactRecord? { isSetup.await() return operationMutex.withLock { - handle().removeContact(publicKey).also { - notifyBackupStateChanged() + withStateRevisionTracking { handle -> + val record = handle.contactRecord(publicKey) + val peers = handle.linkedPeers().filter { PubkyPublicKeyFormat.matches(it.counterparty, publicKey) } + val receiverPaths = (record?.receiverPaths.orEmpty() + peers.map { it.counterpartyReceiverPath }).distinct() + receiverPaths.forEach { handle.blockPeer(publicKey, it) } + handle.removeContact(publicKey) } } } @@ -649,10 +662,16 @@ class PaykitSdkService @Inject constructor( suspend fun clearPrivatePaymentList( counterparty: String, receiverPath: String, - ): PrivatePaymentListDeliveryReport { + ): PrivatePaymentListDeliveryReport? { isSetup.await() return operationMutex.withLock { withStateRevisionTracking { handle -> + if (handle.linkedPeers().any { + it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, counterparty) && + it.counterpartyReceiverPath == receiverPath + }) { + return@withStateRevisionTracking null + } handle.clearPrivatePaymentListAndProcessOutbound(counterparty, receiverPath) } } diff --git a/app/src/main/java/to/bitkit/services/PubkyService.kt b/app/src/main/java/to/bitkit/services/PubkyService.kt index 3735b17bfa..8cb0667ccc 100644 --- a/app/src/main/java/to/bitkit/services/PubkyService.kt +++ b/app/src/main/java/to/bitkit/services/PubkyService.kt @@ -202,8 +202,9 @@ class PubkyService @Inject constructor( publicKey: String, label: String?, receiverPaths: List? = null, + restorePrivateConnection: Boolean = false, ): ContactRecord = ServiceQueue.CORE.background { - paykitSdkService.saveContact(publicKey, label, receiverPaths) + paykitSdkService.saveContact(publicKey, label, receiverPaths, restorePrivateConnection) } suspend fun removeContact(publicKey: String): ContactRecord? = ServiceQueue.CORE.background { diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt index a6ff85e9f5..d89a27f56c 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoSubscriptionTest.kt @@ -88,6 +88,7 @@ class PaykitPaymentRequestRepoSubscriptionTest : BaseUnitTest(StandardTestDispat whenever(paykitSdkService.processPendingPrivateMessages()).thenReturn(emptyList()) whenever(paykitSdkService.receivePrivateMessagesFromLinkedPeers()).thenReturn(emptyList()) whenever(paykitSdkService.paymentRequests()).thenReturn(emptyList()) + whenever(paykitSdkService.linkedPeers()).thenReturn(emptyList()) whenever(settingsStore.isPaykitEnabled).thenReturn(flowOf(true)) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(sharesPrivatePaykitEndpoints = true))) whenever(presentationStore.load(LOCAL_IDENTITY)).thenReturn(emptySet()) diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 42699007fd..134fb4ea22 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -90,6 +90,7 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { whenever(paykitSdkService.processPendingPrivateMessages()).thenReturn(emptyList()) whenever(paykitSdkService.receivePrivateMessagesFromLinkedPeers()).thenReturn(emptyList()) whenever(paykitSdkService.paymentRequests()).thenReturn(emptyList()) + whenever(paykitSdkService.linkedPeers()).thenReturn(emptyList()) whenever(settingsStore.isPaykitEnabled).thenReturn(flowOf(true)) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(sharesPrivatePaykitEndpoints = true))) whenever(presentationStore.load(LOCAL_IDENTITY)).thenReturn(emptySet()) @@ -119,6 +120,32 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { sut.clear() } + @Test + fun `blocking peer hides requests from an earlier snapshot`() = test { + val record = paymentRequestRecord() + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) + sut.refresh().getOrThrow() + assertEquals(1, sut.pendingRequests.value.size) + whenever(paykitSdkService.linkedPeers()).thenReturn( + listOf(linkedPeer(COUNTERPARTY, LinkedPeerState.BLOCKED, record.counterpartyReceiverPath)), + ) + sut.refresh().getOrThrow() + assertTrue(sut.pendingRequests.value.isEmpty()) + assertEquals(listOf(record.paymentRequestId), sut.paymentRequestHistory.value.map { it.paymentRequestId }) + } + + @Test + fun `blocking an already presented accepted request prevents payment`() = test { + val record = paymentRequestRecord(state = PaymentRequestLifecycleState.ACCEPTED) + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) + sut.refresh().getOrThrow() + val request = sut.pendingRequests.value.single() + whenever(paykitSdkService.linkedPeers()).thenReturn( + listOf(linkedPeer(COUNTERPARTY, LinkedPeerState.BLOCKED, record.counterpartyReceiverPath)), + ) + assertEquals(PaykitPaymentRequestError.RequestUnavailable, sut.accept(request).exceptionOrNull()) + } + @Test fun `refresh maps actionable bitcoin request`() = test { val record = paymentRequestRecord(expiresAt = clock.now().plus(60.seconds).toString()) diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 51d8f2cb69..d5a64b66f2 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1458,7 +1458,7 @@ class PubkyRepoTest : BaseUnitTest() { assertTrue(result.isSuccess) assertEquals(VALID_CONTACT_KEY_A, sut.contacts.value.single().publicKey) verifyBlocking(pubkyService) { - saveContact(VALID_CONTACT_KEY_A, profile.name, emptyList()) + saveContact(VALID_CONTACT_KEY_A, profile.name, emptyList(), restorePrivateConnection = true) } } @@ -1588,6 +1588,25 @@ class PubkyRepoTest : BaseUnitTest() { assertEquals(existingContact.name, contacts.first().name) } + @Test + fun `contact deleted during a load stays deleted when the stale snapshot returns`() = test { + authenticateForTesting() + val snapshotReady = CompletableDeferred() + val resumeLoad = CompletableDeferred() + whenever(pubkyService.contactRecords()).doSuspendableAnswer { + snapshotReady.complete(Unit) + resumeLoad.await() + listOf(createContactRecord(VALID_CONTACT_KEY_B, profile = createPaykitProfile("Deleted"))) + } + val load = launch { sut.loadContacts() } + snapshotReady.await() + assertTrue(sut.removeContact(VALID_CONTACT_KEY_B).isSuccess) + resumeLoad.complete(Unit) + load.join() + assertTrue(sut.contacts.value.isEmpty()) + assertFalse(sut.isLoadingContacts.value) + } + @Test fun `loadContacts should return early when no public key`() = test { sut.loadContacts() diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index e61d30889c..4a10a88faa 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -1,5 +1,8 @@ package to.bitkit.services +import com.synonym.paykit.ContactRecord +import com.synonym.paykit.LinkedPeerRecord +import com.synonym.paykit.LinkedPeerState import com.synonym.paykit.EncryptedLinkRecoveryMarkerPolicy import com.synonym.paykit.EndpointManagementScope import com.synonym.paykit.PaykitSdk @@ -14,6 +17,7 @@ import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.doReturn import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -97,6 +101,78 @@ class PaykitSdkServiceTest { } } + @Test + fun `deletion blocks all known receivers before removing the contact`() = runTest { + for (failBlock in listOf(false, true)) { + val sdk = mock() + val contact = mock { on { receiverPaths } doReturn listOf(PaykitReceiverPaths.WALLET) } + whenever(sdk.contactRecord(RING_PUBKY)).thenReturn(contact) + val peer = contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.LINKED) + whenever(sdk.linkedPeers()).thenReturn(listOf(peer)) + if (failBlock) whenever(sdk.blockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER)) + .thenThrow(IllegalStateException("storage failure")) + val service = PaykitSdkService(mock(), mock()) { sdk } + if (failBlock) { + assertFailsWith { service.removeContact(RING_PUBKY) } + verify(sdk, never()).removeContact(any()) + } else { + service.removeContact(RING_PUBKY) + inOrder(sdk) { + verify(sdk).blockPeer(RING_PUBKY, PaykitReceiverPaths.WALLET) + verify(sdk).blockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER) + verify(sdk).removeContact(RING_PUBKY) + } + } + } + } + + @Test + fun `only explicit readd unblocks every saved private receiver`() = runTest { + for (restoreConnection in listOf(false, true)) { + val sdk = mock() + whenever(sdk.saveContact(any())).thenReturn(mock()) + whenever(sdk.linkedPeers()).thenReturn( + listOf( + contactPeer(PaykitReceiverPaths.WALLET, LinkedPeerState.BLOCKED), + contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.BLOCKED), + ), + ) + val service = PaykitSdkService(mock(), mock()) { sdk } + if (restoreConnection) { + service.saveContact(RING_PUBKY, "Contact", restorePrivateConnection = true) + verify(sdk).unblockPeer(RING_PUBKY, PaykitReceiverPaths.WALLET) + verify(sdk).unblockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER) + } else { + assertFailsWith { service.saveContact(RING_PUBKY, "Contact") } + verify(sdk, never()).saveContact(any()) + verify(sdk, never()).unblockPeer(any(), any()) + } + } + } + + @Test + fun `blocked peer cleanup does not attempt network delivery`() = runTest { + val sdk = mock() + whenever(sdk.linkedPeers()).thenReturn(listOf(contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.BLOCKED))) + val service = PaykitSdkService(mock(), mock()) { sdk } + assertNull(service.clearPrivatePaymentList(RING_PUBKY, PaykitReceiverPaths.SERVER)) + verify(sdk, never()).clearPrivatePaymentListAndProcessOutbound(any(), any()) + } + + private fun contactPeer(path: String, state: LinkedPeerState) = LinkedPeerRecord( + counterparty = RING_PUBKY, + counterpartyReceiverPath = path, + state = state, + lastSyncAt = null, + lastPrivateReceiveAt = null, + failureCount = 0u, + localRecoveryAttemptId = null, + localRecoveryMarkerCreatedAt = null, + localRecoveryMarkerLastError = null, + remoteRecoveryAttemptId = null, + remoteRecoveryMarkerObservedAt = null, + ) + private val basePubkyClientConfig = PubkyClientConfig( requestTimeoutSecs = 30uL, localTestnetHost = null, diff --git a/changelog.d/next/1372.fixed.md b/changelog.d/next/1372.fixed.md new file mode 100644 index 0000000000..e9f38ab022 --- /dev/null +++ b/changelog.d/next/1372.fixed.md @@ -0,0 +1 @@ +Deleting a contact now stops private payment requests until you add that contact again, and background refreshes no longer restore deleted contacts. diff --git a/journeys/payment-requests/README.md b/journeys/payment-requests/README.md index a4f2c609b4..42b5a74e85 100644 --- a/journeys/payment-requests/README.md +++ b/journeys/payment-requests/README.md @@ -41,3 +41,5 @@ That run established the issuer shapes captured by the fixture: lowercase `btc`, - Saved-contact recipient: `ReviewContactRecipient` `android layout` can omit test tags applied to plain `Box` and `Column` containers. Use the raw UI Automator hierarchy when a documented container tag is not present in the formatted layout output. + +`delete-and-readd-contact.xml` uses two Bitkit instances to verify that deleting a contact revokes private requests across restart and that explicitly adding the contact again restores a fresh private connection. It does not send funds. diff --git a/journeys/payment-requests/delete-and-readd-contact.xml b/journeys/payment-requests/delete-and-readd-contact.xml new file mode 100644 index 0000000000..273b55ee8c --- /dev/null +++ b/journeys/payment-requests/delete-and-readd-contact.xml @@ -0,0 +1,23 @@ + + + Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". No payment needs to be sent. + + + On the requester, send the payer contact a Payment Request for 5,000 sats with the note "Before deletion" + On the payer, verify the Payment Request confirmation appears, then close it without paying + On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion + Verify the requester is absent from Contacts when the deletion confirmation appears + Leave Contacts and immediately reopen it, wait for the list refresh to finish, and verify the deleted contact does not reappear + Open Payment Requests and verify "Before deletion" has no available Pay action + On the requester, send another Payment Request to the payer with the note "After deletion" + On the payer, return Home and wait for two foreground request polling intervals + Verify no Payment Request confirmation appears for "After deletion" and no payable request from the deleted contact appears in Payment Requests + Restart the payer app, return Home, and wait for two foreground request polling intervals + Verify requests from the deleted contact remain unavailable for payment + On the payer, explicitly add the requester's Pubky key as a contact again + Keep both apps in the foreground until the private connection is established again + On the requester, send a new Payment Request with the note "After readd" + On the payer, open "After readd" from Payment Requests and verify its Payment Request confirmation shows the saved contact name + Close the Payment Request confirmation without paying + + From 50799d9d429db41f82c16e06caecb41d290da877 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 10:16:47 -0500 Subject: [PATCH 2/5] fix: harden contact deletion and payment authorization --- .../repositories/PaykitPaymentRequestRepo.kt | 20 +++- .../java/to/bitkit/repositories/PubkyRepo.kt | 93 +++++++++------- .../to/bitkit/services/PaykitSdkService.kt | 56 ++++++++-- .../contacts/ContactDetailViewModel.kt | 7 ++ .../screens/contacts/EditContactViewModel.kt | 7 ++ .../java/to/bitkit/viewmodels/AppViewModel.kt | 70 +++++++++--- app/src/main/res/values/strings.xml | 1 + .../PaykitPaymentRequestRepoTest.kt | 36 +++++- .../to/bitkit/repositories/PubkyRepoTest.kt | 10 +- .../bitkit/services/PaykitSdkServiceTest.kt | 105 +++++++++++++++++- .../viewmodels/AppViewModelSendFlowTest.kt | 60 ++++++++++ journeys/payment-requests/README.md | 2 + .../delete-and-readd-contact.xml | 4 +- ...elete-contact-with-active-subscription.xml | 16 +++ 14 files changed, 403 insertions(+), 84 deletions(-) create mode 100644 journeys/payment-requests/delete-contact-with-active-subscription.xml diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt index c859b7ca0f..5eda64f99f 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt @@ -700,22 +700,32 @@ class PaykitPaymentRequestRepo @Inject constructor( return PaykitPaymentRequestCreation(request, creatorIdentity, wasPublishedToActiveState) } - suspend fun accept(request: PaykitPaymentRequest): Result = withContext(ioDispatcher) { + suspend fun ensurePaymentAllowed(request: PaykitPaymentRequest): Result = withContext(ioDispatcher) { runSuspendCatching { - if (paykitSdkService.linkedPeers().any { - it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, request.counterparty) && - it.counterpartyReceiverPath == request.counterpartyReceiverPath - }) { + if ( + paykitSdkService.linkedPeers().any { + it.state == LinkedPeerState.BLOCKED && + PubkyPublicKeyFormat.matches(it.counterparty, request.counterparty) && + it.counterpartyReceiverPath == request.counterpartyReceiverPath + } + ) { throw PaykitPaymentRequestError.RequestUnavailable } + } + } + + suspend fun accept(request: PaykitPaymentRequest): Result = withContext(ioDispatcher) { + runSuspendCatching { if (!request.requiresAcceptance) { operationMutex.withLock { + ensurePaymentAllowed(request).getOrThrow() if (_pendingRequests.value.none { it.id == request.id }) { throw PaykitPaymentRequestError.RequestUnavailable } } } else { updateRequest(request, PaymentRequestLifecycleState.ACCEPTED) { + ensurePaymentAllowed(it).getOrThrow() paykitSdkService.acceptPaymentRequest( counterparty = it.counterparty, counterpartyReceiverPath = it.counterpartyReceiverPath, diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index b394ab24ab..39b392e570 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -65,6 +65,7 @@ sealed class PubkyContactError(message: String) : AppError(message) { data object AlreadyExists : PubkyContactError("Contact already exists") data object CannotAddSelf : PubkyContactError("Cannot add your own pubky as a contact") data object InvalidFormat : PubkyContactError("Invalid pubky key format") + data object ActiveSubscription : PubkyContactError("Contact has an active subscription") } data object PubkyAlreadySignedInError : AppError("Already signed in") @@ -682,51 +683,55 @@ class PubkyRepo @Inject constructor( val pk = _publicKey.value ?: return if (!loadContactsMutex.tryLock()) return - val revision = synchronized(contactsLock) { contactsRevision } _isLoadingContacts.update { true } var shouldMarkLoadCompleted = false try { - runSuspendCatching { - withContext(ioDispatcher) { - val records = pubkyService.contactRecords() - val overrides = pubkyStore.data.first().contactProfileOverrides - - coroutineScope { - records.map { record -> - async { - runSuspendCatching { - contactProfile(record.publicKey, record.label, record.profile, overrides) - }.onFailure { - Logger.warn( - "Failed to load contact '${redacted(record.publicKey)}'", - it, - context = TAG, - ) - }.getOrElse { - PubkyProfile.placeholder(record.publicKey.ensurePubkyPrefix()) + var reload: Boolean + do { + val revision = synchronized(contactsLock) { contactsRevision } + reload = false + runSuspendCatching { + withContext(ioDispatcher) { + val records = pubkyService.contactRecords() + val overrides = pubkyStore.data.first().contactProfileOverrides + + coroutineScope { + records.map { record -> + async { + runSuspendCatching { + contactProfile(record.publicKey, record.label, record.profile, overrides) + }.onFailure { + Logger.warn( + "Failed to load contact '${redacted(record.publicKey)}'", + it, + context = TAG, + ) + }.getOrElse { + PubkyProfile.placeholder(record.publicKey.ensurePubkyPrefix()) + } } - } - }.awaitAll().sortedBy { it.name.lowercase() } + }.awaitAll().sortedBy { it.name.lowercase() } + } } - } - }.onSuccess { loadedContacts -> - if (_publicKey.value != pk) { - Logger.debug("Skipped stale contacts load for '${redacted(pk)}'", context = TAG) - return@onSuccess - } - synchronized(contactsLock) { - if (contactsRevision != revision) { - shouldMarkLoadCompleted = true + }.onSuccess { loadedContacts -> + if (_publicKey.value != pk) { + Logger.debug("Skipped stale contacts load for '${redacted(pk)}'", context = TAG) return@onSuccess } - _contacts.update { loadedContacts } + synchronized(contactsLock) { + if (contactsRevision != revision) { + reload = true + return@onSuccess + } + _contacts.update { loadedContacts } + } + markContactsLoaded() + shouldMarkLoadCompleted = true + }.onFailure { + shouldMarkLoadCompleted = _publicKey.value == pk + Logger.error("Failed to load contacts", it, context = TAG) } - markContactsLoaded() - shouldMarkLoadCompleted = true - }.onFailure { - shouldMarkLoadCompleted = _publicKey.value == pk - Logger.error("Failed to load contacts", it, context = TAG) - } + } while (reload && _publicKey.value == pk) } finally { _isLoadingContacts.update { false } loadContactsMutex.unlock() @@ -760,7 +765,12 @@ class PubkyRepo @Inject constructor( val profile = existingProfile?.copy(publicKey = prefixedKey) ?: resolveContactProfile(prefixedKey).getOrThrow() ?: PubkyProfile.placeholder(prefixedKey) - pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey), restorePrivateConnection = true) + pubkyService.saveContact( + prefixedKey, + profile.name, + relevantReceiverPaths(prefixedKey), + restorePrivateConnection = true, + ) updateContacts { current -> (current.filter { it.publicKey != prefixedKey } + profile) .sortedBy { it.name.lowercase() } @@ -831,7 +841,12 @@ class PubkyRepo @Inject constructor( runSuspendCatching { val profile = resolveContactProfile(prefixedKey).getOrThrow() ?: PubkyProfile.placeholder(prefixedKey) - pubkyService.saveContact(prefixedKey, profile.name, relevantReceiverPaths(prefixedKey), restorePrivateConnection = true) + pubkyService.saveContact( + prefixedKey, + profile.name, + relevantReceiverPaths(prefixedKey), + restorePrivateConnection = true, + ) profile }.onFailure { Logger.warn("Failed to import contact '${redacted(prefixedKey)}'", it, context = TAG) diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index 1a77a4656a..0f44078707 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -25,6 +25,7 @@ import com.synonym.paykit.PaymentProofSubmission import com.synonym.paykit.PaymentReference import com.synonym.paykit.PaymentRequestAmount import com.synonym.paykit.PaymentRequestFilter +import com.synonym.paykit.PaymentRequestLifecycleState import com.synonym.paykit.PaymentRequestRecord import com.synonym.paykit.PaymentRequestRecurrence import com.synonym.paykit.PaymentRequestTerms @@ -99,6 +100,7 @@ import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.repositories.Endpoint import to.bitkit.repositories.PaykitBillingPeriod import to.bitkit.repositories.PaykitIssuerInterop +import to.bitkit.repositories.PubkyContactError import to.bitkit.repositories.PublicPaykitRepo import to.bitkit.utils.AppError import to.bitkit.utils.Logger @@ -110,6 +112,7 @@ import javax.inject.Inject import javax.inject.Singleton import kotlin.time.Duration.Companion.minutes import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant data class PaykitPreparedPrivateContactPayment( val resolution: PaykitPrivateContactPaymentResolution, @@ -519,13 +522,13 @@ class PaykitSdkService @Inject constructor( check(restorePrivateConnection || existing != null) { "Contact no longer exists" } val existingPaths = existing?.receiverPaths.orEmpty() val contactPaths = mergedReceiverPaths(existingPaths + receiverPaths.orEmpty()) - handle.saveContact(ContactUpdate(publicKey, contactPaths, label)).also { - if (restorePrivateConnection) { - handle.linkedPeers().filter { - it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, publicKey) - }.forEach { handle.unblockPeer(it.counterparty, it.counterpartyReceiverPath) } - } + if (restorePrivateConnection) { + handle.linkedPeers().filter { + it.state == LinkedPeerState.BLOCKED && + PubkyPublicKeyFormat.matches(it.counterparty, publicKey) + }.forEach { handle.unblockPeer(it.counterparty, it.counterpartyReceiverPath) } } + handle.saveContact(ContactUpdate(publicKey, contactPaths, label)) } } } @@ -535,8 +538,34 @@ class PaykitSdkService @Inject constructor( return operationMutex.withLock { withStateRevisionTracking { handle -> val record = handle.contactRecord(publicKey) - val peers = handle.linkedPeers().filter { PubkyPublicKeyFormat.matches(it.counterparty, publicKey) } - val receiverPaths = (record?.receiverPaths.orEmpty() + peers.map { it.counterpartyReceiverPath }).distinct() + val peers = handle.linkedPeers().filter { + PubkyPublicKeyFormat.matches(it.counterparty, publicKey) + } + val receiverPaths = + (record?.receiverPaths.orEmpty() + peers.map { it.counterpartyReceiverPath }).distinct() + val now = nowMillis() + val hasActiveSubscription = handle.paymentRequests().any { + val endsAt = it.terms?.recurrence?.endsAt?.let { timestamp -> + runCatching { Instant.parse(timestamp).toEpochMilliseconds() }.getOrNull() + } + PubkyPublicKeyFormat.matches(it.counterparty, publicKey) && + it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING && + (endsAt == null || endsAt > now) + } + if (hasActiveSubscription) throw PubkyContactError.ActiveSubscription + peers.filter { it.state == LinkedPeerState.LINKED }.forEach { peer -> + runSuspendCatching { + val report = handle.clearPrivatePaymentListAndProcessOutbound( + publicKey, + peer.counterpartyReceiverPath, + ) + if (report.failedToQueue.isNotEmpty() || report.failedToDeliver.isNotEmpty()) { + Logger.warn("Failed to withdraw private endpoints before contact deletion", context = TAG) + } + }.onFailure { + Logger.warn("Failed to withdraw private endpoints before contact deletion", it, context = TAG) + } + } receiverPaths.forEach { handle.blockPeer(publicKey, it) } handle.removeContact(publicKey) } @@ -666,10 +695,13 @@ class PaykitSdkService @Inject constructor( isSetup.await() return operationMutex.withLock { withStateRevisionTracking { handle -> - if (handle.linkedPeers().any { - it.state == LinkedPeerState.BLOCKED && PubkyPublicKeyFormat.matches(it.counterparty, counterparty) && - it.counterpartyReceiverPath == receiverPath - }) { + if ( + handle.linkedPeers().any { + it.state == LinkedPeerState.BLOCKED && + PubkyPublicKeyFormat.matches(it.counterparty, counterparty) && + it.counterpartyReceiverPath == receiverPath + } + ) { return@withStateRevisionTracking null } handle.clearPrivatePaymentListAndProcessOutbound(counterparty, receiverPath) diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt index 9f0c2d0cbd..949156b945 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt @@ -27,6 +27,7 @@ import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.models.Toast import to.bitkit.repositories.PrivatePaykitPaymentContext import to.bitkit.repositories.PrivatePaykitRepo +import to.bitkit.repositories.PubkyContactError import to.bitkit.repositories.PubkyRepo import to.bitkit.repositories.PublicPaykitPaymentResult import to.bitkit.ui.shared.toast.ToastEventBus @@ -189,6 +190,12 @@ class ContactDetailViewModel @Inject constructor( _effects.emit(ContactDetailEffect.ContactDeleted) } .onFailure { + if (it == PubkyContactError.ActiveSubscription) { + ToastEventBus.send( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.contacts__delete_active_subscription), + ) + } Logger.error("Failed to delete contact '$redactedPublicKey'", it, context = TAG) _uiState.update { state -> state.copy(isLoading = false) } } diff --git a/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt index 7edf27c5a7..3eb8ac3d54 100644 --- a/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/contacts/EditContactViewModel.kt @@ -22,6 +22,7 @@ import to.bitkit.R import to.bitkit.models.PubkyProfile import to.bitkit.models.PubkyProfileLink import to.bitkit.models.Toast +import to.bitkit.repositories.PubkyContactError import to.bitkit.repositories.PubkyRepo import to.bitkit.ui.components.ProfileEditLink import to.bitkit.ui.shared.toast.ToastEventBus @@ -222,6 +223,12 @@ class EditContactViewModel @Inject constructor( _effects.emit(EditContactEffect.DeleteSuccess) } .onFailure { + if (it == PubkyContactError.ActiveSubscription) { + ToastEventBus.send( + type = Toast.ToastType.ERROR, + title = context.getString(R.string.contacts__delete_active_subscription), + ) + } Logger.error("Failed to delete contact '$publicKey'", it, context = TAG) _uiState.update { it.copy(isSaving = false) } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index c95e9d15a7..7db2e7336d 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -4042,6 +4042,7 @@ class AppViewModel @Inject constructor( SendMethod.LIGHTNING -> proceedWithLightningPayment( incomingPaymentRequest, preparedPaymentProofRequest, + contactPaymentContext, amount, ) } @@ -4064,8 +4065,13 @@ class AppViewModel @Inject constructor( beforeSendAttempt = { if (preparedPaymentProofRequest != null) { markOnchainPaymentStarted(incomingPaymentRequest, address).getOrThrow() - onchainPaymentStarted = true } + incomingPaymentRequest?.let { request -> + paykitPaymentRequestRepo.ensurePaymentAllowed(request).onFailure { + paykitPaymentProofRepo.failOnchainPayment(request) + }.getOrThrow() + } + onchainPaymentStarted = preparedPaymentProofRequest != null }, onBroadcast = { txId -> proofRequest = null @@ -4137,9 +4143,11 @@ class AppViewModel @Inject constructor( } } + @Suppress("LongMethod", "CyclomaticComplexMethod") private suspend fun proceedWithLightningPayment( incomingPaymentRequest: PaykitPaymentRequest?, preparedPaymentProofRequest: PaykitPaymentRequest?, + contactPaymentContext: ContactPaymentContext?, amount: ULong, ) { val decodedInvoice = requireNotNull(_sendUiState.value.decodedInvoice) @@ -4168,7 +4176,22 @@ class AppViewModel @Inject constructor( val lnurlComment = savePendingLnurlComment(decodedInvoice, paymentHash) - sendLightning(decodedInvoice.bolt11, paymentAmount).onSuccess { actualPaymentHash -> + var authorizationError: Throwable? = null + val result = sendLightning(decodedInvoice.bolt11, paymentAmount) { + authorizationError = incomingPaymentRequest?.let { + paykitPaymentRequestRepo.ensurePaymentAllowed(it).exceptionOrNull() + } + authorizationError == null + } + authorizationError?.let { + paykitPaymentProofRepo.failLightningPayment(paymentHash) + cancelPaymentProofPreparation(proofRequest) + createdMetadataPaymentId?.let { preActivityMetadataRepo.deletePreActivityMetadata(it) } + lnurlComment?.let { activityRepo.clearPendingLightningMessage(paymentHash) } + handlePaymentPreparationFailure(it, contactPaymentContext) + return + } + result.onSuccess { actualPaymentHash -> proofRequest = null Logger.info("Lightning send result payment hash: $actualPaymentHash", context = TAG) onSendSuccess( @@ -4507,8 +4530,9 @@ class AppViewModel @Inject constructor( private suspend fun sendLightning( bolt11: String, amount: ULong? = null, + onBeforeSend: suspend () -> Boolean, ): Result { - return lightningRepo.payInvoice(bolt11 = bolt11, sats = amount).onSuccess { hash -> + return lightningRepo.payInvoice(bolt11 = bolt11, sats = amount, onBeforeSend = onBeforeSend).onSuccess { hash -> // Wait until matching payment event is received (with timeout for hold invoices) val result = lightningRepo.nodeEvents.watchUntil(LightningRepo.SEND_LN_TIMEOUT) { when (it) { @@ -5146,26 +5170,36 @@ class AppViewModel @Inject constructor( suspend fun prepareHardwareContactPayment(): Boolean { val contactPaymentContext = synchronized(contactPaymentContextLock) { activeContactPaymentContext } - if (isPreparedContactPayment(contactPaymentContext)) return true - val incomingPaymentRequest = contactPaymentContext?.incomingPaymentRequest - val proofPreparation = preparePaymentProof(incomingPaymentRequest) - if (proofPreparation.exceptionOrNull() is PaykitPaymentRequestError.OperationInProgress) { - handlePaymentPreparationFailure(PaykitPaymentRequestError.OperationInProgress, contactPaymentContext) - return false - } - val preparedPaymentProofRequest = proofPreparation.getOrNull() - if (!prepareContactPayment(contactPaymentContext)) { - cancelPaymentProofPreparation(preparedPaymentProofRequest) - return false + if (!isPreparedContactPayment(contactPaymentContext)) { + val proofPreparation = preparePaymentProof(incomingPaymentRequest) + if (proofPreparation.exceptionOrNull() is PaykitPaymentRequestError.OperationInProgress) { + handlePaymentPreparationFailure(PaykitPaymentRequestError.OperationInProgress, contactPaymentContext) + return false + } + val preparedPaymentProofRequest = proofPreparation.getOrNull() + if (!prepareContactPayment(contactPaymentContext)) { + cancelPaymentProofPreparation(preparedPaymentProofRequest) + return false + } + if (preparedPaymentProofRequest != null) { + val walletId = _sendUiState.value.hardwareWalletId ?: WalletScope.default + markOnchainPaymentStarted(incomingPaymentRequest, _sendUiState.value.address, walletId).onFailure { + synchronized(contactPaymentContextLock) { + if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null + } + cancelPaymentProofPreparation(preparedPaymentProofRequest) + handlePaymentPreparationFailure(it, contactPaymentContext) + return false + } + } } - if (preparedPaymentProofRequest != null) { - val walletId = _sendUiState.value.hardwareWalletId ?: WalletScope.default - markOnchainPaymentStarted(incomingPaymentRequest, _sendUiState.value.address, walletId).onFailure { + incomingPaymentRequest?.let { request -> + paykitPaymentRequestRepo.ensurePaymentAllowed(request).onFailure { + paykitPaymentProofRepo.failOnchainPayment(request) synchronized(contactPaymentContextLock) { if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null } - cancelPaymentProofPreparation(preparedPaymentProofRequest) handlePaymentPreparationFailure(it, contactPaymentContext) return false } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d39957fed1..17d09e7795 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -118,6 +118,7 @@ Scan QR Add Contact CONTACTS + End active subscriptions before deleting this contact. Are you sure you want to delete %1$s from your contacts? Delete %1$s? Delete Contact diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt index 134fb4ea22..0a98fd7105 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt @@ -131,7 +131,9 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { ) sut.refresh().getOrThrow() assertTrue(sut.pendingRequests.value.isEmpty()) - assertEquals(listOf(record.paymentRequestId), sut.paymentRequestHistory.value.map { it.paymentRequestId }) + whenever(paykitSdkService.paymentRequests()).thenReturn(emptyList()) + sut.refresh().getOrThrow() + assertTrue(sut.paymentRequestHistory.value.isEmpty()) } @Test @@ -146,6 +148,38 @@ class PaykitPaymentRequestRepoTest : BaseUnitTest(StandardTestDispatcher()) { assertEquals(PaykitPaymentRequestError.RequestUnavailable, sut.accept(request).exceptionOrNull()) } + @Test + fun `accepted request checks blocking after waiting for synchronization`() = test { + val record = paymentRequestRecord(state = PaymentRequestLifecycleState.ACCEPTED) + whenever(paykitSdkService.paymentRequests()).thenReturn(listOf(record)) + sut.refresh().getOrThrow() + val request = sut.pendingRequests.value.single() + val refreshPaused = CompletableDeferred() + val resumeRefresh = CompletableDeferred() + var blocked = false + var readCount = 0 + whenever(paykitSdkService.linkedPeers()).doSuspendableAnswer { + if (readCount++ == 0) { + refreshPaused.complete(Unit) + resumeRefresh.await() + emptyList() + } else if (blocked) { + listOf(linkedPeer(COUNTERPARTY, LinkedPeerState.BLOCKED, record.counterpartyReceiverPath)) + } else { + emptyList() + } + } + val refresh = async { sut.refresh() } + runCurrent() + refreshPaused.await() + val acceptance = async { sut.accept(request) } + runCurrent() + blocked = true + resumeRefresh.complete(Unit) + refresh.await().getOrThrow() + assertEquals(PaykitPaymentRequestError.RequestUnavailable, acceptance.await().exceptionOrNull()) + } + @Test fun `refresh maps actionable bitcoin request`() = test { val record = paymentRequestRecord(expiresAt = clock.now().plus(60.seconds).toString()) diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index d5a64b66f2..d4f6092cf1 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -1589,21 +1589,25 @@ class PubkyRepoTest : BaseUnitTest() { } @Test - fun `contact deleted during a load stays deleted when the stale snapshot returns`() = test { + fun `deletion during initial load preserves other saved contacts`() = test { authenticateForTesting() val snapshotReady = CompletableDeferred() val resumeLoad = CompletableDeferred() + val survivor = createContactRecord(VALID_CONTACT_KEY_A, profile = createPaykitProfile("Survivor")) + var firstRead = true whenever(pubkyService.contactRecords()).doSuspendableAnswer { + if (!firstRead) return@doSuspendableAnswer listOf(survivor) + firstRead = false snapshotReady.complete(Unit) resumeLoad.await() - listOf(createContactRecord(VALID_CONTACT_KEY_B, profile = createPaykitProfile("Deleted"))) + listOf(survivor, createContactRecord(VALID_CONTACT_KEY_B, profile = createPaykitProfile("Deleted"))) } val load = launch { sut.loadContacts() } snapshotReady.await() assertTrue(sut.removeContact(VALID_CONTACT_KEY_B).isSuccess) resumeLoad.complete(Unit) load.join() - assertTrue(sut.contacts.value.isEmpty()) + assertEquals(listOf(VALID_CONTACT_KEY_A), sut.contacts.value.map { it.publicKey }) assertFalse(sut.isLoadingContacts.value) } diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index 4a10a88faa..030b77bcfa 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -1,11 +1,17 @@ package to.bitkit.services import com.synonym.paykit.ContactRecord -import com.synonym.paykit.LinkedPeerRecord -import com.synonym.paykit.LinkedPeerState import com.synonym.paykit.EncryptedLinkRecoveryMarkerPolicy import com.synonym.paykit.EndpointManagementScope +import com.synonym.paykit.LinkedPeerRecord +import com.synonym.paykit.LinkedPeerState import com.synonym.paykit.PaykitSdk +import com.synonym.paykit.PaymentRequestLifecycleState +import com.synonym.paykit.PaymentRequestLocalRole +import com.synonym.paykit.PaymentRequestRecord +import com.synonym.paykit.PaymentRequestRecurrence +import com.synonym.paykit.PaymentRequestTerms +import com.synonym.paykit.PrivatePaymentListDeliveryReport import com.synonym.paykit.PubkyClientConfig import com.synonym.paykit.PubkyLocalSecretKey import com.synonym.paykit.PubkySessionAccess @@ -28,6 +34,7 @@ import to.bitkit.data.sharedpubky.SharedPubkyClient import to.bitkit.ext.fromHex import to.bitkit.ext.toHex import to.bitkit.models.PubkyAuthRequestError +import to.bitkit.repositories.PubkyContactError import to.bitkit.utils.AppError import kotlin.coroutines.cancellation.CancellationException import kotlin.test.assertContentEquals @@ -105,12 +112,15 @@ class PaykitSdkServiceTest { fun `deletion blocks all known receivers before removing the contact`() = runTest { for (failBlock in listOf(false, true)) { val sdk = mock() + whenever(sdk.paymentRequests()).thenReturn(emptyList()) val contact = mock { on { receiverPaths } doReturn listOf(PaykitReceiverPaths.WALLET) } whenever(sdk.contactRecord(RING_PUBKY)).thenReturn(contact) val peer = contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.LINKED) whenever(sdk.linkedPeers()).thenReturn(listOf(peer)) - if (failBlock) whenever(sdk.blockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER)) - .thenThrow(IllegalStateException("storage failure")) + if (failBlock) { + whenever(sdk.blockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER)) + .thenThrow(IllegalStateException("storage failure")) + } val service = PaykitSdkService(mock(), mock()) { sdk } if (failBlock) { assertFailsWith { service.removeContact(RING_PUBKY) } @@ -126,6 +136,66 @@ class PaykitSdkServiceTest { } } + @Test + fun `active subscription prevents deletion until it ends`() = runTest { + for ((role, endsNaturally) in listOf( + PaymentRequestLocalRole.PAYER to false, + PaymentRequestLocalRole.PAYER to true, + PaymentRequestLocalRole.PAYEE to false, + )) { + val sdk = mock() + val recurrence = mock() + val requestTerms = mock { on { this.recurrence } doReturn recurrence } + val request = mock { + on { counterparty } doReturn RING_PUBKY + on { localRole } doReturn role + on { state } doReturn PaymentRequestLifecycleState.ACTIVE_RECURRING + on { terms } doReturn requestTerms + } + whenever(sdk.linkedPeers()).thenReturn(emptyList()) + whenever(sdk.paymentRequests()).thenReturn(listOf(request)) + val service = PaykitSdkService(mock(), mock()) { sdk } + assertFailsWith { service.removeContact(RING_PUBKY) } + verify(sdk, never()).blockPeer(any(), any()) + verify(sdk, never()).removeContact(any()) + if (endsNaturally) { + whenever(recurrence.endsAt).thenReturn("2026-02-01T00:00:00Z") + } else { + whenever(request.state).thenReturn(PaymentRequestLifecycleState.CANCELED) + } + service.removeContact(RING_PUBKY) + verify(sdk).removeContact(RING_PUBKY) + } + } + + @Test + fun `deletion withdraws private endpoints before blocking even when withdrawal fails`() = runTest { + for (failWithdrawal in listOf(false, true)) { + val sdk = mock() + whenever(sdk.paymentRequests()).thenReturn(emptyList()) + whenever(sdk.linkedPeers()).thenReturn( + listOf(contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.LINKED)), + ) + val withdrawal = whenever( + sdk.clearPrivatePaymentListAndProcessOutbound(RING_PUBKY, PaykitReceiverPaths.SERVER), + ) + if (failWithdrawal) { + withdrawal.thenThrow(IllegalStateException("network unavailable")) + } else { + withdrawal.thenReturn( + PrivatePaymentListDeliveryReport(emptyList(), emptyList(), emptyList(), emptyList()), + ) + } + val service = PaykitSdkService(mock(), mock()) { sdk } + service.removeContact(RING_PUBKY) + inOrder(sdk) { + verify(sdk).clearPrivatePaymentListAndProcessOutbound(RING_PUBKY, PaykitReceiverPaths.SERVER) + verify(sdk).blockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER) + verify(sdk).removeContact(RING_PUBKY) + } + } + } + @Test fun `only explicit readd unblocks every saved private receiver`() = runTest { for (restoreConnection in listOf(false, true)) { @@ -150,6 +220,33 @@ class PaykitSdkServiceTest { } } + @Test + fun `failed private connection restoration leaves contact creation retryable`() = runTest { + for (failPeerLookup in listOf(true, false)) { + val sdk = mock() + val peers = listOf( + contactPeer(PaykitReceiverPaths.WALLET, LinkedPeerState.BLOCKED), + contactPeer(PaykitReceiverPaths.SERVER, LinkedPeerState.BLOCKED), + ) + val failure = IllegalStateException("storage failure") + if (failPeerLookup) { + whenever(sdk.linkedPeers()).thenThrow(failure).thenReturn(peers) + } else { + whenever(sdk.linkedPeers()).thenReturn(peers) + whenever(sdk.unblockPeer(RING_PUBKY, PaykitReceiverPaths.SERVER)) + .thenThrow(failure).thenReturn(peers.last().copy(state = LinkedPeerState.NOT_LINKED)) + } + whenever(sdk.saveContact(any())).thenReturn(mock()) + val service = PaykitSdkService(mock(), mock()) { sdk } + assertFailsWith { + service.saveContact(RING_PUBKY, "Contact", restorePrivateConnection = true) + } + verify(sdk, never()).saveContact(any()) + service.saveContact(RING_PUBKY, "Contact", restorePrivateConnection = true) + verify(sdk).saveContact(any()) + } + } + @Test fun `blocked peer cleanup does not attempt network delivery`() = 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 27b8dc61e9..2b1d57adf5 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -139,6 +139,7 @@ import to.bitkit.repositories.PaykitSubscription import to.bitkit.repositories.PaykitSubscriptionId import to.bitkit.repositories.PaykitSubscriptionMetadata import to.bitkit.repositories.PaykitSubscriptionRecurrence +import to.bitkit.repositories.PaymentAbortedBeforeSend import to.bitkit.repositories.PaymentPendingException import to.bitkit.repositories.PendingPaymentRepo import to.bitkit.repositories.PendingPaymentResolution @@ -385,6 +386,14 @@ class AppViewModelSendFlowTest : BaseUnitTest() { true } whenever(paykitPaymentRequestRepo.isPending(any())).thenReturn(true) + whenever { paykitPaymentRequestRepo.ensurePaymentAllowed(any()) }.thenReturn(Result.success(Unit)) + whenever { lightningRepo.payInvoice(any(), anyOrNull(), any()) }.doSuspendableAnswer { + if (it.getArgument Boolean>(2)()) { + lightningRepo.payInvoice(it.getArgument(0), it.getArgument(1)) + } else { + Result.failure(PaymentAbortedBeforeSend()) + } + } whenever(paykitPaymentRequestRepo.isExpired(any())).thenReturn(false) whenever(paykitPaymentRequestRepo.isProcessing(any())).thenReturn(false) whenever(paykitPaymentProofRepo.onchainPaymentResolutions).thenReturn(onchainPaymentResolutions) @@ -6351,6 +6360,53 @@ class AppViewModelSendFlowTest : BaseUnitTest() { verify(lightningRepo).payInvoice(bolt11 = bolt11, sats = null) } + @Test + fun `blocking after lightning preparation prevents dispatch and clears the proof`() = test { + for (preparationSucceeds in listOf(true, false)) { + val request = paymentRequest() + val bolt11 = "lnbcrt1paymentrequest" + val paymentHash = "010203" + val privateContext = PrivatePaykitPaymentContext("bitkit/server", 7uL) + balanceState.value = BalanceState(maxSendLightningSats = 100_000u) + whenever( + paykitPaymentProofRepo.prepare(request, MethodId.Bolt11.rawValue, PaykitPaymentProofKind.Lightning), + ).doSuspendableAnswer { + setSendState( + sut.sendUiState.value.copy(decodedInvoice = lightningInvoice(bolt11, request.amountSats)), + ) + if (preparationSucceeds) { + Result.success(Unit) + } else { + Result.failure(IllegalStateException("proof unavailable")) + } + } + whenever(paykitPaymentRequestRepo.accept(request)).thenReturn(Result.success(Unit)) + whenever(privatePaykitRepo.consumePrivatePaymentList(testPublicKey, privateContext)) + .thenReturn(Result.success(Unit)) + whenever(paykitPaymentProofRepo.associateLightningPayment(request, paymentHash, MethodId.Bolt11.rawValue)) + .doSuspendableAnswer { + whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)) + .thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) + Result.success(Unit) + } + setActiveContactPaymentContext(testPublicKey, privateContext, request) + setSendState( + SendUiState( + address = bolt11, + amount = request.amountSats, + payMethod = SendMethod.LIGHTNING, + isPaymentRequest = true, + ), + ) + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + verify(paykitPaymentRequestRepo).accept(request) + verify(lightningRepo, never()).payInvoice(any(), anyOrNull()) + verify(paykitPaymentProofRepo).failLightningPayment(paymentHash) + clearInvocations(lightningRepo, paykitPaymentProofRepo, paykitPaymentRequestRepo) + } + } + @Test fun `pending incoming lightning payment keeps its proof association`() = test { val request = paymentRequest() @@ -6560,6 +6616,10 @@ class AppViewModelSendFlowTest : BaseUnitTest() { "hardware-wallet", ) } + whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)) + .thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) + assertFalse(sut.prepareHardwareContactPayment()) + verify(paykitPaymentProofRepo).failOnchainPayment(request) } @Test diff --git a/journeys/payment-requests/README.md b/journeys/payment-requests/README.md index 42b5a74e85..61a159491b 100644 --- a/journeys/payment-requests/README.md +++ b/journeys/payment-requests/README.md @@ -43,3 +43,5 @@ That run established the issuer shapes captured by the fixture: lowercase `btc`, `android layout` can omit test tags applied to plain `Box` and `Column` containers. Use the raw UI Automator hierarchy when a documented container tag is not present in the formatted layout output. `delete-and-readd-contact.xml` uses two Bitkit instances to verify that deleting a contact revokes private requests across restart and that explicitly adding the contact again restores a fresh private connection. It does not send funds. + +`delete-contact-with-active-subscription.xml` requires an accepted open-ended payer subscription. It verifies that deletion explains why the contact must stay saved until the subscription ends, then that canceling, deleting, and readding does not revive it. No new payment is sent. Both contact-deletion journeys are mirrored on iOS and Android. diff --git a/journeys/payment-requests/delete-and-readd-contact.xml b/journeys/payment-requests/delete-and-readd-contact.xml index 273b55ee8c..e740907019 100644 --- a/journeys/payment-requests/delete-and-readd-contact.xml +++ b/journeys/payment-requests/delete-and-readd-contact.xml @@ -1,6 +1,6 @@ - Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". No payment needs to be sent. + Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". The payer must have no active subscription with the requester. No payment needs to be sent. On the requester, send the payer contact a Payment Request for 5,000 sats with the note "Before deletion" @@ -8,7 +8,7 @@ On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion Verify the requester is absent from Contacts when the deletion confirmation appears Leave Contacts and immediately reopen it, wait for the list refresh to finish, and verify the deleted contact does not reappear - Open Payment Requests and verify "Before deletion" has no available Pay action + Open Payment Requests and wait for the request list to refresh, and verify "Before deletion" is no longer listed On the requester, send another Payment Request to the payer with the note "After deletion" On the payer, return Home and wait for two foreground request polling intervals Verify no Payment Request confirmation appears for "After deletion" and no payable request from the deleted contact appears in Payment Requests diff --git a/journeys/payment-requests/delete-contact-with-active-subscription.xml b/journeys/payment-requests/delete-contact-with-active-subscription.xml new file mode 100644 index 0000000000..2dc3c983d8 --- /dev/null +++ b/journeys/payment-requests/delete-contact-with-active-subscription.xml @@ -0,0 +1,16 @@ + + + Verifies that an active subscription must end before its contact can be deleted. Requires two authenticated Bitkit instances saved as contacts, with an accepted open-ended subscription on the payer. No new payment needs to be sent. + + + On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion + Verify the error "End active subscriptions before deleting this contact." appears and the contact remains saved + Open Subscriptions and verify the active subscription is still visible + Cancel the subscription and verify it is no longer active + Return to the requester's contact, choose Delete Contact, and confirm deletion + Verify the deletion confirmation appears and the requester is absent from Contacts + Explicitly add the requester's Pubky key as a contact again and wait for the private connection to be established + Return Home and wait for two foreground request polling intervals + Verify the canceled subscription remains inactive and no missed subscription payment confirmation appears + + From 627936b765c75bed9230aa4ac72e87a2ab360716 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 30 Sep 2026 03:00:08 +0100 Subject: [PATCH 3/5] fix: preserve cancellation in contact deletion --- app/src/main/java/to/bitkit/services/PaykitSdkService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index aa12199a56..e2da7cf863 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -559,7 +559,7 @@ class PaykitSdkService @Inject constructor( val now = nowMillis() val hasActiveSubscription = handle.paymentRequests().any { val endsAt = it.terms?.recurrence?.endsAt?.let { timestamp -> - runCatching { Instant.parse(timestamp).toEpochMilliseconds() }.getOrNull() + runSuspendCatching { Instant.parse(timestamp).toEpochMilliseconds() }.getOrNull() } PubkyPublicKeyFormat.matches(it.counterparty, publicKey) && it.state == PaymentRequestLifecycleState.ACTIVE_RECURRING && From 5114d34d60662517e174d99dcd2182ce89e054a2 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 30 Sep 2026 11:11:42 +0100 Subject: [PATCH 4/5] fix: recheck hardware payment retries --- .../screens/wallets/send/HwSendSignScreen.kt | 12 +++- .../screens/wallets/send/HwSendViewModel.kt | 14 ++-- .../java/to/bitkit/ui/sheets/SendSheet.kt | 1 + .../java/to/bitkit/viewmodels/AppViewModel.kt | 27 +++++--- .../wallets/send/HwSendViewModelTest.kt | 69 +++++++++++++++++-- .../viewmodels/AppViewModelSendFlowTest.kt | 51 +++++++++++++- 6 files changed, 152 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendSignScreen.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendSignScreen.kt index f6048341ae..53fd44b879 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendSignScreen.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendSignScreen.kt @@ -44,6 +44,7 @@ fun HwSendSignScreen( satsPerVByte: ULong, viewModel: HwSendViewModel, prepareContactPayment: suspend () -> Boolean, + authorizeContactPayment: suspend (hasAttemptedBroadcast: Boolean) -> Boolean, onBack: () -> Unit, ) { val uiState by viewModel.uiState.collectAsStateWithLifecycle() @@ -72,14 +73,21 @@ fun HwSendSignScreen( isSigning = uiState.isSigning, hasPendingBroadcast = uiState.hasPendingBroadcast, onBack = onBackRequest, - onOpenConnect = { viewModel.signAndBroadcast(request, prepareContactPayment) }, + onOpenConnect = { + viewModel.signAndBroadcast(request, prepareContactPayment, authorizeContactPayment) + }, ) if (uiState.isPassphraseRequired) { HwPassphrasePromptSheet( isVerifying = uiState.isVerifyingPassphrase, onSubmit = { passphrase -> - viewModel.submitPassphrase(request, passphrase, prepareContactPayment) + viewModel.submitPassphrase( + request, + passphrase, + prepareContactPayment, + authorizeContactPayment, + ) }, onDismiss = viewModel::dismissPassphrase, ) diff --git a/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt index 397d6d97cb..f3f84ca4c6 100644 --- a/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/wallets/send/HwSendViewModel.kt @@ -70,7 +70,8 @@ class HwSendViewModel @Inject constructor( fun signAndBroadcast( request: HwSendRequest, - beforeBroadcast: suspend () -> Boolean = { true }, + prepareContactPayment: suspend () -> Boolean = { true }, + authorizeContactPayment: suspend (hasAttemptedBroadcast: Boolean) -> Boolean = { true }, ) { if (_uiState.value.isSigning || signingJob?.isActive == true) return if (pendingBroadcast?.matches(request) == false) return @@ -97,11 +98,14 @@ class HwSendViewModel @Inject constructor( } var payment = checkNotNull(pending) { "Hardware payment was not prepared" } if (payment.isPreparedForBroadcast.not()) { - if (!beforeBroadcast()) return@runCatching + if (!prepareContactPayment()) return@runCatching payment = payment.copy(isPreparedForBroadcast = true) pendingBroadcast = payment } + if (!authorizeContactPayment(payment.hasAttemptedBroadcast)) return@runCatching _uiState.update { it.copy(isBroadcastUnresolved = true) } + payment = payment.copy(hasAttemptedBroadcast = true) + pendingBroadcast = payment val result = withTimeout(BROADCAST_TIMEOUT) { hwWalletRepo.broadcastFunding(payment.signedTx).getOrThrow() } @@ -122,7 +126,8 @@ class HwSendViewModel @Inject constructor( fun submitPassphrase( request: HwSendRequest, passphrase: String, - beforeBroadcast: suspend () -> Boolean = { true }, + prepareContactPayment: suspend () -> Boolean = { true }, + authorizeContactPayment: suspend (hasAttemptedBroadcast: Boolean) -> Boolean = { true }, ) { if (passphrase.isEmpty()) return val state = _uiState.value @@ -137,7 +142,7 @@ class HwSendViewModel @Inject constructor( .onSuccess { if (!_uiState.value.isPassphraseRequired) return@onSuccess _uiState.update { it.copy(isPassphraseRequired = false) } - signAndBroadcast(request, beforeBroadcast) + signAndBroadcast(request, prepareContactPayment, authorizeContactPayment) } .onFailure { error -> if (error is HwPassphraseMismatchError) { @@ -333,6 +338,7 @@ private data class PendingHwSendBroadcast( val request: HwSendRequest, val signedTx: HwFundingSignedTx, val isPreparedForBroadcast: Boolean = false, + val hasAttemptedBroadcast: Boolean = false, ) { fun matches(request: HwSendRequest): Boolean = this.request == request } diff --git a/app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt b/app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt index 0ed710ef41..c774c0f731 100644 --- a/app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt +++ b/app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt @@ -320,6 +320,7 @@ fun SendSheet( satsPerVByte = satsPerVByte, viewModel = hwSendViewModel, prepareContactPayment = appViewModel::prepareHardwareContactPayment, + authorizeContactPayment = appViewModel::authorizeHardwareContactPayment, onBack = { navController.previousBackStackEntry ?.savedStateHandle diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index c33dda02ae..e4b0262b5b 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -5204,19 +5204,26 @@ class AppViewModel @Inject constructor( } } } - incomingPaymentRequest?.let { request -> - paykitPaymentRequestRepo.ensurePaymentAllowed(request).onFailure { - paykitPaymentProofRepo.failOnchainPayment(request) - synchronized(contactPaymentContextLock) { - if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null - } - handlePaymentPreparationFailure(it, contactPaymentContext) - return false - } - } return true } + suspend fun authorizeHardwareContactPayment(hasAttemptedBroadcast: Boolean): Boolean { + val contactPaymentContext = synchronized(contactPaymentContextLock) { activeContactPaymentContext } + val request = contactPaymentContext?.incomingPaymentRequest ?: return true + val error = paykitPaymentRequestRepo.ensurePaymentAllowed(request).exceptionOrNull() ?: return true + if (hasAttemptedBroadcast) { + toast(error) + return false + } + + paykitPaymentProofRepo.failOnchainPayment(request) + synchronized(contactPaymentContextLock) { + if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null + } + handlePaymentPreparationFailure(error, contactPaymentContext) + return false + } + fun completeHardwareContactPayment(txId: String) { val incomingPaymentRequest = synchronized(contactPaymentContextLock) { activeContactPaymentContext?.incomingPaymentRequest diff --git a/app/src/test/java/to/bitkit/ui/screens/wallets/send/HwSendViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/wallets/send/HwSendViewModelTest.kt index 5e700b8c11..aa8ed4d018 100644 --- a/app/src/test/java/to/bitkit/ui/screens/wallets/send/HwSendViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/wallets/send/HwSendViewModelTest.kt @@ -118,7 +118,7 @@ class HwSendViewModelTest : BaseUnitTest() { } @Test - fun `contact preparation runs after signing and only once across broadcast retry`() = test { + fun `contact payment is prepared once and authorized before each broadcast attempt`() = test { whenever(context.getString(any())).thenReturn("message") val fixture = stubSuccessfulPayment() whenever(hwWalletRepo.broadcastFunding(fixture.signedTx)).thenReturn( @@ -132,23 +132,76 @@ class HwSendViewModelTest : BaseUnitTest() { preparationCalls += 1 true } + val authorizationAttempts = mutableListOf() + val authorizeContactPayment: suspend (Boolean) -> Boolean = { + authorizationAttempts += it + true + } - sut.signAndBroadcast(request(), prepareContactPayment) + sut.signAndBroadcast(request(), prepareContactPayment, authorizeContactPayment) advanceUntilIdle() assertEquals(1, preparationCalls) + assertEquals(listOf(false), authorizationAttempts) assertTrue(sut.uiState.value.hasPendingBroadcast) - sut.signAndBroadcast(request(), prepareContactPayment) + sut.signAndBroadcast(request(), prepareContactPayment, authorizeContactPayment) advanceUntilIdle() assertEquals(1, preparationCalls) + assertEquals(listOf(false, true), authorizationAttempts) verify(hwWalletRepo).signFunding(WALLET_ID, fixture.funding) verify(hwWalletRepo, times(2)).broadcastFunding(fixture.signedTx) sut.completeBroadcast() assertFalse(sut.uiState.value.hasPendingBroadcast) } + @Test + fun `denied retry keeps the signed transaction after a failed broadcast`() = test { + whenever(context.getString(any())).thenReturn("message") + val fixture = stubSuccessfulPayment() + whenever(hwWalletRepo.broadcastFunding(fixture.signedTx)) + .thenReturn(Result.failure(BroadcastException.ElectrumException("connection failed"))) + var isAuthorized = true + var preparationCalls = 0 + val authorizationAttempts = mutableListOf() + + sut.signAndBroadcast( + request = request(), + prepareContactPayment = { + preparationCalls += 1 + true + }, + authorizeContactPayment = { + authorizationAttempts += it + isAuthorized + }, + ) + advanceUntilIdle() + + isAuthorized = false + sut.signAndBroadcast( + request = request(), + prepareContactPayment = { + preparationCalls += 1 + true + }, + authorizeContactPayment = { + authorizationAttempts += it + isAuthorized + }, + ) + advanceUntilIdle() + + assertEquals(1, preparationCalls) + assertEquals(listOf(false, true), authorizationAttempts) + verify(hwWalletRepo).signFunding(WALLET_ID, fixture.funding) + verify(hwWalletRepo).broadcastFunding(fixture.signedTx) + assertTrue(sut.uiState.value.hasPendingBroadcast) + assertFalse(sut.uiState.value.isBroadcastUnresolved) + assertFalse(sut.uiState.value.isSigning) + } + @Test fun `passphrase reconnect keeps contact preparation before broadcast`() = test { val fixture = stubSuccessfulPayment() @@ -160,15 +213,21 @@ class HwSendViewModelTest : BaseUnitTest() { preparationCalls += 1 true } + val authorizationAttempts = mutableListOf() + val authorizeContactPayment: suspend (Boolean) -> Boolean = { + authorizationAttempts += it + true + } - sut.signAndBroadcast(request(), prepareContactPayment) + sut.signAndBroadcast(request(), prepareContactPayment, authorizeContactPayment) advanceUntilIdle() assertTrue(sut.uiState.value.isPassphraseRequired) - sut.submitPassphrase(request(), "hidden wallet", prepareContactPayment) + sut.submitPassphrase(request(), "hidden wallet", prepareContactPayment, authorizeContactPayment) advanceUntilIdle() assertEquals(1, preparationCalls) + assertEquals(listOf(false), authorizationAttempts) verify(hwWalletRepo).broadcastFunding(fixture.signedTx) assertFalse(sut.uiState.value.isPassphraseRequired) } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 2fcdb06dd3..a2f1c04bf8 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -6650,9 +6650,58 @@ class AppViewModelSendFlowTest : BaseUnitTest() { "hardware-wallet", ) } + verify(paykitPaymentRequestRepo, never()).ensurePaymentAllowed(request) + whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)) .thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) - assertFalse(sut.prepareHardwareContactPayment()) + assertFalse(sut.authorizeHardwareContactPayment(hasAttemptedBroadcast = false)) + verify(paykitPaymentProofRepo).failOnchainPayment(request) + } + + @Test + fun `hardware retry denial keeps the started proof until cancellation`() = test { + val request = paymentRequest() + val privateContext = PrivatePaykitPaymentContext("bitkit/server", 7uL) + whenever(context.getString(R.string.common__error)).thenReturn("Error") + whenever(paykitPaymentRequestRepo.accept(request)).thenReturn(Result.success(Unit)) + whenever(paykitPaymentRequestRepo.ensurePaymentAllowed(request)) + .thenReturn(Result.failure(PaykitPaymentRequestError.RequestUnavailable)) + whenever(privatePaykitRepo.consumePrivatePaymentList(testPublicKey, privateContext)) + .thenReturn(Result.success(Unit)) + setActiveContactPaymentContext( + publicKey = testPublicKey, + privatePaymentContext = privateContext, + incomingPaymentRequest = request, + isInitialSubscriptionPayment = true, + ) + setSendState( + SendUiState( + address = "bcrt1qpaymentrequest", + amount = request.amountSats, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + isPaymentRequest = true, + isInitialSubscriptionPayment = true, + hardwareWalletId = "hardware-wallet", + ) + ) + val sheet = Sheet.Send(SendRoute.HardwareSign) + sut.showSheet(sheet) + advanceUntilIdle() + assertTrue(sut.prepareHardwareContactPayment()) + + sut.sendEffect.test { + assertFalse(sut.authorizeHardwareContactPayment(hasAttemptedBroadcast = true)) + expectNoEvents() + } + + assertEquals(sheet, sut.currentSheet.value) + verify(toastManager).enqueue(any()) + verify(paykitPaymentProofRepo, never()).failOnchainPayment(request) + + sut.onHardwareSignCancelled() + advanceUntilIdle() + verify(paykitPaymentProofRepo).failOnchainPayment(request) } From 8b24c34b5381bf4a087a618830186d7c3de769d7 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 30 Sep 2026 13:11:42 +0100 Subject: [PATCH 5/5] refactor: simplify hardware payment preparation --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index de321a86ad..a869b9fe0e 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -5227,30 +5227,30 @@ class AppViewModel @Inject constructor( suspend fun prepareHardwareContactPayment(): Boolean { val contactPaymentContext = synchronized(contactPaymentContextLock) { activeContactPaymentContext } + if (isPreparedContactPayment(contactPaymentContext)) return true + val incomingPaymentRequest = contactPaymentContext?.incomingPaymentRequest - if (!isPreparedContactPayment(contactPaymentContext)) { - val proofPreparation = preparePaymentProof(incomingPaymentRequest) - if (proofPreparation.exceptionOrNull() is PaykitPaymentRequestError.OperationInProgress) { - handlePaymentPreparationFailure(PaykitPaymentRequestError.OperationInProgress, contactPaymentContext) - return false - } - val preparedPaymentProofRequest = proofPreparation.getOrNull() - if (!prepareContactPayment(contactPaymentContext)) { + val proofPreparation = preparePaymentProof(incomingPaymentRequest) + if (proofPreparation.exceptionOrNull() is PaykitPaymentRequestError.OperationInProgress) { + handlePaymentPreparationFailure(PaykitPaymentRequestError.OperationInProgress, contactPaymentContext) + return false + } + val preparedPaymentProofRequest = proofPreparation.getOrNull() + if (!prepareContactPayment(contactPaymentContext)) { + cancelPaymentProofPreparation(preparedPaymentProofRequest) + return false + } + if (preparedPaymentProofRequest != null) { + val walletId = _sendUiState.value.hardwareWalletId ?: WalletScope.default + markOnchainPaymentStarted(incomingPaymentRequest, _sendUiState.value.address, walletId).onFailure { + synchronized(contactPaymentContextLock) { + if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null + } + releasePrivatePaymentListIfNeeded(contactPaymentContext) cancelPaymentProofPreparation(preparedPaymentProofRequest) + handlePaymentPreparationFailure(it, contactPaymentContext) return false } - if (preparedPaymentProofRequest != null) { - val walletId = _sendUiState.value.hardwareWalletId ?: WalletScope.default - markOnchainPaymentStarted(incomingPaymentRequest, _sendUiState.value.address, walletId).onFailure { - synchronized(contactPaymentContextLock) { - if (preparedContactPaymentContext == contactPaymentContext) preparedContactPaymentContext = null - } - releasePrivatePaymentListIfNeeded(contactPaymentContext) - cancelPaymentProofPreparation(preparedPaymentProofRequest) - handlePaymentPreparationFailure(it, contactPaymentContext) - return false - } - } } return true }