PR feedback from the amazing @rkapka

prysmaticlabs · May 15, 2024 · d6b9d0b · d6b9d0b
1 parent 3fe6947
commit d6b9d0b
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 16 deletions.
diff --git a/beacon-chain/core/electra/consolidations_test.go b/beacon-chain/core/electra/consolidations_test.go
@@ -259,8 +259,8 @@ func TestProcessConsolidations(t *testing.T) {
 			wantErr: "nil state",
 		},
 		{
-			name: "nil consolidation in slice",
-			state: stateWithActiveBalanceETH(t, 19_000_000),
+			name:    "nil consolidation in slice",
+			state:   stateWithActiveBalanceETH(t, 19_000_000),
 			scs:     []*eth.SignedConsolidation{nil, nil},
 			wantErr: "nil consolidation",
 		},

diff --git a/beacon-chain/core/electra/effective_balance_updates.go b/beacon-chain/core/electra/effective_balance_updates.go
@@ -30,7 +30,7 @@ import (
 //	            or validator.effective_balance + UPWARD_THRESHOLD < balance
 //	        ):
 //	            validator.effective_balance = min(balance - balance % EFFECTIVE_BALANCE_INCREMENT, EFFECTIVE_BALANCE_LIMIT)
-func ProcessEffectiveBalanceUpdates(state state.BeaconState) (state.BeaconState, error) {
+func ProcessEffectiveBalanceUpdates(state state.BeaconState) error {
 	effBalanceInc := params.BeaconConfig().EffectiveBalanceIncrement
 	hysteresisInc := effBalanceInc / params.BeaconConfig().HysteresisQuotient
 	downwardThreshold := hysteresisInc * params.BeaconConfig().HysteresisDownwardMultiplier
@@ -65,9 +65,5 @@ func ProcessEffectiveBalanceUpdates(state state.BeaconState) (state.BeaconState,
 		return false, val, nil
 	}
 
-	if err := state.ApplyToEveryValidator(validatorFunc); err != nil {
-		return nil, err
-	}
-
-	return state, nil
+	return state.ApplyToEveryValidator(validatorFunc)
 }
diff --git a/beacon-chain/core/electra/effective_balance_updates_test.go b/beacon-chain/core/electra/effective_balance_updates_test.go
@@ -53,7 +53,7 @@ func TestProcessEffectiveBalnceUpdates(t *testing.T) {
 				pb := &eth.BeaconStateElectra{
 					Validators: []*eth.Validator{
 						{
-							EffectiveBalance:      params.BeaconConfig().MinActivationBalance,
+							EffectiveBalance:      params.BeaconConfig().MinActivationBalance / 2,
 							WithdrawalCredentials: nil,
 						},
 					},
@@ -94,10 +94,10 @@ func TestProcessEffectiveBalnceUpdates(t *testing.T) {
 						},
 					},
 					Balances: []uint64{
-						params.BeaconConfig().MinActivationBalance - downwardThreshold - 1,
-						params.BeaconConfig().MinActivationBalance - downwardThreshold + 1,
-						params.BeaconConfig().MinActivationBalance + upwardThreshold + 1,
-						params.BeaconConfig().MinActivationBalance + upwardThreshold - 1,
+						params.BeaconConfig().MinActivationBalance - downwardThreshold - 1, // beyond downward threshold
+						params.BeaconConfig().MinActivationBalance - downwardThreshold + 1, // within downward threshold
+						params.BeaconConfig().MinActivationBalance + upwardThreshold + 1,   // beyond upward threshold
+						params.BeaconConfig().MinActivationBalance + upwardThreshold - 1,   // within upward threshold
 					},
 				}
 				st, err := state_native.InitializeFromProtoElectra(pb)
@@ -123,6 +123,8 @@ func TestProcessEffectiveBalnceUpdates(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, params.BeaconConfig().MinActivationBalance+params.BeaconConfig().EffectiveBalanceIncrement, val.EffectiveBalance)
 
+				// Validator 3 has a balance diff within the threshold so the effective balance should not
+				// have changed.
 				val, err = bs.ValidatorAtIndex(3)
 				require.NoError(t, err)
 				require.Equal(t, params.BeaconConfig().MinActivationBalance, val.EffectiveBalance)
@@ -132,10 +134,10 @@ func TestProcessEffectiveBalnceUpdates(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			res, err := electra.ProcessEffectiveBalanceUpdates(tt.state)
-			require.Equal(t, tt.wantErr, err != nil)
+			err := electra.ProcessEffectiveBalanceUpdates(tt.state)
+			require.Equal(t, tt.wantErr, err != nil, "unexpected error returned wanted error=nil (%s), got error=%s", tt.wantErr, err)
 			if tt.check != nil {
-				tt.check(t, res)
+				tt.check(t, tt.state)
 			}
 		})
 	}