From ece913f6368e14a566d3586b940f87fbcc77f9f2 Mon Sep 17 00:00:00 2001 From: Ben Edgington Date: Wed, 3 Feb 2021 12:57:47 +0000 Subject: [PATCH] Add some rudimentary error and debug handling --- src/c-kzg.h | 6 ++---- src/fft_fr_test.c | 9 ++++++--- src/fft_g1_test.c | 6 ++++-- src/fft_util.c | 45 +++++++++++++++++++++++---------------------- src/fft_util.h | 6 +++--- src/fft_util_test.c | 20 +++++++++----------- 6 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/c-kzg.h b/src/c-kzg.h index 4e16f61..d0cba40 100644 --- a/src/c-kzg.h +++ b/src/c-kzg.h @@ -20,16 +20,14 @@ typedef enum { C_KZG_SUCCESS = 0, C_KZG_BADARGS, - c_KZG_ERROR, + C_KZG_ERROR, } C_KZG_RET; #include #include "../inc/blst.h" -#define DEBUG - -#include #ifdef DEBUG +#include #include #define ASSERT(cond, ret) if (!(cond)) \ { \ diff --git a/src/fft_fr_test.c b/src/fft_fr_test.c index 99ec96a..d186493 100644 --- a/src/fft_fr_test.c +++ b/src/fft_fr_test.c @@ -41,7 +41,8 @@ const uint64_t inv_fft_expected[][4] = void compare_sft_fft(void) { // Initialise: ascending values of i (could be anything), and arbitrary size unsigned int size = 12; - FFTSettings fs = new_fft_settings(size); + FFTSettings fs; + TEST_CHECK(new_fft_settings(&fs, size) == C_KZG_SUCCESS); blst_fr data[fs.max_width], out0[fs.max_width], out1[fs.max_width]; for (int i = 0; i < fs.max_width; i++) { fr_from_uint64(data + i, i); @@ -62,7 +63,8 @@ void compare_sft_fft(void) { void roundtrip_fft(void) { // Initialise: ascending values of i, and arbitrary size unsigned int size = 12; - FFTSettings fs = new_fft_settings(size); + FFTSettings fs; + TEST_CHECK(new_fft_settings(&fs, size) == C_KZG_SUCCESS); blst_fr data[fs.max_width], coeffs[fs.max_width]; for (int i = 0; i < fs.max_width; i++) { fr_from_uint64(data + i, i); @@ -84,7 +86,8 @@ void roundtrip_fft(void) { void inverse_fft(void) { // Initialise: ascending values of i - FFTSettings fs = new_fft_settings(4); + FFTSettings fs; + TEST_CHECK(new_fft_settings(&fs, 4) == C_KZG_SUCCESS); blst_fr data[fs.max_width], out[fs.max_width]; for (int i = 0; i < fs.max_width; i++) { fr_from_uint64(&data[i], i); diff --git a/src/fft_g1_test.c b/src/fft_g1_test.c index 35b74ad..731c0e0 100644 --- a/src/fft_g1_test.c +++ b/src/fft_g1_test.c @@ -59,7 +59,8 @@ void p1_sub_works(void) { void compare_sft_fft(void) { // Initialise: arbitrary size unsigned int size = 6; - FFTSettings fs = new_fft_settings(size); + FFTSettings fs; + TEST_CHECK(new_fft_settings(&fs, size) == C_KZG_SUCCESS); blst_p1 data[fs.max_width], slow[fs.max_width], fast[fs.max_width]; make_data(data, fs.max_width); @@ -78,7 +79,8 @@ void compare_sft_fft(void) { void roundtrip_fft(void) { // Initialise: arbitrary size unsigned int size = 10; - FFTSettings fs = new_fft_settings(size); + FFTSettings fs; + TEST_CHECK(new_fft_settings(&fs, size) == C_KZG_SUCCESS); blst_p1 expected[fs.max_width], data[fs.max_width], coeffs[fs.max_width]; make_data(expected, fs.max_width); make_data(data, fs.max_width); diff --git a/src/fft_util.c b/src/fft_util.c index e273afb..717fe46 100644 --- a/src/fft_util.c +++ b/src/fft_util.c @@ -31,41 +31,42 @@ void fr_from_uint64(blst_fr *a, uint64_t n) { blst_fr_from_uint64(a, vals); } -// Returns an array of powers of the root of unity -// Allocates space for the array that needs to be freed later -blst_fr *expand_root_of_unity(blst_fr *root_of_unity, uint64_t width) { - blst_fr *roots = malloc((width + 1) * sizeof(blst_fr)); +// Create an array of powers of the root of unity +// The `out` array must be of size `width + 1` +C_KZG_RET expand_root_of_unity(blst_fr *roots, blst_fr *root_of_unity, uint64_t width) { roots[0] = one; roots[1] = *root_of_unity; for (int i = 2; !is_one(&roots[i - 1]); i++) { - //ASSERT(i <= width, C_KZG_ERROR); - assert(i <= width); + ASSERT(i <= width, C_KZG_ERROR); blst_fr_mul(&roots[i], &roots[i - 1], root_of_unity); } - assert(is_one(&roots[width])); + ASSERT(is_one(&roots[width]), C_KZG_ERROR); - return roots; + return C_KZG_SUCCESS; } -// Return a reversed copy of the list of Fr provided -// `width` is one less than the length of `r` -// Allocates space for the array that needs to be freed later -blst_fr *reverse(blst_fr *r, uint64_t width) { - blst_fr *rr = malloc((width + 1) * sizeof(blst_fr)); +// Create a reversed list of Fr provided +// `width` is one less than the length of `roots` +C_KZG_RET reverse(blst_fr *out, blst_fr *roots, uint64_t width) { for (int i = 0; i <= width; i++) { - rr[i] = r[width - i]; + out[i] = roots[width - i]; } - return rr; + + return C_KZG_SUCCESS; } -FFTSettings new_fft_settings(unsigned int max_scale) { - FFTSettings s; - s.max_width = (uint64_t)1 << max_scale; - blst_fr_from_uint64(&s.root_of_unity, scale2_root_of_unity[max_scale]); - s.expanded_roots_of_unity = expand_root_of_unity(&s.root_of_unity, s.max_width); - s.reverse_roots_of_unity = reverse(s.expanded_roots_of_unity, s.max_width); - return s; +C_KZG_RET new_fft_settings(FFTSettings *s, unsigned int max_scale) { + C_KZG_RET ret; + s->max_width = (uint64_t)1 << max_scale; + blst_fr_from_uint64(&s->root_of_unity, scale2_root_of_unity[max_scale]); + s->expanded_roots_of_unity = malloc((s->max_width + 1) * sizeof(blst_fr)); + s->reverse_roots_of_unity = malloc((s->max_width + 1) * sizeof(blst_fr)); + + ret = expand_root_of_unity(s->expanded_roots_of_unity, &s->root_of_unity, s->max_width); + if (ret != C_KZG_SUCCESS) return ret; + ret = reverse(s->reverse_roots_of_unity, s->expanded_roots_of_unity, s->max_width); + return ret; } void free_fft_settings(FFTSettings *s) { diff --git a/src/fft_util.h b/src/fft_util.h index 82baed4..ccec456 100644 --- a/src/fft_util.h +++ b/src/fft_util.h @@ -72,7 +72,7 @@ typedef struct { bool is_one(const blst_fr *fr_p); bool is_power_of_two(uint64_t n); void fr_from_uint64(blst_fr *a, uint64_t n); -blst_fr *expand_root_of_unity(blst_fr *root_of_unity, uint64_t width); -blst_fr *reverse(blst_fr *r, uint64_t width); -FFTSettings new_fft_settings(unsigned int max_scale); +C_KZG_RET expand_root_of_unity(blst_fr * roots, blst_fr *root_of_unity, uint64_t width); +C_KZG_RET reverse(blst_fr *out, blst_fr *roots, uint64_t width); +C_KZG_RET new_fft_settings(FFTSettings *s, unsigned int max_scale); void free_fft_settings(FFTSettings *s); diff --git a/src/fft_util_test.c b/src/fft_util_test.c index 684c0ff..7541e55 100644 --- a/src/fft_util_test.c +++ b/src/fft_util_test.c @@ -43,8 +43,8 @@ void roots_of_unity_are_plausible(void) { void reverse_works(void) { int n = 24; - blst_fr arr[n + 1]; - blst_fr *rev, diff; + blst_fr arr[n + 1], rev[n + 1]; + blst_fr diff; // Initialise - increasing values arr[0] = one; @@ -53,7 +53,7 @@ void reverse_works(void) { } // Reverse - rev = reverse(arr, n); + TEST_CHECK(reverse(rev, arr, n) == C_KZG_SUCCESS); // Verify - decreasing values for (int i = 0; i < n; i++) { @@ -61,19 +61,17 @@ void reverse_works(void) { TEST_CHECK(true == is_one(&diff)); } TEST_CHECK(true == is_one(rev + n)); - - free(rev); } void expand_roots_is_plausible(void) { // Just test one (largeish) value of scale - unsigned int scale = 20; + unsigned int scale = 15; unsigned int width = 1 << scale; - blst_fr root, *expanded, prod; + blst_fr root, expanded[width + 1], prod; // Initialise blst_fr_from_uint64(&root, scale2_root_of_unity[scale]); - expanded = expand_root_of_unity(&root, width); + TEST_CHECK(expand_root_of_unity(expanded, &root, width) == C_KZG_SUCCESS); // Verify - each pair should multiply to one TEST_CHECK(true == is_one(expanded + 0)); @@ -82,8 +80,6 @@ void expand_roots_is_plausible(void) { blst_fr_mul(&prod, expanded + i, expanded + width - i); TEST_CHECK(true == is_one(&prod)); } - - free(expanded); } void new_fft_settings_is_plausible(void) { @@ -91,7 +87,9 @@ void new_fft_settings_is_plausible(void) { unsigned int scale = 21; unsigned int width = 1 << scale; blst_fr prod; - FFTSettings s = new_fft_settings(scale); + FFTSettings s; + + TEST_CHECK(new_fft_settings(&s, scale) == C_KZG_SUCCESS); // Verify - each pair should multiply to one for (unsigned int i = 1; i <= width; i++) {