Verify events before dispatch #253
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds event verification to ensure that Nostr events are properly signed and validated before being dispatched to relays. It introduces a new NoteService abstraction to handle event transmission and enables better testability by allowing mock services in tests.
Key changes:
- Adds event verification check before dispatching events
- Introduces
NoteServiceinterface andDefaultNoteServiceimplementation for handling event transmission - Makes
WebSocketClientHandler.sendEvent()method public to support the new service architecture
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| NostrSpringWebSocketClient.java | Adds event verification logic and integrates NoteService for event dispatch |
| NoteService.java | Defines interface for event transmission service |
| DefaultNoteService.java | Implements default event transmission logic through WebSocket clients |
| WebSocketClientHandler.java | Changes sendEvent method visibility from protected to public |
| NostrSpringWebSocketClientEventVerificationTest.java | Adds unit tests for event verification functionality |
Comments suppressed due to low confidence (1)
nostr-java-api/src/test/java/nostr/api/unit/NostrSpringWebSocketClientEventVerificationTest.java:25
- The mock setup doesn't verify that the correct parameters are passed to the service. Consider using more specific matchers or verify calls to ensure the event and client map are passed correctly.
Mockito.when(service.send(Mockito.any(), Mockito.any())).thenReturn(List.of());
| return clientMap.values().stream().map(client -> | ||
| client.sendEvent(event)).flatMap(List::stream).distinct().toList(); | ||
| if (event instanceof GenericEvent genericEvent) { | ||
| if (!verify(genericEvent)) { |
There was a problem hiding this comment.
The verify method is being called but is not defined in this class or imported. This will cause a compilation error.
Copilot uses AI. Check for mistakes.
| @@ -27,12 +29,23 @@ public class NostrSpringWebSocketClient implements NostrIF { | |||
| @Getter | |||
| private Identity sender; | |||
|
|
|||
| private NoteService noteService = new DefaultNoteService(); | |||
There was a problem hiding this comment.
[nitpick] The field is initialized with a default implementation but can be overridden by constructors. Consider using final modifier and proper constructor chaining to ensure consistent initialization.
Copilot uses AI. Check for mistakes.
Summary
Testing
mvn -q verify(fails: Docker environment not available for integration tests)https://chatgpt.com/codex/tasks/task_b_688bfcf8c3188331b8fb96cffaf5129f