Skip to content

Remove KZG verification from local block production and blobs fetched from the EL #7713

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

Merged
merged 2 commits into from
Jul 22, 2025

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Jul 8, 2025

Issue Addressed

#7700

Proposed Changes

As described in title, the EL already performs KZG verification on all blobs when they entered the mempool, so it's redundant to perform extra validation on blobs returned from the EL.

This PR removes

  • KZG verification for both blobs and data columns during block production
  • KZG verification for data columns after fetch engine blobs call. I have not done this for blobs because it requires extra changes to check the observed cache, and doesn't feel like it's a worthy optimisation given the number of blobs per block.

This PR does not remove KZG verification on the block publishing path yet.

@jimmygchen jimmygchen added ready-for-review The code is ready for review optimization Something to make Lighthouse run more efficiently. das Data Availability Sampling fulu Required for the upcoming Fulu hard fork labels Jul 8, 2025
Copy link

mergify bot commented Jul 8, 2025

Some required checks have failed. Could you please take a look @jimmygchen? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jul 8, 2025
@jimmygchen jimmygchen force-pushed the skip-kzg-checks-on-el-blobs branch from 0d7ea70 to d7ab88c Compare July 8, 2025 05:55
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jul 8, 2025
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Just noting here that we don't really gain anything from kzg verifying blobs returned by the builder when publishing because we have already signed the message by the time we receive the BlobsBundle.

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

LGTM on the whole!

@jimmygchen jimmygchen removed the ready-for-review The code is ready for review label Jul 22, 2025
@jimmygchen jimmygchen added the ready-for-merge This PR is ready to merge. label Jul 22, 2025
mergify bot added a commit that referenced this pull request Jul 22, 2025
Copy link

mergify bot commented Jul 22, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You can check the last failing draft PR here: #7772.

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

mergify bot added a commit that referenced this pull request Jul 22, 2025
@mergify mergify bot merged commit b48879a into sigp:unstable Jul 22, 2025
34 checks passed
@jimmygchen jimmygchen deleted the skip-kzg-checks-on-el-blobs branch July 23, 2025 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
das Data Availability Sampling fulu Required for the upcoming Fulu hard fork optimization Something to make Lighthouse run more efficiently. ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants