audit fixes/improvements

RELEASE_NOTES.md

@@ -10,7 +10,7 @@
 #### Added
 
 **SSVNetwork**
-- `function setOperatosWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
+- `function setOperatorsWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
 - `function removeOperatorsWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
 - `function setOperatorsWhitelistingContract(uint64[] calldata operatorIds, ISSVWhitelistingContract whitelistingContract)`
 - `function setOperatorsPrivateUnchecked(uint64[] calldata operatorIds)`

contracts/SSVNetwork.sol

@@ -121,15 +121,19 @@ contract SSVNetwork is
     /* Operator External Functions */
     /*******************************/
 
-    function registerOperator(bytes calldata publicKey, uint256 fee, bool setPrivate) external override returns (uint64 id) {
+    function registerOperator(
+        bytes calldata publicKey,
+        uint256 fee,
+        bool setPrivate
+    ) external override returns (uint64 id) {
         _delegate(SSVStorage.load().ssvContracts[SSVModules.SSV_OPERATORS]);
     }
 
     function removeOperator(uint64 operatorId) external override {
         _delegate(SSVStorage.load().ssvContracts[SSVModules.SSV_OPERATORS]);
     }
 
-    function setOperatosWhitelists(
+    function setOperatorsWhitelists(
         uint64[] calldata operatorIds,
         address[] calldata whitelistAddresses
     ) external override {

contracts/interfaces/ISSVOperatorsWhitelist.sol

@@ -6,9 +6,11 @@ import {ISSVWhitelistingContract} from "./external/ISSVWhitelistingContract.sol"
 
 interface ISSVOperatorsWhitelist is ISSVNetworkCore {
     /// @notice Sets a list of whitelisted addresses (EOAs or generic contracts) for a list of operators
+    /// @notice Changes to an operator's whitelist will not impact existing validators registered with that operator
+    /// @notice Only new validator registrations will adhere to the updated whitelist rules
     /// @param operatorIds The operator IDs to set the whitelists for
     /// @param whitelistAddresses The list of addresses to be whitelisted
-    function setOperatosWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses) external;
+    function setOperatorsWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses) external;
 
     /// @notice Removes a list of whitelisted addresses (EOAs or generic contracts) for a list of operators
     /// @param operatorIds Operator IDs for which whitelists are removed

contracts/libraries/OperatorLib.sol

@@ -142,23 +142,25 @@ library OperatorLib {
         checkOperatorsLength(operatorIds);
 
         // create the max number of masks that will be updated
-        uint256[] memory masks = generateBlockMasks(operatorIds, true, s);
+        (uint256[] memory masks, uint256 startBlockIndex) = generateBlockMasks(operatorIds, true, s);
+        uint256 endBlockIndex = startBlockIndex + masks.length;
 
         for (uint256 i; i < addressesLength; ++i) {
             address whitelistAddress = whitelistAddresses[i];
             checkZeroAddress(whitelistAddress);
 
-            // If whitelistAddress is a custom contract, revert also when removing
-            if (isWhitelistingContract(whitelistAddress))
+            // If whitelistAddress is a custom contract, reverts only when registering addresses
+            if (registerAddresses && isWhitelistingContract(whitelistAddress))
                 revert ISSVNetworkCore.AddressIsWhitelistingContract(whitelistAddress);
 
-            for (uint256 blockIndex; blockIndex < masks.length; ++blockIndex) {
+            for (uint256 blockIndex = startBlockIndex; blockIndex < endBlockIndex; ++blockIndex) {
                 // only update storage for updated masks
-                if (masks[blockIndex] != 0) {
+                uint256 mask = masks[blockIndex - startBlockIndex];
+                if (mask != 0) {
                     if (registerAddresses) {
-                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] |= masks[blockIndex];
+                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] |= mask;
                     } else {
-                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] &= ~masks[blockIndex];
+                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] &= ~mask;
                     }
                 }
             }
@@ -169,15 +171,15 @@ library OperatorLib {
         uint64[] calldata operatorIds,
         bool checkOperatorsOwnership,
         StorageData storage s
-    ) internal view returns (uint256[] memory masks) {
-        uint256 blockIndex;
-        uint256 bitPosition;
-        uint64 currentOperatorId;
-
+    ) internal view returns (uint256[] memory masks, uint256 startBlockIndex) {
         uint256 operatorsLength = operatorIds.length;
+        startBlockIndex = operatorIds[0] >> 8;
 
-        // create the max number of masks that will be updated
-        masks = new uint256[]((operatorIds[operatorsLength - 1] >> 8) + 1);
+        // Create the masks array from startBlockIndex to the last block index
+        masks = new uint256[]((operatorIds[operatorsLength - 1] >> 8) - startBlockIndex + 1);
+
+        uint64 currentOperatorId;
+        uint64 prevOperatorId;
 
         for (uint256 i; i < operatorsLength; ++i) {
             currentOperatorId = operatorIds[i];
@@ -186,16 +188,17 @@ library OperatorLib {
                 checkOwner(s.operators[currentOperatorId]);
             }
 
-            if (i > 0 && currentOperatorId <= operatorIds[i - 1]) {
-                if (currentOperatorId == operatorIds[i - 1]) {
+            if (i > 0 && currentOperatorId <= prevOperatorId) {
+                if (currentOperatorId == prevOperatorId) {
                     revert ISSVNetworkCore.OperatorsListNotUnique();
                 }
                 revert ISSVNetworkCore.UnsortedOperatorsList();
             }
 
-            (blockIndex, bitPosition) = getBitmapIndexes(currentOperatorId);
+            (uint256 blockIndex, uint256 bitPosition) = getBitmapIndexes(currentOperatorId);
 
-            masks[blockIndex] |= (1 << bitPosition);
+            masks[blockIndex - startBlockIndex] |= (1 << bitPosition);
+            prevOperatorId = currentOperatorId;
         }
     }
 

contracts/modules/SSVOperatorsWhitelist.sol

@@ -16,7 +16,7 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist {
     /* Operator External Functions */
     /*******************************/
 
-    function setOperatosWhitelists(
+    function setOperatorsWhitelists(
         uint64[] calldata operatorIds,
         address[] calldata whitelistAddresses
     ) external override {
@@ -55,8 +55,9 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist {
 
             // operator already whitelisted?
             // if EOA or generic contract, move it to SSV whitelisting module
-            if (!OperatorLib.isWhitelistingContract(currentWhitelisted)) {
+            if (currentWhitelisted != address(0) && !OperatorLib.isWhitelistingContract(currentWhitelisted)) {
                 (uint256 blockIndex, uint256 bitPosition) = OperatorLib.getBitmapIndexes(operatorId);
+
                 s.addressWhitelistedForOperators[currentWhitelisted][blockIndex] |= (1 << bitPosition);
             }
 

contracts/modules/SSVViews.sol

@@ -87,20 +87,19 @@ contract SSVViews is ISSVViews {
         uint256 internalCount;
 
         // Check whitelisting address for each operator using the internal SSV whitelisting module
-        uint256 whitelistedMask;
-        uint256 matchedMask;
-
-        uint256[] memory masks = OperatorLib.generateBlockMasks(operatorIds, false, s);
+        (uint256[] memory masks, uint256 startBlockIndex) = OperatorLib.generateBlockMasks(operatorIds, false, s);
         uint64[] memory internalWhitelistedOperatorIds = new uint64[](operatorsLength);
 
+        uint256 endBlockIndex = startBlockIndex + masks.length;
         // Check whitelisting status for each mask
-        for (uint256 blockIndex; blockIndex < masks.length; ++blockIndex) {
+        for (uint256 blockIndex = startBlockIndex; blockIndex < endBlockIndex; ++blockIndex) {
+            uint256 mask = masks[blockIndex - startBlockIndex];
             // Only check blocks that have operator IDs
-            if (masks[blockIndex] != 0) {
-                whitelistedMask = s.addressWhitelistedForOperators[addressToCheck][blockIndex];
+            if (mask != 0) {
+                uint256 whitelistedMask = s.addressWhitelistedForOperators[addressToCheck][blockIndex];
 
                 // This will give the matching whitelisted operators
-                matchedMask = whitelistedMask & masks[blockIndex];
+                uint256 matchedMask = whitelistedMask & mask;
 
                 // Now we need to extract operator IDs from matchedMask
                 uint256 blockPointer = blockIndex << 8;
@@ -122,11 +121,10 @@ contract SSVViews is ISSVViews {
 
         // Check if pending operators use an external whitelisting contract and check whitelistedAddress using it
         whitelistedOperatorIds = new uint64[](operatorsLength);
-        uint256 operatorIndex;
         uint256 internalWhitelistIndex;
         uint256 count;
 
-        while (operatorIndex < operatorsLength) {
+        for (uint256 operatorIndex; operatorIndex < operatorsLength; ++operatorIndex) {
             uint64 operatorId = operatorIds[operatorIndex];
 
             // Check if operatorId is already in internalWhitelistedOperatorIds
@@ -148,7 +146,6 @@ contract SSVViews is ISSVViews {
                     whitelistedOperatorIds[count++] = operatorId;
                 }
             }
-            ++operatorIndex;
         }
 
         // Resize whitelistedOperatorIds to the actual number of whitelisted operators

docs/operators.md

@@ -21,7 +21,7 @@ The restriction is only effective when the operator owner sets the privacy statu
 To manage the whitelisted addresses, these 2 data structures are used:
 
 `mapping(uint64 => address) operatorsWhitelist`: Keeps the relation between an operator and a whitelisting contract.
-`mapping(address => mapping(uint256 => uint256)) addressWhitelistedForOperators`: Links an address (EOA/generic contract) ot a list of operators identified by its `operatorId` using bitmaps.
+`mapping(address => mapping(uint256 => uint256)) addressWhitelistedForOperators`: Links an address (EOA/generic contract) to a list of operators identified by its `operatorId` using bitmaps.
 
 ### What is a Whitelisting Contract?
 The operators can choose to whitelist an external contract with custom logic to manage authorized addresses externally. To be used in SSV contracts, it needs to implement the [ISSVWhitelistingContract](../contracts/interfaces/external/ISSVWhitelistingContract.sol) interface, that requires to implement the `isWhitelisted(address account, uint256 operatorId)` function. This function is called in the register validator process, that must return `true/false` to indicate if the caller (`msg.sender`) is whitelisted for the operator.
@@ -35,11 +35,11 @@ To check if an account is whitelisted in a whitelisting contract, use the functi
 ### Legacy whitelisted addresses transition process
 Up until v1.1.1, operators use the `operatorsWhitelist` mapping to save EOAs and generic contracts. Now in v1.2.0, those type of addresses are stored in `addressWhitelistedForOperators`, leaving `operatorsWhitelist` to save only whitelisting contracts.
 When whitelisting a new whitelisting contract, the current address stored in `operatorsWhitelist` will be moved to `addressWhitelistedForOperators`, and the new address stored in `operatorsWhitelist`.
-When whitelising a new EOA/generic contract, it will be saved in `addressWhitelistedForOperators`, leaving the previous address in `operatorsWhitelist` intact.
+When whitelisting a new EOA/generic contract, it will be saved in `addressWhitelistedForOperators`, leaving the previous address in `operatorsWhitelist` intact.
 
 ### Operator whitelist states
 The following table shows all possible combinations of whitelisted addresses for a given operator.
-| Use legacy EOA/generic contract  | Use whitelising contract  | Use EOAs/generic contracts |
+| Use legacy EOA/generic contract  | Use whitelisting contract  | Use EOAs/generic contracts |
 |---|---|---|
 | Y  |   |   |
 | Y  |   | Y  |
@@ -57,7 +57,7 @@ Functions related to whitelisting contracts:
 - Remove: `SSVNetwork.removeOperatorsWhitelistingContract(uint64[] calldata operatorIds)`
 
 Functions related to EOAs/generic contracts:
-- Register multiple addresses to multiple operators: `SSVNetwork.setOperatosWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
+- Register multiple addresses to multiple operators: `SSVNetwork.setOperatorsWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
 - Remove multiple addresses for multiple operators: `SSVNetwork.removeOperatorsWhitelists(uint64[] calldata operatorIds, address[] calldata whitelistAddresses)`
 
 ### Registering validators using whitelisted operators
@@ -69,4 +69,7 @@ When registering validators using `SSVNetwork.registerValidator` or `SSVNetwork.
 
 If the caller is not authorized for any of the whitelisted operators, the transaction will revert with the `CallerNotWhitelistedWithData(<operatorId>)` error.
 
+**Important**: Changes to an operator's whitelist will not impact existing validators registered with that operator. Only new validator registrations will adhere to the updated whitelist rules.
+
+
 

docs/roles.md

@@ -27,7 +27,7 @@ The contract owner can perform operational actions over the contract and protoco
 Only the owner of an operator can execute these functions:
 
 - `SSVNetwork.removeOperator` - Removes an existing operator
-- `SSVNetwork.setOperatosWhitelists` - Sets a list of whitelisted addresses (EOAs or generic contracts) for a list of operators
+- `SSVNetwork.setOperatorsWhitelists` - Sets a list of whitelisted addresses (EOAs or generic contracts) for a list of operators
 - `SSVNetwork.removeOperatorsWhitelists` - Removes a list of whitelisted addresses (EOAs or generic contracts) for a list of operators
 - `SSVNetwork.setOperatorsWhitelistingContract` - Sets a whitelisting contract for a list of operators
 - `SSVNetwork.removeOperatorsWhitelistingContract` - Removes the whitelisting contract set for a list of operators

test-forked/operators-whitelist.ts

@@ -181,7 +181,7 @@ describe('Whitelisting Tests (fork) - Pre-upgrade SSV Core Contracts Tests', ()
     // get the current whitelisted address
     const prevWhitelistedAddress = (await ssvViews.read.getOperatorById([314]))[3];
 
-    await ssvNetwork.write.setOperatosWhitelists([[315, 316, 317], [owners[2].account.address]], {
+    await ssvNetwork.write.setOperatorsWhitelists([[315, 316, 317], [owners[2].account.address]], {
       account: { address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' },
     });
 
@@ -302,7 +302,7 @@ describe('Whitelisting Tests (fork) - Ongoing SSV Core Contracts upgrade Tests',
     await upgradeAllContracts();
 
     // whitelist a different operator using SSV whitelisting module
-    await ssvNetwork.write.setOperatosWhitelists([[315n], ['0xB4084F25DfCb2c1bf6636b420b59eda807953769']], {
+    await ssvNetwork.write.setOperatorsWhitelists([[315n], ['0xB4084F25DfCb2c1bf6636b420b59eda807953769']], {
       account: { address: '0xB4084F25DfCb2c1bf6636b420b59eda807953769' },
     });
 

test/helpers/gas-usage.ts

@@ -98,13 +98,13 @@ const MAX_GAS_PER_GROUP: any = {
   [GasGroup.REGISTER_OPERATOR]: 137000,
   [GasGroup.REMOVE_OPERATOR]: 70500,
   [GasGroup.REMOVE_OPERATOR_WITH_WITHDRAW]: 70500,
-  [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT]: 95500,
+  [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT]: 70000,
   [GasGroup.UPDATE_OPERATOR_WHITELISTING_CONTRACT]: 70000,
   [GasGroup.SET_OPERATOR_WHITELISTING_CONTRACT_10]: 375000,
   [GasGroup.REMOVE_OPERATOR_WHITELISTING_CONTRACT]: 43000,
   [GasGroup.REMOVE_OPERATOR_WHITELISTING_CONTRACT_10]: 130000,
-  [GasGroup.SET_MULTIPLE_OPERATOR_WHITELIST_10_10]: 382000,
-  [GasGroup.REMOVE_MULTIPLE_OPERATOR_WHITELIST_10_10]: 169000,
+  [GasGroup.SET_MULTIPLE_OPERATOR_WHITELIST_10_10]: 381000,
+  [GasGroup.REMOVE_MULTIPLE_OPERATOR_WHITELIST_10_10]: 168000,
   [GasGroup.SET_OPERATORS_PRIVATE_10]: 313000,
   [GasGroup.SET_OPERATORS_PUBLIC_10]: 114000,
 

test/operators/remove.ts

@@ -50,7 +50,7 @@ describe('Remove Operator Tests', () => {
     );
     const { operatorId } = result.eventsByName.OperatorAdded[0].args;
 
-    await ssvNetwork.write.setOperatosWhitelists([[operatorId], [owners[2].account.address]]);
+    await ssvNetwork.write.setOperatorsWhitelists([[operatorId], [owners[2].account.address]]);
 
     await assertEvent(ssvNetwork.write.removeOperator([operatorId]), [
       {