fix: expose rejected incoming payment requests

[ovitrif]

Fixes #714

Expose rejected incoming payment requests through privacy-safe diagnostics and terminal user feedback while preserving automatic recovery.

Description

• Classify malformed incoming requests with stable parse reason codes and log only the redacted counterparty.
• Log each permanent parse rejection once per record and reason.
• Classify endpoint-resolution and payment-target failures with redaction-safe error categories and codes without logging request contents or raw errors.
• Keep automatic presentation retries running every 120 seconds after the initial retry window without repeated warning logs.
• End an explicit Pay attempt after fifteen failures with the localized Payment Request unavailable toast while leaving the request actionable.
• Preserve requested expirations across suspended resolution and retry backoff, then show the localized expired toast exactly once.
• Document the failure contract and the ported Android resolution-failure journey.

Linked Issues/Tasks

• fix: expose rejected incoming payment requests #714
• Parent: epic: Pubky marketplace wallet support #712

Preview

pr721-payment-request-unavailable-2x.mp4

QA Notes

Manual Tests

• 1. iPhone 17 simulator on iOS 26.5/regtest → pair with an iPhone Air Paykit peer: both apps exchange private Paykit data.
• 2. Create profiles on both simulators → save each profile as a contact on the other → send a 1 sat incoming payment request: the request appears on the payer.
• 3. Peer contact-payment settings → disable payment endpoints → payer Payment Requests → tap Pay: explicit resolution retries begin.
• 4. Payment Requests → wait through fifteen explicit attempts: logs show category=resolution reason=no_supported_endpoint and redact the counterparty.
• 5. Fifteenth failed attempt → inspect terminal feedback: PaymentRequestUnavailableToast shows Payment Request and The payment request is no longer available.
• 6. Dismiss terminal feedback → inspect PaymentRequestRow-7abfa801-a3bd-4d74-b75a-18be91d2ddbf: PaymentRequestPay-7abfa801-a3bd-4d74-b75a-18be91d2ddbf remains enabled for another attempt.

Automated Checks

• Focused simulator tests after the base sync: 77 passed across PaykitPaymentRequestServiceTests and PublicPaykitServiceTests.
• Prior focused simulator tests: 72 passed across PaykitPaymentRequestServiceTests and PrivatePaykitServiceTests.
• Prior focused simulator tests: 73 passed across PaykitPaymentRequestServiceTests and PublicPaykitServiceTests.
• Simulator build-and-run: passed on iPhone 17.
• SwiftFormat lint, translation validation, journey XML validation, and changed diff check: passed.
• Android #1217 verification: matching requested-resolution-failure.xml journey confirmed at b79c43d.
• Current head: ffdfd20 on master 60e75e1; signed rebase range-diff passed.
• GitHub Actions: running for ffdfd20.

[ovitrif]

@greptileai

[ovitrif]

The rewritten head preserves the earlier expiry fix. I found two non-blocking coverage and journey-documentation gaps.

[jvsena42]

