From bc4989c5cf6ad8243f407a26151347a5cd57c27d Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:38:42 +0300 Subject: [PATCH 1/2] fix(swift-sdk): gate the ordered bring-up on the seed actually owning the wallet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startWalletSubsystems` built its own `MnemonicResolver` and `KeychainSigner` and did no binding check, so it did not inherit the wrong-seed gate `unlockWalletFromKeychain` applies. On a wallet whose identity is already known it skips discovery and goes straight to the contact sync and drain — deriving DIP-15 contact accounts from a mnemonic the wallet has already rejected. That damage does not heal. `register_contact_account` keys its existence check on `(index, owner, friend)` and never on the xpub, and unlike the external-account side there is no rotation sweep for receiving accounts, so a wrong xpub is written once and every later correct-seed pass no-ops over it. The wallet watches addresses nobody pays to, with no symptom beyond payments that never arrive. The two other drain branches are narrower: a wrong-seed ECDH product fails `RegisterExternal`'s xpub format parse and the rotation sweep rebuilds it, and the auto-accept leg cannot broadcast at all because `KeychainSigner` binds the on-chain public key and rejects a wrong-seed key before signing. **Why a new API rather than calling the unlock.** `unlockWalletFromKeychain` is not a verification primitive: when the queue is non-empty it also starts a detached drain and sets the `draining` guard. Using it as a preflight would have this call race a second drain over a second snapshot — duplicated network and ECDH work, and competing channel-broken and auto-accept writes, which is exactly what that guard exists to prevent. So step 1 is now `verifySeedBinding`, public and side-effect-free, and the unlock is that call plus the drain. No behaviour change for existing unlock callers. **Why in the SDK rather than in each host.** A host that has to remember to gate this call will eventually forget, and the published `seedMismatch` flag cannot serve as that gate: hosts commonly kick the unlock off asynchronously (iOS schedules it in a detached task and returns), so the flag races the very call it is supposed to guard. Verification performed inside the call is ordered with respect to the work it protects; a flag read outside it is not. Cost is a string comparison on the common path — the verify is marker-cached, and a match never touches the Keychain. Reported on dashpay/dashwallet-ios#961, whose call site this unblocks. Verified: `xcodebuild -scheme SwiftDashSDK -sdk iphonesimulator` clean. --- .../PlatformWalletManager.swift | 72 +++++++++++++++---- .../PlatformWalletManagerStartup.swift | 24 ++++++- 2 files changed, 81 insertions(+), 15 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 4a0f658412..c399c090f1 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -671,17 +671,10 @@ public class PlatformWalletManager: ObservableObject { /// through the Keychain-backed resolver per-operation. This unlock does /// two things, both through a resolver (the seed never becomes resident): /// - /// 1. **Verify** the resolved seed binds to this wallet — - /// `platform_wallet_verify_seed_binds_to_wallet_cached` derives the - /// BIP44 account-0 xpub through the resolver and compares it to the - /// persisted one. A mis-mapped Keychain slot derives a different xpub - /// and the call throws, so a wrong seed never signs for this wallet. - /// The outcome is launch-invariant while the mnemonic Keychain item - /// is untouched, so the first success persists a marker (the xpub - /// bound to the item's identity stamp) on the wallet row and later - /// launches skip the resolver. Rust re-runs the full check when the - /// marker no longer matches — wallet re-import, or any rewrite of - /// the mnemonic Keychain item (which changes the stamp). + /// 1. **Verify** the resolved seed binds to this wallet — delegated in + /// full to [`verifySeedBinding`](Self/verifySeedBinding(_:)), which is + /// also the entry point for callers that need the gate WITHOUT the + /// drain below. /// 2. **Drain** (in the background) any contact-crypto deferred while /// the wallet was seedless — `platform_wallet_drain_pending_contact_crypto`. /// The drain re-fetches + decrypts over the network, so it runs in a @@ -701,8 +694,46 @@ public class PlatformWalletManager: ObservableObject { /// throwing. /// - Throws: `PlatformWalletError` if the verify FFI fails (e.g. the /// resolved seed does not bind — a mis-mapped Keychain slot). + /// Whether a wallet has a Keychain seed at all, once the binding holds. + public enum SeedBindingCheck: Sendable, Equatable { + /// A mnemonic is stored for this wallet and it derives the wallet's + /// persisted BIP44 account-0 xpub. Signing for this wallet is sound. + case verified + /// No mnemonic is stored — a genuine watch-only wallet, imported by + /// xpub. Nothing to contradict and nothing that can sign. + case watchOnly + } + + /// Verify that the Keychain seed for `wallet` actually owns it — and do + /// nothing else. + /// + /// This is step 1 of [`unlockWalletFromKeychain`](Self/unlockWalletFromKeychain(_:)) + /// on its own. It exists because that method is not a verification + /// primitive: it also schedules a background drain of the deferred + /// contact crypto. A caller that needs the gate and not the drain — one + /// about to run its own mnemonic-derived work, such as + /// [`startWalletSubsystems`](Self/startWalletSubsystems(wallet:budget:gapLimit:storage:)) + /// — would otherwise have to launch a competing drain to ask the + /// question. Two drains over two snapshots duplicate the network and + /// ECDH work and race each other's channel-broken and auto-accept + /// writes, which is why the unlock path already refuses to stack them. + /// + /// Marker-cached exactly as the unlock is: the outcome is a pure function + /// of (mnemonic, network) against a fixed persisted xpub, so a match + /// costs a string comparison and never touches the Keychain. A rewritten + /// mnemonic item changes its stamp and forces the full check again. + /// + /// Publishes `dashPayUnlockStatus[walletId].seedMismatch` from the + /// result, so a caller may read it afterwards — but callers that must not + /// race the publisher should use the return value, which is ordered with + /// respect to the work it guards. + /// + /// - Returns: `.verified` when the stored seed binds, `.watchOnly` when + /// there is no stored mnemonic to bind. + /// - Throws: `PlatformWalletError` when the seed does not bind (a + /// mis-mapped Keychain slot) or the verify FFI otherwise fails. @discardableResult - public func unlockWalletFromKeychain(_ wallet: ManagedPlatformWallet) throws -> Bool { + public func verifySeedBinding(_ wallet: ManagedPlatformWallet) throws -> SeedBindingCheck { try ensureConfigured() let walletId = wallet.walletId guard walletId.count == 32 else { @@ -716,7 +747,7 @@ public class PlatformWalletManager: ObservableObject { // check; the plaintext is never materialized in Swift. let walletStorage = WalletStorage() guard walletStorage.hasMnemonic(for: walletId) else { - return false + return .watchOnly } let walletHandle = wallet.handle @@ -808,6 +839,21 @@ public class PlatformWalletManager: ObservableObject { throw error } + return .verified + } + + @discardableResult + public func unlockWalletFromKeychain(_ wallet: ManagedPlatformWallet) throws -> Bool { + // Step 1 in full, side-effect-free. A watch-only wallet has nothing + // to unlock and nothing to drain for. + guard try verifySeedBinding(wallet) == .verified else { return false } + + let walletId = wallet.walletId + let walletHandle = wallet.handle + // Resolver-backed signer for the drain: the mnemonic is fetched from + // the Keychain inside the resolver vtable Rust-side; no resident seed. + let coreSigner = MnemonicResolver() + // Heal pre-breadcrumb identity keys so they sign via the resolver // (derive-sign-destroy) rather than the stored scalar. Idempotent and // Keychain-sourced; runs once the seed is confirmed present for this diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index 6bc7ef94aa..141721956d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -97,8 +97,11 @@ extension PlatformWalletManager { /// /// - Throws: only for a malformed request — an unconfigured manager, a /// malformed wallet id, an unknown wallet, or a `budget` outside the - /// supported range. An unreachable Platform, a failed sync pass and an - /// unfinished drain are all reported through the returned status. + /// supported range — or when the stored seed does not bind to this + /// wallet, which is a malformed pairing of the two and the one condition + /// under which none of this may run at all. An unreachable Platform, a + /// failed sync pass and an unfinished drain are all reported through the + /// returned status instead, because Core sync must start regardless. /// /// # Key material /// @@ -123,6 +126,23 @@ extension PlatformWalletManager { ) } + // Nothing below may run on a seed that does not own this wallet. + // Everything this call does with key material is unauthenticated — + // the resolver derives whatever the Keychain holds, and + // `register_contact_account` keys its existence check on the contact + // pair rather than the xpub, so a wrong receiving xpub is written + // once and no later correct-seed pass revisits it. The wallet then + // watches addresses nobody pays to, with no symptom but payments that + // never arrive. + // + // Enforced here rather than left to the host: a client that has to + // remember to gate this call is a client that will eventually forget, + // and the published `seedMismatch` flag cannot be that gate anyway — + // hosts commonly kick the unlock off asynchronously, so the flag + // races this call. The verify is marker-cached, so the common path + // costs a string comparison. + try verifySeedBinding(wallet) + let handle = self.handle // Only a definitive "no such item" means watch-only. A lookup that // failed for another reason — locked device, denied access — must still From 2e3a62c6ac8ca6dcf6ba20b8c8d4e401bdb879b2 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:49:01 +0300 Subject: [PATCH 2/2] fix(swift-sdk): verify against the same store the caller will derive from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review on #4368. **The gate could check a different Keychain than the work uses.** `startWalletSubsystems` takes a `storage` parameter and builds its resolver from it, but the verify I added constructed its own default `WalletStorage`. Where those differ — tests, or any host that injects a store — verification would approve one mnemonic while the derivation ran on another, which is the exact failure this gate exists to prevent, just harder to see. The check now takes the store as a parameter and passes it to the resolver too; the public no-argument form is the default-store convenience and is what the unlock path still uses. **A removed mnemonic left the banner up.** The watch-only early return came before `setDashPaySeedMismatch(walletId, false)`, so a wallet whose seed failed to bind and whose Keychain item was then deleted kept publishing a mismatch for a seed that no longer exists. No mnemonic is not a mismatch. Verified: `xcodebuild -scheme SwiftDashSDK -sdk iphonesimulator` clean. --- .../PlatformWalletManager.swift | 28 ++++++++++++++++--- .../PlatformWalletManagerStartup.swift | 6 +++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index c399c090f1..c3e3376482 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -734,6 +734,22 @@ public class PlatformWalletManager: ObservableObject { /// mis-mapped Keychain slot) or the verify FFI otherwise fails. @discardableResult public func verifySeedBinding(_ wallet: ManagedPlatformWallet) throws -> SeedBindingCheck { + try verifySeedBinding(wallet, storage: WalletStorage()) + } + + /// [`verifySeedBinding`](Self/verifySeedBinding(_:)) against a specific + /// `WalletStorage`. + /// + /// A caller that resolves the mnemonic from one store must be verified + /// against that same store, or the check answers a question about a + /// different Keychain than the one the work will read — approving one + /// mnemonic while the derivation uses another. `startWalletSubsystems` + /// takes a `storage` parameter for exactly this reason and passes it here. + @discardableResult + func verifySeedBinding( + _ wallet: ManagedPlatformWallet, + storage walletStorage: WalletStorage + ) throws -> SeedBindingCheck { try ensureConfigured() let walletId = wallet.walletId guard walletId.count == 32 else { @@ -745,15 +761,19 @@ public class PlatformWalletManager: ObservableObject { // A genuine watch-only wallet (imported by xpub, never holding a // seed) has no Keychain mnemonic — stays watch-only. Existence-only // check; the plaintext is never materialized in Swift. - let walletStorage = WalletStorage() guard walletStorage.hasMnemonic(for: walletId) else { + // No mnemonic is no longer a mismatch: a wallet whose seed failed + // to bind and whose Keychain item was then removed must not keep + // publishing the banner for a seed that is no longer there. + setDashPaySeedMismatch(walletId, false) return .watchOnly } let walletHandle = wallet.handle - // Resolver-backed signer: the mnemonic is fetched from the Keychain - // inside the resolver vtable Rust-side; no resident seed. - let coreSigner = MnemonicResolver() + // Resolver-backed signer, over the SAME store the check above read and + // the caller's work will read: the mnemonic is fetched from the + // Keychain inside the resolver vtable Rust-side; no resident seed. + let coreSigner = MnemonicResolver(storage: walletStorage) // Wrong-seed / wrong-wallet gate, marker-cached: the check is a pure // function of (mnemonic, network) against the wallet's persisted diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index 141721956d..9e65f891cb 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -141,7 +141,11 @@ extension PlatformWalletManager { // hosts commonly kick the unlock off asynchronously, so the flag // races this call. The verify is marker-cached, so the common path // costs a string comparison. - try verifySeedBinding(wallet) + // + // Against `storage`, not a default one: the resolver below reads that + // store, and verifying a different Keychain than the work will use + // would approve one mnemonic while another derives the accounts. + try verifySeedBinding(wallet, storage: storage) let handle = self.handle // Only a definitive "no such item" means watch-only. A lookup that