Documentation and tidy up

src/blst_util.c

@@ -14,30 +14,66 @@
  * limitations under the License.
  */
 
-/** @file blst_util.c */
+/**
+ * @file blst_util.c
+ *
+ * Useful utilities for dealing with field points and group elements that are not directly exposed by the Blst library.
+ */
 
 #include "blst_util.h"
 #include "debug_util.h"
 
-// TODO - find a better way to do this
+/**
+ * Check whether the operand is zero in the finite field.
+ *
+ * @param p The field element to be checked
+ * @retval true The element is zero
+ * @retval false The element is non-zero
+ *
+ * @todo See if there is a more efficient way to check for zero in the finite field.
+ */
 bool fr_is_zero(const blst_fr *p) {
     uint64_t a[4];
     blst_uint64_from_fr(a, p);
     return a[0] == 0 && a[1] == 0 && a[2] == 0 && a[3] == 0;
 }
 
-// TODO - find a better way to do this
+/**
+ * Check whether the operand is one in the finite field.
+ *
+ * @param p The field element to be checked
+ * @retval true The element is one
+ * @retval false The element is not one
+ *
+ * @todo See if there is a more efficient way to check for one in the finite field.
+ */
 bool fr_is_one(const blst_fr *p) {
     uint64_t a[4];
     blst_uint64_from_fr(a, p);
     return a[0] == 1 && a[1] == 0 && a[2] == 0 && a[3] == 0;
 }
 
