-
Notifications
You must be signed in to change notification settings - Fork 187
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
Changes from 79 commits
b4a1077
f4a7d79
0f46172
22c7717
05cba80
443ac1b
10efd5e
099716d
151a533
9818c7e
160cc88
4a9fe1c
e048be4
8bf34db
d90a95a
e17d29c
8dfe59d
daa9262
92d494f
6286b20
a1a3def
b395ac2
63e4db0
36fe774
e64bc74
c6c68c3
37542a0
ea69a35
af43b93
a728bcb
97fba5c
826351d
af8c5e0
023ebfd
d66b298
43b1eb4
420f621
561f95f
09eee8e
72539e0
d8a267e
3603f6b
32fbbbc
ebff958
8c9cf2d
f51b157
5e38855
bc26a85
401ab1a
9100d91
3699c20
9a7c057
0892421
1bd5f95
174229a
3cca1f3
0ae0c7d
3df7fc7
4d3de74
77f49c6
b96f776
882b91e
bca903b
ebcf8b4
d4ec3e7
3245f96
bfca971
5d5a57d
b898788
34ea814
e2962ac
0cdb569
3f3d4eb
3d9e3ab
0183914
eda7909
3e734e0
c6b5586
e9f01e2
3bc7678
679da51
b6d62c5
f361fe4
260bddb
f3c2b50
013aa95
9b139f8
0cbd981
52c4689
80af103
976c27b
1cd96e8
e8db0d9
5ff81ac
4774160
78d6429
e8b4dfd
4f90bfe
f36db52
39b83ca
355d99b
0307a03
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -45,6 +45,19 @@ type Spec[ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// calculations. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
EffectiveBalanceIncrement() uint64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// HysteresisQuotient returns the quotient used in effective balance | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// calculations to create hysteresis. This provides resistance to small | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// balance changes triggering effective balance updates. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
HysteresisQuotient() uint64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// HysteresisDownwardMultiplier returns the multiplier used when checking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if the effective balance should be decreased. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
HysteresisDownwardMultiplier() uint64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// HysteresisUpwardMultiplier returns the multiplier used when checking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if the effective balance should be increased. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
HysteresisUpwardMultiplier() uint64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Time parameters constants. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// SlotsPerEpoch returns the number of slots in an epoch. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -183,6 +196,10 @@ type Spec[ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// GetCometBFTConfigForSlot retrieves the CometBFT config for a specific | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// slot. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GetCometBFTConfigForSlot(slot SlotT) CometBFTConfigT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// GetValidatorSetCapSize retrieves the maximum number of validators | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// allowed in the active set. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GetValidatorSetCapSize() uint32 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// chainSpec is a concrete implementation of the ChainSpec interface, holding | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -245,6 +262,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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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. 🧹 Nitpick (assertive) Add documentation to implementation methods. The implementation methods should include documentation comments for consistency with other methods in the file. + // HysteresisQuotient returns the quotient used in hysteresis calculations.
func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisQuotient() uint64 {
return c.Data.HysteresisQuotient
}
+ // HysteresisDownwardMultiplier returns the multiplier used when adjusting values downward.
func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisDownwardMultiplier() uint64 {
return c.Data.HysteresisDownwardMultiplier
}
+ // HysteresisUpwardMultiplier returns the multiplier used when adjusting values upward.
func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) HysteresisUpwardMultiplier() uint64 {
return c.Data.HysteresisUpwardMultiplier
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// SlotsPerEpoch returns the number of slots per epoch. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (c chainSpec[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -476,3 +511,11 @@ func (c chainSpec[ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]) GetCometBFTConfigForSlot(_ SlotT) CometBFTConfigT { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return c.Data.CometValues | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// GetValidatorSetCapSize 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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
]) GetValidatorSetCapSize() uint32 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return c.Data.ValidatorSetCapSize | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
abi87 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# State Processor | ||
|
||
## Validators handling | ||
|
||
Currently: | ||
|
||
- Any validator whose effective balance is above `EjectionBalance` will stay a validator forever, as we have not (yet) implemented withdrawals facilities. | ||
- Withdrawals are automatically generated only if a validator effective balance goes beyond `MaxEffectiveBalance`. In this case enough balance is scheduled for withdrawal, just enough to make validator's effective balance equal to `MaxEffectiveBalance`. Since `MaxEffectiveBalance` > `EjectionBalance`, the validator will keep being a validator. | ||
- If a deposit is made for a validator with a balance smaller or equal to `EjectionBalance`, no validator will be created[^1] because of the insufficient balance. However currently the whole deposited balance is **not** scheduled for withdrawal at the next epoch. | ||
|
||
[^1]: Technically a validator is made in the BeaconKit state to track the deposit, but such a validator is never returned to the consensus engine. Moreover the deposit should be evicted at the next epoch. |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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" | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
|
@@ -36,8 +37,18 @@ 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) | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
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. 🧹 Nitpick (assertive) Consider optimizing the validator filtering implementation. While the filtering logic is correct, consider these improvements for better efficiency:
- activeVals := make([]ValidatorT, 0, len(vals))
- for _, val := range vals {
- if val.GetEffectiveBalance() > math.U64(sp.cs.EjectionBalance()) {
- activeVals = append(activeVals, val)
- }
- }
+ // Estimate capacity based on typical active/total ratio (e.g., 80%)
+ estimatedCap := (len(vals) * 4) / 5
+ activeVals := make([]ValidatorT, 0, estimatedCap)
+
+ // Use filter pattern for better readability
+ for i := range vals {
+ if vals[i].GetEffectiveBalance() > math.U64(sp.cs.EjectionBalance()) {
+ activeVals = append(activeVals, vals[i])
+ }
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
// TODO: a more efficient handling would be to only send back to consensus | ||||||||||||||||||||||||||||||||||||||
// updated validators (including evicted ones), rather than the full list | ||||||||||||||||||||||||||||||||||||||
return iter.MapErr( | ||||||||||||||||||||||||||||||||||||||
vals, | ||||||||||||||||||||||||||||||||||||||
activeVals, | ||||||||||||||||||||||||||||||||||||||
func(val *ValidatorT) (*transition.ValidatorUpdate, error) { | ||||||||||||||||||||||||||||||||||||||
v := (*val) | ||||||||||||||||||||||||||||||||||||||
return &transition.ValidatorUpdate{ | ||||||||||||||||||||||||||||||||||||||
|
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.
🛠️ Refactor suggestion
Rename method to
GetValidatorSetCap
for consistency.Based on previous review discussions, the method should be renamed while keeping the
uint32
return type.📝 Committable suggestion