-
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
Add in Attestation Validity Check #6983
Conversation
…labs/geth-sharding into fixAttestationNoVerify
// the signature in that attestation. | ||
func VerifyAttestation(ctx context.Context, beaconState *stateTrie.BeaconState, att *ethpb.Attestation) error { | ||
if att == nil || att.Data == nil || att.AggregationBits.Count() == 0 { | ||
return fmt.Errorf("nil or missing attestation data: %v", att) |
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.
return fmt.Errorf("nil or missing attestation data: %v", att) | |
return fmt.Errorf("nil, missing attestation data or no attesting indices: %v", att) |
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.
To confirm, you just shift the ordering of the two functions and added att.AggregationBits.Count() == 0
right?
No , there was a valid indexed attestation check that was missing completely. It deviated from the spec and our old core methods. When changing it to batch verify attestations we missed this whole check. |
I see the `VerifyAttestation |
What type of PR is this?
Bug Fix
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
Fixes #
Other notes for review