fix(sdk): wallet-flow network fixes for SwiftExampleApp #3772
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,12 +401,12 @@ public class PlatformWalletPersistenceHandler { | |
| /// stale post-deletion callbacks can't resurrect a wiped wallet. | ||
| private func ensureWalletRecord(walletId: Data) -> PersistentWallet { | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
| if let existing = try? backgroundContext.fetch(descriptor).first { | ||
| return existing | ||
| } | ||
| let record = PersistentWallet(walletId: walletId, network: self.network) | ||
| backgroundContext.insert(record) | ||
| return record | ||
| } | ||
|
|
@@ -415,11 +415,27 @@ public class PlatformWalletPersistenceHandler { | |
| /// when no row exists. | ||
| private func findWalletRecord(walletId: Data) -> PersistentWallet? { | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
| return try? backgroundContext.fetch(descriptor).first | ||
| } | ||
|
|
||
| /// Predicate matching the `PersistentWallet` row owned by THIS | ||
| /// handler. A handler is constructed per-network, so when | ||
| /// `self.network` is set we scope to `(walletId, networkRaw)` — | ||
| /// otherwise the mainnet handler would find and overwrite the | ||
| /// devnet row (and vice versa) now that the same `walletId` can | ||
| /// have one row per network. When `self.network` is `nil` (the | ||
| /// advanced `configure(sdkPointer:network:nil)` path) we fall | ||
| /// back to walletId-only matching to preserve that behaviour. | ||
| private func walletRecordPredicate(walletId: Data) -> Predicate<PersistentWallet> { | ||
| if let network = self.network { | ||
| let networkRaw = network.rawValue | ||
| return #Predicate { $0.walletId == walletId && $0.networkRaw == networkRaw } | ||
| } | ||
| return #Predicate { $0.walletId == walletId } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| /// Look up a `PersistentWallet` to hang on | ||
| /// `PersistentIdentity.wallet`. Non-creating — returns `nil` if | ||
| /// no row exists (an identity may arrive before its owning | ||
|
|
@@ -429,7 +445,7 @@ public class PlatformWalletPersistenceHandler { | |
| private func fetchWalletForLink(walletId: Data?) -> PersistentWallet? { | ||
| guard let walletId else { return nil } | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
| return try? backgroundContext.fetch(descriptor).first | ||
| } | ||
|
|
@@ -1795,9 +1811,13 @@ public class PlatformWalletPersistenceHandler { | |
| // 1. Resolve the wallet's network from SwiftData. We need it | ||
| // to feed `KeyDerivation.getIdentityAuthenticationPath` | ||
| // so the path chooses the right `coin_type` (mainnet vs | ||
| // testnet). Scope to THIS handler's network via | ||
| // `walletRecordPredicate` — the same `walletId` can now have | ||
| // a row per network, and a bare walletId-only fetch could | ||
| // resolve to a sibling network's row and derive the key on | ||
| // the wrong chain (unusable on-chain). | ||
| let walletDescriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
|
Comment on lines
+1814
to
+1820
Contributor
There was a problem hiding this comment. 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift Move identity-key derivation behind a Rust/platform-wallet FFI entry point. This method still pulls the mnemonic from Keychain, derives the seed, builds the DIP-9 path, and derives the private key in Swift. The new network-scoped lookup helps correctness, but the full mnemonic → seed → path → key pipeline is still implemented on the Swift side instead of persisting only Rust-returned key material. As per coding guidelines, "Do not build derivation paths in Swift", "Do not orchestrate multi-step derivation pipelines in Swift (mnemonic → seed → path → key → store)", and "Identity key derivation and any action using a derived key must route through platform-wallet, not be re-implemented in Swift". 🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Acknowledged, not addressed in this PR. You're right that the mnemonic→seed→DIP-9 path→key pipeline belongs in platform-wallet per the swift-sdk CLAUDE.md rules. This code predates this PR (the PR only changed the wallet-row lookup from a bare predicate to
Contributor
There was a problem hiding this comment.
✏️ Learnings added
🧠 Learnings used |
||
| ) | ||
| guard | ||
| let persistentWallet = try? backgroundContext.fetch(walletDescriptor).first | ||
|
|
@@ -2594,11 +2614,25 @@ public class PlatformWalletPersistenceHandler { | |
| } | ||
| } | ||
|
|
||
| /// Count `PersistentWallet` rows for `walletId` across ALL | ||
| /// networks (deliberately ignores `self.network`). The mnemonic / | ||
| /// metadata in the Keychain are shared by every network's row, so | ||
| /// `deleteWallet` consults this after wiping its own network's row | ||
| /// to decide whether the shared Keychain material can be purged. | ||
| public func walletRowCountAcrossNetworks(walletId: Data) throws -> Int { | ||
| try onQueue { | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: PersistentWallet.predicate(walletId: walletId) | ||
| ) | ||
| return try backgroundContext.fetchCount(descriptor) | ||
| } | ||
| } | ||
|
|
||
| public func identityIdsForWallet(walletId: Data) throws -> [Data] { | ||
| try onQueue { | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
| guard let walletRow = try backgroundContext.fetch(descriptor).first else { | ||
| return [] | ||
| } | ||
|
|
@@ -2611,7 +2645,7 @@ public class PlatformWalletPersistenceHandler { | |
| try onQueue { | ||
| do { | ||
| let walletDescriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| let walletRow = try backgroundContext.fetch(walletDescriptor).first | ||
| let walletNetwork = walletRow?.network | ||
|
|
@@ -2689,32 +2723,63 @@ public class PlatformWalletPersistenceHandler { | |
| try backgroundContext.save() | ||
| } | ||
|
|
||
| // The txo / pending-input / asset-lock tables are keyed | ||
| // by the network-independent walletId (same mnemonic → | ||
| // same id on every network) and carry no network column, | ||
| // so their rows are shared by every network this wallet | ||
| // lives on. Only wipe them when this is the wallet's LAST | ||
| // remaining per-network row — otherwise deleting the | ||
| // wallet from one network would erase a sibling network's | ||
| // cached UTXOs / pending inputs / asset-lock state. | ||
| // (The walletRow itself, deleted below, IS network-scoped | ||
| // via `walletRecordPredicate`.) Counted before walletRow | ||
| // is removed, so `<= 1` means "this is the last one". | ||
| // Guard on `walletRow != nil`: if this handler doesn't | ||
| // own a row for `walletId` (asked to delete a wallet it | ||
| // doesn't have), a sibling network's row can still make | ||
| // the cross-network count 1 — which would wrongly read | ||
| // as "last row" and wipe the shared child tables out | ||
| // from under that other network. No owned row → never | ||
| // treat it as the last one. | ||
| let isLastNetworkRow: Bool | ||
| if walletRow != nil { | ||
| let siblingDescriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: #Predicate<PersistentWallet> { $0.walletId == walletId } | ||
| ) | ||
| isLastNetworkRow = | ||
| ((try? backgroundContext.fetchCount(siblingDescriptor)) ?? 0) <= 1 | ||
| } else { | ||
| isLastNetworkRow = false | ||
| } | ||
|
|
||
| if isLastNetworkRow { | ||
| let txoDescriptor = FetchDescriptor<PersistentTxo>( | ||
| predicate: #Predicate<PersistentTxo> { $0.walletId == walletId } | ||
| ) | ||
| for row in try backgroundContext.fetch(txoDescriptor) { | ||
| backgroundContext.delete(row) | ||
| } | ||
|
|
||
| let pendingDescriptor = FetchDescriptor<PersistentPendingInput>( | ||
| predicate: #Predicate<PersistentPendingInput> { $0.walletId == walletId } | ||
| ) | ||
| for row in try backgroundContext.fetch(pendingDescriptor) { | ||
| backgroundContext.delete(row) | ||
| } | ||
|
|
||
| // `loadCachedAssetLocksOnQueue` rehydrates these rows on | ||
| // the wallet-load path back into the Rust-side | ||
| // `unused_asset_locks` map so an in-flight registration | ||
| // can resume across an app kill. Without this cleanup, | ||
| // delete-then-reimport of the same wallet would | ||
| // resurrect stale Pending / Resumable asset-lock state | ||
| // that the user thought they had wiped. | ||
| let assetLockDescriptor = FetchDescriptor<PersistentAssetLock>( | ||
| predicate: #Predicate<PersistentAssetLock> { $0.walletId == walletId } | ||
| ) | ||
| for row in try backgroundContext.fetch(assetLockDescriptor) { | ||
| backgroundContext.delete(row) | ||
| } | ||
| } | ||
|
|
||
| if let walletRow = walletRow { | ||
|
|
@@ -4029,8 +4094,14 @@ public class PlatformWalletPersistenceHandler { | |
| /// `PersistentWallet` row. Returns `nil` if the wallet row | ||
| /// doesn't exist or its network hasn't been resolved yet. | ||
| private func walletNetwork(walletId: Data) -> Network? { | ||
| // Scope to this handler's network when one is set so a mnemonic | ||
| // that lives on multiple networks resolves to the row for THIS | ||
| // manager's network — not an arbitrary sibling row that would | ||
| // mis-stamp persisted sync state / identity / token writes and | ||
| // feed the wrong coin type into key derivation. Falls back to | ||
| // walletId-only when no network is set (legacy / no-container). | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: walletRecordPredicate(walletId: walletId) | ||
| ) | ||
| guard let wallet = try? backgroundContext.fetch(descriptor).first else { | ||
| return nil | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💬 Nitpick:
predicate(walletId:network:)is over-specified now that walletId is globally uniqueWith the latest delta's invariant that
walletIdis network-scoped and globally unique on its own (the network byte is folded into the digest, and the#Uniqueconstraint on the model is now[\.walletId]), the composite predicate$0.walletId == walletId && $0.networkRaw == networkRawcan never narrow the result set further than the walletId clause alone, but it CAN spuriously return empty for rows whosenetworkRawisnil— the property's own doc-comment (lines 47–49) explicitly permits pre-persistWalletMetadatarows to havenetworkRaw == nil. This is a low-impact mismatch with the model rather than a correctness break, and is worth flagging only because the just-rewritten walletId doc-comment advertises that the network is already baked intowalletId— keeping a sibling lookup helper that still requires anetworkRawmatch invites confusion at future call sites. Consider either dropping the network clause from this predicate or removing the helper in favor ofpredicate(walletId:).source: ['claude']