From f5bbb89b489bc85f286bcc5ed45c30f38032810c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Mon, 2 Sep 2024 16:39:06 +0200 Subject: [PATCH] feat!: Do not encode assertion strings in the programs (#8315) --- avm-transpiler/Cargo.lock | 16 +++++++++ avm-transpiler/Cargo.toml | 3 +- avm-transpiler/src/transpile.rs | 16 +++++++-- avm-transpiler/src/transpile_contract.rs | 14 ++++++-- avm-transpiler/src/utils.rs | 31 ++++++++++++++++- .../src/brillig/brillig_gen/brillig_block.rs | 4 +-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 4 +-- .../noirc_evaluator/src/ssa/ir/instruction.rs | 22 ++++++------ .../noirc_evaluator/src/ssa/ir/printer.rs | 4 +-- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 7 +++- .../bb-prover/src/avm_proving.test.ts | 4 ++- .../circuit-types/src/simulation_error.ts | 8 +++++ .../end-to-end/src/fixtures/fixtures.ts | 2 +- yarn-project/foundation/src/abi/abi.ts | 14 +++++++- .../pxe/src/pxe_service/pxe_service.ts | 29 ++++++++++------ yarn-project/simulator/src/acvm/acvm.ts | 22 ++++++++++++ .../simulator/src/avm/avm_simulator.test.ts | 28 +++++++++------ yarn-project/simulator/src/avm/errors.ts | 2 +- .../simulator/src/avm/fixtures/index.ts | 34 +++++++++++++++++-- .../types/src/abi/contract_artifact.ts | 1 + yarn-project/types/src/noir/index.ts | 2 ++ 21 files changed, 215 insertions(+), 52 deletions(-) diff --git a/avm-transpiler/Cargo.lock b/avm-transpiler/Cargo.lock index f0ab2d61e36..b404fc7bd84 100644 --- a/avm-transpiler/Cargo.lock +++ b/avm-transpiler/Cargo.lock @@ -304,6 +304,7 @@ dependencies = [ "base64 0.21.7", "env_logger", "flate2", + "fxhash", "log", "noirc_errors", "serde", @@ -405,6 +406,12 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "cc" version = "1.1.6" @@ -702,6 +709,15 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "fxhash" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" +dependencies = [ + "byteorder", +] + [[package]] name = "generic-array" version = "0.14.7" diff --git a/avm-transpiler/Cargo.toml b/avm-transpiler/Cargo.toml index d2cf5ce5ad1..d3d106e5037 100644 --- a/avm-transpiler/Cargo.toml +++ b/avm-transpiler/Cargo.toml @@ -9,8 +9,9 @@ license = "MIT OR Apache-2.0" [dependencies] # local -acvm = { path = "../noir/noir-repo/acvm-repo/acvm", features=["bn254"] } +acvm = { path = "../noir/noir-repo/acvm-repo/acvm", features = ["bn254"] } noirc_errors = { path = "../noir/noir-repo/compiler/noirc_errors" } +fxhash = "0.2.1" # external base64 = "0.21" diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index 3c8ae4f4240..db143469536 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -1,6 +1,6 @@ -use std::collections::BTreeMap; - use acvm::acir::brillig::{BitSize, IntegerBitSize, Opcode as BrilligOpcode}; +use fxhash::FxHashMap as HashMap; +use std::collections::BTreeMap; use acvm::acir::circuit::BrilligOpcodeLocation; use acvm::brillig_vm::brillig::{ @@ -1100,6 +1100,18 @@ pub fn patch_debug_info_pcs( debug_infos } +/// Patch the assert messages with updated PCs since transpilation injects extra +/// opcodes into the bytecode. +pub fn patch_assert_message_pcs( + assert_messages: HashMap, + brillig_pcs_to_avm_pcs: &[usize], +) -> HashMap { + assert_messages + .into_iter() + .map(|(brillig_pc, message)| (brillig_pcs_to_avm_pcs[brillig_pc], message)) + .collect() +} + /// Compute an array that maps each Brillig pc to an AVM pc. /// This must be done before transpiling to properly transpile jump destinations. /// This is necessary for two reasons: diff --git a/avm-transpiler/src/transpile_contract.rs b/avm-transpiler/src/transpile_contract.rs index 60da6a798b3..c1f836818b9 100644 --- a/avm-transpiler/src/transpile_contract.rs +++ b/avm-transpiler/src/transpile_contract.rs @@ -8,8 +8,11 @@ use serde::{Deserialize, Serialize}; use acvm::acir::circuit::Program; use noirc_errors::debug_info::ProgramDebugInfo; -use crate::transpile::{brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_debug_info_pcs}; -use crate::utils::extract_brillig_from_acir_program; +use crate::transpile::{ + brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_assert_message_pcs, patch_debug_info_pcs, +}; +use crate::utils::{extract_brillig_from_acir_program, extract_static_assert_messages}; +use fxhash::FxHashMap as HashMap; /// Representation of a contract with some transpiled functions #[derive(Debug, Serialize, Deserialize)] @@ -49,6 +52,7 @@ pub struct AvmContractFunctionArtifact { deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json" )] pub debug_symbols: ProgramDebugInfo, + pub assert_messages: HashMap, } /// Representation of an ACIR contract function but with @@ -93,10 +97,15 @@ impl From for TranspiledContractArtifact { // Extract Brillig Opcodes from acir let acir_program = function.bytecode; let brillig_bytecode = extract_brillig_from_acir_program(&acir_program); + let assert_messages = extract_static_assert_messages(&acir_program); // Map Brillig pcs to AVM pcs (index is Brillig PC, value is AVM PC) let brillig_pcs_to_avm_pcs = map_brillig_pcs_to_avm_pcs(brillig_bytecode); + // Patch the assert messages with updated PCs + let assert_messages = + patch_assert_message_pcs(assert_messages, &brillig_pcs_to_avm_pcs); + // Transpile to AVM let avm_bytecode = brillig_to_avm(brillig_bytecode, &brillig_pcs_to_avm_pcs); @@ -130,6 +139,7 @@ impl From for TranspiledContractArtifact { abi: function.abi, bytecode: base64::prelude::BASE64_STANDARD.encode(compressed_avm_bytecode), debug_symbols: ProgramDebugInfo { debug_infos }, + assert_messages, }, )); } else { diff --git a/avm-transpiler/src/utils.rs b/avm-transpiler/src/utils.rs index c49208ebea2..19f9468e15c 100644 --- a/avm-transpiler/src/utils.rs +++ b/avm-transpiler/src/utils.rs @@ -1,9 +1,11 @@ +use fxhash::FxHashMap as HashMap; + use acvm::acir::circuit::brillig::BrilligFunctionId; use acvm::FieldElement; use log::debug; use acvm::acir::brillig::Opcode as BrilligOpcode; -use acvm::acir::circuit::{Opcode, Program}; +use acvm::acir::circuit::{AssertionPayload, Opcode, Program}; use crate::instructions::AvmInstruction; @@ -36,6 +38,33 @@ pub fn extract_brillig_from_acir_program( &program.unconstrained_functions[0].bytecode } +/// Assertion messages that are static strings are stored in the assert_messages map of the ACIR program. +pub fn extract_static_assert_messages(program: &Program) -> HashMap { + assert_eq!( + program.functions.len(), + 1, + "An AVM program should have only a single ACIR function with a 'BrilligCall'" + ); + let main_function = &program.functions[0]; + main_function + .assert_messages + .iter() + .filter_map(|(location, payload)| { + if let AssertionPayload::StaticString(static_string) = payload { + Some(( + location + .to_brillig_location() + .expect("Assert message is not for the brillig function") + .0, + static_string.clone(), + )) + } else { + None + } + }) + .collect() +} + /// Print inputs, outputs, and instructions in a Brillig program pub fn dbg_print_brillig_program(brillig_bytecode: &[BrilligOpcode]) { debug!("Printing Brillig program..."); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index eeaa60b4323..a7cb1571e34 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -266,7 +266,7 @@ impl<'block> BrilligBlock<'block> { condition, ); match assert_message { - Some(ConstrainError::UserDefined(selector, values)) => { + Some(ConstrainError::Dynamic(selector, values)) => { let payload_values = vecmap(values, |value| self.convert_ssa_value(*value, dfg)); let payload_as_params = vecmap(values, |value| { @@ -280,7 +280,7 @@ impl<'block> BrilligBlock<'block> { selector.as_u64(), ); } - Some(ConstrainError::Intrinsic(message)) => { + Some(ConstrainError::StaticString(message)) => { self.brillig_context.codegen_constrain(condition, Some(message.clone())); } None => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 37ec43fb13b..768cbe27a28 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -647,10 +647,10 @@ impl<'a> Context<'a> { let assert_payload = if let Some(error) = assert_message { match error { - ConstrainError::Intrinsic(string) => { + ConstrainError::StaticString(string) => { Some(AssertionPayload::StaticString(string.clone())) } - ConstrainError::UserDefined(error_selector, values) => { + ConstrainError::Dynamic(error_selector, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload( *error_selector, values, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 36069f17933..e30707effed 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -468,12 +468,10 @@ impl Instruction { let lhs = f(*lhs); let rhs = f(*rhs); let assert_message = assert_message.as_ref().map(|error| match error { - ConstrainError::UserDefined(selector, payload_values) => { - ConstrainError::UserDefined( - *selector, - payload_values.iter().map(|&value| f(value)).collect(), - ) - } + ConstrainError::Dynamic(selector, payload_values) => ConstrainError::Dynamic( + *selector, + payload_values.iter().map(|&value| f(value)).collect(), + ), _ => error.clone(), }); Instruction::Constrain(lhs, rhs, assert_message) @@ -541,7 +539,7 @@ impl Instruction { Instruction::Constrain(lhs, rhs, assert_error) => { f(*lhs); f(*rhs); - if let Some(ConstrainError::UserDefined(_, values)) = assert_error.as_ref() { + if let Some(ConstrainError::Dynamic(_, values)) = assert_error.as_ref() { values.iter().for_each(|&val| { f(val); }); @@ -836,15 +834,15 @@ pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector { #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)] pub(crate) enum ConstrainError { - // These are errors which have been hardcoded during SSA gen - Intrinsic(String), - // These are errors issued by the user - UserDefined(ErrorSelector, Vec), + // Static string errors are not handled inside the program as data for efficiency reasons. + StaticString(String), + // These errors are handled by the program as data. + Dynamic(ErrorSelector, Vec), } impl From for ConstrainError { fn from(value: String) -> Self { - ConstrainError::Intrinsic(value) + ConstrainError::StaticString(value) } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index e8c9d01988e..2b564c14aa7 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -245,10 +245,10 @@ fn display_constrain_error( f: &mut Formatter, ) -> Result { match error { - ConstrainError::Intrinsic(assert_message_string) => { + ConstrainError::StaticString(assert_message_string) => { writeln!(f, " '{assert_message_string:?}'") } - ConstrainError::UserDefined(selector, values) => { + ConstrainError::Dynamic(selector, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload(*selector, values, &function.dfg) { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 6b19aff2674..2318fea8960 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -701,6 +701,11 @@ impl<'a> FunctionContext<'a> { assert_message: &Option>, ) -> Result, RuntimeError> { let Some(assert_message_payload) = assert_message else { return Ok(None) }; + + if let Expression::Literal(ast::Literal::Str(static_string)) = &assert_message_payload.0 { + return Ok(Some(ConstrainError::StaticString(static_string.clone()))); + } + let (assert_message_expression, assert_message_typ) = assert_message_payload.as_ref(); let values = self.codegen_expression(assert_message_expression)?.into_value_list(self); @@ -713,7 +718,7 @@ impl<'a> FunctionContext<'a> { self.builder.record_error_type(error_type_id, assert_message_typ.clone()); } }; - Ok(Some(ConstrainError::UserDefined(error_type_id, values))) + Ok(Some(ConstrainError::Dynamic(error_type_id, values))) } fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { diff --git a/yarn-project/bb-prover/src/avm_proving.test.ts b/yarn-project/bb-prover/src/avm_proving.test.ts index 4b3afb938b8..d48c1eb4859 100644 --- a/yarn-project/bb-prover/src/avm_proving.test.ts +++ b/yarn-project/bb-prover/src/avm_proving.test.ts @@ -39,6 +39,7 @@ import { initExecutionEnvironment, initHostStorage, initPersistableStateManager, + resolveAvmTestContractAssertionMessage, } from '@aztec/simulator/avm/fixtures'; import { jest } from '@jest/globals'; @@ -256,7 +257,8 @@ const proveAndVerifyAvmTestContract = async ( } else { // Explicit revert when an assertion failed. expect(avmResult.reverted).toBe(true); - expect(avmResult.revertReason?.message).toContain(assertionErrString); + expect(avmResult.revertReason).toBeDefined(); + expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!)).toContain(assertionErrString); } const pxResult = trace.toPublicExecutionResult( diff --git a/yarn-project/circuit-types/src/simulation_error.ts b/yarn-project/circuit-types/src/simulation_error.ts index aa0ccd1dfa6..8287da9bc00 100644 --- a/yarn-project/circuit-types/src/simulation_error.ts +++ b/yarn-project/circuit-types/src/simulation_error.ts @@ -108,6 +108,14 @@ export class SimulationError extends Error { return this.originalMessage; } + getOriginalMessage() { + return this.originalMessage; + } + + setOriginalMessage(message: string) { + this.originalMessage = message; + } + /** * Enriches the error with the name of a contract that failed. * @param contractAddress - The address of the contract diff --git a/yarn-project/end-to-end/src/fixtures/fixtures.ts b/yarn-project/end-to-end/src/fixtures/fixtures.ts index 1aa0c265a03..6470f015229 100644 --- a/yarn-project/end-to-end/src/fixtures/fixtures.ts +++ b/yarn-project/end-to-end/src/fixtures/fixtures.ts @@ -5,7 +5,7 @@ export const privateKey2 = Buffer.from('59c6995e998f97a5a0044966f0945389dc9e86da /// Common errors export const U128_UNDERFLOW_ERROR = "Assertion failed: attempt to subtract with underflow 'hi == high'"; export const U128_OVERFLOW_ERROR = "Assertion failed: attempt to add with overflow 'hi == high'"; -export const BITSIZE_TOO_BIG_ERROR = "Assertion failed. 'self.__assert_max_bit_size'"; +export const BITSIZE_TOO_BIG_ERROR = "Assertion failed: call to assert_max_bit_size 'self.__assert_max_bit_size'"; // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5818): Make these a fixed error after transition. export const DUPLICATE_NULLIFIER_ERROR = /dropped|duplicate nullifier|reverted/; export const NO_L1_TO_L2_MSG_ERROR = diff --git a/yarn-project/foundation/src/abi/abi.ts b/yarn-project/foundation/src/abi/abi.ts index 8e77fd0f482..a25a802dace 100644 --- a/yarn-project/foundation/src/abi/abi.ts +++ b/yarn-project/foundation/src/abi/abi.ts @@ -210,6 +210,10 @@ export interface FunctionArtifact extends FunctionAbi { verificationKey?: string; /** Maps opcodes to source code pointers */ debugSymbols: string; + /** + * Public functions store their static assertion messages externally to the bytecode. + */ + assertMessages?: Record; /** Debug metadata for the function. */ debug?: FunctionDebugMetadata; } @@ -369,6 +373,10 @@ export interface FunctionDebugMetadata { * Maps the file IDs to the file contents to resolve pointers */ files: DebugFileMap; + /** + * Public functions store their static assertion messages externally to the bytecode. + */ + assertMessages?: Record; } /** @@ -407,7 +415,11 @@ export function getFunctionDebugMetadata( // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5813) // We only support handling debug info for the contract function entry point. // So for now we simply index into the first debug info. - return { debugSymbols: programDebugSymbols.debug_infos[0], files: contractArtifact.fileMap }; + return { + debugSymbols: programDebugSymbols.debug_infos[0], + files: contractArtifact.fileMap, + assertMessages: functionArtifact.assertMessages, + }; } return undefined; } diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 1b058896756..a68ec01823b 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -66,6 +66,7 @@ import { collectSortedEncryptedLogs, collectSortedNoteEncryptedLogs, collectSortedUnencryptedLogs, + resolveAssertionMessage, resolveOpcodeLocations, } from '@aztec/simulator'; import { type ContractClassWithId, type ContractInstanceWithAddress } from '@aztec/types/contracts'; @@ -762,19 +763,25 @@ export class PXEService implements PXE { originalFailingFunction.functionSelector, ); const noirCallStack = err.getNoirCallStack(); - if (debugInfo && isNoirCallStackUnresolved(noirCallStack)) { - try { - // Public functions are simulated as a single Brillig entry point. - // Thus, we can safely assume here that the Brillig function id is `0`. - const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo, 0); - err.setNoirCallStack(parsedCallStack); - } catch (err) { - this.log.warn( - `Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${originalFailingFunction.functionSelector.toString()}: ${err}`, - ); + if (debugInfo) { + if (isNoirCallStackUnresolved(noirCallStack)) { + const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); + if (assertionMessage) { + err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); + } + try { + // Public functions are simulated as a single Brillig entry point. + // Thus, we can safely assume here that the Brillig function id is `0`. + const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo, 0); + err.setNoirCallStack(parsedCallStack); + } catch (err) { + this.log.warn( + `Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${originalFailingFunction.functionSelector.toString()}: ${err}`, + ); + } } + await this.#enrichSimulationError(err); } - await this.#enrichSimulationError(err); } throw err; diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index cbdb76222f4..ba7b3aa0536 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -103,6 +103,28 @@ export function resolveOpcodeLocations( ); } +/** + * 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 resolveAssertionMessage( + opcodeLocations: OpcodeLocation[], + debug: FunctionDebugMetadata, +): string | undefined { + if (opcodeLocations.length === 0) { + return undefined; + } + + const lastLocation = extractBrilligLocation(opcodeLocations[opcodeLocations.length - 1]); + if (!lastLocation) { + return undefined; + } + + return debug.assertMessages?.[parseInt(lastLocation, 10)]; +} + /** * The function call that executes an ACIR. */ diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 7118684483d..3eb40e1b731 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -28,6 +28,7 @@ import { randomMemoryBytes, randomMemoryFields, randomMemoryUint64s, + resolveAvmTestContractAssertionMessage, } from './fixtures/index.js'; import { type HostStorage } from './journal/host_storage.js'; import { type AvmPersistableStateManager } from './journal/journal.js'; @@ -178,16 +179,20 @@ describe('AVM simulator: transpiled Noir contracts', () => { const bytecode = getAvmTestContractBytecode('u128_addition_overflow'); const results = await new AvmSimulator(initContext()).executeBytecode(bytecode); expect(results.reverted).toBe(true); - expect(results.revertReason?.message).toEqual('Assertion failed: attempt to add with overflow'); + expect(results.revertReason).toBeDefined(); + expect(resolveAvmTestContractAssertionMessage('u128_addition_overflow', results.revertReason!)).toMatch( + 'attempt to add with overflow', + ); }); it('Expect failure on U128::from_integer() overflow', async () => { const bytecode = getAvmTestContractBytecode('u128_from_integer_overflow'); const results = await new AvmSimulator(initContext()).executeBytecode(bytecode); expect(results.reverted).toBe(true); - expect(results.revertReason?.message).toMatch('Assertion failed.'); - // Note: compiler intrinsic messages (like below) are not known to the AVM, they are recovered by the PXE. - // "Assertion failed: call to assert_max_bit_size 'self.__assert_max_bit_size(bit_size)'" + expect(results.revertReason).toBeDefined(); + expect(resolveAvmTestContractAssertionMessage('u128_from_integer_overflow', results.revertReason!)).toMatch( + 'call to assert_max_bit_size', + ); }); }); @@ -208,11 +213,11 @@ describe('AVM simulator: transpiled Noir contracts', () => { const results = await new AvmSimulator(context).executeBytecode(bytecode); expect(results.reverted).toBe(true); - expect(results.revertReason?.message).toEqual("Assertion failed: Nullifier doesn't exist!"); - expect(results.output).toEqual([ - new Fr(0), - ...[..."Nullifier doesn't exist!"].flatMap(c => new Fr(c.charCodeAt(0))), - ]); + expect(results.revertReason).toBeDefined(); + expect(resolveAvmTestContractAssertionMessage('assert_nullifier_exists', results.revertReason!)).toMatch( + "Nullifier doesn't exist!", + ); + expect(results.output).toEqual([]); }); describe.each([ @@ -886,7 +891,10 @@ describe('AVM simulator: transpiled Noir contracts', () => { const results = await new AvmSimulator(context).executeBytecode(callBytecode); expect(results.reverted).toBe(true); // The outer call should revert. - expect(results.revertReason?.message).toEqual('Assertion failed: Values are not equal'); + expect(results.revertReason).toBeDefined(); + expect(resolveAvmTestContractAssertionMessage('assert_same', results.revertReason!)).toMatch( + 'Values are not equal', + ); }); }); }); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 06d35a2abc9..2e0205a7e8a 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -151,7 +151,7 @@ export function revertReasonFromExplicitRevert(revertData: Fr[], context: AvmCon */ export function decodeRevertDataAsMessage(revertData: Fr[]): string { if (revertData.length === 0) { - return 'Assertion failed.'; + return 'Assertion failed'; } else { try { // We remove the first element which is the 'error selector'. diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index 6f2dd3c363d..72bc6d66a7f 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -1,5 +1,6 @@ +import { isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { GasFees, GlobalVariables, Header } from '@aztec/circuits.js'; -import { FunctionSelector } from '@aztec/foundation/abi'; +import { FunctionSelector, getFunctionDebugMetadata } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr } from '@aztec/foundation/fields'; @@ -9,12 +10,19 @@ import { strict as assert } from 'assert'; import { mock } from 'jest-mock-extended'; import merge from 'lodash.merge'; -import { type CommitmentsDB, type PublicContractsDB, type PublicStateDB } from '../../index.js'; +import { + type CommitmentsDB, + type PublicContractsDB, + type PublicStateDB, + resolveAssertionMessage, + traverseCauseChain, +} from '../../index.js'; import { type PublicSideEffectTraceInterface } from '../../public/side_effect_trace_interface.js'; import { AvmContext } from '../avm_context.js'; import { AvmContextInputs, AvmExecutionEnvironment } from '../avm_execution_environment.js'; import { AvmMachineState } from '../avm_machine_state.js'; import { Field, Uint8, Uint64 } from '../avm_memory_types.js'; +import { type AvmRevertReason } from '../errors.js'; import { HostStorage } from '../journal/host_storage.js'; import { AvmPersistableStateManager } from '../journal/journal.js'; import { NullifierManager } from '../journal/nullifiers.js'; @@ -152,3 +160,25 @@ export function getAvmTestContractBytecode(functionName: string): Buffer { ); return artifact.bytecode; } + +export function resolveAvmTestContractAssertionMessage( + functionName: string, + revertReason: AvmRevertReason, +): string | undefined { + const functionArtifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!; + + traverseCauseChain(revertReason, cause => { + revertReason = cause as AvmRevertReason; + }); + + if (!functionArtifact || !revertReason.noirCallStack || !isNoirCallStackUnresolved(revertReason.noirCallStack)) { + return undefined; + } + + const debugMetadata = getFunctionDebugMetadata(AvmTestContractArtifact, functionArtifact); + if (!debugMetadata) { + return undefined; + } + + return resolveAssertionMessage(revertReason.noirCallStack, debugMetadata); +} diff --git a/yarn-project/types/src/abi/contract_artifact.ts b/yarn-project/types/src/abi/contract_artifact.ts index e8cbbb3aabf..5756ab305a7 100644 --- a/yarn-project/types/src/abi/contract_artifact.ts +++ b/yarn-project/types/src/abi/contract_artifact.ts @@ -182,6 +182,7 @@ function generateFunctionArtifact(fn: NoirCompiledContractFunction, contract: No bytecode: Buffer.from(fn.bytecode, 'base64'), verificationKey: mockVerificationKey, debugSymbols: fn.debug_symbols, + assertMessages: fn.assert_messages, }; } diff --git a/yarn-project/types/src/noir/index.ts b/yarn-project/types/src/noir/index.ts index b56d32762a0..a35b0615372 100644 --- a/yarn-project/types/src/noir/index.ts +++ b/yarn-project/types/src/noir/index.ts @@ -64,6 +64,8 @@ export interface NoirFunctionEntry { verification_key?: string; /** The debug information, compressed and base64 encoded. */ debug_symbols: string; + /** Map opcode index to assert message for public functions */ + assert_messages?: Record; } /**