From 076df84ab98d6e7f0a01962da042cab2c5a01dd8 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:43:24 -0300 Subject: [PATCH 1/5] feat: verify a checkpoint's final Inbox endpoint against L1 before accepting a proposal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before a node returns or records a checkpoint proposal as valid, it now confirms through an L1 read that the position the checkpoint finishes at closes a live Inbox bucket committing to the rolling hash the proposal signed. This strengthens a previously content-only acceptance policy; it is not a fix for a confirmed defect. The check runs in all-nodes validation, not just committee attestation, so a proposal without endpoint evidence cannot become this node's accepted optimistic checkpoint parent. The count comes from the authenticated last block of the coherent slot snapshot validation already reads, paired with the signed rolling hash — no unsigned proposer hint. The resolver answers with the closest boundary at or below the bound, so only an exact total counts: with buckets ending at 200 and 400, a checkpoint ending at 200 with the matching hash passes, and one ending at 256, or at 200 with a different hash, does not. Intermediate blocks may still end anywhere. The gate never fails open, and never treats a local uncertainty as misconduct. An unreadable view is separated in diagnostics from a view that answers without a live boundary there; both refuse to validate now, reach neither slashing nor the invalid-slot marker nor a peer penalty, and are not remembered as the proposal's verdict, so a changing head cannot poison it. Reads are retried briefly inside the slot's existing duty budget, so a provider that catches up in time still yields a valid verdict and a stalled read cannot hold the acceptance path open. Cached-valid reuse re-confirms the endpoint against a fresh view. The skip flag records nothing as valid and is documented as testing-only. The proposer's own fast path is taken only by the node that built the checkpoint, which resolved that bucket end against the live Inbox while building its final block. Historical ingestion and proof processing stay outside the gate, since old checkpoints may reference evicted endpoints. Cost is one head read plus one eth_call per checkpoint proposal validated. --- yarn-project/aztec-node/src/factory.ts | 7 +- yarn-project/p2p/src/config.ts | 2 +- .../p2p/src/services/libp2p/libp2p_service.ts | 4 + yarn-project/validator-client/README.md | 27 ++ .../src/checkpoint_endpoint_check.test.ts | 138 +++++++ .../src/checkpoint_endpoint_check.ts | 64 ++++ yarn-project/validator-client/src/config.ts | 3 +- yarn-project/validator-client/src/factory.ts | 5 + yarn-project/validator-client/src/index.ts | 1 + .../src/proposal_handler.test.ts | 348 ++++++++++++++++++ .../validator-client/src/proposal_handler.ts | 195 +++++++++- .../src/validator.ha.integration.test.ts | 9 + .../src/validator.integration.test.ts | 30 +- .../validator-client/src/validator.test.ts | 14 + .../validator-client/src/validator.ts | 7 +- 15 files changed, 841 insertions(+), 13 deletions(-) create mode 100644 yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts create mode 100644 yarn-project/validator-client/src/checkpoint_endpoint_check.ts diff --git a/yarn-project/aztec-node/src/factory.ts b/yarn-project/aztec-node/src/factory.ts index 682f5f3d8930..61e62a65b7a0 100644 --- a/yarn-project/aztec-node/src/factory.ts +++ b/yarn-project/aztec-node/src/factory.ts @@ -6,7 +6,7 @@ import { Blob, getKzg } from '@aztec/blob-lib'; import { EpochCache } from '@aztec/epoch-cache'; import { createEthereumChain } from '@aztec/ethereum/chain'; import { getPublicClient, makeL1HttpTransport } from '@aztec/ethereum/client'; -import { RegistryContract, RollupContract } from '@aztec/ethereum/contracts'; +import { InboxContract, RegistryContract, RollupContract } from '@aztec/ethereum/contracts'; import { pickL1ContractAddresses } from '@aztec/ethereum/l1-contract-addresses'; import type { L1TxUtils } from '@aztec/ethereum/l1-tx-utils'; import { compactArray } from '@aztec/foundation/collection'; @@ -168,6 +168,9 @@ export async function createAztecNodeService( Object.assign(config, l1ContractsAddresses); const rollupContract = new RollupContract(publicClient, config.rollupAddress.toString()); + // Read-only Inbox handle: proposal validation confirms a checkpoint's final message position against the live + // bucket ring before accepting it, so every node needs one, validator or not. + const inboxContract = new InboxContract(publicClient, config.inboxAddress); const [l1GenesisTime, slotDuration, epochDuration, rollupVersionFromRollup, rollupManaLimit] = await Promise.all([ rollupContract.getL1GenesisTime(), rollupContract.getSlotDuration(), @@ -339,6 +342,7 @@ export async function createAztecNodeService( epochCache, blockSource: archiver, l1ToL2MessageSource: archiver, + inbox: inboxContract, keyStoreManager, blobClient, reexecutionTracker, @@ -375,6 +379,7 @@ export async function createAztecNodeService( epochCache, blockSource: archiver, l1ToL2MessageSource: archiver, + inbox: inboxContract, p2pClient, blobClient, dateProvider, diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index e68d853df464..b93137980b19 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -602,7 +602,7 @@ export const p2pConfigMappings: ConfigMappingsType = { }, skipCheckpointProposalValidation: { description: - 'Skip checkpoint proposal validation and always attest, broadcasting the attestation before processing the embedded last block', + 'Skip checkpoint proposal validation, including the live Inbox endpoint check, and always attest, broadcasting the attestation before processing the embedded last block. For testing only', ...booleanConfigHelper(false), }, minTxPoolAgeMs: { diff --git a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts index 3c178fc433fe..9ff5823a4348 100644 --- a/yarn-project/p2p/src/services/libp2p/libp2p_service.ts +++ b/yarn-project/p2p/src/services/libp2p/libp2p_service.ts @@ -1660,6 +1660,10 @@ export class LibP2PService extends WithTracer implements P2PService { source: sender.toString(), }); + // The all-nodes callback runs first, and to completion: it is where the proposal is validated — including + // the check that its final message position closes a live Inbox bucket — and where a valid one becomes this + // node's proposed checkpoint. The validator callback below reuses that verdict, so attesting can never + // outrun it. await this.allNodesCheckpointReceivedCallback(checkpoint, sender); // Call the checkpoint received callback with the core version (without lastBlock) diff --git a/yarn-project/validator-client/README.md b/yarn-project/validator-client/README.md index fe9aa9e09a64..f0198d290106 100644 --- a/yarn-project/validator-client/README.md +++ b/yarn-project/validator-client/README.md @@ -79,6 +79,7 @@ These rules must always hold: 4. **Sequential indexWithinCheckpoint**: Block N must have `indexWithinCheckpoint = parent.indexWithinCheckpoint + 1` 5. **One proposer per slot**: Each slot has exactly one designated proposer. Sending multiple proposals for the same position (slot, indexWithinCheckpoint) with different content is equivocation and slashable 6. **One attestation per slot**: Validators should only attest to one checkpoint per slot. Attesting to different proposals (different archives) for the same slot is equivocation and slashable +7. **A checkpoint ends on a live Inbox bucket boundary**: its blocks may consume any prefix of the message log, but the position the checkpoint finishes at must close a bucket that is still live on L1 and commits to the rolling hash the checkpoint signed. Nodes confirm this against L1 before accepting a proposal (see below); L1 enforces it at publication ## Validation Flow @@ -112,8 +113,34 @@ 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 + checkpoint's signed `inboxRollingHash` ``` +#### Live Inbox endpoint check + +Steps 1-7 are deterministic and local: they say the checkpoint is the one its signed payload describes and that this +node holds the messages it consumed. They cannot say whether the position it finishes at is one L1 will accept, so +the last step reads the Inbox contract before the proposal may be recorded as valid, exposed as this node's +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. + +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 +`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. + ### Attestation Creation After successful checkpoint validation: diff --git a/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts b/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts new file mode 100644 index 000000000000..753437f0181b --- /dev/null +++ b/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts @@ -0,0 +1,138 @@ +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 }, + }, + ); + }, + }; +} + +describe('checkInboxEndpoint', () => { + const hashAt200 = Fr.random(); + const hashAt400 = Fr.random(); + const ring: LiveBucket[] = [ + { seq: 7n, total: 200n, rollingHash: hashAt200 }, + { seq: 8n, total: 400n, rollingHash: hashAt400 }, + ]; + + it('verifies a position where a live bucket ends with the signed rolling hash', async () => { + const inbox = makeInbox(ring); + + await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({ + verified: true, + l1BlockNumber: 900n, + bucketSeq: 7n, + }); + }); + + it('resolves the bucket at the captured head rather than at a moving latest view', async () => { + const inbox = makeInbox(ring, { head: 1234n }); + + const result = await checkInboxEndpoint(inbox, 400n, hashAt400); + + expect(inbox.reads).toEqual([{ upperBound: 400n, blockNumber: 1234n }]); + expect(result).toEqual({ verified: true, l1BlockNumber: 1234n, bucketSeq: 8n }); + }); + + // 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); + + await expect(checkInboxEndpoint(inbox, 256n, hashAt200)).resolves.toEqual({ + verified: false, + reason: 'interior_position', + l1BlockNumber: 900n, + endpointTotal: 200n, + }); + }); + + it('rejects a boundary that commits to a different message prefix than the one signed', async () => { + const inbox = makeInbox(ring); + + await expect(checkInboxEndpoint(inbox, 200n, Fr.random())).resolves.toEqual({ + verified: false, + reason: 'rolling_hash_mismatch', + l1BlockNumber: 900n, + endpointTotal: 200n, + }); + }); + + it('rejects a position no live bucket reaches any more', async () => { + const inbox = makeInbox([{ seq: 20n, total: 5000n, rollingHash: Fr.random() }]); + + await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({ + verified: false, + reason: 'no_live_endpoint', + l1BlockNumber: 900n, + }); + }); + + // 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 }]); + + await expect(checkInboxEndpoint(inbox, 0n, Fr.ZERO)).resolves.toEqual({ + verified: true, + l1BlockNumber: 900n, + bucketSeq: 0n, + }); + }); + + 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 }); + + await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({ + verified: false, + reason: 'unreadable', + err, + }); + }); + + 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 }); + + await expect(checkInboxEndpoint(inbox, 200n, hashAt200)).resolves.toEqual({ + verified: false, + reason: 'unreadable', + err, + }); + }); +}); diff --git a/yarn-project/validator-client/src/checkpoint_endpoint_check.ts b/yarn-project/validator-client/src/checkpoint_endpoint_check.ts new file mode 100644 index 000000000000..e147b31115a5 --- /dev/null +++ b/yarn-project/validator-client/src/checkpoint_endpoint_check.ts @@ -0,0 +1,64 @@ +import type { InboxContract } from '@aztec/ethereum/contracts'; +import type { ViemClient } from '@aztec/ethereum/types'; +import type { Fr } from '@aztec/foundation/curves/bn254'; + +/** + * 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. + */ +export type InboxEndpointReader = Pick & { + client: Pick; +}; + +/** Why a checkpoint's final message position is not a live Inbox bucket endpoint in the L1 view that was read. */ +export type InboxEndpointRejection = + /** No live bucket ends at or below the position: the ring has evicted every bucket that could have matched. */ + | 'no_live_endpoint' + /** The closest live boundary ends earlier, so the position falls inside a bucket rather than closing one. */ + | 'interior_position' + /** A live bucket ends exactly there, but commits to a different message prefix than the checkpoint signed. */ + | 'rolling_hash_mismatch'; + +/** + * 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. + */ +export type InboxEndpointCheckResult = + | { verified: true; l1BlockNumber: bigint; bucketSeq: bigint } + | { verified: false; reason: InboxEndpointRejection; l1BlockNumber: bigint; endpointTotal?: bigint } + | { verified: false; reason: 'unreadable'; err: unknown }; + +/** + * Confirms through L1 that `totalMsgCount` is the end of a live Inbox bucket committing to `inboxRollingHash`. + * + * A checkpoint may consume an arbitrary prefix of the message log across its blocks, but the position it finishes + * at has to be a live bucket boundary for L1 to accept it. The resolver answers with the newest boundary at or + * 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. + */ +export async function checkInboxEndpoint( + inbox: InboxEndpointReader, + totalMsgCount: bigint, + inboxRollingHash: Fr, +): Promise { + try { + const l1BlockNumber = await inbox.client.getBlockNumber(); + const found = await inbox.getBucketAtOrBeforeTotal(totalMsgCount, { blockNumber: l1BlockNumber }); + if (found === undefined) { + return { verified: false, reason: 'no_live_endpoint', l1BlockNumber }; + } + const endpointTotal = found.bucket.totalMsgCount; + if (endpointTotal !== totalMsgCount) { + return { verified: false, reason: 'interior_position', l1BlockNumber, endpointTotal }; + } + if (!found.bucket.rollingHash.equals(inboxRollingHash)) { + return { verified: false, reason: 'rolling_hash_mismatch', l1BlockNumber, endpointTotal }; + } + return { verified: true, l1BlockNumber, bucketSeq: found.seq }; + } catch (err) { + return { verified: false, reason: 'unreadable', err }; + } +} diff --git a/yarn-project/validator-client/src/config.ts b/yarn-project/validator-client/src/config.ts index 38d914a6d6af..395f12bd8463 100644 --- a/yarn-project/validator-client/src/config.ts +++ b/yarn-project/validator-client/src/config.ts @@ -79,7 +79,8 @@ export const validatorClientConfigMappings: ConfigMappingsType< ...booleanConfigHelper(false), }, skipCheckpointProposalValidation: { - description: 'Skip checkpoint proposal validation and always attest (default: false)', + description: + 'Skip checkpoint proposal validation, including the live Inbox endpoint check, and always attest. For testing only (default: false)', defaultValue: false, }, skipPushProposedBlocksToArchiver: { diff --git a/yarn-project/validator-client/src/factory.ts b/yarn-project/validator-client/src/factory.ts index b5d48f1c6cd8..ab99b9c57de3 100644 --- a/yarn-project/validator-client/src/factory.ts +++ b/yarn-project/validator-client/src/factory.ts @@ -12,6 +12,7 @@ import type { TelemetryClient } from '@aztec/telemetry-client'; import type { SlashingProtectionDatabase } from '@aztec/validator-ha-signer/types'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import { ValidatorMetrics } from './metrics.js'; import { ProposalHandler } from './proposal_handler.js'; import { ValidatorClient } from './validator.js'; @@ -23,6 +24,7 @@ export function createProposalHandler( worldState: WorldStateSynchronizer; blockSource: L2BlockSource & L2BlockSink; l1ToL2MessageSource: L1ToL2MessageSource; + inbox: InboxEndpointReader; p2pClient: P2PClient; epochCache: EpochCache; blobClient: BlobClientInterface; @@ -41,6 +43,7 @@ export function createProposalHandler( deps.worldState, deps.blockSource, deps.l1ToL2MessageSource, + deps.inbox, deps.p2pClient.getTxProvider(), deps.epochCache, consensusTimetable, @@ -62,6 +65,7 @@ export function createValidatorClient( p2pClient: P2PClient; blockSource: L2BlockSource & L2BlockSink; l1ToL2MessageSource: L1ToL2MessageSource; + inbox: InboxEndpointReader; telemetry: TelemetryClient; dateProvider: DateProvider; epochCache: EpochCache; @@ -84,6 +88,7 @@ export function createValidatorClient( deps.p2pClient, deps.blockSource, deps.l1ToL2MessageSource, + deps.inbox, txProvider, deps.keyStoreManager, deps.blobClient, diff --git a/yarn-project/validator-client/src/index.ts b/yarn-project/validator-client/src/index.ts index 1cef663abc9b..08947378d2ba 100644 --- a/yarn-project/validator-client/src/index.ts +++ b/yarn-project/validator-client/src/index.ts @@ -1,5 +1,6 @@ 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'; diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 731809128c83..7e39e6e7cb2e 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -41,6 +41,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 { ValidatorMetrics } from './metrics.js'; import { type CheckpointProposalValidationResult, @@ -73,6 +74,56 @@ function mockEmptyInboxView(source: MockProxy) { ); } +/** 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. @@ -109,6 +160,7 @@ describe('ProposalHandler checkpoint validation', () => { let handler: ProposalHandler; let blockSource: MockProxy; let l1ToL2MessageSource: MockProxy; + let inbox: FakeInbox; let epochCache: MockProxy; let checkpointsBuilder: MockProxy; let dateProvider: TestDateProvider; @@ -127,6 +179,9 @@ describe('ProposalHandler checkpoint validation', () => { l1ToL2MessageSource = mock(); mockEmptyInboxView(l1ToL2MessageSource); + // An L1 Inbox that never received a message: its genesis bucket ends at zero, which is where the + // consume-nothing checkpoints of these tests end too. + inbox = makeFakeInbox(); checkpointsBuilder = mock(); checkpointsBuilder.getConfig.mockReturnValue({ @@ -161,6 +216,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, mock(), epochCache, consensusTimetable, @@ -268,6 +324,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, mock(), epochCache, consensusTimetable, @@ -1198,6 +1255,281 @@ describe('ProposalHandler checkpoint validation', () => { checkpointNumber: CheckpointNumber(1), }); }); + + describe('live Inbox endpoint gate', () => { + /** + * Sets up a two-block checkpoint whose blocks, ancestry and consumed content all check out: it starts from + * message total 3, its first block consumes through `midLeafCount` and its last through `lastLeafCount`, so + * the only open question left is whether the position it ends at is a live Inbox bucket endpoint. Returns + * the header the proposal signs, committing to `inboxRollingHash`. + */ + function setupContentValidCheckpoint({ + midLeafCount, + lastLeafCount, + }: { + midLeafCount: number; + lastLeafCount: number; + }) { + const inboxRollingHash = Fr.random(); + const header = makeMatchingHeader({ inboxRollingHash }); + const blockHeader = makeBlockHeader(1, { + slotNumber: SlotNumber(1), + coinbase: header.coinbase, + feeRecipient: header.feeRecipient, + gasFees: header.gasFees, + timestamp: header.timestamp, + }); + unfreeze(blockHeader).lastArchive = new AppendOnlyTreeSnapshot(Fr.ZERO, 0); + // The rebuilt checkpoint is mocked wholesale, so its block only has to satisfy the final structural + // validation; the blocks the archiver serves below are what the counts and the endpoint derive from. + const computedBlock = { + archive: new AppendOnlyTreeSnapshot(archiveRoot, 1), + number: 5, + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: 0, + slot: SlotNumber(1), + header: blockHeader, + body: { txEffects: [] }, + computeDAGasUsed: () => 0, + toBlobFields: () => [], + } as unknown as L2Block; + setupDeepValidationMocks({ + header, + archive: new AppendOnlyTreeSnapshot(archiveRoot, 1), + blocks: [computedBlock], + number: CheckpointNumber(1), + slot: SlotNumber(1), + toBlobFields: () => [], + }); + + // The checkpoint is blocks 5 and 6 of slot 1; block 4, before it, consumed through message total 3. + const midArchive = Fr.random(); + const midBlock = { + archive: new AppendOnlyTreeSnapshot(midArchive, 1), + number: 5, + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: 0, + header: { + globalVariables: GlobalVariables.empty({ slotNumber: SlotNumber(1) }), + state: { l1ToL2MessageTree: { nextAvailableLeafIndex: midLeafCount } }, + lastArchive: new AppendOnlyTreeSnapshot(Fr.random(), 0), + getBlockNumber: () => 5, + }, + } as unknown as L2Block; + const lastBlock = { + archive: new AppendOnlyTreeSnapshot(archiveRoot, 2), + number: 6, + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: 1, + header: { + globalVariables: GlobalVariables.empty({ slotNumber: SlotNumber(1) }), + state: { l1ToL2MessageTree: { nextAvailableLeafIndex: lastLeafCount } }, + lastArchive: new AppendOnlyTreeSnapshot(midArchive, 1), + getBlockNumber: () => 6, + }, + } as unknown as L2Block; + blockSource.getBlocksForSlot.mockResolvedValue([midBlock, lastBlock]); + blockSource.getBlockData.mockImplementation(query => + Promise.resolve( + 'number' in query && query.number === 4 + ? ({ header: { state: { l1ToL2MessageTree: { nextAvailableLeafIndex: 3 } } } } as unknown as BlockData) + : (lastBlock as unknown as BlockData), + ), + ); + mockConsumedRange(3n, BigInt(lastLeafCount), [new Fr(1), new Fr(2), new Fr(3), new Fr(4)], inboxRollingHash); + return { header, inboxRollingHash }; + } + + /** Runs the checkpoint proposal signing `header` through validation. */ + async function validate(header: CheckpointHeader) { + return await handler.handleCheckpointProposal( + await makeProposal({ archiveRoot, checkpointHeader: header }), + proposalInfo, + ); + } + + /** Asserts a refusal that must not reach slashing, the invalid-slot marker or a valid outcome. */ + function expectNonPunitiveRefusal( + result: CheckpointProposalValidationResult, + reason: 'inbox_endpoint_not_live' | 'inbox_endpoint_unverifiable', + ) { + expect(result).toEqual({ isValid: false, reason, checkpointNumber: CheckpointNumber(1) }); + expect(SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[reason]).toBe(false); + expect(handler.hasInvalidProposals(SlotNumber(1))).toBe(false); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('unvalidated'); + } + + // The checkpoint's first block ends at 5, inside the bucket closing at 7: only where the checkpoint itself + // ends has to be a boundary, so the arbitrary prefix its blocks consumed stays allowed. + it('accepts a checkpoint whose final position closes a live bucket with the signed rolling hash', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + + const result = await validate(header); + + 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 }]); + }); + + // A checkpoint that consumed nothing still ends somewhere: the position it inherited, which has to be a live + // boundary like any other, and is one as long as the ring has not evicted it. + it('accepts a checkpoint that consumed no new messages while its inherited position is still live', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 3, lastLeafCount: 3 }); + inbox.setBuckets([{ seq: 2n, total: 3n, rollingHash: inboxRollingHash }]); + + const result = await validate(header); + + expect(result).toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) }); + expect(inbox.reads).toEqual([{ upperBound: 3n, blockNumber: 777n }]); + }); + + // Blocks may consume an arbitrary prefix, so a checkpoint can be entirely content-valid and still finish + // inside a bucket. The resolver answers with the boundary below it, which is not a match. + it('refuses a final position that falls inside a live bucket', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([ + { seq: 3n, total: 3n, rollingHash: Fr.random() }, + { seq: 4n, total: 9n, rollingHash: inboxRollingHash }, + ]); + + expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + }); + + it('refuses a live boundary that commits to a different message prefix than the signed one', async () => { + const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: Fr.random() }]); + + expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + }); + + // A missing endpoint is never special-cased into success: the ring may simply have evicted it. + it('refuses a position no live bucket reaches any more', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 40n, total: 5000n, rollingHash: inboxRollingHash }]); + + expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + }); + + it('refuses, without attributing anything to the proposer, when the L1 view cannot be read', async () => { + const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setUnreadable(new Error('l1 rpc request failed')); + + expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_unverifiable'); + }); + + // A provider still catching up reports the boundary below the checkpoint's end. The gate re-reads within the + // slot's duty budget, so the view recovering in time still yields a valid verdict. + it('accepts once a lagging provider catches up within the duty budget', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 3n, total: 3n, rollingHash: Fr.random() }]); + inbox.onRead(readIndex => { + if (readIndex > 0) { + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + } + }); + // Slot 1's attestation deadline is 40s; leave a couple of seconds of budget for the re-read. + dateProvider.setTime(38_000); + + const result = await validate(header); + + expect(result).toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) }); + expect(inbox.reads.length).toBeGreaterThan(1); + }); + + // The all-nodes callback runs before any attestation, so a checkpoint without endpoint evidence must not + // become the parent this node pipelines the next slot on. + it('does not become the accepted proposed checkpoint when the endpoint is not live', async () => { + const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 3n, total: 3n, rollingHash: Fr.random() }]); + const archiver = mock>(); + const p2p = mock(); + let checkpointHandler: ((proposal: any, sender: any) => Promise) | undefined; + p2p.registerAllNodesCheckpointProposalHandler.mockImplementation(h => { + checkpointHandler = h; + }); + handler.register(p2p, true, archiver); + + await checkpointHandler!(await makeProposal({ archiveRoot, checkpointHeader: header }), {} as any); + + expect(archiver.addProposedCheckpoint).not.toHaveBeenCalled(); + expect(handler.hasInvalidProposals(SlotNumber(1))).toBe(false); + }); + + it('sets the proposed checkpoint once the endpoint is confirmed live', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + const archiver = mock>(); + archiver.addProposedCheckpoint.mockResolvedValue(undefined); + const p2p = mock(); + let checkpointHandler: ((proposal: any, sender: any) => Promise) | undefined; + p2p.registerAllNodesCheckpointProposalHandler.mockImplementation(h => { + checkpointHandler = h; + }); + handler.register(p2p, true, archiver); + + await checkpointHandler!(await makeProposal({ archiveRoot, checkpointHeader: header }), {} as any); + + expect(archiver.addProposedCheckpoint).toHaveBeenCalled(); + }); + + // p2p calls the handler twice for one proposal (all-nodes validation, then attestation) and the second call + // reuses the cached verdict. The ring and the L1 head both move in between, so the endpoint is re-read. + it('re-checks the endpoint on L1 before reusing a cached valid verdict', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + const proposal = await makeProposal({ archiveRoot, checkpointHeader: header }); + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: true, + checkpointNumber: CheckpointNumber(1), + }); + + // The bucket the checkpoint ends at is gone by the time the attestation callback runs. + inbox.setBuckets([{ seq: 40n, total: 5000n, rollingHash: inboxRollingHash }]); + + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: false, + reason: 'inbox_endpoint_not_live', + checkpointNumber: CheckpointNumber(1), + }); + }); + + it('reuses a cached valid verdict, without rebuilding, while the endpoint stays live', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + const proposal = await makeProposal({ archiveRoot, checkpointHeader: header }); + await handler.handleCheckpointProposal(proposal, proposalInfo); + + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: true, + checkpointNumber: CheckpointNumber(1), + }); + expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalledTimes(1); + expect(inbox.reads).toHaveLength(2); + }); + + // 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. + 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')); + const proposal = await makeProposal({ archiveRoot, checkpointHeader: header }); + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: false, + reason: 'inbox_endpoint_unverifiable', + checkpointNumber: CheckpointNumber(1), + }); + + inbox.setUnreadable(undefined); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: true, + checkpointNumber: CheckpointNumber(1), + }); + }); + }); }); /** @@ -1229,6 +1561,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, @@ -1241,6 +1574,19 @@ describe('ProposalHandler checkpoint validation', () => { return { proposal, blockHandler, txProvider }; } + describe('block proposals and the live Inbox', () => { + // A checkpoint has to finish at a live bucket boundary, but its blocks may consume an arbitrary prefix and end + // anywhere in the message log, so the per-block path never asks L1 about the position a block ends at. + it('does not read the Inbox contract while validating a block proposal', async () => { + const { proposal, blockHandler } = await setupGenesisProposal(Fr.random(), [TxHash.random()]); + + const result = await blockHandler.handleBlockProposal(proposal, {} as any, false); + + expect(result).toEqual({ isValid: true, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM) }); + expect(inbox.reads).toEqual([]); + }); + }); + describe('handleBlockProposal duplicate txs', () => { it('rejects a proposal that lists the same tx hash twice, without attempting collection', async () => { const txHash = TxHash.random(); @@ -1394,6 +1740,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, @@ -1455,6 +1802,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index eadd3414ebc0..5818f1883ea7 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -62,6 +62,11 @@ import { import { type TelemetryClient, type Tracer, getTelemetryClient } from '@aztec/telemetry-client'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import { + type InboxEndpointCheckResult, + type InboxEndpointReader, + checkInboxEndpoint, +} from './checkpoint_endpoint_check.js'; import { DutyBudget, DutyBudgetExpiredError } from './duty_budget.js'; import type { ValidatorMetrics } from './metrics.js'; import { @@ -132,11 +137,21 @@ export type CheckpointProposalValidationFailureReason = // consumed. Local-view outcomes, never proposer misconduct. | 'inbox_prefix_unavailable' | 'inbox_prefix_mismatch' + // Streaming Inbox: the checkpoint's final consumed position was not confirmed as a live Inbox bucket endpoint. + // Both describe the L1 view this node read, never proposer misconduct. + | 'inbox_endpoint_unverifiable' + | 'inbox_endpoint_not_live' // The slot's duty budget ran out before validation reached a verdict. Says nothing about the proposal, so it is // neither cached for the next caller nor recorded against the proposer. | 'validation_deadline_expired' | 'checkpoint_validation_failed'; +/** The two outcomes of the live Inbox endpoint check, both of which describe an L1 view rather than a proposer. */ +type CheckpointEndpointReason = Extract< + CheckpointProposalValidationFailureReason, + 'inbox_endpoint_unverifiable' | 'inbox_endpoint_not_live' +>; + /** The streaming-Inbox reasons a checkpoint proposal can fail on; both are retried through a bounded local sync. */ type CheckpointInboxPrefixReason = Extract< CheckpointProposalValidationFailureReason, @@ -194,6 +209,10 @@ const CHECKPOINT_VALIDATION_REASON_TO_OUTCOME: Record< // Not proposer misbehavior: this node's Inbox view could not confirm the consumed prefix, or disagrees with it. inbox_prefix_unavailable: 'unvalidated', inbox_prefix_mismatch: 'unvalidated', + // Nor is an endpoint this node could not confirm: the bucket ring, the local provider and L1 itself all move + // independently of the moment the checkpoint was signed. + inbox_endpoint_unverifiable: 'unvalidated', + inbox_endpoint_not_live: 'unvalidated', // This node ran out of time to look; it observed nothing about the proposer. Recorded by the duty-expiry path // itself, which also refuses to overwrite an outcome the slot already has. validation_deadline_expired: 'unverifiable', @@ -234,6 +253,50 @@ type BlockProposalSlotValidationResult = const MAX_TRACKED_INVALID_PROPOSAL_SLOTS = 1000; +/** + * How long the live Inbox endpoint check keeps re-reading L1 before giving up, and how long it waits between + * attempts. A provider trailing the head by a block or two catches up within a read or two; waiting longer only + * multiplies RPC load for an answer the slot's duty budget may no longer have room for anyway. + */ +const INBOX_ENDPOINT_RETRY_WINDOW_MS = 2_000; +const INBOX_ENDPOINT_RETRY_INTERVAL_S = 0.5; + +/** + * Splits an endpoint check failure into a view this node could not read at all and one that answered but did not + * show the signed position closing a live bucket, so the two stay apart in diagnostics. Neither is attributed to + * the proposer, and an absent result (the duty was already over) counts as unread. + */ +function describeEndpointFailure(result: InboxEndpointCheckResult | undefined): { + reason: CheckpointEndpointReason; + context: LogData; +} { + if (result === undefined || result.verified) { + return { reason: 'inbox_endpoint_unverifiable', context: {} }; + } + if (result.reason === 'unreadable') { + return { + reason: 'inbox_endpoint_unverifiable', + context: { endpointReason: result.reason, err: String(result.err) }, + }; + } + return { + reason: 'inbox_endpoint_not_live', + context: { + endpointReason: result.reason, + endpointTotal: result.endpointTotal, + l1BlockNumber: result.l1BlockNumber, + }, + }; +} + +/** + * Whether a refusal only describes the L1 view read at that instant. Such a verdict is not remembered as the + * proposal's, so the next call re-reads and can still accept it once the view recovers. + */ +function isInboxEndpointReason(reason: CheckpointProposalValidationFailureReason): boolean { + return reason === 'inbox_endpoint_unverifiable' || reason === 'inbox_endpoint_not_live'; +} + /** Block-proposal validation failures that constitute a slashable invalid-block offense. */ export const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ 'state_mismatch', @@ -267,6 +330,11 @@ export const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record< // proposer offense, even when they persist through the attestation deadline. ['inbox_prefix_unavailable']: false, ['inbox_prefix_mismatch']: false, + // The final consumed position was not confirmed as a live Inbox bucket endpoint. An unreadable or trailing L1 + // view, and a ring that moved after the proposal was signed, look the same from here, and none of them is + // evidence that the proposer signed a position that was never an endpoint. + ['inbox_endpoint_unverifiable']: false, + ['inbox_endpoint_not_live']: false, ['invalid_signature']: false, ['last_block_not_found']: false, ['block_fetch_error']: false, @@ -317,6 +385,7 @@ export class ProposalHandler { private worldState: WorldStateSynchronizer, private blockSource: L2BlockSource & L2BlockSink, private l1ToL2MessageSource: L1ToL2MessageSource, + private inbox: InboxEndpointReader, private txProvider: ITxProvider, private epochCache: EpochCache, private timetable: ConsensusTimetable, @@ -352,6 +421,10 @@ export class ProposalHandler { * been deliberately corrupted in tests via `broadcastInvalidBlockProposal` / * `broadcastInvalidCheckpointProposalOnly`). Recording the local archive correctly models the * proposer's own view of its own work. + * + * This is the one path that records a `valid` outcome without running the live Inbox endpoint check: the + * checkpoint ends at the bucket end its own sequencer resolved against the Inbox when it built the last block, + * so the evidence exists, it was just gathered while building rather than while validating. */ public recordOwnCheckpointProposalAsValid(slot: SlotNumber, archive: Fr, checkpointNumber: CheckpointNumber): void { this.reexecutionTracker.recordOutcome(slot, archive, 'valid', checkpointNumber); @@ -499,6 +572,9 @@ export class ProposalHandler { proposer: proposal.getSender()?.toString(), }; + // Test-only escape hatch: nothing is validated, so nothing is recorded as valid either — no outcome on the + // re-execution tracker and no proposed checkpoint — but a validator configured this way still attests without + // any evidence, L1 endpoint included. It must not be set on a production node. if (this.config.skipCheckpointProposalValidation) { this.log.warn(`Skipping checkpoint proposal validation for slot ${proposal.slotNumber}`, proposalInfo); return undefined; @@ -521,6 +597,11 @@ export class ProposalHandler { // shares the proposer's keys sees the same "own" proposal over gossip but never built it, so it has // nothing stored; it falls through to the normal validate-and-persist path below to hydrate the // proposed-checkpoint metadata it needs to build the next slot on top of this checkpoint. + // + // The fast path is not a way around the live endpoint check: the checkpoint's final position is the live + // bucket end its own sequencer resolved against the Inbox while building the checkpoint's last block, and it + // re-reads L1 again in the publication preflight before submitting. That evidence is fresher than a + // re-validation here would be. Only the node that actually built the checkpoint takes this path. const proposer = proposal.getSender(); const ownAddresses = this.getOwnValidatorAddresses?.(); const isOwnProposal = proposer && ownAddresses?.some(addr => addr === proposer.toString()); @@ -1456,6 +1537,67 @@ export class ProposalHandler { return resolved; } + /** + * Confirms through L1 that the position a checkpoint finishes at closes a live Inbox bucket committing to the + * rolling hash it signed. The content checks authenticate what the checkpoint consumed against this node's own + * message log; they cannot tell whether the position it ends at is one L1 will accept, and only a checkpoint's + * final position has to be a bucket boundary. + * + * A view that disagrees, and one that cannot be read at all, are both refusals rather than verdicts about the + * proposer: a provider trailing the head has not seen the message that closed the bucket yet, an eviction or a + * reorg can move the ring after the proposal was signed, and none of that is visible from here. Both are re-read + * within {@link INBOX_ENDPOINT_RETRY_WINDOW_MS} and the slot's duty budget, whichever is shorter, so a view that + * recovers in time still yields a valid verdict, and a stalled read cannot hold the acceptance path open. + */ + private async awaitInboxEndpoint( + slot: SlotNumber, + finalTotalMsgCount: bigint, + inboxRollingHash: Fr, + proposalInfo: LogData, + budget: DutyBudget, + ): Promise<{ accepted: true } | { accepted: false; reason: CheckpointEndpointReason }> { + const timer = new Timer(); + const deadline = new Date( + Math.min(budget.deadline.getTime(), this.dateProvider.now() + INBOX_ENDPOINT_RETRY_WINDOW_MS), + ); + let last: InboxEndpointCheckResult | undefined; + try { + const verified = await budget.run(`inbox endpoint check for slot ${slot}`, () => + retryUntil( + async () => { + last = await checkInboxEndpoint(this.inbox, finalTotalMsgCount, inboxRollingHash); + return last.verified ? last : undefined; + }, + `live Inbox endpoint at message ${finalTotalMsgCount}`, + { deadline, dateProvider: this.dateProvider }, + INBOX_ENDPOINT_RETRY_INTERVAL_S, + ), + ); + this.log.debug(`Checkpoint's final message position confirmed as a live Inbox endpoint`, { + ...proposalInfo, + finalTotalMsgCount, + bucketSeq: verified.bucketSeq, + l1BlockNumber: verified.l1BlockNumber, + waitedMs: timer.ms(), + }); + return { accepted: true }; + } catch (err) { + if (!(err instanceof TimeoutError) && !(err instanceof DutyBudgetExpiredError)) { + throw err; + } + const { reason, context } = describeEndpointFailure(last); + this.log.warn(`Cannot confirm the checkpoint's final message position as a live Inbox endpoint`, { + ...proposalInfo, + reason, + ...context, + finalTotalMsgCount, + inboxRollingHash: inboxRollingHash.toString(), + waitedMs: timer.ms(), + }); + return { accepted: false, reason }; + } + } + async reexecuteTransactions( proposal: BlockProposal, blockNumber: BlockNumber, @@ -1644,15 +1786,31 @@ export class ProposalHandler { // That re-check is a store read like any other, so it runs inside the duty budget rather than unbounded. if (this.lastCheckpointValidationResult && this.lastCheckpointValidationResult.payloadHash === payloadHash) { const cached = this.lastCheckpointValidationResult.result; - const blocksStillLocal = - cached.isValid && - (await budget.run(`cached checkpoint verdict re-check for slot ${slot}`, () => - this.blockSource.getBlockData({ archive: proposal.archive }), - )) !== undefined; - if (!cached.isValid || blocksStillLocal) { + if (!cached.isValid) { this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); return cached; } + const lastBlock = await budget.run(`cached checkpoint verdict re-check for slot ${slot}`, () => + this.blockSource.getBlockData({ archive: proposal.archive }), + ); + if (lastBlock !== undefined) { + // The other half of a valid verdict is an Inbox endpoint that was live in the L1 view read at the time. + // The ring and the head both move between the two calls, so the endpoint is re-read rather than carried + // over: reuse is bound to a view, not just to a payload. + const endpoint = await this.awaitInboxEndpoint( + slot, + this.blockLeafCount(lastBlock), + proposal.checkpointHeader.inboxRollingHash, + proposalInfo, + budget, + ); + if (endpoint.accepted) { + this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); + return cached; + } + // Not the proposal's verdict, only this view's: leave the cached one in place so a later call can reuse it. + return { isValid: false, reason: endpoint.reason, checkpointNumber: cached.checkpointNumber }; + } this.log.warn(`Re-validating checkpoint proposal at slot ${slot}: its blocks are no longer local`, proposalInfo); } @@ -1670,7 +1828,11 @@ export class ProposalHandler { result = await this.validateCheckpointProposal(proposal, proposalInfo, budget); } - this.lastCheckpointValidationResult = { payloadHash, result }; + // An endpoint refusal describes the L1 view of the moment, not this proposal, so it is not remembered as its + // verdict: the attestation call re-reads and can still accept the same payload once the view recovers. + if (result.isValid || !isInboxEndpointReason(result.reason)) { + this.lastCheckpointValidationResult = { payloadHash, result }; + } // Record the outcome on the re-execution tracker. const outcome = result.isValid ? ('valid' as const) : CHECKPOINT_VALIDATION_REASON_TO_OUTCOME[result.reason]; @@ -1844,10 +2006,11 @@ export class ProposalHandler { // messages are already in the db from per-block validation; this list drives the checkpoint's rolling-hash // recomputation in completeCheckpoint. No bucket or L1 endpoint is resolved here: that is the proposer's and // L1's publication check. + const lastBlockTotal = this.blockLeafCount(lastBlock); const consumed = await this.awaitCheckpointConsumedMessages( slot, checkpointStartTotal, - this.blockLeafCount(blocks[blocks.length - 1]), + lastBlockTotal, proposal.checkpointHeader.inboxRollingHash, proposalInfo, budget, @@ -2007,6 +2170,22 @@ export class ProposalHandler { return { isValid: false, reason: 'checkpoint_validation_failed', checkpointNumber }; } + // Everything above is deterministic and local: it says the checkpoint is the one its signed payload describes + // and that this node holds the messages it consumed. What it cannot say is whether the position the checkpoint + // finishes at is one L1 accepts, which is the last thing left before this becomes an accepted parent or an + // attestation. It is checked last so the cheap deterministic work decides the outcome wherever it can, and so + // an offense keeps its own reason instead of being reported as an unconfirmed endpoint. + const endpoint = await this.awaitInboxEndpoint( + slot, + lastBlockTotal, + proposal.checkpointHeader.inboxRollingHash, + proposalInfo, + budget, + ); + if (!endpoint.accepted) { + return { isValid: false, reason: endpoint.reason, checkpointNumber }; + } + this.log.verbose(`Checkpoint proposal validation successful for slot ${slot}`, proposalInfo); return { isValid: true, checkpointNumber }; diff --git a/yarn-project/validator-client/src/validator.ha.integration.test.ts b/yarn-project/validator-client/src/validator.ha.integration.test.ts index cd31f8190049..79525a9046d4 100644 --- a/yarn-project/validator-client/src/validator.ha.integration.test.ts +++ b/yarn-project/validator-client/src/validator.ha.integration.test.ts @@ -41,6 +41,7 @@ import { type MockProxy, mock } from 'jest-mock-extended'; import { type PrivateKeyAccount, generatePrivateKey, privateKeyToAccount } from 'viem/accounts'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import type { ValidatorClientConfig } from './config.js'; import { HAKeyStore } from './key_store/ha_key_store.js'; import type { ExtendedValidatorKeyStore } from './key_store/interface.js'; @@ -59,6 +60,7 @@ describe('ValidatorClient HA Integration', () => { let p2pClient: MockProxy; let blockSource: MockProxy; let l1ToL2MessageSource: MockProxy; + let inbox: InboxEndpointReader; let epochCache: MockProxy; let checkpointsBuilder: MockProxy; let worldState: MockProxy; @@ -112,6 +114,12 @@ describe('ValidatorClient HA Integration', () => { epochCache.filterInCommittee.mockImplementation((_slot, addresses) => Promise.resolve(addresses)); blockSource = mock(); l1ToL2MessageSource = mock(); + // No messages were ever sent to L1 here, so the Inbox's genesis bucket is the only live endpoint. + inbox = { + client: { getBlockNumber: () => Promise.resolve(1n) }, + getBucketAtOrBeforeTotal: () => + Promise.resolve({ seq: 0n, bucket: { rollingHash: Fr.ZERO, totalMsgCount: 0n, timestamp: 0n, msgCount: 0 } }), + }; txProvider = mock(); dateProvider = new TestDateProvider(); blobClient = mock(); @@ -225,6 +233,7 @@ describe('ValidatorClient HA Integration', () => { worldState, blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, diff --git a/yarn-project/validator-client/src/validator.integration.test.ts b/yarn-project/validator-client/src/validator.integration.test.ts index 1ce3252e1306..41afdb2222db 100644 --- a/yarn-project/validator-client/src/validator.integration.test.ts +++ b/yarn-project/validator-client/src/validator.integration.test.ts @@ -26,7 +26,7 @@ import { CheckpointReexecutionTracker, L1PublishedData, PublishedCheckpoint } fr import { type L1RollupConstants, getTimestampForSlot } from '@aztec/stdlib/epoch-helpers'; import { Gas, GasFees } from '@aztec/stdlib/gas'; import { tryStop } from '@aztec/stdlib/interfaces/server'; -import { InboxMessagePrefixRef } from '@aztec/stdlib/messaging'; +import { InboxMessagePrefixRef, type L1ToL2MessageSource } from '@aztec/stdlib/messaging'; import { type BlockProposal, CheckpointProposal, @@ -46,8 +46,35 @@ import { hashTypedData } from 'viem'; import { generatePrivateKey } from 'viem/accounts'; import { CheckpointBuilder, FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import { ValidatorClient } from './validator.js'; +/** + * An Inbox whose live bucket ring ends wherever this node's own message log does: every position the archiver can + * serve is reported as a bucket boundary committing to the rolling hash the archiver holds there. Real buckets also + * close on time and size boundaries; what the checkpoint endpoint gate asks is whether the checkpoint's final + * position closes one, and with which content. + */ +function makeArchiverBackedInbox(messageSource: Pick): InboxEndpointReader { + return { + client: { getBlockNumber: () => Promise.resolve(1n) }, + getBucketAtOrBeforeTotal: async upperBound => { + const position = await messageSource.getMessagePosition(upperBound); + return ( + position && { + seq: 0n, + bucket: { + rollingHash: position.rollingHash, + totalMsgCount: position.totalMessageCount, + timestamp: 0n, + msgCount: 0, + }, + } + ); + }, + }; +} + jest.setTimeout(60_000); describe('ValidatorClient Integration', () => { @@ -221,6 +248,7 @@ describe('ValidatorClient Integration', () => { p2pClient, archiver, archiver, + makeArchiverBackedInbox(archiver), txProvider, keyStoreManager, blobClient, diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 406a4d31372d..038954054771 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -62,6 +62,7 @@ import type { CheckpointBuilder, FullNodeCheckpointsBuilder, } from './checkpoint_builder.js'; +import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import { type ValidatorClientConfig, validatorClientConfigMappings } from './config.js'; import type { ValidationService } from './duties/validation_service.js'; import { HAKeyStore } from './key_store/ha_key_store.js'; @@ -118,6 +119,7 @@ describe('ValidatorClient', () => { let p2pClient: MockProxy; let blockSource: MockProxy; let l1ToL2MessageSource: MockProxy; + let inbox: InboxEndpointReader; let epochCache: MockProxy; let checkpointsBuilder: MockProxy; let worldState: MockProxy; @@ -194,6 +196,17 @@ describe('ValidatorClient', () => { ); epochCache.isEscapeHatchOpenAtSlot.mockResolvedValue(false); l1ToL2MessageSource = mock(); + // An L1 Inbox that never received a message: its genesis bucket is the endpoint the consume-nothing + // checkpoints of these tests end at. + inbox = { + client: { getBlockNumber: () => Promise.resolve(1n) }, + getBucketAtOrBeforeTotal: upperBound => + Promise.resolve( + upperBound >= 0n + ? { seq: 0n, bucket: { rollingHash: Fr.ZERO, totalMsgCount: 0n, timestamp: 0n, msgCount: 0 } } + : undefined, + ), + }; txProvider = mock(); dateProvider = new TestDateProvider(); blobClient = mock(); @@ -241,6 +254,7 @@ describe('ValidatorClient', () => { p2pClient, blockSource, l1ToL2MessageSource, + inbox, txProvider, keyStoreManager, blobClient, diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 675313dca950..2642856aea66 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -60,6 +60,7 @@ import { EventEmitter } from 'events'; import type { TypedDataDefinition } from 'viem'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import { ValidationService } from './duties/validation_service.js'; import { DutyBudget, DutyBudgetExpiredError } from './duty_budget.js'; import { HAKeyStore } from './key_store/ha_key_store.js'; @@ -210,6 +211,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) p2pClient: P2P, blockSource: L2BlockSource & L2BlockSink, l1ToL2MessageSource: L1ToL2MessageSource, + inbox: InboxEndpointReader, txProvider: ITxProvider, keyStoreManager: KeystoreManager, blobClient: BlobClientInterface, @@ -228,6 +230,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) worldState, blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, @@ -594,7 +597,9 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) }); // Validate the checkpoint proposal before attesting (unless skipCheckpointProposalValidation is set). - // Uses the cached result from the all-nodes callback if available (avoids double validation). + // Uses the cached result from the all-nodes callback if available (avoids double validation). Reusing a valid + // verdict re-confirms its Inbox endpoint against L1 first, so nothing is signed on a position that has since + // stopped closing a live bucket. let checkpointNumber: CheckpointNumber; if (this.config.skipCheckpointProposalValidation) { this.log.warn(`Skipping checkpoint proposal validation for slot ${proposalSlotNumber}`, proposalInfo); From d567d9f529d16f8f51c09e66f72aa8ffc3fab3d9 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 16:41:52 -0300 Subject: [PATCH 2/5] fix: run the Inbox endpoint gate once, after a cached content verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- yarn-project/validator-client/README.md | 9 +- .../src/checkpoint_endpoint_check.test.ts | 57 ++----- .../src/fake_inbox_test_helper.ts | 67 ++++++++ yarn-project/validator-client/src/index.ts | 1 - .../src/proposal_handler.test.ts | 61 +------ .../validator-client/src/proposal_handler.ts | 159 +++++++++--------- .../validator-client/src/validator.test.ts | 19 +-- 7 files changed, 179 insertions(+), 194 deletions(-) create mode 100644 yarn-project/validator-client/src/fake_inbox_test_helper.ts diff --git a/yarn-project/validator-client/README.md b/yarn-project/validator-client/README.md index f0198d290106..9ab4e8219bed 100644 --- a/yarn-project/validator-client/README.md +++ b/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,6 +125,11 @@ 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 @@ -132,7 +137,7 @@ block-tag guard reads it again. Failures are re-read for up to two seconds, boun 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 diff --git a/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts b/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts index 753437f0181b..e5541358e0c4 100644 --- a/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts +++ b/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, diff --git a/yarn-project/validator-client/src/fake_inbox_test_helper.ts b/yarn-project/validator-client/src/fake_inbox_test_helper.ts new file mode 100644 index 000000000000..ff145c3326f5 --- /dev/null +++ b/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 }, + }, + ); + }, + }; +} diff --git a/yarn-project/validator-client/src/index.ts b/yarn-project/validator-client/src/index.ts index 08947378d2ba..1cef663abc9b 100644 --- a/yarn-project/validator-client/src/index.ts +++ b/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'; diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 7e39e6e7cb2e..b1b0794e331d 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -41,7 +41,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, @@ -74,56 +74,6 @@ function mockEmptyInboxView(source: MockProxy) { ); } -/** 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. @@ -1370,7 +1320,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 @@ -1382,7 +1332,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 @@ -1510,7 +1460,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')); @@ -1528,6 +1479,8 @@ describe('ProposalHandler checkpoint validation', () => { isValid: true, checkpointNumber: CheckpointNumber(1), }); + expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalledTimes(1); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); }); }); }); diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 5818f1883ea7..40b041172c6f 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -289,14 +289,6 @@ function describeEndpointFailure(result: InboxEndpointCheckResult | undefined): }; } -/** - * Whether a refusal only describes the L1 view read at that instant. Such a verdict is not remembered as the - * proposal's, so the next call re-reads and can still accept it once the view recovers. - */ -function isInboxEndpointReason(reason: CheckpointProposalValidationFailureReason): boolean { - return reason === 'inbox_endpoint_unverifiable' || reason === 'inbox_endpoint_not_live'; -} - /** Block-proposal validation failures that constitute a slashable invalid-block offense. */ export const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ 'state_mismatch', @@ -1538,10 +1530,10 @@ export class ProposalHandler { } /** - * Confirms through L1 that the position a checkpoint finishes at closes a live Inbox bucket committing to the + * Confirms through L1 that the position a checkpoint finishes at ends a live Inbox bucket committing to the * rolling hash it signed. The content checks authenticate what the checkpoint consumed against this node's own * message log; they cannot tell whether the position it ends at is one L1 will accept, and only a checkpoint's - * final position has to be a bucket boundary. + * final position has to be a bucket boundary. Whether that bucket has settled stays an L1-only check. * * A view that disagrees, and one that cannot be read at all, are both refusals rather than verdicts about the * proposer: a provider trailing the head has not seen the message that closed the bucket yet, an eviction or a @@ -1550,12 +1542,20 @@ export class ProposalHandler { * recovers in time still yields a valid verdict, and a stalled read cannot hold the acceptance path open. */ private async awaitInboxEndpoint( - slot: SlotNumber, - finalTotalMsgCount: bigint, - inboxRollingHash: Fr, + proposal: CheckpointProposalCore, + lastBlock: BlockData | undefined, proposalInfo: LogData, budget: DutyBudget, ): Promise<{ accepted: true } | { accepted: false; reason: CheckpointEndpointReason }> { + if (lastBlock === undefined) { + // The block carrying the signed archive went missing between the content verdict and this read, so the + // position to ask L1 about is unknown. That is a local uncertainty like an unreadable view, not misconduct. + this.log.warn(`Cannot read the checkpoint's last block to confirm its final message position`, proposalInfo); + return { accepted: false, reason: 'inbox_endpoint_unverifiable' }; + } + const slot = proposal.slotNumber; + const finalTotalMsgCount = this.blockLeafCount(lastBlock); + const inboxRollingHash = proposal.checkpointHeader.inboxRollingHash; const timer = new Timer(); const deadline = new Date( Math.min(budget.deadline.getTime(), this.dateProvider.now() + INBOX_ENDPOINT_RETRY_WINDOW_MS), @@ -1781,57 +1781,67 @@ export class ProposalHandler { // Check cache: same signed-payload hash means we already validated this exact proposal. A valid verdict rests // on blocks this node holds locally, and p2p makes two calls for one proposal (the all-nodes validation, then - // the attestation), so an archiver rollback in between can prune those blocks. Re-check that the checkpoint's - // last block is still local before reusing a valid verdict, or the attestation outlives what it was based on. - // That re-check is a store read like any other, so it runs inside the duty budget rather than unbounded. - if (this.lastCheckpointValidationResult && this.lastCheckpointValidationResult.payloadHash === payloadHash) { - const cached = this.lastCheckpointValidationResult.result; - if (!cached.isValid) { - this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); - return cached; - } - const lastBlock = await budget.run(`cached checkpoint verdict re-check for slot ${slot}`, () => - this.blockSource.getBlockData({ archive: proposal.archive }), - ); + // the attestation), so an archiver rollback in between can prune those blocks. Re-reading the checkpoint's + // last block confirms it is still there before a valid verdict is reused, or the attestation outlives what it + // was based on. The same read gives the endpoint gate below the position the checkpoint finishes at, so it is + // taken on either path. Store reads run inside the duty budget rather than unbounded. + const cached = + this.lastCheckpointValidationResult?.payloadHash === payloadHash + ? this.lastCheckpointValidationResult.result + : undefined; + if (cached && !cached.isValid) { + this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); + return cached; + } + + let result: CheckpointProposalValidationResult | undefined; + let lastBlock: BlockData | undefined; + if (cached) { + lastBlock = await this.readCheckpointLastBlock(proposal, budget); if (lastBlock !== undefined) { - // The other half of a valid verdict is an Inbox endpoint that was live in the L1 view read at the time. - // The ring and the head both move between the two calls, so the endpoint is re-read rather than carried - // over: reuse is bound to a view, not just to a payload. - const endpoint = await this.awaitInboxEndpoint( - slot, - this.blockLeafCount(lastBlock), - proposal.checkpointHeader.inboxRollingHash, + this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); + result = cached; + } else { + this.log.warn( + `Re-validating checkpoint proposal at slot ${slot}: its blocks are no longer local`, proposalInfo, - budget, ); - if (endpoint.accepted) { - this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); - return cached; - } - // Not the proposal's verdict, only this view's: leave the cached one in place so a later call can reuse it. - return { isValid: false, reason: endpoint.reason, checkpointNumber: cached.checkpointNumber }; } - this.log.warn(`Re-validating checkpoint proposal at slot ${slot}: its blocks are no longer local`, proposalInfo); } - const proposer = proposal.getSender(); - let result: CheckpointProposalValidationResult; - if (!proposer) { - this.log.warn(`Received checkpoint proposal with invalid signature for slot ${proposal.slotNumber}`); - result = { isValid: false as const, reason: 'invalid_signature' }; - } else if (!validateFeeAssetPriceModifier(proposal.feeAssetPriceModifier)) { - this.log.warn( - `Received checkpoint proposal with invalid feeAssetPriceModifier ${proposal.feeAssetPriceModifier} for slot ${proposal.slotNumber}`, - ); - result = { isValid: false, reason: 'invalid_fee_asset_price_modifier' }; - } else { - result = await this.validateCheckpointProposal(proposal, proposalInfo, budget); + if (result === undefined) { + const proposer = proposal.getSender(); + if (!proposer) { + this.log.warn(`Received checkpoint proposal with invalid signature for slot ${proposal.slotNumber}`); + result = { isValid: false as const, reason: 'invalid_signature' }; + } else if (!validateFeeAssetPriceModifier(proposal.feeAssetPriceModifier)) { + this.log.warn( + `Received checkpoint proposal with invalid feeAssetPriceModifier ${proposal.feeAssetPriceModifier} for slot ${proposal.slotNumber}`, + ); + result = { isValid: false, reason: 'invalid_fee_asset_price_modifier' }; + } else { + result = await this.validateCheckpointProposal(proposal, proposalInfo, budget); + } + this.lastCheckpointValidationResult = { payloadHash, result }; + if (result.isValid) { + lastBlock = await this.readCheckpointLastBlock(proposal, budget); + // Blobs follow the content verdict rather than the endpoint gate: the data is the same either way, and + // tying it here uploads a checkpoint's blobs once, on the call that built the verdict (fire and forget). + this.tryUploadBlobsForCheckpoint(proposal, proposalInfo); + } } - // An endpoint refusal describes the L1 view of the moment, not this proposal, so it is not remembered as its - // verdict: the attestation call re-reads and can still accept the same payload once the view recovers. - if (result.isValid || !isInboxEndpointReason(result.reason)) { - this.lastCheckpointValidationResult = { payloadHash, result }; + // A content verdict says the checkpoint is the one its signed payload describes and that this node holds the + // messages it consumed. What it cannot say is whether the position the checkpoint finishes at is one L1 + // accepts, which is the last thing left before this becomes an accepted parent or an attestation. It is gated + // here rather than inside the content validation so a refusal — which describes the L1 view of the moment and + // nothing about the proposer — does not discard a verdict that cost a full rebuild: the next call reuses the + // content verdict, re-reads L1 and can still accept the same payload once the view recovers. + if (result.isValid) { + const endpoint = await this.awaitInboxEndpoint(proposal, lastBlock, proposalInfo, budget); + if (!endpoint.accepted) { + result = { isValid: false, reason: endpoint.reason, checkpointNumber: result.checkpointNumber }; + } } // Record the outcome on the re-execution tracker. @@ -1857,14 +1867,22 @@ export class ProposalHandler { } } - // Upload blobs to filestore if validation passed (fire and forget) - if (result.isValid) { - this.tryUploadBlobsForCheckpoint(proposal, proposalInfo); - } - return result; } + /** + * Reads the checkpoint's last block: the one carrying the signed archive. Its presence is what a reused verdict + * rests on, and its L1-to-L2 leaf count is the cumulative message position the checkpoint finishes at. + */ + private readCheckpointLastBlock( + proposal: CheckpointProposalCore, + budget: DutyBudget, + ): Promise { + return budget.run(`checkpoint last block read for slot ${proposal.slotNumber}`, () => + this.blockSource.getBlockData({ archive: proposal.archive }), + ); + } + /** * Validates a checkpoint proposal by building the full checkpoint and comparing it with the proposal. * @returns Validation result with isValid flag and reason if invalid. @@ -2006,11 +2024,10 @@ export class ProposalHandler { // messages are already in the db from per-block validation; this list drives the checkpoint's rolling-hash // recomputation in completeCheckpoint. No bucket or L1 endpoint is resolved here: that is the proposer's and // L1's publication check. - const lastBlockTotal = this.blockLeafCount(lastBlock); const consumed = await this.awaitCheckpointConsumedMessages( slot, checkpointStartTotal, - lastBlockTotal, + this.blockLeafCount(lastBlock), proposal.checkpointHeader.inboxRollingHash, proposalInfo, budget, @@ -2170,22 +2187,6 @@ export class ProposalHandler { return { isValid: false, reason: 'checkpoint_validation_failed', checkpointNumber }; } - // Everything above is deterministic and local: it says the checkpoint is the one its signed payload describes - // and that this node holds the messages it consumed. What it cannot say is whether the position the checkpoint - // finishes at is one L1 accepts, which is the last thing left before this becomes an accepted parent or an - // attestation. It is checked last so the cheap deterministic work decides the outcome wherever it can, and so - // an offense keeps its own reason instead of being reported as an unconfirmed endpoint. - const endpoint = await this.awaitInboxEndpoint( - slot, - lastBlockTotal, - proposal.checkpointHeader.inboxRollingHash, - proposalInfo, - budget, - ); - if (!endpoint.accepted) { - return { isValid: false, reason: endpoint.reason, checkpointNumber }; - } - this.log.verbose(`Checkpoint proposal validation successful for slot ${slot}`, proposalInfo); return { isValid: true, checkpointNumber }; diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 038954054771..92cd5f82e168 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -62,9 +62,9 @@ import type { CheckpointBuilder, FullNodeCheckpointsBuilder, } from './checkpoint_builder.js'; -import type { InboxEndpointReader } from './checkpoint_endpoint_check.js'; import { type ValidatorClientConfig, validatorClientConfigMappings } from './config.js'; import type { ValidationService } from './duties/validation_service.js'; +import { type FakeInbox, makeFakeInbox } from './fake_inbox_test_helper.js'; import { HAKeyStore } from './key_store/ha_key_store.js'; import { type CheckpointProposalValidationFailureReason, ProposalHandler } from './proposal_handler.js'; import { ValidatorClient } from './validator.js'; @@ -119,7 +119,7 @@ describe('ValidatorClient', () => { let p2pClient: MockProxy; let blockSource: MockProxy; let l1ToL2MessageSource: MockProxy; - let inbox: InboxEndpointReader; + let inbox: FakeInbox; let epochCache: MockProxy; let checkpointsBuilder: MockProxy; let worldState: MockProxy; @@ -198,15 +198,7 @@ describe('ValidatorClient', () => { l1ToL2MessageSource = mock(); // An L1 Inbox that never received a message: its genesis bucket is the endpoint the consume-nothing // checkpoints of these tests end at. - inbox = { - client: { getBlockNumber: () => Promise.resolve(1n) }, - getBucketAtOrBeforeTotal: upperBound => - Promise.resolve( - upperBound >= 0n - ? { seq: 0n, bucket: { rollingHash: Fr.ZERO, totalMsgCount: 0n, timestamp: 0n, msgCount: 0 } } - : undefined, - ), - }; + inbox = makeFakeInbox(); txProvider = mock(); dateProvider = new TestDateProvider(); blobClient = mock(); @@ -894,6 +886,10 @@ describe('ValidatorClient', () => { }, }); + // The checkpoint consumes nothing, so it ends where the Inbox's genesis bucket does; make that bucket + // commit to the hash this proposal signed, so the live endpoint gate confirms it. + inbox.setBuckets([{ seq: 0n, total: 0n, rollingHash: checkpointProposal.checkpointHeader.inboxRollingHash }]); + // Mock validateCheckpointProposal to pass, so handleCheckpointProposal runs its // own checks (signature, fee modifier) and then proceeds to blob upload. const validateCheckpointSpy = jest @@ -934,6 +930,7 @@ describe('ValidatorClient', () => { }, }); + inbox.setBuckets([{ seq: 0n, total: 0n, rollingHash: checkpointProposal.checkpointHeader.inboxRollingHash }]); const validateCheckpointSpy = jest .spyOn(validatorClient.getProposalHandler(), 'validateCheckpointProposal') .mockResolvedValue({ isValid: true, checkpointNumber: CheckpointNumber(1) }); From 9d5f425043b7841798dd20d5a038130914a1429a Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 17:24:13 -0300 Subject: [PATCH 3/5] fix: bound and bind the Inbox endpoint gate, and stop it retracting a valid outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- yarn-project/validator-client/README.md | 42 ++++--- .../src/checkpoint_endpoint_check.test.ts | 36 ++++++ .../src/checkpoint_endpoint_check.ts | 57 +++++++-- .../validator-client/src/duty_budget.ts | 26 +++++ .../src/fake_inbox_test_helper.ts | 22 +++- .../src/proposal_handler.test.ts | 76 ++++++++++-- .../validator-client/src/proposal_handler.ts | 108 ++++++++++++------ .../src/validator.ha.integration.test.ts | 2 +- .../src/validator.integration.test.ts | 2 +- 9 files changed, 301 insertions(+), 70 deletions(-) diff --git a/yarn-project/validator-client/README.md b/yarn-project/validator-client/README.md index 9ab4e8219bed..8f20b5eb99b6 100644 --- a/yarn-project/validator-client/README.md +++ b/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 diff --git a/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts b/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts index e5541358e0c4..b7a4dd1330da 100644 --- a/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts +++ b/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', + }); + }); }); diff --git a/yarn-project/validator-client/src/checkpoint_endpoint_check.ts b/yarn-project/validator-client/src/checkpoint_endpoint_check.ts index e147b31115a5..b3b04269efd6 100644 --- a/yarn-project/validator-client/src/checkpoint_endpoint_check.ts +++ b/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 & { - client: Pick; + 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 { + 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 { + const block = await client.getBlock({ blockNumber, includeTransactions: false }); + return block?.number == null || block.hash == null ? undefined : { number: block.number, hash: block.hash }; +} diff --git a/yarn-project/validator-client/src/duty_budget.ts b/yarn-project/validator-client/src/duty_budget.ts index 4a1d150c2902..8ce82eb5f141 100644 --- a/yarn-project/validator-client/src/duty_budget.ts +++ b/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'; @@ -108,6 +109,31 @@ export class DutyBudget { 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, and a duty past its deadline draws on the same single grace allowance. + */ + public async runWithin(what: string, withinMs: number, fn: (signal: AbortSignal) => Promise): Promise { + if (this.controller.signal.aborted) { + throw new DutyBudgetExpiredError(what, this.deadline); + } + const remainingMs = this.remainingMs() || this.remainingGraceMs(); + if (remainingMs === 0) { + throw new DutyBudgetExpiredError(what, this.deadline); + } + const boundMs = Math.min(remainingMs, 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`), + ); + } + /** Milliseconds left of the single grace allowance, opening it on the first call. Zero once the duty is stopped. */ private remainingGraceMs(): number { if (this.controller.signal.aborted) { diff --git a/yarn-project/validator-client/src/fake_inbox_test_helper.ts b/yarn-project/validator-client/src/fake_inbox_test_helper.ts index ff145c3326f5..9fd3efc944b2 100644 --- a/yarn-project/validator-client/src/fake_inbox_test_helper.ts +++ b/yarn-project/validator-client/src/fake_inbox_test_helper.ts @@ -13,6 +13,10 @@ 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; }; @@ -20,6 +24,9 @@ export type FakeInbox = InboxEndpointReader & { /** 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,11 +53,18 @@ 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); @@ -55,6 +72,9 @@ export function makeFakeInbox( 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 && { diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index b1b0794e331d..15f40fec63d1 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -8,7 +8,7 @@ import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; import { Fr } from '@aztec/foundation/curves/bn254'; import { promiseWithResolvers } from '@aztec/foundation/promise'; import { sleep } from '@aztec/foundation/sleep'; -import { TestDateProvider } from '@aztec/foundation/timer'; +import { TestDateProvider, Timer } from '@aztec/foundation/timer'; import { type FieldsOf, unfreeze } from '@aztec/foundation/types'; import type { P2P } from '@aztec/p2p'; import { BlockHash } from '@aztec/stdlib/block'; @@ -1298,8 +1298,12 @@ describe('ProposalHandler checkpoint validation', () => { ); } - /** Asserts a refusal that must not reach slashing, the invalid-slot marker or a valid outcome. */ - function expectNonPunitiveRefusal( + /** + * Asserts what a refusal actually records: it is not slashable and sets no invalid-slot marker, and the + * slot's outcome is `unvalidated` rather than valid. That last part is not neutral — the sentinel counts + * `unvalidated` as a missed proposal for the slot's proposer whenever no checkpoint for the slot lands. + */ + function expectRefusalRecordedAsUnvalidated( result: CheckpointProposalValidationResult, reason: 'inbox_endpoint_not_live' | 'inbox_endpoint_unverifiable', ) { @@ -1344,14 +1348,14 @@ describe('ProposalHandler checkpoint validation', () => { { seq: 4n, total: 9n, rollingHash: inboxRollingHash }, ]); - expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); }); it('refuses a live boundary that commits to a different message prefix than the signed one', async () => { const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: Fr.random() }]); - expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); }); // A missing endpoint is never special-cased into success: the ring may simply have evicted it. @@ -1359,14 +1363,14 @@ describe('ProposalHandler checkpoint validation', () => { const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setBuckets([{ seq: 40n, total: 5000n, rollingHash: inboxRollingHash }]); - expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); }); it('refuses, without attributing anything to the proposer, when the L1 view cannot be read', async () => { const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setUnreadable(new Error('l1 rpc request failed')); - expectNonPunitiveRefusal(await validate(header), 'inbox_endpoint_unverifiable'); + expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_unverifiable'); }); // A provider still catching up reports the boundary below the checkpoint's end. The gate re-reads within the @@ -1459,6 +1463,64 @@ describe('ProposalHandler checkpoint validation', () => { expect(inbox.reads).toHaveLength(2); }); + // A height is not a view: a provider on a stale fork, or one the chain reorged under, answers a call at a + // captured block as readily as the canonical chain does. An answer that cannot be shown to come from the + // block it was asked at verifies nothing, however exactly its bucket matches. + it('refuses a matching endpoint answered from a view it cannot confirm was the one queried', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + // The block at the captured height keeps changing under the call, so no attempt can bind its answer. + inbox.onRead(readIndex => inbox.setViewHash(`0xreplaced${readIndex}`)); + + expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_unverifiable'); + }); + + // The two seconds are a ceiling on the stage, not a deadline consulted between attempts: a provider that + // accepts the call and never answers must not spend the rest of the slot's duty on one read. + it('gives up at its own ceiling when the L1 read never settles', async () => { + const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setUnresponsive(); + // Start of slot 1, so the duty has its full budget: only the stage's own ceiling can end this read. + dateProvider.setTime(0); + const timer = new Timer(); + + const result = await validate(header); + + // Slot 1's duty runs until its 40s attestation deadline; the endpoint stage may only take two seconds. + expect(timer.ms()).toBeLessThan(10_000); + expectRefusalRecordedAsUnvalidated(result, 'inbox_endpoint_unverifiable'); + }); + + // Pruning the tracker is bookkeeping that happens to read L1 tips. The validator calls this path directly + // for its attestation, so a tips read that never answers must not hold the accepted verdict back. + it('accepts without waiting on the tracker prune to read L1 tips', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + blockSource.getL2Tips.mockReturnValue(new Promise(() => {})); + + await expect(validate(header)).resolves.toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) }); + }); + + // The all-nodes callback and the attestation call the same proposal twice, and `unvalidated` is what the + // sentinel counts as a missed proposal for the slot's proposer. An RPC failure on the second call is this + // node's problem, and must not turn a checkpoint it did validate into a missed proposal for someone else. + it('keeps the slot recorded as valid when a later call cannot read the L1 view', async () => { + const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); + const proposal = await makeProposal({ archiveRoot, checkpointHeader: header }); + await handler.handleCheckpointProposal(proposal, proposalInfo); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); + + inbox.setUnreadable(new Error('l1 rpc request failed')); + + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).resolves.toEqual({ + isValid: false, + reason: 'inbox_endpoint_unverifiable', + checkpointNumber: CheckpointNumber(1), + }); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); + }); + // 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. 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. diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 40b041172c6f..64c01aa9dc8e 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -262,9 +262,9 @@ const INBOX_ENDPOINT_RETRY_WINDOW_MS = 2_000; const INBOX_ENDPOINT_RETRY_INTERVAL_S = 0.5; /** - * Splits an endpoint check failure into a view this node could not read at all and one that answered but did not - * show the signed position closing a live bucket, so the two stay apart in diagnostics. Neither is attributed to - * the proposer, and an absent result (the duty was already over) counts as unread. + * Splits an endpoint check failure into a view this node could not read, or could not identify, and one that + * answered but did not show the signed position ending a live bucket, so the two stay apart in diagnostics. + * Neither is attributed to the proposer, and an absent result (the duty was already over) counts as unread. */ function describeEndpointFailure(result: InboxEndpointCheckResult | undefined): { reason: CheckpointEndpointReason; @@ -273,20 +273,27 @@ function describeEndpointFailure(result: InboxEndpointCheckResult | undefined): if (result === undefined || result.verified) { return { reason: 'inbox_endpoint_unverifiable', context: {} }; } - if (result.reason === 'unreadable') { - return { - reason: 'inbox_endpoint_unverifiable', - context: { endpointReason: result.reason, err: String(result.err) }, - }; + switch (result.reason) { + case 'unreadable': + return { + reason: 'inbox_endpoint_unverifiable', + context: { endpointReason: result.reason, l1BlockNumber: result.l1BlockNumber, err: String(result.err) }, + }; + case 'view_replaced': + return { + reason: 'inbox_endpoint_unverifiable', + context: { endpointReason: result.reason, l1BlockNumber: result.l1BlockNumber }, + }; + default: + return { + reason: 'inbox_endpoint_not_live', + context: { + endpointReason: result.reason, + endpointTotal: result.endpointTotal, + l1BlockNumber: result.l1BlockNumber, + }, + }; } - return { - reason: 'inbox_endpoint_not_live', - context: { - endpointReason: result.reason, - endpointTotal: result.endpointTotal, - l1BlockNumber: result.l1BlockNumber, - }, - }; } /** Block-proposal validation failures that constitute a slashable invalid-block offense. */ @@ -618,6 +625,16 @@ export class ProposalHandler { } await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo); } else if (this.archiver) { + if (budget.signal.aborted) { + // Validation outlived the duty: the callback that started it gave up, stopped the budget and returned, + // and a read that settled afterwards resumed here. Recording the proposal as this node's pipelining + // parent now would be a mutation of accepted state on behalf of a slot nobody is waiting on any more. + this.log.warn( + `Not setting the proposed checkpoint for slot ${proposal.slotNumber}: its duty is over`, + proposalInfo, + ); + return undefined; + } const set = await this.setProposedCheckpoint(proposal, budget); if (set) { this.metrics?.recordCheckpointProposalToPipelinedStateDuration(pipeliningTimer.ms()); @@ -1539,7 +1556,8 @@ export class ProposalHandler { * proposer: a provider trailing the head has not seen the message that closed the bucket yet, an eviction or a * reorg can move the ring after the proposal was signed, and none of that is visible from here. Both are re-read * within {@link INBOX_ENDPOINT_RETRY_WINDOW_MS} and the slot's duty budget, whichever is shorter, so a view that - * recovers in time still yields a valid verdict, and a stalled read cannot hold the acceptance path open. + * recovers in time still yields a valid verdict. That window bounds the whole stage rather than the interval + * between attempts, so a read that never settles is abandoned at it and leaves the rest of the duty its budget. */ private async awaitInboxEndpoint( proposal: CheckpointProposalCore, @@ -1557,21 +1575,24 @@ export class ProposalHandler { const finalTotalMsgCount = this.blockLeafCount(lastBlock); const inboxRollingHash = proposal.checkpointHeader.inboxRollingHash; const timer = new Timer(); - const deadline = new Date( - Math.min(budget.deadline.getTime(), this.dateProvider.now() + INBOX_ENDPOINT_RETRY_WINDOW_MS), - ); let last: InboxEndpointCheckResult | undefined; try { - const verified = await budget.run(`inbox endpoint check for slot ${slot}`, () => - retryUntil( - async () => { - last = await checkInboxEndpoint(this.inbox, finalTotalMsgCount, inboxRollingHash); - return last.verified ? last : undefined; - }, - `live Inbox endpoint at message ${finalTotalMsgCount}`, - { deadline, dateProvider: this.dateProvider }, - INBOX_ENDPOINT_RETRY_INTERVAL_S, - ), + const verified = await budget.runWithin( + `inbox endpoint check for slot ${slot}`, + INBOX_ENDPOINT_RETRY_WINDOW_MS, + signal => + retryUntil( + async () => { + // The window is a race, so an attempt that outlives it has no reader left; the loop it would + // otherwise keep driving stops here rather than starting another read against the same provider. + signal.throwIfAborted(); + last = await checkInboxEndpoint(this.inbox, finalTotalMsgCount, inboxRollingHash); + return last.verified ? last : undefined; + }, + `live Inbox endpoint at message ${finalTotalMsgCount}`, + 0, + INBOX_ENDPOINT_RETRY_INTERVAL_S, + ), ); this.log.debug(`Checkpoint's final message position confirmed as a live Inbox endpoint`, { ...proposalInfo, @@ -1844,15 +1865,30 @@ export class ProposalHandler { } } - // Record the outcome on the re-execution tracker. + // Record the outcome on the re-execution tracker, except where that would forget a validation this node + // already completed. p2p evaluates one proposal twice (all-nodes validation, then attestation) and the second + // look can fail on something purely local; `unvalidated` reaches the sentinel as a missed proposal for the + // slot's proposer, so downgrading a recorded `valid` would charge someone else's validator for an RPC failure + // here. Only the very checkpoint that was validated is protected: another archive at this slot still records. const outcome = result.isValid ? ('valid' as const) : CHECKPOINT_VALIDATION_REASON_TO_OUTCOME[result.reason]; - if (outcome !== undefined) { + const wouldForgetValid = + outcome === 'unvalidated' && + result.checkpointNumber !== undefined && + this.reexecutionTracker.hasReexecuted(result.checkpointNumber, proposal.archive); + if (outcome !== undefined && !wouldForgetValid) { this.reexecutionTracker.recordOutcome(slot, proposal.archive, outcome, result.checkpointNumber); } - // Drop tracker entries for checkpoints that have reached L1 finality. This is bookkeeping the verdict does not - // depend on, so it runs inside the budget and is skipped once the budget is gone rather than holding a caller - // that has already been answered on a store read. + // Tracker pruning is bookkeeping, not part of the verdict, and it reads L1 tips. Nothing waits on it: it runs + // on its own, bounded by whatever is left of the duty so a provider that never answers cannot leave it going + // for the rest of the slot. + void this.pruneReexecutionTracker(slot, proposalInfo, budget); + + return result; + } + + /** Drops re-execution tracker entries for checkpoints that have reached L1 finality. */ + private async pruneReexecutionTracker(slot: SlotNumber, proposalInfo: LogData, budget: DutyBudget): Promise { try { const tips = await budget.run(`reexecution tracker prune for slot ${slot}`, () => this.blockSource.getL2Tips()); const finalizedCheckpointNumber = tips.finalized.checkpoint.number; @@ -1866,8 +1902,6 @@ export class ProposalHandler { this.log.error(`Error pruning reexecution tracker`, err, proposalInfo); } } - - return result; } /** diff --git a/yarn-project/validator-client/src/validator.ha.integration.test.ts b/yarn-project/validator-client/src/validator.ha.integration.test.ts index 79525a9046d4..832ec55ba2fa 100644 --- a/yarn-project/validator-client/src/validator.ha.integration.test.ts +++ b/yarn-project/validator-client/src/validator.ha.integration.test.ts @@ -116,7 +116,7 @@ describe('ValidatorClient HA Integration', () => { l1ToL2MessageSource = mock(); // No messages were ever sent to L1 here, so the Inbox's genesis bucket is the only live endpoint. inbox = { - client: { getBlockNumber: () => Promise.resolve(1n) }, + client: { getBlock: ({ blockNumber }) => Promise.resolve({ number: blockNumber ?? 1n, hash: '0xhead' }) }, getBucketAtOrBeforeTotal: () => Promise.resolve({ seq: 0n, bucket: { rollingHash: Fr.ZERO, totalMsgCount: 0n, timestamp: 0n, msgCount: 0 } }), }; diff --git a/yarn-project/validator-client/src/validator.integration.test.ts b/yarn-project/validator-client/src/validator.integration.test.ts index 41afdb2222db..084f6fc5814a 100644 --- a/yarn-project/validator-client/src/validator.integration.test.ts +++ b/yarn-project/validator-client/src/validator.integration.test.ts @@ -57,7 +57,7 @@ import { ValidatorClient } from './validator.js'; */ function makeArchiverBackedInbox(messageSource: Pick): InboxEndpointReader { return { - client: { getBlockNumber: () => Promise.resolve(1n) }, + client: { getBlock: ({ blockNumber }) => Promise.resolve({ number: blockNumber ?? 1n, hash: '0xhead' }) }, getBucketAtOrBeforeTotal: async upperBound => { const position = await messageSource.getMessagePosition(upperBound); return ( From d6d8a74f21df51c274d5e574bc4e12164abcf8a8 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 23:10:56 -0300 Subject: [PATCH 4/5] fix(validator): record an unreadable Inbox endpoint as unverifiable, not against the proposer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An endpoint refusal recorded `unvalidated`, which the sentinel reports as `checkpoint-unvalidated` and counts as a missed proposal for the slot's proposer. So a node whose own L1 RPC was down through the retry window charged someone else's validator for it, on the first evaluation of an otherwise content-valid proposal — inactivity accounting this gate was never meant to feed. Both endpoint reasons now record `unverifiable`, the outcome for a proposal this observer could not check against anything outside itself, which is counted against nobody. The monotonic protection covers it too, so a later endpoint failure still cannot retract a `valid` this node recorded for the same checkpoint. Co-Authored-By: Claude Opus 5 (1M context) --- yarn-project/validator-client/README.md | 9 +++--- .../src/proposal_handler.test.ts | 28 +++++++++---------- .../validator-client/src/proposal_handler.ts | 18 ++++++------ .../validator-client/src/validator.test.ts | 12 +++++++- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/yarn-project/validator-client/README.md b/yarn-project/validator-client/README.md index 8f20b5eb99b6..db1e3ad1d18a 100644 --- a/yarn-project/validator-client/README.md +++ b/yarn-project/validator-client/README.md @@ -150,10 +150,11 @@ provider and L1 itself all move independently of the moment the proposal was sig 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. -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. +A refusal records `unverifiable` for the slot, which the sentinel reports as `checkpoint-unverifiable`. That status +exists so a refusal is not read as an absent proposal: without a record the sentinel would fall back to +`checkpoint-missed`, which is counted against the proposer. `checkpoint-unverifiable` is counted against nobody — +it says this observer could not check, not that the proposer failed. It also never overwrites 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 diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 15f40fec63d1..9f391839cdcb 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -1299,18 +1299,19 @@ describe('ProposalHandler checkpoint validation', () => { } /** - * Asserts what a refusal actually records: it is not slashable and sets no invalid-slot marker, and the - * slot's outcome is `unvalidated` rather than valid. That last part is not neutral — the sentinel counts - * `unvalidated` as a missed proposal for the slot's proposer whenever no checkpoint for the slot lands. + * Asserts what a refusal actually records: it is not slashable, sets no invalid-slot marker, and records + * the slot as `unverifiable` rather than valid. `unverifiable` is what keeps the refusal out of the + * proposer's accounting: `unvalidated` would reach the sentinel as a missed proposal for that proposer, + * and recording nothing at all would leave the sentinel to fall back to `checkpoint-missed`. */ - function expectRefusalRecordedAsUnvalidated( + function expectRefusalRecordedAsUnverifiable( result: CheckpointProposalValidationResult, reason: 'inbox_endpoint_not_live' | 'inbox_endpoint_unverifiable', ) { expect(result).toEqual({ isValid: false, reason, checkpointNumber: CheckpointNumber(1) }); expect(SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[reason]).toBe(false); expect(handler.hasInvalidProposals(SlotNumber(1))).toBe(false); - expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('unvalidated'); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('unverifiable'); } // The checkpoint's first block ends at 5, inside the bucket closing at 7: only where the checkpoint itself @@ -1348,14 +1349,14 @@ describe('ProposalHandler checkpoint validation', () => { { seq: 4n, total: 9n, rollingHash: inboxRollingHash }, ]); - expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnverifiable(await validate(header), 'inbox_endpoint_not_live'); }); it('refuses a live boundary that commits to a different message prefix than the signed one', async () => { const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: Fr.random() }]); - expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnverifiable(await validate(header), 'inbox_endpoint_not_live'); }); // A missing endpoint is never special-cased into success: the ring may simply have evicted it. @@ -1363,14 +1364,14 @@ describe('ProposalHandler checkpoint validation', () => { const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setBuckets([{ seq: 40n, total: 5000n, rollingHash: inboxRollingHash }]); - expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_not_live'); + expectRefusalRecordedAsUnverifiable(await validate(header), 'inbox_endpoint_not_live'); }); it('refuses, without attributing anything to the proposer, when the L1 view cannot be read', async () => { const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setUnreadable(new Error('l1 rpc request failed')); - expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_unverifiable'); + expectRefusalRecordedAsUnverifiable(await validate(header), 'inbox_endpoint_unverifiable'); }); // A provider still catching up reports the boundary below the checkpoint's end. The gate re-reads within the @@ -1472,7 +1473,7 @@ describe('ProposalHandler checkpoint validation', () => { // The block at the captured height keeps changing under the call, so no attempt can bind its answer. inbox.onRead(readIndex => inbox.setViewHash(`0xreplaced${readIndex}`)); - expectRefusalRecordedAsUnvalidated(await validate(header), 'inbox_endpoint_unverifiable'); + expectRefusalRecordedAsUnverifiable(await validate(header), 'inbox_endpoint_unverifiable'); }); // The two seconds are a ceiling on the stage, not a deadline consulted between attempts: a provider that @@ -1488,7 +1489,7 @@ describe('ProposalHandler checkpoint validation', () => { // Slot 1's duty runs until its 40s attestation deadline; the endpoint stage may only take two seconds. expect(timer.ms()).toBeLessThan(10_000); - expectRefusalRecordedAsUnvalidated(result, 'inbox_endpoint_unverifiable'); + expectRefusalRecordedAsUnverifiable(result, 'inbox_endpoint_unverifiable'); }); // Pruning the tracker is bookkeeping that happens to read L1 tips. The validator calls this path directly @@ -1501,9 +1502,8 @@ describe('ProposalHandler checkpoint validation', () => { await expect(validate(header)).resolves.toEqual({ isValid: true, checkpointNumber: CheckpointNumber(1) }); }); - // The all-nodes callback and the attestation call the same proposal twice, and `unvalidated` is what the - // sentinel counts as a missed proposal for the slot's proposer. An RPC failure on the second call is this - // node's problem, and must not turn a checkpoint it did validate into a missed proposal for someone else. + // The all-nodes callback and the attestation call the same proposal twice. An RPC failure on the second + // call is this node's problem, and must not retract the validation the first call completed. it('keeps the slot recorded as valid when a later call cannot read the L1 view', async () => { const { header, inboxRollingHash } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); inbox.setBuckets([{ seq: 4n, total: 7n, rollingHash: inboxRollingHash }]); diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 64c01aa9dc8e..3da443912673 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -209,10 +209,12 @@ const CHECKPOINT_VALIDATION_REASON_TO_OUTCOME: Record< // Not proposer misbehavior: this node's Inbox view could not confirm the consumed prefix, or disagrees with it. inbox_prefix_unavailable: 'unvalidated', inbox_prefix_mismatch: 'unvalidated', - // Nor is an endpoint this node could not confirm: the bucket ring, the local provider and L1 itself all move - // independently of the moment the checkpoint was signed. - inbox_endpoint_unverifiable: 'unvalidated', - inbox_endpoint_not_live: 'unvalidated', + // An endpoint this node could not confirm is its own failure to check, not the proposer's to answer for: the + // bucket ring, the local provider and L1 itself all move independently of the moment the checkpoint was signed. + // `unverifiable` keeps that out of the proposer's missed-proposal count while still recording that a proposal + // was seen, so the slot is not mistaken for one the proposer skipped. + inbox_endpoint_unverifiable: 'unverifiable', + inbox_endpoint_not_live: 'unverifiable', // This node ran out of time to look; it observed nothing about the proposer. Recorded by the duty-expiry path // itself, which also refuses to overwrite an outcome the slot already has. validation_deadline_expired: 'unverifiable', @@ -1867,12 +1869,12 @@ export class ProposalHandler { // Record the outcome on the re-execution tracker, except where that would forget a validation this node // already completed. p2p evaluates one proposal twice (all-nodes validation, then attestation) and the second - // look can fail on something purely local; `unvalidated` reaches the sentinel as a missed proposal for the - // slot's proposer, so downgrading a recorded `valid` would charge someone else's validator for an RPC failure - // here. Only the very checkpoint that was validated is protected: another archive at this slot still records. + // look can fail on something purely local, so a local-inability outcome never replaces a recorded `valid`: + // the node did validate this checkpoint, and only its second look was unlucky. Only the very checkpoint that + // was validated is protected: another archive at this slot still records. const outcome = result.isValid ? ('valid' as const) : CHECKPOINT_VALIDATION_REASON_TO_OUTCOME[result.reason]; const wouldForgetValid = - outcome === 'unvalidated' && + (outcome === 'unvalidated' || outcome === 'unverifiable') && result.checkpointNumber !== undefined && this.reexecutionTracker.hasReexecuted(result.checkpointNumber, proposal.archive); if (outcome !== undefined && !wouldForgetValid) { diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 92cd5f82e168..1c74de0d6244 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -931,6 +931,11 @@ describe('ValidatorClient', () => { }); inbox.setBuckets([{ seq: 0n, total: 0n, rollingHash: checkpointProposal.checkpointHeader.inboxRollingHash }]); + // The checkpoint consumes nothing, so it ends where the Inbox's genesis bucket does; make that bucket + // commit to the hash this proposal signed, so the live endpoint gate confirms it and the flow reaches + // the signer. + inbox.setBuckets([{ seq: 0n, total: 0n, rollingHash: checkpointProposal.checkpointHeader.inboxRollingHash }]); + const validateCheckpointSpy = jest .spyOn(validatorClient.getProposalHandler(), 'validateCheckpointProposal') .mockResolvedValue({ isValid: true, checkpointNumber: CheckpointNumber(1) }); @@ -974,13 +979,18 @@ describe('ValidatorClient', () => { }, }); + // The checkpoint consumes nothing, so it ends where the Inbox's genesis bucket does; make that bucket + // commit to the hash this proposal signed, so the live endpoint gate confirms it and the flow reaches + // the signer. + inbox.setBuckets([{ seq: 0n, total: 0n, rollingHash: checkpointProposal.checkpointHeader.inboxRollingHash }]); + const validateCheckpointSpy = jest .spyOn(validatorClient.getProposalHandler(), 'validateCheckpointProposal') .mockResolvedValue({ isValid: true, checkpointNumber: CheckpointNumber(1) }); // A short but nonzero budget, and a signer that never answers within it. const deadline = validatorClient.getProposalHandler().getReexecutionDeadline(proposal.slotNumber); - dateProvider.setTime(deadline.getTime() - 200); + dateProvider.setTime(deadline.getTime() - 3_000); const validationService = (validatorClient as unknown as { validationService: ValidationService }) .validationService; jest.spyOn(validationService, 'attestToCheckpointProposal').mockImplementation(() => new Promise(() => {})); From b5f0c774c486ac31fe2a45db7f1e75ad7c930d92 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 23:41:09 -0300 Subject: [PATCH 5/5] fix(validator): stop an unreadable endpoint erasing a slot already determined invalid The tracker keys its per-slot entry by slot alone, and the protection against a local-inability outcome replacing a determination only covered `valid`, and only for the very checkpoint that produced it. An equivocating proposer whose second proposal for the slot reached the endpoint gate and could not be checked therefore erased what the first one established. Uncertainty now never overwrites a verdict in either direction. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/proposal_handler.test.ts | 18 +++++++++++++ .../validator-client/src/proposal_handler.ts | 25 +++++++++++-------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 9f391839cdcb..b10d77b318a4 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -1521,6 +1521,24 @@ describe('ProposalHandler checkpoint validation', () => { expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); }); + // Forgetting a determination is never the safe direction, and the tracker keys its per-slot entry by slot + // alone. An equivocating proposer whose second proposal this node cannot check must not thereby erase what + // the first one established about the slot. + it('keeps a slot recorded as invalid when a later proposal for it cannot be checked', async () => { + // A different archive at this slot was already determined invalid: an equivocating proposer's first one. + reexecutionTracker.recordOutcome(SlotNumber(1), Fr.random(), 'invalid', CheckpointNumber(1)); + + const { header } = setupContentValidCheckpoint({ midLeafCount: 5, lastLeafCount: 7 }); + inbox.setUnreadable(new Error('l1 rpc request failed')); + + await expect(validate(header)).resolves.toEqual({ + isValid: false, + reason: 'inbox_endpoint_unverifiable', + checkpointNumber: CheckpointNumber(1), + }); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('invalid'); + }); + // 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. 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. diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 3da443912673..2f4d1f6ceeb4 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -1867,17 +1867,22 @@ export class ProposalHandler { } } - // Record the outcome on the re-execution tracker, except where that would forget a validation this node - // already completed. p2p evaluates one proposal twice (all-nodes validation, then attestation) and the second - // look can fail on something purely local, so a local-inability outcome never replaces a recorded `valid`: - // the node did validate this checkpoint, and only its second look was unlucky. Only the very checkpoint that - // was validated is protected: another archive at this slot still records. + // Record the outcome on the re-execution tracker, except where uncertainty would replace something this node + // determined. p2p evaluates one proposal twice (all-nodes validation, then attestation) and the second look + // can fail on something purely local, so a local-inability outcome never overwrites a verdict. + // + // A recorded `valid` is protected for the very checkpoint that produced it — a different archive at the same + // slot is a different question, and still records. A recorded `invalid` is protected for the slot outright: + // the tracker keys its slot entry by slot alone, so an equivocating proposer whose second proposal this node + // could not check would otherwise erase the first one's determination. const outcome = result.isValid ? ('valid' as const) : CHECKPOINT_VALIDATION_REASON_TO_OUTCOME[result.reason]; - const wouldForgetValid = - (outcome === 'unvalidated' || outcome === 'unverifiable') && - result.checkpointNumber !== undefined && - this.reexecutionTracker.hasReexecuted(result.checkpointNumber, proposal.archive); - if (outcome !== undefined && !wouldForgetValid) { + const isLocalInability = outcome === 'unvalidated' || outcome === 'unverifiable'; + const wouldForgetVerdict = + isLocalInability && + (this.reexecutionTracker.getOutcomeForSlot(slot) === 'invalid' || + (result.checkpointNumber !== undefined && + this.reexecutionTracker.hasReexecuted(result.checkpointNumber, proposal.archive))); + if (outcome !== undefined && !wouldForgetVerdict) { this.reexecutionTracker.recordOutcome(slot, proposal.archive, outcome, result.checkpointNumber); }