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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Jul 8, 2025 •
edited
Loading

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
@mergify Mergify
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
pawanjay176
pawanjay176 reviewed Jul 10, 2025
View reviewed changes
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.

beacon_node/beacon_chain/src/fetch_blobs/mod.rs
.filter(|col| custody_columns_indices.contains(&col.index))
.map(|col| {
KzgVerifiedCustodyDataColumn::from_asserted_custody(
KzgVerifiedDataColumn::from_execution_verified(col),
Copy link
Member

Choose a reason for hiding this comment

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

We haven't removed the gossip verification in fetch_blobs_v1 above which does kzg verification. Do we want to remove that as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pawanjay176 pawanjay176 pawanjay176 left review comments

@michaelsproul michaelsproul Awaiting requested review from michaelsproul

Assignees
No one assigned
Labels
das Data Availability Sampling fulu Required for the upcoming Fulu hard fork optimization Something to make Lighthouse run more efficiently. ready-for-review The code is ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jimmygchen @pawanjay176