feat(p2p): add tx validation for contract class id verification

.claude/settings.local.json

@@ -0,0 +1,11 @@
+{
+  "permissions": {
+    "allow": [
+      "Bash(gh pr:*)",
+      "Bash(curl -s -X POST https://ethereum-sepolia-rpc.publicnode.com -H \"Content-Type: application/json\" -d '{\"\"\"\"jsonrpc\"\"\"\":\"\"\"\"2.0\"\"\"\",\"\"\"\"method\"\"\"\":\"\"\"\"eth_getBlockByNumber\"\"\"\",\"\"\"\"params\"\"\"\":[\"\"\"\"finalized\"\"\"\",false],\"\"\"\"id\"\"\"\":1}')",
+      "Bash(curl -s -X POST https://ethereum-rpc.publicnode.com -H \"Content-Type: application/json\" -d '{\"\"\"\"jsonrpc\"\"\"\":\"\"\"\"2.0\"\"\"\",\"\"\"\"method\"\"\"\":\"\"\"\"eth_getBlockByNumber\"\"\"\",\"\"\"\"params\"\"\"\":[\"\"\"\"finalized\"\"\"\",false],\"\"\"\"id\"\"\"\":1}')",
+      "Bash(curl -s -X POST https://ethereum-sepolia-rpc.publicnode.com -H \"Content-Type: application/json\" -d '{\"\"\"\"jsonrpc\"\"\"\":\"\"\"\"2.0\"\"\"\",\"\"\"\"method\"\"\"\":\"\"\"\"eth_getBlockByNumber\"\"\"\",\"\"\"\"params\"\"\"\":[\"\"\"\"latest\"\"\"\",false],\"\"\"\"id\"\"\"\":1}')",
+      "Bash(curl -s -X POST https://ethereum-rpc.publicnode.com -H \"Content-Type: application/json\" -d '{\"\"\"\"jsonrpc\"\"\"\":\"\"\"\"2.0\"\"\"\",\"\"\"\"method\"\"\"\":\"\"\"\"eth_getBlockByNumber\"\"\"\",\"\"\"\"params\"\"\"\":[\"\"\"\"latest\"\"\"\",false],\"\"\"\"id\"\"\"\":1}')"
+    ]
+  }
+}

yarn-project/archiver/src/factory.ts

@@ -13,7 +13,7 @@ import { protocolContractNames } from '@aztec/protocol-contracts';
 import { BundledProtocolContractsProvider } from '@aztec/protocol-contracts/providers/bundle';
 import { FunctionType, decodeFunctionSignature } from '@aztec/stdlib/abi';
 import type { ArchiverEmitter } from '@aztec/stdlib/block';
-import { type ContractClassPublic, computePublicBytecodeCommitment } from '@aztec/stdlib/contract';
+import { type ContractClassPublicWithCommitment, computePublicBytecodeCommitment } from '@aztec/stdlib/contract';
 import type { DataStoreConfig } from '@aztec/stdlib/kv-store';
 import { getTelemetryClient } from '@aztec/telemetry-client';
 
@@ -185,17 +185,18 @@ export async function registerProtocolContracts(store: KVArchiverDataStore) {
       continue;
     }
 
-    const contractClassPublic: ContractClassPublic = {
+    const publicBytecodeCommitment = await computePublicBytecodeCommitment(contract.contractClass.packedBytecode);
+    const contractClassPublic: ContractClassPublicWithCommitment = {
       ...contract.contractClass,
+      publicBytecodeCommitment,
     };
 
     const publicFunctionSignatures = contract.artifact.functions
       .filter(fn => fn.functionType === FunctionType.PUBLIC)
       .map(fn => decodeFunctionSignature(fn.name, fn.parameters));
 
     await store.registerContractFunctionSignatures(publicFunctionSignatures);
-    const bytecodeCommitment = await computePublicBytecodeCommitment(contractClassPublic.packedBytecode);
-    await store.addContractClasses([contractClassPublic], [bytecodeCommitment], BlockNumber(blockNumber));
+    await store.addContractClasses([contractClassPublic], BlockNumber(blockNumber));
     await store.addContractInstances([contract.instance], BlockNumber(blockNumber));
   }
 }

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

