Skip to content

feat: toast when a pending payment request expires - #1068

Merged
ovitrif merged 6 commits into
masterfrom
feat/payment-request-expiry-toast
Oct 21, 2026
Merged

ovitrif merged 6 commits into
masterfrom
feat/payment-request-expiry-toast

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Refs #1051
Twin: synonymdev/bitkit-android#1643

Description

  • Shows a toast when a pending incoming payment request expires on its own, so a request no longer turns Expired silently (seen with a Shop order request on 3.0.1). It reuses the existing expired copy: "Payment Request — The payment request has expired."
  • Covers incoming one-time requests the user never accepted or opened, when the expiry timer removes them while the app runs. Several expiring at once give one toast.
  • Skips requests the user opened or accepted (they have their own feedback), requests being paid, subscriptions and outgoing requests. Sign-out and identity switches never trigger it. The toast waits until the app is in front and unlocked, and is dropped when Paykit UI is off.
  • Partial fix of the issue: this is only the toast part. The new Payment Requests screen and the toast's button that opens it wait on the screen's design sign-off.

Out of Scope

  • Payment Requests screen and drawer entry: pending design sign-off.
  • Toast button that opens Payment Requests: comes with that screen.
  • Failed requests: opening or paying a request that fails already shows a toast (request unavailable, or the payment error).
  • Requests that expire while the app is closed: they show as Expired in Subscriptions → Payments.

Design

N/A — reuses the existing expired-request toast.

Preview

A pending request expires on Home: the bell clears and the toast shows.

ios-toast-2x.mp4

QA Notes

Journeys

  • new expiry-toast.xml

Manual Tests

N/A

Automated Checks

  • updated PaykitPaymentRequestServiceTests.swift (an unopened request expiring is reported; an opened or requested one is not)
  • ran SwiftFormat lint and PaykitPaymentRequestServiceTests on a simulator

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T00:17:22.869488Z 95a6aba PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ovitrif ovitrif added this to the 3.2.0 milestone Oct 21, 2026
@ovitrif ovitrif self-assigned this Oct 21, 2026
@ovitrif
ovitrif requested review from a team, ben-kaufman and pwltr and removed request for a team October 21, 2026 00:17

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95a6aba278

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Bitkit/AppScene.swift
Comment thread Bitkit/Services/PaykitPaymentRequestService.swift Outdated
@greptile-apps

greptile-apps Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change reaches the in-app payment-request experience.

The PR appears safe to merge, with a non-blocking case where a paid request can show an incorrect expiration toast.

Findings

  1. P2 Paid requests shown as expired ▶

Summary

Adds a toast when a pending incoming payment request expires.

  • Unopened incoming requests get an expiry toast while the app is open.

Reviews (1) · Last reviewed commit: "feat: toast when a pending payment reque..." · Reviewed by Greptile

Comment thread Bitkit/Services/PaykitPaymentRequestService.swift Outdated
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 6d6866f: the expiry report now comes only from discardExpiredRequests, for proposals never accepted, and skips a request the user opens, so paid or accepted requests are never reported as expired. Adds journeys/payment-requests/expiry-toast.xml and a test for opening an expired request; PaykitPaymentRequestServiceTests pass (269).

@talosmachina talosmachina 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.

No findings. Records unopened incoming one-time proposals that discardExpiredRequests drops and toasts once from AppScene. Reviewed 6d6866f (both commits), full tier.

What I checked, and 4 candidates I ruled out

Read in full: recordSilentExpirations, discardExpiredRequests and all its call sites (requestPresentation, deferPresentation, reconcileExpiredRequests, refresh, actions), scheduleExpiration, recordRequestedPresentationExpiration, the AppScene observer and IncomingPaykitPaymentRequestPresentationFeedback; both new tests in PaykitPaymentRequestServiceTests.swift, and the new journey.
CI: SwiftFormat and validate green; unit and integration tests still running on 6d6866f. Swift is not built on this box, so those jobs are the gate.

Ruled out

  • Two toasts when an automatic presentation finds the request expired: deferPresentation returns .requestExpired(wasRequested: false), whose feedback carries no toast, so the silent path's toast is the only one.
  • Opened request toasted twice: requestPresentation passes openedRequestId, the requested one is excluded by requestedPresentationId, and its own expiry goes through handledRequestedExpirationId.
  • Paid or accepted request past its deadline shown as expired: the filter now requires .proposed, which also matches what scheduleExpiration schedules.
  • Toast over the request being paid: AppScene drops the id from app.contactPaymentContext.

Merge confidence: 4/5, the unit tests are still running on the new head and Swift is not built here.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan at 6d6866f: one LOW, inline with a failing test. Nothing above LOW.

  • No false toasts: paid, accepted and in-flight requests are excluded, and so is the opened request on the presentation path. Identity clear and activation don't emit.
  • Strings: reuses existing keys.
  • Journey: new payment-requests/expiry-toast.xml, with the same text as Android.
  • Parity with synonymdev/bitkit-android#1643: iOS skips only the request being paid during a payment, where Android drops every expiry toast. iOS also counts payment-deadline expiry. Both behaviours are reasonable.

