Skip to content

fix: pause automatic payments after restore - #1779

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

jvsena42 merged 11 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 #1781
Twin: synonymdev/bitkit-ios#1214

This PR:

  1. Pauses automatic Allowance payments after a wallet restore until the user resumes them
  2. Keeps requests under an Allowance whose terms carry limits Bitkit does not check itself on the manual flow

Description

  • Pauses automatic payments when a wallet backup is restored and keeps that pause on the device across app restarts, so every request under an Allowance asks for approval until the user taps Resume. The restored journal, automatic spends and ledger floor still merge as before and still count toward the limits.
  • Shows the existing "Automatic payments paused" notice on the Allowances tab with "This wallet was restored from a backup. Requests ask for your approval until you resume." while the wallet pays through an Allowance; a ledger that is behind this wallet keeps its own description when both apply. Resume, behind the existing payment PIN check, clears the restore pause as well as accepting the current ledger. Translations are added for every locale.
  • Pays a request automatically only through an Allowance whose terms Bitkit checks itself (a per-payment cap and one monthly amount limit, as Bitkit creates) and that fits it, even when an Allowance with a lifetime total, a payment count, a non-monthly period or more than one monthly limit also matches. A request that fits only Allowances with such limits goes to the manual flow without Limit Reached, and Limit Reached is shown only when no matching Allowance fits at all. The same rule applies while the node cannot pay yet, so such a request asks at once instead of waiting.
  • Leaves Allowances this wallet has seen ended out of both checks, so a request whose covering Allowances have all ended asks at once, without Limit Reached, instead of waiting while the node cannot pay yet.
  • Adds the allowance-paused-after-restore.xml journey.

Out of Scope

  • repositories/PaykitAllowanceStore.kt: the restore pause covers every identity on the device and is cleared only by Resume; a wallet restored without any payer Allowance shows no notice, but an Allowance it creates later starts paused until Resume.
  • repositories/PaykitAllowance.kt: a per-payment minimum in received terms is checked by the Paykit SDK for each request and is not treated as an unchecked limit.
  • models/PaykitAllowanceStateBackup.kt: the shared backup format is unchanged; the pause is device-local and set by the restore itself.

Design

N/A — no design available. AllowancesScreen.kt is mapped to todo in docs/screens-map.md; the notice reuses the existing paused card with a new description.

Preview

IMAGE_1

QA Notes

Setup

  • Dev (regtest) build against the staging backend, as in journeys/README.md.
  • Two Bitkit instances saved as each other's contacts and linked over Paykit, set up as in journeys/allowances/README.md: the payer has a Lightning spending balance above 50,000 sats with a usable channel, the payee has receiving capacity, and both have notification permission granted. Start J1 with the $5 a payment, $50 a month allowance from set-and-accept.xml Active. J1 wipes and restores the payer, so use a disposable wallet.

Journeys

  • J1: new allowance-paused-after-restore.xml — after a restore, automatic payments stay paused across a relaunch with the restore notice, a request within the limits asks for approval, and Resume turns automatic payments back on

Manual Tests

  • 1. Payee asks the payer for an Allowance whose terms add a lifetime total or a payment count → payer accepts it on the review sheet → payee sends a one-time request within the per-payment and monthly limits → the request opens for approval and nothing is paid automatically — proposing custom Allowance terms not in Capabilities

Automated Checks

  • added PaykitAllowanceStoreTest.kt — the restore pause survives a reload until Resume, is kept in its own keychain entry apart from the backed-up allowance state, never rewrites state this build cannot read, and counts as paused when it cannot be read
  • added PaykitAllowanceExecutorTest.kt — a restored wallet keeps requests manual until resumed, the local capacity check hands requests back while paused or when every matching Allowance has unchecked limits, an Allowance with unchecked limits stays manual while a Bitkit-style Allowance still pays automatically, and only the strict ledger check fails on an unreadable ledger; with a checked and an unchecked Allowance for one contact, a request only the unchecked one fits stays manual without Limit Reached both while the node cannot pay and when it can, a request the checked one fits pays through it, and a request neither fits raises Limit Reached once; a request whose covering Allowances have all ended stays manual without Limit Reached in both paths; and a checked Allowance the SDK blocks stays manual without Limit Reached when it fits locally
  • added PaykitAllowanceRepoTest.kt — the restore pause shows only while the wallet pays an Allowance, the ledger-behind reason wins when both apply, Resume ends the restore pause, Resume from the restore notice keeps a ledger behind this wallet paused with its own notice, and an unreadable ledger fails Resume without accepting it
  • added PaykitAllowanceTest.kt — terms with a lifetime, count or other period limit are flagged unchecked; a per-payment cap with one monthly amount limit is not
  • added BackupRepoTest.kt — a wallet restore pauses automatic payments before it restores allowance or Paykit SDK state, also without any allowance state
  • added AllowancesScreenTest.kt — the paused notice shows the restore or ledger-behind description for its reason
  • updated AllowancesViewModelTest.kt — follows the pause reason exposed by the repository

Evidence

  • Commit 357faa866: just compile, just test (4,877 unit tests, 0 failures) and just lint (detekt and Android Lint, 0 issues) passed locally on macOS.
  • Each new test for the separate restore pause entry, the pause before restoring wallet state, the ledger check on Resume, the unchecked-limit handback, the checked-allowance payment rule and the ended-allowance handback while the node cannot pay was also run without its fix and failed. The ended-allowance and SDK-blocked cases with the node able to pay already held before and are covered as regressions.
  • Earlier commits: 2301247ff passed the full gate (4,874 unit tests); 52d99e108 passed the full gate (4,870 unit tests); c56662a16 passed the full gate (4,865 unit tests); f4b560aa5 (pause only) passed compileDevDebugKotlin and the Allowance, backup and Allowances UI unit tests.
  • J1 and manual test 1 were not run.

Gaps

  • J1 and manual test 1 were not run on a device: no emulators or test identities were used 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

Reasoning effort is not exposed in the session metadata available to the implementation agent.

🤖 Generated with Claude Code

ben-kaufman and others added 10 commits November 4, 2026 13:09
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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from f99874a (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@ben-kaufman
ben-kaufman marked this pull request as ready for review November 5, 2026 00:33
@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

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

Summary

Automatic allowance payments now stay paused after a wallet restore until the user resumes them.

  • Restored wallets keep automatic Allowance payments paused until the user resumes them.
  • Requests under limits Bitkit cannot check now ask for approval.

No actionable issues were found. Tests and the device journey were not run during this review.

Acknowledged scope: ben-kaufman states that the restore pause covers every identity, can affect allowances created later, leaves per-payment minimum checks to the SDK, and does not change the shared backup format. These choices were not reported as defects.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Restore wallet backup] --> B[Save local restore pause]
    B --> C[Restore allowance and SDK state]
    C --> D[Incoming request]
    D --> E{Automatic payments paused?}
    E -->|Yes| F[Ask for approval]
    E -->|No| G{Checked allowance fits and SDK permits?}
    G -->|Yes| H[Pay automatically]
    G -->|No| F
    I[User taps Resume] --> J[Check and accept ledger]
    J --> K[Clear restore pause]
    K --> 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 #1781 to track it under the milestone and added Closes #1781 to this PR. The twin synonymdev/bitkit-ios#1214 closes synonymdev/bitkit-ios#1216.

@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. Pauses automatic Allowance payments after a wallet restore until Resume, and keeps requests manual under Allowances whose terms carry limits Bitkit does not check. Reviewed 28a9601, full tier, static: the branch was not built here (Bitkit-track policy), reasoned from the code and CI.

What I checked, and 6 candidates I ruled out

Read: PaykitAllowanceExecutor.kt (classifyAboveLimit, autoPay, admitAndPay, recover), PaykitAllowanceRepo.kt, PaykitAllowanceStore.kt, PaykitAllowance.kt (fromRecord, PaykitAllowanceCapacity.fits), BackupRepo.kt (performFullRestoreFromLatestBackup, restoreWalletBackup), Keychain.kt
Call sites traced: autoPay (only PaykitAllowanceRepo), acceptPaymentRequestAutomatically / reserveAutomaticPayment (only admitAndPay), restoreWalletBackup (fresh and retained-payload branches)
CI: lint, detekt, build green at review time; build-and-test and compose-ui still running

