From a347fa0a940479541c5a78b88e91aa41a366cf8a Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 22 Feb 2023 16:26:09 +0000 Subject: [PATCH 01/16] add block opcode --- acir/src/circuit/opcodes.rs | 47 +++- acir/src/native_types/arithmetic.rs | 14 +- acvm/src/compiler/transformers/fallback.rs | 5 +- acvm/src/lib.rs | 59 ++--- acvm/src/pwg.rs | 47 ++-- acvm/src/pwg/arithmetic.rs | 40 ++-- acvm/src/pwg/block.rs | 187 +++++++++++++++ acvm/src/pwg/directives.rs | 253 ++++++++++++--------- acvm/src/pwg/logic.rs | 38 ++-- acvm/src/pwg/range.rs | 17 +- acvm/src/pwg/signature/ecdsa.rs | 38 +++- 11 files changed, 514 insertions(+), 231 deletions(-) create mode 100644 acvm/src/pwg/block.rs diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 4e76c65c5..cb9bb4cbf 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -6,11 +6,24 @@ use crate::serialization::{read_n, read_u16, read_u32, write_bytes, write_u16, w use crate::BlackBoxFunc; use serde::{Deserialize, Serialize}; +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default)] +pub struct BlockId(u32); + +/// Operation on a block +/// We can either write or read at a block index +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BlockOp { + pub operation: Expression, + pub index: Expression, + pub value: Expression, +} + #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] pub enum Opcode { Arithmetic(Expression), BlackBoxFuncCall(BlackBoxFuncCall), Directive(Directive), + Block(BlockId, Vec), } impl Opcode { @@ -21,17 +34,18 @@ impl Opcode { Opcode::Arithmetic(_) => "arithmetic", Opcode::Directive(directive) => directive.name(), Opcode::BlackBoxFuncCall(g) => g.name.name(), + Opcode::Block(_, _) => "block", } } - // We have three types of opcodes allowed in the IR - // Expression, BlackBoxFuncCall and Directives - // When we serialize these opcodes, we use the index + + // When we serialize the opcodes, we use the index // to uniquely identify which category of opcode we are dealing with. pub(crate) fn to_index(&self) -> u8 { match self { Opcode::Arithmetic(_) => 0, Opcode::BlackBoxFuncCall(_) => 1, Opcode::Directive(_) => 2, + Opcode::Block(_, _) => 3, } } @@ -53,6 +67,17 @@ impl Opcode { Opcode::Arithmetic(expr) => expr.write(writer), Opcode::BlackBoxFuncCall(func_call) => func_call.write(writer), Opcode::Directive(directive) => directive.write(writer), + Opcode::Block(id, trace) => { + write_u32(&mut writer, id.0)?; + write_u32(&mut writer, trace.len() as u32)?; + + for op in trace { + op.operation.write(&mut writer)?; + op.index.write(&mut writer)?; + op.value.write(&mut writer)?; + } + Ok(()) + } } } pub fn read(mut reader: R) -> std::io::Result { @@ -74,6 +99,18 @@ impl Opcode { let directive = Directive::read(reader)?; Ok(Opcode::Directive(directive)) } + 3 => { + let id = read_u32(&mut reader)?; + let len = read_u32(&mut reader)?; + let mut trace = Vec::new(); + for _i in 0..len { + let operation = Expression::read(&mut reader)?; + let index = Expression::read(&mut reader)?; + let value = Expression::read(&mut reader)?; + trace.push(BlockOp { operation, index, value }); + } + Ok(Opcode::Block(BlockId(id), trace)) + } _ => Err(std::io::ErrorKind::InvalidData.into()), } } @@ -175,6 +212,10 @@ impl std::fmt::Display for Opcode { witnesses.last().unwrap().witness_index() ), }, + Opcode::Block(id, trace) => { + write!(f, "BLOCK ")?; + write!(f, "(id: {}, len: {}) ", id.0, trace.len()) + } } } } diff --git a/acir/src/native_types/arithmetic.rs b/acir/src/native_types/arithmetic.rs index 67838ad38..dfbc9cd0f 100644 --- a/acir/src/native_types/arithmetic.rs +++ b/acir/src/native_types/arithmetic.rs @@ -149,11 +149,23 @@ impl Expression { Ok(expr) } - // TODO: possibly rename, since linear can have one mul_term pub fn is_linear(&self) -> bool { self.mul_terms.is_empty() } + // Returns a witness belonging to the expression + pub fn get_witness(&self) -> Option { + if self.linear_combinations.is_empty() { + if self.mul_terms.is_empty() { + None + } else { + Some(self.mul_terms[0].1) + } + } else { + Some(self.linear_combinations[0].1) + } + } + pub fn term_addition(&mut self, coefficient: acir_field::FieldElement, variable: Witness) { self.linear_combinations.push((coefficient, variable)) } diff --git a/acvm/src/compiler/transformers/fallback.rs b/acvm/src/compiler/transformers/fallback.rs index 3dffcf9fe..1d2b6fbe9 100644 --- a/acvm/src/compiler/transformers/fallback.rs +++ b/acvm/src/compiler/transformers/fallback.rs @@ -22,9 +22,8 @@ impl FallbackTransformer { for opcode in acir.opcodes { let bb_func_call = match &opcode { - Opcode::Arithmetic(_) | Opcode::Directive(_) => { - // If it is not a black box function, then it is a directive or - // an arithmetic expression which are always supported + Opcode::Arithmetic(_) | Opcode::Directive(_) | Opcode::Block(_, _) => { + // directive, arithmetic expression or block are handled by acvm acir_supported_opcodes.push(opcode); continue; } diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index 52fe34b46..5dcc023a2 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -10,6 +10,7 @@ use acir::{ native_types::{Expression, Witness}, BlackBoxFunc, }; +use pwg::block::Blocks; use std::collections::BTreeMap; use thiserror::Error; @@ -49,6 +50,12 @@ pub enum OpcodeResolutionError { IncorrectNumFunctionArguments(usize, BlackBoxFunc, usize), } +#[derive(PartialEq, Eq, Debug)] +pub enum GateResolution { + Resolved, // Gate is solved + Skip(OpcodeNotSolvable), // Gate cannot be solved +} + pub trait Backend: SmartContract + ProofSystemCompiler + PartialWitnessGenerator {} /// This component will generate the backend specific output for @@ -58,42 +65,38 @@ pub trait PartialWitnessGenerator { fn solve( &self, initial_witness: &mut BTreeMap, - opcodes: Vec, + mut gates_to_resolve: Vec, ) -> Result<(), OpcodeResolutionError> { - if opcodes.is_empty() { - return Ok(()); - } - let mut unsolved_opcodes: Vec = Vec::new(); - - for opcode in opcodes.into_iter() { - let resolution = match &opcode { - Opcode::Arithmetic(expr) => ArithmeticSolver::solve(initial_witness, expr), - Opcode::BlackBoxFuncCall(bb_func) => { - Self::solve_black_box_function_call(initial_witness, bb_func) + let mut unresolved_gates: Vec = Vec::new(); + let mut blocker = Blocks::default(); + while !gates_to_resolve.is_empty() { + unresolved_gates.clear(); + + for gate in &gates_to_resolve { + let result = match gate { + Opcode::Arithmetic(expr) => ArithmeticSolver::solve(initial_witness, expr), + Opcode::BlackBoxFuncCall(bb_func) => { + Self::solve_black_box_function_call(initial_witness, bb_func) + } + Opcode::Directive(directive) => { + Self::solve_directives(initial_witness, directive) + } + Opcode::Block(id, trace) => blocker.solve(*id, trace, initial_witness), + }?; + match result { + GateResolution::Skip(_) => unresolved_gates.push(gate.clone()), + GateResolution::Resolved => (), } - Opcode::Directive(directive) => Self::solve_directives(initial_witness, directive), - }; - - match resolution { - Ok(_) => { - // We do nothing in the happy case - } - Err(OpcodeResolutionError::OpcodeNotSolvable(_)) => { - // For opcode not solvable errors, we push those opcodes to the back as - // it could be because the opcodes are out of order, ie this assignment - // relies on a later opcodes's results - unsolved_opcodes.push(opcode); - } - Err(err) => return Err(err), } + std::mem::swap(&mut gates_to_resolve, &mut unresolved_gates); } - self.solve(initial_witness, unsolved_opcodes) + Ok(()) } fn solve_black_box_function_call( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, - ) -> Result<(), OpcodeResolutionError>; + ) -> Result; // Check if all of the inputs to the function have assignments // Returns true if all of the inputs have been assigned @@ -109,7 +112,7 @@ pub trait PartialWitnessGenerator { fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, - ) -> Result<(), OpcodeResolutionError> { + ) -> Result { pwg::directives::solve_directives(initial_witness, directive) } } diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index e31c34442..80c1e5aa9 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -1,17 +1,20 @@ // Re-usable methods that backends can use to implement their PWG -use crate::{OpcodeNotSolvable, OpcodeResolutionError}; +use crate::OpcodeResolutionError; use acir::{ native_types::{Expression, Witness}, FieldElement, }; use std::collections::BTreeMap; +use self::arithmetic::ArithmeticSolver; + // arithmetic pub mod arithmetic; // Directives pub mod directives; // black box functions +pub mod block; pub mod hash; pub mod logic; pub mod range; @@ -24,45 +27,21 @@ pub mod sorting; pub fn witness_to_value( initial_witness: &BTreeMap, witness: Witness, -) -> Result<&FieldElement, OpcodeResolutionError> { - match initial_witness.get(&witness) { - Some(value) => Ok(value), - None => Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(witness.0), - )), - } +) -> Option<&FieldElement> { + initial_witness.get(&witness) +} + +pub fn expression_to_const(expr: &Expression) -> Option { + expr.is_const().then_some(expr.q_c) } + // TODO: There is an issue open to decide on whether we need to get values from Expressions // TODO versus just getting values from Witness pub fn get_value( expr: &Expression, initial_witness: &BTreeMap, -) -> Result { - let mut result = expr.q_c; - - for term in &expr.linear_combinations { - let coefficient = term.0; - let variable = term.1; - - // Get the value assigned to that variable - let assignment = *witness_to_value(initial_witness, variable)?; - - result += coefficient * assignment; - } - - for term in &expr.mul_terms { - let coefficient = term.0; - let lhs_variable = term.1; - let rhs_variable = term.2; - - // Get the values assigned to those variables - let lhs_assignment = *witness_to_value(initial_witness, lhs_variable)?; - let rhs_assignment = *witness_to_value(initial_witness, rhs_variable)?; - - result += coefficient * lhs_assignment * rhs_assignment; - } - - Ok(result) +) -> Option { + expression_to_const(&ArithmeticSolver::evaluate(expr, initial_witness)) } // Inserts `value` into the initial witness map diff --git a/acvm/src/pwg/arithmetic.rs b/acvm/src/pwg/arithmetic.rs index 2d611db02..466fd1c8d 100644 --- a/acvm/src/pwg/arithmetic.rs +++ b/acvm/src/pwg/arithmetic.rs @@ -4,14 +4,14 @@ use acir::{ }; use std::collections::BTreeMap; -use crate::{OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; /// An Arithmetic solver will take a Circuit's arithmetic gates with witness assignments /// and create the other witness variables pub struct ArithmeticSolver; #[allow(clippy::enum_variant_names)] -enum GateStatus { +pub enum GateStatus { GateSatisfied(FieldElement), GateSolvable(FieldElement, (FieldElement, Witness)), GateUnsolvable, @@ -28,7 +28,7 @@ impl ArithmeticSolver { pub fn solve( initial_witness: &mut BTreeMap, gate: &Expression, - ) -> Result<(), OpcodeResolutionError> { + ) -> Result { let gate = &ArithmeticSolver::evaluate(gate, initial_witness); // Evaluate multiplication term let mul_result = ArithmeticSolver::solve_mul_term(gate, initial_witness); @@ -36,11 +36,9 @@ impl ArithmeticSolver { let gate_status = ArithmeticSolver::solve_fan_in_term(gate, initial_witness); match (mul_result, gate_status) { - (MulTerm::TooManyUnknowns, _) | (_, GateStatus::GateUnsolvable) => { - Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone()), - )) - } + (MulTerm::TooManyUnknowns, _) | (_, GateStatus::GateUnsolvable) => Ok( + GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone())), + ), (MulTerm::OneUnknown(q, w1), GateStatus::GateSolvable(a, (b, w2))) => { if w1 == w2 { // We have one unknown so we can solve the equation @@ -49,19 +47,19 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(()) + Ok(GateResolution::Resolved) } } else { let assignment = -total_sum / (q + b); // Add this into the witness assignments initial_witness.insert(w1, assignment); - Ok(()) + Ok(GateResolution::Resolved) } } else { // TODO: can we be more specific with this error? - Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone()), - )) + Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( + gate.clone(), + ))) } } (MulTerm::OneUnknown(partial_prod, unknown_var), GateStatus::GateSatisfied(sum)) => { @@ -74,13 +72,13 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(()) + Ok(GateResolution::Resolved) } } else { let assignment = -(total_sum / partial_prod); // Add this into the witness assignments initial_witness.insert(unknown_var, assignment); - Ok(()) + Ok(GateResolution::Resolved) } } (MulTerm::Solved(a), GateStatus::GateSatisfied(b)) => { @@ -89,7 +87,7 @@ impl ArithmeticSolver { if !(a + b + gate.q_c).is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(()) + Ok(GateResolution::Resolved) } } ( @@ -104,13 +102,13 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(()) + Ok(GateResolution::Resolved) } } else { let assignment = -(total_sum / coeff); // Add this into the witness assignments initial_witness.insert(unknown_var, assignment); - Ok(()) + Ok(GateResolution::Resolved) } } } @@ -166,7 +164,7 @@ impl ArithmeticSolver { /// Returns the summation of all of the variables, plus the unknown variable /// Returns None, if there is more than one unknown variable /// We cannot assign - fn solve_fan_in_term( + pub fn solve_fan_in_term( arith_gate: &Expression, witness_assignments: &BTreeMap, ) -> GateStatus { @@ -270,8 +268,8 @@ fn arithmetic_smoke_test() { values.insert(c, FieldElement::from(1_i128)); values.insert(d, FieldElement::from(1_i128)); - assert_eq!(ArithmeticSolver::solve(&mut values, &gate_a), Ok(())); - assert_eq!(ArithmeticSolver::solve(&mut values, &gate_b), Ok(())); + assert_eq!(ArithmeticSolver::solve(&mut values, &gate_a), Ok(GateResolution::Resolved)); + assert_eq!(ArithmeticSolver::solve(&mut values, &gate_b), Ok(GateResolution::Resolved)); assert_eq!(values.get(&a).unwrap(), &FieldElement::from(4_i128)); } diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs new file mode 100644 index 000000000..2c35b27c3 --- /dev/null +++ b/acvm/src/pwg/block.rs @@ -0,0 +1,187 @@ +use std::collections::{BTreeMap, HashMap}; + +use acir::{ + circuit::opcodes::{BlockId, BlockOp}, + native_types::Witness, + FieldElement, +}; + +use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; + +use super::{ + arithmetic::{ArithmeticSolver, GateStatus}, + directives::insert_witness, + expression_to_const, +}; + +#[derive(Default)] +pub struct Blocks { + blocks: HashMap, +} + +impl Blocks { + pub fn solve( + &mut self, + id: BlockId, + trace: &Vec, + solved_witness: &mut BTreeMap, + ) -> Result { + let solver = self.blocks.entry(id).or_default(); + solver.solve(solved_witness, trace) + } +} + +#[derive(Default)] +struct BlockSolver { + block_value: HashMap, + solved_operations: usize, +} + +impl BlockSolver { + fn insert_value( + &mut self, + index: u32, + value: FieldElement, + ) -> Result<(), OpcodeResolutionError> { + let entry = self.block_value.entry(index); + match entry { + std::collections::hash_map::Entry::Vacant(e) => { + e.insert(value); + } + std::collections::hash_map::Entry::Occupied(e) => { + if e.get() != &value { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + } + } + Ok(()) + } + + fn get_value(&self, index: u32) -> Option { + self.block_value.get(&index).copied() + } + + // Try to solve block operations from the trace + // As long as operations are resolved, we update/read from the block_value + // We stop when an operation cannot be resolved + pub fn solve( + &mut self, + initial_witness: &mut BTreeMap, + trace: &Vec, + ) -> Result { + let mut unknown = None; + while self.solved_operations < trace.len() { + let op_idx = self.solved_operations; + let op_expr = ArithmeticSolver::evaluate(&trace[op_idx].operation, initial_witness); + if let Some(operation) = expression_to_const(&op_expr) { + let index_expr = ArithmeticSolver::evaluate(&trace[op_idx].index, initial_witness); + if let Some(index) = expression_to_const(&index_expr) { + let index = index.try_to_u64().unwrap() as u32; + if operation == FieldElement::zero() { + let value = + ArithmeticSolver::evaluate(&trace[op_idx].value, initial_witness); + unknown = value.get_witness(); + if value.is_const() { + self.insert_value(index, value.q_c)?; + } else if value.is_linear() { + match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { + GateStatus::GateUnsolvable => break, + GateStatus::GateSolvable(sum, (coef, w)) => { + let map_value = self.get_value(index); + if let Some(map_value) = map_value { + insert_witness( + w, + (map_value - sum - value.q_c) / coef, + initial_witness, + )?; + } else { + unknown = Some(w); + break; + } + } + GateStatus::GateSatisfied(sum) => { + self.insert_value(index, sum + value.q_c)?; + } + } + } else { + break; + } + } else { + let value = + ArithmeticSolver::evaluate(&trace[op_idx].value, initial_witness); + if value.is_const() { + self.insert_value(index, value.q_c)?; + } else { + break; + } + } + } else { + unknown = index_expr.get_witness(); + break; + } + } else { + unknown = op_expr.get_witness(); + break; + } + self.solved_operations += 1; + } + if self.solved_operations == trace.len() { + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(unknown.unwrap().0))) + } + } +} + +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use acir::{ + circuit::opcodes::{BlockId, BlockOp}, + native_types::{Expression, Witness}, + FieldElement, + }; + + use crate::pwg::directives::insert_witness; + + use super::Blocks; + + #[test] + fn test_solver() { + let mut index = FieldElement::zero(); + let mut trace = vec![BlockOp { + operation: Expression::one(), + index: Expression::from_field(index), + value: Expression::from(&Witness(1)), + }]; + index += FieldElement::one(); + trace.push(BlockOp { + operation: Expression::one(), + index: Expression::from_field(index), + value: Expression::from(&Witness(2)), + }); + index += FieldElement::one(); + trace.push(BlockOp { + operation: Expression::one(), + index: Expression::from_field(index), + value: Expression::from(&Witness(3)), + }); + trace.push(BlockOp { + operation: Expression::zero(), + index: Expression::one(), + value: Expression::from(&Witness(4)), + }); + let id = BlockId::default(); + let mut initial_witness = BTreeMap::new(); + let mut value = FieldElement::zero(); + insert_witness(Witness(1), value, &mut initial_witness).unwrap(); + value = FieldElement::one(); + insert_witness(Witness(2), value, &mut initial_witness).unwrap(); + value = value + value; + insert_witness(Witness(3), value, &mut initial_witness).unwrap(); + let mut blocks = Blocks::default(); + blocks.solve(id, &mut trace, &mut initial_witness).unwrap(); + assert_eq!(initial_witness[&Witness(4)], FieldElement::one()); + } +} diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 58da96696..5830fa521 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -8,32 +8,55 @@ use acir::{ use num_bigint::BigUint; use num_traits::{One, Zero}; -use crate::{OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; use super::{get_value, insert_value, sorting::route, witness_to_value}; pub fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, -) -> Result<(), OpcodeResolutionError> { +) -> Result { match directive { Directive::Invert { x, result } => { - let val = witness_to_value(initial_witness, *x)?; - let inverse = val.inverse(); - initial_witness.insert(*result, inverse); - Ok(()) + let val = witness_to_value(initial_witness, *x); + if let Some(val) = val { + let inverse = val.inverse(); + initial_witness.insert(*result, inverse); + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(x.0))) + } } Directive::Quotient { a, b, q, r, predicate } => { - let val_a = get_value(a, initial_witness)?; - let val_b = get_value(b, initial_witness)?; - + let val_a = get_value(a, initial_witness); + let val_b = get_value(b, initial_witness); + if val_a.is_none() { + return Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( + a.clone(), + ))); + } + if val_b.is_none() { + return Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( + b.clone(), + ))); + } + let val_a = val_a.unwrap(); + let val_b = val_b.unwrap(); let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); let int_b = BigUint::from_bytes_be(&val_b.to_be_bytes()); // 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 unresolved let pred_value = match predicate { - Some(pred) => get_value(pred, initial_witness)?, + Some(pred) => { + if let Some(pred) = get_value(pred, initial_witness) { + pred + } else { + return Ok(GateResolution::Skip( + OpcodeNotSolvable::ExpressionHasTooManyUnknowns(pred.clone()), + )); + } + } None => FieldElement::one(), }; @@ -54,106 +77,111 @@ pub fn solve_directives( initial_witness, )?; - Ok(()) + Ok(GateResolution::Resolved) } Directive::Truncate { a, b, c, bit_size } => { - let val_a = get_value(a, initial_witness)?; - - let pow: BigUint = BigUint::one() << bit_size; - - let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let int_b: BigUint = &int_a % &pow; - let int_c: BigUint = (&int_a - &int_b) / &pow; - - insert_witness( - *b, - FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), - initial_witness, - )?; - insert_witness( - *c, - FieldElement::from_be_bytes_reduce(&int_c.to_bytes_be()), - initial_witness, - )?; - - Ok(()) + if let Some(val_a) = get_value(a, initial_witness) { + let pow: BigUint = BigUint::one() << bit_size; + + let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); + let int_b: BigUint = &int_a % &pow; + let int_c: BigUint = (&int_a - &int_b) / &pow; + + insert_witness( + *b, + FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), + initial_witness, + )?; + insert_witness( + *c, + FieldElement::from_be_bytes_reduce(&int_c.to_bytes_be()), + initial_witness, + )?; + + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(a.clone()))) + } } Directive::ToRadix { a, b, radix, is_little_endian } => { - let value_a = get_value(a, initial_witness)?; + if let Some(value_a) = get_value(a, initial_witness) { + let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); - let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); + if *is_little_endian { + // Decompose the integer into its radix digits in little endian form. + let decomposed_integer = big_integer.to_radix_le(*radix); - if *is_little_endian { - // Decompose the integer into its radix digits in little endian form. - let decomposed_integer = big_integer.to_radix_le(*radix); - - if b.len() < decomposed_integer.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - - for (i, witness) in b.iter().enumerate() { - // Fetch the `i'th` digit from the decomposed integer list - // and convert it to a field element. - // If it is not available, which can happen when the decomposed integer - // list is shorter than the witness list, we return 0. - let value = match decomposed_integer.get(i) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), - }; - - insert_value(witness, value, initial_witness)? - } - } else { - // Decompose the integer into its radix digits in big endian form. - let decomposed_integer = big_integer.to_radix_be(*radix); + if b.len() < decomposed_integer.len() { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } - // if it is big endian and the decompoased integer list is shorter - // than the witness list, pad the extra part with 0 first then - // add the decompsed interger list to the witness list. - let padding_len = b.len() - decomposed_integer.len(); - let mut value = FieldElement::zero(); - for (i, witness) in b.iter().enumerate() { - if i >= padding_len { - value = match decomposed_integer.get(i - padding_len) { + for (i, witness) in b.iter().enumerate() { + // Fetch the `i'th` digit from the decomposed integer list + // and convert it to a field element. + // If it is not available, which can happen when the decomposed integer + // list is shorter than the witness list, we return 0. + let value = match decomposed_integer.get(i) { Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::UnreachableCode, - )) - } + None => FieldElement::zero(), }; + + insert_value(witness, value, initial_witness)? + } + } else { + // Decompose the integer into its radix digits in big endian form. + let decomposed_integer = big_integer.to_radix_be(*radix); + + // if it is big endian and the decompoased integer list is shorter + // than the witness list, pad the extra part with 0 first then + // add the decompsed interger list to the witness list. + let padding_len = b.len() - decomposed_integer.len(); + let mut value = FieldElement::zero(); + for (i, witness) in b.iter().enumerate() { + if i >= padding_len { + value = match decomposed_integer.get(i - padding_len) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::UnreachableCode, + )) + } + }; + } + insert_value(witness, value, initial_witness)? } - insert_value(witness, value, initial_witness)? } + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(a.clone()))) } - - Ok(()) } Directive::OddRange { a, b, r, bit_size } => { - let val_a = witness_to_value(initial_witness, *a)?; + if let Some(val_a) = witness_to_value(initial_witness, *a) { + let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); + let pow: BigUint = BigUint::one() << (bit_size - 1); + if int_a >= (&pow << 1) { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } - let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let pow: BigUint = BigUint::one() << (bit_size - 1); - if int_a >= (&pow << 1) { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); + let bb = &int_a & &pow; + let int_r = &int_a - &bb; + let int_b = &bb >> (bit_size - 1); + + insert_witness( + *b, + FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), + initial_witness, + )?; + insert_witness( + *r, + FieldElement::from_be_bytes_reduce(&int_r.to_bytes_be()), + initial_witness, + )?; + + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(a.0))) } - - let bb = &int_a & &pow; - let int_r = &int_a - &bb; - let int_b = &bb >> (bit_size - 1); - - insert_witness( - *b, - FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), - initial_witness, - )?; - insert_witness( - *r, - FieldElement::from_be_bytes_reduce(&int_r.to_bytes_be()), - initial_witness, - )?; - - Ok(()) } Directive::PermutationSort { inputs: a, tuple, bits, sort_by } => { let mut val_a = Vec::new(); @@ -162,7 +190,13 @@ pub fn solve_directives( assert_eq!(element.len(), *tuple as usize); let mut element_val = Vec::with_capacity(*tuple as usize + 1); for e in element { - element_val.push(get_value(e, initial_witness)?); + if let Some(e_val) = get_value(e, initial_witness) { + element_val.push(e_val); + } else { + return Ok(GateResolution::Skip( + OpcodeNotSolvable::ExpressionHasTooManyUnknowns(e.clone()), + )); + } } let field_i = FieldElement::from(i as i128); element_val.push(field_i); @@ -186,23 +220,27 @@ pub fn solve_directives( let value = if value { FieldElement::one() } else { FieldElement::zero() }; insert_witness(*w, value, initial_witness)?; } - Ok(()) + Ok(GateResolution::Resolved) } Directive::Log(info) => { let witnesses = match info { LogInfo::FinalizedOutput(output_string) => { println!("{output_string}"); - return Ok(()); + return Ok(GateResolution::Resolved); } LogInfo::WitnessOutput(witnesses) => witnesses, }; if witnesses.len() == 1 { let witness = &witnesses[0]; - let log_value = witness_to_value(initial_witness, *witness)?; - println!("{}", format_field_string(*log_value)); - - return Ok(()); + if let Some(log_value) = witness_to_value(initial_witness, *witness) { + println!("{}", format_field_string(*log_value)); + return Ok(GateResolution::Resolved); + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + witness.0, + ))); + } } // If multiple witnesses are to be fetched for a log directive, @@ -212,8 +250,13 @@ pub fn solve_directives( // and convert them to hex strings. let mut elements_as_hex = Vec::with_capacity(witnesses.len()); for witness in witnesses { - let element = witness_to_value(initial_witness, *witness)?; - elements_as_hex.push(format_field_string(*element)); + if let Some(element) = witness_to_value(initial_witness, *witness) { + elements_as_hex.push(format_field_string(*element)); + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + witness.0, + ))); + } } // Join all of the hex strings using a comma @@ -223,12 +266,12 @@ pub fn solve_directives( println!("{output_witnesses_string}"); - Ok(()) + Ok(GateResolution::Resolved) } } } -fn insert_witness( +pub fn insert_witness( w: Witness, value: FieldElement, initial_witness: &mut BTreeMap, diff --git a/acvm/src/pwg/logic.rs b/acvm/src/pwg/logic.rs index d4bcb5285..680653f8e 100644 --- a/acvm/src/pwg/logic.rs +++ b/acvm/src/pwg/logic.rs @@ -1,12 +1,12 @@ -use super::witness_to_value; -use crate::OpcodeResolutionError; +use super::{directives::insert_witness, witness_to_value}; +use crate::{GateResolution, OpcodeResolutionError}; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; pub fn solve_logic_opcode( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, -) -> Result<(), OpcodeResolutionError> { +) -> Result { match func_call.name { BlackBoxFunc::AND => LogicSolver::solve_and_gate(initial_witness, func_call), BlackBoxFunc::XOR => LogicSolver::solve_xor_gate(initial_witness, func_call), @@ -25,31 +25,37 @@ impl LogicSolver { result: Witness, num_bits: u32, is_xor_gate: bool, - ) -> Result<(), OpcodeResolutionError> { - let w_l_value = witness_to_value(initial_witness, *a)?; - let w_r_value = witness_to_value(initial_witness, *b)?; - - if is_xor_gate { - let assignment = w_l_value.xor(w_r_value, num_bits); - initial_witness.insert(result, assignment); - } else { - let assignment = w_l_value.and(w_r_value, num_bits); - initial_witness.insert(result, assignment); + ) -> Result { + let w_l_value = witness_to_value(initial_witness, *a); + let w_r_value = witness_to_value(initial_witness, *b); + if w_l_value.is_none() { + return Ok(GateResolution::Skip(crate::OpcodeNotSolvable::MissingAssignment(a.0))); } - Ok(()) + if w_r_value.is_none() { + return Ok(GateResolution::Skip(crate::OpcodeNotSolvable::MissingAssignment(b.0))); + } + let w_l_value = w_l_value.unwrap(); + let w_r_value = w_r_value.unwrap(); + let assignment = if is_xor_gate { + w_l_value.xor(w_r_value, num_bits) + } else { + w_l_value.and(w_r_value, num_bits) + }; + insert_witness(result, assignment, initial_witness)?; + Ok(GateResolution::Resolved) } pub fn solve_and_gate( initial_witness: &mut BTreeMap, gate: &BlackBoxFuncCall, - ) -> Result<(), OpcodeResolutionError> { + ) -> Result { let (a, b, result, num_bits) = extract_input_output(gate); LogicSolver::solve_logic_gate(initial_witness, &a, &b, result, num_bits, false) } pub fn solve_xor_gate( initial_witness: &mut BTreeMap, gate: &BlackBoxFuncCall, - ) -> Result<(), OpcodeResolutionError> { + ) -> Result { let (a, b, result, num_bits) = extract_input_output(gate); LogicSolver::solve_logic_gate(initial_witness, &a, &b, result, num_bits, true) } diff --git a/acvm/src/pwg/range.rs b/acvm/src/pwg/range.rs index bddac3490..90ac89da9 100644 --- a/acvm/src/pwg/range.rs +++ b/acvm/src/pwg/range.rs @@ -1,11 +1,11 @@ -use crate::{pwg::witness_to_value, OpcodeResolutionError}; +use crate::{pwg::witness_to_value, GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; pub fn solve_range_opcode( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, -) -> Result<(), OpcodeResolutionError> { +) -> Result { // TODO: this consistency check can be moved to a general function let defined_input_size = BlackBoxFunc::RANGE .definition() @@ -27,11 +27,12 @@ pub fn solve_range_opcode( let input = func_call.inputs.first().expect("infallible: checked that input size is 1"); - let w_value = witness_to_value(initial_witness, input.witness)?; - - if w_value.num_bits() > input.num_bits { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); + if let Some(w_value) = witness_to_value(initial_witness, input.witness) { + if w_value.num_bits() > input.num_bits { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + Ok(GateResolution::Resolved) + } else { + Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(input.witness.0))) } - - Ok(()) } diff --git a/acvm/src/pwg/signature/ecdsa.rs b/acvm/src/pwg/signature/ecdsa.rs index 2714ec0a8..5db9acf9b 100644 --- a/acvm/src/pwg/signature/ecdsa.rs +++ b/acvm/src/pwg/signature/ecdsa.rs @@ -1,12 +1,12 @@ use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, FieldElement}; use std::collections::BTreeMap; -use crate::{pwg::witness_to_value, OpcodeResolutionError}; +use crate::{pwg::witness_to_value, GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; pub fn secp256k1_prehashed( initial_witness: &mut BTreeMap, gadget_call: &BlackBoxFuncCall, -) -> Result<(), OpcodeResolutionError> { +) -> Result { let mut inputs_iter = gadget_call.inputs.iter(); let mut pub_key_x = [0u8; 32]; @@ -15,8 +15,11 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_x should be 32 bytes long, found only {i} bytes")); - let x_i = witness_to_value(initial_witness, _x_i.witness)?; - *pkx = *x_i.to_be_bytes().last().unwrap() + if let Some(x_i) = witness_to_value(initial_witness, _x_i.witness) { + *pkx = *x_i.to_be_bytes().last().unwrap(); + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(_x_i.witness.0))); + } } let mut pub_key_y = [0u8; 32]; @@ -25,8 +28,11 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_y should be 32 bytes long, found only {i} bytes")); - let y_i = witness_to_value(initial_witness, _y_i.witness)?; - *pky = *y_i.to_be_bytes().last().unwrap() + if let Some(y_i) = witness_to_value(initial_witness, _y_i.witness) { + *pky = *y_i.to_be_bytes().last().unwrap(); + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(_y_i.witness.0))); + } } let mut signature = [0u8; 64]; @@ -35,15 +41,23 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("signature should be 64 bytes long, found only {i} bytes")); - let sig_i = witness_to_value(initial_witness, _sig_i.witness)?; - *sig = *sig_i.to_be_bytes().last().unwrap() + if let Some(sig_i) = witness_to_value(initial_witness, _sig_i.witness) { + *sig = *sig_i.to_be_bytes().last().unwrap() + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + _sig_i.witness.0, + ))); + } } let mut hashed_message = Vec::new(); for msg in inputs_iter { - let msg_i_field = witness_to_value(initial_witness, msg.witness)?; - let msg_i = *msg_i_field.to_be_bytes().last().unwrap(); - hashed_message.push(msg_i); + if let Some(msg_i_field) = witness_to_value(initial_witness, msg.witness) { + let msg_i = *msg_i_field.to_be_bytes().last().unwrap(); + hashed_message.push(msg_i); + } else { + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(msg.witness.0))); + } } let result = @@ -56,7 +70,7 @@ pub fn secp256k1_prehashed( }; initial_witness.insert(gadget_call.outputs[0], result); - Ok(()) + Ok(GateResolution::Resolved) } mod ecdsa_secp256k1 { From 8ca6f0b72e3632019fca23e60717e907986961d3 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 23 Feb 2023 09:06:52 +0000 Subject: [PATCH 02/16] Code review --- acvm/src/lib.rs | 4 +-- acvm/src/pwg/block.rs | 68 +++++++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index 7462635ee..77fcf5341 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -68,7 +68,7 @@ pub trait PartialWitnessGenerator { mut gates_to_resolve: Vec, ) -> Result<(), OpcodeResolutionError> { let mut unresolved_gates: Vec = Vec::new(); - let mut blocker = Blocks::default(); + let mut blocks = Blocks::default(); while !gates_to_resolve.is_empty() { unresolved_gates.clear(); @@ -81,7 +81,7 @@ pub trait PartialWitnessGenerator { Opcode::Directive(directive) => { Self::solve_directives(initial_witness, directive) } - Opcode::Block(id, trace) => blocker.solve(*id, trace, initial_witness), + Opcode::Block(id, trace) => blocks.solve(*id, trace, initial_witness), }?; match result { GateResolution::Skip(_) => unresolved_gates.push(gate.clone()), diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 2c35b27c3..9e544ef92 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -23,7 +23,7 @@ impl Blocks { pub fn solve( &mut self, id: BlockId, - trace: &Vec, + trace: &[BlockOp], solved_witness: &mut BTreeMap, ) -> Result { let solver = self.blocks.entry(id).or_default(); @@ -31,6 +31,9 @@ impl Blocks { } } +/// Maintains the state for solving Block opcode +/// block_value is the value of the Block at the solved_operations step +/// solved_operations is the number of solved elements in the block #[derive(Default)] struct BlockSolver { block_value: HashMap, @@ -67,25 +70,29 @@ impl BlockSolver { pub fn solve( &mut self, initial_witness: &mut BTreeMap, - trace: &Vec, + trace: &[BlockOp], ) -> Result { - let mut unknown = None; - while self.solved_operations < trace.len() { - let op_idx = self.solved_operations; - let op_expr = ArithmeticSolver::evaluate(&trace[op_idx].operation, initial_witness); + for block_op in trace.iter().skip(self.solved_operations) { + let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); if let Some(operation) = expression_to_const(&op_expr) { - let index_expr = ArithmeticSolver::evaluate(&trace[op_idx].index, initial_witness); + let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); if let Some(index) = expression_to_const(&index_expr) { let index = index.try_to_u64().unwrap() as u32; - if operation == FieldElement::zero() { - let value = - ArithmeticSolver::evaluate(&trace[op_idx].value, initial_witness); - unknown = value.get_witness(); + let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); + let value_witness = value.get_witness(); + if operation.is_zero() { + let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); if value.is_const() { self.insert_value(index, value.q_c)?; } else if value.is_linear() { match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { - GateStatus::GateUnsolvable => break, + GateStatus::GateUnsolvable => { + return Ok(GateResolution::Skip( + OpcodeNotSolvable::MissingAssignment( + value_witness.unwrap().0, + ), + )) + } GateStatus::GateSolvable(sum, (coef, w)) => { let map_value = self.get_value(index); if let Some(map_value) = map_value { @@ -95,8 +102,9 @@ impl BlockSolver { initial_witness, )?; } else { - unknown = Some(w); - break; + return Ok(GateResolution::Skip( + OpcodeNotSolvable::MissingAssignment(w.0), + )); } } GateStatus::GateSatisfied(sum) => { @@ -104,32 +112,30 @@ impl BlockSolver { } } } else { - break; + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + value_witness.unwrap().0, + ))); } + } else if value.is_const() { + self.insert_value(index, value.q_c)?; } else { - let value = - ArithmeticSolver::evaluate(&trace[op_idx].value, initial_witness); - if value.is_const() { - self.insert_value(index, value.q_c)?; - } else { - break; - } + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + value_witness.unwrap().0, + ))); } } else { - unknown = index_expr.get_witness(); - break; + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + index_expr.get_witness().unwrap().0, + ))); } } else { - unknown = op_expr.get_witness(); - break; + return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( + op_expr.get_witness().unwrap().0, + ))); } self.solved_operations += 1; } - if self.solved_operations == trace.len() { - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(unknown.unwrap().0))) - } + Ok(GateResolution::Resolved) } } From e8644dbb6e32286e68d0f22a4b41189301f4c778 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 23 Feb 2023 10:49:19 +0000 Subject: [PATCH 03/16] rename blockop into memop --- acir/src/circuit/opcodes.rs | 6 +++--- acvm/src/pwg/block.rs | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index cb9bb4cbf..0e73ad2d0 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -12,7 +12,7 @@ pub struct BlockId(u32); /// Operation on a block /// We can either write or read at a block index #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct BlockOp { +pub struct MemOp { pub operation: Expression, pub index: Expression, pub value: Expression, @@ -23,7 +23,7 @@ pub enum Opcode { Arithmetic(Expression), BlackBoxFuncCall(BlackBoxFuncCall), Directive(Directive), - Block(BlockId, Vec), + Block(BlockId, Vec), } impl Opcode { @@ -107,7 +107,7 @@ impl Opcode { let operation = Expression::read(&mut reader)?; let index = Expression::read(&mut reader)?; let value = Expression::read(&mut reader)?; - trace.push(BlockOp { operation, index, value }); + trace.push(MemOp { operation, index, value }); } Ok(Opcode::Block(BlockId(id), trace)) } diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 9e544ef92..d191363bb 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -1,7 +1,7 @@ use std::collections::{BTreeMap, HashMap}; use acir::{ - circuit::opcodes::{BlockId, BlockOp}, + circuit::opcodes::{BlockId, MemOp}, native_types::Witness, FieldElement, }; @@ -23,7 +23,7 @@ impl Blocks { pub fn solve( &mut self, id: BlockId, - trace: &[BlockOp], + trace: &[MemOp], solved_witness: &mut BTreeMap, ) -> Result { let solver = self.blocks.entry(id).or_default(); @@ -70,7 +70,7 @@ impl BlockSolver { pub fn solve( &mut self, initial_witness: &mut BTreeMap, - trace: &[BlockOp], + trace: &[MemOp], ) -> Result { for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); @@ -144,7 +144,7 @@ mod test { use std::collections::BTreeMap; use acir::{ - circuit::opcodes::{BlockId, BlockOp}, + circuit::opcodes::{BlockId, MemOp}, native_types::{Expression, Witness}, FieldElement, }; @@ -156,24 +156,24 @@ mod test { #[test] fn test_solver() { let mut index = FieldElement::zero(); - let mut trace = vec![BlockOp { + let mut trace = vec![MemOp { operation: Expression::one(), index: Expression::from_field(index), value: Expression::from(&Witness(1)), }]; index += FieldElement::one(); - trace.push(BlockOp { + trace.push(MemOp { operation: Expression::one(), index: Expression::from_field(index), value: Expression::from(&Witness(2)), }); index += FieldElement::one(); - trace.push(BlockOp { + trace.push(MemOp { operation: Expression::one(), index: Expression::from_field(index), value: Expression::from(&Witness(3)), }); - trace.push(BlockOp { + trace.push(MemOp { operation: Expression::zero(), index: Expression::one(), value: Expression::from(&Witness(4)), From ee5a39da021adbe2abaacfe82d570a7aafb04d6f Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 23 Feb 2023 17:30:56 +0000 Subject: [PATCH 04/16] Code review --- acir/src/circuit/opcodes.rs | 1 + acir/src/native_types/arithmetic.rs | 5 +++-- acvm/src/pwg/block.rs | 7 ++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 0e73ad2d0..79d15e6f7 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -23,6 +23,7 @@ pub enum Opcode { Arithmetic(Expression), BlackBoxFuncCall(BlackBoxFuncCall), Directive(Directive), + // Abstract read/write operations on a block of data Block(BlockId, Vec), } diff --git a/acir/src/native_types/arithmetic.rs b/acir/src/native_types/arithmetic.rs index dfbc9cd0f..126b6d3a4 100644 --- a/acir/src/native_types/arithmetic.rs +++ b/acir/src/native_types/arithmetic.rs @@ -153,8 +153,9 @@ impl Expression { self.mul_terms.is_empty() } - // Returns a witness belonging to the expression - pub fn get_witness(&self) -> Option { + // Returns one witness belonging to the expression, in no relevant order + // Returns None if the expression is const + pub fn any_witness(&self) -> Option { if self.linear_combinations.is_empty() { if self.mul_terms.is_empty() { None diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index d191363bb..267d84bb0 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -14,6 +14,7 @@ use super::{ expression_to_const, }; +/// Maps a block to its emulated state #[derive(Default)] pub struct Blocks { blocks: HashMap, @@ -79,7 +80,7 @@ impl BlockSolver { if let Some(index) = expression_to_const(&index_expr) { let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); - let value_witness = value.get_witness(); + let value_witness = value.any_witness(); if operation.is_zero() { let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); if value.is_const() { @@ -125,12 +126,12 @@ impl BlockSolver { } } else { return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - index_expr.get_witness().unwrap().0, + index_expr.any_witness().unwrap().0, ))); } } else { return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - op_expr.get_witness().unwrap().0, + op_expr.any_witness().unwrap().0, ))); } self.solved_operations += 1; From ff662f2ae4c30c8b2ebc865c7b6d77d4656cfe30 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 Feb 2023 11:24:15 +0000 Subject: [PATCH 05/16] Remove the GateResolution change from the branch --- acvm/src/lib.rs | 37 +++-- acvm/src/pwg.rs | 33 ++++- acvm/src/pwg/arithmetic.rs | 36 ++--- acvm/src/pwg/block.rs | 109 +++++++-------- acvm/src/pwg/directives.rs | 239 +++++++++++++------------------- acvm/src/pwg/logic.rs | 27 ++-- acvm/src/pwg/range.rs | 15 +- acvm/src/pwg/signature/ecdsa.rs | 38 ++--- 8 files changed, 241 insertions(+), 293 deletions(-) diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index 43dee090a..b9cdc3e4e 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -50,12 +50,6 @@ pub enum OpcodeResolutionError { IncorrectNumFunctionArguments(usize, BlackBoxFunc, usize), } -#[derive(PartialEq, Eq, Debug)] -pub enum GateResolution { - Resolved, // Gate is solved - Skip(OpcodeNotSolvable), // Gate cannot be solved -} - pub trait Backend: SmartContract + ProofSystemCompiler + PartialWitnessGenerator {} /// This component will generate the backend specific output for @@ -65,15 +59,15 @@ pub trait PartialWitnessGenerator { fn solve( &self, initial_witness: &mut BTreeMap, - mut gates_to_resolve: Vec, + mut opcode_to_solve: Vec, ) -> Result<(), OpcodeResolutionError> { - let mut unresolved_gates: Vec = Vec::new(); + let mut unresolved_opcodes: Vec = Vec::new(); let mut blocks = Blocks::default(); - while !gates_to_resolve.is_empty() { - unresolved_gates.clear(); + while !opcode_to_solve.is_empty() { + unresolved_opcodes.clear(); - for gate in &gates_to_resolve { - let resolution = match gate { + for opcode in &opcode_to_solve { + let resolution = match opcode { Opcode::Arithmetic(expr) => ArithmeticSolver::solve(initial_witness, expr), Opcode::BlackBoxFuncCall(bb_func) => { Self::solve_black_box_function_call(initial_witness, bb_func) @@ -82,20 +76,21 @@ pub trait PartialWitnessGenerator { Self::solve_directives(initial_witness, directive) } Opcode::Block(id, trace) => blocks.solve(*id, trace, initial_witness), - }?; + }; match resolution { - GateResolution::Skip(_) => { + Ok(_) => { + // We do nothing in the happy case + } + Err(OpcodeResolutionError::OpcodeNotSolvable(_)) => { // For opcode not solvable errors, we push those opcodes to the back as // it could be because the opcodes are out of order, i.e. this assignment // relies on a later opcodes' results - unresolved_gates.push(gate.clone()) - } - GateResolution::Resolved => { - // We do nothing in the happy case + unresolved_opcodes.push(opcode.clone()); } + Err(err) => return Err(err), } } - std::mem::swap(&mut gates_to_resolve, &mut unresolved_gates); + std::mem::swap(&mut opcode_to_solve, &mut unresolved_opcodes); } Ok(()) } @@ -103,7 +98,7 @@ pub trait PartialWitnessGenerator { fn solve_black_box_function_call( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, - ) -> Result; + ) -> Result<(), OpcodeResolutionError>; // Check if all of the inputs to the function have assignments // Returns true if all of the inputs have been assigned @@ -119,7 +114,7 @@ pub trait PartialWitnessGenerator { fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { pwg::directives::solve_directives(initial_witness, directive) } } diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index 80c1e5aa9..b47437185 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -1,6 +1,6 @@ // Re-usable methods that backends can use to implement their PWG -use crate::OpcodeResolutionError; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; use acir::{ native_types::{Expression, Witness}, FieldElement, @@ -22,8 +22,7 @@ pub mod signature; pub mod sorting; // Returns the concrete value for a particular witness -// If the witness has no assignment, then -// an error is returned +// Returns None if the witness has no assignment pub fn witness_to_value( initial_witness: &BTreeMap, witness: Witness, @@ -31,6 +30,21 @@ pub fn witness_to_value( initial_witness.get(&witness) } +// Returns the concrete value for a particular witness +// If the witness has no assignment, then +// an error is returned +pub fn witness_to_value_unwrap( + initial_witness: &BTreeMap, + witness: Witness, +) -> Result<&FieldElement, OpcodeResolutionError> { + match initial_witness.get(&witness) { + Some(value) => Ok(value), + None => Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(witness.0), + )), + } +} + pub fn expression_to_const(expr: &Expression) -> Option { expr.is_const().then_some(expr.q_c) } @@ -44,6 +58,19 @@ pub fn get_value( expression_to_const(&ArithmeticSolver::evaluate(expr, initial_witness)) } +pub fn get_value_unwrap( + expr: &Expression, + initial_witness: &BTreeMap, +) -> Result { + let expr = ArithmeticSolver::evaluate(expr, initial_witness); + match expression_to_const(&expr) { + Some(value) => Ok(value), + None => Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(expr.any_witness().unwrap().0), + )), + } +} + // Inserts `value` into the initial witness map // under the key of `witness`. // Returns an error, if there was already a value in the map diff --git a/acvm/src/pwg/arithmetic.rs b/acvm/src/pwg/arithmetic.rs index 466fd1c8d..be7549eb5 100644 --- a/acvm/src/pwg/arithmetic.rs +++ b/acvm/src/pwg/arithmetic.rs @@ -4,7 +4,7 @@ use acir::{ }; use std::collections::BTreeMap; -use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; /// An Arithmetic solver will take a Circuit's arithmetic gates with witness assignments /// and create the other witness variables @@ -28,7 +28,7 @@ impl ArithmeticSolver { pub fn solve( initial_witness: &mut BTreeMap, gate: &Expression, - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { let gate = &ArithmeticSolver::evaluate(gate, initial_witness); // Evaluate multiplication term let mul_result = ArithmeticSolver::solve_mul_term(gate, initial_witness); @@ -36,9 +36,11 @@ impl ArithmeticSolver { let gate_status = ArithmeticSolver::solve_fan_in_term(gate, initial_witness); match (mul_result, gate_status) { - (MulTerm::TooManyUnknowns, _) | (_, GateStatus::GateUnsolvable) => Ok( - GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone())), - ), + (MulTerm::TooManyUnknowns, _) | (_, GateStatus::GateUnsolvable) => { + Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone()), + )) + } (MulTerm::OneUnknown(q, w1), GateStatus::GateSolvable(a, (b, w2))) => { if w1 == w2 { // We have one unknown so we can solve the equation @@ -47,19 +49,19 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(GateResolution::Resolved) + Ok(()) } } else { let assignment = -total_sum / (q + b); // Add this into the witness assignments initial_witness.insert(w1, assignment); - Ok(GateResolution::Resolved) + Ok(()) } } else { // TODO: can we be more specific with this error? - Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( - gate.clone(), - ))) + Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::ExpressionHasTooManyUnknowns(gate.clone()), + )) } } (MulTerm::OneUnknown(partial_prod, unknown_var), GateStatus::GateSatisfied(sum)) => { @@ -72,13 +74,13 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(GateResolution::Resolved) + Ok(()) } } else { let assignment = -(total_sum / partial_prod); // Add this into the witness assignments initial_witness.insert(unknown_var, assignment); - Ok(GateResolution::Resolved) + Ok(()) } } (MulTerm::Solved(a), GateStatus::GateSatisfied(b)) => { @@ -87,7 +89,7 @@ impl ArithmeticSolver { if !(a + b + gate.q_c).is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(GateResolution::Resolved) + Ok(()) } } ( @@ -102,13 +104,13 @@ impl ArithmeticSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain) } else { - Ok(GateResolution::Resolved) + Ok(()) } } else { let assignment = -(total_sum / coeff); // Add this into the witness assignments initial_witness.insert(unknown_var, assignment); - Ok(GateResolution::Resolved) + Ok(()) } } } @@ -268,8 +270,8 @@ fn arithmetic_smoke_test() { values.insert(c, FieldElement::from(1_i128)); values.insert(d, FieldElement::from(1_i128)); - assert_eq!(ArithmeticSolver::solve(&mut values, &gate_a), Ok(GateResolution::Resolved)); - assert_eq!(ArithmeticSolver::solve(&mut values, &gate_b), Ok(GateResolution::Resolved)); + assert_eq!(ArithmeticSolver::solve(&mut values, &gate_a), Ok(())); + assert_eq!(ArithmeticSolver::solve(&mut values, &gate_b), Ok(())); assert_eq!(values.get(&a).unwrap(), &FieldElement::from(4_i128)); } diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 267d84bb0..712b66ed6 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -6,7 +6,7 @@ use acir::{ FieldElement, }; -use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; use super::{ arithmetic::{ArithmeticSolver, GateStatus}, @@ -26,7 +26,7 @@ impl Blocks { id: BlockId, trace: &[MemOp], solved_witness: &mut BTreeMap, - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { let solver = self.blocks.entry(id).or_default(); solver.solve(solved_witness, trace) } @@ -72,71 +72,68 @@ impl BlockSolver { &mut self, initial_witness: &mut BTreeMap, trace: &[MemOp], - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); - if let Some(operation) = expression_to_const(&op_expr) { - let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); - if let Some(index) = expression_to_const(&index_expr) { - let index = index.try_to_u64().unwrap() as u32; - let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); - let value_witness = value.any_witness(); - if operation.is_zero() { - let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); - if value.is_const() { - self.insert_value(index, value.q_c)?; - } else if value.is_linear() { - match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { - GateStatus::GateUnsolvable => { - return Ok(GateResolution::Skip( - OpcodeNotSolvable::MissingAssignment( - value_witness.unwrap().0, - ), - )) - } - GateStatus::GateSolvable(sum, (coef, w)) => { - let map_value = self.get_value(index); - if let Some(map_value) = map_value { - insert_witness( - w, - (map_value - sum - value.q_c) / coef, - initial_witness, - )?; - } else { - return Ok(GateResolution::Skip( - OpcodeNotSolvable::MissingAssignment(w.0), - )); - } - } - GateStatus::GateSatisfied(sum) => { - self.insert_value(index, sum + value.q_c)?; - } + let operation = expression_to_const(&op_expr).ok_or_else(|| { + OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( + op_expr.any_witness().unwrap().0, + )) + })?; + let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); + let index = expression_to_const(&index_expr).ok_or_else(|| { + OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( + index_expr.any_witness().unwrap().0, + )) + })?; + + let index = index.try_to_u64().unwrap() as u32; + let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); + let value_witness = value.any_witness(); + if operation.is_zero() { + let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); + if value.is_const() { + self.insert_value(index, value.q_c)?; + } else if value.is_linear() { + match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { + GateStatus::GateUnsolvable => { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), + )) + } + GateStatus::GateSolvable(sum, (coef, w)) => { + let map_value = self.get_value(index); + if let Some(map_value) = map_value { + insert_witness( + w, + (map_value - sum - value.q_c) / coef, + initial_witness, + )?; + } else { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(w.0), + )); } - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - value_witness.unwrap().0, - ))); } - } else if value.is_const() { - self.insert_value(index, value.q_c)?; - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - value_witness.unwrap().0, - ))); + GateStatus::GateSatisfied(sum) => { + self.insert_value(index, sum + value.q_c)?; + } } } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - index_expr.any_witness().unwrap().0, - ))); + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), + )); } + } else if value.is_const() { + self.insert_value(index, value.q_c)?; } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - op_expr.any_witness().unwrap().0, - ))); + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), + )); } self.solved_operations += 1; } - Ok(GateResolution::Resolved) + Ok(()) } } diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 5830fa521..4bc39e0a9 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -8,55 +8,31 @@ use acir::{ use num_bigint::BigUint; use num_traits::{One, Zero}; -use crate::{GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; -use super::{get_value, insert_value, sorting::route, witness_to_value}; +use super::{get_value_unwrap, insert_value, sorting::route, witness_to_value_unwrap}; pub fn solve_directives( initial_witness: &mut BTreeMap, directive: &Directive, -) -> Result { +) -> Result<(), OpcodeResolutionError> { match directive { Directive::Invert { x, result } => { - let val = witness_to_value(initial_witness, *x); - if let Some(val) = val { - let inverse = val.inverse(); - initial_witness.insert(*result, inverse); - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(x.0))) - } + let val = witness_to_value_unwrap(initial_witness, *x)?; + let inverse = val.inverse(); + initial_witness.insert(*result, inverse); + Ok(()) } Directive::Quotient { a, b, q, r, predicate } => { - let val_a = get_value(a, initial_witness); - let val_b = get_value(b, initial_witness); - if val_a.is_none() { - return Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( - a.clone(), - ))); - } - if val_b.is_none() { - return Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns( - b.clone(), - ))); - } - let val_a = val_a.unwrap(); - let val_b = val_b.unwrap(); + let val_a = get_value_unwrap(a, initial_witness)?; + let val_b = get_value_unwrap(b, initial_witness)?; let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); let int_b = BigUint::from_bytes_be(&val_b.to_be_bytes()); // 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 unresolved let pred_value = match predicate { - Some(pred) => { - if let Some(pred) = get_value(pred, initial_witness) { - pred - } else { - return Ok(GateResolution::Skip( - OpcodeNotSolvable::ExpressionHasTooManyUnknowns(pred.clone()), - )); - } - } + Some(pred) => get_value_unwrap(pred, initial_witness)?, None => FieldElement::one(), }; @@ -77,111 +53,102 @@ pub fn solve_directives( initial_witness, )?; - Ok(GateResolution::Resolved) + Ok(()) } Directive::Truncate { a, b, c, bit_size } => { - if let Some(val_a) = get_value(a, initial_witness) { - let pow: BigUint = BigUint::one() << bit_size; + let val_a = get_value_unwrap(a, initial_witness)?; + let pow: BigUint = BigUint::one() << bit_size; - let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let int_b: BigUint = &int_a % &pow; - let int_c: BigUint = (&int_a - &int_b) / &pow; + let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); + let int_b: BigUint = &int_a % &pow; + let int_c: BigUint = (&int_a - &int_b) / &pow; - insert_witness( - *b, - FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), - initial_witness, - )?; - insert_witness( - *c, - FieldElement::from_be_bytes_reduce(&int_c.to_bytes_be()), - initial_witness, - )?; + insert_witness( + *b, + FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), + initial_witness, + )?; + insert_witness( + *c, + FieldElement::from_be_bytes_reduce(&int_c.to_bytes_be()), + initial_witness, + )?; - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(a.clone()))) - } + Ok(()) } Directive::ToRadix { a, b, radix, is_little_endian } => { - if let Some(value_a) = get_value(a, initial_witness) { - let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); + let value_a = get_value_unwrap(a, initial_witness)?; + let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); - if *is_little_endian { - // Decompose the integer into its radix digits in little endian form. - let decomposed_integer = big_integer.to_radix_le(*radix); + if *is_little_endian { + // Decompose the integer into its radix digits in little endian form. + let decomposed_integer = big_integer.to_radix_le(*radix); - if b.len() < decomposed_integer.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } + if b.len() < decomposed_integer.len() { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } - for (i, witness) in b.iter().enumerate() { - // Fetch the `i'th` digit from the decomposed integer list - // and convert it to a field element. - // If it is not available, which can happen when the decomposed integer - // list is shorter than the witness list, we return 0. - let value = match decomposed_integer.get(i) { + for (i, witness) in b.iter().enumerate() { + // Fetch the `i'th` digit from the decomposed integer list + // and convert it to a field element. + // If it is not available, which can happen when the decomposed integer + // list is shorter than the witness list, we return 0. + let value = match decomposed_integer.get(i) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), + }; + + insert_value(witness, value, initial_witness)? + } + } else { + // Decompose the integer into its radix digits in big endian form. + let decomposed_integer = big_integer.to_radix_be(*radix); + + // if it is big endian and the decompoased integer list is shorter + // than the witness list, pad the extra part with 0 first then + // add the decompsed interger list to the witness list. + let padding_len = b.len() - decomposed_integer.len(); + let mut value = FieldElement::zero(); + for (i, witness) in b.iter().enumerate() { + if i >= padding_len { + value = match decomposed_integer.get(i - padding_len) { Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), + None => { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::UnreachableCode, + )) + } }; - - insert_value(witness, value, initial_witness)? - } - } else { - // Decompose the integer into its radix digits in big endian form. - let decomposed_integer = big_integer.to_radix_be(*radix); - - // if it is big endian and the decompoased integer list is shorter - // than the witness list, pad the extra part with 0 first then - // add the decompsed interger list to the witness list. - let padding_len = b.len() - decomposed_integer.len(); - let mut value = FieldElement::zero(); - for (i, witness) in b.iter().enumerate() { - if i >= padding_len { - value = match decomposed_integer.get(i - padding_len) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::UnreachableCode, - )) - } - }; - } - insert_value(witness, value, initial_witness)? } + insert_value(witness, value, initial_witness)? } - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::ExpressionHasTooManyUnknowns(a.clone()))) } + Ok(()) } Directive::OddRange { a, b, r, bit_size } => { - if let Some(val_a) = witness_to_value(initial_witness, *a) { - let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let pow: BigUint = BigUint::one() << (bit_size - 1); - if int_a >= (&pow << 1) { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } + let val_a = witness_to_value_unwrap(initial_witness, *a)?; + let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); + let pow: BigUint = BigUint::one() << (bit_size - 1); + if int_a >= (&pow << 1) { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } - let bb = &int_a & &pow; - let int_r = &int_a - &bb; - let int_b = &bb >> (bit_size - 1); + let bb = &int_a & &pow; + let int_r = &int_a - &bb; + let int_b = &bb >> (bit_size - 1); - insert_witness( - *b, - FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), - initial_witness, - )?; - insert_witness( - *r, - FieldElement::from_be_bytes_reduce(&int_r.to_bytes_be()), - initial_witness, - )?; + insert_witness( + *b, + FieldElement::from_be_bytes_reduce(&int_b.to_bytes_be()), + initial_witness, + )?; + insert_witness( + *r, + FieldElement::from_be_bytes_reduce(&int_r.to_bytes_be()), + initial_witness, + )?; - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(a.0))) - } + Ok(()) } Directive::PermutationSort { inputs: a, tuple, bits, sort_by } => { let mut val_a = Vec::new(); @@ -190,13 +157,7 @@ pub fn solve_directives( assert_eq!(element.len(), *tuple as usize); let mut element_val = Vec::with_capacity(*tuple as usize + 1); for e in element { - if let Some(e_val) = get_value(e, initial_witness) { - element_val.push(e_val); - } else { - return Ok(GateResolution::Skip( - OpcodeNotSolvable::ExpressionHasTooManyUnknowns(e.clone()), - )); - } + element_val.push(get_value_unwrap(e, initial_witness)?); } let field_i = FieldElement::from(i as i128); element_val.push(field_i); @@ -220,27 +181,22 @@ pub fn solve_directives( let value = if value { FieldElement::one() } else { FieldElement::zero() }; insert_witness(*w, value, initial_witness)?; } - Ok(GateResolution::Resolved) + Ok(()) } Directive::Log(info) => { let witnesses = match info { LogInfo::FinalizedOutput(output_string) => { println!("{output_string}"); - return Ok(GateResolution::Resolved); + return Ok(()); } LogInfo::WitnessOutput(witnesses) => witnesses, }; if witnesses.len() == 1 { let witness = &witnesses[0]; - if let Some(log_value) = witness_to_value(initial_witness, *witness) { - println!("{}", format_field_string(*log_value)); - return Ok(GateResolution::Resolved); - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - witness.0, - ))); - } + let log_value = witness_to_value_unwrap(initial_witness, *witness)?; + println!("{}", format_field_string(*log_value)); + return Ok(()); } // If multiple witnesses are to be fetched for a log directive, @@ -250,13 +206,8 @@ pub fn solve_directives( // and convert them to hex strings. let mut elements_as_hex = Vec::with_capacity(witnesses.len()); for witness in witnesses { - if let Some(element) = witness_to_value(initial_witness, *witness) { - elements_as_hex.push(format_field_string(*element)); - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - witness.0, - ))); - } + let element = witness_to_value_unwrap(initial_witness, *witness)?; + elements_as_hex.push(format_field_string(*element)); } // Join all of the hex strings using a comma @@ -266,7 +217,7 @@ pub fn solve_directives( println!("{output_witnesses_string}"); - Ok(GateResolution::Resolved) + Ok(()) } } } diff --git a/acvm/src/pwg/logic.rs b/acvm/src/pwg/logic.rs index 680653f8e..175b13f62 100644 --- a/acvm/src/pwg/logic.rs +++ b/acvm/src/pwg/logic.rs @@ -1,12 +1,12 @@ -use super::{directives::insert_witness, witness_to_value}; -use crate::{GateResolution, OpcodeResolutionError}; +use super::{directives::insert_witness, witness_to_value_unwrap}; +use crate::OpcodeResolutionError; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; pub fn solve_logic_opcode( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, -) -> Result { +) -> Result<(), OpcodeResolutionError> { match func_call.name { BlackBoxFunc::AND => LogicSolver::solve_and_gate(initial_witness, func_call), BlackBoxFunc::XOR => LogicSolver::solve_xor_gate(initial_witness, func_call), @@ -25,37 +25,30 @@ impl LogicSolver { result: Witness, num_bits: u32, is_xor_gate: bool, - ) -> Result { - let w_l_value = witness_to_value(initial_witness, *a); - let w_r_value = witness_to_value(initial_witness, *b); - if w_l_value.is_none() { - return Ok(GateResolution::Skip(crate::OpcodeNotSolvable::MissingAssignment(a.0))); - } - if w_r_value.is_none() { - return Ok(GateResolution::Skip(crate::OpcodeNotSolvable::MissingAssignment(b.0))); - } - let w_l_value = w_l_value.unwrap(); - let w_r_value = w_r_value.unwrap(); + ) -> Result<(), OpcodeResolutionError> { + let w_l_value = witness_to_value_unwrap(initial_witness, *a)?; + let w_r_value = witness_to_value_unwrap(initial_witness, *b)?; + let assignment = if is_xor_gate { w_l_value.xor(w_r_value, num_bits) } else { w_l_value.and(w_r_value, num_bits) }; insert_witness(result, assignment, initial_witness)?; - Ok(GateResolution::Resolved) + Ok(()) } pub fn solve_and_gate( initial_witness: &mut BTreeMap, gate: &BlackBoxFuncCall, - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { let (a, b, result, num_bits) = extract_input_output(gate); LogicSolver::solve_logic_gate(initial_witness, &a, &b, result, num_bits, false) } pub fn solve_xor_gate( initial_witness: &mut BTreeMap, gate: &BlackBoxFuncCall, - ) -> Result { + ) -> Result<(), OpcodeResolutionError> { let (a, b, result, num_bits) = extract_input_output(gate); LogicSolver::solve_logic_gate(initial_witness, &a, &b, result, num_bits, true) } diff --git a/acvm/src/pwg/range.rs b/acvm/src/pwg/range.rs index 90ac89da9..047149c97 100644 --- a/acvm/src/pwg/range.rs +++ b/acvm/src/pwg/range.rs @@ -1,11 +1,11 @@ -use crate::{pwg::witness_to_value, GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{pwg::witness_to_value_unwrap, OpcodeResolutionError}; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; pub fn solve_range_opcode( initial_witness: &mut BTreeMap, func_call: &BlackBoxFuncCall, -) -> Result { +) -> Result<(), OpcodeResolutionError> { // TODO: this consistency check can be moved to a general function let defined_input_size = BlackBoxFunc::RANGE .definition() @@ -27,12 +27,9 @@ pub fn solve_range_opcode( let input = func_call.inputs.first().expect("infallible: checked that input size is 1"); - if let Some(w_value) = witness_to_value(initial_witness, input.witness) { - if w_value.num_bits() > input.num_bits { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - Ok(GateResolution::Resolved) - } else { - Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(input.witness.0))) + let w_value = witness_to_value_unwrap(initial_witness, input.witness)?; + if w_value.num_bits() > input.num_bits { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); } + Ok(()) } diff --git a/acvm/src/pwg/signature/ecdsa.rs b/acvm/src/pwg/signature/ecdsa.rs index 5db9acf9b..b1888eaf7 100644 --- a/acvm/src/pwg/signature/ecdsa.rs +++ b/acvm/src/pwg/signature/ecdsa.rs @@ -1,12 +1,12 @@ use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, FieldElement}; use std::collections::BTreeMap; -use crate::{pwg::witness_to_value, GateResolution, OpcodeNotSolvable, OpcodeResolutionError}; +use crate::{pwg::witness_to_value_unwrap, OpcodeResolutionError}; pub fn secp256k1_prehashed( initial_witness: &mut BTreeMap, gadget_call: &BlackBoxFuncCall, -) -> Result { +) -> Result<(), OpcodeResolutionError> { let mut inputs_iter = gadget_call.inputs.iter(); let mut pub_key_x = [0u8; 32]; @@ -15,11 +15,8 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_x should be 32 bytes long, found only {i} bytes")); - if let Some(x_i) = witness_to_value(initial_witness, _x_i.witness) { - *pkx = *x_i.to_be_bytes().last().unwrap(); - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(_x_i.witness.0))); - } + let x_i = witness_to_value_unwrap(initial_witness, _x_i.witness)?; + *pkx = *x_i.to_be_bytes().last().unwrap(); } let mut pub_key_y = [0u8; 32]; @@ -28,11 +25,8 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_y should be 32 bytes long, found only {i} bytes")); - if let Some(y_i) = witness_to_value(initial_witness, _y_i.witness) { - *pky = *y_i.to_be_bytes().last().unwrap(); - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(_y_i.witness.0))); - } + let y_i = witness_to_value_unwrap(initial_witness, _y_i.witness)?; + *pky = *y_i.to_be_bytes().last().unwrap(); } let mut signature = [0u8; 64]; @@ -41,23 +35,15 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("signature should be 64 bytes long, found only {i} bytes")); - if let Some(sig_i) = witness_to_value(initial_witness, _sig_i.witness) { - *sig = *sig_i.to_be_bytes().last().unwrap() - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment( - _sig_i.witness.0, - ))); - } + let sig_i = witness_to_value_unwrap(initial_witness, _sig_i.witness)?; + *sig = *sig_i.to_be_bytes().last().unwrap() } let mut hashed_message = Vec::new(); for msg in inputs_iter { - if let Some(msg_i_field) = witness_to_value(initial_witness, msg.witness) { - let msg_i = *msg_i_field.to_be_bytes().last().unwrap(); - hashed_message.push(msg_i); - } else { - return Ok(GateResolution::Skip(OpcodeNotSolvable::MissingAssignment(msg.witness.0))); - } + let msg_i_field = witness_to_value_unwrap(initial_witness, msg.witness)?; + let msg_i = *msg_i_field.to_be_bytes().last().unwrap(); + hashed_message.push(msg_i); } let result = @@ -70,7 +56,7 @@ pub fn secp256k1_prehashed( }; initial_witness.insert(gadget_call.outputs[0], result); - Ok(GateResolution::Resolved) + Ok(()) } mod ecdsa_secp256k1 { From 0052cd331de8bb47758d1592a2232b2e31738cdc Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 Feb 2023 11:36:38 +0000 Subject: [PATCH 06/16] use ok_or instead of a if for readability --- acvm/src/pwg/block.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 712b66ed6..764b305b3 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -86,7 +86,6 @@ impl BlockSolver { index_expr.any_witness().unwrap().0, )) })?; - let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); let value_witness = value.any_witness(); @@ -102,18 +101,16 @@ impl BlockSolver { )) } GateStatus::GateSolvable(sum, (coef, w)) => { - let map_value = self.get_value(index); - if let Some(map_value) = map_value { - insert_witness( - w, - (map_value - sum - value.q_c) / coef, - initial_witness, - )?; - } else { - return Err(OpcodeResolutionError::OpcodeNotSolvable( + let map_value = self.get_value(index).ok_or( + OpcodeResolutionError::OpcodeNotSolvable( OpcodeNotSolvable::MissingAssignment(w.0), - )); - } + ), + )?; + insert_witness( + w, + (map_value - sum - value.q_c) / coef, + initial_witness, + )?; } GateStatus::GateSatisfied(sum) => { self.insert_value(index, sum + value.q_c)?; From 47852ae19a951d6b764840cc8d72d4282c5f5284 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 Feb 2023 17:44:56 +0000 Subject: [PATCH 07/16] Code review --- acvm/src/pwg/block.rs | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 764b305b3..7bec3d416 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -73,19 +73,16 @@ impl BlockSolver { initial_witness: &mut BTreeMap, trace: &[MemOp], ) -> Result<(), OpcodeResolutionError> { + let missing_assignment = |witness| { + OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment(witness)) + }; for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); - let operation = expression_to_const(&op_expr).ok_or_else(|| { - OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( - op_expr.any_witness().unwrap().0, - )) - })?; + let operation = expression_to_const(&op_expr) + .ok_or_else(|| missing_assignment(op_expr.any_witness().unwrap().0))?; let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); - let index = expression_to_const(&index_expr).ok_or_else(|| { - OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( - index_expr.any_witness().unwrap().0, - )) - })?; + let index = expression_to_const(&index_expr) + .ok_or_else(|| missing_assignment(index_expr.any_witness().unwrap().0))?; let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); let value_witness = value.any_witness(); @@ -96,16 +93,10 @@ impl BlockSolver { } else if value.is_linear() { match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { GateStatus::GateUnsolvable => { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), - )) + return Err(missing_assignment(value_witness.unwrap().0)) } GateStatus::GateSolvable(sum, (coef, w)) => { - let map_value = self.get_value(index).ok_or( - OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(w.0), - ), - )?; + let map_value = self.get_value(index).ok_or(missing_assignment(w.0))?; insert_witness( w, (map_value - sum - value.q_c) / coef, @@ -117,16 +108,12 @@ impl BlockSolver { } } } else { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), - )); + return Err(missing_assignment(value_witness.unwrap().0)); } } else if value.is_const() { self.insert_value(index, value.q_c)?; } else { - return Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(value_witness.unwrap().0), - )); + return Err(missing_assignment(value_witness.unwrap().0)); } self.solved_operations += 1; } From 52ba6df3a0cd45bb03b9442cd536051701fd1ffa Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 Feb 2023 17:53:36 +0000 Subject: [PATCH 08/16] Code review --- acvm/src/pwg/block.rs | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 7bec3d416..3cdd04092 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -86,32 +86,21 @@ impl BlockSolver { let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); let value_witness = value.any_witness(); - if operation.is_zero() { - let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); - if value.is_const() { - self.insert_value(index, value.q_c)?; - } else if value.is_linear() { - match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { - GateStatus::GateUnsolvable => { - return Err(missing_assignment(value_witness.unwrap().0)) - } - GateStatus::GateSolvable(sum, (coef, w)) => { - let map_value = self.get_value(index).ok_or(missing_assignment(w.0))?; - insert_witness( - w, - (map_value - sum - value.q_c) / coef, - initial_witness, - )?; - } - GateStatus::GateSatisfied(sum) => { - self.insert_value(index, sum + value.q_c)?; - } + if value.is_const() { + self.insert_value(index, value.q_c)?; + } else if operation.is_zero() && value.is_linear() { + match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { + GateStatus::GateUnsolvable => { + return Err(missing_assignment(value_witness.unwrap().0)) + } + GateStatus::GateSolvable(sum, (coef, w)) => { + let map_value = self.get_value(index).ok_or(missing_assignment(w.0))?; + insert_witness(w, (map_value - sum - value.q_c) / coef, initial_witness)?; + } + GateStatus::GateSatisfied(sum) => { + self.insert_value(index, sum + value.q_c)?; } - } else { - return Err(missing_assignment(value_witness.unwrap().0)); } - } else if value.is_const() { - self.insert_value(index, value.q_c)?; } else { return Err(missing_assignment(value_witness.unwrap().0)); } From 99a597ab7ef0cb6bd78ac2681f7077b4ec3cedb5 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 24 Feb 2023 18:02:40 +0000 Subject: [PATCH 09/16] code review --- acvm/src/pwg/block.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 3cdd04092..81c043fba 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -73,16 +73,18 @@ impl BlockSolver { initial_witness: &mut BTreeMap, trace: &[MemOp], ) -> Result<(), OpcodeResolutionError> { - let missing_assignment = |witness| { - OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment(witness)) + let missing_assignment = |witness: Option| { + OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( + witness.unwrap().0, + )) }; for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); let operation = expression_to_const(&op_expr) - .ok_or_else(|| missing_assignment(op_expr.any_witness().unwrap().0))?; + .ok_or_else(|| missing_assignment(op_expr.any_witness()))?; let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); let index = expression_to_const(&index_expr) - .ok_or_else(|| missing_assignment(index_expr.any_witness().unwrap().0))?; + .ok_or_else(|| missing_assignment(index_expr.any_witness()))?; let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); let value_witness = value.any_witness(); @@ -90,11 +92,9 @@ impl BlockSolver { self.insert_value(index, value.q_c)?; } else if operation.is_zero() && value.is_linear() { match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) { - GateStatus::GateUnsolvable => { - return Err(missing_assignment(value_witness.unwrap().0)) - } + GateStatus::GateUnsolvable => return Err(missing_assignment(value_witness)), GateStatus::GateSolvable(sum, (coef, w)) => { - let map_value = self.get_value(index).ok_or(missing_assignment(w.0))?; + let map_value = self.get_value(index).ok_or(missing_assignment(Some(w)))?; insert_witness(w, (map_value - sum - value.q_c) / coef, initial_witness)?; } GateStatus::GateSatisfied(sum) => { @@ -102,7 +102,7 @@ impl BlockSolver { } } } else { - return Err(missing_assignment(value_witness.unwrap().0)); + return Err(missing_assignment(value_witness)); } self.solved_operations += 1; } From dc1f418c6d1edd1f1715ed6906d8d132b4629428 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Mon, 27 Feb 2023 09:55:23 +0100 Subject: [PATCH 10/16] Code review Update acir/src/circuit/opcodes.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- acir/src/circuit/opcodes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 79d15e6f7..9505768c5 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -103,7 +103,7 @@ impl Opcode { 3 => { let id = read_u32(&mut reader)?; let len = read_u32(&mut reader)?; - let mut trace = Vec::new(); + let mut trace = Vec::with_capacity(len); for _i in 0..len { let operation = Expression::read(&mut reader)?; let index = Expression::read(&mut reader)?; From aff783d6437bf14eaf096dd5c9eb05709b41475f Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Mon, 27 Feb 2023 09:57:16 +0100 Subject: [PATCH 11/16] Code review Update acvm/src/pwg/block.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- acvm/src/pwg/block.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 81c043fba..f4a4464bc 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -47,18 +47,12 @@ impl BlockSolver { index: u32, value: FieldElement, ) -> Result<(), OpcodeResolutionError> { - let entry = self.block_value.entry(index); - match entry { - std::collections::hash_map::Entry::Vacant(e) => { - e.insert(value); - } - std::collections::hash_map::Entry::Occupied(e) => { - if e.get() != &value { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } + match self.block_value.insert(index, value) { + Some(existing_value) if value != existing_value => { + Err(OpcodeResolutionError::UnsatisfiedConstrain) } + _ => Ok(()), } - Ok(()) } fn get_value(&self, index: u32) -> Option { From 1b73484409abba98ad37a70486da929e66eef3e8 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 Feb 2023 08:51:32 +0000 Subject: [PATCH 12/16] fix clippy --- acir/src/circuit/opcodes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 9505768c5..6ee2da3fc 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -103,7 +103,7 @@ impl Opcode { 3 => { let id = read_u32(&mut reader)?; let len = read_u32(&mut reader)?; - let mut trace = Vec::with_capacity(len); + let mut trace = Vec::with_capacity(len as usize); for _i in 0..len { let operation = Expression::read(&mut reader)?; let index = Expression::read(&mut reader)?; From 55ae1c0b41e01b8b4ec53ad559704bbadf39bed0 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 Feb 2023 17:01:11 +0000 Subject: [PATCH 13/16] fix clippy --- acvm/src/pwg/block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index f4a4464bc..e8a3da5f9 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -62,7 +62,7 @@ impl BlockSolver { // Try to solve block operations from the trace // As long as operations are resolved, we update/read from the block_value // We stop when an operation cannot be resolved - pub fn solve( + pub(crate) fn solve( &mut self, initial_witness: &mut BTreeMap, trace: &[MemOp], From b5ba22c94cecf7801ade9d2084275092a75e0f80 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 Feb 2023 17:17:50 +0000 Subject: [PATCH 14/16] comment any_witness --- acir/src/native_types/arithmetic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/acir/src/native_types/arithmetic.rs b/acir/src/native_types/arithmetic.rs index f93b2d5d7..4ec05abce 100644 --- a/acir/src/native_types/arithmetic.rs +++ b/acir/src/native_types/arithmetic.rs @@ -151,6 +151,7 @@ impl Expression { // Returns one witness belonging to the expression, in no relevant order // Returns None if the expression is const + // The function is used during partial witness generation to report unsolved witness pub fn any_witness(&self) -> Option { if self.linear_combinations.is_empty() { if self.mul_terms.is_empty() { From 769a6b7d2b09fb9d182b05e6100f129cd01fbabb Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 Feb 2023 17:36:34 +0000 Subject: [PATCH 15/16] Code review --- acvm/src/pwg.rs | 24 ++---------------------- acvm/src/pwg/block.rs | 8 ++++---- acvm/src/pwg/directives.rs | 22 +++++++++++----------- acvm/src/pwg/logic.rs | 6 +++--- acvm/src/pwg/range.rs | 4 ++-- acvm/src/pwg/signature/ecdsa.rs | 10 +++++----- 6 files changed, 27 insertions(+), 47 deletions(-) diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index a5c52940d..e3d1a941f 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -21,19 +21,10 @@ pub mod range; pub mod signature; pub mod sorting; -// Returns the concrete value for a particular witness -// Returns None if the witness has no assignment -pub fn witness_to_value( - initial_witness: &BTreeMap, - witness: Witness, -) -> Option<&FieldElement> { - initial_witness.get(&witness) -} - // Returns the concrete value for a particular witness // If the witness has no assignment, then // an error is returned -pub fn witness_to_value_unwrap( +pub fn witness_to_value( initial_witness: &BTreeMap, witness: Witness, ) -> Result<&FieldElement, OpcodeResolutionError> { @@ -43,25 +34,14 @@ pub fn witness_to_value_unwrap( } } -pub fn expression_to_const(expr: &Expression) -> Option { - expr.is_const().then_some(expr.q_c) -} - // TODO: There is an issue open to decide on whether we need to get values from Expressions // TODO versus just getting values from Witness pub fn get_value( expr: &Expression, initial_witness: &BTreeMap, -) -> Option { - expression_to_const(&ArithmeticSolver::evaluate(expr, initial_witness)) -} - -pub fn get_value_unwrap( - expr: &Expression, - initial_witness: &BTreeMap, ) -> Result { let expr = ArithmeticSolver::evaluate(expr, initial_witness); - match expression_to_const(&expr) { + match expr.to_const() { Some(value) => Ok(value), None => Err(OpcodeResolutionError::OpcodeNotSolvable( OpcodeNotSolvable::MissingAssignment(expr.any_witness().unwrap().0), diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index e8a3da5f9..8343d4b3b 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -11,7 +11,6 @@ use crate::{OpcodeNotSolvable, OpcodeResolutionError}; use super::{ arithmetic::{ArithmeticSolver, GateStatus}, directives::insert_witness, - expression_to_const, }; /// Maps a block to its emulated state @@ -74,10 +73,11 @@ impl BlockSolver { }; for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); - let operation = expression_to_const(&op_expr) - .ok_or_else(|| missing_assignment(op_expr.any_witness()))?; + let operation = + op_expr.to_const().ok_or_else(|| missing_assignment(op_expr.any_witness()))?; let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); - let index = expression_to_const(&index_expr) + let index = index_expr + .to_const() .ok_or_else(|| missing_assignment(index_expr.any_witness()))?; let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index dacd02fb1..5ea010eef 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -10,7 +10,7 @@ use num_traits::{One, Zero}; use crate::{OpcodeNotSolvable, OpcodeResolutionError}; -use super::{get_value_unwrap, insert_value, sorting::route, witness_to_value_unwrap}; +use super::{get_value, insert_value, sorting::route, witness_to_value}; pub fn solve_directives( initial_witness: &mut BTreeMap, @@ -18,21 +18,21 @@ pub fn solve_directives( ) -> Result<(), OpcodeResolutionError> { match directive { Directive::Invert { x, result } => { - let val = witness_to_value_unwrap(initial_witness, *x)?; + let val = witness_to_value(initial_witness, *x)?; let inverse = val.inverse(); initial_witness.insert(*result, inverse); Ok(()) } Directive::Quotient { a, b, q, r, predicate } => { - let val_a = get_value_unwrap(a, initial_witness)?; - let val_b = get_value_unwrap(b, initial_witness)?; + let val_a = get_value(a, initial_witness)?; + let val_b = get_value(b, initial_witness)?; let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); let int_b = BigUint::from_bytes_be(&val_b.to_be_bytes()); // 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 unresolved let pred_value = match predicate { - Some(pred) => get_value_unwrap(pred, initial_witness)?, + Some(pred) => get_value(pred, initial_witness)?, None => FieldElement::one(), }; @@ -56,7 +56,7 @@ pub fn solve_directives( Ok(()) } Directive::Truncate { a, b, c, bit_size } => { - let val_a = get_value_unwrap(a, initial_witness)?; + let val_a = get_value(a, initial_witness)?; let pow: BigUint = BigUint::one() << bit_size; let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); @@ -77,7 +77,7 @@ pub fn solve_directives( Ok(()) } Directive::ToRadix { a, b, radix, is_little_endian } => { - let value_a = get_value_unwrap(a, initial_witness)?; + let value_a = get_value(a, initial_witness)?; let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); if *is_little_endian { @@ -122,7 +122,7 @@ pub fn solve_directives( Ok(()) } Directive::OddRange { a, b, r, bit_size } => { - let val_a = witness_to_value_unwrap(initial_witness, *a)?; + let val_a = witness_to_value(initial_witness, *a)?; let int_a = BigUint::from_bytes_be(&val_a.to_be_bytes()); let pow: BigUint = BigUint::one() << (bit_size - 1); if int_a >= (&pow << 1) { @@ -153,7 +153,7 @@ pub fn solve_directives( assert_eq!(element.len(), *tuple as usize); let mut element_val = Vec::with_capacity(*tuple as usize + 1); for e in element { - element_val.push(get_value_unwrap(e, initial_witness)?); + element_val.push(get_value(e, initial_witness)?); } let field_i = FieldElement::from(i as i128); element_val.push(field_i); @@ -190,7 +190,7 @@ pub fn solve_directives( if witnesses.len() == 1 { let witness = &witnesses[0]; - let log_value = witness_to_value_unwrap(initial_witness, *witness)?; + let log_value = witness_to_value(initial_witness, *witness)?; println!("{}", format_field_string(*log_value)); return Ok(()); } @@ -202,7 +202,7 @@ pub fn solve_directives( // and convert them to hex strings. let mut elements_as_hex = Vec::with_capacity(witnesses.len()); for witness in witnesses { - let element = witness_to_value_unwrap(initial_witness, *witness)?; + let element = witness_to_value(initial_witness, *witness)?; elements_as_hex.push(format_field_string(*element)); } diff --git a/acvm/src/pwg/logic.rs b/acvm/src/pwg/logic.rs index 175b13f62..ba030901c 100644 --- a/acvm/src/pwg/logic.rs +++ b/acvm/src/pwg/logic.rs @@ -1,4 +1,4 @@ -use super::{directives::insert_witness, witness_to_value_unwrap}; +use super::{directives::insert_witness, witness_to_value}; use crate::OpcodeResolutionError; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; @@ -26,8 +26,8 @@ impl LogicSolver { num_bits: u32, is_xor_gate: bool, ) -> Result<(), OpcodeResolutionError> { - let w_l_value = witness_to_value_unwrap(initial_witness, *a)?; - let w_r_value = witness_to_value_unwrap(initial_witness, *b)?; + let w_l_value = witness_to_value(initial_witness, *a)?; + let w_r_value = witness_to_value(initial_witness, *b)?; let assignment = if is_xor_gate { w_l_value.xor(w_r_value, num_bits) diff --git a/acvm/src/pwg/range.rs b/acvm/src/pwg/range.rs index 047149c97..e47f917b7 100644 --- a/acvm/src/pwg/range.rs +++ b/acvm/src/pwg/range.rs @@ -1,4 +1,4 @@ -use crate::{pwg::witness_to_value_unwrap, OpcodeResolutionError}; +use crate::{pwg::witness_to_value, OpcodeResolutionError}; use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, BlackBoxFunc, FieldElement}; use std::collections::BTreeMap; @@ -27,7 +27,7 @@ pub fn solve_range_opcode( let input = func_call.inputs.first().expect("infallible: checked that input size is 1"); - let w_value = witness_to_value_unwrap(initial_witness, input.witness)?; + let w_value = witness_to_value(initial_witness, input.witness)?; if w_value.num_bits() > input.num_bits { return Err(OpcodeResolutionError::UnsatisfiedConstrain); } diff --git a/acvm/src/pwg/signature/ecdsa.rs b/acvm/src/pwg/signature/ecdsa.rs index fee2003eb..23d7c295c 100644 --- a/acvm/src/pwg/signature/ecdsa.rs +++ b/acvm/src/pwg/signature/ecdsa.rs @@ -1,7 +1,7 @@ use acir::{circuit::opcodes::BlackBoxFuncCall, native_types::Witness, FieldElement}; use std::collections::BTreeMap; -use crate::{pwg::witness_to_value_unwrap, OpcodeResolutionError}; +use crate::{pwg::witness_to_value, OpcodeResolutionError}; pub fn secp256k1_prehashed( initial_witness: &mut BTreeMap, @@ -15,7 +15,7 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_x should be 32 bytes long, found only {i} bytes")); - let x_i = witness_to_value_unwrap(initial_witness, _x_i.witness)?; + let x_i = witness_to_value(initial_witness, _x_i.witness)?; *pkx = *x_i.to_be_bytes().last().unwrap(); } @@ -25,7 +25,7 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("pub_key_y should be 32 bytes long, found only {i} bytes")); - let y_i = witness_to_value_unwrap(initial_witness, _y_i.witness)?; + let y_i = witness_to_value(initial_witness, _y_i.witness)?; *pky = *y_i.to_be_bytes().last().unwrap(); } @@ -35,13 +35,13 @@ pub fn secp256k1_prehashed( .next() .unwrap_or_else(|| panic!("signature should be 64 bytes long, found only {i} bytes")); - let sig_i = witness_to_value_unwrap(initial_witness, _sig_i.witness)?; + let sig_i = witness_to_value(initial_witness, _sig_i.witness)?; *sig = *sig_i.to_be_bytes().last().unwrap() } let mut hashed_message = Vec::new(); for msg in inputs_iter { - let msg_i_field = witness_to_value_unwrap(initial_witness, msg.witness)?; + let msg_i_field = witness_to_value(initial_witness, msg.witness)?; let msg_i = *msg_i_field.to_be_bytes().last().unwrap(); hashed_message.push(msg_i); } From d0f4abdbfd6478ae9adcde1f0b555a71126e1a50 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 28 Feb 2023 17:44:20 +0000 Subject: [PATCH 16/16] move any_witness --- acir/src/native_types/arithmetic.rs | 15 --------------- acvm/src/pwg.rs | 8 +++++--- acvm/src/pwg/arithmetic.rs | 15 +++++++++++++++ acvm/src/pwg/block.rs | 13 +++++++------ 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/acir/src/native_types/arithmetic.rs b/acir/src/native_types/arithmetic.rs index 4ec05abce..4294321b0 100644 --- a/acir/src/native_types/arithmetic.rs +++ b/acir/src/native_types/arithmetic.rs @@ -149,21 +149,6 @@ impl Expression { Ok(expr) } - // Returns one witness belonging to the expression, in no relevant order - // Returns None if the expression is const - // The function is used during partial witness generation to report unsolved witness - pub fn any_witness(&self) -> Option { - if self.linear_combinations.is_empty() { - if self.mul_terms.is_empty() { - None - } else { - Some(self.mul_terms[0].1) - } - } else { - Some(self.linear_combinations[0].1) - } - } - pub fn term_addition(&mut self, coefficient: acir_field::FieldElement, variable: Witness) { self.linear_combinations.push((coefficient, variable)) } diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index e3d1a941f..1d8c66ffa 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -43,9 +43,11 @@ pub fn get_value( let expr = ArithmeticSolver::evaluate(expr, initial_witness); match expr.to_const() { Some(value) => Ok(value), - None => Err(OpcodeResolutionError::OpcodeNotSolvable( - OpcodeNotSolvable::MissingAssignment(expr.any_witness().unwrap().0), - )), + None => { + Err(OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment( + ArithmeticSolver::any_witness_from_expression(&expr).unwrap().0, + ))) + } } } diff --git a/acvm/src/pwg/arithmetic.rs b/acvm/src/pwg/arithmetic.rs index 26cbbc7e9..930780b4e 100644 --- a/acvm/src/pwg/arithmetic.rs +++ b/acvm/src/pwg/arithmetic.rs @@ -229,6 +229,21 @@ impl ArithmeticSolver { result.q_c += expr.q_c; result } + + // Returns one witness belonging to an expression, in no relevant order + // Returns None if the expression is const + // The function is used during partial witness generation to report unsolved witness + pub fn any_witness_from_expression(expr: &Expression) -> Option { + if expr.linear_combinations.is_empty() { + if expr.mul_terms.is_empty() { + None + } else { + Some(expr.mul_terms[0].1) + } + } else { + Some(expr.linear_combinations[0].1) + } + } } #[test] diff --git a/acvm/src/pwg/block.rs b/acvm/src/pwg/block.rs index 8343d4b3b..28798b070 100644 --- a/acvm/src/pwg/block.rs +++ b/acvm/src/pwg/block.rs @@ -73,15 +73,16 @@ impl BlockSolver { }; for block_op in trace.iter().skip(self.solved_operations) { let op_expr = ArithmeticSolver::evaluate(&block_op.operation, initial_witness); - let operation = - op_expr.to_const().ok_or_else(|| missing_assignment(op_expr.any_witness()))?; + let operation = op_expr.to_const().ok_or_else(|| { + missing_assignment(ArithmeticSolver::any_witness_from_expression(&op_expr)) + })?; let index_expr = ArithmeticSolver::evaluate(&block_op.index, initial_witness); - let index = index_expr - .to_const() - .ok_or_else(|| missing_assignment(index_expr.any_witness()))?; + let index = index_expr.to_const().ok_or_else(|| { + missing_assignment(ArithmeticSolver::any_witness_from_expression(&index_expr)) + })?; let index = index.try_to_u64().unwrap() as u32; let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness); - let value_witness = value.any_witness(); + let value_witness = ArithmeticSolver::any_witness_from_expression(&value); if value.is_const() { self.insert_value(index, value.q_c)?; } else if operation.is_zero() && value.is_linear() {