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

Beacon Node API for Validator #1069

Merged
merged 19 commits into from May 28, 2019

Conversation

Projects
None yet
7 participants
@spble
Copy link
Contributor

commented May 9, 2019

This PR proposes an OpenAPI specification for communication between the beacon node and the validator client. This is intended to be as minimal a specification as possible, just enough to allow the validator client implementation to function without any extra features.

This PR formalises the discussions in issues #1011 and #1012.

This RFC/proposal creates an OpenAPI yaml file inside the specs/validator folder which is discussed by a short markdown document in the same folder. A link to this markdown document has been placed in the primary README.md file for the repository.

If you'd like to explore the API proposal directly, please take a look on SwaggerHub

spble added some commits May 8, 2019

Started updating the markdown description of the BNVC REST API, remov…
…ing stuff specific to the issue and conforming to standard text layout.
Fleshed out a lot more of the API, nearly ready.
 - Added all the fields from BeaconBlock(Body)
 - Tagged all paths as 'Minimum for validator'
 - Removed BeaconNode and ValidatorClient conventions
 - Moved the basic non-object schema components to the top
 - Broke out common beacon block properties into the BeaconBlockCommon object
 - Fixed links to Eth2.0 spec
Formatting clean up.
 - Moved /node/fork up with other node endpoints
 - Added descriptions and ordering to tags
 - Removed common merkle_root schema, to be more specific in descriptions.
 - Moved BeaconBlockCommon next to appropriate schemas.
 - Lots of small grammar improvements, full stops at end of descriptions.
Fixed up markdown.
 - Removed TOC
 - Removed all the old spec stuff
 - Uploaded spec to SwaggerHub and provided a link to it.
 - Added a 'license' section to the API description.

@spble spble marked this pull request as ready for review May 13, 2019

@spble spble changed the title [WIP] Beacon Node API for Validator Beacon Node API for Validator May 13, 2019

@hwwhww
Copy link
Contributor

left a comment

It's great to have this yaml file and SwaggerHub doc viewer! 👍👍

A drawback is that we need to maintain both this yaml file and core spec object scheme for the same objects. We need to go through it again when freezing the spec.

Show resolved Hide resolved specs/validator/0_beacon-node-validator-api.md Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml
@arnetheduck

This comment has been minimized.

Copy link
Contributor

commented May 17, 2019

A drawback is that we need to maintain both this yaml file and core spec object scheme for the same objects. We need to go through it again when freezing the spec.

if we formalize the schema format for the spec messages a bit, one can be generated from the other, potentially - that could be useful in the future as well, when defining new messages!

spble added some commits May 20, 2019

Updated API spec with suggestions by @hwwhww.
 - Corrected to use American English instead of Australian
 - Fixed spelling mistake with indices
 - Changed tag to 'MinimalSet' and 'OptionalSet'
 - Added a  response to the list of components
 - Renamed 'block_production' to 'block_proposal'
@spble

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

Thanks very much for your review @hwwhww!

Yes, it is a shame to have it duplicated and would be best to have it generated.
Of course, it is difficult to generate this YAML from the python/markdown, so I am more than happy to maintain this spec and update it after the big freeze!

@shanejonas

This comment has been minimized.

Copy link

commented May 23, 2019

Just wanted to chime in and show that this is possible with JSON-RPC with the OpenRPC Spec.

Here is an example of the playground with the ethereum-json-rpc-specification

It simplifies a lot of what you have already since it follows familiar principles.

@paulhauner

This comment has been minimized.

Copy link
Contributor

commented May 23, 2019

Just wanted to chime in and show that this is possible with JSON-RPC with the OpenRPC Spec.

Thanks. We're not using REST/HTTP just for OpenAPI though (I think we'd use it even if OpenAPI didn't exist). See here for detailed reasoning.

@shanejonas

This comment has been minimized.

Copy link

commented May 23, 2019

REST is great until you want to support other transports. Then it starts to fall apart. JSON-RPC is simple and transport agnostic, and now has the tools to compete.

The arguments outlined (like stateful across multiple calls) only apply to API design and can be avoided.

I’d love to see ETH continue to use JSON-RPC with the rest of the blockchain ecosystem.

@FrankSzendzielarz

This comment has been minimized.

Copy link
Member

commented May 23, 2019

@shanejonas REST APIs (HTTP) are fairly transport agnostic. Can you clarify? Eth 1.x implementers see stateful rpc apis as having been something of a mistake, afaik.

@shanejonas

This comment has been minimized.

Copy link

commented May 23, 2019

With HTTP (and swagger outlined here). When you do want to support websocket, IPC, postMessage/webworker or any other transport in the future you will need yet another spec and debate on how to best describe it.

JSON-RPC doesn't tell you how stateful your API should be. Those are implementation details.

Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml
Show resolved Hide resolved specs/validator/beacon_node_oapi.yaml Outdated

@mpetrunic mpetrunic referenced this pull request May 24, 2019

Merged

Unstub rpc methods #224

spble added some commits May 24, 2019

Addressed some of @djrtwo's suggestions.
 - Rewording of specification goal paragraph
 - Clarify get duties description regarding chain reorgs.
 - Add epoch parameter to get duties, and new error 406
 - Block publishing action is clarified around validation, with a new status code 202
 - The validator pubkey and PoC bit are passed to produce attestation
 - Attestation publishing action is clarified around validation, new status code 202
 - Rewording of genesis_time, 'block' -> 'slot'
 - Update Crosslink to latest spec
 - Added missing signature field to IndexedAttestation
Minor updates.
 - Fixed spelling (and made American English)
 - Clarified the schema for the new poc_bit field, and description.

spble added some commits May 28, 2019

Updated all absolute URLs to the eth2.0-specs repo so that they point…
… to the master branch (instead of dev).
@djrtwo

djrtwo approved these changes May 28, 2019

Copy link
Contributor

left a comment

awesome!
Having some issues with circleci so leaving this unmerged until tomorrow

@djrtwo djrtwo merged commit 4183d84 into ethereum:dev May 28, 2019

1 check failed

ci/circleci: Build Error Your tests failed on CircleCI
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.