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

Add new validator voluntary exit endpoint #57

Closed
jimmygchen opened this issue Mar 23, 2023 · 11 comments
Closed

Add new validator voluntary exit endpoint #57

jimmygchen opened this issue Mar 23, 2023 · 11 comments

Comments

@jimmygchen
Copy link
Contributor

jimmygchen commented Mar 23, 2023

Description

Add a new validator client API that creates and sign a VoluntaryExit object, and publishes it to the Beacon API. This motivation is to improve UX for validator voluntary exits (as VC have access to the voting keys), and to support initiating exit through additional channels (e.g. Lighthouse's Siren UI).

Security

The operation is potentially dangerous and this endpoint must be secured by the existing VC auth scheme, and ideally additional security on the clients (such as command flags).

Proposed API

NOTE: This below specification is now outdated, please see spec PR #58

Property Specification
Path /eth/v1/validators/voluntary_exits
Method POST
Required Headers Authorization
Typical Responses 200

Example Request Body

{
    "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a"
}

Error responses

  • 401: invalid authorization key
  • 400: invalid public key / request
  • 500: internal server error
@jimmygchen
Copy link
Contributor Author

I'll draft a PR on this shortly.

@rolfyone
Copy link
Collaborator

Would the intent for this to be to only support keys you've already got active in your list? I can kind of see from that perspective how it might live in this stack...

It'd probably fit better if the endpoint conforms, eg.
/eth/v1/validator/(pubkey}/voluntary_exit
Also, the message to the beacon-api POST https://ethereum.github.io/beacon-APIs/#/Beacon/submitPoolVoluntaryExit
has epoch in it, so would need to decide if that's a field that needs to be able to be passed in, maybe as an optional query param.

@rolfyone
Copy link
Collaborator

also just realise this is a one-way operation on a validator key, so if someone 'accidentally' does it on a validator to try, they cant change their minds.. other keymanager operations are less terminal than this one.

@jimmygchen
Copy link
Contributor Author

Hey @rolfyone thanks for the comments!

Would the intent for this to be to only support keys you've already got active in your list? I can kind of see from that perspective how it might live in this stack...

Yep that's right - I think that would enable toolings to improve the UX for voluntary exits, as the validator client is already capable of signing the message, either directly or via an external signer.

It'd probably fit better if the endpoint conforms, eg.
/eth/v1/validator/(pubkey}/voluntary_exit

The intention to use a JSON body is to make it harder to accidentally trigger the endpoint (e.g. via curl), probably the opposite of how you would usually design an endpoint.. does this make sense?

Also, the message to the beacon-api POST https://ethereum.github.io/beacon-APIs/#/Beacon/submitPoolVoluntaryExit
has epoch in it, so would need to decide if that's a field that needs to be able to be passed in, maybe as an optional query param.

I was thinking this endpoint would trigger an immediate request to exit based on the current epoch, which is something VC can figure out from beacon API. Are you suggesting to allow option to schedule an exit at a later time?

also just realise this is a one-way operation on a validator key, so if someone 'accidentally' does it on a validator to try, they cant change their minds.. other keymanager operations are less terminal than this one.

Yep that's right, if this is an issue, the other option is to return a SignedVoluntaryObject but then the user would have to perform some checks themselves (e.g. network match) against the Beacon API when submitting the message. Although the former would make the experience nicer.

@rolfyone
Copy link
Collaborator

rolfyone commented Mar 24, 2023

The epoch is just an attribute of the message, we could make it optionally passed in as a query parameter... If its not specified then fill as current?

I think once confirmed (apis chan discussion about confirm=yes param ongoing) then we can push to the normal beacon-api, potentially signing via external signer whatever the validator flow may be...

Json body I'm not super keen on because it doesn't follow the pattern, and I think the pattern does fit really well to this use-case over all.

so the flow to me:

/eth/v1/validator/{pubkey}/voluntary_exit?confirm=yes
-> sign, submit at current epoch
/eth/v1/validator/{pubkey}/voluntary_exit
-> '400: please confirm you intend to exit this validator, it cannot be undone once the message is submitted!'

if the pubkey is not valid, then a 404 would be an appropriate response...

@jimmygchen
Copy link
Contributor Author

Thanks, I think these are great suggestions!

With invalid pubkeys, I think there could be a few variants:

  • pub key not found on validator client => 404 as you suggested
  • invalid format / not parsable as public key => I think 400 (Bad Request) would be more suitable?

@jimmygchen
Copy link
Contributor Author

Created PR for this in #58

@rolfyone
Copy link
Collaborator

With invalid pubkeys, I think there could be a few variants:

  • pub key not found on validator client => 404 as you suggested
  • invalid format / not parsable as public key => I think 400 (Bad Request) would be more suitable?

yep 404 might be good for something thats at least parsable, ideally if someone uses 'freddy' as the pubkey part of the route it could be a 400, but as long as its 4XX im not strongly opnionated.... It's a chance to give a good error that api implementers would potentially use, we can just declare that both error codes might come up

@yorickdowne
Copy link

Re signing and sending being two separate actions: I can think of a few use cases for this. Pools, where NOs are asked to provide pre-signed exit messages. Solo operators who want to put pre-signed messages into a place easily accessed by heirs, so they can exit without needing to understand node operation.

@jimmygchen
Copy link
Contributor Author

Thanks for the input @yorickdowne ! The benefit of separating them into two actions were brought up in discussions as well, and the endpoint has been spec'd (PR #58) as you described (sign & send being two separate actions) for safety and flexibility reasons, and it should satisfy the use cases you mentioned.

Please ignore the original proposal in description :)

@jimmygchen
Copy link
Contributor Author

PR #58 merged. Closing this issue. Thanks everyone for the valuable inputs!

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

No branches or pull requests

3 participants