zk-fy in circuit writer on constraint system?

[mimoo]

izaak's kombucha PR initially moved the zk rows of the witness to kombucha:

    fn zk(&mut self) {
        for _ in 0..ZK_ROWS {
            let row = array_init(|_| self.var(|| F::rand(&mut rand::thread_rng())));
            self.gate(GateSpec {
                typ: GateType::Zero,
                coeffs: vec![],
                row,
            });
        }
    }

you would call this function within your circuit, at the end. Asking the user to call it was a bit error prone imo, it would have been nicer to include by default in the compilation (when you call gates()) and perhaps have a different function (e.g. dangerous_compile_without_privacy()) that would omit it (or perhaps not even offer such a use-case).

this does simplify the rest of the codebase a lot. ConstraintSystem::create has:

        //~ 2. Create a domain for the circuit. That is,
        //~    compute the smallest subgroup of the field that
        //~    has order greater or equal to `n + ZK_ROWS` elements.
        let domain = EvaluationDomains::<F>::create(gates.len() + ZK_ROWS as usize)?;

        assert!(domain.d1.size > ZK_ROWS);

        //~ 3. Pad the circuit: add zero gates to reach the domain size.
        let d1_size = domain.d1.size();
        let mut padding = (gates.len()..d1_size)
            .map(|i| {
                CircuitGate::<F>::zero(array_init(|j| Wire {
                    col: WIRES[j],
                    row: i,
                }))
            })
            .collect();
        gates.append(&mut padding);

and ProverProof::create has:

        //~ 1. Ensure we have room in the witness for the zero-knowledge rows.
        //~    We currently expect the witness not to be of the same length as the domain,
        //~    but instead be of the length of the (smaller) circuit.
        //~    If we cannot add `ZK_ROWS` rows to the columns of the witness before reaching
        //~    the size of the domain, abort.
        let length_witness = witness[0].len();
        let length_padding = d1_size
            .checked_sub(length_witness)
            .ok_or(ProverError::NoRoomForZkInWitness)?;

        if length_padding < ZK_ROWS as usize {
            return Err(ProverError::NoRoomForZkInWitness);
        }

        //~ 1. Pad the witness columns with Zero gates to make them the same length as the domain.
        //~    Then, randomize the last `ZK_ROWS` of each columns.
        for w in &mut witness {
            if w.len() != length_witness {
                return Err(ProverError::WitnessCsInconsistent);
            }

            // padding
            w.extend(std::iter::repeat(G::ScalarField::zero()).take(length_padding));

            // zk-rows
            for row in w.iter_mut().rev().take(ZK_ROWS as usize) {
                *row = <G::ScalarField as UniformRand>::rand(rng);
            }
        }

a few notes:

• we actually still need to make room if the witness doesn't include these ZK_ROWS because other arguments like the permutation or the lookup will still use the same space for zk-knowledge
• I don't think it's a good idea to remove zk anyway
• if we have to handle zk for most arguments on the kimchi side, I think we should continue to handle it for the witness on that side as well