chore!: restrict MemOp to only take witnesses

acvm-repo/acir/src/circuit/opcodes.rs

@@ -102,7 +102,7 @@ pub enum Opcode<F: AcirField> {
         /// Identifier of the array
         block_id: BlockId,
         /// Describe the memory operation to perform
-        op: MemOp<F>,
+        op: MemOp,
     },
 
     /// Initialize an ACIR array from a vector of witnesses.
@@ -174,11 +174,11 @@ pub(super) fn display_opcode<F: AcirField>(
         }
         Opcode::BlackBoxFuncCall(g) => std::fmt::Display::fmt(&g, f),
         Opcode::MemoryOp { block_id, op } => {
-            let is_read = op.operation.is_zero();
-            if is_read {
-                write!(f, "READ {} = b{}[{}]", op.value, block_id.0, op.index)
-            } else {
+            let is_write = op.operation;
+            if is_write {
                 write!(f, "WRITE b{}[{}] = {}", block_id.0, op.index, op.value)
+            } else {
+                write!(f, "READ {} = b{}[{}]", op.value, block_id.0, op.index)
             }
         }
         Opcode::MemoryInit { block_id, init, block_type: databus } => {

acvm-repo/acir/src/circuit/opcodes/memory_operation.rs

@@ -1,5 +1,4 @@
-use crate::native_types::{Expression, Witness};
-use acir_field::AcirField;
+use crate::native_types::Witness;
 use serde::{Deserialize, Serialize};
 
 /// Identifier for a block of memory
@@ -17,25 +16,25 @@ impl std::fmt::Display for BlockId {
 /// We can either write or read at an index in memory
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug, Hash)]
 #[cfg_attr(feature = "arb", derive(proptest_derive::Arbitrary))]
-pub struct MemOp<F> {
+pub struct MemOp {
     /// A constant expression that can be 0 (read) or 1 (write)
-    pub operation: Expression<F>,
+    pub operation: bool,
     /// array index, it must be less than the array length
-    pub index: Expression<F>,
+    pub index: Witness,
     /// the value we are reading, when operation is 0, or the value we write at
     /// the specified index, when operation is 1
-    pub value: Expression<F>,
+    pub value: Witness,
 }
 
-impl<F: AcirField> MemOp<F> {
+impl MemOp {
     /// Creates a `MemOp` which reads from memory at `index` and inserts the read value
     /// into the [`WitnessMap`][crate::native_types::WitnessMap] at `witness`
-    pub fn read_at_mem_index(index: Expression<F>, witness: Witness) -> Self {
-        MemOp { operation: Expression::zero(), index, value: witness.into() }
+    pub fn read_at_mem_index(index: Witness, witness: Witness) -> Self {
+        MemOp { operation: false, index, value: witness }
     }
 
-    /// Creates a `MemOp` which writes the [`Expression`] `value` into memory at `index`.
-    pub fn write_to_mem_index(index: Expression<F>, value: Expression<F>) -> Self {
-        MemOp { operation: Expression::one(), index, value }
+    /// Creates a `MemOp` which writes the value assigned to the [`Witness`] `value` into memory at `index`.
+    pub fn write_to_mem_index(index: Witness, value: Witness) -> Self {
+        MemOp { operation: true, index, value }
     }
 }

acvm-repo/acir/src/parser/mod.rs

@@ -673,16 +673,14 @@ impl<'a> Parser<'a> {
         self.eat_keyword_or_error(Keyword::MemoryRead)?;
 
         // value = blockId[index]
-        let value = self.parse_arithmetic_expression()?;
+        let value = self.eat_witness_or_error()?;
         self.eat_or_error(Token::Equal)?;
         let block_id = self.eat_block_id_or_error()?;
         self.eat_or_error(Token::LeftBracket)?;
-        let index = self.parse_arithmetic_expression()?;
+        let index = self.eat_witness_or_error()?;
         self.eat_or_error(Token::RightBracket)?;
 
-        let operation = Expression::zero();
-
-        Ok(Opcode::MemoryOp { block_id, op: MemOp { index, value, operation } })
+        Ok(Opcode::MemoryOp { block_id, op: MemOp { index, value, operation: false } })
     }
 
     fn parse_memory_write(&mut self) -> ParseResult<Opcode<FieldElement>> {
@@ -691,14 +689,12 @@ impl<'a> Parser<'a> {
         // blockId[index] = value
         let block_id = self.eat_block_id_or_error()?;
         self.eat_or_error(Token::LeftBracket)?;
-        let index = self.parse_arithmetic_expression()?;
+        let index = self.eat_witness_or_error()?;
         self.eat_or_error(Token::RightBracket)?;
         self.eat_or_error(Token::Equal)?;
-        let value = self.parse_arithmetic_expression()?;
-
-        let operation = Expression::one();
+        let value = self.eat_witness_or_error()?;
 
-        Ok(Opcode::MemoryOp { block_id, op: MemOp { index, value, operation } })
+        Ok(Opcode::MemoryOp { block_id, op: MemOp { index, value, operation: true } })
     }
 
     fn parse_brillig_call(&mut self) -> ParseResult<Opcode<FieldElement>> {

acvm-repo/acir/tests/test_program_serialization.rs

@@ -269,8 +269,9 @@ fn memory_op_circuit() {
     public parameters: []
     return values: [w4]
     INIT b0 = [w1, w2]
-    WRITE b0[1] = w3
-    READ w4 = b0[1]
+    ASSERT w5 = 1
+    WRITE b0[w5] = w3
+    READ w4 = b0[w5]
     ";
     let mut circuit = Circuit::from_str(src).unwrap();
     circuit.current_witness_index = 5;
@@ -279,10 +280,10 @@ fn memory_op_circuit() {
 
     let bytes_msgpack =
         Program::serialize_program_with_format(&program, SerializationFormat::Msgpack);
-    insta::assert_compact_debug_snapshot!(bytes_msgpack, @"[31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 205, 147, 75, 78, 195, 48, 16, 134, 147, 180, 220, 131, 235, 176, 64, 28, 97, 228, 56, 3, 178, 136, 31, 204, 216, 133, 46, 219, 44, 216, 38, 237, 5, 16, 207, 166, 82, 133, 0, 33, 46, 192, 193, 48, 137, 120, 172, 104, 23, 168, 170, 55, 182, 199, 163, 255, 255, 52, 250, 157, 77, 219, 227, 96, 164, 87, 214, 112, 115, 185, 250, 58, 131, 17, 26, 175, 94, 101, 32, 66, 227, 225, 92, 121, 131, 204, 160, 76, 129, 23, 123, 15, 214, 73, 91, 32, 207, 39, 203, 67, 212, 150, 198, 7, 70, 249, 106, 145, 151, 86, 158, 130, 42, 146, 91, 21, 239, 179, 52, 91, 246, 21, 63, 118, 120, 223, 55, 78, 22, 253, 126, 228, 166, 63, 237, 215, 214, 85, 173, 117, 72, 226, 211, 185, 106, 117, 40, 193, 35, 105, 174, 95, 74, 101, 80, 16, 72, 171, 115, 101, 186, 103, 174, 111, 206, 64, 190, 239, 39, 127, 175, 244, 174, 67, 253, 47, 177, 145, 40, 3, 174, 19, 107, 102, 235, 149, 6, 27, 25, 38, 91, 155, 83, 178, 163, 115, 26, 110, 70, 255, 236, 72, 141, 132, 71, 112, 130, 98, 96, 163, 39, 207, 211, 108, 240, 228, 66, 94, 42, 249, 171, 90, 175, 8, 125, 32, 3, 29, 33, 55, 195, 71, 193, 140, 228, 65, 199, 88, 139, 19, 228, 250, 45, 38, 63, 194, 121, 18, 145, 184, 128, 239, 79, 81, 127, 0, 205, 96, 137, 59, 34, 3, 0, 0]");
+    insta::assert_compact_debug_snapshot!(bytes_msgpack, @"[31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 141, 145, 59, 78, 195, 64, 16, 134, 19, 39, 185, 71, 74, 232, 64, 156, 0, 33, 36, 40, 120, 148, 8, 9, 173, 214, 235, 1, 173, 176, 119, 151, 217, 117, 192, 165, 227, 130, 214, 78, 56, 64, 4, 4, 98, 75, 17, 2, 196, 171, 133, 92, 36, 29, 37, 13, 61, 43, 91, 188, 26, 196, 84, 51, 163, 249, 53, 51, 223, 239, 116, 243, 221, 80, 48, 195, 165, 208, 217, 241, 248, 51, 39, 130, 6, 48, 184, 99, 33, 34, 8, 67, 14, 185, 17, 160, 53, 225, 194, 131, 163, 214, 165, 84, 76, 122, 160, 79, 226, 98, 13, 2, 137, 209, 170, 224, 38, 25, 185, 190, 100, 251, 132, 123, 181, 115, 110, 235, 94, 221, 41, 170, 142, 137, 20, 92, 84, 131, 113, 177, 168, 53, 160, 217, 6, 148, 73, 30, 132, 62, 49, 128, 129, 78, 111, 125, 46, 128, 34, 97, 50, 112, 185, 160, 213, 53, 189, 73, 187, 246, 119, 212, 91, 103, 7, 132, 77, 218, 115, 222, 58, 78, 231, 7, 179, 143, 155, 203, 247, 113, 188, 181, 51, 179, 240, 186, 18, 61, 169, 108, 105, 250, 222, 127, 179, 115, 241, 168, 90, 191, 161, 186, 223, 87, 158, 74, 149, 228, 82, 1, 150, 235, 94, 134, 213, 111, 195, 14, 245, 67, 104, 252, 67, 241, 252, 75, 209, 188, 81, 200, 59, 212, 0, 81, 20, 45, 59, 251, 151, 238, 215, 157, 198, 181, 10, 93, 159, 179, 31, 221, 116, 140, 96, 66, 20, 164, 212, 233, 172, 121, 69, 75, 38, 36, 176, 132, 233, 30, 232, 244, 193, 154, 96, 1, 24, 164, 150, 138, 71, 190, 252, 73, 63, 0, 160, 171, 184, 233, 173, 1, 0, 0]");
 
     let bytes_default = Program::serialize_program(&program);
-    insta::assert_compact_debug_snapshot!(bytes_default, @"[31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 142, 187, 9, 128, 64, 16, 68, 119, 111, 207, 62, 108, 199, 64, 172, 194, 192, 192, 15, 98, 98, 120, 29, 236, 39, 48, 53, 18, 177, 14, 11, 51, 184, 220, 59, 193, 73, 134, 129, 7, 243, 72, 101, 219, 11, 11, 87, 221, 246, 227, 188, 86, 67, 183, 24, 40, 186, 35, 238, 112, 198, 110, 38, 5, 51, 230, 187, 132, 247, 96, 38, 36, 154, 166, 40, 137, 192, 103, 63, 248, 209, 207, 103, 188, 161, 35, 22, 207, 252, 0, 167, 131, 176, 229, 104, 1, 0, 0]");
+    insta::assert_compact_debug_snapshot!(bytes_default, @"[31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 125, 140, 61, 14, 130, 48, 24, 64, 91, 74, 239, 193, 168, 155, 198, 19, 24, 99, 162, 131, 63, 163, 49, 113, 179, 131, 131, 66, 10, 11, 227, 119, 131, 182, 159, 137, 43, 147, 33, 28, 128, 192, 10, 92, 132, 141, 145, 133, 29, 18, 38, 22, 222, 246, 146, 151, 199, 140, 254, 69, 252, 11, 201, 73, 188, 93, 25, 30, 63, 175, 0, 137, 161, 214, 127, 116, 72, 182, 190, 47, 100, 112, 23, 210, 69, 165, 77, 229, 144, 121, 40, 175, 156, 213, 243, 44, 235, 117, 180, 204, 174, 251, 20, 224, 246, 88, 108, 154, 67, 152, 123, 122, 87, 119, 216, 14, 13, 196, 227, 252, 226, 25, 130, 37, 103, 19, 47, 184, 141, 212, 98, 74, 219, 74, 245, 79, 150, 21, 108, 157, 0, 0, 0]");
 
     assert_deserialization(&program, [bytes_msgpack, bytes_default]);
 }

acvm-repo/acvm/src/compiler/optimizers/common_subexpression/merge_expressions.rs

@@ -205,10 +205,7 @@ impl<F: AcirField> MergeExpressionsOptimizer<F> {
                 }
                 witnesses
             }
-            Opcode::MemoryOp { block_id: _, op } => CircuitSimulator::expr_witness(&op.operation)
-                .chain(CircuitSimulator::expr_witness(&op.index))
-                .chain(CircuitSimulator::expr_witness(&op.value))
-                .collect(),
+            Opcode::MemoryOp { block_id: _, op } => BTreeSet::from([op.index, op.value]),
 
             Opcode::MemoryInit { block_id: _, init, block_type: _ } => {
                 init.iter().cloned().collect()

acvm-repo/acvm/src/compiler/optimizers/common_subexpression/mod.rs

@@ -187,13 +187,7 @@ fn transform_internal_once<F: AcirField>(
                 transformed_opcodes.push(opcode);
             }
             Opcode::MemoryOp { ref op, .. } => {
-                for (_, witness1, witness2) in &op.value.mul_terms {
-                    transformer.mark_solvable(*witness1);
-                    transformer.mark_solvable(*witness2);
-                }
-                for (_, witness) in &op.value.linear_combinations {
-                    transformer.mark_solvable(*witness);
-                }
+                transformer.mark_solvable(op.value);
                 new_acir_opcode_positions.push(acir_opcode_positions[index]);
                 transformed_opcodes.push(opcode);
             }
@@ -316,10 +310,9 @@ where
             }
             Opcode::BlackBoxFuncCall(call) => self.fold_blackbox(call),
             Opcode::MemoryOp { block_id: _, op } => {
-                let MemOp { operation, index, value } = op;
-                self.fold_expr(operation);
-                self.fold_expr(index);
-                self.fold_expr(value);
+                let MemOp { operation: _, index, value } = op;
+                self.fold(*index);
+                self.fold(*value);
             }
             Opcode::MemoryInit { block_id: _, init, block_type: _ } => {
                 for witness in init {

acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs

@@ -167,17 +167,13 @@ impl<'a, F: AcirField> RangeOptimizer<'a, F> {
                     None
                 }
 
-                Opcode::MemoryOp { block_id, op: MemOp { index, .. }, .. } => {
-                    index.to_witness().map(|witness| {
-                        (
-                            witness,
-                            *memory_block_lengths_bit_size
-                                .get(block_id)
-                                .expect("memory must be initialized before any reads/writes"),
-                            true,
-                        )
-                    })
-                }
+                Opcode::MemoryOp { block_id, op: MemOp { index, .. }, .. } => Some((
+                    *index,
+                    *memory_block_lengths_bit_size
+                        .get(block_id)
+                        .expect("memory must be initialized before any reads/writes"),
+                    true,
+                )),
                 Opcode::BlackBoxFuncCall(BlackBoxFuncCall::MultiScalarMul {
                     scalars,
                     predicate,

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

@@ -95,17 +95,14 @@ impl CircuitSimulator {
                     // Memory must be initialized before it can be used.
                     return false;
                 }
-                if !self.can_solve_expression(&op.index) {
+                if !self.solvable_witnesses.contains(&op.index) {
                     return false;
                 }
-                if op.operation.is_zero() {
-                    let Some(w) = op.value.to_witness() else {
-                        return false;
-                    };
-                    self.mark_solvable(w);
+                if op.operation {
+                    self.mark_solvable(op.value);
                     true
                 } else {
-                    self.can_solve_expression(&op.value)
+                    self.solvable_witnesses.contains(&op.value)
                 }
             }
             Opcode::MemoryInit { block_id, init, .. } => {

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

@@ -393,21 +393,19 @@ pub fn validate_witness<F: AcirField>(
 impl<F: AcirField> MemoryOpSolver<F> {
     pub(crate) fn check_memory_op(
         &mut self,
-        op: &MemOp<F>,
+        op: &MemOp,
         witness_map: &WitnessMap<F>,
         opcode_index: usize,
     ) -> Result<(), OpcodeResolutionError<F>> {
-        let operation = get_value(&op.operation, witness_map)?;
-
         // Find the memory index associated with this memory operation.
-        let index = get_value(&op.index, witness_map)?;
+        let index = get_value(&op.index.into(), witness_map)?;
         let memory_index = self.index_from_field(index)?;
 
         // Calculate the value associated with this memory operation.
-        let value = get_value(&op.value, witness_map)?;
+        let value = get_value(&op.value.into(), witness_map)?;
 
         // `operation == 0` for read operation, `operation == 1` for write operation.
-        let is_read_operation = operation.is_zero();
+        let is_read_operation = !op.operation;
 
         if is_read_operation {
             // `value = arr[memory_index]`
@@ -761,13 +759,11 @@ mod tests {
                 block_type: acir::circuit::opcodes::BlockType::Memory,
             },
             // Read from index 0 into witness 3
-            Opcode::MemoryOp {
-                block_id,
-                op: MemOp::read_at_mem_index(FieldElement::zero().into(), Witness(3)),
-            },
+            Opcode::MemoryOp { block_id, op: MemOp::read_at_mem_index(Witness(0), Witness(3)) },
         ]);
 
         let witness_map = WitnessMap::from(BTreeMap::from_iter([
+            (Witness(0), FieldElement::from(0u128)),
             (Witness(1), FieldElement::from(42u128)),
             (Witness(2), FieldElement::from(43u128)),
             (Witness(3), FieldElement::from(42u128)), // Should match value at index 0
@@ -789,13 +785,11 @@ mod tests {
                 init: vec![Witness(1), Witness(2)],
                 block_type: acir::circuit::opcodes::BlockType::Memory,
             },
-            Opcode::MemoryOp {
-                block_id,
-                op: MemOp::read_at_mem_index(FieldElement::zero().into(), Witness(3)),
-            },
+            Opcode::MemoryOp { block_id, op: MemOp::read_at_mem_index(Witness(0), Witness(3)) },
         ]);
 
         let witness_map = WitnessMap::from(BTreeMap::from_iter([
+            (Witness(0), FieldElement::from(0u128)),
             (Witness(1), FieldElement::from(42u128)),
             (Witness(2), FieldElement::from(43u128)),
             (Witness(3), FieldElement::from(99u128)), // Wrong! Should be 42
@@ -819,18 +813,13 @@ mod tests {
                 block_type: acir::circuit::opcodes::BlockType::Memory,
             },
             // Write value from witness 3 to index 0
-            Opcode::MemoryOp {
-                block_id,
-                op: MemOp::write_to_mem_index(FieldElement::zero().into(), Witness(3).into()),
-            },
+            Opcode::MemoryOp { block_id, op: MemOp::write_to_mem_index(Witness(0), Witness(3)) },
             // Read from index 0 into witness 4
-            Opcode::MemoryOp {
-                block_id,
-                op: MemOp::read_at_mem_index(FieldElement::zero().into(), Witness(4)),
-            },
+            Opcode::MemoryOp { block_id, op: MemOp::read_at_mem_index(Witness(0), Witness(4)) },
         ]);
 
         let witness_map = WitnessMap::from(BTreeMap::from_iter([
+            (Witness(0), FieldElement::from(0u128)),
             (Witness(1), FieldElement::from(42u128)), // Initial value at index 0
             (Witness(2), FieldElement::from(43u128)), // Initial value at index 1
             (Witness(3), FieldElement::from(100u128)), // Value to write