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

EIP4844: Refactor verify_kzg_proof() to receive bytes (used in precompile) #3097

Merged
merged 2 commits into from
Nov 18, 2022

Conversation

asn-d6
Copy link
Contributor

@asn-d6 asn-d6 commented Nov 11, 2022

We received reports by client devs that the precompile was annoying to implement because of the modular reductions that had to be done outside of the crypto layer.

This PR makes verify_kzg_proof() bit more high-level so that client devs can interact with it by just passing bytes to it.

Note: Client devs still have to check the assert kzg_to_versioned_hash(data_kzg) == versioned_hash when implementing the precompile.

This way, client devs don't need to convert to field elements themselves, and the KZG library takes care fo it.
@asn-d6 asn-d6 requested a review from dankrad November 11, 2022 23:52
@asn-d6 asn-d6 changed the title Refactor verify_kzg_proof() to receive bytes (used in precompile) EIP4844: Refactor verify_kzg_proof() to receive bytes (used in precompile) Nov 11, 2022
@roberto-bayardo
Copy link
Contributor

roberto-bayardo commented Nov 12, 2022

I assume there will be a sister change for the EIP-4844 itself which still has the precompile function doing the conversion to field element?

FWIW, one could just put PointEvaluationPrecompile itself in the kzg library for all clients to share (which was what I was doing in our geth fork), as it already has a bytes api:

def point_evaluation_precompile(input: Bytes) -> Bytes:

But this change still makes sense if one wants to separate the EL and CL concerns of 4844.

Our EIP-4844 Golang bytes-api implementation from geth which is also by our Prysm fork can be found below. It's a work in progress but feedback is welcome. I've updated it to incorporate the changes proposed here.

https://github.com/mdehoog/go-ethereum/tree/more-kzg-2/crypto/kzg

@asn-d6
Copy link
Contributor Author

asn-d6 commented Nov 13, 2022

I assume there will be a sister change for the EIP-4844 itself which still has the precompile function doing the conversion to field element?

Yep, for sure we also need to update the EIP!

Afte we stabilize the crypto API again on the consensus-specs side (pretty much just this ticket and #3093 remaining) we will sync up the EIP.

FWIW, one could just put PointEvaluationPrecompile itself in the kzg library for all clients to share (which was what I was doing in our geth fork), as it already has a bytes api:

Yep, that's another reasonable approach. No strong opinion here. Whatever is easier for client devs. Let's raise it on the next call?

@hwwhww hwwhww added the Deneb was called: eip-4844 label Nov 15, 2022
@asn-d6 asn-d6 removed the request for review from dankrad November 18, 2022 17:10
@asn-d6 asn-d6 merged commit a456271 into ethereum:dev Nov 18, 2022
asn-d6 added a commit to asn-d6/EIPs that referenced this pull request Nov 18, 2022
asn-d6 added a commit to asn-d6/EIPs that referenced this pull request Nov 22, 2022
eth-bot pushed a commit to ethereum/EIPs that referenced this pull request Nov 22, 2022
…-specs) (#6002)

* EIP4844: Refactor precompile to use bytes-based verify_kzg_proof()

See ethereum/consensus-specs#3097 for the changes to the verify_kzg_proof API

* EIP4844: validate_blob_transaction_wrapper() now uses latest API
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.

4 participants