Skip to content

electra: Add pending deposit and consolidation tests#4024

Merged
hwwhww merged 2 commits into
ethereum:devfrom
mkalinin:process-epoch-tests
Nov 28, 2024
Merged

electra: Add pending deposit and consolidation tests#4024
hwwhww merged 2 commits into
ethereum:devfrom
mkalinin:process-epoch-tests

Conversation

@mkalinin

Copy link
Copy Markdown
Contributor

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

@hwwhww hwwhww left a comment

Copy link
Copy Markdown
Contributor

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.

Comment thread tests/core/pyspec/eth2spec/test/electra/sanity/test_slots.py Outdated
Comment thread tests/core/pyspec/eth2spec/test/electra/sanity/test_slots.py Outdated
Comment thread tests/core/pyspec/eth2spec/test/electra/sanity/test_slots.py Outdated
Comment thread tests/core/pyspec/eth2spec/test/electra/sanity/test_slots.py Outdated
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
Copy Markdown
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)

Comment thread tests/core/pyspec/eth2spec/test/electra/sanity/test_slots.py
@hwwhww hwwhww added the testing CI, actions, tests, testing infra label Nov 28, 2024
@hwwhww hwwhww merged commit efb554f into ethereum:dev Nov 28, 2024

Copy link
Copy Markdown
Member

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

Labels

testing CI, actions, tests, testing infra

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