fix: run the Inbox endpoint gate once, after a cached content verdict

An endpoint refusal used to be withheld from the validation cache, so an
RPC hiccup during the all-nodes callback threw away a content verdict that
cost a full checkpoint rebuild and block re-execution, and the attestation
callback moments later rebuilt the whole checkpoint again inside the same
duty budget. The cached-valid branch also made a second, separate endpoint
call.

The gate is now one step in handleCheckpointProposal: the content verdict
is computed once (cached or fresh) and cached unconditionally, and only
then is the endpoint confirmed, against the last block read for the same
proposal. Refusals stay non-slashable, set no invalid-slot marker and no
peer penalty, record an unvalidated outcome, and are never remembered as
the proposal's verdict, so a recovered L1 view still yields a valid verdict
on the next call — now without rebuilding. Blob upload moves next to the
content verdict so it still fires once per proposal.

Also: the README no longer claims the check "closes" a live bucket, and
names settlement as the L1-only check it does not replace; the internal
endpoint-check module is no longer re-exported; and the three fake Inboxes
in the tests collapse into one helper.

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

Author: spalladino

yarn-project/validator-client/README.md

@@ -113,7 +113,7 @@ When a `CheckpointProposal` is received, before creating attestations:
 6. Verify checkpoint header fields match last block's global variables:
    - slotNumber, coinbase, feeRecipient, gasFees
 7. Verify lastArchiveRoot matches first block's lastArchive
