consensus-specs icon indicating copy to clipboard operation
consensus-specs copied to clipboard

Simplify validator activation flow

Open mkalinin opened this issue 1 year ago • 8 comments

Deposit queue finalization proposed in https://github.com/ethereum/consensus-specs/pull/3818 allows for the following simplification in the validator activation flow:

  • Do not use activation_eligibility_epoch which is currently used by the protocol to finalize new validators before activating them — this is now done by finalizing the deposit queue
  • Activate new validator once it has sufficient effective balance
  • Set activation_eligibility_epoch for backwards compatibility

In future upgrades activation_eligibility_epoch can be deprecated and re-purposed for any other usage e.g. custom EB ceiling etc

ToDo

  • [ ] Fix tests

mkalinin avatar Oct 18 '24 06:10 mkalinin

Now activation_eligibility_epoch will remain set to far future epoch forever. If that's the case it can break the parsing of the validator statuses for API consumers, see https://hackmd.io/ofFJ5gOmQpu1jjHilHbdQQ. Not a big issue but we should communicate this breaking change.

dapplion avatar Oct 18 '24 12:10 dapplion

See the discussion about this PR on discord here:

  • https://discord.com/channels/595666850260713488/598292067260825641/1296728283756498984

I agree with @mcdee's idea:

I agree that it is redundant with your change, but setting it retains backwards-compatibility so avoids immediate issues. It could then be deprecated in Electra and use slowly removed downstream, eventually freeing up a slot in the validator struct (exciting!)

jtraglia avatar Oct 18 '24 15:10 jtraglia

Why not just completely remove activation_eligibility_epoch instead of leaving it unset? It doesn't seem to beak any backward-compatibility, right?

If it breaks, it shouldn't break more than just unsetting it, right?

ppopth avatar Oct 20 '24 14:10 ppopth

Updated the proposed change with preserving the assignment of activation_eligibility_epoch

mkalinin avatar Oct 21 '24 07:10 mkalinin

LGTM.

mcdee avatar Oct 21 '24 09:10 mcdee

The following problem emerged during a deeper analysis of this change:

  • Eth1 bridge deposits yield a pending deposit with slot=GENESIS_SLOT to distinguish them from deposit requests, the distinguishing is important for Eth1 bridge deprecation
  • Side effect from the above is that Eth1 bridge deposits bypass finalization check in the deposit queue

Considering that Eth1 bridge deposits are lagging behind by ~11 hrs, it is fine for them to bypass the finality check because effectively they should be finalized many hours prior Electra. The activation_eligibility_epoch mechanism is already redundant today, the only case where it still matters is a period of non-finality which is longer than 11 hours.

Considering the above, I see three options:

  1. Leave the proposed change as is and assume that there will be no long period of non-finality on the mainnet around the Electra fork
  2. Use different flag e.g. signature=ETH1_BRIDGE_DEPOSIT_SIGNATURE to distinguish Eth1 bridge deposits and do finalize them before processing in the activation queue
  3. Postpone the change to the next upgrade to avoid unnecessary risk and additional considerations

mkalinin avatar Oct 21 '24 11:10 mkalinin

It sounds like 2) is the better option (I'm assuming that 3) would revert to 2) after the next hard fork anyway).

mcdee avatar Oct 22 '24 22:10 mcdee

It sounds like 2) is the better option (I'm assuming that 3) would revert to 2) after the next hard fork anyway).

There will be no Eth1 bridge deposits soon after Electra activation. So for the next hard fork we can safely take take the approach proposed in this PR

mkalinin avatar Oct 23 '24 05:10 mkalinin

Option (2) has a flaw as anyone can submit a deposit request top-up with that signature and bypass finalization. Although, it shouldn’t be exploited in any adversarial way I would postpone this change to the next upgrade, i.e. option (3), another reason for that is that we want to finalize the spec for Electra asap

mkalinin avatar Oct 31 '24 06:10 mkalinin

@mkalinin is this PR still relevant or can it be closed?

ralexstokes avatar Mar 23 '25 20:03 ralexstokes

@mkalinin is this PR still relevant or can it be closed?

it is, i think we might have it for Fulu, but now thinking maybe it is better to postpone it to the next one. The idea is to free up some space in the validator record that we could re-use in the future. Maybe worth making this an EIP?

mkalinin avatar Mar 24 '25 08:03 mkalinin

Hey @mkalinin I'm going to close this PR. Now that Electra is live & the scope for Fulu is mostly finalized, I believe this would need to be an EIP to be included in a future upgrade. Feel free to re-open if you disagree.

jtraglia avatar Jun 04 '25 17:06 jtraglia