[ovitrif]

Refs #1624
Twin: synonymdev/bitkit-ios#1068

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 the request the user opened, the one being paid and accepted requests (they have their own feedback), subscriptions and outgoing requests. Each expired request gets one toast, also when a refresh overlaps the expiry. Sign-out and identity switches never trigger it.
• 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.

toast-2x.mp4

QA Notes

Journeys

• new expiry-toast.xml

Manual Tests

N/A

Automated Checks

• updated AppViewModelSendFlowTest.kt (expired proposal toasts once; repeated and opened-request signals toast once; an unrelated request expiring during a send still toasts)
• updated PaykitPaymentRequestRepoTest.kt (expiry timer and refresh report each expired proposal once, including a refresh crossing the expiry)
• ran compileDevDebugKotlin, detekt, lintDevDebug, AppViewModelSendFlowTest and PaykitPaymentRequestRepoTest

[chatgpt-codex-connector]

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:20:33.069267Z	0d06387	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.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change affects payment-request expiry notices in a bounded app workflow.

The PR appears safe to merge, with a non-blocking duplicate-toast issue when an opened request expires.

Findings

1.  Expiry can show two toasts ▶

Summary

This PR adds a toast when an unopened, one-time incoming payment request expires.

• Unopened incoming payment requests now show a toast when they expire.

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d06387b84

ℹ️ 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".

[github-actions]

Regtest APK

Built from d8cce5a (run).

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

[ovitrif]

Pushed 493b7de: the expiry toast now comes only from the repository expiry timer, for proposals never accepted, and is skipped during an active payment submission or for a request that already got the opened-request toast. This answers the double-toast, in-flight, baseline-race and identity-switch threads. Adds journeys/payment-requests/expiry-toast.xml and repository and view-model tests.

[talosmachina]

No findings. Toasts once when an unopened incoming one-time request is dropped by the repository's expiry timer, through a new expiredProposals flow. Reviewed 493b7de (both commits), full tier.

What I checked, and 5 candidates I ruled out

Read in full: the AppViewModel.kt and PaykitPaymentRequestRepo.kt hunks with their callers: discardExpiredRequestsLocked (all four call sites), scheduleExpirationLocked, applyRequestSnapshotLocked, clearStateLocked, finishExpiredPaymentRequestPresentation, retainPaymentRequestPresentationState; the test changes in PaykitPaymentRequestRepoTest.kt and AppViewModelSendFlowTest.kt, and the new journey.
CI: build, build-local and lint still running on 493b7de. Not built here: this turn has no packages token, so CI is the gate.

Ruled out

• Double toast on an opened request: the action path passes handledRequestId = request.id to discardExpiredRequestsLocked, and finishExpiredPaymentRequestPresentation records the id in expiredFeedbackRequestIds before its own toast, so a later emission is filtered.
• Toast during an active payment: toastExpiredPaymentRequests returns while isPaymentSubmissionActive is true, and the repo filter skips isPaymentInFlight.
• Accepted request past its payment deadline reported as expired: the filter uses isProposalExpired, which requires PROPOSED.
• Old account's requests toasted on sign-out or identity switch: clearStateLocked empties the list without going through discardExpiredRequestsLocked, so nothing is emitted.
• Emission lost before subscription: the emission now comes from a SharedFlow collected from init, and the StateFlow baseline read the first commit had is gone.

Merge confidence: 4/5, CI is still running on the new head and nothing was built here.

[jvsena42]

Code scan at 493b7de: no issues found.

• One toast per expiry: the opened-request toast and the timer toast can't both fire, in either order. The acted-on request is passed as handled (PaykitPaymentRequestRepo.kt:1562).
• No false toasts: identity clear and activation don't emit. Paid, accepted and in-flight requests are excluded.
• Strings: reuses existing keys present in all locales.
• Journey: new payment-requests/expiry-toast.xml, with the same text as iOS.
• Parity: this side drops expiry toasts while a payment is being submitted, and counts only proposal expiry. iOS skips only the request being paid, and also counts payment-deadline expiry. See synonymdev/bitkit-ios#1068.

Device gate (partial): 493b7de — 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.

[ovitrif]

