fix: bound and bind the Inbox endpoint gate, and stop it retracting a valid outcome

Four fixes from a review of the endpoint gate.

An endpoint refusal no longer overwrites a `valid` outcome this node already recorded for the same checkpoint.
The gate runs on both calls p2p makes for one proposal, so a second look failing on a local RPC problem used to
downgrade the slot to `unvalidated` — which the sentinel counts as a missed proposal for that slot's proposer,
feeding epoch performance and the inactivity signal. Only that exact checkpoint is protected; another archive at
the slot still records normally. The test helper asserting refusals is renamed to say what it checks: the refusal
is not slashable and marks no invalid slot, but it does record `unvalidated`, which is not a neutral outcome.

The verdict is bound to the identity of the block it was read at, not to a height. The head is read for its number
and hash, the resolution is pinned to that number, and the block is read again afterwards: a provider serving a
stale fork, or one the chain reorged under, answers a call at a height as readily as the canonical chain, so an
answer whose block is no longer the one at that height is refused as unverifiable rather than passed. A provider
that lags uniformly is still invisible from here, and the README says so.

The advertised two-second ceiling is now a real bound. It was only handed to `retryUntil`, which checks its
deadline after an attempt returns, so one stalled RPC consumed the whole remaining duty; it is now a race, via a
new `DutyBudget.runWithin`, and the abandoned loop checks the signal instead of starting another read.

Tracker pruning no longer runs in the acceptance path. It reads L1 tips, and the restructure had put it on the
cached path too, where the validator calls this method directly without an outer timeout — a hanging tips read
stalled the attestation. It is bookkeeping, so it runs detached, and the pipelining parent is not recorded at all
once the duty has been stopped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: spalladino

yarn-project/validator-client/README.md

@@ -130,21 +130,35 @@ the signed rolling hash; it says nothing about whether that bucket has settled.
 the still-open current bucket therefore passes here and is still rejected by `propose` with
 `Rollup__InboxBucketStillMutable`. Settlement remains an L1-only check that this one does not replace.
 
-Load: one head read plus one `eth_call` per checkpoint proposal validated, that is per slot, and a second pair on
-validators when the attestation path reuses a cached valid verdict. The head is read to pin the call to an explicit
-L1 block, so the verdict names the view it was made in; viem caches it briefly, and the contract wrapper's own
-block-tag guard reads it again. Failures are re-read for up to two seconds, bounded by the slot's duty budget.
-
-The check never fails open. An unreadable L1 view (RPC outage, timeout, a provider trailing the head, a block the
-provider will not serve) is reported as `inbox_endpoint_unverifiable`, and a view that answers without showing the
-signed position ending a live bucket (interior position, evicted endpoint, different rolling hash) as
-`inbox_endpoint_not_live`. Both are refusals to validate now, not accusations: the bucket ring, the local provider
-and L1 itself all move independently of the moment the proposal was signed. Neither reaches slashing, the
+The resolution is bound to the identity of the block it was read at, not to a height: the head is read for its
+number and hash, the `eth_call` is pinned to that number, and the block is read again afterwards. A provider serving
+a stale fork, or one the chain reorged under, answers a call at a height as readily as the canonical chain does, so
+an answer whose block is no longer the one at that height names no view and cannot verify anything. What this does
+not detect is a provider that lags uniformly: its own view is self-consistent, and only the endpoint the node's
+provider can see is ever checked. Load is therefore two block reads plus one `eth_call` per checkpoint proposal
+validated, that is per slot, and a second set on validators when the attestation path reuses a cached valid verdict.
+
+A failure is re-read for up to two seconds. That window is a ceiling on the whole step, enforced as a race and
+capped by whatever is left of the slot's duty budget: a provider that accepts the call and never answers is
+abandoned at it, cannot start another read afterwards, and leaves the following stages their remaining budget.
+
+The check never fails open. An unreadable or unidentifiable L1 view (RPC outage, timeout, a block the provider will
+not serve, a block replaced under the call) is reported as `inbox_endpoint_unverifiable`, and a view that answers
+without showing the signed position ending a live bucket (interior position, evicted endpoint, different rolling
+hash) as `inbox_endpoint_not_live`. Both are refusals to validate now, not accusations: the bucket ring, the local
+provider and L1 itself all move independently of the moment the proposal was signed. Neither reaches slashing, the
 invalid-proposal slot marker or a peer penalty, and neither is remembered as the proposal's verdict, so a view that