One nit, arising from reviewing the Android counterpart (#1217) rather than from this PR's own code: the iOS-only label on this suite stops being true once #1217 lands.

[jvsena42]

Reviewed this against the Android twin (synonymdev/bitkit-android#1217), where I confirmed a real regression. iOS is structurally better and does not have it — worth recording why.

Android bumped its presentation generation unconditionally when a request turned out expired, including for automatic presentations, so an expired request A invalidated the batch and request B behind it was fully resolved over the network and then discarded. Here, presentRequests snapshots activePresentationGeneration for the closure (:888-901) and isCurrentPresentation (:905-912) is checked at AppScene.swift:950 — before the beginPaymentRequest network call at :952 — and again at :953-957, :983, :1012, :1027, :1038. So even when the generation does move, no resolution work is wasted. The new expired branch also returns at :946 before the bump, and discardExpiredRequests only bumps when requestedPresentationId itself is dropped (:1110-1113), which is nil for automatic presentations.

The other two Android bugs are absent too: .expired is effectively unreachable because the SDK derives ProposalExpired at query time and parse checks state != .proposed first (returning .nonActionableState, which is excluded from logging), and synchronizationDate is sampled before the await. And the toast queues can't both be non-empty in one transition — performRefresh guards on requestedId != handledRequestedExpirationId (:1019) and deferPresentation passes it through (:942-948).

Also verified: the expiry double-now() race is fixed (one presentationDate threaded into discardExpiredRequests(at:), :940-945); all five new toast strings exist in en.lproj/Localizable.strings:1477-1481; persistPresentedRequestIds() runs before every discardExpiredRequests return path.

Two non-blocking logging notes inline.

[jvsena42]

Re-reviewed at 5e4bebd. No HIGH/MEDIUM. Nothing new to file — the two candidates I chased both died on verification (details below so they aren't re-chased).

Fund safety / authorization — clean. Amount and counterparty are pinned at approve time and this PR does not move that. SendSheet calls markPresentedIfPending(request) (clearing requestedPresentationId) and sets sendAmountSats from the request; confirmation goes prepareIncomingPaymentRequest → prepareForPayment → perform, which re-checks expiry, pending membership, and single-flight via processingRequestIds. approvedPaymentRequestIds is set only after service.accept succeeds. No path added here re-enters payment: every deferPresentation return value only affects retry scheduling and toasts, and .requestedPresentationEnded marks the request presented and drops the id, so a new Pay tap must go through requestPresentation again. Because markPresentedIfPending nils the id on send-sheet open, the perform expiry path can't enqueue a second "expired" toast alongside the send flow's own error.

Trust boundary — clean, and this was the main thing I wanted to check given the PR surfaces counterparty-side failures. Toasts render only static localization keys; logs carry enum raw values, safeCode ([a-z0-9_-], ≤64 bytes), a metatype name, and the 12-char redacted pubkey prefix. PaykitError.context is dropped everywhere. No homeserver- or counterparty-supplied string reaches the UI or a log verbatim.

Also traced clean: no persisted schema change (PresentationStore.State and PaykitPaymentRequest.ID untouched; ParseFailure and IncomingPaykitPaymentRequestFailureReason are never persisted), so no migration risk from the previously shipped build. parse accepts exactly the set base accepted for both actionable and history records — only failure reporting changed. Both feedback queues drain with while let under a single trigger comparison, so SwiftUI coalescing several increments into one onChange can't drop a toast. Every deferIncomingPaykitPaymentRequestPresentation call site is preceded by isCurrentPresentation(request) with no await in between, so there's no TOCTOU on the requester across the suspension. No seed-derived material touched.

Android parity (#1217): the sheet-restore-cancelled-by-hideSheet bug is structurally absent here — the sheet hides itself before requesting presentation and never restores, terminal feedback lands as a toast on whatever is underneath. The peer-supplied-invoice-logged-on-decode-failure issue is absent too; this PR actually removed the base Logger.warn("...: \(error)") at that site in favour of reason=invalid_payment_target, which is the right direction. Expiry-during-backoff is present on both and handled here via recordRequestedPresentationExpiration + expirationTrigger.

Still dev/QA-facing today (isUIEnabled defaults false, Dev Settings only), so none of this is user-reachable until the flag flips.

[jvsena42]

Reviewed the delta at dc9d1068. No HIGH/MEDIUM. One observation below — I did not put it through a second-stage pass, so treat it as an observation rather than a verdict.

No user-facing signal is dropped, which was the thing worth checking on a commit that suppresses warnings. The change gates only the Logger.warn in presentIncomingPaykitPaymentRequestFeedback behind feedback.isTerminal; the toasts are byte-for-byte unchanged, and .retryScheduled/.ignored already carried toast = nil before this commit. Removing the per-attempt noise is a real improvement — 14 retries at 2s then every 120s per stuck request, into a 5 MB session log, is the same repetition class I flagged on the :431 thread.

isTerminal is a let derived from the deferral case in the struct init, so there's no latch, nothing keyed by request id, and nothing to reset on identity switch, sign-out or relaunch — the persistence and concurrency questions I'd normally ask of a "warn once" mechanism don't arise here. The struct stays Equatable and both sides of the handleStateChange equality tests construct through the same init, so the added field can't desync them.

The observation: an automatic (non-requested) presentation can't reach a terminal deferral — .requestedPresentationEnded is only returned under isRequestedPresentation, and the automatic branch falls through to .retryScheduled indefinitely. So for an automatic request that keeps failing, AppScene now logs nothing on any attempt. For thrown resolution errors that's harmless, since PrivatePaykitService+Payments still logs each one redacted. But for .noEndpoint → noSupportedEndpoint, .notOpened → endpointNotPayable, and the AppScene-originated .paymentTargetNotRoutable / .invalidPaymentTarget, that warn was the only log line, so those reasons are now unobservable for automatic presentations. That is the same as base behaviour — master defers those without logging too — so it's a scope note against "expose rejected incoming payment requests" rather than a regression.

If you want the middle ground: log the first deferral of an automatic request rather than none, e.g. carry isFirstAttempt from presentationRetryAttempts[request.id, default: 0] == 0 and widen the gate to isTerminal || isFirstAttempt. A level downgrade wouldn't help — Logger.debug and Logger.warn both go through the same unfiltered handle.

On the tests: they pin the flag rather than the effect. The suppression itself lives in a private AppScene method using the static Logger, so isTerminal == false / == true is what's asserted; against 5e4bebdf8 they fail at compile time rather than semantically. Worth knowing if the gate at :1099 is ever edited.

[jvsena42]

No findings. Clean at the HIGH/MEDIUM bar.

The delta since my last pass is one commit (00f15f348, "log initial payment request failure"), which takes the middle ground I offered — logging the first deferral of an automatic request rather than none. I checked it only for over-correction, not to reopen the choice:

• Shape landed as once per (request.id, reason) for automatic presentations, backed by automaticPresentationDiagnosticReasons. Bounded at 7 reasons per pending request; the dict is filtered to pending ids on every refresh, on expiry, on markPresentedIfPending, on perform success, and on clear(). No unbounded growth, nothing persisted, nothing keyed across identities.
• Requested (explicit Pay) presentations are untouched: wasRequestedPresentation is captured before the inner call can nil the id, and the guard returns false for anything requested or non-.retryScheduled. Terminal outcomes still force shouldLogDiagnostic = true in the struct init independent of the tuple, and .ignored stays silent.
• Toasts are byte-identical; only diagnosticMessage(for:) gates the log.
• testPresentationDispatcherLogsFirstAutomaticFailurePerReasonAndLifecycle now pins the effect — message text, repeat suppressed, reason change re-logs, record removal and reappearance re-logs — which addresses the "tests pin the flag not the effect" note from my last review.
• IncomingPaykitPaymentRequestFailureReason going Equatable -> Hashable is a synthesised conformance on a String-raw enum.

Gating: PaykitFeatureFlags.isUIEnabled (isUIAvailable && UserDefaults paykitUiEnabled, default false), toggled only from Dev Settings behind the hidden SupportScreen tap. The manager re-checks the flag and refreshIncomingPaykitPaymentRequests() bails without it. Dev/QA-facing today.

Checked and clean:

• Rejected request resurfacing or being paid. reject -> perform(resultingState: .rejected) -> sdk.rejectPaymentRequest happens inside operation(request) before any local mutation; if it throws, the request stays pending and the view toasts it. Only processPendingMessages() is try?-swallowed, and that's the outbound counterparty notification, retried on the next synchronize() — not local state. Every synchronize() re-parses the SDK store, and a .rejected record trips record.state != .proposed -> .nonActionableState, so it never re-enters pendingRequests. The counterparty can't rewrite local lifecycle state. Losing presentedRequestIds from the Keychain can re-show a sheet but never pay.
• Re-issue under a new id produces a new ID triple requiring a fresh Pay tap plus send-sheet confirmation. No auto-pay path exists here or on base, so content-based dedup isn't load-bearing for fund safety.
• Amount TOCTOU. The request reaching beginPaymentRequest is the snapshot from requestsForPresentation(); PaymentAmountContext is built from that object's amountValue and the same object is pinned into ContactPaymentContext. Nothing re-reads the amount from the store between display and pay.
• PaykitResolutionFailureDiagnostics. Log-only strings: case name plus safeCode ([a-z0-9_-], <=64 bytes, else unknown_code), PaykitError.context dropped in every arm, unknown types rendered as metatype name only. Sole consumer is one Logger.warn. No toast, no UI, no counterparty transmission — reject(...reason: nil). No error path leaks balance, liquidity or node id, and no rejection failure reports as success.
• Trust boundaries in PaymentRequestsView. The diff there is two accessibilityIdentifiers embedding paymentRequestId — not rendered, not a path, not a URL. Toasts render static localization keys only.
• AppScene / processing before unlock. Polling runs on .task(id: scenePhase) regardless of PIN, but sheets are hosted in MainNavView, which only renders once isPinVerified — AuthCheck replaces it while locked, so a showSheet(.send) issued while locked has no host until unlock, and the send sheet still requires explicit confirmation. handleScenePhaseChange only refreshes on .active.
• Concurrency. The manager is @Observable @MainActor; deferPresentation(_:diagnosticReason:) is synchronous with no suspension between the wasRequestedPresentation capture and the set insert. Both toast queues drain with while let under one trigger comparison, so Observable coalescing can't drop a toast. Every deferIncomingPaykitPaymentRequestPresentation call is preceded by isCurrentPresentation(request) with no await in between — no reentrancy window.
• Journey and READMEs match the code. Retry counts, the 120s steady interval, the 35s wait covering 1 + 14x2s, every accessibility identifier, and the toast title/description against Localizable.strings. "Request remains available for another attempt" holds — it stays in pendingRequests and isActionDisabled clears when requestedPresentationId nils.
• Migration. No persisted schema touched; the new dict is in-memory. No predicate was removed, so the #697 limit clause doesn't engage here.

One correction to my own framing from earlier: PrivatePaykitService+Errors.swift isn't new — it exists on master, and this PR appends PaykitResolutionFailureDiagnostics to it.

Cross-repo (synonymdev/bitkit-android#1217): the terminal unavailable toast after 15 explicit attempts is present on both. Android is missing the equivalent of requestedPresentationUnavailableTrigger for a requested request that disappears mid-retry for a non-expiry reason — filed there, not here. Your parse-rejection and presentation-failure log dedup is the better shape of the two; I've noted it on the Android side.