Skip to content

feat: add allowances for automatic paykit payments - #799

Open
ovitrif wants to merge 69 commits into
masterfrom
feat/paykit-allowances
Open

ovitrif wants to merge 69 commits into
masterfrom
feat/paykit-allowances

Conversation

@ovitrif

@ovitrif ovitrif commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #806
Twin: synonymdev/bitkit-android#1340
Refs:

This PR adds allowances: a payer sets a per-payment and a monthly limit for a Paykit contact, and that contact's requests within the limits are paid without asking.

Description

It builds on the Paykit SDK 0.1.0 that master pins, where an Allowance binds the two Pubky identities of a contact's single Encrypted Link and covers every Paykit app they use.

  • Adds an Allowances tab to Subscriptions, with an empty state, the list and a Set Allowance sheet with per-payment and monthly USD limits, so a payer can let a contact's requests pay themselves
  • Sends the allowance over the contact's existing private Paykit link and opens a review sheet on the receiver, who accepts or declines; either side can end an allowance from its detail sheet
  • Pays an incoming request within the limits automatically: the app claims the request, the SDK's allowance accounting reserves it before the send and the outcome is reported back afterwards, so a kill and relaunch mid-payment never pays twice
  • Keeps this device the owner of an auto-accepted request, so a payment that never reached the node comes back as an ordinary Payment Request after a restart
  • Keeps a request an allowance covers off the Send sheet until its automatic payment is decided; tapping Pay on it says it is already being paid instead of holding other Paykit sheets back, and a request whose manual sheet is already open is never paid automatically
  • Keeps a started automatic payment running when the caller that triggered it is cancelled, so a request that was accepted is not left unpaid
  • Leaves a request above the per-payment limit, over the monthly cap, or after an end as an ordinary Payment Request, with a Limit Reached notification for the payer and a Payment Executed notification for an automatic one; requests that arrived before the allowance stay manual
  • Attributes automatically paid activity to the contact, tags those rows "Auto-paid" in the Payments tab and shows the amount paid automatically on the allowance detail
  • Counts allowance months from the original anchor, so the monthly cap resets on the same day each month
  • Waits for the payee's next private payment list instead of asking the payer when the previous list was used up, and holds automatic payments until the node has a usable channel, because both cases fell back to manual on regtest; a request over the limits asks right away instead of waiting for the channel
  • Releases an automatic payment's reservation when its execution fails to start, and announces a payment restart recovery settles with the same notification and contact link
  • Translates the allowance strings into all 14 supported languages
  • Leaves wallets without an allowance ledger untouched at launch
  • Covers bitcoin-priced requests only, paid over a bitcoin rail even when the request also prices a USDT rail: a request priced in USD or USDT always asks, a grant names only bitcoin payment endpoints, and a manual payment over the USDT rail stays out of the allowance ledger
  • Keeps an automatic payment's allowance id on its pending proof through a wallet backup and restore
  • Pays automatically over Lightning only, through the request-bound resolution, the payment deadline check and the submission-error handling of the manual flow; a request the payee can take only on-chain asks, because a miner fee would fall outside the allowance's limits
  • Pays automatically only under terms with both a per-payment maximum and a monthly limit: as the payer, Bitkit drops a proposal without them, as Android does, and the capacity check fails closed
  • Caps an automatic payment's routing fee at 1% of the request, since routing fees fall outside the allowance's limits, and leaves a request manual when its endpoints carry no payment list version
  • Keeps a request whose creation time is unknown manual, since it may predate the allowance
  • Asks for the payment PIN or biometric check, when the user requires one for payments, before the payer saves, accepts or ends an allowance (as the Android twin does); the payee's answers never ask, and automatic payments never prompt because the checked grant authorises them. The step reuses the Send PIN screen and its copy, so it adds no strings

Out of Scope

Design

Figma "Bitkit - Experimental New": Allowances empty state, Set Allowance, Allowances list. The receiver's review sheet and the detail sheet have no frame and follow the drawn film. Deliberate differences: the first tab stays "Overview", the list row's right column reads the monthly limit, and the copy typos are fixed.

Preview

Set Allowance Allowances Detail Payments
Recording
auto-pay-clip.mp4

QA Notes

Journeys

At b005f8f the journeys ran cross-device against the Android twin synonymdev/bitkit-android#1340, on an iPhone 17 Pro simulator (iOS 26.5) and an Android 15 emulator on staging regtest; results and recordings are in the QA comment.

  • new set-and-accept.xml — the offer opens on the payee by itself and both rows turn Active; passed with iOS as payee and as payer
  • new auto-pay-under-limit.xml — a $2 request pays itself, the Payments row reads Auto-paid and the detail shows $2.00 paid automatically; passed with iOS as payee and as payer
  • new above-limit-asks.xml — a $20 request raises Limit Reached and arrives as an ordinary Payment Request, paid only by swipe and not tagged Auto-paid; passed with iOS as payee
  • new monthly-cap-reached.xml — the third $4 request on a $10 cap raises Limit Reached and stays an ordinary Payment Request, and the detail reads $8.00 paid automatically; passed with iOS as payee
  • new end-stops-auto-pay.xml — after either side ends it, both rows read Ended and the next request asks; both halves passed with iOS as payee, and the payer-ends half with iOS as payer
  • new pin-gates-allowance-changes.xml (ported from the Android twin) — the iOS payer's Save and End ask for the payment PIN, Back leaves the allowance unchanged, and the payee's Accept never asks; passed on two iOS simulators (QA), and with Android as payer in the cross-device run
  • new restart-never-pays-twice.xml — the payer was killed 0.4 s after the hand-off and relaunched: one payment left the wallet and the payee received one; ran on two iOS 26.5 simulators before the master sync, against Paykit 0.1.0-rc59

Manual Tests

N/A

Automated Checks

  • added PaykitAllowanceTests.swift — limits, terms, capacity, month anchoring, grouping and admission decisions
  • added PaykitAllowanceExecutorTests.swift — reserve, hand-off, claim, payment list outcomes, cancellation of the caller, outcome recording, deferral, channel readiness, granting over a ready link and restart recovery of automatic payments, Lightning-only automatic payment, uncertain Lightning submission errors, a settlement during the send, uncapped terms, the routing-fee cap, an unknown request creation time, an unsaved sending stage, the per-payment limit notification, requests over the limits while a channel reconnects, an execution startup error, an acceptance intent the manual flow saved, and announcing payments recovery settles
  • updated PaykitPaymentProofServiceTests.swift — payment proofs carry the allowance id
  • updated PaykitPaymentStateBackupTests.swift — an automatic payment proof keeps its allowance id through backup, and older backups still restore
  • ran BitkitTests on an iOS 26.5 simulator against Paykit 0.1.0 after the master sync — all pass except SendConfirmationLifecycleTests.testCustomFeeWalletSwitchSurvivesFeeScreenNavigation, which fails the same way on master
  • ran the allowance and private Paykit tests on the current head — all pass

ovitrif added 15 commits October 1, 2026 19:05
An Allowance now binds two identities, so a grant is one SDK Allowance on the
contact's single Encrypted Link and coverage no longer depends on a receiver
folder. Automatic payments claim the request before the automatic Acceptance,
pass the payee's payment app on the proof and settle the private payment list
with the same outcomes the manual flow uses.
A cancelled caller no longer stops an automatic payment after the request was
accepted: admission runs in its own task once the ledger is reconciled.
The shared request flow only shows an accepted request again while this device
owns its acceptance. Automatic payments now record that ownership before the
automatic Acceptance, so a payment that never reached the node returns as an
ordinary Payment Request.
@ovitrif
ovitrif force-pushed the feat/paykit-allowances branch from 1a0051f to 8f0449c Compare October 1, 2026 19:58
@ovitrif
ovitrif changed the base branch from feat/demo-clock-subscriptions to codex/paykit-shared-runtime-local-20260930 October 1, 2026 19:59
@ovitrif

ovitrif commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

I restacked this PR on #856 and pushed 8f0449c (draft, base codex/paykit-shared-runtime-local-20260930).

  • Rebased onto feat: share paykit state across apps #856 and dropped the demo clock commit, so the diff over feat: share paykit state across apps #856 is allowances only; the three tests that asserted allowances ignore the demo clock are gone
  • Removed the local ../paykit-rs reference and the rc55 state-layout shim; the build uses the Paykit 0.1.0-rc59 that feat: share paykit state across apps #856 resolves
  • Adapted allowances to the shared identity: one SDK Allowance per grant on the contact's single link, no receiver folders
  • Automatic payments now claim the request first, pass the payment app on the proof, settle the private payment list like the manual flow and keep this device as the request's owner
  • A cancelled caller no longer stops a payment after the request was accepted

Local verification: BitkitTests on an iOS 26.5 simulator at a593f0e: 1,611 tests pass, 12 skipped; the later merge of #856's head 61203c8 builds, with its test rerun pending. The device journeys and the Preview media wait for free disk on the simulator host.

…yment is decided

A request an allowance covers belongs to the automatic flow from the moment it
arrives. Before, a slow first pass let the incoming-request presentation open a
manual sheet for a request that was then paid automatically.
…owance flow owns

Skipping an owned request left it in the presentable list, so the presentation
re-entered itself on the main thread until the allowance pass finished. The pass
now logs how each covered request ended.
@ovitrif

ovitrif commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed a19ea87 with the device findings and the Preview media.

  • Fixed the request presentation re-entering itself on the main thread whenever the allowance flow owned a request, which froze the payer's UI until the automatic payment finished; found on the simulators
  • A covered request now stays off the Send sheet from the moment it arrives, until the automatic pass finds it manual or pays it; a manual sheet had opened for a request that was then paid automatically
  • Automatic payments record this device as the request's owner before the automatic Acceptance, so a payment that never reached the node comes back as an ordinary Payment Request
  • Merged the latest feat: share paykit state across apps #856 head (05c3234)

Device runs on two iOS 26.5 simulators against Paykit 0.1.0-rc59: set-and-accept.xml, auto-pay-under-limit.xml, restart-never-pays-twice.xml (killed 0.4 s after the hand-off, one payment out and one received) and the payee half of end-stops-auto-pay.xml pass. Local verification: BitkitTests 1,622 tests pass, 12 skipped, before the last #856 merge, which builds. The description has the screenshots and the recording.

@ovitrif

ovitrif commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

I ran the open checks on the current head (a19ea87), on two iOS 26.5 simulators against Paykit 0.1.0-rc59; no code changed.

  • BitkitTests: 1,626 tests pass, 12 skipped
  • above-limit-asks.xml: a $20 request arrived as an ordinary Payment Request, was paid by swipe, shows no Auto-paid tag and did not change the amount paid automatically
  • monthly-cap-reached.xml: on a $5 and $10 allowance, "Cap1" and "Cap2" ($4 each) paid themselves, "Cap3" stayed an ordinary Payment Request and the detail reads $8.00 paid automatically; the Limit Reached notification itself was not observable in the simulator
  • end-stops-auto-pay.xml, payer half: the payer's row reads "Ended · $8.01 paid automatically", the payee's reads Ended, and the next request waited as an ordinary Payment Request

The journeys are ticked in QA Notes; the manual test list is now N/A.

Tapping Pay on a request the allowance flow holds shows that it is already being paid, as on Android, and recovery announces only payments no settlement event announced.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips, I've addressed each point in fd4c701. The allowance, payment request and allowance executor unit tests pass on that head, including new regression tests for 1, 3, 4 and 6.

Channel reconnection hides requests that require manual approval

Fixed in da90209. While no channel is usable, each covered request is now checked against its contact's allowance limits before the wait. A request that fits none of them is handed back for manual payment, raises Limit Reached, and opens its Payment Request sheet. Requests within the limits still wait for the channel. testManagerHandsBackARequestAboveTheLimitsWhileWaitingForAChannel covers your 2,000-sat case.

Pay on a held covered request stalls all Paykit presentation

Fixed in da90209 and 7cb14fb. The skip branch now ends the requested presentation, so other sheets and proposals present again. A Pay tap on a held request shows "This request is already being paid." instead of a spinner. This matches the Android twin.

Request arriving with the acceptance opens a manual sheet, then auto-pays

Fixed both halves in da90209:

  • The automatic pass now skips the request whose manual sheet is open, so a payment you started by hand is never paid automatically underneath it.
  • A refused automatic acceptance drops the acceptance intent only when the executor added it, mirroring the manual flow's alreadySaved guard (testRefusedAcceptanceKeepsAnIntentTheManualFlowSaved).

Payments settled by restart recovery get no contact or notification

Fixed in da90209 and 7cb14fb. When recovery settles an automatic payment, it now raises the same Payment Executed event that links the contact and posts the notification. It does so only when no settlement event already announced that payment, so nothing fires twice (testRecoveryAnnouncesAnAutomaticPaymentItSettles).

Execution startup errors leave unsent attempts reserved

Fixed in da90209. An error from beginPaymentExecution now records the attempt as failed before the request returns to manual handling, since nothing left the wallet. That releases the reservation and the allowance capacity (testExecutionStartupErrorReleasesTheReservation).

New allowance strings are missing from the 14 non-English locales
Allowance screens and notifications lack translations

Fixed in fd4c701. All 47 allowance strings are now translated in all 14 locales. Each locale reuses its file's existing terms and tone, and keeps every placeholder. validate-translations.js reports none of them missing. A native speaker's check of the word chosen for "allowance" would still help, mainly in de, it, pl and ca.

On the Android review's point that a settled automatic payment was treated as the user's own send: iOS doesn't have that gap. The Send sheet and the payment toasts resolve only for payments the user sent from the foreground (pendingPaymentHashes) or QuickPay, and an automatic payment is neither.

@ovitrif
ovitrif requested a review from coreyphillips October 21, 2026 14:12
A reservation blocked for another reason, such as an ended allowance or a manual-only decision, sends the request to manual payment without the Limit Reached notification, as on Android.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 3e4c612, matching the Android twin synonymdev/bitkit-android#1340: Limit Reached now appears only when a reservation is blocked by an amount, count or period limit. A request blocked for another reason, such as an ended allowance or a manual-only decision, still goes to manual payment but without the Limit Reached notification. A new test covers it, and the allowance unit tests pass on this head.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rechecked after 4 new commits (da90209, 7cb14fb, fd4c701, 3e4c612). One point still blocks a merge.

addressed

  • Pay on a held covered request stalls all Paykit presentation (Bitkit/AppScene.swift:1590). Fixed: AppScene.swift:1596-1599 now toasts and calls releaseRequestedPresentation (PaykitPaymentRequestService.swift:2198-2204), which clears requestedPresentationId. That reopens requestsForPresentation(), requestPresentation and presentNextItem, and stops the Pay spinner. I traced this but did not run it, since this machine has no Xcode. Other sheets come back on the next poll, because shouldPresentNextRequest stays false.
  • New allowance strings are missing from the 14 non-English locales (Bitkit/Resources/Localization/en.lproj/Localizable.strings). Fixed: All 47 subscriptions__allowance* keys are now in each of the 14 locales. validate-translations.js reports 0 errors and lists no missing allowance key. A script comparing {placeholder} sets against English found no mismatches, and plutil -lint passes on every file.
  • Execution startup errors leave unsent attempts reserved (Bitkit/Services/PaykitAllowanceExecutor.swift:597). Fixed: If beginPaymentExecution throws, PaykitAllowanceExecutor.swift:647-657 now records .failed before rethrowing. record reports to the SDK and moves the journal entry to .failed, and PaykitAllowanceCapacity.attempts only counts attempts whose status is not .failed, so the capacity is freed. Covered by testExecutionStartupErrorReleasesTheReservation.
  • Allowance screens and notifications lack translations (Bitkit/Resources/Localization/en.lproj/Localizable.strings:1778). Fixed: Same as point 4: all 14 locales now carry the 47 keys, with placeholders intact. I confirmed this with the validator, a placeholder comparison and plutil -lint.

still open

  • Request arriving with the acceptance opens a manual sheet, then auto-pays (Bitkit/AppScene.swift:1424). A request that arrives in the same intake as the payee's acceptance can open as a manual Payment Request and then be paid automatically underneath it. In refreshIncomingPaykitPaymentRequests, the payment request refresh receives the link's messages and updates pendingRequests. That fires the .onChange at AppScene.swift:716, which starts presentNextIncomingPaykitItem() in a Task. The Task can run while paykitAllowanceManager.refresh() is suspended at line 1424. In that case isAutomaticallyHandling checks coversRequest against allowances read before the intake, where the allowance is still proposed. The Send sheet opens. processIncomingRequests then sees the refreshed allowance and auto-pays the same request. This happens when the payer's app was closed while the payee accepted and sent a request. No second payment goes out: the consumed list version, the proof and the ledger each stop it. But the user's swipe fails. If the swipe wins the race instead, the executor's refused acceptance calls discardAcceptanceIntent. That removes the id the manual flow saved in the shared acceptance store. prepareForPayment guards against this with alreadySaved; the executor does not. This comes from code reading; I did not reproduce it. Codex still sees it: The intent ownership guard is fixed at Bitkit/Services/PaykitAllowanceExecutor.swift:600, but Bitkit/AppScene.swift:1426 only excludes sheets already open at that instant. An isolated scheduling test still opened a manual sheet during automatic execution when isAutomaticallyHandling suspended at Bitkit/Services/PaykitAllowanceManager.swift:328 across the allowance refresh.
  • Payments settled by restart recovery get no contact or notification (Bitkit/Services/PaykitAllowanceExecutor.swift:368). An automatic payment that restart recovery marks succeeded is never linked to the contact and raises no Payment Executed notification. recover(identity:) records outcomes through record(...), which only emits .ledgerChanged. The .paidAutomatically event drives both the notification and activity.setContact in handlePaykitAllowanceEvent. Only lightningPaymentSettled sends it, and that function returns early while activeIdentity is nil (line 689) and skips entries already at .succeeded. So after a kill and relaunch, the Activity row gets no contact if the node reports the payment's success before the allowance manager activates, or before recover reads the node. The Auto-paid label still shows because it reads the journal. Sending .paidAutomatically when recovery moves an automatic entry to succeeded would close the gap. Codex still sees it: Reconciliation at Bitkit/Services/PaykitAllowanceExecutor.swift:376 can settle an attempt before the recovery loop, which skips .succeeded at line 395. Running those recovery methods in isolation with that response produced no payment event and left the journal at .sent, so this path still misses the contact and notification.
  • Channel reconnection hides requests that require manual approval (Bitkit/Services/PaykitAllowanceManager.swift:290). An above-limit request cannot open its normal Payment Request sheet while channels are unusable. With a 1,000-sat allowance, a 2,000-sat request and canPayNow == false, I confirmed no eligibility check runs and isAutomaticallyHandling remains true. AppScene consequently suppresses presentation even after the user taps Pay. Determine which requests need manual approval before applying the channel wait. Codex still sees it: The preflight at Bitkit/Services/PaykitAllowanceExecutor.swift:426 still includes expired allowances. An isolated run with an active 1,000-sat grant, an expired 10,000-sat grant and a 2,000-sat request returned false from exceedsLimits, so Bitkit/Services/PaykitAllowanceManager.swift:293 still holds a request requiring manual approval.

