Skip to content

feat(mobile): local notifications for incoming transfers and completed transactions - #666

Closed
nonchalanttee22-lgtm wants to merge 3 commits into
Miracle656:mainfrom
nonchalanttee22-lgtm:main
Closed

feat(mobile): local notifications for incoming transfers and completed transactions#666
nonchalanttee22-lgtm wants to merge 3 commits into
Miracle656:mainfrom
nonchalanttee22-lgtm:main

Conversation

@nonchalanttee22-lgtm

Copy link
Copy Markdown
Contributor

Summary

The Veil mobile app had no notifications — users only discovered incoming transfers or confirmed transactions by manually opening the app. This PR adds local-only notifications via expo-notifications fired by the existing activity poller, with the Veil drape mark as the notification icon, a settings toggle, and full respect for app-lock and hidden-amounts mode. Push/remote notifications are explicitly out of scope.

Closes #630


What changed

Notification infrastructure

  • Installed and configured expo-notifications — requires a new dev build (permissions and notification icon cannot be added OTA)
  • Added the Veil drape mark as a dedicated notification icon asset under assets/images/ in the correct format and resolution for Android and iOS notification trays — does not reuse the existing app icon

Activity poller integration

  • Hooked into the existing activity poller so notification checks run on the same cadence as the feed refresh — no new polling loop or background fetch introduced
  • On each poll, the poller compares the incoming transaction set against the last-seen state:
    • New incoming transfer detected → fire local notification
    • Pending outgoing transaction reaches a final state (confirmed or failed) → fire local notification
  • Notifications are deduplicated against a persisted set of already-notified transaction IDs so reopening the app or backgrounding/foregrounding does not re-fire stale events

Notification content — app-lock and hidden-amounts

  • While the wallet is locked: notification body says "You received a transfer" with no amount, asset, or sender disclosed
  • While hidden-amounts mode is on: same redacted copy regardless of lock state
  • When unlocked and hidden-amounts is off: notification may include the asset type (e.g. "You received XLM") but never the amount in the notification directly — amount is available on tap-to-open
  • Outgoing confirmed: "Your transaction was confirmed" / failed: "Your transaction failed" — no amounts disclosed in either case

Settings toggle

  • Added a notifications settings section with two toggles:
    • Incoming transfers — defaults to on
    • Transaction status updates — defaults to off (chatty; opt-in)
  • Settings persisted to the existing local store alongside other user preferences
  • Permission request is shown once on first toggle-on; if denied, the toggle reflects the OS permission state and links to system settings

Notification icon

  • Veil drape mark added as assets/images/notification-icon.png at the required dimensions
  • Registered in app.json / app.config.ts as the Android notification icon and the iOS notification image — Expo default icon is not used

How to verify

  • Build a new dev build (notification permissions require it — cannot test OTA)
  • Background the app, send a transfer to the wallet address from an external account, and confirm a notification appears in the system tray with the Veil mark and no amount
  • Lock the wallet, trigger an incoming transfer, and confirm the notification body discloses no amount, asset details, or sender
  • Enable hidden-amounts mode (unlocked), trigger a transfer, and confirm the notification is equally redacted
  • Toggle incoming transfer notifications off in settings and confirm no notification fires on the next transfer
  • Toggle transaction status updates on, sign an outgoing transaction, and confirm a notification fires when it reaches a final state
  • Confirm no duplicate notifications fire for the same transaction across multiple poll cycles

Checklist

  • expo-notifications configured and registered in app config
  • New dev build required — noted in PR and release notes
  • Veil drape mark added as dedicated notification icon asset at correct dimensions
  • Notification icon registered in app.json/app.config.ts for Android and iOS
  • Activity poller fires notification on new incoming transfer
  • Activity poller fires notification on pending outgoing transaction reaching final state
  • Notifications deduplicated — no re-fire for already-notified transactions
  • App-lock: no amount, asset, or sender disclosed in notification body
  • Hidden-amounts mode: same redaction applied regardless of lock state
  • Settings toggle for incoming transfers (default on) and transaction status (default off)
  • Permission request shown once; denied state reflected in toggle with link to system settings
  • No push/remote notification infrastructure introduced — local only
  • Closes Mobile: local notifications for incoming transfers and completed transactions #630

@drips-wave

drips-wave Bot commented Aug 27, 2026

Copy link
Copy Markdown

@nonchalanttee22-lgtm Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@vercel

vercel Bot commented Aug 27, 2026

Copy link
Copy Markdown

@nonchalanttee22-lgtm is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Miracle656

Copy link
Copy Markdown
Owner

The notification work here is good — lib/notifications.ts, lib/notificationPrefs.ts, the useNotifications hook and the settings toggle are about 440 lines of real, well-separated code, and wiring prefs separately from delivery is the right split. I want to merge it.

One thing blocks it, and it is not your fault.

The root package-lock.json

This PR carries package-lock.json at +20,100/-655, and it is not cosmetic churn:

  • main has 81 resolved packages in that file; this branch has 1,418
  • Versions move throughout — 0.8.1 → 2.3.0, 3.1.2 → 0.3.31, 1.5.5 → 0.113.0

That would change what npm ci installs for everyone, from a PR about mobile notifications.

Please restore it from main and I will merge straight away:

git checkout origin/main -- package-lock.json
git commit -m "revert unrelated root lockfile churn"

Your intended lockfile change is frontend/mobile/package-lock.json (+107), which is correct and should stay — you added a dependency there.

Why this happened, and why you are the second person to hit it

The repo root declares workspaces: ["sdk", "packages/*"] but its committed lockfile only describes the root's own 81 packages. So running npm install at the repo root — the obvious thing to do — resolves every workspace and rewrites the file with 1,418 entries.

#663 hit the identical thing, byte for byte the same +20,100/-655. That is the repo's problem, not yours, and I have opened an issue for it so the next person does not lose time the same way.

Everything else here looks ready.

Miracle656 added a commit that referenced this pull request Aug 27, 2026
Notification work lands as-is: lib/notifications.ts, lib/notificationPrefs.ts,
the useNotifications hook, the settings toggle and the app.config.ts wiring.
Splitting preferences from delivery is the right shape.

Dropped one thing from the merge: the root package-lock.json, which the branch
carried at +20,100/-655. That is not churn — it takes the root from 81 resolved
packages to 1,418 with versions moving throughout, so it would change what
npm ci installs for everyone, from a PR about mobile notifications. Restored
from main. frontend/mobile/package-lock.json is kept, since expo-notifications
is a real new dependency there.

The contributor did nothing wrong: running npm install at the repo root
rewrites that file, because the root declares workspaces its committed lockfile
does not describe. #663 hit the identical diff. Tracked in #670.

Verified: the three-way merge preserves #674's TxDetailSheet theming (2
useTheme, 0 hardcoded colours) and the current welcome headline. Mobile tsc
shows the 4 pre-existing sdk/src errors plus one for expo-notifications, which
is only because the new dependency is not installed in this working copy — it
is declared in package.json and present in the mobile lockfile.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
@Miracle656

Copy link
Copy Markdown
Owner

Your work is merged. It went in as 017b8celib/notifications.ts, lib/notificationPrefs.ts, the useNotifications hook, the settings toggle, the app.config.ts wiring and the expo-notifications dependency are all on main now.

I merged it by hand rather than through the button so I could drop one file: the root package-lock.json. Your frontend/mobile/package-lock.json was kept, because expo-notifications is a real new dependency there.

To be clear, that was not a mistake on your part — running npm install at the repo root rewrites that file, because the root declares workspaces its committed lockfile does not describe. It takes it from 81 resolved packages to 1,418 with versions moving throughout, so it would have changed what npm ci installs for everyone. #663 produced the byte-identical diff. It is our bug, tracked in #670.

This PR is now showing as open only because that lockfile is the single remaining difference between your branch and main. You have since merged Miracle656:main in, so everything else matches.

Two ways to finish it, your call — I am not closing it on you:

  • Close it, since the work is already in.
  • Or run git checkout origin/main -- package-lock.json, commit, and it will close itself when there is nothing left to differ.

I also checked the merge did not undo anything recent: TxDetailSheet.tsx keeps #674's theming (2 useTheme, no hardcoded colours) and the welcome headline is intact.

Thanks — splitting preferences from delivery was the right call, and it made the merge painless.

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.

Mobile: local notifications for incoming transfers and completed transactions

2 participants