From fc4524d0a9c8bad0033489a757b599020503fa2f Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Mon, 18 Mar 2024 14:27:48 -0300 Subject: [PATCH] feat: Verify registered artifact matches instance class id --- yarn-project/aztec.js/src/index.ts | 1 - .../aztec.js/src/wallet/base_wallet.ts | 6 ++- .../src/interfaces/contract-with-artifact.ts | 20 --------- .../circuit-types/src/interfaces/index.ts | 1 - .../circuit-types/src/interfaces/pxe.ts | 5 +-- yarn-project/circuit-types/src/mocks.ts | 12 +++--- .../pxe/src/pxe_service/pxe_service.ts | 42 +++++++++++-------- .../src/pxe_service/test/pxe_test_suite.ts | 15 ++++--- 8 files changed, 46 insertions(+), 56 deletions(-) delete mode 100644 yarn-project/circuit-types/src/interfaces/contract-with-artifact.ts diff --git a/yarn-project/aztec.js/src/index.ts b/yarn-project/aztec.js/src/index.ts index 3a186701891..8b7785c0584 100644 --- a/yarn-project/aztec.js/src/index.ts +++ b/yarn-project/aztec.js/src/index.ts @@ -82,7 +82,6 @@ export { AztecNode, Body, CompleteAddress, - ContractWithArtifact, ExtendedNote, FunctionCall, GrumpkinPrivateKey, diff --git a/yarn-project/aztec.js/src/wallet/base_wallet.ts b/yarn-project/aztec.js/src/wallet/base_wallet.ts index db025e27f20..1df06b661e9 100644 --- a/yarn-project/aztec.js/src/wallet/base_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/base_wallet.ts @@ -1,6 +1,5 @@ import { AuthWitness, - ContractWithArtifact, ExtendedNote, FunctionCall, GetUnencryptedLogsResponse, @@ -76,7 +75,10 @@ export abstract class BaseWallet implements Wallet { getRecipient(address: AztecAddress): Promise { return this.pxe.getRecipient(address); } - registerContract(contract: ContractWithArtifact): Promise { + registerContract(contract: { + /** Instance */ instance: ContractInstanceWithAddress; + /** Associated artifact */ artifact?: ContractArtifact; + }): Promise { return this.pxe.registerContract(contract); } registerContractClass(artifact: ContractArtifact): Promise { diff --git a/yarn-project/circuit-types/src/interfaces/contract-with-artifact.ts b/yarn-project/circuit-types/src/interfaces/contract-with-artifact.ts deleted file mode 100644 index b42bd1b24d5..00000000000 --- a/yarn-project/circuit-types/src/interfaces/contract-with-artifact.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { ContractArtifact } from '@aztec/foundation/abi'; -import { Fr } from '@aztec/foundation/fields'; -import { ContractInstanceWithAddress } from '@aztec/types/contracts'; - -/** - * Container for contract instance and artifact or class id. - */ -export type ContractWithArtifact = ( - | { - /** The artifact of the contract. */ - artifact: ContractArtifact; - } - | { - /** The class id of the contract artifact. */ - contractClassId: Fr; - } -) & { - /** The contract instance. */ - instance: ContractInstanceWithAddress; -}; diff --git a/yarn-project/circuit-types/src/interfaces/index.ts b/yarn-project/circuit-types/src/interfaces/index.ts index 0e3803aae78..c658367b918 100644 --- a/yarn-project/circuit-types/src/interfaces/index.ts +++ b/yarn-project/circuit-types/src/interfaces/index.ts @@ -1,6 +1,5 @@ export * from './aztec-node.js'; export * from './pxe.js'; -export * from './contract-with-artifact.js'; export * from './sync-status.js'; export * from './configs.js'; export * from './nullifier_tree.js'; diff --git a/yarn-project/circuit-types/src/interfaces/pxe.ts b/yarn-project/circuit-types/src/interfaces/pxe.ts index 1ca14a130d7..94cc7e3a712 100644 --- a/yarn-project/circuit-types/src/interfaces/pxe.ts +++ b/yarn-project/circuit-types/src/interfaces/pxe.ts @@ -11,7 +11,6 @@ import { NoteFilter } from '../notes/note_filter.js'; import { Tx, TxHash, TxReceipt } from '../tx/index.js'; import { TxEffect } from '../tx_effect.js'; import { TxExecutionRequest } from '../tx_execution_request.js'; -import { ContractWithArtifact } from './contract-with-artifact.js'; import { SyncStatus } from './sync-status.js'; // docs:start:pxe-interface @@ -112,9 +111,9 @@ export interface PXE { * deploying a contract. Dapps that wish to interact with contracts already deployed should register * these contracts in their users' PXE Service through this method. * - * @param contract - An object containing contract artifact and instance. + * @param contract - A contract instance to register, with an optional artifact which can be omitted if the contract class has already been registered. */ - registerContract(contract: ContractWithArtifact): Promise; + registerContract(contract: { instance: ContractInstanceWithAddress; artifact?: ContractArtifact }): Promise; /** * Retrieves the addresses of contracts added to this PXE Service. diff --git a/yarn-project/circuit-types/src/mocks.ts b/yarn-project/circuit-types/src/mocks.ts index 71bcae25a4a..30ad57764ac 100644 --- a/yarn-project/circuit-types/src/mocks.ts +++ b/yarn-project/circuit-types/src/mocks.ts @@ -4,6 +4,8 @@ import { MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX, MAX_REVERTIBLE_PUBLIC_CALL_STACK_LENGTH_PER_TX, Proof, + computeContractClassId, + getContractClassFromArtifact, } from '@aztec/circuits.js'; import { ContractArtifact } from '@aztec/foundation/abi'; import { makeTuple } from '@aztec/foundation/array'; @@ -13,7 +15,6 @@ import { Fr } from '@aztec/foundation/fields'; import { Tuple } from '@aztec/foundation/serialize'; import { ContractInstanceWithAddress, SerializableContractInstance } from '@aztec/types/contracts'; -import { ContractWithArtifact } from './interfaces/index.js'; import { FunctionL2Logs, Note, TxL2Logs } from './logs/index.js'; import { makePrivateKernelTailCircuitPublicInputs, makePublicCallRequest } from './mocks_to_purge.js'; import { ExtendedNote } from './notes/index.js'; @@ -62,10 +63,11 @@ export const randomContractArtifact = (): ContractArtifact => ({ export const randomContractInstanceWithAddress = (opts: { contractClassId?: Fr } = {}): ContractInstanceWithAddress => SerializableContractInstance.random(opts).withAddress(AztecAddress.random()); -export const randomDeployedContract = (): ContractWithArtifact => ({ - artifact: randomContractArtifact(), - instance: randomContractInstanceWithAddress(), -}); +export const randomDeployedContract = () => { + const artifact = randomContractArtifact(); + const contractClassId = computeContractClassId(getContractClassFromArtifact(artifact)); + return { artifact, instance: randomContractInstanceWithAddress({ contractClassId }) }; +}; export const randomExtendedNote = ({ note = Note.random(), diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 769f7721272..1f25b401f07 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -1,7 +1,6 @@ import { AuthWitness, AztecNode, - ContractWithArtifact, ExtendedNote, FunctionCall, GetUnencryptedLogsResponse, @@ -32,7 +31,6 @@ import { PartialAddress, PrivateKernelTailCircuitPublicInputs, PublicCallRequest, - computeArtifactHash, computeContractClassId, getContractClassFromArtifact, } from '@aztec/circuits.js'; @@ -43,7 +41,6 @@ import { Fr } from '@aztec/foundation/fields'; import { SerialQueue } from '@aztec/foundation/fifo'; import { DebugLogger, createDebugLogger } from '@aztec/foundation/log'; import { Timer } from '@aztec/foundation/timer'; -import { required } from '@aztec/foundation/validation'; import { AcirSimulator, ExecutionResult, @@ -222,22 +219,31 @@ export class PXEService implements PXE { this.log.info(`Added contract class ${artifact.name} with id ${contractClassId}`); } - public async registerContract(contract: ContractWithArtifact) { - const instance = contract.instance; - const artifact = - 'artifact' in contract - ? contract.artifact - : required( - await this.db.getContractArtifact(contract.contractClassId), - `Unknown artifact for class id ${contract.contractClassId} when registering ${instance.address}`, - ); - const artifactHash = computeArtifactHash(artifact); - const contractClassId = computeContractClassId(getContractClassFromArtifact({ ...artifact, artifactHash })); - await this.db.addContractArtifact(contractClassId, artifact); + public async registerContract(contract: { instance: ContractInstanceWithAddress; artifact?: ContractArtifact }) { + const { instance } = contract; + let { artifact } = contract; + + if (artifact) { + // If the user provides an artifact, validate it against the expected class id and register it + const contractClassId = computeContractClassId(getContractClassFromArtifact(artifact)); + if (!contractClassId.equals(instance.contractClassId)) { + throw new Error( + `Artifact does not match expected class id (computed ${contractClassId} but instance refers to ${instance.contractClassId})`, + ); + } + await this.db.addContractArtifact(contractClassId, artifact); + } else { + // Otherwise, make sure there is an artifact already registered for that class id + artifact = await this.db.getContractArtifact(instance.contractClassId); + if (!artifact) { + throw new Error( + `Missing contract artifact for class id ${instance.contractClassId} for contract ${instance.address}`, + ); + } + } + + this.log.info(`Added contract ${artifact.name} at ${instance.address.toString()}`); await this.db.addContractInstance(instance); - const hasPortal = instance.portalContractAddress && !instance.portalContractAddress.isZero(); - const portalInfo = hasPortal ? ` with portal ${instance.portalContractAddress.toChecksumString()}` : ''; - this.log.info(`Added contract ${artifact.name} at ${instance.address.toString()}${portalInfo}`); await this.synchronizer.reprocessDeferredNotesForContract(instance.address); } diff --git a/yarn-project/pxe/src/pxe_service/test/pxe_test_suite.ts b/yarn-project/pxe/src/pxe_service/test/pxe_test_suite.ts index 6772bced9a1..7c24b72307b 100644 --- a/yarn-project/pxe/src/pxe_service/test/pxe_test_suite.ts +++ b/yarn-project/pxe/src/pxe_service/test/pxe_test_suite.ts @@ -1,5 +1,4 @@ import { - ContractWithArtifact, PXE, TxExecutionRequest, randomContractArtifact, @@ -90,7 +89,7 @@ export const pxeTestSuite = (testName: string, pxeSetup: () => Promise) => }); it('successfully adds a contract', async () => { - const contracts: ContractWithArtifact[] = [randomDeployedContract(), randomDeployedContract()]; + const contracts = [randomDeployedContract(), randomDeployedContract()]; for (const contract of contracts) { await pxe.registerContract(contract); } @@ -109,15 +108,19 @@ export const pxeTestSuite = (testName: string, pxeSetup: () => Promise) => await pxe.registerContractClass(artifact); expect(await pxe.getContractClass(contractClassId)).toEqual(contractClass); - await pxe.registerContract({ contractClassId, instance }); + await pxe.registerContract({ instance }); expect(await pxe.getContractInstance(instance.address)).toEqual(instance); }); it('refuses to register a contract with a class that has not been registered', async () => { const instance = randomContractInstanceWithAddress(); - await expect(pxe.registerContract({ contractClassId: Fr.random(), instance })).rejects.toThrow( - /Unknown artifact/i, - ); + await expect(pxe.registerContract({ instance })).rejects.toThrow(/Missing contract artifact/i); + }); + + it('refuses to register a contract with an artifact with mismatching class id', async () => { + const artifact = randomContractArtifact(); + const instance = randomContractInstanceWithAddress(); + await expect(pxe.registerContract({ instance, artifact })).rejects.toThrow(/Artifact does not match/i); }); it('throws when simulating a tx targeting public entrypoint', async () => {