Skip to content

fix: handle cjit channel ready notification#787

Draft
jvsena42 wants to merge 8 commits into
masterfrom
fix/channelready-cjit
Draft

fix: handle cjit channel ready notification#787
jvsena42 wants to merge 8 commits into
masterfrom
fix/channelready-cjit

Conversation

@jvsena42

@jvsena42 jvsena42 commented Feb 17, 2026

Copy link
Copy Markdown
Member

Fixes #666
Closes #626

This PR adds CJIT (just-in-time channel) payment notifications to the foreground-service path and de-duplicates channel-ready / payment notifications across the two producers (WakeNodeWorker and LightningNodeService), so a single, correctly-formatted notification is shown per event.

  1. Adds a NotifyChannelReady CQRS command + handler to process CJIT payment notifications when a JIT channel becomes ready.
  2. Fixes the missing CJIT notification on the foreground-service path when the app is in the background.
  3. Refactors AppViewModel.notifyChannelReady() to use the new handler, consolidating duplicated CJIT logic.
  4. De-duplicates notifications and fixes amount formatting (addresses review feedback).

Description

When a CJIT payment arrives as a JIT channel opening, LDK emits ChannelReady (not PaymentReceived). The foreground service (LightningNodeService) only handled PaymentReceived and OnchainTransactionReceived, so CJIT payments were silently ignored. The new NotifyChannelReadyHandler encapsulates channel lookup, CJIT verification via Blocktank, activity recording, and notification building, following the same CQRS pattern as NotifyPaymentReceivedHandler.

A ChannelReady (and an incoming payment) event has two notification producersWakeNodeWorker (push/wake path) and the LightningNodeService foreground-service handler — which both fired when active. The review surfaced three issues from this, now resolved:

  • 🟢 Amount formattingWakeNodeWorker now formats amounts with formatToModernDisplay() (space-grouped thousands) instead of raw sats (₿ 48 064, not ₿ 48064).
  • 🟠 Duplicate notificationWakeNodeWorker skips its own user-facing notification when an in-process handler already covers the event (app in foreground, or LightningNodeService.isRunning), leaving a single, richer notification. It still wakes the node, opens the channel, and records the activity. Applies to both the CJIT channel-ready path and the regular incoming-payment (incomingHtlc) path, which shared the same root cause.
  • 🔴 Non-CJIT channel openingWakeNodeWorker only posts the "via new channel" notification when the channel actually has a Blocktank CJIT entry; a regular channel opening no longer surfaces a false "payment received" notification.

Also removed a dead amountOnClose import left in AppViewModel by the refactor.

Preview

fg-notification.webm
wake-2.webm

QA Notes

1. CJIT notification via foreground service

  1. Enable background notifications (foreground service)
  2. Move the app to the background
  3. Send a CJIT payment (first Lightning payment that opens a new channel)
  4. Verify exactly one "Payment Received" notification appears, with a space-formatted amount (e.g. ₿ 48 064)
  5. Tap the notification and verify the transaction sheet shows the correct amount

2. CJIT notification via push (app killed)

  1. Disable background notifications and force-stop the app
  2. Send a CJIT payment
  3. Verify exactly one push notification arrives with a space-formatted amount (this wake/open-channel path still works)

3. Non-CJIT channel opening

  1. Open a regular (non-CJIT) Lightning channel
  2. Verify the "Spending balance ready" toast appears and no payment notification is posted

4. Tests

  • WakeNodeWorkerTest — formatting, non-CJIT (no notification), dedup for CJIT + regular payment
  • LightningNodeServiceTest — channel-ready notification (background) / non-CJIT / foreground
  • NotifyChannelReadyHandlerTest — 6 tests pass
  • just compile, just test, just lint — all green
  • Device journeys under journeys/cjit-notifications/ reproducing all three scenarios

@jvsena42 jvsena42 self-assigned this Feb 17, 2026
@jvsena42 jvsena42 marked this pull request as ready for review February 18, 2026 10:59
@jvsena42 jvsena42 requested a review from ovitrif February 18, 2026 11:00

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tAck

Tests

1. 🟢 CJIT notification via foreground service

nit: we miss currency formatting (space separators for thousands)

2. 🟠 CJIT notification via push (WakeNodeWorker)

Got 2 notifications now, I don't think this used to be the case before (🤔 ❓). Anyways ideally we only show the 2nd one, it's much more polished IMO.

3. 🔴 Non-CJIT channel opening

There's something totally off here, not sure what happened…

Logs: bitkit_logs_1771958898408.zip


PS. Bummer that we have these bugs, because the way the code is architected is really nice 👏🏻 .

val converted = currencyRepo.convertSatsToFiat(sats).getOrNull()

val amountText = converted?.let {
val btcDisplay = it.bitcoinDisplay(settings.displayUnit)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we will have to create a model class for AmountUi to encapsulate all requirements related to displaying monetary values so we can easily replicate enforcing the same rules with minimal effort.

There's a concern not yet covered here but also not in scope of this PR, TBH: we don't format currency symbol applying the same formatting rule of whether the symbol is a prefix of a suffix based on the currencies' symbol locale, as recently implemented for the main in-app amount values UI by @piotr-iohk

@jvsena42 jvsena42 marked this pull request as draft February 25, 2026 11:43
auto-merge was automatically disabled February 25, 2026 11:43

Pull request was converted to draft

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.

Payment Received notification didn't work for CJIT via FG service (not CJIT), app in BG [Bug]: CJIT received sheet & activity item have wrong amount

2 participants