Skip to content

feat: add subscription requests - #736

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

feat: add subscription requests#736
ben-kaufman wants to merge 5 commits into
masterfrom
codex/paykit-subscription-requests

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

  • 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 proposals and pending from accepted subscriptions. Show creator subscribers/received payments, with delete/cancel but no edit.
  • Validate the wire-size limit before uploading/enqueueing, downsample icons, and count valid billing-period payments once without generating payer prompts for creators.

Discover, autopay and renewal UI are intentionally excluded. Icon hosting is public by design. Adds one changelog fragment.

Base: master, including the merged payer PR. No additional unmerged branch dependency.

Linked Issues/Tasks

Screenshot / Video

Recordings are silent at 4× speed using test wallets. All attached media was inspected and shows only Bitkit, including its keyboard; no device home screen, photo picker or other app is included.

Final keyboard and recipient behavior:

ios-keyboard-fix-4x.mp4
Final recipient and keyboard screenshots Final recipient field, expiry and contact spacing Name field with standard button clearance above keyboard Description scrolled into view with keyboard open
Earlier create → recipient → sent → overview walkthrough

This walkthrough predates the final keyboard and recipient sizing/spacing corrections. The recording and screenshots above show those final corrections.

ios-figma-audit-4x.mp4

QA Notes

Manual Tests

  • 1. Subscriptions → Create Subscription → Amount: enter a positive amount; set name and frequency; choose one privately linked contact and expiry → Propose Subscription: sent confirmation shows the recipient, amount and frequency.
  • 2. Create Subscription → upload custom icon → Choose Recipient → back: amount, name, description, frequency and icon remain intact; the receiver sees the same public icon.
  • 3. Receiver → Review & Subscribe → accept and manually pay: creator changes from pending to one subscriber/one received payment attributed to the payer; the creator does not get a payer payment prompt.
  • 4. Pending or accepted creator subscription → Delete → Swipe To Delete: cancellation reaches the receiver; existing payment history is retained; deleted creator proposal remains absent after restart.
  • 5. Send with unavailable connectivity or oversized metadata: no false Sent confirmation, useful error and retained draft; retry works after correcting the condition.
  • 6. regression: Payment Requests → Create Payment Request: shared amount, recipient selection and expiry behave as before.
  • 7. Create Subscription → focus Name and Description in either order: button keeps its standard keyboard clearance, wrapped description/caret stays visible, scrolling and keyboard dismissal work; Choose Recipient and Back preserve the draft while either field is focused.

Live regtest creation/delivery/acceptance/manual on-chain payment, public icon transfer and cancellation passed in both directions with Android. Latest-build confirmation, pending deletion and restart also passed. Offline draft retention/retry was tested on Android. Live Lightning, mainnet and production push were not tested.

Automated Checks

  • 156 focused XCTest cases passed after the final code change. PaykitSubscriptionProposalTests.swift covers UTF-8 wire boundaries, escaped strings, reserved icon space and image downsampling/errors. PaykitPaymentRequestServiceTests.swift covers creator lifecycle, validation before upload/enqueue and proof aggregation, including fractional billing instants.
  • Existing payment-proof, send-confirmation, currency and fiat-formatting suites passed. No automated coverage was removed.
  • Simulator E2E build passed with DEBUG E2E_BUILD, E2E_BACKEND=network, E2E_NETWORK=regtest; SwiftFormat lint passed for the touched Swift files. This was a focused test run, not the entire iOS suite.
  • Final local review found no remaining actionable issues. Keyboard behavior was verified interactively; the focused unit suite does not automate those interactions.

@greptile-apps

greptile-apps Bot commented Sep 9, 2026

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 2/5

This PR is not safe to merge until subscription creation avoids orphaned public icon uploads and canceled creator subscriptions retain an accessible payment history.

Findings

  1. P1 Security Icon Upload Is Nonatomic
  2. P1 Cancellation Hides Payment History
  3. P2 Security Remote Icons Lack Limits
  4. P2 Translations Contain English Placeholders

Summary

  • Builds recurring Paykit proposal terms with wire-size validation and icon downsampling.
  • Separates payer subscriptions from creator-owned proposals and accepted subscriptions.
  • Adds creator payment history, deletion controls, proposal confirmation states, assets, and localized strings.
  • Refactors shared amount, recipient, text-entry, image, and segmented-control components for the new flow.
  • The review identified non-atomic icon publication, inaccessible history after creator cancellation, unbounded remote icon decoding, and localization-workflow issues.

Diagram

sequenceDiagram
    participant Creator
    participant UI as Subscription UI
    participant Manager as Payment Request Manager
    participant Pubky
    participant Paykit
    participant Recipient

    Creator->>UI: Enter amount, metadata, frequency and recipient
    UI->>Manager: proposeSubscription(draft, target)
    Manager->>Manager: Validate expiry, endpoints and recipient
    opt Custom icon
        Manager->>Pubky: Upload public icon
        Pubky-->>Manager: icon_uri
    end
    Manager->>Paykit: Propose recurring payment request
    Paykit-->>Manager: Proposal record
    Manager->>Paykit: Process pending private messages
    alt Delivered
        Paykit-->>Recipient: Subscription proposal
        Manager-->>UI: Sent
    else Delivery pending
        Manager-->>UI: Queued
    end
    Recipient->>Paykit: Accept and submit payment proof
    Paykit-->>Manager: Active subscription and proof
    Manager-->>UI: Subscriber and received-payment history
    Creator->>Manager: Delete subscription
    Manager->>Paykit: Cancel recurring request
Loading

