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: define Open RPC schema #379

Merged
merged 9 commits into from
Mar 27, 2023

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Feb 10, 2023

Defines initial OpenRPC schema for Engine API. Draft PR opened up for review, once agreed in general getPayloadBodies requests should be added before merging.

Folder structure

schemas/ and methods/ are both put into src/engine/openrpc/ to avoid messing Open RPC schema files with method specification files which are main artifacts of src/engine part of this repo.

Schema files follows functional breakdown as it should be easier to read and maintain it this way. I tried to play with functional & version breakdown, like payload_v1.yaml, payload_v2.yaml, and didn't find version breakdown useful at all.

Spellchecker

This PR makes spellchecker case insensitive for .yaml files, this is done merely to avoid adding words respecting the case to the exception list. If case sensitivity for yamls is important by any mean then we'll have to add those words explicitly.

External docs

Tried to play out with externalDocs property. Unfortunately, it isn't rendered by OpenRPC playground at all. But I decided to leave this property as it might be useful. Currently it contains full URL to specification of each method.

Misc

TBH, I am not satisfied with deduplication framework of JSON schema. I was expecting to define ExecutionPayloadV2 by giving a command to use ExecutionPayloadV1 property set with withdrawals field addition. Apparently, it's not allowed. Though, I am not an expert and it might be possible to do, lmk.

ToDo

  • Add definition of payload bodies requests
  • Add tests

@mkalinin mkalinin marked this pull request as ready for review February 27, 2023 10:24
@lightclient
Copy link
Member

lightclient commented Mar 8, 2023

I am not satisfied with deduplication framework of JSON schema

Were you able to try using allOf? This is how signed transactions are able to reuse the unsigned format. See here.

--

From a high level view, this PR generally seems good. No way to really know how correct the schema is without some tests, but not sure if you want to add that here. I worked on a tool rpctestgen which makes it pretty easy to update tests for the eth RPC. I think something similar would be useful for the engine api. Although the regeneration is less of an issue because breaking should require a version bump of the method or object.

Would be great have sample messages to sanity check the spec, regardless of how they are acquired though. Even just listening in over some EL<>CL interactions and snagging a couple unique method calls would probably suffice for now.

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.

Noticed a couple things while reviewing manually

src/engine/openrpc/schemas/payload.yaml Outdated Show resolved Hide resolved
src/engine/openrpc/schemas/payload.yaml Outdated Show resolved Hide resolved
src/engine/openrpc/schemas/payload.yaml Outdated Show resolved Hide resolved
mkalinin and others added 3 commits March 9, 2023 17:13
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
@lightclient lightclient merged commit bf134c9 into ethereum:main Mar 27, 2023
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