-
Notifications
You must be signed in to change notification settings - Fork 42
feat: electra process_registry_updates & slash_validators changes #1420
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
Conversation
…ne eject_validator fn
rodrigo-o
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 🚀 , added a small nit and a comment for the future.
| validators | ||
| |> Enum.with_index() | ||
| |> Enum.reduce_while(state, fn {validator, idx}, state -> | ||
| handle_validator_registry_update( | ||
| state, | ||
| validator, | ||
| idx, | ||
| current_epoch, | ||
| activation_exit_epoch, | ||
| ejection_balance | ||
| ) | ||
| end) | ||
| |> then(fn | ||
| %BeaconState{} = state -> {:ok, state} | ||
| {:error, reason} -> {:error, reason} | ||
| end) | ||
| end | ||
|
|
||
| result = | ||
| validators | ||
| |> Stream.with_index() | ||
| |> Stream.map(fn {v, i} -> | ||
| {{v, i}, Predicates.eligible_for_activation_queue?(v), | ||
| Predicates.active_validator?(v, current_epoch) and | ||
| v.effective_balance <= ejection_balance} | ||
| end) | ||
| |> Stream.filter(&(elem(&1, 1) or elem(&1, 2))) | ||
| |> Stream.map(fn | ||
| {{v, i}, true, b} -> {{%{v | activation_eligibility_epoch: current_epoch + 1}, i}, b} | ||
| {{v, i}, false = _is_eligible, b} -> {{v, i}, b} | ||
| end) | ||
| |> Enum.reduce({:ok, state}, fn | ||
| _, {:error, _} = err -> err | ||
| {{v, i}, should_be_ejected}, {:ok, st} -> eject_validator(st, v, i, should_be_ejected) | ||
| {err, _}, _ -> err | ||
| end) | ||
| defp handle_validator_registry_update( | ||
| state, | ||
| validator, | ||
| idx, | ||
| current_epoch, | ||
| activation_exit_epoch, | ||
| ejection_balance | ||
| ) do | ||
| cond do | ||
| Predicates.eligible_for_activation_queue?(validator) -> | ||
| updated_validator = %Validator{ | ||
| validator | ||
| | activation_eligibility_epoch: current_epoch + 1 | ||
| } | ||
|
|
||
| with {:ok, new_state} <- result do | ||
| new_state.validators | ||
| |> Stream.with_index() | ||
| |> Stream.filter(fn {v, _} -> Predicates.eligible_for_activation?(state, v) end) | ||
| |> Enum.sort_by(fn {%{activation_eligibility_epoch: ep}, i} -> {ep, i} end) | ||
| |> Enum.take(churn_limit) | ||
| |> Enum.reduce(new_state.validators, fn {v, i}, acc -> | ||
| %{v | activation_epoch: activation_exit_epoch} | ||
| |> then(&Aja.Vector.replace_at!(acc, i, &1)) | ||
| end) | ||
| |> then(&{:ok, %BeaconState{new_state | validators: &1}}) | ||
| end | ||
| end | ||
| {:cont, | ||
| %BeaconState{ | ||
| state | ||
| | validators: Aja.Vector.replace_at!(state.validators, idx, updated_validator) | ||
| }} | ||
|
|
||
| defp eject_validator(state, validator, index, false) do | ||
| {:ok, %{state | validators: Aja.Vector.replace_at!(state.validators, index, validator)}} | ||
| end | ||
| Predicates.active_validator?(validator, current_epoch) && | ||
| validator.effective_balance <= ejection_balance -> | ||
| case Mutators.initiate_validator_exit(state, validator) do | ||
| {:ok, state, ejected_validator} -> | ||
| updated_state = %{ | ||
| state | ||
| | validators: Aja.Vector.replace_at!(state.validators, idx, ejected_validator) | ||
| } | ||
|
|
||
| {:cont, updated_state} | ||
|
|
||
| {:error, msg} -> | ||
| {:halt, {:error, msg}} | ||
| end | ||
|
|
||
| Predicates.eligible_for_activation?(state, validator) -> | ||
| updated_validator = %Validator{ | ||
| validator | ||
| | activation_epoch: activation_exit_epoch | ||
| } | ||
|
|
||
| updated_state = %BeaconState{ | ||
| state | ||
| | validators: Aja.Vector.replace_at!(state.validators, idx, updated_validator) | ||
| } | ||
|
|
||
| {:cont, updated_state} | ||
|
|
||
| defp eject_validator(state, validator, index, true) do | ||
| with {:ok, ejected_validator} <- Mutators.initiate_validator_exit(state, validator) do | ||
| {:ok, | ||
| %{state | validators: Aja.Vector.replace_at!(state.validators, index, ejected_validator)}} | ||
| true -> | ||
| {:cont, state} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really prefer how now we have the cond in a separate function! But I'm a bit concerned about the complexity still, it's better than the previous version but I feel it's a really long function for the matching spec:
def process_registry_updates(state: BeaconState) -> None:
current_epoch = get_current_epoch(state)
activation_epoch = compute_activation_exit_epoch(current_epoch)
# Process activation eligibility, ejections, and activations
for index, validator in enumerate(state.validators):
if is_eligible_for_activation_queue(validator): # [Modified in Electra:EIP7251]
validator.activation_eligibility_epoch = current_epoch + 1
elif is_active_validator(validator, current_epoch) and validator.effective_balance <= EJECTION_BALANCE:
initiate_validator_exit(state, ValidatorIndex(index)) # [Modified in Electra:EIP7251]
elif is_eligible_for_activation(state, validator):
validator.activation_epoch = activation_epochI feel that we can simplify it even more, but probably for another moment when we already have all the spec-test running.
Motivation
Implement the changes in Electra related to
process_registry_updatesandslash_validatorsDescription
slash_validatorlinkget_balance_churn_limitlinkget_activation_exit_churn_limitlinkcompute_exit_epoch_and_update_churnlinkinitiate_validator_exitlinkprocess_registry_updateslinkFixed spec tests