Skip to content
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

Prevent validators from exiting when there are open bit or chunk challenges or when not all custody keys were revealed #1348

Open
dankrad opened this issue Aug 8, 2019 · 2 comments
Labels

Comments

@dankrad
Copy link
Contributor

dankrad commented Aug 8, 2019

We need to prevent validators from exiting voluntarily if any of these conditions are fulfilled:

  • Open chunk/bit challenges
  • Custody keys not revealed

This was originally implemented by the function below, but the function and its attachment point was lost over some refactoring of the exit queue:

def eligible(state: BeaconState, index: ValidatorIndex) -> bool:
    validator = state.validator_registry[index]
    # Cannot exit if there are still open chunk challenges
    if len([record for record in state.custody_chunk_challenge_records if record.responder_index == index]) > 0:
        return False
    # Cannot exit if there are still open bit challenges
    if len([record for record in state.custody_bit_challenge_records if record.responder_index == index]) > 0:
        return False
    # Cannot exit if you have not revealed all of your custody keys
    elif validator.next_custody_reveal_period <= get_validators_custody_reveal_period(state, index, validator.exit_epoch):
        return False
    # Cannot exit if you already have
    elif validator.withdrawable_epoch < FAR_FUTURE_EPOCH:
        return False
    # Return minimum time
    else:
        return current_epoch >= validator.exit_epoch + MIN_VALIDATOR_WITHDRAWAL_EPOCHS
@dankrad dankrad added general:bug Something isn't working phase1 labels Aug 8, 2019
@hwwhww
Copy link
Contributor

hwwhww commented Aug 8, 2019

Recap

Case 1: the validator initiated exit at epoch N, and then got slashed at epoch N+k

  1. In initiate_validator_exit, validator.withdrawable_epoch is set to Epoch(validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY)
  2. In process_chunk_challenge and process_bit_challenge, responder.withdrawable_epoch is set to FAR_FUTURE_EPOCH.

Case 2: the validator got challenged at epoch N, and then initiated exit at epoch N+k

  1. In process_chunk_challenge and process_bit_challenge, responder.withdrawable_epoch is set to FAR_FUTURE_EPOCH.
  2. In initiate_validator_exit, validator.withdrawable_epoch is set to Epoch(validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY) <--- oh no!! 🤯

Proposing solutions

Solution 1: update after_process_final_updates to delay withdrawable_epoch

    for index, validator in enumerate(state.validators):
        if index in validator_indices_in_records:
            # Delay withdrawable epochs if challenge records are not empty
            validator.withdrawable_epoch = Epoch(current_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY)
        else:
            # Reset withdrawable epochs if challenge records are empty
            if validator.exit_epoch != FAR_FUTURE_EPOCH and validator.withdrawable_epoch == FAR_FUTURE_EPOCH:
                validator.withdrawable_epoch = Epoch(validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY)

Solution 2: Extract checks into initiate_validator_withdrawable:

#1066 was the patch before #1035 was merged… and somehow I was convinced that #1035 already fixed everything. 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants
@dankrad @hwwhww and others