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

draft beacon GET registrations endpoint #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ciaranmcveigh5
Copy link

@ciaranmcveigh5 ciaranmcveigh5 commented Aug 23, 2022

What does this PR do?

This PR adds a GET request endpoint of "/eth/v1/beacon/validator_registrations" (endpoint still TBD).

The need for this endpoint is to enable distributed validators to efficiently integrate with the builder api.

Currently a validator registers to the builder network via a POST call with a SignedValidatorRegistrationV1 payload. Within SignedValidatorRegistrationV1 is ValidatorRegistrationV1.

class ValidatorRegistrationV1(Container):
    fee_recipient: ExecutionAddress
    gas_limit: uint64
    timestamp: uint64
    pubkey: BLSPubkey

One key field in this payload is the pubkey. In the consensus clients, this pubkey field is automatically set to the validator key. While this is fine in a normal validator set up it isn't with distributed validators.

This is because the validator key is a pubkeyshare of the aggregate pubkey the network will see.

This poses 2 problems say we have a 2 of 3 Distibuted validator with pubkeyshares 0xa, 0xb, 0xc and an aggregate pubkey 0xf.

For DV middlewares to aggregate the pubkeyshare signature the signed payloads must be identicial. In the above scenario they won't be, one will have pubkey=0xa, another pubkey=0xb etc.

Even if we were able to get them to agree on signing a specific payload with pubkey=0xa this would still be an issue. The pubkey 0xa is not what the Ethereum network sees when a registration is sent. It instead see signatures from the aggregate pubkey 0xf. The relays do some verification against the signature and the pubkey in the payload to verify a registration is valid.

The relay would see a signature from 0xf but within the payload the pubkey would be set to 0xa.

As a result we need all registration payloads to use the aggregate pubkey as the pubkey field.

A simple way to achieve this is via a GET endpoint for validator registrations where beacon can take note of all the validators it has access to and generate an array of ValidatorRegistrationV1. This is return to the Validator client which signs it and sends it to the network.

(something about what corver said in terms of this aligning with the current relationship between VC's and beacons ie that a beacon is a trusted data source for the VC)

If Consensus clients don't want to implement this a the default method to return network load it can be enabled with a --get-registrations-from-beacon flag

(Do we mention the current work arounds?) proposerConfig.json validator_definitions.yml - non standard, only possible with VC's that enable configuring individual validators with registration overrides

application/json:
schema:
title: ValidatorRegistrationV1
type: array
Copy link
Author

Choose a reason for hiding this comment

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

I was thinking about making this a specific endpoint with /$validator at the end however all the clients submit the registrations in one big bundle so I thought it made sense to just return an array of registrations for all validators associated with that beacon

@@ -97,6 +97,8 @@ paths:
$ref: "./apis/beacon/pool/sync_committees.yaml"
/eth/v1/beacon/pool/voluntary_exits:
$ref: "./apis/beacon/pool/voluntary_exists.yaml"
/eth/v1/beacon/pool/validator_registration:
Copy link
Author

Choose a reason for hiding this comment

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

Where should this endpoint be, don't know if it fits well into pool

could create a new folder called builder and it would be

/eth/v1/beacon/builder/validator_registration

depends on what the eth guys want

@@ -0,0 +1,21 @@
get:
operationId: getValidatorRegistrationV1
Copy link
Author

Choose a reason for hiding this comment

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

registrations rather than registration

used ValidatorRegistrationV1 since it's the exact spec name

content:
application/json:
schema:
title: ValidatorRegistrationV1
Copy link
Author

Choose a reason for hiding this comment

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

same here registration or registrations

@@ -267,6 +269,8 @@ components:
ConsensusVersion:
enum: [phase0, altair, bellatrix]
example: "phase0"
ValidatorRegistration:
$ref: './types/registration.yaml#/ValidatorRegistration'
Copy link
Author

Choose a reason for hiding this comment

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

schema already exists in registration.yaml

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

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.

1 participant