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..a336ea2fc 100644 --- a/acvm/src/pwg/mod.rs +++ b/acvm/src/pwg/mod.rs @@ -99,18 +99,18 @@ 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 {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}")] + #[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 2e91cc994..ce13ff268 100644 --- a/acvm/tests/solver.rs +++ b/acvm/tests/solver.rs @@ -598,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 baa3b712f..7ee87b96e 100644 --- a/acvm_js/src/execute.rs +++ b/acvm_js/src/execute.rs @@ -5,11 +5,12 @@ use acvm::{ pwg::{ACVMStatus, ErrorLocation, OpcodeResolutionError, ACVM}, }; +use js_sys::Error; use wasm_bindgen::prelude::wasm_bindgen; use crate::{ foreign_call::{resolve_brillig, ForeignCallHandler}, - JsWitnessMap, + JsExecutionError, JsWitnessMap, }; #[wasm_bindgen] @@ -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"); @@ -76,23 +77,34 @@ 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), .. - } => get_assert_message(&circuit.assert_messages, opcode_location), - _ => None, + } => ( + get_assert_message(&circuit.assert_messages, opcode_location), + Some(vec![*opcode_location]), + ), + OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { + let failing_opcode = + call_stack.last().expect("Brillig error call stacks cannot be empty"); + ( + get_assert_message(&circuit.assert_messages, failing_opcode), + Some(call_stack.clone()), + ) + } + _ => (None, None), }; - let error_string = match assert_message { - Some(assert_message) => format!("{}: {}", error, assert_message), + let error_string = match &assert_message { + Some(assert_message) => format!("Assertion failed: {}", assert_message), None => error.to_string(), }; - return Err(error_string.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/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_execution_error.rs b/acvm_js/src/js_execution_error.rs new file mode 100644 index 000000000..d91a9425f --- /dev/null +++ b/acvm_js/src/js_execution_error.rs @@ -0,0 +1,52 @@ +use acvm::acir::circuit::OpcodeLocation; +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 type ExecutionError = Error & { + callStack?: string[]; +}; +"#; + +/// 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 = "Error")] + fn constructor(message: JsString) -> JsExecutionError; +} + +impl JsExecutionError { + /// 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(property), &value).expect("Errors should be objects"), + "Errors should be writable" + ); + } +} 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..af95abdf2 100644 --- a/brillig_vm/src/lib.rs +++ b/brillig_vm/src/lib.rs @@ -30,12 +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: ErrorCallStack, }, /// 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 +125,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 +181,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 +285,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 +546,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