EIP6914 - Reuse indexes with full sweep#3307
Conversation
djrtwo
left a comment
There was a problem hiding this comment.
nice! this is generally what I've had in mind. Nice to see it come out so simple (assuming we don't bound the sweep, but even then, won't be too bad)
hwwhww
left a comment
There was a problem hiding this comment.
great work! 🚀
Just some minor suggestions here. I think this approach works.
| if bls.Verify(pubkey, signing_root, signature): | ||
| state.validators.append(get_validator_from_deposit(pubkey, withdrawal_credentials, amount)) | ||
| state.balances.append(amount) | ||
| index = get_index_for_new_validator(state) |
There was a problem hiding this comment.
Suggesting to define add_validator_to_registry (there could be a better name) and doing the following refactor:
Phase0
def add_validator_to_registry(state: BeaconState, validator: Validator) -> None:
# Add validator and balance entries
state.validators.append(get_validator_from_deposit(pubkey, withdrawal_credentials, amount))
state.balances.append(amount)Replace the two lines with a call to this method
Altair
Modify add_validator_to_registry to be aligned with Altair updates. Which also makes Altair spec leaner as a side effect (no need to redefine entire apply_deposit, just add_validator_to_registry).
Reuse indexes
Modify add_validator_to_registry respectively, and encapsulate get_index_for_new_validator and update or append logic in this feature only.
There was a problem hiding this comment.
The intent of update_or_append_to_list was to prevent having to duplicate initialization statements, which will keep growing with every fork. And to keep this doc agnostic of altair initialization itself. No strong opinion on either way. I'll PR the add_validator_to_registry separately since it's mostly orthogonal to the change
specs/altair/beacon-chain.md
Outdated
| state.balances.append(amount) | ||
| index = get_index_for_new_validator(state) | ||
| validator = get_validator_from_deposit(pubkey, withdrawal_credentials, amount) | ||
| update_or_append_to_list(state.validators, index, validator) |
There was a problem hiding this comment.
Slight preference define the update or append logic explicitly i.e:
if index < len(state.validators):
# Update
else:
# Append|
There's another instance of dead validator records not covered by this PR: incomplete deposits. If there's a valid deposit for a validator for 1ETH, that index will stay taken forever with current design. Should we tackle that case too? This feels more like an attack to bloat space than a by-product of honest usage, so not sure if it's relevant given how un-economic it is. |
I'd say that's very definitely a different discussion to have. You'd have to burn the deposited funds to be able to reuse those indices and that seems unreasonable (at minimum it will be controversial). |
One way of not punching holes in the validator registry (but still keeping the same storage space used) is to not assign indices to validators until they are on the activation queue. This is equivalent to having a cache. There's no real reason to have an index assigned until the validator is ready to be included |
|
Another way of employing the same attack is to deposit 1 ETH to the balance of already withdrawn validator. This is a dead end for this ETH but even in this case overwriting this validator record would be controversial as it is still burning one's ETH. |
Wouldn't this result in a withdrawal, so the attack window would be from the time of the deposit until the withdrawals clock ticked around to reduce the validator's index back to 0? |
I think this attack is impossible at all. The overwriting will happen only to the validators with no balance. So if there is 1 ETH in the balance of some already withdrawn validator, that withdrawn validator will never be overwritten. So no attack window at all. def is_reusable_validator(validator: Validator, balance: Gwei, epoch: Epoch) -> bool:
"""
Check if ``validator`` index can be re-assigned to a new deposit.
"""
return (
epoch > validator.withdrawable_epoch + REUSE_VALIDATOR_INDEX_DELAY
and balance == 0
) |
|
Merging the core here, please use #3335 for follow-up discussions |
Re-use beacon chain indexes
Assign new deposits to existing validator records that have withdrawn long ago
Motivation
Withdrawn indexes are dead weight that adds cost to the protocol:
Safe to reuse epochs constant
SAFE_EPOCHS_TO_REUSE_INDEXuint64(2**16)(= 65,536)From @djrtwo : Tentatively ~3x WS period, so 1 year
Refer to ethereum/EIPs#6914 for security considerations and attacks.
Strategy A: Full-sweep
Strategy B: Bounded sweep
Full sweep cache
Define
reusable_indexes(state)as the set of all validator indexes instatethat satisfy predicateis_reusable_validator.Reusable indexes must satisfy conditions:
validator.withdrawable_epoch < epoch - REUSE_VALIDATOR_INDEX_DELAY:epochis constant through an epoch (duh) andvalidator.withdrawable_epochcan only change if this validator is reused. Re-using is an irreversible action during anepoch. So the setreusable_indexes(state_0)includes all possible setsreusable_indexes(state_child)wherestate_childis a descendant ofstate_0in the same epoch.balance == 0. Balance may not be zero if there's remaining balance from active duties (in extreme configurations). Balance can only be increased with a deposit topup. Balance can only be decreased to zero with a full withdrawal (ifEPOCHS_PER_SLASHINGS_VECTOR < REUSE_VALIDATOR_INDEX_DELAY)Cache strategy
potential_reusable_indexesof all validator indexes withvalidator.withdrawable_epoch < epoch. That set includes all possible reusable indexes on that epoch. Store that list as immutable referenced by all child states inepoch. Instantiate an empty setreused_indexespotential_reusable_indexesand clonereused_indexespotential_reusable_indexesand check index not inreused_indexesand balance == 0. If so, add the index toreused_indexesMIN_VALIDATOR_WITHDRAWABILITY_DELAYthe cached list is guaranteed to be the same for all states at epoch N.Bounded sweep reuse efficiency
Define efficiency as the ratio of re-used indexes with partial sweep by re-used indexes with full sweep (theoretical maximum). We want to maximize efficiency.
We also want to bound computational cost on block processing, such that
avg deposits per block * MAX_VALIDATORS_PER_REUSE_SWEEP << len(state.validators). Note that after EIP6110, deposits per block is bounded only by gas, and could be much higher than current MAX_DEPOSITS = 16.Note that this two goals oppose each other. Also note that missing the opportunity to re-use an index results on a significant long term cost (until new index withdraws) of an extra validator record.
Toy examples
Consider the following patterns of validator records:
------YYmeans 6 non-reusable indexes, 2 reusable indexesMAX_VALIDATORS_PER_REUSE_SWEEP= 2------YY-Y-Y-Y-Y--------YYYYYYYY