feat: implement PushNotificationService with encrypted token registration (Phase 3) #396
Conversation
Add FCMService to handle Firebase Cloud Messaging integration following MIP-05 approach: - Implement background message handler to wake app when notifications arrive - Add FCM token management with automatic refresh handling - Set up foreground and background message listeners - Integrate with existing flutter_background_service for event processing - Add platform checks to skip Firebase on unsupported platforms (Linux, web) - Include
…gging - Add check to prevent duplicate Firebase initialization - Replace logger with debugPrint for better visibility during initialization - Change _messaging from final field to getter for lazy initialization - Add empty stub files for platform-specific Firebase options - Improve initialization flow logging with step-by-step debug messages
Add PushNotificationService to handle encrypted FCM token registration with custom push server: - Implement ECDH + ChaCha20-Poly1305 encryption following MIP-05 privacy approach - Add server public key fetching and initialization flow - Support token registration/unregistration per trade public key - Include platform detection (Android/iOS) in encrypted payload - Add padded payload format to prevent traffic analysis - Integrate service
WalkthroughThis PR completes Phases 2 and 3 of FCM implementation, adds push server URL configuration, introduces PushNotificationService implementing MIP-05 compliant privacy-preserving token management with ECDH encryption, extends FCMService with token refresh notifications, and integrates both services into the app initialization flow via providers. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Initialization
participant FCM as FCMService
participant PushSvc as PushNotificationService
participant PushServer as Push Server
participant Storage as Local Storage
App->>FCM: Initialize FCMService
FCM->>Storage: Get/Store FCM Token
FCM-->>App: FCM Ready + onTokenRefresh callback
App->>PushSvc: Initialize PushNotificationService
PushSvc->>PushServer: GET /api/info
PushServer-->>PushSvc: Server Public Key
PushSvc->>Storage: Store server_pubkey
PushSvc-->>App: PushNotificationService Ready
Note over App: Token refresh occurs
FCM->>FCM: New token received
FCM->>PushSvc: onTokenRefresh(newToken)
PushSvc->>PushSvc: Re-register all known trade pubkeys
PushSvc->>PushSvc: For each trade pubkey:<br/>Encrypt token (ECDH + HKDF + ChaCha20)
PushSvc->>PushServer: POST /api/register<br/>(trade_pubkey, encrypted_token)
PushServer-->>PushSvc: Registration confirmed
PushSvc->>Storage: Track registered trade pubkey
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI Agents
In @lib/main.dart:
- Around line 44-46: The app currently creates FCM/push services twice: once in
_initializeFirebaseMessaging() called from main() and again via
fcmServiceProvider/pushNotificationServiceProvider, causing discarded instances
and uninitialized providers; fix this by having _initializeFirebaseMessaging
return the initialized FCM and PushNotification service instances, then override
fcmServiceProvider and pushNotificationServiceProvider with those returned
instances (e.g., via ProviderScope overrides) before continuing to run the app
so every consumer uses the single initialized instances; alternatively remove
the separate providers and wire consumers to use the instances returned by
_initializeFirebaseMessaging, but do not keep both approaches.
In @lib/services/fcm_service.dart:
- Around line 83-86: The platform guard in initialize() is inconsistent with the
background handler: background handler uses "if (kIsWeb || Platform.isLinux)
return;" but initialize() currently uses "if (!kIsWeb && Platform.isLinux)".
Update the initialize() function in lib/services/fcm_service.dart to skip
initialization on both web and Linux (use the same condition kIsWeb ||
Platform.isLinux), keep the debugPrint message, and return early so behavior
matches the background handler.
- Around line 182-197: The _setupTokenRefreshListener currently only persists
the new FCM token to prefs and leaves a stale TODO about Phase 3; update it to
notify the PushNotificationService so refreshed tokens are re-registered with
the server: add an injectable PushNotificationService (or expose a
callback/stream like _onTokenRefresh) and invoke it when a newToken arrives
after saving to _prefs, ensuring _tokenRefreshSubscription, _fcmTokenKey, and
error handling remain intact and remove the outdated TODO.
- Around line 209-216: The if (isRunning) block around the
FlutterBackgroundService() / isRunning check is a no-op; remove the empty if
branch and its comment (or replace it with the intended behavior such as
returning early or dispatching an event to the running service). Locate the code
that instantiates FlutterBackgroundService() and the await service.isRunning()
call and either delete the whole if (isRunning) { /* comment */ } block or
implement the desired action (e.g., service.sendData(...) or return) so there is
no dead code left.
In @lib/services/push_notification_service.dart:
- Around line 114-116: The debugPrint call uses tradePubkey.substring(0, 16)
which can throw a RangeError for malformed short pubkeys; update the logging to
defensively trim the pubkey (e.g., use a safe truncation that takes
min(tradePubkey.length, 16) or returns the whole string when shorter) and/or
introduce a small helper like shortenPubkey(tradePubkey, 16) and use that in
this debugPrint (and other similar log sites) to avoid crashes while preserving
readable output.
- Around line 262-280: The _createPaddedPayload function can overflow when
tokenBytes.length exceeds available space; add a guard after computing
tokenBytes (and before setRange) that checks tokenBytes.length <=
_paddedPayloadSize - 3 and if not, either throw a clear ArgumentError/RangeError
mentioning deviceToken length or safely truncate the tokenBytes to that maximum
length; ensure the error message references _createPaddedPayload and includes
the offending length and allowed max so callers can handle it.
🧹 Nitpick comments (2)
lib/services/push_notification_service.dart (1)
69-80: Consider validating server public key format before accepting it.The code checks for null/empty but doesn't validate that the public key is a valid 66-character hex string (33-byte compressed EC point). A malformed key will cause a cryptic error later in
_encryptToken. Adding upfront validation improves debuggability.🔎 Suggested validation
if (response.statusCode == 200) { final data = jsonDecode(response.body); _serverPubkey = data['server_pubkey']; - if (_serverPubkey != null && _serverPubkey!.isNotEmpty) { + if (_serverPubkey != null && + _serverPubkey!.length == 66 && + RegExp(r'^[0-9a-fA-F]+$').hasMatch(_serverPubkey!)) { _isInitialized = true; debugPrint('PushService: Initialized successfully'); return true; } else { - _logger.e('Server pubkey is null or empty'); + _logger.e('Server pubkey is invalid: $_serverPubkey'); return false; }lib/services/fcm_service.dart (1)
11-12: Consider standardizing logging approach.The file mixes
debugPrint()(e.g., lines 84, 88, 95) withLoggermethods (e.g., lines 126, 148, 152). Consider using one consistently for easier log filtering and level management.Also applies to: 88-89, 126-127
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/FCM_IMPLEMENTATION.mdlib/core/config.dartlib/firebase_options_platform.dartlib/firebase_options_stub.dartlib/main.dartlib/services/fcm_service.dartlib/services/push_notification_service.dartlib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dart
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/core/config.dartlib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/services/fcm_service.dartlib/services/push_notification_service.dartlib/main.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/core/config.dartlib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/services/fcm_service.dartlib/services/push_notification_service.dartlib/main.dart
lib/shared/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing feature patterns when adding new shared utilities - refer to order, chat, and auth features as implementation examples
Files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/fcm_service.dartlib/services/push_notification_service.dart
lib/main.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Files:
lib/main.dart
🧠 Learnings (19)
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
lib/core/config.dartlib/shared/providers/push_notification_service_provider.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, Riverpod code generation is used with `Riverpod` annotations. Providers like `eventStorageProvider` are generated in `.g.dart` files from annotated functions in the main provider files. These providers are accessible by importing the main provider file (e.g., `mostro_service_provider.dart`), not by importing a separate provider file.
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:26.407Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T12:10:26.407Z
Learning: Applies to **/*.dart : Name Riverpod providers using the `<Feature>Provider` or `<Feature>Notifier` convention
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Organize Riverpod providers by feature in `features/{feature}/providers/` using Notifier pattern for complex state logic
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
Applied to files:
lib/shared/providers/fcm_service_provider.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/shared/providers/providers.dartlib/shared/providers/push_notification_service_provider.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/shared/providers/providers.dartlib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/shared/**/*.dart : Follow existing feature patterns when adding new shared utilities - refer to order, chat, and auth features as implementation examples
Applied to files:
lib/shared/providers/providers.dartlib/main.dart
📚 Learning: 2025-08-21T14:45:43.974Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 272
File: lib/features/relays/widgets/relay_selector.dart:13-15
Timestamp: 2025-08-21T14:45:43.974Z
Learning: In the Mostro mobile app's RelaySelector widget (lib/features/relays/widgets/relay_selector.dart), watching relaysProvider.notifier correctly triggers rebuilds because the relaysProvider itself depends on settingsProvider (line 8 in relays_provider.dart). When blacklist changes via toggleMostroRelayBlacklist(), the settingsProvider updates, causing relaysProvider to rebuild, which then notifies widgets watching the notifier. The UI correctly reflects active/inactive states in real-time through this dependency chain.
Applied to files:
lib/shared/providers/push_notification_service_provider.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/services/fcm_service.dartlib/main.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/services/nostr_service.dart : Manage all relay connections and Nostr messaging through NostrService - automatically reconnect when relay list updates
Applied to files:
lib/services/fcm_service.dart
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to **/*.dart : Always check `mounted` before using BuildContext after async operations to prevent errors on disposed widgets
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Use `S.of(context)!.yourKey` for all user-facing strings instead of hardcoded text
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/screens/**/*.dart : Keep UI code declarative and side-effect free - use post-frame callbacks for side effects like SnackBars/dialogs
Applied to files:
lib/main.dart
📚 Learning: 2025-08-15T01:37:12.243Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 270
File: lib/shared/widgets/order_filter.dart:133-135
Timestamp: 2025-08-15T01:37:12.243Z
Learning: The MostroP2P/mobile project requires Flutter >=3.27.0 as specified in pubspec.yaml, which supports the Color.withValues() method, so usage of withValues() throughout the codebase is valid and should not be flagged as a compatibility issue.
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/data/repositories/**/*.dart : Implement Repository pattern for all data access - all data operations must go through repository classes
Applied to files:
lib/main.dart
🔇 Additional comments (20)
lib/core/config.dart (1)
46-51: LGTM! Push server URL configuration follows established patterns.The new
pushServerUrlconstant correctly usesString.fromEnvironmentfor compile-time configuration, matching the existing pattern formostroPubKey. The default production URL is sensible.lib/services/push_notification_service.dart (5)
23-43: LGTM! Clean service structure with proper dependency injection.The class follows good practices: required FCMService dependency, optional URL override with sensible default fallback to
Config.pushServerUrl, and well-documented constants for the MIP-05 encryption scheme.
153-185: LGTM! Unregister flow is appropriately simpler.The unregistration flow correctly skips initialization checks since it doesn't require encryption—only the trade pubkey is sent. The timeout and error handling are consistent with
registerToken.
195-260: LGTM! Solid MIP-05 encryption implementation.The cryptographic flow correctly implements ECDH key derivation with ephemeral keypairs and ChaCha20-Poly1305 authenticated encryption. The Fortuna PRNG seeding with secure random bytes, proper nonce generation, and standard 128-bit auth tag are all appropriate choices.
282-308: LGTM! HKDF implementation is correct for 32-byte output.The extract-then-expand HKDF construction with SHA256 is properly implemented. The single-iteration expand is sufficient since the requested key length (32 bytes) equals the hash output size.
310-345: LGTM! Helper methods are correctly implemented.The byte manipulation utilities (
_hexToBytes,_randomBytes,_bigIntToBytes,_compressPublicKey) follow standard implementations for cryptographic operations.docs/FCM_IMPLEMENTATION.md (1)
107-108: LGTM! Documentation correctly updated to reflect implementation status.The phase completion markers accurately reflect the implemented functionality in this PR.
lib/shared/providers/providers.dart (1)
3-4: LGTM! Provider exports correctly added to barrel file.The new provider exports follow the existing pattern, making
fcmServiceProviderandpushNotificationServiceProvideraccessible via the shared providers barrel import.lib/shared/providers/fcm_service_provider.dart (1)
5-8: LGTM! Provider follows established patterns.The provider correctly uses
ref.watchto obtainSharedPreferencesAsyncand constructsFCMService. SincesharedPreferencesProvideris overridden at app startup and remains stable, theFCMServiceinstance will be cached by Riverpod. Based on learnings, this follows the<Feature>Providernaming convention.lib/main.dart (1)
93-113: Platform check ordering is correct.Good defensive check:
!kIsWebis evaluated first beforePlatform.isLinux, avoiding the runtime error that would occur ifdart:ioPlatform is accessed on web. The try-catch ensures FCM failures don't crash the app.lib/shared/providers/push_notification_service_provider.dart (1)
5-9: No changes needed. Riverpod'sProvider<T>caches instances by default and only recreates them when watched dependencies change. SincefcmServiceProviderdepends on the stablesharedPreferencesProvider(app singleton),pushNotificationServiceProviderwill maintain a singlePushNotificationServiceinstance throughout the app lifetime. The internal state (_isInitialized,_serverPubkey) is preserved correctly.lib/services/fcm_service.dart (9)
1-9: LGTM!Imports are appropriate for the FCM service functionality.
13-60: LGTM!The background handler properly guards against unsupported platforms, initializes Firebase only when needed, records timestamps for debugging, and sets a pending flag when the background service isn't reachable. Good defensive error handling.
62-76: LGTM!Clean class structure with proper dependency injection for
SharedPreferencesAsync, tracked subscriptions for cleanup, and reasonable timeout/key constants.
132-155: LGTM!Permission request correctly handles both
authorizedandprovisionalstatuses, with proper error handling.
157-180: LGTM!Good timeout handling and graceful degradation when token retrieval fails.
227-237: LGTM!Correctly clears the pending fetch flag set by the background handler. Based on learnings, the background service handles fetching/processing while this just coordinates the wake-up signal.
239-253: LGTM!Good caching strategy - retrieves stored token before making a network call.
255-273: LGTM!Good defensive error handling - ensures local storage cleanup even when the Firebase token deletion fails.
275-282: LGTM!Proper cleanup of subscriptions and state reset.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
lib/services/push_notification_service.dart (4)
36-37: In-memory state is lost on app restart.
_registeredTradePubkeystracking is ephemeral. If the app restarts, re-registration won't occur for previously registered trades until they're registered again through the normal flow.Consider persisting this set (e.g., via
SharedPreferences) if trade pubkeys should survive app restarts, or document this as intentional if the trade flow always re-registers on resumption.
78-89: Consider validating server public key format before storing.The code only checks if
_serverPubkeyis non-null and non-empty, but doesn't validate that it's a valid 66-character hex string (33-byte compressed key). Invalid keys would cause a cryptic error later in_encryptToken.♻️ Suggested validation
if (response.statusCode == 200) { final data = jsonDecode(response.body); _serverPubkey = data['server_pubkey']; - if (_serverPubkey != null && _serverPubkey!.isNotEmpty) { + if (_serverPubkey != null && + _serverPubkey!.length == 66 && + RegExp(r'^[0-9a-fA-F]+$').hasMatch(_serverPubkey!)) { _isInitialized = true; debugPrint('PushService: Initialized successfully'); return true; } else { - _logger.e('Server pubkey is null or empty'); + _logger.e('Server pubkey is invalid (expected 66 hex chars)'); return false; } }
324-350: HKDF implementation only supports single-block expansion.This implementation hardcodes
T[1]expansion and returnsokm.sublist(0, length). It works correctly forlength <= 32(SHA256 output size), but would silently return truncated/invalid output for larger lengths.Since
lengthis always 32 in current usage, this works, but consider either:
- Adding an assertion:
assert(length <= 32)- Implementing full multi-block expansion
♻️ Add safety assertion
Uint8List _hkdfDerive(Uint8List ikm, int length) { + // Single-block HKDF: only supports length <= hash output size + assert(length <= 32, 'HKDF expansion limited to 32 bytes'); + const salt = 'mostro-push-v1'; const info = 'mostro-token-encryption';
144-152: Consider explicit handling for malformed response bodies.The code safely handles unexpected response structures through the outer try-catch block (catching JSON decode exceptions) and by treating missing or false
successfields as failures. However, for better clarity and debugging, consider catching JSON decode exceptions separately and logging the raw response body explicitly, or validating the response structure before accessingdata['success']to distinguish between malformed responses and intentionalsuccess: falseresponses.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/core/config.dartlib/main.dartlib/services/fcm_service.dartlib/services/push_notification_service.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/core/config.dart
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/main.dartlib/services/push_notification_service.dartlib/services/fcm_service.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/main.dartlib/services/push_notification_service.dartlib/services/fcm_service.dart
lib/main.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Configure timeago package locales in app initialization for proper relative time formatting (e.g., 'hace X horas' vs 'hours ago')
Files:
lib/main.dart
lib/services/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Access NostrService for all Nostr protocol interactions - NostrService manages relay connections and messaging
Files:
lib/services/push_notification_service.dartlib/services/fcm_service.dart
🧠 Learnings (6)
📚 Learning: 2025-11-27T12:10:12.081Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.081Z
Learning: Run `flutter test integration_test/` only for significant changes affecting core services and main user flows
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/subscriptions/subscription_manager.dart : Use SubscriptionManager with `fireImmediately: false` during SessionNotifier initialization to prevent premature execution
Applied to files:
lib/main.dartlib/services/fcm_service.dart
📚 Learning: 2025-05-06T15:49:26.443Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/services/mostro_service.dart:70-76
Timestamp: 2025-05-06T15:49:26.443Z
Learning: In the Mostro Mobile codebase, `eventStorageProvider` is exported from `package:mostro_mobile/shared/providers/mostro_service_provider.dart` and not from a separate `event_storage_provider.dart` file.
Applied to files:
lib/main.dart
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/providers/**/*.dart : Use Notifier pattern instead of simple StateNotifier for complex state logic requiring business rule encapsulation
Applied to files:
lib/main.dart
📚 Learning: 2025-11-27T12:10:12.082Z
Learnt from: CR
Repo: MostroP2P/mobile PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-27T12:10:12.082Z
Learning: Applies to lib/features/**/notifiers/**/*.dart : Encapsulate business logic in Notifiers - Notifiers should expose state via providers and handle all complex state transitions
Applied to files:
lib/main.dart
🔇 Additional comments (12)
lib/services/fcm_service.dart (4)
74-77: LGTM! Clean callback pattern for token refresh notification.The nullable callback with null-safe invocation is appropriate for this optional integration point with PushNotificationService.
86-90: LGTM! Platform check now correctly includes web.The initialization correctly skips on both web and Linux platforms with a clear debug message.
194-199: LGTM! Proper error isolation for callback invocation.The try-catch ensures that errors in the callback don't disrupt the token refresh flow.
210-216: LGTM! Simplified foreground handler with clear documentation.The comment explains that the background service subscription mechanism handles event pickup, making the no-op behavior intentional and clear.
lib/main.dart (3)
56-60: LGTM! Conditional provider overrides handle unsupported platforms gracefully.The spread operator with null check ensures providers are only overridden when services are successfully initialized.
98-104: LGTM! Clean encapsulation of related services.The private
_PushServicesclass appropriately groups the FCM and push notification services for the initialization flow.
120-128: LGTM! Proper service initialization and wiring.The sequential initialization of PushNotificationService after FCMService, followed by wiring the token refresh callback, correctly establishes the dependency relationship. The callback intentionally discards the
newTokenparameter sincereRegisterAllTokensfetches the current token internally.lib/services/push_notification_service.dart (5)
16-20: LGTM! Defensive helper for log output.Safely handles short pubkeys to avoid
RangeError, improving logging robustness.
294-322: LGTM! Proper bounds checking and payload construction.The guard against buffer overflow (lines 299-307) is well-implemented with a clear error message. The payload format correctly includes platform byte, big-endian length, token, and random padding.
267-279: LGTM! ChaCha20-Poly1305 encryption correctly configured.The AEAD cipher is properly initialized with the derived key, 128-bit auth tag, nonce, and empty AAD.
380-387: LGTM! Correct EC point compression.The compression correctly uses prefix
0x02for even Y and0x03for odd Y coordinates per SEC1.
231-242: The Fortuna PRNG seeding is correctly implemented and sufficient. The code properly seeds with 32 bytes fromRandom.secure()using theSecureRandom('Fortuna')..seed(KeyParameter(...))pattern, which is the standard practice recommended by PointyCastle's documentation. This approach meets all cryptographic requirements for this use case.Likely an incorrect or invalid review comment.
Description
Implements Phase 3 of FCM integration - encrypted token registration with the Mostro push notification server following MIP-05 privacy approach. The PushNotificationService encrypts FCM device tokens using ECDH key derivation with the server's public key and ChaCha20-Poly1305 encryption, ensuring that tokens are protected in transit and only the notification server can decrypt them. The service fetches the server's public key on initialization and provides methods to register/unregister tokens for specific trades.
Changes
How to Test
Expected output:
Note: Token registration with trades will be tested when integrated with the trade flow. The push server must be running for full testing.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.