diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index 2cdb59f1b2a..6a26a45d88b 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -75,12 +75,6 @@ pub struct Circuit { pub recursive: bool, } -/// This selector indicates that the payload is a string. -/// This is used to parse any error with a string payload directly, -/// to avoid users having to parse the error externally to the ACVM. -/// Only non-string errors need to be parsed externally to the ACVM using the circuit ABI. -pub const STRING_ERROR_SELECTOR: u64 = 0; - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum ExpressionOrMemory { Expression(Expression), @@ -93,10 +87,55 @@ pub enum AssertionPayload { Dynamic(/* error_selector */ u64, Vec), } +#[derive(Debug, Copy, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] +pub struct ErrorSelector(u64); + +impl ErrorSelector { + pub fn new(integer: u64) -> Self { + ErrorSelector(integer) + } + + pub fn as_u64(&self) -> u64 { + self.0 + } +} + +impl Serialize for ErrorSelector { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.0.to_string().serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for ErrorSelector { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s: String = Deserialize::deserialize(deserializer)?; + let as_u64 = s.parse().map_err(serde::de::Error::custom)?; + Ok(ErrorSelector(as_u64)) + } +} + +/// This selector indicates that the payload is a string. +/// This is used to parse any error with a string payload directly, +/// to avoid users having to parse the error externally to the ACVM. +/// Only non-string errors need to be parsed externally to the ACVM using the circuit ABI. +pub const STRING_ERROR_SELECTOR: ErrorSelector = ErrorSelector(0); + +#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] +pub struct RawAssertionPayload { + pub selector: ErrorSelector, + pub data: Vec, +} + #[derive(Clone, PartialEq, Eq, Debug)] pub enum ResolvedAssertionPayload { String(String), - Raw(/*error_selector:*/ u64, Vec), + Raw(RawAssertionPayload), } #[derive(Debug, Copy, Clone)] diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs index 9cf87455acb..c911202c823 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs @@ -5,7 +5,8 @@ use acir::{ circuit::{ brillig::{BrilligInputs, BrilligOutputs}, opcodes::BlockId, - OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, + ErrorSelector, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, + STRING_ERROR_SELECTOR, }, native_types::WitnessMap, FieldElement, @@ -173,11 +174,13 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { let mut revert_values_iter = memory [revert_data_offset..(revert_data_offset + revert_data_size)] .iter(); - let error_selector = revert_values_iter - .next() - .expect("Incorrect revert data size") - .try_into() - .expect("Error selector is not u64"); + let error_selector = ErrorSelector::new( + revert_values_iter + .next() + .expect("Incorrect revert data size") + .try_into() + .expect("Error selector is not u64"), + ); match error_selector { STRING_ERROR_SELECTOR => { @@ -194,10 +197,12 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { } _ => { // If the error selector is not 0, it means the error is a custom error - Some(ResolvedAssertionPayload::Raw( - error_selector, - revert_values_iter.map(|value| value.to_field()).collect(), - )) + Some(ResolvedAssertionPayload::Raw(RawAssertionPayload { + selector: error_selector, + data: revert_values_iter + .map(|value| value.to_field()) + .collect(), + })) } } } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index 1f4867f7656..a4219adbfa6 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -5,8 +5,9 @@ use std::collections::HashMap; use acir::{ brillig::ForeignCallResult, circuit::{ - brillig::BrilligBytecode, opcodes::BlockId, AssertionPayload, ExpressionOrMemory, Opcode, - OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, + brillig::BrilligBytecode, opcodes::BlockId, AssertionPayload, ErrorSelector, + ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, + STRING_ERROR_SELECTOR, }, native_types::{Expression, Witness, WitnessMap}, BlackBoxFunc, FieldElement, @@ -425,8 +426,9 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } } } + let error_selector = ErrorSelector::new(*error_selector); - Some(match *error_selector { + Some(match error_selector { STRING_ERROR_SELECTOR => { // If the error selector is 0, it means the error is a string let string = fields @@ -444,7 +446,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } _ => { // If the error selector is not 0, it means the error is a custom error - ResolvedAssertionPayload::Raw(*error_selector, fields) + ResolvedAssertionPayload::Raw(RawAssertionPayload { + selector: error_selector, + data: fields, + }) } }) } diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs index 99d4b4ccb74..338511874c9 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs @@ -272,7 +272,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { }; // If the failed opcode has an assertion message, integrate it into the error message for backwards compatibility. // Otherwise, pass the raw assertion payload as is. - let (message, raw_assertion_payload) = match &error { + let (message, raw_assertion_payload) = match error { OpcodeResolutionError::UnsatisfiedConstrain { payload: Some(payload), .. @@ -281,8 +281,8 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { payload: Some(payload), .. } => match payload { - ResolvedAssertionPayload::Raw(selector, fields) => { - (error.to_string(), Some((*selector, fields.clone()))) + ResolvedAssertionPayload::Raw(raw_payload) => { + ("Assertion failed".to_string(), Some(raw_payload)) } ResolvedAssertionPayload::String(message) => { (format!("Assertion failed: {}", message), None) diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs b/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs index ae0e0aaa236..b34ea5ddb60 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs @@ -1,14 +1,13 @@ -use acvm::{acir::circuit::OpcodeLocation, FieldElement}; -use js_sys::{Array, Error, JsString, Map, Object, Reflect}; +use acvm::acir::circuit::{OpcodeLocation, RawAssertionPayload}; +use gloo_utils::format::JsValueSerdeExt; +use js_sys::{Array, Error, JsString, Reflect}; use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; -use crate::js_witness_map::field_element_to_js_string; - #[wasm_bindgen(typescript_custom_section)] const EXECUTION_ERROR: &'static str = r#" export type RawAssertionPayload = { - selector: number; - fields: string[]; + selector: string; + data: string[]; }; export type ExecutionError = Error & { callStack?: string[]; @@ -35,7 +34,7 @@ impl JsExecutionError { pub fn new( message: String, call_stack: Option>, - assertion_payload: Option<(u64, Vec)>, + assertion_payload: Option, ) -> Self { let mut error = JsExecutionError::constructor(JsString::from(message)); let js_call_stack = match call_stack { @@ -49,18 +48,8 @@ impl JsExecutionError { None => JsValue::UNDEFINED, }; let assertion_payload = match assertion_payload { - Some((selector, fields)) => { - let raw_payload_map = Map::new(); - raw_payload_map - .set(&JsValue::from_str("selector"), &JsValue::from(selector.to_string())); - let js_fields = Array::new(); - for field in fields { - js_fields.push(&field_element_to_js_string(&field)); - } - raw_payload_map.set(&JsValue::from_str("fields"), &js_fields.into()); - - Object::from_entries(&raw_payload_map).unwrap().into() - } + Some(raw) => ::from_serde(&raw) + .expect("Cannot serialize assertion payload"), None => JsValue::UNDEFINED, }; diff --git a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs index b1e52dc309d..609c88b92c2 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use acvm::acir::circuit::ErrorSelector; use acvm::acir::native_types::Witness; use iter_extended::{btree_map, vecmap}; use noirc_abi::{Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue}; @@ -20,7 +21,7 @@ pub(super) fn gen_abi( input_witnesses: Vec, return_witnesses: Vec, return_visibility: Visibility, - error_types: BTreeMap, + error_types: BTreeMap, ) -> Abi { let (parameters, return_type) = compute_function_abi(context, func_id); let param_witnesses = param_witnesses_from_abi_param(¶meters, input_witnesses); 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 e50316eabcc..2c9d43dc919 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 @@ -270,7 +270,7 @@ impl<'block> BrilligBlock<'block> { condition, payload_values, payload_as_params, - selector.to_u64(), + selector.as_u64(), ); } Some(ConstrainError::Intrinsic(message)) => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index 7f945f19784..7d571a2c3bc 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -12,7 +12,8 @@ use std::collections::{BTreeMap, BTreeSet}; use crate::errors::{RuntimeError, SsaReport}; use acvm::acir::{ circuit::{ - brillig::BrilligBytecode, Circuit, ExpressionWidth, Program as AcirProgram, PublicInputs, + brillig::BrilligBytecode, Circuit, ErrorSelector, ExpressionWidth, Program as AcirProgram, + PublicInputs, }, native_types::Witness, }; @@ -103,13 +104,13 @@ pub struct SsaProgramArtifact { pub main_input_witnesses: Vec, pub main_return_witnesses: Vec, pub names: Vec, - pub error_types: BTreeMap, + pub error_types: BTreeMap, } impl SsaProgramArtifact { fn new( unconstrained_functions: Vec, - error_types: BTreeMap, + error_types: BTreeMap, ) -> Self { let program = AcirProgram { functions: Vec::default(), unconstrained_functions }; Self { @@ -167,11 +168,6 @@ pub fn create_program( "The generated ACIRs should match the supplied function signatures" ); - let error_types = error_types - .into_iter() - .map(|(error_typ_id, error_typ)| (error_typ_id.to_u64(), error_typ)) - .collect(); - let mut program_artifact = SsaProgramArtifact::new(generated_brillig, error_types); // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; 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 ff94cd6757e..9ea5c5e4a96 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 @@ -9,7 +9,7 @@ use self::acir_ir::generated_acir::BrilligStdlibFunc; use super::function_builder::data_bus::DataBus; use super::ir::dfg::CallStack; use super::ir::function::FunctionId; -use super::ir::instruction::{ConstrainError, ErrorSelector, ErrorType}; +use super::ir::instruction::{ConstrainError, ErrorType}; use super::ir::printer::try_to_extract_string_from_error_payload; use super::{ ir::{ @@ -32,7 +32,7 @@ pub(crate) use acir_ir::generated_acir::GeneratedAcir; use noirc_frontend::monomorphization::ast::InlineType; use acvm::acir::circuit::brillig::BrilligBytecode; -use acvm::acir::circuit::{AssertionPayload, OpcodeLocation}; +use acvm::acir::circuit::{AssertionPayload, ErrorSelector, OpcodeLocation}; use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; use acvm::{ @@ -639,7 +639,7 @@ impl<'a> Context<'a> { self.acir_context.vars_to_expressions_or_memory(&acir_vars)?; Some(AssertionPayload::Dynamic( - error_selector.to_u64(), + error_selector.as_u64(), expressions_or_memory, )) } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 4b277ea244d..f5afbfae1bb 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -2,14 +2,14 @@ pub(crate) mod data_bus; use std::{borrow::Cow, collections::BTreeMap, rc::Rc}; -use acvm::FieldElement; +use acvm::{acir::circuit::ErrorSelector, FieldElement}; use noirc_errors::Location; use noirc_frontend::monomorphization::ast::InlineType; use crate::ssa::ir::{ basic_block::BasicBlockId, function::{Function, FunctionId}, - instruction::{Binary, BinaryOp, ErrorSelector, Instruction, TerminatorInstruction}, + instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction}, types::Type, value::{Value, ValueId}, }; 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 582e00b6be2..3084899f455 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 @@ -1,7 +1,10 @@ use std::hash::{Hash, Hasher}; use acvm::{ - acir::{circuit::STRING_ERROR_SELECTOR, BlackBoxFunc}, + acir::{ + circuit::{ErrorSelector, STRING_ERROR_SELECTOR}, + BlackBoxFunc, + }, FieldElement, }; use fxhash::FxHasher; @@ -605,26 +608,17 @@ impl Instruction { pub(crate) type ErrorType = HirType; -#[derive(Debug, Copy, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] -pub(crate) struct ErrorSelector(u64); - -impl ErrorSelector { - pub(crate) fn new(typ: &ErrorType) -> Self { - match typ { - ErrorType::String(_) => Self(STRING_ERROR_SELECTOR), - _ => { - let mut hasher = FxHasher::default(); - typ.hash(&mut hasher); - let hash = hasher.finish(); - assert!(hash != 0, "ICE: Error type {} collides with the string error type", typ); - Self(hash) - } +pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector { + match typ { + ErrorType::String(_) => STRING_ERROR_SELECTOR, + _ => { + let mut hasher = FxHasher::default(); + typ.hash(&mut hasher); + let hash = hasher.finish(); + assert!(hash != 0, "ICE: Error type {} collides with the string error type", typ); + ErrorSelector::new(hash) } } - - pub(crate) fn to_u64(self) -> u64 { - self.0 - } } #[derive(Debug, PartialEq, Eq, Hash, Clone)] 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 bbff3cf8acb..3e924985185 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 @@ -4,16 +4,14 @@ use std::{ fmt::{Formatter, Result}, }; -use acvm::acir::circuit::STRING_ERROR_SELECTOR; +use acvm::acir::circuit::{ErrorSelector, STRING_ERROR_SELECTOR}; use iter_extended::vecmap; use super::{ basic_block::BasicBlockId, dfg::DataFlowGraph, function::Function, - instruction::{ - ConstrainError, ErrorSelector, Instruction, InstructionId, TerminatorInstruction, - }, + instruction::{ConstrainError, Instruction, InstructionId, TerminatorInstruction}, value::{Value, ValueId}, }; @@ -203,7 +201,7 @@ pub(crate) fn try_to_extract_string_from_error_payload( values: &[ValueId], dfg: &DataFlowGraph, ) -> Option { - ((error_selector.to_u64() == STRING_ERROR_SELECTOR) && (values.len() == 1)) + ((error_selector == STRING_ERROR_SELECTOR) && (values.len() == 1)) .then_some(()) .and_then(|()| { let Value::Array { array: values, .. } = &dfg[values[0]] else { 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 c121ac19ff9..698a2b0471a 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 @@ -21,7 +21,7 @@ use self::{ value::{Tree, Values}, }; -use super::ir::instruction::ErrorSelector; +use super::ir::instruction::error_selector_from_type; use super::{ function_builder::data_bus::DataBus, ir::{ @@ -703,8 +703,7 @@ impl<'a> FunctionContext<'a> { let values = self.codegen_expression(assert_message_expression)?.into_value_list(self); - let error_type_id = ErrorSelector::new(assert_message_typ); - + let error_type_id = error_selector_from_type(assert_message_typ); // Do not record string errors in the ABI match assert_message_typ { HirType::String(_) => {} diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index 03d4ac05bd9..21178c55c73 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -1,10 +1,10 @@ use std::{collections::BTreeMap, fmt::Display}; +use acvm::acir::circuit::ErrorSelector; use iter_extended::btree_map; use crate::ssa::ir::{ function::{Function, FunctionId, RuntimeType}, - instruction::ErrorSelector, map::AtomicCounter, }; use noirc_frontend::hir_def::types::Type as HirType; diff --git a/noir/noir-repo/tooling/nargo/src/errors.rs b/noir/noir-repo/tooling/nargo/src/errors.rs index 4b0c29a9b1b..63a72247e2f 100644 --- a/noir/noir-repo/tooling/nargo/src/errors.rs +++ b/noir/noir-repo/tooling/nargo/src/errors.rs @@ -1,11 +1,13 @@ use std::collections::BTreeMap; use acvm::{ - acir::circuit::{OpcodeLocation, ResolvedAssertionPayload, ResolvedOpcodeLocation}, + acir::circuit::{ + ErrorSelector, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, + ResolvedOpcodeLocation, + }, pwg::{ErrorLocation, OpcodeResolutionError}, - FieldElement, }; -use noirc_abi::{Abi, AbiErrorType}; +use noirc_abi::{display_abi_error, Abi, AbiErrorType}; use noirc_errors::{ debug_info::DebugInfo, reporter::ReportedErrors, CustomDiagnostic, FileDiagnostic, }; @@ -13,9 +15,7 @@ use noirc_errors::{ pub use noirc_errors::Location; use noirc_frontend::graph::CrateName; -use noirc_printable_type::{ - decode_value, ForeignCallError, PrintableType, PrintableValue, PrintableValueDisplay, -}; +use noirc_printable_type::ForeignCallError; use thiserror::Error; /// Errors covering situations where a package cannot be compiled. @@ -61,7 +61,7 @@ impl NargoError { /// in tests to expected failure messages pub fn user_defined_failure_message( &self, - error_types: &BTreeMap, + error_types: &BTreeMap, ) -> Option { let execution_error = match self { NargoError::ExecutionError(error) => error, @@ -71,9 +71,9 @@ impl NargoError { match execution_error { ExecutionError::AssertionFailed(payload, _) => match payload { ResolvedAssertionPayload::String(message) => Some(message.to_string()), - ResolvedAssertionPayload::Raw(error_selector, fields) => { - let abi_type = error_types.get(error_selector)?; - let decoded = prepare_for_display(fields, abi_type.clone()); + ResolvedAssertionPayload::Raw(raw) => { + let abi_type = error_types.get(&raw.selector)?; + let decoded = display_abi_error(&raw.data, abi_type.clone()); Some(decoded.to_string()) } }, @@ -160,33 +160,8 @@ fn extract_locations_from_error( ) } -fn prepare_for_display(fields: &[FieldElement], error_type: AbiErrorType) -> PrintableValueDisplay { - match error_type { - AbiErrorType::FmtString { length, item_types } => { - let mut fields_iter = fields.iter().copied(); - let PrintableValue::String(string) = - decode_value(&mut fields_iter, &PrintableType::String { length }) - else { - unreachable!("Got non-string from string decoding"); - }; - let _length_of_items = fields_iter.next(); - let items = item_types.into_iter().map(|abi_type| { - let printable_typ = (&abi_type).into(); - let decoded = decode_value(&mut fields_iter, &printable_typ); - (decoded, printable_typ) - }); - PrintableValueDisplay::FmtString(string, items.collect()) - } - AbiErrorType::Custom(abi_typ) => { - let printable_type = (&abi_typ).into(); - let decoded = decode_value(&mut fields.iter().copied(), &printable_type); - PrintableValueDisplay::Plain(decoded, printable_type) - } - } -} - fn extract_message_from_error( - error_types: &BTreeMap, + error_types: &BTreeMap, nargo_err: &NargoError, ) -> String { match nargo_err { @@ -197,11 +172,11 @@ fn extract_message_from_error( format!("Assertion failed: '{message}'") } NargoError::ExecutionError(ExecutionError::AssertionFailed( - ResolvedAssertionPayload::Raw(error_selector, fields), + ResolvedAssertionPayload::Raw(RawAssertionPayload { selector, data }), .., )) => { - if let Some(error_type) = error_types.get(error_selector) { - format!("Assertion failed: {}", prepare_for_display(fields, error_type.clone())) + if let Some(error_type) = error_types.get(selector) { + format!("Assertion failed: {}", display_abi_error(data, error_type.clone())) } else { "Assertion failed".to_string() } diff --git a/noir/noir-repo/tooling/noir_js/scripts/compile_test_programs.sh b/noir/noir-repo/tooling/noir_js/scripts/compile_test_programs.sh index 5257aaae696..b9f12b5deb1 100755 --- a/noir/noir-repo/tooling/noir_js/scripts/compile_test_programs.sh +++ b/noir/noir-repo/tooling/noir_js/scripts/compile_test_programs.sh @@ -3,3 +3,4 @@ rm -rf ./test/noir_compiled_examples/**/target nargo --program-dir ./test/noir_compiled_examples/assert_lt compile --force nargo --program-dir ./test/noir_compiled_examples/assert_msg_runtime compile --force +nargo --program-dir ./test/noir_compiled_examples/assert_raw_payload compile --force diff --git a/noir/noir-repo/tooling/noir_js/src/index.ts b/noir/noir-repo/tooling/noir_js/src/index.ts index bacb391a464..e7f356a582e 100644 --- a/noir/noir-repo/tooling/noir_js/src/index.ts +++ b/noir/noir-repo/tooling/noir_js/src/index.ts @@ -16,6 +16,7 @@ export { InputMap } from '@noir-lang/noirc_abi'; export { WitnessMap, ForeignCallHandler, ForeignCallInput, ForeignCallOutput } from '@noir-lang/acvm_js'; export { Noir } from './program.js'; +export { ErrorWithPayload } from './witness_generation.js'; /** @ignore */ export { acvm, abi }; diff --git a/noir/noir-repo/tooling/noir_js/src/witness_generation.ts b/noir/noir-repo/tooling/noir_js/src/witness_generation.ts index 1f233422061..a22b0687040 100644 --- a/noir/noir-repo/tooling/noir_js/src/witness_generation.ts +++ b/noir/noir-repo/tooling/noir_js/src/witness_generation.ts @@ -1,4 +1,4 @@ -import { abiEncode, InputMap } from '@noir-lang/noirc_abi'; +import { abiDecodeError, abiEncode, InputMap } from '@noir-lang/noirc_abi'; import { base64Decode } from './base64_decode.js'; import { WitnessMap, @@ -7,8 +7,9 @@ import { createBlackBoxSolver, WasmBlackBoxFunctionSolver, executeCircuitWithBlackBoxSolver, + ExecutionError, } from '@noir-lang/acvm_js'; -import { CompiledCircuit } from '@noir-lang/types'; +import { Abi, CompiledCircuit } from '@noir-lang/types'; let solver: Promise; @@ -30,6 +31,33 @@ const defaultForeignCallHandler: ForeignCallHandler = async (name: string, args: throw Error(`Unexpected oracle during execution: ${name}(${args.join(', ')})`); }; +// Payload is any since it can be of any type defined by the circuit dev. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type ErrorWithPayload = ExecutionError & { decodedAssertionPayload?: any }; + +function parseErrorPayload(abi: Abi, originalError: ExecutionError): Error { + const payload = originalError.rawAssertionPayload; + if (!payload) return originalError; + const enrichedError = originalError as ErrorWithPayload; + + try { + // Decode the payload + const decodedPayload = abiDecodeError(abi, payload); + + if (typeof decodedPayload === 'string') { + // If it's a string, just add it to the error message + enrichedError.message = `Circuit execution failed: ${decodedPayload}`; + } else { + // If not, attach the payload to the original error + enrichedError.decodedAssertionPayload = decodedPayload; + } + } catch (_errorDecoding) { + // Ignore errors decoding the payload + } + + return enrichedError; +} + // Generates the witnesses needed to feed into the chosen proving system export async function generateWitness( compiledProgram: CompiledCircuit, @@ -50,6 +78,10 @@ export async function generateWitness( ); return solvedWitness; } catch (err) { + // Typescript types catched errors as unknown or any, so we need to narrow its type to check if it has raw assertion payload. + if (typeof err === 'object' && err !== null && 'rawAssertionPayload' in err) { + throw parseErrorPayload(compiledProgram.abi, err as ExecutionError); + } throw new Error(`Circuit execution failed: ${err}`); } } diff --git a/noir/noir-repo/tooling/noir_js/test/node/execute.test.ts b/noir/noir-repo/tooling/noir_js/test/node/execute.test.ts index 491bcb0dfc4..b7aa4f3135c 100644 --- a/noir/noir-repo/tooling/noir_js/test/node/execute.test.ts +++ b/noir/noir-repo/tooling/noir_js/test/node/execute.test.ts @@ -1,6 +1,8 @@ import assert_lt_json from '../noir_compiled_examples/assert_lt/target/assert_lt.json' assert { type: 'json' }; import assert_msg_json from '../noir_compiled_examples/assert_msg_runtime/target/assert_msg_runtime.json' assert { type: 'json' }; -import { Noir } from '@noir-lang/noir_js'; +import assert_raw_payload_json from '../noir_compiled_examples/assert_raw_payload/target/assert_raw_payload.json' assert { type: 'json' }; + +import { Noir, ErrorWithPayload } from '@noir-lang/noir_js'; import { CompiledCircuit } from '@noir-lang/types'; import { expect } from 'chai'; @@ -17,7 +19,7 @@ it('returns the return value of the circuit', async () => { expect(returnValue).to.be.eq('0x05'); }); -it('circuit with a dynamic assert message should fail on an assert failure not the foreign call handler', async () => { +it('circuit with a fmt string assert message should fail with the resolved assertion message', async () => { const inputs = { x: '10', y: '5', @@ -26,6 +28,24 @@ it('circuit with a dynamic assert message should fail on an assert failure not t await new Noir(assert_msg_runtime).execute(inputs); } catch (error) { const knownError = error as Error; - expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); + expect(knownError.message).to.equal('Circuit execution failed: Expected x < y but got 10 < 5'); + } +}); + +it('circuit with a raw assert payload should fail with the decoded payload', async () => { + const inputs = { + x: '7', + y: '5', + }; + try { + await new Noir(assert_raw_payload_json).execute(inputs); + } catch (error) { + const knownError = error as ErrorWithPayload; + const invalidXYErrorSelector = Object.keys(assert_raw_payload_json.abi.error_types)[0]; + expect(knownError.rawAssertionPayload!.selector).to.equal(invalidXYErrorSelector); + expect(knownError.decodedAssertionPayload).to.deep.equal({ + x: '0x07', + y: '0x05', + }); } }); diff --git a/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_msg_runtime/src/main.nr b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_msg_runtime/src/main.nr index 40e447cad02..0bcd5c58c24 100644 --- a/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_msg_runtime/src/main.nr +++ b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_msg_runtime/src/main.nr @@ -1,6 +1,4 @@ fn main(x: u64, y: pub u64) { - // A dynamic assertion message is used to show that noirJS will ignore the call and continue execution - // We need this assertion to fail as the `assert_message` oracle in Noir is only called - // upon a failing condition in an assert. + // A fmtstr assertion message is used to show that noirJS will decode the error payload as a string. assert(x < y, f"Expected x < y but got {x} < {y}"); } diff --git a/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/Nargo.toml b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/Nargo.toml new file mode 100644 index 00000000000..f88832264ea --- /dev/null +++ b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "assert_raw_payload" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/src/main.nr b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/src/main.nr new file mode 100644 index 00000000000..4c981330b31 --- /dev/null +++ b/noir/noir-repo/tooling/noir_js/test/noir_compiled_examples/assert_raw_payload/src/main.nr @@ -0,0 +1,9 @@ +struct InvalidXYError { + x: u64, + y: u64, +} + +fn main(x: u64, y: pub u64) { + // A raw assertion payload is used to show that noirJS will decode the error payload as a struct. + assert(x < y, InvalidXYError { x, y }); +} diff --git a/noir/noir-repo/tooling/noir_js_types/src/types.ts b/noir/noir-repo/tooling/noir_js_types/src/types.ts index 664a7c2a457..0258f2f90c9 100644 --- a/noir/noir-repo/tooling/noir_js_types/src/types.ts +++ b/noir/noir-repo/tooling/noir_js_types/src/types.ts @@ -27,6 +27,12 @@ export type AbiErrorType = } | ({ error_kind: 'custom' } & AbiType); +// The payload for a raw assertion error returned on execution. +export type RawAssertionPayload = { + selector: string; + data: string[]; +}; + // Map from witness index to hex string value of witness. export type WitnessMap = Map; diff --git a/noir/noir-repo/tooling/noirc_abi/src/lib.rs b/noir/noir-repo/tooling/noirc_abi/src/lib.rs index 48c88833bdf..35f85b5f59c 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/lib.rs @@ -4,7 +4,10 @@ #![warn(clippy::semicolon_if_nothing_returned)] use acvm::{ - acir::native_types::{Witness, WitnessMap}, + acir::{ + circuit::ErrorSelector, + native_types::{Witness, WitnessMap}, + }, FieldElement, }; use errors::AbiError; @@ -12,7 +15,10 @@ use input_parser::InputValue; use iter_extended::{try_btree_map, try_vecmap, vecmap}; use noirc_frontend::ast::{Signedness, Visibility}; use noirc_frontend::{hir::Context, Type, TypeBinding, TypeVariableKind}; -use noirc_printable_type::PrintableType; +use noirc_printable_type::{ + decode_value as printable_type_decode_value, PrintableType, PrintableValue, + PrintableValueDisplay, +}; use serde::{Deserialize, Serialize}; use std::{borrow::Borrow, ops::Range}; use std::{collections::BTreeMap, str}; @@ -266,7 +272,7 @@ pub struct Abi { pub param_witnesses: BTreeMap>>, pub return_type: Option, pub return_witnesses: Vec, - pub error_types: BTreeMap, + pub error_types: BTreeMap, } impl Abi { @@ -480,7 +486,7 @@ impl Abi { } } -fn decode_value( +pub fn decode_value( field_iterator: &mut impl Iterator, value_type: &AbiType, ) -> Result { @@ -606,6 +612,34 @@ impl AbiErrorType { } } +pub fn display_abi_error( + fields: &[FieldElement], + error_type: AbiErrorType, +) -> PrintableValueDisplay { + match error_type { + AbiErrorType::FmtString { length, item_types } => { + let mut fields_iter = fields.iter().copied(); + let PrintableValue::String(string) = + printable_type_decode_value(&mut fields_iter, &PrintableType::String { length }) + else { + unreachable!("Got non-string from string decoding"); + }; + let _length_of_items = fields_iter.next(); + let items = item_types.into_iter().map(|abi_type| { + let printable_typ = (&abi_type).into(); + let decoded = printable_type_decode_value(&mut fields_iter, &printable_typ); + (decoded, printable_typ) + }); + PrintableValueDisplay::FmtString(string, items.collect()) + } + AbiErrorType::Custom(abi_typ) => { + let printable_type = (&abi_typ).into(); + let decoded = printable_type_decode_value(&mut fields.iter().copied(), &printable_type); + PrintableValueDisplay::Plain(decoded, printable_type) + } + } +} + #[cfg(test)] mod test { use std::collections::BTreeMap; diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs b/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs index fad5abaebba..10c0c43b352 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs @@ -5,12 +5,16 @@ // See Cargo.toml for explanation. use getrandom as _; -use acvm::acir::native_types::{WitnessMap, WitnessStack}; +use acvm::acir::{ + circuit::RawAssertionPayload, + native_types::{WitnessMap, WitnessStack}, +}; use iter_extended::try_btree_map; use noirc_abi::{ + decode_value, display_abi_error, errors::InputParserError, input_parser::{json::JsonTypes, InputValue}, - Abi, MAIN_RETURN_NAME, + Abi, AbiErrorType, MAIN_RETURN_NAME, }; use serde::Serialize; use std::collections::BTreeMap; @@ -26,8 +30,8 @@ use js_witness_map::JsWitnessMap; #[wasm_bindgen(typescript_custom_section)] const INPUT_MAP: &'static str = r#" -import { Field, InputValue, InputMap, Visibility, Sign, AbiType, AbiParameter, Abi, WitnessMap } from "@noir-lang/types"; -export { Field, InputValue, InputMap, Visibility, Sign, AbiType, AbiParameter, Abi, WitnessMap } from "@noir-lang/types"; +import { Field, InputValue, InputMap, Visibility, Sign, AbiType, AbiParameter, Abi, WitnessMap, RawAssertionPayload } from "@noir-lang/types"; +export { Field, InputValue, InputMap, Visibility, Sign, AbiType, AbiParameter, Abi, WitnessMap, RawAssertionPayload } from "@noir-lang/types"; "#; #[wasm_bindgen] @@ -40,6 +44,10 @@ extern "C" { #[derive(Clone, Debug, PartialEq, Eq)] pub type JsInputValue; + #[wasm_bindgen(extends = js_sys::Object, js_name = "RawAssertionPayload", typescript_type = "RawAssertionPayload")] + #[derive(Clone, Debug, PartialEq, Eq)] + pub type JsRawAssertionPayload; + #[wasm_bindgen(extends = js_sys::Object, js_name = "Abi", typescript_type = "Abi")] #[derive(Clone, Debug, PartialEq, Eq)] pub type JsAbi; @@ -122,3 +130,30 @@ pub fn serialise_witness(witness_map: JsWitnessMap) -> Result, JsAbiErro let output = witness_stack.try_into(); output.map_err(|_| JsAbiError::new("Failed to convert to Vec".to_string())) } + +#[wasm_bindgen(js_name = abiDecodeError)] +pub fn abi_decode_error( + abi: JsAbi, + raw_error: JsRawAssertionPayload, +) -> Result { + console_error_panic_hook::set_once(); + let mut abi: Abi = + JsValueSerdeExt::into_serde(&JsValue::from(abi)).map_err(|err| err.to_string())?; + + let raw_error: RawAssertionPayload = + JsValueSerdeExt::into_serde(&JsValue::from(raw_error)).map_err(|err| err.to_string())?; + + let error_type = abi.error_types.remove(&raw_error.selector).expect("Missing error type"); + match error_type { + AbiErrorType::FmtString { .. } => { + let string = display_abi_error(&raw_error.data, error_type).to_string(); + Ok(JsValue::from_str(&string)) + } + AbiErrorType::Custom(typ) => { + let input_value = decode_value(&mut raw_error.data.into_iter(), &typ)?; + let json_types = JsonTypes::try_from_input_value(&input_value, &typ)?; + ::from_serde(&json_types) + .map_err(|err| err.to_string().into()) + } + } +} diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/browser/decode_error.test.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/browser/decode_error.test.ts new file mode 100644 index 00000000000..2407ddc6535 --- /dev/null +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/browser/decode_error.test.ts @@ -0,0 +1,64 @@ +import { expect } from '@esm-bundle/chai'; +import initNoirAbi, { RawAssertionPayload, abiDecodeError } from '@noir-lang/noirc_abi'; + +beforeEach(async () => { + await initNoirAbi(); +}); + +it('Recovers custom field errors', async () => { + const { FAKE_FIELD_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_FIELD_SELECTOR, + data: ['0x000001'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.equal('0x01'); +}); + +it('Recovers custom tuple errors', async () => { + const { FAKE_TUPLE_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_TUPLE_SELECTOR, + data: ['0x000001', '0x000002'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.deep.equal(['0x01', '0x02']); +}); + +it('Recovers custom fmt string errors', async () => { + const { FAKE_FMT_STRING_SELECTOR, abi, SAMPLE_FMT_STRING } = await import('../shared/decode_error'); + + // FmtStrings contain the string serialized to fields + const data = [...SAMPLE_FMT_STRING].map((c) => `0x${c.charCodeAt(0).toString(16)}`); + // Then they contain the length of the values to replace + data.push('0x01'); + // And then the value to replace + data.push('0x07'); + + const payload: RawAssertionPayload = { + selector: FAKE_FMT_STRING_SELECTOR, + data, + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.equal('hello 0x07'); +}); + +it('Recovers struct errors', async () => { + const { FAKE_STRUCT_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_STRUCT_SELECTOR, + data: ['0x01', '0x02'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.deep.equal({ + a: '0x01', + b: '0x02', + }); +}); diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/node/decode_error.test.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/node/decode_error.test.ts new file mode 100644 index 00000000000..711653ad077 --- /dev/null +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/node/decode_error.test.ts @@ -0,0 +1,60 @@ +import { expect } from 'chai'; +import { RawAssertionPayload, abiDecodeError } from '@noir-lang/noirc_abi'; + +it('Recovers custom field errors', async () => { + const { FAKE_FIELD_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_FIELD_SELECTOR, + data: ['0x000001'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.equal('0x01'); +}); + +it('Recovers custom tuple errors', async () => { + const { FAKE_TUPLE_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_TUPLE_SELECTOR, + data: ['0x000001', '0x000002'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.deep.equal(['0x01', '0x02']); +}); + +it('Recovers custom fmt string errors', async () => { + const { FAKE_FMT_STRING_SELECTOR, abi, SAMPLE_FMT_STRING } = await import('../shared/decode_error'); + + // FmtStrings contain the string serialized to fields + const data = [...SAMPLE_FMT_STRING].map((c) => `0x${c.charCodeAt(0).toString(16)}`); + // Then they contain the length of the values to replace + data.push('0x01'); + // And then the value to replace + data.push('0x07'); + + const payload: RawAssertionPayload = { + selector: FAKE_FMT_STRING_SELECTOR, + data, + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.equal('hello 0x07'); +}); + +it('Recovers struct errors', async () => { + const { FAKE_STRUCT_SELECTOR, abi } = await import('../shared/decode_error'); + + const payload: RawAssertionPayload = { + selector: FAKE_STRUCT_SELECTOR, + data: ['0x01', '0x02'], + }; + + const decoded = abiDecodeError(abi, payload); + expect(decoded).to.deep.equal({ + a: '0x01', + b: '0x02', + }); +}); diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts new file mode 100644 index 00000000000..36eb18b5210 --- /dev/null +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts @@ -0,0 +1,46 @@ +import { Abi } from '@noir-lang/noirc_abi'; + +export const FAKE_FIELD_SELECTOR = '1'; +export const FAKE_TUPLE_SELECTOR = '2'; +export const FAKE_FMT_STRING_SELECTOR = '3'; +export const FAKE_STRUCT_SELECTOR = '4'; + +export const SAMPLE_FMT_STRING = 'hello {a}'; + +export const abi: Abi = { + parameters: [ + { + name: 'foo', + type: { kind: 'array', length: 2, type: { kind: 'field' } }, + visibility: 'private', + }, + ], + param_witnesses: { foo: [{ start: 1, end: 3 }] }, + return_type: null, + return_witnesses: [], + error_types: { + [FAKE_FIELD_SELECTOR]: { + error_kind: 'custom', + kind: 'field', + }, + [FAKE_TUPLE_SELECTOR]: { + error_kind: 'custom', + kind: 'tuple', + fields: [{ kind: 'field' }, { kind: 'field' }], + }, + [FAKE_FMT_STRING_SELECTOR]: { + error_kind: 'fmtstring', + length: SAMPLE_FMT_STRING.length, + item_types: [{ kind: 'field' }], + }, + [FAKE_STRUCT_SELECTOR]: { + error_kind: 'custom', + kind: 'struct', + path: 'foo', + fields: [ + { name: 'a', type: { kind: 'field' } }, + { name: 'b', type: { kind: 'field' } }, + ], + }, + }, +};