Description

The function bigfield::assert_equal is intended to constrain this to be equal to the argument other. This function exhibits unexpected behavior if one or both of the operands are constant.

If both are constant, the implementation is as follows:

if (is_constant() && other.is_constant()) {
    std::cerr << "bigfield: calling assert equal on 2 CONSTANT bigfield elements...is this intended?"
              << std::endl;
    return;
}

Thus, in this case, an error message will be printed. However, execution will otherwise proceed. In particular, it is not checked that the two constants are actually equal. Expected behavior would be to assert that the two values are equal and thus stop execution should this assertion fail.

If exactly one of the two operands is constant, the following snippet is the relevant implementation:

else if (other.is_constant()) {
    // TODO(https://github.com/AztecProtocol/barretenberg/issues/998): Something is fishy here
    // evaluate a strict equality - make sure *this is reduced first, or an honest prover
    // might not be able to satisfy these constraints.
    field_t<Builder> t0 = (binary_basis_limbs[0].element - other.binary_basis_limbs[0].element);
    field_t<Builder> t1 = (binary_basis_limbs[1].element - other.binary_basis_limbs[1].element);
    field_t<Builder> t2 = (binary_basis_limbs[2].element - other.binary_basis_limbs[2].element);
    field_t<Builder> t3 = (binary_basis_limbs[3].element - other.binary_basis_limbs[3].element);
    field_t<Builder> t4 = (prime_basis_limb - other.prime_basis_limb);
    t0.assert_is_zero();
    t1.assert_is_zero();
    t2.assert_is_zero();
    t3.assert_is_zero();
    t4.assert_is_zero();
    return;
} else if (is_constant()) {
    other.assert_equal(*this);
    return;
}

The code constrains that the constant and nonconstant operands agree in all binary limbs and the prime limb. As the TODO comment indicates, there is a completeness issue here, as there may be multiple representations possible for bigfield elements for the same element modulo the emulated modulus $p$.

Impact

Against expectation, the function does not actually assert equality if both operands are constants, which is a soundness issue. The error message printed no matter whether the equality holds or not, however, suggests that usage of assert_equal is not intended in case both operands are constant, so such usage is to be considered user error anyway.

If exactly one of the operands is constant, then there is a completeness issue for unreduced operands. This behavior is documented in the code already, however.

Recommendations

Assert equality of the values of the operands in the both-constant case. In the case of exactly one operand being constant, document that they must be reduced when using the function, or carry out a reduction in the function prior to the checks.

Remediation

This issue has been acknowledged by Aztec, and a fix was implemented in commit b3e01f44↗.