[ben-kaufman]

Related to #868.

This PR removes message transport from the awaited refresh after payment preparation errors. Stacked on #1070.

Description

• Reads fresh stored request state while retaining the processing guard, original error, uncertain acceptance ownership and identity-generation checks.
• Preserves full refresh for rejection, cancellation and normal message discovery.

Out of Scope

• Inbox-only updates: late cancellation and proof messages remain dependent on normal full refresh; this failure projection does not receive them.

Design

N/A — no UI changes.

Preview

N/A — existing screens and routes are unchanged.

QA Notes

Setup

Use the journey guide and Payment Request setup at 8967dff8 with disposable regtest wallets and a saved, linked fixture issuer. This source uses Paykit 0.1.1-rc1 and Core 0.8.0-rc10. Keep ordinary payment authorization enabled. The controlled transport/admission faults named below require additional fixture control; compatible issuer/homeserver access and staging merchant/Locks unlock must be established separately. Missing prerequisites leave the affected cases blocked.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

• 1. regression: induce a preparation-claim failure while unrelated message delivery is slow → authorize once → verify Transaction Failed follows fresh request projection, no payment is submitted, and a later retry still requires authorization — controlled SDK/shared-lease and transport fault injection not in Capabilities.

Automated Checks

• added PaykitPaymentRequestServiceTests.swift — retains the processing guard during failure projection and rejects results after identity replacement.
• updated PaykitPaymentRequestServiceTests.swift — removes authoritative stale requests despite transport failure, retains uncertain acceptance ownership across restart, and preserves full rejection recovery.

Evidence

The exact-base negative control executed one test and failed on the expected transport/removal assertions.

Final integration validation

Integration with master 4530ff7b at 8967dff8 retains both USDT-warning cancellation and the payment-review callback. Bottom #1025: all-target build passed (19 targets), 378 focused tests passed, 0 failures/skips. One combined #1070 + #1081 + #1084 + #1085 composition, tree 8f1e72d3 from #1070 7cd096f4 plus #1081 8967dff8, #1084 b7561c08 and #1085 310e689a, covers the stack and those sibling changes: all-target build passed (19 targets) and 2,779 tests passed, 0 failures/skips using the six existing CI exclusions. These are not eight independent full-suite runs.

All eight heads passed SwiftFormat. Translation checks report 0 errors and 15 warnings, all matching master by key, language, file and occurrence count, including security__use_pin. Final toolchain/runtime and complete diagnostic disposition: Xcode 26.1 / iOS 27.0 simulator, serial tests; complete diagnostics inspected, with six SDK-construction QoS warnings and retained fixture/appearance messages matching baseline evidence; no introduced finding. Final-head CI after publication: pending.

Gaps

Earlier diagnostics included a newly observed appearance warning from unchanged hosting-test code and extra occurrences of an existing keychain warning; identical baseline occurrence was not established.

The final Android 206 / iOS 215 comparison cohort was blocked before financial authorization, with no debit. It supplies no final payment-latency result or native benefit claim. Case 1 remains unchecked and unexecuted on this integrated head. Native USDT-warning, PIN and biometric flows have not been verified by this integration validation.

Models used

• Planning/scoping: gpt-6-astra (reasoning: xhigh)
• Implementation: gpt-6-astra (reasoning: xhigh)
• Review: gpt-6-astra (reasoning: xhigh)

[greptile-apps]

Confidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Preparation errors now refresh stored Paykit request state without sending or receiving private messages.

• Failed payment preparation refreshes stored request state before returning its error.

No actionable issues found. Tests were inspected, not run during this review.

ben-kaufman explicitly leaves inbox-only cancellation and proof updates to normal full refresh.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Prepare payment] --> B{Preparation succeeds?}
    B -- Yes --> C[Approve payment]
    B -- No --> D[Keep processing guard]
    D --> E[Invalidate previous refresh]
    E --> F[Read stored request state]
    F --> G{Generation and identity still match?}
    G -- Yes --> H[Update request lists]
    G -- No --> I[Discard stale result]
    H --> J[Return original error]
    I --> J
    J --> K[Release processing guard]

Loading

_{Reviews (1) · Last reviewed commit: "fix: avoid inbox waits after preparation..." · Reviewed by Greptile}

[ovitrif]

