From 914c43efe0204021eb58d168e41732dac8576f63 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Fri, 15 May 2026 14:31:20 +0200 Subject: [PATCH 1/3] savepoint --- .../rs-platform-wallet-ffi/src/manager.rs | 43 ++++ .../src/manager/wallet_lifecycle.rs | 52 ++++ .../PlatformWalletManager.swift | 136 ++++++++++ .../PlatformWalletPersistenceHandler.swift | 237 +++++++++++++++++- .../Security/KeychainManager.swift | 66 +++++ .../Core/Views/WalletDetailView.swift | 20 +- 6 files changed, 534 insertions(+), 20 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index 37661da3502..e5571ad2aee 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -214,3 +214,46 @@ pub unsafe extern "C" fn platform_wallet_manager_destroy( } PlatformWalletFFIResult::ok() } + +/// Remove a single wallet from the manager. +/// +/// Drops the wallet from the manager's internal `wallets` map (which +/// `PlatformAddressSyncManager` and the shielded sync coordinator +/// iterate) and from the lower-level `WalletManager` registry, so no +/// further per-wallet sync touchpoints (BLAST address-balance +/// upserts, identity syncs, shielded-pool scans) fire for this id. +/// +/// Idempotent: returns `Ok` even when the wallet isn't registered. +/// The Swift wrapper relies on this so a retry after a partial +/// failure (e.g. SwiftData save succeeded but Keychain wipe threw, +/// or vice versa) doesn't surface a confusing "not found" error. +/// +/// This does **not** destroy any `PlatformWallet` handles previously +/// vended via `platform_wallet_manager_get_wallet` — +/// `platform_wallet_destroy` is still the right call for those. It +/// also does not touch persisted state on the Swift side; the caller +/// (`PlatformWalletManager.deleteWallet` in the Swift SDK) is +/// responsible for the SwiftData + Keychain wipe. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_manager_remove_wallet( + manager_handle: Handle, + wallet_id: *const [u8; 32], +) -> PlatformWalletFFIResult { + check_ptr!(wallet_id); + let wallet_id_value = *wallet_id; + + let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { + runtime().block_on(manager.remove_wallet(&wallet_id_value)) + }); + let result = unwrap_option_or_return!(option); + match result { + Ok(_) => PlatformWalletFFIResult::ok(), + // Idempotency: a wallet that's already gone is the success + // state callers want. Everything else is a real failure. + Err(platform_wallet::PlatformWalletError::WalletNotFound(_)) => PlatformWalletFFIResult::ok(), + Err(e) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorWalletOperation, + format!("Failed to remove wallet {}: {}", hex::encode(wallet_id_value), e), + ), + } +} diff --git a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs index 1042feb440a..782eca977dd 100644 --- a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs +++ b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs @@ -336,10 +336,50 @@ impl PlatformWalletManager

{ } /// Remove a wallet from the manager. + /// + /// Drops the wallet from the manager's `wallets` map (which + /// `PlatformAddressSyncManager` and the shielded sync coordinator + /// iterate) and from the lower-level `WalletManager` registry. As + /// part of the same call, every identity the wallet owns is + /// unregistered from `IdentitySyncManager` — that sub-manager + /// keeps its own `RwLock>` keyed by + /// identity (independent of the wallet map), so without an + /// explicit unregister the per-identity token-balance sync keeps + /// firing after the wallet itself is gone and the persister + /// writes the resulting balances under the zero wallet-id + /// sentinel. + /// + /// Snapshots the identity ids before the wallet is dropped from + /// `wallet_manager`, so the unregister loop has the full list + /// even though `wm.remove_wallet` clears the underlying + /// `identity_manager.wallet_identities` bucket. pub async fn remove_wallet( &self, wallet_id: &WalletId, ) -> Result, PlatformWalletError> { + // Capture identity ids owned by this wallet before removal. + // Snapshot under a short read lock; the unregister calls + // below await on a different lock and would deadlock if we + // held this one. + let owned_identity_ids: Vec = { + let wm = self.wallet_manager.read().await; + match wm.get_wallet_info(wallet_id) { + Some(info) => info + .identity_manager + .wallet_identities + .get(wallet_id) + .map(|inner| { + use dpp::identity::accessors::IdentityGettersV0; + inner + .values() + .map(|managed| managed.identity.id()) + .collect() + }) + .unwrap_or_default(), + None => Vec::new(), + } + }; + let removed = { let mut wallets = self.wallets.write().await; wallets @@ -350,6 +390,18 @@ impl PlatformWalletManager

