Skip to content

fix: pay first onchain subscription period in one swipe - #881

Open
jvsena42 wants to merge 9 commits into
fix/paykit-auth-figma-parityfrom
fix/subscription-onchain-single-swipe
Open

jvsena42 wants to merge 9 commits into
fix/paykit-auth-figma-parityfrom
fix/subscription-onchain-single-swipe

Conversation

@jvsena42

@jvsena42 jvsena42 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Stacked on #880. iOS port of synonymdev/bitkit-android#1422.

This PR makes accepting a subscription a single swipe when the first payment is paid on-chain, matching the Subscriptions flow in Figma and the existing Lightning behaviour, and brings the Review sheet closer to the frame.

Description

  • Before, an on-chain first payment took two swipes: the Review swipe only subscribed, then the send confirmation opened with a second Swipe To Subscribe & Pay. Figma shows one swipe from Review & Subscribe to Subscribed, and its flow notes do not limit that to Lightning.
  • SendConfirmationView.shouldAutomaticallyPay and requiresManualConfirmation no longer require Lightning funding, so the first payment due on acceptance starts by itself for on-chain savings too. A hardware wallet still opens the send confirmation, because the device has to sign.
  • While that first payment is sent, the sheet keeps the Review & Subscribe layout with the swipe in its completed, loading state, for Lightning and on-chain. Before, InitialSubscriptionPaymentProgress replaced it with a bare progress indicator; that remains only as the fallback when the subscription cannot be found.
  • The swipe on the Review sheet stays visible while the subscription is being accepted.
  • The swipe reads Swipe To Subscribe, as in Figma, whether or not a first payment is due.
  • The Review sheet no longer shows the First billing period ends … Each period is charged in full. line, which is not in the Figma frame.
  • The clock illustration is drawn at the frame's size and 15° rotation.
  • The card on the Review sheet shows the subscription name and one subtitle line, as in Figma. The contact name and truncated pubky lines are removed, along with the SubscriptionCounterparty identifier.
  • SwipeButton gains isConfirmed, which draws the knob at the end of the track for a swipe that was completed on a previous screen.
  • The automatic start waits for the send sheet to finish loading the fee rate and choosing the funding source, and for a calculated on-chain fee. Without a fee rate or a calculated fee it falls back to the manual confirmation, with its fee-rate retry button, instead of sending or failing, so the fee warnings always have a real fee to check and a hardware wallet picked for insufficient savings still gets its confirmation.
  • PIN or biometric confirmation for payments and the existing send warnings still run before the payment is sent.
  • The network fee for the first on-chain payment is no longer shown before it is paid.

Out of Scope

Design

Preview

iPhone 17 simulator on regtest, fresh wallet with on-chain savings only, proposal sent from an Android emulator wallet. 5,000 sats plus a 143 sat fee left the wallet with one swipe.

iOS and Figma

Recording of the single swipe on the current head, in real time: just under a minute from the swipe to Subscribed on the simulator. It took about three minutes before the Paykit runtime updates.

ios_subscription_single_swipe_v4.mp4

QA Notes

Journeys

  • updated review-and-subscribe.xml — with the first period due on acceptance and on-chain savings only, one swipe keeps the Review and Subscribe layout with the swipe loading and ends on Subscribed with no second swipe
  • updated cancellation-during-confirmation.xml — the first period is now paid on acceptance, so the journey advances the subscription clock and cancels while the next unpaid period is on the send confirmation; not driven on a device

Manual Tests

N/A

Automated Checks

  • updated SendConfirmationViewTests.swift — an automatic payment requires manual confirmation only for hardware funding
  • added SendConfirmationViewTests.swift — an automatic payment lacks a fee only on-chain with no calculated fee
  • ran SendConfirmationViewTests, swiftformat --lint and node scripts/validate-translations.js — pass locally

@jvsena42
jvsena42 force-pushed the fix/subscription-onchain-single-swipe branch from fbc5a5b to e69f9ed Compare October 8, 2026 09:11
@jvsena42
jvsena42 marked this pull request as ready for review October 8, 2026 09:37
@greptile-apps

greptile-apps Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

[Medium risk] Changes subscription payment flow for on-chain wallets.

Automatic payment should wait for fee calculation and funding selection before this PR merges.

Findings

  1. P1 High-fee warnings can be skipped ▶
  2. P1 Payment outruns wallet selection ▶

Summary

The PR starts the first on-chain subscription payment after the acceptance swipe and keeps the review layout visible while it sends. It also adds a completed position to SwipeButton and updates the labels, journey, and test.

  • Automatic payment needs to wait for a real fee before checking fee warnings.
  • It also needs to wait for funding selection before deciding whether hardware confirmation is required.

Intentional or deferred items acknowledged by jvsena42: the fee is hidden before payment, the billing-period text is removed to match Figma, and later periods remain manual because neither platform has an auto-pay setting yet.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Accept subscription] --> B[Open send confirmation]
  B --> C[Load fee and choose funding wallet]
  B --> D[Start automatic payment]
  C --> E[Calculate transaction fee]
  D --> F[Check warnings and payment PIN]
  F --> G{Hardware active?}
  G -->|Yes| H[Show manual confirmation]
  G -->|No| I[Send from savings]
  E -. Must finish before warnings .-> F
  C -. Must finish before automatic start .-> D
Loading

Reviews (1) · Last reviewed commit: "chore: add subscription single swipe cha..." · Reviewed by Greptile

Comment thread Bitkit/Views/Wallets/Send/SendConfirmationView.swift
Comment thread Bitkit/Views/Wallets/Send/SendConfirmationView.swift

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

No findings. iOS port of synonymdev/bitkit-android#1422: shouldAutomaticallyPay and requiresManualConfirmation no longer require Lightning funding, and InitialSubscriptionPaymentProgress now renders the shared SubscriptionReviewContent with a confirmed swipe. Reviewed e69f9ed, full tier, reasoned from the code and CI: iOS does not build on this box.

What I checked, and 4 candidates I ruled out

Read in full: InitialSubscriptionPaymentProgress.swift, SwipeButton.swift, the new SubscriptionReviewContent and the review route in SubscriptionsView.swift, the automatic-payment path of SendConfirmationView.swift (startAutomaticPaymentIfNeeded, submitPayment, performPayment)
CI: Greptile and change detection green; Run Tests and build-local were still pending at review time

Ruled out

  • On-chain auto-pay with no fee rate: submitPayment calls wallet.setFeeRate when isFeeRateMissing, and a failure there throws into the .failure route of startAutomaticPaymentIfNeeded, so it never sends at an unset rate.
  • Hardware wallet paying without the device: shouldAutomaticallyPay still excludes hwSend.isActive, and requiresManualConfirmation returns true for a hardware payment, as SendConfirmationViewTests asserts.
  • Missing environment for the new @Environment(PaykitPaymentRequestManager.self): both hosts, SendConfirmationView and LnurlPayConfirm, already read the same environment object, so it is in scope wherever the progress view is shown.
  • isConfirmed leaking into other swipes: it defaults to false and only SubscriptionReviewContent sets it, from isPaying.

Merge confidence: 4/5, no findings, but Run Tests and build-local were still pending and the branch was not built here.

@jvsena42 jvsena42 self-assigned this Oct 8, 2026
@jvsena42
jvsena42 added this pull request to stack #895 October 8, 2026 10:33
@jvsena42
jvsena42 force-pushed the fix/subscription-onchain-single-swipe branch from a58b830 to 8ef8bc1 Compare October 8, 2026 11:40
@jvsena42
jvsena42 requested review from a team, ben-kaufman and coreyphillips and removed request for a team October 8, 2026 11:53
@coreyphillips

