fix: keep lnurl pay comment as activity note

[jvsena42]

Fixes #417
Related to #1275

This PR keeps the comment typed on an LNURL-pay payment as the note of the sent Lightning activity.

Description

• Saves the LNURL-pay comment before sending, because the app sent it to the server and then dropped it, so the activity only showed the invoice's description hash.
• Applies the saved comment to the activity once the payment succeeds or goes pending, and again when the payment sync creates or updates the row, so it survives settlement and relaunch.
• Replaces the stored message only when it is empty or equals the description LDK reports (the hash hex), so an invoice's own description or an existing note is never overwritten.
• Clears the saved comment when the payment definitely fails.
• Reads the payment list before reading the activity row, so the row write cannot race the payment sync and write back a stale pending row.

Out of Scope

• AppViewModel: the comment is stored only for description-hash invoices (LUD-06). LNURL-pay servers that return a plain-description invoice still send the comment without showing it in the activity, to protect invoice descriptions.
• CacheStore: pending comments have no pruning. An entry is left behind only if the app dies or the send coroutine is cancelled between saving and completing the send.
• CoreService: the pending comments are read once per outbound Bolt11 payment during sync rather than once per sync. DataStore caches the value, so the cost is small.
• A failed payment row created before the comment is cleared may still show the comment.
• iOS parity: bitkit-ios needs the same check of whether it keeps the LNURL-pay comment on the activity.

Design

The comment appears in the existing Invoice note slot of the Lightning transaction detail. No layout changes.

• TX Lightning: https://www.figma.com/design/ltqvnKiejWj0JQiqtDf2JJ/?node-id=46920-146974
• LNURL Pay Confirm (comment input, unchanged): https://www.figma.com/design/ltqvnKiejWj0JQiqtDf2JJ/?node-id=46920-148562

Preview

Recorded on a simulation build: staging regtest has no LNURL-pay endpoint that allows comments. The comment "Thanks for the channel" was hardcoded in place of the LNURL checks, and a Blocktank order invoice with a plain description stood in for the LDK description hash. Everything after that (saving, sending, applying on pending and on sync) ran the branch code.

Figma TX Lightning	Confirm	Pending

Succeeded	Home after relaunch	Detail after relaunch

QA Notes

Manual Tests

Needs an LNURL-pay endpoint that issues description-hash invoices (LUD-06) and allows comments, on the same Lightning network as the app: an lnurl-node LNURL.createServer backed by a local LND, as the bitkit-e2e-tests lnurl suite sets up in test/specs/lnurl.e2e.ts. It needs minSendable below maxSendable with 1,000 sats in range, since a fixed amount skips the amount screen. The bitkit-docker lnurl-server does not work for this: it builds the invoice with a plain memo containing the comment, so the note reads LNURL Payment <id> - thanks on this branch and on master alike. The comment input and the LNURL and invoice-description checks were not driven on a device; they are covered by unit tests. The bitkit-e2e-tests lnurl suite has a commented-out InvoiceComment assertion that could be re-enabled against the InvoiceNote tag.

• 1a. Scan LNURL-pay → Send Amount → 1000 → Send Confirm → type comment "thanks" → swipe to pay: success or pending screen shows.
  • 1b. Close → open newest activity → Activity Detail: Invoice note shows "thanks", not the description hash.
  • 1c. Force-stop → relaunch: Home row subtitle and Activity Detail Invoice note still show "thanks".
• 2. LNURL-pay → pay with LSP holding the HTLC (pending) → wait for settlement → Activity Detail: note stays "thanks" after status turns Successful.
• 3. LNURL-pay → leave comment empty → pay: Activity Detail shows the same note as before this change.
• 4. regression: Pay a regular LN invoice with a description: Invoice note shows the invoice description.
• 5. regression: LNURL-pay with comment → payment fails (e.g. no route): no stale comment applied to later payments.

A new journey covers flow 1, committed at journeys/lnurl/lnurl-pay-comment-note.xml and
listed in journeys/README.md (suites table and cross-platform table). It was not run as written
because staging has no LNURL-pay endpoint; the simulated run above covered its payment, note and
relaunch steps.

lnurl-pay-comment-note.xml

diff --git a/journeys/lnurl/lnurl-pay-comment-note.xml b/journeys/lnurl/lnurl-pay-comment-note.xml
new file mode 100644
--- /dev/null
+++ b/journeys/lnurl/lnurl-pay-comment-note.xml
@@ -0,0 +1,60 @@
+<journey name="LNURL Pay Comment Note">
+  <description>
+    Verifies that the comment typed when paying an LNURL-pay request is saved as the Invoice note of
+    the sent Lightning activity, and that it survives an app restart.
+
+    Requires an LNURL-pay endpoint that issues description-hash invoices (LUD-06) and allows comments
+    (commentAllowed of at least 12), reachable from the same Lightning network as the app: an
+    lnurl-node `LNURL.createServer` backed by a local LND, as the bitkit-e2e-tests lnurl suite sets up
+    in test/specs/lnurl.e2e.ts. The endpoint also needs minSendable below maxSendable with 1,000 sats
+    (1,000,000 msat) in range: a fixed amount goes straight to Confirm and skips the amount screen, so
+    the amount steps would fail, and the e2e suite's own pay ranges (149,500-200,999 msat) do not
+    cover 1,000 sats.
+
+    The bitkit-docker lnurl-server is not usable here. It builds the invoice with a plain memo that
+    already contains the comment, so the note reads "LNURL Payment &lt;id&gt; - thanks" on this branch
+    and on master alike, and the run says nothing about the fix. A note in that shape means the
+    endpoint issued a memo invoice rather than a description-hash one.
+
+    The staging regtest backend the dev flavor uses has no LNURL-pay endpoint. Also requires a
+    spending balance of at least 1,000 sats plus routing fees.
+  </description>
+  <actions>
+    <action>
+      Launch the Bitkit app and go to the wallet home screen
+    </action>
+    <action>
+      Run adb shell am start -a android.intent.action.VIEW -d "lightning:&lt;lnurl-pay&gt;" to.bitkit.dev with the LNURL-pay string
+    </action>
+    <action>
+      Verify the send amount screen (testTag "SendNumberField") is visible
+    </action>
+    <action>
+      Enter 1000 on the number pad (testTags "N1", "N0", "N0", "N0") and tap Continue (testTag "ContinueAmount")
+    </action>
+    <action>
+      Verify the comment field (testTag "CommentInput") is visible on the confirm screen
+    </action>
+    <action>
+      Type "thanks" into the comment field (testTag "CommentInput")
+    </action>
+    <action>
+      Swipe the pay slider (testTag "GRAB") to the right
+    </action>
+    <action>
+      Verify the payment success screen (testTag "SendSuccess") or the "Payment Pending" screen is visible — the pending screen carries no testTag, so assert it by its title
+    </action>
+    <action>
+      Close the result screen and open the newest activity row (testTag "ActivityShort-0")
+    </action>
+    <action>
+      Verify the Invoice note (testTag "InvoiceNote") is exactly "thanks", not a memo containing it and not the invoice description hash
+    </action>
+    <action>
+      Run adb shell am force-stop to.bitkit.dev, relaunch the app and open the newest activity row (testTag "ActivityShort-0")
+    </action>
+    <action>
+      Verify the Invoice note (testTag "InvoiceNote") is still exactly "thanks"
+    </action>
+  </actions>
+</journey>

Automated Checks

• Unit tests added: CoreServiceTest.kt covers filling an empty message, replacing the LDK description hash, keeping a different stored message, and ignoring a null or blank comment.
• Unit tests added: AppViewModelSendFlowTest.kt covers saving the comment before paying and applying it on success and on pending, skipping invoices with their own description, and clearing it on failure.
• Unit tests added: ActivityRepoTest.kt covers the new repository methods passing through to the cache and activity service, and a service failure returning a failure result.
• Unit tests added: ActivityServiceLightningMessageTest.kt runs the real activity service with mocked bitkit-core calls. It covers the stale-row race with the payment sync, the no-row case, and processBolt11 creating or updating a row with the comment, including a comment saved mid-read. The two race tests fail against the previous implementation.
• Ran locally: just compile, just test (2613 passed), just lint.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge, with the new LNURL-pay note lifecycle covered across immediate and synchronized activity updates.

Summary

This PR preserves comments entered for description-hash LNURL-pay invoices as Lightning activity notes.

• Persists comments by payment hash before initiating payment.
• Applies comments during immediate success or pending handling and during later payment synchronization.
• Preserves existing invoice descriptions and user notes.
• Clears cached comments after application or definite payment failure.
• Adds focused unit coverage for repository delegation, message merging, synchronization races, send outcomes, and persistence behavior.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[LNURL-pay confirmation] --> B{Comment nonblank and invoice has no direct description?}
  B -- No --> C[Send payment normally]
  B -- Yes --> D[Cache comment by payment hash]
  D --> E[Send Bolt11 payment]
  E -->|Success or pending| F[Try to update existing activity]
  F -->|Activity exists| G[Replace empty or LDK hash message]
  F -->|No activity yet| H[Keep cached comment]
  H --> I[LDK payment synchronization]
  I --> G
  G --> J[Remove cached comment]
  E -->|Definite failure| K[Remove cached comment]

Loading

_{Reviews (1) · Last reviewed commit: "fix: avoid stale row writes when storing..."}

[github-actions]

Regtest APK

Built from 2efcd8d (run).

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

[jvsena42]

Approved — no HIGH/MEDIUM findings at head 05e18eabe. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Description-hash invoice paid without a comment still shows the 64-hex hash as the Invoice note

Checked: Traced the comment from SendUiState.comment through savePendingLnurlComment (AppViewModel.kt:4050-4056) to CacheStore.pendingLightningMessages and back out via ActivityService.setLightningMessageIfEmpty and processBolt11. The key is decodedInvoice.paymentHash.toHex(), which equals ldk-node's PaymentId/event paymentHash for bolt11 (sendLightning already matches events on it), so the entry cannot attach to another payment; a retry re-fetches a new LNURL invoice, so it cannot inherit a stale hash. The description gate is correct: bitkit-core v0.5.14 scanner returns description=None for h-tagged invoices while ldk-node reports the hash hex as PaymentKind.Bolt11.description, so withPendingMessage replaces exactly the hash-hex placeholder and never a real description or user note; withPaymentUpdate from the base branch never touches message. Concurrency between setLightningMessageIfEmpty and processBolt11 (full sync, handlePaymentEvent, async chunks) is safe: both run on the single-threaded CORE queue and for outbound payments processBolt11 has no suspension point between reading the row and writing it (the contact resolver returns synchronously for OUTBOUND), so every interleaving ends with the comment on the row or still in the cache. Upgrade from the released build reads app_cache.json without the new key into the default empty map (kotlinx defaults, ignoreUnknownKeys=true for downgrade); the comment is persisted on the activity row, which ActivityBackupV1 serializes whole, and restoreFromBackup upserts it back with the message intact. Definite-failure, pending (10s timeout / held HTLC), process-death-while-pending and dismissed-sheet paths all leave the comment either applied or in the cache for the next sync. iOS twin absent: bitkit-ios LnurlPayConfirm.swift sends the comment but nothing stores it, as the PR body already notes.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head 2a28e7575. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: MEDIUM (journey, non-blocking) — The example endpoint, the bitkit-docker lnurl-server, returns plain-description invoices, so the journey never exercises the fix

Checked: Round 3 (2a28e75, new journey): Round 3 delta is the new journey and two README rows. Every testTag the journey names exists at head: SendNumberField and ContinueAmount (SendAmountScreen.kt:260, :334), CommentInput (SendConfirmScreen.kt:456, rendered only when commentAllowed > 0 at :850), GRAB (SwipeToConfirm.kt:171), SendSuccess (SendSheet.kt:348), ActivityShort-N (ActivityListSimple.kt:52) and InvoiceNote (ActivityDetailScreen.kt:588). The flow order matches the code: a variable-amount LNURL-pay goes to SendRoute.Amount (AppViewModel.kt:3523), onAmountContinue does not apply QuickPay, the pending screen title is "Payment Pending" and has no tag, and InvoiceNote is the Invoice note block on Activity Detail shown for any Lightning activity with a non-empty message. lightning:lnurl1... is accepted by the bitkit-core scanner's LNURL regex. No bitkit-ios twin exists, and the journey is recorded as not ported in the cross-platform table.

[jvsena42]

Approved — no blocking HIGH/MEDIUM findings at head 7a011ab52. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Round 4 (7a011ab): the journey MEDIUM is fixed. Round 4 covers 7a011ab, which only edits journeys/lnurl/lnurl-pay-comment-note.xml in answer to the memo-only endpoint finding. The journey now points at an lnurl-node LNURL.createServer backed by LND, as bitkit-e2e-tests test/specs/lnurl.e2e.ts sets up. lnurl@0.26.2 (the version in that repo's lockfile) passes descriptionHash = sha256(metadata) to the backend, and lightning-backends@1.6.5 lnd.js sets description_hash on POST /v1/invoices, so those invoices are description-hash invoices with no comment in them. The journey also says the bitkit-docker lnurl-server cannot be used, and that is correct: lnurl-server/routes/pay.js builds the invoice memo as LNURL Payment <id> - <comment>. It requires minSendable < maxSendable with 1,000,000 msat in range and states that the e2e suite's payRequest1 range (149,500-200,999 msat) does not cover it, so the runner has to generate its own payRequest. The InvoiceNote assertions now require the note to equal "thanks" exactly, which fails on a memo-shaped note or the description hash. Since the comment cannot reach the note through the invoice, a pass can only come from the persisted comment. testTags and step order are unchanged from the approved round.

[jvsena42]

The red build here was the known QuickPayRepoTest > reconcile during live dispatched op does not steal completion flake — the one #1193 exists to remove. #1306 landed that fix in master, but this branch had not picked it up: QuickPayRepoTest.kt on the old head had no in-memory cache store at all.

Merged current master in; the file now carries the fix and just test is green on the merged head. The remaining settings and onchain_boost_receive_widgets failures are the e2e side, tracked with the companion branch synonymdev/bitkit-e2e-tests#239.

[ovi-reviewer]

Verdict: ✅ Approve

---

Reaudit: diff 0 files.

Findings:
N/A

Audit:
Already done in comment.

QA: redroid Android 15 emulator

1. ✅ passed: LNURL-pay with comment reached payment success.

   Test 1:
   Original 1a: complete an LNURL-pay with comment "thanks" and reach success or pending.

   1.mp4

2. ✅ passed: The newest activity note was exactly thanks.

   Test 2:
   Original 1b: reopen the newest activity and verify the invoice note is exactly "thanks".

   2.mp4

3. ✅ passed: The note survived force-stop and relaunch.

   Test 3:
   Original 1c: force-stop, relaunch, and verify the home subtitle and invoice note remain "thanks".

   3.mp4

4. ✅ passed: A pending hold payment kept thanks after settlement.

   Test 4:
   Original 2: settle an initially pending LNURL-pay and verify the note remains "thanks".

   4.mp4

5. ✅ passed: Empty-comment LNURL-pay preserved the description-hash note.

   Test 5:
   Original 3: pay LNURL-pay with an empty comment and verify the prior note behavior.

   5.mp4

6. ✅ passed: A regular invoice preserved its description note.

   Test 6:
   Original 4: pay a regular invoice with a description and verify the invoice note.

   6.mp4

7. ✅ passed: A failed LNURL comment did not leak into the next payment.

   Test 7:
   Original 5: fail an LNURL-pay with a comment and verify no stale comment reaches later payments.

   7.mp4

Coverage:
QA: 7 of 7 manual tests passed

---

Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)