-
Notifications
You must be signed in to change notification settings - Fork 906
Fulu EF tests v1.6.0-alpha.0 #7540
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
Conversation
Thanks @ethDreamer, I'll pick it up from here! |
There's something weird going on when I test locally where some Capella and Deneb test files are detected as unused. Not sure what is going on there, and I need to run now.
Ohhh, this might be a change in the test format? And we're not reading those files? |
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 ;)
match T::tree_hash_type() { | ||
TreeHashType::Basic => { | ||
let mut hasher = | ||
MerkleHasher::with_leaves(max_len.div_ceil(T::tree_hash_packing_factor())); |
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.
I guess this is covered by the spec tests in LH - but we should add a unit test if we move this into the ssz library.
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.
Yeah agreed. I'll work on a PR for that
Looks great, really nice work @ethDreamer @michaelsproul 🥳 |
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 🎉
Issue Addressed
Update to EF tests v1.6.0-alpha.0