From c902a01639033665d106e2d9f4e5c7070af8c0bb Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 9 Jun 2023 11:32:28 +0100 Subject: [PATCH] chore(acvm)!: Make internals of ACVM private (#353) --- acvm/src/{compiler.rs => compiler/mod.rs} | 19 +++++------ acvm/src/compiler/optimizers/mod.rs | 5 +-- acvm/src/compiler/optimizers/simplify.rs | 4 +-- acvm/src/compiler/transformers/csat.rs | 6 ++-- acvm/src/compiler/transformers/fallback.rs | 8 ++--- acvm/src/compiler/transformers/mod.rs | 6 ++-- acvm/src/compiler/transformers/r1cs.rs | 6 ++-- acvm/src/pwg/arithmetic.rs | 14 ++++---- acvm/src/pwg/{signature => blackbox}/ecdsa.rs | 2 +- acvm/src/pwg/{ => blackbox}/hash.rs | 13 ++++---- acvm/src/pwg/{ => blackbox}/logic.rs | 6 ++-- acvm/src/pwg/{blackbox.rs => blackbox/mod.rs} | 18 +++++++---- acvm/src/pwg/{ => blackbox}/range.rs | 2 +- acvm/src/pwg/brillig.rs | 4 +-- .../pwg/{directives.rs => directives/mod.rs} | 8 +++-- acvm/src/pwg/{ => directives}/sorting.rs | 4 +-- acvm/src/{pwg.rs => pwg/mod.rs} | 32 +++++++++---------- acvm/src/pwg/oracle.rs | 4 +-- acvm/src/pwg/signature.rs | 1 - 19 files changed, 83 insertions(+), 79 deletions(-) rename acvm/src/{compiler.rs => compiler/mod.rs} (95%) rename acvm/src/pwg/{signature => blackbox}/ecdsa.rs (99%) rename acvm/src/pwg/{ => blackbox}/hash.rs (95%) rename acvm/src/pwg/{ => blackbox}/logic.rs (95%) rename acvm/src/pwg/{blackbox.rs => blackbox/mod.rs} (92%) rename acvm/src/pwg/{ => blackbox}/range.rs (93%) rename acvm/src/pwg/{directives.rs => directives/mod.rs} (97%) rename acvm/src/pwg/{ => directives}/sorting.rs (99%) rename acvm/src/{pwg.rs => pwg/mod.rs} (98%) delete mode 100644 acvm/src/pwg/signature.rs diff --git a/acvm/src/compiler.rs b/acvm/src/compiler/mod.rs similarity index 95% rename from acvm/src/compiler.rs rename to acvm/src/compiler/mod.rs index 21fa8e31c..b925f5c74 100644 --- a/acvm/src/compiler.rs +++ b/acvm/src/compiler/mod.rs @@ -1,20 +1,21 @@ -// The various passes that we can use over ACIR -pub mod optimizers; -pub mod transformers; - -use crate::Language; use acir::{ circuit::{Circuit, Opcode}, native_types::{Expression, Witness}, BlackBoxFunc, FieldElement, }; use indexmap::IndexMap; -use optimizers::GeneralOptimizer; use thiserror::Error; + +use crate::Language; + +// The various passes that we can use over ACIR +mod optimizers; +mod transformers; + +use optimizers::{GeneralOptimizer, RangeOptimizer}; use transformers::{CSatTransformer, FallbackTransformer, R1CSTransformer}; -use self::optimizers::RangeOptimizer; -use self::optimizers::Simplifier; +pub use optimizers::{CircuitSimplifier, SimplifyResult}; #[derive(PartialEq, Eq, Debug, Error)] pub enum CompileError { @@ -27,7 +28,7 @@ pub fn compile( acir: Circuit, np_language: Language, is_opcode_supported: impl Fn(&Opcode) -> bool, - simplifier: &Simplifier, + simplifier: &CircuitSimplifier, ) -> Result { // Instantiate the optimizer. // Currently the optimizer and reducer are one in the same diff --git a/acvm/src/compiler/optimizers/mod.rs b/acvm/src/compiler/optimizers/mod.rs index 8db6f5dfe..b8e116c6a 100644 --- a/acvm/src/compiler/optimizers/mod.rs +++ b/acvm/src/compiler/optimizers/mod.rs @@ -1,7 +1,8 @@ mod general; mod redundant_range; -pub mod simplify; +mod simplify; pub(crate) use general::GeneralOptimizer; pub(crate) use redundant_range::RangeOptimizer; -pub(crate) use simplify::CircuitSimplifier as Simplifier; +// Public as these need to be passed to `acvm::compiler::compile()` +pub use simplify::{CircuitSimplifier, SimplifyResult}; diff --git a/acvm/src/compiler/optimizers/simplify.rs b/acvm/src/compiler/optimizers/simplify.rs index 3c27587e3..ee6d2eabe 100644 --- a/acvm/src/compiler/optimizers/simplify.rs +++ b/acvm/src/compiler/optimizers/simplify.rs @@ -437,7 +437,7 @@ mod tests { FieldElement, }; - use crate::compiler::{optimizers::Simplifier, transformers::FallbackTransformer}; + use crate::compiler::{optimizers::CircuitSimplifier, transformers::FallbackTransformer}; #[test] fn simplify_test() { @@ -465,7 +465,7 @@ mod tests { linear_combinations: vec![(one, a)], q_c: FieldElement::zero(), }; - let mut simplifier = Simplifier::new(1); + let mut simplifier = CircuitSimplifier::new(1); let mut circuit = vec![ Opcode::Arithmetic(gate_a), Opcode::Arithmetic(gate_b), diff --git a/acvm/src/compiler/transformers/csat.rs b/acvm/src/compiler/transformers/csat.rs index 8880e734a..3fd4d037c 100644 --- a/acvm/src/compiler/transformers/csat.rs +++ b/acvm/src/compiler/transformers/csat.rs @@ -15,13 +15,13 @@ use indexmap::IndexMap; /// to calculate the original expression. // Should we give it all of the gates? // Have a single transformer that you instantiate with a width, then pass many gates through -pub struct CSatTransformer { +pub(crate) struct CSatTransformer { width: usize, } impl CSatTransformer { // Configure the width for the optimizer - pub fn new(width: usize) -> CSatTransformer { + pub(crate) fn new(width: usize) -> CSatTransformer { assert!(width > 2); CSatTransformer { width } @@ -30,7 +30,7 @@ impl CSatTransformer { // Still missing dead witness optimization. // To do this, we will need the whole set of arithmetic gates // I think it can also be done before the local optimization seen here, as dead variables will come from the user - pub fn transform( + pub(crate) fn transform( &self, gate: Expression, intermediate_variables: &mut IndexMap, diff --git a/acvm/src/compiler/transformers/fallback.rs b/acvm/src/compiler/transformers/fallback.rs index b9e72fd58..ed5fe7a82 100644 --- a/acvm/src/compiler/transformers/fallback.rs +++ b/acvm/src/compiler/transformers/fallback.rs @@ -1,4 +1,4 @@ -use crate::compiler::optimizers::Simplifier; +use crate::compiler::optimizers::CircuitSimplifier; use super::super::CompileError; use acir::{ @@ -8,14 +8,14 @@ use acir::{ /// The initial transformer to act on a [`Circuit`]. This replaces any unsupported opcodes with /// fallback implementations consisting of well supported opcodes. -pub struct FallbackTransformer; +pub(crate) struct FallbackTransformer; impl FallbackTransformer { //ACIR pass which replace unsupported opcodes using arithmetic fallback - pub fn transform( + pub(crate) fn transform( acir: Circuit, is_supported: impl Fn(&Opcode) -> bool, - simplifier: &Simplifier, + simplifier: &CircuitSimplifier, ) -> Result { let mut acir_supported_opcodes = Vec::with_capacity(acir.opcodes.len()); diff --git a/acvm/src/compiler/transformers/mod.rs b/acvm/src/compiler/transformers/mod.rs index e0d690188..89e17ca68 100644 --- a/acvm/src/compiler/transformers/mod.rs +++ b/acvm/src/compiler/transformers/mod.rs @@ -2,6 +2,6 @@ mod csat; mod fallback; mod r1cs; -pub use csat::CSatTransformer; -pub use fallback::FallbackTransformer; -pub use r1cs::R1CSTransformer; +pub(crate) use csat::CSatTransformer; +pub(crate) use fallback::FallbackTransformer; +pub(crate) use r1cs::R1CSTransformer; diff --git a/acvm/src/compiler/transformers/r1cs.rs b/acvm/src/compiler/transformers/r1cs.rs index b19f78f5f..3bdd29c9c 100644 --- a/acvm/src/compiler/transformers/r1cs.rs +++ b/acvm/src/compiler/transformers/r1cs.rs @@ -1,16 +1,16 @@ use acir::circuit::Circuit; /// Currently a "noop" transformer. -pub struct R1CSTransformer { +pub(crate) struct R1CSTransformer { acir: Circuit, } impl R1CSTransformer { - pub fn new(acir: Circuit) -> Self { + pub(crate) fn new(acir: Circuit) -> Self { Self { acir } } // TODO: We could possibly make sure that all polynomials are at most degree-2 - pub fn transform(self) -> Circuit { + pub(crate) fn transform(self) -> Circuit { self.acir } } diff --git a/acvm/src/pwg/arithmetic.rs b/acvm/src/pwg/arithmetic.rs index dfeb3ce4a..7687cff01 100644 --- a/acvm/src/pwg/arithmetic.rs +++ b/acvm/src/pwg/arithmetic.rs @@ -7,16 +7,16 @@ use super::{OpcodeNotSolvable, OpcodeResolution, OpcodeResolutionError}; /// An Arithmetic solver will take a Circuit's arithmetic gates with witness assignments /// and create the other witness variables -pub struct ArithmeticSolver; +pub(super) struct ArithmeticSolver; #[allow(clippy::enum_variant_names)] -pub enum GateStatus { +pub(super) enum GateStatus { GateSatisfied(FieldElement), GateSolvable(FieldElement, (FieldElement, Witness)), GateUnsolvable, } -pub enum MulTerm { +pub(crate) enum MulTerm { OneUnknown(FieldElement, Witness), // (qM * known_witness, unknown_witness) TooManyUnknowns, Solved(FieldElement), @@ -24,7 +24,7 @@ pub enum MulTerm { impl ArithmeticSolver { /// Derives the rest of the witness based on the initial low level variables - pub fn solve( + pub(super) fn solve( initial_witness: &mut WitnessMap, gate: &Expression, ) -> Result { @@ -162,7 +162,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 - pub fn solve_fan_in_term( + pub(super) fn solve_fan_in_term( arith_gate: &Expression, witness_assignments: &WitnessMap, ) -> GateStatus { @@ -198,7 +198,7 @@ impl ArithmeticSolver { } // Partially evaluate the gate using the known witnesses - pub fn evaluate(expr: &Expression, initial_witness: &WitnessMap) -> Expression { + pub(super) fn evaluate(expr: &Expression, initial_witness: &WitnessMap) -> Expression { let mut result = Expression::default(); for &(c, w1, w2) in &expr.mul_terms { let mul_result = ArithmeticSolver::solve_mul_term_helper(&(c, w1, w2), initial_witness); @@ -230,7 +230,7 @@ impl ArithmeticSolver { // 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 { + pub(super) fn any_witness_from_expression(expr: &Expression) -> Option { if expr.linear_combinations.is_empty() { if expr.mul_terms.is_empty() { None diff --git a/acvm/src/pwg/signature/ecdsa.rs b/acvm/src/pwg/blackbox/ecdsa.rs similarity index 99% rename from acvm/src/pwg/signature/ecdsa.rs rename to acvm/src/pwg/blackbox/ecdsa.rs index 6a93b4ac9..b091137bc 100644 --- a/acvm/src/pwg/signature/ecdsa.rs +++ b/acvm/src/pwg/blackbox/ecdsa.rs @@ -19,7 +19,7 @@ fn to_u8_vec( Ok(result) } -pub fn secp256k1_prehashed( +pub(crate) fn secp256k1_prehashed( initial_witness: &mut WitnessMap, public_key_x_inputs: &[FunctionInput], public_key_y_inputs: &[FunctionInput], diff --git a/acvm/src/pwg/hash.rs b/acvm/src/pwg/blackbox/hash.rs similarity index 95% rename from acvm/src/pwg/hash.rs rename to acvm/src/pwg/blackbox/hash.rs index b0b2a48b6..47f0c931f 100644 --- a/acvm/src/pwg/hash.rs +++ b/acvm/src/pwg/blackbox/hash.rs @@ -7,11 +7,10 @@ use blake2::{Blake2s256, Digest}; use sha2::Sha256; use sha3::Keccak256; +use crate::pwg::{insert_value, witness_to_value}; use crate::{pwg::OpcodeResolution, OpcodeResolutionError}; -use super::{insert_value, witness_to_value}; - -pub fn blake2s256( +pub(crate) fn blake2s256( initial_witness: &mut WitnessMap, inputs: &[FunctionInput], outputs: &[Witness], @@ -29,7 +28,7 @@ pub fn blake2s256( Ok(OpcodeResolution::Solved) } -pub fn sha256( +pub(crate) fn sha256( initial_witness: &mut WitnessMap, inputs: &[FunctionInput], outputs: &[Witness], @@ -47,7 +46,7 @@ pub fn sha256( Ok(OpcodeResolution::Solved) } -pub fn keccak256( +pub(crate) fn keccak256( initial_witness: &mut WitnessMap, inputs: &[FunctionInput], outputs: &[Witness], @@ -65,7 +64,7 @@ pub fn keccak256( Ok(OpcodeResolution::Solved) } -pub fn keccak256_variable_length( +pub(crate) fn keccak256_variable_length( initial_witness: &mut WitnessMap, inputs: &[FunctionInput], var_message_size: FunctionInput, @@ -84,7 +83,7 @@ pub fn keccak256_variable_length( Ok(OpcodeResolution::Solved) } -pub fn hash_to_field_128_security( +pub(crate) fn hash_to_field_128_security( initial_witness: &mut WitnessMap, inputs: &[FunctionInput], output: &Witness, diff --git a/acvm/src/pwg/logic.rs b/acvm/src/pwg/blackbox/logic.rs similarity index 95% rename from acvm/src/pwg/logic.rs rename to acvm/src/pwg/blackbox/logic.rs index 98c019414..094b33d55 100644 --- a/acvm/src/pwg/logic.rs +++ b/acvm/src/pwg/blackbox/logic.rs @@ -1,4 +1,4 @@ -use super::{insert_value, witness_to_value}; +use crate::pwg::{insert_value, witness_to_value}; use crate::{pwg::OpcodeResolution, OpcodeResolutionError}; use acir::{ circuit::opcodes::FunctionInput, @@ -8,7 +8,7 @@ use acir::{ /// Solves a [`BlackBoxFunc::And`][acir::circuit::black_box_functions::BlackBoxFunc::AND] opcode and inserts /// the result into the supplied witness map -pub fn and( +pub(super) fn and( initial_witness: &mut WitnessMap, lhs: &FunctionInput, rhs: &FunctionInput, @@ -25,7 +25,7 @@ pub fn and( /// Solves a [`BlackBoxFunc::XOR`][acir::circuit::black_box_functions::BlackBoxFunc::XOR] opcode and inserts /// the result into the supplied witness map -pub fn xor( +pub(super) fn xor( initial_witness: &mut WitnessMap, lhs: &FunctionInput, rhs: &FunctionInput, diff --git a/acvm/src/pwg/blackbox.rs b/acvm/src/pwg/blackbox/mod.rs similarity index 92% rename from acvm/src/pwg/blackbox.rs rename to acvm/src/pwg/blackbox/mod.rs index d528e17da..fc7e425b3 100644 --- a/acvm/src/pwg/blackbox.rs +++ b/acvm/src/pwg/blackbox/mod.rs @@ -3,16 +3,20 @@ use acir::{ native_types::{Witness, WitnessMap}, }; -use super::{ - hash::{blake2s256, hash_to_field_128_security, keccak256, keccak256_variable_length, sha256}, - logic::{and, xor}, - range::solve_range_opcode, - signature::ecdsa::secp256k1_prehashed, - OpcodeResolution, OpcodeResolutionError, -}; +use super::{OpcodeResolution, OpcodeResolutionError}; use crate::pwg::OpcodeNotSolvable; use crate::PartialWitnessGenerator; +mod ecdsa; +mod hash; +mod logic; +mod range; + +use ecdsa::secp256k1_prehashed; +use hash::{blake2s256, hash_to_field_128_security, keccak256, keccak256_variable_length, sha256}; +use logic::{and, xor}; +use range::solve_range_opcode; + /// Check if all of the inputs to the function have assignments /// /// Returns the first missing assignment if any are missing diff --git a/acvm/src/pwg/range.rs b/acvm/src/pwg/blackbox/range.rs similarity index 93% rename from acvm/src/pwg/range.rs rename to acvm/src/pwg/blackbox/range.rs index c2ebd48f5..31b46a6c3 100644 --- a/acvm/src/pwg/range.rs +++ b/acvm/src/pwg/blackbox/range.rs @@ -1,7 +1,7 @@ use crate::{pwg::witness_to_value, pwg::OpcodeResolution, OpcodeResolutionError}; use acir::{circuit::opcodes::FunctionInput, native_types::WitnessMap}; -pub fn solve_range_opcode( +pub(super) fn solve_range_opcode( initial_witness: &mut WitnessMap, input: &FunctionInput, ) -> Result { diff --git a/acvm/src/pwg/brillig.rs b/acvm/src/pwg/brillig.rs index 1fc27e0bf..2bc52a26a 100644 --- a/acvm/src/pwg/brillig.rs +++ b/acvm/src/pwg/brillig.rs @@ -9,10 +9,10 @@ use crate::{pwg::OpcodeNotSolvable, OpcodeResolution, OpcodeResolutionError}; use super::{get_value, insert_value}; -pub struct BrilligSolver; +pub(super) struct BrilligSolver; impl BrilligSolver { - pub fn solve( + pub(super) fn solve( initial_witness: &mut WitnessMap, brillig: &mut Brillig, ) -> Result { diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives/mod.rs similarity index 97% rename from acvm/src/pwg/directives.rs rename to acvm/src/pwg/directives/mod.rs index 9b18bf7d4..dac7ddd6e 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives/mod.rs @@ -10,7 +10,9 @@ use num_traits::Zero; use crate::{pwg::OpcodeResolution, OpcodeResolutionError}; -use super::{get_value, insert_value, sorting::route, witness_to_value}; +use super::{get_value, insert_value, witness_to_value}; + +mod sorting; /// Attempts to solve the [`Directive`] opcode `directive`. /// If successful, `initial_witness` will be mutated to contain the new witness assignment. @@ -18,7 +20,7 @@ use super::{get_value, insert_value, sorting::route, witness_to_value}; /// Returns `Ok(OpcodeResolution)` to signal whether the directive was successful solved. /// /// Returns `Err(OpcodeResolutionError)` if a circuit constraint is unsatisfied. -pub fn solve_directives( +pub(super) fn solve_directives( initial_witness: &mut WitnessMap, directive: &Directive, ) -> Result { @@ -126,7 +128,7 @@ fn solve_directives_internal( Ordering::Equal }); let b = val_a.iter().map(|a| *a.last().unwrap()).collect(); - let control = route(base, b); + let control = sorting::route(base, b); for (w, value) in bits.iter().zip(control) { let value = if value { FieldElement::one() } else { FieldElement::zero() }; insert_value(w, value, initial_witness)?; diff --git a/acvm/src/pwg/sorting.rs b/acvm/src/pwg/directives/sorting.rs similarity index 99% rename from acvm/src/pwg/sorting.rs rename to acvm/src/pwg/directives/sorting.rs index 9d102a7cd..baeb8677c 100644 --- a/acvm/src/pwg/sorting.rs +++ b/acvm/src/pwg/directives/sorting.rs @@ -158,7 +158,7 @@ impl SortingNetwork { // Computes the control bits of the sorting network which transform inputs into outputs // implementation is based on https://www.mdpi.com/2227-7080/10/1/16 -pub fn route(inputs: Vec, outputs: Vec) -> Vec { +pub(super) fn route(inputs: Vec, outputs: Vec) -> Vec { assert_eq!(inputs.len(), outputs.len()); match inputs.len() { 0 => Vec::new(), @@ -245,7 +245,7 @@ pub fn route(inputs: Vec, outputs: Vec) -> Vec #[cfg(test)] mod tests { - use crate::pwg::sorting::route; + use super::route; use acir::FieldElement; use rand::prelude::*; diff --git a/acvm/src/pwg.rs b/acvm/src/pwg/mod.rs similarity index 98% rename from acvm/src/pwg.rs rename to acvm/src/pwg/mod.rs index bcfc5f208..aa23a3be2 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg/mod.rs @@ -9,27 +9,25 @@ use acir::{ }; use self::{ - arithmetic::ArithmeticSolver, block::Blocks, brillig::BrilligSolver, - directives::solve_directives, oracle::OracleSolver, + arithmetic::ArithmeticSolver, brillig::BrilligSolver, directives::solve_directives, + oracle::OracleSolver, }; use thiserror::Error; // arithmetic -pub mod arithmetic; +pub(crate) mod arithmetic; // Brillig bytecode -pub mod brillig; +mod brillig; // Directives -pub mod directives; +mod directives; // black box functions mod blackbox; -pub mod block; -pub mod hash; -pub mod logic; -pub mod oracle; -pub mod range; -pub mod signature; -pub mod sorting; +mod block; +mod oracle; + +// Re-export `Blocks` so that it can be passed to `pwg::solve` +pub use block::Blocks; #[derive(Debug, PartialEq)] pub enum PartialWitnessGeneratorStatus { @@ -227,11 +225,11 @@ pub fn get_value( } } -// Inserts `value` into the initial witness map -// under the key of `witness`. -// Returns an error, if there was already a value in the map -// which does not match the value that one is about to insert -fn insert_value( +/// Inserts `value` into the initial witness map under the index `witness`. +/// +/// Returns an error if there was already a value in the map +/// which does not match the value that one is about to insert +pub fn insert_value( witness: &Witness, value_to_insert: FieldElement, initial_witness: &mut WitnessMap, diff --git a/acvm/src/pwg/oracle.rs b/acvm/src/pwg/oracle.rs index e91a8a714..e6db9221f 100644 --- a/acvm/src/pwg/oracle.rs +++ b/acvm/src/pwg/oracle.rs @@ -3,11 +3,11 @@ use acir::{circuit::opcodes::OracleData, native_types::WitnessMap}; use super::{arithmetic::ArithmeticSolver, insert_value}; use super::{OpcodeNotSolvable, OpcodeResolution, OpcodeResolutionError}; -pub struct OracleSolver; +pub(super) struct OracleSolver; impl OracleSolver { /// Derives the rest of the witness based on the initial low level variables - pub fn solve( + pub(super) fn solve( initial_witness: &mut WitnessMap, data: &mut OracleData, ) -> Result { diff --git a/acvm/src/pwg/signature.rs b/acvm/src/pwg/signature.rs deleted file mode 100644 index ecc8b2873..000000000 --- a/acvm/src/pwg/signature.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod ecdsa;