Merge pull request #807 from ethereum/JustinDrake-patch-20

Avoid underflow in voluntary exits

Makefile

@@ -17,7 +17,7 @@ clean:
 # runs a limited set of tests against a minimal config
 # run pytest with `-m` option to full suite
 test:
-	pytest -m "sanity and minimal_config" tests/
+	pytest -m minimal_config tests/
 
 
 $(BUILD_DIR)/phase0:

specs/core/0_beacon-chain.md

@@ -2431,13 +2431,15 @@ def process_voluntary_exit(state: BeaconState, exit: VoluntaryExit) -> None:
     Note that this function mutates ``state``.
     """
     validator = state.validator_registry[exit.validator_index]
+    # Verify the validator is active
+    assert is_active_validator(validator, get_current_epoch(state))
     # Verify the validator has not yet exited
     assert validator.exit_epoch == FAR_FUTURE_EPOCH
     # Verify the validator has not initiated an exit
     assert validator.initiated_exit is False
     # Exits must specify an epoch when they become valid; they are not valid before then
     assert get_current_epoch(state) >= exit.epoch
-    # Must have been in the validator set long enough
+    # Verify the validator has been active long enough
     assert get_current_epoch(state) - validator.activation_epoch >= PERSISTENT_COMMITTEE_PERIOD
     # Verify signature
     assert bls_verify(
@@ -2446,7 +2448,7 @@ def process_voluntary_exit(state: BeaconState, exit: VoluntaryExit) -> None:
         signature=exit.signature,
         domain=get_domain(state.fork, exit.epoch, DOMAIN_VOLUNTARY_EXIT)
     )
-    # Run the exit
+    # Initiate exit
     initiate_validator_exit(state, exit.validator_index)
 ```
 

tests/phase0/block_processing/test_process_block_header.py

@@ -10,8 +10,8 @@ from tests.phase0.helpers import (
     build_empty_block_for_next_slot,
 )
 
-# mark entire file as 'sanity' and 'header'
+# mark entire file as 'header'
-pytestmark = [pytest.mark.sanity, pytest.mark.header]
+pytestmark = pytest.mark.header
 
 
 def test_proposer_slashed(state):

tests/phase0/block_processing/test_voluntary_exit.py

@@ -0,0 +1,170 @@
+from copy import deepcopy
+import pytest
+
+import build.phase0.spec as spec
+
+from build.phase0.spec import (
+    get_active_validator_indices,
+    get_current_epoch,
+    process_voluntary_exit,
+)
+from tests.phase0.helpers import (
+    build_voluntary_exit,
+)
+
+
+def test_success(state, pub_to_priv):
+    pre_state = deepcopy(state)
+    #
+    # setup pre_state
+    #
+    # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow for exit
+    pre_state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
+
+    #
+    # build voluntary exit
+    #
+    current_epoch = get_current_epoch(pre_state)
+    validator_index = get_active_validator_indices(pre_state.validator_registry, current_epoch)[0]
+    privkey = pub_to_priv[pre_state.validator_registry[validator_index].pubkey]
+
+    voluntary_exit = build_voluntary_exit(
+        pre_state,
+        current_epoch,
+        validator_index,
+        privkey,
+    )
+
+    post_state = deepcopy(pre_state)
+
+    #
+    # test valid exit
+    #
+    process_voluntary_exit(post_state, voluntary_exit)
+
+    assert not pre_state.validator_registry[validator_index].initiated_exit
+    assert post_state.validator_registry[validator_index].initiated_exit
+
+    return pre_state, voluntary_exit, post_state
+
+
+def test_validator_not_active(state, pub_to_priv):
+    pre_state = deepcopy(state)
+    current_epoch = get_current_epoch(pre_state)
+    validator_index = get_active_validator_indices(pre_state.validator_registry, current_epoch)[0]
+    privkey = pub_to_priv[pre_state.validator_registry[validator_index].pubkey]
+
+    #
+    # setup pre_state
+    #
+    pre_state.validator_registry[validator_index].activation_epoch = spec.FAR_FUTURE_EPOCH
+
+    #
+    # build and test voluntary exit
+    #
+    voluntary_exit = build_voluntary_exit(
+        pre_state,
+        current_epoch,
+        validator_index,
+        privkey,
+    )
+
+    with pytest.raises(AssertionError):
+        process_voluntary_exit(pre_state, voluntary_exit)
+
+    return pre_state, voluntary_exit, None
+
+
+def test_validator_already_exited(state, pub_to_priv):
+    pre_state = deepcopy(state)
+    #
+    # setup pre_state
+    #
+    # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow validator able to exit
+    pre_state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
+
+    current_epoch = get_current_epoch(pre_state)
+    validator_index = get_active_validator_indices(pre_state.validator_registry, current_epoch)[0]
+    privkey = pub_to_priv[pre_state.validator_registry[validator_index].pubkey]
+
+    # but validator already has exited
+    pre_state.validator_registry[validator_index].exit_epoch = current_epoch + 2
+
+    #
+    # build voluntary exit
+    #
+    voluntary_exit = build_voluntary_exit(
+        pre_state,
+        current_epoch,
+        validator_index,
+        privkey,
+    )
+
+    with pytest.raises(AssertionError):
+        process_voluntary_exit(pre_state, voluntary_exit)
+
+    return pre_state, voluntary_exit, None
+
+
+def test_validator_already_initiated_exit(state, pub_to_priv):
+    pre_state = deepcopy(state)
+    #
+    # setup pre_state
+    #
+    # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow validator able to exit
+    pre_state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
+
+    current_epoch = get_current_epoch(pre_state)
+    validator_index = get_active_validator_indices(pre_state.validator_registry, current_epoch)[0]
+    privkey = pub_to_priv[pre_state.validator_registry[validator_index].pubkey]
+
+    # but validator already has initiated exit
+    pre_state.validator_registry[validator_index].initiated_exit = True
+
+    #
+    # build voluntary exit
+    #
+    voluntary_exit = build_voluntary_exit(
+        pre_state,
+        current_epoch,
+        validator_index,
+        privkey,
+    )
+
+    with pytest.raises(AssertionError):
+        process_voluntary_exit(pre_state, voluntary_exit)
+
+    return pre_state, voluntary_exit, None
+
+
+def test_validator_not_active_long_enough(state, pub_to_priv):
+    pre_state = deepcopy(state)
+    #
+    # setup pre_state
+    #
+    current_epoch = get_current_epoch(pre_state)
+    validator_index = get_active_validator_indices(pre_state.validator_registry, current_epoch)[0]
+    privkey = pub_to_priv[pre_state.validator_registry[validator_index].pubkey]
+
+    # but validator already has initiated exit
+    pre_state.validator_registry[validator_index].initiated_exit = True
+
+    #
+    # build voluntary exit
+    #
+    voluntary_exit = build_voluntary_exit(
+        pre_state,
+        current_epoch,
+        validator_index,
+        privkey,
+    )
+
+    assert (
+        current_epoch - pre_state.validator_registry[validator_index].activation_epoch <
+        spec.PERSISTENT_COMMITTEE_PERIOD
+    )
+
+    with pytest.raises(AssertionError):
+        process_voluntary_exit(pre_state, voluntary_exit)
+
+    return pre_state, voluntary_exit, None

tests/phase0/conftest.py

@@ -5,6 +5,7 @@ from build.phase0 import spec
 from tests.phase0.helpers import (
     privkeys_list,
     pubkeys_list,
+    pubkey_to_privkey,
     create_genesis_state,
 )
 
@@ -34,6 +35,11 @@ def pubkeys():
     return pubkeys_list
 
 
+@pytest.fixture
+def pub_to_priv():
+    return pubkey_to_privkey
+
+
 def overwrite_spec_config(config):
     for field in config:
         setattr(spec, field, config[field])

tests/phase0/helpers.py

@@ -13,6 +13,7 @@ from build.phase0.spec import (
     DepositInput,
     DepositData,
     Eth1Data,
+    VoluntaryExit,
     # functions
     get_block_root,
     get_current_epoch,
@@ -82,6 +83,14 @@ def create_genesis_state(num_validators, deposit_data_leaves):
     )
 
 
+def force_registry_change_at_next_epoch(state):
+    # artificially trigger registry update at next epoch transition
+    state.finalized_epoch = get_current_epoch(state) - 1
+    for crosslink in state.latest_crosslinks:
+        crosslink.epoch = state.finalized_epoch
+    state.validator_registry_update_epoch = state.finalized_epoch - 1
+
+
 def build_empty_block_for_next_slot(state):
     empty_block = get_empty_block()
     empty_block.slot = state.slot + 1
@@ -143,3 +152,22 @@ def build_attestation_data(state, slot, shard):
         crosslink_data_root=spec.ZERO_HASH,
         previous_crosslink=deepcopy(state.latest_crosslinks[shard]),
     )
+
+
+def build_voluntary_exit(state, epoch, validator_index, privkey):
+    voluntary_exit = VoluntaryExit(
+        epoch=epoch,
+        validator_index=validator_index,
+        signature=EMPTY_SIGNATURE,
+    )
+    voluntary_exit.signature = bls.sign(
+        message_hash=signed_root(voluntary_exit),
+        privkey=privkey,
+        domain=get_domain(
+            fork=state.fork,
+            epoch=epoch,
+            domain_type=spec.DOMAIN_VOLUNTARY_EXIT,
+        )
+    )
+
+    return voluntary_exit

tests/phase0/test_sanity.py

@@ -43,6 +43,7 @@ from tests.phase0.helpers import (
     build_attestation_data,
     build_deposit_data,
     build_empty_block_for_next_slot,
+    force_registry_change_at_next_epoch,
 )
 
 
@@ -324,10 +325,7 @@ def test_voluntary_exit(state, pubkeys, privkeys):
     # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow for exit
     pre_state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
     # artificially trigger registry update at next epoch transition
-    pre_state.finalized_epoch = get_current_epoch(pre_state) - 1
+    force_registry_change_at_next_epoch(pre_state)
-    for crosslink in pre_state.latest_crosslinks:
-        crosslink.epoch = pre_state.finalized_epoch
-    pre_state.validator_registry_update_epoch = pre_state.finalized_epoch - 1
 
     post_state = deepcopy(pre_state)
 
@@ -369,7 +367,7 @@ def test_voluntary_exit(state, pubkeys, privkeys):
     return pre_state, [initiate_exit_block, exit_block], post_state
 
 
-def test_no_exit_too_long_since_change(state):
+def test_no_exit_churn_too_long_since_change(state):
     pre_state = deepcopy(state)
     validator_index = get_active_validator_indices(
         pre_state.validator_registry,
@@ -382,9 +380,7 @@ def test_no_exit_too_long_since_change(state):
     # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow for exit
     pre_state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH
     # artificially trigger registry update at next epoch transition
-    pre_state.finalized_epoch = get_current_epoch(pre_state) - 1
+    force_registry_change_at_next_epoch(pre_state)
-    for crosslink in pre_state.latest_crosslinks:
-        crosslink.epoch = pre_state.finalized_epoch
     # make epochs since registry update greater than LATEST_SLASHED_EXIT_LENGTH
     pre_state.validator_registry_update_epoch = (
         get_current_epoch(pre_state) - spec.LATEST_SLASHED_EXIT_LENGTH