feat: share paykit state across apps

[ben-kaufman]

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 progress while an explicitly selected incoming request is preparing, without blocking navigation.

• 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

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

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

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

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

• updated automatic-presentation.xml - linked contacts on separate identities automatically present new requests and defer them while another sheet is open.

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

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

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

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

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

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

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

• updated payment-deadline-history.xml - shared request expiry and history remain consistent.

• updated request-summary.xml - request details show the shared request and endpoint correctly.

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

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

• updated requested-resolution-failure.xml - progress is visible while preparing and clears after failure.

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,331 unit tests passed with the normal published rc62 dependency and no local override. Compile, formatting and fresh Detekt checks have no introduced findings; twenty verified baseline Detekt findings remain. Coverage includes bounded discovery, cancellation, 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]

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

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

Summary

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

Diagram

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

Loading

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

[greptile-apps]

Comments Outside Diff

These findings could not be posted inline.

•  Failed cleanup reports success app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt:315 ▶

  When private-list removal fails, this branch records a pending retry but exits with a successful Result. Callers disabling sharing therefore cannot detect that cleanup is incomplete and may report success while previously published private endpoints remain available.

[github-actions]

Regtest APK

Built from b752e7d (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]

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]

QA review

Scope: Reviewed the full PR diff against its merge base, at deac999.

No new actionable code findings.

Device testing needs a fresh wallet. Paykit presentation state, in-flight proofs, and wallet backups written with the previous receiver-path format do not decode. The author treats that migration as out of scope because Paykit has not launched; upgrading a development install that already stored that state leaves payment-request activation unfinished, and restoring one of those backups can leave later wallet backups blocked.

Android leaves a received transaction unassigned when its outputs match more than one contact. iOS #856 still assigns the reserved receiving-address contact in that case. See the open parity thread. This pass did not review the iOS PR.

CI build, lint, and the local e2e suites succeeded at this revision. This review inspected the changed payment, authorization, attribution, and persistence paths and their tests; it did not run those checks or a device. The Paykit journeys listed in the PR were not executed.

Suggested additional test cases

• Android and iOS. Fresh wallets. Receive one transaction whose receiving output is a registered companion-account address for contact A and whose other output matches a request or reservation for contact B. Android should store the companion address and leave the contact empty. Record the iOS result before treating the apps as matched.

Device testing: not performed in this review.

[jvsena42]

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

Checked and clean:

• Auth approval: the claim is parsed the same way at display, in the repo and in the service. authorize() re-checks the claim, clientId and permissions against the displayed state, approvalBootstrap pins the clientId, and a Paykit-only approval sends an empty account payload that validateAccountPayload enforces. Only derivePaykitIdentitySecretKey(generation) leaves the device, and the root key never does. Signup URLs reject companion claims.
• Amount pinning is unchanged (acceptsPaymentAmount / acceptsLightningInvoice). Endpoints are filtered by acceptedPaymentEndpointIdentifiers, and ensurePaymentAllowed runs before both rails.
• Received-payment attribution uses PAYEE records only, with network-checked addresses. Ambiguous matches stay unlabelled and an existing contact is never overwritten.
• The key-generation floor is keyed per public key and cleared by Keychain.wipe(). withStateRevisionTracking finalises under NonCancellable, and restorePrivateContact re-blocks on failure.
• The App Registry fetch per SDK call adds latency but no new offline failure: every shared-state transaction already reads the homeserver.
• Not raised, because nothing reaches them today: execution claims are never released on abandon (releasePaymentRequestExecutionClaim has no call site), and a missing registry counts as generation 1 against the saved floor. Both start to matter once a second executor app, or key rotation, exists.

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

[ben-kaufman]

The payment and backfill fixes are pushed in 2c05d91, with matching changes in iOS #856. All 3,110 Android unit tests passed, along with Kotlin compilation. The two-install journey is added on both platforms but has not been run on devices yet.

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

[piotr-iohk]

QA review

Scope: Follow-up from the full review at deac999 (review), at 2c05d919. This pass inspected the execution-ownership and contact-backfill delta, including payment and history callers. Coverage of the rest of the PR is inherited from that baseline.

No new actionable code findings.

The two-install payment and full-history backfill comments are addressed at this revision. An accepted one-time request stays payable only on the install that stored that acceptance, including after restart, and the other install keeps it in history without pay controls. A finished contact backfill is skipped until the identity, activity, transaction details, or reservations change; missing details and failed address derivation still retry. Recurring periods are outside that guard: an install that has the active payer subscription can still pay one. This pass did not review iOS #856; the attribution parity thread is unchanged on Android in this delta.

Build and lint passed at this revision. The new ownership and backfill tests were inspected, not executed here. Local e2e still had pending suites at check time. The new accepted-device-ownership journey is the device check for this delta.

Device testing: not performed in this review.

Ready for device testing.

[piotr-iohk]

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

[jvsena42]

Follow-up at 2c05d91. 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 local ownership: auto-present, the list, notifications, retry, the hardware path through prepareContactPayment and authorizeHardwareContactPayment, and the software path through the pre-send ensurePaymentAllowed(forExecution=true), including LNURL-pay. The SDK accept runs first, the local id is persisted next, the identity is re-checked, and the final gate requires the persisted id. A stale PROPOSED snapshot on the second install fails at the SDK accept.
• Backfill: PaykitReceivedPaymentContacts now has value equality, and the cache key includes the identity generation, activity revision and reservation attributionVersion. A mid-scan key change aborts the scan.
• The ledger is keyed by normalized identity, cleared by Keychain.wipe(), and kept out of backups.

Not raised:

• An accepted one-time request that no install owns, after a kill between accept and save or a restore to a new device, stays blocked until the payee cancels. That is the stated trade-off, and a payer-side cancel would reopen the race.
• Activation failing closed on an unreadable ledger matches the existing subscription-store behaviour.
• The orphaned-keychain thread is settled.

[ovitrif]

@ben-kaufman build check red

[ben-kaufman]

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

Updated to published rc62 in 172cb30. It includes the SDK fix for outbound batches exhausting peer leases while waiting on shared storage; the earlier batching and lightweight polling changes remain. All 3,314 Android unit tests passed against GitHub Maven without a local override. The staging APK contains the exact native library from that package, with no new lint or compiler findings.

This is not a staging performance sign-off. The latest rc61 run sent in 23-26 seconds but took 4.9-6.4 minutes to appear in Android’s open request list. rc62 fixes a separate multi-peer failure; the one-peer delay remains open, with results in #1406: #1406 (comment).

[piotr-iohk]

QA review

Scope: Full review of the complete PR diff at 172cb30 against merge base b29956a, including affected callers, persistence, authorization, payment execution, polling, attribution, tests and journeys. No baseline coverage was inherited. Unreleased receiver-format migration, guaranteed remote withdrawal on contact deletion, and simultaneous Bitkit use of the same wallet or Pubky identity remain outside the clarified requirements; immediate blocking was assessed.

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

Shared-flow comparisons used iOS 134dce1, rc62 SDK f2c5f71, server af0151a and Appium 72eea46. These were targeted contract comparisons, not full companion reviews.

Validation: git diff --check passed. An isolated Kotlin/JVM check of the pinned auth parser/encoder passed canonical fixture, claim-order and invalid-input cases; it did not exercise native SDK integration. Targeted app unit tests stopped before execution because GitHub Maven returned 401 for required artifacts. The existing latency concern remains unresolved: the latest measurements used rc61 and do not establish rc62 performance. Funded payment/unlock, the 61-contact case, cold start and backup stalls remain unverified; the separate broadcast-outcome dependency is #1384.

Suggested additional test cases

• Android: delete a saved, never-linked contact with link preparation queued behind SDK work. Release the queue, then send a private request. The peer must remain blocked and the request unavailable until explicit re-add.

Device testing: not performed in this review. The author's unchecked journey and fault-injection cases remain unexecuted here.

Findings

• [MEDIUM] Block deleted contacts even when no linked-peer record exists — inline at app/src/main/java/to/bitkit/services/PaykitSdkService.kt:667.

[jvsena42]

Code review at 172cb30 (Paykit rc62): no new findings. The two HIGH threads stay open until an Android device run.

Checked and clean since bb4637dd5:

• Merge of master + rc60 (dc73b29). Every conflict resolution keeps the shared-state model plus master's read lanes.
• Sharing changes. setEnabled is serialized under sharingMutex, and foreground reconciliation uses tryLock, so it skips an active change.
• Cleanup. The registry sync runs before the pending flag is cleared, and stays pending on failure.
• Sends. Proposals send only to the selected contact; the 60 s full refresh still drains the other peers.
• Refresh modes. STORED on the reminder tap, INBOX on the 10 s poll, FULL on maintenance.
• rc59 → rc62. Third-party approvals still pin /pub/paykit/:rw, so the authorizer scope cannot leak. No change to the amount, period or ownership gates.

Discovery now runs outside the SDK lock, and the poll is light, so I expect both HIGHs to be settled or much reduced.

On the iOS twin at the same SDK (synonymdev/bitkit-ios#856, 134dce100), nothing fails:

• first link: ~5 min
• Send → "Sent": 33–100 s
• contact payments off: ~110 s
• one real payment, paid and received

Device gate: not run — the Android build at this head can't download paykit-android 0.1.0-rc62 from GitHub Packages on my machine (401, expired package token). I'll run it once that is fixed.

[ben-kaufman]

Android now coalesces overlapping request refreshes when the completed refresh covers the requested mode and the identity and payment-proof state are unchanged. A stronger refresh, failed refresh, proof change or clock change still causes another read. On-chain events with no affected contact address also skip SDK access.

Pushed in a29e32d, followed by the deleted-contact fix in e8a7686. All 3,320 unit tests passed using the cached published rc62 package, with no local SDK override and no introduced lint findings.

The staging latency, bell Pay navigation and cold-start findings remain open. These app changes need another device run and do not include the additional local SDK optimizations.

[ben-kaufman]

Fixed another startup problem in b4b2141: Crypto was removing Android's global BC provider before installing its replacement. TLS initialization could run during that gap and fail with BKS not found, leaving its verifier unusable for that process.

Crypto now uses a private provider without changing Android's registered providers. Key formats and encrypted payloads are unchanged. The isolated race reproduction failed 3/3 times before the fix and passed 3/3 after it. A cold emulator launch with published rc62 had no BKS/verifier errors and preserved the existing identity.

All 3,322 tests passed, with no new lint findings. This fixes the provider race, not the remaining latency: that cold run still took 98.9s to restore the Paykit session, so the performance issue stays open.

[ben-kaufman]

Public endpoint publication now waits for session restoration, without waiting for the full contacts/profile load. App publication reuses the identity status returned by initialization. Failed or cancelled operations invalidate the backup fingerprint without another remote read, preserving the original error.

All 3,324 unit tests passed on the final code. Detekt has the same 20 existing findings and none in the changed files. The app still uses published rc62; no local SDK override. These changes remove redundant startup/error-path work, but staging performance remains open.

[ben-kaufman]

Message drains now skip handshake advancement for peers already marked Linked. Sending and receiving still validate the key and recovery state; if they detect recovery is needed, the next retry advances it.

The two-peer retry test removes four redundant advancement calls while preserving all sends and receives. Matching tests pass on both platforms. This is in iOS ec2071f5 and Android e7aad37, with rc62 unchanged. It reduces unnecessary work but does not yet close the staging latency issues.

[piotr-iohk]

QA review

Scope: full PR review at 0eaae43 against merge base b29956a, covering shared state, authorization, contact sharing, request/subscription execution and recovery, activity attribution, lifecycle integration, and changed tests/journeys. Checked published rc62 source and targeted iOS parity at 89b1c24; companion checks were not full reviews.

No new actionable code findings.

Validation: CI build and unit tests passed at the reviewed head. Local targeted unit execution stopped before tests because GitHub Maven returned HTTP 401. Changed journey XML/port names and the Android/server claim fixture were checked.

Existing SDK lock/latency and bell Pay concerns and cold-launch target discovery remain open. The latest attributed staging evidence is from 172cb30; subsequent source optimizations do not establish a current-head runtime pass. Failed contact-deletion withdrawal remains an explicit limitation, with cleanup potentially waiting for re-addition.

The iOS companion currently conflicts. Staging latency, funded payment/content unlock and the author's unchecked cases still need verification on integrated builds; separate broadcast-outcome dependencies remain Android #1384 and iOS #844. Device testing: not performed in this review.

[jvsena42]

Reviewed 172cb30..0eaae43 (7 commits). Two LOWs inline.

Checked with no finding: the private crypto provider (no Security.* mutation left, known-vector decrypt test covers compatibility), endpoint publication waiting for session restore, the Linked-peer drain skip, batch withdrawal, and e8a7686 (rc62 block_peer creates a default record when none exists, so a never-linked contact still deletes).

Device gate (partial): tablet relaunched on this head restored its session in 40.5 s and 35.2 s (138 s at 172cb30), with no concurrent_update init failures (three before) and 76 Resolving homeserver lines/min at idle with zero contacts (~200 before). The two-device flows did not run yet: the second emulator was a dev install upgraded from master, which is not a supported path. Rerunning them on a fresh identity; numbers will follow on the latency thread.

[jvsena42]

Re-reviewed 172cb30..a38f515. No HIGH or MEDIUM. Both LOWs from the last pass are closed: the backup invalidation is kept by design, and a38f515 makes the contact-change and bell Pay paths use refreshAfterStateChange().

Device gate: two emulators on staging at 0eaae43, fresh linked pair. First link, request both ways, bell Pay, on-chain payment, sharing OFF/ON, relaunch and contact deletion all completed with no crash, lease expired, RequestUnavailable or timeout lines. a38f515 was not rerun on a device; it only changes which refresh call those paths use. Numbers are in the two threads resolved above.

The remaining wait times (bell Pay → confirm 121 s with no progress shown, sharing OFF ~5 min, relaunch after a sharing change up to 7 min) are tracked in #1419 with a journey, to re-measure once a paykit release with paykit-rs#171 is pinned. iOS twin: synonymdev/bitkit-ios#868.

[piotr-iohk]

QA review

Scope: full review of the complete PR diff at a38f515 against merge base b29956a, including affected callers, persistence, tests and journeys; no baseline coverage inherited. Unsupported concurrent use of one wallet and pre-2.6.0 profile formats were excluded.

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

Shared-flow contracts were compared with iOS f23151b, rc62 SDK f2c5f71 and server af0151a. These were targeted comparisons, not full companion reviews.

Validation: the matching-head CI build and unit-test job passed. Local unit tests stopped before execution because required GitHub Packages artifacts returned HTTP 401. Diff checks passed; all 16 changed XML journeys parsed and have iOS ports (source comparison only).

Existing latency and bell Pay concerns and cold-launch target discovery still need verification at this head; older device measurements do not clear them. Failed withdrawal after contact deletion remains an explicit contract limitation and can require re-addition. Separate rejected-broadcast work remains in Android #1384.

Suggested additional test cases

• Android: with contact payments on, delay Paykit feature-disable cleanup, switch Paykit UI off/on, enable Contact Payments, then release the older cleanup. Expect the latest enabled preference and published endpoints to remain consistent.

Device testing: not performed in this review; the author's unchecked journeys and fault-injection cases remain unexecuted here.

Findings

• [MEDIUM] Serialize Paykit UI cleanup with sharing changes — inline at app/src/main/java/to/bitkit/viewmodels/SettingsViewModel.kt:291.

[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, "1k", for 1,000 sats. Android paid it from Savings: Review & Subscribe, then the fee confirmation. The send completed (−1,141 sats) and iOS showed "1 subscriber, 1 payment". Android then opened Payment Requests for the same "1k" / 1,000 sat request, with Pay and Dismiss, and the bell stayed.

The bell does not stay after the same payment on master. The iOS twin (#856) does not show this sheet when Android is the payer.

This is not the on-chain second swipe. That confirmation is expected.

Open question: can this be attributed to the performance issue (#1419), fixed here, or put in a separate issue?

Android payer	iOS payer
Screen.Recording.2026-10-05.at.12.23.41.mp4	Screen.Recording.2026-10-05.at.11.51.53.mp4

subscription-logs-2026-10-05.zip

[ben-kaufman]

b752e7d adds progress feedback while an explicitly selected request is being prepared, without blocking navigation, plus the cleanup and attribution fixes. iOS has the matching changes in 407c282e. All 3,331 Android tests passed; the complete Detekt report has the same 20 baseline findings and no introduced findings. The app still uses published rc62. The indicator was observed on the emulator, but the full success/failure journeys and staging latency remain open. I am also investigating the paid-subscription bell report separately; it should not be dismissed as a performance issue without tracing the payment and presentation state.

[jvsena42]

Re-reviewed a38f515..b752e7d. No HIGH or MEDIUM; two LOWs inline, neither blocking.

Checked with no finding: feature-disable cleanup now shares sharingMutex with enable and reconcile (lock order sharingMutex → publication → SDK, no inversion); the Bech32 normalization only lowercases fully uppercase BC1/TB1/BCRT1 values, so Base58 and mixed case are untouched and no two addresses collide; the recovery step is bounded to one advance under the existing lock and a failed peer does not stop the others; the progress state is cleared on success, expiry, unavailable, retry exhaustion and identity mismatch.

Device gate: not run for this commit. The progress indicator and the cleanup changes have not been driven on a device by me; the 0eaae43 run stands for everything else. Latency stays with #1419. The paid-subscription bell report from manual testing is open with the author and not covered here.

[jvsena42]

LOW — a Linking or RecoveryRequired peer makes the first "disable contact payments" report an error

These peers are now part of the cleanup keys, but pendingPrivateMessageDrainKeys (:1195, else -> true) counts any peer that is not linked, blocked or unknown as pending, whatever its outbound queue holds. In rc62 a Linking peer's empty list is queued and reported cleared with delivery skipped, so the key still ends in the failed set and PrivateUnavailable is thrown (:1408). The one-step recovery moves RecoveryRequired to Linking at best, which is still pending. For a peer without a saved contact nothing advances the link afterwards (:1054 skips unsaved keys).

It does not stay stuck: disable() still publishes the capability as off, and the next reconcile gets a null report from clearPrivatePaymentLists and clears cleanupPending. So the user sees one generic error toast with the toggle already off. A saved contact stuck in Linking behaved this way before; this commit widens it to peers without a contact (a restored backup with recovering peers, or another app on the identity). Would it be enough to treat a peer whose empty list was cleared and has no pending outbound as done? Same shape on iOS (PrivatePaykitService+Contacts.swift:262).

[jvsena42]

LOW — the indicator can stay up after a notification tap with no identity

requestedPaymentRequestId is also set by onPaykitSubscriptionNotificationTapped (AppViewModel.kt:953), and :951 lets a null identity through. If the user has signed out or turned Paykit off and taps a stale notification, refreshIncomingPaykitPaymentRequests returns early (:887), no retry timer starts, and nothing calls clearRequestedPaymentRequest. The indicator then shows until a sheet opens and closes. This condition only checks currentSheet == null.

The id could get stuck before this commit too; it just was not visible. A cold start from the notification is fine, since the id resolves once the identity loads. Could the tap skip setting the id when Paykit is disabled or there is no identity to wait for, or the condition here also require Paykit enabled?