Fix/pubkey formats #597
Fix/pubkey formats #597
Conversation
WalkthroughIntroduces a new PubkeyFormatter and replaces many legacy npub/hex conversion callsites. ActivePubkeyNotifier now accepts an injectable formatter and stores hex pubkeys. Multiple providers, UI components, domain models, services, and tests were updated to use the formatter for normalization and comparisons. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI
participant Notifier as ActivePubkeyNotifier
participant Storage as AccountSecureStorageService
participant Formatter as PubkeyFormatter
rect #F5F8FF
note left of Notifier: Initialize / Load active pubkey
UI->>Notifier: build() / loadActivePubkey()
Notifier->>Storage: getActivePubkey()
Storage-->>Notifier: storedPubkey?
Notifier->>Formatter: toHex(pubkey)
Formatter-->>Notifier: hexOrNull
alt hexOrNull != null
Notifier-->>UI: state = hex
else
Notifier-->>UI: state = ''
end
end
rect #F5FFF5
note left of Notifier: Set or clear active pubkey
UI->>Notifier: setActivePubkey(inputPubkey)
Notifier->>Formatter: toHex(inputPubkey.trim())
Formatter-->>Notifier: hexOrNull
alt hexOrNull != null
Notifier->>Storage: setActivePubkey(hex)
Notifier-->>UI: state = hex
else
Notifier->>Storage: clearActivePubkey()
Notifier-->>UI: state = ''
end
end
sequenceDiagram
autonumber
actor Caller
participant Provider as MetadataCacheProvider
participant Formatter as PubkeyFormatter
participant API as MetadataSource
Caller->>Provider: request metadata for key
Provider->>Formatter: toNpub(inputKey)
Formatter-->>Provider: npubOrNull
alt npubOrNull present
Provider->>API: fetch metadata by hex (Formatter.toHex(npub))
API-->>Provider: metadata
Provider-->>Caller: ContactModel (with standardized npub)
else
Provider-->>Caller: fallback (original input)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
josefinalliende
force-pushed
the
fix/pubkey-formats
branch
from
0c1da4d to
842ddec
Compare
josefinalliende
force-pushed
the
fix/pubkey-formats
branch
4 times, most recently
from
21356bb to
0646ebc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (14)
lib/utils/public_key_validation_extension.dart (1)
21-23: Make npub validation case-insensitive and more robust.Current checks accept any string >10 chars starting with 'npub1' (lowercase only). This yields false positives and rejects uppercase Bech32. Prefer a stricter check (case-insensitive) or delegate to a real decoder (e.g., via PubkeyFormatter/Rust).
Example minimal fix:
- if (trimmed.startsWith('npub1') && trimmed.length > 10) { + final lower = trimmed.toLowerCase(); + if (lower.startsWith('npub1') && lower.length > 10) { return true; }Optionally, replace the check with:
return PubkeyFormatter(pubkey: trimmed).toHex() != null;if dependency direction is acceptable.Also applies to: 36-39
lib/ui/contact_list/group_welcome_invitation_sheet.dart (3)
129-133: Avoid double-formatting; return raw key and format only at the call site.Currently, the catch block returns an already formatted string, which then gets formatted again in the FutureBuilder, leading to double-ellipsis. Return the raw key on both paths and format once where displayed.
- final npub = PubkeyFormatter(pubkey: widget.welcome.nostrGroupId).toNpub() ?? ''; - return npub; + return PubkeyFormatter(pubkey: widget.welcome.nostrGroupId).toNpub() + ?? widget.welcome.nostrGroupId; } catch (e) { - return widget.welcome.nostrGroupId.formatPublicKey(); + return widget.welcome.nostrGroupId; }
208-214: Wrong fallback for group invite key; should use groupId, not welcomer.When the future hasn't completed, fallback should be the group id, not the welcomer key.
- final displayKey = npubSnapshot.data ?? widget.welcome.welcomer; + final displayKey = npubSnapshot.data ?? widget.welcome.nostrGroupId;
299-303: Same double-formatting issue in DM card; return raw key and format once.- final npub = PubkeyFormatter(pubkey: widget.welcome.welcomer).toNpub() ?? ''; - return npub; + return PubkeyFormatter(pubkey: widget.welcome.welcomer).toNpub() + ?? widget.welcome.welcomer; } catch (e) { - return widget.welcome.welcomer.formatPublicKey(); + return widget.welcome.welcomer; }lib/domain/models/contact_model.dart (2)
36-46: Do not default to empty publicKey on conversion failure. Preserve identity.Falling back to '' risks collisions and lookup bugs. If toNpub() fails or returns empty, keep the original input.
Apply:
- final npub = PubkeyFormatter(pubkey: pubkey).toNpub() ?? ''; + final String? npub = PubkeyFormatter(pubkey: pubkey).toNpub(); + final String safeNpub = (npub != null && npub.isNotEmpty) ? npub : pubkey; @@ - publicKey: npub, + publicKey: safeNpub,
75-85: Equality/hashCode contract broken; normalize consistently and handle nulls.== compares normalized hex, but hashCode uses raw publicKey. Equal objects can yield different hashes, breaking Maps/Sets. Also, toHex() may return null leading to accidental equality.
Apply:
- final hexNpub = PubkeyFormatter(pubkey: publicKey).toHex(); - final otherHexNpub = PubkeyFormatter(pubkey: other.publicKey).toHex(); - - return hexNpub == otherHexNpub && + final String selfHex = PubkeyFormatter(pubkey: publicKey).toHex() ?? publicKey; + final String otherHex = PubkeyFormatter(pubkey: other.publicKey).toHex() ?? other.publicKey; + + return selfHex == otherHex && other.imagePath == imagePath && other.displayName == displayName && other.about == about && other.website == website && other.nip05 == nip05 && other.lud16 == lud16; @@ - return Object.hash(publicKey, imagePath, displayName, about, website, nip05, lud16); + final String hex = PubkeyFormatter(pubkey: publicKey).toHex() ?? publicKey; + return Object.hash(hex, imagePath, displayName, about, website, nip05, lud16);Also applies to: 88-90
lib/domain/services/dm_chat_service.dart (1)
15-21: Handle nullable toNpub() and add explicit typing.toNpub() can be null; passing null into getOtherGroupMember is risky. Also prefer explicit types per guidelines.
- final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); + final String currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub() ?? '';lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
97-104: Treat null/empty toHex() as a signal to fallback to direct comparison.Current logic returns false on empty conversion and never hits the catch.
- final String profileHex = PubkeyFormatter(pubkey: profile.publicKey).toHex() ?? ''; - return profileHex == _activeAccountHex; + final String? profileHex = PubkeyFormatter(pubkey: profile.publicKey).toHex(); + final String candidate = (profileHex != null && profileHex.isNotEmpty) + ? profileHex + : profile.publicKey; + return candidate == _activeAccountHex;lib/ui/settings/general_settings_screen.dart (1)
151-166: Add missingcollectionimport forfirstOrNull
firstOrNullcomes frompackage:collectionbut isn’t imported here. Add at the top:import 'package:collection/collection.dart';lib/config/providers/metadata_cache_provider.dart (3)
69-76: Do not return empty string on failed hex→npub conversion (cache-key collision).Returning '' when
toNpub()yields null will collapse unrelated entries under the same cache key and corrupt lookups. Fall back to the original hex instead.Apply:
Future<String> _safeHexToNpub(String hexPubkey) async { try { - return PubkeyFormatter(pubkey: hexPubkey).toNpub() ?? ''; + final npub = PubkeyFormatter(pubkey: hexPubkey).toNpub(); + return (npub != null && npub.isNotEmpty) ? npub : hexPubkey; } catch (e) { _logger.warning('Failed to convert hex to npub for $hexPubkey: $e'); return hexPubkey; } }
79-86: Do not return empty string on failed npub→hex conversion (bad fetchKey, inconsistent state).Same issue as above; fall back to the original npub string to avoid passing '' to downstream calls and map keys.
Apply:
Future<String> _safeNpubToHex(String npub) async { try { - return PubkeyFormatter(pubkey: npub).toHex() ?? ''; + final hex = PubkeyFormatter(pubkey: npub).toHex(); + return (hex != null && hex.isNotEmpty) ? hex : npub; } catch (e) { _logger.warning('Failed to convert npub to hex for $npub: $e'); return npub; } }
109-111: Use the validator instead of string prefix for bech32 detection.
startsWith('npub1')is brittle (length, checksum, case nuances). Prefer the validation extension you already import.Apply:
- if (publicKey.startsWith('npub1')) { + if (publicKey.isValidNpubPublicKey) { fetchKey = await _safeNpubToHex(publicKey); }test/config/providers/active_pubkey_provider_test.dart (2)
82-88: Test name does not match expectation (null vs empty string).The body expects
isNullbut the title says “sets state to empty string”. Pick one to avoid confusion.Apply:
- test('sets state to empty string', () async { + test('keeps state as null', () async {If provider semantics should actually coerce to
'', change the assertion instead:expect(state, '');.
345-350: Same mismatch: title says “updates state to empty string” but assertion expects null.Align the title with the assertion (or flip the assertion if behavior should be
'').Apply:
- test('updates state to empty string', () async { + test('keeps state as null', () async {
♻️ Duplicate comments (3)
test/config/providers/group_provider_test.dart (3)
192-209: Same FRB-guard dedup as above.
221-238: Same FRB-guard dedup as above.
240-260: Same FRB-guard dedup as above.
🧹 Nitpick comments (25)
test/config/providers/profile_ready_card_visibility_provider_test.dart (1)
149-152: Avoid calling notifier.build() directly in tests.Initialize via the provider to keep Riverpod lifecycle intact.
- notifier = container.read(profileReadyCardVisibilityProvider.notifier); - notifier.build(); - await Future.delayed(Duration.zero); + await container.read(profileReadyCardVisibilityProvider.future); + notifier = container.read(profileReadyCardVisibilityProvider.notifier);test/config/providers/welcomes_provider_test.dart (1)
750-752: Fix misleading comment ("both pending").welcome_4 is ignored in test data; update the comment to avoid confusion.
- // Set up multiple pending welcomes - final pendingWelcomes = [testWelcome1, testWelcome4]; // both pending + // Set up a list containing one pending and one ignored welcome + final pendingWelcomes = [testWelcome1, testWelcome4];test/config/providers/active_account_provider_test.dart (1)
428-428: Remove stray blank lines inside tests.Guideline: don't leave blank lines within a function.
- expect( () => notifier.updateMetadata(metadata: newMetadata), throwsA(isA<Exception>()), );Repeat the same removal for the listed locations.
Also applies to: 443-443, 507-507, 522-522, 542-542, 562-562, 586-586
lib/ui/chat/invite/chat_invite_screen.dart (2)
181-181: Prefer a non-empty fallback when formatting fails.Showing an empty string hurts UX; fall back to truncated raw ID.
- PubkeyFormatter(pubkey: welcome.nostrGroupId).toNpub()?.formatPublicKey() ?? '', + (PubkeyFormatter(pubkey: welcome.nostrGroupId).toNpub() ?? welcome.nostrGroupId) + .formatPublicKey(),
283-283: Same fallback for welcomer pubkey.Keep display consistent if conversion fails.
- PubkeyFormatter(pubkey: welcome.welcomer).toNpub()?.formatPublicKey() ?? '', + (PubkeyFormatter(pubkey: welcome.welcomer).toNpub() ?? welcome.welcomer) + .formatPublicKey(),lib/ui/chat/chat_info/widgets/member_action_buttons.dart (1)
109-120: Prefer using existing notifier method for consistency.Reuse
followsProvider.notifier.isFollowing(...)to centralize comparison logic and avoid format drift here.lib/ui/contact_list/group_welcome_invitation_sheet.dart (1)
127-134: Optional: Make these helpers synchronous and remove FutureBuilder churn.toNpub() is synchronous; these functions need not be async/Future. This will simplify UI and avoid unnecessary rebuild complexity.
If desired, I can provide a small refactor patch across both call sites.
Also applies to: 297-304
lib/config/providers/chat_provider.dart (1)
425-429: Nit: simplify groupId selection for readability.Avoid assigning to the parameter. Use a local with a simple null-coalescing expression.
- final gId = groupId ??= state.selectedGroupId; + final String? gId = groupId ?? state.selectedGroupId;lib/domain/services/dm_chat_service.dart (1)
26-30: Minor naming fixes for clarity.Use concise names and fix typo.
- final otherMemberNpubPubkey = PubkeyFormatter(pubkey: otherMemberPubkey).toNpub() ?? ''; + final String otherMemberNpub = PubkeyFormatter(pubkey: otherMemberPubkey).toNpub() ?? ''; @@ - pubkey: otherMemberNpubPubkey, + pubkey: otherMemberNpub, @@ - final npup = contactModel.publicKey; + final String npub = contactModel.publicKey; @@ - publicKey: npup, + publicKey: npub,Also applies to: 34-40
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
133-141: Optional: robust fallback in sorting when cached hex is missing/empty.Prevents '' from ranking unpredictably if any slipped through.
Outside this hunk, consider:
final aHex = (_pubkeyToHex[a.publicKey]?.isNotEmpty ?? false) ? _pubkeyToHex[a.publicKey]! : a.publicKey; final bHex = (_pubkeyToHex[b.publicKey]?.isNotEmpty ?? false) ? _pubkeyToHex[b.publicKey]! : b.publicKey;lib/ui/settings/general_settings_screen.dart (1)
233-235: Variable name doesn’t match behavior (naming nit).
hasMultipleAccounts = accounts.length > 2means “3+ accounts.” If that’s intended (only show switcher when there are 3+), consider renaming tohasThreeOrMoreAccounts. If you truly mean “2+,” change to>= 2.- final hasMultipleAccounts = accounts.length > 2; + final hasThreeOrMoreAccounts = accounts.length > 2;test/config/providers/group_provider_test.dart (2)
170-191: Deduplicate FRB-guard try/catch into a helper to reduce noise.
Multiple tests repeat the same guard logic.Add a small helper at top of file:
Future<T?> guardFrb<T>(Future<T> Function() fn) async { try { return await fn(); } catch (e) { if (e.toString().contains('flutter_rust_bridge has not been initialized')) { return null; // treat as skipped } rethrow; } }Then:
- try { - final regularGroups = await notifier.getGroupsByType(GroupType.group); + final regularGroups = await guardFrb(() => notifier.getGroupsByType(GroupType.group)); + if (regularGroups == null) return;
639-669: Reasonable skip strategy for FRB-required test.
If you adopt the helper, this block simplifies further.test/utils/pubkey_formatter_test.dart (1)
1-165: Solid unit coverage for conversions and trimming.
Consider adding cases for empty string and uppercase hex to lock in edge behavior.lib/config/providers/active_pubkey_provider.dart (1)
34-38: Normalize-on-read should also persist to storage (optional).
If stored value wasn’t hex, write back the normalized hex to enforce the “storage is hex” invariant.Future<void> loadActivePubkey() async { final pubkey = await AccountSecureStorageService.getActivePubkey(storage: storage); final hexPubkey = pubkeyFormatter(pubkey: pubkey).toHex(); - state = hexPubkey; + state = hexPubkey; + if (hexPubkey != null && pubkey != null && pubkey != hexPubkey) { + await AccountSecureStorageService.setActivePubkey(hexPubkey, storage: storage); + } }lib/config/providers/group_provider.dart (2)
158-159: Consider null-safety improvements for toNpub() calls.While the current implementation works, consider handling the nullable return from
toNpub()more explicitly. The current code falls back to an empty string which could cause issues downstream.- final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); - final otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); + final otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + + if (currentUserNpub == null || otherUserNpub == null) { + _logger.warning('Failed to convert pubkeys to npub format'); + return null; + }
882-885: Extract common conversion logic to reduce duplication.The conversion from npub to hex for member lists is duplicated between
addToGroupandremoveFromGroupmethods.Consider extracting a helper method:
List<String> _convertNpubsToHex(List<String> npubs) { return npubs.map((npub) { final hex = PubkeyFormatter(pubkey: npub).toHex(); if (hex == null) { _logger.warning('Failed to convert npub to hex: $npub'); } return hex ?? ''; }).where((hex) => hex.isNotEmpty).toList(); }Then use it in both methods:
- final usersPubkeyHex = - membersNpubs.map((userNpub) { - return PubkeyFormatter(pubkey: userNpub).toHex() ?? ''; - }).toList(); + final usersPubkeyHex = _convertNpubsToHex(membersNpubs);Also applies to: 936-939
lib/utils/pubkey_formatter.dart (2)
17-34: Consider adding debug logging for conversion failures.The
toNpub()method silently returns null on conversion failures. Consider adding debug logging to help diagnose format conversion issues in production.} catch (e) { + // Log at debug level to avoid noise in production + // but help diagnose conversion issues during development + assert(() { + print('PubkeyFormatter: Failed to convert hex to npub: $e'); + return true; + }()); return null; }
27-31: Consider preserving the original exception for debugging.When catching exceptions during format conversion, the original error information is lost. Consider using a logger or at least preserving the error in debug mode.
} catch (e, stackTrace) { + assert(() { + print('PubkeyFormatter conversion error: $e\n$stackTrace'); + return true; + }()); return null; }Also applies to: 46-50
lib/ui/chat/chat_info/group_chat_info.dart (2)
69-82: Consider extracting hex conversion for better performance.The sorting logic performs multiple hex conversions for the same pubkeys in nested loops. Consider converting once and caching the results.
// Sort members: admins first (A-Z), then regular members (A-Z), current user last + // Pre-convert keys to hex for efficient comparison + final adminHexKeys = admins.map((admin) => + PubkeyFormatter(pubkey: admin.publicKey).toHex() ?? '' + ).toSet(); + final hexCurrentUserNpub = currentUserNpub != null ? + PubkeyFormatter(pubkey: currentUserNpub).toHex() : null; + allMembers.sort((a, b) { final hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex(); final hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex(); - final hexCurrentUserNpub = PubkeyFormatter(pubkey: currentUserNpub).toHex(); - final aIsAdmin = admins.any((admin) { - final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); - return hexAdminKey == hexPubkeyA; - }); - final bIsAdmin = admins.any((admin) { - final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); - return hexAdminKey == hexPubkeyB; - }); + final aIsAdmin = adminHexKeys.contains(hexPubkeyA); + final bIsAdmin = adminHexKeys.contains(hexPubkeyB); final aIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyA); final bIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyB);
232-240: Optimize member list tile admin check.Similar to the sorting logic, the admin check in the list tile builder performs redundant conversions for each member.
Pass the pre-computed admin hex keys set to the builder:
Widget _buildMemberListTile(User member, {String? currentUserNpub}) { + // Consider accepting adminHexKeys as a parameter instead + final adminHexKeys = groupAdmins.map((admin) => + PubkeyFormatter(pubkey: admin.publicKey).toHex() ?? '' + ).toSet(); + final hexMemberKey = PubkeyFormatter(pubkey: member.publicKey).toHex(); + final isAdmin = hexMemberKey != null && adminHexKeys.contains(hexMemberKey); - final isAdmin = groupAdmins.any((admin) { - final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); - final hexMemberKey = PubkeyFormatter(pubkey: member.publicKey).toHex(); - return hexAdminKey == hexMemberKey; - });lib/config/providers/metadata_cache_provider.dart (3)
120-139: Guard against empty/invalid standardized keys before constructing ContactModel.Even with safer converters, protect against accidental empty keys reaching the model.
Apply:
- final standardNpub = await _getStandardizedNpub(publicKey); + final standardNpub = await _getStandardizedNpub(publicKey); + if (standardNpub.isEmpty) { + _logger.warning('Standardized npub is empty for $publicKey; using normalized input instead.'); + // Keep normalized input to avoid collisions. + // _normalizePublicKey is cheap; call again for clarity. + return ContactModel( + displayName: 'Unknown User', + publicKey: _normalizePublicKey(publicKey), + ); + }Please confirm
ContactModel.fromMetadatatolerates non-bech32pubkeyvalues, or if it requires npub only.
24-24: Improve testability by injecting a clock.
DateTime.now()hard-codes time and makes cache expiry hard to unit-test. Consider injecting aClockor passingnowvia constructor forCachedMetadata.isExpired.I can draft a small
NowProviderand wire it here if you want.
63-66: Minor: Remove blank lines within functions per repo style.Several functions contain internal blank lines contrary to the guidelines. Not critical, but easy to tidy up.
Want a quick formatting diff for these sections?
Also applies to: 88-100, 150-175
test/config/providers/active_pubkey_provider_test.dart (1)
127-141: Optional: Strengthen assertions and intent.
- Also
verify()storage writes where relevant (e.g., onsetActivePubkey).- Use
isFalse/isTruematchers consistently.Example:
- expect(wasNotified, false); + expect(wasNotified, isFalse);+ verify(mockStorage.write(key: 'active_account_pubkey', value: testHexPubkey)).called(1);Also applies to: 184-231, 243-269, 296-335, 392-407, 475-502
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
lib/config/providers/active_pubkey_provider.dart(3 hunks)lib/config/providers/auth_provider.dart(2 hunks)lib/config/providers/chat_provider.dart(2 hunks)lib/config/providers/group_provider.dart(9 hunks)lib/config/providers/metadata_cache_provider.dart(3 hunks)lib/config/providers/nostr_keys_provider.dart(2 hunks)lib/domain/models/contact_model.dart(3 hunks)lib/domain/services/dm_chat_service.dart(2 hunks)lib/ui/chat/chat_info/chat_info_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(4 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/chat/chat_info/widgets/member_action_buttons.dart(2 hunks)lib/ui/chat/invite/chat_invite_screen.dart(3 hunks)lib/ui/chat/widgets/chat_header_widget.dart(2 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(3 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(2 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)lib/ui/settings/profile/share_profile_screen.dart(2 hunks)lib/ui/settings/profile/switch_profile_bottom_sheet.dart(3 hunks)lib/utils/pubkey_formatter.dart(1 hunks)lib/utils/public_key_validation_extension.dart(1 hunks)test/config/providers/active_account_provider_test.dart(12 hunks)test/config/providers/active_pubkey_provider_test.dart(9 hunks)test/config/providers/active_pubkey_provider_test.mocks.dart(2 hunks)test/config/providers/group_provider_test.dart(9 hunks)test/config/providers/profile_ready_card_visibility_provider_test.dart(1 hunks)test/config/providers/welcomes_provider_test.dart(2 hunks)test/ui/contact_list/services/welcome_notification_service_simple_test.dart(1 hunks)test/utils/pubkey_formatter_test.dart(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
test/config/providers/profile_ready_card_visibility_provider_test.darttest/ui/contact_list/services/welcome_notification_service_simple_test.dartlib/ui/chat/chat_info/widgets/member_action_buttons.dartlib/ui/chat/chat_info/chat_info_screen.darttest/config/providers/welcomes_provider_test.darttest/config/providers/active_pubkey_provider_test.mocks.dartlib/ui/settings/general_settings_screen.dartlib/config/providers/chat_provider.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/domain/services/dm_chat_service.dartlib/ui/chat/chat_info/dm_chat_info.darttest/config/providers/active_account_provider_test.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/invite/chat_invite_screen.dartlib/utils/public_key_validation_extension.dartlib/config/providers/auth_provider.dartlib/domain/models/contact_model.dartlib/config/providers/metadata_cache_provider.dartlib/ui/settings/profile/share_profile_screen.darttest/utils/pubkey_formatter_test.darttest/config/providers/group_provider_test.dartlib/config/providers/active_pubkey_provider.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
**/*_test.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*_test.dart: Follow the Arrange-Act-Assert convention for tests.
Name test variables clearly. Follow the convention: inputX, mockX, actualX, expectedX, etc.
Write unit tests for each public function. Use test doubles to simulate dependencies, except for third-party dependencies that are not expensive to execute.
Write acceptance tests for each module. Follow the Given-When-Then convention.
Use the standard widget testing for Flutter.
Files:
test/config/providers/profile_ready_card_visibility_provider_test.darttest/ui/contact_list/services/welcome_notification_service_simple_test.darttest/config/providers/welcomes_provider_test.darttest/config/providers/active_account_provider_test.darttest/utils/pubkey_formatter_test.darttest/config/providers/group_provider_test.darttest/config/providers/active_pubkey_provider_test.dart
🧠 Learnings (16)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.905Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.179Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/active_account_provider.dart:140-146
Timestamp: 2025-09-05T00:08:34.466Z
Learning: josefinalliende prefers centralizing validation logic (like trimming whitespace) in providers rather than handling it defensively at every consumer site. This approach was applied to activePubkeyProvider in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.135Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.136Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Use the standard widget testing for Flutter.
Applied to files:
test/ui/contact_list/services/welcome_notification_service_simple_test.darttest/config/providers/welcomes_provider_test.darttest/config/providers/active_pubkey_provider_test.mocks.darttest/config/providers/active_account_provider_test.dart
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/ui/chat/chat_info/widgets/member_action_buttons.dartlib/ui/settings/general_settings_screen.dartlib/config/providers/chat_provider.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/domain/services/dm_chat_service.dartlib/ui/chat/chat_info/dm_chat_info.dartlib/ui/chat/chat_info/group_chat_info.dartlib/utils/public_key_validation_extension.dartlib/config/providers/auth_provider.dartlib/domain/models/contact_model.dartlib/config/providers/metadata_cache_provider.dartlib/ui/settings/profile/share_profile_screen.darttest/utils/pubkey_formatter_test.dartlib/config/providers/active_pubkey_provider.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-09-07T02:15:31.905Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.905Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/chat/chat_info/chat_info_screen.darttest/config/providers/welcomes_provider_test.darttest/config/providers/active_pubkey_provider_test.mocks.dartlib/ui/settings/general_settings_screen.dartlib/config/providers/chat_provider.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/invite/chat_invite_screen.dartlib/utils/public_key_validation_extension.dartlib/config/providers/auth_provider.dartlib/config/providers/metadata_cache_provider.dartlib/ui/settings/profile/share_profile_screen.darttest/utils/pubkey_formatter_test.darttest/config/providers/group_provider_test.dartlib/config/providers/active_pubkey_provider.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/chat/chat_info/chat_info_screen.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-01T14:56:50.988Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#568
File: lib/ui/core/ui/wn_image.dart:1-2
Timestamp: 2025-09-01T14:56:50.988Z
Learning: The whitenoise_flutter project does not target web platforms and any future web version would be in a separate repository, so dart:io imports and file system operations are acceptable in this codebase.
Applied to files:
test/config/providers/active_pubkey_provider_test.mocks.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Write unit tests for each public function. Use test doubles to simulate dependencies, except for third-party dependencies that are not expensive to execute.
Applied to files:
test/config/providers/active_pubkey_provider_test.mocks.darttest/utils/pubkey_formatter_test.dart
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Applied to files:
test/config/providers/active_pubkey_provider_test.mocks.dartlib/ui/settings/general_settings_screen.dartlib/config/providers/chat_provider.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/domain/services/dm_chat_service.dartlib/ui/chat/chat_info/dm_chat_info.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/invite/chat_invite_screen.dartlib/utils/public_key_validation_extension.dartlib/config/providers/auth_provider.dartlib/domain/models/contact_model.dartlib/config/providers/metadata_cache_provider.dartlib/ui/settings/profile/share_profile_screen.darttest/utils/pubkey_formatter_test.dartlib/config/providers/active_pubkey_provider.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-09-01T14:56:47.293Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#568
File: lib/ui/core/ui/wn_image.dart:1-2
Timestamp: 2025-09-01T14:56:47.293Z
Learning: The whitenoise_flutter project does not target web platforms - any web version would be in a separate repository. Therefore, dart:io usage and web compatibility concerns can be ignored in this codebase.
Applied to files:
test/config/providers/active_pubkey_provider_test.mocks.dart
📚 Learning: 2025-09-04T17:54:09.135Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.135Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Applied to files:
lib/ui/settings/general_settings_screen.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/ui/chat/widgets/chat_header_widget.dartlib/domain/services/dm_chat_service.dartlib/ui/chat/chat_info/dm_chat_info.dartlib/ui/chat/chat_info/group_chat_info.dartlib/ui/chat/invite/chat_invite_screen.dartlib/utils/public_key_validation_extension.dartlib/config/providers/auth_provider.dartlib/config/providers/metadata_cache_provider.dartlib/ui/settings/profile/share_profile_screen.darttest/utils/pubkey_formatter_test.dartlib/config/providers/active_pubkey_provider.dartlib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-09-07T02:08:09.179Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.179Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/settings/general_settings_screen.dartlib/ui/contact_list/group_welcome_invitation_sheet.dartlib/utils/pubkey_formatter.dartlib/config/providers/nostr_keys_provider.dartlib/domain/services/dm_chat_service.dartlib/ui/chat/chat_info/dm_chat_info.dartlib/utils/public_key_validation_extension.dartlib/domain/models/contact_model.dartlib/config/providers/metadata_cache_provider.dartlib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/settings/general_settings_screen.dartlib/ui/settings/profile/share_profile_screen.dart
📚 Learning: 2025-09-05T00:08:34.466Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/active_account_provider.dart:140-146
Timestamp: 2025-09-05T00:08:34.466Z
Learning: josefinalliende prefers centralizing validation logic (like trimming whitespace) in providers rather than handling it defensively at every consumer site. This approach was applied to activePubkeyProvider in the whitenoise_flutter codebase.
Applied to files:
lib/ui/contact_list/group_welcome_invitation_sheet.dartlib/config/providers/nostr_keys_provider.dartlib/config/providers/auth_provider.dartlib/config/providers/metadata_cache_provider.dartlib/config/providers/active_pubkey_provider.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-29T09:11:07.281Z
Learnt from: erskingardner
PR: parres-hq/whitenoise_flutter#530
File: rust/src/api/utils.rs:23-26
Timestamp: 2025-08-29T09:11:07.281Z
Learning: In the nostr-sdk Rust library, PublicKey::parse method accepts both hex and bech32 (npub) encoded strings, so there's no need to use format-specific methods like PublicKey::from_bech32 when parsing public keys from string input.
Applied to files:
lib/utils/pubkey_formatter.dartlib/utils/public_key_validation_extension.dartlib/config/providers/metadata_cache_provider.dartlib/ui/contact_list/widgets/contact_list_tile.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Follow the Arrange-Act-Assert convention for tests.
Applied to files:
test/config/providers/active_account_provider_test.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/config/providers/active_pubkey_provider.dart
🔇 Additional comments (41)
test/ui/contact_list/services/welcome_notification_service_simple_test.dart (1)
9-9: Initialize Flutter test binding early — good.Prevents binding-related surprises in tests that touch Widgets or PlatformDispatcher.
test/config/providers/profile_ready_card_visibility_provider_test.dart (1)
35-35: Widget binding initialization — good.Consistent with other test suites in this PR.
test/config/providers/welcomes_provider_test.dart (2)
8-8: Binding initialization — LGTM.Keeps tests consistent and deterministic.
824-827: Readable reflow of newPendingWelcomes — OK.No logic change; improves clarity.
test/config/providers/active_account_provider_test.dart (3)
179-179: Binding initialization — good.Matches the pattern across provider tests.
85-97: Mock default metadata fallback — OK.Reasonable default for absent metadata in tests.
383-391: Multiline account setup — clearer.No behavioral change; improves readability.
lib/ui/chat/invite/chat_invite_screen.dart (1)
16-16: Use PubkeyFormatter import — good.Consolidates formatting behind the new utility.
lib/ui/chat/chat_info/chat_info_screen.dart (1)
23-23: Good: import moved to the library for use in parts.This enables PubkeyFormatter in part files without duplicating imports.
lib/ui/chat/chat_info/widgets/member_action_buttons.dart (1)
14-14: Import aligns with standardized formatter usage.Consistent with PR direction.
lib/ui/contact_list/widgets/contact_list_tile.dart (1)
36-41: LGTM — centralized formatting with sane fallback.Using PubkeyFormatter with a graceful fallback to the original key and formatting for display is correct and consistent with the PR goals.
lib/config/providers/nostr_keys_provider.dart (1)
45-46: LGTM — standardized NPUB derivation via PubkeyFormatter.Consistent with the refactor; null-safe fallback handled downstream.
lib/domain/models/contact_model.dart (1)
3-3: Good move: centralize pubkey conversions via PubkeyFormatter.lib/domain/services/dm_chat_service.dart (1)
7-7: Import change aligns with formatter centralization.lib/ui/chat/widgets/chat_header_widget.dart (3)
12-12: Formatter import looks good.
59-60: LGTM: lazy-init group npub via formatter.
65-67: Call setState when updating _groupNpub in didUpdateWidget.Without setState, UI won’t refresh when nostrGroupId changes.
- _groupNpub = PubkeyFormatter(pubkey: widget.group.nostrGroupId).toNpub(); + setState(() { + _groupNpub = PubkeyFormatter(pubkey: widget.group.nostrGroupId).toNpub(); + });Likely an incorrect or invalid review comment.
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
13-13: Import switch to PubkeyFormatter is correct.lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart (2)
19-19: Formatter import matches refactor direction.
67-72: LGTM: normalize active user pubkey to npub with null-safe fallback.lib/ui/settings/general_settings_screen.dart (1)
27-27: Good switch to centralized formatter import.
Keeps conversions consistent across UI.lib/config/providers/auth_provider.dart (3)
12-12: LGTM on formatter import.
Centralizes key normalization in auth flows.
16-16: Doc tweak looks fine.
Scope accurately reflects responsibilities.
294-299: Consistent hex normalization on logout filtering.
Correctly prevents mismatches when filtering remaining accounts.test/config/providers/group_provider_test.dart (6)
9-9: Initialize Flutter test binding early — good.
Prevents binding-related flakes.
117-119: Typed map literal — good clarity.
136-138: Typed map literal — good clarity.
295-297: Typed map literal — good clarity.
318-320: Typed map literal — good clarity.
345-350: Typed map literal — good clarity.lib/config/providers/active_pubkey_provider.dart (3)
4-4: Formatter import — good.
16-27: DI-friendly formatter factory — nice.
Enables clean mocking in tests.
61-63: Provider wiring — looks correct.lib/config/providers/group_provider.dart (1)
15-15: LGTM! Clean migration to centralized PubkeyFormatter.The import replacement aligns with the PR's goal of centralizing pubkey format conversions.
lib/utils/pubkey_formatter.dart (2)
1-16: Well-designed formatter with dependency injection support!The class structure with injectable conversion functions is excellent for testability and follows SOLID principles. The default wiring to the Rust bridge APIs maintains backward compatibility.
19-19: Potential issue with lowercase normalization for npub format.The code converts the input to lowercase on lines 19 and 38, but npub format (Bech32) is case-sensitive. Converting npub strings to lowercase could break valid npub keys.
String? toNpub() { if (_pubkey == null) return null; - final trimmedPubkey = _pubkey.trim().toLowerCase(); + final trimmedPubkey = _pubkey.trim(); final PublicKeyType? pubkeyType = trimmedPubkey.publicKeyType; if (!trimmedPubkey.isValidPublicKey) return null; if (pubkeyType == PublicKeyType.npub) { return trimmedPubkey; } else if (pubkeyType == PublicKeyType.hex) { + // Only lowercase hex format, not npub + final hexKey = trimmedPubkey.toLowerCase(); try { - final npub = _npubFromHexPubkey(hexPubkey: trimmedPubkey); + final npub = _npubFromHexPubkey(hexPubkey: hexKey); return npub; } catch (e) { return null; } }String? toHex() { if (_pubkey == null) return null; - final trimmedPubkey = _pubkey.trim().toLowerCase(); + final trimmedPubkey = _pubkey.trim(); final PublicKeyType? pubkeyType = trimmedPubkey.publicKeyType; if (!trimmedPubkey.isValidPublicKey) return null; if (pubkeyType == PublicKeyType.hex) { - return trimmedPubkey; + // Normalize hex to lowercase + return trimmedPubkey.toLowerCase(); } else if (pubkeyType == PublicKeyType.npub) { try { final hex = _hexPubkeyFromNpub(npub: trimmedPubkey); - return hex; + // Ensure hex output is lowercase + return hex?.toLowerCase(); } catch (e) { return null; } }Also applies to: 38-38
⛔ Skipped due to learnings
Learnt from: josefinalliende PR: parres-hq/whitenoise_flutter#594 File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0 Timestamp: 2025-09-04T17:54:09.135Z Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.Learnt from: josefinalliende PR: parres-hq/whitenoise_flutter#594 File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0 Timestamp: 2025-09-04T17:54:09.136Z Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.Learnt from: josefinalliende PR: parres-hq/whitenoise_flutter#594 File: lib/config/providers/user_profile_data_provider.dart:0-0 Timestamp: 2025-09-03T20:57:53.202Z Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.Learnt from: josefinalliende PR: parres-hq/whitenoise_flutter#455 File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0 Timestamp: 2025-08-12T11:21:53.640Z Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.Learnt from: erskingardner PR: parres-hq/whitenoise_flutter#530 File: rust/src/api/utils.rs:23-26 Timestamp: 2025-08-29T09:11:07.281Z Learning: In the nostr-sdk Rust library, PublicKey::parse method accepts both hex and bech32 (npub) encoded strings, so there's no need to use format-specific methods like PublicKey::from_bech32 when parsing public keys from string input.Learnt from: josefinalliende PR: parres-hq/whitenoise_flutter#594 File: lib/domain/models/contact_model.dart:36-41 Timestamp: 2025-09-07T02:08:09.179Z Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.lib/ui/chat/chat_info/group_chat_info.dart (2)
33-33: LGTM! Consistent migration to PubkeyFormatter.The conversion properly handles the nullable nostrGroupId.
59-66: Good fix for member deduplication logic!The hex normalization ensures accurate comparison regardless of the original format (hex vs npub), which addresses one of the core issues mentioned in the PR objectives.
lib/config/providers/metadata_cache_provider.dart (2)
112-115: Question: Should metadata fetch be blocked without an active account?
_fetchMetadataForKeythrows if there is no active account, even though fetching someone else’s metadata could be read-only. If the UX allows browsing before login, this will fail.Would you like this to degrade gracefully (skip the check, or log-only) when used in read-only flows?
1-11: Overall direction looks good.Centralizing key normalization and format handling here aligns with the PR goal and will reduce flakiness across providers.
test/config/providers/active_pubkey_provider_test.dart (1)
12-21: LGTM: test vectors and mappings are clear and realistic.Good coverage of both npub and hex paths with whitespace handling.
josefinalliende
force-pushed
the
fix/pubkey-formats
branch
4 times, most recently
from
627f102 to
d4a6fc4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
lib/ui/settings/general_settings_screen.dart (1)
151-166: Remove unreachable catch; handle null/empty hex deterministically.
toHex()returning null won’t throw, so the catch never runs; also avoid assigning ''.Apply:
- try { - // Try to convert npub to hex for matching - String hexKey = selectedProfile.publicKey; - if (selectedProfile.publicKey.isValidNpubPublicKey) { - hexKey = PubkeyFormatter(pubkey: selectedProfile.publicKey).toHex() ?? ''; - } - - selectedAccount = _accounts.where((account) => account.pubkey == hexKey).firstOrNull; - } catch (e) { - final hexProfilePubKey = PubkeyFormatter(pubkey: selectedProfile.publicKey).toHex(); - selectedAccount = - _accounts.where((account) { - final hexAccountPubkey = PubkeyFormatter(pubkey: account.pubkey).toHex(); - return hexAccountPubkey == hexProfilePubKey; - }).firstOrNull; - } + final selectedHex = PubkeyFormatter(pubkey: selectedProfile.publicKey).toHex(); + final needle = (selectedHex != null && selectedHex.isNotEmpty) + ? selectedHex + : selectedProfile.publicKey; // may already be hex + selectedAccount = _accounts.where((account) => account.pubkey == needle).firstOrNull;lib/domain/services/dm_chat_service.dart (1)
18-34: Convert and validate pubkey before API and UI
In lib/domain/services/dm_chat_service.dart (lines 18–34), replace the direct use ofotherMember.publicKeyinwn_users_api.getUserwith a hex‐normalized value viaPubkeyFormatter.toHex()(bail if it returns null), and useformatter.toNpub() ?? otherMemberHexrather thantoNpub() ?? ''when populatingDMChatData.publicKey.lib/config/providers/group_provider.dart (4)
153-173: Avoid false negatives when detecting existing DMs (load members if cache is empty) + null npub guard.If members aren’t cached yet, _findExistingDirectMessage will miss existing threads and create duplicates. Also guard null npub conversions.
Apply:
- final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); - final otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); + final otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + if (currentUserNpub == null || otherUserNpub == null) { + _logger.warning( + 'DM lookup: failed to derive npub(s). active=$activePubkey other=$otherUserPubkeyHex', + ); + return null; + } @@ - final members = getGroupMembers(group.mlsGroupId); - if (members != null && members.length == 2) { + var members = getGroupMembers(group.mlsGroupId); + if (members == null || members.length != 2) { + await loadGroupMembers(group.mlsGroupId); + members = getGroupMembers(group.mlsGroupId); + } + if (members != null && members.length == 2) { final memberPubkeys = members.map((m) => m.publicKey).toSet(); if (memberPubkeys.contains(currentUserNpub) && memberPubkeys.contains(otherUserNpub)) { _logger.info('Found existing DM group: ${group.mlsGroupId}'); return group; } }
417-421: Await ApiError.messageText() in admin processing catch.Fix logging of nested errors.
- if (e is ApiError) { - final errorDetails = e.messageText(); + if (e is ApiError) { + final errorDetails = await e.messageText(); logMessage += '$errorDetails (Type: ${e.runtimeType})'; } else {
812-816: Await ApiError.messageText() in checkForNewGroups catch.Fixes error logging.
- if (e is ApiError) { - final errorDetails = e.messageText(); + if (e is ApiError) { + final errorDetails = await e.messageText(); logMessage += '$errorDetails (Type: ${e.runtimeType})'; } else {
259-263: AwaitApiError.messageText()to resolve the Future
Replace the un-awaited call so you log the actual error string, notInstance of 'Future<String>'.--- a/lib/config/providers/group_provider.dart +++ b/lib/config/providers/group_provider.dart @@ -259,7 +259,7 @@ if (e is ApiError) { - final errorDetails = e.messageText(); + final errorDetails = await e.messageText(); logMessage += '$errorDetails (Type: ${e.runtimeType})'; } else { logMessage += '$e (Type: ${e.runtimeType})';To find any other un-awaited
messageText()calls:#!/bin/bash rg -n --type dart '\bmessageText\(' | rg -v 'await'
♻️ Duplicate comments (3)
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
68-71: Don’t cache empty hex; leave map unset to preserve fallbacks.
Writing '' disables null-coalescing fallbacks and breaks active sorting.Apply:
- final hexPubkey = PubkeyFormatter(pubkey: originalPubKey).toHex(); - if (hexPubkey == null) continue; - _pubkeyToHex[originalPubKey] = hexPubkey; + final hexPubkey = PubkeyFormatter(pubkey: originalPubKey).toHex(); + if (hexPubkey == null || hexPubkey.isEmpty) continue; + _pubkeyToHex[originalPubKey] = hexPubkey;lib/config/providers/group_provider.dart (2)
311-332: Empty npub fallback is acceptable here given presentational User.Matches prior agreement; no change needed.
389-411: Same note: empty npub fallback for admins is fine.Consistent with the presentational User usage.
🧹 Nitpick comments (14)
lib/config/providers/follows_provider.dart (1)
201-204: Normalize findFollowByPubkey using PubkeyFormatter.toHexMirror isFollowing’s normalization to avoid mismatches between npub/hex formats. For example:
User? findFollowByPubkey(String pubkey) { - return state.follows.where((user) => user.pubkey == pubkey).firstOrNull; + final target = PubkeyFormatter(pubkey: pubkey).toHex(); + if (target == null) return null; + return state.follows.firstWhere( + (u) => PubkeyFormatter(pubkey: u.pubkey).toHex() == target, + orElse: () => null, + ); }Update any affected tests accordingly.
test/ui/contact_list/start_chat_bottom_sheet_test.dart (2)
26-29: Mirror production normalization in the mock.
Make MockFollowsNotifier.isFollowing() normalize to hex like the real provider; prevents future drift when inputs are npub.Apply:
@override bool isFollowing(String pubkey) { - return _mockFollows.any((user) => user.pubkey == pubkey); + final target = PubkeyFormatter(pubkey: pubkey).toHex(); + if (target == null) return false; + return _mockFollows.any( + (u) => PubkeyFormatter(pubkey: u.pubkey).toHex() == target, + ); }Also add the missing import at the top:
+import 'package:whitenoise/utils/pubkey_formatter.dart';
245-254: Strengthen the “already a contact” case by mixing formats.
Feed an npub here to exercise normalization end-to-end in the widget path.Apply:
- followsProvider.overrideWith( - () => MockFollowsNotifier([ - User( - pubkey: contact.publicKey, + followsProvider.overrideWith( + () => MockFollowsNotifier([ + User( + pubkey: PubkeyFormatter(pubkey: contact.publicKey).toNpub()!, metadata: const FlutterMetadata(custom: {}), createdAt: DateTime.now(), updatedAt: DateTime.now(), ), ]), ),(Ensure the PubkeyFormatter import is added as noted above.)
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
99-105: Avoid defaulting to '' and remove exception control flow.
Use explicit null/empty handling;toHex()shouldn’t throw here.Apply:
- try { - final String profileHex = PubkeyFormatter(pubkey: profile.publicKey).toHex() ?? ''; - return profileHex == _activeAccountHex; - } catch (e) { - // If conversion fails, try direct comparison as fallback - return profile.publicKey == _activeAccountHex; - } + final profileHex = PubkeyFormatter(pubkey: profile.publicKey).toHex(); + if (profileHex != null && profileHex.isNotEmpty) { + return profileHex == _activeAccountHex; + } + // Fallback: maybe already hex or invalid + return profile.publicKey == _activeAccountHex;test/config/providers/active_pubkey_provider_test.dart (3)
80-85: Rename test to match assertion (null vs empty string).
Title says “empty string” but you assert null.Apply:
-test('sets state to empty string', () async { +test('sets state to null', () async {
344-348: Rename test to match assertion (null vs empty string).
Same mismatch here.Apply:
-test('updates state to empty string', () async { +test('updates state to null', () async {
359-359: Use matcher for clarity.
PreferisFalsefor consistency with the rest of the file.Apply:
-expect(wasNotified, false); +expect(wasNotified, isFalse);lib/domain/services/dm_chat_service.dart (1)
28-34: Nit: variable name typo.npup → npub (or publicKey) for readability.
- final npup = contactModel.publicKey; + final npub = contactModel.publicKey; @@ - publicKey: npup, + publicKey: npub,test/config/providers/group_provider_test.dart (5)
170-190: Redundant try/catch when test is skipped.Since skip is set, body won’t run. Drop the try/catch or remove skip when you enable the test.
192-209: Same as above: skip makes inner try/catch dead code.Consider removing the guard or the skip.
221-238: Same pattern: skipped test with internal guard.Tidy up for clarity.
240-260: Same pattern again.Consolidate: either keep skip or runtime guard, not both.
639-669: Skipped “identify group types” test is fine; same cleanup applies.Either unskip and keep the bridge guard, or keep skip and remove the guard.
lib/config/providers/group_provider.dart (1)
1024-1039: Exclude null conversions in filter; minor perf/readability win.Prevents ''/null from being treated as valid “other” members.
- final otherMembers = - members - .where( - (member) => PubkeyFormatter(pubkey: member.publicKey).toHex() != hexActivePubkey, - ) - .toList(); + final otherMembers = members + .where((member) { + final hexMember = PubkeyFormatter(pubkey: member.publicKey).toHex(); + return hexMember != null && hexMember != hexActivePubkey; + }) + .toList();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
CHANGELOG.md(1 hunks)lib/config/providers/active_pubkey_provider.dart(3 hunks)lib/config/providers/auth_provider.dart(2 hunks)lib/config/providers/chat_provider.dart(2 hunks)lib/config/providers/follows_provider.dart(2 hunks)lib/config/providers/group_provider.dart(17 hunks)lib/config/providers/metadata_cache_provider.dart(3 hunks)lib/config/providers/nostr_keys_provider.dart(2 hunks)lib/domain/models/contact_model.dart(3 hunks)lib/domain/services/dm_chat_service.dart(1 hunks)lib/ui/chat/chat_info/chat_info_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(4 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/chat/chat_info/widgets/member_action_buttons.dart(2 hunks)lib/ui/chat/invite/chat_invite_screen.dart(3 hunks)lib/ui/chat/widgets/chat_header_widget.dart(2 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(3 hunks)lib/ui/contact_list/widgets/chat_list_item_tile.dart(3 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(2 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)lib/ui/settings/profile/share_profile_screen.dart(2 hunks)lib/ui/settings/profile/switch_profile_bottom_sheet.dart(3 hunks)lib/utils/public_key_validation_extension.dart(1 hunks)test/config/providers/active_account_provider_test.dart(12 hunks)test/config/providers/active_pubkey_provider_test.dart(9 hunks)test/config/providers/active_pubkey_provider_test.mocks.dart(2 hunks)test/config/providers/group_provider_test.dart(9 hunks)test/config/providers/profile_ready_card_visibility_provider_test.dart(1 hunks)test/config/providers/welcomes_provider_test.dart(2 hunks)test/ui/contact_list/services/welcome_notification_service_simple_test.dart(1 hunks)test/ui/contact_list/start_chat_bottom_sheet_test.dart(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (22)
- test/ui/contact_list/services/welcome_notification_service_simple_test.dart
- test/config/providers/welcomes_provider_test.dart
- test/config/providers/profile_ready_card_visibility_provider_test.dart
- lib/ui/chat/chat_info/chat_info_screen.dart
- lib/ui/chat/invite/chat_invite_screen.dart
- lib/config/providers/nostr_keys_provider.dart
- lib/ui/settings/profile/share_profile_screen.dart
- lib/ui/contact_list/group_welcome_invitation_sheet.dart
- lib/ui/chat/chat_info/widgets/member_action_buttons.dart
- lib/config/providers/auth_provider.dart
- lib/domain/models/contact_model.dart
- lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart
- lib/ui/chat/chat_info/dm_chat_info.dart
- test/config/providers/active_pubkey_provider_test.mocks.dart
- lib/config/providers/metadata_cache_provider.dart
- lib/ui/chat/chat_info/group_chat_info.dart
- lib/ui/chat/widgets/chat_header_widget.dart
- lib/config/providers/active_pubkey_provider.dart
- lib/config/providers/chat_provider.dart
- lib/ui/contact_list/widgets/contact_list_tile.dart
- test/config/providers/active_account_provider_test.dart
- lib/utils/public_key_validation_extension.dart
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.905Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.179Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.135Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.136Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Applied to files:
lib/config/providers/follows_provider.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/domain/services/dm_chat_service.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-07T02:15:31.905Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.905Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/config/providers/follows_provider.darttest/config/providers/group_provider_test.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/domain/services/dm_chat_service.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-07T13:10:16.511Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.511Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/config/providers/follows_provider.darttest/ui/contact_list/start_chat_bottom_sheet_test.dartlib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/domain/services/dm_chat_service.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Use the standard widget testing for Flutter.
Applied to files:
test/ui/contact_list/start_chat_bottom_sheet_test.dart
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/ui/settings/profile/switch_profile_bottom_sheet.dartlib/domain/services/dm_chat_service.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-04T17:54:09.135Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.135Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Applied to files:
lib/ui/settings/profile/switch_profile_bottom_sheet.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-09-07T02:08:09.179Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.179Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/settings/profile/switch_profile_bottom_sheet.darttest/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dartlib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use Riverpod to manage state. Use StreamProviders to watch for state changes that come from the rust api. See keepAlive if you need to keep the state alive.
Applied to files:
lib/ui/contact_list/widgets/chat_list_item_tile.dart
📚 Learning: 2025-09-05T00:08:34.466Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/active_account_provider.dart:140-146
Timestamp: 2025-09-05T00:08:34.466Z
Learning: josefinalliende prefers centralizing validation logic (like trimming whitespace) in providers rather than handling it defensively at every consumer site. This approach was applied to activePubkeyProvider in the whitenoise_flutter codebase.
Applied to files:
test/config/providers/active_pubkey_provider_test.dartlib/config/providers/group_provider.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Follow the Arrange-Act-Assert convention for tests.
Applied to files:
test/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*.dart : Use the whitenoise rust crate (via flutter_rust_bridge) as the source of all data and only way to trigger changes in our data model.
Applied to files:
lib/ui/settings/general_settings_screen.dart
📚 Learning: 2025-08-23T11:02:28.308Z
Learnt from: Quwaysim
PR: parres-hq/whitenoise_flutter#527
File: lib/ui/core/ui/wn_avatar.dart:1-6
Timestamp: 2025-08-23T11:02:28.308Z
Learning: In the whitenoise_flutter codebase, flutter_screenutil is re-exported through lib/ui/core/themes/src/app_theme.dart, so files that import app_theme.dart do not need to directly import flutter_screenutil to use .w, .h, .sp extensions.
Applied to files:
lib/ui/settings/general_settings_screen.dart
🔇 Additional comments (25)
lib/config/providers/follows_provider.dart (1)
10-10: Good: centralizing formatting via PubkeyFormatter.
Import is appropriate and aligns with the PR’s objective to unify hex/npub handling.test/ui/contact_list/start_chat_bottom_sheet_test.dart (2)
220-224: Nice negative case coverage.
The new test asserting “Remove Contact” is hidden when not following improves clarity of intent.
262-262: LGTM on using setup() helper.
Reduces duplication and keeps tests readable.Also applies to: 267-267
lib/ui/settings/profile/switch_profile_bottom_sheet.dart (1)
13-13: LGTM: centralized formatter import.
Consistent with the PR objective to eliminate direct Rust bridge calls.lib/ui/settings/general_settings_screen.dart (1)
27-27: LGTM: formatter import aligns with refactor.
Removes dependency on utils.dart conversions.test/config/providers/active_pubkey_provider_test.dart (3)
11-11: Generating a mock for PubkeyFormatter is correct.
Enables stable, isolated tests for conversion flows.
30-36: Good: fresh mock per call avoids stubbing bleed-through.
Prevents cross-test leakage.
38-47: Consistent strategy: always mock formatter in tests.
Removing the unused override param simplifies setup and matches your intent.lib/ui/contact_list/widgets/chat_list_item_tile.dart (3)
53-55: API alignment looks good (single-arg getGroupDisplayImage).Switching to getGroupDisplayImage(group.mlsGroupId) matches the provider’s new signature. No issues spotted.
88-90: Consistent image resolution for DM fallback.Call site updated to single-arg variant—good consistency with provider API.
98-101: Final render path uses new signature as well.Good cleanup; all paths now use the same API shape.
lib/domain/services/dm_chat_service.dart (1)
6-6: Good: centralized conversions via PubkeyFormatter.Import aligns with PR objective to remove direct bridge calls and string extensions.
test/config/providers/group_provider_test.dart (6)
9-9: Correct: ensureInitialized added.Prevents binding errors in widget tests.
117-120: Map literal reformatting is fine.No behavior change; readability improves.
136-139: Admin map literal reformatting is fine.Looks good.
295-298: Map literal formatting LGTM.No functional change.
318-321: Admin map formatting LGTM.All good.
345-350: Inline test data maps LGTM.Keeps tests readable.
lib/config/providers/group_provider.dart (7)
15-15: Good move: centralized key formatting.Importing PubkeyFormatter here aligns the provider with the new normalization strategy.
117-117: Right order of ops for display names.Deferring _calculateDisplayNames until after members/types load avoids transient "Unknown" names.
469-476: Signature change to drop current-user param looks good.Helper now derives everything it needs.
491-491: Callsite updated correctly.No behavioral regressions expected.
589-596: DM display name via other member without passing current user — good.Leverages cached members consistently.
804-805: Incremental display-name calculation for new groups — good.Keeps UI responsive on polling.
1053-1070: Display image logic matches new signatures.DM path delegates to getOtherGroupMember; safe default to null otherwise.
| final usersPubkeyHex = | ||
| membersNpubs.map((userNpub) { | ||
| return PubkeyFormatter(pubkey: userNpub).toHex() ?? ''; | ||
| }).toList(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Filter invalid/duplicate hex before calling addMembersToGroup.
Avoid sending ''/null to the API and dedupe.
- final usersPubkeyHex =
- membersNpubs.map((userNpub) {
- return PubkeyFormatter(pubkey: userNpub).toHex() ?? '';
- }).toList();
+ final usersPubkeyHex = membersNpubs
+ .map((npub) => PubkeyFormatter(pubkey: npub).toHex())
+ .whereType<String>()
+ .where((h) => h.isNotEmpty)
+ .toSet()
+ .toList();
+ if (usersPubkeyHex.length != membersNpubs.length) {
+ _logger.warning('addToGroup: filtered invalid/duplicate npubs -> hex');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final usersPubkeyHex = | |
| membersNpubs.map((userNpub) { | |
| return PubkeyFormatter(pubkey: userNpub).toHex() ?? ''; | |
| }).toList(); | |
| // Filter out invalid or empty hex strings and remove duplicates before calling the API | |
| final usersPubkeyHex = membersNpubs | |
| .map((npub) => PubkeyFormatter(pubkey: npub).toHex()) | |
| .whereType<String>() | |
| .where((h) => h.isNotEmpty) | |
| .toSet() | |
| .toList(); | |
| if (usersPubkeyHex.length != membersNpubs.length) { | |
| _logger.warning('addToGroup: filtered invalid/duplicate npubs -> hex'); | |
| } |
🤖 Prompt for AI Agents
In lib/config/providers/group_provider.dart around lines 880 to 884, the code
maps membersNpubs to hex but can produce empty/null strings and duplicates;
before calling addMembersToGroup filter out invalid or empty hex values (e.g.
null/''), validate the hex format (regex or expected length), dedupe the list
(e.g. convert to a Set or use distinct), and then pass the cleaned list to
addMembersToGroup so no empty/invalid/duplicate entries are sent to the API.
josefinalliende
force-pushed
the
fix/pubkey-formats
branch
from
d4a6fc4 to
5acaf68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/domain/models/contact_model.dart (2)
36-41: Do not persist empty publicKey on conversion failure.Using
?? ''can setpublicKeyto an empty string, causing collisions and failed lookups. Fall back to a canonical, non-empty value (npub → hex → original).Apply:
- final npub = PubkeyFormatter(pubkey: pubkey).toNpub() ?? ''; + final formatter = PubkeyFormatter(pubkey: pubkey); + final persistedKey = formatter.toNpub() ?? formatter.toHex() ?? pubkey; ... - publicKey: npub, + publicKey: persistedKey,
87-90: hashCode is inconsistent with ==.
==compares canonical hex;hashCodeuses the rawpublicKey. Equal objects must share the same hash.Apply:
@override int get hashCode { - return Object.hash(publicKey, imagePath, displayName, about, website, nip05, lud16); + final hex = PubkeyFormatter(pubkey: publicKey).toHex() ?? publicKey; + return Object.hash(hex, imagePath, displayName, about, website, nip05, lud16); }
♻️ Duplicate comments (4)
lib/config/providers/group_provider.dart (4)
313-317: Acceptable given presentational User model.
Empty-string fallback aligns with prior discussion; no change requested.
391-396: Same here—fallback is acceptable.
Matches the earlier resolution about presentational User.
882-885: Filter invalid/duplicate hex before API call.
Avoid sending ''/null and dedupe for stability.- final usersPubkeyHex = - membersNpubs.map((userNpub) { - return PubkeyFormatter(pubkey: userNpub).toHex() ?? ''; - }).toList(); + final usersPubkeyHex = membersNpubs + .map((npub) => PubkeyFormatter(pubkey: npub).toHex()) + .whereType<String>() + .where((h) => h.isNotEmpty) + .toSet() + .toList(); + if (usersPubkeyHex.length != membersNpubs.length) { + _logger.warning('addToGroup: filtered invalid/duplicate npubs -> hex'); + }
936-939: Apply the same filtering for removals.
Consistency and cleaner payloads.- final usersPubkeyHex = - membersNpubs.map((userNpub) { - return PubkeyFormatter(pubkey: userNpub).toHex() ?? ''; - }).toList(); + final usersPubkeyHex = membersNpubs + .map((npub) => PubkeyFormatter(pubkey: npub).toHex()) + .whereType<String>() + .where((h) => h.isNotEmpty) + .toSet() + .toList(); + if (usersPubkeyHex.length != membersNpubs.length) { + _logger.warning('removeFromGroup: filtered invalid/duplicate npubs -> hex'); + }
🧹 Nitpick comments (11)
lib/domain/models/contact_model.dart (1)
38-41: Extract “Unknown User” into a constant.Avoid magic strings; move to a shared constants file (e.g.,
kUnknownUserLabel).test/config/providers/active_pubkey_provider_test.dart (3)
80-86: Test name doesn’t match expectation (null vs empty string).Title says “empty string” but the assertion expects
null. Rename for clarity.- test('sets state to empty string', () async { + test('sets state to null', () async {
191-195: Strengthen assertion: verify hex is persisted.Since hex is the storage-of-truth, also verify the write call to storage for stronger guarantees.
final state = container.read(activePubkeyProvider); expect(state, testHexPubkey); + verify(mockStorage.write(key: 'active_account_pubkey', value: testHexPubkey)).called(1);
343-360: Two small fixes: test name and matcher.
- Title says “empty string” but state is asserted
null.- Prefer
isFalsematcher for consistency with the rest of the file.- test('updates state to empty string', () async { + test('updates state to null', () async { ... - expect(wasNotified, false); + expect(wasNotified, isFalse);lib/ui/contact_list/widgets/contact_list_tile.dart (2)
36-41: Fix comment to match behavior.You return
publicKeyHex.formatPublicKey()(shortened), not the “full hex key”.- // Return the full hex key as fallback + // Fallback: show the formatted hex key
36-37: Minor: consider avoiding Future if conversion is sync.If
PubkeyFormatter.toNpub()is synchronous (as it appears), you can drop theFutureBuilderand compute once in-place to reduce rebuild overhead. If it may call into Rust/FFI, keeping the async path is fine.lib/ui/chat/chat_info/group_chat_info.dart (3)
33-33: Type explicitly for clarity and to follow guidelines.
Declare the inferred local type.- final npub = PubkeyFormatter(pubkey: groupDetails?.nostrGroupId).toNpub(); + final String? npub = PubkeyFormatter(pubkey: groupDetails?.nostrGroupId).toNpub();
59-64: Dedup faster by precomputing member-hex set.
Avoid O(n*m) scans inside the loop.- for (final admin in admins) { - if (!members.any((member) { - final hexMemberKey = PubkeyFormatter(pubkey: member.publicKey).toHex(); - final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); - return hexMemberKey == hexAdminKey; - })) { + for (final admin in admins) { + if (!memberHexSet.contains( + PubkeyFormatter(pubkey: admin.publicKey).toHex(), + )) { allMembers.add(admin); } }Outside this hunk, right after allMembers.addAll(members):
final Set<String> memberHexSet = members .map((m) => PubkeyFormatter(pubkey: m.publicKey).toHex()) .whereType<String>() .toSet();
118-118: Harden conversion with try/catch (parity with _loadGroup).
Avoids a crash if activeAccount is malformed.- final currentUserNpub = PubkeyFormatter(pubkey: activeAccount).toNpub(); + try { + final String? currentUserNpub = PubkeyFormatter(pubkey: activeAccount).toNpub(); + if (mounted) { + setState(() { + this.currentUserNpub = currentUserNpub; + }); + } + } catch (e) { + _logger.warning('Error converting active pubkey to npub: $e'); + } - if (mounted) { - setState(() { - this.currentUserNpub = currentUserNpub; - }); - }lib/config/providers/group_provider.dart (2)
158-160: Add explicit types and guard failed conversions.
Prevents pointless scans and clarifies intent.- final currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); - final otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + final String? currentUserNpub = PubkeyFormatter(pubkey: activePubkey).toNpub(); + final String? otherUserNpub = PubkeyFormatter(pubkey: otherUserPubkeyHex).toNpub(); + if (currentUserNpub == null || otherUserNpub == null) { + _logger.warning( + 'findExistingDM: npub conversion failed (active=$activePubkey, other=$otherUserPubkeyHex)', + ); + return null; + }
1026-1045: Make other-member lookup tighter and null-safe.
Avoid extra allocations; guard unexpected hex conversion failures.- User? getOtherGroupMember(String? groupId) { + User? getOtherGroupMember(String? groupId) { if (groupId == null) return null; final activePubkey = ref.read(activePubkeyProvider); if (activePubkey == null || activePubkey.isEmpty) return null; final members = getGroupMembers(groupId); if (members == null || members.isEmpty) return null; - - final hexActivePubkey = PubkeyFormatter(pubkey: activePubkey).toHex(); - final otherMembers = - members - .where( - (member) => PubkeyFormatter(pubkey: member.publicKey).toHex() != hexActivePubkey, - ) - .toList(); - final npubActivePubkey = PubkeyFormatter(pubkey: activePubkey).toNpub(); - if (otherMembers.isEmpty) { - _logger.warning( - 'GroupsProvider: No other members found in DM group $groupId. ' - 'Total members: ${members.length}, Current user: $npubActivePubkey', - ); - return null; - } - return otherMembers.first; + final String? hexActivePubkey = PubkeyFormatter(pubkey: activePubkey).toHex(); + if (hexActivePubkey == null) { + _logger.warning('GroupsProvider: active pubkey failed to convert to hex for group $groupId'); + return null; + } + final int idx = members.indexWhere( + (m) => PubkeyFormatter(pubkey: m.publicKey).toHex() != hexActivePubkey, + ); + if (idx == -1) { + final String? npubActivePubkey = PubkeyFormatter(pubkey: activePubkey).toNpub(); + _logger.warning( + 'GroupsProvider: No other members found in DM group $groupId. ' + 'Total members: ${members.length}, Current user: $npubActivePubkey', + ); + return null; + } + return members[idx]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
CHANGELOG.md(1 hunks)lib/config/providers/active_pubkey_provider.dart(3 hunks)lib/config/providers/auth_provider.dart(2 hunks)lib/config/providers/chat_provider.dart(2 hunks)lib/config/providers/follows_provider.dart(2 hunks)lib/config/providers/group_provider.dart(17 hunks)lib/config/providers/metadata_cache_provider.dart(3 hunks)lib/config/providers/nostr_keys_provider.dart(2 hunks)lib/domain/models/contact_model.dart(3 hunks)lib/domain/services/dm_chat_service.dart(1 hunks)lib/ui/chat/chat_info/chat_info_screen.dart(1 hunks)lib/ui/chat/chat_info/dm_chat_info.dart(1 hunks)lib/ui/chat/chat_info/group_chat_info.dart(4 hunks)lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart(2 hunks)lib/ui/chat/chat_info/widgets/member_action_buttons.dart(2 hunks)lib/ui/chat/invite/chat_invite_screen.dart(3 hunks)lib/ui/chat/widgets/chat_header_widget.dart(2 hunks)lib/ui/contact_list/group_welcome_invitation_sheet.dart(3 hunks)lib/ui/contact_list/widgets/chat_list_item_tile.dart(3 hunks)lib/ui/contact_list/widgets/contact_list_tile.dart(2 hunks)lib/ui/settings/general_settings_screen.dart(2 hunks)lib/ui/settings/profile/share_profile_screen.dart(2 hunks)lib/ui/settings/profile/switch_profile_bottom_sheet.dart(3 hunks)lib/utils/pubkey_formatter.dart(1 hunks)lib/utils/public_key_validation_extension.dart(1 hunks)test/config/providers/active_account_provider_test.dart(12 hunks)test/config/providers/active_pubkey_provider_test.dart(9 hunks)test/config/providers/active_pubkey_provider_test.mocks.dart(2 hunks)test/config/providers/group_provider_test.dart(9 hunks)test/config/providers/profile_ready_card_visibility_provider_test.dart(1 hunks)test/config/providers/welcomes_provider_test.dart(2 hunks)test/ui/contact_list/services/welcome_notification_service_simple_test.dart(1 hunks)test/ui/contact_list/start_chat_bottom_sheet_test.dart(3 hunks)test/utils/pubkey_formatter_test.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (28)
- test/utils/pubkey_formatter_test.dart
- test/config/providers/active_pubkey_provider_test.mocks.dart
- lib/ui/chat/chat_info/chat_info_screen.dart
- lib/ui/chat/invite/chat_invite_screen.dart
- lib/domain/services/dm_chat_service.dart
- test/ui/contact_list/services/welcome_notification_service_simple_test.dart
- test/config/providers/welcomes_provider_test.dart
- lib/ui/chat/chat_info/widgets/group_member_bottom_sheet.dart
- lib/ui/contact_list/group_welcome_invitation_sheet.dart
- lib/config/providers/auth_provider.dart
- lib/config/providers/chat_provider.dart
- lib/config/providers/follows_provider.dart
- lib/utils/pubkey_formatter.dart
- CHANGELOG.md
- lib/utils/public_key_validation_extension.dart
- test/ui/contact_list/start_chat_bottom_sheet_test.dart
- lib/config/providers/nostr_keys_provider.dart
- lib/config/providers/metadata_cache_provider.dart
- lib/ui/chat/widgets/chat_header_widget.dart
- lib/ui/contact_list/widgets/chat_list_item_tile.dart
- lib/ui/chat/chat_info/widgets/member_action_buttons.dart
- test/config/providers/group_provider_test.dart
- test/config/providers/active_account_provider_test.dart
- lib/ui/settings/profile/switch_profile_bottom_sheet.dart
- lib/ui/settings/profile/share_profile_screen.dart
- lib/config/providers/active_pubkey_provider.dart
- lib/ui/chat/chat_info/dm_chat_info.dart
- lib/ui/settings/general_settings_screen.dart
🧰 Additional context used
📓 Path-based instructions (2)
**/*.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*.dart: Always declare the type of each variable and function (parameters and return value). Avoid using 'any'. Create necessary types.
Don't leave blank lines within a function.
One export per file.
Use PascalCase for classes.
Use camelCase for variables, functions, and methods.
Use underscores_case for file and directory names.
Use UPPERCASE for environment variables. Avoid magic numbers and define constants.
Start each function with a verb.
Use verbs for boolean variables. Example: isLoading, hasError, canDelete, etc.
Use complete words instead of abbreviations and correct spelling, except for standard and well-known abbreviations (API, URL, i, j, err, ctx, req, res, next).
Write short functions with a single purpose. Less than 20 instructions.
Name functions with a verb and something else. If it returns a boolean, use isX or hasX, canX, etc. If it doesn't return anything, use executeX or saveX, etc.
Avoid nesting blocks by early checks and returns, or extraction to utility functions.
Use higher-order functions (map, filter, reduce, etc.) to avoid function nesting. Use arrow functions for simple functions (less than 3 instructions). Use named functions for non-simple functions.
Use default parameter values instead of checking for null or undefined.
Reduce function parameters using RO-RO: use an object to pass multiple parameters and to return results. Declare necessary types for input arguments and output.
Use a single level of abstraction in functions.
Don't abuse primitive types and encapsulate data in composite types.
Avoid data validations in functions and use classes with internal validation.
Prefer immutability for data. Use readonly for data that doesn't change. Use 'as const' for literals that don't change.
Declare interfaces to define contracts.
Write small classes with a single purpose. Less than 200 instructions, less than 10 public methods, less than 10 properties.
Use exceptions to handle errors you don't expect. If you catch an exception, it sh...
Files:
lib/ui/contact_list/widgets/contact_list_tile.darttest/config/providers/profile_ready_card_visibility_provider_test.dartlib/ui/chat/chat_info/group_chat_info.dartlib/config/providers/group_provider.dartlib/domain/models/contact_model.darttest/config/providers/active_pubkey_provider_test.dart
**/*_test.dart
📄 CodeRabbit inference engine (.cursor/rules/flutter.mdc)
**/*_test.dart: Follow the Arrange-Act-Assert convention for tests.
Name test variables clearly. Follow the convention: inputX, mockX, actualX, expectedX, etc.
Write unit tests for each public function. Use test doubles to simulate dependencies, except for third-party dependencies that are not expensive to execute.
Write acceptance tests for each module. Follow the Given-When-Then convention.
Use the standard widget testing for Flutter.
Files:
test/config/providers/profile_ready_card_visibility_provider_test.darttest/config/providers/active_pubkey_provider_test.dart
🧠 Learnings (10)
📓 Common learnings
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.199Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.162Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.162Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
📚 Learning: 2025-09-04T17:54:09.162Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/ui/chat/chat_info/dm_chat_info.dart:0-0
Timestamp: 2025-09-04T17:54:09.162Z
Learning: In the whitenoise_flutter codebase, pubkey conversion from hex to npub format is handled using a .toNpub() extension method on String, which eliminates the need for manual format checking or calling npubFromHexPubkey() function directly.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/chat/chat_info/group_chat_info.dartlib/config/providers/group_provider.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-12T11:21:53.640Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#455
File: lib/ui/settings/profile/switch_profile_bottom_sheet.dart:0-0
Timestamp: 2025-08-12T11:21:53.640Z
Learning: In the whitenoise_flutter codebase, ContactModel.publicKey can be stored in either npub format (from metadata cache) or hex format (from account storage). The activeAccountProvider may return either format depending on how the account was originally stored, so normalization to hex format is required when comparing with other hex-normalized keys in sorting logic.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/chat/chat_info/group_chat_info.dartlib/config/providers/group_provider.dartlib/domain/models/contact_model.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-09-03T20:57:53.202Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/user_profile_data_provider.dart:0-0
Timestamp: 2025-09-03T20:57:53.202Z
Learning: In the whitenoise_flutter codebase, pubkey normalization (npub/hex format handling) is implemented in a pubkey validations extension rather than being imported directly into individual providers like user_profile_data_provider.dart.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/chat/chat_info/group_chat_info.dartlib/config/providers/group_provider.dartlib/domain/models/contact_model.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-09-07T02:08:09.199Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/domain/models/contact_model.dart:36-41
Timestamp: 2025-09-07T02:08:09.199Z
Learning: The issue of ContactModel.publicKey potentially being set to an empty string when pubkey.toNpub() conversion fails (causing collision/lookup issues) is being addressed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/config/providers/group_provider.dartlib/domain/models/contact_model.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-09-07T02:15:31.931Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/src/rust/api/utils.dart:12-16
Timestamp: 2025-09-07T02:15:31.931Z
Learning: The lingering PublicKey references in lib/config/providers/auth_provider.dart, lib/config/providers/group_provider.dart, and rust_builder/cargokit/build_tool/lib/src/options.dart identified during the utils.dart API cleanup were fixed in PR #597 in the whitenoise_flutter codebase.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/ui/chat/chat_info/group_chat_info.dartlib/config/providers/group_provider.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-09-07T13:10:16.542Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#597
File: lib/config/providers/group_provider.dart:311-314
Timestamp: 2025-09-07T13:10:16.542Z
Learning: In the whitenoise_flutter codebase, the User class used in group_provider.dart (and similar contexts) is presentational only, not the actual user class from the Rust API. There are plans to remove this User class and replace it with UserProfileData, similar to the planned consolidation with ContactModel.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dartlib/config/providers/group_provider.dartlib/domain/models/contact_model.dart
📚 Learning: 2025-08-29T09:11:07.281Z
Learnt from: erskingardner
PR: parres-hq/whitenoise_flutter#530
File: rust/src/api/utils.rs:23-26
Timestamp: 2025-08-29T09:11:07.281Z
Learning: In the nostr-sdk Rust library, PublicKey::parse method accepts both hex and bech32 (npub) encoded strings, so there's no need to use format-specific methods like PublicKey::from_bech32 when parsing public keys from string input.
Applied to files:
lib/ui/contact_list/widgets/contact_list_tile.dart
📚 Learning: 2025-09-05T00:08:34.509Z
Learnt from: josefinalliende
PR: parres-hq/whitenoise_flutter#594
File: lib/config/providers/active_account_provider.dart:140-146
Timestamp: 2025-09-05T00:08:34.509Z
Learning: josefinalliende prefers centralizing validation logic (like trimming whitespace) in providers rather than handling it defensively at every consumer site. This approach was applied to activePubkeyProvider in the whitenoise_flutter codebase.
Applied to files:
lib/config/providers/group_provider.darttest/config/providers/active_pubkey_provider_test.dart
📚 Learning: 2025-08-08T13:39:00.500Z
Learnt from: CR
PR: parres-hq/whitenoise_flutter#0
File: .cursor/rules/flutter.mdc:0-0
Timestamp: 2025-08-08T13:39:00.500Z
Learning: Applies to **/*_test.dart : Follow the Arrange-Act-Assert convention for tests.
Applied to files:
test/config/providers/active_pubkey_provider_test.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Flutter CI
🔇 Additional comments (11)
lib/domain/models/contact_model.dart (1)
3-3: LGTM: Centralized key formatting import.Importing PubkeyFormatter here improves testability and consistency.
test/config/providers/profile_ready_card_visibility_provider_test.dart (1)
35-35: Good call—initialize test bindings early.
TestWidgetsFlutterBinding.ensureInitialized()avoids SharedPreferences/plugin binding issues underflutter_test.test/config/providers/active_pubkey_provider_test.dart (2)
30-36: Fresh mock per invocation prevents stubbing bleed-through.Returning a new
MockPubkeyFormatterfor each call fixes the state-leak noted earlier. Nice.
38-51: Clear intent: always mock formatter in tests.Hard-wiring
pubkeyFormatter: mockPubkeyFormatter()increateContainerkeeps tests deterministic and simpler.lib/ui/chat/chat_info/group_chat_info.dart (2)
232-236: Admin check normalization looks good.
Hex-compare prevents format mismatches.
237-240: Current-user detection is correct and short-circuits safely.
No action needed.lib/config/providers/group_provider.dart (5)
15-15: Good import.
Centralizing formatting via PubkeyFormatter is the right move.
471-478: Async display-name calculation looks good.
Defer to formatter-backed comparisons.
592-619: DM display-name path reads cleanly.
Using getOtherGroupMember avoids duplicate logic.
806-861: Incremental display-name updates: LGTM.
Keeps cache in sync for new groups only.
1056-1072: Display image resolution path: OK.
Uses cached type and shared member lookup.
| final hexNpub = PubkeyFormatter(pubkey: publicKey).toHex(); | ||
| final otherHexNpub = PubkeyFormatter(pubkey: other.publicKey).toHex(); | ||
|
|
||
| return hexNpub == otherHexNpub && | ||
| other.imagePath == imagePath && |
There was a problem hiding this comment.
Fix equals() null-handling and compare canonical forms.
If toHex() returns null for either side, null == null would incorrectly mark two invalid keys as equal. Also prefer explicit fallbacks to ensure non-null comparisons.
Apply:
- final hexNpub = PubkeyFormatter(pubkey: publicKey).toHex();
- final otherHexNpub = PubkeyFormatter(pubkey: other.publicKey).toHex();
-
- return hexNpub == otherHexNpub &&
+ final hexSelf = PubkeyFormatter(pubkey: publicKey).toHex() ?? publicKey;
+ final hexOther = PubkeyFormatter(pubkey: other.publicKey).toHex() ?? other.publicKey;
+
+ return hexSelf == hexOther &&Optional: cache a canonical hex on the instance (e.g., a lazy getter) to avoid repeated formatting in hot paths.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final hexNpub = PubkeyFormatter(pubkey: publicKey).toHex(); | |
| final otherHexNpub = PubkeyFormatter(pubkey: other.publicKey).toHex(); | |
| return hexNpub == otherHexNpub && | |
| other.imagePath == imagePath && | |
| final hexSelf = PubkeyFormatter(pubkey: publicKey).toHex() ?? publicKey; | |
| final hexOther = PubkeyFormatter(pubkey: other.publicKey).toHex() ?? other.publicKey; | |
| return hexSelf == hexOther && | |
| other.imagePath == imagePath && |
| final hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex(); | ||
| final hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex(); | ||
| final hexCurrentUserNpub = PubkeyFormatter(pubkey: currentUserNpub).toHex(); | ||
| final aIsAdmin = admins.any((admin) { | ||
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | ||
| return hexAdminKey == hexPubkeyA; | ||
| }); | ||
| final bIsAdmin = admins.any((admin) { | ||
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | ||
| return hexAdminKey == hexPubkeyB; | ||
| }); | ||
| final aIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyA); | ||
| final bIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyB); | ||
|
|
There was a problem hiding this comment.
Fix null-arg crash when currentUserNpub is not loaded yet.
Sorting runs before _loadCurrentUserNpub completes; constructing PubkeyFormatter with null will throw.
- final hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex();
- final hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex();
- final hexCurrentUserNpub = PubkeyFormatter(pubkey: currentUserNpub).toHex();
+ final String? hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex();
+ final String? hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex();
+ final String? hexCurrentUser =
+ currentUserNpub == null ? null : PubkeyFormatter(pubkey: currentUserNpub).toHex();
@@
- final aIsAdmin = admins.any((admin) {
+ final bool aIsAdmin = admins.any((admin) {
final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex();
- return hexAdminKey == hexPubkeyA;
+ return hexAdminKey != null && hexAdminKey == hexPubkeyA;
});
- final bIsAdmin = admins.any((admin) {
+ final bool bIsAdmin = admins.any((admin) {
final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex();
- return hexAdminKey == hexPubkeyB;
+ return hexAdminKey != null && hexAdminKey == hexPubkeyB;
});
- final aIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyA);
- final bIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyB);
+ final bool aIsCurrentUser = hexCurrentUser != null && (hexCurrentUser == hexPubkeyA);
+ final bool bIsCurrentUser = hexCurrentUser != null && (hexCurrentUser == hexPubkeyB);Optional: precompute a Set adminHexSet once before sort and use contains() to avoid repeated conversions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex(); | |
| final hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex(); | |
| final hexCurrentUserNpub = PubkeyFormatter(pubkey: currentUserNpub).toHex(); | |
| final aIsAdmin = admins.any((admin) { | |
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | |
| return hexAdminKey == hexPubkeyA; | |
| }); | |
| final bIsAdmin = admins.any((admin) { | |
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | |
| return hexAdminKey == hexPubkeyB; | |
| }); | |
| final aIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyA); | |
| final bIsCurrentUser = currentUserNpub != null && (hexCurrentUserNpub == hexPubkeyB); | |
| final String? hexPubkeyA = PubkeyFormatter(pubkey: a.publicKey).toHex(); | |
| final String? hexPubkeyB = PubkeyFormatter(pubkey: b.publicKey).toHex(); | |
| final String? hexCurrentUser = | |
| currentUserNpub == null ? null : PubkeyFormatter(pubkey: currentUserNpub).toHex(); | |
| final bool aIsAdmin = admins.any((admin) { | |
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | |
| return hexAdminKey != null && hexAdminKey == hexPubkeyA; | |
| }); | |
| final bool bIsAdmin = admins.any((admin) { | |
| final hexAdminKey = PubkeyFormatter(pubkey: admin.publicKey).toHex(); | |
| return hexAdminKey != null && hexAdminKey == hexPubkeyB; | |
| }); | |
| final bool aIsCurrentUser = hexCurrentUser != null && hexCurrentUser == hexPubkeyA; | |
| final bool bIsCurrentUser = hexCurrentUser != null && hexCurrentUser == hexPubkeyB; |
🤖 Prompt for AI Agents
In lib/ui/chat/chat_info/group_chat_info.dart around lines 70 to 83, the code
constructs PubkeyFormatter with currentUserNpub which can be null because
sorting runs before _loadCurrentUserNpub completes; guard against null by only
calling PubkeyFormatter.toHex() for currentUserNpub when currentUserNpub != null
(e.g., compute hexCurrentUserNpub conditionally or use a nullable local), and
update aIsCurrentUser / bIsCurrentUser checks to compare only when
hexCurrentUserNpub is non-null; additionally, precompute a Set<String> of admin
hex keys once before the sort (map admins to hex and put in a set) and replace
admins.any(...) with adminHexSet.contains(...) to avoid repeated conversions.
Description
In #594, CodeRabbit left some comments about trimming and the active pubkey format. That made me realize it wasn’t guaranteed that the value saved in secure storage for a pubkey was consistently in hex format. To ensure consistency, we need to format it before saving. Also code rabbit noticed that in some places pubkey comparison could be in different formats.
My first approach was to use the string extensions .toHex and .toNpub. The problem is that these extensions are hard to mock in tests (and in this case mocking was needed, since the methods call Rust bridge functions). I changed the approach by moving those methods into a new class called PublicKeyFormatter. This made testing in the active pubkey provider much easier. I also replaced the string extension calls and the direct Rust API calls for pubkey formatting with this new util.
Commit by commit:
Type of Change
Checklist
just precommitto ensure that formatting and linting are correctjust check-flutter-coverageto ensure that flutter coverage rules are passingCHANGELOG.mdfile with your changes (if they affect the user experience)Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests