From 472474ea1821ec697b63e561777c4acbb8ecb773 Mon Sep 17 00:00:00 2001 From: Leila Wang Date: Fri, 27 Oct 2023 18:50:08 +0000 Subject: [PATCH] Better error message for compute_note_hash_and_nullifier. --- .../acir-simulator/src/client/db_oracle.ts | 13 ++ .../src/client/private_execution.test.ts | 34 +----- .../src/client/simulator.test.ts | 113 ++++++++++++++++++ .../acir-simulator/src/client/simulator.ts | 30 ++--- yarn-project/acir-simulator/src/test/utils.ts | 12 +- .../pxe/src/contract_data_oracle/index.ts | 13 ++ .../pxe/src/simulator_oracle/index.ts | 16 +++ 7 files changed, 177 insertions(+), 54 deletions(-) create mode 100644 yarn-project/acir-simulator/src/client/simulator.test.ts diff --git a/yarn-project/acir-simulator/src/client/db_oracle.ts b/yarn-project/acir-simulator/src/client/db_oracle.ts index da9233c5453..18559da2716 100644 --- a/yarn-project/acir-simulator/src/client/db_oracle.ts +++ b/yarn-project/acir-simulator/src/client/db_oracle.ts @@ -71,6 +71,19 @@ export interface DBOracle extends CommitmentsDB { selector: FunctionSelector, ): Promise; + /** + * Retrieves the artifact of a specified function within a given contract. + * The function is identified by its name, which is unique within a contract. + * + * @param contractAddress - The AztecAddress representing the contract containing the function. + * @param functionName - The name of the function. + * @returns The corresponding function's artifact as an object. + */ + getFunctionArtifactByName( + contractAddress: AztecAddress, + functionName: string, + ): Promise; + /** * Retrieves the portal contract address associated with the given contract address. * Throws an error if the input contract address is not found or invalid. diff --git a/yarn-project/acir-simulator/src/client/private_execution.test.ts b/yarn-project/acir-simulator/src/client/private_execution.test.ts index df61888a4e6..4b089fa49b9 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.test.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.test.ts @@ -18,7 +18,6 @@ import { computeCallStackItemHash, computeCommitmentNonce, computeSecretMessageHash, - computeUniqueCommitment, computeVarArgsHash, siloCommitment, } from '@aztec/circuits.js/abis'; @@ -226,35 +225,9 @@ describe('Private Execution test suite', () => { throw new Error(`Unknown address ${address}`); }); - oracle.getFunctionArtifact.mockImplementation((_, selector) => - Promise.resolve(getFunctionArtifactWithSelector(StatefulTestContractArtifact, selector)), - ); - }); - - it('should have an artifact for computing note hash and nullifier', async () => { - const storageSlot = Fr.random(); - const note = buildNote(60n, owner, storageSlot); - - // Should be the same as how we compute the values for the ValueNote in the Aztec.nr library. - const valueNoteHash = hashFields(note.preimage); - const innerNoteHash = hashFields([storageSlot, valueNoteHash]); - const siloedNoteHash = siloCommitment(circuitsWasm, contractAddress, innerNoteHash); - const uniqueSiloedNoteHash = computeUniqueCommitment(circuitsWasm, note.nonce, siloedNoteHash); - const innerNullifier = hashFields([uniqueSiloedNoteHash, ownerPk.low, ownerPk.high]); - - const result = await acirSimulator.computeNoteHashAndNullifier( - contractAddress, - note.nonce, - storageSlot, - note.preimage, + oracle.getFunctionArtifactByName.mockImplementation((_, functionName: string) => + Promise.resolve(getFunctionArtifact(StatefulTestContractArtifact, functionName)), ); - - expect(result).toEqual({ - innerNoteHash, - siloedNoteHash, - uniqueSiloedNoteHash, - innerNullifier, - }); }); it('should have a constructor with arguments that inserts notes', async () => { @@ -608,6 +581,9 @@ describe('Private Execution test suite', () => { oracle.getFunctionArtifact.mockImplementation((_, selector) => Promise.resolve(getFunctionArtifactWithSelector(PendingCommitmentsContractArtifact, selector)), ); + oracle.getFunctionArtifactByName.mockImplementation((_, functionName: string) => + Promise.resolve(getFunctionArtifact(PendingCommitmentsContractArtifact, functionName)), + ); }); it('should be able to insert, read, and nullify pending commitments in one call', async () => { diff --git a/yarn-project/acir-simulator/src/client/simulator.test.ts b/yarn-project/acir-simulator/src/client/simulator.test.ts new file mode 100644 index 00000000000..e21c118b719 --- /dev/null +++ b/yarn-project/acir-simulator/src/client/simulator.test.ts @@ -0,0 +1,113 @@ +import { CircuitsWasm, CompleteAddress } from '@aztec/circuits.js'; +import { computeUniqueCommitment, siloCommitment } from '@aztec/circuits.js/abis'; +import { pedersenHashInputs } from '@aztec/circuits.js/barretenberg'; +import { ABIParameterVisibility } from '@aztec/foundation/abi'; +import { AztecAddress } from '@aztec/foundation/aztec-address'; +import { Fr, GrumpkinScalar } from '@aztec/foundation/fields'; +import { TokenContractArtifact } from '@aztec/noir-contracts/artifacts'; + +import { MockProxy, mock } from 'jest-mock-extended'; + +import { getFunctionArtifact } from '../test/utils.js'; +import { DBOracle, FunctionArtifactWithDebugMetadata } from './db_oracle.js'; +import { AcirSimulator } from './simulator.js'; + +describe('Simulator', () => { + let oracle: MockProxy; + let simulator: AcirSimulator; + let circuitsWasm: CircuitsWasm; + let ownerCompleteAddress: CompleteAddress; + let owner: AztecAddress; + const ownerPk = GrumpkinScalar.fromString('2dcc5485a58316776299be08c78fa3788a1a7961ae30dc747fb1be17692a8d32'); + + const hashFields = (data: Fr[]) => + Fr.fromBuffer( + pedersenHashInputs( + circuitsWasm, + data.map(f => f.toBuffer()), + ), + ); + + beforeAll(async () => { + circuitsWasm = await CircuitsWasm.get(); + + ownerCompleteAddress = await CompleteAddress.fromPrivateKeyAndPartialAddress(ownerPk, Fr.random()); + owner = ownerCompleteAddress.address; + }); + + beforeEach(() => { + oracle = mock(); + oracle.getSecretKey.mockResolvedValue(ownerPk); + oracle.getCompleteAddress.mockResolvedValue(ownerCompleteAddress); + + simulator = new AcirSimulator(oracle); + }); + + describe('computeNoteHashAndNullifier', () => { + const artifact = getFunctionArtifact(TokenContractArtifact, 'compute_note_hash_and_nullifier'); + const contractAddress = AztecAddress.random(); + const nonce = Fr.random(); + const storageSlot = Fr.random(); + + const createPreimage = (amount = 123n) => [new Fr(amount), owner.toField(), Fr.random()]; + + it('should compute note hashes and nullifier', async () => { + oracle.getFunctionArtifactByName.mockResolvedValue(artifact); + + const preimage = createPreimage(); + const valueNoteHash = hashFields(preimage); + const innerNoteHash = hashFields([storageSlot, valueNoteHash]); + const siloedNoteHash = siloCommitment(circuitsWasm, contractAddress, innerNoteHash); + const uniqueSiloedNoteHash = computeUniqueCommitment(circuitsWasm, nonce, siloedNoteHash); + const innerNullifier = hashFields([uniqueSiloedNoteHash, ownerPk.low, ownerPk.high]); + + const result = await simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, preimage); + + expect(result).toEqual({ + innerNoteHash, + siloedNoteHash, + uniqueSiloedNoteHash, + innerNullifier, + }); + }); + + it('throw if the contract does not implement "compute_note_hash_and_nullifier"', async () => { + oracle.getFunctionArtifactByName.mockResolvedValue(undefined); + + const preimage = createPreimage(); + await expect( + simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, preimage), + ).rejects.toThrowError(/Mandatory implementation of "compute_note_hash_and_nullifier" missing/); + }); + + it('throw if a note has more fields than "compute_note_hash_and_nullifier" can process', async () => { + const preimage = createPreimage(); + const wrongPreimageLength = preimage.length - 1; + + const modifiedArtifact: FunctionArtifactWithDebugMetadata = { + ...artifact, + parameters: [ + ...artifact.parameters.slice(0, -1), + { + name: 'preimage', + type: { + kind: 'array', + length: wrongPreimageLength, + type: { + kind: 'field', + }, + }, + visibility: ABIParameterVisibility.SECRET, + }, + ], + }; + oracle.getFunctionArtifactByName.mockResolvedValue(modifiedArtifact); + + await expect( + simulator.computeNoteHashAndNullifier(contractAddress, nonce, storageSlot, preimage), + ).rejects.toThrowError( + new RegExp(`"compute_note_hash_and_nullifier" can only handle a maximum of ${wrongPreimageLength} fields`), + ); + }); + }); +}); diff --git a/yarn-project/acir-simulator/src/client/simulator.ts b/yarn-project/acir-simulator/src/client/simulator.ts index 46521e35c60..9367a5fe6d9 100644 --- a/yarn-project/acir-simulator/src/client/simulator.ts +++ b/yarn-project/acir-simulator/src/client/simulator.ts @@ -1,4 +1,4 @@ -import { CallContext, FunctionData, MAX_NOTE_FIELDS_LENGTH } from '@aztec/circuits.js'; +import { CallContext, FunctionData } from '@aztec/circuits.js'; import { Grumpkin } from '@aztec/circuits.js/barretenberg'; import { ArrayType, FunctionSelector, FunctionType, encodeArguments } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; @@ -161,28 +161,24 @@ export class AcirSimulator { storageSlot: Fr, notePreimage: Fr[], ) { - let artifact: FunctionArtifactWithDebugMetadata | undefined = undefined; - - // Brute force - for (let i = notePreimage.length; i < MAX_NOTE_FIELDS_LENGTH; i++) { - const signature = `compute_note_hash_and_nullifier(Field,Field,Field,[Field;${i}])`; - const selector = FunctionSelector.fromSignature(signature); - try { - artifact = await this.db.getFunctionArtifact(contractAddress, selector); - if (artifact !== undefined) break; - } catch (e) { - // ignore - } + const artifact: FunctionArtifactWithDebugMetadata | undefined = await this.db.getFunctionArtifactByName( + contractAddress, + 'compute_note_hash_and_nullifier', + ); + if (!artifact) { + throw new Error( + `Mandatory implementation of "compute_note_hash_and_nullifier" missing in noir contract ${contractAddress.toString()}.`, + ); } - if (artifact == undefined) { + const maxNoteFields = (artifact.parameters[artifact.parameters.length - 1].type as ArrayType).length; + if (maxNoteFields < notePreimage.length) { throw new Error( - `Mandatory implementation of "compute_note_hash_and_nullifier" missing in noir contract ${contractAddress.toString()}.`, + `The note being processed has ${notePreimage.length} fields, while "compute_note_hash_and_nullifier" can only handle a maximum of ${maxNoteFields} fields. Please consider increasing the allowed field size to accommodate all notes generated from the contract.`, ); } - const preimageLen = (artifact.parameters[3].type as ArrayType).length; - const extendedPreimage = notePreimage.concat(Array(preimageLen - notePreimage.length).fill(Fr.ZERO)); + const extendedPreimage = notePreimage.concat(Array(maxNoteFields - notePreimage.length).fill(Fr.ZERO)); const execRequest: FunctionCall = { to: AztecAddress.ZERO, diff --git a/yarn-project/acir-simulator/src/test/utils.ts b/yarn-project/acir-simulator/src/test/utils.ts index b9b9a388c77..acbb2c668d0 100644 --- a/yarn-project/acir-simulator/src/test/utils.ts +++ b/yarn-project/acir-simulator/src/test/utils.ts @@ -46,14 +46,12 @@ export const getFunctionArtifact = ( artifact: ContractArtifact, functionName: string, ): FunctionArtifactWithDebugMetadata => { - const functionIndex = artifact.functions.findIndex(f => f.name === functionName); - if (functionIndex < 0) { + const functionArtifact = artifact.functions.find(f => f.name === functionName); + if (!functionArtifact) { throw new Error(`Unknown function ${functionName}`); } - const functionArtifact = artifact.functions[functionIndex]; const debug = getFunctionDebugMetadata(artifact, functionName); - return { ...functionArtifact, debug }; }; @@ -61,15 +59,13 @@ export const getFunctionArtifactWithSelector = ( artifact: ContractArtifact, functionSelector: FunctionSelector, ): FunctionArtifactWithDebugMetadata => { - const functionIndex = artifact.functions.findIndex(f => + const functionArtifact = artifact.functions.find(f => functionSelector.equals(FunctionSelector.fromNameAndParameters(f.name, f.parameters)), ); - if (functionIndex < 0) { + if (!functionArtifact) { throw new Error(`Unknown function ${functionSelector}`); } - const functionArtifact = artifact.functions[functionIndex]; const debug = getFunctionDebugMetadata(artifact, functionArtifact.name); - return { ...functionArtifact, debug }; }; diff --git a/yarn-project/pxe/src/contract_data_oracle/index.ts b/yarn-project/pxe/src/contract_data_oracle/index.ts index 79be6f6aa44..9713c5cf2ba 100644 --- a/yarn-project/pxe/src/contract_data_oracle/index.ts +++ b/yarn-project/pxe/src/contract_data_oracle/index.ts @@ -44,6 +44,19 @@ export class ContractDataOracle { return tree.getFunctionArtifact(selector); } + /** + * Retrieves the artifact of a specified function within a given contract. + * The function is identified by its name, which is unique within a contract. + * + * @param contractAddress - The AztecAddress representing the contract containing the function. + * @param functionName - The name of the function. + * @returns The corresponding function's artifact as an object. + */ + public async getFunctionArtifactByName(contractAddress: AztecAddress, functionName: string) { + const contract = await this.db.getContract(contractAddress); + return contract?.functions.find(f => f.name === functionName); + } + /** * Retrieves the debug metadata of a specified function within a given contract. * The function is identified by its selector, which is a unique code generated from the function's signature. diff --git a/yarn-project/pxe/src/simulator_oracle/index.ts b/yarn-project/pxe/src/simulator_oracle/index.ts index 720df507636..ed919f0652f 100644 --- a/yarn-project/pxe/src/simulator_oracle/index.ts +++ b/yarn-project/pxe/src/simulator_oracle/index.ts @@ -72,6 +72,22 @@ export class SimulatorOracle implements DBOracle { }; } + async getFunctionArtifactByName( + contractAddress: AztecAddress, + functionName: string, + ): Promise { + const artifact = await this.contractDataOracle.getFunctionArtifactByName(contractAddress, functionName); + if (!artifact) { + return; + } + + const debug = await this.contractDataOracle.getFunctionDebugMetadata(contractAddress, artifact.selector); + return { + ...artifact, + debug, + }; + } + async getPortalContractAddress(contractAddress: AztecAddress): Promise { return await this.contractDataOracle.getPortalContractAddress(contractAddress); }