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

electra: Add pending deposit and consolidation tests #4024

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

mkalinin
Copy link
Collaborator

This PR introduces tests for the entire process_epoch routine that cover the following scenarios:

  • New validator creation and a top up to the newly created validator made in the same run of process_pending_deposits are correctly handled by process_effective_balance_updates
  • Balance move made by process_pending_consolidations is handled correctly by process_effective_balance_updates

Resolves #4021

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @mkalinin!

Just some minor suggestions here. I'll merge the suggestion commits now.

deposit_1 = prepare_pending_deposit(spec, validator_index=index, amount=amount, signed=True)
pending_deposits = [deposit_0, deposit_1]

yield from run_epoch_processing(spec, state, pending_deposits)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
yield from run_epoch_processing(spec, state, pending_deposits)
yield from run_epoch_processing(spec, state, pending_deposits=pending_deposits)

@hwwhww hwwhww merged commit efb554f into ethereum:dev Nov 28, 2024
25 of 26 checks passed
Copy link
Contributor

Choose a reason for hiding this comment

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

I had issues generating spec tests with the changes in this PR

when running make gen_sanity I get the following error

Traceback (most recent call last):
  File "/home/nico/projects/ethereum/consensus-specs/tests/generators/sanity/main.py", line 48, in <module>
    check_mods(all_mods, "sanity")
  File "/home/nico/projects/ethereum/consensus-specs/tests/generators/sanity/venv/lib/python3.10/site-packages/eth2spec/gen_helpers/gen_from_tests/gen.py", line 203, in check_mods
    raise Exception('[ERROR] module problems:\n ' + '\n '.join(problems))
Exception: [ERROR] module problems:
 missing: eth2spec.test.electra.sanity.test_slots
generator sanity finished

which seems to happen because the module name is eth2spec.test.electra.sanity.slots as per change below

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.

[spec test]: Ensure duplicate deposits are handled successfully during electra epoch processing
3 participants