[Merged by Bors] - Allow for a clock disparity on the duties endpoints #2283
Conversation
paulhauner
force-pushed
the
tolerant-duties
branch
from
c47fa9c
to
c2f66a4
Compare
paulhauner
added
ready-for-review
|
This is ready for review! It's running on all the SigP Pyrmont and Mainnet nodes and those pesky errors have stopped. |
| .ok_or_else(|| warp_utils::reject::custom_server_error("unable to read slot clock".into()))? | ||
| .epoch(T::EthSpec::slots_per_epoch()); | ||
|
|
||
| if request_epoch == current_epoch |
There was a problem hiding this comment.
Don't think this makes a functional difference unless MAXIMUM_GOSSIP_CLOCK_DISPARITY is greater than an epoch (lol) but this logic makes a bit more sense to me:
if current_epoch <= request_epoch && request_epoch <= tolerant_current_epoch + 1 {
} else if request_epoch > tolerant_current_epoch + 1 {
} else {
// request < current_epoch
}
There was a problem hiding this comment.
Agree it should be equivalent, but don't have a strong preference either way. The 4-way or has the advantage of being quite explicit.
Writing request_epoch >= current_epoch && request_epoch <= tolerant_current_epoch + 1 is also nice 😝
There was a problem hiding this comment.
Thanks @realbigsean, I agree that your statement is more universal but I do find what we've got to be easier to read. Perhaps that's just because I wrote it 🤔. I might leave it as-is for now 🙏
| .ok_or_else(|| warp_utils::reject::custom_server_error("unable to read slot clock".into()))? | ||
| .epoch(T::EthSpec::slots_per_epoch()); | ||
|
|
||
| if request_epoch == current_epoch |
There was a problem hiding this comment.
Agree it should be equivalent, but don't have a strong preference either way. The 4-way or has the advantage of being quite explicit.
Writing request_epoch >= current_epoch && request_epoch <= tolerant_current_epoch + 1 is also nice 😝
paulhauner
added
ready-for-merge
|
bors r+ |
## Issue Addressed Resolves #2280 ## Proposed Changes Allows for API consumers to call the proposer/attester duties endpoints [`MAXIMUM_GOSSIP_CLOCK_DISPARITY`](https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/beacon_chain/src/beacon_chain.rs#L99-L102) earlier than the current epoch. For additional reasoning, see #2280 (comment). ## Additional Info NA
|
bors r- |
|
Canceled. |
|
bors r+ |
## Issue Addressed Resolves #2280 ## Proposed Changes Allows for API consumers to call the proposer/attester duties endpoints [`MAXIMUM_GOSSIP_CLOCK_DISPARITY`](https://github.com/sigp/lighthouse/blob/b34a79dc0b02e04441ba01fd0f304d1e203d877d/beacon_node/beacon_chain/src/beacon_chain.rs#L99-L102) earlier than the current epoch. For additional reasoning, see #2280 (comment). ## Additional Info NA
|
Pull request successfully merged into unstable. Build succeeded: |
bors
bot
changed the title
Issue Addressed
Resolves #2280
Proposed Changes
Allows for API consumers to call the proposer/attester duties endpoints
MAXIMUM_GOSSIP_CLOCK_DISPARITYearlier than the current epoch. For additional reasoning, see #2280 (comment).Additional Info
NA