diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 1160ec4740..29845b8104 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -15,6 +15,19 @@ import DashSDKFFI /// Use as a root `@StateObject` and pass via `.environmentObject(_:)`. /// Views observe `@Published` properties directly — no coordinator /// class in the middle. +/// Lock-guarded monotonic generation counter, safe to read and bump from +/// any thread. Used to drop shielded sync completion events that belong +/// to a generation already superseded by a `stop`/`clear`, even when a +/// restart happens in the same `@MainActor` turn (a plain boolean gate +/// can't, because the restart re-opens the gate before the stale, +/// previously-enqueued completion task runs). +final class ShieldedSyncGenerationCounter: @unchecked Sendable { + private let lock = NSLock() + private var value: UInt64 = 0 + func current() -> UInt64 { lock.withLock { value } } + @discardableResult func bump() -> UInt64 { lock.withLock { value &+= 1; return value } } +} + @MainActor public class PlatformWalletManager: ObservableObject { // MARK: - Published observables @@ -46,21 +59,26 @@ public class PlatformWalletManager: ObservableObject { /// Last completed shielded sync event emitted by Rust. @Published public internal(set) var lastShieldedSyncEvent: ShieldedSyncEvent? - /// When true, `handleShieldedSyncCompleted` drops incoming events - /// instead of publishing them. Set by `stopShieldedSync` / - /// `clearShielded` (after the Rust drain returns) and cleared by any - /// sync-start (`startShieldedSync` / `syncShieldedNow`). The Rust - /// quiesce barrier guarantees no persistence after stop/clear, but - /// the completion callback is re-dispatched onto this `@MainActor`, - /// so a final, already-dispatched event can land just after stop/ - /// clear returns; this gate keeps the published `lastShieldedSyncEvent` - /// honest for every SDK consumer (not just the example app). Both - /// stop/clear are synchronous on the main actor, so the flag is set - /// before the enqueued trailing-event task can run. + /// Monotonic generation for shielded sync passes. Each `stop`/`clear` + /// bumps it; the FFI completion callback snapshots the generation at + /// enqueue time and `handleShieldedSyncCompleted` drops any event whose + /// snapshot no longer matches the current generation. + /// + /// The Rust quiesce barrier guarantees no persistence after stop/clear, + /// but the completion callback is re-dispatched onto this `@MainActor`, + /// so a final, already-dispatched event can land just after stop/clear + /// returns. A plain boolean gate is bypassable: a caller can stop (set + /// the flag) and restart (clear the flag) in the same actor turn, which + /// re-opens the gate before the stale, previously-enqueued completion + /// task runs — so the old event leaks into the new run. Tying + /// suppression to a generation closes that race: the stale task carries + /// the pre-stop generation, the restart does not reset the counter, so + /// the snapshot mismatches and the event is dropped even on a same-turn + /// restart. /// - /// `internal` (not `private`) because the shielded lifecycle methods - /// that read/write it live in an extension in a separate file. - var suppressShieldedCompletionEvents: Bool = false + /// `nonisolated` + lock-guarded so the FFI callback thread can snapshot + /// it without hopping onto the main actor first. + nonisolated let shieldedSyncGeneration = ShieldedSyncGenerationCounter() /// All wallets currently held by the Rust-side /// `PlatformWalletManager`, keyed by the 32-byte wallet id. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift index 52d56bc60a..5738fd7c28 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerShieldedSync.swift @@ -58,12 +58,14 @@ public struct ShieldedSyncEvent: Sendable { } extension PlatformWalletManager { - func handleShieldedSyncCompleted(_ event: ShieldedSyncEvent) { - // Drop a trailing event that the Rust drain already dispatched - // but the main actor only delivers after stop/clear returned - // (see `suppressShieldedCompletionEvents`). Any sync-start clears - // the flag, so legitimate events are never suppressed. - guard !suppressShieldedCompletionEvents else { return } + func handleShieldedSyncCompleted(_ event: ShieldedSyncEvent, generation: UInt64) { + // Drop a trailing event that the Rust drain already dispatched but + // the main actor only delivers after stop/clear returned. The FFI + // callback snapshots `shieldedSyncGeneration` at enqueue time; a + // stop/clear bumps the counter, so a stale event's snapshot no + // longer matches and is dropped — even if a restart happened in the + // same actor turn (the restart does not reset the counter). + guard generation == shieldedSyncGeneration.current() else { return } lastShieldedSyncEvent = event } @@ -181,8 +183,10 @@ extension PlatformWalletManager { if let intervalSeconds { try setShieldedSyncInterval(seconds: intervalSeconds) } - // A new sync run should publish its completion events again. - suppressShieldedCompletionEvents = false + // No generation reset needed: events emitted by this new run + // snapshot the current generation, so they pass the guard. A + // trailing event from a prior, stopped run still carries the older + // generation and is dropped. try platform_wallet_manager_shielded_sync_start(handle).check() } @@ -193,9 +197,10 @@ extension PlatformWalletManager { ) } try platform_wallet_manager_shielded_sync_stop(handle).check() - // The Rust drain returned; suppress any trailing completion - // event the main actor delivers after this point. - suppressShieldedCompletionEvents = true + // The Rust drain returned; bump the generation so any trailing + // completion event the main actor delivers after this point is + // dropped (its snapshot predates this bump). + shieldedSyncGeneration.bump() } /// Reset the Rust-side shielded state on this manager: @@ -219,10 +224,11 @@ extension PlatformWalletManager { ) } try platform_wallet_manager_shielded_clear(handle).check() - // The Rust drain returned; suppress any trailing completion - // event the main actor delivers after Clear (it would otherwise - // briefly repopulate the mirror the host is about to wipe). - suppressShieldedCompletionEvents = true + // The Rust drain returned; bump the generation so any trailing + // completion event the main actor delivers after Clear is dropped + // (it would otherwise briefly repopulate the mirror the host is + // about to wipe). + shieldedSyncGeneration.bump() } public func isShieldedSyncRunning() throws -> Bool { @@ -273,9 +279,9 @@ extension PlatformWalletManager { "PlatformWalletManager not configured" ) } - // A user-initiated sync should publish its completion event even - // if a prior stop/clear had suppressed events. - suppressShieldedCompletionEvents = false + // No generation reset needed: this run's completion event snapshots + // the current generation and passes the guard, while a trailing + // event from a prior stopped run still carries the older generation. let handle = self.handle try await Task.detached(priority: .userInitiated) { try platform_wallet_manager_shielded_sync_sync_now(handle).check() @@ -586,7 +592,12 @@ func shieldedSyncCompletedCallback( walletResults: results ) + // Snapshot the generation now, on the FFI callback thread, BEFORE the + // event is enqueued onto the main actor. A subsequent stop/clear bumps + // the counter, so this trailing event is dropped when it finally runs. + let generation = handler.manager?.shieldedSyncGeneration.current() ?? 0 + Task { @MainActor [weak manager = handler.manager] in - manager?.handleShieldedSyncCompleted(event) + manager?.handleShieldedSyncCompleted(event, generation: generation) } } diff --git a/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedSyncGenerationTests.swift b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedSyncGenerationTests.swift new file mode 100644 index 0000000000..1c1b708a9a --- /dev/null +++ b/packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedSyncGenerationTests.swift @@ -0,0 +1,124 @@ +import XCTest +@testable import SwiftDashSDK + +/// Regression coverage for the shielded-sync generation guard in +/// `PlatformWalletManager.handleShieldedSyncCompleted(_:generation:)`. +/// +/// The guard exists because a shielded-sync completion event is snapshotted +/// with the current generation on the FFI callback thread, then re-dispatched +/// onto the main actor. A `stop`/`clear` bumps the generation counter, so a +/// trailing event that the main actor delivers *after* stop/clear must be +/// dropped — its snapshot predates the bump. Crucially, a same-turn restart +/// must NOT re-open the gate, because the counter is monotonic and never reset. +/// +/// `handleShieldedSyncCompleted` does not touch the FFI handle — it only reads +/// `shieldedSyncGeneration` and assigns `lastShieldedSyncEvent` — so a bare, +/// unconfigured `PlatformWalletManager()` is safe to drive directly. +/// +/// `PlatformWalletManager` and `handleShieldedSyncCompleted` are main-actor +/// isolated, so the whole suite runs on the main actor. +@MainActor +final class ShieldedSyncGenerationTests: XCTestCase { + + /// Build a deterministic event distinguishable by its timestamp. + private func makeEvent(syncUnixSeconds: UInt64) -> ShieldedSyncEvent { + ShieldedSyncEvent(syncUnixSeconds: syncUnixSeconds, walletResults: []) + } + + /// A completion captured under generation N is dropped once the counter is + /// bumped to N+1 before delivery: `lastShieldedSyncEvent` stays nil. + func testStaleCompletionIsDroppedAfterGenerationBump() { + let manager = PlatformWalletManager() + XCTAssertNil(manager.lastShieldedSyncEvent, "fresh manager has no event") + + // Snapshot the generation at "enqueue" time, the way the FFI callback + // thread does, then advance it (as stop/clear would) before delivery. + let capturedGeneration = manager.shieldedSyncGeneration.current() + manager.shieldedSyncGeneration.bump() + + let staleEvent = makeEvent(syncUnixSeconds: 1_000) + manager.handleShieldedSyncCompleted(staleEvent, generation: capturedGeneration) + + XCTAssertNil( + manager.lastShieldedSyncEvent, + "an event captured under a superseded generation must be dropped" + ) + } + + /// A completion captured under the CURRENT generation is published: + /// `lastShieldedSyncEvent` is set to that exact event. + func testCurrentGenerationCompletionIsPublished() { + let manager = PlatformWalletManager() + + let currentGeneration = manager.shieldedSyncGeneration.current() + let event = makeEvent(syncUnixSeconds: 2_000) + manager.handleShieldedSyncCompleted(event, generation: currentGeneration) + + XCTAssertEqual( + manager.lastShieldedSyncEvent?.syncUnixSeconds, + event.syncUnixSeconds, + "an event captured under the current generation must be published" + ) + } + + /// The exact race the guard closes: a stale event captured under the old + /// generation is dropped even when a same-turn restart follows, and a + /// later event captured under the (unchanged) current generation is still + /// published. Proves `bump()` — not any gate reset — is what invalidates. + func testBumpInvalidatesStaleButNotSubsequentCurrentCompletion() { + let manager = PlatformWalletManager() + + // 1. Capture under the pre-stop generation, then bump (stop/clear). + let staleGeneration = manager.shieldedSyncGeneration.current() + manager.shieldedSyncGeneration.bump() + + // 2. A restart does NOT reset the counter — capture the post-bump + // generation that a fresh run's events would carry. + let liveGeneration = manager.shieldedSyncGeneration.current() + XCTAssertNotEqual( + staleGeneration, + liveGeneration, + "bump must advance the generation so the stale snapshot no longer matches" + ) + + // 3. The trailing stale event (old snapshot) is delivered and dropped. + let staleEvent = makeEvent(syncUnixSeconds: 3_000) + manager.handleShieldedSyncCompleted(staleEvent, generation: staleGeneration) + XCTAssertNil( + manager.lastShieldedSyncEvent, + "the stale completion must not leak into the restarted run" + ) + + // 4. The new run's event (current snapshot) is delivered and published. + let liveEvent = makeEvent(syncUnixSeconds: 4_000) + manager.handleShieldedSyncCompleted(liveEvent, generation: liveGeneration) + XCTAssertEqual( + manager.lastShieldedSyncEvent?.syncUnixSeconds, + liveEvent.syncUnixSeconds, + "the restarted run's completion must be published" + ) + } + + /// A current-generation event followed by a stale (pre-bump) event keeps + /// the first event in place: a late straggler must not clobber valid state. + func testStaleCompletionDoesNotOverwriteAlreadyPublishedEvent() { + let manager = PlatformWalletManager() + + // Publish a valid event under the current generation. + let staleGeneration = manager.shieldedSyncGeneration.current() + let publishedEvent = makeEvent(syncUnixSeconds: 5_000) + manager.handleShieldedSyncCompleted(publishedEvent, generation: staleGeneration) + XCTAssertEqual(manager.lastShieldedSyncEvent?.syncUnixSeconds, publishedEvent.syncUnixSeconds) + + // Bump (stop/clear), then deliver a straggler carrying the old snapshot. + manager.shieldedSyncGeneration.bump() + let straggler = makeEvent(syncUnixSeconds: 6_000) + manager.handleShieldedSyncCompleted(straggler, generation: staleGeneration) + + XCTAssertEqual( + manager.lastShieldedSyncEvent?.syncUnixSeconds, + publishedEvent.syncUnixSeconds, + "a superseded straggler must not overwrite the last valid event" + ) + } +}