Merge branch 'master' into jf/remove-vector-start

Author: jfecher

.github/critical_libraries_status/noir-lang/keccak256/.failures.jsonl

@@ -1,4 +1,5 @@
 {"suite":"keccak256","name":"keccak256::oracle_tests::test_keccak256_1"}
 {"suite":"keccak256","name":"keccak256::oracle_tests::test_keccak256_100"}
 {"suite":"keccak256","name":"keccak256::oracle_tests::test_keccak256_135"}
+{"suite":"keccak256","name":"keccak256::oracle_tests::test_keccak256_136"}
 {"suite":"keccak256","name":"keccak256::oracle_tests::test_keccak256_256"}

acvm-repo/acvm/src/compiler/validator.rs

@@ -367,10 +367,14 @@ pub fn validate_witness<F: AcirField>(
                 solver.check_memory_op(op, &witness_map, opcode_index)?;
             }
             Opcode::MemoryInit { block_id, init, .. } => {
-                MemoryOpSolver::new(init, &witness_map).map(|solver| {
-                    let existing_block_id = block_solvers.insert(*block_id, solver);
-                    assert!(existing_block_id.is_none(), "Memory block already initialized");
-                })?;
+                let solver = MemoryOpSolver::new(init, &witness_map)?;
+                let existing_block_id = block_solvers.insert(*block_id, solver);
+                if existing_block_id.is_some() {
+                    return Err(unsatisfied_constraint(
+                        opcode_index,
+                        format!("Attempted reinitialization of memory block {:?}", block_id.0,),
+                    ));
+                }
             }
             // BrilligCall is unconstrained
             Opcode::BrilligCall { .. } => (),
@@ -443,14 +447,32 @@ mod tests {
     use acir::{
         AcirField, FieldElement,
         circuit::{
-            Circuit, Opcode, PublicInputs,
-            opcodes::{BlackBoxFuncCall, FunctionInput},
+            Circuit, Opcode, OpcodeLocation, PublicInputs,
+            brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs},
+            opcodes::{AcirFunctionId, BlackBoxFuncCall, BlockId, FunctionInput, MemOp},
         },
         native_types::{Expression, Witness, WitnessMap},
     };
     use bn254_blackbox_solver::Bn254BlackBoxSolver;
 
     use super::validate_witness;
+    use crate::pwg::{
+        ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ResolvedAssertionPayload,
+    };
+
+    fn assert_unsatisfied_constraint(
+        result: Result<(), OpcodeResolutionError<FieldElement>>,
+        opcode_index: usize,
+        message: &str,
+    ) {
+        assert_eq!(
+            result.unwrap_err(),
+            OpcodeResolutionError::UnsatisfiedConstrain {
+                opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir(opcode_index)),
+                payload: Some(ResolvedAssertionPayload::String(message.to_string())),
+            },
+        );
+    }
 
     /// Helper to create a simple circuit with the given opcodes
     fn make_circuit(opcodes: Vec<Opcode<FieldElement>>) -> Circuit<FieldElement> {
@@ -511,7 +533,11 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            0,
+            "Invalid witness assignment: w1 + w2 - w3",
+        );
     }
 
     #[test]
@@ -564,7 +590,11 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            0,
+            "RANGE opcode violation: value 256 does not fit in 8 bits",
+        );
     }
 
     #[test]
@@ -604,7 +634,11 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            0,
+            "AND opcode violation: 10 AND 12 != 15 for 8 bits",
+        );
     }
 
     #[test]
@@ -644,7 +678,11 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            0,
+            "XOR opcode violation: 10 XOR 12 != 15 for 8 bits",
+        );
     }
 
     #[test]
@@ -661,15 +699,15 @@ mod tests {
         let witness_map = WitnessMap::default();
 
         let backend = Bn254BlackBoxSolver;
-        // The expression evaluates with missing witness, but won't be zero
-        // so this should fail
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            0,
+            "Invalid witness assignment: w1",
+        );
     }
 
     #[test]
     fn test_call_opcode_valid() {
-        use acir::circuit::opcodes::AcirFunctionId;
-
         let circuit = make_circuit(vec![Opcode::Call {
             id: AcirFunctionId(1),
             inputs: vec![Witness(1), Witness(2)],
@@ -689,8 +727,6 @@ mod tests {
 
     #[test]
     fn test_call_opcode_missing_input() {
-        use acir::circuit::opcodes::AcirFunctionId;
-
         let circuit = make_circuit(vec![Opcode::Call {
             id: AcirFunctionId(1),
             inputs: vec![Witness(1), Witness(2)],
@@ -705,13 +741,14 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_eq!(
+            validate_witness(&backend, witness_map, &circuit).unwrap_err(),
+            OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment(2)),
+        );
     }
 
     #[test]
     fn test_call_opcode_missing_output() {
-        use acir::circuit::opcodes::AcirFunctionId;
-
         let circuit = make_circuit(vec![Opcode::Call {
             id: AcirFunctionId(1),
             inputs: vec![Witness(1), Witness(2)],
@@ -726,13 +763,14 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_eq!(
+            validate_witness(&backend, witness_map, &circuit).unwrap_err(),
+            OpcodeResolutionError::OpcodeNotSolvable(OpcodeNotSolvable::MissingAssignment(3)),
+        );
     }
 
     #[test]
     fn test_call_opcode_skipped_with_zero_predicate() {
-        use acir::circuit::opcodes::AcirFunctionId;
-
         // Predicate is zero, so call should be skipped even with missing witnesses
         let circuit = make_circuit(vec![Opcode::Call {
             id: AcirFunctionId(1),
@@ -756,8 +794,6 @@ mod tests {
 
     #[test]
     fn test_memory_init_and_read() {
-        use acir::circuit::opcodes::{BlockId, MemOp};
-
         let block_id = BlockId(0);
 
         let circuit = make_circuit(vec![
@@ -784,8 +820,6 @@ mod tests {
 
     #[test]
     fn test_memory_read_wrong_value() {
-        use acir::circuit::opcodes::{BlockId, MemOp};
-
         let block_id = BlockId(0);
 
         let circuit = make_circuit(vec![
@@ -805,13 +839,15 @@ mod tests {
         ]));
 
         let backend = Bn254BlackBoxSolver;
-        assert!(validate_witness(&backend, witness_map, &circuit).is_err());
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            1,
+            "Memory read opcode violation at index 0: expected 42 but found 99",
+        );
     }
 
     #[test]
     fn test_memory_write_then_read() {
-        use acir::circuit::opcodes::{BlockId, MemOp};
-
         let block_id = BlockId(0);
 
         let circuit = make_circuit(vec![
@@ -841,8 +877,6 @@ mod tests {
 
     #[test]
     fn test_brillig_call_with_empty_witness_map() {
-        use acir::circuit::brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs};
-
         // Create a BrilligCall opcode with input and output witnesses
         // Brillig calls are unconstrained and should be skipped during validation,
         // so this should pass even with an empty witness map
@@ -862,4 +896,31 @@ mod tests {
         let backend = Bn254BlackBoxSolver;
         assert!(validate_witness(&backend, witness_map, &circuit).is_ok());
     }
+
+    #[test]
+    fn error_on_memory_init_duplicate_block_id() {
+        let block_id = BlockId(0);
+
+        let circuit = make_circuit(vec![
+            Opcode::MemoryInit {
+                block_id,
+                init: vec![],
+                block_type: acir::circuit::opcodes::BlockType::Memory,
+            },
+            Opcode::MemoryInit {
+                block_id,
+                init: vec![],
+                block_type: acir::circuit::opcodes::BlockType::Memory,
+            },
+        ]);
+
+        let witness_map = WitnessMap::default();
+        let backend = Bn254BlackBoxSolver;
+
+        assert_unsatisfied_constraint(
+            validate_witness(&backend, witness_map, &circuit),
+            1,
+            format!("Attempted reinitialization of memory block {}", block_id.0).as_str(),
+        );
+    }
 }

compiler/noirc_evaluator/src/ssa/interpreter/mod.rs

@@ -1379,7 +1379,7 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> {
             elements.iter().zip(element_types.iter().cycle()).enumerate()
         {
             let actual_type = element.get_type();
-            if &actual_type != expected_type {
+            if !actual_type.canonical_eq(expected_type) {
                 return Err(internal(InternalError::MakeArrayElementTypeMismatch {
                     result,
                     index,

compiler/noirc_evaluator/src/ssa/interpreter/tests/instructions.rs

@@ -1161,6 +1161,31 @@ fn make_array() {
     assert_eq!(values[3], Value::vector(hello, Arc::new(vec![Type::char()])));
 }
 
+#[test]
+fn make_array_allows_reference_mutability_mismatch() {
+    // Reference mutability is a frontend concern with no meaning at the SSA
+    // level: the validator accepts a `&mut T` value in a `&T` MakeArray slot,
+    // and the interpreter must agree so that running the post-validation SSA
+    // doesn't fail with `MakeArrayElementTypeMismatch`. The unconstrained
+    // SSA-gen pattern this guards is a tuple `[&mut T, &T]` constructed from
+    // a mutable allocate alongside an immutable one — exactly what the
+    // `pass_vs_prev` fuzzer surfaces when it interprets intermediate SSA
+    // between passes.
+    executes_with_no_errors(
+        "
+        brillig(inline) fn main f0 {
+          b0():
+            v0 = allocate -> &mut Field
+            store Field 1 at v0
+            v1 = allocate -> &Field
+            store Field 2 at v1
+            v2 = make_array [v0, v1] : [&Field; 2]
+            return
+        }
+    ",
+    );
+}
+
 #[test]
 fn nop() {
     executes_with_no_errors(

compiler/noirc_evaluator/src/ssa/ir/function.rs

@@ -372,12 +372,11 @@ pub(crate) struct Signature {
 impl Signature {
     /// Construct a [Signature] whose parameter and return types have all
     /// reference mutability canonicalized away. This makes `&T` and `&mut T`
-    /// compare equal when a [Signature] is used as a map key, matching the
-    /// validator's `types_equal_ignoring_reference_mutability` leniency and
-    /// the frontend's `&mut T → &T` coercion.
+    /// compare equal when a [Signature] is used as a map key, matching
+    /// [Type::canonical_eq] leniency and the frontend's `&mut T → &T` coercion.
     pub(crate) fn new(mut params: Vec<Type>, mut returns: Vec<Type>) -> Self {
         for typ in params.iter_mut().chain(returns.iter_mut()) {
-            typ.canonicalize_reference_mutability();
+            typ.canonicalize();
         }
         Self { params, returns }
     }

compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -635,10 +635,21 @@ impl Instruction {
     }
 
     /// Replaces values present in this instruction with other values according to the given mapping.
-    pub(crate) fn replace_values(&mut self, mapping: &ValueMapping) {
-        if !mapping.is_empty() {
-            self.map_values_mut(|value_id| mapping.get(value_id));
+    ///
+    /// Returns `true` if any value was actually replaced.
+    pub(crate) fn replace_values(&mut self, mapping: &ValueMapping) -> bool {
+        if mapping.is_empty() {
+            return false;
         }
+        let mut changed = false;
+        self.map_values_mut(|value_id| {
+            let new_value = mapping.get(value_id);
+            if new_value != value_id {
+                changed = true;
+            }
+            new_value
+        });
+        changed
     }
 
     /// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction.
@@ -1178,3 +1189,56 @@ where
         focus.set(i, y);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use acvm::acir::brillig::lengths::SemanticLength;
+
+    #[test]
+    fn replace_values_returns_true_only_when_a_value_changes() {
+        let v0 = ValueId::test_new(0);
+        let v1 = ValueId::test_new(1);
+        let v2 = ValueId::test_new(2);
+
+        let mut mapping = ValueMapping::default();
+        mapping.insert(v1, v2);
+
+        let mut instruction_using_v1 = Instruction::Cast(v1, NumericType::NativeField);
+        assert!(instruction_using_v1.replace_values(&mapping));
+        assert!(matches!(instruction_using_v1, Instruction::Cast(v, _) if v == v2));
+
+        let mut instruction_using_v0 = Instruction::Cast(v0, NumericType::NativeField);
+        assert!(!instruction_using_v0.replace_values(&mapping));
+        assert!(matches!(instruction_using_v0, Instruction::Cast(v, _) if v == v0));
+
+        let empty_mapping = ValueMapping::default();
+        let mut instruction = Instruction::Cast(v1, NumericType::NativeField);
+        assert!(!instruction.replace_values(&empty_mapping));
+    }
+
+    #[test]
+    fn replace_values_returns_true_when_a_make_array_element_changes() {
+        let v0 = ValueId::test_new(0);
+        let v1 = ValueId::test_new(1);
+        let v2 = ValueId::test_new(2);
+
+        let mut mapping = ValueMapping::default();
+        mapping.insert(v1, v2);
+
+        let typ = Type::Array(std::sync::Arc::new(vec![Type::field()]), SemanticLength(2));
+        let mut instruction =
+            Instruction::MakeArray { elements: im::Vector::from(vec![v0, v1]), typ: typ.clone() };
+        assert!(instruction.replace_values(&mapping));
+        let Instruction::MakeArray { elements, .. } = instruction else { unreachable!() };
+        assert_eq!(elements[0], v0);
+        assert_eq!(elements[1], v2);
+
+        let mut unrelated =
+            Instruction::MakeArray { elements: im::Vector::from(vec![v0, v0]), typ };
+        assert!(!unrelated.replace_values(&mapping));
+        let Instruction::MakeArray { elements, .. } = unrelated else { unreachable!() };
+        assert_eq!(elements[0], v0);
+        assert_eq!(elements[1], v0);
+    }
+}