Skip to content

Commit 8b9a37c

Browse files
authored
fix(relay): don't skip events in seenEvents for new subscriptions (#375)
The early-return optimization for "already seen" events was causing a bug when running without a cache: when subscription A sees event X, then subscription B is created later and requests the same event X, the relay sends it but it was being skipped because it's "already seen" globally. Without a cache, the event wasn't stored anywhere, so new subscriptions would never receive events they requested if those events had already been seen by other subscriptions. The fix removes the early-return and lets dispatchEvent handle routing to ALL matching subscriptions. Each subscription's eventFirstSeen (checked in eventReceived) handles per-subscription deduplication. Also removes the now-unused getEventIdFromMessage helper method.
1 parent 85c7eb9 commit 8b9a37c

File tree

3 files changed

+84
-104
lines changed

3 files changed

+84
-104
lines changed

core/src/relay/connectivity.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import type { NDK } from "../ndk";
3+
import { NDKSubscriptionManager } from "../subscription/manager";
34
import { NDKRelayConnectivity } from "./connectivity";
45
import { type NDKRelay, NDKRelayStatus } from "./index";
56

@@ -296,4 +297,74 @@ describe("NDKRelayConnectivity", () => {
296297
expect(connectivity["reconnectTimeout"]).toBeUndefined();
297298
});
298299
});
300+
301+
describe("seenEvents deduplication bug", () => {
302+
it("should deliver events to new subscriptions even if event was previously seen", () => {
303+
// This test reproduces the bug where:
304+
// 1. Subscription A sees event X -> event X added to seenEvents
305+
// 2. Subscription B is created later, asks for event X
306+
// 3. Relay sends event X back for subscription B
307+
// 4. BUG: event is skipped because it's in seenEvents
308+
// 5. Subscription B never receives the event
309+
310+
const eventId = "abc123def456789012345678901234567890123456789012345678901234abcd";
311+
const subId = "test-sub";
312+
313+
// Create a real subManager
314+
const subManager = new NDKSubscriptionManager();
315+
316+
// Pre-populate seenEvents (simulating event was seen by subscription A)
317+
subManager.seenEvent(eventId, mockRelay);
318+
319+
// Verify seenEvents is populated
320+
const seenRelays = subManager.seenEvents.get(eventId);
321+
expect(seenRelays).toBeDefined();
322+
expect(seenRelays!.length).toBe(1);
323+
324+
// Create mockNDK with real subManager
325+
const ndkWithSubManager = {
326+
...mockNDK,
327+
subManager,
328+
} as any;
329+
330+
// Create a relay mock with getProtocolHandler
331+
const relayWithProtocolHandler = {
332+
...mockRelay,
333+
getProtocolHandler: vi.fn(() => undefined),
334+
} as any;
335+
336+
// Create connectivity with the NDK that has seenEvents populated
337+
const conn = new NDKRelayConnectivity(relayWithProtocolHandler, ndkWithSubManager);
338+
conn["_status"] = NDKRelayStatus.CONNECTED;
339+
340+
// Track if dispatchEvent was called
341+
const dispatchEventSpy = vi.spyOn(subManager, "dispatchEvent");
342+
343+
// Create a mock relay subscription (subscription B)
344+
const mockRelaySub = {
345+
onevent: vi.fn((event) => {
346+
// This should call dispatchEvent on the subManager
347+
subManager.dispatchEvent(event, mockRelay);
348+
}),
349+
};
350+
351+
// Register the subscription
352+
conn.openSubs.set(subId, mockRelaySub as any);
353+
354+
// Verify subscription is registered
355+
expect(conn.openSubs.get(subId)).toBe(mockRelaySub);
356+
357+
// Send the event via WebSocket message
358+
const eventMsg = `["EVENT","${subId}",{"id":"${eventId}","pubkey":"xyz","created_at":1234567890,"kind":1,"tags":[],"content":"hello","sig":"sig123"}]`;
359+
const mockMessageEvent = new MessageEvent("message", { data: eventMsg });
360+
361+
conn["onMessage"](mockMessageEvent);
362+
363+
// The subscription's onevent should have been called
364+
expect(mockRelaySub.onevent).toHaveBeenCalled();
365+
366+
// And dispatchEvent should have been called to route to all matching subs
367+
expect(dispatchEventSpy).toHaveBeenCalled();
368+
});
369+
});
299370
});

core/src/relay/connectivity.ts

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -346,46 +346,25 @@ export class NDKRelayConnectivity {
346346
*
347347
* @param event - The MessageEvent containing the received message data.
348348
*/
349-
/**
350-
* Fast extraction of event ID from JSON string without parsing.
351-
* Returns event ID if message is EVENT type, null otherwise.
352-
* This optimization avoids expensive JSON.parse() for duplicate events.
353-
*/
354-
private getEventIdFromMessage(msg: string): string | null {
355-
// Fast check: msg[2] === 'E' && msg[3] === 'V' (EVENT message)
356-
// Format: ["EVENT","subId",{...}]
357-
if (msg.charCodeAt(2) !== 69 || msg.charCodeAt(3) !== 86) {
358-
return null;
359-
}
360-
const idPos = msg.indexOf('"id":"');
361-
if (idPos === -1) {
362-
return null;
363-
}
364-
// Extract 64 chars after "id":"
365-
// Event IDs are always 64 hex chars. If not valid, LRU lookup will fail
366-
// and we'll parse normally - no harm done.
367-
return msg.substring(idPos + 6, idPos + 70);
368-
}
369-
370349
private onMessage(event: MessageEvent): void {
371350
this.netDebug?.(event.data, this.ndkRelay, "recv");
372351

373352
// Record any activity from relay
374353
this.keepalive?.recordActivity();
375354

376-
// Early exit for duplicate events before JSON.parse
377-
// This optimization can save significant CPU on busy relays where
378-
// the same events are broadcast to multiple subscriptions
379-
const msg = event.data as string;
380-
const eventId = this.getEventIdFromMessage(msg);
381-
if (eventId && this.ndk) {
382-
const seenRelays = this.ndk.subManager.seenEvents.get(eventId);
383-
if (seenRelays && seenRelays.length > 0) {
384-
// Already processed from any relay, just track this relay saw it
385-
this.ndk.subManager.seenEvent(eventId, this.ndkRelay);
386-
return;
387-
}
388-
}
355+
// NOTE: We intentionally DON'T early-return for "already seen" events.
356+
// The seenEvents optimization was causing a bug: if subscription A sees event X,
357+
// then subscription B is created later and requests event X, the relay sends it
358+
// but it was being skipped because it's "already seen" globally.
359+
//
360+
// Instead, we let dispatchEvent handle routing to ALL matching subscriptions.
361+
// Each subscription's eventFirstSeen (checked in eventReceived) will skip
362+
// re-validation for events it's already processed, maintaining the performance
363+
// benefit while ensuring new subscriptions receive events they need.
364+
//
365+
// The seenEvents tracking in dispatchEvent() is still used for:
366+
// - exclusiveRelay filtering (checking which relays have sent an event)
367+
// - Tracking event provenance
389368

390369
try {
391370
const data = JSON.parse(event.data);

core/src/relay/event-dedup.test.ts

Lines changed: 0 additions & 70 deletions
This file was deleted.

0 commit comments

Comments
 (0)