-recovers within the slot still permits a valid verdict. The proposer's own checkpoints are covered by the endpoint
-its sequencer resolved against the same live ring when it built the checkpoint's final block, plus the publication
-preflight it runs before submitting. Historical checkpoints ingested by the archiver and checkpoints replayed for
-proving are outside this gate: they may reference endpoints the ring has long evicted.
+recovers within the slot still permits a valid verdict.
+
+A refusal is not free, though: like every other outcome this node cannot complete, it records `unvalidated` for the
+slot, which the sentinel reports as a missed proposal for that slot's proposer when no checkpoint for it lands on
+L1. What it will not do is overwrite a `valid` this node already recorded for the same checkpoint, so a later RPC
+failure here cannot retract a validation that succeeded.
+
+The proposer's own checkpoints are covered by the endpoint its sequencer resolved against the same live ring when
+it built the checkpoint's final block, plus the publication preflight it runs before submitting. Historical
+checkpoints ingested by the archiver and checkpoints replayed for proving are outside this gate: they may reference
+endpoints the ring has long evicted.
 
 ### Attestation Creation
 

yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts

@@ -95,7 +95,43 @@ describe('checkInboxEndpoint', () => {
     await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
       verified: false,
       reason: 'unreadable',
+      l1BlockNumber: 900n,
       err,
     });
   });
+
+  // A height is not an identity: a provider serving a stale fork answers a call at it as readily as the canonical
+  // chain does. The block is re-read afterwards, and an answer that belongs to a block that is no longer there
+  // names no view, so it cannot verify anything — including a bucket that matches exactly.
+  it('refuses a matching answer read at a block that is no longer the one at that height', async () => {
+    const inbox = makeFakeInbox(ring);
+    inbox.onRead(() => inbox.setViewHash('0xreplaced'));
+
+    await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
+      verified: false,
+      reason: 'view_replaced',
+      l1BlockNumber: 900n,
+    });
+  });
+
+  it('refuses when the block the answer was read at cannot be identified afterwards', async () => {
+    const inbox = makeFakeInbox(ring);
+    inbox.onRead(() => inbox.setViewHash(null));
+
+    await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toMatchObject({
+      verified: false,
+      reason: 'unreadable',
+      l1BlockNumber: 900n,
+    });
+  });
+
+  it('refuses when the head answers without a block identity', async () => {
+    const inbox = makeFakeInbox(ring);
+    inbox.setViewHash(null);
+
+    await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toMatchObject({
+      verified: false,
+      reason: 'unreadable',
+    });
+  });
 });

yarn-project/validator-client/src/checkpoint_endpoint_check.ts

@@ -1,14 +1,22 @@
 import type { InboxContract } from '@aztec/ethereum/contracts';
-import type { ViemClient } from '@aztec/ethereum/types';
 import type { Fr } from '@aztec/foundation/curves/bn254';
 
+/** The identity of the L1 block a read was made at: a height alone is not one, since a fork answers at it too. */
+type L1View = { number: bigint; hash: string };
+
 /**
- * The L1 reads the checkpoint endpoint check makes: the bucket resolution itself, and the head it is made at, so a
- * verdict names the L1 view that produced it instead of mixing results from a `latest` that moves between reads.
- * {@link InboxContract} satisfies it directly; nothing else needs to be fetched to answer the question.
+ * The L1 reads the checkpoint endpoint check makes: the bucket resolution itself, and the block it is made at,
+ * read before and after so a verdict names the L1 view that produced it instead of mixing results from a `latest`
+ * that moves between reads. {@link InboxContract} and a viem client satisfy it directly; nothing else needs to be
+ * fetched to answer the question.
  */
 export type InboxEndpointReader = Pick<InboxContract, 'getBucketAtOrBeforeTotal'> & {
-  client: Pick<ViemClient, 'getBlockNumber'>;
+  client: {
+    getBlock(args: {
+      blockNumber?: bigint;
+      includeTransactions: false;
+    }): Promise<{ number: bigint | null; hash: string | null } | undefined>;
+  };
 };
 
 /** Why a checkpoint's final message position is not a live Inbox bucket endpoint in the L1 view that was read. */
