Add EIP-7251 to Electra

[ralexstokes]

Adds EIP-7251 to Electra and fixes the tests for the full v0 Electra spec.

Likely will merge #3656 first.

## TODO

- [x] migrate 7251 tests
- [x] fix up constants for EL-init'd exits/withdrawals

[hwwhww]

/cc @etan-status

The gindex will be updated due to the increased number of fields. We should update it to FINALIZED_ROOT_GINDEX_ELECTRA.

Would this be a problem for the light client?

[etan-status]

Not a problem, the light client protocol is designed to be forward compatible, as in, the existing v1 REST endpoints and libp2p endpoints continue to work fine, even if these indices adjust with new forks. However, the sync-protocol.md needs to take the new constant into account though, for both proof generation as well as verification and in the data structures.

Further keep in mind that, due to the periodic nature of these reindexings, without EIP-7495, it is impossible to create an immutable verifier that lives on a different chain / hardware wallet / low-cadence software (EIP-7671 etc), that can consume proofs without needing to be continuously maintained. As we are aiming to break indices with Electra, I'd suggest to prioritize EIP-7495 for Electra as well, and make BeaconState, BeaconBlockBody etc StableContainer so that no further index changes are expected as long as the fundamental architecture stays the same.

This way, we will re-index these containers once (with Electra), but it is the final time that we have to deal with this busywork that just introduces churn at arbitrary times without adding value. We shouldn't have to ever worry about breaking some unrelated component just because a new feature gets added or an old feature gets removed.

On EIP-7495, would appreciate reviews. If we want to transition BeaconState etc to StableContainer, probably makes sense to push the Variant type beyond just a notation but also give it more compact serialization. as in, if we know that an Electra variant is sent, no need for all the leading bits everywhere.

[etan-status]

@hwwhww EIP here to ensure that this is the last time that gindices break: https://ethereum-magicians.org/t/eip-forward-compatible-consensus-data-structures/19673

[jtraglia]

Some nits regarding comments for presets.

[fradamt]

Let's say the current exit epoch is N with churn C. Right now, if you exit balance C, you get still assigned epoch N, and balance to consume is set to 0. If you exit balance 2C, your exit epoch jumps by 2, to N + 2, and the balance to consume is (correctly) set to C. Same if you exit 3C, i.e., exit epoch jumps by 3 to N+3 and balance is set to C.
This behavior is fine, but it seems to me that one would expect it to not have this jump, and instead behave the same whether you fully consume the remaining churn in the current epoch or up to a future epoch, i.e., if you exit 2C, your exit epoch is set to N + 1 with 0 remaining balance to consume.

Suggested change

	state.earliest_exit_epoch += additional_epochs + 1
	if remainder == 0:
	state.earliest_exit_epoch += additional_epochs
	state.exit_balance_to_consume = 0
	elif remainder > 0:
	state.earliest_exit_epoch += additional_epochs + 1
	state.exit_balance_to_consume = per_epoch_churn - remainder

@mkalinin @dapplion I think we discussed this before, let me know if I am forgetting that we made a conscious decision to have this behavior for some reason

[fradamt]

Alternatively we could change the first case to if exit_balance < state.exit_balance_to_consume: instead of <=, and always go to the next exit epoch when the churn is fully consumed.

[mkalinin]

I think this is rather an off-by-one error that should be fixed. If there is enough capacity for a validator to exit in epoch N + k then it should exit in N + k and not N + k + 1, imho (preserving the status quo behaviour). I was playing around with the fix and came up with the following refactor of the computation:

# Read data required for the computation
earliest_exit_epoch = compute_activation_exit_epoch(get_current_epoch(state))
per_epoch_churn = get_activation_exit_churn_limit(state)
if state.earliest_exit_epoch < earliest_exit_epoch:
    exit_balance_to_consume = per_epoch_churn
else:
    earliest_exit_epoch = state.earliest_exit_epoch
    exit_balance_to_consume = state.exit_balance_to_consume

# Borrow more balance from the churn if necessary
if exit_balance > exit_balance_to_consume:
    balance_to_process = exit_balance - exit_balance_to_consume
    additional_epochs = (balance_to_process - 1) // per_epoch_churn + 1  # floor(div(x))
    earliest_exit_epoch += additional_epochs
    exit_balance_to_consume += additional_epochs * per_epoch_churn

# Update state variables
state.exit_balance_to_consume = exit_balance_to_consume - exit_balance
state.earliest_exit_epoch = earliest_exit_epoch

wdyt?

[fradamt]

If state.earliest_exit_epoch > earliest_exit_epoch, we should ensure that the baseline exit epoch is that one, e.g. earliest_exit_epoch = min(state.earliest_exit_epoch, compute_activation_exit_epoch(get_current_epoch(state))

Also state.exit_balance_to_consume here can end up being set to more than one epoch worth of churn.

Btw, the first suggested approach above does fix the off-by-one error in a way that respects the status quo, i.e., if there is enough capacity in epoch N the validator exits in epoch N and not N+1

[mkalinin]

e.g. earliest_exit_epoch = min(state.earliest_exit_epoch, compute_activation_exit_epoch(get_current_epoch(state))

thx, fixed!

Also state.exit_balance_to_consume here can end up being set to more than one epoch worth of churn

In which case? The idea is to push the churn to a minimal epoch required to accommodate exit_balance

[fradamt]

Nevermind, it looks fine!

[fradamt]

Checked the behavior more thoroughly with some test cases and everything looks right

[mkalinin]

Does it make sense to you to open up a PR to update the computation of both churns with the above logic?

[fradamt]

Yeah, sgtm

[ralexstokes]

made an issue to track us fixing this #3677

[fradamt]

These are all duplicates of the phase0 tests (which now just work fine by themselves because run_deposit_processing has been modified to deal with 7251), and can be removed, as in b87f1fc

[hwwhww]

the code merge work lgtm!

I'll enable testgen in another PR and run tests against the mainnet config.