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

Implement EIP-4844 #2349

Merged
merged 199 commits into from
Jan 19, 2023
Merged

Implement EIP-4844 #2349

merged 199 commits into from
Jan 19, 2023

Conversation

acolytec3
Copy link
Contributor

@acolytec3 acolytec3 commented Oct 12, 2022

This PR implements EIP-4844

  • - Add new transaction type for EIP-4844
  • - Add seralize/deserialize handlers for the new transaction type
    • fromNetworkWrapper - deserialize a full network wrapper transaction (includes blobs, kzgProof)
    • fromSerializedTx - deserialize the "minimal" version (i.e. without kzgProof and blobs)
    • serialize - serializer for "minimal" version
    • serializeNetworkWrapper - serializer for "network wrapper" version (including blobs/kzgProof)
  • - Add new excessDataGas field to block header
  • - Add new HASH opcode to evm
  • - Add transaction validation logic to vm.runTx
  • - Integrate c-kzg or rust equivalent for verifying kzg proofs
    • Add BLS point evaluation precompile - partially implemented - c-kzg currently doesn't expose the verify_kzg_proof and commitment_to_point methods we need to finish this out. Have an open issue on c-kzg to address.
    • Implement full validateBlobNetworkWrapper method that verifies blobs match kzg commitments and proof in network wrapper version
    • Write some tests that proves that it works
  • Add engine_getBlobsBundleV1 RPC endpoint to client
    - [ ] - Run client on interop devnet -- This is partially done. My fork contains dockerFiles and scripts to run our client with lodestar and we have been able to sync a chain and receive blob transactions from eth_sendRawTransaction and then transmit those to lodestar via the engine APIs.
  • Update implementation details according to latest execution API PRs -- mainly to update the tx.hash function to use ssz.serialize
    Some additional references (edit by @holgerd77):
  • EIP-4844 EIP specification
  • EIP-4844 calls (Implementers/Breakout/ACD)
  • EIP-4844 consensus-specs
  • EIP-4844 execution engine API

@codecov
Copy link

codecov bot commented Oct 12, 2022

Codecov Report

Merging #2349 (24ae6bd) into master (3ec8845) will decrease coverage by 1.88%.
The diff coverage is 82.14%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 90.10% <100.00%> (+0.59%) ⬆️
blockchain 90.02% <83.33%> (-0.02%) ⬇️
client 87.90% <87.77%> (-0.01%) ⬇️
common 95.82% <100.00%> (-0.08%) ⬇️
devp2p 91.70% <ø> (+0.08%) ⬆️
ethash ∅ <ø> (∅)
evm 80.06% <100.00%> (?)
rlp ?
statemanager 89.61% <ø> (ø)
trie 90.53% <ø> (+0.17%) ⬆️
tx 93.71% <81.61%> (-4.10%) ⬇️
util 84.52% <ø> (-0.59%) ⬇️
vm 84.12% <56.09%> (-1.73%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Short comment on first look

packages/block/src/block.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

looks really good 👍 just may be a couple of more cleanups away from merge

@acolytec3 acolytec3 marked this pull request as ready for review January 17, 2023 01:59
@acolytec3
Copy link
Contributor Author

Okay, I think everything is basically done at this point. I don't have code coverage completely up to the 88% range but it's pretty close (upper 70s).

@g11tech
Copy link
Contributor

g11tech commented Jan 19, 2023

@acolytec3 once we restore the shanghai.json's eip list, we are should be good to merge this! 👍

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm!

@acolytec3 acolytec3 merged commit 2036b55 into master Jan 19, 2023
@holgerd77
Copy link
Member

LGTM. Tschok, tschok.

Merged!

Wow, you did it. You guys are really brave!

Congrats Andrew, Gajinder. Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants