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

BeaconNode <--> ValidatorClient API - Messages #1011

Closed
spble opened this issue Apr 30, 2019 · 26 comments
Closed

BeaconNode <--> ValidatorClient API - Messages #1011

spble opened this issue Apr 30, 2019 · 26 comments
Labels

Comments

@spble
Copy link
Contributor

spble commented Apr 30, 2019

ETH2.0 Beacon Node & Validator Client RPC Proposal

Outline

This issue proposes a minimal communications specification between a beacon node and validator client, targeting phase 0 of the Eth2.0 specification. The protocol is designed to be a simple local communications interface, permitting two separate binaries to communicate.

This specification is intended to describe communication abstractly, without choosing any particular protocol. The protocol used (e.g. gRPC/JSON-RPC) is discussed in a separate issue: #1012.

This issue follows on from a discussion during the Client Architecture session at the Sydney Implementers meeting. This also follow from the Client Architecture Roundtable in Prague.
There is an editable version of this document, here: https://hackmd.io/r7D61hs4RWKm8nz_O2iinQ

Background

The beacon node maintains the state of the beacon chain by communicating with other beacon nodes in the Ethereum Serenity network. Conceptually, it does not maintain keypairs that participate with the beacon chain.

The validator client is conceptually a separate entity which utilises private keys to perform validator related tasks on the beacon chain, which we call validator "duties". This includes the production of beacon blocks and signing of attestations.

Since it is recommended to separate these concerns in the client implementations, it is necessary for us to clearly define the communication between them.

The goal of this specification is to promote interoperability between beacon nodes and validator clients derived from different projects. For example, the validator client from Lighthouse, could communicate with a running instance of the beacon node from Prysm.

This proposal has been adapted from the Lighthouse gRPC protocol specification.

There is also another WIP proposal for a Minimum Validator Interface, which describes additional functions possibly necessary for phase 1 and beyond.

Specification

Entities

The following are the two entities that participate in this protocol:

  • BeaconNode:
    A beacon node instance, run with a --rpc flag to enable the RPC interface. Runs stand-alone.

  • ValidatorClient:
    A validator client instance, which must connect to at least one instance of BeaconNode.

Endpoints

This section summarises API endpoints which are published by an instance of BeaconNode, for the exclusive use of ValidatorClient implementations.

This proposal is a minimum set of messages necessary to enable effective communication, without any extra features. Anything extra is beyond the scope of this document.

Summary Table

Name Type Parameters Returns
get_client_version GET N/A client_version
get_genesis_time GET N/A genesis_time
get_syncing_status GET N/A syncing_status
get_duties GET validator_pubkeys syncing_status, current_version, [ValidatorDuty]
produce_block GET slot, randao_reveal beacon_block
publish_block POST beacon_block N/A
produce_attestation GET slot, shard indexed_attestation
publish_attestation POST indexed_attestation N/A

Status Codes

For each of these endpoints the underlying transport protocol should provide status codes. Assuming this will be based on HTTP, one of the following standard status codes will always be included as part of a response:

Code Meaning
200 The API call succeeded.
40X The request was malformed.
500 The BeaconNode cannot complete the request due to an internal error.
503 The BeaconNode is currently syncing, try again later. A call can be made to get_syncing_status to in order to find out how much has been synchronised.

get_client_version

Requests that the BeaconNode identify information about its implementation in a format similar to a HTTP User-Agent field.

  • Parameters: N/A
  • Returns:
Name Type Description
client_version bytes32 An ASCII-encoded hex string which uniquely defines the implementation of the BeaconNode and its current software version.

Note: Unlike most other endpoints, get_client_version does not return an error 503 while the BeaconNode is syncing, but instead returns status code 200.

get_genesis_time

Requests the genesis_time parameter from the BeaconNode, which should be consistent across all BeaconNodes that follow the same beacon chain.

  • Parameters: N/A
  • Returns:
Name Type Description
genesis_time uint64 The genesis_time, which is a fairly static configuration option for the BeaconNode.

Note: Unlike most other endpoints, get_genesis_time does not return an error 503 while the BeaconNode is syncing, but instead returns status code 200.

get_syncing_status

Requests the BeaconNode to describe if it's currently syncing or not, and if it is, what block it is up to. This is modelled after the Eth1.0 JSON-RPC eth_syncing call.

  • Parameters: N/A
  • Returns:
Name Type Description
syncing false OR SyncingStatus Either false if the node is not syncing, or a SyncingStatus object if it is.

Note: Unlike most other endpoints, get_syncing_status does not return an error 503 while the BeaconNode is syncing, but instead returns status code 200 with the SyncingStatus object.

get_duties

Requests the BeaconNode to provide a set of “duties”, which are actions that should be performed by ValidatorClients. This API call should be polled at every slot, to ensure that any chain reorganisations are catered for, and to ensure that the currently connected BeaconNode is properly synchronised.

  • Parameters:
Name Type Description
validator_pubkeys [bytes48] A list of unique validator public keys, where each item is a 0x encoded hex string.
  • Returns:
Name Type Description
current_version bytes4 The current_version, as described by the current Fork.
validator_duties [ValidatorDuty] A list where each item is a custom ValidatorDuty object.

produce_block

Requests a BeaconNode to produce a valid block, which can then be signed by a ValidatorClient.

  • Parameters:
Name Type Description
slot uint64 The slot for which the block should be proposed.
randao_reveal bytes The ValidatorClient's randao reveal value.
  • Returns:
Name Type Description
beacon_block BeaconBlock A proposed BeaconBlock object, but with the signature field left blank.

publish_block

Instructs the BeaconNode to publish a newly signed beacon block to the beacon network, to be included in the beacon chain.

  • Parameters:
Name Type Description
beacon_block BeaconBlock The BeaconBlock object, as sent from the BeaconNode originally, but now with the signature field completed.
  • Returns: N/A

produce_attestation

Requests that the BeaconNode produce an IndexedAttestation, with a blank signature field, which the ValidatorClient will then sign.

  • Parameters:
Name Type Description
slot uint64 The slot for which the attestation should be proposed.
shard uint64 The shard number for which the attestation is to be proposed.
  • Returns:
Name Type Description
indexed_attestation IndexedAttestation An IndexedAttestation structure with the signature field left blank.

publish_attestation

Instructs the BeaconNode to publish a newly signed IndexedAttestation object, to be incorporated into the beacon chain.

  • Parameters:
Name Type Description
indexed_attestation IndexedAttestation An IndexedAttestation structure, as originally provided by the BeaconNode, but now with the signature field completed.
  • Returns: N/A

Data Structures

Two new data objects are proposed for the sake of implementation, and several other data objects from the Eth2.0 specs are referenced.

The bytes data types are encoded hex strings, with 0x preceeding them. uint64 are decimal encoded integers, and None may be null, which is distinct from 0.

ValidatorDuty

{
    
    # The validator's public key, uniquely identifying them
    'validator_pubkey': 'bytes48',
    # The index of the validator in the committee
    'committee_index': 'uint64',
    # The slot at which the validator must attest.
    'attestation_slot': 'uint64',
    # The shard in which the validator must attest
    'attestation_shard': 'uint64',
    # The slot in which a validator must propose a block. This field can also be None.
    'block_production_slot': 'uint64' or None
}

SyncingStatus

As described by the Eth1.0 JSON-RPC.:

{
    # The block at which syncing started (will only be reset, after the sync reached his head)
    'startingBlock': 'uint64',
    # The current block
    'currentBlock': 'uint64',
    # The estimated highest block, or current target block number
    'highestBlock': 'uint64'    
}

Fork

As described by Fork in the Eth2.0 specs.

BeaconBlock

As described by BeaconBlock in the Eth2.0 specs.

