Audit cyfrin

src/contracts/DelegatorFactory.sol

@@ -94,6 +94,10 @@ contract DelegatorFactory is IDelegatorFactory, Ownable, ERC165 {
      * @inheritdoc IDelegatorFactory
      */
     function create(uint64 type_, bytes calldata data) external returns (address entity_) {
+        if (blacklisted[type_]) {
+            revert DelegatorFactory__VersionBlacklisted();
+        }
+
         entity_ = implementation(type_).cloneDeterministic(keccak256(abi.encode(totalEntities(), type_, data)));
 
         _addDelegatorEntity(entity_);

src/contracts/VaultFactory.sol

@@ -151,6 +151,10 @@ contract VaultFactory is Ownable, IMigratablesFactory {
             revert MigratableFactory__OldVersion();
         }
 
+        if (blacklisted[newVersion]) {
+            revert MigratableFactory__VersionBlacklisted();
+        }
+
         IMigratableEntityProxy(entity_).upgradeToAndCall(
             implementation(newVersion), abi.encodeCall(IVaultTokenized.migrate, (newVersion, data))
         );

src/contracts/libraries/Checkpoints.sol

@@ -285,11 +285,11 @@ library ExtendedCheckpoints {
         uint32 hint = abi.decode(hint_, (uint32));
         Checkpoint256 memory checkpoint = at(self, hint);
         if (checkpoint._key == key) {
-            return (true, checkpoint._key, self._values[checkpoint._value], hint);
+            return (true, checkpoint._key, checkpoint._value, hint);
         }
 
         if (checkpoint._key < key && (hint == length(self) - 1 || at(self, hint + 1)._key > key)) {
-            return (true, checkpoint._key, self._values[checkpoint._value], hint);
+            return (true, checkpoint._key, checkpoint._value, hint);
         }
 
         return upperLookupRecentCheckpoint(self, key);

src/contracts/middleware/AvalancheL1Middleware.sol

@@ -61,10 +61,10 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
 
     uint96 public constant PRIMARY_ASSET_CLASS = 1;
     uint48 public constant MAX_AUTO_EPOCH_UPDATES = 1;
-
     MiddlewareVaultManager private vaultManager;
     EnumerableMap.AddressToUintMap private operators;
     EnumerableSet.UintSet private secondaryAssetClasses;
+    bool private vaultManagerSet;
 
     BalancerValidatorManager public balancerValidatorManager;
 
@@ -77,6 +77,7 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
     mapping(bytes32 => bool) public nodePendingRemoval;
     mapping(address => uint256) public operatorLockedStake;
     mapping(uint48 => mapping(uint96 => bool)) public totalStakeCached;
+    mapping(bytes32 => address) public validationIdToOperator;
     // operatorNodesArray[operator] is used for iteration during certain
     // rebalancing or node-update operations, and has nodes removed once
     // they are effectively retired. This means a node can remain in
@@ -191,9 +192,13 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
     function setVaultManager(
         address vaultManager_
     ) external onlyOwner {
+        if (vaultManagerSet) {
+            revert AvalancheL1Middleware__VaultManagerAlreadySet(address(vaultManager));
+        }
         if (vaultManager_ == address(0)) {
             revert AvalancheL1Middleware__ZeroAddress("vaultManager");
         }
+        vaultManagerSet = true;
         emit VaultManagerUpdated(address(vaultManager), vaultManager_);
         vaultManager = MiddlewareVaultManager(vaultManager_);
     }
@@ -291,6 +296,9 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
     function disableOperator(
         address operator
     ) external onlyOwner updateGlobalNodeStakeOncePerEpoch {
+        if (operatorNodesArray[operator].length > 0) {
+            revert AvalancheL1Middleware__OperatorHasActiveNodes(operator, operatorNodesArray[operator].length);
+        }
         operators.disable(operator);
     }
 
@@ -309,6 +317,9 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
     function removeOperator(
         address operator
     ) external onlyOwner updateGlobalNodeStakeOncePerEpoch {
+        if (operatorNodesArray[operator].length > 0) {
+            revert AvalancheL1Middleware__OperatorHasActiveNodes(operator, operatorNodesArray[operator].length);
+        }
         (, uint48 disabledTime) = operators.getTimes(operator);
         if (disabledTime == 0 || disabledTime + SLASHING_WINDOW > Time.timestamp()) {
             revert AvalancheL1Middleware__OperatorGracePeriodNotPassed(disabledTime, SLASHING_WINDOW);
@@ -366,6 +377,8 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
         bytes32 validationID = balancerValidatorManager.initializeValidatorRegistration(
             input, StakeConversion.stakeToWeight(newStake, WEIGHT_SCALE_FACTOR)
         );
+
+        validationIdToOperator[validationID] = operator;
         nodeStakeCache[epoch][validationID] = newStake;
         nodeStakeCache[epoch + 1][validationID] = newStake;
         nodePendingUpdate[validationID] = true;
@@ -395,7 +408,6 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
         if (rebalancedThisEpoch[operator][currentEpoch]) {
             revert AvalancheL1Middleware__AlreadyRebalanced(operator, currentEpoch);
         }
-        rebalancedThisEpoch[operator][currentEpoch] = true;
 
         if (!operators.contains(operator)) {
             revert AvalancheL1Middleware__OperatorNotRegistered(operator);
@@ -423,6 +435,20 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
         // We only handle the scenario newTotalStake < registeredStake, when removing stake
         leftoverStake = registeredStake - newTotalStake;
 
+        // The minimum stake that results in a weight change of at least 1
+        uint256 minMeaningfulStake = WEIGHT_SCALE_FACTOR;
+
+        if (leftoverStake < minMeaningfulStake) {
+            emit AllNodeStakesUpdated(operator, newTotalStake);
+            return;
+        }
+        // If limitStake is provided, ensure it's at least the minimum meaningful amount
+        if (limitStake > 0 && limitStake < minMeaningfulStake) {
+            revert AvalancheL1Middleware__LimitStakeTooLow(limitStake, minMeaningfulStake);
+        }
+
+        bool hasUpdatedAnyNode = false;
+
         for (uint256 i = length; i > 0 && leftoverStake > 0;) {
             i--;
             bytes32 nodeId = nodesArr[i];
@@ -445,8 +471,23 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
             if (limitStake > 0 && stakeToRemove > limitStake) {
                 stakeToRemove = limitStake;
             }
+
+            if (stakeToRemove < minMeaningfulStake) {
+                continue;
+            }
+
             uint256 newStake = previousStake - stakeToRemove;
+            uint64 oldWeight = StakeConversion.stakeToWeight(previousStake, WEIGHT_SCALE_FACTOR);
+            uint64 newWeight = StakeConversion.stakeToWeight(newStake, WEIGHT_SCALE_FACTOR);
+
+            // Skip this node if the weight wouldn't change (unless we're removing all stake)
+            if (oldWeight == newWeight && newStake > 0) {
+                continue;
+            }
+
             leftoverStake -= stakeToRemove;
+            hasUpdatedAnyNode = true;
+
 
             if (
                 (newStake < assetClasses[PRIMARY_ASSET_CLASS].minValidatorStake)
@@ -460,7 +501,14 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
             }
         }
 
-        // Finally emit updated stake
+        if (!hasUpdatedAnyNode && leftoverStake >= minMeaningfulStake) {
+            revert AvalancheL1Middleware__NoMeaningfulUpdatesAvailable(operator, leftoverStake);
+        }
+
+        if (hasUpdatedAnyNode) {
+            rebalancedThisEpoch[operator][currentEpoch] = true;
+        }
+
         emit AllNodeStakesUpdated(operator, newTotalStake);
     }
 
@@ -534,6 +582,10 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
      * @inheritdoc IAvalancheL1Middleware
      */
     function calcAndCacheStakes(uint48 epoch, uint96 assetClassId) public returns (uint256 totalStake) {
+        if (epoch > getCurrentEpoch()) {
+            revert AvalancheL1Middleware__CannotCacheFutureEpoch(epoch);
+        }
+
         uint48 epochStartTs = getEpochStartTs(epoch);
 
         uint256 length = operators.length();
@@ -655,7 +707,7 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
             bytes32 nodeId = nodeArray[i];
             bytes32 valID = balancerValidatorManager.registeredValidators(abi.encodePacked(uint160(uint256(nodeId))));
 
-            // If no removal/update, just carry over from prevEpoch (only if we haven’t set it yet)
+            // If no removal/update, just carry over from prevEpoch (only if we haven't set it yet)
             if (!nodePendingRemoval[valID] && !nodePendingUpdate[valID]) {
                 if (nodeStakeCache[epoch][valID] == 0) {
                     nodeStakeCache[epoch][valID] = nodeStakeCache[prevEpoch][valID];
@@ -795,25 +847,45 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
         balancerValidatorManager.initializeValidatorWeightUpdate(validationID, scaledWeight);
     }
 
-    function _requireMinSecondaryAssetClasses(uint256 extraNode, address operator) internal view returns (bool) {
+    function _requireMinSecondaryAssetClasses(uint256 extraNode, address operator) internal returns (bool) {
         uint48 epoch = getCurrentEpoch();
-        uint256 nodeCount = operatorNodesArray[operator].length; // existing nodes
-
+
+        // active nodes now excludes those already pending removal
+        uint256 nodeCount = _getActiveNodeCount(operator) + extraNode;
+        if (nodeCount == 0) return false;         // no active nodes ⇒ fail fast
+
         uint256 secCount = secondaryAssetClasses.length();
-        if (secCount == 0) {
-            return true;
-        }
-        for (uint256 i = 0; i < secCount; i++) {
+        if (secCount == 0) return true;           // nothing to check
+
+        for (uint256 i = 0; i < secCount; ++i) {
             uint256 classId = secondaryAssetClasses.at(i);
-            uint256 stake = getOperatorStake(operator, epoch, uint96(classId));
+            uint256 stake   = getOperatorStake(operator, epoch, uint96(classId));
             // Check ratio vs. class's min stake, could add an emit here to debug
-            if (stake / (nodeCount + extraNode) < assetClasses[classId].minValidatorStake) {
+            if (stake / nodeCount < assetClasses[classId].minValidatorStake) {
+                emit DebugSecondaryAssetClassCheck(operator, classId, stake, nodeCount, assetClasses[classId].minValidatorStake);
                 return false;
             }
         }
         return true;
     }
 
+    /**
+     * @dev Returns active (non-pending-removal) node count for an operator
+     * @param operator The operator address
+     * @return count The number of active nodes
+     */
+    function _getActiveNodeCount(address operator) internal view returns (uint256 count) {
+        bytes32[] storage arr = operatorNodesArray[operator];
+        for (uint256 i; i < arr.length; ++i) {
+            bytes32 valID = balancerValidatorManager.registeredValidators(
+                abi.encodePacked(uint160(uint256(arr[i])))
+            );
+            if (!nodePendingRemoval[valID]) {
+                unchecked { ++count; }
+            }
+        }
+    }
+
     /**
      * @notice Checks if the classId is active
      * @param assetClassId The asset class ID
@@ -993,6 +1065,11 @@ contract AvalancheL1Middleware is IAvalancheL1Middleware, AssetClassRegistry {
                 balancerValidatorManager.registeredValidators(abi.encodePacked(uint160(uint256(nodeId))));
             Validator memory validator = balancerValidatorManager.getValidator(validationID);
 
+            // Skip if no validator is registered for this nodeId
+            if (validationID == bytes32(0) || validationIdToOperator[validationID] != operator) {
+                continue;
+            }
+
             if (_wasActiveAt(uint48(validator.startedAt), uint48(validator.endedAt), epochStartTs)) {
                 temp[activeCount++] = nodeId;
             }

src/contracts/vault/VaultTokenized.sol

@@ -167,6 +167,19 @@ contract VaultTokenized is
                     revert Vault__InconsistentRoles();
                 }
             }
+
+            if (params.depositWhitelist && 
+                params.depositWhitelistSetRoleHolder != address(0) && 
+                params.depositorWhitelistRoleHolder == address(0)) {
+                revert Vault__InconsistentRoles();
+            }
+
+            if (params.isDepositLimit && 
+                params.depositLimit == 0 && 
+                params.isDepositLimitSetRoleHolder != address(0) && 
+                params.depositLimitSetRoleHolder == address(0)) {
+                revert Vault__InconsistentRoles();
+            }
         }
 
         vs.collateral = params.collateral;

src/interfaces/IDelegatorFactory.sol

@@ -15,6 +15,7 @@ interface IDelegatorFactory is IERC165 {
     error DelegatorFactory__AlreadyWhitelisted();
     error DelegatorFactory__InvalidImplementation();
     error DelegatorFactory__InvalidType();
+    error DelegatorFactory__VersionBlacklisted();
 
     /**
      * @notice Emitted when an entity is added.

src/interfaces/middleware/IAvalancheL1Middleware.sol

@@ -50,6 +50,11 @@ interface IAvalancheL1Middleware {
     error AvalancheL1Middleware__ManualEpochUpdateRequired(uint48 epochsPending, uint48 maxAutoUpdates);
     error AvalancheL1Middleware__NoEpochsToProcess();
     error AvalancheL1Middleware__TooManyEpochsRequested(uint48 requested, uint48 pending);
+    error AvalancheL1Middleware__LimitStakeTooLow(uint256 limitStake, uint256 minMeaningfulStake);
+    error AvalancheL1Middleware__NoMeaningfulUpdatesAvailable(address operator, uint256 leftoverStake);
+    error AvalancheL1Middleware__CannotCacheFutureEpoch(uint48 epoch);
+    error AvalancheL1Middleware__VaultManagerAlreadySet(address vaultManager);
+    error AvalancheL1Middleware__OperatorHasActiveNodes(address operator, uint256 nodeCount);
 
     // Events
     /**
@@ -109,6 +114,16 @@ interface IAvalancheL1Middleware {
      */
     event NodeStakeCacheManuallyProcessed(uint48 upToEpoch, uint48 epochsProcessedCount);
 
+    /**
+     * @notice Emitted when the secondary asset class check is performed
+     * @param operator The operator
+     * @param classId The asset class ID
+     * @param stake The stake
+     * @param nodeCount The number of nodes
+     * @param minValidatorStake The minimum validator stake
+     */
+    event DebugSecondaryAssetClassCheck(address indexed operator, uint256 classId, uint256 stake, uint256 nodeCount, uint256 minValidatorStake);
+
     /**
      * @dev Simple struct to return operator stake and key.
      */

test/DelegatorFactoryTest.t.sol

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: MIT
+// SPDX-FileCopyrightText: Copyright 2024 ADDPHO
+
+pragma solidity 0.8.25;
+
+import {Test, console2} from "forge-std/Test.sol";
+import {DelegatorFactory} from "../src/contracts/DelegatorFactory.sol";
+import {IDelegatorFactory} from "../src/interfaces/IDelegatorFactory.sol";
+import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
+import {IEntity} from "../src/interfaces/common/IEntity.sol";
+import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+import "@openzeppelin/contracts/utils/introspection/IERC165.sol";
+
+contract DelegatorFactoryTest is Test {
+    address owner;
+    address operator1;
+    address operator2;
+
+    DelegatorFactory factory;
+    MockEntity mockImpl;
+
+    function setUp() public {
+        owner = address(this);
+        operator1 = makeAddr("operator1");
+        operator2 = makeAddr("operator2");
+
+        factory = new DelegatorFactory(owner);
+
+        // Deploy a mock implementation that conforms to IEntity
+        mockImpl = new MockEntity(address(factory), 0);
+
+        // Whitelist the implementation
+        factory.whitelist(address(mockImpl));
+    }
+
+    function testCreateBeforeBlacklist() public {
+        bytes memory initData = abi.encode("test");
+
+        address created = factory.create(0, initData);
+
+        assertTrue(factory.isEntity(created), "Entity should be created and registered");
+    }
+
+    // function testCreateFailsAfterBlacklist() public {
+    //     bytes memory initData = abi.encode("test");
+
+    //     factory.blacklist(0);
+
+    //     factory.create(0, initData); //@note no revert although blacklisted
+    // }
+
+    function testCreateFailsAfterBlacklistFix() public {
+        bytes memory initData = abi.encode("test");
+
+        factory.blacklist(0);
+
+        vm.expectRevert(abi.encodeWithSignature("DelegatorFactory__VersionBlacklisted()"));
+        factory.create(0, initData);
+    }
+}
+
+
+contract MockEntity is IEntity, ERC165 {
+    address public immutable FACTORY;
+    uint64 public immutable TYPE;
+
+    string public data;
+
+    constructor(address factory_, uint64 type_) {
+        FACTORY = factory_;
+        TYPE = type_;
+    }
+
+    function initialize(
+        bytes calldata initData
+    ) external {
+        data = abi.decode(initData, (string));
+    }
+
+    function supportsInterface(
+        bytes4 interfaceId
+    ) public view virtual override(ERC165, IERC165) returns (bool) {
+        return interfaceId == type(IEntity).interfaceId || super.supportsInterface(interfaceId);
+    }
+}