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

Use blst crate for Rust blst dep #351

Merged
merged 2 commits into from
Aug 24, 2023

Conversation

michaelsproul
Copy link
Contributor

We were getting nasty linker errors in Lighthouse due to different versions of the blst library being linked by:

  • crypto/bls: our BLS wrapper that depends on the blst crate from crates.io.
  • c_kzg/c_kzg_min: mainnet/minimal variants of c-kzg which depend on their own libblst.a which is built at compile-time.

As well as causing link errors when the blst versions get out of sync, the current setup also wastes compilation time by compiling blst 3 times.

I think the changes in this PR improve the situation by using a single copy of blst throughout. Due to the links attribute in blst's Cargo.toml, Cargo will now error if we try to compile with multiple copies of the blst crate. We can also make semver-compatible updates to blst downstream without having to bump the blst version in c-kzg, so long as the headers are compatible with the ones vendored in c-kzg.

Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

Nice. LGTM 👍

@jtraglia jtraglia merged commit 4c0d477 into ethereum:main Aug 24, 2023
30 of 32 checks passed
@michaelsproul michaelsproul deleted the use-blst-crate branch August 24, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants