Fix P2P Chat: Restore Simplified NIP-59 Implementation#343
Conversation
WalkthroughImplements P2P chat messaging using NIP-59 by adding extension methods to wrap/unwrap kind-1059 events, and refactors the chat notifier to use these new methods with improved deduplication, optimistic UI updates, and refined error handling. Changes
Sequence DiagramsequenceDiagram
participant User
participant ChatNotifier
participant NostrEvent
participant Crypto
participant Relay
rect rgba(100, 180, 220, 0.2)
Note over User,Relay: Sending P2P Message
User->>ChatNotifier: sendMessage(content)
ChatNotifier->>NostrEvent: p2pWrap(senderKeys, receiverPubkey)
NostrEvent->>Crypto: encrypt with ephemeral key
NostrEvent-->>ChatNotifier: kind-1059 wrapper
ChatNotifier->>Relay: publish(wrapped event)
Relay-->>ChatNotifier: success
ChatNotifier->>ChatNotifier: optimistic update state
ChatNotifier-->>User: message appears in UI
end
rect rgba(100, 220, 100, 0.2)
Note over User,Relay: Receiving P2P Message
Relay->>ChatNotifier: onChatEvent(kind-1059)
ChatNotifier->>ChatNotifier: check if duplicate by ID
alt not duplicate
ChatNotifier->>NostrEvent: p2pUnwrap(receiver)
NostrEvent->>Crypto: decrypt with receiver key
NostrEvent-->>ChatNotifier: kind-1 inner event
ChatNotifier->>ChatNotifier: add to state
else duplicate
ChatNotifier->>ChatNotifier: skip & log
end
ChatNotifier-->>User: message displayed or ignored
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/data/models/nostr_event.dart(1 hunks)lib/features/chat/notifiers/chat_room_notifier.dart(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
lib/features/**
📄 CodeRabbit inference engine (AGENTS.md)
Place application feature code under lib/features//, grouped by domain
Files:
lib/features/chat/notifiers/chat_room_notifier.dart
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Use Dart formatter defaults (two-space indentation, trailing commas) and run flutter format before committing
Resolve all Flutter analyzer warnings (flutter analyze must be clean)
**/*.dart: Remove unused imports and unused dependencies
Do not add // ignore: must_be_immutable to classes within generated files; rely on regeneration and existing file-level ignores
Files:
lib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
lib/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Name Riverpod providers as Provider or Notifier
lib/**/*.dart: Always use localized strings via S.of(context)!.keyName; avoid hardcoded user-facing text
Use clear, concise English for variable names, function names, and code comments; all comments must be in English
Target zero flutter analyze issues
Use latest Flutter/Dart APIs (e.g., Color.withValues() instead of withOpacity())
After async operations, check mounted before using BuildContext
Use const constructors where possible
Files:
lib/features/chat/notifiers/chat_room_notifier.dartlib/data/models/nostr_event.dart
lib/**/notifiers/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Encapsulate business logic in Notifiers (use Notifier pattern for complex state logic)
Files:
lib/features/chat/notifiers/chat_room_notifier.dart
lib/data/**
📄 CodeRabbit inference engine (AGENTS.md)
Store persistence and API code under lib/data/
Files:
lib/data/models/nostr_event.dart
🔇 Additional comments (4)
lib/data/models/nostr_event.dart (1)
317-361: LGTM!The
p2pUnwrapmethod properly validates the wrapper structure, decrypts the content, and verifies that the inner event is both kind 1 and properly signed. The signature validation at lines 352-354 ensures protocol compliance.lib/features/chat/notifiers/chat_room_notifier.dart (3)
146-158: LGTM!The incoming message handling correctly uses
p2pUnwrapand implements proper deduplication by message ID. The sort order ensures messages are displayed correctly.
193-218: LGTM - Optimistic UI pattern is correctly implemented.The message send flow properly uses
p2pWrapand implements optimistic UI updates. After successful publish, the inner event is immediately added to state, and when the relay echoes it back, the deduplication logic in_onChatEvent(line 149) will prevent duplicates. This provides immediate feedback while maintaining consistency.
328-359: LGTM!Historical message loading correctly uses
p2pUnwrapand implements efficient deduplication using a Set to track seen message IDs. The merge logic properly combines existing and historical messages while maintaining sort order.
| Future<NostrEvent> p2pWrap(NostrKeyPairs senderKeys, String receiverPubkey) async { | ||
| if (kind != 1) { | ||
| throw ArgumentError('Expected kind 1 event for P2P chat, got: $kind'); | ||
| } | ||
|
|
||
| if (content == null || content!.isEmpty) { | ||
| throw ArgumentError('Message content is empty'); | ||
| } | ||
|
|
||
| try { | ||
| // The inner event must be signed by the sender | ||
| // This is already done when creating the event with fromPartialData | ||
| final innerEventJson = jsonEncode(toMap()); | ||
|
|
||
| // Generate ephemeral key pair (single-use for this message) | ||
| final ephemeralKeyPair = NostrUtils.generateKeyPair(); | ||
|
|
||
| // Encrypt the inner event with ephemeral key + receiver's public key | ||
| final encryptedContent = await NostrUtils.encryptNIP44( | ||
| innerEventJson, | ||
| ephemeralKeyPair.private, | ||
| receiverPubkey, | ||
| ); | ||
|
|
||
| // Create wrapper (kind 1059) with randomized timestamp | ||
| final wrapper = NostrEvent.fromPartialData( | ||
| kind: 1059, | ||
| content: encryptedContent, | ||
| keyPairs: ephemeralKeyPair, | ||
| tags: [ | ||
| ["p", receiverPubkey], // Identifies the receiver (shared key pubkey) | ||
| ], | ||
| createdAt: _randomizedTimestamp(), | ||
| ); | ||
|
|
||
| return wrapper; | ||
| } catch (e) { | ||
| throw Exception('Failed to wrap P2P chat message: $e'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Validate that the inner event is signed before wrapping.
The method assumes the inner event is already signed (per the comment at line 286), but doesn't verify that id and sig are present before wrapping. This could allow wrapping unsigned events, violating the P2P chat protocol expectations.
Apply this diff to add signature validation:
Future<NostrEvent> p2pWrap(NostrKeyPairs senderKeys, String receiverPubkey) async {
if (kind != 1) {
throw ArgumentError('Expected kind 1 event for P2P chat, got: $kind');
}
if (content == null || content!.isEmpty) {
throw ArgumentError('Message content is empty');
}
+
+ // Verify the inner event is signed
+ if (id == null || sig == null) {
+ throw ArgumentError('Inner event must be signed before wrapping');
+ }
try {
// The inner event must be signed by the sender📝 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.
| Future<NostrEvent> p2pWrap(NostrKeyPairs senderKeys, String receiverPubkey) async { | |
| if (kind != 1) { | |
| throw ArgumentError('Expected kind 1 event for P2P chat, got: $kind'); | |
| } | |
| if (content == null || content!.isEmpty) { | |
| throw ArgumentError('Message content is empty'); | |
| } | |
| try { | |
| // The inner event must be signed by the sender | |
| // This is already done when creating the event with fromPartialData | |
| final innerEventJson = jsonEncode(toMap()); | |
| // Generate ephemeral key pair (single-use for this message) | |
| final ephemeralKeyPair = NostrUtils.generateKeyPair(); | |
| // Encrypt the inner event with ephemeral key + receiver's public key | |
| final encryptedContent = await NostrUtils.encryptNIP44( | |
| innerEventJson, | |
| ephemeralKeyPair.private, | |
| receiverPubkey, | |
| ); | |
| // Create wrapper (kind 1059) with randomized timestamp | |
| final wrapper = NostrEvent.fromPartialData( | |
| kind: 1059, | |
| content: encryptedContent, | |
| keyPairs: ephemeralKeyPair, | |
| tags: [ | |
| ["p", receiverPubkey], // Identifies the receiver (shared key pubkey) | |
| ], | |
| createdAt: _randomizedTimestamp(), | |
| ); | |
| return wrapper; | |
| } catch (e) { | |
| throw Exception('Failed to wrap P2P chat message: $e'); | |
| } | |
| } | |
| Future<NostrEvent> p2pWrap(NostrKeyPairs senderKeys, String receiverPubkey) async { | |
| if (kind != 1) { | |
| throw ArgumentError('Expected kind 1 event for P2P chat, got: $kind'); | |
| } | |
| if (content == null || content!.isEmpty) { | |
| throw ArgumentError('Message content is empty'); | |
| } | |
| // Verify the inner event is signed | |
| if (id == null || sig == null) { | |
| throw ArgumentError('Inner event must be signed before wrapping'); | |
| } | |
| try { | |
| // The inner event must be signed by the sender | |
| // This is already done when creating the event with fromPartialData | |
| final innerEventJson = jsonEncode(toMap()); | |
| // Generate ephemeral key pair (single-use for this message) | |
| final ephemeralKeyPair = NostrUtils.generateKeyPair(); | |
| // Encrypt the inner event with ephemeral key + receiver's public key | |
| final encryptedContent = await NostrUtils.encryptNIP44( | |
| innerEventJson, | |
| ephemeralKeyPair.private, | |
| receiverPubkey, | |
| ); | |
| // Create wrapper (kind 1059) with randomized timestamp | |
| final wrapper = NostrEvent.fromPartialData( | |
| kind: 1059, | |
| content: encryptedContent, | |
| keyPairs: ephemeralKeyPair, | |
| tags: [ | |
| ["p", receiverPubkey], // Identifies the receiver (shared key pubkey) | |
| ], | |
| createdAt: _randomizedTimestamp(), | |
| ); | |
| return wrapper; | |
| } catch (e) { | |
| throw Exception('Failed to wrap P2P chat message: $e'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In lib/data/models/nostr_event.dart around lines 276 to 315, the p2pWrap method
assumes the inner event is signed but does not validate that — add an explicit
check before jsonEncode(toMap()) that the event has non-null id and sig (throw
ArgumentError with a clear message if missing), and if a signature verification
helper exists (e.g., NostrUtils.verifySignature or verifyEventSignature) call it
with the event and senderKeys.public and throw if verification fails; keep the
rest of the wrapping logic unchanged so unsigned or invalidly signed events
cannot be wrapped.
Restored the correct P2P chat implementation using Mostro's simplified NIP-59 protocol. The issue occurred when dispute chat was added, which uses the full NIP-59 standard (with SEAL intermediate layer), breaking P2P chat since both features were sharing the same wrap/unwrap methods.
Changes:
Technical:
kind 1 (signed) → kind 1059 wrapperkind 1 (unsigned) → kind 13 SEAL → kind 1059 wrapperSummary by CodeRabbit
New Features
Bug Fixes