-
Notifications
You must be signed in to change notification settings - Fork 548
chore(deps): c-kzg 2.0 #2240
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
chore(deps): c-kzg 2.0 #2240
Conversation
mattsse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
| DEFAULT.get_or_init(|| alloc::boxed::Box::new(load())) | ||
| } | ||
| } | ||
| Self::Default => c_kzg::ethereum_kzg_settings(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ckzg api is incredibly weird, because this allows providing an argument which is then ignored the second time this function is called because the kzgsettings are cached.
however, for this crate this is perfectly fine
|
@yash-atreya why is this marked as blocked? I'm wondering if there is an issue with integration |
|
no issue with the integration, but this is a breaking change and would further split crate compatibility until everything is migrated properly. but I think we can now send 0.13 but this must be timed with revm release otherwise this can't be upgraded |
Update `c-kzg` from `v1` to `v2`. My motivation here is that `alloy-consensus` now uses `c-kzg` in `v2` and this results in a conflict when using lighthouse in combination with latest alloy. I tried also to disable the `czkg` feature in alloy, but the conflict persisted. See here for the alloy update to `c-kzg v2`: alloy-rs/alloy#2240 Error: ``` error: failed to select a version for `c-kzg`. ... versions that meet the requirements `^1` are: 1.0.3, 1.0.2, 1.0.0 the package `c-kzg` links to the native library `ckzg`, but it conflicts with a previous package which links to `ckzg` as well: package `c-kzg v2.1.0` ... which satisfies dependency `c-kzg = "^2.1"` of package `alloy-consensus v0.13.0` ... which satisfies dependency `alloy-consensus = "^0.13.0"` of package ... ... ``` - Upgrade `alloy-consensus` to `0.14.0` and disable all default features - Upgrade `c-kzg` to `v2.1.0` - Upgrade `alloy-primitives` to `1.0.0` - Adapt the code to the new API `c-kzg` - There is now `NO_PRECOMPUTE` as my understand from https://github.com/ethereum/c-kzg-4844/pull/545/files we should use `0` here as `new_from_trusted_setup_no_precomp` does not precomp. But maybe it is misleading. For all other places I used `RECOMMENDED_PRECOMP_WIDTH` because `8` is matching the recommendation. - `BYTES_PER_G1_POINT` and `BYTES_PER_G2_POINT` are no longer public in `c-kzg` - I adapted two tests that checking for the `Attestation` bitfield size. But I could not pinpoint to what has changed and why now 8 bytes less. I would be happy about any hint, and if this is correct. I found related a PR here: #6915 - Use same fields names, in json, as well as `c-kzg` and `rust_eth_kzg` for `g1_monomial`, `g1_lagrange`, and `g2_monomial`
Motivation
Closes #1745
Solution
PR Checklist