Merge pull request #1454 from ethereum/forkchoice-tests-fixed

Forkchoice tests fixed

specs/core/0_fork-choice.md

@@ -158,7 +158,7 @@ def on_block(store: Store, block: BeaconBlock) -> None:
     # Check that block is later than the finalized epoch slot
     assert block.slot > compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)
     # Check the block is valid and compute the post-state
-    state = state_transition(pre_state, block)
+    state = state_transition(pre_state, block, True)
     # Add new state for this block to the store
     store.block_states[signing_root(block)] = state
 

test_libs/pyspec/eth2spec/test/context.py

@@ -143,7 +143,9 @@ def bls_switch(fn):
     def entry(*args, **kw):
         old_state = bls.bls_active
         bls.bls_active = kw.pop('bls_active', DEFAULT_BLS_ACTIVE)
-        yield from fn(*args, **kw)
+        res = fn(*args, **kw)
+        if res is not None:
+            yield from res
         bls.bls_active = old_state
     return entry
 

test_libs/pyspec/eth2spec/test/fork_choice/test_get_head.py

@@ -1,4 +1,4 @@
-from eth2spec.test.context import with_all_phases, with_state, bls_switch
+from eth2spec.test.context import with_all_phases, spec_state_test
 from eth2spec.test.helpers.attestations import get_valid_attestation
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot
 from eth2spec.test.helpers.state import state_transition_and_sign_block
@@ -27,8 +27,7 @@ def add_attestation_to_store(spec, store, attestation):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_genesis(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -37,8 +36,7 @@ def test_genesis(spec, state):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_chain_no_attestations(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -59,8 +57,7 @@ def test_chain_no_attestations(spec, state):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_split_tie_breaker_no_attestations(spec, state):
     genesis_state = state.copy()
 
@@ -88,8 +85,7 @@ def test_split_tie_breaker_no_attestations(spec, state):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_shorter_chain_but_heavier_weight(spec, state):
     genesis_state = state.copy()
 

test_libs/pyspec/eth2spec/test/fork_choice/test_on_attestation.py

@@ -1,8 +1,10 @@
-from eth2spec.test.context import with_all_phases, with_state, bls_switch, with_phases
+
+from eth2spec.test.context import with_all_phases, spec_state_test, with_phases
+
 
 from eth2spec.test.helpers.block import build_empty_block_for_next_slot
 from eth2spec.test.helpers.attestations import get_valid_attestation
-from eth2spec.test.helpers.state import next_slot
+from eth2spec.test.helpers.state import state_transition_and_sign_block
 
 
 def run_on_attestation(spec, state, store, attestation, valid=True):
@@ -26,27 +28,24 @@ def run_on_attestation(spec, state, store, attestation, valid=True):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_attestation(spec, state):
     store = spec.get_genesis_store(state)
     time = 100
     spec.on_tick(store, time)
 
-    block = build_empty_block_for_next_slot(spec, state, signed=True)
+    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, state, block)
 
     # store block in store
     spec.on_block(store, block)
 
-    next_slot(spec, state)
-
     attestation = get_valid_attestation(spec, state, slot=block.slot)
     run_on_attestation(spec, state, store, attestation)
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_attestation_target_not_in_store(spec, state):
     store = spec.get_genesis_store(state)
     time = 100
@@ -55,28 +54,27 @@ def test_on_attestation_target_not_in_store(spec, state):
     # move to next epoch to make block new target
     state.slot += spec.SLOTS_PER_EPOCH
 
-    block = build_empty_block_for_next_slot(spec, state, signed=True)
+    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, state, block)
 
     # do not add block to store
 
-    next_slot(spec, state)
     attestation = get_valid_attestation(spec, state, slot=block.slot)
     run_on_attestation(spec, state, store, attestation, False)
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_attestation_future_epoch(spec, state):
     store = spec.get_genesis_store(state)
     time = 3 * spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 
-    block = build_empty_block_for_next_slot(spec, state, signed=True)
+    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, state, block)
 
     # store block in store
     spec.on_block(store, block)
-    next_slot(spec, state)
 
     # move state forward but not store
     attestation_slot = block.slot + spec.SLOTS_PER_EPOCH
@@ -87,36 +85,34 @@ def test_on_attestation_future_epoch(spec, state):
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_attestation_same_slot(spec, state):
     store = spec.get_genesis_store(state)
     time = 1 * spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 
-    block = build_empty_block_for_next_slot(spec, state, signed=True)
+    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, state, block)
 
     spec.on_block(store, block)
-    next_slot(spec, state)
 
     attestation = get_valid_attestation(spec, state, slot=block.slot)
     run_on_attestation(spec, state, store, attestation, False)
 
 
 @with_phases(['phase0'])
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_attestation_invalid_attestation(spec, state):
     store = spec.get_genesis_store(state)
     time = 3 * spec.SECONDS_PER_SLOT
     spec.on_tick(store, time)
 
-    block = build_empty_block_for_next_slot(spec, state, signed=True)
+    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, state, block)
 
     spec.on_block(store, block)
-    next_slot(spec, state)
 
     attestation = get_valid_attestation(spec, state, slot=block.slot)
-    # make attestation invalid
+    # make attestation invalid by setting a phase1-only custody bit
-    attestation.custody_bits[0:8] = [0, 0, 0, 0, 1, 1, 1, 1]
+    attestation.custody_bits[0] = 1
     run_on_attestation(spec, state, store, attestation, False)

test_libs/pyspec/eth2spec/test/fork_choice/test_on_block.py

@@ -1,11 +1,12 @@
+from copy import deepcopy
 from eth2spec.utils.ssz.ssz_impl import signing_root
 
-from eth2spec.test.context import with_all_phases, with_state, bls_switch
+from eth2spec.test.context import with_all_phases, spec_state_test
-from eth2spec.test.helpers.block import build_empty_block_for_next_slot
+from eth2spec.test.helpers.block import build_empty_block_for_next_slot, sign_block
-from eth2spec.test.helpers.state import next_epoch, next_epoch_with_attestations
+from eth2spec.test.helpers.state import next_epoch, next_epoch_with_attestations, state_transition_and_sign_block
 
 
-def run_on_block(spec, state, store, block, valid=True):
+def run_on_block(spec, store, block, valid=True):
     if not valid:
         try:
             spec.on_block(store, block)
@@ -19,19 +20,18 @@ def run_on_block(spec, state, store, block, valid=True):
 
 
 def apply_next_epoch_with_attestations(spec, state, store):
-    _, new_blocks, state = next_epoch_with_attestations(spec, state, True, False)
+    _, new_blocks, post_state = next_epoch_with_attestations(spec, state, True, False)
     for block in new_blocks:
         block_root = signing_root(block)
         store.blocks[block_root] = block
-        store.block_states[block_root] = state
+        store.block_states[block_root] = post_state
         last_block = block
     spec.on_tick(store, store.time + state.slot * spec.SECONDS_PER_SLOT)
-    return state, store, last_block
+    return post_state, store, last_block
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_basic(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -41,21 +41,22 @@ def test_basic(spec, state):
 
     # On receiving a block of `GENESIS_SLOT + 1` slot
     block = build_empty_block_for_next_slot(spec, state)
-    run_on_block(spec, state, store, block)
+    state_transition_and_sign_block(spec, state, block)
+    run_on_block(spec, store, block)
 
     # On receiving a block of next epoch
     store.time = time + spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH
     block = build_empty_block_for_next_slot(spec, state)
     block.slot += spec.SLOTS_PER_EPOCH
+    state_transition_and_sign_block(spec, state, block)
 
-    run_on_block(spec, state, store, block)
+    run_on_block(spec, store, block)
 
     # TODO: add tests for justified_root and finalized_root
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_block_checkpoints(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -70,18 +71,18 @@ def test_on_block_checkpoints(spec, state):
     last_block_root = signing_root(last_block)
 
     # Mock the finalized_checkpoint
-    store.block_states[last_block_root].finalized_checkpoint = (
+    fin_state = store.block_states[last_block_root]
+    fin_state.finalized_checkpoint = (
         store.block_states[last_block_root].current_justified_checkpoint
     )
 
-    # On receiving a block of `GENESIS_SLOT + 1` slot
+    block = build_empty_block_for_next_slot(spec, fin_state)
-    block = build_empty_block_for_next_slot(spec, state)
+    state_transition_and_sign_block(spec, deepcopy(fin_state), block)
-    run_on_block(spec, state, store, block)
+    run_on_block(spec, store, block)
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_block_future_block(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -90,12 +91,12 @@ def test_on_block_future_block(spec, state):
 
     # Fail receiving block of `GENESIS_SLOT + 1` slot
     block = build_empty_block_for_next_slot(spec, state)
-    run_on_block(spec, state, store, block, False)
+    state_transition_and_sign_block(spec, state, block)
+    run_on_block(spec, store, block, False)
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_block_bad_parent_root(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -104,13 +105,18 @@ def test_on_block_bad_parent_root(spec, state):
 
     # Fail receiving block of `GENESIS_SLOT + 1` slot
     block = build_empty_block_for_next_slot(spec, state)
+    spec.state_transition(state, block)
+    block.state_root = state.hash_tree_root()
+
     block.parent_root = b'\x45' * 32
-    run_on_block(spec, state, store, block, False)
+
+    sign_block(spec, state, block)
+
+    run_on_block(spec, store, block, False)
 
 
 @with_all_phases
-@with_state
+@spec_state_test
-@bls_switch
 def test_on_block_before_finalized(spec, state):
     # Initialization
     store = spec.get_genesis_store(state)
@@ -124,4 +130,5 @@ def test_on_block_before_finalized(spec, state):
 
     # Fail receiving block of `GENESIS_SLOT + 1` slot
     block = build_empty_block_for_next_slot(spec, state)
-    run_on_block(spec, state, store, block, False)
+    state_transition_and_sign_block(spec, state, block)
+    run_on_block(spec, store, block, False)