Repository navigation
perf: keep request creation ahead of refreshes - #1085
ben-kaufman wants to merge 14 commits into
Conversation
|
Milestone 3.2.0: it is stacked on #1070 (3.2.0), the same Paykit latency track. |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Holds a review token while a one-time payment request checks eligibility, commits its proposal and attempts targeted delivery, so background refreshes wait. Reviewed bee8af8, full tier, reasoned from the code and CI: iOS does not build on this host.
What I checked, and 3 candidates I ruled out
Read in full: the diff; PaykitPaymentRequestManager.propose, PaykitPaymentRequestService.propose, discoverEligibleTargets, proposalWasSent, processPendingMessages, PaykitPaymentActivity, PubkyService.checkRefreshAdmission
Call sites traced: shouldDeferRefresh and waitUntilIdle(includingReviews: true) (contact publication, message drain retries, deep links, proof refresh)
CI: SwiftFormat green; Run Tests still pending at review time
Ruled out
- Creation deferring its own SDK calls: every call on the propose path runs at
.interactive(identity status, linked peers, proposal, targeted delivery, shared requests), andcheckRefreshAdmissiononly rejects.background, so the token cannot block its holder. - Token leaked on throw or cancellation:
endReviewsits in the samedeferasisCreatingRequest, outside the generation check, so it runs on every exit; the new cancellation test checks it. - Idle waiters stuck behind a released token:
endReviewresumes them once reviews are empty, and they re-check payments before proceeding.
Merge confidence: 4/5, Run Tests is still pending and the branch was reasoned about, not built here.
Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the runner does not report it)
 [Medium impact] This PR appears safe to merge. SummaryRequest creation now holds a review token while it checks the recipient, stores the proposal, and attempts delivery.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Start request creation] --> B[Begin review token]
