Skip to content

Conversation

ethDreamer
Copy link
Member

@ethDreamer ethDreamer commented May 29, 2025

Issue Addressed

Update to EF tests v1.6.0-alpha.0

@ethDreamer ethDreamer added the work-in-progress PR is a work-in-progress label May 30, 2025
@jimmygchen jimmygchen self-assigned this Jun 2, 2025
@jimmygchen
Copy link
Member

Thanks @ethDreamer, I'll pick it up from here!

@michaelsproul michaelsproul changed the title Fulu ef tests Fulu EF tests Jun 3, 2025
@michaelsproul
Copy link
Member

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.

tests/mainnet/deneb/epoch_processing/inactivity_updates/pyspec_tests/random_inactivity_scores_empty_participation/pre_epoch.ssz_snappy

Ohhh, this might be a change in the test format? And we're not reading those files?

@michaelsproul michaelsproul changed the title Fulu EF tests Fulu EF tests v1.6.0-alpha.0 Jun 4, 2025
@michaelsproul michaelsproul added test improvement Improve tests das Data Availability Sampling fulu Required for the upcoming Fulu hard fork labels Jun 4, 2025
@michaelsproul michaelsproul removed the work-in-progress PR is a work-in-progress label Jun 4, 2025
@michaelsproul michaelsproul added the ready-for-review The code is ready for review label Jun 4, 2025
Copy link
Member

@michaelsproul michaelsproul left a 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()));
Copy link
Member

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.

Copy link
Member

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

@jimmygchen
Copy link
Member

Looks great, really nice work @ethDreamer @michaelsproul 🥳

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jun 4, 2025
@mergify mergify bot merged commit 2d9fc34 into sigp:unstable Jun 4, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
das Data Availability Sampling fulu Required for the upcoming Fulu hard fork ready-for-merge This PR is ready to merge. test improvement Improve tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants