polish(traces): committee-driven polish of the ancestor-error cascade — INVARIANTS row, INV-20(d), byAddr robustify, candor doc

Scoped follow-up polish on the ancestor-error trace cascade (core algorithm unchanged;
already committee-proven byte-identical to stock ponder's rpc/actions.ts DFS).

- Reword the shipped placeholder: the JSDoc's `<UPSTREAM_ISSUE_URL> (to be filled by the
  reviewer)` now points to "Upstream divergence documented in VALIDATION.md §5" (no invented URL).
- Renumber the cascade invariant INV-19 → INV-20 across code/test/doc. INV-19 was ALREADY
  taken by the code-enforced low-edge chunk-coverage guard from #50 (portal.ts
  invariant("INV-19", …), tested in portal.test.ts). Squatting on INV-19 broke the
  doc⟷code⟷test grep-ability rule (two unrelated invariants under one id); INV-20 is the
  next free id.
- Add the INV-20 catalog row to portal/INVARIANTS.md (matches the table's column format;
  cites cascadeTraceErrors/rankTraces + the INV-20 tests + VALIDATION.md §5).
- Robustify the byAddr test helper: assert ranked.length === sorted.length so the
  positional zip stays a valid oracle instead of silently misaligning if a future fixture
  ever introduces a parityToCallFrame-droppable trace type (safe today: all type:'call').
- Add a non-prefix-closed candor doc-comment above cascadeTraceErrors: an orphan (missing
  intermediate ancestor) inherits nothing — a deliberate conservative choice (never fabricate
  a false smear; no worse than the pre-shim null), unreachable on geth-faithful prefix-closed
  Portal trees.
- Add test INV-20 (d): an inheriting child of an ancestor with an error but NO revertReason
  gets error set AND revertReason === undefined — pinning the UNCONDITIONAL revertReason
  assignment that keeps the Portal store byte-exact with upstream rpc/actions.ts. No guard
  added on frame.revertReason (a guard would diverge the Portal store from the RPC store).

Gates: sync-upstream.sh --test 0.16.6 + 0.16.8 both 333 passed / 18 files, build OK; biome
2.5.2 clean at error level. Mutation-verified: neutering the applied cascade fails INV-20 /
(a) / (b) / (d) ("expected undefined to be 'execution reverted'") while (c) stays green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: dzhelezov

portal/INVARIANTS.md

@@ -3,8 +3,9 @@
 The `portal/` layer is organised around **explicit, provable invariants**. Every module has a functional
 core (pure, unit-tested) behind an imperative shell; the core asserts these invariants at runtime
 (`invariant("INV-…", …)` from `portal-invariant.ts`) and the test suite proves them with property-based
-tests (`fast-check`, seed-pinned for deterministic CI). IDs are stable (INV-1…INV-19) and grep-able
-across **doc ⟷ code ⟷ test**.
+tests (`fast-check`, seed-pinned for deterministic CI). IDs are stable (INV-1…INV-20) and grep-able
+across **doc ⟷ code ⟷ test**. (INV-19 is the low-edge chunk-coverage guard from #50; INV-20 is the
+ancestor-error trace cascade — the numbering is chronological, not table order.)
 
 **Runtime modes** (`PORTAL_CHECKS`, default `on`): `off` disables all checks; `on` runs the O(1) checks
 and throws `InvariantViolation` (a loud crash beats silent corruption); `strict` additionally runs the
@@ -35,6 +36,7 @@ Legacy `C#` tags from the pre-refactor comments are mapped where they existed.
 | **INV-16** (C10) | **Backfill-floor genesis semantics.** `compileFetchSpec`'s `backfillStart` — the LOW-side clamp `chunkRange` (portal-chunks.ts) applies to every chunk fetch — is `0` whenever ANY source omits `fromBlock` (⇒ genesis), and only the `Math.min` of the DEFINED `fromBlock`s when they are ALL defined. Symmetric with `backfillEnd`, where any undefined `toBlock` ⇒ unbounded. | A source with no `fromBlock` starts at genesis (ponder's runtime reads `filter.fromBlock ?? 0`). Taking `Math.min` over only the DEFINED `fromBlock`s let a bounded source (e.g. 15M) raise the floor past an unbounded source's start; `chunkRange` then clamped every fetch to `≥ 15M` and the unbounded source's `[0, 15M)` intervals assembled EMPTY and were marked synced — a permanent silent gap on the PRODUCTION historical path. | `compileFetchSpec` sets `backfillStart = froms.every(defined) ? Math.min(froms) : 0` (mirrors `backfillEnd`); `portal.ts` closes over the frozen spec and `chunkRange(idx, chunkBlocks, backfillStart, end)` uses it as the low bound. | — (pure floor; the empty-assembly it prevents has no safe runtime signal, so it is proven by test). | `portal-filters.test.ts` — "INV-16/C10: an undefined fromBlock anchors backfillStart at genesis (0)" (undefined ⇒ 0, all-defined ⇒ min, all-undefined ⇒ 0); `portal.test.ts` — "regression: an undefined fromBlock is genesis — the [0, min) prefix of an unbounded source is NOT silently skipped (C10, ports #8 by @mo4islona)". Ported from PR #8 (@mo4islona). |
 | **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). |
 | **INV-18** | **Bounded-cutover skip.** An end-capped chain (configured end ≤ finalized) is never live-probed at the historical→realtime cutover, and a fully-bounded app does zero cutover refetches — with omnichain refinements (pending-aware break, two-phase probe, clamp bypass) that never drop parked cross-chain events. **[INV-18 in detail ↓](#inv-18-in-detail)** | — | — | — | — |
+| **INV-20** | **Ancestor-error trace cascade.** Portal historical trace assembly replicates stock-ponder's ancestor-error cascade: within one transaction's trace tree a reverted ancestor's `error`/`revertReason` smears onto every descendant that lacks its OWN error, transitively (an inheriting frame becomes a source for its own children); a frame with its own error keeps it (own-error-wins). So `traces.error`/`traces.revert_reason` are byte-identical to the RPC store path. The inherited `revertReason` is assigned UNCONDITIONALLY (even when the ancestor's is `undefined`) — byte-exact with upstream `rpc/actions.ts`. | Portal delivers geth-faithful per-frame errors and never invokes upstream's `debug_traceBlock*` DFS, so a succeeded child of a reverted parent stored `error=null` where the RPC path inherits the parent's error — a `traces.error`/`revert_reason` divergence between the two store paths. Realtime/fallback already flow through the untouched RPC cascade, so the fork DB must be uniformly ponder-shaped. | `portal-assemble.ts`: `cascadeTraceErrors(sorted)` derives each frame's effective error over the DFS-sorted (`cmpTraceAddr`) frames of a tx, keyed by traceAddress prefix (parent of `[a,b,c]` is `[a,b]`); `rankTraces` applies it to each produced frame. Applied ONLY on the historical path — the realtime path rides the untouched upstream cascade and cannot double-apply. Non-prefix-closed input inherits nothing (conservative; never fabricates a smear; cannot occur for geth-faithful trees). | — (byte-identity property; the divergence it prevents has no safe runtime signal on the app path, so it is proven by test against the RPC-store oracle). | `portal-assemble.test.ts` — "INV-20: real reverted tx — the whole reverted subtree inherits the ancestor error (oracle: RPC store)" (real eth block 20351061 tx 26 fixture), "INV-20 (a): a child with its OWN distinct error SURVIVES (own-error branch wins)", "INV-20 (b): ≥3-deep nesting inherits the ancestor error transitively", "INV-20 (c): a non-reverted tx keeps every frame error/revertReason null", "INV-20 (d): an inheriting child of an ancestor with an error but NO revertReason gets error set AND revertReason === undefined (unconditional assignment)". Mutation-verified: neutering the cascade (empty effective-error map / skipping the applied `cascaded`) fails INV-20 / (a) / (b) / (d) while (c) still passes. Upstream divergence documented in VALIDATION.md §5. |
 
 ## Invariants in detail
 

portal/portal-assemble.test.ts

@@ -540,7 +540,7 @@ test("INV-2: trace assembly respects the interval's UPPER bound (a trace block a
   expect(Number(BigInt((out.traces[0]!.block as any).number))).toBe(HI);
 });
 
-// ── INV-19: ancestor-error cascade (byte-identity with the stock-ponder RPC path) ────────────────────
+// ── INV-20: ancestor-error cascade (byte-identity with the stock-ponder RPC path) ────────────────────
 //
 // Stock ponder's `debug_traceBlockByNumber` DFS (src/rpc/actions.ts) smears a reverted ancestor's
 // error/revertReason onto every descendant lacking its OWN error, transitively; a frame with its own
@@ -556,7 +556,11 @@ const byAddr = (traces: any[]): Map<string, any> => {
     cmpTraceAddr(x.traceAddress ?? [], y.traceAddress ?? []),
   );
   const ranked = rankTraces(traces);
-  // rankTraces preserves cmpTraceAddr order, so the k-th ranked frame is the k-th sorted raw trace.
+  // rankTraces returns one frame per SURVIVING sorted trace in cmpTraceAddr order (parityToCallFrame can
+  // drop a frame — e.g. a non-call/create/suicide type). Every fixture here is type:'call', so nothing
+  // drops; assert that so the positional zip below stays a valid oracle rather than silently misaligning
+  // if a future fixture introduces a droppable type.
+  expect(ranked.length).toBe(sorted.length);
   sorted.forEach((t, i) => {
     m.set((t.traceAddress ?? []).join(','), ranked[i]!.frame);
   });
@@ -586,7 +590,7 @@ const mkErrTrace = (
   ...(revertReason !== undefined ? { revertReason } : {}),
 });
 
-test('INV-19: real reverted tx — the whole reverted subtree inherits the ancestor error (oracle: RPC store)', () => {
+test('INV-20: real reverted tx — the whole reverted subtree inherits the ancestor error (oracle: RPC store)', () => {
   // Real geth callTracer output for eth mainnet block 20351061, tx index 26 (a reverted swap-router call),
   // reshaped into Portal's Parity-flat RawTrace with each frame's OWN error preserved verbatim (Portal's
   // geth-faithful input shape): frames [], [0], [0,2] carry error; [0,1] and its whole subtree are null.
@@ -614,7 +618,7 @@ test('INV-19: real reverted tx — the whole reverted subtree inherits the ances
   }
 });
 
-test('INV-19 (a): a child with its OWN distinct error SURVIVES (own-error branch wins over the parent)', () => {
+test('INV-20 (a): a child with its OWN distinct error SURVIVES (own-error branch wins over the parent)', () => {
   const frames = byAddr([
     mkErrTrace([], 'execution reverted', 'parent revert'),
     mkErrTrace([0]), // null → inherits parent
@@ -632,7 +636,7 @@ test('INV-19 (a): a child with its OWN distinct error SURVIVES (own-error branch
   expect(frames.get('1,0').revertReason).toBe('own revert');
 });
 
-test('INV-19 (b): ≥3-deep nesting inherits the ancestor error transitively', () => {
+test('INV-20 (b): ≥3-deep nesting inherits the ancestor error transitively', () => {
   const frames = byAddr([
     mkErrTrace([], 'execution reverted', 'deep revert'),
     mkErrTrace([0]),
@@ -647,7 +651,7 @@ test('INV-19 (b): ≥3-deep nesting inherits the ancestor error transitively', (
   }
 });
 
-test('INV-19 (c): a non-reverted tx keeps every frame error/revertReason null', () => {
+test('INV-20 (c): a non-reverted tx keeps every frame error/revertReason null', () => {
   const frames = byAddr([
     mkErrTrace([]),
     mkErrTrace([0]),
@@ -660,3 +664,23 @@ test('INV-19 (c): a non-reverted tx keeps every frame error/revertReason null',
     expect(f.revertReason).toBeUndefined();
   }
 });
+
+test('INV-20 (d): an inheriting child of an ancestor that has an error but NO revertReason gets error set AND revertReason === undefined (unconditional assignment)', () => {
+  // Ancestor reverted with an `error` but no `revertReason` (revertReason omitted → undefined). The child
+  // has neither of its own, so it inherits. Upstream `rpc/actions.ts` assigns BOTH fields unconditionally
+  // (`frame.error = error.error; frame.revertReason = error.revertReason`), so the child ends up with the
+  // ancestor's error AND revertReason === undefined (present-as-undefined).
+  const frames = byAddr([
+    mkErrTrace([], 'execution reverted'), // error set, revertReason omitted (undefined)
+    mkErrTrace([0]), // no own error → inherits [ ]'s error + undefined revertReason
+  ]);
+
+  expect(frames.get('').error).toBe('execution reverted');
+  expect(frames.get('').revertReason).toBeUndefined();
+
+  // This PINS upstream-exact behavior: the cascade assigns revertReason UNCONDITIONALLY. Do NOT add a
+  // guard skipping the undefined-revertReason assignment — that would diverge the Portal store from the
+  // RPC store, whose actions.ts writes revertReason on every inheriting frame regardless of its value.
+  expect(frames.get('0').error).toBe('execution reverted'); // inherited the ancestor's error
+  expect(frames.get('0').revertReason).toBeUndefined(); // and its undefined revertReason, unconditionally
+});

portal/portal-assemble.ts

@@ -81,7 +81,7 @@ type CallFrame = {
 export type RankedTrace = { frame: CallFrame; index: number };
 
 /**
- * INV-19 (ancestor-error cascade): reproduce stock ponder's derived trace-error normalization so the
+ * INV-20 (ancestor-error cascade): reproduce stock ponder's derived trace-error normalization so the
  * Portal-path store is byte-identical to the RPC-path store on `traces.error` / `traces.revert_reason`.
  *
  * Upstream `src/rpc/actions.ts` `debug_traceBlockByNumber`/`debug_traceBlockByHash` DFS (shipped as
@@ -98,7 +98,17 @@ export type RankedTrace = { frame: CallFrame; index: number };
  * resolved before its children are visited. Keyed by traceAddress (not the produced frame) so the
  * parentage chain is intact even if `parityToCallFrame` drops a frame from the output.
  *
- * Upstream issue: <UPSTREAM_ISSUE_URL>  (to be filled by the reviewer).
+ * Non-prefix-closed candor: this keys parentage by traceAddress prefix, so a frame inherits ONLY when its
+ * exact parent path is present in the same tx (parent of [0,1] is [0]). If a Portal tree ever omitted an
+ * intermediate ancestor ([0,1] present, [0] absent), the orphan would inherit NOTHING — a deliberate
+ * conservative choice: we never fabricate a false error smear, and the worst case is the SAME null the
+ * pre-shim path already stored (never worse). This cannot occur for real Portal trees — geth-faithful
+ * callTracer trees are always prefix-closed (every frame's parent frame is emitted) — so the branch is
+ * defensive, not reachable. Unlike upstream's identity-threaded `parentFrame` (which would carry an
+ * orphan's grandparent down), we do not walk up past a missing link; that divergence is unobservable on
+ * prefix-closed input and strictly safer on any malformed tree.
+ *
+ * Upstream divergence documented in VALIDATION.md §5.
  */
 const cascadeTraceErrors = (
   sorted: RawTrace[],
@@ -133,7 +143,7 @@ const cascadeTraceErrors = (
  * INV-5 producer: given ONE transaction's traces (reward/no-tx frames already excluded), sort them into
  * pre-order DFS (cmpTraceAddr) and assign each its rank = position in that sorted full list. The matcher
  * only ever consumes `RankedTrace`s, so a matched trace's `index` is its full-tree position, never a
- * filter-local one. INV-19: the ancestor-error cascade is applied over the same sorted tree, so each
+ * filter-local one. INV-20: the ancestor-error cascade is applied over the same sorted tree, so each
  * frame carries its post-cascade `error`/`revertReason` (byte-identical to the RPC path).
  */
 export function rankTraces(traces: RawTrace[]): RankedTrace[] {