-8. Confirm against L1 that the last block's consumed message total closes a live Inbox bucket committing to the
+8. Confirm against L1 that the last block's consumed message total ends a live Inbox bucket committing to the
    checkpoint's signed `inboxRollingHash`
 ```
 
@@ -125,14 +125,19 @@ the last step reads the Inbox contract before the proposal may be recorded as va
 optimistic checkpoint parent, or attested to. It runs on every node, validator or not, because the all-nodes
 validation callback is what makes a proposal the accepted parent for the next slot.
 
+The gate is narrower than what L1 enforces. It confirms only that a live bucket *ends* at that total committing to
+the signed rolling hash; it says nothing about whether that bucket has settled. A checkpoint ending at the total of
+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 closing a live bucket (interior position, evicted endpoint, different rolling hash) as
+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

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

@@ -2,45 +2,8 @@ import { Fr } from '@aztec/foundation/curves/bn254';
 
 import { describe, expect, it } from '@jest/globals';
 
-import { type InboxEndpointReader, checkInboxEndpoint } from './checkpoint_endpoint_check.js';
-
-/** A live bucket of the fake Inbox ring: the cumulative total it ends at and the prefix hash it commits to. */
-type LiveBucket = { seq: bigint; total: bigint; rollingHash: Fr };
-
-/** Records what each endpoint read asked for, so a verdict can be checked against the L1 view it was made in. */
-type EndpointRead = { upperBound: bigint; blockNumber: bigint | undefined };
-
-/**
- * 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.
- */
-function makeInbox(
-  buckets: LiveBucket[],
-  opts: { head?: bigint; failHead?: Error; failBucket?: Error } = {},
-): InboxEndpointReader & { reads: EndpointRead[] } {
-  const head = opts.head ?? 900n;
-  const ordered = [...buckets].sort((a, b) => Number(a.total - b.total));
-  const reads: EndpointRead[] = [];
-  return {
-    reads,
-    client: {
-      getBlockNumber: () => (opts.failHead ? Promise.reject(opts.failHead) : Promise.resolve(head)),
-    },
-    getBucketAtOrBeforeTotal: (upperBound, readOpts) => {
-      reads.push({ upperBound, blockNumber: readOpts?.blockNumber });
-      if (opts.failBucket) {
-        return Promise.reject(opts.failBucket);
-      }
-      const match = ordered.filter(bucket => bucket.total <= upperBound).pop();
-      return Promise.resolve(
-        match && {
-          seq: match.seq,
-          bucket: { rollingHash: match.rollingHash, totalMsgCount: match.total, timestamp: 1n, msgCount: 1 },
-        },
-      );
-    },
-  };
-}
+import { checkInboxEndpoint } from './checkpoint_endpoint_check.js';
+import { type LiveBucket, makeFakeInbox } from './fake_inbox_test_helper.js';
 
 describe('checkInboxEndpoint', () => {
   const hashAt200 = Fr.random();
@@ -51,7 +14,7 @@ describe('checkInboxEndpoint', () => {
   ];
 
   it('verifies a position where a live bucket ends with the signed rolling hash', async () => {
-    const inbox = makeInbox(ring);
+    const inbox = makeFakeInbox(ring);
 
     await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
       verified: true,
@@ -61,7 +24,7 @@ describe('checkInboxEndpoint', () => {
   });
 
   it('resolves the bucket at the captured head rather than at a moving latest view', async () => {
-    const inbox = makeInbox(ring, { head: 1234n });
+    const inbox = makeFakeInbox(ring, { head: 1234n });
 
     const result = await checkInboxEndpoint(inbox, 400n, hashAt400);
 
@@ -71,7 +34,7 @@ describe('checkInboxEndpoint', () => {
 
   // The resolver answers with the closest boundary below the bound, so a lower result is a miss, not a match.
   it('rejects a position inside a bucket, even though a lower boundary resolves', async () => {
-    const inbox = makeInbox(ring);
+    const inbox = makeFakeInbox(ring);
 
     await expect(checkInboxEndpoint(inbox, 256n, hashAt200)).resolves.toEqual({
       verified: false,
@@ -82,7 +45,7 @@ describe('checkInboxEndpoint', () => {
   });
 
   it('rejects a boundary that commits to a different message prefix than the one signed', async () => {
-    const inbox = makeInbox(ring);
+    const inbox = makeFakeInbox(ring);
 
     await expect(checkInboxEndpoint(inbox, 200n, Fr.random())).resolves.toEqual({
       verified: false,
@@ -93,7 +56,7 @@ describe('checkInboxEndpoint', () => {
   });
 
   it('rejects a position no live bucket reaches any more', async () => {
-    const inbox = makeInbox([{ seq: 20n, total: 5000n, rollingHash: Fr.random() }]);
+    const inbox = makeFakeInbox([{ seq: 20n, total: 5000n, rollingHash: Fr.random() }]);
 
     await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
       verified: false,
@@ -105,7 +68,7 @@ describe('checkInboxEndpoint', () => {
   // An empty Inbox still has a genesis bucket ending at zero, so a checkpoint consuming nothing at the start of
   // the chain is verified by the same rule as any other, without special-casing a missing endpoint into success.
   it('verifies the genesis position of an Inbox that never received a message', async () => {
-    const inbox = makeInbox([{ seq: 0n, total: 0n, rollingHash: Fr.ZERO }]);
+    const inbox = makeFakeInbox([{ seq: 0n, total: 0n, rollingHash: Fr.ZERO }]);
 
     await expect(checkInboxEndpoint(inbox, 0n, Fr.ZERO)).resolves.toEqual({
       verified: true,
@@ -116,7 +79,7 @@ describe('checkInboxEndpoint', () => {
 
   it('reports an unreadable view when the head cannot be read', async () => {
     const err = new Error('l1 rpc request failed');
-    const inbox = makeInbox(ring, { failHead: err });
+    const inbox = makeFakeInbox(ring, { failHead: err });
 
     await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
       verified: false,
@@ -127,7 +90,7 @@ describe('checkInboxEndpoint', () => {
 
   it('reports an unreadable view when the bucket read fails at the captured head', async () => {
     const err = new Error('header not found');
-    const inbox = makeInbox(ring, { failBucket: err });
+    const inbox = makeFakeInbox(ring, { failBucket: err });
 
     await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({
       verified: false,

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

@@ -0,0 +1,67 @@
+import { Fr } from '@aztec/foundation/curves/bn254';
+
+import type { InboxEndpointReader } from './checkpoint_endpoint_check.js';
+
+/** A live bucket of the fake Inbox ring: the cumulative total it ends at and the prefix hash it commits to. */
+export type LiveBucket = { seq: bigint; total: bigint; rollingHash: Fr };
+
+/** A fake live Inbox ring for the endpoint check, whose contents and readability tests can move between reads. */
+export type FakeInbox = InboxEndpointReader & {
+  /** What each endpoint read asked for, and the L1 view it was made in. */
+  reads: { upperBound: bigint; blockNumber: bigint | undefined }[];
+  /** Replaces the live ring, the way an eviction or a reorg moves it between two reads. */
+  setBuckets(buckets: LiveBucket[]): void;
+  /** Makes every read fail, the way an unreachable provider does. */
+  setUnreadable(err: Error | undefined): 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;
+
+/**
+ * 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.
+ * Defaults to the genesis bucket of an Inbox that never received a message.
+ */
+export function makeFakeInbox(
+  buckets: LiveBucket[] = [{ seq: 0n, total: 0n, rollingHash: Fr.ZERO }],
+  opts: { head?: bigint; failHead?: Error; failBucket?: Error } = {},
+): FakeInbox {
+  let live = buckets;
+  let failHead = opts.failHead;
+  let failBucket = opts.failBucket;
+  let beforeRead: (readIndex: number) => void = () => {};
+  const reads: FakeInbox['reads'] = [];
+  return {
+    reads,
+    setBuckets: next => {
+      live = next;
+    },
+    setUnreadable: err => {
+      failHead = err;
+      failBucket = err;
+    },
+    onRead: hook => {
+      beforeRead = hook;
+    },
+    client: {
+      getBlockNumber: () => (failHead ? Promise.reject(failHead) : Promise.resolve(opts.head ?? DEFAULT_HEAD)),
+    },
+    getBucketAtOrBeforeTotal: (upperBound, readOpts) => {
+      beforeRead(reads.length);
+      reads.push({ upperBound, blockNumber: readOpts?.blockNumber });
+      if (failBucket) {
+        return Promise.reject(failBucket);
+      }
+      const match = [...live].sort((a, b) => Number(a.total - b.total)).findLast(bucket => bucket.total <= upperBound);
+      return Promise.resolve(
+        match && {
+          seq: match.seq,
+          bucket: { rollingHash: match.rollingHash, totalMsgCount: match.total, timestamp: 1n, msgCount: 1 },
+        },
+      );
+    },
+  };
+}

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

@@ -1,6 +1,5 @@
 export * from './proposal_handler.js';
 export * from './checkpoint_builder.js';
-export * from './checkpoint_endpoint_check.js';
 export * from './config.js';
 export * from './factory.js';
 export * from './validator.js';

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

@@ -40,7 +40,7 @@ import { describe, expect, it, jest } from '@jest/globals';
 import { type MockProxy, mock } from 'jest-mock-extended';
 
 import type { CheckpointBuilder, FullNodeCheckpointsBuilder } from './checkpoint_builder.js';
-import type { InboxEndpointReader } from './checkpoint_endpoint_check.js';
+import { type FakeInbox, makeFakeInbox } from './fake_inbox_test_helper.js';
 import type { ValidatorMetrics } from './metrics.js';
 import {
   type CheckpointProposalValidationResult,
@@ -73,56 +73,6 @@ function mockEmptyInboxView(source: MockProxy<L1ToL2MessageSource>) {
   );
 }
 
-/** A live Inbox bucket: the cumulative message total it ends at, and the prefix hash it commits to. */
-type LiveBucket = { seq: bigint; total: bigint; rollingHash: Fr };
-
-/** A fake live Inbox ring for the checkpoint endpoint gate, whose contents and readability tests can move. */
-type FakeInbox = InboxEndpointReader & {
-  /** What each endpoint read asked for, and the L1 view it was made in. */
-  reads: { upperBound: bigint; blockNumber: bigint | undefined }[];
-  /** Replaces the live ring, the way an eviction or a reorg moves it between two reads. */
-  setBuckets(buckets: LiveBucket[]): void;
-  /** Makes every read fail, the way an unreachable provider does. */
-  setUnreadable(err: Error | undefined): void;
-  /** Runs before each read with its index, so a test can move the L1 view between two attempts. */
-  onRead(hook: (readIndex: number) => void): void;
-};
-
-/** An Inbox resolving an upper bound to the newest live bucket ending at or below it, as the contract does. */
-function makeFakeInbox(buckets: LiveBucket[] = [{ seq: 0n, total: 0n, rollingHash: Fr.ZERO }]): FakeInbox {
-  let live = buckets;
-  let unreadable: Error | undefined;
-  let beforeRead: (readIndex: number) => void = () => {};
-  const reads: FakeInbox['reads'] = [];
-  return {
-    reads,
-    setBuckets: next => {
-      live = next;
-    },
-    setUnreadable: err => {
-      unreadable = err;
-    },
-    onRead: hook => {
-      beforeRead = hook;
-    },
-    client: { getBlockNumber: () => (unreadable ? Promise.reject(unreadable) : Promise.resolve(777n)) },
-    getBucketAtOrBeforeTotal: (upperBound, opts) => {
-      beforeRead(reads.length);
-      reads.push({ upperBound, blockNumber: opts?.blockNumber });
-      if (unreadable) {
-        return Promise.reject(unreadable);
-      }
-      const match = [...live].sort((a, b) => Number(a.total - b.total)).findLast(b => b.total <= upperBound);
-      return Promise.resolve(
-        match && {
-          seq: match.seq,
-          bucket: { rollingHash: match.rollingHash, totalMsgCount: match.total, timestamp: 1n, msgCount: 1 },
-        },
-      );
-    },
-  };
-}
-
 /**
  * The blocks of slot 1 for checkpoint 1, one per archive root, numbered from 1 and each chaining onto the previous
  * one's archive, consuming no Inbox messages.
@@ -1233,7 +1183,7 @@ describe('ProposalHandler checkpoint validation', () => {
         expect(result).toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) });
         // The last block's own consumed total, resolved once against an explicitly captured L1 head. The
         // intermediate block's position is never asked about.
-        expect(inbox.reads).toEqual([{ upperBound: 7n, blockNumber: 777n }]);
+        expect(inbox.reads).toEqual([{ upperBound: 7n, blockNumber: 900n }]);
       });
 
       // A checkpoint that consumed nothing still ends somewhere: the position it inherited, which has to be a live
@@ -1245,7 +1195,7 @@ describe('ProposalHandler checkpoint validation', () => {
         const result = await validate(header);
 
         expect(result).toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) });
-        expect(inbox.reads).toEqual([{ upperBound: 3n, blockNumber: 777n }]);
+        expect(inbox.reads).toEqual([{ upperBound: 3n, blockNumber: 900n }]);
       });
 
       // Blocks may consume an arbitrary prefix, so a checkpoint can be entirely content-valid and still finish
@@ -1373,7 +1323,8 @@ describe('ProposalHandler checkpoint validation', () => {
       });
 
       // A refusal describes the L1 view at that instant, so it is not remembered as this proposal's verdict: the
-      // next call re-reads and can still accept it.
+      // next call re-reads and can still accept it. The content verdict the refused call paid a full rebuild for
+      // is kept, so the attestation call moments later does not rebuild the checkpoint all over again.
       it('accepts on a later call once the endpoint reappears in a recovered view', async () => {
         const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 });
         inbox.setUnreadable(new Error('l1 rpc request failed'));
@@ -1391,6 +1342,8 @@ describe('ProposalHandler checkpoint validation', () => {
           isValid: true,
           checkpointNumber: CheckpointNumber(1),
         });
+        expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalledTimes(1);
+        expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid');
       });
     });
   });