@@ -8,7 +8,11 @@ import {
 } from '@aztec/protocol-contracts/instance-registry';
 import type { L2Block, ValidateCheckpointResult } from '@aztec/stdlib/block';
 import { type PublishedCheckpoint, validateCheckpoint } from '@aztec/stdlib/checkpoint';
-import { computeContractAddressFromInstance, computePublicBytecodeCommitment } from '@aztec/stdlib/contract';
+import {
+  type ContractClassPublicWithCommitment,
+  computeContractAddressFromInstance,
+  computeContractClassId,
+} from '@aztec/stdlib/contract';
 import type { ContractClassLog, PrivateLog, PublicLog } from '@aztec/stdlib/logs';
 import type { UInt64 } from '@aztec/stdlib/types';
 
@@ -315,18 +319,37 @@ export class ArchiverDataStoreUpdater {
       .filter(log => ContractClassPublishedEvent.isContractClassPublishedEvent(log))
       .map(log => ContractClassPublishedEvent.fromLog(log));
 
-    const contractClasses = await Promise.all(contractClassPublishedEvents.map(e => e.toContractClassPublic()));
-    if (contractClasses.length > 0) {
-      contractClasses.forEach(c => this.log.verbose(`${Operation[operation]} contract class ${c.id.toString()}`));
-      if (operation == Operation.Store) {
-        // TODO: Will probably want to create some worker threads to compute these bytecode commitments as they are expensive
-        const commitments = await Promise.all(
-          contractClasses.map(c => computePublicBytecodeCommitment(c.packedBytecode)),
-        );
-        return await this.store.addContractClasses(contractClasses, commitments, blockNum);
-      } else if (operation == Operation.Delete) {
+    if (operation == Operation.Delete) {
+      const contractClasses = contractClassPublishedEvents.map(e => e.toContractClassPublic());
+      if (contractClasses.length > 0) {
+        contractClasses.forEach(c => this.log.verbose(`${Operation[operation]} contract class ${c.id.toString()}`));
         return await this.store.deleteContractClasses(contractClasses, blockNum);
       }
+      return true;
+    }
+
+    // Compute bytecode commitments and validate class IDs in a single pass.
+    const contractClasses: ContractClassPublicWithCommitment[] = [];
+    for (const event of contractClassPublishedEvents) {
+      const contractClass = await event.toContractClassPublicWithBytecodeCommitment();
+      const computedClassId = await computeContractClassId({
+        artifactHash: contractClass.artifactHash,
+        privateFunctionsRoot: contractClass.privateFunctionsRoot,
+        publicBytecodeCommitment: contractClass.publicBytecodeCommitment,
+      });
+      if (!computedClassId.equals(contractClass.id)) {
+        this.log.warn(
+          `Skipping contract class with mismatched id at block ${blockNum}. Claimed ${contractClass.id}, computed ${computedClassId}`,
+          { blockNum, contractClassId: event.contractClassId.toString() },
+        );
+        continue;
+      }
+      contractClasses.push(contractClass);
+    }
+
+    if (contractClasses.length > 0) {
+      contractClasses.forEach(c => this.log.verbose(`${Operation[operation]} contract class ${c.id.toString()}`));
+      return await this.store.addContractClasses(contractClasses, blockNum);
     }
     return true;
   }

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

@@ -24,6 +24,7 @@ import {
 import { Checkpoint, PublishedCheckpoint, randomCheckpointInfo } from '@aztec/stdlib/checkpoint';
 import {
   type ContractClassPublic,
+  type ContractClassPublicWithCommitment,
   type ContractInstanceWithAddress,
   SerializableContractInstance,
   computePublicBytecodeCommitment,
@@ -75,6 +76,13 @@ async function addProposedBlocks(
   return result;
 }
 
+async function withCommitment(contractClass: ContractClassPublic): Promise<ContractClassPublicWithCommitment> {
+  return {
+    ...contractClass,
+    publicBytecodeCommitment: await computePublicBytecodeCommitment(contractClass.packedBytecode),
+  };
+}
+
 describe('KVArchiverDataStore', () => {
   let store: KVArchiverDataStore;
   let publishedCheckpoints: PublishedCheckpoint[];
@@ -2346,11 +2354,7 @@ describe('KVArchiverDataStore', () => {
 
     beforeEach(async () => {
       contractClass = await makeContractClassPublic();
-      await store.addContractClasses(
-        [contractClass],
-        [await computePublicBytecodeCommitment(contractClass.packedBytecode)],
-        BlockNumber(blockNum),
-      );
+      await store.addContractClasses([await withCommitment(contractClass)], BlockNumber(blockNum));
     });
 
     it('returns previously stored contract class', async () => {
@@ -2364,14 +2368,15 @@ describe('KVArchiverDataStore', () => {
 
     it('throws if the same contract class is added again', async () => {
       await expect(
-        store.addContractClasses(
-          [contractClass],
-          [await computePublicBytecodeCommitment(contractClass.packedBytecode)],
-          BlockNumber(blockNum + 1),
-        ),
+        store.addContractClasses([await withCommitment(contractClass)], BlockNumber(blockNum + 1)),
       ).rejects.toThrow(/already exists/);
     });
 
+    it('returns contract class if deleted at a later block number', async () => {
+      await store.deleteContractClasses([contractClass], BlockNumber(blockNum + 1));
+      await expect(store.getContractClass(contractClass.id)).resolves.toMatchObject(contractClass);
+    });
+
     it('returns undefined if contract class is not found', async () => {
       await expect(store.getContractClass(Fr.random())).resolves.toBeUndefined();
     });
@@ -3398,21 +3403,17 @@ describe('KVArchiverDataStore', () => {
 
     it('throws when adding the same contract class twice', async () => {
       const contractClass = await makeContractClassPublic();
-      const commitment = await computePublicBytecodeCommitment(contractClass.packedBytecode);
+      const contractClassWithCommitment = await withCommitment(contractClass);
 
-      await store.addContractClasses([contractClass], [commitment], BlockNumber(1));
-      await expect(store.addContractClasses([contractClass], [commitment], BlockNumber(2))).rejects.toThrow(
+      await store.addContractClasses([contractClassWithCommitment], BlockNumber(1));
+      await expect(store.addContractClasses([contractClassWithCommitment], BlockNumber(2))).rejects.toThrow(
         /already exists/,
       );
     });
 
     it('throws when adding the same contract instance twice', async () => {
       const contractClass = await makeContractClassPublic();
-      await store.addContractClasses(
-        [contractClass],
-        [await computePublicBytecodeCommitment(contractClass.packedBytecode)],
-        BlockNumber(1),
-      );
+      await store.addContractClasses([await withCommitment(contractClass)], BlockNumber(1));
 
       const instance = {
         ...(await SerializableContractInstance.random({

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

@@ -16,6 +16,7 @@ import {
 import type { CheckpointData, PublishedCheckpoint } from '@aztec/stdlib/checkpoint';
 import type {
   ContractClassPublic,
+  ContractClassPublicWithCommitment,
   ContractDataSource,
   ContractInstanceUpdateWithAddress,
   ContractInstanceWithAddress,
@@ -165,19 +166,14 @@ export class KVArchiverDataStore implements ContractDataSource {
 
   /**
    * Add new contract classes from an L2 block to the store's list.
-   * @param data - List of contract classes to be added.
-   * @param bytecodeCommitments - Bytecode commitments for the contract classes.
+   * @param data - List of contract classes (with bytecode commitments) to be added.
    * @param blockNumber - Number of the L2 block the contracts were registered in.
    * @returns True if the operation is successful.
    */
-  async addContractClasses(
-    data: ContractClassPublic[],
-    bytecodeCommitments: Fr[],
-    blockNumber: BlockNumber,
-  ): Promise<boolean> {
+  async addContractClasses(data: ContractClassPublicWithCommitment[], blockNumber: BlockNumber): Promise<boolean> {
     return (
       await Promise.all(
-        data.map((c, i) => this.#contractClassStore.addContractClass(c, bytecodeCommitments[i], blockNumber)),
+        data.map(c => this.#contractClassStore.addContractClass(c, c.publicBytecodeCommitment, blockNumber)),
       )
     ).every(Boolean);
   }

yarn-project/p2p/src/msg_validators/tx_validator/data_validator.test.ts

@@ -1,14 +1,18 @@
 import {
   CONTRACT_CLASS_LOG_SIZE_IN_FIELDS,
+  CONTRACT_CLASS_PUBLISHED_MAGIC_VALUE,
   MAX_CONTRACT_CLASS_LOGS_PER_TX,
   MAX_FR_CALLDATA_TO_ALL_ENQUEUED_CALLS,
 } from '@aztec/constants';
 import { timesParallel } from '@aztec/foundation/collection';
 import { randomInt } from '@aztec/foundation/crypto/random';
 import { Fr } from '@aztec/foundation/curves/bn254';
+import { ProtocolContractAddress } from '@aztec/protocol-contracts';
+import { bufferAsFields } from '@aztec/stdlib/abi';
 import { AztecAddress } from '@aztec/stdlib/aztec-address';
+import { computeContractClassId, computePublicBytecodeCommitment } from '@aztec/stdlib/contract';
 import { LogHash, ScopedLogHash } from '@aztec/stdlib/kernel';
-import { ContractClassLogFields } from '@aztec/stdlib/logs';
+import { ContractClassLog, ContractClassLogFields } from '@aztec/stdlib/logs';
 import { mockTx } from '@aztec/stdlib/testing';
 import {
   TX_ERROR_CALLDATA_COUNT_MISMATCH,
@@ -17,6 +21,8 @@ import {
   TX_ERROR_CONTRACT_CLASS_LOG_COUNT,
   TX_ERROR_CONTRACT_CLASS_LOG_LENGTH,
   TX_ERROR_INCORRECT_CALLDATA,
+  TX_ERROR_INCORRECT_CONTRACT_CLASS_ID,
+  TX_ERROR_MALFORMED_CONTRACT_CLASS_LOG,
   type Tx,
 } from '@aztec/stdlib/tx';
 
@@ -243,4 +249,119 @@ describe('TxDataValidator', () => {
 
     await expectInvalid(badTxs[0], TX_ERROR_CONTRACT_CLASS_LOG_LENGTH);
   });
+
+  describe('contract class id validation', () => {
+    /**
+     * Builds a ContractClassLog encoding a ContractClassPublishedEvent.
+     * Layout: [magic, contractClassId, version, artifactHash, privateFunctionsRoot, ...bytecodeAsFields]
+     */
+    async function buildContractClassLog(opts?: { contractClassId?: Fr }): Promise<{
+      log: ContractClassLog;
+      emittedLength: number;
+    }> {
+      const artifactHash = Fr.random();
+      const privateFunctionsRoot = Fr.random();
+      const packedBytecode = Buffer.from('aabbccdd', 'hex');
+
+      const bytecodeCommitment = await computePublicBytecodeCommitment(packedBytecode);
+      const correctClassId = await computeContractClassId({
+        artifactHash,
+        privateFunctionsRoot,
+        publicBytecodeCommitment: bytecodeCommitment,
+      });
+      const contractClassId = opts?.contractClassId ?? correctClassId;
+
+      const bytecodeFields = bufferAsFields(packedBytecode, CONTRACT_CLASS_LOG_SIZE_IN_FIELDS);
+      let lastNonZero = bytecodeFields.length - 1;
+      while (lastNonZero >= 0 && bytecodeFields[lastNonZero].isZero()) {
+        lastNonZero--;
+      }
+      const bytecodeEmittedFields = bytecodeFields.slice(0, lastNonZero + 1);
+
+      const headerFields = [
+        new Fr(CONTRACT_CLASS_PUBLISHED_MAGIC_VALUE),
+        contractClassId,
+        new Fr(1), // version
+        artifactHash,
+        privateFunctionsRoot,
+      ];
+
+      const emittedFields = [...headerFields, ...bytecodeEmittedFields];
+      const emittedLength = emittedFields.length;
+
+      const allFields = [
+        ...emittedFields,
+        ...Array(CONTRACT_CLASS_LOG_SIZE_IN_FIELDS - emittedFields.length).fill(Fr.ZERO),
+      ];
+
+      const fields = new ContractClassLogFields(allFields);
+      const log = new ContractClassLog(ProtocolContractAddress.ContractClassRegistry, fields, emittedLength);
+      return { log, emittedLength };
+    }
+
+    async function injectContractClassLog(tx: Tx, log: ContractClassLog, emittedLength: number) {
+      tx.contractClassLogFields.push(log.fields);
+      const logHashes = tx.data.forPublic!.nonRevertibleAccumulatedData.contractClassLogsHashes;
+      const emptyIdx = logHashes.findIndex(h => h.isEmpty());
+      if (emptyIdx >= 0) {
+        logHashes[emptyIdx] = LogHash.from({
+          value: await log.fields.hash(),
+          length: emittedLength,
+        }).scope(log.contractAddress);
+      }
+    }
+
+    it('allows transactions with correct contract class ids', async () => {
+      const tx = await mockTx(2, {
+        numberOfNonRevertiblePublicCallRequests: 1,
+        numberOfRevertiblePublicCallRequests: 0,
+      });
+      const { log, emittedLength } = await buildContractClassLog();
+      await injectContractClassLog(tx, log, emittedLength);
+      await tx.recomputeHash();
+      await expect(validator.validateTx(tx)).resolves.toEqual({ result: 'valid' });
+    });
+
+    it('rejects transactions with incorrect contract class ids', async () => {
+      const tx = await mockTx(3, {
+        numberOfNonRevertiblePublicCallRequests: 1,
+        numberOfRevertiblePublicCallRequests: 0,
+      });
+      const { log, emittedLength } = await buildContractClassLog({ contractClassId: Fr.random() });
+      await injectContractClassLog(tx, log, emittedLength);
+      await tx.recomputeHash();
+      await expect(validator.validateTx(tx)).resolves.toEqual({
+        result: 'invalid',
+        reason: [TX_ERROR_INCORRECT_CONTRACT_CLASS_ID],
+      });
+    });
+
+    it('rejects transactions with malformed contract class logs', async () => {
+      const tx = await mockTx(4, {
+        numberOfNonRevertiblePublicCallRequests: 1,
+        numberOfRevertiblePublicCallRequests: 0,
+      });
+      const headerFields = [
+        new Fr(CONTRACT_CLASS_PUBLISHED_MAGIC_VALUE),
+        Fr.random(),
+        new Fr(1),
+        Fr.random(),
+        Fr.random(),
+        new Fr(999999), // bogus bytecode length
+      ];
+      const allFields = [
+        ...headerFields,
+        ...Array(CONTRACT_CLASS_LOG_SIZE_IN_FIELDS - headerFields.length).fill(Fr.ZERO),
+      ];
+      const fields = new ContractClassLogFields(allFields);
+      const log = new ContractClassLog(ProtocolContractAddress.ContractClassRegistry, fields, headerFields.length);
+      await injectContractClassLog(tx, log, headerFields.length);
+      await tx.recomputeHash();
+      const result = await validator.validateTx(tx);
+      expect(result.result).toBe('invalid');
+      expect(result.result === 'invalid' && result.reason[0]).toMatch(
+        new RegExp(`${TX_ERROR_INCORRECT_CONTRACT_CLASS_ID}|${TX_ERROR_MALFORMED_CONTRACT_CLASS_LOG}`),
+      );
+    });
+  });
 });