From fd28657426260ce3c53517b75a27eb5c4a74e234 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 5 Apr 2023 18:35:33 +0100 Subject: [PATCH] feat(acvm)!: make `Backend` trait object safe (#180) * chore: add test to check that `Backend` is object safe * chore: make `Backend` trait object safe --- acvm/src/lib.rs | 69 ++++++++++++++++++-------------------- acvm/src/pwg/directives.rs | 21 +++++++++++- 2 files changed, 53 insertions(+), 37 deletions(-) diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index 34e465174..35a58ff2c 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -10,14 +10,13 @@ pub mod pwg; use crate::pwg::{arithmetic::ArithmeticSolver, oracle::OracleSolver}; use acir::{ circuit::{ - directives::Directive, opcodes::{BlackBoxFuncCall, OracleData}, Circuit, Opcode, }, native_types::{Expression, Witness}, BlackBoxFunc, }; -use pwg::block::Blocks; +use pwg::{block::Blocks, directives::solve_directives}; use std::collections::BTreeMap; use thiserror::Error; @@ -65,6 +64,22 @@ pub enum OpcodeResolution { InProgress, } +/// Check if all of the inputs to the function have assignments +/// +/// Returns the first missing assignment if any are missing +fn first_missing_assignment( + witness_assignments: &BTreeMap, + func_call: &BlackBoxFuncCall, +) -> Option { + func_call.inputs.iter().find_map(|input| { + if witness_assignments.contains_key(&input.witness) { + None + } else { + Some(input.witness) + } + }) +} + pub trait Backend: SmartContract + ProofSystemCompiler + PartialWitnessGenerator {} /// This component will generate the backend specific output for @@ -89,18 +104,16 @@ pub trait PartialWitnessGenerator { Opcode::Arithmetic(expr) => ArithmeticSolver::solve(initial_witness, expr), Opcode::BlackBoxFuncCall(bb_func) => { if let Some(unassigned_witness) = - Self::any_missing_assignment(initial_witness, bb_func) + first_missing_assignment(initial_witness, bb_func) { Ok(OpcodeResolution::Stalled(OpcodeNotSolvable::MissingAssignment( unassigned_witness.0, ))) } else { - Self::solve_black_box_function_call(initial_witness, bb_func) + self.solve_black_box_function_call(initial_witness, bb_func) } } - Opcode::Directive(directive) => { - Self::solve_directives(initial_witness, directive) - } + Opcode::Directive(directive) => solve_directives(initial_witness, directive), Opcode::Block(block) | Opcode::ROM(block) | Opcode::RAM(block) => { blocks.solve(block.id, &block.trace, initial_witness) } @@ -160,37 +173,10 @@ pub trait PartialWitnessGenerator { } fn solve_black_box_function_call( + &self, initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, ) -> Result; - - // Check if all of the inputs to the function have assignments - // Returns the first missing assignment if any are missing - fn any_missing_assignment( - initial_witness: &mut BTreeMap, - func_call: &BlackBoxFuncCall, - ) -> Option { - func_call.inputs.iter().find_map(|input| { - if initial_witness.contains_key(&input.witness) { - None - } else { - Some(input.witness) - } - }) - } - - fn solve_directives( - initial_witness: &mut BTreeMap, - directive: &Directive, - ) -> Result { - match pwg::directives::solve_directives(initial_witness, directive) { - Ok(_) => Ok(OpcodeResolution::Solved), - Err(OpcodeResolutionError::OpcodeNotSolvable(unsolved)) => { - Ok(OpcodeResolution::Stalled(unsolved)) - } - Err(err) => Err(err), - } - } } pub trait SmartContract { @@ -325,13 +311,15 @@ mod test { }; use crate::{ - pwg::block::Blocks, OpcodeResolution, OpcodeResolutionError, PartialWitnessGenerator, + pwg::block::Blocks, Backend, OpcodeResolution, OpcodeResolutionError, + PartialWitnessGenerator, }; struct StubbedPwg; impl PartialWitnessGenerator for StubbedPwg { fn solve_black_box_function_call( + &self, _initial_witness: &mut BTreeMap, _func_call: &BlackBoxFuncCall, ) -> Result { @@ -408,4 +396,13 @@ mod test { assert!(unsolved_opcodes.is_empty(), "should be fully solved"); assert!(unresolved_oracles.is_empty(), "should have no unresolved oracles"); } + + #[test] + fn test_backend_object_safety() { + // This test doesn't do anything at runtime. + // We just want to ensure that the `Backend` trait is object safe and this test will refuse to compile + // if this property is broken. + #[allow(dead_code)] + fn check_object_safety(_backend: Box) {} + } } diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 2feb2b231..808003b7b 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -8,13 +8,32 @@ use acir::{ use num_bigint::BigUint; use num_traits::Zero; -use crate::OpcodeResolutionError; +use crate::{OpcodeResolution, OpcodeResolutionError}; use super::{get_value, insert_value, sorting::route, witness_to_value}; +/// Attempts to solve the [`Directive`] opcode `directive`. +/// If successful, `initial_witness` will be mutated to contain the new witness assignment. +/// +/// Returns `Ok(OpcodeResolution)` to signal whether the directive was successful solved. +/// +/// Returns `Err(OpcodeResolutionError)` if a circuit constraint is unsatisfied. pub fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, +) -> Result { + match solve_directives_internal(initial_witness, directive) { + Ok(_) => Ok(OpcodeResolution::Solved), + Err(OpcodeResolutionError::OpcodeNotSolvable(unsolved)) => { + Ok(OpcodeResolution::Stalled(unsolved)) + } + Err(err) => Err(err), + } +} + +fn solve_directives_internal( + initial_witness: &mut BTreeMap, + directive: &Directive, ) -> Result<(), OpcodeResolutionError> { match directive { Directive::Invert { x, result } => {