fix: streamline paykit foreground sync

[ben-kaufman]

Description

This PR makes incoming Paykit requests appear sooner while Bitkit is open, without running the full synchronization flow on every inbox check.

• Updates Paykit to 0.1.0-rc54 and uses its backup-content fingerprint so transient SDK operation leases do not trigger wallet backup uploads. Real changes, including partial failures and cancellation, still request a backup; unreadable fingerprints are handled conservatively.
• Checks incoming private messages after 5, 10, 15, then 30 seconds of inactivity. New requests reset that backoff.
• Keeps contact/path discovery, endpoint maintenance, proof reconciliation, and eligible-recipient refresh on a separate 30/60/120-second schedule.
• Preserves the existing bounded handshake burst, foreground lifecycle, 120-second unresolved-request presentation retry, and private-only resolution.

The shorter inbox interval trades extra request-check traffic for responsiveness. Repeated request changes can keep the foreground inbox at its five-second minimum delay. This is intentional for active exchanges and is not a claim of lower battery usage overall.

SDK changes: pubky/paykit-rs#157
Release: https://github.com/pubky/paykit-rs/releases/tag/v0.1.0-rc54

Design

N/A — no UI changes.

Preview

N/A — no UI changes.

QA Notes

Manual Tests

• 1. Linked Paykit contact → keep Wallet open for two minutes → send a new request from the contact: Payment Request opens automatically, normally within the next 30-second inbox check plus processing time.
• 2. regression: background Bitkit → send a request → return to Wallet: the foreground refresh discovers it; no recurring polling continues while backgrounded.
• 3. Payment Request → dismiss the sheet → open the bell → reopen the request: the request remains available and contact/amount are unchanged.
• 4. Add a contact on both peers → return to Wallet: the existing bounded handshake burst still establishes the private connection; no public payment fallback is introduced.

Automated Checks

• Extended the existing polling test in AppViewModelSendFlowTest.kt to verify the five-second reset when requests change, subsequent 10/15-second backoff, independent maintenance, and stopping polling.
• Added PaykitBackupStateTrackingTest.kt for equal, changed, and unreadable fingerprints, partial failures, and coroutine cancellation using the same tracking helper as the SDK service.
• All 2,479 unit tests passed across 166 suites against rc54 downloaded from GitHub Maven, with no failures or skipped tests.
• just compile, just test, just lint, and the dev debug APK build passed. No local SDK artifact was substituted.

[greptile-apps]

Confidence Score: 4/5

The implementation appears safe to merge, with non-blocking test gaps around backup fingerprint decisions and request-driven inbox backoff reset.

Findings

1.  Fingerprint branches lack tests ▶
2.  Backoff reset remains untested ▶

Summary

• Polls incoming requests after 5, 10, 15, and then 30 seconds, resetting the inbox cadence when requests change.
• Runs endpoint, proof, and eligible-recipient maintenance on a separate 30/60/120-second schedule.
• Preserves the 120-second long-term presentation retry independently of inbox polling.
• Uses Paykit rc54’s suspendable backup revision and checks it after successful, failed, or cancelled operations.
• Extends polling cadence tests, although reset behavior and backup fingerprint branches still need focused coverage.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[App enters foreground] --> B[Start initial bounded sync burst]
    A --> C[Start recurring inbox loop]
    C --> D[Wait 5 / 10 / 15 / 30 seconds]
    D --> E{Maintenance due?}
    E -- No --> F[Refresh incoming requests]
    E -- Yes --> G[Refresh private endpoints]
    G --> H[Reconcile payment proofs]
    H --> F
    F --> I{Pending requests changed?}
    I -- Yes --> J[Reset inbox delay to 5 seconds]
    I -- No --> K[Advance inbox backoff]
    E -- Yes --> L[Refresh eligible recipients]
    J --> D
    K --> D
    L --> I
    M[App leaves foreground] --> N[Cancel polling and retry jobs]

Loading

_{Reviews (1) · Last reviewed commit: "fix: streamline paykit foreground sync"}

[github-actions]

Regtest APK

Built from f030603 (run).

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

[jvsena42]

Reviewed as a first pass, with the backup change as the thing that mattered. No HIGH or MEDIUM. Two LOW notes inline, neither blocking.

The backup-skip logic is correct, and the conservative-on-error claim holds. This was the risk worth the time: making backups conditional on a fingerprint invites a false negative where a real state change stops being backed up and is lost on restore. I enumerated every path that bumped backupStateVersion on master against head:

Case	master	head
Success, content changed	bump	bump	same
Success, only a lease/counter changed	bump (spurious)	no bump	the intended fix
Block throws (partial failure)	bump if revision differs	finally -> bump if differs	same
CancellationException mid-block	bump if differs	finally + NonCancellable -> bump if differs	same, and the post-read can no longer itself be cancelled
Previous read unreadable	skip (null != null is false)	bump	head is stricter than master — this fixes a latent false negative
Next read unreadable	skip if previous also null	bump	stricter
Unconditional notifyBackupStateChanged() callers (importSession, signUp, signIn, completeAuth, publishPaykitProfile, uploadProfileAvatar, deletePaykitProfile, removeContact, clearStateLocked)	bump	bump	untouched

:927 reads previousRevision == null || nextRevision == null || previousRevision != nextRevision, so it skips only when both reads succeed and match. The author's "unreadable fingerprints are handled conservatively" is accurate, and the pre-read case is actually an improvement over master.

What the fingerprint covers, and why a false negative needs a hash collision. It is SHA-256 over the same export_backup_state() payload the wallet backup itself ships (exportBackupString = hex(export_backup_state().export_bytes()), consumed at PrivatePaykitRepo.kt:511 into WalletBackupV1.paykitSdkBackupState). So the only state that can change without moving the fingerprint is state that is not in the backup — operation leases, and rc54's in-memory private_payment_list_publications. Concurrency cannot manufacture a mismatch either: every tracked call holds operationMutex, so the before/after reads are serialized per op.

The rc51 -> rc54 bump, checked against the actual SDK diff rather than the release notes alone (gh pr diff 157 --repo pubky/paykit-rs, plus files at the tag):

• New backupStateRevision() — adopted here.
• PrivateStreamIntakeReport.receive_batch_id became Option<u64>. The app never reads receiveBatchId — verified two ways (git grep at head and a working-tree grep -rn); only .counterparty and .error are read, at PaykitPaymentRequestRepo.kt:961-969. No adaptation needed.
• Empty receives no longer allocate a batch or checkpoint Noise, and refresh_active_identity returns the previous state instead of rewriting initialized_at. Behaviour, not schema.
• Unchanged reservation-backed lists are reused in memory scoped to the link id, so report.queued is empty on reuse. applyPrivatePaykitRepo.applyPrivatePaymentListDeliveryReport only records cache entries for queued/cleared and derives retry keys from them, so an empty queued on an already-cached publish is a no-op. Not broken.
• rc53's fetchPubkyFileBounded is opt-in and unused here.
• No wire/path/schema changes, consistent with the diff — no field removals in storage/records.rs, only additions.

Gating trace, since it decides severity: ContentView.kt:296 starts the polling loop unconditionally on ON_START, so the timer runs for everyone. The work inside is gated — inbox refresh returns early at AppViewModel.kt:807 unless isPaykitEnabled (default false, SettingsStore.kt:42), a public key exists and a wallet exists; the maintenance call is gated inside canPublishPrivateEndpoints() on sharesPrivatePaykitEndpoints (default false), which is pre-existing and whose 30/60/120s cadence is unchanged. So: affects opted-in users on released builds.

Upgrade path from the shipped releases. v2.4.1 (published Latest) pins 0.1.0-rc31 and v2.5.0 pins 0.1.0-rc51, verified with git cat-file -p "<tag>^{commit}:gradle/libs.versions.toml". export_backup_state is the same function those releases already ran for every wallet backup and tolerates identity_state: None; rc54 makes no persisted-schema change. If the first post-upgrade read fails anyway, head backs up. Restore-time bumps stay suppressed by shouldSkipBackup() (BackupRepo.kt:134,360), and polling cannot start before state is loaded (walletExists() / isAvailable() / isSetup.await()).

Also checked and clean: the diff adds zero Logger.* calls and touches nothing seed-derived. Faster polling cannot re-present a dismissed request — dismissal marks presentedRequestIds and persists per identity (PaykitPaymentRequestRepo.kt:324-328), automaticPendingRequests() filters on it, and subscription dismissals are re-intersected on every sync (:680). Identity pinning is unchanged: refresh() captures stateGeneration and activeIdentity before taking the lock (:385-386). No reentrancy — the loop is sequential, refresh() holds operationMutex, there is a single job with an isActive check at :828, cancelled on ON_STOP. The 120s presentation retry is preserved via the new constant at :5444. Conventions are right: runSuspendCatching on both suspend reads, Duration arithmetic, no runBlocking, no System.currentTimeMillis().

Not re-raising greptile's point about the fingerprint branches being untested — I agree with it given the stakes, and it is already on record.

Could not verify: whether uniffi-Kotlin cancellation actually drops the Rust future rather than detaching it onto an internal runtime. If it detaches, a cancelled op could write after the finally read. The Rust storage layer uses synchronous transactions so this is unlikely, but only an instrumented test would settle it. I also did not inspect the binary AAR, so the exact Kotlin signature of backupStateRevision() is inferred from the committed Swift binding plus the fact that this compiles.

[jvsena42]

Addendum — f030603a3 landed while I was posting the review above. It closes the test gap, no new findings. My two LOW notes still stand as written; both are about behaviour this commit did not change.

The refactor extracts withPaykitBackupStateTracking as a top-level suspend function over readRevision/onChange/operation, which is what makes it testable without a live SDK. The predicate is carried over unchanged — previousRevision == null || nextRevision == null || previousRevision != nextRevision — and withContext(NonCancellable) is preserved around the post-read, so the enumeration in my review still holds line for line.

PaykitBackupStateTrackingTest covers the decision in both directions: all four (before, after) combinations including both unreadable variants, partial failure preserving the original error via assertSame, and cancellation after mutation.

The cancellation test is the one worth calling out — it is stronger than the iOS twin's. It launches a real coroutine, mutates, then cancelAndJoin()s, with a yield() inside readRevision. That yield() is the load-bearing part: in a cancelled coroutine it would throw if the post-read were not inside withContext(NonCancellable), so the test actually proves the NonCancellable wrapper does its job rather than just asserting the happy path. That is the property I most wanted covered here, since it is what keeps a cancelled-mid-write operation from silently skipping its backup.

For cross-repo reference, the iOS twin landed the equivalent suite in the same window (synonymdev/bitkit-ios#748, PaykitBackupStateTrackingTests.swift), with the same four-case table and the same partial-failure case. The two platforms now express the same rule in the same shape, which is the right outcome for a pair of PRs that have to agree about when a wallet backup is owed.

[ben-kaufman]

The staging pubky_paykit job failed before starting the emulator or running Bitkit. sdkmanager could not parse/download the Android repository metadata and then reported that system-images;android-33;default;x86_64 could not be found. This is runner/SDK setup, not a failing app assertion. Build, lint and detekt passed.

I tried rerunning just that job, but GitHub rejects the retry while the parent workflow is still running. It needs a retry once the remaining matrix jobs finish.

[jvsena42]

tAck

• ✅ Send request with recipient in foreground
• ✅ Send request with recipient in background
• ✅ Cross-test with iOS
• ⚠️ Found some UI bugs, not introduced by this branch. I'll test again after merge on master