From 4d86489c360e60167fbc9cf5f25291268975f372 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:49:48 +0200 Subject: [PATCH 01/11] fix: decouple wallet restore from profile refresh --- .../java/to/bitkit/repositories/BackupRepo.kt | 7 +- .../java/to/bitkit/repositories/PubkyRepo.kt | 43 +++++++++- .../to/bitkit/viewmodels/WalletViewModel.kt | 4 +- .../to/bitkit/repositories/BackupRepoTest.kt | 52 ++++++++++++ .../to/bitkit/repositories/PubkyRepoTest.kt | 81 +++++++++++++++++++ .../java/to/bitkit/ui/WalletViewModelTest.kt | 28 ++++++- changelog.d/next/restore-identity.fixed.md | 1 + .../wallet/restore-during-identity-outage.xml | 18 +++++ 8 files changed, 224 insertions(+), 10 deletions(-) create mode 100644 changelog.d/next/restore-identity.fixed.md create mode 100644 journeys/wallet/restore-during-identity-outage.xml diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index dabef786d4..f9ea2491d3 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -743,8 +743,8 @@ class BackupRepo @Inject constructor( // propagate and fail the backup rather than upload an empty set over the stored ones. val hwWalletNames = hwWalletStore.backupSnapshot().takeIf { it.isNotEmpty() } val cacheData = cacheStore.data.first() - val pubkySession = pubkyRepo.snapshotSessionBackupState().getOrDefault(null) - val pubkyContactProfileOverrides = pubkyRepo.snapshotContactProfileOverrides().getOrDefault(null) + val pubkySession = pubkyRepo.snapshotSessionBackupState().getOrThrow() + val pubkyContactProfileOverrides = pubkyRepo.snapshotContactProfileOverrides().getOrThrow() val payload = MetadataBackupV1( createdAt = currentTimeMillis(), @@ -782,6 +782,9 @@ class BackupRepo @Inject constructor( val privateRepo = privatePaykitRepo.get() val privateVersion = privateRepo.backupStateVersion.value val paykitSdkBackupState = privateRepo.backupSnapshot().getOrThrow() + check(paykitSdkBackupState.state != null || !pubkyRepo.hasIdentityForBackup()) { + "Paykit identity is unavailable for backup" + } val reservationRepo = privatePaykitAddressReservationRepo.get() val reservationVersion = reservationRepo.backupStateVersion.value val privateReservations = reservationRepo.backupSnapshot().getOrThrow() diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 39a03aefb4..8340349b42 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -333,6 +333,8 @@ class PubkyRepo @Inject constructor( } private val initializationReady = CompletableDeferred() + private val restoreInitializationLock = Any() + private var restoreInitialization: Pair? = null init { scope.launch { initialize() }.invokeOnCompletion { @@ -350,9 +352,38 @@ class PubkyRepo @Inject constructor( runSuspendCatching { pubkyService.republishIdentityIfNeeded(publicKey.value) } } - suspend fun initialize() = withContext(ioDispatcher) { - val restored = initializeMutex.withLock { initializeSession() } + suspend fun initialize() = initializeForRevision() + + fun scheduleInitializationAfterRestore() { + val job = synchronized(restoreInitializationLock) { + val revision = storedIdentityRevision.get() + restoreInitialization?.takeIf { it.first == revision && !it.second.isCompleted }?.second + ?: scope.launch(start = CoroutineStart.LAZY) { + initializeForRevision(revision) + }.also { pending -> + restoreInitialization = revision to pending + pending.invokeOnCompletion { + synchronized(restoreInitializationLock) { + if (restoreInitialization?.second === pending) restoreInitialization = null + } + } + } + } + job.start() + } + + private suspend fun initializeForRevision( + expectedRevision: Long? = null, + ): Result = withContext(ioDispatcher) { + val (restored, revision) = initializeMutex.withLock { + if (expectedRevision != null && expectedRevision != storedIdentityRevision.get()) { + return@withContext Result.success(Unit) + } + initializeSession() to storedIdentityRevision.get() + } + if (revision != storedIdentityRevision.get()) return@withContext Result.success(Unit) if (restored) loadIdentityData() + if (revision != storedIdentityRevision.get()) return@withContext Result.success(Unit) checkAdoptedSource() } @@ -1576,6 +1607,14 @@ class PubkyRepo @Inject constructor( !keychain.loadString(Keychain.Key.PUBKY_SECRET_KEY.name).isNullOrEmpty() } + internal suspend fun hasIdentityForBackup(): Boolean = withContext(ioDispatcher) { + _publicKey.value != null || listOf( + Keychain.Key.PAYKIT_SESSION, + Keychain.Key.PUBKY_SECRET_KEY, + Keychain.Key.SHARED_PUBKY_SOURCE, + ).any { !keychain.loadString(it.name).isNullOrEmpty() } + } + suspend fun parseAuthUrl(authUrl: String): Result = runSuspendCatching { withContext(ioDispatcher) { if (PubkyAuthRequest.isSignupUrl(authUrl) && !PubkyAuthRequest.isGrantSignupUrl(authUrl)) { diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 114798b9d0..1d74917e2e 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -235,7 +235,7 @@ class WalletViewModel @Inject constructor( if (resumePendingWalletBackup) { backupRepo.performFullRestoreFromLatestBackup(onCacheRestored = walletRepo::loadFromCache) .getOrThrow() - pubkyRepo.initialize() + pubkyRepo.scheduleInitializationAfterRestore() } else { restoreFromMostRecentBackup() } @@ -273,7 +273,7 @@ class WalletViewModel @Inject constructor( .getOrThrow() } - pubkyRepo.initialize() + pubkyRepo.scheduleInitializationAfterRestore() } private suspend fun restoreFromRNRemoteBackup() { diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index cd7289587f..1eee30e7dd 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -189,9 +189,61 @@ class BackupRepoTest : BaseUnitTest() { }.thenReturn(Result.success(Unit)) whenever { vssBackupClient.listKeys() }.thenReturn(Result.success(emptyList())) + whenever(pubkyRepo.hasIdentityForBackup()).thenReturn(false) sut = createSut() } + @Test + fun `metadata backup preserves complete data when identity records cannot be read`() = test { + stubMetadataBackupReads() + val error = AppError("keychain unavailable") + whenever(pubkyRepo.snapshotSessionBackupState()).thenReturn(Result.failure(error)) + assertEquals(error, sut.triggerBackup(BackupCategory.METADATA).exceptionOrNull()) + verify(vssBackupClient, never()).putObject(eq(BackupCategory.METADATA.name), any()) + + stubMetadataBackupReads() + sut.triggerBackup(BackupCategory.METADATA).getOrThrow() + verify(vssBackupClient).putObject(eq(BackupCategory.METADATA.name), any()) + } + + @Test + fun `metadata backup preserves complete data when contact overrides cannot be read`() = test { + stubMetadataBackupReads() + val error = AppError("store unavailable") + whenever(pubkyRepo.snapshotContactProfileOverrides()).thenReturn(Result.failure(error)) + assertEquals(error, sut.triggerBackup(BackupCategory.METADATA).exceptionOrNull()) + verify(vssBackupClient, never()).putObject(eq(BackupCategory.METADATA.name), any()) + + stubMetadataBackupReads() + sut.triggerBackup(BackupCategory.METADATA).getOrThrow() + verify(vssBackupClient).putObject(eq(BackupCategory.METADATA.name), any()) + } + + @Test + fun `wallet backup preserves complete data while stored identity is unresolved`() = test { + whenever(pubkyRepo.hasIdentityForBackup()).thenReturn(true) + assertTrue(sut.triggerBackup(BackupCategory.WALLET).isFailure) + verify(vssBackupClient, never()).putObject(eq(BackupCategory.WALLET.name), any()) + + whenever(privatePaykitRepo.backupSnapshot()).thenReturn( + Result.success(PrivatePaykitBackupSnapshot("fresh-sdk-state", 0)), + ) + sut.triggerBackup(BackupCategory.WALLET).getOrThrow() + val payload = argumentCaptor() + verify(vssBackupClient).putObject(eq(BackupCategory.WALLET.name), payload.capture()) + val backup = json.decodeFromString(payload.firstValue.decodeToString()) + assertEquals("fresh-sdk-state", backup.paykitSdkBackupState) + } + + @Test + fun `wallet backup cannot treat unreadable identity as absent`() = test { + whenever(pubkyRepo.hasIdentityForBackup()).doSuspendableAnswer { + throw IllegalStateException("keychain unavailable") + } + assertTrue(sut.triggerBackup(BackupCategory.WALLET).isFailure) + verify(vssBackupClient, never()).putObject(eq(BackupCategory.WALLET.name), any()) + } + @Test fun `wallet backup waits for submission and includes the latest pending state`() = test { val statuses = MutableStateFlow( diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 1a5b05c58f..a6d429a882 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -2412,6 +2412,87 @@ class PubkyRepoTest : BaseUnitTest() { assertNull(result.getOrNull()) } + @Test + fun `scheduled restore activation returns while authentication is held and coalesces duplicates`() = test { + sut.awaitInitialization() + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn("restored_session") + val release = CompletableDeferred() + whenever(pubkyService.importSession("restored_session")).doSuspendableAnswer { release.await() } + clearInvocations(pubkyService) + try { + repeat(3) { sut.scheduleInitializationAfterRestore() } + runCurrent() + assertNull(sut.publicKey.value) + assertFalse(sut.isAuthenticated.value) + verify(pubkyService).importSession("restored_session") + } finally { + release.complete(VALID_SELF_KEY) + } + advanceUntilIdle() + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + verify(pubkyService).importSession("restored_session") + } + + @Test + fun `new restored identity proceeds while old scheduled profile loading is held`() = test { + sut.awaitInitialization() + var session: String? = "first_session" + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session } + whenever(keychain.delete(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { session = null } + whenever(pubkyService.importSession("first_session")).thenReturn(VALID_SELF_KEY) + whenever(pubkyService.importSession("second_session")).thenReturn(VALID_CONTACT_KEY_B) + val releaseFirst = CompletableDeferred() + val releaseSecond = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + releaseFirst.await() + createResolution(VALID_SELF_KEY, pubkyProfile = createPubkyProfile(name = "Obsolete")) + } + whenever(pubkyService.resolveContactProfile(VALID_CONTACT_KEY_B, true)).doSuspendableAnswer { + releaseSecond.await() + createResolution(VALID_CONTACT_KEY_B, pubkyProfile = createPubkyProfile(name = "Current")) + } + try { + sut.scheduleInitializationAfterRestore() + runCurrent() + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + sut.restoreSessionBackupState(null).getOrThrow() + assertNull(sut.publicKey.value) + session = "second_session" + sut.scheduleInitializationAfterRestore() + runCurrent() + assertEquals(VALID_CONTACT_KEY_B, sut.publicKey.value) + releaseFirst.complete(Unit) + runCurrent() + sut.scheduleInitializationAfterRestore() + runCurrent() + verify(pubkyService).importSession("second_session") + } finally { + releaseFirst.complete(Unit) + releaseSecond.complete(Unit) + } + advanceUntilIdle() + assertEquals(VALID_CONTACT_KEY_B, sut.publicKey.value) + assertEquals("Current", sut.profile.value?.name) + } + + @Test + fun `backup identity includes persisted and adopted credentials and fails on unreadable storage`() = test { + sut.awaitInitialization() + assertFalse(sut.hasIdentityForBackup()) + val identityKeys = listOf( + Keychain.Key.PAYKIT_SESSION, + Keychain.Key.PUBKY_SECRET_KEY, + Keychain.Key.SHARED_PUBKY_SOURCE, + ) + for (key in identityKeys) { + whenever(keychain.loadString(key.name)).thenReturn("stored") + assertTrue(sut.hasIdentityForBackup()) + whenever(keychain.loadString(key.name)).thenReturn(null) + } + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenAnswer { throw TestAppError("unreadable") } + assertTrue(runSuspendCatching { sut.hasIdentityForBackup() }.isFailure) + } + @Test fun `awaitInitialization shares startup and preserves it when a waiter is cancelled`() = test { val imported = CompletableDeferred() diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 91cdfe0477..2dbdff6abe 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -23,7 +23,6 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify -import org.mockito.kotlin.verifyBlocking import org.mockito.kotlin.whenever import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore @@ -411,12 +410,33 @@ class WalletViewModelTest : BaseUnitTest() { } @Test - fun `post restore profile failure allows proceeding only while safety markers stay clear`() = test { + fun `completed wallet restore does not wait for optional identity activation`() = test { + val release = CompletableDeferred() whenever(backupRepo.performFullRestoreFromLatestBackup(any())).thenReturn(Result.success(Unit)) - whenever(pubkyRepo.initialize()).doSuspendableAnswer { throw AppError("profile cache unavailable") } + whenever(pubkyRepo.initialize()).doSuspendableAnswer { + release.await() + Result.success(Unit) + } + try { + sut.onBackupRestoreRetry() + runCurrent() + assertEquals(RestoreState.Completed, sut.restoreState.value) + verify(pubkyRepo).scheduleInitializationAfterRestore() + verify(pubkyRepo, never()).initialize() + verify(backupRepo).setRestorePending(false) + } finally { + release.complete(Unit) + } + } + + @Test + fun `proceed rechecks safety markers after an optional restore failure`() = test { + whenever(backupRepo.performFullRestoreFromLatestBackup(any())) + .thenReturn(Result.failure(AppError("metadata cache unavailable"))) sut.onBackupRestoreRetry() advanceUntilIdle() assertEquals(RestoreState.BackupFailed(1, hasPendingWalletRestore = false), sut.restoreState.value) + verify(pubkyRepo, never()).scheduleInitializationAfterRestore() whenever(backupRepo.hasPendingWalletRestore()).thenReturn(true) var dismissed = false @@ -589,7 +609,7 @@ class WalletViewModelTest : BaseUnitTest() { walletState.value = walletState.value.copy(walletExists = true) advanceUntilIdle() - verifyBlocking(pubkyRepo) { initialize() } + verify(pubkyRepo).scheduleInitializationAfterRestore() } @Test diff --git a/changelog.d/next/restore-identity.fixed.md b/changelog.d/next/restore-identity.fixed.md new file mode 100644 index 0000000000..7d934b9487 --- /dev/null +++ b/changelog.d/next/restore-identity.fixed.md @@ -0,0 +1 @@ +Wallet restore finishes without waiting for optional profile refresh, while incomplete identity data cannot replace complete backups. diff --git a/journeys/wallet/restore-during-identity-outage.xml b/journeys/wallet/restore-during-identity-outage.xml new file mode 100644 index 0000000000..fcaad5e534 --- /dev/null +++ b/journeys/wallet/restore-during-identity-outage.xml @@ -0,0 +1,18 @@ + + + Use a disposable test-network wallet with a complete VSS backup and a saved Pubky identity. + Required wallet, private reservation and USDT recovery imports must succeed before the injected + delay starts. Delay only final identity activation or subsequent profile/contact reads. + Keep metadata credential replacement and required restore imports available. These complete-app + fault controls are not currently provisioned; component test results are separate evidence. + Never use live funds or reset an existing wallet. + + + Restore the complete backup and hold final identity activation; verify ordinary wallet access resumes after required recovery completes, without reporting Paykit authenticated or payment-ready + Trigger a full backup while the saved identity cannot be exported; verify failure retains the previous complete WALLET record and required pending backup work + Close the restore screen, release the identity fault and verify retained work completes and a fresh complete backup succeeds once + Repeat with delayed profile/contact enrichment, then replace or remove the identity; verify late results cannot republish the old identity or overwrite current profile data + Restart during the outage; verify saved credentials resume recovery without duplicating a payment or contact mutation + Fail a required wallet or USDT restore instead; verify spending and address allocation remain gated and retry resumes the same required recovery + + From ac2a7c6680589bd80e75de1ffb097b44f1cf2f28 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:50:23 +0200 Subject: [PATCH 02/11] docs: associate restore note with PR 1686 --- changelog.d/next/{restore-identity.fixed.md => 1686.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{restore-identity.fixed.md => 1686.fixed.md} (100%) diff --git a/changelog.d/next/restore-identity.fixed.md b/changelog.d/next/1686.fixed.md similarity index 100% rename from changelog.d/next/restore-identity.fixed.md rename to changelog.d/next/1686.fixed.md From d4ba9076b18901721fd9c5a156a0005d16fe6380 Mon Sep 17 00:00:00 2001 From: benk10 Date: Mon, 26 Oct 2026 11:09:53 +0200 Subject: [PATCH 03/11] fix: preserve active identity during scheduled restore setup --- .../java/to/bitkit/repositories/PubkyRepo.kt | 7 ++++- .../to/bitkit/repositories/PubkyRepoTest.kt | 30 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index 8340349b42..41854bbf9a 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -379,7 +379,12 @@ class PubkyRepo @Inject constructor( if (expectedRevision != null && expectedRevision != storedIdentityRevision.get()) { return@withContext Result.success(Unit) } - initializeSession() to storedIdentityRevision.get() + val restored = if (expectedRevision != null && _publicKey.value != null) { + true + } else { + initializeSession() + } + restored to storedIdentityRevision.get() } if (revision != storedIdentityRevision.get()) return@withContext Result.success(Unit) if (restored) loadIdentityData() diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index a6d429a882..937806b02b 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -48,6 +48,8 @@ import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.yield import org.junit.Before import org.junit.Test import org.mockito.Mockito.clearInvocations @@ -2433,6 +2435,34 @@ class PubkyRepoTest : BaseUnitTest() { verify(pubkyService).importSession("restored_session") } + @Test + fun `scheduled metadata refresh preserves an identity already activated by restore`() = test { + sut.awaitInitialization() + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn("restored_session") + whenever(pubkyService.importSession("restored_session")).thenReturn(VALID_SELF_KEY) + assertTrue(sut.restoreSessionIfNeeded()) + val originalSignIn = assertNotNull(sut.currentSignIn()) + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + clearInvocations(pubkyService) + whenever(pubkyService.importSession("restored_session")).doSuspendableAnswer { + throw PaykitException.Transport("transport_error", "controlled unavailable re-import") + } + val refreshed = CompletableDeferred() + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).doSuspendableAnswer { + refreshed.complete(Unit) + createResolution(VALID_SELF_KEY, pubkyProfile = createPubkyProfile(name = "Refreshed")) + } + sut.scheduleInitializationAfterRestore() + yield() + advanceUntilIdle() + assertEquals(VALID_SELF_KEY, sut.publicKey.value) + assertTrue(sut.isCurrent(originalSignIn)) + assertFalse(sut.sessionRestorationFailed.value) + verify(pubkyService, never()).importSession(any()) + withTimeout(1.seconds) { refreshed.await() } + verify(pubkyService).resolveContactProfile(VALID_SELF_KEY, true) + } + @Test fun `new restored identity proceeds while old scheduled profile loading is held`() = test { sut.awaitInitialization() From 7b795fdc057c708ad01196c07f750b94a777508b Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 1 Nov 2026 02:10:32 +0200 Subject: [PATCH 04/11] fix: stop queued allowance offers after deactivation --- .../repositories/PaykitAllowanceRepo.kt | 32 +++-- .../to/bitkit/services/PaykitSdkService.kt | 2 + .../repositories/PaykitAllowanceRepoTest.kt | 109 +++++++++++++----- changelog.d/next/1686.fixed.md | 2 +- 4 files changed, 106 insertions(+), 39 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt index 7fe687eaac..0d3384955f 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt @@ -122,7 +122,11 @@ class PaykitAllowanceRepo @Inject constructor( { limits, monthAnchor, endpoints -> limits.terms(monthAnchor, endpoints) } @Volatile - private var activeIdentity: String? = null + private var activation: Activation? = null + + private val activeIdentity: String? get() = activation?.identity + + private class Activation(val identity: String) private val _entries = MutableStateFlow>(emptyList()) @@ -185,7 +189,7 @@ class PaykitAllowanceRepo @Inject constructor( } withContext(ioDispatcher) { val identityChanged = activeIdentity != normalizedIdentity - activeIdentity = normalizedIdentity + activation = Activation(normalizedIdentity) executor.activate(normalizedIdentity) if (identityChanged) { manualRequestSignatures.clear() @@ -214,7 +218,7 @@ class PaykitAllowanceRepo @Inject constructor( } private fun clear() { - activeIdentity = null + activation = null manualRequestSignatures.clear() _offersToAnnounce.update { emptyList() } announcedOfferKeys.clear() @@ -317,11 +321,22 @@ class PaykitAllowanceRepo @Inject constructor( } } - suspend fun accept(entryId: String): Result = respond(entry(entryId)) { acceptProposal(it) } + suspend fun accept(entryId: String): Result { + val acceptingActivation = activation + return respond(entry(entryId)) { acceptProposal(it, acceptingActivation) } + } - private suspend fun acceptProposal(allowance: PaykitAllowance): AllowanceRecord { + private suspend fun acceptProposal( + allowance: PaykitAllowance, + acceptingActivation: Activation?, + canContinue: () -> Boolean = { true }, + ): AllowanceRecord { val proposalStreamItemId = allowance.proposalStreamItemId ?: throw PaykitAllowanceError.Unavailable - return paykitSdkService.acceptAllowance(allowance.counterparty, allowance.allowanceId, proposalStreamItemId) + return paykitSdkService.acceptAllowance(allowance.counterparty, allowance.allowanceId, proposalStreamItemId) { + if (acceptingActivation == null || activation !== acceptingActivation || !canContinue()) { + throw PaykitAllowanceError.Unavailable + } + } } /** @@ -336,7 +351,8 @@ class PaykitAllowanceRepo @Inject constructor( canAnnounce: () -> Boolean = { true }, canContinue: () -> Boolean = { true }, ): List = withContext(ioDispatcher) { - val identity = activeIdentity ?: return@withContext emptyList() + val acceptingActivation = activation ?: return@withContext emptyList() + val identity = acceptingActivation.identity val isCurrent = { activeIdentity == identity && canAnnounce() } val offers = _entries.value.filter { entry -> val answerable = entry.allowances.filter { it.isAnswerable } @@ -359,7 +375,7 @@ class PaykitAllowanceRepo @Inject constructor( }.onFailure { Logger.warn("Failed to mark an allowance offer before accepting it", it, context = TAG) } .isSuccess if (isMarked) { - respond(offer) { acceptProposal(it) } + respond(offer) { acceptProposal(it, acceptingActivation, canContinue) } .onSuccess { accepted += offer } .onFailure { Logger.warn("Failed to accept an allowance offer", 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 314ae4ef7f..3ac3016d1a 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -1321,10 +1321,12 @@ class PaykitSdkService @Inject constructor( counterparty: String, allowanceId: String, expectedProposalStreamItemId: ULong, + admit: suspend () -> Unit, ): AllowanceRecord { isSetup.await() return operationLock.withLock(Priority.Interactive) { withStateRevisionTracking { handle -> + admit() completeSdkCall { handle.acceptAllowance(counterparty, allowanceId, expectedProposalStreamItemId) } } } diff --git a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt index f796489cfa..d080b226c2 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt @@ -9,6 +9,8 @@ import com.synonym.paykit.LinkedPeerState import com.synonym.paykit.OutboundPrivateSendReport import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update @@ -334,7 +336,10 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { fun `received ask is presented once and accepting answers it`() = test { val proposalRecord = receivedAsk() records = listOf(proposalRecord) - whenever(sdk.acceptAllowance(any(), any(), any())).thenReturn(proposalRecord) + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() + proposalRecord + } sut.activate(identity) val proposal = checkNotNull(sut.proposalForPresentation()) @@ -345,16 +350,45 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.accept(proposal.id).isSuccess) - verify(sdk).acceptAllowance(fixtures.counterpartyKey, ALLOWANCE_ID, 1uL) + verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq(ALLOWANCE_ID), eq(1uL), any()) verify(sdk).processOutboundPrivateMessages(fixtures.counterpartyKey) } + @Test + fun `paused repository does not accept a queued offer`() = test { + for (reactivate in listOf(false, true)) { + records = listOf(receivedProposal()) + val waiting = CompletableDeferred() + val release = CompletableDeferred() + var accepted = 0 + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + waiting.complete(Unit) + release.await() + it.getArgument Unit>(3).invoke() + accepted += 1 + records.single() + } + sut.activate(identity) + val accepting = async { sut.acceptOffersFromAllowers() } + waiting.await() + + sut.pause(identity) + assertTrue(sut.entries.value.isEmpty()) + if (reactivate) sut.activate(identity) + release.complete(Unit) + assertTrue(accepting.await().isEmpty()) + + assertEquals(0, accepted, "A queued offer stays unanswered while allowances are disabled") + } + } + @Test fun `offer from the allower is accepted without the review sheet and not announced twice`() = test { // The proposer took the allower role, so this wallet is the allowee. val offer = receivedProposal() records = listOf(offer) - whenever(sdk.acceptAllowance(any(), any(), any())).thenAnswer { + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() records = listOf(fixtures.record(localRole = AllowanceLocalRole.ALLOWEE, proposedByMe = false)) records.single() } @@ -364,7 +398,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { val accepted = sut.acceptOffersFromAllowers() assertEquals(listOf(fixtures.counterpartyKey), accepted.map { it.counterparty }) - verify(sdk).acceptAllowance(fixtures.counterpartyKey, ALLOWANCE_ID, 1uL) + verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq(ALLOWANCE_ID), eq(1uL), any()) verify(sdk).processOutboundPrivateMessages(fixtures.counterpartyKey) val entry = sut.entries.value.single() assertEquals(PaykitAllowance.Status.ACTIVE, entry.status(fixtures.now)) @@ -374,13 +408,13 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { sut.markOffersAnnounced(accepted) assertTrue(sut.acceptOffersFromAllowers().isEmpty(), "An accepted offer is not accepted or announced twice") - verify(sdk, times(1)).acceptAllowance(any(), any(), any()) + verify(sdk, times(1)).acceptAllowance(any(), any(), any(), any()) } @Test fun `failed offer accept stays unanswered and is retried and announced once on the next refresh`() = test { records = listOf(receivedProposal()) - whenever(sdk.acceptAllowance(any(), any(), any())).thenThrow(RuntimeException("offline")) + whenever(sdk.acceptAllowance(any(), any(), any(), any())).thenThrow(RuntimeException("offline")) sut.activate(identity) assertTrue(sut.acceptOffersFromAllowers().isEmpty(), "A failed accept is not announced") @@ -392,7 +426,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertEquals(listOf(fixtures.counterpartyKey), accepted.map { it.counterparty }) sut.markOffersAnnounced(accepted) assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk, times(2)).acceptAllowance(any(), any(), any()) + verify(sdk, times(2)).acceptAllowance(any(), any(), any(), any()) } @Test @@ -404,7 +438,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers(canAnnounce = { false }).isEmpty()) assertTrue(sut.acceptOffersFromAllowers(canContinue = { false }).isEmpty()) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) } @Test @@ -417,20 +451,26 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { proposedByMe = false, ) records = listOf(receivedProposal(), secondOffer) - whenever(sdk.acceptAllowance(any(), any(), any())).thenAnswer { records.first() } + var isActive = true + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() + records = listOf(fixtures.record(localRole = AllowanceLocalRole.ALLOWEE, proposedByMe = false), secondOffer) + isActive = false + records.first() + } sut.activate(identity) - var checks = 0 - val announced = sut.acceptOffersFromAllowers(canContinue = { ++checks == 1 }) + val announced = sut.acceptOffersFromAllowers(canContinue = { isActive }) assertEquals(1, announced.size, "An offer already accepted is announced, since it is never announced later") - verify(sdk, times(1)).acceptAllowance(any(), any(), any()) + verify(sdk, times(1)).acceptAllowance(any(), any(), any(), any()) } @Test fun `offer whose accept committed as its refresh was cancelled is announced once on the next refresh`() = test { records = listOf(receivedProposal()) - whenever(sdk.acceptAllowance(any(), any(), any())).thenAnswer { + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() records = listOf(fixtures.record(localRole = AllowanceLocalRole.ALLOWEE, proposedByMe = false)) throw CancellationException("The app left the foreground") } @@ -449,7 +489,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers().isEmpty()) assertTrue(sut.acceptOffersFromAllowers().isEmpty(), "It is announced once") assertTrue(localState.acceptingOfferIds.isEmpty()) - verify(sdk, times(1)).acceptAllowance(any(), any(), any()) + verify(sdk, times(1)).acceptAllowance(any(), any(), any(), any()) } @Test @@ -503,7 +543,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) } @Test @@ -516,7 +556,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { val announced = sut.acceptOffersFromAllowers(canAnnounce = { ++checks == 1 }) assertTrue(announced.isEmpty(), "An accept that outlived its session is not announced") - verify(sdk).acceptAllowance(fixtures.counterpartyKey, ALLOWANCE_ID, 1uL) + verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq(ALLOWANCE_ID), eq(1uL), any()) } @Test @@ -539,7 +579,8 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { ), offerX, ) - whenever(sdk.acceptAllowance(any(), any(), any())).thenAnswer { + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() if (it.getArgument(1) == "Y") { records = listOf( fixtures.record( @@ -564,21 +605,22 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { sut.acceptOffersFromAllowers() - verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq("X"), any()) - verify(sdk, never()).acceptAllowance(eq(asker), eq("X"), any()) + verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq("X"), any(), any()) + verify(sdk, never()).acceptAllowance(eq(asker), eq("X"), any(), any()) } @Test fun `offer accepted after the identity changed is not announced`() = test { records = listOf(receivedProposal()) - whenever(sdk.acceptAllowance(any(), any(), any())).thenAnswer { + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() sut.deactivate() records.single() } sut.activate(identity) assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk).acceptAllowance(fixtures.counterpartyKey, ALLOWANCE_ID, 1uL) + verify(sdk).acceptAllowance(eq(fixtures.counterpartyKey), eq(ALLOWANCE_ID), eq(1uL), any()) } @Test @@ -589,7 +631,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) assertEquals(entryKey, sut.proposalForPresentation()?.id) assertEquals(PaykitAllowance.Status.AWAITING_MY_ANSWER, sut.entries.value.single().status(fixtures.now)) } @@ -621,7 +663,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) } @Test @@ -630,7 +672,7 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertTrue(sut.acceptOffersFromAllowers().isEmpty()) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) } @Test @@ -644,7 +686,10 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { lastEventAt = "2026-09-01T10:00:00Z", ) records = listOf(offer, ask) - whenever(sdk.acceptAllowance(any(), any(), any())).thenReturn(ask) + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() + ask + } sut.activate(identity) // The offer is accepted automatically, so the sheet presents the other contact's ask. @@ -653,8 +698,8 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { assertEquals(fixtures.otherCounterpartyKey, presented.counterparty) assertTrue(sut.accept(presented.id).isSuccess) - verify(sdk).acceptAllowance(fixtures.otherCounterpartyKey, ALLOWANCE_ID, 1uL) - verify(sdk, never()).acceptAllowance(eq(fixtures.counterpartyKey), any(), any()) + verify(sdk).acceptAllowance(eq(fixtures.otherCounterpartyKey), eq(ALLOWANCE_ID), eq(1uL), any()) + verify(sdk, never()).acceptAllowance(eq(fixtures.counterpartyKey), any(), any(), any()) } @Test @@ -666,12 +711,15 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { terms = fixtures.terms(perPaymentMaximum = null, monthlyLimit = null, expiresAt = "2027-09-24T12:00:00Z"), ) records = listOf(expiryOnly) - whenever(sdk.acceptAllowance(any(), any(), any())).thenReturn(expiryOnly) + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + it.getArgument Unit>(3).invoke() + expiryOnly + } sut.activate(identity) assertNull(sut.proposalForPresentation()) assertTrue(sut.accept(entryKey).isFailure) - verify(sdk, never()).acceptAllowance(any(), any(), any()) + verify(sdk, never()).acceptAllowance(any(), any(), any(), any()) } @Test @@ -905,7 +953,8 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { ) private fun acceptFlipsRecordToActive() { - whenever { sdk.acceptAllowance(any(), any(), any()) }.thenAnswer { + whenever { sdk.acceptAllowance(any(), any(), any(), any()) }.doSuspendableAnswer { + it.getArgument Unit>(3).invoke() records = listOf(fixtures.record(localRole = AllowanceLocalRole.ALLOWEE, proposedByMe = false)) records.single() } diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index 0bb5e1f700..95cf1e5b08 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without waiting for optional profile refresh, retries missing profiles after reconnecting, and protects complete backups from incomplete identity data. +Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting, while complete backups reject incomplete identity data and queued allowance offers respect disabled allowances. From 78269d8a1cae1ac127be93a96fba16f7d68d1019 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 1 Nov 2026 06:05:32 +0200 Subject: [PATCH 05/11] Preserve active allowance ownership during contact refresh --- .../repositories/PaykitAllowanceRepo.kt | 2 +- .../repositories/PaykitAllowanceRepoTest.kt | 52 +++++++++++++++++++ changelog.d/next/1686.fixed.md | 2 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt index 0d3384955f..80c5b0311a 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt @@ -189,7 +189,7 @@ class PaykitAllowanceRepo @Inject constructor( } withContext(ioDispatcher) { val identityChanged = activeIdentity != normalizedIdentity - activation = Activation(normalizedIdentity) + if (identityChanged) activation = Activation(normalizedIdentity) executor.activate(normalizedIdentity) if (identityChanged) { manualRequestSignatures.clear() diff --git a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt index d080b226c2..bd89682e41 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt @@ -354,6 +354,58 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { verify(sdk).processOutboundPrivateMessages(fixtures.counterpartyKey) } + @Test + fun `same identity activation keeps a queued manual acceptance valid`() = test { + records = listOf(receivedAsk()) + val waiting = CompletableDeferred() + val release = CompletableDeferred() + var accepted = 0 + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + waiting.complete(Unit) + release.await() + it.getArgument Unit>(3).invoke() + accepted += 1 + records.single() + } + sut.activate(identity) + val accepting = async { sut.accept(entryKey) } + waiting.await() + + sut.activate(identity) + release.complete(Unit) + + assertTrue(accepting.await().isSuccess) + assertEquals(1, accepted) + verify(sdk, times(1)).acceptAllowance(any(), any(), any(), any()) + verify(sdk).processOutboundPrivateMessages(fixtures.counterpartyKey) + } + + @Test + fun `same identity activation keeps a queued offer acceptance valid`() = test { + records = listOf(receivedProposal()) + val waiting = CompletableDeferred() + val release = CompletableDeferred() + var accepted = 0 + whenever(sdk.acceptAllowance(any(), any(), any(), any())).doSuspendableAnswer { + waiting.complete(Unit) + release.await() + it.getArgument Unit>(3).invoke() + accepted += 1 + records.single() + } + sut.activate(identity) + val accepting = async { sut.acceptOffersFromAllowers() } + waiting.await() + + sut.activate(identity) + release.complete(Unit) + + assertEquals(listOf(fixtures.counterpartyKey), accepting.await().map { it.counterparty }) + assertEquals(1, accepted) + verify(sdk, times(1)).acceptAllowance(any(), any(), any(), any()) + verify(sdk).processOutboundPrivateMessages(fixtures.counterpartyKey) + } + @Test fun `paused repository does not accept a queued offer`() = test { for (reactivate in listOf(false, true)) { diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index 95cf1e5b08..c42042a9a7 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting, while complete backups reject incomplete identity data and queued allowance offers respect disabled allowances. +Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting, while complete backups reject incomplete identity data and queued allowance acceptance survives contact sync but respects disabled allowances. From 1b1426fbebeccd616c29311fa5bf652089727a84 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 1 Nov 2026 22:32:12 +0100 Subject: [PATCH 06/11] fix: preserve pending profile setup and revoke stale activation --- .../repositories/PaykitAllowanceRepo.kt | 30 ++++++++--- .../java/to/bitkit/repositories/PubkyRepo.kt | 6 ++- .../to/bitkit/services/PaykitSdkService.kt | 3 +- .../java/to/bitkit/viewmodels/AppViewModel.kt | 5 +- .../repositories/PaykitAllowanceRepoTest.kt | 50 +++++++++++++++++ .../to/bitkit/repositories/PubkyRepoTest.kt | 33 ++++++++++++ .../bitkit/services/PaykitSdkServiceTest.kt | 23 ++++++++ .../viewmodels/AppViewModelSendFlowTest.kt | 54 ++++++++++++++++++- changelog.d/next/1686.fixed.md | 2 +- 9 files changed, 192 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt index 80c5b0311a..87872e8d4a 100644 --- a/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PaykitAllowanceRepo.kt @@ -105,6 +105,8 @@ class PaykitAllowanceRepo @Inject constructor( private val scope = appScope(ioDispatcher, TAG) private val refreshMutex = Mutex() private val publishLock = Any() + private val activationLock = Any() + private var activationRevision = Any() private val isProcessingRequests = AtomicBoolean(false) private val isAcceptingOffers = AtomicBoolean(false) private val updates = MutableStateFlow(0) @@ -181,16 +183,22 @@ class PaykitAllowanceRepo @Inject constructor( // region Lifecycle - suspend fun activate(identity: String?) { + suspend fun activate(identity: String?, canActivate: suspend () -> Boolean = { true }) { val normalizedIdentity = identity?.let(PubkyPublicKeyFormat::normalized) if (normalizedIdentity == null) { deactivate() return } + val revision = synchronized(activationLock) { activationRevision } withContext(ioDispatcher) { - val identityChanged = activeIdentity != normalizedIdentity - if (identityChanged) activation = Activation(normalizedIdentity) - executor.activate(normalizedIdentity) + if (!canActivate()) return@withContext + val identityChanged = synchronized(activationLock) { + if (activationRevision !== revision) return@withContext + val changed = activeIdentity != normalizedIdentity + if (changed) activation = Activation(normalizedIdentity) + executor.activate(normalizedIdentity) + changed + } if (identityChanged) { manualRequestSignatures.clear() _offersToAnnounce.update { emptyList() } @@ -202,8 +210,11 @@ class PaykitAllowanceRepo @Inject constructor( } fun deactivate() { - executor.activate(null) - clear() + synchronized(activationLock) { + activationRevision = Any() + executor.activate(null) + clear() + } } /** @@ -211,8 +222,11 @@ class PaykitAllowanceRepo @Inject constructor( * the executor settle attempts left open before allowances were turned off; null keeps its current one. */ suspend fun pause(identity: String? = null) { - executor.stopAdmitting() - clear() + synchronized(activationLock) { + activationRevision = Any() + executor.stopAdmitting() + clear() + } val normalizedIdentity = identity?.let(PubkyPublicKeyFormat::normalized) ?: return withContext(ioDispatcher) { executor.pause(normalizedIdentity) } } diff --git a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt index e722c3dd25..0de7df3737 100644 --- a/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/PubkyRepo.kt @@ -399,7 +399,11 @@ class PubkyRepo @Inject constructor( loadIdentityData() } else { currentSignIn()?.takeIf { _profile.value == null }?.let { signIn -> - scope.launch { loadProfile(signIn, onlyIfMissing = true) } + scope.launch { + if (!settingsStore.isPubkyProfileSetupPending.first()) { + loadProfile(signIn, onlyIfMissing = true) + } + } } } restored diff --git a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt index 3ac3016d1a..b7581de454 100644 --- a/app/src/main/java/to/bitkit/services/PaykitSdkService.kt +++ b/app/src/main/java/to/bitkit/services/PaykitSdkService.kt @@ -1325,8 +1325,7 @@ class PaykitSdkService @Inject constructor( ): AllowanceRecord { isSetup.await() return operationLock.withLock(Priority.Interactive) { - withStateRevisionTracking { handle -> - admit() + withStateRevisionTracking(beforeOperation = admit) { handle -> completeSdkCall { handle.acceptAllowance(counterparty, allowanceId, expectedProposalStreamItemId) } } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 95bcf05879..965e6d7f15 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -991,7 +991,10 @@ class AppViewModel @Inject constructor( isPaymentRequestIdentityActivating = true val requestRefresh = try { paykitPaymentRequestRepo.activate(state.publicKey) - if (state.areAllowancesEnabled) paykitAllowanceRepo.activate(state.publicKey) + if (!isStillCurrent()) return + if (state.areAllowancesEnabled && areAllowancesEnabled()) { + paykitAllowanceRepo.activate(state.publicKey) { areAllowancesEnabled() && isStillCurrent() } + } // Allowances may have turned off while this activated, before the activation took effect. if (!state.areAllowancesEnabled || !areAllowancesEnabled()) paykitAllowanceRepo.pause(state.publicKey) if (!pubkyRepo.isCurrent(signIn)) return diff --git a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt index bd89682e41..1c90c987ab 100644 --- a/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PaykitAllowanceRepoTest.kt @@ -10,10 +10,12 @@ import com.synonym.paykit.OutboundPrivateSendReport import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.async import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update +import kotlinx.coroutines.test.StandardTestDispatcher import org.junit.After import org.junit.Assume.assumeTrue import org.junit.Before @@ -117,6 +119,54 @@ class PaykitAllowanceRepoTest : BaseUnitTest() { // region Entries + @Test + fun `pause revokes an activation queued on the dispatcher`() = test { + val queued = PaykitAllowanceRepo( + ioDispatcher = StandardTestDispatcher(testScheduler), + paykitSdkService = sdk, + executor = executor, + lightningRepo = lightningRepo, + activityRepo = activityRepo, + clock = clock, + ) + val activation = async(start = CoroutineStart.UNDISPATCHED) { queued.activate(identity) } + + queued.pause() + activation.await() + + verify(executor, never()).activate(identity) + } + + @Test + fun `pause revokes activation while its caller admission is suspended`() = test { + val admissionStarted = CompletableDeferred() + val finishAdmission = CompletableDeferred() + val activation = async { + sut.activate(identity) { + admissionStarted.complete(Unit) + finishAdmission.await() + true + } + } + admissionStarted.await() + sut.pause() + finishAdmission.complete(Unit) + activation.await() + + verify(executor, never()).activate(identity) + sut.activate(identity) + verify(executor).activate(identity) + } + + @Test + fun `rejected activation does not recover or refresh allowances`() = test { + sut.activate(identity) { false } + + verify(executor, never()).activate(identity) + verify(executor, never()).recover(any()) + verify(sdk, never()).listAllowances(any()) + } + @Test fun `entries list one grant per contact with the grant's limits`() = test { records = listOf( diff --git a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt index 81a7f15999..3ebe00ec95 100644 --- a/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/PubkyRepoTest.kt @@ -2461,6 +2461,39 @@ class PubkyRepoTest : BaseUnitTest() { verify(pubkyService).resolveContactProfile(VALID_SELF_KEY, true) } + @Test + fun `reconnect skips expected missing profile until local setup completes`() = test { + sut.awaitInitialization() + whenever(keychain.loadString(Keychain.Key.PAYKIT_SESSION.name)).thenReturn("restored_session") + whenever(pubkyService.importSession("restored_session")).thenReturn(VALID_SELF_KEY) + whenever(pubkyService.resolveContactProfile(VALID_SELF_KEY, true)).thenAnswer { + throw PaykitException.Transport("transport_error", "offline") + } + assertTrue(sut.restoreSessionIfNeeded()) + val originalSignIn = assertNotNull(sut.currentSignIn()) + profileSetupPending.value = true + clearInvocations(pubkyService) + + repeat(3) { + assertFalse(sut.restoreSessionIfNeeded()) + yield() + advanceUntilIdle() + } + + assertTrue(sut.isCurrent(originalSignIn)) + verify(pubkyService, never()).resolveContactProfile(VALID_SELF_KEY, true) + verify(pubkyService, never()).importSession(any()) + profileSetupPending.value = false + doSuspendableAnswer { + createResolution(VALID_SELF_KEY, pubkyProfile = createPubkyProfile(name = "Completed")) + }.whenever(pubkyService).resolveContactProfile(VALID_SELF_KEY, true) + assertFalse(sut.restoreSessionIfNeeded()) + yield() + advanceUntilIdle() + assertEquals("Completed", sut.profile.value?.name) + verify(pubkyService).resolveContactProfile(VALID_SELF_KEY, true) + } + @Test fun `active identity retries its missing profile on reconnect without delaying wallet maintenance`() = test { sut.awaitInitialization() diff --git a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt index 2c3cc12ac6..7251792372 100644 --- a/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt +++ b/app/src/test/java/to/bitkit/services/PaykitSdkServiceTest.kt @@ -1804,6 +1804,29 @@ class PaykitSdkServiceTest { verify(sdk, never()).processOutboundPrivateMessages(any()) } + @Test + fun `rejected allowance admission preserves cached backup and version`() = runTest { + val sdk = mock() + whenever(sdk.stateRevision()).thenReturn("state") + whenever(sdk.observedBackupStateRevision()).thenReturn(ObservedBackupStateRevision("state", "backup")) + whenever { sdk.backupStateRevision() }.thenReturn("backup") + whenever { sdk.processPendingPrivateMessages() }.thenReturn(emptyList()) + val service = PaykitSdkService(mock(), mock(), mock(), settingsStore = mock()) { sdk } + service.processPendingPrivateMessages() + val originalVersion = service.backupStateVersion.value + val failure = AppError("Admission revoked") + + val thrown = assertFailsWith { + service.acceptAllowance(RING_PUBKY, "allowance", 1uL) { throw failure } + } + + assertSame(failure, thrown) + verify(sdk, never()).acceptAllowance(any(), any(), any()) + assertEquals(originalVersion, service.backupStateVersion.value) + service.processPendingPrivateMessages() + verify(sdk, times(1)).backupStateRevision() + } + @Test fun `cancelling active acceptance waits for its durable call and preserves backup notification`() = 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 89de855cf8..6d62184ce3 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -326,6 +326,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { } private val isRecoveryMode = MutableStateFlow(false) private val isPaykitEnabled = MutableStateFlow(false) + private val areAllowancesEnabled = MutableStateFlow(true) private val walletState = MutableStateFlow(WalletState()) private val nodeEventUpdates = MutableSharedFlow() private val pubkyPublicKey = MutableStateFlow(null) @@ -904,7 +905,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { private fun stubSettingsStore() { whenever(settingsStore.data).thenReturn(settingsData) whenever(settingsStore.isPaykitEnabled).thenReturn(isPaykitEnabled) - whenever(settingsStore.areAllowancesEnabled).thenReturn(flowOf(true)) + whenever(settingsStore.areAllowancesEnabled).thenReturn(areAllowancesEnabled) whenever { settingsStore.update(any()) }.thenAnswer { val transform = it.getArgument<(SettingsData) -> SettingsData>(0) settingsData.value = transform(settingsData.value) @@ -3274,6 +3275,57 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals("PaymentRequestUnavailableToast", terminalToast.testTag) } + @Test + fun `contact activation cannot reopen allowances after switching off`() = test { + val activationStarted = CompletableDeferred() + val finishActivation = CompletableDeferred() + whenever(paykitPaymentRequestRepo.activate(testPublicKey)).doSuspendableAnswer { + activationStarted.complete(Unit) + finishActivation.await() + } + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + activationStarted.await() + + areAllowancesEnabled.value = false + runCurrent() + verify(paykitAllowanceRepo).pause() + clearInvocations(paykitAllowanceRepo) + + finishActivation.complete(Unit) + runCurrent() + + verify(paykitAllowanceRepo, never()).activate(anyOrNull(), any()) + } + + @Test + fun `allowance admission rechecks deletion after the settings read`() = test { + val settingsReadStarted = CompletableDeferred() + val finishSettingsRead = CompletableDeferred() + var admitted: Boolean? = null + whenever { paykitAllowanceRepo.activate(anyOrNull(), any()) }.doSuspendableAnswer { + val admission = it.getArgument Boolean>(1) + whenever(settingsStore.areAllowancesEnabled).thenReturn( + flow { + settingsReadStarted.complete(Unit) + finishSettingsRead.await() + emit(true) + } + ) + admitted = admission() + } + pubkyPublicKey.value = testPublicKey + isPaykitEnabled.value = true + pubkyContactsLoadVersion.value = 1L + runCurrent() + settingsReadStarted.await() + isProfileDeletionInProgress.value = true + finishSettingsRead.complete(Unit) + runCurrent() + + assertEquals(false, admitted) + } + @Test fun `latest identity activation blocks retries from the previous payment request queue`() = test { val request = paymentRequest() diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index c42042a9a7..23f68f489c 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting, while complete backups reject incomplete identity data and queued allowance acceptance survives contact sync but respects disabled allowances. +Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting without reloading profiles still being set up, while complete backups reject incomplete identity data and queued allowance acceptance survives contact sync but respects disabled allowances and profile removal. From 181d664c38a40df156cad4388168b5ab2df132c4 Mon Sep 17 00:00:00 2001 From: benk10 Date: Mon, 2 Nov 2026 00:04:31 +0100 Subject: [PATCH 07/11] fix: resume allowances after a paused activation --- .../java/to/bitkit/viewmodels/AppViewModel.kt | 18 +++++- .../viewmodels/AppViewModelSendFlowTest.kt | 55 +++++++++++++++++++ changelog.d/next/1686.fixed.md | 2 +- journeys/allowances/allowances-turned-off.xml | 4 +- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 965e6d7f15..ed33346ba1 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -576,6 +576,7 @@ class AppViewModel @Inject constructor( private var isCompletingRestoreHold = false private var addressValidationJob: Job? = null private var lastPrivatePaykitContactKeys: Set = emptySet() + private val allowancePauseRevision = MutableStateFlow(0L) private val isPaykitEnabled = settingsStore.isPaykitEnabled .map { PaykitFeatureFlags.isUiEnabled(it) } .stateIn(viewModelScope, SharingStarted.Eagerly, false) @@ -932,15 +933,20 @@ class AppViewModel @Inject constructor( combine( settingsStore.isPaykitEnabled.map { PaykitFeatureFlags.isUiEnabled(it) }, settingsStore.areAllowancesEnabled, - ) { isPaykitEnabled, areAllowancesEnabled -> isPaykitEnabled to areAllowancesEnabled }, + allowancePauseRevision, + ) { isPaykitEnabled, areAllowancesEnabled, pauseRevision -> + Triple(isPaykitEnabled, areAllowancesEnabled, pauseRevision) + }, privatePaykitRepo.isProfileDeletionInProgress, - ) { publicKey, contacts, contactsLoadVersion, (isPaykitEnabled, areAllowancesEnabled), deleting -> + ) { publicKey, contacts, contactsLoadVersion, allowanceSettings, deleting -> + val (isPaykitEnabled, areAllowancesEnabled, pauseRevision) = allowanceSettings PaykitContactSyncState( publicKey = publicKey, contactKeys = contacts.map { it.publicKey }.toSet(), contactsLoaded = contactsLoadVersion > 0L, isPaykitEnabled = isPaykitEnabled, areAllowancesEnabled = areAllowancesEnabled, + allowancePauseRevision = pauseRevision, ).takeUnless { deleting } } .distinctUntilChanged() @@ -952,7 +958,11 @@ class AppViewModel @Inject constructor( settingsStore.areAllowancesEnabled .distinctUntilChanged() .filter { !it } - .collect { paykitAllowanceRepo.pause() } + .collect { + paykitAllowanceRepo.pause() + // Off/on can coalesce while contact sync waits; revoked activation still needs a fresh sync. + allowancePauseRevision.update { it + 1 } + } } } @@ -1019,6 +1029,7 @@ class AppViewModel @Inject constructor( private fun isCurrentPaykitContactSync(state: PaykitContactSyncState, signIn: PubkySignIn): Boolean = pubkyRepo.isCurrent(signIn) && isPaykitEnabled.value && !privatePaykitRepo.isProfileDeletionInProgress.value && + state.allowancePauseRevision == allowancePauseRevision.value && (pubkyRepo.contactsLoadVersion.value > 0L) == state.contactsLoaded && pubkyRepo.contacts.value.map { it.publicKey }.toSet() == state.contactKeys @@ -9054,6 +9065,7 @@ private data class PaykitContactSyncState( val contactsLoaded: Boolean, val isPaykitEnabled: Boolean, val areAllowancesEnabled: Boolean, + val allowancePauseRevision: Long, ) sealed class SendEffect { diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 6d62184ce3..c57be8d4c0 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -138,6 +138,7 @@ import to.bitkit.models.PaykitAmount import to.bitkit.models.PaykitAsset import to.bitkit.models.PaykitRequestPricing import to.bitkit.models.PubkyProfile +import to.bitkit.models.PubkyPublicKeyFormat import to.bitkit.models.SamRockPaymentMethod import to.bitkit.models.SamRockSetupRequest import to.bitkit.models.SendFailureDetails @@ -170,6 +171,8 @@ import to.bitkit.repositories.OnchainSendEvidence import to.bitkit.repositories.OnchainSendNotDispatchedError import to.bitkit.repositories.OnchainSendOutcome import to.bitkit.repositories.PaykitAllowanceEvent +import to.bitkit.repositories.PaykitAllowanceExecutor +import to.bitkit.repositories.PaykitAllowanceLocalState import to.bitkit.repositories.PaykitAllowanceRepo import to.bitkit.repositories.PaykitBillingPeriod import to.bitkit.repositories.PaykitOnchainPaymentProofResolution @@ -3298,6 +3301,58 @@ class AppViewModelSendFlowTest : BaseUnitTest() { verify(paykitAllowanceRepo, never()).activate(anyOrNull(), any()) } + @Test + fun `off on during allowance admission activates the latest enabled state`() = test { + val sdk = mock() + val executor = mock() + whenever(executor.events).thenReturn(MutableSharedFlow()) + whenever(executor.localState(any())).thenReturn(PaykitAllowanceLocalState()) + whenever(sdk.listAllowances(any())).thenReturn(emptyList()) + val realAllowances = PaykitAllowanceRepo( + ioDispatcher = testDispatcher, + paykitSdkService = sdk, + executor = executor, + lightningRepo = lightningRepo, + activityRepo = activityRepo, + clock = Clock.System, + ) + val activationStarted = CompletableDeferred() + val finishActivation = CompletableDeferred() + var holdNextAdmission = true + whenever { paykitAllowanceRepo.activate(anyOrNull(), any()) }.doSuspendableAnswer { + val identity = it.getArgument(0) + val admission = it.getArgument Boolean>(1) + realAllowances.activate(identity) { + if (holdNextAdmission) { + holdNextAdmission = false + activationStarted.complete(Unit) + finishActivation.await() + } + admission() + } + } + whenever { paykitAllowanceRepo.pause(anyOrNull()) }.doSuspendableAnswer { + realAllowances.pause(it.getArgument(0)) + } + enablePaykitUi() + pubkyPublicKey.value = testPublicKey + activationStarted.await() + + areAllowancesEnabled.value = false + runCurrent() + verify(executor).stopAdmitting() + areAllowancesEnabled.value = true + runCurrent() + verify(executor, never()).activate(any()) + + finishActivation.complete(Unit) + runCurrent() + + val identity = requireNotNull(PubkyPublicKeyFormat.normalized(testPublicKey)) + verify(executor).activate(identity) + verify(executor).recover(identity) + } + @Test fun `allowance admission rechecks deletion after the settings read`() = test { val settingsReadStarted = CompletableDeferred() diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index 23f68f489c..7a9d63b22e 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without optional profile refresh waits and retries missing profiles after reconnecting without reloading profiles still being set up, while complete backups reject incomplete identity data and queued allowance acceptance survives contact sync but respects disabled allowances and profile removal. +Wallet restore finishes without optional profile refresh waits, missing profiles retry after reconnecting without interrupting active setup, incomplete identity backups remain blocked, and allowances resume safely after re-enabling without reviving cancelled work. diff --git a/journeys/allowances/allowances-turned-off.xml b/journeys/allowances/allowances-turned-off.xml index 59af61c2e7..12b8acabc7 100644 --- a/journeys/allowances/allowances-turned-off.xml +++ b/journeys/allowances/allowances-turned-off.xml @@ -2,7 +2,9 @@ Allowances can be turned off from Dev Settings. With them off the Subscriptions screen shows only the Overview and Payments tabs, and a request covered by an Active allowance is not paid - automatically: it opens as an ordinary Payment Request. Turning them back on shows the tab again. + automatically: it opens as an ordinary Payment Request. Turning them back on restores both the tab + and its Active allowance row. On Android, repeat the off/on sequence while contact loading is + delayed, then let it finish: the latest enabled state must restore the row without restarting the app. Launch the E2E Bitkit app on both instances with an Active allowance, per set-and-accept.xml From b6751e22956653c4f2a80112143880b47298e30d Mon Sep 17 00:00:00 2001 From: benk10 Date: Sat, 31 Oct 2026 00:34:36 +0200 Subject: [PATCH 08/11] test: report Pubky alias state on routing failures --- .../to/bitkit/build/PubkyAuthManifestTest.kt | 62 +++++++++++++++---- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/app/src/test/java/to/bitkit/build/PubkyAuthManifestTest.kt b/app/src/test/java/to/bitkit/build/PubkyAuthManifestTest.kt index 8354250793..9029439678 100644 --- a/app/src/test/java/to/bitkit/build/PubkyAuthManifestTest.kt +++ b/app/src/test/java/to/bitkit/build/PubkyAuthManifestTest.kt @@ -74,18 +74,25 @@ class PubkyAuthManifestTest { ) val allUrls = signupUrls + authUrls + unrelatedUrls - assertRoutes(packageManager, application.packageName, allUrls, null) + assertRoutes(packageManager, application.packageName, allUrls, null, "initial defaults") packageManager.setComponentEnabledSetting( signupAlias, PackageManager.COMPONENT_ENABLED_STATE_ENABLED, PackageManager.DONT_KILL_APP, ) - assertRoutes(packageManager, application.packageName, signupUrls + authUrls.last(), signupAlias) + assertRoutes( + packageManager, + application.packageName, + signupUrls + authUrls.last(), + signupAlias, + "signup enabled", + ) assertRoutes( packageManager, application.packageName, authUrls.dropLast(1) + unrelatedUrls, null, + "signup enabled", ) packageManager.setComponentEnabledSetting( @@ -98,15 +105,15 @@ class PubkyAuthManifestTest { PackageManager.COMPONENT_ENABLED_STATE_ENABLED, PackageManager.DONT_KILL_APP, ) - assertRoutes(packageManager, application.packageName, authUrls, authAlias) - assertRoutes(packageManager, application.packageName, signupUrls + unrelatedUrls, null) + assertRoutes(packageManager, application.packageName, authUrls, authAlias, "authorization enabled") + assertRoutes(packageManager, application.packageName, signupUrls + unrelatedUrls, null, "authorization enabled") packageManager.setComponentEnabledSetting( authAlias, PackageManager.COMPONENT_ENABLED_STATE_DISABLED, PackageManager.DONT_KILL_APP, ) - assertRoutes(packageManager, application.packageName, allUrls, null) + assertRoutes(packageManager, application.packageName, allUrls, null, "both explicitly disabled") } private fun assertRoutes( @@ -114,19 +121,50 @@ class PubkyAuthManifestTest { packageName: String, urls: List, alias: ComponentName?, + phase: String, ) { urls.forEach { val intent = Intent(Intent.ACTION_VIEW, it.toUri()) .addCategory(Intent.CATEGORY_BROWSABLE) .setPackage(packageName) val resolved = packageManager.queryIntentActivities(intent, PackageManager.MATCH_DEFAULT_ONLY) - if (alias == null) { - assertTrue(resolved.isEmpty(), it) - } else { - val activity = resolved.single().activityInfo - assertEquals(alias.className, activity.name, it) - assertEquals("to.bitkit.ui.MainActivity", activity.targetActivity) - assertTrue(activity.exported) + fun diagnostics() = buildString { + append("phase=$phase url=$it package=$packageName expected=${alias?.className}") + listOf("to.bitkit.ui.MainActivityPubkySignup", "to.bitkit.ui.MainActivityPubkyAuth").forEach { name -> + val component = ComponentName(packageName, name) + append("; alias=$name override=${packageManager.getComponentEnabledSetting(component)}") + runCatching { + packageManager.getActivityInfo( + component, + PackageManager.ComponentInfoFlags.of(PackageManager.MATCH_DISABLED_COMPONENTS.toLong()), + ) + }.onSuccess { info -> + append(" enabled=${info.enabled} exported=${info.exported} target=${info.targetActivity}") + }.onFailure { error -> + append(" activityInfoError=${error.javaClass.simpleName}") + } + } + resolved.forEach { result -> + val info = result.activityInfo + append("; resolved=${info.packageName}/${info.name} target=${info.targetActivity}") + append(" enabled=${info.enabled} exported=${info.exported} match=${result.match}") + } + } + try { + if (alias == null) { + assertTrue(resolved.isEmpty(), it) + } else { + assertEquals(1, resolved.size, it) + val activity = resolved.single().activityInfo + assertEquals(alias.className, activity.name, it) + assertEquals("to.bitkit.ui.MainActivity", activity.targetActivity) + assertTrue(activity.exported) + } + } catch (error: AssertionError) { + val message = runCatching { diagnostics() }.getOrElse { diagnosticError -> + "phase=$phase url=$it diagnosticsUnavailable=${diagnosticError.javaClass.simpleName}" + } + throw AssertionError(message, error) } } } From 4056d2c0036e5a4e51698e26b3d1615825dd77f5 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 4 Nov 2026 05:47:55 -0500 Subject: [PATCH 09/11] fix: keep VSS operations with their original wallet --- .../bitkit/data/WalletConnectionRecovery.kt | 4 +- .../to/bitkit/data/backup/VssBackupClient.kt | 139 +++-- .../bitkit/data/backup/VssBackupClientLdk.kt | 75 +-- .../data/backup/VssClientCoordinator.kt | 158 ++++++ .../java/to/bitkit/repositories/BackupRepo.kt | 214 ++++---- .../to/bitkit/repositories/LightningRepo.kt | 478 +++++++++--------- .../to/bitkit/viewmodels/BackupsViewModel.kt | 9 +- .../data/WalletConnectionRecoveryTest.kt | 47 +- .../data/backup/VssBackupClientLdkTest.kt | 38 +- .../bitkit/data/backup/VssBackupClientTest.kt | 53 +- .../data/backup/VssClientCoordinatorTest.kt | 188 +++++++ .../to/bitkit/repositories/BackupRepoTest.kt | 65 ++- .../bitkit/repositories/LightningRepoTest.kt | 64 ++- .../bitkit/test/ConnectionRecoveryFixture.kt | 4 +- changelog.d/next/1686.fixed.md | 2 +- 15 files changed, 1069 insertions(+), 469 deletions(-) create mode 100644 app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt create mode 100644 app/src/test/java/to/bitkit/data/backup/VssClientCoordinatorTest.kt diff --git a/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt b/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt index e1c70bbffb..fdecbf8854 100644 --- a/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt +++ b/app/src/main/java/to/bitkit/data/WalletConnectionRecovery.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.keychain.Keychain import to.bitkit.data.keychain.hasPendingWalletRestore @@ -30,6 +31,7 @@ class WalletConnectionRecovery @Inject constructor( private val db: AppDb, private val storeIdProvider: VssStoreIdProvider, private val settingsStore: SettingsStore, + private val vssClients: VssClientCoordinator, ) { companion object { internal const val KEY = "WALLET_CONNECTION_RECOVERY" @@ -130,7 +132,7 @@ class WalletConnectionRecovery @Inject constructor( currentCoroutineContext().ensureActive() beginWipe() currentCoroutineContext().ensureActive() - block() + vssClients.withSuspendedClients { block() } } finally { storeIdProvider.clearCache() endWipe() diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt index eef9af0b15..728b112e41 100644 --- a/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt +++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClient.kt @@ -3,24 +3,27 @@ package to.bitkit.data.backup import com.synonym.vssclient.KeyVersion import com.synonym.vssclient.VssItem import com.synonym.vssclient.vssDelete +import com.synonym.vssclient.vssDeriveStoreId import com.synonym.vssclient.vssGet import com.synonym.vssclient.vssListKeys import com.synonym.vssclient.vssNewClientWithLnurlAuth import com.synonym.vssclient.vssStore -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.delay -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import to.bitkit.data.keychain.Keychain import to.bitkit.di.IoDispatcher import to.bitkit.env.Env +import to.bitkit.ext.runSuspendCatching import to.bitkit.utils.Logger import to.bitkit.utils.ServiceError import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Duration.Companion.seconds class MnemonicNotAvailableException : Exception("Mnemonic not available") @@ -28,19 +31,12 @@ class MnemonicNotAvailableException : Exception("Mnemonic not available") @Singleton class VssBackupClient @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, - private val vssStoreIdProvider: VssStoreIdProvider, + private val coordinator: VssClientCoordinator, private val keychain: Keychain, ) { - @Volatile - private var isSetup = CompletableDeferred() - private val setupMutex = Mutex() - suspend fun setup(walletIndex: Int = 0): Result = withContext(ioDispatcher) { - setupMutex.withLock { - val gate = isSetup - runCatching { - if (gate.isCompleted && !gate.isCancelled) return@runCatching - + runCatching { + coordinator.setup(VssClientCoordinator.Domain.APP) { val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw MnemonicNotAvailableException() @@ -48,13 +44,13 @@ class VssBackupClient @Inject constructor( Logger.debug("VSS client setting up…", context = TAG) val vssUrl = Env.vssServerUrl val lnurlAuthServerUrl = Env.lnurlAuthServerUrl - val vssStoreId = vssStoreIdProvider.getVssStoreId(walletIndex) Logger.verbose("Building VSS client with vssUrl: '$vssUrl'", context = TAG) Logger.verbose("Building VSS client with lnurlAuthServerUrl: '$lnurlAuthServerUrl'", context = TAG) if (lnurlAuthServerUrl.isBlank()) { throw ServiceError.VssAuthRequired() } val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name) + val vssStoreId = vssDeriveStoreId(Env.vssStoreIdPrefix, mnemonic, passphrase) vssNewClientWithLnurlAuth( baseUrl = vssUrl, @@ -63,14 +59,13 @@ class VssBackupClient @Inject constructor( passphrase = passphrase, lnurlAuthServerUrl = lnurlAuthServerUrl, ) - gate.complete(Unit) - Logger.info("VSS client setup with server: '$vssUrl'", context = TAG) + Logger.info("VSS client setup for wallet[$walletIndex] with server: '$vssUrl'", context = TAG) } - }.onFailure { - gate.completeExceptionally(it) - if (isSetup === gate) isSetup = CompletableDeferred() - Logger.error("VSS client setup error", it, context = TAG) } + }.onFailure { + currentCoroutineContext().ensureActive() + if (it is CancellationException && it !is TimeoutCancellationException) throw it + Logger.error("VSS client setup error", it, context = TAG) } } @@ -85,49 +80,43 @@ class VssBackupClient @Inject constructor( baseDelayMs: Long = 1000L, logger: SetupRetryLogger.() -> Unit, ): Result = withContext(ioDispatcher) { - val log = SetupRetryLogger().apply(logger) - var attempt = 0 - while (attempt < maxAttempts) { - val result = setup() - if (result.isSuccess) { - log.onSuccess(attempt + 1) - return@withContext Result.success(Unit) - } - val exception = result.exceptionOrNull() - if (exception != null && exception !is MnemonicNotAvailableException) { - return@withContext result - } - attempt++ - if (attempt < maxAttempts) { - val delayMs = baseDelayMs * attempt - log.onRetry(attempt, maxAttempts, delayMs) - delay(delayMs) + coordinator.withWallet { + val log = SetupRetryLogger().apply(logger) + var attempt = 0 + while (attempt < maxAttempts) { + val result = setup() + if (result.isSuccess) { + log.onSuccess(attempt + 1) + return@withWallet Result.success(Unit) + } + val exception = result.exceptionOrNull() + if (exception != null && exception !is MnemonicNotAvailableException) { + return@withWallet result + } + attempt++ + if (attempt < maxAttempts) { + val delayMs = baseDelayMs * attempt + log.onRetry(attempt, maxAttempts, delayMs) + delay(delayMs) + } } + log.onExhausted(maxAttempts) + Result.failure(MnemonicNotAvailableException()) } - log.onExhausted(maxAttempts) - Result.failure(MnemonicNotAvailableException()) - } - - fun reset() { - synchronized(this) { - isSetup.cancel() - isSetup = CompletableDeferred() - } - vssStoreIdProvider.clearCache() - Logger.debug("VSS client reset", context = TAG) } suspend fun putObject( key: String, data: ByteArray, ): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS 'putObject' call for '$key'", context = TAG) - runCatching { - vssStore( - key = key, - value = data, - ) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.APP) { + vssStore( + key = key, + value = data, + ) + } }.onSuccess { Logger.verbose("VSS 'putObject' success for '$key' at version: '${it.version}'", context = TAG) }.onFailure { @@ -136,12 +125,13 @@ class VssBackupClient @Inject constructor( } suspend fun getObject(key: String): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS 'getObject' call for '$key'", context = TAG) - runCatching { - vssGet( - key = key, - ) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.APP) { + vssGet( + key = key, + ) + } }.onSuccess { if (it == null) { Logger.verbose("VSS 'getObject' success null for '$key'", context = TAG) @@ -154,10 +144,11 @@ class VssBackupClient @Inject constructor( } suspend fun listKeys(prefix: String? = null): Result> = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS 'listKeys' call with prefix: '$prefix'", context = TAG) - runCatching { - vssListKeys(prefix = prefix) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.APP) { + vssListKeys(prefix = prefix) + } }.onSuccess { Logger.verbose("VSS 'listKeys' success - found ${it.size} key(s)", context = TAG) }.onFailure { @@ -166,10 +157,11 @@ class VssBackupClient @Inject constructor( } suspend fun deleteObject(key: String): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS 'deleteObject' call for '$key'", context = TAG) - runCatching { - vssDelete(key = key) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.APP) { + vssDelete(key = key) + } }.onSuccess { wasDeleted -> if (wasDeleted) { Logger.verbose("VSS 'deleteObject' success for '$key' - key was deleted", context = TAG) @@ -182,16 +174,17 @@ class VssBackupClient @Inject constructor( } suspend fun deleteAllKeys(): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS 'deleteAllKeys' call", context = TAG) - runCatching { - val keys = vssListKeys(prefix = null) - var deletedCount = 0 - for (keyVersion in keys) { - val wasDeleted = vssDelete(key = keyVersion.key) - if (wasDeleted) deletedCount++ + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.APP) { + val keys = vssListKeys(prefix = null) + var deletedCount = 0 + for (keyVersion in keys) { + val wasDeleted = vssDelete(key = keyVersion.key) + if (wasDeleted) deletedCount++ + } + deletedCount } - deletedCount }.onSuccess { Logger.verbose("VSS 'deleteAllKeys' success - deleted $it key(s)", context = TAG) }.onFailure { diff --git a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt index 7842dbf194..bd5fe8766e 100644 --- a/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt +++ b/app/src/main/java/to/bitkit/data/backup/VssBackupClientLdk.kt @@ -3,29 +3,32 @@ package to.bitkit.data.backup import com.synonym.vssclient.KeyVersion import com.synonym.vssclient.LdkNamespace import com.synonym.vssclient.VssItem +import com.synonym.vssclient.vssDeriveStoreId import com.synonym.vssclient.vssLdkDelete import com.synonym.vssclient.vssLdkGet import com.synonym.vssclient.vssLdkListKeys import com.synonym.vssclient.vssNewLdkClientWithLnurlAuth -import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import to.bitkit.data.keychain.Keychain import to.bitkit.di.IoDispatcher import to.bitkit.env.Env +import to.bitkit.ext.runSuspendCatching import to.bitkit.utils.Logger import to.bitkit.utils.ServiceError import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException import kotlin.time.Duration.Companion.seconds @Singleton class VssBackupClientLdk @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, - private val vssStoreIdProvider: VssStoreIdProvider, + private val coordinator: VssClientCoordinator, private val keychain: Keychain, ) { companion object { @@ -38,16 +41,9 @@ class VssBackupClientLdk @Inject constructor( ) } - @Volatile - private var isSetup = CompletableDeferred() - private val setupMutex = Mutex() - suspend fun setup(walletIndex: Int = 0): Result = withContext(ioDispatcher) { - setupMutex.withLock { - val gate = isSetup - runCatching { - if (gate.isCompleted && !gate.isCancelled) return@runCatching - + runCatching { + coordinator.setup(VssClientCoordinator.Domain.LDK) { val mnemonic = keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name) ?: throw MnemonicNotAvailableException() @@ -58,38 +54,30 @@ class VssBackupClientLdk @Inject constructor( val passphrase = keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name) vssNewLdkClientWithLnurlAuth( baseUrl = Env.vssServerUrl, - storeId = vssStoreIdProvider.getVssStoreId(walletIndex), + storeId = vssDeriveStoreId(Env.vssStoreIdPrefix, mnemonic, passphrase), mnemonic = mnemonic, passphrase = passphrase, lnurlAuthServerUrl = Env.lnurlAuthServerUrl, ) - gate.complete(Unit) - Logger.info("VSS LDK client setup", context = TAG) + Logger.info("VSS LDK client setup for wallet[$walletIndex]", context = TAG) } - }.onFailure { - gate.completeExceptionally(it) - if (isSetup === gate) isSetup = CompletableDeferred() - Logger.error("VSS LDK client setup error", it, context = TAG) } + }.onFailure { + currentCoroutineContext().ensureActive() + if (it is CancellationException && it !is TimeoutCancellationException) throw it + Logger.error("VSS LDK client setup error", it, context = TAG) } } - fun reset() { - synchronized(this) { - isSetup.cancel() - isSetup = CompletableDeferred() - } - Logger.debug("VSS LDK client reset", context = TAG) - } - suspend fun getObject( key: String, namespace: LdkNamespace = LdkNamespace.Default, ): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS LDK 'getObject' call for '$key'", context = TAG) - runCatching { - vssLdkGet(key = key, namespace = namespace) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.LDK) { + vssLdkGet(key = key, namespace = namespace) + } }.onSuccess { if (it == null) { Logger.verbose("VSS LDK 'getObject' success null for '$key'", context = TAG) @@ -105,10 +93,11 @@ class VssBackupClientLdk @Inject constructor( key: String, namespace: LdkNamespace = LdkNamespace.Default, ): Result = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS LDK 'deleteObject' call for '$key'", context = TAG) - runCatching { - vssLdkDelete(key = key, namespace = namespace) + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.LDK) { + vssLdkDelete(key = key, namespace = namespace) + } }.onSuccess { wasDeleted -> if (wasDeleted) { Logger.verbose("VSS LDK 'deleteObject' success for '$key' - key was deleted", context = TAG) @@ -120,11 +109,23 @@ class VssBackupClientLdk @Inject constructor( } } + suspend fun deleteObjects(keys: List, walletIndex: Int = 0): Result = withContext(ioDispatcher) { + runSuspendCatching { + coordinator.withWallet { + setup(walletIndex).getOrThrow() + coordinator.withClient(VssClientCoordinator.Domain.LDK) { + keys.forEach { vssLdkDelete(key = it, namespace = LdkNamespace.Default) } + } + } + } + } + suspend fun listAllKeysTagged(): Result>> = withContext(ioDispatcher) { - isSetup.await() Logger.verbose("VSS LDK 'listAllKeysTagged' call", context = TAG) - runCatching { - NAMESPACES.flatMap { ns -> vssLdkListKeys(namespace = ns).map { ns to it } } + runSuspendCatching { + coordinator.withClient(VssClientCoordinator.Domain.LDK) { + NAMESPACES.flatMap { ns -> vssLdkListKeys(namespace = ns).map { ns to it } } + } }.onSuccess { Logger.verbose("VSS LDK 'listAllKeysTagged' success - found ${it.size} key(s)", context = TAG) }.onFailure { diff --git a/app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt b/app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt new file mode 100644 index 0000000000..e7768e136c --- /dev/null +++ b/app/src/main/java/to/bitkit/data/backup/VssClientCoordinator.kt @@ -0,0 +1,158 @@ +package to.bitkit.data.backup + +import com.synonym.vssclient.vssShutdownClient +import com.synonym.vssclient.vssShutdownLdkClient +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Job +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.joinAll +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.coroutines.AbstractCoroutineContextElement +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.cancellation.CancellationException + +/** Owns the two standalone native slots; the running Lightning node has a separate lifetime. */ +@Singleton +class VssClientCoordinator @Inject constructor() { + enum class Domain { APP, LDK } + + class Owner internal constructor(internal val generation: Long) + + private class Admission(val coordinator: VssClientCoordinator, val owner: Owner) : + AbstractCoroutineContextElement(Key) { + companion object Key : CoroutineContext.Key + } + + private class Client { + val setupMutex = Mutex() + var ready = CompletableDeferred() + var attempted = false + } + + private val lock = Any() + private val transitionMutex = Mutex() + private var generation = 0L + private var suspended = false + private val jobs = mutableSetOf() + private val clients = Domain.entries.associateWith { Client() } + private val _owners = MutableStateFlow(Owner(generation)) + val owners = _owners.asStateFlow() + + fun captureOwner(): Owner = synchronized(lock) { + if (suspended) throw CancellationException("VSS clients are suspended") + Owner(generation) + } + + private fun checkOwner(owner: Owner) = synchronized(lock) { + if (suspended || owner.generation != generation) { + throw CancellationException("VSS wallet owner changed") + } + } + + /** Retain admission across payload capture, retries, compound requests and their local tails. */ + suspend fun withWallet(owner: Owner? = null, block: suspend () -> T): T = coroutineScope { + val inherited = currentCoroutineContext()[Admission]?.takeIf { it.coordinator === this@VssClientCoordinator } + val original = owner ?: inherited?.owner ?: captureOwner() + val job = checkNotNull(currentCoroutineContext()[Job]) + synchronized(lock) { + checkOwner(original) + jobs.add(job) + } + try { + withContext(Admission(this@VssClientCoordinator, original)) { + val result = runCatching { block() } + currentCoroutineContext().ensureActive() + checkOwner(original) + result.getOrThrow() + } + } finally { + synchronized(lock) { jobs.remove(job) } + } + } + + suspend fun setup(domain: Domain, block: suspend () -> Unit) = withWallet { + val client = clients.getValue(domain) + client.setupMutex.withLock { + currentCoroutineContext().ensureActive() + val gate = synchronized(lock) { client.ready } + if (gate.isCompleted && !gate.isCancelled) return@withLock + try { + synchronized(lock) { client.attempted = true } + block() + currentCoroutineContext().ensureActive() + gate.complete(Unit) + } catch (error: Throwable) { + gate.completeExceptionally(error) + synchronized(lock) { + if (client.ready === gate) client.ready = CompletableDeferred() + } + throw error + } + } + } + + suspend fun withClient(domain: Domain, block: suspend () -> T): T = withWallet { + val gate = synchronized(lock) { clients.getValue(domain).ready } + gate.await() + currentCoroutineContext().ensureActive() + block() + } + + /** Close admission before credentials change, including cleanup when the caller is cancelled. */ + suspend fun withSuspendedClients( + shutdown: (Domain) -> Unit = { + when (it) { + Domain.APP -> vssShutdownClient() + Domain.LDK -> vssShutdownLdkClient() + } + }, + block: suspend () -> T, + ): T { + check(currentCoroutineContext()[Admission]?.coordinator !== this) { + "Cannot replace a wallet inside an admitted VSS operation" + } + val original = captureOwner() + return transitionMutex.withLock { + val pending = synchronized(lock) { + checkOwner(original) + suspended = true + _owners.update { null } + generation += 1 + clients.values.forEach { it.ready.cancel() } + jobs.toList() + } + var shutdownComplete = false + try { + withContext(NonCancellable) { + pending.forEach { it.cancel(CancellationException("VSS wallet retired")) } + pending.joinAll() + clients.forEach { (domain, client) -> if (client.attempted) shutdown(domain) } + shutdownComplete = true + } + currentCoroutineContext().ensureActive() + block() + } finally { + synchronized(lock) { + if (shutdownComplete) { + clients.values.forEach { + it.ready = CompletableDeferred() + it.attempted = false + } + suspended = false + _owners.update { Owner(generation) } + } + } + } + } + } +} diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index f09a091bf1..ee050e3dca 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -5,6 +5,8 @@ import com.synonym.bitkitcore.UsdtException import com.synonym.vssclient.VssException import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.NonCancellable @@ -16,6 +18,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop @@ -47,6 +50,7 @@ import to.bitkit.data.WidgetsStore import to.bitkit.data.backup.UsdtRecoveryBackupStore import to.bitkit.data.backup.VssBackupClient import to.bitkit.data.backup.VssBackupClientLdk +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.entities.TransferEntity import to.bitkit.data.hwWalletNames @@ -114,6 +118,7 @@ class BackupRepo @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val cacheStore: CacheStore, private val vssBackupClient: VssBackupClient, + private val vssClients: VssClientCoordinator, private val vssBackupClientLdk: VssBackupClientLdk, private val settingsStore: SettingsStore, private val widgetsStore: WidgetsStore, @@ -149,16 +154,21 @@ class BackupRepo @Inject constructor( private val usdtRecoveryBackup = UsdtRecoveryBackupStore(vssBackupClient) private val scope = appScope(ioDispatcher, TAG) - private val backupJobs = mutableMapOf() + private val backupJobs = ConcurrentHashMap.newKeySet() + private val backupJobLock = Any() + private val pendingBackups = ConcurrentHashMap() private val statusObserverJobs = mutableListOf() private val dataListenerJobs = mutableListOf() private var periodicCheckJob: Job? = null + private var setupJob: Job? = null + private var setupGeneration: Long? = null private val runningBackups = ConcurrentHashMap.newKeySet() // Tracks active jobs since app start private val failedBackupRequired = ConcurrentHashMap() private val temporaryFailureRetries = ConcurrentHashMap() private val temporaryFailureSince = ConcurrentHashMap() + @Volatile private var isObserving = false private var lastNotificationTime = 0L @@ -172,8 +182,6 @@ class BackupRepo @Inject constructor( fun reset() { stopObservingBackups() - vssBackupClient.reset() - vssBackupClientLdk.reset() } fun setWiping(isWiping: Boolean) = _isWiping.update { isWiping } @@ -224,26 +232,41 @@ class BackupRepo @Inject constructor( isObserving = true Logger.debug("Start observing backup statuses and data store changes", context = TAG) - scope.launch { - vssBackupClient.setupWithRetry { - onSuccess = { attempt -> - Logger.debug("VSS client setup succeeded on attempt $attempt", context = TAG) - } - onRetry = { attempt, maxAttempts, delayMs -> - Logger.debug( - "VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)", - context = TAG, - ) - } - onExhausted = { maxAttempts -> - Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG) + setupJob = scope.launch { + vssClients.owners.collectLatest { owner -> + if (owner == null) return@collectLatest + vssClients.withWallet(owner) { + if (setupGeneration != null && setupGeneration != owner.generation) { + failedBackupRequired.clear() + temporaryFailureRetries.clear() + temporaryFailureSince.clear() + } + setupGeneration = owner.generation + vssBackupClient.setupWithRetry { + onSuccess = { attempt -> + Logger.debug("VSS client setup succeeded on attempt $attempt", context = TAG) + } + onRetry = { attempt, maxAttempts, delayMs -> + Logger.debug( + "VSS client setup deferred, retrying in ${delayMs}ms (attempt $attempt/$maxAttempts)", + context = TAG, + ) + } + onExhausted = { maxAttempts -> + Logger.warn("VSS client setup failed after $maxAttempts attempts", context = TAG) + } + }.onSuccess { + vssBackupClientLdk.setup() + _isWiping.first { !it } + cacheStore.backupStatuses.first().forEach { (category, status) -> + if (status.shouldBackup(category)) scheduleBackup(category) + } + } } - }.onSuccess { - scope.launch { vssBackupClientLdk.setup() } } } - scope.launch { + launchForWallet { BackupCategory.entries.forEach { category -> if (category !in runningBackups) { cacheStore.updateBackupStatus(category) { status -> @@ -266,11 +289,18 @@ class BackupRepo @Inject constructor( fun stopObservingBackups() { if (!isObserving) return - isObserving = false + val pendingJobs = synchronized(backupJobLock) { + isObserving = false + backupJobs.toList().also { + backupJobs.clear() + pendingBackups.clear() + } + } + setupJob?.cancel() + setupJob = null // Cancel all backup jobs - backupJobs.values.forEach { it.cancel() } - backupJobs.clear() + pendingJobs.forEach { it.cancel() } // Cancel backup status observer jobs statusObserverJobs.forEach { it.cancel() } @@ -472,8 +502,13 @@ class BackupRepo @Inject constructor( } } + private fun launchForWallet(block: suspend CoroutineScope.() -> Unit): Job { + val owner = runCatching { vssClients.captureOwner() }.getOrNull() ?: return Job().apply { cancel() } + return scope.launch { vssClients.withWallet(owner) { block() } } + } + private fun markBackupRequired(category: BackupCategory) { - scope.launch { + launchForWallet { failedBackupRequired -= category cacheStore.updateBackupStatus(category) { it.copy(required = maxOf(currentTimeMillis(), it.required + 1, it.synced + 1)) @@ -482,50 +517,45 @@ class BackupRepo @Inject constructor( } } - private fun scheduleBackup(category: BackupCategory) { - backupJobs[category]?.cancel() - - Logger.verbose("Scheduling backup for: '$category'", context = TAG) - - backupJobs[category] = scope.launch { - runningBackups += category - cacheStore.updateBackupStatus(category) { - it.copy(running = true) - } - - delay(BACKUP_DEBOUNCE) - if (category == BackupCategory.WALLET) paykitSdkService.isPaymentSubmissionActive.first { !it } - - val status = cacheStore.backupStatuses.first()[category] ?: BackupItemStatus() - if (status.isRequired && !shouldSkipBackup(category)) { - triggerBackup(category) - } else { - Logger.debug("Backup no longer needed for: '$category'", context = TAG) - runningBackups -= category - cacheStore.updateBackupStatus(category) { - it.copy(running = false) - } + private fun scheduleBackup( + category: BackupCategory, + expectedOwner: VssClientCoordinator.Owner? = null, + requireObservation: Boolean = true, + ) { + val owner = expectedOwner ?: runCatching { vssClients.captureOwner() }.getOrNull() ?: return + val job = synchronized(backupJobLock) { + if (requireObservation && !isObserving) return + var reserved = false + pendingBackups.compute(category) { _, previous -> + previous?.takeIf { it.generation >= owner.generation } ?: owner.also { reserved = true } } - }.also { job -> - job.invokeOnCompletion { exception -> - if (exception != null) { - Logger.debug("Backup job cancelled for: '$category'", context = TAG) - scope.launch { - runningBackups -= category - cacheStore.updateBackupStatus(category) { - it.copy(running = false) - } + if (!reserved) return + Logger.debug("Scheduling backup for: '$category'", context = TAG) + scope.launch(start = CoroutineStart.LAZY) { + try { + vssClients.withWallet(owner) { + delay(BACKUP_DEBOUNCE) + if (category == BackupCategory.WALLET) paykitSdkService.isPaymentSubmissionActive.first { !it } + pendingBackups.remove(category, owner) + val status = cacheStore.backupStatuses.first()[category] ?: BackupItemStatus() + if (status.shouldBackup(category)) triggerBackup(category) } + } finally { + pendingBackups.remove(category, owner) } + }.also { scheduled -> + backupJobs.add(scheduled) + scheduled.invokeOnCompletion { backupJobs.remove(scheduled) } } } + job.start() } private fun checkForFailedBackups() { val currentTime = currentTimeMillis() // find if there are any backup categories that have been failing for more than 30 minutes - scope.launch { + launchForWallet { val backupStatuses = cacheStore.backupStatuses.first() val hasFailedBackups = BackupCategory.entries.any { category -> val status = backupStatuses[category] ?: BackupItemStatus() @@ -548,7 +578,7 @@ class BackupRepo @Inject constructor( lastNotificationTime = currentTime - scope.launch { + launchForWallet { ToastEventBus.send( type = Toast.ToastType.ERROR, title = context.getString(R.string.settings__backup__failed_title), @@ -560,10 +590,12 @@ class BackupRepo @Inject constructor( } suspend fun triggerBackup(category: BackupCategory): Result = withContext(ioDispatcher) { - if (category == BackupCategory.WALLET || category == BackupCategory.METADATA) { - walletBackupMutex.withLock { performBackup(category) } - } else { - performBackup(category) + vssClients.withWallet { + if (category == BackupCategory.WALLET || category == BackupCategory.METADATA) { + walletBackupMutex.withLock { performBackup(category) } + } else { + performBackup(category) + } } } @@ -621,15 +653,20 @@ class BackupRepo @Inject constructor( suspend fun persistWalletBackup(): Result = withContext(ioDispatcher) { - walletBackupMutex.withLock { uploadWalletBackup() } - }.onFailure { markBackupRequired(BackupCategory.WALLET) } - - suspend fun persistUsdtRecoveryBackup(snapshot: String): Result = runSuspendCatching { - cacheStore.updateBackupStatus(BackupCategory.USDT_RECOVERY) { - it.copy(required = maxOf(currentTimeMillis(), it.required + 1, it.synced + 1)) + vssClients.withWallet { + walletBackupMutex.withLock { uploadWalletBackup() } + .onFailure { markBackupRequired(BackupCategory.WALLET) } + } } - uploadUsdtRecoveryBackup(snapshot).getOrThrow() - }.onFailure { markBackupRequired(BackupCategory.USDT_RECOVERY) } + + suspend fun persistUsdtRecoveryBackup(snapshot: String): Result = vssClients.withWallet { + runSuspendCatching { + cacheStore.updateBackupStatus(BackupCategory.USDT_RECOVERY) { + it.copy(required = maxOf(currentTimeMillis(), it.required + 1, it.synced + 1)) + } + uploadUsdtRecoveryBackup(snapshot).getOrThrow() + }.onFailure { markBackupRequired(BackupCategory.USDT_RECOVERY) } + } private suspend fun uploadUsdtRecoveryBackup(snapshot: String? = null): Result = withContext(ioDispatcher) { runSuspendCatching { @@ -691,10 +728,10 @@ class BackupRepo @Inject constructor( val retryDelay = (TEMPORARY_FAILURE_RETRY_DELAY * (1 shl (attempt - 1).coerceAtMost(MAX_RETRY_SHIFT))) .coerceAtMost(TEMPORARY_FAILURE_RETRY_MAX_DELAY) Logger.warn("Retrying backup for: '$category' in '$retryDelay'", e, context = TAG) - scope.launch { + launchForWallet { delay(retryDelay) - if (!failedBackupRequired.remove(category, backupRequired) || !isObserving) return@launch - val status = cacheStore.backupStatuses.first()[category] ?: return@launch + if (!failedBackupRequired.remove(category, backupRequired) || !isObserving) return@launchForWallet + val status = cacheStore.backupStatuses.first()[category] ?: return@launchForWallet if (status.shouldBackup(category)) scheduleBackup(category) } } @@ -1212,21 +1249,23 @@ class BackupRepo @Inject constructor( } suspend fun getLatestBackupTime(): ULong? = withContext(ioDispatcher) { - runCatching { - withTimeout(VSS_TIMESTAMP_TIMEOUT) { - vssBackupClient.setup().getOrThrow() - coroutineScope { - BackupCategory.entries - .filter { it != BackupCategory.LIGHTNING_CONNECTIONS && it != BackupCategory.USDT_RECOVERY } - .map { category -> async { getRemoteBackupTimestamp(category) } } - .mapNotNull { it.await() } - .filter { it > 0uL } - .maxOrNull() + vssClients.withWallet { + runCatching { + withTimeout(VSS_TIMESTAMP_TIMEOUT) { + vssBackupClient.setup().getOrThrow() + coroutineScope { + BackupCategory.entries + .filter { it != BackupCategory.LIGHTNING_CONNECTIONS && it != BackupCategory.USDT_RECOVERY } + .map { category -> async { getRemoteBackupTimestamp(category) } } + .mapNotNull { it.await() } + .filter { it > 0uL } + .maxOrNull() + } } - } - }.onFailure { e -> - Logger.warn("Failed to get VSS backup timestamp: $e", context = TAG) - }.getOrNull() + }.onFailure { e -> + Logger.warn("Failed to get VSS backup timestamp: $e", context = TAG) + }.getOrNull() + } } private suspend fun getRemoteBackupTimestamp(category: BackupCategory): ULong? { @@ -1243,11 +1282,12 @@ class BackupRepo @Inject constructor( } fun scheduleFullBackup() { + val owner = runCatching { vssClients.captureOwner() }.getOrNull() ?: return Logger.debug("Scheduling backups for all categories", context = TAG) BackupCategory.entries .filter { it != BackupCategory.LIGHTNING_CONNECTIONS } .forEach { - scheduleBackup(it) + scheduleBackup(it, expectedOwner = owner, requireObservation = false) } } diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index 46d9fc0edd..2d4d34eba1 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt @@ -64,6 +64,7 @@ import to.bitkit.data.CacheStore import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.backup.VssBackupClientLdk +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.keychain.Keychain import to.bitkit.data.keychain.requireWalletRestoreComplete import to.bitkit.di.BgDispatcher @@ -128,6 +129,7 @@ class LightningRepo @Inject constructor( private val onchainSendAttemptStore: OnchainSendAttemptStore, private val connectivityRepo: ConnectivityRepo, private val vssBackupClientLdk: VssBackupClientLdk, + private val vssClients: VssClientCoordinator, private val urlValidator: UrlValidator, private val electrumProbeService: ElectrumProbeService, ) : OnchainReleasing { @@ -353,153 +355,159 @@ class LightningRepo @Inject constructor( shouldValidateGraph: Boolean = true, shouldRetryYieldToStop: Boolean = false, shouldCancelPendingStop: Boolean = true, - ): Result = withContext(bgDispatcher) { - if (_isRecoveryMode.value) { - return@withContext Result.failure(RecoveryModeError()) - } + ): Result = vssClients.withWallet { + withContext(bgDispatcher) { + if (_isRecoveryMode.value) { + return@withContext Result.failure(RecoveryModeError()) + } - if (shouldCancelPendingStop) cancelPendingStop() + if (shouldCancelPendingStop) cancelPendingStop() - eventHandler?.let { _eventHandlers.add(it) } + eventHandler?.let { _eventHandlers.add(it) } - // Track retry state outside mutex to avoid deadlock (Mutex is non-reentrant) - var shouldRetryStart = false - var shouldRestartForGraphReset = false - var initialLifecycleState: NodeLifecycleState + // Track retry state outside mutex to avoid deadlock (Mutex is non-reentrant) + var shouldRetryStart = false + var shouldRestartForGraphReset = false + var initialLifecycleState: NodeLifecycleState - val result = lifecycleMutex.withLock { - if (isWiping) return@withLock Result.failure(WipeInProgressError()) - initialLifecycleState = _lightningState.value.nodeLifecycleState - if (initialLifecycleState.isRunningOrStarting()) { - return@withLock skipStartForRunningNode( - lifecycleState = initialLifecycleState, - customServerUrl = customServerUrl, - customRgsServerUrl = customRgsServerUrl, - ) - } + val result = lifecycleMutex.withLock { + if (isWiping) return@withLock Result.failure(WipeInProgressError()) + initialLifecycleState = _lightningState.value.nodeLifecycleState + if (initialLifecycleState.isRunningOrStarting()) { + return@withLock skipStartForRunningNode( + lifecycleState = initialLifecycleState, + customServerUrl = customServerUrl, + customRgsServerUrl = customRgsServerUrl, + ) + } - runCatching { - _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Starting) } - - // Setup if needed - if (lightningService.node == null) { - val setupResult = - setup(walletIndex, customServerUrl, customRgsServerUrl, channelMigration) - if (setupResult.isFailure) { - _lightningState.update { - it.copy( - nodeLifecycleState = NodeLifecycleState.ErrorStarting( - setupResult.exceptionOrNull() ?: NodeSetupError() + runCatching { + _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Starting) } + + // Setup if needed + if (lightningService.node == null) { + val setupResult = + setup(walletIndex, customServerUrl, customRgsServerUrl, channelMigration) + if (setupResult.isFailure) { + _lightningState.update { + it.copy( + nodeLifecycleState = NodeLifecycleState.ErrorStarting( + setupResult.exceptionOrNull() ?: NodeSetupError() + ) ) - ) + } + return@withLock setupResult } - return@withLock setupResult } - } - if (lightningService.status?.isRunning == true) { - Logger.info("LDK node already running", context = TAG) - runSuspendCatching { lightningService.reconcileWatchOnlyAccounts() } - .onFailure { - Logger.warn("Failed to reconcile Paykit Server accounts during startup", it, context = TAG) - } - return@withLock adoptRunningNode() - } + if (lightningService.status?.isRunning == true) { + Logger.info("LDK node already running", context = TAG) + runSuspendCatching { lightningService.reconcileWatchOnlyAccounts() } + .onFailure { + Logger.warn( + "Failed to reconcile Paykit Server accounts during startup", + it, + context = TAG, + ) + } + return@withLock adoptRunningNode() + } - lightningService.start(timeout, ::onEvent) + lightningService.start(timeout, ::onEvent) - _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Running) } + _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Running) } - // Initial state sync - syncState() - logNodeSupportSummary("node started") - refreshChannelCache() + // Initial state sync + syncState() + logNodeSupportSummary("node started") + refreshChannelCache() - if (shouldValidateGraph && !lightningService.aresRequiredPeersInNetworkGraph()) { - Logger.warn("Network graph is stale, resetting and restarting...", context = TAG) + if (shouldValidateGraph && !lightningService.aresRequiredPeersInNetworkGraph()) { + Logger.warn("Network graph is stale, resetting and restarting...", context = TAG) - lightningService.stop() - clearNetworkGraph(walletIndex) + lightningService.stop() + clearNetworkGraph(walletIndex) - _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Stopped) } - shouldRestartForGraphReset = true - return@withLock Result.success(Unit) - } + _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.Stopped) } + shouldRestartForGraphReset = true + return@withLock Result.success(Unit) + } - // Post-startup tasks (non-blocking) - disconnectIdlePeers().onFailure { - Logger.error("Failed to disconnect idle peers", it, context = TAG) - } - logNodeSupportSummary("idle peers disconnected") + // Post-startup tasks (non-blocking) + disconnectIdlePeers().onFailure { + Logger.error("Failed to disconnect idle peers", it, context = TAG) + } + logNodeSupportSummary("idle peers disconnected") - sync().onFailure { e -> - Logger.warn("Initial sync failed, event-driven sync will retry", e, context = TAG) - } - Result.success(Unit) - }.getOrElse { e -> - if (e is CancellationException) { - withContext(NonCancellable) { reconcileCancelledStart(initialLifecycleState) } - throw e - } + sync().onFailure { e -> + Logger.warn("Initial sync failed, event-driven sync will retry", e, context = TAG) + } + Result.success(Unit) + }.getOrElse { e -> + if (e is CancellationException) { + withContext(NonCancellable) { reconcileCancelledStart(initialLifecycleState) } + throw e + } - val currentState = _lightningState.value.nodeLifecycleState - if (currentState.isRunning()) { - Logger.warn("Start error but node is $currentState, skipping retry", e, context = TAG) - return@withLock Result.success(Unit) - } + val currentState = _lightningState.value.nodeLifecycleState + if (currentState.isRunning()) { + Logger.warn("Start error but node is $currentState, skipping retry", e, context = TAG) + return@withLock Result.success(Unit) + } + + if (lightningService.status?.isRunning == true) { + Logger.warn("Start error but LDK node is running, adopting it", e, context = TAG) + return@withLock adoptRunningNode() + } - if (lightningService.status?.isRunning == true) { - Logger.warn("Start error but LDK node is running, adopting it", e, context = TAG) - return@withLock adoptRunningNode() + if (shouldRetry) { + Logger.warn("Start error, will retry...", e, context = TAG) + _lightningState.update { it.copy(nodeLifecycleState = initialLifecycleState) } + shouldRetryStart = true + Result.failure(e) + } else { + _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.ErrorStarting(e)) } + Result.failure(e) + } } + } - if (shouldRetry) { - Logger.warn("Start error, will retry...", e, context = TAG) - _lightningState.update { it.copy(nodeLifecycleState = initialLifecycleState) } - shouldRetryStart = true - Result.failure(e) - } else { - _lightningState.update { it.copy(nodeLifecycleState = NodeLifecycleState.ErrorStarting(e)) } - Result.failure(e) + // Retry OUTSIDE the mutex to avoid deadlock (Kotlin Mutex is non-reentrant) + if (shouldRetryStart) { + delay(2.seconds) + // A stop requested after this start began wins over the retry; a foreground start cancels it + if (shouldRetryYieldToStop && pendingStopJob.get() != null) { + Logger.info("Skipped start retry because a stop was requested", context = TAG) + return@withContext Result.failure(NodeStartYieldedToStopError(result.exceptionOrNull())) } + return@withContext startNode( + walletIndex = walletIndex, + timeout = timeout, + shouldRetry = false, + customServerUrl = customServerUrl, + customRgsServerUrl = customRgsServerUrl, + channelMigration = channelMigration, + shouldValidateGraph = shouldValidateGraph, + shouldCancelPendingStop = !shouldRetryYieldToStop, + ) } - } - // Retry OUTSIDE the mutex to avoid deadlock (Kotlin Mutex is non-reentrant) - if (shouldRetryStart) { - delay(2.seconds) - // A stop requested after this start began wins over the retry; a foreground start cancels it - if (shouldRetryYieldToStop && pendingStopJob.get() != null) { - Logger.info("Skipped start retry because a stop was requested", context = TAG) - return@withContext Result.failure(NodeStartYieldedToStopError(result.exceptionOrNull())) + // Restart after graph reset OUTSIDE the mutex to avoid deadlock + if (shouldRestartForGraphReset) { + return@withContext start( + walletIndex = walletIndex, + timeout = timeout, + shouldRetry = shouldRetry, + customServerUrl = customServerUrl, + customRgsServerUrl = customRgsServerUrl, + eventHandler = eventHandler, + channelMigration = channelMigration, + shouldValidateGraph = false, // Prevent infinite loop + ) } - return@withContext startNode( - walletIndex = walletIndex, - timeout = timeout, - shouldRetry = false, - customServerUrl = customServerUrl, - customRgsServerUrl = customRgsServerUrl, - channelMigration = channelMigration, - shouldValidateGraph = shouldValidateGraph, - shouldCancelPendingStop = !shouldRetryYieldToStop, - ) - } - // Restart after graph reset OUTSIDE the mutex to avoid deadlock - if (shouldRestartForGraphReset) { - return@withContext start( - walletIndex = walletIndex, - timeout = timeout, - shouldRetry = shouldRetry, - customServerUrl = customServerUrl, - customRgsServerUrl = customRgsServerUrl, - eventHandler = eventHandler, - channelMigration = channelMigration, - shouldValidateGraph = false, // Prevent infinite loop - ) + result } - - result } private suspend fun adoptRunningNode(): Result { @@ -729,14 +737,16 @@ class LightningRepo @Inject constructor( } } - suspend fun resetNetworkGraph(walletIndex: Int = 0): Result = withContext(bgDispatcher) { - Logger.warn("Resetting network graph (manual)", context = TAG) - runCatching { - if (lightningService.node != null) { - lightningService.stop() + suspend fun resetNetworkGraph(walletIndex: Int = 0): Result = vssClients.withWallet { + withContext(bgDispatcher) { + Logger.warn("Resetting network graph (manual)", context = TAG) + runSuspendCatching { + if (lightningService.node != null) { + lightningService.stop() + } + // Propagate VSS failures: a manual reset that leaves the graph in VSS is ineffective. + clearNetworkGraph(walletIndex).getOrThrow() } - // Propagate VSS failures: a manual reset that leaves the graph in VSS is ineffective. - clearNetworkGraph(walletIndex).getOrThrow() } } @@ -749,8 +759,7 @@ class LightningRepo @Inject constructor( } return runSuspendCatching { - vssBackupClientLdk.setup(walletIndex).getOrThrow() - vssBackupClientLdk.deleteObject("network_graph").getOrThrow() + vssBackupClientLdk.deleteObjects(listOf("network_graph"), walletIndex).getOrThrow() Logger.info("Cleared network graph from VSS", context = TAG) }.onFailure { Logger.warn("Failed to clear network graph from VSS", it, context = TAG) @@ -2451,100 +2460,110 @@ class LightningRepo @Inject constructor( * so callers can require any scores sync timestamp to be strictly newer to prove a post-reset download. */ @OptIn(ExperimentalTime::class) - suspend fun resetPathfindingScores(walletIndex: Int = 0): Result = withContext(bgDispatcher) { - Logger.info("Resetting pathfinding scores", context = TAG) - - waitForNodeToStop().onFailure { return@withContext Result.failure(it) } - stop().onFailure { - Logger.error("Failed to stop node during pathfinding scores reset", it, context = TAG) - return@withContext Result.failure(it) - } + suspend fun resetPathfindingScores(walletIndex: Int = 0): Result = vssClients.withWallet { + withContext(bgDispatcher) { + Logger.info("Resetting pathfinding scores", context = TAG) - runCatching { - val lifecycleState = _lightningState.value.nodeLifecycleState - check(lifecycleState == NodeLifecycleState.Stopped) { - "Node lifecycle changed to '$lifecycleState' during pathfinding scores reset" + waitForNodeToStop().onFailure { return@withContext Result.failure(it) } + stop().onFailure { + Logger.error("Failed to stop node during pathfinding scores reset", it, context = TAG) + return@withContext Result.failure(it) } - // Gate the destructive VSS deletes on the previous node's release, so the old node cannot - // re-persist scores over the delete while it is still draining. - lightningService.awaitNodeRelease() - vssBackupClientLdk.setup(walletIndex).getOrThrow() - vssBackupClientLdk.deleteObject(VSS_KEY_SCORER).getOrThrow() - vssBackupClientLdk.deleteObject(VSS_KEY_EXTERNAL_SCORES_CACHE).getOrThrow() - }.onFailure { - Logger.error("Failed to delete pathfinding scores from VSS", it, context = TAG) - start(walletIndex = walletIndex, shouldRetry = false, shouldValidateGraph = false).onFailure { startError -> - Logger.error("Failed to restart node after pathfinding scores reset failure", startError, context = TAG) + + runSuspendCatching { + val lifecycleState = _lightningState.value.nodeLifecycleState + check(lifecycleState == NodeLifecycleState.Stopped) { + "Node lifecycle changed to '$lifecycleState' during pathfinding scores reset" + } + // Gate the destructive VSS deletes on the previous node's release, so the old node cannot + // re-persist scores over the delete while it is still draining. + lightningService.awaitNodeRelease() + vssBackupClientLdk.deleteObjects( + listOf(VSS_KEY_SCORER, VSS_KEY_EXTERNAL_SCORES_CACHE), + walletIndex, + ).getOrThrow() + }.onFailure { + Logger.error("Failed to delete pathfinding scores from VSS", it, context = TAG) + start(walletIndex = walletIndex, shouldRetry = false, shouldValidateGraph = false) + .onFailure { startError -> + Logger.error( + "Failed to restart node after pathfinding scores reset failure", + startError, + context = TAG, + ) + } + return@withContext Result.failure(it) } - return@withContext Result.failure(it) - } - val resetAtSecs = nowMillis() / 1000 + val resetAtSecs = nowMillis() / 1000 - start(walletIndex = walletIndex, shouldRetry = false, shouldValidateGraph = false) - .map { resetAtSecs } - .onSuccess { - Logger.info("Pathfinding scores reset at '$resetAtSecs'", context = TAG) - } + start(walletIndex = walletIndex, shouldRetry = false, shouldValidateGraph = false) + .map { resetAtSecs } + .onSuccess { + Logger.info("Pathfinding scores reset at '$resetAtSecs'", context = TAG) + } + } } - suspend fun resetPaymentRoutingCachesAndWait(walletIndex: Int = 0): Result = withContext(bgDispatcher) { - val refreshStartedAtMs = nowMillis() - val refreshStartedAtSecs = (refreshStartedAtMs / 1000).toULong() - val requiresRgsRefresh = Env.network != Network.REGTEST && - !settingsStore.data.first().rgsServerUrl.isNullOrEmpty() - val requiresScorerRefresh = Env.ldkScorerUrl != null - val resetErrors = mutableListOf() + suspend fun resetPaymentRoutingCachesAndWait(walletIndex: Int = 0): Result = vssClients.withWallet { + withContext(bgDispatcher) { + val refreshStartedAtMs = nowMillis() + val refreshStartedAtSecs = (refreshStartedAtMs / 1000).toULong() + val requiresRgsRefresh = Env.network != Network.REGTEST && + !settingsStore.data.first().rgsServerUrl.isNullOrEmpty() + val requiresScorerRefresh = Env.ldkScorerUrl != null + val resetErrors = mutableListOf() - Logger.info( - "Started payment routing refresh rgs='$requiresRgsRefresh' scorer='$requiresScorerRefresh'", - context = TAG, - ) + Logger.info( + "Started payment routing refresh rgs='$requiresRgsRefresh' scorer='$requiresScorerRefresh'", + context = TAG, + ) - val resetResult = withContext(NonCancellable) reset@{ - stop().onFailure { - start( - walletIndex = walletIndex, - shouldRetry = false, - shouldValidateGraph = false, - ).onFailure { startError -> - Logger.error( - "Failed to restart node after payment routing refresh stop failure", - startError, - context = TAG, - ) + val resetResult = withContext(NonCancellable) reset@{ + stop().onFailure { + start( + walletIndex = walletIndex, + shouldRetry = false, + shouldValidateGraph = false, + ).onFailure { startError -> + Logger.error( + "Failed to restart node after payment routing refresh stop failure", + startError, + context = TAG, + ) + } + return@reset Result.failure(it) } - return@reset Result.failure(it) - } - clearNetworkGraph(walletIndex).onFailure { - resetErrors.add(it) - } + clearNetworkGraph(walletIndex).onFailure { + resetErrors.add(it) + } - resetPathfindingScores(walletIndex).onFailure { - resetErrors.add(it) - } + resetPathfindingScores(walletIndex).onFailure { + resetErrors.add(it) + } - resetErrors.firstOrNull()?.let { - Result.failure(it) - } ?: Result.success(Unit) - } - resetResult.onFailure { - return@withContext Result.failure(it) - } + resetErrors.firstOrNull()?.let { + Result.failure(it) + } ?: Result.success(Unit) + } + resetResult.onFailure { + return@withContext Result.failure(it) + } - val result = waitForPaymentRoutingDataRefresh( - walletIndex = walletIndex, - refreshStartedAtMs = refreshStartedAtMs, - refreshStartedAtSecs = refreshStartedAtSecs, - requiresRgsRefresh = requiresRgsRefresh, - requiresScorerRefresh = requiresScorerRefresh, - ) - result.onSuccess { - Logger.info( - "Finished payment routing refresh elapsedMs='${nowMillis() - refreshStartedAtMs}'", - context = TAG, + val result = waitForPaymentRoutingDataRefresh( + walletIndex = walletIndex, + refreshStartedAtMs = refreshStartedAtMs, + refreshStartedAtSecs = refreshStartedAtSecs, + requiresRgsRefresh = requiresRgsRefresh, + requiresScorerRefresh = requiresScorerRefresh, ) + result.onSuccess { + Logger.info( + "Finished payment routing refresh elapsedMs='${nowMillis() - refreshStartedAtMs}'", + context = TAG, + ) + } } } @@ -2605,30 +2624,35 @@ class LightningRepo @Inject constructor( * The caller still awaits the [Result], so a live caller keeps reporting the outcome; a cancelled * one only loses the reporting, never the restart. */ - suspend fun restartNodeDetached(): Result = scope.async { restartNode() }.await() + suspend fun restartNodeDetached(): Result { + val owner = vssClients.captureOwner() + return scope.async { vssClients.withWallet(owner) { restartNode() } }.await() + } suspend fun restartNode( channelMigration: ChannelDataMigration? = null, walletIndex: Int = 0, - ): Result = withContext(bgDispatcher) { - Logger.info("Restarting node", context = TAG) - stop().onFailure { - Logger.error("Failed to stop node during restart", it, context = TAG) - return@withContext Result.failure(it) - } - startNode( - walletIndex = walletIndex, - channelMigration = channelMigration, - shouldRetryYieldToStop = true - ).onFailure { - if (it is NodeStartYieldedToStopError) { - Logger.info("Deferred node restart to a requested stop", context = TAG) - } else { - Logger.error("Failed to start node during restart", it, context = TAG) + ): Result = vssClients.withWallet { + withContext(bgDispatcher) { + Logger.info("Restarting node", context = TAG) + stop().onFailure { + Logger.error("Failed to stop node during restart", it, context = TAG) + return@withContext Result.failure(it) + } + startNode( + walletIndex = walletIndex, + channelMigration = channelMigration, + shouldRetryYieldToStop = true + ).onFailure { + if (it is NodeStartYieldedToStopError) { + Logger.info("Deferred node restart to a requested stop", context = TAG) + } else { + Logger.error("Failed to start node during restart", it, context = TAG) + } + return@withContext Result.failure(it) + }.onSuccess { + Logger.info("Node restarted successfully", context = TAG) } - return@withContext Result.failure(it) - }.onSuccess { - Logger.info("Node restarted successfully", context = TAG) } } diff --git a/app/src/main/java/to/bitkit/viewmodels/BackupsViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/BackupsViewModel.kt index 350c2c716b..01d68197f7 100644 --- a/app/src/main/java/to/bitkit/viewmodels/BackupsViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/BackupsViewModel.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import to.bitkit.data.CacheStore +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.models.BackupCategory import to.bitkit.models.BackupItemStatus import to.bitkit.models.NodeLifecycleState @@ -25,6 +26,7 @@ class BackupsViewModel @Inject constructor( private val cacheStore: CacheStore, private val backupRepo: BackupRepo, private val lightningRepo: LightningRepo, + private val vssClients: VssClientCoordinator, ) : ViewModel() { private val _uiState = MutableStateFlow(BackupStatusUiState()) @@ -63,9 +65,12 @@ class BackupsViewModel @Inject constructor( } fun retryBackup(category: BackupCategory) { + val owner = runCatching { vssClients.captureOwner() }.getOrNull() ?: return viewModelScope.launch { - delay(500) // small delay for UX feedback - backupRepo.triggerBackup(category) + vssClients.withWallet(owner) { + delay(500) // small delay for UX feedback + backupRepo.triggerBackup(category) + } } } } diff --git a/app/src/test/java/to/bitkit/data/WalletConnectionRecoveryTest.kt b/app/src/test/java/to/bitkit/data/WalletConnectionRecoveryTest.kt index 5324176595..ef5943b176 100644 --- a/app/src/test/java/to/bitkit/data/WalletConnectionRecoveryTest.kt +++ b/app/src/test/java/to/bitkit/data/WalletConnectionRecoveryTest.kt @@ -5,12 +5,15 @@ import android.content.Context import androidx.test.core.app.ApplicationProvider import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.async +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.withContext import org.junit.After import org.junit.Before import org.junit.Test @@ -22,6 +25,7 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.entities.ConfigEntity import to.bitkit.data.keychain.Keychain @@ -45,6 +49,7 @@ class WalletConnectionRecoveryTest : BaseUnitTest() { private val keychain = mock() private val db = mock() private val binding = mock() + private val vssClients = VssClientCoordinator() private val config = MutableStateFlow(listOf(ConfigEntity())) private var record: String? = null private var pending = false @@ -82,7 +87,47 @@ class WalletConnectionRecoveryTest : BaseUnitTest() { @After fun tearDown() = runBlocking { settings.reset() } - private fun instance(store: SettingsStore = settings) = WalletConnectionRecovery(keychain, db, binding, store) + private fun instance(store: SettingsStore = settings) = WalletConnectionRecovery( + keychain, + db, + binding, + store, + vssClients, + ) + + @Test + fun `identity change waits for VSS cleanup and keeps admission closed through credentials`() = test { + val cleanup = CompletableDeferred() + var changed = false + var drained = false + val old = launch { + vssClients.withWallet { + try { + awaitCancellation() + } finally { + withContext(NonCancellable) { + cleanup.await() + drained = true + } + } + } + } + runCurrent() + val transition = launch { + sut.withIdentityChange { + assertTrue(drained) + assertFailsWith { vssClients.withWallet {} } + changed = true + } + } + runCurrent() + assertFalse(changed) + cleanup.complete(Unit) + transition.join() + assertTrue(changed) + old.join() + vssClients.withWallet {} + } @Test fun `queued wallet generation is revoked during same seed replacement`() = test { diff --git a/app/src/test/java/to/bitkit/data/backup/VssBackupClientLdkTest.kt b/app/src/test/java/to/bitkit/data/backup/VssBackupClientLdkTest.kt index f80e27d0b0..6b8c919349 100644 --- a/app/src/test/java/to/bitkit/data/backup/VssBackupClientLdkTest.kt +++ b/app/src/test/java/to/bitkit/data/backup/VssBackupClientLdkTest.kt @@ -1,14 +1,16 @@ package to.bitkit.data.backup +import com.synonym.vssclient.vssDeriveStoreId import kotlinx.coroutines.runBlocking import org.junit.Before import org.junit.Test import org.mockito.Mockito.mockStatic -import org.mockito.kotlin.any import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import to.bitkit.data.keychain.Keychain +import to.bitkit.env.Env import to.bitkit.test.BaseUnitTest +import kotlin.test.assertEquals import kotlin.test.assertIs import kotlin.test.assertNull import kotlin.test.assertTrue @@ -17,27 +19,55 @@ class VssBackupClientLdkTest : BaseUnitTest() { private lateinit var sut: VssBackupClientLdk - private val vssStoreIdProvider = mock() private val keychain = mock() @Before fun setUp() = runBlocking { sut = VssBackupClientLdk( ioDispatcher = testDispatcher, - vssStoreIdProvider = vssStoreIdProvider, + coordinator = VssClientCoordinator(), keychain = keychain, ) } + @Test + fun `setup derives store id from the same captured credentials as the signer`() = test { + whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(TEST_MNEMONIC) + whenever(keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)).thenReturn("test-passphrase") + var setupArguments: List? = null + var deriveArguments: List? = null + mockStatic(Class.forName(VSS_FFI_CLASS)) { invocation -> + when (invocation.method.name) { + "vssDeriveStoreId" -> { + deriveArguments = invocation.arguments.toList() + "captured-store" + } + "vssNewLdkClientWithLnurlAuth" -> { + setupArguments = invocation.arguments.take(5) + null + } + else -> null + } + }.use { + assertTrue(sut.setup().isSuccess) + assertEquals(listOf(Env.vssStoreIdPrefix, TEST_MNEMONIC, "test-passphrase"), deriveArguments) + assertEquals( + listOf(Env.vssServerUrl, "captured-store", TEST_MNEMONIC, "test-passphrase", Env.lnurlAuthServerUrl), + setupArguments, + ) + } + } + @Test fun `setup succeeding after a failure leaves the client usable`() = test { whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null) assertIs(sut.setup().exceptionOrNull()) whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(TEST_MNEMONIC) - whenever(vssStoreIdProvider.getVssStoreId(any())).thenReturn("test-store-id") mockStatic(Class.forName(VSS_FFI_CLASS)).use { + whenever(vssDeriveStoreId(Env.vssStoreIdPrefix, TEST_MNEMONIC, null)) + .thenReturn("captured-store") assertTrue(sut.setup().isSuccess) val result = sut.getObject("key") diff --git a/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt b/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt index 1ab0912034..abb2309777 100644 --- a/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt +++ b/app/src/test/java/to/bitkit/data/backup/VssBackupClientTest.kt @@ -1,6 +1,7 @@ package to.bitkit.data.backup import com.synonym.vssclient.VssItem +import com.synonym.vssclient.vssDeriveStoreId import com.synonym.vssclient.vssStore import kotlinx.coroutines.runBlocking import org.junit.Before @@ -8,10 +9,9 @@ import org.junit.Test import org.mockito.Mockito.mockStatic import org.mockito.kotlin.any import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import to.bitkit.data.keychain.Keychain +import to.bitkit.env.Env import to.bitkit.test.BaseUnitTest import kotlin.test.assertEquals import kotlin.test.assertIs @@ -22,14 +22,13 @@ class VssBackupClientTest : BaseUnitTest() { private lateinit var sut: VssBackupClient - private val vssStoreIdProvider = mock() private val keychain = mock() @Before fun setUp() = runBlocking { sut = VssBackupClient( ioDispatcher = testDispatcher, - vssStoreIdProvider = vssStoreIdProvider, + coordinator = VssClientCoordinator(), keychain = keychain, ) } @@ -45,23 +44,31 @@ class VssBackupClientTest : BaseUnitTest() { } @Test - fun `setup does not call vssStoreIdProvider when mnemonic is not available`() = test { - whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(null) - - sut.setup() - - verify(vssStoreIdProvider, never()).getVssStoreId(any()) - } - - @Test - fun `setup checks mnemonic before proceeding with vss initialization`() = test { + fun `setup derives store id from the same captured credentials as the signer`() = test { whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(TEST_MNEMONIC) - whenever(vssStoreIdProvider.getVssStoreId(any())).thenReturn("test-store-id") - - // Setup will fail on native VSS calls, but we verify we passed the mnemonic check - runCatching { sut.setup() } - - verify(vssStoreIdProvider).getVssStoreId(any()) + whenever(keychain.loadString(Keychain.Key.BIP39_PASSPHRASE.name)).thenReturn("test-passphrase") + var setupArguments: List? = null + var deriveArguments: List? = null + mockStatic(Class.forName(VSS_FFI_CLASS)) { invocation -> + when (invocation.method.name) { + "vssDeriveStoreId" -> { + deriveArguments = invocation.arguments.toList() + "captured-store" + } + "vssNewClientWithLnurlAuth" -> { + setupArguments = invocation.arguments.take(5) + null + } + else -> null + } + }.use { + assertTrue(sut.setup().isSuccess) + assertEquals(listOf(Env.vssStoreIdPrefix, TEST_MNEMONIC, "test-passphrase"), deriveArguments) + assertEquals( + listOf(Env.vssServerUrl, "captured-store", TEST_MNEMONIC, "test-passphrase", Env.lnurlAuthServerUrl), + setupArguments, + ) + } } @Test @@ -80,9 +87,10 @@ class VssBackupClientTest : BaseUnitTest() { assertIs(sut.setup().exceptionOrNull()) whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(TEST_MNEMONIC) - whenever(vssStoreIdProvider.getVssStoreId(any())).thenReturn("test-store-id") mockStatic(Class.forName(VSS_FFI_CLASS)).use { + whenever(vssDeriveStoreId(Env.vssStoreIdPrefix, TEST_MNEMONIC, null)) + .thenReturn("captured-store") assertTrue(sut.setup().isSuccess) val result = sut.getObject("METADATA") @@ -97,9 +105,10 @@ class VssBackupClientTest : BaseUnitTest() { whenever(keychain.loadString(Keychain.Key.BIP39_MNEMONIC.name)) .thenReturn(null) .thenReturn(TEST_MNEMONIC) - whenever(vssStoreIdProvider.getVssStoreId(any())).thenReturn("test-store-id") mockStatic(Class.forName(VSS_FFI_CLASS)).use { + whenever(vssDeriveStoreId(Env.vssStoreIdPrefix, TEST_MNEMONIC, null)) + .thenReturn("captured-store") assertTrue(sut.setupWithRetry(baseDelayMs = 0L) {}.isSuccess) val item = VssItem("METADATA", byteArrayOf(), 1L) diff --git a/app/src/test/java/to/bitkit/data/backup/VssClientCoordinatorTest.kt b/app/src/test/java/to/bitkit/data/backup/VssClientCoordinatorTest.kt new file mode 100644 index 0000000000..437d300ea7 --- /dev/null +++ b/app/src/test/java/to/bitkit/data/backup/VssClientCoordinatorTest.kt @@ -0,0 +1,188 @@ +package to.bitkit.data.backup + +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.withContext +import org.junit.Test +import to.bitkit.test.BaseUnitTest +import to.bitkit.utils.AppError +import kotlin.coroutines.cancellation.CancellationException +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@OptIn(ExperimentalCoroutinesApi::class) +class VssClientCoordinatorTest : BaseUnitTest() { + private val sut = VssClientCoordinator() + + @Test + fun `retirement waits for cancelled setup cleanup before shutdown and replacement`() = test { + val cleanup = CompletableDeferred() + var shutdown = false + val old = launch { + sut.setup(VssClientCoordinator.Domain.APP) { + try { + awaitCancellation() + } finally { + withContext(NonCancellable) { cleanup.await() } + } + } + } + runCurrent() + val transition = launch { sut.withSuspendedClients({ shutdown = true }) {} } + runCurrent() + assertFalse(shutdown) + assertFalse(transition.isCompleted) + assertFailsWith { sut.withWallet {} } + cleanup.complete(Unit) + transition.join() + assertTrue(old.isCancelled) + assertTrue(shutdown) + sut.setup(VssClientCoordinator.Domain.APP) {} + assertEquals("replacement", sut.withClient(VssClientCoordinator.Domain.APP) { "replacement" }) + } + + @Test + fun `both domains and local result tails finish before credential change`() = test { + VssClientCoordinator.Domain.entries.forEach { sut.setup(it) {} } + val release = CompletableDeferred() + val finished = mutableSetOf() + val shutdown = mutableSetOf() + VssClientCoordinator.Domain.entries.forEach { domain -> + launch { + sut.withClient(domain) { + try { + awaitCancellation() + } finally { + withContext(NonCancellable) { + release.await() + finished.add(domain) + } + } + } + } + } + runCurrent() + val transition = launch { + sut.withSuspendedClients({ shutdown.add(it) }) { + assertEquals(VssClientCoordinator.Domain.entries.toSet(), finished) + assertEquals(finished, shutdown) + assertFailsWith { sut.captureOwner() } + } + } + runCurrent() + assertTrue(shutdown.isEmpty()) + release.complete(Unit) + transition.join() + } + + @Test + fun `old payload and compound followup cannot enter replacement client`() = test { + sut.setup(VssClientCoordinator.Domain.APP) {} + val owner = sut.captureOwner() + var calls = 0 + val followup = CompletableDeferred() + val original = launch { + sut.withWallet(owner) { + sut.withClient(VssClientCoordinator.Domain.APP) { calls++ } + followup.await() + sut.withClient(VssClientCoordinator.Domain.APP) { calls++ } + } + } + runCurrent() + sut.withSuspendedClients({}) {} + assertTrue(original.isCancelled) + sut.setup(VssClientCoordinator.Domain.APP) {} + assertFailsWith { sut.withWallet(owner) { calls++ } } + assertEquals(1, calls) + } + + @Test + fun `cancelled retirement still drains and reopens without changing credentials`() = test { + val cleanup = CompletableDeferred() + var changed = false + launch { + sut.withWallet { + try { + awaitCancellation() + } finally { + withContext(NonCancellable) { cleanup.await() } + } + } + } + runCurrent() + val transition = launch { sut.withSuspendedClients({}) { changed = true } } + runCurrent() + transition.cancel() + assertFailsWith { sut.captureOwner() } + cleanup.complete(Unit) + transition.join() + assertFalse(changed) + sut.withWallet {} + } + + @Test + fun `credential failure reopens uninitialized slots and invalidates previous owner`() = test { + val original = sut.captureOwner() + assertFailsWith { sut.withSuspendedClients({}) { throw FixtureFailure() } } + assertFailsWith { sut.withWallet(original) {} } + var initialized = false + sut.setup(VssClientCoordinator.Domain.APP) { initialized = true } + assertTrue(initialized) + } + + @Test + fun `setup waiters share successful setup and calls can overlap`() = test { + val setupRelease = CompletableDeferred() + var setups = 0 + val first = launch { + sut.setup(VssClientCoordinator.Domain.APP) { + setups++ + setupRelease.await() + } + } + val second = launch { sut.setup(VssClientCoordinator.Domain.APP) { setups++ } } + runCurrent() + setupRelease.complete(Unit) + first.join() + second.join() + assertEquals(1, setups) + val release = CompletableDeferred() + var entered = 0 + val calls = List(2) { + async { + sut.withClient(VssClientCoordinator.Domain.APP) { + entered++ + release.await() + } + } + } + runCurrent() + assertEquals(2, entered) + release.complete(Unit) + calls.forEach { it.await() } + } + + @Test + fun `retirement wakes calls waiting for explicit setup`() = test { + val waiting = launch { sut.withClient(VssClientCoordinator.Domain.APP) { error("Not initialized") } } + runCurrent() + sut.withSuspendedClients({}) {} + assertTrue(waiting.isCancelled) + } + + @Test + fun `shutdown failure leaves admission closed`() = test { + sut.setup(VssClientCoordinator.Domain.APP) {} + assertFailsWith { sut.withSuspendedClients({ throw FixtureFailure() }) {} } + assertFailsWith { sut.withWallet {} } + } + + private class FixtureFailure : AppError("fixture failure") +} diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index b0591370db..1570a36cce 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -65,6 +65,7 @@ import to.bitkit.data.backup.UsdtRecoveryBackupStore import to.bitkit.data.backup.UsdtRecoveryBackupV1 import to.bitkit.data.backup.VssBackupClient import to.bitkit.data.backup.VssBackupClientLdk +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.dao.TransferDao import to.bitkit.data.entities.TransferEntity import to.bitkit.data.keychain.Keychain @@ -115,6 +116,7 @@ class BackupRepoTest : BaseUnitTest() { private val context = mock() private val cacheStore = mock() private val vssBackupClient = mock() + private val vssClients = VssClientCoordinator() private val vssBackupClientLdk = mock() private val settingsStore = mock() private val widgetsStore = mock() @@ -177,6 +179,7 @@ class BackupRepoTest : BaseUnitTest() { whenever { cacheStore.updateBackupStatus(any(), any()) }.thenReturn(Unit) whenever { cacheStore.update(any()) }.thenReturn(Unit) whenever(cacheStore.data).thenReturn(cacheData) + whenever(cacheStore.backupStatuses).thenReturn(cacheData.map { it.backupStatuses }) whenever(settingsStore.data).thenReturn(settingsData) whenever { settingsStore.update(any()) }.thenReturn(Unit) whenever(widgetsStore.data).thenReturn(widgetsData) @@ -222,6 +225,65 @@ class BackupRepoTest : BaseUnitTest() { sut = createSut() } + @Test + fun `explicit first wallet backup does not wait for node observers`() = test { + val statuses = MutableStateFlow(mapOf(BackupCategory.WALLET to BackupItemStatus(required = 1_000))) + stubBackupStatuses(statuses, CompletableDeferred(Unit)) {} + sut.scheduleFullBackup() + advanceTimeBy(5_000) + runCurrent() + verify(vssBackupClient).setup() + verify(vssBackupClient, times(1)).putObject(eq(BackupCategory.WALLET.name), any()) + verify(vssBackupClient, never()).setupWithRetry(any(), any(), any()) + } + + @Test + fun `observer bootstrap requeues a required backup after a failed identity transition`() = test { + val statuses = MutableStateFlow(mapOf(BackupCategory.SETTINGS to BackupItemStatus(required = 1_000))) + stubBackupStatuses(statuses, CompletableDeferred(Unit)) {} + stubBackupObservers() + try { + sut.startObservingBackups() + runCurrent() + verify(vssBackupClient, times(1)).setupWithRetry(any(), any(), any()) + val recovery = connectionRecoveryFixture(settingsStore, keychain, vssClients) + assertFailsWith { + recovery.withIdentityChange { throw BackupRepoTestError("credentials unchanged") } + } + runCurrent() + verify(vssBackupClient, times(2)).setupWithRetry(any(), any(), any()) + verify(vssBackupClientLdk, times(2)).setup() + advanceTimeBy(5_000) + runCurrent() + verify(vssBackupClient, times(1)).putObject(eq(BackupCategory.SETTINGS.name), any()) + } finally { + sut.stopObservingBackups() + } + } + + @Test + fun `backup failure status drains before replacement credentials`() = test { + val entered = CompletableDeferred() + val release = CompletableDeferred() + var replaced = false + var crossed = false + whenever(cacheStore.updateBackupStatus(eq(BackupCategory.WALLET), any())).doSuspendableAnswer { + entered.complete(Unit) + release.await() + crossed = replaced + Unit + } + whenever(pubkyRepo.hasIdentityForBackup()).thenReturn(true) + assertTrue(sut.persistWalletBackup().isFailure) + entered.await() + val recovery = connectionRecoveryFixture(settingsStore, keychain, vssClients) + recovery.withIdentityChange { replaced = true } + release.complete(Unit) + runCurrent() + assertTrue(replaced) + assertFalse(crossed) + } + @Test fun `metadata backup preserves complete data when identity records cannot be read`() = test { stubMetadataBackupReads() @@ -3066,6 +3128,7 @@ class BackupRepoTest : BaseUnitTest() { ioDispatcher = ioDispatcher, cacheStore = cacheStore, vssBackupClient = vssBackupClient, + vssClients = vssClients, vssBackupClientLdk = vssBackupClientLdk, settingsStore = settingsStore, widgetsStore = widgetsStore, @@ -3094,7 +3157,7 @@ class BackupRepoTest : BaseUnitTest() { lightningService = lightningService, clock = clock, db = db, - connectionRecovery = connectionRecoveryFixture(settingsStore, keychain), + connectionRecovery = connectionRecoveryFixture(settingsStore, keychain, vssClients), ) private class BackupRepoTestError(message: String) : AppError(message) diff --git a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt index 18c097ea37..c4aab73634 100644 --- a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt @@ -68,6 +68,7 @@ import to.bitkit.data.CacheStore import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore import to.bitkit.data.backup.VssBackupClientLdk +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.keychain.Keychain import to.bitkit.data.keychain.WalletBackupRestorePendingError import to.bitkit.ext.createChannelDetails @@ -132,6 +133,7 @@ class LightningRepoTest : BaseUnitTest() { private val lnurlService = mock() private val connectivityRepo = mock() private val vssBackupClientLdk = mock() + private val vssClients = VssClientCoordinator() private val electrumProbeService = mock() private val urlValidator = UrlValidator { Result.success(Unit) } private val probePaymentA = "probe-payment-a" @@ -161,7 +163,7 @@ class LightningRepoTest : BaseUnitTest() { preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = onchainSendAttemptStore, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = urlValidator, electrumProbeService = electrumProbeService, ) @@ -1034,23 +1036,61 @@ class LightningRepoTest : BaseUnitTest() { assertEquals(NodeLifecycleState.Stopped, sut.lightningState.value.nodeLifecycleState) } + @Test + fun `detached restart drains its old stop without starting replacement wallet`() = test { + startNodeForTesting() + val entered = CompletableDeferred() + val release = CompletableDeferred() + whenever(lightningService.stop()).doSuspendableAnswer { + entered.complete(Unit) + release.await() + } + val restart = launch { sut.restartNodeDetached() } + entered.await() + var replaced = false + val recovery = connectionRecoveryFixture(settingsStore, vssClients = vssClients) + val transition = launch { recovery.withIdentityChange { replaced = true } } + runCurrent() + assertFalse(replaced) + release.complete(Unit) + transition.join() + restart.join() + assertTrue(replaced) + assertTrue(restart.isCancelled) + verify(lightningService, times(1)).start(anyOrNull(), any()) + verify(vssBackupClientLdk, never()).deleteObjects(any(), any()) + } + + @Test + fun `manual graph reset cannot delete after wallet identity replacement`() = test { + val entered = CompletableDeferred() + whenever(lightningService.resetNetworkGraph(any())).doSuspendableAnswer { + entered.complete(Unit) + awaitCancellation() + } + val reset = launch { sut.resetNetworkGraph() } + entered.await() + val recovery = connectionRecoveryFixture(settingsStore, vssClients = vssClients) + recovery.withIdentityChange {} + reset.join() + assertTrue(reset.isCancelled) + verify(vssBackupClientLdk, never()).deleteObjects(any(), any()) + } + @Test fun `resetNetworkGraph clears local cache and VSS copy`() = test { - whenever(vssBackupClientLdk.setup(any())).thenReturn(Result.success(Unit)) - whenever(vssBackupClientLdk.deleteObject(any(), any())).thenReturn(Result.success(true)) + whenever(vssBackupClientLdk.deleteObjects(any(), any())).thenReturn(Result.success(Unit)) val result = sut.resetNetworkGraph() assertTrue(result.isSuccess) verify(lightningService).resetNetworkGraph(0) - verify(vssBackupClientLdk).setup(0) - verify(vssBackupClientLdk).deleteObject(eq("network_graph"), any()) + verify(vssBackupClientLdk).deleteObjects(listOf("network_graph"), 0) } @Test fun `resetNetworkGraph fails when VSS delete fails`() = test { - whenever(vssBackupClientLdk.setup(any())).thenReturn(Result.success(Unit)) - whenever(vssBackupClientLdk.deleteObject(any(), any())) + whenever(vssBackupClientLdk.deleteObjects(any(), any())) .thenReturn(Result.failure(RuntimeException("vss unavailable"))) val result = sut.resetNetworkGraph() @@ -2082,7 +2122,7 @@ class LightningRepoTest : BaseUnitTest() { preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = store, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = urlValidator, electrumProbeService = electrumProbeService, ) @@ -2508,7 +2548,7 @@ class LightningRepoTest : BaseUnitTest() { keychain = keychain, lnurlService = lnurlService, cacheStore = cacheStore, preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = store, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, urlValidator = urlValidator, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = urlValidator, electrumProbeService = electrumProbeService, ) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(coinSelectAuto = false))) @@ -2578,7 +2618,7 @@ class LightningRepoTest : BaseUnitTest() { keychain = keychain, lnurlService = lnurlService, cacheStore = cacheStore, preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = store, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, urlValidator = urlValidator, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = urlValidator, electrumProbeService = electrumProbeService, ) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(coinSelectAuto = false))) @@ -2659,7 +2699,7 @@ class LightningRepoTest : BaseUnitTest() { keychain = keychain, lnurlService = lnurlService, cacheStore = cacheStore, preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = store, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, urlValidator = urlValidator, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = urlValidator, electrumProbeService = electrumProbeService, ) whenever(settingsStore.data).thenReturn(flowOf(SettingsData(coinSelectAuto = false))) @@ -3137,7 +3177,7 @@ class LightningRepoTest : BaseUnitTest() { preActivityMetadataRepo = preActivityMetadataRepo, onchainSendAttemptStore = onchainSendAttemptStore, connectivityRepo = connectivityRepo, - vssBackupClientLdk = vssBackupClientLdk, + vssBackupClientLdk = vssBackupClientLdk, vssClients = vssClients, urlValidator = failingValidator, electrumProbeService = electrumProbeService, ) diff --git a/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt b/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt index 5f8ccf5859..373b2ef185 100644 --- a/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt +++ b/app/src/test/java/to/bitkit/test/ConnectionRecoveryFixture.kt @@ -7,6 +7,7 @@ import org.mockito.kotlin.whenever import to.bitkit.data.AppDb import to.bitkit.data.SettingsStore import to.bitkit.data.WalletConnectionRecovery +import to.bitkit.data.backup.VssClientCoordinator import to.bitkit.data.backup.VssStoreIdProvider import to.bitkit.data.entities.ConfigEntity import to.bitkit.data.keychain.Keychain @@ -14,6 +15,7 @@ import to.bitkit.data.keychain.Keychain fun connectionRecoveryFixture( settings: SettingsStore, keychain: Keychain = mock(), + vssClients: VssClientCoordinator = VssClientCoordinator(), currentBinding: () -> String = { "test-wallet-binding" }, ): WalletConnectionRecovery { val db = mock() @@ -21,5 +23,5 @@ fun connectionRecoveryFixture( whenever(db.configDao().getAll()).thenReturn(MutableStateFlow(listOf(ConfigEntity()))) val binding = mock() whenever(binding.getBackupWalletBinding(any())).thenAnswer { currentBinding() } - return WalletConnectionRecovery(keychain, db, binding, settings) + return WalletConnectionRecovery(keychain, db, binding, settings, vssClients) } diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index 7a9d63b22e..280e800320 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without optional profile refresh waits, missing profiles retry after reconnecting without interrupting active setup, incomplete identity backups remain blocked, and allowances resume safely after re-enabling without reviving cancelled work. +Wallet restore finishes without optional profile refresh waits, missing profiles retry after reconnecting without interrupting active setup, incomplete identity backups remain blocked, and allowances resume safely after re-enabling without reviving cancelled work, while backup uploads, retries and routing resets stay with their original wallet during identity changes. From b93444c35968be6d827a00a20132f6a14a947ffd Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 4 Nov 2026 09:19:12 -0500 Subject: [PATCH 10/11] fix: keep queued backups marked as running --- .../java/to/bitkit/repositories/BackupRepo.kt | 40 ++++++--- .../to/bitkit/repositories/BackupRepoTest.kt | 84 ++++++++++++++++++- changelog.d/next/1686.fixed.md | 2 +- journeys/README.md | 1 + journeys/backup/scheduled-backup-status.xml | 16 ++++ 5 files changed, 128 insertions(+), 15 deletions(-) create mode 100644 journeys/backup/scheduled-backup-status.xml diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index ee050e3dca..cc93471e30 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -268,14 +268,12 @@ class BackupRepo @Inject constructor( launchForWallet { BackupCategory.entries.forEach { category -> - if (category !in runningBackups) { - cacheStore.updateBackupStatus(category) { status -> - if (status.running) { - Logger.debug("Clearing stale running flag for: '$category'", context = TAG) - status.copy(running = false) - } else { - status - } + cacheStore.updateBackupStatus(category) { status -> + if (status.running && category !in runningBackups && !pendingBackups.containsKey(category)) { + Logger.debug("Clearing stale running flag for: '$category'", context = TAG) + status.copy(running = false) + } else { + status } } } @@ -534,11 +532,27 @@ class BackupRepo @Inject constructor( scope.launch(start = CoroutineStart.LAZY) { try { vssClients.withWallet(owner) { - delay(BACKUP_DEBOUNCE) - if (category == BackupCategory.WALLET) paykitSdkService.isPaymentSubmissionActive.first { !it } - pendingBackups.remove(category, owner) - val status = cacheStore.backupStatuses.first()[category] ?: BackupItemStatus() - if (status.shouldBackup(category)) triggerBackup(category) + try { + cacheStore.updateBackupStatus(category) { it.copy(running = true) } + delay(BACKUP_DEBOUNCE) + if (category == BackupCategory.WALLET) { + paykitSdkService.isPaymentSubmissionActive.first { !it } + } + pendingBackups.remove(category, owner) + val status = cacheStore.backupStatuses.first()[category] ?: BackupItemStatus() + if (status.copy(running = category in runningBackups).shouldBackup(category)) { + triggerBackup(category) + } + } finally { + withContext(NonCancellable) { + pendingBackups.remove(category, owner) + cacheStore.updateBackupStatus(category) { + it.copy( + running = category in runningBackups || pendingBackups.containsKey(category) + ) + } + } + } } } finally { pendingBackups.remove(category, owner) diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index 1570a36cce..8b41369ed4 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -246,9 +246,13 @@ class BackupRepoTest : BaseUnitTest() { sut.startObservingBackups() runCurrent() verify(vssBackupClient, times(1)).setupWithRetry(any(), any(), any()) + assertTrue(statuses.value.getValue(BackupCategory.SETTINGS).running) val recovery = connectionRecoveryFixture(settingsStore, keychain, vssClients) assertFailsWith { - recovery.withIdentityChange { throw BackupRepoTestError("credentials unchanged") } + recovery.withIdentityChange { + assertFalse(statuses.value.getValue(BackupCategory.SETTINGS).running) + throw BackupRepoTestError("credentials unchanged") + } } runCurrent() verify(vssBackupClient, times(2)).setupWithRetry(any(), any(), any()) @@ -387,6 +391,7 @@ class BackupRepoTest : BaseUnitTest() { runCurrent() assertTrue(statuses.value.getValue(BackupCategory.WALLET).isRequired) + assertTrue(statuses.value.getValue(BackupCategory.WALLET).running) verify(privatePaykitRepo, never()).backupSnapshot() verify(vssBackupClient, never()).putObject(eq(BackupCategory.WALLET.name), any()) @@ -2031,6 +2036,83 @@ class BackupRepoTest : BaseUnitTest() { } } + @Test + fun `scheduled backup stays running until completion or observation stops`() = test { + val category = BackupCategory.SETTINGS + val statuses = MutableStateFlow(mapOf(category to BackupItemStatus(required = 1_000))) + stubBackupStatuses(statuses, CompletableDeferred(Unit)) {} + stubBackupObservers() + try { + sut.startObservingBackups() + runCurrent() + assertTrue(statuses.value.getValue(category).running) + advanceTimeBy(4_999) + runCurrent() + assertTrue(statuses.value.getValue(category).running) + verify(vssBackupClient, never()).putObject(eq(category.name), any()) + + sut.stopObservingBackups() + runCurrent() + assertFalse(statuses.value.getValue(category).running) + assertTrue(statuses.value.getValue(category).isRequired) + + sut.startObservingBackups() + runCurrent() + assertTrue(statuses.value.getValue(category).running) + advanceTimeBy(5_000) + runCurrent() + verify(vssBackupClient).putObject(eq(category.name), any()) + assertFalse(statuses.value.getValue(category).running) + assertFalse(statuses.value.getValue(category).isRequired) + } finally { + sut.stopObservingBackups() + } + } + + @Test + fun `delayed startup cleanup preserves a newly scheduled backup status`() = test { + val category = BackupCategory.SETTINGS + val statuses = MutableStateFlow(mapOf(category to BackupItemStatus(required = 1_000))) + val finishSetup = CompletableDeferred() + val finishCleanup = CompletableDeferred() + var cleanupEntered = false + whenever(vssBackupClient.setupWithRetry(any(), any(), any())).doSuspendableAnswer { + finishSetup.await() + Result.success(Unit) + } + whenever(cacheStore.backupStatuses).thenReturn(statuses) + whenever(cacheStore.updateBackupStatus(any(), any())).doSuspendableAnswer { + val updatedCategory = it.getArgument(0) + val transform = it.getArgument<(BackupItemStatus) -> BackupItemStatus>(1) + if (updatedCategory == category && !cleanupEntered) { + cleanupEntered = true + finishCleanup.await() + } + statuses.update { current -> + current + (updatedCategory to transform(current[updatedCategory] ?: BackupItemStatus())) + } + } + stubBackupObservers() + try { + sut.startObservingBackups() + runCurrent() + assertTrue(cleanupEntered) + assertTrue(statuses.value.getValue(category).running) + + finishCleanup.complete(Unit) + runCurrent() + assertTrue(statuses.value.getValue(category).running) + advanceTimeBy(5_000) + runCurrent() + verify(vssBackupClient).putObject(eq(category.name), any()) + assertFalse(statuses.value.getValue(category).isRequired) + } finally { + finishCleanup.complete(Unit) + finishSetup.complete(Unit) + sut.stopObservingBackups() + } + } + @Test fun `start observing backs up stale required status after clearing running flag`() = test { val backupStatuses = MutableStateFlow( diff --git a/changelog.d/next/1686.fixed.md b/changelog.d/next/1686.fixed.md index 280e800320..5857ddcd17 100644 --- a/changelog.d/next/1686.fixed.md +++ b/changelog.d/next/1686.fixed.md @@ -1 +1 @@ -Wallet restore finishes without optional profile refresh waits, missing profiles retry after reconnecting without interrupting active setup, incomplete identity backups remain blocked, and allowances resume safely after re-enabling without reviving cancelled work, while backup uploads, retries and routing resets stay with their original wallet during identity changes. +Wallet restore finishes without optional profile refresh waits, missing profiles retry after reconnecting without interrupting active setup, incomplete identity backups remain blocked, and allowances resume safely after re-enabling without reviving cancelled work, while backup uploads, retries and routing resets stay with their original wallet during identity changes, and scheduled backups stay marked as running until they finish or stop. diff --git a/journeys/README.md b/journeys/README.md index 5770c36ac8..cc9f3e3023 100644 --- a/journeys/README.md +++ b/journeys/README.md @@ -171,6 +171,7 @@ Known differences in the corpus, as of the iOS port (synonymdev/bitkit-ios#691): | `send/prepared-fee-recovery.md` | Same four manual fault cases and file on both platforms; the recipe records original-send versus replacement fee retention and iOS Max/identity-flow adaptations. These faults are not journey capabilities. | | `settings/electrum-server-error-toasts.xml` | not ported — iOS still shows one generic message for every manual Electrum connect failure | | `onchain-receive/*` | port pending in synonymdev/bitkit-ios#588, which wires `onchainTransactionConfirmed` into the same received-sheet flow but carries no `journeys/` files. Two adaptations when it lands: iOS suppresses replayed historical receives with a `pendingRestoreActivitySeen` flag cleared by the first post-restore on-chain sync, not the one-hour block-timestamp guard used here, so a stale-confirmation step has to drive a restore instead of a clock; and iOS has no foreground-service path, so `confirmed-only-background-notification.xml` has no counterpart | +| `backup/scheduled-backup-status.xml` | Android-only status regression in the VSS ownership scheduler; iOS #1121 changed VSS admission without changing backup scheduling or its running flag. | | `backup/show-mnemonic-long-words.xml` | not ported — the long-word fit is an Android-only change (synonymdev/bitkit-android#633); whether iOS wraps long words at larger text sizes is unchecked | | `security/forgot-pin-nav-bar-inset.xml` | not ported — Android-only fix (synonymdev/bitkit-android#1572): the Android sheet host adds no navigation-bar inset, so each sheet screen pads itself. iOS sheets keep their content inside the bottom safe area | | `deeplinks/pubky-contact.xml` | ported — same contact routing and unlock behavior | diff --git a/journeys/backup/scheduled-backup-status.xml b/journeys/backup/scheduled-backup-status.xml new file mode 100644 index 0000000000..a9eb251f13 --- /dev/null +++ b/journeys/backup/scheduled-backup-status.xml @@ -0,0 +1,16 @@ + + + On an unfunded disposable dev wallet with working staging VSS and Electrum, verify + that a queued backup shows Running during its five-second debounce and becomes + Latest Backup after acknowledgement. Start at Home in English. No profile, + payment, recovery-phrase access or wallet reset is required. Android-only regression: + the iOS VSS ownership change did not alter backup scheduling or its running status. + + + Open Settings, select the General tab, and open Local Currency (testTag "CurrenciesSettings") + Choose a different local currency, then immediately open bitkit://screen/backup-settings + Verify that the Settings row shows Running and does not show Retry while its backup is queued + Wait for the Settings row to show Latest Backup after the VSS acknowledgement + Verify that Settings has no Failed Backup message or Retry button + + From a2c58f2427635719b63263b9441b8d22a7b6c858 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 4 Nov 2026 10:38:10 -0500 Subject: [PATCH 11/11] fix: clear status for unadmitted backups --- .../java/to/bitkit/repositories/BackupRepo.kt | 22 +++-- .../to/bitkit/repositories/BackupRepoTest.kt | 81 +++++++++++++++++++ 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt index cc93471e30..ae971375ff 100644 --- a/app/src/main/java/to/bitkit/repositories/BackupRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/BackupRepo.kt @@ -156,7 +156,12 @@ class BackupRepo @Inject constructor( private val backupJobs = ConcurrentHashMap.newKeySet() private val backupJobLock = Any() - private val pendingBackups = ConcurrentHashMap() + private val pendingBackups = ConcurrentHashMap() + private class PendingBackup(val owner: VssClientCoordinator.Owner) { + @Volatile + var admitted = false + } + private val statusObserverJobs = mutableListOf() private val dataListenerJobs = mutableListOf() private var periodicCheckJob: Job? = null @@ -269,7 +274,7 @@ class BackupRepo @Inject constructor( launchForWallet { BackupCategory.entries.forEach { category -> cacheStore.updateBackupStatus(category) { status -> - if (status.running && category !in runningBackups && !pendingBackups.containsKey(category)) { + if (status.running && category !in runningBackups && pendingBackups[category]?.admitted != true) { Logger.debug("Clearing stale running flag for: '$category'", context = TAG) status.copy(running = false) } else { @@ -523,39 +528,42 @@ class BackupRepo @Inject constructor( val owner = expectedOwner ?: runCatching { vssClients.captureOwner() }.getOrNull() ?: return val job = synchronized(backupJobLock) { if (requireObservation && !isObserving) return + val pending = PendingBackup(owner) var reserved = false pendingBackups.compute(category) { _, previous -> - previous?.takeIf { it.generation >= owner.generation } ?: owner.also { reserved = true } + previous?.takeIf { it.owner.generation >= owner.generation } ?: pending.also { reserved = true } } if (!reserved) return Logger.debug("Scheduling backup for: '$category'", context = TAG) scope.launch(start = CoroutineStart.LAZY) { try { vssClients.withWallet(owner) { + pending.admitted = true try { cacheStore.updateBackupStatus(category) { it.copy(running = true) } delay(BACKUP_DEBOUNCE) if (category == BackupCategory.WALLET) { paykitSdkService.isPaymentSubmissionActive.first { !it } } - pendingBackups.remove(category, owner) + pendingBackups.remove(category, pending) val status = cacheStore.backupStatuses.first()[category] ?: BackupItemStatus() if (status.copy(running = category in runningBackups).shouldBackup(category)) { triggerBackup(category) } } finally { withContext(NonCancellable) { - pendingBackups.remove(category, owner) + pendingBackups.remove(category, pending) cacheStore.updateBackupStatus(category) { it.copy( - running = category in runningBackups || pendingBackups.containsKey(category) + running = category in runningBackups || + pendingBackups[category]?.admitted == true ) } } } } } finally { - pendingBackups.remove(category, owner) + pendingBackups.remove(category, pending) } }.also { scheduled -> backupJobs.add(scheduled) diff --git a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt index 8b41369ed4..42d4eeb1b5 100644 --- a/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/BackupRepoTest.kt @@ -7,8 +7,11 @@ import com.synonym.paykit.PaykitException import com.synonym.vssclient.VssItem import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Delay import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.InternalCoroutinesApi import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.awaitCancellation @@ -20,6 +23,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.currentTime import kotlinx.coroutines.test.runCurrent @@ -98,6 +102,7 @@ import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors import java.util.concurrent.TimeUnit import javax.inject.Provider +import kotlin.coroutines.CoroutineContext import kotlin.coroutines.cancellation.CancellationException import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -265,6 +270,82 @@ class BackupRepoTest : BaseUnitTest() { } } + @Test + fun `retired replacement reservation cannot leave a cancelled backup running`() = test { + for (retireBeforeCleanup in listOf(true, false)) { + val category = BackupCategory.SETTINGS + val statuses = MutableStateFlow(mapOf(category to BackupItemStatus(required = 1_000))) + stubBackupStatuses(statuses, CompletableDeferred(Unit)) {} + stubBackupObservers() + val finishCleanup = CompletableDeferred() + var holdCleanup = false + var cleanupEntered = false + var holdDispatch = false + val queued = mutableListOf>() + val delegate = StandardTestDispatcher(testScheduler) + + @OptIn(InternalCoroutinesApi::class) + val dispatcher = object : CoroutineDispatcher(), Delay by (delegate as Delay) { + override fun dispatch(context: CoroutineContext, block: Runnable) { + if (holdDispatch) queued += context to block else delegate.dispatch(context, block) + } + } + sut = createSut(ioDispatcher = dispatcher) + whenever(cacheStore.updateBackupStatus(any(), any())).doSuspendableAnswer { + val target = it.getArgument(0) + val transform = it.getArgument<(BackupItemStatus) -> BackupItemStatus>(1) + if (target == category && holdCleanup && !cleanupEntered) { + cleanupEntered = true + finishCleanup.await() + } + statuses.update { it + (target to transform(it[target] ?: BackupItemStatus())) } + } + try { + sut.startObservingBackups() + runCurrent() + assertTrue(statuses.value.getValue(category).running) + holdCleanup = true + sut.stopObservingBackups() + runCurrent() + assertTrue(cleanupEntered) + + holdDispatch = true + sut.scheduleFullBackup() + holdDispatch = false + assertTrue(queued.isNotEmpty()) + if (retireBeforeCleanup) { + val retiring = launch(start = CoroutineStart.UNDISPATCHED) { + vssClients.withSuspendedClients(shutdown = {}) { + assertFalse(statuses.value.getValue(category).running) + } + } + assertNull(vssClients.owners.value) + finishCleanup.complete(Unit) + runCurrent() + retiring.join() + } else { + finishCleanup.complete(Unit) + runCurrent() + vssClients.withSuspendedClients(shutdown = {}) { + assertFalse(statuses.value.getValue(category).running) + } + } + queued.forEach { (context, block) -> delegate.dispatch(context, block) } + queued.clear() + runCurrent() + + assertFalse(statuses.value.getValue(category).running) + assertTrue(statuses.value.getValue(category).isRequired) + verify(vssBackupClient, never()).putObject(eq(category.name), any()) + } finally { + holdDispatch = false + finishCleanup.complete(Unit) + queued.forEach { (context, block) -> delegate.dispatch(context, block) } + sut.reset() + } + } + } + @Test fun `backup failure status drains before replacement credentials`() = test { val entered = CompletableDeferred()