new since the last review

worth doing, does not block

  • Recovery and a settlement event can both announce one automatic payment (Bitkit/Services/PaykitAllowanceExecutor.swift:389). Recovery and the node's settlement event can each announce the same payment, which posts two Payment Executed notifications. recover reads wasSettled from the journal at PaykitAllowanceExecutor.swift:389, then awaits record, which suspends inside sdk.recordPaymentOutcome before it updates the journal (:476-487). If lightningPaymentSettled runs during that suspension, it still sees the entry at .sent (:747-750), records it and sends .paidAutomatically (:754-761). Recovery then resumes and announces too (:392-393). This is the restart case: activate sets activeIdentity just before recover, and LDK replays the paymentSuccessful event around node start. Whether both fire depends on the SDK accepting a second identical outcome report; if it throws instead, the event path only logs a warning. I found this by reading the code and did not reproduce it. A cheap fix is to have record report whether its updateLocalState closure moved the entry to .succeeded, and announce only from the caller that made that move.
  • Recovery and settlement can notify twice for one payment (Bitkit/Services/PaykitAllowanceExecutor.swift:389). A settlement arriving during recovery can raise Payment Executed twice. wasSettled is captured before record suspends, so lightningPaymentSettled can also capture the unsettled journal entry and both paths announce it. I reproduced two .paidAutomatically events in isolation by pausing the outcome save and delivering settlement before it completed.

No manual request sheet opens while an allowance refresh or pass may still take the request, the limit check ignores expired allowances, and Payment Executed is raised once, from whichever path settles the journal entry, including payments reconciliation settled before recovery.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips, efe4aa2 addresses the three points still open from your recheck, plus the new double-announce finding. The allowance, payment request and allowance executor unit tests pass on that head.

Request arriving with the acceptance opens a manual sheet, then auto-pays

While an allowance refresh or automatic pass is running, the presentation loop now opens no sheet for a request from a contact this wallet grants, or was asked to grant, an allowance. That holds even after isAutomaticallyHandling has returned. The check runs synchronously right before the sheet is shown, so it does not depend on the stale read across the suspension. A request whose sheet opened before the pass started is still excluded from the pass, so neither ordering lets both flows run. The acceptance intent guard from da90209 stays.

Payments settled by restart recovery get no contact or notification

Payment Executed is now raised in one place: the journal update that moves an automatic entry to succeeded. Settlement, recovery, and attempts that reconciliation already settled all go through it. The recovery loop now also moves the journal for an attempt the SDK already shows as succeeded, so the case Codex ran (reconciled before the loop) links the contact and notifies (testRecoveryAnnouncesAPaymentReconciliationAlreadySettled).

Channel reconnection hides requests that require manual approval

exceedsLimits now counts only allowances active right now, so an expired grant no longer makes a request look within limits (testLimitCheckIgnoresAnExpiredAllowance, your 1,000-sat active, 10,000-sat expired case).

Recovery and a settlement event can both announce one automatic payment

Fixed with the same change: only the call that moves the entry to succeeded announces. The journal update runs inside the executor actor with no suspension, so a settlement and recovery can't both see the entry unsettled (testSettlementAndRecoveryAnnounceOnePaymentOnce).

@ovitrif
ovitrif requested a review from coreyphillips October 21, 2026 14:52
The journal keeps the list version an automatic attempt consumed, and recording the attempt failed releases it, after a restart too, so the manual payment that follows can use the same list. Pay on a request the allowance flow holds keeps the requests sheet open, as on Android.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 69da189, matching two changes on the Android twin synonymdev/bitkit-android#1340:

  • An automatic payment now records the payee's payment list version it consumed. When the attempt is recorded as failed, by a failed settlement or by restart recovery, that list is released, so the manual payment that follows can pay from the same list instead of waiting for a new one. The release happens only while that list is still the one consumed.
  • Tapping Pay in the requests sheet on a request the allowance flow is already paying keeps the sheet open and shows that it is already being paid.

Two new tests cover the release, and the allowance, payment request and private Paykit unit tests pass on this head.

…iled

Recovery moves the journal of attempts the SDK already settled as failed too, and releases the payee's list they consumed, as on Android.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 6f74f1a, matching the Android twin synonymdev/bitkit-android#1340. Restart recovery now also updates the journal for an attempt the SDK's reconciliation already settled as failed, and releases the payee's payment list it consumed. A new test covers it, and the allowance unit tests pass on this head.

coreyphillips
coreyphillips previously approved these changes Oct 21, 2026

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rechecked after 3 new commits (efe4aa2, 69da189, 6f74f1a). Nothing blocking remains.