{ let mut wm = self.wallet_manager.write().await; let _ = wm.remove_wallet(wallet_id); } + + // Identity-sync unregister last, after the wallet is gone + // from both maps. If unregister returned a real error + // (currently it can't — the upstream signature is async + // infallible) we'd still have a fully-removed wallet, which + // is the right state for callers retrying. + for identity_id in &owned_identity_ids { + self.identity_sync_manager + .unregister_identity(identity_id) + .await; + } + Ok(removed) } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 8f6cdc3dc0c..973ef7f6294 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -203,6 +203,12 @@ public class PlatformWalletManager: ObservableObject { } let idData = withUnsafeBytes(of: &walletId) { Data($0) } + // Clear any tombstone left behind by an earlier same-session + // `deleteWallet` for this id (re-import of a previously-wiped + // wallet) so the subsequent `setWalletName` write and any + // post-registration sync rounds actually land in SwiftData. + // No-op when the id was never tombstoned. + persistenceHandler?.clearTombstone(walletId: idData) if let name = name, !name.isEmpty { persistenceHandler?.setWalletName(walletId: idData, name: name) } @@ -245,6 +251,12 @@ public class PlatformWalletManager: ObservableObject { } let idData = withUnsafeBytes(of: &walletId) { Data($0) } + // Clear any tombstone left behind by an earlier same-session + // `deleteWallet` for this id (re-import of a previously-wiped + // wallet) so the subsequent `setWalletName` write and any + // post-registration sync rounds actually land in SwiftData. + // No-op when the id was never tombstoned. + persistenceHandler?.clearTombstone(walletId: idData) if let name = name, !name.isEmpty { persistenceHandler?.setWalletName(walletId: idData, name: name) } @@ -311,6 +323,11 @@ public class PlatformWalletManager: ObservableObject { let managedWallet = ManagedPlatformWallet(handle: walletHandle, walletId: walletId) restored.append(managedWallet) self.wallets[walletId] = managedWallet + // Clear any tombstone for this id so re-loading after + // a same-session delete-then-restart-without-process-exit + // (rare, but possible in tests / unusual flows) lets + // future sync rounds write again. + persistenceHandler.clearTombstone(walletId: walletId) } catch { // Log and skip — one wallet failing doesn't fail the // whole restore. Usually means wallet_id / xpub @@ -322,6 +339,125 @@ public class PlatformWalletManager: ObservableObject { return restored } + // MARK: - Wallet deletion + + /// Fully wipe a wallet: Rust manager-side removal, Swift in-memory + /// drop, SwiftData rows (including orphans the `@Relationship` + /// graph doesn't reach), per-identity Keychain private keys, and + /// `WalletStorage` mnemonic + metadata. + /// + /// Steps run least-to-most-destructive so a partial failure on a + /// retry can pick up where the previous call left off: + /// + /// 1. Tell Rust to drop the wallet from the manager's internal + /// list. The Rust call also unregisters the wallet's + /// identities from `IdentitySyncManager` so per-identity token + /// sync stops firing. Idempotent — a missing wallet returns + /// `Ok`. + /// 2. Remove from `wallets[:]`. `ManagedPlatformWallet.deinit` + /// then runs and `platform_wallet_destroy` drops the + /// handle-registry entry on the Rust side. + /// 3. Wipe SwiftData rows on the persistence handler's serial + /// queue: tombstones the id, deletes `PersistentWallet` + /// (cascading accounts → addresses → TXOs / platformAddresses), + /// explicitly cascades identities + their token balances, + /// sweeps orphan transactions / pending inputs, and drops the + /// network sync state row when this is the last wallet on the + /// network. Returns the cascaded identity ids. Rolls back on + /// failure and rethrows. + /// 4. Wipe per-identity Keychain entries: the `privkey__*` + /// rows via `KeychainManager.deleteAllPrivateKeys(for:)` and + /// the wallet-derived `identity_privkey.` rows via + /// `KeychainManager.deleteAllIdentityPrivateKeys(forWalletId:)`. + /// 5. Wipe `WalletStorage`: metadata blob first, mnemonic last. + /// Mnemonic-last makes the mnemonic the retry anchor — if any + /// step earlier in the sequence fails, the pre-flight in + /// `hasArtifacts` will still see the mnemonic and accept the + /// retry. + /// + /// Throws `notFound` only when no artifact for this id is present + /// anywhere (in-memory, SwiftData, or keychain). Throws on + /// partial failure with the error from the failing step; callers + /// can retry — `notFound` is suppressed as long as any artifact + /// remains. + public func deleteWallet(walletId: Data) throws { + try ensureConfigured() + guard walletId.count == 32 else { + throw PlatformWalletError.invalidParameter( + "walletId must be 32 bytes, got \(walletId.count)" + ) + } + + let storage = WalletStorage() + let hasInMemory = wallets[walletId] != nil + let hasMnemonic = storage.hasMnemonic(for: walletId) + // `metadata(for:)` returns nil for `errSecItemNotFound`. A + // throw here is a real Keychain error (locked / authentication + // / etc.) — propagate rather than guess at presence. + let hasMetadata = try storage.metadata(for: walletId) != nil + + // Tell Rust to drop the wallet from the manager-side + // collection (which also unregisters the wallet's identities + // from IdentitySyncManager). Idempotent — a missing wallet + // returns Ok. + try walletId.withUnsafeBytes { raw in + guard let base = raw.baseAddress?.assumingMemoryBound(to: FFIByteTuple32.self) else { + throw PlatformWalletError.nullPointer( + "wallet_id buffer base address was nil" + ) + } + try platform_wallet_manager_remove_wallet(handle, base).check() + } + + // Drop the in-memory entry. The last strong ref to + // `ManagedPlatformWallet` releases, deinit fires, + // `platform_wallet_destroy` drops the handle. + wallets.removeValue(forKey: walletId) + + // SwiftData wipe — runs on the persistence handler's serial + // queue. Returns the cascaded identity ids (used below to + // drive the keychain wipes) plus whether a `PersistentWallet` + // row was found. + let outcome: PlatformWalletPersistenceHandler.WalletDeletionOutcome + if let persistenceHandler = persistenceHandler { + outcome = try persistenceHandler.deleteWalletData(walletId: walletId) + } else { + outcome = .init(foundWalletRow: false, deletedIdentityIds: []) + } + + // Nothing was present anywhere — surface `notFound` rather + // than silently succeeding. The retry-safety property holds + // because the SwiftData + Rust + keychain wipes above are all + // idempotent on missing state. + if !hasInMemory + && !hasMnemonic + && !hasMetadata + && !outcome.foundWalletRow { + throw PlatformWalletError.notFound( + "No wallet found with id \(walletId.map { String(format: "%02x", $0) }.joined())" + ) + } + + // Per-identity Keychain rows. Two parallel storage schemes: + // * `privkey__` — keyed by identity, + // swept directly. + // * `identity_privkey.` — keyed by + // derivation path, owning wallet recorded in the metadata + // blob; the by-walletId sweep walks the keychain once and + // filters on `IdentityPrivateKeyMetadata.walletId`. + for identityId in outcome.deletedIdentityIds { + _ = KeychainManager.shared.deleteAllPrivateKeys(for: identityId) + } + _ = KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId) + + // WalletStorage last. Metadata first so the mnemonic stays + // alive until the very end — if any earlier step fails on a + // retry, the mnemonic anchors the pre-flight and the retry + // resumes. Both calls are idempotent on missing rows. + try storage.deleteMetadata(for: walletId) + try storage.deleteMnemonic(for: walletId) + } + // MARK: - Per-wallet lookup /// Return the managed wallet with the given 32-byte id, or `nil` diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 965e236e55d..eb6467ff968 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -46,6 +46,24 @@ public class PlatformWalletPersistenceHandler { /// atomically. private var inChangeset = false + /// Wallet ids whose `deleteWalletData` has run this process. + /// Mutated only on `serialQueue`. Every wallet-keyed persister + /// callback short-circuits when its `walletId` is tombstoned — + /// `PlatformWalletManager.deleteWallet` drops the wallet from the + /// Rust manager so no _new_ sync work touches it, but Rust tasks + /// that already cloned the wallet's `Arc` before that call keep + /// running on their captured snapshot. Those late callbacks would + /// otherwise see no `PersistentWallet` row, fall through + /// `ensureWalletRecord`'s create-on-missing path, and resurrect + /// the row we just wiped along with whatever child entity the + /// callback was carrying. + /// + /// Cleared by `clearTombstone(walletId:)` when the wallet is + /// intentionally re-created (`createWallet`) or re-loaded + /// (`loadFromPersistor`). Survives only the current process — a + /// stale Rust task can't outlive process restart anyway. + private var tombstonedWalletIds: Set = [] + public init(modelContainer: ModelContainer, network: Network? = nil) { self.modelContainer = modelContainer self.network = network @@ -90,6 +108,7 @@ public class PlatformWalletPersistenceHandler { entries: [(UInt8, Data, UInt64, UInt32, UInt32, UInt32)] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } for (_, addressHash, balance, nonce, accountIndex, addressIndex) in entries { let descriptor = FetchDescriptor( predicate: #Predicate { $0.addressHash == addressHash } @@ -159,6 +178,7 @@ public class PlatformWalletPersistenceHandler { lastKnownRecentBlock: UInt64 ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } guard let network = walletNetwork(walletId: walletId) else { return } @@ -230,11 +250,10 @@ public class PlatformWalletPersistenceHandler { /// Utxo records so views observing via `@Query` update automatically. func persistWalletChangeset(walletId: Data, changeset: UnsafePointer) { onQueue { + // Stale-callback guard — wallet has been deleted. + guard let wallet = ensureWalletRecord(walletId: walletId) else { return } let cs = changeset.pointee - // Ensure PersistentWallet exists (lightweight upsert). - let wallet = ensureWalletRecord(walletId: walletId) - // Chain update. if cs.has_chain { if cs.chain.has_synced_height { @@ -266,7 +285,15 @@ public class PlatformWalletPersistenceHandler { } /// Find or create the `PersistentWallet` record for this wallet id. - private func ensureWalletRecord(walletId: Data) -> PersistentWallet { + /// + /// Returns `nil` when the wallet has been tombstoned by + /// `deleteWalletData` — a stale persister callback from + /// already-cloned Rust state must not resurrect the row we just + /// wiped. Callers run on `serialQueue` and short-circuit on nil. + private func ensureWalletRecord(walletId: Data) -> PersistentWallet? { + if tombstonedWalletIds.contains(walletId) { + return nil + } let descriptor = FetchDescriptor( predicate: #Predicate { $0.walletId == walletId } ) @@ -278,6 +305,17 @@ public class PlatformWalletPersistenceHandler { return record } + /// Clear a wallet id's tombstone so future persister callbacks + /// for it are accepted again. Called by + /// `PlatformWalletManager.createWallet` / + /// `PlatformWalletManager.loadFromPersistor` after the wallet has + /// been intentionally re-created or re-loaded. + public func clearTombstone(walletId: Data) { + onQueue { + _ = tombstonedWalletIds.remove(walletId) + } + } + /// Look up a `PersistentWallet` to hang on /// `PersistentIdentity.wallet`. Non-creating — returns `nil` if /// no row exists (an identity may arrive before its owning @@ -880,6 +918,7 @@ public class PlatformWalletPersistenceHandler { removed: [Data] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { let identityId = entry.identityId let descriptor = FetchDescriptor( @@ -1182,6 +1221,7 @@ public class PlatformWalletPersistenceHandler { removed: [(identityId: Data, keyId: UInt32)] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { // PersistentPublicKey is keyed on (identity, keyId) via // its parent relationship; fetch by keyId + identityId @@ -1306,6 +1346,7 @@ public class PlatformWalletPersistenceHandler { removals: [TokenBalanceRemovalSnapshot] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } let network = walletNetwork(walletId: walletId) ?? .testnet for entry in upserts { @@ -1442,6 +1483,7 @@ public class PlatformWalletPersistenceHandler { removedIncoming: [ContactRequestRemovalSnapshot] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { let ownerId = entry.ownerIdentityId let ownerDescriptor = FetchDescriptor( @@ -1805,6 +1847,7 @@ public class PlatformWalletPersistenceHandler { entries: [CoreAddressEntrySnapshot] ) { onQueue { + guard !tombstonedWalletIds.contains(walletId) else { return } guard let account = fetchAccount(walletId: walletId, key: accountKey) else { return } @@ -2028,7 +2071,7 @@ public class PlatformWalletPersistenceHandler { /// manager's SDK; birth height is SPV's confirmed tip at creation). func persistWalletMetadata(walletId: Data, network: Network, birthHeight: UInt32) { onQueue { - let wallet = ensureWalletRecord(walletId: walletId) + guard let wallet = ensureWalletRecord(walletId: walletId) else { return } wallet.network = network wallet.birthHeight = birthHeight wallet.lastUpdated = Date() @@ -2042,13 +2085,193 @@ public class PlatformWalletPersistenceHandler { /// travel through a Rust-side callback. public func setWalletName(walletId: Data, name: String) { onQueue { - let wallet = ensureWalletRecord(walletId: walletId) + guard let wallet = ensureWalletRecord(walletId: walletId) else { return } wallet.name = name wallet.lastUpdated = Date() try? backgroundContext.save() } } + /// Result of a `deleteWalletData` call. + public struct WalletDeletionOutcome: Sendable { + /// `true` when a `PersistentWallet` row was found and deleted. + /// `false` when the row was already gone (only orphan / keychain + /// state was reachable from the caller). + public let foundWalletRow: Bool + /// 32-byte identity ids of every `PersistentIdentity` that was + /// cascaded as part of this delete. Surfaced so the caller can + /// drive Keychain wipes (`KeychainManager.deleteAllPrivateKeys`, + /// `KeychainManager.deleteAllIdentityPrivateKeys`) that the + /// SwiftData cascade doesn't reach. + public let deletedIdentityIds: [Data] + } + + /// Wipe a wallet's entire SwiftData footprint. + /// + /// Deletes: + /// + /// - The `PersistentWallet` row, which `@Relationship(.cascade)` + /// takes through `accounts → coreAddresses → txos` and + /// `accounts → platformAddresses`. + /// - The wallet's `PersistentIdentity` rows (an explicit cascade + /// on this single code path — the schema-level rule stays + /// `.nullify` so other paths don't accidentally take identities + /// down). Each identity carries its own cascade chain + /// (`publicKeys`, `dpnsNames`, `dashpayProfile`, + /// `contactRequests`, `documents`). + /// - `PersistentTokenBalance` rows whose `identityId` matches one + /// of the cascaded identities. `PersistentTokenBalance.identity` + /// is `.nullify`, so deleting the identity alone would leave + /// orphan rows behind. + /// - `PersistentPendingInput` rows where the denormalized + /// `walletId` matches. The schema-level cascade is via + /// `PersistentTransaction.pendingInputs`, but transactions + /// aren't wallet-owned, so the pending rows would otherwise + /// linger. + /// - `PersistentTransaction` rows whose `outputs`, `inputs`, and + /// `pendingInputs` are all empty after the above. Transactions + /// are intentionally not wallet-owned (one on-chain tx can fund + /// multiple wallets); the orphan sweep catches the rows that + /// nobody references anymore. + /// - `PersistentPlatformAddressesSyncState` for the wallet's + /// network **only when this was the last wallet on that + /// network**. The BLAST checkpoint is shared across wallets on a + /// network; deleting it while siblings still exist would force + /// them to re-scan, and leaving it after the last sibling is + /// removed would make a fresh import on the same network skip + /// pre-checkpoint balances. + /// + /// Tombstones the wallet id on `serialQueue` before the first + /// delete so any persister callback that arrives later (from Rust + /// work that already cloned the wallet's `Arc`) short-circuits + /// instead of resurrecting the row through `ensureWalletRecord`'s + /// create-on-missing path. The tombstone is cleared by + /// `clearTombstone(walletId:)` from the manager's create / load + /// paths. + /// + /// Atomic: the whole sweep runs inside a single `save()`. On + /// failure, `rollback()` reverts the pending deletes; the + /// tombstone is still cleared so a retry can attempt the wipe + /// again. The thrown error propagates so the caller can leave its + /// other state (Rust manager, in-memory dict, keychain) alone. + @discardableResult + public func deleteWalletData(walletId: Data) throws -> WalletDeletionOutcome { + var thrown: Error? + var foundWalletRow = false + var deletedIdentityIds: [Data] = [] + onQueue { + // Block any concurrent / stale persister callback from + // resurrecting the row we're about to wipe. Set before any + // fetch so even a fetch that races us sees the tombstone + // on its own onQueue entry. + tombstonedWalletIds.insert(walletId) + do { + let walletDescriptor = FetchDescriptor( + predicate: PersistentWallet.predicate(walletId: walletId) + ) + let walletRow = try backgroundContext.fetch(walletDescriptor).first + + // Capture the wallet's network before delete so the + // "last on this network" sync-state cleanup below has + // something to scope against. May be nil for rows that + // were created by a changeset before metadata filled + // network in — in that case we skip the sync-state + // cleanup (caller can wipe via a follow-up restart). + let walletNetwork = walletRow?.network + + if let walletRow = walletRow { + foundWalletRow = true + // Identities first — the schema rule is `.nullify`, + // so deleting the wallet would otherwise leave a + // set of orphan identity rows with `wallet == nil`. + // Snapshot before delete because the relationship + // array becomes unsafe to iterate once SwiftData + // starts processing the wallet's own deletion. + let identitiesToDelete = Array(walletRow.identities) + deletedIdentityIds = identitiesToDelete.map { $0.identityId } + + // PersistentTokenBalance.identity is `.nullify`, + // so without an explicit sweep the balances would + // orphan with `identity = nil`. Predicate-driven + // fetch by `identityId` denorm — no relationship + // walk needed. + for identityId in deletedIdentityIds { + let balanceDescriptor = FetchDescriptor( + predicate: PersistentTokenBalance.predicate(identityId: identityId) + ) + let balanceRows = (try? backgroundContext.fetch(balanceDescriptor)) ?? [] + for row in balanceRows { + backgroundContext.delete(row) + } + } + + for identity in identitiesToDelete { + backgroundContext.delete(identity) + } + backgroundContext.delete(walletRow) + } + + // Pending inputs: walletId is denormalized + indexed, + // direct predicate fetch. + let pendingDescriptor = FetchDescriptor( + predicate: #Predicate { $0.walletId == walletId } + ) + let pendingRows = try backgroundContext.fetch(pendingDescriptor) + for row in pendingRows { + backgroundContext.delete(row) + } + + // Orphan transactions: scan all and drop the ones with + // no surviving relationships. The TXO cascade above + // already emptied this wallet's contributions; what's + // left here is rows that nobody references. + let txDescriptor = FetchDescriptor() + let txRows = try backgroundContext.fetch(txDescriptor) + for tx in txRows where tx.outputs.isEmpty + && tx.inputs.isEmpty + && tx.pendingInputs.isEmpty { + backgroundContext.delete(tx) + } + + // Network sync-state: only drop the row when no other + // wallet on the same network remains. Other wallets + // share that BLAST checkpoint and would lose their + // progress if we cleared it; a fresh import after the + // last wallet on a network is removed would otherwise + // inherit a stale start height. + if let walletNetwork = walletNetwork { + let networkRaw = walletNetwork.rawValue + let siblingDescriptor = FetchDescriptor( + predicate: #Predicate { $0.networkRaw == networkRaw } + ) + let siblings = (try? backgroundContext.fetch(siblingDescriptor)) ?? [] + let remaining = siblings.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) + } + } + } + + try backgroundContext.save() + } catch { + backgroundContext.rollback() + thrown = error + } + } + if let thrown = thrown { + throw thrown + } + return WalletDeletionOutcome( + foundWalletRow: foundWalletRow, + deletedIdentityIds: deletedIdentityIds + ) + } + // MARK: - Watch-only Restore: Account xpub /// Upsert a `PersistentAccount` row with the full `AccountSpecFFI` @@ -2057,7 +2280,7 @@ public class PlatformWalletPersistenceHandler { /// that uniquely identifies an account across variants. func persistAccount(walletId: Data, spec: AccountSpecFFI) { onQueue { - let wallet = ensureWalletRecord(walletId: walletId) + guard let wallet = ensureWalletRecord(walletId: walletId) else { return } let typeTag = UInt32(spec.type_tag) let index = spec.index let registrationIndex = spec.registration_index diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift index b096b2d8043..e370c604f2d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift @@ -747,6 +747,72 @@ extension KeychainManager { let status = SecItemDelete(query as CFDictionary) return status == errSecSuccess || status == errSecItemNotFound } + + /// Delete every `identity_privkey.` keychain row + /// whose `IdentityPrivateKeyMetadata.walletId` matches the given + /// wallet id (32-byte `Data`). Used by + /// `PlatformWalletManager.deleteWallet` to wipe the wallet-derived + /// per-identity signing keys the Rust persister callback path + /// writes — those rows are keyed by derivation path, not identity + /// id, so the per-identity sweep (`deleteAllPrivateKeys(for:)`) + /// alone leaves them behind. + /// + /// Idempotent. Returns `true` after the sweep completes regardless + /// of how many rows matched. Decode errors on individual rows are + /// skipped (the row stays) so a single corrupted metadata blob + /// can't break the whole wipe. + @discardableResult + public nonisolated func deleteAllIdentityPrivateKeys(forWalletId walletId: Data) -> Bool { + let walletIdHex = walletId.map { String(format: "%02x", $0) }.joined() + var query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: serviceName, + kSecMatchLimit as String: kSecMatchLimitAll, + kSecReturnAttributes as String: true, + ] + if let accessGroup = accessGroup { + query[kSecAttrAccessGroup as String] = accessGroup + } + + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + guard status == errSecSuccess, let items = result as? [[String: Any]] else { + // `errSecItemNotFound` is the empty-keychain case — still a + // successful sweep. + return true + } + + let decoder = JSONDecoder() + for item in items { + guard let account = item[kSecAttrAccount as String] as? String, + account.hasPrefix("identity_privkey.") + else { + continue + } + guard let metadataData = item[kSecAttrGeneric as String] as? Data, + let metadata = try? decoder.decode( + IdentityPrivateKeyMetadata.self, + from: metadataData + ) + else { + continue + } + guard metadata.walletId.caseInsensitiveCompare(walletIdHex) == .orderedSame else { + continue + } + + var deleteQuery: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: serviceName, + kSecAttrAccount as String: account, + ] + if let accessGroup = accessGroup { + deleteQuery[kSecAttrAccessGroup as String] = accessGroup + } + SecItemDelete(deleteQuery as CFDictionary) + } + return true + } } // MARK: - Platform-address private-key storage — REMOVED diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift index 1df76b683c0..7fbb2f1450e 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/WalletDetailView.swift @@ -185,6 +185,7 @@ struct WalletDetailView: View { struct WalletInfoView: View { @Environment(\.dismiss) var dismiss @Environment(\.modelContext) var modelContext + @EnvironmentObject var walletManager: PlatformWalletManager let wallet: PersistentWallet var onWalletDeleted: () -> Void = {} @@ -627,19 +628,14 @@ struct WalletInfoView: View { await MainActor.run { isDeleting = true } - // Cascade-delete rules on `accounts` / `identities` null out - // or cascade the children automatically. - modelContext.delete(wallet) + // `PlatformWalletManager.deleteWallet` handles the full wipe: + // Rust manager-side drop, in-memory dict removal, SwiftData + // cascade + orphan sweep (transactions / pending inputs / + // identities the @Relationship rule doesn't reach), and the + // Keychain mnemonic + metadata blobs. do { - try modelContext.save() - let storage = WalletStorage() - try storage.deleteMnemonic(for: walletId) - // Keychain metadata is independent of the mnemonic - // row — clear it here so a deleted wallet doesn't - // leave stale name/description behind. - try storage.deleteMetadata(for: walletId) + try walletManager.deleteWallet(walletId: walletId) } catch { - modelContext.rollback() SDKLogger.error( "Failed to fully delete wallet: \(error.localizedDescription)" ) @@ -656,8 +652,6 @@ struct WalletInfoView: View { dismiss() onWalletDeleted() } - // TODO(platform-wallet): expose wallet removal on PlatformWalletManager - // so the Rust side also drops the in-memory handle. } } From 9ae8fa9a2d2c97e6bcfdba21e87f9545d9da3926 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Fri, 15 May 2026 15:04:14 +0200 Subject: [PATCH 2/3] feat(swift-sdk): deleteWallet wipes full wallet footprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `PlatformWalletManager.deleteWallet(walletId:)` wipes the wallet's Rust manager state, SwiftData rows (including orphans the `@Relationship` graph misses: `PersistentTransaction`, `PersistentPendingInput`, `PersistentTokenBalance`), identity-keyed and wallet-keyed Keychain entries, and the network sync state when no sibling wallets remain. Closes the data-leakage path where `modelContext.delete(wallet)` alone left orphan rows on the same network for the next wallet to see — the bug that prompted this on dashwallet-ios and was present in the SDK's own example app. Rust-side adds: - `platform_wallet_manager_remove_wallet` FFI (idempotent on missing). - `PlatformWalletPersistence::retire_wallet` trait method (default no-op). - `FFIPersister` retired set: drops non-registration `store`/`flush` for retired wallets and auto-clears on a `wallet_metadata`-bearing changeset so a same-session reimport recovers. - `PlatformWalletManager::remove_wallet` snapshots the wallet's identity ids and `IdentitySyncManager::unregister_identity`s each before dropping the wallet, so per-identity token sync stops. - `IdentitySyncManager::apply_fresh_balances` re-checks the live state under its write lock before persisting, closing the race where a mid-sync unregister could still emit a token balance. Swift-side adds: - `PlatformWalletPersistenceHandler.deleteWalletData` + companion `identityIdsForWallet` for the orchestration. - `KeychainManager.deleteAllKeychainItems(forIdentityId:)` (sweeps both `privkey_*` and `specialkey_*` schemes) and `deleteAllIdentityPrivateKeysForWallet(walletId:)` throwing variants so partial-failure surfaces instead of silently passing. Tests: - `WalletDeletionTests` (Swift, in-memory `ModelContainer`): cascade + orphan-tx sweep, idempotency, network-sync conditional cleanup (both branches), and per-scheme Keychain sweeps. - `retired_wallet_drops_non_registration_store_and_flush` and `registration_store_clears_retirement` (Rust): persister retirement semantics. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../rs-platform-wallet-ffi/src/manager.rs | 30 +- .../rs-platform-wallet-ffi/src/persistence.rs | 159 +++++++- .../src/changeset/traits.rs | 7 + .../src/manager/identity_sync.rs | 156 +++++--- .../src/manager/wallet_lifecycle.rs | 30 +- .../PlatformWalletManager.swift | 116 +----- .../PlatformWalletPersistenceHandler.swift | 343 +++++------------- .../Security/KeychainManager.swift | 118 +++--- .../KeyManagerTests.swift | 2 +- .../WalletDeletionTests.swift | 208 +++++++++++ 10 files changed, 661 insertions(+), 508 deletions(-) create mode 100644 packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index e5571ad2aee..aa00c0831c6 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -215,25 +215,7 @@ pub unsafe extern "C" fn platform_wallet_manager_destroy( PlatformWalletFFIResult::ok() } -/// Remove a single wallet from the manager. -/// -/// Drops the wallet from the manager's internal `wallets` map (which -/// `PlatformAddressSyncManager` and the shielded sync coordinator -/// iterate) and from the lower-level `WalletManager` registry, so no -/// further per-wallet sync touchpoints (BLAST address-balance -/// upserts, identity syncs, shielded-pool scans) fire for this id. -/// -/// Idempotent: returns `Ok` even when the wallet isn't registered. -/// The Swift wrapper relies on this so a retry after a partial -/// failure (e.g. SwiftData save succeeded but Keychain wipe threw, -/// or vice versa) doesn't surface a confusing "not found" error. -/// -/// This does **not** destroy any `PlatformWallet` handles previously -/// vended via `platform_wallet_manager_get_wallet` — -/// `platform_wallet_destroy` is still the right call for those. It -/// also does not touch persisted state on the Swift side; the caller -/// (`PlatformWalletManager.deleteWallet` in the Swift SDK) is -/// responsible for the SwiftData + Keychain wipe. +/// Remove one wallet from the manager. Idempotent on missing wallets. #[no_mangle] pub unsafe extern "C" fn platform_wallet_manager_remove_wallet( manager_handle: Handle, @@ -250,10 +232,16 @@ pub unsafe extern "C" fn platform_wallet_manager_remove_wallet( Ok(_) => PlatformWalletFFIResult::ok(), // Idempotency: a wallet that's already gone is the success // state callers want. Everything else is a real failure. - Err(platform_wallet::PlatformWalletError::WalletNotFound(_)) => PlatformWalletFFIResult::ok(), + Err(platform_wallet::PlatformWalletError::WalletNotFound(_)) => { + PlatformWalletFFIResult::ok() + } Err(e) => PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, - format!("Failed to remove wallet {}: {}", hex::encode(wallet_id_value), e), + format!( + "Failed to remove wallet {}: {}", + hex::encode(wallet_id_value), + e + ), ), } } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 4fc7ddba2df..4d395ab7f2e 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -23,7 +23,7 @@ use platform_wallet::changeset::{ }; use platform_wallet::wallet::platform_wallet::WalletId; use platform_wallet::wallet::{PerAccountPlatformAddressState, PerWalletPlatformAddressState}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::ffi::CString; use std::os::raw::c_void; use std::slice; @@ -362,10 +362,39 @@ pub struct PersistenceCallbacks { unsafe impl Send for PersistenceCallbacks {} unsafe impl Sync for PersistenceCallbacks {} +impl Default for PersistenceCallbacks { + fn default() -> Self { + Self { + context: std::ptr::null_mut(), + on_changeset_begin_fn: None, + on_changeset_end_fn: None, + on_store_fn: None, + on_flush_fn: None, + on_persist_address_balances_fn: None, + on_persist_wallet_changeset_fn: None, + on_persist_sync_state_fn: None, + on_persist_account_registrations_fn: None, + on_load_wallet_list_fn: None, + on_load_wallet_list_free_fn: None, + on_persist_wallet_metadata_fn: None, + on_persist_account_address_pools_fn: None, + on_persist_identities_fn: None, + on_persist_identity_keys_fn: None, + on_persist_token_balances_fn: None, + on_persist_contacts_fn: None, + on_get_core_tx_record_fn: None, + on_get_core_tx_record_free_fn: None, + } + } +} + /// In-memory persister that accumulates changesets and notifies via callbacks. pub struct FFIPersister { callbacks: PersistenceCallbacks, pending: RwLock>, + /// Wallet ids removed from the manager. Stale handles may still call + /// `store`; registration metadata clears retirement for same-id reimport. + retired: RwLock>, } impl FFIPersister { @@ -373,6 +402,7 @@ impl FFIPersister { Self { callbacks, pending: RwLock::new(BTreeMap::new()), + retired: RwLock::new(BTreeSet::new()), } } } @@ -383,6 +413,14 @@ impl PlatformWalletPersistence for FFIPersister { wallet_id: WalletId, changeset: PlatformWalletChangeSet, ) -> Result<(), PersistenceError> { + if wallet_id != WalletId::default() { + if changeset.wallet_metadata.is_some() { + self.retired.write().remove(&wallet_id); + } else if self.retired.read().contains(&wallet_id) { + return Ok(()); + } + } + // Bracket the whole per-kind callback sequence with a // begin/end pair so clients (Swift, etc.) can treat the // round as a single atomic transaction: begin opens a @@ -909,6 +947,10 @@ impl PlatformWalletPersistence for FFIPersister { } fn flush(&self, wallet_id: WalletId) -> Result<(), PersistenceError> { + if wallet_id != WalletId::default() && self.retired.read().contains(&wallet_id) { + return Ok(()); + } + // Notify caller. if let Some(cb) = self.callbacks.on_flush_fn { let result = unsafe { cb(self.callbacks.context, wallet_id.as_ptr()) }; @@ -926,6 +968,15 @@ impl PlatformWalletPersistence for FFIPersister { Ok(()) } + fn retire_wallet(&self, wallet_id: WalletId) { + if wallet_id == WalletId::default() { + return; + } + + self.pending.write().remove(&wallet_id); + self.retired.write().insert(wallet_id); + } + fn load(&self) -> Result { // If Swift hasn't wired up `on_load_wallet_list_fn` there's // nothing to restore — treat as a fresh client. @@ -2287,3 +2338,109 @@ unsafe fn slice_from_raw<'a>(ptr: *const u8, len: usize) -> &'a [u8] { slice::from_raw_parts(ptr, len) } } + +#[cfg(test)] +mod tests { + use super::*; + + use platform_wallet::changeset::WalletMetadataEntry; + use std::sync::atomic::{AtomicUsize, Ordering}; + + struct CallbackCounts { + metadata: AtomicUsize, + stores: AtomicUsize, + flushes: AtomicUsize, + } + + impl CallbackCounts { + fn new() -> Self { + Self { + metadata: AtomicUsize::new(0), + stores: AtomicUsize::new(0), + flushes: AtomicUsize::new(0), + } + } + } + + unsafe extern "C" fn metadata_callback( + context: *mut c_void, + _wallet_id: *const u8, + _network: FFINetwork, + _birth_height: u32, + ) -> i32 { + let counts = &*(context as *const CallbackCounts); + counts.metadata.fetch_add(1, Ordering::SeqCst); + 0 + } + + unsafe extern "C" fn store_callback(context: *mut c_void, _wallet_id: *const u8) -> i32 { + let counts = &*(context as *const CallbackCounts); + counts.stores.fetch_add(1, Ordering::SeqCst); + 0 + } + + unsafe extern "C" fn flush_callback(context: *mut c_void, _wallet_id: *const u8) -> i32 { + let counts = &*(context as *const CallbackCounts); + counts.flushes.fetch_add(1, Ordering::SeqCst); + 0 + } + + fn callbacks(counts: &CallbackCounts) -> PersistenceCallbacks { + PersistenceCallbacks { + context: counts as *const CallbackCounts as *mut c_void, + on_persist_wallet_metadata_fn: Some(metadata_callback), + on_store_fn: Some(store_callback), + on_flush_fn: Some(flush_callback), + ..Default::default() + } + } + + #[test] + fn retired_wallet_drops_non_registration_store_and_flush() { + let counts = CallbackCounts::new(); + let persister = FFIPersister::new(callbacks(&counts)); + let wallet_id = [7u8; 32]; + + persister + .store(wallet_id, PlatformWalletChangeSet::default()) + .unwrap(); + assert_eq!(counts.stores.load(Ordering::SeqCst), 1); + assert!(persister.pending.read().contains_key(&wallet_id)); + + persister.retire_wallet(wallet_id); + assert!(!persister.pending.read().contains_key(&wallet_id)); + + persister + .store(wallet_id, PlatformWalletChangeSet::default()) + .unwrap(); + persister.flush(wallet_id).unwrap(); + + assert_eq!(counts.metadata.load(Ordering::SeqCst), 0); + assert_eq!(counts.stores.load(Ordering::SeqCst), 1); + assert_eq!(counts.flushes.load(Ordering::SeqCst), 0); + } + + #[test] + fn registration_store_clears_retirement() { + let counts = CallbackCounts::new(); + let persister = FFIPersister::new(callbacks(&counts)); + let wallet_id = [8u8; 32]; + + persister.retire_wallet(wallet_id); + + let registration = PlatformWalletChangeSet { + wallet_metadata: Some(WalletMetadataEntry { + network: key_wallet::Network::Testnet, + birth_height: 42, + }), + ..Default::default() + }; + persister.store(wallet_id, registration).unwrap(); + persister.flush(wallet_id).unwrap(); + + assert_eq!(counts.metadata.load(Ordering::SeqCst), 1); + assert_eq!(counts.stores.load(Ordering::SeqCst), 1); + assert_eq!(counts.flushes.load(Ordering::SeqCst), 1); + assert!(!persister.retired.read().contains(&wallet_id)); + } +} diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index 1e567e451ed..685a5b24830 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -134,6 +134,13 @@ pub trait PlatformWalletPersistence: Send + Sync { /// clear that wallet's buffer. fn flush(&self, wallet_id: WalletId) -> Result<(), PersistenceError>; + /// Stop accepting future writes for a wallet removed from the manager. + /// + /// Persisters that can receive callbacks from stale wallet handles should + /// drop any buffered state and ignore later non-registration writes for this + /// id. Backends without an in-memory callback gate can use the default no-op. + fn retire_wallet(&self, _wallet_id: WalletId) {} + /// Load the full client state from storage. /// /// Returns a [`ClientStartState`] — a ready-to-boot snapshot covering diff --git a/packages/rs-platform-wallet/src/manager/identity_sync.rs b/packages/rs-platform-wallet/src/manager/identity_sync.rs index b998ea73e01..7023190d91f 100644 --- a/packages/rs-platform-wallet/src/manager/identity_sync.rs +++ b/packages/rs-platform-wallet/src/manager/identity_sync.rs @@ -541,7 +541,14 @@ where return; } - // Build the changeset and update our own cache in lockstep. + self.apply_fresh_balances(identity_id, fresh_balances).await; + } + + async fn apply_fresh_balances( + &self, + identity_id: Identifier, + fresh_balances: BTreeMap>, + ) { let mut cs = TokenBalanceChangeSet::default(); for (token_id, maybe_balance) in &fresh_balances { let key = (identity_id, *token_id); @@ -555,6 +562,16 @@ where } } + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + + let mut state = self.state.write().await; + let Some(existing_row) = state.get(&identity_id).cloned() else { + return; + }; + // The persister API is wallet-scoped (`store(wallet_id, ..)`) // but this manager is identity-scoped. Use the zero-byte // sentinel — the FFI / SQLite token-balance write paths key @@ -569,67 +586,31 @@ where ); } - // TODO(identity-sync nonce): once token-id → contract-id - // resolution lands on the registry (currently keyed by token - // id only), fetch the per-(identity, contract) nonce here via - // `self.sdk.get_identity_contract_nonce(identity_id, - // contract_id, false, None).await` and replicate it onto - // every token row that shares the same contract. The - // `IdentityTokenSyncInfo::contract_id` field is plumbed - // through with a `Identifier::default()` placeholder so the - // FFI mirror shape doesn't have to change when this lands. - - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_secs()) - .unwrap_or(0); - - // Rewrite the per-identity cache row from the freshly fetched - // balances. Tokens that returned `None` (i.e. removed on - // Platform) drop out of the row; tokens that returned `Some` - // get the new balance. We rebuild rather than splice so that - // the row always reflects the latest watched-token set - // intersected with what Platform reports. - let mut state = self.state.write().await; - if let Some(existing_row) = state.get(&identity_id).cloned() { - // Rebuild from the *live* row (which may have been mutated - // by concurrent `update_watched_tokens` / `unregister_identity` - // while our network calls were in flight) rather than the - // stale `token_ids` snapshot. This way mid-sync registry - // changes are preserved: newly added tokens keep their - // initial state, and tokens removed during the pass stay - // removed. - let mut new_tokens: Vec = - Vec::with_capacity(existing_row.tokens.len()); - for prior in &existing_row.tokens { - match fresh_balances.get(&prior.token_id) { - Some(Some(amount)) => { - new_tokens.push(IdentityTokenSyncInfo { - balance: *amount, - ..*prior - }); - } - Some(None) => { - // Platform reported the token removed for - // this identity — drop the row. - } - None => { - // Batch didn't cover this token (added mid- - // sync, or batch failed) — keep prior state. - new_tokens.push(*prior); - } + let mut new_tokens: Vec = + Vec::with_capacity(existing_row.tokens.len()); + for prior in &existing_row.tokens { + match fresh_balances.get(&prior.token_id) { + Some(Some(amount)) => { + new_tokens.push(IdentityTokenSyncInfo { + balance: *amount, + ..*prior + }); + } + Some(None) => {} + None => { + new_tokens.push(*prior); } } + } - state.insert( + state.insert( + identity_id, + IdentityTokenSyncState { identity_id, - IdentityTokenSyncState { - identity_id, - last_sync_unix: now, - tokens: new_tokens, - }, - ); - } + last_sync_unix: now, + tokens: new_tokens, + }, + ); } } @@ -652,6 +633,7 @@ mod tests { use super::*; use crate::changeset::{ClientStartState, PersistenceError, PlatformWalletChangeSet}; + use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; /// Test-only persister that swallows every `store` call and /// records nothing. Lifecycle / registry tests don't need the @@ -678,6 +660,37 @@ mod tests { } } + struct RecordingPersister { + stores: AtomicUsize, + } + + impl RecordingPersister { + fn new() -> Self { + Self { + stores: AtomicUsize::new(0), + } + } + } + + impl PlatformWalletPersistence for RecordingPersister { + fn store( + &self, + _wallet_id: WalletId, + _changeset: PlatformWalletChangeSet, + ) -> Result<(), PersistenceError> { + self.stores.fetch_add(1, AtomicOrdering::SeqCst); + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { + Ok(()) + } + + fn load(&self) -> Result { + Ok(ClientStartState::default()) + } + } + /// Build a manager wired to a no-op persister. The SDK is /// constructed via `SdkBuilder::new_mock` so we don't need a /// running runtime for the registry/lifecycle tests below; none @@ -688,6 +701,18 @@ mod tests { Arc::new(IdentitySyncManager::new(sdk, persister)) } + fn make_recording_manager() -> ( + Arc>, + Arc, + ) { + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let persister = Arc::new(RecordingPersister::new()); + ( + Arc::new(IdentitySyncManager::new(sdk, Arc::clone(&persister))), + persister, + ) + } + /// `register_identity` populates a row with zero-balance /// placeholders for each token, and `state_for_identity` returns /// the cloned row. Validates the read API the FFI snapshot path @@ -770,6 +795,21 @@ mod tests { mgr.unregister_identity(&Identifier::from([99u8; 32])).await; } + #[tokio::test] + async fn unregistered_identity_does_not_persist_fresh_balances() { + let (mgr, persister) = make_recording_manager(); + let id_a = Identifier::from([1u8; 32]); + let token_x = Identifier::from([10u8; 32]); + let mut fresh = BTreeMap::new(); + fresh.insert(token_x, Some(5u64)); + + mgr.register_identity(id_a, [token_x]).await; + mgr.unregister_identity(&id_a).await; + mgr.apply_fresh_balances(id_a, fresh).await; + + assert_eq!(persister.stores.load(AtomicOrdering::SeqCst), 0); + } + /// `set_interval` clamps to >=1s and is read back via `interval`. /// Default interval matches the documented constant. Pinned so /// future tuning surfaces in the test suite. diff --git a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs index 782eca977dd..017a9f36756 100644 --- a/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs +++ b/packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs @@ -335,32 +335,13 @@ impl PlatformWalletManager

