[ovitrif]

Closes #1583
Stacked on #1707
Twin: synonymdev/bitkit-ios#1141

A restored wallet keeps its allowance journal and automatic-payment history: the allowance state now travels in the wallet backup.

Description

• Adds each identity's allowance state to the wallet backup as an optional allowances field of paykitPaymentState: allowance groups, the attempt journal, the lasting auto-paid totals and auto-paid request ids from fix: allowance follow-ups #1707, presented proposals, offers still to announce, notified requests and the trusted-time watermark.
• The wire format is shared with iOS (fix: allowance follow-ups bitkit-ios#1141): dates are RFC 3339 strings with milliseconds, journal stages ASCII lowercase under any device locale, and US dollar limits decimal strings, so a backup made on either app restores on the other. iOS ignores the auto-paid totals it does not keep (it derives them from the SDK ledger); on Android, each recovery pass folds every succeeded automatic attempt in the restored SDK ledger that the totals do not count yet back into them, so an iOS backup, a journal trimmed past 200 entries or a missing journal entry loses nothing. A stage this version does not know restores as unknown, which recovery settles from the node.
• Restore validates the allowance state with the payment proofs, before anything is written, and replaces each restored identity's state before recovery runs. Open attempts can then still be settled from the node instead of holding allowance capacity; a restored attempt the backup caught before its send is settled by the node when it knows the payment, so a payment that went out after the backup keeps its capacity used; such an attempt waits while the node cannot list payments yet, and recovery runs again once the node is running, and "Paid automatically" totals and the "Auto-paid" tag survive.
• The private Paykit backup also keeps which automatic attempt consumed each payee's payment list, as an optional field, so a restored attempt that failed can still give its list back. A backup without it restores with no owner.
• A change to backed-up allowance state asks for a wallet backup; a new trusted-time watermark alone does not, since it moves on every allowance check.
• No backup is written for a wallet without allowance state: the field stays absent.
• A backup fails, and is retried, while the stored allowance state cannot be read, so a corrupt local record never replaces a recoverable remote backup.

Out of Scope

None.

Design

N/A — no UI changes.

Preview

The allowances list, the auto-paid payments and an ended allowance's detail on a wallet restored from its recovery phrase (video at 2x).

b1708-restored-verify2x.mp4

QA Notes

Setup

Two dev (regtest) builds of this branch, set up as in journeys/allowances/README.md, with a backup service the wallet can restore from and the Active allowance from set-and-accept.xml.

Journeys

• J1: new restore-keeps-history.xml — after Reset And Restore, the payer keeps the allowance, its "Paid automatically" total and the "· Auto-paid" tag

Manual Tests

N/A

Retired: 1. (restore onto a fresh install), replaced by J1, since Reset And Restore on the same instance reads the allowance state back from the backup.

Automated Checks

• added PaykitAllowanceStateBackupTest.kt — allowance state round-trips through the shared wire format, pinned by a golden JSON; an unknown stage restores as unknown; stages are written in ASCII under a Turkish locale; a backup without lasting totals rebuilds them from its journal
• added PaykitAllowanceStoreTest.kt — only backed-up changes ask for a backup; unreadable state fails the snapshot; restore replaces each restored identity's state
• updated PrivatePaykitRepoTest.kt — a restored payment list can still be given back by the attempt that consumed it
• updated BackupRepoTest.kt — restore brings back each identity's allowance state
• updated PaykitAllowanceRepoTest.kt — a running node recovers attempts left open for it
• updated PaykitAllowanceExecutorTest.kt — recovery adds ledger successes the lasting totals miss, once; a restored attempt the node knows as paid settles as paid, and waits while the node cannot list payments

Evidence

• PaykitAllowanceStateBackupTest, PaykitAllowanceStoreTest, BackupRepoTest, PaykitPaymentStateBackupTest, PaykitAllowanceExecutorTest and PaykitAllowanceRepoTest pass on this head.
• 1. (retired) passed at f5f3c97 (the head before the last commit, which only lets a restore read entries that omit optional fields) on Android 16 emulators against a local regtest: after a wallet backup, a fresh install restored from the recovery phrase shows the same five allowances, the same "Paid automatically" totals ($7.97 and $3.19 at the live rate) and the same four "· Auto-paid" payments, with no attempt left open.

Gaps

• J1 has not been run yet; the run of 1. predates the ledger fold, the once-only count and the node check for restored attempts.

Models used

• Planning/scoping: claude-opus-5-5 (reasoning: medium)
• Implementation: claude-opus-5-5 (reasoning: medium)
• Review: Not performed

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-27T04:48:07.245658Z	24a46e6	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change reaches wallet backup and restore for people using allowances.

The PR appears safe to merge, with a non-blocking improvement needed to protect saved history when local allowance data is unreadable.

Findings

1.  Unreadable state erases saved history ▶

Summary

Adds each identity’s allowance state to wallet backups and restores it before identity startup.

• Wallet restores bring back allowance records and automatic-payment history.

_{Reviews (1) · Last reviewed commit: "fix: restore an allowance backup that om..." · Reviewed by Greptile}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24a46e61f4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Regtest APK

Built from 0f06291 (run).

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

[ovitrif]

Pushed da79536: the wallet backup's allowance snapshot now reads the stored state strictly and fails on state it cannot decode, so a corrupt local record never replaces the remote backup with one missing every identity's journal and history (#1708 (comment), #1708 (comment)). The changelog point is answered in its thread. PaykitAllowanceStoreTest, BackupRepoTest and detekt pass on this head.

[ben-kaufman]

Restoring an iOS backup after more than 200 allowance attempts loses older payments from the automatic-payment total and the Auto-paid labels. iOS trims its journal to 200 entries and omits the lasting totals and request IDs from its backup, so this reconstruction can only recover the retained entries. Android recovery also skips SDK attempts that have no journal entry. Could we rebuild these values from the full accounting history, or include complete totals and IDs in the shared backup, and cover an iOS restore with more than 200 attempts?

Resolved in 8c81493.

[talosmachina]

No findings. Puts each identity's allowance state (groups, journal, lasting auto-paid totals, presented, unannounced and notified ids, trusted-time watermark) into the wallet backup under paykitPaymentState.allowances, validates it before restore writes anything, and replaces the restored identities' state before recovery runs. Reviewed da7953617, full tier, stacked on #1707 (fix/allowance-follow-ups at 2a75d6d06), so this is the delta against that branch.

What I checked, and 7 candidates I ruled out

Read in full: PaykitAllowanceStateBackup.kt, PaykitAllowanceStore.kt, the restore path of BackupRepo.kt (performFullRestoreFromLatestBackup, restoreWalletBackup, uploadWalletBackup, markBackupFailed), the three test files.
Traced: withAutoPaid and PaykitAllowanceRepo.publish (where the lasting totals and the Auto-paid set are read), PaykitAllowanceRepo.activate and recoverOpenAttempts (what reads the restored journal), the twin's PaykitAllowanceStateBackup.swift, PaykitAllowanceKeychainStore.restoreBackup and PaykitAllowanceTime on synonymdev/bitkit-ios#1141.
Build: not built here. On the Bitkit Android track this review is reasoned from the code and from CI.
CI: build (with testDevDebugUnitTest), lint and detekt green on 24a46e61f; on da7953617 they were still running when I posted.

Ruled out

• iOS cannot read an Android backup: the app's Json has encodeDefaults = true and explicitNulls on, so every key the Swift decoder requires (presentedProposalIds, unannouncedOfferIds, notifiedRequestIds, the journal's nullable fields as null) is written. The extra autoPaid* keys are ignored by Codable.
• Android cannot read an iOS backup: stages are matched ignoring case, with UNKNOWN as the fallback. requestId.billingPeriodStartsAt is a nullable string on both sides, and Instant.parse accepts the .SSSZ timestamps ISO8601DateFormatter writes with fractional seconds.
• Fractional USD limits truncated by BigDecimal(...).toInt(): both apps only offer whole-dollar stops ([1, 5, 10, 20, 50] and [10, 50, 100, 200, 500] on iOS, Int on Android), so a non-integer string never reaches a real backup.
• Key drift between apps for presented and unannounced ids: PaykitAllowance.key and iOS scopedId both build <normalized counterparty>/<allowanceId>, and the identity map is normalized on write and on restore on both sides.
• Restore bumps backupStateVersion and triggers an upload of half-restored state: observeBackupChanges drops the change while _isRestoring is raised, and the bump happens inside restoreWalletBackup.
• Strict backupSnapshot() makes a corrupt record fail every backup: that is the trade-off the author chose in answer to the earlier unreadable-state threads. save() still goes through the lenient loadAll() and overwrites the record, so it heals on the next allowance write.
• Missing changelog fragment: answered in its thread. Allowances have not shipped in a release yet, so there is no shipped loss of history to describe.

Merge confidence: 4/5, no findings, but CI on da7953617 had not finished when I posted and the branch was not built here.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to the reviewing session)

[ovitrif]

Could we rebuild these values from the full accounting history, or include complete totals and IDs in the shared backup, and cover an iOS restore with more than 200 attempts?

@ben-kaufman Done in 8c81493, from the accounting history: every recovery pass (which runs on activation, so right after a restore) now folds each succeeded automatic attempt in the SDK ledger that the lasting totals do not count yet into autoPaidSatsByAllowance and autoPaidRequestIds, keyed by its request so nothing counts twice. That covers an iOS backup past the 200-entry journal and Android attempts whose journal entry is missing, and keeps the shared backup format as it is. PaykitAllowanceExecutorTest checks that a ledger success missing from the totals is added once and a failed one is not.

[ben-kaufman]

Recovery now double-counts a payment when the SDK ledger says SUCCEEDED but its journal entry is still SENT or UNKNOWN. This can happen if the app stops after recordPaymentOutcome saves success and before the local journal update. The ledger fold adds the amount first, then syncSettledJournal calls applyJournalStage, which adds it again without checking the counted request IDs. A 1,000-sat payment therefore leaves a saved 2,000-sat “Paid automatically” total. Could we make both paths share the same counting guard and cover a succeeded ledger attempt whose journal is still pending?

Resolved in 652a95c.

[ovitrif]

Pushed ab457ea: the wallet backup now keeps which allowance attempt consumed each payment list (fbbfcfc), so a restored failed attempt can still give its list back. It also brings in #1707's latest head. This answers @ben-kaufman's comment on BackupRepo.kt:953.

[jvsena42]

Re-reviewed to 67b3756. No new findings.

• Releases are owner-checked and become no-ops after the first.
• A list version is cleared only after its release succeeds.
• The new claim lock is taken only in consume and release, with no re-entry or cross-lock.
• Restore writes the ledger and claims together before any recovery can read the journal.
• Backups stay compatible both ways: v3.0.0 ignores the new owner field, and older backups default to an empty owner. The key matches across Android and iOS.

Device gate (partial): 67b3756, not run yet. Pending: the allowance journeys paired across both platforms, including restore-keeps-history.xml.

[piotr-iohk]

QA review

Scope: Full review of the full PR diff against its merge base, at 67b3756.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The wallet backup now carries each identity's allowance journal and automatic-payment history, and recovery settles a restored submitted attempt from the node once the node can list payments. The shared allowance wire format was compared with bitkit-ios#1141 at f085773 only; that PR was not reviewed.

Device testing: not performed in this review. The unit tests named in the PR were inspected, not executed. A local locale check confirmed that SUBMITTED and FAILED do not stay ASCII under Turkish and Azerbaijani default locales.

Suggested additional test cases

• Platform: Android with the system locale set to Turkish or Azerbaijani, then iOS.
• Setup: a payer allowance whose journal includes a submitted or failed attempt, and a wallet backup taken on that Android device.
• Action: restore the backup on iOS.
• Expected: that identity's allowances and journal restore, and a submitted attempt the node lists as paid stays paid.

Findings

• [MEDIUM] Write allowance journal stages in ASCII lowercase — inline at app/src/main/java/to/bitkit/models/PaykitAllowanceStateBackup.kt:86.

Reviewer model: grok-4.7
Reasoning effort: high

[ovitrif]

Pushed d6ba1c9: allowance journal stages in the wallet backup are now written as ASCII lowercase names under any device locale. Before, a Turkish or Azerbaijani locale wrote submıtted and faıled, and iOS dropped that identity's allowance state on restore. This answers @piotr-iohk's QA finding; a unit test covers the tr-TR case.

[piotr-iohk]

QA review

Scope: Re-reviewed the full PR at bc5f8ef after the comparison base moved from 21c6f49 to 2122f57d and no prior coverage baseline was available. Merge base is 2122f57.

No new actionable code findings.

Allowance groups, the attempt journal, lasting auto-paid totals and request ids, and the private payment-list owner travel in the wallet backup. Restore validates that state before replacing it. A backup fails when the stored allowance state cannot be decoded, so a corrupt local record does not replace a good remote backup. A trusted-time watermark change alone does not request a backup. Recovery adds ledger successes the totals do not already count, once. A restored attempt whose ledger status is submitted is settled from the node when the journal is still prepared or submitted and the payment hash is known, and it stays open until the node can list payments.

Compared the shared backup contract with bitkit-ios#1141 at f085773; that PR was not reviewed. Field names, lowercase stages, and RFC 3339 timestamps match. Android stores lasting totals and rebuilds them when a backup omits them; iOS derives totals from the ledger and ignores the extra fields. Android fails the wallet backup if allowance state is unreadable; the iOS twin leaves that state out and still uploads.

Validation: read the new unit tests and J1; did not run them. Device testing: not performed in this review. The author's fresh-install run is on f5f3c9713, before the ledger fold and the node wait, and J1 is still unchecked.

Suggested additional test cases

• Android, regtest, setup from journeys/allowances/README.md with the active allowance. After an automatic payment is handed to the node, wait until Data Backups shows a wallet backup for that attempt, then kill the payer before the attempt settles. Restore once the node lists that payment as succeeded. Expected: Paid automatically includes it, and a later request's Limit Reached decision counts it toward the monthly cap.
• Android restore of a wallet backup made by the iOS twin at f085773, which omits lasting totals. Expected: Paid automatically matches the succeeded automatic payments, Auto-paid tags appear, and running recovery again does not add those amounts a second time.

Earlier findings reconciled at this revision: 8 verified fixed, 1 not substantiated.

Reviewer model: grok-4.7
Reasoning effort: high

[jvsena42]

Re-reviewed to bc5f8ef. No new findings. d6ba1c9 writes journal stages with lowercase(Locale.ROOT), and the reader still accepts the older strings through a case-insensitive match. v3.0.0 writes no allowance state, so there is no installed base to migrate.

Device gate (partial): bc5f8ef, not run yet. Pending: restore-keeps-history.xml, paired with iOS.

[ben-kaufman]

The source review at bc5f8ef3 found no new material issues. J1 remains unrun and independent security coverage is incomplete.

Reviewer model: claude-opus-5-5, gpt-6.1-sol
Reasoning effort: high for both

The base branch was changed.

[ovitrif]

Retargeted this PR to master before merging its base, #1707, so deleting that branch would not close it. The base change dismissed the approval, so it needs a re-approval.

[piotr-iohk]

QA review

Scope: Full reassessment of the complete PR diff at bc5f8ef against merge base 65f20828, with supplied comparison base 527208e6. The comparison base changed and no verified baseline was supplied; no coverage is inherited.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

Compared the shared backup and recovery contracts with iOS f3b58258, including payment-list ownership and restored totals; this was a targeted comparison, not a full iOS review. The earlier automatic-attempt fix leaves the manual boundary described in the finding.

Validation: inspected native regression tests and J1; diff whitespace and changed XML/resource checks passed. A focused Kotlin recovery probe confirmed the hashless decision with stubbed dependencies. Full local Android unit execution stopped because the Android SDK is absent. GitHub build and unit-test CI passed for the pinned head.

Device testing: not performed in this review. The author lists J1 as unrun; the reported fresh-install restore predates the later recovery changes.

Findings

• [HIGH] Keep restored manual Lightning attempts unresolved when their hash is absent — inline at app/src/main/java/to/bitkit/repositories/PaykitAllowanceExecutor.kt:392.

Reviewer model: gpt-6.1-sol
Reasoning effort: high

[ovitrif]

Pushed f0eaf48: fix: journal a manual lightning hash before the send.

• A manual Lightning payment now journals its invoice hash when the attempt is begun, before the SUBMITTED stage, instead of right before the send. A backup taken between the two used to restore a hashless attempt, which recovery failed without asking the node. That released the request's reservation even if the payment had gone out.
• Answers the [HIGH] finding from @piotr-iohk in fix: keep allowance history across a wallet restore #1708 (comment).
• Added a regression in PaykitAllowanceExecutorTest.kt: a manual Lightning attempt backed up at SUBMITTED, restored and recovered, settles as succeeded from the node.

[ovitrif]

Updated this branch with master so its approval sticks: the ruleset dismisses approvals on a branch behind its base. No change to the PR's own code.

[piotr-iohk]

QA review

Scope: Re-reviewed the full PR diff against its merge base, at 0f06291, after the comparison base moved to a56c75be. No prior coverage was inherited. That merge base is the pinned base; it is the second parent of the head.

No new actionable code findings.

A restored wallet keeps each identity's allowance journal, auto-paid totals, and the owner of a consumed private payment list. Recovery waits while the node cannot list payments, then settles a restored open attempt from the node, and adds ledger successes the totals do not already count. Compared with twin bitkit-ios#1141 at f3b58258: timestamps, lowercase stages, request ids, and whole-dollar USD labels match, and restore-keeps-history differs only in platform controls.

Non-blocking consistency suggestions. Android stores USD labels with BigDecimal.toInt(). iOS's shared-format test round-trips perPaymentUsd "4.99"; Android would store 4. Today's stops are whole dollars, and the sat caps still restore unchanged. iOS also skips one allowance identity it cannot decode and restores the rest of the wallet backup; Android fails that wallet restore when one identity does not decode.

Suggested additional test cases

1. Android, dev regtest, two instances, Active allowance from set-and-accept.xml. On the payer, back up after an allowance Lightning attempt has its payment hash journaled and before that payment settles, then Reset And Restore. Expected: while the node cannot list payments, the attempt stays open and is not marked failed. Once it can, a payment the node lists as succeeded is included once in Paid automatically and is not sent again. A payment the node does not list is not sent again.
2. Cross-restore between Android 0f06291 and iOS f3b58258. On one platform, create the Active allowance and one auto-paid $2 request, back up, and restore that backup on the other. Expected: the allowance is Active, Paid automatically matches the pre-restore total at the current rate, and the payment row subtitle ends in · Auto-paid.

Device testing: not performed in this review.

Ready for device testing.

Inspected PaykitAllowanceStateBackupTest, PaykitAllowanceStoreTest, BackupRepoTest, PaykitAllowanceExecutorTest, PaykitAllowanceRepoTest, and PrivatePaykitRepoTest at this head. This review did not execute them. The author reports they pass on this head.

Earlier findings reconciled at this revision: 1 still present, 6 verified fixed.

Existing concerns

• [LOW] Add the required changelog fragment for the restore fix — still unresolved; already tracked.

Reviewer model: grok-4.7
Reasoning effort: high