addressed

  • Request arriving with the acceptance opens a manual sheet, then auto-pays (Bitkit/AppScene.swift:1424). Fixed: The guard at AppScene.swift:1604 runs synchronously right before showSheet at :1615, and updateDepth goes up before the first await in refresh() and processIncomingRequests (PaykitAllowanceManager.swift:149, :296), so no window is left open. A sheet opened before the pass is still excluded at AppScene.swift:1426, and recordedAcceptanceIntent (PaykitAllowanceExecutor.swift:630, :644) keeps the manual intent. Confirmed by reading; testManagerMayCoverAContactItWasAskedToGrant passes.
  • Payments settled by restart recovery get no contact or notification (Bitkit/Services/PaykitAllowanceExecutor.swift:368). Fixed: Recovery now records through record into applyJournalOutcome (PaykitAllowanceExecutor.swift:508-531), which sends .paidAutomatically when it moves an automatic entry to succeeded. The case reconciliation already settled is handled at :409-418. testRecoveryAnnouncesAPaymentReconciliationAlreadySettled passes on my run, with one announcement across two recoveries.
  • Channel reconnection hides requests that require manual approval (Bitkit/Services/PaykitAllowanceManager.swift:290). Fixed: processIncomingRequests hands over-limit requests back before the channel wait (PaykitAllowanceManager.swift:301-310). exceedsLimits now counts only allowances active at now() (PaykitAllowanceExecutor.swift:439). For a 1,000-sat active allowance and a 2,000-sat request, fits returns false at PaykitAllowance.swift:299, so the request gets its manual id and line 1451 presents it. testLimitCheckIgnoresAnExpiredAllowance passes.
  • Recovery and a settlement event can both announce one automatic payment (Bitkit/Services/PaykitAllowanceExecutor.swift:389). Fixed: wasSettled and announceAutomaticPayment are gone. The only .paidAutomatically send is in applyJournalOutcome (PaykitAllowanceExecutor.swift:525), whose check and stage write run in one synchronous closure on the actor. A settlement and recovery that both pass the SDK call can't both see the entry unsettled. testSettlementAndRecoveryAnnounceOnePaymentOnce passes.
  • Recovery and settlement can notify twice for one payment (Bitkit/Services/PaykitAllowanceExecutor.swift:389). Fixed: The same change answers this one: the decision to announce is made after the suspension inside sdk.recordPaymentOutcome, by the synchronous journal update at PaykitAllowanceExecutor.swift:512-529. The interleaving you reproduced now yields one event, because the second caller finds the entry already succeeded.

new since the last review

worth doing, does not block

  • Pay tapped during an allowance refresh spins until the next poll (Bitkit/AppScene.swift:1604). The new skip at AppScene.swift:1604 neither releases requestedPresentationId nor schedules a retry, so a tapped Pay can wait up to the next 10-second poll. The isAutomaticallyHandling branch above it does release. While requestedPresentationId is set, requestsForPresentation returns only that request, and presentNextItem skips subscription and allowance proposals. The Pay button keeps its spinner and every other Paykit sheet waits. Refreshes started by .ledgerChanged and .paidAutomatically (AppScene.swift:1952, :1969) have no presentation follow-up. finishManualPayment emits .ledgerChanged, so paying one request from a contact and then tapping Pay on the next can hit this. Recovery then waits for the poll to reach line 1451. mayCover matches ended, declined and expired allowances, none of which can start covering a request during a refresh. The skip also catches a request whose own Send sheet is already open and retrying, which line 1426 already keeps out of the pass. Found by reading, not reproduced. Two possible fixes: limit mayCover to proposed or accepted, unexpired allowances; or call presentNextIncomingPaykitItem() when updateDepth returns to zero.

nits

  • Recovery reads the Keychain journal once per settled attempt (Bitkit/Services/PaykitAllowanceExecutor.swift:412). The new .succeeded, .failed branch in recover calls localState(identity:) for every terminal attempt in the SDK history (PaykitAllowanceExecutor.swift:412). Each call is a Keychain read plus a JSON decode of every identity's state. The loop has no await, so on a wallet with a long ledger history it holds the executor actor at launch, and isHandling calls from the presentation path queue behind it. Loading the journal once before the loop and reloading only after applyJournalOutcome would avoid this.

A conversion only prices other rails, and the SDK requires a bitcoin rail to pay the bitcoin amount, so such a request is covered, as on Android. A manual payment made over the USDT rail stays out of the bitcoin ledger.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed 98824b6, matching the Android twin synonymdev/bitkit-android#1340. A bitcoin-priced request that also prices a USDT rail through a conversion is now covered by an allowance and pays automatically over a bitcoin rail. The SDK's execution checks require the bitcoin amount on a bitcoin rail, so a payment can't be counted at a converted amount. A manual payment of such a request made over the USDT rail stays out of the bitcoin ledger. The allowance and payment request unit tests pass on this head.

A request held back while allowances refresh is presented as soon as the update ends instead of at the next poll, only an allowance that could start covering the contact holds a request back, and recovery reads the journal once.
A list released when an automatic payment fails before broadcast is cleared from its journal entry first, so recording the attempt failed does not release it again after another payment consumed the same version.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips, thanks for the recheck. Your two non-blocking items and the finding from your #841 review are fixed in 4a1e391:

Pay tapped during an allowance refresh spins until the next poll

Fixed in 723cdf5:

  • mayCover now counts only an allowance that is proposed or active right now, so an ended, declined or expired one never holds a request back.
  • The presentation loop runs again as soon as the allowance update ends, so a held-back Pay presents then rather than at the next poll.

Recovery reads the Keychain journal once per settled attempt

Fixed in 723cdf5: recovery reads the journal once before the loop.

Failure cleanup can release another payment's list consumption (from your #841 review)

Fixed in 4a1e391. When an automatic payment fails before broadcast, its list version is cleared from the journal entry before the list is released, so recording the attempt as failed has nothing to release again. A durable release now happens only for an attempt whose list was never released: a settlement failure after an uncertain hand-off, or restart recovery. While that list version is still consumed, no other payment can consume it (testPaymentListReleasedBeforeBroadcastIsNeverReleasedAgain).

The allowance and private Paykit unit tests pass on this head.

Recovery's cached journal stages only skip settled attempts; moving an attempt to failed checks its current stage in the same journal update, so a settlement that ran while recovery waited is not released twice. The allowance payability comment matches the conversion change.
@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

I pushed d500fab for @coreyphillips's two notes on #799's code from his #841 approval:

  • Restart recovery's cached journal stages now only skip attempts that were already settled. Moving an attempt to failed checks its current stage in the same journal update, so a settlement that ran while recovery was waiting can't trigger a second list rollback (testRecoveryNeverReleasesAListASettlementReleasedWhileItWaited).
  • The isAllowancePayable doc comment now matches 98824b6: a conversion no longer keeps a request manual.

The allowance unit tests pass on this head.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rechecked after 4 new commits (98824b6, 723cdf5, 4a1e391, d500fab). Nothing blocking remains.

addressed

  • Pay tapped during an allowance refresh spins until the next poll (Bitkit/AppScene.swift:1604). Fixed: mayCover (PaykitAllowanceManager.swift:343-348) now matches only proposed or currently active allowances. updateDepth is observed (:71), and AppScene.swift:716-721 re-runs presentNextIncomingPaykitItem when isUpdating returns to false. With requestedPresentationId set, that goes straight to the request pass (AppScene.swift:208). The skip at :1610 is checked after the isHandling await, so a pass that resumes after the refresh ends proceeds, and one that skips is re-run on the transition. testManagerMayNotCoverAContactWhoseAllowanceEnded covers the narrowing.
  • Recovery reads the Keychain journal once per settled attempt (Bitkit/Services/PaykitAllowanceExecutor.swift:412). Fixed: recover builds journalStages once before the loop (PaykitAllowanceExecutor.swift:397-400). The .succeeded, .failed branch looks up that dictionary at :419 and reaches the store through applyJournalOutcome only for an entry that was still open.

new since the last review

worth doing, does not block

  • Settlement and recovery can each still release the same payment list (Bitkit/Services/PaykitAllowanceExecutor.swift:494). d500fab guards only recovery's .succeeded, .failed branch, so a failed attempt can still have its list released twice. record (PaykitAllowanceExecutor.swift:494-500) releases whenever the outcome is .failed. Without onlyIfOpen, applyJournalOutcome returns the entry even when it is already .failed. The SDK does not stop the second report either: in paykit-rs v0.1.0 history.rs, apply_outcome rejects only a change between terminal statuses, so reporting Failed twice succeeds. Two interleavings remain: 1. lightningPaymentSettled filters the journal (:836) -> awaits sdk.recordPaymentOutcome recovery moves the attempt to failed and releases settlement resumes -> applyJournalOutcome returns the entry -> releases again 2. recovery .submitted/.unknown: verifiedOutcome reads the stage -> awaits the Lightning lookup a settlement records failed and releases recovery record(.failed) -> releases again The second release undoes another payment's consumption only if that payment consumed the same version in between. That is the case raised on #841, and the author's reply says it can no longer happen. I confirmed this by reading the code and the SDK source; I did not reproduce it. One fix: in record, release only when this call moved the entry out of an open stage, as settledEntry already does for Payment Executed (for example, pass onlyIfOpen: true for .failed).
  • A bitcoin-priced request that accepts only USDT is now held for an allowance (Bitkit/Services/PaykitAllowanceManager.swift:176). After 98824b6, isAllowancePayable is just amount.asset == .btc (PaykitAllowance.swift:333-335), so coversRequest now takes a bitcoin-priced request whose only accepted endpoint is usdt-arbitrum-address. Bitkit can send such a request: CreatePaymentRequestView lets the payee deselect Savings and Spending. An allowance can never pay it. The SDK's match_allowance_request skips endpoints that carry a fixed conversion rate and returns NoEligibleEndpoint. Bitkit still holds it, though: - While canPayNow() is false, processIncomingRequests holds it (PaykitAllowanceManager.swift:301-309). - isAutomaticallyHandling keeps it off the Send sheet (AppScene.swift:1600). - Tapping Pay shows the "already being paid" toast. So a request payable over USDT, which needs no channel, waits on the Lightning node, possibly indefinitely if channels stay unusable. Over the limits, it also raises Limit Reached through exceedsLimits or :606. Before this commit, the conversion check kept it manual. One fix: have coversRequest require an allowed Lightning endpoint in request.acceptedPaymentEndpointIdentifiers. That would also stop on-chain-only requests from waiting on the channel. Found by reading the code and the SDK source; not run.

