Improvements possible to make KeccakCircuit::new
more robust
In circuits/src/keccak/mod.rs, the function KeccakCircuit::new
contains the following code:
for input in inputs.0 {
let assigned_input = AssignedKeccakInput::from_keccak_padded_input(
ctx, &range, input,
);
let circuit_id = Self::compute_circuit_id(
ctx,
&range,
&mut keccak,
&assigned_input,
);
if is_fixed {
Self::compute_proof_id_fixed_length(
ctx,
&range,
&mut keccak,
&assigned_input,
);
panic!("Keccak fixed length no longer supported");
} else {
// Specification: Proof ID Computation
Self::compute_proof_id(
ctx,
&range,
&mut keccak,
&circuit_id,
&assigned_input,
);
}
// Specification: Curve-to-Field Hash
Self::commitment_point_hash_query(
ctx,
&range,
&mut keccak,
&assigned_input,
);
public_inputs.push(assigned_input);
}
// Specification: Final Digest Computation.
// Here we select only the even var_len_queries because those
// contain the proofIds. The odd ones contain the circuitIds
// which are not hashes into the final digest.
let intermediate_outputs = keccak
.var_len_queries()
.iter()
.skip(1)
.step_by(2) // we skip the circuitId computations
.flat_map(|query| query.output_bytes_assigned().to_vec())
.collect::<Vec<_>>();
The logic to extract the proofId
s towards the end of this snippet, involving skipping the first item and then skipping every second item, relies on the following:
The function
compute_circuit_id
creates exactly one var len query each.We are in the variable-length case (as the fixed-length case panics), so
compute_proof_id
is used instead ofcompute_proof_id_fixed_length
.The function
compute_proof_id
creates exactly one var len query, and the result of that query is theproofId
we would like to extract.The function
commitment_point_hash_query
does not create any var len query (only a fixed len one).
While the outlined logic is correct, it would be more robust to handle the proofId
s the same way as the circuitId
s. In the case of KeccakMultiVarHasher::finalize
, the result of the query is extracted and returned immediately,
keccak_chip.keccak_var_len(ctx, range, preimage, len);
return keccak_chip
.var_len_queries()
.last()
.expect("no queries")
.output_bytes_assigned()
.to_vec();
and the functions KeccakChip::multi_var_query
and KeccakCircuit::compute_circuit_id
forward this return value.
One could do something analogous in compute_proof_id
(that is, extract the result immediately after creating the query and returning it), and then in KeccakCircuit::new
, one could push the return value to intermediate_outputs
instead of needing to rely on the precise query order to obtain it at the end. This would make the code less likely to break and require changes should some of the called functions be changed to create more or less var len queries. It would also make it easier to see that the code is correct, not requiring to check the above four bullet points.