-void fr_from_uint64(blst_fr *a, const uint64_t n) {
+/**
+ * Create a field element from a single 64-bit unsigned integer.
+ *
+ * @remark This can only generate a tiny fraction of possible field elements, and is mostly useful for testing.
+ *
+ * @param out The field element equivalent of @p n
+ * @param n   The 64-bit integer to be converted
+ */
+void fr_from_uint64(blst_fr *out, uint64_t n) {
     uint64_t vals[] = {n, 0, 0, 0};
-    blst_fr_from_uint64(a, vals);
+    blst_fr_from_uint64(out, vals);
 }
 
+/**
+ * Test whether two field elements are equal.
+ *
+ * @param[in] aa The first element
+ * @param[in] bb The second element
+ * @retval true if @p aa and @p bb are equal
+ * @retval false otherwise
+ */
 bool fr_equal(const blst_fr *aa, const blst_fr *bb) {
     uint64_t a[4], b[4];
     blst_uint64_from_fr(a, aa);
@@ -45,10 +81,27 @@ bool fr_equal(const blst_fr *aa, const blst_fr *bb) {
     return a[0] == b[0] && a[1] == b[1] && a[2] == b[2] && a[3] == b[3];
 }
 
+/**
+ * Negate a field element.
+ *
+ * @param[out] out The negation of @p in
+ * @param[in]  in  The element to be negated
+ */
 void fr_negate(blst_fr *out, const blst_fr *in) {
     blst_fr_cneg(out, in, true);
 }
 
+/**
+ * Exponentiation of a field element.
+ *
+ * Uses square and multiply for log(@p n) performance.
+ *
+ * @remark A 64-bit exponent is sufficient for our needs here.
+ *
+ * @param[out] out @p a raised to the power of @p n
+ * @param[in]  a   The field element to be exponentiated
+ * @param[in]  n   The exponent
+ */
 void fr_pow(blst_fr *out, const blst_fr *a, uint64_t n) {
     blst_fr tmp = *a;
     *out = fr_one;
@@ -62,36 +115,84 @@ void fr_pow(blst_fr *out, const blst_fr *a, uint64_t n) {
     }
 }
 
+/**
+ * Division of two field elements.
+ *
+ * @param[out] out @p a divided by @p b in the field
+ * @param[in]  a   The dividend
+ * @param[in]  b   The divisor
+ */
 void fr_div(blst_fr *out, const blst_fr *a, const blst_fr *b) {
     blst_fr_eucl_inverse(out, b);
     blst_fr_mul(out, out, a);
 }
 
+/**
+ * Multiply a G1 group element by a field element.
+ *
+ * @param[out] out [@p b]@p a
+ * @param[in]  a   The G1 group element
+ * @param[in]  b   The multiplier
+ */
 void p1_mul(blst_p1 *out, const blst_p1 *a, const blst_fr *b) {
     blst_scalar s;
     blst_scalar_from_fr(&s, b);
     blst_p1_mult(out, a, s.b, 8 * sizeof(blst_scalar));
 }
 
+/**
+ * Subtraction of G1 group elements.
+ *
+ * @param[out] out @p a - @p b
+ * @param[in]  a   A G1 group element
+ * @param[in]  b   The G1 group element to be subtracted
+ */
 void p1_sub(blst_p1 *out, const blst_p1 *a, const blst_p1 *b) {
     blst_p1 bneg = *b;
     blst_p1_cneg(&bneg, true);
     blst_p1_add_or_double(out, a, &bneg);
 }
 
+/**
+ * Multiply a G2 group element by a field element.
+ *
+ * @param[out] out [@p b]@p a
+ * @param[in]  a   The G2 group element
+ * @param[in]  b   The multiplier
+ */
 void p2_mul(blst_p2 *out, const blst_p2 *a, const blst_fr *b) {
     blst_scalar s;
     blst_scalar_from_fr(&s, b);
     blst_p2_mult(out, a, s.b, 8 * sizeof(blst_scalar));
 }
 
+/**
+ * Subtraction of G2 group elements.
+ *
+ * @param[out] out @p a - @p b
+ * @param[in]  a   A G2 group element
+ * @param[in]  b   The G2 group element to be subtracted
+ */
 void p2_sub(blst_p2 *out, const blst_p2 *a, const blst_p2 *b) {
     blst_p2 bneg = *b;
     blst_p2_cneg(&bneg, true);
     blst_p2_add_or_double(out, a, &bneg);
 }
 
-// TODO: would be good to have an optimised multiexp for this
+/**
+ * Calculate a linear combination of G1 group elements.
+ *
+ * Calculates `[coeffs_0]p_0 + [coeffs_1]p_1 + ... + [coeffs_n]p_n` where `n` is `len - 1`.
+ *
+ * @param[out] out    The resulting sum-product
+ * @param[in]  p      Array of G1 group elements, length @p len
+ * @param[in]  coeffs Array of field elements, length @p len
+ * @param[in]  len    The number of group/field elements
+ *
+ * @todo This could be substantially improved with an optimised multi-scalar multiplication. (1) Benchmark and see if
+ * this is a bottleneck. (2) If so, look into optimised routines. [Notes from
+ * Mamy](https://github.com/vacp2p/research/issues/7#issuecomment-690083000) on the topic.
+ */
 void linear_combination_g1(blst_p1 *out, const blst_p1 *p, const blst_fr *coeffs, const uint64_t len) {
     blst_p1 tmp;
     blst_p1_from_affine(out, &identity_g1_affine);
@@ -101,23 +202,35 @@ void linear_combination_g1(blst_p1 *out, const blst_p1 *p, const blst_fr *coeffs
     }
 }
 
-bool pairings_verify(const blst_p1 *aa1, const blst_p2 *aa2, const blst_p1 *bb1, const blst_p2 *bb2) {
+/**
+ * Perform pairings and test whether the outcomes are equal in G_T.
+ *
+ * Tests whether `e(a1, a2) == e(b1, b2)`
+ *
+ * @param[in] a1 A G1 group point for the first pairing
+ * @param[in] a2 A G2 group point for the first pairing
+ * @param[in] b1 A G1 group point for the second pairing
+ * @param[in] b2 A G2 group point for the second pairing
+ * @retval true  The pairings were equal
+ * @retval false The pairings were not equal
+ */
+bool pairings_verify(const blst_p1 *a1, const blst_p2 *a2, const blst_p1 *b1, const blst_p2 *b2) {
     blst_fp12 loop0, loop1, gt_point;
-    blst_p1_affine a1, b1;
+    blst_p1_affine aa1, bb1;
-    blst_p2_affine a2, b2;
+    blst_p2_affine aa2, bb2;
 
     // As an optimisation, we want to invert one of the pairings,
     // so we negate one of the points.
-    blst_p1 a1neg = *aa1;
+    blst_p1 a1neg = *a1;
     blst_p1_cneg(&a1neg, true);
 
-    blst_p1_to_affine(&a1, &a1neg);
+    blst_p1_to_affine(&aa1, &a1neg);
-    blst_p1_to_affine(&b1, bb1);
+    blst_p1_to_affine(&bb1, b1);
-    blst_p2_to_affine(&a2, aa2);
+    blst_p2_to_affine(&aa2, a2);
-    blst_p2_to_affine(&b2, bb2);
+    blst_p2_to_affine(&bb2, b2);
 
-    blst_miller_loop(&loop0, &a2, &a1);
+    blst_miller_loop(&loop0, &aa2, &aa1);
-    blst_miller_loop(&loop1, &b2, &b1);
+    blst_miller_loop(&loop1, &bb2, &bb1);
 
     blst_fp12_mul(&gt_point, &loop0, &loop1);
     blst_final_exp(&gt_point, &gt_point);

src/blst_util.h

@@ -28,7 +28,7 @@ static const blst_p1_affine identity_g1_affine = {{0L, 0L, 0L, 0L, 0L, 0L}, {0L,
 
 bool fr_is_zero(const blst_fr *p);
 bool fr_is_one(const blst_fr *p);
-void fr_from_uint64(blst_fr *a, const uint64_t n);
+void fr_from_uint64(blst_fr *out, uint64_t n);
 bool fr_equal(const blst_fr *aa, const blst_fr *bb);
 void fr_negate(blst_fr *out, const blst_fr *in);
 void fr_pow(blst_fr *out, const blst_fr *a, const uint64_t n);

src/c_kzg.h

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-/** @file c_kzg_util.c */
+/** @file c_kzg.h */
 
 #ifndef C_KZG_H
 #define C_KZG_H
@@ -22,11 +22,20 @@
 #include <stdbool.h>
 #include "../inc/blst.h"
 
+/**
+ * The common return type for all routines in which something can go wrong.
+ *
+ * @warning In the case of @p C_KZG_OK or @p C_KZG_BADARGS, the caller can assume that all memory allocated by the
+ * called routines has been deallocated. However, in the case of @p C_KZG_ERROR or @p C_KZG_MALLOC being returned, these
+ * are unrecoverable and memory may have been leaked.
+ *
+ * @todo Check that memory is not leaked in the case of C_KZG_BADARGS.
+ */
 typedef enum {
-    C_KZG_OK = 0,  // Success!
+    C_KZG_OK = 0,  /**< Success! */
-    C_KZG_BADARGS, // The supplied data is invalid in some way
+    C_KZG_BADARGS, /**< The supplied data is invalid in some way */
-    C_KZG_ERROR,   // Internal error - should never occur
+    C_KZG_ERROR,   /**< Internal error - this should never occur and indicates a bug in the library */
-    C_KZG_MALLOC,  // Could not allocate memory
+    C_KZG_MALLOC,  /**< Could not allocate memory */
 } C_KZG_RET;
 
 #ifdef DEBUG
@@ -40,6 +49,19 @@ typedef enum {
 #else
 #define ASSERT(cond, ret)                                                                                              \
     if (!(cond)) return (ret)
-#endif
+#endif // DEBUG
 
-#endif
+/** @def ASSERT
+ *
+ * Handle errors.
+ *
+ * This macro comes in two versions according to whether `DEBUG` is defined or not (`-DDEBUG` compiler flag).
+ *   - `DEBUG` is undefined: when @p cond is false, return from the function with the value @p ret, otherwise continue.
+ *   - `DEBUG` is defined: when @p cond is false, print file and line number information and abort the run. This is very
+ * useful for dubugging. The @p ret parameter is ignored in this case.
+ *
+ * @param cond The condition to be tested
+ * @param ret  The return code to be returned in case the condition is false
+ */
+
+#endif // C_KZG_H

src/c_kzg_util.c

@@ -14,11 +14,23 @@
  * limitations under the License.
  */
 
-/** @file c_kzg_util.c */
+/**
+ *  @file c_kzg_util.c
+ *
+ * Utilities useful across the library.
+ */
 
 #include <stdlib.h> // malloc
 #include "c_kzg_util.h"
 
+/**
+ * Wrapped `malloc()` that reports failures to allocate.
+ *
+ * @param[out] p Pointer to the allocated space
+ * @param[in]  n The number of bytes to be allocated
+ * @retval C_CZK_OK      All is well
+ * @retval C_CZK_MALLOC  Memory allocation failed
+ */
 C_KZG_RET c_kzg_malloc(void **p, size_t n) {
     if (n > 0) {
         *p = malloc(n);

src/fk20_proofs_test.c

@@ -112,7 +112,10 @@ void fk_single(void) {
     bool result;
 
     TEST_CHECK(n_len >= 2 * poly_len);
-    TEST_CHECK(init_poly_with_coeffs(&p, coeffs, poly_len) == C_KZG_OK);
+    TEST_CHECK(init_poly(&p, poly_len) == C_KZG_OK);
+    for (uint64_t i = 0; i < poly_len; i++) {
+        fr_from_uint64(&p.coeffs[i], coeffs[i]);
+    }
 
     // Initialise the secrets and data structures
     generate_trusted_setup(&s1, &s2, &secret, secrets_len);
@@ -164,7 +167,10 @@ void fk_single_strided(void) {
     bool result;
 
     TEST_CHECK(n_len >= 2 * poly_len);
-    TEST_CHECK(init_poly_with_coeffs(&p, coeffs, poly_len) == C_KZG_OK);
+    TEST_CHECK(init_poly(&p, poly_len) == C_KZG_OK);
+    for (uint64_t i = 0; i < poly_len; i++) {
+        fr_from_uint64(&p.coeffs[i], coeffs[i]);
+    }
 
     // Initialise the secrets and data structures
     generate_trusted_setup(&s1, &s2, &secret, secrets_len);

src/poly.c

@@ -14,18 +14,40 @@
  * limitations under the License.
  */
 
-/** @file poly.c */
+/**
+ *  @file poly.c
+ *
+ * Operations on polynomials defined over the finite field.
+ */
 
 #include <stdlib.h> // NULL, free()
 #include "c_kzg_util.h"
 #include "poly.h"
 
+/**
+ * Internal utility for calculating the length to be allocated for the result of dividing two polynomials.
+ *
+ * @param[in] dividend The dividend polynomial
+ * @param[in] divisor The divisor polynomial
+ * @return Size of polynomial that needs to be allocated to hold the result of the division
+ */
+static uint64_t poly_quotient_length(const poly *dividend, const poly *divisor) {
+    return dividend->length >= divisor->length ? dividend->length - divisor->length + 1 : 0;
+}
+
+/**
+ * Evaluate a polynomial over the finite field at a point.
+ *
+ * @param[out] out The value of the polynomial at the point @p x
+ * @param[in]  p   The polynomial
+ * @param[in]  x   The x-coordinate to be evaluated
+ */
 void eval_poly(blst_fr *out, const poly *p, const blst_fr *x) {
     blst_fr tmp;
     uint64_t i;
 
     if (p->length == 0) {
-        fr_from_uint64(out, 0);
+        *out = fr_zero;
         return;
     }
 
@@ -45,13 +67,21 @@ void eval_poly(blst_fr *out, const poly *p, const blst_fr *x) {
     }
 }
 
-// Call this to find out how much space to allocate for the result of `poly_long_div()`
+/**
-uint64_t poly_quotient_length(const poly *dividend, const poly *divisor) {
+ * Polynomial division in the finite field.
-    return dividend->length >= divisor->length ? dividend->length - divisor->length + 1 : 0;
+ *
-}
+ * Returns the polynomial resulting from dividing @p dividend by @p divisor.
-
+ *
-// `out` must be an uninitialised poly and has space allocated for it here, which
+ * @remark @p out must be an uninitialised #poly. Space is allocated for it here, which
-// must be freed by calling `free_poly()` later.
+ * must be later reclaimed by calling #free_poly().
+ *
+ * @param[out] out      An uninitialised poly type that will contain the result of the division
+ * @param[in]  dividend The dividend polynomial
+ * @param[in]  divisor  The divisor polynomial
+ * @retval C_CZK_OK      All is well
+ * @retval C_CZK_BADARGS Invalid parameters were supplied
+ * @retval C_CZK_MALLOC  Memory allocation failed
+ */
 C_KZG_RET poly_long_div(poly *out, const poly *dividend, const poly *divisor) {
     uint64_t a_pos = dividend->length - 1;
     uint64_t b_pos = divisor->length - 1;
@@ -87,19 +117,48 @@ C_KZG_RET poly_long_div(poly *out, const poly *dividend, const poly *divisor) {
     return C_KZG_OK;
 }
 
-C_KZG_RET init_poly_with_coeffs(poly *out, const uint64_t *coeffs, const uint64_t length) {
+/**
+ * Initialise a polynomial of the given size with the given coefficients.
+ *
+ * @remark This allocates space for the polynomial coefficients that must be later reclaimed by calling #free_poly.
+ *
+ * @param[out] out    The initialised polynomial structure
+ * @param[in]  coeffs `coeffs[i]` is the coefficient of the `x^i` term of the polynomial
+ * @param[in]  length The number of coefficients, which is one more than the polynomial's degree
+ * @retval C_CZK_OK      All is well
+ * @retval C_CZK_MALLOC  Memory allocation failed
+ */
+C_KZG_RET init_poly_with_coeffs(poly *out, const blst_fr *coeffs, uint64_t length) {
     ASSERT(init_poly(out, length) == C_KZG_OK, C_KZG_MALLOC);
     for (uint64_t i = 0; i < length; i++) {
-        fr_from_uint64(&out->coeffs[i], coeffs[i]);
+        out->coeffs[i] = coeffs[i];
     }
     return C_KZG_OK;
 }
 
-C_KZG_RET init_poly(poly *out, const uint64_t length) {
+/**
+ * Initialise an empty polynomial of the given size.
+ *
+ * @remark This allocates space for the polynomial coefficients that must be later reclaimed by calling #free_poly.
+ *
+ * @param[out] out    The initialised polynomial structure
+ * @param[in]  length The number of coefficients required, which is one more than the polynomial's degree
+ * @retval C_CZK_OK      All is well
+ * @retval C_CZK_MALLOC  Memory allocation failed
+ */
+C_KZG_RET init_poly(poly *out, uint64_t length) {
     out->length = length;
     return c_kzg_malloc((void **)&out->coeffs, length * sizeof *out->coeffs);
 }
 
+/**
+ * Reclaim the memory used by a polynomial.
+ *
+ * @remark To avoid memory leaks, this must be called for polynomials initialised with #init_poly or
+ * #init_poly_with_coeffs after use.
+ *
+ * @param[in,out] p The polynomial
+ */
 void free_poly(poly *p) {
     if (p->coeffs != NULL) {
         free(p->coeffs);

src/poly.h

@@ -30,8 +30,7 @@ typedef struct {
 } poly;
 
 void eval_poly(blst_fr *out, const poly *p, const blst_fr *x);
-uint64_t poly_quotient_length(const poly *dividend, const poly *divisor);
 C_KZG_RET poly_long_div(poly *out, const poly *dividend, const poly *divisor);
-C_KZG_RET init_poly_with_coeffs(poly *out, const uint64_t *coeffs, const uint64_t length);
+C_KZG_RET init_poly_with_coeffs(poly *out, const blst_fr *coeffs, uint64_t length);
-C_KZG_RET init_poly(poly *out, const uint64_t length);
+C_KZG_RET init_poly(poly *out, uint64_t length);
 void free_poly(poly *p);

src/poly_test.c

@@ -19,18 +19,6 @@
 #include "test_util.h"
 #include "poly.h"
 
-void poly_div_length(void) {
-    poly a, b;
-    init_poly(&a, 17);
-    init_poly(&b, 5);
-    TEST_CHECK(13 == poly_quotient_length(&a, &b));
-    TEST_CHECK(1 == poly_quotient_length(&a, &a));
-    TEST_CHECK(0 == poly_quotient_length(&b, &a));
-
-    free_poly(&a);
-    free_poly(&b);
-}
-
 void poly_div_0(void) {
     blst_fr a[3], b[2], expected[2];
     poly dividend, divisor, actual;
@@ -191,7 +179,6 @@ void poly_eval_nil_check(void) {
 
 TEST_LIST = {
     {"POLY_TEST", title},
-    {"poly_div_length", poly_div_length},
     {"poly_div_0", poly_div_0},
     {"poly_div_1", poly_div_1},
     {"poly_div_2", poly_div_2},