search

ethereum / c-kzg-4844

A minimal implementation of the Polynomial Commitments API for EIP-4844 and EIP-7594, written in C.
Apache License 2.0
112 stars 105 forks source link

Pre-compute root of unity in fr-form #491

Closed jtraglia closed 1 month ago

jtraglia commented 1 month ago

This replaces SCALE2_ROOT_OF_UNITY in uint64s-form with ROOT_OF_UNITY in fr-form. I don't have a strong opinion about this, but I was hoping to simplify this. It should at least be moved outside of the header. I provide updated instructions (scripts) for generating this constant. Had to remove some tests for compute_roots_of_unity though.

I would prefer a C function which uses PRIMITIVE_ROOT_OF_UNITY and generates the root of unity with blst, but there isn't a blst_fr_pow function and it seems quite difficult (maybe impossible) to do so ourselves.

asn-d6 commented 1 month ago

This code has been around since 4844 and has been working nicely. I have a small preference for leaving old code sleep.

If you like this PR, how about you add a small C test that shows that the new value you added is the same value from the uint64 array? Just to show that the new code and the old code (that will be removed) do the same thing.

jtraglia commented 1 month ago

If you like this PR, how about you add a small C test that shows that the new value you added is the same value from the uint64 array? Just to show that the new code and the old code (that will be removed) do the same thing.

Okay yeah, I'll make that test. I do like this PR.

jtraglia commented 1 month ago

@asn-d6 Okay this should be ready now. I'm glad we added these tests back, good suggestion.