Optimize strategy for onboarding builders at the fork

[twoeths]

Description

• avoid eager signature validation for the whole pending_deposits queue when onboard builders at fork transition
• this keeps front-running resistance semantic of the current spec while helping the fork transition runs faster

this is under discussion, making this as Draft in the mean time

[jtraglia]

Hey @twoeths thanks for the PR! I actually really like this change. There could be a lot of new validator deposits at the fork and being forced to verify all of these then is impractical. Your change makes that a little better; it will only do a signature check when there are validator/builder deposits with the same pubkey (which is unlikely). The only concern I have with this approach is that we will iterate over the list of pending deposits a lot, but I expect this is still much faster than the signature checks.

[twoeths]

The only concern I have with this approach is that we will iterate over the list of pending deposits a lot, but I expect this is still much faster than the signature checks.

implementations should cache pending deposits by pubkey, lodestar implementation: ChainSafe/lodestar#9374
this is just the same to is_pending_validator(), implementation should have different caching strategies there I believe

[jihoonsong]

Here is another version of onboard_builders_from_pending_deposits. I've refactored it so the flow reads as: filter out deposits for validator and then apply the rest as builder deposits. I believe it's equivalent and all tests pass, but I'd appreciate a cross-check.

def onboard_builders_from_pending_deposits(state: BeaconState) -> None:
    """
    Applies any pending deposit for builders, effectively
    onboarding builders at the fork.
    """
    validator_pubkeys = [v.pubkey for v in state.validators]

    pending_deposits = []
    for deposit in state.pending_deposits:
        # Deposits for existing validators stay in pending queue
        if deposit.pubkey in validator_pubkeys:
            pending_deposits.append(deposit)
            continue

        # Note that the function apply_deposit_for_builder can mutate the
        # state and may add a builder to the registry. For this reason, the
        # list of builder pubkeys must be recomputed each iteration.
        builder_pubkeys = [b.pubkey for b in state.builders]
        if deposit.pubkey not in builder_pubkeys:
            # Deposits for new validators stay in pending queue
            if not is_builder_withdrawal_credential(deposit.withdrawal_credentials):
                pending_deposits.append(deposit)
                continue

            # If there is a valid pending deposit for a new validator with this pubkey,
            # keep this deposit in the pending queue to be applied to that validator later
            if is_pending_validator(pending_deposits, deposit.pubkey):
                pending_deposits.append(deposit)
                continue

        # Apply a deposit for a builder
        apply_deposit_for_builder(
            state,
            deposit.pubkey,
            deposit.withdrawal_credentials,
            deposit.amount,
            deposit.signature,
            deposit.slot,
        )

    state.pending_deposits = pending_deposits

[jtraglia]

There's some support for this from client devs on discord:

Potuz

Terence

Stefan

[jtraglia]

I'm going to make an executive decision to include this PR in the upcoming release.