Device gate (partial): 6d6866f — not run yet. Pending: expiry-toast.xml, plus backgrounding the app across the expiry and resuming, to see whether the toast shows, shows late or is lost.

Device results will follow in a separate review.

Comment thread Bitkit/Services/PaykitPaymentRequestService.swift
Comment thread Bitkit/Services/PaykitPaymentRequestService.swift
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 1466d65: fixes both LOWs (expired toast twice on dismiss/decline; silent expiry missed when a refresh drops the request first), each with a test.

@ovitrif
ovitrif requested review from jvsena42 and pwltr October 21, 2026 12:58
Comment thread Bitkit/Services/PaykitPaymentRequestService.swift
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed ca81e9e: parity with synonymdev/bitkit-android#1643 (raised there by @coreyphillips). A refresh no longer reports an expired proposal that the snapshot still keeps, so the expiry timer can't report the same request a second time.

@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed bebfdf4: each expired request is now reported once, even when a refresh that started before the expiry puts it back (@pwltr's P3), with a test.

coreyphillips
coreyphillips previously approved these changes Oct 21, 2026

@coreyphillips coreyphillips 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.

Two independent reviews, nothing blocking a merge.

worth doing, does not block

  • Expiry toast is not gated on unlock, scene phase or the Paykit UI flag (Bitkit/AppScene.swift:1830). toastSilentlyExpiredPaymentRequests calls app.toast with no unlock, scene-phase or Paykit flag check, so the toast can show over the PIN screen and is then never repeated. Toasts render in a window at .alert + 1 (ToastWindowManager.swift:174), above the AuthCheck PIN screen. With PIN enabled, going to background sets isPinVerified = false. A request that expires while the app is suspended is reported when the expiry task resumes on foreground. The toast then shows over the PIN pad and hides after 4 seconds. Because reportedSilentExpirationIds already holds the id, it never shows again after unlock. The same loss happens for an expiry in the few seconds after backgrounding, while the app still runs but is not visible. Automatic presentation already waits for isPinVerified || !settings.pinEnabled and scenePhase == .active. There is also no PaykitFeatureFlags.isUIEnabled check. Disabling Paykit UI from Dev Settings (ContactPaymentsService.disablePaykitUI) does not clear the manager, so a pending request still toasts after the bell is hidden. The Android twin returns early when isPaykitEnabled is false. Found by reading the code. Not reproduced on a device. A fix is to hold the toast until the scene is active and unlocked, and to check the flag.
  • A request expiring mid-decline is reported as silently expired (Bitkit/Services/PaykitPaymentRequestService.swift:2813). If a request expires while its Decline call is in flight, the user sees "The payment request has expired." right after tapping Decline. recordSilentExpirations skips requestedPresentationId and the opened id but not processingRequestIds. Decline from the Payment Requests list or details runs dismiss, then reject, then perform. perform keeps the id in processingRequestIds while service.reject runs. If the expiry timer fires during that call, reconcileExpiredRequests reports the request. If the SDK then refuses the reject, app.toast(error) shows a second toast. The pay path avoids this only because AppScene compares against contactPaymentContext. Skipping processingRequestIds in the filter would cover both cases inside the manager, where a unit test can reach it. This is a narrow race, found by reading the code.
  • Explicit expiry feedback can be followed by a duplicate toast (Bitkit/Services/PaykitPaymentRequestService.swift:2815). An in-flight refresh can restore a request after its explicit expiry feedback and trigger another toast. Hold a refresh after its snapshot is read, expire a request with requestedPresentationId set, then resume the refresh. The requested-expiry path never adds the ID to reportedSilentExpirationIds, so the final discardExpiredRequests reports it again. Running the extracted manager methods confirmed that both expiry triggers increment for the same request.

nits

  • clear() leaves silentlyExpiredRequests holding the previous identity's requests (Bitkit/Services/PaykitPaymentRequestService.swift:2238). clear() resets reportedSilentExpirationIds but not silentlyExpiredRequests. After sign-out or an identity switch, the manager keeps the last batch of expired requests from the previous identity, including notes and counterparty, until the next report. Nothing reads the array unless silentExpirationTrigger changes, so this is cleanup only. Resetting it next to reportedSilentExpirationIds would match the rest of clear().

@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks. Fixed all four in 6a17ec6:

  • Toast behind the PIN screen or with Paykit UI off: AppScene now holds the toast until the scene is active and unlocked (isPinVerified || !settings.pinEnabled, as automatic presentation does), shows it on unlock or return to the foreground, and drops it when Paykit UI is off.
  • Expiring mid-decline: recordSilentExpirations skips processingRequestIds. testRequestExpiringWhileBeingActedOnIsNotReportedAsSilentExpiration pauses an accept, expires the request, and checks no report.
  • Requested expiry followed by a duplicate: the requested-expiry path now marks the id as reported. testRequestedExpiryRestoredByRefreshIsNotReportedAgain covers a refresh restoring it.
  • clear() now resets silentlyExpiredRequests too.

@coreyphillips coreyphillips 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.

Rechecked after 1 new commit (6a17ec6). Nothing blocking remains.

addressed

  • Expiry toast is not gated on unlock, scene phase or the Paykit UI flag (Bitkit/AppScene.swift:1830). Fixed: showPendingExpiryToast (AppScene.swift:1841-1847) drops the toast when PaykitFeatureFlags.isUIEnabled is false. Otherwise it holds it until scenePhase == .active and isPinVerified || !settings.pinEnabled hold. It retries from .onChange(of: isPinVerified) (544) and .onChange(of: scenePhase) (712). I traced background, then foreground with PIN: line 1275 clears isPinVerified first, so the toast now waits for the PIN entry. Checked by reading only; no Xcode on this machine.
  • A request expiring mid-decline is reported as silently expired (Bitkit/Services/PaykitPaymentRequestService.swift:2813). Fixed: recordSilentExpirations now skips processingRequestIds (PaykitPaymentRequestService.swift:2817). perform inserts the id before service.reject and removes it in a defer, after the catch path's refresh(excludingProtectedRequestId:). So neither the timer nor that refresh reports a request mid-decline. Without the line, the new test's reconcileExpiredRequests call would bump the trigger to 1. I read the test, I did not run it.
  • clear() leaves silentlyExpiredRequests holding the previous identity's requests (Bitkit/Services/PaykitPaymentRequestService.swift:2238). Fixed: clear() now resets silentlyExpiredRequests next to reportedSilentExpirationIds (PaykitPaymentRequestService.swift:2238-2239).

still open

  • Explicit expiry feedback can be followed by a duplicate toast (Bitkit/Services/PaykitPaymentRequestService.swift:2815). An in-flight refresh can restore a request after its explicit expiry feedback and trigger another toast. Hold a refresh after its snapshot is read, expire a request with requestedPresentationId set, then resume the refresh. The requested-expiry path never adds the ID to reportedSilentExpirationIds, so the final discardExpiredRequests reports it again. Running the extracted manager methods confirmed that both expiry triggers increment for the same request. Line 2840 marks the id when recordRequestedPresentationExpiration fires, which covers the timer and refresh paths. The new test covers exactly that. deferPresentation passes the expired requested id as handledRequestedExpirationId, so the guard at 2833 returns before the insert. Its PaymentRequestExpiredToast feedback (AppScene.swift:39) therefore leaves the id unmarked. So does discardExpiredRequests(openedRequestId:) from perform (2702), whose requestExpired error a Decline toasts. In both cases a refresh holding an older snapshot can restore the request and toast it again. Inserting the handled and opened ids in discardExpiredRequests would close both.

new since the last review

worth doing, does not block

  • Held expiry toast survives a sign-out behind the PIN screen (Bitkit/AppScene.swift:628). hasPendingExpiryToast is not reset when the Pubky profile is cleared, so a toast held behind the PIN screen still shows after a sign-out. Sequence: a request expires while the app is backgrounded, so queueSilentExpiryToast sets the flag and waits. On return, handleScenePhaseChange runs pubkyProfile.checkAdoptedSource() and restoreSessionIfNeeded() behind the PIN screen. Either can call clearAuthenticatedState, which sets authState = .idle. AppScene.swift:628 then calls paykitPaymentRequestManager.clear() but leaves hasPendingExpiryToast set. After unlock the user sees "source lost" or "session expired", then "The payment request has expired" for a request of the profile that was just signed out. The request really did expire, so the effect is small. But the PR says sign-outs never lead to this toast. Resetting the flag next to the clear() call at line 628 would fix it. Found by reading; not reproduced.
  • Deferred expiry toast survives sign-out (Bitkit/AppScene.swift:1842). Expiry feedback queued behind the PIN screen survives sign-out. For an adopted identity, let a request expire in the background, remove the identity in its source app, then return to Bitkit. checkAdoptedSource() clears authentication, but the session observers never reset hasPendingExpiryToast. Unlocking then shows the previous session's expiry toast. Exercising the extracted callbacks confirmed that clearing the manager leaves this toast queued.

@ovitrif
ovitrif merged commit e7a21e8 into master Oct 21, 2026
22 of 24 checks passed
@ovitrif
ovitrif deleted the feat/payment-request-expiry-toast branch October 21, 2026 18:07
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks, merged. The sign-out reset is in #1093.

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.

5 participants