Status code / slot reorganization #374
Conversation
* Added `activation_slot`, `exit_slot`, `penalized_slot`, `withdrawal_slot`, use these to determine if a validator is active * Universal min activation/exit delay of 256 slots * Min exit time of ~1 day, but penalization delays this to ~18 days * Penalty calculation period of `[time penalized - 18 days, time penalized + 18 days]`; made the total penalties array fixed size and wraparound to make calculation more fine-grained * Processes withdrawals in all epochs, not just dynasty-changing epochs * Change `get_shuffling` function to take slot as argument Not yet done: * Removed `shard_committees` from the state * Removed persistent committees from the state
Co-Authored-By: vbuterin <v@buterin.com>
Cleanups * (typo) Remove `get_new_validator_registry_delta_chain_tip` from table of contents * (typo) Update "Routines for updating validator status" in table of contents * Update `FAR_FUTURE_SLOT` from `2**63` to `2**64 - 1` * Put more constants in "Initial values", homogenise * Cleanup note formatting * Remove `ZERO_BALANCE_VALIDATOR_TTL` logic (to be possibly reintroduced in phase 2). * Cleanup `min_empty_validator_index` * Rename `deposit` to `amount` in `process_deposit` and `DepositData`. * (typo) Remove new line under `process_penalties_and_exits` * (typo) "Status codes" => "Status flags" in the table of contents * (typo) `(state.slot - EPOCH_LENGTH) % LATEST_RANDAO_MIXES_LENGTH` => Use `SEED_LOOKAHEAD` instead. * Put `state.validator_registry_latest_change_slot = state.slot` in `update_validator_registry`. * Use `GENESIS_SLOT` for `last_poc_change_slot=0` and `second_last_poc_change_slot=0`. Bugfixes * (typo) `validator_exit` => `exit.validator_index` * Separate initial deposits and initial activations to avoid double activations * Replace `proposer.status != EXITED_WITH_PENALTY` with `validator.penalized_slot > state.slot` in two different places. * Replace `status == EXITED_WITH_PENALTY` with `validator.penalized_slot <= state.slot` (and validator active) in two different places.
|
Comments regarding
|
| Activate the validator with the given ``index``. | ||
| Note that this function mutates ``state``. | ||
| """ | ||
| def activate_validator(state: BeaconState, index: int, genesis: bool) -> None: |
There was a problem hiding this comment.
Could take in param activation_slot instead of genesis to remove the forever genesis conditional sitting around.
There was a problem hiding this comment.
I like the idea that the fact that activation is delayed by 256 slots is part of the activation function, and not whatever is calling it; it feels like the right place to have that logic. activation_slot would make things less intuitive in that regard imo; it would also break symmetry with exit_validator and other similar functions that don't have an activation slot.
There was a problem hiding this comment.
- Did we agree on no withdrawal logic in phase 0? That is, can the 20 lines of code starting from
def eligible(index):be deleted? (That would explain a lot of the above!)
Did we? I thought we agreed that we finish at validators being in the WITHDRAWABLE state, and "no withdrawals" meant we don't try to move beyond that point (as doing that would require for example specifying a withdrawal shard, and in general is more dependent on state execution).
If we don't go up to WITHDRAWABLE in phase 0, then in phase 1 we would either need to have special-case code for a one-time sweep that converts all already-eligible accounts to the WITHDRAWABLE state, or have a large one-time backlog of validators that go through the exit queue, both of which seem uglier than just implementing it now.
total_penalties * 3=> Explain the hardcoded number 3 (maybe create a new constant)
It ensures that penalties are 100% in the specific case where enough validators misbehave to cause a safety failure. I'm ok with making it a constant or not making it a constant.
This 3 is related to the justification/finalization threshold (1/3). We might define that in the spec in general so that it can be set depending on the instantiation of the protocol (for instance #357). This number here can then be a function of the safety threshold constant(s) |
Also trim trailing whitespace
I'm pro having them end in the withdrawable state. I think it's cleaner to in terms of processing penalties. |
| state.latest_penalized_exit_balances[(state.slot // EPOCH_LENGTH) % LATEST_PENALIZED_EXIT_LENGTH] += get_effective_balance(state, index) | ||
|
|
||
| whistleblower_index = get_beacon_proposer_index(state, state.slot) | ||
| whistleblower_reward = get_effective_balance(state, index) // WHISTLEBLOWER_REWARD_QUOTIENT |
There was a problem hiding this comment.
get_effective_balance(state, index) is called twice in this function, could be refactored:
exit_validator(state, index)
validator = state.validator_registry[index]
last_penalized_epoch = (state.slot // EPOCH_LENGTH) % LATEST_PENALIZED_EXIT_LENGTH
effective_balance = get_effective_balance(state, index)
state.latest_penalized_exit_balances[last_penalized_epoch] += effective_balance
whistleblower_index = get_beacon_proposer_index(state, state.slot)
whistleblower_reward = effective_balance // WHISTLEBLOWER_REWARD_QUOTIENTThere was a problem hiding this comment.
get_effective_balance is not a significant computational cost and it's not much longer than a variable name, so not sure that's actually a net improvement, but I'm fine wither way.
There was a problem hiding this comment.
helper var not needed here in the spec imo
|
fixed merge conflict. Ready to merge @vbuterin |
|
I've also implemented these changes in code, if y'all wanna see what the diff looks like in code, here it is 😄 https://github.com/prysmaticlabs/prysm/pull/1233/files |
Sync ethereum/consensus-specs#374 part1: types, constants, deposit_helpers
Supersedes #350 and #354
activation_slot,exit_slot,penalized_slot,withdrawal_slot, use these to determine if a validator is active[time penalized - 18 days, time penalized + 18 days]; made the total penalties array fixed size and wraparound to make calculation more fine-grainedget_shufflingfunction to take slot as argumentNot yet done:
shard_committeesfrom the state