Implement Fee Recipient: CLI flags and Calling Beacon API #10312
Conversation
james-prysm
added
Enhancement
cmd/validator/flags/flags.go
Outdated
| @@ -322,6 +323,26 @@ var ( | |||
| Usage: "Enables more verbose logging for counting down to duty", | |||
| Value: false, | |||
| } | |||
|
|
|||
| // ValidatorsProposerConfigDirFlag defines the path or URL to a file with proposer config. | |||
| ValidatorsProposerConfigDirFlag = &cli.StringFlag{ | |||
There was a problem hiding this comment.
This is a file path and not a dir path so we should rename the flag and clarify the usage to say it is a path to a JSON file containing validator mappings to ETH addresses for receiving transaction fees when proposing blocks
cmd/validator/flags/flags.go
Outdated
| // ValidatorsProposerConfigFlag defines the path or URL to a file with proposer config. | ||
| ValidatorsProposerConfigURLFlag = &cli.StringFlag{ | ||
| Name: "validators-proposer-config-url", | ||
| Usage: "Set URL to load proposer config (.json) for per validator mapping to an eth address to receive gas fees when proposing block (i.e. --validators-proposer-config-url=https://example.com/api/getConfig). File format found in docs", |
Co-authored-by: Raul Jordan <[email protected]>
Co-authored-by: Raul Jordan <[email protected]>
Co-authored-by: Raul Jordan <[email protected]>
validator/client/service.go
Outdated
| @@ -186,6 +189,7 @@ func (v *ValidatorService) Start() { | |||
| emitAccountMetrics: v.emitAccountMetrics, | |||
| startBalances: make(map[[fieldparams.BLSPubkeyLength]byte]uint64), | |||
| prevBalance: make(map[[fieldparams.BLSPubkeyLength]byte]uint64), | |||
| pubkeyHexToValidatorIndex: make(map[[fieldparams.BLSPubkeyLength]byte]types.ValidatorIndex), | |||
There was a problem hiding this comment.
| pubkeyHexToValidatorIndex: make(map[[fieldparams.BLSPubkeyLength]byte]types.ValidatorIndex), | |
| pubkeyToValidatorIndex: make(map[[fieldparams.BLSPubkeyLength]byte]types.ValidatorIndex), |
| var validatorToFeeRecipientArray []*ethpb.PrepareBeaconProposerRequest_FeeRecipientContainer | ||
| // need to check for pubkey to validator index mappings | ||
| for _, key := range pubkeys { | ||
| feeRecipient := common.HexToAddress(fieldparams.EthBurnAddressHex) |
There was a problem hiding this comment.
Move this to L992 and change it to v.prepareBeaconProposalConfig.DefaultConfig.FeeRecipient as default. Then you can get rid of the else to just
if option != nil {
feeRecipient = option.FeeRecipient
}There was a problem hiding this comment.
oh i do need to fix something ( whether option as found or not) there but the default config fee recipient might not be the same as the burn address so that's why it was there.
| @@ -0,0 +1,10 @@ | |||
| package validator_service_config | |||
There was a problem hiding this comment.
Add some comments to these structs to explain what they are for. Maybe name the file to fee_recipients_config.go. We want to make it clear this is about fee recipients and not about something else proposer-related
validator/client/iface/validator.go
Outdated
| @@ -57,4 +57,5 @@ type Validator interface { | |||
| ReceiveBlocks(ctx context.Context, connectionErrorChannel chan<- error) | |||
| HandleKeyReload(ctx context.Context, newKeys [][fieldparams.BLSPubkeyLength]byte) (bool, error) | |||
| CheckDoppelGanger(ctx context.Context) error | |||
| PrepareBeaconProposer(ctx context.Context, km keymanager.IKeymanager) error | |||
There was a problem hiding this comment.
From the interface, this feels like it is about preparing proposer duties, which is unclear. We should probably rename this to prepare fee recipients
validator/client/validator.go
Outdated
| type PrepareBeaconProposalConfig struct { | ||
| ProposeConfig map[[fieldparams.BLSPubkeyLength]byte]*ValidatorProposerOptions | ||
| DefaultConfig *ValidatorProposerOptions | ||
| } | ||
|
|
||
| type ValidatorProposerOptions struct { | ||
| FeeRecipient common.Address | ||
| } |
There was a problem hiding this comment.
We already have these in the config.go file in the same package, right? If so, could we move those there or use the types that were defined over there?
There was a problem hiding this comment.
very similar but not the same these uses the byte versions for easier use in our go code. I'll move them together then.
validator/node/node.go
Outdated
| @@ -368,6 +372,8 @@ func (c *ValidatorClient) registerPrometheusService(cliCtx *cli.Context) error { | |||
| } | |||
|
|
|||
| func (c *ValidatorClient) registerValidatorService(cliCtx *cli.Context) error { | |||
| jsonValidator := validator.New() | |||
There was a problem hiding this comment.
validator.New() sounds like a big conflict with the concept of an eth2 validator. Maybe we should alias the package to jsonValidation, so you can do jsonValidator := jsonValidation.New()
There was a problem hiding this comment.
You can probably move this to right above where it is used, above line 413
There was a problem hiding this comment.
i am doing validations in a different way now so thinking of just removing for now
| field_params "github.com/prysmaticlabs/prysm/config/fieldparams" | ||
| ) | ||
|
|
||
| type FeeRecipientFileConfig struct { |
There was a problem hiding this comment.
Add comments to these structs about how they are used
What type of PR is this?
Feature
relates to #10292
What does this PR do? Why is it needed?
This PR focuses on allowing the user to set fee recipients for their validator keys whether through a file location or a url to send to the beacon node to be cached.