From ed7203954990acb6156a1ce884089da10e17c6ba Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Mon, 20 Jul 2026 18:33:29 +0200 Subject: [PATCH] feat(swift-sdk): make wallet deletion asynchronous --- .../PlatformWalletManager.swift | 126 +++-- .../PlatformWalletPersistenceHandler.swift | 517 ++++++++++-------- .../Core/Views/WalletDetailView.swift | 2 +- .../WalletDeletionTests.swift | 76 ++- 4 files changed, 424 insertions(+), 297 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index f2255597c2..785da18d89 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -207,6 +207,12 @@ public class PlatformWalletManager: ObservableObject { private var modelContainer: ModelContainer? private var signerNetwork: Network? + /// Async deletion yields while the persistence handler sweeps SwiftData. + /// Reject create/load/another delete during that window so actor + /// reentrancy cannot recreate a deterministic wallet id that the in-flight + /// deletion is about to remove from persistence. + private var walletDeletionInProgress = false + /// Retained for the lifetime of the FFI handle so the event-handler /// context pointer remains valid. private var eventHandler: PlatformWalletEventHandler? @@ -337,6 +343,7 @@ public class PlatformWalletManager: ObservableObject { birthHeight: UInt32? = nil ) throws -> ManagedPlatformWallet { try ensureConfigured() + try ensureNoWalletDeletionInProgress() var walletHandle: Handle = NULL_HANDLE var walletId: FFIByteTuple32 = (0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0) @@ -379,6 +386,7 @@ public class PlatformWalletManager: ObservableObject { birthHeight: UInt32? = nil ) throws -> ManagedPlatformWallet { try ensureConfigured() + try ensureNoWalletDeletionInProgress() guard seed.count == 64 else { throw PlatformWalletError.invalidParameter( "seed must be 64 bytes, got \(seed.count)" @@ -441,6 +449,7 @@ public class PlatformWalletManager: ObservableObject { @discardableResult public func loadFromPersistor() throws -> [ManagedPlatformWallet] { try ensureConfigured() + try ensureNoWalletDeletionInProgress() try platform_wallet_manager_load_from_persistor(handle).check() @@ -970,18 +979,25 @@ public class PlatformWalletManager: ObservableObject { // MARK: - Wallet deletion - /// Fully wipe a wallet's Rust, SwiftData, and Keychain footprint. - /// - /// Requires the manager to have been `configure`d with a - /// `ModelContainer` — the per-identity Keychain sweep needs the - /// wallet's identity ids, which only the persistence handler can - /// resolve. The no-persistence configuration mode is rejected - /// here rather than silently leaving identity key material behind. + /// Fully wipe a wallet's Rust, SwiftData, and Keychain footprint without + /// blocking the main actor while SwiftData and Keychain perform their + /// potentially expensive database sweeps. /// - /// Deleting an already-removed wallet succeeds unless an - /// operation fails. - public func deleteWallet(walletId: Data) throws { + /// `PlatformWalletManager` is main-actor isolated because its published + /// state and Rust handle are UI-facing. The persistence handler, however, + /// owns a dedicated serial queue with awaitable app-facing APIs, so its + /// database sweep does not freeze the UI. Rust/in-memory mutations remain + /// on the main actor. Deleting an already-removed wallet succeeds unless + /// an operation fails. + public func deleteWallet(walletId: Data) async throws { + let startedAt = ContinuousClock.now + let walletLabel = walletId.prefix(4) + .map { String(format: "%02x", $0) } + .joined() + try ensureConfigured() + try beginWalletDeletion() + defer { walletDeletionInProgress = false } guard walletId.count == 32 else { throw PlatformWalletError.invalidParameter( "walletId must be 32 bytes, got \(walletId.count)" @@ -993,24 +1009,22 @@ public class PlatformWalletManager: ObservableObject { ) } - let identityIds = try persistenceHandler.identityIdsForWallet(walletId: walletId) - - // Wipe Keychain BEFORE the SwiftData identity deletion runs. - // Order matters for retry-safety: if `deleteWalletData` - // commits identity rows and then throws partway, a retry - // would see `identityIdsForWallet == []` and the - // `deleteAllKeychainItems(forIdentityId:)` sweep below - // could no longer find the keys to purge. Doing the - // keychain side first leaves at worst stale SwiftData - // rows on a retry — repeating the wipe is harmless, and - // every keychain call here is idempotent (no-op on "not - // found"). Mnemonic / metadata stay in `WalletStorage` - // for now so a retry can still derive any missed key. - for identityId in identityIds { - try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId) - } - try KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId) + var phaseStartedAt = ContinuousClock.now + let identityIds = try await persistenceHandler.identityIdsForWallet( + walletId: walletId) + try await Task.detached(priority: .userInitiated) { + for identityId in identityIds { + try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId) + } + try KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId) + }.value + SDKLogger.log( + "🗑️ wallet \(walletLabel)… delete keychain phase: " + + String(describing: phaseStartedAt.duration(to: .now)), + minimumLevel: .low) + + phaseStartedAt = .now try walletId.withUnsafeBytes { raw in guard let base = raw.baseAddress?.assumingMemoryBound(to: FFIByteTuple32.self) else { throw PlatformWalletError.nullPointer( @@ -1021,31 +1035,32 @@ public class PlatformWalletManager: ObservableObject { } wallets.removeValue(forKey: walletId) - // Drop the needs-unlock banner state immediately so a re-created wallet - // with the same deterministic id doesn't inherit a stale banner (the - // poller would also prune it, but not until the next tick). dashPayUnlockStatus.removeValue(forKey: walletId) - - try persistenceHandler.deleteWalletData(walletId: walletId) - - // The mnemonic + metadata blobs in the Keychain are keyed by - // `walletId`. With network-scoped wallet ids the same mnemonic - // maps to a DIFFERENT id per network, so a given id is owned by - // exactly one network's wallet and carries its own mnemonic - // copy — purging it can't orphan a sibling network (those live - // under their own distinct ids). The `walletRowCountAcrossNetworks - // == 0` check is therefore expected to be true right after - // `deleteWalletData` removes this id's lone row; it is retained - // as a defensive guard (and to stay correct should the id model - // ever change) so we never delete the phrase while any row for - // this exact id still exists. - let remaining = try persistenceHandler.walletRowCountAcrossNetworks(walletId: walletId) + SDKLogger.log( + "🗑️ wallet \(walletLabel)… delete Rust/in-memory phase: " + + String(describing: phaseStartedAt.duration(to: .now)), + minimumLevel: .low) + + phaseStartedAt = .now + try await persistenceHandler.deleteWalletData(walletId: walletId) + SDKLogger.log( + "🗑️ wallet \(walletLabel)… delete SwiftData phase: " + + String(describing: phaseStartedAt.duration(to: .now)), + minimumLevel: .low) + + let remaining = try await persistenceHandler.walletRowCountAcrossNetworks( + walletId: walletId) if remaining == 0 { - let storage = WalletStorage() - // Delete metadata first so the mnemonic remains available for retry. - try storage.deleteMetadata(for: walletId) - try storage.deleteMnemonic(for: walletId) + try await Task.detached(priority: .userInitiated) { + let storage = WalletStorage() + try storage.deleteMetadata(for: walletId) + try storage.deleteMnemonic(for: walletId) + }.value } + SDKLogger.log( + "🗑️ wallet \(walletLabel)… delete total: " + + String(describing: startedAt.duration(to: .now)), + minimumLevel: .low) } // MARK: - Per-wallet lookup @@ -1238,6 +1253,19 @@ public class PlatformWalletManager: ObservableObject { } } + private func ensureNoWalletDeletionInProgress() throws { + guard !walletDeletionInProgress else { + throw PlatformWalletError.walletOperation( + "wallet deletion is already in progress" + ) + } + } + + private func beginWalletDeletion() throws { + try ensureNoWalletDeletionInProgress() + walletDeletionInProgress = true + } + /// Starts the SPV progress polling loop. Cancelled on deinit. /// /// `@Published` assignments are gated on inequality so that identical diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 519b32953f..aa1f80604a 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -85,6 +85,34 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { try serialQueue.sync(execute: body) } + /// Asynchronously run app-facing work on the same queue that owns + /// `backgroundContext`. FFI callbacks keep using synchronous `onQueue`, + /// while UI-driven operations can await long SwiftData sweeps without + /// blocking MainActor or parking a cooperative task thread in `sync`. + private func onQueueAsync( + _ body: @escaping @Sendable () throws -> T + ) async throws -> T { + try await withCheckedThrowingContinuation { continuation in + serialQueue.async { + do { + continuation.resume(returning: try body()) + } catch { + continuation.resume(throwing: error) + } + } + } + } + +#if DEBUG + /// Test seam for proving that async app-facing persistence calls suspend + /// MainActor while waiting behind already-enqueued persistence work. + func enqueuePersistenceOperationForTesting( + _ operation: @escaping @Sendable () -> Void + ) { + serialQueue.async(execute: operation) + } +#endif + // MARK: - Platform Address Balances /// Apply an incremental BLAST balance changeset to SwiftData. @@ -3828,275 +3856,288 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// metadata in the Keychain are shared by every network's row, so /// `deleteWallet` consults this after wiping its own network's row /// to decide whether the shared Keychain material can be purged. - public func walletRowCountAcrossNetworks(walletId: Data) throws -> Int { - try onQueue { - let descriptor = FetchDescriptor( - predicate: PersistentWallet.predicate(walletId: walletId) - ) - return try backgroundContext.fetchCount(descriptor) + public func walletRowCountAcrossNetworks(walletId: Data) async throws -> Int { + try await onQueueAsync { + try self.walletRowCountAcrossNetworksOnQueue(walletId: walletId) } } - public func identityIdsForWallet(walletId: Data) throws -> [Data] { - try onQueue { - let descriptor = FetchDescriptor( - predicate: walletRecordPredicate(walletId: walletId) - ) - guard let walletRow = try backgroundContext.fetch(descriptor).first else { - return [] - } - return walletRow.identities.map { $0.identityId } + private func walletRowCountAcrossNetworksOnQueue(walletId: Data) throws -> Int { + let descriptor = FetchDescriptor( + predicate: PersistentWallet.predicate(walletId: walletId) + ) + return try backgroundContext.fetchCount(descriptor) + } + + public func identityIdsForWallet(walletId: Data) async throws -> [Data] { + try await onQueueAsync { + try self.identityIdsForWalletOnQueue(walletId: walletId) } } - /// Wipe a wallet's SwiftData footprint. - public func deleteWalletData(walletId: Data) throws { - try onQueue { - do { - let walletDescriptor = FetchDescriptor( - predicate: walletRecordPredicate(walletId: walletId) - ) - let walletRow = try backgroundContext.fetch(walletDescriptor).first - let walletNetwork = walletRow?.network - - if let walletRow = walletRow { - // Wallet → identities is `.nullify`; this delete - // path cascades them explicitly. - let identitiesToDelete = Array(walletRow.identities) - let identityIds = identitiesToDelete.map { $0.identityId } - - for identityId in identityIds { - let balanceDescriptor = FetchDescriptor( - predicate: PersistentTokenBalance.predicate(identityId: identityId) - ) - for row in try backgroundContext.fetch(balanceDescriptor) { - backgroundContext.delete(row) - } - } + private func identityIdsForWalletOnQueue(walletId: Data) throws -> [Data] { + let descriptor = FetchDescriptor( + predicate: walletRecordPredicate(walletId: walletId) + ) + guard let walletRow = try backgroundContext.fetch(descriptor).first else { + return [] + } + return walletRow.identities.map { $0.identityId } + } - // SwiftData fatals during save() whenever it has - // to null out a non-optional inverse on a child - // being processed in the same save batch (the - // canonical wording is - // `Cannot remove PersistentX from relationship - // Y on PersistentZ because an appropriate - // default value is not configured`). - // Marking children for delete in the SAME batch - // doesn't help — SwiftData still walks their - // inverses during the merge phase. - // - // The workaround is to delete each layer in its - // own `save()`, parent last, so by the time the - // parent's delete runs its relationship - // collections are empty and SwiftData has no - // inverse to clean up. Costs us atomicity (four - // saves) — acceptable for a user-initiated wipe. - // - // PHASE 1: delete every identity's cascade-children - // whose inverse to identity is non-optional (DPNS - // names, DashPay profile, DashPay contact profiles, - // DashPay contact requests, DashPay payments, DashPay - // ignored senders). PublicKey, Document, and - // TokenBalance inverses to identity are already - // Optional and don't need pre-deletion. - // - // Every one of these rows has a non-optional - // `owner: PersistentIdentity`, so omitting any of them - // makes PHASE 2's identity delete hit the SwiftData - // fatal PHASE 1 exists to avoid — aborting the wipe and - // leaving sender-controlled DashPay strings (contact - // profile display name / public message / avatar URL), - // plaintext counterparty/memo/amount/txid (payments), - // and privacy-relevant ignored-sender ids on disk after - // a user-initiated wallet wipe. - for identity in identitiesToDelete { - for name in Array(identity.dpnsNames) { - backgroundContext.delete(name) - } - if let profile = identity.dashpayProfile { - backgroundContext.delete(profile) - } - for contactProfile in Array(identity.contactProfiles) { - backgroundContext.delete(contactProfile) - } - for cr in Array(identity.contactRequests) { - backgroundContext.delete(cr) - } - for payment in Array(identity.dashpayPayments) { - backgroundContext.delete(payment) - } - for ignored in Array(identity.dashpayIgnoredSenders) { - backgroundContext.delete(ignored) - } - } - try backgroundContext.save() + /// Wipe a wallet's SwiftData footprint on the handler's native serial + /// queue without blocking MainActor. + public func deleteWalletData(walletId: Data) async throws { + try await onQueueAsync { + try self.deleteWalletDataOnQueue(walletId: walletId) + } + } - // PHASE 2: delete the identities themselves now - // that their problematic cascade children are - // gone from the store. - for identity in identitiesToDelete { - backgroundContext.delete(identity) + private func deleteWalletDataOnQueue(walletId: Data) throws { + do { + let walletDescriptor = FetchDescriptor( + predicate: walletRecordPredicate(walletId: walletId) + ) + let walletRow = try backgroundContext.fetch(walletDescriptor).first + let walletNetwork = walletRow?.network + + if let walletRow = walletRow { + // Wallet → identities is `.nullify`; this delete + // path cascades them explicitly. + let identitiesToDelete = Array(walletRow.identities) + let identityIds = identitiesToDelete.map { $0.identityId } + + for identityId in identityIds { + let balanceDescriptor = FetchDescriptor( + predicate: PersistentTokenBalance.predicate(identityId: identityId) + ) + for row in try backgroundContext.fetch(balanceDescriptor) { + backgroundContext.delete(row) } - try backgroundContext.save() + } - // PHASE 3: delete the wallet's accounts. Same - // reasoning — `PersistentAccount.wallet` is - // non-optional; deleting accounts in their own - // save() pass leaves the wallet's `accounts` - // collection empty when the wallet itself is - // deleted. - let accountsToDelete = Array(walletRow.accounts) - for account in accountsToDelete { - backgroundContext.delete(account) + // SwiftData fatals during save() whenever it has + // to null out a non-optional inverse on a child + // being processed in the same save batch (the + // canonical wording is + // `Cannot remove PersistentX from relationship + // Y on PersistentZ because an appropriate + // default value is not configured`). + // Marking children for delete in the SAME batch + // doesn't help — SwiftData still walks their + // inverses during the merge phase. + // + // The workaround is to delete each layer in its + // own `save()`, parent last, so by the time the + // parent's delete runs its relationship + // collections are empty and SwiftData has no + // inverse to clean up. Costs us atomicity (four + // saves) — acceptable for a user-initiated wipe. + // + // PHASE 1: delete every identity's cascade-children + // whose inverse to identity is non-optional (DPNS + // names, DashPay profile, DashPay contact profiles, + // DashPay contact requests, DashPay payments, DashPay + // ignored senders). PublicKey, Document, and + // TokenBalance inverses to identity are already + // Optional and don't need pre-deletion. + // + // Every one of these rows has a non-optional + // `owner: PersistentIdentity`, so omitting any of them + // makes PHASE 2's identity delete hit the SwiftData + // fatal PHASE 1 exists to avoid — aborting the wipe and + // leaving sender-controlled DashPay strings (contact + // profile display name / public message / avatar URL), + // plaintext counterparty/memo/amount/txid (payments), + // and privacy-relevant ignored-sender ids on disk after + // a user-initiated wallet wipe. + for identity in identitiesToDelete { + for name in Array(identity.dpnsNames) { + backgroundContext.delete(name) + } + if let profile = identity.dashpayProfile { + backgroundContext.delete(profile) + } + for contactProfile in Array(identity.contactProfiles) { + backgroundContext.delete(contactProfile) + } + for cr in Array(identity.contactRequests) { + backgroundContext.delete(cr) + } + for payment in Array(identity.dashpayPayments) { + backgroundContext.delete(payment) + } + for ignored in Array(identity.dashpayIgnoredSenders) { + backgroundContext.delete(ignored) } - try backgroundContext.save() } + try backgroundContext.save() - // The txo / pending-input / asset-lock tables are keyed - // by raw `walletId` with no relationship to - // `PersistentWallet`, so the wallet-row delete below - // does not cascade them — purge them explicitly. - // `walletId` is network-scoped (key-wallet folds a - // domain tag + network discriminant into the digest), - // so every row under this id belongs to this wallet on - // this network alone and the purge can't touch a - // sibling network's cached state; a mnemonic's rows on - // other networks live under different walletIds, tied - // together only by `walletGroupId`. - let txoDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(txoDescriptor) { - backgroundContext.delete(row) + // PHASE 2: delete the identities themselves now + // that their problematic cascade children are + // gone from the store. + for identity in identitiesToDelete { + backgroundContext.delete(identity) } + try backgroundContext.save() - let pendingDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(pendingDescriptor) { - backgroundContext.delete(row) + // PHASE 3: delete the wallet's accounts. Same + // reasoning — `PersistentAccount.wallet` is + // non-optional; deleting accounts in their own + // save() pass leaves the wallet's `accounts` + // collection empty when the wallet itself is + // deleted. + let accountsToDelete = Array(walletRow.accounts) + for account in accountsToDelete { + backgroundContext.delete(account) } + try backgroundContext.save() + } - // `loadCachedAssetLocksOnQueue` rehydrates these rows on - // the wallet-load path back into the Rust-side - // `unused_asset_locks` map so an in-flight registration - // can resume across an app kill. Without this cleanup, - // delete-then-reimport of the same wallet would - // resurrect stale Pending / Resumable asset-lock state - // that the user thought they had wiped. - let assetLockDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(assetLockDescriptor) { - backgroundContext.delete(row) - } + // The txo / pending-input / asset-lock tables are keyed + // by raw `walletId` with no relationship to + // `PersistentWallet`, so the wallet-row delete below + // does not cascade them — purge them explicitly. + // `walletId` is network-scoped (key-wallet folds a + // domain tag + network discriminant into the digest), + // so every row under this id belongs to this wallet on + // this network alone and the purge can't touch a + // sibling network's cached state; a mnemonic's rows on + // other networks live under different walletIds, tied + // together only by `walletGroupId`. + let txoDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(txoDescriptor) { + backgroundContext.delete(row) + } - // Shielded (Orchard) per-wallet state. These four - // tables are keyed by raw `walletId` (no relationship - // to `PersistentWallet`), so the wallet-row delete - // below does not cascade them — purge them explicitly - // or they leak after a wipe and could resurface / - // mis-attribute if the same `walletId` is reimported - // (activity rows rehydrate into Rust via the - // `on_load_shielded_activity_fn` callback as ghost - // history and suppress fresh scan-derived entries). - let shieldedNoteDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(shieldedNoteDescriptor) { - backgroundContext.delete(row) - } + let pendingDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(pendingDescriptor) { + backgroundContext.delete(row) + } - let shieldedOutgoingNoteDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(shieldedOutgoingNoteDescriptor) { - backgroundContext.delete(row) - } + // `loadCachedAssetLocksOnQueue` rehydrates these rows on + // the wallet-load path back into the Rust-side + // `unused_asset_locks` map so an in-flight registration + // can resume across an app kill. Without this cleanup, + // delete-then-reimport of the same wallet would + // resurrect stale Pending / Resumable asset-lock state + // that the user thought they had wiped. + let assetLockDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(assetLockDescriptor) { + backgroundContext.delete(row) + } - let shieldedSyncStateDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(shieldedSyncStateDescriptor) { - backgroundContext.delete(row) - } + // Shielded (Orchard) per-wallet state. These four + // tables are keyed by raw `walletId` (no relationship + // to `PersistentWallet`), so the wallet-row delete + // below does not cascade them — purge them explicitly + // or they leak after a wipe and could resurface / + // mis-attribute if the same `walletId` is reimported + // (activity rows rehydrate into Rust via the + // `on_load_shielded_activity_fn` callback as ghost + // history and suppress fresh scan-derived entries). + let shieldedNoteDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedNoteDescriptor) { + backgroundContext.delete(row) + } - let shieldedActivityDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(shieldedActivityDescriptor) { - backgroundContext.delete(row) - } + let shieldedOutgoingNoteDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedOutgoingNoteDescriptor) { + backgroundContext.delete(row) + } - let shieldedViewingKeyDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(shieldedViewingKeyDescriptor) { - backgroundContext.delete(row) - } + let shieldedSyncStateDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedSyncStateDescriptor) { + backgroundContext.delete(row) + } - // Masternode aggregation rows, keyed by raw `walletId` (no - // relationship to `PersistentWallet`), so the wallet-row - // delete below does not cascade them. `MasternodeSync` never - // prunes on an empty aggregation (its no-prune-on-empty - // rule), so without an explicit purge a delete-then-reimport - // of the same wallet resurrects stale masternode rows - // indefinitely. - let masternodeDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == walletId } - ) - for row in try backgroundContext.fetch(masternodeDescriptor) { - backgroundContext.delete(row) - } + let shieldedActivityDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedActivityDescriptor) { + backgroundContext.delete(row) + } - if let walletRow = walletRow { - backgroundContext.delete(walletRow) - } + let shieldedViewingKeyDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(shieldedViewingKeyDescriptor) { + backgroundContext.delete(row) + } - try backgroundContext.save() + // Masternode aggregation rows, keyed by raw `walletId` (no + // relationship to `PersistentWallet`), so the wallet-row + // delete below does not cascade them. `MasternodeSync` never + // prunes on an empty aggregation (its no-prune-on-empty + // rule), so without an explicit purge a delete-then-reimport + // of the same wallet resurrects stale masternode rows + // indefinitely. + let masternodeDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + for row in try backgroundContext.fetch(masternodeDescriptor) { + backgroundContext.delete(row) + } - // Orphan sweep: drop tx rows no longer referenced by any - // wallet. A row is referenced through the TXO graph - // (outputs / inputs / pendingInputs) OR through the - // `involvedAccounts` join — payload-only special txs - // (e.g. a ProRegTx matching a provider owner key) have - // no TXOs anywhere yet legitimately belong to a live - // account, so sweeping on the TXO relations alone would - // erase another wallet's payload-only history. The - // deleted wallet's own payload-only rows still qualify: - // its accounts were deleted (and their join links - // nullified) in the earlier save above. - let txRows = try backgroundContext.fetch(FetchDescriptor()) - for tx in txRows where tx.outputs.isEmpty && - tx.inputs.isEmpty && - tx.pendingInputs.isEmpty && - tx.involvedAccounts.isEmpty { - backgroundContext.delete(tx) - } + if let walletRow = walletRow { + backgroundContext.delete(walletRow) + } - if let walletNetwork = walletNetwork { - let networkRaw = walletNetwork.rawValue - let siblingDescriptor = FetchDescriptor( - predicate: #Predicate { $0.networkRaw == networkRaw } + try backgroundContext.save() + + // Orphan sweep: drop tx rows no longer referenced by any + // wallet. A row is referenced through the TXO graph + // (outputs / inputs / pendingInputs) OR through the + // `involvedAccounts` join — payload-only special txs + // (e.g. a ProRegTx matching a provider owner key) have + // no TXOs anywhere yet legitimately belong to a live + // account, so sweeping on the TXO relations alone would + // erase another wallet's payload-only history. The + // deleted wallet's own payload-only rows still qualify: + // its accounts were deleted (and their join links + // nullified) in the earlier save above. + let txRows = try backgroundContext.fetch(FetchDescriptor()) + for tx in txRows where tx.outputs.isEmpty && + tx.inputs.isEmpty && + tx.pendingInputs.isEmpty && + tx.involvedAccounts.isEmpty { + backgroundContext.delete(tx) + } + + if let walletNetwork = walletNetwork { + let networkRaw = walletNetwork.rawValue + let siblingDescriptor = FetchDescriptor( + predicate: #Predicate { $0.networkRaw == networkRaw } + ) + let remaining = try backgroundContext.fetch(siblingDescriptor) + .filter { $0.walletId != walletId } + if remaining.isEmpty { + let scopeId = syncStateScopeId(for: walletNetwork) + let syncDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == scopeId } ) - let remaining = try backgroundContext.fetch(siblingDescriptor) - .filter { $0.walletId != walletId } - if remaining.isEmpty { - let scopeId = syncStateScopeId(for: walletNetwork) - let syncDescriptor = FetchDescriptor( - predicate: #Predicate { $0.walletId == scopeId } - ) - if let syncRow = try backgroundContext.fetch(syncDescriptor).first { - backgroundContext.delete(syncRow) - } + if let syncRow = try backgroundContext.fetch(syncDescriptor).first { + backgroundContext.delete(syncRow) } } - - try backgroundContext.save() - } catch { - backgroundContext.rollback() - throw error } + + try backgroundContext.save() + } catch { + backgroundContext.rollback() + throw error } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift index 279e10da6f..81fb24241b 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift @@ -959,7 +959,7 @@ struct WalletInfoView: View { // identities the @Relationship rule doesn't reach), and the // Keychain mnemonic + metadata blobs. do { - try walletManager.deleteWallet(walletId: walletId) + try await walletManager.deleteWallet(walletId: walletId) } catch { SDKLogger.error( "Failed to fully delete wallet: \(error.localizedDescription)" diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift index fd151ba4a9..6d5365ec1b 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift @@ -4,7 +4,65 @@ import XCTest final class WalletDeletionTests: XCTestCase { - func testDeleteWalletDataRemovesWalletFootprintAndLastNetworkSyncState() throws { + func testDeleteWalletDataRemovesWalletAndIsIdempotent() async throws { + let container = try DashModelContainer.createInMemory() + let context = ModelContext(container) + let walletId = Data(repeating: 0x31, count: 32) + let identityId = Data(repeating: 0x32, count: 32) + + let wallet = PersistentWallet(walletId: walletId, network: .testnet) + let identity = PersistentIdentity(identityId: identityId, network: .testnet) + identity.wallet = wallet + wallet.identities.append(identity) + context.insert(wallet) + context.insert(identity) + try context.save() + + let handler = PlatformWalletPersistenceHandler( + modelContainer: container, + network: .testnet) + try await handler.deleteWalletData(walletId: walletId) + try await handler.deleteWalletData(walletId: walletId) + + XCTAssertTrue(try fetch(PersistentWallet.self, in: container).isEmpty) + XCTAssertTrue(try fetch(PersistentIdentity.self, in: container).isEmpty) + } + + func testDeleteWalletDataDoesNotBlockMainActor() async throws { + let container = try DashModelContainer.createInMemory() + let handler = PlatformWalletPersistenceHandler( + modelContainer: container, + network: .testnet) + let blockerStarted = expectation(description: "persistence blocker started") + let heartbeatRan = expectation(description: "main actor heartbeat ran") + let releaseBlocker = DispatchSemaphore(value: 0) + + handler.enqueuePersistenceOperationForTesting { + blockerStarted.fulfill() + releaseBlocker.wait() + } + await fulfillment(of: [blockerStarted], timeout: 1) + + let deletion = Task { @MainActor in + try await handler.deleteWalletData( + walletId: Data(repeating: 0x33, count: 32)) + } + await Task.yield() + Task { @MainActor in + heartbeatRan.fulfill() + } + + DispatchQueue.global().asyncAfter(deadline: .now() + 1) { + // Safety release prevents a failed liveness assertion from leaving + // the persistence queue (and the test process) deadlocked. + releaseBlocker.signal() + } + await fulfillment(of: [heartbeatRan], timeout: 0.5) + releaseBlocker.signal() + try await deletion.value + } + + func testDeleteWalletDataRemovesWalletFootprintAndLastNetworkSyncState() async throws { let container = try DashModelContainer.createInMemory() let context = ModelContext(container) let walletId = Data(repeating: 0x44, count: 32) @@ -74,8 +132,8 @@ final class WalletDeletionTests: XCTestCase { try context.save() let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) - try handler.deleteWalletData(walletId: walletId) - try handler.deleteWalletData(walletId: walletId) + try await handler.deleteWalletData(walletId: walletId) + try await handler.deleteWalletData(walletId: walletId) XCTAssertTrue(try fetch(PersistentWallet.self, in: container).isEmpty) XCTAssertTrue(try fetch(PersistentIdentity.self, in: container).isEmpty) @@ -88,7 +146,7 @@ final class WalletDeletionTests: XCTestCase { XCTAssertEqual(transactions.first?.txid, liveTx.txid) } - func testDeleteWalletDataRemovesAssetLockRowsAndPreservesOtherWallets() throws { + func testDeleteWalletDataRemovesAssetLockRowsAndPreservesOtherWallets() async throws { // Regression test for the Codex P2 wallet-deletion finding on // dashpay/platform#3634: `deleteWalletData` previously left // `PersistentAssetLock` rows behind, so the wallet-load path @@ -128,7 +186,7 @@ final class WalletDeletionTests: XCTestCase { try context.save() let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) - try handler.deleteWalletData(walletId: walletId) + try await handler.deleteWalletData(walletId: walletId) let remaining = try fetch(PersistentAssetLock.self, in: container) XCTAssertEqual(remaining.count, 1) @@ -136,7 +194,7 @@ final class WalletDeletionTests: XCTestCase { XCTAssertEqual(remaining.first?.outPointHex, "cafebabe:1") } - func testDeleteWalletDataPreservesSiblingPayloadOnlyTransactions() throws { + func testDeleteWalletDataPreservesSiblingPayloadOnlyTransactions() async throws { // Regression test for the thepastaclaw blocking finding on // dashpay/platform#4108: the post-delete orphan sweep matched // any `PersistentTransaction` with empty outputs / inputs / @@ -190,7 +248,7 @@ final class WalletDeletionTests: XCTestCase { try context.save() let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) - try handler.deleteWalletData(walletId: doomedId) + try await handler.deleteWalletData(walletId: doomedId) // The sibling's payload-only tx survives with its join intact; // the deleted wallet's own payload-only tx is swept. @@ -202,7 +260,7 @@ final class WalletDeletionTests: XCTestCase { ) } - func testDeleteWalletDataKeepsNetworkSyncStateWhenSiblingWalletRemains() throws { + func testDeleteWalletDataKeepsNetworkSyncStateWhenSiblingWalletRemains() async throws { let container = try DashModelContainer.createInMemory() let context = ModelContext(container) let walletId = Data(repeating: 0x99, count: 32) @@ -222,7 +280,7 @@ final class WalletDeletionTests: XCTestCase { try context.save() let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) - try handler.deleteWalletData(walletId: walletId) + try await handler.deleteWalletData(walletId: walletId) let wallets = try fetch(PersistentWallet.self, in: container) XCTAssertEqual(wallets.map(\.walletId), [siblingId])