Skip to content

fix: contact request or pay sheet and pay delay - #1349

Merged
jvsena42 merged 21 commits into
masterfrom
fix/contact-pay-request-sheet
Sep 29, 2026
Merged

jvsena42 merged 21 commits into
masterfrom
fix/contact-pay-request-sheet

Conversation

@jvsena42

@jvsena42 jvsena42 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

iOS port: synonymdev/bitkit-ios#809

This PR:

  1. Fixes the Request or Pay sheet sometimes not showing when paying a contact that can receive payment requests
  2. Fixes the long delay between tapping Pay on a contact and the amount screen opening
  3. Adds a loading state to Pay while the payment is being prepared

Description

  • Checks the open contact's payment request support when Contact Detail opens, and again on Pay if it is still unknown (waiting at most 2 s before falling back to paying), because the shared list of request-capable contacts is filled in the background and could miss a linked contact for up to two minutes, so Pay skipped the sheet
  • Publishes the payer's own Paykit endpoints to the contact in the background instead of before resolving the payment, because that publish waited behind the app's own background link refreshes and took about 9 of the 10 seconds Pay needed; resolving the contact's payment already advances the private link
  • Shows a spinner on the Contact Detail Pay button while the payment is prepared, and keeps the Request or Pay sheet open with its Pay button loading and Request disabled until the amount screen has opened, so the tap no longer looks ignored
  • Adds a journey for the Request or Pay flow from Contact Detail

Out of Scope

  • Paykit start-up contention: right after launch, session restore and the initial link refresh hold the Paykit SDK, so Pay can still take much longer for the first minute
  • Paykit SDK: a process kill during a link operation left the link stuck for a few minutes ("peer link operation already in progress") before recovering

Design

Preview

Warm app, contact linked on bitkit/wallet: open the contact, tap Pay, then Pay on the sheet.

Before: the sheet closes and nothing happens for about 7 s before the amount screen opens.

before.mp4

After: Pay shows a spinner on the sheet and the amount screen opens in about 1.5 s.

after.mp4

QA Notes

Journeys

  • new contact-request-or-pay.xml — Pay on a linked contact offers Request or Pay, Pay shows progress and opens the amount screen within 3 s, Request opens the Payment Request amount screen

Manual Tests

N/A

Automated Checks

  • added ContactDetailViewModelTest.kt — Pay shows the sheet for an eligible contact, refreshes eligibility when the contact is not yet known, falls back to paying when the contact cannot receive requests, skips the check for an unsaved contact, paying from the sheet opens the payment, a later Pay tap rechecks eligibility after an earlier check found none, the sheet closes when the contact stops being eligible but stays open while a payment is preparing, dismissing the sheet while paying cancels the payment or the amount-screen scan, a stalled eligibility check is cancelled before paying, a recent check is reused and a stale one repeated, and Pay stays loading until the amount screen opens; an incomplete or failed check is not reused, a checked target that leaves the eligible list is checked again, and leaving the screen cancels a pending payment or amount-screen scan
  • added PaykitPaymentRequestRepoTest.kt — the single-contact refresh adds a newly eligible contact, removes one that is no longer linked or stopped accepting requests, keeps a known target while the capability lookup fails, runs while a full refresh is still scanning, is not overwritten by an older full refresh and does not overwrite a newer one, leaves a full refresh's results for other contacts intact, a failed single-contact lookup neither overwrites an older full refresh nor reports itself complete, and a failed full refresh still drops contacts that are no longer saved but keeps a newer single-contact result
  • added AppViewModelSendFlowTest.kt — a cancelled contact scan clears its payment context
  • added PrivatePaykitRepoTest.kt — a saved-contact payment opens while the endpoint publish is still stalled, and repeated payments run one publish per contact at a time

jvsena42 and others added 5 commits September 28, 2026 08:18
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42 jvsena42 self-assigned this Sep 28, 2026
@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Contact payment flow adds request eligibility check and loading state.

The PR appears safe to merge; no outstanding findings or new actionable issues remain.

Summary

The PR refreshes a contact’s payment-request eligibility, moves endpoint publishing off the payment preparation path, and adds loading feedback and a journey for Request or Pay.

  • The latest change keeps the sheet visible during payment preparation if eligibility changes, while disabling Request.
  • The three previous findings are resolved in the current code.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Tap Pay on Contact Detail] --> B[Check request eligibility]
  B -->|Eligible| C[Show Request or Pay sheet]
  B -->|Not eligible or timed out| D[Prepare payment]
  C -->|Pay| D
  C -->|Request| E[Open request amount screen]
  D --> F[Show loading until payment opens or preparation ends]
Loading

Reviews (3) · Last reviewed commit: "fix: keep request or pay sheet open whil..."

