diff --git a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr index 586ad030846..e80df65b8a3 100644 --- a/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr @@ -229,6 +229,21 @@ contract AvmTest { helper_with_failed_assertion() } + #[public] + fn external_call_to_assertion_failure() { + AvmTest::at(context.this_address()).assertion_failure().call(&mut context); + } + + #[public] + fn divide_by_zero() -> u8 { + 1 / 0 + } + + #[public] + fn external_call_to_divide_by_zero() { + AvmTest::at(context.this_address()).divide_by_zero().call(&mut context); + } + #[public] fn debug_logging() { dep::aztec::oracle::debug_log::debug_log("just text"); diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts index c594663f8de..026a2e8602f 100644 --- a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts +++ b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts @@ -33,15 +33,32 @@ describe('e2e_avm_simulator', () => { }); describe('Assertions', () => { - it('PXE processes failed assertions and fills in the error message with the expression', async () => { - await expect(avmContract.methods.assertion_failure().simulate()).rejects.toThrow( - "Assertion failed: This assertion should fail! 'not_true == true'", - ); - }); - it('PXE processes failed assertions and fills in the error message with the expression (even complex ones)', async () => { - await expect(avmContract.methods.assert_nullifier_exists(123).simulate()).rejects.toThrow( - "Assertion failed: Nullifier doesn't exist! 'context.nullifier_exists(nullifier, context.this_address())'", - ); + describe('Not nested', () => { + it('PXE processes user code assertions and recovers message', async () => { + await expect(avmContract.methods.assertion_failure().simulate()).rejects.toThrow( + "Assertion failed: This assertion should fail! 'not_true == true'", + ); + }); + it('PXE processes user code assertions and recovers message (complex)', async () => { + await expect(avmContract.methods.assert_nullifier_exists(123).simulate()).rejects.toThrow( + "Assertion failed: Nullifier doesn't exist! 'context.nullifier_exists(nullifier, context.this_address())'", + ); + }); + it('PXE processes intrinsic assertions and recovers message', async () => { + await expect(avmContract.methods.divide_by_zero().simulate()).rejects.toThrow('Division by zero'); + }); + }); + describe('Nested', () => { + it('PXE processes user code assertions and recovers message', async () => { + await expect(avmContract.methods.external_call_to_assertion_failure().simulate()).rejects.toThrow( + "Assertion failed: This assertion should fail! 'not_true == true'", + ); + }); + it('PXE processes intrinsic assertions and recovers message', async () => { + await expect(avmContract.methods.external_call_to_divide_by_zero().simulate()).rejects.toThrow( + 'Division by zero', + ); + }); }); }); diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index bbd031eb315..dce850d220b 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -1,18 +1,15 @@ -import { type NoirCallStack, type SourceCodeLocation } from '@aztec/circuit-types'; -import { type Fr } from '@aztec/circuits.js'; -import type { BrilligFunctionId, FunctionAbi, FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; +import { type NoirCallStack } from '@aztec/circuit-types'; +import type { FunctionDebugMetadata } from '@aztec/foundation/abi'; import { createDebugLogger } from '@aztec/foundation/log'; import { type ExecutionError, type ForeignCallInput, type ForeignCallOutput, - type RawAssertionPayload, executeCircuitWithReturnWitness, } from '@noir-lang/acvm_js'; -import { abiDecodeError } from '@noir-lang/noirc_abi'; -import { traverseCauseChain } from '../common/errors.js'; +import { resolveOpcodeLocations, traverseCauseChain } from '../common/errors.js'; import { type ACVMWitness } from './acvm_types.js'; import { type ORACLE_NAMES } from './oracle/index.js'; @@ -37,112 +34,6 @@ export interface ACIRExecutionResult { returnWitness: ACVMWitness; } -/** - * Extracts a brillig location from an opcode location. - * @param opcodeLocation - The opcode location to extract from. It should be in the format `acirLocation.brilligLocation` or `acirLocation`. - * @returns The brillig location if the opcode location contains one. - */ -function extractBrilligLocation(opcodeLocation: string): string | undefined { - const splitted = opcodeLocation.split('.'); - if (splitted.length === 2) { - return splitted[1]; - } - return undefined; -} - -/** - * 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 getSourceCodeLocationsFromOpcodeLocation( - opcodeLocation: string, - debug: FunctionDebugMetadata, - brilligFunctionId?: BrilligFunctionId, -): SourceCodeLocation[] { - const { debugSymbols, files } = debug; - - let callStack = debugSymbols.locations[opcodeLocation] || []; - if (callStack.length === 0) { - const brilligLocation = extractBrilligLocation(opcodeLocation); - if (brilligFunctionId !== undefined && brilligLocation !== undefined) { - callStack = debugSymbols.brillig_locations[brilligFunctionId][brilligLocation] || []; - } - } - return callStack.map(call => { - const { file: fileId, span } = call; - - const { path, source } = files[fileId]; - - const locationText = source.substring(span.start, span.end); - const precedingText = source.substring(0, span.start); - 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, - }; - }); -} - -/** - * 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. - */ -export function resolveOpcodeLocations( - opcodeLocations: OpcodeLocation[], - debug: FunctionDebugMetadata, - brilligFunctionId?: BrilligFunctionId, -): SourceCodeLocation[] { - return opcodeLocations.flatMap(opcodeLocation => - getSourceCodeLocationsFromOpcodeLocation(opcodeLocation, debug, brilligFunctionId), - ); -} - -export function resolveAssertionMessage(errorPayload: RawAssertionPayload, abi: FunctionAbi): string | undefined { - const decoded = abiDecodeError( - { parameters: [], error_types: abi.errorTypes, return_type: null }, // eslint-disable-line camelcase - errorPayload, - ); - - if (typeof decoded === 'string') { - return decoded; - } else { - return JSON.stringify(decoded); - } -} - -export function resolveAssertionMessageFromRevertData(revertData: Fr[], abi: FunctionAbi): string | undefined { - if (revertData.length == 0) { - return undefined; - } - - const [errorSelector, ...errorData] = revertData; - - return resolveAssertionMessage( - { - selector: errorSelector.toBigInt().toString(), - data: errorData.map(f => f.toString()), - }, - abi, - ); -} - -export function resolveAssertionMessageFromError(err: Error, abi: FunctionAbi): string { - if (typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { - return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, abi)}`; - } else { - return err.message; - } -} - /** * The function call that executes an ACIR. */ diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index 3c1aac84528..1ba4fa601b9 100644 --- a/yarn-project/simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/simulator/src/avm/avm_machine_state.ts @@ -2,7 +2,7 @@ import { type Fr } from '@aztec/circuits.js'; import { GAS_DIMENSIONS, type Gas } from './avm_gas.js'; import { TaggedMemory } from './avm_memory_types.js'; -import { OutOfGasError } from './errors.js'; +import { type AvmRevertReason, OutOfGasError } from './errors.js'; /** * A few fields of machine state are initialized from AVM session inputs or call instruction arguments @@ -12,6 +12,16 @@ export type InitialAvmMachineState = { daGasLeft: number; }; +/** + * Used to track the call stack and revert data of nested calls. + * This is used to provide a more detailed revert reason when a contract call reverts. + * It is only a heuristic and may not always provide the correct revert reason. + */ +type TrackedRevertInfo = { + revertDataRepresentative: Fr[]; + recursiveRevertReason: AvmRevertReason; +}; + type CallStackEntry = { callPc: number; returnPc: number; @@ -30,6 +40,12 @@ export class AvmMachineState { public nextPc: number = 0; /** return/revertdata of the last nested call. */ public nestedReturndata: Fr[] = []; + /** + * Used to track the call stack and revert data of nested calls. + * This is used to provide a more detailed revert reason when a contract call reverts. + * It is only a heuristic and may not always provide the correct revert reason. + */ + public collectedRevertInfo: TrackedRevertInfo | undefined; /** * On INTERNALCALL, internal call stack is pushed to with the current pc and the return pc. diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 70682ec440a..f4d84ead171 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -20,6 +20,7 @@ import { type MemoryValue, TypeTag, type Uint8, type Uint64 } from './avm_memory import { AvmSimulator } from './avm_simulator.js'; import { isAvmBytecode, markBytecodeAsAvm } from './bytecode_utils.js'; import { + getAvmTestContractArtifact, getAvmTestContractBytecode, initContext, initExecutionEnvironment, @@ -321,10 +322,10 @@ describe('AVM simulator: transpiled Noir contracts', () => { expect(results.reverted).toBe(true); expect(results.revertReason).toBeDefined(); + expect(results.output).toHaveLength(1); // Error selector for static string error expect( resolveAvmTestContractAssertionMessage('assert_nullifier_exists', results.revertReason!, results.output), ).toMatch("Nullifier doesn't exist!"); - expect(results.output).toHaveLength(1); // Error selector for static string error }); describe.each([ @@ -929,14 +930,16 @@ describe('AVM simulator: transpiled Noir contracts', () => { it(`Nested call with not enough gas (expect failure)`, async () => { const gas = [/*l2=*/ 5, /*da=*/ 10000].map(g => new Fr(g)); - const calldata: Fr[] = [value0, value1, ...gas]; + const targetFunctionSelector = FunctionSelector.fromSignature( + 'nested_call_to_add_with_gas(Field,Field,Field,Field)', + ); + const calldata: Fr[] = [targetFunctionSelector.toField(), value0, value1, ...gas]; const context = createContext(calldata); - const callBytecode = getAvmTestContractBytecode('nested_call_to_add_with_gas'); - const nestedBytecode = getAvmTestContractBytecode('public_dispatch'); - mockGetBytecode(worldStateDB, nestedBytecode); + const artifact = getAvmTestContractArtifact('public_dispatch'); + mockGetBytecode(worldStateDB, artifact.bytecode); const contractClass = makeContractClassPublic(0, { - bytecode: nestedBytecode, + bytecode: artifact.bytecode, selector: FunctionSelector.random(), }); mockGetContractClass(worldStateDB, contractClass); @@ -945,16 +948,12 @@ describe('AVM simulator: transpiled Noir contracts', () => { mockTraceFork(trace); - const results = await new AvmSimulator(context).executeBytecode(callBytecode); - // TODO(7141): change this once we don't force rethrowing of exceptions. - // Outer frame should not revert, but inner should, so the forwarded return value is 0 - // expect(results.revertReason).toBeUndefined(); - // expect(results.reverted).toBe(false); + const results = await new AvmSimulator(context).executeBytecode(artifact.bytecode); expect(results.reverted).toBe(true); - expect(results.revertReason?.message).toEqual('Not enough L2GAS gas left'); + expect(results.revertReason?.message).toMatch('Not enough L2GAS gas left'); - // Nested call should NOT have been made and therefore should not be traced - expect(trace.traceNestedCall).toHaveBeenCalledTimes(0); + // Nested call should have been made (and failed). + expect(trace.traceNestedCall).toHaveBeenCalledTimes(1); }); it(`Nested static call which modifies storage (expect failure)`, async () => { @@ -971,7 +970,8 @@ describe('AVM simulator: transpiled Noir contracts', () => { const contractInstance = makeContractInstanceFromClassId(contractClass.id); mockGetContractInstance(worldStateDB, contractInstance); - mockTraceFork(trace); + const nestedTrace = mock(); + mockTraceFork(trace, nestedTrace); const results = await new AvmSimulator(context).executeBytecode(callBytecode); @@ -980,9 +980,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { 'Static call cannot update the state, emit L2->L1 messages or generate logs', ); - // TODO(7141): external call doesn't recover from nested exception until - // we support recoverability of reverts (here and in kernel) - //expectTracedNestedCall(context.environment, results, nestedTrace, /*isStaticCall=*/true); + expectTracedNestedCall(context.environment, nestedTrace, /*isStaticCall=*/ true); // Nested call should NOT have been able to write storage expect(trace.tracePublicStorageWrite).toHaveBeenCalledTimes(0); diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 788a3908b2b..7b6f7e26aef 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -12,7 +12,6 @@ import { AvmExecutionError, InvalidProgramCounterError, NoBytecodeForContractError, - revertDataFromExceptionalHalt, revertReasonFromExceptionalHalt, revertReasonFromExplicitRevert, } from './errors.js'; @@ -134,12 +133,8 @@ export class AvmSimulator { } const revertReason = revertReasonFromExceptionalHalt(err, this.context); - // Note: "exceptional halts" cannot return data, hence [] - const results = new AvmContractCallResult( - /*reverted=*/ true, - /*output=*/ revertDataFromExceptionalHalt(err), - revertReason, - ); + // Note: "exceptional halts" cannot return data, hence []. + const results = new AvmContractCallResult(/*reverted=*/ true, /*output=*/ [], revertReason); this.log.debug(`Context execution results: ${results.toString()}`); this.printOpcodeTallies(); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 444d95ce28b..f9c5d819a77 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -21,6 +21,13 @@ export class NoBytecodeForContractError extends AvmExecutionError { } } +export class ArithmeticError extends AvmExecutionError { + constructor(message: string) { + super(message); + this.name = 'ArithmeticError'; + } +} + /** * Error is thrown when the program counter goes to an invalid location. * There is no instruction at the provided pc @@ -78,18 +85,6 @@ export class StaticCallAlterationError extends InstructionExecutionError { } } -/** - * Error thrown to propagate a nested call's revert. - * @param message - the error's message - * @param nestedError - the revert reason of the nested call - */ -export class RethrownError extends AvmExecutionError { - constructor(message: string, public nestedError: AvmRevertReason, public revertData: Fr[]) { - super(message); - this.name = 'RethrownError'; - } -} - /** * Meaningfully named alias for ExecutionError when used in the context of the AVM. * Maintains a recursive structure reflecting the AVM's external callstack/errorstack, where @@ -101,14 +96,7 @@ export class AvmRevertReason extends ExecutionError { } } -/** - * Helper to create a "revert reason" error optionally with a nested error cause. - * - * @param message - the error message - * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack - * @param nestedError - the error that caused this one (if this is not the root-cause itself) - */ -function createRevertReason(message: string, context: AvmContext, nestedError?: AvmRevertReason): AvmRevertReason { +function createRevertReason(message: string, revertData: Fr[], context: AvmContext): AvmRevertReason { // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this. // If the function selector is the public dispatch selector, we need to extract the actual function selector from the calldata. // We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention. @@ -118,6 +106,18 @@ function createRevertReason(message: string, context: AvmContext, nestedError?: if (functionSelector.toField().equals(new Fr(PUBLIC_DISPATCH_SELECTOR)) && context.environment.calldata.length > 0) { functionSelector = FunctionSelector.fromField(context.environment.calldata[0]); } + + // If we are reverting due to the same error that we have been tracking, we use the nested error as the cause. + let nestedError = undefined; + const revertDataEquals = (a: Fr[], b: Fr[]) => a.length === b.length && a.every((v, i) => v.equals(b[i])); + if ( + context.machineState.collectedRevertInfo && + revertDataEquals(context.machineState.collectedRevertInfo.revertDataRepresentative, revertData) + ) { + nestedError = context.machineState.collectedRevertInfo.recursiveRevertReason; + message = context.machineState.collectedRevertInfo.recursiveRevertReason.message; + } + return new AvmRevertReason( message, /*failingFunction=*/ { @@ -130,25 +130,13 @@ function createRevertReason(message: string, context: AvmContext, nestedError?: } /** - * Create a "revert reason" error for an exceptional halt, - * creating the recursive structure if the halt was a RethrownError. + * Create a "revert reason" error for an exceptional halt. * * @param haltingError - the lower-level error causing the exceptional halt * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack */ export function revertReasonFromExceptionalHalt(haltingError: AvmExecutionError, context: AvmContext): AvmRevertReason { - // A RethrownError has a nested/child AvmRevertReason - const nestedError = haltingError instanceof RethrownError ? haltingError.nestedError : undefined; - return createRevertReason(haltingError.message, context, nestedError); -} - -/** - * Extracts revert data from an exceptional halt. Currently this is only used to manually bubble up revertadata. - * @param haltingError - the lower-level error causing the exceptional halt - * @returns the revert data for the exceptional halt - */ -export function revertDataFromExceptionalHalt(haltingError: AvmExecutionError): Fr[] { - return haltingError instanceof RethrownError ? haltingError.revertData : []; + return createRevertReason(haltingError.message, [], context); } /** @@ -158,26 +146,5 @@ export function revertDataFromExceptionalHalt(haltingError: AvmExecutionError): * @param context - the context of the AVM execution used to extract the failingFunction and noirCallStack */ export function revertReasonFromExplicitRevert(revertData: Fr[], context: AvmContext): AvmRevertReason { - const revertMessage = decodeRevertDataAsMessage(revertData); - return createRevertReason(revertMessage, context); -} - -/** - * Interpret revert data as a message string. - * - * @param revertData - output data of an explicit REVERT instruction - */ -export function decodeRevertDataAsMessage(revertData: Fr[]): string { - if (revertData.length === 0) { - return 'Assertion failed'; - } else { - try { - // We remove the first element which is the 'error selector'. - const revertOutput = revertData.slice(1); - // Try to interpret the output as a text string. - return 'Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber())); - } catch (e) { - return 'Assertion failed: '; - } - } + return createRevertReason('Assertion failed: ', revertData, context); } diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index 4b511b31ace..271fca8618e 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -1,6 +1,6 @@ import { isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { GasFees, GlobalVariables, MAX_L2_GAS_PER_ENQUEUED_CALL } from '@aztec/circuits.js'; -import { FunctionSelector } from '@aztec/foundation/abi'; +import { type FunctionArtifact, FunctionSelector } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; @@ -124,12 +124,17 @@ export function getAvmTestContractFunctionSelector(functionName: string): Functi return FunctionSelector.fromNameAndParameters(artifact.name, params); } -export function getAvmTestContractBytecode(functionName: string): Buffer { +export function getAvmTestContractArtifact(functionName: string): FunctionArtifact { const artifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!; assert( !!artifact?.bytecode, `No bytecode found for function ${functionName}. Try re-running bootstrap.sh on the repository root.`, ); + return artifact; +} + +export function getAvmTestContractBytecode(functionName: string): Buffer { + const artifact = getAvmTestContractArtifact(functionName); return artifact.bytecode; } @@ -138,12 +143,11 @@ export function resolveAvmTestContractAssertionMessage( revertReason: AvmRevertReason, output: Fr[], ): string | undefined { - const functionArtifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!; - traverseCauseChain(revertReason, cause => { revertReason = cause as AvmRevertReason; }); + const functionArtifact = AvmTestContractArtifact.functions.find(f => f.name === functionName); if (!functionArtifact || !revertReason.noirCallStack || !isNoirCallStackUnresolved(revertReason.noirCallStack)) { return undefined; } diff --git a/yarn-project/simulator/src/avm/opcodes/arithmetic.ts b/yarn-project/simulator/src/avm/opcodes/arithmetic.ts index 571c1b64ed5..30a1ead0efa 100644 --- a/yarn-project/simulator/src/avm/opcodes/arithmetic.ts +++ b/yarn-project/simulator/src/avm/opcodes/arithmetic.ts @@ -1,5 +1,6 @@ import type { AvmContext } from '../avm_context.js'; import { type Field, type MemoryValue } from '../avm_memory_types.js'; +import { ArithmeticError } from '../errors.js'; import { Opcode } from '../serialization/instruction_serialization.js'; import { Addressing } from './addressing_mode.js'; import { ThreeOperandInstruction } from './instruction_impl.js'; @@ -58,11 +59,14 @@ export class Div extends ThreeOperandArithmeticInstruction { static readonly opcode = Opcode.DIV_8; // FIXME: needed for gas. protected compute(a: MemoryValue, b: MemoryValue): MemoryValue { + if (b.toBigInt() === 0n) { + throw new ArithmeticError('Division by zero'); + } + return a.div(b); } } -// TODO: This class now temporarily has a tag, until all tags are removed. export class FieldDiv extends ThreeOperandArithmeticInstruction { static type: string = 'FDIV'; static readonly opcode = Opcode.FDIV_8; // FIXME: needed for gas. diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts index 58dd8152391..4bc38e78d2c 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -231,7 +231,9 @@ describe('External Calls', () => { argsSizeOffset, successOffset, ); - await expect(() => instruction.execute(context)).rejects.toThrow( + await instruction.execute(context); + // Ideally we'd mock the nested call. + expect(context.machineState.collectedRevertInfo?.recursiveRevertReason.message).toMatch( 'Static call cannot update the state, emit L2->L1 messages or generate logs', ); }); diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index d1200cdc491..7bf31a24a45 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -5,7 +5,6 @@ import { type AvmContractCallResult } from '../avm_contract_call_result.js'; import { gasLeftToGas } from '../avm_gas.js'; import { type Field, TypeTag, Uint1 } from '../avm_memory_types.js'; import { AvmSimulator } from '../avm_simulator.js'; -import { RethrownError } from '../errors.js'; import { Opcode, OperandType } from '../serialization/instruction_serialization.js'; import { Addressing } from './addressing_mode.js'; import { Instruction } from './instruction.js'; @@ -76,24 +75,22 @@ abstract class ExternalCall extends Instruction { const nestedCallResults: AvmContractCallResult = await simulator.execute(); const success = !nestedCallResults.reverted; - // TRANSITIONAL: We rethrow here so that the MESSAGE gets propagated. - // This means that for now, the caller cannot recover from errors. - if (!success) { - if (!nestedCallResults.revertReason) { - throw new Error('A reverted nested call should be assigned a revert reason in the AVM execution loop'); - } - // The nested call's revertReason will be used to track the stack of error causes down to the root. - throw new RethrownError( - nestedCallResults.revertReason.message, - nestedCallResults.revertReason, - nestedCallResults.output, - ); - } - // Save return/revert data for later. const fullReturnData = nestedCallResults.output; context.machineState.nestedReturndata = fullReturnData; + // If the nested call reverted, we try to save the reason and the revert data. + // This will be used by the caller to try to reconstruct the call stack. + // This is only a heuristic and may not always work. It is intended to work + // for the case where a nested call reverts and the caller always rethrows + // (in Noir code). + if (!success) { + context.machineState.collectedRevertInfo = { + revertDataRepresentative: fullReturnData, + recursiveRevertReason: nestedCallResults.revertReason!, + }; + } + // Write our success flag into memory. memory.set(successOffset, new Uint1(success ? 1 : 0)); diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index d27db358c56..0d117501df2 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -12,8 +12,8 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { Timer } from '@aztec/foundation/timer'; import { fromACVMField, witnessMapToFields } from '../acvm/deserialize.js'; -import { type ACVMWitness, Oracle, acvm, extractCallStack, resolveAssertionMessageFromError } from '../acvm/index.js'; -import { ExecutionError } from '../common/errors.js'; +import { type ACVMWitness, Oracle, acvm, extractCallStack } from '../acvm/index.js'; +import { ExecutionError, resolveAssertionMessageFromError } from '../common/errors.js'; import { type ClientExecutionContext } from './client_execution_context.js'; /** diff --git a/yarn-project/simulator/src/client/unconstrained_execution.ts b/yarn-project/simulator/src/client/unconstrained_execution.ts index f926b29bdca..70e488434db 100644 --- a/yarn-project/simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/simulator/src/client/unconstrained_execution.ts @@ -4,8 +4,8 @@ import { type Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { witnessMapToFields } from '../acvm/deserialize.js'; -import { Oracle, acvm, extractCallStack, resolveAssertionMessageFromError, toACVMWitness } from '../acvm/index.js'; -import { ExecutionError } from '../common/errors.js'; +import { Oracle, acvm, extractCallStack, toACVMWitness } from '../acvm/index.js'; +import { ExecutionError, resolveAssertionMessageFromError } from '../common/errors.js'; import { type ViewDataOracle } from './view_data_oracle.js'; // docs:start:execute_unconstrained_function diff --git a/yarn-project/simulator/src/common/errors.ts b/yarn-project/simulator/src/common/errors.ts index c02ec606a8d..f51536ac457 100644 --- a/yarn-project/simulator/src/common/errors.ts +++ b/yarn-project/simulator/src/common/errors.ts @@ -1,5 +1,14 @@ -import { type FailingFunction, type NoirCallStack, SimulationError } from '@aztec/circuit-types'; +import { + type FailingFunction, + type NoirCallStack, + SimulationError, + type SourceCodeLocation, +} from '@aztec/circuit-types'; import { type Fr } from '@aztec/circuits.js'; +import type { BrilligFunctionId, FunctionAbi, FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; + +import { type RawAssertionPayload } from '@noir-lang/acvm_js'; +import { abiDecodeError } from '@noir-lang/noirc_abi'; /** * An error that occurred during the execution of a function. @@ -65,3 +74,109 @@ export function createSimulationError(error: Error, revertData?: Fr[]): Simulati return new SimulationError(rootCause.message, aztecCallStack, revertData, noirCallStack, { cause: rootCause }); } + +/** + * Extracts a brillig location from an opcode location. + * @param opcodeLocation - The opcode location to extract from. It should be in the format `acirLocation.brilligLocation` or `acirLocation`. + * @returns The brillig location if the opcode location contains one. + */ +function extractBrilligLocation(opcodeLocation: string): string | undefined { + const splitted = opcodeLocation.split('.'); + if (splitted.length === 2) { + return splitted[1]; + } + return undefined; +} + +/** + * 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 getSourceCodeLocationsFromOpcodeLocation( + opcodeLocation: string, + debug: FunctionDebugMetadata, + brilligFunctionId?: BrilligFunctionId, +): SourceCodeLocation[] { + const { debugSymbols, files } = debug; + + let callStack = debugSymbols.locations[opcodeLocation] || []; + if (callStack.length === 0) { + const brilligLocation = extractBrilligLocation(opcodeLocation); + if (brilligFunctionId !== undefined && brilligLocation !== undefined) { + callStack = debugSymbols.brillig_locations[brilligFunctionId][brilligLocation] || []; + } + } + return callStack.map(call => { + const { file: fileId, span } = call; + + const { path, source } = files[fileId]; + + const locationText = source.substring(span.start, span.end); + const precedingText = source.substring(0, span.start); + 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, + }; + }); +} + +/** + * 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. + */ +export function resolveOpcodeLocations( + opcodeLocations: OpcodeLocation[], + debug: FunctionDebugMetadata, + brilligFunctionId?: BrilligFunctionId, +): SourceCodeLocation[] { + return opcodeLocations.flatMap(opcodeLocation => + getSourceCodeLocationsFromOpcodeLocation(opcodeLocation, debug, brilligFunctionId), + ); +} + +export function resolveAssertionMessage(errorPayload: RawAssertionPayload, abi: FunctionAbi): string | undefined { + const decoded = abiDecodeError( + { parameters: [], error_types: abi.errorTypes, return_type: null }, // eslint-disable-line camelcase + errorPayload, + ); + + if (typeof decoded === 'string') { + return decoded; + } else { + return JSON.stringify(decoded); + } +} + +export function resolveAssertionMessageFromRevertData(revertData: Fr[], abi: FunctionAbi): string | undefined { + if (revertData.length == 0) { + return undefined; + } + + const [errorSelector, ...errorData] = revertData; + + return resolveAssertionMessage( + { + selector: errorSelector.toBigInt().toString(), + data: errorData.map(f => f.toString()), + }, + abi, + ); +} + +export function resolveAssertionMessageFromError(err: Error, abi: FunctionAbi): string { + if (typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { + return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, abi)}`; + } else { + return err.message; + } +}