Address remaining reviews

Author: 4l0n50

circuit/src/builder/circuit_builder.rs

@@ -10,7 +10,7 @@ use super::{BuilderConfig, ExpressionBuilder};
 use crate::CircuitBuilderError;
 use crate::builder::public_input_tracker::PublicInputTracker;
 use crate::circuit::Circuit;
-use crate::op::{NonPrimitiveOpType, WitnessHintFiller};
+use crate::op::{DefaultHint, NonPrimitiveOpType, WitnessHintFiller};
 use crate::ops::MmcsVerifyConfig;
 use crate::types::{ExprId, NonPrimitiveOpId, WitnessAllocator, WitnessId};
 
@@ -140,9 +140,8 @@ where
         count: usize,
         label: &'static str,
     ) -> Vec<ExprId> {
-        (0..count)
-            .map(|_| self.expr_builder.add_witness_hint(label))
-            .collect()
+        self.expr_builder
+            .add_witness_hints(DefaultHint { n_outputs: count }, label)
     }
 
     /// Adds a constant to the circuit (deduplicated).
@@ -437,7 +436,6 @@ where
 #[cfg(test)]
 mod tests {
     use alloc::vec;
-    use alloc::vec::Vec;
 
     use p3_baby_bear::BabyBear;
     use p3_field::PrimeCharacteristicRing;
@@ -713,39 +711,12 @@ mod tests {
         assert_eq!(circuit.primitive_ops.len(), 2);
     }
 
-    #[derive(Debug, Clone)]
-    struct ConstantHint<const C: usize> {
-        inputs: Vec<ExprId>,
-    }
-
-    impl<const C: usize> ConstantHint<C> {
-        pub fn new(input: ExprId) -> Self {
-            Self {
-                inputs: vec![input],
-            }
-        }
-    }
-
-    impl<F: Field, const C: usize> WitnessHintFiller<F> for ConstantHint<C> {
-        fn inputs(&self) -> &[ExprId] {
-            &self.inputs
-        }
-
-        fn n_outputs(&self) -> usize {
-            1
-        }
-
-        fn compute_outputs(&self, _inputs_val: Vec<F>) -> Result<Vec<F>, crate::CircuitError> {
-            Ok(vec![F::from_usize(C)])
-        }
-    }
     #[test]
     fn test_build_with_witness_hint() {
         let mut builder = CircuitBuilder::<BabyBear>::new();
-        let a = builder.add_const(BabyBear::ZERO);
-        let mock_filler = ConstantHint::<1>::new(a);
-        let b = builder.alloc_witness_hints(mock_filler, "a");
-        assert_eq!(b.len(), 1);
+        let default_hint = DefaultHint { n_outputs: 1 };
+        let a = builder.alloc_witness_hints(default_hint, "a");
+        assert_eq!(a.len(), 1);
         let circuit = builder
             .build()
             .expect("Circuit with operations should build");
@@ -757,7 +728,7 @@ mod tests {
             crate::op::Op::Unconstrained {
                 inputs, outputs, ..
             } => {
-                assert_eq!(*inputs, vec![WitnessId(0)]);
+                assert_eq!(*inputs, vec![]);
                 assert_eq!(*outputs, vec![WitnessId(1)]);
             }
             _ => panic!("Expected Unconstrained at index 0"),

circuit/src/builder/compiler/expression_lowerer.rs

@@ -10,7 +10,7 @@ use crate::Op;
 use crate::builder::CircuitBuilderError;
 use crate::builder::compiler::get_witness_id;
 use crate::expr::{Expr, ExpressionGraph};
-use crate::op::{DefaultHint, WitnessHintFiller};
+use crate::op::WitnessHintFiller;
 use crate::types::{ExprId, WitnessAllocator, WitnessId};
 
 /// Sparse disjoint-set "find" with path compression over a HashMap (iterative).
@@ -187,7 +187,7 @@ where
             }
         }
 
-        // Pass C: emit arithmetic ops in creation order; tie outputs to class slot if connected
+        // Pass C: emit arithmetic and unconstrained ops in creation order; tie outputs to class slot if connected
         let mut hints_sequence = vec![];
         let mut fillers_iter = self.hints_with_fillers.iter().cloned();
         for (expr_idx, expr) in self.graph.nodes().iter().enumerate() {
@@ -200,7 +200,9 @@ where
                     expr_to_widx.insert(expr_id, out_widx);
                     hints_sequence.push(out_widx);
                     if *last_hint {
-                        let filler = fillers_iter.next().unwrap_or(DefaultHint::boxed_default());
+                        let filler = fillers_iter.next().expect(
+                            "By construction, every sequence of witness must haver one filler",
+                        );
                         let inputs = filler
                             .inputs()
                             .iter()

circuit/src/op.rs

@@ -419,25 +419,27 @@ impl<F> Clone for Box<dyn WitnessHintFiller<F>> {
 }
 
 #[derive(Debug, Clone, Default)]
-pub struct DefaultHint {}
+pub struct DefaultHint {
+    pub n_outputs: usize,
+}
 
 impl DefaultHint {
-    pub fn boxed_default<F: Default>() -> Box<dyn WitnessHintFiller<F>> {
+    pub fn boxed_default<F: Default + Clone>() -> Box<dyn WitnessHintFiller<F>> {
         Box::new(Self::default())
     }
 }
 
-impl<F: Default> WitnessHintFiller<F> for DefaultHint {
+impl<F: Default + Clone> WitnessHintFiller<F> for DefaultHint {
     fn inputs(&self) -> &[ExprId] {
         &[]
     }
 
     fn n_outputs(&self) -> usize {
-        1
+        self.n_outputs
     }
 
     fn compute_outputs(&self, _inputs_val: Vec<F>) -> Result<Vec<F>, CircuitError> {
-        Ok(vec![F::default()])
+        Ok(vec![F::default(); self.n_outputs])
     }
 }
 

circuit/src/utils.rs

@@ -4,6 +4,7 @@ use core::marker::PhantomData;
 
 use p3_field::{ExtensionField, Field, PrimeField64};
 use p3_uni_stark::{Entry, SymbolicExpression};
+use p3_util::log2_ceil_u64;
 
 use crate::op::WitnessHintFiller;
 use crate::{CircuitBuilder, CircuitError, ExprId};
@@ -167,16 +168,11 @@ impl<BF: PrimeField64, F: ExtensionField<BF>> WitnessHintFiller<F> for BinaryDec
     }
 
     fn compute_outputs(&self, inputs_val: Vec<F>) -> Result<Vec<F>, CircuitError> {
-        if inputs_val.len() != 1 {
-            return Err(CircuitError::UnconstrainedOpInputLengthMismatch {
-                expected: 1,
-                got: inputs_val.len(),
-            });
-        }
         let val: u64 = inputs_val[0].as_basis_coefficients_slice()[0].as_canonical_u64();
         let bits = (0..self.n_bits)
             .map(|i| F::from_bool(val >> i & 1 == 1))
             .collect();
+        debug_assert!(self.n_bits as u64 >= log2_ceil_u64(val));
         Ok(bits)
     }
 }