Code review changes & small bug fixes

Author: kaladivo

AGENTS.md

@@ -79,7 +79,7 @@ IMPORTANT: Always run `bun run check-code` after making changes. It runs typeche
 - `lib/` contains shared app helpers (Nostr pool, token text parsing, topbar config)
 - `types/appTypes.ts` contains app-local shared types
 - `apps/push/src/` is split by concern: `http.ts` (Bun API), `ownership.ts` (signed challenge verification), `storage.ts` (SQLite persistence for subscriptions/pubkeys/challenges/seen outer event ids), `relayWatcher.ts` (relay subscription for outer `kind: 1059` events with catch-up vs live delivery gating), and `push.ts` (Web Push delivery + invalid subscription cleanup, including stale subscriptions tied to an old VAPID keypair)
-- Push service proof events use `kind: 27235` with short-lived per-pubkey challenge nonces; the server never decrypts NIP-17 payloads and only emits generic notifications for outer `kind: 1059` events tagged `["linky","push"]`, so sender self-copies / reactions / edits can sync over relays without triggering push
+- Push service proof events use `kind: 27235` with short-lived per-pubkey challenge nonces; `/subscribe` and `/unsubscribe` both require valid proofs per affected pubkey, full unsubscribe only happens when the last proven pubkey is removed, the server never decrypts NIP-17 payloads, and it only emits generic notifications for outer `kind: 1059` events tagged `["linky","push"]`, so sender self-copies / reactions / edits can sync over relays without triggering push
 
 ## Code Conventions
 
@@ -109,15 +109,15 @@ IMPORTANT: Always run `bun run check-code` after making changes. It runs typeche
 - PWA service worker is built from `apps/web-app/src/sw.ts` via Vite PWA `injectManifest`; changes there affect both prod and dev SW behavior
 - Dev mode now keeps the registered PWA service worker alive for push testing; use `#advanced/push-debug` to inspect persistent client/SW push logs and manually reset service workers/caches when needed
 - Push registration now validates the live `PushSubscription.options.applicationServerKey` against the current server VAPID public key and forces a re-subscribe on mismatch; open clients also re-register when the service worker emits `pushsubscriptionchange`
-- Push registration persists a stable browser `installationId` plus the last server-registered endpoint in localStorage; subscribe calls also request cleanup of legacy subscriptions without an installation id for the same pubkey, the push server replaces stale endpoints for the same installation, and the client best-effort unregisters the previous endpoint when the browser rotates/replaces the current subscription, preventing duplicate generic notifications from old endpoints
+- Push registration persists a stable browser `installationId` plus the last server-registered endpoint in localStorage; subscribe calls also request cleanup of legacy subscriptions without an installation id for the same pubkey, the push server replaces stale endpoints for the same installation, and the client best-effort unregisters the previous endpoint with a signed unsubscribe proof when the browser rotates/replaces the current subscription, preventing duplicate generic notifications from old endpoints
 - Cashu payments now publish actual token chat messages to the recipient without the outer push marker and emit one separate notify-only wrapped event per payment (`kind: 24133`, `["linky","payment_notice"]`) as the sole push trigger; receiver inbox sync never stores that notice in chat history, but the service worker and open-app notification paths render it as `You received money` / `Přijali jste peníze`
 - The PWA service worker mirrors the active `nsec` into IndexedDB on app startup/login, clears it on logout, and for closed-app push delivery it fetches the outer `kind: 1059` event from relays, decrypts it locally in the service worker, uses `You received money` / `Přijali jste peníze` copy for Cashu token messages and notify-only payment notice events, and still shows a generic fallback notification when decrypt/validation fails; any open Linky window client still suppresses the service-worker notification in favor of in-app notification logic, while inbox sync keeps actual Cashu token chat messages silent in notification surfaces and uses the notify-only payment event for the user-visible payment alert
 - Chat retention is enforced in `useMessagesDomain` (latest 500 messages/contact, 3000 global; reactions capped to 5000 and orphaned reactions are pruned)
 - Wallet top-up receive quotes are cached in owner-scoped localStorage until claimed/expired, so dismissing the QR screen does not drop a pending receive
 - Push service env is documented in `apps/push/.env.example`; `PUSH_VAPID_SUBJECT`, `PUSH_VAPID_PUBLIC_KEY`, and `PUSH_VAPID_PRIVATE_KEY` must be set before `apps/push` starts
 - `apps/push` CORS allowlist is configured via `PUSH_CORS_ORIGIN`; it accepts `*` or a comma-separated list of allowed web app origins
 - `apps/push` relay watcher defaults now match the web app chat publish relays (`wss://relay.damus.io`, `wss://nos.lol`, `wss://relay.0xchat.com`, `wss://shu01.shugur.net`) unless overridden via `PUSH_DEFAULT_RELAYS`
-- `apps/push` relay watcher uses a 3-day catch-up `since` window to accommodate NIP-59 randomized outer `created_at`, persists seen outer event ids in SQLite, suppresses notification delivery until EOSE switches the watcher into live mode, periodically refreshes the subscription to reset reconnect `since` drift, and prunes old seen-event rows on the server cleanup interval
+- `apps/push` relay watcher uses a 3-day catch-up `since` window to accommodate NIP-59 randomized outer `created_at`, persists seen outer event ids in SQLite, keeps a size-bounded in-memory seen-id cache for O(1) hot-path dedupe checks, suppresses notification delivery until EOSE switches the watcher into live mode, periodically refreshes the subscription to reset reconnect `since` drift, and prunes both cache-expired ids and old SQLite seen-event rows on the server cleanup interval
 - Container publishing is handled by `.github/workflows/push-image.yml`, which builds `apps/push/Dockerfile` and publishes the image to GHCR
 
 ## Maintaining This File

apps/push/README.md

@@ -79,15 +79,7 @@ Every pubkey listed in `recipientPubkeys` needs its own proof.
 
 ### `POST /unsubscribe`
 
-Remove an entire stored subscription by endpoint:
-
-```json
-{
-  "endpoint": "https://example.push/service"
-}
-```
-
-Or remove only selected pubkeys from a subscription with ownership proofs:
+Remove selected pubkeys from a subscription with ownership proofs. If you remove the subscription's last remaining pubkey, the whole subscription row is deleted:
 
 ```json
 {
@@ -114,6 +106,8 @@ Or remove only selected pubkeys from a subscription with ownership proofs:
 }
 ```
 
+Every pubkey listed in `recipientPubkeys` needs its own unsubscribe proof. Full subscription removal requires proving ownership for the subscription's current pubkeys.
+
 ### `GET /health`
 
 Simple health check.

apps/push/src/guards.ts

@@ -339,36 +339,22 @@ export function readUnsubscribeRequest(value: unknown): UnsubscribeRequestBody {
   }
 
   const endpoint = readEndpointOrSubscriptionEndpoint(value);
-  const recipientPubkeysValue = value.recipientPubkeys;
-  const proofsValue = value.proofs;
-  let recipientPubkeys: string[] | null = null;
-  let proofs: OwnershipProofInput[] = [];
-
-  if (recipientPubkeysValue !== undefined) {
-    recipientPubkeys = uniqueStrings(
-      readStringArray(recipientPubkeysValue, "recipientPubkeys").map((pubkey) =>
-        readPubkey(pubkey, "recipientPubkeys[]"),
-      ),
-    );
-    if (recipientPubkeys.length === 0) {
-      throw new RequestError(
-        400,
-        "invalid_request",
-        "recipientPubkeys must contain at least one pubkey when provided",
-      );
-    }
-    proofs = readOwnershipProofs(proofsValue);
-  } else if (proofsValue !== undefined) {
+  const recipientPubkeys = uniqueStrings(
+    readStringArray(value.recipientPubkeys, "recipientPubkeys").map((pubkey) =>
+      readPubkey(pubkey, "recipientPubkeys[]"),
+    ),
+  );
+  if (recipientPubkeys.length === 0) {
     throw new RequestError(
       400,
       "invalid_request",
-      "proofs may only be provided when recipientPubkeys are provided",
+      "recipientPubkeys must contain at least one pubkey",
     );
   }
 
   return {
     endpoint,
     recipientPubkeys,
-    proofs,
+    proofs: readOwnershipProofs(value.proofs),
   };
 }

apps/push/src/http.ts

@@ -256,18 +256,6 @@ export function createHttpHandler({
         );
 
         const body = readUnsubscribeRequest(await readJsonBody(request));
-        if (body.recipientPubkeys === null) {
-          const removed = storage.unregisterSubscription(body.endpoint);
-          console.info(
-            `[push] unsubscribe endpoint=${hashEndpoint(body.endpoint)} removed=${removed} ip=${ip}`,
-          );
-          return jsonResponse(config, request, 200, {
-            ok: true,
-            removed,
-            endpoint: body.endpoint,
-          });
-        }
-
         const consumedChallengeNonces = ownershipVerifier.verifyProofs(
           "unsubscribe",
           body.recipientPubkeys,

apps/push/src/index.ts

@@ -31,9 +31,11 @@ const relayWatcher = new RelayWatcher({
 
 relayWatcher.start();
 const cleanupTimer = setInterval(() => {
-  storage.pruneChallenges(Date.now());
-  storage.pruneSeenEvents(Date.now(), RelayWatcher.SEEN_EVENT_RETENTION_MS);
-  rateLimiter.prune(Date.now());
+  const nowMs = Date.now();
+  storage.pruneChallenges(nowMs);
+  storage.pruneSeenEvents(nowMs, RelayWatcher.SEEN_EVENT_RETENTION_MS);
+  relayWatcher.pruneSeen(nowMs);
+  rateLimiter.prune(nowMs);
 }, 60 * 1000);
 
 const server = Bun.serve({

apps/push/src/relayWatcher.test.ts

@@ -0,0 +1,52 @@
+import { describe, expect, it } from "bun:test";
+
+import { SeenEventIdCache } from "./relayWatcher";
+
+describe("SeenEventIdCache", () => {
+  it("drops expired entries on direct lookup", () => {
+    const cache = new SeenEventIdCache({
+      ttlMs: 10,
+      maxEntries: 10,
+    });
+
+    cache.markSeen("event-1", 100);
+
+    expect(cache.has("event-1", 109)).toBe(true);
+    expect(cache.has("event-1", 110)).toBe(false);
+    expect(cache.size).toBe(0);
+  });
+
+  it("prunes only expired prefixes during periodic cleanup", () => {
+    const cache = new SeenEventIdCache({
+      ttlMs: 10,
+      maxEntries: 10,
+    });
+
+    cache.markSeen("event-1", 100);
+    cache.markSeen("event-2", 105);
+    cache.markSeen("event-3", 120);
+
+    cache.pruneExpired(115);
+
+    expect(cache.size).toBe(1);
+    expect(cache.has("event-1", 115)).toBe(false);
+    expect(cache.has("event-2", 115)).toBe(false);
+    expect(cache.has("event-3", 115)).toBe(true);
+  });
+
+  it("evicts the oldest cached ids when the cache reaches capacity", () => {
+    const cache = new SeenEventIdCache({
+      ttlMs: 100,
+      maxEntries: 2,
+    });
+
+    cache.markSeen("event-1", 100);
+    cache.markSeen("event-2", 101);
+    cache.markSeen("event-3", 102);
+
+    expect(cache.size).toBe(2);
+    expect(cache.has("event-1", 150)).toBe(false);
+    expect(cache.has("event-2", 150)).toBe(true);
+    expect(cache.has("event-3", 150)).toBe(true);
+  });
+});

apps/push/src/relayWatcher.ts

@@ -13,6 +13,11 @@ interface RelayWatcherOptions {
   eventDedupeTtlMs: number;
 }
 
+interface SeenEventIdCacheOptions {
+  ttlMs: number;
+  maxEntries: number;
+}
+
 interface ClosableSubscription {
   close: (reason?: string) => void;
 }
@@ -109,17 +114,73 @@ function describeEventRecipients(event: NostrEvent): string {
   return recipients.length > 0 ? recipients.join(",") : "none";
 }
 
+export class SeenEventIdCache {
+  private readonly entries = new Map<string, number>();
+  private readonly ttlMs: number;
+  private readonly maxEntries: number;
+
+  constructor(options: SeenEventIdCacheOptions) {
+    this.ttlMs = options.ttlMs;
+    this.maxEntries = options.maxEntries;
+  }
+
+  has(eventId: string, nowMs: number): boolean {
+    const expiresAt = this.entries.get(eventId);
+    if (expiresAt === undefined) {
+      return false;
+    }
+
+    if (expiresAt <= nowMs) {
+      this.entries.delete(eventId);
+      return false;
+    }
+
+    return true;
+  }
+
+  markSeen(eventId: string, nowMs: number): void {
+    this.entries.delete(eventId);
+    this.entries.set(eventId, nowMs + this.ttlMs);
+    this.pruneOverflow();
+  }
+
+  pruneExpired(nowMs: number): void {
+    // Entries are inserted in first-seen order, so with a fixed TTL the oldest
+    // ids also expire first and cleanup can stop at the first live entry.
+    for (const [eventId, expiresAt] of this.entries) {
+      if (expiresAt > nowMs) {
+        break;
+      }
+      this.entries.delete(eventId);
+    }
+  }
+
+  get size(): number {
+    return this.entries.size;
+  }
+
+  private pruneOverflow(): void {
+    while (this.entries.size > this.maxEntries) {
+      const oldestEventId = this.entries.keys().next().value;
+      if (typeof oldestEventId !== "string") {
+        return;
+      }
+      this.entries.delete(oldestEventId);
+    }
+  }
+}
+
 export class RelayWatcher {
   private static readonly CATCH_UP_LOOKBACK_SECONDS = 3 * 24 * 60 * 60;
   private static readonly LIVE_SUBSCRIPTION_REFRESH_MS = 10 * 60 * 1000;
+  private static readonly SEEN_EVENT_CACHE_MAX_ENTRIES = 50_000;
   static readonly SEEN_EVENT_RETENTION_MS = 7 * 24 * 60 * 60 * 1000;
 
   private readonly relayUrls: string[];
   private readonly storage: PushStorage;
   private readonly pushDelivery: PushDeliveryService;
-  private readonly seenEventIds = new Map<string, number>();
+  private readonly seenEventIds: SeenEventIdCache;
   private readonly pool = new SimplePool({ enableReconnect: true });
-  private readonly eventDedupeTtlMs: number;
   private liveDeliveryEnabled = false;
   private refreshIntervalHandle: ReturnType<typeof setInterval> | null = null;
   private subscription: ClosableSubscription | null = null;
@@ -128,7 +189,10 @@ export class RelayWatcher {
     this.relayUrls = options.relayUrls;
     this.storage = options.storage;
     this.pushDelivery = options.pushDelivery;
-    this.eventDedupeTtlMs = options.eventDedupeTtlMs;
+    this.seenEventIds = new SeenEventIdCache({
+      ttlMs: options.eventDedupeTtlMs,
+      maxEntries: RelayWatcher.SEEN_EVENT_CACHE_MAX_ENTRIES,
+    });
   }
 
   start(): void {
@@ -190,27 +254,21 @@ export class RelayWatcher {
   }
 
   private markSeen(eventId: string, nowMs: number): boolean {
-    this.pruneSeen(nowMs);
-
-    if (this.seenEventIds.has(eventId)) {
+    if (this.seenEventIds.has(eventId, nowMs)) {
       return false;
     }
 
     if (!this.storage.recordSeenEvent(eventId, nowMs)) {
-      this.seenEventIds.set(eventId, nowMs + this.eventDedupeTtlMs);
+      this.seenEventIds.markSeen(eventId, nowMs);
       return false;
     }
 
-    this.seenEventIds.set(eventId, nowMs + this.eventDedupeTtlMs);
+    this.seenEventIds.markSeen(eventId, nowMs);
     return true;
   }
 
-  private pruneSeen(nowMs: number): void {
-    for (const [eventId, expiresAt] of this.seenEventIds.entries()) {
-      if (expiresAt <= nowMs) {
-        this.seenEventIds.delete(eventId);
-      }
-    }
+  pruneSeen(nowMs: number): void {
+    this.seenEventIds.pruneExpired(nowMs);
   }
 
   private async handleEvent(event: NostrEvent): Promise<void> {