Skip to content

fix: refresh Paykit views and local completion - #914

Open
ben-kaufman wants to merge 2 commits into
masterfrom
codex/paykit-observation-proof-handoff
Open

ben-kaufman wants to merge 2 commits into
masterfrom
codex/paykit-observation-proof-handoff

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

This PR:

  1. Refreshes existing Paykit display projections with one validated observation.
  2. Completes verified hardware-payment recovery locally before remote proof delivery or acknowledgement finishes.

Related: SDK #183, Server #78. Pins published rc73; neither PR needs to merge for this consumer build.

Description

  • Reuse existing refresh coalescing and display projections with identity/generation-checked observations, retaining previous data on observation failure while authoritative eligibility, authentication and action getters remain unchanged.
  • Publish hardware-recovery resolution after exact transaction verification and durable proof/activity/follow-up persistence, before serialized remote delivery; normal iOS proof delivery was already asynchronous.
  • Handle Pending resolution on the main queue and navigate before acknowledgement, preserving payer/wallet/transaction guards, actual SDK identity verification, retained proof on failure and restart retry without another payment.
  • Pin exact rc73 revision 0eff3d4e694ba0dbbea2d47852fe796e456671b0; the actual SwiftPM archive matches published SHA256 511715acf3ca84e322d19ea6651c767c98b5325845d571abe2a9527b1eb6ae26, with no local override or unrelated dependency change.

Out of Scope

Design

N/A - no UI layout or copy changes.

Preview

Not recorded: a dedicated funded hardware-payment recovery journey remains unrun; deterministic local fixtures cover the ordering change.

QA Notes

Journeys

  • updated shop-onchain-proof.xml - distinguishes durable local completion from remote acknowledgement; funded journey not run.

Manual Tests

N/A.

Automated Checks

  • added ContactsManagerTests.swift, PaykitPaymentRequestServiceTests.swift - preserve prior displays on failed/wrong-identity observations and keep cached display separate from action authority.
  • added PaykitSdkIdentityCheckTests.swift - enforce observation identity and cancellation without overlapping native operations.
  • added PaykitPaymentProofServiceTests.swift, TransferServiceActivityTests.swift - prove durable held-delivery/restart recovery and main-thread Pending completion before held acknowledgement, with exact-context rejection and proof retention.
  • updated ContactsListViewTests.swift, ProfileDestinationViewTests.swift, PaykitPaymentRequestServiceTests.swift - cover the explicit display loader and full-observation failure.
  • ran a clean published-package build and full CI-selected unit suite with locked versions on the dedicated simulator: 2,371 passed, zero failures (BASE: 2,363); eight tests added, none removed, same six CI exclusions, no funded/UI target. The test-only follow-up 3d2283e2 replaces fixed navigation/acknowledgement sleeps with bounded completion expectations; all 2,371 tests passed again with no introduced diagnostics and a clean bounded independent review. Production code and the measured read path are unchanged.
  • ran complete BASE diagnostic comparisons: all 79 compiler warnings and 102 SwiftFormat findings are pre-existing; zero introduced. Translation output is identical: zero errors, 1,898 existing warnings. Diff, project and journey XML checks passed.
  • ran bounded repetitions before the final published-package suite: 30/30 hold/recovery cases, 10/10 revised live-event Pending cases, and 137/137 contact-boundary cases passed; final local and published SDK artifacts are byte-identical.

Measurement Limits

  • Actual request-display refresh reads: two to one; contact display: one to one. Message processing is unchanged; broader SDK getter comparisons are not the iOS pipeline.
  • Matched 250 ms holds show existing warm-display isolation, and the hardware-recovery fix now publishes durable local resolution before delivery releases, retaining exact proof for restart retry. These are synthetic local isolation/recovery checks, not measured network or unlock speedups.
  • Published rc72/rc73 Swift bindings against the actual staging Homeserver: 60/60 native samples passed in ABBA blocks. Warm stored-request refresh measured midpoint medians 1,639.432 ms versus 814.080 ms (24 per arm); new handles measured 1,639.525 ms versus 828.013 ms (6 per arm). Fixture: 32 contacts, empty request/peer/receipt collections, no injected delay or shorter safety wait. This measures the service refresh, not rendered UI, cold app startup, populated history, linking, payment or unlock latency. Contacts-only and already-cached display have no demonstrated improvement.

@greptile-apps

greptile-apps Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High impact] Updates payment proof delivery and contact observation logic.

The PR appears safe to merge, with a non-blocking improvement needed in the acknowledgement test.

Findings

  1. P2 Test can pass too early ▶

Summary

Refreshes Paykit displays through one identity-checked observation and lets verified hardware-payment recovery reach Success before remote delivery or local acknowledgement finishes.

  • Contacts and request refreshes use one identity-checked observation for display data.
  • Verified hardware payments can reach local Success before proof delivery finishes.

Diagram

sequenceDiagram
    participant Recovery
    participant Store as Local proof store
    participant Activity as Local activity
    participant Pending
    participant SDK as Paykit SDK
    Recovery->>Recovery: Verify original transaction
    Recovery->>Store: Save verified proof
    Recovery->>Activity: Save local follow-up
    Recovery->>Store: Save completion awaiting consumption
    Recovery-->>Pending: Publish resolution on main queue
    Pending->>Pending: Check payer, request, wallet and transaction
    Pending->>Pending: Navigate to Success
    par Delivery
        Recovery->>SDK: Submit proof
    and Acknowledgement
        Pending->>SDK: Check active identity
        Pending->>Store: Save consumption when checks pass
    end
Loading

Reviews (1) · Last reviewed commit: "fix: refresh Paykit views and local comp..." · Reviewed by Greptile

Comment thread BitkitTests/TransferServiceActivityTests.swift

@talosmachina talosmachina 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.

No findings. Swaps the request and contact display reads for one identity- and generation-checked observeState observation, and publishes verified hardware-payment resolution before proof delivery on the reconcile path. Reviewed abc28553, full tier, reasoned from the code, the pinned paykit-rs source and CI; iOS is not built here.

What I checked, and 4 candidates I ruled out

Read in full: completeHardwareOnchainPayment, consumeOnchainPaymentResolution, submit/submitInBackground (PaykitPaymentProofService.swift), PaykitSdkService.observeState, withSdk, refreshPaykitKey, PaykitSdkOperationLock (PubkyService.swift), ContactsManager.loadContacts, PaykitPaymentRequestService.synchronize, PaykitPaymentRequestManager.refresh/performRefresh, SendPendingScreen.applyOnchainPaymentResolution
SDK side: observe_state, contact_records, list_payment_requests, load_remote_state at paykit-rs 0eff3d4 (tag v0.1.0-rc73, matches Package.resolved)
Call sites traced: onchainPaymentResolutionPublisher (AppScene, HwSendSignView, SendPendingScreen), loadContacts(for:) (AppScene, ContactsManager), service.synchronize (only performRefresh)
CI: validate, detect-changes and Greptile green; Run Tests, Run Integration Tests and build-local still running at review time

Ruled out

  • Duplicate proof submission when resolution is published before submit on the reconcile path: consumeOnchainPaymentResolution can now interleave with the foreground submit(completed) and start a second submit, both doing a check-then-submit against paymentRequests(). The same interleave already exists on the default deliverInBackground path, and the SDK documents a repeated proof for the same occurrence as evidence that implies no second payment.
  • observeState failing where the old getters returned data: signed-out, no Paykit identity secret, and uninitialized identity all errored on the old contact_records() / payment_requests() path too, because the shared storage transaction needs the same session and secret. A missing blob with no registry noise key still yields default state, so missingDataIsEmpty: false loses no empty-list case.
  • performRefresh now requires activeIdentity before syncing: the only state this skips is a manager whose stored acceptance or subscription state failed to load. Outbound proof delivery still runs from PaykitPaymentProofService.submit and the other processPendingMessages callers.
  • Dropped identity guard after consume in SendPendingScreen: applyOnchainPaymentResolution already matches resolution.identity against pubkyProfile.publicKey on entry, and navigation now runs on the main queue via the added .receive(on:).

Merge confidence: 4/5, the unit and build jobs were still running and the iOS target was not built here.

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.

2 participants