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

Engine API: break down the spec by fork scopes #327

Merged
merged 4 commits into from
Dec 20, 2022

Conversation

mkalinin
Copy link
Collaborator

In anticipation of changes proposed in #321 this PR decouples Shanghai spec from Paris.

I would also suggest we clean up the transition code in a separate PR. Currently, newPayloadV2 and forkchoiceUpdatedV2 specs are based on the corresponding V1 versions. To get rid of transition logic we would need the new spec to redefine the following parts:

cc @ralexstokes

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

this works for me

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

I'm ambivalent on this. It seems a bit nicer from the perspective of an implementer, but from an observer it is sort of frustrating that the methods are now defined by fork. This is rarely the behavior observers expect (and one of my biggest complaints of the beacon spec). They end up jump through various forks until they find the method they need.

Maybe we could resolve that by having a better index of all the methods and that would be the best of both worlds. It would be nice to consider a different approach though where methods are grouped more by functionality instead and the hardfork-relevant information is noted within the method's specification.

@mkalinin
Copy link
Collaborator Author

Maybe we could resolve that by having a better index of all the methods and that would be the best of both worlds. It would be nice to consider a different approach though where methods are grouped more by functionality instead and the hardfork-relevant information is noted within the method's specification.

The proposal #321 suggests to use a reference table of all methods, we may add a Description column to this table describing the origins of this or that method to make navigation easy. I can see how functionality approach is more practical in some ways, the HF-base approach has natural points of solidifying a portion of spec, and discerning solidified part from yet experimental one. Let's debate on this, I am fully open to any solution which would be convenient for spec writers, devs and observers.

@mkalinin
Copy link
Collaborator Author

mkalinin commented Dec 9, 2022

Updated this PR with decoupling Paris spec from the main spec document. So, Engine API spec employs the following structure:

execution-apis/src/engine
├─ common.md
├─ paris.md
├─ shanghai.md
├─ blob-extension.md

cc @djrtwo

@mkalinin mkalinin changed the title Engine API: decouple shanghai spec Engine API: break down the spec by fork scopes Dec 9, 2022
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

I'm still slightly against the fork-scope format, but in the interest of moving things forward, the PR looks acceptable!

@mkalinin
Copy link
Collaborator Author

I have added src/engine/experimental/ and placed blob-extension.md into this folder, cc @protolambda

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.

3 participants