fix(sdk): wallet-flow network fixes for SwiftExampleApp #3772
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ struct CreateWalletView: View { | |
| @Environment(\.dismiss) var dismiss | ||
| @Environment(\.modelContext) private var modelContext | ||
| @EnvironmentObject var walletManager: PlatformWalletManager | ||
| @EnvironmentObject var walletManagerStore: WalletManagerStore | ||
| @EnvironmentObject var platformState: AppState | ||
|
|
||
| @State private var walletLabel: String = "" | ||
|
|
@@ -313,94 +314,102 @@ struct CreateWalletView: View { | |
| print("PIN length: \(walletPin.count)") | ||
| print("Import option enabled: \(showImportOption)") | ||
|
|
||
| let selectedNetworks: [Network] = [ | ||
| createForMainnet ? Network.mainnet : nil, | ||
| createForTestnet ? Network.testnet : nil, | ||
| (createForDevnet && shouldShowDevnet) ? Network.devnet : nil, | ||
| (createForRegtest && shouldShowRegtest) ? Network.regtest : nil, | ||
| ].compactMap { $0 } | ||
|
|
||
| guard !selectedNetworks.isEmpty else { | ||
| struct MissingNetwork: LocalizedError { | ||
| var errorDescription: String? { "No network selected" } | ||
| } | ||
| throw MissingNetwork() | ||
| } | ||
|
|
||
| // Create the wallet in EVERY ticked network. Each | ||
| // network has its own `PlatformWalletManager` (the | ||
| // Rust manager is network-locked at construction and | ||
| // stamps its own network onto the wallet), so routing | ||
| // through the active manager alone would ignore the | ||
| // passed `network`. `backgroundManager(for:)` returns | ||
| // the warm cached manager for the active network and | ||
| // builds one on demand for the others. `walletId` is | ||
| // network-independent, so the Keychain mnemonic + | ||
| // metadata are written once and the `isImported` flag | ||
| // is stamped on every per-network row. | ||
| try await MainActor.run { | ||
| var createdWalletId: Data? | ||
| for net in selectedNetworks { | ||
| do { | ||
| let mgr = try walletManagerStore.backgroundManager(for: net) | ||
| let managed = try mgr.createWallet( | ||
| mnemonic: mnemonicPhrase, | ||
| network: net, | ||
| name: walletLabel | ||
| ) | ||
| createdWalletId = managed.walletId | ||
| } catch { | ||
| // An "already exists" throw for one network | ||
| // (wallet was previously created there) is | ||
| // non-fatal — keep creating the others. | ||
| SDKLogger.error( | ||
| "Wallet creation skipped for \(net.displayName): \(error.localizedDescription)" | ||
| ) | ||
| } | ||
|
Comment on lines
+365
to
+386
Collaborator
There was a problem hiding this comment. 🟡 Suggestion: Brittle substring match on "already exists" to classify a benign error The branch classifying the per-network failure as benign relies on a case-insensitive substring search for source: ['claude']
Member
Author
There was a problem hiding this comment. Agreed it's brittle. Acknowledged but not changing in this PR — the proper fix is a typed FFI error code / dedicated
Comment on lines
+364
to
+386
Collaborator
There was a problem hiding this comment. 🟡 Suggestion: Benign 'wallet already exists' classification still relies on a substring match against a Rust display string, now duplicated into WalletDetailView Carried-forward from cee4b2c — re-validated and STILL VALID. The per-network create loop classifies a benign duplicate by This PR also propagates the same heuristic to source: ['claude', 'codex']
Member
Author
There was a problem hiding this comment. Same finding as the existing thread on this file (#discussion_r3331111688) — acknowledged and deferred. The robust fix is a typed
Comment on lines
+365
to
+386
Collaborator
There was a problem hiding this comment. 🟡 Suggestion: Duplicate-wallet classification still relies on substring-matching a Rust display string Carried forward from prior reviews (cee4b2c, 815751a) and still valid at 14d353e. The per-network create loop classifies a benign duplicate by source: ['claude', 'codex']
Member
Author
There was a problem hiding this comment. Acknowledged, deferring (same as the existing thread on this file). The robust fix is a typed |
||
| } | ||
|
|
||
| guard let walletId = createdWalletId else { | ||
| struct AllNetworksFailed: LocalizedError { | ||
| var errorDescription: String? { | ||
| "Wallet could not be created on any selected network." | ||
| } | ||
| } | ||
| throw AllNetworksFailed() | ||
| } | ||
|
|
||
| // Persist the mnemonic in the iOS Keychain keyed | ||
| // by walletId (network-independent) so the | ||
| // recovery flow can enumerate wallets on launch. | ||
| // Best-effort — failure here doesn't block. | ||
| let storage = WalletStorage() | ||
| do { | ||
| try storage.storeMnemonic(mnemonicPhrase, for: walletId) | ||
| } catch { | ||
| SDKLogger.error( | ||
| "Failed to persist mnemonic to keychain: \(error.localizedDescription)" | ||
| ) | ||
| } | ||
|
|
||
| // Stamp `isImported` on every per-network row for | ||
| // this walletId. The persister callbacks run | ||
| // synchronously from `createWallet` via the | ||
| // background contexts; autosave propagates the | ||
| // rows into the main context before this fetch. | ||
| let descriptor = FetchDescriptor<PersistentWallet>( | ||
| predicate: PersistentWallet.predicate(walletId: walletId) | ||
| ) | ||
| let rows = (try? modelContext.fetch(descriptor)) ?? [] | ||
| for row in rows { | ||
| row.isImported = showImportOption | ||
| } | ||
| if !rows.isEmpty { | ||
| try? modelContext.save() | ||
| } | ||
|
|
||
| // Mirror the name + ticked networks + birth height | ||
| // into the keychain alongside the mnemonic so an | ||
| // orphan-recovery after a wipe restores the | ||
| // original label / networks / birth height. | ||
| do { | ||
| let metadata = WalletKeychainMetadata( | ||
| name: walletLabel, | ||
| walletDescription: nil, | ||
| networks: selectedNetworks.map { $0.networkName }, | ||
| birthHeight: rows.first?.birthHeight | ||
| ) | ||
| try storage.setMetadata(metadata, for: walletId) | ||
| } catch { | ||
| SDKLogger.error( | ||
| "Failed to persist wallet metadata to keychain: \(error.localizedDescription)" | ||
|
|
@@ -409,14 +418,19 @@ struct CreateWalletView: View { | |
| dismiss() | ||
| } | ||
|
|
||
| print("=== WALLET CREATION SUCCESS - networks: \(selectedNetworks.map { $0.displayName }) ===") | ||
| } catch { | ||
| print("=== WALLET CREATION ERROR ===") | ||
| print("Error: \(error)") | ||
|
|
||
| await MainActor.run { | ||
| self.error = error | ||
| isCreating = false | ||
| // Pop the pushed `SeedBackupView` so the error alert | ||
| // (bound to this view) is actually visible — otherwise | ||
| // the backup screen sits on top with its submit button | ||
| // stuck disabled and no feedback. | ||
| showBackupScreen = false | ||
| } | ||
| } | ||
| } | ||
|
|
||
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']