Skip to content

Commit 96c6079

Browse files
dzhelezovclaude
andcommitted
fix(portal): head-clamp ceilings, construction-time discovery floor, extend-local field checks, trace receipts, head-pin (bug-hunt wave 2A) [WIP: source only]
Five correctness fixes from a triple-blind bug hunt. SOURCE changes only in this commit; regression tests + INVARIANTS.md rows + mutation-verification are follow-up (see the WIP PR body). Two of the five were empirically reproduced (BUG-A discovery floor, BUG-B head-clamp). FIX 1 — head-clamp root cause (BUG-B). dataEnd() = min(backfillEnd??∞, portalHead??∞) so desiredTo/coveredTo/endHint/raEnd are all head-clamped even when every source is bounded; removed the `spec.backfillEnd !== undefined` escape from the INV-9 assert (now strictly stronger — holds by construction). FIX 2 — discovery floor at construction (BUG-A). Floor derived from the spec (min over factories of fromBlock ?? 0), pinned at construction and re-pinned per call before any fetch; requiredFactoryIntervals/interval-start only refine it downward. Dropped the `discStartIdx === undefined` escapes from the INV-3 asserts. FIX 3 — extend-local neededMissing. The needed-field crash check now compares the matched-data map sizes before/after THIS runStreams call, so a data-bearing base + event-less extend tail (dataset lacking a needed column) is tolerated, not crash-looped. FIX 4 — trace/transfer receipts. buildTraces now emits toSyncReceipt for matched trace txs when a trace/transfer filter has hasTransactionReceipt (deduped by hash across all branches), so buildEvents no longer throws "Missing transaction receipt". FIX 5 — PORTAL_FINALIZED_HEAD pin. ensureChunkSize only adopts the live probe as the head when there is no pin (scaling still uses the live value), so the pin governs finality/delegation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 94b804b commit 96c6079

2 files changed

Lines changed: 106 additions & 43 deletions

File tree

portal/portal-assemble.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,15 @@ const buildMatchers = (
165165
};
166166
};
167167

168-
/** Per-chunk trace assembly: full-tree ranking then client-side filtering (INV-5). */
168+
/** Per-chunk trace assembly: full-tree ranking then client-side filtering (INV-5). `onReceipt` (FIX 4)
169+
* is invoked with each MATCHED trace's parent transaction so the caller can emit a receipt when a
170+
* trace/transfer filter has `hasTransactionReceipt` — the log/tx branches never see these txs. */
169171
const buildTraces = (
170172
cd: ChunkData,
171173
lo: number,
172174
hi: number,
173175
matchers: Matchers,
176+
onReceipt?: (rawTx: RawTx, header: RawHeader) => void,
174177
): { trace: SyncTrace; block: SyncBlock; transaction: SyncTransaction }[] => {
175178
const out: {
176179
trace: SyncTrace;
@@ -195,6 +198,7 @@ const buildTraces = (
195198
const rawTx = txByIdx.get(txIndex);
196199
for (const { frame } of rankTraces(traces)) {
197200
if (!matchers.traceMatched(frame, bn)) continue;
201+
if (rawTx) onReceipt?.(rawTx, tb.header); // FIX 4: receipt for the matched trace's tx
198202
out.push({
199203
trace: {
200204
trace: frame,
@@ -243,6 +247,15 @@ export function assembleRange(
243247
const syncTxs: SyncTransaction[] = [];
244248
const syncReceipts: SyncTransactionReceipt[] = [];
245249
const seenTx = new Set<string>();
250+
// FIX 4: receipts are deduped by tx hash across ALL branches (log, tx-filter, trace/transfer) — a tx
251+
// matched by more than one source must yield exactly one receipt row.
252+
const seenReceipt = new Set<string>();
253+
const pushReceipt = (raw: RawTx, hdr: RawHeader): void => {
254+
if (!raw.hash || seenReceipt.has(raw.hash)) return;
255+
256+
seenReceipt.add(raw.hash);
257+
syncReceipts.push(toSyncReceipt(raw, hdr));
258+
};
246259

247260
// INV-2 is enforced BY CONSTRUCTION here: every emitting branch below sits behind the single
248261
// `inRange` predicate (there is deliberately no redundant per-row assert — it could never fire),
@@ -258,7 +271,7 @@ export function assembleRange(
258271
if (tx.hash && !seenTx.has(tx.hash)) {
259272
seenTx.add(tx.hash);
260273
syncTxs.push(toSyncTransaction(tx, hdr));
261-
if (spec.needReceipts) syncReceipts.push(toSyncReceipt(tx, hdr));
274+
if (spec.needReceipts) pushReceipt(tx, hdr);
262275
}
263276
}
264277
}
@@ -285,13 +298,20 @@ export function assembleRange(
285298
if (raw.hash) seenTx.add(raw.hash);
286299
blocksByNumber.set(bn, toSyncBlockHeader(tb.header));
287300
syncTxs.push(tx);
288-
if (spec.needReceipts)
289-
syncReceipts.push(toSyncReceipt(raw, tb.header));
301+
if (spec.needReceipts) pushReceipt(raw, tb.header);
290302
}
291303
}
292304

305+
// FIX 4: a trace/transfer filter with hasTransactionReceipt needs a receipt for every matched trace tx;
306+
// those txs ride on the trace query (fields include RECEIPT_FIELDS when needReceipts) and are seen by no
307+
// other branch, so emit them here (deduped via pushReceipt). Without this, buildEvents throws
308+
// "Missing transaction receipt" on a legit trace-receipt config.
309+
const needTraceReceipts =
310+
spec.traceFilters.some((f) => f.hasTransactionReceipt) ||
311+
spec.transferFilters.some((f) => f.hasTransactionReceipt);
312+
const onTraceReceipt = needTraceReceipts ? pushReceipt : undefined;
293313
const traces = spec.needTraces
294-
? chunks.flatMap((cd) => buildTraces(cd, lo, hi, matchers))
314+
? chunks.flatMap((cd) => buildTraces(cd, lo, hi, matchers, onTraceReceipt))
295315
: [];
296316

297317
// C9: highest block with data — a LOOP (Math.max(...spread) RangeErrors on ~100k+ keys) — INCLUDING

portal/portal.ts

Lines changed: 81 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -124,13 +124,42 @@ export const createPortalHistoricalSync = (
124124
const delegated = new Set<string>(); // interval keys routed to RPC (finality gap)
125125
let chunkBlocks = cfg.chunkBlocks;
126126
let chunkSizeP: Promise<void> | undefined;
127-
let discStartIdx: number | undefined; // factory-deploy chunk = discovery floor (C4: clamps DOWNWARD)
128127
let portalHead: number | undefined = cfg.finalizedHead;
129128
let startTime = 0;
130129

130+
// FIX 2 (INV-3/INV-15): the discovery floor is the earliest block ANY factory could create a child —
131+
// `min` over the compiled spec's factories of `fromBlock ?? 0` (undefined ⇒ genesis). It is a property
132+
// of the SPEC, not of the intervals ponder happens to still need, so it is pinned at CONSTRUCTION (and
133+
// re-pinned per call, since chunkBlocks can scale) — see `pinDiscoveryFloor`. `discFloorBlock` is the
134+
// downward-clamped floor BLOCK (grid-independent, so it survives a rescale); requiredFactoryIntervals /
135+
// the interval start only REFINE it downward (C4/INV-4).
136+
const specFloorBlock =
137+
spec.factories.length > 0
138+
? Math.min(...spec.factories.map((f) => f.fromBlock ?? 0))
139+
: undefined;
140+
let discFloorBlock = specFloorBlock;
141+
131142
const ikey = (i: Interval): string => `${i[0]}-${i[1]}`;
143+
// FIX 1 (INV-9/INV-13): the data ceiling is the LOWER of the configured backfill end and the live Portal
144+
// head — clamp BOTH. With every source bounded (`backfillEnd` defined) the old `backfillEnd ?? portalHead`
145+
// ignored the head, so a frontier chunk's `desiredTo`/`coveredTo` extended PAST the head; the Portal
146+
// 204s/truncates above its head, `coveredTo` recorded phantom coverage, and once the head advanced later
147+
// intervals blind-hit the stale cache and were marked synced EMPTY (permanent silent gap). Clamping here
148+
// flows to desiredTo, coveredTo, endHint and raEnd, and re-arms the INV-13 extend as the head advances.
132149
const dataEnd = (): number =>
133-
spec.backfillEnd ?? portalHead ?? Number.POSITIVE_INFINITY;
150+
Math.min(
151+
spec.backfillEnd ?? Number.POSITIVE_INFINITY,
152+
portalHead ?? Number.POSITIVE_INFINITY,
153+
);
154+
155+
// FIX 2: snap `discFloorBlock` to the current grid and install it as the discovery floor. Idempotent —
156+
// called at construction and again on every syncBlockRangeData before any fetch (chunkBlocks may have
157+
// scaled since). No-op when there are no factories.
158+
const pinDiscoveryFloor = (): void => {
159+
if (discFloorBlock === undefined) return;
160+
161+
discovery.setFloor(idxOf(discFloorBlock, chunkBlocks) * chunkBlocks);
162+
};
134163

135164
// finality-gap fallback: Portal serves only finalized data, and its finalized head can (rarely) lag
136165
// Ponder's target. Any interval reaching past the head is delegated whole to the stock RPC sync.
@@ -161,7 +190,12 @@ export const createPortalHistoricalSync = (
161190

162191
const h = await client.finalizedHead();
163192
if (h !== undefined) {
164-
portalHead = h;
193+
// FIX 5: a live probe drives chunk SCALING, but it must NOT overwrite an explicit
194+
// PORTAL_FINALIZED_HEAD pin — the pin is authoritative for the finality/delegation decision, and
195+
// clobbering it here (which happened whenever the pin was set but PORTAL_CHUNK_FIXED was not) let
196+
// intervals above the pin but below the live head be served instead of delegated. Only adopt the
197+
// probe as the head when there is no pin; scaling may still use the live `h`.
198+
if (cfg.finalizedHead === undefined) portalHead = h;
165199
chunkBlocks = scaleChunkBlocks(cfg.chunkBlocks, h);
166200
log.debug({
167201
service: 'portal',
@@ -184,6 +218,17 @@ export const createPortalHistoricalSync = (
184218
token: RowToken,
185219
): Promise<void> => {
186220
const neededMissing = new Set<string>();
221+
// FIX 3: the needed-field crash check must consider ONLY the rows THIS call adds. On a frontier EXTEND
222+
// `cd` already carries the base chunk's data; the tail streams the disjoint range (coveredTo, desiredTo]
223+
// whose block numbers are all > coveredTo (new map keys), so a size delta captures exactly the tail's
224+
// matched rows. Inspecting the whole accumulated `cd` (as before) let a data-bearing base + an
225+
// event-less tail whose dataset lacks a needed column throw fatally → evict → crash-loop on retry.
226+
const matchedSize = (): number =>
227+
cd.logs.size +
228+
cd.traceBlocks.size +
229+
cd.txBlocks.size +
230+
cd.blockHeaders.size;
231+
const matchedBefore = matchedSize();
187232
const onRows = (n: number): void => {
188233
if (token.freed) return;
189234

@@ -268,15 +313,10 @@ export const createPortalHistoricalSync = (
268313
});
269314
}
270315
}
271-
// A NEEDED field the dataset lacked on THIS range: crash ONLY IF the chunk yielded MATCHED data —
272-
// an event the indexer processes would be incomplete. Event-less (old/irrelevant) ranges proceed.
273-
if (
274-
neededMissing.size &&
275-
(cd.logs.size ||
276-
cd.traceBlocks.size ||
277-
cd.txBlocks.size ||
278-
cd.blockHeaders.size)
279-
) {
316+
// A NEEDED field the dataset lacked on THIS range: crash ONLY IF this call added MATCHED data — an
317+
// event the indexer processes would be incomplete. An event-less (old/irrelevant) range — or an
318+
// event-less EXTEND tail over a data-bearing base (FIX 3) — proceeds.
319+
if (neededMissing.size && matchedSize() > matchedBefore) {
280320
throw new Error(
281321
`Portal dataset for ${chain.name} is missing [${[...neededMissing].join(', ')}] on blocks [${from},${to}], which contain matched data your indexer needs — a Portal dataset-completeness gap. Failing fast rather than serving incomplete data; report the gap to SQD, or start your indexer past the affected range.`,
282322
);
@@ -294,19 +334,21 @@ export const createPortalHistoricalSync = (
294334
spec.backfillStart,
295335
dataEnd(),
296336
);
297-
// INV-9: a Portal data request never targets past the known finalized head (an explicit backfill
298-
// toBlock may exceed it by configuration — the delegation branch already guards served intervals).
337+
// INV-9: a Portal data request never targets past the known finalized head. `dataEnd()` now clamps to
338+
// the head (FIX 1), so `desiredTo <= portalHead` holds BY CONSTRUCTION whenever the head is known — the
339+
// former `spec.backfillEnd !== undefined` escape (which let a bounded backfill over-reach) is gone and
340+
// this assert is a strictly stronger tripwire. Intervals past the head are already delegated to RPC.
299341
invariant(
300342
'INV-9',
301-
portalHead === undefined ||
302-
desiredTo <= portalHead ||
303-
spec.backfillEnd !== undefined,
343+
portalHead === undefined || desiredTo <= portalHead,
304344
'data request targets past the Portal finalized head',
305345
() => ({ idx, desiredTo, portalHead }),
306346
);
347+
// Discovery scans as far as the backfill will need in one pass; head-clamped (FIX 1) via dataEnd().
348+
const de = dataEnd();
307349
const ensureOpts = {
308350
chunkBlocks,
309-
endHint: spec.backfillEnd ?? portalHead ?? desiredTo,
351+
endHint: Number.isFinite(de) ? de : desiredTo,
310352
};
311353

312354
const cached = dataCache.get(idx);
@@ -332,11 +374,12 @@ export const createPortalHistoricalSync = (
332374
const extended = (async (): Promise<ChunkData> => {
333375
const cd = await prev;
334376
await discovery.ensure(desiredTo, ensureOpts); // discovery must reach the extended tail too
377+
// FIX 2: the floor is pinned from the spec at construction, so a factory sync always has a floor —
378+
// the former `discStartIdx === undefined` escape (which silently disabled this check on the very
379+
// chunks that fetched before requiredFactoryIntervals arrived) is gone.
335380
invariant(
336381
'INV-3',
337-
spec.factories.length === 0 ||
338-
discStartIdx === undefined ||
339-
discovery.through() >= desiredTo,
382+
spec.factories.length === 0 || discovery.through() >= desiredTo,
340383
'chunk extend under a stale discovery watermark',
341384
() => ({ idx, through: discovery.through(), desiredTo }),
342385
);
@@ -358,11 +401,10 @@ export const createPortalHistoricalSync = (
358401
const token: RowToken = { rows: 0, freed: false };
359402
const p = (async (): Promise<ChunkData> => {
360403
await discovery.ensure(desiredTo, ensureOpts); // children ≤ this chunk are known (INV-3)
404+
// FIX 2: floor pinned from the spec at construction ⇒ no `discStartIdx === undefined` escape (see extend).
361405
invariant(
362406
'INV-3',
363-
spec.factories.length === 0 ||
364-
discStartIdx === undefined ||
365-
discovery.through() >= desiredTo,
407+
spec.factories.length === 0 || discovery.through() >= desiredTo,
366408
'data fetch under a stale discovery watermark',
367409
() => ({ idx, through: discovery.through(), desiredTo }),
368410
);
@@ -400,6 +442,10 @@ export const createPortalHistoricalSync = (
400442
}
401443
};
402444

445+
// FIX 2: pin the discovery floor from the spec NOW, before the first fetch (re-pinned per call once
446+
// chunkBlocks is finalized). A no-op when there are no factory sources.
447+
pinDiscoveryFloor();
448+
403449
return {
404450
async syncBlockRangeData(params) {
405451
const { interval, requiredFactoryIntervals, syncStore } = params;
@@ -454,28 +500,25 @@ export const createPortalHistoricalSync = (
454500
chunkBlocks = capped;
455501
for (const entry of dataCache.values()) freeToken(entry.token);
456502
dataCache.clear();
457-
discStartIdx = undefined;
458-
discovery.reset();
503+
discovery.reset(); // discFloorBlock (block-space) survives; re-pinned to the new grid just below
459504
log.debug({
460505
service: 'portal',
461506
msg: `Portal ${chain.name}: dense sources → chunkBlocks capped to ${chunkBlocks} (grid reset)`,
462507
});
463508
}
464509

465-
// pin the discovery floor at the factory's real start (NOT block 0). C4: clamp DOWNWARD only.
466-
if (requiredFactoryIntervals.length > 0) {
467-
const floor = idxOf(
468-
Math.min(
469-
...requiredFactoryIntervals
470-
.map((r) => r.interval[0])
471-
.concat(interval[0]),
472-
),
473-
chunkBlocks,
510+
// FIX 2: (re-)pin the discovery floor BEFORE any fetch. Base floor = the spec's earliest factory
511+
// start (`discFloorBlock`, seeded at construction); requiredFactoryIntervals and the interval start
512+
// only REFINE it downward (C4/INV-4). Applied on EVERY call (not just when ponder hands over
513+
// requiredFactoryIntervals) so an early spanning-chunk fetch never runs without discovery.
514+
if (discFloorBlock !== undefined) {
515+
discFloorBlock = Math.min(
516+
discFloorBlock,
517+
interval[0],
518+
...requiredFactoryIntervals.map((r) => r.interval[0]),
474519
);
475-
discStartIdx =
476-
discStartIdx === undefined ? floor : Math.min(discStartIdx, floor);
477-
discovery.setFloor(discStartIdx * chunkBlocks);
478520
}
521+
pinDiscoveryFloor();
479522

480523
const startIdx = idxOf(interval[0], chunkBlocks);
481524
const endIdx = idxOf(interval[1], chunkBlocks);

0 commit comments

Comments
 (0)