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

Set fee recipient per validator #2883

Closed
michaelsproul opened this issue Dec 24, 2021 · 12 comments
Closed

Set fee recipient per validator #2883

michaelsproul opened this issue Dec 24, 2021 · 12 comments
Labels
A0 enhancement New feature or request

Comments

@michaelsproul
Copy link
Member

It would be nice to be able to set the fee recipient on a per-validator basis so that validators sharing a BN need not use the same address

@beetrootkid
Copy link

beetrootkid commented Jan 11, 2022

Can I just check that by 'per validator', you mean validator account rather than validator client? Also, any insight on how / where the validator will store the validator account | fee recipient association?

Just to add a bit more colour - for non-pooling staking providers, this will be an important feature.

@pawanjay176
Copy link
Member

If we require different fee_recipients for different validators from the VC, then I think we would need to change the /validators/beacon/{slot} api to also accept an optional fee_recipient parameter to pass to the beacon node when requesting a block.

@kanewallmann
Copy link
Contributor

This feature is highly desirable for Rocket Pool (and likely for other staking providers). The main use case for us is for users running both Rocket Pool validators and solo validators simultaneously. Being able to direct priority fees to themselves or to Rocket Pool based on which validator proposed the block.

@pk910
Copy link
Contributor

pk910 commented Jan 17, 2022

I've added a fee-recipient & fee-recipient-file flag to the validator client which works the same way as graffiti / graffiti-file does.
The fee-recipient is pushed to the beacon node via an optional fee_recipient parameter in the /eth/v2/validator/blocks/{slot} api call as @pawanjay176 described above.

Currently testing it on kintsugi to see if it's working like that :) I'm not very familiar with rust and it's just a learning-by-doing copy-paste implementation of the existing graffiti / graffiti-file code.

@pawanjay176
Copy link
Member

Turns out that since the CL needs to ask the EL to start preparing a payload well in advance of it's proposal duties (check this issue for more details #2715) , we cannot send the fee_recipient field with the /validators/beacon/{slot} api.

As mentioned in the above linked issue, we would need to have an api like prepareBeaconProposer on the VC which would indicate the feeRecipient to be used for a given validator index. There is already a draft PR here ethereum/beacon-APIs#178

Apologies for the misinformation, specially to @pk910 for the extra work 😅

@pk910
Copy link
Contributor

pk910 commented Jan 17, 2022

Yea, saw it and makes sense. Didn't know that the EL needs to prepare blocks before that api call. (lighthouse currently does that all synchronously)
Doesn't matter about the extra work - that was a great first project to get a little bit known to the code :)

Maybe I'll try on implementing the prepare_beacon_proposer endpoint..
I just didn't fully understand how that works securely, yet. It seems everyone with network access to the api might override the fee-recipient of all validators connected to that BN just with the validator index number without any signature or so?

And prepare_beacon_proposer should be called regularly for all validators once per epoch - even if the validator is not selected for proposing..
Wouldn't it make more sense to call it just for validators that are selected for proposing? maybe at the beginning of the epoch or so?

@pawanjay176
Copy link
Member

It seems everyone with network access to the api might override the fee-recipient of all validators connected to that BN

Yep, the proposal assumes that the BN api is closed off to anyone but the authenticated VCs.

Wouldn't it make more sense to call it just for validators that are selected for proposing

Proposer shuffling is known only 1 epoch in advance. So if we are at the final slot s of epoch e, then we won't know if we are the proposer in epoch e+1 until we process a slot for s. So we need to accommodate the corner case.

Maybe I'll try on implementing the prepare_beacon_proposer endpoint

I think @paulhauner would be able to guide you better on this one, I think there are a fair bit of intricacies to take care of related to caching proposer duties here.

@michaelsproul
Copy link
Member Author

Summarising some conversation from this Discord thread:

  • The VC needs to send a new prepare_beacon_proposer message each time it detects that one of its beacon nodes has gone offline and then come back online. This gives the BN has a better chance of having fee recipient info after it restarts.
  • The VC should check that the fee recipient is as it expects when signing and refuse to sign if there's a mismatch. This protects against the BN or the EL being malicious and converts cases where the BN lacks the correct fee recipient into signing failures (i.e. equivalent to if the BN were offline).

@joaocenoura
Copy link

Hi @michaelsproul, I was following the discussion on Discord. Did you guys agreed on feeding this information via file or exposing an endpoint via HTTP? In the context of staking providers, an API could possibly be the easiest path to integrate with. In any case, could you guys allow updating the fee recipient per validator without needing to restart BN or VC?

Just to add an alternative, VC could pool this configuration via HTTP, in a similar way it re-reads from a file.

@xrchz
Copy link
Contributor

xrchz commented Feb 1, 2022

The VC should check that the fee recipient is as it expects when signing and refuse to sign if there's a mismatch. This protects against the BN or the EL being malicious and converts cases where the BN lacks the correct fee recipient into signing failures (i.e. equivalent to if the BN were offline).

I think this needs to be somewhat configurable, to support using alternative BN and/or EL as a fallback (and letting them have the fees when in use).

@jj13jj
Copy link

jj13jj commented Feb 2, 2022 via email

bors bot pushed a commit that referenced this issue Feb 8, 2022
…t (similar to graffiti / graffiti-file) (#2924)

## Issue Addressed

#2883 

## Proposed Changes

* Added `suggested-fee-recipient` & `suggested-fee-recipient-file` flags to validator client (similar to graffiti / graffiti-file implementation).
* Added proposer preparation service to VC, which sends the fee-recipient of all known validators to the BN via [/eth/v1/validator/prepare_beacon_proposer](ethereum/beacon-APIs#178) api once per slot
* Added [/eth/v1/validator/prepare_beacon_proposer](ethereum/beacon-APIs#178) api endpoint and preparation data caching
* Added cleanup routine to remove cached proposer preparations when not updated for 2 epochs

## Additional Info

Changed the Implementation following the discussion in #2883.



Co-authored-by: pk910 <philipp@pk910.de>
Co-authored-by: Paul Hauner <paul@paulhauner.com>
Co-authored-by: Philipp K <philipp@pk910.de>
bors bot pushed a commit that referenced this issue Feb 8, 2022
…t (similar to graffiti / graffiti-file) (#2924)

## Issue Addressed

#2883 

## Proposed Changes

* Added `suggested-fee-recipient` & `suggested-fee-recipient-file` flags to validator client (similar to graffiti / graffiti-file implementation).
* Added proposer preparation service to VC, which sends the fee-recipient of all known validators to the BN via [/eth/v1/validator/prepare_beacon_proposer](ethereum/beacon-APIs#178) api once per slot
* Added [/eth/v1/validator/prepare_beacon_proposer](ethereum/beacon-APIs#178) api endpoint and preparation data caching
* Added cleanup routine to remove cached proposer preparations when not updated for 2 epochs

## Additional Info

Changed the Implementation following the discussion in #2883.



Co-authored-by: pk910 <philipp@pk910.de>
Co-authored-by: Paul Hauner <paul@paulhauner.com>
Co-authored-by: Philipp K <philipp@pk910.de>
@paulhauner
Copy link
Member

Resolved via #2924 which implements setting the fee recipient via:

  • Global flag on VC.
  • Per-validator via validator_definitions.yml
  • Per-validator via a --suggested-fee-recipient-file

More information here: https://lighthouse-book.sigmaprime.io/suggested-fee-recipient.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants