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 support for multiple fee recipients #4894

Merged
merged 14 commits into from
Feb 1, 2022
Merged

Conversation

tbenr
Copy link
Contributor

@tbenr tbenr commented Jan 23, 2022

PR Description

The PR introduces two new params for validator client:

  • --Xvalidators-proposer-config: allows to specify a file path or a URL from which retrieve the configuration

  • --Xvalidators-proposer-config-refresh-enabled: enables reload of the config every time VC is about to call prepare_beacon_proposer API (which currently happen at the beginning of each epoch)

the config is an extensible json file which allows to specify:

  • a proposer configuration for multiple validator public key.
  • a default proposer configuration for validator public keys not included in the previous mapping.

proposer configuration currently contains only fee_recipient. It can be extended to include other information (ie block builder URL)

behaviour details

  • if --Xvalidators-proposer-config is specified, VC during initialisation will try to load the configuration. If fails (or the json config is invalid\incorrect) VC will fail start up
  • if --Xvalidators-proposer-config-refresh-enabled is activated, if config reload fails, the last valid configuration will be used.

config json structure

{
  "proposer_config": {
    "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": {
      "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3",
    }
  },
  "default_config": {
    "fee_recipient": "0x6e35733c5af9B61374A128e6F85f553aF09ff89A"
  }
}

proposer_config is optional
default_config is mandatory
fee_recipient is mandatory in each config

How fee recipient will be actually chosen

  • when --Xvalidators-proposer-config is used in VC

    • fee_recipient from a specific validator in the config
    • otherwise, fee_recipient from default_config in the config
    • otherwise, --Xvalidator-proposer-default-fee-recipient at beacon node
  • when --Xvalidators-proposer-default-fee-recipient is used in VC

    • fee_recipient from --Xvalidators-proposer-default-fee-recipient in VC
    • otherwise, --Xvalidator-proposer-default-fee-recipient at beacon node

additional notes

currently, IO is blocking (file access as well as URL access). We have 5 threads on validator runner, to be evaluated if it is enough for the moment. It can be improved by migrating to async IO.

Fixed Issue(s)

fixes #4850

Documentation

  • I thought about documentation and added the documentation label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@tbenr tbenr changed the title WIP: add support for multiple fee recipients add support for multiple fee recipients Jan 25, 2022
@tbenr tbenr marked this pull request as ready for review January 27, 2022 13:57
@tbenr tbenr changed the title add support for multiple fee recipients Add support for multiple fee recipients Jan 27, 2022
Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

I'm a bit out of it after staying up late so not sure I've properly understood this. Left a few comments but will have to come back to review it properly. Sorry.

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

Makes much more sense now I'm awake properly. Looks good generally, mostly just tweaks to how concurrency is handled.

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM. Given the OwnedValidators change turned out to be smaller than I expected feel free to pull that into here before merging as well.

@tbenr
Copy link
Contributor Author

tbenr commented Feb 1, 2022

LGTM. Given the OwnedValidators change turned out to be smaller than I expected feel free to pull that into here before merging as well.

I improved it by managing potential not-yet-existing indices

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.

Ability to specify different 'fee recipients' for different validator accounts running on a client
2 participants