Skip to content

Commit 3deb8b5

Browse files
dzhelezovclaude
andauthored
fix(portal): factory_addresses write-side idempotence (#53) + INV-15 hardening test (#21) + wave-4 doc follow-ups (#54)
* chore(portal): wave-4 follow-up hygiene (FinalizedHead stubs, doc drift) Promised in the PR #45 merge comment: - portal-realtime.test.ts: two bare-number `finalizedHead: async () => 673` stubs → object form `async () => ({ number: 673 })`. FinalizedHead is object-only since #45 (the bare-number variant was dropped); these two test-only stubs were the last holdouts. - pr-quality-gate/SKILL.md: drop `realtime.ts` from the source↔test pair list and `statediffs.json` from the fixture list — both deleted in #45. - harness/euler-multichain/probe.mjs: rewrite the stale header comment; the mirrored `portal/realtime.ts` module was deleted in #45. The comment now describes what the probe does with no dead cross-reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(portal): factory_addresses write-side idempotence (#53) + INV-15 gate test (#21) # INV-17 — write-side idempotence for factory child addresses (#53) `ponder_sync.factory_addresses` is the only sync-store table with no uniqueness story: upstream `insertChildAddresses` is a plain INSERT with no `ON CONFLICT`, and the table has no `UNIQUE (factory_id, chain_id, address)`. Dedup is read-side only (`getChildAddresses` min-merges to a set). A second writer on the same store durably duplicates the whole child set (a SIGKILL/resume chaos run ended with every child ×2). App behavior is unaffected — the only consumer collapses duplicates — but store-identity/digest tooling breaks and the table grows unbounded. The fork owns only its own flush call site (the store method is upstream), so the guard lives there: before each `insertChildAddresses`, dedupe the flush against the store's persisted rows and insert a child 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). Re-flushing the same set (a resumed/re-running writer with a fresh in-memory discovery queue) inserts nothing; a lower re-discovery updates the effective min. Scope (candor, documented on INV-17): this closes the resumed/sequential single-writer case — the realistic production shape (single-writer app lock). It does NOT close a true two-concurrent-writers TOCTOU race (both read absence, both INSERT); only a DB UNIQUE + ON CONFLICT DO UPDATE LEAST would, and the fork can't add that cleanly — factory_addresses is created by a hash-keyed kysely migration, so a real constraint needs a NEW duplicate-collapsing migration in upstream migrations.ts for both pinned versions. The #53 concurrent writer was a fixed harness-driver defect; the residual gap is documented and left to a future DB-constraint change. Tests (portal.test.ts), both mutation-verified: - second writer re-flushing an already-persisted set → no re-insert (revert to plain flush → "expected […] to have a length of 1 but got 2"). - strictly-lower re-discovery is re-inserted so read-side LEAST wins (blanket DO-NOTHING predicate → "expected [] to have a length of 1 but got +0"). # INV-15 hardening gate test (#21 §1, TEST-ONLY) Pins INV-15's silent dependence on `isLogFactoryMatched`'s `fromBlock` floor: a factory creation log BELOW `factory.fromBlock` delivered to `scanWindow` must be discarded by the matcher — else it would enter `pendingChildren` below the walk floor and the min-merge guard would suppress re-queueing at its in-range block (a restart loss). Mutation-verified: bypassing the matcher records the sub-floor child → the test fails ("expected true to be false" at `.has(BELOW_FLOOR_CHILD)"). #21 §2's FIX-2 floor-refinement overscan is intentionally left OPEN. Both gates green: scripts/sync-upstream.sh {0.16.6,0.15.17} --test → 208 tests each (205 baseline + 3 added). biome error-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(portal): PR #54 review round 2 — factory-alias hole, case-normalize, INV-17 candor Fix round 2 on the INV-17 write-side idempotence guard (#53), per the gated review: 1. BLOCKER (factory-alias hole): the sync-store keys factory_addresses by STORE IDENTITY (factory minus id/sourceId; sync-store/index.ts insertChildAddresses ~:459 and getChildAddresses ~:502 both strip { id, sourceId } and upsert/select the factories row on the remaining value under UNIQUE (factory)). Two config sources aliasing the same on-chain factory (identical minus id/sourceId) each read empty then both insert -> duplicate child rows on the FIRST flush, breaking the PR's own idempotence claim. The guard now canonicalizes the flush by store identity (storeFactoryKey): ONE read + ONE dedupe + ONE insert per canonical group. The mock store is RE-KEYED the same way the real store keys (strip id/sourceId) — its prior factory.id keying is exactly why the hole was invisible. New regression test 'alias-hole' (two aliased sources -> exactly one store row). 2. Case normalization: getChildAddresses returns stored text verbatim and min-merges case-sensitively; children are lowercased at discovery. The persisted lookup is now lowercase-normalized so a checksummed pre-existing row is still deduped. New regression test 'case-normalization' (checksummed row -> no re-insert). 3. INV-17 doc (perf, candor): the guard is a live uncached getChildAddresses per canonical pending factory per flush-bearing interval (worst ~O(N^2) over an accreting backfill) — documented as a known cost with a per-process cache as the follow-up candidate. Not implemented here (correctness first). 4. INV-17 wording: (a) the ×2-children observation is re-attributed to the chaos DRIVER's two concurrent live instances (a fixed harness defect), NOT SIGKILL/ resume itself — consistent with the SCOPE section. (b) The single-writer claim is upgraded from "sequential" to TRANSACTIONAL: read->dedupe->insert->insertIntervals run in ONE tx-scoped syncStore transaction per interval (runtime/historical.ts :1310-1352); residual risk is genuinely only the two-transactions/two-processes case. Both gates green (0.16.6 and 0.15.17: 210/210, exit 0). Both mutations bite named tests: revert canonicalization to factory.id -> alias-hole fails (expected 2 to be 1); drop the lowercase-normalize -> case-normalization fails (length +0 but got 1). Biome error-clean on the changed portal files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent e2185c8 commit 3deb8b5

7 files changed

Lines changed: 485 additions & 23 deletions

File tree

.agents/skills/pr-quality-gate/SKILL.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ by `scripts/sync-upstream.sh`. Keep that in mind for Step 2.
2323
3. If tests are missing, **write them** (Step 3 conventions) before proceeding, or flag it explicitly
2424
in the PR body under a `## Missing tests` section.
2525

26-
The current source ↔ test pairs are `portal.ts`, `portal-transform.ts`, `realtime.ts`,
26+
The current source ↔ test pairs are `portal.ts`, `portal-transform.ts`,
2727
`portal-realtime.ts`, `portal-realtime-wire.ts` — each with a matching `*.test.ts`.
2828

2929
## Step 2: Run the Portal tests (they must be green)
@@ -59,8 +59,8 @@ fixtures. Match the existing style rather than inventing utilities:
5959
- **Mock the Portal per test:** stand up a local `http` server or pass a `fetchImpl` stub that replays
6060
canned batches (see `mockPortal(...)` in `portal-realtime-wire.test.ts`). Model realistic Portal
6161
responses — `204`/`409`/`503`, finalized-head advances, chunk boundaries.
62-
- **Fixtures** live in `portal/__fixtures__/*.json` (`receipts.json`, `statediffs.json`,
63-
`traces.json`), loaded with `readFileSync(join(__dirname, "__fixtures__"), ...)`.
62+
- **Fixtures** live in `portal/__fixtures__/*.json` (`receipts.json`, `traces.json`),
63+
loaded with `readFileSync(join(__dirname, "__fixtures__"), ...)`.
6464
- **Matchers:** `toBe` / `toEqual` for values, `toMatch` for hex-shape checks, `toMatchInlineSnapshot`
6565
for large structured output. Assert against the **RPC shape** the Portal transform must reproduce
6666
(hex normalization, accessList shape, receipt/status fields) — that byte-for-byte parity vs RPC is

harness/euler-multichain/probe.mjs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#!/usr/bin/env node
2-
// Endpoint health probe for the realtime soak — evaluates each endpoint's latency / tip-freshness /
3-
// responsiveness. Standalone (no imports beyond node) so it survives an overnight run alongside ponder.
4-
// Mirrors portal/realtime.ts (unit-tested). Writes PROBE_METRICS_FILE (rolling JSON) + a .log trail.
2+
// Endpoint health probe for the realtime soak — polls each configured endpoint and records its
3+
// latency, tip-freshness (how far behind chain head it reports), and responsiveness. Standalone (no
4+
// imports beyond node) so it survives an overnight run alongside ponder. Writes PROBE_METRICS_FILE
5+
// (rolling JSON) + a .log trail.
56
//
67
// REALTIME_RPC_KEY=... [SQD_RPC_KEY=...] [PROBE_INTERVAL_MS=15000] node probe.mjs
78
import fs from 'node:fs';

portal/INVARIANTS.md

Lines changed: 3 additions & 2 deletions
Large diffs are not rendered by default.

portal/portal-invariant.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* portal-invariant.ts — runtime invariant checks.
33
*
44
* The Portal layer is organised around explicit, provable invariants (portal/INVARIANTS.md,
5-
* INV-1…INV-16). This module lets the code ASSERT them at runtime and cross-reference the
5+
* INV-1…INV-17). This module lets the code ASSERT them at runtime and cross-reference the
66
* catalog by id, so a violated assumption is a loud, attributable crash rather than silent
77
* data corruption.
88
*

portal/portal-realtime.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ test('portalRealtimeEvents: a 1-block orphan at tip heals via 409 fork negotiati
438438
anchor,
439439
fetchImpl: mockForkFetch(conns, bodies, () => ac.abort()),
440440
signal: ac.signal,
441-
finalizedHead: async () => 673, // floor stays at the anchor; nothing new finalizes
441+
finalizedHead: async () => ({ number: 673 }), // floor stays at the anchor; nothing new finalizes
442442
finalizePollMs: 999999,
443443
})) {
444444
events.push(e);
@@ -884,7 +884,7 @@ test('portalRealtimeEvents: the unknown-parent gap fatal MESSAGE carries the win
884884
anchor,
885885
fetchImpl: mockFetch(batches, () => ac.abort()),
886886
signal: ac.signal,
887-
finalizedHead: async () => 673,
887+
finalizedHead: async () => ({ number: 673 }),
888888
finalizePollMs: 999999,
889889
});
890890
let msg = '';

0 commit comments

Comments
 (0)