From 7d2df1685c73b14d85ed6db6e855a963f2ad5541 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:08:02 -0300 Subject: [PATCH 01/25] fix(archiver): distinguish RPC uncertainty from chain replacement and scan the deployment block An L1 block this node depends on now reads back as canonical, positively replaced, or unreadable. An RPC exception, a lagging provider and a pruned range all read as unreadable, and none of them deletes messages, restarts recovery or claims a replacement any more. A head reporting fewer messages than the local log is also no longer truncated against while the certified syncpoint above it is still canonical, so a provider that is merely behind cannot prune valid speculative consumers. Ordinary ingestion now re-reads the deployment block while the scanned cursor still sits on it. The Inbox's first message can be emitted by a later transaction in that block, and an exclusive cursor defaulting to it skipped index 0 permanently, including on stores a zero-anchor rollback had already rewound onto it. A missing recovery candidate no longer reports the Inbox's tip hash as the hash it expected at that count. Original findings: SUP-01, SUP-04, SUP-06, SUP-10. Co-Authored-By: Claude Opus 5 (1M context) --- .../scripts/generate-artifacts.sh | 1 + .../archiver/src/archiver-sync.test.ts | 202 +++++++++++++++++- yarn-project/archiver/src/errors.ts | 5 +- .../src/modules/inbox_message_synchronizer.ts | 181 +++++++++++++--- 4 files changed, 343 insertions(+), 46 deletions(-) diff --git a/l1-contracts/l1-artifacts/scripts/generate-artifacts.sh b/l1-contracts/l1-artifacts/scripts/generate-artifacts.sh index 8a224afdcdb0..5c0245208477 100755 --- a/l1-contracts/l1-artifacts/scripts/generate-artifacts.sh +++ b/l1-contracts/l1-artifacts/scripts/generate-artifacts.sh @@ -15,6 +15,7 @@ contracts=( "EscapeHatch" "SlashingProposer" "EmpireBase" + "EpochProofExtLib" "RollupOperationsExtLib" "ValidatorOperationsExtLib" "RewardExtLib" diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index 0d8fa7ff0400..f66f2f76b0f1 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1846,7 +1846,7 @@ describe('Archiver Sync', () => { // resumes there. No comparison with the Inbox covered them, so they leave no syncpoint and nothing was // announced. expect(await getStoredLeaves()).toEqual(asHex(msgs.slice(0, 4))); - expect((await archiverStore.messages.getScannedL1Block())?.l1BlockNumber).toEqual(106n); + expect((await archiverStore.messages.getScannedL1Block())?.l1BlockNumber).toEqual(107n); expect(await archiverStore.messages.getSynchedL1Block()).toBeUndefined(); expect(archiver.getL1BlockNumber()).toBeUndefined(); @@ -2105,12 +2105,12 @@ describe('Archiver Sync', () => { const [a] = randomLeaves(1); fake.addMessages(CheckpointNumber(1), 2n, [a]); fake.setL1BlockNumber(4n); - // While the first batch (blocks 1-2) is being fetched, L1 replaces block 2 with a block carrying no message and + // While the batch covering blocks 2-3 is being fetched, L1 replaces block 2 with a block carrying no message and // shortens to it: the batch's logs belong to the old chain, its syncpoint block to the new one. const readLogs = inboxContract.getMessageSentEvents.getMockImplementation()!; inboxContract.getMessageSentEvents.mockImplementation(async (from, to) => { const logs = await readLogs(from, to); - if (to === 2n) { + if (to === 3n) { inboxContract.getMessageSentEvents.mockImplementation(readLogs); fake.removeMessagesAfter(0); fake.reorgL1BlocksFrom(2n); @@ -2289,28 +2289,28 @@ describe('Archiver Sync', () => { fake.addMessages(CheckpointNumber(1), 10n, [c]); fake.setL1BlockNumber(10n); - // The provider answers the blocks 1-2 range without B and then stops serving logs, so the empty blocks 3-4 + // The provider answers the blocks 2-3 range without B and then stops serving logs, so the empty blocks 4-5 // batch is the last one to be scanned. Every block involved stays canonical: only the response was incomplete. const readLogs = inboxContract.getMessageSentEvents.getMockImplementation()!; inboxContract.getMessageSentEvents.mockImplementation(async (from, to) => { const logs = await readLogs(from, to); - return to === 2n ? logs.slice(0, 1) : logs; + return to === 3n ? logs.slice(0, 1) : logs; }); - fake.setMessageSentEventsFailure(from => from >= 5n); + fake.setMessageSentEventsFailure(from => from >= 6n); await expect(archiver.syncImmediate()).rejects.toThrow(/Cannot serve MessageSent logs/); expect(await getStoredLeaves()).toEqual(asHex([a])); expect(archiver.getL1BlockNumber()).toBeUndefined(); - // A later view of L1 ends exactly at block 4, the last block scanned. Nothing has compared the log with the + // A later view of L1 ends exactly at block 5, the last block scanned. Nothing has compared the log with the // Inbox there, so the head must not be answered from the scanned cursor: B is still missing. inboxContract.getMessageSentEvents.mockImplementation(readLogs); fake.setMessageSentEventsFailure(undefined); - fake.setL1BlockNumber(4n); + fake.setL1BlockNumber(5n); await archiver.syncImmediate(); expect(await getStoredLeaves()).toEqual(asHex([a, b])); - expect(archiver.getL1BlockNumber()).toEqual(4n); + expect(archiver.getL1BlockNumber()).toEqual(5n); fake.setL1BlockNumber(10n); await archiver.syncImmediate(); @@ -2354,7 +2354,7 @@ describe('Archiver Sync', () => { const [a] = randomLeaves(1); fake.addMessages(CheckpointNumber(1), 2n, [a]); fake.setL1BlockNumber(4n); - fake.setMessageSentEventsFailure((_from, to) => to >= 3n); + fake.setMessageSentEventsFailure((_from, to) => to >= 4n); await expect(archiver.syncImmediate()).rejects.toThrow(/Cannot serve MessageSent logs/); expect(await getStoredLeaves()).toEqual(asHex([a])); expect(await archiverStore.messages.getSynchedL1Block()).toBeUndefined(); @@ -2369,6 +2369,188 @@ describe('Archiver Sync', () => { expect(archiver.getL1BlockNumber()).toEqual(0n); }); + describe('provider uncertainty versus chain replacement', () => { + /** L1 block numbers the provider currently refuses to answer for, as a momentarily unavailable one would. */ + const unreadable = new Set(); + const withUnreadableL1Blocks = () => { + const readBlock = publicClient.getBlock.getMockImplementation()!; + publicClient.getBlock.mockImplementation(((args: { blockNumber?: bigint } = {}) => + args?.blockNumber !== undefined && unreadable.has(args.blockNumber) + ? Promise.reject(new Error('provider unavailable')) + : readBlock(args)) as any); + }; + + beforeEach(() => { + unreadable.clear(); + withUnreadableL1Blocks(); + }); + + it('does not delete messages when a provider reports a head behind the certified syncpoint', async () => { + const [a, b] = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 100n, [a]); + fake.addMessages(CheckpointNumber(1), 106n, [b]); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + const [block1, block2] = await addLocalBlocksConsuming([1, 2]); + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + + // A lagged provider answers at block 105, before B was mined. Block 110 is still canonical, so the log and + // the block that consumed B must survive. + fake.setL1BlockNumber(105n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + expect(await localBlockNumbers()).toEqual([block1.number, block2.number]); + expect(pruneSpy).not.toHaveBeenCalled(); + expect(archiver.getL1BlockNumber()).toEqual(110n); + + // The provider catches up and nothing had to be refetched. + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + expect(await localBlockNumbers()).toEqual([block1.number, block2.number]); + }); + + it('truncates to a genuinely shorter chain once the syncpoint block is positively replaced', async () => { + const [a, b] = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 100n, [a]); + fake.addMessages(CheckpointNumber(1), 106n, [b]); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + await addLocalBlocksConsuming([1, 2]); + + // L1 really does drop B and shorten: the syncpoint's block is replaced, so nothing vouches for the tail. + fake.removeMessagesAfter(1); + fake.reorgL1BlocksFrom(106n); + fake.setL1BlockNumber(105n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(archiver.getL1BlockNumber()).toEqual(105n); + }); + + it('does not start recovery when the scanned cursor block cannot be read', async () => { + await useArchiver({ batchSize: 1 }); + const [a, b] = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 2n, [a]); + fake.setL1BlockNumber(4n); + await archiver.syncImmediate(); + const [block1] = await addLocalBlocksConsuming([1]); + + fake.addMessages(CheckpointNumber(1), 6n, [b]); + fake.setL1BlockNumber(8n); + unreadable.add(4n); + await archiver.syncImmediate(); + + // The cursor's block was unreadable, so the pass waited rather than rolling the log back. + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(await localBlockNumbers()).toEqual([block1.number]); + expect(pruneSpy).not.toHaveBeenCalled(); + expect(synchronizer.isRecoveringMessages()).toBe(false); + + // Once the provider answers for it again, the same forward ingestion continues; nothing had to be refetched. + unreadable.clear(); + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + expect(pruneSpy).not.toHaveBeenCalled(); + expect(archiver.getL1BlockNumber()).toEqual(8n); + }); + }); + + describe('deployment block ingestion', () => { + // The Inbox's first message can be sent by a later transaction inside the block the contracts were deployed + // in. An exclusive scanned cursor defaulting to that block would resume one block later and never read it. + it('fetches a message emitted in the deployment block when the head is still there', async () => { + const [a] = randomLeaves(1); + fake.addMessages(CheckpointNumber(1), 0n, [a]); + fake.setL1BlockNumber(0n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + + it('fetches a deployment-block message once L1 has advanced past it', async () => { + const [a] = randomLeaves(1); + fake.addMessages(CheckpointNumber(1), 0n, [a]); + fake.setL1BlockNumber(6n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(archiver.getL1BlockNumber()).toEqual(6n); + }); + + it('does not lose index 0 when a later message arrives after it', async () => { + const [a, b] = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 0n, [a]); + fake.setL1BlockNumber(2n); + await archiver.syncImmediate(); + fake.addMessages(CheckpointNumber(1), 4n, [b]); + fake.setL1BlockNumber(6n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + + it('re-reads the deployment block after a restart, one L1 block at a time', async () => { + await useArchiver({ batchSize: 1 }); + const [a] = randomLeaves(1); + fake.addMessages(CheckpointNumber(1), 0n, [a]); + fake.setL1BlockNumber(4n); + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex([a])); + + // A fresh archiver over the same store resumes from the persisted cursor and must not re-read or duplicate. + const restarted = await buildArchiver('archiver_message_recovery', { batchSize: 1, store: archiverStore }); + try { + fake.setL1BlockNumber(6n); + await restarted.archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(restarted.synchronizer.isRecoveringMessages()).toBe(false); + } finally { + await restarted.archiver.stop(); + } + }); + + it('unsticks a store whose cursor was already rewound onto the deployment block', async () => { + const [a] = randomLeaves(1); + fake.addMessages(CheckpointNumber(1), 0n, [a]); + // Reproduce what a zero-anchor recovery persists: cursor pinned at the deployment block, empty log. + await archiverStore.messages.setMessageSyncState({ + l1Block: { l1BlockNumber: 0n, l1BlockHash: fake.getL1BlockHash(0n) }, + authenticated: false, + }); + fake.setL1BlockNumber(6n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a])); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + + it('stays synced when the deployment block holds no message', async () => { + fake.setL1BlockNumber(4n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual([]); + expect(archiver.getL1BlockNumber()).toEqual(4n); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + + it('keeps exclusive semantics once the cursor is past the deployment block', async () => { + const [a, b] = randomLeaves(2); + fake.addMessages(CheckpointNumber(1), 2n, [a]); + fake.setL1BlockNumber(4n); + await archiver.syncImmediate(); + fake.addMessages(CheckpointNumber(1), 6n, [b]); + fake.setL1BlockNumber(8n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex([a, b])); + expect(synchronizer.isRecoveringMessages()).toBe(false); + }); + }); + it('leaves the batch that refills the log after a rollback uncommitted when it disagrees with the Inbox', async () => { const [a, b, c, d] = randomLeaves(4); fake.addMessages(CheckpointNumber(1), 100n, [a]); diff --git a/yarn-project/archiver/src/errors.ts b/yarn-project/archiver/src/errors.ts index 1b64961a7565..db1d613ed32f 100644 --- a/yarn-project/archiver/src/errors.ts +++ b/yarn-project/archiver/src/errors.ts @@ -109,11 +109,12 @@ export class BlockAlreadyCheckpointedError extends Error { export class InboxMessagePrefixChangedError extends Error { constructor( public readonly totalMessageCount: bigint, - public readonly expected: Fr, + /** The rolling hash the prefix was expected to have, when a caller knows it for this exact count. */ + public readonly expected: Fr | undefined, public readonly actual: Fr | undefined, ) { super( - `Inbox message prefix at count ${totalMessageCount} changed from ${expected.toString()} to ` + + `Inbox message prefix at count ${totalMessageCount} changed from ${expected?.toString() ?? 'unavailable'} to ` + `${actual?.toString() ?? 'unavailable'} while a replacement was being prepared`, ); this.name = 'InboxMessagePrefixChangedError'; diff --git a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts index 706bb4addfa4..68149e2092f0 100644 --- a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts +++ b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts @@ -86,7 +86,9 @@ export type InboxMessageRecoveryProgress = { * needs no event lookups), or a stored message whose event L1 still emits at the same index and hash within five L1 * blocks of the height it was observed at, found by walking the log backwards with a bounded number of event lookups * per pass. A lookup that misses moves the search to an older candidate, and running out of candidates falls back to - * the deployment block. Once an anchor is chosen the log is rolled back to it in one store transaction: the suffix + * the deployment block, which is itself re-read: the Inbox's first message can be emitted by a later transaction in + * the block the contracts were deployed in, so the deployment block is the one block an exclusive cursor may not + * skip. Once an anchor is chosen the log is rolled back to it in one store transaction: the suffix * rows are deleted, the proposed blocks that consumed more messages than the retained count are pruned with their * descendants, the scanned cursor rewinds to the block before the anchor's and the syncpoint is cleared. Nothing is * fetched in that pass; ordinary forward ingestion refills the log from the rewound cursor, rewriting the retained @@ -100,11 +102,20 @@ export type InboxMessageRecoveryProgress = { * deleted rows are re-fetched, and published checkpoints are never deleted by this path. An RPC exception is not a * miss and commits nothing. * - * Recovery is pinned to the head it started against: a merely advancing `latest` does not reset it, only a replaced - * or unavailable head does. Event lookups are bounded above by that head, so an anchor can never sit at or past it - * and leave the rewound cursor unreachable. The search position is process-local; after a restart, anchor discovery + * Recovery is pinned to the head it started against: a merely advancing `latest` does not reset it, only a positively + * replaced head does. Event lookups are bounded above by that head, so an anchor can never sit at or past it and + * leave the rewound cursor unreachable. The search position is process-local; after a restart, anchor discovery * starts over from the stored log, which is correct. * + * Every L1 block this class depends on is checked with three outcomes, not two: canonical, positively replaced, or + * unreadable. An RPC exception, a provider behind the height and a pruned range all read as unreadable, and none of + * them is evidence of a reorg. Unreadable therefore commits nothing, deletes nothing, logs no replacement and keeps + * an in-flight recovery's search position; only a block that reads back with a different hash restarts recovery. For + * the same reason a head reporting fewer messages than the local log is not truncated against while a syncpoint + * above it is still canonical: that syncpoint certified the whole log at a higher block, so the shortfall is the + * provider's view, not the chain's. When neither reading can settle the ambiguity the pass reports pending rather + * than inventing evidence either way. + * * The inherited finalized-height shortcut is kept: a stored message observed at or below the finality marker * persisted by the last sync that reached agreement with L1 is accepted as an anchor without a lookup, and the marker * only advances on such agreement. A message re-mined above the finalized height whose old height was below it can @@ -166,11 +177,20 @@ export class InboxMessageSynchronizer { private async syncPass(head: L1BlockId, finalizedL1Block: L1BlockId | undefined): Promise { if (this.recovery !== undefined) { const pinnedHead = this.recovery.head; - if (await this.isHeadStillCanonical(pinnedHead)) { + const pinnedStatus = await this.checkL1Block(pinnedHead); + if (pinnedStatus === 'canonical') { const result = await this.continueRecovery(); // Recovery is complete relative to the head it was pinned to; blocks after it still need normal ingestion. return result.status === 'synced' && !sameL1Block(pinnedHead, head) ? { ...result, status: 'pending' } : result; } + if (pinnedStatus === 'unknown') { + // The pinned head could not be read. That is a provider problem, not evidence its chain is gone: keep the + // search position and its lookup progress and try again next pass. + this.log.verbose(`Could not confirm the L1 head recovery is pinned to; keeping recovery progress`, { + ...this.getRecoveryProgress(), + }); + return pending(); + } this.log.warn(`L1 head ${this.recovery.head.l1BlockNumber} recovery was pinned to has been replaced`, { ...this.getRecoveryProgress(), }); @@ -198,8 +218,8 @@ export class InboxMessageSynchronizer { const local = await this.stores.messages.getSyncedMessagePosition(); if (positionMatches(local, remote)) { // The state was read by block number: only a head that is still canonical proves it was this head's. - if (!(await this.isHeadStillCanonical(head))) { - this.log.verbose(`L1 head ${head.l1BlockNumber} was replaced while reading the Inbox state`); + if ((await this.checkL1Block(head)) !== 'canonical') { + this.log.verbose(`Could not confirm L1 head ${head.l1BlockNumber} after reading the Inbox state`); return pending(); } await this.stores.messages.setMessageSyncState({ l1Block: head, authenticated: true, finalizedL1Block }); @@ -207,12 +227,20 @@ export class InboxMessageSynchronizer { } if (remote.totalMessagesInserted < local.totalMessageCount) { + // A head shorter than the local log is ambiguous: the chain really did shorten, or this provider is behind the + // one the log was certified against. A retained syncpoint above this head that is still canonical settles it as + // lag, and lag must not delete messages or claim a lower head as synced. + if (await this.isLaggedView(head, persistedSyncPoint)) { + return pending(); + } // A shorter canonical sequence whose tip hash is our prefix hash at that count is a pure truncation; the tip // itself proves where it ends, so no old placement lookup is needed. const localAtRemote = await this.stores.messages.getMessagePosition(remote.totalMessagesInserted); if (localAtRemote !== undefined && localAtRemote.rollingHash.equals(remote.rollingHash)) { - if (!(await this.isHeadStillCanonical(head))) { - this.log.verbose(`L1 head ${head.l1BlockNumber} was replaced while reading the Inbox state; not truncating`); + if ((await this.checkL1Block(head)) !== 'canonical') { + this.log.verbose( + `Could not confirm L1 head ${head.l1BlockNumber} after reading the Inbox state; ` + `not truncating`, + ); return pending(); } return this.truncate(localAtRemote, head, finalizedL1Block); @@ -220,8 +248,9 @@ export class InboxMessageSynchronizer { return this.startRecovery(head, remote, finalizedL1Block); } - if (head.l1BlockNumber <= cursor.l1BlockNumber) { - // A head at or below what has already been scanned, and the log does not agree with it: there is no forward + const ingestFrom = this.ingestionStartFor(cursor); + if (head.l1BlockNumber < ingestFrom) { + // A head below the first block still to be scanned, and the log does not agree with it: there is no forward // range to fetch, so find where the local log and the canonical one part ways. return this.startRecovery(head, remote, finalizedL1Block); } @@ -230,18 +259,31 @@ export class InboxMessageSynchronizer { // inherited prefix. That is only sound while the cursor's block is still on the chain: after a reorg below it, the // messages read up to it may belong to a chain L1 no longer has, so the log has to be compared with the canonical // one instead. - if (persistedCursor !== undefined && !(await this.isHeadStillCanonical(persistedCursor))) { - this.log.warn(`L1 block ${cursor.l1BlockNumber} the message log was scanned through has been replaced`, { - cursor, - syncPoint: persistedSyncPoint, - headL1BlockNumber: head.l1BlockNumber, - }); - return this.startRecovery(head, remote, finalizedL1Block); + if (persistedCursor !== undefined) { + const cursorStatus = await this.checkL1Block(persistedCursor); + if (cursorStatus === 'unknown') { + // Nothing was learned about the cursor's block, so the inherited prefix is neither proven nor disproven. + // Fetching forward would inherit an unverified prefix and recovering would delete on no evidence: wait. + this.log.verbose(`Could not confirm L1 block ${cursor.l1BlockNumber} the message log was scanned through`, { + cursor, + syncPoint: persistedSyncPoint, + headL1BlockNumber: head.l1BlockNumber, + }); + return pending(); + } + if (cursorStatus === 'replaced') { + this.log.warn(`L1 block ${cursor.l1BlockNumber} the message log was scanned through has been replaced`, { + cursor, + syncPoint: persistedSyncPoint, + headL1BlockNumber: head.l1BlockNumber, + }); + return this.startRecovery(head, remote, finalizedL1Block); + } } let headBatch: InboxMessage[]; try { - headBatch = await this.ingestForward(cursor.l1BlockNumber + 1n, head); + headBatch = await this.ingestForward(ingestFrom, head); } catch (err) { if (err instanceof CapturedHeadReplacedError) { this.log.verbose(`L1 head ${head.l1BlockNumber} was replaced while fetching L1 to L2 messages`); @@ -256,10 +298,10 @@ export class InboxMessageSynchronizer { throw err; } - if (!(await this.isHeadStillCanonical(head))) { - // The chain moved under the fetch: the logs may belong to another chain than the position they are compared - // with, so neither the head batch nor a recovery is committed; the next pass reads the replacement head. - this.log.verbose(`L1 head ${head.l1BlockNumber} was replaced while fetching L1 to L2 messages`); + if ((await this.checkL1Block(head)) !== 'canonical') { + // The chain may have moved under the fetch: the logs would then belong to another chain than the position they + // are compared with, so neither the head batch nor a recovery is committed; the next pass reads a fresh head. + this.log.verbose(`Could not confirm L1 head ${head.l1BlockNumber} after fetching L1 to L2 messages`); return pending(); } const positionAfterHeadBatch = @@ -283,6 +325,48 @@ export class InboxMessageSynchronizer { return this.startRecovery(head, remote, finalizedL1Block); } + /** + * Whether a head shorter than the local log is a lagged provider view rather than a real chain replacement. + * + * The syncpoint is the highest L1 block at which the whole stored log was found equal to the Inbox's own position. + * If that block is above this head and still canonical, the chain did not shorten past it: the messages the head + * appears to be missing are on the chain, and this provider simply has not reached them. Deleting them here would + * throw away certified messages and prune the proposed blocks that consumed them, only for the next pass to fetch + * them straight back. + * + * An unreadable syncpoint block is not treated as lag: without positive evidence the shorter head is handled by the + * ordinary path, which authenticates whatever it retains. + */ + private async isLaggedView(head: L1BlockId, syncPoint: L1BlockId | undefined): Promise { + if (syncPoint === undefined || syncPoint.l1BlockNumber <= head.l1BlockNumber) { + return false; + } + if ((await this.checkL1Block(syncPoint)) !== 'canonical') { + return false; + } + this.log.verbose( + `L1 head ${head.l1BlockNumber} is behind the certified syncpoint at ${syncPoint.l1BlockNumber}, which is ` + + `still canonical; keeping the message log and waiting for the provider to catch up`, + { headL1BlockNumber: head.l1BlockNumber, syncPointL1BlockNumber: syncPoint.l1BlockNumber }, + ); + return true; + } + + /** + * First L1 block ordinary ingestion must read, given the scanned cursor. + * + * The cursor is exclusive, so fetching normally resumes at the block after it. The deployment block is the + * exception: message index 0 can be emitted by a later transaction in that very block, and a cursor sitting at it + * means nothing has read it yet — that is where the archiver starts with no persisted cursor, and where the + * zero-anchor rollback rewinds to. Resuming one block later would skip index 0 permanently, since no later message + * can fill the gap and every pass would rediscover the same disagreement. Only the deployment block is re-read; + * genuine completed cursors keep exclusive semantics, and re-reading it is harmless because the store rewrites an + * unchanged message in place. + */ + private ingestionStartFor(cursor: L1BlockId): bigint { + return cursor.l1BlockNumber <= this.l1Start.l1BlockNumber ? this.l1Start.l1BlockNumber : cursor.l1BlockNumber + 1n; + } + /** * Fetches messages forward in bounded L1 block ranges and commits each batch with the scanned cursor that covers * it, except for the batch reaching the head, which is returned staged instead of stored. No intermediate batch is @@ -307,7 +391,7 @@ export class InboxMessageSynchronizer { // Logs and the batch-end block are read by number: only a head still canonical after both reads proves they // came from the captured chain, so a batch is never committed under a replacement chain's cursor. const l1Block = await this.l1BlockIdFor(end, head); - if (!(await this.isHeadStillCanonical(head))) { + if ((await this.checkL1Block(head)) !== 'canonical') { throw new CapturedHeadReplacedError(head); } await this.storeMessages(messages, { l1Block, authenticated: false }); @@ -396,11 +480,13 @@ export class InboxMessageSynchronizer { headL1BlockNumber: recovery.head.l1BlockNumber, lookups: recovery.lookups, }); - return { keep: zeroMessagePosition(), anchorL1Block: this.l1Start.l1BlockNumber + 1n }; + return { keep: zeroMessagePosition(), anchorL1Block: this.l1Start.l1BlockNumber }; } const candidate = await this.stores.messages.getL1ToL2Message(candidateIndex); if (candidate === undefined) { - throw new InboxMessagePrefixChangedError(candidateIndex + 1n, recovery.remote.rollingHash, undefined); + // The row the search expected is gone; nothing here knows what its rolling hash was, so report the position + // with no expected value rather than the Inbox's tip hash, which belongs to a different count entirely. + throw new InboxMessagePrefixChangedError(candidateIndex + 1n, undefined, undefined); } if (finalizedL1Block !== undefined && candidate.l1BlockNumber <= finalizedL1Block.l1BlockNumber) { this.log.info(`Anchoring L1 to L2 message recovery at finalized L1 block ${candidate.l1BlockNumber}`, { @@ -459,9 +545,17 @@ export class InboxMessageSynchronizer { const cursor = await this.l1BlockIdFor(maxBigint(anchor.anchorL1Block - 1n, this.l1Start.l1BlockNumber), head); // The anchor event and the cursor block were both read by number: only a head still canonical after both reads // proves they came from the captured chain, so a rollback never commits against a replacement chain. - if (!(await this.isHeadStillCanonical(head))) { - this.log.warn(`L1 head ${head.l1BlockNumber} was replaced during L1 to L2 message recovery; restarting`); - this.recovery = undefined; + const headStatus = await this.checkL1Block(head); + if (headStatus !== 'canonical') { + // A replaced head invalidates the anchor evidence, so the search starts over against the new view. An + // unreadable one proves nothing: keep the recovery and its lookup progress and retry next pass. + this.log.warn(`Could not confirm L1 head ${head.l1BlockNumber} during L1 to L2 message recovery`, { + headStatus, + ...this.getRecoveryProgress(), + }); + if (headStatus === 'replaced') { + this.recovery = undefined; + } return pending(); } this.log.warn( @@ -511,17 +605,36 @@ export class InboxMessageSynchronizer { return { l1BlockNumber, l1BlockHash: Buffer32.fromString(block.hash) }; } - private async isHeadStillCanonical(head: L1BlockId): Promise { + /** + * Whether an L1 block this pass depends on is still the one that was captured. + * + * An exception or a missing answer is `unknown`, not `replaced`: a provider that lags behind the height, is + * temporarily unreachable, or answers a pruned range cannot distinguish a reorg from its own view. Treating that + * as a replacement would delete messages and restart recovery on nothing more than an RPC failure, so callers keep + * their pending work and retry instead. Only a block that reads back with a different hash is `replaced`. + */ + private async checkL1Block(block: L1BlockId): Promise { + let remote; try { - const block = await this.publicClient.getBlock({ blockNumber: head.l1BlockNumber, includeTransactions: false }); - return Buffer32.fromString(block.hash).equals(head.l1BlockHash); + remote = await this.publicClient.getBlock({ blockNumber: block.l1BlockNumber, includeTransactions: false }); } catch (err) { - this.log.debug(`Could not read L1 block ${head.l1BlockNumber} to confirm the captured head: ${err}`); - return false; + this.log.debug(`Could not read L1 block ${block.l1BlockNumber} to confirm it is still canonical: ${err}`); + return 'unknown'; + } + if (remote?.hash === undefined || remote.hash === null) { + this.log.debug(`L1 block ${block.l1BlockNumber} was returned without a hash; canonicality is unknown`); + return 'unknown'; } + return Buffer32.fromString(remote.hash).equals(block.l1BlockHash) ? 'canonical' : 'replaced'; } } +/** + * Whether a captured L1 block is still the canonical one at its height, was positively replaced, or could not be + * read. `unknown` is deliberately not merged into `replaced`: only the latter is evidence of a chain replacement. + */ +type L1BlockStatus = 'canonical' | 'replaced' | 'unknown'; + /** The L1 head a sync pass was captured against is no longer canonical; the pass's uncommitted work is discarded. */ class CapturedHeadReplacedError extends Error { constructor(head: L1BlockId) { From ea25fe61c650754c5ec0d94f989ab8a54f15f496 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:08:17 -0300 Subject: [PATCH 02/25] fix(l1-artifacts): export the libraries the generic Rollup deployer has to link The Rollup bytecode links against EpochProofExtLib, but neither the artifact generator's contract list nor RollupArtifact.libraries.libraryCode carried it, so deployL1Contract threw before deploying. Add it, and drop ValidatorSelectionLib, which is no longer a link reference and would otherwise be deployed for nothing. A unit test now asserts the two sides of the map agree in both directions. Also passes the generated RollupAbi to the preflight simulation directly: it already merges the combined errors ABI at generation time, so merging ErrorsAbi into it again was a no-op. Original findings: UMB-SOL-01, SUP-S05, SIMPLIFY C04. Co-Authored-By: Claude Opus 5 (1M context) --- yarn-project/ethereum/src/contracts/rollup.ts | 5 ++-- .../ethereum/src/l1_artifacts.test.ts | 25 +++++++++++++++++++ yarn-project/ethereum/src/l1_artifacts.ts | 12 ++++----- 3 files changed, 33 insertions(+), 9 deletions(-) create mode 100644 yarn-project/ethereum/src/l1_artifacts.test.ts diff --git a/yarn-project/ethereum/src/contracts/rollup.ts b/yarn-project/ethereum/src/contracts/rollup.ts index 97be4a3d1961..a1fb0b79b725 100644 --- a/yarn-project/ethereum/src/contracts/rollup.ts +++ b/yarn-project/ethereum/src/contracts/rollup.ts @@ -7,7 +7,6 @@ import type { ViemSignature } from '@aztec/foundation/eth-signature'; import { createLogger } from '@aztec/foundation/log'; import { makeBackoff, retry } from '@aztec/foundation/retry'; import { getErrorCause } from '@aztec/foundation/types'; -import { ErrorsAbi } from '@aztec/l1-artifacts/ErrorsAbi'; import { EscapeHatchAbi } from '@aztec/l1-artifacts/EscapeHatchAbi'; import { RollupAbi } from '@aztec/l1-artifacts/RollupAbi'; import { RollupStorage } from '@aztec/l1-artifacts/RollupStorage'; @@ -37,7 +36,7 @@ import type { L1ContractAddresses } from '../l1_contract_addresses.js'; import type { L1ReaderConfig } from '../l1_reader.js'; import type { L1TxRequest, L1TxUtils, ReadOnlyL1TxUtils } from '../l1_tx_utils/index.js'; import type { ViemClient } from '../types.js'; -import { formatViemError, mergeAbis } from '../utils.js'; +import { formatViemError } from '../utils.js'; import { GSEContract } from './gse.js'; import type { L1EventLog } from './log.js'; import { SlasherContract } from './slasher_contract.js'; @@ -925,7 +924,7 @@ export class RollupContract { }, { time: opts.time }, opts.stateOverrides ?? [], - mergeAbis([RollupAbi, ErrorsAbi]), + RollupAbi, ); return decodeFunctionResult({ abi: RollupAbi, functionName: 'validateCheckpointHeaderAndInbox', data: result }); } diff --git a/yarn-project/ethereum/src/l1_artifacts.test.ts b/yarn-project/ethereum/src/l1_artifacts.test.ts new file mode 100644 index 000000000000..26de03ce230b --- /dev/null +++ b/yarn-project/ethereum/src/l1_artifacts.test.ts @@ -0,0 +1,25 @@ +import { RollupArtifact } from './l1_artifacts.js'; + +describe('l1 artifacts', () => { + describe('RollupArtifact libraries', () => { + const { linkReferences, libraryCode } = RollupArtifact.libraries; + const linkedNames = Object.values(linkReferences).flatMap(refs => Object.keys(refs)); + + // The generic deployer deploys exactly the libraries in libraryCode and links exactly the names in + // linkReferences, so a name in one but not the other is either a deployment failure or a wasted deployment. + it('supplies code for every link reference the compiler emitted', () => { + expect(linkedNames.filter(name => !(name in libraryCode))).toEqual([]); + }); + + it('carries no library the rollup bytecode does not link against', () => { + expect(Object.keys(libraryCode).filter(name => !linkedNames.includes(name))).toEqual([]); + }); + + it('exports nonempty bytecode for every library', () => { + for (const [name, lib] of Object.entries(libraryCode)) { + expect(`${name}:${lib.contractBytecode.slice(0, 2)}`).toEqual(`${name}:0x`); + expect(lib.contractBytecode.length).toBeGreaterThan(2); + } + }); + }); +}); diff --git a/yarn-project/ethereum/src/l1_artifacts.ts b/yarn-project/ethereum/src/l1_artifacts.ts index a8273e56b8fd..e9aa7162e4d7 100644 --- a/yarn-project/ethereum/src/l1_artifacts.ts +++ b/yarn-project/ethereum/src/l1_artifacts.ts @@ -3,6 +3,8 @@ import { CoinIssuerBytecode, DateGatedRelayerAbi, DateGatedRelayerBytecode, + EpochProofExtLibAbi, + EpochProofExtLibBytecode, FeeAssetHandlerAbi, FeeAssetHandlerBytecode, FeeJuicePortalAbi, @@ -50,8 +52,6 @@ import { TestERC20Bytecode, ValidatorOperationsExtLibAbi, ValidatorOperationsExtLibBytecode, - ValidatorSelectionLibAbi, - ValidatorSelectionLibBytecode, } from '@aztec/l1-artifacts'; import type { Hex } from 'viem'; @@ -81,10 +81,10 @@ export const RollupArtifact = { libraries: { linkReferences: RollupLinkReferences, libraryCode: { - ValidatorSelectionLib: { - name: 'ValidatorSelectionLib', - contractAbi: ValidatorSelectionLibAbi, - contractBytecode: ValidatorSelectionLibBytecode as Hex, + EpochProofExtLib: { + name: 'EpochProofExtLib', + contractAbi: EpochProofExtLibAbi, + contractBytecode: EpochProofExtLibBytecode as Hex, }, RollupOperationsExtLib: { name: 'RollupOperationsExtLib', From fd507223fee8b0fc0f99f42fdbbb4f1fa240a179 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:08:17 -0300 Subject: [PATCH 03/25] fix(stdlib): measure checkpoint proposal size from its serialization getSize counted eight bytes for feeAssetPriceModifier while toBuffer writes the 32 serializeSignedBigInt produces, undercounting every proposal by 24 bytes. Measure the buffer instead, which cannot drift as optional fields are added, and cover the optional proposal shapes. Original finding: UMB-NODE-I01. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/p2p/checkpoint_proposal.test.ts | 28 ++++++++++++++++++- .../stdlib/src/p2p/checkpoint_proposal.ts | 27 ++++-------------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts index 98c4d6a26bc4..81df2510bee9 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts @@ -7,7 +7,7 @@ import { bufferToHex, hexToBuffer } from '@aztec/foundation/string'; import { InboxMessagePrefixRef } from '../messaging/inbox_message_prefix_ref.js'; import { CheckpointHeader } from '../rollup/checkpoint_header.js'; -import { makeCheckpointProposal } from '../tests/mocks.js'; +import { makeCheckpointProposal, mockTx } from '../tests/mocks.js'; import { BlockHeader } from '../tx/block_header.js'; import { TxHash } from '../tx/tx_hash.js'; import { CheckpointProposal } from './checkpoint_proposal.js'; @@ -34,6 +34,32 @@ const makeLegacyFixtureCheckpointProposal = () => ); describe('CheckpointProposal serialization / deserialization', () => { + it.each([ + ['no lastBlock', () => makeCheckpointProposal({})], + ['a nonzero fee asset price modifier', () => makeCheckpointProposal({ feeAssetPriceModifier: -1234n })], + ['a lastBlock', () => makeCheckpointProposal({ lastBlock: {} })], + [ + 'a lastBlock carrying its txs', + async () => { + const tx = await mockTx(1); + return makeCheckpointProposal({ lastBlock: { txHashes: [await tx.getTxHash()], txs: [tx] } }); + }, + ], + [ + 'a lastBlock with an inbox prefix reference', + () => { + const checkpointHeader = CheckpointHeader.random(); + return makeCheckpointProposal({ + checkpointHeader, + lastBlock: { inboxPrefixRef: new InboxMessagePrefixRef(checkpointHeader.inboxRollingHash) }, + }); + }, + ], + ])('reports the actual serialized size with %s', async (_name, build) => { + const proposal = await build(); + expect(proposal.getSize()).toEqual(proposal.toBuffer().length); + }); + it('round-trips with a lastBlock', async () => { const proposal = await makeCheckpointProposal({ lastBlock: {} }); const deserialized = CheckpointProposal.fromBuffer(proposal.toBuffer()); diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts index 0459de3e84b2..8ca8d697c0a6 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts @@ -365,29 +365,12 @@ export class CheckpointProposal extends Gossipable implements Signable { return new CheckpointProposal(checkpointHeader, archive, feeAssetPriceModifier, signature, signatureContext); } + /** + * Size in bytes of the serialized proposal. Measured from the serialization itself so it cannot drift from + * `toBuffer` as optional fields are added or their widths change. + */ getSize(): number { - let size = - this.checkpointHeader.toBuffer().length + - this.archive.size + - this.signature.getSize() + - 8 /* feeAssetPriceModifier */ + - 4 /* chainId */ + - 20 /* rollupAddress */ + - 4; /* hasLastBlock flag */ - - if (this.lastBlock) { - size += - this.lastBlock.blockHeader.getSize() + - 4 /* indexWithinCheckpoint */ + - this.lastBlock.signature.getSize() + - 4 /* txHashes.length */ + - this.lastBlock.txHashes.length * TxHash.SIZE + - 4 /* hasSignedTxs flag */ + - (this.lastBlock.signedTxs ? this.lastBlock.signedTxs.getSize() : 0) + - (this.lastBlock.inboxPrefixRef ? 4 /* hasInboxPrefixRef flag */ + this.lastBlock.inboxPrefixRef.getSize() : 0); - } - - return size; + return this.toBuffer().length; } static empty(): CheckpointProposal { From af8ccfc50587460efe04381be73f95c0e2cb315c Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:08:18 -0300 Subject: [PATCH 04/25] fix(sequencer): reject a production config that cannot clear a mandatory Inbox backlog validateNetworkConsensusConfig applies MIN_BLOCKS_FOR_INBOX_CATCHUP to a generated profile's configured maxBlocksPerCheckpoint, but a running proposer is bounded by the smaller of that cap and what its own slot timings derive. Timings that shrink the derived count below the floor leave a proposer whose publications L1 always rejects, losing every one of its slots. Check the effective count at startup and before committing a config update; a rejected update leaves the previous config and timetable in place. Fast local/e2e profiles deliberately run one or two blocks per slot against an Inbox nobody floods, so they warn instead of failing. isFastLocalProfile now names that condition once for both callers. Original finding: SUP-02. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/sequencer/sequencer.test.ts | 61 ++++++++++++++++++- .../src/sequencer/sequencer.ts | 45 +++++++++++++- yarn-project/stdlib/src/timetable/budgets.ts | 11 +++- 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 74883fa47efe..34df55df217d 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -43,7 +43,7 @@ import { type WorldStateSynchronizer, type WorldStateSynchronizerStatus, } from '@aztec/stdlib/interfaces/server'; -import type { L1ToL2MessageSource } from '@aztec/stdlib/messaging'; +import { type L1ToL2MessageSource, MIN_BLOCKS_FOR_INBOX_CATCHUP } from '@aztec/stdlib/messaging'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; import { AppendOnlyTreeSnapshot } from '@aztec/stdlib/trees'; import { BlockHeader, GlobalVariables, type Tx } from '@aztec/stdlib/tx'; @@ -93,6 +93,7 @@ describe('sequencer', () => { >; let sequencer: TestSequencer; + let config: SequencerConfig & Pick; const slotDuration = 8; const ethereumSlotDuration = 4; @@ -390,7 +391,7 @@ describe('sequencer', () => { dateProvider = new TestDateProvider(); signatureContext = { chainId: chainId.toNumber(), rollupAddress: EthAddress.random() }; - const config: SequencerConfig & Pick = { + config = { maxTxsPerBlock: 4, l1ChainId: signatureContext.chainId, // With aztecSlotDuration=8 and ethereumSlotDuration=4 (fast profile), a 2s block duration derives @@ -419,6 +420,62 @@ describe('sequencer', () => { sequencer.updateConfig(config); }); + describe('Inbox catch-up capacity guard', () => { + // Production profile: 12s ethereum slots keep the conservative budgets, so maxBlocks is driven purely by the + // slot and block durations. floor((S - 1 - D - 2*2 - 1) / D) with S=36 gives 8 blocks at D=3 and 2 at D=10. + const productionConstants = () => ({ ...l1Constants, slotDuration: 36, ethereumSlotDuration: 12 }); + + const buildSequencer = (overrides: Partial, constants = productionConstants()) => { + const sequencerConfig = { ...config, blockDurationMs: 3000, ...overrides }; + return new TestSequencer( + publisherFactory, + validatorClient, + globalVariableBuilder, + p2p, + worldState, + slasherClient, + l2BlockSource, + l1ToL2MessageSource, + checkpointsBuilder as unknown as FullNodeCheckpointsBuilder, + constants, + dateProvider, + epochCache, + rollupContract, + inboxContract, + sequencerConfig, + ); + }; + + it.each([1, 2, 3])('rejects a configured cap of %i block(s) per checkpoint', maxBlocksPerCheckpoint => { + expect(() => buildSequencer({ maxBlocksPerCheckpoint })).toThrow(/streaming-Inbox backlog/); + }); + + it('accepts a configured cap at the floor', () => { + expect(() => buildSequencer({ maxBlocksPerCheckpoint: MIN_BLOCKS_FOR_INBOX_CATCHUP })).not.toThrow(); + }); + + it('rejects timings that derive fewer blocks than the floor even when the configured cap is above it', () => { + // D=10 leaves floor((36 - 1 - 10 - 4 - 1) / 10) = 2 sub-slots, under a generous configured cap. + expect(() => buildSequencer({ maxBlocksPerCheckpoint: 8, blockDurationMs: 10_000 })).toThrow( + /streaming-Inbox backlog/, + ); + }); + + it('warns rather than rejects on a fast local profile', () => { + expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1, blockDurationMs: 2000 }, l1Constants)).not.toThrow(); + }); + + it('leaves the committed config and timetable intact when an update is rejected', () => { + const sequencer = buildSequencer({ maxBlocksPerCheckpoint: 8 }); + const before = sequencer.getTimeTable(); + + expect(() => sequencer.updateConfig({ blockDurationMs: 10_000 })).toThrow(/streaming-Inbox backlog/); + + expect(sequencer.getTimeTable()).toBe(before); + expect(sequencer.getTimeTable().blockDuration).toEqual(3); + }); + }); + describe('perBlockAllocationMultiplier guard', () => { it('rejects a multiplier below the network minimum', () => { expect(() => sequencer.updateConfig({ perBlockAllocationMultiplier: 1.0 })).toThrow( diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 07f3f405917c..9dfa28d5e6cd 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -36,10 +36,10 @@ import { SequencerConfigSchema, type WorldStateSynchronizer, } from '@aztec/stdlib/interfaces/server'; -import type { L1ToL2MessageSource } from '@aztec/stdlib/messaging'; +import { type L1ToL2MessageSource, MIN_BLOCKS_FOR_INBOX_CATCHUP } from '@aztec/stdlib/messaging'; import type { CoordinationSignatureContext } from '@aztec/stdlib/p2p'; import { pickFromSchema } from '@aztec/stdlib/schemas'; -import { ProposerTimetable, buildProposerTimetable } from '@aztec/stdlib/timetable'; +import { ProposerTimetable, buildProposerTimetable, isFastLocalProfile } from '@aztec/stdlib/timetable'; import { Attributes, type TelemetryClient, type Tracer, getTelemetryClient, trackSpan } from '@aztec/telemetry-client'; import { FullNodeCheckpointsBuilder, NodeKeystoreAdapter, type ValidatorClient } from '@aztec/validator-client'; @@ -215,11 +215,52 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter= MIN_BLOCKS_FOR_INBOX_CATCHUP) { + return; + } + + const detail = + `this sequencer can build at most ${effectiveMaxBlocks} block(s) per checkpoint ` + + `(MAX_BLOCKS_PER_CHECKPOINT ${config.maxBlocksPerCheckpoint}, ${timetableMaxBlocks} derived from slot ` + + `timings), below the ${MIN_BLOCKS_FOR_INBOX_CATCHUP} needed to clear a mandatory streaming-Inbox backlog`; + + if (isFastLocalProfile(this.l1Constants.ethereumSlotDuration)) { + this.log.warn(`Inbox catch-up capacity below the floor: ${detail}.`, { + effectiveMaxBlocks, + maxBlocksPerCheckpoint: config.maxBlocksPerCheckpoint, + timetableMaxBlocks, + minBlocksForInboxCatchup: MIN_BLOCKS_FOR_INBOX_CATCHUP, + }); + return; + } + + throw new Error( + `Rejecting sequencer configuration: ${detail}. Raise MAX_BLOCKS_PER_CHECKPOINT, lower the block duration, ` + + `or raise the slot duration.`, + ); + } + /** * Checks this node's configured per-block allocation against the network admission limit. A node * advertises and admits txs up to the limit derived from the network-minimum multipliers (see diff --git a/yarn-project/stdlib/src/timetable/budgets.ts b/yarn-project/stdlib/src/timetable/budgets.ts index c455bd800b65..bcab8cb6cff6 100644 --- a/yarn-project/stdlib/src/timetable/budgets.ts +++ b/yarn-project/stdlib/src/timetable/budgets.ts @@ -43,6 +43,14 @@ export const FAST_PROFILE_CHECKPOINT_PROPOSAL_PREPARE_TIME = 0.5; /** Fast-profile minimum block-building budget (seconds). See {@link FAST_PROFILE_P2P_PROPAGATION_TIME}. */ export const FAST_PROFILE_MIN_BLOCK_DURATION = 1; +/** + * Whether a network's Ethereum slot duration marks it as a fast local/e2e profile with mocked p2p rather than a + * production deployment. See {@link FAST_PROFILE_ETHEREUM_SLOT_DURATION}. + */ +export function isFastLocalProfile(ethereumSlotDuration: number): boolean { + return ethereumSlotDuration < FAST_PROFILE_ETHEREUM_SLOT_DURATION; +} + /** Resolved operational timing budgets used to size the proposer build window. */ export type ResolvedTimingBudgets = { minBlockDuration: number; @@ -70,8 +78,7 @@ export function getDefaultCheckpointProposalSyncGrace(blockDuration: number): nu export function resolveTimingBudgets(ethereumSlotDuration: number, opts: ResolvedTimingBudgets): ResolvedTimingBudgets { const { minBlockDuration, p2pPropagationTime, checkpointProposalPrepareTime, checkpointProposalInitTime } = opts; - const isFastProfile = ethereumSlotDuration < FAST_PROFILE_ETHEREUM_SLOT_DURATION; - if (!isFastProfile) { + if (!isFastLocalProfile(ethereumSlotDuration)) { return { minBlockDuration, p2pPropagationTime, checkpointProposalPrepareTime, checkpointProposalInitTime }; } From 00a12357f8b638dd69b7c7deefd1f8c597664c5b Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:39:01 -0300 Subject: [PATCH 05/25] fix(sequencer): budget the proposal send against the receive window, not the attestation deadline The pre-gossip preflight was bounded by the attestation deadline, a full ethereum slot plus a block duration after peers stop accepting proposals for the slot. A verdict arriving in that gap led the proposer to sign and gossip a proposal every peer refuses on ingress. Derive the send budget from the consensus receive deadline instead, reserving one propagation budget, and reuse that as the forced-tail hard stop rather than repeating the formula. Re-check it after signing (which may be remote) and after the queued archiver insertion, immediately before broadcast, so neither can carry the job past the point where a send is still useful; a signature already produced keeps its duty record, and the local optimistic tip stays advanced. Send timeouts report as proposal_send_timeout, separately from an invalid header. The job tests now start at the target slot's build frame opening rather than the target slot start, which is when a proposer actually takes its turn; anchoring at the slot start put every job past the deadlines the timetable derives. Original finding: NODE-01. Co-Authored-By: Claude Opus 5 (1M context) --- .../sequencer/checkpoint_proposal_job.test.ts | 81 +++++++++++++++++-- .../src/sequencer/checkpoint_proposal_job.ts | 62 ++++++++++++-- .../src/timetable/proposer_timetable.ts | 13 +++ 3 files changed, 143 insertions(+), 13 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index 1eb680abdab0..a20672e2c17d 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -176,9 +176,11 @@ describe('CheckpointProposalJob', () => { // ManualDateProvider freezes time (it does not track real wall-clock progression), so timing-sensitive // assertions on dateProvider.now() are deterministic regardless of how long the test takes to execute. dateProvider = new ManualDateProvider(); - // Set time to be at the start of the slot (slot 1 starts at l1GenesisTime + slotDuration - ethereumSlotDuration) - const slotStartTime = Number(l1GenesisTime) + newSlotNumber * slotDuration - ethereumSlotDuration; - dateProvider.setTime(slotStartTime * 1000); // Convert to milliseconds + // Start at the target slot's build frame opening (target_slot_start - S - E), which is when a proposer actually + // begins its turn. Anchoring at the target slot start instead would put every job past the deadlines the + // timetable derives from the build frame, including the proposal send deadline. + const buildFrameStart = Number(l1GenesisTime) + (newSlotNumber - 1) * slotDuration - ethereumSlotDuration; + dateProvider.setTime(buildFrameStart * 1000); // Convert to milliseconds epochCache = mockDeep(); epochCache.getCommittee.mockResolvedValue({ @@ -396,6 +398,11 @@ describe('CheckpointProposalJob', () => { deadline: undefined, isLastBlock: false, }); + // Freezes the clock past every tx-waiting deadline but still inside the proposal send budget. ManualDateProvider + // does not advance, so a job that genuinely waits for txs would hang; tests whose subject is the give-up path + // rather than the waiting itself start here instead. + const setTimePastTxWaits = () => + dateProvider.setTime((job.getTimetable().getCheckpointProposalSendDeadline(SlotNumber(newSlotNumber)) - 1) * 1000); const makeSingleBlockTimetable = () => makeProposerTimetable({ l1Constants, @@ -483,6 +490,7 @@ describe('CheckpointProposalJob', () => { job.updateConfig({ minTxsPerBlock: 2 }); + setTimePastTxWaits(); const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeUndefined(); @@ -1295,6 +1303,7 @@ describe('CheckpointProposalJob', () => { const waitSpy = jest.spyOn(job, 'waitUntilNextSubslot'); job.updateConfig({ minTxsPerBlock: 0 }); + setTimePastTxWaits(); const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeDefined(); @@ -1353,6 +1362,7 @@ describe('CheckpointProposalJob', () => { const waitSpy = jest.spyOn(job, 'waitUntilNextSubslot'); job.updateConfig({ minTxsPerBlock: 5, buildCheckpointIfEmpty: false }); + setTimePastTxWaits(); const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeUndefined(); @@ -1592,6 +1602,7 @@ describe('CheckpointProposalJob', () => { validatorClient.collectAttestations.mockResolvedValue(getAttestations(lastBlock)); job.updateConfig({ minTxsPerBlock: 1, buildCheckpointIfEmpty: false }); + setTimePastTxWaits(); const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeDefined(); @@ -2042,18 +2053,23 @@ describe('CheckpointProposalJob', () => { // point at which its signature or its send could still land. describe('preflight deadlines', () => { const attestationDeadlineMs = () => job.getTimetable().getAttestationDeadline(SlotNumber(newSlotNumber)) * 1000; + const sendDeadlineMs = () => + job.getTimetable().getCheckpointProposalSendDeadline(SlotNumber(newSlotNumber)) * 1000; const l1PublishDeadlineMs = () => (Number(l1Constants.l1GenesisTime) + newSlotNumber * slotDuration + slotDuration - ethereumSlotDuration) * 1000; - it('does not sign the checkpoint when the pre-gossip preflight resolves after the attestation deadline', async () => { + it('does not sign the checkpoint when the pre-gossip preflight resolves after the send deadline', async () => { mockSubslots(1); streamingInbox.set(leaves(2)); + // A verdict that lands after peers stop accepting proposals but well before the attestation cutoff: the + // two are a whole ethereum slot plus a block duration apart, and only the earlier one bounds the send. publisher.validateCheckpointHeaderAndInbox.mockImplementation(() => { - dateProvider.setTime(attestationDeadlineMs() + 1_000); + dateProvider.setTime(sendDeadlineMs() + 1_000); return Promise.resolve(0n); }); await setupMultipleBlocks(1, [1]); + expect(sendDeadlineMs()).toBeLessThan(attestationDeadlineMs()); const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeUndefined(); @@ -2062,15 +2078,15 @@ describe('CheckpointProposalJob', () => { expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('header_validation_timeout'); }); - it('abandons a pre-gossip preflight that does not answer within the remaining attestation window', async () => { + it('abandons a pre-gossip preflight that does not answer within the remaining send window', async () => { mockSubslots(1); streamingInbox.set(leaves(2)); publisher.validateCheckpointHeaderAndInbox.mockImplementation(() => new Promise(() => {})); await setupMultipleBlocks(1, [1]); - // Once the block is built there are 200ms left to gossip a proposal validators could still attest to. + // Once the block is built there are 200ms left to gossip a proposal peers would still accept. const completeCheckpoint = checkpointBuilder.completeCheckpoint.bind(checkpointBuilder); jest.spyOn(checkpointBuilder, 'completeCheckpoint').mockImplementation(() => { - dateProvider.setTime(attestationDeadlineMs() - 200); + dateProvider.setTime(sendDeadlineMs() - 200); return completeCheckpoint(); }); @@ -2081,6 +2097,55 @@ describe('CheckpointProposalJob', () => { expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('header_validation_timeout'); }); + it('does not gossip when a slow signer returns after the send deadline', async () => { + mockSubslots(1); + streamingInbox.set(leaves(2)); + const createProposal = validatorClient.createCheckpointProposal.getMockImplementation()!; + validatorClient.createCheckpointProposal.mockImplementation(((...args: unknown[]) => { + dateProvider.setTime(sendDeadlineMs() + 1_000); + return (createProposal as (...a: unknown[]) => unknown)(...args); + }) as any); + await setupMultipleBlocks(1, [1]); + + const checkpoint = await job.executeAndAwait(); + + expect(checkpoint).toBeUndefined(); + // The signature was produced and its duty record stands; only the send is abandoned. + expect(validatorClient.createCheckpointProposal).toHaveBeenCalledTimes(1); + expect(p2p.broadcastCheckpointProposal).not.toHaveBeenCalled(); + expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('proposal_send_timeout'); + }); + + it('does not gossip when the queued archiver insertion returns after the send deadline', async () => { + mockSubslots(1); + streamingInbox.set(leaves(2)); + blockSink.addProposedCheckpoint.mockImplementation(() => { + dateProvider.setTime(sendDeadlineMs() + 1_000); + return Promise.resolve(); + }); + await setupMultipleBlocks(1, [1]); + + const checkpoint = await job.executeAndAwait(); + + expect(checkpoint).toBeUndefined(); + expect(validatorClient.createCheckpointProposal).toHaveBeenCalledTimes(1); + expect(p2p.broadcastCheckpointProposal).not.toHaveBeenCalled(); + expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('proposal_send_timeout'); + }); + + it('broadcasts once when preflight, signing and insertion all fit inside the send budget', async () => { + mockSubslots(1); + streamingInbox.set(leaves(2)); + const { lastBlock } = await setupMultipleBlocks(1, [1]); + validatorClient.collectAttestations.mockResolvedValue(getAttestations(lastBlock)); + + const checkpoint = await job.executeAndAwait(); + + expect(checkpoint).toBeDefined(); + expect(p2p.broadcastCheckpointProposal).toHaveBeenCalledTimes(1); + expect(metrics.recordCheckpointProposalFailed).not.toHaveBeenCalledWith('proposal_send_timeout'); + }); + it('does not sign the checkpoint when the job is interrupted while the pre-gossip preflight runs', async () => { mockSubslots(1); streamingInbox.set(leaves(2)); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index 929339434464..f603c03be386 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -542,6 +542,44 @@ export class CheckpointProposalJob implements Traceable { return new Date(this.timetable.getAttestationDeadline(this.targetSlot) * 1000); } + /** + * The latest moment at which this proposal may still be sent: the consensus receive deadline less one propagation + * budget. Everything on the path to `broadcastCheckpointProposal` — the pre-gossip preflight, signing, the local + * archiver insertion — is budgeted against it, because a proposal that leaves this node later than this is refused + * on ingress by every peer. This is deliberately tighter than {@link getAttestationDeadline}, which bounds + * attestation collection, and than {@link getL1PublishDeadline}, which bounds the send. + */ + private getProposalSendDeadline(): Date { + return new Date(this.timetable.getCheckpointProposalSendDeadline(this.targetSlot) * 1000); + } + + /** + * Whether there is still time to gossip this slot's proposal. Signing may be remote (HA) and the archiver insertion + * resolves through a queue, so neither is guaranteed to be quick; a timer wrapped around the preflight alone does + * not cover them. Callers re-check before each further side effect, and abandon the send rather than starting one + * that peers will refuse. An already-produced signature and its duty record are left untouched. + */ + private reportSendBudgetExpired(stage: string, deadline: Date): void { + const context = { + slot: this.targetSlot, + checkpointNumber: this.checkpointNumber, + stage, + deadline: deadline.toISOString(), + reason: 'proposal_send_timeout', + }; + this.log.warn( + `Checkpoint proposal for slot ${this.targetSlot} missed the proposal send deadline during ${stage}; ` + + `not gossiping it`, + context, + ); + this.metrics.recordCheckpointProposalFailed('proposal_send_timeout'); + this.eventEmitter.emit('header-validation-failed', { + slot: this.targetSlot, + checkpointNumber: this.checkpointNumber, + reason: `proposal send deadline ${deadline.toISOString()} passed during ${stage}`, + }); + } + /** * Latest L1 block the propose can still land in for the target slot: the last Ethereum block inside the target slot * (`target_slot_start + S - E`). This is one ethereum slot later than `attestation_deadline`, which bounds when @@ -1079,15 +1117,16 @@ export class CheckpointProposalJob implements Traceable { // rules. If this fails the slot is aborted before any gossip work. The pipelined parent is supplied through // the simulation overrides, so this verdict is conditional on that parent landing; the pre-publication // preflight repeats it once the parent has landed, keeping only the assumptions still outstanding then. - // The simulation is bounded by the attestation deadline: a verdict that arrives once no validator can attest - // any more must not lead to signing. + // The simulation is bounded by the proposal send deadline, not the attestation deadline: a verdict arriving + // once peers have stopped accepting proposals for this slot must not lead to signing and gossiping one. + const sendDeadline = this.getProposalSendDeadline(); let bucketHint: bigint; try { bucketHint = await this.preflightWithinDeadline( checkpoint.header, streamingState, this.checkpointSimulationOverridesPlan, - this.getAttestationDeadline(), + sendDeadline, ); } catch (err) { if (err instanceof SequencerInterruptedError) { @@ -1121,6 +1160,13 @@ export class CheckpointProposalJob implements Traceable { checkpointProposalOptions, ); + // Signing may be remote (HA), so the preflight's own budget check does not cover it. Stop here rather than + // start the archiver insertion for a proposal no peer will accept; the signature and its duty record stand. + if (this.dateProvider.now() >= sendDeadline.getTime()) { + this.reportSendBudgetExpired('signing', sendDeadline); + return undefined; + } + // Advance our own optimistic proposed-checkpoint tip locally before gossiping. Gossipsub // doesn't echo our own messages back, so this is how the proposer makes its own proposed // checkpoint visible for pipelining the next slot. Built from local checkpoint data — never @@ -1128,6 +1174,13 @@ export class CheckpointProposalJob implements Traceable { // Fail closed: if this throws, the outer catch aborts the slot before gossiping. await this.syncProposedCheckpointToArchiver(checkpoint, blocksInCheckpoint.length, feeAssetPriceModifier); + // The insertion resolves through the archiver's queue, so re-check immediately before the broadcast itself. + // The local tip stays advanced: it is this node's own optimistic state, not something peers acted on. + if (this.dateProvider.now() >= sendDeadline.getTime()) { + this.reportSendBudgetExpired('archiver insertion', sendDeadline); + return undefined; + } + const blockProposedAt = this.dateProvider.now(); if (this.config.skipBroadcastCheckpointProposal) { // Test-only: suppress the CheckpointProposal so peers never see a proposed checkpoint for @@ -1466,8 +1519,7 @@ export class CheckpointProposalJob implements Traceable { if (this.dateProvider.now() / 1000 < lastBlockBuildTime) { return { deadline: new Date(lastBlockBuildTime * 1000), pastLastBlockBuildTime: false }; } - const hardStop = - this.timetable.getCheckpointProposalReceiveDeadline(this.targetSlot) - this.timetable.p2pPropagationTime; + const hardStop = this.timetable.getCheckpointProposalSendDeadline(this.targetSlot); return { deadline: new Date(hardStop * 1000), pastLastBlockBuildTime: true }; } diff --git a/yarn-project/stdlib/src/timetable/proposer_timetable.ts b/yarn-project/stdlib/src/timetable/proposer_timetable.ts index 42f01fdd3672..3555e99837be 100644 --- a/yarn-project/stdlib/src/timetable/proposer_timetable.ts +++ b/yarn-project/stdlib/src/timetable/proposer_timetable.ts @@ -166,6 +166,19 @@ export class ProposerTimetable extends ConsensusTimetable { return this.maxBlocksPerCheckpoint; } + /** + * Latest moment at which the proposer may still start work that ends in gossiping a checkpoint proposal: + * {@link getCheckpointProposalReceiveDeadline} less one propagation budget, so a proposal sent at it still reaches + * peers before they stop accepting proposals for the slot. + * + * This is not {@link getAttestationDeadline}, which is a full ethereum slot plus a block duration later and bounds + * when attestations must exist, not when the proposal must have been sent. Work budgeted against the attestation + * deadline can finish after peers have already refused the proposal. + */ + public getCheckpointProposalSendDeadline(slot: SlotNumber): number { + return this.getCheckpointProposalReceiveDeadline(slot) - this.p2pPropagationTime; + } + /** * Selects the next block sub-slot to build for the target slot given the current wall-clock time. * From b881a5dd1ecb8579d762c22943aee11ae64abb78 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 14:54:08 -0300 Subject: [PATCH 06/25] fix(validator): bound checkpoint validation and stop retries nobody reads retryUntil only consults its deadline once an attempt returns, so an attempt that never settles never reaches it. The bound now lives where every entry path passes through it: the all-nodes checkpoint callback, which p2p awaits before the validator's own and which is the only one a non-validator node runs. One DutyBudget per slot races the whole callback, is threaded into the cached-verdict re-check and the readiness waits, and can be stopped early; an already-expired budget still allows the single attempt callers rely on, bounded by a grace so even that cannot hang. Attestation signing is gated in the one place attestations are produced, covering the cached-verdict path. Nothing is signed after the slot's attestation deadline, and a remote signer returning past it has its output discarded rather than added to the pool or gossiped; the signing-protection record it produced still stands. The proposer's own attestations run inside its publish budget and pass no deadline, so they are unaffected. Tx collection and the block's bundle read start together. A catch handler on the loser silenced its rejection but did not stop it: once collection fails, the bundle's retries are now cancelled instead of forcing archiver syncs for the rest of the slot. The shared archiver run itself is never cancelled. Original findings: NODE-03, SUP-07. Co-Authored-By: Claude Opus 5 (1M context) --- .../validator-client/src/duty_budget.ts | 89 +++++++ .../src/proposal_handler.test.ts | 67 +++++ .../validator-client/src/proposal_handler.ts | 231 ++++++++++++------ .../validator-client/src/validator.test.ts | 45 ++++ .../validator-client/src/validator.ts | 37 ++- 5 files changed, 388 insertions(+), 81 deletions(-) create mode 100644 yarn-project/validator-client/src/duty_budget.ts diff --git a/yarn-project/validator-client/src/duty_budget.ts b/yarn-project/validator-client/src/duty_budget.ts new file mode 100644 index 000000000000..085a090694a8 --- /dev/null +++ b/yarn-project/validator-client/src/duty_budget.ts @@ -0,0 +1,89 @@ +import type { DateProvider } from '@aztec/foundation/timer'; +import { execWithSignal } from '@aztec/foundation/timer'; + +/** + * How long a run gets when the budget has already run out. The duty still makes the single attempt its callers + * rely on, but bounded, so an unresponsive read cannot hold an expired duty open. Long enough for a local store + * read, short enough that nothing waits on it. + */ +const EXPIRED_BUDGET_GRACE_MS = 1_000; + +/** Thrown when a duty's absolute budget runs out while work it was awaiting is still outstanding. */ +export class DutyBudgetExpiredError extends Error { + constructor( + public readonly what: string, + public readonly deadline: Date, + ) { + super(`Duty budget for ${what} ran out at ${deadline.toISOString()}`); + this.name = 'DutyBudgetExpiredError'; + } +} + +/** + * The one absolute budget a slot's duty runs under, shared by every stage instead of each read starting a fresh + * full timeout of its own. + * + * Two separate problems need solving and a timeout alone solves neither on its own. A read that never settles has + * to stop blocking the duty, which needs a race; and racing does not stop the losing continuation, because + * JavaScript cannot be killed, so anything that continuation would start afterwards has to consult the budget + * first. {@link run} covers the race and hands the attempt the signal; {@link expired} covers everything a caller + * is about to begin. + * + * The budget can also be stopped early, for work whose sibling has already failed: retries that no caller will + * read any more are cancelled rather than left forcing syncs for the rest of the slot. + */ +export class DutyBudget { + private readonly controller = new AbortController(); + + constructor( + public readonly deadline: Date, + private readonly dateProvider: DateProvider, + ) {} + + /** Milliseconds left before the budget runs out; zero once it has, or once the duty was stopped. */ + public remainingMs(): number { + if (this.controller.signal.aborted) { + return 0; + } + return Math.max(0, this.deadline.getTime() - this.dateProvider.now()); + } + + /** Whether the budget is gone, because the deadline passed or because the duty was stopped. */ + public expired(): boolean { + return this.remainingMs() === 0; + } + + /** The signal callees should pass on and check; aborted once the budget is gone. */ + public get signal(): AbortSignal { + return this.controller.signal; + } + + /** + * Stops the duty now. Work already in flight is not killed — it settles into a caller that has moved on — but + * every stage that checks the budget stops starting more. + */ + public stop(reason: string): void { + if (!this.controller.signal.aborted) { + this.controller.abort(new DutyBudgetExpiredError(reason, this.deadline)); + } + } + + /** + * Runs `fn` bounded by the budget, throwing {@link DutyBudgetExpiredError} rather than waiting on a read that + * never settles. The attempt itself keeps running, so `fn` should honour the signal it is handed and callers + * must still check {@link expired} before starting anything further. + * + * A budget that has already run out still gets {@link EXPIRED_BUDGET_GRACE_MS}: a duty past its deadline is + * expected to make the one attempt its callers rely on — a proposal whose blocks are already local validates + * without waiting for anything — and the point here is that even that attempt cannot hang. A duty that was + * explicitly stopped gets no grace: its result has no reader left. + */ + public async run(what: string, fn: (signal: AbortSignal) => Promise): Promise { + if (this.controller.signal.aborted) { + throw new DutyBudgetExpiredError(what, this.deadline); + } + const remainingMs = this.remainingMs() || EXPIRED_BUDGET_GRACE_MS; + const signal = AbortSignal.any([this.controller.signal, AbortSignal.timeout(remainingMs)]); + return await execWithSignal(fn, signal, () => new DutyBudgetExpiredError(what, this.deadline)); + } +} diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 424b5a337798..8bbba2a072c9 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -6,6 +6,7 @@ import { MAX_FEE_ASSET_PRICE_MODIFIER_BPS } from '@aztec/ethereum/contracts'; import { BlockNumber, CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; import { Fr } from '@aztec/foundation/curves/bn254'; +import { sleep } from '@aztec/foundation/sleep'; import { TestDateProvider } from '@aztec/foundation/timer'; import { type FieldsOf, unfreeze } from '@aztec/foundation/types'; import type { P2P } from '@aztec/p2p'; @@ -398,6 +399,47 @@ describe('ProposalHandler checkpoint validation', () => { }); }); + // retryUntil only consults its deadline once an attempt returns, so an attempt that never settles never + // reaches it. The duty budget races the whole loop, so the caller settles instead of being held open. + it('settles within the duty budget when a readiness read never returns', async () => { + blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); + // attestation_deadline(slot=1) is 40s; leave a short but nonzero budget. + dateProvider.setTime(39_800); + + const result = await handler.handleCheckpointProposal(await makeProposal(), proposalInfo); + + expect(result).toEqual({ isValid: false, reason: 'last_block_not_found' }); + }); + + it('settles within the grace when a readiness read never returns past the deadline', async () => { + blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(41_000); + + const result = await handler.handleCheckpointProposal(await makeProposal(), proposalInfo); + + expect(result).toEqual({ isValid: false, reason: 'last_block_not_found' }); + }); + + // The cached-valid path re-reads the checkpoint's blocks before reusing a verdict; that read is a store + // call like any other and must not be able to hold the duty open either. + it('settles when the cached-verdict re-check never returns', async () => { + const archiveRoot = Fr.random(); + blockSource.getBlocksForSlot.mockResolvedValue(makeSlotBlocks([archiveRoot])); + blockSource.getCheckpointData.mockResolvedValue({ checkpointNumber: CheckpointNumber(1) } as CheckpointData); + const proposal = await makeProposal({ archiveRoot }); + + // First call caches a verdict; it is not valid, so make one that is by re-running against a valid result. + const cached = { isValid: true as const, checkpointNumber: CheckpointNumber(1) }; + (handler as unknown as { lastCheckpointValidationResult: unknown }).lastCheckpointValidationResult = { + payloadHash: proposal.getPayloadHash(), + result: cached, + }; + blockSource.getBlockData.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(39_800); + + await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).rejects.toThrow(/Duty budget/); + }); + // With <1s remaining the old Math.floor(...) timeout collapsed to 0 ("never time out"). The fix uses // a strictly-positive fractional timeout, so the wait still terminates instead of hanging. it('terminates with a fractional sub-second timeout when <1s remains before the deadline', async () => { @@ -1399,6 +1441,31 @@ describe('ProposalHandler checkpoint validation', () => { expect(txProvider.getTxsForBlockProposal).not.toHaveBeenCalled(); }); + // The bundle read and tx collection are started together. Once collection has failed there is no proposal + // left to validate, so the bundle's retries must stop rather than keep forcing archiver syncs for the rest + // of the slot; a catch handler on the abandoned promise only silences its rejection. + it('stops the bundle retries when tx collection rejects', async () => { + const { proposal, blockHandler, txProvider } = await setupStreamingProposal(signedRef, { + nowMs: BEFORE_DEADLINE_MS, + }); + // The metadata check passes so both siblings start; only the bundle read keeps missing. + l1ToL2MessageSource.getMessagePosition.mockImplementation(count => + Promise.resolve(count === 0n ? position(0n, Fr.ZERO) : count === 2n ? position(2n, prefixHash) : undefined), + ); + l1ToL2MessageSource.getL1ToL2MessageRange.mockRejectedValue(new Error('range is not fully synced')); + txProvider.getTxsForBlockProposal.mockRejectedValue(new Error('tx provider down')); + + const startMs = Date.now(); + await expect(blockHandler.handleBlockProposal(proposal, {} as any, true)).rejects.toThrow('tx provider down'); + const elapsedMs = Date.now() - startMs; + + // It gave up as soon as its sibling failed rather than running out the remaining budget. + expect(elapsedMs).toBeLessThan(WAIT_BUDGET_MS); + const syncsAtGiveUp = blockSource.syncImmediate.mock.calls.length; + await sleep(3 * WAIT_INTERVAL_MS); + expect(blockSource.syncImmediate).toHaveBeenCalledTimes(syncsAtGiveUp); + }); + it('rejects immediately without syncing when the attestation deadline has already passed', async () => { const { proposal, blockHandler } = await setupStreamingProposal(signedRef, { nowMs: PAST_DEADLINE_MS }); mockLocalView(undefined); diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 718e48c9e668..e4efefa7ff46 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -62,6 +62,7 @@ import { import { type TelemetryClient, type Tracer, getTelemetryClient } from '@aztec/telemetry-client'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; +import { DutyBudget, DutyBudgetExpiredError } from './duty_budget.js'; import type { ValidatorMetrics } from './metrics.js'; import { type StreamingBlockCheckReason, @@ -442,64 +443,28 @@ export class ProposalHandler { proposal: ValidatedCheckpointProposalCore, _sender: PeerId, ): Promise => { + // This callback runs before the validator's own, and does the reads and waits the attestation later rests on, + // so the duty's absolute budget starts here rather than around the attestation alone. A wrapper further in + // would leave this work — and every non-validator node, which only ever runs this callback — unbounded. + const budget = new DutyBudget(this.getReexecutionDeadline(proposal.slotNumber), this.dateProvider); try { - const pipeliningTimer = new Timer(); - const proposalInfo: LogData = { - slot: proposal.slotNumber, - archive: proposal.archive.toString(), - proposer: proposal.getSender()?.toString(), - }; - - if (this.config.skipCheckpointProposalValidation) { - this.log.warn(`Skipping checkpoint proposal validation for slot ${proposal.slotNumber}`, proposalInfo); - return undefined; - } - - if (await this.epochCache.isEscapeHatchOpenAtSlot(proposal.slotNumber)) { - this.log.warn( - `Escape hatch open for slot ${proposal.slotNumber}, skipping checkpoint proposal validation`, - proposalInfo, - ); - return undefined; - } - - // A proposal is "own" when it was signed by a validator key this node also owns. The true local - // proposer already built, validated, and stored this checkpoint before broadcasting, so a matching - // proposed checkpoint is already in its archiver — skip the redundant re-validation. An HA peer that - // 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. - const proposer = proposal.getSender(); - const ownAddresses = this.getOwnValidatorAddresses?.(); - const isOwnProposal = proposer && ownAddresses?.some(addr => addr === proposer.toString()); - - if (isOwnProposal) { - const existing = await this.archiver?.getProposedCheckpointData({ slot: proposal.slotNumber }); - if (existing?.archive.root.equals(proposal.archive)) { - this.log.debug(`Skipping sync for existing own checkpoint proposal at slot ${proposal.slotNumber}`); - return undefined; - } - } - - const result = await this.handleCheckpointProposal(proposal, proposalInfo); - if (!result.isValid) { - // Track invalid checkpoint proposals so offense observers (the attested-invalid-proposal watcher) - // work on non-validator nodes too. This handler runs for all nodes; validators also mark via the - // failure callback below (idempotent). - if (SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[result.reason]) { - this.markInvalidProposalSlot(proposal.slotNumber); - } - await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo); - } else if (this.archiver) { - const set = await this.setProposedCheckpoint(proposal); - if (set) { - this.metrics?.recordCheckpointProposalToPipelinedStateDuration(pipeliningTimer.ms()); - } - } + return await budget.run(`checkpoint proposal validation for slot ${proposal.slotNumber}`, () => + this.handleAllNodesCheckpointProposal(proposal, budget), + ); } catch (err) { - this.log.warn(`Error handling checkpoint proposal for slot ${proposal.slotNumber}`, { err }); + if (err instanceof DutyBudgetExpiredError) { + this.log.warn(`Checkpoint proposal handling for slot ${proposal.slotNumber} ran out of duty budget`, { + slot: proposal.slotNumber, + deadline: budget.deadline.toISOString(), + }); + } else { + this.log.warn(`Error handling checkpoint proposal for slot ${proposal.slotNumber}`, { err }); + } + return undefined; + } finally { + // Anything still retrying belongs to a duty nobody will read from any more. + budget.stop(`checkpoint proposal validation for slot ${proposal.slotNumber}`); } - return undefined; }; p2pClient.registerAllNodesCheckpointProposalHandler(checkpointHandler); @@ -507,6 +472,71 @@ export class ProposalHandler { return this; } + /** + * The body of the all-nodes checkpoint proposal callback: validates, caches and (for pipelining) records the + * proposal as this node's proposed checkpoint. Runs inside the slot's duty budget, which every stage consults + * before starting further work. + */ + private async handleAllNodesCheckpointProposal( + proposal: ValidatedCheckpointProposalCore, + budget: DutyBudget, + ): Promise { + const pipeliningTimer = new Timer(); + const proposalInfo: LogData = { + slot: proposal.slotNumber, + archive: proposal.archive.toString(), + proposer: proposal.getSender()?.toString(), + }; + + if (this.config.skipCheckpointProposalValidation) { + this.log.warn(`Skipping checkpoint proposal validation for slot ${proposal.slotNumber}`, proposalInfo); + return undefined; + } + + if (await this.epochCache.isEscapeHatchOpenAtSlot(proposal.slotNumber)) { + this.log.warn( + `Escape hatch open for slot ${proposal.slotNumber}, skipping checkpoint proposal validation`, + proposalInfo, + ); + return undefined; + } + + // A proposal is "own" when it was signed by a validator key this node also owns. The true local + // proposer already built, validated, and stored this checkpoint before broadcasting, so a matching + // proposed checkpoint is already in its archiver — skip the redundant re-validation. An HA peer that + // 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. + const proposer = proposal.getSender(); + const ownAddresses = this.getOwnValidatorAddresses?.(); + const isOwnProposal = proposer && ownAddresses?.some(addr => addr === proposer.toString()); + + if (isOwnProposal) { + const existing = await this.archiver?.getProposedCheckpointData({ slot: proposal.slotNumber }); + if (existing?.archive.root.equals(proposal.archive)) { + this.log.debug(`Skipping sync for existing own checkpoint proposal at slot ${proposal.slotNumber}`); + return undefined; + } + } + + const result = await this.handleCheckpointProposal(proposal, proposalInfo, budget); + if (!result.isValid) { + // Track invalid checkpoint proposals so offense observers (the attested-invalid-proposal watcher) + // work on non-validator nodes too. This handler runs for all nodes; validators also mark via the + // failure callback below (idempotent). + if (SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT[result.reason]) { + this.markInvalidProposalSlot(proposal.slotNumber); + } + await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo); + } else if (this.archiver) { + const set = await this.setProposedCheckpoint(proposal); + if (set) { + this.metrics?.recordCheckpointProposalToPipelinedStateDuration(pipeliningTimer.ms()); + } + } + return undefined; + } + /** * Processes a block proposal: collects its txs and, if requested, re-executes them to check the resulting * block against the proposal. Expects the proposal to have already passed p2p ingress validation (signature @@ -625,11 +655,20 @@ export class ProposalHandler { // Collect txs from the proposal. We start doing this as early as possible, // and we do it even if we don't plan to re-execute the txs, so that we have them if another node needs them. // The block's message bundle is an independent read, so derive it concurrently with the collection. + const bundleStop = new AbortController(); const txsPromise = this.collectProposalTxs(proposal, blockNumber, proposalSender, proposalInfo); - const bundlePromise = this.awaitStreamingBlockBundle(proposal, blockNumber, parentBlock, proposalInfo); + const bundlePromise = this.awaitStreamingBlockBundle( + proposal, + blockNumber, + parentBlock, + proposalInfo, + bundleStop.signal, + ); // Promise.all settles on the first rejection, so without a handler of its own the loser's later rejection - // would surface as an unhandled rejection. Awaiting below still observes whichever rejected first. - txsPromise.catch(() => {}); + // would surface as an unhandled rejection. Awaiting below still observes whichever rejected first. A handler + // silences the rejection but does not stop the work: once tx collection has failed there is no proposal left + // to validate, so the bundle's retries are cancelled instead of forcing archiver syncs for the rest of the slot. + txsPromise.catch(() => bundleStop.abort()); bundlePromise.catch(() => {}); const [collected, bundle] = await Promise.all([txsPromise, bundlePromise]); if (collected === 'invalid_embedded_txs') { @@ -795,19 +834,28 @@ export class ProposalHandler { * passed on entry (nothing is forced in that case) or when it passes while waiting; anything other than the * timeout propagates. Callers own their own logging and whatever they fall back to on `undefined`, and check * the deadline themselves when they need to tell "no budget on entry" apart from "timed out while waiting". + * + * `stop` cancels the loop for callers whose result nobody will read any more — a sibling read that already + * failed, or a duty whose budget is gone. Attaching a rejection handler to an abandoned promise keeps the + * process quiet but does not stop it forcing an archiver sync every half second for the rest of the slot. The + * shared archiver run itself is never cancelled, only this caller's retries. */ private async awaitLocalSync( slotNumber: SlotNumber, what: string, resolve: () => Promise, + stop?: AbortSignal, ): Promise { const deadline = this.getReexecutionDeadline(slotNumber); - if (deadline.getTime() - this.dateProvider.now() <= 0) { + if (deadline.getTime() - this.dateProvider.now() <= 0 || stop?.aborted) { return undefined; } try { return await retryUntil( async () => { + if (stop?.aborted) { + throw new TimeoutError(`Stopped waiting for ${what}`); + } await this.blockSource.syncImmediate(); return await resolve(); }, @@ -1027,7 +1075,7 @@ export class ProposalHandler { * checkpoint can land on L1 in the target slot; all nodes agree on it. Loosened from the previous * next-wall-clock-slot-boundary bound (see the timetable spec / refactor notes). */ - private getReexecutionDeadline(slotNumber: SlotNumber): Date { + public getReexecutionDeadline(slotNumber: SlotNumber): Date { return new Date(this.timetable.getAttestationDeadline(slotNumber) * 1000); } @@ -1101,6 +1149,7 @@ export class ProposalHandler { blockNumber: BlockNumber, parentBlock: 'genesis' | BlockData, proposalInfo: LogData, + stop?: AbortSignal, ): Promise { const readBundle = async (): Promise => { const metadata = await this.checkStreamingBlockMetadata(proposal, blockNumber, parentBlock); @@ -1118,10 +1167,15 @@ export class ProposalHandler { ...proposalInfo, }); const timer = new Timer(); - const resolved = await this.awaitLocalSync(slotNumber, `inbox bundle for block ${blockNumber}`, async () => { - const result = await readBundle(); - return !result.accepted && isRetryableStreamingBlockCheckReason(result.reason) ? undefined : result; - }); + const resolved = await this.awaitLocalSync( + slotNumber, + `inbox bundle for block ${blockNumber}`, + async () => { + const result = await readBundle(); + return !result.accepted && isRetryableStreamingBlockCheckReason(result.reason) ? undefined : result; + }, + stop, + ); if (resolved === undefined) { this.log.warn(`Timed out reading a consistent Inbox bundle, rejecting proposal`, { reason: 'inbox_prefix_sync_timeout', @@ -1340,6 +1394,7 @@ export class ProposalHandler { lastBlockTotal: bigint, checkpointInboxRollingHash: Fr, proposalInfo: LogData, + budget?: DutyBudget, ): Promise<{ accepted: true; messages: Fr[] } | { accepted: false; reason: CheckpointInboxPrefixReason }> { const read = () => this.readCheckpointConsumedMessages(checkpointStartTotal, lastBlockTotal, checkpointInboxRollingHash); @@ -1353,10 +1408,15 @@ export class ProposalHandler { lastBlockTotal, ...proposalInfo, }); - const resolved = await this.awaitLocalSync(slot, `inbox prefix for checkpoint at slot ${slot}`, async () => { - const result = await read(); - return result.accepted ? result : undefined; - }); + const resolved = await this.awaitLocalSync( + slot, + `inbox prefix for checkpoint at slot ${slot}`, + async () => { + const result = await read(); + return result.accepted ? result : undefined; + }, + budget?.signal, + ); if (resolved === undefined) { this.log.warn(`Timed out waiting for the checkpoint's consumed Inbox prefix to sync, refusing to attest`, { reason: 'inbox_prefix_sync_timeout', @@ -1508,6 +1568,7 @@ export class ProposalHandler { async handleCheckpointProposal( proposal: ValidatedCheckpointProposalCore, proposalInfo: LogData, + budget: DutyBudget = new DutyBudget(this.getReexecutionDeadline(proposal.slotNumber), this.dateProvider), ): Promise { const slot = proposal.slotNumber; const payloadHash = proposal.getPayloadHash(); @@ -1516,9 +1577,15 @@ export class ProposalHandler { // 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 || (await this.blockSource.getBlockData({ archive: proposal.archive })) !== undefined) { + 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) { this.log.debug(`Returning cached validation result for checkpoint proposal at slot ${slot}`, proposalInfo); return cached; } @@ -1536,7 +1603,7 @@ export class ProposalHandler { ); result = { isValid: false, reason: 'invalid_fee_asset_price_modifier' }; } else { - result = await this.validateCheckpointProposal(proposal, proposalInfo); + result = await this.validateCheckpointProposal(proposal, proposalInfo, budget); } this.lastCheckpointValidationResult = { payloadHash, result }; @@ -1573,6 +1640,7 @@ export class ProposalHandler { async validateCheckpointProposal( proposal: CheckpointProposalCore, proposalInfo: LogData, + budget: DutyBudget = new DutyBudget(this.getReexecutionDeadline(proposal.slotNumber), this.dateProvider), ): Promise { const slot = proposal.slotNumber; @@ -1591,17 +1659,21 @@ export class ProposalHandler { // out after a single attempt instead of looping (the immediate-timeout semantics of the deadline overload). let snapshot: CheckpointBlocksSnapshot | undefined; try { - snapshot = await retryUntil( - async () => { - await this.blockSource.syncImmediate(); - return await this.readCheckpointBlocksSnapshot(slot, proposal.archive); - }, - `waiting for block with archive ${proposal.archive.toString()} for slot ${slot}`, - { deadline, dateProvider: this.dateProvider }, - 0.5, + // `retryUntil` only checks its own deadline after an attempt returns, so an attempt that never settles never + // reaches it. Race the whole loop against the budget as well, so a hanging read cannot hold the duty open. + snapshot = await budget.run(`checkpoint blocks snapshot for slot ${slot}`, () => + retryUntil( + async () => { + await this.blockSource.syncImmediate(); + return await this.readCheckpointBlocksSnapshot(slot, proposal.archive); + }, + `waiting for block with archive ${proposal.archive.toString()} for slot ${slot}`, + { deadline, dateProvider: this.dateProvider }, + 0.5, + ), ); } catch (err) { - if (err instanceof TimeoutError) { + if (err instanceof TimeoutError || err instanceof DutyBudgetExpiredError) { this.log.warn(`Timed out waiting for block with archive matching checkpoint proposal`, proposalInfo); return { isValid: false, reason: 'last_block_not_found' }; } @@ -1699,6 +1771,7 @@ export class ProposalHandler { this.blockLeafCount(blocks[blocks.length - 1]), proposal.checkpointHeader.inboxRollingHash, proposalInfo, + budget, ); if (!consumed.accepted) { this.log.warn(`Streaming Inbox checkpoint content check failed, refusing to attest`, { diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index f18c832ee443..e10db25ea84e 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -63,6 +63,7 @@ import type { FullNodeCheckpointsBuilder, } from './checkpoint_builder.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'; import { type CheckpointProposalValidationFailureReason, ProposalHandler } from './proposal_handler.js'; import { ValidatorClient } from './validator.js'; @@ -902,6 +903,50 @@ describe('ValidatorClient', () => { validateCheckpointSpy.mockRestore(); }); + // Signing may be remote (HA), so a request that started in time can return past the attestation deadline. + // Peers reject a stale attestation, so it must not reach the pool or be handed back for gossip; the + // signing-protection record it produced still stands. + it('discards a checkpoint attestation whose signer returns past the attestation deadline', async () => { + const addCheckpointAttestationsSpy = jest.spyOn(p2pClient, 'addOwnCheckpointAttestations'); + epochCache.filterInCommittee.mockResolvedValue([EthAddress.fromString(validatorAccounts[0].address)]); + + const checkpointProposal = await makeCheckpointProposal({ + archiveRoot: proposal.archive, + checkpointHeader: makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }), + lastBlock: { + blockHeader: makeBlockHeader(1, { blockNumber: BlockNumber(123), slotNumber: proposal.slotNumber }), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + txHashes: proposal.txHashes, + }, + }); + + const validateCheckpointSpy = jest + .spyOn(validatorClient.getProposalHandler(), 'validateCheckpointProposal') + .mockResolvedValue({ isValid: true, checkpointNumber: CheckpointNumber(1) }); + + // The signer takes long enough that the slot's attestation deadline passes while it runs. + const deadline = validatorClient.getProposalHandler().getReexecutionDeadline(proposal.slotNumber); + const validationService = (validatorClient as unknown as { validationService: ValidationService }) + .validationService; + const attest = validationService.attestToCheckpointProposal.bind(validationService); + jest + .spyOn(validationService, 'attestToCheckpointProposal') + .mockImplementation(async (...args: Parameters) => { + const attestations = await attest(...args); + dateProvider.setTime(deadline.getTime() + 1_000); + return attestations; + }); + + const attestations = await validatorClient.attestToCheckpointProposal( + ValidatedCheckpointProposalCore(checkpointProposal), + sender, + ); + + expect(attestations).toBeUndefined(); + expect(addCheckpointAttestationsSpy).not.toHaveBeenCalled(); + validateCheckpointSpy.mockRestore(); + }); + it('should not attest to a checkpoint proposal that references a middle block instead of the last', async () => { const addCheckpointAttestationsSpy = jest.spyOn(p2pClient, 'addOwnCheckpointAttestations'); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 3940619e97ac..2c72ed2f08d5 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -627,7 +627,14 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return undefined; } - return await this.createCheckpointAttestationsFromProposal(proposal, attestors, checkpointNumber); + // Everything above may have waited on local state to catch up, so the signing gate takes the slot's consensus + // attestation deadline: nothing signed after it reaches a peer that would still accept it. + return await this.createCheckpointAttestationsFromProposal( + proposal, + attestors, + checkpointNumber, + this.proposalHandler.getReexecutionDeadline(proposalSlotNumber), + ); } /** @@ -651,21 +658,47 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return true; } + /** + * The one place a checkpoint attestation is produced, so `deadline` gates every path that reaches it, the + * cached-verdict one included. Callers responding to a peer's proposal pass the slot's consensus attestation + * deadline; the proposer's own attestations are produced inside its publish budget instead and pass none. + */ private async createCheckpointAttestationsFromProposal( proposal: CheckpointProposalCore, attestors: EthAddress[] = [], checkpointNumber: CheckpointNumber, + deadline?: Date, ): Promise { // Equivocation check: must happen right before signing to minimize the race window if (!this.shouldAttestToSlot(proposal.slotNumber)) { return undefined; } + const expired = () => deadline !== undefined && this.dateProvider.now() >= deadline.getTime(); + if (expired()) { + this.log.warn(`Not requesting an attestation for slot ${proposal.slotNumber}: past the attestation deadline`, { + slot: proposal.slotNumber, + deadline: deadline!.toISOString(), + }); + return undefined; + } + const attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); - // Track the proposal we attested to (to prevent equivocation) + // Track the proposal we attested to (to prevent equivocation). The signing-protection record stands even when + // the signature itself turns out to be too late to use. this.lastAttestedProposal = proposal; + // Signing may be remote (HA), so a request that started in time can still return past the deadline. Peers + // reject a stale attestation, so discard it rather than putting it in the pool or handing it back for gossip. + if (expired()) { + this.log.warn(`Discarding attestations for slot ${proposal.slotNumber}: the signer returned too late`, { + slot: proposal.slotNumber, + deadline: deadline!.toISOString(), + }); + return undefined; + } + await this.p2pClient.addOwnCheckpointAttestations(attestations); return attestations; } From 6f006648c5f4f67f90ed99c3de89491ebf47870e Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:10:36 -0300 Subject: [PATCH 07/25] fix(prover): join proof arrival with local completion before handing off a sub-tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A checkpoint's proofs can be ready before the local work that uses the sub-tree has finished. An empty block's root rollup is enqueued from startNewBlock, and a cached identical-input proof returns immediately, so the sub-tree result could resolve while the caller still held a block's processing fork — and the consumer it resolved to would then tear the sub-tree down underneath that caller. Both sides now wait for local completion, not for the pieces to be present: - A block counts as done only once its proof output has been compared with the header and archive built locally. `verifyBuiltBlockAgainstSyncedState` records that, and sub-tree resolution requires it for every block, so parity landing during `db.close()` or verification no longer resolves early. - The prover node joins the sub-tree result with its own block loop before resolving the proofs and starting teardown. A processing failure rejects the join, so failure always wins over a later success. --- .../src/orchestrator/block-proving-state.ts | 15 ++ .../orchestrator/checkpoint-proving-state.ts | 9 + .../checkpoint-sub-tree-orchestrator.test.ts | 240 ++++++++++++++++++ .../checkpoint-sub-tree-orchestrator.ts | 15 +- .../src/job/checkpoint-prover.test.ts | 95 +++++++ .../prover-node/src/job/checkpoint-prover.ts | 35 ++- 6 files changed, 401 insertions(+), 8 deletions(-) diff --git a/yarn-project/prover-client/src/orchestrator/block-proving-state.ts b/yarn-project/prover-client/src/orchestrator/block-proving-state.ts index a1b48de90321..65383de004b0 100644 --- a/yarn-project/prover-client/src/orchestrator/block-proving-state.ts +++ b/yarn-project/prover-client/src/orchestrator/block-proving-state.ts @@ -57,6 +57,7 @@ export class BlockProvingState { private builtArchive: AppendOnlyTreeSnapshot | undefined; private endState: StateReference | undefined; private endSpongeBlob: SpongeBlob | undefined; + private verified = false; private txs: TxProvingState[] = []; private error: string | undefined; @@ -216,6 +217,20 @@ export class BlockProvingState { return this.builtArchive; } + /** + * Records that this block's proof outputs were checked against the locally built header and archive and agreed. + * Field presence is not the same thing: the archive snapshot is captured before the fork is closed and before the + * comparison runs, so a caller that only looks for the pieces can see a block that has not been verified at all. + */ + public markVerified() { + this.verified = true; + } + + /** Whether {@link markVerified} has recorded a successful check for this block. */ + public isVerified() { + return this.verified; + } + public getStartSpongeBlob() { return this.startSpongeBlob; } diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-proving-state.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-proving-state.ts index 7ebede67f5a3..6dbd40ef668e 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-proving-state.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-proving-state.ts @@ -236,6 +236,15 @@ export class CheckpointProvingState { : this.blockProofs.getChildren(rootLocation).map(c => c?.provingOutput); } + /** + * Whether every block of this checkpoint has been started and has passed its local verification. A checkpoint + * whose blocks are still being started has not, so a proof arriving before the caller reached the later blocks + * cannot be mistaken for a finished sub-tree. + */ + public allBlocksVerified() { + return this.blocks.length === this.totalNumBlocks && this.blocks.every(block => block?.isVerified()); + } + /** Sibling path of the archive tree captured before any block in this checkpoint landed. */ public getLastArchiveSiblingPath() { return this.lastArchiveSiblingPath; diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts index e74aded283ae..93fb400083d0 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts @@ -4,10 +4,17 @@ import { padArrayEnd, sum } from '@aztec/foundation/collection'; import { Fr } from '@aztec/foundation/curves/bn254'; import { EthAddress } from '@aztec/foundation/eth-address'; import { createLogger } from '@aztec/foundation/log'; +import { type PromiseWithResolvers, promiseWithResolvers } from '@aztec/foundation/promise'; +import { retryUntil } from '@aztec/foundation/retry'; +import { sleep } from '@aztec/foundation/sleep'; import { L1ToL2MessageSponge, ScopedL2ToL1Message, computeBlockOutHash } from '@aztec/stdlib/messaging'; +import type { CheckpointConstantData } from '@aztec/stdlib/rollup'; import { makeScopedL2ToL1Message } from '@aztec/stdlib/testing'; +import type { BlockHeader } from '@aztec/stdlib/tx'; import { TestContext, makeTestDeferredJobQueue } from '../mocks/test_context.js'; +import type { BlockProvingState } from './block-proving-state.js'; +import type { CheckpointProvingState } from './checkpoint-proving-state.js'; import { CheckpointSubTreeOrchestrator } from './checkpoint-sub-tree-orchestrator.js'; import { ChonkCache } from './chonk-cache.js'; @@ -311,4 +318,237 @@ describe('prover/orchestrator/checkpoint-sub-tree', () => { await subTree.stop(); } }); + + describe('local completion join', () => { + it('withholds the result while a block whose proofs are already in is still using its fork', async () => { + // An empty block's root rollup is enqueued from startNewBlock, so its proof — and the checkpoint's parity + // proof — can land while the caller still holds the block's processing fork and has not called + // setBlockCompleted. Resolving here would hand the sub-tree to a consumer that may tear it down underneath + // that caller. + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = await context.makeCheckpoint(1); + const subTree = await startInspectableSubTree(1, constants, l1ToL2Messages, previousBlockHeader); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + const { blockNumber, timestamp } = blocks[0].header.globalVariables; + + await subTree.startNewBlock(blockNumber, timestamp, 0, l1ToL2Messages); + await waitForAllProofs(subTree); + + await sleep(50); + expect(result.settled).toBe(false); + + await subTree.setBlockCompleted(blockNumber, blocks[0].header); + await expect(subTree.getSubTreeResult()).resolves.toBeDefined(); + } finally { + await subTree.stop(); + } + }); + + it('withholds the result while verification runs, even with header, archive and proofs all present', async () => { + // The narrower window inside setBlockCompleted: the archive snapshot is captured and the fork closed before + // verification runs, so every piece the resolution used to look for is present while the block is still + // unverified. Parity landing in that window must not resolve the sub-tree. + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = await context.makeCheckpoint(1); + const subTree = await startInspectableSubTree(1, constants, l1ToL2Messages, previousBlockHeader); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + const { blockNumber, timestamp } = blocks[0].header.globalVariables; + + subTree.holdVerification(); + await subTree.startNewBlock(blockNumber, timestamp, 0, l1ToL2Messages); + await waitForAllProofs(subTree); + + const completed = subTree.setBlockCompleted(blockNumber, blocks[0].header); + await sleep(50); + expect(result.settled).toBe(false); + + subTree.releaseVerification(); + await completed; + await expect(subTree.getSubTreeResult()).resolves.toBeDefined(); + } finally { + subTree.releaseVerification(); + await subTree.stop(); + } + }); + + it('withholds the result until the last block of a multi-block checkpoint is verified', async () => { + // Same race with the block merge in play: the second block is message-only, so its root proof is enqueued + // from startNewBlock and the merge can complete while that block is still being driven. + const l1ToL2MessagesPerBlock = [[], [new Fr(2001)]]; + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = + await context.makeCheckpointWithMessagesPerBlock(l1ToL2MessagesPerBlock, { numTxsPerBlock: [1, 0] }); + const subTree = await startInspectableSubTree(2, constants, l1ToL2Messages, previousBlockHeader); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + + const first = blocks[0].header.globalVariables; + await subTree.startNewBlock(first.blockNumber, first.timestamp, blocks[0].txs.length, []); + await subTree.addTxs(blocks[0].txs); + await subTree.setBlockCompleted(first.blockNumber, blocks[0].header); + + const second = blocks[1].header.globalVariables; + await subTree.startNewBlock(second.blockNumber, second.timestamp, 0, l1ToL2MessagesPerBlock[1]); + await waitForAllProofs(subTree); + + await sleep(50); + expect(result.settled).toBe(false); + + await subTree.setBlockCompleted(second.blockNumber, blocks[1].header); + await expect(subTree.getSubTreeResult()).resolves.toBeDefined(); + } finally { + await subTree.stop(); + } + }); + + it('rejects and never resolves when the built archive disagrees with the synced one', async () => { + // A verification mismatch reports through provingState.reject rather than by throwing, so the failure has to + // win over the proofs that are already in hand. + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = await context.makeCheckpoint(1); + const subTree = await startInspectableSubTree(1, constants, l1ToL2Messages, previousBlockHeader, { + worldState: withStaleArchiveSnapshots(context.worldState), + }); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + const { blockNumber, timestamp } = blocks[0].header.globalVariables; + + await subTree.startNewBlock(blockNumber, timestamp, 0, l1ToL2Messages); + await waitForAllProofs(subTree); + await subTree.setBlockCompleted(blockNumber, blocks[0].header); + + await expect(subTree.getSubTreeResult()).rejects.toThrow(/Archive tree mismatch/); + expect(result.resolved).toBe(false); + } finally { + await subTree.stop(); + } + }); + + it('does not publish a result when the caller supplies a header the block does not match', async () => { + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = await context.makeCheckpoint(1); + const subTree = await startInspectableSubTree(1, constants, l1ToL2Messages, previousBlockHeader); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + const { blockNumber, timestamp } = blocks[0].header.globalVariables; + + await subTree.startNewBlock(blockNumber, timestamp, 0, l1ToL2Messages); + await waitForAllProofs(subTree); + + // Any header that is not this block's: completion must refuse it rather than verify against it. + await expect(subTree.setBlockCompleted(blockNumber, previousBlockHeader)).rejects.toThrow( + /Block header mismatch/, + ); + + await sleep(50); + expect(result.settled).toBe(false); + } finally { + await subTree.stop(); + } + }); + }); + + /** Starts a sub-tree that exposes its checkpoint state and can park local verification on demand. */ + function startInspectableSubTree( + numBlocks: number, + constants: CheckpointConstantData, + l1ToL2Messages: Fr[], + previousBlockHeader: BlockHeader, + { worldState = context.worldState }: { worldState?: typeof context.worldState } = {}, + ): Promise { + return InspectableSubTree.startInspectable( + worldState, + context.prover, + EthAddress.ZERO, + chonkCache, + EpochNumber(1), + false, + makeTestDeferredJobQueue(), + constants, + l1ToL2Messages, + Fr.ZERO, + numBlocks, + previousBlockHeader, + ); + } }); + +/** + * A sub-tree that lets a test observe the checkpoint's proofs and hold local verification open, so proofs can be + * made to land while a block is deliberately left unfinished. + */ +class InspectableSubTree extends CheckpointSubTreeOrchestrator { + private verificationGate: PromiseWithResolvers | undefined; + + public static async startInspectable( + ...args: Parameters + ): Promise { + const subTree = await InspectableSubTree.start(...args); + if (!(subTree instanceof InspectableSubTree)) { + throw new Error('The start factory did not construct the subclass it was called on.'); + } + return subTree; + } + + public getCheckpointProvingState(): CheckpointProvingState { + if (!this.provingState) { + throw new Error('Sub-tree has no checkpoint state.'); + } + return this.provingState; + } + + /** Parks every subsequent local verification until {@link releaseVerification}. */ + public holdVerification(): void { + this.verificationGate ??= promiseWithResolvers(); + } + + public releaseVerification(): void { + this.verificationGate?.resolve(); + } + + protected override async verifyBuiltBlockAgainstSyncedState(provingState: BlockProvingState) { + await this.verificationGate?.promise; + return await super.verifyBuiltBlockAgainstSyncedState(provingState); + } +} + +/** Waits until every block proof of the checkpoint and its parity proof are in hand. */ +async function waitForAllProofs(subTree: InspectableSubTree): Promise { + await retryUntil( + () => { + const state = subTree.getCheckpointProvingState(); + return state.getSubTreeOutputProofs().every(proof => !!proof) && !!state.getInboxParityProof(); + }, + 'checkpoint proofs', + 30, + 0.05, + ); +} + +/** Records how a promise settled without awaiting it, so a test can assert it stayed pending. */ +function trackSettlement(promise: Promise) { + const status = { settled: false, resolved: false }; + void promise.then( + () => { + status.settled = true; + status.resolved = true; + }, + () => { + status.settled = true; + }, + ); + return status; +} + +/** + * A world state whose archive snapshots are taken one block early, so the block's built archive cannot agree with + * the synced one and verification must reject the checkpoint. + */ +function withStaleArchiveSnapshots(worldState: T): T { + return new Proxy(worldState, { + get(target, prop, receiver) { + const value = Reflect.get(target, prop, receiver); + if (prop === 'getSnapshot' && typeof value === 'function') { + return (blockNumber: number) => value.call(target, blockNumber - 1); + } + return typeof value === 'function' ? value.bind(target) : value; + }, + }); +} diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts index 6c12bcfae800..c84bc41d6d5c 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts @@ -204,7 +204,7 @@ export class CheckpointSubTreeOrchestrator extends ProvingScheduler { telemetryClient: TelemetryClient = getTelemetryClient(), bindings?: LoggerBindings, ): Promise { - const subTree = new CheckpointSubTreeOrchestrator( + const subTree = new this( dbProvider, prover, proverId, @@ -889,6 +889,13 @@ export class CheckpointSubTreeOrchestrator extends ProvingScheduler { // Parity not proven yet — retried when the inbox parity proof lands. return; } + // Proofs being present is not the same as the checkpoint being finished locally. The archive snapshot is + // captured before the block's fork is closed and before its outputs are compared with it, and parity can land + // during those awaits and reach here directly, so a proof-only gate would hand the sub-tree off — and let its + // caller tear it down — while a block is still being finalized. Retried as each block finishes verifying. + if (!provingState.allBlocksVerified()) { + return; + } this.subTreeResult.resolve({ blockProofOutputs: nonEmpty, inboxParityProof, @@ -982,6 +989,12 @@ export class CheckpointSubTreeOrchestrator extends ProvingScheduler { provingState.reject(`New archive mismatch.`); return; } + + // Only now is this block's proof output known to agree with what was built locally. Recording it here (rather + // than inferring it from the pieces being present) is what lets the sub-tree resolution wait for it, and it is + // the last piece for a block whose proofs arrived before its fork was closed. + provingState.markVerified(); + this.checkAndEnqueueSubTreeResolution(provingState.parentCheckpoint); } private getDbForBlock(blockNumber: BlockNumber): MerkleTreeWriteOperations { diff --git a/yarn-project/prover-node/src/job/checkpoint-prover.test.ts b/yarn-project/prover-node/src/job/checkpoint-prover.test.ts index 36d6f75a9997..4301dbcc99e0 100644 --- a/yarn-project/prover-node/src/job/checkpoint-prover.test.ts +++ b/yarn-project/prover-node/src/job/checkpoint-prover.test.ts @@ -417,6 +417,101 @@ describe('CheckpointProver', () => { }); }); + // ---------------- proofs ready before local processing finishes ---------------- + + describe('handoff join', () => { + /** A sub-tree whose proofs are ready from the outset, with its first block parked on a gate. */ + function makeEagerSubTree(gate: Promise, stop: jest.Mock<() => Promise>) { + let firstBlock = true; + return { + getSubTreeResult: () => + Promise.resolve({ + blockProofOutputs: [{ tag: 'block-proof-output' }], + inboxParityProof: { tag: 'inbox-parity-proof' }, + previousArchiveSiblingPath: makeTuple(ARCHIVE_HEIGHT, () => Fr.ZERO), + } as unknown as SubTreeResult), + startNewBlock: () => { + if (firstBlock) { + firstBlock = false; + return gate; + } + return Promise.resolve(); + }, + startChonkVerifierCircuits: () => Promise.resolve(), + addTxs: () => Promise.resolve(), + setBlockCompleted: () => Promise.resolve(), + cancel: () => {}, + stop, + }; + } + + it('holds the proofs and the sub-tree until the block loop is done with them', async () => { + // A cached identical-input proof, or a checkpoint of empty blocks, can make the sub-tree's proofs available + // before the caller has finished creating and using its processing forks. Handing them over then — and + // tearing the sub-tree down on the way — pulls it out from under the loop still running against it. + checkpoint = await Checkpoint.random(CheckpointNumber(1), { numBlocks: 2, txsPerBlock: 0 }); + + txProvider.getTxsForBlock.mockReset(); + txProvider.getTxsForBlock.mockResolvedValue({ txs: [], missingTxs: [] }); + + const firstBlockGate = promiseWithResolvers(); + const stop = jest.fn(() => Promise.resolve()); + proverFactory.createCheckpointSubTreeOrchestrator.mockResolvedValue( + makeEagerSubTree(firstBlockGate.promise, stop) as any, + ); + dbProvider.fork.mockResolvedValue({ + appendLeaves: () => Promise.resolve(), + close: () => Promise.resolve(), + } as any); + publicProcessorFactory.create.mockReturnValue({ process: () => Promise.resolve([[], []]) } as any); + + const prover = makeProver(); + let handedOff = false; + const proofs = prover.whenSubTreeProofsReady().then(result => { + handedOff = true; + return result; + }); + + // The proofs are ready but the first block is still parked, so nothing may be handed over or released. + await sleep(50); + expect(handedOff).toBe(false); + expect(stop).not.toHaveBeenCalled(); + + firstBlockGate.resolve(); + await expect(proofs).resolves.toEqual({ + blockProofOutputs: [{ tag: 'block-proof-output' }], + inboxParityProof: { tag: 'inbox-parity-proof' }, + }); + await prover.whenDone(); + expect(stop).toHaveBeenCalledTimes(1); + expect(prover.isFailed()).toBe(false); + }); + + it('fails rather than handing off proofs that arrived before a block loop that then broke', async () => { + // Same early arrival, but the loop gives up. The failure has to win: no success handoff off the back of + // proofs the local work never validated, and exactly one teardown. + checkpoint = await Checkpoint.random(CheckpointNumber(1), { numBlocks: 2, txsPerBlock: 0 }); + + txProvider.getTxsForBlock.mockReset(); + txProvider.getTxsForBlock.mockResolvedValue({ txs: [], missingTxs: [] }); + + const stop = jest.fn(() => Promise.resolve()); + proverFactory.createCheckpointSubTreeOrchestrator.mockResolvedValue( + makeEagerSubTree(Promise.resolve(), stop) as any, + ); + // The base block was unwound underneath the prover, so the loop cannot fork it. + dbProvider.fork.mockRejectedValue(new Error('Unable to get meta data for block 0')); + + const prover = makeProver(); + + await expect(prover.whenSubTreeProofsReady()).rejects.toThrow(/did not complete block processing/); + await prover.whenDone(); + expect(stop).toHaveBeenCalledTimes(1); + expect(prover.isFailed()).toBe(true); + expect(onFailed).toHaveBeenCalledTimes(1); + }); + }); + // ---------------- data-plane reorg fork fault ---------------- describe('data-plane reorg fault', () => { diff --git a/yarn-project/prover-node/src/job/checkpoint-prover.ts b/yarn-project/prover-node/src/job/checkpoint-prover.ts index 5518bab686a7..8788b72c85cc 100644 --- a/yarn-project/prover-node/src/job/checkpoint-prover.ts +++ b/yarn-project/prover-node/src/job/checkpoint-prover.ts @@ -109,6 +109,15 @@ export class CheckpointProver { * InboxParity proof (which feeds the checkpoint root in the top tree). */ private readonly subTreeProofs: PromiseWithResolvers = promiseWithResolvers(); + /** + * Settles when this prover has finished using the sub-tree: resolved once every block has been processed and + * handed over, rejected if processing gave up. The sub-tree's proofs can be ready before that — a single empty + * block needs none of the local work, and a cached identical-input proof returns immediately — so handing the + * proofs off and releasing the orchestrator on proof arrival alone would pull the sub-tree out from under a + * caller still building forks against it. + */ + private readonly localProcessingDone: PromiseWithResolvers = promiseWithResolvers(); + // Three independent lifecycle facts — deliberately not collapsed into one status enum, because several // combinations are legal and relied on: a prover can be `completed` and then `cancelled` (routine // teardown of an already-proven checkpoint), or `completed` and then `failed` (block proving was @@ -146,6 +155,7 @@ export class CheckpointProver { // Mark subTreeProofs as observed so a cancel that lands before any consumer awaits // does not surface as an unhandled rejection. this.subTreeProofs.promise.catch(() => {}); + this.localProcessingDone.promise.catch(() => {}); deps.log.info(`Created CheckpointProver ${this.id}`, { checkpointNumber: this.checkpoint.number, epochNumber: this.epochNumber, @@ -334,9 +344,16 @@ export class CheckpointProver { this.previousBlockHeader, ); subTreeStarted = true; - // Bridge the sub-tree's result onto subTreeProofs. - void this.subTree.getSubTreeResult().then( - result => { + // Bridge the sub-tree's result onto subTreeProofs, once this prover is also done with the sub-tree. + void this.subTree + .getSubTreeResult() + .then(async result => { + // Join with local processing before handing anything off. Proofs can be ready while the loop below is + // still creating forks and completing later blocks; resolving and tearing down here would close the + // orchestrator's dbs under it. A processing failure rejects the join instead, and the shared catch below + // routes it to `failSubTreeProofs` — which the failing path has already called — so a failure always wins + // over a later success. + await this.localProcessingDone.promise; this.deps.log.info(`Sub-tree block proofs ready for checkpoint ${this.checkpoint.number}`, { checkpointNumber: this.checkpoint.number, blockProofCount: result.blockProofOutputs.length, @@ -355,9 +372,8 @@ export class CheckpointProver { // top-tree job, a rebuilt EpochSession, failure upload) read only `whenSubTreeProofsReady()` and // this prover's own fields (`checkpoint`, `txs`, headers, sibling paths), never the sub-tree. this.teardownPromise = this.teardownSubTree(); - }, - err => this.failSubTreeProofs(err instanceof Error ? err : new Error(String(err))), - ); + }) + .catch(err => this.failSubTreeProofs(err instanceof Error ? err : new Error(String(err)))); if (signal.aborted) { return; } @@ -442,7 +458,12 @@ export class CheckpointProver { }, ); } finally { - if (!this.completed) { + if (this.completed) { + // Release the result callback: it may already be holding a finished set of proofs. + this.localProcessingDone.resolve(); + } else { + // Fail the join first, so the result callback cannot resolve or start a teardown while this one runs. + this.localProcessingDone.reject(new Error(`Checkpoint ${this.id} did not complete block processing`)); if (subTreeStarted) { await this.teardownSubTree(); } From bd2116390bde766fd84c8dc21e874e649bde15cc Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:11:15 -0300 Subject: [PATCH 08/25] test(prover): model parity and empty blocks the way the orchestrator schedules them The developer proving-time simulation gated inbox parity behind the first block root and refused a zero-transaction block anywhere but first. Production does neither: a zero-tx block enqueues its own block root from startNewBlock at any position, and parity runs independently of block-root production, joining at checkpoint-root readiness. The join is now reevaluated from whichever dependency finishes last and guarded against a duplicate enqueue, with coverage for no/one/many transactions, mixed empty blocks, and parity landing first or last. This corrects a developer model. It is not a measured production regression, and its timings are model output. --- .../src/test/epoch_proving_sim.test.ts | 176 +++++++++++++++--- 1 file changed, 154 insertions(+), 22 deletions(-) diff --git a/yarn-project/prover-client/src/test/epoch_proving_sim.test.ts b/yarn-project/prover-client/src/test/epoch_proving_sim.test.ts index 6dc5dfadcf54..6dd2f2b2c9c5 100644 --- a/yarn-project/prover-client/src/test/epoch_proving_sim.test.ts +++ b/yarn-project/prover-client/src/test/epoch_proving_sim.test.ts @@ -46,6 +46,9 @@ type Worker = { end: number; }; +// A finished job together with the window the simulated worker held it for. +type Completion = Job & { start: number; end: number }; + // New type definitions for flexible test configuration // Each public tx group: [count, avmDuration in ms] type PublicTxGroup = [count: number, avmDurationMs: number]; @@ -62,7 +65,8 @@ type TestConfig = { // State tracking for dependency resolution type SimState = { - // Parity tracking: one InboxParity proof per checkpoint, gating the first block root. + // Parity tracking: one InboxParity proof per checkpoint. It is proven from the start of the checkpoint and feeds + // the checkpoint root rollup, so it runs alongside block production rather than gating any block root. inboxParityComplete: Map; // Public tx dependency tracking (aggregate per block) @@ -78,11 +82,17 @@ type SimState = { txTreeLeafCount: Map; // total leaves (txs) per block txTreeNextLeafIndex: Map; // next leaf index to assign per block + // Per-block guard so a block root is queued exactly once, whichever dependency completes last + blockRootEnqueued: Map; + // Block merge tree tracking per checkpoint // Key format: "checkpoint-level" blockTreeNodes: Map>; blockRootComplete: Map; + // Per-checkpoint guard so the checkpoint root is queued exactly once, whichever of its two dependencies lands last + checkpointRootEnqueued: Map; + // Checkpoint merge tree tracking checkpointTreeNodes: Map>; // level -> index -> completed checkpointRootComplete: Map; @@ -93,7 +103,6 @@ type SimState = { // Block configuration for determining proof types blockConfigs: Map; blocksPerCheckpoint: Map; - isFirstBlock: Map; totalCheckpoints: number; }; @@ -241,14 +250,15 @@ function initializeState(checkpoints: Checkpoint[]): SimState { txTreeComplete: new Map(), txTreeLeafCount: new Map(), txTreeNextLeafIndex: new Map(), + blockRootEnqueued: new Map(), blockTreeNodes: new Map(), blockRootComplete: new Map(), + checkpointRootEnqueued: new Map(), checkpointTreeNodes: new Map(), checkpointRootComplete: new Map(), rootRollupComplete: false, blockConfigs: new Map(), blocksPerCheckpoint: new Map(), - isFirstBlock: new Map(), totalCheckpoints: checkpoints.length, }; @@ -258,6 +268,7 @@ function initializeState(checkpoints: Checkpoint[]): SimState { state.blocksPerCheckpoint.set(cp, blocks.length); state.inboxParityComplete.set(cp, false); state.blockRootComplete.set(cp, false); + state.checkpointRootEnqueued.set(cp, false); state.checkpointRootComplete.set(cp, false); for (let b = 1; b <= blocks.length; b++) { @@ -265,13 +276,8 @@ function initializeState(checkpoints: Checkpoint[]): SimState { const block = blocks[b - 1]; const { totalTxs } = getBlockTxCount(block); - // Empty blocks are only allowed as the first block in a checkpoint (matches orchestrator constraint) - if (totalTxs === 0 && b !== 1) { - throw new Error(`Cannot create a block with 0 txs, unless it's the first block. Checkpoint ${cp}, Block ${b}`); - } - state.blockConfigs.set(key, block); - state.isFirstBlock.set(key, b === 1); + state.blockRootEnqueued.set(key, false); state.vmComplete.set(key, 0); state.chonkComplete.set(key, 0); state.publicBaseEnqueued.set(key, 0); @@ -284,7 +290,7 @@ function initializeState(checkpoints: Checkpoint[]): SimState { return state; } -function fillQueue(queues: Queues, checkpoints: Checkpoint[]): void { +function fillQueue(queues: Queues, checkpoints: Checkpoint[], state: SimState): void { for (let cp = 1; cp <= checkpoints.length; cp++) { const blocks = checkpoints[cp - 1]; @@ -296,6 +302,12 @@ function fillQueue(queues: Queues, checkpoints: Checkpoint[]): void { const privateTxs = block[0]; const publicGroups = block.slice(1) as PublicTxGroup[]; + // A block with no txs has no base or merge proof whose completion would queue its block root, so it is queued + // as soon as the block starts. This holds wherever the block sits in the checkpoint. + if (getBlockTxCount(block).totalTxs === 0) { + tryEnqueueBlockRoot(cp, b, state, queues); + } + // Add private tx base rollups if (privateTxs > 0) { queues[ProvingRequestType.PRIVATE_TX_BASE_ROLLUP].push( @@ -323,7 +335,7 @@ function enqueueDependentJobs(job: Job, state: SimState, queues: Queues, checkpo switch (type) { case ProvingRequestType.INBOX_PARITY: { state.inboxParityComplete.set(checkpoint, true); - tryEnqueueBlockRoot(checkpoint, 1, state, queues); + tryEnqueueCheckpointRoot(checkpoint, state, queues); break; } @@ -488,20 +500,19 @@ function handleTxMergeComplete( function tryEnqueueBlockRoot(checkpoint: number, blk: number, state: SimState, queue: Queues): void { const key = blockKey(checkpoint, blk); - const blockConfig = state.blockConfigs.get(key)!; - const { totalTxs } = getBlockTxCount(blockConfig); - const isFirst = state.isFirstBlock.get(key)!; - // Need tx tree complete - if (!state.txTreeComplete.get(key)) { + if (state.blockRootEnqueued.get(key)) { return; } - // First block also needs the checkpoint's InboxParity proof - if (isFirst && !state.inboxParityComplete.get(checkpoint)) { + // A block root depends only on its own tx tree. The checkpoint's parity proof is not a dependency. + if (!state.txTreeComplete.get(key)) { return; } + const { totalTxs } = getBlockTxCount(state.blockConfigs.get(key)!); + state.blockRootEnqueued.set(key, true); + const blockRootType = getBlockRootType(totalTxs); queue[blockRootType].push(createJob(checkpoint, blk, blockRootType)); } @@ -569,11 +580,19 @@ function handleBlockMergeComplete( tryEnqueueBlockMerge(checkpoint, level, index, state, queues); } +// The checkpoint root joins the two independent branches of a checkpoint: its reduced block tree and its single +// InboxParity proof. Called whenever either lands, so whichever is last drives the enqueue. function tryEnqueueCheckpointRoot(checkpoint: number, state: SimState, queues: Queues): void { - if (!state.blockRootComplete.get(checkpoint)) { + if (state.checkpointRootEnqueued.get(checkpoint)) { + return; + } + + if (!state.blockRootComplete.get(checkpoint) || !state.inboxParityComplete.get(checkpoint)) { return; } + state.checkpointRootEnqueued.set(checkpoint, true); + const numBlocks = state.blocksPerCheckpoint.get(checkpoint)!; const checkpointRootType = getCheckpointRootType(numBlocks); queues[checkpointRootType].push(createJob(checkpoint, 0, checkpointRootType)); @@ -685,6 +704,7 @@ type SimulationResult = { workerUtilization: number; jobBreakdown: Record; workerActivity: WorkerActivityPoint[]; + completions: Completion[]; }; }; @@ -693,9 +713,9 @@ function runSimulationWithConfig(config: TestConfig): SimulationResult { const state = initializeState(checkpoints); const queues: Queues = times(Object.values(ProvingRequestType).length, () => []) as any; const workerPool: Worker[] = []; - const completed: Job[] = []; + const completed: Completion[] = []; - fillQueue(queues, checkpoints); + fillQueue(queues, checkpoints, state); let time = 0; let totalWorkerTime = 0; @@ -731,7 +751,7 @@ function runSimulationWithConfig(config: TestConfig): SimulationResult { // Process completions and enqueue dependent jobs for (const worker of justCompleted) { - completed.push(worker.job); + completed.push({ ...worker.job, start: worker.start, end: worker.end }); enqueueDependentJobs(worker.job, state, queues, checkpoints.length); } @@ -780,6 +800,7 @@ function runSimulationWithConfig(config: TestConfig): SimulationResult { workerUtilization, jobBreakdown, workerActivity, + completions: completed, }, }; } @@ -880,3 +901,114 @@ describe('epoch proving simulation', () => { expect(result.results.totalTimeMs).toBeLessThan(30 * 72 * 1000); // targeting root rollup in less than 30 slots }); }); + +const BLOCK_ROOT_TYPES = [ + ProvingRequestType.BLOCK_ROOT_ROLLUP, + ProvingRequestType.BLOCK_ROOT_SINGLE_TX_ROLLUP, + ProvingRequestType.BLOCK_ROOT_NO_TXS_ROLLUP, +]; + +const CHECKPOINT_ROOT_TYPES = [ + ProvingRequestType.CHECKPOINT_ROOT_ROLLUP, + ProvingRequestType.CHECKPOINT_ROOT_SINGLE_BLOCK_ROLLUP, +]; + +function simulate(name: string, checkpoints: Checkpoint[], workers: number): SimulationResult { + return runSimulationWithConfig({ name, workers, checkpoints }); +} + +function completionsOf(result: SimulationResult, types: ProvingRequestType[]): Completion[] { + return result.results.completions.filter(c => types.includes(c.type)); +} + +function soleCompletion(result: SimulationResult, types: ProvingRequestType[]): Completion { + const found = completionsOf(result, types); + expect(found).toHaveLength(1); + return found[0]; +} + +// The moment a checkpoint's block tree is fully reduced: the last block root or block merge to finish for it. +function blockTreeReadyAt(result: SimulationResult, checkpoint: number): number { + const ends = completionsOf(result, [...BLOCK_ROOT_TYPES, ProvingRequestType.BLOCK_MERGE_ROLLUP]) + .filter(c => c.checkpoint === checkpoint) + .map(c => c.end); + return Math.max(...ends); +} + +describe('checkpoint dependency model', () => { + it('proves an empty block root without waiting for inbox parity', () => { + const result = simulate('no transactions', [[[0]]], 16); + + const blockRoot = soleCompletion(result, BLOCK_ROOT_TYPES); + const parity = soleCompletion(result, [ProvingRequestType.INBOX_PARITY]); + + expect(blockRoot.type).toBe(ProvingRequestType.BLOCK_ROOT_NO_TXS_ROLLUP); + expect(blockRoot.start).toBe(0); + expect(parity.start).toBe(0); + expect(blockRoot.end).toBeLessThan(parity.end); + }); + + it('holds the checkpoint root until parity lands when parity finishes last', () => { + const result = simulate('parity last', [[[0], [0]]], 1); + + const parity = soleCompletion(result, [ProvingRequestType.INBOX_PARITY]); + const checkpointRoot = soleCompletion(result, CHECKPOINT_ROOT_TYPES); + + expect(checkpointRoot.type).toBe(ProvingRequestType.CHECKPOINT_ROOT_ROLLUP); + expect(blockTreeReadyAt(result, 1)).toBeLessThan(parity.end); + expect(checkpointRoot.start).toBe(parity.end); + }); + + it('holds the checkpoint root until the block tree lands when parity finishes first', () => { + const result = simulate('parity first', [[[2]]], 16); + + const parity = soleCompletion(result, [ProvingRequestType.INBOX_PARITY]); + const checkpointRoot = soleCompletion(result, CHECKPOINT_ROOT_TYPES); + const blockTreeReady = blockTreeReadyAt(result, 1); + + expect(parity.end).toBeLessThan(blockTreeReady); + expect(checkpointRoot.start).toBe(blockTreeReady); + }); + + it('routes a single-transaction block through the single-tx block root', () => { + const result = simulate('one transaction', [[[1]]], 16); + + const blockRoot = soleCompletion(result, BLOCK_ROOT_TYPES); + const base = soleCompletion(result, [ProvingRequestType.PRIVATE_TX_BASE_ROLLUP]); + + expect(blockRoot.type).toBe(ProvingRequestType.BLOCK_ROOT_SINGLE_TX_ROLLUP); + expect(completionsOf(result, [ProvingRequestType.TX_MERGE_ROLLUP])).toEqual([]); + expect(blockRoot.start).toBeGreaterThanOrEqual(base.end); + }); + + it('reduces a multi-transaction block before its block root', () => { + const result = simulate('multiple transactions', [[[4]]], 16); + + const blockRoot = soleCompletion(result, BLOCK_ROOT_TYPES); + const bases = completionsOf(result, [ProvingRequestType.PRIVATE_TX_BASE_ROLLUP]); + + expect(blockRoot.type).toBe(ProvingRequestType.BLOCK_ROOT_ROLLUP); + expect(bases).toHaveLength(4); + expect(blockRoot.start).toBeGreaterThanOrEqual(Math.max(...bases.map(b => b.end))); + }); + + it('proves zero-transaction blocks at any position in a checkpoint', () => { + const result = simulate('mixed empty blocks', [[[0], [2], [0], [1]]], 16); + + const blockRoots = completionsOf(result, BLOCK_ROOT_TYPES).sort((a, b) => a.block - b.block); + const parity = soleCompletion(result, [ProvingRequestType.INBOX_PARITY]); + const checkpointRoot = soleCompletion(result, CHECKPOINT_ROOT_TYPES); + + expect(blockRoots.map(r => [r.block, r.type])).toEqual([ + [1, ProvingRequestType.BLOCK_ROOT_NO_TXS_ROLLUP], + [2, ProvingRequestType.BLOCK_ROOT_ROLLUP], + [3, ProvingRequestType.BLOCK_ROOT_NO_TXS_ROLLUP], + [4, ProvingRequestType.BLOCK_ROOT_SINGLE_TX_ROLLUP], + ]); + // Neither empty block waits on parity or on the blocks around it. + expect(blockRoots.filter(r => r.type === ProvingRequestType.BLOCK_ROOT_NO_TXS_ROLLUP).map(r => r.start)).toEqual([ + 0, 0, + ]); + expect(checkpointRoot.start).toBe(Math.max(parity.end, blockTreeReadyAt(result, 1))); + }); +}); From 4ef9a730734b6a56a264a7d61180d26f303ff130 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:17:59 -0300 Subject: [PATCH 09/25] test: make three fixtures exercise what they claim to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The archiver re-mine fixture moved messages from block 100 to 120 without replacing any block identity, so it passed as an ordinary forward append and never reached recovery. It now re-mines the descendants too, and is split from the plain append case it was standing in for. Discarding unchanged work that moved outside the bounded lookup window is the accepted cost of rolling back before refetching, and the test says so. `findInsertingBlock` sampled a lower scan bound after observing the message, so the first block it found was where the search started rather than where the message was inserted. It now bisects the whole chain and confirms at both states — the block resolves a membership witness at the message's own index and its actual parent resolves none — retrying a genuine timing miss a bounded number of times. The rollup sample scenario splits 2 then 3 messages but captured `data[0]`, the first block with an empty inherited sponge. Samples are now selected by shape — inherited message count and bundle size — and the selector fails loudly when nothing, or more than one thing, matches. The committed sample still holds the old capture; correcting it needs the owning generator workflow, which was not run here. --- .../archiver/src/archiver-sync.test.ts | 48 +++++++- .../cross-chain/streaming_inbox.test.ts | 112 +++++++++++++----- .../regenerate_rollup_sample_inputs.test.ts | 76 ++++++++++-- 3 files changed, 194 insertions(+), 42 deletions(-) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index f66f2f76b0f1..e6b44f551240 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1604,23 +1604,21 @@ describe('Archiver Sync', () => { ); const randomLeaves = (count: number) => times(count, () => Fr.random()); - it('re-mines the same messages beyond the lookup window and appends new ones without touching proposed blocks', async () => { + it('appends new messages without disturbing the stored ones or the blocks that consumed them', async () => { const msgs = randomLeaves(3); fake.addMessages(CheckpointNumber(1), 100n, msgs); fake.setL1BlockNumber(110n); await archiver.syncImmediate(); await addLocalBlocksConsuming([3]); - // The messages move 20 L1 blocks later, well past the +-5 window a lookup around their old height covers, - // and two new ones follow them. - fake.moveMessagesToL1Block(100n, 120n); + // The L1 blocks holding the stored messages are untouched and two new messages follow them: a plain forward + // append, with nothing to look up or roll back. const appended = randomLeaves(2); fake.addMessages(CheckpointNumber(2), 121n, appended); fake.setL1BlockNumber(125n); await archiver.syncImmediate(); expect(await getStoredLeaves()).toEqual(asHex([...msgs, ...appended])); - // Unchanged content is a plain forward append; nothing needed to be looked up or pruned. expect(eventByHashSpy).not.toHaveBeenCalled(); expect(pruneSpy).not.toHaveBeenCalled(); expect(await localBlockNumbers()).toEqual([1]); @@ -1628,6 +1626,46 @@ describe('Archiver Sync', () => { expect(synchronizer.isRecoveringMessages()).toBe(false); }); + it('discards a block consuming unchanged messages that were re-mined beyond the lookup window', async () => { + // The finalized marker stays below the messages' L1 block, so the inherited-finality shortcut cannot anchor + // them without a lookup. + fake.setFinalizedL1BlockNumber(95n); + const msgs = randomLeaves(3); + fake.addMessages(CheckpointNumber(1), 100n, msgs); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + await addLocalBlocksConsuming([3]); + + // L1 replaces every block from 100 on. The three messages survive the replacement with their content, index + // and rolling hash intact, but are re-mined 20 blocks later, past the +-5 window a lookup around their old + // height covers, and two new messages follow them. + fake.moveMessagesToL1Block(100n, 120n); + const appended = randomLeaves(2); + fake.addMessages(CheckpointNumber(2), 121n, appended); + fake.reorgL1BlocksFrom(100n); + fake.setL1BlockNumber(125n); + await archiver.syncImmediate(); + + // Every bounded lookup misses, so the anchor falls back to the deployment block and the block that consumed + // the three messages is pruned even though they come straight back unchanged. That is the accepted cost of a + // conservative recovery, not a defect: the rollback precedes the refetch. + expect(eventByHashSpy).toHaveBeenCalledTimes(3); + expect(pruneSpy).toHaveBeenCalledTimes(1); + expect(pruneSpy).toHaveBeenCalledWith( + expect.objectContaining({ blocks: [expect.objectContaining({ number: 1 })] }), + ); + expect(await localBlockNumbers()).toEqual([]); + expect(await getStoredLeaves()).toEqual(asHex([...msgs, ...appended])); + expect(archiver.getL1BlockNumber()).toEqual(125n); + expect(synchronizer.isRecoveringMessages()).toBe(false); + + // Syncing again at the same head refetches from L1 rather than resurrecting the pruned block. + await archiver.syncImmediate(); + expect(await getStoredLeaves()).toEqual(asHex([...msgs, ...appended])); + expect(await localBlockNumbers()).toEqual([]); + expect(pruneSpy).toHaveBeenCalledTimes(1); + }); + it('rolls back to the newest message still found on L1 and re-fetches the rest, dropping unchanged work', async () => { const [a, b, c, d] = randomLeaves(4); fake.addMessages(CheckpointNumber(1), 100n, [a, b]); diff --git a/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts b/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts index e9f2162c62d3..7f41846065aa 100644 --- a/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts +++ b/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts @@ -95,30 +95,86 @@ describe('single-node/cross-chain/streaming_inbox', () => { 0.1, ); + /** Leaves in a block's committed L1-to-L2 message tree; genesis holds none and an unknown block reports none. */ + const committedMessageCount = async (blockNumber: number): Promise => { + if (blockNumber <= 0) { + return 0n; + } + const data = await aztecNode.getBlockData(BlockNumber(blockNumber)); + return data && BigInt(data.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); + }; + /** - * Finds the L2 block that inserted `msgHash` into the L1-to-L2 message tree by scanning forward from - * `fromBlock` for the first block whose committed tree resolves a membership witness. Under the streaming - * Inbox a message enters the tree at the first block the proposer builds after its archiver observed the - * message, which need not be the first block of a checkpoint. Returns the block-data (checkpoint number + index - * within checkpoint) of that block. + * Finds the L2 block that inserted `msgHash` into the L1-to-L2 message tree. Under the streaming Inbox a message + * enters the tree at the first block the proposer builds after its archiver observed it, which need not be the + * first block of a checkpoint. Returns the block-data (checkpoint number + index within checkpoint) of that block. + * + * The tree is append-only, so every block built after the insertion resolves a membership witness for the message + * just as the inserting one does: retaining membership is not inserting it, and scanning forward from a block + * number sampled by the caller reports where the search started whenever the message was already inserted by then. + * The block is instead located by the message's compact leaf index against the committed leaf count, which grows + * monotonically along the chain: the inserting block is the first whose count is past the index, found by bisecting + * the whole chain rather than trusting any sampled bound. + * + * The result is then confirmed at both states: the block's parent has no membership witness for the message and + * the block itself resolves one at the message's own compact index. A chain that moves under the search (the tip + * advancing, a prune) fails that confirmation, which is a genuine timing miss and is retried. */ - const findInsertingBlock = (msgHash: Fr, fromBlock: BlockNumber) => { - return retryUntil( - async () => { - const tip = await aztecNode.getBlockNumber(); - for (let n = fromBlock; n <= tip; n = BlockNumber(n + 1)) { - const witness = await aztecNode.getL1ToL2MessageMembershipWitness(n, msgHash); - if (witness !== undefined) { - const data = await aztecNode.getBlockData(n); - return { blockNumber: n, checkpointNumber: data!.checkpointNumber, index: data!.indexWithinCheckpoint }; - } + const findInsertingBlock = async (msgHash: Fr) => { + const attempts = 3; + for (let attempt = 0; attempt < attempts; attempt++) { + const { leafIndex } = await retryUntil( + async () => { + const index = await aztecNode.getL1ToL2MessageIndex(msgHash); + return index === undefined ? undefined : { leafIndex: index }; + }, + `node assigns a compact index to message ${msgHash.toString()}`, + 240, + 0.5, + ); + + // The chain holds the message once its tip's tree has grown past the message's index. + const { tip } = await retryUntil( + async () => { + const tip = await aztecNode.getBlockNumber(); + const count = await committedMessageCount(tip); + return count !== undefined && count > leafIndex ? { tip } : undefined; + }, + `a block committing message ${msgHash.toString()}`, + 240, + 0.5, + ); + + // Bisect for the first block past the index: genesis holds no messages, the tip holds this one. + let below = 0; + let holding = Number(tip); + while (holding - below > 1) { + const middle = below + Math.floor((holding - below) / 2); + const count = await committedMessageCount(middle); + if (count !== undefined && count > leafIndex) { + holding = middle; + } else { + below = middle; } - return undefined; - }, - `find block inserting message ${msgHash.toString()}`, - 240, - 0.5, - ); + } + + const blockNumber = BlockNumber(holding); + const data = await aztecNode.getBlockData(blockNumber); + const witness = await aztecNode.getL1ToL2MessageMembershipWitness(blockNumber, msgHash); + const parentWitness = + below === 0 ? undefined : await aztecNode.getL1ToL2MessageMembershipWitness(BlockNumber(below), msgHash); + if (data !== undefined && witness !== undefined && witness[0] === leafIndex && parentWitness === undefined) { + return { blockNumber, checkpointNumber: data.checkpointNumber, index: data.indexWithinCheckpoint }; + } + log.warn(`Block ${blockNumber} did not confirm as the one inserting ${msgHash.toString()}; searching again`, { + attempt, + leafIndex, + hasBlockData: data !== undefined, + resolvedIndex: witness?.[0], + parentHoldsMessage: parentWitness !== undefined, + }); + } + throw new Error(`Could not confirm which block inserted message ${msgHash.toString()} in ${attempts} attempts`); }; /** @@ -188,7 +244,7 @@ describe('single-node/cross-chain/streaming_inbox', () => { // The background feeder drives block production; findInsertingBlock polls the committed tree without // sending its own wallet txs (which would race the feeder on the nonce). - const found = await findInsertingBlock(msgHash, BlockNumber(blockAtSend + 1)); + const found = await findInsertingBlock(msgHash); log.warn(`Message ${msgHash.toString()} inserted at block ${found.blockNumber}`, { checkpointNumber: found.checkpointNumber, index: found.index, @@ -227,7 +283,6 @@ describe('single-node/cross-chain/streaming_inbox', () => { const maxDelaySeconds = BigInt(t.constants.ethereumSlotDuration) + 2n * BigInt(slotDuration); await withBackgroundFeeder(async () => { - const blockAtSend = await aztecNode.getBlockNumber(); const wallClockAtSend = Date.now(); const [, secretHash] = await generateClaimSecret(); const message = { recipient: testContract.address, content: Fr.random(), secretHash }; @@ -237,7 +292,7 @@ describe('single-node/cross-chain/streaming_inbox', () => { // The background feeder drives block production; findInsertingBlock polls the committed tree without // sending its own wallet txs (which would race the feeder on the nonce). - const inserting = await findInsertingBlock(msgHash, BlockNumber(blockAtSend + 1)); + const inserting = await findInsertingBlock(msgHash); const wallClockLatencyMs = Date.now() - wallClockAtSend; const insertingBlock = (await aztecNode.getBlock(inserting.blockNumber))!; const includingBlockTs = insertingBlock.header.globalVariables.timestamp; @@ -270,7 +325,6 @@ describe('single-node/cross-chain/streaming_inbox', () => { 0.5, ); - const blockAtSend = await aztecNode.getBlockNumber(); const [, secretHash] = await generateClaimSecret(); const message = { recipient: testContract.address, content: Fr.random(), secretHash }; const { msgHash } = await sendMessageToL2(message); @@ -279,7 +333,7 @@ describe('single-node/cross-chain/streaming_inbox', () => { // Do not feed txs; the sequencer builds empty checkpoints until the message ages past the lag, at which // point a zero-tx block consumes it. findInsertingBlock polls the committed tree without sending txs, so // the pool stays empty and the block that consumes the message carries only the bundle. - const inserting = await findInsertingBlock(msgHash, BlockNumber(blockAtSend + 1)); + const inserting = await findInsertingBlock(msgHash); const insertingBlock = (await aztecNode.getBlock(inserting.blockNumber, { includeTransactions: true }))!; log.warn(`Message ${msgHash.toString()} inserted at block ${inserting.blockNumber}`, { @@ -313,14 +367,13 @@ describe('single-node/cross-chain/streaming_inbox', () => { // double-spend revert. Mirrors cross_chain_public_message.test.ts. it('consumes a streaming-inserted message by compact index and rejects double-spend', async () => { const l1Account = t.ethAccount; - const blockAtSend = await aztecNode.getBlockNumber(); const [secret, secretHash] = await generateClaimSecret(); const message = { recipient: testContract.address, content: Fr.random(), secretHash }; const { msgHash, globalLeafIndex } = await sendMessageToL2(message); log.warn(`Sent message ${msgHash.toString()} with compact index ${globalLeafIndex}`); await waitForMessageReady(msgHash, 'public'); - const inserting = await findInsertingBlock(msgHash, BlockNumber(blockAtSend + 1)); + const inserting = await findInsertingBlock(msgHash); const { receipt: txReceipt } = await testContract.methods .consume_message_from_arbitrary_sender_public(message.content, secret, l1Account, globalLeafIndex.toBigInt()) @@ -365,12 +418,11 @@ describe('single-node/cross-chain/streaming_inbox', () => { // Do not drive L2 blocks while waiting: an extra block here only shifts the timing of the inserting block. // The archiver polls L1 within an Ethereum slot; no later L1 block is needed for the message to be usable. await waitForMessageObserved(msgHash); - const blockAtSend = await aztecNode.getBlockNumber(); const { receipt } = await testContract.methods .consume_message_from_arbitrary_sender_public(message.content, secret, l1Account, globalLeafIndex.toBigInt()) .send({ from: user1Address, wait: { dontThrowOnRevert: true } }); - const inserting = await findInsertingBlock(msgHash, BlockNumber(blockAtSend + 1)); + const inserting = await findInsertingBlock(msgHash); const consumeBlock = BlockNumber(Number(receipt.blockNumber)); log.warn(`Consume tx for ${msgHash.toString()} landed in block ${consumeBlock}`, { insertingBlock: inserting.blockNumber, diff --git a/yarn-project/prover-client/src/test/regenerate_rollup_sample_inputs.test.ts b/yarn-project/prover-client/src/test/regenerate_rollup_sample_inputs.test.ts index 79059cc680af..d864d6adbbf9 100644 --- a/yarn-project/prover-client/src/test/regenerate_rollup_sample_inputs.test.ts +++ b/yarn-project/prover-client/src/test/regenerate_rollup_sample_inputs.test.ts @@ -37,13 +37,23 @@ import { type CheckpointTopTreeData, TopTreeOrchestrator } from '../orchestrator // before the (two-input) block root, whereas one- or two-tx blocks feed the block root directly, so a // dedicated three-tx scenario is what regenerates the tx-merge sample. The samples for the variants // that thread a start sponge from a previous block are taken from mid-checkpoint blocks, so those -// scenarios need a per-block message distribution. Every scenario also produces the root rollup. +// scenarios need a per-block message distribution, and they declare which block they mean: blocks are +// proven concurrently, so the order the inputs were captured in does not identify them. Every scenario +// also produces the root rollup. const describeOrSkip = isGenerateTestDataEnabled() ? describe : describe.skip; describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { let context: TestContext; let log: Logger; + /** + * How many L1-to-L2 messages a block inherits from earlier blocks of its checkpoint (its start sponge) and how + * many its own bundle inserts. A scenario that runs a block-root circuit once per block identifies the run it + * means by this shape, which is a property of the block itself, rather than by the order the inputs happened to + * be captured in. + */ + type MessageShape = { inherited: number; bundle: number }; + interface Scenario { numCheckpoints: number; numBlocksPerCheckpoint: number; @@ -56,6 +66,8 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { l1ToL2MessagesPerBlock?: Fr[][]; /** Circuits whose sample inputs this scenario is responsible for regenerating. */ dump: CircuitName[]; + /** The block whose inputs each dumped block-root sample must be taken from. */ + sampleFrom?: Partial>; } // `makeCheckpoint` puts the scenario's whole message list into the first block, so the most a @@ -76,6 +88,9 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { numTxsPerBlock: 1, numL1ToL2Messages: withMessages, dump: ['rollup-block-root-single-tx', 'rollup-block-merge', 'rollup-checkpoint-root'], + // All the messages go to the first block, so the single-tx sample is the block that carries the bundle; the + // two blocks after it insert nothing and only inherit the sponge. + sampleFrom: { 'rollup-block-root-single-tx': { inherited: 0, bundle: withMessages } }, }, // Messages split across both blocks so the block-root sample is taken from a mid-checkpoint block with a // non-empty bundle, exercising the per-block sponge continuity asserts in the circuit. @@ -86,6 +101,9 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { numL1ToL2Messages: 0, // Overridden by l1ToL2MessagesPerBlock. l1ToL2MessagesPerBlock: [times(2, i => new Fr(0xb00 + i)), times(3, i => new Fr(0xc00 + i))], dump: ['rollup-block-root'], + // The second block: it inherits the two messages the first block inserted and inserts three of its own, so + // its start sponge is a continuation rather than the initial empty one. + sampleFrom: { 'rollup-block-root': { inherited: 2, bundle: 3 } }, }, // Three txs in a block force a tx-merge to pair the base proofs down to the two the block root // takes; one- or two-tx blocks feed the block root directly and never exercise tx-merge. @@ -105,6 +123,7 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { numL1ToL2Messages: 0, // Overridden by l1ToL2MessagesPerBlock. l1ToL2MessagesPerBlock: [times(2, i => new Fr(0x900 + i)), times(3, i => new Fr(0xa00 + i))], dump: ['rollup-block-root-no-txs'], + sampleFrom: { 'rollup-block-root-no-txs': { inherited: 2, bundle: 3 } }, }, // The checkpoint-merge only appears with three checkpoints. Independently-built checkpoints do // not carry the inbox message state forward, so this scenario runs with no L1-to-L2 messages and @@ -118,6 +137,51 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { }, ]; + /** The message shape of a captured input, or undefined for circuits that take no message bundle. */ + const messageShapeOf = (captured: unknown): MessageShape | undefined => { + const { inputs } = (captured ?? {}) as { + inputs?: { message_bundle?: { num_msgs?: string }; start_msg_sponge?: { num_absorbed?: string } }; + }; + const bundle = inputs?.message_bundle?.num_msgs; + const inherited = inputs?.start_msg_sponge?.num_absorbed; + return bundle === undefined || inherited === undefined + ? undefined + : { inherited: Number(BigInt(inherited)), bundle: Number(BigInt(bundle)) }; + }; + + /** + * Picks the one captured input a scenario means to commit for `circuitName`. A circuit the scenario runs once has + * a single candidate; one it runs per block is identified by the message shape the scenario declared. Nothing + * captured, several runs with no declared shape, or a declared shape matching no run or more than one all throw: + * committing whichever input happened to be captured first would silently pin the wrong block. + */ + const selectSample = (circuitName: CircuitName, wanted: MessageShape | undefined): unknown => { + const captured = getTestData(circuitName) ?? []; + const shapes = () => captured.map(entry => JSON.stringify(messageShapeOf(entry) ?? 'no bundle')).join(', '); + if (captured.length === 0) { + throw new Error(`No test data captured for ${circuitName}; scenario does not exercise it.`); + } + if (wanted === undefined) { + if (captured.length > 1) { + throw new Error( + `${circuitName} ran ${captured.length} times (${shapes()}); declare in sampleFrom which run to commit.`, + ); + } + return captured[0]; + } + const matching = captured.filter(entry => { + const shape = messageShapeOf(entry); + return shape?.inherited === wanted.inherited && shape.bundle === wanted.bundle; + }); + if (matching.length !== 1) { + throw new Error( + `Expected exactly one ${circuitName} run inheriting ${wanted.inherited} messages and inserting ` + + `${wanted.bundle}, found ${matching.length} of ${captured.length} runs (${shapes()}).`, + ); + } + return matching[0]; + }; + beforeEach(async () => { log = createLogger('prover-client:test:regenerate-rollup-sample-inputs'); context = await TestContext.new(log, { proverCount: 1 }); @@ -136,6 +200,7 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { numL1ToL2Messages, l1ToL2MessagesPerBlock, dump, + sampleFrom, }) => { const makeProcessedTxOpts = (_: unknown, txIndex: number) => ({ privateOnly: txIndex % 2 === 0 }); const checkpoints = await timesAsync(numCheckpoints, () => @@ -204,12 +269,9 @@ describeOrSkip('prover/regenerate-rollup-sample-inputs', () => { } for (const circuitName of dump) { - const data = getTestData(circuitName); - if (!data || data.length === 0) { - throw new Error(`No test data captured for ${circuitName}; scenario does not exercise it.`); - } - updateProtocolCircuitSampleInputs(circuitName, TOML.stringify(data[0] as any)); - log.info(`Regenerated sample inputs for ${circuitName}`); + const sample = selectSample(circuitName, sampleFrom?.[circuitName]); + updateProtocolCircuitSampleInputs(circuitName, TOML.stringify(sample as any)); + log.info(`Regenerated sample inputs for ${circuitName}`, messageShapeOf(sample)); } } finally { await Promise.all(subTrees.map(s => s.stop())); From 198f554ac9535cacf60b77429f52b730182eea9e Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 18:13:19 -0300 Subject: [PATCH 10/25] fix(validator): separate an unexpected Inbox range-read failure from ordinary sync lag Every failure of the bundle range read yields the same non-punitive verdict, but a store fault or a broken provider is not the same thing as a node that is behind L1. Unexpected failures are now reported with bounded structured context while genuine lag stays at debug, so an operator can tell them apart without changing what the validator does with the proposal. The classification matches on message text, since the archiver's typed range error does not survive a JSON-RPC hop. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/streaming_inbox_checks.ts | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/yarn-project/validator-client/src/streaming_inbox_checks.ts b/yarn-project/validator-client/src/streaming_inbox_checks.ts index d509ac376ccd..8078c962e15a 100644 --- a/yarn-project/validator-client/src/streaming_inbox_checks.ts +++ b/yarn-project/validator-client/src/streaming_inbox_checks.ts @@ -1,6 +1,9 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; +import { createLogger } from '@aztec/foundation/log'; import type { InboxMessagePrefixRef, L1ToL2MessageSource } from '@aztec/stdlib/messaging'; +const log = createLogger('validator-client:streaming_inbox_checks'); + /** * Reason a streaming-Inbox block proposal fails the per-block acceptance checks. Follows the handler's existing * `{ isValid, reason }` string style. @@ -179,6 +182,10 @@ export async function checkStreamingBlockProposalMetadata( * same local-view condition as a missing prefix hash and the caller retries both the same way. A replacement that * lands *after* this read is not this function's race: the bundle and the proposal still agree, so re-execution is * consistent, and the archiver's insert guard is what refuses to store the block. + * + * Every failure of the range read is non-punitive, but not every failure is ordinary sync lag: a store fault or a + * broken provider surfaces here as the same verdict. {@link logRangeReadFailure} separates the two in the logs so an + * operator can tell them apart without changing what the validator does with the proposal. */ export async function readStreamingBlockBundle( messageSource: Pick, @@ -193,7 +200,8 @@ export async function readStreamingBlockBundle( messages, end: { rollingHash: endRollingHash }, } = await messageSource.getL1ToL2MessageRange(parentTotalMsgCount, endTotalMsgCount)); - } catch { + } catch (err) { + logRangeReadFailure(parentTotalMsgCount, endTotalMsgCount, err); return { accepted: false, reason: 'inbox_prefix_unavailable' }; } @@ -204,6 +212,53 @@ export async function readStreamingBlockBundle( return { accepted: true, bundle: messages }; } +/** + * Message fragments that identify an ordinary local-view shortfall from the archiver's range read: the range reaches + * past what this node has synced, or its bounds are not a valid range at all. Matched on the message text rather than + * on the error's class, since a JSON-RPC hop between the validator and its message source leaves the class behind + * while preserving the message. + */ +const EXPECTED_RANGE_READ_FAILURES = ['is not fully synced', 'Invalid Inbox leaf count range']; + +/** Upper bound on the error text carried into a log line, so a verbose provider error cannot blow up a log record. */ +const MAX_LOGGED_ERROR_LENGTH = 200; + +/** Ranges whose unexpected read failure has already been reported, so the caller's retry loop does not repeat it. */ +const reportedRangeReadFailures = new Set(); + +/** Cap on {@link reportedRangeReadFailures}, which is cleared wholesale once reached rather than evicted per entry. */ +const MAX_REPORTED_RANGE_READ_FAILURES = 256; + +/** + * Reports a failed Inbox range read. A range the archiver has not synced is the expected outcome for a node behind + * L1 and is logged at debug; anything else means the message source failed for a reason this check did not anticipate + * and is logged once per range and error, at warn. The verdict handed back to the caller is the same either way, so + * this only changes what an operator can see, never whether a proposal is penalised. + */ +function logRangeReadFailure(startTotalMsgCount: bigint, endTotalMsgCount: bigint, err: unknown): void { + const message = err instanceof Error ? err.message : String(err); + const error = message.slice(0, MAX_LOGGED_ERROR_LENGTH); + const context = { startTotalMsgCount, endTotalMsgCount, error }; + if (EXPECTED_RANGE_READ_FAILURES.some(fragment => message.includes(fragment))) { + log.debug(`Inbox message range [${startTotalMsgCount}, ${endTotalMsgCount}) is not available locally`, context); + return; + } + + const key = `${startTotalMsgCount}-${endTotalMsgCount}-${error}`; + if (reportedRangeReadFailures.has(key)) { + return; + } + if (reportedRangeReadFailures.size >= MAX_REPORTED_RANGE_READ_FAILURES) { + reportedRangeReadFailures.clear(); + } + reportedRangeReadFailures.add(key); + log.warn( + `Inbox message range [${startTotalMsgCount}, ${endTotalMsgCount}) could not be read; treating the block's ` + + `prefix as unconfirmed`, + context, + ); +} + /** * Runs the per-block acceptance checks a validator applies to a streaming block proposal, and derives the * message-leaf bundle the block consumes. Composes {@link checkStreamingBlockProposalMetadata} with From 5397c7d61fc609cd0cc67b8f27631efe6685b6ff Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:20:46 -0300 Subject: [PATCH 11/25] refactor: drop unused streaming Inbox plumbing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Behavior-preserving reductions alongside the code they belong to: - The pre-gossip bucket hint was threaded through the broadcast result and never read; the publication preflight resolves its own hint fresh. Removed, along with the fisherman dummy that existed to fill it. - Selecting a bundle that ends exactly on the resolved endpoint reuses the range the resolver already read and checked, instead of reading the same prefix again. A shorter or longer end still gets its own read, so no endpoint hash is ever carried into a different prefix. - Recovery state carried a finalized L1 block that nothing consumed, and a synced-status downgrade after continuing recovery that could not be reached — continuation only ever returns pending. - The pending-checkpoint override field types are derived from the log-override type that owns them rather than restated, and two consumed-message-count call sites reuse the existing leaf-count helper. - The Noir fixed-message-subtree fixture builder and its root helper had no callers left. --- .../src/tests/rollup_fixture_builder.nr | 52 +------------------ .../src/modules/data_store_updater.ts | 2 +- .../src/modules/inbox_message_synchronizer.ts | 27 ++++------ .../archiver/src/modules/l1_synchronizer.ts | 4 +- .../src/contracts/chain_state_override.ts | 45 ++++------------ .../src/sequencer/checkpoint_proposal_job.ts | 38 +++++--------- 6 files changed, 38 insertions(+), 130 deletions(-) diff --git a/noir-projects/fnd/noir-protocol-circuits/crates/rollup-lib/src/tests/rollup_fixture_builder.nr b/noir-projects/fnd/noir-protocol-circuits/crates/rollup-lib/src/tests/rollup_fixture_builder.nr index 6ca236b27c8f..78cedab31afc 100644 --- a/noir-projects/fnd/noir-protocol-circuits/crates/rollup-lib/src/tests/rollup_fixture_builder.nr +++ b/noir-projects/fnd/noir-protocol-circuits/crates/rollup-lib/src/tests/rollup_fixture_builder.nr @@ -28,7 +28,7 @@ use types::{ }, hash::{poseidon2_hash, poseidon2_hash_with_separator}, merkle_tree::{ - append_only_tree::append_leaves_to_snapshot, compute_empty_tree_root, MerkleTree, + append_only_tree::append_leaves_to_snapshot, MerkleTree, test_utils::SingleSubtreeMerkleTree, }, proof::proof_data::{AvmV2ProofData, ProofData}, @@ -139,52 +139,6 @@ impl RollupFixtureBuilder { (previous_snapshot, sibling_path_for_insertion) } - pub fn build_l1_to_l2_message_subtree_for_insertion( - self, - slot_number: Field, - ) -> (AppendOnlyTreeSnapshot, [Field; L1_TO_L2_MSG_TREE_HEIGHT - L1_TO_L2_MSG_SUBTREE_HEIGHT], AppendOnlyTreeSnapshot) { - // Create a subtree of size 4 filled with subtree roots before the current slot. - let new_slot_index = self.get_l1_to_l2_message_next_available_leaf_index(slot_number - 1) - as u32 - >> L1_TO_L2_MSG_SUBTREE_HEIGHT; - let start_slot_index = new_slot_index - new_slot_index % 4; - let mut subtree_roots = [0; 4]; - for i in start_slot_index..new_slot_index { - subtree_roots[i % 4] = - self.get_l1_to_l2_message_subtree_root(slot_number - 4 + i as Field); - } - let empty_subtree_root = compute_empty_tree_root::(); - subtree_roots[new_slot_index % 4] = empty_subtree_root; - - let mut tree = SingleSubtreeMerkleTree::<4, 2, L1_TO_L2_MSG_TREE_HEIGHT - L1_TO_L2_MSG_SUBTREE_HEIGHT>::new_tree_roots_at_index::( - subtree_roots, - start_slot_index as Field, - ); - let previous_snapshot = AppendOnlyTreeSnapshot { - root: tree.get_root(), - next_available_leaf_index: self.get_l1_to_l2_message_next_available_leaf_index( - slot_number - 1, - ), - }; - - // Get the sibling path for inserting the current l1-to-l2 messages. - let sibling_path_for_insertion = tree.get_sibling_path(new_slot_index as Field); - - // Insert subtree of the current l1-to-l2 messages. - tree.update_leaf( - new_slot_index as Field, - self.get_l1_to_l2_message_subtree_root(slot_number), - ); - let new_snapshot = AppendOnlyTreeSnapshot { - root: tree.get_root(), - next_available_leaf_index: self.get_l1_to_l2_message_next_available_leaf_index( - slot_number, - ), - }; - - (previous_snapshot, sibling_path_for_insertion, new_snapshot) - } - /// Builds a per-block L1-to-L2 message bundle appended to an empty tree, returning /// `(previous_snapshot, leaves, frontier_hint, new_snapshot)`. The resulting `new_snapshot` is exactly what the /// block root circuit recomputes from the same inputs. @@ -588,10 +542,6 @@ impl RollupFixtureBuilder { } } - fn get_l1_to_l2_message_subtree_root(_self: Self, slot_number: Field) -> Field { - slot_number * 7639901 - } - fn get_l1_to_l2_message_next_available_leaf_index(_self: Self, slot_number: Field) -> Field { (slot_number + 1) * (1 << L1_TO_L2_MSG_SUBTREE_HEIGHT) as Field } diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index f69f4d239162..898208c1246f 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -407,7 +407,7 @@ export class ArchiverDataStoreUpdater { if (block === undefined || block.header.getBlockNumber() !== blockNumber) { throw new Error(`Block ${blockNumber} is missing from the store`); } - return BigInt(block.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); + return blockLeafCount(block); } /** diff --git a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts index 68149e2092f0..c8cb2eb8759e 100644 --- a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts +++ b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts @@ -37,7 +37,6 @@ type RecoveryState = { head: L1BlockId; /** The Inbox's position at `head`. */ remote: InboxContractState; - finalizedL1Block: L1BlockId | undefined; /** The next stored message to look up on L1, or undefined once the search has run out of candidates. */ nextCandidateIndex: bigint | undefined; /** Number of per-message event lookups made so far, for progress reporting. */ @@ -176,12 +175,9 @@ export class InboxMessageSynchronizer { private async syncPass(head: L1BlockId, finalizedL1Block: L1BlockId | undefined): Promise { if (this.recovery !== undefined) { - const pinnedHead = this.recovery.head; - const pinnedStatus = await this.checkL1Block(pinnedHead); + const pinnedStatus = await this.checkL1Block(this.recovery.head); if (pinnedStatus === 'canonical') { - const result = await this.continueRecovery(); - // Recovery is complete relative to the head it was pinned to; blocks after it still need normal ingestion. - return result.status === 'synced' && !sameL1Block(pinnedHead, head) ? { ...result, status: 'pending' } : result; + return await this.continueRecovery(); } if (pinnedStatus === 'unknown') { // The pinned head could not be read. That is a provider problem, not evidence its chain is gone: keep the @@ -245,14 +241,14 @@ export class InboxMessageSynchronizer { } return this.truncate(localAtRemote, head, finalizedL1Block); } - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } const ingestFrom = this.ingestionStartFor(cursor); if (head.l1BlockNumber < ingestFrom) { // A head below the first block still to be scanned, and the log does not agree with it: there is no forward // range to fetch, so find where the local log and the canonical one part ways. - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } // Forward ingestion inherits the scanned log as canonical, and the head batch's comparison then certifies that @@ -277,7 +273,7 @@ export class InboxMessageSynchronizer { syncPoint: persistedSyncPoint, headL1BlockNumber: head.l1BlockNumber, }); - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } } @@ -293,7 +289,7 @@ export class InboxMessageSynchronizer { this.log.warn(`Fetched L1 to L2 messages do not continue the local log: ${err.message}`, { inboxMessage: err.inboxMessage, }); - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } throw err; } @@ -316,13 +312,13 @@ export class InboxMessageSynchronizer { this.log.warn(`Head batch of L1 to L2 messages does not continue the local log: ${err.message}`, { inboxMessage: err.inboxMessage, }); - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } throw err; } return synced(); } - return this.startRecovery(head, remote, finalizedL1Block); + return this.startRecovery(head, remote); } /** @@ -420,11 +416,7 @@ export class InboxMessageSynchronizer { } } - private async startRecovery( - head: L1BlockId, - remote: InboxContractState, - finalizedL1Block: L1BlockId | undefined, - ): Promise { + private async startRecovery(head: L1BlockId, remote: InboxContractState): Promise { const local = await this.stores.messages.getSyncedMessagePosition(); // Messages past the canonical count cannot be on the canonical chain at their index, so the search for a common // message starts at the canonical tip or the local one, whichever is lower. @@ -432,7 +424,6 @@ export class InboxMessageSynchronizer { this.recovery = { head, remote, - finalizedL1Block, nextCandidateIndex: lastCandidate < 0n ? undefined : lastCandidate, lookups: 0, startedAt: new Timer(), diff --git a/yarn-project/archiver/src/modules/l1_synchronizer.ts b/yarn-project/archiver/src/modules/l1_synchronizer.ts index 93d15e109d37..59cb2852d991 100644 --- a/yarn-project/archiver/src/modules/l1_synchronizer.ts +++ b/yarn-project/archiver/src/modules/l1_synchronizer.ts @@ -42,7 +42,7 @@ import { import type { RejectedCheckpoint } from '../store/block_store.js'; import { type ArchiverDataStores, getArchiverSynchPoint } from '../store/data_stores.js'; import type { L2TipsCache } from '../store/l2_tips_cache.js'; -import { ArchiverDataStoreUpdater } from './data_store_updater.js'; +import { ArchiverDataStoreUpdater, blockLeafCount } from './data_store_updater.js'; import { InboxMessageSynchronizer } from './inbox_message_synchronizer.js'; import type { ArchiverInstrumentation } from './instrumentation.js'; import { validateCheckpointAttestationsFromCalldata } from './validation.js'; @@ -549,7 +549,7 @@ export class ArchiverL1Synchronizer implements Traceable { if (lastBlock === undefined || lastBlock.header.getBlockNumber() !== lastBlockNumber) { return false; } - const consumedCount = BigInt(lastBlock.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); + const consumedCount = blockLeafCount(lastBlock); const position = await this.stores.messages.getMessagePosition(consumedCount); return position !== undefined && position.rollingHash.equals(checkpoint.header.inboxRollingHash); } diff --git a/yarn-project/ethereum/src/contracts/chain_state_override.ts b/yarn-project/ethereum/src/contracts/chain_state_override.ts index b17d841b0683..dfe03b814a79 100644 --- a/yarn-project/ethereum/src/contracts/chain_state_override.ts +++ b/yarn-project/ethereum/src/contracts/chain_state_override.ts @@ -1,29 +1,20 @@ import { toHex as toPaddedHex } from '@aztec/foundation/bigint-buffer'; -import type { CheckpointNumber, SlotNumber } from '@aztec/foundation/branded-types'; -import type { Buffer32 } from '@aztec/foundation/buffer'; +import type { CheckpointNumber } from '@aztec/foundation/branded-types'; import { merge } from '@aztec/foundation/collection'; import type { Fr } from '@aztec/foundation/curves/bn254'; import type { StateOverride } from 'viem'; -import { type FeeHeader, RollupContract } from './rollup.js'; +import { type FeeHeader, RollupContract, type TempCheckpointLogOverrideFields } from './rollup.js'; /** - * Override values for the pending checkpoint that the simulation should treat as already applied. - * Every field is optional at plan-building time so callers can populate them incrementally; whatever - * is present at translation time is forwarded to the partial `tempCheckpointLogs` helper so the - * load-bearing `slotNumber` can land even if other fields could not be derived locally. + * Override values for the pending checkpoint that the simulation should treat as already applied: the + * `tempCheckpointLogs` cell fields plus the archive root, which lives in its own mapping. Every field is optional at + * plan-building time so callers can populate them incrementally; whatever is present at translation time is forwarded + * to the partial `tempCheckpointLogs` helper so the required `slotNumber` can land even if other fields could not be + * derived locally. */ -export type PendingCheckpointOverrideState = { - archive?: Fr; - feeHeader?: FeeHeader; - headerHash?: Fr; - outHash?: Fr; - payloadDigest?: Buffer32; - slotNumber?: SlotNumber; - inboxMsgTotal?: bigint; - inboxConsumedBucket?: bigint; -}; +export type PendingCheckpointOverrideState = TempCheckpointLogOverrideFields & { archive?: Fr }; export type ChainTipsOverride = { pending?: CheckpointNumber; @@ -105,14 +96,7 @@ export class SimulationOverridesBuilder { * back to 0 and the contract treats the pending tip as belonging to epoch 0, triggering a phantom * prune that silently undoes the `pending` override. */ - public withPendingTempCheckpointLogFields(fields: { - headerHash?: Fr; - outHash?: Fr; - payloadDigest?: Buffer32; - slotNumber?: SlotNumber; - inboxMsgTotal?: bigint; - inboxConsumedBucket?: bigint; - }): this { + public withPendingTempCheckpointLogFields(fields: Omit): this { this.assertPendingCheckpointNumber(); this.pendingCheckpointState = { ...(this.pendingCheckpointState ?? {}), ...fields }; return this; @@ -172,17 +156,10 @@ export async function buildSimulationOverridesStateOverride( } if (plan.pendingCheckpointState) { + // `archive` rides along in the same state but lives in its own mapping, and the temp-log helper ignores it. rollupStateDiff.push( ...extractRollupStateDiff( - await rollup.makeTempCheckpointLogOverride(plan.chainTipsOverride!.pending!, { - headerHash: plan.pendingCheckpointState.headerHash, - outHash: plan.pendingCheckpointState.outHash, - payloadDigest: plan.pendingCheckpointState.payloadDigest, - slotNumber: plan.pendingCheckpointState.slotNumber, - inboxMsgTotal: plan.pendingCheckpointState.inboxMsgTotal, - inboxConsumedBucket: plan.pendingCheckpointState.inboxConsumedBucket, - feeHeader: plan.pendingCheckpointState.feeHeader, - }), + await rollup.makeTempCheckpointLogOverride(plan.chainTipsOverride!.pending!, plan.pendingCheckpointState), ), ); } diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index f603c03be386..bbc03889b9f3 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -120,11 +120,6 @@ type CheckpointProposalBroadcast = { checkpoint: Checkpoint; proposal: CheckpointProposal; blockProposedAt: number; - /** - * Sequence number of the live Inbox bucket the checkpoint's final message position resolved to at the pre-gossip - * preflight: the unsigned L1 `propose` lookup aid. Re-resolved by the pre-publication preflight before the send. - */ - bucketHint: bigint; /** The checkpoint's final streaming state, for the pre-publication preflight. */ streamingState: StreamingCheckpointState; }; @@ -1102,14 +1097,8 @@ export class CheckpointProposalJob implements Traceable { ); this.metrics.recordCheckpointSuccess(); // Return a broadcast result with a dummy proposal — fisherman mode skips attestation collection and never - // publishes, so the bucket hint is never read. - return { - checkpoint, - proposal: undefined!, - blockProposedAt: this.dateProvider.now(), - bucketHint: 0n, - streamingState, - }; + // publishes. + return { checkpoint, proposal: undefined!, blockProposedAt: this.dateProvider.now(), streamingState }; } // Validate the header and the Inbox consumption against L1 state before broadcasting: the parent the header @@ -1120,9 +1109,8 @@ export class CheckpointProposalJob implements Traceable { // The simulation is bounded by the proposal send deadline, not the attestation deadline: a verdict arriving // once peers have stopped accepting proposals for this slot must not lead to signing and gossiping one. const sendDeadline = this.getProposalSendDeadline(); - let bucketHint: bigint; try { - bucketHint = await this.preflightWithinDeadline( + await this.preflightWithinDeadline( checkpoint.header, streamingState, this.checkpointSimulationOverridesPlan, @@ -1195,10 +1183,8 @@ export class CheckpointProposalJob implements Traceable { this.checkpointMetrics.noteCheckpointBroadcast(this.dateProvider.now()); } - // Return immediately after broadcast — attestation collection happens in the background. The bucket hint is - // the live bucket the preflight resolved the header's final position to, whether or not a last block was held - // for broadcast. - return { checkpoint, proposal, blockProposedAt, bucketHint, streamingState }; + // Return immediately after broadcast — attestation collection happens in the background. + return { checkpoint, proposal, blockProposedAt, streamingState }; } catch (err) { if (err && (err instanceof DutyAlreadySignedError || err instanceof SlashingProtectionError)) { // swallow this error. It's already been logged by a function deeper in the stack @@ -1574,9 +1560,11 @@ export class CheckpointProposalJob implements Traceable { * L1 bucket end, or when its prospective end would pass the threshold one bucket below the cap and could leave the * last legal endpoint behind. The lookup is bounded by the checkpoint cap on a non-final block, so a mandatory * bucket beyond this block's own reach is not stranded by a nearer endpoint, and additionally by one block's - * capacity on the final block. The block then ends at the further of what the lookup allows and the safe local - * step, so consulting L1 never consumes less than staying below the threshold would have; a block that consulted - * L1 may legitimately end inside a bucket. Nothing is retained: the next attempt decides again. + * capacity on the final block. A non-final block then ends at the further of what the lookup allows and the safe + * local step, so consulting L1 never consumes less than staying below the threshold would have, and it may + * legitimately end inside a bucket. The final block instead ends exactly on the resolved boundary, so whenever the + * last live boundary within reach sits behind the safe local step it consumes fewer messages than the local log + * alone would allow. Nothing is retained: the next attempt decides again. * * An endpoint that cannot be resolved on a non-final block (local lag, no live endpoint yet) leaves the block with * that safe local step and is retried on the next block; on the final block it abandons the checkpoint. A local @@ -1648,8 +1636,10 @@ export class CheckpointProposalJob implements Traceable { bucketSeq: resolved.bucketSeq, end, }); - // Re-read the selected prefix so the signed hash is the one at `end`, never the farther endpoint's. - return this.readStreamingRange(state, end); + // An end short of or past the endpoint needs its own read, so the signed hash is the one at `end` and never the + // endpoint's; landing exactly on the endpoint reuses the snapshot the resolver already read and checked against + // the cursor's hash. + return end === endpointTotal ? { kind: 'consume', range: resolved.range } : this.readStreamingRange(state, end); } /** From 477df975d8c5e6a12d1bd4ab4aaa0805fe53568d Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 15:46:05 -0300 Subject: [PATCH 12/25] test(stdlib): drop an await on a synchronous tx hash --- yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts index 81df2510bee9..24f9e223d9f1 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.test.ts @@ -42,7 +42,7 @@ describe('CheckpointProposal serialization / deserialization', () => { 'a lastBlock carrying its txs', async () => { const tx = await mockTx(1); - return makeCheckpointProposal({ lastBlock: { txHashes: [await tx.getTxHash()], txs: [tx] } }); + return makeCheckpointProposal({ lastBlock: { txHashes: [tx.getTxHash()], txs: [tx] } }); }, ], [ From 63b803aad14b4329e1d39c04075edb09ef6b6422 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 16:26:23 -0300 Subject: [PATCH 13/25] refactor: drop dead resolution branch and global range-read log cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The block-root proof callback re-enqueued the sub-tree resolution for a single-block checkpoint, but verifyBuiltBlockAgainstSyncedState already enqueues it after marking the block verified, and the resolution gate refuses to fire until every block is verified — so the extra call could only ever be a no-op. Reporting an Inbox range-read failure no longer needs a module logger, a process-wide dedupe Set and a truncation constant: the unexpected errors carry their bounded text on the result, and the proposal handler's existing one-shot timeout warn reports it. Sync lag stays silent and non-punitive, and the classification still matches on message text since the error class does not survive a JSON-RPC hop. Co-Authored-By: Claude Opus 5 (1M context) --- .../checkpoint-sub-tree-orchestrator.ts | 5 +- .../validator-client/src/proposal_handler.ts | 2 + .../src/streaming_inbox_checks.test.ts | 12 ++++ .../src/streaming_inbox_checks.ts | 57 +++++-------------- 4 files changed, 31 insertions(+), 45 deletions(-) diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts index c84bc41d6d5c..cf8f8a079139 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts @@ -763,11 +763,10 @@ export class CheckpointSubTreeOrchestrator extends ProvingScheduler { // Verification is called from both here and setBlockCompleted. Whichever runs last // will be the first to see all three pieces (header, proof output, archive) and run the checks. + // It enqueues the sub-tree resolution itself, so a single-block checkpoint needs nothing further. await this.verifyBuiltBlockAgainstSyncedState(provingState); - if (checkpointProvingState.totalNumBlocks === 1) { - this.checkAndEnqueueSubTreeResolution(checkpointProvingState); - } else { + if (checkpointProvingState.totalNumBlocks > 1) { this.checkAndEnqueueNextBlockMergeRollup(checkpointProvingState, leafLocation); } }, diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index e4efefa7ff46..cfcd8d9a7a3b 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -1180,6 +1180,8 @@ export class ProposalHandler { this.log.warn(`Timed out reading a consistent Inbox bundle, rejecting proposal`, { reason: 'inbox_prefix_sync_timeout', firstReason: first.reason, + // Set only when the message source failed for a reason the checks did not anticipate, rather than sync lag. + error: first.error, slot: slotNumber, waitedMs: timer.ms(), ...proposalInfo, diff --git a/yarn-project/validator-client/src/streaming_inbox_checks.test.ts b/yarn-project/validator-client/src/streaming_inbox_checks.test.ts index 788d02426eb8..6da435e675cf 100644 --- a/yarn-project/validator-client/src/streaming_inbox_checks.test.ts +++ b/yarn-project/validator-client/src/streaming_inbox_checks.test.ts @@ -256,5 +256,17 @@ describe('checkStreamingBlockProposal', () => { const result = await readStreamingBlockBundle(view, metadata as typeof metadata & { accepted: true }); expect(result).toEqual({ accepted: false, reason: 'inbox_prefix_unavailable' }); }); + + it('carries the error text of a range read that failed for an unanticipated reason', async () => { + const failing = { + getL1ToL2MessageRange: () => Promise.reject(new Error('database is closed')), + }; + const result = await readStreamingBlockBundle(failing, { + parentTotalMsgCount: 0n, + endTotalMsgCount: 1n, + inboxPrefixRef: InboxMessagePrefixRef.random(), + }); + expect(result).toEqual({ accepted: false, reason: 'inbox_prefix_unavailable', error: 'database is closed' }); + }); }); }); diff --git a/yarn-project/validator-client/src/streaming_inbox_checks.ts b/yarn-project/validator-client/src/streaming_inbox_checks.ts index 8078c962e15a..f7d98f9f69e9 100644 --- a/yarn-project/validator-client/src/streaming_inbox_checks.ts +++ b/yarn-project/validator-client/src/streaming_inbox_checks.ts @@ -1,9 +1,6 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; -import { createLogger } from '@aztec/foundation/log'; import type { InboxMessagePrefixRef, L1ToL2MessageSource } from '@aztec/stdlib/messaging'; -const log = createLogger('validator-client:streaming_inbox_checks'); - /** * Reason a streaming-Inbox block proposal fails the per-block acceptance checks. Follows the handler's existing * `{ isValid, reason }` string style. @@ -92,6 +89,11 @@ export type StreamingBlockCheckResult = /** A check failed; `reason` mirrors the acceptance condition that rejected the proposal. */ accepted: false; reason: StreamingBlockCheckReason; + /** + * Text of an unexpected message-source failure behind an `inbox_prefix_unavailable` verdict, so a caller whose + * retries run out can say why. Absent for ordinary sync lag and for every other reason. + */ + error?: string; }; /** @@ -184,8 +186,8 @@ export async function checkStreamingBlockProposalMetadata( * consistent, and the archiver's insert guard is what refuses to store the block. * * Every failure of the range read is non-punitive, but not every failure is ordinary sync lag: a store fault or a - * broken provider surfaces here as the same verdict. {@link logRangeReadFailure} separates the two in the logs so an - * operator can tell them apart without changing what the validator does with the proposal. + * broken provider surfaces here as the same verdict. The unexpected ones carry their error text on the result so a + * caller that gives up waiting can report it, without changing what the validator does with the proposal. */ export async function readStreamingBlockBundle( messageSource: Pick, @@ -201,8 +203,7 @@ export async function readStreamingBlockBundle( end: { rollingHash: endRollingHash }, } = await messageSource.getL1ToL2MessageRange(parentTotalMsgCount, endTotalMsgCount)); } catch (err) { - logRangeReadFailure(parentTotalMsgCount, endTotalMsgCount, err); - return { accepted: false, reason: 'inbox_prefix_unavailable' }; + return { accepted: false, reason: 'inbox_prefix_unavailable', error: unexpectedRangeReadError(err) }; } if (!endRollingHash.equals(inboxPrefixRef.inboxRollingHash)) { @@ -220,43 +221,15 @@ export async function readStreamingBlockBundle( */ const EXPECTED_RANGE_READ_FAILURES = ['is not fully synced', 'Invalid Inbox leaf count range']; -/** Upper bound on the error text carried into a log line, so a verbose provider error cannot blow up a log record. */ -const MAX_LOGGED_ERROR_LENGTH = 200; - -/** Ranges whose unexpected read failure has already been reported, so the caller's retry loop does not repeat it. */ -const reportedRangeReadFailures = new Set(); +/** Upper bound on the error text carried onto a result, so a verbose provider error cannot blow up a log record. */ +const MAX_REPORTED_ERROR_LENGTH = 200; -/** Cap on {@link reportedRangeReadFailures}, which is cleared wholesale once reached rather than evicted per entry. */ -const MAX_REPORTED_RANGE_READ_FAILURES = 256; - -/** - * Reports a failed Inbox range read. A range the archiver has not synced is the expected outcome for a node behind - * L1 and is logged at debug; anything else means the message source failed for a reason this check did not anticipate - * and is logged once per range and error, at warn. The verdict handed back to the caller is the same either way, so - * this only changes what an operator can see, never whether a proposal is penalised. - */ -function logRangeReadFailure(startTotalMsgCount: bigint, endTotalMsgCount: bigint, err: unknown): void { +/** Bounded text of a range-read failure the checks did not anticipate, or undefined for ordinary sync lag. */ +function unexpectedRangeReadError(err: unknown): string | undefined { const message = err instanceof Error ? err.message : String(err); - const error = message.slice(0, MAX_LOGGED_ERROR_LENGTH); - const context = { startTotalMsgCount, endTotalMsgCount, error }; - if (EXPECTED_RANGE_READ_FAILURES.some(fragment => message.includes(fragment))) { - log.debug(`Inbox message range [${startTotalMsgCount}, ${endTotalMsgCount}) is not available locally`, context); - return; - } - - const key = `${startTotalMsgCount}-${endTotalMsgCount}-${error}`; - if (reportedRangeReadFailures.has(key)) { - return; - } - if (reportedRangeReadFailures.size >= MAX_REPORTED_RANGE_READ_FAILURES) { - reportedRangeReadFailures.clear(); - } - reportedRangeReadFailures.add(key); - log.warn( - `Inbox message range [${startTotalMsgCount}, ${endTotalMsgCount}) could not be read; treating the block's ` + - `prefix as unconfirmed`, - context, - ); + return EXPECTED_RANGE_READ_FAILURES.some(fragment => message.includes(fragment)) + ? undefined + : message.slice(0, MAX_REPORTED_ERROR_LENGTH); } /** From 632a85673a9d43dfcd766b94ea4c7867d7bc23db Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 16:56:03 -0300 Subject: [PATCH 14/25] fix(prover): publish a block's archive only once its fork is closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The archive snapshot has to be read from the block's fork, so it existed before `close()` returned while the header and the proof output could both already be in hand. Verification runs from the block root proof callback as well as from completion, so a proof landing inside that close found every piece present, marked the block verified and resolved the sub-tree with the fork still open — the same handoff the completion join exists to prevent, reached by a different path. Capturing the snapshot into a local and publishing it after the close removes the window entirely rather than adding another guard. Covered by a test that parks completion inside the close and releases the block root proof into it. --- .../checkpoint-sub-tree-orchestrator.test.ts | 87 +++++++++++++++++-- .../checkpoint-sub-tree-orchestrator.ts | 8 +- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts index 93fb400083d0..7ccb2cb8200f 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.test.ts @@ -371,6 +371,42 @@ describe('prover/orchestrator/checkpoint-sub-tree', () => { } }); + it('withholds the result while the block fork is still closing', async () => { + // The narrowest window: the archive snapshot is captured from the fork, so it exists before `close()` + // returns. A block root proof landing during the close runs verification itself and would find header, + // proof and archive all present, so publishing the archive any earlier resolves the sub-tree with the + // block's own fork still open. + const closeGate = promiseWithResolvers(); + const proofGate = promiseWithResolvers(); + const { constants, blocks, l1ToL2Messages, previousBlockHeader } = await context.makeCheckpoint(1); + const subTree = await startInspectableSubTree(1, constants, l1ToL2Messages, previousBlockHeader, { + worldState: withHeldForkClose(context.worldState, closeGate.promise), + prover: withHeldBlockRootProof(context.prover, proofGate.promise), + }); + try { + const result = trackSettlement(subTree.getSubTreeResult()); + const { blockNumber, timestamp } = blocks[0].header.globalVariables; + + await subTree.startNewBlock(blockNumber, timestamp, 0, l1ToL2Messages); + + // Park completion inside the fork close, then let the block root proof land in that window. + const completed = subTree.setBlockCompleted(blockNumber, blocks[0].header); + await sleep(50); + proofGate.resolve(); + await waitForAllProofs(subTree); + + expect(result.settled).toBe(false); + + closeGate.resolve(); + await completed; + await expect(subTree.getSubTreeResult()).resolves.toBeDefined(); + } finally { + proofGate.resolve(); + closeGate.resolve(); + await subTree.stop(); + } + }); + it('withholds the result until the last block of a multi-block checkpoint is verified', async () => { // Same race with the block merge in play: the second block is message-only, so its root proof is enqueued // from startNewBlock and the merge can complete while that block is still being driven. @@ -451,11 +487,14 @@ describe('prover/orchestrator/checkpoint-sub-tree', () => { constants: CheckpointConstantData, l1ToL2Messages: Fr[], previousBlockHeader: BlockHeader, - { worldState = context.worldState }: { worldState?: typeof context.worldState } = {}, + { + worldState = context.worldState, + prover = context.prover, + }: { worldState?: typeof context.worldState; prover?: typeof context.prover } = {}, ): Promise { return InspectableSubTree.startInspectable( worldState, - context.prover, + prover, EthAddress.ZERO, chonkCache, EpochNumber(1), @@ -542,13 +581,45 @@ function trackSettlement(promise: Promise) { * the synced one and verification must reject the checkpoint. */ function withStaleArchiveSnapshots(worldState: T): T { - return new Proxy(worldState, { - get(target, prop, receiver) { - const value = Reflect.get(target, prop, receiver); - if (prop === 'getSnapshot' && typeof value === 'function') { - return (blockNumber: number) => value.call(target, blockNumber - 1); + return replacingMethod(worldState, 'getSnapshot', getSnapshot => blockNumber => getSnapshot(Number(blockNumber) - 1)); +} + +/** A world state whose block forks park in `close()` until `held` settles. */ +function withHeldForkClose(worldState: T, held: Promise): T { + return replacingMethod(worldState, 'fork', fork => async (...args) => { + const db = await fork(...args); + if (typeof db !== 'object' || db === null) { + throw new Error('Expected a fork.'); + } + return replacingMethod(db, 'close', close => async () => { + await held; + return await close(); + }); + }); +} + +/** A prover that withholds an empty block's root proof until `held` settles. */ +function withHeldBlockRootProof(prover: T, held: Promise): T { + return replacingMethod(prover, 'getBlockRootNoTxsRollupProof', getProof => async (...args) => { + await held; + return await getProof(...args); + }); +} + +/** Forwards every member to `target` except the named method, which `replace` rebuilds from the original. */ +function replacingMethod( + target: T, + name: string, + replace: (original: (...args: unknown[]) => unknown) => (...args: unknown[]) => unknown, +): T { + return new Proxy(target, { + get(_target, prop) { + const value = Reflect.get(target, prop); + if (typeof value !== 'function') { + return value; } - return typeof value === 'function' ? value.bind(target) : value; + const method = value.bind(target); + return prop === name ? replace(method) : method; }, }); } diff --git a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts index cf8f8a079139..73f84607e777 100644 --- a/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts +++ b/yarn-project/prover-client/src/orchestrator/checkpoint-sub-tree-orchestrator.ts @@ -471,16 +471,22 @@ export class CheckpointSubTreeOrchestrator extends ProvingScheduler { this.dbs.delete(provingState.blockNumber); // Update the archive tree, capture the snapshot, and close the fork deterministically. + let builtArchive: AppendOnlyTreeSnapshot; try { this.logger.verbose( `Updating archive tree with block ${provingState.blockNumber} header ${(await header.hash()).toString()}`, ); await db.updateArchive(header); - provingState.setBuiltArchive(await getTreeSnapshot(MerkleTreeId.ARCHIVE, db)); + builtArchive = await getTreeSnapshot(MerkleTreeId.ARCHIVE, db); } finally { await db.close(); } + // Publish the archive only once the fork is closed. Verification also runs from the block root proof callback, + // so an archive published before the close lets a proof arriving during it find every piece present, verify, + // and resolve the sub-tree while this block's fork is still open. + provingState.setBuiltArchive(builtArchive); + await this.verifyBuiltBlockAgainstSyncedState(provingState); return header; From 2c0fbae9b7f00335f920904bbbe8ff2ac088c665 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 22:37:00 -0300 Subject: [PATCH 15/25] fix(validator): carry the duty budget through every checkpoint validation stage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Racing the caller only bounds that caller's wait. The stages after it — the published-checkpoint lookup, the checkpoint start position, the previous-checkpoint reads, the world-state fork and the checkpoint rebuild — ran unbounded, so a read that never settled held the duty open and its losing continuation could still walk into the next stage. Each stage now runs inside the slot's budget, and recording the proposed checkpoint is barred after its own block read rather than only before it. The direct attestation path had no whole-duty bound at all: its preliminary epoch and committee reads and the remote signing await sat outside every deadline. It now takes one absolute budget for the slot and passes it through validation into signing, and the signing-protection record is written when the request goes out rather than when it comes back, so a signer that never answers cannot leave the slot unprotected. Running out of time is this node giving up, not an observation about the proposal, so it answers with its own `validation_deadline_expired` reason, records no outcome and caches no verdict for the next caller. The expired-budget grace is now one absolute allowance for the whole duty instead of a fresh second for every call, which let a duty already past its deadline walk stage by stage through as many seconds as it had stages. Co-Authored-By: Claude Opus 5 (1M context) --- .../validator-client/src/duty_budget.test.ts | 61 +++++++ .../validator-client/src/duty_budget.ts | 44 ++++- .../src/proposal_handler.test.ts | 110 ++++++++++++- .../validator-client/src/proposal_handler.ts | 151 +++++++++++++----- .../validator-client/src/validator.test.ts | 42 +++++ .../validator-client/src/validator.ts | 93 ++++++++--- 6 files changed, 424 insertions(+), 77 deletions(-) create mode 100644 yarn-project/validator-client/src/duty_budget.test.ts diff --git a/yarn-project/validator-client/src/duty_budget.test.ts b/yarn-project/validator-client/src/duty_budget.test.ts new file mode 100644 index 000000000000..391d27305673 --- /dev/null +++ b/yarn-project/validator-client/src/duty_budget.test.ts @@ -0,0 +1,61 @@ +import { TestDateProvider } from '@aztec/foundation/timer'; + +import { describe, expect, it } from '@jest/globals'; + +import { DutyBudget, DutyBudgetExpiredError } from './duty_budget.js'; + +describe('DutyBudget', () => { + let dateProvider: TestDateProvider; + + beforeEach(() => { + dateProvider = new TestDateProvider(); + }); + + /** A budget whose deadline is already `ms` in the past. */ + const expiredBudget = (ms = 5_000) => new DutyBudget(new Date(dateProvider.now() - ms), dateProvider); + + const hang = () => new Promise(() => {}); + + it('lets a duty past its deadline finish a read that answers at once', async () => { + await expect(expiredBudget().run('read', () => Promise.resolve('answer'))).resolves.toEqual('answer'); + }); + + // The grace exists so a duty entered past its deadline can still make the one attempt its callers rely on. It + // is one allowance for the whole duty: renewing it per stage would let an expired duty walk through as many + // seconds as it has stages. + it('gives a duty past its deadline one grace allowance, not one per stage', async () => { + const budget = expiredBudget(); + + const firstStage = Date.now(); + await expect(budget.run('first stage', hang)).rejects.toThrow(DutyBudgetExpiredError); + expect(Date.now() - firstStage).toBeGreaterThanOrEqual(500); + + const secondStage = Date.now(); + await expect(budget.run('second stage', hang)).rejects.toThrow(DutyBudgetExpiredError); + expect(Date.now() - secondStage).toBeLessThan(200); + }); + + it('reports the grace as somewhere left to go, and the deadline as gone, past the deadline', () => { + const budget = expiredBudget(); + + expect(budget.expired()).toBe(true); + expect(budget.canContinue()).toBe(true); + }); + + it('stops offering the grace once the duty is stopped', async () => { + const budget = expiredBudget(); + budget.stop('sibling failed'); + + expect(budget.canContinue()).toBe(false); + await expect(budget.run('read', () => Promise.resolve('answer'))).rejects.toThrow(DutyBudgetExpiredError); + }); + + it('bounds a read by what is left of the budget rather than by a fresh full timeout', async () => { + const budget = new DutyBudget(new Date(dateProvider.now() + 300), dateProvider); + + const started = Date.now(); + await expect(budget.run('read', hang)).rejects.toThrow(DutyBudgetExpiredError); + + expect(Date.now() - started).toBeLessThan(1_000); + }); +}); diff --git a/yarn-project/validator-client/src/duty_budget.ts b/yarn-project/validator-client/src/duty_budget.ts index 085a090694a8..4a1d150c2902 100644 --- a/yarn-project/validator-client/src/duty_budget.ts +++ b/yarn-project/validator-client/src/duty_budget.ts @@ -2,9 +2,9 @@ import type { DateProvider } from '@aztec/foundation/timer'; import { execWithSignal } from '@aztec/foundation/timer'; /** - * How long a run gets when the budget has already run out. The duty still makes the single attempt its callers - * rely on, but bounded, so an unresponsive read cannot hold an expired duty open. Long enough for a local store - * read, short enough that nothing waits on it. + * How long a duty gets in total once its budget has already run out. The duty still makes the single attempt its + * callers rely on, but bounded, so an unresponsive read cannot hold an expired duty open. Long enough for a local + * store read, short enough that nothing waits on it. */ const EXPIRED_BUDGET_GRACE_MS = 1_000; @@ -35,6 +35,9 @@ export class DutyBudgetExpiredError extends Error { export class DutyBudget { private readonly controller = new AbortController(); + /** Absolute end of the one grace allowance, fixed by the first run that finds the budget already gone. */ + private graceDeadlineMs: number | undefined; + constructor( public readonly deadline: Date, private readonly dateProvider: DateProvider, @@ -48,11 +51,24 @@ export class DutyBudget { return Math.max(0, this.deadline.getTime() - this.dateProvider.now()); } - /** Whether the budget is gone, because the deadline passed or because the duty was stopped. */ + /** + * Whether the budget is gone, because the deadline passed or because the duty was stopped. This is the hard + * question, the one output nobody would accept any more depends on: a signature produced after it is unusable. + * Work that only needs to finish the attempt already under way asks {@link canContinue} instead. + */ public expired(): boolean { return this.remainingMs() === 0; } + /** + * Whether the duty may start or commit further work: either it is still within budget, or it is within the + * single grace allowance described on {@link run}. Asking opens that allowance if nothing has opened it yet, + * so a duty entered past its deadline can finish the one attempt its callers rely on and no more. + */ + public canContinue(): boolean { + return this.remainingMs() > 0 || this.remainingGraceMs() > 0; + } + /** The signal callees should pass on and check; aborted once the budget is gone. */ public get signal(): AbortSignal { return this.controller.signal; @@ -75,15 +91,29 @@ export class DutyBudget { * * A budget that has already run out still gets {@link EXPIRED_BUDGET_GRACE_MS}: a duty past its deadline is * expected to make the one attempt its callers rely on — a proposal whose blocks are already local validates - * without waiting for anything — and the point here is that even that attempt cannot hang. A duty that was - * explicitly stopped gets no grace: its result has no reader left. + * without waiting for anything — and the point here is that even that attempt cannot hang. The grace is one + * absolute allowance for the whole duty, anchored by the first run that finds the budget gone and shared by + * every run after it, so a duty past its deadline cannot walk stage by stage through a fresh second each time. + * A duty that was explicitly stopped gets no grace: its result has no reader left. */ public async run(what: string, fn: (signal: AbortSignal) => Promise): Promise { if (this.controller.signal.aborted) { throw new DutyBudgetExpiredError(what, this.deadline); } - const remainingMs = this.remainingMs() || EXPIRED_BUDGET_GRACE_MS; + const remainingMs = this.remainingMs() || this.remainingGraceMs(); + if (remainingMs === 0) { + throw new DutyBudgetExpiredError(what, this.deadline); + } const signal = AbortSignal.any([this.controller.signal, AbortSignal.timeout(remainingMs)]); return await execWithSignal(fn, signal, () => new DutyBudgetExpiredError(what, this.deadline)); } + + /** 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) { + return 0; + } + this.graceDeadlineMs ??= this.dateProvider.now() + EXPIRED_BUDGET_GRACE_MS; + return Math.max(0, this.graceDeadlineMs - this.dateProvider.now()); + } } diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index 8bbba2a072c9..c269d9246b79 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -6,6 +6,7 @@ import { MAX_FEE_ASSET_PRICE_MODIFIER_BPS } from '@aztec/ethereum/contracts'; import { BlockNumber, CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types'; 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 { type FieldsOf, unfreeze } from '@aztec/foundation/types'; @@ -400,7 +401,8 @@ describe('ProposalHandler checkpoint validation', () => { }); // retryUntil only consults its deadline once an attempt returns, so an attempt that never settles never - // reaches it. The duty budget races the whole loop, so the caller settles instead of being held open. + // reaches it. The duty budget races the whole loop, so the caller settles instead of being held open. Running + // out of time is this node giving up rather than a verdict, so it is reported as its own reason. it('settles within the duty budget when a readiness read never returns', async () => { blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); // attestation_deadline(slot=1) is 40s; leave a short but nonzero budget. @@ -408,7 +410,7 @@ describe('ProposalHandler checkpoint validation', () => { const result = await handler.handleCheckpointProposal(await makeProposal(), proposalInfo); - expect(result).toEqual({ isValid: false, reason: 'last_block_not_found' }); + expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); }); it('settles within the grace when a readiness read never returns past the deadline', async () => { @@ -417,7 +419,7 @@ describe('ProposalHandler checkpoint validation', () => { const result = await handler.handleCheckpointProposal(await makeProposal(), proposalInfo); - expect(result).toEqual({ isValid: false, reason: 'last_block_not_found' }); + expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); }); // The cached-valid path re-reads the checkpoint's blocks before reusing a verdict; that read is a store @@ -437,7 +439,9 @@ describe('ProposalHandler checkpoint validation', () => { blockSource.getBlockData.mockImplementation(() => new Promise(() => {})); dateProvider.setTime(39_800); - await expect(handler.handleCheckpointProposal(proposal, proposalInfo)).rejects.toThrow(/Duty budget/); + const result = await handler.handleCheckpointProposal(proposal, proposalInfo); + + expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); }); // With <1s remaining the old Math.floor(...) timeout collapsed to 0 ("never time out"). The fix uses @@ -449,7 +453,47 @@ describe('ProposalHandler checkpoint validation', () => { dateProvider.setTime(39_700); const result = await handler.handleCheckpointProposal(await makeProposal(), proposalInfo); - expect(result).toEqual({ isValid: false, reason: 'last_block_not_found' }); + expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); + }); + + // The snapshot wait is only the first stage. Every read after it is another chance for the duty to hang, so + // the budget has to cover the whole method rather than the wait it started with. + it('settles when a read after the snapshot never returns', async () => { + const archiveRoot = Fr.random(); + blockSource.getBlocksForSlot.mockResolvedValue(makeSlotBlocks([archiveRoot])); + blockSource.getCheckpointData.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(39_800); + + const result = await handler.handleCheckpointProposal(await makeProposal({ archiveRoot }), proposalInfo); + + expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); + }); + + // Running out of time is this node giving up, not something it learned about the proposal, so it neither + // caches a verdict for the next caller nor records an outcome the sentinel would read as a missed proposal. + it('neither caches nor records an outcome when the duty budget runs out', async () => { + const recordSpy = jest.spyOn(reexecutionTracker, 'recordOutcome'); + blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(39_800); + const proposal = await makeProposal(); + + expect(await handler.handleCheckpointProposal(proposal, proposalInfo)).toEqual({ + isValid: false, + reason: 'validation_deadline_expired', + }); + expect(recordSpy).not.toHaveBeenCalled(); + + // The next caller revalidates from scratch instead of inheriting the abandoned duty's non-verdict. + const archiveRoot = proposal.archive; + blockSource.getBlocksForSlot.mockResolvedValue(makeSlotBlocks([archiveRoot])); + blockSource.getCheckpointData.mockResolvedValue({ checkpointNumber: CheckpointNumber(1) } as CheckpointData); + dateProvider.setTime(20_000); + + expect(await handler.handleCheckpointProposal(proposal, proposalInfo)).toEqual({ + isValid: false, + reason: 'checkpoint_already_published', + checkpointNumber: CheckpointNumber(1), + }); }); }); @@ -483,6 +527,41 @@ describe('ProposalHandler checkpoint validation', () => { expect(metrics.recordCheckpointProposalToPipelinedStateDuration).toHaveBeenCalledWith(expect.any(Number)); }); + // Recording a proposed checkpoint is a mutation the next slot builds on top of. Its own block read can settle + // long after the duty was answered, so the insertion has to be barred by the budget, not just preceded by it. + it('does not record a proposed checkpoint from a block read that lands after the budget is gone', async () => { + const proposal = await makeProposal(); + const p2p = mock(); + let checkpointHandler: ((proposal: any, sender: any) => Promise) | undefined; + p2p.registerAllNodesCheckpointProposalHandler.mockImplementation(handler => { + checkpointHandler = handler; + }); + + const archiver = mock>(); + archiver.addProposedCheckpoint.mockResolvedValue(undefined); + + const blockRead = promiseWithResolvers(); + blockSource.getBlockData.mockReturnValue(blockRead.promise); + + jest + .spyOn(handler, 'handleCheckpointProposal') + .mockResolvedValue({ isValid: true, checkpointNumber: CheckpointNumber(3) }); + + // 200ms of budget left, and a block read that answers only after it is spent. + dateProvider.setTime(39_800); + handler.register(p2p, true, archiver); + await checkpointHandler!(proposal, {} as any); + + blockRead.resolve({ + checkpointNumber: CheckpointNumber(3), + header: { getBlockNumber: () => 9 }, + indexWithinCheckpoint: 2, + } as BlockData); + await sleep(50); + + expect(archiver.addProposedCheckpoint).not.toHaveBeenCalled(); + }); + // The checkpoint-validation block-sync deadline is the L1 publish deadline (12s/one Ethereum // slot before the last L1 block of the target slot), which is later than the target-slot start // used for block re-execution. With slotDuration=24, ethereumSlotDuration=4 and proposal slot=1: @@ -997,6 +1076,27 @@ describe('ProposalHandler checkpoint validation', () => { expect(mockDispose).toHaveBeenCalled(); }); + // A stage that settles after the deadline must not carry the duty into the next one: the fork arriving late + // cannot be turned into a checkpoint rebuild nobody is waiting for. + it('starts no reconstruction stage when the world-state fork settles past the deadline', async () => { + setupDeepValidationMocks({ header: makeHeader() }); + const fork = promiseWithResolvers(); + checkpointsBuilder.getFork.mockReturnValue(fork.promise as any); + + // 200ms of budget left, and a fork that answers only after it is spent. + dateProvider.setTime(39_800); + const proposal = await makeProposal({ archiveRoot, checkpointHeader: makeHeader() }); + const validation = handler.handleCheckpointProposal(proposal, proposalInfo); + await sleep(400); + fork.resolve({ + [Symbol.asyncDispose]: mockDispose, + getTreeInfo: () => Promise.resolve({ root: Fr.ZERO.toBuffer() }), + }); + + expect(await validation).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); + expect(checkpointsBuilder.openCheckpoint).not.toHaveBeenCalled(); + }); + it('disposes fork even when validation fails', async () => { setupDeepValidationMocks({ header: CheckpointHeader.empty() }); diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index cfcd8d9a7a3b..e0d047314762 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -132,6 +132,9 @@ export type CheckpointProposalValidationFailureReason = // consumed. Local-view outcomes, never proposer misconduct. | 'inbox_prefix_unavailable' | 'inbox_prefix_mismatch' + // 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 streaming-Inbox reasons a checkpoint proposal can fail on; both are retried through a bounded local sync. */ @@ -191,6 +194,8 @@ 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', + // This node ran out of time to look; it observed nothing about the proposer. + validation_deadline_expired: undefined, checkpoint_validation_failed: 'invalid', }; @@ -268,6 +273,7 @@ export const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record< // A reorg / divergent local chain, not a proposer offense (mirrors the block path's initial_state_mismatch). ['initial_archive_mismatch']: false, ['checkpoint_already_published']: false, + ['validation_deadline_expired']: false, }; /** @@ -493,7 +499,10 @@ export class ProposalHandler { return undefined; } - if (await this.epochCache.isEscapeHatchOpenAtSlot(proposal.slotNumber)) { + const escapeHatchOpen = await budget.run(`escape hatch check for slot ${proposal.slotNumber}`, () => + this.epochCache.isEscapeHatchOpenAtSlot(proposal.slotNumber), + ); + if (escapeHatchOpen) { this.log.warn( `Escape hatch open for slot ${proposal.slotNumber}, skipping checkpoint proposal validation`, proposalInfo, @@ -512,7 +521,9 @@ export class ProposalHandler { const isOwnProposal = proposer && ownAddresses?.some(addr => addr === proposer.toString()); if (isOwnProposal) { - const existing = await this.archiver?.getProposedCheckpointData({ slot: proposal.slotNumber }); + const existing = await budget.run(`own proposed checkpoint read for slot ${proposal.slotNumber}`, () => + Promise.resolve(this.archiver?.getProposedCheckpointData({ slot: proposal.slotNumber })), + ); if (existing?.archive.root.equals(proposal.archive)) { this.log.debug(`Skipping sync for existing own checkpoint proposal at slot ${proposal.slotNumber}`); return undefined; @@ -529,7 +540,7 @@ export class ProposalHandler { } await this.checkpointProposalValidationFailureCallback?.(proposal, result, proposalInfo); } else if (this.archiver) { - const set = await this.setProposedCheckpoint(proposal); + const set = await this.setProposedCheckpoint(proposal, budget); if (set) { this.metrics?.recordCheckpointProposalToPipelinedStateDuration(pipeliningTimer.ms()); } @@ -1571,6 +1582,28 @@ export class ProposalHandler { proposal: ValidatedCheckpointProposalCore, proposalInfo: LogData, budget: DutyBudget = new DutyBudget(this.getReexecutionDeadline(proposal.slotNumber), this.dateProvider), + ): Promise { + try { + return await this.validateAndRecordCheckpointProposal(proposal, proposalInfo, budget); + } catch (err) { + if (!(err instanceof DutyBudgetExpiredError)) { + throw err; + } + // Every stage runs inside the budget, so an expiry anywhere lands here with nothing cached and nothing + // recorded: the duty stopped looking, which is not an observation about the proposal. + this.log.warn(`Ran out of duty budget validating the checkpoint proposal for slot ${proposal.slotNumber}`, { + ...proposalInfo, + deadline: budget.deadline.toISOString(), + }); + return { isValid: false, reason: 'validation_deadline_expired' }; + } + } + + /** {@link handleCheckpointProposal}'s body, outside its duty-budget handling. */ + private async validateAndRecordCheckpointProposal( + proposal: ValidatedCheckpointProposalCore, + proposalInfo: LogData, + budget: DutyBudget, ): Promise { const slot = proposal.slotNumber; const payloadHash = proposal.getPayloadHash(); @@ -1616,15 +1649,21 @@ export class ProposalHandler { this.reexecutionTracker.recordOutcome(slot, proposal.archive, outcome, result.checkpointNumber); } - // Drop tracker entries for checkpoints that have reached L1 finality. + // 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. try { - const tips = await this.blockSource.getL2Tips(); + const tips = await budget.run(`reexecution tracker prune for slot ${slot}`, () => this.blockSource.getL2Tips()); const finalizedCheckpointNumber = tips.finalized.checkpoint.number; if (finalizedCheckpointNumber > 0) { this.reexecutionTracker.removeBefore(CheckpointNumber(finalizedCheckpointNumber + 1)); } } catch (err) { - this.log.error(`Error pruning reexecution tracker`, err, proposalInfo); + if (err instanceof DutyBudgetExpiredError) { + this.log.debug(`Skipped pruning the reexecution tracker for slot ${slot}: the duty budget is gone`); + } else { + this.log.error(`Error pruning reexecution tracker`, err, proposalInfo); + } } // Upload blobs to filestore if validation passed (fire and forget) @@ -1675,7 +1714,12 @@ export class ProposalHandler { ), ); } catch (err) { - if (err instanceof TimeoutError || err instanceof DutyBudgetExpiredError) { + // A budget expiry is the duty giving up, not a verdict on the proposal: it propagates to the caller, which + // answers with `validation_deadline_expired` and records nothing. + if (err instanceof DutyBudgetExpiredError) { + throw err; + } + if (err instanceof TimeoutError) { this.log.warn(`Timed out waiting for block with archive matching checkpoint proposal`, proposalInfo); return { isValid: false, reason: 'last_block_not_found' }; } @@ -1691,7 +1735,9 @@ export class ProposalHandler { const lastBlock = blocks[lastBlockIndex]; // Refuse to attest if the block's enclosing checkpoint has already been published to L1. - const existingCheckpoint = await this.blockSource.getCheckpointData({ number: lastBlock.checkpointNumber }); + const existingCheckpoint = await budget.run(`published checkpoint check for slot ${slot}`, () => + this.blockSource.getCheckpointData({ number: lastBlock.checkpointNumber }), + ); if (existingCheckpoint) { this.log.warn(`Refusing to attest to checkpoint proposal whose checkpoint is already on L1`, { ...proposalInfo, @@ -1753,7 +1799,9 @@ export class ProposalHandler { // The checkpoint's Inbox consumption starts at the leaf count of the block before its first block. Without that // block the consumed bundle cannot be derived; an empty bundle would make a valid proposal fail its rolling-hash // recomputation and be classified as a proposer offense, so a missing parent is a local fetch failure instead. - const checkpointStartTotal = await this.getPreBlockConsumedTotal(firstBlock.number); + const checkpointStartTotal = await budget.run(`checkpoint start position read for slot ${slot}`, () => + this.getPreBlockConsumedTotal(firstBlock.number), + ); if (checkpointStartTotal === undefined) { this.log.warn(`Block before checkpoint proposal's first block ${firstBlock.number} is unavailable locally`, { ...proposalInfo, @@ -1788,20 +1836,24 @@ export class ProposalHandler { // Collect the out hashes of all the checkpoints before this one in the same epoch. // See note on the analogous block-proposal site: the helper handles pipelining lag. const epoch = getEpochAtSlot(slot, this.epochCache.getL1Constants()); - const previousCheckpointOutHashes = await getPreviousCheckpointOutHashes({ - blockSource: this.blockSource, - epoch, - checkpointNumber, - l1Constants: this.epochCache.getL1Constants(), - pipeliningEnabled: true, - log: this.log, - }); + const previousCheckpointOutHashes = await budget.run(`previous checkpoint out hashes for slot ${slot}`, () => + getPreviousCheckpointOutHashes({ + blockSource: this.blockSource, + epoch, + checkpointNumber, + l1Constants: this.epochCache.getL1Constants(), + pipeliningEnabled: true, + log: this.log, + }), + ); - const previousInboxRollingHash = await getPreviousCheckpointInboxRollingHash({ - blockSource: this.blockSource, - checkpointNumber, - log: this.log, - }); + const previousInboxRollingHash = await budget.run(`previous checkpoint Inbox position for slot ${slot}`, () => + getPreviousCheckpointInboxRollingHash({ + blockSource: this.blockSource, + checkpointNumber, + log: this.log, + }), + ); // Fork world state at the block before the first block. getFork syncs world state to the parent block // first (see its doc): the block source (archiver) can already hold the block while world state still @@ -1812,9 +1864,14 @@ export class ProposalHandler { const parentBlockNumber = BlockNumber(firstBlock.number - 1); let forkResult: MerkleTreeWriteOperations; try { - const parentBlockHash = (await this.blockSource.getBlockData({ number: parentBlockNumber }))?.blockHash; - forkResult = await this.checkpointsBuilder.getFork(parentBlockNumber, parentBlockHash); + forkResult = await budget.run(`world state fork at block ${parentBlockNumber} for slot ${slot}`, async () => { + const parentBlockHash = (await this.blockSource.getBlockData({ number: parentBlockNumber }))?.blockHash; + return await this.checkpointsBuilder.getFork(parentBlockNumber, parentBlockHash); + }); } catch (err) { + if (err instanceof DutyBudgetExpiredError) { + throw err; + } this.log.warn(`Failed to fork world state at block ${parentBlockNumber} for checkpoint proposal`, { ...proposalInfo, parentBlockNumber, @@ -1839,21 +1896,21 @@ export class ProposalHandler { return { isValid: false, reason: 'initial_archive_mismatch', checkpointNumber }; } - // Create checkpoint builder with all existing blocks - const checkpointBuilder = await this.checkpointsBuilder.openCheckpoint( - checkpointNumber, - constants, - proposal.feeAssetPriceModifier, - l1ToL2Messages, - previousCheckpointOutHashes, - previousInboxRollingHash, - fork, - blocks, - this.log.getBindings(), - ); - - // Complete the checkpoint to get computed values - const computedCheckpoint = await checkpointBuilder.completeCheckpoint(); + // Create checkpoint builder with all existing blocks, and complete it to get the computed values. + const computedCheckpoint = await budget.run(`checkpoint reconstruction for slot ${slot}`, async () => { + const checkpointBuilder = await this.checkpointsBuilder.openCheckpoint( + checkpointNumber, + constants, + proposal.feeAssetPriceModifier, + l1ToL2Messages, + previousCheckpointOutHashes, + previousInboxRollingHash, + fork, + blocks, + this.log.getBindings(), + ); + return await checkpointBuilder.completeCheckpoint(); + }); // Compare checkpoint header with proposal if (!computedCheckpoint.header.equals(proposal.checkpointHeader)) { @@ -1964,11 +2021,13 @@ export class ProposalHandler { * pipeline building on top of the checkpoint. Does not retry, since validation already waited for the * last block to sync. */ - private async setProposedCheckpoint(proposal: CheckpointProposalCore): Promise { + private async setProposedCheckpoint(proposal: CheckpointProposalCore, budget: DutyBudget): Promise { if (!this.archiver) { return false; } - const blockData = await this.blockSource.getBlockData({ archive: proposal.archive }); + const blockData = await budget.run(`proposed checkpoint block read for slot ${proposal.slotNumber}`, () => + this.blockSource.getBlockData({ archive: proposal.archive }), + ); if (!blockData) { this.log.debug(`Block data not found for checkpoint proposal archive, cannot set proposed checkpoint`, { archive: proposal.archive.toString(), @@ -1976,6 +2035,16 @@ export class ProposalHandler { return false; } + // The read above can settle after the duty is over. Accepting a pipelining parent is a mutation of state the + // next slot builds on, so it needs the budget checked here rather than only before the read. + if (!budget.canContinue()) { + this.log.warn(`Not recording the proposed checkpoint for slot ${proposal.slotNumber}: the duty budget is gone`, { + archive: proposal.archive.toString(), + deadline: budget.deadline.toISOString(), + }); + return false; + } + await this.archiver.addProposedCheckpoint({ header: proposal.checkpointHeader, checkpointNumber: blockData.checkpointNumber, diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index e10db25ea84e..406a4d31372d 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -947,6 +947,48 @@ describe('ValidatorClient', () => { validateCheckpointSpy.mockRestore(); }); + // A remote (HA) signer that never answers must not hold the duty open either. The request stands as issued, + // so the equivocation record it produced is kept, but nothing is pooled or handed back for gossip. + it('settles when the remote signer never returns, pooling nothing and keeping the signing record', async () => { + const addCheckpointAttestationsSpy = jest.spyOn(p2pClient, 'addOwnCheckpointAttestations'); + epochCache.filterInCommittee.mockResolvedValue([EthAddress.fromString(validatorAccounts[0].address)]); + + const checkpointProposal = await makeCheckpointProposal({ + archiveRoot: proposal.archive, + checkpointHeader: makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }), + lastBlock: { + blockHeader: makeBlockHeader(1, { blockNumber: BlockNumber(123), slotNumber: proposal.slotNumber }), + indexWithinCheckpoint: IndexWithinCheckpoint(0), + txHashes: proposal.txHashes, + }, + }); + + 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); + const validationService = (validatorClient as unknown as { validationService: ValidationService }) + .validationService; + jest.spyOn(validationService, 'attestToCheckpointProposal').mockImplementation(() => new Promise(() => {})); + + const attestations = await validatorClient.attestToCheckpointProposal( + ValidatedCheckpointProposalCore(checkpointProposal), + sender, + ); + + expect(attestations).toBeUndefined(); + expect(addCheckpointAttestationsSpy).not.toHaveBeenCalled(); + // The slot stays protected: a signer that never answered may still have signed. + expect( + (validatorClient as unknown as { lastAttestedProposal?: { slotNumber: SlotNumber } }).lastAttestedProposal + ?.slotNumber, + ).toEqual(proposal.slotNumber); + validateCheckpointSpy.mockRestore(); + }); + it('should not attest to a checkpoint proposal that references a middle block instead of the last', async () => { const addCheckpointAttestationsSpy = jest.spyOn(p2pClient, 'addOwnCheckpointAttestations'); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index 2c72ed2f08d5..b49d5dd16efd 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -61,6 +61,7 @@ import type { TypedDataDefinition } from 'viem'; import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; import { ValidationService } from './duties/validation_service.js'; +import { DutyBudget, DutyBudgetExpiredError } from './duty_budget.js'; import { HAKeyStore } from './key_store/ha_key_store.js'; import type { ExtendedValidatorKeyStore } from './key_store/interface.js'; import { NodeKeystoreAdapter } from './key_store/node_keystore_adapter.js'; @@ -519,12 +520,41 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) async attestToCheckpointProposal( proposal: ValidatedCheckpointProposalCore, _proposalSender: PeerId, + ): Promise { + // The all-nodes callback ran under a budget of its own; this one repeats the committee reads, may revalidate + // and then signs, so it takes an absolute budget for the same slot covering all of it. Without one the + // preliminary reads and the signer await are unbounded, and p2p waits on this callback. + const budget = new DutyBudget(this.proposalHandler.getReexecutionDeadline(proposal.slotNumber), this.dateProvider); + try { + return await this.attestToCheckpointProposalWithinBudget(proposal, budget); + } catch (err) { + if (!(err instanceof DutyBudgetExpiredError)) { + throw err; + } + this.log.warn(`Ran out of duty budget attesting to the checkpoint proposal for slot ${proposal.slotNumber}`, { + slot: proposal.slotNumber, + deadline: budget.deadline.toISOString(), + }); + return undefined; + } finally { + // Anything still retrying belongs to a duty nobody will read from any more. + budget.stop(`checkpoint attestation for slot ${proposal.slotNumber}`); + } + } + + /** {@link attestToCheckpointProposal}'s body, with every stage bound to the slot's duty budget. */ + private async attestToCheckpointProposalWithinBudget( + proposal: ValidatedCheckpointProposalCore, + budget: DutyBudget, ): Promise { const proposalSlotNumber = proposal.slotNumber; const proposer = proposal.getSender(); // If escape hatch is open for this slot's epoch, do not attest. - if (await this.epochCache.isEscapeHatchOpenAtSlot(proposalSlotNumber)) { + const escapeHatchOpen = await budget.run(`escape hatch check for slot ${proposalSlotNumber}`, () => + this.epochCache.isEscapeHatchOpenAtSlot(proposalSlotNumber), + ); + if (escapeHatchOpen) { this.log.warn(`Escape hatch open for slot ${proposalSlotNumber}, skipping checkpoint attestation handling`); return undefined; } @@ -544,7 +574,9 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } // Check that I have any address in the committee where this checkpoint will land before attesting - const inCommittee = await this.epochCache.filterInCommittee(proposalSlotNumber, this.getValidatorAddresses()); + const inCommittee = await budget.run(`committee membership check for slot ${proposalSlotNumber}`, () => + this.epochCache.filterInCommittee(proposalSlotNumber, this.getValidatorAddresses()), + ); const partOfCommittee = inCommittee.length > 0; const proposalInfo = { @@ -564,7 +596,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) this.log.warn(`Skipping checkpoint proposal validation for slot ${proposalSlotNumber}`, proposalInfo); checkpointNumber = CheckpointNumber(0); } else { - const validationResult = await this.proposalHandler.handleCheckpointProposal(proposal, proposalInfo); + const validationResult = await this.proposalHandler.handleCheckpointProposal(proposal, proposalInfo, budget); if (!validationResult.isValid) { this.log.warn(`Checkpoint proposal validation failed: ${validationResult.reason}`, proposalInfo); return undefined; @@ -627,14 +659,10 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return undefined; } - // Everything above may have waited on local state to catch up, so the signing gate takes the slot's consensus - // attestation deadline: nothing signed after it reaches a peer that would still accept it. - return await this.createCheckpointAttestationsFromProposal( - proposal, - attestors, - checkpointNumber, - this.proposalHandler.getReexecutionDeadline(proposalSlotNumber), - ); + // Everything above may have waited on local state to catch up, so the signing gate takes the slot's duty budget, + // whose deadline is the consensus attestation deadline: nothing signed after it reaches a peer that would still + // accept it. + return await this.createCheckpointAttestationsFromProposal(proposal, attestors, checkpointNumber, budget); } /** @@ -659,42 +687,59 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) } /** - * The one place a checkpoint attestation is produced, so `deadline` gates every path that reaches it, the - * cached-verdict one included. Callers responding to a peer's proposal pass the slot's consensus attestation - * deadline; the proposer's own attestations are produced inside its publish budget instead and pass none. + * The one place a checkpoint attestation is produced, so `budget` gates every path that reaches it, the + * cached-verdict one included. Callers responding to a peer's proposal pass the slot's duty budget, whose + * deadline is the consensus attestation deadline; the proposer's own attestations are produced inside its + * publish budget instead and pass none. */ private async createCheckpointAttestationsFromProposal( proposal: CheckpointProposalCore, attestors: EthAddress[] = [], checkpointNumber: CheckpointNumber, - deadline?: Date, + budget?: DutyBudget, ): Promise { // Equivocation check: must happen right before signing to minimize the race window if (!this.shouldAttestToSlot(proposal.slotNumber)) { return undefined; } - const expired = () => deadline !== undefined && this.dateProvider.now() >= deadline.getTime(); - if (expired()) { + if (budget?.expired()) { this.log.warn(`Not requesting an attestation for slot ${proposal.slotNumber}: past the attestation deadline`, { slot: proposal.slotNumber, - deadline: deadline!.toISOString(), + deadline: budget.deadline.toISOString(), }); return undefined; } - const attestations = await this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); - - // Track the proposal we attested to (to prevent equivocation). The signing-protection record stands even when - // the signature itself turns out to be too late to use. + // Track the proposal we attested to (to prevent equivocation) before the request goes out, not after it comes + // back. A remote signer may have signed a request this node stopped waiting for, so the protection record has + // to stand for every request that was issued, however it ends. this.lastAttestedProposal = proposal; + const sign = () => this.validationService.attestToCheckpointProposal(proposal, attestors, checkpointNumber); + let attestations: CheckpointAttestation[]; + try { + attestations = budget + ? await budget.run(`checkpoint attestation signing for slot ${proposal.slotNumber}`, sign) + : await sign(); + } catch (err) { + if (!(err instanceof DutyBudgetExpiredError)) { + throw err; + } + // Signing may be remote (HA) and may never answer. The request stands as issued; this caller stops waiting. + this.log.warn(`Abandoning attestations for slot ${proposal.slotNumber}: the signer did not answer in time`, { + slot: proposal.slotNumber, + deadline: budget!.deadline.toISOString(), + }); + return undefined; + } + // Signing may be remote (HA), so a request that started in time can still return past the deadline. Peers // reject a stale attestation, so discard it rather than putting it in the pool or handing it back for gossip. - if (expired()) { + if (budget?.expired()) { this.log.warn(`Discarding attestations for slot ${proposal.slotNumber}: the signer returned too late`, { slot: proposal.slotNumber, - deadline: deadline!.toISOString(), + deadline: budget.deadline.toISOString(), }); return undefined; } From 86cafd43f4b3a3854377b17164fa22330fe5ef78 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 22:37:13 -0300 Subject: [PATCH 16/25] fix(archiver): keep an unreadable certified syncpoint from authorizing truncation The guard before the shorter-head truncation asked a three-outcome question and answered it with a boolean, so a syncpoint that could not be read came back the same as one that was positively replaced. A provider answering `latest` at an older canonical block then truncated the log to it and pruned the speculative blocks that had consumed the messages above it, without anything ever establishing that those messages were gone. The guard now reports lag, positive replacement and unresolved separately. Only positive replacement lets the head be shortened to; an unreadable syncpoint keeps the log and waits. A head that disagrees with the local log at its own count is a different question and still goes to recovery, which finds its anchor on L1, so a chain that really did shorten below the syncpoint is not left waiting for a block that will never come back. Co-Authored-By: Claude Opus 5 (1M context) --- .../archiver/src/archiver-sync.test.ts | 86 +++++++++++++++++++ .../src/modules/inbox_message_synchronizer.ts | 52 +++++++++-- 2 files changed, 129 insertions(+), 9 deletions(-) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index e6b44f551240..7fb9da967514 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1810,6 +1810,92 @@ describe('Archiver Sync', () => { expect(archiver.getL1BlockNumber()).toEqual(112n); }); + // A head reporting fewer messages than the local log looks the same whether the chain shortened or this + // provider is behind. The certified syncpoint above it is the only thing that tells them apart, so a + // syncpoint that cannot be read leaves the question open rather than settling it destructively. + describe('shorter head under a syncpoint that cannot be read', () => { + /** Certifies five messages at L1 block 110 and adds two local blocks consuming them. */ + const certifyThroughBlock110 = async () => { + const msgs = randomLeaves(5); + fake.addMessages(CheckpointNumber(1), 100n, msgs.slice(0, 3)); + fake.addMessages(CheckpointNumber(1), 102n, msgs.slice(3)); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + const blocks = await addLocalBlocksConsuming([3, 5]); + return { msgs, blockNumbers: blocks.map(b => b.number) }; + }; + + /** Makes reads of one L1 block fail the way a provider that has not reached it does; returns the undo. */ + const breakReadsOf = (l1BlockNumber: bigint, how: 'throws' | 'no block') => { + const readBlock = publicClient.getBlock.getMockImplementation()!; + publicClient.getBlock.mockImplementation((async (args: { blockNumber?: bigint } = {}) => { + if (args.blockNumber === l1BlockNumber) { + if (how === 'throws') { + throw new Error('connection reset by peer'); + } + return undefined; + } + return await readBlock(args); + }) as any); + return () => publicClient.getBlock.mockImplementation(readBlock); + }; + + /** A provider that has only reached block 105, where the Inbox holds the first three messages. */ + const showLaggedHead = () => { + fake.removeMessagesAfter(3); + fake.setL1BlockNumber(105n); + }; + + it.each(['throws', 'no block'] as const)( + 'keeps the certified messages and their blocks when the syncpoint read %s', + async how => { + const { msgs, blockNumbers } = await certifyThroughBlock110(); + breakReadsOf(110n, how); + showLaggedHead(); + + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex(msgs)); + expect(await localBlockNumbers()).toEqual(blockNumbers); + expect(pruneSpy).not.toHaveBeenCalled(); + // The lower head is not advertised as a newly certified view of the log. + expect(archiver.getL1BlockNumber()).toEqual(110n); + }, + ); + + it('truncates once the syncpoint reads back as a different block', async () => { + const { msgs } = await certifyThroughBlock110(); + // Only block 110 is re-mined, so the head at 105 keeps its identity and is positive evidence of a shorter + // chain rather than of a provider that has not caught up. + fake.reorgL1BlocksFrom(110n); + showLaggedHead(); + + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex(msgs.slice(0, 3))); + expect(pruneSpy).toHaveBeenCalledTimes(1); + expect(archiver.getL1BlockNumber()).toEqual(105n); + }); + + it('resumes without deleting or refetching anything once the provider catches up', async () => { + const { msgs, blockNumbers } = await certifyThroughBlock110(); + const restoreReads = breakReadsOf(110n, 'throws'); + showLaggedHead(); + await archiver.syncImmediate(); + + restoreReads(); + fake.addMessages(CheckpointNumber(1), 102n, msgs.slice(3)); + fake.setL1BlockNumber(110n); + await archiver.syncImmediate(); + + expect(await getStoredLeaves()).toEqual(asHex(msgs)); + expect(await localBlockNumbers()).toEqual(blockNumbers); + expect(pruneSpy).not.toHaveBeenCalled(); + expect(eventByHashSpy).not.toHaveBeenCalled(); + expect(archiver.getL1BlockNumber()).toEqual(110n); + }); + }); + it('recovers from a same-height head replacement', async () => { const msgs = randomLeaves(3); fake.addMessages(CheckpointNumber(1), 100n, msgs); diff --git a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts index c8cb2eb8759e..6969d80bfa6e 100644 --- a/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts +++ b/yarn-project/archiver/src/modules/inbox_message_synchronizer.ts @@ -226,13 +226,21 @@ export class InboxMessageSynchronizer { // A head shorter than the local log is ambiguous: the chain really did shorten, or this provider is behind the // one the log was certified against. A retained syncpoint above this head that is still canonical settles it as // lag, and lag must not delete messages or claim a lower head as synced. - if (await this.isLaggedView(head, persistedSyncPoint)) { + const laterSyncPoint = await this.checkLaterSyncPoint(head, persistedSyncPoint); + if (laterSyncPoint === 'lagged') { return pending(); } // A shorter canonical sequence whose tip hash is our prefix hash at that count is a pure truncation; the tip // itself proves where it ends, so no old placement lookup is needed. const localAtRemote = await this.stores.messages.getMessagePosition(remote.totalMessagesInserted); if (localAtRemote !== undefined && localAtRemote.rollingHash.equals(remote.rollingHash)) { + // A head that agrees with the local log through its own count is what a provider that has not caught up + // looks like, so shortening to it needs the later certified syncpoint to have been positively replaced. An + // unreadable syncpoint is no such evidence: an older canonical ancestor cannot prove the messages certified + // above it are gone, and deleting them would prune the speculative blocks that consumed them for nothing. + if (laterSyncPoint === 'unknown') { + return pending(); + } if ((await this.checkL1Block(head)) !== 'canonical') { this.log.verbose( `Could not confirm L1 head ${head.l1BlockNumber} after reading the Inbox state; ` + `not truncating`, @@ -241,6 +249,8 @@ export class InboxMessageSynchronizer { } return this.truncate(localAtRemote, head, finalizedL1Block); } + // The head disagrees with the local log at the head's own count, so this is not a view of the same chain that + // has yet to catch up: recovery searches L1 for a common anchor and only rolls back to one it found there. return this.startRecovery(head, remote); } @@ -322,7 +332,7 @@ export class InboxMessageSynchronizer { } /** - * Whether a head shorter than the local log is a lagged provider view rather than a real chain replacement. + * What a retained syncpoint above a head shorter than the local log says about that head. * * The syncpoint is the highest L1 block at which the whole stored log was found equal to the Inbox's own position. * If that block is above this head and still canonical, the chain did not shorten past it: the messages the head @@ -330,22 +340,39 @@ export class InboxMessageSynchronizer { * throw away certified messages and prune the proposed blocks that consumed them, only for the next pass to fetch * them straight back. * - * An unreadable syncpoint block is not treated as lag: without positive evidence the shorter head is handled by the - * ordinary path, which authenticates whatever it retains. + * A syncpoint that cannot be read is the same three-outcome question as any other block, and the answer has to + * survive to the caller: an unreadable one is no evidence that the certified messages above the head are gone, and + * truncating on it would delete them on the strength of a provider that could not answer. Only a syncpoint that + * reads back with a different hash is positive evidence the certified view was replaced, which the ordinary + * shorter-head path is then free to act on. */ - private async isLaggedView(head: L1BlockId, syncPoint: L1BlockId | undefined): Promise { + private async checkLaterSyncPoint(head: L1BlockId, syncPoint: L1BlockId | undefined): Promise { if (syncPoint === undefined || syncPoint.l1BlockNumber <= head.l1BlockNumber) { - return false; + return 'none'; } - if ((await this.checkL1Block(syncPoint)) !== 'canonical') { - return false; + const status = await this.checkL1Block(syncPoint); + if (status === 'unknown') { + this.log.verbose( + `Could not confirm the certified syncpoint at ${syncPoint.l1BlockNumber} above L1 head ` + + `${head.l1BlockNumber}; keeping the message log until the shorter head can be explained`, + { headL1BlockNumber: head.l1BlockNumber, syncPointL1BlockNumber: syncPoint.l1BlockNumber }, + ); + return 'unknown'; + } + if (status === 'replaced') { + this.log.warn( + `Certified syncpoint at ${syncPoint.l1BlockNumber} has been replaced; the shorter L1 head ` + + `${head.l1BlockNumber} is handled as a chain replacement`, + { headL1BlockNumber: head.l1BlockNumber, syncPointL1BlockNumber: syncPoint.l1BlockNumber }, + ); + return 'replaced'; } this.log.verbose( `L1 head ${head.l1BlockNumber} is behind the certified syncpoint at ${syncPoint.l1BlockNumber}, which is ` + `still canonical; keeping the message log and waiting for the provider to catch up`, { headL1BlockNumber: head.l1BlockNumber, syncPointL1BlockNumber: syncPoint.l1BlockNumber }, ); - return true; + return 'lagged'; } /** @@ -626,6 +653,13 @@ export class InboxMessageSynchronizer { */ type L1BlockStatus = 'canonical' | 'replaced' | 'unknown'; +/** + * What a retained certified syncpoint says about a head that reports fewer messages than the local log: there is no + * later syncpoint to ask, the syncpoint is still canonical so the head is a lagged view, the syncpoint was positively + * replaced, or it could not be read and the shortfall stays unexplained. + */ +type LaterSyncPointStatus = 'none' | 'lagged' | 'replaced' | 'unknown'; + /** The L1 head a sync pass was captured against is no longer canonical; the pass's uncommitted work is discarded. */ class CapturedHeadReplacedError extends Error { constructor(head: L1BlockId) { From a87164c8dc624cdf392aa5356118cab2cc16356b Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 22:37:14 -0300 Subject: [PATCH 17/25] docs(sequencer): state how far the Inbox capacity floor's fast-profile exemption reaches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isFastLocalProfile` is a threshold on the Ethereum slot duration, not a declaration that a node is a development one, so the exemption also covers a real network running short Ethereum slots — which faster L1 slots do not make safe, since the per-block message cap is unchanged. Say so where the exemption is taken, and pin the boundary in a test so the strict rejection one second slower is not lost. Co-Authored-By: Claude Opus 5 (1M context) --- .../sequencer-client/src/sequencer/sequencer.test.ts | 8 ++++++++ yarn-project/sequencer-client/src/sequencer/sequencer.ts | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 34df55df217d..40dad0da26f5 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -45,6 +45,7 @@ import { } from '@aztec/stdlib/interfaces/server'; import { type L1ToL2MessageSource, MIN_BLOCKS_FOR_INBOX_CATCHUP } from '@aztec/stdlib/messaging'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; +import { FAST_PROFILE_ETHEREUM_SLOT_DURATION } from '@aztec/stdlib/timetable'; import { AppendOnlyTreeSnapshot } from '@aztec/stdlib/trees'; import { BlockHeader, GlobalVariables, type Tx } from '@aztec/stdlib/tx'; import type { FullNodeCheckpointsBuilder, ValidatorClient } from '@aztec/validator-client'; @@ -465,6 +466,13 @@ describe('sequencer', () => { expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1, blockDurationMs: 2000 }, l1Constants)).not.toThrow(); }); + // The exemption is a threshold on the Ethereum slot duration, not a declaration that this is a development + // network, so pin where it ends: the same undersized configuration one second slower is rejected outright. + it('rejects the same undersized configuration at the fast-profile boundary', () => { + const atBoundary = { ...productionConstants(), ethereumSlotDuration: FAST_PROFILE_ETHEREUM_SLOT_DURATION }; + expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1 }, atBoundary)).toThrow(/streaming-Inbox backlog/); + }); + it('leaves the committed config and timetable intact when an update is rejected', () => { const sequencer = buildSequencer({ maxBlocksPerCheckpoint: 8 }); const before = sequencer.getTimeTable(); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 9dfa28d5e6cd..55d107ad5dfb 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -233,6 +233,13 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter Date: Wed, 9 Sep 2026 23:07:34 -0300 Subject: [PATCH 18/25] fix(validator): charge a duty that ran out to nobody, rather than to the proposer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recording nothing for a slot is not neutral. With no record and no checkpoint on L1 the sentinel reads the slot as one the proposer never proposed in — `checkpoint-missed`, or `blocks-missed` — and both are counted in its missed-proposal statistics, which feed inactivity. So a node whose validation duty ran out silently charged the proposer for its own clock. The taxonomy gains one outcome for what actually happened: `unverifiable`, this observer could not check the proposal against anything outside itself. It travels tracker → sentinel → statistics as `checkpoint-unverifiable`, says a proposal was seen so the missed-proposal fallback does not apply, and is in no missed-proposal filter. It never revises an outcome the slot already has: a duty that gave up learned nothing that could. Real missed and invalid proposals keep exactly the accounting they had. Co-Authored-By: Claude Opus 5 (1M context) --- .../aztec-node/src/sentinel/sentinel.test.ts | 37 +++++++++++++++++++ .../aztec-node/src/sentinel/sentinel.ts | 24 ++++++++---- .../aztec-node/src/sentinel/store.test.ts | 3 +- yarn-project/aztec-node/src/sentinel/store.ts | 4 ++ .../checkpoint_reexecution_tracker.ts | 5 ++- yarn-project/stdlib/src/validators/schemas.ts | 1 + yarn-project/stdlib/src/validators/types.ts | 6 +++ .../src/proposal_handler.test.ts | 25 ++++++++++--- .../validator-client/src/proposal_handler.ts | 19 +++++++--- 9 files changed, 105 insertions(+), 19 deletions(-) diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts index ee95f5af5187..d55e8aa07958 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts @@ -194,6 +194,26 @@ describe('sentinel', () => { expect(activity[proposer.toString()]).toEqual('checkpoint-unvalidated'); }); + // An observer that ran out of time, or could not check a proposal against L1, records that it saw one, so the + // slot is not read as one the proposer skipped — the fallback below would otherwise make it checkpoint-missed + // or blocks-missed, both of which are counted against the proposer. + it('flags checkpoint as unverifiable when tracker outcome is unverifiable', async () => { + reexecutionTracker.recordOutcome(slot, block.archive.root, 'unverifiable', CheckpointNumber(1)); + p2p.getCheckpointAttestationsForSlot.mockResolvedValue([]); + p2p.hasBlockProposalsForSlot.mockResolvedValue(true); + const activity = await sentinel.getSlotActivity(slot, epoch, proposer, committee); + expect(activity[proposer.toString()]).toEqual('checkpoint-unverifiable'); + }); + + it('does not tag attestors as missed when the checkpoint is unverifiable', async () => { + reexecutionTracker.recordOutcome(slot, block.archive.root, 'unverifiable', CheckpointNumber(1)); + p2p.getCheckpointAttestationsForSlot.mockResolvedValue(attestations.slice(0, -1)); + + const activity = await sentinel.getSlotActivity(slot, epoch, proposer, committee); + expect(activity[proposer.toString()]).toEqual('checkpoint-unverifiable'); + expect(activity[committee[3].toString()]).not.toBe('attestation-missed'); + }); + it('flags as blocks-missed when there is no tracker outcome and no block proposals (case 1)', async () => { p2p.getCheckpointAttestationsForSlot.mockResolvedValue([]); p2p.hasBlockProposalsForSlot.mockResolvedValue(false); @@ -407,6 +427,23 @@ describe('sentinel', () => { expect(stats.missedProposals.total).toEqual(5); }); + // The taxonomy's whole point: a slot this node could not check must not reach the proposer's inactivity + // accounting, while the slots it could check keep the accounting they always had. + it('does not count checkpoint-unverifiable as a missed proposal', () => { + const stats = sentinel.computeStatsForValidator(validator, [ + { slot: SlotNumber(1), status: 'checkpoint-mined' }, + { slot: SlotNumber(2), status: 'checkpoint-unverifiable' }, + { slot: SlotNumber(3), status: 'checkpoint-unverifiable' }, + { slot: SlotNumber(4), status: 'checkpoint-invalid' }, + { slot: SlotNumber(5), status: 'checkpoint-missed' }, + ]); + + expect(stats.missedProposals.count).toEqual(2); + expect(stats.missedProposals.total).toEqual(5); + // The unverifiable slots do not extend the streak the two real misses start either. + expect(stats.missedProposals.currentStreak).toEqual(2); + }); + it('resets streaks correctly', () => { const stats = sentinel.computeStatsForValidator(validator, [ { slot: SlotNumber(1), status: 'checkpoint-mined' }, diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.ts b/yarn-project/aztec-node/src/sentinel/sentinel.ts index ef2fc3c69340..e7ef775b4c81 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.ts @@ -78,10 +78,9 @@ function statusToCategory(status: ValidatorStatusInSlot): ValidatorStatusType { * Triggering per-epoch evaluation off local L2 state — rather than waiting for L1 proof * publication — decouples slashing from prover availability. * - * ## Six-case taxonomy in `getSlotActivity` + * ## Proposer taxonomy in `getSlotActivity` * - * For each slot, the sentinel assigns the proposer one of six statuses, ranked highest-confidence - * first: + * For each slot, the sentinel assigns the proposer one status, ranked highest-confidence first: * * - `checkpoint-mined` — a checkpoint covering this slot has landed on L1 * (fetched on demand via `archiver.getCheckpoint({ slot })`). @@ -93,18 +92,22 @@ function statusToCategory(status: ValidatorStatusInSlot): ValidatorStatusType { * - `checkpoint-unvalidated` — the local node observed a checkpoint proposal but could not * validate it (missing blocks/txs, timeouts). Treated as * proposer-fault for slashing. + * - `checkpoint-unverifiable` — the local node observed a checkpoint proposal and found its + * content valid, but could not check it against an authority + * outside itself. Recorded so the slot is not mistaken for one + * without a proposal, and charged to nobody. * - `checkpoint-missed` — block proposals seen on P2P but no checkpoint proposal at all. * - `blocks-missed` — no block proposals seen for this slot. * * Missing-attestor faults are recorded only in `checkpoint-mined` and `checkpoint-valid`, where - * the local node has positive evidence the checkpoint was canonical or valid. In the other four - * cases the proposer is at fault and no attestor penalty applies. + * the local node has positive evidence the checkpoint was canonical or valid. In the other cases + * no attestor penalty applies. * * ## Re-execution tracker * * `CheckpointReexecutionTracker` is populated by the validator client's checkpoint proposal * handler. Every early return in `validateCheckpointProposal` records an outcome - * (`valid` / `invalid` / `unvalidated`) keyed by slot. + * (`valid` / `invalid` / `unvalidated` / `unverifiable`) keyed by slot. * * ## Inactivity slashing * @@ -114,7 +117,7 @@ function statusToCategory(status: ValidatorStatusInSlot): ValidatorStatusType { * (read from `SentinelStore.epochMap`). Only validators meeting both conditions are emitted as * `WANT_TO_SLASH_EVENT` with `OffenseType.INACTIVITY`. The slot-level counters that feed this — * `missedProposals` and `missedAttestations` — include the four proposer-fault statuses plus - * `attestation-missed`. + * `attestation-missed`; `checkpoint-unverifiable` is a local-inability record and is in neither. * * ## Escape hatch * @@ -513,6 +516,7 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme | 'checkpoint-valid' | 'checkpoint-invalid' | 'checkpoint-unvalidated' + | 'checkpoint-unverifiable' | 'checkpoint-missed' | 'blocks-missed'; if (checkpoint) { @@ -523,6 +527,10 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme status = 'checkpoint-invalid'; } else if (reexecutionOutcome === 'unvalidated') { status = 'checkpoint-unvalidated'; + } else if (reexecutionOutcome === 'unverifiable') { + // A proposal this node saw but could not check against anything outside itself. Recording it stops the + // fallback below reading the slot as one the proposer never proposed in. + status = 'checkpoint-unverifiable'; } else { // No L1 checkpoint, no local re-execution outcome for this slot. Distinguish "proposer // sent block proposals but never made a checkpoint" from "proposer sent nothing". @@ -652,6 +660,8 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme lastProposal: this.computeFromSlot(lastProposal?.slot), lastAttestation: this.computeFromSlot(lastAttestation?.slot), totalSlots: history.length, + // `checkpoint-unverifiable` is deliberately absent: it records this node's own inability to check a + // proposal it did see, so counting it here would charge the proposer for an observer's failed L1 read. missedProposals: this.computeMissed(history, 'proposer', [ 'checkpoint-missed', 'blocks-missed', diff --git a/yarn-project/aztec-node/src/sentinel/store.test.ts b/yarn-project/aztec-node/src/sentinel/store.test.ts index cd8adf3d352e..8218237ac643 100644 --- a/yarn-project/aztec-node/src/sentinel/store.test.ts +++ b/yarn-project/aztec-node/src/sentinel/store.test.ts @@ -27,12 +27,13 @@ describe('sentinel-store', () => { it('inserts new validators with all statuses', async () => { const slot = SlotNumber(1); - const validators: `0x${string}`[] = times(8, () => EthAddress.random().toString()); + const validators: `0x${string}`[] = times(9, () => EthAddress.random().toString()); const statuses: ValidatorStatusInSlot[] = [ 'checkpoint-mined', 'checkpoint-valid', 'checkpoint-invalid', 'checkpoint-unvalidated', + 'checkpoint-unverifiable', 'checkpoint-missed', 'blocks-missed', 'attestation-sent', diff --git a/yarn-project/aztec-node/src/sentinel/store.ts b/yarn-project/aztec-node/src/sentinel/store.ts index 6f74c8c50a76..1e45adf19274 100644 --- a/yarn-project/aztec-node/src/sentinel/store.ts +++ b/yarn-project/aztec-node/src/sentinel/store.ts @@ -161,6 +161,8 @@ export class SentinelStore { return 7; case 'checkpoint-unvalidated': return 8; + case 'checkpoint-unverifiable': + return 9; default: { const _exhaustive: never = status; throw new Error(`Unknown status: ${status}`); @@ -186,6 +188,8 @@ export class SentinelStore { return 'checkpoint-invalid'; case 8: return 'checkpoint-unvalidated'; + case 9: + return 'checkpoint-unverifiable'; default: throw new Error(`Unknown status: ${status}`); } diff --git a/yarn-project/stdlib/src/checkpoint/checkpoint_reexecution_tracker.ts b/yarn-project/stdlib/src/checkpoint/checkpoint_reexecution_tracker.ts index c4b85c73e2ea..9c9333b0c3f2 100644 --- a/yarn-project/stdlib/src/checkpoint/checkpoint_reexecution_tracker.ts +++ b/yarn-project/stdlib/src/checkpoint/checkpoint_reexecution_tracker.ts @@ -10,8 +10,11 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; * - `unvalidated` — the local node could not complete validation for non-deterministic reasons * (missing blocks/txs, timeouts, infra errors). Treated as proposer-fault for * slashing but surfaced separately for telemetry. + * - `unverifiable` — this observer could not check the proposal against an authority outside itself, + * so it learned nothing about the proposer. Never proposer-fault: it is recorded + * only to keep the absence of a verdict from being read as an absent proposal. */ -export type ReexecutionOutcome = 'valid' | 'invalid' | 'unvalidated'; +export type ReexecutionOutcome = 'valid' | 'invalid' | 'unvalidated' | 'unverifiable'; /** * Tracks two pieces of per-slot state collected during proposal handling: diff --git a/yarn-project/stdlib/src/validators/schemas.ts b/yarn-project/stdlib/src/validators/schemas.ts index 15bc07beb498..6e3dc41171c6 100644 --- a/yarn-project/stdlib/src/validators/schemas.ts +++ b/yarn-project/stdlib/src/validators/schemas.ts @@ -17,6 +17,7 @@ export const ValidatorStatusInSlotSchema = zodFor()( 'checkpoint-valid', 'checkpoint-invalid', 'checkpoint-unvalidated', + 'checkpoint-unverifiable', 'checkpoint-missed', 'blocks-missed', 'attestation-sent', diff --git a/yarn-project/stdlib/src/validators/types.ts b/yarn-project/stdlib/src/validators/types.ts index 4adedf0dab43..27093dada71d 100644 --- a/yarn-project/stdlib/src/validators/types.ts +++ b/yarn-project/stdlib/src/validators/types.ts @@ -11,6 +11,11 @@ export type ValidatorStatusType = 'proposer' | 'attestation'; * - `checkpoint-missed` — block proposals seen but no checkpoint proposal (case 2). * - `checkpoint-unvalidated` — checkpoint proposal seen but local re-execution couldn't * validate (missing txs, timeouts, etc.) (case 3). + * - `checkpoint-unverifiable` — checkpoint proposal seen and content-valid, but this observer could + * not check it against an authority outside itself (an L1 read that + * failed or answered from a view it could not pin). Recorded so the + * slot is not mistaken for one without a proposal, and never counted + * against the proposer: the failure is this node's. * - `checkpoint-invalid` — checkpoint proposal re-executed and rejected as invalid (case 4). * - `checkpoint-valid` — checkpoint proposal re-executed locally as valid (case 5). * - `checkpoint-mined` — checkpoint published on L1 (case 6). @@ -25,6 +30,7 @@ export type ValidatorStatusInSlot = | 'checkpoint-valid' | 'checkpoint-invalid' | 'checkpoint-unvalidated' + | 'checkpoint-unverifiable' | 'checkpoint-missed' | 'blocks-missed' | 'attestation-sent' diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index c269d9246b79..f2c19780becc 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -469,10 +469,10 @@ describe('ProposalHandler checkpoint validation', () => { expect(result).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); }); - // Running out of time is this node giving up, not something it learned about the proposal, so it neither - // caches a verdict for the next caller nor records an outcome the sentinel would read as a missed proposal. - it('neither caches nor records an outcome when the duty budget runs out', async () => { - const recordSpy = jest.spyOn(reexecutionTracker, 'recordOutcome'); + // Running out of time is this node giving up, not something it learned about the proposal. It caches no + // verdict, and records the one outcome that is charged to nobody — recording nothing would leave the sentinel + // to fall back to `checkpoint-missed`, which is counted against the proposer. + it('records the slot as unverifiable, and caches nothing, when the duty budget runs out', async () => { blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); dateProvider.setTime(39_800); const proposal = await makeProposal(); @@ -481,7 +481,7 @@ describe('ProposalHandler checkpoint validation', () => { isValid: false, reason: 'validation_deadline_expired', }); - expect(recordSpy).not.toHaveBeenCalled(); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('unverifiable'); // The next caller revalidates from scratch instead of inheriting the abandoned duty's non-verdict. const archiveRoot = proposal.archive; @@ -495,6 +495,21 @@ describe('ProposalHandler checkpoint validation', () => { checkpointNumber: CheckpointNumber(1), }); }); + + // A duty that gave up learned nothing that could revise what the slot already says, in either direction. + it('leaves an outcome the slot already has alone when a later duty runs out', async () => { + const proposal = await makeProposal(); + reexecutionTracker.recordOutcome(SlotNumber(1), proposal.archive, 'valid', CheckpointNumber(1)); + + blockSource.getBlocksForSlot.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(39_800); + + expect(await handler.handleCheckpointProposal(proposal, proposalInfo)).toEqual({ + isValid: false, + reason: 'validation_deadline_expired', + }); + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('valid'); + }); }); describe('checkpoint proposal pipelining timing', () => { diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index e0d047314762..08fa5b1e834f 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -194,8 +194,9 @@ 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', - // This node ran out of time to look; it observed nothing about the proposer. - validation_deadline_expired: undefined, + // 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', checkpoint_validation_failed: 'invalid', }; @@ -1589,12 +1590,20 @@ export class ProposalHandler { if (!(err instanceof DutyBudgetExpiredError)) { throw err; } - // Every stage runs inside the budget, so an expiry anywhere lands here with nothing cached and nothing - // recorded: the duty stopped looking, which is not an observation about the proposal. + // Every stage runs inside the budget, so an expiry anywhere lands here with nothing cached: the duty + // stopped looking, which is not a verdict on the proposal and must not be reused as one. this.log.warn(`Ran out of duty budget validating the checkpoint proposal for slot ${proposal.slotNumber}`, { ...proposalInfo, deadline: budget.deadline.toISOString(), }); + // Something does have to be recorded, though. With no record and no checkpoint on L1 the sentinel reads the + // slot as one the proposer never proposed in, which is counted against it — so silence here would blame the + // proposer for this node's clock running out. `unverifiable` says a proposal was seen and could not be + // checked, and is counted against nobody. It never revises an outcome already recorded for the slot: a duty + // that gave up learned nothing that could. + if (this.reexecutionTracker.getOutcomeForSlot(proposal.slotNumber) === undefined) { + this.reexecutionTracker.recordOutcome(proposal.slotNumber, proposal.archive, 'unverifiable'); + } return { isValid: false, reason: 'validation_deadline_expired' }; } } @@ -1715,7 +1724,7 @@ export class ProposalHandler { ); } catch (err) { // A budget expiry is the duty giving up, not a verdict on the proposal: it propagates to the caller, which - // answers with `validation_deadline_expired` and records nothing. + // answers with `validation_deadline_expired`, which is charged to nobody. if (err instanceof DutyBudgetExpiredError) { throw err; } From f46347ae4b70814c20569b6ff81f4071c553cead Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 9 Sep 2026 23:37:23 -0300 Subject: [PATCH 19/25] fix(validator): close the remaining gaps in the duty budget and the unassessable-slot accounting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second review pass over the budget work found four things the first one missed. The reads inside the Inbox prefix wait were still unbounded — the first read, and every attempt of the retry loop, which consults its deadline only after an attempt returns. So was the fork's archive-root read. Each now runs inside the budget, and the attestation entry point races its whole body as well, so an await this audit missed cannot hold p2p's callback open past the slot. Racing a resource acquisition is not enough on its own: the losing continuation could still open a world-state fork nobody would ever close, because the caller never reached the `await using` that owns it, and could start rebuilding a checkpoint against a fork the caller was already disposing. Both now check the signal they are handed, and the fork read closes what it opened. An expiry in a stage before validation is reached never passed through the recording inside, so on a node that is not a validator — where the all-nodes callback is the only handler there is — the slot still read as one the proposer skipped. The outermost boundary records too. And an unassessable slot was being left in the denominator of the missed-proposal rate while excluded from its numerator, so one real miss among nine unverifiable slots reported as 10% missed rather than 100%. A slot nobody could assess is not a slot survived: it now counts towards neither half, and does not break a miss streak. Co-Authored-By: Claude Opus 5 (1M context) --- .../aztec-node/src/sentinel/sentinel.test.ts | 22 +++++- .../aztec-node/src/sentinel/sentinel.ts | 17 ++++- .../src/proposal_handler.test.ts | 22 ++++++ .../validator-client/src/proposal_handler.ts | 74 ++++++++++++++----- .../validator-client/src/validator.ts | 30 +++++++- 5 files changed, 139 insertions(+), 26 deletions(-) diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts index d55e8aa07958..e61c2fdbc699 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.test.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.test.ts @@ -29,6 +29,7 @@ import { import type { ValidatorStats, ValidatorStatusHistory, + ValidatorStatusInSlot, ValidatorsEpochPerformance, ValidatorsStats, } from '@aztec/stdlib/validators'; @@ -439,11 +440,28 @@ describe('sentinel', () => { ]); expect(stats.missedProposals.count).toEqual(2); - expect(stats.missedProposals.total).toEqual(5); - // The unverifiable slots do not extend the streak the two real misses start either. + // Out of the denominator too, so unknowns cannot dilute the misses that are real: two of the three slots + // this node could actually assess were missed, not two of five. + expect(stats.missedProposals.total).toEqual(3); + expect(stats.missedProposals.rate).toBeCloseTo(2 / 3); + // Nor do they break the streak the two real misses form. expect(stats.missedProposals.currentStreak).toEqual(2); }); + // A proposer whose checkpoints this node can never check must not come out looking better than one whose + // checkpoints it can: an unassessable slot is not a slot survived. + it('does not let unverifiable slots dilute the missed-proposal rate', () => { + const stats = sentinel.computeStatsForValidator(validator, [ + { slot: SlotNumber(1), status: 'checkpoint-missed' }, + ...times(9, (i): { slot: SlotNumber; status: ValidatorStatusInSlot } => ({ + slot: SlotNumber(i + 2), + status: 'checkpoint-unverifiable', + })), + ]); + + expect(stats.missedProposals).toEqual(expect.objectContaining({ count: 1, total: 1, rate: 1, currentStreak: 1 })); + }); + it('resets streaks correctly', () => { const stats = sentinel.computeStatsForValidator(validator, [ { slot: SlotNumber(1), status: 'checkpoint-mined' }, diff --git a/yarn-project/aztec-node/src/sentinel/sentinel.ts b/yarn-project/aztec-node/src/sentinel/sentinel.ts index e7ef775b4c81..9ff5d4785293 100644 --- a/yarn-project/aztec-node/src/sentinel/sentinel.ts +++ b/yarn-project/aztec-node/src/sentinel/sentinel.ts @@ -41,6 +41,14 @@ export type SentinelRuntimeConfig = Pick< Pick & Pick; +/** + * Statuses that record only that this node could not assess the slot. They are evidence of nothing, so they are + * left out of both halves of a missed-duty rate: counting them as misses would charge a validator for an + * observer's failure, and counting them as successes would let unknowns dilute the misses that are real. A slot + * nobody could assess simply does not count towards the rate. + */ +const UNASSESSABLE_STATUSES: ValidatorStatusInSlot[] = ['checkpoint-unverifiable']; + /** Maps a validator status to its category: proposer or attestation. */ function statusToCategory(status: ValidatorStatusInSlot): ValidatorStatusType { switch (status) { @@ -660,8 +668,9 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme lastProposal: this.computeFromSlot(lastProposal?.slot), lastAttestation: this.computeFromSlot(lastAttestation?.slot), totalSlots: history.length, - // `checkpoint-unverifiable` is deliberately absent: it records this node's own inability to check a - // proposal it did see, so counting it here would charge the proposer for an observer's failed L1 read. + // `checkpoint-unverifiable` is deliberately absent, and {@link UNASSESSABLE_STATUSES} keeps it out of the + // denominator too: it records this node's own inability to check a proposal it did see, so counting it + // either way would misreport the proposer. missedProposals: this.computeMissed(history, 'proposer', [ 'checkpoint-missed', 'blocks-missed', @@ -679,7 +688,9 @@ export class Sentinel extends (EventEmitter as new () => WatcherEmitter) impleme filter: ValidatorStatusInSlot[], ) { const relevantHistory = history.filter( - h => !computeOverCategory || statusToCategory(h.status) === computeOverCategory, + h => + !UNASSESSABLE_STATUSES.includes(h.status) && + (!computeOverCategory || statusToCategory(h.status) === computeOverCategory), ); const filteredHistory = relevantHistory.filter(h => filter.includes(h.status)); return { diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index f2c19780becc..731809128c83 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -496,6 +496,25 @@ describe('ProposalHandler checkpoint validation', () => { }); }); + // The recording inside `handleCheckpointProposal` never runs for an expiry in a stage before it — and on a + // node that is not a validator the all-nodes callback is the only handler there is, so the outermost + // boundary has to record too, or the slot reads as one the proposer skipped. + it('records the slot as unverifiable when the duty runs out before validation is reached', async () => { + const proposal = await makeProposal(); + const p2p = mock(); + let checkpointHandler: ((proposal: any, sender: any) => Promise) | undefined; + p2p.registerAllNodesCheckpointProposalHandler.mockImplementation(handler => { + checkpointHandler = handler; + }); + epochCache.isEscapeHatchOpenAtSlot.mockImplementation(() => new Promise(() => {})); + dateProvider.setTime(39_800); + + handler.register(p2p, true); + await checkpointHandler!(proposal, {} as any); + + expect(reexecutionTracker.getOutcomeForSlot(SlotNumber(1))).toEqual('unverifiable'); + }); + // A duty that gave up learned nothing that could revise what the slot already says, in either direction. it('leaves an outcome the slot already has alone when a later duty runs out', async () => { const proposal = await makeProposal(); @@ -1110,6 +1129,9 @@ describe('ProposalHandler checkpoint validation', () => { expect(await validation).toEqual({ isValid: false, reason: 'validation_deadline_expired' }); expect(checkpointsBuilder.openCheckpoint).not.toHaveBeenCalled(); + // The caller never reached the `await using` that would have closed this fork, so the abandoned read has + // to close it: otherwise every duty that gives up here leaks a world-state fork. + expect(mockDispose).toHaveBeenCalled(); }); it('disposes fork even when validation fails', async () => { diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index 08fa5b1e834f..eadd3414ebc0 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -464,6 +464,10 @@ export class ProposalHandler { slot: proposal.slotNumber, deadline: budget.deadline.toISOString(), }); + // An expiry in a stage before validation is reached — the escape-hatch or own-proposal read — never + // passes through the recording inside. This is the outermost boundary, and on a node that is not a + // validator it is the only one, so the neutral record is made here too. + this.recordUncheckedProposal(proposal); } else { this.log.warn(`Error handling checkpoint proposal for slot ${proposal.slotNumber}`, { err }); } @@ -856,14 +860,17 @@ export class ProposalHandler { slotNumber: SlotNumber, what: string, resolve: () => Promise, - stop?: AbortSignal, + { budget, stop = budget?.signal }: { budget?: DutyBudget; stop?: AbortSignal } = {}, ): Promise { const deadline = this.getReexecutionDeadline(slotNumber); if (deadline.getTime() - this.dateProvider.now() <= 0 || stop?.aborted) { return undefined; } - try { - return await retryUntil( + // `retryUntil` consults its deadline only after an attempt returns, so a sync or a read that never settles + // never reaches it. A caller that owns a duty budget races the whole loop against it as well; one that does + // not (the block-proposal paths) keeps the deadline-between-attempts behaviour it always had. + const loop = () => + retryUntil( async () => { if (stop?.aborted) { throw new TimeoutError(`Stopped waiting for ${what}`); @@ -875,6 +882,10 @@ export class ProposalHandler { { deadline, dateProvider: this.dateProvider }, 0.5, ); + try { + // A budget expiry propagates: it is the duty giving up, which the caller answers with its own reason + // rather than as a local-view verdict against the proposer. + return budget ? await budget.run(what, loop) : await loop(); } catch (err) { if (err instanceof TimeoutError) { return undefined; @@ -1186,7 +1197,7 @@ export class ProposalHandler { const result = await readBundle(); return !result.accepted && isRetryableStreamingBlockCheckReason(result.reason) ? undefined : result; }, - stop, + { stop }, ); if (resolved === undefined) { this.log.warn(`Timed out reading a consistent Inbox bundle, rejecting proposal`, { @@ -1412,7 +1423,8 @@ export class ProposalHandler { ): Promise<{ accepted: true; messages: Fr[] } | { accepted: false; reason: CheckpointInboxPrefixReason }> { const read = () => this.readCheckpointConsumedMessages(checkpointStartTotal, lastBlockTotal, checkpointInboxRollingHash); - const first = await read(); + const what = `inbox prefix for checkpoint at slot ${slot}`; + const first = budget ? await budget.run(what, read) : await read(); if (first.accepted) { return first; } @@ -1424,12 +1436,12 @@ export class ProposalHandler { }); const resolved = await this.awaitLocalSync( slot, - `inbox prefix for checkpoint at slot ${slot}`, + what, async () => { const result = await read(); return result.accepted ? result : undefined; }, - budget?.signal, + { budget }, ); if (resolved === undefined) { this.log.warn(`Timed out waiting for the checkpoint's consumed Inbox prefix to sync, refusing to attest`, { @@ -1596,18 +1608,26 @@ export class ProposalHandler { ...proposalInfo, deadline: budget.deadline.toISOString(), }); - // Something does have to be recorded, though. With no record and no checkpoint on L1 the sentinel reads the - // slot as one the proposer never proposed in, which is counted against it — so silence here would blame the - // proposer for this node's clock running out. `unverifiable` says a proposal was seen and could not be - // checked, and is counted against nobody. It never revises an outcome already recorded for the slot: a duty - // that gave up learned nothing that could. - if (this.reexecutionTracker.getOutcomeForSlot(proposal.slotNumber) === undefined) { - this.reexecutionTracker.recordOutcome(proposal.slotNumber, proposal.archive, 'unverifiable'); - } + this.recordUncheckedProposal(proposal); return { isValid: false, reason: 'validation_deadline_expired' }; } } + /** + * Records that a proposal was seen and could not be checked, for a duty that ran out of time. + * + * Something does have to be recorded. With no record and no checkpoint on L1 the sentinel reads the slot as one + * the proposer never proposed in, which is counted against it — so silence here would blame the proposer for + * this node's clock running out. `unverifiable` says a proposal was seen and could not be checked, and is + * counted against nobody. It never revises an outcome the slot already has: a duty that gave up learned nothing + * that could. + */ + private recordUncheckedProposal(proposal: CheckpointProposalCore): void { + if (this.reexecutionTracker.getOutcomeForSlot(proposal.slotNumber) === undefined) { + this.reexecutionTracker.recordOutcome(proposal.slotNumber, proposal.archive, 'unverifiable'); + } + } + /** {@link handleCheckpointProposal}'s body, outside its duty-budget handling. */ private async validateAndRecordCheckpointProposal( proposal: ValidatedCheckpointProposalCore, @@ -1873,9 +1893,17 @@ export class ProposalHandler { const parentBlockNumber = BlockNumber(firstBlock.number - 1); let forkResult: MerkleTreeWriteOperations; try { - forkResult = await budget.run(`world state fork at block ${parentBlockNumber} for slot ${slot}`, async () => { + const what = `world state fork at block ${parentBlockNumber} for slot ${slot}`; + forkResult = await budget.run(what, async signal => { const parentBlockHash = (await this.blockSource.getBlockData({ number: parentBlockNumber }))?.blockHash; - return await this.checkpointsBuilder.getFork(parentBlockNumber, parentBlockHash); + const opened = await this.checkpointsBuilder.getFork(parentBlockNumber, parentBlockHash); + if (signal.aborted) { + // The caller lost the race and never reached the `await using` that would have closed this fork, so + // closing it is this continuation's job: otherwise every abandoned duty leaks a world-state fork. + await opened[Symbol.asyncDispose](); + throw new DutyBudgetExpiredError(what, budget.deadline); + } + return opened; }); } catch (err) { if (err instanceof DutyBudgetExpiredError) { @@ -1895,7 +1923,9 @@ export class ProposalHandler { // built on (e.g. a reorg), so recomputing the checkpoint against it would be meaningless. This mirrors // the block-proposal re-execution check and fails fast with a clean, non-slashable result instead of a // confusing downstream mismatch. - const forkArchiveRoot = new Fr((await fork.getTreeInfo(MerkleTreeId.ARCHIVE)).root); + const forkArchiveRoot = new Fr( + (await budget.run(`fork archive root for slot ${slot}`, () => fork.getTreeInfo(MerkleTreeId.ARCHIVE))).root, + ); if (!forkArchiveRoot.equals(proposal.checkpointHeader.lastArchiveRoot)) { this.log.warn(`Fork archive root does not match checkpoint proposal's last archive`, { ...proposalInfo, @@ -1906,7 +1936,8 @@ export class ProposalHandler { } // Create checkpoint builder with all existing blocks, and complete it to get the computed values. - const computedCheckpoint = await budget.run(`checkpoint reconstruction for slot ${slot}`, async () => { + const reconstruction = `checkpoint reconstruction for slot ${slot}`; + const computedCheckpoint = await budget.run(reconstruction, async signal => { const checkpointBuilder = await this.checkpointsBuilder.openCheckpoint( checkpointNumber, constants, @@ -1918,6 +1949,11 @@ export class ProposalHandler { blocks, this.log.getBindings(), ); + // The caller's `await using` starts closing the fork the moment it loses the race, and the rebuild reads + // from that fork, so a late `openCheckpoint` must not run one against a fork that is going away. + if (signal.aborted) { + throw new DutyBudgetExpiredError(reconstruction, budget.deadline); + } return await checkpointBuilder.completeCheckpoint(); }); diff --git a/yarn-project/validator-client/src/validator.ts b/yarn-project/validator-client/src/validator.ts index b49d5dd16efd..675313dca950 100644 --- a/yarn-project/validator-client/src/validator.ts +++ b/yarn-project/validator-client/src/validator.ts @@ -526,7 +526,11 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) // preliminary reads and the signer await are unbounded, and p2p waits on this callback. const budget = new DutyBudget(this.proposalHandler.getReexecutionDeadline(proposal.slotNumber), this.dateProvider); try { - return await this.attestToCheckpointProposalWithinBudget(proposal, budget); + // Every stage inside consults the budget, and the whole body is raced against it as well: an await this + // audit missed, or one a later change adds, still cannot hold p2p's callback open past the slot. + return await budget.run(`checkpoint attestation for slot ${proposal.slotNumber}`, () => + this.attestToCheckpointProposalWithinBudget(proposal, budget), + ); } catch (err) { if (!(err instanceof DutyBudgetExpiredError)) { throw err; @@ -744,7 +748,29 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter) return undefined; } - await this.p2pClient.addOwnCheckpointAttestations(attestations); + // The pool write is another await that can outlast the slot, and what comes back from here is gossiped, so + // it is bounded and the deadline rechecked after it rather than only after signing. + const pool = () => this.p2pClient.addOwnCheckpointAttestations(attestations); + try { + budget ? await budget.run(`attestation pool write for slot ${proposal.slotNumber}`, pool) : await pool(); + } catch (err) { + if (!(err instanceof DutyBudgetExpiredError)) { + throw err; + } + this.log.warn(`Abandoning attestations for slot ${proposal.slotNumber}: the pool did not accept them in time`, { + slot: proposal.slotNumber, + deadline: budget!.deadline.toISOString(), + }); + return undefined; + } + + if (budget?.expired()) { + this.log.warn(`Not gossiping attestations for slot ${proposal.slotNumber}: the deadline passed`, { + slot: proposal.slotNumber, + deadline: budget.deadline.toISOString(), + }); + return undefined; + } return attestations; } From d03049fb2fe7e32010b1e3a9850a426b7d2b0610 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 02:10:34 -0300 Subject: [PATCH 20/25] fix(sequencer): exempt the e2e cadence from the Inbox catch-up floor The floor was meant to reject only production configs, but its exemption reused isFastLocalProfile, which is strictly below FAST_PROFILE_ETHEREUM_SLOT_DURATION. DEFAULT_L1_BLOCK_TIME sits at exactly that boundary, deliberately, so single-node e2e runs keep the production timing budgets; at that cadence a 16s L2 slot derives 2 block opportunities and every default-cadence e2e run was rejected at node creation. Make the exemption boundary-inclusive and pin its end one second above the boundary instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/sequencer/sequencer.test.ts | 16 +++++++++-- .../src/sequencer/sequencer.ts | 28 ++++++++++++------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 40dad0da26f5..01ca0d14910a 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -466,11 +466,21 @@ describe('sequencer', () => { expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1, blockDurationMs: 2000 }, l1Constants)).not.toThrow(); }); + // The single-node e2e default (DEFAULT_L1_BLOCK_TIME) sits at exactly this boundary, deliberately, so that + // those runs keep the production timing budgets. Rejecting there would fail every default-cadence e2e run. + it('warns rather than rejects at the fast-profile boundary', () => { + const atBoundary = { ...productionConstants(), ethereumSlotDuration: FAST_PROFILE_ETHEREUM_SLOT_DURATION }; + expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1 }, atBoundary)).not.toThrow(); + }); + // The exemption is a threshold on the Ethereum slot duration, not a declaration that this is a development // network, so pin where it ends: the same undersized configuration one second slower is rejected outright. - it('rejects the same undersized configuration at the fast-profile boundary', () => { - const atBoundary = { ...productionConstants(), ethereumSlotDuration: FAST_PROFILE_ETHEREUM_SLOT_DURATION }; - expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1 }, atBoundary)).toThrow(/streaming-Inbox backlog/); + it('rejects the same undersized configuration just above the fast-profile boundary', () => { + const aboveBoundary = { + ...productionConstants(), + ethereumSlotDuration: FAST_PROFILE_ETHEREUM_SLOT_DURATION + 1, + }; + expect(() => buildSequencer({ maxBlocksPerCheckpoint: 1 }, aboveBoundary)).toThrow(/streaming-Inbox backlog/); }); it('leaves the committed config and timetable intact when an update is rejected', () => { diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 55d107ad5dfb..211bea98781e 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -39,7 +39,11 @@ import { import { type L1ToL2MessageSource, MIN_BLOCKS_FOR_INBOX_CATCHUP } from '@aztec/stdlib/messaging'; import type { CoordinationSignatureContext } from '@aztec/stdlib/p2p'; import { pickFromSchema } from '@aztec/stdlib/schemas'; -import { ProposerTimetable, buildProposerTimetable, isFastLocalProfile } from '@aztec/stdlib/timetable'; +import { + FAST_PROFILE_ETHEREUM_SLOT_DURATION, + ProposerTimetable, + buildProposerTimetable, +} from '@aztec/stdlib/timetable'; import { Attributes, type TelemetryClient, type Tracer, getTelemetryClient, trackSpan } from '@aztec/telemetry-client'; import { FullNodeCheckpointsBuilder, NodeKeystoreAdapter, type ValidatorClient } from '@aztec/validator-client'; @@ -230,16 +234,20 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter TypedEventEmitter Date: Thu, 10 Sep 2026 09:47:40 -0300 Subject: [PATCH 21/25] test(e2e): give proof_boundary a propagation budget that clears the Inbox floor MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING leaves attestationPropagationTime per-test, and proof_boundary was the only suite on the profile not setting one. At the default of 2 its 24s slot derives 3 block opportunities against a floor of 4, so the new capacity check refused the config and every case failed in setupTest. Its two siblings already pin 1 and 0.5; 1 derives 4 here. --- .../block-production/proof_boundary.parallel.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/yarn-project/end-to-end/src/multi-node/block-production/proof_boundary.parallel.test.ts b/yarn-project/end-to-end/src/multi-node/block-production/proof_boundary.parallel.test.ts index 6588864fcbd0..c3f1b89385fe 100644 --- a/yarn-project/end-to-end/src/multi-node/block-production/proof_boundary.parallel.test.ts +++ b/yarn-project/end-to-end/src/multi-node/block-production/proof_boundary.parallel.test.ts @@ -50,6 +50,10 @@ describe('multi-node/block-production/proof_boundary', () => { test = await MultiNodeTestContext.setup({ ...MOCK_GOSSIP_MULTI_VALIDATOR_OPTS, ...MULTI_VALIDATOR_BLOCK_PRODUCTION_TIMING, + // The profile leaves this per-test. At the default of 2 the 24s slot derives only 3 block opportunities, + // under the Inbox catch-up floor, and the sequencer refuses the config at startup; 1 derives the 4 the + // floor wants. Gossip is mocked here, so the shorter propagation budget costs this suite nothing. + attestationPropagationTime: 1, initialValidators: validators, ...overrides, }); From 9a21000501cdd3cf3b70b98e26964308aa23f3ac Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 10:15:34 -0300 Subject: [PATCH 22/25] fix(sequencer): do not start a tx poll the proposal send budget cannot cover A proposer holding 500ms of send budget would still enter a 500ms transaction poll, spend the whole remainder on a wait it could not act on, and then miss the send deadline during signing and archiver insertion. Peers refuse the proposal once that deadline passes, so the poll cannot produce a block anyone would accept. Check the remaining budget before each wait and, when a full interval will not fit, stop waiting and let the checkpoint go with the blocks already built. This is the defect class the send-deadline budgeting was introduced for: do not start work the budget cannot cover. The deadline itself is unchanged. --- .../sequencer/checkpoint_proposal_job.test.ts | 48 +++++++++++++++++++ .../src/sequencer/checkpoint_proposal_job.ts | 20 ++++++++ 2 files changed, 68 insertions(+) diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index a20672e2c17d..edb39f056dfa 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -1401,6 +1401,49 @@ describe('CheckpointProposalJob', () => { expect(publisher.enqueueProposeCheckpoint).not.toHaveBeenCalled(); }); + // The tx-polling interval, which must match TXS_POLLING_MS in checkpoint_proposal_job.ts. + const TXS_POLLING_MS = 500; + + // Sets the clock inside the tx-waiting deadline, so only the send budget can stop the wait, and pins the send + // deadline `remainingMs` away from it. Returns a spy that counts waits and advances the clock like a real one. + const armTxPollWithSendBudget = (remainingMs: number) => { + jest + .spyOn(job.getTimetable(), 'selectNextSubslot') + .mockReturnValueOnce(subslot(10, 0, true)) + .mockReturnValue(noSubslot()); + p2p.getPendingTxCount.mockResolvedValue(10); + p2p.hasEligiblePendingTxs.mockResolvedValue(false); + + // The wait-for-txs deadline is the subslot deadline (+10s) less minBlockDuration (2s), so +1s is well inside it. + const nowMs = (buildFrameStartSeconds() + 1) * 1000; + dateProvider.setTime(nowMs); + jest.spyOn(job.getTimetable(), 'getCheckpointProposalSendDeadline').mockReturnValue((nowMs + remainingMs) / 1000); + + job.updateConfig({ minTxsPerBlock: 5, buildCheckpointIfEmpty: false }); + return jest.spyOn(job, 'waitForTxsPollingInterval').mockImplementation(() => { + dateProvider.setTime(dateProvider.now() + TXS_POLLING_MS); + return Promise.resolve(); + }); + }; + + it('does not start a tx poll the proposal send budget cannot cover', async () => { + const pollSpy = armTxPollWithSendBudget(TXS_POLLING_MS - 1); + + await job.executeAndAwait(); + + // A poll here would end past the send deadline, so it buys nothing and costs the rest of the budget. + expect(pollSpy).not.toHaveBeenCalled(); + expect(checkpointBuilder.buildBlockCalls).toHaveLength(0); + }); + + it('still waits for txs when a full poll fits inside the proposal send budget', async () => { + const pollSpy = armTxPollWithSendBudget(TXS_POLLING_MS + 1); + + await job.executeAndAwait(); + + expect(pollSpy).toHaveBeenCalled(); + }); + it('stops building when selectNextSubslot returns false', async () => { // Mock timetable to stop after 1 block (simulating time running out) jest @@ -2587,6 +2630,11 @@ class TestCheckpointProposalJob extends CheckpointProposalJob { await this.pendingRequests.awaitRequests(); } + /** Widened so tests whose subject is whether the job waits at all can observe or stub the wait. */ + public override waitForTxsPollingInterval(): Promise { + return super.waitForTxsPollingInterval(); + } + /** Wraps execute + awaitPendingSubmission so tests see the full pipeline complete. */ public async executeAndAwait(): Promise { const result = await this.execute(); diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index bbc03889b9f3..c0cb7636efc0 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -2001,6 +2001,26 @@ export class CheckpointProposalJob implements Traceable { return { canStartBuilding: false, minTxs }; } + // Never start a poll the send budget cannot cover. Peers refuse this slot's proposal once the send deadline + // passes, so a full interval that ends past it cannot produce a block anyone would accept, and spending it + // leaves nothing for the signing and archiver insertion still to come. Stop waiting and let the checkpoint + // go with the blocks already built. + const sendDeadline = this.getProposalSendDeadline(); + if (sendDeadline.getTime() - now.getTime() < TXS_POLLING_MS) { + this.log.verbose( + `Not waiting for txs to build block ${blockNumber} at index ${indexWithinCheckpoint} in slot ` + + `${this.targetSlot}: a poll would outlast the proposal send deadline`, + { + blockNumber, + slot: this.targetSlot, + indexWithinCheckpoint, + minTxs, + sendDeadline: sendDeadline.toISOString(), + }, + ); + return { canStartBuilding: false, minTxs }; + } + // Wait a bit before checking again this.setState(SequencerState.WAITING_FOR_TXS); this.log.verbose( From 69e96fd6a3e48abaf0725f9935b61f3b0ee8859c Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 10:33:03 -0300 Subject: [PATCH 23/25] fix(sequencer): do not wait out a sub-slot the proposal send budget cannot cover Giving up on a block for want of txs falls through to a wait for the next sub-slot, which is longer than the tx poll the previous commit stopped starting and is governed by the same budget. Without this the poll guard just moves the overrun: the block is abandoned promptly and the slot is then spent waiting anyway, carrying the blocks already built past the send deadline. Break out instead, so the checkpoint goes while it can still be sent. --- .../sequencer/checkpoint_proposal_job.test.ts | 23 +++++++++++++++++++ .../src/sequencer/checkpoint_proposal_job.ts | 10 ++++++++ 2 files changed, 33 insertions(+) diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts index edb39f056dfa..07b0ff6470f7 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.test.ts @@ -1444,6 +1444,29 @@ describe('CheckpointProposalJob', () => { expect(pollSpy).toHaveBeenCalled(); }); + it('does not wait out another sub-slot once the proposal send budget is spent', async () => { + // Two buildable sub-slots, so a failed first block would normally wait for the second and retry. + jest + .spyOn(job.getTimetable(), 'selectNextSubslot') + .mockReturnValueOnce(subslot(10, 0, false)) + .mockReturnValueOnce(subslot(20, 1, true)) + .mockReturnValue(noSubslot()); + p2p.getPendingTxCount.mockResolvedValue(10); + p2p.hasEligiblePendingTxs.mockResolvedValue(false); + + const nowMs = (buildFrameStartSeconds() + 1) * 1000; + dateProvider.setTime(nowMs); + jest + .spyOn(job.getTimetable(), 'getCheckpointProposalSendDeadline') + .mockReturnValue((nowMs + TXS_POLLING_MS - 1) / 1000); + const subslotSpy = jest.spyOn(job, 'waitUntilNextSubslot'); + + job.updateConfig({ minTxsPerBlock: 5, buildCheckpointIfEmpty: false }); + await job.executeAndAwait(); + + expect(subslotSpy).not.toHaveBeenCalled(); + }); + it('stops building when selectNextSubslot returns false', async () => { // Mock timetable to stop after 1 block (simulating time running out) jest diff --git a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts index c0cb7636efc0..e894e8515c00 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -1288,6 +1288,16 @@ export class CheckpointProposalJob implements Traceable { if (timingInfo.isLastBlock) { break; } + // Waiting out a whole sub-slot is subject to the same budget as waiting out a tx poll, and is longer: once + // the proposal can no longer be sent, another attempt cannot produce a block any peer would accept, and the + // wait would take the checkpoint we already have past the deadline with it. + if (this.getProposalSendDeadline().getTime() - this.dateProvider.now() < TXS_POLLING_MS) { + this.log.verbose( + `Not waiting for another sub-slot in slot ${this.targetSlot}: the proposal send deadline is too close`, + { slot: this.targetSlot, checkpointNumber: this.checkpointNumber, blocksBuilt }, + ); + break; + } // Otherwise, if there is still time for more blocks, we wait until the next subslot and try again await this.waitUntilNextSubslot(timingInfo.deadline); continue; From 2e738504d2b1301fe5cdbee34920d8515ca28711 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 11:10:19 -0300 Subject: [PATCH 24/25] test(e2e): hold block production to force the same-block message consume A block freezes its L1-to-L2 message set when it is prepared, which happens before the message's L1 tx is mined, so a consume tx sent as soon as the archiver observes the message joins the block already building and reverts one block early. The case retried five times, but the attempts were a slot apart and so all landed at the same phase, repeating one experiment rather than sampling five; it passed only when something unrelated delayed the send into the gap it needs. Hold production while the message is sent and observed and the consume tx is queued, so the first block prepared after the resume both inserts the message and executes the tx. The same-block condition is now asserted rather than retried for, since a consume one block late proves nothing. --- .../cross-chain/streaming_inbox.test.ts | 81 ++++++++++--------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts b/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts index 7f41846065aa..a4d4b6486546 100644 --- a/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts +++ b/yarn-project/end-to-end/src/single-node/cross-chain/streaming_inbox.test.ts @@ -1,9 +1,11 @@ import type { Archiver } from '@aztec/archiver'; import type { AztecAddress } from '@aztec/aztec.js/addresses'; +import { NO_WAIT } from '@aztec/aztec.js/contracts'; import { generateClaimSecret } from '@aztec/aztec.js/ethereum'; import { Fr } from '@aztec/aztec.js/fields'; import type { Logger } from '@aztec/aztec.js/log'; import type { AztecNode } from '@aztec/aztec.js/node'; +import { waitForTx } from '@aztec/aztec.js/node'; import { TxExecutionResult } from '@aztec/aztec.js/tx'; import type { Wallet } from '@aztec/aztec.js/wallet'; import { BlockNumber, SlotNumber } from '@aztec/foundation/branded-types'; @@ -398,52 +400,55 @@ describe('single-node/cross-chain/streaming_inbox', () => { // Test 5 (forced same-block consumption): a public tx consuming a message that the *same* block inserts must // succeed. The block builder appends the block's messages to its fork before executing txs, matching the prover // and the block-root circuit (which pins each tx's L1-to-L2 tree snapshot to the post-append root); if it did - // not, this tx would revert at proposal time and succeed at proving time, making the epoch unprovable. The node - // simulates public calls against the messages predicted for the next block, so a consume tx sent as soon as the - // node's archiver has observed the message passes simulation and lands in the pool before the inserting block - // is built. Observation is the only trigger: a message in a mined L1 block is consumable at once, without waiting - // for a descendant L1 block. The send is timed to the archiver's observation and retried with fresh messages when a - // block slips in between. + // not, this tx would revert at proposal time and succeed at proving time, making the epoch unprovable. + // + // A block freezes its L1-to-L2 message set when it is prepared, which happens before the message's L1 tx is even + // mined, so a consume tx sent while a block is already building can only join that block -- one too early, against + // a frozen set that does not hold the message, and it reverts. Waiting for the archiver alone cannot avoid this: + // it leaves which block the tx joins up to where the send lands in the block cadence. Block production is + // therefore held while the message is sent and observed and the consume tx is put in the pool, so that the first + // block prepared after the resume is the one that both inserts the message and executes the tx. it('consumes a message in the same block that inserts it', async () => { const l1Account = t.ethAccount; - const maxAttempts = 5; - let sameBlockReceipt: { blockNumber: BlockNumber; msgHash: Fr; globalLeafIndex: bigint } | undefined; + const sequencer = t.context.aztecNodeService.getSequencer()!; + const [secret, secretHash] = await generateClaimSecret(); + const message = { recipient: testContract.address, content: Fr.random(), secretHash }; - for (let attempt = 0; attempt < maxAttempts && sameBlockReceipt === undefined; attempt++) { - const [secret, secretHash] = await generateClaimSecret(); - const message = { recipient: testContract.address, content: Fr.random(), secretHash }; - const { msgHash, globalLeafIndex, txReceipt: l1Receipt } = await sendMessageToL2(message); - log.warn(`Attempt ${attempt}: sent message ${msgHash.toString()} in L1 block ${l1Receipt.blockNumber}`); + // Proven before the pause so the proof window cannot expire while nothing is being built. + await markAsProven(); + await sequencer.pause(); - // Do not drive L2 blocks while waiting: an extra block here only shifts the timing of the inserting block. - // The archiver polls L1 within an Ethereum slot; no later L1 block is needed for the message to be usable. - await waitForMessageObserved(msgHash); + const { msgHash, globalLeafIndex } = await sendMessageToL2(message); + // The archiver polls L1 independently of block production, so it still observes the message while paused. + await waitForMessageObserved(msgHash); - const { receipt } = await testContract.methods - .consume_message_from_arbitrary_sender_public(message.content, secret, l1Account, globalLeafIndex.toBigInt()) - .send({ from: user1Address, wait: { dontThrowOnRevert: true } }); - const inserting = await findInsertingBlock(msgHash); - const consumeBlock = BlockNumber(Number(receipt.blockNumber)); - log.warn(`Consume tx for ${msgHash.toString()} landed in block ${consumeBlock}`, { - insertingBlock: inserting.blockNumber, - consumeBlock, - executionResult: receipt.executionResult, - }); + // NO_WAIT: the tx has to reach the pool while production is held, so it cannot be awaited here -- nothing will + // mine it until the resume below. Simulation runs against the messages predicted for the next block, which the + // archiver has already observed, so the consume simulates against a tree that holds the message. + const { txHash } = await testContract.methods + .consume_message_from_arbitrary_sender_public(message.content, secret, l1Account, globalLeafIndex.toBigInt()) + .send({ from: user1Address, wait: NO_WAIT }); + log.warn(`Queued consume for ${msgHash.toString()} while production is held`, { txHash: txHash.toString() }); - if (consumeBlock !== inserting.blockNumber) { - // A block was built between eligibility and the tx's arrival; that is timing, not a bug. - log.warn(`Consume did not land in the inserting block; retrying with a fresh message`); - continue; - } + // Resuming on a slot boundary gives the proposer its whole build frame for the block that carries both. + await t.monitor.waitUntilNextL2Slot(); + await sequencer.start(); - // Same block reached: the consume must have succeeded against the block's own messages. - expect(receipt.executionResult).toBe(TxExecutionResult.SUCCESS); - sameBlockReceipt = { blockNumber: consumeBlock, msgHash, globalLeafIndex: globalLeafIndex.toBigInt() }; - } + const receipt = await waitForTx(aztecNode, txHash); + const consumeBlock = BlockNumber(Number(receipt.blockNumber)); + const inserting = await findInsertingBlock(msgHash); + log.warn(`Consume tx for ${msgHash.toString()} landed in block ${consumeBlock}`, { + insertingBlock: inserting.blockNumber, + consumeBlock, + executionResult: receipt.executionResult, + }); - if (sameBlockReceipt === undefined) { - throw new Error(`Could not produce a same-block consume in ${maxAttempts} attempts`); - } + // The premise of the case: holding production must put both in the same block. Anything else is a real failure + // -- a consume one block late would succeed against an already-inserted message and prove nothing. + expect(consumeBlock).toBe(inserting.blockNumber); + expect(receipt.executionResult).toBe(TxExecutionResult.SUCCESS); + + const sameBlockReceipt = { blockNumber: consumeBlock, msgHash, globalLeafIndex: globalLeafIndex.toBigInt() }; const [resolvedIndex] = (await aztecNode.getL1ToL2MessageMembershipWitness( sameBlockReceipt.blockNumber, sameBlockReceipt.msgHash, From cd5d1f41d33b9a5c7370c2c896f45a891199c9ab Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Thu, 10 Sep 2026 11:25:53 -0300 Subject: [PATCH 25/25] test(archiver): drain the block-triggered sync inside the local-block helper addBlock resolves once the block is stored but triggers a sync it does not await, so every fixture that adds local blocks and then moves the L1 head backwards races the pass left in flight: recovery against the stale head can commit after the pass for the new head and leave the old height as the synced one. Two fixtures hit this, the second failing about twice in twelve runs. Drain once in the shared helper rather than at each call site, so the hazard is gone for the whole block. --- yarn-project/archiver/src/archiver-sync.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index 7fb9da967514..67b5d09aea1f 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -1583,7 +1583,14 @@ describe('Archiver Sync', () => { // Local blocks are placed far ahead on L1 so their slot never expires while the tests move the L1 head. const LOCAL_BLOCKS_L1_BLOCK = 5000n; - /** Locally proposed blocks chained on genesis, each consuming through the given message counts. */ + /** + * Locally proposed blocks chained on genesis, each consuming through the given message counts. + * + * `addBlock` resolves once the block is stored but triggers a sync it does not await, so the pass it starts + * outlives this helper with the head captured as it is now. Tests that then move the head backwards would race + * it: recovery against the stale head can commit after the pass for the new head and leave the old height as + * the synced one. Draining it here settles that pass before the caller changes anything. + */ const addLocalBlocksConsuming = async (leafCounts: number[]) => { const { checkpoint } = await mockCheckpointAndMessages(CheckpointNumber(1), { startBlockNumber: BlockNumber(1), @@ -1596,6 +1603,7 @@ describe('Archiver Sync', () => { for (const block of checkpoint.blocks) { await addLocalBlock(block); } + await archiver.syncImmediate(); return checkpoint.blocks; }; const localBlockNumbers = async () =>