Skip to content

Comments

feat: add electra types#1799

Merged
morph-dev merged 3 commits intoethereum:masterfrom
morph-dev:pectra
May 3, 2025
Merged

feat: add electra types#1799
morph-dev merged 3 commits intoethereum:masterfrom
morph-dev:pectra

Conversation

@morph-dev
Copy link
Collaborator

What was wrong?

We don't have any Pectra types defined.

How was it fixed?

I rebased #1763 and added few more things on top (rebase is first commit, mine is second):

  • Added proper (I think) fork digest
    • I added test that checks that all fork digests are as expected (had to move some stuff around)
  • Added pectra fork as part of the config
  • Added more types to ef-tests
  • Few other smaller changes

Planned work for the followup PRs:

To-Do

@morph-dev morph-dev self-assigned this May 2, 2025
@morph-dev morph-dev requested a review from KolbyML May 2, 2025 21:35
@KolbyML
Copy link
Member

KolbyML commented May 2, 2025

@morph-dev please look at my review of the previous PR here, #1763 (comment)

impl ProcessBeaconBlock for SignedBeaconBlockElectra {
    fn process_beacon_block(&self) -> anyhow::Result<ProcessedBlock> {

is generating invalid execution headers

@morph-dev
Copy link
Collaborator Author

@morph-dev please look at my review of the previous PR here, #1763 (comment)

Yes, that's what I was planning on doing.

However, requests inside BeaconBlockBody are grouped by type, and I'm not sure in which order they should be iterated for execution header. I assume it would be "deposit requests" then "withdrawals requests" then "consolidations requests", but I'm not 100% sure.

Will look into it tomorrow...

@KolbyML
Copy link
Member

KolbyML commented May 2, 2025

@morph-dev please look at my review of the previous PR here, #1763 (comment)

Yes, that's what I was planning on doing.

However, requests inside BeaconBlockBody are grouped by type, and I'm not sure in which order they should be iterated for execution header. I assume it would be "deposit requests" then "withdrawals requests" then "consolidations requests", but I'm not 100% sure.

Will look into it tomorrow...

Here is how it is performed in the consensus-spec https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/beacon-chain.md#new-get_execution_requests_list

@morph-dev
Copy link
Collaborator Author

Thanks for the info! Done.
Ready for review.

Copy link
Member

@KolbyML KolbyML left a comment

Choose a reason for hiding this comment

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

I haven't verified if ForkName::Electra => [0xad, 0x53, 0x2c, 0xeb] is correct but other then that the PR looks good

:shipit:

@morph-dev morph-dev merged commit 07d7047 into ethereum:master May 3, 2025
14 checks passed
@morph-dev morph-dev deleted the pectra branch May 3, 2025 10:17
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