fix(quartz): audit fixes for authorsMissingOutbox — giftwrap carve-out + EXCEPT

Audit of the authorsMissingOutbox anti-join surfaced one correctness bug and one performance win: - Bug (semantic): kind-1059 giftwraps store a random one-time key in event_headers.pubkey (the real recipient is only a hash), so the query returned an unbounded set of ephemeral keys that can never own a 10002 — junk for the outbox model this feeds. Both the SQLite path and the generic default now exclude kind 1059 from the "authors" set. - Performance: replaced the DISTINCT + correlated NOT EXISTS scan with an index-only EXCEPT (all authors minus 10002 owners). Both sides ride the unconditional query_by_kind_pubkey_created covering index — so it does NOT depend on the optional pubkey-alone index — and measured ~3x faster (44ms vs 137ms at 152k events / 20k authors); the gap widens with author count, since the old form paid one seek per distinct author. A loose-index skip-scan was rejected: it needs the pubkey-alone index and degrades to a full scan per author without it. Also: corrected the KDocs (the old text implied an efficient index-only distinct that wasn't guaranteed), added a giftwrap-exclusion test, and added FsAuthorsMissingOutboxTest — the only coverage of the IEventStore DEFAULT implementation, which EventStore always overrides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CuLzfXyVZ16ozG8oJ7hBBc
2026-08-10 08:27:04 +00:00 · 2026-07-09 02:50:39 +00:00
parent cf4eddeaad
commit 4f75b9d092
5 changed files with 162 additions and 21 deletions
@@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.nip01Core.core.Event
 import com.vitorpamplona.quartz.nip01Core.core.HexKey
 import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
 import com.vitorpamplona.quartz.nip65RelayList.AdvertisedRelayListEvent

 interface IEventStore : AutoCloseable {
@@ -124,8 +125,8 @@ interface IEventStore : AutoCloseable {
    suspend fun count(filters: List<Filter>): Int

    /**
-     * Every distinct author with at least one stored event that has NO
-     * NIP-65 relay list (kind 10002 / "outbox") in this store.
+     * Every distinct identity author with at least one stored event that has
+     * NO NIP-65 relay list (kind 10002 / "outbox") in this store.
     *
     * This is a whole-store anti-join — the set of all authors minus the
     * authors who already have an outbox — which the positive-only nostr
@@ -135,11 +136,17 @@ interface IEventStore : AutoCloseable {
     * deleted (NIP-09) or expired (NIP-40) is reported as missing, because
     * no row remains for it. Order is unspecified.
     *
+     * GiftWraps (kind 1059) are NOT counted as authors: their `pubkey` is a
+     * random one-time key, so including them would return an unbounded set of
+     * ephemeral keys that can never own a 10002 — useless to the outbox model
+     * this feeds.
+     *
     * The default implementation walks the store: it collects the authors
     * that DO have an outbox, then streams every event and keeps the
-     * authors not in that set. Correct for any store but O(events). SQLite
-     * overrides it with a single `SELECT DISTINCT … NOT EXISTS` scan that
-     * seeks the outbox lookup on the `(kind, pubkey, …)` index.
+     * authors not in that set. Correct for any store but O(events), and it
+     * decodes every event just to read its pubkey. SQLite overrides it with
+     * an index-only `EXCEPT` over `event_headers` that never materialises an
+     * event (see `QueryBuilder.authorsMissingKind`).
     */
    suspend fun authorsMissingOutbox(): List<HexKey> {
        val withOutbox = HashSet<HexKey>()
@@ -147,7 +154,7 @@ interface IEventStore : AutoCloseable {

        val missing = LinkedHashSet<HexKey>()
        query<Event>(Filter()) { event ->
-            if (event.pubKey !in withOutbox) missing.add(event.pubKey)
+            if (event.kind != GiftWrapEvent.KIND && event.pubKey !in withOutbox) missing.add(event.pubKey)
        }
        return missing.toList()
    }
@@ -31,6 +31,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
 import com.vitorpamplona.quartz.nip01Core.store.IdAndTime
 import com.vitorpamplona.quartz.nip01Core.store.RawEvent
 import com.vitorpamplona.quartz.nip01Core.store.sqlite.sql.where
+import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
 import com.vitorpamplona.quartz.utils.EventFactory

 class QueryBuilder(
@@ -604,11 +605,22 @@ class QueryBuilder(
    // -----------------------------------------------------------------

    /**
-     * Distinct authors with at least one stored event that have NO stored
-     * event of [kind]. The outer scan collects every distinct `pubkey`;
-     * the correlated `NOT EXISTS` is a point lookup on
-     * `query_by_kind_pubkey_created` (kind, pubkey, …), so the cost is one
-     * distinct-pubkey pass plus a seek per author. Order is unspecified.
+     * Distinct identity authors with at least one stored event that have NO
+     * stored event of [kind], as an `EXCEPT` of two sets over `event_headers`:
+     * all authors, minus the authors that have a [kind]. Both sides are
+     * answered index-only off `query_by_kind_pubkey_created`
+     * (kind, pubkey, …) — which is created unconditionally, so this does not
+     * depend on the optional pubkey-alone index — and `EXCEPT` diffs them
+     * through one temp b-tree. That is ~3× faster than a
+     * `DISTINCT … NOT EXISTS` correlated scan, which pays one index seek per
+     * distinct author; the gap widens with author cardinality. Order is
+     * unspecified (`EXCEPT` returns pubkey-sorted, which callers must not rely
+     * on).
+     *
+     * GiftWraps (kind 1059) are excluded from the "authors" set: their
+     * `pubkey` is a random one-time key (the real recipient lives only in
+     * `pubkey_owner_hash`), so counting them would return an unbounded set of
+     * ephemeral keys that can never own a [kind] event.
     */
    fun authorsMissingKind(
        kind: Int,
@@ -616,11 +628,9 @@ class QueryBuilder(
    ): List<HexKey> {
        val sql =
            """
-            SELECT DISTINCT present.pubkey FROM event_headers AS present
-            WHERE NOT EXISTS (
-                SELECT 1 FROM event_headers AS wanted
-                WHERE wanted.kind = ? AND wanted.pubkey = present.pubkey
-            )
+            SELECT DISTINCT pubkey FROM event_headers WHERE kind <> ${GiftWrapEvent.KIND}
+            EXCEPT
+            SELECT pubkey FROM event_headers WHERE kind = ?
            """.trimIndent()
        return db.prepare(sql).use { stmt ->
            stmt.bindLong(1, kind.toLong())
@@ -23,7 +23,9 @@ package com.vitorpamplona.quartz.nip01Core.store.sqlite
 import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync
 import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent
 import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent
+import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
 import com.vitorpamplona.quartz.nip65RelayList.AdvertisedRelayListEvent
+import com.vitorpamplona.quartz.utils.EventFactory
 import kotlin.test.Test
 import kotlin.test.assertEquals

@@ -86,6 +88,24 @@ class AuthorsMissingOutboxTest : BaseDBTest() {
            assertEquals(setOf(signer.pubKey), db.authorsMissingOutbox().toSet())
        }

+    @Test
+    fun giftWrapSenderIsNotCountedAsAuthor() =
+        forEachDB { db ->
+            val noteAuthor = NostrSignerSync()
+            db.insert(noteAuthor.sign(TextNoteEvent.build("hi")))
+
+            // A kind-1059 giftwrap stores an ephemeral one-time key as its
+            // pubkey (the real recipient is only a hash). It has no outbox and
+            // never will — but it must NOT be reported as "missing" one, or the
+            // result set would grow by one junk key per received DM.
+            val ephemeralSender = "aa".repeat(32)
+            db.insert(
+                EventFactory.create("bb".repeat(32), ephemeralSender, 1L, GiftWrapEvent.KIND, emptyArray(), "", "00".repeat(64)),
+            )
+
+            assertEquals(setOf(noteAuthor.pubKey), db.authorsMissingOutbox().toSet())
+        }
+
    @Test
    fun mixOfAuthorsReportsOnlyThoseWithoutOutbox() =
        forEachDB { db ->
@@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.normalizer.normalizeRelayUrl
 import com.vitorpamplona.quartz.nip01Core.relay.sockets.okhttp.BasicOkHttpWebSocket
 import com.vitorpamplona.quartz.nip01Core.store.sqlite.DefaultIndexingStrategy
 import com.vitorpamplona.quartz.nip01Core.store.sqlite.EventStore
+import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
 import com.vitorpamplona.quartz.nip65RelayList.AdvertisedRelayListEvent
 import com.vitorpamplona.quartz.utils.EventFactory
 import kotlinx.coroutines.runBlocking
@@ -47,9 +48,9 @@ import kotlin.test.assertEquals
 *    owners into a set, then stream EVERY event (`query(Filter())`) and keep
 *    the authors not in that set. Correct for any store, but it decodes all
 *    1M events off SQLite into `Event` objects.
- *  - **sqlite** — `EventStore.authorsMissingOutbox()`, a single
- *    `SELECT DISTINCT pubkey ... NOT EXISTS` that never decodes an event and
- *    seeks the outbox check on the `(kind, pubkey, created_at)` index.
+ *  - **sqlite** — `EventStore.authorsMissingOutbox()`, an index-only `EXCEPT`
+ *    over `event_headers` (all authors minus the 10002 owners) that never
+ *    decodes an event, riding the `(kind, pubkey, created_at)` covering index.
 *
 * Corpus: the benchmark first **syncs a real sample from a popular relay**
 * (kind 1 notes + kind 10002 relay lists from [RELAY]) so the pubkey
@@ -91,7 +92,7 @@ class AuthorsMissingOutboxBenchmark {

        val missing = LinkedHashSet<HexKey>()
        store.query<Event>(Filter()) { event ->
-            if (event.pubKey !in withOutbox) missing.add(event.pubKey)
+            if (event.kind != GiftWrapEvent.KIND && event.pubKey !in withOutbox) missing.add(event.pubKey)
        }
        return missing.toList()
    }
@@ -235,7 +236,7 @@ class AuthorsMissingOutboxBenchmark {
            println("\n  result: %,d authors missing an outbox (of %,d distinct authors)".format(sqliteResult.size, allAuthors.size))
            println("  ── timings over $runs runs (best-of) ──")
            println("  generic (decode all %,d events)  best=%,9.1f ms  runs=%s".format(total, genericBest, genericMs.joinToString { "%.0f".format(it) }))
-            println("  sqlite  (DISTINCT ... NOT EXISTS) best=%,9.1f ms  runs=%s".format(sqliteBest, sqliteMs.joinToString { "%.1f".format(it) }))
+            println("  sqlite  (index-only EXCEPT)       best=%,9.1f ms  runs=%s".format(sqliteBest, sqliteMs.joinToString { "%.1f".format(it) }))
            println("  → sqlite is %.1f× faster at %,d events".format(genericBest / sqliteBest, total))
        } finally {
            store.close()
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.quartz.nip01Core.store.fs
+
+import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync
+import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent
+import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent
+import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
+import com.vitorpamplona.quartz.nip65RelayList.AdvertisedRelayListEvent
+import com.vitorpamplona.quartz.utils.EventFactory
+import com.vitorpamplona.quartz.utils.Secp256k1Instance
+import kotlinx.coroutines.runBlocking
+import java.nio.file.Files
+import java.nio.file.Path
+import kotlin.io.path.exists
+import kotlin.test.AfterTest
+import kotlin.test.BeforeTest
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+/**
+ * `authorsMissingOutbox()` against [FsEventStore], which does NOT override the
+ * method — so this is the ONLY coverage of the `IEventStore` interface DEFAULT
+ * implementation (the SQLite tests always hit the override). It pins the
+ * default's behaviour, and asserts it agrees with the same scenarios the SQLite
+ * suite checks: 10002 exclusion, NIP-09 deletion re-exposing an author, and the
+ * giftwrap-sender carve-out.
+ */
+class FsAuthorsMissingOutboxTest {
+    private lateinit var root: Path
+    private lateinit var store: FsEventStore
+
+    @BeforeTest
+    fun setup() {
+        Secp256k1Instance
+        root = Files.createTempDirectory("fs-missing-outbox-")
+        store = FsEventStore(root)
+    }
+
+    @AfterTest
+    fun tearDown() {
+        store.close()
+        if (root.exists()) {
+            Files.walk(root).use { s -> s.sorted(Comparator.reverseOrder()).forEach { Files.deleteIfExists(it) } }
+        }
+    }
+
+    @Test
+    fun defaultImplReportsOnlyAuthorsWithoutOutbox() =
+        runBlocking {
+            val withOutbox = NostrSignerSync()
+            val noOutbox = NostrSignerSync()
+
+            store.insert(withOutbox.sign(TextNoteEvent.build("a")))
+            store.insert(AdvertisedRelayListEvent.create(emptyList(), withOutbox))
+            store.insert(noOutbox.sign(TextNoteEvent.build("b")))
+
+            assertEquals(setOf(noOutbox.pubKey), store.authorsMissingOutbox().toSet())
+        }
+
+    @Test
+    fun defaultImplExcludesGiftWrapSenders() =
+        runBlocking {
+            val noteAuthor = NostrSignerSync()
+            store.insert(noteAuthor.sign(TextNoteEvent.build("hi")))
+            store.insert(
+                EventFactory.create("bb".repeat(32), "aa".repeat(32), 1L, GiftWrapEvent.KIND, emptyArray(), "", "00".repeat(64)),
+            )
+
+            assertEquals(setOf(noteAuthor.pubKey), store.authorsMissingOutbox().toSet())
+        }
+
+    @Test
+    fun defaultImplReExposesAuthorAfterOutboxDeleted() =
+        runBlocking {
+            val signer = NostrSignerSync()
+            store.insert(signer.sign(TextNoteEvent.build("content")))
+            val relayList = AdvertisedRelayListEvent.create(emptyList(), signer)
+            store.insert(relayList)
+            assertEquals(emptySet(), store.authorsMissingOutbox().toSet())
+
+            store.insert(signer.sign(DeletionEvent.build(listOf(relayList))))
+            assertEquals(setOf(signer.pubKey), store.authorsMissingOutbox().toSet())
+        }
+}