IndexedAttestation

As described by IndexedAttestation in the Eth2.0 specs.

Optional Extras

Endpoint: get_fork

Requests the BeaconNode to provide which fork version it is currently on.

  • Parameters: N/A
  • Returns:
Name Type Description
fork Fork Provides the current version information for the fork which the BeaconNode is currently following.
chain_id uint64 Sometimes called the network id, this number discerns the active chain for the BeaconNode. Analagous to Eth1.0 JSON-RPC net_version.
@GregTheGreek
Copy link
Contributor

This is great!

For get_duties i would add that array order must be maintained.

@ralexstokes
Copy link
Member

another option is that we define the interfaces such that the beacon node returns everything the validator client provides in the request, such that the response is a strict superset of the request.

this is kind of the route that (afaiui) graphQL takes, where a server will "fill in the blanks" for a given client.

i would argue this is helpful for a few reasons (probably others):

  1. debuggability -- the complete packet of info can be in one place, rather than having to manually thread a request and response message
  2. safety -- this came to mind after reading: BeaconNode <--> ValidatorClient API - Messages #1011 (comment); a payload of "duties" lacking the full context is dangerous imo. as a validator, i should know for which pubkeys i should perform which duties... the current suggestion assumes i'll keep an ordered list somewhere that i can check against which seems error prone in the case that (i) nodes cycle, (ii) i have a bug in my list-checking code, etc.
  3. backwards compatibility -- the fewer constraints we place on the API today, the less change will be necessary in the future

i think this notion of safety is pretty important so i would extend the design principle across every API (and really this stands for any RPC we will define for the whole endeavor) -- the only downside is the additional communication overhead but i don't see it being that major

@Mikerah
Copy link
Contributor

Mikerah commented May 1, 2019

Can you put this issue into a PR? It would make it much easier to review and add comments directly to this spec. Great work!

@GregTheGreek
Copy link
Contributor

GregTheGreek commented May 1, 2019

@ralexstokes I think this is probably better to ask existing teams from eth1.x what would make sense (ethers.js, web3.js,... Etc)

I'll forward this off to my tooling group and see if they have Any input.

Edit; I realize this isn't beacon chain RPC as I thought... Lol none the less I'll pass it along

@spble
Copy link
Contributor Author

spble commented May 1, 2019

Can you put this issue into a PR? It would make it much easier to review and add comments directly to this spec. Great work!

Thanks! I was originally going to do it as a PR, and happy to do so, I'm just not sure of a sensible place for the doc to reside. Open to suggestion!

@Mikerah
Copy link
Contributor

Mikerah commented May 1, 2019

Perhaps, you can place it under specs/validator?

@hwwhww hwwhww added the general:RFC Request for Comments label May 1, 2019
@GregTheGreek
Copy link
Contributor

Are we sure we want to be using indexedAttestations? rather Attestation or AttestationData

@wemeetagain
Copy link
Contributor

I think the get_duties call could/should be split out into proposer assignment and committee assignment.

It seems like the committee only needs to be checked once per epoch (and can be looked up one epoch beforehand), whereas the block production assignment must be checked every slot.

I'm thinking something that mirrors the two functions here: https://github.com/ethereum/eth2.0-specs/blob/dev/specs/validator/0_beacon-chain-validator.md#validator-assignments

@mpetrunic
Copy link
Contributor

mpetrunic commented May 3, 2019

How about introducing namespacing for methods (similar to eth1)?
Since in the future there will be a lot more methods exposed from beacon node (for block explorers for example) and it will be hard to differentiate purpose.
Some proposal would be:

  • beacon_get_client_version
  • beacon_get_genesis_time
  • beacon_get_syncing_status
  • validator_get_duties
  • validator_produce_block
  • validator_publish_block
  • validator_produce_attestation
  • validator_publish_attestation

@wemeetagain
Copy link
Contributor

wemeetagain commented May 4, 2019

produce_block is a very high level method that incorporates many component pieces.
Does anyone have thoughts about exposing a lower level interface that allows the validator to create a block itself?
Likewise with produce_attestation and letting the validator create an Attestation according to the steps here: https://github.com/ethereum/eth2.0-specs/blob/dev/specs/validator/0_beacon-chain-validator.md#construct-attestation

@FrankSzendzielarz
Copy link
Member

Regarding the actual message format, I'd suggest only little things, like keeping consistent datatypes - so get syncing status would be more useful if it just returned the syncing status, which could include the info that it was not syncing, or simply return null. Returning false or object makes implementation a bit more complicated as you have to check if you get Bool or Object.

Also - I am not up to speed on the actual Eth 2.0 stuff but are there cases for PUT as well as POST here?

Finally, (referring to the other issue on the actual protocol), I'd suggest a REST-like structure of the messages and roles (eventually one node may serve multiple APIs not just this beacon one):

https://beaconapi20190506111547.azurewebsites.net/

@spble
Copy link
Contributor Author

spble commented May 7, 2019

Are we sure we want to be using indexedAttestations? rather Attestation or AttestationData

@GregTheGreek - I think this was resolved on Gitter. IndexedAttestation is easiest because it also includes the size of the committee, which is necessary for formulating the custody bits.

It could also be possible for the BeaconNode to send AttestationData, then the ValidatorClient return IndexedAttestation, however to keep things simple and symmetric, we chose to just keep it is as an IndexedAttestation in both directions.

@spble
Copy link
Contributor Author

spble commented May 7, 2019

I think the get_duties call could/should be split out into proposer assignment and committee assignment.

@wemeetagain - I think our plan is to check both pieces of information every slot, since both the committee and block production could theoretically change at any point due to forks.

produce_block is a very high level method that incorporates many component pieces.
Does anyone have thoughts about exposing a lower level interface that allows the validator to create a block itself?

We decided to keep the API minimal, and keep functionality in the ValidatorClient only to what is absolutely necessary. Your proposal to have lower level interfaces which allow the ValidatorClient to create blocks itself requires a great deal more functionality be moved into the ValidatorClient. So that we don't need to have all the heavy logic in the ValidatorClient, such as committee selection, etc., we decided to keep that all in the BeaconNode, and have it produce the block. That way the ValidatorClient only needs to check that it won't be slashed, then sign and return the data.

@spble
Copy link
Contributor Author

spble commented May 7, 2019

How about introducing namespacing for methods (similar to eth1)?

@mpetrunic - This is a fantastic idea. I will introduce namespacing in my next revision of this API proposal.

@spble
Copy link
Contributor Author

spble commented May 7, 2019

Regarding the actual message format, I'd suggest only little things, like keeping consistent datatypes - so get syncing status would be more useful if it just returned the syncing status, which could include the info that it was not syncing, or simply return null. Returning false or object makes implementation a bit more complicated as you have to check if you get Bool or Object.

@FrankSzendzielarz - This is a good point, I will keep the data types of the return values consistent in my next revision of the proposal.

I also agree that HTTP-REST is a better choice than JSON-RPC, which is discussed in more detail in #1012.

@spble
Copy link
Contributor Author

spble commented May 7, 2019

Perhaps, you can place it under specs/validator?

Good idea @Mikerah.

@djrtwo - Do you think it makes sense to submit a PR with a Markdown RFC in specs/validator?
If so, I'll go ahead and merge the comments/discussions from this thread and #1012 into that.

@paulhauner
Copy link
Contributor

How about introducing namespacing for methods (similar to eth1)?

@mpetrunic - This is a fantastic idea. I will introduce namespacing in my next revision of this API proposal.

