Assessment reports>Gnark support in Universal Proof Aggregation circuits>Discussion>Confusing implementation of InCircuitHash and FieldElementRepresentation for AssignedVerificationKey

Confusing implementation of InCircuitHash and FieldElementRepresentation for AssignedVerificationKey

While AssignedVerificationKey has been changed to include new fields h1 and h2 for verification of the proof of knowledge for the Pedersen commitment, the implementations of the InCircuitHash, FieldElementRepresentation, and InCircuitPartialHash have not been updated. The way these traits are used in the Poseidon implementation and the universal batch verifier chip results in behavior consistent with the specification, as handling of hashing of h1 and h2 is done via the extra terms that var_len_poseidon_no_len_check_with_extra_terms now takes.

However, these unchanged implementations of InCircuitHash and FieldElementRepresentation for AssignedVerificationKey are locally unexpected; for example, FieldElementRepresentation is documented as follows:

/// Interface for types that can be represented as a vector of field elements
pub trait FieldElementRepresentation<F: EccPrimeField> {
    /// Returns the representation of `self` as assigned field elements.
    fn representation(&self) -> Vec<AssignedValue<F>>;

    /// Returns the total number of field elements in `self`.
    fn num_elements(&self) -> usize;
}

The comments here would suggest that representation returns a vector of field elements that completely determines self, while in fact h1 and h2 are left out.

Given the narrow usage of AssignedVerificationKey in the codebase, it appears unlikely that mistakes will arise from confusion about what these two traits ensure in the near future. For best practice, we would nevertheless recommend to document that not all parts of self might be represented or hashed and/or possibly rename the trait.

Zellic © 2025Back to top ↑