lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

chore: Add c-kzg library behind a runtime flag

Open kevaundray opened this issue 1 year ago • 3 comments

Issue Addressed

This adds back the c-kzg library (updated to the latest commit).

This PR is reliant on https://github.com/sigp/lighthouse/pull/6117 since the new c-kzg API requires g1_monomial. Once that gets merged, then the lines modified should reduce.

Proposed Changes

Please list or describe the changes introduced by this PR.

Additional Info

Please provide any additional information. For example, future considerations or information useful for reviewers.

kevaundray avatar Jul 16 '24 19:07 kevaundray

This closes #6107 though note that we cannot have two c-kzg libraries in this repository because the c-kzg library links to a c static lib, so the mainnet path will always be "affected" -- Another solution would be to maintain a branch that gets merged into stable which has the rust library and the c-kzg-4844 code. Then another branch which is used for peerdas which has both c-kzg-peerdas and the rust lib. This mainly just imposes a cost to maintaining both branches.

kevaundray avatar Jul 16 '24 19:07 kevaundray

@kevaundray I had a chat with @michaelsproul and he suggested that it might not be a good idea to have two versions of c-kzg - we had some issues with it before and had to rename a bunch of things to avoid symbol collisions.

So it might be better to hold off on this one and add this switch once the c-kzg main version supports PeerDAS. Right now we're going to focus on getting our das stuff onto unstable (main dev branch).

jimmygchen avatar Jul 22 '24 23:07 jimmygchen

Now that we've merged das to unstable please update to point at unstable by either:

  1. Rebasing on unstable (if your branch has a small number of commits that are easy to tease out), or
  2. Merging origin/das into this PR: git fetch origin; git merge origin/das. This will result in the smallest number of conflict resolutions and is better for branches that already contain merge commits or have extensive history.

michaelsproul avatar Aug 27 '24 05:08 michaelsproul