If we roll with REST-HTTP (#1012) we can achieve this with urls (e.g., /beacon/foo, /validator/bar).

@FrankSzendzielarz
Copy link
Member

FrankSzendzielarz commented May 7, 2019

@spble @djrtwo "Do you think it makes sense to submit a PR with a Markdown RFC in specs/validator?
If so, I'll go ahead and merge the comments/discussions from this thread and #1012 into that."

Let me know but if you want I can fine tune the swagger and give you the auto generated MD from the swagger codegen. Have a look at the docs folder in the golang zip in #1012 if it suits. (or the attachment here)
docs.zip

In fact, @djrtwo if the Swagger http approach works, could do all specs as actual code in a test web first, and auto-gen the MD on the basis of the code.

@FrankSzendzielarz
Copy link
Member

@mpetrunic
Copy link
Contributor

@mpetrunic eg https://beaconapi20190506111547.azurewebsites.net/swagger/ui/index

I would pull validator endpoints under root /validator (remove beacon prefix). Also block and attestation endpoints should go under validator root. Mainly because those routes are not going to be used by general public (etherjs, block explorers etc) so there is no need for people to scroll trough them.

@FrankSzendzielarz
Copy link
Member

FrankSzendzielarz commented May 7, 2019

@mpetrunic I can change that to reflect what is agreed on here. I am not familiar with the actual spec but it strikes me that for at least testnets you will have single implementations running either as Beacon or Validator - in which case I think the above API is the api the Beacon would be offering. If this is the case , I'd say the beacon prefix should remain in place. (but yeah - whatever is appropriate)

@mpetrunic
Copy link
Contributor

@FrankSzendzielarz Let me give you some example of what I meant:
GET /beacon/Block - should return latest(head) beacon block
GET /validator/Block - should produce block without signature

Currently only these methods belong in beacon root :
GET /beacon/client/version
GET /beacon/client/genesistime
GET /beacon/client/syncstatus

@FrankSzendzielarz
Copy link
Member

@mpetrunic Ah I see. Yes that makes sense.

spble added a commit to sigp/consensus-specs that referenced this issue May 8, 2019
@mpetrunic
Copy link
Contributor

@spble During implementation I noticed some "bottlenecks". I think we should add method validator_get_validator_index(publicKey) and in get_duties we expect validatorIndex instead of public key.
Getting index from public key is relatively expensive operation as it requires O(n) search trough validator registry (> 65536 records). Of course beacon chain could cache publicKey<->index mapping but still it probably makes much more sense for validator to obain his index on start and store it in memory.

@paulhauner
Copy link
Contributor

@mpetrunic good point! I have two potentially interesting thoughts around this:

Searching for pubkeys in the validator registry

Lighthouse also found this O(n) search problem whilst bench-marking deposits processing at large numbers of validators (300k, 4M). When processing a Deposit, you need to do the same search through the validator_registry to see if the validator already exists. We found this was massively blowing-up our block processing times so we ended up including a public key cache with our BeaconState.

As a result, we have a map hanging around to do O(1) lookups of pubkey -> validator index which suits this purpose perfectly.

ID'ing validators via pubkey vs. validator index

There is an argument that we could use the validator_index as a canonical reference to a specific validator, but I'm a little cautious of it because it seems like we might face edge-cases or design decisions that break the invariant that it's impossible for the same validator_index to point to a different pubkey.

Given that a pubkey is a really solid unique-ID for a validator, I'm tempted to rely upon it. IIRC @djrtwo was leaning towards validator_index so maybe he'd like to throw a hat in the ring.

Of course, I'm keen to hear other's thoughts too.

@spble
Copy link
Contributor Author

spble commented May 13, 2019

Since consensus in #1012 was to use a REST API, I have written up a new proposal using the OpenAPI specification. I have tried to incorporate all the suggestions in this thread, though the identification of validators via pubkey vs index remains an open question; I have left it as pubkey for now.

I've opened a PR #1069 which contains the proposed spec, so I will close this issue for now and we can discuss/refine the spec further on the PR.

A 'live' version of the API spec can be viewed on SwaggerHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants