DAS Branch: Improve Documentation and Variable Naming #441
Conversation
src/c_kzg_4844.c
Outdated
| if (ret != C_KZG_OK) goto out; | ||
|
|
||
| /////////////////////////////////////////////////////////////////////////// | ||
| // STEP 2: Compute the vanishing polynomial V(X) (as a commitment in G_2). |
There was a problem hiding this comment.
Let's use Z(x) for the vanishing since that's what the specs are using.
src/c_kzg_4844.c
Outdated
| blst_fr_eucl_inverse(&h_pow, &inv_h_pow); | ||
| g2_mul(&h_pow_g2, blst_p2_generator(), &h_pow); | ||
|
|
||
| /* Compute [s^n - h^n] in G_2 */ | ||
| g2_sub(&s_pow_minus_h_pow, &s->g2_values_monomial[n], &h_pow_g2); | ||
| /* Compute [V(tau)] = [tau^n - h^n] in G_2 */ |
There was a problem hiding this comment.
Slight preference for s over tau because that's what many Ethereum-related documents have been using. It also looks a bit better in code docs because it's a single letter. No strong opinion.
There was a problem hiding this comment.
I did not want to use s because the KZGSettings are called s everywhere. But I can call it s if you think it makes sense. What do you think?
There was a problem hiding this comment.
Reasonable point. Let's keep it as tau. We can change it later if we want.
src/c_kzg_4844.c
Outdated
| @@ -3788,15 +3810,19 @@ C_KZG_RET verify_cell_kzg_proof( | |||
| if (ret != C_KZG_OK) goto out; | |||
| } | |||
|
|
|||
| /* Calculate the input value */ | |||
| /* Calculate the value x that identifies the coset | |||
There was a problem hiding this comment.
Seems like you are wrapping the lines a bit too early here? I don't remember if we have precise rules, but we can certainly go up to 80 columns.
This PR aims to improve readability and documentation of the code in the
dasbranch (this one).The following has been done:
verify_cell_kzg_proof_batch: A comment was wrong (powers ofrstart atr^0).verify_cell_kzg_proof: Comments and function documentation improvedverify_kzg_proof_multi_impl: Comments, documentation, variable names improved + minor code reordering