feat: add subscription requests - #1239
Conversation
The PR should not merge until payment history for deleted creator subscriptions remains reachable. Findings
|
| val proposals = subscriptions.filter { it.isPayer && it.isProposalVisible(now) } | ||
| val active = subscriptions.filter { it.isPayer && it.isActive(now) } | ||
| val expired = subscriptions.filter { it.isPayer && it.isExpired(now) && acceptedAt(it.id) != null } | ||
| val created = subscriptions.filter { it.isCreatedVisible(now) } |
There was a problem hiding this comment.
Deletion hides payment history
Deleting a creator-owned subscription changes it to CANCELED, but this filter only exposes proposed or active creator subscriptions. The canceled record remains in repository state while disappearing from the only list that links to its detail screen. Because received payments are shown only on that detail screen and are excluded from global payment history, deleting a paid subscription makes its retained payment history inaccessible.
There was a problem hiding this comment.
Kept subscriptions with received payments in the Created list after deletion, so their payment history remains accessible. They show as expired, while canceled proposals without payments still disappear.
jvsena42
left a comment
There was a problem hiding this comment.
Reviewed for fund draining specifically. No critical or high findings.
This adds the creator/payee side and re-tags the existing payer logic with isPayer guards. Every path that moves sats is untouched and still needs a fresh user action per period (acceptSubscriptionAndStartPayment → send flow; due periods → openIncomingPaymentRequest → send flow). Nothing auto-pays, nothing pays a cancelled or dismissed period, and the persisted-before-cleared dismissal ordering from the earlier fix is intact (dismissSubscriptionPayment, :289-322).
I specifically checked for the three bugs confirmed on the earlier subscriptions work — none repeat: creation uses a callback rather than a StateFlow, and the proof/dismissal code is unchanged, so neither the proof-kept-forever wedge nor the queue-cleared-before-persist ordering is reintroduced.
Also verified clean: amount shown vs paid is still enforced by acceptsPaymentAmount; cross-identity is guarded by expectedIdentity/generation under operationMutex with the SDK re-checking identity in uploadProfileAvatar and proposePaymentRequest, and publishCreatedSubscription gating on isCurrentState; payee records can't leak into one-off history (toPaykitPaymentRequest:1188 rejects recurrence != null); the notification scheduler, monthly-cost, proposals and accept paths are all isPayer-filtered; runSuspendCatching throughout the new suspend paths; creationMutex.tryLock guards double-submit.
Three LOW notes inline, all dev/QA-facing today (isPaykitEnabled default false). Two of them also apply to the iOS twin (synonymdev/bitkit-ios#736).
| every = 1u, | ||
| unit = draft.frequency.rawValue, | ||
| startsAt = timestamp, | ||
| anchor = timestamp, |
There was a problem hiding this comment.
Low, disclosed, but worth a decision: anchoring the billing grid at proposal time lets the first billed period be arbitrarily short.
startsAt = anchor = proposal time, but the payer may accept any time up to proposalExpiresAt — and PaymentRequestExpiration goes up to 30 days. periodsThrough() bills the period containing acceptedAt in full, so with a Day plan and a Week expiry, accepting 6d23h later buys a period with an hour left, and the next full charge falls due an hour after that. Two consented payments within the hour.
It is disclosed — SubscriptionsScreen.kt:703-712 shows "First billing period ends {date}. Each period is charged in full." — and every charge needs its own consent, which is why this is LOW rather than higher. But this PR is the first Bitkit code that generates such terms, so it's the right place to decide.
Either set startsAt to the proposal's expiry so the grid begins after the acceptance window, or cap the expiry options relative to the billing unit (hide Month for monthly, Week/Month for weekly). A payer-side minimum-first-period rule would also work but is a cross-platform behaviour change.
Same line-for-line on iOS (PaykitPaymentRequestService.swift:648-653) — worth deciding once for both.
| PaykitSubscriptionMetadata(description, benefits) | ||
| }.getOrDefault(PaykitSubscriptionMetadata(null, emptyList())) | ||
| val iconUri = subscription["icon_uri"]?.jsonPrimitive?.contentOrNull | ||
| ?.clean(512) |
There was a problem hiding this comment.
Low: the only validation on the payee-supplied icon is the pubky:// prefix, so it can point at any key's blob.
The URI goes to Coil via SubscriptionAvatar → PubkyImage and renders in place of the counterparty's avatar on the review sheet, list rows and detail screen. A proposer setting icon_uri = pubky://<someone-else>/pub/bitkit.to/…avatar puts that person's picture on the subscription, and the payer's device fetches from that homeserver on every render.
After the prefix check, require the URI's host segment to match counterparty (PubkyPublicKeyFormat.matches(...)) and fall back to null — default icon or contact avatar — otherwise.
Same check on iOS (PaykitSubscription.swift, also prefix-only).
| contentType = "image/jpeg", | ||
| expectedIdentity = expectedIdentity, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Low: the public icon blob is uploaded before the proposal is committed, so a later failure leaves an orphaned world-readable file.
uploadProfileAvatar publishes under /pub/bitkit.to/… and runs before the post-upload validateProposalExpiration/validate(proposal) and before proposePaymentRequest. Any throw after this point leaves the blob on the homeserver, and each retry uploads another. Peer discovery succeeding but proposePaymentRequest failing on an identity check or transport is the ordinary way in.
It also holds operationMutex across a network upload, blocking refresh/dismiss/accept for the duration.
Reordering so the upload is the last fallible step before proposePaymentRequest fixes both, or delete the blob on failure (the SDK exposes path). Worth considering doing the upload outside the mutex and re-validating identity afterwards.
(For the record, reservedIconUri is sized against the staging namespace, which is longer than mainnet bitkit.to, so that preflight bound is conservative — not a bug.)
Description
This PR adds subscription proposals to contacts, building on the payer flow merged in #1186.
Discover, autopay and renewal UI are intentionally excluded. Icon hosting is public by design. Base is
master; no unmerged payer dependency remains.iOS counterpart: synonymdev/bitkit-ios#736.
Figma: Intro, Overview, Create Subscription, Choose Recipient, Sent Proposal.
Deferred custom-icon SDK work
Current Pubky image fetching buffers the complete file before passing it to the image loader. Upload-side downsampling does not bound downloads of images supplied by others.
When the forthcoming Paykit bounded-fetch and recoverable-publication support is integrated for iOS #736, include Android #1239 or its merged successor in the coordinated update and validation. This includes Kotlin bindings and Android decode/cache limits.
Shared-avatar handling and ambiguous proposal-publication outcomes remain SDK concerns. These limitations do not establish that Android currently deletes images unsafely. The current Android implementation and icons remain unchanged. The SDK integration is deferred to that follow-up.
Preview
Create → recipient → sent → overview using test wallets, silent at 4× speed. The first three seconds of the accelerated source clip were trimmed to exclude the system photo picker. All attached media was inspected and shows only Bitkit, including its keyboard; no device home screen or other app is included.
The walkthrough predates the final recipient corrections. The first screenshot below shows the final timer, Contacts spacing, Paste inset and 52dp field height.
android-pr-bitkit-only-4x.mp4
Final recipient, sent confirmation and overview screenshots
QA Notes
Manual Tests
Live regtest creation/delivery/acceptance/manual on-chain payments, custom icons and paid cancellation passed in both directions with iOS. Offline error/draft retention/retry and pending deletion also passed. Live transport-queued flushing, Lightning, mainnet and production push were not tested.
Automated Checks
ScreensMapTest.kt.PaykitSubscriptionProposalTest.ktcovers UTF-8/wire-size boundaries.PaykitPaymentRequestRepoSubscriptionTest.ktcovers creator terms, queued delivery, oversize rejection before icon upload/enqueue, pending cancellation and duplicate/off-schedule proofs. No automated coverage was removed.CreateSubscriptionScreenTest.kt,SubscriptionsScreenTest.ktand existingCreatePaymentRequestScreenTest.ktregression coverage. Includes all four frequency tabs, no-Discover empty state, compact 520dp confirmation layouts, loading state, single recipient, expiry, and truthful sent/queued copy.E2E=true E2E_BACKEND=networkand the dev flavor. Detekt retains its upstream advisory configuration; the cohesive shared subscription test fixture has a non-blocking LargeClass advisory, reviewed explicitly.