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.

Sign up for GitHub

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

Jump to bottom

Refine validate_merge_block unit tests #2713

Merged
merged 2 commits into from
Nov 6, 2021
Merged

Refine validate_merge_block unit tests #2713

merged 2 commits into from
Nov 6, 2021

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Nov 5, 2021

Change set

  • Breaks test_terminal_validity into two groups of tests:
    • test_is_valid_terminal_pow_block
    • test_validate_merge_block
  • Adjusts tests to the new signature and semantics of get_pow_block function
  • Adds terminal block hash checks
  • A bit of refactor
    • Moves PowBlock related helpers to the new pow_block.py

zilm13
zilm13 reviewed Nov 5, 2021
View reviewed changes
Copy link
Contributor

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

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

Looks good

tests/core/pyspec/eth2spec/test/helpers/pow_block.py


def prepare_empty_pow_block(spec, rng=Random(3131)):
return spec.PowBlock(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe prepare_random_pow_block is more correct?

djrtwo
djrtwo approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

Looks good!

I'll let you decide how to address @zilm13's comments and then click "merge"

@mkalinin
Copy link
Collaborator Author

mkalinin commented Nov 6, 2021

Looks good!

I'll let you decide how to address @zilm13's comments and then click "merge"

Addressed. PowChain is my first class written in python

@djrtwo
Copy link
Contributor

djrtwo commented Nov 6, 2021

looks good!

@djrtwo djrtwo merged commit e6b726b into ethereum:dev Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zilm13 zilm13 zilm13 left review comments

@djrtwo djrtwo djrtwo approved these changes

@hwwhww hwwhww Awaiting requested review from hwwhww

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mkalinin @djrtwo @zilm13