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

[Merged by Bors] - attester_duties: remove unnecessary case #4614

Closed
wants to merge 1 commit into from

Conversation

zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented Aug 13, 2023

Since tolerant_current_epoch is expected to be either current_epoch or current_epoch+1, we can eliminate a case here.

And added a comment about compute_historic_attester_duties , since RelativeEpoch::from_epoch will only allow request_epoch == current_epoch-1 when request_epoch < current_epoch.

@zhiqiangxu zhiqiangxu changed the title remove unnecessary case attester_duties: remove unnecessary case Aug 13, 2023
@michaelsproul michaelsproul added ready-for-review The code is ready for review HTTP-API labels Aug 14, 2023
Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 16, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 17, 2023
Since `tolerant_current_epoch` is expected to be either `current_epoch` or `current_epoch+1`, we can eliminate a case here. 

And added a comment about `compute_historic_attester_duties` , since `RelativeEpoch::from_epoch` will only allow `request_epoch == current_epoch-1` when `request_epoch < current_epoch`.
@bors
Copy link

bors bot commented Aug 17, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Aug 17, 2023
Since `tolerant_current_epoch` is expected to be either `current_epoch` or `current_epoch+1`, we can eliminate a case here. 

And added a comment about `compute_historic_attester_duties` , since `RelativeEpoch::from_epoch` will only allow `request_epoch == current_epoch-1` when `request_epoch < current_epoch`.
@bors
Copy link

bors bot commented Aug 17, 2023

Build failed (retrying...):

@michaelsproul
Copy link
Member

bors r-

@bors
Copy link

bors bot commented Aug 17, 2023

Canceled.

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 17, 2023
Since `tolerant_current_epoch` is expected to be either `current_epoch` or `current_epoch+1`, we can eliminate a case here. 

And added a comment about `compute_historic_attester_duties` , since `RelativeEpoch::from_epoch` will only allow `request_epoch == current_epoch-1` when `request_epoch < current_epoch`.
@bors
Copy link

bors bot commented Aug 17, 2023

@bors bors bot changed the title attester_duties: remove unnecessary case [Merged by Bors] - attester_duties: remove unnecessary case Aug 17, 2023
@bors bors bot closed this Aug 17, 2023
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
Since `tolerant_current_epoch` is expected to be either `current_epoch` or `current_epoch+1`, we can eliminate a case here. 

And added a comment about `compute_historic_attester_duties` , since `RelativeEpoch::from_epoch` will only allow `request_epoch == current_epoch-1` when `request_epoch < current_epoch`.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
Since `tolerant_current_epoch` is expected to be either `current_epoch` or `current_epoch+1`, we can eliminate a case here. 

And added a comment about `compute_historic_attester_duties` , since `RelativeEpoch::from_epoch` will only allow `request_epoch == current_epoch-1` when `request_epoch < current_epoch`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTTP-API ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants