Validator manager move should respect suggested fee recipient

[LefterisJP]

Description

I tried the awesome validator manager that comes with lighthouse (https://lighthouse-book.sigmaprime.io/validator-manager-move.html). Specifically the move command as it is much better than some custom scripts I had. It works fine but has one important flaw.

It moves the validator from one VC to another successfully but it ignores the suggested_fee_recipient that's inside the validator_definitions.yml for the validator. It moves it without any fee recipient. So for VCs that run validators for multiple people/groups, each with different fee recipient the moving is incomplete in the sense that all fee recipients configurations are not moved and default to the default one given by the CLI.

You can provide a suggested-fee-recipient for a group of validators when you move them through the tool but that is still too manual. Detecting the source fee recipient and entering it in the target validator definitions yml should be done.

Version

5.1.1. stable.

Present Behaviour

validator's suggested fee recipient is not moved

Expected Behaviour

it should be 🐇

Miscellaneous

The validator manager could also get some extra utility functions. Such as editing a validator (or validators) fee recipient and other editablefields. Deleting validators. And also querying a VC's validators. All things that can be done via the API but having the manager do it would be useful. Can open other issues for those if you believe it a valid request.

[michaelsproul]

I think maybe the best way to make move respect the fee-recipient would be to add the fee recipient as a non-standard field to the DELETE response, which is already non-standard. When we use move we are (unfortunately) already straying outside the standard KM spec because we obtain the whole keystore via DELETE. This is something the KM spec intentionally does not allow (for security) although it might be good to change that.

Some extra validator manager functionality is underway already in:

• Validator manager commands for standard key-manager APIs #5347

There's probably more that could be added, like fee recipients, etc. We have a broad issue for this here:

• Validator manager commands for standard key-manager APIs #4854

[LefterisJP]

Hey @michaelsproul thanks for the response!

I think maybe the best way to make move respect the fee-recipient would be to add the fee recipient as a non-standard field to the DELETE response, which is already non-standard. When we use move we are (unfortunately) already straying outside the standard KM spec because we obtain the whole keystore via DELETE. This is something the KM spec intentionally does not allow (for security) although it might be good to change that.

Hmm why non-standard? I have some scripts which I trust less than your code but will keep maintaining them and improving them which use the keystore API.

Isn't it possible by essentially for every validator doing the following during the move?

In the source VC: GET /eth/v1/validator/{pubkey}/feerecipient to get the fee recipient saved (before deleting)
In the destination VC: POST /eth/v1/validator/{pubkey}/feerecipient to set the fee recipient we got in the above step right after moving and before finalizing the move?

Some extra validator manager functionality is underway already in:

Cool! I mean we can do almost everything via the API. But this code is so sensitive and important that having a standard tool in lighthouse to do so would be amazing.

[michaelsproul]

Hmm why non-standard?

Using the standard fee-recipient APIs as you suggest would be feasible, but it wouldn't be atomic. If we use non-standard fields in DELETE we could read atomically, however we would need a non-standard POST too, so maybe it's not worth it. Probably better to use the standard API as you suggest

[chong-he]

@PacoBits from Discord reported that the graffiti field is also not moved to the destination node