no outputs for mmcs

sai-deng · sai-deng · commit 20f8c2d77ceb · 2025-10-23T09:43:17.000-07:00
diff --git a/circuit/src/builder/compiler/non_primitive_lowerer.rs b/circuit/src/builder/compiler/non_primitive_lowerer.rs
@@ -90,24 +90,24 @@ impl<'a> NonPrimitiveLowerer<'a> {
                         "MmcsVerify index input",
                     )?;
 
-                    // Map root outputs
+                    // Map root inputs (assert op; no outputs)
                     let root_widx: Vec<WitnessId> = (ext + 1..expected)
                         .map(|i| {
                             get_witness_id(
                                 self.expr_to_widx,
                                 witness_exprs[i],
-                                "MmcsVerify root output",
+                                "MmcsVerify root input",
                             )
                         })
                         .collect::<Result<_, _>>()?;
 
-                    // Build Op with executor
+                    // Build Op with executor: all in inputs; no outputs
                     let mut inputs = leaf_widx.clone();
                     inputs.push(index_widx);
-                    let outputs = root_widx.clone();
+                    inputs.extend(root_widx);
                     lowered_ops.push(Op::NonPrimitiveOpWithExecutor {
                         inputs,
-                        outputs,
+                        outputs: Vec::new(),
                         executor: alloc::boxed::Box::new(MmcsVerifyExecutor::new()),
                         op_id: *op_id,
                     });
@@ -239,8 +239,8 @@ mod tests {
                 ..
             } => {
                 assert_eq!(executor.op_type(), &NonPrimitiveOpType::MmcsVerify);
-                assert_eq!(inputs, &vec![WitnessId(0), WitnessId(1)]);
-                assert_eq!(outputs, &vec![WitnessId(2)]);
+                assert_eq!(inputs, &vec![WitnessId(0), WitnessId(1), WitnessId(2)]);
+                assert!(outputs.is_empty());
             }
             _ => panic!("Expected NonPrimitiveOpWithExecutor(MmcsVerify)"),
         }
@@ -281,17 +281,16 @@ mod tests {
                 ..
             } => {
                 assert_eq!(executor.op_type(), &NonPrimitiveOpType::MmcsVerify);
-                // Verify leaf witnesses (0..8) + index (8)
-                assert_eq!(inputs.len(), 9);
+                // Verify leaf witnesses (0..8) + index (8) + root (9..16)
+                assert_eq!(inputs.len(), 17);
                 for (i, &wid) in inputs.iter().enumerate().take(8) {
                     assert_eq!(wid, WitnessId(i as u32));
                 }
                 assert_eq!(inputs[8], WitnessId(8));
-                // Verify root witnesses (9..16) as outputs
-                assert_eq!(outputs.len(), 8);
-                for (i, &wid) in outputs.iter().enumerate() {
-                    assert_eq!(wid, WitnessId((9 + i) as u32));
+                for (i, &wid) in inputs.iter().enumerate().skip(9) {
+                    assert_eq!(wid, WitnessId(i as u32));
                 }
+                assert!(outputs.is_empty());
             }
             _ => panic!("Expected NonPrimitiveOpWithExecutor(MmcsVerify)"),
         }
@@ -341,11 +340,11 @@ mod tests {
                 } => {
                     assert_eq!(executor.op_type(), &NonPrimitiveOpType::MmcsVerify);
                     let base = (i * 3) as u32;
-                    assert_eq!(inputs.len(), 2); // leaf (1) + index (1)
+                    assert_eq!(inputs.len(), 3); // leaf (1) + index (1) + root(1)
                     assert_eq!(inputs[0], WitnessId(base));
                     assert_eq!(inputs[1], WitnessId(base + 1));
-                    assert_eq!(outputs.len(), 1);
-                    assert_eq!(outputs[0], WitnessId(base + 2));
+                    assert_eq!(inputs[2], WitnessId(base + 2));
+                    assert!(outputs.is_empty());
                     assert_eq!(*op_id, NonPrimitiveOpId(i as u32));
                 }
                 _ => panic!("Expected NonPrimitiveOpWithExecutor(MmcsVerify)"),
diff --git a/circuit/src/ops/mmcs.rs b/circuit/src/ops/mmcs.rs
@@ -207,13 +207,7 @@ pub trait MmcsOps<F> {
         root_expr: &[ExprId],
     ) -> Result<NonPrimitiveOpId, CircuitBuilderError>;
 
-    /// Allocate root outputs as witness hints and add MMCS verify that will set-or-verify them.
-    fn add_mmcs_verify_allocating_root(
-        &mut self,
-        leaf_expr: &[ExprId],
-        index_expr: &ExprId,
-        root_len: usize,
-    ) -> Result<(NonPrimitiveOpId, Vec<ExprId>), CircuitBuilderError>;
+    // Deprecated: MMCS is an assert op and does not write outputs.
 }
 
 impl<F> MmcsOps<F> for CircuitBuilder<F>
@@ -237,27 +231,7 @@ where
         Ok(self.push_non_primitive_op(NonPrimitiveOpType::MmcsVerify, inputs, "mmcs_verify"))
     }
 
-    fn add_mmcs_verify_allocating_root(
-        &mut self,
-        leaf_expr: &[ExprId],
-        index_expr: &ExprId,
-        root_len: usize,
-    ) -> Result<(NonPrimitiveOpId, Vec<ExprId>), CircuitBuilderError> {
-        self.ensure_op_enabled(NonPrimitiveOpType::MmcsVerify)?;
-
-        let root_expr: Vec<ExprId> = self.alloc_witness_hints(root_len, "mmcs_root");
-
-        // Push op with inputs including root exprs (they are outputs conceptually, but part of op spec)
-        let mut inputs = vec![];
-        inputs.extend(leaf_expr);
-        inputs.push(*index_expr);
-        inputs.extend(&root_expr);
-
-        let op_id =
-            self.push_non_primitive_op(NonPrimitiveOpType::MmcsVerify, inputs, "mmcs_verify");
-
-        Ok((op_id, root_expr))
-    }
+    // Deprecated: MMCS is an assert op and does not write outputs.
 }
 
 /// Executor for MMCS verification operations
@@ -288,7 +262,7 @@ impl<F: Field> NonPrimitiveExecutor<F> for MmcsVerifyExecutor {
     fn execute(
         &self,
         inputs: &[WitnessId],
-        outputs: &[WitnessId],
+        _outputs: &[WitnessId],
         ctx: &mut ExecutionContext<F>,
     ) -> Result<(), CircuitError> {
         // Get the configuration
@@ -304,30 +278,21 @@ impl<F: Field> NonPrimitiveExecutor<F> for MmcsVerifyExecutor {
         // Get private data
         let NonPrimitiveOpPrivateData::MmcsVerify(private_data) = ctx.get_private_data()?;
 
-        // Validate input size matches configuration
-        let expected_input_size = config.input_size();
-        if inputs.len() != expected_input_size {
+        // Validate input size: leaf(ext) + index(1) + root(ext)
+        let ext_digest_elems = config.ext_field_digest_elems;
+        let min_inputs = ext_digest_elems + 1 + ext_digest_elems;
+        if inputs.len() < min_inputs {
             return Err(CircuitError::IncorrectNonPrimitiveOpPrivateDataSize {
                 op: self.op_type.clone(),
-                expected: expected_input_size,
+                expected: min_inputs,
                 got: inputs.len(),
             });
         }
 
-        // Extract leaf, index from inputs; root from outputs
-        let ext_digest_elems = config.ext_field_digest_elems;
+        // Extract leaf, index, and root from inputs
         let leaf_wids = &inputs[..ext_digest_elems];
         let index_wid = inputs[ext_digest_elems];
-
-        // Root comes from outputs (the value we're verifying against)
-        if outputs.len() < ext_digest_elems {
-            return Err(CircuitError::IncorrectNonPrimitiveOpPrivateDataSize {
-                op: self.op_type.clone(),
-                expected: ext_digest_elems,
-                got: outputs.len(),
-            });
-        }
-        let root_wids = &outputs[..ext_digest_elems];
+        let root_wids = &inputs[ext_digest_elems + 1..ext_digest_elems + 1 + ext_digest_elems];
 
         // Validate leaf values match private data
         let witness_leaf: Vec<F> = leaf_wids
@@ -365,7 +330,7 @@ impl<F: Field> NonPrimitiveExecutor<F> for MmcsVerifyExecutor {
             });
         }
 
-        // Set-or-verify for roots: if unset, set; if set, verify equality.
+        // Verify roots match exactly (assert op; do not write)
         let private_data_root = private_data
             .path_states
             .last()
@@ -384,20 +349,14 @@ impl<F: Field> NonPrimitiveExecutor<F> for MmcsVerifyExecutor {
         }
 
         for (i, &wid) in root_wids.iter().enumerate() {
-            match ctx.get_witness(wid) {
-                Ok(existing) => {
-                    if existing != private_data_root[i] {
-                        return Err(CircuitError::IncorrectNonPrimitiveOpPrivateData {
-                            op: self.op_type.clone(),
-                            operation_index: ctx.operation_id(),
-                            expected: alloc::format!("root: {private_data_root:?}"),
-                            got: alloc::format!("root: existing witness value at {wid:?}"),
-                        });
-                    }
-                }
-                Err(_) => {
-                    ctx.set_witness(wid, private_data_root[i])?;
-                }
+            let existing = ctx.get_witness(wid)?;
+            if existing != private_data_root[i] {
+                return Err(CircuitError::IncorrectNonPrimitiveOpPrivateData {
+                    op: self.op_type.clone(),
+                    operation_index: ctx.operation_id(),
+                    expected: alloc::format!("root: {:?}", private_data_root),
+                    got: alloc::format!("root witness at {:?}", wid),
+                });
             }
         }
 
diff --git a/circuit/src/tables/mmcs.rs b/circuit/src/tables/mmcs.rs
@@ -275,7 +275,7 @@ impl<'a, F: CircuitField> MmcsTraceBuilder<'a, F> {
         for op in &self.circuit.non_primitive_ops {
             let Op::NonPrimitiveOpWithExecutor {
                 inputs,
-                outputs,
+                _outputs,
                 executor,
                 op_id,
             } = op
@@ -288,7 +288,7 @@ impl<'a, F: CircuitField> MmcsTraceBuilder<'a, F> {
 
             let ext = config.ext_field_digest_elems;
             let leaf = &inputs[..ext];
-            let root = &outputs[..ext];
+            let root = &inputs[ext + 1..ext + 1 + ext];
 
             let private_data = self
                 .non_primitive_op_private_data
