From f47a7947767c873b1b736c378f75fe14533819a9 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Sep 2023 08:03:05 +0000 Subject: [PATCH 1/5] feat: add support for assert msgs & brillig stacks --- yarn-project/acir-simulator/package.json | 2 +- yarn-project/acir-simulator/src/acvm/acvm.ts | 100 +++---- .../src/client/private_execution.ts | 260 +++++++++--------- .../src/client/unconstrained_execution.ts | 133 ++++----- .../acir-simulator/src/public/executor.ts | 8 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 17 +- .../src/e2e_nested_contract.test.ts | 4 +- .../src/contracts/child_contract/src/main.nr | 13 +- .../src/easy_private_state.nr | 2 +- .../src/state_vars/immutable_singleton.nr | 2 +- .../noir-aztec/src/state_vars/map.nr | 2 +- .../noir-aztec/src/state_vars/public_state.nr | 2 +- .../noir-aztec/src/state_vars/set.nr | 2 +- .../noir-aztec/src/state_vars/singleton.nr | 2 +- yarn-project/types/src/simulation_error.ts | 41 ++- yarn-project/yarn.lock | 10 +- 16 files changed, 306 insertions(+), 294 deletions(-) diff --git a/yarn-project/acir-simulator/package.json b/yarn-project/acir-simulator/package.json index 0cf158d4d26..1dd64c67eed 100644 --- a/yarn-project/acir-simulator/package.json +++ b/yarn-project/acir-simulator/package.json @@ -35,7 +35,7 @@ "@aztec/circuits.js": "workspace:^", "@aztec/foundation": "workspace:^", "@aztec/types": "workspace:^", - "acvm_js": "github:noir-lang/acvm-js-wasm#arv/0.24.1", + "acvm_js": "github:noir-lang/acvm-js-wasm#arv/0.25.0", "levelup": "^5.1.1", "memdown": "^6.1.1", "tslib": "^2.4.0" diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index adcbdb736a6..bf07687045a 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -1,11 +1,12 @@ -import { FunctionDebugMetadata } from '@aztec/foundation/abi'; +import { FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; -import { NoirCallStack } from '@aztec/types'; +import { NoirCallStack, SourceCodeLocation } from '@aztec/types'; import { + ExecutionError, ForeignCallInput, ForeignCallOutput, WasmBlackBoxFunctionSolver, @@ -70,18 +71,14 @@ export interface ACIRExecutionResult { partialWitness: ACVMWitness; } -/** - * Extracts the opcode location from an ACVM error string. - */ -function extractOpcodeLocationFromError(err: string): string | undefined { - const match = err.match(/^Cannot satisfy constraint (?[0-9]+(?:\.[0-9]+)?)/); - return match?.groups?.opcodeLocation; -} - /** * Extracts the call stack from the location of a failing opcode and the debug metadata. + * One opcode can point to multiple calls due to inlining. */ -function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionDebugMetadata): NoirCallStack { +function getSourceCodeLocationsFromOpcodeLocation( + opcodeLocation: string, + debug: FunctionDebugMetadata, +): SourceCodeLocation[] { const { debugSymbols, files } = debug; const callStack = debugSymbols.locations[opcodeLocation] || []; @@ -104,33 +101,16 @@ function getCallStackFromOpcodeLocation(opcodeLocation: string, debug: FunctionD } /** - * Extracts source code locations from an ACVM error if possible. - * @param errMessage - The ACVM error. + * Extracts the source code locations for an array of opcode locations + * @param opcodeLocations - The opcode locations that caused the error. * @param debug - The debug metadata of the function. - * @returns The source code locations or undefined if they couldn't be extracted from the error. + * @returns The source code locations. */ -export function processAcvmError(errMessage: string, debug: FunctionDebugMetadata): NoirCallStack | undefined { - const opcodeLocation = extractOpcodeLocationFromError(errMessage); - if (!opcodeLocation) { - return undefined; - } - - return getCallStackFromOpcodeLocation(opcodeLocation, debug); -} - -/** - * An error thrown by the ACVM during simulation. Optionally contains a noir call stack. - */ -export class ACVMError extends Error { - constructor( - message: string, - /** - * The noir call stack of the error, if it could be extracted. - */ - public callStack?: NoirCallStack, - ) { - super(message); - } +export function resolveOpcodeLocations( + opcodeLocations: OpcodeLocation[], + debug: FunctionDebugMetadata, +): SourceCodeLocation[] { + return opcodeLocations.flatMap(opcodeLocation => getSourceCodeLocationsFromOpcodeLocation(opcodeLocation, debug)); } /** @@ -141,13 +121,8 @@ export async function acvm( acir: Buffer, initialWitness: ACVMWitness, callback: ACIRCallback, - debug?: FunctionDebugMetadata, ): Promise { const logger = createDebugLogger('aztec:simulator:acvm'); - // This is a workaround to avoid the ACVM removing the information about the underlying error. - // We should probably update the ACVM to let proper errors through. - let oracleError: Error | undefined = undefined; - const partialWitness = await executeCircuitWithBlackBoxSolver( solver, acir, @@ -169,33 +144,36 @@ export async function acvm( } else { typedError = new Error(`Error in oracle callback ${err}`); } - oracleError = typedError; - logger.error(`Error in oracle callback ${name}:`, typedError.message, typedError.stack); + logger.error(`Error in oracle callback ${name}`); throw typedError; } }, - ).catch((acvmErrorString: string) => { - if (oracleError) { - throw oracleError; - } - - if (debug) { - const callStack = processAcvmError(acvmErrorString, debug); - - if (callStack) { - throw new ACVMError( - `Assertion failed: '${callStack[callStack.length - 1]?.locationText ?? 'Unknown'}'`, - callStack, - ); - } - } - // If we cannot find a callstack, throw the original error. - throw new ACVMError(acvmErrorString); - }); + ); return Promise.resolve({ partialWitness }); } +/** + * Extracts the call stack from an thrown by the acvm. + * @param error - The error to extract from. + * @param debug - The debug metadata of the function called. + * @returns The call stack, if available. + */ +export function extractCallStack( + error: Error | ExecutionError, + debug?: FunctionDebugMetadata, +): NoirCallStack | undefined { + if (!('callStack' in error) || !error.callStack) { + return undefined; + } + const { callStack } = error; + if (!debug) { + return callStack; + } + + return resolveOpcodeLocations(callStack, debug); +} + /** * Adapts the buffer to the field size. * @param originalBuf - The buffer to adapt. diff --git a/yarn-project/acir-simulator/src/client/private_execution.ts b/yarn-project/acir-simulator/src/client/private_execution.ts index ef002781d71..82d309c61cd 100644 --- a/yarn-project/acir-simulator/src/client/private_execution.ts +++ b/yarn-project/acir-simulator/src/client/private_execution.ts @@ -13,11 +13,14 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { to2Fields } from '@aztec/foundation/serialize'; import { FunctionL2Logs, NotePreimage, NoteSpendingInfo, SimulationError } from '@aztec/types'; +import { ExecutionError } from 'acvm_js'; + import { extractPrivateCircuitPublicInputs, frToAztecAddress } from '../acvm/deserialize.js'; import { ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, + extractCallStack, fromACVMField, toACVMField, toACVMWitness, @@ -69,136 +72,135 @@ export class PrivateFunctionExecution { const encryptedLogs = new FunctionL2Logs([]); const unencryptedLogs = new FunctionL2Logs([]); - const { partialWitness } = await acvm( - await AcirSimulator.getSolver(), - acir, - initialWitness, - { - packArguments: async args => { - return toACVMField(await this.context.packedArgsCache.pack(args.map(fromACVMField))); - }, - getSecretKey: ([ownerX], [ownerY]) => this.context.getSecretKey(this.contractAddress, ownerX, ownerY), - getPublicKey: async ([acvmAddress]) => { - const address = frToAztecAddress(fromACVMField(acvmAddress)); - const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address); - return [publicKey.x, publicKey.y, partialAddress].map(toACVMField); - }, - getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) => - this.context.getNotes( - this.contractAddress, - slot, - +numSelects, - selectBy, - selectValues, - sortBy, - sortOrder, - +limit, - +offset, - +returnSize, - ), - getRandomField: () => Promise.resolve(toACVMField(Fr.random())), - notifyCreatedNote: ([storageSlot], preimage, [innerNoteHash]) => { - this.context.pushNewNote( - this.contractAddress, - fromACVMField(storageSlot), - preimage.map(f => fromACVMField(f)), - fromACVMField(innerNoteHash), - ); - - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1040): remove newNotePreimages - // as it is redundant with pendingNoteData. Consider renaming pendingNoteData->pendingNotePreimages. - newNotePreimages.push({ - storageSlot: fromACVMField(storageSlot), - preimage: preimage.map(f => fromACVMField(f)), - }); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - notifyNullifiedNote: async ([slot], [nullifier], acvmPreimage, [innerNoteHash]) => { - newNullifiers.push({ - preimage: acvmPreimage.map(f => fromACVMField(f)), - storageSlot: fromACVMField(slot), - nullifier: fromACVMField(nullifier), - }); - await this.context.pushNewNullifier(fromACVMField(nullifier), this.contractAddress); - this.context.nullifyPendingNotes(fromACVMField(innerNoteHash), this.contractAddress, fromACVMField(slot)); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - callPrivateFunction: async ([acvmContractAddress], [acvmFunctionSelector], [acvmArgsHash]) => { - const contractAddress = fromACVMField(acvmContractAddress); - const functionSelector = fromACVMField(acvmFunctionSelector); - this.log( - `Calling private function ${contractAddress.toString()}:${functionSelector} from ${this.callContext.storageContractAddress.toString()}`, - ); - - const childExecutionResult = await this.callPrivateFunction( - frToAztecAddress(contractAddress), - FunctionSelector.fromField(functionSelector), - fromACVMField(acvmArgsHash), - this.callContext, - this.curve, - ); - - nestedExecutionContexts.push(childExecutionResult); - - return toAcvmCallPrivateStackItem(childExecutionResult.callStackItem); - }, - getL1ToL2Message: ([msgKey]) => { - return this.context.getL1ToL2Message(fromACVMField(msgKey)); - }, - getCommitment: ([commitment]) => this.context.getCommitment(this.contractAddress, commitment), - debugLog: (...args) => { - this.log(oracleDebugCallToFormattedStr(args)); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - debugLogWithPrefix: (arg0, ...args) => { - this.log(`${acvmFieldMessageToString(arg0)}: ${oracleDebugCallToFormattedStr(args)}`); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - enqueuePublicFunctionCall: async ([acvmContractAddress], [acvmFunctionSelector], [acvmArgsHash]) => { - const enqueuedRequest = await this.enqueuePublicFunctionCall( - frToAztecAddress(fromACVMField(acvmContractAddress)), - FunctionSelector.fromField(fromACVMField(acvmFunctionSelector)), - this.context.packedArgsCache.unpack(fromACVMField(acvmArgsHash)), - this.callContext, - ); - - this.log( - `Enqueued call to public function (with side-effect counter #${enqueuedRequest.sideEffectCounter}) ${acvmContractAddress}:${acvmFunctionSelector}`, - ); - enqueuedPublicFunctionCalls.push(enqueuedRequest); - return toAcvmEnqueuePublicFunctionResult(enqueuedRequest); - }, - emitUnencryptedLog: message => { - // https://github.com/AztecProtocol/aztec-packages/issues/885 - const log = Buffer.concat(message.map(charBuffer => convertACVMFieldToBuffer(charBuffer).subarray(-1))); - unencryptedLogs.logs.push(log); - this.log(`Emitted unencrypted log: "${log.toString('ascii')}"`); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - emitEncryptedLog: ([acvmContractAddress], [acvmStorageSlot], [encPubKeyX], [encPubKeyY], acvmPreimage) => { - const contractAddress = AztecAddress.fromBuffer(convertACVMFieldToBuffer(acvmContractAddress)); - const storageSlot = fromACVMField(acvmStorageSlot); - const preimage = acvmPreimage.map(f => fromACVMField(f)); - - const notePreimage = new NotePreimage(preimage); - const noteSpendingInfo = new NoteSpendingInfo(notePreimage, contractAddress, storageSlot); - const ownerPublicKey = new Point(fromACVMField(encPubKeyX), fromACVMField(encPubKeyY)); - - const encryptedNotePreimage = noteSpendingInfo.toEncryptedBuffer(ownerPublicKey, this.curve); - - encryptedLogs.logs.push(encryptedNotePreimage); - - return Promise.resolve(ZERO_ACVM_FIELD); - }, - getPortalContractAddress: async ([aztecAddress]) => { - const contractAddress = AztecAddress.fromString(aztecAddress); - const portalContactAddress = await this.context.db.getPortalContractAddress(contractAddress); - return Promise.resolve(toACVMField(portalContactAddress)); - }, + const { partialWitness } = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, { + packArguments: async args => { + return toACVMField(await this.context.packedArgsCache.pack(args.map(fromACVMField))); + }, + getSecretKey: ([ownerX], [ownerY]) => this.context.getSecretKey(this.contractAddress, ownerX, ownerY), + getPublicKey: async ([acvmAddress]) => { + const address = frToAztecAddress(fromACVMField(acvmAddress)); + const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address); + return [publicKey.x, publicKey.y, partialAddress].map(toACVMField); + }, + getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) => + this.context.getNotes( + this.contractAddress, + slot, + +numSelects, + selectBy, + selectValues, + sortBy, + sortOrder, + +limit, + +offset, + +returnSize, + ), + getRandomField: () => Promise.resolve(toACVMField(Fr.random())), + notifyCreatedNote: ([storageSlot], preimage, [innerNoteHash]) => { + this.context.pushNewNote( + this.contractAddress, + fromACVMField(storageSlot), + preimage.map(f => fromACVMField(f)), + fromACVMField(innerNoteHash), + ); + + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1040): remove newNotePreimages + // as it is redundant with pendingNoteData. Consider renaming pendingNoteData->pendingNotePreimages. + newNotePreimages.push({ + storageSlot: fromACVMField(storageSlot), + preimage: preimage.map(f => fromACVMField(f)), + }); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + notifyNullifiedNote: async ([slot], [nullifier], acvmPreimage, [innerNoteHash]) => { + newNullifiers.push({ + preimage: acvmPreimage.map(f => fromACVMField(f)), + storageSlot: fromACVMField(slot), + nullifier: fromACVMField(nullifier), + }); + await this.context.pushNewNullifier(fromACVMField(nullifier), this.contractAddress); + this.context.nullifyPendingNotes(fromACVMField(innerNoteHash), this.contractAddress, fromACVMField(slot)); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + callPrivateFunction: async ([acvmContractAddress], [acvmFunctionSelector], [acvmArgsHash]) => { + const contractAddress = fromACVMField(acvmContractAddress); + const functionSelector = fromACVMField(acvmFunctionSelector); + this.log( + `Calling private function ${contractAddress.toString()}:${functionSelector} from ${this.callContext.storageContractAddress.toString()}`, + ); + + const childExecutionResult = await this.callPrivateFunction( + frToAztecAddress(contractAddress), + FunctionSelector.fromField(functionSelector), + fromACVMField(acvmArgsHash), + this.callContext, + this.curve, + ); + + nestedExecutionContexts.push(childExecutionResult); + + return toAcvmCallPrivateStackItem(childExecutionResult.callStackItem); + }, + getL1ToL2Message: ([msgKey]) => { + return this.context.getL1ToL2Message(fromACVMField(msgKey)); + }, + getCommitment: ([commitment]) => this.context.getCommitment(this.contractAddress, commitment), + debugLog: (...args) => { + this.log(oracleDebugCallToFormattedStr(args)); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + debugLogWithPrefix: (arg0, ...args) => { + this.log(`${acvmFieldMessageToString(arg0)}: ${oracleDebugCallToFormattedStr(args)}`); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + enqueuePublicFunctionCall: async ([acvmContractAddress], [acvmFunctionSelector], [acvmArgsHash]) => { + const enqueuedRequest = await this.enqueuePublicFunctionCall( + frToAztecAddress(fromACVMField(acvmContractAddress)), + FunctionSelector.fromField(fromACVMField(acvmFunctionSelector)), + this.context.packedArgsCache.unpack(fromACVMField(acvmArgsHash)), + this.callContext, + ); + + this.log( + `Enqueued call to public function (with side-effect counter #${enqueuedRequest.sideEffectCounter}) ${acvmContractAddress}:${acvmFunctionSelector}`, + ); + enqueuedPublicFunctionCalls.push(enqueuedRequest); + return toAcvmEnqueuePublicFunctionResult(enqueuedRequest); + }, + emitUnencryptedLog: message => { + // https://github.com/AztecProtocol/aztec-packages/issues/885 + const log = Buffer.concat(message.map(charBuffer => convertACVMFieldToBuffer(charBuffer).subarray(-1))); + unencryptedLogs.logs.push(log); + this.log(`Emitted unencrypted log: "${log.toString('ascii')}"`); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + emitEncryptedLog: ([acvmContractAddress], [acvmStorageSlot], [encPubKeyX], [encPubKeyY], acvmPreimage) => { + const contractAddress = AztecAddress.fromBuffer(convertACVMFieldToBuffer(acvmContractAddress)); + const storageSlot = fromACVMField(acvmStorageSlot); + const preimage = acvmPreimage.map(f => fromACVMField(f)); + + const notePreimage = new NotePreimage(preimage); + const noteSpendingInfo = new NoteSpendingInfo(notePreimage, contractAddress, storageSlot); + const ownerPublicKey = new Point(fromACVMField(encPubKeyX), fromACVMField(encPubKeyY)); + + const encryptedNotePreimage = noteSpendingInfo.toEncryptedBuffer(ownerPublicKey, this.curve); + + encryptedLogs.logs.push(encryptedNotePreimage); + + return Promise.resolve(ZERO_ACVM_FIELD); + }, + getPortalContractAddress: async ([aztecAddress]) => { + const contractAddress = AztecAddress.fromString(aztecAddress); + const portalContactAddress = await this.context.db.getPortalContractAddress(contractAddress); + return Promise.resolve(toACVMField(portalContactAddress)); }, - this.abi.debug, - ).catch((err: Error) => { - throw SimulationError.fromError(this.contractAddress, selector, err); + }).catch((err: Error | ExecutionError) => { + throw SimulationError.fromError( + this.contractAddress, + selector, + err.cause instanceof Error ? err.cause : err, + extractCallStack(err, this.abi.debug), + ); }); const publicInputs = extractPrivateCircuitPublicInputs(partialWitness, acir); diff --git a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts index 2bba8bea403..862c6109385 100644 --- a/yarn-project/acir-simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/acir-simulator/src/client/unconstrained_execution.ts @@ -6,7 +6,15 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { AztecNode, SimulationError } from '@aztec/types'; import { extractReturnWitness, frToAztecAddress } from '../acvm/deserialize.js'; -import { ACVMField, ZERO_ACVM_FIELD, acvm, fromACVMField, toACVMField, toACVMWitness } from '../acvm/index.js'; +import { + ACVMField, + ZERO_ACVM_FIELD, + acvm, + extractCallStack, + fromACVMField, + toACVMField, + toACVMWitness, +} from '../acvm/index.js'; import { AcirSimulator } from '../index.js'; import { ClientTxExecutionContext } from './client_execution_context.js'; import { FunctionAbiWithDebugMetadata } from './db_oracle.js'; @@ -38,73 +46,72 @@ export class UnconstrainedFunctionExecution { const acir = Buffer.from(this.abi.bytecode, 'base64'); const initialWitness = toACVMWitness(1, this.args); - const { partialWitness } = await acvm( - await AcirSimulator.getSolver(), - acir, - initialWitness, - { - getSecretKey: ([ownerX], [ownerY]) => this.context.getSecretKey(this.contractAddress, ownerX, ownerY), - getPublicKey: async ([acvmAddress]) => { - const address = frToAztecAddress(fromACVMField(acvmAddress)); - const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address); - return [publicKey.x, publicKey.y, partialAddress].map(toACVMField); - }, - getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) => - this.context.getNotes( - this.contractAddress, - slot, - +numSelects, - selectBy, - selectValues, - sortBy, - sortOrder, - +limit, - +offset, - +returnSize, - ), - getRandomField: () => Promise.resolve(toACVMField(Fr.random())), - debugLog: (...params) => { - this.log(oracleDebugCallToFormattedStr(params)); - return Promise.resolve(ZERO_ACVM_FIELD); - }, - getL1ToL2Message: ([msgKey]) => this.context.getL1ToL2Message(fromACVMField(msgKey)), - getCommitment: ([commitment]) => this.context.getCommitment(this.contractAddress, commitment), - storageRead: async ([slot], [numberOfElements]) => { - if (!aztecNode) { - const errMsg = `Aztec node is undefined, cannot read storage`; - this.log.error(errMsg); - throw new Error(errMsg); - } + const { partialWitness } = await acvm(await AcirSimulator.getSolver(), acir, initialWitness, { + getSecretKey: ([ownerX], [ownerY]) => this.context.getSecretKey(this.contractAddress, ownerX, ownerY), + getPublicKey: async ([acvmAddress]) => { + const address = frToAztecAddress(fromACVMField(acvmAddress)); + const { publicKey, partialAddress } = await this.context.db.getCompleteAddress(address); + return [publicKey.x, publicKey.y, partialAddress].map(toACVMField); + }, + getNotes: ([slot], [numSelects], selectBy, selectValues, sortBy, sortOrder, [limit], [offset], [returnSize]) => + this.context.getNotes( + this.contractAddress, + slot, + +numSelects, + selectBy, + selectValues, + sortBy, + sortOrder, + +limit, + +offset, + +returnSize, + ), + getRandomField: () => Promise.resolve(toACVMField(Fr.random())), + debugLog: (...params) => { + this.log(oracleDebugCallToFormattedStr(params)); + return Promise.resolve(ZERO_ACVM_FIELD); + }, + getL1ToL2Message: ([msgKey]) => this.context.getL1ToL2Message(fromACVMField(msgKey)), + getCommitment: ([commitment]) => this.context.getCommitment(this.contractAddress, commitment), + storageRead: async ([slot], [numberOfElements]) => { + if (!aztecNode) { + const errMsg = `Aztec node is undefined, cannot read storage`; + this.log.error(errMsg); + throw new Error(errMsg); + } - const makeLogMsg = (slot: bigint, value: string) => - `Oracle storage read: slot=${slot.toString(16)} value=${value}`; + const makeLogMsg = (slot: bigint, value: string) => + `Oracle storage read: slot=${slot.toString(16)} value=${value}`; - const startStorageSlot = fromACVMField(slot); - const values = []; - for (let i = 0; i < Number(numberOfElements); i++) { - const storageSlot = startStorageSlot.value + BigInt(i); - const value = await aztecNode.getPublicStorageAt(this.contractAddress, storageSlot); - if (value === undefined) { - const logMsg = makeLogMsg(storageSlot, 'undefined'); - this.log(logMsg); - throw new Error(logMsg); - } - const frValue = Fr.fromBuffer(value); - const logMsg = makeLogMsg(storageSlot, frValue.toString()); + const startStorageSlot = fromACVMField(slot); + const values = []; + for (let i = 0; i < Number(numberOfElements); i++) { + const storageSlot = startStorageSlot.value + BigInt(i); + const value = await aztecNode.getPublicStorageAt(this.contractAddress, storageSlot); + if (value === undefined) { + const logMsg = makeLogMsg(storageSlot, 'undefined'); this.log(logMsg); - values.push(frValue); + throw new Error(logMsg); } - return values.map(v => toACVMField(v)); - }, - getPortalContractAddress: async ([aztecAddress]) => { - const contractAddress = AztecAddress.fromString(aztecAddress); - const portalContactAddress = await this.context.db.getPortalContractAddress(contractAddress); - return Promise.resolve(toACVMField(portalContactAddress)); - }, + const frValue = Fr.fromBuffer(value); + const logMsg = makeLogMsg(storageSlot, frValue.toString()); + this.log(logMsg); + values.push(frValue); + } + return values.map(v => toACVMField(v)); + }, + getPortalContractAddress: async ([aztecAddress]) => { + const contractAddress = AztecAddress.fromString(aztecAddress); + const portalContactAddress = await this.context.db.getPortalContractAddress(contractAddress); + return Promise.resolve(toACVMField(portalContactAddress)); }, - this.abi.debug, - ).catch((err: Error) => { - throw SimulationError.fromError(this.contractAddress, this.functionData.selector, err); + }).catch((err: Error) => { + throw SimulationError.fromError( + this.contractAddress, + this.functionData.selector, + err.cause instanceof Error ? err.cause : err, + extractCallStack(err, this.abi.debug), + ); }); const returnValues: ACVMField[] = extractReturnWitness(acir, partialWitness); diff --git a/yarn-project/acir-simulator/src/public/executor.ts b/yarn-project/acir-simulator/src/public/executor.ts index 5ed691cef2a..9d69adc1fec 100644 --- a/yarn-project/acir-simulator/src/public/executor.ts +++ b/yarn-project/acir-simulator/src/public/executor.ts @@ -16,6 +16,7 @@ import { ZERO_ACVM_FIELD, acvm, convertACVMFieldToBuffer, + extractCallStack, extractPublicCircuitPublicInputs, frToAztecAddress, fromACVMField, @@ -140,7 +141,12 @@ export class PublicExecutor { return Promise.resolve(toACVMField(portalContactAddress)); }, }).catch((err: Error) => { - throw SimulationError.fromError(execution.contractAddress, selector, err); + throw SimulationError.fromError( + execution.contractAddress, + selector, + err.cause instanceof Error ? err.cause : err, + extractCallStack(err), + ); }); const { diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index f4d5610890a..dcfd5e8ef2d 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -3,7 +3,7 @@ import { collectEncryptedLogs, collectEnqueuedPublicFunctionCalls, collectUnencryptedLogs, - processAcvmError, + resolveOpcodeLocations, } from '@aztec/acir-simulator'; import { AztecAddress, @@ -42,6 +42,7 @@ import { TxReceipt, TxStatus, getNewContractPublicFunctions, + isNoirCallStackUnresolved, toContractDao, } from '@aztec/types'; @@ -406,16 +407,10 @@ export class AztecRPCServer implements AztecRPC { originalFailingFunction.contractAddress, originalFailingFunction.functionSelector, ); - if (debugInfo) { - const noirCallStack = processAcvmError(err.message, debugInfo); - if (noirCallStack) { - err.setNoirCallStack(noirCallStack); - err.updateMessage( - `Assertion failed in public execution: '${ - noirCallStack[noirCallStack.length - 1]?.locationText ?? 'Unknown' - }'`, - ); - } + const noirCallStack = err.getNoirCallStack(); + if (debugInfo && isNoirCallStackUnresolved(noirCallStack)) { + const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo); + err.setNoirCallStack(parsedCallStack); } await this.#enrichSimulationError(err); this.log(err.toString()); diff --git a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts index ae58b45e67e..0caa4840668 100644 --- a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts @@ -52,7 +52,7 @@ describe('e2e_nested_contract', () => { parentContract.methods .entryPoint(childContract.address, childContract.methods.valueInternal.selector.toField()) .simulate(), - ).rejects.toThrowError(/Assertion failed: '.*'/); + ).rejects.toThrowError(/Assertion failed: Sender must be this contract '.*'/); }, 100_000); it('performs public nested calls', async () => { @@ -75,7 +75,7 @@ describe('e2e_nested_contract', () => { parentContract.methods .enqueueCallToChild(childContract.address, childContract.methods.pubIncValueInternal.selector.toField(), 42n) .simulate(), - ).rejects.toThrowError(/Assertion failed in public execution: '.*'/); + ).rejects.toThrowError(/Assertion failed: Sender must be this contract '.*'/); }, 100_000); it('enqueues multiple public calls', async () => { diff --git a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr index 170985a3d89..419bb4b14ef 100644 --- a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr @@ -3,7 +3,10 @@ mod storage; // A contract used along with `Parent` contract to test nested calls. contract Child { use crate::storage::Storage; - use dep::aztec::oracle::logs::emit_unencrypted_log; + use dep::aztec::{ + abi::CallContext, + oracle::logs::emit_unencrypted_log, + }; use dep::std::option::Option; #[aztec(private)] @@ -17,13 +20,17 @@ contract Child { input + context.chain_id() + context.version() } + fn check_sender(call_context: CallContext) { + assert_eq(call_context.msg_sender, call_context.storage_contract_address, "Sender must be this contract"); + } + // Returns a sum of the input and the chain id and version of the contract in private circuit public input's return_values. // Can only be called from this contract. #[aztec(private)] fn valueInternal( input: Field, ) -> Field { - assert(inputs.call_context.msg_sender == inputs.call_context.storage_contract_address); + check_sender(inputs.call_context); input + context.chain_id() + context.version() } @@ -60,7 +67,7 @@ contract Child { #[aztec(public)] fn pubIncValueInternal(new_value: Field) -> Field { let storage = Storage::init(Option::none(), Option::some(&mut context)); - assert(inputs.call_context.msg_sender == inputs.call_context.storage_contract_address); + check_sender(inputs.call_context); let old_value = storage.current_value.read(); storage.current_value.write(old_value + new_value); let _hash = emit_unencrypted_log(new_value); diff --git a/yarn-project/noir-libs/easy-private-state/src/easy_private_state.nr b/yarn-project/noir-libs/easy-private-state/src/easy_private_state.nr index a5072a3f6f2..0c6761ec0fb 100644 --- a/yarn-project/noir-libs/easy-private-state/src/easy_private_state.nr +++ b/yarn-project/noir-libs/easy-private-state/src/easy_private_state.nr @@ -24,7 +24,7 @@ impl EasyPrivateUint { public_context: Option<&mut PublicContext>, storage_slot: Field, ) -> Self { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); let set = Set { private_context, public_context, diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/immutable_singleton.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/immutable_singleton.nr index 175876a88ea..1d135f03137 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/immutable_singleton.nr +++ b/yarn-project/noir-libs/noir-aztec/src/state_vars/immutable_singleton.nr @@ -22,7 +22,7 @@ impl ImmutableSingleton { storage_slot: Field, note_interface: NoteInterface, ) -> Self { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); ImmutableSingleton { context, storage_slot, diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/map.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/map.nr index ab6485bca8d..0adb1ea0303 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/map.nr +++ b/yarn-project/noir-libs/noir-aztec/src/state_vars/map.nr @@ -19,7 +19,7 @@ impl Map { Field, ) -> V, ) -> Map { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); Map { private_context, public_context, diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/public_state.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/public_state.nr index 6e09fca6fc8..ee10446a7fd 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/public_state.nr +++ b/yarn-project/noir-libs/noir-aztec/src/state_vars/public_state.nr @@ -17,7 +17,7 @@ impl PublicState { storage_slot: Field, serialisation_methods: TypeSerialisationInterface, ) -> Self { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); PublicState { storage_slot, serialisation_methods, diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr index 1864fe96902..b8813c989a3 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr +++ b/yarn-project/noir-libs/noir-aztec/src/state_vars/set.nr @@ -25,7 +25,7 @@ impl Set { storage_slot: Field, note_interface: NoteInterface, ) -> Self { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); Set { private_context, public_context, diff --git a/yarn-project/noir-libs/noir-aztec/src/state_vars/singleton.nr b/yarn-project/noir-libs/noir-aztec/src/state_vars/singleton.nr index 561a5740c94..e386107f78b 100644 --- a/yarn-project/noir-libs/noir-aztec/src/state_vars/singleton.nr +++ b/yarn-project/noir-libs/noir-aztec/src/state_vars/singleton.nr @@ -23,7 +23,7 @@ impl Singleton { storage_slot: Field, note_interface: NoteInterface, ) -> Self { - assert(storage_slot != 0); // Storage slot 0 not allowed. Storage slots must start from 1. + assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1."); Singleton { context, storage_slot, diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index 9477e758984..f17f72b86d3 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -1,4 +1,5 @@ import { AztecAddress, FunctionSelector } from '@aztec/circuits.js'; +import { OpcodeLocation } from '@aztec/foundation/abi'; /** * Address and selector of a function that failed during simulation. @@ -47,12 +48,20 @@ export interface SourceCodeLocation { /** * A stack of noir source code locations. */ -export type NoirCallStack = SourceCodeLocation[]; +export type NoirCallStack = SourceCodeLocation[] | OpcodeLocation[]; + +/** + * Checks if a call stack is unresolved. + */ +export function isNoirCallStackUnresolved(callStack: NoirCallStack): callStack is OpcodeLocation[] { + return typeof callStack[0] === 'string'; +} /** * An error during the simulation of a function call. */ export class SimulationError extends Error { + private originalMessage: string; private functionErrorStack: FailingFunction[]; // We want to maintain a public constructor for proper printing. @@ -63,6 +72,8 @@ export class SimulationError extends Error { options?: ErrorOptions, ) { super(message, options); + this.originalMessage = message; + this.addSourceCodeSnippetToMessage(); this.functionErrorStack = [failingFunction]; } @@ -73,18 +84,14 @@ export class SimulationError extends Error { static fromError( failingContract: AztecAddress, failingselector: FunctionSelector, - err: Error & { - /** - * The noir call stack. - */ - callStack?: NoirCallStack; - }, + err: Error, + callStack?: NoirCallStack, ) { const failingFunction = { contractAddress: failingContract, functionSelector: failingselector }; if (err instanceof SimulationError) { return SimulationError.extendPreviousSimulationError(failingFunction, err); } - return new SimulationError(err.message, failingFunction, err?.callStack, { + return new SimulationError(err.message, failingFunction, callStack, { cause: err, }); } @@ -139,20 +146,29 @@ export class SimulationError extends Error { failingFunction.functionName ?? failingFunction.functionSelector.toString() }`; }), - ...noirCallStack.map( - sourceCodeLocation => - ` at ${sourceCodeLocation.filePath}:${sourceCodeLocation.line} '${sourceCodeLocation.locationText}'`, + ...noirCallStack.map(errorLocation => + typeof errorLocation === 'string' + ? ` at opcode ${errorLocation}` + : ` at ${errorLocation.filePath}:${errorLocation.line} '${errorLocation.locationText}'`, ), ]; return [`Simulation error: ${this.message}`, ...stackLines.reverse()].join('\n'); } + private addSourceCodeSnippetToMessage() { + if (this.noirErrorStack && !isNoirCallStackUnresolved(this.noirErrorStack) && this.noirErrorStack.length) { + this.updateMessage( + `${this.originalMessage} '${this.noirErrorStack[this.noirErrorStack.length - 1].locationText}'`, + ); + } + } + /** * Updates the error message. This is needed because in some engines the stack also contains the message. * @param newMessage - The new message of this error. */ - updateMessage(newMessage: string) { + private updateMessage(newMessage: string) { const oldMessage = this.message; this.message = newMessage; if (this.stack?.startsWith(`Error: ${oldMessage}`)) { @@ -181,6 +197,7 @@ export class SimulationError extends Error { */ setNoirCallStack(callStack: NoirCallStack) { this.noirErrorStack = callStack; + this.addSourceCodeSnippetToMessage(); } toJSON() { diff --git a/yarn-project/yarn.lock b/yarn-project/yarn.lock index c60584ae0c5..89cbaf38815 100644 --- a/yarn-project/yarn.lock +++ b/yarn-project/yarn.lock @@ -70,7 +70,7 @@ __metadata: "@rushstack/eslint-patch": ^1.1.4 "@types/jest": ^29.5.0 "@types/node": ^18.7.23 - acvm_js: "github:noir-lang/acvm-js-wasm#arv/0.24.1" + acvm_js: "github:noir-lang/acvm-js-wasm#arv/0.25.0" jest: ^29.5.0 jest-mock-extended: ^3.0.4 levelup: ^5.1.1 @@ -3957,10 +3957,10 @@ __metadata: languageName: node linkType: hard -"acvm_js@github:noir-lang/acvm-js-wasm#arv/0.24.1": - version: 0.0.0-675753c - resolution: "acvm_js@https://github.com/noir-lang/acvm-js-wasm.git#commit=8e61f6fd9f4f8b65ad718912d9b3f01a6b869c11" - checksum: 3c40827d2f41dbc946a976b3a6f0091d1de6832f26bf45eec59204cf37d78f1db213cf2cc4a884996bacc4eb410b7c48d761de22058d8fc339c1c1a7a8b60ec8 +"acvm_js@github:noir-lang/acvm-js-wasm#arv/0.25.0": + version: 0.0.0-9d02637 + resolution: "acvm_js@https://github.com/noir-lang/acvm-js-wasm.git#commit=73cc4c22f4d443c9b287ebd3824a80b912fd0e58" + checksum: 2a2923073a9835ec0200a7bc7818023e8ab2cc87aef6238c9ee3af2a844d535dfadfd2dbc75ed82593ea4cd26a0288f91989ee7bb3a37481f63be21b76628574 languageName: node linkType: hard From 1c1862e13b6c7f3e9488f23e7dbcce82edb4972a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Sep 2023 09:26:19 +0000 Subject: [PATCH 2/5] refactor: improve simulation error --- .../src/aztec-node/http-node.test.ts | 7 ++- yarn-project/types/src/simulation_error.ts | 52 +++++++++++++------ 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts index 32f105ccda5..82c1f0de3da 100644 --- a/yarn-project/aztec-node/src/aztec-node/http-node.test.ts +++ b/yarn-project/aztec-node/src/aztec-node/http-node.test.ts @@ -503,13 +503,12 @@ describe('HttpNode', () => { it('should fetch a simulation error', async () => { const tx = mockTx(); - const simulationError = new SimulationError('Failing function', { - contractAddress: AztecAddress.ZERO, - functionSelector: FunctionSelector.empty(), - }); + const simulationError = SimulationError.new('Fake simulation error', AztecAddress.ZERO, FunctionSelector.empty()); + const response = { simulationError: simulationError.toJSON(), }; + setFetchMock(response); await expect(httpNode.simulatePublicCalls(tx)).rejects.toThrow(simulationError); diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index f17f72b86d3..a43b46d0988 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -61,37 +61,58 @@ export function isNoirCallStackUnresolved(callStack: NoirCallStack): callStack i * An error during the simulation of a function call. */ export class SimulationError extends Error { - private originalMessage: string; - private functionErrorStack: FailingFunction[]; - - // We want to maintain a public constructor for proper printing. - constructor( + private constructor( message: string, - failingFunction: FailingFunction, + private originalMessage: string, + private functionErrorStack: FailingFunction[], private noirErrorStack?: NoirCallStack, options?: ErrorOptions, ) { super(message, options); - this.originalMessage = message; this.addSourceCodeSnippetToMessage(); - this.functionErrorStack = [failingFunction]; } private addCaller(failingFunction: FailingFunction) { this.functionErrorStack.unshift(failingFunction); } - static fromError( + /** + * Creates a new simulation error + * @param message - The error message + * @param failingContract - The address of the contract that failed. + * @param failingSelector - The selector of the function that failed. + * @param callStack - The noir call stack of the error. + * @returns - The simulation error. + */ + public static new( + message: string, + failingContract: AztecAddress, + failingSelector: FunctionSelector, + callStack?: NoirCallStack, + ) { + const failingFunction = { contractAddress: failingContract, functionSelector: failingSelector }; + return new SimulationError(message, message, [failingFunction], callStack); + } + + /** + * Creates a new simulation error from an error thrown during simulation. + * @param failingContract - The address of the contract that failed. + * @param failingSelector - The selector of the function that failed. + * @param err - The error that was thrown. + * @param callStack - The noir call stack of the error. + * @returns - The simulation error. + */ + public static fromError( failingContract: AztecAddress, - failingselector: FunctionSelector, + failingSelector: FunctionSelector, err: Error, callStack?: NoirCallStack, ) { - const failingFunction = { contractAddress: failingContract, functionSelector: failingselector }; + const failingFunction = { contractAddress: failingContract, functionSelector: failingSelector }; if (err instanceof SimulationError) { return SimulationError.extendPreviousSimulationError(failingFunction, err); } - return new SimulationError(err.message, failingFunction, callStack, { + return new SimulationError(err.message, err.message, [failingFunction], callStack, { cause: err, }); } @@ -203,14 +224,13 @@ export class SimulationError extends Error { toJSON() { return { message: this.message, + originalMessage: this.originalMessage, functionErrorStack: this.functionErrorStack, noirErrorStack: this.noirErrorStack, }; } - static fromJSON(obj: any) { - const error = new SimulationError(obj.message, obj.functionErrorStack[0], obj.noirErrorStack); - error.functionErrorStack = obj.functionErrorStack; - return error; + static fromJSON(obj: ReturnType) { + return new SimulationError(obj.message, obj.originalMessage, obj.functionErrorStack, obj.noirErrorStack); } } From 405fc6a2504cd436458edf1c46bb820147e9b2cd Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Sep 2023 09:28:28 +0000 Subject: [PATCH 3/5] style: unify visibility --- yarn-project/types/src/simulation_error.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index a43b46d0988..ed349c3599f 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -84,7 +84,7 @@ export class SimulationError extends Error { * @param callStack - The noir call stack of the error. * @returns - The simulation error. */ - public static new( + static new( message: string, failingContract: AztecAddress, failingSelector: FunctionSelector, @@ -102,7 +102,7 @@ export class SimulationError extends Error { * @param callStack - The noir call stack of the error. * @returns - The simulation error. */ - public static fromError( + static fromError( failingContract: AztecAddress, failingSelector: FunctionSelector, err: Error, @@ -117,7 +117,7 @@ export class SimulationError extends Error { }); } - static extendPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { + private static extendPreviousSimulationError(failingFunction: FailingFunction, previousError: SimulationError) { previousError.addCaller(failingFunction); return previousError; } From a4c192fd97bb02817194d0a3623744d73b593a46 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Sep 2023 09:51:42 +0000 Subject: [PATCH 4/5] style: remove old promise resolve --- yarn-project/acir-simulator/src/acvm/acvm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index bf07687045a..f33d2110f3c 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -150,7 +150,7 @@ export async function acvm( }, ); - return Promise.resolve({ partialWitness }); + return { partialWitness }; } /** From e42235877f5360021ab170855d7164a980d4f9ec Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 5 Sep 2023 12:20:41 +0000 Subject: [PATCH 5/5] feat: get/set for message & stack --- yarn-project/acir-simulator/src/acvm/acvm.ts | 6 +- .../src/aztec_rpc_server/aztec_rpc_server.ts | 3 - yarn-project/types/src/simulation_error.ts | 80 ++++++++++--------- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/yarn-project/acir-simulator/src/acvm/acvm.ts b/yarn-project/acir-simulator/src/acvm/acvm.ts index f33d2110f3c..b1bed953536 100644 --- a/yarn-project/acir-simulator/src/acvm/acvm.ts +++ b/yarn-project/acir-simulator/src/acvm/acvm.ts @@ -89,11 +89,15 @@ function getSourceCodeLocationsFromOpcodeLocation( const locationText = source.substring(span.start, span.end + 1); const precedingText = source.substring(0, span.start); - const line = precedingText.split('\n').length; + const previousLines = precedingText.split('\n'); + // Lines and columns in stacks are one indexed. + const line = previousLines.length; + const column = previousLines[previousLines.length - 1].length + 1; return { filePath: path, line, + column, fileSource: source, locationText, }; diff --git a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts index dcfd5e8ef2d..a0b2284afbe 100644 --- a/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts +++ b/yarn-project/aztec-rpc/src/aztec_rpc_server/aztec_rpc_server.ts @@ -341,7 +341,6 @@ export class AztecRPCServer implements AztecRPC { } catch (err) { if (err instanceof SimulationError) { await this.#enrichSimulationError(err); - this.log(err.toString()); } throw err; } @@ -382,7 +381,6 @@ export class AztecRPCServer implements AztecRPC { } catch (err) { if (err instanceof SimulationError) { await this.#enrichSimulationError(err); - this.log(err.toString()); } throw err; } @@ -413,7 +411,6 @@ export class AztecRPCServer implements AztecRPC { err.setNoirCallStack(parsedCallStack); } await this.#enrichSimulationError(err); - this.log(err.toString()); } throw err; diff --git a/yarn-project/types/src/simulation_error.ts b/yarn-project/types/src/simulation_error.ts index ed349c3599f..1da8a4cac30 100644 --- a/yarn-project/types/src/simulation_error.ts +++ b/yarn-project/types/src/simulation_error.ts @@ -35,6 +35,10 @@ export interface SourceCodeLocation { * The line number of the call. */ line: number; + /** + * The column number of the call. + */ + column: number; /** * The source code of the file. */ @@ -62,14 +66,44 @@ export function isNoirCallStackUnresolved(callStack: NoirCallStack): callStack i */ export class SimulationError extends Error { private constructor( - message: string, private originalMessage: string, private functionErrorStack: FailingFunction[], private noirErrorStack?: NoirCallStack, options?: ErrorOptions, ) { - super(message, options); - this.addSourceCodeSnippetToMessage(); + super(originalMessage, options); + Object.defineProperties(this, { + message: { + configurable: false, + enumerable: true, + /** + * Getter for the custom error message. It has to be defined here because JS errors have the message property defined + * in the error itself, not its prototype. Thus if we define it as a class getter will be shadowed. + * @returns The message. + */ + get() { + return this.getMessage(); + }, + }, + stack: { + configurable: false, + enumerable: true, + /** + * Getter for the custom error stack. It has to be defined here due to the same issue as the message. + * @returns The stack. + */ + get() { + return this.getStack(); + }, + }, + }); + } + + getMessage() { + if (this.noirErrorStack && !isNoirCallStackUnresolved(this.noirErrorStack) && this.noirErrorStack.length) { + return `${this.originalMessage} '${this.noirErrorStack[this.noirErrorStack.length - 1].locationText}'`; + } + return this.originalMessage; } private addCaller(failingFunction: FailingFunction) { @@ -91,7 +125,7 @@ export class SimulationError extends Error { callStack?: NoirCallStack, ) { const failingFunction = { contractAddress: failingContract, functionSelector: failingSelector }; - return new SimulationError(message, message, [failingFunction], callStack); + return new SimulationError(message, [failingFunction], callStack); } /** @@ -112,7 +146,7 @@ export class SimulationError extends Error { if (err instanceof SimulationError) { return SimulationError.extendPreviousSimulationError(failingFunction, err); } - return new SimulationError(err.message, err.message, [failingFunction], callStack, { + return new SimulationError(err.message, [failingFunction], callStack, { cause: err, }); } @@ -152,51 +186,27 @@ export class SimulationError extends Error { }); } - /** - * Returns a string representation of the error. - * @returns The string. - */ - toString() { + getStack() { const functionCallStack = this.getCallStack(); const noirCallStack = this.getNoirCallStack(); // Try to resolve the contract and function names of the stack of failing functions. const stackLines: string[] = [ ...functionCallStack.map(failingFunction => { - return ` at ${failingFunction.contractName ?? failingFunction.contractAddress.toString()}.${ + return `at ${failingFunction.contractName ?? failingFunction.contractAddress.toString()}.${ failingFunction.functionName ?? failingFunction.functionSelector.toString() }`; }), ...noirCallStack.map(errorLocation => typeof errorLocation === 'string' - ? ` at opcode ${errorLocation}` - : ` at ${errorLocation.filePath}:${errorLocation.line} '${errorLocation.locationText}'`, + ? `at opcode ${errorLocation}` + : `at ${errorLocation.locationText} (${errorLocation.filePath}:${errorLocation.line}:${errorLocation.column})`, ), ]; return [`Simulation error: ${this.message}`, ...stackLines.reverse()].join('\n'); } - private addSourceCodeSnippetToMessage() { - if (this.noirErrorStack && !isNoirCallStackUnresolved(this.noirErrorStack) && this.noirErrorStack.length) { - this.updateMessage( - `${this.originalMessage} '${this.noirErrorStack[this.noirErrorStack.length - 1].locationText}'`, - ); - } - } - - /** - * Updates the error message. This is needed because in some engines the stack also contains the message. - * @param newMessage - The new message of this error. - */ - private updateMessage(newMessage: string) { - const oldMessage = this.message; - this.message = newMessage; - if (this.stack?.startsWith(`Error: ${oldMessage}`)) { - this.stack = this.stack?.replace(`Error: ${oldMessage}`, `Error: ${newMessage}`); - } - } - /** * The aztec function stack that failed during simulation. */ @@ -218,12 +228,10 @@ export class SimulationError extends Error { */ setNoirCallStack(callStack: NoirCallStack) { this.noirErrorStack = callStack; - this.addSourceCodeSnippetToMessage(); } toJSON() { return { - message: this.message, originalMessage: this.originalMessage, functionErrorStack: this.functionErrorStack, noirErrorStack: this.noirErrorStack, @@ -231,6 +239,6 @@ export class SimulationError extends Error { } static fromJSON(obj: ReturnType) { - return new SimulationError(obj.message, obj.originalMessage, obj.functionErrorStack, obj.noirErrorStack); + return new SimulationError(obj.originalMessage, obj.functionErrorStack, obj.noirErrorStack); } }