Make builders non-validating staked actors#4788
Conversation
jtraglia
force-pushed
the
builders-no-updated-withdrawal-types
branch
from
2e93bc6 to
f709f7d
Compare
…4789) I should have done this in the previous refactoring PR. * Rename `get_sweep_withdrawals` to `get_validators_sweep_withdrawals`. * Remove `validator_index` parameter from function. * Update comments & variables to reflect this change. If/when we merge #4788 which adds a builders sweep, this distinction will be necessary.
specs/gloas/beacon-chain.md
Outdated
| pubkey: BLSPubkey | ||
| withdrawal_credentials: Bytes32 | ||
| balance: Gwei | ||
| exit_epoch: Epoch |
There was a problem hiding this comment.
I think builder should also have a withdrawable_epoch. Once the exit_epoch is reached the builder becomes inactive, i.e. it cannot bid, and then until withdrawable_epoch the balance is frozen on the beacon chain. Without that freeze the DoS protection isn’t as strong as it can be
There was a problem hiding this comment.
Right now, builders cannot submit bids after initiating their exit. So there is a freeze. I chatted with @fradamt about this some and he has some concerns with not allowing builders to submit bids during this period. So maybe we'll change it & add a withdrawable_epoch field.
consensus-specs/specs/gloas/beacon-chain.md
Lines 1049 to 1050 in 4284aec
jtraglia
changed the title
| current_epoch = compute_epoch_at_slot(state.slot) | ||
| return not builder.slashed or current_epoch >= builder.withdrawable_epoch | ||
| ``` | ||
| def get_builder_withdrawals( |
There was a problem hiding this comment.
Not sure if it is just me but I've found get_builder_withdrawals a bit confusing as it sounds like general withdrawals although it's actually builder payments. Perhaps we could use something like get_builder_payment_withdrawals.
This comment also applies to state.builder_pending_withdrawals as well. It's a pair of builder_pending_withdrawals and builder_pending_payments, but perhaps it's better to distinguish it via something like payment withdrawals as we now introduce sweep withdrawals for builders.
There was a problem hiding this comment.
Yes, I also find this confusing. Let's rename this in a separate PR though.
) Gate the withdrawal sweep optimization (using min of validator count and MaxValidatorsPerWithdrawalsSweep) behind a hidden feature flag that defaults to false. Enable the flag for spectests to match consensus spec. The backported changes were from [4788](ethereum/consensus-specs#4788) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
) Gate the withdrawal sweep optimization (using min of validator count and MaxValidatorsPerWithdrawalsSweep) behind a hidden feature flag that defaults to false. Enable the flag for spectests to match consensus spec. The backported changes were from [4788](ethereum/consensus-specs#4788) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
The return type from `get_expected_withdrawals` has become quite complex. @potuz suggested we make a named type for this instead ([here](#4788 (comment))). This PR introduces a new `ExpectedWithdrawals` type (dataclass) for this. --------- Co-authored-by: JihoonSong <jihoonsong@users.noreply.github.com>
- Introduce builder entity to beacon state - add builder deposit, withdrawal and withdrawal sweep - bump spec test version to `v1.7.0-alpha.1` - skipping certain fork choice spec test as there are retroactive change to the proposer boost spec. Basically covers gloas beacon-chain spec change from v1.6.1 to v1.7.0-alpha.1 https://github.com/ethereum/consensus-specs/compare/v1.6.0...v1.7.0-alpha.1?path=specs/gloas/beacon-chain.md Spec ref: ethereum/consensus-specs#4788 --------- Co-authored-by: Nico Flaig <nflaig@protonmail.com>
) Gate the withdrawal sweep optimization (using min of validator count and MaxValidatorsPerWithdrawalsSweep) behind a hidden feature flag that defaults to false. Enable the flag for spectests to match consensus spec. The backported changes were from [4788](ethereum/consensus-specs#4788) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
9 participants
This PR makes a new
Buildertype, tracked instate.builders.Some key improvements:
Note: This PR does not fully test everything. We can handle that in separate PRs.