Use memcmp for field element comparisons

src/common/fr.c

@@ -19,6 +19,7 @@
 
 #include <inttypes.h> /* For uint*_t */
 #include <stdbool.h>  /* For bool */
+#include <string.h>   /* For memcmp */
 
 /**
  * Test whether two field elements are equal.
@@ -28,38 +29,11 @@
  *
  * @retval  true    The two elements are equal.
  * @retval  false   The two elements are not equal.
- */
-bool fr_equal(const fr_t *a, const fr_t *b) {
-    uint64_t _a[4], _b[4];
-    blst_uint64_from_fr(_a, a);
-    blst_uint64_from_fr(_b, b);
-    return _a[0] == _b[0] && _a[1] == _b[1] && _a[2] == _b[2] && _a[3] == _b[3];
-}
-
-/**
- * Test whether the operand is one in the finite field.
- *
- * @param[in]   p   The field element to be checked
  *
- * @retval  true    The element is one
- * @retval  false   The element is not one
+ * @remark The field elements are assumed to be in reduced (canonical) form.
  */
-bool fr_is_one(const fr_t *p) {
-    uint64_t a[4];
-    blst_uint64_from_fr(a, p);
-    return a[0] == 1 && a[1] == 0 && a[2] == 0 && a[3] == 0;
-}
-
-/**
- * Test whether the operand is null (all 0xff's).
- *
- * @param[in]   p   The field element to be checked
- *
- * @retval  true    The element is null
- * @retval  false   The element is not null
- */
-bool fr_is_null(const fr_t *p) {
-    return fr_equal(p, &FR_NULL);
+bool fr_equal(const fr_t *a, const fr_t *b) {
+    return memcmp(a, b, sizeof(fr_t)) == 0;
 }
 
 /**

src/common/fr.h

@@ -48,11 +48,6 @@ static const fr_t FR_ONE = {
     {0x00000001fffffffeL, 0x5884b7fa00034802L, 0x998c4fefecbc4ff5L, 0x1824b159acc5056fL}
 };
 
-/** This used to represent a missing element. It's an invalid value. */
-static const fr_t FR_NULL = {
-    {0xffffffffffffffffL, 0xffffffffffffffffL, 0xffffffffffffffffL, 0xffffffffffffffffL}
-};
-
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Public Functions
 ////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -62,8 +57,6 @@ extern "C" {
 #endif
 
 bool fr_equal(const fr_t *a, const fr_t *b);
-bool fr_is_one(const fr_t *p);
-bool fr_is_null(const fr_t *p);
 void fr_div(fr_t *out, const fr_t *a, const fr_t *b);
 void fr_pow(fr_t *out, const fr_t *a, uint64_t n);
 void fr_from_uint64(fr_t *out, uint64_t n);

src/eip4844/eip4844.c

@@ -51,20 +51,6 @@ static const char *RANDOM_CHALLENGE_DOMAIN_VERIFY_BLOB_KZG_PROOF_BATCH = "RCKZGB
 // Helper Functions
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 
-/**
- * Test whether the operand is zero in the finite field.
- *
- * @param[in]   fr  The field element to be checked
- *
- * @retval  true    The element is zero
- * @retval  false   The element is not zero
- */
-static bool fr_is_zero(const fr_t *fr) {
-    uint64_t a[4];
-    blst_uint64_from_fr(a, fr);
-    return a[0] == 0 && a[1] == 0 && a[2] == 0 && a[3] == 0;
-}
-
 /**
  * Montgomery batch inversion in finite field.
  *
@@ -91,7 +77,7 @@ static C_KZG_RET fr_batch_inv(fr_t *out, const fr_t *a, int len) {
     }
 
     /* Bail on any zero input */
-    if (fr_is_zero(&accumulator)) {
+    if (fr_equal(&accumulator, &FR_ZERO)) {
         return C_KZG_BADARGS;
     }
 

src/eip7594/eip7594.c

@@ -206,9 +206,9 @@ C_KZG_RET recover_cells_and_kzg_proofs(
     ret = new_g1_array(&recovered_proofs_g1, CELLS_PER_EXT_BLOB);
     if (ret != C_KZG_OK) goto out;
 
-    /* Initialize all cells as missing */
+    /* Initialize all cells to zero */
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_EXT_BLOB; i++) {
-        recovered_cells_fr[i] = FR_NULL;
+        recovered_cells_fr[i] = FR_ZERO;
     }
 
     /* Populate recovered_cells_fr with available cells at the right places */

src/eip7594/fft.c

@@ -171,7 +171,7 @@ static void g1_fft_fast(
         g1_fft_fast(out + half, in + stride, stride * 2, roots, roots_stride * 2, half);
         for (size_t i = 0; i < half; i++) {
             /* If the scalar is one, we can skip the multiplication */
-            if (fr_is_one(&roots[i * roots_stride])) {
+            if (fr_equal(&roots[i * roots_stride], &FR_ONE)) {
                 y_times_root = out[i + half];
             } else {
                 g1_mul(&y_times_root, &out[i + half], &roots[i * roots_stride]);

src/eip7594/recovery.c

@@ -196,7 +196,6 @@ static bool is_in_array(const uint64_t *arr, size_t arr_size, uint64_t value) {
  *
  * @remark `reconstructed_data_out` and `cells` can point to the same memory.
  * @remark The array `cells` must be in the correct order (according to cell_indices).
- * @remark Missing cells in `cells` should be equal to FR_NULL.
  */
 C_KZG_RET recover_cells(
     fr_t *reconstructed_data_out,
@@ -280,17 +279,7 @@ C_KZG_RET recover_cells(
      * P(x) is the polynomial we want to reconstruct (degree FIELD_ELEMENTS_PER_BLOB - 1).
      */
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_EXT_BLOB; i++) {
-        if (fr_is_null(&cells_brp[i])) {
-            /*
-             * We handle this situation differently because FR_NULL is an invalid value. The right
-             * hand side, vanishing_poly_eval[i], will always be zero when cells_brp[i] is null, so
-             * the multiplication would still be result in zero, but we shouldn't depend on blst
-             * handling invalid values like this.
-             */
-            extended_evaluation_times_zero[i] = FR_ZERO;
-        } else {
-            blst_fr_mul(&extended_evaluation_times_zero[i], &cells_brp[i], &vanishing_poly_eval[i]);
-        }
+        blst_fr_mul(&extended_evaluation_times_zero[i], &cells_brp[i], &vanishing_poly_eval[i]);
     }
 
     /*

src/setup/setup.c

@@ -111,11 +111,11 @@ static C_KZG_RET expand_root_of_unity(fr_t *out, const fr_t *root, size_t width)
     /* Compute powers of root */
     for (i = 2; i <= width; i++) {
         blst_fr_mul(&out[i], &out[i - 1], root);
-        if (fr_is_one(&out[i])) break;
+        if (fr_equal(&out[i], &FR_ONE)) break;
     }
 
     /* We expect the last entry to be one */
-    if (i != width || !fr_is_one(&out[width])) {
+    if (i != width || !fr_equal(&out[width], &FR_ONE)) {
         return C_KZG_BADARGS;
     }
 

src/test/tests.c

@@ -2027,9 +2027,9 @@ static void test_vanishing_polynomial_for_missing_cells(void) {
     for (size_t i = 0; i < FIELD_ELEMENTS_PER_EXT_BLOB; i++) {
         if (i % CELLS_PER_EXT_BLOB == 1 || i % CELLS_PER_EXT_BLOB == 0) {
             /* Every CELLS_PER_EXT_BLOB-th evaluation should be zero */
-            ASSERT("evaluation is zero", fr_is_zero(&fft_result[i]));
+            ASSERT("evaluation is zero", fr_equal(&fft_result[i], &FR_ZERO));
         } else {
-            ASSERT("evaluation is not zero", !fr_is_zero(&fft_result[i]));
+            ASSERT("evaluation is not zero", !fr_equal(&fft_result[i], &FR_ZERO));
         }
     }
 }