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

Add tests of processing objects of N-2 fork version #2976

Merged
merged 4 commits into from
Aug 18, 2022
Merged

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Aug 18, 2022

  • test_process_voluntary_exit.py
    • test_voluntary_exit_with_previous_fork_version__valid:
      • signing voluntary exit object with ALTAIR_FORK_VERSION
      • processing voluntary exit object with Bellatrix state
      • result: valid
    • test_voluntary_exit_with_genesis_fork_version__invalid:
      • signing voluntary exit object with GENESIS_FORK_VERSION
      • processing voluntary exit object with Bellatrix state
      • result: invalid
  • test_process_deposit.py
    • all fork versions are valid (no exception), but only GENESIS_FORK_VERSION can add new entry
  • Refactoring:
    • Moved run_voluntary_exit_processing and run_deposit_processing function to the shared helper files.

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.

great!

)


def _run_voluntary_exit_processing_with_specific_fork_version(
Copy link
Contributor

Choose a reason for hiding this comment

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

The name seems slightly off given that this is specifically for an epoch in the previous signature domain (epoch 0, and a fork at epoch 1). that said, it doesn' matter too too much.

Copy link
Contributor

@djrtwo djrtwo Aug 18, 2022

Choose a reason for hiding this comment

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

to make the name more apt and add a couple of tests -- maybe a bool here -- is_before_fork_epoch -- that says if you utilize a voluntary exit with an epoch pre or post the most recent state.fork.epoch. Then we could write a test that has Altair fork_Version but for after fork.epoch which should fail (and a bellatrix fork_version for before the bellatrix fork)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good suggestion 👍 added the new bool flag in 6c00b48

hwwhww and others added 3 commits August 19, 2022 00:03
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
…t_process_deposit.py

Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
@djrtwo djrtwo mentioned this pull request Aug 18, 2022
1 task
@djrtwo djrtwo merged commit 9bd248e into dev Aug 18, 2022
@djrtwo djrtwo deleted the fork-version-tests branch August 18, 2022 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants