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.

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

Update test_process_sync_aggregate.py #2583

Merged
merged 5 commits into from
Sep 2, 2021
Merged

Update test_process_sync_aggregate.py #2583

merged 5 commits into from
Sep 2, 2021

Conversation

asanso
Copy link
Contributor

@asanso asanso commented Sep 2, 2021

The test_invalid_signature_bad_domain is about testing bad domain . There is a little copy/cargo issue with test_invalid_signature_missing_participant

Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

Nice catch!

I agree the spirit of this test is better expressed by just isolating the bad signature domain. We have other tests with bad bits so we are not losing any test coverage with this change.

I left one syntax suggestion -- the sync_committee_bits field is expecting a Sequence[bool] and it may work w/ a Sequence[int] but this would just be an idiosyncrasy of python and we can be clearing w/ explicit bool

…ggregate/test_process_sync_aggregate.py

Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
@ralexstokes ralexstokes merged commit 89c865e into ethereum:dev Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants