Skip to content

feat: add Paykit subscriptions - #1186

Open
ben-kaufman wants to merge 24 commits into
masterfrom
codex/paykit-subscriptions-android
Open

feat: add Paykit subscriptions#1186
ben-kaufman wants to merge 24 commits into
masterfrom
codex/paykit-subscriptions-android

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the payer side of Paykit subscriptions, building on the merged payment-proof work in #1199 and #1178.

  • Receives private subscription proposals and lets the payer review and confirm them.
  • Discloses the first billing period’s end date/time and full-period charge on Review, including short initial intervals.
  • Requires an immediately due first period to be paid before showing the subscribed success state; definitely failed payments remain due and can be retried, while uncertain submissions stay protected until their outcome is known.
  • Turns each unpaid billing period into the existing incoming payment-request flow, including automatic presentation, the bell queue, and the Payments tab.
  • Schedules local due-payment notifications and opens the corresponding request when the notification is selected.
  • Adds the Figma-aligned Subscriptions and Payments tabs, subscription detail/status views, cancellation, empty states, and fixed/open-ended summaries.
  • Shows a normalized Monthly Cost across all active subscriptions, independent of whether the current periods are already paid.
  • Associates payment proofs with their exact billing period so recurring periods remain independent while retaining private-only resolution and fresh Private Payment List requirements.
  • Reuses the shared amount-entry component without duplicating its secondary currency amount.
  • Keeps queued delivery visible even when a request has a note, and lets long request details scroll while Pay/Dismiss stay accessible.
  • Keeps subscribe-and-pay inside one continuously presented sheet, including initial-payment failure and retry, without the modal dismissal/reappearance gap.
  • Retains the settled payment rail in completed request and subscription history so Lightning feedback is purple and on-chain feedback is orange, including success confetti.

This PR intentionally covers payer-side receipt only. Subscription creation, Discover, automatic payment without confirmation, and renewal are not included because they are outside this MVP or are not currently exposed by Paykit.

Dependencies:

Preview

Final Android and cross-platform proof recordings were completed locally; they are not attached to the PR.

QA Notes

Manual Tests

  • Receive an immediate-start private subscription proposal and verify review → payment → success stays in one continuously presented sheet.
  • Exercise initial-payment failure and retry and verify the Send sheet remains presented while the first period stays due.
  • Verify Lightning and on-chain subscription success choose purple/orange confetti and matching completed-payment icons.
  • Receive a future-start proposal, trigger its due notification, and open the exact billing-period payment.
  • Dismiss and reopen a due request from the bell queue and Payments tab, then pay it with fresh private payment details.
  • Review active, expired, fixed-term, and open-ended subscription details and cancel an active subscription.

Automated Checks

  • Full testDevDebugUnitTest suite: 2,421 tests passed on the latest UI fixes
  • Focused PaykitPaymentRequestRepoTest, PaykitPaymentRequestRepoSubscriptionTest, SubscriptionsScreenTest, and AppViewModelSendFlowTest
  • CreatePaymentRequestScreenTest: all 4 emulator UI tests passed. The extended amount test verifies a single secondary amount in Bitcoin and fiat modes. The sent-screen test also verifies queued versus sent delivery with a nonempty note, and failed before the wording fix.
  • compileDevDebugKotlin
  • compileDevDebugAndroidTestKotlin
  • detekt --rerun-tasks
  • arm64 assembleDevDebug, install, and cold launch on API 36 emulator CodexBullTlsUiApi36
  • Temporary emulator check: a supported 20-line note scrolls with Pay/Dismiss still visible. The probe and temporary visibility changes were removed.
  • git diff --check

Known limitation

Full recovery of orphaned Lightning submissions is tracked in LDK Node #116. A missing payment-history record is not proof of failure, so unknown started attempts remain protected and may block subscription cancellation. The latest submission-error fixes and first-period disclosure are covered by automated regressions. The earlier recordings/manual checks predate these review fixes; no new crash-injection E2E or UI recording was performed for this update.

@greptile-apps

This comment has been minimized.

@jvsena42

Copy link
Copy Markdown
Member

could test the PR stack feature for this case

@ben-kaufman
ben-kaufman force-pushed the codex/paykit-payment-proofs-android branch from f189535 to a1a7907 Compare August 28, 2026 13:29
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from 2eecc75 to baf9894 Compare August 28, 2026 14:36
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-payment-proofs-android branch from 9e4ee27 to 19823e6 Compare August 31, 2026 13:10
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from baf9894 to 5a876f8 Compare August 31, 2026 13:10
Comment thread app/src/main/java/to/bitkit/repositories/PaykitPaymentProofRepo.kt
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from 5a876f8 to bb2f12f Compare August 31, 2026 17:53
@ben-kaufman
ben-kaufman changed the base branch from codex/paykit-payment-proofs-android to codex/paykit-hardware-payment-proofs-android August 31, 2026 17:54
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from bb2f12f to 050b32b Compare August 31, 2026 17:58
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

Done: the PR now uses the stack directly. #1186 is based on the hardware-proof fix #1199, which is based on #1178, so each review contains only its own layer.

ovitrif

This comment was marked as resolved.

@ben-kaufman
ben-kaufman requested a review from ovitrif August 31, 2026 23:02

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

Duplicated component

Image

@ben-kaufman
ben-kaufman force-pushed the codex/paykit-hardware-payment-proofs-android branch from 82d6c73 to e1053b0 Compare September 1, 2026 12:48
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from 1cbcc6f to 4cfdf4e Compare September 1, 2026 12:48
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-hardware-payment-proofs-android branch from e1053b0 to 850c252 Compare September 1, 2026 12:54
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from 4cfdf4e to dd53772 Compare September 1, 2026 12:54
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 Fixed in dd53772. The duplicated fiat value came from replacing the subscription review sheet while its content was still composed. The flow now reuses the existing dismiss-delay-present sheet transition and waits for that transition before validating the destination. The focused send-flow regression passes.

@ben-kaufman
ben-kaufman requested a review from jvsena42 September 1, 2026 13:03
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

The duplicated subscription payment UI is fixed in signed commit dd53772. The review sheet now completes its normal dismiss transition before the Send sheet is presented, and the focused send-flow regression covers the sequence. @jvsena42 please re-review the updated head when E2E finishes.

ovitrif

This comment was marked as resolved.

@ben-kaufman
ben-kaufman force-pushed the codex/paykit-hardware-payment-proofs-android branch from 850c252 to bac8656 Compare September 1, 2026 18:06
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-subscriptions-android branch from dd53772 to 9bd7677 Compare September 1, 2026 18:06
@ben-kaufman
ben-kaufman force-pushed the codex/paykit-hardware-payment-proofs-android branch from bac8656 to 6bb8501 Compare September 1, 2026 18:19

Copy link
Copy Markdown
Contributor Author

Pushed 90b6c09, matching the iOS safeguard. Uncertain Lightning submission errors now retain the payment proof and open the existing Pending screen. An already-started proof also blocks another attempt before private details are consumed, including switching to a hardware payment. Definite failures still clear normally, and unrelated proof-delivery errors remain best-effort.

All 2,380 unit tests pass. Compilation and detekt completed, with only the existing lint findings.

Full crash recovery remains a separate LDK Node issue: synonymdev/ldk-node#116. Missing payment history is not treated as failure, and crash-injection E2E has not been run for this gap.

ovitrif

This comment was marked as 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.

Traced proposal → accept → auto-started first payment → period materialisation → presentation → proof lifecycle → cancellation, and cross-checked the period math against iOS #685.

All nine threads from my earlier pass are genuinely fixed at head — queue emptied before the dismissal is persisted (now persist-first with a bail, :279-287), the cross-identity keychain write (isCurrentState guard precedes every write, :632; activate()/clear() bump the generation before taking the mutex), _onchainPaymentResolution coalescing (now a list consumed per item, :471-484), and the retry hijack (gated on isInitialSubscriptionPayment, SendSheet.kt:520-527). Not re-raising the total-debit-on-auto-start one you closed as a cross-platform product decision.

Fund safety checks out. No auto-pay path beyond the first period — shouldAutomaticallyPay (:5332-5333) requires isInitialSubscriptionPayment, which only acceptSubscriptionAndStartPayment / retryInitialSubscriptionPaymentInCurrentSheet / showInitialSubscriptionPaymentFailure set; every later period goes through presentIncomingPaymentRequestOrStop and needs a swipe. QuickPay can't short-circuit a presented request (:3348, :3400). The amount is pinned msat-exact by acceptsPaymentAmount/acceptsLightningInvoice (:3836-3850) before every send. Double-pay is guarded three ways (isSubmittingPaymentRequest, prepare() throwing OperationInProgress on a started proof, and in-flight/completed ids filtered out of the due queue). Acceptance is persisted before the queue is mutated (:512-514).

One medium and two low notes inline.

Comment thread app/src/main/java/to/bitkit/repositories/PaykitPaymentProofRepo.kt
Comment thread app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
Comment thread app/src/main/java/to/bitkit/repositories/PaykitSubscription.kt
@jvsena42

jvsena42 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Regression test — pre-dispatch node timeout leaves a permanently started proof

Test-only, applies cleanly onto codex/paykit-subscriptions-android (5339b96).

./gradlew :app:testDevDebugUnitTest --tests 'to.bitkit.repositories.PaykitPaymentProofRepoTest'

Fails on head (32 tests, 1 failed; the other 31 pass):

PaykitPaymentProofRepoTest > node run timeout before dispatch clears proof FAILED
java.lang.AssertionError: Expected value to be true.
  at PaykitPaymentProofRepoTest.kt:352

Line 352 is assertTrue(repo.failLightningPayment(PAYMENT_HASH, NodeRunTimeoutError("payInvoice"))) — it fails because the error falls to else -> return false and is classified uncertain.

Passes with is NodeRunTimeoutError, added to the definite branch at :318-323. All 32 green, and importantly no sibling test brokeuncertain lightning submission preserves proof until settlement still passes, because it lists a bare AppError("payment outcome unknown") and NodeRunTimeoutError is a distinct subclass, so the new branch doesn't swallow the genuinely-unknown case.

For the downstream consequence the test uses the store's own predicate rather than new mocking: after the failure a fresh prepare succeeds (on unfixed code it's rejected with OperationInProgress, as the sibling test asserts), plus storedProofs.none { it.requestId == request.id && it.paymentStarted } — that paymentStarted filter is exactly what PaykitPaymentProofStore.inFlightRequestIds uses at PaykitPaymentProofStore.kt:42-44. Asserting on inFlightRequestIds directly isn't meaningful here since the store is a mock in this class.

⚠️ The one-liner is a partial fix. It doesn't cover the other pre-dispatch path — the bare AppError("Cannot execute 'payInvoice': node is '…' and not starting") at LightningRepo.kt:252-256. By type alone that is indistinguishable from a genuinely-unknown outcome, so closing it properly needs a distinct error type there too.

pr1186-regression.diff
diff --git a/app/src/test/java/to/bitkit/repositories/PaykitPaymentProofRepoTest.kt b/app/src/test/java/to/bitkit/repositories/PaykitPaymentProofRepoTest.kt
index 20a3064c7..e406beba1 100644
--- a/app/src/test/java/to/bitkit/repositories/PaykitPaymentProofRepoTest.kt
+++ b/app/src/test/java/to/bitkit/repositories/PaykitPaymentProofRepoTest.kt
@@ -342,6 +342,20 @@ class PaykitPaymentProofRepoTest : BaseUnitTest(StandardTestDispatcher()) {
         }
     }
 
+    @Test
+    fun `node run timeout before dispatch clears proof`() = test {
+        val request = paymentRequest(MethodId.Bolt11.rawValue)
+        val repo = paymentProofRepo()
+        repo.prepare(request, MethodId.Bolt11.rawValue, PaykitPaymentProofKind.Lightning).getOrThrow()
+        repo.associateLightningPayment(request, PAYMENT_HASH, MethodId.Bolt11.rawValue).getOrThrow()
+
+        assertTrue(repo.failLightningPayment(PAYMENT_HASH, NodeRunTimeoutError("payInvoice")))
+
+        assertTrue(storedProofs.isEmpty())
+        repo.prepare(request, MethodId.Bolt11.rawValue, PaykitPaymentProofKind.Lightning).getOrThrow()
+        assertTrue(storedProofs.none { it.requestId == request.id && it.paymentStarted })
+    }
+
     @Test
     fun `onchain proof uses selected endpoint and transaction id`() = test {
         val txid = "ab".repeat(32)

@ben-kaufman
ben-kaufman requested a review from jvsena42 September 7, 2026 17:26

@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 at 60bbd450f. No HIGH/MEDIUM. One trivial parity nit inline; everything else I chased died on verification.

The iOS twin's rail-flip defect is absent here, and the reason is worth recording. On #685 the initial-subscription auto-pay can be dispatched on the Lightning rail and then broadcast on-chain with no swipe and no fee shown, because two validators check different amounts. Android doesn't have that pair. The rail is set once at :2906-2910 and corrected once at :2919-2923, and the correction uses the requested incomingPaymentRequest.amountSats, not the invoice amount — the zero-amount canSend at :2029 only decides whether a Lightning invoice is offered, never which rail is used. The bolt11 and LNURL insufficient-capacity paths abort (:3121-3134, :3176-3185) rather than falling back to on-chain. shouldAutomaticallyPay is decided once at :3431 exactly as on iOS, but only after navigateToSendRoute(SendRoute.Confirm) at :2966, so it already sees the final rail; the only post-navigation writer is the funding-source switch button, which isn't composed while the auto-pay spinner is up (SendConfirmScreen.kt:245), and handleSanityChecks never writes payMethod. So the balance dialog that provides the suspension window on iOS can't flip anything here.

No background payment path, which was my first check given a scheduler is involved. The scheduler is fed only by PaykitPaymentRequestRepo, whose synchronizeLocked sits behind isAvailable() and whose activate() is only called when isPaykitEnabled; turning the flag off runs clear()clearStateLocked()subscriptionNotificationScheduler.cancel(). The worker only posts a notification whose tap lands on the normal swipe-to-pay confirm.

Recurring-spend safety traced clean. acceptsPaymentAmount and acceptsLightningInvoiceAmountMsats are msat-exact and evaluated on every send, plus onScanLightning rejects a fixed-amount invoice that differs. Period caps hold: one request id per period keyed on billingPeriodStartsAt, periodsThrough only emits started periods with end > acceptedAt, the due queue excludes paid/locally-completed/in-flight/dismissed, prepare() throws OperationInProgress on any started proof, and submitReady dedups by exact billing period. Nothing auto-pays beyond the first Lightning period — isInitialSubscriptionPayment is only set by the accept/retry/failure paths, and a notification tap or external intent lands on the swipe screen. Cancellation refuses while a started proof exists and the scheduler diff cancels stale unique work names.

Migration is safe: every new persisted field has a default, a master-era proof decodes with paymentStarted=false and is still reconciled via paymentIdentifier, and corrupt blobs decode to empty rather than crashing.

All the earlier fixes verified correct, not merely present — the typed NodeNotRunningError/NodeRunTimeoutError classifier (with asNodeException() returning the AppError itself so the when actually matches), the pending-collector auto-dismiss guard, the null-identity fallback, dismiss-persisted-before-queue-mutation, the isCurrentState guard preceding every keychain write, resolution list+consume, the retry-hijack guard ordering, and the hardware sign/cancel window where the results collector completes the proof before onSendSuccess clears the context.

Three candidates I dropped rather than post: an unpinned auto-pay amount (real as a code fact, but the binding exposes no amend-terms API, so it's defensive against a protocol feature that doesn't exist — and it contradicts a verdict I already left on this PR); a claimed double-pay when proof prep fails (premise is wrong — associateLightningPayment creates a started proof when none exists, and completeOnchainPayment has the same fallback, so the period leaves the due queue); and a supposedly-dead subscriptionAcceptedAt (it's called via a :: method reference at SubscriptionsScreen.kt:122, so deleting it would break the build).

Comment thread app/src/main/java/to/bitkit/ui/screens/subscriptions/SubscriptionsScreen.kt Outdated
@ben-kaufman
ben-kaufman requested a review from jvsena42 September 7, 2026 18:11

@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 the delta at f0ec557c3. Single commit, one file, and it's correct — details on the thread. No HIGH/MEDIUM; nothing further from me on this PR.

jvsena42

This comment was marked as resolved.

@ben-kaufman
ben-kaufman requested a review from jvsena42 September 8, 2026 15:05
@ben-kaufman

Copy link
Copy Markdown
Contributor Author

@jvsena42 fixed

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

QA Notes

Pixel_10_Pro_XL emulator, Android 17 / API 37, dev flavor on regtest. All six Manual Tests passed, driven on 5a61869 immediately before the latest hardware-send and payment-request fixes landed.

The on-chain subscription payment settled for real: LDK broadcast c907aeeb13e5e68091af8744c337e7d6094bd0e795615addc0e3762f42916f0d paid 1000 sats to the payee's private p2wpkh endpoint, the success screen rendered orange confetti, and the completed-payment row showed a matching orange entry. The Lightning half of that same comparison did not settle on this regtest topology, because the wallet's only channel is with the Blocktank LSP while the test invoice came from a standalone lnd node, so no route exists; that selector's purple branch is unchanged on this branch and rendered correctly in an earlier run.

I read the commits since that run as code. The queued-versus-sent confirmation copy is total over the two-value PaykitPaymentRequestDeliveryStatus, the new BackHandler reuses the exact guard the on-screen back already applied, handleFailure now releases isBroadcastUnresolved on every failure path rather than one branch, and cancelling a hardware signature falls back to hiding the sheet when there is no back entry.

Approve.

Coverage

  • Journeys: 0%
    • No agent-journey suite covers the changed screens.
  • Unit tests: 2%
    • New HwSendViewModelTest regressions plus updated CreatePaymentRequestScreenTest.
  • QA: 6%
    • All six Manual Tests driven on emulator at the preceding head.
  • Total: 3%


Reviewed by Claude Code (claude-opus-5 high) via gh-pr-review-loop skill

@ovitrif
ovitrif dismissed their stale review September 8, 2026 17:34

addressed - reaudit confirmed

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

The duplicated amount display is fixed — 2311655a46 removes the right one. NumberPadTextField still renders the secondary amount exactly once via its default showSecondaryField = true, and the new instrumented test pins a single $/ node across the unit toggle. No over-correction. My earlier CHANGES_REQUESTED is resolved; all nine of my earlier threads plus the two later inline notes remain fixed at head.

One LOW inline, on the cancel path. Everything is behind isPaykitEnabled (default false, Dev Settings only) — isAvailable() gates synchronizeLocked, and turning the flag off runs clear() -> clearStateLocked() -> subscriptionNotificationScheduler.cancel(). Dev/QA-facing today, so not blocking, but the LOW ships unchanged the day the flag flips.

Delta checked hunk by hunk: e39a3d4637 selects queued vs sent description — both string resources exist, and request is the creation-time snapshot, so the copy won't flip if delivery completes while the screen is up. The IncomingPaymentRequestDetailsScreen scroll change is sound: the scrolled column contains no weight/FillHeight/lazy list (the Modifier.weight(1f) uses are RowScope weights inside a Row), and the Pay/Dismiss row sits outside the scroll region, so the buttons stay reachable with a 256-char note. The master merge only brought SendSheet.kt:326 into PR-touched files, which doesn't interact with the subscription retry guard.

Checked and clean — the fund-drain surface specifically:

  • Caps. Amount is fixed by SDK terms and copied verbatim per period, with msat-exact pinning before every send. End date is enforced in period generation with a hard MAX_PERIODS = 10_000, and one request id per period keyed on billingPeriod.startsAt.
  • Approver pinned (TOCTOU). takeIf { it == displayedSubscription } at the ViewModel, and again under the mutex in the repo with it == subscription && it.isProposalActionable(now). Data-class equality covers amountSats, recurrence and endpoints, so an amended proposal can't be approved against the displayed one.
  • Double payment. Period boundaries are computed purely in UTC from the payee anchor, so device timezone and DST changes can't shift billingPeriodStartsAt and mint a second id for the same period — that was my main worry going in. The due queue excludes paid, locally-completed, in-flight and dismissed; prepare() throws OperationInProgress on a started proof; and the initial-payment retry re-looks-up pendingRequest(id), so an in-flight first attempt can't be retried. The restoration fallback only ever moves acceptedAt later, never earlier, so no extra period can appear.
  • Cancellation. The SDK cancel is the durable step and runs first; local state then drops all unpaid periods, and the scheduler diff cancels stale unique work. A notification already posted before cancel is harmless — the tap only sets requestedPaymentRequestId, and presentation does a live pendingRequest() lookup that clears when absent. Nothing is reconstructed from intent extras.
  • Auto-start paths. The only one is the initial Lightning software payment, requiring isInitialSubscriptionPayment && LIGHTNING && hardwareWalletId == null; on-chain and hardware go through the normal fee/swipe screen. PIN-for-payments and biometrics are honoured on that path at the screen level, independent of the spinner branch, and PIN failure cancels the payment. A notification tap while locked is queued behind isPaymentRequestPresentationBlocked(), which includes !_isAuthenticated. Per-notification requestCode = id means two due notifications can't share a FLAG_ONE_SHOT PendingIntent and swap extras.
  • Proofs. Derived locally (txid from broadcast, or LDK payment hash) and submitted with the exact billing period. Paid periods come from the payer's own SDK record, so a counterparty marking a period paid could only reduce charges.
  • Trust boundaries. Issuer strings are bounded and clipped — note take(256), description 1024, benefits 8x160 — and rendered with maxLines/ellipsis. The Money.kt diff adds only a prefix/showSymbol passthrough and Tag.kt adds AddTagButton with a string resource, so no untrusted input reaches either. Amounts are clamped before display.
  • Concurrency. PaykitPaymentRequestPresentationStore does every read-modify-write under one Mutex, a corrupt blob decodes to empty, dismissSubscriptionPayment persists before mutating _pendingRequests, synchronizeLocked guards with isCurrentState before any write, and the scheduler is @Synchronized.

Cross-repo: the inline finding does not apply to synonymdev/bitkit-ios#685. Its SwipeButton runs onComplete in an unstructured Task { @MainActor in … } that survives an interactive dismiss, so the cancel completes and updates local state even if the sheet is swiped away. No need to reopen that one.

loading = loading,
onConfirm = {
loading = true
scope.launch {

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.

Swipe-to-cancel runs the repo call in the sheet's Compose scope with no dismiss guard, so dismissing mid-cancel leaves the SDK record CANCELED while the period stays due and payable for up to 120s.

scope here is rememberCoroutineScope() (:852), which dies with the composition. Trace: swipe -> loading = true -> scope.launch { onCancel() } -> PaykitPaymentRequestRepo.cancel -> updateSubscription -> paykitSdkService.cancelPaymentRequest (FFI under operationMutex, completes). While that's in flight the sheet is still dismissable — scrim tap, back, or drag — so hideSheet() nulls _currentSheet, SubscriptionSheet stops composing, and the scope is cancelled. processPendingMessages() then throws CancellationException (correctly rethrown by runSuspendCatching) before applySubscriptionRecordLocked and synchronizeAfterSubscriptionAction.

Result: the SDK has it CANCELED, but _subscriptions still shows Active and _pendingRequests still lists the current period as due — in the Payments tab, the bell, and pendingRequest(id) lookups — until the next poll, which is 30/60/120s. In that window "Pay" on the stale row goes through the full swipe flow (accept(request) for a billing period only checks pending) and submits a proof against a CANCELED request; "Cancel" again reaches the SDK a second time and toasts.

LOW because no sats move without a deliberate swipe and the view self-heals on the next poll. But it's the same shape as the dismissal-before-persist issue we already fixed, just on the cancel path — and acceptSubscriptionAndStartPayment already got the guard it needs via isAcceptingSubscription in ContentView.kt:502. Cancel just didn't get the same treatment.

Narrowest fix: add an isCancellingSubscription StateFlow set around cancelSubscription in AppViewModel and include it in dismissEnabled alongside isAcceptingSubscription — that also blocks the BackHandler and scrim in SheetHost. Alternatively run the repo call in viewModelScope and await() it from the sheet.

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.

Moved cancellation into the ViewModel scope so it finishes updating the subscription and due payments even if the sheet is dismissed.

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.

Confirmed fixed in c3769567b, via the second option — viewModelScope.async { … }.await(). The repo call now outlives the composition, so applySubscriptionRecordLocked and synchronizeAfterSubscriptionAction still run when the sheet is dismissed mid-cancel, while the caller keeps the result. viewModelScope uses a SupervisorJob, so a failed cancel can't take the scope down with it, and .onFailure(::toast) still reports. Test added.

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.

3 participants