Comment on lines +593 to +598
let iconURI: String? = if let iconData = draft.iconData {
try await sdk.uploadProfileAvatar(
bytes: Self.compressedSubscriptionIcon(iconData),
contentType: "image/jpeg",
expectedIdentity: expectedIdentity
)

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 security Icon Upload Is Nonatomic

The icon is published before the final expiry check and proposal write. If the draft expires or proposePaymentRequest fails after the upload, the UI reports that no proposal was created but the selected image remains publicly hosted, with no rollback or cleanup path. The upload also separates the initial recipient and payment-rail checks from the final proposal, so unlinking the contact or disabling a rail during the upload can submit stale terms because the final SDK call rechecks only the identity. Revalidate eligibility and endpoint settings immediately before proposing, and remove an uploaded blob after failure or defer publication until the proposal can be committed.

How this was verified: The icon upload is an externally visible write performed before the final expiry check and proposal call, while the later proposal boundary validates the identity but not current recipient eligibility or endpoint settings.

Knowledge Base Used:

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.

Confirmed that a failed proposal can leave the icon public. The SDK reuses content-based avatar names and can fail after enqueueing, so deleting on every error could break a shared image or a queued proposal. This remains open, including the payment-option recheck after upload.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed. Because avatar names are content-addressed and an upload may already be referenced by a queued proposal, unconditional cleanup on failure is unsafe. The remaining actionable fix is to revalidate expiry, current recipient eligibility, and accepted payment endpoints immediately after the upload and before constructing/enqueueing the proposal; if any changed, fail without enqueueing rather than using the stale pre-upload snapshot. The orphaned-icon case likely needs an SDK-level ownership/cleanup mechanism to resolve safely, so I’m keeping this finding open for the post-upload validation.

Comment on lines +470 to +472
func isCreatedVisible(at date: Date) -> Bool {
isCreatedByUser && (isProposalVisible(at: date) || isActive(at: date))
}

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 Cancellation Hides Payment History

Canceling a creator-owned subscription makes its retained payment history unreachable. A canceled subscription is neither proposed nor active, so isCreatedVisible removes it from the Created section. The Expired section accepts only payer-side subscriptions, and creator payments are exposed only through receivedPaymentRequests() on the now-hidden detail screen. Deleting an accepted subscription with prior payments therefore removes the user's only route to those payments, contrary to the stated retained-history behavior. Keep canceled creator subscriptions with payments in a historical section or persist their payment rows into accessible history.

Knowledge Base Used: Payment request management

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.

Canceled or ended creator subscriptions with payments now appear in the Expired section, where their payment history remains accessible. Deleted proposals without payments still disappear.

.prefix(8)
.compactMap { Self.trimmed($0, limit: 160) }
iconURI = Self.trimmed(subscription["icon_uri"] as? String, limit: 512)
.flatMap { $0.hasPrefix("pubky://") ? $0 : nil }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 security Remote Icons Lack Limits

Incoming subscription metadata can reference any pubky:// resource, and the image loader downloads and fully decodes that resource without a byte or pixel limit. A malicious proposer can point icon_uri at an oversized or decompression-heavy public image; opening the subscription list then downloads it, decodes it with UIImage(data:), and caches the raw data, potentially causing excessive memory and storage use. Bind the URI to the proposal creator where appropriate and enforce download and decoded-dimension limits before constructing the image.

How this was verified: The proposal-controlled URI is accepted solely by its scheme, then fetched and decoded at full resolution without a size check before being cached.

Knowledge Base Used: Contacts and Pubky identity

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.

Confirmed that the loader has no download or decoded-image limit. The pinned SDK returns the full response before the app can inspect it, so an app-side size check alone would not cap the download. This remains open pending a bounded fetch path.

Comment on lines +1213 to +1238
"subscriptions__choose_recipient" = "Choose Recipient";
"subscriptions__create" = "Create";
"subscriptions__content_too_long" = "Shorten the subscription name or description and try again.";
"subscriptions__icon_error" = "Could not load this image. Choose another image and try again.";
"subscriptions__create_subscription" = "Create Subscription";
"subscriptions__created" = "Created";
"subscriptions__created_summary" = "1 subscriber · {count} payments";
"subscriptions__created_summary_single_payment" = "1 subscriber · 1 payment";
"subscriptions__custom_icon" = "Icon";
"subscriptions__custom_icon_description" = "Tap to upload a custom icon";
"subscriptions__delete_subscription" = "Delete Subscription";
"subscriptions__description" = "Description";
"subscriptions__description_placeholder" = "What is this subscription for?";
"subscriptions__name" = "Subscription Name";
"subscriptions__name_placeholder" = "Subscription name";
"subscriptions__pending" = "Pending";
"subscriptions__proposal_queued_description" = "Your subscription proposal is queued and will send automatically.";
"subscriptions__proposal_queued_headline" = "Queued\n<accent>Proposal</accent>";
"subscriptions__proposal_queued_title" = "Queued";
"subscriptions__proposal_queued_status" = "Proposal queued";
"subscriptions__proposal_sent_description" = "You have sent a subscription proposal to";
"subscriptions__proposal_sent_headline" = "Sent\n<accent>Proposal</accent>";
"subscriptions__proposal_sent_status" = "Proposal sent";
"subscriptions__propose_subscription" = "Propose Subscription";
"subscriptions__subscribers" = "Subscribers";
"subscriptions__swipe_to_delete" = "Swipe To Delete";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Translations Contain English Placeholders

The new subscription strings were copied as English values into every non-English localization file instead of going through the repository's translation workflow. Because validation checks key presence rather than translated content, these placeholders silently pass CI and become indistinguishable from completed translations. This leaves the entire flow untranslated and prevents missing-key warnings from tracking the work. Keep only the English source strings until genuine translations are pulled, or add actual translations for each locale. The same pattern appears in the Arabic, Catalan, Czech, German, Greek, Latin American Spanish, Spanish, Italian, Dutch, Polish, Brazilian Portuguese, Portuguese, and Russian localization files.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

Removed the new English placeholders from the non-English files. The flow uses the existing English fallback until translations arrive, and missing-translation warnings can track those keys again.

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.

1 participant