Copy link
Copy Markdown
Contributor

Two independent reviews, nothing blocking a merge.

worth doing, does not block

  • Missing fee rate on-chain fails the first payment instead of falling back (Bitkit/Views/Wallets/Send/SendConfirmationView.swift:666). When the fee rate cannot be loaded, an automatic on-chain first payment lands on the First Payment Failed screen. The PR says it should fall back to the manual confirmation. submitPayment(isAutomatic:) (SendConfirmationView.swift:664-675) calls try await wallet.setFeeRate(...) whenever isFeeRateMissing is true. That call comes before the new automaticPaymentLacksFee check, and setFeeRate throws AppError("Fees unavailable from bitkit-core") when there are no fresh or cached estimates (WalletViewModel.swift:648). This is exactly the state after loadFeeRateWithRetry gives up in SendSheet.setUpSend. The throw reaches startAutomaticPaymentIfNeeded, which pushes .failure. The user is already subscribed and is told the first payment failed. Before this PR, on-chain went to the manual confirmation, which shows the SendConfirmRetryFeeRate button. I confirmed this by reading the code path, not on a device. A fix is to check isAutomatic && isFeeRateMissing and call showManualConfirmation() before the setFeeRate call.
  • Review sheet no longer shows who the subscription is from before a one-swipe payment (Bitkit/Views/Subscriptions/SubscriptionsView.swift:991). After this PR, the Review and Subscribe card shows only fields the payee chose. The title is subscription.note. The avatar is metadata.iconURI whenever the payee sets one (SubscriptionAvatar). The contact name and truncated pubky lines are gone, and the details screen does not show the counterparty either. Before this PR, an on-chain first payment still opened the send confirmation, which shows the contact as recipient. Now one swipe both accepts and pays, on Lightning or on-chain, and at no point is the payer shown who they are paying. This matches the Figma frame, so it may be intended. Still, a proposal with the note "Netflix" and a borrowed icon looks the same as a real one. I could not confirm whether Paykit only accepts proposals from established contacts, which is why this is not blocking. Consider keeping a counterparty line on the card or on the details screen.
  • Fee-rate outages bypass the manual retry screen (Bitkit/Views/Wallets/Send/SendConfirmationView.swift:666). An automatic on-chain first payment opens the failure screen when fee estimates are unavailable instead of exposing the existing manual retry. SendSheet catches the initial fee load error and clears isSetupPending, then this second setFeeRate attempt throws into startAutomaticPaymentIfNeeded, which appends .failure. Before this branch, the manual confirmation remained open with SendConfirmRetryFeeRate.
  • Journey does not force the on-chain payment path (journeys/subscriptions/review-and-subscribe.xml:17). The journey only requires an unspecified spendable balance, so it can select Lightning and pass without exercising the new on-chain single-swipe path. Require on-chain savings above the payment plus fee, with no usable Lightning balance, to make this journey cover the behavior claimed by the pull request.

nits

  • Changelog fragment is not named after the PR number (changelog.d/next/subscription-onchain-single-swipe.changed.md). The fragment is named changelog.d/next/subscription-onchain-single-swipe.changed.md. AGENTS.md asks for <issue-or-pr>.<category>.md, which here would be 881.changed.md.
  • Journey still describes a creator contact card on the review sheet (journeys/subscriptions/review-and-subscribe.xml:22). review-and-subscribe.xml still says "Verify the Review and Subscribe sheet appears with 5,000 sats and the creator contact card". The card now shows only the subscription name and cadence, so the step should describe the subscription card.

coreyphillips
coreyphillips previously approved these changes Oct 8, 2026
@jvsena42
jvsena42 force-pushed the fix/subscription-onchain-single-swipe branch from 8ef8bc1 to d3a9962 Compare October 8, 2026 16:08
@jvsena42

