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 official trusted setup #377

Merged
merged 19 commits into from
Oct 18, 2023
Merged

Conversation

jtraglia
Copy link
Member

This PR implements the changes in:

As an overview, this PR will:

  • Replace the old trusted setup with the new (official) trusted setup.
  • Remove references to minimal preset (they're the same now).
  • Update the reference tests since the trusted setup has changed.
  • Update bindings to work with singular FIELD_ELEMENTS_PER_BLOB value.

@jtraglia jtraglia requested a review from a team October 17, 2023 14:52
@StefanBratanov
Copy link
Contributor

LGTM on the java binding side. Can only delete trusted_setup_4.txt and trusted_setup_4_old.txt in the testFixtures resources

@jtraglia jtraglia marked this pull request as ready for review October 18, 2023 12:42
@jtraglia
Copy link
Member Author

It was brought to my attention that -Wpedantic is causing compiler errors on some versions of GCC. This is making it difficult to perform builds with CI systems. This doesn't have anything to do with this PR, but I'd like to make the change here for simplicity. We recently enabled these warnings (treated as errors) here:

Copy link
Contributor

@asn-d6 asn-d6 left a comment

Choose a reason for hiding this comment

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

Looks great to me! Thanks for all the great work @jtraglia !

Copy link
Contributor

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

LGTM 🔥🔥🔥

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

nodejs bindings look good 👍

@jtraglia
Copy link
Member Author

I think we have approval from most client teams. Going to merge.

@jtraglia jtraglia merged commit d637761 into ethereum:main Oct 18, 2023
33 checks passed
@jtraglia jtraglia deleted the official-trusted-setup branch October 18, 2023 18:32
bors bot pushed a commit to sigp/lighthouse that referenced this pull request Oct 20, 2023
## Issue Addressed

Makes lighthouse compliant with new kzg changes in https://github.com/ethereum/consensus-specs/releases/tag/v1.4.0-beta.3

## Proposed Changes

1. Adds new official trusted setup
2. Refactors kzg to match upstream changes in ethereum/c-kzg-4844#377
3. Updates pre-generated `BlobBundle` to work with official trusted setup. ~~Using json here instead of ssz to account for different value of `MaxBlobCommitmentsPerBlock` in minimal and mainnet. By using json, we can just use one pre generated bundle for both minimal and mainnet. Size of 2 separate ssz bundles is approximately equal to one json bundle cc @jimmygchen~~ 
Dunno what I was doing, ssz works without any issues  
4. Stores trusted_setup as just bytes in eth2_network_config so that we don't have kzg dependency in that lib and in lcli. 


Co-authored-by: realbigsean <seananderson33@gmail.com>
Co-authored-by: realbigsean <seananderson33@GMAIL.com>
bors bot pushed a commit to sigp/lighthouse that referenced this pull request Oct 21, 2023
## Issue Addressed

Makes lighthouse compliant with new kzg changes in https://github.com/ethereum/consensus-specs/releases/tag/v1.4.0-beta.3

## Proposed Changes

1. Adds new official trusted setup
2. Refactors kzg to match upstream changes in ethereum/c-kzg-4844#377
3. Updates pre-generated `BlobBundle` to work with official trusted setup. ~~Using json here instead of ssz to account for different value of `MaxBlobCommitmentsPerBlock` in minimal and mainnet. By using json, we can just use one pre generated bundle for both minimal and mainnet. Size of 2 separate ssz bundles is approximately equal to one json bundle cc @jimmygchen~~ 
Dunno what I was doing, ssz works without any issues  
4. Stores trusted_setup as just bytes in eth2_network_config so that we don't have kzg dependency in that lib and in lcli. 


Co-authored-by: realbigsean <seananderson33@gmail.com>
Co-authored-by: realbigsean <seananderson33@GMAIL.com>
DaniPopes added a commit to DaniPopes/c-kzg-4844 that referenced this pull request Nov 11, 2023
Now that header definitions are stable across builds (ethereum#377), we can remove the
Rust `bindgen` build dependency. This also transitively removes the dependency
on `libclang`.
DaniPopes added a commit to DaniPopes/c-kzg-4844 that referenced this pull request Nov 11, 2023
Now that header definitions are stable across builds (ethereum#377), we can remove the
Rust `bindgen` build dependency by gating it to an optional feature. This also
transitively removes the dependency on `libclang` for Rust bindings.
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.

8 participants