Skip to content

feat(swift-sdk): make wallet deletion asynchronous #4170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
llbartekll wants to merge 1 commit into
base: v4.1-dev
Choose a base branch
from
Draft

feat(swift-sdk): make wallet deletion asynchronous #4170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 77 additions & 49 deletions packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,12 @@ public class PlatformWalletManager: ObservableObject {
private var modelContainer: ModelContainer?
private var signerNetwork: Network?

/// Async deletion yields while the persistence handler sweeps SwiftData.
/// Reject create/load/another delete during that window so actor
/// reentrancy cannot recreate a deterministic wallet id that the in-flight
/// deletion is about to remove from persistence.
private var walletDeletionInProgress = false
Comment on lines +210 to +214

@thepastaclaw thepastaclaw Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: Add coverage for the manager-level deletion gate

The new walletDeletionInProgress flag and its checks in both createWallet overloads, loadFromPersistor, and deleteWallet are the only protection against manager actor reentrancy while deletion is suspended. The added tests call PlatformWalletPersistenceHandler.deleteWalletData directly, and the idempotence test performs two completed deletions sequentially, so removing beginWalletDeletion or any create/load guard would not fail the suite. Add a manager-level test that blocks the first deletion on the persistence queue, verifies that an overlapping delete and create/load attempts are rejected, then releases the first deletion and verifies the gate is cleared.

source: ['codex']


