Make remote notification work again

[dewabisma]

Summary

• Updated how we parse json for tx event, since FCM seems to stringify each nested object
• Show tx detail on processing tx event
• Add unit tests

Already tested on my iPhone 14 it works!

---

Note

Medium Risk
Medium risk because it changes push-notification/deep-link handling and transaction JSON deserialization (including SDK model parsing), which can affect navigation and transaction display across multiple entry points.

Overview
Restores notification-driven navigation by removing the global navigatorKey pattern and routing everything through Riverpod intent providers.

Push/local notification taps and app launches now set transactionIntentProvider, and HomeScreen consumes intents in initState to open showTransactionDetailSheet (also making the sheet tolerate a missing active account id). Local-notification payload decoding is wrapped with error handling and telemetry.

Improves transaction event parsing to support FCM-style payloads (string-encoded nested objects, int amounts, top-level extrinsicHash) via new SDK helpers in json_dynamic_parse.dart, updates TransferEvent/ReversibleTransferEvent deserialization accordingly, and adds unit tests in both the app and quantus_sdk.

^{Reviewed by Cursor Bugbot for commit b8979cd. Bugbot is set up for automated code reviews on this repo. Configure here.}

[n13]

PR #487 Review — Make remote notification work again

Past discussion of this PR

Overall this is a nice, well-scoped change. Replacing the GlobalKey<NavigatorState>-based navigation with the existing *IntentProvider pattern is consistent with how paymentIntentProvider/sharedAccountIntentProvider already work in HomeScreen, and the new SDK-side json_dynamic_parse.dart helpers neatly DRY up the FCM-vs-REST payload split. The added tests for both shapes are solid.

A few real concerns to look at before merging.

Correctness

1. Cold-start race on transactionIntentProvider (Bugbot's finding is real)

    ref.listen(transactionIntentProvider, (_, transaction) {
      if (transaction == null) return;

      ref.read(transactionIntentProvider.notifier).state = null;
      final active = ref.read(activeAccountProvider).value;

      showTransactionDetailSheet(context, transaction, active?.account.accountId);
    });

ref.listen only fires on changes that happen after subscription. On a true cold start the order is:

1. _ResonanceWalletAppState.initState schedules a post-frame callback.
2. First frame → WalletInitializer renders the loading spinner and starts its async _checkWalletAndMigration / migration prompt.
3. Post-frame fires → handleLaunchByNotification() and setupNotificationTapHandlers() race the wallet init.
4. WalletInitializer finally swaps to HomeScreen, which subscribes.

If FCM/local-notification resolution wins the race (small payload, no migration, slow keystore read), transactionIntentProvider gets set to a non-null value before HomeScreen's ref.listen exists, and the sheet never opens. iPhone 14 happens to be on the "wallet init wins" side, but this is environment-dependent (Android cold start, migration prompt path, slow keychain, etc.).

The reply on the PR says ref.listen has no fireImmediately, but it does in flutter_riverpod ≥ 2.0 (WidgetRef.listen(provider, listener, {bool fireImmediately = false, ...})). The minimal fix:

ref.listen(transactionIntentProvider, (_, transaction) { ... }, fireImmediately: true);

A more robust alternative is to drain the provider once after HomeScreen mounts:

@override
void initState() {
  super.initState();
  WidgetsBinding.instance.addPostFrameCallback((_) {
    final pending = ref.read(transactionIntentProvider);
    if (pending != null) {
      ref.read(transactionIntentProvider.notifier).state = null;
      showTransactionDetailSheet(context, pending, ref.read(activeAccountProvider).value?.account.accountId);
    }
  });
}

(fireImmediately is the smaller change and matches the pattern.)

2. deserializeTxEventFromJsonIfPossible swallows errors silently

  TransactionEvent? deserializeTxEventFromJsonIfPossible(dynamic json) {
    final txType = json['type'];
    TransactionEvent? event;

    try {
      if (txType == EventType.TRANSFER.name) {
        event = TransferEvent.fromJson(json);
      } else if (txType == EventType.REVERSIBLE_TRANSFER.name) {
        event = ReversibleTransferEvent.deserializeFromJson(json);
      } else if (txType == EventType.PENDING_TRANSACTION.name) {
        event = PendingTransactionEvent.fromJson(json);
      }
    } catch (e) {
      print('Failed deserializing event: $e');
    }

    return event;
  }

With the new strict helpers in json_dynamic_parse.dart (FormatException on unexpected shapes/types), any backend or FCM payload change will be caught here and reduced to a print. Per the "fail early, never silent failures" rule, this should at least go through TelemetryService().sendError(...) so the next payload regression doesn't quietly break tx-detail navigation again. Bonus: include the offending txType/keys in the report.

3. navigateToTransactionFromPayloadIfPossible declares a nullable Map but immediately indexes into it

  void navigateToTransactionFromPayloadIfPossible(Map<String, dynamic>? json) {
    final event = deserializeTxEventFromJsonIfPossible(json);
    ...
  }

deserializeTxEventFromJsonIfPossible does json['type'] on the first line, which throws on null. Either drop the ? (callers in the diff never pass null), or null-guard at the top of deserializeTxEventFromJsonIfPossible. Right now the ? is misleading.

4. LocalNotificationsService jsonDecode(payload) is unguarded

  Future<void> handleLaunchByNotification() async {
    ...
    final json = jsonDecode(payload);
    txService.navigateToTransactionFromPayloadIfPossible(json);
  }

If the payload ever isn't valid JSON, this throws into Flutter's zoned error handler and the launch flow dies silently. Either wrap it (with a telemetry log per #2) or assert the shape. Same in setupNotificationsClickListener.

Smaller things

5. Dead routes after this PR

'/account' and '/transactions' in app.dart both => WalletInitializer() and were only reached via the now-removed navigatorKey.currentState?.pushNamed(...) calls. Worth deleting them and the 'app_links' route comment to avoid future confusion.

6. _TransactionDetailSheet with activeAccountId == null is half-wired

  String get _title {
    if (activeAccountId == null) return 'Transaction Details';
    if (_isPending) return 'Sending';
    if (tx.isReversibleScheduled) return _isSend ? 'Scheduled' : 'Receiving';
    return _isSend ? 'Sent' : 'Received';
  }

Only the title falls back. _isSend (used by _statusColor, _DetailsSection, _AmountSection.colorizeAmount) silently becomes false because tx.from == null is always false, so a notification opened while the active account is still loading will always render as "received from tx.from" even when the user actually sent it. Given the intent is to surface this from a notification tap, it's worth either:

• waiting until activeAccountProvider has a value before showing the sheet, or
• threading the role decision through _isSend as bool? and rendering a neutral layout.

The simplest defense is in HomeScreen: if active == null, listen-and-re-fire instead of opening the sheet immediately.

7. unregisterDevice clears the cache even on failure

  Future<void> unregisterDevice() async {
    final token = await getDeviceToken();
    ...
    try {
      _cachedToken = null;
      await _senotiService.unregisterDevice(token, _platform);
    } catch (e) {
      debugPrint('Failed to unregister device: $e');
    }
  }

If the backend call throws, we've already discarded the token; next getDeviceToken() will fetch a fresh one and unregisterDevice will then try to delete a token the server never knew about. If clearing on failure is intentional, fine — otherwise move _cachedToken = null after the await.

Nice things

• json_dynamic_parse.dart is a clean shared layer; TransferEvent.fromJson and ReversibleTransferEvent.fromJson now read almost identically.
• The new tests cover REST nested-map, FCM string-encoded, integer amounts, and extrinsicHash at both nesting levels — exactly the cases that were broken.
• Dropping the global navigatorKey is a real improvement and matches the existing intent-provider pattern.

Suggested merge bar

Blocking: address #1 (race) and at least #2 (silent error). #3, #4, #5, #6 are small follow-ups but easy to fold into this PR. #7 is a judgment call.

Happy to draft any of these fixes if you want — just say the word.

[n13]

^^ Let's avoid any post frame callbacks - that almost always is a bug. There's always a race condition.

Anything that can be done in post frame callback should be done in another way.

[n13]

I fixed some issues

[n13]

Reviewed PR #487 — the diff, current files in repo, the bugbot findings, and my previous review on this PR (Past discussion of this PR). Below is a focused follow-up against the latest state of the branch — the ref.listenManual + fireImmediately: true refactor in HomeScreen.initState has landed since my earlier pass, so several of those concerns are resolved. Some real issues remain.

What's now solid

• The cold-start race is fixed by ref.listenManual<TransactionEvent?>(transactionIntentProvider, ..., fireImmediately: true) in _HomeScreenState.initState. Same fix applied to paymentIntentProvider and sharedAccountIntentProvider — nice cleanup, those used to live in build.
• Title falls back to 'Transaction Details' when activeAccountId == null.
• quantus_sdk/lib/src/models/json_dynamic_parse.dart is a clean, DRY layer for the FCM-stringified-vs-REST-nested split. TransferEvent.fromJson / ReversibleTransferEvent.fromJson now read almost identically.
• Tests cover REST nested map, FCM-style stringified objects, int amounts, and extrinsicHash at both nesting levels — those were exactly the broken cases.
• Dropping the global navigatorKey is a real architectural win and matches the existing intent-provider pattern.

Still blocking

1. Silent error swallow in deserializeTxEventFromJsonIfPossible

  TransactionEvent? deserializeTxEventFromJsonIfPossible(dynamic json) {
    final txType = json['type'];
    TransactionEvent? event;

    try {
      if (txType == EventType.TRANSFER.name) {
        event = TransferEvent.fromJson(json);
      } else if (txType == EventType.REVERSIBLE_TRANSFER.name) {
        event = ReversibleTransferEvent.deserializeFromJson(json);
      } else if (txType == EventType.PENDING_TRANSACTION.name) {
        event = PendingTransactionEvent.fromJson(json);
      }
    } catch (e) {
      print('Failed deserializing event: $e');
    }

    return event;
  }

The whole point of this PR is that a backend/FCM payload change silently broke notification navigation. With the new strict FormatException helpers, any next regression lands here and becomes a print. Per the project rule "fail early, never silent failures", this should go through TelemetryService().sendError(...) (and debugPrint not print), including txType and the payload keys. Otherwise we're rebuilding the exact same trap.

2. _isSend is half-wired when activeAccountId is null

  bool get _isSend => tx.from == activeAccountId;
  bool get _isPending => tx is PendingTransactionEvent;

  String get _title {
    if (activeAccountId == null) return 'Transaction Details';
    ...
  }

Only the title falls back. _isSend is consumed by _statusColor, _DetailsSection (which picks tx.to vs tx.from for the address row), and _AmountSection.colorizeAmount. When activeAccountId == null, _isSend silently becomes false, so a notification opened while activeAccountProvider is still loading always renders as "Received from tx.from" — even when the user is actually the sender.

Two clean options:

• Resolve the role from accountsProvider instead of activeAccountProvider (the user owns the address — we know which side from accountsProvider.notifier.getAccountWithId(...), which FirebaseMessagingService._remoteMessageToNotificationData is already doing).
• In HomeScreen's listener, gate on active != null and re-fire when active becomes available, instead of opening the sheet with a missing identity.

3. navigateToTransactionFromPayloadIfPossible(Map<String, dynamic>? json) is misleading

  void navigateToTransactionFromPayloadIfPossible(Map<String, dynamic>? json) {
    final event = deserializeTxEventFromJsonIfPossible(json);
    ...
  }

deserializeTxEventFromJsonIfPossible does json['type'] on its first line, which throws on null. Either drop the ? (every caller in the diff passes a non-null Map/decoded payload), or null-guard at the top of deserializeTxEventFromJsonIfPossible. Currently the ? lies about safety.

4. Unguarded jsonDecode in the local-notifications path

  Future<void> handleLaunchByNotification() async {
    final notificationAppLaunchDetails = await _notificationPlugin.getNotificationAppLaunchDetails();
    if (notificationAppLaunchDetails?.didNotificationLaunchApp != true) return;

    final payload = notificationAppLaunchDetails!.notificationResponse?.payload;
    if (payload == null || payload.isEmpty) return;

    final txService = _ref.read(transactionServiceProvider);
    final json = jsonDecode(payload);

    txService.navigateToTransactionFromPayloadIfPossible(json);
  }

Same in setupNotificationsClickListener. If the payload is ever malformed, this throws into the zoned error handler and the launch flow dies silently. Wrap with telemetry (folds into #1) or assert the shape and bail with a log.

Smaller stuff (folder for a follow-up)

5. Dead routes in app.dart

      initialRoute: '/',
      routes: {
        '/': (context) => const WalletInitializer(),
        '/account': (context) => const WalletInitializer(),
        '/transactions': (context) => const WalletInitializer(),
      },

Both deep-link routes were only ever reached via the now-removed navigatorKey.currentState?.pushNamed(...). DeepLinkService resolves links through app_links and sets intent providers directly — these route entries (and the misleading comment that was removed) can go.

6. unregisterDevice clears the cache before the backend call

  Future<void> unregisterDevice() async {
    final token = await getDeviceToken();
    if (token == null) { ... return; }
    try {
      _cachedToken = null;
      await _senotiService.unregisterDevice(token, _platform);
    } catch (e) { debugPrint('Failed to unregister device: $e'); }
  }

If the backend call throws, we've already discarded the token; the next getDeviceToken() will fetch a fresh one and we'll then try to delete a token the server never knew about. If clearing on failure is intentional, fine — otherwise move _cachedToken = null after the await.

7. Nits

• print('Failed deserializing event: $e') → debugPrint, and likewise the two print('Received link …') in DeepLinkService. Stays out of release logs and matches the rest of the file.
• bool __hasRegisteredHandlers in FirebaseMessagingService has a double underscore — Dart convention is single underscore for private.
• In transactionIntentProvider flow, ref.read(transactionIntentProvider.notifier).state = null; runs before the sheet opens — good, prevents re-fire — worth a one-line // clear before showing so dismissing -> reopening home doesn't refire if you want, but I'd keep it uncommented per the no-narration rule.

json_dynamic_parse.dart specifics

Map<String, dynamic>? jsonMapOrNull(dynamic value) {
  if (value == null) return null;
  if (value is Map<String, dynamic>) return value;
  if (value is Map) return Map<String, dynamic>.from(value);
  if (value is String) {
    final trimmed = value.trim();
    if (trimmed.isEmpty) return null;
    final decoded = jsonDecode(trimmed);
    ...
  }
  throw FormatException('Expected Map or JSON object string, got ${value.runtimeType}');
}

• jsonDecode here can also throw FormatException on malformed JSON — that's correct behavior (fail early), it just stacks with Rust Bindings #1 (the swallow upstream).
• nestedAccountId returns '' on missing id — that matches the previous json['from']?['id'] as String? ?? '' behavior, so I'd leave it. Worth a /// doc note that empty string means "unknown holder", since downstream _isSend comparisons rely on string equality.
• dateTimeFromJson only accepts String / DateTime. FCM keeps ISO strings, so fine — but worth a comment that millis-int is intentionally not supported so a future contributor doesn't "helpfully" add it without test coverage.

Suggested merge bar

• Blocking: Rust Bindings #1 (silent swallow) and Feature/dilithium signatures #2 (_isSend when active is null).
• Trivial to fold in this PR: Refactor1 #3, Fix warnings1 #4, Refactor screens #5.
• New design #6/Pull actual fee #7 are judgment calls.

[cursor]

Null activeAccountId causes incorrect send/receive display

Low Severity

When activeAccountId is null, _isSend silently defaults to false, causing _AmountSection to always colorize the amount as received and _DetailsSection to always show the "FROM" label with tx.from. The _title getter properly guards against null with a generic "Transaction Details" string, but this guard isn't propagated to the child widgets that rely on _isSend. On cold start from a notification tap, activeAccountProvider may still be loading when the listenManual callback fires with fireImmediately: true, making this a reachable path.

Additional Locations (1)

• mobile-app/lib/v2/screens/home/home_screen.dart#L47-L53

 

^{Reviewed by Cursor Bugbot for commit 19d0556. Configure here.}

[n13]

active account ID can't be null here should not be optional

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b8979cd. Configure here.}

[cursor]

Showing UI from listenManual fireImmediately during initState

Medium Severity

The ref.listenManual calls with fireImmediately: true in initState will synchronously invoke the callback if the provider already holds a non-null value. This attempts to show a bottom sheet or push a navigation route (showTransactionDetailSheet, Navigator.push) during initState, before the widget's first build has completed. On a cold-start from a notification, handleLaunchByNotification (async, fire-and-forget from app.dart) could resolve and populate transactionIntentProvider before HomeScreen is created, triggering immediate UI display during construction.

Additional Locations (1)

• mobile-app/lib/v2/screens/home/home_screen.dart#L54-L65

 

^{Reviewed by Cursor Bugbot for commit b8979cd. Configure here.}

[n13]

Explaining this AI code

Here is the precise flow when the user taps a push notification while the app is fully terminated, mapped to the current code on beast/remote-notification-update with our fixes applied.

Phase 1 — OS launches the process

The OS hands the Flutter engine the notification metadata. Two channels coexist:

• FCM remote notification → available later via FirebaseMessaging.getInitialMessage().
• Local notification (foreground-shown FCM is rebuilt as a local notification, plus any other local ones) → available later via FlutterLocalNotificationsPlugin.getNotificationAppLaunchDetails().

Crucially, neither is consumed synchronously. They both have to wait for WidgetsFlutterBinding.ensureInitialized.

Phase 2 — main() synchronous setup

void main() async {
  WidgetsFlutterBinding.ensureInitialized();
  ...
  await QuantusSdk.init();
  Telemetrydecksdk.start(...);
  runApp(
    const ProviderScope(
      child: AppInitializer(child: AppLifecycleManager(child: ResonanceWalletApp())),
    ),
  );
}

After runApp, three widget tree levels mount in this order: AppInitializer → AppLifecycleManager → ResonanceWalletApp → MaterialApp → WalletInitializer (initial route '/').

Phase 3 — initState chain at boot

These run in widget-tree order, all before a single pixel is painted:

1. AppInitializer.initState kicks off async _initialize():

   • Subscribes to remoteConfigProvider; if enableRemoteNotifications flips true it will later call fcmService.init() and fcmService.setupNotificationTapHandlers().
   • await notificationService.init() (local notifications).
   • ref.read(historyPollingManagerProvider).

2. ResonanceWalletApp.initState (mobile-app/lib/app.dart):

   void initState() {
     super.initState();
     ref.read(notificationIntegrationServiceProvider);
     ref.read(deepLinkServiceProvider).init();
     final localNotifications = ref.read(localNotificationsServiceProvider);
     localNotifications.setupNotificationsClickListener();
     localNotifications.handleLaunchByNotification();
     ...
   }

   handleLaunchByNotification() is async fire-and-forget. It will eventually decode the payload and call txService.navigateToTransactionFromPayloadIfPossible(json), which sets transactionIntentProvider.state = event.

3. MaterialApp renders WalletInitializer as the initial route, which immediately starts _checkWalletAndMigration() and shows a CircularProgressIndicator while _loading == true (mobile-app/lib/wallet_initializer.dart:195).

Phase 4 — The race

By design, several async tasks now run concurrently while the spinner is on screen:

Task	Effect
WalletInitializer._checkWalletAndMigration	reads secure storage, decides HomeScreen vs WelcomeScreen vs migration sheet
localNotifications.handleLaunchByNotification	decodes payload, sets transactionIntentProvider
FCM _handleInitialMessage (once setupNotificationTapHandlers runs via remoteConfigProvider)	decodes RemoteMessage.data, sets transactionIntentProvider
activeAccountProvider	asynchronously loads the active DisplayAccount

There is no single "winner" we can guarantee — it depends on keychain latency, whether migration runs, Firebase warm-up cost, Android vs iOS, etc. This is exactly the race n13 flagged.

Phase 5 — HomeScreen.initState (the part our fix touches)

@override
void initState() {
  super.initState();

  ref.listenManual<TransactionEvent?>(transactionIntentProvider, _onTransactionIntent);
  ref.listenManual<PaymentIntent?>(paymentIntentProvider, _onPaymentIntent);
  ref.listenManual<String?>(sharedAccountIntentProvider, _onSharedIntent);
  ref.listenManual<AsyncValue<DisplayAccount?>>(activeAccountProvider, (_, async) {
    if (async.value == null) return;
    _onTransactionIntent(null, ref.read(transactionIntentProvider));
  });

  Future.microtask(_drainPendingIntents);
}

Two distinct mechanisms are wired up here:

• The three intent listeners catch any state change that happens after HomeScreen mounted. No fireImmediately, so they will not run synchronously inside initState. This is what fixes bugbot's "Showing UI from listenManual fireImmediately during initState" finding.
• The activeAccountProvider listener is the "second-chance" path: even if a tx intent is already set but activeAccountProvider is still loading, we will re-attempt the navigation the moment the account resolves.

Then Future.microtask(_drainPendingIntents) schedules a one-shot drain to run after initState returns but before the next event-loop turn. That handles the cold-start case where the intent was already set before HomeScreen ever subscribed.

void _onTransactionIntent(TransactionEvent? _, TransactionEvent? transaction) {
  if (transaction == null || !mounted) return;
  final active = ref.read(activeAccountProvider).value;
  if (active == null) return;
  ref.read(transactionIntentProvider.notifier).state = null;
  showTransactionDetailSheet(context, transaction, active.account.accountId);
}

The intent is only cleared after we have both a non-null transaction and a non-null active account — so we never silently drop the intent (n13's correctness concern), and the sheet is never opened with a null identity (n13's _isSend concern).

Phase 6 — HomeScreen.build (first frame)

HomeScreen.build runs immediately after initState. It paints the empty balance skeleton + activity skeleton (because balanceDisplayProvider, activeAccountTransactionsProvider are still loading).

Important: at this point no modal sheet has been pushed yet. The microtask has not fired. This is why removing fireImmediately: true matters — without that change, showTransactionDetailSheet (which calls showModalBottomSheet) would have been invoked synchronously inside initState, while the widget had no rendered subtree.

Phase 7 — The microtask fires (post-build, pre-next-frame)

_drainPendingIntents() runs. Four outcomes depending on what raced:

Case A — Intent already set, active already loaded (the "iPhone 14 happy path"):

• Reads transactionIntentProvider → non-null event.
• Reads activeAccountProvider.value → non-null account.
• Clears intent, calls showTransactionDetailSheet.
• User sees HomeScreen with the transaction detail sheet animated up over it.

Case B — Intent already set, active still loading (slow keychain / migration path / Android cold start):

• _onTransactionIntent early-returns because active == null. Intent stays set.
• A bit later, ActiveAccountNotifier emits its DisplayAccount? value.
• The listenManual<AsyncValue<DisplayAccount?>>(activeAccountProvider, …) listener fires, reads the still-pending intent, calls _onTransactionIntent again — this time active is non-null, intent clears, sheet shows.
• User briefly sees HomeScreen skeleton, then the sheet pops in.

Case C — Intent set after HomeScreen mounted (FCM init was slow, or a runtime tap):

• The microtask sees no pending intent.
• Later, transactionIntentProvider flips → the dedicated listenManual<TransactionEvent?> fires → same _onTransactionIntent path runs.

Case D — Intent set after HomeScreen mounted but active still loading:

• Tx-intent listener fires → active null → returns without clearing.
• Active loads → active-account listener re-drains → sheet shows.

Why this is correct

1. No lost intents. Across every race ordering, the intent is only cleared at the moment we actually call showTransactionDetailSheet. Either the tx-intent listener or the active-account listener (whichever happens to be the second one to receive its value) drives the sheet.
2. No UI from initState. Future.microtask runs after initState returns and after the synchronous build step is queued, so showModalBottomSheet always finds a fully constructed Navigator. No fireImmediately: true anywhere.
3. No addPostFrameCallback. Per your rule. The microtask handles "after current sync stack" without any frame-callback dependency.
4. No _isSend ambiguity. showTransactionDetailSheet and _TransactionDetailSheet now take a non-nullable String activeAccountId. The compiler enforces that we never call it without identity, so _isSend, _DetailsSection's TO/FROM label, and _AmountSection's colorization are all consistent.
5. No telemetry payload leak. If deserialization fails, we send e.runtimeType.toString() (e.g. "FormatException") plus the controlled txType enum name — never the FormatException message, which would have included the raw payload value via the quantus_sdk json helpers.
6. Pattern parity. Payment intents (deep-link "pay" URLs) and shared-account intents (deep-link "share" URLs) follow the exact same listen + drain shape, so we get the same race-safety for free across all three entry points.

What the user actually sees, in order

1. OS launch animation / native splash.
2. ~100–600ms of WalletInitializer's CircularProgressIndicator.
3. HomeScreen rendered with empty balance + activity skeletons.
4. TransactionDetailSheet animates up from the bottom — either immediately on the same frame (Case A/C), or after activeAccountProvider resolves (Case B/D, typically a few hundred ms later).
5. When the user dismisses the sheet, they're left on HomeScreen with balance/activity populated normally.

[n13]

Now I have a comprehensive understanding of the PR. Here's my review:

---

PR #487 Review: "Make remote notification work again"

Author: @dewabisma | Branch: beast/remote-notification-update -> main | +352 / -93 across 12 files

Summary

This PR replaces the global navigatorKey navigation pattern with Riverpod intent-based navigation. Instead of pushing named routes directly from notification handlers, the services now set state on transactionIntentProvider, and HomeScreen consumes those intents to show UI. It also fixes FCM payload parsing (FCM stringifies nested objects) and adds tests.

---

Things I Like

1. Removing global_navigator_key.dart - Global navigator keys are a well-known anti-pattern in Flutter. Using Riverpod state providers to express navigation intent is architecturally superior.

2. json_dynamic_parse.dart - Clean utility module that centralizes the logic for handling FCM's quirky serialization (string-encoded nested objects, int amounts). Well-structured with proper error messages.

3. Error handling + telemetry in LocalNotificationsService - Previously jsonDecode failures were silent. Now they log and report.

4. Removed addPostFrameCallback - Aligns with your project rule to avoid post frame callbacks.

5. Good test coverage - Both SDK-level and app-level unit tests covering REST-style and FCM-style payloads.

---

Issues / Concerns

1. Race condition: intent consumed before HomeScreen is built

In _HomeScreenState.initState(), Future.microtask(_drainPendingIntents) will fire in the same microtask queue. If a cold-launch notification sets transactionIntentProvider before HomeScreen mounts, _drainPendingIntents should catch it. But if the intent is set after initState but before activeAccountProvider resolves, it'll be silently dropped (the if (active == null) return; guard in _onTransactionIntent swallows it without re-queuing).

The activeAccountProvider listener partially addresses this by re-checking the transaction intent when the account loads — that's good. But there's still a window: if the intent arrives after the account loaded but before listenManual fires its first callback (e.g., between frames), it could be missed. In practice this is likely fine for the happy path, but worth noting.

2. _cachedToken = null side-effect in unregisterDevice

      await _senotiService.unregisterDevice(token, _platform);
      _cachedToken = null;

This is correct behavior (clearing the cache after unregister), but the method accepts token as a parameter from the caller. If the caller passed a stale token different from _cachedToken, you'd still null out _cachedToken. It works today because unregisterDevice is only called with _cachedToken, but it's fragile. Minor nit.

3. navigateToTransactionFromPayloadIfPossible signature change

The parameter changed from Map<String, dynamic>? (nullable) to Map<String, dynamic> (non-null). All call-sites already null-check before calling, so this is fine — but it's a public API change on TransactionService that could bite external callers of this service if any exist.

4. ReversibleTransferEvent.fromJson uses jsonMapRequired for block

This throws a FormatException if block is null/empty. Previously it was a hard cast (as Map<String, dynamic>) which would also throw. Same behavior, but it means a ReversibleTransferEvent without a block field will crash instead of gracefully falling back. Given this is a required field for reversible transfers, that's likely intentional — just confirming.

5. Removed routes /account and /transactions

      routes: {'/': (context) => const WalletInitializer()},

These routes were previously used for deep linking. The PR now routes everything through intent providers, which is fine — but make sure no other code (e.g., web URLs registered in AndroidManifest/Info.plist) still references these named routes. If something externally tries to navigate to /transactions, it'll get a route-not-found error.

---

Minor Nits

• The __hasRegisteredHandlers (double underscore) → _hasRegisteredHandlers rename is a nice cleanup.
• nestedAccountId handles sender/receiver as new field names alongside legacy from/to — good backward compat.
• Test file at mobile-app/test/unit/transaction_service_test.dart doesn't test ReversibleTransferEvent deserialization with FCM-style payloads — might want to add that later.

---

Verdict

Solid PR that fixes a real problem (notifications broken due to navigator key timing issues) with a better architectural pattern. The FCM payload parsing is well-handled. The main thing to verify is the cold-start race condition around intent delivery vs. account loading, which the active-account listener mostly covers. Ship it with confidence.

[n13]

App init race conditions are always a bit of a pain but I think it's good enough for now

LGTM