Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EIP-7002:Execution layer triggerable withdrawals #14031

Merged
merged 19 commits into from
May 31, 2024
Merged

EIP-7002:Execution layer triggerable withdrawals #14031

merged 19 commits into from
May 31, 2024

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented May 21, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

implements https://eips.ethereum.org/EIPS/eip-7002 with changes described in https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/beacon-chain.md

  • includes spec tests

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@james-prysm james-prysm added the Electra electra hardfork label May 21, 2024
@james-prysm james-prysm marked this pull request as ready for review May 24, 2024 22:12
@james-prysm james-prysm requested a review from a team as a code owner May 24, 2024 22:12
@james-prysm james-prysm changed the title EIP-7002:Execution layer triggerable withdrawals -WIP EIP-7002:Execution layer triggerable withdrawals May 24, 2024
@james-prysm james-prysm requested a review from rkapka May 24, 2024 22:27
@james-prysm james-prysm added the Ready For Review A pull request ready for code review label May 24, 2024
beacon-chain/core/electra/transition.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
if isFullExitRequest {
//# Only exit validator if it has no pending withdrawals in the queue
if pendingBalanceToWithdraw == 0 {
maxExitEpoch, churn := validators.MaxExitEpochAndChurn(st)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be defined outside of the for loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, if there are multiple values the loop updates the state I think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Good point

beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
james-prysm and others added 3 commits May 28, 2024 19:03
Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

vIdx, exists := st.ValidatorIndexByPubkey(bytesutil.ToBytes48(wr.ValidatorPubkey))
if !exists {
log.Debugln("Skipping execution layer withdrawal request, validator index not found")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between Debug and Debugln?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm not sure i used debugln since i know it adds a new line not sure if debug also does

@@ -75,6 +86,107 @@ import (
// withdrawable_epoch=withdrawable_epoch,
// ))
func ProcessExecutionLayerWithdrawRequests(ctx context.Context, st state.BeaconState, wrs []*enginev1.ExecutionLayerWithdrawalRequest) (state.BeaconState, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the spec the function is called process_execution_layer_withdrawal_request (Withdraw --> Withdrawal). The parameter also has Withdrawal in the name,

beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
beacon-chain/core/electra/withdrawals.go Outdated Show resolved Hide resolved
@james-prysm james-prysm requested review from rkapka and saolyn May 29, 2024 20:34
@james-prysm james-prysm added this pull request to the merge queue May 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 30, 2024
@james-prysm james-prysm added this pull request to the merge queue May 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks May 30, 2024
@james-prysm james-prysm added this pull request to the merge queue May 31, 2024
Merged via the queue into develop with commit de04ce8 May 31, 2024
16 of 17 checks passed
@james-prysm james-prysm deleted the eip7002 branch May 31, 2024 17:26
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Electra electra hardfork Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants