Skip to content

Add to epoch processing tests generation states before and after full epoch processing #4155

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

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -326,25 +326,6 @@ def test_apply_pending_deposit_top_up__less_effective_balance(spec, state):
assert state.validators[validator_index].effective_balance == initial_effective_balance


@with_electra_and_later
@spec_state_test
def test_apply_pending_deposit_top_up__zero_balance(spec, state):
validator_index = 0
amount = spec.MIN_ACTIVATION_BALANCE // 4
pending_deposit = prepare_pending_deposit(spec, validator_index, amount, signed=True)

initial_balance = 0
initial_effective_balance = 0
state.balances[validator_index] = initial_balance
state.validators[validator_index].effective_balance = initial_effective_balance

yield from run_pending_deposit_applying(spec, state, pending_deposit, validator_index)

assert state.balances[validator_index] == initial_balance + amount
# unchanged effective balance
assert state.validators[validator_index].effective_balance == initial_effective_balance


@with_electra_and_later
@spec_state_test
@always_bls
Expand Down
21 changes: 13 additions & 8 deletions tests/core/pyspec/eth2spec/test/helpers/deposits.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
from eth2spec.test.helpers.forks import is_post_altair, is_post_electra
from eth2spec.test.helpers.keys import pubkeys, privkeys
from eth2spec.test.helpers.state import get_balance
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_to
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_from, run_epoch_processing_to, \
run_process_slots_up_to_epoch_boundary
Comment on lines +7 to +8
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
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_from, run_epoch_processing_to, \
run_process_slots_up_to_epoch_boundary
from eth2spec.test.helpers.epoch_processing import (
run_epoch_processing_from,
run_epoch_processing_to,
run_process_slots_up_to_epoch_boundary,
)

from eth2spec.utils import bls
from eth2spec.utils.merkle_minimal import calc_merkle_tree_from_leaves, get_merkle_proof
from eth2spec.utils.ssz.ssz_impl import hash_tree_root
Expand Down Expand Up @@ -360,7 +361,6 @@ def run_deposit_request_processing(
deposit_request,
validator_index,
effective=True):

"""
Run ``process_deposit_request``, yielding:
- pre-state ('pre')
Expand Down Expand Up @@ -409,6 +409,8 @@ def run_pending_deposit_applying(spec, state, pending_deposit, validator_index,
Enqueue ``pending_deposit`` and run epoch processing with ``process_pending_deposits``, yielding:
- pre-state ('pre')
- post-state ('post').
- pre-epoch-state ('pre_epoch'), state before epoch transition
- post-epoch-state ('post_epoch'), state after epoch transition
"""
assert is_post_electra(spec)

