From b55d6066e97a5b4cdf344ee1c746b35cb74b578f Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:45:48 +0200 Subject: [PATCH 1/5] Align the schema-array TSDoc with the schema docs The MESSAGE_SCHEMA_SQL and CONVERSATION_SCHEMA_SQL comments claimed the arrays "match" docs/schema/message.sql and docs/schema/conversation.sql; those files mirror the DDL and reference the boot repair by comment, while the DO $unwrap$ statement lives only in the arrays. Same wording as the handbook now uses. --- src/lib/conversation-store.ts | 2 +- src/lib/message-store.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/conversation-store.ts b/src/lib/conversation-store.ts index 8ebdbffb..01367c7f 100644 --- a/src/lib/conversation-store.ts +++ b/src/lib/conversation-store.ts @@ -115,7 +115,7 @@ export interface ConversationStore { updatePublishState(id: string, state: NostrPublishState): Promise; } -/** Idempotent SQL for conversation tables (DDL plus boot-time unwrap of `nostr_event` values stored as jsonb string scalars in `conversation_message`; matches `docs/schema/conversation.sql`). */ +/** Idempotent SQL for conversation tables (DDL plus boot-time unwrap of `nostr_event` values stored as jsonb string scalars in `conversation_message`; `docs/schema/conversation.sql` mirrors the DDL and documents the boot repair statement by comment, the `DO $unwrap$` block lives only in this array). */ export const CONVERSATION_SCHEMA_SQL: readonly string[] = [ `CREATE TABLE IF NOT EXISTS conversation ( id uuid PRIMARY KEY, diff --git a/src/lib/message-store.ts b/src/lib/message-store.ts index a46539e2..ea57160d 100644 --- a/src/lib/message-store.ts +++ b/src/lib/message-store.ts @@ -326,7 +326,7 @@ export interface ZapIngestRow { receipt: Record; } -/** Idempotent SQL for the forum table (DDL plus boot-time unwrap of `nostr_event` values stored as jsonb string scalars; matches `docs/schema/message.sql`). */ +/** Idempotent SQL for the forum table (DDL plus boot-time unwrap of `nostr_event` values stored as jsonb string scalars; `docs/schema/message.sql` mirrors the DDL and documents the boot repair statement by comment, the `DO $unwrap$` block lives only in this array). */ export const MESSAGE_SCHEMA_SQL: readonly string[] = [ `CREATE TABLE IF NOT EXISTS message ( id uuid PRIMARY KEY, From 147b3858b95384a01ac9926280d49c60e82e7ec3 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 10 Sep 2026 22:47:23 +0200 Subject: [PATCH 2/5] Document the audit-trigger and zap-dedupe behaviour in TSDoc The schema array in db-change.ts carries the log_db_change trigger body, and that body now stores an unchanged bytea column as a sha256 reference instead of the full value. Its doc comment still described only the one-time view_key rewrite, although the file header and docs/schema/db_change.sql both describe the new behaviour. In zap-index.ts the ingest path returns early for a receipt whose terminal decision this process already persisted, and persistZapIngest suppresses the repeated write. The handbook describes that; the doc comments of indexZapReceipt, indexOpenZapReceipts and ingestOneReceipt did not. They now say where the skip happens and that the memory is process-local, so the first tick after a restart may rewrite one row per known receipt. --- src/lib/db-change.ts | 2 +- src/lib/nostr/zap-index.ts | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/lib/db-change.ts b/src/lib/db-change.ts index 04a3fe8a..bc726797 100644 --- a/src/lib/db-change.ts +++ b/src/lib/db-change.ts @@ -13,7 +13,7 @@ import type { SqlClient } from '@/lib/auth/sql'; -/** Idempotent SQL for the append-only change log (DDL plus one-time live `view_key` rewrite; matches `docs/schema/db_change.sql`). */ +/** Idempotent SQL for the append-only change log (DDL, the `log_db_change` trigger body that stores an unchanged bytea column as a `sha256` reference instead of the full value, and the one-time live `view_key` rewrite; matches `docs/schema/db_change.sql`). */ export const DB_CHANGE_SCHEMA_SQL: readonly string[] = [ `CREATE EXTENSION IF NOT EXISTS pgcrypto;`, `CREATE TABLE IF NOT EXISTS db_change ( diff --git a/src/lib/nostr/zap-index.ts b/src/lib/nostr/zap-index.ts index f161dae4..360cb02e 100644 --- a/src/lib/nostr/zap-index.ts +++ b/src/lib/nostr/zap-index.ts @@ -162,6 +162,10 @@ function zapIngestRow(args: { * The provider pubkey check is case-insensitive hex. Callers must already * have verified the Nostr signature (`verifyEvent`). * + * Each decision is persisted at most once per receipt id per store instance + * in this process, so re-validating a known receipt writes no further ingest + * row. + * * @param store - Forum store. * @param messageId - Forum row id. * @param receipt - Kind 9735. @@ -264,6 +268,10 @@ export async function indexZapReceipt(args: { * Query zap relays for kind:9735 receipts on recent forum notes and index * validated ones. * + * Receipts whose terminal decision this process already persisted are skipped, + * so a steady state writes no ingest rows at all. The memory is process-local, + * so the first tick after a restart may rewrite one row per known receipt. + * * @param args - Store, auth, querier, relay urls, timeout, clock, fetch. * @returns Resolves when the tick's ingest pass finishes. */ @@ -337,6 +345,10 @@ export async function indexOpenZapReceipts(args: { /** * Validate and index one candidate receipt event. * + * Returns right after id validation when this process already persisted a + * terminal decision for the receipt id on this store instance, that is + * `indexed`, or `rejected` with reason `duplicate`. + * * @param event - Queried frame. * @param args - Ingest collaborators. */ From f4fa9bdb89692c36aa25800444f1bed899f93ca7 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:03:00 +0200 Subject: [PATCH 3/5] Correct three overstated claims in the new doc comments A logic review of these comments found that two of them were simply wrong and a third read as unconditional. Fixed against the code and the repository's own tests. indexZapReceipt said that re-validating a known receipt writes no further ingest row. persistZapIngest only skips when the same outcome:reason repeats, so a later different decision does write another row. The test "persists again when a non-terminal decision later becomes indexed" asserts exactly that, two rows. The comment now says so. indexOpenZapReceipts said a steady state writes no ingest rows at all. Only indexed and rejected:duplicate short-circuit; every other rejection reason is re-validated on each tick and writes again when the decision changes. The claim now carries that condition, and ingestOneReceipt names the counter-case too. DB_CHANGE_SCHEMA_SQL described the bytea reference without its conditions. The collapse happens on UPDATE only, and only for a column that is both unchanged and not hashed by db_change_redact, which is what docs/schema/db_change.sql has said all along. The memory's doc comment now also records its asymmetry with deleteById: both store adapters forget a receipt id when the message goes away and would record it again, while this process-local map does not. --- src/lib/db-change.ts | 2 +- src/lib/nostr/zap-index.ts | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/lib/db-change.ts b/src/lib/db-change.ts index bc726797..d40dbcd6 100644 --- a/src/lib/db-change.ts +++ b/src/lib/db-change.ts @@ -13,7 +13,7 @@ import type { SqlClient } from '@/lib/auth/sql'; -/** Idempotent SQL for the append-only change log (DDL, the `log_db_change` trigger body that stores an unchanged bytea column as a `sha256` reference instead of the full value, and the one-time live `view_key` rewrite; matches `docs/schema/db_change.sql`). */ +/** Idempotent SQL for the append-only change log (DDL, the `log_db_change` trigger body that on UPDATE stores an unchanged and unredacted bytea column as a `{unchanged, sha256, bytes}` reference instead of the full value, and the one-time live `view_key` rewrite; matches `docs/schema/db_change.sql`). */ export const DB_CHANGE_SCHEMA_SQL: readonly string[] = [ `CREATE EXTENSION IF NOT EXISTS pgcrypto;`, `CREATE TABLE IF NOT EXISTS db_change ( diff --git a/src/lib/nostr/zap-index.ts b/src/lib/nostr/zap-index.ts index 360cb02e..5792ca26 100644 --- a/src/lib/nostr/zap-index.ts +++ b/src/lib/nostr/zap-index.ts @@ -33,7 +33,13 @@ const providerPubkeyCache = new Map(); /** * Last persisted ingest `outcome:reason` per receipt id, keyed by message store. - * Empty after process restart; first tick may rewrite one row per known receipt. + * Empty after process restart; the first tick then re-persists the decisions it + * has forgotten. + * + * Note the asymmetry with `MessageStore.deleteById`: both store adapters forget + * the receipt id when the message goes away and would record it again, but this + * map does not, so a terminal decision here keeps suppressing that write until + * the process restarts. */ const zapDecisions = new WeakMap>(); @@ -162,9 +168,9 @@ function zapIngestRow(args: { * The provider pubkey check is case-insensitive hex. Callers must already * have verified the Nostr signature (`verifyEvent`). * - * Each decision is persisted at most once per receipt id per store instance - * in this process, so re-validating a known receipt writes no further ingest - * row. + * The same `outcome:reason` is persisted at most once per receipt id per store + * instance in this process. A later, different decision for that receipt does + * write another ingest row. * * @param store - Forum store. * @param messageId - Forum row id. @@ -269,8 +275,11 @@ export async function indexZapReceipt(args: { * validated ones. * * Receipts whose terminal decision this process already persisted are skipped, - * so a steady state writes no ingest rows at all. The memory is process-local, - * so the first tick after a restart may rewrite one row per known receipt. + * that is `indexed`, or `rejected` with reason `duplicate`. Every other + * rejection reason is re-validated on each tick and writes again whenever the + * decision changes, so a steady state writes no ingest rows only while the + * decisions themselves are stable. The memory is process-local, so the first + * tick after a restart re-persists the decisions it has forgotten. * * @param args - Store, auth, querier, relay urls, timeout, clock, fetch. * @returns Resolves when the tick's ingest pass finishes. @@ -347,7 +356,8 @@ export async function indexOpenZapReceipts(args: { * * Returns right after id validation when this process already persisted a * terminal decision for the receipt id on this store instance, that is - * `indexed`, or `rejected` with reason `duplicate`. + * `indexed`, or `rejected` with reason `duplicate`. Every other rejection + * reason is re-validated on each call. * * @param event - Queried frame. * @param args - Ingest collaborators. From 75bfb8eb81c555836deff75d26876301fafd3b68 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:22:28 +0200 Subject: [PATCH 4/5] State the bounds the dedupe actually guarantees Two more review findings, both fair. The claim that the same outcome:reason is persisted at most once per receipt id per store instance was literally false. persistZapIngest sets the memory only after recordZapIngest resolves, startNostrWorker fires each tick with setInterval without awaiting the previous one, and a failed write leaves the memory untouched. Two overlapping ticks or a retry can therefore write a second identical row. The comment now says that the memory is consulted before the write, that this is not a guarantee, and why. The wording about the first tick after a restart had also lost the handbook's "may". It is conditional for good reason: the tick returns early without persisting anything when there are no relay urls or no event ids, and listLatest only covers currently recent notes, so a receipt whose message has aged out is never queried again at all. Both places are conditional again, and the tick's own comment now records that ticks are not serialised. --- src/lib/nostr/zap-index.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/lib/nostr/zap-index.ts b/src/lib/nostr/zap-index.ts index 5792ca26..b39736e1 100644 --- a/src/lib/nostr/zap-index.ts +++ b/src/lib/nostr/zap-index.ts @@ -33,8 +33,9 @@ const providerPubkeyCache = new Map(); /** * Last persisted ingest `outcome:reason` per receipt id, keyed by message store. - * Empty after process restart; the first tick then re-persists the decisions it - * has forgotten. + * Empty after process restart; the first tick may then re-persist a forgotten + * decision, but only for the receipts that tick still queries. A receipt whose + * message has aged out of `listLatest` is never asked for again. * * Note the asymmetry with `MessageStore.deleteById`: both store adapters forget * the receipt id when the message goes away and would record it again, but this @@ -168,9 +169,12 @@ function zapIngestRow(args: { * The provider pubkey check is case-insensitive hex. Callers must already * have verified the Nostr signature (`verifyEvent`). * - * The same `outcome:reason` is persisted at most once per receipt id per store - * instance in this process. A later, different decision for that receipt does - * write another ingest row. + * A repeated identical `outcome:reason` is normally not written again, because + * the memory is consulted before the write. That is not a guarantee: the memory + * is set only after the write resolves, worker ticks are not serialised, and a + * failed write leaves the memory untouched, so two overlapping ticks or a retry + * can still produce a second identical row. A later, different decision for that + * receipt always writes another ingest row. * * @param store - Forum store. * @param messageId - Forum row id. @@ -279,7 +283,9 @@ export async function indexZapReceipt(args: { * rejection reason is re-validated on each tick and writes again whenever the * decision changes, so a steady state writes no ingest rows only while the * decisions themselves are stable. The memory is process-local, so the first - * tick after a restart re-persists the decisions it has forgotten. + * tick after a restart may re-persist decisions it has forgotten, bounded by the + * receipts that tick queries at all. Ticks are not serialised, so the bound is + * per tick, not per receipt across concurrent ticks. * * @param args - Store, auth, querier, relay urls, timeout, clock, fetch. * @returns Resolves when the tick's ingest pass finishes. From 229ee969d7b1a9b9afe156d647a5fe24fb16d6ab Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:38:51 +0200 Subject: [PATCH 5/5] Bring the handbook and persistZapIngest in line with the corrected comments The handbook still stated the dedupe rule unconditionally: "A repeated identical outcome:reason is not written again." That contradicts the comments this pull request just corrected, and the comments are the side that survived two rounds of checking against the actual concurrency model. The handbook now carries the same bound and the same three reasons. persistZapIngest's own comment had the same problem in the same file: it read as an unconditional skip, three lines below a comment saying that is not a guarantee. It now says the memory is consulted before the write and set only after it resolves, so two overlapping ticks can both pass the check. --- docs/handbook/functions.md | 2 +- src/lib/nostr/zap-index.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/handbook/functions.md b/docs/handbook/functions.md index 6a4f9b39..8fc154fd 100644 --- a/docs/handbook/functions.md +++ b/docs/handbook/functions.md @@ -1253,7 +1253,7 @@ ## Function: indexZapReceipt - **Purpose:** Validate provider pubkey (case-insensitive hex) and add sats once per receipt id. Callers verify the Nostr signature first. Persists a `nostr_zap_ingest` row (`indexed`, or `rejected` with reason `pubkey` / `amount` / `duplicate`); store throw logs `nostr.zap.ingest.record_failed` and does not change the boolean result. -- **Ingest dedupe:** One `nostr_zap_ingest` row is written per receipt per decision change per process (memory is per store instance and empty after a restart, so the first tick after boot may write one `rejected`/`duplicate` row per known receipt). A repeated identical `outcome:reason` is not written again. Receipts whose remembered decision is already terminal never reach this function: `indexOpenZapReceipts` skips them before validation. +- **Ingest dedupe:** One `nostr_zap_ingest` row is written per receipt per decision change per process (memory is per store instance and empty after a restart, so the first tick after boot may write one `rejected`/`duplicate` row per known receipt). A repeated identical `outcome:reason` is normally not written again, because the memory is consulted before the write; that is not a guarantee, since the memory is set only after the write resolves, worker ticks are not serialised, and a failed write leaves it untouched. Receipts whose remembered decision is already terminal never reach this function: `indexOpenZapReceipts` skips them before validation. - **Inputs:** store, messageId, receipt, providerPubkey, amountSats; optional receiptEvent / noteEventId for debug rows. - **Returns / side effects:** boolean; logs indexed/rejected; records ingest. - **Used by:** `indexOpenZapReceipts` (worker tick). diff --git a/src/lib/nostr/zap-index.ts b/src/lib/nostr/zap-index.ts index b39736e1..1c33cc6b 100644 --- a/src/lib/nostr/zap-index.ts +++ b/src/lib/nostr/zap-index.ts @@ -114,8 +114,9 @@ function receiptFrame(event: NostrEventFrame): Record { /** * Persist an ingest decision without failing the tick. - * Skips the write when this process already persisted the same outcome:reason - * for the receipt id on this store instance. + * Skips the write when the memory already holds the same outcome:reason for the + * receipt id on this store instance. The memory is set only after the write + * resolves, so two overlapping ticks can both pass this check. * * @param store - Forum store. * @param row - Ingest row.