From 262ddf657a448cd7ac35d0a855164fca09ab0ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20Gir=C3=A1ldez?= Date: Fri, 13 Oct 2023 12:03:19 -0400 Subject: [PATCH] chore: deduplicate code --- acvm-repo/acvm/src/pwg/mod.rs | 82 +++++++++-------------------------- tooling/debugger/src/lib.rs | 2 +- 2 files changed, 22 insertions(+), 62 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 5b42981f0f6..c1a57a6e367 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -177,6 +177,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { self.instruction_pointer } + /// Returns the location for the next opcode to execute, or None if execution is finished pub fn location(&self) -> Option { if self.instruction_pointer >= self.opcodes.len() { // evaluation finished @@ -256,6 +257,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } pub fn solve_opcode(&mut self) -> ACVMStatus { + self.step_opcode(false) + } + + pub fn step_opcode(&mut self, step_into_brillig: bool) -> ACVMStatus { let opcode = &self.opcodes[self.instruction_pointer]; let resolution = match opcode { @@ -272,33 +277,15 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { let solver = self.block_solvers.entry(*block_id).or_default(); solver.solve_memory_op(op, &mut self.witness_map, predicate) } - Opcode::Brillig(_) => match self.solve_brillig_opcode() { - Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), + Opcode::Brillig(_) => match self.step_brillig_opcode(step_into_brillig) { + Ok(BrilligSolverStatus::ForeignCallWait(foreign_call)) => { + return self.wait_for_foreign_call(foreign_call) + } + Ok(BrilligSolverStatus::InProgress) => return self.status(ACVMStatus::InProgress), res => res.map(|_| ()), }, }; - self.handle_resolution(resolution) - } - - pub fn step_opcode(&mut self) -> ACVMStatus { - match &self.opcodes[self.instruction_pointer] { - Opcode::Brillig(_) => { - let resolution = match self.step_brillig_opcode() { - Ok(BrilligSolverStatus::ForeignCallWait(foreign_call)) => { - return self.wait_for_foreign_call(foreign_call) - } - Ok(BrilligSolverStatus::InProgress) => { - return self.status(ACVMStatus::InProgress) - } - res => res.map(|_| ()), - }; - self.handle_resolution(resolution) - } - _ => self.solve_opcode(), - } - } - fn handle_resolution(&mut self, resolution: Result<(), OpcodeResolutionError>) -> ACVMStatus { match resolution { Ok(()) => { self.instruction_pointer += 1; @@ -332,49 +319,18 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } } - fn solve_brillig_opcode( + fn step_brillig_opcode( &mut self, - ) -> Result, OpcodeResolutionError> { - let Opcode::Brillig(brillig) = &self.opcodes[self.instruction_pointer] else { - unreachable!("Not executing a Brillig opcode"); - }; - let witness = &mut self.witness_map; - if BrilligSolver::::should_skip(witness, brillig)? { - BrilligSolver::::zero_out_brillig_outputs(witness, brillig).map(|_| None) - } else { - // If we're resuming execution after resolving a foreign call then - // there will be a cached `BrilligSolver` to avoid recomputation. - let mut solver: BrilligSolver<'_, B> = match self.brillig_solver.take() { - Some(solver) => solver, - None => { - BrilligSolver::new(witness, brillig, self.backend, self.instruction_pointer)? - } - }; - match solver.solve()? { - BrilligSolverStatus::ForeignCallWait(foreign_call) => { - // Cache the current state of the solver - self.brillig_solver = Some(solver); - Ok(Some(foreign_call)) - } - BrilligSolverStatus::InProgress => { - unreachable!("Brillig solver still in progress") - } - BrilligSolverStatus::Finished => { - // Write execution outputs - solver.finalize(witness, brillig)?; - Ok(None) - } - } - } - } - - fn step_brillig_opcode(&mut self) -> Result { + step_into: bool, + ) -> Result { let Opcode::Brillig(brillig) = &self.opcodes[self.instruction_pointer] else { unreachable!("Not executing a Brillig opcode"); }; let witness = &mut self.witness_map; - // Try to use the cached `BrilligSolver` when stepping through opcodes + // Try to use the cached `BrilligSolver` which we will have if: + // - stepping into a Brillig block + // - resuming execution from a foreign call let mut solver: BrilligSolver<'_, B> = match self.brillig_solver.take() { Some(solver) => solver, None => { @@ -385,13 +341,17 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { BrilligSolver::new(witness, brillig, self.backend, self.instruction_pointer)? } }; - let status = solver.step()?; + + let status = if step_into { solver.step()? } else { solver.solve()? }; match status { BrilligSolverStatus::ForeignCallWait(_) => { // Cache the current state of the solver self.brillig_solver = Some(solver); } BrilligSolverStatus::InProgress => { + if !step_into { + unreachable!("Brillig solver still in progress") + } // Cache the current state of the solver self.brillig_solver = Some(solver); } diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 9d46450780c..5e7325334ed 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -27,7 +27,7 @@ struct DebugContext<'backend, B: BlackBoxFunctionSolver> { impl<'backend, B: BlackBoxFunctionSolver> DebugContext<'backend, B> { fn step_opcode(&mut self) -> Result { - let solver_status = self.acvm.as_mut().unwrap().step_opcode(); + let solver_status = self.acvm.as_mut().unwrap().step_opcode(true); self.handle_acvm_status(solver_status) }