Skip to content

fix(dashpay): defer contact auth until preview closes - #994

Merged
llbartekll merged 1 commit into
developfrom
codex/fix-contact-pin-presentation-race
Aug 12, 2026
Merged

fix(dashpay): defer contact auth until preview closes#994
llbartekll merged 1 commit into
developfrom
codex/fix-contact-pin-presentation-race

Conversation

@llbartekll

@llbartekll llbartekll commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • defer Send/Accept authentication until the contact preview sheet has fully dismissed
  • consume pending actions exactly once and revalidate relationship state before dispatch
  • show a user-facing error when recipient DashPay keys become unavailable during dismissal

Root cause

The contact preview started authentication and dismissed itself in the same tap. Depending on SwiftUI scroll/focus and keyboard timing, the PIN modal could be attached to the disappearing preview controller, briefly appear, then be removed and reported as Authentication failed before any PIN digits were entered.

Verification

  • xcodebuild -workspace DashWallet.xcworkspace -scheme dashpay -configuration Debug -destination generic/platform=iOS Simulator CODE_SIGNING_ALLOWED=NO EXCLUDED_ARCHS=x86_64 ONLY_ACTIVE_ARCH=YES build
  • git diff --check
  • manual confirmation from reporter that the adjusted flow works

Summary by CodeRabbit

  • Bug Fixes
    • Improved send and accept contact request flows by completing preview dismissal before applying actions.
    • Added validation to prevent sending when required keys are missing or when contact details conflict.
    • Prevented accepting requests that are no longer available.
    • Improved handling when preview actions are cancelled or become outdated.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

AddContactScreen now queues send and accept actions from the preview, dismisses the sheet first, and dispatches each action afterward with updated state validation.

Changes

Contact preview actions

Layer / File(s) Summary
Preview action state and dismissal wiring
DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift
The screen stores pending send and accept actions. Preview dismissal triggers deferred dispatch, and CTA handlers no longer dismiss the preview directly.
Validated send and accept dispatch
DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift
Send rechecks collision and DashPay key state. Accept proceeds only when the incoming request still exists. Pending actions are consumed before dispatch.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AddContactScreen
  participant PreviewSheet
  participant performPendingPreviewAction
  participant ContactRequestFlow
  AddContactScreen->>PreviewSheet: Queue send or accept action
  PreviewSheet->>AddContactScreen: Complete dismissal
  AddContactScreen->>performPendingPreviewAction: Dispatch pending action
  performPendingPreviewAction->>ContactRequestFlow: Revalidate current state
  ContactRequestFlow-->>AddContactScreen: Send or accept, or report invalid state
Loading

Possibly related PRs

Suggested reviewers: quantumexplorer

🚥 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 title clearly and concisely describes deferring contact authentication until the preview closes, which is the primary change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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 codex/fix-contact-pin-presentation-race

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift (1)

81-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the explicit nil initializer.

SwiftLint reports implicit_optional_initialization on Line 81. Optional stored properties initialize to nil by default.

Proposed fix
-    `@State` private var pendingPreviewAction: PendingPreviewAction? = nil
+    `@State` private var pendingPreviewAction: PendingPreviewAction?

As per coding guidelines, “Use SwiftFormat and SwiftLint conventions.”

🤖 Prompt for 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.

In `@DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift` at
line 81, Remove the explicit nil initializer from the pendingPreviewAction state
property declaration, relying on Swift’s default optional initialization while
preserving its existing optional type and behavior.

Sources: Coding guidelines, Linters/SAST tools

🤖 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.

Nitpick comments:
In `@DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift`:
- Line 81: Remove the explicit nil initializer from the pendingPreviewAction
state property declaration, relying on Swift’s default optional initialization
while preserving its existing optional type and behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f53b18db-43ef-4b07-a647-eb05bc7b317f

📥 Commits

Reviewing files that changed from the base of the PR and between ec66323 and 07e8f7c.

📒 Files selected for processing (1)
  • DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/AddContactScreen.swift

@llbartekll
llbartekll requested a review from romchornyi August 12, 2026 06:56
@romchornyi

Copy link
Copy Markdown
Contributor

Revalidating after dismissal is the right call — but two of its outcomes are silent.

In performPendingPreviewAction, the .send path's default: return and the .accept path's guard case .theyAskedUs ... else { return } both drop the action with no feedback. By that point the preview sheet is already gone, so the user tapped "Send Contact Request" (or "Accept"), watched the sheet close, and nothing happens — no error, no confirmation, no way to tell whether it worked.

It's a narrow window, but it's exactly the window this PR opens up: the action is now dispatched after the dismissal, so a relationship change during that transition lands in these branches rather than being impossible as before.

.missingDashPayKeys already gets a message; the remaining cases deserve one too. Something like "This contact's status changed — please try again" for the .send default, and the same for a request that's no longer pending on the .accept side, would keep the tap from evaporating.

@llbartekll
llbartekll merged commit bb01ab3 into develop Aug 12, 2026
3 checks passed
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