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
AztecProtocol · Dec 18, 2024 · 51d82aa · 51d82aa
1 parent b721253
commit 51d82aa
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 16 deletions.
diff --git 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<ArchiverInstrumentation>;
   let archiverStore: ArchiverDataStore;
   let now: number;
+  let l1Constants: L1RollupConstants;
 
   let rollupRead: MockProxy<MockRollupContractRead>;
   let inboxRead: MockProxy<MockInboxContractRead>;
@@ -61,9 +65,12 @@ describe('Archiver', () => {
   let l2BlockProposedLogs: Log<bigint, number, false, undefined, true, typeof RollupAbi, 'L2BlockProposed'>[];
   let l2MessageSentLogs: Log<bigint, number, false, undefined, true, typeof InboxAbi, 'MessageSent'>[];
 
+  let logger: Logger;
+
   const GENESIS_ROOT = new Fr(GENESIS_ARCHIVE_ROOT).toString();
 
   beforeEach(() => {
+    logger = createLogger('archiver:test');
     now = +new Date();
     publicClient = mock<PublicClient<HttpTransport, Chain>>({
       // Return a block with a reasonable timestamp
@@ -89,20 +96,21 @@ describe('Archiver', () => {
     const tracer = new NoopTelemetryClient().getTracer();
     instrumentation = mock<ArchiverInstrumentation>({ isEnabled: () => true, tracer });
     archiverStore = new MemoryArchiverStore(1000);
+    l1Constants = {
+      l1GenesisTime: BigInt(now),
+      l1StartBlock: 0n,
+      epochDuration: 4,
+      slotDuration: 24,
+      ethereumSlotDuration: 12,
+    };
 
     archiver = new Archiver(
       publicClient,
       { rollupAddress, inboxAddress, registryAddress },
       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
@@ -507,6 +507,10 @@ export class Archiver implements ArchiveSource, Traceable {
     return Promise.resolve();
   }
 
+  public getL1Constants(): L1RollupConstants {
+    return this.l1constants;
+  }
+
   public getRollupAddress(): Promise<EthAddress> {
     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
@@ -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
@@ -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<EpochC
   return [startSlot, startSlot + BigInt(constants.epochDuration) - 1n];
 }
 
-/** Returns the range of L1 timestamps (inclusive) for a given epoch number. */
+/**
+ * Returns the range of L1 timestamps (inclusive) for a given epoch number.
+ * Note that the endTimestamp is the start timestamp of the last L1 slot for the epoch.
+ */
 export function getTimestampRangeForEpoch(
   epochNumber: bigint,
-  constants: Pick<EpochConstants, 'l1GenesisTime' | 'slotDuration' | 'epochDuration'>,
+  constants: Pick<EpochConstants, 'l1GenesisTime' | 'slotDuration' | 'epochDuration' | 'ethereumSlotDuration'>,
 ) {
   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
@@ -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})`);