From 27f6a7a2fa78bed1ac21ce7e2910890ed541955d Mon Sep 17 00:00:00 2001 From: james-prysm Date: Thu, 24 Oct 2024 13:59:01 -0500 Subject: [PATCH 1/8] addressing bug with withdrawals for devnet 5 --- beacon-chain/core/blocks/withdrawals.go | 49 ++++---- .../state/state-native/getters_withdrawal.go | 112 ++++++++++-------- 2 files changed, 87 insertions(+), 74 deletions(-) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index e2f202ce9081..2e86c47afebf 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -120,35 +120,36 @@ func ValidateBLSToExecutionChange(st state.ReadOnlyBeaconState, signed *ethpb.Si // // Spec pseudocode definition: // -// def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: -// expected_withdrawals, partial_withdrawals_count = get_expected_withdrawals(state) # [Modified in Electra:EIP7251] +// def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: +// # [Modified in Electra:EIP7251] +// expected_withdrawals, processed_partial_withdrawals_count = get_expected_withdrawals(state) // -// assert len(payload.withdrawals) == len(expected_withdrawals) +// assert len(payload.withdrawals) == len(expected_withdrawals) // -// for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals): -// assert withdrawal == expected_withdrawal -// decrease_balance(state, withdrawal.validator_index, withdrawal.amount) +// for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals): +// assert withdrawal == expected_withdrawal +// decrease_balance(state, withdrawal.validator_index, withdrawal.amount) // -// # Update pending partial withdrawals [New in Electra:EIP7251] -// state.pending_partial_withdrawals = state.pending_partial_withdrawals[partial_withdrawals_count:] +// # Update pending partial withdrawals [New in Electra:EIP7251] +// state.pending_partial_withdrawals = state.pending_partial_withdrawals[processed_partial_withdrawals_count:] // -// # Update the next withdrawal index if this block contained withdrawals -// if len(expected_withdrawals) != 0: -// latest_withdrawal = expected_withdrawals[-1] -// state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) +// # Update the next withdrawal index if this block contained withdrawals +// if len(expected_withdrawals) != 0: +// latest_withdrawal = expected_withdrawals[-1] +// state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) // -// # Update the next validator index to start the next withdrawal sweep -// if len(expected_withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: -// # Next sweep starts after the latest withdrawal's validator index -// next_validator_index = ValidatorIndex((expected_withdrawals[-1].validator_index + 1) % len(state.validators)) -// state.next_withdrawal_validator_index = next_validator_index -// else: -// # Advance sweep by the max length of the sweep if there was not a full set of withdrawals -// next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP -// next_validator_index = ValidatorIndex(next_index % len(state.validators)) -// state.next_withdrawal_validator_index = next_validator_index +// # Update the next validator index to start the next withdrawal sweep +// if len(expected_withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: +// # Next sweep starts after the latest withdrawal's validator index +// next_validator_index = ValidatorIndex((expected_withdrawals[-1].validator_index + 1) % len(state.validators)) +// state.next_withdrawal_validator_index = next_validator_index +// else: +// # Advance sweep by the max length of the sweep if there was not a full set of withdrawals +// next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP +// next_validator_index = ValidatorIndex(next_index % len(state.validators)) +// state.next_withdrawal_validator_index = next_validator_index func ProcessWithdrawals(st state.BeaconState, executionData interfaces.ExecutionData) (state.BeaconState, error) { - expectedWithdrawals, partialWithdrawalsCount, err := st.ExpectedWithdrawals() + expectedWithdrawals, processedPartialWithdrawalsCount, err := st.ExpectedWithdrawals() if err != nil { return nil, errors.Wrap(err, "could not get expected withdrawals") } @@ -192,7 +193,7 @@ func ProcessWithdrawals(st state.BeaconState, executionData interfaces.Execution } if st.Version() >= version.Electra { - if err := st.DequeuePartialWithdrawals(partialWithdrawalsCount); err != nil { + if err := st.DequeuePartialWithdrawals(processedPartialWithdrawalsCount); err != nil { return nil, fmt.Errorf("unable to dequeue partial withdrawals from state: %w", err) } } diff --git a/beacon-chain/state/state-native/getters_withdrawal.go b/beacon-chain/state/state-native/getters_withdrawal.go index ed78964044e8..e13be439142e 100644 --- a/beacon-chain/state/state-native/getters_withdrawal.go +++ b/beacon-chain/state/state-native/getters_withdrawal.go @@ -48,57 +48,60 @@ func (b *BeaconState) NextWithdrawalValidatorIndex() (primitives.ValidatorIndex, // // Spec definition: // -// def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal], uint64]: -// epoch = get_current_epoch(state) -// withdrawal_index = state.next_withdrawal_index -// validator_index = state.next_withdrawal_validator_index -// withdrawals: List[Withdrawal] = [] +// def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal], uint64]: +// epoch = get_current_epoch(state) +// withdrawal_index = state.next_withdrawal_index +// validator_index = state.next_withdrawal_validator_index +// withdrawals: List[Withdrawal] = [] +// processed_partial_withdrawals_count = 0 // -// # [New in Electra:EIP7251] Consume pending partial withdrawals -// for withdrawal in state.pending_partial_withdrawals: -// if withdrawal.withdrawable_epoch > epoch or len(withdrawals) == MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: -// break +// # [New in Electra:EIP7251] Consume pending partial withdrawals +// for withdrawal in state.pending_partial_withdrawals: +// if withdrawal.withdrawable_epoch > epoch or len(withdrawals) == MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: +// break // -// validator = state.validators[withdrawal.index] -// has_sufficient_effective_balance = validator.effective_balance >= MIN_ACTIVATION_BALANCE -// has_excess_balance = state.balances[withdrawal.index] > MIN_ACTIVATION_BALANCE -// if validator.exit_epoch == FAR_FUTURE_EPOCH and has_sufficient_effective_balance and has_excess_balance: -// withdrawable_balance = min(state.balances[withdrawal.index] - MIN_ACTIVATION_BALANCE, withdrawal.amount) -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=withdrawal.index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=withdrawable_balance, -// )) -// withdrawal_index += WithdrawalIndex(1) +// validator = state.validators[withdrawal.index] +// has_sufficient_effective_balance = validator.effective_balance >= MIN_ACTIVATION_BALANCE +// has_excess_balance = state.balances[withdrawal.index] > MIN_ACTIVATION_BALANCE +// if validator.exit_epoch == FAR_FUTURE_EPOCH and has_sufficient_effective_balance and has_excess_balance: +// withdrawable_balance = min(state.balances[withdrawal.index] - MIN_ACTIVATION_BALANCE, withdrawal.amount) +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=withdrawal.index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=withdrawable_balance, +// )) +// withdrawal_index += WithdrawalIndex(1) // -// partial_withdrawals_count = len(withdrawals) +// processed_partial_withdrawals_count += 1 // -// # Sweep for remaining. -// bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP) -// for _ in range(bound): -// validator = state.validators[validator_index] -// balance = state.balances[validator_index] -// if is_fully_withdrawable_validator(validator, balance, epoch): -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=validator_index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=balance, -// )) -// withdrawal_index += WithdrawalIndex(1) -// elif is_partially_withdrawable_validator(validator, balance): -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=validator_index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=balance - get_validator_max_effective_balance(validator), # [Modified in Electra:EIP7251] -// )) -// withdrawal_index += WithdrawalIndex(1) -// if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: -// break -// validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) -// return withdrawals, partial_withdrawals_count +// # Sweep for remaining. +// bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP) +// for _ in range(bound): +// validator = state.validators[validator_index] +// # [Modified in Electra:EIP7251] +// partially_withdrawn_balance = sum(withdrawal.amount for withdrawal in withdrawals if withdrawal.validator_index == validator_index) +// balance = state.balances[validator_index] - partially_withdrawn_balance +// if is_fully_withdrawable_validator(validator, balance, epoch): +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=validator_index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=balance, +// )) +// withdrawal_index += WithdrawalIndex(1) +// elif is_partially_withdrawable_validator(validator, balance): +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=validator_index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=balance - get_validator_max_effective_balance(validator), # [Modified in Electra:EIP7251] +// )) +// withdrawal_index += WithdrawalIndex(1) +// if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: +// break +// validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) +// return withdrawals, processed_partial_withdrawals_count func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, error) { if b.version < version.Capella { return nil, 0, errNotSupported("ExpectedWithdrawals", b.version) @@ -113,7 +116,7 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err epoch := slots.ToEpoch(b.slot) // Electra partial withdrawals functionality. - var partialWithdrawalsCount uint64 + var processedPartialWithdrawalsCount uint64 if b.version >= version.Electra { for _, w := range b.pendingPartialWithdrawals { if w.WithdrawableEpoch > epoch || len(withdrawals) >= int(params.BeaconConfig().MaxPendingPartialsPerWithdrawalsSweep) { @@ -140,7 +143,7 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err }) withdrawalIndex++ } - partialWithdrawalsCount++ + processedPartialWithdrawalsCount++ } } @@ -155,6 +158,15 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err if err != nil { return nil, 0, errors.Wrapf(err, "could not retrieve balance at index %d", validatorIndex) } + if b.version >= version.Electra { + var partiallyWithdrawnBalance uint64 + for _, w := range withdrawals { + if w.Index == uint64(validatorIndex) { + partiallyWithdrawnBalance += w.Amount + } + } + balance = balance - partiallyWithdrawnBalance + } if helpers.IsFullyWithdrawableValidator(val, balance, epoch, b.version) { withdrawals = append(withdrawals, &enginev1.Withdrawal{ Index: withdrawalIndex, @@ -181,7 +193,7 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err } } - return withdrawals, partialWithdrawalsCount, nil + return withdrawals, processedPartialWithdrawalsCount, nil } func (b *BeaconState) PendingPartialWithdrawals() ([]*ethpb.PendingPartialWithdrawal, error) { From 0bd792d07d3b6a9d7467181f108b8b3dd8bb9b75 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Thu, 24 Oct 2024 14:02:37 -0500 Subject: [PATCH 2/8] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48e15ab28b0a..1647c04bdfa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Fixed mesh size by appending `gParams.Dhi = gossipSubDhi` - Fix skipping partial withdrawals count. +- EIP7521 - fixing withdrawal bug ### Security From 7c3647b037f6c44e3242071267370f4532445deb Mon Sep 17 00:00:00 2001 From: james-prysm Date: Thu, 24 Oct 2024 14:22:42 -0500 Subject: [PATCH 3/8] fixing if statement check --- beacon-chain/state/state-native/getters_withdrawal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/state/state-native/getters_withdrawal.go b/beacon-chain/state/state-native/getters_withdrawal.go index e13be439142e..80e02eaab171 100644 --- a/beacon-chain/state/state-native/getters_withdrawal.go +++ b/beacon-chain/state/state-native/getters_withdrawal.go @@ -161,7 +161,7 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err if b.version >= version.Electra { var partiallyWithdrawnBalance uint64 for _, w := range withdrawals { - if w.Index == uint64(validatorIndex) { + if w.ValidatorIndex == validatorIndex { partiallyWithdrawnBalance += w.Amount } } From e9070686845c708df7bc2671f291b8dc4c86b55b Mon Sep 17 00:00:00 2001 From: james-prysm Date: Thu, 24 Oct 2024 15:22:18 -0500 Subject: [PATCH 4/8] adding test --- .../state-native/getters_withdrawal_test.go | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/beacon-chain/state/state-native/getters_withdrawal_test.go b/beacon-chain/state/state-native/getters_withdrawal_test.go index f6a6158f318c..bc3895006f24 100644 --- a/beacon-chain/state/state-native/getters_withdrawal_test.go +++ b/beacon-chain/state/state-native/getters_withdrawal_test.go @@ -367,4 +367,52 @@ func TestExpectedWithdrawals(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(10), partialWithdrawalsCount) }) + t.Run("electra same validator has one partially and one fully withdrawable", func(t *testing.T) { + s, _ := util.DeterministicGenesisStateElectra(t, 1) + vals := make([]*ethpb.Validator, 100) + balances := make([]uint64, 100) + for i := range vals { + balances[i] = params.BeaconConfig().MaxEffectiveBalance + val := ðpb.Validator{ + WithdrawalCredentials: make([]byte, 32), + EffectiveBalance: params.BeaconConfig().MaxEffectiveBalance, + WithdrawableEpoch: primitives.Epoch(1), + ExitEpoch: params.BeaconConfig().FarFutureEpoch, + } + val.WithdrawalCredentials[0] = params.BeaconConfig().ETH1AddressWithdrawalPrefixByte + val.WithdrawalCredentials[31] = byte(i) + vals[i] = val + } + balances[1] += params.BeaconConfig().MinDepositAmount + vals[1].WithdrawableEpoch = primitives.Epoch(0) + require.NoError(t, s.SetValidators(vals)) + require.NoError(t, s.SetBalances(balances)) + // Give validator a pending balance to withdraw. + require.NoError(t, s.AppendPendingPartialWithdrawal(ðpb.PendingPartialWithdrawal{ + Index: 1, + Amount: balances[1], // will only deduct excess even though balance is more that minimum activation + WithdrawableEpoch: primitives.Epoch(0), + })) + p, err := s.PendingPartialWithdrawals() + require.NoError(t, err) + require.Equal(t, 1, len(p)) + expected, _, err := s.ExpectedWithdrawals() + require.NoError(t, err) + require.Equal(t, 2, len(expected)) + + withdrawalFull := &enginev1.Withdrawal{ + Index: 1, + ValidatorIndex: 1, + Address: vals[1].WithdrawalCredentials[12:], + Amount: balances[1] - params.BeaconConfig().MinDepositAmount, // subtract the partial from this + } + withdrawalPartial := &enginev1.Withdrawal{ + Index: 0, + ValidatorIndex: 1, + Address: vals[1].WithdrawalCredentials[12:], + Amount: params.BeaconConfig().MinDepositAmount, + } + require.DeepEqual(t, withdrawalPartial, expected[0]) + require.DeepEqual(t, withdrawalFull, expected[1]) + }) } From e297cf2379a444d1217850c831ab19b6636be1d2 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 09:42:17 -0500 Subject: [PATCH 5/8] terence's review comments --- CHANGELOG.md | 2 +- beacon-chain/core/blocks/withdrawals.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e67fd59f7253..06ad62221175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,7 +43,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Fixed mesh size by appending `gParams.Dhi = gossipSubDhi` - Fix skipping partial withdrawals count. - wait for the async StreamEvent writer to exit before leaving the http handler, avoiding race condition panics [pr](https://github.com/prysmaticlabs/prysm/pull/14557) -- EIP7521 - fixing withdrawal bug +- EIP7521 - fixing withdrawal bug, pending partial withdrawals was not taken account of in the withdrawal sweep. The already withdrawn balance should be deducted from the balance considered in the sweep. ### Security diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index 2e86c47afebf..0af6ed0c77d7 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -121,8 +121,7 @@ func ValidateBLSToExecutionChange(st state.ReadOnlyBeaconState, signed *ethpb.Si // Spec pseudocode definition: // // def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: -// # [Modified in Electra:EIP7251] -// expected_withdrawals, processed_partial_withdrawals_count = get_expected_withdrawals(state) +// expected_withdrawals, processed_partial_withdrawals_count = get_expected_withdrawals(state) # [Modified in Electra:EIP7251] // // assert len(payload.withdrawals) == len(expected_withdrawals) // From 1148036690c4b5b31cf480194b96db85bd4b0b05 Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 09:48:22 -0500 Subject: [PATCH 6/8] attempting to fix weird comment formatting --- beacon-chain/core/blocks/withdrawals.go | 45 +++++++++++++------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/beacon-chain/core/blocks/withdrawals.go b/beacon-chain/core/blocks/withdrawals.go index 0af6ed0c77d7..f52378243b3d 100644 --- a/beacon-chain/core/blocks/withdrawals.go +++ b/beacon-chain/core/blocks/withdrawals.go @@ -120,33 +120,34 @@ func ValidateBLSToExecutionChange(st state.ReadOnlyBeaconState, signed *ethpb.Si // // Spec pseudocode definition: // -// def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: -// expected_withdrawals, processed_partial_withdrawals_count = get_expected_withdrawals(state) # [Modified in Electra:EIP7251] +// def process_withdrawals(state: BeaconState, payload: ExecutionPayload) -> None: // -// assert len(payload.withdrawals) == len(expected_withdrawals) +// expected_withdrawals, processed_partial_withdrawals_count = get_expected_withdrawals(state) # [Modified in Electra:EIP7251] // -// for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals): -// assert withdrawal == expected_withdrawal -// decrease_balance(state, withdrawal.validator_index, withdrawal.amount) +// assert len(payload.withdrawals) == len(expected_withdrawals) // -// # Update pending partial withdrawals [New in Electra:EIP7251] -// state.pending_partial_withdrawals = state.pending_partial_withdrawals[processed_partial_withdrawals_count:] +// for expected_withdrawal, withdrawal in zip(expected_withdrawals, payload.withdrawals): +// assert withdrawal == expected_withdrawal +// decrease_balance(state, withdrawal.validator_index, withdrawal.amount) // -// # Update the next withdrawal index if this block contained withdrawals -// if len(expected_withdrawals) != 0: -// latest_withdrawal = expected_withdrawals[-1] -// state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) +// # Update pending partial withdrawals [New in Electra:EIP7251] +// state.pending_partial_withdrawals = state.pending_partial_withdrawals[processed_partial_withdrawals_count:] // -// # Update the next validator index to start the next withdrawal sweep -// if len(expected_withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: -// # Next sweep starts after the latest withdrawal's validator index -// next_validator_index = ValidatorIndex((expected_withdrawals[-1].validator_index + 1) % len(state.validators)) -// state.next_withdrawal_validator_index = next_validator_index -// else: -// # Advance sweep by the max length of the sweep if there was not a full set of withdrawals -// next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP -// next_validator_index = ValidatorIndex(next_index % len(state.validators)) -// state.next_withdrawal_validator_index = next_validator_index +// # Update the next withdrawal index if this block contained withdrawals +// if len(expected_withdrawals) != 0: +// latest_withdrawal = expected_withdrawals[-1] +// state.next_withdrawal_index = WithdrawalIndex(latest_withdrawal.index + 1) +// +// # Update the next validator index to start the next withdrawal sweep +// if len(expected_withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: +// # Next sweep starts after the latest withdrawal's validator index +// next_validator_index = ValidatorIndex((expected_withdrawals[-1].validator_index + 1) % len(state.validators)) +// state.next_withdrawal_validator_index = next_validator_index +// else: +// # Advance sweep by the max length of the sweep if there was not a full set of withdrawals +// next_index = state.next_withdrawal_validator_index + MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP +// next_validator_index = ValidatorIndex(next_index % len(state.validators)) +// state.next_withdrawal_validator_index = next_validator_index func ProcessWithdrawals(st state.BeaconState, executionData interfaces.ExecutionData) (state.BeaconState, error) { expectedWithdrawals, processedPartialWithdrawalsCount, err := st.ExpectedWithdrawals() if err != nil { From d6d93c9b6272d2c332e57fbfa58764293bd8c45f Mon Sep 17 00:00:00 2001 From: james-prysm Date: Fri, 25 Oct 2024 09:51:00 -0500 Subject: [PATCH 7/8] moving back more comments --- .../state/state-native/getters_withdrawal.go | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/beacon-chain/state/state-native/getters_withdrawal.go b/beacon-chain/state/state-native/getters_withdrawal.go index 80e02eaab171..95e15deb4701 100644 --- a/beacon-chain/state/state-native/getters_withdrawal.go +++ b/beacon-chain/state/state-native/getters_withdrawal.go @@ -48,60 +48,60 @@ func (b *BeaconState) NextWithdrawalValidatorIndex() (primitives.ValidatorIndex, // // Spec definition: // -// def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal], uint64]: -// epoch = get_current_epoch(state) -// withdrawal_index = state.next_withdrawal_index -// validator_index = state.next_withdrawal_validator_index -// withdrawals: List[Withdrawal] = [] -// processed_partial_withdrawals_count = 0 +// def get_expected_withdrawals(state: BeaconState) -> Tuple[Sequence[Withdrawal], uint64]: +// epoch = get_current_epoch(state) +// withdrawal_index = state.next_withdrawal_index +// validator_index = state.next_withdrawal_validator_index +// withdrawals: List[Withdrawal] = [] +// processed_partial_withdrawals_count = 0 // -// # [New in Electra:EIP7251] Consume pending partial withdrawals -// for withdrawal in state.pending_partial_withdrawals: -// if withdrawal.withdrawable_epoch > epoch or len(withdrawals) == MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: -// break +// # [New in Electra:EIP7251] Consume pending partial withdrawals +// for withdrawal in state.pending_partial_withdrawals: +// if withdrawal.withdrawable_epoch > epoch or len(withdrawals) == MAX_PENDING_PARTIALS_PER_WITHDRAWALS_SWEEP: +// break // -// validator = state.validators[withdrawal.index] -// has_sufficient_effective_balance = validator.effective_balance >= MIN_ACTIVATION_BALANCE -// has_excess_balance = state.balances[withdrawal.index] > MIN_ACTIVATION_BALANCE -// if validator.exit_epoch == FAR_FUTURE_EPOCH and has_sufficient_effective_balance and has_excess_balance: -// withdrawable_balance = min(state.balances[withdrawal.index] - MIN_ACTIVATION_BALANCE, withdrawal.amount) -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=withdrawal.index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=withdrawable_balance, -// )) -// withdrawal_index += WithdrawalIndex(1) +// validator = state.validators[withdrawal.index] +// has_sufficient_effective_balance = validator.effective_balance >= MIN_ACTIVATION_BALANCE +// has_excess_balance = state.balances[withdrawal.index] > MIN_ACTIVATION_BALANCE +// if validator.exit_epoch == FAR_FUTURE_EPOCH and has_sufficient_effective_balance and has_excess_balance: +// withdrawable_balance = min(state.balances[withdrawal.index] - MIN_ACTIVATION_BALANCE, withdrawal.amount) +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=withdrawal.index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=withdrawable_balance, +// )) +// withdrawal_index += WithdrawalIndex(1) // -// processed_partial_withdrawals_count += 1 +// processed_partial_withdrawals_count += 1 // -// # Sweep for remaining. -// bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP) -// for _ in range(bound): -// validator = state.validators[validator_index] -// # [Modified in Electra:EIP7251] -// partially_withdrawn_balance = sum(withdrawal.amount for withdrawal in withdrawals if withdrawal.validator_index == validator_index) -// balance = state.balances[validator_index] - partially_withdrawn_balance -// if is_fully_withdrawable_validator(validator, balance, epoch): -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=validator_index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=balance, -// )) -// withdrawal_index += WithdrawalIndex(1) -// elif is_partially_withdrawable_validator(validator, balance): -// withdrawals.append(Withdrawal( -// index=withdrawal_index, -// validator_index=validator_index, -// address=ExecutionAddress(validator.withdrawal_credentials[12:]), -// amount=balance - get_validator_max_effective_balance(validator), # [Modified in Electra:EIP7251] -// )) -// withdrawal_index += WithdrawalIndex(1) -// if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: -// break -// validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) -// return withdrawals, processed_partial_withdrawals_count +// # Sweep for remaining. +// bound = min(len(state.validators), MAX_VALIDATORS_PER_WITHDRAWALS_SWEEP) +// for _ in range(bound): +// validator = state.validators[validator_index] +// # [Modified in Electra:EIP7251] +// partially_withdrawn_balance = sum(withdrawal.amount for withdrawal in withdrawals if withdrawal.validator_index == validator_index) +// balance = state.balances[validator_index] - partially_withdrawn_balance +// if is_fully_withdrawable_validator(validator, balance, epoch): +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=validator_index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=balance, +// )) +// withdrawal_index += WithdrawalIndex(1) +// elif is_partially_withdrawable_validator(validator, balance): +// withdrawals.append(Withdrawal( +// index=withdrawal_index, +// validator_index=validator_index, +// address=ExecutionAddress(validator.withdrawal_credentials[12:]), +// amount=balance - get_validator_max_effective_balance(validator), # [Modified in Electra:EIP7251] +// )) +// withdrawal_index += WithdrawalIndex(1) +// if len(withdrawals) == MAX_WITHDRAWALS_PER_PAYLOAD: +// break +// validator_index = ValidatorIndex((validator_index + 1) % len(state.validators)) +// return withdrawals, processed_partial_withdrawals_count func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, error) { if b.version < version.Capella { return nil, 0, errNotSupported("ExpectedWithdrawals", b.version) From 7da95b8fcf673db1e8d4bc396652ba58d010bf08 Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Thu, 31 Oct 2024 08:22:18 -0500 Subject: [PATCH 8/8] Update CHANGELOG.md Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a62fa845b5c..3be79b1024cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Certain deb files were returning a 404 which made building new docker images without an existing cache impossible. This has been fixed with updates to rules_oci and bazel-lib. - Fixed an issue where the length check between block body KZG commitments and the existing cache from the database was incompatible. -- EIP7521 - fixing withdrawal bug, pending partial withdrawals was not taken account of in the withdrawal sweep. The already withdrawn balance should be deducted from the balance considered in the sweep. +- EIP7521 - Fixes withdrawal bug by accounting for pending partial withdrawals and deducting already withdrawn amounts from the sweep balance. ### Security