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

Optimizing EIP-4844 block validation (using KZG proofs) #2915

Merged
merged 8 commits into from
Jun 26, 2022

Conversation

asn-d6
Copy link
Contributor

@asn-d6 asn-d6 commented Jun 14, 2022

Hey!

This PR introduces KZG proofs for block validation on the consensus-side, the same way that ethereum/EIPs#5088 introduces KZG proofs for mempool transaction validation on the execution-side.

The main protocol change in terms of communication is that we are now adding a KZG proof inBlobsSidecar. The KZG proof is just there to speed up the validation of the commitments in the BeaconBlockBody and has no other value. Hence it doesn't actually need to go on-chain and can remain in the sidecar.

Please read ethereum/EIPs#5088 for the details on how the crypto/validation logic works.

Let me know if you have any questions :)

@hwwhww hwwhww added the Deneb was called: eip-4844 label Jun 17, 2022
@dankrad
Copy link
Contributor

dankrad commented Jun 20, 2022

Looking goods. Would it make sense to put the polynomial and KZG functions into a separate file?

@asn-d6 asn-d6 force-pushed the consensus-4844-proofs-optimization branch from 3a86b99 to 1862bc7 Compare June 22, 2022 12:34
@asn-d6
Copy link
Contributor Author

asn-d6 commented Jun 22, 2022

Looking goods. Would it make sense to put the polynomial and KZG functions into a separate file?

Pushed a commit which splits the BLS/poly/KZG code into a separate file. Let me know if you would like the organization (or the filename) to be different.

I also pushed a commit that uses the native Python pow to compute modular inverses.

@asn-d6 asn-d6 force-pushed the consensus-4844-proofs-optimization branch from 1862bc7 to e7e5207 Compare June 22, 2022 12:42
specs/eip4844/polynomial-commitments.md Outdated Show resolved Hide resolved
specs/eip4844/polynomial-commitments.md Outdated Show resolved Hide resolved
specs/eip4844/polynomial-commitments.md Outdated Show resolved Hide resolved
specs/eip4844/polynomial-commitments.md Show resolved Hide resolved
specs/eip4844/validator.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

Great work @asn-d6 @adietrichs @dankrad
lgtm with my naked eyes.

if we merge it, I can test it with #2901 better.

@asn-d6
Copy link
Contributor Author

asn-d6 commented Jun 23, 2022

Great work @asn-d6 @adietrichs @dankrad lgtm with my naked eyes.

if we merge it, I can test it with #2901 better.

Thanks for the review! Feel free to merge whenever!

Copy link
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

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

Good stuff! Looks good to me from an implementer's perspective.

```python
def div(x: BLSFieldElement, y: BLSFieldElement) -> BLSFieldElement:
"""Divide two field elements: `x` by `y`"""
return x * inv(y) % BLS_MODULUS
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Replace inv with bls_modular_inverse

@hwwhww hwwhww merged commit d4a2bdc into ethereum:dev Jun 26, 2022
asn-d6 added a commit to asn-d6/consensus-specs that referenced this pull request Jul 13, 2022
- Move more code into polynomial-commitments.md
- Implement aggregated sidecar verification logic from PR ethereum#2915
- Rename `kzgs` to `kzg_commitments`

Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb was called: eip-4844
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants