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

feature(state-processor): update validators EffectiveBalance only when epoch turns #2142

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions mod/chain-spec/pkg/chain/chain_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ type Spec[
// calculations.
EffectiveBalanceIncrement() uint64

HysteresisQuotient() uint64

HysteresisDownwardMultiplier() uint64

HysteresisUpwardMultiplier() uint64

abi87 marked this conversation as resolved.
Show resolved Hide resolved
// Time parameters constants.

// SlotsPerEpoch returns the number of slots in an epoch.
Expand Down Expand Up @@ -245,6 +251,24 @@ func (c chainSpec[
return c.Data.EffectiveBalanceIncrement
}

func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisQuotient() uint64 {
return c.Data.HysteresisQuotient
}

func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisDownwardMultiplier() uint64 {
return c.Data.HysteresisDownwardMultiplier
}

func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisUpwardMultiplier() uint64 {
return c.Data.HysteresisUpwardMultiplier
}

// SlotsPerEpoch returns the number of slots per epoch.
func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
Expand Down
5 changes: 5 additions & 0 deletions mod/chain-spec/pkg/chain/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ type SpecData[
// EffectiveBalanceIncrement is the effective balance increment.
EffectiveBalanceIncrement uint64 `mapstructure:"effective-balance-increment"`

HysteresisQuotient uint64 `mapstructure:"hysteresis-quotient"`

HysteresisDownwardMultiplier uint64 `mapstructure:"hysteresis-downward-multiplier"`

HysteresisUpwardMultiplier uint64 `mapstructure:"hysteresis-upward-multiplier"`
abi87 marked this conversation as resolved.
Show resolved Hide resolved
// Time parameters constants.
//
// SlotsPerEpoch is the number of slots per epoch.
Expand Down
73 changes: 60 additions & 13 deletions mod/state-transition/pkg/core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package core
import (
"bytes"

"github.com/berachain/beacon-kit/mod/consensus-types/pkg/types"
"github.com/berachain/beacon-kit/mod/errors"
"github.com/berachain/beacon-kit/mod/primitives/pkg/common"
"github.com/berachain/beacon-kit/mod/primitives/pkg/constants"
Expand Down Expand Up @@ -197,34 +198,26 @@ func (sp *StateProcessor[
]) ProcessSlots(
st BeaconStateT, slot math.Slot,
) (transition.ValidatorUpdates, error) {
var (
validatorUpdates transition.ValidatorUpdates
epochValidatorUpdates transition.ValidatorUpdates
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nit: reduced epochValidatorUpdates scope (it's only used when processing epochs)

)

var res transition.ValidatorUpdates
stateSlot, err := st.GetSlot()
if err != nil {
return nil, err
}

// Iterate until we are "caught up".
for ; stateSlot < slot; stateSlot++ {
// Process the slot
if err = sp.processSlot(st); err != nil {
return nil, err
}

// Process the Epoch Boundary.
boundary := (stateSlot.Unwrap()+1)%sp.cs.SlotsPerEpoch() == 0
if boundary {
if epochValidatorUpdates, err =
sp.processEpoch(st); err != nil {
var epochUpdates transition.ValidatorUpdates
if epochUpdates, err = sp.processEpoch(st); err != nil {
return nil, err
}
validatorUpdates = append(
validatorUpdates,
epochValidatorUpdates...,
)
res = append(res, epochUpdates...)
}

// We update on the state because we need to
Expand All @@ -234,7 +227,7 @@ func (sp *StateProcessor[
}
}

return validatorUpdates, nil
return res, nil
}

// processSlot is run when a slot is missed.
Expand Down Expand Up @@ -336,6 +329,9 @@ func (sp *StateProcessor[
if err := sp.processRewardsAndPenalties(st); err != nil {
return nil, err
}
if err := sp.processEffectiveBalanceUpdates(st); err != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is one of the main point of the PR: along Eth 2.0 specs, EffectiveBalances must be updated only once per epoch, not every slot

return nil, err
}
if err := sp.processSlashingsReset(st); err != nil {
return nil, err
}
Expand Down Expand Up @@ -516,3 +512,54 @@ func (sp *StateProcessor[

return nil
}

// processEffectiveBalanceUpdates as defined in the Ethereum 2.0 specification.
// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#effective-balances-updates
//
//nolint:lll
func (sp *StateProcessor[
_, _, _, BeaconStateT, _, _, _, _, _, _, _, _, _, _, _, _, _,
]) processEffectiveBalanceUpdates(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is straight from Eth 2.0 specs.
In essence, at each epoch, check if a validator balance has moved from its effective balance more than a certain amount. If so update the balance

st BeaconStateT,
) error {
// Update effective balances with hysteresis
validators, err := st.GetValidators()
if err != nil {
return err
}

var (
hysteresisIncrement = sp.cs.EffectiveBalanceIncrement() / sp.cs.HysteresisQuotient()
downwardThreshold = hysteresisIncrement * sp.cs.HysteresisDownwardMultiplier()
upwardThreshold = hysteresisIncrement * sp.cs.HysteresisUpwardMultiplier()

idx math.U64
balance math.Gwei
)

for _, val := range validators {
idx, err = st.ValidatorIndexByPubkey(val.GetPubkey())
if err != nil {
return err
}

balance, err = st.GetBalance(idx)
if err != nil {
return err
}

if balance+math.Gwei(downwardThreshold) < val.GetEffectiveBalance() ||
val.GetEffectiveBalance()+math.Gwei(upwardThreshold) < balance {
updatedBalance := types.ComputeEffectiveBalance(
balance,
math.U64(sp.cs.EffectiveBalanceIncrement()),
math.U64(sp.cs.MaxEffectiveBalance()),
)
val.SetEffectiveBalance(updatedBalance)
if err = st.UpdateValidatorAtIndex(idx, val); err != nil {
return err
}
}
}
return nil
}
11 changes: 10 additions & 1 deletion mod/state-transition/pkg/core/state_processor_committee.go
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible TODO

Mainnet follows: If balance below ejection balance after 1 epoch --> trigger withdrawal for validator

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package core

import (
"github.com/berachain/beacon-kit/mod/primitives/pkg/math"
"github.com/berachain/beacon-kit/mod/primitives/pkg/transition"
"github.com/sourcegraph/conc/iter"
)
Expand All @@ -36,8 +37,16 @@ func (sp *StateProcessor[
return nil, err
}

// filter out validators whose effective balance is not sufficient to validate
activeVals := make([]ValidatorT, 0, len(vals))
for _, val := range vals {
if val.GetEffectiveBalance() > math.U64(sp.cs.EjectionBalance()) {
activeVals = append(activeVals, val)
}
}
Comment on lines +41 to +46
Copy link
Collaborator Author

@abi87 abi87 Nov 9, 2024

Choose a reason for hiding this comment

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

you may ask why we didn't do it so far. The reason is so far:

  • Validator are activated as soon as they are added
  • They never reduces their stake
    • In fact this is not exactly true. We do reduce stake if a validator's balance goes beyond MaxEffectiveBalance but we reduce it just enough to get it to MaxEffectiveBalance. So the validator will stay active.

We need to change this to allow a capped validator set where validators may leave due to a validator with more stake coming in

abi87 marked this conversation as resolved.
Show resolved Hide resolved

return iter.MapErr(
vals,
activeVals,
func(val *ValidatorT) (*transition.ValidatorUpdate, error) {
v := (*val)
return &transition.ValidatorUpdate{
Expand Down
Loading
Loading