| **INV-17** | **Factory-children write-side idempotence.** Re-flushing an already-persisted child set does NOT durably duplicate rows: before every `insertChildAddresses`, the flush is deduped against the store's persisted rows (`getChildAddresses`) and a child is inserted only if it is not yet persisted OR is re-discovered at a STRICTLY LOWER creation block — the write-side analogue of the read-side min-merge (LEAST semantics). So a resumed or re-running writer that re-queues the same children from a fresh in-memory discovery queue inserts nothing; a lower re-discovery updates the effective min; equal/higher re-flushes are no-ops. The flush is FIRST grouped by STORE IDENTITY (factory minus `id`/`sourceId`, matching how the sync-store keys `factory_addresses`) so two config sources aliasing the same on-chain factory (identical factory minus `id`/`sourceId`) collapse to ONE read + ONE dedupe + ONE insert — otherwise both would read absence and each insert the same children (a first-flush duplicate the read-side min-merge cannot repair). The persisted lookup is LOWERCASE-normalized (children are lowercased at discovery) so a checksummed pre-existing row is still deduped. Addresses #53 (the fork-side hardening it proposes). **Known cost (candor):** the guard is a live UNCACHED `getChildAddresses` (SELECT of all rows) per canonical pending factory per flush-bearing interval — over a long accreting backfill this is worst-case ~O(N²) in the child count. It is left uncached in this PR (correctness first); a per-process read-through cache of the persisted min-map is the documented follow-up candidate. | Upstream's `insertChildAddresses` is a plain `INSERT` with no `ON CONFLICT`, and `ponder_sync.factory_addresses` has no `UNIQUE (factory_id, chain_id, address)` (the only sync-store table with no uniqueness story) — so a second writer on the same store durably duplicates the whole child set (the ×2-children observation came from the chaos DRIVER running two concurrent live instances — a fixed harness-driver defect — NOT from SIGKILL/resume itself, which is single-writer; see SCOPE below). App behavior is unaffected (the only consumer, `getChildAddresses`, min-merges to a set), but store-identity/digest tooling breaks and the table grows unbounded under repeated violations. The in-memory min-merge guard (INV-15) suppresses re-queueing WITHIN one process; a second live process has a fresh queue and needs a store-side guard. **SCOPE (candor):** the read→dedupe→insert→`insertIntervals` sequence all runs inside ONE store transaction per interval (the `syncStore` is tx-scoped — created from the tx handle in `runtime/historical.ts:1310-1352`), so for a single writer the guard is TRANSACTIONAL, not merely sequential: no interleaving read/insert can slip a duplicate past it, and a failed flush rolls the whole interval back. This fully closes the resumed single-writer case (the realistic production shape — production holds a single-writer app lock). The residual risk is genuinely ONLY the two-transactions / two-concurrent-processes case: two writers in two transactions can both read absence and both `INSERT`. That TOCTOU race the portal-layer dedupe does NOT close; only a DB `UNIQUE` + `ON CONFLICT DO UPDATE … LEAST` would, and the fork cannot add that cleanly — `factory_addresses` is created by a hash-keyed kysely migration (`2025_02_26_0_factories`), so a real constraint needs a NEW duplicate-collapsing migration in upstream `migrations.ts` for both pinned versions, materially larger blast radius. The #53 concurrent writer was a fixed harness-driver defect; the residual concurrent-writer gap is documented and left to a future DB-constraint change. | `portal.ts` reads `syncStore.getChildAddresses({ factory })` per pending factory before the flush and keeps a child only when `persisted.get(address) === undefined \|\| persisted.get(address) > block`; the store method is upstream, so the guard lives at the fork's own call site. | — (idempotence property; the duplicate it prevents has no safe runtime signal on the app path, so it is proven by test). | `portal.test.ts` — "regression (#53, write-side idempotence): a SECOND writer re-flushing an already-persisted child set does NOT re-insert it — the store keeps exactly one row at the min block" (mutation-verified: reverting the dedupe to a plain flush fails it — `expected […] to have a length of 1 but got 2`), "regression (#53, write-side idempotence): a re-discovery at a STRICTLY LOWER creation block is re-inserted so read-side LEAST resolves to the min (equal/higher blocks stay deduped)" (mutation-verified: a blanket `DO NOTHING` predicate `prev !== undefined` fails it — `expected [] to have a length of 1 but got +0`), "regression (#53, alias-hole): two sources whose factories differ ONLY in id/sourceId (store aliases) discovering the same child in one interval end with EXACTLY ONE store row — not two" (mutation-verified: reverting the canonicalization to key by `factory.id` fails it — the store ends with the child ×2), "regression (#53, case-normalization): a CHECKSUMMED pre-existing store row is deduped against the lowercased discovered child" (mutation-verified: dropping the lowercase-normalize of the persisted map fails it — the checksummed row no longer matches so the child is re-inserted). |
0 commit comments