jvsena42 commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Went through the review.

  • Missing fee rate fails the first payment (both entries): fixed in dae43af. An automatic payment with no fee rate opens the manual confirmation, with SendConfirmRetryFeeRate, before setFeeRate is called. No unit test was added for this branch, and the outage was not reproduced on a device.
  • Journey does not force on-chain: fixed in d3a9962. review-and-subscribe.xml now requires on-chain savings above the payment plus fee and no usable Lightning balance.
  • Journey wording / changelog name: fixed in d3a9962 and c327c29. The step describes the subscription card; the fragment is 881.changed.md.
  • cancellation-during-confirmation.xml relied on the on-chain first payment stopping at the confirmation, as flagged on the Android PR. It is updated in d3a9962 to cancel on the next unpaid period. Not driven on a device.
  • Review sheet no longer shows the counterparty: not changed. The card follows the Figma frame, which has the subscription name and one subtitle line only. Paykit delivers proposals only from contacts both sides have saved, so the payer has already added the sender, but the review screen itself does not name them. Adding a counterparty line would diverge from the frame, so it is left as a design decision.

jvsena42 and others added 9 commits October 9, 2026 08:24
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jvsena42
jvsena42 force-pushed the fix/subscription-onchain-single-swipe branch from d3a9962 to 51e143a Compare October 9, 2026 11:26
@jvsena42
jvsena42 requested a review from talosmachina October 9, 2026 11:32

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

No findings. Re-review of the diff since e69f9ed: the branch was rebased onto the new base, and the four original commits are unchanged (git range-diff), so the delta is 1d5cffd9, 177782d8, 9935eb07 and the journey and changelog commits. Reviewed 51e143a, full tier, reasoned from the code and CI: iOS does not build on this box.

What I checked, and 4 candidates I ruled out

Read in full: setUpSend and cleanup in SendSheet.swift (the new isSetupPending), in SendConfirmationView.swift the .task and the new .onChange(of: isSetupPending), startAutomaticPaymentIfNeeded, submitPayment(isAutomatic:), settleTransactionFee, calculateTransactionFee, automaticPaymentLacksFee and showManualConfirmation, the SubscriptionProviderCard change, and both subscription journeys
Earlier findings: I had none. Greptile's two P1s on e69f9ed are fixed in the code. The automatic start now waits for isSetupPending to clear, which happens after selectHardwareFundingSourceIfNeeded. The fee warnings now run after settleTransactionFee(), and a zero on-chain fee falls back to manual confirmation
@coreyphillips's missing-fee-rate point: submitPayment now calls showManualConfirmation() before setFeeRate when isFeeRateMissing, so an outage no longer lands on First Payment Failed
CI: validate and change detection green; Run Tests, Run Integration Tests and build-local still pending at review time

Ruled out

  • Automatic payment re-firing after the manual fallback: showManualConfirmation sets requiresPaymentConfirmation, which makes shouldAutomaticallyPay false, so the isSetupPending change and later fee updates cannot restart it.
  • isSetupPending stuck true: the defer skips the reset only when the task was cancelled. Both cancel sites either start a new task that sets it again (setUpSend) or tear the sheet down (cleanup).
  • settleTransactionFee never converging: after three tries it returns with whatever fee it has, and a fee of 0 on-chain goes to manual confirmation rather than sending.
  • Hardware wallet picked after the decision: the .task sees isSetupPending and does not start; the onChange re-evaluates shouldAutomaticallyPay with hwSend already active and opens the manual confirmation.

Merge confidence: 4/5, no findings, but the fee-rate fallback has no unit test (the PR says so), the test and build jobs were still pending, and the branch was not built here.

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

QA review

Scope: Reviewed the full PR diff against its merge base, at 51e143ab. There was no saved baseline. Android bitkit-android#1422 at 02420758 was compared only for this first-subscription payment, not reviewed as a whole.

2 actionable findings — resolve or provide an evidence-backed rebuttal.

The fee-rate fallback and the wait for hardware selection when the payment is already on-chain both hold in this revision. Two gaps remain where that automatic path still does not match a manual send or the Android port: manual coin selection, and hardware selection after Lightning is deferred and later falls back to on-chain.

Run Tests, Run Integration Tests, and build-local passed on this revision. This review inspected SendConfirmationViewTests and did not run them locally. Device testing: not performed in this review.

Suggested additional test cases

  • iOS. Advanced coin selection set to Manual. On-chain savings cover a 5,000 sat subscription plus fee, with no usable Lightning balance. Accept a proposal whose first period is due. The coin picker opens, and First Payment Failed does not. After coins are chosen, the payment sends and Subscribed appears.
  • iOS. Lightning channels exist but none are usable. Hot savings are below the amount, and a paired hardware wallet can cover the amount plus fee. Accept the same proposal. The hardware send confirmation opens, and First Payment Failed does not. Retry while those channels stay unusable does not repeat the failure.
  • iOS. On-chain savings only, with fee estimates unavailable. Accept the proposal. The manual confirmation opens with SendConfirmRetryFeeRate, and First Payment Failed does not.

Findings

  • [MEDIUM] Manual coin selection ends on First Payment Failed — inline at Bitkit/Views/Wallets/Send/SendConfirmationView.swift:195.
  • [MEDIUM] Deferred Lightning fallback skips the hardware wallet — inline at Bitkit/Views/Wallets/Send/SendSheet.swift:384.


private var shouldAutomaticallyPay: Bool {
preparingRequest == nil && app.contactPaymentContext?.isInitialSubscriptionPayment == true && app.selectedWalletToPayFrom == .lightning &&
preparingRequest == nil && app.contactPaymentContext?.isInitialSubscriptionPayment == true &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM] Manual coin selection ends on First Payment Failed

Trigger: accept a subscription whose first period is paid on-chain while Advanced coin selection is Manual and no coins are selected.

shouldAutomaticallyPay now includes that on-chain payment. submitPayment(isAutomatic:) still calls requireManualCoinSelection, which opens the UTXO picker and throws CancellationError so a manual swipe can reset. startAutomaticPaymentIfNeeded treats every CancellationError as a failed payment and pushes First Payment Failed on top of the picker. Retry starts the same automatic path, so it fails again until coin selection is changed or the period is paid later by hand.

The subscription is accepted and no transaction is broadcast. The payer cannot choose coins for this automatic payment.

Expected: open the coin picker and continue the payment after coins are chosen, as a manual on-chain send already does and as Android does at 02420758 (prepareCoinSelection navigates to coin selection and returns).

Evidence basis: source analysis at 51e143ab. SendConfirmationSwipeTests expects this cancellation to stop submission without authorizing payment; it does not cover the automatic subscription caller.

setupTask = Task {
defer {
if !Task.isCancelled {
isSetupPending = false

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[MEDIUM] Deferred Lightning fallback skips the hardware wallet

Trigger: accept a subscription while Lightning channels exist but none are usable, or while the node is not running yet. Hot savings cannot cover the amount, and a paired hardware wallet can.

Scan keeps the unified invoice on Lightning in those states. setUpSend runs selectHardwareFundingSourceIfNeeded only when the wallet is already on-chain, then clears isSetupPending. The later fallback in validatePaymentAfterSync switches to on-chain without selecting hardware. Automatic payment then sends from savings and lands on First Payment Failed. Retry scans the same way while channels stay unusable, so it repeats. The balance check that kept the sheet open already counted the hardware balance.

Expected: after falling back to on-chain, select a hardware wallet that can pay and show its confirmation, which is what happens when scan chooses on-chain before the sheet opens. Android at 02420758 switches the pay method and then calls selectHardwareFundingSourceForAmount before the automatic start.

Evidence basis: source analysis at 51e143ab. The new setup wait does cover hardware selection when the payment is already on-chain at setup.

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.

4 participants