-
Notifications
You must be signed in to change notification settings - Fork 231
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
Implement the /eth/v1/validator/prepare_beacon_proposer end-point #3901
Conversation
dd5143e
to
bb85f6f
Compare
needs a documentation update to highlight the new source of fee recipient |
@@ -239,6 +239,10 @@ type | |||
epoch*: Epoch | |||
active*: bool | |||
|
|||
PrepareBeaconProposerBody* = object | |||
validator_index*: ValidatorIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RestValidatorIndex
I believe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RestValidatorIndex
was something that raised in prominence when we tried to introduce a wider ValidatorIndex
reform (which wasn't merged). In the current code, it's used only for a single field and all other REST data types use the spec ValidatorIndex
type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wider VI is unrelated - RestValidatorIndex
is used to differentiate between errors: a validator index that is larger than 2**32
but smaller than 2**41
(or something like that) is a valid but not yet assigned validator index - the ValidatorIndex
parser is unable to distinguish these two cases, resulting in a poor error message, or in some requests, invalid responses (because some requests give an empty response for a not-yet-assigned validator index)
@@ -374,8 +378,11 @@ proc getExecutionPayload( | |||
terminalBlockHash | |||
latestFinalized = | |||
node.dag.loadExecutionBlockRoot(node.dag.finalizedHead.blck) | |||
feeRecipient = node.config.getSuggestedFeeRecipient(pubkey).valueOr: | |||
node.config.defaultFeeRecipient | |||
dynamicFeeRecipient = node.dynamicFeeRecipientsStore.getDynamicFeeRecipient( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, I think this can be written as gDFR() or gSFR() or Opt.some(default)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The valueOr
construct effectively works as a right-associative operator while or
is left-associative. For this particular purpose, right-associativity produces better code which is equivalent to a hand-written tree of if statements.
Will be addressed with a separate PR against the |
No description provided.