Skip to content

Commit a809432

Browse files
mo4islonaclaude
andcommitted
fix(portal): floor the stream-mode finality boundary at persisted finality (restart regression)
The wave-4 monotonic head cache and the wire's emitFinalize gate are in-memory only. Across a restart, clampFinalizedToPortalHead adopts the startup /finalized-head probe verbatim — a lagging load-balanced replica can report a head BELOW the safe checkpoint ponder already persisted, and ponder's own migrate-time "cannot move backwards" guard checks only the RPC finalized block, fetched BEFORE the clamp. Realtime then re-streams (head, persisted] and re-executes every event on rows crash recovery cannot revert (their reorg-table rows were deleted at finalize) — double-indexing — and the first finalize in the range regresses the persisted safe checkpoint verbatim. Fix, same shape as pipes-sdk's FinalizedWatermark (their PR #88): a floor seeded from persisted state. clampFinalizedToPortalHead gains `floor`; a head below it clamps UP (safe: (head, floor] is finalized history, identical across replicas) and overrides a PORTAL_FINALIZED_HEAD pin below it. The wiring passes the per-chain persisted safe checkpoint (crashRecoveryCheckpoint → checkpointBlockNumber) at getLocalSyncProgress, and the previously adopted boundary at the omnichain/multichain backfill-cutover clamps (shouldCatchup assigns ALL chains' boundaries when ANY chain advances, so one chain's advance could adopt another's stale-LOW probe; isolated's per-chain break is immune). Review fix (dzhelezov, High): checkpointBlockNumber is chainId-guarded. In omnichain ordering finalizeOmnichain writes the OMNICHAIN checkpoint verbatim to every chain's row (no per-chain where clause; "it is not an invariant that chainId and checkpoint.chainId are the same"), so a row's checkpoint can encode another chain's block height — as a floor it would either disable the Portal-head clamp (foreign height above local RPC finality → the wave-4 stream-mode fatal, a crash-loop) or under-protect (below the local safe point). A foreign-chain checkpoint now yields NO floor, mirroring upstream's own crash-recovery chainId guard in runtime/historical.ts; multichain and isolated write per-chain checkpoints and keep full protection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent e2185c8 commit a809432

5 files changed

Lines changed: 367 additions & 26 deletions

File tree

portal/INVARIANTS.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,10 @@ Five fixes from a four-lens review (client/transport, historical data path, real
8686
- **Monotonic head cache + stream-mode stale-head fatal → INV-9.** `refreshPortalHead`/`ensureChunkSize` adopted ANY probe result, but load-balanced replicas answer independently — a later probe can return a LOWER head, regressing the cache. In stream mode a regressed (stale-LOW) head made an interval at/below the true head read as "past the head", and that branch returned `[]` — marked synced while realtime streams only above the boundary: the G4/C11 silent gap, reopened. Two changes: the cached head only ever rises (`max`; every observation is ≤ the true head, so the cached max is a safe UPPER bound that never exceeds the true finalized head), and the stream-mode known-head-below-interval branch is now FATAL (it never legitimately fires — the clamp bounds historical at the boundary head — so reaching it means replica lag persisted through all retries: the same "boundary cannot be located" condition as an unknown head). Mutation-verified: reverting the `max` fails `portal-shell.test.ts` "INV-9: the cached Portal head is MONOTONIC…"; the `[]`→throw change is pinned by "INV-9: stream-realtime mode with a KNOWN head below the interval is FATAL…". **Residual (tracked, #47):** the `max`-keep guarantees only that the cached head never OVER-states finality; it does not verify range COMPLETENESS against the specific replica serving a given `/finalized-stream` POST — a replica lagging the cached head can 204 a tail that `dataChunk` then records as `coveredTo` (a phantom-coverage hole pre-existing on main — PR #15 FIX 1 documents the 204/truncate shape — whose exposure `max`-keeping extends from "until the next lower probe" to the process lifetime). The honest claim is: cached max never exceeds the true finalized head; completeness against the serving replica is not yet verified. Fix is a client-side range-completeness guard (#47), deferred pending one empirical check of Portal 204 semantics.
8787
- **Schema-degradation bound keyed per-FIELD, not per-column → INV-13 (availability).** `stream()`'s "dropping its field didn't help → real error" bound keyed on `err.tag` — the BARE column name — so a dataset missing the same column in TWO tables (`logs_bloom` in blocks AND transactions: the Monad-style shape FIX 3 exists for; also `nonce`, `gas_used`) dropped the first, then hard-threw on the second table's 400 → chunk rejected → G1-evicted → refetched → identical failure: a deterministic crash-loop. Now keyed by the table-qualified `err.fieldKey`; a RE-thrown fieldKey still means its own drop didn't fix its own 400, so the bound survives. Mutation-verified: reverting the keying fails `portal-client.test.ts` "field degradation: the SAME column missing in TWO tables drops each independently…".
8888
- **Stream-mode droppable BLOCK fields → INV-10 (availability).** See the B3 completion note above. Mutation-verified: restoring the `tableKey === 'transaction'` restriction fails `portal-realtime.test.ts` "streamHotBlocks: a DROPPABLE BLOCK-field 400 (dataset lacks mix_hash) degrades too…".
89-
- **Bounded realtime `/finalized-head` probe → INV-10 (availability).** The wire's `portalFinalizedHead` was a bare `fetch().then(r => r.json())` — no timeout, no abort signal, no body bound — while its historical twin had been explicitly hardened against exactly this (issue #14 / PR #16). One black-holed connection froze finalize emission mid-run (`portalRealtimeEvents` awaits the probe inline in the block loop) and startup (`clampFinalizedToPortalHead`'s 3-attempt retry never reached attempt 2). The probe implementation is now SHARED: `probeFinalizedHead` in portal-client.ts (two-phase connect-abort + own-the-lock body read, hash-carrying) backs both the client's `finalizedHead()` and the wire's `portalFinalizedHead()` — deleting the duplication that let the weak copy exist. Mutation-verified: restoring the bare fetch fails `portal-realtime-wire.test.ts` "portalFinalizedHead: a HUNG probe is BOUNDED…" (times out instead of resolving `undefined`).
89+
- **Bounded realtime `/finalized-head` probe → INV-10 (availability).** The wire's `portalFinalizedHead` was a bare `fetch().then(r => r.json())` — no timeout, no abort signal, no body bound — while its historical twin had been explicitly hardened against exactly this (issue #14 / PR #16). One black-holed connection froze finalize emission mid-run (`portalRealtimeEvents` awaits the probe inline in the block loop) and startup (`clampFinalizedToPortalHead`'s 3-attempt retry never reached attempt 2). The probe implementation is now SHARED: `probeFinalizedHead` in portal-client.ts (two-phase connect-abort + own-the-lock body read, hash-carrying) backs both the client's `finalizedHead()` and the wire's `portalFinalizedHead()` — deleting the duplication that let the weak copy exist. Mutation-verified: restoring the bare fetch fails `portal-realtime-wire.test.ts` "portalFinalizedHead: a HUNG probe is BOUNDED…" (times out instead of resolving `undefined`).
90+
91+
### Persisted-finality floor (stream-mode restart regression)
92+
93+
The wave-4 monotonic head cache and the wire's `emitFinalize` gate are both IN-MEMORY: neither survives a restart. In stream mode `clampFinalizedToPortalHead` adopts the startup `/finalized-head` probe as the finality boundary, and load-balanced replicas answer independently — so a restart can land on a replica whose head Y sits BELOW finality ponder already PERSISTED (safe checkpoint X, written by `finalizeOmnichain`/`finalizeMultichain` from an earlier finalize). Ponder's own migrate-time guard ("Finalized block for chain … cannot move backwards") checks only the RPC-derived finalized block, which is fetched BEFORE the clamp — the regressed Portal head bypasses it entirely. The consequence is not a rollback (finalized history never forks) but DOUBLE-INDEXING: crash recovery reverts user tables only ABOVE X (rows at/below X lost their reorg-table rows at finalize, by design), historical yields nothing (recovery checkpoint X > boundary Y), and realtime re-streams `(Y, X]` — ponder's realtime event loop has no filter against the recovery checkpoint, so every event in the range executes AGAIN on committed rows. It compounds: the first finalize in `(Y, X]` passes `emitFinalize` (its floor was seeded from the regressed Y) and `finalizeOmnichain` writes the checkpoint VERBATIM, regressing the persisted safe checkpoint itself. The same regression exists mid-run at the backfill-cutover refetch in the omnichain/multichain orderings: `shouldCatchup` is computed per chain but the new boundaries are assigned to ALL chains, so one chain's advance adopts another chain's stale-LOW probe (the isolated ordering is immune — its per-chain `break` refuses a non-advancing boundary).
94+
95+
The fix is the same shape as pipes-sdk's `FinalizedWatermark` (PR #88 there): a monotonic floor SEEDED FROM PERSISTED STATE. `clampFinalizedToPortalHead` takes a `floor` (block number) the boundary never goes below — a head below it is clamped UP to the floor (safe: `(head, floor]` is finalized history, identical across replicas; the lagging replica just hasn't re-marked it final) with a WARN log. The wiring passes ponder's persisted per-chain safe checkpoint (`crashRecoveryCheckpoint`, decoded via `checkpointBlockNumber`) at the startup site (`getLocalSyncProgress`) and the previously adopted boundary at the two cutover sites. `checkpointBlockNumber` is chainId-GUARDED (review, PR #55): in the omnichain ordering `finalizeOmnichain` writes the OMNICHAIN checkpoint verbatim to every chain's row (no per-chain where clause — "it is not an invariant that `chainId` and `checkpoint.chainId` are the same"), so a row's checkpoint can encode ANOTHER chain's block height; used as a floor it would either sit above the local RPC finalized block and disable the Portal-head clamp outright (the boundary lands above the Portal head — the wave-4 stream-mode stale-head FATAL, a deterministic crash-loop) or sit below the local safe point and silently under-protect. A checkpoint whose encoded chainId differs from the chain's yields NO floor (the pre-floor behavior; the multichain/isolated orderings write per-chain checkpoints and keep full protection) — mirroring upstream's own crash-recovery chainId guard in `runtime/historical.ts`, pinned by "a FOREIGN-chain checkpoint yields NO floor…". The floor also overrides a `PORTAL_FINALIZED_HEAD` pin below it — a pin below persisted finality would re-open the double-indexing hole, so correctness wins over the pin. Historical intervals at/below the floor are already cached in the sync store (they were finalized), so the floored boundary triggers no Portal data request a lagging replica would 204. Pinned by `portal-realtime-wire.test.ts` "a probed head BELOW the persisted floor is clamped UP to the floor…", "…floor at/above the RPC finalized block returns it unchanged…", "…the floor overrides a PORTAL_FINALIZED_HEAD pin below it…", and the inert-floor case "…a floor at/below the probed head is inert…".

portal/portal-realtime-wire.test.ts

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import type {
66
LightBlock,
77
LogFilter,
88
} from '@/internal/types.js';
9+
import { encodeCheckpoint, ZERO_CHECKPOINT } from '@/utils/checkpoint.js';
910
import type { PortalRealtimeEvent } from './portal-realtime.js';
1011
import {
1112
assertStreamModeSupported,
1213
buildPortalLogRequests,
14+
checkpointBlockNumber,
1315
clampFinalizedToPortalHead,
1416
discoverChildAddresses,
1517
getPortalRealtimeEventGenerator,
@@ -479,6 +481,148 @@ test('clampFinalizedToPortalHead: an explicit PORTAL_FINALIZED_HEAD pin below th
479481
expect(hexToNumber(out.number)).toBe(900);
480482
});
481483

484+
// ─────────────────────────────── persisted-finality floor ───────────────────────────────
485+
486+
// Shared scaffolding for the floor tests: RPC finalized at 1000, an rpc mock that records the block
487+
// request, and a floor passed in by the caller (as the wiring does from ponder's persisted checkpoint).
488+
const rpcFinalized1000 = {
489+
number: '0x3e8',
490+
hash: '0xh',
491+
parentHash: '0xp',
492+
timestamp: '0x1',
493+
} as LightBlock;
494+
const recordingRpc = (blockNumberHex: string) => {
495+
const calls: any[] = [];
496+
const rpc = {
497+
request: async (req: any) => {
498+
calls.push(req);
499+
return {
500+
number: blockNumberHex,
501+
hash: '0xfloorblock',
502+
parentHash: '0xpp',
503+
timestamp: '0x2',
504+
logsBloom: `0x${'0'.repeat(512)}`,
505+
sha3Uncles: '0x0',
506+
miner: '0x0',
507+
stateRoot: '0x0',
508+
transactionsRoot: '0x0',
509+
receiptsRoot: '0x0',
510+
gasUsed: '0x0',
511+
gasLimit: '0x0',
512+
extraData: '0x',
513+
nonce: '0x0',
514+
mixHash: '0x0',
515+
difficulty: '0x0',
516+
size: '0x0',
517+
transactions: [],
518+
};
519+
},
520+
} as any;
521+
522+
return { rpc, calls };
523+
};
524+
525+
test('clampFinalizedToPortalHead: a probed head BELOW the persisted floor is clamped UP to the floor — a restart against a lagging replica must not re-stream already-finalized (unrevertable) blocks', async () => {
526+
process.env.PORTAL_REALTIME = 'stream';
527+
// Last run persisted finality at 950; this restart's probe hits a lagging replica reporting 900.
528+
// Without the floor, realtime would stream from 901 and re-index (900, 950] — rows crash recovery
529+
// cannot revert (their reorg-table rows were deleted at finalize).
530+
const fetchImpl = (async () => ({
531+
json: async () => ({ number: 900 }),
532+
})) as any;
533+
const { rpc, calls } = recordingRpc('0x3b6'); // 950
534+
const out = await clampFinalizedToPortalHead({
535+
chain: { portal: 'http://p', name: 'c' } as any,
536+
rpc,
537+
finalizedBlock: rpcFinalized1000,
538+
floor: 950,
539+
fetchImpl,
540+
});
541+
expect(calls[0]!.params[0]).toBe('0x3b6'); // the boundary block is fetched at the FLOOR (950), not 900
542+
expect(hexToNumber(out.number)).toBe(950);
543+
});
544+
545+
test('clampFinalizedToPortalHead: a floor at/below the probed head is inert — the head is adopted exactly as before', async () => {
546+
process.env.PORTAL_REALTIME = 'stream';
547+
const fetchImpl = (async () => ({
548+
json: async () => ({ number: 900 }),
549+
})) as any;
550+
const { rpc, calls } = recordingRpc('0x384'); // 900
551+
const out = await clampFinalizedToPortalHead({
552+
chain: { portal: 'http://p', name: 'c' } as any,
553+
rpc,
554+
finalizedBlock: rpcFinalized1000,
555+
floor: 900, // == head → no effect
556+
fetchImpl,
557+
});
558+
expect(calls[0]!.params[0]).toBe('0x384'); // clamped to the head, same as the floorless behavior
559+
expect(hexToNumber(out.number)).toBe(900);
560+
});
561+
562+
test('clampFinalizedToPortalHead: a floor at/above the RPC finalized block returns it unchanged — the floor never RAISES the boundary past ponder’s own finality', async () => {
563+
process.env.PORTAL_REALTIME = 'stream';
564+
const fetchImpl = (async () => ({
565+
json: async () => ({ number: 900 }),
566+
})) as any;
567+
const { rpc, calls } = recordingRpc('0x0');
568+
const out = await clampFinalizedToPortalHead({
569+
chain: { portal: 'http://p', name: 'c' } as any,
570+
rpc,
571+
finalizedBlock: rpcFinalized1000,
572+
floor: 1_000, // floor == RPC finalized → pass-through, no block refetch
573+
fetchImpl,
574+
});
575+
expect(out).toBe(rpcFinalized1000);
576+
expect(calls.length).toBe(0);
577+
});
578+
579+
test('clampFinalizedToPortalHead: the floor overrides a PORTAL_FINALIZED_HEAD pin below it — a pin below persisted finality must not re-open the double-indexing hole', async () => {
580+
process.env.PORTAL_REALTIME = 'stream';
581+
process.env.PORTAL_FINALIZED_HEAD = '900'; // operator pin BELOW the persisted floor 950
582+
let probed = false;
583+
const fetchImpl = (async () => {
584+
probed = true;
585+
return { json: async () => ({ number: 5000 }) };
586+
}) as any;
587+
const { rpc, calls } = recordingRpc('0x3b6'); // 950
588+
const out = await clampFinalizedToPortalHead({
589+
chain: { portal: 'http://p', name: 'c' } as any,
590+
rpc,
591+
finalizedBlock: rpcFinalized1000,
592+
floor: 950,
593+
fetchImpl,
594+
});
595+
expect(probed).toBe(false); // the pin still suppresses the live probe
596+
expect(calls[0]!.params[0]).toBe('0x3b6'); // but the boundary is the FLOOR, not the pin
597+
expect(hexToNumber(out.number)).toBe(950);
598+
});
599+
600+
test('checkpointBlockNumber: decodes the block number out of a persisted checkpoint string; undefined stays undefined', () => {
601+
const checkpoint = encodeCheckpoint({
602+
...ZERO_CHECKPOINT,
603+
blockTimestamp: 1_700_000_000n,
604+
chainId: 1n,
605+
blockNumber: 12_345n,
606+
});
607+
expect(checkpointBlockNumber(checkpoint, 1)).toBe(12_345);
608+
expect(checkpointBlockNumber(undefined, 1)).toBeUndefined();
609+
});
610+
611+
test("checkpointBlockNumber: a FOREIGN-chain checkpoint yields NO floor — omnichain finalize writes the omnichain checkpoint verbatim to every chain's row, so its block height is another chain's", () => {
612+
// finalizeOmnichain updates PONDER_CHECKPOINT with no per-chain where clause: chain 1's row can carry
613+
// a checkpoint encoding chain 8453's block height. Used as chain 1's floor it either disables the
614+
// Portal-head clamp (foreign height above the local RPC finalized → the stream-mode stale-head FATAL,
615+
// a crash-loop) or silently under-protects (foreign height below the local safe point).
616+
const foreign = encodeCheckpoint({
617+
...ZERO_CHECKPOINT,
618+
blockTimestamp: 1_700_000_000n,
619+
chainId: 8_453n,
620+
blockNumber: 99_999_999n, // a Base-scale height, nonsense on an Ethereum-scale chain
621+
});
622+
expect(checkpointBlockNumber(foreign, 1)).toBeUndefined();
623+
expect(checkpointBlockNumber(foreign, 8_453)).toBe(99_999_999); // same chain → still a valid floor
624+
});
625+
482626
// ─────────────────────────────── end-to-end generator ───────────────────────────────
483627

484628
// mock the Portal /stream — one NDJSON connection per entry in `connections` (then 204) — and

0 commit comments

Comments
 (0)