Skip to content

fix: pause automatic payments after restore - #1214

Merged
ovitrif merged 7 commits into
masterfrom
fix/allowance-pause-after-restore
Nov 5, 2026
Merged

ovitrif merged 7 commits into
masterfrom
fix/allowance-pause-after-restore

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Closes #1216
Twin: synonymdev/bitkit-android#1779

This PR:

  1. Pauses automatic allowance payments after a wallet restore until the user resumes them
  2. Keeps requests manual when the matched allowance has a limit Bitkit does not check itself

Description

  • Restore pause: restoring a wallet backup now pauses automatic payments. The pause is kept on this device only, never in a backup, so it survives a relaunch and every restore starts it again, also when no allowance the wallet pays through existed at the restore. The pause is saved before any Paykit state is restored, also for a backup without payment state. While it lasts, a covered request asks for approval as an ordinary Payment Request, exactly like a ledger behind the wallet, and the limit check used while the node cannot pay yet hands it back without waiting. The restored journal, spends and ledger floor still count toward the limits.
  • Paused notice: the Allowances tab shows "Automatic payments paused" with "This wallet was restored from a backup. Requests ask for your approval until you resume." (subscriptions__allowances_paused_restore_description, added to every locale) when the wallet has a proposed or accepted allowance it pays through. When the ledger is also behind the wallet, the existing ledger explanation is shown instead. A wallet without such an allowance sees no notice.
  • Resume: the existing Resume, behind the same PIN check, now also ends the restore pause and keeps taking the ledger read now for the ledger case. Resuming the restore pause reads the ledger again first: a ledger that has fallen behind since is not accepted and the tab shows the ledger notice instead, and a ledger that cannot be read keeps payments paused and shows the existing error.
  • Limits Bitkit does not check: Bitkit checks the per-payment cap and one monthly amount itself. An allowance whose terms also set a lifetime amount, a payment count, another period or a second monthly limit never pays automatically. Both the early hand-back while the node cannot pay yet and the automatic payment measure the request against the same set, every allowance that covers the contact and has not ended here, and apply one rule: a request pays automatically only through the first candidate the SDK leaves unblocked whose allowance has Bitkit's own limits and fits it, even when another allowance also matches; when there is none, Limit Reached is raised only if no matching allowance fits at all, and otherwise the request asks without it, also when the fitting allowance is blocked by the SDK. An allowance with an End saved here counts on neither path, and allowances that all ended hand the request back without Limit Reached. Bitkit never proposes such terms, but a received proposal can; allowances with Bitkit's own terms keep paying automatically.

Out of Scope

  • Bitkit/Services/PaykitAllowanceExecutor.swift: the restore pause is one per device rather than per identity, so resuming with one Pubky identity also resumes for another identity used on the same device after the restore.
  • Bitkit/Services/PaykitAllowance.swift: a non-zero per-payment minimum is still left to the SDK's eligibility check rather than treated as a limit Bitkit does not check.
  • journeys/allowances/restore-keeps-history.xml: unchanged; after its restore the Allowances tab now also shows the paused notice, which none of its steps assert against. The allowances journeys README now says to tap Resume after a restoring journey.

Design

N/A — no design available.

Preview

QA Notes

Setup

  • Two Bitkit instances on regtest with Paykit UI enabled, saved as each other's contacts and linked, with the $5 a payment, $50 a month allowance from set-and-accept.xml Active, as in the allowances journeys README. The payer needs a usable channel after the restore and its notification permission granted.
  • The journey resets and restores the payer, so use disposable wallets.

Journeys

  • J1: new allowance-paused-after-restore.xml — after Reset And Restore the payer shows the restore notice with Resume, a covered request asks and is paid manually, the restore notice survives a relaunch, and Resume brings automatic payments back; same actions as the Android twin

Manual Tests

  • 1. Payer accepts an allowance asked for with a lifetime or payment count limit → the payee sends a request within the limits → it asks as an ordinary Payment Request — an allowance proposal with custom terms not in Capabilities

Automated Checks

  • added PaykitAllowanceExecutorTests.swift — restoring a wallet payload pauses automatic payments before its Paykit state is restored, also without payment state, and a pause that cannot be saved fails the restore; a restore pause keeps a covered request manual across a relaunch and Resume lets it pay; the limit check hands a request back while paused, or when only allowances with limits Bitkit does not check cover it, without Limit Reached; the manager shows the restore pause only for a proposed or accepted payer allowance and clears it on Resume; resuming the restore pause never accepts a ledger that fell behind and stays paused when the ledger cannot be read; the ledger explanation wins when both pauses apply; allowances with a lifetime, payment count or other period limit stay manual while Bitkit's own terms pay automatically; with a checked and an unchecked allowance for one contact, a request only the unchecked one fits asks without Limit Reached both in the early hand-back and in automatic payment, a request the checked one fits pays through it, and a request no allowance fits raises Limit Reached on both paths; with the checked allowance blocked by the SDK, the request asks without Limit Reached whether the unchecked candidate is unblocked or blocked for no eligible endpoint; a checked allowance that fits but the SDK blocks asks without Limit Reached; allowances that all ended here hand the request back without Limit Reached on both paths
  • added PaykitAllowanceTests.swift — records with limits Bitkit does not check are flagged and Bitkit's own terms are not; the restore pause stays across store reloads and a backup restore until cleared

Evidence

  • On commit 45377181, iPhone 17 simulator (iOS 26.2), Xcode 27.2: CI's unit test selection (xcodebuild test, skipping the integration suites as in unit-tests.yml) passed, BitkitTests 2992 executed with 1 skipped and BitkitUITests 44 executed with 4 skipped, 0 failures; SwiftFormat 0.64.0 lint is clean on the changed files; node scripts/validate-translations.js reports 0 errors.
  • On commit 54fc9a8e (review fixes), same simulator and Xcode: the same unit test selection passed, BitkitTests 2997 executed with 1 skipped and BitkitUITests 44 executed with 4 skipped, 0 failures; SwiftFormat 0.64.0 lint is clean on the changed Swift files; node scripts/validate-translations.js reports 0 errors; the journey's actions match the Android twin apart from ids, the reset and restore controls, relaunch commands and notification checks.
  • On commit 93a29bf5 (allowance selection rule), same simulator and Xcode: the same unit test selection passed, BitkitTests 3001 executed with 1 skipped and BitkitUITests 44 executed with 4 skipped, 0 failures; SwiftFormat 0.64.0 lint is clean on the changed Swift files; node scripts/validate-translations.js reports 0 errors. The four new tests were checked to fail with the rule reverted.
  • On commit 91d9cabf (matching set, SDK order and ended allowances), same simulator and Xcode: the same unit test selection passed, BitkitTests 3003 executed with 1 skipped and BitkitUITests 44 executed with 4 skipped, 0 failures; SwiftFormat 0.64.0 lint is clean on the changed Swift files. The new tests were checked to fail with the previous selection restored.

Gaps

  • J1 and 1 were not run: no two-instance regtest setup with an Active allowance was available for this change.

Models used

  • Planning/scoping: Claude Opus 5.5 (reasoning: Not exposed)
  • Implementation: Claude Opus 5.5 (reasoning: Not exposed)
  • Review: Claude Opus 5.5 (reasoning: Not exposed)
  • Review rounds: 4

The session metadata does not expose a reasoning setting.

🤖 Generated with Claude Code

ben-kaufman and others added 6 commits November 4, 2026 13:17
Restoring a wallet backup now pauses automatic allowance payments until
the user resumes them. The pause is kept on this device only, so a
relaunch keeps it and every restore starts it again. While paused, a
covered request asks for approval as an ordinary Payment Request,
exactly like a ledger behind this wallet. The Allowances tab shows the
paused notice with a restore explanation to a wallet that pays through
an allowance, preferring the ledger explanation when both apply, and
Resume behind the existing PIN check ends both pauses. The restored
journal, spends and ledger floor still count toward the limits.

Automatic payment also hands a request back to the manual flow when the
matched allowance has a limit Bitkit does not check itself: a lifetime
amount, a payment count, another period or a second monthly limit.
Allowances with Bitkit's own per-payment and monthly terms keep paying
automatically.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restoring a wallet payload now pauses automatic payments through a
helper that takes the allowance store, before any Paykit state comes
back and also for a payload without payment state. The pause is kept
per device, so it applies even when no allowance this wallet pays
through existed at the restore.

The restore notice shows only for a proposed or accepted allowance this
wallet pays through. Resuming the restore pause reads the ledger again
first: a ledger that has fallen behind since is not accepted and shows
its own notice, and a ledger that cannot be read keeps payments paused.
While the node cannot pay yet, a request covered only by allowances with
limits Bitkit does not check is handed back right away.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The restore pause journey now follows the shared actions: the paused
request is paid from the sheet, relaunch and Resume are checked as
separate steps, and the restore text and Resume button are verified.
The allowances README notes that a restoring journey leaves automatic
payments paused until Resume.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A contact can have an allowance with only the per-payment cap and the
monthly amount Bitkit checks, and another with further limits. Both the
early hand-back while the node cannot pay and the automatic payment now
apply one rule: a request pays automatically only through a checked
allowance that fits it, even when another allowance also matches. When
no checked allowance fits but another matching allowance fits within
the cap and monthly amount, the request asks without Limit Reached.
Limit Reached is raised only when no matching allowance fits at all.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Automatic payment now measures the request against every allowance
that covers the contact and has not ended here, the same set as the
early hand-back while the node cannot pay. It pays through the first
candidate the SDK leaves unblocked whose allowance has only the limits
Bitkit checks and fits the request. When there is none, Limit Reached
is raised only if no matching allowance fits at all; otherwise the
request asks without it, also when the fitting allowance is blocked.
An allowance with an End saved here counts on neither path, and
allowances that all ended hand the request back without Limit Reached.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ben-kaufman
ben-kaufman marked this pull request as ready for review November 5, 2026 00:34
@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Critical impact] The PR appears safe to merge; no actionable issues were found.

Summary

Adds a device-local pause for automatic allowance payments after a wallet backup is restored.

  • Restored wallets keep allowance payments manual until the payer resumes them.
  • Only allowances with limits Bitkit checks can pay requests automatically.

Acknowledged scope from ben-kaufman: the pause is shared across identities on this device; nonzero per-payment minimums remain checked by the SDK; the existing restore-history journey is unchanged, with the README explaining when to tap Resume.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Restore wallet payload] --> B[Save device-local pause]
  B --> C[Restore payment state]
  C --> D[Covered requests ask for approval]
  D --> E[User taps Resume]
  E --> F{Ledger behind?}
  F -->|Yes| G[Keep paused and show ledger notice]
  F -->|Read fails| H[Keep paused and show error]
  F -->|No| I[Accept current ledger and clear pause]
  I --> J{Fitting allowance with checked limits and unblocked SDK candidate?}
  J -->|Yes| K[Continue automatic payment]
  J -->|No| D
Loading

Reviews (1) · Last reviewed commit: "chore: rename changelog fragment" · Reviewed by Greptile

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Placed in 3.2.0: a non-blocking fix found after the 3.1 cut. I filed #1216 to track it under the milestone and added Closes #1216 to this PR. The twin synonymdev/bitkit-android#1779 closes synonymdev/bitkit-android#1781.

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

No findings. iOS twin of synonymdev/bitkit-android#1779: pauses automatic allowance payments after a wallet restore until Resume, and keeps requests manual under allowances with limits Bitkit does not check. Reviewed 7ce2cd4, full tier, static: iOS does not build on this reviewer's Linux host, reasoned from the code and CI.

What I checked, and 7 candidates I ruled out

Read: PaykitAllowanceExecutor.swift, PaykitAllowanceManager.swift, PaykitAllowance.swift, BackupService.swift, Keychain.swift, AppReset.swift, AppScene.swift (restore and incoming-request paths), AllowancesView.swift, every Localizable.strings; compared against the Android twin
Call sites traced: processIncomingRequests to autoPay / exceedsLimits, acceptPaymentRequestAutomatically and reserveAutomaticPayment (only admitAndPay), performFullRestoreFromLatestBackup including the pending-restore retry
CI: SwiftFormat and validate green at review time; unit and integration test jobs still running

Ruled out

  • Pause bypass: autoPay checks the pause before admitAndPay, the only caller of the SDK accept and reserve calls; recover() starts no payment.
  • Restore without the pause: fresh and retained payloads both reach restorePaykitPaymentState, which saves the pause first; a failed save throws inside the WALLET category, which is fatal, so the restore stays pending rather than continuing unpaused.
  • Read failure: an unreadable pause counts as paused, as on Android.
  • Wipe and reinstall: wipeEntireKeychain removes the item, it is not in the backup snapshot, it is device-only and not synchronizable, and the orphaned-keychain check wipes a leftover on a fresh install.
  • Resume ordering: a behind ledger keeps the pause and shows the ledger notice, an unreadable one throws, otherwise the ledger is accepted before the pause is cleared, so a failed clear leaves a retryable state.
  • Unchecked-limit rule: hasUncheckedLimits gives the same result as Android's for zero, one monthly, monthly with a count, non-monthly and multiple limits; Limit Reached only fires when nothing fits.
  • Translations: the key is in all 15 .lproj files with valid syntax.

Merge confidence: 4/5, no findings, but the unit test job was still running and J1 has not been run on a simulator or device.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the review session's metadata does not report a reasoning setting)

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

Code scan at 7ce2cd41d: no HIGH, MEDIUM or LOW. Reviewed as a funds change: it gates automatic Paykit payments.

Checked and clean:

  • Nothing pays before Resume. autoPay checks the pause synchronously inside the actor, before inFlightRequestIds.insert and before any await. admitAndPay is reached only from autoPay, and acceptPaymentRequestAutomatically/reserveAutomaticPayment only from admitAndPay. recover() only records outcomes. The notification and widget targets hold no allowance code.
  • Restore order: performRestore(.wallet) retains the payload, then restorePaykitPaymentState writes the pause before the allowance local state and long before the SDK state restore. A payload with no payment state still writes the pause.
  • If setPausedAfterRestore throws, the WALLET category fails as fatal and the retained payload stays pending. Wallet start and the Paykit poll are blocked meanwhile, and relaunch retries.
  • The keychain item is AfterFirstUnlockThisDeviceOnly and non-synchronizable, and not part of backupSnapshot()/restoreBackup. A read failure counts as paused. Deleting a missing item does not throw.
  • The wallet wipe and the orphaned-keychain check cannot clear the pause on a live restored wallet.
  • Resume runs checkedIsLedgerBehind first: if behind, the ledger notice shows and the pause stays; if unreadable, it throws. Then acceptCurrentLedger, resumeAfterRestore. It sits behind authorization.authorize and isWorking.
  • After Resume, a request the pause handed over stays manual for the rest of the session but auto-pays after a relaunch, because manualRequestIds is in memory. That matches the copy ("until you resume"), the PR's own testAWalletRestoreKeepsCoveredRequestsManualAcrossARelaunchUntilResumed, and the ledgerBehind pause on master, so it is not a finding.
  • The hasUncheckedLimits/fit() rule: an allowance with no cap never auto-pays, SDK order wins, and Limit Reached shows only on .none.
  • The new key exists in all 15 .lproj files.
  • Android twin synonymdev/bitkit-android#1779: same gate, fail-closed read, Resume order and notice gating.

perf: not relevant (one keychain read per covered request per pass).

Device gate (partial): 7ce2cd41d — not run yet; allowance-paused-after-restore.xml pending on two simulators with an Active allowance. The device result follows in a second review.

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

Device gate at 7ce2cd41d. Two iPhone 17 simulators, staging regtest, linked as Paykit contacts. Inbound for the payee came from a host LDK node with Manual Setup channels, since Blocktank no longer sells channels.

  • Precondition PASS. The $5/$50 allowance was set and the payee accepted it on its own.
  • allowance-paused-after-restore steps 1–7 PASS. Reset and restore landed on "Wallet restored" in about 87 s, with balances and the channel intact.
  • (a) PASS. AllowancesPaused shows "Automatic payments paused" with the restore copy and AllowancesResume, and the row stays "Active · $5 a payment".
  • (b) PASS. A covered $2 request while paused opened the Payment Request sheet in the foreground and posted no notification. The log shows Kept an incoming request manual: automatic payments are paused since the wallet was restored.
  • Step 11 PASS. Paid manually: Bitcoin Sent, $1.99 (2,494 sats).
  • (c) PASS. The paused notice came back after two terminate+launch relaunches.
  • Resume PASS. The notice was gone 35 s after the tap.
  • (d) PASS. After resume, a covered $2 request was paid on its own: ended as started, 2,508 sats sent, paykit_allowance notification shown, no Swipe. ⚠️ The banner copy itself was not captured on screen. iOS doesn't keep the foreground banner in Notification Center and the log redacts it.

One LOW inline on the journey.

Not from this PR, seen during the gate: a just-restored payer couldn't pay the contact until the payee relaunched, because the payee's private payment list stopped republishing. Reset Wallet also leaves the session without an app log until the next launch. After a relaunch, the Allowances list briefly shows an empty or payee-shaped row until the Pubky session restores. Payments stay paused during that window, since the executor reads the keychain flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ben-kaufman
ben-kaufman requested a review from jvsena42 November 5, 2026 19:24

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

Re-approving at e2e6af309. The only change since my approval is the journey fix for my LOW: steps 10-11 now accept the confirm view. PaymentRequestConfirm (SendConfirmationView.swift:432) and PaymentRequestFor (PaymentRequestSummary.swift:23) both exist. It matches the Android twin. No app code changed.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed in session metadata)

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman Merging: @jvsena42 approved the head e2e6af30 after a device run of the restore-pause journey at 7ce2cd41; the only commit since changes journey files, no app code. CI is green and no review thread is open.

@ovitrif
ovitrif merged commit 2d5744b into master Nov 5, 2026
20 checks passed
@ovitrif
ovitrif deleted the fix/allowance-pause-after-restore branch November 5, 2026 21:08
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.

bug: automatic payments continue after restore

4 participants