@ovitrif

ovitrif commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips, thanks for the approval. Your two non-blocking notes are tracked for 3.2.0 rather than pushed now, since a new head would make your approval stale:

Both are listed under Out of Scope.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rechecked after replies, with no new commit since d500fab. Nothing blocking remains.

still open

  • Settlement and recovery can each still release the same payment list (Bitkit/Services/PaykitAllowanceExecutor.swift:494). d500fab guards only recovery's .succeeded, .failed branch, so a failed attempt can still have its list released twice. record (PaykitAllowanceExecutor.swift:494-500) releases whenever the outcome is .failed. Without onlyIfOpen, applyJournalOutcome returns the entry even when it is already .failed. The SDK does not stop the second report either: in paykit-rs v0.1.0 history.rs, apply_outcome rejects only a change between terminal statuses, so reporting Failed twice succeeds. Two interleavings remain: 1. lightningPaymentSettled filters the journal (:836) -> awaits sdk.recordPaymentOutcome recovery moves the attempt to failed and releases settlement resumes -> applyJournalOutcome returns the entry -> releases again 2. recovery .submitted/.unknown: verifiedOutcome reads the stage -> awaits the Lightning lookup a settlement records failed and releases recovery record(.failed) -> releases again The second release undoes another payment's consumption only if that payment consumed the same version in between. That is the case raised on #841, and the author's reply says it can no longer happen. I confirmed this by reading the code and the SDK source; I did not reproduce it. One fix: in record, release only when this call moved the entry out of an open stage, as settledEntry already does for Payment Executed (for example, pass onlyIfOpen: true for .failed). HEAD is still d500fab, and record at PaykitAllowanceExecutor.swift:494-500 still releases on any .failed outcome without onlyIfOpen, so the double release stands. Your reply concedes the point and tracks it in #1089 (milestone 3.2.0, which describes both interleavings correctly). That is a valid deferral of a non-blocking point under the repo's policy, but it is not a fix or a rebuttal, so it stays open without holding the merge.
  • A bitcoin-priced request that accepts only USDT is now held for an allowance (Bitkit/Services/PaykitAllowanceManager.swift:176). After 98824b6, isAllowancePayable is just amount.asset == .btc (PaykitAllowance.swift:333-335), so coversRequest now takes a bitcoin-priced request whose only accepted endpoint is usdt-arbitrum-address. Bitkit can send such a request: CreatePaymentRequestView lets the payee deselect Savings and Spending. An allowance can never pay it. The SDK's match_allowance_request skips endpoints that carry a fixed conversion rate and returns NoEligibleEndpoint. Bitkit still holds it, though: - While canPayNow() is false, processIncomingRequests holds it (PaykitAllowanceManager.swift:301-309). - isAutomaticallyHandling keeps it off the Send sheet (AppScene.swift:1600). - Tapping Pay shows the "already being paid" toast. So a request payable over USDT, which needs no channel, waits on the Lightning node, possibly indefinitely if channels stay unusable. Over the limits, it also raises Limit Reached through exceedsLimits or :606. Before this commit, the conversion check kept it manual. One fix: have coversRequest require an allowed Lightning endpoint in request.acceptedPaymentEndpointIdentifiers. That would also stop on-chain-only requests from waiting on the channel. Found by reading the code and the SDK source; not run. isAllowancePayable is still just amount.asset == .btc (PaykitAllowance.swift:333-335), and coversRequest (PaykitAllowanceManager.swift:176) still ignores acceptedPaymentEndpointIdentifiers, so a USDT-only bitcoin request is still held. Your reply agrees and tracks it in #1090 (milestone 3.2.0, accurate description and suggested fix). As a deferral rather than a code change or a rebuttal, it stays open, non-blocking.

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.

feat: add allowances for automatic paykit payments

5 participants