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

@@ -395,7 +395,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 +422,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 +458,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 +488,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);
     }
 

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,8 @@ 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);
 
     // Events
     /**

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);
+    }
+}

test/middleware/AvalancheL1MiddlewareTest.t.sol

@@ -1914,6 +1914,160 @@ contract AvalancheL1MiddlewareTest is Test {
         assertTrue(middleware.totalStakeCached(epochToTest, primaryAssetClass), "Stake remains cached");
     }
 
+    // function test_DustLimitStakeCausesFakeRebalancing() public {
+    //     address attacker = makeAddr("attacker");
+    //     address delegatedStaker = makeAddr("delegatedStaker");
+
+    //     _calcAndWarpOneEpoch();
+
+    //     // Step 1. First, give Alice a large allocation and create nodes
+    //     uint256 initialDeposit = 1000 ether;
+    //     (uint256 depositAmount, uint256 initialShares) = _deposit(delegatedStaker, initialDeposit);
+    //     console2.log("Initial deposit:", depositAmount);
+    //     console2.log("Initial shares:", initialShares);
+
+    //     // Set large L1 limit and give Alice all the shares initially
+    //     _setL1Limit(bob, validatorManagerAddress, assetClassId, depositAmount, delegator);
+    //     _setOperatorL1Shares(bob, validatorManagerAddress, assetClassId, alice, initialShares, delegator);
+
+    //     // Step 2. Create nodes that will use this stake
+    //     // move to next epoch
+    //     _calcAndWarpOneEpoch();
+    //     (, bytes32[] memory validationIDs,) = 
+    //         _createAndConfirmNodes(alice, 2, 0, true);
+
+    //     uint48 epoch2 = _calcAndWarpOneEpoch();
+
+    //     // Verify nodes have the stake
+    //     uint256 totalNodeStake = 0;
+    //     for (uint i = 0; i < validationIDs.length; i++) {
+    //         uint256 nodeStake = middleware.getNodeStake(epoch2, validationIDs[i]);
+    //         totalNodeStake += nodeStake;
+    //         console2.log("Node", i, "stake:", nodeStake);
+    //     }
+    //     console2.log("Total stake in nodes:", totalNodeStake); 
+
+    //     uint256 operatorTotalStake = middleware.getOperatorStake(alice, epoch2, assetClassId);
+    //     uint256 operatorUsedStake = middleware.getOperatorUsedStakeCached(alice);
+    //     console2.log("Operator total stake (from delegation):", operatorTotalStake);
+    //     console2.log("Operator used stake (in nodes):", operatorUsedStake);
+
+    //     // Step 3. Delegated staker withdraws, reducing Alice's available stake
+    //     console2.log("\n--- Delegated staker withdrawing 60% ---");
+    //     uint256 withdrawAmount = (initialDeposit * 60) / 100; // 600 ether
+    //     vm.startPrank(delegatedStaker);
+    //     (uint256 burnedShares, ) = vault.withdraw(delegatedStaker, withdrawAmount);
+    //     vm.stopPrank();        
+
+    //     console2.log("Withdrawn amount:", withdrawAmount);
+    //     console2.log("Burned shares:", burnedShares);
+    //     console2.log("Remaining shares for Alice:", initialShares - burnedShares);
+
+    //     // Step 4. Reduce Alice's operator shares to reflect the withdrawal
+    //     uint256 newOperatorShares = initialShares - burnedShares;
+    //     _setOperatorL1Shares(bob, validatorManagerAddress, assetClassId, alice, newOperatorShares, delegator);
+
+    //     console2.log("Updated Alice's operator shares to:", newOperatorShares);
+
+    //     // Step 5. Move to next epoch - this creates the imbalance
+    //     uint48 epoch3  = _calcAndWarpOneEpoch();
+
+    //     uint256 newOperatorTotalStake = middleware.getOperatorStake(alice, epoch3, assetClassId);
+    //     uint256 currentUsedStake = middleware.getOperatorUsedStakeCached(alice);
+
+    //     console2.log("\n--- After withdrawal (imbalance created) ---");
+    //     console2.log("Alice's new total stake (reduced):", newOperatorTotalStake);
+    //     console2.log("Alice's used stake (still in nodes):", currentUsedStake);                
+
+    //     // Step 6. Attacker prevents legitimate rebalancing
+    //     console2.log("\n--- ATTACKER PREVENTS REBALANCING ---");
+
+    //     // Move to final window where forceUpdateNodes can be called
+    //     _warpToLastHourOfCurrentEpoch();
+
+    //     // Attacker front-runs with dust limitStake attack
+    //     console2.log("Attacker executing dust forceUpdateNodes...");
+    //     vm.prank(attacker);
+    //     middleware.forceUpdateNodes(alice, 1); // 1 wei - minimal removal
+
+    //     // Check if any meaningful stake was actually removed
+    //     uint256 stakeAfterDustAttack = middleware.getOperatorUsedStakeCached(alice);
+    //     console2.log("Used stake after dust attack:", stakeAfterDustAttack);
+
+    //     uint256 actualRemoved = currentUsedStake > stakeAfterDustAttack ? 
+    //         currentUsedStake - stakeAfterDustAttack : 0;
+    //     console2.log("Stake actually removed by dust attack:", actualRemoved);   
+
+    //     // The key issue: minimal stake removed, but still excess remains
+    //     uint256 remainingExcess = stakeAfterDustAttack > newOperatorTotalStake ?
+    //         stakeAfterDustAttack - newOperatorTotalStake : 0;
+    //     console2.log("REMAINING EXCESS after dust attack:", remainingExcess);
+
+    //     // 7. Try legitimate rebalancing - should be blocked
+    //     console2.log("\n--- Attempting legitimate rebalancing ---");
+    //     vm.expectRevert(); // Should revert with AvalancheL1Middleware__AlreadyRebalanced
+    //     middleware.forceUpdateNodes(alice, 0); // Proper rebalancing with no limit
+    //     console2.log("Legitimate rebalancing blocked by AlreadyRebalanced");                                     
+    // }
+
+    function test_DustLimitStakeCausesFakeRebalancingFix() public {
+        address attacker = makeAddr("attacker");
+        address delegatedStaker = makeAddr("delegatedStaker");
+
+        _calcAndWarpOneEpoch();
+
+        // Setup initial stake and nodes
+        uint256 initialDeposit = 1000 ether;
+        (uint256 depositAmount, uint256 initialShares) = _deposit(delegatedStaker, initialDeposit);
+
+        _setL1Limit(bob, validatorManagerAddress, assetClassId, depositAmount, delegator);
+        _setOperatorL1Shares(bob, validatorManagerAddress, assetClassId, alice, initialShares, delegator);
+
+        _calcAndWarpOneEpoch();
+        (, bytes32[] memory validationIDs,) = _createAndConfirmNodes(alice, 2, 0, true);
+
+        uint48 epoch2 = _calcAndWarpOneEpoch();
+
+        // Verify node stakes
+        uint256 totalNodeStake = 0;
+        for (uint i = 0; i < validationIDs.length; i++) {
+            uint256 nodeStake = middleware.getNodeStake(epoch2, validationIDs[i]);
+            totalNodeStake += nodeStake;
+        }
+
+        middleware.getOperatorStake(alice, epoch2, assetClassId);
+        middleware.getOperatorUsedStakeCached(alice);
+
+        // Withdraw and update operator shares
+        uint256 withdrawAmount = (initialDeposit * 60) / 100;
+        vm.startPrank(delegatedStaker);
+        (uint256 burnedShares, ) = vault.withdraw(delegatedStaker, withdrawAmount);
+        vm.stopPrank();        
+
+        uint256 newOperatorShares = initialShares - burnedShares;
+        _setOperatorL1Shares(bob, validatorManagerAddress, assetClassId, alice, newOperatorShares, delegator);
+
+        uint48 epoch3 = _calcAndWarpOneEpoch();
+        middleware.calcAndCacheNodeStakeForAllOperators();
+
+        uint256 newOperatorTotalStake = middleware.getOperatorStake(alice, epoch3, assetClassId);
+        uint256 currentUsedStake = middleware.getOperatorUsedStakeCached(alice);
+
+        // Verify excess stake scenario
+        assertGt(newOperatorTotalStake, currentUsedStake, "Setup creates excess available stake");
+
+        _warpToLastHourOfCurrentEpoch();
+
+        // Test forceUpdateNodes behavior with excess stake
+        vm.prank(attacker);
+        middleware.forceUpdateNodes(alice, 1);
+
+        assertFalse(middleware.rebalancedThisEpoch(alice, epoch3), "No rebalancing flag set for excess stake");
+
+        middleware.forceUpdateNodes(alice, 0);
+        assertFalse(middleware.rebalancedThisEpoch(alice, epoch3), "Still no rebalancing flag");
+    }
+
     ///////////////////////////////
     // INTERNAL HELPERS
     ///////////////////////////////