Skip to content

fix(validator): separate an unexpected Inbox range-read failure from ordinary sync lag - #179

Draft
spalladino wants to merge 1 commit into
spl/fi-s07-continuation-sample-selectorfrom
spl/fi-s08-store-failure-diagnostics
Draft

spalladino wants to merge 1 commit into
spl/fi-s07-continuation-sample-selectorfrom
spl/fi-s08-store-failure-diagnostics

Conversation

@spalladino

@spalladino spalladino commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

A validator deriving the message bundle a streaming block proposal consumes reads a count range from its own
message source. Every failure of that read produces the same verdict, inbox_prefix_unavailable: a local-view
outcome that is retried through the bounded archiver sync and never treated as proposer misbehavior. That is
correct, but it flattens two very different situations. A node that is simply behind L1, or on the stale side of
an L1 reorg, fails that read as a matter of course and recovers on its own. A node whose store is faulted, or
whose message-source provider is broken, fails it for a reason the checks never anticipated, and an operator
currently has nothing in the logs to tell the two apart.

This rung keeps the two apart in the diagnostics only. An unanticipated failure now carries its bounded error
text on the check result, and the proposal handler's existing one-shot timeout warn — the log line already
emitted when the bounded sync runs out — reports it. Ordinary sync lag attaches nothing and stays silent.

The validator's behavior toward the proposal is unchanged. The verdict, its retryability, the precedence of
the checks and the non-punitive classification are all exactly what they were; only what an operator can see
differs. The tests assert this directly: making the unexpected-failure path punitive reddens them.

The classification matches on the error's message text, not on its class. That normally reads as a smell and
here it is deliberate: the validator's message source can sit behind a JSON-RPC hop, which leaves the archiver's
typed range error class behind while preserving its message. The reason is recorded in a comment on the fragment
list so nobody "fixes" it into an instanceof check later.

The attached text is bounded at 200 characters (MAX_REPORTED_ERROR_LENGTH), so a pathological provider error
string cannot blow up a log record. A test pins the bound against a 5000-character message.

Stack position

This is rung 16 of 25 in the Fast Inbox node stack, stacked on spl/fi-s07-continuation-sample-selector.

Sources

Reconstructs the final state of 198f554ac9 ("separate an unexpected Inbox range-read failure from ordinary sync
lag") as superseded by the validator-client hunks only of 63b803aad1 ("drop dead resolution branch and
global range-read log cache"), both from old PR
#25440.

63b803aad1 is the final simplification of 198f554ac9, so the simplified shape was implemented directly rather
than porting the first commit and then deleting most of it: there is no module logger, no process-wide dedupe
Set of already-reported ranges, and no separate truncation constant for a log line. The unexpected error's
bounded text rides on the result and the handler's existing warn reports it.

Files: validator-client/src/streaming_inbox_checks.ts (optional error on the failing
StreamingBlockCheckResult, the EXPECTED_RANGE_READ_FAILURES fragment list, the 200-character bound and the
unexpectedRangeReadError classifier), validator-client/src/proposal_handler.ts (two lines: the error field
on the timeout warn and the comment explaining when it is set), and
validator-client/src/streaming_inbox_checks.test.ts.

Tests run

  • yarn build from yarn-project: green.
  • yarn format and yarn lint over the whole project: clean, no residue.
  • yarn workspace @aztec-labs/validator-client test (full package): base on the parent branch was
    10 suites / 279 passed / 3 skipped; on this branch 10 suites / 283 passed / 3 skipped. The four extra
    tests are exactly the ones added here.
  • No e2e suites were run; this rung has no e2e surface and CI covers the package.

Red/green, with the production change stashed and the tests left in place:

  • carries the error text of a range read that failed for an unanticipated reason fails —
    - "error": "database is closed" missing from the result.
  • bounds the error text so a verbose provider failure cannot blow up a log record fails the same way.
  • The two attaches no context to ordinary sync lag cases (one per expected fragment) pass on the unchanged
    code, as intended: they are the guard that the quiet path stays quiet, and they redden if anyone later attaches
    context unconditionally.
  • Verdict-unchanged proof: mutating readStreamingBlockBundle so the unexpected branch returns a non-retryable
    reason reddens both unexpected-failure tests. Each test asserts the whole result object plus
    isRetryableStreamingBlockCheckReason(reason) === true, so any move toward punishing a store fault is caught.

Deliberately left out

  • The prover-client hunk of 63b803aad1 — the dead re-enqueue branch in the block-root proof callback of
    checkpoint-sub-tree-orchestrator.ts — is not in this PR. It belongs to the deferred proof-lifecycle work and must not enter this stack. Only the validator-client hunks of that
    commit were ported.
  • No whole-duty abstraction (DutyBudget, the deferred whole-duty timeout work). Nothing in this change touches duty bounds, cancellation or
    late-continuation cleanup, which are in the immediate neighbourhood of the handler code this PR edits.
  • No global log cache. The dedupe Set and its size cap from 198f554ac9 were never introduced.
  • The checkpoint-level prefix wait (awaitCheckpointConsumedMessages) reads its range through a separate path
    whose result type carries no error field, and the source left it untouched; this PR does the same. If the same
    diagnostics are wanted there, it is a follow-up, not a silent widening of this rung.

…ordinary sync lag

Every failure of the bundle range read yields the same non-punitive verdict, but a store fault or a broken
provider is not the same thing as a node that is behind L1. An unexpected failure now carries its bounded error
text on the check result, and the proposal handler's existing one-shot timeout warn reports it, so an operator
can tell the two apart without changing what the validator does with the proposal. Genuine lag stays silent.

The classification matches on message text, since the archiver's typed range error does not survive a JSON-RPC
hop between the validator and its message source.
@spalladino
spalladino added this pull request to stack #188 September 12, 2026 04:52
@spalladino spalladino added the ci-draft Run CI on this draft PR label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-draft Run CI on this draft PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant