From d6899001f64713a3b200d6b15f509e6bf623bb6a Mon Sep 17 00:00:00 2001 From: benk10 Date: Fri, 16 Oct 2026 11:10:57 -0500 Subject: [PATCH] fix: defer registry sync during profile deletion --- .../bitkit/repositories/PrivatePaykitRepo.kt | 14 +++- .../screens/profile/EditProfileViewModel.kt | 2 +- .../repositories/PrivatePaykitRepoTest.kt | 67 +++++++++++++++++++ .../to/bitkit/services/PubkyServiceTest.kt | 29 ++++++++ .../profile/EditProfileViewModelTest.kt | 13 ++-- changelog.d/next/1538.fixed.md | 1 + journeys/profile/README.md | 4 ++ journeys/profile/delete-profile.xml | 1 + 8 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 changelog.d/next/1538.fixed.md diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt index 0b4759a492..1afcf97e9d 100644 --- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt @@ -481,12 +481,20 @@ class PrivatePaykitRepo @Inject constructor( } } - suspend fun removePublishedEndpointsForCleanup(context: String): Result = withContext(serializedDispatcher) { + suspend fun removePublishedEndpointsForCleanup(context: String): Result = + removePublishedEndpointsForCleanup(context, deferAppSync = false) + + suspend fun removePublishedEndpointsForCleanup( + context: String, + deferAppSync: Boolean, + ): Result = withContext(serializedDispatcher) { runSuspendCatching { updateContactSharingCleanupPending(true) removePublishedEndpoints().getOrThrow() - syncPaykitAppAfterCleanup().getOrThrow() - updateContactSharingCleanupPending(false) + if (!deferAppSync) { + syncPaykitAppAfterCleanup().getOrThrow() + updateContactSharingCleanupPending(false) + } }.onFailure { updateContactSharingCleanupPending(true) Logger.warn("Failed to remove private Paykit endpoints during '$context'", it, context = TAG) diff --git a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt index 4c5abbe74f..b8da23acf3 100644 --- a/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt +++ b/app/src/main/java/to/bitkit/ui/screens/profile/EditProfileViewModel.kt @@ -269,7 +269,7 @@ class EditProfileViewModel @Inject constructor( } try { privatePaykitRepo.beginProfileDeletion() - privatePaykitRepo.removePublishedEndpointsForCleanup(TAG) + privatePaykitRepo.removePublishedEndpointsForCleanup(TAG, deferAppSync = true) val result = pubkyRepo.deleteProfileWithSessionRetry() if (result.isSuccess) { privatePaykitRepo.closeAndClear() diff --git a/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt index eb32f9122b..fb031c0076 100644 --- a/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt @@ -2308,6 +2308,73 @@ class PrivatePaykitRepoTest : BaseUnitTest(StandardTestDispatcher()) { ) } + @Test + fun `profile deletion withdraws private lists without an intermediate registry sync`() = test { + cacheData.value = PrivatePaykitCacheData(contacts = mapOf(CONTACT_KEY to cachedPublishedContact())) + sut = createSut() + + sut.removePublishedEndpointsForCleanup("profile deletion", deferAppSync = true).getOrThrow() + + verify(paykitSdkService).clearPrivatePaymentLists(listOf(CONTACT_KEY)) + verify(publicPaykitRepo, never()).syncPaykitApp() + assertTrue(cacheData.value.contacts.isEmpty()) + assertTrue(cacheData.value.cleanupPending) + + sut.closeAndClear().getOrThrow() + assertFalse(cacheData.value.cleanupPending) + } + + @Test + fun `interrupted profile deletion retries registry sync from persisted cleanup state`() = test { + sut.removePublishedEndpointsForCleanup("profile deletion", deferAppSync = true).getOrThrow() + assertTrue(cacheData.value.cleanupPending) + sut = createSut() + whenever(publicPaykitRepo.syncPaykitApp()).thenReturn(Result.failure(AppError("Offline"))) + + assertTrue(sut.retryPendingEndpointRemoval(emptyList()).isFailure) + assertTrue(cacheData.value.cleanupPending) + assertTrue(settingsData.value.publicPaykitCleanupPending) + + whenever(publicPaykitRepo.syncPaykitApp()).thenReturn(Result.success(Unit)) + sut.retryPendingEndpointRemoval(emptyList()).getOrThrow() + assertFalse(cacheData.value.cleanupPending) + verify(publicPaykitRepo, times(2)).syncPaykitApp() + } + + @Test + fun `cancelled profile withdrawal preserves cleanup for retry`() = test { + cacheData.value = PrivatePaykitCacheData(contacts = mapOf(CONTACT_KEY to cachedPublishedContact())) + sut = createSut() + val started = CompletableDeferred() + whenever(paykitSdkService.clearPrivatePaymentLists(listOf(CONTACT_KEY))).doSuspendableAnswer { + started.complete(Unit) + awaitCancellation() + } + val cleanup = async { sut.removePublishedEndpointsForCleanup("profile deletion", deferAppSync = true) } + started.await() + cleanup.cancel() + assertFailsWith { cleanup.await() } + + assertTrue(cacheData.value.cleanupPending) + assertTrue(cacheData.value.contacts.getValue(CONTACT_KEY).hasPublishedPrivatePaymentList) + verify(publicPaykitRepo, never()).syncPaykitApp() + } + + @Test + fun `deferred registry sync does not acknowledge undelivered private withdrawals`() = test { + cacheData.value = PrivatePaykitCacheData(contacts = mapOf(CONTACT_KEY to cachedPublishedContact())) + sut = createSut() + whenever(paykitSdkService.pendingOutboundPrivateCounterparties()).thenReturn(listOf(CONTACT_KEY)) + + val result = sut.removePublishedEndpointsForCleanup("profile deletion", deferAppSync = true) + + assertTrue(result.isFailure) + assertTrue(cacheData.value.cleanupPending) + assertTrue(cacheData.value.contacts.getValue(CONTACT_KEY).hasPublishedPrivatePaymentList) + verify(publicPaykitRepo, never()).syncPaykitApp() + sut.closeAndClear().getOrThrow() + } + @Test fun `cleanup removal marks cleanup pending when private endpoint removal fails`() = test { settingsData.value = SettingsData( diff --git a/app/src/test/java/to/bitkit/services/PubkyServiceTest.kt b/app/src/test/java/to/bitkit/services/PubkyServiceTest.kt index e068c3676f..38320d75e4 100644 --- a/app/src/test/java/to/bitkit/services/PubkyServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PubkyServiceTest.kt @@ -1,5 +1,6 @@ package to.bitkit.services +import com.synonym.paykit.EndpointSyncReport import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.cancelAndJoin @@ -12,6 +13,7 @@ import org.mockito.Mockito.mockStatic import org.mockito.kotlin.KInvocationOnMock import org.mockito.kotlin.doReturn import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.spy import org.mockito.kotlin.whenever @@ -29,6 +31,33 @@ import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds class PubkyServiceTest : BaseUnitTest() { + @Test + fun `public cleanup reconciles disabled capabilities even after endpoint removal fails`() = test { + for (failure in listOf(null, "endpoints", "registry")) { + val paykit = mock() + val sut = PubkyService(paykit) + val expected = AppError("Cleanup failed") + whenever(paykit.syncPublicEndpoints(emptyList())).thenAnswer { + if (failure == "endpoints") throw expected + EndpointSyncReport(published = emptyList(), removed = emptyList(), failed = emptyList()) + } + whenever(paykit.syncPaykitApp(privatePaymentsEnabled = false)).thenAnswer { + if (failure == "registry") throw expected + } + + val result = runSuspendCatching { sut.removeBitkitPaymentEndpoints() } + + assertEquals(failure == null, result.isSuccess) + if (failure != null) { + assertEquals(expected.message, assertIs(result.exceptionOrNull()).message) + } + inOrder(paykit).apply { + verify(paykit).syncPublicEndpoints(emptyList()) + verify(paykit).syncPaykitApp(privatePaymentsEnabled = false) + } + } + } + @Test fun `cancelling a public read cancels the underlying call`() = test { val paykit = mock() diff --git a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt index f8f8dada49..78dc54c34a 100644 --- a/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/screens/profile/EditProfileViewModelTest.kt @@ -13,6 +13,7 @@ import org.junit.Test import org.mockito.Mockito.clearInvocations import org.mockito.kotlin.any import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -194,7 +195,7 @@ class EditProfileViewModelTest : BaseUnitTest() { } assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { - verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any()) + verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) verify(pubkyRepo).deleteProfileWithSessionRetry() verify(privatePaykitRepo).closeAndClear() } @@ -215,7 +216,7 @@ class EditProfileViewModelTest : BaseUnitTest() { } assertFalse(sut.uiState.value.showDeleteFailureDialog) inOrder(privatePaykitRepo, pubkyRepo).apply { - verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any()) + verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) verify(pubkyRepo).deleteProfileWithSessionRetry() verify(privatePaykitRepo).closeAndClear() } @@ -235,6 +236,8 @@ class EditProfileViewModelTest : BaseUnitTest() { assertTrue(sut.uiState.value.showDeleteFailureDialog) assertFalse(sut.uiState.value.isDeleting) + verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) + verify(privatePaykitRepo, never()).closeAndClear() } @Test @@ -296,7 +299,7 @@ class EditProfileViewModelTest : BaseUnitTest() { @Test fun `deleteProfile should continue when private cleanup fails`() = test { val sut = createSut() - whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any()) } + whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) } .thenReturn(Result.failure(TestAppError("cleanup failed"))) whenever(pubkyRepo.deleteProfileWithSessionRetry()).thenReturn(Result.success(Unit)) advanceUntilIdle() @@ -310,7 +313,7 @@ class EditProfileViewModelTest : BaseUnitTest() { assertFalse(sut.uiState.value.showDeleteFailureDialog) assertFalse(sut.uiState.value.isDeleting) inOrder(privatePaykitRepo, pubkyRepo).apply { - verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any()) + verify(privatePaykitRepo).removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) verify(pubkyRepo).deleteProfileWithSessionRetry() verify(privatePaykitRepo).closeAndClear() } @@ -339,6 +342,8 @@ class EditProfileViewModelTest : BaseUnitTest() { whenever(pubkyRepo.publicKey).thenReturn(MutableStateFlow(TEST_PUBLIC_KEY)) whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any()) } .thenReturn(Result.success(Unit)) + whenever { privatePaykitRepo.removePublishedEndpointsForCleanup(any(), deferAppSync = eq(true)) } + .thenReturn(Result.success(Unit)) whenever { privatePaykitRepo.closeAndClear() }.thenReturn(Result.success(Unit)) return EditProfileViewModel( diff --git a/changelog.d/next/1538.fixed.md b/changelog.d/next/1538.fixed.md new file mode 100644 index 0000000000..415f5504d2 --- /dev/null +++ b/changelog.d/next/1538.fixed.md @@ -0,0 +1 @@ +Avoid an unnecessary registry update during profile deletion while preserving pending cleanup and retries. diff --git a/journeys/profile/README.md b/journeys/profile/README.md index 6b9b25ff20..ef82168389 100644 --- a/journeys/profile/README.md +++ b/journeys/profile/README.md @@ -11,3 +11,7 @@ For the reported background-resume case, verify returning directly to Profile wi Also check both Contacts entry points during delayed or failed session restoration: the drawer and Continue on the Contacts intro. They must keep a saved identity on the recovery screen, including when identity lookup is still pending. A wallet without a saved identity must still reach onboarding. With an imported grant that cannot be restored and Ring unavailable, Disconnect must forget the local identity and return to onboarding without requiring server access. The remote profile is not deleted by Disconnect. Repeat with a live session and failed endpoint cleanup or revocation: it must report failure and retain the identity. These checks require session/network fault injection beyond the journey runner capabilities. + +Profile deletion withdraws private payment lists before removing the profile and performs App Registry reconciliation during final public cleanup. An intermediate registry update is deferred; the existing private cleanup marker stays pending until deletion succeeds or normal cleanup retry completes. Public endpoint removal, capability checks, session revocation and uncertain-write safety waits remain required. + +For controlled fault testing, interrupt deletion after private withdrawal but before profile removal, then restart with the same identity. Verify cleanup remains pending, normal retry reconciles the registry, and a failed retry keeps the marker. Also hold a private withdrawal undelivered: it must remain pending, not be acknowledged by the deferred registry step. Check that Disconnect and disabling private sharing still reconcile the registry. These checks require operation gates and network fault injection beyond the journey runner capabilities. diff --git a/journeys/profile/delete-profile.xml b/journeys/profile/delete-profile.xml index 14d27866f1..d0a03ba960 100644 --- a/journeys/profile/delete-profile.xml +++ b/journeys/profile/delete-profile.xml @@ -3,6 +3,7 @@ Precondition: an onboarded disposable test wallet with Paykit enabled and a profile created through Bitkit. The profile and its contacts will be deleted; do not use a personal identity. Record the sequence so progress can be checked even when deletion completes quickly. + Start with zero contacts and record the deletion time without a fixed latency target. Repeat with 62 imported contacts, once after preparation settles and once immediately after import. Measure confirmation to onboarding and check logs for any uncertain-write safety wait. During deletion, check the indicator on Delete Profile. Save stays disabled with its label visible.