Skip to content

fix: present incoming requests automatically - #858

Merged
ovitrif merged 4 commits into
masterfrom
fix/payment-request-auto-popup
Oct 1, 2026
Merged

ovitrif merged 4 commits into
masterfrom
fix/payment-request-auto-popup

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Closes #863
Refs: synonymdev/bitkit-android#1404

This PR opens the existing Payment Request confirmation when a new request enters the pending queue.

Description

  • Triggers presentation for newly pending request IDs while preserving existing sheet ownership, retry, identity, and reviewed-request rules.
  • Defers presentation while the scene is inactive or PIN locked and resumes after unlock, without treating an interruption during preparation as a payment-target failure.

Out of Scope

  • Paykit delivery and payment execution latency: SDK preparation, polling, and payment submission are unchanged. Profile loading improvements are tracked in fix: speed up pubky profile loading #854.
  • Confirmation layout: the Android companion fixes clipped amount and payment controls; this PR preserves the existing iOS layout.

Design

Uses the existing Payment Request confirmation sheet in Bitkit handoff. No layout changes; the direct frame link has not been verified.

Preview

Incoming request opens after Receive closes

QA Notes

Journeys

  • new automatic-presentation.xml - Arrival opens confirmation, another sheet defers it, and reviewed requests stay dismissed.
  • updated request-summary.xml - Confirmation opens without the bell and preserves requester and note details.

Two-wallet regtest checks verified automatic presentation from Home, Receive remaining open while a second request arrived, presentation after Receive closed, and no repeated presentation after dismissing reviewed requests. Android also paid a 1,753-sat request successfully. Device QA applied #854 and Android #1399 in separate scratch builds; this PR does not claim delivery or execution latency is resolved.

Manual Tests

N/A

Automated Checks

  • updated PaykitPaymentRequestServiceTests.swift - Pauses refresh after publishing pending requests and verifies immediate dispatch; unchanged IDs do not retrigger presentation.

The final branch builds and passes all 177 selected request, private-payment, and confirmation tests on an iPhone 16 simulator. SwiftFormat lint passes for the two changed Swift files.

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Changes when incoming payment requests display to the user.

The PR should not merge until interruption during payment preparation resumes presentation promptly after foregrounding or unlock.

Summary

The PR detects newly pending Payment Request IDs and attempts to open the existing confirmation without waiting for refresh to finish. It also gates presentation on the active scene and PIN state, adds an unlock attempt, and updates a unit test, journeys, and the changelog.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[New pending request ID] --> B[Attempt presentation]
  B --> C[Await payment preparation and scan]
  C --> D{Scene active and PIN verified?}
  D -- Yes --> E[Open confirmation]
  D -- No --> F[Defer as target not routable]
  F --> G[Automatic request: 120-second retry deadline]
  G --> H[Foreground or unlock attempt cannot select request until deadline]
Loading

Reviews (1) · Last reviewed commit: "fix: present incoming payment requests a..."

@greptile-apps

greptile-apps Bot commented Oct 1, 2026

Copy link
Copy Markdown

Comments Outside Diff

These findings could not be posted inline.

  • P1 Backgrounding delays request confirmation Bitkit/AppScene.swift:1326 ▶

    If the app backgrounds or locks while payment preparation is in progress, this guard treats that interruption as a payment-target failure. For an automatically presented request, that sets a 120-second retry deadline. Returning to the app or unlocking can trigger another presentation attempt, but the request remains ineligible until the deadline, so its confirmation may not appear for two minutes.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Fixed in 7d67d34. Backgrounding or locking during preparation now exits without deferring the request or setting a retry deadline. The existing ownership checks still run before cleanup, and foregrounding or unlocking can retry immediately. The final branch builds and all 177 selected request, private-payment, and confirmation tests pass.

@ovi-reviewer ovi-reviewer Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advice: ✅ Approve

Review: diff 5 files.
Open synonymdev/bitkit-android#1405 presents the request when it arrives and has matching automatic-presentation and request-summary journeys. Android master still waits for refresh to finish. The confirmation layout fix applies only to Android; this iOS change keeps the current layout.

Findings:
1 inline (1 MEDIUM)

QA:
Tests running: 0 of 2 passed.


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

Comment thread Bitkit/AppScene.swift Outdated

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Full review of the complete PR diff against its merge base, at e3d857a.

No new actionable code findings.

Newly pending payment-request ids dispatch confirmation before the rest of refresh finishes. Reviewed ids stay filtered by requestsForPresentation(), and an open sheet still owns the screen until it closes. If the scene leaves the foreground or the PIN locks during preparation, send state is reset and the request stays eligible, with no 120-second automatic retry. Unlock calls presentation on its own; an already unlocked return to the foreground presents after the existing foreground refresh.

The background-during-preparation note on this comment matches that reset-and-remain-eligible path at this revision. Shared automatic-presentation and request-summary steps match bitkit-android#1405 at 5985d70, aside from id versus testTag. The Android confirmation-layout journey stays out of scope here.

Validation: This review inspected testPendingRequestArrivalDispatchesPresentationBeforeRefreshFinishes and did not run it. Run Integration Tests passed on this head. Run Tests was still in progress. The author reports the selected request tests passed locally; that run was not repeated here.

Recommended before device testing: Wait for Run Tests on e3d857a.

Suggested additional test cases

  • Platform: iOS, regtest, two linked Bitkit wallets, payer funded for at least 21,000 sats, PIN enabled.

  • Setup: payer unlocked on Home.

  • Action: send a 21,000-sat request and, before confirmation is visible, background the payer so the PIN locks; then foreground and unlock.

  • Expected: PaymentRequestConfirm appears after unlock without a 120-second wait. Dismiss it and background and unlock again; that same request stays dismissed.

  • Platform: iOS, same wallets, PIN disabled.

  • Setup: payer on Home.

  • Action: background the payer during preparation, then return to the foreground.

  • Expected: PaymentRequestConfirm appears with the foreground refresh, without a 120-second deferral.

Device testing: not performed in this review.

@ovi-reviewer ovi-reviewer Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advice: ✅ Approve

Reaudit: diff 3 files.
No new findings; the rest is in the review.

QA:
Tests queued.


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

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Follow-up review of the changes since e3d857a, at 8284ac0. The inherited baseline is the previous full review of the complete PR diff against merge base 0cdb704. This pass inspected the interruption-reset delta and rechecked the presentation, unlock, and foreground paths those tests rely on.

No new actionable code findings.

canPresentPreparedRequest still clears send state and leaves the request immediately eligible when the scene is inactive or the PIN is locked, and it skips that reset when the prepared context was already replaced. Returning from that guard does not call deferPresentation, so the 120-second automatic retry is not scheduled. Greptile's background-delay note does not apply at this revision. The coverage thread is addressed by testInterruptedPreparationRemainsImmediatelyPresentableAfterForegroundOrUnlock and testInterruptedPreparationPreservesReplacementPaymentContext.

automatic-presentation.xml and request-summary.xml still match bitkit-android#1405 at 5985d70, aside from id versus testTag. Android was not reviewed beyond those journey files.

Validation: Run Tests and Run Integration Tests passed on 8284ac0. This review inspected the new tests and did not run them. e2e-tests-local was still in progress and is not evidence for this change.

Ready for device testing of the journeys listed on the PR.

Suggested additional test cases

  • Platform: iOS, regtest, two linked Bitkit wallets, payer funded for at least 21,000 sats, PIN enabled.

  • Setup: payer unlocked on Home.

  • Action: send a 21,000-sat request and, before confirmation is visible, background the payer so the PIN locks; then foreground and unlock.

  • Expected: PaymentRequestConfirm appears after unlock without a 120-second wait. Dismiss it, then background and unlock again; that same request stays dismissed.

  • Platform: iOS, same wallets, PIN disabled.

  • Setup: payer on Home.

  • Action: background the payer during preparation, then return to the foreground.

  • Expected: PaymentRequestConfirm appears with the foreground refresh, without a 120-second deferral.

Device testing: not performed in this review.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@ovitrif
ovitrif merged commit e33dc08 into master Oct 1, 2026
18 checks passed
@ovitrif
ovitrif deleted the fix/payment-request-auto-popup branch October 1, 2026 17:16
@jvsena42 jvsena42 mentioned this pull request Oct 1, 2026
6 of 20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: incoming payment requests do not open the confirmation automatically

3 participants