chore(mbps): clean up TODOs for multiple blocks per slot

yarn-project/end-to-end/src/e2e_l1_publisher/e2e_l1_publisher.test.ts

@@ -354,6 +354,7 @@ describe('L1Publisher integration', () => {
       chainId: globalVariables.chainId,
       version: globalVariables.version,
       slotNumber: globalVariables.slotNumber,
+      timestamp: globalVariables.timestamp,
       coinbase: globalVariables.coinbase,
       feeRecipient: globalVariables.feeRecipient,
       gasFees: globalVariables.gasFees,

yarn-project/p2p/src/services/encoding.ts

@@ -58,7 +58,8 @@ const DefaultMaxSizesKb: Record<TopicType, number> = {
   // Proposals may carry some tx objects, so we allow a larger size capped at 10mb
   // Note this may not be enough for carrying all tx objects in a block
   [TopicType.block_proposal]: 1024 * 10,
-  // TODO(palla/mbps): Check size for checkpoint proposal
+  // Checkpoint proposals carry almost the same data as a block proposal (see the lastBlockProposal)
+  // Only diff is an additional header, which is pretty small compared to the 10mb limit
   [TopicType.checkpoint_proposal]: 1024 * 10,
 };
 

yarn-project/prover-client/src/light/lightweight_checkpoint_builder.test.ts

@@ -3,17 +3,16 @@ import { BlockNumber, CheckpointNumber, SlotNumber } from '@aztec/foundation/bra
 import { timesAsync } from '@aztec/foundation/collection';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { getVKTreeRoot } from '@aztec/noir-protocol-circuits-types/vk-tree';
-import { ProtocolContractsList, protocolContractsHash } from '@aztec/protocol-contracts';
+import { ProtocolContractsList } from '@aztec/protocol-contracts';
 import { computeFeePayerBalanceLeafSlot } from '@aztec/protocol-contracts/fee-juice';
 import { PublicDataWrite } from '@aztec/stdlib/avm';
 import { AztecAddress } from '@aztec/stdlib/aztec-address';
 import { EthAddress } from '@aztec/stdlib/block';
 import { GasFees } from '@aztec/stdlib/gas';
 import { accumulateCheckpointOutHashes } from '@aztec/stdlib/messaging';
-import { CheckpointConstantData } from '@aztec/stdlib/rollup';
 import { mockProcessedTx } from '@aztec/stdlib/testing';
 import { PublicDataTreeLeaf } from '@aztec/stdlib/trees';
-import type { ProcessedTx } from '@aztec/stdlib/tx';
+import type { CheckpointGlobalVariables, ProcessedTx } from '@aztec/stdlib/tx';
 import { GlobalVariables } from '@aztec/stdlib/tx';
 import { NativeWorldStateService } from '@aztec/world-state/native';
 
@@ -41,18 +40,16 @@ describe('LightweightCheckpointBuilder', () => {
     await worldState.close();
   });
 
-  const makeCheckpointConstants = (slotNumber: SlotNumber): CheckpointConstantData => {
-    return CheckpointConstantData.from({
+  const makeCheckpointConstants = (slotNumber: SlotNumber): CheckpointGlobalVariables => {
+    return {
       chainId: Fr.ZERO,
       version: Fr.ZERO,
-      vkTreeRoot: getVKTreeRoot(),
-      protocolContractsHash,
-      proverId: Fr.ZERO,
       slotNumber,
+      timestamp: BigInt(slotNumber) * 123n,
       coinbase: EthAddress.ZERO,
       feeRecipient: AztecAddress.ZERO,
       gasFees: GasFees.empty(),
-    });
+    };
   };
 
   const makeGlobalVariables = (blockNumber: BlockNumber, slotNumber: SlotNumber): GlobalVariables => {

yarn-project/prover-client/src/mocks/test_context.ts

@@ -250,7 +250,7 @@ export class TestContext {
     const previousCheckpointOutHashes = this.checkpointOutHashes;
     const builder = await LightweightCheckpointBuilder.startNewCheckpoint(
       checkpointNumber,
-      constants,
+      { ...constants, timestamp },
       l1ToL2Messages,
       previousCheckpointOutHashes,
       cleanFork,

yarn-project/sequencer-client/src/global_variable_builder/global_builder.ts

@@ -120,7 +120,7 @@ export class GlobalVariableBuilder implements GlobalVariableBuilderInterface {
     coinbase: EthAddress,
     feeRecipient: AztecAddress,
     slotNumber: SlotNumber,
-  ): Promise<CheckpointGlobalVariables & { timestamp: bigint }> {
+  ): Promise<CheckpointGlobalVariables> {
     const { chainId, version } = this;
 
     const timestamp = getTimestampForSlot(slotNumber, {

yarn-project/sequencer-client/src/publisher/sequencer-publisher.ts

@@ -639,22 +639,6 @@ export class SequencerPublisher {
     options: { forcePendingCheckpointNumber?: CheckpointNumber },
   ): Promise<bigint> {
     const ts = BigInt((await this.l1TxUtils.getBlock()).timestamp + this.ethereumSlotDuration);
-
-    // TODO(palla/mbps): This should not be needed, there's no flow where we propose with zero attestations. Or is there?
-    // If we have no attestations, we still need to provide the empty attestations
-    // so that the committee is recalculated correctly
-    // const ignoreSignatures = attestationsAndSigners.attestations.length === 0;
-    // if (ignoreSignatures) {
-    //   const { committee } = await this.epochCache.getCommittee(block.header.globalVariables.slotNumber);
-    //   if (!committee) {
-    //     this.log.warn(`No committee found for slot ${block.header.globalVariables.slotNumber}`);
-    //     throw new Error(`No committee found for slot ${block.header.globalVariables.slotNumber}`);
-    //   }
-    //   attestationsAndSigners.attestations = committee.map(committeeMember =>
-    //     CommitteeAttestation.fromAddress(committeeMember),
-    //   );
-    // }
-
     const blobFields = checkpoint.toBlobFields();
     const blobs = getBlobsPerL1Block(blobFields);
     const blobInput = getPrefixedEthBlobCommitments(blobs);

yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.timing.test.ts

@@ -62,7 +62,7 @@ class TimingAwareMockCheckpointBuilder extends MockCheckpointBuilder {
   public recordedBuildTimes: Array<{ blockNumber: number; startTime: number; endTime: number }> = [];
 
   constructor(
-    constants: CheckpointGlobalVariables & { timestamp: bigint },
+    constants: CheckpointGlobalVariables,
     checkpointNumber: CheckpointNumber,
     private readonly dateProvider: ManualDateProvider,
     private readonly getSecondsIntoSlot: () => number,
@@ -368,7 +368,7 @@ describe('CheckpointProposalJob Timing Tests', () => {
     );
 
     // Create timing-aware checkpoint builder
-    const checkpointConstants: CheckpointGlobalVariables & { timestamp: bigint } = { ...globalVariables };
+    const checkpointConstants: CheckpointGlobalVariables = { ...globalVariables };
     checkpointBuilder = new TimingAwareMockCheckpointBuilder(
       checkpointConstants,
       checkpointNumber,

yarn-project/sequencer-client/src/sequencer/sequencer.ts

@@ -160,7 +160,6 @@ export class Sequencer extends (EventEmitter as new () => TypedEventEmitter<Sequ
     } catch (err) {
       this.emit('checkpoint-error', { error: err as Error });
       if (err instanceof SequencerTooSlowError) {
-        // TODO(palla/mbps): Add missing states
         // Log as warn only if we had to abort halfway through the block proposal
         const logLvl = [SequencerState.INITIALIZING_CHECKPOINT, SequencerState.PROPOSER_CHECK].includes(
           err.proposedState,

yarn-project/slasher/src/watchers/epoch_prune_watcher.ts

@@ -172,6 +172,7 @@ export class EpochPruneWatcher extends (EventEmitter as new () => WatcherEmitter
       chainId: gv.chainId,
       version: gv.version,
       slotNumber: gv.slotNumber,
+      timestamp: gv.timestamp,
       coinbase: gv.coinbase,
       feeRecipient: gv.feeRecipient,
       gasFees: gv.gasFees,

yarn-project/stdlib/src/p2p/block_proposal.ts

@@ -56,8 +56,6 @@ export class BlockProposal extends Gossipable {
     /** The per-block header containing block state and global variables */
     public readonly blockHeader: BlockHeader,
 
-    // TODO(palla/mbps): Is this really needed? Can we just derive it from the indexWithinCheckpoint of the parent block and the slot number?
-    // See the block-proposal-handler, we have a lot of extra validations to check this is correct, so maybe we can avoid storing it here.
     /** Index of this block within the checkpoint (0-indexed) */
     public readonly indexWithinCheckpoint: IndexWithinCheckpoint,
 

yarn-project/stdlib/src/rollup/checkpoint_header.ts

@@ -19,8 +19,8 @@ import type { UInt64 } from '../types/shared.js';
 
 /**
  * Header of a checkpoint. A checkpoint is a collection of blocks submitted to L1 all within the same slot.
- * TODO(palla/mbps): Should this include chainId and version as well? Is this used just in circuits?
- * TODO(palla/mbps): What about CheckpointNumber?
+ * This header is verified as-is in the rollup circuits, posted to the L1 rollup contract, stored in the archiver,
+ * and exposed via the Aztec Node API. See `CheckpointData` for a struct that includes the header plus extra metadata.
  */
 export class CheckpointHeader {
   constructor(

yarn-project/stdlib/src/tx/global_variables.ts

@@ -22,10 +22,10 @@ import { schemas } from '../schemas/index.js';
 import type { UInt64 } from '../types/index.js';
 
 /**
- * Global variables that are constant across the entire slot.
- * TODO(palla/mbps): Should timestamp be included here as well?
+ * Global variables that are constant across the entire checkpoint (slot).
+ * Excludes blockNumber since that varies per block within a checkpoint.
  */
-export type CheckpointGlobalVariables = Omit<FieldsOf<GlobalVariables>, 'blockNumber' | 'timestamp'>;
+export type CheckpointGlobalVariables = Omit<FieldsOf<GlobalVariables>, 'blockNumber'>;
 
 /**
  * Global variables of the L2 block.

yarn-project/validator-client/src/block_proposal_handler.ts

@@ -477,11 +477,12 @@ export class BlockProposalHandler {
     await this.worldState.syncImmediate(parentBlockNumber);
     using fork = await this.worldState.fork(parentBlockNumber);
 
-    // Build checkpoint constants from proposal (excludes blockNumber and timestamp which are per-block)
+    // Build checkpoint constants from proposal (excludes blockNumber which is per-block)
     const constants: CheckpointGlobalVariables = {
       chainId: new Fr(config.l1ChainId),
       version: new Fr(config.rollupVersion),
       slotNumber: slot,
+      timestamp: blockHeader.globalVariables.timestamp,
       coinbase: blockHeader.globalVariables.coinbase,
       feeRecipient: blockHeader.globalVariables.feeRecipient,
       gasFees: blockHeader.globalVariables.gasFees,

yarn-project/validator-client/src/checkpoint_builder.test.ts

@@ -41,6 +41,7 @@ describe('CheckpointBuilder', () => {
     chainId: new Fr(1),
     version: new Fr(1),
     slotNumber,
+    timestamp: BigInt(Date.now()),
     coinbase: EthAddress.random(),
     feeRecipient: AztecAddress.fromField(Fr.random()),
     gasFees: GasFees.empty(),

yarn-project/validator-client/src/validator.integration.test.ts

@@ -279,6 +279,7 @@ describe('ValidatorClient Integration', () => {
       feeRecipient: await AztecAddress.random(),
       gasFees: GasFees.empty(),
       slotNumber: slot,
+      timestamp: BigInt(Date.now()),
     };
 
     using fork = await proposer.worldStateDb.fork();

yarn-project/validator-client/src/validator.test.ts

@@ -482,6 +482,44 @@ describe('ValidatorClient', () => {
       uploadBlobsSpy.mockRestore();
     });
 
+    it('should not attest to a checkpoint proposal that references a middle block instead of the last', async () => {
+      const addCheckpointAttestationsSpy = jest.spyOn(p2pClient, 'addOwnCheckpointAttestations');
+
+      // First validate a block proposal so the validator has seen a block for this slot
+      const didValidate = await validatorClient.validateBlockProposal(proposal, sender);
+      expect(didValidate).toBe(true);
+
+      // Create 3 blocks for the slot, each with a distinct archive root
+      const block1Archive = new AppendOnlyTreeSnapshot(Fr.random(), 1);
+      const block2Archive = new AppendOnlyTreeSnapshot(Fr.random(), 2);
+      const block3Archive = new AppendOnlyTreeSnapshot(Fr.random(), 3);
+      const blocks = [
+        { archive: block1Archive, number: 1 },
+        { archive: block2Archive, number: 2 },
+        { archive: block3Archive, number: 3 },
+      ] as unknown as L2Block[];
+
+      // Proposal references the middle block's archive (block 2), not the last (block 3)
+      const checkpointProposal = await makeCheckpointProposal({
+        archiveRoot: block2Archive.root,
+        checkpointHeader: makeCheckpointHeader(0, { slotNumber: proposal.slotNumber }),
+        lastBlock: {
+          blockHeader: makeBlockHeader(1, { blockNumber: BlockNumber(2), slotNumber: proposal.slotNumber }),
+          indexWithinCheckpoint: IndexWithinCheckpoint(1),
+          txHashes: proposal.txHashes,
+        },
+      });
+
+      // Mock getBlockHeaderByArchive to return a header so retryUntil succeeds
+      blockSource.getBlockHeaderByArchive.mockResolvedValue(makeBlockHeader());
+      blockSource.getBlocksForSlot.mockResolvedValue(blocks);
+
+      // Checkpoint validation should fail: proposal points to block 2 but last block in slot is block 3
+      const attestations = await validatorClient.attestToCheckpointProposal(checkpointProposal, sender);
+      expect(attestations).toBeUndefined();
+      expect(addCheckpointAttestationsSpy).not.toHaveBeenCalled();
+    });
+
     it('should wait for previous block to sync', async () => {
       epochCache.filterInCommittee.mockResolvedValue([EthAddress.fromString(validatorAccounts[0].address)]);
       blockSource.getBlockHeaderByArchive.mockResolvedValueOnce(undefined);

yarn-project/validator-client/src/validator.ts

@@ -643,6 +643,12 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter)
       return { isValid: false, reason: 'no_blocks_for_slot' };
     }
 
+    // Ensure the last block for this slot matches the archive in the checkpoint proposal
+    if (!blocks.at(-1)?.archive.root.equals(proposal.archive)) {
+      this.log.warn(`Last block archive mismatch for checkpoint proposal`, proposalInfo);
+      return { isValid: false, reason: 'last_block_archive_mismatch' };
+    }
+
     this.log.debug(`Found ${blocks.length} blocks for slot ${slot}`, {
       ...proposalInfo,
       blockNumbers: blocks.map(b => b.number),
@@ -737,6 +743,7 @@ export class ValidatorClient extends (EventEmitter as new () => WatcherEmitter)
       chainId: gv.chainId,
       version: gv.version,
       slotNumber: gv.slotNumber,
+      timestamp: gv.timestamp,
       coinbase: gv.coinbase,
       feeRecipient: gv.feeRecipient,
       gasFees: gv.gasFees,