From 4c7cf89184589685cc79c9b9947d2f86699611dc Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 29 Aug 2024 16:33:45 -0400 Subject: [PATCH 01/22] feat: wip ensuring values and function types match ACVM FunctionInput bit_size --- acvm-repo/acir/src/circuit/opcodes.rs | 2 +- .../src/circuit/opcodes/black_box_function_call.rs | 13 ++++++++++--- acvm-repo/acvm/src/pwg/mod.rs | 6 +++++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes.rs b/acvm-repo/acir/src/circuit/opcodes.rs index d7f0f5f6f1f..1c7d95d6886 100644 --- a/acvm-repo/acir/src/circuit/opcodes.rs +++ b/acvm-repo/acir/src/circuit/opcodes.rs @@ -40,7 +40,7 @@ pub enum Opcode { /// values which define the opcode. /// /// A general expression of assert-zero opcode is the following: - /// ``` + /// ```text /// \sum_{i,j} {q_M}_{i,j}w_iw_j + \sum_i q_iw_i +q_c = 0 /// ``` /// diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 6a301ec5115..6515c335b78 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -1,5 +1,5 @@ use crate::native_types::Witness; -use crate::BlackBoxFunc; +use crate::{AcirField, BlackBoxFunc}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; // Note: Some functions will not use all of the witness @@ -13,8 +13,8 @@ pub enum ConstantOrWitnessEnum { #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct FunctionInput { - pub input: ConstantOrWitnessEnum, - pub num_bits: u32, + input: ConstantOrWitnessEnum, + num_bits: u32, } impl FunctionInput { @@ -25,6 +25,10 @@ impl FunctionInput { } } + pub fn input(self) -> ConstantOrWitnessEnum { + self.input + } + pub fn num_bits(&self) -> u32 { self.num_bits } @@ -32,8 +36,11 @@ impl FunctionInput { pub fn witness(witness: Witness, num_bits: u32) -> FunctionInput { FunctionInput { input: ConstantOrWitnessEnum::Witness(witness), num_bits } } +} +impl FunctionInput { pub fn constant(value: F, num_bits: u32) -> FunctionInput { + assert!(value.num_bits() <= num_bits); FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits } } } diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 647c11bd3c3..710ed3841de 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -638,7 +638,11 @@ pub fn input_to_value( input: FunctionInput, ) -> Result> { match input.input { - ConstantOrWitnessEnum::Witness(witness) => Ok(*witness_to_value(initial_witness, witness)?), + ConstantOrWitnessEnum::Witness(witness) => { + let initial_value = *witness_to_value(initial_witness, witness)?; + assert!(initial_value.num_bits() <= input.num_bits); + Ok(initial_value) + }, ConstantOrWitnessEnum::Constant(value) => Ok(value), } } From 575c2f64e58906f72be78a98b89bd419c5d99ebd Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 00:32:58 -0400 Subject: [PATCH 02/22] wip debugging regression_4080, add error for InvalidInputBitSize, throw error instead of asserting in FunctionInput::constant, convert FieldElement from/to BigUInt to fix distinct-input patch amount modulo, set opcode_location for InvalidInputBitSize --- acvm-repo/acir/src/circuit/opcodes.rs | 2 +- .../opcodes/black_box_function_call.rs | 28 ++- acvm-repo/acir/src/lib.rs | 1 + acvm-repo/acir_field/src/field_element.rs | 12 ++ .../compiler/optimizers/redundant_range.rs | 24 ++- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 6 +- acvm-repo/acvm/src/pwg/mod.rs | 28 ++- acvm-repo/acvm/tests/solver.rs | 195 +++++++++--------- acvm-repo/acvm_js/src/execute.rs | 4 + compiler/noirc_evaluator/src/errors.rs | 5 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 7 +- .../regression_4080/src/main.nr | 3 +- tooling/fuzzer/src/dictionary/mod.rs | 24 +-- tooling/nargo/src/errors.rs | 4 + tooling/nargo/src/ops/execute.rs | 4 + tooling/nargo/src/ops/test.rs | 3 + 16 files changed, 219 insertions(+), 131 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes.rs b/acvm-repo/acir/src/circuit/opcodes.rs index 1c7d95d6886..598aa65052e 100644 --- a/acvm-repo/acir/src/circuit/opcodes.rs +++ b/acvm-repo/acir/src/circuit/opcodes.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; mod black_box_function_call; mod memory_operation; -pub use black_box_function_call::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput}; +pub use black_box_function_call::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize}; pub use memory_operation::{BlockId, MemOp}; #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 8d5fd18cb20..795c2142418 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -1,6 +1,8 @@ use crate::native_types::Witness; use crate::{AcirField, BlackBoxFunc}; + use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use thiserror::Error; // Note: Some functions will not use all of the witness // So we need to supply how many bits of the witness is needed @@ -29,6 +31,10 @@ impl FunctionInput { self.input } + pub fn input_ref(&self) -> &ConstantOrWitnessEnum { + &self.input + } + pub fn num_bits(&self) -> u32 { self.num_bits } @@ -38,10 +44,26 @@ impl FunctionInput { } } + +#[derive(Clone, PartialEq, Eq, Debug, Error)] +#[error("FunctionInput value has too many bits: value: {value}, {value_num_bits} >= {num_bits}")] +pub struct InvalidInputBitSize { + pub value: String, + pub value_num_bits: u32, + pub num_bits: u32, +} + impl FunctionInput { - pub fn constant(value: F, num_bits: u32) -> FunctionInput { - assert!(value.num_bits() <= num_bits); - FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits } + pub fn constant(value: F, num_bits: u32) -> Result, InvalidInputBitSize> { + if value.num_bits() <= num_bits { + Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits }) + } else { + Err(InvalidInputBitSize { + value: format!("{}", value), + value_num_bits: value.num_bits(), + num_bits, + }) + } } } diff --git a/acvm-repo/acir/src/lib.rs b/acvm-repo/acir/src/lib.rs index 845a1d6ad5a..36331427b9f 100644 --- a/acvm-repo/acir/src/lib.rs +++ b/acvm-repo/acir/src/lib.rs @@ -12,6 +12,7 @@ pub use acir_field; pub use acir_field::{AcirField, FieldElement}; pub use brillig; pub use circuit::black_box_functions::BlackBoxFunc; +pub use circuit::opcodes::InvalidInputBitSize; #[cfg(test)] mod reflection { diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs index 2323f008dbe..3d22cb4a909 100644 --- a/acvm-repo/acir_field/src/field_element.rs +++ b/acvm-repo/acir_field/src/field_element.rs @@ -137,6 +137,18 @@ impl<'de, T: PrimeField> Deserialize<'de> for FieldElement { } } +impl From for FieldElement { + fn from(a: BigUint) -> FieldElement { + FieldElement(F::from(a)) + } +} + +impl Into for FieldElement { + fn into(self) -> BigUint { + F::into(self.0) + } +} + impl From for FieldElement { fn from(a: u128) -> FieldElement { FieldElement(F::from(a)) diff --git a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs index b03b6715abe..06196b73c0f 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs @@ -1,6 +1,6 @@ use acir::{ circuit::{ - opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput}, + opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum}, Circuit, Opcode, }, native_types::Witness, @@ -73,10 +73,13 @@ impl RangeOptimizer { } } - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { - input: - FunctionInput { input: ConstantOrWitnessEnum::Witness(witness), num_bits }, - }) => Some((*witness, *num_bits)), + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => { + if let ConstantOrWitnessEnum::Witness(witness) = input.input() { + Some((witness.clone(), input.num_bits())) + } else { + None + } + } _ => None, }) else { @@ -107,10 +110,13 @@ impl RangeOptimizer { let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len()); for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() { let (witness, num_bits) = match opcode { - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { - input: - FunctionInput { input: ConstantOrWitnessEnum::Witness(w), num_bits: bits }, - }) => (w, bits), + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) if matches!(input.input_ref(), ConstantOrWitnessEnum::Witness(..)) => { + if let ConstantOrWitnessEnum::Witness(witness) = input.input() { + (witness, input.num_bits()) + } else { + unreachable!("The matches! above ensures only a witness is possible") + } + }, _ => { // If its not the range opcode, add it to the opcode // list and continue; diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index def4216fe15..88227f18449 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -42,11 +42,11 @@ fn first_missing_assignment( inputs: &[FunctionInput], ) -> Option { inputs.iter().find_map(|input| { - if let ConstantOrWitnessEnum::Witness(witness) = input.input { - if witness_assignments.contains_key(&witness) { + if let ConstantOrWitnessEnum::Witness(ref witness) = input.input_ref() { + if witness_assignments.contains_key(witness) { None } else { - Some(witness) + Some(*witness) } } else { None diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 710ed3841de..d3b4087203d 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -6,7 +6,7 @@ use acir::{ brillig::ForeignCallResult, circuit::{ brillig::{BrilligBytecode, BrilligFunctionId}, - opcodes::{AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput}, + opcodes::{AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize}, AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, }, @@ -128,6 +128,8 @@ pub enum OpcodeResolutionError { }, #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")] IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 }, + #[error("Cannot solve opcode: {invalid_input_bit_size}")] + InvalidInputBitSize { opcode_location: ErrorLocation, invalid_input_bit_size: InvalidInputBitSize }, #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), #[error("Failed to solve brillig function")] @@ -387,6 +389,13 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { *opcode_index = ErrorLocation::Resolved(location); *assertion_payload = self.extract_assertion_payload(location); } + OpcodeResolutionError::InvalidInputBitSize { + opcode_location: opcode_index, + .. + } => { + let location = OpcodeLocation::Acir(self.instruction_pointer()); + *opcode_index = ErrorLocation::Resolved(location); + } // All other errors are thrown normally. _ => (), }; @@ -637,11 +646,22 @@ pub fn input_to_value( initial_witness: &WitnessMap, input: FunctionInput, ) -> Result> { - match input.input { + match input.input() { ConstantOrWitnessEnum::Witness(witness) => { let initial_value = *witness_to_value(initial_witness, witness)?; - assert!(initial_value.num_bits() <= input.num_bits); - Ok(initial_value) + if initial_value.num_bits() <= input.num_bits() { + Ok(initial_value) + } else { + Err(OpcodeResolutionError::InvalidInputBitSize { + opcode_location: ErrorLocation::Unresolved, + + invalid_input_bit_size: InvalidInputBitSize { + value: format!("{}", initial_value), + value_num_bits: initial_value.num_bits(), + num_bits: input.num_bits(), + }, + }) + } }, ConstantOrWitnessEnum::Constant(value) => Ok(value), } diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 2a06e07f092..3838afe7ea3 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -18,6 +18,7 @@ use acvm_blackbox_solver::StubbedBlackBoxSolver; use bn254_blackbox_solver::{field_from_hex, Bn254BlackBoxSolver, POSEIDON2_CONFIG}; use brillig_vm::brillig::HeapValueType; +use num_bigint::BigUint; use proptest::arbitrary::any; use proptest::prelude::*; use proptest::result::maybe_ok; @@ -776,15 +777,15 @@ fn constant_or_witness_to_function_inputs( xs: Vec, offset: usize, num_bits: Option, -) -> Vec> { +) -> Result>, OpcodeResolutionError> { let num_bits = num_bits.unwrap_or(FieldElement::max_num_bits()); xs.into_iter() .enumerate() .map(|(i, (x, use_constant))| { if use_constant { - FunctionInput::constant(x, num_bits) + FunctionInput::constant(x, num_bits).map_err(From::from) } else { - FunctionInput::witness(Witness((i + offset) as u32), num_bits) + Ok(FunctionInput::witness(Witness((i + offset) as u32), num_bits)) } }) .collect() @@ -810,9 +811,9 @@ fn solve_array_input_blackbox_call( num_outputs: usize, num_bits: Option, f: F, -) -> Vec +) -> Result, OpcodeResolutionError> where - F: FnOnce((Vec>, Vec)) -> BlackBoxFuncCall, + F: FnOnce((Vec>, Vec)) -> Result, OpcodeResolutionError>, { let initial_witness_vec: Vec<_> = inputs.iter().enumerate().map(|(i, (x, _))| (Witness(i as u32), *x)).collect(); @@ -821,8 +822,8 @@ where .collect(); let initial_witness = WitnessMap::from(BTreeMap::from_iter(initial_witness_vec)); - let inputs = constant_or_witness_to_function_inputs(inputs, 0, num_bits); - let op = Opcode::BlackBoxFuncCall(f((inputs.clone(), outputs.clone()))); + let inputs = constant_or_witness_to_function_inputs(inputs, 0, num_bits)?; + let op = Opcode::BlackBoxFuncCall(f((inputs.clone(), outputs.clone()))?); let opcodes = vec![op]; let unconstrained_functions = vec![]; let mut acvm = @@ -831,10 +832,10 @@ where assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); - outputs + Ok(outputs .iter() .map(|witness| *witness_map.get(witness).expect("all witnesses to be set")) - .collect() + .collect()) } prop_compose! { @@ -919,7 +920,7 @@ fn bigint_solve_binary_op_opt( modulus: Vec, lhs: Vec, rhs: Vec, -) -> Vec { +) -> Result, OpcodeResolutionError> { let initial_witness_vec: Vec<_> = lhs .iter() .chain(rhs.iter()) @@ -934,8 +935,8 @@ fn bigint_solve_binary_op_opt( .collect(); let initial_witness = WitnessMap::from(BTreeMap::from_iter(initial_witness_vec)); - let lhs = constant_or_witness_to_function_inputs(lhs, 0, None); - let rhs = constant_or_witness_to_function_inputs(rhs, lhs.len(), None); + let lhs = constant_or_witness_to_function_inputs(lhs, 0, None)?; + let rhs = constant_or_witness_to_function_inputs(rhs, lhs.len(), None)?; let to_op_input = if middle_op.is_some() { 2 } else { 0 }; @@ -966,10 +967,10 @@ fn bigint_solve_binary_op_opt( let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); - output_witnesses + Ok(output_witnesses .iter() .map(|witness| *witness_map.get(witness).expect("all witnesses to be set")) - .collect() + .collect()) } // Solve the given BlackBoxFuncCall with witnesses: 1, 2 as x, y, resp. @@ -978,10 +979,10 @@ fn solve_blackbox_func_call( blackbox_func_call: impl Fn( Option, Option, - ) -> BlackBoxFuncCall, + ) -> Result, OpcodeResolutionError>, lhs: (FieldElement, bool), // if false, use a Witness rhs: (FieldElement, bool), // if false, use a Witness -) -> FieldElement { +) -> Result> { let (lhs, lhs_constant) = lhs; let (rhs, rhs_constant) = rhs; @@ -998,7 +999,7 @@ fn solve_blackbox_func_call( rhs_opt = Some(rhs); } - let op = Opcode::BlackBoxFuncCall(blackbox_func_call(lhs_opt, rhs_opt)); + let op = Opcode::BlackBoxFuncCall(blackbox_func_call(lhs_opt, rhs_opt)?); let opcodes = vec![op]; let unconstrained_functions = vec![]; let mut acvm = @@ -1007,60 +1008,60 @@ fn solve_blackbox_func_call( assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); - witness_map[&Witness(3)] + Ok(witness_map[&Witness(3)]) } // N inputs // 32 outputs fn sha256_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; - BlackBoxFuncCall::SHA256 { + Ok(BlackBoxFuncCall::SHA256 { inputs: function_inputs, outputs: outputs.try_into().expect("SHA256 returns 32 outputs"), - } + }) } // N inputs // 32 outputs fn blake2s_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; - BlackBoxFuncCall::Blake2s { + Ok(BlackBoxFuncCall::Blake2s { inputs: function_inputs, outputs: outputs.try_into().expect("Blake2s returns 32 outputs"), - } + }) } // N inputs // 32 outputs fn blake3_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; - BlackBoxFuncCall::Blake3 { + Ok(BlackBoxFuncCall::Blake3 { inputs: function_inputs, outputs: outputs.try_into().expect("Blake3 returns 32 outputs"), - } + }) } // variable inputs // 32 outputs fn keccak256_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; let function_inputs_len = function_inputs.len(); - BlackBoxFuncCall::Keccak256 { + Ok(BlackBoxFuncCall::Keccak256 { inputs: function_inputs, var_message_size: FunctionInput::constant( function_inputs_len.into(), FieldElement::max_num_bits(), - ), + )?, outputs: outputs.try_into().expect("Keccak256 returns 32 outputs"), - } + }) } // var_message_size is the number of bytes to take @@ -1072,65 +1073,65 @@ fn keccak256_op( // 32 outputs fn keccak256_invalid_message_size_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; let function_inputs_len = function_inputs.len(); - BlackBoxFuncCall::Keccak256 { + Ok(BlackBoxFuncCall::Keccak256 { inputs: function_inputs, var_message_size: FunctionInput::constant( (function_inputs_len - 1).into(), FieldElement::max_num_bits(), - ), + )?, outputs: outputs.try_into().expect("Keccak256 returns 32 outputs"), - } + }) } // 25 inputs // 25 outputs fn keccakf1600_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; - BlackBoxFuncCall::Keccakf1600 { + Ok(BlackBoxFuncCall::Keccakf1600 { inputs: function_inputs.try_into().expect("Keccakf1600 expects 25 inputs"), outputs: outputs.try_into().expect("Keccakf1600 returns 25 outputs"), - } + }) } // N inputs // N outputs fn poseidon2_permutation_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (inputs, outputs) = function_inputs_and_outputs; let len = inputs.len() as u32; - BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len } + Ok(BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len }) } // N inputs // N outputs fn poseidon2_permutation_invalid_len_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (inputs, outputs) = function_inputs_and_outputs; let len = (inputs.len() as u32) + 1; - BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len } + Ok(BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len }) } // 24 inputs (16 + 8) // 8 outputs fn sha256_compression_op( function_inputs_and_outputs: (Vec>, Vec), -) -> BlackBoxFuncCall { +) -> Result, OpcodeResolutionError> { let (function_inputs, outputs) = function_inputs_and_outputs; let mut function_inputs = function_inputs.into_iter(); let inputs = core::array::from_fn(|_| function_inputs.next().unwrap()); let hash_values = core::array::from_fn(|_| function_inputs.next().unwrap()); - BlackBoxFuncCall::Sha256Compression { + Ok(BlackBoxFuncCall::Sha256Compression { inputs: Box::new(inputs), hash_values: Box::new(hash_values), outputs: outputs.try_into().unwrap(), - } + }) } fn into_repr_vec(fields: T) -> Vec @@ -1150,13 +1151,13 @@ where fn run_both_poseidon2_permutations( inputs: Vec, -) -> (Vec, Vec) { +) -> Result<(Vec, Vec), OpcodeResolutionError> { let result = solve_array_input_blackbox_call( inputs.clone(), inputs.len(), None, poseidon2_permutation_op, - ); + )?; let poseidon2_t = POSEIDON2_CONFIG.t as usize; let poseidon2_d = 5; @@ -1179,7 +1180,7 @@ fn run_both_poseidon2_permutations( let expected_result = external_poseidon2.permutation(&into_repr_vec(drop_use_constant(&inputs))); - (into_repr_vec(result), expected_result) + Ok((into_repr_vec(result), expected_result)) } // Using the given BigInt modulus, solve the following circuit: @@ -1195,7 +1196,7 @@ fn bigint_solve_binary_op( lhs: Vec, rhs: Vec, ) -> Vec { - bigint_solve_binary_op_opt(Some(middle_op), modulus, lhs, rhs) + bigint_solve_binary_op_opt(Some(middle_op), modulus, lhs, rhs).unwrap() } // Using the given BigInt modulus, solve the following circuit: @@ -1206,69 +1207,69 @@ fn bigint_solve_from_to_le_bytes( modulus: Vec, inputs: Vec, ) -> Vec { - bigint_solve_binary_op_opt(None, modulus, inputs, vec![]) + bigint_solve_binary_op_opt(None, modulus, inputs, vec![]).unwrap() } fn function_input_from_option( witness: Witness, opt_constant: Option, -) -> FunctionInput { +) -> Result, OpcodeResolutionError> { opt_constant - .map(|constant| FunctionInput::constant(constant, FieldElement::max_num_bits())) - .unwrap_or(FunctionInput::witness(witness, FieldElement::max_num_bits())) + .map(|constant| FunctionInput::constant(constant, FieldElement::max_num_bits()).map_err(From::from)) + .unwrap_or(Ok(FunctionInput::witness(witness, FieldElement::max_num_bits()))) } -fn and_op(x: Option, y: Option) -> BlackBoxFuncCall { - let lhs = function_input_from_option(Witness(1), x); - let rhs = function_input_from_option(Witness(2), y); - BlackBoxFuncCall::AND { lhs, rhs, output: Witness(3) } +fn and_op(x: Option, y: Option) -> Result, OpcodeResolutionError> { + let lhs = function_input_from_option(Witness(1), x)?; + let rhs = function_input_from_option(Witness(2), y)?; + Ok(BlackBoxFuncCall::AND { lhs, rhs, output: Witness(3) }) } -fn xor_op(x: Option, y: Option) -> BlackBoxFuncCall { - let lhs = function_input_from_option(Witness(1), x); - let rhs = function_input_from_option(Witness(2), y); - BlackBoxFuncCall::XOR { lhs, rhs, output: Witness(3) } +fn xor_op(x: Option, y: Option) -> Result, OpcodeResolutionError> { + let lhs = function_input_from_option(Witness(1), x)?; + let rhs = function_input_from_option(Witness(2), y)?; + Ok(BlackBoxFuncCall::XOR { lhs, rhs, output: Witness(3) }) } fn prop_assert_commutative( - op: impl Fn(Option, Option) -> BlackBoxFuncCall, + op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, x: (FieldElement, bool), y: (FieldElement, bool), ) -> (FieldElement, FieldElement) { - (solve_blackbox_func_call(&op, x, y), solve_blackbox_func_call(&op, y, x)) + (solve_blackbox_func_call(&op, x, y).unwrap(), solve_blackbox_func_call(&op, y, x).unwrap()) } fn prop_assert_associative( - op: impl Fn(Option, Option) -> BlackBoxFuncCall, + op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, x: (FieldElement, bool), y: (FieldElement, bool), z: (FieldElement, bool), use_constant_xy: bool, use_constant_yz: bool, ) -> (FieldElement, FieldElement) { - let f_xy = (solve_blackbox_func_call(&op, x, y), use_constant_xy); - let f_f_xy_z = solve_blackbox_func_call(&op, f_xy, z); + let f_xy = (solve_blackbox_func_call(&op, x, y).unwrap(), use_constant_xy); + let f_f_xy_z = solve_blackbox_func_call(&op, f_xy, z).unwrap(); - let f_yz = (solve_blackbox_func_call(&op, y, z), use_constant_yz); - let f_x_f_yz = solve_blackbox_func_call(&op, x, f_yz); + let f_yz = (solve_blackbox_func_call(&op, y, z).unwrap(), use_constant_yz); + let f_x_f_yz = solve_blackbox_func_call(&op, x, f_yz).unwrap(); (f_f_xy_z, f_x_f_yz) } fn prop_assert_identity_l( - op: impl Fn(Option, Option) -> BlackBoxFuncCall, + op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, op_identity: (FieldElement, bool), x: (FieldElement, bool), ) -> (FieldElement, FieldElement) { - (solve_blackbox_func_call(op, op_identity, x), x.0) + (solve_blackbox_func_call(op, op_identity, x).unwrap(), x.0) } fn prop_assert_zero_l( - op: impl Fn(Option, Option) -> BlackBoxFuncCall, + op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, op_zero: (FieldElement, bool), x: (FieldElement, bool), ) -> (FieldElement, FieldElement) { - (solve_blackbox_func_call(op, op_zero, x), FieldElement::zero()) + (solve_blackbox_func_call(op, op_zero, x).unwrap(), FieldElement::zero()) } // Test that varying one of the inputs produces a different result @@ -1282,14 +1283,14 @@ fn prop_assert_injective( op: F, ) -> (bool, String) where - F: FnOnce((Vec>, Vec)) -> BlackBoxFuncCall + F: FnOnce((Vec>, Vec)) -> Result, OpcodeResolutionError> + Clone, { let equal_inputs = drop_use_constant_eq(&inputs, &distinct_inputs); let message = format!("not injective:\n{:?}\n{:?}", &inputs, &distinct_inputs); let outputs_not_equal = - solve_array_input_blackbox_call(inputs, num_outputs, num_bits, op.clone()) - != solve_array_input_blackbox_call(distinct_inputs, num_outputs, num_bits, op); + solve_array_input_blackbox_call(inputs, num_outputs, num_bits, op.clone()).unwrap() + != solve_array_input_blackbox_call(distinct_inputs, num_outputs, num_bits, op).unwrap(); (equal_inputs || outputs_not_equal, message) } @@ -1322,12 +1323,12 @@ prop_compose! { -> (Vec, Vec) { let (_size, patch_location, patch_value) = size_and_patch; let (inputs, distinct_inputs) = inputs_distinct_inputs; + let modulus = if let Some(max_input_bits) = max_input_bits { + 1u128 << max_input_bits + } else { + 1 + }; let to_input = |(x, use_constant)| { - let modulus = if let Some(max_input_bits) = max_input_bits { - 2u128 << max_input_bits - } else { - 1 - }; (FieldElement::from(x % modulus), use_constant) }; let inputs: Vec<_> = inputs.into_iter().map(to_input).collect(); @@ -1338,9 +1339,11 @@ prop_compose! { let distinct_inputs_len = distinct_inputs.len(); let positive_patch_value = std::cmp::max(patch_value, 1); if distinct_inputs_len != 0 { - distinct_inputs[patch_location % distinct_inputs_len].0 += FieldElement::from(positive_patch_value) + let previous_input = &mut distinct_inputs[patch_location % distinct_inputs_len].0; + let patched_input: BigUint = (*previous_input + FieldElement::from(positive_patch_value)).into(); + *previous_input = (patched_input % BigUint::from(modulus)).into(); } else { - distinct_inputs.push((FieldElement::zero(), true)) + distinct_inputs.push((FieldElement::zero(), true)); } } @@ -1352,17 +1355,17 @@ prop_compose! { fn poseidon2_permutation_zeroes() { let use_constants: [bool; 4] = [false; 4]; let inputs: Vec<_> = [FieldElement::zero(); 4].into_iter().zip(use_constants).collect(); - let (result, expected_result) = run_both_poseidon2_permutations(inputs); + let (results, expected_results) = run_both_poseidon2_permutations(inputs).unwrap(); - let internal_expected_result = vec![ + let internal_expected_results = vec![ field_from_hex("18DFB8DC9B82229CFF974EFEFC8DF78B1CE96D9D844236B496785C698BC6732E"), field_from_hex("095C230D1D37A246E8D2D5A63B165FE0FADE040D442F61E25F0590E5FB76F839"), field_from_hex("0BB9545846E1AFA4FA3C97414A60A20FC4949F537A68CCECA34C5CE71E28AA59"), field_from_hex("18A4F34C9C6F99335FF7638B82AEED9018026618358873C982BBDDE265B2ED6D"), ]; - assert_eq!(expected_result, into_repr_vec(internal_expected_result)); - assert_eq!(result, expected_result); + assert_eq!(expected_results, into_repr_vec(internal_expected_results)); + assert_eq!(results, expected_results); } #[test] @@ -1375,7 +1378,7 @@ fn sha256_zeros() { .into_iter() .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } #[test] @@ -1393,7 +1396,7 @@ fn sha256_compression_zeros() { .into_iter() .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } #[test] @@ -1406,7 +1409,7 @@ fn blake2s_zeros() { .into_iter() .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } #[test] @@ -1419,7 +1422,7 @@ fn blake3_zeros() { .into_iter() .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } #[test] @@ -1432,7 +1435,7 @@ fn keccak256_zeros() { .into_iter() .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } #[test] @@ -1474,7 +1477,7 @@ fn keccakf1600_zeros() { .map(|x: u128| FieldElement::from(x)) .collect(); - assert_eq!(results, expected_results); + assert_eq!(results, Ok(expected_results)); } // NOTE: an "average" bigint is large, so consider increasing the number of proptest shrinking @@ -1511,13 +1514,13 @@ proptest! { // test that AND(x, x) == x #[test] fn and_self_identity(x in field_element()) { - prop_assert_eq!(solve_blackbox_func_call(and_op, x, x), x.0); + prop_assert_eq!(solve_blackbox_func_call(and_op, x, x).unwrap(), x.0); } // test that XOR(x, x) == 0 #[test] fn xor_self_zero(x in field_element()) { - prop_assert_eq!(solve_blackbox_func_call(xor_op, x, x), FieldElement::zero()); + prop_assert_eq!(solve_blackbox_func_call(xor_op, x, x).unwrap(), FieldElement::zero()); } #[test] @@ -1547,7 +1550,7 @@ proptest! { #[test] fn poseidon2_permutation_matches_external_impl(inputs in proptest::collection::vec(field_element(), 4)) { - let (result, expected_result) = run_both_poseidon2_permutations(inputs); + let (result, expected_result) = run_both_poseidon2_permutations(inputs).unwrap(); prop_assert_eq!(result, expected_result) } @@ -1584,7 +1587,7 @@ proptest! { #[test] fn keccak256_injective(inputs_distinct_inputs in any_distinct_inputs(Some(8), 0, 32)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, Some(32), keccak256_op); + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, Some(8), keccak256_op); prop_assert!(result, "{}", message); } diff --git a/acvm-repo/acvm_js/src/execute.rs b/acvm-repo/acvm_js/src/execute.rs index 98a0c4c3abe..e9cd62098de 100644 --- a/acvm-repo/acvm_js/src/execute.rs +++ b/acvm-repo/acvm_js/src/execute.rs @@ -201,6 +201,10 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { opcode_location: ErrorLocation::Resolved(opcode_location), .. } => Some(vec![*opcode_location]), + | OpcodeResolutionError::InvalidInputBitSize { + opcode_location: ErrorLocation::Resolved(opcode_location), + .. + } => Some(vec![*opcode_location]), OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { Some(call_stack.clone()) } diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 2c7ec0f8e1a..d7fe695442d 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::FieldElement; +use acvm::{FieldElement, acir::InvalidInputBitSize}; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; @@ -54,6 +54,8 @@ pub enum RuntimeError { UnconstrainedOracleReturnToConstrained { call_stack: CallStack }, #[error("Could not resolve some references to the array. All references must be resolved at compile time")] UnknownReference { call_stack: CallStack }, + #[error("{invalid_input_bit_size}")] + InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -155,6 +157,7 @@ impl RuntimeError { | RuntimeError::UnsupportedIntegerSize { call_stack, .. } | RuntimeError::NestedSlice { call_stack, .. } | RuntimeError::BigIntModulus { call_stack, .. } + | RuntimeError::InvalidInputBitSize { call_stack, .. } | RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack } | RuntimeError::UnconstrainedOracleReturnToConstrained { call_stack } | RuntimeError::UnknownReference { call_stack } => call_stack, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index a0be1ee19cf..e44e99388f1 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1502,7 +1502,12 @@ impl AcirContext { let num_bits = typ.bit_size::(); match self.vars[&input].as_constant() { Some(constant) if allow_constant_inputs => { - single_val_witnesses.push(FunctionInput::constant(*constant, num_bits)); + single_val_witnesses.push(FunctionInput::constant(*constant, num_bits).map_err(|invalid_input_bit_size| { + RuntimeError::InvalidInputBitSize { + invalid_input_bit_size, + call_stack: self.get_call_stack(), + } + })?); } _ => { let witness_var = self.get_or_create_witness_var(input)?; diff --git a/test_programs/noir_test_success/regression_4080/src/main.nr b/test_programs/noir_test_success/regression_4080/src/main.nr index 781d3e33ea3..71231fb8ef2 100644 --- a/test_programs/noir_test_success/regression_4080/src/main.nr +++ b/test_programs/noir_test_success/regression_4080/src/main.nr @@ -1,7 +1,8 @@ // This test checks that `var^var` is assigned the correct type. // https://github.com/noir-lang/noir/issues/4080 -#[test(should_fail_with = "attempt to add with overflow")] +// NOTE: expected error is the following, but the 'failed_assertion' is None in check_expected_failure_message +#[test(should_fail_with = "Cannot solve opcode: FunctionInput value has too many bits")] fn main() { let var1: u8 = ((255 + 1) ^ (255 + 1)) - ((255 + 1) - (255 + 1)); assert_eq(var1, 0); diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs index 942462c4f37..ee7be65efca 100644 --- a/tooling/fuzzer/src/dictionary/mod.rs +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -10,7 +10,7 @@ use acvm::{ circuit::{ brillig::{BrilligBytecode, BrilligInputs}, directives::Directive, - opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput}, + opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum}, Circuit, Opcode, Program, }, native_types::Expression, @@ -83,18 +83,18 @@ fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet< } } - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { - input: FunctionInput { input: ConstantOrWitnessEnum::Constant(c), num_bits }, - }) => { - let field = 1u128.wrapping_shl(*num_bits); - constants.insert(F::from(field)); - constants.insert(F::from(field - 1)); - constants.insert(*c); + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) if matches!(input.input(), ConstantOrWitnessEnum::Constant(..)) => { + if let ConstantOrWitnessEnum::Constant(c) = input.input() { + let field = 1u128.wrapping_shl(input.num_bits()); + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + constants.insert(c); + } else { + unreachable!("matches! above ensures the other case is matched") + } } - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { - input: FunctionInput { input: ConstantOrWitnessEnum::Witness(_), num_bits }, - }) => { - let field = 1u128.wrapping_shl(*num_bits); + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => { + let field = 1u128.wrapping_shl(input.num_bits()); constants.insert(F::from(field)); constants.insert(F::from(field - 1)); } diff --git a/tooling/nargo/src/errors.rs b/tooling/nargo/src/errors.rs index 0b8378702cb..4a72bb1ec78 100644 --- a/tooling/nargo/src/errors.rs +++ b/tooling/nargo/src/errors.rs @@ -115,6 +115,10 @@ fn extract_locations_from_error( OpcodeResolutionError::IndexOutOfBounds { opcode_location: error_location, .. }, acir_call_stack, ) + | ExecutionError::SolvingError( + OpcodeResolutionError::InvalidInputBitSize { opcode_location: error_location, .. }, + acir_call_stack, + ) | ExecutionError::SolvingError( OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: error_location, .. }, acir_call_stack, diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index 59d554d7ca5..eb03bdf01c1 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -89,6 +89,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> | OpcodeResolutionError::IndexOutOfBounds { opcode_location: ErrorLocation::Resolved(opcode_location), .. + } + | OpcodeResolutionError::InvalidInputBitSize { + opcode_location: ErrorLocation::Resolved(opcode_location), + .. } => { let resolved_location = ResolvedOpcodeLocation { acir_function_index: self.current_function_index, diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index efe648e09b0..2c46783e4d6 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -191,6 +191,9 @@ fn check_expected_failure_message( None => return TestStatus::Pass, }; + // TODO: cleanup + dbg!(test_function.failure_reason()); + let expected_failure_message_matches = matches!(&failed_assertion, Some(message) if message.contains(expected_failure_message)); if expected_failure_message_matches { From 0021a68fa46f880b656a9d528ec22e28daa1dd40 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 13:56:01 -0400 Subject: [PATCH 03/22] ensure that 'should_fail_with' in noir tests matches against the error that the user sees, cargo fmt/clippy --- acvm-repo/acir/src/circuit/opcodes.rs | 4 +- .../opcodes/black_box_function_call.rs | 1 - .../compiler/optimizers/redundant_range.rs | 6 ++- acvm-repo/acvm/src/pwg/mod.rs | 11 +++-- acvm-repo/acvm/tests/solver.rs | 48 +++++++++++++++---- acvm-repo/acvm_js/src/execute.rs | 2 +- compiler/noirc_evaluator/src/errors.rs | 2 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 14 +++--- tooling/fuzzer/src/dictionary/mod.rs | 4 +- tooling/nargo/src/ops/test.rs | 15 ++++-- 10 files changed, 76 insertions(+), 31 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes.rs b/acvm-repo/acir/src/circuit/opcodes.rs index 598aa65052e..848d7bda84b 100644 --- a/acvm-repo/acir/src/circuit/opcodes.rs +++ b/acvm-repo/acir/src/circuit/opcodes.rs @@ -13,7 +13,9 @@ use serde::{Deserialize, Serialize}; mod black_box_function_call; mod memory_operation; -pub use black_box_function_call::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize}; +pub use black_box_function_call::{ + BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize, +}; pub use memory_operation::{BlockId, MemOp}; #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 795c2142418..83dd1bf1278 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -44,7 +44,6 @@ impl FunctionInput { } } - #[derive(Clone, PartialEq, Eq, Debug, Error)] #[error("FunctionInput value has too many bits: value: {value}, {value_num_bits} >= {num_bits}")] pub struct InvalidInputBitSize { diff --git a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs index 06196b73c0f..159ca9ba85d 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs @@ -110,13 +110,15 @@ impl RangeOptimizer { let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len()); for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() { let (witness, num_bits) = match opcode { - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) if matches!(input.input_ref(), ConstantOrWitnessEnum::Witness(..)) => { + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) + if matches!(input.input_ref(), ConstantOrWitnessEnum::Witness(..)) => + { if let ConstantOrWitnessEnum::Witness(witness) = input.input() { (witness, input.num_bits()) } else { unreachable!("The matches! above ensures only a witness is possible") } - }, + } _ => { // If its not the range opcode, add it to the opcode // list and continue; diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index d3b4087203d..59bf852579d 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -6,7 +6,9 @@ use acir::{ brillig::ForeignCallResult, circuit::{ brillig::{BrilligBytecode, BrilligFunctionId}, - opcodes::{AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize}, + opcodes::{ + AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize, + }, AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, }, @@ -129,7 +131,10 @@ pub enum OpcodeResolutionError { #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")] IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 }, #[error("Cannot solve opcode: {invalid_input_bit_size}")] - InvalidInputBitSize { opcode_location: ErrorLocation, invalid_input_bit_size: InvalidInputBitSize }, + InvalidInputBitSize { + opcode_location: ErrorLocation, + invalid_input_bit_size: InvalidInputBitSize, + }, #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), #[error("Failed to solve brillig function")] @@ -662,7 +667,7 @@ pub fn input_to_value( }, }) } - }, + } ConstantOrWitnessEnum::Constant(value) => Ok(value), } } diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 3838afe7ea3..012b4b0fe6d 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -813,7 +813,9 @@ fn solve_array_input_blackbox_call( f: F, ) -> Result, OpcodeResolutionError> where - F: FnOnce((Vec>, Vec)) -> Result, OpcodeResolutionError>, + F: FnOnce( + (Vec>, Vec), + ) -> Result, OpcodeResolutionError>, { let initial_witness_vec: Vec<_> = inputs.iter().enumerate().map(|(i, (x, _))| (Witness(i as u32), *x)).collect(); @@ -979,7 +981,10 @@ fn solve_blackbox_func_call( blackbox_func_call: impl Fn( Option, Option, - ) -> Result, OpcodeResolutionError>, + ) -> Result< + BlackBoxFuncCall, + OpcodeResolutionError, + >, lhs: (FieldElement, bool), // if false, use a Witness rhs: (FieldElement, bool), // if false, use a Witness ) -> Result> { @@ -1215,24 +1220,35 @@ fn function_input_from_option( opt_constant: Option, ) -> Result, OpcodeResolutionError> { opt_constant - .map(|constant| FunctionInput::constant(constant, FieldElement::max_num_bits()).map_err(From::from)) + .map(|constant| { + FunctionInput::constant(constant, FieldElement::max_num_bits()).map_err(From::from) + }) .unwrap_or(Ok(FunctionInput::witness(witness, FieldElement::max_num_bits()))) } -fn and_op(x: Option, y: Option) -> Result, OpcodeResolutionError> { +fn and_op( + x: Option, + y: Option, +) -> Result, OpcodeResolutionError> { let lhs = function_input_from_option(Witness(1), x)?; let rhs = function_input_from_option(Witness(2), y)?; Ok(BlackBoxFuncCall::AND { lhs, rhs, output: Witness(3) }) } -fn xor_op(x: Option, y: Option) -> Result, OpcodeResolutionError> { +fn xor_op( + x: Option, + y: Option, +) -> Result, OpcodeResolutionError> { let lhs = function_input_from_option(Witness(1), x)?; let rhs = function_input_from_option(Witness(2), y)?; Ok(BlackBoxFuncCall::XOR { lhs, rhs, output: Witness(3) }) } fn prop_assert_commutative( - op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, + op: impl Fn( + Option, + Option, + ) -> Result, OpcodeResolutionError>, x: (FieldElement, bool), y: (FieldElement, bool), ) -> (FieldElement, FieldElement) { @@ -1240,7 +1256,10 @@ fn prop_assert_commutative( } fn prop_assert_associative( - op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, + op: impl Fn( + Option, + Option, + ) -> Result, OpcodeResolutionError>, x: (FieldElement, bool), y: (FieldElement, bool), z: (FieldElement, bool), @@ -1257,7 +1276,10 @@ fn prop_assert_associative( } fn prop_assert_identity_l( - op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, + op: impl Fn( + Option, + Option, + ) -> Result, OpcodeResolutionError>, op_identity: (FieldElement, bool), x: (FieldElement, bool), ) -> (FieldElement, FieldElement) { @@ -1265,7 +1287,10 @@ fn prop_assert_identity_l( } fn prop_assert_zero_l( - op: impl Fn(Option, Option) -> Result, OpcodeResolutionError>, + op: impl Fn( + Option, + Option, + ) -> Result, OpcodeResolutionError>, op_zero: (FieldElement, bool), x: (FieldElement, bool), ) -> (FieldElement, FieldElement) { @@ -1283,7 +1308,10 @@ fn prop_assert_injective( op: F, ) -> (bool, String) where - F: FnOnce((Vec>, Vec)) -> Result, OpcodeResolutionError> + F: FnOnce( + (Vec>, Vec), + ) + -> Result, OpcodeResolutionError> + Clone, { let equal_inputs = drop_use_constant_eq(&inputs, &distinct_inputs); diff --git a/acvm-repo/acvm_js/src/execute.rs b/acvm-repo/acvm_js/src/execute.rs index e9cd62098de..c3627d0eff5 100644 --- a/acvm-repo/acvm_js/src/execute.rs +++ b/acvm-repo/acvm_js/src/execute.rs @@ -201,7 +201,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { opcode_location: ErrorLocation::Resolved(opcode_location), .. } => Some(vec![*opcode_location]), - | OpcodeResolutionError::InvalidInputBitSize { + OpcodeResolutionError::InvalidInputBitSize { opcode_location: ErrorLocation::Resolved(opcode_location), .. } => Some(vec![*opcode_location]), diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index d7fe695442d..13a237bbea0 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::{FieldElement, acir::InvalidInputBitSize}; +use acvm::{acir::InvalidInputBitSize, FieldElement}; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index e44e99388f1..fdb3dfc429b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1502,12 +1502,14 @@ impl AcirContext { let num_bits = typ.bit_size::(); match self.vars[&input].as_constant() { Some(constant) if allow_constant_inputs => { - single_val_witnesses.push(FunctionInput::constant(*constant, num_bits).map_err(|invalid_input_bit_size| { - RuntimeError::InvalidInputBitSize { - invalid_input_bit_size, - call_stack: self.get_call_stack(), - } - })?); + single_val_witnesses.push( + FunctionInput::constant(*constant, num_bits).map_err( + |invalid_input_bit_size| RuntimeError::InvalidInputBitSize { + invalid_input_bit_size, + call_stack: self.get_call_stack(), + }, + )?, + ); } _ => { let witness_var = self.get_or_create_witness_var(input)?; diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs index ee7be65efca..6bc15d7c5fe 100644 --- a/tooling/fuzzer/src/dictionary/mod.rs +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -83,7 +83,9 @@ fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet< } } - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) if matches!(input.input(), ConstantOrWitnessEnum::Constant(..)) => { + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) + if matches!(input.input(), ConstantOrWitnessEnum::Constant(..)) => + { if let ConstantOrWitnessEnum::Constant(c) = input.input() { let field = 1u128.wrapping_shl(input.num_bits()); constants.insert(F::from(field)); diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 2c46783e4d6..370a4235f61 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -191,11 +191,16 @@ fn check_expected_failure_message( None => return TestStatus::Pass, }; - // TODO: cleanup - dbg!(test_function.failure_reason()); - - let expected_failure_message_matches = - matches!(&failed_assertion, Some(message) if message.contains(expected_failure_message)); + // Match the failure message that the user will see, i.e. the failed_assertion + // if present or else the error_diagnostic's message, against the + // expected_failure_message + let expected_failure_message_matches = failed_assertion + .as_ref() + .or_else(|| { + error_diagnostic.as_ref().map(|file_diagnostic| &file_diagnostic.diagnostic.message) + }) + .map(|message| message.contains(expected_failure_message)) + .unwrap_or(false); if expected_failure_message_matches { return TestStatus::Pass; } From d5ebc499242575522966eb17692e0d78cbafdc2d Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 16:05:14 -0400 Subject: [PATCH 04/22] cargo clippy/fmt --- acvm-repo/acir_field/src/field_element.rs | 6 +++--- .../acvm/src/compiler/optimizers/redundant_range.rs | 2 +- acvm-repo/acvm/src/pwg/mod.rs | 9 +++++++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs index 3d22cb4a909..102821d0911 100644 --- a/acvm-repo/acir_field/src/field_element.rs +++ b/acvm-repo/acir_field/src/field_element.rs @@ -143,9 +143,9 @@ impl From for FieldElement { } } -impl Into for FieldElement { - fn into(self) -> BigUint { - F::into(self.0) +impl From> for BigUint { + fn from(a: FieldElement) -> BigUint { + F::into(a.0) } } diff --git a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs index 159ca9ba85d..e01908d3222 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs @@ -75,7 +75,7 @@ impl RangeOptimizer { Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => { if let ConstantOrWitnessEnum::Witness(witness) = input.input() { - Some((witness.clone(), input.num_bits())) + Some((witness, input.num_bits())) } else { None } diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 59bf852579d..22a90031a44 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -159,6 +159,15 @@ impl From for OpcodeResolutionError { } } +impl From for OpcodeResolutionError { + fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { + Self::InvalidInputBitSize { + opcode_location: ErrorLocation::Unresolved, + invalid_input_bit_size, + } + } +} + pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { status: ACVMStatus, From 07f3e69da465d11c64dc7620d8f5d540382fac6e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 16:24:30 -0400 Subject: [PATCH 05/22] nargo fmt --- .../compile_success_empty/unquote_struct/src/main.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test_programs/compile_success_empty/unquote_struct/src/main.nr b/test_programs/compile_success_empty/unquote_struct/src/main.nr index e90711dd710..603440b5c76 100644 --- a/test_programs/compile_success_empty/unquote_struct/src/main.nr +++ b/test_programs/compile_success_empty/unquote_struct/src/main.nr @@ -10,11 +10,13 @@ fn foo(x: Field, y: u32) -> u32 { // Given a function, wrap its parameters in a struct definition comptime fn output_struct(f: FunctionDefinition) -> Quoted { - let fields = f.parameters().map(|param: (Quoted, Type)| { + let fields = f.parameters().map( + |param: (Quoted, Type)| { let name = param.0; let typ = param.1; quote { $name: $typ, } - }).join(quote {}); + } + ).join(quote {}); quote { struct Foo { $fields } From adfe49004f5bae333758724b9328d14d70b1c73e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 16:55:19 -0400 Subject: [PATCH 06/22] patch acvm_js test to use inputs < 2^128 since the FunctionInputs used for the bytecode are of the form: FunctionInput::witness(Witness(_), 128) --- acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index 80fbf14e8f1..f928727d38f 100644 --- a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -6,7 +6,7 @@ export const bytecode = Uint8Array.from([ ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], + [2, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], @@ -14,11 +14,11 @@ export const initialWitnessMap = new Map([ export const expectedWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], + [2, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], [6, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [7, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], + [7, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], [8, '0x0000000000000000000000000000000000000000000000000000000000000000'], ]); From 6606cf0e4786036298536a8540275139c5efae23 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Wed, 4 Sep 2024 19:03:17 -0400 Subject: [PATCH 07/22] another attempt to get the bit size within the expected range --- acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index f928727d38f..8481ff59854 100644 --- a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -6,7 +6,7 @@ export const bytecode = Uint8Array.from([ ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], + [2, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], @@ -14,11 +14,11 @@ export const initialWitnessMap = new Map([ export const expectedWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], + [2, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], [6, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [7, '0x00000000000000000000000000a45d632d270d45f1181294833fc48d823f272c'], + [7, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], [8, '0x0000000000000000000000000000000000000000000000000000000000000000'], ]); From dd645b0052ab5334ffcad2b8f85609028fb1ea44 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 5 Sep 2024 11:36:29 -0400 Subject: [PATCH 08/22] update expected bit size for multi-scalar-mul TS tests to maximum --- .../acir/tests/test_program_serialization.rs | 17 +++++++---------- .../acvm_js/test/shared/multi_scalar_mul.ts | 12 ++++++------ 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/acvm-repo/acir/tests/test_program_serialization.rs b/acvm-repo/acir/tests/test_program_serialization.rs index 7bed57e22a0..3ddccccc636 100644 --- a/acvm-repo/acir/tests/test_program_serialization.rs +++ b/acvm-repo/acir/tests/test_program_serialization.rs @@ -62,13 +62,15 @@ fn multi_scalar_mul_circuit() { let multi_scalar_mul: Opcode = Opcode::BlackBoxFuncCall(BlackBoxFuncCall::MultiScalarMul { points: vec![ - FunctionInput::witness(Witness(1), 128), - FunctionInput::witness(Witness(2), 128), + FunctionInput::witness(Witness(1), FieldElement::max_num_bits()), + FunctionInput::witness(Witness(2), FieldElement::max_num_bits()), FunctionInput::witness(Witness(3), 1), ], scalars: vec![ - FunctionInput::witness(Witness(4), 128), - FunctionInput::witness(Witness(5), 128), + FunctionInput::witness(Witness(4), FieldElement::max_num_bits()), + FunctionInput::witness(Witness(5), FieldElement::max_num_bits()), + + ], outputs: (Witness(6), Witness(7), Witness(8)), }); @@ -90,12 +92,7 @@ fn multi_scalar_mul_circuit() { let bytes = Program::serialize_program(&program); - let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 232, - 142, 158, 210, 130, 149, 240, 112, 234, 212, 156, 78, 12, 39, 67, 71, 158, 142, 80, 29, 44, - 228, 66, 90, 168, 119, 189, 74, 115, 131, 174, 78, 115, 58, 124, 70, 254, 130, 59, 74, 253, - 68, 255, 255, 221, 39, 54, 221, 93, 91, 132, 193, 0, 0, 0, - ]; + let expected_serialization: Vec = vec![31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, 227, 70, 74, 11, 86, 194, 195, 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, 240, 25, 249, 11, 238, 40, 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0]; assert_eq!(bytes, expected_serialization) } diff --git a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index 8481ff59854..a6f6fb17ace 100644 --- a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -1,12 +1,12 @@ // See `multi_scalar_mul_circuit` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 232, 142, 158, 210, 130, 149, 240, - 112, 234, 212, 156, 78, 12, 39, 67, 71, 158, 142, 80, 29, 44, 228, 66, 90, 168, 119, 189, 74, 115, 131, 174, 78, 115, - 58, 124, 70, 254, 130, 59, 74, 253, 68, 255, 255, 221, 39, 54, 221, 93, 91, 132, 193, 0, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, 227, 70, 74, 11, 86, 194, 195, + 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, + 240, 25, 249, 11, 238, 40, 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0 ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], + [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], @@ -14,11 +14,11 @@ export const initialWitnessMap = new Map([ export const expectedWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [2, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], + [2, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], [3, '0x0000000000000000000000000000000000000000000000000000000000000000'], [4, '0x0000000000000000000000000000000000000000000000000000000000000001'], [5, '0x0000000000000000000000000000000000000000000000000000000000000000'], [6, '0x0000000000000000000000000000000000000000000000000000000000000001'], - [7, '0x0000000000000000000000000000000000270d45f1181294833fc48d823f272c'], + [7, '0x0000000000000002cf135e7506a45d632d270d45f1181294833fc48d823f272c'], [8, '0x0000000000000000000000000000000000000000000000000000000000000000'], ]); From 2c1894027588c99b573618d2cd8cd38a78781a70 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 5 Sep 2024 11:42:15 -0400 Subject: [PATCH 09/22] cargo fmt / yarn lint --- acvm-repo/acir/tests/test_program_serialization.rs | 9 ++++++--- acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/acvm-repo/acir/tests/test_program_serialization.rs b/acvm-repo/acir/tests/test_program_serialization.rs index 3ddccccc636..172cfc71d95 100644 --- a/acvm-repo/acir/tests/test_program_serialization.rs +++ b/acvm-repo/acir/tests/test_program_serialization.rs @@ -69,8 +69,6 @@ fn multi_scalar_mul_circuit() { scalars: vec![ FunctionInput::witness(Witness(4), FieldElement::max_num_bits()), FunctionInput::witness(Witness(5), FieldElement::max_num_bits()), - - ], outputs: (Witness(6), Witness(7), Witness(8)), }); @@ -92,7 +90,12 @@ fn multi_scalar_mul_circuit() { let bytes = Program::serialize_program(&program); - let expected_serialization: Vec = vec![31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, 227, 70, 74, 11, 86, 194, 195, 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, 240, 25, 249, 11, 238, 40, 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0]; + let expected_serialization: Vec = vec![ + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, + 227, 70, 74, 11, 86, 194, 195, 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, + 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, 240, 25, 249, 11, 238, 40, + 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0, + ]; assert_eq!(bytes, expected_serialization) } diff --git a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts index a6f6fb17ace..ffb9952b136 100644 --- a/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts +++ b/acvm-repo/acvm_js/test/shared/multi_scalar_mul.ts @@ -1,8 +1,8 @@ // See `multi_scalar_mul_circuit` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, 227, 70, 74, 11, 86, 194, 195, - 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, - 240, 25, 249, 11, 238, 40, 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0 + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255, 227, 70, 74, 11, 86, 194, + 195, 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176, 144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, + 233, 240, 25, 249, 11, 238, 40, 245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0, ]); export const initialWitnessMap = new Map([ [1, '0x0000000000000000000000000000000000000000000000000000000000000001'], From d6ba7c3f158ce8c25ee391985bde23c6cd77b193 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 5 Sep 2024 15:33:26 -0400 Subject: [PATCH 10/22] using only the value + AcirField for InvalidInputBitSize --- .../opcodes/black_box_function_call.rs | 20 +++++++-------- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 2 +- acvm-repo/acvm/src/pwg/mod.rs | 25 +++++++++---------- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 83dd1bf1278..8165756eb71 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -45,22 +45,20 @@ impl FunctionInput { } #[derive(Clone, PartialEq, Eq, Debug, Error)] -#[error("FunctionInput value has too many bits: value: {value}, {value_num_bits} >= {num_bits}")] -pub struct InvalidInputBitSize { - pub value: String, - pub value_num_bits: u32, - pub num_bits: u32, +#[error("FunctionInput value has too many bits: value: {value}, {} >= {max_bits}", value.num_bits())] +pub struct InvalidInputBitSize { + pub value: F, + pub max_bits: u32, } impl FunctionInput { - pub fn constant(value: F, num_bits: u32) -> Result, InvalidInputBitSize> { - if value.num_bits() <= num_bits { - Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits }) + pub fn constant(value: F, max_bits: u32) -> Result, InvalidInputBitSize> { + if value.num_bits() <= max_bits { + Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits: max_bits }) } else { Err(InvalidInputBitSize { - value: format!("{}", value), - value_num_bits: value.num_bits(), - num_bits, + value, + max_bits, }) } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index 1bce4aa6c5e..30ce224ca63 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -49,7 +49,7 @@ impl AcvmBigIntSolver { Ok(()) } - pub(crate) fn bigint_op( + pub(crate) fn bigint_op( &mut self, lhs: u32, rhs: u32, diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 22a90031a44..f061129d0ff 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -39,7 +39,7 @@ pub use self::brillig::{BrilligSolver, BrilligSolverStatus}; pub use brillig::ForeignCallWaitInfo; #[derive(Debug, Clone, PartialEq)] -pub enum ACVMStatus { +pub enum ACVMStatus { /// All opcodes have been solved. Solved, @@ -64,7 +64,7 @@ pub enum ACVMStatus { RequiresAcirCall(AcirCallWaitInfo), } -impl std::fmt::Display for ACVMStatus { +impl std::fmt::Display for ACVMStatus { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { ACVMStatus::Solved => write!(f, "Solved"), @@ -76,7 +76,7 @@ impl std::fmt::Display for ACVMStatus { } } -pub enum StepResult<'a, F, B: BlackBoxFunctionSolver> { +pub enum StepResult<'a, F: AcirField, B: BlackBoxFunctionSolver> { Status(ACVMStatus), IntoBrillig(BrilligSolver<'a, F, B>), } @@ -120,7 +120,7 @@ impl std::fmt::Display for ErrorLocation { } #[derive(Clone, PartialEq, Eq, Debug, Error)] -pub enum OpcodeResolutionError { +pub enum OpcodeResolutionError { #[error("Cannot solve opcode: {0}")] OpcodeNotSolvable(#[from] OpcodeNotSolvable), #[error("Cannot satisfy constraint")] @@ -133,7 +133,7 @@ pub enum OpcodeResolutionError { #[error("Cannot solve opcode: {invalid_input_bit_size}")] InvalidInputBitSize { opcode_location: ErrorLocation, - invalid_input_bit_size: InvalidInputBitSize, + invalid_input_bit_size: InvalidInputBitSize, }, #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), @@ -149,7 +149,7 @@ pub enum OpcodeResolutionError { AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 }, } -impl From for OpcodeResolutionError { +impl From for OpcodeResolutionError { fn from(value: BlackBoxResolutionError) -> Self { match value { BlackBoxResolutionError::Failed(func, reason) => { @@ -159,8 +159,8 @@ impl From for OpcodeResolutionError { } } -impl From for OpcodeResolutionError { - fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { +impl From> for OpcodeResolutionError { + fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { Self::InvalidInputBitSize { opcode_location: ErrorLocation::Unresolved, invalid_input_bit_size, @@ -168,7 +168,7 @@ impl From for OpcodeResolutionError { } } -pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { +pub struct ACVM<'a, F: AcirField, B: BlackBoxFunctionSolver> { status: ACVMStatus, backend: &'a B, @@ -646,7 +646,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { // Returns the concrete value for a particular witness // If the witness has no assignment, then // an error is returned -pub fn witness_to_value( +pub fn witness_to_value( initial_witness: &WitnessMap, witness: Witness, ) -> Result<&F, OpcodeResolutionError> { @@ -670,9 +670,8 @@ pub fn input_to_value( opcode_location: ErrorLocation::Unresolved, invalid_input_bit_size: InvalidInputBitSize { - value: format!("{}", initial_value), - value_num_bits: initial_value.num_bits(), - num_bits: input.num_bits(), + value: initial_value, + max_bits: input.num_bits(), }, }) } From 3ae49c46abbddc2b90127d4b5c195a4450729602 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 5 Sep 2024 16:30:07 -0400 Subject: [PATCH 11/22] propagate RuntimeError parameter --- compiler/noirc_driver/src/lib.rs | 8 +-- compiler/noirc_evaluator/src/errors.rs | 14 ++-- compiler/noirc_evaluator/src/ssa.rs | 10 +-- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 68 +++++++++---------- .../ssa/acir_gen/acir_ir/generated_acir.rs | 4 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 68 +++++++++---------- .../src/ssa/opt/assert_constant.rs | 9 +-- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 7 +- .../src/ssa/ssa_gen/context.rs | 8 +-- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 48 ++++++------- 10 files changed, 123 insertions(+), 121 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index a315e7ed397..455cb0685a2 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -4,7 +4,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] use abi_gen::{abi_type_from_hir_type, value_from_hir_expression}; -use acvm::acir::circuit::ExpressionWidth; +use acvm::{acir::circuit::ExpressionWidth, FieldElement}; use clap::Args; use fm::{FileId, FileManager}; use iter_extended::vecmap; @@ -146,7 +146,7 @@ pub fn parse_expression_width(input: &str) -> Result), } impl From for CompileError { @@ -155,8 +155,8 @@ impl From for CompileError { } } -impl From for CompileError { - fn from(error: RuntimeError) -> Self { +impl From> for CompileError { + fn from(error: RuntimeError) -> Self { Self::RuntimeError(error) } } diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index e1944b42dbc..daf01d74ea3 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::{acir::InvalidInputBitSize, FieldElement}; +use acvm::{acir::InvalidInputBitSize, AcirField, FieldElement}; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; @@ -16,7 +16,7 @@ use crate::ssa::ir::{dfg::CallStack, types::NumericType}; use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone, Error)] -pub enum RuntimeError { +pub enum RuntimeError { #[error(transparent)] InternalError(#[from] InternalError), #[error("Range constraint of {num_bits} bits is too large for the Field size")] @@ -55,7 +55,7 @@ pub enum RuntimeError { #[error("Could not resolve some references to the array. All references must be resolved at compile time")] UnknownReference { call_stack: CallStack }, #[error("{invalid_input_bit_size}")] - InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, + InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -136,7 +136,7 @@ pub enum InternalError { Unexpected { expected: String, found: String, call_stack: CallStack }, } -impl RuntimeError { +impl RuntimeError { fn call_stack(&self) -> &CallStack { match self { RuntimeError::InternalError( @@ -168,8 +168,8 @@ impl RuntimeError { } } -impl From for FileDiagnostic { - fn from(error: RuntimeError) -> FileDiagnostic { +impl From> for FileDiagnostic { + fn from(error: RuntimeError) -> FileDiagnostic { let call_stack = vecmap(error.call_stack(), |location| *location); let file_id = call_stack.last().map(|location| location.file).unwrap_or_default(); let diagnostic = error.into_diagnostic(); @@ -177,7 +177,7 @@ impl From for FileDiagnostic { } } -impl RuntimeError { +impl RuntimeError { fn into_diagnostic(self) -> Diagnostic { match self { RuntimeError::InternalError(cause) => { diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index ad6645df228..541aca47c10 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -79,7 +79,7 @@ pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec); pub(crate) fn optimize_into_acir( program: Program, options: &SsaEvaluatorOptions, -) -> Result { +) -> Result> { let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); let ssa_gen_span_guard = ssa_gen_span.enter(); @@ -207,7 +207,7 @@ impl SsaProgramArtifact { pub fn create_program( program: Program, options: &SsaEvaluatorOptions, -) -> Result { +) -> Result> { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); let debug_functions = program.debug_functions.clone(); @@ -385,7 +385,7 @@ impl SsaBuilder { force_brillig_runtime: bool, print_codegen_timings: bool, emit_ssa: &Option, - ) -> Result { + ) -> Result> { let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?; if let Some(emit_ssa) = emit_ssa { let mut emit_ssa_dir = emit_ssa.clone(); @@ -412,9 +412,9 @@ impl SsaBuilder { /// The same as `run_pass` but for passes that may fail fn try_run_pass( mut self, - pass: fn(Ssa) -> Result, + pass: fn(Ssa) -> Result>, msg: &str, - ) -> Result { + ) -> Result> { self.ssa = time(msg, self.print_codegen_timings, || pass(self.ssa))?; Ok(self.print(msg)) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 7b1062306c6..48c47b46ff8 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -320,7 +320,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result { + ) -> Result> { let var_data = &self.vars[&var]; if let AcirVarData::Const(constant) = var_data { // Note that this will return a 0 if the inverse is not available @@ -365,7 +365,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let pred_mul_var = self.mul_var(var, predicate)?; self.assert_eq_var(pred_mul_var, predicate, None) } @@ -383,7 +383,7 @@ impl AcirContext { /// Returns an `AcirVar` that is `1` if `lhs` equals `rhs` and /// 0 otherwise. - pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -406,7 +406,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -443,7 +443,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -473,7 +473,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result { + ) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; if lhs_expr.is_zero() { @@ -507,7 +507,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, assert_message: Option>, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -541,7 +541,7 @@ impl AcirContext { pub(crate) fn vars_to_expressions_or_memory( &self, values: &[AcirValue], - ) -> Result>, RuntimeError> { + ) -> Result>, RuntimeError> { let mut result = Vec::with_capacity(values.len()); for value in values { match value { @@ -568,7 +568,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result { + ) -> Result> { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, AcirType::Array(_, _) => { @@ -595,7 +595,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the multiplication of `lhs` and `rhs` - pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { let lhs_data = self.vars[&lhs].clone(); let rhs_data = self.vars[&rhs].clone(); @@ -676,14 +676,14 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the subtraction of `lhs` and `rhs` - pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { let neg_rhs = self.neg_var(rhs); self.add_var(lhs, neg_rhs) } /// Adds a new Variable to context whose value will /// be constrained to be the addition of `lhs` and `rhs` - pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { + pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -746,7 +746,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the expression `lhs + k * rhs` - fn add_mul_var(&mut self, lhs: AcirVar, k: F, rhs: AcirVar) -> Result { + fn add_mul_var(&mut self, lhs: AcirVar, k: F, rhs: AcirVar) -> Result> { let k_var = self.add_constant(k); let intermediate = self.mul_var(k_var, rhs)?; @@ -754,7 +754,7 @@ impl AcirContext { } /// Adds a new variable that is constrained to be the logical NOT of `x`. - pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { + pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result> { let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = self.add_constant((1_u128 << bit_size) - 1); @@ -768,7 +768,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result<(AcirVar, AcirVar), RuntimeError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { let zero = self.add_constant(F::zero()); let one = self.add_constant(F::one()); @@ -947,7 +947,7 @@ impl AcirContext { rhs: AcirVar, offset: AcirVar, bits: u32, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { const fn num_bits() -> usize { std::mem::size_of::() * 8 } @@ -1004,7 +1004,7 @@ impl AcirContext { lhs: AcirVar, leading: AcirVar, max_bit_size: u32, - ) -> Result { + ) -> Result> { let max_power_of_two = self.add_constant(F::from(2_u128).pow(&F::from(max_bit_size as u128 - 1))); @@ -1023,7 +1023,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - ) -> Result<(AcirVar, AcirVar), RuntimeError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { // We derive the signed division from the unsigned euclidean division. // note that this is not euclidean division! // If `x` is a signed integer, then `sign(x)x >= 0` @@ -1080,7 +1080,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result { + ) -> Result> { let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder) } @@ -1091,7 +1091,7 @@ impl AcirContext { variable: AcirVar, numeric_type: &NumericType, message: Option, - ) -> Result { + ) -> Result> { match numeric_type { NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { // If `variable` is constant then we don't need to add a constraint. @@ -1127,7 +1127,7 @@ impl AcirContext { lhs: AcirVar, rhs: u32, max_bit_size: u32, - ) -> Result { + ) -> Result> { // 2^{rhs} let divisor = self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128))); let one = self.add_constant(F::one()); @@ -1149,7 +1149,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_count: u32, - ) -> Result { + ) -> Result> { let pow_last = self.add_constant(F::from(1_u128 << (bit_count - 1))); let pow = self.add_constant(F::from(1_u128 << (bit_count))); @@ -1197,7 +1197,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, max_bits: u32, - ) -> Result { + ) -> Result> { // Returns a `Witness` that is constrained to be: // - `1` if lhs >= rhs // - `0` otherwise @@ -1265,7 +1265,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - ) -> Result { + ) -> Result> { // Flip the result of calling more than equal method to // compute less than. let comparison = self.more_than_eq_var(lhs, rhs, bit_size)?; @@ -1281,7 +1281,7 @@ impl AcirContext { name: BlackBoxFunc, mut inputs: Vec, mut output_count: usize, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // Separate out any arguments that should be constants let (constant_inputs, constant_outputs) = match name { BlackBoxFunc::PedersenCommitment | BlackBoxFunc::PedersenHash => { @@ -1420,9 +1420,9 @@ impl AcirContext { } BlackBoxFunc::AES128Encrypt => { let invalid_input = "aes128_encrypt - operation requires a plaintext to encrypt"; - let input_size = match inputs.first().expect(invalid_input) { - AcirValue::Array(values) => Ok::(values.len()), - AcirValue::DynamicArray(dyn_array) => Ok::(dyn_array.len), + let input_size: usize = match inputs.first().expect(invalid_input) { + AcirValue::Array(values) => Ok::>(values.len()), + AcirValue::DynamicArray(dyn_array) => Ok::>(dyn_array.len), _ => { return Err(RuntimeError::InternalError(InternalError::General { message: "aes128_encrypt requires an array of inputs".to_string(), @@ -1502,7 +1502,7 @@ impl AcirContext { &mut self, inputs: Vec, allow_constant_inputs: bool, - ) -> Result>>, RuntimeError> { + ) -> Result>>, RuntimeError> { let mut witnesses = Vec::new(); for input in inputs { let mut single_val_witnesses = Vec::new(); @@ -1544,7 +1544,7 @@ impl AcirContext { radix_var: AcirVar, limb_count: u32, result_element_type: AcirType, - ) -> Result { + ) -> Result> { let radix = match self.vars[&radix_var].as_constant() { Some(radix) => radix.to_u128() as u32, None => { @@ -1581,7 +1581,7 @@ impl AcirContext { input_var: AcirVar, limb_count: u32, result_element_type: AcirType, - ) -> Result { + ) -> Result> { let two_var = self.add_constant(2_u128); self.radix_decompose(endian, input_var, two_var, limb_count, result_element_type) } @@ -1650,7 +1650,7 @@ impl AcirContext { unsafe_return_values: bool, brillig_function_index: BrilligFunctionId, brillig_stdlib_func: Option, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let predicate = self.var_to_expression(predicate)?; if predicate.is_zero() { // If the predicate has a constant value of zero, the brillig call will never be executed. @@ -1731,7 +1731,7 @@ impl AcirContext { fn range_constraint_value( context: &mut AcirContext, value: &AcirValue, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { match value { AcirValue::Var(var, typ) => { let numeric_type = match typ { @@ -2010,7 +2010,7 @@ impl AcirContext { inputs: Vec, output_count: usize, predicate: AcirVar, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let inputs = self.prepare_inputs_for_black_box_func_call(inputs, false)?; let inputs = inputs .iter() diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 0cad7b9c978..7ea2a857776 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -377,7 +377,7 @@ impl GeneratedAcir { radix: u32, limb_count: u32, bit_size: u32, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let radix_big = BigUint::from(radix); assert_eq!( BigUint::from(2u128).pow(bit_size), @@ -549,7 +549,7 @@ impl GeneratedAcir { &mut self, witness: Witness, num_bits: u32, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // We class this as an error because users should instead // do `as Field`. if num_bits >= F::max_num_bits() { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 15b44fde65d..dda1fc5f6f2 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -292,7 +292,7 @@ impl Ssa { self, brillig: &Brillig, expression_width: ExpressionWidth, - ) -> Result { + ) -> Result> { let mut acirs = Vec::new(); // TODO: can we parallelize this? let mut shared_context = SharedContext::default(); @@ -373,7 +373,7 @@ impl<'a> Context<'a> { ssa: &Ssa, function: &Function, brillig: &Brillig, - ) -> Result>, RuntimeError> { + ) -> Result>, RuntimeError> { match function.runtime() { RuntimeType::Acir(inline_type) => { match inline_type { @@ -405,7 +405,7 @@ impl<'a> Context<'a> { main_func: &Function, ssa: &Ssa, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?; @@ -463,7 +463,7 @@ impl<'a> Context<'a> { mut self, main_func: &Function, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let inputs = try_vecmap(dfg[main_func.entry_block()].parameters(), |param_id| { @@ -521,7 +521,7 @@ impl<'a> Context<'a> { &mut self, params: &[ValueId], dfg: &DataFlowGraph, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // The first witness (if any) is the next one let start_witness = self.acir_context.current_witness_index().0; for param_id in params { @@ -554,15 +554,15 @@ impl<'a> Context<'a> { Ok(witnesses) } - fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result { + fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result> { self.create_value_from_type(param_type, &mut |this, typ| this.add_numeric_input_var(&typ)) } fn create_value_from_type( &mut self, param_type: &Type, - make_var: &mut impl FnMut(&mut Self, NumericType) -> Result, - ) -> Result { + make_var: &mut impl FnMut(&mut Self, NumericType) -> Result>, + ) -> Result> { match param_type { Type::Numeric(numeric_type) => { let typ = AcirType::new(*numeric_type); @@ -618,7 +618,7 @@ impl<'a> Context<'a> { fn add_numeric_input_var( &mut self, numeric_type: &NumericType, - ) -> Result { + ) -> Result> { let acir_var = self.acir_context.add_variable(); if matches!(numeric_type, NumericType::Signed { .. } | NumericType::Unsigned { .. }) { self.acir_context.range_constrain_var(acir_var, numeric_type, None)?; @@ -633,7 +633,7 @@ impl<'a> Context<'a> { dfg: &DataFlowGraph, ssa: &Ssa, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let instruction = &dfg[instruction_id]; self.acir_context.set_call_stack(dfg.get_call_stack(instruction_id)); let mut warnings = Vec::new(); @@ -752,7 +752,7 @@ impl<'a> Context<'a> { ssa: &Ssa, brillig: &Brillig, result_ids: &[ValueId], - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let mut warnings = Vec::new(); match instruction { @@ -894,7 +894,7 @@ impl<'a> Context<'a> { result_ids: &[ValueId], output_values: Vec, dfg: &DataFlowGraph, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { for (result_id, output) in result_ids.iter().zip(output_values) { if let AcirValue::Array(_) = &output { let array_id = dfg.resolve(*result_id); @@ -988,7 +988,7 @@ impl<'a> Context<'a> { &mut self, instruction: InstructionId, dfg: &DataFlowGraph, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let mut mutable_array_set = false; // Pass the instruction between array methods rather than the internal fields themselves @@ -1065,7 +1065,7 @@ impl<'a> Context<'a> { array: ValueId, index: ValueId, store_value: Option, - ) -> Result { + ) -> Result> { let array_id = dfg.resolve(array); let array_typ = dfg.type_of_value(array_id); // Compiler sanity checks @@ -1104,7 +1104,7 @@ impl<'a> Context<'a> { array: Vector, index: FieldElement, store_value: Option, - ) -> Result { + ) -> Result> { let array_size: usize = array.len(); let index = match index.try_to_u64() { Some(index_const) => index_const as usize, @@ -1161,7 +1161,7 @@ impl<'a> Context<'a> { index: ValueId, store_value: Option, offset: usize, - ) -> Result<(AcirVar, Option), RuntimeError> { + ) -> Result<(AcirVar, Option), RuntimeError> { let array_typ = dfg.type_of_value(array_id); let block_id = self.ensure_array_is_initialized(array_id, dfg)?; @@ -1210,7 +1210,7 @@ impl<'a> Context<'a> { &mut self, store_value: &AcirValue, dummy_value: &AcirValue, - ) -> Result { + ) -> Result> { match (store_value, dummy_value) { (AcirValue::Var(store_var, _), AcirValue::Var(dummy_var, _)) => { let true_pred = @@ -1257,7 +1257,7 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*block_id, &index_var)?; - Ok::(AcirValue::Var(read, AcirType::field())) + Ok::>(AcirValue::Var(read, AcirType::field())) })?; let mut elements = im::Vector::new(); @@ -1285,7 +1285,7 @@ impl<'a> Context<'a> { mut var_index: AcirVar, dfg: &DataFlowGraph, mut index_side_effect: bool, - ) -> Result { + ) -> Result> { let block_id = self.ensure_array_is_initialized(array, dfg)?; let results = dfg.instruction_results(instruction); let res_typ = dfg.type_of_value(results[0]); @@ -1348,7 +1348,7 @@ impl<'a> Context<'a> { ssa_type: &Type, block_id: BlockId, var_index: &mut AcirVar, - ) -> Result { + ) -> Result> { let one = self.acir_context.add_constant(FieldElement::one()); match ssa_type.clone() { Type::Numeric(numeric_type) => { @@ -1389,7 +1389,7 @@ impl<'a> Context<'a> { store_value: AcirValue, dfg: &DataFlowGraph, mutate_array: bool, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // Pass the instruction between array methods rather than the internal fields themselves let array = match dfg[instruction] { Instruction::ArraySet { array, .. } => array, @@ -1464,7 +1464,7 @@ impl<'a> Context<'a> { value: &AcirValue, block_id: BlockId, var_index: &mut AcirVar, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let one = self.acir_context.add_constant(FieldElement::one()); match value { AcirValue::Var(store_var, _) => { @@ -1483,7 +1483,7 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?; - Ok::(AcirValue::Var(read, AcirType::field())) + Ok::>(AcirValue::Var(read, AcirType::field())) })?; self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?; } @@ -1495,7 +1495,7 @@ impl<'a> Context<'a> { &mut self, array: ValueId, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result> { // Use the SSA ID to get or create its block ID let block_id = self.block_id(&array); @@ -1534,7 +1534,7 @@ impl<'a> Context<'a> { array_id: ValueId, supplied_acir_value: Option<&AcirValue>, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result> { let element_type_sizes = self.internal_block_id(&array_id); // Check whether an internal type sizes array has already been initialized // Need to look into how to optimize for slices as this could lead to different element type sizes @@ -1650,12 +1650,12 @@ impl<'a> Context<'a> { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(source, &index_var)?; - Ok::(AcirValue::Var(read, AcirType::field())) + Ok::>(AcirValue::Var(read, AcirType::field())) })?; let array: Vector = init_values.into(); self.initialize_array(destination, array_len, Some(AcirValue::Array(array)))?; @@ -1668,7 +1668,7 @@ impl<'a> Context<'a> { array_id: ValueId, var_index: AcirVar, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result> { if !can_omit_element_sizes_array(array_typ) { let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1812,7 +1812,7 @@ impl<'a> Context<'a> { &mut self, terminator: &TerminatorInstruction, dfg: &DataFlowGraph, - ) -> Result<(Vec, Vec), RuntimeError> { + ) -> Result<(Vec, Vec), RuntimeError> { let (return_values, call_stack) = match terminator { TerminatorInstruction::Return { return_values, call_stack } => { (return_values, call_stack.clone()) @@ -1929,7 +1929,7 @@ impl<'a> Context<'a> { &mut self, binary: &Binary, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result> { let lhs = self.convert_numeric_value(binary.lhs, dfg)?; let rhs = self.convert_numeric_value(binary.rhs, dfg)?; @@ -2012,7 +2012,7 @@ impl<'a> Context<'a> { rhs: ValueId, dfg: &DataFlowGraph, op: BinaryOp, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // We try to optimize away operations that are guaranteed not to overflow let max_lhs_bits = dfg.get_value_max_num_bits(lhs); let max_rhs_bits = dfg.get_value_max_num_bits(rhs); @@ -2105,7 +2105,7 @@ impl<'a> Context<'a> { bit_size: u32, max_bit_size: u32, dfg: &DataFlowGraph, - ) -> Result { + ) -> Result> { let mut var = self.convert_numeric_value(value_id, dfg)?; match &dfg[value_id] { Value::Instruction { instruction, .. } => { @@ -2140,7 +2140,7 @@ impl<'a> Context<'a> { arguments: &[ValueId], dfg: &DataFlowGraph, result_ids: &[ValueId], - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { match intrinsic { Intrinsic::BlackBox(black_box) => { // Slices are represented as a tuple of (length, slice contents). @@ -2778,7 +2778,7 @@ impl<'a> Context<'a> { &mut self, old_slice: &mut Vector, input: AcirValue, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { match input { AcirValue::Var(_, _) => { old_slice.push_back(input); diff --git a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs index ae0681a55ff..61f5a0837c4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs @@ -7,6 +7,7 @@ use crate::{ value::ValueId, }, ssa_gen::Ssa, + FieldElement, }, }; @@ -24,7 +25,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn evaluate_static_assert_and_assert_constant( mut self, - ) -> Result { + ) -> Result> { for function in self.functions.values_mut() { for block in function.reachable_blocks() { // Unfortunately we can't just use instructions.retain(...) here since @@ -54,7 +55,7 @@ impl Ssa { fn check_instruction( function: &mut Function, instruction: InstructionId, -) -> Result { +) -> Result> { let assert_constant_id = function.dfg.import_intrinsic(Intrinsic::AssertConstant); let static_assert_id = function.dfg.import_intrinsic(Intrinsic::StaticAssert); match &function.dfg[instruction] { @@ -79,7 +80,7 @@ fn evaluate_assert_constant( function: &Function, instruction: InstructionId, arguments: &[ValueId], -) -> Result { +) -> Result> { if arguments.iter().all(|arg| function.dfg.is_constant(*arg)) { Ok(false) } else { @@ -98,7 +99,7 @@ fn evaluate_static_assert( function: &Function, instruction: InstructionId, arguments: &[ValueId], -) -> Result { +) -> Result> { if arguments.len() != 2 { panic!("ICE: static_assert called with wrong number of arguments") } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index f3e11e04e3a..fd1fc4b6015 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -33,6 +33,7 @@ use crate::{ value::ValueId, }, ssa_gen::Ssa, + FieldElement, }, }; use fxhash::FxHashMap as HashMap; @@ -40,7 +41,7 @@ use fxhash::FxHashMap as HashMap; impl Ssa { /// Loop unrolling can return errors, since ACIR functions need to be fully unrolled. /// This meta-pass will keep trying to unroll loops and simplifying the SSA until no more errors are found. - pub(crate) fn unroll_loops_iteratively(mut ssa: Ssa) -> Result { + pub(crate) fn unroll_loops_iteratively(mut ssa: Ssa) -> Result> { // Try to unroll loops first: let mut unroll_errors; (ssa, unroll_errors) = ssa.try_to_unroll_loops(); @@ -71,7 +72,7 @@ impl Ssa { /// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state. /// Returns the ssa along with all unrolling errors encountered #[tracing::instrument(level = "trace", skip(self))] - pub(crate) fn try_to_unroll_loops(mut self) -> (Ssa, Vec) { + pub(crate) fn try_to_unroll_loops(mut self) -> (Ssa, Vec>) { let mut errors = vec![]; for function in self.functions.values_mut() { // Loop unrolling in brillig can lead to a code explosion currently. This can @@ -147,7 +148,7 @@ fn find_all_loops(function: &Function) -> Loops { impl Loops { /// Unroll all loops within a given function. /// Any loops which fail to be unrolled (due to using non-constant indices) will be unmodified. - fn unroll_each_loop(mut self, function: &mut Function) -> Vec { + fn unroll_each_loop(mut self, function: &mut Function) -> Vec> { let mut unroll_errors = vec![]; while let Some(next_loop) = self.yet_to_unroll.pop() { // If we've previously modified a block in this loop we need to refresh the context. diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index fb7091a8854..e4839d3eda4 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -272,7 +272,7 @@ impl<'a> FunctionContext<'a> { value: impl Into, negative: bool, typ: Type, - ) -> Result { + ) -> Result> { let value = value.into(); if let Type::Numeric(numeric_type) = typ { @@ -703,7 +703,7 @@ impl<'a> FunctionContext<'a> { pub(super) fn extract_current_value( &mut self, lvalue: &ast::LValue, - ) -> Result { + ) -> Result> { Ok(match lvalue { ast::LValue::Ident(ident) => { let (reference, should_auto_deref) = self.ident_lvalue(ident); @@ -755,7 +755,7 @@ impl<'a> FunctionContext<'a> { array: &ast::LValue, index: &ast::Expression, location: &Location, - ) -> Result<(ValueId, ValueId, LValue, Option), RuntimeError> { + ) -> Result<(ValueId, ValueId, LValue, Option), RuntimeError> { let (old_array, array_lvalue) = self.extract_current_value_recursive(array)?; let index = self.codegen_non_tuple_expression(index)?; let array_lvalue = Box::new(array_lvalue); @@ -782,7 +782,7 @@ impl<'a> FunctionContext<'a> { fn extract_current_value_recursive( &mut self, lvalue: &ast::LValue, - ) -> Result<(Values, LValue), RuntimeError> { + ) -> Result<(Values, LValue), RuntimeError> { match lvalue { ast::LValue::Ident(ident) => { let (variable, should_auto_deref) = self.ident_lvalue(ident); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 2318fea8960..57280c7222c 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -13,7 +13,7 @@ use noirc_frontend::monomorphization::ast::{self, Expression, Program}; use crate::{ errors::RuntimeError, - ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic}, + ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic, FieldElement}, }; use self::{ @@ -40,7 +40,7 @@ pub(crate) const SSA_WORD_SIZE: u32 = 32; pub(crate) fn generate_ssa( program: Program, force_brillig_runtime: bool, -) -> Result { +) -> Result> { // see which parameter has call_data/return_data attribute let is_databus = DataBusBuilder::is_databus(&program.main_function_signature); @@ -124,7 +124,7 @@ pub(crate) fn generate_ssa( impl<'a> FunctionContext<'a> { /// Codegen a function's body and set its return value to that of its last parameter. /// For functions returning nothing, this will be an empty list. - fn codegen_function_body(&mut self, body: &Expression) -> Result<(), RuntimeError> { + fn codegen_function_body(&mut self, body: &Expression) -> Result<(), RuntimeError> { let entry_block = self.increment_parameter_rcs(); let return_value = self.codegen_expression(body)?; let results = return_value.into_value_list(self); @@ -134,7 +134,7 @@ impl<'a> FunctionContext<'a> { Ok(()) } - fn codegen_expression(&mut self, expr: &Expression) -> Result { + fn codegen_expression(&mut self, expr: &Expression) -> Result> { match expr { Expression::Ident(ident) => Ok(self.codegen_ident(ident)), Expression::Literal(literal) => self.codegen_literal(literal), @@ -163,7 +163,7 @@ impl<'a> FunctionContext<'a> { /// Codegen any non-tuple expression so that we can unwrap the Values /// tree to return a single value for use with most SSA instructions. - fn codegen_non_tuple_expression(&mut self, expr: &Expression) -> Result { + fn codegen_non_tuple_expression(&mut self, expr: &Expression) -> Result> { Ok(self.codegen_expression(expr)?.into_leaf().eval(self)) } @@ -192,7 +192,7 @@ impl<'a> FunctionContext<'a> { self.codegen_ident_reference(ident).map(|value| value.eval(self).into()) } - fn codegen_literal(&mut self, literal: &ast::Literal) -> Result { + fn codegen_literal(&mut self, literal: &ast::Literal) -> Result> { match literal { ast::Literal::Array(array) => { let elements = @@ -258,7 +258,7 @@ impl<'a> FunctionContext<'a> { &mut self, elements: Vec, typ: Type, - ) -> Result { + ) -> Result> { if typ.is_nested_slice() { return Err(RuntimeError::NestedSlice { call_stack: self.builder.get_call_stack() }); } @@ -294,7 +294,7 @@ impl<'a> FunctionContext<'a> { self.builder.array_constant(array, typ).into() } - fn codegen_block(&mut self, block: &[Expression]) -> Result { + fn codegen_block(&mut self, block: &[Expression]) -> Result> { let mut result = Self::unit_value(); for expr in block { result = self.codegen_expression(expr)?; @@ -302,7 +302,7 @@ impl<'a> FunctionContext<'a> { Ok(result) } - fn codegen_unary(&mut self, unary: &ast::Unary) -> Result { + fn codegen_unary(&mut self, unary: &ast::Unary) -> Result> { match unary.operator { UnaryOp::Not => { let rhs = self.codegen_expression(&unary.rhs)?; @@ -351,7 +351,7 @@ impl<'a> FunctionContext<'a> { }) } - fn codegen_reference(&mut self, expr: &Expression) -> Result { + fn codegen_reference(&mut self, expr: &Expression) -> Result> { match expr { Expression::Ident(ident) => Ok(self.codegen_ident_reference(ident)), Expression::ExtractTupleField(tuple, index) => { @@ -362,13 +362,13 @@ impl<'a> FunctionContext<'a> { } } - fn codegen_binary(&mut self, binary: &ast::Binary) -> Result { + fn codegen_binary(&mut self, binary: &ast::Binary) -> Result> { let lhs = self.codegen_non_tuple_expression(&binary.lhs)?; let rhs = self.codegen_non_tuple_expression(&binary.rhs)?; Ok(self.insert_binary(lhs, binary.operator, rhs, binary.location)) } - fn codegen_index(&mut self, index: &ast::Index) -> Result { + fn codegen_index(&mut self, index: &ast::Index) -> Result> { let array_or_slice = self.codegen_expression(&index.collection)?.into_value_list(self); let index_value = self.codegen_non_tuple_expression(&index.index)?; // Slices are represented as a tuple in the form: (length, slice contents). @@ -401,7 +401,7 @@ impl<'a> FunctionContext<'a> { element_type: &ast::Type, location: Location, length: Option, - ) -> Result { + ) -> Result> { // base_index = index * type_size let index = self.make_array_index(index); let type_size = Self::convert_type(element_type).size_of_type(); @@ -454,7 +454,7 @@ impl<'a> FunctionContext<'a> { ); } - fn codegen_cast(&mut self, cast: &ast::Cast) -> Result { + fn codegen_cast(&mut self, cast: &ast::Cast) -> Result> { let lhs = self.codegen_non_tuple_expression(&cast.lhs)?; let typ = Self::convert_non_tuple_type(&cast.r#type); @@ -478,7 +478,7 @@ impl<'a> FunctionContext<'a> { /// br loop_entry(v4) /// loop_end(): /// ... This is the current insert point after codegen_for finishes ... - fn codegen_for(&mut self, for_expr: &ast::For) -> Result { + fn codegen_for(&mut self, for_expr: &ast::For) -> Result> { let loop_entry = self.builder.insert_block(); let loop_body = self.builder.insert_block(); let loop_end = self.builder.insert_block(); @@ -549,7 +549,7 @@ impl<'a> FunctionContext<'a> { /// br end_if() /// end_if: // No block parameter is needed. Without an else, the unit value is always returned. /// ... This is the current insert point after codegen_if finishes ... - fn codegen_if(&mut self, if_expr: &ast::If) -> Result { + fn codegen_if(&mut self, if_expr: &ast::If) -> Result> { let condition = self.codegen_non_tuple_expression(&if_expr.condition)?; let then_block = self.builder.insert_block(); @@ -589,7 +589,7 @@ impl<'a> FunctionContext<'a> { Ok(result) } - fn codegen_tuple(&mut self, tuple: &[Expression]) -> Result { + fn codegen_tuple(&mut self, tuple: &[Expression]) -> Result> { Ok(Tree::Branch(try_vecmap(tuple, |expr| self.codegen_expression(expr))?)) } @@ -597,14 +597,14 @@ impl<'a> FunctionContext<'a> { &mut self, tuple: &Expression, field_index: usize, - ) -> Result { + ) -> Result> { let tuple = self.codegen_expression(tuple)?; Ok(Self::get_field(tuple, field_index)) } /// Generate SSA for a function call. Note that calls to built-in functions /// and intrinsics are also represented by the function call instruction. - fn codegen_call(&mut self, call: &ast::Call) -> Result { + fn codegen_call(&mut self, call: &ast::Call) -> Result> { let function = self.codegen_non_tuple_expression(&call.func)?; let mut arguments = Vec::with_capacity(call.arguments.len()); @@ -653,7 +653,7 @@ impl<'a> FunctionContext<'a> { /// If the variable is immutable, no special handling is necessary and we can return the given /// ValueId directly. If it is mutable, we'll need to allocate space for the value and store /// the initial value before returning the allocate instruction. - fn codegen_let(&mut self, let_expr: &ast::Let) -> Result { + fn codegen_let(&mut self, let_expr: &ast::Let) -> Result> { let mut values = self.codegen_expression(&let_expr.expression)?; values = values.map(|value| { @@ -678,7 +678,7 @@ impl<'a> FunctionContext<'a> { expr: &Expression, location: Location, assert_payload: &Option>, - ) -> Result { + ) -> Result> { let expr = self.codegen_non_tuple_expression(expr)?; let true_literal = self.builder.numeric_constant(true, Type::bool()); @@ -699,7 +699,7 @@ impl<'a> FunctionContext<'a> { fn codegen_constrain_error( &mut self, assert_message: &Option>, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let Some(assert_message_payload) = assert_message else { return Ok(None) }; if let Expression::Literal(ast::Literal::Str(static_string)) = &assert_message_payload.0 { @@ -721,7 +721,7 @@ impl<'a> FunctionContext<'a> { Ok(Some(ConstrainError::Dynamic(error_type_id, values))) } - fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { + fn codegen_assign(&mut self, assign: &ast::Assign) -> Result> { let lhs = self.extract_current_value(&assign.lvalue)?; let rhs = self.codegen_expression(&assign.expression)?; @@ -734,7 +734,7 @@ impl<'a> FunctionContext<'a> { Ok(Self::unit_value()) } - fn codegen_semi(&mut self, expr: &Expression) -> Result { + fn codegen_semi(&mut self, expr: &Expression) -> Result> { self.codegen_expression(expr)?; Ok(Self::unit_value()) } From 9dea841e7da8d83c5c6c7529050b7d2e488904a8 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Thu, 5 Sep 2024 18:37:31 -0400 Subject: [PATCH 12/22] responding to review comments --- .../opcodes/black_box_function_call.rs | 5 +-- acvm-repo/acvm/tests/solver.rs | 6 ++- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 41 ++++++++++++++--- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 15 +++++-- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 4 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 45 +++++++++++++++---- 6 files changed, 90 insertions(+), 26 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index 8165756eb71..dc70dc8f1e9 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -56,10 +56,7 @@ impl FunctionInput { if value.num_bits() <= max_bits { Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits: max_bits }) } else { - Err(InvalidInputBitSize { - value, - max_bits, - }) + Err(InvalidInputBitSize { value, max_bits }) } } } diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 012b4b0fe6d..c8e1bf9de2c 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -1317,8 +1317,10 @@ where let equal_inputs = drop_use_constant_eq(&inputs, &distinct_inputs); let message = format!("not injective:\n{:?}\n{:?}", &inputs, &distinct_inputs); let outputs_not_equal = - solve_array_input_blackbox_call(inputs, num_outputs, num_bits, op.clone()).unwrap() - != solve_array_input_blackbox_call(distinct_inputs, num_outputs, num_bits, op).unwrap(); + solve_array_input_blackbox_call(inputs, num_outputs, num_bits, op.clone()) + .expect("injectivity test operations to have valid input") + != solve_array_input_blackbox_call(distinct_inputs, num_outputs, num_bits, op) + .expect("injectivity test operations to have valid input"); (equal_inputs || outputs_not_equal, message) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 48c47b46ff8..410aedc0c26 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -383,7 +383,11 @@ impl AcirContext { /// Returns an `AcirVar` that is `1` if `lhs` equals `rhs` and /// 0 otherwise. - pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { + pub(crate) fn eq_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + ) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -595,7 +599,11 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the multiplication of `lhs` and `rhs` - pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { + pub(crate) fn mul_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + ) -> Result> { let lhs_data = self.vars[&lhs].clone(); let rhs_data = self.vars[&rhs].clone(); @@ -676,14 +684,22 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the subtraction of `lhs` and `rhs` - pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { + pub(crate) fn sub_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + ) -> Result> { let neg_rhs = self.neg_var(rhs); self.add_var(lhs, neg_rhs) } /// Adds a new Variable to context whose value will /// be constrained to be the addition of `lhs` and `rhs` - pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result> { + pub(crate) fn add_var( + &mut self, + lhs: AcirVar, + rhs: AcirVar, + ) -> Result> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -746,7 +762,12 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the expression `lhs + k * rhs` - fn add_mul_var(&mut self, lhs: AcirVar, k: F, rhs: AcirVar) -> Result> { + fn add_mul_var( + &mut self, + lhs: AcirVar, + k: F, + rhs: AcirVar, + ) -> Result> { let k_var = self.add_constant(k); let intermediate = self.mul_var(k_var, rhs)?; @@ -754,7 +775,11 @@ impl AcirContext { } /// Adds a new variable that is constrained to be the logical NOT of `x`. - pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result> { + pub(crate) fn not_var( + &mut self, + x: AcirVar, + typ: AcirType, + ) -> Result> { let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = self.add_constant((1_u128 << bit_size) - 1); @@ -1422,7 +1447,9 @@ impl AcirContext { let invalid_input = "aes128_encrypt - operation requires a plaintext to encrypt"; let input_size: usize = match inputs.first().expect(invalid_input) { AcirValue::Array(values) => Ok::>(values.len()), - AcirValue::DynamicArray(dyn_array) => Ok::>(dyn_array.len), + AcirValue::DynamicArray(dyn_array) => { + Ok::>(dyn_array.len) + } _ => { return Err(RuntimeError::InternalError(InternalError::General { message: "aes128_encrypt requires an array of inputs".to_string(), diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index dda1fc5f6f2..5f7b99e0cf2 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -554,7 +554,10 @@ impl<'a> Context<'a> { Ok(witnesses) } - fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result> { + fn convert_ssa_block_param( + &mut self, + param_type: &Type, + ) -> Result> { self.create_value_from_type(param_type, &mut |this, typ| this.add_numeric_input_var(&typ)) } @@ -1257,7 +1260,10 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*block_id, &index_var)?; - Ok::>(AcirValue::Var(read, AcirType::field())) + Ok::>(AcirValue::Var( + read, + AcirType::field(), + )) })?; let mut elements = im::Vector::new(); @@ -1483,7 +1489,10 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?; - Ok::>(AcirValue::Var(read, AcirType::field())) + Ok::>(AcirValue::Var( + read, + AcirType::field(), + )) })?; self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?; } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index fd1fc4b6015..b589bcca9b1 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -41,7 +41,9 @@ use fxhash::FxHashMap as HashMap; impl Ssa { /// Loop unrolling can return errors, since ACIR functions need to be fully unrolled. /// This meta-pass will keep trying to unroll loops and simplifying the SSA until no more errors are found. - pub(crate) fn unroll_loops_iteratively(mut ssa: Ssa) -> Result> { + pub(crate) fn unroll_loops_iteratively( + mut ssa: Ssa, + ) -> Result> { // Try to unroll loops first: let mut unroll_errors; (ssa, unroll_errors) = ssa.try_to_unroll_loops(); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 57280c7222c..4a7f2768361 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -124,7 +124,10 @@ pub(crate) fn generate_ssa( impl<'a> FunctionContext<'a> { /// Codegen a function's body and set its return value to that of its last parameter. /// For functions returning nothing, this will be an empty list. - fn codegen_function_body(&mut self, body: &Expression) -> Result<(), RuntimeError> { + fn codegen_function_body( + &mut self, + body: &Expression, + ) -> Result<(), RuntimeError> { let entry_block = self.increment_parameter_rcs(); let return_value = self.codegen_expression(body)?; let results = return_value.into_value_list(self); @@ -134,7 +137,10 @@ impl<'a> FunctionContext<'a> { Ok(()) } - fn codegen_expression(&mut self, expr: &Expression) -> Result> { + fn codegen_expression( + &mut self, + expr: &Expression, + ) -> Result> { match expr { Expression::Ident(ident) => Ok(self.codegen_ident(ident)), Expression::Literal(literal) => self.codegen_literal(literal), @@ -163,7 +169,10 @@ impl<'a> FunctionContext<'a> { /// Codegen any non-tuple expression so that we can unwrap the Values /// tree to return a single value for use with most SSA instructions. - fn codegen_non_tuple_expression(&mut self, expr: &Expression) -> Result> { + fn codegen_non_tuple_expression( + &mut self, + expr: &Expression, + ) -> Result> { Ok(self.codegen_expression(expr)?.into_leaf().eval(self)) } @@ -192,7 +201,10 @@ impl<'a> FunctionContext<'a> { self.codegen_ident_reference(ident).map(|value| value.eval(self).into()) } - fn codegen_literal(&mut self, literal: &ast::Literal) -> Result> { + fn codegen_literal( + &mut self, + literal: &ast::Literal, + ) -> Result> { match literal { ast::Literal::Array(array) => { let elements = @@ -294,7 +306,10 @@ impl<'a> FunctionContext<'a> { self.builder.array_constant(array, typ).into() } - fn codegen_block(&mut self, block: &[Expression]) -> Result> { + fn codegen_block( + &mut self, + block: &[Expression], + ) -> Result> { let mut result = Self::unit_value(); for expr in block { result = self.codegen_expression(expr)?; @@ -351,7 +366,10 @@ impl<'a> FunctionContext<'a> { }) } - fn codegen_reference(&mut self, expr: &Expression) -> Result> { + fn codegen_reference( + &mut self, + expr: &Expression, + ) -> Result> { match expr { Expression::Ident(ident) => Ok(self.codegen_ident_reference(ident)), Expression::ExtractTupleField(tuple, index) => { @@ -362,7 +380,10 @@ impl<'a> FunctionContext<'a> { } } - fn codegen_binary(&mut self, binary: &ast::Binary) -> Result> { + fn codegen_binary( + &mut self, + binary: &ast::Binary, + ) -> Result> { let lhs = self.codegen_non_tuple_expression(&binary.lhs)?; let rhs = self.codegen_non_tuple_expression(&binary.rhs)?; Ok(self.insert_binary(lhs, binary.operator, rhs, binary.location)) @@ -589,7 +610,10 @@ impl<'a> FunctionContext<'a> { Ok(result) } - fn codegen_tuple(&mut self, tuple: &[Expression]) -> Result> { + fn codegen_tuple( + &mut self, + tuple: &[Expression], + ) -> Result> { Ok(Tree::Branch(try_vecmap(tuple, |expr| self.codegen_expression(expr))?)) } @@ -721,7 +745,10 @@ impl<'a> FunctionContext<'a> { Ok(Some(ConstrainError::Dynamic(error_type_id, values))) } - fn codegen_assign(&mut self, assign: &ast::Assign) -> Result> { + fn codegen_assign( + &mut self, + assign: &ast::Assign, + ) -> Result> { let lhs = self.extract_current_value(&assign.lvalue)?; let rhs = self.codegen_expression(&assign.expression)?; From 59a0e1349d9fbb9504cad81a7e418ea3e18d4e37 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Fri, 6 Sep 2024 13:05:52 -0400 Subject: [PATCH 13/22] refactoring matches and updating test for overflow --- .../compiler/optimizers/redundant_range.rs | 34 +++++++++++-------- .../regression_4080/src/main.nr | 3 +- tooling/fuzzer/src/dictionary/mod.rs | 25 +++++++------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs index e01908d3222..3570a36a7e7 100644 --- a/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs +++ b/acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs @@ -109,22 +109,28 @@ impl RangeOptimizer { let mut new_order_list = Vec::with_capacity(order_list.len()); let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len()); for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() { - let (witness, num_bits) = match opcode { - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) - if matches!(input.input_ref(), ConstantOrWitnessEnum::Witness(..)) => - { - if let ConstantOrWitnessEnum::Witness(witness) = input.input() { - (witness, input.num_bits()) - } else { - unreachable!("The matches! above ensures only a witness is possible") - } - } - _ => { - // If its not the range opcode, add it to the opcode - // list and continue; + let (witness, num_bits) = { + // If its not the range opcode, add it to the opcode + // list and continue; + let mut push_non_range_opcode = || { optimized_opcodes.push(opcode.clone()); new_order_list.push(order_list[idx]); - continue; + }; + + match opcode { + Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => { + match input.input() { + ConstantOrWitnessEnum::Witness(witness) => (witness, input.num_bits()), + _ => { + push_non_range_opcode(); + continue; + } + } + } + _ => { + push_non_range_opcode(); + continue; + } } }; // If we've already applied the range constraint for this witness then skip this opcode. diff --git a/test_programs/noir_test_success/regression_4080/src/main.nr b/test_programs/noir_test_success/regression_4080/src/main.nr index 71231fb8ef2..781d3e33ea3 100644 --- a/test_programs/noir_test_success/regression_4080/src/main.nr +++ b/test_programs/noir_test_success/regression_4080/src/main.nr @@ -1,8 +1,7 @@ // This test checks that `var^var` is assigned the correct type. // https://github.com/noir-lang/noir/issues/4080 -// NOTE: expected error is the following, but the 'failed_assertion' is None in check_expected_failure_message -#[test(should_fail_with = "Cannot solve opcode: FunctionInput value has too many bits")] +#[test(should_fail_with = "attempt to add with overflow")] fn main() { let var1: u8 = ((255 + 1) ^ (255 + 1)) - ((255 + 1) - (255 + 1)); assert_eq(var1, 0); diff --git a/tooling/fuzzer/src/dictionary/mod.rs b/tooling/fuzzer/src/dictionary/mod.rs index 6bc15d7c5fe..e10da8cc54a 100644 --- a/tooling/fuzzer/src/dictionary/mod.rs +++ b/tooling/fuzzer/src/dictionary/mod.rs @@ -86,20 +86,21 @@ fn build_dictionary_from_circuit(circuit: &Circuit) -> HashSet< Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) if matches!(input.input(), ConstantOrWitnessEnum::Constant(..)) => { - if let ConstantOrWitnessEnum::Constant(c) = input.input() { - let field = 1u128.wrapping_shl(input.num_bits()); - constants.insert(F::from(field)); - constants.insert(F::from(field - 1)); - constants.insert(c); - } else { - unreachable!("matches! above ensures the other case is matched") + match input.input() { + ConstantOrWitnessEnum::Constant(c) => { + let field = 1u128.wrapping_shl(input.num_bits()); + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + constants.insert(c); + } + _ => { + let field = 1u128.wrapping_shl(input.num_bits()); + constants.insert(F::from(field)); + constants.insert(F::from(field - 1)); + } } } - Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => { - let field = 1u128.wrapping_shl(input.num_bits()); - constants.insert(F::from(field)); - constants.insert(F::from(field - 1)); - } + _ => (), } } From 92f936a491aef53d90b8ac03b5bdf6a93d7f372e Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 9 Sep 2024 15:05:51 -0400 Subject: [PATCH 14/22] removed parameter from invalid bit size error, disable ACVM bit size checks for bitwise and range ops --- .../opcodes/black_box_function_call.rs | 13 ++- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 2 +- .../src/pwg/blackbox/embedded_curve_ops.rs | 16 ++-- acvm-repo/acvm/src/pwg/blackbox/hash.rs | 9 +- acvm-repo/acvm/src/pwg/blackbox/logic.rs | 7 +- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 2 +- acvm-repo/acvm/src/pwg/blackbox/pedersen.rs | 4 +- acvm-repo/acvm/src/pwg/blackbox/range.rs | 5 +- .../src/pwg/blackbox/signature/schnorr.rs | 4 +- acvm-repo/acvm/src/pwg/blackbox/utils.rs | 4 +- acvm-repo/acvm/src/pwg/mod.rs | 17 ++-- compiler/noirc_driver/src/lib.rs | 8 +- compiler/noirc_evaluator/src/errors.rs | 14 +-- compiler/noirc_evaluator/src/ssa.rs | 10 +- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 93 +++++++------------ .../ssa/acir_gen/acir_ir/generated_acir.rs | 4 +- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 77 +++++++-------- .../src/ssa/opt/assert_constant.rs | 9 +- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 9 +- .../src/ssa/ssa_gen/context.rs | 8 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 75 +++++---------- 21 files changed, 169 insertions(+), 221 deletions(-) diff --git a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs index dc70dc8f1e9..f527522cceb 100644 --- a/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs +++ b/acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs @@ -45,18 +45,21 @@ impl FunctionInput { } #[derive(Clone, PartialEq, Eq, Debug, Error)] -#[error("FunctionInput value has too many bits: value: {value}, {} >= {max_bits}", value.num_bits())] -pub struct InvalidInputBitSize { - pub value: F, +#[error("FunctionInput value has too many bits: value: {value}, {value_num_bits} >= {max_bits}")] +pub struct InvalidInputBitSize { + pub value: String, + pub value_num_bits: u32, pub max_bits: u32, } impl FunctionInput { - pub fn constant(value: F, max_bits: u32) -> Result, InvalidInputBitSize> { + pub fn constant(value: F, max_bits: u32) -> Result, InvalidInputBitSize> { if value.num_bits() <= max_bits { Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits: max_bits }) } else { - Err(InvalidInputBitSize { value, max_bits }) + let value_num_bits = value.num_bits(); + let value = format!("{}", value); + Err(InvalidInputBitSize { value, value_num_bits, max_bits }) } } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index 30ce224ca63..a0a08b57048 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -27,7 +27,7 @@ impl AcvmBigIntSolver { ) -> Result<(), OpcodeResolutionError> { let bytes = inputs .iter() - .map(|input| input_to_value(initial_witness, *input).unwrap().to_u128() as u8) + .map(|input| input_to_value(initial_witness, *input, false).unwrap().to_u128() as u8) .collect::>(); self.bigint_solver.bigint_from_bytes(&bytes, modulus, output)?; Ok(()) diff --git a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs index c290faeaa4a..9e511571275 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs @@ -15,11 +15,11 @@ pub(super) fn multi_scalar_mul( outputs: (Witness, Witness, Witness), ) -> Result<(), OpcodeResolutionError> { let points: Result, _> = - points.iter().map(|input| input_to_value(initial_witness, *input)).collect(); + points.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); let points: Vec<_> = points?.into_iter().collect(); let scalars: Result, _> = - scalars.iter().map(|input| input_to_value(initial_witness, *input)).collect(); + scalars.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); let mut scalars_lo = Vec::new(); let mut scalars_hi = Vec::new(); for (i, scalar) in scalars?.into_iter().enumerate() { @@ -47,12 +47,12 @@ pub(super) fn embedded_curve_add( input2: [FunctionInput; 3], outputs: (Witness, Witness, Witness), ) -> Result<(), OpcodeResolutionError> { - let input1_x = input_to_value(initial_witness, input1[0])?; - let input1_y = input_to_value(initial_witness, input1[1])?; - let input1_infinite = input_to_value(initial_witness, input1[2])?; - let input2_x = input_to_value(initial_witness, input2[0])?; - let input2_y = input_to_value(initial_witness, input2[1])?; - let input2_infinite = input_to_value(initial_witness, input2[2])?; + let input1_x = input_to_value(initial_witness, input1[0], false)?; + let input1_y = input_to_value(initial_witness, input1[1], false)?; + let input1_infinite = input_to_value(initial_witness, input1[2], false)?; + let input2_x = input_to_value(initial_witness, input2[0], false)?; + let input2_y = input_to_value(initial_witness, input2[1], false)?; + let input2_infinite = input_to_value(initial_witness, input2[2], false)?; let (res_x, res_y, res_infinite) = backend.ec_add( &input1_x, &input1_y, diff --git a/acvm-repo/acvm/src/pwg/blackbox/hash.rs b/acvm-repo/acvm/src/pwg/blackbox/hash.rs index b51139f76b7..234ab6162ca 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/hash.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/hash.rs @@ -34,7 +34,7 @@ fn get_hash_input( for input in inputs.iter() { let num_bits = input.num_bits() as usize; - let witness_assignment = input_to_value(initial_witness, *input)?; + let witness_assignment = input_to_value(initial_witness, *input, false)?; let bytes = witness_assignment.fetch_nearest_bytes(num_bits); message_input.extend(bytes); } @@ -42,7 +42,8 @@ fn get_hash_input( // Truncate the message if there is a `message_size` parameter given match message_size { Some(input) => { - let num_bytes_to_take = input_to_value(initial_witness, *input)?.to_u128() as usize; + let num_bytes_to_take = + input_to_value(initial_witness, *input, false)?.to_u128() as usize; // If the number of bytes to take is more than the amount of bytes available // in the message, then we error. @@ -78,7 +79,7 @@ fn to_u32_array( ) -> Result<[u32; N], OpcodeResolutionError> { let mut result = [0; N]; for (it, input) in result.iter_mut().zip(inputs) { - let witness_value = input_to_value(initial_witness, *input)?; + let witness_value = input_to_value(initial_witness, *input, false)?; *it = witness_value.to_u128() as u32; } Ok(result) @@ -133,7 +134,7 @@ pub(crate) fn solve_poseidon2_permutation_opcode( // Read witness assignments let mut state = Vec::new(); for input in inputs.iter() { - let witness_assignment = input_to_value(initial_witness, *input)?; + let witness_assignment = input_to_value(initial_witness, *input, false)?; state.push(witness_assignment); } diff --git a/acvm-repo/acvm/src/pwg/blackbox/logic.rs b/acvm-repo/acvm/src/pwg/blackbox/logic.rs index 7ce0827d932..8468b0ca27a 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/logic.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/logic.rs @@ -51,8 +51,11 @@ fn solve_logic_opcode( result: Witness, logic_op: impl Fn(F, F) -> F, ) -> Result<(), OpcodeResolutionError> { - let w_l_value = input_to_value(initial_witness, *a)?; - let w_r_value = input_to_value(initial_witness, *b)?; + // TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these once we figure out how to combine these with existing + // noirc_frontend/noirc_evaluator overflow error messages + let skip_bitsize_checks = true; + let w_l_value = input_to_value(initial_witness, *a, skip_bitsize_checks)?; + let w_r_value = input_to_value(initial_witness, *b, skip_bitsize_checks)?; let assignment = logic_op(w_l_value, w_r_value); insert_value(&result, assignment, initial_witness) diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index 88227f18449..8b8bfc5cfc5 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -108,7 +108,7 @@ pub(crate) fn solve( for (it, input) in state.iter_mut().zip(inputs.as_ref()) { let num_bits = input.num_bits() as usize; assert_eq!(num_bits, 64); - let witness_assignment = input_to_value(initial_witness, *input)?; + let witness_assignment = input_to_value(initial_witness, *input, false)?; let lane = witness_assignment.try_to_u64(); *it = lane.unwrap(); } diff --git a/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs b/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs index b1b95393b19..654814bf92d 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/pedersen.rs @@ -17,7 +17,7 @@ pub(super) fn pedersen( outputs: (Witness, Witness), ) -> Result<(), OpcodeResolutionError> { let scalars: Result, _> = - inputs.iter().map(|input| input_to_value(initial_witness, *input)).collect(); + inputs.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); let scalars: Vec<_> = scalars?.into_iter().collect(); let (res_x, res_y) = backend.pedersen_commitment(&scalars, domain_separator)?; @@ -36,7 +36,7 @@ pub(super) fn pedersen_hash( output: Witness, ) -> Result<(), OpcodeResolutionError> { let scalars: Result, _> = - inputs.iter().map(|input| input_to_value(initial_witness, *input)).collect(); + inputs.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); let scalars: Vec<_> = scalars?.into_iter().collect(); let res = backend.pedersen_hash(&scalars, domain_separator)?; diff --git a/acvm-repo/acvm/src/pwg/blackbox/range.rs b/acvm-repo/acvm/src/pwg/blackbox/range.rs index 054730bb6c0..4f9be14360e 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/range.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/range.rs @@ -8,7 +8,10 @@ pub(crate) fn solve_range_opcode( initial_witness: &WitnessMap, input: &FunctionInput, ) -> Result<(), OpcodeResolutionError> { - let w_value = input_to_value(initial_witness, *input)?; + // TODO(https://github.com/noir-lang/noir/issues/5985): + // re-enable bitsize checks + let skip_bitsize_checks = true; + let w_value = input_to_value(initial_witness, *input, skip_bitsize_checks)?; if w_value.num_bits() > input.num_bits() { return Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, diff --git a/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs b/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs index 4f8e88373ba..a856303d065 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/signature/schnorr.rs @@ -21,8 +21,8 @@ pub(crate) fn schnorr_verify( message: &[FunctionInput], output: Witness, ) -> Result<(), OpcodeResolutionError> { - let public_key_x: &F = &input_to_value(initial_witness, public_key_x)?; - let public_key_y: &F = &input_to_value(initial_witness, public_key_y)?; + let public_key_x: &F = &input_to_value(initial_witness, public_key_x, false)?; + let public_key_y: &F = &input_to_value(initial_witness, public_key_y, false)?; let signature = to_u8_array(initial_witness, signature)?; let message = to_u8_vec(initial_witness, message)?; diff --git a/acvm-repo/acvm/src/pwg/blackbox/utils.rs b/acvm-repo/acvm/src/pwg/blackbox/utils.rs index 9b9157421e5..b966cb0cc5d 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/utils.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/utils.rs @@ -8,7 +8,7 @@ pub(crate) fn to_u8_array( ) -> Result<[u8; N], OpcodeResolutionError> { let mut result = [0; N]; for (it, input) in result.iter_mut().zip(inputs) { - let witness_value_bytes = input_to_value(initial_witness, *input)?.to_be_bytes(); + let witness_value_bytes = input_to_value(initial_witness, *input, false)?.to_be_bytes(); let byte = witness_value_bytes .last() .expect("Field element must be represented by non-zero amount of bytes"); @@ -23,7 +23,7 @@ pub(crate) fn to_u8_vec( ) -> Result, OpcodeResolutionError> { let mut result = Vec::with_capacity(inputs.len()); for input in inputs { - let witness_value_bytes = input_to_value(initial_witness, *input)?.to_be_bytes(); + let witness_value_bytes = input_to_value(initial_witness, *input, false)?.to_be_bytes(); let byte = witness_value_bytes .last() .expect("Field element must be represented by non-zero amount of bytes"); diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index f061129d0ff..d86b1c80c3d 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -133,7 +133,7 @@ pub enum OpcodeResolutionError { #[error("Cannot solve opcode: {invalid_input_bit_size}")] InvalidInputBitSize { opcode_location: ErrorLocation, - invalid_input_bit_size: InvalidInputBitSize, + invalid_input_bit_size: InvalidInputBitSize, }, #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), @@ -159,8 +159,8 @@ impl From for OpcodeResolutionError { } } -impl From> for OpcodeResolutionError { - fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { +impl From for OpcodeResolutionError { + fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { Self::InvalidInputBitSize { opcode_location: ErrorLocation::Unresolved, invalid_input_bit_size, @@ -656,21 +656,26 @@ pub fn witness_to_value( } } +// TODO(https://github.com/noir-lang/noir/issues/5985): +// remove skip_bitsize_checks pub fn input_to_value( initial_witness: &WitnessMap, input: FunctionInput, + skip_bitsize_checks: bool, ) -> Result> { match input.input() { ConstantOrWitnessEnum::Witness(witness) => { let initial_value = *witness_to_value(initial_witness, witness)?; - if initial_value.num_bits() <= input.num_bits() { + if skip_bitsize_checks || initial_value.num_bits() <= input.num_bits() { Ok(initial_value) } else { + let value_num_bits = initial_value.num_bits(); + let value = format!("{}", initial_value); Err(OpcodeResolutionError::InvalidInputBitSize { opcode_location: ErrorLocation::Unresolved, - invalid_input_bit_size: InvalidInputBitSize { - value: initial_value, + value, + value_num_bits, max_bits: input.num_bits(), }, }) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 8059cd024d5..18a13517b75 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -4,7 +4,7 @@ #![warn(clippy::semicolon_if_nothing_returned)] use abi_gen::{abi_type_from_hir_type, value_from_hir_expression}; -use acvm::{acir::circuit::ExpressionWidth, FieldElement}; +use acvm::acir::circuit::ExpressionWidth; use clap::Args; use fm::{FileId, FileManager}; use iter_extended::vecmap; @@ -142,7 +142,7 @@ pub fn parse_expression_width(input: &str) -> Result), + RuntimeError(RuntimeError), } impl From for CompileError { @@ -151,8 +151,8 @@ impl From for CompileError { } } -impl From> for CompileError { - fn from(error: RuntimeError) -> Self { +impl From for CompileError { + fn from(error: RuntimeError) -> Self { Self::RuntimeError(error) } } diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index daf01d74ea3..e1944b42dbc 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::{acir::InvalidInputBitSize, AcirField, FieldElement}; +use acvm::{acir::InvalidInputBitSize, FieldElement}; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; @@ -16,7 +16,7 @@ use crate::ssa::ir::{dfg::CallStack, types::NumericType}; use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone, Error)] -pub enum RuntimeError { +pub enum RuntimeError { #[error(transparent)] InternalError(#[from] InternalError), #[error("Range constraint of {num_bits} bits is too large for the Field size")] @@ -55,7 +55,7 @@ pub enum RuntimeError { #[error("Could not resolve some references to the array. All references must be resolved at compile time")] UnknownReference { call_stack: CallStack }, #[error("{invalid_input_bit_size}")] - InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, + InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -136,7 +136,7 @@ pub enum InternalError { Unexpected { expected: String, found: String, call_stack: CallStack }, } -impl RuntimeError { +impl RuntimeError { fn call_stack(&self) -> &CallStack { match self { RuntimeError::InternalError( @@ -168,8 +168,8 @@ impl RuntimeError { } } -impl From> for FileDiagnostic { - fn from(error: RuntimeError) -> FileDiagnostic { +impl From for FileDiagnostic { + fn from(error: RuntimeError) -> FileDiagnostic { let call_stack = vecmap(error.call_stack(), |location| *location); let file_id = call_stack.last().map(|location| location.file).unwrap_or_default(); let diagnostic = error.into_diagnostic(); @@ -177,7 +177,7 @@ impl From> for FileDiagnostic { } } -impl RuntimeError { +impl RuntimeError { fn into_diagnostic(self) -> Diagnostic { match self { RuntimeError::InternalError(cause) => { diff --git a/compiler/noirc_evaluator/src/ssa.rs b/compiler/noirc_evaluator/src/ssa.rs index 541aca47c10..ad6645df228 100644 --- a/compiler/noirc_evaluator/src/ssa.rs +++ b/compiler/noirc_evaluator/src/ssa.rs @@ -79,7 +79,7 @@ pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec); pub(crate) fn optimize_into_acir( program: Program, options: &SsaEvaluatorOptions, -) -> Result> { +) -> Result { let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); let ssa_gen_span_guard = ssa_gen_span.enter(); @@ -207,7 +207,7 @@ impl SsaProgramArtifact { pub fn create_program( program: Program, options: &SsaEvaluatorOptions, -) -> Result> { +) -> Result { let debug_variables = program.debug_variables.clone(); let debug_types = program.debug_types.clone(); let debug_functions = program.debug_functions.clone(); @@ -385,7 +385,7 @@ impl SsaBuilder { force_brillig_runtime: bool, print_codegen_timings: bool, emit_ssa: &Option, - ) -> Result> { + ) -> Result { let ssa = ssa_gen::generate_ssa(program, force_brillig_runtime)?; if let Some(emit_ssa) = emit_ssa { let mut emit_ssa_dir = emit_ssa.clone(); @@ -412,9 +412,9 @@ impl SsaBuilder { /// The same as `run_pass` but for passes that may fail fn try_run_pass( mut self, - pass: fn(Ssa) -> Result>, + pass: fn(Ssa) -> Result, msg: &str, - ) -> Result> { + ) -> Result { self.ssa = time(msg, self.print_codegen_timings, || pass(self.ssa))?; Ok(self.print(msg)) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 410aedc0c26..c00d7d57528 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -320,7 +320,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result> { + ) -> Result { let var_data = &self.vars[&var]; if let AcirVarData::Const(constant) = var_data { // Note that this will return a 0 if the inverse is not available @@ -365,7 +365,7 @@ impl AcirContext { &mut self, var: AcirVar, predicate: AcirVar, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let pred_mul_var = self.mul_var(var, predicate)?; self.assert_eq_var(pred_mul_var, predicate, None) } @@ -383,11 +383,7 @@ impl AcirContext { /// Returns an `AcirVar` that is `1` if `lhs` equals `rhs` and /// 0 otherwise. - pub(crate) fn eq_var( - &mut self, - lhs: AcirVar, - rhs: AcirVar, - ) -> Result> { + pub(crate) fn eq_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -410,7 +406,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result> { + ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -447,7 +443,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result> { + ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -477,7 +473,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, typ: AcirType, - ) -> Result> { + ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; if lhs_expr.is_zero() { @@ -511,7 +507,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, assert_message: Option>, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -545,7 +541,7 @@ impl AcirContext { pub(crate) fn vars_to_expressions_or_memory( &self, values: &[AcirValue], - ) -> Result>, RuntimeError> { + ) -> Result>, RuntimeError> { let mut result = Vec::with_capacity(values.len()); for value in values { match value { @@ -572,7 +568,7 @@ impl AcirContext { rhs: AcirVar, typ: AcirType, predicate: AcirVar, - ) -> Result> { + ) -> Result { let numeric_type = match typ { AcirType::NumericType(numeric_type) => numeric_type, AcirType::Array(_, _) => { @@ -599,11 +595,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the multiplication of `lhs` and `rhs` - pub(crate) fn mul_var( - &mut self, - lhs: AcirVar, - rhs: AcirVar, - ) -> Result> { + pub(crate) fn mul_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_data = self.vars[&lhs].clone(); let rhs_data = self.vars[&rhs].clone(); @@ -684,22 +676,14 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the subtraction of `lhs` and `rhs` - pub(crate) fn sub_var( - &mut self, - lhs: AcirVar, - rhs: AcirVar, - ) -> Result> { + pub(crate) fn sub_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let neg_rhs = self.neg_var(rhs); self.add_var(lhs, neg_rhs) } /// Adds a new Variable to context whose value will /// be constrained to be the addition of `lhs` and `rhs` - pub(crate) fn add_var( - &mut self, - lhs: AcirVar, - rhs: AcirVar, - ) -> Result> { + pub(crate) fn add_var(&mut self, lhs: AcirVar, rhs: AcirVar) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -762,12 +746,7 @@ impl AcirContext { /// Adds a new Variable to context whose value will /// be constrained to be the expression `lhs + k * rhs` - fn add_mul_var( - &mut self, - lhs: AcirVar, - k: F, - rhs: AcirVar, - ) -> Result> { + fn add_mul_var(&mut self, lhs: AcirVar, k: F, rhs: AcirVar) -> Result { let k_var = self.add_constant(k); let intermediate = self.mul_var(k_var, rhs)?; @@ -775,11 +754,7 @@ impl AcirContext { } /// Adds a new variable that is constrained to be the logical NOT of `x`. - pub(crate) fn not_var( - &mut self, - x: AcirVar, - typ: AcirType, - ) -> Result> { + pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = self.add_constant((1_u128 << bit_size) - 1); @@ -793,7 +768,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result<(AcirVar, AcirVar), RuntimeError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { let zero = self.add_constant(F::zero()); let one = self.add_constant(F::one()); @@ -972,7 +947,7 @@ impl AcirContext { rhs: AcirVar, offset: AcirVar, bits: u32, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { const fn num_bits() -> usize { std::mem::size_of::() * 8 } @@ -1029,7 +1004,7 @@ impl AcirContext { lhs: AcirVar, leading: AcirVar, max_bit_size: u32, - ) -> Result> { + ) -> Result { let max_power_of_two = self.add_constant(F::from(2_u128).pow(&F::from(max_bit_size as u128 - 1))); @@ -1048,7 +1023,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - ) -> Result<(AcirVar, AcirVar), RuntimeError> { + ) -> Result<(AcirVar, AcirVar), RuntimeError> { // We derive the signed division from the unsigned euclidean division. // note that this is not euclidean division! // If `x` is a signed integer, then `sign(x)x >= 0` @@ -1105,7 +1080,7 @@ impl AcirContext { rhs: AcirVar, bit_size: u32, predicate: AcirVar, - ) -> Result> { + ) -> Result { let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder) } @@ -1116,7 +1091,7 @@ impl AcirContext { variable: AcirVar, numeric_type: &NumericType, message: Option, - ) -> Result> { + ) -> Result { match numeric_type { NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { // If `variable` is constant then we don't need to add a constraint. @@ -1152,7 +1127,7 @@ impl AcirContext { lhs: AcirVar, rhs: u32, max_bit_size: u32, - ) -> Result> { + ) -> Result { // 2^{rhs} let divisor = self.add_constant(F::from(2_u128).pow(&F::from(rhs as u128))); let one = self.add_constant(F::one()); @@ -1174,7 +1149,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_count: u32, - ) -> Result> { + ) -> Result { let pow_last = self.add_constant(F::from(1_u128 << (bit_count - 1))); let pow = self.add_constant(F::from(1_u128 << (bit_count))); @@ -1222,7 +1197,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, max_bits: u32, - ) -> Result> { + ) -> Result { // Returns a `Witness` that is constrained to be: // - `1` if lhs >= rhs // - `0` otherwise @@ -1290,7 +1265,7 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - ) -> Result> { + ) -> Result { // Flip the result of calling more than equal method to // compute less than. let comparison = self.more_than_eq_var(lhs, rhs, bit_size)?; @@ -1306,7 +1281,7 @@ impl AcirContext { name: BlackBoxFunc, mut inputs: Vec, mut output_count: usize, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // Separate out any arguments that should be constants let (constant_inputs, constant_outputs) = match name { BlackBoxFunc::PedersenCommitment | BlackBoxFunc::PedersenHash => { @@ -1446,10 +1421,8 @@ impl AcirContext { BlackBoxFunc::AES128Encrypt => { let invalid_input = "aes128_encrypt - operation requires a plaintext to encrypt"; let input_size: usize = match inputs.first().expect(invalid_input) { - AcirValue::Array(values) => Ok::>(values.len()), - AcirValue::DynamicArray(dyn_array) => { - Ok::>(dyn_array.len) - } + AcirValue::Array(values) => Ok::(values.len()), + AcirValue::DynamicArray(dyn_array) => Ok::(dyn_array.len), _ => { return Err(RuntimeError::InternalError(InternalError::General { message: "aes128_encrypt requires an array of inputs".to_string(), @@ -1529,7 +1502,7 @@ impl AcirContext { &mut self, inputs: Vec, allow_constant_inputs: bool, - ) -> Result>>, RuntimeError> { + ) -> Result>>, RuntimeError> { let mut witnesses = Vec::new(); for input in inputs { let mut single_val_witnesses = Vec::new(); @@ -1571,7 +1544,7 @@ impl AcirContext { radix_var: AcirVar, limb_count: u32, result_element_type: AcirType, - ) -> Result> { + ) -> Result { let radix = match self.vars[&radix_var].as_constant() { Some(radix) => radix.to_u128() as u32, None => { @@ -1608,7 +1581,7 @@ impl AcirContext { input_var: AcirVar, limb_count: u32, result_element_type: AcirType, - ) -> Result> { + ) -> Result { let two_var = self.add_constant(2_u128); self.radix_decompose(endian, input_var, two_var, limb_count, result_element_type) } @@ -1677,7 +1650,7 @@ impl AcirContext { unsafe_return_values: bool, brillig_function_index: BrilligFunctionId, brillig_stdlib_func: Option, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let predicate = self.var_to_expression(predicate)?; if predicate.is_zero() { // If the predicate has a constant value of zero, the brillig call will never be executed. @@ -1758,7 +1731,7 @@ impl AcirContext { fn range_constraint_value( context: &mut AcirContext, value: &AcirValue, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { match value { AcirValue::Var(var, typ) => { let numeric_type = match typ { @@ -2037,7 +2010,7 @@ impl AcirContext { inputs: Vec, output_count: usize, predicate: AcirVar, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let inputs = self.prepare_inputs_for_black_box_func_call(inputs, false)?; let inputs = inputs .iter() diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 7ea2a857776..0cad7b9c978 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -377,7 +377,7 @@ impl GeneratedAcir { radix: u32, limb_count: u32, bit_size: u32, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let radix_big = BigUint::from(radix); assert_eq!( BigUint::from(2u128).pow(bit_size), @@ -549,7 +549,7 @@ impl GeneratedAcir { &mut self, witness: Witness, num_bits: u32, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // We class this as an error because users should instead // do `as Field`. if num_bits >= F::max_num_bits() { diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 5f7b99e0cf2..15b44fde65d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -292,7 +292,7 @@ impl Ssa { self, brillig: &Brillig, expression_width: ExpressionWidth, - ) -> Result> { + ) -> Result { let mut acirs = Vec::new(); // TODO: can we parallelize this? let mut shared_context = SharedContext::default(); @@ -373,7 +373,7 @@ impl<'a> Context<'a> { ssa: &Ssa, function: &Function, brillig: &Brillig, - ) -> Result>, RuntimeError> { + ) -> Result>, RuntimeError> { match function.runtime() { RuntimeType::Acir(inline_type) => { match inline_type { @@ -405,7 +405,7 @@ impl<'a> Context<'a> { main_func: &Function, ssa: &Ssa, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let entry_block = &dfg[main_func.entry_block()]; let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?; @@ -463,7 +463,7 @@ impl<'a> Context<'a> { mut self, main_func: &Function, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let dfg = &main_func.dfg; let inputs = try_vecmap(dfg[main_func.entry_block()].parameters(), |param_id| { @@ -521,7 +521,7 @@ impl<'a> Context<'a> { &mut self, params: &[ValueId], dfg: &DataFlowGraph, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { // The first witness (if any) is the next one let start_witness = self.acir_context.current_witness_index().0; for param_id in params { @@ -554,18 +554,15 @@ impl<'a> Context<'a> { Ok(witnesses) } - fn convert_ssa_block_param( - &mut self, - param_type: &Type, - ) -> Result> { + fn convert_ssa_block_param(&mut self, param_type: &Type) -> Result { self.create_value_from_type(param_type, &mut |this, typ| this.add_numeric_input_var(&typ)) } fn create_value_from_type( &mut self, param_type: &Type, - make_var: &mut impl FnMut(&mut Self, NumericType) -> Result>, - ) -> Result> { + make_var: &mut impl FnMut(&mut Self, NumericType) -> Result, + ) -> Result { match param_type { Type::Numeric(numeric_type) => { let typ = AcirType::new(*numeric_type); @@ -621,7 +618,7 @@ impl<'a> Context<'a> { fn add_numeric_input_var( &mut self, numeric_type: &NumericType, - ) -> Result> { + ) -> Result { let acir_var = self.acir_context.add_variable(); if matches!(numeric_type, NumericType::Signed { .. } | NumericType::Unsigned { .. }) { self.acir_context.range_constrain_var(acir_var, numeric_type, None)?; @@ -636,7 +633,7 @@ impl<'a> Context<'a> { dfg: &DataFlowGraph, ssa: &Ssa, brillig: &Brillig, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let instruction = &dfg[instruction_id]; self.acir_context.set_call_stack(dfg.get_call_stack(instruction_id)); let mut warnings = Vec::new(); @@ -755,7 +752,7 @@ impl<'a> Context<'a> { ssa: &Ssa, brillig: &Brillig, result_ids: &[ValueId], - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let mut warnings = Vec::new(); match instruction { @@ -897,7 +894,7 @@ impl<'a> Context<'a> { result_ids: &[ValueId], output_values: Vec, dfg: &DataFlowGraph, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { for (result_id, output) in result_ids.iter().zip(output_values) { if let AcirValue::Array(_) = &output { let array_id = dfg.resolve(*result_id); @@ -991,7 +988,7 @@ impl<'a> Context<'a> { &mut self, instruction: InstructionId, dfg: &DataFlowGraph, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let mut mutable_array_set = false; // Pass the instruction between array methods rather than the internal fields themselves @@ -1068,7 +1065,7 @@ impl<'a> Context<'a> { array: ValueId, index: ValueId, store_value: Option, - ) -> Result> { + ) -> Result { let array_id = dfg.resolve(array); let array_typ = dfg.type_of_value(array_id); // Compiler sanity checks @@ -1107,7 +1104,7 @@ impl<'a> Context<'a> { array: Vector, index: FieldElement, store_value: Option, - ) -> Result> { + ) -> Result { let array_size: usize = array.len(); let index = match index.try_to_u64() { Some(index_const) => index_const as usize, @@ -1164,7 +1161,7 @@ impl<'a> Context<'a> { index: ValueId, store_value: Option, offset: usize, - ) -> Result<(AcirVar, Option), RuntimeError> { + ) -> Result<(AcirVar, Option), RuntimeError> { let array_typ = dfg.type_of_value(array_id); let block_id = self.ensure_array_is_initialized(array_id, dfg)?; @@ -1213,7 +1210,7 @@ impl<'a> Context<'a> { &mut self, store_value: &AcirValue, dummy_value: &AcirValue, - ) -> Result> { + ) -> Result { match (store_value, dummy_value) { (AcirValue::Var(store_var, _), AcirValue::Var(dummy_var, _)) => { let true_pred = @@ -1260,10 +1257,7 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*block_id, &index_var)?; - Ok::>(AcirValue::Var( - read, - AcirType::field(), - )) + Ok::(AcirValue::Var(read, AcirType::field())) })?; let mut elements = im::Vector::new(); @@ -1291,7 +1285,7 @@ impl<'a> Context<'a> { mut var_index: AcirVar, dfg: &DataFlowGraph, mut index_side_effect: bool, - ) -> Result> { + ) -> Result { let block_id = self.ensure_array_is_initialized(array, dfg)?; let results = dfg.instruction_results(instruction); let res_typ = dfg.type_of_value(results[0]); @@ -1354,7 +1348,7 @@ impl<'a> Context<'a> { ssa_type: &Type, block_id: BlockId, var_index: &mut AcirVar, - ) -> Result> { + ) -> Result { let one = self.acir_context.add_constant(FieldElement::one()); match ssa_type.clone() { Type::Numeric(numeric_type) => { @@ -1395,7 +1389,7 @@ impl<'a> Context<'a> { store_value: AcirValue, dfg: &DataFlowGraph, mutate_array: bool, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // Pass the instruction between array methods rather than the internal fields themselves let array = match dfg[instruction] { Instruction::ArraySet { array, .. } => array, @@ -1470,7 +1464,7 @@ impl<'a> Context<'a> { value: &AcirValue, block_id: BlockId, var_index: &mut AcirVar, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let one = self.acir_context.add_constant(FieldElement::one()); match value { AcirValue::Var(store_var, _) => { @@ -1489,10 +1483,7 @@ impl<'a> Context<'a> { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?; - Ok::>(AcirValue::Var( - read, - AcirType::field(), - )) + Ok::(AcirValue::Var(read, AcirType::field())) })?; self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?; } @@ -1504,7 +1495,7 @@ impl<'a> Context<'a> { &mut self, array: ValueId, dfg: &DataFlowGraph, - ) -> Result> { + ) -> Result { // Use the SSA ID to get or create its block ID let block_id = self.block_id(&array); @@ -1543,7 +1534,7 @@ impl<'a> Context<'a> { array_id: ValueId, supplied_acir_value: Option<&AcirValue>, dfg: &DataFlowGraph, - ) -> Result> { + ) -> Result { let element_type_sizes = self.internal_block_id(&array_id); // Check whether an internal type sizes array has already been initialized // Need to look into how to optimize for slices as this could lead to different element type sizes @@ -1659,12 +1650,12 @@ impl<'a> Context<'a> { source: BlockId, destination: BlockId, array_len: usize, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { let init_values = try_vecmap(0..array_len, |i| { let index_var = self.acir_context.add_constant(i); let read = self.acir_context.read_from_memory(source, &index_var)?; - Ok::>(AcirValue::Var(read, AcirType::field())) + Ok::(AcirValue::Var(read, AcirType::field())) })?; let array: Vector = init_values.into(); self.initialize_array(destination, array_len, Some(AcirValue::Array(array)))?; @@ -1677,7 +1668,7 @@ impl<'a> Context<'a> { array_id: ValueId, var_index: AcirVar, dfg: &DataFlowGraph, - ) -> Result> { + ) -> Result { if !can_omit_element_sizes_array(array_typ) { let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?; @@ -1821,7 +1812,7 @@ impl<'a> Context<'a> { &mut self, terminator: &TerminatorInstruction, dfg: &DataFlowGraph, - ) -> Result<(Vec, Vec), RuntimeError> { + ) -> Result<(Vec, Vec), RuntimeError> { let (return_values, call_stack) = match terminator { TerminatorInstruction::Return { return_values, call_stack } => { (return_values, call_stack.clone()) @@ -1938,7 +1929,7 @@ impl<'a> Context<'a> { &mut self, binary: &Binary, dfg: &DataFlowGraph, - ) -> Result> { + ) -> Result { let lhs = self.convert_numeric_value(binary.lhs, dfg)?; let rhs = self.convert_numeric_value(binary.rhs, dfg)?; @@ -2021,7 +2012,7 @@ impl<'a> Context<'a> { rhs: ValueId, dfg: &DataFlowGraph, op: BinaryOp, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { // We try to optimize away operations that are guaranteed not to overflow let max_lhs_bits = dfg.get_value_max_num_bits(lhs); let max_rhs_bits = dfg.get_value_max_num_bits(rhs); @@ -2114,7 +2105,7 @@ impl<'a> Context<'a> { bit_size: u32, max_bit_size: u32, dfg: &DataFlowGraph, - ) -> Result> { + ) -> Result { let mut var = self.convert_numeric_value(value_id, dfg)?; match &dfg[value_id] { Value::Instruction { instruction, .. } => { @@ -2149,7 +2140,7 @@ impl<'a> Context<'a> { arguments: &[ValueId], dfg: &DataFlowGraph, result_ids: &[ValueId], - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { match intrinsic { Intrinsic::BlackBox(black_box) => { // Slices are represented as a tuple of (length, slice contents). @@ -2787,7 +2778,7 @@ impl<'a> Context<'a> { &mut self, old_slice: &mut Vector, input: AcirValue, - ) -> Result<(), RuntimeError> { + ) -> Result<(), RuntimeError> { match input { AcirValue::Var(_, _) => { old_slice.push_back(input); diff --git a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs index 61f5a0837c4..ae0681a55ff 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs @@ -7,7 +7,6 @@ use crate::{ value::ValueId, }, ssa_gen::Ssa, - FieldElement, }, }; @@ -25,7 +24,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn evaluate_static_assert_and_assert_constant( mut self, - ) -> Result> { + ) -> Result { for function in self.functions.values_mut() { for block in function.reachable_blocks() { // Unfortunately we can't just use instructions.retain(...) here since @@ -55,7 +54,7 @@ impl Ssa { fn check_instruction( function: &mut Function, instruction: InstructionId, -) -> Result> { +) -> Result { let assert_constant_id = function.dfg.import_intrinsic(Intrinsic::AssertConstant); let static_assert_id = function.dfg.import_intrinsic(Intrinsic::StaticAssert); match &function.dfg[instruction] { @@ -80,7 +79,7 @@ fn evaluate_assert_constant( function: &Function, instruction: InstructionId, arguments: &[ValueId], -) -> Result> { +) -> Result { if arguments.iter().all(|arg| function.dfg.is_constant(*arg)) { Ok(false) } else { @@ -99,7 +98,7 @@ fn evaluate_static_assert( function: &Function, instruction: InstructionId, arguments: &[ValueId], -) -> Result> { +) -> Result { if arguments.len() != 2 { panic!("ICE: static_assert called with wrong number of arguments") } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index b589bcca9b1..f3e11e04e3a 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -33,7 +33,6 @@ use crate::{ value::ValueId, }, ssa_gen::Ssa, - FieldElement, }, }; use fxhash::FxHashMap as HashMap; @@ -41,9 +40,7 @@ use fxhash::FxHashMap as HashMap; impl Ssa { /// Loop unrolling can return errors, since ACIR functions need to be fully unrolled. /// This meta-pass will keep trying to unroll loops and simplifying the SSA until no more errors are found. - pub(crate) fn unroll_loops_iteratively( - mut ssa: Ssa, - ) -> Result> { + pub(crate) fn unroll_loops_iteratively(mut ssa: Ssa) -> Result { // Try to unroll loops first: let mut unroll_errors; (ssa, unroll_errors) = ssa.try_to_unroll_loops(); @@ -74,7 +71,7 @@ impl Ssa { /// If any loop cannot be unrolled, it is left as-is or in a partially unrolled state. /// Returns the ssa along with all unrolling errors encountered #[tracing::instrument(level = "trace", skip(self))] - pub(crate) fn try_to_unroll_loops(mut self) -> (Ssa, Vec>) { + pub(crate) fn try_to_unroll_loops(mut self) -> (Ssa, Vec) { let mut errors = vec![]; for function in self.functions.values_mut() { // Loop unrolling in brillig can lead to a code explosion currently. This can @@ -150,7 +147,7 @@ fn find_all_loops(function: &Function) -> Loops { impl Loops { /// Unroll all loops within a given function. /// Any loops which fail to be unrolled (due to using non-constant indices) will be unmodified. - fn unroll_each_loop(mut self, function: &mut Function) -> Vec> { + fn unroll_each_loop(mut self, function: &mut Function) -> Vec { let mut unroll_errors = vec![]; while let Some(next_loop) = self.yet_to_unroll.pop() { // If we've previously modified a block in this loop we need to refresh the context. diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index e4839d3eda4..fb7091a8854 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -272,7 +272,7 @@ impl<'a> FunctionContext<'a> { value: impl Into, negative: bool, typ: Type, - ) -> Result> { + ) -> Result { let value = value.into(); if let Type::Numeric(numeric_type) = typ { @@ -703,7 +703,7 @@ impl<'a> FunctionContext<'a> { pub(super) fn extract_current_value( &mut self, lvalue: &ast::LValue, - ) -> Result> { + ) -> Result { Ok(match lvalue { ast::LValue::Ident(ident) => { let (reference, should_auto_deref) = self.ident_lvalue(ident); @@ -755,7 +755,7 @@ impl<'a> FunctionContext<'a> { array: &ast::LValue, index: &ast::Expression, location: &Location, - ) -> Result<(ValueId, ValueId, LValue, Option), RuntimeError> { + ) -> Result<(ValueId, ValueId, LValue, Option), RuntimeError> { let (old_array, array_lvalue) = self.extract_current_value_recursive(array)?; let index = self.codegen_non_tuple_expression(index)?; let array_lvalue = Box::new(array_lvalue); @@ -782,7 +782,7 @@ impl<'a> FunctionContext<'a> { fn extract_current_value_recursive( &mut self, lvalue: &ast::LValue, - ) -> Result<(Values, LValue), RuntimeError> { + ) -> Result<(Values, LValue), RuntimeError> { match lvalue { ast::LValue::Ident(ident) => { let (variable, should_auto_deref) = self.ident_lvalue(ident); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 4a7f2768361..2318fea8960 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -13,7 +13,7 @@ use noirc_frontend::monomorphization::ast::{self, Expression, Program}; use crate::{ errors::RuntimeError, - ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic, FieldElement}, + ssa::{function_builder::data_bus::DataBusBuilder, ir::instruction::Intrinsic}, }; use self::{ @@ -40,7 +40,7 @@ pub(crate) const SSA_WORD_SIZE: u32 = 32; pub(crate) fn generate_ssa( program: Program, force_brillig_runtime: bool, -) -> Result> { +) -> Result { // see which parameter has call_data/return_data attribute let is_databus = DataBusBuilder::is_databus(&program.main_function_signature); @@ -124,10 +124,7 @@ pub(crate) fn generate_ssa( impl<'a> FunctionContext<'a> { /// Codegen a function's body and set its return value to that of its last parameter. /// For functions returning nothing, this will be an empty list. - fn codegen_function_body( - &mut self, - body: &Expression, - ) -> Result<(), RuntimeError> { + fn codegen_function_body(&mut self, body: &Expression) -> Result<(), RuntimeError> { let entry_block = self.increment_parameter_rcs(); let return_value = self.codegen_expression(body)?; let results = return_value.into_value_list(self); @@ -137,10 +134,7 @@ impl<'a> FunctionContext<'a> { Ok(()) } - fn codegen_expression( - &mut self, - expr: &Expression, - ) -> Result> { + fn codegen_expression(&mut self, expr: &Expression) -> Result { match expr { Expression::Ident(ident) => Ok(self.codegen_ident(ident)), Expression::Literal(literal) => self.codegen_literal(literal), @@ -169,10 +163,7 @@ impl<'a> FunctionContext<'a> { /// Codegen any non-tuple expression so that we can unwrap the Values /// tree to return a single value for use with most SSA instructions. - fn codegen_non_tuple_expression( - &mut self, - expr: &Expression, - ) -> Result> { + fn codegen_non_tuple_expression(&mut self, expr: &Expression) -> Result { Ok(self.codegen_expression(expr)?.into_leaf().eval(self)) } @@ -201,10 +192,7 @@ impl<'a> FunctionContext<'a> { self.codegen_ident_reference(ident).map(|value| value.eval(self).into()) } - fn codegen_literal( - &mut self, - literal: &ast::Literal, - ) -> Result> { + fn codegen_literal(&mut self, literal: &ast::Literal) -> Result { match literal { ast::Literal::Array(array) => { let elements = @@ -270,7 +258,7 @@ impl<'a> FunctionContext<'a> { &mut self, elements: Vec, typ: Type, - ) -> Result> { + ) -> Result { if typ.is_nested_slice() { return Err(RuntimeError::NestedSlice { call_stack: self.builder.get_call_stack() }); } @@ -306,10 +294,7 @@ impl<'a> FunctionContext<'a> { self.builder.array_constant(array, typ).into() } - fn codegen_block( - &mut self, - block: &[Expression], - ) -> Result> { + fn codegen_block(&mut self, block: &[Expression]) -> Result { let mut result = Self::unit_value(); for expr in block { result = self.codegen_expression(expr)?; @@ -317,7 +302,7 @@ impl<'a> FunctionContext<'a> { Ok(result) } - fn codegen_unary(&mut self, unary: &ast::Unary) -> Result> { + fn codegen_unary(&mut self, unary: &ast::Unary) -> Result { match unary.operator { UnaryOp::Not => { let rhs = self.codegen_expression(&unary.rhs)?; @@ -366,10 +351,7 @@ impl<'a> FunctionContext<'a> { }) } - fn codegen_reference( - &mut self, - expr: &Expression, - ) -> Result> { + fn codegen_reference(&mut self, expr: &Expression) -> Result { match expr { Expression::Ident(ident) => Ok(self.codegen_ident_reference(ident)), Expression::ExtractTupleField(tuple, index) => { @@ -380,16 +362,13 @@ impl<'a> FunctionContext<'a> { } } - fn codegen_binary( - &mut self, - binary: &ast::Binary, - ) -> Result> { + fn codegen_binary(&mut self, binary: &ast::Binary) -> Result { let lhs = self.codegen_non_tuple_expression(&binary.lhs)?; let rhs = self.codegen_non_tuple_expression(&binary.rhs)?; Ok(self.insert_binary(lhs, binary.operator, rhs, binary.location)) } - fn codegen_index(&mut self, index: &ast::Index) -> Result> { + fn codegen_index(&mut self, index: &ast::Index) -> Result { let array_or_slice = self.codegen_expression(&index.collection)?.into_value_list(self); let index_value = self.codegen_non_tuple_expression(&index.index)?; // Slices are represented as a tuple in the form: (length, slice contents). @@ -422,7 +401,7 @@ impl<'a> FunctionContext<'a> { element_type: &ast::Type, location: Location, length: Option, - ) -> Result> { + ) -> Result { // base_index = index * type_size let index = self.make_array_index(index); let type_size = Self::convert_type(element_type).size_of_type(); @@ -475,7 +454,7 @@ impl<'a> FunctionContext<'a> { ); } - fn codegen_cast(&mut self, cast: &ast::Cast) -> Result> { + fn codegen_cast(&mut self, cast: &ast::Cast) -> Result { let lhs = self.codegen_non_tuple_expression(&cast.lhs)?; let typ = Self::convert_non_tuple_type(&cast.r#type); @@ -499,7 +478,7 @@ impl<'a> FunctionContext<'a> { /// br loop_entry(v4) /// loop_end(): /// ... This is the current insert point after codegen_for finishes ... - fn codegen_for(&mut self, for_expr: &ast::For) -> Result> { + fn codegen_for(&mut self, for_expr: &ast::For) -> Result { let loop_entry = self.builder.insert_block(); let loop_body = self.builder.insert_block(); let loop_end = self.builder.insert_block(); @@ -570,7 +549,7 @@ impl<'a> FunctionContext<'a> { /// br end_if() /// end_if: // No block parameter is needed. Without an else, the unit value is always returned. /// ... This is the current insert point after codegen_if finishes ... - fn codegen_if(&mut self, if_expr: &ast::If) -> Result> { + fn codegen_if(&mut self, if_expr: &ast::If) -> Result { let condition = self.codegen_non_tuple_expression(&if_expr.condition)?; let then_block = self.builder.insert_block(); @@ -610,10 +589,7 @@ impl<'a> FunctionContext<'a> { Ok(result) } - fn codegen_tuple( - &mut self, - tuple: &[Expression], - ) -> Result> { + fn codegen_tuple(&mut self, tuple: &[Expression]) -> Result { Ok(Tree::Branch(try_vecmap(tuple, |expr| self.codegen_expression(expr))?)) } @@ -621,14 +597,14 @@ impl<'a> FunctionContext<'a> { &mut self, tuple: &Expression, field_index: usize, - ) -> Result> { + ) -> Result { let tuple = self.codegen_expression(tuple)?; Ok(Self::get_field(tuple, field_index)) } /// Generate SSA for a function call. Note that calls to built-in functions /// and intrinsics are also represented by the function call instruction. - fn codegen_call(&mut self, call: &ast::Call) -> Result> { + fn codegen_call(&mut self, call: &ast::Call) -> Result { let function = self.codegen_non_tuple_expression(&call.func)?; let mut arguments = Vec::with_capacity(call.arguments.len()); @@ -677,7 +653,7 @@ impl<'a> FunctionContext<'a> { /// If the variable is immutable, no special handling is necessary and we can return the given /// ValueId directly. If it is mutable, we'll need to allocate space for the value and store /// the initial value before returning the allocate instruction. - fn codegen_let(&mut self, let_expr: &ast::Let) -> Result> { + fn codegen_let(&mut self, let_expr: &ast::Let) -> Result { let mut values = self.codegen_expression(&let_expr.expression)?; values = values.map(|value| { @@ -702,7 +678,7 @@ impl<'a> FunctionContext<'a> { expr: &Expression, location: Location, assert_payload: &Option>, - ) -> Result> { + ) -> Result { let expr = self.codegen_non_tuple_expression(expr)?; let true_literal = self.builder.numeric_constant(true, Type::bool()); @@ -723,7 +699,7 @@ impl<'a> FunctionContext<'a> { fn codegen_constrain_error( &mut self, assert_message: &Option>, - ) -> Result, RuntimeError> { + ) -> Result, RuntimeError> { let Some(assert_message_payload) = assert_message else { return Ok(None) }; if let Expression::Literal(ast::Literal::Str(static_string)) = &assert_message_payload.0 { @@ -745,10 +721,7 @@ impl<'a> FunctionContext<'a> { Ok(Some(ConstrainError::Dynamic(error_type_id, values))) } - fn codegen_assign( - &mut self, - assign: &ast::Assign, - ) -> Result> { + fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { let lhs = self.extract_current_value(&assign.lvalue)?; let rhs = self.codegen_expression(&assign.expression)?; @@ -761,7 +734,7 @@ impl<'a> FunctionContext<'a> { Ok(Self::unit_value()) } - fn codegen_semi(&mut self, expr: &Expression) -> Result> { + fn codegen_semi(&mut self, expr: &Expression) -> Result { self.codegen_expression(expr)?; Ok(Self::unit_value()) } From 821b2d4aa26348b61bf3a13ae4f88c76f3ba9ca3 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 9 Sep 2024 15:46:48 -0400 Subject: [PATCH 15/22] add 'bitsize' to cspell.json, replace usage of InvalidInputBitSize with UnsupportedIntegerSize in noirc_evaluator --- compiler/noirc_evaluator/src/errors.rs | 14 ++++++++------ .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 6 ++++-- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 1 + cspell.json | 1 + 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index e1944b42dbc..15d3ca66d4a 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::{acir::InvalidInputBitSize, FieldElement}; +use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; @@ -32,8 +32,13 @@ pub enum RuntimeError { TypeConversion { from: String, into: String, call_stack: CallStack }, #[error("{name:?} is not initialized")] UnInitialized { name: String, call_stack: CallStack }, - #[error("Integer sized {num_bits:?} is over the max supported size of {max_num_bits:?}")] - UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, call_stack: CallStack }, + #[error("Integer{} sized {num_bits:?} is over the max supported size of {max_num_bits:?}", opt_value.as_ref().map(|x| format!(" ({})", x)).unwrap_or("".to_string()))] + UnsupportedIntegerSize { + opt_value: Option, + num_bits: u32, + max_num_bits: u32, + call_stack: CallStack, + }, #[error("Could not determine loop bound at compile-time")] UnknownLoopBound { call_stack: CallStack }, #[error("Argument is not constant")] @@ -54,8 +59,6 @@ pub enum RuntimeError { UnconstrainedOracleReturnToConstrained { call_stack: CallStack }, #[error("Could not resolve some references to the array. All references must be resolved at compile time")] UnknownReference { call_stack: CallStack }, - #[error("{invalid_input_bit_size}")] - InvalidInputBitSize { invalid_input_bit_size: InvalidInputBitSize, call_stack: CallStack }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -160,7 +163,6 @@ impl RuntimeError { | RuntimeError::UnsupportedIntegerSize { call_stack, .. } | RuntimeError::NestedSlice { call_stack, .. } | RuntimeError::BigIntModulus { call_stack, .. } - | RuntimeError::InvalidInputBitSize { call_stack, .. } | RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack } | RuntimeError::UnconstrainedOracleReturnToConstrained { call_stack } | RuntimeError::UnknownReference { call_stack } => call_stack, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index c00d7d57528..7de71094ff6 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1512,8 +1512,10 @@ impl AcirContext { Some(constant) if allow_constant_inputs => { single_val_witnesses.push( FunctionInput::constant(*constant, num_bits).map_err( - |invalid_input_bit_size| RuntimeError::InvalidInputBitSize { - invalid_input_bit_size, + |invalid_input_bit_size| RuntimeError::UnsupportedIntegerSize { + opt_value: Some(invalid_input_bit_size.value), + num_bits: invalid_input_bit_size.value_num_bits, + max_num_bits: invalid_input_bit_size.max_bits, call_stack: self.get_call_stack(), }, )?, diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 15b44fde65d..07310a6f929 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1943,6 +1943,7 @@ impl<'a> Context<'a> { let max_integer_bit_size = FieldElement::max_num_bits() / 2; if *bit_size > max_integer_bit_size { return Err(RuntimeError::UnsupportedIntegerSize { + opt_value: None, num_bits: *bit_size, max_num_bits: max_integer_bit_size, call_stack: self.acir_context.get_call_stack(), diff --git a/cspell.json b/cspell.json index 293523d7c15..4d83c535e7d 100644 --- a/cspell.json +++ b/cspell.json @@ -25,6 +25,7 @@ "bindgen", "bitand", "bitmask", + "bitsize", "blackbox", "boilerplate", "boilerplates", From 52eabc56dcd217f7620536cd6422bb3b4487b2f4 Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Tue, 10 Sep 2024 10:55:27 -0400 Subject: [PATCH 16/22] split UnsupportedIntegerSize into self and InvalidBlackBoxInputBitSize --- compiler/noirc_evaluator/src/errors.rs | 9 ++++++--- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 12 +++++++----- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 1 - 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index 15d3ca66d4a..c4ba08f9acd 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -32,9 +32,11 @@ pub enum RuntimeError { TypeConversion { from: String, into: String, call_stack: CallStack }, #[error("{name:?} is not initialized")] UnInitialized { name: String, call_stack: CallStack }, - #[error("Integer{} sized {num_bits:?} is over the max supported size of {max_num_bits:?}", opt_value.as_ref().map(|x| format!(" ({})", x)).unwrap_or("".to_string()))] - UnsupportedIntegerSize { - opt_value: Option, + #[error("Integer sized {num_bits:?} is over the max supported size of {max_num_bits:?}")] + UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, call_stack: CallStack }, + #[error("Integer {value}, sized {num_bits:?}, is over the max supported size of {max_num_bits:?} for the blackbox function's inputs")] + InvalidBlackBoxInputBitSize { + value: String, num_bits: u32, max_num_bits: u32, call_stack: CallStack, @@ -161,6 +163,7 @@ impl RuntimeError { | RuntimeError::StaticAssertFailed { call_stack } | RuntimeError::IntegerOutOfBounds { call_stack, .. } | RuntimeError::UnsupportedIntegerSize { call_stack, .. } + | RuntimeError::InvalidBlackBoxInputBitSize { call_stack, .. } | RuntimeError::NestedSlice { call_stack, .. } | RuntimeError::BigIntModulus { call_stack, .. } | RuntimeError::UnconstrainedSliceReturnToConstrained { call_stack } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 7de71094ff6..9586d08e10c 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1512,11 +1512,13 @@ impl AcirContext { Some(constant) if allow_constant_inputs => { single_val_witnesses.push( FunctionInput::constant(*constant, num_bits).map_err( - |invalid_input_bit_size| RuntimeError::UnsupportedIntegerSize { - opt_value: Some(invalid_input_bit_size.value), - num_bits: invalid_input_bit_size.value_num_bits, - max_num_bits: invalid_input_bit_size.max_bits, - call_stack: self.get_call_stack(), + |invalid_input_bit_size| { + RuntimeError::InvalidBlackBoxInputBitSize { + value: invalid_input_bit_size.value, + num_bits: invalid_input_bit_size.value_num_bits, + max_num_bits: invalid_input_bit_size.max_bits, + call_stack: self.get_call_stack(), + } }, )?, ); diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 07310a6f929..15b44fde65d 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1943,7 +1943,6 @@ impl<'a> Context<'a> { let max_integer_bit_size = FieldElement::max_num_bits() / 2; if *bit_size > max_integer_bit_size { return Err(RuntimeError::UnsupportedIntegerSize { - opt_value: None, num_bits: *bit_size, max_num_bits: max_integer_bit_size, call_stack: self.acir_context.get_call_stack(), From 010df44c695b04dc168e84d35d9ef0ae0adf720f Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 14:44:50 +0100 Subject: [PATCH 17/22] chore: remove unnecessary trait constraints --- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 2 +- acvm-repo/acvm/src/pwg/mod.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index a0a08b57048..ccad3510682 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -49,7 +49,7 @@ impl AcvmBigIntSolver { Ok(()) } - pub(crate) fn bigint_op( + pub(crate) fn bigint_op( &mut self, lhs: u32, rhs: u32, diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index d86b1c80c3d..ae470684dc1 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -39,7 +39,7 @@ pub use self::brillig::{BrilligSolver, BrilligSolverStatus}; pub use brillig::ForeignCallWaitInfo; #[derive(Debug, Clone, PartialEq)] -pub enum ACVMStatus { +pub enum ACVMStatus { /// All opcodes have been solved. Solved, @@ -64,7 +64,7 @@ pub enum ACVMStatus { RequiresAcirCall(AcirCallWaitInfo), } -impl std::fmt::Display for ACVMStatus { +impl std::fmt::Display for ACVMStatus { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { ACVMStatus::Solved => write!(f, "Solved"), @@ -120,7 +120,7 @@ impl std::fmt::Display for ErrorLocation { } #[derive(Clone, PartialEq, Eq, Debug, Error)] -pub enum OpcodeResolutionError { +pub enum OpcodeResolutionError { #[error("Cannot solve opcode: {0}")] OpcodeNotSolvable(#[from] OpcodeNotSolvable), #[error("Cannot satisfy constraint")] @@ -149,7 +149,7 @@ pub enum OpcodeResolutionError { AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 }, } -impl From for OpcodeResolutionError { +impl From for OpcodeResolutionError { fn from(value: BlackBoxResolutionError) -> Self { match value { BlackBoxResolutionError::Failed(func, reason) => { @@ -159,7 +159,7 @@ impl From for OpcodeResolutionError { } } -impl From for OpcodeResolutionError { +impl From for OpcodeResolutionError { fn from(invalid_input_bit_size: InvalidInputBitSize) -> Self { Self::InvalidInputBitSize { opcode_location: ErrorLocation::Unresolved, @@ -646,7 +646,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { // Returns the concrete value for a particular witness // If the witness has no assignment, then // an error is returned -pub fn witness_to_value( +pub fn witness_to_value( initial_witness: &WitnessMap, witness: Witness, ) -> Result<&F, OpcodeResolutionError> { From b9d52c761686fe8391e9925ca42daf1ef6eb03e4 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 14:54:32 +0100 Subject: [PATCH 18/22] . --- acvm-repo/acvm/src/pwg/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index ae470684dc1..0f0da74a698 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -76,7 +76,7 @@ impl std::fmt::Display for ACVMStatus { } } -pub enum StepResult<'a, F: AcirField, B: BlackBoxFunctionSolver> { +pub enum StepResult<'a, F, B: BlackBoxFunctionSolver> { Status(ACVMStatus), IntoBrillig(BrilligSolver<'a, F, B>), } From 404c8386222d6dab43b96e41c64176daf6ae7dc3 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 14:56:24 +0100 Subject: [PATCH 19/22] . --- acvm-repo/acvm/src/pwg/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 0f0da74a698..5ffe81e813d 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -168,7 +168,7 @@ impl From for OpcodeResolutionError { } } -pub struct ACVM<'a, F: AcirField, B: BlackBoxFunctionSolver> { +pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { status: ACVMStatus, backend: &'a B, From e92a509c882ad0103225430a248a6065100679ac Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 11 Sep 2024 14:56:58 +0100 Subject: [PATCH 20/22] Update acvm-repo/acvm/src/pwg/mod.rs --- acvm-repo/acvm/src/pwg/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 5ffe81e813d..c73893ceea6 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -670,7 +670,7 @@ pub fn input_to_value( Ok(initial_value) } else { let value_num_bits = initial_value.num_bits(); - let value = format!("{}", initial_value); + let value = initial_value.to_string(); Err(OpcodeResolutionError::InvalidInputBitSize { opcode_location: ErrorLocation::Unresolved, invalid_input_bit_size: InvalidInputBitSize { From 136b48caffae207ba7cf39500f3fae5ea0fcb45c Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 15:04:37 +0100 Subject: [PATCH 21/22] chore: remove extra `From` impls --- acvm-repo/acir_field/src/field_element.rs | 12 ------------ acvm-repo/acvm/tests/solver.rs | 4 ++-- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs index 102821d0911..2323f008dbe 100644 --- a/acvm-repo/acir_field/src/field_element.rs +++ b/acvm-repo/acir_field/src/field_element.rs @@ -137,18 +137,6 @@ impl<'de, T: PrimeField> Deserialize<'de> for FieldElement { } } -impl From for FieldElement { - fn from(a: BigUint) -> FieldElement { - FieldElement(F::from(a)) - } -} - -impl From> for BigUint { - fn from(a: FieldElement) -> BigUint { - F::into(a.0) - } -} - impl From for FieldElement { fn from(a: u128) -> FieldElement { FieldElement(F::from(a)) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index c8e1bf9de2c..93841b082c4 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -1370,8 +1370,8 @@ prop_compose! { let positive_patch_value = std::cmp::max(patch_value, 1); if distinct_inputs_len != 0 { let previous_input = &mut distinct_inputs[patch_location % distinct_inputs_len].0; - let patched_input: BigUint = (*previous_input + FieldElement::from(positive_patch_value)).into(); - *previous_input = (patched_input % BigUint::from(modulus)).into(); + let patched_input: BigUint = (*previous_input + FieldElement::from(positive_patch_value)).into_repr().into(); + *previous_input = FieldElement::from_be_bytes_reduce(&patched_input.to_bytes_be()); } else { distinct_inputs.push((FieldElement::zero(), true)); } From 876d5430ebef22c3778e095ad37c1f3202feb022 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 11 Sep 2024 15:53:58 +0100 Subject: [PATCH 22/22] . --- acvm-repo/acvm/tests/solver.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 93841b082c4..a7b3808efc4 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -1371,7 +1371,7 @@ prop_compose! { if distinct_inputs_len != 0 { let previous_input = &mut distinct_inputs[patch_location % distinct_inputs_len].0; let patched_input: BigUint = (*previous_input + FieldElement::from(positive_patch_value)).into_repr().into(); - *previous_input = FieldElement::from_be_bytes_reduce(&patched_input.to_bytes_be()); + *previous_input = FieldElement::from_be_bytes_reduce(&(patched_input % BigUint::from(modulus)).to_bytes_be()); } else { distinct_inputs.push((FieldElement::zero(), true)); }