crate-crypto / go-eth-kzg

Apache License 2.0
29 stars 22 forks source link

Reject domain sizes non power of2 #11

Closed GottfriedHerold closed 1 year ago

GottfriedHerold commented 1 year ago

Proposed change: NewDomain should panic if the passed argument is not a power of 2. I raised a related issue with https://github.com/ethereum/c-kzg-4844/issues/226

The semantics for the case where the domain size is not a power of 2 is not documented; there are multiple options, with the most meaningful being the following: If the domain size m is not a power of 2, polynomials are given as degree m-1-polynomials by their evaluations at g^0, ..., g^m-1; These evaluation points only form a subset of the larger set g^0, ..., g^M-1, where M is the next power of 2 after M. FFTs operate in the larger space. The C code tries to achieve this behaviour (well, maybe: While there are some next-power-of-2 calls and careful considerations that seem to achieve this, that case is never tested / documented / seriously considered or needed). My current plan to resolve the issue in the C code is to change to just reject non-powers of 2 and add some remarks in the code in order to keep it simple.

For your current Go code, you actually require / assume that the length of polys == Domain.Cardinality, so (different from the C case), the current Go implementation does not support the general case at the moment. For the sake of simplicity, I would suggest to just reject non-powers-of-two and keep things as they are, otherwise.

Note that this PR also includes some minor changes that are (also) part of a separate less-controversial PR to be finished later. These changes are just also included here to simplify rebasing / merging.