fix expected withdrawals (#15191)

* fixed underflow with expected withdrawals

* update comment

* Revert "update comment"

This reverts commit e07da541ac.

* attempting to fix comment indents

* fixing another missed tab in comments

* trying tabs one more time for fixing tabs

* adding undeflow safety

* fixing error typo

* missed wrapping the error

beacon-chain/state/state-native/getters_withdrawal.go

@@ -62,9 +62,11 @@ func (b *BeaconState) NextWithdrawalValidatorIndex() (primitives.ValidatorIndex,
 //
 //			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
+//			total_withdrawn = sum(w.amount for w in withdrawals if w.validator_index == withdrawal.validator_index)
+//			balance = state.balances[withdrawal.validator_index] - total_withdrawn
+//			has_excess_balance = balance > 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)
+//				withdrawable_balance = min(balance - MIN_ACTIVATION_BALANCE, withdrawal.amount)
 //				withdrawals.append(Withdrawal(
 //					index=withdrawal_index,
 //					validator_index=withdrawal.index,
@@ -132,9 +134,19 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err
 				return nil, 0, fmt.Errorf("could not retrieve balance at index %d: %w", w.Index, err)
 			}
 			hasSufficientEffectiveBalance := v.EffectiveBalance() >= params.BeaconConfig().MinActivationBalance
-			hasExcessBalance := vBal > params.BeaconConfig().MinActivationBalance
+			var totalWithdrawn uint64
+			for _, wi := range withdrawals {
+				if wi.ValidatorIndex == w.Index {
+					totalWithdrawn += wi.Amount
+				}
+			}
+			balance, err := mathutil.Sub64(vBal, totalWithdrawn)
+			if err != nil {
+				return nil, 0, errors.Wrapf(err, "failed to subtract balance %d with total withdrawn %d", vBal, totalWithdrawn)
+			}
+			hasExcessBalance := balance > params.BeaconConfig().MinActivationBalance
 			if v.ExitEpoch() == params.BeaconConfig().FarFutureEpoch && hasSufficientEffectiveBalance && hasExcessBalance {
-				amount := min(vBal-params.BeaconConfig().MinActivationBalance, w.Amount)
+				amount := min(balance-params.BeaconConfig().MinActivationBalance, w.Amount)
 				withdrawals = append(withdrawals, &enginev1.Withdrawal{
 					Index:          withdrawalIndex,
 					ValidatorIndex: w.Index,
@@ -165,7 +177,10 @@ func (b *BeaconState) ExpectedWithdrawals() ([]*enginev1.Withdrawal, uint64, err
 					partiallyWithdrawnBalance += w.Amount
 				}
 			}
-			balance = balance - partiallyWithdrawnBalance
+			balance, err = mathutil.Sub64(balance, partiallyWithdrawnBalance)
+			if err != nil {
+				return nil, 0, errors.Wrapf(err, "could not subtract balance %d with partial withdrawn balance %d", balance, partiallyWithdrawnBalance)
+			}
 		}
 		if helpers.IsFullyWithdrawableValidator(val, balance, epoch, b.version) {
 			withdrawals = append(withdrawals, &enginev1.Withdrawal{

beacon-chain/state/state-native/getters_withdrawal_test.go

@@ -416,3 +416,37 @@ func TestExpectedWithdrawals(t *testing.T) {
 		require.DeepEqual(t, withdrawalFull, expected[1])
 	})
 }
+
+func TestExpectedWithdrawals_underflow_electra(t *testing.T) {
+	s, err := state_native.InitializeFromProtoUnsafeElectra(&ethpb.BeaconStateElectra{})
+	require.NoError(t, err)
+	vals := make([]*ethpb.Validator, 1)
+	balances := make([]uint64, 1)
+	balances[0] = 2015_000_000_000 //Validator A begins leaking ETH due to inactivity, and over time, its balance decreases to 2,015 ETH
+	val := &ethpb.Validator{
+		WithdrawalCredentials: make([]byte, 32),
+		EffectiveBalance:      params.BeaconConfig().MaxEffectiveBalanceElectra,
+		WithdrawableEpoch:     primitives.Epoch(0),
+		ExitEpoch:             params.BeaconConfig().FarFutureEpoch,
+	}
+	val.WithdrawalCredentials[0] = params.BeaconConfig().CompoundingWithdrawalPrefixByte
+	val.WithdrawalCredentials[31] = byte(0)
+	vals[0] = val
+
+	require.NoError(t, s.SetValidators(vals))
+	require.NoError(t, s.SetBalances(balances))
+	require.NoError(t, s.AppendPendingPartialWithdrawal(&ethpb.PendingPartialWithdrawal{
+		Amount:            1008_000_000_000,
+		WithdrawableEpoch: primitives.Epoch(0),
+	}))
+	require.NoError(t, s.AppendPendingPartialWithdrawal(&ethpb.PendingPartialWithdrawal{
+		Amount:            1008_000_000_000,
+		WithdrawableEpoch: primitives.Epoch(0),
+	}))
+	expected, _, err := s.ExpectedWithdrawals()
+	require.NoError(t, err)
+	require.Equal(t, 3, len(expected)) // is a fully withdrawable validator
+	require.Equal(t, uint64(1008_000_000_000), expected[0].Amount)
+	require.Equal(t, uint64(975_000_000_000), expected[1].Amount)
+	require.Equal(t, uint64(32_000_000_000), expected[2].Amount)
+}

changelog/james-prysm_withdrawal-change.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- fixed underflow with balances in leaking edge case with expected withdrawals.