From 51d82aaf62b84885eb388b1565c1a119c60d849e Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Wed, 18 Dec 2024 15:37:36 -0300 Subject: [PATCH] fix: Archiver does not jump the gun on epoch completed (#10801) Archiver's isEpochComplete would return `true` when the first L1 block of the last epoch L2 slot was mined, ignoring the fact that the L2 block could be posted on a later L1 slot. This fixes the check so it reports the epoch as completed only once the last L1 block of the last L2 slot of the epoch is mined. Fixes #10800 --- .../archiver/src/archiver/archiver.test.ts | 112 ++++++++++++++++-- .../archiver/src/archiver/archiver.ts | 19 ++- .../src/epoch-helpers/index.test.ts | 28 +++++ .../circuit-types/src/epoch-helpers/index.ts | 14 ++- .../end-to-end/src/e2e_epochs.test.ts | 1 + 5 files changed, 158 insertions(+), 16 deletions(-) create mode 100644 yarn-project/circuit-types/src/epoch-helpers/index.test.ts diff --git a/yarn-project/archiver/src/archiver/archiver.test.ts b/yarn-project/archiver/src/archiver/archiver.test.ts index 79ca76e7cd3..305a89874f0 100644 --- a/yarn-project/archiver/src/archiver/archiver.test.ts +++ b/yarn-project/archiver/src/archiver/archiver.test.ts @@ -1,9 +1,10 @@ -import { InboxLeaf, L2Block } from '@aztec/circuit-types'; +import { InboxLeaf, type L1RollupConstants, L2Block } from '@aztec/circuit-types'; import { GENESIS_ARCHIVE_ROOT, PrivateLog } from '@aztec/circuits.js'; import { DefaultL1ContractsConfig } from '@aztec/ethereum'; import { Blob } from '@aztec/foundation/blob'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; +import { type Logger, createLogger } from '@aztec/foundation/log'; import { sleep } from '@aztec/foundation/sleep'; import { type InboxAbi, RollupAbi } from '@aztec/l1-artifacts'; import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; @@ -26,7 +27,9 @@ import { type ArchiverInstrumentation } from './instrumentation.js'; import { MemoryArchiverStore } from './memory_archiver_store/memory_archiver_store.js'; interface MockRollupContractRead { + /** Given an L2 block number, returns the archive. */ archiveAt: (args: readonly [bigint]) => Promise<`0x${string}`>; + /** Given an L2 block number, returns provenBlockNumber, provenArchive, pendingBlockNumber, pendingArchive, archiveForLocalPendingBlockNumber, provenEpochNumber. */ status: (args: readonly [bigint]) => Promise<[bigint, `0x${string}`, bigint, `0x${string}`, `0x${string}`]>; } @@ -52,6 +55,7 @@ describe('Archiver', () => { let instrumentation: MockProxy; let archiverStore: ArchiverDataStore; let now: number; + let l1Constants: L1RollupConstants; let rollupRead: MockProxy; let inboxRead: MockProxy; @@ -61,9 +65,12 @@ describe('Archiver', () => { let l2BlockProposedLogs: Log[]; let l2MessageSentLogs: Log[]; + let logger: Logger; + const GENESIS_ROOT = new Fr(GENESIS_ARCHIVE_ROOT).toString(); beforeEach(() => { + logger = createLogger('archiver:test'); now = +new Date(); publicClient = mock>({ // Return a block with a reasonable timestamp @@ -89,6 +96,13 @@ describe('Archiver', () => { const tracer = new NoopTelemetryClient().getTracer(); instrumentation = mock({ isEnabled: () => true, tracer }); archiverStore = new MemoryArchiverStore(1000); + l1Constants = { + l1GenesisTime: BigInt(now), + l1StartBlock: 0n, + epochDuration: 4, + slotDuration: 24, + ethereumSlotDuration: 12, + }; archiver = new Archiver( publicClient, @@ -96,13 +110,7 @@ describe('Archiver', () => { archiverStore, { pollingIntervalMs: 1000, batchSize: 1000 }, instrumentation, - { - l1GenesisTime: BigInt(now), - l1StartBlock: 0n, - epochDuration: 4, - slotDuration: 24, - ethereumSlotDuration: 12, - }, + l1Constants, ); blocks = blockNumbers.map(x => L2Block.random(x, txsPerBlock, x + 1, 2)); @@ -369,6 +377,94 @@ describe('Archiver', () => { expect((await archiver.getContractClassLogs({ fromBlock: 2, toBlock: 3 })).logs).toEqual([]); }, 10_000); + it('reports an epoch as pending if the current L2 block is not in the last slot of the epoch', async () => { + const { l1StartBlock, slotDuration, ethereumSlotDuration, epochDuration } = l1Constants; + const notLastL2SlotInEpoch = epochDuration - 2; + const l1BlockForL2Block = l1StartBlock + BigInt((notLastL2SlotInEpoch * slotDuration) / ethereumSlotDuration); + expect(notLastL2SlotInEpoch).toEqual(2); + + logger.info(`Syncing L2 block on slot ${notLastL2SlotInEpoch} mined in L1 block ${l1BlockForL2Block}`); + const l2Block = blocks[0]; + l2Block.header.globalVariables.slotNumber = new Fr(notLastL2SlotInEpoch); + blocks = [l2Block]; + + const rollupTxs = blocks.map(makeRollupTx); + publicClient.getBlockNumber.mockResolvedValueOnce(l1BlockForL2Block); + rollupRead.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]); + makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString()); + rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx)); + + await archiver.start(false); + + // Epoch should not yet be complete + expect(await archiver.isEpochComplete(0n)).toBe(false); + + // Wait until block 1 is processed + while ((await archiver.getBlockNumber()) !== 1) { + await sleep(100); + } + + // Epoch should not be complete + expect(await archiver.isEpochComplete(0n)).toBe(false); + }); + + it('reports an epoch as complete if the current L2 block is in the last slot of the epoch', async () => { + const { l1StartBlock, slotDuration, ethereumSlotDuration, epochDuration } = l1Constants; + const lastL2SlotInEpoch = epochDuration - 1; + const l1BlockForL2Block = l1StartBlock + BigInt((lastL2SlotInEpoch * slotDuration) / ethereumSlotDuration); + expect(lastL2SlotInEpoch).toEqual(3); + + logger.info(`Syncing L2 block on slot ${lastL2SlotInEpoch} mined in L1 block ${l1BlockForL2Block}`); + const l2Block = blocks[0]; + l2Block.header.globalVariables.slotNumber = new Fr(lastL2SlotInEpoch); + blocks = [l2Block]; + + const rollupTxs = blocks.map(makeRollupTx); + publicClient.getBlockNumber.mockResolvedValueOnce(l1BlockForL2Block); + rollupRead.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]); + makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString()); + rollupTxs.forEach(tx => publicClient.getTransaction.mockResolvedValueOnce(tx)); + + await archiver.start(false); + + // Epoch should not yet be complete + expect(await archiver.isEpochComplete(0n)).toBe(false); + + // Wait until block 1 is processed + while ((await archiver.getBlockNumber()) !== 1) { + await sleep(100); + } + + // Epoch should be complete once block was synced + expect(await archiver.isEpochComplete(0n)).toBe(true); + }); + + it('reports an epoch as pending if the current L1 block is not the last one on the epoch and no L2 block landed', async () => { + const { l1StartBlock, slotDuration, ethereumSlotDuration, epochDuration } = l1Constants; + const notLastL1BlockForEpoch = l1StartBlock + BigInt((epochDuration * slotDuration) / ethereumSlotDuration) - 2n; + expect(notLastL1BlockForEpoch).toEqual(6n); + + logger.info(`Syncing archiver to L1 block ${notLastL1BlockForEpoch}`); + publicClient.getBlockNumber.mockResolvedValueOnce(notLastL1BlockForEpoch); + rollupRead.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 0n, GENESIS_ROOT, GENESIS_ROOT]); + + await archiver.start(true); + expect(await archiver.isEpochComplete(0n)).toBe(false); + }); + + it('reports an epoch as complete if the current L1 block is the last one on the epoch and no L2 block landed', async () => { + const { l1StartBlock, slotDuration, ethereumSlotDuration, epochDuration } = l1Constants; + const lastL1BlockForEpoch = l1StartBlock + BigInt((epochDuration * slotDuration) / ethereumSlotDuration) - 1n; + expect(lastL1BlockForEpoch).toEqual(7n); + + logger.info(`Syncing archiver to L1 block ${lastL1BlockForEpoch}`); + publicClient.getBlockNumber.mockResolvedValueOnce(lastL1BlockForEpoch); + rollupRead.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 0n, GENESIS_ROOT, GENESIS_ROOT]); + + await archiver.start(true); + expect(await archiver.isEpochComplete(0n)).toBe(true); + }); + // TODO(palla/reorg): Add a unit test for the archiver handleEpochPrune xit('handles an upcoming L2 prune', () => {}); diff --git a/yarn-project/archiver/src/archiver/archiver.ts b/yarn-project/archiver/src/archiver/archiver.ts index c116f738515..ab59cca5420 100644 --- a/yarn-project/archiver/src/archiver/archiver.ts +++ b/yarn-project/archiver/src/archiver/archiver.ts @@ -507,6 +507,10 @@ export class Archiver implements ArchiveSource, Traceable { return Promise.resolve(); } + public getL1Constants(): L1RollupConstants { + return this.l1constants; + } + public getRollupAddress(): Promise { return Promise.resolve(this.l1Addresses.rollupAddress); } @@ -566,17 +570,22 @@ export class Archiver implements ArchiveSource, Traceable { return true; } + // If we haven't run an initial sync, just return false. + const l1Timestamp = this.l1Timestamp; + if (l1Timestamp === undefined) { + return false; + } + // If not, the epoch may also be complete if the L2 slot has passed without a block - // We compute this based on the timestamp for the given epoch and the timestamp of the last L1 block - const l1Timestamp = this.getL1Timestamp(); + // We compute this based on the end timestamp for the given epoch and the timestamp of the last L1 block const [_startTimestamp, endTimestamp] = getTimestampRangeForEpoch(epochNumber, this.l1constants); // For this computation, we throw in a few extra seconds just for good measure, // since we know the next L1 block won't be mined within this range. Remember that - // l1timestamp is the timestamp of the last l1 block we've seen, so this 3s rely on - // the fact that L1 won't mine two blocks within 3s of each other. + // l1timestamp is the timestamp of the last l1 block we've seen, so this relies on + // the fact that L1 won't mine two blocks within this time of each other. // TODO(palla/reorg): Is the above a safe assumption? - const leeway = 3n; + const leeway = 1n; return l1Timestamp + leeway >= endTimestamp; } diff --git a/yarn-project/circuit-types/src/epoch-helpers/index.test.ts b/yarn-project/circuit-types/src/epoch-helpers/index.test.ts new file mode 100644 index 00000000000..4fa879b7f6c --- /dev/null +++ b/yarn-project/circuit-types/src/epoch-helpers/index.test.ts @@ -0,0 +1,28 @@ +import { type EpochConstants, getTimestampRangeForEpoch } from './index.js'; + +describe('EpochHelpers', () => { + let constants: EpochConstants; + const l1GenesisTime = 1734440000n; + + beforeEach(() => { + constants = { + l1GenesisBlock: 10n, + l1GenesisTime: l1GenesisTime, + epochDuration: 4, + slotDuration: 24, + ethereumSlotDuration: 12, + }; + }); + + it('returns timestamp range for initial epoch', () => { + const [start, end] = getTimestampRangeForEpoch(0n, constants); + expect(start).toEqual(l1GenesisTime); + expect(end).toEqual(l1GenesisTime + BigInt(24 * 3 + 12)); + }); + + it('returns timestamp range for second epoch', () => { + const [start, end] = getTimestampRangeForEpoch(1n, constants); + expect(start).toEqual(l1GenesisTime + BigInt(24 * 4)); + expect(end).toEqual(l1GenesisTime + BigInt(24 * 4) + BigInt(24 * 3 + 12)); + }); +}); diff --git a/yarn-project/circuit-types/src/epoch-helpers/index.ts b/yarn-project/circuit-types/src/epoch-helpers/index.ts index 50c698e1160..90a22f1a4b5 100644 --- a/yarn-project/circuit-types/src/epoch-helpers/index.ts +++ b/yarn-project/circuit-types/src/epoch-helpers/index.ts @@ -14,11 +14,13 @@ export const EmptyL1RollupConstants: L1RollupConstants = { ethereumSlotDuration: 1, }; +// REFACTOR: Merge this type with L1RollupConstants export type EpochConstants = { l1GenesisBlock: bigint; l1GenesisTime: bigint; epochDuration: number; slotDuration: number; + ethereumSlotDuration: number; }; /** Returns the slot number for a given timestamp. */ @@ -40,15 +42,21 @@ export function getSlotRangeForEpoch(epochNumber: bigint, constants: Pick, + constants: Pick, ) { const [startSlot, endSlot] = getSlotRangeForEpoch(epochNumber, constants); + const ethereumSlotsPerL2Slot = constants.slotDuration / constants.ethereumSlotDuration; return [ constants.l1GenesisTime + startSlot * BigInt(constants.slotDuration), - constants.l1GenesisTime + endSlot * BigInt(constants.slotDuration), + constants.l1GenesisTime + + endSlot * BigInt(constants.slotDuration) + + BigInt((ethereumSlotsPerL2Slot - 1) * constants.ethereumSlotDuration), ]; } diff --git a/yarn-project/end-to-end/src/e2e_epochs.test.ts b/yarn-project/end-to-end/src/e2e_epochs.test.ts index 6a5e89dfe77..539cd573cc0 100644 --- a/yarn-project/end-to-end/src/e2e_epochs.test.ts +++ b/yarn-project/end-to-end/src/e2e_epochs.test.ts @@ -94,6 +94,7 @@ describe('e2e_epochs', () => { slotDuration: L1_BLOCK_TIME_IN_S * L2_SLOT_DURATION_IN_L1_SLOTS, l1GenesisBlock: await rollup.getL1StartBlock(), l1GenesisTime: await rollup.getL1GenesisTime(), + ethereumSlotDuration: L1_BLOCK_TIME_IN_S, }; logger.info(`L2 genesis at L1 block ${constants.l1GenesisBlock} (timestamp ${constants.l1GenesisTime})`);