-
Notifications
You must be signed in to change notification settings - Fork 169
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
Optional randao verification when producing blocks #222
Conversation
4edba50
to
f7e6584
Compare
CI seems to be failing on an unrelated error in the blinded_blocks API, weirdly it doesn't occur locally for me with spectral 6.3.0 |
f7e6584
to
f89ade3
Compare
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.
looks reasonable but would prefer more client teams to take a look
Since Teku isn't validating the randao reveal when creating blocks now so it would be nice if we could just ignore this param and not need to make any changes. As such I'd have a mild preference for It would also mean that |
Thanks for the feedback @ajsutton, @djrtwo 🙏 How does this sound moving forward?
|
Yep that works for me. |
Alternatively we could use a value-less |
Can you rebase with master? |
ab72517
to
b39d248
Compare
Done! |
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.
approving. will merge in 2 days unless any additional comments
looks like we're good to merge @djrtwo! thanks! |
b39d248
to
cd71d99
Compare
…3540) ## Issue Addressed ethereum/beacon-APIs#222 ## Proposed Changes Update Lighthouse's randao verification API to match the `beacon-APIs` spec. We implemented the API before spec stabilisation, and it changed slightly in the course of review. Rather than a flag `verify_randao` taking a boolean value, the new API uses a `skip_randao_verification` flag which takes no argument. The new spec also requires the randao reveal to be present and equal to the point-at-infinity when `skip_randao_verification` is set. I've also updated the `POST /lighthouse/analysis/block_rewards` API to take blinded blocks as input, as the execution payload is irrelevant and we may want to assess blocks produced by builders. ## Additional Info This is technically a breaking change, but seeing as I suspect I'm the only one using these parameters/APIs, I think we're OK to include this in a patch release.
…3540) ## Issue Addressed ethereum/beacon-APIs#222 ## Proposed Changes Update Lighthouse's randao verification API to match the `beacon-APIs` spec. We implemented the API before spec stabilisation, and it changed slightly in the course of review. Rather than a flag `verify_randao` taking a boolean value, the new API uses a `skip_randao_verification` flag which takes no argument. The new spec also requires the randao reveal to be present and equal to the point-at-infinity when `skip_randao_verification` is set. I've also updated the `POST /lighthouse/analysis/block_rewards` API to take blinded blocks as input, as the execution payload is irrelevant and we may want to assess blocks produced by builders. ## Additional Info This is technically a breaking change, but seeing as I suspect I'm the only one using these parameters/APIs, I think we're OK to include this in a patch release.
…igp#3540) ## Issue Addressed ethereum/beacon-APIs#222 ## Proposed Changes Update Lighthouse's randao verification API to match the `beacon-APIs` spec. We implemented the API before spec stabilisation, and it changed slightly in the course of review. Rather than a flag `verify_randao` taking a boolean value, the new API uses a `skip_randao_verification` flag which takes no argument. The new spec also requires the randao reveal to be present and equal to the point-at-infinity when `skip_randao_verification` is set. I've also updated the `POST /lighthouse/analysis/block_rewards` API to take blinded blocks as input, as the execution payload is irrelevant and we may want to assess blocks produced by builders. ## Additional Info This is technically a breaking change, but seeing as I suspect I'm the only one using these parameters/APIs, I think we're OK to include this in a patch release.
Recently I've been speculatively creating blocks using the
/eth/v2/validator/blocks/{slot}
endpoint, polling the endpoint live at every slot.Some beacon node implementations verify the
randao_reveal
signature when this endpoint is used, which causes speculative block proposal to fail (because the private key for the trueproposer_index
is not known).A few months ago I added an optional
verify_randao
parameter to disable the verification in Lighthouse: sigp/lighthouse#3116. Recently I've discovered that Nimbus have started to verify therandao_reveal
by default too, so I'd like to implore them to adopt the same parameter.As far as I know the other clients (Teku/Prysm/Lodestar) don't verify the
randao_reveal
currently, so I think it would be acceptable for them to delay implementing theverify_randao
parameter until they decide to. Alternatively they could maintain the same semantics and acceptverify_randao=false/null
, and error ifverify_randao=true
. I would like to impose the minimum burden on implementers, given that this is a relatively niche use case.More background: