-
Notifications
You must be signed in to change notification settings - Fork 692
Slashable vote data cleanup #1697
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
Slashable vote data cleanup #1697
Conversation
i also found a bug in the test code so good thing you took a second look! and while normally we should add a test for the broken test case, i want to discuss updating the BLS spec before making any further changes |
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.
@hwwhww 👍 from me structurally, relying on you to review correctness.
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.
Sorry! Some more nitpicks of the previous PR.
@hwwhww i made those updates :) the related tests pass locally -- trying to sort why the CI failed... seems unrelated to these edits... |
@ralexstokes thanks! 👍 |
The previous implementation was using the `domain_type` as the `domain` when it should in fact be the value calculated by `get_domain`.
This change is more in line with how this functionality will ultimately be used so we want to route our query through the `BeaconState`.
c8a2bbe
to
569849c
Compare
What was wrong?
Fixes #1696.
How was it fixed?
Implemented the suggested changes from the review here: #1658 (review)
Cute Animal Picture