Convert BrilligSolver Trap failure into UnsatisfiedConstrain error

manastech · Jun 26, 2024 · 904eaf1 · 904eaf1
1 parent a322e9c
commit 904eaf1
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 22 deletions.
diff --git 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>, 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>, 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
@@ -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<FieldElement>> 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,
+                )))
+            }
         }
     }