[ovitrif]

Fixes #1163
Twin: synonymdev/bitkit-android#1708

This PR journals a manual Lightning payment's hash when its allowance attempt is reserved, so a backup taken before the send still lets the node settle the attempt.

Description

• Passes the invoice's payment hash to the allowance ledger when a manual Lightning payment of an incoming request is reserved, and journals it with the attempt. Before, the hash was added only just before the send, so a backup taken between the two writes restored the attempt without a hash; recovery then recorded it as failed even when the payment had gone through, which freed the request for a second payment. On-chain payments are unchanged.

Out of Scope

• None.

Design

N/A — no UI changes.

Preview

N/A

QA Notes

Setup

N/A — no device testing required: the window between the two journal writes cannot be staged for a backup from the UI, so the regression test covers it.

Journeys

N/A — not drivable; see Manual Tests.

Manual Tests

N/A

Automated Checks

• added PaykitAllowanceExecutorTests.swift — a manual Lightning attempt restored from a backup taken before the send settles as succeeded through the node

Evidence

• Unit tests at 900d403 on an iOS 26.5 simulator: the full unit suite passed.
• The new test fails on an executor that takes the hash but still journals the attempt without it: recovery records the attempt as failed instead of succeeded.

Gaps

• No device run: the backup has to be taken inside the window between reserving the attempt and sending, which the app gives no way to stage.

Models used

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

Reasoning values are the harness's effort setting for this session.

[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-28T19:13:32.455432Z	900d403	PR opened

ℹ️ 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.

[talosmachina]

No findings. Journals the invoice's payment hash when a manual Lightning payment of an incoming request reserves its allowance attempt, so a backup taken before the send restores an entry recovery can settle through the node instead of marking it failed. Reviewed 900d403d49d5a1949c5346c43dc2320617e7cf70, full tier, static: iOS does not build on this reviewer's host, so nothing was compiled or run here.

What I checked, and 4 candidates I ruled out

Read in full: PaykitAllowanceExecutor.swift (manual payment, recovery, verifiedOutcome, lightningPaymentSettled), SendConfirmationView.performPayment, the backup mapping in PaykitAllowanceStateBackup.swift, the new test.
Call sites traced: beginManualPayment (one production caller, SendConfirmationView.swift:1218), lightningPaymentSettled (AppViewModel.swift:1835, :1871), manualLightningPaymentSent.
CI: SwiftFormat and validate green; Run Tests, Run Integration Tests and build-local still pending at review time.

Ruled out

• Hash differs from the one actually paid: the early hash and the send both read app.scannedLightningInvoice?.paymentHash.hex under the same .lightning check, and manualLightningPaymentSent overwrites it with the same lowercased value.
• Settlement events now match an entry before the send: lightningPaymentSettled already matched .submitted; with the hash set earlier, a match needs a node event for this invoice's hash in the few awaits between reserving and manualLightningPaymentSent, and a second live attempt on the same request is refused with alreadyRecorded, so I found no sequence that reaches it.
• A live crash between reserve and send now settles differently: with no backup, the node does not know the hash, so verifiedOutcome returns .failed exactly as the nil-hash branch did; the change only alters the outcome when the node reports the payment.
• Restored entry loses the hash: PaykitAllowanceStateBackup encodes and restores paymentHash (lines 76, 126, 168), which the new test round-trips.

Merge confidence: 4/5, the unit suite carrying the new test had not finished in CI when this was reviewed, and nothing was built here.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (not exposed to this run)

[piotr-iohk]

QA review

Scope: Reviewed the full PR diff against its merge base, at 900d403.

No new actionable code findings.

Reserving a manual Lightning payment of an incoming request now journals that invoice's payment hash with the allowance attempt. A backup taken after that reservation and before the send restores an attempt recovery can settle from the node when the payment succeeded, so the allowance hold stays in place (#1163). An attempt that never reaches the node still ends failed: cancellation before submission records that outcome, and recovery treats a submitted hash the node does not know as failed. On-chain attempts still journal no hash. The same reservation-time hash is what bitkit-android#1708 journals at f0eaf480; only that contract was compared.

Validation: Run Tests passed on this revision and includes testRecoverySettlesAManualLightningPaymentRestoredFromABackupTakenBeforeTheSend. This review inspected that test and did not re-run it. Device testing: not performed. The interval between the reservation write and the send is inside the send operation and cannot be staged from the UI, so that unit test is the check for this regression.

Reviewer model: grok-4.7
Reasoning effort: high

[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 at 54316de because the base moved to 031625d, and the new master commits touch SendConfirmationView.swift. The merge base is the PR base. No coverage was carried over from the earlier review at 900d403.

No new actionable code findings.

The change now journals the invoice hash when a manual Lightning payment of an incoming request reserves its allowance attempt. The automatic path already did this at .prepared. The hash is the same one later passed to manualLightningPaymentSent, which overwrites it, and to the send. A restored entry still at .submitted is settled by the node: succeeded settles it, pending records it as unknown so the hold stays, and failed or not found releases it. While the node has no payment history yet (.unavailable), recovery leaves the entry alone and tries again later. A kill without a restore still settles as failed, because the node does not know a payment it never sent. On-chain payments and the no-hash path are unchanged. Master's isSubmittingPayment re-entry guard does not interact with this change, and the merge commit resolved cleanly.

Parity: bitkit-android#1708 @ 0f06291 was compared on this contract only. It also journals the lowercased Lightning hash at reservation, and it waits while the node cannot list payments, like the iOS .unavailable path. That PR names bitkit-ios#1141 as its twin and covers allowance backup more broadly. It was not reviewed beyond this contract.

Validation: unit-tests run passed at 54316de and includes the new restore-before-send test. Integration and e2e checks were still pending when this review was done. I ran nothing locally: this host can't build iOS, so all conclusions come from reading the source. Device testing: not performed in this review. The author notes that the backup window can't be staged from the UI.

Reviewer model: claude-opus-5-5
Reasoning effort: high

[ovitrif]

@ben-kaufman @piotr-iohk this PR needs one approval to merge for 3.1.0. It is the iOS twin of the Android fix in synonymdev/bitkit-android#1708. CI is green at 54316de, the branch is up to date with master, and no threads are open. The QA review at this head found nothing to fix. Could one of you approve it?

[piotr-iohk]

Approving at 54316decf6a412fb048d2a318800fba05b5fbb98 following device validation on the pinned CI build.

On a clean-installed iPhone 17 simulator (iOS 26.0), using local regtest and staging VSS backups:

• Manually paid an incoming 3,000-sat Lightning request and observed Bitcoin Sent.
• After backup, Reset And Restore, and recovery from the same phrase, the wallet retained its expected 499,498-sat balance.
• The same request remained Proof submitted, with no Pay action offered.
• Background/resume and a cold restart preserved the paid state. The paired node recorded only the original two transfers, with no duplicates.

This checks the ordinary completed-payment restore path. The exact backup window between reservation and send cannot be staged through the UI and was not exercised on-device. The current-head passing unit suite includes testRecoverySettlesAManualLightningPaymentRestoredFromABackupTakenBeforeTheSend, which covers that regression. Current unit, integration and E2E checks passed.

[ovitrif]

@piotr-iohk Merging this PR on your approval of head 54316de, with your device validation on that head, checks green and no open threads.