From 07421abc64ef74739de6d12347371e62234987f9 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 21 May 2026 22:09:47 +0700 Subject: [PATCH 1/2] fix(swift-sdk): tie shielded completion suppression to a sync generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the boolean `suppressShieldedCompletionEvents` gate with a lock-guarded monotonic generation counter. The boolean was bypassable: a stop (set flag) followed by a restart (clear flag) in the same @MainActor turn re-opened the gate before the stale, already-enqueued completion task ran, leaking the previous run's completion event into the new run. The FFI completion callback now snapshots the generation on the callback thread before enqueueing onto the main actor; stop/clear bump the generation after the Rust drain returns; the main-actor handler drops any event whose snapshot no longer matches the current generation. Start / sync-now no longer reset anything — new events carry the current generation and pass the guard, while a trailing event from a prior run still carries the older generation and is dropped, even on a same-turn restart. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PlatformWalletManager.swift | 46 +++++++++++------ .../PlatformWalletManagerShieldedSync.swift | 49 ++++++++++++------- 2 files changed, 62 insertions(+), 33 deletions(-) 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) } } From a70eb9d2edb4f1e18610368235c4463343d802e6 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Mon, 25 May 2026 14:14:27 +0700 Subject: [PATCH 2/2] test(swift-sdk): cover shielded completion generation guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regression coverage for the generation-gated suppression in PlatformWalletManager.handleShieldedSyncCompleted(_:generation:). Asserts a completion captured under a superseded generation is dropped (including on a same-turn stop→restart, where a boolean gate would leak the stale event), a current-generation completion is published, and a late straggler does not clobber an already-published event. Lives in the SwiftExampleAppTests Xcode target — the only test target on this repo that links the real SwiftDashSDK (the SwiftTests SPM package depends on a C mock, not the SDK). Verified locally via xcodebuild test (4 cases pass on the iOS Simulator); not currently CI-enforced since the Swift CI job is build-only. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ShieldedSyncGenerationTests.swift | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedSyncGenerationTests.swift 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" + ) + } +}