Skip to content

feat: share paykit state across apps - #1401

Merged
ovitrif merged 81 commits into
masterfrom
codex/paykit-shared-runtime-local-20260930
Oct 7, 2026
Merged

ovitrif merged 81 commits into
masterfrom
codex/paykit-shared-runtime-local-20260930

Conversation

@ben-kaufman

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

Copy link
Copy Markdown
Contributor

Closes #1356

This PR moves Bitkit to Paykit's identity-wide shared state using the published 0.1.0-rc69 SDK.

SDK: https://github.com/pubky/paykit-rs/releases/tag/v0.1.0-rc69

Companions: iOS #856, Paykit Server #46.

Description

  • Completes reconciled hardware payments without rebroadcasting and retains the result across Activity recreation until completion is acknowledged.
  • Supports one-time absolute payment deadlines, with expiry checks at submission, separate acceptance deadlines, and late proof delivery.
  • Deletes contacts with one bulk block/remove operation, pauses background contact preparation during profile deletion, and batches local cleanup. Active subscriptions, busy peer leases and public contact markers remain protected.
  • Explicit contact re-add and import save contacts and unblock their selected peers atomically, without per-contact writes or compensating re-block loops. Label edits do not unblock peers.
  • Uses encrypted homeserver state and one Encrypted Link per contact identity, shared by authorized Paykit apps.
  • Stores wallet reservations, pending payment proofs, and recovery backups locally while following shared request state and execution claims.
  • Retains broadcast transaction IDs before remote identity reads, defers publication for unavailable contact links, and preserves manually detached activity contacts through sync.
  • Saves one-time acceptance intent before the remote call and includes it in wallet backups, so interrupted acceptance and wallet restore can resume safely, including shared-state lock conflicts. Execution rechecks subscription state after asynchronous lookups, and acceptance IDs are removed only when shared state confirms payment, cancellation, or rejection.
  • Uses atomic claim-and-accept after preparation, with interactive queue priority. Acceptance is durable before proceeding; peer delivery runs separately and remains retryable. Proof-triggered refreshes and backup exports wait until payment submission finishes, without dropping pending work or crossing identity changes.
  • Lets Bitkit authorize Paykit access, a watch-only account, or both as independently requested claims, without sharing wallet spending keys.
  • Shows the requested access in the authorization sheet. Server reconnect requests Paykit access without allocating another watch-only account.
  • Pays the exact endpoint supplied by a Payment Request, permits a fixed on-chain destination for unpaid recurring periods, and attributes received activity through its matching receiving address, including companion accounts used by Paykit Server. Unchanged history backfills are skipped, while missing transaction details and failed address derivation remain retryable.
  • Handles contact publication, requests, and subscriptions using app-owned endpoints inside the shared identity; resolves Paykit from GitHub Packages rather than mavenLocal().
  • Keeps local contact-sharing settings off when cleanup fails, retries withdrawals and registry updates, and discovers shared-state recipients even while links are recovering, and keeps unfinished withdrawals pending. Serializes private/public cleanup with sharing changes and coalesces foreground retries.
  • Includes app ownership fields when validating subscription proposals against the transport size limit. One-time and subscription proposals deliver to the selected recipient without draining unrelated peers.
  • Reuses validated Paykit keys and backup fingerprints for unchanged state, refreshes keys after identity errors, and retries session restoration during foreground maintenance.
  • Checks incoming private messages during the ten-second foreground poll without draining outbound work. Full synchronization runs on startup, explicit refreshes, and elapsed-time maintenance after 30 seconds, then every 60 seconds. Notification handling can reload saved requests without network intake. Temporary session-restoration transport and lock failures retain the saved session for retry instead of starting another auth flow.
  • Temporary restoration failures retain credentials and get a bounded short retry window while foregrounded, online and Paykit-enabled. Restoration stays single-flight; identity changes and cancellation stop pending retries without interrupting active SDK writes. Invalid credentials still require recovery.
  • Completes public payment setup before preparing private contacts in the background. Coalesces repeated preparation, reuses established links, backs off unavailable lookups, and invalidates pending publication during cleanup. Recipient-discovery timeouts apply to the lookup itself, not time queued behind other SDK work.
  • Reads public request capabilities up to eight contacts at a time outside the SDK mutation queue, without waiting for all-contact preparation. Private-message retries drain only the affected peers; idle cleanup skips unrelated work. Full private cleanup refreshes the registry once and retains retry state until that refresh succeeds.
  • Reopens a queued payment sheet when it replaces a dismissed sheet, even when the expanded flag remains unchanged.
  • Shows incoming requests in the existing payment sheet while preparing, with payment disabled until validation finishes. The same sheet stays open across preparation retries; closing it stops automatic retries and prevents a late result from reopening it. Failed payments remain retryable.
  • Prioritizes selected-recipient and request-delivery work over queued background reads while preserving active SDK calls and identity-change barriers.
  • Normalizes uppercase Bech32 request addresses for attribution while preserving validation and ambiguity checks.
  • Runs Dev Settings Paykit-disable cleanup through the same coordinator as contact-sharing changes.
  • Retains a subscription reminder's selected period through failed refreshes and temporary exclusions. A missing reminder does not block manual Pay or retries for another request; it resumes after that flow closes. Identity changes discard it.
  • Updates an open Sent receipt from matching outgoing request history, preserving a nonblank note and falling back to the creation snapshot when no match is available.

Out of Scope

  • Guaranteed private-list withdrawal on contact deletion. Deletion blocks immediately even if withdrawal fails. The old list can remain at the peer, and registry cleanup can remain pending until the contact is explicitly re-added.
  • Migration from receiver-folder data. Paykit has not launched, so that development data is unsupported.
  • Homeserver lock-finalization safety: the SDK cooldown is a mitigation, not a fix for a write completing after lock expiry.

Design

N/A — no design available.

Preview

QA Notes

Journeys

  • J1 updated delete-profile.xml - bulk deletion with 62 contacts; active-preparation runs completed on both platforms, with a settled/idle repeat still pending.

  • J2 updated import-all-contacts.xml - Continue completes public setup without waiting for every contact to link; retest with 61 contacts and unavailable profiles.

  • J20 Repeat import-all-contacts.xml with the same 62-contact identity after profile deletion. Three imports and two overlapping deletions completed on rc68; profile metadata timeouts and fully idle deletion remain open.

  • J3 new cancellation-during-confirmation.xml - a subscription canceled while confirmation is open cannot be paid after its cancellation is received.

  • J4 new fixed-onchain-destination.xml - later unpaid subscription periods can use the request's fixed on-chain address, while paid periods remain unavailable.

  • J5 new contact-payment-sharing.xml - disabling contact payments stays off after leaving and returning to Settings.

  • J6 updated automatic-presentation.xml - linked contacts on separate identities show new requests in the payment sheet, keep payment disabled during preparation, and defer presentation while another sheet is open. The reviewer verified three queued-sheet dismissal/replacement cases on 7bc3ccb/rc69, including no reopening after dismissal: feat: share paykit state across apps #1401 (comment).

  • J7 new paykit-only-approval.xml - approves Paykit access without creating a watch-only account.

  • J8 new paykit-reconnect.xml - renews server access without replacing its account or invoices.

  • J9 new accepted-device-ownership.xml - only the accepting install can resume a one-time request after restart.

  • J10 updated contact-request-or-pay.xml - contact payments and requests use identity-wide state.

  • J11 updated delete-and-readd-contact.xml - deletion blocks private requests and refreshes the list without waiting for another poll.

  • J12 updated definite-pre-broadcast-retry.xml - a failed request can retry immediately using fresh state.

  • J13 updated issuer-interoperability.xml - requests from another app retain their exact endpoint and request context.

  • J14 updated payment-deadline-history.xml - expired one-time and unsupported recurring deadlines remain visible without enabling payment.

  • J15 updated request-summary.xml - request details show the shared request and endpoint correctly; an open no-note Sent receipt follows matching request history.

  • J16 updated wallet-leg.xml - authorizes the server, pays its exact request destination, attributes the received payment, and preserves manual detachment after sync and restart.

  • J17 updated create-and-propose.xml - oversized proposals are rejected before sending, and shorter proposals use the shared identity and app ownership.

  • J18 updated requested-resolution-failure.xml - one preparing sheet stays visible across retries; closing it stops automatic retries and prevents reopening.

  • J19 new absolute-payment-deadline.xml - accepted requests remain payable until their payment deadline; expiry during callbacks, fee/PIN entry, signing or queue waits prevents submission, while earlier uncertain broadcasts and late proofs remain recoverable. The Android lost-response/expiry/reconciliation case passed on 50811b2/rc68; the remaining deadline and reattachment cases are still unverified.

  • J21 new delete-newly-saved-contact.xml - deletion from Contact Saved returns to Contacts; Back does not reopen the deleted contact. Fixed-head device verification remains pending.

Manual Tests

  • 1 With network fault injection, overlap resume and identity-readiness requests during a failed restore, then recover and retry. Waiting callers must share the active attempt; a later attempt must remain available. Delay the own-profile lookup and confirm saved contacts load independently.

  • 2 Follow paykit-clock-changes.md, including a cold-launch reminder with a failed startup refresh. A later successful refresh must open the selected unpaid period without another notification tap.

  • 3 Hold sharing withdrawal in progress, foreground the app, then request sharing on again. Cleanup must not overlap, and publication must wait for it to finish. Repeat with foreground cleanup already active, and with Paykit UI disabled/re-enabled before Contact Payments is enabled; this requires fault injection.

  • 4 Drop the acceptance response after its durable commit, restart Bitkit, then refresh and retry the accepted one-time request. The accepting installation must retain ownership, and retry must not duplicate a payment. This requires fault injection.

  • 5 Inject private withdrawal and public/app-registry update failures, then disable contact payments. Both sharing settings must stay off and cleanup must remain pending until recovery, without re-sharing cleared endpoints. Repeat with only the public/app update failing, and with a recipient removed by another authorized app while Bitkit has no local contact cache, including Linking and RecoveryRequired recipients.

  • 6 Force-stop while a shared-state lock is held, relaunch, and keep the app foregrounded and connected. Session setup must recover after the lock expires without another resume or connectivity event.

  • 7 Back up an accepted but unpaid one-time request, stop the original wallet, then restore on a replacement install and retry. Automated wallet backup/restore is not a journey capability. Running the same wallet on multiple devices concurrently is unsupported.

  • 8 Hardware broadcast recovery: follow the manual fault-injection checklist. Drop a successful broadcast response, let the deadline expire, and verify reconciliation completes without rebroadcasting, restores navigation, and survives Activity/view recreation. The Android lost-response/expiry/reconciliation case passed on 50811b2/rc68 with a Trezor emulator and local regtest. Activity reattachment and the remaining failure combinations still require controlled fault injection; these are not automated journey capabilities.

Automated Checks

  • added PaykitReceivedPaymentContactsTest.kt and ActivityServicePaykitContactsTest.kt - receiving-address attribution, rejection of unrelated outputs, companion-account lookup, and backfill cache invalidation.
  • updated PrivatePaykitContactResolverTest.kt and LightningServiceTest.kt - reservation/request ambiguity checks and account-specific derivation.
  • added RefreshContactPaykitLinkUseCaseTest.kt - refreshes an identity link without receiver selection.
  • added PaykitKeyGenerationTest.kt - initial generation selection, cached key reuse, remote rotation, rollback rejection, and invalidation after identity errors.
  • updated PaykitBackupStateTrackingTest.kt, ContactPaymentSettingsRepoTest.kt, and AppViewModelSendFlowTest.kt - cached backup fingerprints, uncertain-write checks, disabled sharing after cleanup failure, and foreground session-restoration retries.
  • updated PaykitSdkServiceTest.kt, PubkyRepoTest.kt, and PubkyAuthApprovalViewModelTest.kt - identity setup, authorizer access, separate or combined claims, and discovery timeouts that exclude SDK queue waits.
  • updated PaykitPaymentRequestRepoTest.kt, PaykitPaymentProofRepoTest.kt, and PrivatePaykitRepoTest.kt - exact destinations, execution ownership, fresh snapshots after state changes, publication, and cleanup failure reporting with retained retry state.
  • updated PaykitPaymentRequestPresentationStoreTest.kt, PaykitPaymentRequestRepoSubscriptionTest.kt, and AppViewModelSendFlowTest.kt - durable acceptance intent and wallet restore, identity-switch guards, and acceptance before LNURL invoice lookup.
  • updated CreatePaymentRequestScreenTest.kt - live Sent and Proof submitted states, note precedence, full-ID/outgoing matching, and creation-snapshot fallback.
  • removed RefreshContactPaykitReceiversUseCaseTest.kt - contact refresh targets the identity instead of discovering receiver folders.
  • ran the iOS/Android/server regtest flow on published rc58: Android paid a fresh 17,000-sat request to its exact address, the server confirmed it, and iOS attributed it to Buyer. All 37 simultaneous-sync readiness samples stayed healthy.
  • verified rc69 resolves from GitHub Maven without a local override; the downloaded AAR matches published module metadata and its native library matches the APK.

All 3,587 unit tests and production compilation passed against published rc69 without a local override. Deferred-restoration tests cover retry bounds, identity changes, cancellation, single-flight calls and foreground/connectivity lifecycle. Final format and Detekt reports match the verified baseline: 19 existing findings, none introduced. Coverage includes atomic acceptance, payment-priority barriers, deferred backup and proof refreshes, absolute deadlines, hardware broadcast uncertainty, late proofs, bulk contact cleanup, cancellation and identity changes. Current-head matched device timings and the complete deadline device journey remain unverified; outstanding Sent receipt and cold-start reminder checks are not closed by these unit tests.

Performance is not signed off. Paired mobile measurements on the rc66/rc67 runtime recorded Send Request at 12.11s, confirm/swipe to native send at 21.76s, first-returned LINKED at about 81s, and full sharing cleanup at 42.96s. These single samples predate the final queue fix and do not establish current-head latency. Cold start, backup stalls, sharing cleanup, reminder recovery and full content unlock remain open in #1406 and #1419 and the unchecked journeys above.

The rc68 62-contact staging retest completed three imports and two deletions of the same identity, without reproducing the previous contact-save/re-import stall. Deletion reached onboarding in 20.56s and 17.34s. The final preview took 103.51s with 45 profile timeouts; Import All then took 5.78s and Continue took 22.51s. These are individual UI-polled observations. Preparation retries remained active, so fully idle deletion and private-link readiness are unverified. Metadata-timeout causes remain unproven, and no payment was attempted.

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 0/5

[High risk] Restructures Paykit payment state and authentication across the app.

The PR is not safe to merge until inbound attribution, private-sharing failure handling, and existing backup restoration are addressed.

Findings

  1. P1 Security Unrelated output misattributes payment ▶
  2. P1 Failed cleanup leaves sharing advertised ▶
  3. P1 Existing Paykit backups cannot restore ▶

Summary

The PR moves Paykit contact links, payment requests, authorization claims, and app publication to identity-wide shared state, and updates payment proofs and received-payment attribution. It also changes persisted Paykit backup formats. Review findings concern inbound contact attribution, private-sharing cleanup failures, and restoring existing backups.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Shared Paykit requests] --> B[Contact endpoint index]
  B --> C[Inbound activity attribution]
  A --> D[Bitkit request and payment flow]
  D --> E[Local pending proofs and wallet backup]
  F[Sharing settings] --> G[Private-list cleanup]
  G --> H[Published app capabilities]
Loading

Reviews (1) · Last reviewed commit: "docs: clarify paykit integration contrac..."

Comment thread app/src/main/java/to/bitkit/services/CoreService.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/models/PaykitPaymentStateBackup.kt
@greptile-apps

This comment was marked as outdated.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from fd71df5 (run).

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

@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 91 files.
Pair PR synonymdev/bitkit-ios#856: equivalent.

Findings:
9 inline (1 MEDIUM, 8 LOW)

QA:
Tests queued.


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

Comment thread app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PaykitPaymentProofRepoTest.kt Outdated
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PrivatePaykitRepoTest.kt Outdated
Comment thread app/src/main/java/to/bitkit/services/PaykitSdkService.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PaykitReceivedPaymentContactsTest.kt Outdated
Comment thread app/src/main/java/to/bitkit/services/PaykitSdkService.kt
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Fixed the cleanup reporting issue from this comment in e0ade20. Failed private-list withdrawal now returns a failure to callers while keeping the pending marker and cached publication for retry. The sharing preference stays disabled.

ovi-reviewer[bot]

This comment was marked as resolved.

piotr-iohk

This comment was marked as outdated.

jvsena42

This comment was marked as outdated.

@ben-kaufman

This comment was marked as resolved.

piotr-iohk

This comment was marked as resolved.

@piotr-iohk

piotr-iohk commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Staging e2e pass, including paykit suite: https://github.com/synonymdev/bitkit-android/actions/runs/36861644059 ✅

@ben-kaufman
ben-kaufman requested a review from piotr-iohk October 1, 2026 12:33
jvsena42

This comment was marked as resolved.

@ben-kaufman
ben-kaufman requested a review from jvsena42 October 1, 2026 13:05
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-shared-runtime-local-20260930 branch from 098464f to ae0764a Compare October 1, 2026 13:30
@ovitrif

This comment was marked as outdated.

ovi-reviewer[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

@ovitrif I checked the red build. APK compilation passed. LightningNodeServiceTest failed during setup because Robolectric could not download android-all-instrumented:14-robolectric-10818077-i7 (Connection reset by peer), not because of a Paykit assertion or compile error.

The updated branch passes compilation and all 3,124 unit tests locally. The new push starts a fresh CI run.

Comment thread app/src/main/java/to/bitkit/services/CoreService.kt Fixed
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Fixed J6 in 7bc3ccb: when Receive closes and a queued request replaces it, SheetHost now expands the replacement even if its expanded flag stays true. The emulator regression fails without the fix. The actual rc68 request flow also showed the queued incoming sheet after Receive closed, once only.

This also updates Android to published rc69. All 3,580 unit tests and four SheetHost emulator tests passed; the APK matches the remote Maven native library, with no introduced lint/format findings. The side chat’s hardware fixes are preserved.

J19 is still unconfirmed. Accepted requests already use the payment deadline rather than proposal expiry; the strengthened LNURL callback-failure/retry test passes. I have not added an expiry bypass or marked the device report resolved. The request/state evidence requested above is still needed.

rc69 bounds a contended SDK call to one acquisition batch. It does not shorten lock expiry or the uncertain-write safety wait, and it is not a claim that startup/linking performance is resolved.

@ben-kaufman
ben-kaufman requested a review from piotr-iohk October 7, 2026 13:34
ovitrif
ovitrif previously approved these changes Oct 7, 2026

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

lgtm

Triggered a final bot review and post to summarize all tests it did on my side, verdict stays Approve.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

The new CI run failed one manifest-alias test before it toggled either alias. That manifest/test code is unchanged by 7bc3ccb. Two isolated executions and a forced full run of all 3,580 tests passed locally; I could not reproduce a routing defect. I reran the failed CI job unchanged rather than changing alias behavior without evidence. The rerun is pending, so this is not yet marked resolved.

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

Suggestion: 👍 Approve

Reaudit: diff 14 files.
New findings: 1 inline (1 LOW); the rest is in the review.
Pair PR synonymdev/bitkit-ios#856 at b633684 handles definite and uncertain hardware failures the same way and has the same contact-deletion journey. The queued sheet fix is specific to Android Compose.

QA:
Tests queued.
Test 1 passed at fd04581.
Tests 2, J12, J16 passed at 9ffb08a.
Test 3 passed at 53d459a.
Test 4 passed at cd4648b.
Test 5 passed at 98a1570.
Test 6 passed at 3280a4e.
Tests 7-8, J1, J6, J9, J14, J19 failed at 660c397.
Tests J2, J5, J10, J13 passed at 660c397.
Test J4 passed in the review.
Tests J15, J17 passed at 9083227.

Replies:

@jvsena42: single-device relaunch and inferred lock ownership (comment)

rc69 limits batches that compete for the lock. Lock expiry and the safety wait after an uncertain write are unchanged. There is no new relaunch measurement, so the latency check remains open.

@ben-kaufman: repeat the single-device relaunch (comment)

The extra startup attempt remains removed. The repeated relaunch measurement and redacted SDK lock timings remain unverified.

@ben-kaufman: distinguish acceptance from backup/restore failure (comment)

Test 7 remains open; this change adds no device evidence for acceptance or restore. The result must show whether acceptance failed or was committed before the loss.

@ben-kaufman: distinguish queued presentation and accepted-request expiry (comment)

The Receive-to-request replacement now has a regression test in SheetHost. The accepted-request retry test separates proposal expiry from the payment deadline. J19, request IDs and timestamps, and the remaining hardware reattachment and injected-failure cases still need device evidence.

Note

Retest Suggested 1, 2, 6, J6, J10, J12, J18, J19, J21, 8

@ovi-reviewer retest 1,2,6,J6,J10,J12,J18,J19,J21,8

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

Comment thread app/src/main/java/to/bitkit/ui/components/SheetHost.kt
@jvsena42

jvsena42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Device gate (partial) — 7bc3ccb (Paykit rc69)

Two emulators on staging, build installed over the rc68 state from 50811b2. Device clock, UTC−3, 2026-10-07. Still running: relaunch timings and a payment in the other direction.

Queued request after closing a sheet: 3 of 3 presented. The phone sent a 1 000 sat request while the tablet had a sheet open; after the sheet was dismissed the Payment Request sheet opened by itself each time.

# Sheet open on the tablet Request sent (phone) Dismissal Dismissed at Request sheet in layout Ready (Decoded incoming Paykit payment request target) Then
1 Receive 10:39:47.78 Back 10:43:46.51 first dump, 10:43:47 10:43:51.663 (5.2 s) paid
2 Receive 10:47:10.84 scrim tap 10:49:48.93 first dump, 10:49:49 10:49:53.526 (4.6 s) dismissed with Back
3 Send, first screen 10:52:01.19 swipe down 10:54:22.49 first dump, 10:54:23 10:54:27.736 (5.2 s) paid
  • The blocking sheet stayed up in every sample while waiting (20, 11 and 10 samples at ~10 s); nothing replaced it.
  • Dismissing the request sheet with Back in attempt 2: Home with no sheet in 23 samples over 68 s, no reopen, the app stayed responsive.
  • Control with no new request: Receive, Send and the Settings backup sheet opened and closed 10 times in total (Back and scrim tap); nothing re-expanded after a close.
  • The log has no line for a request arriving while another sheet is open, so "already received" was inferred from the wait (2–4 min after Sent), not observed.

Upgrade in place: the session restores on both from rc68-written state; no failed validation, recovery_required or concurrent_update line. First launch: tablet 64.0 s with two shared_state_busy deferrals, phone 15.6 s with none.

10:37:24.083 Start proc (tablet)
10:37:46.845 DEBUG [PaykitSdkService.kt:399] Republished Pubky identity
10:37:51.278 WARN  [PubkyRepo.kt:488] Deferred session restoration, keeping saved session [SharedStateBusy='code=shared_state_busy, context=Pubky shared state remains locked; retry later']
10:37:54.796 ERROR [BackupRepo.kt:553] Backup failed for: 'WALLET' [AppError='code=shared_state_busy, ...']
10:38:02.150 WARN  [PubkyRepo.kt:488] Deferred session restoration, keeping saved session [AppError='code=shared_state_busy, ...']
10:38:09.131 WARN  [AppViewModel.kt:737] Failed to refresh public Paykit endpoints [SharedStateBusy=...]
10:38:16.079 WARN  [AppViewModel.kt:737] Failed to refresh public Paykit endpoints [SharedStateBusy=...]
10:38:28.040 INFO  [PubkyRepo.kt:437] Restored paykit session for 'pubkyc8…yy5c9co'

Payment, attempt 3: swipe → broadcast 29.9 s, no WARN/ERROR (f2_tablet_attempts.log).

10:54:58.479 DEBUG [AppViewModel.kt:4244] Swipe to pay event, checking send confirmation conditions
10:55:00.614 INFO  [Keychain.kt:144] Upserted value for key 'PAYKIT_PENDING_PAYMENT_PROOFS'
   -- 9.8 s silent --
10:55:10.462 INFO  [PrivatePaykitRepo.kt:431] Consumed private Paykit payment list version 67 for 'pubkypi…ce7qh7o'
   -- 5.4 s silent --
10:55:15.826 INFO  [Keychain.kt:144] Upserted value for key 'PAYKIT_ACCEPTED_PAYMENT_REQUESTS'
   -- 5.4 s silent --
10:55:21.249 INFO  [Keychain.kt:144] Upserted value for key 'PAYKIT_PRESENTED_PAYMENT_REQUESTS'
10:55:23.913 DEBUG [LightningRepo.kt:1493] sendOnChain: sats=1000, isTransfer=false, isMaxAmount=false, satsPerVByte=1
10:55:28.344 INFO  [AppViewModel.kt:4464] Onchain send result txid: 10e9068b…

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Re-reviewed 50811b2..7bc3ccb. One MEDIUM, inline: the rc69 bump makes the slow launch slower.

Queued request after closing a sheet: fixed, verified on the emulators. SheetHost.kt:145 keys the expand effect on visibilityKey as well, so a sheet that replaces the dismissed one in the same frame is expanded. 3 of 3 attempts presented the Payment Request sheet by itself after the blocking sheet was dismissed (Receive with Back, Receive with a scrim tap, Send with a swipe down), ready 4.6–5.2 s after the dismissal. Dismissing the request sheet did not reopen it over 68 s, and ten open/close rounds on Receive, Send and the Settings backup sheet showed nothing re-expanding. Timeline: #1401 (comment).

rc69 otherwise: the session restores from rc68-written state on both emulators. No failed validation, recovery_required or concurrent_update line in any log; every lock error is shared_state_busy, which the app already handles wherever it handles ConcurrentUpdate (PaykitExceptionExt.kt:13, PaykitPaymentRequestRepo.kt:1004).

Payments (three, 1 000 sats on-chain): swipe → broadcast 30.0 s, 31.0 s and 36.8 s, no WARN/ERROR apart from one RequestUnavailable refresh warning. Same range as rc68.

The test changes (BroadcastExceptionExtTest, AppViewModelSendFlowTest, SheetHostTest) read correctly; I did not run the instrumented SheetHostTest.

Device gate: 7bc3ccb — queued-sheet 3/3, controls clean; payments 30–37 s; relaunch 12.8–33.0 s when not deferred, 64–115 s when deferred (6 of 11 launches), no crash or ANR. Hardware paths are code-only as before.

Comment thread gradle/libs.versions.toml

@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 at 7bc3ccb, covering the entire delta since 9c87876, affected callers and prior concerns, with unchanged coverage inherited from the completed baseline. Base and merge base remain 6564f3b; identities and ancestry were verified locally.

No new actionable code findings.

The hardware-proof concern is addressed in source: typed pre-broadcast failures release a first attempt, while earlier uncertain submissions remain protected. Checked the Paykit rc69 delta at dd97fc9a4fa82e3d49fd3744157a3b8c5e1ba689 and compared hardware recovery, sheet handling and contact-deletion navigation with iOS 480b8d25, scoped to those flows. Broadcast classification was verified against Core v0.5.18.

Validation: inspected changed native tests. Targeted local unit tests stopped before compilation because the required JetBrains JDK 21 download returned HTTP 400. The author reports passing unit and SheetHost tests; this is attributed evidence. The local code-result artifact passed qa_contract.py validation. Remaining hardware fault-injection checks, accepted-request deadline recovery, performance and startup recovery remain unverified by this review. Best-effort withdrawal after contact deletion remains the documented limitation. Concurrent wallet/shared-Pubky use and pre-2.6.0 profiles remain excluded by product support rules.

Device testing: not performed in this review.

jvsena42
jvsena42 previously approved these changes Oct 7, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree on merge now and improve the performance on follow up PRs

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Thanks for the current-head device checks. The three successful queued-sheet cases in #1401 (comment) verify the J6 presentation fix, including dismissal without reopening. J19 still needs the accepted-request fixture already requested; it is not covered by those passes. The unchanged manifest-test rerun and all seven E2E shards are also green. The newly reported deferred-restore timing regression is being addressed separately in its inline thread.

piotr-iohk
piotr-iohk previously approved these changes Oct 7, 2026
@ben-kaufman
ben-kaufman dismissed stale reviews from piotr-iohk, jvsena42, and ovitrif via fd71df5 October 7, 2026 14:56

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

Verdict: ⛔️ Request Changes

Reaudit: diff 14 files.
New findings: 1 inline (1 LOW); the rest is in the review.
Pair PR synonymdev/bitkit-ios#856 at b633684 has the same definite-versus-uncertain hardware failure handling and the identical contact-deletion journey; the queued sheet effect fix is specific to Android Compose.

QA:
Tested on 8 Android 15 emulators; two Android 16 emulators (Pixel 10 Pro); Android 15 emulator (Pixel 10 Pro).

🟢 Tests 1, 3, 4, 5, 6, 7, J2, J3, J4, J5, J7, J8, J10, J11, J13, J15, J17, J18, J20
Test 1

Passed.

1.mp4

Test 3

Passed.

3.mp4

Test 4

After the acceptance response read failed, restart and retry showed the same request Accepted and paid it once, without a second acceptance.


Test 5

Passed on the second try on m5a-android-1 after the first failed on m1a for an environment reason (airplane mode does not block adb-reversed loopback): both sharing settings stayed off, cleanup stayed pending while the failure flag was set, then finished without publishing the endpoint again.

5.mp4

Test 6

Passed.


Test 7

The first attempt on m5a-android-1 could not establish a live payer SDK identity before acceptance. On m1a, unique fixture relay mappings and the legitimate native cooldown produced accepted state and durable intent; completed wallet backup, sequential matching-identity restore, and payment retry succeeded.

7.mp4

Test J2

Passed.

J2.mp4

Test J3

Cancellation closed the unpaid confirmation and removed payment controls.

J3.mp4

Test J4

Passed.

J4.mp4

Test J5

Passed.

J5.mp4

Test J7

Passed.

J7.mp4

Test J8

Passed.


Test J10

Linked contact Pay and Request routes completed within the timing budget.

J10.mp4

Test J11

Deleted contact requests stayed blocked until explicit re-add.

J11-readd.mp4

Test J13

Canonical issuer request reaches the existing confirmation flow with saved-contact attribution.

J13.mp4

Test J15

Passed.


Test J17

Passed.


Test J18

Passed.

J18-setup.mp4

Test J20

Passed.


🟠 Tests 2, 8, J1, J9, J12, J14, J16, J21
Test 2

Blocked: Native clock control did not take effect.

2.mp4

Test 8

Failed, and nothing shows whether our setup or the app caused it: Broadcast recovery fault controls unavailable.


Test J1

Failed, and nothing shows whether our setup or the app caused it: 62-contact deletion precondition unavailable.


Test J9

Failed, and nothing shows whether our setup or the app caused it: Managed device pair disappeared during setup.

J9-A.mp4

Test J12

Blocked: Fixture callback restoration failed over SSH.

J12.mp4

Test J14

Blocked: Automatic proposal presentation interrupted the explicit Overview route.

J14.mp4

Test J16

Blocked: Marketplace fixture grant blocked after wiped seller recovery.

J16-auth2.mp4

Test J21

Incomplete: the run ended before this test finished.


🔴 Tests J6, J19
Test J6

Deferred request did not present after Receive closed.

J6-setup.mp4

Test J19

Accepted request did not return to review after proposal expiry.

J19.mp4

Tip

Tests 3, 4, 7 worth a journey
Test 3
  • Enable contact payments for a saved linked contact.
  • Delay endpoint cleanup, turn sharing off, foreground the app, and request sharing on.
  • Fail private withdrawal while foreground cleanup is active and confirm sharing stays off with cleanup pending.
  • Disable and re-enable Paykit UI before enabling Contact Payments.
  • Release the faults and verify withdrawal completes before publication resumes.

Test 4

  • Open the payer payment requests and choose Pay on the 21 000 sat First request
  • Cut the network and swipe to pay
  • See the error toast while the balance stays 150 000
  • Restart Bitkit and restore the network
  • Open that request, see Accepted, and pay it
  • See Bitcoin Sent, then the request marked proof submitted

Test 7

  • Create a one-time request from a linked contact
  • Accept the request without paying and complete the wallet backup
  • Stop the original wallet
  • Restore its recovery phrase on the replacement install
  • Dismiss the ordinary restore warning and verify the wallet restored
  • Open the accepted request and tap Pay
  • Swipe to pay and verify Bitcoin Sent

Replies:

@jvsena42: single-device relaunch and inferred lock ownership (comment)

I read the single-device timeline. rc69 bounds contended acquisition batches; lock expiry and the uncertain-write safety wait remain unchanged. There is no new relaunch measurement here, so the latency check remains open.

@ben-kaufman: repeat the single-device relaunch (comment)

The extra startup attempt remains removed. The requested repeated relaunch measurement and redacted SDK lock timings remain unverified.

@ben-kaufman: distinguish acceptance from backup/restore failure (comment)

Test 7 remains open; this delta adds no acceptance/restore device evidence. Its result must distinguish failure to reach acceptance from loss after a committed acceptance.

@ben-kaufman: distinguish queued presentation and accepted-request expiry (comment)

Receive-to-request replacement now has direct SheetHost regression coverage. The accepted-request retry regression separates proposal expiry from the payment deadline. J19, request IDs and timestamps, and the remaining hardware reattachment and injected-failure cases still need device evidence.

Note

Retest Suggested 1, 2, 6, J6, J10, J12, J18, J19, J21, 8

@ovi-reviewer retest 1,2,6,J6,J10,J12,J18,J19,J21,8

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

@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 at fd71df54a, covering the entire five-file delta since 7bc3ccb1a, affected callers and prior concerns, with unchanged coverage inherited from the completed baseline. Base, merge base and ancestry verified locally.

No new actionable code findings.

No new actionable code findings in the follow-up. Source addresses the deferred-restoration retry gap with bounded retries, single-flight restoration and identity/lifecycle guards. Same-device timing verification and the lock-holder investigation remain open in that thread and #1419; unit coverage does not establish recovery latency. Checked scoped recovery parity with iOS 480b8d25 and restoration/error contracts in Paykit rc69 dd97fc9a. Best-effort withdrawal after contact deletion remains the documented limitation.

Validation: inspected the seven added retry tests and existing restoration callers/tests. CI compilation and unit tests passed at the exact reviewed head. Local targeted tests stopped before compilation because the configured JetBrains JDK 21 download returned HTTP 400. The controlled lock-holder manual case and baseline outstanding hardware/deadline, reminder and performance checks remain unexecuted here. Concurrent wallet/shared-Pubky use and pre-2.6.0 profile compatibility retain the baseline product exclusions.

Device testing: not performed in this review.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

@ovitrif could you check the ovi-reviewer report requesting changes: #1401 (review). Something worth to be reported as follow up?

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Following up on #1401 (review):

  • J6: the queued-sheet fix in 7bc3ccb is retained in fd71df5. Joao subsequently verified all three dismissal cases on devices: feat: share paykit state across apps #1401 (comment). Please identify the APK commit and request/dismissal timeline for the red result so we can distinguish an older run from a remaining failure.
  • J19 remains open. The screenshots show "Error fetching lnurl invoice", which alone does not establish a proposal-expiry rejection. Both platforms distinguish proposal expiry from the payment deadline; Android also tests callback failure followed by reopening an accepted request after proposal expiry. Please provide the request ID, committed acceptance, both deadlines, and redacted callback response/retry logs to identify the failing step.
  • Test 7: the results now report successful accepted-request backup, matching-identity restore and retry, but the Replies section still says no device evidence exists. Please reconcile these statements and identify the tested build.

The review's code delta is 9c87876..7bc3ccb, while the current head is fd71df5. Please refresh the verdict with current versus carried-forward results clearly separated. Bounded foreground restore retries are tested, but their same-device latency is still unverified. No additional code change or resolution of J19 is justified by the supplied evidence yet.

@ovitrif

ovitrif commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

I left an agent investigate the local artifacts from the tests run concerning what is referenced in:

AI agreed with my approach:

Go, then a worker investigates if those are worth raising as issues -- they aren't even confirmed yet to be either

  1. Local test env issue, or
  2. Pre-existing bugs not introduced by this PR

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

Verdict: ✅ Approve

Reaudit: diff 14 files.
New findings: 1 inline (1 LOW); the rest is in the review.
Pair PR synonymdev/bitkit-ios#856 at b633684 has the same definite-versus-uncertain hardware failure handling and the identical contact-deletion journey; the queued sheet effect fix is specific to Android Compose.

QA:
Tested on 8 Android 15 emulators; two Android 16 emulators (Pixel 10 Pro); Android 15 emulator (Pixel 10 Pro).

🟢 Tests 1, 3, 4, 5, 6, 7, J2, J3, J4, J5, J7, J8, J10, J11, J13, J15, J17, J18, J20
Test 1

Passed.

Ran on fd04581.

1.mp4

Test 3

Passed.

Ran on 53d459a.

3.mp4

Test 4

After the acceptance response read failed, restart and retry showed the same request Accepted and paid it once, without a second acceptance.

Ran on 9ffb08a.


Test 5

Passed on the second try on m5a-android-1 after the first failed on m1a for an environment reason (airplane mode does not block adb-reversed loopback): both sharing settings stayed off, cleanup stayed pending while the failure flag was set, then finished without publishing the endpoint again.

Ran on 9ffb08a.

5.mp4

Test 6

Passed.

Ran on 3280a4e.


Test 7

The first attempt on m5a-android-1 could not establish a live payer SDK identity before acceptance. On m1a, unique fixture relay mappings and the legitimate native cooldown produced accepted state and durable intent; completed wallet backup, sequential matching-identity restore, and payment retry succeeded.

Ran on 98a1570.

7.mp4

Test J2

Passed.

Ran on 660c397.

J2.mp4

Test J3

Cancellation closed the unpaid confirmation and removed payment controls.

Ran on 12a0045.

J3.mp4

Test J4

Passed.

Ran on fe364a5.

J4.mp4

Test J5

Passed.

Ran on 660c397.

J5.mp4

Test J7

Passed.

Ran on 12a0045.

J7.mp4

Test J8

Passed.

Ran on 53d459a.


Test J10

Linked contact Pay and Request routes completed within the timing budget.

Ran on 660c397.

J10.mp4

Test J11

Deleted contact requests stayed blocked until explicit re-add.

Ran on 12a0045.

J11-readd.mp4

Test J13

Canonical issuer request reaches the existing confirmation flow with saved-contact attribution.

Ran on 660c397.

J13.mp4

Test J15

Passed.

Ran on 53d459a.


Test J17

Passed.

Ran on 9083227.


Test J18

Passed.

Ran on 53d459a.

J18-setup.mp4

Test J20

Passed.

Ran on 660c397.


🟠 Tests 2, 8, J1, J6, J9, J12, J14, J16, J19, J21
Test 2

Blocked: Native clock control did not take effect.

Ran on 660c397.

2.mp4

Test 8

Failed, and nothing shows whether our setup or the app caused it: Broadcast recovery fault controls unavailable.

Ran on 9c87876.


Test J1

Failed, and nothing shows whether our setup or the app caused it: 62-contact deletion precondition unavailable.

Ran on 9c87876.


Test J6

Incomplete: our run at 660c397 predates the queued-sheet fix in 7bc3ccb; @jvsena42 passed all three dismissal cases on devices at 7bc3ccb (device gate).

Ran on 660c397.

J6-setup.mp4

Test J9

Failed, and nothing shows whether our setup or the app caused it: Managed device pair disappeared during setup.

Ran on 9c87876.

J9-A.mp4

Test J12

Blocked: Fixture callback restoration failed over SSH.

Ran on 660c397.

J12.mp4

Test J14

Blocked: Automatic proposal presentation interrupted the explicit Overview route.

Ran on 660c397.

J14.mp4

Test J16

Blocked: Marketplace fixture grant blocked after wiped seller recovery.

Ran on 660c397.

J16-auth2.mp4

Test J19

Blocked: Accepted request did not return to review after proposal expiry.

Ran on 660c397.

J19.mp4

Test J21

Incomplete: the run ended before this test finished.


Tip

Tests 3, 4, 7 worth a journey
Test 3
  • Enable contact payments for a saved linked contact.
  • Delay endpoint cleanup, turn sharing off, foreground the app, and request sharing on.
  • Fail private withdrawal while foreground cleanup is active and confirm sharing stays off with cleanup pending.
  • Disable and re-enable Paykit UI before enabling Contact Payments.
  • Release the faults and verify withdrawal completes before publication resumes.

Test 4

  • Open the payer payment requests and choose Pay on the 21 000 sat First request
  • Cut the network and swipe to pay
  • See the error toast while the balance stays 150 000
  • Restart Bitkit and restore the network
  • Open that request, see Accepted, and pay it
  • See Bitcoin Sent, then the request marked proof submitted

Test 7

  • Create a one-time request from a linked contact
  • Accept the request without paying and complete the wallet backup
  • Stop the original wallet
  • Restore its recovery phrase on the replacement install
  • Dismiss the ordinary restore warning and verify the wallet restored
  • Open the accepted request and tap Pay
  • Swipe to pay and verify Bitcoin Sent

Note

Retest Suggested 1, 2, 6, J6, J10, J12, J18, J19, J21, 8

@ovi-reviewer retest 1,2,6,J6,J10,J12,J18,J19,J21,8

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

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

lgtm

@ovitrif
ovitrif merged commit a1bc86e into master Oct 7, 2026
21 checks passed
@ovi-reviewer

ovi-reviewer Bot commented Oct 7, 2026

Copy link
Copy Markdown

Two corrections to the review:

  • Test 7: the recording and screenshots under it come from a later run that stopped before the request was accepted, not from the run at 98a1570 that passed. That pass posted no media.
  • J19: the "Error fetching lnurl invoice" in that test came from a failure our test injected into the LNURL callback on purpose. Whether the request should return to review after Retry is not established; J19 needs another run.

@ovitrif

ovitrif commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman @piotr-iohk the local test artifacts are investigated now:

  • J6: the red result ran on 660c397 (local APK built 10:15Z, before 7bc3ccb). The second request decoded at 10:46:39 but never became visible, which is the queued-sheet bug 7bc3ccb fixes. Joao's device check at 7bc3ccb covers it, so no failure remains.
  • J19: the "Error fetching lnurl invoice" came from a fault our test injected into the LNURL callback on purpose, after the app had committed acceptance (10:49:23.288, 11.8 s before proposal expiry). Request 1f8091b4-3fc5-4610-9fc3-e2238ec555a5, 21,000 sat, proposal expiry 10:49:35Z, payment deadline 10:54:05Z. Our issuer fixture synced the acceptance only after expiry, so its InvalidConflict is our artifact. Why the screen stayed on Payment Failed after Retry is not established from the 1-2 s recordings; it needs another run.
  • Test 7: the pass ran at 98a1570 (rc62) and posted no media; the media under it came from a later run that stopped before acceptance (corrected in feat: share paykit state across apps #1401 (comment)). Accept, backup and restore changed after 98a1570, so test 7 is unverified at fd71df5.

None of these shows a bug introduced by the PR, so there is nothing to report as a follow-up issue.

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.

chore: update paykit to the pubky 0.14 release

5 participants