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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - #4512 inactivity calculation for Altair #4807

Closed
wants to merge 11 commits into from
Closed

[Merged by Bors] - #4512 inactivity calculation for Altair #4807

wants to merge 11 commits into from

Conversation

zack-scott
Copy link
Contributor

Issue Addressed

#4512
Which issue # does this PR address?

Proposed Changes

Add inactivity calculation for Altair

Please list or describe the changes introduced by this PR.
Add inactivity calculation for Altair

Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@jimmygchen jimmygchen added work-in-progress PR is a work-in-progress waiting-on-author The reviewer has suggested changes and awaits thier implementation. HTTP-API and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 5, 2023
@zack-scott
Update attestation_rewards.rs
7008469 
Updated inactivity calculation to only occur during incorrect voting
@zack-scott zack-scott marked this pull request as ready for review October 14, 2023 02:44
@zack-scott zack-scott requested a review from jimmygchen October 14, 2023 02:44
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Oct 17, 2023
@jimmygchen jimmygchen mentioned this pull request Oct 18, 2023
Closed
@jimmygchen jimmygchen self-assigned this Oct 19, 2023
jimmygchen
jimmygchen approved these changes Oct 19, 2023
View reviewed changes
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

Nice work! I'm impressed at your thoroughness on this 🙏
And thanks a lot for your help finding bugs!

Separation issues have been raised below, so i think we can go ahead and merge this

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 19, 2023
@jimmygchen
Copy link
Member

jimmygchen commented Oct 19, 2023 •
edited
Loading

Hey sorry @zack-scott, about to merge and realize there's a minor lint issue on CI, would you mind fixing this please?
https://github.com/sigp/lighthouse/actions/runs/6515110219/job/17844942785?pr=4807

It might be auto-fixable with make lint-fix

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-merge This PR is ready to merge. labels Oct 19, 2023
@zack-scott
Copy link
Contributor Author

zack-scott commented Oct 19, 2023 •
edited
Loading

@jimmygchen I fixed the issue with the command you provided. Sorry about that, I was using make cargo-fmt to check for lint issues as suggested in the documentation https://lighthouse-book.sigmaprime.io/setup.html#using-make

@jimmygchen
Copy link
Member

Thanks for fixing and linking the doc! It's slightly outdated, will add lint to the docs too :)

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 19, 2023
@zack-scott
Copy link
Contributor Author

@jimmygchen I see that another issue occurred. I ran make lint locally, and I am not seeing the same compilation issues. I am running rustc version 1.72.1

@jimmygchen
Copy link
Member

Our CI is running latest stable version (1.73.0) so it might be why it's catching additional lint errors.

@jimmygchen
Copy link
Member

ah looks like there are some compilation issues after the Deneb branch was merged to unstable, I'll push a fix.

@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Oct 20, 2023
@zack-scott @jimmygchen
#4512 inactivity calculation for Altair (#4807)
ceac757 
## Issue Addressed
#4512 
Which issue # does this PR address?

## Proposed Changes
Add inactivity calculation for Altair

Please list or describe the changes introduced by this PR.
Add inactivity calculation for Altair

## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: Jimmy Chen <[email protected]>
@bors
Copy link

bors bot commented Oct 20, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Oct 20, 2023
@zack-scott @jimmygchen
#4512 inactivity calculation for Altair (#4807)
94a1bac 
## Issue Addressed
#4512 
Which issue # does this PR address?

## Proposed Changes
Add inactivity calculation for Altair

Please list or describe the changes introduced by this PR.
Add inactivity calculation for Altair

## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: Jimmy Chen <[email protected]>
@bors
Copy link

bors bot commented Oct 20, 2023

Build failed:

@jimmygchen
Copy link
Member

Looks like there might be some bad caches, I've addressed it in #4868 and cleared cache. Let's try it again.

bors r+.

bors bot pushed a commit that referenced this pull request Oct 20, 2023
@zack-scott @jimmygchen
#4512 inactivity calculation for Altair (#4807)
b119882 
## Issue Addressed
#4512 
Which issue # does this PR address?

## Proposed Changes
Add inactivity calculation for Altair

Please list or describe the changes introduced by this PR.
Add inactivity calculation for Altair

## Additional Info

Please provide any additional information. For example, future considerations
or information useful for reviewers.


Co-authored-by: Jimmy Chen <[email protected]>
@bors
Copy link

bors bot commented Oct 20, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title #4512 inactivity calculation for Altair [Merged by Bors] - #4512 inactivity calculation for Altair Oct 20, 2023
@bors bors bot closed this Oct 20, 2023
@jimmygchen jimmygchen mentioned this pull request Oct 20, 2023
Closed
@zack-scott zack-scott deleted the 4512-inactivity-calculation-altair branch October 20, 2023 17:44
@chenrui333 chenrui333 mentioned this pull request Jan 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimmygchen jimmygchen jimmygchen approved these changes

Assignees

@jimmygchen jimmygchen

Labels
HTTP-API ready-for-merge This PR is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zack-scott @CLAassistant @jimmygchen @michaelsproul