Skip to content

Conversation

@jvsena42
Copy link
Member

@jvsena42 jvsena42 commented May 2, 2025

Attach the node instance to a foreground service so it keeps running when the app is in background

  • Move shared states and logic from WalletViewModel to the repositories
  • Implement the WalletService
  • Deprecate WalletViewModel

Related to #50
Closes #76

payment_background.webm

@jvsena42 jvsena42 self-assigned this May 2, 2025
@jvsena42 jvsena42 changed the title Keep node running in background Keep node running when the app is in background May 2, 2025
@jvsena42 jvsena42 marked this pull request as ready for review May 9, 2025 10:38
@jvsena42 jvsena42 requested a review from ovitrif May 9, 2025 10:38
Copy link
Collaborator

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks a lot for your perseverance on this, it reached a pretty nice shape 🚀

Overall, everything that was pointed out in previous review is fixed very nice and a little more, with great care 🙌. Also, the wallet sync is more robust, as an additional improvement alongside what the FG service adds.

💡 remark: (added an issue for it: #134)

Maybe one last optimization of big impact for users on my mind, but not blocking here:

Having the notification show up without wallet info, (ie. on fresh install with no wallet or after wallet wipe) feels a bit strange from the user’s perspective IMHO. Maybe we should look if there’s an optimal solution to run & stop it in connection to whether there is a wallet or not.

Test Results 2

  • ✅ send onchain, LN
  • ✅ CJIT: BG receive (via wake2) - after node stop via notification
  • ✅ UX to stop node android-service via notification
  • ✅ receive: LN, LN in bg via node FG service
  • ✅ balance updated correctly after: restore, create, fund - sync seems faster 👏
  • ✅ on fresh install > create wallet > fund > mine - all good
  • ✅ node stopped when wiping wallet + great logging 👏
  • ✅ node lifecycle is synced to to wallet exists state
  • ✅ register for notifications - works great
  • ✅ pull-to-refresh ux 👏

ovitrif
ovitrif previously approved these changes May 9, 2025
@ovitrif ovitrif self-requested a review May 9, 2025 15:47
@ovitrif ovitrif merged commit b623227 into master May 9, 2025
1 check passed
@ovitrif ovitrif deleted the feat/keep-node-in-background branch May 9, 2025 15:48
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.

Node stops as soon as app goes to background

3 participants