Skip to content

Conversation

@mkalinin
Copy link
Contributor

@mkalinin mkalinin commented Oct 18, 2024

Deposit queue finalization proposed in #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

@dapplion
Copy link
Member

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.

@jtraglia
Copy link
Member

See the discussion about this PR on discord here:

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!)

@ppopth
Copy link
Member

ppopth commented Oct 20, 2024

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?

Copy link
Member

@ppopth ppopth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea looks good for me

@mkalinin
Copy link
Contributor Author

Updated the proposed change with preserving the assignment of activation_eligibility_epoch

@mcdee
Copy link
Contributor

mcdee commented Oct 21, 2024

LGTM.

@mkalinin
Copy link
Contributor Author

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

@mcdee
Copy link
Contributor

mcdee commented Oct 22, 2024

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

@mkalinin
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@ralexstokes
Copy link
Member

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

@mkalinin
Copy link
Contributor Author

@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?

@jtraglia
Copy link
Member

jtraglia commented Jun 4, 2025

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 jtraglia closed this Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants