Skip to content

feat: share paykit state across apps - #856

Open
ben-kaufman wants to merge 89 commits into
masterfrom
codex/paykit-shared-runtime-local-20260930
Open

ben-kaufman wants to merge 89 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 #815

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: Android #1401, Paykit Server #46.

Description

  • Reconciles hardware payments against their own wallet activity, preserves that wallet scope through backup/restore, and completes confirmed sends without rebroadcasting after expiry. Recovery results stay available across view recreation.
  • 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 instead of substituting a later private list, permits a fixed on-chain destination for unpaid recurring periods, and attributes received activity through its matching receiving address, including companion accounts.
  • Combines reservation and request attribution, leaves ambiguous transactions unlabeled, and skips unchanged history backfills.
  • Handles contact publication, requests, and subscriptions using app-owned endpoints inside the shared identity.
  • Saves contact-sharing OFF and cleanup-pending before withdrawal, preventing new endpoint publication during cleanup. Serializes private/public cleanup with sharing changes and coalesces foreground retries. Retries failed withdrawals and registry updates, and discovers shared-state recipients even while links are recovering, and keeps unfinished withdrawals pending.
  • Includes app ownership fields when validating subscription proposals against the transport size limit. One-time and subscription proposals revalidate and deliver only to the selected saved recipient, without draining unrelated peers.
  • Reuses validated Paykit keys and backup fingerprints for unchanged state, and refreshes keys after identity errors without replaying failed writes.
  • 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 do not show a session-expired warning or force reauthorization; 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. Unchanged contact keys do not trigger preparation when SwiftUI rebuilds the view.
  • 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.
  • Shows incoming requests in the existing payment sheet while preparing, with payment disabled until validation finishes. Closing the sheet 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.
  • Retains due-reminder targets through failed, canceled or stale refreshes. Manual Pay and payment retries take priority without dropping an unrelated reminder or interrupting an active payment.
  • 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.

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; fully idle deletion and private-link readiness 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.

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

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

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

  • 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 request-summary.xml - request details show the shared request and endpoint correctly.

  • J15 updated open-watch-only-link.xml - the OS handoff opens the requested authorization flow.

  • 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 - progress is visible while preparing and clears after failure.

  • 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 lost-response/expiry/reconciliation case passed on b633684/rc69 in the normal app with a Trezor emulator and local regtest. The remaining deadline and reattachment cases are still unverified.

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

  • J22 new delete-newly-saved-contact.xml - deletion from Contact Saved returns to Contacts; Back does not reopen the deleted contact. Verified by the reviewer in three staging deletion/re-add runs on 291328d; see the device report.

Manual Tests

  • 1 With network fault injection, overlap foreground/connectivity recovery requests while restoration fails, then recover and retry. Waiting callers must share the active attempt; a later attempt must remain available.

  • 2 Tap a due subscription reminder during background contact preparation, both with Bitkit open and on cold launch. Follow the reminder checks and record tap-to-sheet timing separately from authentication and SDK lock waits.

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

  • 7 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. Lost-response/expiry/reconciliation passed on b633684/rc69 with no extra broadcast after expiry, normal completion/navigation and one matching proof. View reattachment, process death and other failure combinations remain unverified; these require controlled fault injection and are not automated journey capabilities.

Automated Checks

  • added PaykitReceivedPaymentContactsTests.swift - combined attribution, cache invalidation, and a database-backed test of backfill retry, saved contact attribution, and skipped completed scans.
  • added AddressSearchCoordinatorTests.swift - companion-account lookup, isolated search indexes, and conservative handling of unknown outputs.
  • updated PaykitSdkClientConfigTests.swift and PubkyProfileManagerTests.swift - shared identity setup, cached key reuse, rotation and rollback rejection, and identity switching.
  • updated PaykitBackupStateTrackingTests.swift - cached backup fingerprints and rechecking uncertain writes.
  • updated PubkyAuthRequestTests.swift, PubkyAuthApprovalSheetTests.swift, and WatchOnlyAccountServiceTests.swift - independent claims, combined consent, and malformed request rejection.
  • updated PrivatePaykitServiceTests.swift, PaykitContactLifecycleTests.swift, and ContactPaymentsServiceTests.swift - publication ordering, deferred work, contact cleanup, and attribution.
  • updated PaykitPaymentRequestServiceTests.swift, PaykitPaymentProofServiceTests.swift, and PaykitPaymentStateBackupTests.swift - request destinations, execution ownership, fresh snapshots after state changes, and retained wallet payment state.
  • added PaykitPaymentActivityTests.swift and updated PaykitSdkOperationLockTests.swift - deferred proof refresh and delivery, payment-priority barriers, cancellation, and backup admission across wallet changes.
  • removed PaykitReceiverNoiseKeyStoreTests.swift - keys belong to the identity, not individual receivers; authorizer coverage is in PaykitSdkClientConfigTests.swift.
  • ran the iOS/Android/server regtest flow on published rc58: a 17,000-sat payment used the request's exact address, the server confirmed it, and iOS showed Received from Buyer. All 37 simultaneous-sync readiness samples stayed healthy.
  • verified SwiftPM resolves rc69 from the published release tag and its downloaded framework archive matches the manifest checksum.

Local validation against published rc69: the simulator app/test build and 610 focused native tests passed, covering hardware send coordination, request execution, proof reconciliation and contact navigation. SwiftPM uses the published release without a local package override. Complete compiler, formatter and translation reports have no introduced diagnostics against the base; the 18 existing format findings remain unchanged. These tests do not replace the unchecked device journeys or establish payment-flow latency.

Performance is not signed off. Paired mobile measurements on the rc66/rc67 runtime recorded Send Request at 13.68s, confirm/swipe to native send at 15.56s, first-returned LINKED at about 80s, and private sharing withdrawal at 30.63s (not full cleanup). These single samples predate the final queue fix and do not establish current-head latency. Cold start, backup stalls, sharing cleanup, reminder failure recovery and full content unlock remain open in #868 and the unchecked journeys above.

The reminder cold-launch network-failure journey is not yet device-tested.

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. Preview took 15.89-16.44s, Import All 1.358-1.830s and Continue 8.993-9.530s. Deletion reached onboarding in 17.068s and 15.324s. These are individual UI-polled observations. Preparation retries remained active, so fully idle deletion and private-link readiness are unverified; no payment was attempted.

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 0/5

[High risk] Updates payment SDK and refactors payment state sharing across apps.

The PR should not merge until received-payment attribution and compatibility with existing persisted payment state are addressed.

Findings

  1. P1 Security Unrelated payments gain payer labels ▶
  2. P1 Older wallet backups cannot restore ▶
  3. P1 Existing pending proofs become unreadable ▶
  4. P1 Legacy reservation keys lose contacts ▶

Summary

This PR moves Paykit integration from receiver-specific local state to identity-wide shared state, adds independent authorization claims, and uses shared requests for payment and received-activity attribution.

  • The new attribution path can assign an unrelated historical receipt to a request counterparty.
  • Existing wallet backups, local pending proofs, and reservation ledgers need compatibility handling for their changed persisted formats.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Shared Paykit requests] --> B[Request endpoint resolution]
  B --> C[Payment and proof]
  A --> D[Endpoint-to-contact index]
  D --> E[Historical received activity backfill]
  F[Local proof and reservation state] --> C
  G[Wallet backup] --> F
Loading

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

Comment thread Bitkit/Services/PaykitReceivedPaymentContacts.swift Outdated
Comment thread Bitkit/Models/PaykitPaymentStateBackup.swift
Comment thread Bitkit/Services/PaykitPaymentProofService.swift
Comment thread Bitkit/Services/PrivatePaykitAddressReservationStore.swift

@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 72 files.
Pair PR synonymdev/bitkit-android#1401: equivalent.

Findings:
3 inline (1 MEDIUM, 2 LOW)

QA:
Tests running: 7 of 9 passed.


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

Comment thread Bitkit/Services/CoreService.swift Outdated
Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated
Comment thread Bitkit/Services/PrivatePaykitService+Backup.swift

@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 merge base ab88d1c9, at 4c7f705.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

Compatibility with the unreleased receiver-path backup, pending-proof, and reservation formats is an intentional out-of-scope break. Paykit has not launched, and this PR does not claim those development documents remain readable. Authorization still shares only the requested watch-only account and generation-bound Paykit secret, and payment requests resolve through the request endpoint rather than a later private list.

The new marketplace wallet-leg consent step does not match the on-screen Paykit access copy or the Android companion's action text.

GitHub reports unit tests and integration tests succeeded on this revision. This review did not run them. The local e2e job was still running and is not evidence. bitkit-android#1401 was compared only for the updated consent journey step, not reviewed in full.

Recommended before device testing: correct the wallet-leg Paykit access action so that journey checks the localized consent copy.

Device testing: not performed in this review.

Findings

  • [LOW] Wallet-leg journey checks the wrong Paykit access copy — inline at journeys/pubky-marketplace/wallet-leg.xml:19.

Comment thread journeys/pubky-marketplace/wallet-leg.xml Outdated

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

One MEDIUM (two installs can pay one request twice) and two LOWs inline. Paykit is ungated on main, so these are user-facing from the next release.

Checked and clean:

  • Auth sheet: approval is pinned to the immutable config.request, with rawUrl re-checked in approveAuthRequest. The requester clientID and relayOrigin are displayed. A Paykit-only claim skips watch-only account allocation, while a combined claim still goes through the watch-only consent step. PubkyAuthClaim.encode refuses mismatched payload and claim combinations.
  • The exported Paykit secret is a one-way blake3 derivation of root and generation, signed and encrypted to the relay channel. Nothing logs the payload.
  • No new auto-start payment path. Amounts are still gated by validateIncomingPaymentRequestAmounts, endpoints are limited to acceptedPaymentEndpointIdentifiers, and the post-broadcast lookup reuses the captured contactPaymentContext.
  • No app-group or keychain-access-group changes, and Env.keychainGroup stays private.
  • Biometric and PIN checks run in submitPayment before performPayment takes the execution claim, so declining auth leaves no claim.
  • Not raised, because nothing reaches them today: claims are never released on abandon (no releasePaymentRequestExecutionClaim call site), and a missing registry counts as generation 1 against the saved floor (PubkyService.swift:1051). Both start to matter once a second executor app, or key rotation, exists. Same on synonymdev/bitkit-android#1401.

Non-blocking: is there a Figma frame for the new PubkyAuthPaykitAccess block in the approval sheet? Link it and I'll diff the implementation against it on the next pass.

Comment thread Bitkit/Services/PaykitPaymentRequestService.swift
Comment thread Bitkit/AppScene.swift Outdated
Comment thread Bitkit/Utilities/Keychain.swift

@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

Retest for the review: journey J8 fails; journey J4 passes now.

QA:
Tested on two iOS 26.5 simulators (iPhone 17 Pro), regtest.
Tests J1, J2, J3, J5, J6, J7, J9 already done in review.

🟢 Test J4
Test J4

Passed.

J4-retry-104009.mp4
J4-retry-104009-buyer.mp4

🔴 Test J8
Test J8

Written review Back control unavailable.

J8-retry-104009.mp4
J8-retry-104009-buyer.mp4
J8-retry-104009-resume.mp4
J8-retry-104009-buyer-resume.mp4
log
Timed out after 3000ms waiting for UI predicate exists for identifier NavigationBack.


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

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Updated in 71904a7. For the reported J8 failure, an automatically opened review is the root of the send sheet, so it has no Back button. The journey and README now use a downward swipe from the drag indicator. The consent step also matches Android. I have not rerun the full marketplace journey, so it remains unchecked.

For the design question, no Figma frame was supplied for this authorization UI. The PR keeps N/A — no design available.

@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 4c7f705, at 36ed45d. The inherited baseline is the full review of the PR diff against merge base ab88d1c. That base and merge base are unchanged, and 4c7f705 is an ancestor of this head. This pass covered the payment-ownership, received-payment attribution, reservation, keychain, and journey delta, plus the callers those paths use.

No new actionable code findings.

A one-time request stays payable on the install that stored its acceptance. Once that accepted state is visible, another install's refresh leaves the request out of pending and auto-presentation, and payment, retry, and send authorization require the local acceptance id. testOnlyAcceptingInstallCanResumeOneTimePayment covers the stale proposal and the restarted accepting install. The two-install payment comment matches this gate: the SDK execution claim still succeeds again for app id bitkit. Received-payment labeling requires the wallet receiving output and one contact across the transaction's mapped outputs, and it stops when the identity or reservation revision changes during lookup. The wallet-leg consent step now asks for private Paykit data and messages without sharing identity or spending keys, matching pubky_auth__paykit_access_description, and the automatic review is dismissed with a downward swipe. That resolves the previous consent finding.

This review did not run the simulator tests. Unit tests and integration tests were still running on this revision. Device testing was not performed. accepted-device-ownership and wallet-leg remain unchecked on the PR. The PR description's regtest payment report was not re-executed here.

ovi-reviewer[bot]

This comment was marked as resolved.

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

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

Follow-up at 36ed45d. One MEDIUM is still open: recurring periods are double-payable across installs. I replied on the existing two-install thread rather than opening a new one.

Resolved:

  • Two-install double pay for one-time requests. Every entry needs the local acceptance id: auto-present, notifications, list and detail, retry, finishPayment, SendConfirmationView, LnurlPayConfirm, quickpay and the hardware path. ensurePaymentAllowed requires isApprovedForPayment and re-checks generation, identity and approval after the async linkedPeers call. A stale proposal on the second install fails at the SDK accept, which re-validates Proposed inside the locked transaction.
  • Backfill is skipped while the identity, contact snapshot, activity revision and reservation revision are unchanged. Every ActivityService write invalidates it, and an incomplete scan is not cached.
  • Attribution requires the receiving address to be an actual output and a single contact across all mapped outputs, and conflicts stay unlabelled. The live path re-checks auth, identity and snapshot after the async lookup.
  • The ledger is keyed by normalized identity, removed by wipeEntireKeychain(), and kept out of backups.
  • accepted-device-ownership.xml matches the Android copy apart from identifiers.

Not raised:

  • An accepted one-time request that no install owns stays blocked until the payee cancels. That is the stated trade-off.
  • Activation failing closed on a ledger read error matches the existing subscription-store behaviour.
  • ovi-reviewer's open J8 thread is not repeated here.

@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 2 times, most recently from 23d6ddb to fe281dd Compare October 1, 2026 13:52

@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 13 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-android#1401: equivalent.

QA:
Tests wait for CI.


Reviewed by gpt-6.1-sol-high 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.

@ben-kaufman please resolve the merge conflicts. QA review has not been performed for this request.

