Assessment reports>Universal Proof Aggregator>Discussion>Documentation misaligned

Documentation is sometimes misaligned with the implementation

During the review of the codebase, we noticed some places where behavior of the code deviates from the documented behavior.

For example, the NEBRA UPA v1.1.0 Protocol Specification refers in "Circuit Statements" to the Groth16 verification with an outdated link.

In circuits/src/keccak/mod.rs, there are several functions with comments where the order in which things are appended is opposite to the implementation — for example, the function flatten:

/// Appends the vk hash to the application public inputs.
pub fn flatten(&self) -> Vec<AssignedValue<F>> {
    let mut result = vec![self.app_vk_hash];
    result.extend_from_slice(&self.app_public_inputs);
    result
}

The result begins with the vk hash, so it would be correct to document this as "Appends the application public inputs to the vk hash" or "Concatenates the vk hash and the application public inputs".

The documenting comment for the function num_field_elements in the same file appears to be incorrect as well.

/// Returns the number of field elements which will be keccak'd together.
#[cfg(test)]
pub fn num_field_elements(&self) -> usize {
    self.len.value().get_lower_32() as usize
}

Here, the return value is the number of public inputs. However, the circuit ID is also part of the field elements that the Keccak hash is taken of. Thus, the implementation does not match the specification given by the preceding comment.

The above two examples were corrected in commit

Zellic © 2024Back to top ↑