AssertionError during on_attestation if an attestation references a new target checkpoint

[ericsson49]

Recent update to process_slots induces consequences in other parts of the eth2.0 specs, which invoke the process_slots method - AssertError can be thrown, while it worked fine before the update, in the case state.slot == slot. In the update diff, one can see that process_slots is guarded with state.slot < slot to prevent such problems.
However, there are other cases when process_slots is called, but corresponding call site is not updated with such a guard.
One such example is store_target_checkpoint_state. That leads to problems. For example, one can create attestations during the first (i.e. zero) epoch, however, once an epoch is justified, i.e. there is a new checkpoint, store_target_checkpoint_state is called, which tries to invoke process_slots, which results in the AssertError, unless next epoch arrives.

Here is a snippet, illustrating the problem.

from eth2spec.utils import bls
bls.bls_active = False

from eth2spec.phase0 import spec

configs_path = '../../../configs/'
from eth2spec.config import config_util
from importlib import reload
config_util.prepare_config(configs_path, 'minimal')
reload(spec)

from eth2spec.test.helpers.deposits import prepare_genesis_deposits
from eth2spec.test.helpers.keys import pubkey_to_privkey

eth1_block_hash=b'\x42' * 32
eth1_timestamp=spec.MIN_GENESIS_TIME
deposit_count = 16
deposits, deposit_root, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True)
genesis_state = spec.initialize_beacon_state_from_eth1(eth1_block_hash, eth1_timestamp, deposits)

store = spec.get_forkchoice_store(genesis_state)

def set_slot(slot):
    spec.on_tick(store, store.genesis_time+spec.SECONDS_PER_SLOT*slot)

def mk_block(slot,head,atts):
    head_state = store.block_states[head].copy()
    if head_state.slot < slot-1:
        spec.process_slots(head_state, slot-1)
    state = head_state.copy()
    spec.process_slots(state, slot)

    proposer = spec.get_beacon_proposer_index(state)
    SK = pubkey_to_privkey[state.validators[proposer].pubkey]
    randao_reveal = spec.get_epoch_signature(state, spec.BeaconBlock(slot=slot), SK)
    eth1vote = spec.get_eth1_vote(state, [])
    block = spec.BeaconBlock(
        slot=slot, proposer_index=proposer, parent_root=head,
        body=spec.BeaconBlockBody(attestations=atts, eth1_data=eth1vote))
    block.state_root = spec.compute_new_state_root(head_state.copy(), block)
    block_signature = spec.get_block_signature(state, block, SK)
    return spec.SignedBeaconBlock(message=block, signature=block_signature)

def mk_atts(slot, head):
    head_state = store.block_states[head].copy()
    if head_state.slot < int(slot)-1:
        spec.process_slots(head_state, slot-1)
    state = head_state.copy()
    if state.slot < slot:
        spec.process_slots(state, slot)
    
    src = state.current_justified_checkpoint
    start_slot = spec.compute_start_slot_at_epoch(spec.get_current_epoch(state))
    epoch_boundary_block_root = head if start_slot == state.slot else spec.get_block_root_at_slot(state, start_slot)
    target_chkpt = spec.Checkpoint(epoch=spec.get_current_epoch(state), root=epoch_boundary_block_root)
    atts = []
    committee_count = spec.get_committee_count_at_slot(state, slot)
    for index in range(committee_count):
        data = spec.AttestationData(slot=slot, index=index, beacon_block_root=head,
            source=state.current_justified_checkpoint,
            target=target_chkpt)
        committee = spec.get_beacon_committee(state, slot, index)
        for i in range(len(committee)):
            bits = spec.Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE]([0] * len(committee))
            bits[i] = 1
            att_sig = spec.get_attestation_signature(state, data,
                    pubkey_to_privkey[state.validators[committee[i]].pubkey])
            atts.append(spec.Attestation(aggregation_bits=bits, data=data, signature=att_sig))
    return atts

def do_step():
    slot = spec.get_current_slot(store)
    head = spec.get_head(store)
    atts = mk_atts(slot, head)
    slot = slot + 1
    set_slot(slot)
    for a in atts:
        spec.on_attestation(store, a)
    b = mk_block(slot, head, atts)
    spec.on_block(store, b)

for i in range(spec.SLOTS_PER_EPOCH):
    do_step()

try:
    do_step()
except AssertionError as e:
    print('exception', e)
    pass

# wait until next epoch arrives
set_slot(spec.SLOTS_PER_EPOCH * 2)
do_step()

Traceback (most recent call last):
  File "testcase0.py", line 85, in <module>
    do_step()
  File "testcase0.py", line 78, in do_step
    spec.on_attestation(store, a)
  File "/mnt/c/Users/ericsson/IdeaProjects/tmp/eth2.0-specs_v0.11.3/venv/lib/python3.8/site-packages/eth2spec/phase0/spec.py", line 1519, in on_attestation
    store_target_checkpoint_state(store, attestation.data.target)
  File "/mnt/c/Users/ericsson/IdeaProjects/tmp/eth2.0-specs_v0.11.3/venv/lib/python3.8/site-packages/eth2spec/phase0/spec.py", line 1442, in store_target_checkpoint_state
    process_slots(base_state, compute_start_slot_at_epoch(target.epoch))
  File "/mnt/c/Users/ericsson/IdeaProjects/tmp/eth2.0-specs_v0.11.3/venv/lib/python3.8/site-packages/eth2spec/phase0/spec.py", line 836, in process_slots
    assert state.slot < slot
AssertionError

One can wait until next epoch, then do_step() is successfull.

One possible solution is to add a guard around process_slots call in the store_target_checkpoint_state method

def store_target_checkpoint_state(store: Store, target: Checkpoint) -> None:
    # Store target checkpoint state if not yet seen
    if target not in store.checkpoint_states:
        base_state = store.block_states[target.root].copy()
        epoch_start_slot = compute_start_slot_at_epoch(target.epoch)
        if base_state.slot < epoch_start_slot:
            process_slots(base_state, epoch_start_slot)
        store.checkpoint_states[target] = base_state

[adiasg]

Summary of issue: When a new attestation is received with target not in store.checkpoint_states, there can be a failure in assert state.slot < slot in the call on_attestation > store_target_checkpoint_state > process_slots.

Cause of issue: process_slots is called in store_target_checkpoint_state to adjust the checkpoint state for "pulled up" epoch boundary blocks (see image), which breaks after the new changes when target.root isn't a pulled up block.

One can wait until next epoch, then do_step() is successfull.

This is working because of the set_slot(spec.SLOTS_PER_EPOCH * 2) call, which skips all blocks until the next epoch, leading to a pulled-up epoch boundary block.

Solution: The suggested solution is the appropriate.

[ericsson49]

it's reproducible in both dev and v0.11.3