Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup roots of unity in KZGSettings #467

Merged
merged 10 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bindings/rust/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ cargo test --release
```
cargo bench
```

## Update `generated.rs`

```
cargo build --features generate-bindings
```
1 change: 1 addition & 0 deletions bindings/rust/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ fn make_bindings(header_path: &str, blst_headers_dir: &str, bindings_out_path: &
.allowlist_type("C_KZG_RET")
.allowlist_var("BYTES_PER_.*")
.allowlist_var("FIELD_ELEMENTS_PER_BLOB")
.allowlist_var("FIELD_ELEMENTS_PER_EXT_BLOB")
.allowlist_file(".*eip.*.h")
.allowlist_file(".*setup.h")
/*
Expand Down
12 changes: 5 additions & 7 deletions bindings/rust/src/bindings/generated.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions src/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,12 +662,12 @@ static void g1_fft_fast(
*/
C_KZG_RET g1_fft(g1_t *out, const g1_t *in, size_t n, const KZGSettings *s) {
/* Ensure the length is valid */
if (n > s->max_width || !is_power_of_two(n)) {
if (n > FIELD_ELEMENTS_PER_EXT_BLOB || !is_power_of_two(n)) {
return C_KZG_BADARGS;
}

uint64_t stride = s->max_width / n;
g1_fft_fast(out, in, 1, s->expanded_roots_of_unity, stride, n);
uint64_t stride = FIELD_ELEMENTS_PER_EXT_BLOB / n;
g1_fft_fast(out, in, 1, s->roots_of_unity, stride, n);

return C_KZG_OK;
}
Expand All @@ -685,11 +685,11 @@ C_KZG_RET g1_fft(g1_t *out, const g1_t *in, size_t n, const KZGSettings *s) {
*/
C_KZG_RET g1_ifft(g1_t *out, const g1_t *in, size_t n, const KZGSettings *s) {
/* Ensure the length is valid */
if (n > s->max_width || !is_power_of_two(n)) {
if (n > FIELD_ELEMENTS_PER_EXT_BLOB || !is_power_of_two(n)) {
return C_KZG_BADARGS;
}

uint64_t stride = s->max_width / n;
uint64_t stride = FIELD_ELEMENTS_PER_EXT_BLOB / n;
g1_fft_fast(out, in, 1, s->reverse_roots_of_unity, stride, n);

fr_t inv_len;
Expand Down
34 changes: 27 additions & 7 deletions src/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ extern "C" {
/** The number of field elements in a blob. */
#define FIELD_ELEMENTS_PER_BLOB 4096

/** The number of field elements in an extended blob */
#define FIELD_ELEMENTS_PER_EXT_BLOB (FIELD_ELEMENTS_PER_BLOB * 2)

/** The number of bytes in a blob. */
#define BYTES_PER_BLOB (FIELD_ELEMENTS_PER_BLOB * BYTES_PER_FIELD_ELEMENT)

Expand Down Expand Up @@ -100,14 +103,31 @@ typedef Bytes48 KZGProof;

/** Stores the setup and parameters needed for computing KZG proofs. */
typedef struct {
/** The length of `roots_of_unity`, a power of 2. */
uint64_t max_width;
/** Powers of the primitive root of unity determined by `SCALE2_ROOT_OF_UNITY` in bit-reversal
* permutation order, length `max_width`. */
/**
* Roots of unity for the subgroup of size `domain_size`.
*
* The array contains `domain_size + 1` elements, it starts and ends with Fr::one().
*/
fr_t *roots_of_unity;
/** The expanded roots of unity. */
fr_t *expanded_roots_of_unity;
/** The bit-reversal permuted roots of unity. */
/**
* Roots of unity for the subgroup of size `domain_size` in bit-reversed order.
*
* This array is derived by applying a bit-reversal permutation to `roots_of_unity`
* excluding the last element. Essentially:
* `brp_roots_of_unity = bit_reversal_permutation(roots_of_unity[:-1])`
*
* The array contains `domain_size` elements.
*/
fr_t *brp_roots_of_unity;
asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
/**
* Roots of unity for the subgroup of size `domain_size` in reversed order.
*
* It is the reversed version of `roots_of_unity`. Essentially:
* `reverse_roots_of_unity = reverse(roots_of_unity)`
*
* This array is primarily used in FFTs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel that this remark adds all that much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the python pseudocode in an attempt to highlight the difference between this and brp_roots_of_unity (both in terms of "reverse" vs "reverse-bit order" and in terms of excluding the last element).

With regards to "This array is primarily used in FFTs.", I was confused on why we need the reverse roots in the first place, so this would be useful to me, but I understand that it might be superfluous.

* The array contains `domain_size + 1` elements, it starts and ends with Fr::one().
*/
fr_t *reverse_roots_of_unity;
asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
/** G1 group elements from the trusted setup in monomial form. */
asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
g1_t *g1_values_monomial;
Expand Down
18 changes: 9 additions & 9 deletions src/eip4844.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ static C_KZG_RET evaluate_polynomial_in_evaluation_form(
fr_t *inverses_in = NULL;
fr_t *inverses = NULL;
uint64_t i;
const fr_t *roots_of_unity = s->roots_of_unity;
const fr_t *brp_roots_of_unity = s->brp_roots_of_unity;

ret = new_fr_array(&inverses_in, FIELD_ELEMENTS_PER_BLOB);
if (ret != C_KZG_OK) goto out;
Expand All @@ -210,20 +210,20 @@ static C_KZG_RET evaluate_polynomial_in_evaluation_form(
* given, we can just return the result directly. Note that special-casing this is
* necessary, as the formula below would divide by zero otherwise.
*/
if (fr_equal(x, &roots_of_unity[i])) {
if (fr_equal(x, &brp_roots_of_unity[i])) {
*out = p->evals[i];
ret = C_KZG_OK;
goto out;
}
blst_fr_sub(&inverses_in[i], x, &roots_of_unity[i]);
blst_fr_sub(&inverses_in[i], x, &brp_roots_of_unity[i]);
}

ret = fr_batch_inv(inverses, inverses_in, FIELD_ELEMENTS_PER_BLOB);
if (ret != C_KZG_OK) goto out;

*out = FR_ZERO;
for (i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
blst_fr_mul(&tmp, &inverses[i], &roots_of_unity[i]);
blst_fr_mul(&tmp, &inverses[i], &brp_roots_of_unity[i]);
blst_fr_mul(&tmp, &tmp, &p->evals[i]);
blst_fr_add(out, out, &tmp);
}
Expand Down Expand Up @@ -428,7 +428,7 @@ static C_KZG_RET compute_kzg_proof_impl(

fr_t tmp;
Polynomial q;
const fr_t *roots_of_unity = s->roots_of_unity;
const fr_t *brp_roots_of_unity = s->brp_roots_of_unity;
uint64_t i;
/* m != 0 indicates that the evaluation point z equals root_of_unity[m-1] */
uint64_t m = 0;
Expand All @@ -439,15 +439,15 @@ static C_KZG_RET compute_kzg_proof_impl(
if (ret != C_KZG_OK) goto out;

for (i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
if (fr_equal(z, &roots_of_unity[i])) {
if (fr_equal(z, &brp_roots_of_unity[i])) {
/* We are asked to compute a KZG proof inside the domain */
m = i + 1;
inverses_in[i] = FR_ONE;
continue;
}
// (p_i - y) / (ω_i - z)
blst_fr_sub(&q.evals[i], &polynomial->evals[i], y_out);
blst_fr_sub(&inverses_in[i], &roots_of_unity[i], z);
blst_fr_sub(&inverses_in[i], &brp_roots_of_unity[i], z);
}

ret = fr_batch_inv(inverses, inverses_in, FIELD_ELEMENTS_PER_BLOB);
Expand All @@ -462,7 +462,7 @@ static C_KZG_RET compute_kzg_proof_impl(
for (i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
if (i == m) continue;
/* Build denominator: z * (z - ω_i) */
blst_fr_sub(&tmp, z, &roots_of_unity[i]);
blst_fr_sub(&tmp, z, &brp_roots_of_unity[i]);
blst_fr_mul(&inverses_in[i], &tmp, z);
}

Expand All @@ -473,7 +473,7 @@ static C_KZG_RET compute_kzg_proof_impl(
if (i == m) continue;
/* Build numerator: ω_i * (p_i - y) */
blst_fr_sub(&tmp, &polynomial->evals[i], y_out);
blst_fr_mul(&tmp, &tmp, &roots_of_unity[i]);
blst_fr_mul(&tmp, &tmp, &brp_roots_of_unity[i]);
/* Do the division: (p_i - y) * ω_i / (z * (z - ω_i)) */
blst_fr_mul(&tmp, &tmp, &inverses[i]);
blst_fr_add(&q.evals[m], &q.evals[m], &tmp);
Expand Down
Loading
Loading