Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ describe('CheckpointAttestationValidator', () => {

it('accepts attestation for current slot inside the straggler window', async () => {
// Attestation is for slot 98 (current wallclock slot), but targetSlot is 99 (pipelining).
// attestationWindowIntoTargetSlot = 2*p2p = 4s ⇒ straggler grace 4s+500ms disparity.
// attestationWindowIntoTargetSlot now spans target-slot start to the L1 publish deadline:
// aztecSlotDuration - 2*ethereumSlotDuration = 72 - 24 = 48s ⇒ straggler grace 48s+500ms.
const header = CheckpointHeader.random({ slotNumber: SlotNumber(98) });
const mockAttestation = makeCheckpointAttestation({
header,
Expand All @@ -119,7 +120,39 @@ describe('CheckpointAttestationValidator', () => {
epoch: EpochNumber(1),
slot: SlotNumber(98),
ts: 1000n,
nowMs: 1003000n, // 3000ms elapsed, within 4500ms straggler grace
nowMs: 1003000n, // 3000ms elapsed, within 48500ms straggler grace
});
epochCache.isInCommittee.mockResolvedValue(true);
epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(proposer.address);

const result = await validator.validate(mockAttestation);
expect(result).toEqual({ result: 'accept' });
});

it('accepts attestation arriving well into the target slot (previously rejected past ~4.5s)', async () => {
// 30s into the target slot — past the old 4.5s window, well within the new 48.5s window.
const header = CheckpointHeader.random({ slotNumber: SlotNumber(98) });
const mockAttestation = makeCheckpointAttestation({
header,
attesterSigner: attester,
proposerSigner: proposer,
});

epochCache.getTargetAndNextSlot.mockReturnValue({
targetSlot: SlotNumber(99),
nextSlot: SlotNumber(100),
});
epochCache.getSlotNow.mockReturnValue(SlotNumber(98));
epochCache.getL1Constants.mockReturnValue({
slotDuration: 72,
ethereumSlotDuration: 12,
} as any);

epochCache.getEpochAndSlotNow.mockReturnValue({
epoch: EpochNumber(1),
slot: SlotNumber(98),
ts: 1000n,
nowMs: 1030000n, // 30s elapsed — accepted now, would have been rejected pre-fix
});
epochCache.isInCommittee.mockResolvedValue(true);
epochCache.getProposerAttesterAddressInSlot.mockResolvedValue(proposer.address);
Expand Down Expand Up @@ -151,7 +184,7 @@ describe('CheckpointAttestationValidator', () => {
epoch: EpochNumber(1),
slot: SlotNumber(99),
ts: 1000n,
nowMs: 1005000n, // 5000ms elapsed, past 4500ms straggler cutoff
nowMs: 1049000n, // 49s elapsed, past the 48.5s straggler cutoff (48s window + 500ms disparity)
});
epochCache.isInCommittee.mockResolvedValue(true);

Expand Down
24 changes: 11 additions & 13 deletions yarn-project/p2p/src/msg_validators/clock_tolerance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,11 @@ describe('clock_tolerance', () => {
});

describe('PipeliningWindow.acceptsAttestation', () => {
// Config: 72s slot, 2s p2p.
// Under early pipelining, attestationWindowIntoTargetSlot = 2*p2p = 4s,
// giving straggler attestations 4s+500ms grace after the receiver rolls
// into the next slot.
// Config: 72s slot, 12s Ethereum slot.
// attestationWindowIntoTargetSlot now spans target-slot start to the L1 publish deadline:
// aztecSlotDuration - 2*ethereumSlotDuration = 72 - 24 = 48s, giving straggler attestations
// 48s + 500ms grace after the receiver rolls into the target slot, so the proposer can keep
// collecting useful attestations until right before the publish deadline.
let epochCache: ReturnType<typeof mock<EpochCacheInterface>>;
let pipeliningWindow: PipeliningWindow;

Expand All @@ -282,34 +283,31 @@ describe('clock_tolerance', () => {
epoch: 1 as any,
slot: SlotNumber(100),
ts: 1000n,
nowMs: 1003000n, // 3000ms elapsed, within 4500ms straggler grace
nowMs: 1003000n, // 3000ms elapsed, within 48500ms straggler grace
});

expect(pipeliningWindow.acceptsAttestation(SlotNumber(100))).toBe(true);
});

it('rejects a current-slot attestation past the straggler window', () => {
it('accepts an attestation arriving well into the target slot (previously rejected past ~4.5s)', () => {
epochCache.getEpochAndSlotNow.mockReturnValue({
epoch: 1 as any,
slot: SlotNumber(100),
ts: 1000n,
nowMs: 1005000n, // 5000ms elapsed, past 4500ms cutoff
nowMs: 1030000n, // 30s into the target slot — past the old 4.5s window, within the new 48.5s
});

expect(pipeliningWindow.acceptsAttestation(SlotNumber(100))).toBe(false);
expect(pipeliningWindow.acceptsAttestation(SlotNumber(100))).toBe(true);
});

it('scales the straggler window with p2pPropagationTime', () => {
// With p2pPropagationTime = 4, straggler window = 8s + 500ms disparity.
const longer = new PipeliningWindow(epochCache, { l1PublishingTime: 12, p2pPropagationTime: 4 });
it('rejects a current-slot attestation past the L1 publish deadline window', () => {
epochCache.getEpochAndSlotNow.mockReturnValue({
epoch: 1 as any,
slot: SlotNumber(100),
ts: 1000n,
nowMs: 1007000n,
nowMs: 1049000n, // 49s elapsed, past the 48.5s cutoff (48s window + 500ms disparity)
});

expect(longer.acceptsAttestation(SlotNumber(100))).toBe(true);
expect(pipeliningWindow.acceptsAttestation(SlotNumber(100))).toBe(false);
});
});
Expand Down
20 changes: 15 additions & 5 deletions yarn-project/sequencer-client/src/sequencer/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,17 @@ These constants come from `@aztec/stdlib/timetable` (see `stdlib/src/timetable/i
| `WAITING_FOR_TXS` | `initializeDeadline + checkpointInitializationTime` |
| `CREATING_BLOCK` | same as `WAITING_FOR_TXS` |
| `WAITING_UNTIL_NEXT_BLOCK` | same as `WAITING_FOR_TXS` |
| `ASSEMBLING_CHECKPOINT` | `aztecSlotDuration` (assembly completes by the build-slot boundary) |
| `ASSEMBLING_CHECKPOINT` | `aztecSlotDuration + pipeliningAttestationGracePeriod` |
| `COLLECTING_ATTESTATIONS` | same as `ASSEMBLING_CHECKPOINT` |
| `PUBLISHING_CHECKPOINT` | `2 * aztecSlotDuration - l1PublishingTime` (extends into the target slot) |
| `PUBLISHING_CHECKPOINT` | `2 * aztecSlotDuration - ethereumSlotDuration` (extends into the target slot) |

`ASSEMBLING_CHECKPOINT` and `COLLECTING_ATTESTATIONS` must be *entered* before the build-slot boundary; once entered, attestation collection itself has its own `checkpointAttestationDeadline = 2 * aztecSlotDuration - l1PublishingTime`, so a late attestation arriving after the boundary is still accepted. The publishing deadline extends into the target slot because that is when the L1 tx is actually submitted.
In production-like timing, `pipeliningAttestationGracePeriod` is zero, so `ASSEMBLING_CHECKPOINT` and
`COLLECTING_ATTESTATIONS` must be *entered* before the build-slot boundary. Local networks with
`l1PublishingTime < ethereumSlotDuration` can use the target-slot attestation window as grace while preserving the
L1-geometry publishing cutoff. Once entered, attestation collection itself has its own
`checkpointAttestationDeadline = 2 * aztecSlotDuration - ethereumSlotDuration`, so a late attestation arriving after
the boundary is still accepted. The publishing deadline extends into the target slot because that is when the L1 tx is
actually submitted.

## Example: 72 s slot, 8 s sub-slots

Expand Down Expand Up @@ -208,7 +214,9 @@ The current sub-slot is dropped without committing anything. The loop retries on

### Build slot ends before attestations arrive

`assertTimeLeft` will reject `PUBLISHING_CHECKPOINT` if the attestation deadline has passed; the slot is abandoned, and `checkpoint-publish-failed` is emitted. The `PUBLISHING_CHECKPOINT` deadline allows spillover into the target slot (`2 * aztecSlotDuration - l1PublishingTime`) precisely to absorb a small overrun.
`assertTimeLeft` will reject `PUBLISHING_CHECKPOINT` if the attestation deadline has passed; the slot is abandoned, and
`checkpoint-publish-failed` is emitted. The `PUBLISHING_CHECKPOINT` deadline allows spillover into the target slot
(`2 * aztecSlotDuration - ethereumSlotDuration`) precisely to absorb a small overrun.

### Pipelined parent fails on L1

Expand All @@ -230,4 +238,6 @@ aztecSlotDuration ≥ checkpointInitializationTime

Block duration should be ≥ `minExecutionTime` (otherwise no sub-slot ever has enough headroom). `p2pPropagationTime` should be measured against the deployment's actual p2p latency: it directly determines how much of each slot is spent on the cooldown.

`l1PublishingTime` should fit inside the Ethereum slot the target slot maps to. The default of 12 s lines up with one Ethereum slot; congested deployments may need to increase it (which only affects the `PUBLISHING_CHECKPOINT` deadline, not the number of blocks built).
`l1PublishingTime` should fit inside the Ethereum slot the target slot maps to. The default of 12 s lines up with one
Ethereum slot; fast local networks may reduce it to use the target-slot attestation window as assembly and attestation
grace.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { type P2P, P2PClientState } from '@aztec/p2p';
import type { SlasherClientInterface } from '@aztec/slasher';
import { AztecAddress } from '@aztec/stdlib/aztec-address';
import type { L2Block, L2BlockSink, L2BlockSource, ProposedCheckpointSink } from '@aztec/stdlib/block';
import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers';
import { type L1RollupConstants, getSlotStartBuildTimestamp } from '@aztec/stdlib/epoch-helpers';
import { GasFees } from '@aztec/stdlib/gas';
import type {
BlockBuilderOptions,
Expand Down Expand Up @@ -301,8 +301,9 @@ describe('CheckpointProposalJob Timing Tests', () => {
}

/** Create a TimingTestCheckpointProposalJob with current mocks */
function createJob(): TimingTestCheckpointProposalJob {
const setStateFn = jest.fn();
function createJob(
setStateFn: (state: SequencerState, slot?: SlotNumber, timeReferenceSlot?: SlotNumber) => void = jest.fn(),
): TimingTestCheckpointProposalJob {
const eventEmitter = new EventEmitter() as TypedEventEmitter<SequencerEvents>;

return new TimingTestCheckpointProposalJob(
Expand Down Expand Up @@ -1138,4 +1139,89 @@ describe('CheckpointProposalJob Timing Tests', () => {
expect(publisher.sendRequestsAt).toHaveBeenCalledWith(targetSlot);
});
});

describe('Build-frame deadline enforcement', () => {
// Regression coverage for the frame bug: build-frame state deadlines must be measured against
// the build slot (`slotNow`), not the target slot. Passing `targetSlot` shifted every deadline a
// full Aztec slot into the future (frame B), so they never fired.

// States the job owns and sets while building inside the build frame. Their deadlines must be
// enforced against `slotNow`.
const buildFrameStates = [
SequencerState.INITIALIZING_CHECKPOINT,
SequencerState.WAITING_FOR_TXS,
SequencerState.CREATING_BLOCK,
SequencerState.WAITING_UNTIL_NEXT_BLOCK,
SequencerState.ASSEMBLING_CHECKPOINT,
SequencerState.COLLECTING_ATTESTATIONS,
SequencerState.PUBLISHING_CHECKPOINT,
];

it('passes the target slot to observers and the build slot to deadline checks for build-frame states', async () => {
const { blocks, txs } = await createTestBlocksAndTxs(2);
mockP2pWithTxs(txs);
// Force a single WAITING_FOR_TXS poll before the first block by reporting no pending txs once.
p2p.getPendingTxCount.mockResolvedValueOnce(0);
checkpointBuilder.seedBlocks(
blocks,
blocks.map((_, i) => [txs[i]]),
);
checkpointBuilder.setExecutionDurations([5, 5]);

validatorClient.collectAttestations.mockResolvedValue(getAttestations(blocks[1]));

setTimeInSlot(1);

const observedSlots = new Map<
SequencerState,
{ slot: SlotNumber | undefined; timeReferenceSlot: SlotNumber | undefined }
>();
const setStateFn = jest.fn((state: SequencerState, slot?: SlotNumber, timeReferenceSlot?: SlotNumber) => {
observedSlots.set(state, { slot, timeReferenceSlot });
});

const job = createJob(setStateFn);
await job.execute();
await job.awaitPendingSubmission();

// The build and target slot differ, so the bug would be observable.
expect(Number(targetSlot)).toBe(Number(slotNumber) + PROPOSER_PIPELINING_SLOT_OFFSET);

for (const state of buildFrameStates) {
expect(observedSlots.has(state)).toBe(true);
expect(observedSlots.get(state)?.slot).toBe(targetSlot);
expect(observedSlots.get(state)?.timeReferenceSlot).toBe(slotNumber);
}
});

it('abandons the slot when a build-frame deadline is missed (assembly past the build-slot boundary)', async () => {
const { blocks, txs } = await createTestBlocksAndTxs(1);
mockP2pWithTxs(txs);
checkpointBuilder.seedBlocks(blocks, [[txs[0]]]);
// Single block runs 50s -> 80s (in the build frame), so ASSEMBLING_CHECKPOINT is entered at
// ~80s into the build frame, past its deadline (checkpointAssemblyDeadline = 72s in frame A).
checkpointBuilder.setExecutionDurations([30]);

validatorClient.collectAttestations.mockResolvedValue(getAttestations(blocks[0]));

setTimeInSlot(50);

// Enforcing setStateFn mirroring Sequencer.setState: measures the deadline against the
// explicit time reference slot when one is provided.
const setStateFn = (state: SequencerState, slot?: SlotNumber, timeReferenceSlot?: SlotNumber) => {
const slotForTiming = timeReferenceSlot ?? slot;
if (slotForTiming !== undefined) {
const ref = getSlotStartBuildTimestamp(slotForTiming, l1Constants);
timetable.assertTimeLeft(state, dateProvider.nowInSeconds() - ref);
}
};

const job = createJob(setStateFn);
const checkpoint = await job.execute();

// Past the build-frame assembly deadline: the SequencerTooSlowError is caught inside
// proposeCheckpoint, which returns undefined, so no checkpoint is produced.
expect(checkpoint).toBeUndefined();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class CheckpointProposalJob implements Traceable {
private readonly metrics: SequencerMetrics,
private readonly checkpointMetrics: CheckpointProposalJobMetricsRecorder,
protected readonly eventEmitter: TypedEventEmitter<SequencerEvents>,
private readonly setStateFn: (state: SequencerState, slot?: SlotNumber) => void,
private readonly setStateFn: (state: SequencerState, slot?: SlotNumber, timeReferenceSlot?: SlotNumber) => void,
public readonly tracer: Tracer,
bindings?: LoggerBindings,
private readonly proposedCheckpointData?: ProposedCheckpointData,
Expand Down Expand Up @@ -328,7 +328,8 @@ export class CheckpointProposalJob implements Traceable {
private async enqueueCheckpointForSubmission(result: CheckpointProposalResult): Promise<void> {
const { checkpoint, attestations, attestationsSignature } = result;

this.setStateFn(SequencerState.PUBLISHING_CHECKPOINT, this.targetSlot);
// Build-frame deadlines are measured against `slotNow`; observers still see `targetSlot`.
this.setStateFn(SequencerState.PUBLISHING_CHECKPOINT, this.targetSlot, this.slotNow);
const aztecSlotDuration = this.l1Constants.slotDuration;
const submissionSlotStart = Number(getTimestampForSlot(this.targetSlot, this.l1Constants));
const txTimeoutAt = new Date((submissionSlotStart + aztecSlotDuration) * 1000);
Expand Down Expand Up @@ -518,7 +519,7 @@ export class CheckpointProposalJob implements Traceable {
const feeRecipient = this.validatorClient.getFeeRecipientForAttestor(this.attestorAddress);

// Start the checkpoint
this.setStateFn(SequencerState.INITIALIZING_CHECKPOINT, this.targetSlot);
this.setStateFn(SequencerState.INITIALIZING_CHECKPOINT, this.targetSlot, this.slotNow);
this.logCheckpointEvent('slot-started', `Starting checkpoint proposal for slot ${this.targetSlot}`, {
buildSlot: this.slotNow,
submissionSlot: this.targetSlot,
Expand Down Expand Up @@ -671,7 +672,7 @@ export class CheckpointProposalJob implements Traceable {

// Assemble and broadcast the checkpoint proposal, including the last block that was not
// broadcasted yet, and wait to collect the committee attestations.
this.setStateFn(SequencerState.ASSEMBLING_CHECKPOINT, this.targetSlot);
this.setStateFn(SequencerState.ASSEMBLING_CHECKPOINT, this.targetSlot, this.slotNow);
const checkpoint = await checkpointBuilder.completeCheckpoint();

// Final validation: per-block limits are only checked if the operator set them explicitly.
Expand Down Expand Up @@ -963,7 +964,7 @@ export class CheckpointProposalJob implements Traceable {
/** Sleeps until it is time to produce the next block in the slot */
@trackSpan('CheckpointProposalJob.waitUntilNextSubslot')
private async waitUntilNextSubslot(nextSubslotStart: number) {
this.setStateFn(SequencerState.WAITING_UNTIL_NEXT_BLOCK, this.targetSlot);
this.setStateFn(SequencerState.WAITING_UNTIL_NEXT_BLOCK, this.targetSlot, this.slotNow);
this.log.verbose(`Waiting until time for the next block at ${nextSubslotStart}s into slot`, {
slot: this.targetSlot,
});
Expand Down Expand Up @@ -1034,7 +1035,7 @@ export class CheckpointProposalJob implements Traceable {
`Building block ${blockNumber} at index ${indexWithinCheckpoint} for slot ${this.targetSlot} with ${availableTxs} available txs`,
{ slot: this.targetSlot, blockNumber, indexWithinCheckpoint },
);
this.setStateFn(SequencerState.CREATING_BLOCK, this.targetSlot);
this.setStateFn(SequencerState.CREATING_BLOCK, this.targetSlot, this.slotNow);

// Per-block limits are operator overrides (from SEQ_MAX_L2_BLOCK_GAS etc.) further capped
// by remaining checkpoint-level budgets inside CheckpointBuilder before each block is built.
Expand Down Expand Up @@ -1205,7 +1206,7 @@ export class CheckpointProposalJob implements Traceable {
}

// Wait a bit before checking again
this.setStateFn(SequencerState.WAITING_FOR_TXS, this.targetSlot);
this.setStateFn(SequencerState.WAITING_FOR_TXS, this.targetSlot, this.slotNow);
this.log.verbose(
`Waiting for enough txs to build block ${blockNumber} at index ${indexWithinCheckpoint} in slot ${this.targetSlot} (have ${availableTxs} but need ${minTxs})`,
{ blockNumber, slot: this.targetSlot, indexWithinCheckpoint },
Expand All @@ -1221,7 +1222,7 @@ export class CheckpointProposalJob implements Traceable {
broadcast: CheckpointProposalBroadcast,
): Promise<{ attestations: CommitteeAttestationsAndSigners; attestationsSignature: Signature } | undefined> {
const { proposal, blockProposedAt } = broadcast;
this.setStateFn(SequencerState.COLLECTING_ATTESTATIONS, this.targetSlot);
this.setStateFn(SequencerState.COLLECTING_ATTESTATIONS, this.targetSlot, this.slotNow);
const attestations = await this.waitForAttestations(proposal);
if (!attestations) {
return undefined;
Expand Down
1 change: 1 addition & 0 deletions yarn-project/sequencer-client/src/sequencer/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export type SequencerEvents = {
newState: SequencerState;
secondsIntoSlot?: number;
slot?: SlotNumber;
timeReferenceSlot?: SlotNumber;
}) => void;
/**
* Emitted by the sequencer once it has decided it is going to attempt to build a
Expand Down
Loading
Loading