From 904eaf1e240d8726bcfe6f82154b425fb765dc33 Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Tue, 25 Jun 2024 16:19:24 -0600 Subject: [PATCH] Convert BrilligSolver Trap failure into UnsatisfiedConstrain error --- acvm-repo/acvm/src/pwg/brillig.rs | 42 ++++++++++++++++++------------- tooling/debugger/src/context.rs | 15 ++++++++--- 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 3a639df044a..321c50f2a17 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -161,14 +161,28 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { match vm_status { VMStatus::Finished { .. } => Ok(BrilligSolverStatus::Finished), VMStatus::InProgress => Ok(BrilligSolverStatus::InProgress), + VMStatus::ForeignCallWait { function, inputs } => { + Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs })) + } VMStatus::Failure { reason, call_stack } => { - let payload = match reason { + match reason { FailureReason::RuntimeError { message } => { - Some(ResolvedAssertionPayload::String(message)) + let call_stack = call_stack + .iter() + .map(|brillig_index| OpcodeLocation::Brillig { + acir_index: self.acir_index, + brillig_index: *brillig_index, + }) + .collect(); + Err(OpcodeResolutionError::BrilligFunctionFailed { + payload: Some(ResolvedAssertionPayload::String(message)), + call_stack: call_stack, + }) } + FailureReason::Trap { revert_data_offset, revert_data_size } => { // Since noir can only revert with strings currently, we can parse return data as a string - if revert_data_size == 0 { + let payload = if revert_data_size == 0 { None } else { let memory = self.vm.get_memory(); @@ -206,22 +220,14 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { })) } } - } - } - }; - Err(OpcodeResolutionError::BrilligFunctionFailed { - payload, - call_stack: call_stack - .iter() - .map(|brillig_index| OpcodeLocation::Brillig { - acir_index: self.acir_index, - brillig_index: *brillig_index, + }; + + Err(OpcodeResolutionError::UnsatisfiedConstrain { + opcode_location: super::ErrorLocation::Unresolved, + payload, }) - .collect(), - }) - } - VMStatus::ForeignCallWait { function, inputs } => { - Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs })) + } + } } } } diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index d18ec5f0786..07125616ce9 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -5,7 +5,7 @@ use acvm::acir::native_types::{Witness, WitnessMap, WitnessStack}; use acvm::brillig_vm::MemoryValue; use acvm::pwg::{ ACVMStatus, AcirCallWaitInfo, BrilligSolver, BrilligSolverStatus, ForeignCallWaitInfo, - OpcodeNotSolvable, StepResult, ACVM, + OpcodeNotSolvable, OpcodeResolutionError, StepResult, ACVM, }; use acvm::{BlackBoxFunctionSolver, FieldElement}; @@ -471,9 +471,16 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { self.brillig_solver = Some(solver); self.handle_foreign_call(foreign_call) } - Err(err) => DebugCommandResult::Error(NargoError::ExecutionError( - ExecutionError::SolvingError(err, None), - )), + Err(err) => { + if let OpcodeResolutionError::UnsatisfiedConstrain { .. } = err { + // return solver ownership so brillig_solver it has the right opcode location + self.brillig_solver = Some(solver); + } + // TODO: should we return solver ownership in all Err scenarios>? + DebugCommandResult::Error(NargoError::ExecutionError(ExecutionError::SolvingError( + err, None, + ))) + } } }