{ Ok(platform_wallet) } - /// Remove a wallet from the manager. - /// - /// Drops the wallet from the manager's `wallets` map (which - /// `PlatformAddressSyncManager` and the shielded sync coordinator - /// iterate) and from the lower-level `WalletManager` registry. As - /// part of the same call, every identity the wallet owns is - /// unregistered from `IdentitySyncManager` — that sub-manager - /// keeps its own `RwLock>` keyed by - /// identity (independent of the wallet map), so without an - /// explicit unregister the per-identity token-balance sync keeps - /// firing after the wallet itself is gone and the persister - /// writes the resulting balances under the zero wallet-id - /// sentinel. - /// - /// Snapshots the identity ids before the wallet is dropped from - /// `wallet_manager`, so the unregister loop has the full list - /// even though `wm.remove_wallet` clears the underlying - /// `identity_manager.wallet_identities` bucket. + /// Remove a wallet from the manager and retire its persister writes. pub async fn remove_wallet( &self, wallet_id: &WalletId, ) -> Result, PlatformWalletError> { - // Capture identity ids owned by this wallet before removal. - // Snapshot under a short read lock; the unregister calls - // below await on a different lock and would deadlock if we - // held this one. + self.persister.retire_wallet(*wallet_id); + let owned_identity_ids: Vec = { let wm = self.wallet_manager.read().await; match wm.get_wallet_info(wallet_id) { @@ -391,11 +372,6 @@ impl PlatformWalletManager

