feat: Archiver does not sync blocks with invalid attestations

Author: spalladino

yarn-project/archiver/package.json

@@ -69,6 +69,7 @@
     "@aztec/blob-lib": "workspace:^",
     "@aztec/blob-sink": "workspace:^",
     "@aztec/constants": "workspace:^",
+    "@aztec/epoch-cache": "workspace:^",
     "@aztec/ethereum": "workspace:^",
     "@aztec/foundation": "workspace:^",
     "@aztec/kv-store": "workspace:^",

yarn-project/archiver/src/archiver/archiver.test.ts

@@ -2,9 +2,11 @@ import { Blob } from '@aztec/blob-lib';
 import type { BlobSinkClientInterface } from '@aztec/blob-sink/client';
 import { BlobWithIndex } from '@aztec/blob-sink/types';
 import { GENESIS_ARCHIVE_ROOT } from '@aztec/constants';
+import type { EpochCache, EpochCommitteeInfo } from '@aztec/epoch-cache';
 import { DefaultL1ContractsConfig, InboxContract, RollupContract, type ViemPublicClient } from '@aztec/ethereum';
 import { Buffer16, Buffer32 } from '@aztec/foundation/buffer';
 import { times } from '@aztec/foundation/collection';
+import { Secp256k1Signer } from '@aztec/foundation/crypto';
 import { EthAddress } from '@aztec/foundation/eth-address';
 import { Fr } from '@aztec/foundation/fields';
 import { type Logger, createLogger } from '@aztec/foundation/log';
@@ -13,10 +15,11 @@ import { sleep } from '@aztec/foundation/sleep';
 import { bufferToHex, withoutHexPrefix } from '@aztec/foundation/string';
 import { openTmpStore } from '@aztec/kv-store/lmdb-v2';
 import { type InboxAbi, RollupAbi } from '@aztec/l1-artifacts';
-import { L2Block } from '@aztec/stdlib/block';
+import { CommitteeAttestation, L2Block } from '@aztec/stdlib/block';
 import type { L1RollupConstants } from '@aztec/stdlib/epoch-helpers';
 import { PrivateLog } from '@aztec/stdlib/logs';
 import { InboxLeaf } from '@aztec/stdlib/messaging';
+import { makeBlockAttestationFromBlock } from '@aztec/stdlib/testing';
 import { getTelemetryClient } from '@aztec/telemetry-client';
 
 import { jest } from '@jest/globals';
@@ -30,6 +33,8 @@ import { KVArchiverDataStore } from './kv_archiver_store/kv_archiver_store.js';
 import { updateRollingHash } from './structs/inbox_message.js';
 
 interface MockRollupContractRead {
+  /** Returns the target committee size */
+  getTargetCommitteeSize: () => Promise<bigint>;
   /** Returns the rollup version. */
   getVersion: () => Promise<bigint>;
   /** Given an L2 block number, returns the archive. */
@@ -81,9 +86,19 @@ describe('Archiver', () => {
     publicClient.getBlockNumber.mockResolvedValue(nums.at(-1)!);
   };
 
+  const makeBlock = async (blockNumber: number) => {
+    const block = await L2Block.random(blockNumber, txsPerBlock, blockNumber + 1, 2);
+    block.header.globalVariables.timestamp = BigInt(now + Number(ETHEREUM_SLOT_DURATION) * (blockNumber + 1));
+    block.body.txEffects.forEach((txEffect, i) => {
+      txEffect.privateLogs = times(getNumPrivateLogsForTx(block.number, i), () => PrivateLog.random());
+    });
+    return block;
+  };
+
   let publicClient: MockProxy<ViemPublicClient>;
   let instrumentation: MockProxy<ArchiverInstrumentation>;
   let blobSinkClient: MockProxy<BlobSinkClientInterface>;
+  let epochCache: MockProxy<EpochCache>;
   let archiverStore: ArchiverDataStore;
   let l1Constants: L1RollupConstants & { l1StartBlockHash: Buffer32 };
   let now: number;
@@ -132,6 +147,8 @@ describe('Archiver', () => {
     }) as any);
 
     blobSinkClient = mock<BlobSinkClientInterface>();
+    epochCache = mock<EpochCache>();
+    epochCache.getCommitteeForEpoch.mockResolvedValue({ committee: [] as EthAddress[] } as EpochCommitteeInfo);
 
     const tracer = getTelemetryClient().getTracer('');
     instrumentation = mock<ArchiverInstrumentation>({ isEnabled: () => true, tracer });
@@ -152,17 +169,12 @@ describe('Archiver', () => {
       archiverStore,
       { pollingIntervalMs: 1000, batchSize: 1000 },
       blobSinkClient,
+      epochCache,
       instrumentation,
       l1Constants,
     );
 
-    blocks = await Promise.all(blockNumbers.map(x => L2Block.random(x, txsPerBlock, x + 1, 2)));
-    blocks.forEach((block, i) => {
-      block.header.globalVariables.timestamp = BigInt(now + Number(ETHEREUM_SLOT_DURATION) * (i + 1));
-      block.body.txEffects.forEach((txEffect, i) => {
-        txEffect.privateLogs = times(getNumPrivateLogsForTx(block.number, i), () => PrivateLog.random());
-      });
-    });
+    blocks = await Promise.all(blockNumbers.map(makeBlock));
 
     // TODO(palla/archiver) Instead of guessing the archiver requests with mockResolvedValueOnce,
     // we should use a mock implementation that returns the expected value based on the input.
@@ -171,7 +183,7 @@ describe('Archiver', () => {
     // blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
     // blobsFromBlocks.forEach(blobs => blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobs));
 
-    // rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    // rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     // publicClient.getTransaction.mockImplementation((args: { hash?: `0x${string}` }) => {
     //   const index = parseInt(withoutHexPrefix(args.hash!));
     //   if (index > blocks.length) {
@@ -252,7 +264,7 @@ describe('Archiver', () => {
     let latestBlockNum = await archiver.getBlockNumber();
     expect(latestBlockNum).toEqual(0);
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
 
     mockL1BlockNumbers(2500n, 2510n, 2520n);
@@ -334,7 +346,7 @@ describe('Archiver', () => {
 
     const numL2BlocksInTest = 2;
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
 
     // Here we set the current L1 block number to 102. L1 to L2 messages after this should not be read.
@@ -368,6 +380,68 @@ describe('Archiver', () => {
     });
   }, 10_000);
 
+  it('ignores block 2 because it had invalid attestations', async () => {
+    let latestBlockNum = await archiver.getBlockNumber();
+    expect(latestBlockNum).toEqual(0);
+
+    // Setup a committee of 3 signers
+    mockRollupRead.getTargetCommitteeSize.mockResolvedValue(3n);
+    const signers = times(3, Secp256k1Signer.random);
+    const committee = signers.map(signer => signer.address);
+    epochCache.getCommitteeForEpoch.mockResolvedValue({ committee } as EpochCommitteeInfo);
+
+    // Add the attestations from the signers to all 3 blocks
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b, signers)));
+    const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
+    const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
+
+    // And define a bad block 2 with attestations from random signers
+    const badBlock2 = await makeBlock(2);
+    const badBlock2RollupTx = await makeRollupTx(badBlock2, times(3, Secp256k1Signer.random));
+    const badBlock2BlobHashes = await makeVersionedBlobHashes(badBlock2);
+    const badBlock2Blobs = await makeBlobsFromBlock(badBlock2);
+
+    // During the first archiver loop, we fetch block 1 and the block 2 with bad attestations
+    publicClient.getBlockNumber.mockResolvedValue(85n);
+    makeL2BlockProposedEvent(70n, 1n, blocks[0].archive.root.toString(), blobHashes[0]);
+    makeL2BlockProposedEvent(80n, 2n, badBlock2.archive.root.toString(), badBlock2BlobHashes);
+    mockRollup.read.status.mockResolvedValue([0n, GENESIS_ROOT, 2n, badBlock2.archive.root.toString(), GENESIS_ROOT]);
+    publicClient.getTransaction.mockResolvedValueOnce(rollupTxs[0]).mockResolvedValueOnce(badBlock2RollupTx);
+    blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobsFromBlocks[0]).mockResolvedValueOnce(badBlock2Blobs);
+
+    // Start archiver, the bad block 2 should not be synced
+    await archiver.start(true);
+    latestBlockNum = await archiver.getBlockNumber();
+    expect(latestBlockNum).toEqual(1);
+
+    // Now we go for another loop, where a proper block 2 is proposed with correct attestations
+    // IRL there would be an "Invalidated" event, but we are not currently relying on it
+    logger.warn(`Adding new block 2 with correct attestations and a block 3`);
+    publicClient.getBlockNumber.mockResolvedValue(100n);
+    makeL2BlockProposedEvent(90n, 2n, blocks[1].archive.root.toString(), blobHashes[1]);
+    makeL2BlockProposedEvent(95n, 3n, blocks[2].archive.root.toString(), blobHashes[2]);
+    mockRollup.read.status.mockResolvedValue([
+      0n,
+      GENESIS_ROOT,
+      3n,
+      blocks[2].archive.root.toString(),
+      blocks[0].archive.root.toString(),
+    ]);
+    publicClient.getTransaction.mockResolvedValueOnce(rollupTxs[1]).mockResolvedValueOnce(rollupTxs[2]);
+    blobSinkClient.getBlobSidecar.mockResolvedValueOnce(blobsFromBlocks[1]).mockResolvedValueOnce(blobsFromBlocks[2]);
+
+    // Now we should move to block 3
+    await waitUntilArchiverBlock(3);
+    latestBlockNum = await archiver.getBlockNumber();
+    expect(latestBlockNum).toEqual(3);
+
+    // And block 2 should return the proper one
+    const [block2] = await archiver.getPublishedBlocks(2, 1);
+    expect(block2.block.number).toEqual(2);
+    expect(block2.block.archive.root.toString()).toEqual(blocks[1].archive.root.toString());
+    expect(block2.attestations.length).toEqual(3);
+  }, 10_000);
+
   it('skip event search if no changes found', async () => {
     const loggerSpy = jest.spyOn((archiver as any).log, 'debug');
 
@@ -376,7 +450,7 @@ describe('Archiver', () => {
 
     const numL2BlocksInTest = 2;
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
 
     mockL1BlockNumbers(50n, 100n);
@@ -414,7 +488,7 @@ describe('Archiver', () => {
 
     const numL2BlocksInTest = 2;
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
 
     let mockedBlockNum = 0n;
@@ -538,7 +612,7 @@ describe('Archiver', () => {
     blocks = [l2Block];
     const blobHashes = await makeVersionedBlobHashes(l2Block);
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     publicClient.getBlockNumber.mockResolvedValue(l1BlockForL2Block);
     mockRollup.read.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]);
     makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString(), blobHashes);
@@ -570,7 +644,7 @@ describe('Archiver', () => {
     blocks = [l2Block];
     const blobHashes = await makeVersionedBlobHashes(l2Block);
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     publicClient.getBlockNumber.mockResolvedValue(l1BlockForL2Block);
     mockRollup.read.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]);
     makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString(), blobHashes);
@@ -630,7 +704,7 @@ describe('Archiver', () => {
     blocks = [l2Block];
     const blobHashes = await makeVersionedBlobHashes(l2Block);
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     publicClient.getBlockNumber.mockResolvedValue(lastL1BlockForEpoch);
     mockRollup.read.status.mockResolvedValueOnce([0n, GENESIS_ROOT, 1n, l2Block.archive.root.toString(), GENESIS_ROOT]);
     makeL2BlockProposedEvent(l1BlockForL2Block, 1n, l2Block.archive.root.toString(), blobHashes);
@@ -660,7 +734,7 @@ describe('Archiver', () => {
   it('handles a block gap due to a spurious L2 prune', async () => {
     expect(await archiver.getBlockNumber()).toEqual(0);
 
-    const rollupTxs = await Promise.all(blocks.map(makeRollupTx));
+    const rollupTxs = await Promise.all(blocks.map(b => makeRollupTx(b)));
     const blobHashes = await Promise.all(blocks.map(makeVersionedBlobHashes));
     const blobsFromBlocks = await Promise.all(blocks.map(b => makeBlobsFromBlock(b)));
 
@@ -805,7 +879,11 @@ describe('Archiver', () => {
  * @param block - The L2Block.
  * @returns A fake tx with calldata that corresponds to calling process in the Rollup contract.
  */
-async function makeRollupTx(l2Block: L2Block) {
+async function makeRollupTx(l2Block: L2Block, signers: Secp256k1Signer[] = []) {
+  const attestations = signers
+    .map(signer => makeBlockAttestationFromBlock(l2Block, signer))
+    .map(blockAttestation => CommitteeAttestation.fromSignature(blockAttestation.signature))
+    .map(committeeAttestation => committeeAttestation.toViem());
   const header = l2Block.header.toPropose().toViem();
   const blobInput = Blob.getPrefixedEthBlobCommitments(await Blob.getBlobsPerBlock(l2Block.body.toBlobFields()));
   const archive = toHex(l2Block.archive.root.toBuffer());
@@ -820,7 +898,7 @@ async function makeRollupTx(l2Block: L2Block) {
         stateReference,
         oracleInput: { feeAssetPriceModifier: 0n },
       },
-      RollupContract.packAttestations([]),
+      RollupContract.packAttestations(attestations),
       blobInput,
     ],
   });