Skip to content

feat: add subscription requests - #1239

Open
ben-kaufman wants to merge 7 commits into
masterfrom
codex/paykit-subscription-requests-android
Open

feat: add subscription requests#1239
ben-kaufman wants to merge 7 commits into
masterfrom
codex/paykit-subscription-requests-android

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Description

This PR adds subscription proposals to contacts, building on the payer flow merged in #1186.

  • Create a daily, weekly, monthly or yearly subscription with an amount, name, optional description and optional public Pubky icon.
  • Reuse Payment Request amount entry, recipient rows and expiry controls; send to one saved, privately linked contact.
  • Distinguish queued from sent and pending from accepted; show creator subscriber/received-payment counts and support pending/active deletion without editing.
  • Validate the wire-size limit before icon upload/enqueue, downsample selected images, and aggregate valid billing-period proofs without creating payer prompts for creators.
  • Keep the newly merged screen map in sync and add one user-facing changelog fragment.

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 Final recipient field, timer, Paste inset and Contacts spacing Subscription request sent confirmation Subscriptions overview after sending proposal

QA Notes

Manual Tests

  • 1. Subscriptions → Create Subscription → Amount: set a positive amount, name and frequency → Choose Recipient: only one saved/private-linked contact can be selected; expiry is configurable → Propose Subscription: confirmation shows recipient, amount and frequency.
  • 2. Create Subscription → upload custom icon → Choose Recipient → back: draft remains intact; the same public icon appears on the receiving device.
  • 3. Receiver → Review & Subscribe → accept and manually pay: creator shows one subscriber/one received payment attributed to the payer, without a payer payment prompt on the creator.
  • 4. Creator pending or accepted subscription → Delete → Swipe To Delete: request expires at the receiver and payment history remains available.
  • 5. Device offline → Propose Subscription: failure must not claim Sent; restore connectivity and retry the retained draft successfully.
  • 6. regression: Payment Requests → Create Payment Request: shared amount entry, contact selection and expiry remain functional.

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

  • Full suite: 2,430 unit tests passed with zero failures/skips on the final reviewed source, including ScreensMapTest.kt.
  • PaykitSubscriptionProposalTest.kt covers UTF-8/wire-size boundaries. PaykitPaymentRequestRepoSubscriptionTest.kt covers creator terms, queued delivery, oversize rejection before icon upload/enqueue, pending cancellation and duplicate/off-schedule proofs. No automated coverage was removed.
  • Nine Compose tests passed: CreateSubscriptionScreenTest.kt, SubscriptionsScreenTest.kt and existing CreatePaymentRequestScreenTest.kt regression coverage. Includes all four frequency tabs, no-Discover empty state, compact 520dp confirmation layouts, loading state, single recipient, expiry, and truthful sent/queued copy.
  • App and instrumentation builds plus detekt completed using E2E=true E2E_BACKEND=network and the dev flavor. Detekt retains its upstream advisory configuration; the cohesive shared subscription test fixture has a non-blocking LargeClass advisory, reviewed explicitly.
  • Final local review found no remaining actionable issues. The final UI changes reuse the existing Payment Request picker and shared subscription card; no changed-UI detekt findings remain.

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

The PR should not merge until payment history for deleted creator subscriptions remains reachable.

Findings

  1. P1 Deletion hides payment history

Summary

  • Extends Paykit proposal terms and repository models to support recurring creator records.
  • Adds the multi-step Compose creation and confirmation flow.
  • Separates payer and creator subscription processing, notifications, and payment presentation.
  • Adds wire-size, repository, and Compose coverage.
  • Creator payment history becomes unreachable after deleting a paid subscription.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Create subscription details] --> B[Choose saved private-linked contact]
    B --> C[Validate proposal wire size]
    C --> D{Custom icon?}
    D -- Yes --> E[Compress and upload public icon]
    D -- No --> F[Build recurring Paykit proposal]
    E --> F
    F --> G[Enqueue proposal]
    G --> H{Delivered immediately?}
    H -- Yes --> I[Sent confirmation]
    H -- No --> J[Queued confirmation]
    G --> K[Creator subscription record]
    K --> L[Pending or active creator list]
    L --> M[Subscription details]
    M --> N[Received-payment history]
    M --> O[Delete or cancel]
    O --> P[Canceled record retained]
    P -. currently filtered from UI .-> N
Loading

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) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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.

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

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,

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.

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)

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.

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 SubscriptionAvatarPubkyImage 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,
)
}

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.

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

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.

2 participants