@jvsena42 thanks. Pushed 85ff0d1 for the parity point: an unopened proposal now also gets the toast when its payment deadline passes first, matching iOS (synonymdev/bitkit-ios#1068). The other difference stays by design: Android skips the toast during any active payment submission, iOS skips the request being paid; both keep the toast away from a payment in progress.

[jvsena42]

Delta at 85ff0d1: no issues found. The expiry toast now also fires when an unopened request passes its payment deadline. The match is PROPOSED plus isExpired, instead of proposal expiry only (PaykitPaymentRequestRepo.kt:1750-1751).

• Same exclusions: the handled request, outgoing requests, subscription periods and in-flight payments are still left out.
• No other states: accepted or paid requests aren't PROPOSED, so they don't toast.
• Parity: iOS (synonymdev/bitkit-ios#1068) already counted payment-deadline expiry, so the two platforms now agree there.

Device gate (partial): 85ff0d1 — not run yet. Pending: expiry-toast.xml, plus one request that reaches its payment deadline while unopened, and backgrounding the app across the expiry.

[ovitrif]

Pushed 47aa0b8, parity with the iOS twin (synonymdev/bitkit-ios#1068, raised there by @pwltr): a refresh drops expired proposals while parsing, so when the wall clock passed expiry before the timer fired (clock jump, suspend) the toast was missed. The refresh now reports expired proposals from the list it replaces; new PaykitPaymentRequestRepoTest case covers it.

[coreyphillips]

Two independent reviews, nothing blocking a merge.

worth doing, does not block

• Refresh can report a request it keeps, then the timer reports it again (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1747). A refresh that runs while a request expires can show the expired toast twice. applyRequestSnapshotLocked parses the snapshot with the now it reads at line 1259. replacePendingRequests then checks the old list against a later clock.now() (line 1747). A request whose expiresAt falls between the two passes parsing, so it stays in incoming, and it is also emitted as expired. scheduleExpirationLocked then sees a past expiresAt and schedules a zero-delay job. discardExpiredRequestsLocked emits the same request again. toastExpiredPaymentRequests dedupes only against expiredFeedbackRequestIds, which this path never fills, and ToastQueueManager.enqueue does not dedupe, so two identical toasts show. The window is the stretch of datastore and USDT repo calls between those two lines, so it needs the 10 second poll to land as a request expires. I traced this from the code. I could not run a reproduction because the Synonym packages do not resolve on this machine. Either fix closes it: pass the parse now into replacePendingRequests, or report only old ids that are missing from incoming.
• Any active payment submission drops expiry toasts for unrelated requests (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1805). toastExpiredPaymentRequests returns early for the whole batch whenever isPaymentSubmissionActive is true. The signal fires once, and the requests are already gone from pendingRequests, so nothing re-reports them. If request B expires while the user pays something else, B never gets a toast. Hardware signing keeps submission active for the whole signing job (HwSendViewModel.kt:101), which can last a minute or more. The description says only requests being paid are skipped. The repo already covers that case: it filters out in-flight requests and the handledRequestId, and accepting a one-time request removes it from the pending list before payment. Skipping only the active request, or holding the toast until submission ends, would match the description.
• No test sends both expiry signals for an opened request (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt). Showing one toast for an opened request depends on two collectors running independently. onPendingPaykitRequestsChanged calls finishExpiredPaymentRequestPresentation, which fills expiredFeedbackRequestIds. toastExpiredPaymentRequests skips requestedPaymentRequestId and that set. I traced both orderings and each gives one toast. The existing tests named expired explicit request shows the expired toast once and explicit request expiring during backoff shows the expired toast once never emit on expiredPaykitProposals. In production both signals fire together, so this new dedupe path is not exercised. A test should open a request, clear pendingPaykitPaymentRequests, emit the same request on expiredPaykitProposals, and verify exactly one enqueue.
• A refresh across the deadline emits the same expiry twice (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1747). A refresh crossing the deadline can show the expired toast twice. Parsing uses an earlier now, so incoming can still contain the request when this later clock read reports it expired. The immediate expiry timer then removes it and emits it again. Executing the extracted replacement, timer, and toast methods with the clock advanced between parsing and replacement produced two events and two toasts for one request.
• Unrelated payment submissions permanently discard expiry notifications (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1805). An unopened request still expires silently if an unrelated payment is submitting. Ordinary wallet sends also set this global flag, so this return discards unrelated expiry events. The flow has no replay, and the expired request has already been removed, so completing the send cannot recover the notification. Executing the extracted handler confirmed zero toasts after the flag returned to false.

nits

• KDoc on new private functions (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1752). replacePendingRequests, expiredProposals and toastExpiredPaymentRequests are private but have KDoc. The repo rules say not to comment private functions. The expiredProposals KDoc also says "opened", but the repo does not check that; the view model does.
• Test name says the request did not expire, but its stub says it did (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:2693). pending request removed without expiring shows no expired toast stubs isExpired(request) to return true, so the request did expire. The test actually proves that removing a request without the expiredProposals signal shows no toast. The name should say that.

[ovitrif]

@coreyphillips thanks. Fixed the points worth doing in e775d63:

• Refresh reporting a request it keeps: replacePendingRequests now reports only expired proposals missing from incoming, so the timer can't report one again. Same fix on the iOS twin (feat: toast when a pending payment request expires bitkit-ios#1068, ca81e9e2).
• Payment submission dropping unrelated expiry toasts: removed the global isPaymentSubmissionActive check. The repo already excludes the handled, in-flight and accepted requests, so an unrelated request expiring during a send now gets its toast (test flipped to cover that).
• Both signals for an opened request: new test opens a request, clears the pending list, emits it on expiredProposals, and checks one toast.
• Nits: dropped the KDoc on the private functions, renamed the test to pending request removed without the expiry signal shows no expired toast.
  Re-requesting your review on the new head.

[ovitrif]

Pushed 503d4f7, parity with synonymdev/bitkit-ios#1068 (@pwltr's P3 there): a refresh that started before an expiry can restore the request, so the view model now toasts each expired request once. New test emits the same request twice and checks one toast.

[coreyphillips]

Rechecked after 2 new commits (e775d63, 503d4f7). Nothing blocking remains.

addressed

• Refresh can report a request it keeps, then the timer reports it again (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1747). Fixed: PaykitPaymentRequestRepo.kt:1747-1748 now drops any id the snapshot keeps. A request that expires between the two clock reads is no longer reported by the refresh, and the zero-delay timer scheduled at line 1410 reports it once. The view model also dedupes per id now (AppViewModel.kt:1806-1809). I traced this and did not run it. The new repo test does not reproduce the window (filed separately).
• Any active payment submission drops expiry toasts for unrelated requests (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1805). Fixed: AppViewModel.kt:1804 no longer returns on isPaymentSubmissionActive, so a request that expires during an unrelated send gets its toast. The test at AppViewModelSendFlowTest.kt:2710 now asserts that. Removing the whole guard brought a smaller problem for the request actually being paid, filed as a new finding.
• KDoc on new private functions (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1752). Fixed: KDoc is gone from replacePendingRequests, expiredProposals(now, ...) and toastExpiredPaymentRequests. The public expiredProposals KDoc (PaykitPaymentRequestRepo.kt:362) no longer claims to exclude opened requests. The inline comments that remain give a reason, as elsewhere in the file.
• Test name says the request did not expire, but its stub says it did (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:2693). Fixed: AppViewModelSendFlowTest.kt:2693 is now pending request removed without the expiry signal shows no expired toast. That matches what it asserts.
• A refresh across the deadline emits the same expiry twice (app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt:1747). Fixed: Same fix as point 1. The keptIds filter at PaykitPaymentRequestRepo.kt:1747-1748 stops the refresh from emitting a request it keeps, so only the timer's discardExpiredRequestsLocked reports it. The view model's per-id set would absorb a repeat anyway.
• Unrelated payment submissions permanently discard expiry notifications (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1805). Fixed: Same fix as point 2. The global flag no longer gates toastExpiredPaymentRequests (AppViewModel.kt:1804), so expiry events during an unrelated send are no longer discarded.

still open

• No test sends both expiry signals for an opened request (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt). Showing one toast for an opened request depends on two collectors running independently. onPendingPaykitRequestsChanged calls finishExpiredPaymentRequestPresentation, which fills expiredFeedbackRequestIds. toastExpiredPaymentRequests skips requestedPaymentRequestId and that set. I traced both orderings and each gives one toast. The existing tests named expired explicit request shows the expired toast once and explicit request expiring during backoff shows the expired toast once never emit on expiredPaykitProposals. In production both signals fire together, so this new dedupe path is not exercised. A test should open a request, clear pendingPaykitPaymentRequests, emit the same request on expiredPaykitProposals, and verify exactly one enqueue. Codex still sees it: AppViewModelSendFlowTest.kt:2760 makes opening fail immediately with RequestExpired, so line 2772 finishes expiry feedback before either flow changes. The test still needs to keep the request open or retrying until pending-list removal triggers feedback, then emit expiredPaykitProposals.

new since the last review

worth doing, does not block

• Request being paid can show two toasts if it expires before acceptance (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1806). A request that expires in the gap between tapping Pay and accept() now gets the new expired toast and then an error toast from the payment path. The removed isPaymentSubmissionActive guard was what suppressed the first one. None of the exclusions the reply relies on covers this gap: - onSheetVisible already cleared requestedPaymentRequestId (AppViewModel.kt:1451). - The pending entry is still PROPOSED with isPaymentInFlight = false. That flag only changes on a snapshot refresh, and observePaymentProofChanges suppresses refreshes while submission is active (line 1409). - handledRequestId only applies when updateRequest notices the expiry itself. Traced sequence (no run, Synonym packages are not cached here): 1. onConfirmPay sets isSubmittingPaymentRequest. 2. proceedWithPayment waits SCREEN_TRANSITION_DELAY, then runs proof prepare, claimForPayment and private list consumption. 3. The proposal expiry passes in that time. The timer's discardExpiredRequestsLocked() emits the request, and toastExpiredPaymentRequests shows "Payment Request / The payment request has expired." 4. Then either validateIncomingPaymentRequest finds it not pending and toasts RequestUnavailable, or accept() throws RequestExpired and toast(error) shows "Error / Payment request has expired". The hardware path through prepareContactPayment has the same gap. The description still says requests being paid are skipped. Its Automated Checks line ("active submission do not") no longer matches the flipped test. Skipping activeIncomingPaymentRequest()?.id while isSubmittingPaymentRequest is set would keep the unrelated-request fix and restore the old behaviour for the active one.
• New repo regression test passes against the code it was meant to fix (app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt:1320). refresh does not report a proposal the snapshot still keeps does not reproduce the gap between the parse now and the later clock.now(). The test clock is fixed within a refresh. With clockJumpMillis = 5_000 and expiry at +10s, the request is unexpired at both reads, so the old expiredProposals(clock.now()) also reported nothing and the test passes without the keptIds filter. To cover it, move the clock between the two reads, for example by having the usdtPayments.verifiedReceipts stub set clockJumpMillis = 10_000. Then assert that the refresh reports nothing and the zero-delay timer reports the request exactly once.

[ovitrif]

@coreyphillips thanks for the recheck. Pushed d8cce5a for the remaining points:

• Request being paid showing two toasts: the expiry toast now skips the request in the active contact payment (the one being paid), as well as the opened one, matching the iOS twin. Unrelated requests that expire during a send still get their toast.
• Repo test not covering the window: refresh crossing expiry leaves the report to the expiry timer now moves the clock between parsing and the list replace (from the verifiedReceipts stub) and asserts one report overall, from the timer. It fails without the keptIds filter and passes with it.
• Both signals for an opened request: the test now keeps the request retrying (waiting for payment details), so pending-list removal drives the expiry feedback, then emits expiredProposals; it asserts one expired toast.

[coreyphillips]

Rechecked after 1 new commit (d8cce5a). Nothing blocking remains.

addressed

• No test sends both expiry signals for an opened request (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt). Fixed: AppViewModelSendFlowTest.kt:2753-2784 now keeps the opened request retrying (WaitingForUpdatedPaymentList), so requestedPaymentRequestId stays set. It then clears pending, emits expiredPaykitProposals and asserts one PaymentRequestExpiredToast. Traced: the pending collector runs first and toasts via finishExpiredPaymentRequestPresentation, which adds the ID at AppViewModel.kt:1834. Without that add, the toast collector would toast again, so the test guards the dedupe. Not run: the LDK and Synonym dependencies are not cached here.
• Request being paid can show two toasts if it expires before acceptance (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1806). Fixed: AppViewModel.kt:1806 now also excludes activeIncomingPaymentRequest()?.id. Traced: onConfirmPay sets isSubmittingPaymentRequest (line 7085) before the hardware branch. onPendingPaykitRequestsChanged then counts the request as protected (line 1388) and keeps the active context, so the expiry toast is skipped and only the payment path's toast shows. Unrelated requests still toast (test at line 2710), and the description now matches.
• New repo regression test passes against the code it was meant to fix (app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt:1320). Fixed: PaykitPaymentRequestRepoTest.kt:1331 moves the clock inside the verifiedReceipts stub. That is after refresh reads now (PaykitPaymentRequestRepo.kt:1259) and before replacePendingRequests reads clock.now() (line 1748). Traced: without keptIds, both the replace and the zero-delay timer report the request, which is two entries, so the single-report assertion fails. With the filter only the timer reports. Not run, same dependency gap.

new since the last review

worth doing, does not block

• No test covers skipping the expiry toast for the request being paid (app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt:2710). Removing activeIncomingPaymentRequest()?.id from handledIds at AppViewModel.kt:1806 would leave every test green. I checked all five expiredPaykitProposals.emit calls (lines 2683, 2723, 2744, 2746, 2776). None runs with an active contact payment context. The test at 2710 covers only the unrelated request. Point 2's fix is therefore unguarded. A test should open a request into the Send sheet and confirm pay so submission is active. It should then clear pending, emit the same request on expiredPaykitProposals, and assert no PaymentRequestExpiredToast.

nits

• Repo test comment places the clock jump after parsing (app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt:1329). The comment says the clock passes expiry "after the snapshot is parsed". But verifiedReceipts runs at PaykitPaymentRequestRepo.kt:1282, before parsing at line 1350. The test works because parsing uses the now read at line 1259. "After the refresh reads its time" would be accurate.

[ovitrif]

@coreyphillips thanks, merged. The test for the request being paid and the comment fix are in #1660.