lib/core/config.dart (1)

12-26: Pubkey is duplicated, not derived — risk of silent divergence.

The comment on Line 21 says "derived from first trusted node," but mostroPubKey uses a hardcoded string identical to the one in trustedMostroNodes. Dart const context prevents runtime derivation from the list. If someone updates one location and forgets the other, the values silently diverge.

Consider adding a comment explicitly noting this coupling, or extracting the hex string into a single private constant referenced by both.

Suggested approach

+  // Canonical pubkey for the primary Mostro node — used in both
+  // trustedMostroNodes and as the default for mostroPubKey.
+  static const String _defaultMostroPubKey =
+      '82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390';
+
   // Trusted Mostro nodes registry
   static const List<Map<String, String>> trustedMostroNodes = [
     {
-      'pubkey':
-          '82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390',
+      'pubkey': _defaultMostroPubKey,
       'name': 'Mostro P2P',
     },
   ];

-  // Mostro hexkey (backward compatible, derived from first trusted node)
+  // Mostro hexkey (backward compatible)
   static const String mostroPubKey = String.fromEnvironment(
     'MOSTRO_PUB_KEY',
-    defaultValue:
-        '82fa8cb978b43c79b2156585bac2c011176a21d2aead6d9f7c575c005be88390',
+    defaultValue: _defaultMostroPubKey,
   );

lib/features/mostro/mostro_nodes_provider.dart (1)

1-10: File placement doesn't match feature folder convention.

The provider file is at lib/features/mostro/mostro_nodes_provider.dart but the coding guidelines specify organizing providers under features/{feature}/providers/. Similarly, the notifier and model would go under their respective subdirectories. Consider moving to:

• lib/features/mostro/providers/mostro_nodes_provider.dart
• lib/features/mostro/notifiers/mostro_nodes_notifier.dart
• lib/features/mostro/models/mostro_node.dart

This can be deferred but worth tracking. As per coding guidelines: "Use feature-based folder structure with organization as features/{feature}/{screens|providers|notifiers|widgets}/".

lib/features/mostro/mostro_node.dart (1)

1-8: Consider making metadata fields final for a true value-object.

name, picture, website, and about are non-final, yet the class already provides withMetadata() for immutable-style updates, and the notifier always replaces nodes in state via withMetadata(). Making these fields final would enforce the immutable pattern consistently and prevent accidental in-place mutation.

♻️ Suggested diff

 class MostroNode {
   final String pubkey;
-  String? name;
-  String? picture;
-  String? website;
-  String? about;
+  final String? name;
+  final String? picture;
+  final String? website;
+  final String? about;
   final bool isTrusted;
   final DateTime? addedAt;

test/features/mostro/mostro_node_test.dart (1)

93-105: withMetadata cannot clear a field back to null — consider whether a test should document that.

Because withMetadata uses name: name ?? this.name, there's no way for a caller to explicitly reset a field to null. If that's intentional, a brief test documenting the behavior (e.g., "calling withMetadata(name: null) preserves the existing name") would make the contract explicit. If clearing is needed in Phase 2 for kind 0 metadata, the signature will need a sentinel or a separate method.

lib/features/mostro/mostro_node.dart (1)

1-8: Consider making metadata fields final for a fully immutable value object.

name, picture, website, and about are non-final, yet the class provides withMetadata for creating updated copies and the MostroNodesNotifier always replaces nodes in state (never mutating in place). Mutable fields invite accidental direct mutation that would bypass Riverpod state notifications.

♻️ Suggested change

 class MostroNode {
   final String pubkey;
-  String? name;
-  String? picture;
-  String? website;
-  String? about;
+  final String? name;
+  final String? picture;
+  final String? website;
+  final String? about;
   final bool isTrusted;
   final DateTime? addedAt;

lib/features/mostro/mostro_nodes_notifier.dart (3)

200-211: _saveCustomNodes rethrows — callers propagate unhandled errors.

addCustomNode, removeCustomNode, and updateCustomNodeName all await _saveCustomNodes(...) without try-catch. If persistence fails, the rethrown exception bubbles up to the caller (likely UI code). This is arguably the correct design (persist-before-state ensures consistency), but the public method signatures return Future<bool> / Future<void> with no documentation that they can throw on I/O failure. Consider either:

1. Catching in the public methods, logging, and returning false / failing gracefully, or
2. Documenting the throwing behavior so callers know to handle it.

---

65-81: selectNode doesn't handle errors from updateMostroInstance.

If updateMostroInstance throws (e.g., SharedPreferences failure), the error propagates unhandled. A try-catch with logging would keep this consistent with the defensive pattern used elsewhere.

🛡️ Suggested fix

   Future<void> selectNode(String pubkey) async {
     MostroNode? node;
     for (final n in state) {
       if (n.pubkey == pubkey) {
         node = n;
         break;
       }
     }
     if (node == null) {
       logger.w('Cannot select unknown node: $pubkey');
       return;
     }
 
-    await _ref.read(settingsProvider.notifier).updateMostroInstance(pubkey);
-    logger.i('Selected Mostro node: ${node.displayName}');
+    try {
+      await _ref.read(settingsProvider.notifier).updateMostroInstance(pubkey);
+      logger.i('Selected Mostro node: ${node.displayName}');
+    } catch (e) {
+      logger.e('Failed to select node ${node.displayName}: $e');
+      rethrow;
+    }
   }

---

171-175: Getter allocations on every access — acceptable but worth noting.

trustedNodes and customNodes create a new filtered list on every call. Methods like addCustomNode (line 96-98) and removeCustomNode (line 123-125) call these getters and then immediately spread them into a new state list. With small node counts this is fine, but if this pattern is reused in hot paths later, consider caching.

lib/features/mostro/mostro_nodes_notifier.dart (2)

1-14: File should live under notifiers/ subfolder per project conventions.

The coding guidelines require the feature-based folder structure features/{feature}/{screens|providers|notifiers|widgets}/. This notifier should be at lib/features/mostro/notifiers/mostro_nodes_notifier.dart.

As per coding guidelines: lib/features/**/*.dart: Use feature-based folder structure with organization as features/{feature}/{screens|providers|notifiers|widgets}/

---

135-148: updateCustomNodeName silently no-ops for trusted nodes.

If called with a trusted node's pubkey, customNodes won't include it, so the map produces no change — yet _saveCustomNodes and a full state rebuild still run. Consider either guarding early (like removeCustomNode does) or documenting this is by-design.

🛡️ Optional early guard

  Future<void> updateCustomNodeName(String pubkey, String newName) async {
+   if (!customNodes.any((n) => n.pubkey == pubkey)) {
+     logger.w('Cannot update name: $pubkey is not a custom node');
+     return;
+   }
    final updatedCustom = customNodes.map((n) {