Conversation
Co-authored-by: Mikhail Kalinin <[email protected]>
This reverts commit dc2a2bd.
| if source_index == target_index: | ||
| return |
There was a problem hiding this comment.
would it be more straightforward to use if request_source_pubkey == request_target_pubkey here?
There was a problem hiding this comment.
I don't have a strong opinion here. Whatever we choose, we do use source_index and target_index later and it probably makes sense to define them
| if get_consolidation_churn_limit(state) <= MIN_ACTIVATION_BALANCE: | ||
| return |
There was a problem hiding this comment.
test case idea: have a validator get partial withdrawal and then process its consolidation at the same block.
mkalinin
left a comment
mkalinin
left a comment
There was a problem hiding this comment.
LGTM! 👍
We already have consolidation smart contract implementation (ethereum/sys-asm#14). The other missing part is the EL specification, and IMO it would be reasonable to wait for the decision on the alternative proposal to EIP-7685, which is drafted and explained here: ethereum/execution-apis#551. The proposal does also affects CL as it would need to keep all requests in the BeaconBlockBody instead of the ExecutionPayload structure.
…layer_withdrawal_request`
hwwhww
left a comment
hwwhww
left a comment
There was a problem hiding this comment.
I switched the order of process_deposit_receipt and process_execution_layer_withdrawal_request calls.
The PR LGTM!
| # Verify the same withdrawal address | ||
| assert source_validator.withdrawal_credentials[12:] == target_validator.withdrawal_credentials[12:] |
There was a problem hiding this comment.
@hwwhww it is great to see that the consolidation of validators with different withdrawal addresses is possible now.
Was there any reason why it was not allowed initially (why ever this constraint has existed?), but became possible by this PR?
There was a problem hiding this comment.
It was not allowed initially because initially consolidation was authorized by the validator’s pubkey which isn’t secure enough to change the owner (withdrawal creds) of the funds. In order to make this possible, a consolidation request system smart contract has been introduced on EL with the corresponding changes on CL to make it possible to sign requests with withdrawal creds which unlocked moving funds between validators with different creds
4 participants
As decided prior to the interop, we want to move to EL-triggered consolidations, to simplify the way this feature affects staking pools. Like withdrawal requests, consolidation requests go in the execution payload, and processing failures result in a no-op instead of an invalid block.