feat: [PLONK_AUDIT_4-8] fixes #743 #744
Conversation
There was a problem hiding this comment.
Looks good, though see comments below about proof_quotient_polynomial_at_zeta and proof_linearised_polynomial_at_zeta.
Also, I tried to work through whether any of the other components of the proof need to be checked to be < p_mod. As far as I can tell, any digressionis are caught by the precompile contracts for BN_ADD and BN_MUL which revert if points are not on the curve. So, that seems OK to me.
| @@ -607,12 +607,54 @@ contract PlonkVerifier { | |||
|
|
|||
| } | |||
|
|
|||
| function check_proof_openings_size(bytes memory proof) | |||
There was a problem hiding this comment.
Looks good, though am wondering whether you also need a check for proof_linearised_polynomial_at_zeta. My reasoning is that this is passed as a parameter into fr_acc_mul() within fold_state, and all of the others passed into fr_acc_mul within fold_state are checked.
Looking at the Gnark code (and the provided README for the audit) I see that both proof_quotient_polynomial_at_zeta and proof_linearised_polynomial_at_zeta correspond to BatchedProof.ClaimedValues[0:2] which are typed as fr.Element. That makes me think both should be checked ?
There was a problem hiding this comment.
Ah right 👍 I don't know what I didn't check them... I'm adding the checks for the those 2 values
the openings (= values) of the proof commitments are ensured to be below r_mod : fixes #743