Expand All @@ -422,10 +424,6 @@ def run_pending_deposit_applying(spec, state, pending_deposit, validator_index,
# append pending deposit
state.pending_deposits.append(pending_deposit)

# run to the very beginning of the epoch processing to avoid
# any updates to the validator registry (e.g. ejections)
run_epoch_processing_to(spec, state, "process_justification_and_finalization")

pre_validator_count = len(state.validators)
pre_balance = 0
pre_effective_balance = 0
Expand All @@ -436,12 +434,19 @@ def run_pending_deposit_applying(spec, state, pending_deposit, validator_index,
pre_balance = get_balance(state, validator_index)
pre_effective_balance = state.validators[validator_index].effective_balance

yield 'pre', state
run_process_slots_up_to_epoch_boundary(spec, state)
yield "pre_epoch", state
run_epoch_processing_to(
spec, state, "process_pending_deposits", disable_slots_processing=True)

yield 'pre', state
spec.process_pending_deposits(state)

yield 'post', state

continue_state = state.copy()
run_epoch_processing_from(spec, continue_state, "process_pending_deposits")
yield 'post_epoch', continue_state

if effective:
if is_top_up:
# Top-ups don't add validators
Expand Down
47 changes: 40 additions & 7 deletions tests/core/pyspec/eth2spec/test/helpers/epoch_processing.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

from eth2spec.test.helpers.forks import (
is_post_altair,
is_post_capella,
Expand Down Expand Up @@ -40,11 +39,28 @@ def get_process_calls(spec):
]


def run_epoch_processing_to(spec, state, process_name: str):
def run_epoch_processing_to(spec, state, process_name: str, disable_slots_processing=False):
"""
Processes to the next epoch transition, up to, but not including, the sub-transition named ``process_name``
"""
slot = state.slot + (spec.SLOTS_PER_EPOCH - state.slot % spec.SLOTS_PER_EPOCH)
if not disable_slots_processing:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to put it this way:

Suggested change
if not disable_slots_processing:
if enable_slot_processing:

run_process_slots_up_to_epoch_boundary(spec, state)

# process components of epoch transition before final-updates
for name in get_process_calls(spec):
if name == process_name:
break
# only run when present. Later phases introduce more to the epoch-processing.
if hasattr(spec, name):
getattr(spec, name)(state)


def run_process_slots_up_to_epoch_boundary(spec, state):
"""
Processes slots until the next epoch transition
"""
slot = state.slot + (spec.SLOTS_PER_EPOCH - state.slot %
spec.SLOTS_PER_EPOCH)

# transition state to slot before epoch state transition
if state.slot < slot - 1:
Expand All @@ -53,12 +69,20 @@ def run_epoch_processing_to(spec, state, process_name: str):
# start transitioning, do one slot update before the epoch itself.
spec.process_slot(state)

# process components of epoch transition before final-updates

def run_epoch_processing_from(spec, state, process_name: str):
"""
Processes to the next epoch transition, from, but not including, the sub-transition named ``process_name``
"""
assert (state.slot + 1) % spec.SLOTS_PER_EPOCH == 0

processing = False
for name in get_process_calls(spec):
if name == process_name:
break
processing = True
continue
# only run when present. Later phases introduce more to the epoch-processing.
if hasattr(spec, name):
if processing and hasattr(spec, name):
getattr(spec, name)(state)


Expand All @@ -67,8 +91,17 @@ def run_epoch_processing_with(spec, state, process_name: str):
Processes to the next epoch transition, up to and including the sub-transition named ``process_name``
- pre-state ('pre'), state before calling ``process_name``
- post-state ('post'), state after calling ``process_name``
- pre-epoch-state ('pre_epoch'), state before epoch transition
- post-epoch-state ('post_epoch'), state after epoch transition
The state passed by reference will be modified to be the ``process_name``post state.
"""
run_epoch_processing_to(spec, state, process_name)
run_process_slots_up_to_epoch_boundary(spec, state)
yield "pre_epoch", state
run_epoch_processing_to(spec, state, process_name,
disable_slots_processing=True)
yield 'pre', state
getattr(spec, process_name)(state)
yield 'post', state
continue_state = state.copy()
run_epoch_processing_from(spec, continue_state, process_name)
yield 'post_epoch', continue_state
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from eth2spec.test.context import spec_state_test, with_all_phases
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_to
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_to, run_epoch_processing_from, \
run_process_slots_up_to_epoch_boundary
from eth2spec.test.helpers.withdrawals import (
set_compounding_withdrawal_credential,
)
Expand All @@ -16,7 +17,9 @@ def run_test_effective_balance_hysteresis(spec, state, with_compounding_credenti
assert is_post_electra(spec) or not with_compounding_credentials
# Prepare state up to the final-updates.
# Then overwrite the balances, we only want to focus to be on the hysteresis based changes.
run_epoch_processing_to(spec, state, 'process_effective_balance_updates')
run_process_slots_up_to_epoch_boundary(spec, state)
yield 'pre_epoch', state
run_epoch_processing_to(spec, state, 'process_effective_balance_updates', disable_slots_processing=True)
# Set some edge cases for balances
max = spec.MAX_EFFECTIVE_BALANCE_ELECTRA if with_compounding_credentials else spec.MAX_EFFECTIVE_BALANCE
min = spec.config.EJECTION_BALANCE
Expand Down Expand Up @@ -69,3 +72,6 @@ def run_test_effective_balance_hysteresis(spec, state, with_compounding_credenti

for i, (_, _, post_eff, name) in enumerate(cases):
assert state.validators[i].effective_balance == post_eff, name

run_epoch_processing_from(spec, state, 'process_effective_balance_updates')
yield 'post_epoch', state
20 changes: 20 additions & 0 deletions tests/formats/epoch_processing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ An SSZ-snappy encoded `BeaconState`, the state before running the epoch sub-tran

An SSZ-snappy encoded `BeaconState`, the state after applying the epoch sub-transition. No value if the sub-transition processing is aborted.

### `pre_epoch.ssz_snappy`

An SSZ-snappy encoded `BeaconState`, the state before running the epoch transition.

### `post_epoch.ssz_snappy`

An SSZ-snappy encoded `BeaconState`, the state after applying the epoch transition. No value if the transition processing is aborted.

## Condition

A handler of the `epoch_processing` test-runner should process these cases,
Expand Down Expand Up @@ -50,3 +58,15 @@ Sub-transitions:
- `pending_deposits` (>=Electra)

The resulting state should match the expected `post` state.

## Condition (alternative)

Instead of having a different handler for each sub-transition, a single handler for all cases should load `pre_full` state, call `process_epoch` and then assert that the result state should match `post_full` state.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why it should be instead and couldn't be in addition to the pre and post state checks?


This has the advantages:

- Less code to maintain for the epoch processing handler.
- Works with single pass epoch processing.
- Can detect bugs related to data dependencies between different sub-transitions.

As a disadvantage this condition takes more resources to compute, but just a constant amount per test vector.