-
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
Tests with signatures, fixes #1074 #1102
Conversation
test_libs/pyspec/eth2spec/test/block_processing/test_process_block_header.py
Outdated
Show resolved
Hide resolved
Ah, looks like this branch doesn't have the changes from #1052 integrated. Is the intention to modify |
IMO it's safer to make it more explicit by removing the default value |
It's branched off of that, so it should. You mean the proposal for BLS test-improvements? This PR is in draft mode mostly because:
Ideally I think BLS should be ON, if pytests were fast enough. But we already discussed this, and it will be off for CI where possible, but ON where needed + in generated vectors. For now I just like the explicitness, as it helped me tracking what needs to be updated, and finding testing bugs (things not being signed). If things are stable we may go for implicit signed True/False, but I don't think it's a netto win now. |
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.
Last set of cleanups and forced-on bls tests look good.
Headed to bed. good to merge when you're up
Made the pytests respect BLS signatures. This enables us to use the tests to generate properly BLS-signed test-vectors with.
Note: there are a few exceptions:
The block-operations and epoch transitions are fully signed now 🎉
TODO:
Cover more of epoch transition with new testsfuture PRImplement some basic SSZ caching, to speed up tests (the hashes of fields of empty blocks being cached would help a lot! Tests typically change only a part of the block).CI runs fast enough, SSZ caching to be implemented in separate PR