@@ -23,12 +31,18 @@ export type InboxEndpointRejection =
 /**
  * The outcome of one endpoint check. A rejection describes the L1 view that was read at `l1BlockNumber`, not the
  * proposer: the bucket ring, this node's provider and the chain itself all move independently of the moment the
- * checkpoint was signed. `unreadable` is a view this node could not obtain at all.
+ * checkpoint was signed. The last two are views this node could not obtain, or could not identify, at all.
  */
 export type InboxEndpointCheckResult =
   | { verified: true; l1BlockNumber: bigint; bucketSeq: bigint }
   | { verified: false; reason: InboxEndpointRejection; l1BlockNumber: bigint; endpointTotal?: bigint }
-  | { verified: false; reason: 'unreadable'; err: unknown };
+  /** A read threw, or answered without a block identity: the provider is unreachable, erroring or unsynced. */
+  | { verified: false; reason: 'unreadable'; l1BlockNumber?: bigint; err: unknown }
+  /** The block the resolution was read at is no longer the one at that height, so the answer names no view. */
+  | { verified: false; reason: 'view_replaced'; l1BlockNumber: bigint };
+
+/** What an L1 block read answers with when it names no block: no verdict can be bound to a view like that. */
+const UNIDENTIFIED_BLOCK = 'the L1 block was returned without a number or a hash';
 
 /**
  * Confirms through L1 that `totalMsgCount` is the end of a live Inbox bucket committing to `inboxRollingHash`.
@@ -38,15 +52,34 @@ export type InboxEndpointCheckResult =
  * below the bound, so only an exact total is a match: a lower one means the position sits inside a bucket. The
  * bucket's own rolling hash then has to be the one the checkpoint signed, or the boundary commits to different
  * message content than the checkpoint was built on.
+ *
+ * The resolution is read at one captured block and bound to that block's identity: a provider serving a stale
+ * fork, or one the chain reorged under, answers a call by height as readily as the canonical chain does. Re-reading
+ * the block at that height afterwards is what names the view the answer came from, and a view that cannot be shown
+ * to be the one queried yields a refusal rather than a pass.
  */
 export async function checkInboxEndpoint(
   inbox: InboxEndpointReader,
   totalMsgCount: bigint,
   inboxRollingHash: Fr,
 ): Promise<InboxEndpointCheckResult> {
+  let queried: L1View | undefined;
   try {
-    const l1BlockNumber = await inbox.client.getBlockNumber();
+    queried = await readL1View(inbox.client);
+    if (queried === undefined) {
+      return { verified: false, reason: 'unreadable', err: UNIDENTIFIED_BLOCK };
+    }
+    const l1BlockNumber = queried.number;
     const found = await inbox.getBucketAtOrBeforeTotal(totalMsgCount, { blockNumber: l1BlockNumber });
+
+    const confirmed = await readL1View(inbox.client, l1BlockNumber);
+    if (confirmed === undefined) {
+      return { verified: false, reason: 'unreadable', l1BlockNumber, err: UNIDENTIFIED_BLOCK };
+    }
+    if (confirmed.hash !== queried.hash) {
+      return { verified: false, reason: 'view_replaced', l1BlockNumber };
+    }
+
     if (found === undefined) {
       return { verified: false, reason: 'no_live_endpoint', l1BlockNumber };
     }
@@ -59,6 +92,12 @@ export async function checkInboxEndpoint(
     }
     return { verified: true, l1BlockNumber, bucketSeq: found.seq };
   } catch (err) {
-    return { verified: false, reason: 'unreadable', err };
+    return { verified: false, reason: 'unreadable', l1BlockNumber: queried?.number, err };
   }
 }
+
+/** Reads the block at `blockNumber`, or the head when it is omitted, as a number and hash that identify it. */
+async function readL1View(client: InboxEndpointReader['client'], blockNumber?: bigint): Promise<L1View | undefined> {
+  const block = await client.getBlock({ blockNumber, includeTransactions: false });
+  return block?.number == null || block.hash == null ? undefined : { number: block.number, hash: block.hash };
+}

yarn-project/validator-client/src/duty_budget.ts

@@ -1,3 +1,4 @@
+import { TimeoutError } from '@aztec/foundation/error';
 import type { DateProvider } from '@aztec/foundation/timer';
 import { execWithSignal } from '@aztec/foundation/timer';
 
@@ -86,4 +87,25 @@ export class DutyBudget {
     const signal = AbortSignal.any([this.controller.signal, AbortSignal.timeout(remainingMs)]);
     return await execWithSignal(fn, signal, () => new DutyBudgetExpiredError(what, this.deadline));
   }
+
+  /**
+   * Runs `fn` bounded by the shorter of the budget and `withinMs`, for a stage that advertises a ceiling of its
+   * own. The ceiling is the race, not a deadline the stage consults between attempts: an attempt that never
+   * settles is abandoned at it, and what is left of the duty stays available to the stages after it.
+   *
+   * Like {@link run}, the abandoned attempt keeps running, so `fn` has to honour the signal it is handed rather
+   * than start anything further with it.
+   */
+  public async runWithin<T>(what: string, withinMs: number, fn: (signal: AbortSignal) => Promise<T>): Promise<T> {
+    if (this.controller.signal.aborted) {
+      throw new DutyBudgetExpiredError(what, this.deadline);
+    }
+    const boundMs = Math.min(this.remainingMs() || EXPIRED_BUDGET_GRACE_MS, withinMs);
+    const signal = AbortSignal.any([this.controller.signal, AbortSignal.timeout(boundMs)]);
+    return await execWithSignal(fn, signal, () =>
+      this.controller.signal.aborted
+        ? new DutyBudgetExpiredError(what, this.deadline)
+        : new TimeoutError(`Timeout running ${what} after ${boundMs}ms`),
+    );
+  }
 }

yarn-project/validator-client/src/fake_inbox_test_helper.ts

@@ -13,13 +13,20 @@ export type FakeInbox = InboxEndpointReader & {
   setBuckets(buckets: LiveBucket[]): void;
   /** Makes every read fail, the way an unreachable provider does. */
   setUnreadable(err: Error | undefined): void;
+  /** Makes every bucket read hang, the way a provider that accepts a call and never answers does. */
+  setUnresponsive(): void;
+  /** Replaces the block reported at the head's height, the way a provider answering from a stale fork does. */
+  setViewHash(hash: string | null): void;
   /** Runs before each bucket read with its index, so a test can move the L1 view between two attempts. */
   onRead(hook: (readIndex: number) => void): void;
 };
 
 /** The L1 head fake reads are pinned to, unless a test asks for another one. */
 const DEFAULT_HEAD = 900n;
 
+/** The hash of the block at the head's height, unless a test replaces it mid-check. */
+const DEFAULT_HEAD_HASH = '0xhead';
+
 /**
  * An Inbox holding the given live buckets, resolving an upper bound the way the contract does: the newest live
  * bucket ending at or below it, or nothing at all once the ring has evicted every bucket that could have matched.
@@ -32,7 +39,10 @@ export function makeFakeInbox(
   let live = buckets;
   let failHead = opts.failHead;
   let failBucket = opts.failBucket;
+  let unresponsive = false;
+  let viewHash: string | null = DEFAULT_HEAD_HASH;
   let beforeRead: (readIndex: number) => void = () => {};
+  const head = opts.head ?? DEFAULT_HEAD;
   const reads: FakeInbox['reads'] = [];
   return {
     reads,
@@ -43,18 +53,28 @@ export function makeFakeInbox(
       failHead = err;
       failBucket = err;
     },
+    setUnresponsive: () => {
+      unresponsive = true;
+    },
+    setViewHash: hash => {
+      viewHash = hash;
+    },
     onRead: hook => {
       beforeRead = hook;
     },
     client: {
-      getBlockNumber: () => (failHead ? Promise.reject(failHead) : Promise.resolve(opts.head ?? DEFAULT_HEAD)),
+      getBlock: ({ blockNumber }) =>
+        failHead ? Promise.reject(failHead) : Promise.resolve({ number: blockNumber ?? head, hash: viewHash }),
     },
     getBucketAtOrBeforeTotal: (upperBound, readOpts) => {
       beforeRead(reads.length);
       reads.push({ upperBound, blockNumber: readOpts?.blockNumber });
       if (failBucket) {
         return Promise.reject(failBucket);
       }
+      if (unresponsive) {
+        return new Promise(() => {});
+      }
       const match = [...live].sort((a, b) => Number(a.total - b.total)).findLast(bucket => bucket.total <= upperBound);
       return Promise.resolve(
         match && {