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

Remove extraneous length check from deneb forkchoice #3368

Merged
merged 2 commits into from
Jun 1, 2023
Merged

Conversation

ralexstokes
Copy link
Member

Remove duplicate check:

verify_blob_kzg_proof_batch checks that len(expected_kzg_commitments) == len(blobs) == len(proofs) so it is duplicate to have here.

I'd say we assume this functionality is part of the API of verify_blob_kzg_proof_batch and if so we can remove this check here and also under the validator.md guide.

What do we think @asn-d6 @jtraglia @hwwhww ?

Remove duplicate check
Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

I think that's a fair assumption. LGTM 👍

@hwwhww
Copy link
Contributor

hwwhww commented May 18, 2023

I git-blamed it. This helper had more assertions before the blob decoupling.

@ralexstokes
Unless we do want to extend validate_blob content later, it would be simpler if we just remove validate_blob and use verify_blob_kzg_proof_batch directly in is_data_available.

@dapplion
Copy link
Collaborator

PR title should be something meaningful

@djrtwo djrtwo changed the title Update fork-choice.md Remove extraneous length check from deneb forkchoice May 18, 2023
@hwwhww hwwhww mentioned this pull request Jun 1, 2023
5 tasks
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.

6 participants