Bug: WalletModel.continuations dictionary accessed without synchronization

[alltheseas]

Problem

The continuations dictionary in WalletModel is accessed from network callbacks (handle_nwc_response) and UI thread (waitForResponse) without synchronization, causing data races and potential crashes.

Location

damus/Features/Wallet/Models/WalletModel.swift lines 46, 212-240

class WalletModel: ObservableObject {
    private var continuations: [NoteId: CheckedContinuation<WalletConnect.Response.Result, any Error>] = [:]
    
    func handle_nwc_response(response: WalletConnect.FullWalletResponse) {
        // Called from background thread (network callback)
        self.resume(request: response.req_id, with: result)  // RACE
    }
    
    func waitForResponse(for requestId: NoteId, ...) async throws -> ... {
        return try await withCheckedThrowingContinuation({ continuation in
            self.continuations[requestId] = continuation  // RACE
        })
    }
}

Root Cause

Swift dictionaries are not thread-safe. Concurrent read/write from different threads causes undefined behavior.

Impact

• Crash: "Invalid memory access" or EXC_BAD_ACCESS
• Lost wallet responses
• Continuation resumed twice

Suggested Fix

Either:

1. Add NSLock protection around all continuations access
2. Convert WalletModel to an actor (preferred per Swift concurrency guidelines)

Test Plan

• Test multiple concurrent wallet requests
• Test response arriving during request setup
• Run with ThreadSanitizer

Changelog-Fixed: Fix race condition in WalletModel continuation handling

[linear]

IOS-2282

[alltheseas]

Crash Confirmed in Integration Testing

During WalletModelIntegrationTests, concurrent calls to waitForResponse() caused consistent crashes:

NSInvalidArgumentException: -[__NSCFNumber count]: unrecognized selector sent to instance 0x8000000000000000

Stack trace shows:

• WalletModel.waitForResponse at line 214: self.continuations[requestId] = continuation
• Multiple Tasks writing to the dictionary concurrently

Test workaround:
Tests were modified to run sequentially (testSequentialTimeouts instead of testConcurrentTimeouts) to avoid triggering the crash, but this confirms the race condition is real and causes crashes in practice.

Reproduction:

// This crashes due to concurrent dictionary access:
for _ in 0..<5 {
    Task {
        let requestId = NoteId(Data((0..<32).map { _ in UInt8.random(in: 0...255) }))
        _ = try await wallet.waitForResponse(for: requestId, timeout: .milliseconds(100))
    }
}