[ben-kaufman]

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 #1645; paired with iOS #1085.

Description

• Defers routine background refresh between dependent creation calls and releases the token on success, queued delivery, failure or cancellation.
• Preserves fresh authority checks, durable proposal storage, explicit Ordered/Interactive work and other active review tokens.

Out of Scope

None.

Design

N/A — no UI changes.

Preview

N/A — no user-visible behaviour change.

QA Notes

Setup

N/A — this internal scheduling change is exercised through the actual request-creation caller in automated tests. No new device journey is introduced.

Journeys

N/A — no user-visible behaviour change.

Manual Tests

N/A

Automated Checks

• added PaykitPaymentRequestRepoTest.kt — exercises creation admission through the actual caller, cancellation cleanup and a subsequent creation.
• updated PaykitPaymentRequestRepoTest.kt — checks queued delivery, pre-commit rejection, identity replacement and preservation of another review token.

Evidence

Validation for e552783b: the exact-base negative control failed at the intended admission assertion, and all 219 focused tests passed. Detekt SARIF and Lint XML/SARIF report zero findings. A combined build with #1656 passed all 4,440 unit tests, including the identical changed files and all 109 request repository tests. No isolated latency improvement is claimed. Work already admitted to the SDK continues normally.

Gaps

The final combined device run was blocked by Android input/unlock responsiveness before any payment authorization. Full payment timings and cancellation/expiry scenarios remain unverified on that build.

Models used

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

Model and effort were verified from the contributing execution records, including the independently linked review sessions.

[ovitrif]

Milestone 3.2.0: the Android twin of synonymdev/bitkit-ios#1085, on the same Paykit latency track (3.2.0).

[talosmachina]

No findings. propose() takes a payment review token after its pre-checks pass and holds it through the fresh eligibility check, the commit and the targeted delivery. While the token is held, checkRefreshAdmission(Priority.Background) turns background refresh away. The token is released in the existing finally. Reviewed e552783, full tier, reasoned from the code and from CI. Per the repo's Bitkit rule the branch was not built here.

What I checked, and 5 candidates I ruled out

Read in full: PaykitPaymentRequestRepo.propose and the helpers it calls (targetContext, eligibleTargets, processPendingMessages, proposalWasSent, publishCreatedRequest); PaykitSdkService.beginPaymentReview, checkRefreshAdmission, proposePaymentRequest, receiveAndObserveState; the background callers of checkRefreshAdmission in PrivatePaykitRepo.
Tests read: the three new cases and the updated ones in PaykitPaymentRequestRepoTest.kt.
CI at review time: detekt and lint green; build, build-local and unit tests still pending.

Ruled out

• Creation blocking itself: every SDK call made under the token runs at Interactive priority or without an admission check. That covers identityStatus, linkedPeers, canReceivePaymentRequests on the Interactive lane, proposePaymentRequest, processOutboundPrivateMessages(counterparty, Interactive) and allPaymentRequests(..., Interactive). Only Background priority is rejected, so creation never trips over its own token.
• Token leak on early exit: endReview is assigned only after the amount, availability and expiry checks pass, and it is a nullable var invoked in finally. A throw before it skips release, which is correct because nothing was taken. A throw or cancellation after it releases.
• Ending someone else's review: beginPaymentReview adds a fresh Any() to the set and the closure removes only that instance. A concurrent send-sheet review survives. The first new test asserts this.
• Retry loops spinning while the token is held: on OperationInProgress the PrivatePaykitRepo drain retry resets to the first delay and waits on withTimeoutOrNull(retryDelay) before trying again, and contact preparation delays. Neither busy-loops for the length of a slow delivery.
• proposeSubscription not covered: it shares creationMutex but takes no token. The PR scopes itself to one-time requests, so this is a scope choice, not a defect.

Merge confidence: 4/5, the build and unit test jobs had not finished when I reviewed, and I did not build the branch.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

One-time request creation now holds a review token through fresh eligibility checks, proposal storage, targeted delivery, and local publication.

• One-time request creation holds off background refreshes until it finishes.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Validate draft] --> B[Begin review]
    B --> C[Check fresh eligibility]
    C --> D[Store proposal]
    D --> E[Attempt targeted delivery]
    E --> F[Publish local result]
    C --> G[Finally: release this review]
    D --> G
    E --> G
    F --> G
    G --> H[Clear creation state and unlock]

Loading

_{Reviews (1) · Last reviewed commit: "perf: keep request creation ahead of ref..." · Reviewed by Greptile}

[github-actions]

Regtest APK

Built from c475334 (run).

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

[ovi-reviewer]

Verdict: ✅ Approve

Findings: None.

QA for e552783:

Test 1: request creation admission and cleanup ✅ The complete request-repository suite passed, checked from Gradle and JUnit results.

I ran PaykitPaymentRequestRepoTest at the exact head. Its assertions cover review admission during fresh eligibility, proposal commit and targeted delivery; cancellation during eligibility and delivery followed by another creation; queued delivery; fresh eligibility rejection; identity replacement; and preservation of another active review token. All cases passed without errors or skips.

Test 2: SDK admission regression ✅ The complete SDK-service suite passed, checked from Gradle and JUnit results.

I ran PaykitSdkServiceTest at the same head. The review-admission regression confirms passive refresh remains deferred until every review ends, releasing one token leaves another intact, and explicit operations remain available. All cases passed without errors or skips.

Check 1: exact-head build ✅ `assembleDevDebug` succeeded from the unmodified reviewed head.

I verified the checkout's commit and clean tracked state and saved the resulting universal APK's SHA-256.

No manual device journeys were run: the QA notes list none for this internal scheduling change. The admission behavior was exercised through automated assertions; native payment timing was not measured. No planned case remains unrun.

Full review: 8564ba2 to e552783, both changed files and the reached eligibility, delivery, cancellation, identity and admission paths. The earlier source review also covers this head.

Twin: synonymdev/bitkit-ios#1085 perf: keep request creation ahead of refreshes at 310e689 matches the one-time creation admission scope and token cleanup in source.

---

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

[ovitrif]

utAck

[piotr-iohk]

QA review

Scope: Full review of the complete PR diff against merge base 8564ba2, at e552783.

No new actionable code findings.

One-time request creation takes the existing payment-review token after the local amount, availability, and expiry checks, and holds it through fresh eligibility, proposal commit, targeted delivery, and local publication. Background refresh admission fails with OperationInProgress during that interval. Ordered and interactive work stays admitted. The closer runs in the existing finally on success, pre-commit rejection, failure, and cancellation, and removes the token this creation added.

The same one-time window is in bitkit-ios#1085 at 310e689. Subscription creation is unchanged on both platforms. Inbox polling, contact preparation, and private-message drain already retry after OperationInProgress, so a deferred refresh resumes on the next attempt.

Validation: inspected the creation path, admission helpers, background callers, and PaykitPaymentRequestRepoTest. This review did not execute those tests. CI build 39637426788 for e552783 includes testDevDebugUnitTest in the successful build job.

Device testing: not performed in this review. Journeys and manual tests are N/A for this internal scheduling change.

Merge depends on open bitkit-android#1645 (8564ba2).

[jvsena42]

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. No path holds the admission across a suspension it doesn't own.
• Twin synonymdev/bitkit-ios#1085 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): e552783 — not run yet; pending that measurement against master. Device results will follow in a separate review.