-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Check that the headers are equal while ignoring the signature for proposer slashings #7252
Conversation
…poser slashings. Resolves sigp/beacon-fuzz#74
…s/prysm into fix-proposer-slashing-issue
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.
LGTM, thanks @prestonvanloon
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
testutil.ResetCache() | ||
if err := blocks.VerifyProposerSlashing(tt.args.beaconState, tt.args.slashing); (err != nil || tt.wantErr != "") && err.Error() != tt.wantErr { |
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.
we should use our assertion library over here
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.
Does the assertion library support this behavior?
I want: "if the test expects an error, assert the error is as expected"
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.
Not yet, it does have assert.ErrorContains
which would be the closest to it. But yeah maybe not worth it to change now.
Codecov Report
@@ Coverage Diff @@
## master #7252 +/- ##
==========================================
- Coverage 60.07% 59.46% -0.61%
==========================================
Files 323 418 +95
Lines 27422 29941 +2519
==========================================
+ Hits 16473 17805 +1332
- Misses 8733 9231 +498
- Partials 2216 2905 +689 |
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
We had a minor spec deviation in checking proposer slashings.
Which issues(s) does this PR fix?
Fixes sigp/beacon-fuzz#74
Other notes for review
See sigp/beacon-fuzz#74 for more context.