-
Notifications
You must be signed in to change notification settings - Fork 117
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
feat(blockchain): introducing validator size cap size #2119
base: fix-maximum-number-withdrawals
Are you sure you want to change the base?
Changes from all commits
b395ac2
63e4db0
36fe774
e64bc74
c6c68c3
37542a0
ea69a35
af43b93
a728bcb
826351d
43b1eb4
561f95f
72539e0
3603f6b
32fbbbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -183,6 +183,10 @@ type Spec[ | |
// GetCometBFTConfigForSlot retrieves the CometBFT config for a specific | ||
// slot. | ||
GetCometBFTConfigForSlot(slot SlotT) CometBFTConfigT | ||
|
||
// GetValidatorSetCap retrieves the maximum number of validators | ||
// allowed in the active set. | ||
GetValidatorsSetCapSize() uint32 | ||
} | ||
|
||
// chainSpec is a concrete implementation of the ChainSpec interface, holding | ||
|
@@ -476,3 +480,11 @@ func (c chainSpec[ | |
]) GetCometBFTConfigForSlot(_ SlotT) CometBFTConfigT { | ||
return c.Data.CometValues | ||
} | ||
|
||
// GetValidatorSetCap retrieves the maximum number of validators | ||
// allowed in the active set. | ||
func (c chainSpec[ | ||
abi87 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT, | ||
]) GetValidatorsSetCapSize() uint32 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
return c.Data.ValidatorSetCapSize | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,8 @@ | |
package core | ||
|
||
import ( | ||
"fmt" | ||
|
||
"github.com/berachain/beacon-kit/mod/primitives/pkg/common" | ||
"github.com/berachain/beacon-kit/mod/primitives/pkg/constants" | ||
"github.com/berachain/beacon-kit/mod/primitives/pkg/encoding/hex" | ||
|
@@ -106,6 +108,17 @@ func (sp *StateProcessor[ | |
} | ||
} | ||
|
||
// BeaconKit enforces a cap on the validator set size. | ||
// If genesis deposits breaches the cap we return an error. | ||
//#nosec:G701 // can't overflow. | ||
if uint32(len(deposits)) > sp.cs.GetValidatorsSetCapSize() { | ||
return nil, fmt.Errorf("validator set cap %d, deposits count %d: %w", | ||
sp.cs.GetValidatorsSetCapSize(), | ||
len(deposits), | ||
ErrHitValidatorsSetCap, | ||
) | ||
} | ||
Comment on lines
+114
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here I assume deposits in genesis maps one to one with genesis validators and we do not have two different updates for the same validator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may be necessary to allow multiple deposits per genesis validator if each deposit is capped to some amount like 32 |
||
|
||
for _, deposit := range deposits { | ||
if err := sp.processDeposit(st, deposit); err != nil { | ||
return nil, err | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -196,12 +196,35 @@ func (sp *StateProcessor[ | |
math.Gwei(sp.cs.MaxEffectiveBalance()), | ||
) | ||
|
||
// TODO: This is a bug that lives on bArtio. Delete this eventually. | ||
if sp.cs.DepositEth1ChainID() == bArtioChainID { | ||
if err := st.AddValidatorBartio(val); err != nil { | ||
if !sp.processingGenesis { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor optimization which we may drop for readability. |
||
// BeaconKit enforces a cap on the validator set size. If a deposit is made | ||
// that would breach the cap, we mark the validator as immediately | ||
// withdrawable, so that it will be evicted in the next block along with | ||
// the amount deposited. | ||
validators, err := st.GetValidators() | ||
if err != nil { | ||
return err | ||
} | ||
} else if err := st.AddValidator(val); err != nil { | ||
|
||
//#nosec:G701 // can't overflow. | ||
if uint32(len(validators)) >= sp.cs.GetValidatorsSetCapSize() { | ||
var slot math.Slot | ||
slot, err = st.GetSlot() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
epoch := sp.cs.SlotToEpoch(slot) | ||
val.SetWithdrawableEpoch(epoch) | ||
} | ||
} | ||
|
||
// TODO: This is a bug that lives on bArtio. Delete this eventually. | ||
if sp.cs.DepositEth1ChainID() == bArtioChainID { | ||
return st.AddValidatorBartio(val) | ||
} | ||
Comment on lines
+223
to
+225
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replicating here the fix from #2111 to avoid double counting Bartio Validators balances (done implicitly in AddValidatorBartio) |
||
|
||
if err := st.AddValidator(val); err != nil { | ||
return err | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit + comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems comment was changed, but function name still needs to be changed to
GetValidatorSetCap