add operator whitelisting tests

contracts/SSVNetwork.sol

@@ -9,6 +9,8 @@ import "./interfaces/ISSVOperatorsWhitelist.sol";
 import "./interfaces/ISSVDAO.sol";
 import "./interfaces/ISSVViews.sol";
 
+import "./interfaces/external/ISSVWhitelistingContract.sol";
+
 import "./libraries/Types.sol";
 import "./libraries/CoreLib.sol";
 import "./libraries/SSVStorage.sol";

contracts/SSVNetworkViews.sol

@@ -67,6 +67,9 @@ contract SSVNetworkViews is UUPSUpgradeable, Ownable2StepUpgradeable, ISSVViews
     ) external view override returns (uint64[] memory whitelistedOperatorIds) {
         return ssvNetwork.getWhitelistedOperators(operatorIds, whitelistedAddress);
     }
+    function isWhitelistingContract(address contractAddress) external view returns (bool isWhitelistingContract) {
+        return ssvNetwork.isWhitelistingContract(contractAddress);
+    }
 
     /***********************************/
     /* Cluster External View Functions */

contracts/interfaces/ISSVNetworkCore.sol

@@ -92,7 +92,9 @@ interface ISSVNetworkCore {
     error EmptyPublicKeysList(); // df83e679
     error InvalidContractAddress(); // 0xa710429d
     error AddressIsWhitelistingContract(address contractAddress); // 0x71cadba7
+    error InvalidWhitelistingContract(); // 0x81ed29ff
     error InvalidWhitelistAddressesLength(); // 0xcbb362dc
+    error ZeroAddressNotAllowed(); // 0x8579befe
 
     // legacy errors
     error ValidatorAlreadyExists(); // 0x8d09a73e

contracts/interfaces/ISSVOperatorsWhitelist.sol

@@ -41,28 +41,28 @@ interface ISSVOperatorsWhitelist is ISSVNetworkCore {
     /**
      * @dev Emitted when the whitelist of an operator is updated.
      * @param operatorId operator's ID.
-     * @param whitelisted operator's new whitelisted address.
+     * @param whitelistAddress operator's new whitelisted address.
      */
-    event OperatorWhitelistUpdated(uint64 indexed operatorId, address whitelisted);
+    event OperatorWhitelistUpdated(uint64 indexed operatorId, address whitelistAddress);
 
     /**
      * @dev Emitted when a list of adresses are whitelisted for a set of operators.
      * @param operatorIds operators' IDs.
      * @param whitelistAddresses operators' new whitelist addresses (EOAs or generic contracts).
      */
-    event OperatorMultipleWhitelistUpdated(uint64[] indexed operatorIds, address[] whitelistAddresses);
+    event OperatorMultipleWhitelistUpdated(uint64[] operatorIds, address[] whitelistAddresses);
 
     /**
      * @dev Emitted when a list of adresses are de-whitelisted for a set of operators.
      * @param operatorIds operators' IDs.
      * @param whitelistAddresses operators' list of whitelist addresses to be removed (EOAs or generic contracts).
      */
-    event OperatorMultipleWhitelistRemoved(uint64[] indexed operatorIds, address[] whitelistAddresses);
+    event OperatorMultipleWhitelistRemoved(uint64[] operatorIds, address[] whitelistAddresses);
 
     /**
      * @dev Emitted when the whitelisting contract of an operator is updated.
      * @param operatorIds operators' IDs.
      * @param whitelistingContract operators' new whitelisting contract address.
      */
-    event OperatorWhitelistingContractUpdated(uint64[] indexed operatorIds, address whitelistingContract);
+    event OperatorWhitelistingContractUpdated(uint64[] operatorIds, address whitelistingContract);
 }

contracts/interfaces/ISSVViews.sol

@@ -31,7 +31,7 @@ interface ISSVViews is ISSVNetworkCore {
     /// @return fee The fee associated with the operator (SSV)
     /// @return validatorCount The count of validators associated with the operator
     /// @return whitelistedContract The whitelisted contract address of the operator, if any
-    /// @return useWhitelistedContract A boolean indicating if the operator uses a whitelisted contract
+    /// @return isPrivate A boolean indicating if the operator is private (uses whitelisting contract or SSV Whitelisting module)
     /// @return active A boolean indicating if the operator is active
     function getOperatorById(
         uint64 operatorId
@@ -43,7 +43,7 @@ interface ISSVViews is ISSVNetworkCore {
             uint256 fee,
             uint32 validatorCount,
             address whitelistedContract,
-            bool useWhitelistedContract,
+            bool isPrivate,
             bool active
         );
 
@@ -56,6 +56,11 @@ interface ISSVViews is ISSVNetworkCore {
         address whitelistedAddress
     ) external view returns (uint64[] memory whitelistedOperatorIds);
 
+    /// @notice Checks if the given address is a whitelisting contract (implements ISSVWhitelistingContract)
+    /// @param contractAddress The address to check
+    /// @return isWhitelistingContract A boolean indicating if the address is a whitelisting contract
+    function isWhitelistingContract(address contractAddress) external view returns (bool isWhitelistingContract);
+
     /// @notice Checks if the cluster can be liquidated
     /// @param owner The owner address of the cluster
     /// @param operatorIds The IDs of the operators in the cluster

contracts/interfaces/external/ISSVWhitelistingContract.sol

@@ -5,5 +5,5 @@ interface ISSVWhitelistingContract {
     /// @notice Checks if the caller is whitelisted
     /// @param account The account that is being checked for whitelisting
     /// @param operatorId The SSV Operator Id which is being checked
-    function isWhitelisted(address account, uint64 operatorId) external view returns (bool);
+    function isWhitelisted(address account, uint256 operatorId) external view returns (bool);
 }

contracts/libraries/OperatorLib.sol

@@ -151,32 +151,40 @@ library OperatorLib {
         if (addressesLength == 0) revert ISSVNetworkCore.InvalidWhitelistAddressesLength();
         if (operatorsLength == 0) revert ISSVNetworkCore.InvalidOperatorIdsLength();
 
+        ISSVNetworkCore.Operator storage operator;
+        for (uint256 i; i < operatorsLength; ++i) {
+            operator = s.operators[operatorIds[i]];
+
+            checkOwner(operator);
+            if (addAddresses) {
+                if (!operator.whitelisted) {
+                    operator.whitelisted = true;
+                }
+            }
+        }
+
         // create the max number of masks that will be updated
         uint256[] memory masks = generateBlockMasks(operatorIds);
 
-        for (uint256 i = 0; i < addressesLength; ++i) {
-            address addr = whitelistAddresses[i];
+        for (uint256 i; i < addressesLength; ++i) {
+            address whitelistAddress = whitelistAddresses[i];
+            checkZeroAddress(whitelistAddress);
 
-            if (isWhitelistingContract(addr)) revert ISSVNetworkCore.AddressIsWhitelistingContract(addr);
+            // If whitelistAddress is a custom contract, revert also when removing
+            if (isWhitelistingContract(whitelistAddress))
+                revert ISSVNetworkCore.AddressIsWhitelistingContract(whitelistAddress);
 
             for (uint256 blockIndex; blockIndex < masks.length; ++blockIndex) {
                 // only update storage for updated masks
                 if (masks[blockIndex] != 0) {
                     if (addAddresses) {
-                        s.addressWhitelistedForOperators[addr][blockIndex] |= masks[blockIndex];
+                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] |= masks[blockIndex];
                     } else {
-                        s.addressWhitelistedForOperators[addr][blockIndex] &= ~masks[blockIndex];
+                        s.addressWhitelistedForOperators[whitelistAddress][blockIndex] &= ~masks[blockIndex];
                     }
                 }
             }
         }
-
-        // TODO see the way to not iterate over operatorIds twice
-        for (uint256 i = 0; i < operatorsLength; ++i) {
-            if (!s.operators[operatorIds[i]].whitelisted) {
-                s.operators[operatorIds[i]].whitelisted = true;
-            }
-        }
     }
 
     function updateWhitelistingContract(
@@ -200,29 +208,42 @@ library OperatorLib {
         s.operatorsWhitelist[operatorId] = address(whitelistingContract);
     }
 
-    function getBitmapIndexes(uint64 operatorId) internal pure returns (uint256 blockIndex, uint256 bitPosition) {
-        blockIndex = operatorId >> 8; // Equivalent to operatorId / 256
-        bitPosition = operatorId & 0xFF; // Equivalent to operatorId % 256
-    }
-
-    function isWhitelistingContract(address whitelistingContract) internal view returns (bool) {
-        // TODO create type for whitelisting contracts?
-        return ERC165Checker.supportsInterface(whitelistingContract, type(ISSVWhitelistingContract).interfaceId);
-    }
-
     function generateBlockMasks(uint64[] calldata operatorIds) internal pure returns (uint256[] memory masks) {
         uint256 blockIndex;
         uint256 bitPosition;
+        uint64 currentOperatorId;
 
         uint256 operatorsLength = operatorIds.length;
 
         // create the max number of masks that will be updated
         masks = new uint256[]((operatorIds[operatorsLength - 1] >> 8) + 1);
 
-        for (uint256 i = 0; i < operatorsLength; ++i) {
-            (blockIndex, bitPosition) = getBitmapIndexes(operatorIds[i]);
+        for (uint256 i; i < operatorsLength; ++i) {
+            currentOperatorId = operatorIds[i];
+
+            if (i > 0 && currentOperatorId <= operatorIds[i - 1]) {
+                if (currentOperatorId == operatorIds[i - 1]) {
+                    revert ISSVNetworkCore.OperatorsListNotUnique();
+                }
+                revert ISSVNetworkCore.UnsortedOperatorsList();
+            }
+
+            (blockIndex, bitPosition) = getBitmapIndexes(currentOperatorId);
 
             masks[blockIndex] |= (1 << bitPosition);
         }
     }
+
+    function getBitmapIndexes(uint64 operatorId) internal pure returns (uint256 blockIndex, uint256 bitPosition) {
+        blockIndex = operatorId >> 8; // Equivalent to operatorId / 256
+        bitPosition = operatorId & 0xFF; // Equivalent to operatorId % 256
+    }
+
+    function checkZeroAddress(address whitelistAddress) internal pure {
+        if (whitelistAddress == address(0)) revert ISSVNetworkCore.ZeroAddressNotAllowed();
+    }
+
+    function isWhitelistingContract(address whitelistingContract) internal view returns (bool) {
+        return ERC165Checker.supportsInterface(whitelistingContract, type(ISSVWhitelistingContract).interfaceId);
+    }
 }

contracts/modules/SSVOperators.sol

@@ -52,6 +52,7 @@ contract SSVOperators is ISSVOperators {
 
     function removeOperator(uint64 operatorId) external override {
         StorageData storage s = SSVStorage.load();
+
         Operator memory operator = s.operators[operatorId];
         operator.checkOwner();
 
@@ -62,6 +63,7 @@ contract SSVOperators is ISSVOperators {
         operator.snapshot.balance = 0;
         operator.validatorCount = 0;
         operator.fee = 0;
+        operator.whitelisted = false;
 
         s.operators[operatorId] = operator;
 

contracts/modules/SSVOperatorsWhitelist.sol

@@ -17,12 +17,14 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist {
     /*******************************/
 
     function setOperatorWhitelist(uint64 operatorId, address whitelistAddress) external override {
-        StorageData storage s = SSVStorage.load();
-        s.operators[operatorId].checkOwner();
+        OperatorLib.checkZeroAddress(whitelistAddress);
 
         if (OperatorLib.isWhitelistingContract(whitelistAddress))
             revert AddressIsWhitelistingContract(whitelistAddress);
 
+        StorageData storage s = SSVStorage.load();
+        s.operators[operatorId].checkOwner();
+
         // Set the bit at bitPosition for the operatorId in the corresponding uint256 blockIndex
         (uint256 blockIndex, uint256 bitPosition) = OperatorLib.getBitmapIndexes(operatorId);
 
@@ -53,13 +55,16 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist {
         uint64[] calldata operatorIds,
         ISSVWhitelistingContract whitelistingContract
     ) external {
+        // Reverts also when whitelistingContract == address(0)
+        if (!OperatorLib.isWhitelistingContract(address(whitelistingContract))) revert InvalidWhitelistingContract();
+
         uint256 operatorsLength = operatorIds.length;
         if (operatorsLength == 0) revert InvalidOperatorIdsLength();
 
         StorageData storage s = SSVStorage.load();
         Operator storage operator;
 
-        for (uint256 i = 0; i < operatorsLength; ++i) {
+        for (uint256 i; i < operatorsLength; ++i) {
             uint64 operatorId = operatorIds[i];
 
             operator = s.operators[operatorId];
@@ -89,12 +94,11 @@ contract SSVOperatorsWhitelist is ISSVOperatorsWhitelist {
         StorageData storage s = SSVStorage.load();
         Operator storage operator;
 
-        for (uint256 i = 0; i < operatorsLength; ++i) {
+        for (uint256 i; i < operatorsLength; ++i) {
             uint64 operatorId = operatorIds[i];
             operator = s.operators[operatorId];
 
             operator.checkOwner();
-            operator.whitelisted = false;
 
             s.operatorsWhitelist[operatorId] = address(0);
         }

contracts/modules/SSVViews.sol

@@ -51,21 +51,27 @@ contract SSVViews is ISSVViews {
 
     function getOperatorById(
         uint64 operatorId
-    ) external view override returns (address, uint256, uint32, address, bool, bool) {
-        ISSVNetworkCore.Operator memory operator = SSVStorage.load().operators[operatorId];
-
-        address whitelistedContract = SSVStorage.load().operatorsWhitelist[operatorId];
-        bool useWhitelistedContract = OperatorLib.isWhitelistingContract(whitelistedContract);
-        bool isActive = operator.snapshot.block == 0 ? false : true;
-
-        return (
-            operator.owner,
-            operator.fee.expand(),
-            operator.validatorCount,
-            whitelistedContract,
-            useWhitelistedContract,
-            isActive
-        );
+    )
+        external
+        view
+        override
+        returns (
+            address owner,
+            uint256 fee,
+            uint32 validatorCount,
+            address whitelistedContract,
+            bool isPrivate,
+            bool isActive
+        )
+    {
+        ISSVNetworkCore.Operator storage operator = SSVStorage.load().operators[operatorId];
+
+        owner = operator.owner;
+        fee = operator.fee.expand();
+        validatorCount = operator.validatorCount;
+        whitelistedContract = SSVStorage.load().operatorsWhitelist[operatorId];
+        isPrivate = operator.whitelisted;
+        isActive = operator.snapshot.block != 0;
     }
 
     function getWhitelistedOperators(
@@ -80,7 +86,7 @@ contract SSVViews is ISSVViews {
         // create the max number of masks that will be updated
         uint256[] memory masks = OperatorLib.generateBlockMasks(operatorIds);
 
-        uint256 count = 0;
+        uint256 count;
         whitelistedOperatorIds = new uint64[](operatorsLength);
 
         uint256 whitelistedMask;
@@ -97,7 +103,7 @@ contract SSVViews is ISSVViews {
 
                 // Now we need to extract operator IDs from matchedMask
                 uint256 blockPointer = blockIndex << 8;
-                for (uint256 bit = 0; bit < 256; bit++) {
+                for (uint256 bit; bit < 256; ++bit) {
                     if (matchedMask & (1 << bit) != 0) {
                         whitelistedOperatorIds[count++] = uint64(blockPointer + bit);
                         if (count == operatorsLength) {
@@ -114,6 +120,10 @@ contract SSVViews is ISSVViews {
         }
     }
 
+    function isWhitelistingContract(address contractAddress) external view override returns (bool) {
+        return OperatorLib.isWhitelistingContract(contractAddress);
+    }
+
     /***********************************/
     /* Cluster External View Functions */
     /***********************************/

contracts/test/mocks/MockWhitelistingContract.sol

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.18;
+
+import "../../interfaces/external/ISSVWhitelistingContract.sol";
+import "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+
+contract MockWhitelistingContract is ISSVWhitelistingContract, ERC165 {
+    mapping(address => bool) private whitelisted;
+
+    constructor(address[] memory whitelistedAddresses) {
+        for (uint i; i < whitelistedAddresses.length; ++i) {
+            whitelisted[whitelistedAddresses[i]] = true;
+        }
+    }
+
+    function isWhitelisted(address account, uint256) external view override returns (bool) {
+        return whitelisted[account];
+    }
+
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
+        return interfaceId == type(ISSVWhitelistingContract).interfaceId || super.supportsInterface(interfaceId);
+    }
+}

test/helpers/contract-helpers.ts

@@ -101,6 +101,7 @@ export const initializeContract = async function () {
   const ssvClustersMod = await hre.viem.deployContract('SSVClusters');
   const ssvDAOMod = await hre.viem.deployContract('SSVDAO');
   const ssvViewsMod = await hre.viem.deployContract('contracts/modules/SSVViews.sol:SSVViews');
+  const ssvWhitelistMod = await hre.viem.deployContract('SSVOperatorsWhitelist');
 
   const ssvNetworkFactory = await ethers.getContractFactory('SSVNetwork');
   const ssvNetworkProxy = await await upgrades.deployProxy(
@@ -138,6 +139,8 @@ export const initializeContract = async function () {
 
   await ssvNetwork.write.updateMaximumOperatorFee([CONFIG.maximumOperatorFee as bigint]);
 
+  ssvNetwork.write.updateModule([4, await ssvWhitelistMod.address]);
+
   for (let i = 1; i < 7; i++) {
     await ssvToken.write.mint([owners[i].account.address, 10000000000000000000n]);
   }