Skip to content

feat: make foreground service opt-in#1053

Merged
jvsena42 merged 13 commits into
masterfrom
feat/fg-service-optional
Jul 2, 2026
Merged

feat: make foreground service opt-in#1053
jvsena42 merged 13 commits into
masterfrom
feat/fg-service-optional

Conversation

@jvsena42

@jvsena42 jvsena42 commented Jun 29, 2026

Copy link
Copy Markdown
Member

Fixes #1007

This PR:

  1. Adds a "Keep Bitkit active in background" toggle to the Background Payments screen that makes the persistent foreground service opt-in and disabled by default. The Lightning node still runs while the app is open, and background payments still arrive via push notifications; enabling the toggle keeps Bitkit active in the background for more reliable payments, at the cost of a persistent notification.
  2. Updates the Background Payments screen and the notification preview to match the latest design.
  3. Always shows the amount in payment notifications and removes the "Include amount in notifications" option.
  4. Fixes a Lightning event-handler leak so toggling the service on and off no longer accumulates stale handlers.

Description

  • New persisted setting, off by default. The foreground service start gate now also requires it, and turning it off stops a running service immediately so the persistent notification disappears at once.
  • Stopping the foreground service no longer stops the in-app node when an activity is active; in the foreground the node lifecycle is owned by the wallet view model, so toggling the service off leaves the node running. When the app is backgrounded, the node still stops as before.
  • The Background Payments screen is reordered to match the design: "Get paid when Bitkit is closed", "Keep Bitkit active in background", a Notifications section with the Customize button, and a Preview of the payment notification. The "Keep Bitkit active in background" switch is disabled while notifications are denied.
  • The notification preview is redesigned to the Android notification card style, with the app icon, title, time, amount, and an expand chevron.
  • Removed the show-notification-details setting and its privacy toggle. Payment notifications now always include the amount across the foreground service, in-app handler, and the background push path.
  • Added a remove-event-handler API and de-register the foreground service handler when the service is destroyed, preventing stale handlers from accumulating across service restarts.

Preview

disable-fg-switch.webm
disable-notifications.webm
running-from-master.webm
wake-to-pay.webm
enable-fg-service.webm

QA Notes

FIGMA

Manual Tests

  • 1. Settings → Background Payments: "Keep Bitkit active in background" is off by default and no persistent Bitkit notification is shown in the status bar.
  • 2. Background Payments → toggle Keep active on: persistent Bitkit notification appears → background the app: node stays alive.
  • 3. Background Payments → toggle Keep active off: persistent notification disappears immediately → with the app still open, receive a payment: still works (node keeps running).
  • 4. Revoke notification permission in Android settings → Background Payments: "Get paid when Bitkit is closed" reads off and "Keep Bitkit active in background" is disabled.
  • 5. regression: Keep active off, app closed → receive a payment: push notification arrives and shows the amount.
  • 6. Background Payments: layout matches the design (two switches with explanations, Notifications/Customize, Preview card).

Automated Checks

  • Unit tests added: default value and setter for the new toggle in SettingsViewModelTest.kt.
  • Test coverage removed: hidden-amount assertions deleted from ReceivedNotificationContentTest.kt and NotifyChannelReadyHandlerTest.kt because the show-notification-details option no longer exists; SettingsData stubs updated in NotifyPaymentReceivedHandlerTest.kt.
  • Ran locally: just compile, the affected unit tests, and just lint.
  • CI: standard compile, unit test, and detekt checks run by the PR bot.

@jvsena42 jvsena42 self-assigned this Jun 29, 2026
@jvsena42 jvsena42 marked this pull request as ready for review June 29, 2026 20:35
@greptile-apps

This comment was marked as outdated.

Comment thread app/src/main/java/to/bitkit/ui/MainActivity.kt
Comment thread app/src/main/java/to/bitkit/repositories/LightningRepo.kt Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9a98728be

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/ui/MainActivity.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/MainActivity.kt
@jvsena42 jvsena42 requested review from ovitrif and piotr-iohk June 30, 2026 11:17
@ovitrif ovitrif added this to the 2.4.0 milestone Jun 30, 2026
@jvsena42 jvsena42 enabled auto-merge June 30, 2026 14:39

@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.

Reviewed locally. I left two comments: one lifecycle issue around the background transition, and one changelog cleanup. Leaving this as a neutral review.

Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt Outdated
Comment thread changelog.d/next/fg-service-optional.added.md Outdated
ovitrif
ovitrif previously approved these changes Jun 30, 2026

@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. The two comments from my previous review are more like nits, the first one didn't really repro in my tests.

piotr-iohk
piotr-iohk previously approved these changes Jul 1, 2026

@piotr-iohk piotr-iohk 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

@jvsena42 jvsena42 marked this pull request as draft July 1, 2026 09:55
auto-merge was automatically disabled July 1, 2026 09:55

Pull request was converted to draft

@jvsena42

This comment was marked as resolved.

@jvsena42 jvsena42 dismissed stale reviews from ovitrif and piotr-iohk via e571137 July 1, 2026 13:48
@jvsena42 jvsena42 marked this pull request as ready for review July 1, 2026 14:04
@jvsena42 jvsena42 requested a review from piotr-iohk July 1, 2026 14:04
@jvsena42

This comment was marked as outdated.

@jvsena42 jvsena42 enabled auto-merge July 1, 2026 14:05
Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e571137fb2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt Outdated
@jvsena42 jvsena42 marked this pull request as draft July 1, 2026 14:19
auto-merge was automatically disabled July 1, 2026 14:19

Pull request was converted to draft

…he LN node would keep running after app ON_STOP
@jvsena42 jvsena42 marked this pull request as ready for review July 1, 2026 16:05
@jvsena42 jvsena42 enabled auto-merge July 1, 2026 17:06
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Tested again on device (Samsung S22, Android 16).

Observations

Testing "Get paid when Bitkit is closed" ON:

  • Keep Bitkit active OFF + Battery savings OFF: background payments work. Push wake path runs end-to-end (FCM → WakeNodeWorker → payment notification). Matches QA case 5.
  • Keep Bitkit active OFF + Battery savings ON: background payments don't work for me. No reliable push wake in logs; payments only land when the app is open or woken manually.
  • Keep Bitkit active ON + Battery savings ON: works. Node stays up via LightningNodeService and the persistent notification — expected workaround on aggressive OEM power saving.

Attaching session logs.
bitkit_logs_2026-07-02_09-06-43.zip

Implementation looks correct to me — approve.

Non-blocking: the text under "Get paid when Bitkit is closed" (settings__bg__enabled) over-promises a bit. It says you can get paid when the app is closed if you're online, but battery saver can block that path even with notifications on. Something like:

Background payments are enabled. You can receive funds when the app is closed, if your device is online. On some phones, turning off battery saver improves reliability; if payments are still missed, enable "Keep Bitkit active in background" below.

Worth a QA note too: case 5 with battery saver off; Samsung + battery saver on = known limitation for the default (no persistent notification) path.

@piotr-iohk piotr-iohk disabled auto-merge July 2, 2026 09:33
@piotr-iohk

Copy link
Copy Markdown
Collaborator

Disabled auto-merge so there’s room to update settings__bg__enabled if you want

@jvsena42

jvsena42 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Tested again on device (Samsung S22, Android 16).

Observations

Testing "Get paid when Bitkit is closed" ON:

  • Keep Bitkit active OFF + Battery savings OFF: background payments work. Push wake path runs end-to-end (FCM → WakeNodeWorker → payment notification). Matches QA case 5.
  • Keep Bitkit active OFF + Battery savings ON: background payments don't work for me. No reliable push wake in logs; payments only land when the app is open or woken manually.
  • Keep Bitkit active ON + Battery savings ON: works. Node stays up via LightningNodeService and the persistent notification — expected workaround on aggressive OEM power saving.

Attaching session logs. bitkit_logs_2026-07-02_09-06-43.zip

Implementation looks correct to me — approve.

Non-blocking: the text under "Get paid when Bitkit is closed" (settings__bg__enabled) over-promises a bit. It says you can get paid when the app is closed if you're online, but battery saver can block that path even with notifications on. Something like:

Background payments are enabled. You can receive funds when the app is closed, if your device is online. On some phones, turning off battery saver improves reliability; if payments are still missed, enable "Keep Bitkit active in background" below.

Worth a QA note too: case 5 with battery saver off; Samsung + battery saver on = known limitation for the default (no persistent notification) path.

I worth a specific investigation. It is possible to display some warning when the app is in battery save mode and also enable specific permissions to don't limit the app on battery safe mode, but first we need to improve the node consumption.
Also we have we need to be careful not use too long texts

@jvsena42

jvsena42 commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

@jvsena42 jvsena42 merged commit e715043 into master Jul 2, 2026
18 checks passed
@jvsena42 jvsena42 deleted the feat/fg-service-optional branch July 2, 2026 10:07
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.

Polish Notifications Permissions Request

3 participants