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

Validator Balance update Refactor #255

Merged
merged 7 commits into from
Oct 19, 2023
Merged

Validator Balance update Refactor #255

merged 7 commits into from
Oct 19, 2023

Conversation

Sidu28
Copy link
Contributor

@Sidu28 Sidu28 commented Oct 17, 2023 •
edited
Loading

This PR refactors the validator balance updater to process multiple balance updates. We allow this for verifying withdrawal credentials and withdrawals, it makes sense to do it here as well

@Sidu28 Sidu28 changed the title init Allow validators to update balance when slashed to 0 Oct 17, 2023
@Sidu28 Sidu28 marked this pull request as draft October 17, 2023 19:43
@Sidu28 Sidu28 changed the title Allow validators to update balance when slashed to 0 Validator Balance update Refactor Oct 17, 2023
@Sidu28 Sidu28 marked this pull request as ready for review October 18, 2023 20:44
@Sidu28
Copy link
Contributor Author

Sidu28 commented Oct 18, 2023

@gpsanant this changes interfaces a bit, shouldn't be too bad

ChaoticWalrus
ChaoticWalrus requested changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@ChaoticWalrus ChaoticWalrus left a comment

Choose a reason for hiding this comment

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

Seems like a good change. Should also update docs, and we should align w/ FE etc on merging this.

@Sidu28 Sidu28 requested a review from ChaoticWalrus October 19, 2023 05:10
@ChaoticWalrus
Copy link
Contributor

Seems good now. @Sidu28 would be good to see test coverage for all these functions that accept arrays (even just a simple test w/ 2 inputs, preferably with the validators having different amounts so we can be sure that we don't have an error of e.g. reading always from one entry of the array) -- capturing these as TODOs for the moment is OK by me though.

@Sidu28
Copy link
Contributor Author

Sidu28 commented Oct 19, 2023

Seems good now. @Sidu28 would be good to see test coverage for all these functions that accept arrays (even just a simple test w/ 2 inputs, preferably with the validators having different amounts so we can be sure that we don't have an error of e.g. reading always from one entry of the array) -- capturing these as TODOs for the moment is OK by me though.

I'll make a ticket for it!

@Sidu28 Sidu28 merged commit 996ff72 into master Oct 19, 2023
@Sidu28 Sidu28 deleted the new-proof branch October 19, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChaoticWalrus ChaoticWalrus ChaoticWalrus approved these changes

@gpsanant gpsanant Awaiting requested review from gpsanant

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sidu28 @ChaoticWalrus