refactor!: BlockHash type cleanup (#19970)

As Santiago proposed
[here](#19899 (review))
makes sense to have `BlockHash` extend from `Fr` instead from `Buffer32`
because the hash is the output of Poseidon2 hash and hence natively a
field.

Also introduced branding to avoid collisions with `Fr` and cleaned up
the type from unused functions.

Author: benesjan

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

@@ -4,7 +4,7 @@ import type { EthAddress } from '@aztec/foundation/eth-address';
 import { isDefined } from '@aztec/foundation/types';
 import type { FunctionSelector } from '@aztec/stdlib/abi';
 import type { AztecAddress } from '@aztec/stdlib/aztec-address';
-import { CheckpointedL2Block, CommitteeAttestation, L2Block, type L2Tips } from '@aztec/stdlib/block';
+import { type BlockHash, CheckpointedL2Block, CommitteeAttestation, L2Block, type L2Tips } from '@aztec/stdlib/block';
 import { Checkpoint, PublishedCheckpoint } from '@aztec/stdlib/checkpoint';
 import type { ContractClassPublic, ContractDataSource, ContractInstanceWithAddress } from '@aztec/stdlib/contract';
 import { type L1RollupConstants, getSlotRangeForEpoch } from '@aztec/stdlib/epoch-helpers';
@@ -121,7 +121,7 @@ export abstract class ArchiverDataSourceBase
     return this.store.getCheckpointedBlocks(from, limit);
   }
 
-  public getBlockHeaderByHash(blockHash: Fr): Promise<BlockHeader | undefined> {
+  public getBlockHeaderByHash(blockHash: BlockHash): Promise<BlockHeader | undefined> {
     return this.store.getBlockHeaderByHash(blockHash);
   }
 
@@ -347,15 +347,15 @@ export abstract class ArchiverDataSourceBase
     return this.store.getBlocks(from, limit);
   }
 
-  public getCheckpointedBlockByHash(blockHash: Fr): Promise<CheckpointedL2Block | undefined> {
+  public getCheckpointedBlockByHash(blockHash: BlockHash): Promise<CheckpointedL2Block | undefined> {
     return this.store.getCheckpointedBlockByHash(blockHash);
   }
 
   public getCheckpointedBlockByArchive(archive: Fr): Promise<CheckpointedL2Block | undefined> {
     return this.store.getCheckpointedBlockByArchive(archive);
   }
 
-  public async getL2BlockByHash(blockHash: Fr): Promise<L2Block | undefined> {
+  public async getL2BlockByHash(blockHash: BlockHash): Promise<L2Block | undefined> {
     const checkpointedBlock = await this.store.getCheckpointedBlockByHash(blockHash);
     return checkpointedBlock?.block;
   }

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

@@ -351,7 +351,7 @@ export class BlockStore {
   }
 
   private async addBlockToDatabase(block: L2Block, checkpointNumber: number, indexWithinCheckpoint: number) {
-    const blockHash = BlockHash.fromField(await block.hash());
+    const blockHash = await block.hash();
 
     await this.#blocks.set(block.number, {
       header: block.header.toBuffer(),
@@ -624,7 +624,7 @@ export class BlockStore {
     }
   }
 
-  async getCheckpointedBlockByHash(blockHash: Fr): Promise<CheckpointedL2Block | undefined> {
+  async getCheckpointedBlockByHash(blockHash: BlockHash): Promise<CheckpointedL2Block | undefined> {
     const blockNumber = await this.#blockHashIndex.getAsync(blockHash.toString());
     if (blockNumber === undefined) {
       return undefined;

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

@@ -335,7 +335,7 @@ describe('KVArchiverDataStore', () => {
       await store.addCheckpoints(publishedCheckpoints);
       const lastCheckpoint = publishedCheckpoints[publishedCheckpoints.length - 1];
       const lastBlock = lastCheckpoint.checkpoint.blocks[0];
-      const blockHash = (await lastBlock.header.hash()).toField();
+      const blockHash = await lastBlock.header.hash();
       const archive = lastBlock.archive.root;
 
       // Verify block and header exist before removing
@@ -639,7 +639,7 @@ describe('KVArchiverDataStore', () => {
       // Check each block by its hash
       for (let i = 0; i < checkpoint.checkpoint.blocks.length; i++) {
         const block = checkpoint.checkpoint.blocks[i];
-        const blockHash = (await block.header.hash()).toField();
+        const blockHash = await block.header.hash();
         const retrievedBlock = await store.getCheckpointedBlockByHash(blockHash);
 
         expect(retrievedBlock).toBeDefined();
@@ -797,8 +797,8 @@ describe('KVArchiverDataStore', () => {
       await store.addProposedBlocks([block1, block2]);
 
       // getBlockByHash should work for uncheckpointed blocks
-      const hash1 = (await block1.header.hash()).toField();
-      const hash2 = (await block2.header.hash()).toField();
+      const hash1 = await block1.header.hash();
+      const hash2 = await block2.header.hash();
 
       const retrieved1 = await store.getBlockByHash(hash1);
       expect(retrieved1!.equals(block1)).toBe(true);
@@ -874,7 +874,7 @@ describe('KVArchiverDataStore', () => {
       });
       await store.addProposedBlocks([block1]);
 
-      const hash = (await block1.header.hash()).toField();
+      const hash = await block1.header.hash();
 
       // getCheckpointedBlockByHash should return undefined
       expect(await store.getCheckpointedBlockByHash(hash)).toBeUndefined();
@@ -1666,15 +1666,15 @@ describe('KVArchiverDataStore', () => {
     it('retrieves a block by its hash', async () => {
       const expectedCheckpoint = publishedCheckpoints[5];
       const expectedBlock = expectedCheckpoint.checkpoint.blocks[0];
-      const blockHash = (await expectedBlock.header.hash()).toField();
+      const blockHash = await expectedBlock.header.hash();
       const retrievedBlock = await store.getCheckpointedBlockByHash(blockHash);
 
       expect(retrievedBlock).toBeDefined();
       expectCheckpointedBlockEquals(retrievedBlock!, expectedBlock, expectedCheckpoint);
     });
 
     it('returns undefined for non-existent block hash', async () => {
-      const nonExistentHash = Fr.random();
+      const nonExistentHash = BlockHash.random();
       await expect(store.getCheckpointedBlockByHash(nonExistentHash)).resolves.toBeUndefined();
     });
   });
@@ -1707,15 +1707,15 @@ describe('KVArchiverDataStore', () => {
 
     it('retrieves a block header by its hash', async () => {
       const expectedBlock = publishedCheckpoints[7].checkpoint.blocks[0];
-      const blockHash = (await expectedBlock.header.hash()).toField();
+      const blockHash = await expectedBlock.header.hash();
       const retrievedHeader = await store.getBlockHeaderByHash(blockHash);
 
       expect(retrievedHeader).toBeDefined();
       expect(retrievedHeader!.equals(expectedBlock.header)).toBe(true);
     });
 
     it('returns undefined for non-existent block hash', async () => {
-      const nonExistentHash = Fr.random();
+      const nonExistentHash = BlockHash.random();
       await expect(store.getBlockHeaderByHash(nonExistentHash)).resolves.toBeUndefined();
     });
   });
@@ -3320,7 +3320,7 @@ describe('KVArchiverDataStore', () => {
       await store.addProposedBlocks([block1, block2]);
 
       // Verify block2 is retrievable by hash and archive before removal
-      const block2Hash = (await block2.header.hash()).toField();
+      const block2Hash = await block2.header.hash();
       const block2Archive = block2.archive.root;
 
       expect(await store.getBlockByHash(block2Hash)).toBeDefined();
@@ -3346,7 +3346,7 @@ describe('KVArchiverDataStore', () => {
       }
 
       // Verify block1's data is still intact
-      const block1Hash = (await block1.header.hash()).toField();
+      const block1Hash = await block1.header.hash();
       const block1Archive = block1.archive.root;
 
       expect(await store.getBlockByHash(block1Hash)).toBeDefined();

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

@@ -291,7 +291,7 @@ export class KVArchiverDataStore implements ContractDataSource {
    * Returns the block for the given hash, or undefined if not exists.
    * @param blockHash - The block hash to return.
    */
-  getCheckpointedBlockByHash(blockHash: Fr): Promise<CheckpointedL2Block | undefined> {
+  getCheckpointedBlockByHash(blockHash: BlockHash): Promise<CheckpointedL2Block | undefined> {
     return this.#blockStore.getCheckpointedBlockByHash(blockHash);
   }
   /**
@@ -312,8 +312,8 @@ export class KVArchiverDataStore implements ContractDataSource {
    * Returns the block for the given hash, or undefined if not exists.
    * @param blockHash - The block hash to return.
    */
-  getBlockByHash(blockHash: Fr): Promise<L2Block | undefined> {
-    return this.#blockStore.getBlockByHash(BlockHash.fromField(blockHash));
+  getBlockByHash(blockHash: BlockHash): Promise<L2Block | undefined> {
+    return this.#blockStore.getBlockByHash(blockHash);
   }
   /**
    * Returns the block for the given archive root, or undefined if not exists.
@@ -357,8 +357,8 @@ export class KVArchiverDataStore implements ContractDataSource {
    * 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> {
-    return this.#blockStore.getBlockHeaderByHash(BlockHash.fromField(blockHash));
+  getBlockHeaderByHash(blockHash: BlockHash): Promise<BlockHeader | undefined> {
+    return this.#blockStore.getBlockHeaderByHash(blockHash);
   }
 
   /**

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

@@ -271,7 +271,7 @@ export class LogStore {
     });
   }
 
-  #packWithBlockHash(blockHash: Fr, data: Buffer<ArrayBufferLike>[]): Buffer<ArrayBufferLike> {
+  #packWithBlockHash(blockHash: BlockHash, data: Buffer<ArrayBufferLike>[]): Buffer<ArrayBufferLike> {
     return Buffer.concat([blockHash.toBuffer(), ...data]);
   }
 
@@ -282,7 +282,7 @@ export class LogStore {
       throw new Error('Failed to read block hash from log entry buffer');
     }
 
-    return BlockHash.fromField(blockHash);
+    return new BlockHash(blockHash);
   }
 
   deleteLogs(blocks: L2Block[]): Promise<boolean> {

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

@@ -195,7 +195,7 @@ export class MockL2BlockSource implements L2BlockSource, ContractDataSource {
     return checkpoint;
   }
 
-  public async getCheckpointedBlockByHash(blockHash: Fr): Promise<CheckpointedL2Block | undefined> {
+  public async getCheckpointedBlockByHash(blockHash: BlockHash): Promise<CheckpointedL2Block | undefined> {
     for (const block of this.l2Blocks) {
       const hash = await block.hash();
       if (hash.equals(blockHash)) {
@@ -225,7 +225,7 @@ export class MockL2BlockSource implements L2BlockSource, ContractDataSource {
     );
   }
 
-  public async getL2BlockByHash(blockHash: Fr): Promise<L2Block | undefined> {
+  public async getL2BlockByHash(blockHash: BlockHash): Promise<L2Block | undefined> {
     for (const block of this.l2Blocks) {
       const hash = await block.hash();
       if (hash.equals(blockHash)) {
@@ -240,7 +240,7 @@ export class MockL2BlockSource implements L2BlockSource, ContractDataSource {
     return Promise.resolve(block);
   }
 
-  public async getBlockHeaderByHash(blockHash: Fr): Promise<BlockHeader | undefined> {
+  public async getBlockHeaderByHash(blockHash: BlockHash): Promise<BlockHeader | undefined> {
     for (const block of this.l2Blocks) {
       const hash = await block.hash();
       if (hash.equals(blockHash)) {
@@ -322,7 +322,7 @@ export class MockL2BlockSource implements L2BlockSource, ContractDataSource {
     return {
       data: txEffect,
       l2BlockNumber: block.number,
-      l2BlockHash: BlockHash.fromField(await block.hash()),
+      l2BlockHash: await block.hash(),
       txIndexInBlock: block.body.txEffects.indexOf(txEffect),
     };
   }
@@ -343,7 +343,7 @@ export class MockL2BlockSource implements L2BlockSource, ContractDataSource {
             TxExecutionResult.SUCCESS,
             undefined,
             txEffect.transactionFee.toBigInt(),
-            BlockHash.fromField(await block.hash()),
+            await block.hash(),
             block.number,
           );
         }

yarn-project/aztec-node/src/aztec-node/server.ts

@@ -570,8 +570,8 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
    * @returns The requested block.
    */
   public async getBlock(block: BlockParameter): Promise<L2Block | undefined> {
-    if (BlockHash.isL2BlockHash(block)) {
-      return this.getBlockByHash(Fr.fromBuffer(block.toBuffer()));
+    if (BlockHash.isBlockHash(block)) {
+      return this.getBlockByHash(block);
     }
     const blockNumber = block === 'latest' ? await this.getBlockNumber() : (block as BlockNumber);
     if (blockNumber === BlockNumber.ZERO) {
@@ -585,9 +585,9 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
    * @param blockHash - The block hash being requested.
    * @returns The requested block.
    */
-  public async getBlockByHash(blockHash: Fr): Promise<L2Block | undefined> {
+  public async getBlockByHash(blockHash: BlockHash): Promise<L2Block | undefined> {
     const initialBlockHash = await this.#getInitialHeaderHash();
-    if (blockHash.equals(Fr.fromBuffer(initialBlockHash.toBuffer()))) {
+    if (blockHash.equals(initialBlockHash)) {
       return this.buildInitialBlock();
     }
     return await this.blockSource.getL2BlockByHash(blockHash);
@@ -697,8 +697,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
     if (referenceBlock) {
       const initialBlockHash = await this.#getInitialHeaderHash();
       if (!referenceBlock.equals(initialBlockHash)) {
-        const blockHashFr = Fr.fromBuffer(referenceBlock.toBuffer());
-        const header = await this.blockSource.getBlockHeaderByHash(blockHashFr);
+        const header = await this.blockSource.getBlockHeaderByHash(referenceBlock);
         if (!header) {
           throw new Error(
             `Block ${referenceBlock.toString()} not found in the node. This might indicate a reorg has occurred.`,
@@ -718,8 +717,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
     if (referenceBlock) {
       const initialBlockHash = await this.#getInitialHeaderHash();
       if (!referenceBlock.equals(initialBlockHash)) {
-        const blockHashFr = Fr.fromBuffer(referenceBlock.toBuffer());
-        const header = await this.blockSource.getBlockHeaderByHash(blockHashFr);
+        const header = await this.blockSource.getBlockHeaderByHash(referenceBlock);
         if (!header) {
           throw new Error(
             `Block ${referenceBlock.toString()} not found in the node. This might indicate a reorg has occurred.`,
@@ -915,7 +913,7 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
       }
       return {
         l2BlockNumber: BlockNumber(Number(blockNumber)),
-        l2BlockHash: BlockHash.fromField(blockHash),
+        l2BlockHash: new BlockHash(blockHash),
         data: index,
       };
     });
@@ -1118,14 +1116,13 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
   }
 
   public async getBlockHeader(block: BlockParameter = 'latest'): Promise<BlockHeader | undefined> {
-    if (BlockHash.isL2BlockHash(block)) {
+    if (BlockHash.isBlockHash(block)) {
       const initialBlockHash = await this.#getInitialHeaderHash();
       if (block.equals(initialBlockHash)) {
         // Block source doesn't handle initial header so we need to handle the case separately.
         return this.worldStateSynchronizer.getCommitted().getInitialHeader();
       }
-      const blockHashFr = Fr.fromBuffer(block.toBuffer());
-      return this.blockSource.getBlockHeaderByHash(blockHashFr);
+      return this.blockSource.getBlockHeaderByHash(block);
     } else {
       // Block source doesn't handle initial header so we need to handle the case separately.
       const blockNumber = block === 'latest' ? await this.getBlockNumber() : (block as BlockNumber);
@@ -1442,15 +1439,14 @@ export class AztecNodeService implements AztecNode, AztecNodeAdmin, Traceable {
       return this.worldStateSynchronizer.getCommitted();
     }
 
-    if (BlockHash.isL2BlockHash(block)) {
+    if (BlockHash.isBlockHash(block)) {
       const initialBlockHash = await this.#getInitialHeaderHash();
       if (block.equals(initialBlockHash)) {
         // Block source doesn't handle initial header so we need to handle the case separately.
         return this.worldStateSynchronizer.getSnapshot(BlockNumber.ZERO);
       }
 
-      const blockHashFr = Fr.fromBuffer(block.toBuffer());
-      const header = await this.blockSource.getBlockHeaderByHash(blockHashFr);
+      const header = await this.blockSource.getBlockHeaderByHash(block);
       if (!header) {
         throw new Error(
           `Block hash ${block.toString()} not found when querying world state. If the node API has been queried with anchor block hash possibly a reorg has occurred.`,

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

@@ -59,12 +59,12 @@ describe('e2e_simple', () => {
       const initialBlockByHash = await aztecNode.getBlock(initialHeaderHash);
       expect(initialBlockByHash).toBeDefined();
       const initialBlockHash = await initialBlockByHash!.hash();
-      expect(initialBlockHash.equals(initialHeaderHash.toField())).toBeTrue();
+      expect(initialBlockHash.equals(initialHeaderHash)).toBeTrue();
       expect(initialBlockByHash?.body.txEffects.length).toBe(0);
       const initialBlockByNumber = await aztecNode.getBlock(BlockNumber.ZERO);
       expect(initialBlockByNumber).toBeDefined();
       const initialBlockByNumberHash = await initialBlockByNumber!.hash();
-      expect(initialBlockByNumberHash.equals(initialHeaderHash.toField())).toBeTrue();
+      expect(initialBlockByNumberHash.equals(initialHeaderHash)).toBeTrue();
       expect(initialBlockByNumber?.body.txEffects.length).toBe(0);
     });
 

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

@@ -113,7 +113,9 @@ describe('e2e_snapshot_sync', () => {
 
     log.warn(`Checking for L2 block ${L2_TARGET_BLOCK_NUM} with hash ${blockHash} on both nodes`);
     const getBlockHashLeafIndex = (node: AztecNode) =>
-      node.findLeavesIndexes(BlockNumber(L2_TARGET_BLOCK_NUM), MerkleTreeId.ARCHIVE, [blockHash]).then(([i]) => i);
+      node
+        .findLeavesIndexes(BlockNumber(L2_TARGET_BLOCK_NUM), MerkleTreeId.ARCHIVE, [blockHash.toFr()])
+        .then(([i]) => i);
     expect(await getBlockHashLeafIndex(context.aztecNode)).toBeDefined();
     expect(await getBlockHashLeafIndex(node)).toBeDefined();
 
@@ -228,7 +230,9 @@ describe('e2e_snapshot_sync', () => {
 
     log.warn(`Checking for L2 block ${L2_TARGET_BLOCK_NUM} with hash ${blockHash} on both nodes`);
     const getBlockHashLeafIndex = (node: AztecNode) =>
-      node.findLeavesIndexes(BlockNumber(L2_TARGET_BLOCK_NUM), MerkleTreeId.ARCHIVE, [blockHash]).then(([i]) => i);
+      node
+        .findLeavesIndexes(BlockNumber(L2_TARGET_BLOCK_NUM), MerkleTreeId.ARCHIVE, [blockHash.toFr()])
+        .then(([i]) => i);
     expect(await getBlockHashLeafIndex(context.aztecNode)).toBeDefined();
     expect(await getBlockHashLeafIndex(node)).toBeDefined();
 

yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.test.ts

@@ -322,7 +322,7 @@ describe('KV TX pool', () => {
     // modify tx1 to return no archive indices
     tx1.data.constants.anchorBlockHeader.globalVariables.blockNumber = BlockNumber(1);
     const tx1HeaderHash = await tx1.data.constants.anchorBlockHeader.hash();
-    const tx1HeaderHashFr = tx1HeaderHash.toField();
+    const tx1HeaderHashFr = tx1HeaderHash;
     db.findLeafIndices.mockImplementation((tree, leaves) => {
       if (tree === MerkleTreeId.ARCHIVE) {
         return Promise.resolve((leaves as Fr[]).map(l => (l.equals(tx1HeaderHashFr) ? undefined : 1n)));