chore(forks): Handle MaxValidatorsPerWithdrawalsSweep based on planned fork upgrade #2194
Conversation
WalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2194 +/- ##
===========================================
+ Coverage 27.49% 40.42% +12.93%
===========================================
Files 365 4 -361
Lines 16812 47 -16765
Branches 12 12
===========================================
- Hits 4622 19 -4603
+ Misses 11881 28 -11853
+ Partials 309 0 -309 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range comments (1)
mod/config/pkg/spec/base.go (1)
Line range hint
1-136: Consider documenting the upgrade strategyThe introduction of pre/post upgrade distinctions in the spec suggests an upgrade mechanism. Consider adding documentation in the package comments explaining:
- The upgrade strategy and its implications
- How pre/post upgrade values are determined and used
- The relationship between fork epochs and upgrade-specific values
This will help maintainers understand the upgrade process and its impact on the system.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (7)
mod/chain-spec/pkg/chain/chain_spec.go(2 hunks)mod/chain-spec/pkg/chain/data.go(2 hunks)mod/config/pkg/spec/base.go(3 hunks)mod/config/pkg/spec/boonet.go(1 hunks)mod/state-transition/pkg/core/state/statedb.go(1 hunks)mod/state-transition/pkg/core/state/upgrade.go(1 hunks)mod/state-transition/pkg/core/state_processor_withdrawals.go(7 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
mod/chain-spec/pkg/chain/chain_spec.go
[warning] 498-498: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block
(revive)
mod/chain-spec/pkg/chain/data.go
132-132: File is not goimports-ed
(goimports)
🔇 Additional comments (16)
mod/state-transition/pkg/core/state/upgrade.go (1)
31-44: Verify chain ID constants and fork height
Let's verify the chain ID constants and fork height values are correctly defined and consistent across the codebase.
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Chain ID constants and fork heights are correctly defined and consistently used
The verification shows that:
BartioChainIDandBoonetEth1ChainIDare properly defined in the spec package, withBoonetEth1ChainID = 80000BoonetFork2Heightis defined as99999999999999inmod/config/pkg/spec/special_cases.go- The constants are consistently used across the codebase for fork logic, particularly in:
- State transitions
- Deposit validations
- Withdrawal processing
- Block building
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify chain ID constants and fork height definitions
# Check for chain ID constant definitions
echo "Checking chain ID definitions..."
rg -A 2 "BartioChainID|BoonetEth1ChainID"
# Check for fork height constant definition and usage
echo "Checking fork height definition and usage..."
rg -A 2 "BoonetFork2Height"
# Check for other potential fork version checks
echo "Checking for other fork version checks..."
ast-grep --pattern 'if $_ < $_Fork$_Height'
Length of output: 9726
mod/config/pkg/spec/boonet.go (4)
37-37: LGTM! Variable renaming improves clarity.
The renaming from testnetSpec to boonetSpec better reflects the context and purpose of this configuration.
52-57: 🧹 Nitpick (assertive)
Address TODO and verify the upgrade-related property rename.
Two important points need attention:
- The TODO indicates this value needs to be determined for the boonet upgrade.
- The property has been renamed to include "PostUpgrade", suggesting a distinction between pre and post upgrade behavior.
Would you like me to help create a GitHub issue to track the determination of this value?
✅ Verification successful
The pre/post upgrade property distinction is well-defined, but the post-upgrade value needs determination
The codebase shows a clear and intentional distinction between pre and post-upgrade behavior:
- Pre-upgrade: Value is 2^14 (16384), consistent with ETH2.0 spec
- Post-upgrade: Value is intentionally set to 43 as the largest prime number below the minimum validator set size (46)
The TODO can be addressed as the post-upgrade value's rationale is already documented in both the code comment and the property documentation. The value of 43 appears to be a deliberate choice rather than a placeholder.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related pre/post upgrade properties and their usage
rg "MaxValidatorsPerWithdrawalsSweep(Pre|Post)Upgrade"
Length of output: 950
Script:
#!/bin/bash
# Let's check the documentation and usage context for these properties
rg -A 3 "MaxValidatorsPerWithdrawalsSweepPreUpgrade specifies" mod/chain-spec/pkg/chain/data.go
rg -A 3 "MaxValidatorsPerWithdrawalsSweepPostUpgrade specifies" mod/chain-spec/pkg/chain/data.go
# Let's also check where and how these values are used
ast-grep --pattern 'MaxValidatorsPerWithdrawalsSweep$_'
Length of output: 839
40-40: Verify the Chain ID constant value.
The comment indicates Chain ID should be 80000. Let's verify the constant is correctly defined.
✅ Verification successful
Chain ID constant is correctly defined as 80000
The constant BoonetEth1ChainID is correctly defined in mod/config/pkg/spec/chain_ids.go with the expected value of 80000, and it's properly used in the boonet configuration file. The comment in the code also accurately reflects this value.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BoonetEth1ChainID constant definition
rg -A 1 "BoonetEth1ChainID\s*=\s*"
Length of output: 36
Script:
#!/bin/bash
# Let's try a broader search for the constant name without the assignment operator
rg "BoonetEth1ChainID" --type go
Length of output: 1365
Script:
#!/bin/bash
# Let's also check for any chain ID related constants in the config package
fd . -e go -E "vendor/*" --exec grep -l "ChainID" {}
Length of output: 1380
Script:
#!/bin/bash
# Let's look at the content of the file containing our target line
cat mod/config/pkg/spec/boonet.go
Length of output: 2134
43-45:
Verify the EVM inflation address.
This address is critical for token minting. Please ensure this is the correct production address for the Boonet environment.
mod/config/pkg/spec/base.go (3)
58-62: LGTM! Improved readability with scientific notation
The Gwei value constants are correctly defined using scientific notation, making the values more readable while maintaining the standard Ethereum specifications.
133-133: LGTM! Proper consensus parameters assignment
The CometValues assignment is correct and properly initialized with the default consensus parameters.
123-125: Verify the post-upgrade withdrawal sweep configuration
The renaming to MaxValidatorsPerWithdrawalsSweepPreUpgrade suggests there should be a corresponding post-upgrade value. Please ensure this is properly configured elsewhere in the codebase.
✅ Verification successful
Pre/post upgrade withdrawal sweep configuration is properly implemented
The codebase has a complete implementation for handling both pre and post-upgrade withdrawal sweep configurations:
- The
MaxValidatorsPerWithdrawalsSweepmethod in the chain spec properly handles both cases using anisPostUpgradefunction - Pre-upgrade value is set to 1 << 14 (16384)
- Post-upgrade value is configurable per network (e.g., set to 43 in boonet)
- The implementation includes proper documentation and type safety
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the post-upgrade configuration
echo "Searching for MaxValidatorsPerWithdrawalsSweepPostUpgrade configuration..."
rg "MaxValidatorsPerWithdrawalsSweepPostUpgrade" -A 2
echo "Checking for related interface methods..."
ast-grep --pattern 'interface Spec {
$$$
MaxValidatorsPerWithdrawalsSweep($$$) $$$
$$$
}'
Length of output: 1414
Script:
#!/bin/bash
# Let's verify the interface and implementation details
echo "Checking for the interface definition..."
ast-grep --pattern 'interface Spec {
$$$
MaxValidatorsPerWithdrawalsSweep($$$) $$$
}'
echo "\nChecking implementation of MaxValidatorsPerWithdrawalsSweep..."
rg "func.*MaxValidatorsPerWithdrawalsSweep" -A 3
echo "\nChecking for upgrade-related conditions..."
rg "IsWithdrawalsSweepUpgradeActive|WithdrawalsSweepUpgradeHeight" -A 2
Length of output: 545
Script:
#!/bin/bash
# Let's check the chain spec implementation more broadly
echo "Checking chain spec implementation..."
rg "MaxValidatorsPerWithdrawalsSweep" -B 2 -A 2 mod/chain-spec/pkg/chain/chain_spec.go
echo "\nChecking for upgrade height configuration..."
rg "WithdrawalsSweepUpgrade" mod/chain-spec/pkg/chain/data.go
echo "\nChecking for usage of withdrawal sweep values..."
rg "MaxValidatorsPerWithdrawalsSweep" mod/config/pkg/spec/base.go -B 2 -A 2
Length of output: 1384
mod/chain-spec/pkg/chain/data.go (2)
136-140: 🧹 Nitpick (assertive)
Documentation needs clarification on post-upgrade value determination
The documentation for MaxValidatorsPerWithdrawalsSweepPostUpgrade needs more detail:
- What determines the "minimum possible amount of total validators"?
- Why is a prime number specifically chosen?
- How is this value calculated or determined?
This information is crucial for understanding the system's behavior post-upgrade.
132-140: 🧹 Nitpick (assertive)
Verify upgrade transition handling
The split of MaxValidatorsPerWithdrawalsSweep into pre/post upgrade variants suggests a significant change in withdrawal processing. Please ensure:
- The transition between pre and post upgrade values is handled gracefully
- There are no edge cases during the upgrade transition
- The change in withdrawal sweep size doesn't impact system stability
✅ Verification successful
Let me gather more information about the withdrawal processing logic and upgrade handling.
Let me gather more specific information about the upgrade transition handling.
Based on the gathered information, I can now provide a final response about the withdrawal sweep transition handling.
Transition handling is properly implemented with clear upgrade boundaries
The upgrade transition for withdrawal sweep sizes is handled safely through:
- A clear upgrade boundary check via
IsPostUpgrade()that uses a well-defined height (BoonetFork2Height) - A clean switch between pre/post values in
MaxValidatorsPerWithdrawalsSweep()based on the upgrade status - Conservative default values:
- Pre-upgrade: ETH2.0 spec compatible value (2^14)
- Post-upgrade: Smaller prime number (43) based on minimum validator set size
The implementation avoids edge cases by:
- Using a high upgrade height (99999999999999) as a safety barrier
- Having a clear chain ID-based branching logic
- Maintaining backward compatibility with pre-upgrade values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for upgrade transition handling code
rg -A 10 "IsPostUpgrade|UpgradeStatus"
# Search for withdrawal processing logic that might be affected
ast-grep --pattern 'func $_($$) {
$$$
withdrawal
$$$
}'
Length of output: 3505
Script:
#!/bin/bash
# Search for withdrawal sweep size usage
rg -A 5 "MaxValidatorsPerWithdrawalsSweep"
# Look for withdrawal processing functions
rg -A 10 "func.*[Ww]ithdrawal.*"
# Search for upgrade-related configuration
rg -A 5 "BoonetFork2Height|Fork2"
Length of output: 118081
Script:
#!/bin/bash
# Look for MaxValidatorsPerWithdrawalsSweep usage in state processor
rg -A 10 "MaxValidatorsPerWithdrawalsSweep.*IsPostUpgrade"
# Check for upgrade-related state transitions
rg -A 10 "func.*IsPostUpgrade"
# Look for upgrade height configuration
rg -A 5 "BoonetFork2Height.*="
Length of output: 1001
🧰 Tools
🪛 golangci-lint (1.62.2)
132-132: File is not goimports-ed
(goimports)
mod/state-transition/pkg/core/state_processor_withdrawals.go (5)
29-29: Importing the 'state' package for upgrade checks
The addition of the import "github.com/berachain/beacon-kit/mod/state-transition/pkg/core/state" is appropriate as it allows access to state.IsPostUpgrade needed for upgrade-related logic.
97-97: Passing 'slot' parameter enhances chain-specific logic
Including the slot parameter in the call to processWithdrawalsBartio enables the function to apply chain-specific logic based on the current slot, which aligns with the PR objectives.
121-121: Passing 'slot' parameter to 'processWithdrawalsBartio'
Adding slot to the function call ensures consistency and allows for proper handling of withdrawals before BoonetFork2Height on the Boonet chain.
129-129: Passing 'slot' parameter to 'processWithdrawalsDefault'
Including slot in the call to processWithdrawalsDefault provides necessary context for processing withdrawals in the default case, supporting the new upgrade logic.
141-141: Adding 'slot' parameter to 'processWithdrawalsBartio' function signature
The slot parameter is correctly added to the function signature, allowing internal logic to utilize the current slot information.
mod/chain-spec/pkg/chain/chain_spec.go (1)
158-162: Ensure all implementations and usages of MaxValidatorsPerWithdrawalsSweep are updated
Changing the method signature of a public interface can have widespread impact. Please verify that all implementations and calls to MaxValidatorsPerWithdrawalsSweep are updated to match the new signature.
Run the following script to verify all declarations and usages of MaxValidatorsPerWithdrawalsSweep:
✅ Verification successful
Based on the search results, I can generate the final response as we have comprehensive information about both the interface definition and all its usages:
All implementations and usages of MaxValidatorsPerWithdrawalsSweep are correctly aligned with the new signature
The verification shows that:
- The interface definition in
chain_spec.gomatches the implementation in the same file - All three usage locations correctly provide the required parameters:
state_processor_withdrawals.go(2 occurrences)statedb.go(1 occurrence)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all implementations and usages of MaxValidatorsPerWithdrawalsSweep are updated to the new signature.
# Search for all method declarations and interface definitions
rg 'MaxValidatorsPerWithdrawalsSweep\(' -A 3 -B 3
# Search for all calls to MaxValidatorsPerWithdrawalsSweep
rg 'MaxValidatorsPerWithdrawalsSweep\(' --type go
Length of output: 3337
|
|
||
| // IsPostUpgrade returns true if the chain is post-upgrade (Fork2 on Boonet). | ||
| // | ||
| // TODO: Jank. Refactor into better fork version management. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Track technical debt: Fork version management needs improvement
The TODO comment indicates this is a temporary solution. We should track this technical debt to ensure it gets properly addressed in the future.
Would you like me to create a GitHub issue to track the implementation of a more robust fork version management system?
| func IsPostUpgrade(chainID uint64, slot math.Slot) bool { | ||
| switch chainID { | ||
| case spec.BartioChainID: | ||
| return false | ||
| case spec.BoonetEth1ChainID: | ||
| if slot < math.U64(spec.BoonetFork2Height) { | ||
| return false | ||
| } | ||
|
|
||
| return true | ||
| default: | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing error handling and documentation
While the implementation is functional, there are several areas for improvement:
- The default case returns
true, which might silently pass unknown chain IDs - The magic number
spec.BoonetFork2Heightcould benefit from additional context - The function lacks documentation about the significance of the upgrade state
Consider applying these improvements:
-// IsPostUpgrade returns true if the chain is post-upgrade (Fork2 on Boonet).
+// IsPostUpgrade determines if the chain has passed its designated upgrade point.
+//
+// For BartioChain: Always returns false as it hasn't undergone the upgrade
+// For BoonetEth1: Returns true if the slot is beyond BoonetFork2Height (upgrade point)
+// For other chains: Returns true by default
+//
+// Parameters:
+// - chainID: The identifier for the blockchain network
+// - slot: The current slot number being processed
+//
+// Returns:
+// bool: true if the chain is in post-upgrade state, false otherwise
func IsPostUpgrade(chainID uint64, slot math.Slot) bool {
switch chainID {
case spec.BartioChainID:
return false
case spec.BoonetEth1ChainID:
+ // BoonetFork2Height represents the slot at which the network upgrade occurs
if slot < math.U64(spec.BoonetFork2Height) {
return false
}
return true
default:
+ // TODO: Consider if defaulting to true is the safest option
return true
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func IsPostUpgrade(chainID uint64, slot math.Slot) bool { | |
| switch chainID { | |
| case spec.BartioChainID: | |
| return false | |
| case spec.BoonetEth1ChainID: | |
| if slot < math.U64(spec.BoonetFork2Height) { | |
| return false | |
| } | |
| return true | |
| default: | |
| return true | |
| } | |
| } | |
| // IsPostUpgrade determines if the chain has passed its designated upgrade point. | |
| // | |
| // For BartioChain: Always returns false as it hasn't undergone the upgrade | |
| // For BoonetEth1: Returns true if the slot is beyond BoonetFork2Height (upgrade point) | |
| // For other chains: Returns true by default | |
| // | |
| // Parameters: | |
| // - chainID: The identifier for the blockchain network | |
| // - slot: The current slot number being processed | |
| // | |
| // Returns: | |
| // bool: true if the chain is in post-upgrade state, false otherwise | |
| func IsPostUpgrade(chainID uint64, slot math.Slot) bool { | |
| switch chainID { | |
| case spec.BartioChainID: | |
| return false | |
| case spec.BoonetEth1ChainID: | |
| // BoonetFork2Height represents the slot at which the network upgrade occurs | |
| if slot < math.U64(spec.BoonetFork2Height) { | |
| return false | |
| } | |
| return true | |
| default: | |
| // TODO: Consider if defaulting to true is the safest option | |
| return true | |
| } | |
| } |
| // BERA per block minting. | ||
| // | ||
| // TODO: determine correct value for boonet upgrade. | ||
| testnetSpec.EVMInflationPerBlock = 2.5e9 | ||
| boonetSpec.EVMInflationPerBlock = 2.5e9 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Address TODO: Determine inflation rate for boonet upgrade.
The inflation rate is a critical economic parameter. The TODO comment indicates this value needs to be determined for the boonet upgrade.
Would you like me to help create a GitHub issue to track this decision?
| // Fork-related values. | ||
| DenebPlusForkEpoch: 9999999999999998, | ||
| ElectraForkEpoch: 9999999999999999, | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding documentation for fork epoch values
The fork epoch values are set to effectively infinite values, which is a good practice for disabling forks until they're ready. Consider adding a comment explaining this intention.
+ // Fork epochs are set to effectively infinite values to disable them until ready
DenebPlusForkEpoch: 9999999999999998,
ElectraForkEpoch: 9999999999999999,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fork-related values. | |
| DenebPlusForkEpoch: 9999999999999998, | |
| ElectraForkEpoch: 9999999999999999, | |
| // Fork epochs are set to effectively infinite values to disable them until ready | |
| DenebPlusForkEpoch: 9999999999999998, | |
| ElectraForkEpoch: 9999999999999999, | |
| sp.cs.MaxValidatorsPerWithdrawalsSweep( | ||
| state.IsPostUpgrade, spec.BartioChainID, slot, | ||
| )) |
There was a problem hiding this comment.
Call 'state.IsPostUpgrade' function instead of passing it as a reference
In lines 194-195, state.IsPostUpgrade is passed as a function reference to MaxValidatorsPerWithdrawalsSweep, but it should be called with the appropriate parameters to return a boolean value. This could lead to a logical error or compile-time issue.
Apply this diff to fix the issue:
nextValidatorIndex += math.ValidatorIndex(
- sp.cs.MaxValidatorsPerWithdrawalsSweep(
- state.IsPostUpgrade, spec.BartioChainID, slot,
+ sp.cs.MaxValidatorsPerWithdrawalsSweep(
+ state.IsPostUpgrade(spec.BartioChainID, slot),
+ spec.BartioChainID, slot,
))This ensures that state.IsPostUpgrade is invoked properly with spec.BartioChainID and slot as arguments, returning the expected boolean value needed by MaxValidatorsPerWithdrawalsSweep.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sp.cs.MaxValidatorsPerWithdrawalsSweep( | |
| state.IsPostUpgrade, spec.BartioChainID, slot, | |
| )) | |
| sp.cs.MaxValidatorsPerWithdrawalsSweep( | |
| state.IsPostUpgrade(spec.BartioChainID, slot), spec.BartioChainID, slot, | |
| )) |
| sp.cs.MaxValidatorsPerWithdrawalsSweep( | ||
| state.IsPostUpgrade, sp.cs.DepositEth1ChainID(), slot, | ||
| )) |
There was a problem hiding this comment.
Call 'state.IsPostUpgrade' function instead of passing it as a reference
Similarly, in lines 271-272, state.IsPostUpgrade is passed as a reference instead of being called. This should be corrected to prevent potential runtime errors.
Apply this diff to fix the issue:
nextValidatorIndex += math.ValidatorIndex(
- sp.cs.MaxValidatorsPerWithdrawalsSweep(
- state.IsPostUpgrade, sp.cs.DepositEth1ChainID(), slot,
+ sp.cs.MaxValidatorsPerWithdrawalsSweep(
+ state.IsPostUpgrade(sp.cs.DepositEth1ChainID(), slot),
+ sp.cs.DepositEth1ChainID(), slot,
))This modification calls state.IsPostUpgrade with sp.cs.DepositEth1ChainID() and slot, ensuring that the boolean result is correctly passed to MaxValidatorsPerWithdrawalsSweep.
Committable suggestion skipped: line range outside the PR's diff.
| totalValidators, s.cs.MaxValidatorsPerWithdrawalsSweep( | ||
| IsPostUpgrade, s.cs.DepositEth1ChainID(), slot, | ||
| ), |
There was a problem hiding this comment.
Undefined variable IsPostUpgrade
The variable IsPostUpgrade is used as a parameter but is not defined within the current scope. This will result in a compilation error. Please define IsPostUpgrade before using it.
Apply this diff to define IsPostUpgrade:
slot, err := s.GetSlot()
if err != nil {
return nil, err
}
+// Determine if the chain is post-upgrade based on the current slot
+IsPostUpgrade := s.IsPostUpgrade(slot)
bound := min(
totalValidators, s.cs.MaxValidatorsPerWithdrawalsSweep(
IsPostUpgrade, s.cs.DepositEth1ChainID(), slot,
),
)Committable suggestion skipped: line range outside the PR's diff.
| if isPostUpgrade(chainID, slot) { | ||
| return c.Data.MaxValidatorsPerWithdrawalsSweepPostUpgrade | ||
| } else { | ||
| return c.Data.MaxValidatorsPerWithdrawalsSweepPreUpgrade | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify conditional logic by removing unnecessary else clause
Since the if block ends with a return statement, the else clause is unnecessary. Removing it improves code readability and aligns with Go's idiomatic practices.
Apply the following diff to simplify the code:
func (c chainSpec[
DomainTypeT, EpochT, ExecutionAddressT, SlotT, CometBFTConfigT,
]) MaxValidatorsPerWithdrawalsSweep(
isPostUpgrade func(uint64, SlotT) bool,
chainID uint64, slot SlotT,
) uint64 {
if isPostUpgrade(chainID, slot) {
return c.Data.MaxValidatorsPerWithdrawalsSweepPostUpgrade
- } else {
+ }
return c.Data.MaxValidatorsPerWithdrawalsSweepPreUpgrade
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if isPostUpgrade(chainID, slot) { | |
| return c.Data.MaxValidatorsPerWithdrawalsSweepPostUpgrade | |
| } else { | |
| return c.Data.MaxValidatorsPerWithdrawalsSweepPreUpgrade | |
| } | |
| if isPostUpgrade(chainID, slot) { | |
| return c.Data.MaxValidatorsPerWithdrawalsSweepPostUpgrade | |
| } | |
| return c.Data.MaxValidatorsPerWithdrawalsSweepPreUpgrade |
🧰 Tools
🪛 golangci-lint (1.62.2)
[warning] 498-498: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block
(revive)
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Documentation