fix(dashpay): recover a restored identity in-session and repaint contact payments - #950
Merged
QuantumExplorer merged 5 commits intoAug 11, 2026
Merged
Conversation
6 tasks
📝 WalkthroughWalkthroughThe changes update DashPay payment refresh notifications, same-seed identity recovery retries, Platform startup error handling, and runtime reset cleanup. ChangesDashPay payment refresh
Same-seed identity recovery
Platform startup
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant DWCurrentUserIdentityInfo
participant Wallet
participant PlatformDiscovery
participant RetryTask
Runtime->>DWCurrentUserIdentityInfo: Start recovery
DWCurrentUserIdentityInfo->>Wallet: Capture wallet and network context
DWCurrentUserIdentityInfo->>PlatformDiscovery: Run immediate discovery
PlatformDiscovery-->>DWCurrentUserIdentityInfo: Return identity, empty result, or error
DWCurrentUserIdentityInfo->>RetryTask: Schedule validated retry
RetryTask->>PlatformDiscovery: Run delayed discovery
PlatformDiscovery-->>DWCurrentUserIdentityInfo: Return discovery result
Runtime->>DWCurrentUserIdentityInfo: Cancel recovery during reset
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…act payments Two independent defects behind "DashPay contacts and transactions only come back after a resync". **The identity was found once, or not until relaunch.** Recovery runs inside `PlatformAddressSyncCoordinator.performStart`, which early-returns when the coordinator is already running — so a restored wallet gets exactly one scan, in the seconds right after restore, when the network is least likely to answer. A scan that came back empty was then recorded in `completedContexts` as final for the process, because an empty result was reported as success. No identity means no DashPay tabs, no contacts and no contact payment history for the whole session. Only a found identity now ends the search. An empty or failed scan retries on a 20s/60s/180s backoff. The retries run outside the runtime-start pipeline, so each pass re-resolves the live wallet through `SwiftDashSDKHost.shared.wallet` and checks `runningNetwork` instead of holding the handle it started with — a wipe, wallet switch or network switch between retries ends the search rather than scanning against a torn-down runtime. **A failed `platformAddressWallet()` took DashPay down with it.** That path returned before the shielded bind, the DashPay sync start AND identity recovery, none of which use the address wallet. It now records `lastError` and continues; `addressWallet` was already an optional the rest of the method handles, since the no-Platform-account branch sets it to nil and carries on. **The feed never learned that contact payments had arrived.** A DashPay row's true direction, amount and contact name come from `DashPayPaymentTxLookup`, whose rows are written by an app-pulled projection into entities `saveTouchesFeedRows` filters out, and are read through a computed property on rows that were already rendered. The one DashPay-aware reload fires when the identity is adopted — before the sync loop has fetched anything. So the feed kept dash-spv's misread direction (an outgoing contact payment reads as incoming) and a nameless "?" avatar until something unrelated happened to touch `PersistentTransaction`. Whether that happened decided whether the bug appeared: a wallet still catching up repainted by accident, a quiet one never did. The lookup now posts `DWDashPayPaymentTxLookupDidChange` when the snapshot actually changes (`PaymentInfo` gained `Equatable` for the comparison), and `observeDashPay()` reloads the feed on it. Gated on a real change so the projection's timer cannot turn into a periodic rebuild of the whole history list. Also stop arming the projection's 60s throttle from a call that returned early for want of an identity: that spent the launch's first window on a no-op, and the identity typically lands seconds later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swift`:
- Around line 661-676: Update fullReset to cancel and await any active identity
recovery tasks before calling SwiftDashSDKHost.shared.stop(), ensuring suspended
discoverIdentities or syncDpnsNames work cannot access a released wallet. Also
invalidate the context so a subsequent recoverIfNeeded call can start fresh
rather than returning because activeContexts still contains the old recovery.
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/PlatformAddressSyncCoordinator.swift`:
- Line 837: Preserve the address-wallet startup error separately from the
consumer-facing lastError in the coordinator’s startup flow around
addressWalletError. Restore that stored error after successful Platform address
sync handling in handleSyncEvent, rather than allowing success to clear it
permanently, and clear the deferred error during teardown.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c32132d1-af19-488e-83c1-9b7ef139cb69
📒 Files selected for processing (4)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Contacts/SwiftDashSDKContactsService.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/PlatformAddressSyncCoordinator.swiftDashWallet/Sources/UI/Home/Views/HomeViewModel.swift
romchornyi
force-pushed
the
fix/dashpay-restore-identity-and-feed-refresh
branch
from
August 9, 2026 21:45
7a18300 to
4ce79f4
Compare
5 tasks
…ry on teardown Both findings from the review on #950. **A successful address sync erased the address-wallet failure.** Making `platformAddressWallet()` non-fatal meant the Platform address sync now starts even when that wallet is missing — and `handleSyncEvent` clears `lastError` on every successful pass. The startup failure was published once and then wiped by the first success, so the status screen reported a healthy sync over address surfaces that cannot work. The two fail independently, so the startup error is held in its own field and re-published wherever `lastError` is cleared on success; teardown clears it. **Recovery retries could outlive the wallet.** The first attempt runs inside the serialized runtime start, which is what kept the wallet handle alive across its FFI scan. The retries deliberately run outside that serialization and only re-check the wallet *before* awaiting, so a pass suspended inside `discoverIdentities` could still be running when `fullReset` dropped the handle. `cancelPendingWork()` cancels the pending backoffs and — the part that matters — awaits each task, since a task suspended in a synchronous FFI call never observes cancellation. `fullReset` calls it before `SwiftDashSDKHost.stop()`. Two smaller consequences of the same ownership question: - `recoverIfNeeded` now cancels a superseded backoff *before* its guards. An attempt already in flight keeps `activeContexts` set, and returning early there left the old backoff scheduled behind it. - A finished retry task no longer removes itself from `retryTasks`. That dictionary is teardown's handle on the work, and a self-removal could delete an entry a newer start had already replaced, putting that task out of reach. Awaiting a finished task is free.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/PlatformAddressSyncCoordinator.swift`:
- Around line 123-133: Update the lifecycle comment for
addressWalletStartupError near its declaration to include clearDisplay() as
another path that clears the retained error during local-state reset or
network-switch preparation, alongside teardown and successful wallet resolution.
- Line 134: Remove the explicit “= nil” initializer from the
addressWalletStartupError property declaration; keep it as an optional String
property so Swift’s default initialization supplies nil.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bbed5a0a-2cbe-4446-afd6-000f0ff7b3e8
📒 Files selected for processing (3)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/PlatformAddressSyncCoordinator.swiftDashWallet/Sources/Infrastructure/SwiftDashSDK/SwiftDashSDKWalletRuntime.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swift
…cycle Review follow-up on #950. The doc comment claimed the value is "cleared only by teardown or a start that resolves the wallet", but `clearDisplay()` also clears it, and that runs on wipe and on network-switch preparation too. Those are all paths that invalidate the wallet the error was recorded against, so the behaviour is right — the description was not, which is exactly what the repo's comment rule exists to catch. Also drop the redundant `= nil` on the optional (SwiftLint `redundant_optional_initialization`).
Member
|
Empty is provably empty, there is no reason to rescan if the network proves to you that you have no identity. |
…retry Review follow-up on #950: "empty is provably empty, there is no reason to rescan if the network proves to you that you have no identity." That is right, and it is right *because* platform#4352 landed. Before it, an empty result could mean either "Platform says this seed owns no identity" or "we never reached Platform", so retrying an empty was the only way to survive the second case. Now an unanswered scan raises `IdentityDiscoveryIncomplete` and the two are distinguishable, which is what makes a returned empty trustworthy enough to record as final. So the backoff now fires only when the scan threw. A scan that returns — with or without an identity — marks the context complete and stops. A wallet that genuinely owns no identity no longer pays three pointless network round trips per launch. `attempt` returns "Platform answered" rather than "an identity was found"; the call site and the doc comments say so.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swift`:
- Around line 762-769: The SameSeedIdentityRecoveryPipeline.run flow currently
treats post-discovery failures like incomplete discovery. Track discovery
completion separately from refreshNames or identity-adoption errors, and make
recoverIfNeeded return false only when discoverIdentities reports an
incomplete/failed scan; if discovery completed successfully, preserve completion
and avoid scheduling discovery backoffs even when syncDpnsNames fails. Add a
regression test covering successful discovery followed by syncDpnsNames failure
and assert that no discovery retry is scheduled.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f747b84f-39bf-4139-875a-043672f768a5
📒 Files selected for processing (1)
DashWallet/Sources/Infrastructure/SwiftDashSDK/Identity/DWCurrentUserIdentityInfo.swift
Review: "the swift client should not be doing this, it should be in rust." Correct, and `packages/swift-sdk/CLAUDE.md` says so outright — "No iteration / gap-limit walks / policy loops in Swift", and "if it's deciding anything — how many, which index, which path, which key, which order — move the decision to Rust. If you find a decision that Rust doesn't currently let you ask for by a single call, add the helper in the Rust library first." A hand-rolled 20s/60s/180s backoff with its own per-context bookkeeping is exactly that. Reverting it here rather than carrying it as a stopgap, so the policy has one home when it lands in `rs-platform-wallet` alongside the startup-ordering work. Little is lost in the meantime: platform#4352 already made an unreachable scan raise `IdentityDiscoveryIncomplete` instead of an empty success, so the restored behaviour marks a context complete only when Platform actually answered, and a failed scan is retried on the next runtime start. The gap that remains is healing time within one session, not correctness. `cancelPendingWork` and its `fullReset` hook go with it — they existed only to keep those retry tasks from outliving the wallet. What stays in this PR is the part that has no Rust equivalent: the transaction feed not repainting when DashPay payment rows land, and the address-wallet startup failure no longer taking the DashPay subsystems down with it.
Contributor
Author
|
Agreed, and fixed — in two steps, because the first one only fixed half of it.
That decision now lives in So this PR no longer decides anything about rescanning; it fixes the two real bugs it set out to fix (a restored identity not recovering in-session, and the home feed not repainting DashPay payment rows). |
QuantumExplorer
approved these changes
Aug 11, 2026
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
3 participants
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.
Issue being fixed or feature implemented
QA: "transactions on DashPay and contacts come back if you resync" — after restore-from-seed the identity sometimes does not appear, and contact payment rows render with the wrong direction and a
?avatar until the app is relaunched.These are two independent defects that happen to share a symptom, which is why neither reproduced reliably.
1 — the identity was found once, or not until relaunch. Recovery runs inside
PlatformAddressSyncCoordinator.performStart, which early-returns when the coordinator is already running, so a restored wallet gets exactly one scan — in the seconds right after restore, when the network is least likely to answer. A scan that came back empty was recorded incompletedContextsas final for the process, because an empty result arrived as success. No identity means no DashPay tabs, no contacts and no contact payment history for the whole session. Whether it happened came down to whether the SDK's quorum endpoint answered in those first seconds.2 — the feed never learned that contact payments had arrived. A DashPay row's true direction, amount and contact name come from
DashPayPaymentTxLookup. Those rows are written by an app-pulled projection into entitiesHomeViewModel.saveTouchesFeedRowsfilters out, and are read through a computed property (Transaction.dashPayPayment) on rows that were already rendered. The only DashPay-aware reload fires when the identity is adopted — before the sync loop has fetched anything. So the feed kept dash-spv's misread direction (an outgoing contact payment reads as incoming+amount) until something unrelated touchedPersistentTransaction. A wallet still catching up on SPV repainted by accident; a quiet, already-synced one never did.Companion Platform fix: dashpay/platform#4352 — that one stops
discover_identitiesfrom reporting an unreachable scan as "no identity exists". The changes here are independently effective, since an empty scan is no longer treated as final either way.What was done?
Identity recovery
DWCurrentUserIdentityInfo.swift—DWSameSeedIdentityRecoveryCoordinatornow marks a context complete only when an identity was actually found. An empty or failed scan retries on a 20s/60s/180s backoff. The scan body moved intoattempt(...), which returns whether the wallet has an identity;scheduleRetries(...)walks the schedule.Retries run outside the runtime-start pipeline, so each pass re-resolves the live wallet via
SwiftDashSDKHost.shared.walletand checksrunningNetworkrather than holding the handle it was started with — a wipe, wallet switch or network switch between retries ends the search instead of scanning against a torn-down runtime. A freshperformStartcancels any pending backoff, since it re-runs the same first attempt.PlatformAddressSyncCoordinator.swift— a failingplatformAddressWallet()no longer returns. It returned before the shielded bind,startDashPaySync()and identity recovery, none of which use the address wallet, so one failure took all three down for the session. The error is carried inaddressWalletErrorand lands onlastErrorat the end;addressWalletwas already an optional the rest of the method handles, since the no-Platform-account branch sets it to nil and continues.Transaction feed
SwiftDashSDKContactsService.swift—DashPayPaymentTxLookup.store(_:)compares the incoming snapshot against the current one and postsDWDashPayPaymentTxLookupDidChangeonly on a real change (PaymentInfogainedEquatable). Gating on change matters: the projection re-runs on a timer, and an unconditional post would rebuild the whole history list every pass.HomeViewModel.swift—observeDashPay()subscribes to that notification and funnels it throughtxReloadRequests.SwiftDashSDKContactsService.swift—refreshPaymentsProjection()arms its 60s throttle only when it actually pulled. Previously a call that returned early for want of an identity still spent the launch's first window, and the identity typically lands seconds later.How Has This Been Tested?
No new tests. The unit-test target is currently broken (pre-existing, noted in
CLAUDE.md), and the changed behaviour lives in@MainActortypes that need a liveManagedPlatformWallet. The existingSameSeedIdentityRecoveryPipelinetests inDashWalletTests/SwiftDashSDKCoreLifecycleTests.swiftstill apply unchanged — the pipeline's sequencing was not touched, only the coordinator's interpretation of its outcome. The Platform-side counterpart of this fix does carry unit tests (dashpay/platform#4352).Not verified on-device — this is the main gap. Suggested manual checks on testnet:
🪪 IDENT-RECOVERY :: scan found no identity; will retryfollowed by a successful pass within ~20s, and DashPay tabs appearing without a relaunch. Escape hatch if it still fails: Menu → Security → Wallets → Identities → Find identities.Breaking Changes
None.
One behavioural note for reviewers:
PlatformAddressSyncCoordinatorcan now finishperformStartwithisRunning == trueand a non-nillastError, where it previously bailed out. That state is what drives the platform-sync status UI, and it is deliberate — the address surfaces are empty andlastErrorsays why, while Shielded and DashPay keep working.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit