fix(chain-1986): add MAX_BASE_SIGNER_COUNT to VerificationLib (#111)

* fix(chain-1986): add MAX_BASE_SIGNER_COUNT to VerificationLib

* chore: impl review feedback

Author: xenoliss

base/src/libraries/VerificationLib.sol

@@ -30,6 +30,9 @@ library VerificationLib {
     /// @notice The length of a signature in bytes.
     uint256 internal constant SIGNATURE_LENGTH_THRESHOLD = 65;
 
+    /// @notice The max allowed base signer count
+    uint256 internal constant MAX_BASE_SIGNER_COUNT = 16;
+
     /// @dev Slot for the `VerificationLibStorage` struct in storage.
     ///      Computed from:
     ///         keccak256(abi.encode(uint256(keccak256("coinbase.storage.VerificationLib")) - 1)) &
@@ -59,6 +62,9 @@ library VerificationLib {
     /// @notice Thrown when threshold is invalid (0 or exceeds validator count).
     error InvalidThreshold();
 
+    /// @notice Thrown when base signer count is too high.
+    error BaseSignerCountTooHigh();
+
     /// @notice Thrown when a validator address is 0.
     error InvalidValidatorAddress();
 
@@ -92,6 +98,7 @@ library VerificationLib {
         VerificationLibStorage storage $ = getVerificationLibStorage();
 
         require(threshold > 0 && threshold <= validators.length, InvalidThreshold());
+        require(validators.length <= MAX_BASE_SIGNER_COUNT, BaseSignerCountTooHigh());
 
         for (uint256 i; i < validators.length; i++) {
             require(validators[i] != address(0), InvalidValidatorAddress());
@@ -122,12 +129,15 @@ library VerificationLib {
         require(validator != address(0), InvalidValidatorAddress());
         require(!$.validators[validator], ValidatorAlreadyAdded());
 
-        $.validators[validator] = true;
-
+        uint128 newValidatorCount;
         unchecked {
-            $.validatorCount++;
+            newValidatorCount = $.validatorCount + 1;
         }
 
+        require(newValidatorCount <= MAX_BASE_SIGNER_COUNT, BaseSignerCountTooHigh());
+        $.validators[validator] = true;
+        $.validatorCount = newValidatorCount;
+
         emit ValidatorAdded(validator);
     }
 

base/test/BridgeValidator.t.sol

@@ -366,6 +366,23 @@ contract BridgeValidatorTest is CommonTest {
     ///                 Guardian/VerificationLib Tests          ///
     //////////////////////////////////////////////////////////////
 
+    function test_initialize_revertsWhenAboveMaxBaseSignerCount() public {
+        // Unset the initializer slot.
+        vm.store(
+            address(bridgeValidator),
+            bytes32(0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffbf601132),
+            bytes32(0)
+        );
+
+        address[] memory validators = new address[](VerificationLib.MAX_BASE_SIGNER_COUNT + 1);
+        for (uint256 i; i < VerificationLib.MAX_BASE_SIGNER_COUNT + 1; i++) {
+            validators[i] = vm.addr(i + 2);
+        }
+
+        vm.expectRevert(VerificationLib.BaseSignerCountTooHigh.selector);
+        bridgeValidator.initialize(validators, 3);
+    }
+
     function test_setThreshold_onlyGuardian_revertsForNonGuardian() public {
         vm.expectRevert(BridgeValidator.CallerNotGuardian.selector);
         bridgeValidator.setThreshold(1);
@@ -437,6 +454,17 @@ contract BridgeValidatorTest is CommonTest {
         bridgeValidator.removeValidator(vm.addr(1));
     }
 
+    function test_addValidator_revertsWhenAboveMaxBaseSignerCount() public {
+        vm.startPrank(cfg.guardians[0]);
+
+        for (uint256 i; i < VerificationLib.MAX_BASE_SIGNER_COUNT - 1; i++) {
+            bridgeValidator.addValidator(vm.addr(i + 2));
+        }
+
+        vm.expectRevert(VerificationLib.BaseSignerCountTooHigh.selector);
+        bridgeValidator.addValidator(vm.addr(0x42));
+    }
+
     function test_removeValidator_asGuardian_emitsEvent_andKeepsRegistering() public {
         // Add a second validator, keep threshold at 1, then remove it
         address newValidator = vm.addr(2);