chore: typos and some refactors in acvm/src/pwg

acvm-repo/acvm/src/pwg/arithmetic.rs

@@ -24,7 +24,7 @@ pub(crate) enum MulTerm<F> {
 
 impl ExpressionSolver {
     /// Derives the rest of the witness in the provided expression based on the known witness values
-    /// 1. Fist we simplify the expression based on the known values and try to reduce the multiplication and linear terms
+    /// 1. First we simplify the expression based on the known values and try to reduce the multiplication and linear terms
     /// 2. If we end up with only the constant term;
     ///     - if it is 0 then the opcode is solved, if not,
     ///     - the assert_zero opcode is not satisfied and we return an error
@@ -148,8 +148,8 @@ impl ExpressionSolver {
         arith_opcode: &Expression<F>,
         witness_assignments: &WitnessMap<F>,
     ) -> Result<MulTerm<F>, OpcodeStatus<F>> {
-        // First note that the mul term can only contain one/zero term
-        // We are assuming it has been optimized.
+        // First note that the mul term can only contain one/zero term,
+        // e.g. that it has been optimized, or else we're returning OpcodeUnsolvable
         match arith_opcode.mul_terms.len() {
             0 => Ok(MulTerm::Solved(F::zero())),
             1 => Ok(ExpressionSolver::solve_mul_term_helper(
@@ -195,13 +195,12 @@ impl ExpressionSolver {
     }
 
     /// Returns the summation of all of the variables, plus the unknown variable
-    /// Returns None, if there is more than one unknown variable
-    /// We cannot assign
+    /// Returns [`OpcodeStatus::OpcodeUnsolvable`], if there is more than one unknown variable
     pub(super) fn solve_fan_in_term<F: AcirField>(
         arith_opcode: &Expression<F>,
         witness_assignments: &WitnessMap<F>,
     ) -> OpcodeStatus<F> {
-        // This is assuming that the fan-in is more than 0
+        // If the fan-in has more than 0 num_unknowns:
 
         // This is the variable that we want to assign the value to
         let mut unknown_variable = (F::zero(), Witness::default());
@@ -276,7 +275,7 @@ impl ExpressionSolver {
 /// is ±1.
 ///
 /// Field inversion is the most significant cost of solving [`Opcode::AssertZero`][acir::circuit::opcodes::Opcode::AssertZero]
-/// opcodes, we can avoid this in the situation
+/// opcodes, which we can avoid when the denominator is ±1.
 fn quick_invert<F: AcirField>(numerator: F, denominator: F) -> F {
     if denominator == F::one() {
         numerator

acvm-repo/acvm/src/pwg/blackbox/hash.rs

@@ -12,8 +12,7 @@ use crate::pwg::{input_to_value, insert_value};
 /// If successful, `initial_witness` will be mutated to contain the new witness assignment.
 pub(super) fn solve_generic_256_hash_opcode<F: AcirField>(
     initial_witness: &mut WitnessMap<F>,
-    #[allow(clippy::ptr_arg)] //Clippy mistakenly believes that it can be replaced by &[..]
-    inputs: &Vec<FunctionInput<F>>,
+    inputs: &[FunctionInput<F>],
     var_message_size: Option<&FunctionInput<F>>,
     outputs: &[Witness; 32],
     hash_function: fn(data: &[u8]) -> Result<[u8; 32], BlackBoxResolutionError>,
@@ -42,7 +41,10 @@ fn get_hash_input<F: AcirField>(
     // 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)?
+                .try_into_u128()
+                .map(|num_bytes_to_take| num_bytes_to_take as usize)
+                .expect("expected a 'num_bytes_to_take' that fit into a u128");
 
             // If the number of bytes to take is more than the amount of bytes available
             // in the message, then we error.
@@ -83,7 +85,9 @@ fn to_u32_array<const N: usize, F: AcirField>(
     let mut result = [0; N];
     for (it, input) in result.iter_mut().zip(inputs) {
         let witness_value = input_to_value(initial_witness, *input)?;
-        *it = witness_value.to_u128() as u32;
+        *it = witness_value
+            .try_into_u128()
+            .expect("expected the 'witness_value' to fit into a u128") as u32;
     }
     Ok(result)
 }

acvm-repo/acvm/src/pwg/blackbox/logic.rs

@@ -59,14 +59,13 @@ fn solve_logic_opcode<F: AcirField>(
     pedantic_solving: bool,
     logic_op: impl Fn(F, F) -> F,
 ) -> Result<(), OpcodeResolutionError<F>> {
-    // TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these by
-    // default once we figure out how to combine these with existing
-    // noirc_frontend/noirc_evaluator overflow error messages
-    let skip_bitsize_checks = !pedantic_solving;
     let w_l_value = input_to_value(initial_witness, *a)?;
     let w_r_value = input_to_value(initial_witness, *b)?;
     let assignment = logic_op(w_l_value, w_r_value);
-    if !skip_bitsize_checks {
+    // TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these by
+    // default once we figure out how to combine these with existing
+    // noirc_frontend/noirc_evaluator overflow error messages
+    if pedantic_solving {
         check_bit_size(w_l_value, num_bits)?;
         check_bit_size(w_r_value, num_bits)?;
     }

acvm-repo/acvm/src/pwg/blackbox/utils.rs

@@ -8,11 +8,12 @@ pub(crate) fn to_u8_array<const N: usize, F: AcirField>(
 ) -> Result<[u8; N], OpcodeResolutionError<F>> {
     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 byte = witness_value_bytes
-            .last()
-            .expect("Field element must be represented by non-zero amount of bytes");
-        *it = *byte;
+        let byte: u8 = input_to_value(initial_witness, *input)?
+            .try_into_u128()
+            .expect("expected input to fit into a u8")
+            .try_into()
+            .expect("expected input to fit into a u8");
+        *it = byte;
     }
     Ok(result)
 }
@@ -23,11 +24,12 @@ pub(crate) fn to_u8_vec<F: AcirField>(
 ) -> Result<Vec<u8>, OpcodeResolutionError<F>> {
     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 byte = witness_value_bytes
-            .last()
-            .expect("Field element must be represented by non-zero amount of bytes");
-        result.push(*byte);
+        let byte: u8 = input_to_value(initial_witness, *input)?
+            .try_into_u128()
+            .expect("expected input to fit into a u8")
+            .try_into()
+            .expect("expected input to fit into a u8");
+        result.push(byte);
     }
     Ok(result)
 }

acvm-repo/acvm/src/pwg/memory_op.rs

@@ -32,22 +32,19 @@ impl<F: AcirField> MemoryOpSolver<F> {
     }
 
     fn len(&self) -> u32 {
-        self.block_value.len() as u32
+        u32::try_from(self.block_value.len()).expect("expected a length that fits into a u32")
     }
 
     /// Convert a field element into a memory index
     /// Only 32 bits values are valid memory indices
     fn index_from_field(&self, index: F) -> Result<MemoryIndex, OpcodeResolutionError<F>> {
-        if index.num_bits() <= 32 {
-            let memory_index = index.try_to_u64().unwrap() as MemoryIndex;
-            Ok(memory_index)
-        } else {
-            Err(OpcodeResolutionError::IndexOutOfBounds {
+        index.try_to_u32().ok_or_else({
+            || OpcodeResolutionError::IndexOutOfBounds {
                 opcode_location: ErrorLocation::Unresolved,
                 index,
                 array_size: self.len(),
-            })
-        }
+            }
+        })
     }
 
     /// Update the 'block_value' map with the provided index/value
@@ -103,6 +100,7 @@ impl<F: AcirField> MemoryOpSolver<F> {
         &mut self,
         op: &MemOp<F>,
         initial_witness: &mut WitnessMap<F>,
+        pedantic_solving: bool,
     ) -> Result<(), OpcodeResolutionError<F>> {
         let operation = get_value(&op.operation, initial_witness)?;
 
@@ -118,6 +116,16 @@ impl<F: AcirField> MemoryOpSolver<F> {
 
         // `operation == 0` implies a read operation. (`operation == 1` implies write operation).
         let is_read_operation = operation.is_zero();
+        if pedantic_solving {
+            // We expect that the 'operation' should resolve to either 0 or 1.
+            if !is_read_operation && !operation.is_one() {
+                let opcode_location = ErrorLocation::Unresolved;
+                return Err(OpcodeResolutionError::MemoryOperationLargerThanOne {
+                    opcode_location,
+                    operation,
+                });
+            }
+        }
 
         if is_read_operation {
             // `value_read = arr[memory_index]`
@@ -173,7 +181,8 @@ mod tests {
         let mut block_solver = MemoryOpSolver::new(&init, &initial_witness).unwrap();
 
         for op in trace {
-            block_solver.solve_memory_op(&op, &mut initial_witness).unwrap();
+            let pedantic_solving = true;
+            block_solver.solve_memory_op(&op, &mut initial_witness, pedantic_solving).unwrap();
         }
 
         assert_eq!(initial_witness[&Witness(4)], FieldElement::from(2u128));
@@ -197,7 +206,9 @@ mod tests {
         let mut err = None;
         for op in invalid_trace {
             if err.is_none() {
-                err = block_solver.solve_memory_op(&op, &mut initial_witness).err();
+                let pedantic_solving = true;
+                err =
+                    block_solver.solve_memory_op(&op, &mut initial_witness, pedantic_solving).err();
             }
         }
 

acvm-repo/acvm/src/pwg/mod.rs

@@ -258,6 +258,8 @@ pub enum OpcodeResolutionError<F> {
     AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 },
     #[error("(--pedantic): Predicates are expected to be 0 or 1, but found: {pred_value}")]
     PredicateLargerThanOne { opcode_location: ErrorLocation, pred_value: F },
+    #[error("(--pedantic): Memory operations are expected to be 0 or 1, but found: {operation}")]
+    MemoryOperationLargerThanOne { opcode_location: ErrorLocation, operation: F },
 }
 
 impl<F> From<BlackBoxResolutionError> for OpcodeResolutionError<F> {
@@ -524,7 +526,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> ACVM<'a, F, B> {
                     .block_solvers
                     .get_mut(block_id)
                     .expect("Memory block should have been initialized before use");
-                solver.solve_memory_op(op, &mut self.witness_map)
+                solver.solve_memory_op(op, &mut self.witness_map, self.backend.pedantic_solving())
             }
             Opcode::BrilligCall { .. } => match self.solve_brillig_call_opcode() {
                 Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call),