@jvsena42

jvsena42 commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Device gate at 5dd730b (rc68), final. Two simulators on staging, same identities as the rc65 run, UTC 2026-10-07. No crash; 0 shared_state_busy, requestUnavailable, storage_error.

Quiet launches regressed against rc65. After the first slow quiet launch I added two more rounds:

Launch Sim First log line Deferred session restoration Paykit session restored Time
Quiet #2, 3 min 12 s on Home B 10:03:34.228 none 10:03:45.403 11.2 s
Quiet #2 A 10:03:36.076 10:04:09.594 10:04:36.533 60.5 s
Quiet #3, session never touched after restore B 10:07:50.988 none 10:08:01.640 10.7 s
Quiet #3 A 10:07:52.453 10:08:25.578 10:08:52.625 60.2 s

Across the run, 5 of 11 launches took about 60 s (the table above has the first seven), including 4 of 6 launches after three minutes idle. On rc65 and rc63 a quiet launch on these simulators was 11–12 s. The shape is always the same: one Deferred session restoration, keeping saved session about 33 s after launch, then 1–3 ConcurrentUpdate … Pubky resource is locked or changed from refreshBip21 / refreshPublicPaykitEndpointsOnForeground, restored at about 60 s. In quiet #3 the app did nothing between restore and the next terminate. The rc65 worst cases did not recur (no shared_state_busy, no six-minute launch).

WARN Stopped waiting for Pubky identity republishing appears on 9 of 11 launches, fast and slow alike.

Measure rc68 rc65
Send Request → "Sent" ≤ 9 s, 6–14 s ≤ 14 s
Swipe → broadcast ~15 s, ~27 s 34–42 s
Sharing OFF / ON 31–45 s / 14–20 s 25–43 s / ≤ 21 s
Delete a linked contact ≤ 17 s ≤ 17 s
Re-link after re-add ≤ 1 min 38 s, ≤ 2 min 40 s 2 min 55 s – 3 min 7 s
Launch → Request and Pay offered 1 min 19 s – 3 min 10 s 1 min 14 s – 2 min 51 s

Not run: the payment-deadline journey. The Create Payment Request screen only offers "Expires in" (1 hour at the shortest), with no payment-deadline field, so it needs a controlled issuer. The hardware-deadline MEDIUM in my review is therefore from code only.

The review stays at changes requested for that MEDIUM. The launch regression goes to #868.

Log lines

App log, UTC, Pubky and Paykit lines (the iOS log has no SDK-level lines). Keys and txids shortened.

