-
Notifications
You must be signed in to change notification settings - Fork 129
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(staking): limit valset by config #1886
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant SP as StateProcessor
participant ST as State
participant V as Validator
SP->>ST: processStateTransition()
alt Error Handling
ST-->>SP: return error
else Successful Processing
SP->>V: processSyncCommitteeUpdates()
V-->>SP: return updates
end
SP->>ST: processForcedWithdrawals()
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
examples/berad/go.sum
is excluded by!**/*.sum
Files selected for processing (5)
- examples/berad/go.mod (1 hunks)
- examples/berad/pkg/chain-spec/berachain.go (1 hunks)
- examples/berad/pkg/state-transition/state_processor.go (1 hunks)
- examples/berad/pkg/state-transition/state_processor_committee.go (2 hunks)
- examples/berad/pkg/state-transition/state_processor_staking.go (2 hunks)
Additional comments not posted (2)
examples/berad/pkg/chain-spec/berachain.go (1)
34-37
: LGTM!The addition of the
MaxCommitteeSize
field and the improved comment forBGTContractAddress
enhance the clarity and functionality of theBeraChainSpec
struct.examples/berad/pkg/state-transition/state_processor_committee.go (1)
38-57
: Refactor approved!The refactoring of
processSyncCommitteeUpdates
to use an imperative approach enhances readability and maintainability. The logic for handling validators based onMaxCommitteeSize
is clear and well-structured.Consider verifying that the function's behavior aligns with the intended logic throughout the codebase.
Verification successful
To ensure the function
processSyncCommitteeUpdates
aligns with the intended logic throughout the codebase, let's examine how its output is used in the calling functions. This will help verify that the handling of validators based onMaxCommitteeSize
is consistent.
Function Behavior Verified
The usage of
processSyncCommitteeUpdates
across the codebase consistently handles the logic for validators based onMaxCommitteeSize
. The refactored function's behavior aligns with the intended logic, ensuring correct handling of validator updates.
- Files Verified:
state_processor.go
state_processor_genesis.go
state_processor_committee.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `processSyncCommitteeUpdates` to ensure it aligns with the intended logic. # Test: Search for the function usage. Expect: Consistent handling of validators based on `MaxCommitteeSize`. rg --type go -A 5 $'processSyncCommitteeUpdates'Length of output: 3777
Script:
#!/bin/bash # Check the context around the usage of processSyncCommitteeUpdates in the calling functions rg --type go -A 10 -B 5 'processSyncCommitteeUpdates'Length of output: 7939
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1886 +/- ##
==========================================
- Coverage 21.89% 21.87% -0.03%
==========================================
Files 338 338
Lines 15758 15777 +19
Branches 21 21
==========================================
Hits 3451 3451
- Misses 12190 12209 +19
Partials 117 117
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- examples/berad/pkg/state-transition/state_processor_committee.go (2 hunks)
Additional comments not posted (1)
examples/berad/pkg/state-transition/state_processor_committee.go (1)
38-58
: Improved readability and maintainability.The refactoring to an imperative style enhances the clarity of the code. The explicit handling of the validator updates is straightforward and easy to follow.
However, ensure that
MaxCommitteeSize
is always less than or equal to the length ofvals
to avoid potential index out-of-bounds errors.
no longer uses the giga library bc idk if i can iter w index but code cleaner?
will implement forced withdrawals in next PR + will do some statedb updates there as well
Summary by CodeRabbit
New Features
MaxCommitteeSize
, in the BeraChainSpec for enhanced configuration options.Bug Fixes
Documentation
Refactor