diff --git a/DashWallet/Sources/Models/CrowdNode/Services/TransactionObserver.swift b/DashWallet/Sources/Models/CrowdNode/Services/TransactionObserver.swift index 8c960ec51..0d4512e5d 100644 --- a/DashWallet/Sources/Models/CrowdNode/Services/TransactionObserver.swift +++ b/DashWallet/Sources/Models/CrowdNode/Services/TransactionObserver.swift @@ -90,8 +90,7 @@ public final class TransactionObserver { /// Persisted rows decoded to `ObservedTransaction`, newest-first /// (`firstSeen` desc). Empty (logged) when the SDK host has no container /// yet or the network is unsupported. Safe from any thread — trampolines - /// to main because `mainContext` and the `Transaction` wrapper are - /// main-bound (same shape as `SwiftDashSDKWalletSource.allTransactions`). + /// to main because this scanner reads through `mainContext`. static func fetchObserved( fetchLimit: Int? = nil, firstSeenAtOrAfter: UInt64? = nil diff --git a/DashWallet/Sources/Models/Transactions/Model/Transaction.swift b/DashWallet/Sources/Models/Transactions/Model/Transaction.swift index 6e4ec3396..865db0ac8 100644 --- a/DashWallet/Sources/Models/Transactions/Model/Transaction.swift +++ b/DashWallet/Sources/Models/Transactions/Model/Transaction.swift @@ -26,8 +26,8 @@ import SwiftDashSDK /// stop/start (e.g. an SPV restart), which tears down the `ModelContainer` and /// resets its context; reading any model property afterwards traps ("instance /// was destroyed by calling ModelContext.reset"). Snapshotting every UI-read -/// field at wrap time — on the main actor, while the model is alive — makes -/// the wrapper immune to that teardown. +/// field at wrap time — on the fetch thread, while the model is alive — makes +/// the wrapper immune to that teardown (and thread-safe to hand around). class Transaction: TransactionDataItem, Identifiable { enum State { case ok @@ -73,8 +73,9 @@ class Transaction: TransactionDataItem, Identifiable { /// the send intent (see the synthetic initializer). let externalSentAddresses: [String] - /// Must be called on the main actor — reads `p`'s relationships - /// (`outputs`/`inputs`), which are bound to the model-context actor. + /// Must be called on the thread that owns `p`'s `ModelContext` (the + /// fetch thread) — reads `p`'s relationships (`outputs`/`inputs`), + /// which are bound to that context. init(_ p: PersistentTransaction) { txid = p.txid direction = p.direction @@ -118,12 +119,12 @@ class Transaction: TransactionDataItem, Identifiable { /// CoinJoin "mixing operation" flag — drives grouping into the single /// "Mixing Transactions" home-screen row. /// - /// Computed and cached on the MAIN actor at wrap time - /// (`SwiftDashSDKWalletSource.fetchAndWrapOnMain`), because deciding - /// membership traverses SwiftData relationships (outputs → coreAddress → - /// account) that are bound to the model-context actor and must not be read - /// from the background grouping queue. Defaults to false; the home tx source - /// is the sole producer of home-list wrappers and always populates it. + /// Computed and cached at wrap time on the fetch thread + /// (`SwiftDashSDKWalletSource.fetchAndWrap`), because deciding membership + /// traverses SwiftData relationships (outputs → coreAddress → account) + /// that are bound to the fetching `ModelContext` and must not be read + /// after the wrap. Defaults to false; the home tx source is the sole + /// producer of home-list wrappers and always populates it. var sdkCoinJoinMixing: Bool = false /// True only for CoinJoin mixing transactions. The flag is computed via @@ -398,7 +399,7 @@ class Transaction: TransactionDataItem, Identifiable { } init(persistentTransaction p: PersistentTransaction) { - // Freeze every UI-read field now, on the main actor where `p`'s model + // Freeze every UI-read field now, on the thread where `p`'s model // context is alive. After this the wrapper never dereferences `p` // again, so it stays valid after a ModelContext reset (see SDKSnapshot). self.snapshot = SDKSnapshot(p) diff --git a/DashWallet/Sources/UI/Home/Views/HomeViewModel.swift b/DashWallet/Sources/UI/Home/Views/HomeViewModel.swift index 5f9d0dd5e..ab980a79e 100644 --- a/DashWallet/Sources/UI/Home/Views/HomeViewModel.swift +++ b/DashWallet/Sources/UI/Home/Views/HomeViewModel.swift @@ -62,6 +62,11 @@ class HomeViewModel: ObservableObject { /// Debounce timer for sync state changes to prevent excessive reloads (Fix #4) private var syncStateDebounceWorkItem: DispatchWorkItem? private let syncStateDebounceInterval: TimeInterval = 0.5 + + /// Funnel for every "wallet content changed, reload the tx list" trigger. + /// Throttled in `observeWallet()` so the save/balance notification storm + /// during sync coalesces into at most one full reload per interval. + private let txReloadRequests = PassthroughSubject() @Published private(set) var txItems: [TransactionGroup] = [] @Published var shortcutItems: [ShortcutAction] = [] @@ -117,6 +122,13 @@ class HomeViewModel: ObservableObject { self.onSyncStateChanged() self.recalculateHeight() + // First load right away — `onSyncStateChanged()` above only schedules + // one behind its 0.5s debounce, which reads as a visibly empty list + // at startup before the rows pop in. There's no notification storm to + // coalesce yet, and the fetch runs on `queue`, so this is safe to + // start immediately. + self.reloadTxsAndShortcuts() + self.observeCoinJoinSweep() self.observeWallet() self.observeNetworkChange() @@ -194,12 +206,34 @@ class HomeViewModel: ObservableObject { } private func observeWallet() { - NotificationCenter.default.publisher(for: Notification.Name.fiatCurrencyDidChange) + // All reload triggers below funnel into this one throttled pipeline. + // During sync the persister saves a SwiftData batch several times per + // second (with a balance notification usually alongside), and each + // full reload is expensive — unthrottled, the storm kept the reload + // queue permanently busy and the list janky. `throttle` (not + // `debounce`) so a continuous save storm still repaints once per + // interval instead of starving until it ends; `latest: true` + // guarantees a trailing reload that picks up the final batch. A lone + // event (e.g. a received tx while idle) passes through immediately. + txReloadRequests + .throttle(for: .seconds(1), scheduler: DispatchQueue.main, latest: true) .sink { [weak self] _ in self?.reloadTxsAndShortcuts() } .store(in: &cancellableBag) + // Each trigger hops to the main queue before `send()` — the + // notifications post from arbitrary threads (persister save thread, + // balance updates) and PassthroughSubject requires serialized sends. + // (DispatchQueue.main, not RunLoop.main: the latter is default-mode + // only and stalls delivery during scroll tracking.) + NotificationCenter.default.publisher(for: Notification.Name.fiatCurrencyDidChange) + .receive(on: DispatchQueue.main) + .sink { [weak self] _ in + self?.txReloadRequests.send() + } + .store(in: &cancellableBag) + // Reload when SwiftData saves — Rust's persister callback writes // PersistentTransaction rows on every Core SPV / BLAST batch, and // SwiftData posts NSManagedObjectContextDidSave under the hood. The @@ -208,20 +242,19 @@ class HomeViewModel: ObservableObject { // directly from SwiftData via SwiftDashSDKWalletSource and use this // notification as the reload trigger. NotificationCenter.default.publisher(for: .NSManagedObjectContextDidSave) - .receive(on: RunLoop.main) + .receive(on: DispatchQueue.main) .sink { [weak self] _ in - DWLogger.log("HomeViewModel: SwiftData saved, reloading tx list") - self?.reloadTxsAndShortcuts() + self?.txReloadRequests.send() } .store(in: &cancellableBag) - // Fix #5: Balance changes often indicate new transactions, so reload the full - // transaction list, not just shortcuts. This ensures newly received or sent - // transactions appear in the UI promptly. + // Balance changes often indicate new transactions, so reload the full + // transaction list, not just shortcuts. This ensures newly received or + // sent transactions appear in the UI promptly. NotificationCenter.default.publisher(for: NSNotification.Name.DSWalletBalanceDidChange) + .receive(on: DispatchQueue.main) .sink { [weak self] _ in - DWLogger.log("HomeViewModel: Wallet balance changed, reloading transactions and shortcuts") - self?.reloadTxsAndShortcuts() + self?.txReloadRequests.send() } .store(in: &cancellableBag) @@ -256,10 +289,17 @@ class HomeViewModel: ObservableObject { self.coinJoinTxSets = [:] self.coinJoinWithdrawalSet = CoinJoinWithdrawalTxSet() + // Snapshot each provider's metadata once per reload — + // `availableMetadata` copies the whole dictionary through the + // provider's serial queue, so reading it per-transaction costs + // O(n) queue hops + dictionary copies per reload. + let metadataSnapshots = self.metadataProviders.map { $0.availableMetadata } + let giftCardTxIds = Set(GiftCardMetadataProvider.shared.availableMetadata.keys) + var items: [TransactionListDataItem] = transactions.compactMap { wrappedTx -> TransactionListDataItem? in Tx.shared.updateRateIfNeeded(for: wrappedTx) - if !self.passesFilter(transaction: wrappedTx, displayMode: self.displayMode) { + if !self.passesFilter(transaction: wrappedTx, displayMode: self.displayMode, giftCardTxIds: giftCardTxIds) { return nil } @@ -287,7 +327,7 @@ class HomeViewModel: ObservableObject { } } - return .tx(wrappedTx, self.resolveMetadata(for: wrappedTx.txHashData)) + return .tx(wrappedTx, self.resolveMetadata(for: wrappedTx.txHashData, in: metadataSnapshots)) } self.txByHash.removeAll() @@ -624,12 +664,18 @@ extension HomeViewModel { } private func resolveMetadata(for txId: Data) -> TxRowMetadata? { + resolveMetadata(for: txId, in: metadataProviders.map { $0.availableMetadata }) + } + + /// `snapshots` holds one pre-copied `availableMetadata` per provider, in + /// provider (priority) order — full-reload loops snapshot once and reuse + /// across all transactions instead of re-copying per row. + private func resolveMetadata(for txId: Data, in snapshots: [[Data: TxRowMetadata]]) -> TxRowMetadata? { var finalMetadata: TxRowMetadata? = nil // Metadata will not be replaced if already found, so in case // of conflicts metadataProviders should be sorted by priority - for provider in self.metadataProviders { - let providerMetadata = provider.availableMetadata + for providerMetadata in snapshots { guard let metadata = providerMetadata[txId] else { continue } if finalMetadata == nil { @@ -660,7 +706,10 @@ extension HomeViewModel { return finalMetadata } - private func passesFilter(transaction: Transaction, displayMode: HomeTxDisplayMode) -> Bool { + /// `giftCardTxIds` is an optional pre-snapshotted key set for the + /// `.giftCard` mode — full-reload loops pass it to avoid copying the + /// provider's dictionary per transaction; single-tx callers omit it. + private func passesFilter(transaction: Transaction, displayMode: HomeTxDisplayMode, giftCardTxIds: Set? = nil) -> Bool { switch displayMode { case .all: return true @@ -671,7 +720,8 @@ extension HomeViewModel { case .rewards: return transaction.isCoinbaseTransaction case .giftCard: - return GiftCardMetadataProvider.shared.availableMetadata[transaction.txHashData] != nil + let ids = giftCardTxIds ?? Set(GiftCardMetadataProvider.shared.availableMetadata.keys) + return ids.contains(transaction.txHashData) } } } @@ -788,11 +838,13 @@ protocol TransactionSource { /// for the existing UIKit + Combine home view: instead of `@Query`, we do /// a synchronous fetch and feed the existing `Transaction` wrapper. /// -/// `SwiftDashSDKHost` is `@MainActor`-isolated; this getter is invoked -/// from `HomeViewModel.queue` (a background dispatch queue), so the read -/// trampolines through `DispatchQueue.main.sync`. The fetch itself is -/// fast (<10ms for a few hundred rows), so blocking the worker queue -/// briefly is acceptable. +/// Threading: only the `@MainActor` host's handles (`modelContainer` + +/// active `walletId`) are read through a brief main-thread hop. The fetch +/// and the per-tx wrapping run on the CALLER's thread against a private +/// `ModelContext`, so a full home-list reload never blocks the main +/// thread mid-scroll. A private context reads the last SAVED state — +/// which is exactly what the `NSManagedObjectContextDidSave` reload +/// trigger guarantees is current. class SwiftDashSDKWalletSource: TransactionSource { var allTransactions: Array { Self.fetchAll().sorted { $0.date > $1.date } @@ -802,20 +854,33 @@ class SwiftDashSDKWalletSource: TransactionSource { /// Safe from any thread. Shared read for every tx-history consumer that /// used to enumerate DashSync's `DSWallet.allTransactions`. static func fetchAll() -> [Transaction] { - onMain { fetchAndWrapOnMain() } + guard let (container, walletId) = hostHandles() else { return [] } + return fetchAndWrap(in: ModelContext(container), walletId: walletId) } /// Single transaction by txid (wire order — the same `Data` as /// `DSTransaction.txHashData`). Safe from any thread. static func fetch(txid: Data) -> Transaction? { - onMain { fetchOnMain(txid: txid) } + guard let (container, walletId) = hostHandles() else { return nil } + return fetchOne(txid: txid, in: ModelContext(container), walletId: walletId) + } + + /// The host is `@MainActor`-isolated; grab its container + active-wallet + /// id in one brief main hop (two property reads — unlike the fetches, + /// cheap enough to block a worker queue on). `ModelContainer` is + /// `Sendable`, so the caller then opens its own `ModelContext` on its + /// own thread and all SwiftData work stays there. + private static func hostHandles() -> (container: ModelContainer, walletId: Data)? { + onMain { + guard let container = SwiftDashSDKHost.shared.modelContainer, + let walletId = SwiftDashSDKHost.shared.wallet?.walletId else { + return nil + } + return (container, walletId) + } } - /// Main-thread trampoline: `mainContext` and the `Transaction` wrapping - /// (the per-tx CoinJoin membership walk over SwiftData relationships — - /// outputs → coreAddress → account) are main-bound, while callers run on - /// background queues. Fetches are fast (<10ms for a few hundred rows), so - /// briefly blocking the worker queue is acceptable. + /// Main-thread trampoline for the `@MainActor`-isolated host reads. private static func onMain(_ body: @MainActor () -> T) -> T { if Thread.isMainThread { return MainActor.assumeIsolated(body) @@ -825,16 +890,11 @@ class SwiftDashSDKWalletSource: TransactionSource { } } - @MainActor - private static func fetchOnMain(txid: Data) -> Transaction? { - guard let container = SwiftDashSDKHost.shared.modelContainer, - let walletId = SwiftDashSDKHost.shared.wallet?.walletId else { - return nil - } + private static func fetchOne(txid: Data, in context: ModelContext, walletId: Data) -> Transaction? { var descriptor = FetchDescriptor( predicate: #Predicate { $0.txid == txid }) descriptor.fetchLimit = 1 - guard let row = (try? container.mainContext.fetch(descriptor))?.first else { + guard let row = (try? context.fetch(descriptor))?.first else { return nil } // Membership check: a tx the active wallet doesn't participate in @@ -858,14 +918,13 @@ class SwiftDashSDKWalletSource: TransactionSource { /// `transaction` (funds in) and the `spendingTransaction` (funds out). /// One walletId-scoped fetch per reload (not per transaction), so the /// timeline stays a single indexed scan on large wallets. - @MainActor private static func activeWalletTxids( - in container: ModelContainer, + in context: ModelContext, walletId: Data ) -> Set { let descriptor = FetchDescriptor( predicate: #Predicate { $0.walletId == walletId }) - guard let txos = try? container.mainContext.fetch(descriptor) else { return [] } + guard let txos = try? context.fetch(descriptor) else { return [] } var txids = Set() for txo in txos { if let producing = txo.transaction { txids.insert(producing.txid) } @@ -874,20 +933,15 @@ class SwiftDashSDKWalletSource: TransactionSource { return txids } - @MainActor - private static func fetchAndWrapOnMain() -> [Transaction] { - guard let container = SwiftDashSDKHost.shared.modelContainer, - let walletId = SwiftDashSDKHost.shared.wallet?.walletId else { - return [] - } - let txids = activeWalletTxids(in: container, walletId: walletId) + private static func fetchAndWrap(in context: ModelContext, walletId: Data) -> [Transaction] { + let txids = activeWalletTxids(in: context, walletId: walletId) guard !txids.isEmpty else { return [] } let descriptor = FetchDescriptor( predicate: #Predicate { txids.contains($0.txid) }, sortBy: [SortDescriptor(\.firstSeen, order: .reverse)]) let rows: [PersistentTransaction] do { - rows = try container.mainContext.fetch(descriptor) + rows = try context.fetch(descriptor) } catch { DWLogger.log("HomeViewModel: PersistentTransaction fetch failed: \(error)") return [] @@ -905,13 +959,13 @@ class SwiftDashSDKWalletSource: TransactionSource { /// Account type owning a TXO — canonical path is `coreAddress?.account`; /// `account` is the fallback used before the address row is linked. - @MainActor private static func ownerAccountType(_ txo: PersistentTxo) -> UInt32? { (txo.coreAddress?.account ?? txo.account)?.accountType } - /// CoinJoin mixing-operation detection (main actor — traverses SwiftData - /// relationships). DashSync grouped by CoinJoin-account *role*, not tx + /// CoinJoin mixing-operation detection. Traverses SwiftData relationships, + /// so it must run on the thread that owns the row's `ModelContext` (the + /// fetch thread). DashSync grouped by CoinJoin-account *role*, not tx /// structure, so the SDK's structural `typedKind` (mixing rounds only) is /// too narrow. We classify a tx as a mixing operation when: /// 1. it DEPOSITS into the CoinJoin account (≥1 CoinJoin output) — covers @@ -921,7 +975,6 @@ class SwiftDashSDKWalletSource: TransactionSource { /// collateral spend (the tiny "Sent 0.0001" txs). A Standard output /// marks the CoinJoin→BIP44 sweep or an internal transfer out, which /// must stay an individual row — matching DashSync's "Send" exclusion. - @MainActor private static func isCoinJoinMixingTx(_ p: PersistentTransaction) -> Bool { if p.typedKind == .coinJoin { return true } if p.outputs.contains(where: { ownerAccountType($0) == coinJoinAccountType }) {