feat(wallet): switch between on-device wallets — Reset Wallet becomes a Wallets screen - #790
Merged
Merged
Conversation
…itch-wallet phase 0) The app assumed a single SDK wallet and arbitrarily bound `PlatformWalletManager.firstWallet`, while every wallet-data read scanned all persisted rows across all wallets. Phase 0 makes the app honest about which wallet is active and scopes reads to it — no UI, no behavior change for existing single-wallet installs. Active-wallet registry: `WalletEnvironment` gains per-network `activeWalletId(for:)` / `setActiveWalletId(_:for:)` backed by a per-network UserDefaults key holding the raw walletId `Data` (nil = unset), following the file's existing stateless-static-namespace pattern (no new singleton). `SwiftDashSDKHost` resolves the persisted active walletId among the manager's loaded `wallets`, falling back to `firstWallet` when unset or missing, and writes the resolved id back on every bind (fallback included, and after `createOrImportWallet`) so the registry is concrete after first launch. For a single-wallet install `firstWallet` == the only wallet, so the resolved wallet is unchanged. Scoped reads: `PersistentTransaction` deliberately carries no walletId (one row is shared across wallets), so per-wallet membership is recovered by joining through `PersistentTxo.walletId` — gather the active wallet's txids from its walletId-scoped TXO rows (union of the producing `transaction` and the spending `spendingTransaction`) and match transactions in that set. Scoped sites: the home tx list and point-lookup (HomeViewModel), the receive-address used/received-total reads (SwiftDashSDKReceiveAddressReader), ZenLedger address export, and the CrowdNode TransactionObserver scan. The TXO join is one walletId-scoped fetch per reload (not per transaction), so the home timeline stays a single indexed scan on large wallets. Readers that run before the host binds a wallet return empty, which is correct. Co-Authored-By: Claude Opus <noreply@anthropic.com>
…h-wallet phase 1) Phase 0 made the app honest about which wallet is active (per-network registry + walletId-scoped reads) but left no way to change the active wallet at runtime. Phase 1 adds that engine. No UI (Phase 2 owns UI). Reuse the network-switch sequence, don't duplicate it. `switchWallet(to:)` on SwiftDashSDKWalletRuntime validates the target, repoints the per-network active-wallet registry, then runs the exact stop -> clear -> load -> start sequence `refresh` already owns for a network switch — the only difference is the network is unchanged, so the host's Phase-0 `resolveActiveWallet` binds the newly-recorded wallet when it rebuilds. Added an awaitable variant of the serial-chain `enqueue` so the switch can block until its rebuild (and every op queued before it) completes, then verify the bind and post the change signal. A new `.walletDidChange` refresh trigger never elides the rebuild (same network would otherwise skip it). Validation is synchronous on the main actor before any teardown; failures surface as `SwitchError` (unsupportedNetwork / unknownWallet / bindFailed) rather than logged-and-swallowed. Switching to the already-active wallet is a no-op. Typed change signal: `SwiftDashSDKWalletState.activeWalletDidChangeNotification` (new name, not a re-emission of any DS* name nor of balanceDidChange — guardrail #5), posted on the main thread after the new wallet is bound and its balance seeded. Consumer audit (wired vs. already-covered): - DWCurrentUserIdentityInfo: WIRED — added the notification to its lazy- invalidation observers so the next read rebuilds from the new wallet's identity rows. - SwiftDashSDKContactsService: WIRED — ownerId changes with the wallet; observes and refreshes. Forces the identity snapshot fresh first because NotificationCenter delivery order between the two observers isn't guaranteed. - HomeViewModel: WIRED — treats the switch like a network change (clearCachedData) because the per-hash tx cache belongs to the old wallet and a same-total balance event wouldn't clear it. - MainTabbarController: WIRED via a new unconditional rebuild. The existing reconfigureDashPayTabsIfNeeded only ADDS tabs (early-returns without an identity), so switching to a wallet without an identity needs a rebuild that also REMOVES the DashPay tabs; selection resets to Home. - BalanceModel / BalanceNotifier / DWPhoneWCSessionManager: NOT wired — they follow SwiftDashSDKWalletState.$balance / balanceDidChange, which the switch sequence already drives (clearAllState clear + the SPV re-seed). Registry hygiene on wipe: SwiftDashSDKWalletWiper clears the active-wallet registry for both registry-backed networks. The wipe removes all wallets (mnemonics are network-agnostic), so every recorded active id now points at nothing; Phase 0's fallback would mask a stale id, but clearing it keeps the registry honest. Co-Authored-By: Claude Opus <noreply@anthropic.com>
…phase 2) Replaces the Security menu's destructive "Reset Wallet" entry with a "Wallets" screen listing the on-device SwiftDashSDK wallets for the current network, one marked active, with switch-on-tap, rename, and per-wallet remove. Add Wallet is Phase 3 and is intentionally absent (no disabled controls, no stubs). Menu wiring: `SecurityMenuViewModel`'s "Reset Wallet" item becomes a "Wallets" item; `SecurityMenuScreen` pushes a `UIHostingController` wrapping the new SwiftUI screen (matching the sibling push pattern). `DWResetWalletInfoViewController` is kept — the Wallets screen still presents it for the last-wallet remove path. WalletsScreen + WalletsViewModel (SwiftUI + @mainactor VM, SwiftUI-first): - List source: `SwiftDashSDKHost.shared.manager?.wallets` intersected with the walletIds that have a persisted mnemonic (`SwiftDashSDKHost.persistedMnemonics()`) — a wallet without a mnemonic isn't switchable. Per row: display name (`PersistentWallet.name` else "Wallet <prefix>…"), balance (`ManagedPlatformWallet.balance().total` via the app's `formattedDashAmount`), DPNS username (read from the wallet's pinned `PersistentIdentity` row directly — target-neutral SDK model, so no DASHPAY gate), and an active checkmark (`WalletEnvironment.activeWalletId(for:)`, network via `isTestnet`). - Switch: confirmation alert → `SwiftDashSDKWalletRuntime.switchWallet` with a blocking progress overlay; refreshes on `activeWalletDidChangeNotification`; surfaces `SwitchError` in an alert on failure (never claims success on failure). - Rename: alert with TextField → writes `PersistentWallet.name` via the host `modelContainer` mainContext (empty clears to nil). - Remove: recovery-phrase confirmation sheet spelling out that only this device's copy is destroyed (funds recoverable only with the phrase). Verified read-only by deriving the walletId from the entered mnemonic (`Wallet(mnemonic:network:).id`, which persists nothing) and comparing. Removing the active wallet auto-switches to another wallet first; the last wallet routes to the existing full reset flow. Guardrail #1 (no copy-then-adapt): the per-wallet deletion body is promoted out of `SwiftDashSDKWalletWiper` into an internal `deleteWalletFromSDK(_:)` (manager `deleteWallet` + mnemonic delete), reused by both the full wipe loop and the new remove flow. Both `dashpay` and `dashwallet` targets are registered in project.pbxproj (this UI is not DashPay-gated). `dashpay` builds green; the new code also compiles clean under `dashwallet` (its remaining failures are the pre-existing DASHPAY-only breakage this branch documents). Co-Authored-By: Claude Opus <noreply@anthropic.com>
…tch-wallet phase 3) Phase 2 shipped the Wallets screen (switch / rename / remove) but left Add Wallet for Phase 3. This adds it and makes the two genuinely per-wallet DWGlobalOptions flags wallet-scoped. Add Wallet (WalletsScreen + WalletsViewModel): A "+" in the screen header presents a sheet with two paths. - Create New Wallet: generate a 12-word mnemonic via SwiftDashSDK's `Mnemonic.generate(wordCount: 12)`, show it in a clean SwiftUI phrase grid (DWPreviewSeedPhraseModel is too DashSync-entangled to reuse — it dual-writes to DSWallet — so a new grid, no new UIKit), and require an explicit "I have written it down" toggle before creating. - Import from Phrase: multiline entry, `Mnemonic.validate` gate. If the derived walletId is already on device, a typed `.alreadyOnDevice` state offers switching to it instead of fabricating a success. Both paths add additively then `switchWallet(to:)` the new wallet (awaited, progress state), dismissing on success. All SDK/mnemonic work lives in the VM. Additive host path (SwiftDashSDKHost): `createOrImportWallet` is NOT additive — it rebuilds the runtime (`buildRuntime` calls `stop()`), pins the new wallet active in the registry, and publishes it as bound. That is correct for onboarding's first/sole wallet but would unbind the active wallet when adding a second. New `addWallet(mnemonic:)` uses the LIVE running manager, persists the mnemonic, and does NOT touch the registry or publish (the UI switches explicitly). The create-then-persist-with-rollback body is factored into a shared `createAndPersist` used by both paths (no duplication). `addWallet` detects the idempotent "already exists" case up front by deriving the walletId (`Wallet(mnemonic:network:).id`) and checking `persistedMnemonics()`, returning `.alreadyExists` rather than silently no-opping. Neither path touches DashSync. Per-wallet flags (DWGlobalOptions): `walletNeedsBackup` and `userHasBalance` describe a per-wallet fact but were app-global. Their getters/setters now resolve `DW_WALLET_NEEDS_BACKUP_<hex>` / `DW_WALLET_HAS_BALANCE_<hex>` scoped by the active walletId (`DWWalletEnvironment.activeWalletIdHex`, a new @objc accessor over the same per-network registry — one place owns it). One-time migration: on first read with no per-wallet key, seed from the legacy `DW_GLOB_*` key's effective value (which carries the registered default, e.g. needs-backup = YES), then the legacy key goes dormant (not deleted) and remains the fallback while no wallet is active yet (onboarding sets these before a walletId is resolved). The 30+ existing call sites keep the property names as the active-wallet view, unchanged. `shortcuts` stays global (a UI preference). New wallets: created → needs-backup TRUE (phrase shown, not verified — matches onboarding), imported → FALSE (user holds the phrase — matches recover). DSDynamicOptions reads/writes UserDefaults live on every accessor (no in-memory cache), so a wallet switch needs no cache invalidation — the next read simply resolves the new active wallet's key. B2 has nothing to wire. dashpay builds green (ARCHS=arm64). Nothing added references DASHPAY-only symbols un-gated: the per-wallet flags live outside any `#ifdef DASHPAY`, and the UI/host code is target-neutral SDK surface. No files added, so no pbxproj/plist changes. Co-Authored-By: Claude Opus <noreply@anthropic.com>
…phase 4) CrowdNode state describes ONE wallet's CrowdNode account (bound to a funding address in a specific wallet), but was stored app-global — so a wallet switch would show wallet A's CrowdNode balance/account under wallet B (money-display-adjacent wrongness). Scope the per-wallet keys the same way DWGlobalOptions phase 3 scoped its per-wallet flags: effective key = `<legacyKey>_<activeWalletIdHex>`, resolved live per access via a single `resolvedKey(_:)` seam in CrowdNodeDefaults (and CoinJoinWithdrawalStore), seeded once from the legacy key on first read, with the legacy key kept dormant as the no-active-wallet fallback. Per-wallet: account/primary address, online account state, last known balance, confirmation-dialog / online-info shown, signed-email message id, pending confirmed-notification, last withdrawal block, and the CoinJoin withdrawal txid tag set. Kept global: the CrowdNode info / withdrawal-limits education flags (user-level) and the withdrawal limits / fee (API service parameters). CrowdNodeDefaults caches values in in-memory `_`-backed fields, so a wallet switch also invalidates that cache; CrowdNode.shared caches the active wallet's published state, so it observes activeWalletDidChangeNotification (mirroring SwiftDashSDKContactsService) and reloads from the now-active wallet's defaults without wiping them. resetForWipe (both stores) now clears EVERY wallet's per-wallet keys by prefix enumeration plus the dormant legacy keys — the wipe destroys every wallet. The shared per-wallet deletion primitive deleteWalletFromSDK now also clears the removed wallet's per-wallet CrowdNode + withdrawal keys, so the Wallets-screen Remove flow drops them without touching the UI. Co-Authored-By: Claude Opus <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Replaces the Security menu's destructive Reset Wallet with a Wallets screen: multiple wallets on device, one active at a time — switch, rename, add (create/import), and per-wallet remove. The SDK layer was already multi-wallet (
loadFromPersistorrestores all wallets; mnemonics are walletId-keyed inWalletStorage); the app's single-wallet constraint wasmanager.firstWalletplus unscoped reads, both removed here.The four phases (one commit each)
bcd1cf024— registry + scoped reads. Per-networkactiveWalletIdinWalletEnvironment(stateless-namespace pattern);SwiftDashSDKHost.resolveActiveWallet(fallbackfirstWallet, resolved id written back). Every wallet-data read scoped:PersistentTransactiondeliberately has no walletId (a row can be shared across wallets), so membership is recovered by joining throughPersistentTxo.walletId— home tx list, point lookups, receive-address used/received-total, ZenLedger export, CrowdNode observer. No behavior change for single-wallet installs.14a8eaa8f— the switch engine.SwiftDashSDKWalletRuntime.switchWallet(to:)reuses the exact network-switch stop→clear→load→start sequence (awaitable serial-chain variant); typedSwitchError; newactiveWalletDidChangeNotificationposted after bind+seed. Consumers wired: identity snapshot invalidation, contacts refresh (with explicit ordering), home cache clear, and an unconditional tab rebuild (the existing reconfigure only ever ADDED DashPay tabs — switching to an identity-less wallet needs removal too). Wiper clears the registry.4d51ca46a— the Wallets screen. SwiftUI +@MainActorVM underUI/Menu/Security/Wallets/; list shows name (PersistentWallet.name), balance, DashPay username, active check. Switch (confirm + progress + error surfacing), rename, and Remove behind recovery-phrase confirmation verified SDK-natively (Wallet(mnemonic:network:).id— derives keys locally, persists nothing). Per-wallet deletion promoted OUT of the wiper into a shared primitive (one body, wipe + remove). Removing the active wallet auto-switches first; the last wallet routes to the preserved full-reset flow (DWResetWalletInfoViewController), keeping today's semantics.0378b4fbe+5aef64184— add wallet & per-wallet state. AdditiveSwiftDashSDKHost.addWallet(mnemonic:)(the existingcreateOrImportWallettears down and rebinds the runtime — onboarding keeps it; adding must not). Create (phrase display + written-down confirmation) and import (with an honest.alreadyExistsstate) both auto-switch after.walletNeedsBackup/userHasBalancebecome per-wallet (per-wallet UserDefaults keys, one-time seed from the legacy global values, legacy fallback pre-onboarding; created wallets need backup, imported don't). CrowdNode + CoinJoin-withdrawal state scoped per wallet the same way (account address, signup state, balance, withdrawal txids — API limits and once-ever education flags stay global);CrowdNode.sharedreloads on the change notification; wipe/remove clear per-wallet keys through the shared primitive.Verification
PersistentTxofetches walletId-scoped;firstWalletonly as documented fallback), concurrency review of the serial-chain refactor (all mutations MainActor-confined), wiper-orphan coverage,Wallet(mnemonic:)non-persistence, pbxproj dual-target registration +plutil -lint, and an independentdashpayarm64-sim build after each phase — all green.dashwalletscheme's failures are pre-existing (files this branch never touches; CLAUDE.md documents that target as not kept green); the new UI compiles cleanly under it.Known follow-ups (deliberate)
CrowdNode.restoreState → validatePrefsstill reads frozen DashSync (containsAddress) — pre-existing, untouched.🤖 Generated with Claude Code