B --> C[Check recipient]
C --> D[Store proposal]
D --> E[Attempt targeted delivery]
E --> F[Return sent or queued request]
C --> G[Error or cancellation]
D --> G
F --> H[Release own review token]
G --> H
Reviews (1) · Last reviewed commit: "perf: keep request creation ahead of ref..." · Reviewed by Greptile |
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against merge base b1d65a7f, at bee8af89. Full coverage of bitkit-ios#1085; no earlier assessment baseline.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a shared PaykitPaymentActivity review from just before the service call until the function returns. That span includes the fresh eligibility read, proposal commit, and targeted delivery. The same defer ends that review on success, queued delivery, rejection, cancellation, clear(), and identity replacement, while a separate review token stays held. checkRefreshAdmission rejects .background work while the review set is non-empty and admits .ordered and .interactive work. Creation uses .interactive for identity status, linked peers, the proposal, targeted delivery, and the follow-up shared-request read. Subscription creation is outside this token. Compared only that one-time admission contract with bitkit-android#1661 at e552783b, which also wraps one-time propose and leaves subscription creation outside the token.
Validation: read the added and updated PaykitPaymentRequestServiceTests cases and did not run them. SwiftFormat passed on this revision. Run Tests was still in progress for bee8af89.
Device testing: not performed in this review. Journeys and manual tests are N/A in the PR because this is internal admission with no user-visible change.
Reviewer model: grok-4.7
Reasoning effort: Not exposed (cursor-cloud run-info reports originalModelName and has no reasoning-effort field)
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None.
QA for 310e689:
Test 1: focused automated regression ✅ The exact head built and both payment-request and payment-activity XCTest suites passed.
Run with Xcode 27.0 on an iOS 26.5 simulator. I checked admission through eligibility, proposal commit and targeted delivery, cancellation cleanup, queued delivery, rejection, identity replacement and preservation of another review through the XCTest assertions and result bundle. SwiftFormat lint also passed for both changed files.
automated-2x.2x.mp4
XCTest host recording at 2×; assertion results establish the behavior.
No manual device journeys were run: the QA notes request none, and this internal scheduling change has no user-visible behavior. No native timing improvement was measured. The current-head CI test job also passed.
Full review: 7cd096f4 to 310e689a, both changed files; the admission change was exercised through automated checks rather than a manual device case.
Twin: synonymdev/bitkit-android#1661 perf: keep request creation ahead of refreshes at e552783b matches the one-time creation admission scope.
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
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 310e689a after the comparison base changed from e16d13bc to 7cd096f4 and no verified baseline coverage was supplied. This pass covers the current diff from merge base 7cd096f4.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a shared PaykitPaymentActivity review from just before the service call until the function returns. That span includes the fresh eligibility read, proposal commit, and targeted delivery. The same defer ends that review on success, queued delivery, rejection, cancellation, clear(), and identity replacement, while a separate review token stays held. checkRefreshAdmission rejects .background work while the review set is non-empty and admits .ordered and .interactive work. Creation uses .interactive for identity status, linked peers, the proposal, targeted delivery, and the follow-up shared-request read. Subscription creation stays outside this token. Compared only that one-time admission contract with bitkit-android#1661 at e552783b, which also wraps one-time propose and leaves subscription creation outside the token. Already-admitted SDK work can still finish.
Validation: this review did not execute the suite. Run Tests passed on this revision; its log shows PaykitPaymentRequestServiceTests compiled and testProposeBuildsOneTimeBitcoinTermsAndDrainsOutbox and testCancelledProposalReleasesReviewAndAllowsAnotherCreation passed. SwiftFormat passed on this revision. The unit-test workflow includes this suite.
Device testing: not performed in this review. Journeys and manual tests are N/A in the PR because this is internal admission with no user-visible change.
Reviewer model: grok-4.7
Reasoning effort: Not exposed (cursor-cloud run-info reports originalModelName and has no reasoning-effort field)
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire PR diff against merge base 05c92291, at f73b5747. Inherited coverage is empty because the comparison base changed from 7cd096f4 to 05c9229 and no verified baseline was supplied.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a shared PaykitPaymentActivity review across the fresh eligibility read, proposal commit, and targeted delivery. The same defer releases that review when the attempt returns or throws, including success, queued delivery, rejection, cancellation, and an in-flight attempt that finishes after clear() or an identity change. Background refresh admission rejects only background work while a review is open; eligibility, the proposal write, and targeted delivery stay on the interactive lane. Subscription creation leaves the token untouched. The same one-time boundary is present in bitkit-android#1661 at e552783b. A background SDK operation that already passed admission can still finish.
Validation: this review did not execute the suite locally. Run Tests passed on this revision; its log shows PaykitPaymentRequestServiceTests compiled and the creation-admission cases passed, including eligibility-through-delivery, cancellation cleanup, queued delivery, pre-commit rejection, and identity replacement. SwiftFormat passed on this revision.
Device testing: not performed in this review. Journeys and manual tests are N/A because this is internal admission with no user-visible change.
Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: xhigh
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no findings.
Checked and clean:
- Interactive request creation is admitted ahead of queued background refreshes and releases its priority on every exit: success, error and cancellation. A refresh that waited is run after the release, not dropped, and no path holds the admission across a suspension it doesn't own.
- Twin synonymdev/bitkit-android#1661 has the same scope and release ordering.
- No user-visible change, no string keys, no migration surface.
Performance-relevant (locking/priority admission). The gate runs idle-then-act: at least 10 s on Home with polling and a pending retry, then Continue on the request amount screen. It times that to the end of creation and compares the queue value of the Finished Paykit operation … queue= work= line for the interactive creation calls, head vs master, plus the time to the first completed background inbox refresh after release.
Device gate (partial): f73b574 — not run yet; pending that measurement against master. Device results will follow in a separate review.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire PR diff against merge base 02986d70, at f5961d30. Inherited coverage is empty because the comparison base changed and no verified baseline was supplied.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a shared PaykitPaymentActivity review from just before PaykitPaymentRequestService.propose until that function returns. The span covers the fresh eligibility read, proposal commit, and targeted delivery. The same defer releases that review when the attempt returns or throws, including success, queued delivery, rejection, cancellation, and an in-flight attempt that finishes after clear() or an identity change. A separate review token stays held. checkRefreshAdmission rejects background work while a review is open and admits ordered and interactive work, so creation's identity, linked-peer, proposal, targeted-delivery, and follow-up shared-request calls remain admitted. Subscription creation leaves the token untouched. The same one-time boundary is present in bitkit-android#1661 at c4753342. A background SDK operation that already passed admission can still finish; the inbox poll continues and runs again after the review ends.
Validation: this review did not execute the suite locally. Run Tests passed on this revision; its log shows PaykitPaymentRequestServiceTests ran with 0 failures, including testProposeBuildsOneTimeBitcoinTermsAndDrainsOutbox, testCancelledProposalReleasesReviewAndAllowsAnotherCreation, testProposeRemainsCreatedWhenImmediateDeliveryFails, testProposeRevalidatesTargetBeforeEnqueueing, testProposalCompletionAfterClearDoesNotRepopulateManager, and testProposalDoesNotCommitAfterSdkIdentityChanges. SwiftFormat passed on this revision.
Device testing: not performed in this review. Journeys and manual tests are N/A because this is internal admission with no user-visible change.
Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire PR diff against merge base 974be754, at 89f05b40. The comparison base changed from 02986d70 to 974be754, and no verified baseline coverage was supplied, so inherited coverage is empty. The base edit in PaykitPaymentRequestServiceTests.swift only isolates the cached channel count in an unrelated sync-fallback test.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a shared PaykitPaymentActivity review from just before PaykitPaymentRequestService.propose until that function returns. The span covers the fresh eligibility read, proposal commit, and targeted delivery. The same defer releases that review when the attempt returns or throws, including success, queued delivery, rejection, cancellation, and an in-flight attempt that finishes after clear() or an identity change. A separate review token stays held. checkRefreshAdmission rejects background work while a review is open and admits ordered and interactive work, so creation's identity, linked-peer, proposal, targeted-delivery, and follow-up shared-request calls remain admitted. Subscription creation leaves the token untouched. The same one-time boundary is present in bitkit-android#1661 at c4753342. A background SDK operation that already passed admission can still finish. An overlapping inbox refresh is rejected and the next scheduled poll runs later; message-drain retries wait until the review ends.
Validation: local execution was not part of this code review. Run Tests passed on this revision; its log shows PaykitPaymentRequestServiceTests passed, including testProposeBuildsOneTimeBitcoinTermsAndDrainsOutbox, testCancelledProposalReleasesReviewAndAllowsAnotherCreation, testProposeRemainsCreatedWhenImmediateDeliveryFails, testProposeRevalidatesTargetBeforeEnqueueing, testProposalCompletionAfterClearDoesNotRepopulateManager, and testProposalDoesNotCommitAfterSdkIdentityChanges. SwiftFormat passed on this revision.
Device testing: not performed in this review. Journeys and manual tests are N/A because this is internal admission with no user-visible change.
Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict:
Findings: None.
QA for 89f05b4:
Test 1: request-creation admission regression ✅ The unchanged creation cases passed in current-head CI.
I checked the current-head test log: the request and activity suites passed, including eligibility through targeted delivery, cancellation followed by another creation, queued delivery, pre-commit rejection, identity replacement and preservation of another review token.
The two propose functions, activity implementation and six creation regression tests are unchanged from 310e689. I retained the earlier recorded regression result; I did not rerun that suite locally.
Test 2: native timing against master ⏭️ The suggested timing comparison remains unrun.
The requested idle-then-create comparison has no passing report. Interactive queue time, creation duration and the first completed background inbox refresh after release were not measured against master. No native latency improvement is claimed.
Check 1: build and formatting ✅ The exact head compiled and both changed Swift files passed formatting lint.
Compile-only arm64 simulator build with Xcode 27.0 targeting iOS 26.5; swiftformat --lint on the two net PR Swift files. No app was launched.
Check 2: delta and twin ✅ The creation admission contract remains unchanged.
The token still covers fresh eligibility, proposal commit and targeted delivery. Its unconditional release preserves other review tokens; creation stays on Interactive admission. Checked the merged-parent changes for admission, retry, identity and lifecycle interactions.
Twin: synonymdev/bitkit-android#1661 perf: keep request creation ahead of refreshes at c4753342 matches the one-time creation token scope and unconditional release.
No manual device journeys were run: this PR's QA notes specify none for the internal scheduling change. Review scope: contiguous delta 310e689a..89f05b40, including merged-parent interactions and the complete two-file net PR diff. No change was driven through a new manual device case.
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
…kit-request-creation-admission
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire PR diff against merge base 7a3cc5a9, at 3f559e92. Inherited coverage is empty because the comparison base changed from 974be754 to 7a3cc5a9 and no verified baseline was supplied.
No new actionable code findings.
One-time creation in PaykitPaymentRequestManager.propose holds a PaykitPaymentActivity review from just before PaykitPaymentRequestService.propose until that function returns. The span covers the fresh eligibility read, proposal commit, and targeted delivery. The same defer releases that review on success, queued delivery, rejection, cancellation, and an in-flight attempt that finishes after clear() or an identity change. A separate review token stays held. checkRefreshAdmission rejects background work while a review is open and admits ordered and interactive work, so creation's identity, linked-peer, proposal, targeted-delivery, and follow-up shared-request calls remain admitted. Subscription creation leaves the token untouched. The same one-time boundary is present in bitkit-android#1661 at c4753342. The new base's app-lock path in SendSheet keeps its own send-sheet payment review, separate from the creation token. A background SDK operation that already passed admission can still finish. An overlapping inbox refresh is rejected, and message-drain retries wait until the review ends.
Validation: this review did not execute the suite. SwiftFormat passed on this revision. Run Tests was still in progress, so this review does not treat the suite as passed on this revision.
Device testing: not performed in this review. Journeys and manual tests are N/A because this is internal admission with no user-visible change.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict:
Findings: None.
QA for 3f559e9:
Test 1: request-creation admission regression ✅ The retained creation cases and newly imported app-lock regression passed in pinned-head CI.
I checked the pinned-head test log: the request and activity suites passed, including eligibility through targeted delivery, cancellation followed by another creation, queued delivery, pre-commit rejection, identity replacement and preservation of another review token. The new testAppLockKeepsOtherRequestScreensUntilDismissal also passed.
The creation functions, delivery helper, activity implementation and creation regression tests are unchanged from 89f05b4. I retained the earlier recorded regression result; I did not rerun XCTest locally. The app-lock assertions cover hosted-view disappearance, restoration of the same amount and payment context, and cleanup on dismissal; they do not establish a native PIN journey.
Test 2: native timing against master ⏭️ The requested timing comparison remains unrun.
The idle-then-create comparison still has no passing report. Interactive queue time, creation duration and the first completed background inbox refresh after release were not measured against master. No native latency improvement is claimed.
Check 1: build and formatting ✅ The pinned head compiled and the affected Swift files passed formatting lint.
Compile-only arm64 simulator build with Xcode 27.0; swiftformat --lint on both net PR Swift files and the delta's SendSheet.swift. The contiguous delta also passed git diff --check. No app was launched.
Check 2: delta and twin ✅ Creation admission and independent review-token release remain intact.
I checked the app-lock retention, remounting and dismissal cleanup imported from synonymdev/bitkit-ios#1070 fix: keep Paykit inbox observation together, alongside creation, cancellation, identity and refresh-admission interactions. The creation token still spans fresh eligibility, proposal commit and targeted delivery; its release preserves other review tokens. Creation uses Interactive admission.
Twin: synonymdev/bitkit-android#1661 perf: keep request creation ahead of refreshes at c4753342 matches the one-time creation token scope and unconditional release.
No device run: the PR's QA notes request no manual cases or journeys for the internal scheduling change. Review scope: complete contiguous delta 89f05b40..3f559e92, including parent app-lock changes, plus the complete two-file net PR diff. No change was driven through a new manual device case.
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
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the entire PR diff against merge base ac364f01, at 40e36d7e. Inherited coverage is empty because no verified baseline was supplied and the comparison base changed from 7a3cc5a9 to ac364f01.
No new actionable code findings.
One-time request creation holds the existing review token from before fresh eligibility through proposal commit and targeted delivery, and releases it on success, queued delivery, pre-commit rejection, clear, identity replacement, and cancellation. A second review token remains held. Ordered and interactive creation calls keep their existing admission; background refresh admission stays closed while the token is held. Subscription creation stays outside this token, matching bitkit-android#1661 at c4753342. That comparison covered only this admission contract.
Unit/UI run 40412990785 checked out merge a7a99aa (40e36d7e into ac364f01). PaykitPaymentRequestServiceTests passed there, 299 tests and 0 failures, including testCancelledProposalReleasesReviewAndAllowsAnotherCreation and testProposeBuildsOneTimeBitcoinTermsAndDrainsOutbox. SwiftFormat 40412990660 passed for the same head. This review did not re-run the suite. Device testing: not performed. The change has no user-visible behavior, and no device journey is indicated.
Reviewer model: grok-4.7
Reasoning effort: high
There was a problem hiding this comment.
Verdict:
Findings: None.
QA for 40e36d7:
Test 1: request-creation admission regression ✅ The request service cases passed in current-head CI.
The current-head review reports the CI run passed the request service suite, including creation admission and cancellation. I retained that result; I did not rerun the suite locally.
Test 2: native timing against master ⏭️ The requested timing comparison remains unrun.
The idle-then-create comparison has no passing report. Creation duration, interactive queue waits and the first completed background refresh after release were not measured against master. No native latency improvement is claimed.
Check 1: pinned-head build ✅ The head compiled for the arm64 iOS simulator.
The build used Xcode 27.0. The contiguous delta also passed git diff --check.
Check 2: delta and twin ✅ The creation admission boundary remains intact.
The token spans fresh eligibility, proposal commit and targeted delivery, and its release preserves other review tokens. I checked the merged changes for interactions with creation, refresh, cancellation and lifecycle behavior.
Twin: synonymdev/bitkit-android#1661 perf: keep request creation ahead of refreshes at c4753342 matches the one-time creation token scope and release.
No device run: the PR's QA notes request no manual cases or journeys for its internal scheduling change. Review scope: contiguous delta 3f559e9..40e36d7, including merged-parent interactions and the net PR change. No change was driven through a new device case.
Reviewer model: Unknown
Reasoning effort: Not exposed
Model and effort were not available in the review run's accessible metadata.
Reviewed by gpt-6.1-terra-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR at 40e36d7e against merge base ac364f01 because a forced full review was requested. Nothing was inherited from a baseline. Coverage includes the production change in PaykitPaymentRequestManager.propose, the updated and new PaykitPaymentRequestServiceTests cases, the PaykitPaymentActivity review-token contract, and every waitUntilIdle(includingReviews:) and background-admission consumer this change now affects.
No new actionable code findings.
The review token is taken only after the manager's guards pass. A defer releases it on success, queued delivery, error, cancellation and identity or state replacement. The creation itself is never held back by its own gate. Eligibility recheck, proposal commit and targeted outbound delivery all run at .interactive priority, and PaykitSdkService.checkRefreshAdmission rejects only .background work. The waiters this change newly holds up are the stored-proof refresh, contact-list load, contact deep-link preparation, background endpoint republish and background message-retry scheduling. Each one waits for the release and then continues rather than being dropped. A periodic full refresh of the recipient list that hits the deferral returns early and keeps the existing targets. The post-commit invalidateRefresh() only cancels and clears the current refresh task; it starts no new background work inside the token.
Parity: the Android twin bitkit-android#1661 at c4753342 wraps the same one-time creation flow (eligibility, commit, delivery, publish) in beginPaymentReview() and releases it in finally. On both platforms subscription proposals remain outside the token. This was checked by comparing source for this flow only. It is not a full review of the Android PR.
Validation: Run Tests 40412990785 passed for the PR merge build of this head. This reviewer ran no build or tests; Swift execution is not available on this host. The changed unit tests use PaymentRequestSdkMock with an injected PaykitPaymentActivity, so they do not exercise the production shared-activity gate in PaykitSdkService. The priority analysis above covers that gap by reading the source, not by running it.
Device testing: not performed in this review. The PR declares no user-visible change and no manual cases.
Suggested additional test cases
- iOS, two linked Bitkit wallets with distinct Pubky identities and request support enabled. Open Contacts and a contact detail, then send a one-time payment request. Expected: the request completes with Sent or Queued status, with no extra delay caused by the app's own refresh gate. After the sent screen, the contacts list and incoming-request refresh resume and show current data without needing to navigate away.
- iOS, same setup. Start a payment request, then background the app or dismiss the sheet while creation is in flight. Expected: after returning, background refreshes (incoming requests, contact list) run normally. Nothing stays deferred once the creation has finished or been cancelled.
Reviewer model: claude-opus-5-5
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up at 8830af54, covering the commit since 40e36d7e. Inherited coverage is the completed baseline review against the same merge base ac364f01: payment-request review-token acquisition and release, admission priorities, activity waiters, gated refresh consumers, post-commit refresh invalidation, the create-request caller, the request-service tests, inspected unit run 40412990785 for that previous head, and the Android creation-token comparison at c4753342. Those paths are unchanged. This pass reviewed the recreated send confirmation.
No new actionable code findings.
A recreated confirmation keeps the parent isSubmittingPayment flag. The swipe stays busy from that flag, and submitPayment returns before another PIN or biometric prompt while the flag is set. performPayment raises the flag and clears it on every exit. The in-flight attempt is an unstructured task, so it continues across the PIN lock that replaces MainNavView with AuthCheck. After that attempt leaves performPayment, a manual confirmation has a cleared flag and accepts another swipe. An automatic subscription duplicate sets hasStartedAutomaticPayment on the new view; a failed subscription that still has a request id retries by dismissing the sheet and presenting a new one.
Validation: Run Tests 40850999737 passed on 8830af54, including testRecreatedConfirmationDoesNotAuthorizeWhilePaymentIsSubmitting and testManualPaymentKeepsTheSwipeBusyWhenConfirmationIsRecreated. SwiftFormat 40850999696 passed on the same commit. This review did not build or run tests. Device testing: not performed.
The Android comparison at the twin pin is limited to onConfirmPay, which refuses a second submission for the whole in-flight attempt.
Suggested additional test cases
- iOS, PIN enabled, funded regtest wallet. Swipe to pay a Lightning invoice and background the app until the lock screen appears while the confirmation is still busy. Unlock. Expected: the confirmation stays busy, does not ask for PIN or biometrics again, and the original payment completes or fails once.
Reviewer model: grok-4.7
Reasoning effort: high
Twin: synonymdev/bitkit-android#1661.
This PR holds the existing review admission token while a one-time payment request checks eligibility, commits its proposal and attempts targeted delivery. Stacked on #1070.
Description
Out of Scope
None.
Design
N/A — no UI changes.
Preview
N/A — no user-visible behaviour change.
QA Notes
Setup
N/A — no device journeys or manual cases are requested for this change. Automated coverage is listed below; the shared journey guide remains the device-test reference.
Journeys
N/A — no user-visible behaviour change.
Manual Tests
N/A
Automated Checks
PaykitPaymentRequestServiceTests.swift— exercises creation admission through the actual caller, cancellation cleanup and a subsequent creation.PaykitPaymentRequestServiceTests.swift— checks queued delivery, pre-commit rejection, identity replacement and preservation of another review token.Evidence
The exact-base negative control failed at the expected refresh-admission assertion.
Final integration validation
Integration with master
4530ff7bat310e689aretains 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, tree8f1e72d3from #10707cd096f4plus #10818967dff8, #1084b7561c08and #1085310e689a, 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
Already-admitted work is not preempted. No native timing improvement is claimed.
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. Native USDT-warning, PIN and biometric flows have not been verified by this integration validation.
October 26 master integration
Published integration head
6f1e35164231070936b14e8706f95c11832c9eacincorporates actual master2ce210252be8ae7ea130d2422a2fa704a191ef7dthrough ordinary signed merges, preserving the stacked base and prior child changes. The two parent conflict resolutions retain review-state ownership, root-safe subscription Back navigation, one outer USDT status task, prompt recipient-scoped proof reconciliation and the terminal-refresh exit. No local assertion/timeout weakening or new test suppression was introduced.Validation: two 19-target builds and two canonical hosted unit suites passed: 2,910 on the parent and 2,937 on the combined tree, zero failures/skips with the exact six CI integration exclusions. This PR is covered by the combined stack/sibling tree
f471e8a6(2,937 units), including this branch; these are two suites, not eight independent runs. Full SwiftFormat0.64 passed on all eight final trees (0/844); translations had zero errors and 15 warnings matching execution on complete actual-master source. Independent frozen integration review found no actionable issue. Local environment: Xcode26.1/SDK26.1, iOS26.5 simulator.Complete diagnostics retained: two App Intents notices per build; parent1,082/combined1,099 runtime scan records, including four/six SDK/Pubky/DHT QoS stacks, fault fixtures and hosted appearance/identity notices. Source comparisons are not exact-base runtime replay or warning-free evidence. New-head hosted CI follows publication. Master's #1130 retired Blocktank skips and E2E285 channel/migration omissions remain coverage gaps, not fixes for original #1109. Historical #1054 preparation/UI-launch causes remain unresolved. Existing QA case IDs and native gaps remain: no new payment, funded USDT, physical hardware, latency or merchant-unlock journey was run by this integration.
October 26 lifecycle integration and camera fixture
Published head
40e36d7edc169f443fb919fb6dd424f8fa937ef5includes approved and merged #1098 through masterb8b7f3d9902de5669c025bb839232c1d7e98d2df. The overlapping lifecycle test retains one owned switch task per current context, holds resumed preparation until confirmation is visible, and keeps the exact-one-success assertion, cancellation/context guards and retained presentation/app-lock behavior. Seven ordinary signed merges preserve stacked bases and prior child changes; #1084/#1085 also retain merged #1081 through #1070.The existing unfunded USDT UI case now resets camera permission before launch, explicitly waits for the camera alert, allows it and waits for dismissal before manual entry. This adds six test-only lines and two assertions; no test, assertion or timeout was removed or weakened. Production camera behavior and the merged Receive blank-band checks are preserved.
Final validation passed two 19-target builds, 2,913 parent canonical units and 2,940 units on combined tree
db653be56e228dec55f4e6c0fd57e42e7d8475c2, with zero failures/skips and the exact six CI integration exclusions. This branch is included in that combined composition. These are two suites, not seven independent runs. The combined unfunded UI case passed, including both Receive blank-band swipes and explicit camera handling, using runner-only loopback endpoint settings. Full SwiftFormat 0.64 passed on all seven final trees (0/844); translations are unchanged. Independent final review found no actionable issue. All 151 distinct commits across local current/prospective base ranges verified cryptographically; all 129 distinct commits across the final published GitHub ranges report verified=true/reason=valid.Complete diagnostics remain retained: parent 1,107/combined 1,125 broad records, including four/six SDK/Pubky/DHT QoS stacks, App Intents notices, hosted appearance and fault-fixture messages. The combined shared-keychain -25300 publication diagnostic has unchanged emitter/test source; its exact runtime cause remains unestablished. Source comparison is not exact-base runtime replay or warning-free evidence.
The old Receive-dismissal failure in #1070 run 40394372963 did not reproduce in the controlled original-head run; that run passed and used XCTest's default camera interruption handler. The camera-fixture comparison passed with an identical 157-file production app bundle. Neither result establishes the Receive failure's cause or repair. Local Xcode 26.1/SDK 26.1/iOS 26.5 differs from hosted CI; the completed hosted results are recorded below. The daemon launch issue #1110, original hardware-transfer issue #1109 and earlier coverage gaps remain. No new payment, funded USDT, physical-hardware, final latency or merchant-unlock result is claimed. João, Piotr and Ovi remain requested; existing pending requests are preserved without duplicate notification.
October 26 hosted CI confirmation
For published head
40e36d7edc169f443fb919fb6dd424f8fa937ef5, unit/UI run 40412990785 passed on attempt 1: 2,938 unit tests, zero failures/skips; 43 UI executions, 39 passed and four existing prerequisite skips, zero failures. SwiftFormat 40412990660 passed: 0/844 files require formatting, 84 skipped. These workflows tested GitHub's PR merge commit using Xcode 16.4/SDK 18.5 on iOS 26.2.Complete workflow diagnostics were inspected. The 571 duplicate-symbol records match the retained passing #1025 run 40376172874; workflow/cache-action sources match actual master. This is retained-log/source comparison, not an exact-base runtime replay or warning-free result. The cache setup also emitted an xcodebuild-version/head-pipeline Broken pipe exception; its version-command stack and subsequent successful setup/tests are retained.
No manual CI rerun was requested. Historical Receive-dismissal/daemon-launch causes and original hardware-transfer issue #1109 remain unresolved; existing skipped/deferred coverage and final Android 206/iOS 215 payment/unlock measurement remain incomplete. Existing human review requests are preserved.
October 29 inherited app-lock guard
Published 8830af54 carries the four-file R1 correction from #1039 at aae380fc: a recreated confirmation uses the retained submission flag for its busy state and refuses another authorization while the original payment is in flight. This branch retains its existing Core 0.8.0-rc10, LDK 0.7.0-rc.77, Paykit 0.1.1-rc1 and VSS 0.5.23 pins, stacked base and prior history.
On this exact branch candidate, Xcode 27.1 / SDK 27.1 / iOS 26.5 passed the complete selected unit suite: 2,940 passed, 0 failed, 0 skipped, with the six existing unit-workflow integration exclusions. Both new confirmation regression tests passed. Full SwiftFormat 0.64.0 passed: 0/844 files require formatting; 84 existing exclusions. The private package copy was corrected before this branch's first invocation; the sibling #1054's initial build-path failure remains retained. This branch's first full test run passed with unchanged source and dependency pins.
Complete diagnostics were inspected: two AppIntents metadata notices, six native SDK/Pubky/mainline DHT construction QoS warnings, and retained fault-fixture/host output. Forty-six emitter/test files were compared with this branch's pre-fix head; the warning-emitting methods in the two changed files are unchanged. No introduced actionable finding was found. This is source and retained-evidence comparison, not an exact-base runtime replay or warning-free claim. The identical production patch reuses the two independent R1 source reviews and guard negative control from #1039, with the older child callers inspected separately.
All 14 commits in the actual current-parent PR range verified cryptographically locally and report GitHub verified=true / reason=valid after the normal push. Hosted unit/UI run 40850999737 passed on merge
b767b782into actual parentd9566ad8: 2,979 passed executions, 0 failed, 4 explicit fixture skips (2,950 unique passing definitions; one parameterized test has 30 runs). Both new confirmation tests passed, and hosted SwiftFormat passed 0/844. Full artifact and nested diagnostics were audited; the 286 duplicate native-symbol warning set matches retained history. A native keyboard queue timeout in the unchanged, passing tags-resume lifecycle test is retained. The hosted merge has the exact same source tree and package pins as this published branch; it is a separate Xcode 16.4/iOS 26.2 result from the local Xcode 27.1 run.Existing authored QA cases keep their recorded status and documented prerequisites. This propagation does not supply new native timing or controlled-fixture execution on the child. The external native R1 report on #1039 reports two successful controlled regtest executions at aae380f; it is attributed parent evidence, not a device run on this child. No new financial journey, binary adoption or final Android206/iOS215 latency/unlock result is claimed.
Models used
gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)