From e3841085203c145ceb6e85bf27ea52f63d2729d1 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 31 Aug 2023 16:59:09 +0000 Subject: [PATCH 1/7] feat: brillig errors with callstacks --- acvm/src/pwg/brillig.rs | 19 +++++++++++++--- acvm/src/pwg/mod.rs | 29 ++++++++--------------- acvm/tests/solver.rs | 9 +++----- acvm_js/src/execute.rs | 29 ++++++++++++++++------- acvm_js/src/js_execution_error.rs | 38 +++++++++++++++++++++++++++++++ acvm_js/src/lib.rs | 3 ++- brillig_vm/src/lib.rs | 15 ++++++++---- 7 files changed, 101 insertions(+), 41 deletions(-) create mode 100644 acvm_js/src/js_execution_error.rs diff --git a/acvm/src/pwg/brillig.rs b/acvm/src/pwg/brillig.rs index 85c102583..56bc406f5 100644 --- a/acvm/src/pwg/brillig.rs +++ b/acvm/src/pwg/brillig.rs @@ -1,6 +1,9 @@ use acir::{ brillig::{RegisterIndex, Value}, - circuit::brillig::{Brillig, BrilligInputs, BrilligOutputs}, + circuit::{ + brillig::{Brillig, BrilligInputs, BrilligOutputs}, + OpcodeLocation, + }, native_types::WitnessMap, FieldElement, }; @@ -18,6 +21,7 @@ impl BrilligSolver { initial_witness: &mut WitnessMap, brillig: &Brillig, bb_solver: &B, + acir_index: usize, ) -> Result, OpcodeResolutionError> { // If the predicate is `None`, then we simply return the value 1 // If the predicate is `Some` but we cannot find a value, then we return stalled @@ -107,8 +111,17 @@ impl BrilligSolver { Ok(None) } VMStatus::InProgress => unreachable!("Brillig VM has not completed execution"), - VMStatus::Failure { message } => { - Err(OpcodeResolutionError::BrilligFunctionFailed(message, vm.program_counter())) + VMStatus::Failure { message, call_stack } => { + Err(OpcodeResolutionError::BrilligFunctionFailed { + message, + call_stack: call_stack + .iter() + .map(|brillig_index| OpcodeLocation::Brillig { + acir_index, + brillig_index: *brillig_index, + }) + .collect(), + }) } VMStatus::ForeignCallWait { function, inputs } => { Ok(Some(ForeignCallWaitInfo { function, inputs })) diff --git a/acvm/src/pwg/mod.rs b/acvm/src/pwg/mod.rs index 8a39311b3..add165889 100644 --- a/acvm/src/pwg/mod.rs +++ b/acvm/src/pwg/mod.rs @@ -103,14 +103,14 @@ pub enum OpcodeResolutionError { OpcodeNotSolvable(#[from] OpcodeNotSolvable), #[error("backend does not currently support the {0} opcode. ACVM does not currently have a fallback for this opcode.")] UnsupportedBlackBoxFunc(BlackBoxFunc), - #[error("Cannot satisfy constraint {opcode_location}")] + #[error("Cannot satisfy constraint")] UnsatisfiedConstrain { opcode_location: ErrorLocation }, - #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?} at {opcode_location}")] + #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")] IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 }, #[error("failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), - #[error("failed to solve brillig function, reason: {0} at index {1}")] - BrilligFunctionFailed(String, usize), + #[error("Failed to solve brillig function, reason: {message}")] + BrilligFunctionFailed { message: String, call_stack: Vec }, } impl From for OpcodeResolutionError { @@ -258,7 +258,12 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> { solver.solve_memory_op(op, &mut self.witness_map, predicate) } Opcode::Brillig(brillig) => { - match BrilligSolver::solve(&mut self.witness_map, brillig, self.backend) { + match BrilligSolver::solve( + &mut self.witness_map, + brillig, + self.backend, + self.instruction_pointer, + ) { Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), res => res.map(|_| ()), } @@ -289,20 +294,6 @@ impl<'backend, B: BlackBoxFunctionSolver> ACVM<'backend, B> { self.instruction_pointer(), )); } - // If a brillig function has failed, we return an unsatisfied constraint error - // We intentionally ignore the brillig failure message, as there is no way to - // propagate this to the caller. - OpcodeResolutionError::BrilligFunctionFailed( - _, - brillig_instruction_pointer, - ) => { - error = OpcodeResolutionError::UnsatisfiedConstrain { - opcode_location: ErrorLocation::Resolved(OpcodeLocation::Brillig { - acir_index: self.instruction_pointer(), - brillig_index: *brillig_instruction_pointer, - }), - } - } // All other errors are thrown normally. _ => (), }; diff --git a/acvm/tests/solver.rs b/acvm/tests/solver.rs index a888a062d..ce13ff268 100644 --- a/acvm/tests/solver.rs +++ b/acvm/tests/solver.rs @@ -4,7 +4,6 @@ use acir::{ brillig::{BinaryFieldOp, Opcode as BrilligOpcode, RegisterIndex, RegisterOrMemory, Value}, circuit::{ brillig::{Brillig, BrilligInputs, BrilligOutputs}, - directives::Directive, opcodes::{BlockId, MemOp}, Opcode, OpcodeLocation, }, @@ -599,11 +598,9 @@ fn unsatisfied_opcode_resolved_brillig() { let solver_status = acvm.solve(); assert_eq!( solver_status, - ACVMStatus::Failure(OpcodeResolutionError::UnsatisfiedConstrain { - opcode_location: ErrorLocation::Resolved(OpcodeLocation::Brillig { - acir_index: 0, - brillig_index: 2 - }), + ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed { + message: "explicit trap hit in brillig".to_string(), + call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 2 }] }), "The first opcode is not satisfiable, expected an error indicating this" ); diff --git a/acvm_js/src/execute.rs b/acvm_js/src/execute.rs index 9dda594e4..3ca2fd120 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -9,7 +9,7 @@ use wasm_bindgen::prelude::wasm_bindgen; use crate::{ foreign_call::{resolve_brillig, ForeignCallHandler}, - JsWitnessMap, + JsExecutionError, JsWitnessMap, }; #[wasm_bindgen] @@ -39,7 +39,7 @@ pub async fn execute_circuit( circuit: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); let solver = WasmBlackBoxFunctionSolver::initialize().await; @@ -61,7 +61,7 @@ pub async fn execute_circuit_with_black_box_solver( circuit: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); let circuit: Circuit = Circuit::read(&*circuit).expect("Failed to deserialize circuit"); @@ -76,15 +76,26 @@ pub async fn execute_circuit_with_black_box_solver( unreachable!("Execution should not stop while in `InProgress` state.") } ACVMStatus::Failure(error) => { - let assert_message = match &error { + let (assert_message, call_stack) = match &error { OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Resolved(opcode_location), } | OpcodeResolutionError::IndexOutOfBounds { opcode_location: ErrorLocation::Resolved(opcode_location), .. - } => circuit.assert_messages.get(opcode_location).cloned(), - _ => None, + } => ( + circuit.assert_messages.get(opcode_location).cloned(), + Some(vec![*opcode_location]), + ), + OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { + let failing_opcode = + call_stack.last().expect("Brillig error call stacks cannot be empty"); + ( + circuit.assert_messages.get(failing_opcode).cloned(), + Some(call_stack.clone()), + ) + } + _ => (None, None), }; let error_string = match assert_message { @@ -92,10 +103,12 @@ pub async fn execute_circuit_with_black_box_solver( None => error.to_string(), }; - return Err(error_string.into()); + return Err(JsExecutionError::create(error_string, call_stack)); } ACVMStatus::RequiresForeignCall(foreign_call) => { - let result = resolve_brillig(&foreign_call_handler, &foreign_call).await?; + let result = resolve_brillig(&foreign_call_handler, &foreign_call) + .await + .map_err(|err| JsExecutionError::create(err, None))?; acvm.resolve_pending_foreign_call(result); } diff --git a/acvm_js/src/js_execution_error.rs b/acvm_js/src/js_execution_error.rs new file mode 100644 index 000000000..c9ec30a71 --- /dev/null +++ b/acvm_js/src/js_execution_error.rs @@ -0,0 +1,38 @@ +use gloo_utils::format::JsValueSerdeExt; +use js_sys::{Error, JsString}; +use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; + +use acvm::acir::circuit::OpcodeLocation; + +#[wasm_bindgen(typescript_custom_section)] +const EXECUTION_ERROR: &'static str = r#" +export class ExecutionError extends Error { + constructor(message: string, private callStack?: string[]) { + super(message); + } +} +"#; + +// ExecutionError +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(extends = Error, js_name = "ExecutionError", typescript_type = "ExecutionError")] + #[derive(Clone, Debug, PartialEq, Eq)] + pub type JsExecutionError; + + #[wasm_bindgen(constructor, js_class = "ExecutionError")] + pub fn new(message: JsString, call_stack: JsValue) -> JsExecutionError; +} + +impl JsExecutionError { + pub fn create(message: String, call_stack: Option>) -> JsExecutionError { + let call_stack = match call_stack { + Some(call_stack) => { + let call_stack: Vec<_> = call_stack.iter().map(|loc| format!("{}", loc)).collect(); + ::from_serde(&call_stack).unwrap() + } + None => JsValue::UNDEFINED, + }; + JsExecutionError::new(JsString::from(message).into(), call_stack) + } +} diff --git a/acvm_js/src/lib.rs b/acvm_js/src/lib.rs index 027ddd64b..6914c0de5 100644 --- a/acvm_js/src/lib.rs +++ b/acvm_js/src/lib.rs @@ -14,6 +14,7 @@ cfg_if::cfg_if! { mod js_witness_map; mod logging; mod public_witness; + mod js_execution_error; pub use build_info::build_info; pub use compression::{compress_witness, decompress_witness}; @@ -21,6 +22,6 @@ cfg_if::cfg_if! { pub use js_witness_map::JsWitnessMap; pub use logging::{init_log_level, LogLevel}; pub use public_witness::{get_public_parameters_witness, get_public_witness, get_return_witness}; - + pub use js_execution_error::JsExecutionError; } } diff --git a/brillig_vm/src/lib.rs b/brillig_vm/src/lib.rs index d0eb6ac25..c2c5da375 100644 --- a/brillig_vm/src/lib.rs +++ b/brillig_vm/src/lib.rs @@ -36,6 +36,7 @@ pub enum VMStatus { InProgress, Failure { message: String, + call_stack: Vec, }, /// The VM process is not solvable as a [foreign call][Opcode::ForeignCall] has been /// reached where the outputs are yet to be resolved. @@ -121,7 +122,10 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { /// Indicating that the VM encountered a `Trap` Opcode /// or an invalid state. fn fail(&mut self, message: String) -> VMStatus { - self.status(VMStatus::Failure { message }); + let mut error_stack: Vec<_> = + self.call_stack.iter().map(|value| value.to_usize()).collect(); + error_stack.push(self.program_counter); + self.status(VMStatus::Failure { call_stack: error_stack, message }); self.status.clone() } @@ -174,7 +178,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { } Opcode::Return => { if let Some(register) = self.call_stack.pop() { - self.set_program_counter(register.to_usize()) + self.set_program_counter(register.to_usize() + 1) } else { self.fail("return opcode hit, but callstack already empty".to_string()) } @@ -278,7 +282,7 @@ impl<'bb_solver, B: BlackBoxFunctionSolver> VM<'bb_solver, B> { } Opcode::Call { location } => { // Push a return location - self.call_stack.push(Value::from(self.program_counter + 1)); + self.call_stack.push(Value::from(self.program_counter)); self.set_program_counter(*location) } Opcode::Const { destination, value } => { @@ -539,7 +543,10 @@ mod tests { let status = vm.process_opcode(); assert_eq!( status, - VMStatus::Failure { message: "explicit trap hit in brillig".to_string() } + VMStatus::Failure { + message: "explicit trap hit in brillig".to_string(), + call_stack: vec![1] + } ); // The register at index `2` should have not changed as we jumped over the add opcode From e11b810abc1b17d7e066b95321e340b75583bcfe Mon Sep 17 00:00:00 2001 From: sirasistant Date: Fri, 1 Sep 2023 08:38:47 +0000 Subject: [PATCH 2/7] feat: improve error handling --- acvm_js/src/execute.rs | 11 +++++------ acvm_js/src/foreign_call/mod.rs | 25 +++++++++++-------------- acvm_js/src/js/executionError.js | 6 ++++++ acvm_js/src/js_execution_error.rs | 22 +++++++++++----------- 4 files changed, 33 insertions(+), 31 deletions(-) create mode 100644 acvm_js/src/js/executionError.js diff --git a/acvm_js/src/execute.rs b/acvm_js/src/execute.rs index 3ca2fd120..1b63e744f 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -5,6 +5,7 @@ use acvm::{ pwg::{ACVMStatus, ErrorLocation, OpcodeResolutionError, ACVM}, }; +use js_sys::Error; use wasm_bindgen::prelude::wasm_bindgen; use crate::{ @@ -39,7 +40,7 @@ pub async fn execute_circuit( circuit: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); let solver = WasmBlackBoxFunctionSolver::initialize().await; @@ -61,7 +62,7 @@ pub async fn execute_circuit_with_black_box_solver( circuit: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); let circuit: Circuit = Circuit::read(&*circuit).expect("Failed to deserialize circuit"); @@ -103,12 +104,10 @@ pub async fn execute_circuit_with_black_box_solver( None => error.to_string(), }; - return Err(JsExecutionError::create(error_string, call_stack)); + return Err(JsExecutionError::create(error_string, call_stack).into()); } ACVMStatus::RequiresForeignCall(foreign_call) => { - let result = resolve_brillig(&foreign_call_handler, &foreign_call) - .await - .map_err(|err| JsExecutionError::create(err, None))?; + let result = resolve_brillig(&foreign_call_handler, &foreign_call).await?; acvm.resolve_pending_foreign_call(result); } diff --git a/acvm_js/src/foreign_call/mod.rs b/acvm_js/src/foreign_call/mod.rs index b4e60d06b..9ccaf733f 100644 --- a/acvm_js/src/foreign_call/mod.rs +++ b/acvm_js/src/foreign_call/mod.rs @@ -1,6 +1,6 @@ use acvm::{brillig_vm::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo}; -use js_sys::JsString; +use js_sys::{Error, JsString}; use wasm_bindgen::{prelude::wasm_bindgen, JsValue}; mod inputs; @@ -30,7 +30,7 @@ extern "C" { pub(super) async fn resolve_brillig( foreign_call_callback: &ForeignCallHandler, foreign_call_wait_info: &ForeignCallWaitInfo, -) -> Result { +) -> Result { // Prepare to call let name = JsString::from(foreign_call_wait_info.function.clone()); let inputs = inputs::encode_foreign_call_inputs(&foreign_call_wait_info.inputs); @@ -40,38 +40,35 @@ pub(super) async fn resolve_brillig( // The Brillig VM checks that the number of return values from // the foreign call is valid so we don't need to do it here. - outputs::decode_foreign_call_result(outputs) + outputs::decode_foreign_call_result(outputs).map_err(|message| Error::new(&message)) } -#[allow(dead_code)] async fn perform_foreign_call( foreign_call_handler: &ForeignCallHandler, name: JsString, inputs: js_sys::Array, -) -> Result { +) -> Result { // Call and await let this = JsValue::null(); let ret_js_val = foreign_call_handler .call2(&this, &name, &inputs) - .map_err(|err| format!("Error calling `foreign_call_callback`: {}", format_js_err(err)))?; + .map_err(|err| wrap_js_error("Error calling `foreign_call_callback`", &err))?; let ret_js_prom: js_sys::Promise = ret_js_val.into(); let ret_future: wasm_bindgen_futures::JsFuture = ret_js_prom.into(); let js_resolution = ret_future .await - .map_err(|err| format!("Error awaiting `foreign_call_handler`: {}", format_js_err(err)))?; + .map_err(|err| wrap_js_error("Error awaiting `foreign_call_handler`", &err))?; // Check that result conforms to expected shape. if !js_resolution.is_array() { - return Err("`foreign_call_handler` must return a Promise".into()); + return Err(Error::new("Expected `foreign_call_handler` to return an array")); } Ok(js_sys::Array::from(&js_resolution)) } -#[allow(dead_code)] -fn format_js_err(err: JsValue) -> String { - match err.as_string() { - Some(str) => str, - None => "Unknown".to_owned(), - } +fn wrap_js_error(message: &str, err: &JsValue) -> Error { + let new_error = Error::new(message); + new_error.set_cause(err); + new_error } diff --git a/acvm_js/src/js/executionError.js b/acvm_js/src/js/executionError.js new file mode 100644 index 000000000..33ffce95b --- /dev/null +++ b/acvm_js/src/js/executionError.js @@ -0,0 +1,6 @@ +export class ExecutionError extends Error { + constructor(message, callStack) { + super(message); + this.callStack = callStack; + } +} diff --git a/acvm_js/src/js_execution_error.rs b/acvm_js/src/js_execution_error.rs index c9ec30a71..4d0e679e0 100644 --- a/acvm_js/src/js_execution_error.rs +++ b/acvm_js/src/js_execution_error.rs @@ -1,20 +1,17 @@ -use gloo_utils::format::JsValueSerdeExt; -use js_sys::{Error, JsString}; -use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; - use acvm::acir::circuit::OpcodeLocation; +use js_sys::{Array, Error, JsString}; +use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; #[wasm_bindgen(typescript_custom_section)] const EXECUTION_ERROR: &'static str = r#" -export class ExecutionError extends Error { - constructor(message: string, private callStack?: string[]) { - super(message); - } +export declare class ExecutionError extends Error { + callStack?: string[] | undefined; + constructor(message: string, callStack?: string[] | undefined); } "#; // ExecutionError -#[wasm_bindgen] +#[wasm_bindgen(module = "src/js/executionError.js")] extern "C" { #[wasm_bindgen(extends = Error, js_name = "ExecutionError", typescript_type = "ExecutionError")] #[derive(Clone, Debug, PartialEq, Eq)] @@ -28,8 +25,11 @@ impl JsExecutionError { pub fn create(message: String, call_stack: Option>) -> JsExecutionError { let call_stack = match call_stack { Some(call_stack) => { - let call_stack: Vec<_> = call_stack.iter().map(|loc| format!("{}", loc)).collect(); - ::from_serde(&call_stack).unwrap() + let js_array = Array::new(); + for loc in call_stack { + js_array.push(&JsValue::from(format!("{}", loc))); + } + js_array.into() } None => JsValue::UNDEFINED, }; From db5ca9179c5dc3c3d7fb2cef1b6f5fa420a1b20a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Fri, 1 Sep 2023 09:37:55 +0000 Subject: [PATCH 3/7] fix: switch to raw error --- acvm_js/src/execute.rs | 7 +++++- acvm_js/src/js_execution_error.rs | 42 +++++++++++++++---------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/acvm_js/src/execute.rs b/acvm_js/src/execute.rs index 1b63e744f..843e520d6 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -104,7 +104,12 @@ pub async fn execute_circuit_with_black_box_solver( None => error.to_string(), }; - return Err(JsExecutionError::create(error_string, call_stack).into()); + let mut err = JsExecutionError::new(error_string.into()); + if let Some(call_stack) = call_stack { + err.set_call_stack(call_stack); + } + + return Err(err.into()); } ACVMStatus::RequiresForeignCall(foreign_call) => { let result = resolve_brillig(&foreign_call_handler, &foreign_call).await?; diff --git a/acvm_js/src/js_execution_error.rs b/acvm_js/src/js_execution_error.rs index 4d0e679e0..454c606b0 100644 --- a/acvm_js/src/js_execution_error.rs +++ b/acvm_js/src/js_execution_error.rs @@ -1,38 +1,38 @@ use acvm::acir::circuit::OpcodeLocation; -use js_sys::{Array, Error, JsString}; +use js_sys::{Array, Error, JsString, Reflect}; use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; #[wasm_bindgen(typescript_custom_section)] const EXECUTION_ERROR: &'static str = r#" -export declare class ExecutionError extends Error { - callStack?: string[] | undefined; - constructor(message: string, callStack?: string[] | undefined); -} +export type ExecutionError = Error & { + callStack?: string[]; +}; "#; -// ExecutionError -#[wasm_bindgen(module = "src/js/executionError.js")] +/// JsExecutionError is a raw js error. +/// It'd be ideal that execution error was a subclass of Error, but for that we'd need to use JS snippets or a js module. +/// Currently JS snippets don't work with a nodejs target. And a module would be too much for just a custom error type. +#[wasm_bindgen] extern "C" { #[wasm_bindgen(extends = Error, js_name = "ExecutionError", typescript_type = "ExecutionError")] #[derive(Clone, Debug, PartialEq, Eq)] pub type JsExecutionError; - #[wasm_bindgen(constructor, js_class = "ExecutionError")] - pub fn new(message: JsString, call_stack: JsValue) -> JsExecutionError; + #[wasm_bindgen(constructor, js_class = "Error")] + pub fn new(message: JsString) -> JsExecutionError; } impl JsExecutionError { - pub fn create(message: String, call_stack: Option>) -> JsExecutionError { - let call_stack = match call_stack { - Some(call_stack) => { - let js_array = Array::new(); - for loc in call_stack { - js_array.push(&JsValue::from(format!("{}", loc))); - } - js_array.into() - } - None => JsValue::UNDEFINED, - }; - JsExecutionError::new(JsString::from(message).into(), call_stack) + /// Sets the call stack in an execution error. + pub fn set_call_stack(&mut self, call_stack: Vec) { + let js_array = Array::new(); + for loc in call_stack { + js_array.push(&JsValue::from(format!("{}", loc))); + } + assert!( + Reflect::set(self, &JsValue::from("callStack"), &js_array) + .expect("Errors should be objects"), + "Errors should be writable" + ); } } From f29dd96b8fd3fe633a49a860f2525e26fa5d7680 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 4 Sep 2023 10:20:21 +0000 Subject: [PATCH 4/7] feat: improve js error building --- acvm_js/src/execute.rs | 9 ++------- acvm_js/src/js_execution_error.rs | 32 ++++++++++++++++++++++--------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/acvm_js/src/execute.rs b/acvm_js/src/execute.rs index 8422ad4c6..61c804fd6 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -99,17 +99,12 @@ pub async fn execute_circuit_with_black_box_solver( _ => (None, None), }; - let error_string = match assert_message { + let error_string = match &assert_message { Some(assert_message) => format!("{}: {}", error, assert_message), None => error.to_string(), }; - let mut err = JsExecutionError::new(error_string.into()); - if let Some(call_stack) = call_stack { - err.set_call_stack(call_stack); - } - - return Err(err.into()); + return Err(JsExecutionError::new(error_string.into(), call_stack).into()); } ACVMStatus::RequiresForeignCall(foreign_call) => { let result = resolve_brillig(&foreign_call_handler, &foreign_call).await?; diff --git a/acvm_js/src/js_execution_error.rs b/acvm_js/src/js_execution_error.rs index 454c606b0..d91a9425f 100644 --- a/acvm_js/src/js_execution_error.rs +++ b/acvm_js/src/js_execution_error.rs @@ -19,19 +19,33 @@ extern "C" { pub type JsExecutionError; #[wasm_bindgen(constructor, js_class = "Error")] - pub fn new(message: JsString) -> JsExecutionError; + fn constructor(message: JsString) -> JsExecutionError; } impl JsExecutionError { - /// Sets the call stack in an execution error. - pub fn set_call_stack(&mut self, call_stack: Vec) { - let js_array = Array::new(); - for loc in call_stack { - js_array.push(&JsValue::from(format!("{}", loc))); - } + /// Creates a new execution error with the given call stack. + /// Call stacks won't be optional in the future, after removing ErrorLocation in ACVM. + pub fn new(message: String, call_stack: Option>) -> Self { + let mut error = JsExecutionError::constructor(JsString::from(message)); + let js_call_stack = match call_stack { + Some(call_stack) => { + let js_array = Array::new(); + for loc in call_stack { + js_array.push(&JsValue::from(format!("{}", loc))); + } + js_array.into() + } + None => JsValue::UNDEFINED, + }; + + error.set_property("callStack", js_call_stack); + + error + } + + fn set_property(&mut self, property: &str, value: JsValue) { assert!( - Reflect::set(self, &JsValue::from("callStack"), &js_array) - .expect("Errors should be objects"), + Reflect::set(self, &JsValue::from(property), &value).expect("Errors should be objects"), "Errors should be writable" ); } From 1336ceff8f95ff0823051e6a10bb634fc9a596e5 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 4 Sep 2023 10:51:24 +0000 Subject: [PATCH 5/7] feat: update message if assertion is present --- acvm_js/src/execute.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm_js/src/execute.rs b/acvm_js/src/execute.rs index 61c804fd6..7ee87b96e 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -100,7 +100,7 @@ pub async fn execute_circuit_with_black_box_solver( }; let error_string = match &assert_message { - Some(assert_message) => format!("{}: {}", error, assert_message), + Some(assert_message) => format!("Assertion failed: {}", assert_message), None => error.to_string(), }; From e31bf7a4e254f244bdb4236ff149a7c98298ace7 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 4 Sep 2023 12:23:42 +0000 Subject: [PATCH 6/7] fix: remove old js file --- acvm/src/pwg/mod.rs | 6 +++--- acvm_js/src/js/executionError.js | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) delete mode 100644 acvm_js/src/js/executionError.js diff --git a/acvm/src/pwg/mod.rs b/acvm/src/pwg/mod.rs index add165889..a336ea2fc 100644 --- a/acvm/src/pwg/mod.rs +++ b/acvm/src/pwg/mod.rs @@ -99,15 +99,15 @@ impl std::fmt::Display for ErrorLocation { #[derive(Clone, PartialEq, Eq, Debug, Error)] pub enum OpcodeResolutionError { - #[error("cannot solve opcode: {0}")] + #[error("Cannot solve opcode: {0}")] OpcodeNotSolvable(#[from] OpcodeNotSolvable), - #[error("backend does not currently support the {0} opcode. ACVM does not currently have a fallback for this opcode.")] + #[error("Backend does not currently support the {0} opcode. ACVM does not currently have a fallback for this opcode.")] UnsupportedBlackBoxFunc(BlackBoxFunc), #[error("Cannot satisfy constraint")] UnsatisfiedConstrain { opcode_location: ErrorLocation }, #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")] IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 }, - #[error("failed to solve blackbox function: {0}, reason: {1}")] + #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), #[error("Failed to solve brillig function, reason: {message}")] BrilligFunctionFailed { message: String, call_stack: Vec }, diff --git a/acvm_js/src/js/executionError.js b/acvm_js/src/js/executionError.js deleted file mode 100644 index 33ffce95b..000000000 --- a/acvm_js/src/js/executionError.js +++ /dev/null @@ -1,6 +0,0 @@ -export class ExecutionError extends Error { - constructor(message, callStack) { - super(message); - this.callStack = callStack; - } -} From abd2ff5fa1e06ada680952005bdab825b5a59aab Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 4 Sep 2023 13:03:30 +0000 Subject: [PATCH 7/7] docs: add doc comment on error call stack --- brillig_vm/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/brillig_vm/src/lib.rs b/brillig_vm/src/lib.rs index c2c5da375..af95abdf2 100644 --- a/brillig_vm/src/lib.rs +++ b/brillig_vm/src/lib.rs @@ -30,13 +30,16 @@ pub use memory::Memory; use num_bigint::BigUint; pub use registers::Registers; +/// The error call stack contains the opcode indexes of the call stack at the time of failure, plus the index of the opcode that failed. +pub type ErrorCallStack = Vec; + #[derive(Debug, PartialEq, Eq, Clone)] pub enum VMStatus { Finished, InProgress, Failure { message: String, - call_stack: Vec, + call_stack: ErrorCallStack, }, /// The VM process is not solvable as a [foreign call][Opcode::ForeignCall] has been /// reached where the outputs are yet to be resolved.