feat: Archiver does not sync blocks with invalid attestations (#15896)

Archiver now checks committee attestations and refuses to sync a block
if it does not pass validation.

Note that this addresses scenarios where the proposer is malicious, but
does not handle cases where the entire committee is and produces
signatures for a block with an unattested parent. That'll be left for a
future PR.

Builds on #15813

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,81 @@ 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);
+    badBlock2.archive.root = new Fr(0x1002);
+    const badBlock2RollupTx = await makeRollupTx(badBlock2, times(3, Secp256k1Signer.random));
+    const badBlock2BlobHashes = await makeVersionedBlobHashes(badBlock2);
+    const badBlock2Blobs = await makeBlobsFromBlock(badBlock2);
+
+    // Return the archive root for the bad block 2 when queried
+    mockRollupRead.archiveAt.mockImplementation((args: readonly [bigint]) =>
+      Promise.resolve((args[0] === 2n ? badBlock2 : blocks[Number(args[0] - 1n)]).archive.root.toString()),
+    );
+
+    logger.warn(`Created 3 valid blocks`);
+    blocks.forEach(block => logger.warn(`Block ${block.number} with root ${block.archive.root.toString()}`));
+    logger.warn(`Created invalid block 2 with root ${badBlock2.archive.root.toString()}`);
+
+    // 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]);
+    mockRollupRead.archiveAt.mockImplementation((args: readonly [bigint]) =>
+      Promise.resolve(blocks[Number(args[0] - 1n)].archive.root.toString()),
+    );
+
+    // 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 +463,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 +501,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;
@@ -460,7 +547,7 @@ describe('Archiver', () => {
     // Lets take a look to see if we can find re-org stuff!
     await sleep(2000);
 
-    expect(loggerSpy).toHaveBeenCalledWith(`L2 prune has been detected.`);
+    expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining(`L2 prune has been detected`), expect.anything());
 
     // Should also see the block number be reduced
     latestBlockNum = await archiver.getBlockNumber();
@@ -538,7 +625,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 +657,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 +717,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 +747,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 +892,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,8 +911,8 @@ async function makeRollupTx(l2Block: L2Block) {
         stateReference,
         oracleInput: { feeAssetPriceModifier: 0n },
       },
-      RollupContract.packAttestations([]),
-      [],
+      RollupContract.packAttestations(attestations),
+      signers.map(signer => signer.address.toString()),
       blobInput,
     ],
   });