Slow quiet launch, 60.2 s (simulator A, quiet #3, A_L5_quiet3_10-07-52.log, first log line 10:07:52.453):

10:07:58.360 WARN: Stopped waiting for Pubky identity republishing - PaykitSdkService
10:07:58.593 DEBUG: Republished Pubky identity - PaykitSdkService
            (27.0 s with no Pubky or Paykit line)
10:08:25.578 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
10:08:35.571 WARN: Failed to refresh public paykit endpoints after receive refresh: ConcurrentUpdate(code: "concurrent_update", context: "Pubky resource is locked or changed; retry from current state") - WalletViewModel
10:08:42.519 WARN: Failed to refresh public Paykit endpoints on foreground: ConcurrentUpdate(code: "concurrent_update", context: "Pubky resource is locked or changed; retry from current state") - WalletViewModel
10:08:52.625 INFO: Paykit session restored for pubkyyks9epx… - PubkyProfileManager

Fast launch, 10.9 s (simulator A, A_L3_3c_after_toggle_09-53-10.log, first log line 09:53:10.205):

09:53:15.958 WARN: Stopped waiting for Pubky identity republishing - PaykitSdkService
09:53:16.729 DEBUG: Republished Pubky identity - PaykitSdkService
09:53:21.098 INFO: Paykit session restored for pubkyyks9epx… - PubkyProfileManager

Both republish the identity about 6 s after launch. The fast one restores 4.4 s later; the slow one is silent for 27 s, defers, and restores 27 s after that.

Payment #1 on the payer, swipe ended 09:37:22 (B_L0_upgrade_09-33-51.log):

09:37:07.948 DEBUG: Showing sheet send - SheetViewModel
09:37:11.578 INFO: Opened private Paykit payment for pubkyyks9epx... using payment list version 33 - PrivatePaykit
09:37:26.097 DEBUG: Backup starting for: 'WALLET' - BackupService
09:37:26.278 INFO: Consumed private Paykit payment list version 33 for pubkyyks9epx... - PrivatePaykit
09:37:26.280 INFO: Updated paykit_accepted_payment_requests - Keychain
09:37:33.770 WARN: No UTXO selected, using default selection algorithm.
09:37:36.985 INFO: Sending 1000 sats to bcrt1qmctcg5… with fee rate 1 sats/vbyte (isMaxAmount: false)
09:37:37.282 Onchain send result txid aad28eb6…

Of the ~15 s from swipe to broadcast, about 4 s go to consuming the payment list and about 10.7 s pass between Updated paykit_accepted_payment_requests and Sending 1000 sats, with only the UTXO selection warning in between.

Raw SDK error in a toast (simulator A, Pay tap 09:53:45, A_L3_3c_after_toggle_09-53-10.log):

09:53:57.538 WARN: Failed to resolve Paykit contact payment for pubkydso8zn5...: reason=concurrent_update/concurrent_update - PrivatePaykit
09:54:20.415 ERROR: Failed to pay contact pubkydso8zn5...: ConcurrentUpdate(code: "concurrent_update", context: "peer link operation already in progress for counterparty dso8zn57…") - PaymentNavigationHelper

Re-add lost right after delete (simulator B, B_L2_3a_quiet_09-48-25.log; delete confirmed 09:56:03, first Save 09:56:32, second Save 09:57:05):

09:56:21.909 DEBUG: Loaded 0 SDK contact records - ContactsManager
09:56:24.251 INFO: Received deeplink: bitkit://contact
09:56:30.071 INFO: Removed contact pubkyyks9epx... - ContactsManager
09:56:32.207 WARN: Failed to prune private Paykit endpoints for unsaved contacts: CancellationError() - PrivatePaykit
09:56:58.485 INFO: Received deeplink: bitkit://contact
09:57:06.098 INFO: Added contact pubkyyks9epx... - ContactsManager

The first Save has no Added contact line; the add screen was opened by the deeplink 6 s before Removed contact was logged.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Historical test results — earlier revision (October 7, 2026)

Tested iOS 5dd730b24930be3c7636b6e3b29f00f84f2a6f0e (Paykit rc68), using Stag6 with 62 prepared contacts plus the controlled cross-platform counterparty. The PR has advanced since this run; these results describe the tested revision and have not been verified on the latest head.

Both directions of the paired 1,000-sat request payment completed, with independent regtest confirmation. Recipient-sharing-OFF rejection and contact delete/re-add also worked; private Request/Pay availability recovered after re-add.

Intermittent slow session restoration was observed, including 60.79s and 73.80s. Background preparation remained active, so this run does not establish performance after preparation fully settles. The detailed report includes differing sharing/relaunch conditions and the Android enabled-state observation correction.

Full timings, limitations and attached logs/evidence. Repeated measurements are labelled Run 1–3, using the same profile. This is historical device evidence, not verification of the current head or a merge approval.

@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 2 files.
No new findings; the rest is in the review.

QA:
Tests running.
Tests 1, J1, J9, J17 passed at 32fcc1d.
Tests 2-4, 6, J7 passed at 976538f.
Tests 6, J2, J4, J6, J11, J12, J13, J18, J19, J21 failed at 5dd730b.
Tests J5, J14 passed at a684677.
Tests J8, J10 passed at cb23093.
Test J15 passed at 5dd730b.

Replies:

@piotr-iohk: The same-identity re-import stalled and prevented settled-deletion measurement. (comment)

The later rc68 re-imports completed with preparation retries active. Strictly settled deletion remains unverified.

@jvsena42: The rc63-to-rc65 upgrade restored both identities; latency and remaining device checks stayed open. (comment)

I have retained that upgrade result separately from the earlier rc62-to-rc63 failure. It does not close the remaining device or latency checks.

@ben-kaufman: Linked-contact recovery needs logs from both peers, the SDK error and the last successful link check. (comment)

This documentation change supplies no new peer logs or recovery evidence. Linked-contact recovery and fixture-blocked journeys remain unverified.

@piotr-iohk: Three rc68 imports and two deletions completed, but peer retries persisted and strictly settled deletion remains unverified. (comment)

I have retained the successful re-import results and the background-overlap caveat. They do not certify strictly settled deletion or private-link readiness.

@ben-kaufman: Hardware reconciliation now preserves wallet scope and view-recreation state; real-device lost-response testing remains open. (comment)

The new manual checklist covers lost responses, expiry, completion, view recreation and mismatched resolutions. Those fault-injection checks still require device execution.

@jvsena42: Quiet launches regressed to about 60 seconds; deadline testing needs a controlled issuer and was not run. (comment)

I have retained the launch regression under #868 and the controlled-issuer requirement. This documentation delta changes neither restoration nor the expired-retry behavior when no transaction reconciles; it provides no device evidence to close those concerns.

Note

Retest Suggested J19, 7

@ovi-reviewer retest J19,7

Reviewed by gpt-6.1-sol-medium 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 05ec22d3, covering the entire delta and affected recovery, persistence and UI paths since 5dd730b2. Unchanged coverage is inherited from the completed baseline; Git ancestry, base and merge base were verified.

No new actionable code findings.

The issuer deadline documentation is corrected. The existing expired hardware retry with no reconciled transaction concern remains: a broadcast that never reached the network supplies no resolution to unlock the sign screen. The new matching-transaction recovery path does not settle that case. The view-recreation regression-test request also remains; the author now explicitly requires manual fault injection. Earlier startup/performance reports are not cleared by this delta.

Validation: Swift parsing and a standalone harness using the pinned coordinator with dependency stubs passed uncertain-broadcast, expired-retry, wallet-mismatch and no-rebroadcast checks. The harness omits the observation macro and does not establish SwiftUI event delivery; native XCTest suites were inspected, not executed locally. Matching unit CI and integration CI were still running. Targeted source comparisons used Android 804e7547 and BitkitCore b53fa54a; neither was reviewed in full. The older Android coordinator does not establish parity for the new iOS recovery observer. Concurrent wallet/shared-Pubky installations and pre-2.6.0 profile compatibility remain excluded by the supplied support rules.

Device testing: not performed in this review. The author's deadline and hardware lost-response/view-reattachment checks remain unexecuted here and require a controlled issuer and fault-injection setup.

@ovi-reviewer

This comment has been minimized.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Fixed the raw Paykit error and a contact re-add navigation race in d99d5a6. Paykit failures now use the existing localized retry message, and cancellation stays silent. A delayed deletion callback only returns to Contacts if the user is still viewing or editing that same contact, so it cannot dismiss a newer Add Contact screen.

439 focused native tests passed, including these paths and the hardware-expiry fix. The full quick-delete/re-add sequence still needs a staging recheck on this head. Startup and linking performance remain open; these fixes do not establish that those delays are resolved.

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

Reviewed 05ec22d..d99d5a6. The hardware-deadline MEDIUM is fixed and resolved. One new MEDIUM inline, introduced by the contact navigation fix in this commit.

Checked with no finding: contactPaymentErrorDescription (PaymentNavigationHelper.swift:295-299) maps every PaykitError, bare or wrapped in AppError, to the localized retry message and CancellationError to nil, at all three toast sites; the specific messages for no endpoint, link pending and waiting for the updated list are unchanged. A delete of contact X can no longer pop an Add screen or another contact's screen.

Device gate: a retest of the quick delete → re-add sequence and the error toast is running on two simulators at this head; results will follow as a comment with log lines.

Comment thread Bitkit/ViewModels/NavigationViewModel.swift Outdated

@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 13 files.
No new findings; the rest is in the review.
The hardware deadline fix matches the pair PR synonymdev/bitkit-android#1401, which restores the prior navigation-lock state after queued expiry. Android already shows localized contact-payment failure guidance; the contact deletion route guard fixes SwiftUI callbacks specific to iOS.

QA:
Tests running.
Tests 1, J1, J9, J17 passed at 32fcc1d.
Tests 2-3, 5, J7 passed at 976538f.
Tests 6, J4, J11, J12, J13, J18, J19, J20 failed at 5dd730b.
Tests J2, J6 failed at 05ec22d.
Tests J5, J14 passed at a684677.
Tests J8, J10 passed at cb23093.
Test J15 passed at 5dd730b.

Replies:

@ben-kaufman: Keeping the linked-contact recovery result open. (comment)

The requested peer logs are still needed. Linked-contact recovery and fixture-blocked checks remain unverified.

@ben-kaufman: Real-device lost-response/expiry fault-injection remains an open QA check. (comment)

Lost-response, expiry, and view-recreation checks remain pending.

@ben-kaufman: I am keeping the startup and deadline checks open. (comment)

The reported timings remain open under #868. Hardware lost-response and deadline checks still need fault injection.

@jvsena42: Raw SDK error in a toast; re-add right after delete did nothing. (comment)

The SDK-error and re-add navigation fixes have regression coverage; the complete staging sequence remains pending. Quiet-launch delays remain tracked in #868.

@jvsena42: Quiet launches regressed against rc65; deadline testing needs a controlled issuer. (comment)

I verified the expired-retry correction in code and retained the controlled-issuer check for QA. The quiet-launch regression remains tracked in #868.

@ben-kaufman: The full quick-delete/re-add sequence still needs a staging recheck on this head. (comment)

The code and regression tests address both reported contact failures. The full quick-delete/re-add sequence and SDK-contention behavior remain queued for staging verification; startup and linking performance stay open.

Note

Retest Suggested J10, J11, J19, 7

@ovi-reviewer retest J10,J11,J19,7

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

@jvsena42

jvsena42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Device gate (partial) at d99d5a6 (rc68), two simulators on staging, UTC 2026-10-07.

Launch after install: restored in 12.0 s (A) and 11.6 s (B), no Deferred session restoration, no concurrent_update.

Quick delete → re-add: fixed. Four attempts, each with the Add Contact deeplink sent while the deletion was still finishing and Save tapped 5–10 s after Removed contact:

# Who deletes Add screen up Removed contact Add screen still up after it Save Added contact
1 B deletes A ~11:56:10 11:56:23.064 yes ~11:56:33 11:56:37.765
2 A deletes B ~11:58:27 11:58:38.364 yes ~11:58:43 11:58:49.439
3 B deletes A 12:00:29 (after the delete) 12:00:00.641 n/a ~12:00:34 12:00:36.715
4 B deletes A 12:01:18–20 12:01:31.417 yes ~12:01:36 12:01:41.813

All four ended on Contact Saved. No CancellationError and no Failed to prune line in either log; on 5dd730b the same sequence lost the Save and logged Failed to prune private Paykit endpoints for unsaved contacts: CancellationError().

Delete from the Contact Saved screen (attempts 3 and 4) left the "Unable to load contact." screen; details and log lines are in the MEDIUM thread.

Pay taps right after a re-add (three taps, 5–10 s after Contact Saved): each spun about 31 s, logged Failed to resolve Paykit contact payment … reason=recovery_required/recovery_required, and opened the plain amount sheet about 8 s later. No toast, so no raw SDK text was seen:

11:56:42    (B: Pay tap on the re-added contact)
11:57:13.132 WARN: Failed to resolve Paykit contact payment for pubkyyks9epx...: reason=recovery_required/recovery_required - PrivatePaykit
11:57:20.681 DEBUG: Showing sheet send - SheetViewModel

Still running: a Pay tap right after a relaunch (the case that showed the raw error before) and a sanity payment.

@jvsena42

jvsena42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Device retest at d99d5a6 (rc68), rest of the run. Two simulators on staging, UTC 2026-10-07. No crash; 0 shared_state_busy, requestUnavailable, CancellationError.

Error toast: not exercised. No Pay tap in this run ended in a toast, so the localized retry message was not seen on a device. The case that showed the raw text before needs a sharing OFF/ON cycle before a relaunch, which I did not repeat. No raw SDK text appeared anywhere. The fix is confirmed in code only.

Relaunch on B was slow again (65 s), same shape as before (B_L1_relaunch_12-05-46.log):

12:05:46     (launch)
12:06:20.017 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
12:06:30.020 WARN: Failed to refresh public paykit endpoints after receive refresh: ConcurrentUpdate(code: "concurrent_update", context: "Pubky resource is locked or changed; retry from current state")
12:06:33.461 ERROR: Backup failed for: 'WALLET': ConcurrentUpdate(code: "concurrent_update", …)
12:06:51.458 INFO: Paykit session restored for pubkydso8…

A contact deeplink sent during that minute was only handled at 12:06:37.169. For comparison, the launch after install on both simulators restored in 12.0 s and 11.6 s with none of these lines.

Sanity payments:

Step A → B request Reference (5dd730b)
Send Request → "Sent" ≤ ~31 s (upper bound, first look) ≤ 9–14 s
Send tap → sheet opens on B ~35 s 19–25 s
Preparing → ready 12.9 s 3.6 s
Swipe → Consumed private Paykit payment list ~13 s 4–6 s
Swipe → broadcast ~30 s 15–27 s
12:08:49.759 DEBUG: Showing sheet send
12:08:50.058 DEBUG: Backup starting for: 'WALLET'
12:09:02.671 INFO: Opened private Paykit payment for pubkyyks9epx... using payment list version 56
12:09:07.993 INFO: Backup succeeded for: 'WALLET'
            (swipe ~12:09:07)
12:09:16.643 DEBUG: Backup starting for: 'WALLET'
12:09:20.180 INFO: Consumed private Paykit payment list version 56
12:09:37.080 INFO: Sending 1000 sats to bcrt1q9z…
12:09:37.305 INFO: Successfully broadcast transaction dba8cd9e…

No WARN or ERROR between the sheet and the broadcast. This payment ran about two minutes after B's slow relaunch and four minutes after a re-add, and every step was slower than in the earlier run on a settled pair; I have one sample, so I am not calling it a regression. A second, older request paid from A took 32.0 s from sheet to ready (Showing sheet send 12:03:09.221 → Opened private Paykit payment … version 52 12:03:41.207, nothing logged in between) and ~18 s from swipe to broadcast. After both payments the bell cleared and nothing reopened.

Pay right after a re-add costs about 39 s each time before the plain amount sheet: 31 s to Failed to resolve Paykit contact payment … reason=recovery_required/recovery_required, then 8 s to Showing sheet send. Three of three.

Review state: changes requested stands for the Contact Saved delete MEDIUM.

@ovi-reviewer

ovi-reviewer Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Interim test results for d99d5a6

The run is still going; the final review follows when it ends. This is a progress note, nothing is approved or requested here.

14 passed so far, 4 failed.

🟠 Test 4: failed at d99d5a6; waits for round 2

Post-commit acceptance response loss is unavailable.

screenshot

🟠 Test J2: failed at d99d5a6; waits for round 2

Staging contact fixture creation failed.

screenshot

🟠 Test J4: failed at d99d5a6; waits for round 2

Required fixed-address monthly proposal cannot be prepared.

🟠 Test J21: failed at d99d5a6; waits for round 2

Deadline-history fixture was not prepared.

screenshot

🟢 Test 1: passed at 32fcc1d; its recheck on d99d5a6 is still to come

On the exact-head binary, two Home → Bitkit foreground cycles (19:17:40–19:17:57 UTC) overlapped devctl offline/online (19:17:40–19:17:56) while native Pubky relay connections failed for 12 seconds, including the auth-relay interval 19:17:47–19:17:59; the device recorded one import/re-sign-in failure for that shared active interval and preserved its session for a later attempt. After healing the…

🟢 Test 3: passed at 976538f; its recheck on d99d5a6 is still to come

The one-shot QA adapter fault threw SharedStateBusy after the native SDK returned an accepted record, before its caller received success; it did not intercept a native SDK read. After restoring the clean source and baseline app, restarting and refreshing retained the request and Pay action, and retry reached Bitcoin Sent for 21,000 sats. The fault build hash, nonce log, accepted record state,…

🟢 Test 5: passed at 976538f; its recheck on d99d5a6 is still to come

Accepted the 21,000-sat request while its real LNURL callback failed before invoice creation, then waited for All Synced and stopped the original wallet. Restored its recovery phrase on the replacement install, which recovered the accepted request, profile and Lightning channel, and retried to Bitcoin Sent after restoring the callback. The receiver recorded one settled 21,000-sat invoice and the…

🟢 Test J1: passed at 32fcc1d; its recheck on d99d5a6 is still to come

Imported 62 disposable fixture friends into a profile originally created through Bitkit, then drove Edit Profile → Delete → Yes, Delete after the preparation sweep settled and again immediately after import with preparation active. Both recordings show the dimmed spinner and disabled profile actions, followed by profile onboarding in about 5 seconds settled and 3 seconds active; Home → Profile…

🟢 Test J3: passed at 32fcc1d; its recheck on d99d5a6 is still to come

The linked rc64 fixture proposed a monthly 5000-sat request whose serialized terms prove payment_deadline null and the explicit fixed P2WPKH address; Bitkit accepted it and opened the funded on-chain confirmation without payment. After the issuer canceled it, the foreground timeline showed confirmation closing at t=2.1 s and the request moving to the Expired section; its details offered no Pay…

🟢 Test J5: passed at a684677; its recheck on d99d5a6 is still to come

All nine actions driven in order. General toggle started at 1, one tap disabled sharing, then the completed update showed ContactPaymentsToggle value 0. A 30-second timeline retained value 0 after completion. Returned to wallet Home, reopened Settings and General, and value remained 0. The other wallet remained a distinct identity, not a second install of this wallet. Linked proof is the actual…

🟢 Test J7: passed at 976538f; its recheck on d99d5a6 is still to come

Opened the same fresh request on both independent wallets; A accepted it and showed SendFailure while the issuer confirmed Accepted without proof or receiver dispatch. B safely dismissed the stale review, then after restart retained the request in history without Pay; after A restarted, its retained Pay completed with SendSuccess, one new settled21,000-sat receiver invoice and one shared proof.…

🟢 Test J8: passed at cb23093

The fresh Paykit-only request displayed exactly /pub/paykit with READ, WRITE access and private Paykit data/message consent without spending keys; watch-only consent was absent. Cancel dismissed authorization, the Contacts screen stayed stable for30.9seconds, and before/after watch-only account names, paths and tracking lists were identical and empty.

🟢 Test J9: passed at 32fcc1d; its recheck on d99d5a6 is still to come

Created one active, tracked service account through the exact Server46 grant and recorded paykit account, m/84'/1'/1', tracking enabled, and its xpub from the wallet’s own persisted account metadata. Opened a fresh server-generated /setup/reconnect URL scoped exactly to /pub/paykit/:rw and paykit-access-v1; PubkyAuthPaykitAccess explained private data/messages and no identity or spending keys,…

🟢 Test J10: passed at cb23093; its recheck on d99d5a6 is still to come

Both staging wallets saved each other and the funded payer opened ContactViewName, then RequestOrPaySheet. The recording shows Pay loading with Request disabled and SendAmount opening within 2.25 seconds; closing it restored ContactPay, and tapping Request opened PaymentRequestAmount. Identifiers were asserted through XcodeBuildMCP; the XML journey ran end to end without submitting a payment.

🟢 Test J14: passed at a684677

Sent completion matched its description. The 21000-sat request opened automatically with Seat2Requester and Lunch last week; Show Details hid From/For identifiers and showed the contact and invoice note. The separate 5000-sat request with no note opened automatically and showed Not specified.

🟢 Test J15: passed at 5dd730b

Terminated pinned staging app, opened exact public dummy watch-only-account-v1 URI from XML, and native PubkyAuthWatchOnlyConsent exists predicate succeeded. No system Open prompt was presented. Cancel was tapped; native gone predicate succeeded. Opened exact unsupported-v1 URI; native PubkyAuthInvalidRequestToast exists captured Invalid auth request. Final consent gone predicate succeeded. All…

🟢 Test J16: passed at 9cb87b8; its recheck on d99d5a6 is still to come

Seat1Creator proposed Journey Sub, 5,000 sats monthly, to linked contact Seat1Payer. Choose Recipient stayed disabled until the name and amount were set. A 600-letter description was rejected on Propose Subscription with "Shorten the subscription name or description and try again" and no sent confirmation. Replacing it with "Monthly support" produced the sent sheet (Sent / Proposal) for…

🟢 Test J17: passed at 32fcc1d; its recheck on d99d5a6 is still to come

Created a monthly 5,000-sat proposal to the linked Seat1Payer contact. The 600-letter description produced the shorten-content error, which disappeared during the saved 30-second timeline without a sent confirmation; replacing it with Monthly support produced Sent Proposal for Journey Sub and a CREATED row labelled Proposal sent after OK.

The other 10 tests run next.

@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 d99d5a67, covering the entire delta since 05ec22d3, affected callers and tests, and the complete current PR inventory. Unchanged coverage is inherited from the completed baseline; ancestry, base and merge base were verified locally, and HEAD was rechecked before returning.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The expired hardware-retry navigation concern is fixed in source. Started proofs retain reconciliation and duplicate-payment protection after dismissal. The contact-deletion regression was independently confirmed and reconciled with its existing canonical thread. Startup and linking performance remain unresolved.

Validation: changed Swift files passed syntax parsing. A standalone Swift harness using the pinned navigation helper confirmed that Contact Saved remains on the stack after deletion; it does not exercise SwiftUI. Native XCTest assertions were inspected, not executed here. The author's 439-test report is attributed evidence; matching unit and integration CI remained in progress when collected. Structured code-result and diff-anchor validation passed. Targeted comparison used Android 804e7547, whose older expiry path still locks dismissal; this does not establish current Android parity or a full Android review.

Device testing: not performed in this review. Hardware lost-response, no-reconciliation and retained-resolution view-recreation checks still require the author's controlled issuer and fault-injection setup. Concurrent wallet/shared-Pubky installations and pre-2.6.0 profile compatibility remain excluded by the supplied product rules.

Findings

  • [MEDIUM] Return to Contacts after deleting from Contact Saved — inline at Bitkit/ViewModels/NavigationViewModel.swift:209.


func returnToContactsAfterRemoving(publicKey: String) {
switch currentRoute {
case let .contactDetail(shownKey), let .editContact(shownKey):

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.

[MEDIUM] Return to Contacts after deleting from Contact Saved

After adding a contact, tap Delete on the Contact Saved screen and confirm. AddContactView sets .contactSaved(publicKey:), and MainNavView mounts ContactDetailView with showsDeleteAction: true for that route. Both deletion callbacks now call this helper, but its switch excludes .contactSaved, leaving the path unchanged while the contacts publisher clears profile. This path would leave the user on the deleted contact's empty screen instead of returning to Contacts.

Include the matching .contactSaved(shownKey) route and move it out of the negative test cases, while preserving newer Add and unrelated-contact routes. testDeletedContactDoesNotDismissANewerRoute currently asserts the regression.

Evidence basis: source analysis at d99d5a67 and an executed standalone Swift harness using this exact helper; no SwiftUI or device execution was performed here. This is the same mechanism as the canonical thread, which also contains an attributed simulator reproduction at this head.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 291328d, as detailed in the canonical thread: #856 (comment). Matching Contact Saved/detail/edit entries are removed, while newer Add and unrelated-contact routes remain. Navigation tests pass; fixed-head device verification remains open.

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.

Re-reviewed d99d5a6..291328d. No findings. The Contact Saved delete MEDIUM is fixed and resolved, verified on the simulator: three deletes from the Contact Saved screen all returned to Contacts with no error screen, Back did not restore the deleted contact, and each re-add reached Contact Saved. The Edit-screen delete still returns by itself.

Also in this commit: HwFundingSigner.swift:622 now drops the pending payment only on a definite pre-broadcast failure (InvalidHex, InvalidTransaction) with no prior attempt, matching Android 50811b2a5; Electrum and unclassified errors keep the signed transaction for retry.

Device gate: two simulators on staging at 291328d, contacts only (timeline and log lines in the resolved thread). Payments, launches and sharing were last driven at d99d5a6 and 5dd730b. The hardware deadline, reconciliation and failure paths are verified in code and unit tests only; the manual fault-injection checklist is unexecuted. The localized error toast from d99d5a6 was not seen on a device.

Open and tracked in #868: about one minute to restore the session in roughly half of launches, and the delete and Contacts-reload waits noted in the thread.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Updated to published Paykit rc69 in b633684, on top of the hardware and contact-navigation fixes in 291328d. SwiftPM resolved the release artifact and 610 focused native tests passed.

The SDK now returns temporary lock contention after one bounded acquisition batch instead of repeating it as a revision conflict. The staging SDK test reduced a blocked call from about 26s to 3s, but still recovered only when the 60s lock expired. This is not a new mobile startup or linking speed claim; those timings remain open.

The reviewer’s three Contact Saved deletion runs are now recorded as passed. Hardware fault-injection and the other unchecked device scenarios remain open.

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

The matching hardware lost-response case now passed on b633684 / published rc69, using the normal iOS app, a Trezor emulator and disposable local regtest funds.

Core independently accepted the signed payment while the proxy dropped the response and held reconciliation reads. Core made two initial attempts with identical bytes. Retry after the actual deadline added no broadcast attempt. Releasing the reads automatically reached Bitcoin Sent, returned Home, and delivered exactly one matching proof to the issuer. The attempt count remained two.

This verifies that recovery path, not the whole checklist. View reattachment, process death and other injected failure combinations remain unverified. It is not a latency result or physical-hardware test, and no public-network payment was made.

@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 12 files.
No new findings; the rest is in the review.
The hardware pre-submission classifier matches synonymdev/bitkit-android#1401 at 7bc3ccb; J22 has identical filename, name and action prose on both platforms. Published rc69 changes shared lock backoff for both mobile consumers without a new mobile latency claim.

QA:
Tests queued.

Replies:

Error toast: not exercised. (comment)

The localized toast remains a device QA gap. Contact Saved deletion is fixed in 291328d and separately confirmed in the canonical thread. The rc68 relaunch and payment waits remain open in #868; rc69 changes contention backoff without establishing current mobile latency.


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

@ben-kaufman

Copy link
Copy Markdown
Contributor Author

The latest CI unit failure was the test’s two-second wait for contact loading to start; its sign-out/publication assertions did not fail. 480b8d2 gives that setup wait ten seconds, with the existing load/sign-out gates and assertions unchanged. All 22 tests in the class passed ten repetitions after the edit. This changes only the test, not app delays or safety timeouts. CI is rerunning.

@jvsena42

jvsena42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Device gate (partial) — b633684 (Paykit rc69)

Two simulators on staging, build installed over the rc68 state from 291328d. UTC 2026-10-07. Still running: payment request round trip, contact delete and re-add.

Upgrade in place: the session restores on both from rc68-written state. No failed validation, recovery_required or concurrent_update line in any of the 14 logs; every lock error is now shared_state_busy.

Relaunch (terminate, launch), first log line → Paykit session restored:

# first log line Deferred session restoration lines restored total
A1 13:39:02.907 13:39:10.514, 13:39:28.729, 13:39:51.934 13:40:39.480 96.6 s
A2 13:41:08.074 13:41:15.564, 13:41:34.205 13:47:11.099 363.0 s
A3 13:47:15.634 none 13:47:26.323 10.7 s
A4 13:48:38.706 13:48:46.187, 13:49:04.525, 13:49:29.790 13:50:26.357 107.7 s
A5 13:51:08.700 none 13:51:19.372 10.7 s
A6 13:52:28.070 13:52:35.570, 13:52:54.092, 13:53:24.402 13:54:09.367 101.3 s
B1 13:39:17.454 13:39:25.557, 13:39:44.275, 13:40:12.401 13:40:57.372 99.9 s
B2 13:41:43.678 none 13:41:54.607 10.9 s
B3 13:47:22.847 13:47:30.854, 13:47:48.398, 13:48:18.512 13:49:00.785 97.9 s
B4 13:49:54.116 none 13:50:05.811 11.7 s
B5 13:51:23.220 13:51:31.161, 13:51:49.840, 13:52:18.887 13:53:17.530 114.3 s
B6 13:53:58.291 13:54:06.275, 13:54:28.357, 13:54:53.478 13:55:42.403 104.1 s

8 of 12 launches were deferred. Not deferred: 10.7–11.7 s. Deferred: 96.6–114.3 s, and one at 363 s. At d99d5a6 and 291328d (rc68) on the same simulators the deferred launches took 60–68 s with a single deferral, so in this run the slow case is about 35–45 s longer and takes three deferrals instead of one. The first launch after the install showed the same split (A 58.0 s with two deferrals, B 11.7 s).

Slow launch, B5 (B_bitkit_foreground_2026-10-07_13-51-23.log):

13:51:23.220 PERF: init(walletIndex:) took 0.0 seconds on core queue
13:51:28.900 WARN: Stopped waiting for Pubky identity republishing - PaykitSdkService [waitForRepublish line: 486]
13:51:31.161 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager [resolveSessionInitialization line: 1786]
13:51:43.331 WARN: Failed to refresh public paykit endpoints after receive refresh: SharedStateBusy(code: "shared_state_busy", context: "Pubky shared state remains locked; retry later") - WalletViewModel [refreshBip21 line: 1496]
13:51:49.840 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
13:51:59.428 WARN: Failed to refresh public Paykit endpoints on foreground: SharedStateBusy(code: "shared_state_busy", ...) - WalletViewModel [line: 1358]
13:52:02.691 ERROR: Backup failed for: 'WALLET': SharedStateBusy(code: "shared_state_busy", ...) - BackupService [triggerBackup line: 262]
13:52:18.887 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
13:52:18.891 DEBUG: paykit_session loaded from keychain
   -- 48.9 s without a paykit line, no WARN/ERROR --
13:53:07.824 DEBUG: paykit_session loaded from keychain
13:53:08.293 INFO: Updated paykit_session - Keychain
13:53:17.530 INFO: Paykit session restored for pubkydso8zn5... - PubkyProfileManager [line: 322]
13:53:19.122 INFO: Loaded 1 contacts - ContactsManager

Fast launch, B2 (B_bitkit_foreground_2026-10-07_13-41-43.log):

13:41:43.678 PERF: init(walletIndex:) took 0.0 seconds on core queue
13:41:49.184 WARN: Stopped waiting for Pubky identity republishing - PaykitSdkService
13:41:54.607 INFO: Paykit session restored for pubkydso8zn5... - PubkyProfileManager [line: 322]
13:41:56.069 INFO: Loaded 1 contacts - ContactsManager

The 363 s launch, A2 (A_bitkit_foreground_2026-10-07_13-41-08.log): after two deferrals a third attempt started at 13:41:58 and logged neither Deferred nor restored for 5 min 9 s. The app stayed responsive, but Contacts opened as a screen titled "Profile" with only a spinner (13:45:13, unchanged at 13:46:42). It restored by itself.

13:41:15.564 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager [line: 1786]
13:41:28.101 WARN: Failed to refresh public paykit endpoints after receive refresh: SharedStateBusy(code: "shared_state_busy", ...) - WalletViewModel [refreshBip21 line: 1496]
13:41:34.205 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
13:41:43.323 WARN: Failed to refresh public Paykit endpoints on foreground: SharedStateBusy(...) - WalletViewModel [line: 1358]
13:41:46.588 ERROR: Backup failed for: 'WALLET': SharedStateBusy(...) - BackupService [line: 262]
13:41:58.655 DEBUG: Upserting paykit_session - Keychain
13:42:01.254 DEBUG: paykit_session loaded from keychain
   -- 304.8 s with no paykit/pubky line (the ~1/s `paykit_session loaded` polling stops too), no WARN/ERROR --
13:47:06.042 DEBUG: paykit_session loaded from keychain
13:47:11.099 INFO: Paykit session restored for pubkyyks9epx... - PubkyProfileManager [line: 322]

Notes on the method: each kill was issued 38–127 s after the previous restore (A3 1 s after, B3 325 s after), and in every deferred launch the last keychain poll line was 0.1–1.2 s before the kill. In every deferred launch the WALLET backup fails once with shared_state_busy at about +38 s. The Deferred session restoration line carries no error code on iOS; the code is only on the neighbouring lines.

@jvsena42

jvsena42 commented Oct 7, 2026

Copy link
Copy Markdown
Member

Device gate (partial 2) — b633684 (Paykit rc69)

Payment request round trip, both directions, UTC 2026-10-07. No shared_state_busy, concurrent_update, failed validation or recovery_required line in either log during these flows. Still running: contact delete and re-add.

step B requests from A A requests from B
Send Request tap → "Sent" 12.7–17.1 s 18.3–23.1 s
"Sent" → sheet opens by itself on the payer (idle on Home) 6.9–9.0 s 14.9–17.5 s
sheet open → Opened private Paykit payment 3.7 s 3.8 s
swipe → Onchain send result txid 18.6 s 19.2 s

Both payments completed and both sides show the activity. Payer log for the second one (B_bitkit_foreground_2026-10-07_13-53-58.log), no WARN/ERROR apart from No UTXO selected:

14:02:08.416 DEBUG: Showing sheet send - SheetViewModel
14:02:12.216 INFO: Opened private Paykit payment for pubkyyks9epx... using payment list version 71 - PrivatePaykit
14:02:12.250 INFO: Updated paykit_presented_payment_requests - Keychain
   (swipe 14:02:31.5–14:02:32.7)
14:02:34.652 INFO: Saved paykit_pending_payment_proofs - Keychain
   -- 8.2 s --
14:02:42.893 INFO: Consumed private Paykit payment list version 71 for pubkyyks9epx... - PrivatePaykit
14:02:42.896 INFO: Updated paykit_accepted_payment_requests - Keychain
   -- 4.9 s --
14:02:47.768 INFO: Updated paykit_presented_payment_requests - Keychain
14:02:47.769 WARN: No UTXO selected, using default selection algorithm.
14:02:51.894 INFO: Sending 1000 sats to bcrt1qkpcl… with fee rate 1 sats/vbyte
14:02:51.986 INFO: Onchain send result txid: 6ab228a8…

Seen in passing on the payer after the first payment: Synced LDK payments - Added: 2 - Updated: 7 logged 17 times within 14:00:18.586–.605.

@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 291328d..480b8d2: the rc69 bump (b633684) and a test timeout (480b8d2). One MEDIUM, inline: rc69 makes the slow launch slower.

rc69 otherwise: the session restores from rc68-written state on both simulators. No failed validation, recovery_required or concurrent_update line in any of the 14 logs; every lock error is shared_state_busy, which the app already handles wherever it handles ConcurrentUpdate (PubkyProfileManager.swift:1820, PaykitPaymentRequestService.swift:1787).

Payments: a request and payment in each direction completed, swipe → broadcast 18.6 s and 19.2 s, no lock errors. Contacts: delete from the Edit screen returned to Contacts and the re-add reached Contact Saved in under 5 s. Numbers and log lines: #856 (comment).

480b8d2 only raises a test wait from 2 s to 10 s (ContactPaymentsServiceTests.swift:514).

Device gate: b633684 — relaunch 10.7–11.7 s when not deferred, 97–114 s when deferred (8 of 12, one at 363 s); payments 18.6 s and 19.2 s; contact delete and re-add pass; no crash. 480b8d2 is test-only and was not driven. Hardware paths are code-only as before.

requirement = {
kind = exactVersion;
version = "0.1.0-rc56";
version = "0.1.0-rc69";

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.

[MEDIUM] With rc69 a deferred session restore takes 97–114 s, against 60–68 s on rc68

When the first restore attempt after a launch hits the shared-state lock, rc69 returns shared_state_busy within seconds. The app is deferred three times in the first ~50 s, then logs no restore activity for about 49 s, and the attempt after that gap succeeds in ~10 s. At d99d5a6 and 291328d (rc68, same simulators) the deferred launches had one deferral and restored in 60–68 s.

Two simulators on staging at b633684, terminate then launch, 12 relaunches: 8 deferred at 96.6–114.3 s, one of them 363 s; 4 not deferred at 10.7–11.7 s. Full table and the fast/slow log excerpts: #856 (comment).

Slow launch, B5 (B_bitkit_foreground_2026-10-07_13-51-23.log, UTC):

13:51:23.220 PERF: init(walletIndex:) took 0.0 seconds on core queue
13:51:31.161 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager [resolveSessionInitialization line: 1786]
13:51:43.331 WARN: Failed to refresh public paykit endpoints after receive refresh: SharedStateBusy(code: "shared_state_busy", context: "Pubky shared state remains locked; retry later") - WalletViewModel [refreshBip21 line: 1496]
13:51:49.840 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
13:51:59.428 WARN: Failed to refresh public Paykit endpoints on foreground: SharedStateBusy(...) - WalletViewModel [line: 1358]
13:52:02.691 ERROR: Backup failed for: 'WALLET': SharedStateBusy(...) - BackupService [triggerBackup line: 262]
13:52:18.887 WARN: Deferred session restoration, keeping saved session - PubkyProfileManager
   -- 48.9 s without a paykit line, no WARN/ERROR --
13:53:07.824 DEBUG: paykit_session loaded from keychain
13:53:17.530 INFO: Paykit session restored for pubkydso8zn5... - PubkyProfileManager [line: 322]

Fast launch, B2: first line 13:41:43.678, Paykit session restored 13:41:54.607, no deferral.

The 363 s launch (A2) is the same pattern with a longer gap: two deferrals, a third attempt started at 13:41:58, then 304.8 s with no paykit or pubky line and no Deferred or restored result. During it Contacts opened as a screen titled "Profile" with only a spinner. It restored by itself at 13:47:11.099. Seen once in 12.

Fix: retry a deferred restoration on a short bounded backoff (a few seconds) instead of waiting for the next trigger. The launch after install restored at 58 s with two deferrals, so the lock was probably free well before the three-deferral launches tried again; the app log does not show when it was released.

Android shows the same at 7bc3ccb1a (64–115 s, three deferrals in the slowest): synonymdev/bitkit-android#1401 (review). What holds the lock at launch remains the open question in #868.

@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 480b8d25, covering the entire delta since d99d5a67, affected callers and tests, the rc68-to-rc69 SDK delta, and the complete current PR inventory. Supported unchanged coverage is inherited from the completed baseline. Ancestry, base and merge base were verified locally; HEAD was rechecked before returning.

No new actionable code findings.

The Contact Saved deletion concern is fixed in source, including stale Back history and newer-route preservation. Hardware failures now retain signed transactions and pending proofs unless local decoding definitively failed before any submission. Earlier uncertain attempts remain protected.

Validation: changed Swift files passed syntax parsing; a standalone harness using the exact pinned navigation helper and key normalizer passed 10 route assertions. Native XCTest assertions were inspected, not executed. Code-result and publication-preview validation passed. Pinned-head unit and integration CI were still running when collected. Dependency inspection used Paykit dd97fc9a and Core b53fa54a. Targeted comparison at Android 804e7547 retains older hardware error and expiry behavior; this does not establish current Android parity or a full Android review.

Startup, linking and backup delays remain unresolved; attributed rc69 relaunch measurements do not establish improved latency. The author's hardware lost-response recovery pass applies to b633684 and that single scenario. View reattachment, process death and other unchecked fault cases remain unverified.

Device testing: not performed in this review. Remaining author journeys and fault-injection cases remain required runtime work. Concurrent wallet/shared-Pubky installations and pre-2.6.0 profile compatibility remain excluded under the supplied product rules.

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

4 participants