diff --git a/yarn-project/archiver/src/archiver-sync.test.ts b/yarn-project/archiver/src/archiver-sync.test.ts index 667ca1dfc984..50291ec30865 100644 --- a/yarn-project/archiver/src/archiver-sync.test.ts +++ b/yarn-project/archiver/src/archiver-sync.test.ts @@ -24,6 +24,7 @@ import { openTmpStore } from '@aztec/kv-store/lmdb-v2'; import { GENESIS_BLOCK_HEADER_HASH, L2BlockSourceEvents, type L2BlockSourceUpdatedEvent } from '@aztec/stdlib/block'; import type { ProposedCheckpointInput } from '@aztec/stdlib/checkpoint'; import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; +import { InboxBucketRef } from '@aztec/stdlib/messaging'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; import { mockCheckpointAndMessages } from '@aztec/stdlib/testing'; import { ConsensusTimetable } from '@aztec/stdlib/timetable'; @@ -38,7 +39,7 @@ import { type MockProxy, mock } from 'jest-mock-extended'; import type { GetBlockReturnType } from 'viem'; import { Archiver, type ArchiverEmitter } from './archiver.js'; -import { BlockOrCheckpointSlotExpiredError } from './errors.js'; +import { BlockOrCheckpointSlotExpiredError, InboxPrefixMismatchError } from './errors.js'; import type { ArchiverInstrumentation } from './modules/instrumentation.js'; import { ArchiverL1Synchronizer } from './modules/l1_synchronizer.js'; import { type ArchiverDataStores, createArchiverDataStores } from './store/data_stores.js'; @@ -2324,6 +2325,37 @@ describe('Archiver Sync', () => { return blocks; }; + it('rejects the caller and emits no update event when a proposed block fails the prefix guard', async () => { + // The facade's promise is tied to the atomic updater call, so a prefix rejection has to surface to the + // producer and leave the pass looking like a no-op: no block stored, no tip moved, no aggregate event. + addMessages(); + fake.setL1BlockNumber(l1BlockNumber); + await archiver.syncImmediate(); + + const updateSpy = jest.fn(); + archiver.events.on(L2BlockSourceEvents.L2BlockSourceUpdated, updateSpy); + try { + const [block] = await makeBlocksConsumingThrough([2]); + + // The reference names a prefix this archiver's messages do not hash to at the block's end count. + await expect(archiver.addBlock(block, new InboxBucketRef(Fr.random()))).rejects.toThrow( + InboxPrefixMismatchError, + ); + + expect(await archiver.getBlockNumber()).toEqual(BlockNumber(0)); + expect(await archiver.getBlock({ number: BlockNumber(1) })).toBeUndefined(); + expect(updateSpy).not.toHaveBeenCalled(); + expect(pruneSpy).not.toHaveBeenCalled(); + + // The same block with the right reference is accepted, so the rejection was the guard and not the fixture. + const inboxRollingHash = (await archiverStore.messages.getInboxRollingHashAt(2n))!; + await archiver.addBlock(block, new InboxBucketRef(inboxRollingHash)); + expect(await archiver.getBlockNumber()).toEqual(BlockNumber(1)); + } finally { + archiver.events.off(L2BlockSourceEvents.L2BlockSourceUpdated, updateSpy); + } + }); + it('prunes from the first proposed block that consumed a removed message', async () => { const { early } = addMessages(); fake.setL1BlockNumber(l1BlockNumber); @@ -2417,7 +2449,7 @@ describe('Archiver Sync', () => { }); }); - it('prunes when the reorg merges away the bucket boundary a block ended on', async () => { + it('keeps the proposed chain when the reorg merges away the bucket boundary a block ended on', async () => { const { early, late } = addMessages(); fake.setL1BlockNumber(l1BlockNumber); await archiver.syncImmediate(); @@ -2427,25 +2459,27 @@ describe('Archiver Sync', () => { await archiver.addBlock(block); } expect(await archiver.getBlockNumber()).toEqual(BlockNumber(2)); - // The rolling hash the checkpoint carrying these blocks committed to. - const consumedRollingHash = (await archiverStore.messages.getInboxBucket(2n))!.inboxRollingHash; + // The prefix hash block 1 signed, which is also the boundary hash of the bucket it ended on. + const block1PrefixHash = (await archiverStore.messages.getInboxRollingHashAt(2n))!; // The messages of block 100 are re-mined into block 102, so all four end up in a single bucket. Every leaf - // survives under its own index, but the boundary at leaf count 2 does not, and block 1 ended on it: nothing - // can derive the messages that block inserted any more, so the chain built on it goes. + // survives under its own index and so does every prefix hash; only the boundary at leaf count 2 is gone. A + // block is authenticated by the prefix it consumed, not by the boundary it stopped on, so nothing is pruned. fake.retimeMessages(100n, 102n); await archiver.syncImmediate(); expect(await getStoredLeaves()).toEqual(asHex([...early, ...late])); - expect(await archiver.getBlockNumber()).toEqual(BlockNumber(0)); - expect(pruneSpy).toHaveBeenCalledWith( - expect.objectContaining({ type: L2BlockSourceEvents.L2PruneUncheckpointed, blocks }), - ); + expect(await archiver.getBlockNumber()).toEqual(BlockNumber(2)); + expect(pruneSpy).not.toHaveBeenCalled(); + // The bucket metadata is rewritten even though not a single message moved. expect(await archiverStore.messages.getInboxBucket(1n)).toMatchObject({ msgCount: 4, l1BlockNumber: 102n }); expect(await archiverStore.messages.getInboxBucket(2n)).toBeUndefined(); - // The merged bucket still carries the hash the sealed header committed to, so a checkpoint whose blocks the - // merge left alone stays publishable; here they did not survive it. - expect(await archiverStore.messages.getInboxBucketByRollingHash(consumedRollingHash)).toMatchObject({ seq: 1n }); + // Block 1's signed prefix still resolves by count, so its bundle is still derivable — even though no bucket + // ends there any more. + expect(await archiverStore.messages.getInboxRollingHashAt(2n)).toEqual(block1PrefixHash); + expect(await archiverStore.messages.getInboxBucketByRollingHash(block1PrefixHash)).toBeUndefined(); + expect(await archiverStore.messages.getL1ToL2MessagesBetweenLeafCounts(0n, 2n)).toEqual(early); + expect(await archiverStore.messages.getL1ToL2MessagesBetweenLeafCounts(2n, 4n)).toEqual(late); }); it('keeps the proposed chain when the reorg splits the bucket a block consumed', async () => { @@ -2476,7 +2510,7 @@ describe('Archiver Sync', () => { expect(pruneSpy).not.toHaveBeenCalled(); }); - it('prunes when the reorg extends the last canonical bucket past the boundary a block ended on', async () => { + it('keeps the proposed chain when the reorg extends the last canonical bucket past a block boundary', async () => { const { early, late } = addMessages(); // L1 block 101 carries block 100's timestamp, so a message mined there joins the bucket block 100 opened. fake.shareTimestampWithL1Block(101n, 100n); @@ -2490,18 +2524,18 @@ describe('Archiver Sync', () => { expect(await archiver.getBlockNumber()).toEqual(BlockNumber(2)); // The two messages of L1 block 102 are re-mined into 101, which leaves block 100 untouched: the bucket the - // rollback walks back to is canonical on both of its own L1 blocks, and still absorbs the re-mined messages, - // so the boundary block 1 ended on is gone even though nothing below it moved. + // rollback walks back to is canonical on both of its own L1 blocks and now absorbs the re-mined messages too, + // so the boundary block 1 ended on is gone even though nothing below it moved. Content-wise the chain is + // unchanged, so both blocks stay. fake.retimeMessages(102n, 101n); await archiver.syncImmediate(); expect(await getStoredLeaves()).toEqual(asHex([...early, ...late])); expect(await archiverStore.messages.getInboxBucket(1n)).toMatchObject({ msgCount: 4, totalMsgCount: 4n }); expect(await archiverStore.messages.getInboxBucket(2n)).toBeUndefined(); - expect(await archiver.getBlockNumber()).toEqual(BlockNumber(0)); - expect(pruneSpy).toHaveBeenCalledWith( - expect.objectContaining({ type: L2BlockSourceEvents.L2PruneUncheckpointed, blocks }), - ); + expect(await archiver.getBlockNumber()).toEqual(BlockNumber(2)); + expect(pruneSpy).not.toHaveBeenCalled(); + expect(await archiverStore.messages.getL1ToL2MessagesBetweenLeafCounts(0n, 2n)).toEqual(early); }); it('prunes from the first leaf the reorg changed, not from the start of its bucket', async () => { diff --git a/yarn-project/archiver/src/archiver.ts b/yarn-project/archiver/src/archiver.ts index 7ecdd302ca06..b49bd6868fa3 100644 --- a/yarn-project/archiver/src/archiver.ts +++ b/yarn-project/archiver/src/archiver.ts @@ -31,7 +31,7 @@ import { getTimestampForSlot, getTimestampRangeForEpoch, } from '@aztec/stdlib/epoch-helpers'; -import type { L2ToL1MembershipWitness } from '@aztec/stdlib/messaging'; +import type { InboxBucketRef, L2ToL1MembershipWitness } from '@aztec/stdlib/messaging'; import { ConsensusTimetable } from '@aztec/stdlib/timetable'; import type { BlockHeader, TxHash } from '@aztec/stdlib/tx'; import { type TelemetryClient, type Traceable, type Tracer, trackSpan } from '@aztec/telemetry-client'; @@ -55,6 +55,8 @@ export type { ArchiverEmitter }; type AddBlockRequest = { type: 'block'; block: L2Block; + /** The block proposal's signed Inbox prefix reference; absent only on the trusted compatibility path. */ + inboxPrefixRef: InboxBucketRef | undefined; resolve: () => void; reject: (err: Error) => void; }; @@ -313,11 +315,14 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra * The block will be processed by the sync loop. * Implements the L2BlockSink interface. * @param block - The L2 block to add. + * @param inboxPrefixRef - The block proposal's signed Inbox prefix reference, validated against this archiver's + * own messages in the same transaction as the insert. See {@link L2BlockSink.addBlock} for when it may be + * omitted. * @returns A promise that resolves when the block has been added to the store, or rejects on error. */ - public addBlock(block: L2Block): Promise { + public addBlock(block: L2Block, inboxPrefixRef?: InboxBucketRef): Promise { const promise = promiseWithResolvers(); - this.inboundQueue.push({ block, ...promise, type: 'block' }); + this.inboundQueue.push({ block, inboxPrefixRef, ...promise, type: 'block' }); this.log.debug(`Queued block ${block.number} for processing`); void this.trySyncImmediate(); return promise.promise; @@ -379,7 +384,7 @@ export class Archiver extends ArchiverDataSourceBase implements L2BlockSink, Tra try { if (type === 'block') { - const [durationMs] = await elapsed(() => this.updater.addProposedBlock(item.block)); + const [durationMs] = await elapsed(() => this.updater.addProposedBlock(item.block, item.inboxPrefixRef)); this.instrumentation.processNewProposedBlock(durationMs, item.block); blocksAdded.push(item.block); } else { diff --git a/yarn-project/archiver/src/errors.ts b/yarn-project/archiver/src/errors.ts index 5acfa036b551..d028afdcab49 100644 --- a/yarn-project/archiver/src/errors.ts +++ b/yarn-project/archiver/src/errors.ts @@ -131,6 +131,71 @@ export class InboxMessageRangeNotSyncedError extends Error { } } +/** + * Thrown when a proposed block's signed Inbox prefix reference cannot be checked against the local view, because the + * archiver has not synced a message at the block's end count or cannot serve its consumed range whole. Distinguishes + * "our view is behind, retry" from {@link InboxPrefixMismatchError}'s "our view disagrees". + */ +export class InboxPrefixNotSyncedError extends Error { + constructor( + public readonly blockNumber: number, + public readonly endTotalMsgCount: bigint, + cause?: string, + ) { + super( + `Cannot confirm the Inbox prefix at message count ${endTotalMsgCount} for proposed block ${blockNumber}` + + (cause ? `: ${cause}` : ''), + ); + this.name = 'InboxPrefixNotSyncedError'; + } +} + +/** + * Thrown when a proposed block's signed Inbox prefix reference does not match the canonical prefix this archiver + * holds at the block's end count. The block consumed messages this node's view of L1 does not back, so inserting it + * would put a chain nothing can replay into the store. + */ +export class InboxPrefixMismatchError extends Error { + constructor( + public readonly blockNumber: number, + public readonly endTotalMsgCount: bigint, + public readonly expected: Fr, + public readonly actual: Fr, + ) { + super( + `Proposed block ${blockNumber} references Inbox prefix ${expected.toString()} at message count ` + + `${endTotalMsgCount}, but the canonical prefix there is ${actual.toString()}`, + ); + this.name = 'InboxPrefixMismatchError'; + } +} + +/** Thrown when a proposed block's parent is not in the store, so its consumed range has no lower bound. */ +export class ProposedBlockParentNotFoundError extends Error { + constructor( + public readonly blockNumber: number, + public readonly parentBlockNumber: number, + ) { + super(`Cannot resolve parent block ${parentBlockNumber} of proposed block ${blockNumber}`); + this.name = 'ProposedBlockParentNotFoundError'; + } +} + +/** Thrown when a proposed block's end message count is below its parent's, so consumption would rewind. */ +export class InboxConsumptionRewindsError extends Error { + constructor( + public readonly blockNumber: number, + public readonly endTotalMsgCount: bigint, + public readonly parentTotalMsgCount: bigint, + ) { + super( + `Proposed block ${blockNumber} consumes through message count ${endTotalMsgCount}, ` + + `behind its parent's ${parentTotalMsgCount}`, + ); + this.name = 'InboxConsumptionRewindsError'; + } +} + /** Thrown when a proposed checkpoint number is stale (already processed). */ export class ProposedCheckpointStaleError extends Error { constructor( diff --git a/yarn-project/archiver/src/modules/data_source_base.ts b/yarn-project/archiver/src/modules/data_source_base.ts index 93409b88b465..17419914b96b 100644 --- a/yarn-project/archiver/src/modules/data_source_base.ts +++ b/yarn-project/archiver/src/modules/data_source_base.ts @@ -336,6 +336,10 @@ export abstract class ArchiverDataSourceBase return this.stores.messages.getInboxBucketByRollingHash(inboxRollingHash); } + public getInboxRollingHashAt(totalMsgCount: bigint): Promise { + return this.stores.messages.getInboxRollingHashAt(totalMsgCount); + } + public getL1ToL2MessagesBetweenBuckets(fromExclusive: bigint, toInclusive: bigint): Promise { return this.stores.messages.getL1ToL2MessagesBetweenBuckets(fromExclusive, toInclusive); } diff --git a/yarn-project/archiver/src/modules/data_store_updater.test.ts b/yarn-project/archiver/src/modules/data_store_updater.test.ts index 8a385ff54876..4ab8a2daab0d 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.test.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.test.ts @@ -9,10 +9,12 @@ import { BundledProtocolContractsProvider } from '@aztec/protocol-contracts/prov import { getPublishableStandardContracts } from '@aztec/standard-contracts'; import { bufferAsFields } from '@aztec/stdlib/abi'; import { AztecAddress } from '@aztec/stdlib/aztec-address'; -import { GENESIS_BLOCK_HEADER_HASH, L2Block } from '@aztec/stdlib/block'; +import { GENESIS_BLOCK_HEADER_HASH, L2Block, type ValidateCheckpointResult } from '@aztec/stdlib/block'; import { ContractClassLog, ContractClassLogFields, PrivateLog } from '@aztec/stdlib/logs'; +import { InboxBucketRef, updateInboxRollingHash } from '@aztec/stdlib/messaging'; import { CheckpointHeader } from '@aztec/stdlib/rollup'; import '@aztec/stdlib/testing/jest'; +import { AppendOnlyTreeSnapshot } from '@aztec/stdlib/trees'; import { BlockHeader } from '@aztec/stdlib/tx'; import { jest } from '@jest/globals'; @@ -20,10 +22,17 @@ import { readFileSync } from 'fs'; import { dirname, resolve } from 'path'; import { fileURLToPath } from 'url'; +import { InboxConsumptionRewindsError, InboxPrefixMismatchError, InboxPrefixNotSyncedError } from '../errors.js'; import { registerProtocolContracts, registerStandardContracts } from '../factory.js'; import { type ArchiverDataStores, createArchiverDataStores } from '../store/data_stores.js'; import { L2TipsCache } from '../store/l2_tips_cache.js'; -import { makeCheckpoint, makePublishedCheckpoint } from '../test/mock_structs.js'; +import { + makeCheckpoint, + makeInboxMessages, + makeL1BlockHash, + makeL1BlockNumberForBucket, + makePublishedCheckpoint, +} from '../test/mock_structs.js'; import { ArchiverDataStoreUpdater } from './data_store_updater.js'; /** @@ -619,4 +628,273 @@ describe('ArchiverDataStoreUpdater', () => { ); }); }); + + describe('proposed-block Inbox prefix guard', () => { + /** Three messages in two buckets: bucket 1 = [m0, m1] (boundary at count 2), bucket 2 = [m2] (count 3). */ + const twoBucketSpec = [ + { seq: 1n, timestamp: 100n }, + { seq: 1n, timestamp: 100n }, + { seq: 2n, timestamp: 200n }, + ]; + + /** Builds the messages `twoBucketSpec` describes, chained and bucketed as the archiver would store them. */ + const makeMessages = (spec: { seq: bigint; timestamp: bigint }[] = twoBucketSpec) => + makeInboxMessages(spec.length, { + overrideFn: (msg, i) => ({ + ...msg, + bucketSeq: spec[i].seq, + bucketTimestamp: spec[i].timestamp, + l1BlockNumber: makeL1BlockNumberForBucket(spec[i].timestamp), + l1BlockHash: makeL1BlockHash(makeL1BlockNumberForBucket(spec[i].timestamp)), + }), + }); + + /** A proposed block at `number` whose header claims to have consumed through `leafCount` messages. */ + const makeBlockConsumingThrough = async (number: number, leafCount: number) => { + const block = await L2Block.random(BlockNumber(number), { + checkpointNumber: CheckpointNumber(1), + indexWithinCheckpoint: IndexWithinCheckpoint(number - 1), + slotNumber: SlotNumber(100), + ...(previousArchive ? { lastArchive: previousArchive } : {}), + }); + block.header.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot(Fr.random(), leafCount); + previousArchive = block.archive; + return block; + }; + let previousArchive: AppendOnlyTreeSnapshot | undefined; + + /** + * Everything a rejected insertion must leave untouched: the block chain and its tips, the logs, the contract + * class/instance data extracted from a block's logs, and the pending-chain validation status. Read fresh each + * time (the tips cache is constructed per call) so a stale cache cannot mask a write. + */ + const snapshotStoreState = async (blockNumber = 1) => ({ + latestBlockNumber: await store.blocks.getLatestL2BlockNumber(), + checkpointedBlockNumber: await store.blocks.getCheckpointedL2BlockNumber(), + block: await store.blocks.getBlock({ number: BlockNumber(blockNumber) }), + privateLogs: await store.logs.getPrivateLogsForBlock(blockNumber), + publicLogs: await store.logs.getPublicLogsForBlock(blockNumber), + contractClass: await store.contractClasses.getContractClass(contractClassId), + contractInstance: await store.contractInstances.getContractInstance(instanceAddress, 1n), + validationStatus: await store.blocks.getPendingChainValidationStatus(), + tips: await new L2TipsCache(store.blocks, GENESIS_BLOCK_HEADER_HASH).getL2Tips(), + }); + + beforeEach(() => { + previousArchive = undefined; + }); + + it('accepts a block whose reference matches the canonical prefix at its header count', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const block = await makeBlockConsumingThrough(1, 2); + + await updater.addProposedBlock(block, new InboxBucketRef(msgs[1].inboxRollingHash)); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(1); + }); + + it('accepts a block whose header count became interior to a current bucket', async () => { + // The messages are re-mined as a single bucket, so nothing ends at count 2 any more. The prefix hash there is + // unchanged, which is what the block signed, so it is still exactly as valid. + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + await store.messages.removeL1ToL2Messages(0n); + await store.messages.addL1ToL2MessageBuckets(msgs.map(msg => ({ ...msg, bucketSeq: 1n, bucketTimestamp: 100n }))); + expect(await store.messages.getInboxBucketByTotalMsgCount(2n)).toBeUndefined(); + + const block = await makeBlockConsumingThrough(1, 2); + await updater.addProposedBlock(block, new InboxBucketRef(msgs[1].inboxRollingHash)); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(1); + }); + + it('accepts an empty block that re-signs its parent prefix, and a genesis one at the zero hash', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + + const genesisEmpty = await makeBlockConsumingThrough(1, 0); + await updater.addProposedBlock(genesisEmpty, new InboxBucketRef(Fr.ZERO)); + + const consuming = await makeBlockConsumingThrough(2, 2); + await updater.addProposedBlock(consuming, new InboxBucketRef(msgs[1].inboxRollingHash)); + + const empty = await makeBlockConsumingThrough(3, 2); + await updater.addProposedBlock(empty, new InboxBucketRef(msgs[1].inboxRollingHash)); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(3); + }); + + it('rejects an empty block that re-signs the wrong prefix', async () => { + // An empty range is not permission to skip the hash: the unchanged count must still name the signed prefix. + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const block = await makeBlockConsumingThrough(1, 0); + + await expect(updater.addProposedBlock(block, new InboxBucketRef(msgs[0].inboxRollingHash))).rejects.toThrow( + InboxPrefixMismatchError, + ); + }); + + it('rejects a reference that does not match the canonical prefix, writing nothing', async () => { + // The block carries every kind of write the insert would make — logs, a contract class, a contract instance — + // and the call also supplies a pending validation status, so a partial commit anywhere would show up. + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const block = await makeBlockConsumingThrough(1, 2); + block.body.txEffects[0].contractClassLogs = [contractClassLog]; + block.body.txEffects[0].privateLogs = [PrivateLog.fromBuffer(getSampleContractInstancePublishedEventPayload())]; + const before = await snapshotStoreState(); + + await expect( + updater.addProposedBlock(block, new InboxBucketRef(Fr.random()), { + valid: true, + } satisfies ValidateCheckpointResult), + ).rejects.toThrow(InboxPrefixMismatchError); + + // Nothing moved: blocks, tips, logs, contract data and the validation status are all as they were. + expect(await snapshotStoreState()).toEqual(before); + expect(await store.blocks.getBlock({ number: BlockNumber(1) })).toBeUndefined(); + expect(await store.logs.getPrivateLogsForBlock(1)).toEqual([]); + expect(await store.logs.getPublicLogsForBlock(1)).toEqual([]); + expect(await store.contractClasses.getContractClass(contractClassId)).toBeUndefined(); + expect(await store.contractInstances.getContractInstance(instanceAddress, 1n)).toBeUndefined(); + }); + + it('rejects a header count past the synced tip as unavailable, not as a mismatch', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const block = await makeBlockConsumingThrough(1, 9); + + await expect(updater.addProposedBlock(block, new InboxBucketRef(msgs[2].inboxRollingHash))).rejects.toThrow( + InboxPrefixNotSyncedError, + ); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(0); + }); + + it('rejects a block whose consumption rewinds below its parent', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const parent = await makeBlockConsumingThrough(1, 3); + await updater.addProposedBlock(parent, new InboxBucketRef(msgs[2].inboxRollingHash)); + + const child = await makeBlockConsumingThrough(2, 2); + await expect(updater.addProposedBlock(child, new InboxBucketRef(msgs[1].inboxRollingHash))).rejects.toThrow( + InboxConsumptionRewindsError, + ); + }); + + it('skips the guard when no reference is supplied', async () => { + // The compatibility path: fixtures build arbitrary leaf counts with no matching Inbox state. Nothing is + // validated, and the block is inserted as before. + const block = await makeBlockConsumingThrough(1, 500); + await expect(updater.addProposedBlock(block)).resolves.not.toThrow(); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(1); + }); + + describe('transaction ordering against a message replacement', () => { + // Both orderings are real serializations, not simulated ones: every updater entry point runs as one whole + // transaction on the shared KV store, whose writer queue admits one at a time, and the archiver's sync pass + // drains the inbound proposal queue before it touches L1 messages. So two sequential calls are exactly the two + // ways these can interleave, with no barriers or test hooks needed. + + /** A canonical re-delivery of the same three messages as one bucket: the boundary at count 2 disappears. */ + const repartition = (msgs: ReturnType) => + msgs.map(msg => ({ ...msg, bucketSeq: 1n, bucketTimestamp: 100n })); + + /** A canonical re-delivery that replaces the last message, so the prefix at count 3 changes. */ + const replaceTail = (msgs: ReturnType) => { + const leaf = Fr.random(); + return [ + ...msgs.slice(0, 2), + { + ...msgs[2], + leaf, + inboxRollingHash: updateInboxRollingHash(msgs[1].inboxRollingHash, leaf), + bucketSeq: 2n, + bucketTimestamp: 250n, + }, + ]; + }; + + it('replacement first, pure repartition: the late block is still accepted', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + + await updater.replaceMessagesAndPruneProposedBlocks({ + lastCanonicalBucketSeq: undefined, + firstInvalidatedIndex: undefined, + messages: repartition(msgs), + syncPoint: { l1BlockNumber: 200n, l1BlockHash: makeL1BlockHash(200n) }, + finalizedL1Block: undefined, + }); + + const block = await makeBlockConsumingThrough(1, 2); + await updater.addProposedBlock(block, new InboxBucketRef(msgs[1].inboxRollingHash)); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(1); + }); + + it('replacement first, changed content: the late block is rejected before any write', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const replaced = replaceTail(msgs); + + await updater.replaceMessagesAndPruneProposedBlocks({ + lastCanonicalBucketSeq: 1n, + firstInvalidatedIndex: 2n, + messages: replaced.slice(2), + syncPoint: { l1BlockNumber: 200n, l1BlockHash: makeL1BlockHash(200n) }, + finalizedL1Block: undefined, + }); + const before = await snapshotStoreState(); + + // The block was built against the pre-reorg prefix at count 3, which no longer exists. + const block = await makeBlockConsumingThrough(1, 3); + await expect(updater.addProposedBlock(block, new InboxBucketRef(msgs[2].inboxRollingHash))).rejects.toThrow( + InboxPrefixMismatchError, + ); + + expect(await snapshotStoreState()).toEqual(before); + expect(await store.blocks.getBlock({ number: BlockNumber(1) })).toBeUndefined(); + }); + + it('insertion first, pure repartition: the inserted block is preserved', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const block = await makeBlockConsumingThrough(1, 2); + await updater.addProposedBlock(block, new InboxBucketRef(msgs[1].inboxRollingHash)); + + const pruned = await updater.replaceMessagesAndPruneProposedBlocks({ + lastCanonicalBucketSeq: undefined, + firstInvalidatedIndex: undefined, + messages: repartition(msgs), + syncPoint: { l1BlockNumber: 200n, l1BlockHash: makeL1BlockHash(200n) }, + finalizedL1Block: undefined, + }); + + expect(pruned).toEqual([]); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(1); + // And the block's bundle is still derivable by count, though no bucket ends at its boundary any more. + expect(await store.messages.getL1ToL2MessagesBetweenLeafCounts(0n, 2n)).toEqual([msgs[0].leaf, msgs[1].leaf]); + }); + + it('insertion first, changed content: the inserted block and its descendants are pruned', async () => { + const msgs = makeMessages(); + await store.messages.addL1ToL2MessageBuckets(msgs); + const consuming = await makeBlockConsumingThrough(1, 3); + await updater.addProposedBlock(consuming, new InboxBucketRef(msgs[2].inboxRollingHash)); + const descendant = await makeBlockConsumingThrough(2, 3); + await updater.addProposedBlock(descendant, new InboxBucketRef(msgs[2].inboxRollingHash)); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(2); + + const pruned = await updater.replaceMessagesAndPruneProposedBlocks({ + lastCanonicalBucketSeq: 1n, + firstInvalidatedIndex: 2n, + messages: replaceTail(msgs).slice(2), + syncPoint: { l1BlockNumber: 200n, l1BlockHash: makeL1BlockHash(200n) }, + finalizedL1Block: undefined, + }); + + expect(pruned.map(b => b.number)).toEqual([1, 2]); + expect(await store.blocks.getLatestL2BlockNumber()).toBe(0); + }); + }); + }); }); diff --git a/yarn-project/archiver/src/modules/data_store_updater.ts b/yarn-project/archiver/src/modules/data_store_updater.ts index baefce3f36d6..f20c9c5dd837 100644 --- a/yarn-project/archiver/src/modules/data_store_updater.ts +++ b/yarn-project/archiver/src/modules/data_store_updater.ts @@ -1,3 +1,4 @@ +import { INITIAL_L2_BLOCK_NUM } from '@aztec/constants'; import type { L1BlockId } from '@aztec/ethereum/l1-types'; import { BlockNumber, CheckpointNumber } from '@aztec/foundation/branded-types'; import { filterAsync } from '@aztec/foundation/collection'; @@ -7,7 +8,7 @@ import { ContractInstancePublishedEvent, ContractInstanceUpdatedEvent, } from '@aztec/protocol-contracts/instance-registry'; -import type { CommitteeAttestation, L2Block, ValidateCheckpointResult } from '@aztec/stdlib/block'; +import type { BlockData, CommitteeAttestation, L2Block, ValidateCheckpointResult } from '@aztec/stdlib/block'; import { type L1PublishedData, type ProposedCheckpointInput, @@ -20,8 +21,15 @@ import { computeContractClassId, } from '@aztec/stdlib/contract'; import type { ContractClassLog, PrivateLog, PublicLog } from '@aztec/stdlib/logs'; +import type { InboxBucketRef } from '@aztec/stdlib/messaging'; import type { UInt64 } from '@aztec/stdlib/types'; +import { + InboxConsumptionRewindsError, + InboxPrefixMismatchError, + InboxPrefixNotSyncedError, + ProposedBlockParentNotFoundError, +} from '../errors.js'; import type { ArchiverDataStores } from '../store/data_stores.js'; import type { L2TipsCache } from '../store/l2_tips_cache.js'; import type { InboxMessageReplacement } from '../store/message_store.js'; @@ -32,6 +40,14 @@ enum Operation { Delete, } +/** + * A block's L1-to-L2 message tree leaf count: the cumulative Inbox message count consumed through it. Messages are + * indexed compactly with no padding, so the leaf count is exactly the prefix length the block consumed. + */ +function blockLeafCount(block: Pick | BlockData): bigint { + return BigInt(block.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); +} + /** Result of adding checkpoints with information about any pruned blocks. */ type ReconcileCheckpointsResult = { /** Blocks that were pruned due to conflict with L1 checkpoints. */ @@ -55,19 +71,34 @@ export class ArchiverDataStoreUpdater { * This is an uncheckpointed block that has been proposed by the sequencer but not yet included in a checkpoint on L1. * Extracts ContractClassPublished, ContractInstancePublished, ContractInstanceUpdated events from the block logs. * + * When `inboxPrefixRef` is given, the block's signed Inbox prefix reference is validated against the canonical + * messages this store holds *inside the same transaction as the writes*. That closes the late-insert race: a block + * built before an L1 reorg rolled back the messages it consumed used to be insertable after + * {@link removeProposedBlocksConsumingMessagesFrom} had already pruned the chain it belongs to, landing on the + * parent that survived the prune with nothing left to remove it. Because both the message replacement and this + * insertion run as whole transactions on the same store, the two orderings converge: a replacement that commits + * first makes this validation fail, and an insertion that commits first is visible to the replacement's prune. + * * @param block - The proposed L2 block to add. + * @param inboxPrefixRef - The block proposal's signed Inbox prefix reference. Omitted only on the trusted + * compatibility path (see {@link L2BlockSink.addBlock}), which skips the guard and keeps prior behavior. * @param pendingChainValidationStatus - Optional validation status to set. * @returns True if the operation is successful. + * @throws InboxPrefixNotSyncedError, InboxPrefixMismatchError, ProposedBlockParentNotFoundError or + * InboxConsumptionRewindsError when a supplied reference does not check out; nothing is written in that case. */ - // TODO: a block built before an L1 reorg rolled back the messages it consumed can be pushed here after - // `removeProposedBlocksConsumingMessagesFrom` already pruned the chain it belongs to, and lands on the parent that - // survived the prune with nothing left to remove it. Closing it needs the builder to carry the message-store state - // it built against so this insert can reject a stale one. public async addProposedBlock( block: L2Block, + inboxPrefixRef?: InboxBucketRef, pendingChainValidationStatus?: ValidateCheckpointResult, ): Promise { const result = await this.stores.db.transactionAsync(async () => { + // Validate the signed prefix before anything block-associated is written, so a rejection leaves the store + // exactly as it was: the transaction is only committed if the whole callback resolves. + if (inboxPrefixRef !== undefined) { + await this.validateProposedBlockInboxPrefix(block, inboxPrefixRef); + } + await this.stores.blocks.addProposedBlock(block); const opResults = await Promise.all([ @@ -86,6 +117,62 @@ export class ArchiverDataStoreUpdater { return result; } + /** + * Checks a proposed block's signed Inbox prefix reference against the canonical messages this store holds, and + * that the exact range it consumed is available whole. + * + * The block's end count comes from its own signed header's L1-to-L2 leaf count and the start from its stored + * parent's. Because each message's rolling hash chains the one before it, a matching hash at the end count proves + * the proposer consumed exactly the prefix this store holds, which makes the range between the two counts + * canonical by content. The range is then read to prove it is available whole, so a block whose messages this + * store cannot serve never enters the chain — reading it is about availability, not authentication. + * + * Deliberately absent is any requirement that either count sit on a boundary of the current bucket partition: an + * L1 reorg that re-mines the same messages under different boundaries leaves every leaf and prefix hash intact, so + * a block that ended on a boundary the reorg merged away is still exactly as valid as when it was signed. + * + * Must run inside the transaction that writes the block. + */ + private async validateProposedBlockInboxPrefix(block: L2Block, inboxPrefixRef: InboxBucketRef): Promise { + const blockNumber = block.number; + const endCount = blockLeafCount(block); + const parentCount = await this.getParentBlockLeafCount(block); + if (endCount < parentCount) { + throw new InboxConsumptionRewindsError(blockNumber, endCount, parentCount); + } + + const canonicalHash = await this.stores.messages.getInboxRollingHashAt(endCount); + if (canonicalHash === undefined) { + throw new InboxPrefixNotSyncedError(blockNumber, endCount, 'no message is stored at that count'); + } + if (!canonicalHash.equals(inboxPrefixRef.inboxRollingHash)) { + throw new InboxPrefixMismatchError(blockNumber, endCount, inboxPrefixRef.inboxRollingHash, canonicalHash); + } + + try { + await this.stores.messages.getL1ToL2MessagesBetweenLeafCounts(parentCount, endCount); + } catch (err) { + throw new InboxPrefixNotSyncedError(blockNumber, endCount, `${err}`); + } + } + + /** + * The cumulative Inbox message count consumed through a proposed block's parent: its L1-to-L2 tree leaf count, or + * zero when the block is the first of the chain. Throws when the parent is not in the store, since its consumed + * range would then have no lower bound. Must run inside the caller's transaction. + */ + private async getParentBlockLeafCount(block: L2Block): Promise { + const parentBlockNumber = block.number - 1; + if (parentBlockNumber < INITIAL_L2_BLOCK_NUM) { + return 0n; + } + const parent = await this.stores.blocks.getBlockData({ number: BlockNumber(parentBlockNumber) }); + if (parent === undefined) { + throw new ProposedBlockParentNotFoundError(block.number, parentBlockNumber); + } + return blockLeafCount(parent); + } + /** * Reconciles local blocks with incoming checkpoints from L1. * Adds new checkpoints to the store with contract class/instance extraction from logs. @@ -304,8 +391,11 @@ export class ArchiverDataStoreUpdater { /** * Replaces the messages above the last canonical Inbox bucket with the ones the canonical chain delivers, and - * prunes the proposed blocks that consumed a leaf the swap changed or ended on a bucket boundary it took away, in - * a single transaction. + * prunes the proposed blocks that consumed a leaf the swap changed, in a single transaction. + * + * Only changed leaves prune. A swap that re-delivers the very same messages under different bucket boundaries + * rewrites the partition and the metadata derived from the L1 blocks while dropping nothing, because a block is + * authenticated by the message prefix it consumed rather than by the boundary it stopped on. * * Splitting the two would make the prune unrecoverable, for the same reason * {@link rewindMessagesAndPruneProposedBlocks} keeps them together: the next sync pass would find the local @@ -344,9 +434,11 @@ export class ArchiverDataStoreUpdater { * count of messages consumed through it, so a leaf count above the index means the block consumed a message the * local chain no longer backs, and one equal to it means the block stopped below it. * - * The index is the first leaf whose value actually changed, or whose bucket boundary the reorg took away, so a - * reorg that re-mines the same messages in a different L1 block prunes nothing, and one that replaces the tail of - * a bucket keeps the blocks that stopped below the replaced leaf. + * The index is the first leaf whose value actually changed or that the canonical chain no longer carries — bucket + * boundaries play no part. A reorg that re-mines the same messages prunes nothing, however differently it + * partitions them into buckets, because a block is authenticated by the message prefix it consumed and not by the + * boundary it happened to stop on; one that replaces the tail of a bucket keeps the blocks that stopped below the + * replaced leaf. * * Checkpointed blocks are never touched: a message store that disagrees with a checkpoint L1 accepted means one * of the two views of L1 is mid-reorg and this one is not necessarily the right one, so only the archive @@ -369,9 +461,7 @@ export class ArchiverDataStoreUpdater { checkpointedBlockNumber > 0 ? await this.stores.blocks.getBlockData({ number: BlockNumber(checkpointedBlockNumber) }) : undefined; - const checkpointedTipLeafCount = BigInt( - checkpointedTip?.header.state.l1ToL2MessageTree.nextAvailableLeafIndex ?? 0, - ); + const checkpointedTipLeafCount = checkpointedTip === undefined ? 0n : blockLeafCount(checkpointedTip); if (firstInvalidatedIndex < checkpointedTipLeafCount) { this.log.warn( `Not pruning proposed blocks: rollback index ${firstInvalidatedIndex} is below the checkpointed tip's leaf count`, @@ -384,9 +474,7 @@ export class ArchiverDataStoreUpdater { from: BlockNumber(checkpointedBlockNumber + 1), limit: latestBlockNumber - checkpointedBlockNumber, }); - const firstAffected = proposedBlocks.find( - block => BigInt(block.header.state.l1ToL2MessageTree.nextAvailableLeafIndex) > firstInvalidatedIndex, - ); + const firstAffected = proposedBlocks.find(block => blockLeafCount(block) > firstInvalidatedIndex); if (firstAffected === undefined) { return []; } diff --git a/yarn-project/archiver/src/modules/l1_synchronizer.ts b/yarn-project/archiver/src/modules/l1_synchronizer.ts index fd3301c43f75..cb4d5778b86d 100644 --- a/yarn-project/archiver/src/modules/l1_synchronizer.ts +++ b/yarn-project/archiver/src/modules/l1_synchronizer.ts @@ -849,24 +849,19 @@ export class ArchiverL1Synchronizer implements Traceable { } /** - * Returns the lowest index at or above `fromIndex` that the canonical chain no longer backs, or undefined when - * every stored message from there on survives the swap whole. An index is invalidated when its leaf changed, and - * also when the bucket boundary that closed on it is gone: a block ends on a bucket boundary and consumers derive - * the messages it inserted from the buckets that boundary belongs to, so a boundary that stops existing takes the - * blocks that ended on it with it. + * Returns the lowest index at or above `fromIndex` whose leaf the canonical chain no longer backs, or undefined + * when the canonical chain re-delivers every stored message from there on unchanged. An index is invalidated only + * when its leaf changed or the canonical chain does not carry it at all. * * Comparing consensus rolling hashes is enough to compare whole prefixes of leaves: each one chains the hash - * before it, so equal hashes at an index mean equal leaves at every index through it. A message that the canonical - * chain does not carry at all counts as invalidated. + * before it, so equal hashes at an index mean equal leaves at every index through it. * - * The boundary comparison is deliberately one-sided. A reorg that merges two buckets into one destroys the - * boundary between them, so the blocks that ended there are pruned; one that splits a bucket only adds a boundary - * no block ever ended on, and every boundary that existed still does, so it prunes nothing. - * - * The one boundary this cannot settle is the newest one: when the canonical fetch reaches the L1 head it cannot - * tell a bucket that closed from one that is still open, and a bucket that carries on absorbing messages later - * takes its boundary away. That exposure is the same one ordinary ingestion has, since it stores an open head - * bucket as readily as a closed one, and is neither created nor widened here. + * Bucket boundaries play no part. A block is authenticated by the message prefix it consumed — its signed header + * count paired with the signed prefix rolling hash — and not by the boundary it happened to stop on, so a reorg + * that re-mines the same messages under different boundaries leaves every block on the proposed chain exactly as + * valid as when it was signed, and drops nothing. Only the completed checkpoint endpoint has to sit on a current + * boundary, and that is re-checked where it matters: the proposer re-resolves its L1 `bucketHint` before + * publishing, and a validator re-resolves the final position before attesting. */ private async findFirstInvalidatedMessageIndex( fromIndex: bigint, @@ -877,29 +872,11 @@ export class ArchiverL1Synchronizer implements Traceable { const canonicalAt = (index: bigint) => firstCanonicalIndex === undefined ? undefined : canonicalMessages[Number(index - firstCanonicalIndex)]; - let previous: { stored: InboxMessage; canonical: InboxMessage } | undefined; for await (const stored of this.stores.messages.iterateL1ToL2Messages({ start: fromIndex })) { const canonical = canonicalAt(stored.index); if (canonical === undefined || !canonical.inboxRollingHash.equals(stored.inboxRollingHash)) { return stored.index; } - if (previous !== undefined) { - const storedBucketClosed = previous.stored.bucketSeq !== stored.bucketSeq; - const canonicalBucketClosed = previous.canonical.bucketSeq !== canonical.bucketSeq; - if (storedBucketClosed && !canonicalBucketClosed) { - return previous.stored.index; - } - } - previous = { stored, canonical }; - } - - // The last message we hold closes the last bucket we hold, so a canonical bucket that carries on past it takes - // that boundary away as much as a merge does. - if (previous !== undefined) { - const next = canonicalAt(previous.stored.index + 1n); - if (next !== undefined && next.bucketSeq === previous.canonical.bucketSeq) { - return previous.stored.index; - } } return undefined; } diff --git a/yarn-project/archiver/src/store/message_store.test.ts b/yarn-project/archiver/src/store/message_store.test.ts index 057504304f6a..f635491930a8 100644 --- a/yarn-project/archiver/src/store/message_store.test.ts +++ b/yarn-project/archiver/src/store/message_store.test.ts @@ -722,6 +722,68 @@ describe('MessageStore', () => { expect(await messageStore.getInboxBucketByRollingHash(Fr.ZERO)).toMatchObject({ seq: 0n, totalMsgCount: 0n }); }); + describe('getInboxRollingHashAt', () => { + it('returns the prefix hash at every synced count, on a boundary or interior to a bucket', async () => { + const msgs = makeBucketedMessages(threeBucketSpec); + await messageStore.addL1ToL2MessageBuckets(msgs); + + // Every count resolves, whether or not a bucket ends there: msgs[2] closes bucket 1 and msgs[3] is interior + // to bucket 2, and both are addressable by the prefix length they complete. + for (const [index, msg] of msgs.entries()) { + expect(await messageStore.getInboxRollingHashAt(BigInt(index) + 1n)).toEqual(msg.inboxRollingHash); + } + // The interior count carries a hash no bucket does, which is exactly the case a merge leaves behind. + expect(await messageStore.getInboxBucketByRollingHash(msgs[3].inboxRollingHash)).toBeUndefined(); + }); + + it('returns the zero hash at count zero, even on an empty store', async () => { + expect(await messageStore.getInboxRollingHashAt(0n)).toEqual(Fr.ZERO); + + await messageStore.addL1ToL2MessageBuckets(makeBucketedMessages(threeBucketSpec)); + expect(await messageStore.getInboxRollingHashAt(0n)).toEqual(Fr.ZERO); + }); + + it('returns undefined past the synced tip', async () => { + const msgs = makeBucketedMessages(threeBucketSpec); + await messageStore.addL1ToL2MessageBuckets(msgs); + + expect(await messageStore.getInboxRollingHashAt(BigInt(msgs.length) + 1n)).toBeUndefined(); + expect(await messageStore.getInboxRollingHashAt(1000n)).toBeUndefined(); + }); + + it('stops resolving counts a removal dropped', async () => { + const msgs = makeBucketedMessages(threeBucketSpec); + await messageStore.addL1ToL2MessageBuckets(msgs); + await messageStore.removeL1ToL2Messages(msgs[4].index); + + expect(await messageStore.getInboxRollingHashAt(4n)).toEqual(msgs[3].inboxRollingHash); + expect(await messageStore.getInboxRollingHashAt(5n)).toBeUndefined(); + expect(await messageStore.getInboxRollingHashAt(6n)).toBeUndefined(); + }); + + it('rejects a negative count', async () => { + await expect(messageStore.getInboxRollingHashAt(-1n)).rejects.toThrow('Invalid Inbox message count'); + }); + + it('keeps every prefix hash across a pure repartition of the same messages', async () => { + const msgs = makeBucketedMessages(threeBucketSpec); + await messageStore.addL1ToL2MessageBuckets(msgs); + const before = await Promise.all(msgs.map((_, i) => messageStore.getInboxRollingHashAt(BigInt(i) + 1n))); + + // A reorg re-mines buckets 2 and 3 as a single bucket: same leaves, moved boundary. + await messageStore.removeL1ToL2Messages(msgs[3].index); + const merged = [msgs[3], msgs[4], msgs[5]].map(msg => ({ ...msg, bucketSeq: 2n, bucketTimestamp: 250n })); + await messageStore.addL1ToL2MessageBuckets(merged); + + expect(await Promise.all(msgs.map((_, i) => messageStore.getInboxRollingHashAt(BigInt(i) + 1n)))).toEqual( + before, + ); + // The boundary at count 4 is gone as a bucket, but the prefix at that count still authenticates. + expect(await messageStore.getInboxBucketByRollingHash(msgs[3].inboxRollingHash)).toBeUndefined(); + expect(await messageStore.getInboxRollingHashAt(4n)).toEqual(msgs[3].inboxRollingHash); + }); + }); + it('drops rolling-hash entries of buckets a removal deletes or rewrites', async () => { const msgs = makeBucketedMessages(threeBucketSpec); await messageStore.addL1ToL2MessageBuckets(msgs); diff --git a/yarn-project/archiver/src/store/message_store.ts b/yarn-project/archiver/src/store/message_store.ts index 1591df23d2dc..4632b1f4dab0 100644 --- a/yarn-project/archiver/src/store/message_store.ts +++ b/yarn-project/archiver/src/store/message_store.ts @@ -106,7 +106,7 @@ function toBucketL1Span(seq: bigint, snapshot: BucketSnapshot): InboxBucketL1Spa export type InboxMessageReplacement = { /** Sequence of the newest bucket known to sit on canonical L1 blocks, if any. */ lastCanonicalBucketSeq: bigint | undefined; - /** Lowest message index whose leaf or bucket boundary the canonical chain no longer backs, if any. */ + /** Lowest message index whose leaf the canonical chain no longer backs, if any. Boundaries play no part. */ firstInvalidatedIndex: bigint | undefined; /** The canonical messages from the last canonical bucket's opening L1 block onwards. */ messages: InboxMessage[]; @@ -510,10 +510,10 @@ export class MessageStore { * Replaces everything the store holds above the given bucket with the messages the canonical chain delivers for the * same range, and moves the sync point to the L1 block that range was fetched up to, all in a single transaction. * - * The caller has already compared the two views: `firstInvalidatedIndex` is the lowest index whose leaf changed or - * whose bucket boundary the canonical chain took away, and is undefined when the canonical chain re-delivers the - * very same leaves under the same bucket boundaries, in which case not a single message is dropped and only the - * bucket metadata derived from the L1 blocks moves. The steps depend on each other in order: + * The caller has already compared the two views: `firstInvalidatedIndex` is the lowest index whose leaf changed, + * and is undefined when the canonical chain re-delivers the very same leaves — under whatever bucket boundaries — + * in which case not a single message is dropped and only the bucket partition and the metadata derived from the L1 + * blocks move. The steps depend on each other in order: * the removal rewrites the snapshot of the bucket left holding the last surviving message, so it has to run while * that snapshot is still there; the snapshots above the last canonical bucket then go, because a re-delivery that * renumbers or merges buckets would otherwise be rejected as incomplete or leave a stale snapshot behind; and only @@ -579,6 +579,30 @@ export class MessageStore { return bucket !== undefined && bucket.totalMsgCount === totalMsgCount ? bucket : undefined; } + /** + * Returns the consensus rolling hash of the canonical message prefix of length `totalMsgCount`, or undefined when + * this archiver has not synced a message at index `totalMsgCount - 1`. + * + * This is the count-addressed counterpart of {@link getInboxBucketByRollingHash}: it answers "what does the prefix + * of this many messages hash to" without requiring the count to sit on a boundary of the bucket partition the + * archiver currently holds. A block's L1-to-L2 tree leaf count is exactly such a prefix length, and an L1 reorg + * that merges buckets can leave it permanently interior, so consumers that authenticate a block's consumed prefix + * resolve it this way rather than through a bucket. + * + * Count zero is the genesis base case and returns the zero hash. Because each message's hash chains the one before + * it, equality at a count proves equality of every leaf through it. + */ + public async getInboxRollingHashAt(totalMsgCount: bigint): Promise { + if (totalMsgCount < 0n) { + throw new Error(`Invalid Inbox message count ${totalMsgCount}`); + } + if (totalMsgCount === 0n) { + return Fr.ZERO; + } + const buffer = await this.#l1ToL2Messages.getAsync(this.indexToKey(totalMsgCount - 1n)); + return buffer && deserializeInboxMessage(buffer).inboxRollingHash; + } + /** * Returns the Inbox bucket whose consensus rolling hash equals the given one, or undefined if no synced bucket * carries it. A rolling hash commits to every message absorbed up to its bucket, so it identifies a bucket by diff --git a/yarn-project/archiver/src/test/mock_archiver.ts b/yarn-project/archiver/src/test/mock_archiver.ts index 228dc1e41e15..1bdc9bd5a267 100644 --- a/yarn-project/archiver/src/test/mock_archiver.ts +++ b/yarn-project/archiver/src/test/mock_archiver.ts @@ -22,6 +22,10 @@ export class MockArchiver extends MockL2BlockSource implements L2BlockSource, L1 this.messageSource.replaceInboxBuckets(buckets); } + public setInboxRollingHashAt(totalMsgCount: bigint, inboxRollingHash: Fr) { + this.messageSource.setInboxRollingHashAt(totalMsgCount, inboxRollingHash); + } + getL1ToL2MessageIndex(_l1ToL2Message: Fr): Promise { return this.messageSource.getL1ToL2MessageIndex(_l1ToL2Message); } @@ -42,6 +46,10 @@ export class MockArchiver extends MockL2BlockSource implements L2BlockSource, L1 return this.messageSource.getInboxBucketByRollingHash(inboxRollingHash); } + getInboxRollingHashAt(totalMsgCount: bigint): Promise { + return this.messageSource.getInboxRollingHashAt(totalMsgCount); + } + getL1ToL2MessagesBetweenBuckets(fromExclusive: bigint, toInclusive: bigint): Promise { return this.messageSource.getL1ToL2MessagesBetweenBuckets(fromExclusive, toInclusive); } diff --git a/yarn-project/archiver/src/test/mock_l1_to_l2_message_source.ts b/yarn-project/archiver/src/test/mock_l1_to_l2_message_source.ts index 72dd44cbbd9e..d3b4d282ae99 100644 --- a/yarn-project/archiver/src/test/mock_l1_to_l2_message_source.ts +++ b/yarn-project/archiver/src/test/mock_l1_to_l2_message_source.ts @@ -14,6 +14,12 @@ export class MockL1ToL2MessageSource implements L1ToL2MessageSource { * repartition the buckets (as an L1 reorg does) while the indexed leaves stay exactly as they were. */ private leavesByIndex = new Map(); + /** + * The canonical prefix rolling hash, keyed by prefix length. Like {@link leavesByIndex} this is kept apart from the + * bucket partition, so a boundary a repartition takes away stays resolvable as an interior prefix — which is what + * an already-signed proposal reference names once its bucket is merged. + */ + private rollingHashByCount = new Map(); constructor(private blockNumber: number) {} @@ -24,11 +30,20 @@ export class MockL1ToL2MessageSource implements L1ToL2MessageSource { // out of order keeps every leaf at the index the archiver would give it. const firstIndex = bucket.totalMsgCount - BigInt(msgs.length); msgs.forEach((msg, i) => this.leavesByIndex.set(firstIndex + BigInt(i), msg)); + this.setInboxRollingHashAt(bucket.totalMsgCount, bucket.inboxRollingHash); + } + + /** Registers the canonical prefix rolling hash at a message count that no current bucket needs to end on. */ + public setInboxRollingHashAt(totalMsgCount: bigint, inboxRollingHash: Fr) { + if (totalMsgCount > 0n) { + this.rollingHashByCount.set(totalMsgCount, inboxRollingHash); + } } /** - * Replaces the current bucket partition without touching the indexed leaf log, modelling an L1 reorg that re-mines - * the same messages under different bucket boundaries. + * Replaces the current bucket partition without touching the indexed leaf log or the prefix hashes already + * registered, modelling an L1 reorg that re-mines the same messages under different bucket boundaries. The new + * buckets' own boundary hashes are registered as prefix hashes too. */ public replaceInboxBuckets(buckets: { bucket: InboxBucket; msgs: Fr[] }[]) { this.buckets = new Map(); @@ -36,6 +51,7 @@ export class MockL1ToL2MessageSource implements L1ToL2MessageSource { for (const { bucket, msgs } of buckets) { this.buckets.set(bucket.seq, bucket); this.messagesPerBucket.set(bucket.seq, msgs); + this.setInboxRollingHashAt(bucket.totalMsgCount, bucket.inboxRollingHash); } } @@ -65,6 +81,13 @@ export class MockL1ToL2MessageSource implements L1ToL2MessageSource { return Promise.resolve([...this.buckets.values()].find(bucket => bucket.inboxRollingHash.equals(inboxRollingHash))); } + getInboxRollingHashAt(totalMsgCount: bigint): Promise { + if (totalMsgCount < 0n) { + return Promise.reject(new Error(`Invalid Inbox message count ${totalMsgCount}`)); + } + return Promise.resolve(totalMsgCount === 0n ? Fr.ZERO : this.rollingHashByCount.get(totalMsgCount)); + } + getLatestInboxBucketAtOrBefore(timestamp: bigint): Promise { const atOrBefore = [...this.buckets.values()] .filter(bucket => bucket.timestamp <= timestamp) diff --git a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts index 2dd7010c8a00..4b6b47b09ded 100644 --- a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.test.ts @@ -118,8 +118,8 @@ describe('NodePublicCallsSimulator', () => { }); /** - * Mocks the Inbox so the next-block prediction selects a two-message bundle: the fork's message total (0) - * resolves to bucket 0, and bucket 1 holds both messages. + * Mocks the Inbox so the next-block prediction selects a two-message bundle: the prefix at the fork's message + * total (0) is the genesis zero hash, and bucket 1 holds both messages. */ const mockInboxSelection = (timestamp = 0n) => { const makeBucket = (seq: bigint, totalMsgCount: bigint): InboxBucket => ({ @@ -133,9 +133,9 @@ describe('NodePublicCallsSimulator', () => { l1BlockHash: Buffer32.fromBigInt(seq), }); const bundle = [new Fr(0x1234), new Fr(0x5678)]; - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(makeBucket(0n, 0n)); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(Fr.ZERO); l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(makeBucket(1n, 2n)); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(bundle); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(bundle); return bundle; }; @@ -173,9 +173,9 @@ describe('NodePublicCallsSimulator', () => { }); blockSource.getPendingChainValidationStatus.mockResolvedValue({ valid: true }); blockSource.getProposedCheckpointData.mockResolvedValue(undefined); - // No Inbox bucket resolves to the fork's message total by default, so the next-block message prediction + // No Inbox prefix is synced at the fork's message total by default, so the next-block message prediction // bails out and tests see the bare tip state unless they opt into it. - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(undefined); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(undefined); epochCache.getL1Constants.mockReturnValue(EmptyL1RollupConstants); globalVariableBuilder.buildCheckpointGlobalVariables.mockImplementation((_c, _f, slotNumber) => diff --git a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.ts b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.ts index 847e7f9e91a3..f5891d93b347 100644 --- a/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.ts +++ b/yarn-project/aztec-node/src/aztec-node/node_public_calls_simulator.ts @@ -44,7 +44,7 @@ import { type TelemetryClient, getTelemetryClient } from '@aztec/telemetry-clien import { applyPublicDataOverrides } from './public_data_overrides.js'; /** Inbox queries the simulator needs to predict the message bundle the next block would consume. */ -type SimulatorInboxSource = InboxBucketSource & Pick; +type SimulatorInboxSource = InboxBucketSource & Pick; /** Config fields the simulator needs — a narrow subset of `AztecNodeConfig`. */ export interface NodePublicCallsSimulatorConfig { @@ -300,9 +300,9 @@ export class NodePublicCallsSimulator { ): Promise { try { const parentTotalMsgCount = (await fork.getTreeInfo(MerkleTreeId.L1_TO_L2_MESSAGE_TREE)).size; - const parentBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(parentTotalMsgCount); - if (parentBucket === undefined) { - this.log.debug(`Inbox bucket at message total ${parentTotalMsgCount} not synced; simulating against the tip`, { + const parentInboxRollingHash = await this.l1ToL2MessageSource.getInboxRollingHashAt(parentTotalMsgCount); + if (parentInboxRollingHash === undefined) { + this.log.debug(`Inbox prefix at message total ${parentTotalMsgCount} not synced; simulating against the tip`, { parentTotalMsgCount, }); return; @@ -325,7 +325,7 @@ export class NodePublicCallsSimulator { now: BigInt(Math.floor(this.dateProvider.now() / 1000)), isEligible: this.getInboxBucketEligibility(l1Constants.ethereumSlotDuration), ethereumSlotDuration: l1Constants.ethereumSlotDuration, - parent: { seq: parentBucket.seq, totalMsgCount: parentBucket.totalMsgCount }, + cursor: { totalMsgCount: parentTotalMsgCount, inboxRollingHash: parentInboxRollingHash }, checkpointStartTotalMsgCount, perBlockCap: MAX_L1_TO_L2_MSGS_PER_BLOCK, perCheckpointCap: MAX_L1_TO_L2_MSGS_PER_CHECKPOINT, diff --git a/yarn-project/sequencer-client/src/index.ts b/yarn-project/sequencer-client/src/index.ts index 2b68a635811e..5d51b7c2dd43 100644 --- a/yarn-project/sequencer-client/src/index.ts +++ b/yarn-project/sequencer-client/src/index.ts @@ -13,7 +13,7 @@ export { immediateEligibility, } from './sequencer/inbox_bucket_eligibility.js'; export { - type ConsumedBucketCursor, + type ConsumedMessagePrefixCursor, type InboxBucketSelection, type InboxBucketSource, type SelectInboxBucketInput, diff --git a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts index a65d86047d59..c01df730a082 100644 --- a/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts +++ b/yarn-project/sequencer-client/src/publisher/l1_publisher.integration.test.ts @@ -381,11 +381,18 @@ describe('L1Publisher integration', () => { getBlockNumber(): Promise { return Promise.resolve(BlockNumber(blocks.at(-1)?.number ?? BlockNumber.ZERO)); }, - // Streaming L1->L2 message reconstruction: the world-state synchronizer resolves each - // block's consumed message bundle from the Inbox buckets registered per published block in buildAndPublishBlock. + // Streaming L1->L2 message reconstruction: the world-state synchronizer resolves each block's consumed + // message bundle from the compact count range its header commits to, over the Inbox buckets registered per + // published block in buildAndPublishBlock. getInboxBucketByTotalMsgCount(totalMsgCount: bigint) { return messageSource.getInboxBucketByTotalMsgCount(totalMsgCount); }, + getInboxRollingHashAt(totalMsgCount: bigint) { + return messageSource.getInboxRollingHashAt(totalMsgCount); + }, + getL1ToL2MessagesBetweenLeafCounts(startLeafCount: bigint, endLeafCount: bigint) { + return messageSource.getL1ToL2MessagesBetweenLeafCounts(startLeafCount, endLeafCount); + }, getL1ToL2MessagesBetweenBuckets(fromExclusive: bigint, toInclusive: bigint) { return messageSource.getL1ToL2MessagesBetweenBuckets(fromExclusive, toInclusive); }, @@ -586,8 +593,10 @@ describe('L1Publisher integration', () => { const allSentMessages: Fr[] = []; let mirroredThroughSeq = 0n; let mirroredThroughTotal = 0n; - // The last Inbox bucket this checkpoint chain has consumed through; genesis sentinel to start. - let parent = { seq: 0n, totalMsgCount: 0n }; + // The Inbox message prefix this checkpoint chain has consumed through; the genesis base case to start. + let cursor = { totalMsgCount: 0n, inboxRollingHash: Fr.ZERO }; + // Sequence of the bucket the chain last consumed through, for the L1 propose hint. + let consumedBucketSeq = 0n; let previousInboxRollingHash = Fr.ZERO; const blobFieldsPerCheckpoint: Fr[][] = []; // The below batched blob is used for testing different epochs with 1..numberOfConsecutiveBlocks blocks on L1. @@ -678,15 +687,15 @@ describe('L1Publisher integration', () => { now: cutoffTimestamp, isEligible: immediateEligibility, ethereumSlotDuration: config.ethereumSlotDuration, - parent, - checkpointStartTotalMsgCount: parent.totalMsgCount, + cursor, + checkpointStartTotalMsgCount: cursor.totalMsgCount, perBlockCap: MAX_L1_TO_L2_MSGS_PER_BLOCK, perCheckpointCap: MAX_L1_TO_L2_MSGS_PER_CHECKPOINT, isLastBlock: true, cutoffTimestamp, }); const currentL1ToL2Messages = selection.consume ? selection.bundle : []; - const bucketHint = selection.consume ? selection.bucket.seq : parent.seq; + const bucketHint = selection.consume ? selection.bucket.seq : consumedBucketSeq; const checkpoint = await buildCheckpoint( globalVariables, @@ -698,7 +707,11 @@ describe('L1Publisher integration', () => { previousInboxRollingHash = checkpoint.header.inboxRollingHash; const block = checkpoint.blocks[0]; if (selection.consume) { - parent = { seq: selection.bucket.seq, totalMsgCount: selection.bucket.totalMsgCount }; + cursor = { + totalMsgCount: selection.bucket.totalMsgCount, + inboxRollingHash: selection.bucket.inboxRollingHash, + }; + consumedBucketSeq = selection.bucket.seq; } const totalManaUsed = txs.reduce((acc, tx) => acc.add(new Fr(tx.gasUsed.billedGas.l2Gas)), Fr.ZERO); diff --git a/yarn-project/sequencer-client/src/sequencer/automine/automine_sequencer.ts b/yarn-project/sequencer-client/src/sequencer/automine/automine_sequencer.ts index 3acd2ffc5c63..6a9c860e9ec3 100644 --- a/yarn-project/sequencer-client/src/sequencer/automine/automine_sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/automine/automine_sequencer.ts @@ -24,7 +24,7 @@ import { getTimestampForSlot, } from '@aztec/stdlib/epoch-helpers'; import { InsufficientValidTxsError, type WorldStateSynchronizer } from '@aztec/stdlib/interfaces/server'; -import { type L1ToL2MessageSource, getInboxCutoffTimestamp } from '@aztec/stdlib/messaging'; +import { InboxBucketRef, type L1ToL2MessageSource, getInboxCutoffTimestamp } from '@aztec/stdlib/messaging'; import type { CoordinationSignatureContext } from '@aztec/stdlib/p2p'; import { MerkleTreeId } from '@aztec/stdlib/trees'; import type { FailedTx, Tx } from '@aztec/stdlib/tx'; @@ -475,28 +475,62 @@ export class AutomineSequencer { // checkpoint's final block; select its bundle from the newest synced bucket with the last-block censorship floor. // Automine never waits for L1 confirmations: anvil mines on demand, so a bucket's opening block gains a // descendant only when the next transaction is sent, which may be long after the block that consumes it. - // The parent total is the fork's L1-to-L2 leaf count (compact indexing), which resolves the parent bucket. + // The parent total is the fork's L1-to-L2 leaf count (compact indexing), and the cursor is that count plus the + // prefix hash there — not a bucket, so an L1 reorg that repartitioned the same messages and left the parent's + // count interior does not stall automine forever. const parentInfo = await fork.getTreeInfo(MerkleTreeId.L1_TO_L2_MESSAGE_TREE); const parentTotalMsgCount = parentInfo.size; - const parentBucket = await this.deps.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(parentTotalMsgCount); - if (parentBucket === undefined) { - this.log.warn(`Automine streaming inbox: parent bucket for total ${parentTotalMsgCount} not synced; skipping`); + const parentInboxRollingHash = await this.deps.l1ToL2MessageSource.getInboxRollingHashAt(parentTotalMsgCount); + if (parentInboxRollingHash === undefined) { + this.log.warn(`Automine streaming inbox: Inbox prefix at total ${parentTotalMsgCount} not synced; skipping`); return undefined; } + const cursor = { totalMsgCount: parentTotalMsgCount, inboxRollingHash: parentInboxRollingHash }; const selection = await selectInboxBucketForBlock({ messageSource: this.deps.l1ToL2MessageSource, now: BigInt(Math.floor(this.deps.dateProvider.now() / 1000)), isEligible: immediateEligibility, ethereumSlotDuration: this.deps.l1Constants.ethereumSlotDuration, - parent: { seq: parentBucket.seq, totalMsgCount: parentBucket.totalMsgCount }, + cursor, checkpointStartTotalMsgCount: parentTotalMsgCount, perBlockCap: MAX_L1_TO_L2_MSGS_PER_BLOCK, perCheckpointCap: MAX_L1_TO_L2_MSGS_PER_CHECKPOINT, isLastBlock: true, cutoffTimestamp: getInboxCutoffTimestamp(SlotNumber(targetSlot), this.deps.l1Constants), }); + // The selector reports when no checkpoint ending on this block can satisfy the censorship floor. Automine's + // single block is always the checkpoint's last, so building it would only produce a checkpoint L1 reverts. + if (selection.insufficientFinalBlockCapacity) { + this.log.warn( + `Automine streaming inbox: the mandatory Inbox backlog does not fit a single-block checkpoint; skipping`, + ); + return undefined; + } + const streamingBundle = selection.consume ? selection.bundle : []; - const bucketHint = selection.consume ? selection.bucket.seq : parentBucket.seq; + // The block's own consumed prefix, which is also the checkpoint's since automine builds a single block. + const consumedPrefix = selection.consume + ? { totalMsgCount: selection.bucket.totalMsgCount, inboxRollingHash: selection.bucket.inboxRollingHash } + : cursor; + + // Only the checkpoint endpoint has to be a current boundary, since L1 reads the header's rolling hash out of the + // bucket the hint names. Automine's single block is that endpoint, so resolve it rather than assuming the + // cursor's count still is one, and match the hash too: a bucket ending at the same count over different messages + // is a hint `propose` would revert on. + const finalBucket = await this.deps.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(consumedPrefix.totalMsgCount); + if (finalBucket === undefined || !finalBucket.inboxRollingHash.equals(consumedPrefix.inboxRollingHash)) { + this.log.warn( + `Automine streaming inbox: consumed prefix at total ${consumedPrefix.totalMsgCount} does not resolve to a ` + + `current bucket carrying its rolling hash, so no checkpoint can be published against it; skipping`, + { + consumedTotalMsgCount: consumedPrefix.totalMsgCount, + consumedInboxRollingHash: consumedPrefix.inboxRollingHash.toString(), + resolvedInboxRollingHash: finalBucket?.inboxRollingHash.toString(), + }, + ); + return undefined; + } + const bucketHint = finalBucket.seq; const checkpointBuilder = await this.deps.checkpointsBuilder.startCheckpoint( checkpointNumber, @@ -536,7 +570,7 @@ export class AutomineSequencer { // first means the archiver already has the proposed entry when L1 polling fires; the L1 // sync path then promotes the existing proposed checkpoint via promoteProposedToCheckpointed // rather than re-adding it. - await this.deps.archiver.addBlock(buildResult.block); + await this.deps.archiver.addBlock(buildResult.block, new InboxBucketRef(consumedPrefix.inboxRollingHash)); await this.deps.archiver.addProposedCheckpoint({ header: checkpoint.header, checkpointNumber, 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 1faeaed7a4eb..b302eafa8bf0 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 @@ -1220,7 +1220,7 @@ describe('CheckpointProposalJob', () => { l1ToL2MessageSource.getLatestInboxBucketAtOrBefore .mockResolvedValueOnce(makeBucket(2n, 2n, 1n)) .mockResolvedValue(makeBucket(3n, 4n, 3n)); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts .mockResolvedValueOnce([new Fr(1), new Fr(2)]) .mockResolvedValue([new Fr(3), new Fr(4)]); mockInboxBuckets(l1ToL2MessageSource, [makeBucket(2n, 2n, 1n), makeBucket(3n, 4n, 3n)]); @@ -1509,7 +1509,7 @@ describe('CheckpointProposalJob', () => { }; const bundle = Array.from({ length: 5 }, (_, i) => new Fr(i + 1)); l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(bucket); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(bundle); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(bundle); mockInboxBuckets(l1ToL2MessageSource, [bucket]); checkpointBuilder.inboxRollingHash = bucket.inboxRollingHash; @@ -1553,7 +1553,7 @@ describe('CheckpointProposalJob', () => { }; const bundle = Array.from({ length: 5 }, (_, i) => new Fr(i + 1)); l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(bucket); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(bundle); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(bundle); mockInboxBuckets(l1ToL2MessageSource, [bucket]); checkpointBuilder.inboxRollingHash = bucket.inboxRollingHash; @@ -1595,7 +1595,7 @@ describe('CheckpointProposalJob', () => { }; const bundle = Array.from({ length: 5 }, (_, i) => new Fr(i + 1)); l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(bucket); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(bundle); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(bundle); mockInboxBuckets(l1ToL2MessageSource, [bucket]); checkpointBuilder.inboxRollingHash = bucket.inboxRollingHash; @@ -1626,7 +1626,7 @@ describe('CheckpointProposalJob', () => { const bundle = Array.from({ length: Number(bucket.totalMsgCount) }, (_, i) => new Fr(i + 1)); l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(bucket); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(bundle); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(bundle); mockInboxBuckets(l1ToL2MessageSource, [bucket]); checkpointBuilder.inboxRollingHash = bucket.inboxRollingHash; @@ -1678,20 +1678,15 @@ describe('CheckpointProposalJob', () => { await setupSingleBucketCheckpoint(bucket); mockInboxBuckets(l1ToL2MessageSource, [{ ...bucket, seq: 1n }]); - const infoLog = jest.spyOn(job.log, 'info'); - const checkpoint = await job.executeAndAwait(); expect(checkpoint).toBeDefined(); // The L1 propose hint (4th positional arg) is the re-resolved sequence number, not the one built against. expect(publisher.enqueueProposeCheckpoint.mock.calls[0][3]).toBe(1n); - // The signed block proposal carries only the rolling hash, which the merge leaves alone, so validators on - // either side of it resolve the same bucket. + // The signed block proposal carries only the prefix rolling hash, which the merge leaves alone. The + // consumption cursor is a count and a hash rather than a bucket, so the merge does not disturb it at all and + // the slot is not abandoned. expect(validatorClient.createBlockProposal.mock.calls[0][7]?.inboxRollingHash).toEqual(bucket.inboxRollingHash); - expect(infoLog).toHaveBeenCalledWith( - 'Inbox bucket reference re-resolved after L1 reorg', - expect.objectContaining({ builtSeq: 2n, resolvedSeq: 1n }), - ); }); it('re-resolves the hint again for a reorg that lands while attestations are collected', async () => { @@ -1712,7 +1707,7 @@ describe('CheckpointProposalJob', () => { expect(publisher.enqueueProposeCheckpoint.mock.calls[0][3]).toBe(1n); expect(infoLog).toHaveBeenCalledWith( 'Inbox bucket hint re-resolved after L1 reorg', - expect.objectContaining({ builtSeq: 2n, publishedSeq: 1n }), + expect.objectContaining({ previousSeq: 2n, publishedSeq: 1n }), ); }); @@ -1753,6 +1748,24 @@ describe('CheckpointProposalJob', () => { expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('inbox_consumption_insufficient'); }); + it('abandons the slot when the final position is a boundary over different messages', async () => { + // The pre-proposal gate has to prove the count *and* the hash. A bucket ending exactly where the checkpoint + // does but over different messages is a hint `propose` would revert on, so nothing may be published — even + // though the per-block prefix check (which is count-addressed, and left consistent here) passed. + const bucket = pendingBucket(); + await setupSingleBucketCheckpoint(bucket); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue({ + ...bucket, + inboxRollingHash: new Fr(0xdead), + }); + + const checkpoint = await job.executeAndAwait(); + + expect(checkpoint).toBeUndefined(); + expect(publisher.enqueueProposeCheckpoint).not.toHaveBeenCalled(); + expect(metrics.recordCheckpointProposalFailed).toHaveBeenCalledWith('inbox_bucket_reorged'); + }); + it('aborts the checkpoint before signing when the consumed bucket vanishes at the per-block refresh', async () => { // The block builds against bucket 2, then an L1 reorg reorders the messages before the proposal is signed: no // bucket carries the rolling hash the block consumed through any more. The block is known to be built on @@ -1818,12 +1831,8 @@ describe('CheckpointProposalJob', () => { l1ToL2MessageSource.getLatestInboxBucketAtOrBefore.mockResolvedValue(buckets[3]); l1ToL2MessageSource.getInboxBucket.mockImplementation(seq => Promise.resolve(buckets[Number(seq) - 1])); mockInboxBuckets(l1ToL2MessageSource, buckets); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockImplementation((from, to) => - Promise.resolve( - Array.from({ length: Number(totals[Number(to) - 1] - (from === 0n ? 0n : totals[Number(from) - 1])) }, () => - Fr.random(), - ), - ), + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockImplementation((from, to) => + Promise.resolve(Array.from({ length: Number(to - from) }, () => Fr.random())), ); const { lastBlock } = await setupMultipleBlocks(2, [2, 1]); @@ -2127,7 +2136,8 @@ describe('CheckpointProposalJob', () => { // The checkpoint should be aborted since the archiver sync failure now propagates expect(checkpoint).toBeUndefined(); - expect(blockSink.addBlock).toHaveBeenCalledWith(block); + // The push carries the block's signed Inbox prefix reference, which the archiver re-validates on insert. + expect(blockSink.addBlock).toHaveBeenCalledWith(block, expect.objectContaining({ inboxRollingHash: Fr.ZERO })); // Should not attempt to collect attestations since the error aborts the loop expect(validatorClient.collectAttestations).not.toHaveBeenCalled(); }); 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 3ec51cc4ea82..0a0c57e84202 100644 --- a/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts +++ b/yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts @@ -93,7 +93,7 @@ import { immediateEligibility, } from './inbox_bucket_eligibility.js'; import { - type ConsumedBucketCursor, + type ConsumedMessagePrefixCursor, type InboxBucketSelection, selectInboxBucketForBlock, } from './inbox_bucket_selector.js'; @@ -150,15 +150,23 @@ type BlockBuildingResult = /** * Running state of streaming Inbox message selection across the blocks of one checkpoint. - * Consumption starts from the parent checkpoint's last-consumed bucket and advances one block at a time. + * Consumption starts from the parent checkpoint's consumed message prefix and advances one block at a time. */ type StreamingCheckpointState = { /** Cumulative Inbox message count consumed as of the parent checkpoint; the per-checkpoint cap origin (fixed). */ checkpointStartTotalMsgCount: bigint; - /** The last bucket consumed so far (parent checkpoint's at the first block); advances as blocks consume. */ - parent: ConsumedBucketCursor; - /** Reference to the last consumed bucket; reused by blocks that consume nothing. */ - lastBucketRef: InboxBucketRef; + /** + * The message prefix consumed so far (the parent checkpoint's at the first block); advances as blocks consume. + * A count and a hash rather than a bucket, so an L1 repartition that leaves the position interior to a current + * bucket does not strand the checkpoint mid-build. + */ + cursor: ConsumedMessagePrefixCursor; + /** + * Sequence number of the bucket the consumed position last resolved to, for the L1 propose hint. Pure metadata: + * a reorg that renumbers buckets moves it while leaving the consumed prefix — and so the signed headers and + * proposals — untouched, which is why it is tracked apart from the cursor and re-read before every publish. + */ + lastResolvedBucketSeq?: bigint; }; /** @@ -870,11 +878,25 @@ export class CheckpointProposalJob implements Traceable { return undefined; } - // Streaming Inbox censorship floor: re-derive the mandatory-consumption verdict from the final consumed - // position, independently of the per-block selection that produced it. Runs before the checkpoint is assembled, - // so the held last block and the checkpoint proposal are both still ungossiped and the propose tx is never - // sent for a checkpoint L1 would revert and validators would refuse to attest. - const consumption = await this.checkCheckpointConsumption(streamingState); + // Streaming Inbox: the completed checkpoint's final consumed position must resolve to a current bucket, since + // that is the one L1 reads the header's rolling hash out of via the `bucketHint`. Individual blocks are not + // held to this — a repartition that keeps their messages leaves them valid — but a checkpoint ending on a + // position no bucket carries cannot be published at all, so the slot is abandoned instead. + const finalBucket = await this.resolveFinalConsumedBucket(streamingState); + if (finalBucket === undefined) { + this.reportConsumedPrefixVanished(streamingState, 'build-failed', { + blocksBuilt: blocksInCheckpoint.length, + note: 'the blocks themselves are content-valid; only the checkpoint endpoint is unpublishable', + }); + return undefined; + } + streamingState.lastResolvedBucketSeq = finalBucket.seq; + + // Censorship floor: re-derive the mandatory-consumption verdict from the final consumed position, + // independently of the per-block selection that produced it. Runs before the checkpoint is assembled, so the + // held last block and the checkpoint proposal are both still ungossiped and the propose tx is never sent for a + // checkpoint L1 would revert and validators would refuse to attest. + const consumption = await this.checkCheckpointConsumption(streamingState, finalBucket); if (!consumption.sufficient) { const { cutoffTimestamp, nextBucket } = consumption; const context = { @@ -883,8 +905,8 @@ export class CheckpointProposalJob implements Traceable { blocksBuilt: blocksInCheckpoint.length, blocksRemaining: this.config.maxBlocksPerCheckpoint - blocksInCheckpoint.length, checkpointStartTotalMsgCount: streamingState.checkpointStartTotalMsgCount, - consumedBucketSeq: streamingState.parent.seq, - consumedTotalMsgCount: streamingState.parent.totalMsgCount, + consumedBucketSeq: finalBucket.seq, + consumedTotalMsgCount: finalBucket.totalMsgCount, unconsumedBucketSeq: nextBucket?.seq, unconsumedBucketTotalMsgCount: nextBucket?.totalMsgCount, cutoffTimestamp, @@ -1164,23 +1186,29 @@ export class CheckpointProposalJob implements Traceable { blocksInCheckpoint.push(block); usedTxs.forEach(tx => txHashesAlreadyIncluded.add(tx.txHash.toString())); - // Streaming Inbox: the block built successfully, so advance the consumption cursor and carry this block's - // rolling-hash bucket reference. A block that consumed nothing reuses the parent bucket reference. The bucket - // is re-resolved before the proposal is signed: if an L1 reorg dropped it since selection, the block is built - // on messages the local store no longer holds and must not be signed, stored or gossiped, so the whole - // checkpoint is abandoned here. + // Streaming Inbox: the block built successfully, so advance the consumption cursor to the prefix it consumed + // through and carry that prefix as this block's signed reference. A block that consumed nothing re-signs the + // cursor's existing prefix. The prefix is re-checked by count before the proposal is signed: if an L1 reorg + // changed the messages since selection, the block is built on a prefix the local store no longer holds and + // must not be signed, stored or gossiped, so the whole checkpoint is abandoned here. A repartition that kept + // the same messages passes, since the count is not required to still be a bucket boundary. let blockBucketRef: InboxBucketRef | undefined = undefined; if (streamingState && selection) { if (selection.consume) { - streamingState.parent = { seq: selection.bucket.seq, totalMsgCount: selection.bucket.totalMsgCount }; - streamingState.lastBucketRef = InboxBucketRef.fromBucket(selection.bucket); + streamingState.cursor = { + totalMsgCount: selection.bucket.totalMsgCount, + inboxRollingHash: selection.bucket.inboxRollingHash, + }; } - const consumedBucket = await this.refreshConsumedBucket(streamingState); - if (consumedBucket === undefined) { - this.reportConsumedBucketVanished(streamingState, 'build-failed', { blockNumber, blocksBuilt }); + if (!(await this.isConsumedPrefixStillCanonical(streamingState))) { + this.reportConsumedPrefixVanished(streamingState, 'build-failed', { blockNumber, blocksBuilt }); return { aborted: true }; } - blockBucketRef = streamingState.lastBucketRef; + // The reference is the cursor's prefix hash, which the block's own header count has to name for the pair to + // authenticate anything. Both come from the same fork the builder inserted the selected bundle into, and + // {@link syncProposedBlockToArchiver} re-checks the pair against the archiver's messages before the proposal + // is broadcast, so a builder/selector disagreement aborts the checkpoint there rather than being gossiped. + blockBucketRef = new InboxBucketRef(streamingState.cursor.inboxRollingHash); } // Sign the block proposal. This will throw if HA signing fails. @@ -1200,7 +1228,7 @@ export class CheckpointProposalJob implements Traceable { // so we avoid polluting our archive with a block that would fail. // We wait for the sync to succeed, as this helps catch consistency errors, even if it means we lose some time for block-building. // If this throws, we abort the entire checkpoint. - await this.syncProposedBlockToArchiver(block); + await this.syncProposedBlockToArchiver(block, blockBucketRef); // If this is the last block, do not broadcast it, since it will be included in the checkpoint proposal. if (timingInfo.isLastBlock) { @@ -1255,66 +1283,58 @@ export class CheckpointProposalJob implements Traceable { } /** - * Resolves where a streaming-Inbox checkpoint's consumption starts: the parent checkpoint's last-consumed bucket. - * The parent's cumulative consumed total is the L1-to-L2 message tree leaf count of the fork - * this checkpoint builds on (compact indexing makes leaf count equal cumulative message count), which resolves the - * parent bucket by total. Genesis is the `total = 0` case, resolving the genesis bucket 0. + * Resolves where a streaming-Inbox checkpoint's consumption starts: the parent checkpoint's consumed message + * prefix. The parent's cumulative consumed total is the L1-to-L2 message tree leaf count of the fork this + * checkpoint builds on (compact indexing makes leaf count equal cumulative message count), and the prefix hash at + * that count is what a first block consuming nothing would re-sign. Genesis is the `total = 0` case, whose prefix + * hash is zero. + * + * The count is not required to sit on a current bucket boundary. The parent checkpoint committed to it when it was + * one, and an L1 reorg that re-mines the same messages under different boundaries can have made it interior since; + * the messages are unchanged, so the checkpoint can still be built on top of it. */ private async resolveStreamingCheckpointStart(fork: MerkleTreeWriteOperations): Promise { const parentInfo = await fork.getTreeInfo(MerkleTreeId.L1_TO_L2_MESSAGE_TREE); const parentTotalMsgCount = parentInfo.size; - const parentBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(parentTotalMsgCount); - if (parentBucket === undefined) { + const inboxRollingHash = await this.l1ToL2MessageSource.getInboxRollingHashAt(parentTotalMsgCount); + if (inboxRollingHash === undefined) { throw new Error( - `Streaming inbox: cannot resolve parent Inbox bucket for cumulative total ${parentTotalMsgCount} ` + + `Streaming inbox: cannot resolve the Inbox prefix hash at cumulative total ${parentTotalMsgCount} ` + `(checkpoint ${this.checkpointNumber}); local Inbox view has not synced it`, ); } return { checkpointStartTotalMsgCount: parentTotalMsgCount, - parent: { seq: parentBucket.seq, totalMsgCount: parentBucket.totalMsgCount }, - lastBucketRef: InboxBucketRef.fromBucket(parentBucket), + cursor: { totalMsgCount: parentTotalMsgCount, inboxRollingHash }, }; } /** - * Re-resolves the last consumed bucket by its rolling hash, before the block that consumed it is signed. Returns - * the bucket as the local Inbox view holds it now, or undefined when no bucket carries the hash any more. + * Re-checks that the consumed prefix the cursor points at is still canonical, before the block that consumed it is + * signed. Returns true when the local Inbox view still hashes that count to the same value. * - * Under the message-only rolling hash an L1 reorg that merges buckets leaves the hash of the consumed prefix - * intact but shifts every sequence number after the merge point. Nothing outside this job consumes the sequence - * number — validators resolve the reference by rolling hash, and the L1 `propose` hint is resolved from the header - * by {@link resolveBucketHint} at publish time — but the next block's bundle selection inside this checkpoint - * does, so the cursor is re-pointed at the bucket's current sequence number here. + * The prefix is checked *by count*, not by resolving a bucket. An L1 reorg that re-mines the same messages under + * different bucket boundaries leaves every leaf and every prefix hash intact while moving the boundaries, so the + * block is exactly as valid as it was: requiring the count to still be a boundary here would abandon a slot whose + * blocks are perfectly content-valid. A hash that changed at the same count is the real failure — a reorder or a + * dropped message — and means the block was built on messages the local store no longer holds, so it must not be + * signed, stored or gossiped. The caller aborts the checkpoint in that case. * - * No bucket carrying the hash means a reorder or a dropped message: the consumed prefix no longer ends on a bucket - * boundary, and the block built on it can never be accepted. The caller aborts the checkpoint in that case. + * The final position's boundary requirement is enforced once, at the end of block building, and re-checked by + * {@link resolveBucketHint} at publish time, because only the completed checkpoint's endpoint has to resolve to a + * bucket for L1. */ - private async refreshConsumedBucket(state: StreamingCheckpointState): Promise { - const builtSeq = state.parent.seq; - const { inboxRollingHash } = state.lastBucketRef; - const bucket = await this.l1ToL2MessageSource.getInboxBucketByRollingHash(inboxRollingHash); - if (bucket === undefined) { - return undefined; - } - if (bucket.seq !== builtSeq) { - this.log.info(`Inbox bucket reference re-resolved after L1 reorg`, { - slot: this.targetSlot, - checkpointNumber: this.checkpointNumber, - builtSeq, - resolvedSeq: bucket.seq, - inboxRollingHash: inboxRollingHash.toString(), - }); - state.parent = { seq: bucket.seq, totalMsgCount: bucket.totalMsgCount }; - } - return bucket; + private async isConsumedPrefixStillCanonical(state: StreamingCheckpointState): Promise { + const { totalMsgCount, inboxRollingHash } = state.cursor; + const canonicalHash = await this.l1ToL2MessageSource.getInboxRollingHashAt(totalMsgCount); + return canonicalHash !== undefined && canonicalHash.equals(inboxRollingHash); } /** - * Reports that no local Inbox bucket carries the rolling hash this checkpoint consumed through, so the slot is - * abandoned: the checkpoint event under `failureEvent`, an operator warning and the failure metric. + * Reports that the local Inbox view no longer backs the message prefix this checkpoint consumed through, so the + * slot is abandoned: the checkpoint event under `failureEvent`, an operator warning and the failure metric. */ - private reportConsumedBucketVanished( + private reportConsumedPrefixVanished( state: StreamingCheckpointState, failureEvent: 'build-failed' | 'publish-failed', extraContext: Record = {}, @@ -1323,14 +1343,14 @@ export class CheckpointProposalJob implements Traceable { const context = { slot: this.targetSlot, checkpointNumber: this.checkpointNumber, - builtSeq: state.parent.seq, - inboxRollingHash: state.lastBucketRef.inboxRollingHash.toString(), + builtTotalMsgCount: state.cursor.totalMsgCount, + inboxRollingHash: state.cursor.inboxRollingHash.toString(), reason: 'inbox_bucket_reorged', ...extraContext, }; this.logCheckpointEvent(failureEvent, `Checkpoint ${phase} failed for slot ${this.targetSlot}`, context); this.log.warn( - `No Inbox bucket carries the rolling hash this checkpoint consumed through; abandoning slot ` + + `The local Inbox view no longer backs the message prefix this checkpoint consumed through; abandoning slot ` + `${this.targetSlot} after an L1 reorg reordered or dropped bridged messages`, context, ); @@ -1361,25 +1381,25 @@ export class CheckpointProposalJob implements Traceable { failureEvent: 'build-failed' | 'publish-failed', ): Promise { const phase = failureEvent === 'build-failed' ? 'build' : 'publish'; - const builtSeq = state.parent.seq; const inboxRollingHash = header.inboxRollingHash; const bucket = await this.l1ToL2MessageSource.getInboxBucketByRollingHash(inboxRollingHash); if (bucket === undefined) { - this.reportConsumedBucketVanished(state, failureEvent, { headerInboxRollingHash: inboxRollingHash.toString() }); + this.reportConsumedPrefixVanished(state, failureEvent, { headerInboxRollingHash: inboxRollingHash.toString() }); return undefined; } - if (bucket.seq !== builtSeq) { + if (state.lastResolvedBucketSeq !== undefined && state.lastResolvedBucketSeq !== bucket.seq) { this.log.info(`Inbox bucket hint re-resolved after L1 reorg`, { slot: this.targetSlot, checkpointNumber: this.checkpointNumber, - builtSeq, + previousSeq: state.lastResolvedBucketSeq, publishedSeq: bucket.seq, + consumedTotalMsgCount: bucket.totalMsgCount, inboxRollingHash: inboxRollingHash.toString(), }); } + state.lastResolvedBucketSeq = bucket.seq; - const resolved = { ...state, parent: { seq: bucket.seq, totalMsgCount: bucket.totalMsgCount } }; - const { sufficient, nextBucket, cutoffTimestamp } = await this.checkCheckpointConsumption(resolved); + const { sufficient, nextBucket, cutoffTimestamp } = await this.checkCheckpointConsumption(state, bucket); if (!sufficient) { const context = { slot: this.targetSlot, @@ -1411,13 +1431,16 @@ export class CheckpointProposalJob implements Traceable { * absent, past the cutoff, or a cap-escape. The bucket and cutoff the verdict was taken against are returned for * diagnostics. * - * Derived from the consumption cursor rather than from the per-block selections that advanced it, so a selector that - * stops short of a mandatory bucket is caught here regardless of how it reported the shortfall. + * `finalBucket` is the bucket {@link resolveFinalConsumedBucket} matched the consumed position against; + * identifying "the first bucket left unconsumed" needs it, since that is the sequence after `finalBucket`'s. The + * per-checkpoint cap origin still comes from the cursor's fixed start count, so a selector that stopped short of a + * mandatory bucket is caught here regardless of how it reported the shortfall. */ private async checkCheckpointConsumption( state: StreamingCheckpointState, + finalBucket: InboxBucket, ): Promise<{ sufficient: boolean; nextBucket: InboxBucket | undefined; cutoffTimestamp: bigint }> { - const nextBucket = await this.l1ToL2MessageSource.getInboxBucket(state.parent.seq + 1n); + const nextBucket = await this.l1ToL2MessageSource.getInboxBucket(finalBucket.seq + 1n); const cutoffTimestamp = getInboxCutoffTimestamp(this.targetSlot, this.l1Constants); const sufficient = isInboxConsumptionSufficient({ nextBucket, @@ -1428,6 +1451,28 @@ export class CheckpointProposalJob implements Traceable { return { sufficient, nextBucket, cutoffTimestamp }; } + /** + * Resolves the current Inbox bucket the checkpoint's consumed position ends on, or undefined when that position is + * not a current boundary carrying the consumed prefix hash. + * + * This is the one place the boundary rule still applies. L1 reads the checkpoint header's rolling hash out of the + * bucket the `bucketHint` names, so a final position no current bucket ends on — or one whose bucket carries a + * different hash — is one `propose` would revert on, however content-valid the individual blocks are. Checked + * before anything is gossiped so the slot is abandoned rather than spent on a checkpoint nobody can publish. + * + * Both halves of the cursor are proven, not just the count: the bucket ending at that count is unique, so + * comparing its rolling hash to the cursor's is what rules out a bucket that ends there over *different* messages + * (a reorg that replaced the tail while keeping the count). Matching both is exactly the condition + * `ProposeLib.validateInboxConsumption` checks against the hint. + */ + private async resolveFinalConsumedBucket(state: StreamingCheckpointState): Promise { + const bucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(state.cursor.totalMsgCount); + if (bucket === undefined || !bucket.inboxRollingHash.equals(state.cursor.inboxRollingHash)) { + return undefined; + } + return bucket; + } + /** * Selects this block's streaming-Inbox message bundle against the current consumption cursor, mirroring the L1 * predicate in `ProposeLib.validateInboxConsumption`. Does not mutate the cursor; the caller @@ -1444,7 +1489,7 @@ export class CheckpointProposalJob implements Traceable { now: BigInt(Math.floor(nowSeconds)), isEligible: this.isInboxBucketEligible, ethereumSlotDuration: this.l1Constants.ethereumSlotDuration, - parent: state.parent, + cursor: state.cursor, checkpointStartTotalMsgCount: state.checkpointStartTotalMsgCount, perBlockCap: MAX_L1_TO_L2_MSGS_PER_BLOCK, perCheckpointCap: MAX_L1_TO_L2_MSGS_PER_CHECKPOINT, @@ -2017,7 +2062,7 @@ export class CheckpointProposalJob implements Traceable { * and fee analysis only, and pushing them to the archiver causes spurious reorg cascades * whenever the real proposer's block arrives from L1. */ - private async syncProposedBlockToArchiver(block: L2Block): Promise { + private async syncProposedBlockToArchiver(block: L2Block, inboxPrefixRef: InboxBucketRef | undefined): Promise { if (this.config.skipPushProposedBlocksToArchiver || this.config.fishermanMode) { this.log.warn(`Skipping push of proposed block ${block.number} to archiver`, { blockNumber: block.number, @@ -2025,11 +2070,17 @@ export class CheckpointProposalJob implements Traceable { }); return; } + // The archiver re-validates this reference against its own messages inside the insert transaction, which is what + // stops a block built before an L1 reorg from landing after the reorg pruned the chain it belongs to. Streaming + // block building always produces one, so a missing reference here is a wiring bug, not a compatibility case. + if (inboxPrefixRef === undefined) { + throw new Error(`Streaming inbox: proposed block ${block.number} has no signed Inbox prefix reference`); + } this.log.debug(`Syncing proposed block ${block.number} to archiver`, { blockNumber: block.number, slot: block.header.globalVariables.slotNumber, }); - await this.blockSink.addBlock(block); + await this.blockSink.addBlock(block, inboxPrefixRef); } /** diff --git a/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.test.ts b/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.test.ts index 7240fe4ebdf0..0c18dd1f7069 100644 --- a/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.test.ts @@ -51,31 +51,42 @@ function makeSource(specs: TestBucketSpec[]): { const ordered = [...buckets.values()].sort((a, b) => Number(a.seq - b.seq)); const source: InboxBucketSource = { getInboxBucket: (seq: bigint) => Promise.resolve(buckets.get(seq)), + getInboxBucketByTotalMsgCount: (totalMsgCount: bigint) => + Promise.resolve( + totalMsgCount === 0n ? GENESIS_BUCKET : (ordered.find(b => b.totalMsgCount === totalMsgCount) ?? undefined), + ), getLatestInboxBucketAtOrBefore: (timestamp: bigint) => { const eligible = ordered.filter(b => b.timestamp <= timestamp); return Promise.resolve(eligible.length === 0 ? undefined : eligible[eligible.length - 1]); }, - getL1ToL2MessagesBetweenBuckets: (fromExclusive: bigint, toInclusive: bigint) => { - const toBucket = buckets.get(toInclusive); - if (toBucket === undefined) { - return Promise.resolve([]); - } - let startIndex = 0n; - if (fromExclusive > 0n) { - const fromBucket = buckets.get(fromExclusive); - if (fromBucket === undefined) { - return Promise.resolve([]); - } - startIndex = fromBucket.lastMessageIndex + 1n; - } - return Promise.resolve(leaves.slice(Number(startIndex), Number(toBucket.lastMessageIndex + 1n))); - }, + // The count range addresses the leaf log directly, exactly as the archiver's does, so a cursor interior to a + // current bucket still slices the right suffix. + getL1ToL2MessagesBetweenLeafCounts: (startLeafCount: bigint, endLeafCount: bigint) => + Promise.resolve(leaves.slice(Number(startLeafCount), Number(endLeafCount))), }; return { source, buckets, leaves }; } -const GENESIS_PARENT = { seq: 0n, totalMsgCount: 0n }; +/** The genesis sentinel bucket, which every partition ends the "consumed nothing" position on. */ +const GENESIS_BUCKET: InboxBucket = { + seq: 0n, + inboxRollingHash: Fr.ZERO, + totalMsgCount: 0n, + timestamp: 0n, + msgCount: 0, + lastMessageIndex: 0n, + l1BlockNumber: 0n, + l1BlockHash: Buffer32.ZERO, +}; + +const GENESIS_CURSOR = { totalMsgCount: 0n, inboxRollingHash: Fr.ZERO }; + +/** The cursor a block inherits after consuming through a bucket: that bucket's count and rolling hash. */ +const cursorAfter = (bucket: InboxBucket) => ({ + totalMsgCount: bucket.totalMsgCount, + inboxRollingHash: bucket.inboxRollingHash, +}); // Pinned cross-layer values shared with the L1 Foundry harness: genesisTime=100000, slotDuration=36, // ethereumSlotDuration=12. @@ -118,7 +129,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 1_000_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result.consume).toBe(false); }); @@ -134,7 +145,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 250n + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result).toMatchObject({ consume: true }); if (result.consume) { @@ -155,7 +166,7 @@ describe('selectInboxBucketForBlock', () => { isEligible, messageSource: source, now: 1_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result).toMatchObject({ consume: true }); if (result.consume) { @@ -176,7 +187,7 @@ describe('selectInboxBucketForBlock', () => { isEligible: () => Promise.resolve(false), messageSource: source, now: 1_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result.consume).toBe(false); }); @@ -194,7 +205,7 @@ describe('selectInboxBucketForBlock', () => { isEligible, messageSource: source, now: 1_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result).toMatchObject({ consume: true }); if (result.consume) { @@ -215,7 +226,7 @@ describe('selectInboxBucketForBlock', () => { isEligible, messageSource: source, now: 1_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result.consume).toBe(false); expect(isEligible.asked).toEqual([12n, 11n, 10n, 9n, 8n, 7n, 6n, 5n, 4n]); @@ -253,7 +264,7 @@ describe('selectInboxBucketForBlock', () => { isEligible: tracker.isEligible, messageSource: source, now: 214n, // block 100 could have a child by now, but none is visible; block 99's child is block 100 - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result).toMatchObject({ consume: true }); @@ -274,7 +285,7 @@ describe('selectInboxBucketForBlock', () => { isEligible: immediateEligibility, messageSource: source, now: 1_000n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(result).toMatchObject({ consume: true }); if (result.consume) { @@ -293,7 +304,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 300n + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, perBlockCap: 400, }); expect(result).toMatchObject({ consume: true }); @@ -314,7 +325,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 200n + ETHEREUM_SLOT_DURATION, - parent: { seq: 1n, totalMsgCount: buckets.get(1n)!.totalMsgCount }, + cursor: cursorAfter(buckets.get(1n)!), checkpointStartTotalMsgCount: 0n, perCheckpointCap: 1000, }); @@ -331,7 +342,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 150n + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, }); expect(first).toMatchObject({ consume: true }); if (!first.consume) { @@ -339,22 +350,97 @@ describe('selectInboxBucketForBlock', () => { } expect(first.bucket.seq).toBe(1n); - // Block 2, later sub-slot (now=312): bucket 2 is now confirmed too; parent is block 1's bucket. + // Block 2, later sub-slot (now=312): bucket 2 is now confirmed too; the cursor is at block 1's position. const second = await selectInboxBucketForBlock({ ...baseInput, messageSource: source, now: 300n + ETHEREUM_SLOT_DURATION, - parent: { seq: first.bucket.seq, totalMsgCount: first.bucket.totalMsgCount }, + cursor: cursorAfter(first.bucket), checkpointStartTotalMsgCount: 0n, }); expect(second).toMatchObject({ consume: true }); if (second.consume) { expect(second.bucket.seq).toBe(2n); expect(second.bundle).toHaveLength(3); // only bucket 2's messages, not bucket 1's - expect(second.bundle).toEqual(await source.getL1ToL2MessagesBetweenBuckets(1n, 2n)); + expect(second.bundle).toEqual(await source.getL1ToL2MessagesBetweenLeafCounts(2n, 5n)); } }); + describe('a cursor left interior by a pure repartition', () => { + // `B1=[m1,m2], B2=[m3]` becoming `B1'=[m1], B2'=[m2,m3]`: the same three messages under moved boundaries. A + // cursor at count 2 was B1's boundary and is now inside B2'. + const repartitioned = () => + makeSource([ + { seq: 1n, timestamp: 100n, msgCount: 1 }, + { seq: 2n, timestamp: 200n, msgCount: 2 }, + ]); + const interiorCursor = { totalMsgCount: 2n, inboxRollingHash: new Fr(0xb1) }; + + it('consumes the suffix of the bucket containing it, exactly through the next boundary', async () => { + const { source, leaves } = repartitioned(); + const result = await selectInboxBucketForBlock({ + ...baseInput, + messageSource: source, + now: 200n + ETHEREUM_SLOT_DURATION, + cursor: interiorCursor, + checkpointStartTotalMsgCount: 2n, + }); + expect(result).toMatchObject({ consume: true }); + if (result.consume) { + // B2' has a strictly greater total than the cursor even though it is the bucket the cursor sits inside, so it + // is selected and only its unconsumed suffix is bundled. + expect(result.bucket.seq).toBe(2n); + expect(result.bucket.totalMsgCount).toBe(3n); + expect(result.bundle).toEqual([leaves[2]]); + } + }); + + it('measures the per-block cap from the cursor count, not from the containing bucket start', async () => { + const { source } = repartitioned(); + const result = await selectInboxBucketForBlock({ + ...baseInput, + messageSource: source, + now: 200n + ETHEREUM_SLOT_DURATION, + cursor: interiorCursor, + checkpointStartTotalMsgCount: 2n, + perBlockCap: 1, // the suffix is one message, so it still fits + }); + expect(result).toMatchObject({ consume: true }); + }); + + it('fails closed on the last block when nothing past it can be consumed', async () => { + // Nothing is eligible past the cursor, so the block consumes nothing and the checkpoint would end on a count no + // current bucket carries. That is unpublishable on L1, and must not be reported as "consumed everything". + const { source } = repartitioned(); + const result = await selectInboxBucketForBlock({ + ...baseInput, + messageSource: source, + now: 100n, // bucket 2 (opened at 200) is not yet visible, let alone eligible + cursor: interiorCursor, + checkpointStartTotalMsgCount: 2n, + isLastBlock: true, + cutoffTimestamp: 0n, + }); + expect(result.consume).toBe(false); + expect(result.insufficientFinalBlockCapacity).toBe(true); + }); + + it('does not fail closed when the last block restores a boundary', async () => { + const { source } = repartitioned(); + const result = await selectInboxBucketForBlock({ + ...baseInput, + messageSource: source, + now: 200n + ETHEREUM_SLOT_DURATION, + cursor: interiorCursor, + checkpointStartTotalMsgCount: 2n, + isLastBlock: true, + cutoffTimestamp: 0n, + }); + expect(result).toMatchObject({ consume: true }); + expect(result.insufficientFinalBlockCapacity).toBeUndefined(); + }); + }); + it('applies the cutoff as a consumption floor on the last block', async () => { // Bucket sits at the cutoff for slot 10 but is not yet confirmed, so a non-last block skips it. const cutoff = cutoffForSlot(10n); @@ -364,7 +450,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: cutoff + ETHEREUM_SLOT_DURATION - 1n, // bucket's opening block has no descendant yet - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, isLastBlock: false, cutoffTimestamp: cutoff, }); @@ -374,7 +460,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: cutoff + ETHEREUM_SLOT_DURATION - 1n, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, isLastBlock: true, cutoffTimestamp: cutoff, }); @@ -394,7 +480,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: atCutoff.source, now: cutoff, // before eligibility would admit it, forcing reliance on the cutoff floor - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, isLastBlock: true, cutoffTimestamp: cutoff, }); @@ -404,7 +490,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: pastCutoff.source, now: cutoff, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, isLastBlock: true, cutoffTimestamp: cutoff, }); @@ -425,7 +511,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: cutoff + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, perBlockCap: 256, perCheckpointCap: 1024, isLastBlock: true, @@ -451,7 +537,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: cutoff + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, perBlockCap: 256, perCheckpointCap: 1024, isLastBlock: false, @@ -471,15 +557,16 @@ describe('selectInboxBucketForBlock', () => { counts.map((msgCount, i) => ({ seq: BigInt(i + 1), timestamp: cutoff - BigInt(counts.length - i), msgCount })), ); const caps = { perBlockCap: 256, perCheckpointCap: 1024 }; - const sufficiencyAt = async (parent: { seq: bigint; totalMsgCount: bigint }) => + const sufficiencyAfterBucket = async (seq: bigint) => isInboxConsumptionSufficient({ - nextBucket: await source.getInboxBucket(parent.seq + 1n), + nextBucket: await source.getInboxBucket(seq + 1n), cutoffTimestamp: cutoff, checkpointStartTotalMsgCount: 0n, perCheckpointCap: caps.perCheckpointCap, }); - let parent = GENESIS_PARENT; + let cursor = GENESIS_CURSOR; + let lastConsumedSeq = 0n; for (let block = 1; block <= MIN_BLOCKS_FOR_INBOX_CATCHUP; block++) { const isLastBlock = block === MIN_BLOCKS_FOR_INBOX_CATCHUP; const result = await selectInboxBucketForBlock({ @@ -487,7 +574,7 @@ describe('selectInboxBucketForBlock', () => { ...caps, messageSource: source, now: cutoff + ETHEREUM_SLOT_DURATION, - parent, + cursor, isLastBlock, cutoffTimestamp: cutoff, }); @@ -498,13 +585,14 @@ describe('selectInboxBucketForBlock', () => { // Each block advances by exactly one bucket, which is what makes the bound tight. expect(result.bucket.seq).toBe(BigInt(block)); expect(result.insufficientFinalBlockCapacity).toBeUndefined(); - parent = { seq: result.bucket.seq, totalMsgCount: result.bucket.totalMsgCount }; + cursor = cursorAfter(result.bucket); + lastConsumedSeq = result.bucket.seq; } - expect(parent.totalMsgCount).toBe(772n); - expect(await sufficiencyAt(parent)).toBe(true); + expect(cursor.totalMsgCount).toBe(772n); + expect(await sufficiencyAfterBucket(lastConsumedSeq)).toBe(true); // One block short of the bound the backlog is still mandatory, so the floor really needs all of them. - expect(await sufficiencyAt({ seq: 6n, totalMsgCount: 771n })).toBe(false); + expect(await sufficiencyAfterBucket(lastConsumedSeq - 1n)).toBe(false); }); it('makes a bucket opened at the cutoff confirmable within the build frame', async () => { @@ -537,7 +625,7 @@ describe('selectInboxBucketForBlock', () => { isEligible: tracker.isEligible, messageSource: source, now, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, isLastBlock: false, cutoffTimestamp: cutoff, }); @@ -553,7 +641,7 @@ describe('selectInboxBucketForBlock', () => { ...baseInput, messageSource: source, now: 100n + ETHEREUM_SLOT_DURATION, - parent: GENESIS_PARENT, + cursor: GENESIS_CURSOR, perBlockCap: 4096, perCheckpointCap: 1024, }); diff --git a/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.ts b/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.ts index e05834860cba..d8e77f459156 100644 --- a/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.ts +++ b/yarn-project/sequencer-client/src/sequencer/inbox_bucket_selector.ts @@ -14,19 +14,32 @@ const MAX_ELIGIBILITY_WALK_L1_BLOCKS = 8; const log: Logger = createLogger('sequencer:inbox-bucket-selector'); -/** The subset of the archiver's Inbox-bucket queries the selector needs. */ +/** The subset of the archiver's Inbox queries the selector needs. */ export type InboxBucketSource = Pick< L1ToL2MessageSource, - 'getInboxBucket' | 'getLatestInboxBucketAtOrBefore' | 'getL1ToL2MessagesBetweenBuckets' + | 'getInboxBucket' + | 'getInboxBucketByTotalMsgCount' + | 'getLatestInboxBucketAtOrBefore' + | 'getL1ToL2MessagesBetweenLeafCounts' >; /** - * The last-consumed Inbox bucket a block streams from. Only the sequence number and cumulative message count are - * needed: the sequence number bounds the derived bundle, and the count is the per-block/per-checkpoint cap origin. - * At a checkpoint's first block this is the parent checkpoint's last-consumed bucket; the genesis base case is - * `{ seq: 0, totalMsgCount: 0 }` (bundles derive from the start of the Inbox). + * The Inbox message prefix a block streams from: the cumulative count consumed so far and the rolling hash of that + * prefix. At a checkpoint's first block this is the parent checkpoint's consumed position; the genesis base case is + * `{ totalMsgCount: 0, inboxRollingHash: Fr.ZERO }` (bundles derive from the start of the Inbox). + * + * The cursor is deliberately a count and a hash rather than a bucket. An L1 reorg that re-mines the same messages + * under different bucket boundaries can leave the cursor's count interior to a current bucket, and every comparison + * the selector makes is against the count, so the cursor keeps working: the containing bucket has a strictly greater + * cumulative total, so it stays a valid target whose suffix the next block consumes. A cursor carrying a sequence + * number would instead compare equal to that bucket and could never advance past it. */ -export type ConsumedBucketCursor = Pick; +export type ConsumedMessagePrefixCursor = { + /** Cumulative Inbox message count consumed so far; the lower bound of the next bundle. */ + totalMsgCount: bigint; + /** Consensus rolling hash of that prefix; what a block consuming nothing re-signs as its own reference. */ + inboxRollingHash: Fr; +}; /** Inputs to a single block's streaming Inbox-bucket selection. */ export type SelectInboxBucketInput = { @@ -47,8 +60,8 @@ export type SelectInboxBucketInput = { * for a descendant to appear. */ ethereumSlotDuration: number; - /** The last bucket consumed by this checkpoint so far (parent checkpoint's at the first block). */ - parent: ConsumedBucketCursor; + /** The message prefix consumed by this checkpoint so far (the parent checkpoint's at the first block). */ + cursor: ConsumedMessagePrefixCursor; /** Cumulative Inbox message count consumed as of the parent checkpoint; the per-checkpoint cap origin. */ checkpointStartTotalMsgCount: bigint; /** Maximum number of messages this block may consume. */ @@ -75,7 +88,7 @@ type InboxBucketConsumption = bundle: Fr[]; } | { - /** The block consumes nothing; it reuses the parent bucket reference. */ + /** The block consumes nothing; it reuses the cursor's prefix reference. */ consume: false; }; @@ -101,10 +114,10 @@ export type InboxBucketSelection = InboxBucketConsumption & { * the checkpoint's last block, also consider the cutoff bucket (newest opened at or before `cutoffTimestamp`) and * take whichever is newer, so the checkpoint reaches the censorship floor even when eligibility preferred less: a * mandatory bucket is consumed whether or not it is confirmed. - * 2. If nothing is newer than the parent bucket, consume nothing. + * 2. If nothing reaches past the cursor's count, consume nothing. * 3. Otherwise walk back from the candidate to the newest bucket whose consumption fits both the per-block cap - * (`bucket.totalMsgCount - parent.totalMsgCount`) and the per-checkpoint cap - * (`bucket.totalMsgCount - checkpointStartTotalMsgCount`). If even the first bucket past the parent overshoots the + * (`bucket.totalMsgCount - cursor.totalMsgCount`) and the per-checkpoint cap + * (`bucket.totalMsgCount - checkpointStartTotalMsgCount`). If even the first bucket past the cursor overshoots the * per-checkpoint cap, consume nothing — the L1 cap-escape (`ProposeLib` allows leaving a bucket unconsumed when * consuming through it would exceed the per-checkpoint cap). * 4. On the last block only, check the resulting position against the censorship floor with the shared @@ -112,6 +125,12 @@ export type InboxBucketSelection = InboxBucketConsumption & { * onto a prefix that still leaves a mandatory bucket behind; that is reported as * `insufficientFinalBlockCapacity` rather than passed off as a usable selection. * + * Every comparison against the cursor is on cumulative message counts, never on bucket sequence numbers. An L1 reorg + * that re-partitions the same messages can leave the cursor interior to a current bucket, and that bucket's total is + * then strictly greater than the cursor's, so it is selected and the block consumes its suffix — which also puts the + * checkpoint back on a boundary. A sequence-number comparison would treat the containing bucket as already consumed + * and strand the cursor forever. + * * The `<=` comparison on the cutoff makes a bucket exactly at the cutoff mandatory, matching the strict `>` "past * cutoff" test on L1 (`next.timestamp > cutoff` leaves it optional). * @@ -124,9 +143,27 @@ export async function selectInboxBucketForBlock(input: SelectInboxBucketInput): return consumption; } - const { messageSource, parent, checkpointStartTotalMsgCount, perCheckpointCap, cutoffTimestamp } = input; - const finalSeq = consumption.consume ? consumption.bucket.seq : parent.seq; - const nextBucket = await messageSource.getInboxBucket(finalSeq + 1n); + const { messageSource, cursor, checkpointStartTotalMsgCount, perCheckpointCap, cutoffTimestamp } = input; + + // The first bucket left unconsumed is the one after the bucket the checkpoint ends on, so the final position has to + // name a current bucket to identify it. When the block consumes nothing the position is the cursor's, which an L1 + // repartition may have left interior: no current bucket ends there, the checkpoint could not be published against + // this partition anyway (L1 reads the header's rolling hash out of the `bucketHint` bucket), and passing + // `undefined` on would assert the opposite of the truth, since to `isInboxConsumptionSufficient` a missing next + // bucket means everything has been consumed. Fail closed instead. + const finalBucket = consumption.consume + ? consumption.bucket + : await messageSource.getInboxBucketByTotalMsgCount(cursor.totalMsgCount); + if (finalBucket === undefined) { + log.warn( + `Consumed Inbox prefix at count ${cursor.totalMsgCount} is interior to a current bucket; no checkpoint can ` + + `end on this block`, + { cursorTotalMsgCount: cursor.totalMsgCount, inboxRollingHash: cursor.inboxRollingHash.toString() }, + ); + return { ...consumption, insufficientFinalBlockCapacity: true }; + } + + const nextBucket = await messageSource.getInboxBucket(finalBucket.seq + 1n); const sufficient = isInboxConsumptionSufficient({ nextBucket, cutoffTimestamp, @@ -140,7 +177,7 @@ export async function selectInboxBucketForBlock(input: SelectInboxBucketInput): async function selectConsumption(input: SelectInboxBucketInput): Promise { const { messageSource, - parent, + cursor, checkpointStartTotalMsgCount, perBlockCap, perCheckpointCap, @@ -157,7 +194,7 @@ async function selectConsumption(input: SelectInboxBucketInput): Promise parent.seq) { - const blockCount = selected.totalMsgCount - parent.totalMsgCount; + while (selected !== undefined && selected.totalMsgCount > cursor.totalMsgCount) { + const blockCount = selected.totalMsgCount - cursor.totalMsgCount; const checkpointCount = selected.totalMsgCount - checkpointStartTotalMsgCount; if (blockCount <= perBlockCapBig && checkpointCount <= perCheckpointCapBig) { - const bundle = await messageSource.getL1ToL2MessagesBetweenBuckets(parent.seq, selected.seq); + const bundle = await messageSource.getL1ToL2MessagesBetweenLeafCounts( + cursor.totalMsgCount, + selected.totalMsgCount, + ); return { consume: true, bucket: selected, bundle }; } - if (selected.seq - 1n <= parent.seq) { + if (selected.seq === 0n) { break; } selected = await messageSource.getInboxBucket(selected.seq - 1n); @@ -192,24 +232,32 @@ const openingL1Block = (bucket: InboxBucket) => `${bucket.l1BlockNumber}:${bucke * ({@link selectSettledBucket}) rather than consuming nothing. */ async function selectEligibleBucket(input: SelectInboxBucketInput): Promise { - const { messageSource, now, isEligible, parent } = input; + const { messageSource, now, isEligible, cursor } = input; const walkedL1Blocks = new Set(); let candidate = await messageSource.getLatestInboxBucketAtOrBefore(now); - while (candidate !== undefined && candidate.seq > parent.seq) { + while (candidate !== undefined && candidate.totalMsgCount > cursor.totalMsgCount) { walkedL1Blocks.add(openingL1Block(candidate)); if (walkedL1Blocks.size > MAX_ELIGIBILITY_WALK_L1_BLOCKS) { const settled = await selectSettledBucket(input); log.warn( `No eligible Inbox bucket within ${MAX_ELIGIBILITY_WALK_L1_BLOCKS} L1 blocks of the head bucket; ` + (settled === undefined ? 'consuming nothing' : `falling back to bucket ${settled.seq}`), - { headBucketSeq: candidate.seq, parentBucketSeq: parent.seq, fallbackBucketSeq: settled?.seq, now }, + { + headBucketSeq: candidate.seq, + cursorTotalMsgCount: cursor.totalMsgCount, + fallbackBucketSeq: settled?.seq, + now, + }, ); return settled; } if (await isEligible(candidate, now)) { return candidate; } + if (candidate.seq === 0n) { + break; + } candidate = await messageSource.getInboxBucket(candidate.seq - 1n); } return candidate; @@ -222,11 +270,11 @@ async function selectEligibleBucket(input: SelectInboxBucketInput): Promise { - const { messageSource, now, isEligible, parent, ethereumSlotDuration } = input; + const { messageSource, now, isEligible, cursor, ethereumSlotDuration } = input; const settledBy = now - 2n * BigInt(ethereumSlotDuration); const settled = await messageSource.getLatestInboxBucketAtOrBefore(settledBy); - if (settled === undefined || settled.seq <= parent.seq) { + if (settled === undefined || settled.totalMsgCount <= cursor.totalMsgCount) { return undefined; } return (await isEligible(settled, now)) ? settled : undefined; diff --git a/yarn-project/sequencer-client/src/test/utils.ts b/yarn-project/sequencer-client/src/test/utils.ts index b22f3fbf0a1e..e7ca4f04acec 100644 --- a/yarn-project/sequencer-client/src/test/utils.ts +++ b/yarn-project/sequencer-client/src/test/utils.ts @@ -238,18 +238,32 @@ export const GENESIS_INBOX_BUCKET: InboxBucket = { /** * Wires the Inbox lookups a checkpoint proposal job makes outside bundle selection: the consumption cursor's start - * (resolved from the fork's L1-to-L2 leaf count, which defaults to an empty tree) and the L1 propose bucket hint + * (the prefix hash at the fork's L1-to-L2 leaf count, which defaults to an empty tree) and the L1 propose bucket hint * (re-resolved from the rolling hash the checkpoint header commits to). Buckets are served on top of the genesis * sentinel, so a test that seeds none gets a checkpoint that consumes nothing. + * + * `interiorPrefixes` registers prefix hashes at counts no seeded bucket ends on, standing in for a boundary an L1 + * reorg merged away: the count stays authenticatable even though no bucket carries the hash any more. */ -export function mockInboxBuckets(source: MockProxy, buckets: InboxBucket[] = []): void { +export function mockInboxBuckets( + source: MockProxy, + buckets: InboxBucket[] = [], + interiorPrefixes: { totalMsgCount: bigint; inboxRollingHash: Fr }[] = [], +): void { const all = [GENESIS_INBOX_BUCKET, ...buckets]; + const prefixes = new Map([ + ...buckets.map(bucket => [bucket.totalMsgCount, bucket.inboxRollingHash] as const), + ...interiorPrefixes.map(prefix => [prefix.totalMsgCount, prefix.inboxRollingHash] as const), + ]); source.getInboxBucketByTotalMsgCount.mockImplementation(totalMsgCount => Promise.resolve(all.find(bucket => bucket.totalMsgCount === totalMsgCount)), ); source.getInboxBucketByRollingHash.mockImplementation(inboxRollingHash => Promise.resolve(all.find(bucket => bucket.inboxRollingHash.equals(inboxRollingHash))), ); + source.getInboxRollingHashAt.mockImplementation(totalMsgCount => + Promise.resolve(totalMsgCount === 0n ? Fr.ZERO : prefixes.get(totalMsgCount)), + ); } /** diff --git a/yarn-project/stdlib/src/block/l2_block_source.ts b/yarn-project/stdlib/src/block/l2_block_source.ts index f665404094f6..d2d4ac297cf5 100644 --- a/yarn-project/stdlib/src/block/l2_block_source.ts +++ b/yarn-project/stdlib/src/block/l2_block_source.ts @@ -20,6 +20,7 @@ import type { CheckpointInfo } from '../checkpoint/checkpoint_info.js'; import type { PublishedCheckpoint } from '../checkpoint/published_checkpoint.js'; import type { L1RollupConstants } from '../epoch-helpers/index.js'; import { MAX_RPC_CHECKPOINTS_DATA_LEN } from '../interfaces/api_limit.js'; +import type { InboxBucketRef } from '../messaging/inbox_bucket.js'; import type { L2ToL1MembershipWitness } from '../messaging/l2_to_l1_membership.js'; import { CheckpointHeader } from '../rollup/checkpoint_header.js'; import type { IndexedTxEffect } from '../tx/indexed_tx_effect.js'; @@ -313,11 +314,22 @@ export interface L2BlockSource { */ export interface L2BlockSink { /** - * Adds a block to the store. + * Adds a proposed block to the store. + * + * Every path that inserts a block originating from a signed proposal — the checkpoint proposer, automine, and a + * validator's re-execution — must pass `inboxPrefixRef`, the proposal's signed Inbox prefix reference. The sink + * validates it against its own canonical messages in the same transaction as the writes, which is what stops a + * block built before an L1 reorg from landing after the reorg already pruned the chain it belongs to. + * + * Omitting it is a trusted compatibility path, not validation: the block is inserted with the reference check + * skipped. Only callers that synthesize blocks without matching Inbox state (fixtures, tools) may do so. + * * @param block - The L2 block to add. - * @throws If block number is not incremental (i.e., not exactly one more than the last stored block). + * @param inboxPrefixRef - The signed Inbox prefix reference from the block's proposal, if it came from one. + * @throws If block number is not incremental (i.e., not exactly one more than the last stored block), or if a + * supplied prefix reference does not match the sink's canonical messages at the block's L1-to-L2 leaf count. */ - addBlock(block: L2Block): Promise; + addBlock(block: L2Block, inboxPrefixRef?: InboxBucketRef): Promise; } /** diff --git a/yarn-project/stdlib/src/interfaces/archiver.test.ts b/yarn-project/stdlib/src/interfaces/archiver.test.ts index 71cc2c88a1e1..76eedf25e372 100644 --- a/yarn-project/stdlib/src/interfaces/archiver.test.ts +++ b/yarn-project/stdlib/src/interfaces/archiver.test.ts @@ -238,6 +238,11 @@ describe('ArchiverApiSchema', () => { expect(result).toMatchObject({ seq: 4n, inboxRollingHash }); }); + it('getInboxRollingHashAt', async () => { + expect(await context.client.getInboxRollingHashAt(0n)).toEqual(Fr.ZERO); + expect(await context.client.getInboxRollingHashAt(3n)).toEqual(new Fr(3n)); + }); + it('getL1ToL2MessagesBetweenBuckets', async () => { const result = await context.client.getL1ToL2MessagesBetweenBuckets(0n, 3n); expect(result).toEqual([expect.any(Fr)]); @@ -659,6 +664,10 @@ class MockArchiver implements ArchiverApi { l1BlockHash: Buffer32.fromBigInt(20n), }); } + getInboxRollingHashAt(totalMsgCount: bigint): Promise { + expect(typeof totalMsgCount).toEqual('bigint'); + return Promise.resolve(totalMsgCount === 0n ? Fr.ZERO : new Fr(totalMsgCount)); + } getL1ToL2MessagesBetweenBuckets(fromExclusive: bigint, toInclusive: bigint): Promise { expect(typeof fromExclusive).toEqual('bigint'); expect(typeof toInclusive).toEqual('bigint'); diff --git a/yarn-project/stdlib/src/interfaces/archiver.ts b/yarn-project/stdlib/src/interfaces/archiver.ts index 06c6f8fb4dfc..25c3b4576132 100644 --- a/yarn-project/stdlib/src/interfaces/archiver.ts +++ b/yarn-project/stdlib/src/interfaces/archiver.ts @@ -150,6 +150,7 @@ export const ArchiverApiSchema: ApiSchemaFor = { input: z.tuple([schemas.Fr]), output: InboxBucketSchema.optional(), }), + getInboxRollingHashAt: z.function({ input: z.tuple([schemas.BigInt]), output: schemas.Fr.optional() }), getL1ToL2MessagesBetweenBuckets: z.function({ input: z.tuple([schemas.BigInt, schemas.BigInt]), output: z.array(schemas.Fr), diff --git a/yarn-project/stdlib/src/messaging/inbox_bucket.ts b/yarn-project/stdlib/src/messaging/inbox_bucket.ts index 1c116adf87c0..64c10be3cf80 100644 --- a/yarn-project/stdlib/src/messaging/inbox_bucket.ts +++ b/yarn-project/stdlib/src/messaging/inbox_bucket.ts @@ -55,17 +55,33 @@ export const InboxBucketSchema = z.object({ }) satisfies z.ZodType; /** - * Content-addressed reference to a settled Inbox rolling-hash bucket, carried alongside a block proposal so a - * validator can look the bucket up in its own Inbox view and derive the consumed-message bundle itself, rather than - * trusting a proposer-supplied message list. The rolling hash commits to every message the Inbox absorbed up to and - * including the bucket, so it identifies the bucket by content and survives an L1 reorg that only re-times or - * renumbers buckets; the sequence number and timestamp are read from the locally resolved bucket, never from the - * wire. A wrong reference can only cause a lookup miss, or select a bundle that re-execution of the block then - * rejects; the checkpoint header's `inboxRollingHash` remains the signed consensus commitment. + * Content-addressed reference to the ordered Inbox message prefix a block consumed through, carried alongside a + * block proposal so a validator can confirm the prefix against its own Inbox view and read the consumed-message + * bundle itself, rather than trusting a proposer-supplied message list. + * + * The reference is only half of the pair that authenticates a block: it must be interpreted together with the + * *count* the block's signed header commits to in `state.l1ToL2MessageTree.nextAvailableLeafIndex`. Because every + * message's rolling hash chains the one before it, a hash matching at that count proves the proposer consumed + * exactly the leaves the validator holds, which is what makes the range between the parent's count and this one + * canonical by content. + * + * The referenced position need **not** be a boundary of the bucket partition a node currently holds. An L1 reorg + * that re-mines the same messages under different bucket boundaries leaves every leaf and every prefix hash intact + * while moving the boundaries, so a block that ended on a boundary the reorg merged away stays exactly as valid as + * when it was signed. Only a completed checkpoint's final position must still resolve to a current bucket, since + * that is what the L1 `bucketHint` and the censorship/cap asserts are read against. + * + * The name is retained from when the reference did name a bucket; it is kept so the wire format and every call site + * stay untouched, and the terminology migration is deferred. A wrong reference can only cause a confirmation miss, + * or select a bundle that re-execution of the block then rejects; the checkpoint header's `inboxRollingHash` remains + * the signed consensus commitment. */ export class InboxBucketRef { constructor( - /** Consensus rolling hash (truncated sha256 chain) after the last message absorbed into the referenced bucket. */ + /** + * Consensus rolling hash (truncated sha256 chain) of the canonical message prefix the block consumed through, + * i.e. after the message at `header.state.l1ToL2MessageTree.nextAvailableLeafIndex - 1`. Zero at genesis. + */ public readonly inboxRollingHash: Fr, ) {} @@ -80,7 +96,11 @@ export class InboxBucketRef { return new InboxBucketRef(fields.inboxRollingHash); } - /** Derives a wire reference from a bucket snapshot as tracked by the archiver. */ + /** + * Derives a wire reference from a bucket snapshot as tracked by the archiver. A bucket's rolling hash *is* the + * prefix hash at its cumulative total, so this is the convenience form for a position that happens to sit on a + * current boundary; a position interior to a bucket is referenced by constructing the prefix hash directly. + */ static fromBucket(bucket: InboxBucket): InboxBucketRef { return new InboxBucketRef(bucket.inboxRollingHash); } diff --git a/yarn-project/stdlib/src/messaging/l1_to_l2_message_source.ts b/yarn-project/stdlib/src/messaging/l1_to_l2_message_source.ts index 7792eca972f6..cbd4acf47911 100644 --- a/yarn-project/stdlib/src/messaging/l1_to_l2_message_source.ts +++ b/yarn-project/stdlib/src/messaging/l1_to_l2_message_source.ts @@ -46,10 +46,27 @@ export interface L1ToL2MessageSource { * identifies the bucket by content: an L1 reorg that re-times or merges buckets can move a bucket's sequence * number while leaving the hash intact, and a proposer resolves the sequence number it publishes this way rather * than trusting the one it captured at build time. `Fr.ZERO` resolves the genesis sentinel bucket (sequence 0). + * + * Only a completed checkpoint's final consumed position has to resolve this way, since that is what the L1 + * `bucketHint` and the censorship/cap checks are taken against. An intermediate block's consumed position is + * authenticated by count instead, with {@link getInboxRollingHashAt}. * @param inboxRollingHash - The consensus rolling hash to resolve to a bucket. */ getInboxBucketByRollingHash(inboxRollingHash: Fr): Promise; + /** + * Returns the consensus rolling hash of the canonical message prefix of length `totalMsgCount`, or undefined when + * the source has not synced a message at index `totalMsgCount - 1`. + * + * The count addresses a canonical compact message index and need not land on a boundary of the bucket partition + * the source currently holds, so a block's committed leaf count stays authenticatable after an L1 reorg has merged + * away the bucket that once ended there. Each message's rolling hash chains the one before it, so equality at a + * count proves equality of every leaf through it: this is how a validator checks that a proposed block consumed + * the same message prefix it holds itself. `totalMsgCount === 0` is the genesis base case and returns `Fr.ZERO`. + * @param totalMsgCount - The cumulative Inbox message count (leaf count) whose prefix hash to return. + */ + getInboxRollingHashAt(totalMsgCount: bigint): Promise; + /** * Returns the message leaves absorbed into buckets in the range `(fromExclusive, toInclusive]`, in insertion * order, for streaming message-bundle derivation. Both bounds must name buckets the source diff --git a/yarn-project/stdlib/src/p2p/block_proposal.ts b/yarn-project/stdlib/src/p2p/block_proposal.ts index 8bfe553c184f..bc52387ec9a2 100644 --- a/yarn-project/stdlib/src/p2p/block_proposal.ts +++ b/yarn-project/stdlib/src/p2p/block_proposal.ts @@ -87,9 +87,14 @@ export class BlockProposal extends Gossipable implements Signable { public readonly signedTxs?: SignedTxs, /** - * Reference to the Inbox bucket this block proposes to consume, when the proposer commits to one. Validators - * resolve it against their own Inbox view and derive the consumed message bundle from it rather than trusting a - * proposer-supplied message list. Covered by the proposal signature (part of `getPayloadToSign`). + * The signed cumulative Inbox message-prefix hash this block proposes to have consumed through, when the + * proposer commits to one. Interpreted together with the end count this proposal's own `blockHeader` commits to + * in `state.l1ToL2MessageTree.nextAvailableLeafIndex`: validators confirm the prefix hash at that count against + * their own Inbox view and read the consumed message bundle from the resulting count range, rather than trusting + * a proposer-supplied message list. The position may be interior to a current bucket, which is what a reorg that + * repartitions the same messages leaves behind. Covered by the proposal signature (part of `getPayloadToSign`), + * so the count and the hash are signed as a pair. The `bucketRef` name is retained from when the reference named + * a bucket; see {@link InboxBucketRef}. */ public readonly bucketRef?: InboxBucketRef, ) { @@ -129,9 +134,10 @@ export class BlockProposal extends Gossipable implements Signable { /** * Get the payload to sign for this block proposal. - * The signature is over: blockHeader + indexWithinCheckpoint + archiveRoot + txHashes, plus the bucket reference - * when set. Appending only when set binds the reference to the signature so a relay cannot strip or inject it - * without breaking recovery. + * The signature is over: blockHeader + indexWithinCheckpoint + archiveRoot + txHashes, plus the Inbox prefix + * reference when set. Appending only when set binds the reference to the signature so a relay cannot strip or + * inject it without breaking recovery. Signing it alongside the header is what makes the (end count, prefix hash) + * pair a single authenticated statement about what the block consumed. */ getPayloadToSign(): Buffer { return serializeToBuffer([ @@ -268,7 +274,7 @@ export class BlockProposal extends Gossipable implements Signable { } else { buffer.push(0); // hasSignedTxs = false } - // Optional bucket-reference tail. Appended only when set, so a proposal without a reference + // Optional Inbox prefix-reference tail. Appended only when set, so a proposal without a reference // serializes without the tail and a decoder that reaches EOF reads it as unset. if (this.bucketRef) { buffer.push(1); // hasBucketRef = true @@ -299,7 +305,7 @@ export class BlockProposal extends Gossipable implements Signable { } } - // Optional bucket-reference tail. A buffer that ends after the signedTxs flag decodes as + // Optional Inbox prefix-reference tail. A buffer that ends after the signedTxs flag decodes as // "no reference", so proposals written without the tail round-trip cleanly. let bucketRef: InboxBucketRef | undefined; if (!reader.isEmpty()) { diff --git a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts index a22dd24f21c2..5cf6d39238fd 100644 --- a/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts +++ b/yarn-project/stdlib/src/p2p/checkpoint_proposal.ts @@ -71,8 +71,10 @@ export type CheckpointLastBlock = Omit & { /** The signed transactions in the last block (optional, for DA guarantees) */ signedTxs?: SignedTxs; /** - * Reference to the Inbox bucket the last block proposes to consume. When set, its rolling hash must equal the - * checkpoint header's `inboxRollingHash` (enforced at construction). + * The signed cumulative Inbox message-prefix hash the last block proposes to have consumed through. When set it + * must equal the checkpoint header's `inboxRollingHash` (enforced at construction), which is what makes the last + * block's position the checkpoint's own: unlike an intermediate block's, that position must resolve to a current + * bucket, because L1 reads the header's hash out of the bucket the `propose` hint names. */ bucketRef?: InboxBucketRef; }; @@ -110,8 +112,9 @@ export class CheckpointProposal extends Gossipable implements Signable { ) { super(); - // Check that last block properties match those of the checkpoint. The last block's bucket reference - // commits to the same rolling hash as the checkpoint header. Only enforced when the reference is set. + // Check that last block properties match those of the checkpoint. The last block's Inbox prefix reference + // commits to the same rolling hash as the checkpoint header, so the checkpoint's consumed position is exactly + // its last block's. Only enforced when the reference is set. if (lastBlock?.bucketRef && !lastBlock.bucketRef.inboxRollingHash.equals(checkpointHeader.inboxRollingHash)) { throw new Error( `CheckpointProposal lastBlock bucketRef rolling hash ${lastBlock.bucketRef.inboxRollingHash} does not match checkpoint inboxRollingHash ${checkpointHeader.inboxRollingHash}`, @@ -294,7 +297,7 @@ export class CheckpointProposal extends Gossipable implements Signable { } else { buffer.push(0); // hasSignedTxs = false } - // Optional bucket-reference tail. Appended only when set, so a proposal without a reference + // Optional Inbox prefix-reference tail. Appended only when set, so a proposal without a reference // serializes without the tail and a decoder that reaches EOF reads it as unset. if (this.lastBlock.bucketRef) { buffer.push(1); // hasBucketRef = true @@ -336,7 +339,7 @@ export class CheckpointProposal extends Gossipable implements Signable { } } - // Optional bucket-reference tail. A buffer that ends after the signedTxs flag decodes as + // Optional Inbox prefix-reference tail. A buffer that ends after the signedTxs flag decodes as // "no reference", so proposals written without the tail round-trip cleanly. let bucketRef: InboxBucketRef | undefined; if (!reader.isEmpty()) { diff --git a/yarn-project/validator-client/src/proposal_handler.test.ts b/yarn-project/validator-client/src/proposal_handler.test.ts index d6d1bda8229e..e8b49286257e 100644 --- a/yarn-project/validator-client/src/proposal_handler.test.ts +++ b/yarn-project/validator-client/src/proposal_handler.test.ts @@ -10,9 +10,9 @@ import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer'; import { Fr } from '@aztec/foundation/curves/bn254'; import { TestDateProvider } from '@aztec/foundation/timer'; import { type FieldsOf, unfreeze } from '@aztec/foundation/types'; -import type { P2P } from '@aztec/p2p'; +import type { P2P, PeerId } from '@aztec/p2p'; import { BlockHash, GENESIS_BLOCK_HEADER_HASH } from '@aztec/stdlib/block'; -import type { BlockData, L2Block, L2BlockSink, L2BlockSource } from '@aztec/stdlib/block'; +import { type BlockData, L2Block, type L2BlockSink, type L2BlockSource } from '@aztec/stdlib/block'; import { type Checkpoint, CheckpointReexecutionTracker, type ProposedCheckpointData } from '@aztec/stdlib/checkpoint'; import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers'; import type { ITxProvider, ValidatorClientFullConfig, WorldStateSynchronizer } from '@aztec/stdlib/interfaces/server'; @@ -37,7 +37,11 @@ import { type MockProxy, mock } from 'jest-mock-extended'; import type { CheckpointBuilder, FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; import type { ValidatorMetrics } from './metrics.js'; -import { ProposalHandler } from './proposal_handler.js'; +import { + ProposalHandler, + SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT, + SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT, +} from './proposal_handler.js'; /** Creates a checkpoint proposal core with the given overrides. */ async function makeProposal(overrides: Parameters[0] = {}) { @@ -71,6 +75,21 @@ describe('ProposalHandler checkpoint validation', () => { blockSource.syncImmediate.mockResolvedValue(undefined); l1ToL2MessageSource = mock(); + // An Inbox that has absorbed nothing: count 0 is the genesis prefix (zero hash) and the genesis sentinel bucket, + // so a proposal that consumes nothing passes both the per-block prefix check and the checkpoint endpoint check. + // Tests that care about consumption override these. + l1ToL2MessageSource.getInboxRollingHashAt.mockImplementation(total => + Promise.resolve(total === 0n ? Fr.ZERO : undefined), + ); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockImplementation(total => + Promise.resolve(total === 0n ? genesisBucket : undefined), + ); + l1ToL2MessageSource.getInboxBucketByRollingHash.mockImplementation(hash => + Promise.resolve(hash.isZero() ? genesisBucket : undefined), + ); + // Range reads default to empty rather than to an unconfigured `undefined`, which the handler treats as a range + // it cannot serve. Tests whose subject is the bundle mock this explicitly. + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([]); checkpointsBuilder = mock(); checkpointsBuilder.getConfig.mockReturnValue({ @@ -623,13 +642,14 @@ describe('ProposalHandler checkpoint validation', () => { ), ); - // The checkpoint's final position is still a live bucket boundary; only the parent's is gone. + // The checkpoint's final position is still a live bucket boundary carrying the header's rolling hash; only the + // parent checkpoint's position is interior now. l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockImplementation(total => Promise.resolve( total === 7n ? ({ seq: 1n, - inboxRollingHash: Fr.random(), + inboxRollingHash: header.inboxRollingHash, totalMsgCount: 7n, timestamp: 10n, msgCount: 7, @@ -849,12 +869,14 @@ describe('ProposalHandler checkpoint validation', () => { * handler wired to accept it up to the block-number guard. */ async function setupGenesisProposal(proposalArchive: Fr, txHashes?: TxHash[]) { + // A block consuming nothing: leaf count 0, so its signed prefix is the genesis zero hash. That makes the + // streaming metadata checks pass and leaves the guard under test as what decides the outcome. + const blockHeader = makeBlockHeader(1, { slotNumber: SlotNumber(1) }); + blockHeader.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot(Fr.random(), 0); const proposal = ValidatedBlockProposal( await makeBlockProposal({ - blockHeader: makeBlockHeader(1, { slotNumber: SlotNumber(1) }), + blockHeader, archiveRoot: proposalArchive, - // A consume-nothing reference to the genesis bucket, so the streaming metadata checks pass and the - // guard under test is what decides the outcome. bucketRef: InboxBucketRef.fromBucket(genesisBucket), ...(txHashes ? { txHashes } : {}), }), @@ -864,8 +886,6 @@ describe('ProposalHandler checkpoint validation', () => { blockSource.getGenesisValues.mockResolvedValue({ genesisArchiveRoot: proposal.blockHeader.lastArchive.root, } as any); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(genesisBucket); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(genesisBucket); const txProvider = mock(); txProvider.getTxsForBlockProposal.mockResolvedValue({ txs: [], missingTxs: [] } as any); @@ -1010,9 +1030,12 @@ describe('ProposalHandler checkpoint validation', () => { // local latency into a slashable invalid-proposal verdict against an honest proposer. it('processes a proposal whose receive window closed while it waited to be processed', async () => { const signer = Secp256k1Signer.random(); + // Consumes nothing, so its signed prefix is the genesis zero hash and the arrival window is what decides. + const blockHeader = makeBlockHeader(1, { slotNumber: SlotNumber(1) }); + blockHeader.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot(Fr.random(), 0); const proposal = ValidatedBlockProposal( await makeBlockProposal({ - blockHeader: makeBlockHeader(1, { slotNumber: SlotNumber(1) }), + blockHeader, archiveRoot: Fr.random(), bucketRef: InboxBucketRef.fromBucket(genesisBucket), signer, @@ -1023,8 +1046,6 @@ describe('ProposalHandler checkpoint validation', () => { blockSource.getGenesisValues.mockResolvedValue({ genesisArchiveRoot: proposal.blockHeader.lastArchive.root, } as any); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(genesisBucket); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(genesisBucket); epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(signer.address); // Slot 1's proposal receive window is [-4s, 17s]; 30s is well past its close. epochCache.getEpochAndSlotNow.mockReturnValue({ @@ -1060,23 +1081,44 @@ describe('ProposalHandler checkpoint validation', () => { // Streaming Inbox: a block proposal's L1-to-L2 bundle is derived from its bucket reference and gated by the four // acceptance checks, replacing the legacy per-checkpoint inHash comparison. describe('handleBlockProposal streaming inbox checks', () => { - const bucket = (overrides: Partial = {}): InboxBucket => ({ - seq: 1n, - inboxRollingHash: new Fr(0xabc), - totalMsgCount: 2n, - timestamp: 100n, - msgCount: 2, - lastMessageIndex: 1n, - l1BlockNumber: 10n, - l1BlockHash: Buffer32.fromBigInt(10n), - ...overrides, + /** The prefix hash the proposals under test claim to have consumed through, at count `PROPOSED_END_COUNT`. */ + const PROPOSED_PREFIX_HASH = new Fr(0xabc); + const PROPOSED_END_COUNT = 2n; + + // attestation_deadline(slot=1) = 1*24 + 24 - 8 = 40s. Waits run on a real timer against the remaining budget + // read off the fake clock, so holding it 2s short of the deadline keeps the tests short. + const DEADLINE_MS = 40_000; + const WAIT_BUDGET_MS = 2_000; + const BEFORE_DEADLINE_MS = DEADLINE_MS - WAIT_BUDGET_MS; + const PAST_DEADLINE_MS = DEADLINE_MS + 1_000; + const WAIT_INTERVAL_MS = 500; + const proposalSender = mock(); + + /** A complete successful re-execution result for tests that stop short of executing a real block. */ + const makeSuccessfulReexecution = async (block?: L2Block) => ({ + block: block ?? (await L2Block.random(BlockNumber(1))), + failedTxs: [], + reexecutionTimeMs: 0, + totalManaUsed: 0, }); - /** Genesis-parent streaming block proposal at slot 1, with the handler wired to reach the streaming checks. */ - async function setupStreamingProposal(bucketRef: InboxBucketRef | undefined, options: { nowMs?: number } = {}) { + /** + * Genesis-parent streaming block proposal at slot 1, with the handler wired to reach the streaming checks. The + * header's L1-to-L2 leaf count is set explicitly, since that is the signed end count the prefix reference is + * interpreted against. + */ + async function setupStreamingProposal( + bucketRef: InboxBucketRef | undefined, + options: { nowMs?: number; endCount?: bigint } = {}, + ) { + const blockHeader = makeBlockHeader(1, { slotNumber: SlotNumber(1) }); + blockHeader.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot( + Fr.random(), + Number(options.endCount ?? PROPOSED_END_COUNT), + ); const proposal = ValidatedBlockProposal( await makeBlockProposal({ - blockHeader: makeBlockHeader(1, { slotNumber: SlotNumber(1) }), + blockHeader, archiveRoot: Fr.random(), txHashes: [], bucketRef, @@ -1110,57 +1152,55 @@ describe('ProposalHandler checkpoint validation', () => { } // The metadata checks are point lookups against the local Inbox view, so they run before tx collection: a - // proposer signing a bogus bucket reference must not be able to make validators spend their window fetching + // proposer signing a bogus prefix reference must not be able to make validators spend their window fetching // txs over P2P for a proposal a map lookup rejects. - it('rejects without collecting txs when the referenced bucket is unknown', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); - const { proposal, blockHandler, txProvider } = await setupStreamingProposal(ref); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); + it('rejects without collecting txs when the referenced prefix is not synced', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler, txProvider } = await setupStreamingProposal(ref, { nowMs: 100_000 }); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(undefined); const reexecuteSpy = jest.spyOn(blockHandler, 'reexecuteTransactions'); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_unavailable', }); expect(txProvider.getTxsForBlockProposal).not.toHaveBeenCalled(); expect(reexecuteSpy).not.toHaveBeenCalled(); }); - it('rejects without collecting txs when the proposal carries no bucket reference', async () => { + it('rejects without collecting txs when the proposal carries no prefix reference', async () => { const { proposal, blockHandler, txProvider } = await setupStreamingProposal(undefined); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_unavailable', }); expect(txProvider.getTxsForBlockProposal).not.toHaveBeenCalled(); }); - it('re-executes with the bundle derived from the buckets when the checks pass', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); + it('re-executes with the bundle read by count when the checks pass', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); const { proposal, blockHandler, txProvider } = await setupStreamingProposal(ref); const derivedBundle = [new Fr(1000), new Fr(1001)]; - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(bucket()); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue( - bucket({ seq: 0n, totalMsgCount: 0n, msgCount: 0 }), - ); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue(derivedBundle); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(derivedBundle); const reexecuteSpy = jest .spyOn(blockHandler, 'reexecuteTransactions') - .mockResolvedValue({ block: undefined } as any); + .mockResolvedValue(await makeSuccessfulReexecution()); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result.isValid).toBe(true); // A valid reference still triggers tx collection, which also feeds this node's ability to serve txs to peers. expect(txProvider.getTxsForBlockProposal).toHaveBeenCalledTimes(1); - // The block re-executes with the derived per-block bundle (streaming is the only path). + // The bundle is the exact count range between the parent's position (genesis) and the signed end count. + expect(l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts).toHaveBeenCalledWith(0n, PROPOSED_END_COUNT); expect(reexecuteSpy).toHaveBeenCalledWith( proposal, BlockNumber(INITIAL_L2_BLOCK_NUM), @@ -1172,111 +1212,240 @@ describe('ProposalHandler checkpoint validation', () => { ); }); + // The whole point of A-1925: a reorg that re-mines the same messages under different bucket boundaries leaves + // the prefix at the block's end count intact, so the proposal stays valid even though no bucket ends there. + it('attests to a proposal whose end count a repartition left interior to a current bucket', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + // No bucket carries the hash and none ends at the count: the boundary is gone, the content is not. + l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(undefined); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([new Fr(1000), new Fr(1001)]); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution()); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result.isValid).toBe(true); + expect(blockSource.syncImmediate).not.toHaveBeenCalled(); + }); + + // A content-changing replacement can commit between the point check and the bundle read. Re-executing against + // the post-replacement leaves would report `state_mismatch` — a slashable verdict — for a proposal that was + // valid on the view it was checked against, so the bundle read re-confirms the prefix and the proposal is + // rejected as a local-view disagreement instead. + it('does not re-execute when a replacement lands between the point check and the bundle read', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: PAST_DEADLINE_MS }); + // The first lookup (the point check) confirms; every later one sees the replaced prefix. + l1ToL2MessageSource.getInboxRollingHashAt + .mockResolvedValueOnce(PROPOSED_PREFIX_HASH) + .mockResolvedValue(new Fr(0xdead)); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([new Fr(9000), new Fr(9001)]); + const reexecuteSpy = jest.spyOn(blockHandler, 'reexecuteTransactions'); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result).toEqual({ + isValid: false, + blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), + reason: 'prefix_mismatch', + }); + expect(reexecuteSpy).not.toHaveBeenCalled(); + expect(SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT).not.toContain('prefix_mismatch'); + }); + + it('does not re-execute, and does not throw, when the bundle range fails after a dropped suffix', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: PAST_DEADLINE_MS }); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + // The replacement shortened the chain, so the range the proposal names cannot be served whole any more. + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockRejectedValue( + new Error('Inbox message range [0, 2) is not fully synced'), + ); + const reexecuteSpy = jest.spyOn(blockHandler, 'reexecuteTransactions'); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result).toEqual({ + isValid: false, + blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), + reason: 'prefix_unavailable', + }); + expect(reexecuteSpy).not.toHaveBeenCalled(); + }); + + it('re-reads the bundle after a retry so the leaves and the confirmed prefix share one view', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); + const staleLeaves = [new Fr(9000), new Fr(9001)]; + const freshLeaves = [new Fr(1000), new Fr(1001)]; + // Before the sync this node holds the stale suffix; after it, the canonical one. The retry must hand + // re-execution the leaves it confirmed, never the pre-sync ones. + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(new Fr(0xdead)); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(staleLeaves); + blockSource.syncImmediate.mockImplementation(() => { + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue(freshLeaves); + return Promise.resolve(); + }); + const reexecuteSpy = jest + .spyOn(blockHandler, 'reexecuteTransactions') + .mockResolvedValue(await makeSuccessfulReexecution()); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result.isValid).toBe(true); + expect(reexecuteSpy).toHaveBeenCalledWith( + proposal, + BlockNumber(INITIAL_L2_BLOCK_NUM), + CheckpointNumber.INITIAL, + [], + freshLeaves, + expect.anything(), + expect.anything(), + ); + }); + + it('classifies an archiver insert rejection as a non-slashable validation failure', async () => { + // A reorg can land between the precheck and the insert. The archiver then rejects the block, and that is a + // disagreement between this node's view and the proposal, not proposer misbehavior. + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([new Fr(1000), new Fr(1001)]); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution()); + const mismatch = new Error('prefix moved'); + mismatch.name = 'InboxPrefixMismatchError'; + blockSource.addBlock.mockRejectedValue(mismatch); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result).toMatchObject({ isValid: false, reason: 'prefix_mismatch' }); + expect(SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT).not.toContain('prefix_mismatch'); + }); + + it('forwards the proposal reference verbatim to the archiver insert', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([new Fr(1000), new Fr(1001)]); + const block = await L2Block.random(BlockNumber(1)); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution(block)); + + await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(blockSource.addBlock).toHaveBeenCalledWith(block, ref); + }); + // An L1 reorg that orphans the messages a proposal consumed reaches this node as an archiver rollback, which - // re-downloads the bucket at that position from the canonical chain with a different rolling hash. Resolving - // the reference by hash is what makes a proposal this node accepted before the rollback fail after it, so no - // attestation is signed for a block that L1 will not accept. Driven off a real message store so the rollback, - // the bucket resolution and the re-read the handler does after forcing a sync are the production ones. - it('rejects a proposal it accepted before its archiver rewound the bucket', async () => { + // re-downloads the messages at those indices from the canonical chain with different rolling hashes. Checking + // the reference against the prefix hash at the block's end count is what makes a proposal this node accepted + // before the rollback fail after it, so no attestation is signed for a block that L1 will not accept. Driven off + // a real message store so the rollback, the prefix lookup and the re-read after forcing a sync are production. + it('rejects a proposal it accepted before its archiver rewound the messages', async () => { const stores = await createArchiverStore( { archiverStoreMapSizeKb: 1024 * 1024, dataDirectory: undefined, dataStoreMapSizeKb: 1024 * 1024 }, GENESIS_BLOCK_HEADER_HASH, ); const messages = makeInboxMessages(2); await stores.messages.addL1ToL2MessageBuckets(messages); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockImplementation(inboxRollingHash => - stores.messages.getInboxBucketByRollingHash(inboxRollingHash), - ); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockImplementation(total => - stores.messages.getInboxBucketByTotalMsgCount(total), + l1ToL2MessageSource.getInboxRollingHashAt.mockImplementation(total => + stores.messages.getInboxRollingHashAt(total), ); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockImplementation((from, to) => - stores.messages.getL1ToL2MessagesBetweenBuckets(from, to), + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockImplementation((from, to) => + stores.messages.getL1ToL2MessagesBetweenLeafCounts(from, to), ); - const consumedBucket = await stores.messages.getInboxBucket(2n); - const { proposal, blockHandler } = await setupStreamingProposal(InboxBucketRef.fromBucket(consumedBucket!), { - nowMs: 10_000, - }); + const { proposal, blockHandler } = await setupStreamingProposal( + new InboxBucketRef(messages[1].inboxRollingHash), + { nowMs: 10_000, endCount: 2n }, + ); - expect(await blockHandler.handleBlockProposal(proposal, {} as any, false)).toEqual({ + expect(await blockHandler.handleBlockProposal(proposal, proposalSender, false)).toEqual({ isValid: true, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), }); - // The reorg drops the second message and mines a different one in its place: bucket 2 keeps its sequence - // number and message count, but no bucket commits to the rolling hash the proposal signed any more. The - // handler cannot tell that from lag, so it waits out the remaining budget (held short here) and rejects. + // The reorg drops the second message and mines a different one in its place. The prefix at count 2 is present + // but is a different hash now, which from here is indistinguishable from lag: the handler waits out the + // remaining budget (held short here) and only then rejects, as a mismatch. await stores.messages.rewindMessagesTo({ l1BlockNumber: 0n, l1BlockHash: Buffer32.ZERO }, 1n); await stores.messages.addL1ToL2MessageBuckets( makeInboxMessages(1, { initialIndex: 1n, initialInboxHash: messages[0].inboxRollingHash }), ); dateProvider.setTime(38_000); - expect(await blockHandler.handleBlockProposal(proposal, {} as any, false)).toEqual({ + expect(await blockHandler.handleBlockProposal(proposal, proposalSender, false)).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_mismatch', }); await stores.db.close(); }); - // A bucket the proposer already consumed is on L1 by construction, so a bucket this node cannot resolve is - // (usually) local archiver lag, not a divergence: the handler forces a sync and re-checks until the - // attestation deadline instead of dropping the attestation on the spot. - describe('bucket sync wait', () => { - // attestation_deadline(slot=1) = 1*24 + 24 - 8 = 40s. Waits run on a real timer against the remaining - // budget read off the fake clock, so holding it 2s short of the deadline keeps the tests short. - const DEADLINE_MS = 40_000; - const WAIT_BUDGET_MS = 2_000; - const BEFORE_DEADLINE_MS = DEADLINE_MS - WAIT_BUDGET_MS; - const PAST_DEADLINE_MS = DEADLINE_MS + 1_000; - const WAIT_INTERVAL_MS = 500; - - /** The bucket a proposal under test consumes through. */ - const eligibleBucket = (overrides: Partial = {}) => bucket({ timestamp: 10n, ...overrides }); - - /** Wires the parent-bucket lookup and the bundle read for a proposal that consumes `eligibleBucket()`. */ + // The messages the proposer consumed are on L1 by construction, so a prefix this node cannot confirm is + // (usually) local archiver lag, not a divergence: the handler forces a sync and re-checks until the attestation + // deadline instead of dropping the attestation on the spot. Both prefix outcomes are waited out — a node that + // has not followed a reorg yet holds a *present* but non-canonical hash, which looks identical from here. + describe('prefix sync wait', () => { + /** Wires the bundle read for a proposal whose prefix check passes. */ function mockAcceptedSurroundings() { - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue( - eligibleBucket({ seq: 0n, totalMsgCount: 0n, msgCount: 0 }), - ); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue([new Fr(1000), new Fr(1001)]); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([new Fr(1000), new Fr(1001)]); } - it('attests once the referenced bucket shows up on a later archiver sync', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); + it('attests once the referenced prefix shows up on a later archiver sync', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); mockAcceptedSurroundings(); - // Unknown on arrival, synced by the time the wait re-checks. - l1ToL2MessageSource.getInboxBucketByRollingHash + // Unsynced on arrival, present by the time the wait re-checks. + l1ToL2MessageSource.getInboxRollingHashAt .mockResolvedValueOnce(undefined) - .mockResolvedValue(eligibleBucket()); - jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue({ block: undefined } as any); + .mockResolvedValue(PROPOSED_PREFIX_HASH); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution()); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result.isValid).toBe(true); expect(result.blockNumber).toEqual(BlockNumber(INITIAL_L2_BLOCK_NUM)); }); - it('rejects with bucket_unknown when the bucket never syncs, no earlier than the deadline', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); + // The A-1393 behavior a hard mismatch reject would have broken: this node may be the stale side of the reorg, + // in which case it holds a present-but-wrong hash at that count and the forced sync fixes it. + it('attests once the forced sync replaces our stale prefix with the canonical one', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); + mockAcceptedSurroundings(); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(new Fr(0xdead)); + blockSource.syncImmediate.mockImplementation(() => { + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + return Promise.resolve(); + }); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution()); + + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + + expect(result.isValid).toBe(true); + expect(blockSource.syncImmediate).toHaveBeenCalled(); + }); + + it('rejects with prefix_unavailable when the prefix never syncs, no earlier than the deadline', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); const { proposal, blockHandler, txProvider } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS, }); mockAcceptedSurroundings(); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(undefined); const startMs = Date.now(); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); const elapsedMs = Date.now() - startMs; expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_unavailable', }); // The wait runs out the remaining budget and gives up within one retry interval of the deadline. expect(elapsedMs).toBeGreaterThanOrEqual(WAIT_BUDGET_MS - 100); @@ -1285,104 +1454,70 @@ describe('ProposalHandler checkpoint validation', () => { expect(txProvider.getTxsForBlockProposal).not.toHaveBeenCalled(); }); - it('rejects immediately without syncing when the attestation deadline has already passed', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); - const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: PAST_DEADLINE_MS }); + it('rejects with prefix_mismatch only after waiting out the same budget', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); mockAcceptedSurroundings(); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(new Fr(0xdead)); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const startMs = Date.now(); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); + const elapsedMs = Date.now() - startMs; expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_mismatch', }); - // With no budget left there is nothing to wait for, so the archiver is not poked at all. - expect(blockSource.syncImmediate).not.toHaveBeenCalled(); + expect(elapsedMs).toBeGreaterThanOrEqual(WAIT_BUDGET_MS - 100); + expect(blockSource.syncImmediate).toHaveBeenCalled(); }); - it('rejects immediately without syncing when the proposal carries no bucket reference', async () => { - const { proposal, blockHandler } = await setupStreamingProposal(undefined, { - nowMs: BEFORE_DEADLINE_MS, - }); + it('rejects immediately without syncing when the attestation deadline has already passed', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); + const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: PAST_DEADLINE_MS }); + mockAcceptedSurroundings(); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(undefined); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_unavailable', }); + // With no budget left there is nothing to wait for, so the archiver is not poked at all. expect(blockSource.syncImmediate).not.toHaveBeenCalled(); }); - // Under immediate consumption a proposer can consume a bucket whose L1 block is then reorged and re-mined at - // a different timestamp. The rolling hash commits to the messages alone, so a re-time leaves it unchanged and - // the proposal stays valid against a validator still holding the pre-reorg bucket. - it('attests to a proposal referencing a re-timed bucket, whose rolling hash a re-time leaves alone', async () => { - const ref = new InboxBucketRef(new Fr(0xabc)); - const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); - mockAcceptedSurroundings(); - // This node still holds the orphaned block's timestamp; the reference carries only the rolling hash. - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(eligibleBucket({ timestamp: 14n })); - jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue({ block: undefined } as any); - - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); - - expect(result.isValid).toBe(true); - expect(blockSource.syncImmediate).not.toHaveBeenCalled(); - }); - - it('attests to a proposal whose bucket now sits at a different sequence number', async () => { - // An earlier reorg merged buckets on this node, renumbering the one the proposer consumed at seq 2. The - // reference carries only the rolling hash, so the bucket is found at whatever sequence number it now has. - const ref = new InboxBucketRef(new Fr(0xabc)); - const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); - mockAcceptedSurroundings(); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(eligibleBucket({ seq: 1n })); - jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue({ block: undefined } as any); - - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); - - expect(result.isValid).toBe(true); - expect(blockSource.syncImmediate).not.toHaveBeenCalled(); - }); - - it('rejects with bucket_unknown when no local bucket carries the rolling hash', async () => { - // "Not synced yet" and "reorged away" are indistinguishable to a validator: both leave no local bucket - // carrying the hash, and both are an unknown bucket. - const ref = new InboxBucketRef(new Fr(0xabc)); - const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: PAST_DEADLINE_MS }); - mockAcceptedSurroundings(); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); + it('rejects immediately without syncing when the proposal carries no prefix reference', async () => { + const { proposal, blockHandler } = await setupStreamingProposal(undefined, { + nowMs: BEFORE_DEADLINE_MS, + }); const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); expect(result).toEqual({ isValid: false, blockNumber: BlockNumber(INITIAL_L2_BLOCK_NUM), - reason: 'bucket_unknown', + reason: 'prefix_unavailable', }); + expect(blockSource.syncImmediate).not.toHaveBeenCalled(); }); - it('attests when the forced sync replaces our stale bucket with the proposed one', async () => { - // This validator held the orphaned side of an L1 reorg, so no local bucket carries the proposed hash until - // the forced sync rolls it back and re-syncs. - const ref = new InboxBucketRef(new Fr(0xabc)); + // A reorg that only re-times or re-partitions a bucket leaves every message where it was, so the prefix hash + // at the block's end count is unchanged and no wait is needed at all. + it('attests without syncing when only the bucket metadata moved', async () => { + const ref = new InboxBucketRef(PROPOSED_PREFIX_HASH); const { proposal, blockHandler } = await setupStreamingProposal(ref, { nowMs: BEFORE_DEADLINE_MS }); mockAcceptedSurroundings(); - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(undefined); - blockSource.syncImmediate.mockImplementation(() => { - l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(eligibleBucket()); - return Promise.resolve(); - }); - jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue({ block: undefined } as any); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(PROPOSED_PREFIX_HASH); + jest.spyOn(blockHandler, 'reexecuteTransactions').mockResolvedValue(await makeSuccessfulReexecution()); - const result = await blockHandler.handleBlockProposal(proposal, {} as any, true); + const result = await blockHandler.handleBlockProposal(proposal, proposalSender, true); expect(result.isValid).toBe(true); - expect(result.blockNumber).toEqual(BlockNumber(INITIAL_L2_BLOCK_NUM)); + expect(blockSource.syncImmediate).not.toHaveBeenCalled(); }); }); }); @@ -1426,15 +1561,62 @@ describe('ProposalHandler checkpoint validation', () => { ); } + /** + * The rolling hash `makeCheckpointHeader(0, ...)` commits to. The final consumed position must resolve to a + * current bucket carrying exactly this hash, which is what L1 reads out of the `bucketHint` bucket. + */ + const CHECKPOINT_ROLLING_HASH = new Fr(0x215); + + /** The current bucket the checkpoint's final position ends on, carrying the header's rolling hash. */ + const finalBucket = (overrides: Partial = {}): InboxBucket => ({ + ...genesisBucket, + seq: 1n, + totalMsgCount: 2n, + inboxRollingHash: CHECKPOINT_ROLLING_HASH, + ...overrides, + }); + + it('refuses to attest when the final position resolves to no current bucket', async () => { + // Only the completed checkpoint's endpoint is held to the boundary rule: L1 reads the header's rolling hash + // out of the bucket the hint names, so an interior endpoint is one `propose` would revert on. Reported apart + // from insufficient consumption, and never as sufficient just because it could not be resolved. + handler.updateConfig(config); + const { archiveRoot } = setupCensorshipMocks(2); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(undefined); + + const result = await handler.handleCheckpointProposal(await makeSlot10Proposal(archiveRoot), proposalInfo); + + expect(result).toEqual({ + isValid: false, + reason: 'inbox_final_endpoint_unresolved', + checkpointNumber: CheckpointNumber(1), + }); + expect(SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT['inbox_final_endpoint_unresolved']).toBe(false); + }); + + it('refuses to attest when the final bucket carries a different rolling hash', async () => { + handler.updateConfig(config); + const { archiveRoot } = setupCensorshipMocks(2); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue( + finalBucket({ inboxRollingHash: new Fr(0xdead) }), + ); + + const result = await handler.handleCheckpointProposal(await makeSlot10Proposal(archiveRoot), proposalInfo); + + expect(result).toEqual({ + isValid: false, + reason: 'inbox_final_endpoint_mismatch', + checkpointNumber: CheckpointNumber(1), + }); + expect(SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT['inbox_final_endpoint_mismatch']).toBe(false); + }); + it('refuses to attest when a mandatory bucket (at or before the cutoff) is left unconsumed', async () => { handler.updateConfig(config); const { archiveRoot } = setupCensorshipMocks(2); // cutoff(slot=10) is the build frame start: with l1GenesisTime=0, slotDuration=24 and // ethereumSlotDuration=4 that is (10-1)*24 - 4 = 212. - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue({ - seq: 1n, - totalMsgCount: 2n, - } as InboxBucket); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(finalBucket()); // The next (first unconsumed) bucket opened at t=100 <= cutoff 212 is mandatory and was left unconsumed. l1ToL2MessageSource.getInboxBucket.mockResolvedValue({ seq: 2n, @@ -1454,10 +1636,7 @@ describe('ProposalHandler checkpoint validation', () => { it('does not reject on censorship when the first unconsumed bucket is past the cutoff', async () => { handler.updateConfig(config); const { archiveRoot } = setupCensorshipMocks(2); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue({ - seq: 1n, - totalMsgCount: 2n, - } as InboxBucket); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(finalBucket()); // Next bucket opened at t=213 > cutoff 212: not mandatory, so the censorship check passes and validation // proceeds past it to the checkpoint rebuild (which mismatches here, an unrelated reason). l1ToL2MessageSource.getInboxBucket.mockResolvedValue({ diff --git a/yarn-project/validator-client/src/proposal_handler.ts b/yarn-project/validator-client/src/proposal_handler.ts index a5d1bd5f199b..45b79af236da 100644 --- a/yarn-project/validator-client/src/proposal_handler.ts +++ b/yarn-project/validator-client/src/proposal_handler.ts @@ -55,7 +55,7 @@ import type { } from '@aztec/stdlib/p2p'; import type { ConsensusTimetable } from '@aztec/stdlib/timetable'; import { MerkleTreeId } from '@aztec/stdlib/trees'; -import type { CheckpointGlobalVariables, FailedTx, Tx, TxHash } from '@aztec/stdlib/tx'; +import type { BlockHeader, CheckpointGlobalVariables, FailedTx, Tx, TxHash } from '@aztec/stdlib/tx'; import { InvalidBlockProposalTxsError, ReExFailedTxsError, @@ -69,10 +69,12 @@ import { type TelemetryClient, type Tracer, getTelemetryClient } from '@aztec/te import type { FullNodeCheckpointsBuilder } from './checkpoint_builder.js'; import type { ValidatorMetrics } from './metrics.js'; import { + RETRYABLE_STREAMING_BLOCK_CHECK_REASONS, type StreamingBlockCheckReason, + type StreamingBlockCheckResult, type StreamingBlockMetadataCheckResult, checkStreamingBlockProposalMetadata, - getStreamingBlockBundle, + readStreamingBlockBundle, } from './streaming_inbox_checks.js'; export type BlockProposalValidationFailureReason = @@ -133,8 +135,46 @@ export type CheckpointProposalValidationFailureReason = | 'out_hash_mismatch' // Streaming Inbox last-block censorship failure. | 'inbox_consumption_insufficient' + // Streaming Inbox final-endpoint failures: this node's Inbox view cannot confirm the checkpoint's final position. + | 'inbox_final_endpoint_unresolved' + | 'inbox_final_endpoint_mismatch' | 'checkpoint_validation_failed'; +/** + * Maps an archiver insert rejection to the matching per-block validation reason, or undefined when the error is not + * an Inbox prefix rejection and must keep propagating. + * + * Matched on the error name rather than with `instanceof`, because the archiver package is only a dev dependency + * here: the validator talks to its archiver through the `L2BlockSink` interface and must not take a runtime + * dependency on the implementation to classify its errors. Both outcomes are local-view disagreements, so neither is + * in {@link SLASHABLE_BLOCK_PROPOSAL_VALIDATION_RESULT}. + */ +function getInboxPrefixInsertFailureReason(err: unknown): StreamingBlockCheckReason | undefined { + if (!(err instanceof Error)) { + return undefined; + } + switch (err.name) { + case 'InboxPrefixMismatchError': + return 'prefix_mismatch'; + case 'InboxPrefixNotSyncedError': + case 'ProposedBlockParentNotFoundError': + return 'prefix_unavailable'; + case 'InboxConsumptionRewindsError': + return 'consumption_moves_backwards'; + default: + return undefined; + } +} + +/** Outcome of the streaming-Inbox final-consumption checks for a checkpoint proposal. */ +type CheckpointConsumptionCheckResult = + | { sufficient: true } + | { + sufficient: false; + /** Which of the three final-consumption conditions failed. */ + reason: 'inbox_final_endpoint_unresolved' | 'inbox_final_endpoint_mismatch' | 'inbox_consumption_insufficient'; + }; + /** * Mapping from a checkpoint-proposal validation failure reason to the tracker outcome that * `handleCheckpointProposal` should record. `undefined` means do not record (signature @@ -159,6 +199,9 @@ const CHECKPOINT_VALIDATION_REASON_TO_OUTCOME: Record< archive_mismatch: 'invalid', out_hash_mismatch: 'invalid', inbox_consumption_insufficient: 'invalid', + // Not proposer misbehavior: this node's Inbox view could not confirm the final position, or disagrees with it. + inbox_final_endpoint_unresolved: 'unvalidated', + inbox_final_endpoint_mismatch: 'unvalidated', checkpoint_validation_failed: 'invalid', }; @@ -225,6 +268,10 @@ export const SLASHABLE_CHECKPOINT_PROPOSAL_VALIDATION_RESULT: Record< // Streaming Inbox last-block censorship: kept out of slashing while the streaming path is new; L1 `propose` is the // authoritative reject (Rollup__UnconsumedInboxMessages). ['inbox_consumption_insufficient']: false, + // This node's Inbox view could not confirm the checkpoint's final consumed position, or disagrees with it. Both are + // local-view outcomes — a trailing archiver or a reorg this node has not followed yet — not a proposer offense. + ['inbox_final_endpoint_unresolved']: false, + ['inbox_final_endpoint_mismatch']: false, ['invalid_signature']: false, ['last_block_not_found']: false, ['block_fetch_error']: false, @@ -575,7 +622,7 @@ export class ProposalHandler { } // Streaming Inbox: run the metadata checks before committing to any network work. They are point lookups - // against our own Inbox view, so a proposal carrying a bucket reference that does not resolve locally is + // against our own Inbox view, so a proposal carrying a prefix reference that does not resolve locally is // rejected without a proposer being able to make us spend the validation window collecting its txs. const streamingMetadata = await this.awaitStreamingBlockMetadata(proposal, blockNumber, parentBlock, proposalInfo); if (!streamingMetadata.accepted) { @@ -591,15 +638,27 @@ export class ProposalHandler { // 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 txsPromise = this.collectProposalTxs(proposal, blockNumber, proposalSender, proposalInfo); - const bundlePromise = getStreamingBlockBundle(this.l1ToL2MessageSource, streamingMetadata); + const bundlePromise = this.awaitStreamingBlockBundle(proposal, blockNumber, parentBlock, proposalInfo); // 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(() => {}); bundlePromise.catch(() => {}); - const [collected, l1ToL2Messages] = await Promise.all([txsPromise, bundlePromise]); + const [collected, bundle] = await Promise.all([txsPromise, bundlePromise]); if (collected === 'invalid_embedded_txs') { return { isValid: false, blockNumber, reason: collected }; } + // The bundle read re-confirms the signed prefix after reading the leaves, so a message replacement that landed + // between the metadata check and the read is caught here rather than surfacing as a slashable `state_mismatch` + // from re-execution against leaves the proposer never saw. + if (!bundle.accepted) { + this.log.warn(`Streaming Inbox bundle read failed, skipping processing`, { + reason: bundle.reason, + bucketRef: proposal.bucketRef?.toInspect(), + ...proposalInfo, + }); + return { isValid: false, blockNumber, reason: bundle.reason }; + } + const l1ToL2Messages = bundle.bundle; const { txs, missingTxs } = collected; // Record the tx-collection outcome on the re-execution tracker @@ -666,9 +725,25 @@ export class ProposalHandler { return { isValid: false, blockNumber, reason, reexecutionResult }; } - // If we succeeded, push this block into the archiver (unless disabled) + // If we succeeded, push this block into the archiver (unless disabled), carrying the proposal's signed Inbox + // prefix reference so the archiver re-validates it against its own messages inside the insert transaction. The + // metadata check above already matched it, but an L1 reorg can land in between, and that rejection is a + // disagreement between this node's view and the proposal — not proposer misbehavior — so it is classified as a + // validation failure rather than left to escape as an unhandled proposal error. if (reexecutionResult?.block && !this.config.skipPushProposedBlocksToArchiver) { - await this.blockSource.addBlock(reexecutionResult.block); + try { + await this.blockSource.addBlock(reexecutionResult.block, proposal.bucketRef); + } catch (err) { + const reason = getInboxPrefixInsertFailureReason(err); + if (reason === undefined) { + throw err; + } + this.log.warn(`Archiver rejected the re-executed block's Inbox prefix: ${err}`, { + ...proposalInfo, + reason, + }); + return { isValid: false, blockNumber, reason, reexecutionResult }; + } } this.log.info( @@ -988,17 +1063,76 @@ export class ProposalHandler { } /** - * Runs the streaming-Inbox metadata checks, waiting out a local sync lag. A bucket the proposer consumed is on L1 - * by the time the proposal arrives, so a bucket this node cannot resolve by rolling hash is usually its own - * archiver trailing L1 — or this node being the stale side of an L1 reorg, which the forced sync rolls back. The - * two are indistinguishable from here (a reorg that changed the messages leaves no bucket carrying the hash - * either), so both cases (and the equivalent one where the block before the checkpoint's first block has not - * synced) force an archiver sync and re-check every half second until the bucket resolves or the attestation - * deadline passes, instead of dropping the attestation on the spot. Every other reason is a structural rejection - * and returns immediately. + * Reads the block's message bundle and re-confirms the signed prefix against it, waiting out a local sync lag the + * same way {@link awaitStreamingBlockMetadata} does. + * + * The metadata check already confirmed the prefix, but only as a point lookup: a content-changing message + * replacement can commit between it and this read. {@link readStreamingBlockBundle} therefore reads the leaves + * first and re-confirms the prefix hash afterwards, so the pair is taken from one stable view — and a replacement + * landing on either side of the read shows up as an unconfirmed prefix rather than as leaves the proposer never + * saw. Because it is the same local-view condition, the retry re-runs the *whole* read (range plus confirmation) + * on each attempt rather than reusing a range from an earlier view. + */ + private async awaitStreamingBlockBundle( + proposal: BlockProposal, + blockNumber: BlockNumber, + parentBlock: 'genesis' | BlockData, + proposalInfo: LogData, + ): Promise { + const readBundle = async (): Promise => { + // Re-run the metadata check too: it resolves the count range this read is taken over, and the parent/ + // checkpoint-start counts it derives can themselves move while a reorg is being followed. + const metadata = await this.checkStreamingBlockMetadata(proposal, blockNumber, parentBlock); + if (!metadata.accepted) { + return metadata; + } + return readStreamingBlockBundle(this.l1ToL2MessageSource, metadata); + }; + + const first = await readBundle(); + if (first.accepted || !RETRYABLE_STREAMING_BLOCK_CHECK_REASONS.includes(first.reason)) { + return first; + } + + const slotNumber = proposal.slotNumber; + this.log.info(`Inbox bundle read did not confirm the signed prefix, awaiting archiver sync`, { + reason: first.reason, + ...proposalInfo, + }); + const timer = new Timer(); + const resolved = await this.awaitLocalSync(slotNumber, `inbox bundle for block ${blockNumber}`, async () => { + const result = await readBundle(); + return !result.accepted && RETRYABLE_STREAMING_BLOCK_CHECK_REASONS.includes(result.reason) ? undefined : result; + }); + if (resolved === undefined) { + this.log.warn(`Timed out reading a consistent Inbox bundle, rejecting proposal`, { + reason: 'prefix_sync_timeout', + firstReason: first.reason, + slot: slotNumber, + waitedMs: timer.ms(), + ...proposalInfo, + }); + return first; + } + return resolved; + } + + /** + * Runs the streaming-Inbox metadata checks, waiting out a local sync lag. The messages the proposer consumed are on + * L1 by the time the proposal arrives, so a prefix this node cannot confirm at the block's signed end count is + * usually its own archiver trailing L1 — or this node being the stale side of an L1 reorg, which the forced sync + * rolls back. + * + * Both prefix outcomes are waited out, not just the missing one. A node that has not yet followed a reorg holds a + * *present* prefix hash at that count which simply is not canonical any more, and from here that is + * indistinguishable from a proposer naming a prefix that never existed. Hard-rejecting the mismatch would drop an + * attestation this node would have made moments later, so both it and the unavailable case (and the equivalent one + * where the block before the checkpoint's first block has not synced) force an archiver sync and re-check every + * half second until the prefix resolves or the attestation deadline passes. Every other reason is a structural + * rejection and returns immediately. * * The wait is bounded by the same consensus deadline as the other sync waits here, so a proposer referencing a - * bucket that never appears can at most make validators poll their own archiver for the remainder of its own + * prefix that never appears can at most make validators poll their own archiver for the remainder of its own * slot — which it could waste anyway by not proposing. */ private async awaitStreamingBlockMetadata( @@ -1009,24 +1143,26 @@ export class ProposalHandler { ): Promise { const first = await this.checkStreamingBlockMetadata(proposal, blockNumber, parentBlock); const bucketRef = proposal.bucketRef; - if (first.accepted || bucketRef === undefined || first.reason !== 'bucket_unknown') { + if (first.accepted || bucketRef === undefined || !RETRYABLE_STREAMING_BLOCK_CHECK_REASONS.includes(first.reason)) { return first; } const slotNumber = proposal.slotNumber; const inboxRollingHash = bucketRef.inboxRollingHash.toString(); - this.log.info(`Referenced Inbox bucket ${inboxRollingHash} not synced locally, awaiting archiver sync`, { + this.log.info(`Referenced Inbox prefix ${inboxRollingHash} unconfirmed locally, awaiting archiver sync`, { + reason: first.reason, inboxRollingHash, ...proposalInfo, }); const timer = new Timer(); - const resolved = await this.awaitLocalSync(slotNumber, `inbox bucket ${inboxRollingHash}`, async () => { + const resolved = await this.awaitLocalSync(slotNumber, `inbox prefix ${inboxRollingHash}`, async () => { const result = await this.checkStreamingBlockMetadata(proposal, blockNumber, parentBlock); - return !result.accepted && result.reason === 'bucket_unknown' ? undefined : result; + return !result.accepted && RETRYABLE_STREAMING_BLOCK_CHECK_REASONS.includes(result.reason) ? undefined : result; }); if (resolved === undefined) { - this.log.warn(`Timed out waiting for Inbox bucket ${inboxRollingHash} to sync, rejecting proposal`, { - reason: 'bucket_sync_timeout', + this.log.warn(`Timed out waiting for Inbox prefix ${inboxRollingHash} to sync, rejecting proposal`, { + reason: 'prefix_sync_timeout', + firstReason: first.reason, slot: slotNumber, inboxRollingHash, waitedMs: timer.ms(), @@ -1038,10 +1174,10 @@ export class ProposalHandler { } /** - * Runs the streaming-Inbox per-block metadata checks for a block proposal, returning the bucket range its message - * bundle derives from or a rejection reason. The parent block's consumed total and the checkpoint's starting total - * are derived from L1-to-L2 tree leaf counts; a parent whose count does not sit on a bucket boundary is rejected - * inside {@link checkStreamingBlockProposalMetadata}. + * Runs the streaming-Inbox per-block metadata checks for a block proposal, returning the cumulative message-count + * range its bundle derives from or a rejection reason. The block's end total comes from its own signed header, and + * the parent's and the checkpoint's starting totals from the L1-to-L2 tree leaf counts of the local chain; none of + * the three has to sit on a boundary of the bucket partition this node currently holds. */ private async checkStreamingBlockMetadata( proposal: BlockProposal, @@ -1056,13 +1192,14 @@ export class ProposalHandler { ); if (checkpointStartTotalMsgCount === undefined) { // The block before the checkpoint's first block has not synced locally, so the per-checkpoint cap origin is - // unavailable: treat as an unknown local view. Like an unknown bucket this is local lag rather than a - // divergence, and `awaitStreamingBlockMetadata` waits it out by re-running the whole check after a sync. - return { accepted: false, reason: 'bucket_unknown' }; + // unavailable: treat it as an unresolvable local view. Like an unconfirmable prefix this is local lag rather + // than a divergence, and `awaitStreamingBlockMetadata` waits it out by re-running the whole check after a sync. + return { accepted: false, reason: 'prefix_unavailable' }; } return checkStreamingBlockProposalMetadata({ messageSource: this.l1ToL2MessageSource, bucketRef: proposal.bucketRef, + endTotalMsgCount: this.headerLeafCount(proposal.blockHeader), parentTotalMsgCount, checkpointStartTotalMsgCount, perBlockCap: MAX_L1_TO_L2_MSGS_PER_BLOCK, @@ -1072,7 +1209,12 @@ export class ProposalHandler { /** A block's L1-to-L2 message tree leaf count: the cumulative Inbox message count it consumed through. */ private blockLeafCount(block: BlockData | L2Block): bigint { - return BigInt(block.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); + return this.headerLeafCount(block.header); + } + + /** A block header's L1-to-L2 message tree leaf count, for the signed header a proposal carries. */ + private headerLeafCount(header: BlockHeader): bigint { + return BigInt(header.state.l1ToL2MessageTree.nextAvailableLeafIndex); } /** The cumulative Inbox message count consumed through a block: its L1-to-L2 tree leaf count (0 at genesis). */ @@ -1109,49 +1251,80 @@ export class ProposalHandler { } /** - * Enforces the streaming-Inbox last-block minimum-consumption (censorship) rule for a checkpoint, mirroring - * `ProposeLib.validateInboxConsumption`: the first bucket the checkpoint left unconsumed must be absent, past the - * cutoff, or a cap-escape. Returns true (sufficient) when the checkpoint's consumption cannot be resolved against - * the local Inbox view, deferring to L1 `propose` as the authoritative reject. + * Enforces the streaming-Inbox final-consumption rules for a checkpoint, mirroring + * `ProposeLib.validateInboxConsumption`: + * + * 1. The checkpoint's final consumed position must resolve to a bucket in this node's current Inbox partition at + * exactly the last block's leaf count, and that bucket's rolling hash must equal the checkpoint header's. This + * is the one place the stricter boundary rule still applies — L1 reads the header's hash out of the bucket the + * `bucketHint` names, so a final position that is interior to a current bucket is one `propose` would revert + * on, whatever the individual blocks committed. A single count-addressed bucket lookup proves both halves: the + * bucket ending at that count is unique, so comparing its hash proves canonical prefix equality too. + * 2. The first bucket the checkpoint left unconsumed must be absent, past the cutoff, or a cap-escape. + * + * An unresolvable or mismatching final endpoint is reported separately from insufficient consumption: it means + * this node's Inbox view and the proposal disagree (or this node is mid-reorg), which is not proposer misbehavior + * and must never be reported as sufficient just because it could not be checked. */ - private async isLastBlockConsumptionSufficient(slot: SlotNumber, blocks: L2Block[]): Promise { + private async checkCheckpointFinalConsumption( + slot: SlotNumber, + blocks: L2Block[], + checkpointInboxRollingHash: Fr, + ): Promise { const lastBlockTotal = this.blockLeafCount(blocks[blocks.length - 1]); const checkpointStartTotal = await this.getPreBlockConsumedTotal(blocks[0].number); + if (checkpointStartTotal === undefined) { + return { sufficient: false, reason: 'inbox_final_endpoint_unresolved' }; + } + const lastConsumedBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(lastBlockTotal); - if (checkpointStartTotal === undefined || lastConsumedBucket === undefined) { - return true; + if (lastConsumedBucket === undefined) { + return { sufficient: false, reason: 'inbox_final_endpoint_unresolved' }; + } + if (!lastConsumedBucket.inboxRollingHash.equals(checkpointInboxRollingHash)) { + return { sufficient: false, reason: 'inbox_final_endpoint_mismatch' }; } + const nextBucket = await this.l1ToL2MessageSource.getInboxBucket(lastConsumedBucket.seq + 1n); const cutoffTimestamp = getInboxCutoffTimestamp(slot, this.epochCache.getL1Constants()); - return isInboxConsumptionSufficient({ + const sufficient = isInboxConsumptionSufficient({ nextBucket, cutoffTimestamp, checkpointStartTotalMsgCount: checkpointStartTotal, perCheckpointCap: MAX_L1_TO_L2_MSGS_PER_CHECKPOINT, }); + return sufficient ? { sufficient: true } : { sufficient: false, reason: 'inbox_consumption_insufficient' }; } /** * Derives the ordered list of L1-to-L2 messages a checkpoint consumed across its blocks: the compact message-count - * range between the parent checkpoint's consumed position and the checkpoint's last block. Empty when the - * checkpoint consumed nothing or its final consumption position does not resolve to a current Inbox bucket. + * range between the parent checkpoint's consumed position and the checkpoint's last block. * - * Only the final position is resolved as a bucket, which is the live rule this proposal must satisfy. The start - * position is a count committed by an already checkpointed block, which the archiver never prunes, so an L1 reorg - * that merges buckets can leave it permanently interior to the current partition; reading the range by count keeps - * that historical bound resolvable instead of silently deriving an empty bundle for a valid proposal. + * Both bounds are counts, not buckets. The start is a count committed by an already checkpointed block, which the + * archiver never prunes, so an L1 reorg that merges buckets can leave it permanently interior to the current + * partition; the end has just been resolved to a current bucket by + * {@link checkCheckpointFinalConsumption}. Reading the range by count keeps both resolvable instead of silently + * deriving an empty bundle for a valid proposal, and a range this node cannot serve whole propagates as a + * rejection rather than as a wrong rolling-hash recomputation. */ - private async deriveCheckpointConsumedMessages(blocks: L2Block[]): Promise { + private async deriveCheckpointConsumedMessages(blocks: L2Block[]): Promise { const checkpointStartTotal = await this.getPreBlockConsumedTotal(blocks[0].number); const lastBlockTotal = this.blockLeafCount(blocks[blocks.length - 1]); - if (checkpointStartTotal === undefined || lastBlockTotal <= checkpointStartTotal) { - return []; + if (checkpointStartTotal === undefined) { + return undefined; } - const endBucket = await this.l1ToL2MessageSource.getInboxBucketByTotalMsgCount(lastBlockTotal); - if (endBucket === undefined) { + if (lastBlockTotal <= checkpointStartTotal) { return []; } - return this.l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts(checkpointStartTotal, endBucket.totalMsgCount); + try { + return await this.l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts(checkpointStartTotal, lastBlockTotal); + } catch (err) { + this.log.warn(`Cannot read the messages this checkpoint consumed: ${err}`, { + checkpointStartTotal, + lastBlockTotal, + }); + return undefined; + } } async reexecuteTransactions( @@ -1456,20 +1629,35 @@ export class ProposalHandler { const constants = this.extractCheckpointConstants(firstBlock); const checkpointNumber = firstBlock.checkpointNumber; - // Streaming Inbox: on the last block of a checkpoint, enforce the minimum-consumption - // (censorship) rule before attesting. Reject (no attestation) if a mandatory bucket was left unconsumed. - if (!(await this.isLastBlockConsumptionSufficient(slot, blocks))) { - this.log.warn(`Streaming Inbox last-block censorship check failed, refusing to attest`, { + // Streaming Inbox: the checkpoint's final consumed position must resolve to a bucket carrying the header's + // rolling hash (what L1 checks the `bucketHint` against), and must leave no mandatory bucket unconsumed. + // Individual blocks inside the checkpoint are not held to the boundary rule; only this endpoint is. + const finalConsumption = await this.checkCheckpointFinalConsumption( + slot, + blocks, + proposal.checkpointHeader.inboxRollingHash, + ); + if (!finalConsumption.sufficient) { + this.log.warn(`Streaming Inbox final-consumption check failed, refusing to attest`, { ...proposalInfo, + reason: finalConsumption.reason, checkpointNumber, }); - return { isValid: false, reason: 'inbox_consumption_insufficient', checkpointNumber }; + return { isValid: false, reason: finalConsumption.reason, checkpointNumber }; } - // Derive the checkpoint's consumed L1-to-L2 message list from the Inbox buckets between the parent checkpoint's + // Derive the checkpoint's consumed L1-to-L2 message list from the count range between the parent checkpoint's // consumed position and the last block's (compact indexing). The messages are already in the db from per-block - // validation; this list only drives the checkpoint's rolling-hash recomputation in completeCheckpoint. + // validation; this list only drives the checkpoint's rolling-hash recomputation in completeCheckpoint, so a + // range this node cannot serve whole has to reject rather than recompute a hash from a short list. const l1ToL2Messages = await this.deriveCheckpointConsumedMessages(blocks); + if (l1ToL2Messages === undefined) { + this.log.warn(`Cannot derive the messages this checkpoint consumed, refusing to attest`, { + ...proposalInfo, + checkpointNumber, + }); + return { isValid: false, reason: 'inbox_final_endpoint_unresolved', checkpointNumber }; + } // 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. 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 dfbf2bf17dab..4e4576cc1a30 100644 --- a/yarn-project/validator-client/src/streaming_inbox_checks.test.ts +++ b/yarn-project/validator-client/src/streaming_inbox_checks.test.ts @@ -1,101 +1,103 @@ -import { Buffer32 } from '@aztec/foundation/buffer'; import { Fr } from '@aztec/foundation/curves/bn254'; -import type { InboxBucket } from '@aztec/stdlib/messaging'; +import { promiseWithResolvers } from '@aztec/foundation/promise'; import { InboxBucketRef } from '@aztec/stdlib/messaging'; import { describe, expect, it } from '@jest/globals'; import { type StreamingBlockCheckInput, - type StreamingInboxBucketSource, + type StreamingInboxMessageSource, checkStreamingBlockProposal, + readStreamingBlockBundle, } from './streaming_inbox_checks.js'; const PER_BLOCK_CAP = 1024; const PER_CHECKPOINT_CAP = 1024; -const NOW = 10_000n; /** - * In-memory Inbox-bucket view mirroring the archiver store's index semantics: buckets keyed by sequence number and - * resolvable by rolling hash, a flat leaves array indexed by global message index, and - * `getL1ToL2MessagesBetweenBuckets` slicing that array by the `(from, to]` bucket range - * (start = fromBucket.lastMessageIndex + 1, genesis when from == 0). + * In-memory Inbox view mirroring the archiver store's count-addressed semantics: a flat leaf log indexed by global + * message index, and a prefix rolling hash per count. Prefix hashes are kept apart from any bucket partition, exactly + * as the store keeps them apart from its bucket snapshots, so a test can move boundaries without touching either. */ -class FakeInboxView implements StreamingInboxBucketSource { - private readonly buckets = new Map(); +class FakeInboxView implements StreamingInboxMessageSource { private readonly leaves: Fr[] = []; + private readonly rollingHashByCount = new Map(); - constructor() { - // Genesis sentinel bucket 0 {total 0}, as the archiver store holds it (the "consumed nothing" base case). - this.buckets.set(0n, { - seq: 0n, - inboxRollingHash: Fr.ZERO, - totalMsgCount: 0n, - timestamp: 0n, - msgCount: 0, - lastMessageIndex: 0n, - l1BlockNumber: 0n, - l1BlockHash: Buffer32.ZERO, - }); - } - - /** Appends a bucket of `msgCount` leaves opened at `timestamp`, with a rolling hash derived from `seq`. */ - addBucket(seq: number, msgCount: number, timestamp: number, rollingHash?: Fr): InboxBucket { + /** + * Appends `msgCount` leaves and registers the prefix hash at the resulting count. Returns the count and the hash, + * which together are what a block proposal signs. + */ + addMessages(msgCount: number, rollingHash?: Fr): { totalMsgCount: bigint; inboxRollingHash: Fr } { const priorTotal = this.leaves.length; for (let i = 0; i < msgCount; i++) { this.leaves.push(new Fr(1000 + priorTotal + i)); } const totalMsgCount = BigInt(this.leaves.length); - const bucket: InboxBucket = { - seq: BigInt(seq), - inboxRollingHash: rollingHash ?? new Fr(500 + seq), - totalMsgCount, - timestamp: BigInt(timestamp), - msgCount, - lastMessageIndex: totalMsgCount - 1n, - l1BlockNumber: BigInt(seq), - l1BlockHash: Buffer32.fromBigInt(BigInt(seq)), - }; - this.buckets.set(BigInt(seq), bucket); - return bucket; + const inboxRollingHash = rollingHash ?? new Fr(500_000 + this.leaves.length); + this.rollingHashByCount.set(totalMsgCount, inboxRollingHash); + return { totalMsgCount, inboxRollingHash }; } - getInboxBucketByRollingHash(inboxRollingHash: Fr): Promise { - return Promise.resolve([...this.buckets.values()].find(b => b.inboxRollingHash.equals(inboxRollingHash))); + /** The prefix hash at a count, as the store would return it. */ + prefixAt(totalMsgCount: bigint): Fr { + const hash = totalMsgCount === 0n ? Fr.ZERO : this.rollingHashByCount.get(totalMsgCount); + if (hash === undefined) { + throw new Error(`Test view has no prefix hash at count ${totalMsgCount}`); + } + return hash; } - getInboxBucketByTotalMsgCount(totalMsgCount: bigint): Promise { - if (totalMsgCount === 0n) { - return Promise.resolve(this.buckets.get(0n)); - } - return Promise.resolve([...this.buckets.values()].find(b => b.totalMsgCount === totalMsgCount)); + /** Drops the prefix hash at a count, standing in for a node that has not synced that far. */ + forgetPrefixAt(totalMsgCount: bigint): void { + this.rollingHashByCount.delete(totalMsgCount); } - getL1ToL2MessagesBetweenBuckets(fromExclusive: bigint, toInclusive: bigint): Promise { - const toBucket = this.buckets.get(toInclusive); - if (toBucket === undefined) { - return Promise.resolve([]); - } - let startIndex = 0n; - if (fromExclusive > 0n) { - const fromBucket = this.buckets.get(fromExclusive); - if (fromBucket === undefined) { - return Promise.resolve([]); - } - startIndex = fromBucket.lastMessageIndex + 1n; + /** + * Replaces the leaf at `index` and every prefix hash from there on, standing in for a content-changing canonical + * message replacement. Leaves the counts alone, so only the hashes move. + */ + replaceLeafFrom(index: number): void { + this.leaves[index] = new Fr(90_000 + index); + for (let count = index + 1; count <= this.leaves.length; count++) { + this.rollingHashByCount.set(BigInt(count), new Fr(70_000 + count)); } - return Promise.resolve(this.leaves.slice(Number(startIndex), Number(toBucket.lastMessageIndex + 1n))); } -} -function refFor(bucket: InboxBucket, rollingHash = bucket.inboxRollingHash): InboxBucketRef { - return new InboxBucketRef(rollingHash); + /** Drops every leaf from `index` on, standing in for a replacement that shortened the canonical chain. */ + truncateLeavesFrom(index: number): void { + this.leaves.length = index; + } + + /** + * Runs `onceRead` immediately after the next range read resolves, so a test can land a replacement in the exact + * window between reading the leaves and re-confirming the prefix. Deterministic: no timers, no sleeps. + */ + runAfterNextRangeRead(onceRead: () => void): void { + this.afterNextRangeRead = onceRead; + } + private afterNextRangeRead: (() => void) | undefined; + + getInboxRollingHashAt(totalMsgCount: bigint): Promise { + return Promise.resolve(totalMsgCount === 0n ? Fr.ZERO : this.rollingHashByCount.get(totalMsgCount)); + } + + getL1ToL2MessagesBetweenLeafCounts(startLeafCount: bigint, endLeafCount: bigint): Promise { + if (startLeafCount > endLeafCount || endLeafCount > BigInt(this.leaves.length)) { + return Promise.reject(new Error(`Invalid Inbox leaf count range [${startLeafCount}, ${endLeafCount})`)); + } + const leaves = this.leaves.slice(Number(startLeafCount), Number(endLeafCount)); + const after = this.afterNextRangeRead; + this.afterNextRangeRead = undefined; + after?.(); + return Promise.resolve(leaves); + } } function baseInput(overrides: Partial): StreamingBlockCheckInput { return { messageSource: new FakeInboxView(), bucketRef: undefined, + endTotalMsgCount: 0n, parentTotalMsgCount: 0n, checkpointStartTotalMsgCount: 0n, perBlockCap: PER_BLOCK_CAP, @@ -105,155 +107,411 @@ function baseInput(overrides: Partial): StreamingBlock } describe('checkStreamingBlockProposal', () => { - describe('check 1: a bucket carrying the referenced rolling hash exists', () => { - it('rejects a proposal with no bucket reference', async () => { + describe('check 1: the proposal carries a prefix reference', () => { + it('rejects a proposal with no prefix reference', async () => { const result = await checkStreamingBlockProposal(baseInput({ bucketRef: undefined })); - expect(result).toEqual({ accepted: false, reason: 'bucket_unknown' }); + expect(result).toEqual({ accepted: false, reason: 'prefix_unavailable' }); }); + }); - it('rejects promptly when the referenced bucket is unknown (no waiting)', async () => { + describe('check 2: consumption moves forward', () => { + it('rejects when the end count is behind the parent block', async () => { const view = new FakeInboxView(); - const start = Date.now(); + const prefix = view.addMessages(3); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: new InboxBucketRef(new Fr(1)) }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: 3n, + parentTotalMsgCount: 5n, + }), ); - expect(result).toEqual({ accepted: false, reason: 'bucket_unknown' }); - // The happy path rejects immediately; there is no bounded wait yet. Assert it did not sleep. - expect(Date.now() - start).toBeLessThan(500); + expect(result).toEqual({ accepted: false, reason: 'consumption_moves_backwards' }); }); + }); - it('resolves the referenced bucket by rolling hash, whatever sequence number it sits at', async () => { + describe('check 3: caps', () => { + it('accepts a block consuming exactly the per-block cap', async () => { const view = new FakeInboxView(); - const bucket = view.addBucket(7, 3, 100); - const result = await checkStreamingBlockProposal(baseInput({ messageSource: view, bucketRef: refFor(bucket) })); - expect(result).toEqual({ accepted: true, bundle: [new Fr(1000), new Fr(1001), new Fr(1002)] }); + const prefix = view.addMessages(PER_BLOCK_CAP); + const result = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: prefix.totalMsgCount, + perBlockCap: PER_BLOCK_CAP, + }), + ); + expect(result.accepted).toBe(true); }); - it('rejects as unknown when no bucket carries the referenced hash, whatever else is synced', async () => { + it('rejects a block consuming one over the per-block cap', async () => { const view = new FakeInboxView(); - const bucket = view.addBucket(1, 3, 100); + const prefix = view.addMessages(4); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket, new Fr(999)) }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: 4n, + perBlockCap: 3, + }), ); - expect(result).toEqual({ accepted: false, reason: 'bucket_unknown' }); + expect(result).toEqual({ accepted: false, reason: 'bundle_over_block_cap' }); }); - }); - describe('check 2: consumption moves forward', () => { - it('rejects when the bucket total is behind the parent block', async () => { + it('rejects when the running checkpoint total exceeds the per-checkpoint cap', async () => { const view = new FakeInboxView(); - const bucket = view.addBucket(1, 3, 100); // total 3 + view.addMessages(3); // the checkpoint's earlier consumption, total 3 + const prefix = view.addMessages(3); // total 6 const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket), parentTotalMsgCount: 5n }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: 6n, + parentTotalMsgCount: 3n, + checkpointStartTotalMsgCount: 0n, + perCheckpointCap: 5, // 6 - 0 = 6 > 5 + }), ); - expect(result).toEqual({ accepted: false, reason: 'bucket_moves_backwards' }); + expect(result).toEqual({ accepted: false, reason: 'checkpoint_over_msg_cap' }); }); - it('accepts an empty-consumption block that reuses the parent bucket (empty bundle)', async () => { + it('measures the caps from the signed header count, not from a bucket', async () => { + // Both caps are computed before any lookup, so an over-cap proposal is rejected without touching the store. const view = new FakeInboxView(); - const bucket = view.addBucket(1, 3, 100); // total 3 + view.addMessages(8); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket), parentTotalMsgCount: 3n }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(new Fr(1)), + endTotalMsgCount: 8n, + parentTotalMsgCount: 0n, + perBlockCap: 4, + }), ); - expect(result).toEqual({ accepted: true, bundle: [] }); + expect(result).toEqual({ accepted: false, reason: 'bundle_over_block_cap' }); }); }); - describe('bucket age is not a check', () => { - it('accepts a bucket opened one second ago', async () => { - // How long a proposer waits before consuming a bucket is its own policy; L1 has no age rule, so neither do we. + describe('check 4: the prefix hash at the signed end count', () => { + it('accepts an end count that is a current bucket boundary', async () => { + const view = new FakeInboxView(); + const prefix = view.addMessages(3); + const result = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: prefix.totalMsgCount, + }), + ); + expect(result).toEqual({ accepted: true, bundle: [new Fr(1000), new Fr(1001), new Fr(1002)] }); + }); + + it('accepts an end count that a pure repartition left interior to a current bucket', async () => { + // The signed reference names the prefix at count 2, which was a boundary when the block was built. A reorg then + // re-mined the same three messages as one bucket, so nothing ends at 2 any more — but every leaf and every + // prefix hash is unchanged, so the block is exactly as valid as when it was signed. const view = new FakeInboxView(); - const bucket = view.addBucket(1, 2, Number(NOW) - 1); - const result = await checkStreamingBlockProposal(baseInput({ messageSource: view, bucketRef: refFor(bucket) })); + const interior = view.addMessages(2); + view.addMessages(1); + const result = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(interior.inboxRollingHash), + endTotalMsgCount: interior.totalMsgCount, + }), + ); expect(result).toEqual({ accepted: true, bundle: [new Fr(1000), new Fr(1001)] }); }); - }); - describe('check 3: caps', () => { - it('accepts a block consuming exactly the per-block cap', async () => { + it('accepts a following block whose parent count is interior', async () => { + // Block N ended at the now-interior count 2; block N+1 consumes the suffix through count 3. const view = new FakeInboxView(); - const bucket = view.addBucket(1, PER_BLOCK_CAP, 100); + view.addMessages(2); + const end = view.addMessages(1); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket), perBlockCap: PER_BLOCK_CAP }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + endTotalMsgCount: 3n, + parentTotalMsgCount: 2n, + }), ); - expect(result.accepted).toBe(true); + expect(result).toEqual({ accepted: true, bundle: [new Fr(1002)] }); }); - it('rejects a block consuming one over the per-block cap', async () => { + it('reports a mismatch when the canonical prefix at that count is a different hash', async () => { const view = new FakeInboxView(); - const bucket = view.addBucket(1, 4, 100); + const prefix = view.addMessages(3); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket), perBlockCap: 3 }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(new Fr(999)), + endTotalMsgCount: prefix.totalMsgCount, + }), ); - expect(result).toEqual({ accepted: false, reason: 'bundle_over_block_cap' }); + expect(result).toEqual({ accepted: false, reason: 'prefix_mismatch' }); }); - it('rejects when the running checkpoint total exceeds the per-checkpoint cap', async () => { + it('reports unavailable when nothing is synced at that count', async () => { const view = new FakeInboxView(); - view.addBucket(1, 3, 100); // total 3, the checkpoint's earlier consumption - const bucket = view.addBucket(2, 3, 100); // total 6 + const prefix = view.addMessages(3); + view.forgetPrefixAt(prefix.totalMsgCount); const result = await checkStreamingBlockProposal( baseInput({ messageSource: view, - bucketRef: refFor(bucket), - parentTotalMsgCount: 3n, - checkpointStartTotalMsgCount: 0n, - perCheckpointCap: 5, // 6 - 0 = 6 > 5 + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: prefix.totalMsgCount, }), ); - expect(result).toEqual({ accepted: false, reason: 'checkpoint_over_msg_cap' }); + expect(result).toEqual({ accepted: false, reason: 'prefix_unavailable' }); + }); + + it('rejects promptly, without waiting, on either prefix outcome', async () => { + // The bounded catch-up wait lives in the proposal handler; these checks are point lookups and must not sleep. + const view = new FakeInboxView(); + const start = Date.now(); + const result = await checkStreamingBlockProposal( + baseInput({ messageSource: view, bucketRef: new InboxBucketRef(new Fr(1)), endTotalMsgCount: 1n }), + ); + expect(result).toEqual({ accepted: false, reason: 'prefix_unavailable' }); + expect(Date.now() - start).toBeLessThan(500); }); }); - describe('bundle derivation', () => { - it('derives the bundle for a genesis-parent first block', async () => { + describe('empty blocks', () => { + it('checks the prefix at an unchanged count instead of skipping the hash', async () => { const view = new FakeInboxView(); - const bucket = view.addBucket(1, 3, 100); // leaves at global indices 0..2 + const prefix = view.addMessages(3); + const accepted = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: 3n, + parentTotalMsgCount: 3n, + }), + ); + expect(accepted).toEqual({ accepted: true, bundle: [] }); + + const rejected = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(new Fr(999)), + endTotalMsgCount: 3n, + parentTotalMsgCount: 3n, + }), + ); + expect(rejected).toEqual({ accepted: false, reason: 'prefix_mismatch' }); + }); + + it('accepts a genesis empty block whose reference is the zero hash', async () => { const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(bucket), parentTotalMsgCount: 0n }), + baseInput({ bucketRef: new InboxBucketRef(Fr.ZERO), endTotalMsgCount: 0n, parentTotalMsgCount: 0n }), ); - expect(result).toEqual({ accepted: true, bundle: [new Fr(1000), new Fr(1001), new Fr(1002)] }); + expect(result).toEqual({ accepted: true, bundle: [] }); + }); + + it('rejects a genesis empty block whose reference is not the zero hash', async () => { + const result = await checkStreamingBlockProposal( + baseInput({ bucketRef: new InboxBucketRef(new Fr(7)), endTotalMsgCount: 0n, parentTotalMsgCount: 0n }), + ); + expect(result).toEqual({ accepted: false, reason: 'prefix_mismatch' }); + }); + }); + + // The metadata check is a point lookup; a content-changing message replacement can commit between it and the + // bundle read. `readStreamingBlockBundle` reads the leaves first and re-confirms the prefix afterwards, so the + // pair always comes from one stable view. Without that, the caller would either re-execute against leaves the + // proposer never saw — reporting a slashable state mismatch for an honest proposal — or see the range throw. + describe('bundle read re-confirms the prefix after reading', () => { + it('rejects when a replacement lands between the point check and the read', async () => { + const view = new FakeInboxView(); + view.addMessages(2); + const end = view.addMessages(1); + + // The metadata check passes against the pre-replacement view. + const metadata = await checkStreamingBlockProposal( + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + endTotalMsgCount: 3n, + parentTotalMsgCount: 2n, + }), + ); + expect(metadata.accepted).toBe(true); + + // Now the replacement commits, then the bundle is read: the leaves changed under the same counts. + view.replaceLeafFrom(2); + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: false, reason: 'prefix_mismatch' }); + }); + + it('rejects when a replacement lands in the window between the read and the confirmation', async () => { + const view = new FakeInboxView(); + view.addMessages(2); + const end = view.addMessages(1); + // The replacement commits the instant the leaves have been read, which is the narrowest window there is. + view.runAfterNextRangeRead(() => view.replaceLeafFrom(2)); + + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: false, reason: 'prefix_mismatch' }); }); - it('derives the bundle spanning multiple buckets since the parent', async () => { + it('reports a range it cannot serve whole as prefix_unavailable rather than throwing', async () => { const view = new FakeInboxView(); - view.addBucket(1, 2, 100); // parent consumed through here, total 2 - view.addBucket(2, 2, 100); // total 4 - const proposed = view.addBucket(3, 1, 100); // total 5 + view.addMessages(2); + const end = view.addMessages(1); + // A replacement dropped the suffix, so the range the proposal names no longer exists. + view.truncateLeavesFrom(2); + + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: false, reason: 'prefix_unavailable' }); + }); + + it('reports prefix_unavailable when the prefix hash is gone even though the range still reads', async () => { + const view = new FakeInboxView(); + view.addMessages(2); + const end = view.addMessages(1); + view.runAfterNextRangeRead(() => view.forgetPrefixAt(3n)); + + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: false, reason: 'prefix_unavailable' }); + }); + + it('returns the leaves it confirmed when nothing moved', async () => { + const view = new FakeInboxView(); + view.addMessages(2); + const end = view.addMessages(1); + + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: true, bundle: [new Fr(1002)] }); + }); + + it('confirms an empty range without reading messages', async () => { + const view = new FakeInboxView(); + const end = view.addMessages(2); + // An empty bundle still has to name the signed prefix, so the confirmation runs even with nothing to read. + view.runAfterNextRangeRead(() => { + throw new Error('an empty range must not be read'); + }); + + const result = await readStreamingBlockBundle(view, { + parentTotalMsgCount: 2n, + endTotalMsgCount: 2n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }); + + expect(result).toEqual({ accepted: true, bundle: [] }); + }); + + it('does not resolve before the read completes, so a caller cannot observe a torn pair', async () => { + // Pins the ordering: the confirmation is awaited after the read, so there is no interleaving in which a + // caller receives leaves whose prefix has not been re-confirmed. + const view = new FakeInboxView(); + view.addMessages(2); + const end = view.addMessages(1); + const gate = promiseWithResolvers(); + const order: string[] = []; + view.runAfterNextRangeRead(() => order.push('read')); + + const pending = readStreamingBlockBundle( + { + getInboxRollingHashAt: async count => { + await gate.promise; + order.push('confirm'); + return view.getInboxRollingHashAt(count); + }, + getL1ToL2MessagesBetweenLeafCounts: (from, to) => view.getL1ToL2MessagesBetweenLeafCounts(from, to), + }, + { + parentTotalMsgCount: 2n, + endTotalMsgCount: 3n, + bucketRef: new InboxBucketRef(end.inboxRollingHash), + }, + ); + + // The read has happened; the confirmation is still blocked, so nothing has been handed back yet. + await Promise.resolve(); + expect(order).toEqual(['read']); + + gate.resolve(); + expect(await pending).toEqual({ accepted: true, bundle: [new Fr(1002)] }); + expect(order).toEqual(['read', 'confirm']); + }); + }); + + describe('bundle derivation', () => { + it('derives the bundle for a genesis-parent first block', async () => { + const view = new FakeInboxView(); + const prefix = view.addMessages(3); const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(proposed), parentTotalMsgCount: 2n }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(prefix.inboxRollingHash), + endTotalMsgCount: 3n, + parentTotalMsgCount: 0n, + }), ); - // Bundle = leaves at global indices 2,3,4 (buckets 2 and 3), derived after resolving the parent bucket (seq 1). - expect(result).toEqual({ accepted: true, bundle: [new Fr(1002), new Fr(1003), new Fr(1004)] }); + expect(result).toEqual({ accepted: true, bundle: [new Fr(1000), new Fr(1001), new Fr(1002)] }); }); - it('rejects when the parent leaf count does not sit on a bucket boundary (padded legacy parent)', async () => { + it('derives the bundle spanning everything since the parent count', async () => { const view = new FakeInboxView(); - view.addBucket(1, 2, 100); // total 2 - const proposed = view.addBucket(2, 2, 100); // total 4 - // Parent leaf count 3 is between bucket boundaries (2 and 4): unresolvable. + view.addMessages(2); // parent consumed through here, total 2 + view.addMessages(2); // total 4 + const proposed = view.addMessages(1); // total 5 const result = await checkStreamingBlockProposal( - baseInput({ messageSource: view, bucketRef: refFor(proposed), parentTotalMsgCount: 3n }), + baseInput({ + messageSource: view, + bucketRef: new InboxBucketRef(proposed.inboxRollingHash), + endTotalMsgCount: 5n, + parentTotalMsgCount: 2n, + }), ); - expect(result).toEqual({ accepted: false, reason: 'parent_bucket_unresolved' }); + expect(result).toEqual({ accepted: true, bundle: [new Fr(1002), new Fr(1003), new Fr(1004)] }); }); }); describe('running-total accumulation across a checkpoint', () => { - it('accumulates the per-checkpoint total across three blocks against a fixed start', async () => { - // Checkpoint starts after bucket 1 (total 2). Three blocks each consume 2 messages: totals 4, 6, 8. + it('accumulates the per-checkpoint total across blocks against a fixed start', async () => { + // Checkpoint starts at total 2. Blocks consume 2 messages each: totals 4, 6, 8. const view = new FakeInboxView(); - view.addBucket(1, 2, 100); // checkpoint start total 2 - const b2 = view.addBucket(2, 2, 100); // total 4 - const b3 = view.addBucket(3, 2, 100); // total 6 - const b4 = view.addBucket(4, 2, 100); // total 8 + view.addMessages(2); // checkpoint start total 2 + const b2 = view.addMessages(2); // total 4 + view.addMessages(2); // total 6 + const b4 = view.addMessages(2); // total 8 const checkpointStart = 2n; - // Block 1 (parent = bucket 1): checkpoint delta 4 - 2 = 2. const r1 = await checkStreamingBlockProposal( baseInput({ messageSource: view, - bucketRef: refFor(b2), + bucketRef: new InboxBucketRef(b2.inboxRollingHash), + endTotalMsgCount: 4n, parentTotalMsgCount: 2n, checkpointStartTotalMsgCount: checkpointStart, perCheckpointCap: 6, @@ -261,11 +519,12 @@ describe('checkStreamingBlockProposal', () => { ); expect(r1.accepted).toBe(true); - // Block 3 (parent = bucket 3): checkpoint delta 8 - 2 = 6, exactly at the cap. + // Checkpoint delta 8 - 2 = 6, exactly at the cap. const r3 = await checkStreamingBlockProposal( baseInput({ messageSource: view, - bucketRef: refFor(b4), + bucketRef: new InboxBucketRef(b4.inboxRollingHash), + endTotalMsgCount: 8n, parentTotalMsgCount: 6n, checkpointStartTotalMsgCount: checkpointStart, perCheckpointCap: 6, @@ -277,14 +536,14 @@ describe('checkStreamingBlockProposal', () => { const r3Tight = await checkStreamingBlockProposal( baseInput({ messageSource: view, - bucketRef: refFor(b4), + bucketRef: new InboxBucketRef(b4.inboxRollingHash), + endTotalMsgCount: 8n, parentTotalMsgCount: 6n, checkpointStartTotalMsgCount: checkpointStart, perCheckpointCap: 5, }), ); expect(r3Tight).toEqual({ accepted: false, reason: 'checkpoint_over_msg_cap' }); - void b3; }); }); }); diff --git a/yarn-project/validator-client/src/streaming_inbox_checks.ts b/yarn-project/validator-client/src/streaming_inbox_checks.ts index 058ad2857989..512dfd85697f 100644 --- a/yarn-project/validator-client/src/streaming_inbox_checks.ts +++ b/yarn-project/validator-client/src/streaming_inbox_checks.ts @@ -1,29 +1,43 @@ import type { Fr } from '@aztec/foundation/curves/bn254'; -import type { InboxBucket, InboxBucketRef, L1ToL2MessageSource } from '@aztec/stdlib/messaging'; +import type { InboxBucketRef, L1ToL2MessageSource } from '@aztec/stdlib/messaging'; /** * Reason a streaming-Inbox block proposal fails the per-block acceptance checks. Follows the * handler's existing `{ isValid, reason }` string style. + * + * `prefix_unavailable` and `prefix_mismatch` are both local-view outcomes rather than proposer misbehavior, and the + * handler retries both once after forcing a local sync: this node can be the one holding the stale prefix. */ export type StreamingBlockCheckReason = - | 'bucket_unknown' - | 'parent_bucket_unresolved' - | 'bucket_moves_backwards' + | 'prefix_unavailable' + | 'prefix_mismatch' + | 'consumption_moves_backwards' | 'bundle_over_block_cap' | 'checkpoint_over_msg_cap'; -/** The subset of the archiver's Inbox-bucket queries the per-block streaming checks need. */ -export type StreamingInboxBucketSource = Pick< +/** Failure reasons that mean "this node cannot resolve the proposal's prefix", which a local sync may fix. */ +export const RETRYABLE_STREAMING_BLOCK_CHECK_REASONS: StreamingBlockCheckReason[] = [ + 'prefix_unavailable', + 'prefix_mismatch', +]; + +/** The subset of the archiver's Inbox queries the per-block streaming checks need. */ +export type StreamingInboxMessageSource = Pick< L1ToL2MessageSource, - 'getInboxBucketByRollingHash' | 'getInboxBucketByTotalMsgCount' | 'getL1ToL2MessagesBetweenBuckets' + 'getInboxRollingHashAt' | 'getL1ToL2MessagesBetweenLeafCounts' >; /** Inputs to the per-block streaming Inbox metadata checks. */ export type StreamingBlockMetadataCheckInput = { - /** Archiver Inbox-bucket queries (resolved against this node's own Inbox view). */ - messageSource: Pick; - /** The proposal's bucket reference: the rolling hash of the bucket the block consumed through. */ + /** Archiver Inbox prefix lookup (resolved against this node's own Inbox view). */ + messageSource: Pick; + /** + * The proposal's signed Inbox prefix reference: the rolling hash of the message prefix the block consumed through. + * Interpreted together with {@link endTotalMsgCount}; it need not name a current bucket boundary. + */ bucketRef: InboxBucketRef | undefined; + /** Cumulative Inbox message count consumed through this block, from its signed header's L1-to-L2 leaf count. */ + endTotalMsgCount: bigint; /** Cumulative Inbox message count consumed through the parent block (its L1-to-L2 tree leaf count; 0 at genesis). */ parentTotalMsgCount: bigint; /** Cumulative Inbox message count consumed as of the parent checkpoint; the per-checkpoint cap origin. */ @@ -36,23 +50,28 @@ export type StreamingBlockMetadataCheckInput = { /** Inputs to the full per-block streaming Inbox acceptance checks, metadata plus bundle derivation. */ export type StreamingBlockCheckInput = Omit & { - messageSource: StreamingInboxBucketSource; + messageSource: StreamingInboxMessageSource; }; -/** The buckets a passing block proposal consumes between; the input to bundle derivation. */ -export type StreamingBlockBucketRange = { - /** The bucket the block consumes through, resolved from this node's own Inbox view. */ - bucket: InboxBucket; - /** The bucket the parent block consumed through. Equal to `bucket` when the block consumes nothing. */ - parentBucket: InboxBucket; +/** The message-count range a passing block proposal consumes; the input to bundle derivation. */ +export type StreamingBlockCountRange = { + /** Cumulative Inbox message count the block's bundle starts at, inclusive. */ + parentTotalMsgCount: bigint; + /** Cumulative Inbox message count the block's bundle ends at, exclusive. Equal to the start for an empty bundle. */ + endTotalMsgCount: bigint; + /** + * The signed prefix reference the end count was confirmed against, carried through so the bundle read can + * re-confirm it without the caller having to prove again that the proposal had one. + */ + bucketRef: InboxBucketRef; }; /** Result of the per-block streaming Inbox metadata checks. */ export type StreamingBlockMetadataCheckResult = | ({ - /** Every metadata check passed; the block's bundle can be derived from the returned bucket range. */ + /** Every metadata check passed; the block's bundle can be derived from the returned count range. */ accepted: true; - } & StreamingBlockBucketRange) + } & StreamingBlockCountRange) | { /** A check failed; `reason` mirrors the L1 acceptance condition that would have rejected the proposal. */ accepted: false; @@ -75,102 +94,141 @@ export type StreamingBlockCheckResult = /** * Runs the metadata half of the per-block acceptance checks a validator applies to a streaming block proposal: * every check that is a bounded number of point lookups against the local Inbox view, with no message data read. - * Mirrors the L1 acceptance conditions: * - * 1. **Exists**: a bucket carrying the referenced rolling hash resolves in this node's own Inbox view. The hash - * commits to every message up to the bucket, so it is the bucket's identity: a bucket an L1 reorg re-timed or - * renumbered still resolves, and one whose messages changed does not. An unknown bucket is an immediate reject - * here; the caller decides whether it is worth waiting for a local sync and re-running the checks (the validator's - * proposal handler does, bounded by the attestation deadline), since "not synced yet" and "reorged away" look the - * same from here. Sequence number, timestamp and message counts are read from the locally resolved bucket, never - * from the wire. - * 2. **Moves forward**: the bucket's cumulative total is at least the parent block's, so consumption never rewinds. - * Equal totals mean the block consumes nothing (empty bundle). + * A block's consumed position is a pair of signed values: the end count from its header's L1-to-L2 leaf count, and + * the prefix rolling hash from the proposal's reference. Because each message's rolling hash chains the one before + * it, a matching hash at the end count proves the proposer consumed exactly the message prefix this node holds, so + * the bundle between the parent's count and this one is determined by content alone. The checks, in order: + * + * 1. **Reference present**: a streaming proposal must carry a prefix reference to authenticate against. + * 2. **Moves forward**: the end count is at least the parent block's, so consumption never rewinds. Equal counts + * mean the block consumes nothing (empty bundle), which still has its prefix hash checked. * 3. **Caps**: the per-block message count and the running per-checkpoint total fit their respective caps. - * 4. **Parent boundary**: the parent block's cumulative total sits on a bucket boundary, so the consumed range is - * well defined. + * 4. **Prefix matches**: the canonical prefix hash at the end count exists locally and equals the signed reference. + * + * Deliberately absent is any requirement that the end count sit on a boundary of the bucket partition this node + * currently holds. An L1 reorg that re-mines the same messages under different bucket boundaries leaves every leaf + * and every prefix hash intact while moving the boundaries, so a block that ended on a boundary the reorg merged + * away is still exactly as valid as when it was signed. Only a completed checkpoint's final position must resolve to + * a current bucket, because that is what the L1 `bucketHint` and the censorship/cap asserts are taken against. * - * There is deliberately no check on how recently the bucket was opened. *When* a bucket becomes consumable is the - * proposer's own policy — it waits for the bucket's opening L1 block to gain a canonical descendant so it does not - * build on a block its archiver will disown — and L1 has no matching rule: `propose` accepts any bucket the - * censorship cutoff and the caps allow, whenever it is proposed. A validator that rejected a young bucket would - * therefore refuse to attest to a checkpoint L1 accepts, so validators check only what L1 checks plus consistency - * with their own Inbox view. + * There is likewise no check on how recently a bucket was opened. *When* messages become consumable is the + * proposer's own policy — it waits for the opening L1 block to gain a canonical descendant so it does not build on a + * block its archiver will disown — and L1 has no matching rule: `propose` accepts any position the censorship cutoff + * and the caps allow, whenever it is proposed. A validator that rejected a young bucket would therefore refuse to + * attest to a checkpoint L1 accepts, so validators check only what L1 checks plus consistency with their own view. * * Because this phase is cheap and needs nothing off the network, a caller can run it before committing to any * expensive work on a proposal — notably before collecting the proposal's transactions over P2P. * - * The reject branch is a single function so a caller can re-run the whole check after forcing a local sync, which - * is how the validator's proposal handler turns a `bucket_unknown` into a bounded wait. + * The reject branch is a single function so a caller can re-run the whole check after forcing a local sync, which is + * how the validator's proposal handler turns either of the two prefix outcomes into a bounded wait. Both are + * retried: a node whose archiver has not yet followed a reorg holds a *present* prefix hash at the end count that + * simply is not the canonical one any more, which is indistinguishable from a proposer naming a prefix that never + * existed. */ export async function checkStreamingBlockProposalMetadata( input: StreamingBlockMetadataCheckInput, ): Promise { - const { messageSource, bucketRef, parentTotalMsgCount, checkpointStartTotalMsgCount, perBlockCap, perCheckpointCap } = - input; - - // A streaming proposal must carry a bucket reference to derive its bundle from. + const { + messageSource, + bucketRef, + endTotalMsgCount, + parentTotalMsgCount, + checkpointStartTotalMsgCount, + perBlockCap, + perCheckpointCap, + } = input; + + // Check 1: a streaming proposal must carry a prefix reference to authenticate its consumed range against. if (bucketRef === undefined) { - return { accepted: false, reason: 'bucket_unknown' }; - } - - // Check 1: a bucket carrying the referenced rolling hash exists in our own Inbox view. - const bucket = await messageSource.getInboxBucketByRollingHash(bucketRef.inboxRollingHash); - if (bucket === undefined) { - return { accepted: false, reason: 'bucket_unknown' }; + return { accepted: false, reason: 'prefix_unavailable' }; } // Check 2: consumption moves forward relative to the parent block. - if (bucket.totalMsgCount < parentTotalMsgCount) { - return { accepted: false, reason: 'bucket_moves_backwards' }; + if (endTotalMsgCount < parentTotalMsgCount) { + return { accepted: false, reason: 'consumption_moves_backwards' }; } // Check 3a: the per-block message count fits the per-block cap. - const blockCount = bucket.totalMsgCount - parentTotalMsgCount; - if (blockCount > BigInt(perBlockCap)) { + if (endTotalMsgCount - parentTotalMsgCount > BigInt(perBlockCap)) { return { accepted: false, reason: 'bundle_over_block_cap' }; } // Check 3b: the running per-checkpoint total fits the per-checkpoint cap. - const checkpointCount = bucket.totalMsgCount - checkpointStartTotalMsgCount; - if (checkpointCount > BigInt(perCheckpointCap)) { + if (endTotalMsgCount - checkpointStartTotalMsgCount > BigInt(perCheckpointCap)) { return { accepted: false, reason: 'checkpoint_over_msg_cap' }; } - // Check 4: the parent bucket is the one whose cumulative total equals the parent block's leaf count (messages are - // indexed compactly, with no padding); a parent whose count does not sit on a bucket boundary is unresolvable. - const parentBucket = await messageSource.getInboxBucketByTotalMsgCount(parentTotalMsgCount); - if (parentBucket === undefined) { - return { accepted: false, reason: 'parent_bucket_unresolved' }; + // Check 4: the canonical prefix at the signed end count hashes to the signed reference. An empty block is checked + // here too: its unchanged count must still name the prefix the proposer signed, so an empty range is never a + // licence to skip the hash. + const canonicalHash = await messageSource.getInboxRollingHashAt(endTotalMsgCount); + if (canonicalHash === undefined) { + return { accepted: false, reason: 'prefix_unavailable' }; + } + if (!canonicalHash.equals(bucketRef.inboxRollingHash)) { + return { accepted: false, reason: 'prefix_mismatch' }; } - return { accepted: true, bucket, parentBucket }; + return { accepted: true, parentTotalMsgCount, endTotalMsgCount, bucketRef }; } /** - * Derives the message-leaf bundle a streaming block proposal consumes (for re-execution): the leaves between the - * parent bucket and the proposed one, bounded by the per-block cap that - * {@link checkStreamingBlockProposalMetadata} already enforced. + * Reads the message-leaf bundle a streaming block proposal consumes (for re-execution) and re-confirms the signed + * prefix hash *after* the read, so the leaves handed to re-execution and the confirmed prefix come from the same + * stable view of the local Inbox. + * + * The re-confirmation is what makes this safe to call after + * {@link checkStreamingBlockProposalMetadata}. That check is a point lookup, and an L1 reorg can commit a + * content-changing message replacement between it and this read. Without the second check the range would either + * return the *new* leaves — making re-execution report a `state_mismatch` against a proposal that was valid on the + * view it was checked against, which is a slashable verdict for an honest proposer — or fail outright after a + * dropped suffix. Reading first and confirming second catches both: a replacement landing on either side of the + * read leaves the prefix hash at the end count no longer matching the signed reference. + * + * A range the source cannot serve whole is reported as `prefix_unavailable` rather than thrown, since it is the same + * local-view condition as a missing prefix hash and the caller retries both the same way. + * + * A replacement that lands *after* the confirmation 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. */ -export function getStreamingBlockBundle( - messageSource: Pick, - range: StreamingBlockBucketRange, -): Promise { - const { bucket, parentBucket } = range; - return parentBucket.seq === bucket.seq - ? Promise.resolve([]) - : messageSource.getL1ToL2MessagesBetweenBuckets(parentBucket.seq, bucket.seq); +export async function readStreamingBlockBundle( + messageSource: StreamingInboxMessageSource, + range: StreamingBlockCountRange, +): Promise { + const { parentTotalMsgCount, endTotalMsgCount, bucketRef } = range; + + let bundle: Fr[]; + try { + bundle = + endTotalMsgCount === parentTotalMsgCount + ? [] + : await messageSource.getL1ToL2MessagesBetweenLeafCounts(parentTotalMsgCount, endTotalMsgCount); + } catch { + return { accepted: false, reason: 'prefix_unavailable' }; + } + + const canonicalHash = await messageSource.getInboxRollingHashAt(endTotalMsgCount); + if (canonicalHash === undefined) { + return { accepted: false, reason: 'prefix_unavailable' }; + } + if (!canonicalHash.equals(bucketRef.inboxRollingHash)) { + return { accepted: false, reason: 'prefix_mismatch' }; + } + + return { accepted: true, bundle }; } /** * 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 - * {@link getStreamingBlockBundle} for callers that have no use for running the two phases separately. + * {@link readStreamingBlockBundle} for callers that have no use for running the two phases separately. */ export async function checkStreamingBlockProposal(input: StreamingBlockCheckInput): Promise { const metadata = await checkStreamingBlockProposalMetadata(input); if (!metadata.accepted) { return metadata; } - return { accepted: true, bundle: await getStreamingBlockBundle(input.messageSource, metadata) }; + return readStreamingBlockBundle(input.messageSource, metadata); } diff --git a/yarn-project/validator-client/src/validator.integration.test.ts b/yarn-project/validator-client/src/validator.integration.test.ts index 3d3922dad677..63bf56d59974 100644 --- a/yarn-project/validator-client/src/validator.integration.test.ts +++ b/yarn-project/validator-client/src/validator.integration.test.ts @@ -260,13 +260,13 @@ describe('ValidatorClient Integration', () => { l1ToL2Messages, }); - // Resolve the Inbox bucket this block consumed through (keyed by its cumulative L1-to-L2 leaf count) and attach - // the reference, mirroring the sequencer's block-building loop which carries a bucketRef on every proposal. - // Without it the validator's streaming acceptance check rejects the proposal as - // `bucket_unknown`. A block that consumed nothing resolves to the genesis (or reused parent) bucket. + // Resolve the Inbox message prefix this block consumed through (keyed by its cumulative L1-to-L2 leaf count) and + // attach the reference, mirroring the sequencer's block-building loop which carries a bucketRef on every + // proposal. Without it the validator's streaming acceptance check rejects the proposal as `prefix_unavailable`. + // A block that consumed nothing resolves to the genesis zero hash. const blockTotal = BigInt(block.header.state.l1ToL2MessageTree.nextAvailableLeafIndex); - const bucket = await proposer.archiver.getInboxBucketByTotalMsgCount(blockTotal); - const bucketRef = bucket ? InboxBucketRef.fromBucket(bucket) : undefined; + const inboxRollingHash = await proposer.archiver.getInboxRollingHashAt(blockTotal); + const bucketRef = inboxRollingHash ? new InboxBucketRef(inboxRollingHash) : undefined; const proposal = await proposer.validator.createBlockProposal( block.header, diff --git a/yarn-project/validator-client/src/validator.test.ts b/yarn-project/validator-client/src/validator.test.ts index 2cd934076242..159c713781c8 100644 --- a/yarn-project/validator-client/src/validator.test.ts +++ b/yarn-project/validator-client/src/validator.test.ts @@ -372,19 +372,35 @@ describe('ValidatorClient', () => { const targetSlotStart = Number(l1GenesisTime) + Number(targetSlot) * slotDuration; return new Date((targetSlotStart + slotDuration - 2 * ethereumSlotDuration) * 1000); }; + /** + * A block header consuming nothing: leaf count 0. Paired with a checkpoint header committing to the zero + * rolling hash, that makes the streaming per-block and final-endpoint checks self-consistent against the + * genesis Inbox this suite mocks, leaving each test's own subject as what decides the outcome. + */ + const makeConsumeNothingBlockHeader = (overrides: Parameters[1]) => { + const header = makeBlockHeader(1, overrides); + header.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot(Fr.random(), 0); + return header; + }; + const makeConsumeNothingCheckpointHeader = (overrides: Parameters[1] = {}) => + makeCheckpointHeader(0, { inboxRollingHash: Fr.ZERO, ...overrides }); + const makeCheckpointProposalForSlot = () => makeCheckpointProposal({ archiveRoot: proposal.archive, - checkpointHeader: makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }), + checkpointHeader: makeConsumeNothingCheckpointHeader({ slotNumber: proposal.slotNumber }), lastBlock: { - blockHeader: makeBlockHeader(1, { blockNumber: BlockNumber(123), slotNumber: proposal.slotNumber }), + blockHeader: makeConsumeNothingBlockHeader({ + blockNumber: BlockNumber(123), + slotNumber: proposal.slotNumber, + }), indexWithinCheckpoint: IndexWithinCheckpoint(0), txHashes: proposal.txHashes, }, }); const makeCheckpointProposalWithHeaderMismatch = async () => { - const proposalHeader = makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }); - const computedHeader = makeCheckpointHeader(0, { + const proposalHeader = makeConsumeNothingCheckpointHeader({ slotNumber: proposal.slotNumber }); + const computedHeader = makeConsumeNothingCheckpointHeader({ slotNumber: proposal.slotNumber, totalManaUsed: new Fr(999), }); @@ -392,7 +408,7 @@ describe('ValidatorClient', () => { archiveRoot: proposal.archive, checkpointHeader: proposalHeader, lastBlock: { - blockHeader: makeBlockHeader(1, { blockNumber, slotNumber: proposal.slotNumber }), + blockHeader: makeConsumeNothingBlockHeader({ blockNumber, slotNumber: proposal.slotNumber }), indexWithinCheckpoint: IndexWithinCheckpoint(0), txHashes: proposal.txHashes, }, @@ -400,7 +416,7 @@ describe('ValidatorClient', () => { const checkpointBlock = { ...blockBuildResult.block, number: blockNumber, - header: makeBlockHeader(1, { blockNumber, slotNumber: proposal.slotNumber }), + header: makeConsumeNothingBlockHeader({ blockNumber, slotNumber: proposal.slotNumber }), archive: new AppendOnlyTreeSnapshot(proposal.archive, blockNumber), checkpointNumber: CheckpointNumber(1), } as unknown as L2Block; @@ -472,6 +488,8 @@ describe('ValidatorClient', () => { beforeEach(async () => { const blockHeader = makeBlockHeader(1, { blockNumber: BlockNumber(100), slotNumber: SlotNumber(100) }); + // The block consumes nothing, so its signed end count is 0 and its prefix reference is the genesis zero hash. + blockHeader.state.l1ToL2MessageTree = new AppendOnlyTreeSnapshot(Fr.random(), 0); blockNumber = BlockNumber(blockHeader.globalVariables.blockNumber); proposal = ValidatedBlockProposal(await makeBlockProposal({ blockHeader, bucketRef: genesisBucketRef })); // The proposal targets slot 100, which under pipelining is built during the previous slot. Set the @@ -540,18 +558,28 @@ describe('ValidatorClient', () => { checkpointNumber: CheckpointNumber(1), indexWithinCheckpoint: IndexWithinCheckpoint(0), } as unknown as BlockData; + // Archive lookups resolve the parent; number lookups do not, so the block-number collision guard sees a free + // slot at this block's number. The one exception is the parent's own number, which the checkpoint path reads + // to find where the checkpoint's consumption started — without it that check cannot resolve at all. blockSource.getBlockData.mockImplementation(query => - Promise.resolve('number' in query ? undefined : parentBlockData), + Promise.resolve(!('number' in query) || query.number === blockNumber - 1 ? parentBlockData : undefined), ); blockSource.getGenesisValues.mockResolvedValue({ genesisArchiveRoot: new Fr(GENESIS_ARCHIVE_ROOT) }); blockSource.syncImmediate.mockImplementation(() => Promise.resolve()); - // Resolve every Inbox bucket query to the genesis bucket, so streaming checks accept with an empty bundle. - l1ToL2MessageSource.getInboxBucket.mockResolvedValue(genesisInboxBucket); + // An Inbox that has absorbed nothing: the prefix at count 0 is the zero hash and count 0 resolves to the + // genesis sentinel bucket, so the streaming checks accept with an empty bundle. + // Only the genesis sentinel exists, so nothing is left unconsumed and the censorship floor is satisfied. + l1ToL2MessageSource.getInboxBucket.mockImplementation(seq => + Promise.resolve(seq === 0n ? genesisInboxBucket : undefined), + ); l1ToL2MessageSource.getInboxBucketByRollingHash.mockResolvedValue(genesisInboxBucket); - l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockResolvedValue(genesisInboxBucket); - l1ToL2MessageSource.getL1ToL2MessagesBetweenBuckets.mockResolvedValue([]); + l1ToL2MessageSource.getInboxBucketByTotalMsgCount.mockImplementation(total => + Promise.resolve(total === 0n ? genesisInboxBucket : undefined), + ); + l1ToL2MessageSource.getInboxRollingHashAt.mockResolvedValue(Fr.ZERO); + l1ToL2MessageSource.getL1ToL2MessagesBetweenLeafCounts.mockResolvedValue([]); const clonedBlockHeader = blockHeader.clone(); blockBuildResult = { @@ -705,10 +733,7 @@ describe('ValidatorClient', () => { const futureSlot = SlotNumber(proposal.slotNumber + 20); const futureProposal = ValidatedBlockProposal( await makeBlockProposal({ - blockHeader: makeBlockHeader(1, { - blockNumber, - slotNumber: futureSlot, - }), + blockHeader: makeConsumeNothingBlockHeader({ blockNumber, slotNumber: futureSlot }), bucketRef: genesisBucketRef, }), );