feat: share paykit state across apps by ben-kaufman · Pull Request #1401 · synonymdev/bitkit-android

ben-kaufman · 2026-10-01T03:54:51Z

Closes #1356

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

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

Companions: iOS #856, Paykit Server #33.

Description

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

Manual Tests

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.
Force a lock conflict after acceptance commits but before its response read, restart Bitkit, then refresh and retry the accepted one-time request. Lock fault injection is not a journey capability.
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.
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.
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.

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.
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 rc62 resolves from GitHub Maven without a local override.

All 3,506 unit tests passed with the published rc62 dependency and no local override. Production and AndroidTest compilation passed. Formatting and fresh Detekt checks have no introduced findings; 19 verified baseline Detekt findings remain. Coverage includes preparing-sheet ownership, cancellation, queue ordering, wallet-wipe isolation, scoped retries, cleanup failures, and identity, payment and recovery behavior.

Staging performance is not signed off. Reviewer device testing confirmed a completed payment but still measured long send/preparation delays and missing progress feedback after bell Pay. Separate SDK-source profiling identified automatic request preparation holding the SDK queue for 33–36s, while request-list emission took under 2ms. That source build is not published rc62 or the latest #171 head. Cold start, 61-contact import, backup stalls, sharing cleanup and full content unlock remain open in #1406 and #1419 and the unchecked journeys above.

greptile-apps · 2026-10-01T04:18:59Z

Confidence 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

Unrelated output misattributes payment ▶
Failed cleanup leaves sharing advertised ▶
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]

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

github-actions · 2026-10-01T04:23:18Z

Regtest APK

Built from 9ffb08a (run).

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

ovi-reviewer

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)

ben-kaufman · 2026-10-01T04:52:38Z

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.

piotr-iohk · 2026-10-01T12:26:10Z

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

ben-kaufman · 2026-10-01T14:51:44Z

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

ben-kaufman · 2026-10-05T18:56:08Z

I checked: master moved again after the previous merge. I’m updating the branch onto that head while preserving the queue and cancellation fixes.

ben-kaufman · 2026-10-05T20:00:36Z

Updated with the latest master, preserving its contact/session guards and the restored queue fixes. Proof-store changes now refresh payment requests only when payment-relevant state changes; backup notifications remain unchanged. This avoids request refreshes when an unstarted proof is simply prepared or cancelled. All 3,487 unit tests pass, with no introduced formatting or lint findings. The device performance measurements are still in progress.

jvsena42

Re-reviewed 11f6861..d44d5bb (master merge plus one commit). No HIGH or MEDIUM; one LOW inline, not blocking.

Checked with no finding:

b43b5d7: for PubkyRepo.kt, PaykitSdkService.kt, ContactDetailViewModel.kt and EditContactViewModel.kt the merge delta equals master's #1417 diff; PubkyService.kt differs only by the dropped receiverPaths argument. The PR side is intact (completeSdkCall shields, blockPeer on delete, lock priorities). The new isStillCurrent check in saveContact sits under operationLock between the shielded identity read and the shielded save, and every session install takes the same lock, so the guard holds. It reads only an AtomicLong and a StateFlow, so no lock-order inversion. ContactSaveSessionChangeTest is adapted to the PR-side types.
d44d5bb: the new signal is built from completed proofs and in-flight request ids, the same two inputs synchronizeLocked reads. Started, completed, failed-and-released, submitted-and-removed, restore and identity change all emit. The backup version is still bumped on every save, and refresh coalescing still reads it directly.

Device gate: not run for these commits.

jvsena42 · 2026-10-05T20:39:05Z

-                refreshPaymentRequestTargets()
+    private fun observeIncomingPaykitPaymentRequests() {
+        viewModelScope.launch {
+            paykitPaymentProofRepo.paymentRequestStateChanges(pubkyRepo.publicKey).collect {


LOW — a rejected Pay no longer refreshes the request list

Before d44d5bb every proof-store save triggered this STORED refresh, including the prepare / cancelPreparation pair around a failed claim. Those two are now filtered out, which is the intent, but they were also what refreshed the list after a pre-start failure.

Scenario: the user taps Pay on a subscription period that another app already paid while the local projection still lists it. proceedWithPayment prepares the proof (:4096), claimForPayment throws RequestUnavailable on the paid period (PaykitPaymentRequestRepo.kt:868-881), the preparation is cancelled (:4105), and handlePaymentPreparationFailure (:5762) only toasts and hides the sheet. The request keeps showing Pay until the next 10 s poll, or longer while offline since the poll is skipped. The same applies to the other pre-start failures (blocked peer in ensurePaymentAllowed, acceptIncomingPaymentRequestIfNeeded, associateLightningPaymentProof).

Not blocking: the claim check still prevents a second payment. Would a STORED refresh in handlePaymentPreparationFailure, when an incoming request is involved, be worth adding?

Fixed in 0d21ff9. A failed incoming-request preparation now schedules a forced STORED refresh after dismissing the sheet, without proof reconciliation or another full inbox/send pass. The test holds that refresh open and checks that the sheet is already dismissed. iOS still refreshes from its proof-state changes, so it does not need the same fix.

ben-kaufman · 2026-10-05T21:20:10Z

Updated in 0d21ff9. Refreshes fetch from the SDK outside the request-list mutex, then apply the result after checking identity and intervening request actions. This lets presentation and local controls proceed during a network refresh. Public contact payment resolution also uses the separate read lane.

Proposal status now recognizes delivery completed by another drain, and skips the extra lookup when the exact message has a confirmed failed checkpoint. Failed incoming-payment preparation refreshes the stored request list without delaying dismissal. All 3,497 unit tests pass, with no introduced lint findings. Comparable device timings are still in progress; this is not a performance sign-off.

ben-kaufman · 2026-10-05T22:26:17Z

Fixed an uncaught cleanup error in 98a1570: after acceptance failed, checking the identity during proof cleanup could throw another lock-conflict error before the failure UI ran. That lookup is now inside the existing error handler. A failed lookup keeps the proofs, and cancellation still propagates. The regression test reproduces the escape before the fix; all 3,498 unit tests pass with no new lint findings.

I also checked iOS; its cleanup already catches this error. Please share the crash stack to confirm whether this is the crash from case 2, and the logs for case 3 and J12/J15, including the failed step. Those cleanup/linking failures are still open; this fix doesn't establish their cause. J8 also needs the missing private-list fixture before it can test that flow.

ovi-reviewer

Suggestion: 👍 Approve

Reaudit: diff 15 files.
No new findings; the rest is in the review.
The pair synonymdev/bitkit-ios#856 carries the same proposal-delivery fallback and journey assertion. Kotlin flow filtering and mutex changes preserve the shared data and payment contract.

QA:
Tests running.

Replies:

Please share the crash stack to confirm whether this is the crash from case 2. (comment)

@ben-kaufman Case 2 committed acceptance, then exited with a fatal ConcurrentUpdate during the injected response-read conflict. The recorded excerpt has no stack frames tying that exception to proof cleanup, so I cannot confirm that attribution yet. I verified the cleanup regression in this fix; Test 2 still needs to run again. Case 3 kept cleanup markers pending after the public-only fault recovered. J12 stopped at the initial linked-issuer precondition with a handshake-in-progress response; J15 stopped at linked-peer verification because fixture setup authority returned HTTP 401. Those records do not establish an app cause for the linking failures, and the full logs remain in the test artifacts.

Note

Retest Suggested J5, J16

@ovi-reviewer retest J5,J16

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

ben-kaufman · 2026-10-05T23:41:25Z

Fixed the missing foreground cleanup retry for case 3 in bb6cef7. Existing foreground-online maintenance now invokes the existing reconciliation helper, covering both private and public pending cleanup under the sharing lock. There is no new timer or retry state, and cleared private state performs no SDK work. The regression covers a failed cleanup, successful recovery on the same foreground connection, and no further public cleanup after the marker clears.

Compile and all 3,499 unit tests pass. Complete lint/format reports match the verified baseline: 19 pre-existing Detekt findings and no new diagnostics. This is unit/source coverage, not a device replay of the injected fault.

Case 2 remains unproven: the recorded ConcurrentUpdate after committed acceptance is not tied to proof cleanup by a stack, and neither cleanup fix establishes that the crash is solved. Please retain the full fatal stack and injection timeline, rerun cases 2 and 3 on this head, and retest J5/J16. The J12 handshake/J15 authority-401 and J8 missing-private-list fixture failures remain separate from app attribution.

@ovi-reviewer retest J5,J16

piotr-iohk

QA review

Scope: Full review of the complete PR diff against merge base d963abc, at b101e89. There is no prior completed baseline.

No new actionable code findings.

Execution claims, local one-time acceptance, the subscription recheck after the blocked-peer lookup, exact request destinations, receiving-address attribution, and the 41/124-byte auth payloads match the stated behavior. Paykit v0.1.0-rc62 rejects an execution claim once a request is canceled, and request-bound destinations return no payment-list version, which is the condition that allows a previously used on-chain address for an unpaid subscription period. iOS #856 at 060d7a0 matches those claim, recheck, and payload contracts; the comparison covers those contracts only.

Contact deletion still blocks the peer when private-list withdrawal fails. That best-effort limit is stated in the PR and in the open thread. State-backed SDK calls stay shielded across the five-minute pending-write cooldown; the author is keeping that behavior in the cooldown thread. A failed incoming-request Pay now schedules a forced stored refresh after the sheet closes, which is the behavior discussed in the stale Pay thread.

Build, lint, and the local e2e shards passed on this revision. This review inspected those runs and did not execute the unit suite locally. The e2e workflow runs the existing Appium shards; the journeys/ files remain manual.

Device testing: not performed in this review.

Ready for device testing.

piotr-iohk

QA review

Scope: Full review of the complete PR diff against merge base d963abc, at 9ffb08ac. No completed baseline was supplied, so this pass did not inherit prior coverage. Concurrent use of one wallet or one Pubky identity across installations, pre-2.6.0 profile formats, and guaranteed private-list withdrawal when deleting a contact stay outside this review.

No new actionable code findings.

One-time payment still requires the saved local acceptance plus a current shared accepted request. Subscription execution rechecks the SDK claim and rejects an already paid period before a wallet send. The confirm amount is the request amount, a mismatch is rejected, and swipe stays disabled while the request is preparing. Closing the sheet during preparation records the dismissal so a late result does not reopen it. A failed incoming preparation still forces a stored refresh after the sheet closes.

The head commit runs the 10-second inbox poll and maintenance intake at background priority, and it postpones routine contact-endpoint refresh while a sheet, overlay, or active contact payment is open. The same 10-second inbox, maintenance at 30 seconds then every 60 seconds, background priority, and sheet deferral are in iOS 9cb87b8 on bitkit-ios#856. That comparison is limited to those shared contracts, not a full iOS review.

The open deletion-withdrawal thread matches the PR’s stated best-effort limit. The shielded shared-state read remains an intentional queue hold: cancelling the SDK wait does not release the homeserver lock. Neither is a new defect at this head.

Validation: tests and journeys were inspected and not executed in this review. Build, lint, and detekt passed for this revision. On the local e2e run, settings, lnurl/transfer, and onboarding/backup/numberpad had passed; lightning/security, onchain/boost/receive/widgets, multi-address, and send were still pending. Runtime coverage belongs in the updated journeys. paykit.e2e.ts already walks the existing Paykit path, so this change does not need a new Appium spec.

Device testing: not performed in this review.

Ready for device testing.

Suggested additional test cases

Android, foreground, with a saved contact and an incoming request shown in the send sheet. Leave that sheet open through a maintenance round (first after 30 seconds, then every 60 seconds). Expected: Pay stays disabled until preparation finishes, and routine contact-endpoint refresh does not start until the sheet is closed and a later maintenance round runs. The same guard is in iOS 9cb87b8.

ovi-reviewer

Suggestion: 👍 Approve

Reaudit: diff 22 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#856 at 060d7a0 prepares incoming payments in the existing send confirmation sheet and passes background and interactive PaykitSdkOperationLock priorities through the same SDK wrappers.

QA:
Tests running.

Replies:

ben-kaufman: Pushed 75e5429. Sharing changes and foreground cleanup now use the same gate across both private and public withdrawal, so cleanup cannot overlap a later… (comment)

The branch still shares one withdrawal gate and drains only the selected recipient. It does not report the larger-contact or sharing-off retest.

piotr-iohk: Observation from manual checks. After paying an on-chain subscription, Android still offers that same request. Fresh wallets. iOS sent a daily subscription,… (comment)

Send-sheet preparation adds no paid-period check, so the paid request still being offered is not fixed here.

ben-kaufman: Fixed an uncaught cleanup error in 98a1570: after acceptance failed, checking the identity during proof cleanup could throw another lock-conflict error… (comment)

No stack here ties the case-2 ConcurrentUpdate to proof cleanup, and the pending endpoint-cleanup retry does not establish that cause.

Note

Retest Suggested 3, J5, J16, J17

@ovi-reviewer retest 3,J5,J16,J17

Reviewed by grok-4.7-xhigh via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

This was referenced Oct 1, 2026

feat: share paykit state across apps synonymdev/bitkit-ios#856

Open

feat: use identity-wide shared paykit state pubky/paykit-server#33

Merged

ben-kaufman marked this pull request as ready for review October 1, 2026 04:15

ben-kaufman requested review from jvsena42, ovitrif and piotr-iohk October 1, 2026 04:17

greptile-apps Bot reviewed Oct 1, 2026

View reviewed changes

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

This comment was marked as outdated.

Sign in to view

ovi-reviewer Bot reviewed Oct 1, 2026

View reviewed changes