{ let _ = wm.remove_wallet(wallet_id); } - // Identity-sync unregister last, after the wallet is gone - // from both maps. If unregister returned a real error - // (currently it can't — the upstream signature is async - // infallible) we'd still have a fully-removed wallet, which - // is the right state for callers retrying. for identity_id in &owned_identity_ids { self.identity_sync_manager .unregister_identity(identity_id) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 973ef7f6294..8cf31bb03ad 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -203,12 +203,6 @@ public class PlatformWalletManager: ObservableObject { } let idData = withUnsafeBytes(of: &walletId) { Data($0) } - // Clear any tombstone left behind by an earlier same-session - // `deleteWallet` for this id (re-import of a previously-wiped - // wallet) so the subsequent `setWalletName` write and any - // post-registration sync rounds actually land in SwiftData. - // No-op when the id was never tombstoned. - persistenceHandler?.clearTombstone(walletId: idData) if let name = name, !name.isEmpty { persistenceHandler?.setWalletName(walletId: idData, name: name) } @@ -251,12 +245,6 @@ public class PlatformWalletManager: ObservableObject { } let idData = withUnsafeBytes(of: &walletId) { Data($0) } - // Clear any tombstone left behind by an earlier same-session - // `deleteWallet` for this id (re-import of a previously-wiped - // wallet) so the subsequent `setWalletName` write and any - // post-registration sync rounds actually land in SwiftData. - // No-op when the id was never tombstoned. - persistenceHandler?.clearTombstone(walletId: idData) if let name = name, !name.isEmpty { persistenceHandler?.setWalletName(walletId: idData, name: name) } @@ -323,11 +311,6 @@ public class PlatformWalletManager: ObservableObject { let managedWallet = ManagedPlatformWallet(handle: walletHandle, walletId: walletId) restored.append(managedWallet) self.wallets[walletId] = managedWallet - // Clear any tombstone for this id so re-loading after - // a same-session delete-then-restart-without-process-exit - // (rare, but possible in tests / unusual flows) lets - // future sync rounds write again. - persistenceHandler.clearTombstone(walletId: walletId) } catch { // Log and skip — one wallet failing doesn't fail the // whole restore. Usually means wallet_id / xpub @@ -341,45 +324,9 @@ public class PlatformWalletManager: ObservableObject { // MARK: - Wallet deletion - /// Fully wipe a wallet: Rust manager-side removal, Swift in-memory - /// drop, SwiftData rows (including orphans the `@Relationship` - /// graph doesn't reach), per-identity Keychain private keys, and - /// `WalletStorage` mnemonic + metadata. + /// Fully wipe a wallet's Rust, SwiftData, and Keychain footprint. /// - /// Steps run least-to-most-destructive so a partial failure on a - /// retry can pick up where the previous call left off: - /// - /// 1. Tell Rust to drop the wallet from the manager's internal - /// list. The Rust call also unregisters the wallet's - /// identities from `IdentitySyncManager` so per-identity token - /// sync stops firing. Idempotent — a missing wallet returns - /// `Ok`. - /// 2. Remove from `wallets[:]`. `ManagedPlatformWallet.deinit` - /// then runs and `platform_wallet_destroy` drops the - /// handle-registry entry on the Rust side. - /// 3. Wipe SwiftData rows on the persistence handler's serial - /// queue: tombstones the id, deletes `PersistentWallet` - /// (cascading accounts → addresses → TXOs / platformAddresses), - /// explicitly cascades identities + their token balances, - /// sweeps orphan transactions / pending inputs, and drops the - /// network sync state row when this is the last wallet on the - /// network. Returns the cascaded identity ids. Rolls back on - /// failure and rethrows. - /// 4. Wipe per-identity Keychain entries: the `privkey__*` - /// rows via `KeychainManager.deleteAllPrivateKeys(for:)` and - /// the wallet-derived `identity_privkey.` rows via - /// `KeychainManager.deleteAllIdentityPrivateKeys(forWalletId:)`. - /// 5. Wipe `WalletStorage`: metadata blob first, mnemonic last. - /// Mnemonic-last makes the mnemonic the retry anchor — if any - /// step earlier in the sequence fails, the pre-flight in - /// `hasArtifacts` will still see the mnemonic and accept the - /// retry. - /// - /// Throws `notFound` only when no artifact for this id is present - /// anywhere (in-memory, SwiftData, or keychain). Throws on - /// partial failure with the error from the failing step; callers - /// can retry — `notFound` is suppressed as long as any artifact - /// remains. + /// Deleting an already-removed wallet succeeds unless an operation fails. public func deleteWallet(walletId: Data) throws { try ensureConfigured() guard walletId.count == 32 else { @@ -388,18 +335,6 @@ public class PlatformWalletManager: ObservableObject { ) } - let storage = WalletStorage() - let hasInMemory = wallets[walletId] != nil - let hasMnemonic = storage.hasMnemonic(for: walletId) - // `metadata(for:)` returns nil for `errSecItemNotFound`. A - // throw here is a real Keychain error (locked / authentication - // / etc.) — propagate rather than guess at presence. - let hasMetadata = try storage.metadata(for: walletId) != nil - - // Tell Rust to drop the wallet from the manager-side - // collection (which also unregisters the wallet's identities - // from IdentitySyncManager). Idempotent — a missing wallet - // returns Ok. try walletId.withUnsafeBytes { raw in guard let base = raw.baseAddress?.assumingMemoryBound(to: FFIByteTuple32.self) else { throw PlatformWalletError.nullPointer( @@ -409,51 +344,18 @@ public class PlatformWalletManager: ObservableObject { try platform_wallet_manager_remove_wallet(handle, base).check() } - // Drop the in-memory entry. The last strong ref to - // `ManagedPlatformWallet` releases, deinit fires, - // `platform_wallet_destroy` drops the handle. wallets.removeValue(forKey: walletId) - // SwiftData wipe — runs on the persistence handler's serial - // queue. Returns the cascaded identity ids (used below to - // drive the keychain wipes) plus whether a `PersistentWallet` - // row was found. - let outcome: PlatformWalletPersistenceHandler.WalletDeletionOutcome - if let persistenceHandler = persistenceHandler { - outcome = try persistenceHandler.deleteWalletData(walletId: walletId) - } else { - outcome = .init(foundWalletRow: false, deletedIdentityIds: []) + let identityIds = try persistenceHandler?.identityIdsForWallet(walletId: walletId) ?? [] + for identityId in identityIds { + try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId) } + try KeychainManager.shared.deleteAllIdentityPrivateKeysForWallet(walletId: walletId) - // Nothing was present anywhere — surface `notFound` rather - // than silently succeeding. The retry-safety property holds - // because the SwiftData + Rust + keychain wipes above are all - // idempotent on missing state. - if !hasInMemory - && !hasMnemonic - && !hasMetadata - && !outcome.foundWalletRow { - throw PlatformWalletError.notFound( - "No wallet found with id \(walletId.map { String(format: "%02x", $0) }.joined())" - ) - } - - // Per-identity Keychain rows. Two parallel storage schemes: - // * `privkey__` — keyed by identity, - // swept directly. - // * `identity_privkey.` — keyed by - // derivation path, owning wallet recorded in the metadata - // blob; the by-walletId sweep walks the keychain once and - // filters on `IdentityPrivateKeyMetadata.walletId`. - for identityId in outcome.deletedIdentityIds { - _ = KeychainManager.shared.deleteAllPrivateKeys(for: identityId) - } - _ = KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId) + try persistenceHandler?.deleteWalletData(walletId: walletId) - // WalletStorage last. Metadata first so the mnemonic stays - // alive until the very end — if any earlier step fails on a - // retry, the mnemonic anchors the pre-flight and the retry - // resumes. Both calls are idempotent on missing rows. + let storage = WalletStorage() + // Delete metadata first so the mnemonic remains available for retry. try storage.deleteMetadata(for: walletId) try storage.deleteMnemonic(for: walletId) } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index eb6467ff968..b1ea76ad755 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -46,24 +46,6 @@ public class PlatformWalletPersistenceHandler { /// atomically. private var inChangeset = false - /// Wallet ids whose `deleteWalletData` has run this process. - /// Mutated only on `serialQueue`. Every wallet-keyed persister - /// callback short-circuits when its `walletId` is tombstoned — - /// `PlatformWalletManager.deleteWallet` drops the wallet from the - /// Rust manager so no _new_ sync work touches it, but Rust tasks - /// that already cloned the wallet's `Arc` before that call keep - /// running on their captured snapshot. Those late callbacks would - /// otherwise see no `PersistentWallet` row, fall through - /// `ensureWalletRecord`'s create-on-missing path, and resurrect - /// the row we just wiped along with whatever child entity the - /// callback was carrying. - /// - /// Cleared by `clearTombstone(walletId:)` when the wallet is - /// intentionally re-created (`createWallet`) or re-loaded - /// (`loadFromPersistor`). Survives only the current process — a - /// stale Rust task can't outlive process restart anyway. - private var tombstonedWalletIds: Set = [] - public init(modelContainer: ModelContainer, network: Network? = nil) { self.modelContainer = modelContainer self.network = network @@ -85,8 +67,8 @@ public class PlatformWalletPersistenceHandler { /// recursive entry. The internal helpers in this file all /// assume they are already on the queue and call /// `backgroundContext` directly. - private func onQueue(_ body: () -> T) -> T { - serialQueue.sync(execute: body) + private func onQueue(_ body: () throws -> T) rethrows -> T { + try serialQueue.sync(execute: body) } // MARK: - Platform Address Balances @@ -108,7 +90,6 @@ public class PlatformWalletPersistenceHandler { entries: [(UInt8, Data, UInt64, UInt32, UInt32, UInt32)] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } for (_, addressHash, balance, nonce, accountIndex, addressIndex) in entries { let descriptor = FetchDescriptor( predicate: #Predicate { $0.addressHash == addressHash } @@ -178,7 +159,6 @@ public class PlatformWalletPersistenceHandler { lastKnownRecentBlock: UInt64 ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } guard let network = walletNetwork(walletId: walletId) else { return } @@ -250,8 +230,7 @@ public class PlatformWalletPersistenceHandler { /// Utxo records so views observing via `@Query` update automatically. func persistWalletChangeset(walletId: Data, changeset: UnsafePointer) { onQueue { - // Stale-callback guard — wallet has been deleted. - guard let wallet = ensureWalletRecord(walletId: walletId) else { return } + let wallet = ensureWalletRecord(walletId: walletId) let cs = changeset.pointee // Chain update. @@ -285,15 +264,7 @@ public class PlatformWalletPersistenceHandler { } /// Find or create the `PersistentWallet` record for this wallet id. - /// - /// Returns `nil` when the wallet has been tombstoned by - /// `deleteWalletData` — a stale persister callback from - /// already-cloned Rust state must not resurrect the row we just - /// wiped. Callers run on `serialQueue` and short-circuit on nil. - private func ensureWalletRecord(walletId: Data) -> PersistentWallet? { - if tombstonedWalletIds.contains(walletId) { - return nil - } + private func ensureWalletRecord(walletId: Data) -> PersistentWallet { let descriptor = FetchDescriptor( predicate: #Predicate { $0.walletId == walletId } ) @@ -305,17 +276,6 @@ public class PlatformWalletPersistenceHandler { return record } - /// Clear a wallet id's tombstone so future persister callbacks - /// for it are accepted again. Called by - /// `PlatformWalletManager.createWallet` / - /// `PlatformWalletManager.loadFromPersistor` after the wallet has - /// been intentionally re-created or re-loaded. - public func clearTombstone(walletId: Data) { - onQueue { - _ = tombstonedWalletIds.remove(walletId) - } - } - /// Look up a `PersistentWallet` to hang on /// `PersistentIdentity.wallet`. Non-creating — returns `nil` if /// no row exists (an identity may arrive before its owning @@ -918,7 +878,6 @@ public class PlatformWalletPersistenceHandler { removed: [Data] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { let identityId = entry.identityId let descriptor = FetchDescriptor( @@ -1221,7 +1180,6 @@ public class PlatformWalletPersistenceHandler { removed: [(identityId: Data, keyId: UInt32)] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { // PersistentPublicKey is keyed on (identity, keyId) via // its parent relationship; fetch by keyId + identityId @@ -1346,7 +1304,6 @@ public class PlatformWalletPersistenceHandler { removals: [TokenBalanceRemovalSnapshot] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } let network = walletNetwork(walletId: walletId) ?? .testnet for entry in upserts { @@ -1483,7 +1440,6 @@ public class PlatformWalletPersistenceHandler { removedIncoming: [ContactRequestRemovalSnapshot] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } for entry in upserts { let ownerId = entry.ownerIdentityId let ownerDescriptor = FetchDescriptor( @@ -1847,7 +1803,6 @@ public class PlatformWalletPersistenceHandler { entries: [CoreAddressEntrySnapshot] ) { onQueue { - guard !tombstonedWalletIds.contains(walletId) else { return } guard let account = fetchAccount(walletId: walletId, key: accountKey) else { return } @@ -2071,7 +2026,7 @@ public class PlatformWalletPersistenceHandler { /// manager's SDK; birth height is SPV's confirmed tip at creation). func persistWalletMetadata(walletId: Data, network: Network, birthHeight: UInt32) { onQueue { - guard let wallet = ensureWalletRecord(walletId: walletId) else { return } + let wallet = ensureWalletRecord(walletId: walletId) wallet.network = network wallet.birthHeight = birthHeight wallet.lastUpdated = Date() @@ -2085,122 +2040,45 @@ public class PlatformWalletPersistenceHandler { /// travel through a Rust-side callback. public func setWalletName(walletId: Data, name: String) { onQueue { - guard let wallet = ensureWalletRecord(walletId: walletId) else { return } + let wallet = ensureWalletRecord(walletId: walletId) wallet.name = name wallet.lastUpdated = Date() try? backgroundContext.save() } } - /// Result of a `deleteWalletData` call. - public struct WalletDeletionOutcome: Sendable { - /// `true` when a `PersistentWallet` row was found and deleted. - /// `false` when the row was already gone (only orphan / keychain - /// state was reachable from the caller). - public let foundWalletRow: Bool - /// 32-byte identity ids of every `PersistentIdentity` that was - /// cascaded as part of this delete. Surfaced so the caller can - /// drive Keychain wipes (`KeychainManager.deleteAllPrivateKeys`, - /// `KeychainManager.deleteAllIdentityPrivateKeys`) that the - /// SwiftData cascade doesn't reach. - public let deletedIdentityIds: [Data] + public func identityIdsForWallet(walletId: Data) throws -> [Data] { + try onQueue { + let descriptor = FetchDescriptor( + predicate: PersistentWallet.predicate(walletId: walletId) + ) + guard let walletRow = try backgroundContext.fetch(descriptor).first else { + return [] + } + return walletRow.identities.map { $0.identityId } + } } - /// Wipe a wallet's entire SwiftData footprint. - /// - /// Deletes: - /// - /// - The `PersistentWallet` row, which `@Relationship(.cascade)` - /// takes through `accounts → coreAddresses → txos` and - /// `accounts → platformAddresses`. - /// - The wallet's `PersistentIdentity` rows (an explicit cascade - /// on this single code path — the schema-level rule stays - /// `.nullify` so other paths don't accidentally take identities - /// down). Each identity carries its own cascade chain - /// (`publicKeys`, `dpnsNames`, `dashpayProfile`, - /// `contactRequests`, `documents`). - /// - `PersistentTokenBalance` rows whose `identityId` matches one - /// of the cascaded identities. `PersistentTokenBalance.identity` - /// is `.nullify`, so deleting the identity alone would leave - /// orphan rows behind. - /// - `PersistentPendingInput` rows where the denormalized - /// `walletId` matches. The schema-level cascade is via - /// `PersistentTransaction.pendingInputs`, but transactions - /// aren't wallet-owned, so the pending rows would otherwise - /// linger. - /// - `PersistentTransaction` rows whose `outputs`, `inputs`, and - /// `pendingInputs` are all empty after the above. Transactions - /// are intentionally not wallet-owned (one on-chain tx can fund - /// multiple wallets); the orphan sweep catches the rows that - /// nobody references anymore. - /// - `PersistentPlatformAddressesSyncState` for the wallet's - /// network **only when this was the last wallet on that - /// network**. The BLAST checkpoint is shared across wallets on a - /// network; deleting it while siblings still exist would force - /// them to re-scan, and leaving it after the last sibling is - /// removed would make a fresh import on the same network skip - /// pre-checkpoint balances. - /// - /// Tombstones the wallet id on `serialQueue` before the first - /// delete so any persister callback that arrives later (from Rust - /// work that already cloned the wallet's `Arc`) short-circuits - /// instead of resurrecting the row through `ensureWalletRecord`'s - /// create-on-missing path. The tombstone is cleared by - /// `clearTombstone(walletId:)` from the manager's create / load - /// paths. - /// - /// Atomic: the whole sweep runs inside a single `save()`. On - /// failure, `rollback()` reverts the pending deletes; the - /// tombstone is still cleared so a retry can attempt the wipe - /// again. The thrown error propagates so the caller can leave its - /// other state (Rust manager, in-memory dict, keychain) alone. - @discardableResult - public func deleteWalletData(walletId: Data) throws -> WalletDeletionOutcome { - var thrown: Error? - var foundWalletRow = false - var deletedIdentityIds: [Data] = [] - onQueue { - // Block any concurrent / stale persister callback from - // resurrecting the row we're about to wipe. Set before any - // fetch so even a fetch that races us sees the tombstone - // on its own onQueue entry. - tombstonedWalletIds.insert(walletId) + /// Wipe a wallet's SwiftData footprint. + public func deleteWalletData(walletId: Data) throws { + try onQueue { do { let walletDescriptor = FetchDescriptor( predicate: PersistentWallet.predicate(walletId: walletId) ) let walletRow = try backgroundContext.fetch(walletDescriptor).first - - // Capture the wallet's network before delete so the - // "last on this network" sync-state cleanup below has - // something to scope against. May be nil for rows that - // were created by a changeset before metadata filled - // network in — in that case we skip the sync-state - // cleanup (caller can wipe via a follow-up restart). let walletNetwork = walletRow?.network if let walletRow = walletRow { - foundWalletRow = true - // Identities first — the schema rule is `.nullify`, - // so deleting the wallet would otherwise leave a - // set of orphan identity rows with `wallet == nil`. - // Snapshot before delete because the relationship - // array becomes unsafe to iterate once SwiftData - // starts processing the wallet's own deletion. + // Wallet identity relationships are `.nullify`; this delete path cascades them explicitly. let identitiesToDelete = Array(walletRow.identities) - deletedIdentityIds = identitiesToDelete.map { $0.identityId } - - // PersistentTokenBalance.identity is `.nullify`, - // so without an explicit sweep the balances would - // orphan with `identity = nil`. Predicate-driven - // fetch by `identityId` denorm — no relationship - // walk needed. - for identityId in deletedIdentityIds { + let identityIds = identitiesToDelete.map { $0.identityId } + + for identityId in identityIds { let balanceDescriptor = FetchDescriptor( predicate: PersistentTokenBalance.predicate(identityId: identityId) ) - let balanceRows = (try? backgroundContext.fetch(balanceDescriptor)) ?? [] - for row in balanceRows { + for row in try backgroundContext.fetch(balanceDescriptor) { backgroundContext.delete(row) } } @@ -2211,47 +2089,33 @@ public class PlatformWalletPersistenceHandler { backgroundContext.delete(walletRow) } - // Pending inputs: walletId is denormalized + indexed, - // direct predicate fetch. let pendingDescriptor = FetchDescriptor( predicate: #Predicate { $0.walletId == walletId } ) - let pendingRows = try backgroundContext.fetch(pendingDescriptor) - for row in pendingRows { + for row in try backgroundContext.fetch(pendingDescriptor) { backgroundContext.delete(row) } - // Orphan transactions: scan all and drop the ones with - // no surviving relationships. The TXO cascade above - // already emptied this wallet's contributions; what's - // left here is rows that nobody references. - let txDescriptor = FetchDescriptor() - let txRows = try backgroundContext.fetch(txDescriptor) - for tx in txRows where tx.outputs.isEmpty - && tx.inputs.isEmpty - && tx.pendingInputs.isEmpty { + let txRows = try backgroundContext.fetch(FetchDescriptor()) + for tx in txRows where tx.outputs.isEmpty && + tx.inputs.isEmpty && + tx.pendingInputs.isEmpty { backgroundContext.delete(tx) } - // Network sync-state: only drop the row when no other - // wallet on the same network remains. Other wallets - // share that BLAST checkpoint and would lose their - // progress if we cleared it; a fresh import after the - // last wallet on a network is removed would otherwise - // inherit a stale start height. if let walletNetwork = walletNetwork { let networkRaw = walletNetwork.rawValue let siblingDescriptor = FetchDescriptor( predicate: #Predicate { $0.networkRaw == networkRaw } ) - let siblings = (try? backgroundContext.fetch(siblingDescriptor)) ?? [] - let remaining = siblings.filter { $0.walletId != walletId } + 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 { + if let syncRow = try backgroundContext.fetch(syncDescriptor).first { backgroundContext.delete(syncRow) } } @@ -2260,16 +2124,9 @@ public class PlatformWalletPersistenceHandler { try backgroundContext.save() } catch { backgroundContext.rollback() - thrown = error + throw error } } - if let thrown = thrown { - throw thrown - } - return WalletDeletionOutcome( - foundWalletRow: foundWalletRow, - deletedIdentityIds: deletedIdentityIds - ) } // MARK: - Watch-only Restore: Account xpub @@ -2280,78 +2137,78 @@ public class PlatformWalletPersistenceHandler { /// that uniquely identifies an account across variants. func persistAccount(walletId: Data, spec: AccountSpecFFI) { onQueue { - guard let wallet = ensureWalletRecord(walletId: walletId) else { return } - let typeTag = UInt32(spec.type_tag) - let index = spec.index - let registrationIndex = spec.registration_index - let keyClass = spec.key_class - var userIdentityId = Data(count: 32) - withUnsafeBytes(of: spec.user_identity_id) { src in - userIdentityId.withUnsafeMutableBytes { dst in - dst.copyMemory(from: src) + let wallet = ensureWalletRecord(walletId: walletId) + let typeTag = UInt32(spec.type_tag) + let index = spec.index + let registrationIndex = spec.registration_index + let keyClass = spec.key_class + var userIdentityId = Data(count: 32) + withUnsafeBytes(of: spec.user_identity_id) { src in + userIdentityId.withUnsafeMutableBytes { dst in + dst.copyMemory(from: src) + } } - } - var friendIdentityId = Data(count: 32) - withUnsafeBytes(of: spec.friend_identity_id) { src in - friendIdentityId.withUnsafeMutableBytes { dst in - dst.copyMemory(from: src) + var friendIdentityId = Data(count: 32) + withUnsafeBytes(of: spec.friend_identity_id) { src in + friendIdentityId.withUnsafeMutableBytes { dst in + dst.copyMemory(from: src) + } } - } - let xpubBytes: Data - if let xpubPtr = spec.account_xpub_bytes, spec.account_xpub_bytes_len > 0 { - xpubBytes = Data(bytes: xpubPtr, count: Int(spec.account_xpub_bytes_len)) - } else { - xpubBytes = Data() - } - - // Upsert keyed by the full account identity. We can't easily - // express the identity tuple in a #Predicate with local `Data` - // captures, so fetch by (walletId, accountType, accountIndex) - // and verify the richer fields in Swift. - let descriptor = FetchDescriptor( - predicate: #Predicate { - $0.wallet.walletId == walletId - && $0.accountType == typeTag - && $0.accountIndex == index + let xpubBytes: Data + if let xpubPtr = spec.account_xpub_bytes, spec.account_xpub_bytes_len > 0 { + xpubBytes = Data(bytes: xpubPtr, count: Int(spec.account_xpub_bytes_len)) + } else { + xpubBytes = Data() } - ) - let existing = (try? backgroundContext.fetch(descriptor)) ?? [] - let match = existing.first { acc in - // `standardTag` splits Standard accounts into BIP44 (0) - // and BIP32 (1) variants. Without it, the second emit - // (whichever the Rust side serializes last) silently - // aliases onto the first row and the BIP32 account is - // never persisted as its own record. - acc.standardTag == spec.standard_tag - && acc.registrationIndex == registrationIndex - && acc.keyClass == keyClass - && acc.userIdentityId == userIdentityId - && acc.friendIdentityId == friendIdentityId - } - let account: PersistentAccount - if let match = match { - account = match - } else { - account = PersistentAccount( - wallet: wallet, - accountType: typeTag, - accountIndex: index, - accountTypeName: accountTypeName( - for: spec.type_tag, - standardTag: spec.standard_tag - ) + + // Upsert keyed by the full account identity. We can't easily + // express the identity tuple in a #Predicate with local `Data` + // captures, so fetch by (walletId, accountType, accountIndex) + // and verify the richer fields in Swift. + let descriptor = FetchDescriptor( + predicate: #Predicate { + $0.wallet.walletId == walletId + && $0.accountType == typeTag + && $0.accountIndex == index + } ) - backgroundContext.insert(account) + let existing = (try? backgroundContext.fetch(descriptor)) ?? [] + let match = existing.first { acc in + // `standardTag` splits Standard accounts into BIP44 (0) + // and BIP32 (1) variants. Without it, the second emit + // (whichever the Rust side serializes last) silently + // aliases onto the first row and the BIP32 account is + // never persisted as its own record. + acc.standardTag == spec.standard_tag + && acc.registrationIndex == registrationIndex + && acc.keyClass == keyClass + && acc.userIdentityId == userIdentityId + && acc.friendIdentityId == friendIdentityId + } + let account: PersistentAccount + if let match = match { + account = match + } else { + account = PersistentAccount( + wallet: wallet, + accountType: typeTag, + accountIndex: index, + accountTypeName: accountTypeName( + for: spec.type_tag, + standardTag: spec.standard_tag + ) + ) + backgroundContext.insert(account) + } + account.standardTag = spec.standard_tag + account.registrationIndex = registrationIndex + account.keyClass = keyClass + account.userIdentityId = userIdentityId + account.friendIdentityId = friendIdentityId + account.accountExtendedPubKeyBytes = xpubBytes + account.lastUpdated = Date() + if !self.inChangeset { try? backgroundContext.save() } } - account.standardTag = spec.standard_tag - account.registrationIndex = registrationIndex - account.keyClass = keyClass - account.userIdentityId = userIdentityId - account.friendIdentityId = friendIdentityId - account.accountExtendedPubKeyBytes = xpubBytes - account.lastUpdated = Date() - if !self.inChangeset { try? backgroundContext.save() } - } // onQueue } // MARK: - Watch-only Restore: Load diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift index e370c604f2d..5bf547c313f 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift @@ -233,44 +233,56 @@ public final class KeychainManager: Sendable { /// - Returns: true if deletion completed (even if no keys existed) @discardableResult public func deleteAllPrivateKeys(for identityId: Data) -> Bool { + do { + try deleteAllPrivateKeysThrowing(for: identityId) + return true + } catch { + return false + } + } + + public nonisolated func deleteAllPrivateKeysThrowing(for identityId: Data) throws { + let identityHex = identityId.map { String(format: "%02x", $0) }.joined() + try deleteItems(accountPrefixes: ["privkey_\(identityHex)_"]) + } + + public nonisolated func deleteAllKeychainItems(forIdentityId identityId: Data) throws { + let identityHex = identityId.map { String(format: "%02x", $0) }.joined() + try deleteItems(accountPrefixes: [ + "privkey_\(identityHex)_", + "specialkey_\(identityHex)_" + ]) + } + + private nonisolated func deleteItems(accountPrefixes: [String]) throws { var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: serviceName, - kSecMatchLimit as String: kSecMatchLimitAll + kSecMatchLimit as String: kSecMatchLimitAll, + kSecReturnAttributes as String: true ] if let accessGroup = accessGroup { query[kSecAttrAccessGroup as String] = accessGroup } - // First, find all keys for this identity var result: AnyObject? let searchStatus = SecItemCopyMatching(query as CFDictionary, &result) + if searchStatus == errSecItemNotFound { + return + } + guard searchStatus == errSecSuccess, let items = result as? [[String: Any]] else { + throw KeychainError.retrieveFailed(searchStatus) + } - let identityHex = identityId.map { String(format: "%02x", $0) }.joined() - - if searchStatus == errSecSuccess, - let items = result as? [[String: Any]] { - // Filter items for this identity and delete them - for item in items { - if let account = item[kSecAttrAccount as String] as? String, - account.hasPrefix("privkey_\(identityHex)_") { - var deleteQuery: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: serviceName, - kSecAttrAccount as String: account - ] - - if let accessGroup = accessGroup { - deleteQuery[kSecAttrAccessGroup as String] = accessGroup - } - - SecItemDelete(deleteQuery as CFDictionary) - } + for item in items { + guard let account = item[kSecAttrAccount as String] as? String, + accountPrefixes.contains(where: { account.hasPrefix($0) }) + else { + continue } + try deleteGenericPassword(account: account) } - - return true } // MARK: - Special Keys (Voting, Owner, Payout) @@ -432,6 +444,23 @@ public final class KeychainManager: Sendable { // MARK: - Private Helpers + private nonisolated func deleteGenericPassword(account: String) throws { + var query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: serviceName, + kSecAttrAccount as String: account + ] + + if let accessGroup = accessGroup { + query[kSecAttrAccessGroup as String] = accessGroup + } + + let status = SecItemDelete(query as CFDictionary) + guard status == errSecSuccess || status == errSecItemNotFound else { + throw KeychainError.deleteFailed(status) + } + } + /// Nonisolated because the result only depends on the arguments /// — no access to actor-isolated state — and the function is /// shared between the `@MainActor` wrapper methods and the @@ -748,21 +777,18 @@ extension KeychainManager { return status == errSecSuccess || status == errSecItemNotFound } - /// Delete every `identity_privkey.` keychain row - /// whose `IdentityPrivateKeyMetadata.walletId` matches the given - /// wallet id (32-byte `Data`). Used by - /// `PlatformWalletManager.deleteWallet` to wipe the wallet-derived - /// per-identity signing keys the Rust persister callback path - /// writes — those rows are keyed by derivation path, not identity - /// id, so the per-identity sweep (`deleteAllPrivateKeys(for:)`) - /// alone leaves them behind. - /// - /// Idempotent. Returns `true` after the sweep completes regardless - /// of how many rows matched. Decode errors on individual rows are - /// skipped (the row stays) so a single corrupted metadata blob - /// can't break the whole wipe. + /// Delete wallet-derived identity private keys. @discardableResult public nonisolated func deleteAllIdentityPrivateKeys(forWalletId walletId: Data) -> Bool { + do { + try deleteAllIdentityPrivateKeysForWallet(walletId: walletId) + return true + } catch { + return false + } + } + + public nonisolated func deleteAllIdentityPrivateKeysForWallet(walletId: Data) throws { let walletIdHex = walletId.map { String(format: "%02x", $0) }.joined() var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, @@ -776,10 +802,11 @@ extension KeychainManager { var result: AnyObject? let status = SecItemCopyMatching(query as CFDictionary, &result) + if status == errSecItemNotFound { + return + } guard status == errSecSuccess, let items = result as? [[String: Any]] else { - // `errSecItemNotFound` is the empty-keychain case — still a - // successful sweep. - return true + throw KeychainError.retrieveFailed(status) } let decoder = JSONDecoder() @@ -801,17 +828,8 @@ extension KeychainManager { continue } - var deleteQuery: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: serviceName, - kSecAttrAccount as String: account, - ] - if let accessGroup = accessGroup { - deleteQuery[kSecAttrAccessGroup as String] = accessGroup - } - SecItemDelete(deleteQuery as CFDictionary) + try deleteGenericPassword(account: account) } - return true } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/KeyManagerTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/KeyManagerTests.swift index fff8bd68551..144f300b914 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/KeyManagerTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/KeyManagerTests.swift @@ -175,7 +175,7 @@ final class KeyManagerTests: XCTestCase { ]) // Encode to WIF - guard let wif = KeyFormatter.toWIF(originalKey, isTestnet: true) else { + guard let wif = KeyFormatter.toWIF(originalKey, network: .testnet) else { XCTFail("Failed to encode to WIF") return } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift new file mode 100644 index 00000000000..64c2e96041f --- /dev/null +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift @@ -0,0 +1,208 @@ +import SwiftData +import XCTest +@testable import SwiftDashSDK + +final class WalletDeletionTests: XCTestCase { + + func testIdentityIdsForWallet() throws { + let container = try DashModelContainer.createInMemory() + let context = ModelContext(container) + let walletId = Data(repeating: 0x11, count: 32) + let identityA = Data(repeating: 0x22, count: 32) + let identityB = Data(repeating: 0x33, count: 32) + + let wallet = PersistentWallet(walletId: walletId, network: .testnet) + let first = PersistentIdentity(identityId: identityA, network: .testnet) + let second = PersistentIdentity(identityId: identityB, network: .testnet) + first.wallet = wallet + second.wallet = wallet + wallet.identities.append(contentsOf: [first, second]) + + context.insert(wallet) + context.insert(first) + context.insert(second) + try context.save() + + let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) + + XCTAssertEqual( + Set(try handler.identityIdsForWallet(walletId: walletId)), + Set([identityA, identityB]) + ) + XCTAssertEqual( + try handler.identityIdsForWallet(walletId: Data(repeating: 0xff, count: 32)), + [] + ) + } + + func testDeleteWalletDataRemovesWalletFootprintAndLastNetworkSyncState() throws { + let container = try DashModelContainer.createInMemory() + let context = ModelContext(container) + let walletId = Data(repeating: 0x44, count: 32) + let identityId = Data(repeating: 0x55, count: 32) + + let wallet = PersistentWallet(walletId: walletId, network: .testnet) + let identity = PersistentIdentity(identityId: identityId, network: .testnet) + identity.wallet = wallet + wallet.identities.append(identity) + + let balance = PersistentTokenBalance( + tokenId: "token-a", + identityId: identityId, + balance: 10, + network: .testnet + ) + balance.identity = identity + identity.tokenBalances.append(balance) + + let pendingTx = PersistentTransaction( + txid: Data(repeating: 0x66, count: 32), + transactionData: Data([0x01]) + ) + let pending = PersistentPendingInput( + outpoint: Data(repeating: 0x67, count: 36), + inputIndex: 0, + spendingTxid: pendingTx.txid, + spendingTransaction: pendingTx, + walletId: walletId + ) + pendingTx.pendingInputs.append(pending) + + let orphanTx = PersistentTransaction( + txid: Data(repeating: 0x77, count: 32), + transactionData: Data([0x02]) + ) + + let liveTx = PersistentTransaction( + txid: Data(repeating: 0x88, count: 32), + transactionData: Data([0x03]) + ) + let liveTxo = PersistentTxo( + transaction: liveTx, + vout: 0, + amount: 1, + address: "yTest" + ) + liveTx.outputs.append(liveTxo) + + let syncState = PersistentPlatformAddressesSyncState( + walletId: Self.syncStateScopeId(for: .testnet), + network: .testnet, + syncHeight: 10, + syncTimestamp: 20, + lastKnownRecentBlock: 30 + ) + + context.insert(wallet) + context.insert(identity) + context.insert(balance) + context.insert(pendingTx) + context.insert(pending) + context.insert(orphanTx) + context.insert(liveTx) + context.insert(liveTxo) + context.insert(syncState) + try context.save() + + let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) + try handler.deleteWalletData(walletId: walletId) + try handler.deleteWalletData(walletId: walletId) + + XCTAssertTrue(try fetch(PersistentWallet.self, in: container).isEmpty) + XCTAssertTrue(try fetch(PersistentIdentity.self, in: container).isEmpty) + XCTAssertTrue(try fetch(PersistentTokenBalance.self, in: container).isEmpty) + XCTAssertTrue(try fetch(PersistentPendingInput.self, in: container).isEmpty) + XCTAssertTrue(try fetch(PersistentPlatformAddressesSyncState.self, in: container).isEmpty) + + let transactions = try fetch(PersistentTransaction.self, in: container) + XCTAssertEqual(transactions.count, 1) + XCTAssertEqual(transactions.first?.txid, liveTx.txid) + } + + func testDeleteWalletDataKeepsNetworkSyncStateWhenSiblingWalletRemains() throws { + let container = try DashModelContainer.createInMemory() + let context = ModelContext(container) + let walletId = Data(repeating: 0x99, count: 32) + let siblingId = Data(repeating: 0xaa, count: 32) + + context.insert(PersistentWallet(walletId: walletId, network: .testnet)) + context.insert(PersistentWallet(walletId: siblingId, network: .testnet)) + context.insert( + PersistentPlatformAddressesSyncState( + walletId: Self.syncStateScopeId(for: .testnet), + network: .testnet, + syncHeight: 10, + syncTimestamp: 20, + lastKnownRecentBlock: 30 + ) + ) + try context.save() + + let handler = PlatformWalletPersistenceHandler(modelContainer: container, network: .testnet) + try handler.deleteWalletData(walletId: walletId) + + let wallets = try fetch(PersistentWallet.self, in: container) + XCTAssertEqual(wallets.map(\.walletId), [siblingId]) + XCTAssertEqual(try fetch(PersistentPlatformAddressesSyncState.self, in: container).count, 1) + } + + @MainActor + func testThrowingKeychainSweepsUseIsolatedService() throws { + let manager = KeychainManager(serviceName: "org.dash.wallet-delete-tests.\(UUID().uuidString)") + let identityId = Data(repeating: 0xbb, count: 32) + let walletId = Data(repeating: 0xcc, count: 32) + let publicKey = Self.hex(Data(repeating: 0xdd, count: 33)) + + XCTAssertNotNil(manager.storePrivateKey(Data(repeating: 0x01, count: 32), identityId: identityId, keyIndex: 0)) + XCTAssertNotNil(manager.storeSpecialKey(Data(repeating: 0x02, count: 32), identityId: identityId, keyType: .voting)) + XCTAssertNotNil( + manager.storeIdentityPrivateKey( + Data(repeating: 0x03, count: 32), + derivationPath: "m/9'/5'/3'/1'", + metadata: .init( + identityId: "identity", + keyId: 1, + walletId: Self.hex(walletId), + identityIndex: 0, + keyIndex: 0, + derivationPath: "m/9'/5'/3'/1'", + publicKey: publicKey, + publicKeyHash: Self.hex(Data(repeating: 0xee, count: 20)), + keyType: 0, + purpose: 0, + securityLevel: 0 + ) + ) + ) + + XCTAssertNotNil(manager.retrievePrivateKey(identityId: identityId, keyIndex: 0)) + XCTAssertNotNil(manager.retrieveSpecialKey(identityId: identityId, keyType: .voting)) + XCTAssertNotNil(manager.retrieveIdentityPrivateKey(publicKeyHex: publicKey)) + + try manager.deleteAllKeychainItems(forIdentityId: identityId) + + XCTAssertNil(manager.retrievePrivateKey(identityId: identityId, keyIndex: 0)) + XCTAssertNil(manager.retrieveSpecialKey(identityId: identityId, keyType: .voting)) + XCTAssertNotNil(manager.retrieveIdentityPrivateKey(publicKeyHex: publicKey)) + + try manager.deleteAllIdentityPrivateKeysForWallet(walletId: walletId) + + XCTAssertNil(manager.retrieveIdentityPrivateKey(publicKeyHex: publicKey)) + } + + private static func syncStateScopeId(for network: Network) -> Data { + var data = Data("platform-sync:\(network.networkName)".utf8.prefix(32)) + if data.count < 32 { + data.append(Data(repeating: 0, count: 32 - data.count)) + } + return data + } + + private func fetch(_ type: T.Type, in container: ModelContainer) throws -> [T] { + try ModelContext(container).fetch(FetchDescriptor()) + } + + private static func hex(_ data: Data) -> String { + data.map { String(format: "%02x", $0) }.joined() + } +} From 99c528781f5a155feab5a4e744cdd7fbd5ce30d0 Mon Sep 17 00:00:00 2001 From: Bartosz Rozwarski Date: Fri, 15 May 2026 15:19:22 +0200 Subject: [PATCH 3/3] refactor(swift-sdk): drop KeychainManager Bool wrappers, dedupe hex Removes the `do { try X; return true } catch { return false }` bridge pattern around `deleteAllPrivateKeys(for:)` and `deleteAllIdentityPrivateKeys(forWalletId:)`. The throwing variants take over the canonical names; callers `try` directly. Drops the parallel `*Throwing` / `*ForWallet` names. Replaces seven inline `map { String(format: "%02x", $0) }.joined()` hex-format duplications with the existing `Data.toHexString()` extension, and removes the same-shape `hex(_:)` helper from `WalletDeletionTests` in favor of the same extension. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PlatformWalletManager.swift | 2 +- .../Security/KeychainManager.swift | 50 ++++++------------- .../WalletDeletionTests.swift | 12 ++--- 3 files changed, 19 insertions(+), 45 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 8cf31bb03ad..45407511912 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -350,7 +350,7 @@ public class PlatformWalletManager: ObservableObject { for identityId in identityIds { try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId) } - try KeychainManager.shared.deleteAllIdentityPrivateKeysForWallet(walletId: walletId) + try KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId) try persistenceHandler?.deleteWalletData(walletId: walletId) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift index 5bf547c313f..bff301c86ff 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Security/KeychainManager.swift @@ -147,7 +147,7 @@ public final class KeychainManager: Sendable { // Add metadata let metadata: [String: Any] = [ - "identityId": identityId.map { String(format: "%02x", $0) }.joined(), + "identityId": identityId.toHexString(), "keyIndex": keyIndex, "createdAt": Date().timeIntervalSince1970 ] @@ -228,26 +228,15 @@ public final class KeychainManager: Sendable { return status == errSecSuccess || status == errSecItemNotFound } - /// Delete all private keys for an identity - /// - Parameter identityId: The identity ID (32 bytes) - /// - Returns: true if deletion completed (even if no keys existed) - @discardableResult - public func deleteAllPrivateKeys(for identityId: Data) -> Bool { - do { - try deleteAllPrivateKeysThrowing(for: identityId) - return true - } catch { - return false - } - } - - public nonisolated func deleteAllPrivateKeysThrowing(for identityId: Data) throws { - let identityHex = identityId.map { String(format: "%02x", $0) }.joined() - try deleteItems(accountPrefixes: ["privkey_\(identityHex)_"]) + /// Delete every `privkey__*` keychain row for `identityId`. + public nonisolated func deleteAllPrivateKeys(for identityId: Data) throws { + try deleteItems(accountPrefixes: ["privkey_\(identityId.toHexString())_"]) } + /// Delete every per-identity keychain row — both `privkey_*` and + /// `specialkey_*` schemes — for `identityId`. public nonisolated func deleteAllKeychainItems(forIdentityId identityId: Data) throws { - let identityHex = identityId.map { String(format: "%02x", $0) }.joined() + let identityHex = identityId.toHexString() try deleteItems(accountPrefixes: [ "privkey_\(identityHex)_", "specialkey_\(identityHex)_" @@ -466,13 +455,11 @@ public final class KeychainManager: Sendable { /// shared between the `@MainActor` wrapper methods and the /// off-actor `storePrivateKeyNonisolated` path. private nonisolated func generateKeyIdentifier(identityId: Data, keyIndex: Int32) -> String { - let identityHex = identityId.map { String(format: "%02x", $0) }.joined() - return "privkey_\(identityHex)_\(keyIndex)" + return "privkey_\(identityId.toHexString())_\(keyIndex)" } private func generateSpecialKeyIdentifier(identityId: Data, keyType: SpecialKeyType) -> String { - let identityHex = identityId.map { String(format: "%02x", $0) }.joined() - return "specialkey_\(identityHex)_\(keyType.rawValue)" + return "specialkey_\(identityId.toHexString())_\(keyType.rawValue)" } } @@ -504,7 +491,7 @@ extension KeychainManager { } } guard rc == 0 else { return "" } - return out.map { String(format: "%02x", $0) }.joined() + return Data(out).toHexString() } } @@ -777,19 +764,10 @@ extension KeychainManager { return status == errSecSuccess || status == errSecItemNotFound } - /// Delete wallet-derived identity private keys. - @discardableResult - public nonisolated func deleteAllIdentityPrivateKeys(forWalletId walletId: Data) -> Bool { - do { - try deleteAllIdentityPrivateKeysForWallet(walletId: walletId) - return true - } catch { - return false - } - } - - public nonisolated func deleteAllIdentityPrivateKeysForWallet(walletId: Data) throws { - let walletIdHex = walletId.map { String(format: "%02x", $0) }.joined() + /// Delete every `identity_privkey.` keychain row whose + /// `IdentityPrivateKeyMetadata.walletId` matches `walletId`. + public nonisolated func deleteAllIdentityPrivateKeys(forWalletId walletId: Data) throws { + let walletIdHex = walletId.toHexString() var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: serviceName, diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift index 64c2e96041f..c703419d31f 100644 --- a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/WalletDeletionTests.swift @@ -151,7 +151,7 @@ final class WalletDeletionTests: XCTestCase { let manager = KeychainManager(serviceName: "org.dash.wallet-delete-tests.\(UUID().uuidString)") let identityId = Data(repeating: 0xbb, count: 32) let walletId = Data(repeating: 0xcc, count: 32) - let publicKey = Self.hex(Data(repeating: 0xdd, count: 33)) + let publicKey = Data(repeating: 0xdd, count: 33).toHexString() XCTAssertNotNil(manager.storePrivateKey(Data(repeating: 0x01, count: 32), identityId: identityId, keyIndex: 0)) XCTAssertNotNil(manager.storeSpecialKey(Data(repeating: 0x02, count: 32), identityId: identityId, keyType: .voting)) @@ -162,12 +162,12 @@ final class WalletDeletionTests: XCTestCase { metadata: .init( identityId: "identity", keyId: 1, - walletId: Self.hex(walletId), + walletId: walletId.toHexString(), identityIndex: 0, keyIndex: 0, derivationPath: "m/9'/5'/3'/1'", publicKey: publicKey, - publicKeyHash: Self.hex(Data(repeating: 0xee, count: 20)), + publicKeyHash: Data(repeating: 0xee, count: 20).toHexString(), keyType: 0, purpose: 0, securityLevel: 0 @@ -185,7 +185,7 @@ final class WalletDeletionTests: XCTestCase { XCTAssertNil(manager.retrieveSpecialKey(identityId: identityId, keyType: .voting)) XCTAssertNotNil(manager.retrieveIdentityPrivateKey(publicKeyHex: publicKey)) - try manager.deleteAllIdentityPrivateKeysForWallet(walletId: walletId) + try manager.deleteAllIdentityPrivateKeys(forWalletId: walletId) XCTAssertNil(manager.retrieveIdentityPrivateKey(publicKeyHex: publicKey)) } @@ -201,8 +201,4 @@ final class WalletDeletionTests: XCTestCase { private func fetch(_ type: T.Type, in container: ModelContainer) throws -> [T] { try ModelContext(container).fetch(FetchDescriptor()) } - - private static func hex(_ data: Data) -> String { - data.map { String(format: "%02x", $0) }.joined() - } }