From e823cf89abe30a4469c220595ee0317c44b10db4 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:55:16 +0200 Subject: [PATCH 1/7] fix: decouple wallet restore from profile refresh --- Bitkit/AppScene.swift | 2 +- Bitkit/Managers/PubkyProfileManager.swift | 57 +++++++++- .../PrivatePaykitService+Backup.swift | 12 +- BitkitTests/PrivatePaykitServiceTests.swift | 104 +++++++++++++++++ BitkitTests/PubkyProfileManagerTests.swift | 106 ++++++++++++++++++ changelog.d/next/restore-identity.fixed.md | 1 + .../wallet/restore-during-identity-outage.xml | 18 +++ 7 files changed, 290 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/Bitkit/AppScene.swift b/Bitkit/AppScene.swift index 47c9c2995..c39b57cfb 100644 --- a/Bitkit/AppScene.swift +++ b/Bitkit/AppScene.swift @@ -1169,7 +1169,7 @@ struct AppScene: View { widgets.loadSavedWidgets() widgets.objectWillChange.send() - await pubkyProfile.initialize() + pubkyProfile.scheduleInitializationAfterRestore() await startWallet(completingBackupRestore: true) } diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift index 52b60f8cf..d29dd6b1e 100644 --- a/Bitkit/Managers/PubkyProfileManager.swift +++ b/Bitkit/Managers/PubkyProfileManager.swift @@ -108,6 +108,8 @@ class PubkyProfileManager: ObservableObject { private var isSignupInFlight = false private var initializationTask: Task? + private var initializationRevision: UUID? + private var restoreInitialization: (id: UUID, revision: UUID, task: Task)? private var completedRecoveryVersion = 0 private var sessionRestorationDeferred = false private var readOnlySignOutRevision: UUID? @@ -209,30 +211,65 @@ class PubkyProfileManager: ObservableObject { await initialize(mode: .userVisible, initializeSession: initializeSession) } + func scheduleInitializationAfterRestore( + initializeSession: @escaping @Sendable () async throws -> SessionInitializationResult = { + try await PubkyProfileManager.initializePersistedSession() + } + ) { + let revision = Self.sessionRevision + guard restoreInitialization?.revision != revision, Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return } + clearAuthenticatedState(clearCachedProfile: false) + let previousRevision = initializationRevision + let initialization = beginInitialization(mode: .userVisible, initializeSession: initializeSession) + let id = UUID() + let task = Task { + defer { + if restoreInitialization?.id == id { + restoreInitialization = nil + } + } + guard revision == Self.sessionRevision else { return } + await initialization?.value + guard revision == Self.sessionRevision else { return } + if let previousRevision, previousRevision != revision { + await initialize(mode: .userVisible, initializeSession: initializeSession) + } + } + restoreInitialization = (id, revision, task) + } + private func initialize( mode: SessionInitializationMode, initializeSession: @escaping @Sendable () async throws -> SessionInitializationResult ) async { + await beginInitialization(mode: mode, initializeSession: initializeSession)?.value + } + + private func beginInitialization( + mode: SessionInitializationMode, + initializeSession: @escaping @Sendable () async throws -> SessionInitializationResult + ) -> Task? { if let initializationTask { - await initializationTask.value - return + return initializationTask } - guard Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return } + guard Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return nil } isRestoringSession = true let revision = Self.sessionRevision let task = Task { defer { initializationTask = nil + initializationRevision = nil isRestoringSession = false } - guard Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return } + guard revision == Self.sessionRevision, Self.sessionMutationCount == 0, !Self.isProfileDeletionInFlight else { return } await initializeSessionState(mode: mode, initializeSession: initializeSession) if case .automaticRecovery = mode, revision == Self.sessionRevision { completedRecoveryVersion += 1 } } initializationTask = task - await task.value + initializationRevision = revision + return task } /// Retry saved credentials after startup or connectivity failures without replacing active identity work. @@ -1740,6 +1777,16 @@ class PubkyProfileManager: ObservableObject { return PubkyPublicKeyFormat.matches(adopted.pubky, publicKey) } + nonisolated static func hasStoredIdentityForBackup() throws -> Bool { + if AdoptedPubkyReference.current != nil { + return true + } + // Malformed credential bytes must not permit an identity-free replacement backup. + return try [KeychainEntryType.paykitSession, .pubkySecretKey].contains { + try Keychain.load(key: $0)?.isEmpty == false + } + } + nonisolated static func hasStoredIdentity( adopted: (sourceApp: String, pubky: String)? = AdoptedPubkyReference.current ) throws -> Bool { diff --git a/Bitkit/Services/PrivatePaykitService+Backup.swift b/Bitkit/Services/PrivatePaykitService+Backup.swift index 589b4c5cf..9c2a92cb6 100644 --- a/Bitkit/Services/PrivatePaykitService+Backup.swift +++ b/Bitkit/Services/PrivatePaykitService+Backup.swift @@ -3,13 +3,17 @@ import Foundation // MARK: - Backup extension PrivatePaykitService { - func backupSnapshot() async throws -> (state: String?, sdkVersion: UInt64) { - let initialSdkVersion = await PaykitSdkService.shared.backupStateVersion - guard try await PaykitSdkService.shared.currentPublicKey() != nil else { + func backupSnapshot( + sdkService: PaykitSdkService = .shared, + hasStoredIdentity: @Sendable () throws -> Bool = { try PubkyProfileManager.hasStoredIdentityForBackup() } + ) async throws -> (state: String?, sdkVersion: UInt64) { + let initialSdkVersion = await sdkService.backupStateVersion + guard try await sdkService.currentPublicKey() != nil else { + guard try !hasStoredIdentity() else { throw PubkyServiceError.sessionNotActive } return (nil, initialSdkVersion) } // Wallet backups may be required before broadcast; they cannot wait for payment to finish. - let exported = try await PaykitSdkService.shared.exportBackupSnapshot(priority: .interactive) + let exported = try await sdkService.exportBackupSnapshot(priority: .interactive) let backup = Backup( sdkState: exported.state, consumedPrivatePaymentListVersions: state.contacts.compactMapValues { contactState in diff --git a/BitkitTests/PrivatePaykitServiceTests.swift b/BitkitTests/PrivatePaykitServiceTests.swift index 3740ad349..c0bfa70bb 100644 --- a/BitkitTests/PrivatePaykitServiceTests.swift +++ b/BitkitTests/PrivatePaykitServiceTests.swift @@ -3,6 +3,88 @@ import Paykit import XCTest final class PrivatePaykitServiceTests: XCTestCase { + @MainActor + func testUnresolvedStoredIdentityCannotReplaceCompleteWalletBackup() async throws { + let sdk = BackupIdentitySdk(noPointer: .init()) + let sdkService = PaykitSdkService(sdkFactory: { sdk }) + let privateService = PrivatePaykitService() + let suite = "PendingIdentityBackup.\(UUID().uuidString)" + let defaults = try XCTUnwrap(UserDefaults(suiteName: suite)) + defer { defaults.removePersistentDomain(forName: suite) } + var remote = Data("complete-wallet".utf8) + var uploads = 0 + let backup = BackupService(defaults: defaults, backupData: { _ in + let snapshot = try await privateService.backupSnapshot(sdkService: sdkService, hasStoredIdentity: { true }) + return Data((snapshot.state ?? "null").utf8) + }, uploadBackup: { _, data in + uploads += 1 + remote = data + }) + await backup.triggerBackup(category: .wallet) + XCTAssertEqual(remote, Data("complete-wallet".utf8)) + XCTAssertEqual(uploads, 0) + XCTAssertTrue(backup.getBackupStatus(category: .wallet).isRequired) + XCTAssertEqual(sdk.exportCount, 0) + + sdk.publicKey = "restored-identity" + await backup.triggerBackup(category: .wallet) + XCTAssertEqual(uploads, 1) + XCTAssertTrue(String(decoding: remote, as: UTF8.self).contains("fresh-sdk-backup")) + XCTAssertFalse(backup.getBackupStatus(category: .wallet).isRequired) + backup.setWiping(true) + await backup.stopObservingBackups() + } + + func testNoIdentityCanProduceEmptyPrivateBackupButUnreadableIdentityCannot() async throws { + let sdk = BackupIdentitySdk(noPointer: .init()) + let sdkService = PaykitSdkService(sdkFactory: { sdk }) + let service = PrivatePaykitService() + let empty = try await service.backupSnapshot(sdkService: sdkService, hasStoredIdentity: { false }) + XCTAssertNil(empty.state) + do { + _ = try await service.backupSnapshot(sdkService: sdkService, hasStoredIdentity: { + throw PubkyServiceError.sessionNotActive + }) + XCTFail("Unreadable identity must not become an empty backup") + } catch PubkyServiceError.sessionNotActive {} + XCTAssertEqual(sdk.exportCount, 0) + } + + @MainActor + func testMalformedStoredCredentialsCannotProduceIdentityFreeBackup() async throws { + let keys: [KeychainEntryType] = [.paykitSession, .pubkySecretKey] + let savedValues = try keys.map { try Keychain.load(key: $0) } + let savedReference = AdoptedPubkyReference.current + addTeardownBlock { + AdoptedPubkyReference.current = savedReference + for (key, data) in zip(keys, savedValues) { + if let data { + try Keychain.upsert(key: key, data: data) + } else { + try Keychain.delete(key: key) + } + } + } + AdoptedPubkyReference.current = nil + for key in keys { + try Keychain.delete(key: key) + } + let sdk = BackupIdentitySdk(noPointer: .init()) + let sdkService = PaykitSdkService(sdkFactory: { sdk }) + let service = PrivatePaykitService() + for key in keys { + try Keychain.upsert(key: key, data: Data([0xFF])) + do { + _ = try await service.backupSnapshot(sdkService: sdkService) + XCTFail("Malformed stored credential must prevent an identity-free backup: \(key)") + } catch PubkyServiceError.sessionNotActive {} + try Keychain.delete(key: key) + } + XCTAssertEqual(sdk.exportCount, 0) + let empty = try await service.backupSnapshot(sdkService: sdkService) + XCTAssertNil(empty.state) + } + override func setUp() { super.setUp() // Constructing `PrivatePaykitService` and mutating it writes the real cache state, injecting @@ -3167,3 +3249,25 @@ private extension PrivatePaykitService { invalidateContactPreparation() } } + +private final class BackupIdentitySdk: PaykitSdk, @unchecked Sendable { + var publicKey: String? + var exportCount = 0 + + override func identityStatus() async throws -> IdentityStatus? { + IdentityStatus(publicKey: publicKey, capability: .privateLinkCapable) + } + + override func initialize() async throws -> IdentityStatus { + IdentityStatus(publicKey: publicKey, capability: .privateLinkCapable) + } + + override func exportBackupString() async throws -> String { + exportCount += 1 + return "fresh-sdk-backup" + } + + override func observedBackupStateRevision() throws -> ObservedBackupStateRevision? { + nil + } +} diff --git a/BitkitTests/PubkyProfileManagerTests.swift b/BitkitTests/PubkyProfileManagerTests.swift index 91465c895..8a021497b 100644 --- a/BitkitTests/PubkyProfileManagerTests.swift +++ b/BitkitTests/PubkyProfileManagerTests.swift @@ -1009,6 +1009,112 @@ final class PubkyProfileManagerTests: XCTestCase { XCTAssertTrue(manager.isProfileSetupPending) } + @MainActor + func testScheduledInitializationReturnsWhilePendingAndSharesAwaitedRecovery() async { + let manager = RecoveryProfileManager() + manager.publicKey = "previous-identity" + manager.authState = .authenticated + let started = expectation(description: "retained initialization started") + let (stream, release) = AsyncStream.makeStream() + defer { release.finish() } + manager.scheduleInitializationAfterRestore { + started.fulfill() + for await _ in stream {} + return .restored(publicKey: "restored-identity") + } + XCTAssertTrue(manager.isRestoringSession) + XCTAssertNil(manager.currentSession) + XCTAssertNil(manager.automaticPaykitSession) + XCTAssertEqual(manager.authState, .idle) + await fulfillment(of: [started], timeout: 2) + manager.scheduleInitializationAfterRestore { + XCTFail("Overlapping scheduling must share the retained operation") + return .noSession + } + let waiting = Task { + await manager.initialize { + XCTFail("Awaited initialization must share the retained operation") + return .noSession + } + } + await Task.yield() + release.finish() + await waiting.value + XCTAssertEqual(manager.publicKey, "restored-identity") + XCTAssertFalse(manager.isRestoringSession) + } + + @MainActor + func testScheduledInitializationCannotApplyAfterIdentityReplacement() async throws { + let manager = RecoveryProfileManager() + let started = expectation(description: "scheduled initialization started") + let (stream, release) = AsyncStream.makeStream() + defer { release.finish() } + manager.scheduleInitializationAfterRestore { + started.fulfill() + for await _ in stream {} + return .restored(publicKey: "old-identity") + } + await fulfillment(of: [started], timeout: 2) + let completion = Task { await manager.initialize { .noSession } } + try await PubkyProfileManager.restoreSessionBackupState( + nil, + deleteKeychainValue: { _ in }, + removeOwnSharedRecords: {}, + forgetSessionAccess: { + release.finish() + await completion.value + } + ) + XCTAssertNil(manager.currentSession) + XCTAssertEqual(manager.authState, .idle) + XCTAssertFalse(manager.isRestoringSession) + } + + @MainActor + func testRestoreSchedulingWaitsForStaleInitializationThenActivatesCurrentCredentialsOnce() async throws { + let manager = RecoveryProfileManager() + let oldStarted = expectation(description: "old initialization started") + let newStarted = expectation(description: "current credentials activated") + let (oldStream, releaseOld) = AsyncStream.makeStream() + defer { releaseOld.finish() } + let old = Task { + await manager.initialize { + oldStarted.fulfill() + for await _ in oldStream {} + return .restored(publicKey: "old-identity") + } + } + await fulfillment(of: [oldStarted], timeout: 2) + try await PubkyProfileManager.restoreSessionBackupState( + nil, + deleteKeychainValue: { _ in }, + removeOwnSharedRecords: {}, + forgetSessionAccess: {} + ) + let (newStream, releaseNew) = AsyncStream.makeStream() + defer { releaseNew.finish() } + manager.scheduleInitializationAfterRestore { + newStarted.fulfill() + for await _ in newStream {} + return .restored(publicKey: "new-identity") + } + XCTAssertNil(manager.currentSession) + releaseOld.finish() + await old.value + await fulfillment(of: [newStarted], timeout: 2) + let waiting = Task { + await manager.initialize { + XCTFail("Current activation must be shared") + return .noSession + } + } + await Task.yield() + releaseNew.finish() + await waiting.value + XCTAssertEqual(manager.publicKey, "new-identity") + } + @MainActor func testRecoveryWaitsForStartupAndCoalescesConnectivityEvents() async { let manager = RecoveryProfileManager() diff --git a/changelog.d/next/restore-identity.fixed.md b/changelog.d/next/restore-identity.fixed.md new file mode 100644 index 000000000..7d934b948 --- /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 000000000..fcaad5e53 --- /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 ec20b26434979c10b5805222a2e8adcc88fe1978 Mon Sep 17 00:00:00 2001 From: benk10 Date: Sun, 25 Oct 2026 00:55:57 +0200 Subject: [PATCH 2/7] docs: associate restore note with PR 1121 --- changelog.d/next/{restore-identity.fixed.md => 1121.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{restore-identity.fixed.md => 1121.fixed.md} (100%) diff --git a/changelog.d/next/restore-identity.fixed.md b/changelog.d/next/1121.fixed.md similarity index 100% rename from changelog.d/next/restore-identity.fixed.md rename to changelog.d/next/1121.fixed.md From 754a899182bb39869b37d1f3e9601afecad8c593 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 27 Oct 2026 15:18:53 +0200 Subject: [PATCH 3/7] fix: serialize backup observer cancellable mutations --- Bitkit/Services/BackupService.swift | 66 ++++++++++++++--------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift index 94fe0d95a..0c4a33ea8 100644 --- a/Bitkit/Services/BackupService.swift +++ b/Bitkit/Services/BackupService.swift @@ -113,7 +113,7 @@ class BackupService { @MainActor weak var usdtWallet: UsdtWalletManager? private var backupJobs: [BackupCategory: Task] = [:] private var runningBackupTasks: [BackupCategory: Task] = [:] - private var cancellables = Set() + @MainActor private var cancellables = Set() private var periodicCheckTask: Task? private var isObserving = false private var isRestoring = false @@ -213,8 +213,10 @@ class BackupService { guard shouldStart == true else { return } Logger.debug("Start observing backup statuses and data store changes", context: "BackupService") - startBackupStatusObservers() - startDataStoreListeners() + await MainActor.run { + startBackupStatusObservers() + startDataStoreListeners() + } startPeriodicBackupFailureCheck() } } @@ -236,7 +238,7 @@ class BackupService { await walletBackupWriter.waitUntilIdle() await usdtBackupWriter.waitUntilIdle() guard shouldStop == true else { return } - cancellables.removeAll() + await MainActor.run { cancellables.removeAll() } Logger.debug("Stopped observing backup statuses and data store changes", context: "BackupService") } @@ -618,7 +620,7 @@ class BackupService { // MARK: - Private Methods - private func startBackupStatusObservers() { + @MainActor private func startBackupStatusObservers() { for category in BackupCategory.allCases { let categoryPublisher = backupStatusesPublisher .prepend(getAllBackupStatuses()) @@ -646,27 +648,24 @@ class BackupService { Logger.debug("Started \(BackupCategory.allCases.count) backup status observers", context: "BackupService") } - private func startDataStoreListeners() { - Task { @MainActor [weak self] in - guard let self else { return } - // SETTINGS - SettingsViewModel.shared.settingsPublisher - .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) - .sink { [weak self] _ in - guard let self, !self.shouldSkipBackup() else { return } - markBackupRequired(category: .settings) - } - .store(in: &cancellables) + @MainActor private func startDataStoreListeners() { + // SETTINGS + SettingsViewModel.shared.settingsPublisher + .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) + .sink { [weak self] _ in + guard let self, !self.shouldSkipBackup() else { return } + markBackupRequired(category: .settings) + } + .store(in: &cancellables) - // WIDGETS - SettingsViewModel.shared.widgetsPublisher - .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) - .sink { [weak self] _ in - guard let self, !self.shouldSkipBackup() else { return } - markBackupRequired(category: .widgets) - } - .store(in: &cancellables) - } + // WIDGETS + SettingsViewModel.shared.widgetsPublisher + .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) + .sink { [weak self] _ in + guard let self, !self.shouldSkipBackup() else { return } + markBackupRequired(category: .widgets) + } + .store(in: &cancellables) // Record requirements synchronously; the scheduled upload already has a debounce. WalletBackupChangeTracker.localChanges @@ -714,16 +713,13 @@ class BackupService { .store(in: &cancellables) // APP STATE (UserDefaults changes, etc.) - Task { @MainActor [weak self] in - guard let self else { return } - SettingsViewModel.shared.appStatePublisher - .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) - .sink { [weak self] _ in - guard let self, !self.shouldSkipBackup() else { return } - markBackupRequired(category: .metadata) - } - .store(in: &cancellables) - } + SettingsViewModel.shared.appStatePublisher + .debounce(for: .milliseconds(500), scheduler: DispatchQueue.main) + .sink { [weak self] _ in + guard let self, !self.shouldSkipBackup() else { return } + markBackupRequired(category: .metadata) + } + .store(in: &cancellables) // BLOCKTANK CoreService.shared.blocktank.stateChangedPublisher From 0b77f099edcb76daf06f13421e4f2291d22e81c6 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 29 Oct 2026 00:36:09 +0200 Subject: [PATCH 4/7] fix: retry missing profiles after reconnect --- Bitkit/Managers/PubkyProfileManager.swift | 12 ++- BitkitTests/PubkyProfileManagerTests.swift | 75 +++++++++++++++++++ changelog.d/next/1121.fixed.md | 2 +- .../wallet/restore-during-identity-outage.xml | 1 + 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift index d29dd6b1e..0e4e3ac65 100644 --- a/Bitkit/Managers/PubkyProfileManager.swift +++ b/Bitkit/Managers/PubkyProfileManager.swift @@ -287,7 +287,17 @@ class PubkyProfileManager: ObservableObject { if completedRecoveryVersion != recoveryVersion, revision == Self.sessionRevision { return } - guard !Task.isCancelled, publicKey == nil, authState == .idle, Self.sessionMutationCount == 0 else { return } + guard !Task.isCancelled else { return } + if let session = automaticPaykitSession { + if isInitialized, profile == nil { + Task { + guard automaticPaykitSession == session, isInitialized, profile == nil else { return } + await loadProfile() + } + } + return + } + guard publicKey == nil, authState == .idle, Self.sessionMutationCount == 0 else { return } do { guard try hasStoredIdentity() else { return } await initialize(mode: .automaticRecovery, initializeSession: initializeSession) diff --git a/BitkitTests/PubkyProfileManagerTests.swift b/BitkitTests/PubkyProfileManagerTests.swift index d8357df8d..c80845e66 100644 --- a/BitkitTests/PubkyProfileManagerTests.swift +++ b/BitkitTests/PubkyProfileManagerTests.swift @@ -1802,6 +1802,67 @@ final class PubkyProfileManagerTests: XCTestCase { } } + @MainActor + func testSessionRecoveryReloadsMissingProfileWithoutReauthenticationOrWaiting() async { + await withRestoredProfileDefaults { + let stub = RemoteProfileStub(profiles: [ringKeyA: makeProfile(publicKey: ringKeyA, name: "Recovered")]) + await stub.makeUnreachable(ringKeyA) + let manager = PubkyProfileManager(remoteProfileResolver: { try await stub.resolve($0) }) + manager.publicKey = ringKeyA + manager.authState = .authenticated + manager.isInitialized = true + let session = manager.currentSession + await manager.loadProfile() + XCTAssertNil(manager.profile) + + await stub.makeReachable(ringKeyA) + await stub.setHoldsRequests(true) + for _ in 0 ..< 3 { + await manager.restoreSessionIfNeeded(hasStoredIdentity: { + XCTFail("An active identity must not inspect or import saved credentials") + return false + }) + } + await stub.waitForRequests(2) + XCTAssertTrue(manager.isLoadingProfile) + XCTAssertEqual(manager.currentSession, session) + let heldRequests = await stub.requests + XCTAssertEqual(heldRequests, [ringKeyA, ringKeyA]) + + await stub.release(request: 1) + await waitUntil("the missing profile recovers") { manager.profile?.name == "Recovered" } + await manager.restoreSessionIfNeeded() + await Task.yield() + let completedRequests = await stub.requests + XCTAssertEqual(completedRequests, [ringKeyA, ringKeyA]) + XCTAssertEqual(manager.cachedName, "Recovered") + } + } + + @MainActor + func testSessionRecoveryDropsMissingProfileResultFromClearedStateWithSameKey() async { + await withRestoredProfileDefaults { + let stub = RemoteProfileStub( + profiles: [ringKeyA: makeProfile(publicKey: ringKeyA, name: "Stale")], + holdsRequests: true + ) + let manager = ObservedProfileReadManager(remoteProfileResolver: { try await stub.resolve($0) }) + manager.publicKey = ringKeyA + manager.authState = .authenticated + manager.isInitialized = true + + await manager.restoreSessionIfNeeded() + await stub.waitForRequests(1) + manager.clearAuthenticatedStateForTesting() + manager.publicKey = ringKeyA + manager.authState = .authenticated + await stub.release(request: 0) + await waitUntil("the old profile read finishes") { manager.completedReads == 1 } + XCTAssertNil(manager.profile) + XCTAssertNil(manager.cachedName) + } + } + // MARK: - Profile saves /// The review regression: a remembered Ring profile with tags was removed remotely. Adoption reuses its cached row and @@ -4250,6 +4311,10 @@ private actor RemoteProfileStub { unreachableKeys.insert(key) } + func makeReachable(_ key: String) { + unreachableKeys.remove(key) + } + func waitForRequests(_ count: Int, file: StaticString = #filePath, line: UInt = #line) async { guard requests.count < count else { return } let id = UUID() @@ -4440,3 +4505,13 @@ private actor SessionRecoveryAttempts { return count } } + +@MainActor +private final class ObservedProfileReadManager: PubkyProfileManager { + private(set) var completedReads = 0 + + override func loadProfile() async { + await super.loadProfile() + completedReads += 1 + } +} diff --git a/changelog.d/next/1121.fixed.md b/changelog.d/next/1121.fixed.md index 7d934b948..5afda00c4 100644 --- a/changelog.d/next/1121.fixed.md +++ b/changelog.d/next/1121.fixed.md @@ -1 +1 @@ -Wallet restore finishes without waiting for optional profile refresh, while incomplete identity data cannot replace complete backups. +Wallet restore finishes without waiting for optional profile refresh, retries missing profile data after reconnecting, and preserves complete backups when identity data is unavailable. diff --git a/journeys/wallet/restore-during-identity-outage.xml b/journeys/wallet/restore-during-identity-outage.xml index fcaad5e53..e549f8d36 100644 --- a/journeys/wallet/restore-during-identity-outage.xml +++ b/journeys/wallet/restore-during-identity-outage.xml @@ -11,6 +11,7 @@ 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 + Fail the restored identity's own public profile read, then reconnect or resume while staying on Home; verify wallet maintenance continues while the read is held, and the saved profile replaces the placeholder after release without opening Profile or re-importing the active session 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 e5143b1304e9c29f503e9082f75e857555ccf47f Mon Sep 17 00:00:00 2001 From: benk10 Date: Sat, 31 Oct 2026 18:55:32 +0200 Subject: [PATCH 5/7] fix: skip profile recovery while setup is pending --- Bitkit/Managers/PubkyProfileManager.swift | 4 +-- BitkitTests/PubkyProfileManagerTests.swift | 34 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/Bitkit/Managers/PubkyProfileManager.swift b/Bitkit/Managers/PubkyProfileManager.swift index 55874f22c..e4a9cd266 100644 --- a/Bitkit/Managers/PubkyProfileManager.swift +++ b/Bitkit/Managers/PubkyProfileManager.swift @@ -301,9 +301,9 @@ class PubkyProfileManager: ObservableObject { } guard !Task.isCancelled else { return } if let session = automaticPaykitSession { - if isInitialized, profile == nil { + if isInitialized, profile == nil, !isProfileSetupPending { Task { - guard automaticPaykitSession == session, isInitialized, profile == nil else { return } + guard automaticPaykitSession == session, isInitialized, profile == nil, !isProfileSetupPending else { return } await loadProfile() } } diff --git a/BitkitTests/PubkyProfileManagerTests.swift b/BitkitTests/PubkyProfileManagerTests.swift index c920424ef..687cf5f3d 100644 --- a/BitkitTests/PubkyProfileManagerTests.swift +++ b/BitkitTests/PubkyProfileManagerTests.swift @@ -1829,6 +1829,7 @@ final class PubkyProfileManagerTests: XCTestCase { @MainActor func testSessionRecoveryReloadsMissingProfileWithoutReauthenticationOrWaiting() async { await withRestoredProfileDefaults { + UserDefaults.standard.set(false, forKey: "pubky_profile_setup_pending") let stub = RemoteProfileStub(profiles: [ringKeyA: makeProfile(publicKey: ringKeyA, name: "Recovered")]) await stub.makeUnreachable(ringKeyA) let manager = PubkyProfileManager(remoteProfileResolver: { try await stub.resolve($0) }) @@ -1863,9 +1864,42 @@ final class PubkyProfileManagerTests: XCTestCase { } } + @MainActor + func testSessionRecoveryDoesNotRefetchProfileWhileSetupIsPending() async { + await withRestoredProfileDefaults { + UserDefaults.standard.set(true, forKey: "pubky_profile_setup_pending") + let unexpectedRead = expectation(description: "An unfinished profile does not need a remote read") + unexpectedRead.isInverted = true + let stub = RemoteProfileStub() + let manager = PubkyProfileManager(remoteProfileResolver: { + unexpectedRead.fulfill() + return try await stub.resolve($0) + }) + manager.publicKey = ringKeyA + manager.authState = .authenticated + manager.isInitialized = true + let session = manager.currentSession + XCTAssertTrue(manager.isProfileSetupPending) + XCTAssertNotNil(manager.automaticPaykitSession) + + for _ in 0 ..< 3 { + await manager.restoreSessionIfNeeded(hasStoredIdentity: { + XCTFail("An active identity must not reload saved credentials") + return false + }) + } + await fulfillment(of: [unexpectedRead], timeout: 0.2) + let requests = await stub.requests + XCTAssertTrue(requests.isEmpty) + XCTAssertEqual(manager.currentSession, session) + XCTAssertTrue(manager.isProfileSetupPending) + } + } + @MainActor func testSessionRecoveryDropsMissingProfileResultFromClearedStateWithSameKey() async { await withRestoredProfileDefaults { + UserDefaults.standard.set(false, forKey: "pubky_profile_setup_pending") let stub = RemoteProfileStub( profiles: [ringKeyA: makeProfile(publicKey: ringKeyA, name: "Stale")], holdsRequests: true From b9f3bbda22cf8010e79c2c6bd24b69002f543be7 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 3 Nov 2026 10:47:56 -0500 Subject: [PATCH 6/7] fix: preserve backups when cached channel reads fail --- Bitkit/Services/BackupService.swift | 6 +- BitkitTests/BlocktankBackupTests.swift | 92 ++++++++++++++++++++++++++ changelog.d/next/1121.fixed.md | 2 +- 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 BitkitTests/BlocktankBackupTests.swift diff --git a/Bitkit/Services/BackupService.swift b/Bitkit/Services/BackupService.swift index e1c2226f4..ac0d64c47 100644 --- a/Bitkit/Services/BackupService.swift +++ b/Bitkit/Services/BackupService.swift @@ -1294,9 +1294,9 @@ class BackupService { return try JSONEncoder().encode(payload) case .blocktank: - let orders = try await CoreService.shared.blocktank.orders() - let cjitEntries = try await CoreService.shared.blocktank.cjitOrders() - let info = try? await CoreService.shared.blocktank.info(refresh: false) + let orders = try await CoreService.shared.blocktank.orders(refresh: false) + let cjitEntries = try await CoreService.shared.blocktank.cjitOrders(refresh: false) + let info = try await CoreService.shared.blocktank.info(refresh: false) let payload = BlocktankBackupV1( version: 1, diff --git a/BitkitTests/BlocktankBackupTests.swift b/BitkitTests/BlocktankBackupTests.swift new file mode 100644 index 000000000..a5af51643 --- /dev/null +++ b/BitkitTests/BlocktankBackupTests.swift @@ -0,0 +1,92 @@ +@testable import Bitkit +import BitkitCore +import SQLite3 +import XCTest + +@MainActor +final class BlocktankBackupTests: XCTestCase { + private var databaseDirectory: URL! + + override func setUp() async throws { + try await super.setUp() + _ = Bitkit.CoreService.shared.blocktank + await drainCoreServiceQueue() + databaseDirectory = FileManager.default.temporaryDirectory + .appendingPathComponent("BlocktankBackupTests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: databaseDirectory, withIntermediateDirectories: true) + _ = try initDb(basePath: databaseDirectory.path) + try await Bitkit.CoreService.shared.blocktank.upsertOrdersList([.mock(id: "cached-order")]) + } + + override func tearDown() async throws { + await drainCoreServiceQueue() + await repointCoreToAppStorage() + if let databaseDirectory { + try FileManager.default.removeItem(at: databaseDirectory) + } + try await super.tearDown() + } + + func testAbsentCachedInfoDoesNotPreventCompleteCachedBackup() async throws { + let defaults = try makeIsolatedDefaults() + var uploads: [Data] = [] + let service = BackupService(defaults: defaults, uploadBackup: { key, data in + XCTAssertEqual(key, BackupCategory.blocktank.rawValue) + uploads.append(data) + }) + + let outcome = await service.triggerBackup(category: .blocktank) + XCTAssertTrue(outcome == .completed) + XCTAssertEqual(uploads.count, 1) + let payload = try JSONDecoder().decode(BlocktankBackupV1.self, from: XCTUnwrap(uploads.first)) + XCTAssertEqual(payload.orders.map(\.id), ["cached-order"]) + XCTAssertTrue(payload.cjitEntries.isEmpty) + XCTAssertNil(payload.info) + XCTAssertFalse(service.getBackupStatus(category: .blocktank).isRequired) + await service.stopObservingBackups() + } + + func testUnreadableCachedDataPreservesAcknowledgedBackupAndPendingRevision() async throws { + for table in ["orders", "cjit_entries", "info"] { + let defaults = try makeIsolatedDefaults(table) + var uploads: [Data] = [] + let service = BackupService(defaults: defaults, uploadBackup: { _, data in + uploads.append(data) + }) + let initial = await service.triggerBackup(category: .blocktank) + XCTAssertTrue(initial == .completed, table) + let acknowledged = try XCTUnwrap(uploads.last) + let synced = service.getBackupStatus(category: .blocktank).synced + service.markBackupRequired(category: .blocktank, schedule: false) + let pending = service.getBackupStatus(category: .blocktank) + XCTAssertTrue(pending.isRequired, table) + + try execute("ALTER TABLE \(table) RENAME TO unreadable_\(table)") + let failed = await service.triggerBackup(category: .blocktank) + XCTAssertTrue(failed == .failed, table) + XCTAssertEqual(uploads, [acknowledged], table) + let afterFailure = service.getBackupStatus(category: .blocktank) + XCTAssertEqual(afterFailure.synced, synced, table) + XCTAssertEqual(afterFailure.required, pending.required, table) + XCTAssertTrue(afterFailure.isRequired, table) + XCTAssertFalse(afterFailure.running, table) + + try execute("ALTER TABLE unreadable_\(table) RENAME TO \(table)") + let recovered = await service.triggerBackup(category: .blocktank) + XCTAssertTrue(recovered == .completed, table) + XCTAssertEqual(uploads.count, 2, table) + XCTAssertFalse(service.getBackupStatus(category: .blocktank).isRequired, table) + await service.stopObservingBackups() + } + } + + private func execute(_ sql: String) throws { + var connection: OpaquePointer? + let opened = sqlite3_open_v2(databaseDirectory.appendingPathComponent("blocktank.db").path, + &connection, SQLITE_OPEN_READWRITE, nil) + defer { sqlite3_close(connection) } + guard opened == SQLITE_OK, sqlite3_exec(connection, sql, nil, nil, nil) == SQLITE_OK else { + throw NSError(domain: "BlocktankBackupTests", code: Int(sqlite3_errcode(connection))) + } + } +} diff --git a/changelog.d/next/1121.fixed.md b/changelog.d/next/1121.fixed.md index 5afda00c4..7a81bf808 100644 --- a/changelog.d/next/1121.fixed.md +++ b/changelog.d/next/1121.fixed.md @@ -1 +1 @@ -Wallet restore finishes without waiting for optional profile refresh, retries missing profile data after reconnecting, and preserves complete backups when identity data is unavailable. +Wallet restore finishes without waiting for optional profile refresh, retries missing profile data after reconnecting, and preserves complete backups when identity or cached channel data is unavailable. From 25f25eaed54e0c0cb7f39d863c96412cd3a3f94c Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 4 Nov 2026 04:46:29 -0500 Subject: [PATCH 7/7] fix: drain VSS calls before replacing wallet clients --- Bitkit/Services/VssBackupClient.swift | 334 ++++++++++++-------- Bitkit/Utilities/AppReset.swift | 14 +- BitkitTests/VssClientCoordinatorTests.swift | 234 ++++++++++++++ changelog.d/next/1121.fixed.md | 2 +- 4 files changed, 453 insertions(+), 131 deletions(-) create mode 100644 BitkitTests/VssClientCoordinatorTests.swift diff --git a/Bitkit/Services/VssBackupClient.swift b/Bitkit/Services/VssBackupClient.swift index 3c7995d2f..67183fac5 100644 --- a/Bitkit/Services/VssBackupClient.swift +++ b/Bitkit/Services/VssBackupClient.swift @@ -1,68 +1,146 @@ import Foundation import VssRustClientFfi -/// Actor to coordinate VSS client setup (ensures only one setup runs at a time) -private actor VssSetupCoordinator { - private enum SetupState { - case idle - case inProgress(Task) - case completed +/// Owns setup and calls until both native slots can be retired together. +/// Calls remain concurrent; reset closes admission and waits without holding a network lock. +actor VssClientCoordinator { + enum Domain { case app, ldk } + + private struct Setup { + let id = UUID() + let task: Task } - private var state: SetupState = .idle + private var generation = UUID() + private var setups: [Domain: Setup] = [:] + private var ready: Set = [] + private var activeCalls = 0 + private var resetting = false + private var idleWaiters: [CheckedContinuation] = [] + private var resetWaiters: [CheckedContinuation] = [] + + func withClient( + domain: Domain, + setup: @escaping () async throws -> Void, + operation: () async throws -> T + ) async throws -> T { + try Task.checkCancellation() + guard !resetting else { throw CancellationError() } + let owner = generation + activeCalls += 1 + defer { finishCall() } + + let result: Result + do { + try await awaitSetup(domain: domain, owner: owner, action: setup) + try validate(owner) + result = try await .success(operation()) + } catch { + result = .failure(error) + } + try validate(owner) + return try result.get() + } - func awaitSetup(setupAction: @escaping () async throws -> Void) async throws { - switch state { - case .completed: - Logger.debug("VssSetupCoordinator: already completed, returning", context: "VssBackupClient") + /// Cancellation of the caller cannot abandon retirement. Native calls already + /// admitted may finish, but their results cannot escape into the next owner. + func reset(shutdown: () -> Void) async { + if resetting { + await withCheckedContinuation { resetWaiters.append($0) } return + } + await closeAdmission() + shutdown() + reopenAdmission() + } - case let .inProgress(existingTask): - Logger.debug("VssSetupCoordinator: setup in progress, waiting for existing task", context: "VssBackupClient") - try await existingTask.value - Logger.debug("VssSetupCoordinator: existing task completed", context: "VssBackupClient") - return + /// Keep admission closed while the caller replaces or removes wallet credentials. + func withSuspendedClients(shutdown: () -> Void, operation: () async throws -> T) async throws -> T { + guard !resetting else { throw CancellationError() } + await closeAdmission() + shutdown() + defer { reopenAdmission() } + return try await operation() + } - case .idle: - Logger.debug("VssSetupCoordinator: idle, starting new setup", context: "VssBackupClient") - let task = Task { - try await setupAction() - } - state = .inProgress(task) + private func closeAdmission() async { + resetting = true + generation = UUID() + setups.values.forEach { $0.task.cancel() } + if activeCalls > 0 { + await withCheckedContinuation { idleWaiters.append($0) } + } + } - do { - try await task.value - state = .completed - Logger.debug("VssSetupCoordinator: setup completed successfully", context: "VssBackupClient") - } catch { - // Reset on any error to allow retry attempts - state = .idle - Logger.debug("VssSetupCoordinator: setup failed, resetting to idle", context: "VssBackupClient") - throw error + private func reopenAdmission() { + setups.removeAll() + ready.removeAll() + resetting = false + let waiters = resetWaiters + resetWaiters.removeAll() + waiters.forEach { $0.resume() } + } + + private func awaitSetup(domain: Domain, owner: UUID, action: @escaping () async throws -> Void) async throws { + if ready.contains(domain) { + return + } + let setup: Setup + if let existing = setups[domain] { + setup = existing + } else { + setup = Setup(task: Task { try await action() }) + setups[domain] = setup + } + do { + try await setup.task.value + guard !resetting, generation == owner else { throw CancellationError() } + if setups[domain]?.id == setup.id { + ready.insert(domain) + setups[domain] = nil + } + } catch { + if generation == owner, setups[domain]?.id == setup.id { + setups[domain] = nil } + throw error } } - func reset() { - Logger.debug("VssSetupCoordinator: reset called", context: "VssBackupClient") - if case let .inProgress(task) = state { - task.cancel() + private func validate(_ owner: UUID) throws { + try Task.checkCancellation() + guard !resetting, generation == owner else { throw CancellationError() } + } + + private func finishCall() { + activeCalls -= 1 + if activeCalls == 0 { + let waiters = idleWaiters + idleWaiters.removeAll() + waiters.forEach { $0.resume() } } - state = .idle } } class VssBackupClient { static let shared = VssBackupClient() - private let setupCoordinator = VssSetupCoordinator() - private let ldkSetupCoordinator = VssSetupCoordinator() + private let coordinator = VssClientCoordinator() private init() {} func reset() async { - await setupCoordinator.reset() - await ldkSetupCoordinator.reset() + await coordinator.reset { + vssShutdownClient() + vssShutdownLdkClient() + } + } + + func withSuspendedClients(_ operation: () async throws -> T) async throws -> T { + try await coordinator.withSuspendedClients(shutdown: { + vssShutdownClient() + vssShutdownLdkClient() + }, operation: operation) } /// Returns LNURL-auth params when configured. Callers fail closed if this returns nil. @@ -76,7 +154,11 @@ class VssBackupClient { } let passphraseRaw = try Keychain.loadString(key: .bip39Passphrase(index: walletIndex)) let passphrase = passphraseRaw?.isEmpty == true ? nil : passphraseRaw - let storeId = try await VssStoreIdProvider.shared.getVssStoreId(walletIndex: walletIndex) + let storeId = try vssDeriveStoreId( + prefix: Env.vssStoreIdPrefix, + mnemonic: mnemonic, + passphrase: passphrase + ) return (Env.vssServerUrl, storeId, mnemonic, passphrase, lnurlAuthServerUrl) } @@ -129,36 +211,36 @@ class VssBackupClient { } func putObject(key: String, data: Data) async throws -> VssItem { - try await awaitSetup() + try await coordinator.withClient(domain: .app, setup: { [self] in try await setup() }) { + Logger.debug("VSS 'putObject' call for '\(key)'", context: "VssBackupClient") - Logger.debug("VSS 'putObject' call for '\(key)'", context: "VssBackupClient") - - do { - let item = try await vssStore(key: key, value: data) - Logger.debug("VSS 'putObject' success for '\(key)' at version: \(item.version)", context: "VssBackupClient") - return item - } catch { - Logger.debug("VSS 'putObject' error for '\(key)': \(error)", context: "VssBackupClient") - throw error + do { + let item = try await vssStore(key: key, value: data) + Logger.debug("VSS 'putObject' success for '\(key)' at version: \(item.version)", context: "VssBackupClient") + return item + } catch { + Logger.debug("VSS 'putObject' error for '\(key)': \(error)", context: "VssBackupClient") + throw error + } } } func getObject(key: String) async throws -> VssItem? { - try await awaitSetup() - - Logger.debug("VSS 'getObject' call for '\(key)'", context: "VssBackupClient") + try await coordinator.withClient(domain: .app, setup: { [self] in try await setup() }) { + Logger.debug("VSS 'getObject' call for '\(key)'", context: "VssBackupClient") - do { - if let item = try await vssGet(key: key) { - Logger.debug("VSS 'getObject' success for '\(key)' at version \(item.version)", context: "VssBackupClient") - return item - } else { - Logger.debug("VSS 'getObject' success null for '\(key)'", context: "VssBackupClient") - return nil + do { + if let item = try await vssGet(key: key) { + Logger.debug("VSS 'getObject' success for '\(key)' at version \(item.version)", context: "VssBackupClient") + return item + } else { + Logger.debug("VSS 'getObject' success null for '\(key)'", context: "VssBackupClient") + return nil + } + } catch { + Logger.warn("VSS 'getObject' error for '\(key)': \(error)", context: "VssBackupClient") + throw error } - } catch { - Logger.warn("VSS 'getObject' error for '\(key)': \(error)", context: "VssBackupClient") - throw error } } @@ -169,37 +251,41 @@ class VssBackupClient { /// Returns app-level keys with version info (for debug UI). func listKeyVersions() async throws -> [KeyVersion] { - try await awaitSetup() - Logger.debug("VSS 'listKeyVersions' call", context: "VssBackupClient") - do { - let versions = try await vssListKeys(prefix: nil) - Logger.debug("VSS 'listKeyVersions' success: \(versions.count) key(s)", context: "VssBackupClient") - return versions - } catch { - Logger.debug("VSS 'listKeyVersions' error: \(error)", context: "VssBackupClient") - throw error + try await coordinator.withClient(domain: .app, setup: { [self] in try await setup() }) { + Logger.debug("VSS 'listKeyVersions' call", context: "VssBackupClient") + do { + let versions = try await vssListKeys(prefix: nil) + Logger.debug("VSS 'listKeyVersions' success: \(versions.count) key(s)", context: "VssBackupClient") + return versions + } catch { + Logger.debug("VSS 'listKeyVersions' error: \(error)", context: "VssBackupClient") + throw error + } } } /// Deletes a single app-level key. func deleteKey(_ key: String) async throws -> Bool { - try await awaitSetup() - Logger.debug("VSS 'deleteKey' call for '\(key)'", context: "VssBackupClient") - do { - let wasDeleted = try await vssDelete(key: key) - Logger.debug("VSS 'deleteKey' success for '\(key)': \(wasDeleted)", context: "VssBackupClient") - return wasDeleted - } catch { - Logger.debug("VSS 'deleteKey' error for '\(key)': \(error)", context: "VssBackupClient") - throw error + try await coordinator.withClient(domain: .app, setup: { [self] in try await setup() }) { + Logger.debug("VSS 'deleteKey' call for '\(key)'", context: "VssBackupClient") + do { + let wasDeleted = try await vssDelete(key: key) + Logger.debug("VSS 'deleteKey' success for '\(key)': \(wasDeleted)", context: "VssBackupClient") + return wasDeleted + } catch { + Logger.debug("VSS 'deleteKey' error for '\(key)': \(error)", context: "VssBackupClient") + throw error + } } } /// Deletes all app-level keys (lists then deletes each). func deleteAllKeys() async throws { - let versions = try await listKeyVersions() - for kv in versions { - _ = try await deleteKey(kv.key) + try await coordinator.withClient(domain: .app, setup: { [self] in try await setup() }) { + let versions = try await vssListKeys(prefix: nil) + for version in versions { + _ = try await vssDelete(key: version.key) + } } } @@ -213,64 +299,56 @@ class VssBackupClient { /// Returns all LDK keys across default, monitors, and archivedMonitors namespaces. func listAllKeysTaggedLdk() async throws -> [(LdkNamespace, KeyVersion)] { - try await awaitLdkSetup() - Logger.debug("VSS 'listAllKeysTaggedLdk' call", context: "VssBackupClient") - var result: [(LdkNamespace, KeyVersion)] = [] - for ns in Self.ldkNamespacesForList { - do { - let keys = try await vssLdkListKeys(namespace: ns) - result.append(contentsOf: keys.map { (ns, $0) }) - } catch { - Logger.debug("VSS 'listAllKeysTaggedLdk' error for namespace \(ns): \(error)", context: "VssBackupClient") - throw error + try await coordinator.withClient(domain: .ldk, setup: { [self] in try await setupLdk() }) { + Logger.debug("VSS 'listAllKeysTaggedLdk' call", context: "VssBackupClient") + var result: [(LdkNamespace, KeyVersion)] = [] + for ns in Self.ldkNamespacesForList { + do { + let keys = try await vssLdkListKeys(namespace: ns) + result.append(contentsOf: keys.map { (ns, $0) }) + } catch { + Logger.debug("VSS 'listAllKeysTaggedLdk' error for namespace \(ns): \(error)", context: "VssBackupClient") + throw error + } } + Logger.debug("VSS 'listAllKeysTaggedLdk' success: \(result.count) key(s)", context: "VssBackupClient") + return result } - Logger.debug("VSS 'listAllKeysTaggedLdk' success: \(result.count) key(s)", context: "VssBackupClient") - return result } /// Gets a single LDK key value by key and namespace. func getObjectLdk(key: String, namespace: LdkNamespace) async throws -> VssItem? { - try await awaitLdkSetup() - Logger.debug("VSS 'getObjectLdk' call for '\(key)'", context: "VssBackupClient") - do { - return try await vssLdkGet(key: key, namespace: namespace) - } catch { - Logger.debug("VSS 'getObjectLdk' error for '\(key)': \(error)", context: "VssBackupClient") - throw error + try await coordinator.withClient(domain: .ldk, setup: { [self] in try await setupLdk() }) { + Logger.debug("VSS 'getObjectLdk' call for '\(key)'", context: "VssBackupClient") + do { + return try await vssLdkGet(key: key, namespace: namespace) + } catch { + Logger.debug("VSS 'getObjectLdk' error for '\(key)': \(error)", context: "VssBackupClient") + throw error + } } } /// Deletes a single LDK key by key and namespace. func deleteObjectLdk(key: String, namespace: LdkNamespace) async throws -> Bool { - try await awaitLdkSetup() - Logger.debug("VSS 'deleteObjectLdk' call for '\(key)'", context: "VssBackupClient") - do { - let wasDeleted = try await vssLdkDelete(key: key, namespace: namespace) - Logger.debug("VSS 'deleteObjectLdk' success for '\(key)': \(wasDeleted)", context: "VssBackupClient") - return wasDeleted - } catch { - Logger.debug("VSS 'deleteObjectLdk' error for '\(key)': \(error)", context: "VssBackupClient") - throw error + try await coordinator.withClient(domain: .ldk, setup: { [self] in try await setupLdk() }) { + Logger.debug("VSS 'deleteObjectLdk' call for '\(key)'", context: "VssBackupClient") + do { + let wasDeleted = try await vssLdkDelete(key: key, namespace: namespace) + Logger.debug("VSS 'deleteObjectLdk' success for '\(key)': \(wasDeleted)", context: "VssBackupClient") + return wasDeleted + } catch { + Logger.debug("VSS 'deleteObjectLdk' error for '\(key)': \(error)", context: "VssBackupClient") + throw error + } } } func deleteLdkScorerCache() async throws { - _ = try await deleteObjectLdk(key: "scorer", namespace: .default) - _ = try await deleteObjectLdk(key: "external_pathfinding_scores_cache", namespace: .default) - Logger.info("Cleared LDK scorer cache from VSS", context: "VssBackupClient") - } - - private func awaitSetup() async throws { - try await setupCoordinator.awaitSetup { [self] in - try await setup() - } - } - - /// Lazily sets up the LDK client when first needed (debug screen). Independent of the app client. - private func awaitLdkSetup() async throws { - try await ldkSetupCoordinator.awaitSetup { [self] in - try await setupLdk() + try await coordinator.withClient(domain: .ldk, setup: { [self] in try await setupLdk() }) { + _ = try await vssLdkDelete(key: "scorer", namespace: .default) + _ = try await vssLdkDelete(key: "external_pathfinding_scores_cache", namespace: .default) + Logger.info("Cleared LDK scorer cache from VSS", context: "VssBackupClient") } } diff --git a/Bitkit/Utilities/AppReset.swift b/Bitkit/Utilities/AppReset.swift index f309a6f01..8bf5cdffa 100644 --- a/Bitkit/Utilities/AppReset.swift +++ b/Bitkit/Utilities/AppReset.swift @@ -39,9 +39,19 @@ enum AppReset { BackupService.shared.setWiping(false) } - // Stop backup observers and reset VSS client await BackupService.shared.stopObservingBackups() - await VssBackupClient.shared.reset() + try await VssBackupClient.shared.withSuspendedClients { + try await wipeWithClientsSuspended(app: app, wallet: wallet, session: session, toastType: toastType) + } + } + + @MainActor + private static func wipeWithClientsSuspended( + app: AppViewModel, + wallet: WalletViewModel, + session: SessionManager, + toastType: Toast.ToastType + ) async throws { VssStoreIdProvider.shared.clearCache() OnChainHwService.shared.stopAllWatchers() diff --git a/BitkitTests/VssClientCoordinatorTests.swift b/BitkitTests/VssClientCoordinatorTests.swift new file mode 100644 index 000000000..503848776 --- /dev/null +++ b/BitkitTests/VssClientCoordinatorTests.swift @@ -0,0 +1,234 @@ +@testable import Bitkit +import XCTest + +final class VssClientCoordinatorTests: XCTestCase { + func testResetDrainsOldSetupAndRejectsLateCompletion() async throws { + let coordinator = VssClientCoordinator() + let gate = Gate() + let started = expectation(description: "Original setup started") + let cancelled = expectation(description: "Reset cancelled original setup") + let retired = expectation(description: "Retirement must await old setup") + let shutdowns = Counter() + let old = Task { + try await coordinator.withClient(domain: .app, setup: { + await withTaskCancellationHandler { + started.fulfill() + await gate.wait() + } onCancel: { cancelled.fulfill() } + }, operation: { XCTFail("Retired setup must not dispatch a call") }) + } + await fulfillment(of: [started], timeout: 2) + let reset = Task { await coordinator.reset { shutdowns.increment(); retired.fulfill() } } + await fulfillment(of: [cancelled], timeout: 2) + try await Task.sleep(nanoseconds: 100_000_000) + XCTAssertEqual(shutdowns.value, 0) + do { + try await coordinator.withClient(domain: .ldk, setup: { XCTFail("Admission during drain") }, operation: {}) + XCTFail("Admission during reset must fail") + } catch is CancellationError {} catch { XCTFail("Unexpected error: \(error)") } + await gate.release() + await assertCancelled(old) + await reset.value + await fulfillment(of: [retired], timeout: 2) + let replacement = expectation(description: "Replacement setup runs") + try await coordinator.withClient(domain: .app, setup: { replacement.fulfill() }, operation: {}) + await fulfillment(of: [replacement], timeout: 2) + } + + func testResetDrainsBothDomainsAndRejectsTheirResults() async throws { + let coordinator = VssClientCoordinator() + let appGate = Gate() + let ldkGate = Gate() + let started = expectation(description: "Both calls admitted") + started.expectedFulfillmentCount = 2 + let retired = expectation(description: "Both calls must drain before shutdown") + let shutdowns = Counter() + let app = Task { + try await coordinator.withClient(domain: .app, setup: {}, operation: { + started.fulfill() + await appGate.wait() + }) + } + let ldk = Task { + try await coordinator.withClient(domain: .ldk, setup: {}, operation: { + started.fulfill() + await ldkGate.wait() + }) + } + await fulfillment(of: [started], timeout: 2) + let reset = Task { await coordinator.reset { shutdowns.increment(); retired.fulfill() } } + await waitForClosedAdmission(coordinator) + try await Task.sleep(nanoseconds: 100_000_000) + XCTAssertEqual(shutdowns.value, 0) + await appGate.release() + await assertCancelled(app) + try await Task.sleep(nanoseconds: 100_000_000) + XCTAssertEqual(shutdowns.value, 0) + await ldkGate.release() + await assertCancelled(ldk) + await reset.value + await fulfillment(of: [retired], timeout: 2) + } + + func testCancelledResetStillWaitsForNativeCall() async throws { + let coordinator = VssClientCoordinator() + let gate = Gate() + let started = expectation(description: "Call admitted") + let retired = expectation(description: "Native retirement") + let shutdowns = Counter() + let old = Task { + try await coordinator.withClient(domain: .app, setup: {}, operation: { + started.fulfill() + await gate.wait() + }) + } + await fulfillment(of: [started], timeout: 2) + let reset = Task { await coordinator.reset { shutdowns.increment(); retired.fulfill() } } + await waitForClosedAdmission(coordinator) + reset.cancel() + try await Task.sleep(nanoseconds: 100_000_000) + XCTAssertEqual(shutdowns.value, 0) + await gate.release() + await assertCancelled(old) + await reset.value + await fulfillment(of: [retired], timeout: 2) + XCTAssertEqual(shutdowns.value, 1) + try await coordinator.withClient(domain: .app, setup: {}, operation: {}) + } + + func testAdmissionStaysClosedThroughoutCredentialRemovalAndReopensAfterFailure() async throws { + let coordinator = VssClientCoordinator() + try await coordinator.withClient(domain: .app, setup: {}, operation: {}) + do { + try await coordinator.withSuspendedClients(shutdown: {}, operation: { + for domain in [VssClientCoordinator.Domain.app, .ldk] { + do { + try await coordinator.withClient(domain: domain, setup: { + XCTFail("Old credentials must not be read") + }, operation: {}) + XCTFail("Wallet transition must keep admission closed") + } catch is CancellationError {} catch { XCTFail("Unexpected error: \(error)") } + } + throw URLError(.cannotWriteToFile) + }) + XCTFail("Transition failure must propagate") + } catch let error as URLError { XCTAssertEqual(error.code, .cannotWriteToFile) } + let setup = expectation(description: "Failure still retires cached setup") + try await coordinator.withClient(domain: .app, setup: { setup.fulfill() }, operation: {}) + await fulfillment(of: [setup], timeout: 2) + } + + func testRetiredNativeFailureIsCancellationRatherThanNewOwnerAuthFailure() async { + let coordinator = VssClientCoordinator() + let gate = Gate() + let started = expectation(description: "Old call admitted") + let old = Task { + try await coordinator.withClient(domain: .app, setup: {}, operation: { + started.fulfill() + await gate.wait() + throw URLError(.userAuthenticationRequired) + }) + } + await fulfillment(of: [started], timeout: 2) + let reset = Task { await coordinator.reset {} } + await waitForClosedAdmission(coordinator) + await gate.release() + await assertCancelled(old) + await reset.value + } + + func testConcurrentCallsShareSetupAndRemainConcurrent() async throws { + let coordinator = VssClientCoordinator() + let setupGate = Gate() + let callGate = Gate() + let setupStarted = expectation(description: "One setup") + setupStarted.assertForOverFulfill = true + let callsStarted = expectation(description: "Both calls run concurrently") + callsStarted.expectedFulfillmentCount = 2 + let setup = { setupStarted.fulfill(); await setupGate.wait() } + let operation = { callsStarted.fulfill(); await callGate.wait() } + let first = Task { try await coordinator.withClient(domain: .app, setup: setup, operation: operation) } + await fulfillment(of: [setupStarted], timeout: 2) + let second = Task { try await coordinator.withClient(domain: .app, setup: setup, operation: operation) } + await setupGate.release() + await fulfillment(of: [callsStarted], timeout: 2) + await callGate.release() + try await first.value + try await second.value + } + + func testFailedSetupCanRetryWithoutAdmittingItsOperation() async throws { + let coordinator = VssClientCoordinator() + do { + try await coordinator.withClient(domain: .app, setup: { throw URLError(.notConnectedToInternet) }, operation: { + XCTFail("Failed setup must not dispatch") + }) + XCTFail("Setup must fail") + } catch let error as URLError { XCTAssertEqual(error.code, .notConnectedToInternet) } + let value = try await coordinator.withClient(domain: .app, setup: {}, operation: { 42 }) + XCTAssertEqual(value, 42) + } + + func testCancelledWaiterDoesNotInvalidateSuccessfulSharedSetup() async throws { + let coordinator = VssClientCoordinator() + let gate = Gate() + let started = expectation(description: "Setup started") + let first = Task { + try await coordinator.withClient(domain: .app, setup: { + started.fulfill() + await gate.wait() + }, operation: { XCTFail("Cancelled caller must not dispatch") }) + } + await fulfillment(of: [started], timeout: 2) + first.cancel() + await gate.release() + await assertCancelled(first) + try await coordinator.withClient(domain: .app, setup: { XCTFail("Setup already succeeded") }, operation: {}) + } + + private func assertCancelled(_ task: Task, file: StaticString = #filePath, line: UInt = #line) async { + do { + try await task.value + XCTFail("Retired call must fail", file: file, line: line) + } catch is CancellationError {} catch { XCTFail("Unexpected error: \(error)", file: file, line: line) } + } + + private func waitForClosedAdmission(_ coordinator: VssClientCoordinator) async { + for _ in 0 ..< 100 { + do { try await coordinator.withClient(domain: .app, setup: {}, operation: {}) } + catch is CancellationError { return } + catch { XCTFail("Unexpected error: \(error)"); return } + await Task.yield() + } + XCTFail("Reset did not close admission") + } + + private final class Counter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + var value: Int { + lock.withLock { count } + } + + func increment() { + lock.withLock { count += 1 } + } + } + + /// Deliberately ignores cancellation, like a native FFI call already in progress. + private actor Gate { + private var released = false + private var waiters: [CheckedContinuation] = [] + func wait() async { + guard !released else { return } + await withCheckedContinuation { waiters.append($0) } + } + + func release() { + released = true + let pending = waiters + waiters.removeAll() + pending.forEach { $0.resume() } + } + } +} diff --git a/changelog.d/next/1121.fixed.md b/changelog.d/next/1121.fixed.md index 7a81bf808..02bcfbd43 100644 --- a/changelog.d/next/1121.fixed.md +++ b/changelog.d/next/1121.fixed.md @@ -1 +1 @@ -Wallet restore finishes without waiting for optional profile refresh, retries missing profile data after reconnecting, and preserves complete backups when identity or cached channel data is unavailable. +Wallet restore finishes without waiting for optional profile refresh, retries missing profile data after reconnecting, preserves complete backups when identity or cached channel data is unavailable, and keeps retired backup calls from crossing wallet resets.