peerDAS: Initial refactor of recover_polynomial() #3591
Conversation
asn-d6
force-pushed
the
peerdas_refactor_recovery_polynomial
branch
from
06a5d03 to
212c1fe
Compare
| """ | ||
| Given Z(x), return polynomial Q_1(x)=(E*Z)(k*x) and Q_2(x)=Z(k*x) and k^{-1} | ||
| """ | ||
| shift_factor = BLSFieldElement(PRIMITIVE_ROOT_OF_UNITY) |
There was a problem hiding this comment.
The c-kzg prototype currently uses 5, but I think it makes sense to use PRIMITIVE_ROOT_OF_UNITY (7); I will update c-kzg to use this. I'm not exactly sure if it makes a difference. Also, we call it scale_factor which I slightly prefer.
There was a problem hiding this comment.
5 is not a primitive root of unity. This was a mistake we had in some research code some years ago. (Not important here but it is in the rest of the code -- we need to use the same sequence of roots of unity everywhere)
Here the only thing that matters is that it's not in our evaluation domain, so there is no risk that the vanishing polynomial is zero anywhere on the shifted domain.
Co-authored-by: Justin Traglia <[email protected]>
There was a problem hiding this comment.
The last assertion in recover_polynomial:
for cell_id, cell in zip(cell_ids, cells):
start = cell_id * FIELD_ELEMENTS_PER_CELL
end = (cell_id + 1) * FIELD_ELEMENTS_PER_CELL
assert reconstructed_data[start:end] == cellIn what case it would fail? Or it's just for debugging?
It's a sanity check similar to the asserts in The asserts essentially check that the recovered data is consistent with the data we used to do the recovery. |
|
Updated the PR here based on your reviews. Thanks all! |
Co-authored-by: Justin Traglia <[email protected]>
Co-authored-by: Justin Traglia <[email protected]>
|
lgtm |
Hello all,
took a stab at splitting
recover_polynomial()into three helper functions, as discussed in #3557.I also sprinkled a few comments around with references to Vitalik's ethresearch post. Comments can and should be greatly improved in the future.
Commit 06a5d03 is a slightly opinionated refactor of
construct_vanishing_polynomial()which I don't have a strong opinion on.IMO the
recover_polynomial()function can be further improved in terms of helper functions, variable naming, and overall documentation, but I suggest we do this in iterated steps./cc @hwwhww @dankrad @jtraglia