Ruled out

  • Pause bypass: autoPay checks the pause before admitAndPay, and admitAndPay is the only caller of the SDK accept and reserve calls; recover only settles existing attempts.
  • Restore entry without the pause: both the fresh WALLET restore and the retained PAYKIT_PENDING_BACKUP_RESTORE payload go through restoreWalletBackup, which writes the pause before any Paykit state.
  • Pause read failure: isPausedAfterRestore defaults to paused on a read error, fail closed.
  • Pause leaking past a wipe: Keychain.wipe clears the whole DataStore, so the new key goes with it; it is not part of the backed-up allowance state.
  • False Limit Reached on unchecked terms: classifyAboveLimit and admitAndPay notify only when fitting is empty; a request that fits only an unchecked Allowance returns manual without a notice.
  • Resume accepting a stale ledger: from the restore notice, isLedgerBehindOrThrow runs first, a behind ledger returns without clearing the pause, an unreadable one throws.

Merge confidence: 4/5, no findings, but CI's unit test job was still running and J1 has not been run on a 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 28a960106: no HIGH, MEDIUM or LOW. Reviewed as a funds change: it gates automatic Paykit payments.

Checked and clean:

  • Only one path pays automatically: acceptPaymentRequestAutomatically and reserveAutomaticPayment are called only from admitAndPay, and autoPay checks the pause before ensureReconciled. recover/recoverOpenAttempts only record outcomes. Subscription periods never auto-pay (billingPeriod == null).
  • Both restore entry points (a fresh WALLET payload, and the retained PAYKIT_PENDING_BACKUP_RESTORE payload) go through restoreWalletBackup, which writes the pause first. pubkyRepo.initialize() runs only after the full restore, so no identity session handles requests before the pause exists. If pauseAfterRestore() throws, the WALLET category fails, and the pending marker keeps hasPendingWalletRestore() true.
  • The pause has its own wallet-indexed keychain key. A read failure counts as paused. It is not in backupSnapshot() or WalletBackupV1, and Keychain.wipe() clears it only together with the wallet.
  • Resume from RESTORED runs isLedgerBehindOrThrow first: if the ledger is behind, the pause stays with the ledger notice; if the ledger is unreadable, Resume fails and the pause stays. Then acceptCurrentLedger, resumeAfterRestore, recover, refresh. A failed keychain delete leaves a paused state that can be retried. The UI goes through authorizeThen(isGated = true).
  • After Resume, fits applies the per-payment cap and the current monthly window at pay time, so a backlog cannot pay more than one period's cap. A request the pause handed over can auto-pay after Resume once the in-memory manual mark is dropped (relaunch). That matches the copy ("Requests ask for your approval until you resume") and the LEDGER_BEHIND behaviour on master, so it is not a finding.
  • hasUncheckedLimits flags lifetime limits, monthly counts, and non-monthly or extra periods. fits refuses an allowance with no cap. Limit Reached shows only when nothing fits.
  • The new key exists in all 16 values*/strings.xml.
  • iOS twin synonymdev/bitkit-ios#1214 has the same gate, the same fail-closed read, the same Resume order and the same notice gating.

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

Device gate (partial): 28a960106 — not run yet; allowance-paused-after-restore.xml pending on two linked emulators. The device result follows in a second review.

@jvsena42

jvsena42 commented Nov 5, 2026

Copy link
Copy Markdown
Member

Device gate (partial): 28a960106, allowance-paused-after-restore.xml steps 1-10 on two emulators (payer Pixel_9, payee Pixel_Tablet, staging regtest, times UTC). Still running: relaunch, Resume, and auto-pay after Resume.

  • Steps 1-7 PASS. The $5 / $50 allowance was Active, a WALLET backup was made after acceptance, then Reset And Restore from the phrase. The reset took about 85 s and the restore about 117 s; balances came back. The keychain write PAYKIT_ALLOWANCE_RESTORE_PAUSE happened at 11:19:11, during the restore.
  • Step 8 PASS. AllowancesPaused reads "Automatic payments paused" / "This wallet was restored from a backup. Requests ask for your approval until you resume." AllowancesResume is present, and the payee's row is Active ($5 a payment, $50 monthly).
  • Step 10 PASS. A $2.00 request with the note "Paused" sent at 11:23:41 was kept manual, and no "Payment Executed" notification was posted:
    11:24:33.459 Kept an incoming request manual: paused after a wallet restore (PaykitAllowanceExecutor.kt:627)
    11:24:33.460 Decided 'MANUAL' for an incoming request covered by an allowance
    
    The request's confirm screen opened directly (from Paee, for "Paused", $2.00, Swipe To Pay). From send to the sheet took 52 s, because of request polling plus sync.
  • Journey drift, not app behaviour: the request opened straight on its confirm screen, with no PaymentRequestsSheet list and no bell, so step 11's PaymentRequestPay-<id> does not exist on this route. The journey should say "the request opens for approval, on its confirm screen or in the sheet".

@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 result: no HIGH or MEDIUM, approving. One LOW inline, on the journey text. It can be fixed in this PR or moved to a follow-up issue, your choice; please reply on the thread saying which.

Device gate: 28a960106 — journeys: 2 run (set-and-accept.xml passed; allowance-paused-after-restore.xml steps 1-10 and 12-15 passed, step 11 drifted, step 17 partial); perf: Resume tap → notice gone 20 s, request sent → payer decision 52 s paused and 2 min 4 s after Resume, restore 117 s, no ANR or crash; Figma: not compared; l10n: 1 key, not checked on a small screen; iOS: synonymdev/bitkit-ios#1214 gate running.

Payer on Pixel_9, payee on Pixel_Tablet, staging regtest.

  • The paused notice reads exactly "Automatic payments paused" / "This wallet was restored from a backup. Requests ask for your approval until you resume." AllowancesResume is present (set through SecondaryButton(testTag = …)), and the allowance row stays Active. The pause key was written during the restore, at 11:19:11.
  • A $2 request while paused was kept manual, and no "Payment Executed" notification was posted:
    11:24:33.459 Kept an incoming request manual: paused after a wallet restore (PaykitAllowanceExecutor.kt:627)
    11:24:33.460 Decided 'MANUAL' for an incoming request covered by an allowance
    
  • After force-stop and relaunch the notice and Resume were still shown. For about 2 minutes after the relaunch, the tab showed the empty "Set up allowances" state while allowances loaded.
  • Resume: tap at 11:33:00.077, pause key deleted at 11:33:15.169, notice gone by 11:33:19.87. There is no master baseline, since master has no restore pause. Paykit operations were queueing 3-5 s each at the time, with lock contention left over from a snapshot boot.
  • ⚠️ Step 17 partial. After Resume, the next $2 request was admitted automatically:
    11:37:03.327 Handed an allowance payment to the node (PaykitAllowanceExecutor.kt:897)
    11:37:03.328 Decided 'STARTED' for an incoming request covered by an allowance
    11:37:06.409 LDK event fired: {"type":"PaymentFailed", … ROUTE_NOT_FOUND}
    
    The payment could not route, because the payee has no inbound Lightning capacity, so "Auto-pay sent $2.00" never fired. Staging Blocktank returns 404 for POST /channels, /cjit and /gift/order, so no inbound channel could be bought. The payee's own 300k Manual Setup channel has outbound capacity only. The PR's part (Resume turns automatic payments back on) is shown by the decision line. Delivery of the notification is not shown.

Skipped — covered by CI at 28a960106: unit tests, lint, local e2e (all green).

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 f99874a5d. The only change since my approval is the journey fix for my LOW: steps 10-11 now accept the confirm screen. PaymentRequestConfirm (SendConfirmScreen.kt:260) and PaymentRequestFor (PaymentRequestSummary.kt:62) both exist. No app code changed.

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

@jvsena42
jvsena42 merged commit 2ffe993 into master Nov 5, 2026
23 checks passed
@jvsena42
jvsena42 deleted the fix/allowance-pause-after-restore branch November 5, 2026 19:56
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