Skip to content

Conversation

@yash-atreya
Copy link
Contributor

Motivation

Closes #1745

Solution

  • Bump c-kzg to 2.0
  • Address breaking changes

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@yash-atreya yash-atreya marked this pull request as ready for review March 25, 2025 06:29
Copy link
Member

@mattsse mattsse left a 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),
Copy link
Member

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

@github-project-automation github-project-automation bot moved this to Reviewed in Alloy Mar 25, 2025
@yash-atreya yash-atreya added blocked This cannot move forward until something else changes breaking Breaking changes have been made labels Mar 25, 2025
@kevaundray
Copy link
Contributor

kevaundray commented Mar 25, 2025

@yash-atreya why is this marked as blocked? I'm wondering if there is an issue with integration

@mattsse
Copy link
Member

mattsse commented Mar 25, 2025

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

@yash-atreya yash-atreya requested a review from DaniPopes March 26, 2025 06:50
@mattsse mattsse merged commit c04dbec into main Mar 28, 2025
27 checks passed
@mattsse mattsse deleted the yash/bump-c-kzg branch March 28, 2025 13:44
@github-project-automation github-project-automation bot moved this from Reviewed to Done in Alloy Mar 28, 2025
mergify bot pushed a commit to sigp/lighthouse that referenced this pull request Jul 9, 2025
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked This cannot move forward until something else changes breaking Breaking changes have been made

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug] Upgrade c-kzg dependency to 2.0

6 participants