Skip to content

fix(swift-example-app): route orphan recovery to per-network managers#3612

Merged
QuantumExplorer merged 1 commit intov3.1-devfrom
swift-example-app/cross-network-orphan-recovery
May 7, 2026
Merged

fix(swift-example-app): route orphan recovery to per-network managers#3612
QuantumExplorer merged 1 commit intov3.1-devfrom
swift-example-app/cross-network-orphan-recovery

Conversation

@QuantumExplorer
Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer commented May 7, 2026

Issue being fixed or feature implemented

When restoring orphan-mnemonic wallets after a reinstall, every recovered wallet was landing under the active network — so a regtest mnemonic recovered while the user was on Testnet ended up persisted with networkRaw = testnet, and the Local tab stayed stuck empty until the user manually switched networks and re-ran recovery.

Three issues stacked on top of each other to produce the observed UX:

  1. Wallet metadata stamped from the wrong manager. The Rust PlatformWalletManager is network-locked and register_wallet records wallet.network = self.sdk.network. ContentView.recoverWallet always called createWallet on the env-injected active walletManager, so the persisted row's network reflected the active manager — not the original network read from keychain metadata.
  2. Regtest SDK construction failed mid-session. OptionsView auto-disables useDockerSetup when leaving regtest. With the toggle off, SDK(network: .regtest) returned DAPI addresses not available for network: Regtest because regtest has no remote DAPI defaults on the Rust side — so even when recovery tried to route to the regtest manager, it couldn't build one.
  3. Cross-context propagation lag. Even when recovery did succeed, the new row landed in the target manager's background ModelContext and the launch-time main context's @Query consumers didn't observe it in time for the post-recovery isImported flush. The wallet only appeared in the correct tab after the next app launch when the main context was rebuilt fresh from disk.

What was done?

  • Per-network routing. Added WalletManagerStore.backgroundManager(for:) and a makeActive: Bool overload on activate(...). The recovery flow now resolves each orphan's original network from keychain metadata and calls createWallet on the manager for that network, lazy-building one (with its own SDK) without changing the user's currently visible network.
  • Regtest SDK construction. SDK(network:) now forces local DAPI for regtest unconditionally — the useDockerSetup toggle still controls non-regtest networks, but for regtest local DAPI is the only valid option, so building it on demand from any context now succeeds.
  • Pre-warm + retry for cross-context propagation. Bootstrap now scans keychain orphans, resolves their networks from metadata, and pre-builds per-network managers for any network whose target ≠ the active one. After createWallet, recoverWallet retries the mainContext.fetch up to 10× with a 50 ms yield between attempts so the cross-context merge has time to land before we move on; once the fetch finds the row, the existing row.isImported = true; modelContext.save() block fires and @Query re-evaluates immediately.

How Has This Been Tested?

Manual end-to-end on iOS Simulator (iPhone 17, iOS 26.3):

  • Reinstall scenario with 2 testnet + 2 regtest orphan mnemonics in keychain. Before the fix: all four landed under Testnet, Local tab stayed empty. After the fix: the testnet orphans appear under the Testnet tab and the regtest orphans appear under the Local tab on the same run, no app relaunch required.
  • Verified that the regtest orphans still recover correctly when useDockerSetup is off at the time of recovery (the new SDK constructor branch covers this).
  • xcodebuild -project SwiftExampleApp/SwiftExampleApp.xcodeproj -scheme SwiftExampleApp -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 17,arch=arm64' buildBUILD SUCCEEDED.

Breaking Changes

None. New APIs are additive (WalletManagerStore.backgroundManager(for:), activate(...makeActive:) overload with default-true preserves the existing call sites). The SDK(network:) change for regtest only affects a previously-failing path.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed regtest network configuration to ensure proper setup
    • Improved orphan wallet recovery with enhanced data synchronization and retry logic
  • Improvements

    • Enhanced wallet manager initialization to correctly handle multi-network wallet recovery
    • Optimized wallet environment handling for better data consistency across network switches

When restoring orphan-mnemonic wallets after a reinstall, every
recovered wallet was landing under the active network — so a regtest
mnemonic recovered from a Testnet active context ended up persisted as
testnet, with the user's Local tab stuck empty until they manually
switched and re-recovered.

Three issues stacked together:

1. The Rust `PlatformWalletManager` is network-locked and stamps
   `wallet.network = self.sdk.network` at registration time. The
   recovery flow always called `createWallet` on the env-injected
   active manager, so the wallet's persisted network reflected the
   wrong manager rather than the wallet's original network from
   keychain metadata. Fixed by adding `WalletManagerStore.backgroundManager(for:)`
   that lazy-builds a per-network manager (and SDK) without flipping
   the user-visible active network, and routing each orphan to the
   manager for its `metadata.resolvedNetworks.first`.

2. `SDK(network: .regtest)` was failing with `DAPI addresses not
   available for network: Regtest` whenever `useDockerSetup` had been
   auto-disabled by leaving regtest. Regtest has no remote DAPI on the
   Rust side, so the SDK constructor now always uses local DAPI for
   regtest regardless of the toggle.

3. After `createWallet`, the new row lived in the target manager's
   background `ModelContext`, but the launch-time main context didn't
   pick up the cross-context save in time for the post-recovery
   `isImported` flush, so `@Query` consumers never re-evaluated and
   the wallet only appeared after the next launch. Pre-warming
   per-network managers for orphan networks at bootstrap, plus a
   short retry on the `mainContext.fetch` after `createWallet`,
   makes the wallet appear in its correct tab on the same run.
@QuantumExplorer QuantumExplorer requested a review from shumkov as a code owner May 7, 2026 10:51
@github-actions github-actions Bot added this to the v3.1.0 milestone May 7, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR extends the wallet manager store with background per-network manager materialization while hardening regtest DAPI setup requirements. It modifies the app to recover orphan-network wallets using background managers with cross-context retry-fetch logic and pre-warms wallet managers for orphan networks during bootstrap.

Changes

Multi-Network Wallet Recovery with Background Manager Materialization

Layer / File(s) Summary
SDK Regtest Setup
packages/swift-sdk/Sources/SwiftDashSDK/SDK.swift
SDK.init(network:) now sets forceLocal to true when either network is .regtest or useDockerSetup toggle is enabled. Updated documentation explains regtest must always use local DAPI addresses.
Store API Extension
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/WalletManagerStore.swift
activate(network:sdk:) now accepts makeActive: Bool = true parameter, reusing cached per-network managers and conditionally updating activeManager only when makeActive is true. New public method backgroundManager(for:) materializes per-network managers without swapping active manager.
Wallet Recovery Flow
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/ContentView.swift
ContentView injects WalletManagerStore environment object. During orphan wallet recovery, routes wallet creation through per-network background manager derived from restoredNetwork. After wallet creation, retries fetching the PersistentWallet row from main modelContext (up to 10 attempts) before applying import metadata.
App Bootstrap & Orphan Pre-Warming
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/SwiftExampleAppApp.swift
Scene injection switches to walletManagerStore instead of computed walletManager. Bootstrap now calls preWarmOrphanNetworkManagers() after initial manager activation to resolve keychain mnemonics to networks and request background manager creation for networks differing from the active network.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A wallet hops through networks with grace,
Background managers fill every space,
Regtest stands firm with DAPI near,
Orphan wallets now reappear,
SwiftData sync delays disappear! 🌟

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix(swift-example-app): route orphan recovery to per-network managers' directly matches the main objective: fixing recovery of orphan-mnemonic wallets by routing them to per-network managers instead of the active network.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch swift-example-app/cross-network-orphan-recovery

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented May 7, 2026

Review Gate

Commit: 167275f8

  • Debounce: 4m ago (need 30m)

  • CI checks: checks still running (1 pending)

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (03:55 AM PT Thursday)

  • Run review now (check to override)

Copy link
Copy Markdown
Member Author

@QuantumExplorer QuantumExplorer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self Reviewed

@QuantumExplorer QuantumExplorer merged commit 0fd99fc into v3.1-dev May 7, 2026
17 of 18 checks passed
@QuantumExplorer QuantumExplorer deleted the swift-example-app/cross-network-orphan-recovery branch May 7, 2026 10:55
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "21bc88957ac7d4e5ccdcd8676381e879ec43d44b4c0db87ccb9e7466822d1a60"
)

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants