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..db1e3ad1d18a 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,54 @@ 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 ends 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. + +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. + +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. + +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 +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..b7a4dd1330da --- /dev/null +++ b/yarn-project/validator-client/src/checkpoint_endpoint_check.test.ts @@ -0,0 +1,137 @@ +import { Fr } from '@aztec/foundation/curves/bn254'; + +import { describe, expect, it } from '@jest/globals'; + +import { checkInboxEndpoint } from './checkpoint_endpoint_check.js'; +import { type LiveBucket, makeFakeInbox } from './fake_inbox_test_helper.js'; + +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 = makeFakeInbox(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 = makeFakeInbox(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 = makeFakeInbox(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 = makeFakeInbox(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 = makeFakeInbox([{ 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 = makeFakeInbox([{ 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 = makeFakeInbox(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 = makeFakeInbox(ring, { failBucket: err }); + + 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 new file mode 100644 index 000000000000..b3b04269efd6 --- /dev/null +++ b/yarn-project/validator-client/src/checkpoint_endpoint_check.ts @@ -0,0 +1,103 @@ +import type { InboxContract } from '@aztec/ethereum/contracts'; +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 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: { + 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. */ +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. 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 } + /** 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`. + * + * 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. + * + * 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 { + 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 }; + } + 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', 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/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/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/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/fake_inbox_test_helper.ts b/yarn-project/validator-client/src/fake_inbox_test_helper.ts new file mode 100644 index 000000000000..9fd3efc944b2 --- /dev/null +++ b/yarn-project/validator-client/src/fake_inbox_test_helper.ts @@ -0,0 +1,87 @@ +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; + /** Makes every bucket read hang, the way a provider that accepts a call and never answers does. */ + setUnresponsive(): void; + /** Replaces the block reported at the head's height, the way a provider answering from a stale fork does. */ + setViewHash(hash: string | null): void; + /** Runs before each bucket read with its index, so a test can move the L1 view between two attempts. */ + onRead(hook: (readIndex: number) => void): void; +}; + +/** The L1 head fake reads are pinned to, unless a test asks for another one. */ +const DEFAULT_HEAD = 900n; + +/** The hash of the block at the head's height, unless a test replaces it mid-check. */ +const DEFAULT_HEAD_HASH = '0xhead'; + +/** + * An Inbox holding the given live buckets, resolving an upper bound the way the contract does: the newest live + * bucket ending at or below it, or nothing at all once the ring has evicted every bucket that could have matched. + * 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 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, + setBuckets: next => { + live = next; + }, + setUnreadable: err => { + failHead = err; + failBucket = err; + }, + setUnresponsive: () => { + unresponsive = true; + }, + setViewHash: hash => { + viewHash = hash; + }, + onRead: hook => { + beforeRead = hook; + }, + client: { + getBlock: ({ blockNumber }) => + failHead ? Promise.reject(failHead) : Promise.resolve({ number: blockNumber ?? head, hash: viewHash }), + }, + getBucketAtOrBeforeTotal: (upperBound, readOpts) => { + beforeRead(reads.length); + reads.push({ upperBound, blockNumber: readOpts?.blockNumber }); + if (failBucket) { + return Promise.reject(failBucket); + } + if (unresponsive) { + return new Promise(() => {}); + } + const match = [...live].sort((a, b) => Number(a.total - b.total)).findLast(bucket => bucket.total <= upperBound); + return Promise.resolve( + match && { + seq: match.seq, + bucket: { rollingHash: match.rollingHash, totalMsgCount: match.total, timestamp: 1n, msgCount: 1 }, + }, + ); + }, + }; +} diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 731809128c83..b10d77b318a4 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'; @@ -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 FakeInbox, makeFakeInbox } from './fake_inbox_test_helper.js'; import type { ValidatorMetrics } from './metrics.js'; import { type CheckpointProposalValidationResult, @@ -109,6 +110,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 +129,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 +166,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, mock(), epochCache, consensusTimetable, @@ -268,6 +274,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, mock(), epochCache, consensusTimetable, @@ -1198,6 +1205,364 @@ 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 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 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('unverifiable'); + } + + // 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: 900n }]); + }); + + // 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: 900n }]); + }); + + // 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 }, + ]); + + 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() }]); + + expectRefusalRecordedAsUnverifiable(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 }]); + + 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')); + + 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 + // 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 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}`)); + + 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 + // 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); + expectRefusalRecordedAsUnverifiable(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. 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 }]); + 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'); + }); + + // 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. + 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), + }); + expect(checkpointsBuilder.openCheckpoint).toHaveBeenCalledTimes(1); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); + }); + }); }); /** @@ -1229,6 +1594,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, @@ -1241,6 +1607,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 +1773,7 @@ describe('ProposalHandler checkpoint validation', () => { mock(), blockSource, l1ToL2MessageSource, + inbox, txProvider, epochCache, consensusTimetable, @@ -1455,6 +1835,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..2f4d1f6ceeb4 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,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', + // 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', @@ -234,6 +255,49 @@ 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, 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; + context: LogData; +} { + if (result === undefined || result.verified) { + return { reason: 'inbox_endpoint_unverifiable', context: {} }; + } + 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, + }, + }; + } +} + /** Block-proposal validation failures that constitute a slashable invalid-block offense. */ export const SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT: BlockProposalValidationFailureReason[] = [ 'state_mismatch', @@ -267,6 +331,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 +386,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 +422,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 +573,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 +598,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()); @@ -545,6 +627,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()); @@ -1456,6 +1548,79 @@ export class ProposalHandler { return resolved; } + /** + * 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. 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 + * 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. 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, + 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(); + let last: InboxEndpointCheckResult | undefined; + try { + 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, + 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, @@ -1639,48 +1804,98 @@ 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; - 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) { + // 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) { this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); - return cached; + result = cached; + } else { + this.log.warn( + `Re-validating checkpoint proposal at slot ${slot}: its blocks are no longer local`, + proposalInfo, + ); } - 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); + } } - 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. + // 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]; - if (outcome !== undefined) { + 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); } - // 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; @@ -1694,13 +1909,19 @@ export class ProposalHandler { this.log.error(`Error pruning reexecution tracker`, err, proposalInfo); } } + } - // 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 }), + ); } /** @@ -1847,7 +2068,7 @@ export class ProposalHandler { const consumed = await this.awaitCheckpointConsumedMessages( slot, checkpointStartTotal, - this.blockLeafCount(blocks[blocks.length - 1]), + this.blockLeafCount(lastBlock), proposal.checkpointHeader.inboxRollingHash, proposalInfo, budget, 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..832ec55ba2fa 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: { getBlock: ({ blockNumber }) => Promise.resolve({ number: blockNumber ?? 1n, hash: '0xhead' }) }, + 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..084f6fc5814a 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: { getBlock: ({ blockNumber }) => Promise.resolve({ number: blockNumber ?? 1n, hash: '0xhead' }) }, + 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..1c74de0d6244 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -64,6 +64,7 @@ import type { } from './checkpoint_builder.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'; @@ -118,6 +119,7 @@ describe('ValidatorClient', () => { let p2pClient: MockProxy; let blockSource: MockProxy; let l1ToL2MessageSource: MockProxy; + let inbox: FakeInbox; let epochCache: MockProxy; let checkpointsBuilder: MockProxy; let worldState: MockProxy; @@ -194,6 +196,9 @@ 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 = makeFakeInbox(); txProvider = mock(); dateProvider = new TestDateProvider(); blobClient = mock(); @@ -241,6 +246,7 @@ describe('ValidatorClient', () => { p2pClient, blockSource, l1ToL2MessageSource, + inbox, txProvider, keyStoreManager, blobClient, @@ -880,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 @@ -920,6 +930,12 @@ 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) }); @@ -963,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(() => {})); 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);