Change BUILDER_INDEX_FLAG to 2^63#5402
Conversation
The `BUILDER_INDEX_FLAG` tries to communicate an unreachable value, i.e. indices above that index represent builders, those below it validators. 2^40 was likely chosen as that is the Fulu validator registry limit. However, EIP-7688 (currently slated for devnet-7) drops that limit by replacing the validators list from a triangle to a smaller structure that grows based on actual demand. That doesn't make 2^40 reachable (in fact, the current practical limit is 2^32 because the deposit contract runs out of slots at that count). But I got asked about 2^40 still being safe, so it is clear that 2^40 doesn't naturally indicate "unreachable" without extra thoughts about why it's specifically that value and not something else. Other argument was 2^52 to be below JavaScript Number.MAX_SAFE_INTEGER. It is not relevant in this context, checked with Lodestar. Therefore, proposing 2^63, it's the highest bit and clearly associated with it being just a flag, not reachable in any circumstance. Clarity.
|
@nflaig I seem to recall Lodestar having issues with values greater than 2**52. Would this change work in Lodestar? |
@etan-status what exactly did you look at? the conversion from validator to builder index (and vice versa) should be fine but we use |
|
I have discussed with @wemeetagain whether 2^63 would be fine for Lodestar. As far as I understood him, it should be. It would be great if he could confirm though, in case I misunderstood him. The motivation here is, that all of 2^40, 2^52, and 2^63 are practically unreachable. But as If Even cleaner would be to just get rid of this random index entirely. Like, just add the deposit_slot to each builder deposit message (in the emitted builder deposit smart contract event), drop the random 0x03 validator credential type that's solely used for the transition, drop conflation of builder indices with validators etc, and drop requiring the deposit queue to be full (and the onboard flow that needs to verify ALL bls in the entire deposit queue) in the days leading to the fork activation. but maybe I'm missing something there and it's more complicated than that 😅 |
|
Why would you be forced to use bigint? The practical limit is uint32, that's the type we use in nimbus. |
|
https://github.com/ethereum/consensus-specs/blob/master/specs/phase0/deposit-contract.md#constants
|
the
yes, it's not that simple, firstly, you don't have the slot on the EL side so you'd need to use a timestamp but the bigger problem is that you would have limited space to onboard builders at the fork and there is risk that sophisticated builders backrun the contract deployment to fill them |
I mean we use just look at this js snippet 2**63 + 5 === 2**63 // true <-- the +5 builder index vanishes
2**63 + 5 === 2**63 + 1 // true <-- builders 1,2,3,4,5 all collapse to the same value
2**40 + 5 === 2**40 // false <-- works fine todayfor reference, this is our implementation function hasBuilderIndexFlag(index: number): boolean {
// Equivalent to `(index & BUILDER_INDEX_FLAG) != 0`
return Math.floor(index / BUILDER_INDEX_FLAG) % 2 === 1;
}
/**
* Check if a validator index represents a builder (has the builder flag set).
* Spec: https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.1/specs/gloas/beacon-chain.md#new-is_builder_index
*/
export function isBuilderIndex(validatorIndex: number): boolean {
// Note: Can't use bitwise AND (&) because BUILDER_INDEX_FLAG exceeds 32 bits in JS bitwise operations.
return hasBuilderIndexFlag(validatorIndex);
}
/**
* Convert a builder index to a flagged validator index for use in Withdrawal containers.
* Spec: https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.1/specs/gloas/beacon-chain.md#new-convert_builder_index_to_validator_index
*/
export function convertBuilderIndexToValidatorIndex(builderIndex: BuilderIndex): ValidatorIndex {
// Note: Can't use bitwise OR (|) because BUILDER_INDEX_FLAG exceeds 32 bits in JS bitwise operations.
return hasBuilderIndexFlag(builderIndex) ? builderIndex : builderIndex + BUILDER_INDEX_FLAG;
}
/**
* Convert a flagged validator index back to a builder index.
* Spec: https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.1/specs/gloas/beacon-chain.md#new-convert_validator_index_to_builder_index
*/
export function convertValidatorIndexToBuilderIndex(validatorIndex: ValidatorIndex): BuilderIndex {
// Note: Can't use bitwise AND (&) because BUILDER_INDEX_FLAG exceeds 32 bits in JS bitwise operations.
return hasBuilderIndexFlag(validatorIndex) ? validatorIndex - BUILDER_INDEX_FLAG : validatorIndex;
}if you change |
|
Yes, this only exists for withdrawals. This solution is a lot easier than changing the 
|
|
nimbus converts validatorindex to the uint32 for internal computation on ingress (i.e., if validator index > uint32 max, reject immediately, e.g., https://github.com/status-im/nimbus-eth2/blob/2361ee12d96e5910ead093ce714242c3215a21d9/beacon_chain/gossip_processing/gossip_validation.nim#L1047). but gotcha, nimbus also uses uint64 for builderindices (and also has to, for serialization). could go with a custom type that serializes with the extra bit set while internally being uint32, but probably not worth it here, it's not a performance critical section. Did we consider having a separate builder_withdrawals next to withdrawals? would be symmetric to how we have separate builder deposits next to regular deposits, and separate validators / builders in the state. are validators / builders separated everywhere else already? if separating the withdrawals into the two kinds would drop the flag, also makes the specific bit discussion moot, as the bit would simply be gone. for getting rid of the 0x03 prefix that conflates validator / builder deposits, and also the 50s lagspike (or messy cache) for processing the entire deposit queue at the fork transition (the onboard function), what would be a way that reuses the most existing mechanisms / gets rid of most of the special snowflakes? e.g., would this work:
|
|
btw, I'm not against 2^52 either, if that fixes the JavaScript stuff. subsequent bits if we ever have them would then just count down from there (i.e., 2^51, 2^50 etc) it's just the 2^40 that's weird, with 7688 dropping VALIDATOR_REGISTRY_LIMIT, 2^40 just isn't grounded anymore. |
|
as in, my preference would be:
|
I am not sure @jtraglia looked at this after EIP-8282, I personally think it's nice and a clean separation might be useful down the road, I just can't estimate the effort right now.
you don't need to process the entire deposit queue, only if there is a builder deposit with overlapping pubkey to a validator deposit
you can't drain the queue before the fork if that's what you mean, the problem is that someone will just fill up the spots, and yes, the fee would go up exponentially making it impossible for anyone to join after first few spots are taken which a sophisticated builder can easily take up. And once we start to dequeue after the fork it means if you got in earlier you will be able to bid earlier too since we dequeue only a limited amount per payload
technically this would work, but requires updates to the storage layout of the contract afaik and we would have this timestamp in there just for the onboarding, afterwards it's tech debt, while how the current onboarding mechanism works, it can be removed after gloas, similar to how we removed the old validator deposit mechanism now for fulu
for JSON it should be fine, we are using strings there |
but after the fork, isn't it allowed for same pubkey to be in both lists? it reintroduces the old griefing attack where the node operator is untrusted. the builder key owner can steal any pre-fork deposits by simply frontrunning it with a 1-ETH validator deposit that shares the same pubkey. then, the onboard_builders_from_pending_deposits logic credits all subsequent 0x03 topup to their validator (with their own frontrun withdrawal address) instead of to the builder. the mitigation from decentralized staking pool doesn't work, where they first wait for finality of the operator registering the validator with the correct credential (1 ETH deposit), before releasing the remaining 31 ETH from the pool. because here, even if you wait for the first 1 ETH builder deposit queue entry to be finalized, you can still get griefed again and again at each subsequent pre-fork deposit.
not sure about "easily take up", there's a real cost involved: they would have to keep paying exponentially to keep the queue long after the fork, and any other builder can still just.. pay the fee once if they absolutely want to be in within the first 10 minutes of the fork (note that the spammer pays a fee for every single queue spot that they want to fill). the draining per slot into the CL could be set quite high, there's no reason to artificially slow it down.
yep, would have to store the timestamp, at least for the entries before the fork. for later entries, the timestamp field could be dropped, but not sure if worth the complexity.
overall, the leftovers of the 0x03 mechanism feel quite clunky to me in their current form. it seems like a highly coordinated effort to be a builder from the getgo.
pre-gloas, any application based on builders works only incidentally, conditional on how many validators opt into mevboost. there can be minutes between blocks from a particular builder with whom one partners. is it truly that prohibitive if there is a period of 12 minutes where we'll just have solo blocks, given that this is already a possibility today? if it is a big priority, the timestamp would be an option. also not very great, though, admittedly... |
yes, we have two separate contracts so there is no way to frontrun
because builder deposits use the same contract as validator, that why we have the
that's how it works after the fork, and yes, it's not possible to do this which is why 8282 is great. All my comments refer to using the builder deposit contract to onboard builders before the fork which means
deposit queue is quite large, so this isn't really easy, @jtraglia can a tool to help out builders to do this, generally builders are much more sophisticated than validators and they only need to send 1 ETH, they can than topup after the fork instantly if they don't wanna risk more ETH during the onboarding flow. |
for validators there is no risk. the staking pool requires node operator to deposit 1 ETH from their own share with correct credential to register their validator, wait for that to finalize, and then release the remaining 31 ETH. the node operator cannot frontrun the topup, only the initial registration. with the builder, this process doesn't work, each individual topup using the old deposit contract can be frontrun. but "they can than topup after the fork instantly if they don't wanna risk more ETH during the onboarding flow." yeah this would indeed be a mitigation.
same as for consolidation and withdrawal requests. the contract would be deployed early on and would just keep accumulating the builder deposits (with higher and higher fee). on the fork transition block, CL obtains the front of the queued entries via execution_requests (up to some large cap per block, tunable at subsequent hard forks). |
|
From Discord conversation, it was shown that the same mitigation that protects against validator_key owner stealing topups still works for builder_key owner. as in, top up 1 ETH first into the builder, that forces the add_builder_to_registry, because the a subsequence 0x01 deposit for the same pubkey would still get added to the builder, as now so, no extra griefing attack possible! can retract that part. |
|
From Discord conversation it seems that there is no desire to bump the number.
Option to drop the flag entirely (i.e., just have a second list in the EL header for builder exits) is separate of this PR. |
3 participants
The
BUILDER_INDEX_FLAGtries to communicate an unreachable value, i.e. indices above that index represent builders, those below it validators.2^40 was likely chosen as that is the Fulu validator registry limit. However, EIP-7688 (currently slated for devnet-7) drops that limit by replacing the validators list from a triangle to a smaller structure that grows based on actual demand.
That doesn't make 2^40 reachable (in fact, the current practical limit is 2^32 because the deposit contract runs out of slots at that count). But I got asked about 2^40 still being safe, so it is clear that 2^40 doesn't naturally indicate "unreachable" without extra thoughts about why it's specifically that value and not something else.
Other argument was 2^52 to be below JavaScript Number.MAX_SAFE_INTEGER. It is not relevant in this context, checked with Lodestar.
Therefore, proposing 2^63, it's the highest bit and clearly associated with it being just a flag, not reachable in any circumstance. Clarity.