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

Builder updates for Blobs (EIP-4844) #3808

Closed
wants to merge 21 commits into from

Conversation

jimmygchen
Copy link
Member

@jimmygchen jimmygchen commented Dec 15, 2022

Issue Addressed

#3689

Builder spec: ethereum/builder-specs#61
4844 Builder flow: https://hackmd.io/@jimmygchen/B1dLR74Io

Proposed Changes

Builder updates for blobs. Implemented based on builder spec PR here.

  • Implement BuilderBid variants for Eip4844
  • Handle get_payload response containing kzg_blob_commitments (Eip4844 variant of BuilderBid)
  • Handle new propose_blinded_beacon_block response containing blobs sidecar
  • Fix BuilderBid deserialization issues. See tests.
  • Add mock execution layer tests
  • Code cleanup: lots of clone() & unwrap were added to make things compile.

@jimmygchen jimmygchen marked this pull request as draft December 15, 2022 07:31
@jimmygchen jimmygchen changed the title Builder updates for Blobs Builder updates for Blobs (EIP-4844) Dec 15, 2022
# Conflicts:
#	beacon_node/http_api/src/lib.rs
@@ -135,10 +134,10 @@ impl BuilderHttpClient {
}

/// `POST /eth/v1/builder/blinded_blocks`
pub async fn post_builder_blinded_blocks<E: EthSpec>(
pub async fn post_builder_blinded_blocks<E: EthSpec, Payload: DeserializeOwned>(
Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking to create a trait for ExecutionPayload and ExecutionPayloadAndSidecar to restrict the bound, but having some issues with getting this to compile

pub trait BuilderExecutionPayloadResponse: DeserializeOwned {}
impl<T: EthSpec> BuilderExecutionPayloadResponse for ExecutionPayloadAndBlobsSidecar<T> {}
impl<T: EthSpec> BuilderExecutionPayloadResponse for ExecutionPayload<T> {}

@jimmygchen jimmygchen marked this pull request as ready for review January 11, 2023 06:43
@jimmygchen
Copy link
Member Author

jimmygchen commented Jan 11, 2023

@realbigsean I have implemented the most of the changes we discussed earlier. I'm now trying to work on the tests, but would be helpful if you could review the changes I have so far when you have some time.

# Conflicts:
#	beacon_node/execution_layer/src/lib.rs
#	beacon_node/http_api/src/publish_blocks.rs
#	lcli/src/new_testnet.rs
# Conflicts:
#	beacon_node/execution_layer/src/lib.rs
#	beacon_node/execution_layer/src/test_utils/mock_builder.rs
#	beacon_node/http_api/src/publish_blocks.rs
#	common/eth2_network_config/src/lib.rs
#	consensus/types/src/execution_payload.rs
@jimmygchen
Copy link
Member Author

This one is getting quite outdated, new draft PR created here:

#4345

@jimmygchen jimmygchen closed this May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants