Off-by-one error prevents reaching MAX_VERIFIED_VALIDATORS limit

[naddison36]

The stakeEth(...) function is used to perform the initial 1 ETH deposit for a new validator that is in the REGISTERED state. The contract defines a constant MAX_VERIFIED_VALIDATORS set to 48, which is intended to be the maximum number of validators the system can manage.

However, the check to enforce this limit contains an off-by-one error. The require statement uses a strict less-than (<) operator, which prevents the number of validators from reaching the intended maximum.

function stakeEth(...) external onlyRegistrator whenNotPaused {
    // ...
    bytes32 pubKeyHash = _hashPubKey(validatorStakeData.pubkey);
    ValidatorState currentState = validator[pubKeyHash].state;
    // ...
    if (currentState == ValidatorState.REGISTERED) {
        // ...
        // Limits the number of validator balance proofs to verifyBalances
        // @audit-issue This check should use `<=` to allow exactly 48 validators.
        require(
            verifiedValidators.length + 1 < MAX_VERIFIED_VALIDATORS,
            "Max validators"
        );

        // ...
        firstDeposit = true;
        validator[pubKeyHash].state = ValidatorState.STAKED;
    }
    // ...
}

When there are 47 validators in the verifiedValidators array, verifiedValidators.length is 47. The check evaluates to 47 + 1 < 48, which is 48 < 48. This condition is false, causing the transaction to revert. As a result, it is impossible to stake for the 48th validator, effectively limiting the system to a maximum of 47 validators instead of the intended 48.

Recommendation(s): Consider changing the strict inequality operator (<) to a less-than-or-equal-to operator (<=). This will ensure that the number of verified validators can reach the MAX_VERIFIED_VALIDATORS limit.

Code Change Checklist

To be completed before internal review begins:

• The contract code is complete
• Executable deployment file
• Fork tests that test after the deployment file runs
• Unit tests *if needed
• The owner has done a full checklist review of the code + tests

Internal review:

• Two approvals by internal reviewers

[github-actions]

	Warnings
⚠️	👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against 4228f4a

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 40.76%. Comparing base (a241689) to head (4228f4a).
⚠️ Report is 243 commits behind head on nicka/pectra.

Additional details and impacted files

@@               Coverage Diff                @@
##           nicka/pectra    #2683      +/-   ##
================================================
- Coverage         42.66%   40.76%   -1.91%     
================================================
  Files               121      122       +1     
  Lines              5670     5748      +78     
  Branches           1502     1524      +22     
================================================
- Hits               2419     2343      -76     
- Misses             3248     3402     +154     
  Partials              3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sparrowDom]

LGTM