Milestone 3.2.0: it is stacked on #1070 (3.2.0), the same Paykit latency track.

[talosmachina]

No findings. On a preparation failure, the post-failure refresh in perform now reads stored request state (.stored) instead of a full refresh, so a slow or failing outbound/inbox transport no longer blocks or voids the projection of the authoritative queue. Rejection and other callers keep the .full default. Reviewed 6dd46e4, full tier, reasoned from the code: iOS does not build on this reviewer's Linux host.

What I checked, and 4 candidates I ruled out

Read in full: perform, prepareForPayment, both refresh overloads, performRefresh, PaykitPaymentRequestService.synchronize, invalidateRefresh, the new and changed tests and the mock hooks they use (setAcceptanceResponseError(_:committedRecord:), pauseNextPaymentRequestList).
Call sites traced: failureRefreshMode is passed only from prepareForPayment; reject and the other perform callers keep .full.
CI: SwiftFormat and Greptile green; Run Tests still pending at review time.

Ruled out

• A concurrent full refresh is downgraded: invalidateRefresh() drops the in-flight task first, and a later caller asking for .full while the .stored task runs re-runs at its own mode (mode > refreshTask.mode).
• Processing guard released early: the stored refresh is awaited inside perform before the defer, and the new test covers a second prepareForPayment returning operationInProgress while it is paused.
• Result applied after an identity switch: performRefresh checks refreshGeneration and activeIdentity after synchronize; covered by the replacement-identity test.
• Uncertain acceptance never delivered: the queued outbound message is no longer pushed on the failure path, but the next full refresh's processPendingMessages delivers it, and the PR names late cancellation and proof discovery as out of scope.

Merge confidence: 4/5, no findings and the change is covered by tests, but Run Tests had not finished and the branch was not built here.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed

[piotr-iohk]

QA review

Scope: Reviewed the full PR diff against its merge base, at 6dd46e46.

No new actionable code findings.

A failed prepareForPayment now awaits a stored-state refresh, then returns the original error, without sending or receiving private messages. Rejection and cancellation still refresh in full. The processing guard stays held across that refresh, uncertain acceptance ownership is kept, and a generation or identity change discards the refresh instead of approving the failed request.

Compared with bitkit-android 4bb41772: preparation failures already reload stored request state. iOS waits for that projection before returning the error and holds the in-flight guard; Android starts the stored refresh after the failure UI.

Validation: the new and updated PaykitPaymentRequestServiceTests were inspected at this revision and not executed here, because this host has no iOS toolchain. SwiftFormat passed on this commit. The unit-tests workflow was still in progress.

Device testing: not performed in this review. The author's manual case needs controlled SDK lease and transport fault injection, which the journey capabilities do not provide.

[ovi-reviewer]

Verdict: ⚠️ Not verified — live fault case 1 remains unrun.

Findings: None.

QA for 8967dff: Xcode 27.0, iOS 26.5 simulator.

Test A1: payment-request regressions ✅ XCTest passed on the unmodified head.

The app built successfully and all 280 PaykitPaymentRequestServiceTests passed, with no failures or skips. This includes authoritative removal despite transport failure, the processing guard, identity replacement, uncertain acceptance ownership across restart, and full rejection recovery. SwiftFormat passed for both changed Swift files.

XCTest results establish the assertions; the recording captures the simulator during the run.

A1-2x.2x.mp4

Test A2: native failure and retry supplement ✅ The mocked claim failure followed fresh projection, and retry requested PIN again.

A test-only XCTest hosted the production confirmation, PIN and failure views with a mock wallet and the existing Paykit SDK mock. The preparation callback invoked the real payment-request manager. After PIN approval, the injected claim error completed its stored projection before Transaction Failed appeared. Try Again and another swipe opened a fresh PIN prompt.

XCTest confirmed zero outbound/inbox calls, no payment approval, two PIN requests, one PIN approval, one preparation and zero wallet dispatches. This checks the native views and manager at their callback boundary; it does not exercise a live issuer or real shared lease. Production app source was unchanged.

A2-2x.2x.mp4

Native views with test-only claim fault, at 2× speed.

Test 1: live claim failure with slow unrelated delivery ⏭️ Shared-lease contention and slow peer delivery were not reproduced.

The available issuer fixtures do not provide control of the iOS payer's shared lease. The mock supplement above cannot establish this live integration case. No native latency improvement is claimed.

Full review covered the three-file diff, including preparation, refresh coalescing, cancellation, identity invalidation and rejection/cancellation siblings. Live issuer-to-payment behavior remains undriven. The earlier code review and QA review also reported no findings; their earlier-head inspection is not execution evidence for this head.

Twin: no dedicated Android twin is linked for this diff. The parent stack's twin is synonymdev/bitkit-android#1645; Android already refreshes stored state after preparation failure. Transport exclusion matches; iOS awaits projection before returning the error, while Android schedules it after failure handling.

Reviewer model: gpt-6.1-sol
Reasoning effort: high

---

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

[piotr-iohk]

QA review

Scope: Re-reviewed the full PR at 8967dff8 after the comparison base changed from b1d65a7 to 7cd096f4. No baseline coverage was supplied, so this is a full reassessment with nothing inherited. The merge base is 7cd096f.

No new actionable code findings.

A failed prepareForPayment awaits a stored-state refresh and then returns the original error. The refresh reads the authoritative request list while the processing guard stays held, so a second prepare is rejected. Message send and receive stay on the full refresh used by rejection, cancellation, and ordinary discovery. A generation or identity change discards the projection and leaves the request unapproved. Uncertain acceptance keeps its local owner. A request removed from stored state leaves the pending and presentation lists.

Compared with bitkit-android d5526d40: preparation failures also reload stored request state and skip inbox transport. iOS waits for that projection before returning the error and holds the in-flight guard. Android shows the failure, then starts the stored refresh. Late cancellation and proof messages remain for the next full refresh. Foreground polling still performs that refresh after payment activity ends.

Validation: the new and updated PaykitPaymentRequestServiceTests were inspected at this revision. This host has no iOS toolchain, so this review did not execute them. unit-tests and SwiftFormat passed on this commit.

Device testing: not performed in this review.

Recommended before device testing: manual case 1 needs a fixture that can fail the payer's preparation claim and stall unrelated message delivery. Journey capabilities do not include that fault injection, so case 1 cannot be run until that fixture exists.

[piotr-iohk]

QA review

Scope: Re-reviewed the full PR at db83ed7d after the comparison base changed from 7cd096f to 05c92291. No baseline coverage was supplied, so this is a full reassessment with nothing inherited. The merge base is 05c9229.

No new actionable code findings.

A failed prepareForPayment awaits a stored-state refresh and then returns the original error. The processing guard stays held, so a second prepare is rejected. Message send and receive stay on the full refresh used by rejection, cancellation, and ordinary discovery. A generation or identity change discards the projection and leaves the request unapproved. Uncertain acceptance keeps its local owner. A request removed from stored state leaves the pending and presentation lists. Late cancellation and proof messages still arrive on the next full refresh.

Compared with bitkit-android 801fea07: preparation failures also reload stored request state and skip inbox transport. iOS waits for that projection before returning the error and holds the in-flight guard. Android shows the failure, then starts the stored refresh.

Validation: the new and updated PaykitPaymentRequestServiceTests were inspected at this revision. This review did not execute them. unit-tests and SwiftFormat passed on this commit. PaykitPaymentRequestServiceTests is not in the unit-test skip list.

Device testing: not performed in this review.

Recommended before device testing: manual case 1 needs a fixture that can fail the payer's preparation claim and stall unrelated message delivery. Journey capabilities do not include that fault injection, so case 1 cannot be run until that fixture exists.

Reviewer model: grok-4.7
Reasoning effort: xhigh

[jvsena42]

Code scan: no findings.

Checked and clean:

• A prepareForPayment failure now goes to the failure UI without awaiting the inbox transport (receive/process) under the processing guard. The stored refresh still runs, so the list catches up without blocking the error.
• Android master already behaves this way (AppViewModel.kt:7903-7918 launches the stored refresh after the failure UI), so there is no twin to port.
• No UI text changed, no journeys apply.

Performance-relevant (removes a wait on the failure path). The gate times prepareForPayment throw → SendFailure shown, and counts receive/process calls in that window: expected 0 at head vs 2 on master.

Device gate (partial): db83ed7 — not run yet; pending that measurement against master. Device results will follow in a separate review.