From 78f47f2069ec753c08dd7b278f7fc073b086cc34 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 19 Mar 2019 12:23:17 +0000 Subject: [PATCH 1/3] Avoid underflow in voluntary exits --- specs/core/0_beacon-chain.md | 1 + 1 file changed, 1 insertion(+) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index a631bf2fc6..be6a52e682 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -2472,6 +2472,7 @@ def process_voluntary_exit(state: BeaconState, exit: VoluntaryExit) -> None: # 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 + assert validator.activation_epoch != FAR_FUTURE_EPOCH assert get_current_epoch(state) - validator.activation_epoch >= PERSISTENT_COMMITTEE_PERIOD # Verify signature assert bls_verify( From b664453a342d88b20a351a90e4aac7ce5a901fa5 Mon Sep 17 00:00:00 2001 From: Justin Date: Tue, 19 Mar 2019 20:43:05 +0000 Subject: [PATCH 2/3] Update 0_beacon-chain.md --- specs/core/0_beacon-chain.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 940343e51a..910646487f 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -2435,8 +2435,9 @@ def process_voluntary_exit(state: BeaconState, exit: VoluntaryExit) -> None: 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 - assert validator.activation_epoch != FAR_FUTURE_EPOCH + # Verify the validator is active + assert is_active_validator(validator, state) + # Verify the validator has been active long enough assert get_current_epoch(state) - validator.activation_epoch >= PERSISTENT_COMMITTEE_PERIOD # Verify signature assert bls_verify( @@ -2445,7 +2446,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) ``` From acd7fdd762b19a3758e4fadd481f672b7843d32b Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 19 Mar 2019 15:49:01 -0600 Subject: [PATCH 3/3] add a few voluntary exit tests --- Makefile | 2 +- specs/core/0_beacon-chain.md | 4 +- .../test_process_block_header.py | 4 +- .../block_processing/test_voluntary_exit.py | 170 ++++++++++++++++++ tests/phase0/conftest.py | 6 + tests/phase0/helpers.py | 28 +++ tests/phase0/test_sanity.py | 12 +- 7 files changed, 213 insertions(+), 13 deletions(-) rename tests/phase0/{ => block_processing}/test_process_block_header.py (85%) create mode 100644 tests/phase0/block_processing/test_voluntary_exit.py diff --git a/Makefile b/Makefile index b45cec410e..88f17dcf99 100644 --- a/Makefile +++ b/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: diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 85e35d595b..212cedb955 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -2431,14 +2431,14 @@ 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 - # Verify the validator is active - assert is_active_validator(validator, state) # Verify the validator has been active long enough assert get_current_epoch(state) - validator.activation_epoch >= PERSISTENT_COMMITTEE_PERIOD # Verify signature diff --git a/tests/phase0/test_process_block_header.py b/tests/phase0/block_processing/test_process_block_header.py similarity index 85% rename from tests/phase0/test_process_block_header.py rename to tests/phase0/block_processing/test_process_block_header.py index 83d99e574b..4ec7e336f3 100644 --- a/tests/phase0/test_process_block_header.py +++ b/tests/phase0/block_processing/test_process_block_header.py @@ -10,8 +10,8 @@ build_empty_block_for_next_slot, ) -# mark entire file as 'sanity' and 'header' -pytestmark = [pytest.mark.sanity, pytest.mark.header] +# mark entire file as 'header' +pytestmark = pytest.mark.header def test_proposer_slashed(state): diff --git a/tests/phase0/block_processing/test_voluntary_exit.py b/tests/phase0/block_processing/test_voluntary_exit.py new file mode 100644 index 0000000000..80fad86a1b --- /dev/null +++ b/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 diff --git a/tests/phase0/conftest.py b/tests/phase0/conftest.py index e92896e924..395929028d 100644 --- a/tests/phase0/conftest.py +++ b/tests/phase0/conftest.py @@ -5,6 +5,7 @@ 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]) diff --git a/tests/phase0/helpers.py b/tests/phase0/helpers.py index 510361e9c0..2c79940790 100644 --- a/tests/phase0/helpers.py +++ b/tests/phase0/helpers.py @@ -13,6 +13,7 @@ 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 diff --git a/tests/phase0/test_sanity.py b/tests/phase0/test_sanity.py index 8c7e7d28b1..b9d44a72cb 100644 --- a/tests/phase0/test_sanity.py +++ b/tests/phase0/test_sanity.py @@ -43,6 +43,7 @@ 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 - for crosslink in pre_state.latest_crosslinks: - crosslink.epoch = pre_state.finalized_epoch - pre_state.validator_registry_update_epoch = pre_state.finalized_epoch - 1 + force_registry_change_at_next_epoch(pre_state) 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 - for crosslink in pre_state.latest_crosslinks: - crosslink.epoch = pre_state.finalized_epoch + force_registry_change_at_next_epoch(pre_state) # 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