diff --git a/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKHost.swift b/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKHost.swift index 1599adfbc..dabf5b77a 100644 --- a/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKHost.swift +++ b/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKHost.swift @@ -68,8 +68,8 @@ enum MnemonicFirstWalletCreation { persistMnemonic: () throws -> Void, retrieveMnemonic: () throws -> String, rollbackMnemonic: () -> Void, - createWallet: () throws -> Wallet - ) throws -> Wallet { + createWallet: () async throws -> Wallet + ) async throws -> Wallet { do { try persistMnemonic() guard try retrieveMnemonic() == mnemonic else { @@ -84,7 +84,7 @@ enum MnemonicFirstWalletCreation { } do { - return try createWallet() + return try await createWallet() } catch { rollbackMnemonic() throw MnemonicFirstWalletCreationError.walletCreation(error) @@ -515,7 +515,7 @@ final class SwiftDashSDKHost { let handles = try await buildRuntime(for: network) let createdWallet: ManagedPlatformWallet do { - createdWallet = try createAndPersist( + createdWallet = try await createAndPersist( mnemonic: mnemonic, manager: handles.manager, network: handles.network, @@ -527,7 +527,10 @@ final class SwiftDashSDKHost { // the scan anchors at the tip. birthHeight: isImported ? Self.importedWalletBirthHeight(for: handles.network) - : nil) + : nil, + // Onboarding is not lifecycle-queue-serialized: keep the + // persist→create critical section MainActor-atomic. + offMainCreate: false) if provisionAcrossSupportedNetworks { let persistedWalletIds = Set(try WalletStorage().listWalletIdsWithMnemonic()) @@ -544,13 +547,14 @@ final class SwiftDashSDKHost { let (targetManager, isTemporary) = try await managerForStoredWalletOperation( network: targetNetwork) do { - _ = try createAndPersist( + _ = try await createAndPersist( mnemonic: mnemonic, manager: targetManager, network: targetNetwork, birthHeight: isImported ? Self.importedWalletBirthHeight(for: targetNetwork) - : nil) + : nil, + offMainCreate: false) } catch { if isTemporary { await targetManager.shutdown() } throw error @@ -580,8 +584,10 @@ final class SwiftDashSDKHost { return createdWallet } - /// Outcome of `addWallet(mnemonic:isImported:)`. - enum AddWalletResult { + /// Outcome of `addWallet(mnemonic:isImported:)`. `Sendable` because it + /// crosses the lifecycle queue's awaitable seam + /// (`SwiftDashSDKWalletRuntime.performAddWallet`). + enum AddWalletResult: Sendable { /// The wallet was created and its mnemonic persisted; the running /// runtime is unchanged (the caller switches to it explicitly). case added(walletId: Data) @@ -635,6 +641,11 @@ final class SwiftDashSDKHost { /// Shares the persist-then-create transaction with /// `createOrImportWallet` (`createAndPersist`); differs only in that it /// uses the LIVE manager and does not publish or set-active. + /// + /// Interactive callers route through + /// `SwiftDashSDKWalletRuntime.performAddWallet`, which runs this method + /// as one link of the serial lifecycle chain so queued refresh/reset + /// operations cannot interleave with the multi-network provisioning. @discardableResult func addWallet(mnemonic: String, isImported: Bool) async throws -> AddWalletResult { let mnemonic = Mnemonic.normalizePhrase(mnemonic) @@ -670,7 +681,7 @@ final class SwiftDashSDKHost { network: targetNetwork) } do { - _ = try createAndPersist( + _ = try await createAndPersist( mnemonic: mnemonic, manager: targetManager, network: targetNetwork, @@ -679,7 +690,11 @@ final class SwiftDashSDKHost { // wallets from that network's tip. birthHeight: isImported ? Self.importedWalletBirthHeight(for: targetNetwork) - : nil) + : nil, + // The interactive add runs as one lifecycle-queue op + // (`performAddWallet`), so refreshes cannot observe the + // suspension the off-main create introduces. + offMainCreate: true) } catch { if isTemporary { await targetManager.shutdown() } throw error @@ -708,12 +723,25 @@ final class SwiftDashSDKHost { /// unsynced headers the SDK falls back to the network's newest /// hardcoded checkpoint), `0` scans from genesis (imported mnemonic /// whose history predates this device). + /// `offMainCreate` picks the SDK create overload. `true` (interactive + /// add): the async overload — the blocking FFI leaves the MainActor, + /// but the transaction gains a REAL suspension point between the + /// mnemonic persist and the wallet rows appearing; safe only when the + /// caller is serialized against runtime refreshes (the add flow runs on + /// the lifecycle queue). `false` (onboarding/migration): the sync + /// overload — no suspension between persist and create, so a + /// concurrently scheduled `startIfReady`/refresh can never observe the + /// half-state (mnemonic present, no wallet rows) and build a competing + /// runtime; `createOrImportWallet` is NOT queue-serialized (the + /// migrator is awaited by refresh itself — enqueueing would deadlock), + /// so it must keep the MainActor-atomic critical section. private func createAndPersist( mnemonic: String, manager: PlatformWalletManager, network: Network, - birthHeight: UInt32? - ) throws -> ManagedPlatformWallet { + birthHeight: UInt32?, + offMainCreate: Bool + ) async throws -> ManagedPlatformWallet { let walletId: Data do { // This is the same deterministic id contract used by addWallet's @@ -736,7 +764,7 @@ final class SwiftDashSDKHost { } do { - return try MnemonicFirstWalletCreation.run( + return try await MnemonicFirstWalletCreation.run( mnemonic: mnemonic, persistMnemonic: { try storage.storeMnemonic(mnemonic, for: walletId) @@ -752,12 +780,31 @@ final class SwiftDashSDKHost { } }, createWallet: { - try manager.createWallet( - mnemonic: mnemonic, - network: network, - name: "dashwallet", - createDefaultAccounts: true, - birthHeight: birthHeight) + if offMainCreate { + // Async SDK overload: the blocking native create + // runs on the SDK's dedicated queue, not the main + // thread. + return try await manager.createWallet( + mnemonic: mnemonic, + network: network, + name: "dashwallet", + createDefaultAccounts: true, + birthHeight: birthHeight) + } + // Sync SDK overload, forced by the explicit non-async + // function type (an async context would otherwise + // prefer the async one): blocks the MainActor for the + // whole create, keeping persist→create atomic for the + // unserialized onboarding path. + let syncCreate: () throws -> ManagedPlatformWallet = { + try manager.createWallet( + mnemonic: mnemonic, + network: network, + name: "dashwallet", + createDefaultAccounts: true, + birthHeight: birthHeight) + } + return try syncCreate() }) } catch MnemonicFirstWalletCreationError.mnemonicRoundTripMismatch { Self.logger.error("🪺 HOST :: mnemonic persistence round-trip mismatch") diff --git a/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKWalletRuntime.swift b/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKWalletRuntime.swift index 5f94e50b7..43349fb73 100644 --- a/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKWalletRuntime.swift +++ b/DashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKWalletRuntime.swift @@ -37,6 +37,24 @@ final class SerialAsyncLifecycleQueue { currentTask = task return task } + + /// Value-returning variant: the operation is one link of the same serial + /// chain (full barrier semantics — everything enqueued later waits for + /// it), and its result or error is handed back to the caller through a + /// continuation. The chain itself stays `Task`. + func enqueueAwaitable( + _ operation: @escaping @MainActor () async throws -> T + ) async throws -> T { + try await withCheckedThrowingContinuation { continuation in + enqueue { + do { + continuation.resume(returning: try await operation()) + } catch { + continuation.resume(throwing: error) + } + } + } + } } @objc(DWSwiftDashSDKWalletRuntime) @@ -282,6 +300,24 @@ final class SwiftDashSDKWalletRuntime: NSObject { publishActiveWalletDidChange(reason: "wallet-switch") } + /// Additive wallet provisioning (`SwiftDashSDKHost.addWallet`) as ONE + /// link of the serial lifecycle chain, so a queued `refresh`/`fullReset` + /// can never interleave with the multi-network create. Like + /// `switchNetwork(to:)`, this MUST NOT be called from within a lifecycle + /// operation already running on the chain — it awaits its own enqueued + /// op and would self-await-deadlock the queue. The add flow's post-add + /// `switchWallet` is deliberately a SECOND, sequential chain op (the + /// caller awaits this method first), never nested inside it. + @MainActor + func performAddWallet(mnemonic: String, isImported: Bool) async throws + -> SwiftDashSDKHost.AddWalletResult { + try await lifecycleQueue.enqueueAwaitable { + try await SwiftDashSDKHost.shared.addWallet( + mnemonic: mnemonic, + isImported: isImported) + } + } + /// Validate that `walletId` is a switchable target on the current network: /// the network must be SDK-supported and a mnemonic for `walletId` must be /// persisted in `WalletStorage` — the same keychain surface the host loads diff --git a/DashWallet/Sources/UI/Menu/Security/Wallets/WalletsViewModel.swift b/DashWallet/Sources/UI/Menu/Security/Wallets/WalletsViewModel.swift index 3f604c538..9ac0f30e4 100644 --- a/DashWallet/Sources/UI/Menu/Security/Wallets/WalletsViewModel.swift +++ b/DashWallet/Sources/UI/Menu/Security/Wallets/WalletsViewModel.swift @@ -265,10 +265,11 @@ final class WalletsViewModel: ObservableObject { addInProgress = true defer { addInProgress = false } - // Blocking overlay from the FIRST moment: provisioning is seconds of - // blocking MainActor work (wallet creation FFI on both networks, plus - // the other network's SDK build), and without this phase the app + // Blocking overlay from the FIRST moment: without this phase the app // looked frozen until the post-add switch finally raised the window. + // The create FFI itself now runs off-main (async SDK createWallet), + // but the mirror-network leg still blocks the MainActor for its + // SDK build + loadFromPersistor — TODO(etap-C). WalletLifecycleOverlayPresenter.shared.ensureActive() let state = WalletLifecycleTransitionState.shared guard state.tryBegin(.addingWallet(isImport: isImported)) else { @@ -276,9 +277,12 @@ final class WalletsViewModel: ObservableObject { return nil } // Give UIKit one runloop turn to commit the overlay window before the - // blocking provisioning starts (the Obj-C wipe's 0.1 s dispatch_after - // trick): the phase-apply Task was already enqueued by the sink, and - // both it and the CoreAnimation commit run while this sleeps. + // provisioning starts (the Obj-C wipe's 0.1 s dispatch_after trick). + // Still required even with the off-main create: on the mirror-repair + // path the FIRST provisioning work is the other network's synchronous + // SDK build on the MainActor, with no suspension point before it — + // without this sleep the window would not be committed until that + // block ends. Remove together with etap C. try? await Task.sleep(for: .milliseconds(100)) let opID = String(UUID().uuidString.prefix(8)) @@ -287,7 +291,12 @@ final class WalletsViewModel: ObservableObject { let result: SwiftDashSDKHost.AddWalletResult do { - result = try await SwiftDashSDKHost.shared.addWallet( + // Through the runtime's serial lifecycle chain: a queued + // refresh/fullReset can no longer interleave with the + // multi-network provisioning. The post-add switch below stays + // OUTSIDE the chain (its own sequential op — nesting it here + // would self-await-deadlock the queue). + result = try await SwiftDashSDKWalletRuntime.shared.performAddWallet( mnemonic: normalized, isImported: isImported) } catch { diff --git a/DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift b/DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift index 223314982..397557021 100644 --- a/DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift +++ b/DashWalletTests/SwiftDashSDKCoreLifecycleTests.swift @@ -78,6 +78,40 @@ final class SwiftDashSDKCoreLifecycleTests: XCTestCase { ]) } + /// The value-returning variant is one link of the same serial chain: + /// its result comes back to the caller, its error propagates, and ops + /// enqueued around it stay strictly ordered. + func testEnqueueAwaitableReturnsValueThrowsAndKeepsChainOrder() async throws { + let queue = SerialAsyncLifecycleQueue() + var events: [String] = [] + + queue.enqueue { events.append("before") } + let value = try await queue.enqueueAwaitable { () async throws -> Int in + events.append("awaitable") + return 41 + } + queue.enqueue { events.append("after") } + + XCTAssertEqual(value, 41) + XCTAssertEqual(events, ["before", "awaitable"]) + + do { + _ = try await queue.enqueueAwaitable { () async throws -> Int in + events.append("throwing") + throw CoreLifecycleTestError.start + } + XCTFail("Expected the enqueued error to propagate") + } catch CoreLifecycleTestError.start { + // Expected — and the chain must survive a thrown link. + } + + _ = try await queue.enqueueAwaitable { () async throws -> Int in + events.append("tail") + return 0 + } + XCTAssertEqual(events, ["before", "awaitable", "after", "throwing", "tail"]) + } + func testProcessCacheReusesValuesPerNetworkAndSeparatesNetworks() { final class Token {} diff --git a/DashWalletTests/WalletWipeSerialExecutorTests.swift b/DashWalletTests/WalletWipeSerialExecutorTests.swift index 9210c6b7a..d0f520afe 100644 --- a/DashWalletTests/WalletWipeSerialExecutorTests.swift +++ b/DashWalletTests/WalletWipeSerialExecutorTests.swift @@ -512,27 +512,29 @@ final class RecoveryPhraseRoutingTests: XCTestCase { } final class MnemonicFirstWalletCreationTests: XCTestCase { - func testPersistenceFailureDoesNotCreateWallet() { + func testPersistenceFailureDoesNotCreateWallet() async { var storedMnemonic: String? var createCalled = false - XCTAssertThrowsError(try MnemonicFirstWalletCreation.run( - mnemonic: "test mnemonic", - persistMnemonic: { - storedMnemonic = "partially written" - throw WalletLifecycleTestError.mnemonicPersistence - }, - retrieveMnemonic: { - storedMnemonic ?? "" - }, - rollbackMnemonic: { - storedMnemonic = nil - }, - createWallet: { - createCalled = true - return 1 - } - )) { error in + do { + _ = try await MnemonicFirstWalletCreation.run( + mnemonic: "test mnemonic", + persistMnemonic: { + storedMnemonic = "partially written" + throw WalletLifecycleTestError.mnemonicPersistence + }, + retrieveMnemonic: { + storedMnemonic ?? "" + }, + rollbackMnemonic: { + storedMnemonic = nil + }, + createWallet: { + createCalled = true + return 1 + }) + XCTFail("expected mnemonicPersistence") + } catch { guard case MnemonicFirstWalletCreationError.mnemonicPersistence = error else { return XCTFail("unexpected error: \(error)") } @@ -542,11 +544,11 @@ final class MnemonicFirstWalletCreationTests: XCTestCase { XCTAssertNil(storedMnemonic) } - func testMnemonicIsAvailableBeforeWalletCreation() throws { + func testMnemonicIsAvailableBeforeWalletCreation() async throws { let mnemonic = "test mnemonic" var storedMnemonic: String? - let wallet = try MnemonicFirstWalletCreation.run( + let wallet = try await MnemonicFirstWalletCreation.run( mnemonic: mnemonic, persistMnemonic: { storedMnemonic = mnemonic @@ -566,25 +568,27 @@ final class MnemonicFirstWalletCreationTests: XCTestCase { XCTAssertEqual(storedMnemonic, mnemonic) } - func testCreationFailureRemovesProvisionalMnemonicAndPropagatesError() { + func testCreationFailureRemovesProvisionalMnemonicAndPropagatesError() async { let mnemonic = "test mnemonic" var storedMnemonic: String? - XCTAssertThrowsError(try MnemonicFirstWalletCreation.run( - mnemonic: mnemonic, - persistMnemonic: { - storedMnemonic = mnemonic - }, - retrieveMnemonic: { - storedMnemonic ?? "" - }, - rollbackMnemonic: { - storedMnemonic = nil - }, - createWallet: { () throws -> Int in - throw WalletLifecycleTestError.walletCreation - } - )) { error in + do { + _ = try await MnemonicFirstWalletCreation.run( + mnemonic: mnemonic, + persistMnemonic: { + storedMnemonic = mnemonic + }, + retrieveMnemonic: { + storedMnemonic ?? "" + }, + rollbackMnemonic: { + storedMnemonic = nil + }, + createWallet: { () async throws -> Int in + throw WalletLifecycleTestError.walletCreation + }) + XCTFail("expected walletCreation") + } catch { guard case MnemonicFirstWalletCreationError.walletCreation = error else { return XCTFail("unexpected error: \(error)") } @@ -593,25 +597,36 @@ final class MnemonicFirstWalletCreationTests: XCTestCase { XCTAssertNil(storedMnemonic) } - func testCreationFailureRestoresPreviousMnemonic() { + /// The rollback must also run when the failure happens AFTER the async + /// create closure suspended (the off-main SDK create) — same transaction + /// semantics as the old synchronous closure. + func testCreationFailureAfterSuspensionRestoresPreviousMnemonic() async { let previousMnemonic = "previous mnemonic" let replacementMnemonic = "replacement mnemonic" var storedMnemonic: String? = previousMnemonic - XCTAssertThrowsError(try MnemonicFirstWalletCreation.run( - mnemonic: replacementMnemonic, - persistMnemonic: { - storedMnemonic = replacementMnemonic - }, - retrieveMnemonic: { - storedMnemonic ?? "" - }, - rollbackMnemonic: { - storedMnemonic = previousMnemonic - }, - createWallet: { () throws -> Int in - throw WalletLifecycleTestError.walletCreation - })) + do { + _ = try await MnemonicFirstWalletCreation.run( + mnemonic: replacementMnemonic, + persistMnemonic: { + storedMnemonic = replacementMnemonic + }, + retrieveMnemonic: { + storedMnemonic ?? "" + }, + rollbackMnemonic: { + storedMnemonic = previousMnemonic + }, + createWallet: { () async throws -> Int in + await Task.yield() + throw WalletLifecycleTestError.walletCreation + }) + XCTFail("expected walletCreation") + } catch { + guard case MnemonicFirstWalletCreationError.walletCreation = error else { + return XCTFail("unexpected error: \(error)") + } + } XCTAssertEqual(storedMnemonic, previousMnemonic) }