fix: Remove block number from p2p proposals and attestations (#17886)

Block number was NOT being signed over, this meant any node could tweak
it and cause receiving nodes to consider the proposal incorrect. We fix
this by fetching the parent node using the last archive root (which
required adding new indices to the archiver) and computing the block
number by adding one to it.

As for attestations, the block number was not used at all, so we could
remove it.

Fixes A-128

Author: spalladino

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

@@ -106,7 +106,7 @@ describe('Archiver', () => {
   let blobSinkClient: MockProxy<BlobSinkClientInterface>;
   let epochCache: MockProxy<EpochCache>;
   let archiverStore: ArchiverDataStore;
-  let l1Constants: L1RollupConstants & { l1StartBlockHash: Buffer32 };
+  let l1Constants: L1RollupConstants & { l1StartBlockHash: Buffer32; genesisArchiveRoot: Fr };
   let now: number;
 
   let mockRollupRead: MockProxy<MockRollupContractRead>;
@@ -167,6 +167,7 @@ describe('Archiver', () => {
       slotDuration: 24,
       ethereumSlotDuration: 12,
       proofSubmissionEpochs: 1,
+      genesisArchiveRoot: new Fr(GENESIS_ARCHIVE_ROOT),
     };
 
     archiver = new Archiver(

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

@@ -150,7 +150,7 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
     private readonly blobSinkClient: BlobSinkClientInterface,
     private readonly epochCache: EpochCache,
     private readonly instrumentation: ArchiverInstrumentation,
-    private readonly l1constants: L1RollupConstants & { l1StartBlockHash: Buffer32 },
+    private readonly l1constants: L1RollupConstants & { l1StartBlockHash: Buffer32; genesisArchiveRoot: Fr },
     private readonly log: Logger = createLogger('archiver'),
   ) {
     super();
@@ -184,10 +184,11 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
 
     const rollup = new RollupContract(publicClient, config.l1Contracts.rollupAddress);
 
-    const [l1StartBlock, l1GenesisTime, proofSubmissionEpochs] = await Promise.all([
+    const [l1StartBlock, l1GenesisTime, proofSubmissionEpochs, genesisArchiveRoot] = await Promise.all([
       rollup.getL1StartBlock(),
       rollup.getL1GenesisTime(),
       rollup.getProofSubmissionEpochs(),
+      rollup.getGenesisArchiveTreeRoot(),
     ] as const);
 
     const l1StartBlockHash = await publicClient
@@ -204,6 +205,7 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
       slotDuration,
       ethereumSlotDuration,
       proofSubmissionEpochs: Number(proofSubmissionEpochs),
+      genesisArchiveRoot: Fr.fromHexString(genesisArchiveRoot),
     };
 
     const opts = merge({ pollingIntervalMs: 10_000, batchSize: 100 }, mapArchiverConfig(config));
@@ -977,6 +979,10 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
     return Promise.resolve(this.l1constants);
   }
 
+  public getGenesisValues(): Promise<{ genesisArchiveRoot: Fr }> {
+    return Promise.resolve({ genesisArchiveRoot: this.l1constants.genesisArchiveRoot });
+  }
+
   public getRollupAddress(): Promise<EthAddress> {
     return Promise.resolve(this.l1Addresses.rollupAddress);
   }
@@ -1097,6 +1103,22 @@ export class Archiver extends (EventEmitter as new () => ArchiverEmitter) implem
     return limitWithProven === 0 ? [] : await this.store.getPublishedBlocks(from, limitWithProven);
   }
 
+  public getPublishedBlockByHash(blockHash: Fr): Promise<PublishedL2Block | undefined> {
+    return this.store.getPublishedBlockByHash(blockHash);
+  }
+
+  public getPublishedBlockByArchive(archive: Fr): Promise<PublishedL2Block | undefined> {
+    return this.store.getPublishedBlockByArchive(archive);
+  }
+
+  public getBlockHeaderByHash(blockHash: Fr): Promise<BlockHeader | undefined> {
+    return this.store.getBlockHeaderByHash(blockHash);
+  }
+
+  public getBlockHeaderByArchive(archive: Fr): Promise<BlockHeader | undefined> {
+    return this.store.getBlockHeaderByArchive(archive);
+  }
+
   /**
    * Gets an l2 block.
    * @param number - The block number to return.
@@ -1592,9 +1614,21 @@ export class ArchiverStoreHelper
   getPublishedBlock(number: number): Promise<PublishedL2Block | undefined> {
     return this.store.getPublishedBlock(number);
   }
+  getPublishedBlockByHash(blockHash: Fr): Promise<PublishedL2Block | undefined> {
+    return this.store.getPublishedBlockByHash(blockHash);
+  }
+  getPublishedBlockByArchive(archive: Fr): Promise<PublishedL2Block | undefined> {
+    return this.store.getPublishedBlockByArchive(archive);
+  }
   getBlockHeaders(from: number, limit: number): Promise<BlockHeader[]> {
     return this.store.getBlockHeaders(from, limit);
   }
+  getBlockHeaderByHash(blockHash: Fr): Promise<BlockHeader | undefined> {
+    return this.store.getBlockHeaderByHash(blockHash);
+  }
+  getBlockHeaderByArchive(archive: Fr): Promise<BlockHeader | undefined> {
+    return this.store.getBlockHeaderByArchive(archive);
+  }
   getTxEffect(txHash: TxHash): Promise<IndexedTxEffect | undefined> {
     return this.store.getTxEffect(txHash);
   }

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

@@ -61,6 +61,18 @@ export interface ArchiverDataStore {
    */
   getPublishedBlock(number: number): Promise<PublishedL2Block | undefined>;
 
+  /**
+   * Returns the block for the given hash, or undefined if not exists.
+   * @param blockHash - The block hash to return.
+   */
+  getPublishedBlockByHash(blockHash: Fr): Promise<PublishedL2Block | undefined>;
+
+  /**
+   * Returns the block for the given archive root, or undefined if not exists.
+   * @param archive - The archive root to return.
+   */
+  getPublishedBlockByArchive(archive: Fr): Promise<PublishedL2Block | undefined>;
+
   /**
    * Gets up to `limit` amount of published L2 blocks starting from `from`.
    * @param from - Number of the first block to return (inclusive).
@@ -77,6 +89,18 @@ export interface ArchiverDataStore {
    */
   getBlockHeaders(from: number, limit: number): Promise<BlockHeader[]>;
 
+  /**
+   * Returns the block header for the given hash, or undefined if not exists.
+   * @param blockHash - The block hash to return.
+   */
+  getBlockHeaderByHash(blockHash: Fr): Promise<BlockHeader | undefined>;
+
+  /**
+   * Returns the block header for the given archive root, or undefined if not exists.
+   * @param archive - The archive root to return.
+   */
+  getBlockHeaderByArchive(archive: Fr): Promise<BlockHeader | undefined>;
+
   /**
    * Gets a tx effect.
    * @param txHash - The hash of the tx corresponding to the tx effect.

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

@@ -142,6 +142,28 @@ export function describeArchiverDataStore(
         await store.addBlocks(blocks);
         await expect(store.unwindBlocks(5, 1)).rejects.toThrow(/can only unwind blocks from the tip/i);
       });
+
+      it('unwound blocks and headers cannot be retrieved by hash or archive', async () => {
+        await store.addBlocks(blocks);
+        const lastBlock = blocks[blocks.length - 1];
+        const blockHash = await lastBlock.block.hash();
+        const archive = lastBlock.block.archive.root;
+
+        // Verify block and header exist before unwinding
+        expect(await store.getPublishedBlockByHash(blockHash)).toBeDefined();
+        expect(await store.getPublishedBlockByArchive(archive)).toBeDefined();
+        expect(await store.getBlockHeaderByHash(blockHash)).toBeDefined();
+        expect(await store.getBlockHeaderByArchive(archive)).toBeDefined();
+
+        // Unwind the block
+        await store.unwindBlocks(lastBlock.block.number, 1);
+
+        // Verify neither block nor header can be retrieved after unwinding
+        expect(await store.getPublishedBlockByHash(blockHash)).toBeUndefined();
+        expect(await store.getPublishedBlockByArchive(archive)).toBeUndefined();
+        expect(await store.getBlockHeaderByHash(blockHash)).toBeUndefined();
+        expect(await store.getBlockHeaderByArchive(archive)).toBeUndefined();
+      });
     });
 
     describe('getBlocks', () => {
@@ -179,6 +201,86 @@ export function describeArchiverDataStore(
       });
     });
 
+    describe('getPublishedBlockByHash', () => {
+      beforeEach(async () => {
+        await store.addBlocks(blocks);
+      });
+
+      it('retrieves a block by its hash', async () => {
+        const expectedBlock = blocks[5];
+        const blockHash = await expectedBlock.block.hash();
+        const retrievedBlock = await store.getPublishedBlockByHash(blockHash);
+
+        expect(retrievedBlock).toBeDefined();
+        expectBlocksEqual([retrievedBlock!], [expectedBlock]);
+      });
+
+      it('returns undefined for non-existent block hash', async () => {
+        const nonExistentHash = Fr.random();
+        await expect(store.getPublishedBlockByHash(nonExistentHash)).resolves.toBeUndefined();
+      });
+    });
+
+    describe('getPublishedBlockByArchive', () => {
+      beforeEach(async () => {
+        await store.addBlocks(blocks);
+      });
+
+      it('retrieves a block by its archive root', async () => {
+        const expectedBlock = blocks[3];
+        const archive = expectedBlock.block.archive.root;
+        const retrievedBlock = await store.getPublishedBlockByArchive(archive);
+
+        expect(retrievedBlock).toBeDefined();
+        expectBlocksEqual([retrievedBlock!], [expectedBlock]);
+      });
+
+      it('returns undefined for non-existent archive root', async () => {
+        const nonExistentArchive = Fr.random();
+        await expect(store.getPublishedBlockByArchive(nonExistentArchive)).resolves.toBeUndefined();
+      });
+    });
+
+    describe('getBlockHeaderByHash', () => {
+      beforeEach(async () => {
+        await store.addBlocks(blocks);
+      });
+
+      it('retrieves a block header by its hash', async () => {
+        const expectedBlock = blocks[7];
+        const blockHash = await expectedBlock.block.hash();
+        const retrievedHeader = await store.getBlockHeaderByHash(blockHash);
+
+        expect(retrievedHeader).toBeDefined();
+        expect(retrievedHeader!.equals(expectedBlock.block.getBlockHeader())).toBe(true);
+      });
+
+      it('returns undefined for non-existent block hash', async () => {
+        const nonExistentHash = Fr.random();
+        await expect(store.getBlockHeaderByHash(nonExistentHash)).resolves.toBeUndefined();
+      });
+    });
+
+    describe('getBlockHeaderByArchive', () => {
+      beforeEach(async () => {
+        await store.addBlocks(blocks);
+      });
+
+      it('retrieves a block header by its archive root', async () => {
+        const expectedBlock = blocks[2];
+        const archive = expectedBlock.block.archive.root;
+        const retrievedHeader = await store.getBlockHeaderByArchive(archive);
+
+        expect(retrievedHeader).toBeDefined();
+        expect(retrievedHeader!.equals(expectedBlock.block.getBlockHeader())).toBe(true);
+      });
+
+      it('returns undefined for non-existent archive root', async () => {
+        const nonExistentArchive = Fr.random();
+        await expect(store.getBlockHeaderByArchive(nonExistentArchive)).resolves.toBeUndefined();
+      });
+    });
+
     describe('getSyncedL2BlockNumber', () => {
       it('returns the block number before INITIAL_L2_BLOCK_NUM if no blocks have been added', async () => {
         await expect(store.getSynchedL2BlockNumber()).resolves.toEqual(INITIAL_L2_BLOCK_NUM - 1);

yarn-project/archiver/src/archiver/kv_archiver_store/block_store.ts

@@ -66,13 +66,21 @@ export class BlockStore {
   /** Index mapping a contract's address (as a string) to its location in a block */
   #contractIndex: AztecAsyncMap<string, BlockIndexValue>;
 
+  /** Index mapping block hash to block number */
+  #blockHashIndex: AztecAsyncMap<string, number>;
+
+  /** Index mapping block archive to block number */
+  #blockArchiveIndex: AztecAsyncMap<string, number>;
+
   #log = createLogger('archiver:block_store');
 
   constructor(private db: AztecAsyncKVStore) {
     this.#blocks = db.openMap('archiver_blocks');
     this.#blockTxs = db.openMap('archiver_block_txs');
     this.#txEffects = db.openMap('archiver_tx_effects');
     this.#contractIndex = db.openMap('archiver_contract_index');
+    this.#blockHashIndex = db.openMap('archiver_block_hash_index');
+    this.#blockArchiveIndex = db.openMap('archiver_block_archive_index');
     this.#lastSynchedL1Block = db.openSingleton('archiver_last_synched_l1_block');
     this.#lastProvenL2Block = db.openSingleton('archiver_last_proven_l2_block');
     this.#pendingChainValidationStatus = db.openSingleton('archiver_pending_chain_validation_status');
@@ -132,6 +140,10 @@ export class BlockStore {
           blockHash.toString(),
           Buffer.concat(block.block.body.txEffects.map(tx => tx.txHash.toBuffer())),
         );
+
+        // Update indices for block hash and archive
+        await this.#blockHashIndex.set(blockHash.toString(), block.block.number);
+        await this.#blockArchiveIndex.set(block.block.archive.root.toString(), block.block.number);
       }
 
       await this.#lastSynchedL1Block.set(blocks[blocks.length - 1].l1.blockNumber);
@@ -170,6 +182,11 @@ export class BlockStore {
         await Promise.all(block.block.body.txEffects.map(tx => this.#txEffects.delete(tx.txHash.toString())));
         const blockHash = (await block.block.hash()).toString();
         await this.#blockTxs.delete(blockHash);
+
+        // Clean up indices
+        await this.#blockHashIndex.delete(blockHash);
+        await this.#blockArchiveIndex.delete(block.block.archive.root.toString());
+
         this.#log.debug(`Unwound block ${blockNumber} ${blockHash}`);
       }
 
@@ -205,6 +222,66 @@ export class BlockStore {
     return this.getBlockFromBlockStorage(blockNumber, blockStorage);
   }
 
+  /**
+   * Gets an L2 block by its hash.
+   * @param blockHash - The hash of the block to return.
+   * @returns The requested L2 block.
+   */
+  async getBlockByHash(blockHash: L2BlockHash): Promise<PublishedL2Block | undefined> {
+    const blockNumber = await this.#blockHashIndex.getAsync(blockHash.toString());
+    if (blockNumber === undefined) {
+      return undefined;
+    }
+    return this.getBlock(blockNumber);
+  }
+
+  /**
+   * Gets an L2 block by its archive root.
+   * @param archive - The archive root of the block to return.
+   * @returns The requested L2 block.
+   */
+  async getBlockByArchive(archive: Fr): Promise<PublishedL2Block | undefined> {
+    const blockNumber = await this.#blockArchiveIndex.getAsync(archive.toString());
+    if (blockNumber === undefined) {
+      return undefined;
+    }
+    return this.getBlock(blockNumber);
+  }
+
+  /**
+   * Gets a block header by its hash.
+   * @param blockHash - The hash of the block to return.
+   * @returns The requested block header.
+   */
+  async getBlockHeaderByHash(blockHash: L2BlockHash): Promise<BlockHeader | undefined> {
+    const blockNumber = await this.#blockHashIndex.getAsync(blockHash.toString());
+    if (blockNumber === undefined) {
+      return undefined;
+    }
+    const blockStorage = await this.#blocks.getAsync(blockNumber);
+    if (!blockStorage || !blockStorage.header) {
+      return undefined;
+    }
+    return L2BlockHeader.fromBuffer(blockStorage.header).toBlockHeader();
+  }
+
+  /**
+   * Gets a block header by its archive root.
+   * @param archive - The archive root of the block to return.
+   * @returns The requested block header.
+   */
+  async getBlockHeaderByArchive(archive: Fr): Promise<BlockHeader | undefined> {
+    const blockNumber = await this.#blockArchiveIndex.getAsync(archive.toString());
+    if (blockNumber === undefined) {
+      return undefined;
+    }
+    const blockStorage = await this.#blocks.getAsync(blockNumber);
+    if (!blockStorage || !blockStorage.header) {
+      return undefined;
+    }
+    return L2BlockHeader.fromBuffer(blockStorage.header).toBlockHeader();
+  }
+
   /**
    * Gets the headers for a sequence of L2 blocks.
    * @param start - Number of the first block to return (inclusive).