/// Retained for the lifetime of the FFI handle so the event-handler
/// context pointer remains valid.
private var eventHandler: PlatformWalletEventHandler?
Expand Down Expand Up @@ -337,6 +343,7 @@ public class PlatformWalletManager: ObservableObject {
birthHeight: UInt32? = nil
) throws -> ManagedPlatformWallet {
try ensureConfigured()
try ensureNoWalletDeletionInProgress()
var walletHandle: Handle = NULL_HANDLE
var walletId: FFIByteTuple32 =
(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0)
Expand Down Expand Up @@ -379,6 +386,7 @@ public class PlatformWalletManager: ObservableObject {
birthHeight: UInt32? = nil
) throws -> ManagedPlatformWallet {
try ensureConfigured()
try ensureNoWalletDeletionInProgress()
guard seed.count == 64 else {
throw PlatformWalletError.invalidParameter(
"seed must be 64 bytes, got \(seed.count)"
Expand Down Expand Up @@ -441,6 +449,7 @@ public class PlatformWalletManager: ObservableObject {
@discardableResult
public func loadFromPersistor() throws -> [ManagedPlatformWallet] {
try ensureConfigured()
try ensureNoWalletDeletionInProgress()

try platform_wallet_manager_load_from_persistor(handle).check()

Expand Down Expand Up @@ -970,18 +979,25 @@ public class PlatformWalletManager: ObservableObject {

// MARK: - Wallet deletion

/// Fully wipe a wallet's Rust, SwiftData, and Keychain footprint.
///
/// Requires the manager to have been `configure`d with a
/// `ModelContainer` — the per-identity Keychain sweep needs the
/// wallet's identity ids, which only the persistence handler can
/// resolve. The no-persistence configuration mode is rejected
/// here rather than silently leaving identity key material behind.
/// Fully wipe a wallet's Rust, SwiftData, and Keychain footprint without
/// blocking the main actor while SwiftData and Keychain perform their
/// potentially expensive database sweeps.
///
/// Deleting an already-removed wallet succeeds unless an
/// operation fails.
public func deleteWallet(walletId: Data) throws {
/// `PlatformWalletManager` is main-actor isolated because its published
/// state and Rust handle are UI-facing. The persistence handler, however,
/// owns a dedicated serial queue with awaitable app-facing APIs, so its
/// database sweep does not freeze the UI. Rust/in-memory mutations remain
/// on the main actor. Deleting an already-removed wallet succeeds unless
/// an operation fails.
public func deleteWallet(walletId: Data) async throws {
let startedAt = ContinuousClock.now
let walletLabel = walletId.prefix(4)
.map { String(format: "%02x", $0) }
.joined()

try ensureConfigured()
try beginWalletDeletion()
defer { walletDeletionInProgress = false }
guard walletId.count == 32 else {
throw PlatformWalletError.invalidParameter(
"walletId must be 32 bytes, got \(walletId.count)"
Expand All @@ -993,24 +1009,22 @@ public class PlatformWalletManager: ObservableObject {
)
}

let identityIds = try persistenceHandler.identityIdsForWallet(walletId: walletId)

// Wipe Keychain BEFORE the SwiftData identity deletion runs.
// Order matters for retry-safety: if `deleteWalletData`
// commits identity rows and then throws partway, a retry
// would see `identityIdsForWallet == []` and the
// `deleteAllKeychainItems(forIdentityId:)` sweep below
// could no longer find the keys to purge. Doing the
// keychain side first leaves at worst stale SwiftData
// rows on a retry — repeating the wipe is harmless, and
// every keychain call here is idempotent (no-op on "not
// found"). Mnemonic / metadata stay in `WalletStorage`
// for now so a retry can still derive any missed key.
for identityId in identityIds {
try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId)
}
try KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId)
var phaseStartedAt = ContinuousClock.now
let identityIds = try await persistenceHandler.identityIdsForWallet(
walletId: walletId)

try await Task.detached(priority: .userInitiated) {
for identityId in identityIds {
try KeychainManager.shared.deleteAllKeychainItems(forIdentityId: identityId)
}
try KeychainManager.shared.deleteAllIdentityPrivateKeys(forWalletId: walletId)
}.value
Comment on lines +1013 to +1021

@thepastaclaw thepastaclaw Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Blocking: Quiesce child FFI handles before awaiting deletion

Deletion now yields before removing the wallet from wallets, allowing another main-actor turn to obtain its ManagedPlatformWallet; previously retained wrappers are also unaffected by the manager gate. Each wrapper owns an independent handle whose Rust Arc<PlatformWallet> remains in PLATFORM_WALLET_STORAGE, and platform_wallet_manager_remove_wallet removes only the manager's references. Because ManagedPlatformWallet operations run through these handles and several async operations persist changes through the same callback-backed persister, an operation started from a stale handle can reach the synchronous persistence callbacks after deleteWalletData has completed and recreate wallet rows after deletion reports success. Add a shared FFI deletion/quiescence state that rejects new child-handle operations and waits for already-running operations before the final persistence sweep; removing the published Swift wrapper before the first await reduces exposure but does not protect wrappers already retained elsewhere.

source: ['codex']

SDKLogger.log(
"🗑️ wallet \(walletLabel)… delete keychain phase: "
+ String(describing: phaseStartedAt.duration(to: .now)),
minimumLevel: .low)

phaseStartedAt = .now
try walletId.withUnsafeBytes { raw in
guard let base = raw.baseAddress?.assumingMemoryBound(to: FFIByteTuple32.self) else {
throw PlatformWalletError.nullPointer(
Expand All @@ -1021,31 +1035,32 @@ public class PlatformWalletManager: ObservableObject {
}

wallets.removeValue(forKey: walletId)
// Drop the needs-unlock banner state immediately so a re-created wallet
// with the same deterministic id doesn't inherit a stale banner (the
// poller would also prune it, but not until the next tick).
dashPayUnlockStatus.removeValue(forKey: walletId)

try persistenceHandler.deleteWalletData(walletId: walletId)

// The mnemonic + metadata blobs in the Keychain are keyed by
// `walletId`. With network-scoped wallet ids the same mnemonic
// maps to a DIFFERENT id per network, so a given id is owned by
// exactly one network's wallet and carries its own mnemonic
// copy — purging it can't orphan a sibling network (those live
// under their own distinct ids). The `walletRowCountAcrossNetworks
// == 0` check is therefore expected to be true right after
// `deleteWalletData` removes this id's lone row; it is retained
// as a defensive guard (and to stay correct should the id model
// ever change) so we never delete the phrase while any row for
// this exact id still exists.
let remaining = try persistenceHandler.walletRowCountAcrossNetworks(walletId: walletId)
SDKLogger.log(
"🗑️ wallet \(walletLabel)… delete Rust/in-memory phase: "
+ String(describing: phaseStartedAt.duration(to: .now)),
minimumLevel: .low)

phaseStartedAt = .now
try await persistenceHandler.deleteWalletData(walletId: walletId)
SDKLogger.log(
"🗑️ wallet \(walletLabel)… delete SwiftData phase: "
+ String(describing: phaseStartedAt.duration(to: .now)),
minimumLevel: .low)

let remaining = try await persistenceHandler.walletRowCountAcrossNetworks(
walletId: walletId)
if remaining == 0 {
let storage = WalletStorage()
// Delete metadata first so the mnemonic remains available for retry.
try storage.deleteMetadata(for: walletId)
try storage.deleteMnemonic(for: walletId)
try await Task.detached(priority: .userInitiated) {
let storage = WalletStorage()
try storage.deleteMetadata(for: walletId)
try storage.deleteMnemonic(for: walletId)
}.value
}
SDKLogger.log(
"🗑️ wallet \(walletLabel)… delete total: "
+ String(describing: startedAt.duration(to: .now)),
minimumLevel: .low)
}

// MARK: - Per-wallet lookup
Expand Down Expand Up @@ -1238,6 +1253,19 @@ public class PlatformWalletManager: ObservableObject {
}
}

private func ensureNoWalletDeletionInProgress() throws {
guard !walletDeletionInProgress else {
throw PlatformWalletError.walletOperation(
"wallet deletion is already in progress"
)
}
}

private func beginWalletDeletion() throws {
try ensureNoWalletDeletionInProgress()
walletDeletionInProgress = true
}

/// Starts the SPV progress polling loop. Cancelled on deinit.
///
/// `@Published` assignments are gated on inequality so that identical
Expand Down
Loading
Loading