Skip to content

Commit

Permalink
fix: Archiver does not jump the gun on epoch completed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
spalladino committed Dec 18, 2024
1 parent 96887b6 commit edc7e87
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 16 deletions.
112 changes: 104 additions & 8 deletions yarn-project/archiver/src/archiver/archiver.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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}`]>;
}

Expand All @@ -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>;
Expand All @@ -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
Expand All @@ -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));
Expand Down Expand Up @@ -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', () => {});

Expand Down
19 changes: 14 additions & 5 deletions yarn-project/archiver/src/archiver/archiver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,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);
}
Expand Down Expand Up @@ -567,17 +571,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;
}

Expand Down
28 changes: 28 additions & 0 deletions yarn-project/circuit-types/src/epoch-helpers/index.test.ts
Original file line number Diff line number Diff line change
@@ -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));
});
});
14 changes: 11 additions & 3 deletions yarn-project/circuit-types/src/epoch-helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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),
];
}

Expand Down
1 change: 1 addition & 0 deletions yarn-project/end-to-end/src/e2e_epochs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

logger.info(`L2 genesis at L1 block ${constants.l1GenesisBlock} (timestamp ${constants.l1GenesisTime})`);
Expand Down

0 comments on commit edc7e87

Please sign in to comment.