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

Add support for Electra #613

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from
Draft

Add support for Electra #613

wants to merge 30 commits into from

Conversation

jtraglia
Copy link
Collaborator

@jtraglia jtraglia commented Apr 29, 2024

📝 Summary

This PR makes the necessary changes for Electra.

TODO:

  • Extend BidTraceV2WithBlobFields with request counts.

⛱ Motivation and Context

📚 References

Other Electra branches of dependencies:


✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy
  • I have seen and agree to CONTRIBUTING.md

@jtraglia jtraglia marked this pull request as ready for review May 9, 2024 14:32
@@ -425,6 +453,8 @@ which is sufficient data to set the bid of the builder. The `Transactions`
and `Withdrawals` fields are required to construct the full SignedBeaconBlock
and are parsed asynchronously.

TODO(JWT): Does this need to be updated? It hasn't been updated for Deneb.
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be separate upgrade added deneb support here #618

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, good idea. Thank you for making that PR. I will remove this comment.

@jtraglia
Copy link
Collaborator Author

I'm working on testing this with kurtosis this week. There are some minor issues. Most importantly, I forgot that we needed to add Electra support to flashbots/builder so simulation works. Will mark as a draft for now.

@jtraglia jtraglia marked this pull request as draft May 15, 2024 18:54
common/utils.go Outdated Show resolved Hide resolved
@@ -878,6 +879,9 @@ func TestCheckSubmissionPayloadAttrs(t *testing.T) {
for _, tc := range cases {
t.Run(tc.description, func(t *testing.T) {
_, _, backend := startTestBackend(t)
backend.relay.capellaEpoch = 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe setting this should be part of startTestBackend

// deneb specific logging
if payload.Deneb != nil {
if payload.Version >= spec.DataVersionDeneb {
blobs, err := payload.Blobs()
Copy link
Collaborator

@avalonche avalonche May 16, 2024

Choose a reason for hiding this comment

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

the purpose of the GetBlockSubmissionInfo struct is so we don't need to extract each field individually and handle the error once. you should be able to access these fields directly e.g. submission.Blobs

return ErrHeaderHTRMismatch
}

if len(bb.Electra.Message.Body.BlobKZGCommitments) != len(payload.Electra.BlobsBundle.Commitments) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

potential to extract to a separate function with BlobKZGCommitments as input

withdrawalsRoot phase0.Root
parentBeaconRoot *phase0.Root
payloadAttributes beaconclient.PayloadAttributes
depositReceiptsRoot phase0.Root
Copy link
Collaborator

Choose a reason for hiding this comment

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

has https://github.com/attestantio/go-eth2-client/blob/master/api/v1/payloadattributesevent.go been updated for latest payload attributes?
If so it can replace the types https://github.com/flashbots/mev-boost-relay/blob/main/beaconclient/prod_beacon_instance.go#L63-L83 which need updating to accept the new payload attributes. You'll also need to update processPayloadAttributes in service.go with the new payload attributes

@avalonche
Copy link
Collaborator

👍 feel free to fork the builder to build images for kurtosis. Let me know if you need me to rebase the builder from the latest geth electra branch

@jtraglia
Copy link
Collaborator Author

Let me know if you need me to rebase the builder from the latest geth electra branch

Yes please. That would be very useful! This is latest geth electra branch:

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.

2 participants