[ovitrif]

Fixes #1908

Description

• Fixes a refused Spark send that could settle as SendRefused while a competing Spark send of the same invoice paid: settledSparkSendOutcome now reads sparkRepo.sendRecordMark() before isSendLive, as earlierLightningSend and earlierNodeSend already do. A send the mark counts as recorded is then already admitted or in sendsInFlight when liveness is read, so a send that finishes during the row read still keeps the refused one pending.

Found by @coreyphillips in #1903 (comment).

Out of Scope

None.

Design

N/A — no UI changes.

Preview

N/A — no user-visible changes.

QA Notes

Setup

N/A — no device testing required: the window is two adjacent statements on the refused send's thread, which only a unit test can stage.

Journeys

N/A — no user-visible behaviour change.

Manual Tests

N/A

Automated Checks

• added LightningRepoTest.kt — a refused Spark send stays pending when a competing send is admitted and records as its mark is read, then ends during the row read

Evidence

• LightningRepoTest passes on this PR's head; the added test fails on the old order with SendRefused.
• just lint passes.

Gaps

None.

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: ✅ Completed

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-14T01:39:38.812774Z	eddb097	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.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Reads Spark's send-record mark before checking whether a send is still live.

• Refused Spark sends stay pending when a competing send records.

_{Reviews (1) · Last reviewed commit: "fix: spark send mark read before livenes..." · Reviewed by Greptile}

[talosmachina]

No findings. Moves sendRecordMark() ahead of the first isSendLive read in settledSparkSendOutcome, so a competing Spark send can no longer be admitted and recorded between the two reads, then end during the row read and leave the refused send settled as SendRefused. Reviewed eddb097, light tier (3 changed lines in LightningRepo.kt, plus the test).

What I checked, and 2 candidates I ruled out

Read: settledSparkSendOutcome, isSendLive, the Spark branch of payInvoice and payInvoiceWithLdk in LightningRepo.kt; payInvoice, isSendInFlight, sendRecordMark and hasRecordedSendSince in SparkRepo.kt.

Ordering traced: in SparkRepo.payInvoice the send goes into sendsInFlight (putIfAbsent) before sendRecords.incrementAndGet(), so any send the mark counts as recorded was already in flight or finished when the mark was read. With the mark first, a send recorded before it and still running is caught by wasLive, and a send recorded after it is caught by hasRecordedSendSince. Under the old order a send recorded between wasLive and the mark was missed by both checks, which is the window the new test stages. This matches the mark-before-check order already used in the Spark branch of payInvoice and in payInvoiceWithLdk.

Ruled out

• A send recorded before the mark that ends before wasLive: it finished before the row read, so lightningSend returns its own row (COMPLETED settles as success). If that row is FAILED, the competing send failed too, so SendRefused is the right outcome.
• The new test passing for the wrong reason: the second sendRecordMark() call is the one in settledSparkSendOutcome (the first is read at the top of the Spark branch), so the competing send is recorded exactly between the mark and the liveness read, and lightningSend clears it before the second isSendLive.

Build: not built here. On the Bitkit track an automated review never runs the branch's Gradle build, so this is reasoned from the code. CI (build-and-test, lint) was still running when this review was posted.

Merge confidence: 4/5, no findings and the race is covered by the new test, but CI had not finished at review time.

Reviewer model: claude-opus-5-5
Reasoning effort: Not exposed (the run's metadata available to this reviewer does not carry the setting)

[github-actions]

Regtest APK

Built from eddb097 (run).

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

[coreyphillips]

Two independent reviews, nothing blocking a merge.

nits

• New comment says a counted send is live at the liveness read; it may have ended (app/src/main/java/to/bitkit/repositories/LightningRepo.kt:1643-1644)

[ovitrif]

Merging once the required checks are green: approved by @coreyphillips on eddb097, and his comment-wording nit is moved to #1919 to keep that approval.