@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from def4a3e (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

jvsena42 and others added 2 commits September 28, 2026 08:49
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42
jvsena42 marked this pull request as draft September 28, 2026 11:58
@jvsena42
jvsena42 marked this pull request as ready for review September 28, 2026 11:58
@jvsena42
jvsena42 requested review from a team, coreyphillips and pwltr and removed request for a team September 28, 2026 11:58
Comment thread app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt Outdated
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42
jvsena42 marked this pull request as draft September 28, 2026 12:06
@jvsena42
jvsena42 marked this pull request as ready for review September 28, 2026 12:06
jvsena42 and others added 4 commits September 28, 2026 09:13
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt Outdated
@coreyphillips

This comment has been minimized.

@jvsena42 jvsena42 added this to the 2.6.0 milestone Sep 28, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jvsena42 and others added 4 commits September 28, 2026 15:11
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42

Copy link
Copy Markdown
Member Author

Thanks, all four fixed, one commit each:

  • 2 s fallback still waits on the lookup (bcd8f4b): when Pay's 2 s wait times out, the eligibility check is now cancelled and joined before payment resolution starts, so it no longer holds the Paykit SDK mutex that beginSavedContactPayment needs. Covered by pay tap cancels a stalled eligibility check before paying.
  • Loading sheet closes before the amount screen opens (62b1d8f): the screen now hands the scan job from openContactPayment() back to the view model, which keeps Pay loading (and the Request or Pay sheet open) until that scan finishes. Dismissing the sheet during that phase cancels the scan too. Covered by the updated pay tests and dismissing the sheet while the amount screen opens cancels the scan.
  • Pay re-checks eligibility for contacts without requests (cf14c7e): a check that finished in the last 30 s is reused, so the check started when Contact Detail opens usually answers the tap without another lookup; after that window Pay checks again, still capped at 2 s. Covered by pay tap reuses a recent check that found no request support and pay tap rechecks eligibility once an earlier empty check is stale.
  • Repeated taps stack endpoint publishes (572d228): one pre-payment publish per contact at a time; a tap while one is running reuses it. Covered by beginSavedContactPayment runs one endpoint publish per contact at a time.

@jvsena42
jvsena42 requested a review from pwltr September 28, 2026 18:35
@jvsena42
jvsena42 enabled auto-merge September 29, 2026 11:42

@pwltr pwltr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Three behavioral issues remain around incomplete eligibility checks and Contact Detail lifecycle cleanup. I also noted one cross-platform identifier documentation mismatch.

Comment thread app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt Outdated
Comment thread journeys/README.md Outdated
@coreyphillips

Copy link
Copy Markdown
Contributor

Two independent reviews, nothing blocking a merge.

worth doing, does not block

  • Pay spinner sticks if the OpenPayment effect is emitted with no collector (app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt:214). If the user navigates away from Contact Detail while a no-sheet Pay is preparing, isPayLoading never clears and Pay stays disabled with a spinner when they come back. I confirmed this by reading the code, not on a device. How it happens: 1. openPayment() emits ContactDetailEffect.OpenPayment into _effects, a MutableSharedFlow(extraBufferCapacity = 1) with no replay (ContactDetailViewModel.kt). 2. Only onPaymentOpening() clears isPayLoading on the success path, and only the screen's LaunchedEffect collector calls it. 3. A shared flow with no subscribers drops what is emitted to it. extraBufferCapacity only buffers for subscribers that already exist. To reproduce, open a contact that cannot receive requests and tap Pay. While the spinner shows, tap Activity (or Edit). ContactDetailScreen leaves composition, but the ViewModel survives on the back stack. beginSavedContactPayment then succeeds and the effect is lost. When the user returns, isPayLoading is still true. ActionButton is disabled while loading, and onClickPay returns early. Pay stays stuck until the screen is popped. This is likely during the first minute after launch, when the PR itself says Pay can take much longer. The dropped effect predates this PR. The stuck loading state is new. Possible fixes: clear isPayLoading when the emit has no collector (for example, check _effects.subscriptionCount), use a Channel for effects, or reset the loading state when the screen resumes.
  • Failed full refresh drops a newer target (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:466). A failed full refresh can remove a newer per-contact result. If refreshEligibleTargets() starts with saved contact A, refreshEligibleTarget() then adds newly saved contact B, and the older full refresh fails, this failure handler filters the current targets using its old saved-key snapshot and drops B without checking targetWriteTickets. Request or Pay stays unavailable for B until another refresh.
  • Cancelling the scan leaves its contact context active (app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt:147). Dismissing the sheet after the scan starts cancels paymentScanJob, but it does not clear the contact payment context set by AppViewModel.prepareContactPaymentContextForScan(). The cancelled scan's completion handler also skips cleanup. isPaymentRequestPresentationBlocked() then continues to see an active contact context, so incoming payment requests can remain hidden until another action clears it.
  • The Design section omits the mapped Contact Detail frame (docs/screens-map.md:62). The PR description says no design is available, but docs/screens-map.md maps ContactDetailScreen.kt to Contacts › Contact details. Since this PR adds a user-visible Pay loading state on that screen, the repository rules require the Design section to link that frame.

nits

  • A reused eligibility check can bring back a target the shared list has dropped (app/src/main/java/to/bitkit/ui/screens/contacts/ContactDetailViewModel.kt:180). onClickPay falls back to awaitPaymentRequestTarget(), which reuses a completed deferred for up to 30 s. observePaymentRequestTarget may meanwhile have set paymentRequestTarget to null because the contact left eligibleTargets. In that case the stale non-null result from the deferred still opens the sheet. Request then goes to a contact that just stopped accepting requests. The window is small, and the later request call would presumably fail cleanly. One fix: drop the cached deferred whenever the observer sees the target disappear.

jvsena42 and others added 4 commits September 29, 2026 13:03
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42

Copy link
Copy Markdown
Member Author

Fixes for @pwltr's review and @coreyphillips's second review, one commit per concern:

  • 554de3d fix: only trust completed contact eligibility checks
    • A failed or timed-out single-contact lookup no longer takes a write ticket or touches eligibleTargets. refreshEligibleTarget now returns PaykitPaymentRequestTargetCheck(target, isComplete), the same as iOS refreshEligibleTarget(publicKey:) returning the current target when discovery.isComplete is false. A lookup that succeeds with no request path still counts as complete, so a contact that stopped accepting requests is still removed.
    • Contact Detail sets the 30 s reuse timestamp only for a completed check. A Result.failure or an incomplete check is looked up again on the next Pay tap.
    • A failed full refresh keeps targets written by a single-contact refresh that started after it.
    • A checked target is no longer reused once the contact leaves eligibleTargets.
  • ecd7a97 fix: cancel contact pay when contact detail leaves: Contact Detail dismisses the Request or Pay sheet when it leaves composition. This cancels the pending Pay job and the scan returned by AppViewModel, clears the loading state, and closes the sheet. Nothing is left to emit into an uncollected effects flow, and no scan can open Send over Activity or Edit. The ViewModel also cancels the scan in onCleared.
  • 0a1257e fix: clear contact context when its scan is cancelled: a cancelled scan clears the contact payment context it set, if that context is still active (compared by identity, so a newer scan's context is kept). Incoming payment requests are no longer blocked by it.
  • def4a3e docs: drop matched request or pay identifier row: iOS now uses RequestOrPaySheet, so the row is gone from the platform-difference table.
  • PR body: the Design section links both Contacts › Contact details frames from Handoff v62.

Evidence

  • New unit tests, all passing:
    • PaykitPaymentRequestRepoTest.kt: failed single recipient refresh does not overwrite an older full refresh, failed full refresh keeps a newer single recipient result, and single recipient refresh keeps a known target while capability lookup fails, which now asserts isComplete == false.
    • ContactDetailViewModelTest.kt: pay tap rechecks eligibility after an incomplete check, pay tap rechecks eligibility after a failed check, pay tap rechecks eligibility once a checked target leaves the eligible list, leaving the screen while paying cancels the payment, and leaving the screen while the amount screen opens cancels the scan.
    • AppViewModelSendFlowTest.kt: cancelled contact scan clears its payment context.
  • just test: 2960 tests, 0 failures. just compile and just lint report no new findings (the launchScan complexity finding is also on master).
  • Emulator recording (Pixel_9, regtest dev build at def4a3e, contact "Apple" linked with request support):
    1. On Contact Detail, tap Pay and then Activity straight away. Activity stays on screen with no Send sheet opening over it.
    2. Press Back. Contact Detail shows Pay enabled, with no spinner and no leftover Request or Pay sheet.
    3. Tap Pay. Request or Pay opens.
    4. Tap Pay in the sheet. The Send amount screen opens (send_amount_screen, SendNumberField, ContinueAmount in the layout dump).
contact-pay-leave-and-return.mp4

@jvsena42

Copy link
Copy Markdown
Member Author

@coreyphillips all five items from your second review are addressed; details, tests and an emulator recording are in #1349 (comment).

  • Stuck Pay spinner after the effect is emitted with no collector: leaving Contact Detail now cancels the pending Pay job and scan and clears the loading state (ecd7a97).
  • Failed full refresh drops a newer target: the failure handler keeps targets written by a single-contact refresh that started after it (554de3d, failed full refresh keeps a newer single recipient result).
  • Cancelled scan leaves its contact context active: a cancelled scan clears the context it set, if that context is still active (0a1257e, cancelled contact scan clears its payment context).
  • Design section: now links both Contacts › Contact details frames from Handoff v62.
  • Reused check bringing back a dropped target: a checked target is discarded once the contact leaves eligibleTargets (554de3d, pay tap rechecks eligibility once a checked target leaves the eligible list).

@jvsena42
jvsena42 merged commit 24bdc25 into master Sep 29, 2026
21 checks passed
@jvsena42
jvsena42 deleted the fix/contact-pay-request-sheet branch September 29, 2026 16:42
@ovitrif ovitrif removed this from the 2.6.0 milestone Sep 29, 2026
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.

4 participants